diff mbox

[6/7] drm/exynos: dsi: move TE irq handler registration position

Message ID 1412144353-13114-7-git-send-email-yj44.cho@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

YoungJun Cho Oct. 1, 2014, 6:19 a.m. UTC 
The drm_helper_hpd_irq_event() does dpms control and panel is
initialized and displayed on by it.
So should register TE irq handler(exynos_dsi_te_irq_handler())
beforehand.

Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
Acked-by: Inki Dae <inki.dae@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

Comments

Inki Dae Nov. 14, 2014, 1:49 a.m. UTC | #1 
On 2014? 10? 01? 15:19, YoungJun Cho wrote:
> The drm_helper_hpd_irq_event() does dpms control and panel is
> initialized and displayed on by it.
> So should register TE irq handler(exynos_dsi_te_irq_handler())
> beforehand.

This patch also includes some codes not relevant to register TE irq
handler before drm_helper_hpd_irq_event call. How about separating this
patch into two patches?

And below is my comment.

Thanks,
Inki Dae

> 
> Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
> Acked-by: Inki Dae <inki.dae@samsung.com>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_dsi.c | 19 ++++++++-----------
>  1 file changed, 8 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> index 24741d8..ded69df 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> @@ -1143,6 +1143,7 @@ static int exynos_dsi_init(struct exynos_dsi *dsi)
>  static int exynos_dsi_register_te_irq(struct exynos_dsi *dsi)
>  {
>  	int ret;
> +	int te_gpio_irq;
>  
>  	dsi->te_gpio = of_get_named_gpio(dsi->panel_node, "te-gpios", 0);
>  	if (!gpio_is_valid(dsi->te_gpio)) {
> @@ -1157,14 +1158,10 @@ static int exynos_dsi_register_te_irq(struct exynos_dsi *dsi)
>  		goto out;
>  	}
>  
> -	/*
> -	 * This TE GPIO IRQ should not be set to IRQ_NOAUTOEN, because panel
> -	 * calls drm_panel_init() first then calls mipi_dsi_attach() in probe().
> -	 * It means that te_gpio is invalid when exynos_dsi_enable_irq() is
> -	 * called by drm_panel_init() before panel is attached.
> -	 */
> -	ret = request_threaded_irq(gpio_to_irq(dsi->te_gpio),
> -					exynos_dsi_te_irq_handler, NULL,
> +	te_gpio_irq = gpio_to_irq(dsi->te_gpio);
> +
> +	irq_set_status_flags(te_gpio_irq, IRQ_NOAUTOEN);

I think with IRQ_NOAUTOEN, te irq wouldn't be enabled automatically. So
shouldn't we call enable_irq() and disable_irq() somewhere?

> +	ret = request_threaded_irq(te_gpio_irq, exynos_dsi_te_irq_handler, NULL,
>  					IRQF_TRIGGER_RISING, "TE", dsi);
>  	if (ret) {
>  		dev_err(dsi->dev, "request interrupt failed with %d\n", ret);
> @@ -1195,9 +1192,6 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
>  	dsi->mode_flags = device->mode_flags;
>  	dsi->panel_node = device->dev.of_node;
>  
> -	if (dsi->connector.dev)
> -		drm_helper_hpd_irq_event(dsi->connector.dev);
> -
>  	/*
>  	 * This is a temporary solution and should be made by more generic way.
>  	 *
> @@ -1211,6 +1205,9 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
>  			return ret;
>  	}
>  
> +	if (dsi->connector.dev)
> +		drm_helper_hpd_irq_event(dsi->connector.dev);
> +
>  	return 0;
>  }
>  
>
YoungJun Cho Nov. 14, 2014, 2:08 a.m. UTC | #2 
Hi Inki,

On 11/14/2014 10:49 AM, Inki Dae wrote:
> On 2014? 10? 01? 15:19, YoungJun Cho wrote:
>> The drm_helper_hpd_irq_event() does dpms control and panel is
>> initialized and displayed on by it.
>> So should register TE irq handler(exynos_dsi_te_irq_handler())
>> beforehand.
>
> This patch also includes some codes not relevant to register TE irq
> handler before drm_helper_hpd_irq_event call. How about separating this
> patch into two patches?
>
> And below is my comment.
>
> Thanks,
> Inki Dae
>
>>
>> Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
>> Acked-by: Inki Dae <inki.dae@samsung.com>
>> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
>> ---
>>   drivers/gpu/drm/exynos/exynos_drm_dsi.c | 19 ++++++++-----------
>>   1 file changed, 8 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>> index 24741d8..ded69df 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>> @@ -1143,6 +1143,7 @@ static int exynos_dsi_init(struct exynos_dsi *dsi)
>>   static int exynos_dsi_register_te_irq(struct exynos_dsi *dsi)
>>   {
>>   	int ret;
>> +	int te_gpio_irq;
>>
>>   	dsi->te_gpio = of_get_named_gpio(dsi->panel_node, "te-gpios", 0);
>>   	if (!gpio_is_valid(dsi->te_gpio)) {
>> @@ -1157,14 +1158,10 @@ static int exynos_dsi_register_te_irq(struct exynos_dsi *dsi)
>>   		goto out;
>>   	}
>>
>> -	/*
>> -	 * This TE GPIO IRQ should not be set to IRQ_NOAUTOEN, because panel
>> -	 * calls drm_panel_init() first then calls mipi_dsi_attach() in probe().
>> -	 * It means that te_gpio is invalid when exynos_dsi_enable_irq() is
>> -	 * called by drm_panel_init() before panel is attached.
>> -	 */
>> -	ret = request_threaded_irq(gpio_to_irq(dsi->te_gpio),
>> -					exynos_dsi_te_irq_handler, NULL,
>> +	te_gpio_irq = gpio_to_irq(dsi->te_gpio);
>> +
>> +	irq_set_status_flags(te_gpio_irq, IRQ_NOAUTOEN);
>
> I think with IRQ_NOAUTOEN, te irq wouldn't be enabled automatically. So
> shouldn't we call enable_irq() and disable_irq() somewhere?

The te_gpio irq triggering is done by exynos_dsi_enable_irq() and 
exynos_dsi_disable_irq() like dsi irq.

And I'll separate patch with others also.

Thank you.
Best regards YJ

>
>> +	ret = request_threaded_irq(te_gpio_irq, exynos_dsi_te_irq_handler, NULL,
>>   					IRQF_TRIGGER_RISING, "TE", dsi);
>>   	if (ret) {
>>   		dev_err(dsi->dev, "request interrupt failed with %d\n", ret);
>> @@ -1195,9 +1192,6 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
>>   	dsi->mode_flags = device->mode_flags;
>>   	dsi->panel_node = device->dev.of_node;
>>
>> -	if (dsi->connector.dev)
>> -		drm_helper_hpd_irq_event(dsi->connector.dev);
>> -
>>   	/*
>>   	 * This is a temporary solution and should be made by more generic way.
>>   	 *
>> @@ -1211,6 +1205,9 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
>>   			return ret;
>>   	}
>>
>> +	if (dsi->connector.dev)
>> +		drm_helper_hpd_irq_event(dsi->connector.dev);
>> +
>>   	return 0;
>>   }
>>
>>
>
>
diff mbox

Patch

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index 24741d8..ded69df 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -1143,6 +1143,7 @@  static int exynos_dsi_init(struct exynos_dsi *dsi)
 static int exynos_dsi_register_te_irq(struct exynos_dsi *dsi)
 {
 	int ret;
+	int te_gpio_irq;
 
 	dsi->te_gpio = of_get_named_gpio(dsi->panel_node, "te-gpios", 0);
 	if (!gpio_is_valid(dsi->te_gpio)) {
@@ -1157,14 +1158,10 @@  static int exynos_dsi_register_te_irq(struct exynos_dsi *dsi)
 		goto out;
 	}
 
-	/*
-	 * This TE GPIO IRQ should not be set to IRQ_NOAUTOEN, because panel
-	 * calls drm_panel_init() first then calls mipi_dsi_attach() in probe().
-	 * It means that te_gpio is invalid when exynos_dsi_enable_irq() is
-	 * called by drm_panel_init() before panel is attached.
-	 */
-	ret = request_threaded_irq(gpio_to_irq(dsi->te_gpio),
-					exynos_dsi_te_irq_handler, NULL,
+	te_gpio_irq = gpio_to_irq(dsi->te_gpio);
+
+	irq_set_status_flags(te_gpio_irq, IRQ_NOAUTOEN);
+	ret = request_threaded_irq(te_gpio_irq, exynos_dsi_te_irq_handler, NULL,
 					IRQF_TRIGGER_RISING, "TE", dsi);
 	if (ret) {
 		dev_err(dsi->dev, "request interrupt failed with %d\n", ret);
@@ -1195,9 +1192,6 @@  static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
 	dsi->mode_flags = device->mode_flags;
 	dsi->panel_node = device->dev.of_node;
 
-	if (dsi->connector.dev)
-		drm_helper_hpd_irq_event(dsi->connector.dev);
-
 	/*
 	 * This is a temporary solution and should be made by more generic way.
 	 *
@@ -1211,6 +1205,9 @@  static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
 			return ret;
 	}
 
+	if (dsi->connector.dev)
+		drm_helper_hpd_irq_event(dsi->connector.dev);
+
 	return 0;
 }