All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/exynos: remove hardware overlays disable from fimd probe
@ 2014-05-28  6:11 Rahul Sharma
  2014-06-02  9:11 ` Andrzej Hajda
  0 siblings, 1 reply; 12+ messages in thread
From: Rahul Sharma @ 2014-05-28  6:11 UTC (permalink / raw)
  To: dri-devel
  Cc: linux-samsung-soc, inki.dae, kgene.kim, joshi, r.sh.open,
	Rahul Sharma

System hangs when FIMD registers are accessed to disable
hardware overlays. This is because of the clocks which are
not enabled before register access.

'Hardware overlay disable' is cleaned from the FIMD probe.

Signed-off-by: Rahul Sharma <rahul.sharma@samsung.com>
---
Based on exynos-drm-next branch.

 drivers/gpu/drm/exynos/exynos_drm_fimd.c |   17 -----------------
 1 file changed, 17 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index bd30d0c..2ec634f 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -703,19 +703,6 @@ static void fimd_win_disable(struct exynos_drm_manager *mgr, int zpos)
 	win_data->enabled = false;
 }
 
-static void fimd_clear_win(struct fimd_context *ctx, int win)
-{
-	writel(0, ctx->regs + WINCON(win));
-	writel(0, ctx->regs + VIDOSD_A(win));
-	writel(0, ctx->regs + VIDOSD_B(win));
-	writel(0, ctx->regs + VIDOSD_C(win));
-
-	if (win == 1 || win == 2)
-		writel(0, ctx->regs + VIDOSD_D(win));
-
-	fimd_shadow_protect_win(ctx, win, false);
-}
-
 static void fimd_window_suspend(struct exynos_drm_manager *mgr)
 {
 	struct fimd_context *ctx = mgr->ctx;
@@ -898,16 +885,12 @@ static int fimd_bind(struct device *dev, struct device *master, void *data)
 {
 	struct fimd_context *ctx = fimd_manager.ctx;
 	struct drm_device *drm_dev = data;
-	int win;
 
 	fimd_mgr_initialize(&fimd_manager, drm_dev);
 	exynos_drm_crtc_create(&fimd_manager);
 	if (ctx->display)
 		exynos_drm_create_enc_conn(drm_dev, ctx->display);
 
-	for (win = 0; win < WINDOWS_NR; win++)
-		fimd_clear_win(ctx, win);
-
 	return 0;
 
 }
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm/exynos: remove hardware overlays disable from fimd probe
  2014-05-28  6:11 [PATCH] drm/exynos: remove hardware overlays disable from fimd probe Rahul Sharma
@ 2014-06-02  9:11 ` Andrzej Hajda
  2014-06-02  9:42   ` Rahul Sharma
  0 siblings, 1 reply; 12+ messages in thread
From: Andrzej Hajda @ 2014-06-02  9:11 UTC (permalink / raw)
  To: Rahul Sharma, dri-devel
  Cc: kgene.kim, joshi, linux-samsung-soc, Szyprowski, Marek

Hi Rahul,

On 05/28/2014 08:11 AM, Rahul Sharma wrote:
> System hangs when FIMD registers are accessed to disable
> hardware overlays. This is because of the clocks which are
> not enabled before register access.
> 
> 'Hardware overlay disable' is cleaned from the FIMD probe.
> 
> Signed-off-by: Rahul Sharma <rahul.sharma@samsung.com>

This patch causes regression on some exynos4210-universal_c210 devices,
everything works expect colors are incorrect - it seems blue component
is very dark, almost black.

Regards
Andrzej

> ---
> Based on exynos-drm-next branch.
> 
>  drivers/gpu/drm/exynos/exynos_drm_fimd.c |   17 -----------------
>  1 file changed, 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index bd30d0c..2ec634f 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -703,19 +703,6 @@ static void fimd_win_disable(struct exynos_drm_manager *mgr, int zpos)
>  	win_data->enabled = false;
>  }
>  
> -static void fimd_clear_win(struct fimd_context *ctx, int win)
> -{
> -	writel(0, ctx->regs + WINCON(win));
> -	writel(0, ctx->regs + VIDOSD_A(win));
> -	writel(0, ctx->regs + VIDOSD_B(win));
> -	writel(0, ctx->regs + VIDOSD_C(win));
> -
> -	if (win == 1 || win == 2)
> -		writel(0, ctx->regs + VIDOSD_D(win));
> -
> -	fimd_shadow_protect_win(ctx, win, false);
> -}
> -
>  static void fimd_window_suspend(struct exynos_drm_manager *mgr)
>  {
>  	struct fimd_context *ctx = mgr->ctx;
> @@ -898,16 +885,12 @@ static int fimd_bind(struct device *dev, struct device *master, void *data)
>  {
>  	struct fimd_context *ctx = fimd_manager.ctx;
>  	struct drm_device *drm_dev = data;
> -	int win;
>  
>  	fimd_mgr_initialize(&fimd_manager, drm_dev);
>  	exynos_drm_crtc_create(&fimd_manager);
>  	if (ctx->display)
>  		exynos_drm_create_enc_conn(drm_dev, ctx->display);
>  
> -	for (win = 0; win < WINDOWS_NR; win++)
> -		fimd_clear_win(ctx, win);
> -
>  	return 0;
>  
>  }
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm/exynos: remove hardware overlays disable from fimd probe
  2014-06-02  9:11 ` Andrzej Hajda
@ 2014-06-02  9:42   ` Rahul Sharma
  2014-06-02 10:11     ` Tomasz Figa
                       ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Rahul Sharma @ 2014-06-02  9:42 UTC (permalink / raw)
  To: Andrzej Hajda
  Cc: dri-devel@lists.freedesktop.org, linux-samsung-soc, Inki Dae,
	Kukjin Kim, sunil joshi, Szyprowski, Marek

On 2 June 2014 14:41, Andrzej Hajda <a.hajda@samsung.com> wrote:
> Hi Rahul,
>
> On 05/28/2014 08:11 AM, Rahul Sharma wrote:
>> System hangs when FIMD registers are accessed to disable
>> hardware overlays. This is because of the clocks which are
>> not enabled before register access.
>>
>> 'Hardware overlay disable' is cleaned from the FIMD probe.
>>
>> Signed-off-by: Rahul Sharma <rahul.sharma@samsung.com>
>
> This patch causes regression on some exynos4210-universal_c210 devices,
> everything works expect colors are incorrect - it seems blue component
> is very dark, almost black.
>

Oh.... Sorry for that. I did not see any problem on 5250/5420/5800. I do not
have setup for 4210. Better we should revert this patch.

Would you please help me by verifying the following patch on 4210? This
is an alternate solution to the same problem.

http://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg31426.html

Thanks Andrej, for bringing it to notice.

Regards,
Rahul Sharma

> Regards
> Andrzej
>
>> ---
>> Based on exynos-drm-next branch.
>>
>>  drivers/gpu/drm/exynos/exynos_drm_fimd.c |   17 -----------------
>>  1 file changed, 17 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> index bd30d0c..2ec634f 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> @@ -703,19 +703,6 @@ static void fimd_win_disable(struct exynos_drm_manager *mgr, int zpos)
>>       win_data->enabled = false;
>>  }
>>
>> -static void fimd_clear_win(struct fimd_context *ctx, int win)
>> -{
>> -     writel(0, ctx->regs + WINCON(win));
>> -     writel(0, ctx->regs + VIDOSD_A(win));
>> -     writel(0, ctx->regs + VIDOSD_B(win));
>> -     writel(0, ctx->regs + VIDOSD_C(win));
>> -
>> -     if (win == 1 || win == 2)
>> -             writel(0, ctx->regs + VIDOSD_D(win));
>> -
>> -     fimd_shadow_protect_win(ctx, win, false);
>> -}
>> -
>>  static void fimd_window_suspend(struct exynos_drm_manager *mgr)
>>  {
>>       struct fimd_context *ctx = mgr->ctx;
>> @@ -898,16 +885,12 @@ static int fimd_bind(struct device *dev, struct device *master, void *data)
>>  {
>>       struct fimd_context *ctx = fimd_manager.ctx;
>>       struct drm_device *drm_dev = data;
>> -     int win;
>>
>>       fimd_mgr_initialize(&fimd_manager, drm_dev);
>>       exynos_drm_crtc_create(&fimd_manager);
>>       if (ctx->display)
>>               exynos_drm_create_enc_conn(drm_dev, ctx->display);
>>
>> -     for (win = 0; win < WINDOWS_NR; win++)
>> -             fimd_clear_win(ctx, win);
>> -
>>       return 0;
>>
>>  }
>>
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm/exynos: remove hardware overlays disable from fimd probe
  2014-06-02  9:42   ` Rahul Sharma
@ 2014-06-02 10:11     ` Tomasz Figa
  2014-06-02 10:42       ` Andrzej Hajda
  2014-06-02 10:32     ` [PATCH] drm/exynos: remove hardware overlays disable from fimd probe YoungJun Cho
  2014-06-02 11:06     ` Inki Dae
  2 siblings, 1 reply; 12+ messages in thread
From: Tomasz Figa @ 2014-06-02 10:11 UTC (permalink / raw)
  To: Rahul Sharma, Andrzej Hajda
  Cc: Kukjin Kim, sunil joshi, dri-devel@lists.freedesktop.org,
	linux-samsung-soc, Szyprowski, Marek

Hi Rahul, Andrzej,

On 02.06.2014 11:42, Rahul Sharma wrote:
> On 2 June 2014 14:41, Andrzej Hajda <a.hajda@samsung.com> wrote:
>> Hi Rahul,
>>
>> On 05/28/2014 08:11 AM, Rahul Sharma wrote:
>>> System hangs when FIMD registers are accessed to disable
>>> hardware overlays. This is because of the clocks which are
>>> not enabled before register access.
>>>
>>> 'Hardware overlay disable' is cleaned from the FIMD probe.
>>>
>>> Signed-off-by: Rahul Sharma <rahul.sharma@samsung.com>
>>
>> This patch causes regression on some exynos4210-universal_c210 devices,
>> everything works expect colors are incorrect - it seems blue component
>> is very dark, almost black.
>>
> 
> Oh.... Sorry for that. I did not see any problem on 5250/5420/5800. I do not
> have setup for 4210. Better we should revert this patch.
> 
> Would you please help me by verifying the following patch on 4210? This
> is an alternate solution to the same problem.
> 
> http://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg31426.html
> 
> Thanks Andrej, for bringing it to notice.

I don't see how this patch could introduce such regression, as all the
affected registers seem to be properly reconfigured in fimd_win_commit()
anyway.

IMHO instead of reverting the patch, this issue should be investigated
and fixed properly.

Best regards,
Tomasz

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm/exynos: remove hardware overlays disable from fimd probe
  2014-06-02  9:42   ` Rahul Sharma
  2014-06-02 10:11     ` Tomasz Figa
@ 2014-06-02 10:32     ` YoungJun Cho
  2014-06-02 11:06     ` Inki Dae
  2 siblings, 0 replies; 12+ messages in thread
From: YoungJun Cho @ 2014-06-02 10:32 UTC (permalink / raw)
  To: Rahul Sharma, Andrzej Hajda
  Cc: linux-samsung-soc, Kukjin Kim, sunil joshi,
	dri-devel@lists.freedesktop.org, Szyprowski, Marek

Hi Rahul,

On 06/02/2014 06:42 PM, Rahul Sharma wrote:
> On 2 June 2014 14:41, Andrzej Hajda <a.hajda@samsung.com> wrote:
>> Hi Rahul,
>>
>> On 05/28/2014 08:11 AM, Rahul Sharma wrote:
>>> System hangs when FIMD registers are accessed to disable
>>> hardware overlays. This is because of the clocks which are
>>> not enabled before register access.
>>>
>>> 'Hardware overlay disable' is cleaned from the FIMD probe.
>>>
>>> Signed-off-by: Rahul Sharma <rahul.sharma@samsung.com>
>>
>> This patch causes regression on some exynos4210-universal_c210 devices,
>> everything works expect colors are incorrect - it seems blue component
>> is very dark, almost black.
>>
>
> Oh.... Sorry for that. I did not see any problem on 5250/5420/5800. I do not
> have setup for 4210. Better we should revert this patch.
>
> Would you please help me by verifying the following patch on 4210? This
> is an alternate solution to the same problem.
>
> http://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg31426.html

In my humble opinion, the patch is required to modify like below:

+       ret = pm_runtime_get_sync(ctx->dev);
+       if (ret < 0) {

When the bootloader enabled FIMD to show logo(boot image) or animation,
this function pm_runtime_get_sync() in fimd_bind() would return 1.

Thank you.
Best regards YJ

>
> Thanks Andrej, for bringing it to notice.
>
> Regards,
> Rahul Sharma
>
>> Regards
>> Andrzej
>>
>>> ---
>>> Based on exynos-drm-next branch.
>>>
>>>   drivers/gpu/drm/exynos/exynos_drm_fimd.c |   17 -----------------
>>>   1 file changed, 17 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>>> index bd30d0c..2ec634f 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>>> @@ -703,19 +703,6 @@ static void fimd_win_disable(struct exynos_drm_manager *mgr, int zpos)
>>>        win_data->enabled = false;
>>>   }
>>>
>>> -static void fimd_clear_win(struct fimd_context *ctx, int win)
>>> -{
>>> -     writel(0, ctx->regs + WINCON(win));
>>> -     writel(0, ctx->regs + VIDOSD_A(win));
>>> -     writel(0, ctx->regs + VIDOSD_B(win));
>>> -     writel(0, ctx->regs + VIDOSD_C(win));
>>> -
>>> -     if (win == 1 || win == 2)
>>> -             writel(0, ctx->regs + VIDOSD_D(win));
>>> -
>>> -     fimd_shadow_protect_win(ctx, win, false);
>>> -}
>>> -
>>>   static void fimd_window_suspend(struct exynos_drm_manager *mgr)
>>>   {
>>>        struct fimd_context *ctx = mgr->ctx;
>>> @@ -898,16 +885,12 @@ static int fimd_bind(struct device *dev, struct device *master, void *data)
>>>   {
>>>        struct fimd_context *ctx = fimd_manager.ctx;
>>>        struct drm_device *drm_dev = data;
>>> -     int win;
>>>
>>>        fimd_mgr_initialize(&fimd_manager, drm_dev);
>>>        exynos_drm_crtc_create(&fimd_manager);
>>>        if (ctx->display)
>>>                exynos_drm_create_enc_conn(drm_dev, ctx->display);
>>>
>>> -     for (win = 0; win < WINDOWS_NR; win++)
>>> -             fimd_clear_win(ctx, win);
>>> -
>>>        return 0;
>>>
>>>   }
>>>
>>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm/exynos: remove hardware overlays disable from fimd probe
  2014-06-02 10:11     ` Tomasz Figa
@ 2014-06-02 10:42       ` Andrzej Hajda
  2014-06-02 11:48         ` Andrzej Hajda
  0 siblings, 1 reply; 12+ messages in thread
From: Andrzej Hajda @ 2014-06-02 10:42 UTC (permalink / raw)
  To: Tomasz Figa, Rahul Sharma
  Cc: Kukjin Kim, sunil joshi, dri-devel@lists.freedesktop.org,
	linux-samsung-soc, Szyprowski, Marek

On 06/02/2014 12:11 PM, Tomasz Figa wrote:
> Hi Rahul, Andrzej,
> 
> On 02.06.2014 11:42, Rahul Sharma wrote:
>> On 2 June 2014 14:41, Andrzej Hajda <a.hajda@samsung.com> wrote:
>>> Hi Rahul,
>>>
>>> On 05/28/2014 08:11 AM, Rahul Sharma wrote:
>>>> System hangs when FIMD registers are accessed to disable
>>>> hardware overlays. This is because of the clocks which are
>>>> not enabled before register access.
>>>>
>>>> 'Hardware overlay disable' is cleaned from the FIMD probe.
>>>>
>>>> Signed-off-by: Rahul Sharma <rahul.sharma@samsung.com>
>>>
>>> This patch causes regression on some exynos4210-universal_c210 devices,
>>> everything works expect colors are incorrect - it seems blue component
>>> is very dark, almost black.
>>>
>>
>> Oh.... Sorry for that. I did not see any problem on 5250/5420/5800. I do not
>> have setup for 4210. Better we should revert this patch.
>>
>> Would you please help me by verifying the following patch on 4210? This
>> is an alternate solution to the same problem.
>>
>> http://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg31426.html
>>
>> Thanks Andrej, for bringing it to notice.
> 
> I don't see how this patch could introduce such regression, as all the
> affected registers seem to be properly reconfigured in fimd_win_commit()
> anyway.
> 
> IMHO instead of reverting the patch, this issue should be investigated
> and fixed properly.
> 
> Best regards,
> Tomasz
> 

I am looking at the problem, it is quite strange as it happens only on
one of two targets I have access to. Anyway it seems that something
should be added to fimd initialization sequence if we want to remove hw
accessing code from probe.

Regards
Andrzej

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm/exynos: remove hardware overlays disable from fimd probe
  2014-06-02  9:42   ` Rahul Sharma
  2014-06-02 10:11     ` Tomasz Figa
  2014-06-02 10:32     ` [PATCH] drm/exynos: remove hardware overlays disable from fimd probe YoungJun Cho
@ 2014-06-02 11:06     ` Inki Dae
  2 siblings, 0 replies; 12+ messages in thread
From: Inki Dae @ 2014-06-02 11:06 UTC (permalink / raw)
  To: Rahul Sharma
  Cc: linux-samsung-soc, sunil joshi, dri-devel@lists.freedesktop.org,
	Andrzej Hajda, Kukjin Kim, Szyprowski, Marek

On 2014년 06월 02일 18:42, Rahul Sharma wrote:
> On 2 June 2014 14:41, Andrzej Hajda <a.hajda@samsung.com> wrote:
>> Hi Rahul,
>>
>> On 05/28/2014 08:11 AM, Rahul Sharma wrote:
>>> System hangs when FIMD registers are accessed to disable
>>> hardware overlays. This is because of the clocks which are
>>> not enabled before register access.
>>>
>>> 'Hardware overlay disable' is cleaned from the FIMD probe.
>>>
>>> Signed-off-by: Rahul Sharma <rahul.sharma@samsung.com>
>>
>> This patch causes regression on some exynos4210-universal_c210 devices,
>> everything works expect colors are incorrect - it seems blue component
>> is very dark, almost black.

That issue would be because your boot-loader enabled a hardware overlay
but kernel driver enables another hardware overlay, which would also be
why it happens only on one target of your targets, and why it works
correctly by cleaning hardware overlays at fimd probe.

>>
> 
> Oh.... Sorry for that. I did not see any problem on 5250/5420/5800. I do not
> have setup for 4210. Better we should revert this patch.
> 

No, it should work correctly as is because fimd overlays will be
reconfigured at booting time by register_framebuffer() if console
framebuffer is enabled. Otherwise, a master process, X or Weston, will
reconfigure fimd overlays after kernel booting.

Thanks,
Inki Dae

> Would you please help me by verifying the following patch on 4210? This
> is an alternate solution to the same problem.
> 
> http://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg31426.html
> 
> Thanks Andrej, for bringing it to notice.
> 
> Regards,
> Rahul Sharma
> 
>> Regards
>> Andrzej
>>
>>> ---
>>> Based on exynos-drm-next branch.
>>>
>>>  drivers/gpu/drm/exynos/exynos_drm_fimd.c |   17 -----------------
>>>  1 file changed, 17 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>>> index bd30d0c..2ec634f 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>>> @@ -703,19 +703,6 @@ static void fimd_win_disable(struct exynos_drm_manager *mgr, int zpos)
>>>       win_data->enabled = false;
>>>  }
>>>
>>> -static void fimd_clear_win(struct fimd_context *ctx, int win)
>>> -{
>>> -     writel(0, ctx->regs + WINCON(win));
>>> -     writel(0, ctx->regs + VIDOSD_A(win));
>>> -     writel(0, ctx->regs + VIDOSD_B(win));
>>> -     writel(0, ctx->regs + VIDOSD_C(win));
>>> -
>>> -     if (win == 1 || win == 2)
>>> -             writel(0, ctx->regs + VIDOSD_D(win));
>>> -
>>> -     fimd_shadow_protect_win(ctx, win, false);
>>> -}
>>> -
>>>  static void fimd_window_suspend(struct exynos_drm_manager *mgr)
>>>  {
>>>       struct fimd_context *ctx = mgr->ctx;
>>> @@ -898,16 +885,12 @@ static int fimd_bind(struct device *dev, struct device *master, void *data)
>>>  {
>>>       struct fimd_context *ctx = fimd_manager.ctx;
>>>       struct drm_device *drm_dev = data;
>>> -     int win;
>>>
>>>       fimd_mgr_initialize(&fimd_manager, drm_dev);
>>>       exynos_drm_crtc_create(&fimd_manager);
>>>       if (ctx->display)
>>>               exynos_drm_create_enc_conn(drm_dev, ctx->display);
>>>
>>> -     for (win = 0; win < WINDOWS_NR; win++)
>>> -             fimd_clear_win(ctx, win);
>>> -
>>>       return 0;
>>>
>>>  }
>>>
>>
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm/exynos: remove hardware overlays disable from fimd probe
  2014-06-02 10:42       ` Andrzej Hajda
@ 2014-06-02 11:48         ` Andrzej Hajda
  2014-06-09  4:13           ` Inki Dae
  0 siblings, 1 reply; 12+ messages in thread
From: Andrzej Hajda @ 2014-06-02 11:48 UTC (permalink / raw)
  To: Tomasz Figa, Rahul Sharma
  Cc: linux-samsung-soc, sunil joshi, dri-devel@lists.freedesktop.org,
	Kukjin Kim, Szyprowski, Marek

On 06/02/2014 12:42 PM, Andrzej Hajda wrote:
> On 06/02/2014 12:11 PM, Tomasz Figa wrote:
>> Hi Rahul, Andrzej,
>>
>> On 02.06.2014 11:42, Rahul Sharma wrote:
>>> On 2 June 2014 14:41, Andrzej Hajda <a.hajda@samsung.com> wrote:
>>>> Hi Rahul,
>>>>
>>>> On 05/28/2014 08:11 AM, Rahul Sharma wrote:
>>>>> System hangs when FIMD registers are accessed to disable
>>>>> hardware overlays. This is because of the clocks which are
>>>>> not enabled before register access.
>>>>>
>>>>> 'Hardware overlay disable' is cleaned from the FIMD probe.
>>>>>
>>>>> Signed-off-by: Rahul Sharma <rahul.sharma@samsung.com>
>>>>
>>>> This patch causes regression on some exynos4210-universal_c210 devices,
>>>> everything works expect colors are incorrect - it seems blue component
>>>> is very dark, almost black.
>>>>
>>>
>>> Oh.... Sorry for that. I did not see any problem on 5250/5420/5800. I do not
>>> have setup for 4210. Better we should revert this patch.
>>>
>>> Would you please help me by verifying the following patch on 4210? This
>>> is an alternate solution to the same problem.
>>>
>>> http://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg31426.html
>>>
>>> Thanks Andrej, for bringing it to notice.
>>
>> I don't see how this patch could introduce such regression, as all the
>> affected registers seem to be properly reconfigured in fimd_win_commit()
>> anyway.
>>
>> IMHO instead of reverting the patch, this issue should be investigated
>> and fixed properly.
>>
>> Best regards,
>> Tomasz
>>
> 
> I am looking at the problem, it is quite strange as it happens only on
> one of two targets I have access to. Anyway it seems that something
> should be added to fimd initialization sequence if we want to remove hw
> accessing code from probe.

The problem is that fimd does not clear unused windows.
Simple patch which helps:

diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index 2ec634f..b58fce2 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -741,6 +741,8 @@ static void fimd_apply(struct exynos_drm_manager *mgr)
                win_data = &ctx->win_data[i];
                if (win_data->enabled)
                        fimd_win_commit(mgr, i);
+               else
+                       fimd_win_disable(mgr, i);
        }

        fimd_commit(mgr);

But I am not fully familiar with window management code, so I do not
know if it does not breaks other stuff.

Regards
Andrzej

> 
> Regards
> Andrzej
> 

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm/exynos: remove hardware overlays disable from fimd probe
  2014-06-02 11:48         ` Andrzej Hajda
@ 2014-06-09  4:13           ` Inki Dae
  2014-06-09 14:10             ` [PATCH] drm/exynos: disable unused windows on apply Andrzej Hajda
  0 siblings, 1 reply; 12+ messages in thread
From: Inki Dae @ 2014-06-09  4:13 UTC (permalink / raw)
  To: Andrzej Hajda
  Cc: Tomasz Figa, Rahul Sharma, Kukjin Kim, sunil joshi,
	dri-devel@lists.freedesktop.org, linux-samsung-soc,
	Szyprowski, Marek

On 2014년 06월 02일 20:48, Andrzej Hajda wrote:
> On 06/02/2014 12:42 PM, Andrzej Hajda wrote:
>> On 06/02/2014 12:11 PM, Tomasz Figa wrote:
>>> Hi Rahul, Andrzej,
>>>
>>> On 02.06.2014 11:42, Rahul Sharma wrote:
>>>> On 2 June 2014 14:41, Andrzej Hajda <a.hajda@samsung.com> wrote:
>>>>> Hi Rahul,
>>>>>
>>>>> On 05/28/2014 08:11 AM, Rahul Sharma wrote:
>>>>>> System hangs when FIMD registers are accessed to disable
>>>>>> hardware overlays. This is because of the clocks which are
>>>>>> not enabled before register access.
>>>>>>
>>>>>> 'Hardware overlay disable' is cleaned from the FIMD probe.
>>>>>>
>>>>>> Signed-off-by: Rahul Sharma <rahul.sharma@samsung.com>
>>>>>
>>>>> This patch causes regression on some exynos4210-universal_c210 devices,
>>>>> everything works expect colors are incorrect - it seems blue component
>>>>> is very dark, almost black.
>>>>>
>>>>
>>>> Oh.... Sorry for that. I did not see any problem on 5250/5420/5800. I do not
>>>> have setup for 4210. Better we should revert this patch.
>>>>
>>>> Would you please help me by verifying the following patch on 4210? This
>>>> is an alternate solution to the same problem.
>>>>
>>>> http://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg31426.html
>>>>
>>>> Thanks Andrej, for bringing it to notice.
>>>
>>> I don't see how this patch could introduce such regression, as all the
>>> affected registers seem to be properly reconfigured in fimd_win_commit()
>>> anyway.
>>>
>>> IMHO instead of reverting the patch, this issue should be investigated
>>> and fixed properly.
>>>
>>> Best regards,
>>> Tomasz
>>>
>>
>> I am looking at the problem, it is quite strange as it happens only on
>> one of two targets I have access to. Anyway it seems that something
>> should be added to fimd initialization sequence if we want to remove hw
>> accessing code from probe.
> 
> The problem is that fimd does not clear unused windows.
> Simple patch which helps:
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index 2ec634f..b58fce2 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -741,6 +741,8 @@ static void fimd_apply(struct exynos_drm_manager *mgr)
>                 win_data = &ctx->win_data[i];
>                 if (win_data->enabled)
>                         fimd_win_commit(mgr, i);
> +               else
> +                       fimd_win_disable(mgr, i);
>         }
> 
>         fimd_commit(mgr);
> 
> But I am not fully familiar with window management code, so I do not
> know if it does not breaks other stuff.

It looks good to me. Can you post it?

Thanks,
Inki Dae

> 
> Regards
> Andrzej
> 
>>
>> Regards
>> Andrzej
>>
> 
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] drm/exynos: disable unused windows on apply
  2014-06-09  4:13           ` Inki Dae
@ 2014-06-09 14:10             ` Andrzej Hajda
  2014-06-23 12:12               ` Andrzej Hajda
  0 siblings, 1 reply; 12+ messages in thread
From: Andrzej Hajda @ 2014-06-09 14:10 UTC (permalink / raw)
  To: inki.dae; +Cc: Andrzej Hajda, dri-devel

The patch disables non-enabled HW windows on applying
configuration, it will allow to clear windows enabled
by bootloader.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_fimd.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index bb45ab2..33161ad 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -741,6 +741,8 @@ static void fimd_apply(struct exynos_drm_manager *mgr)
 		win_data = &ctx->win_data[i];
 		if (win_data->enabled)
 			fimd_win_commit(mgr, i);
+		else
+			fimd_win_disable(mgr, i);
 	}
 
 	fimd_commit(mgr);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm/exynos: disable unused windows on apply
  2014-06-09 14:10             ` [PATCH] drm/exynos: disable unused windows on apply Andrzej Hajda
@ 2014-06-23 12:12               ` Andrzej Hajda
  2014-06-23 12:43                 ` Inki Dae
  0 siblings, 1 reply; 12+ messages in thread
From: Andrzej Hajda @ 2014-06-23 12:12 UTC (permalink / raw)
  To: inki.dae; +Cc: dri-devel

Gently ping.

Regards
Andrzej

On 06/09/2014 04:10 PM, Andrzej Hajda wrote:
> The patch disables non-enabled HW windows on applying
> configuration, it will allow to clear windows enabled
> by bootloader.
>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_fimd.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index bb45ab2..33161ad 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -741,6 +741,8 @@ static void fimd_apply(struct exynos_drm_manager *mgr)
>  		win_data = &ctx->win_data[i];
>  		if (win_data->enabled)
>  			fimd_win_commit(mgr, i);
> +		else
> +			fimd_win_disable(mgr, i);
>  	}
>  
>  	fimd_commit(mgr);

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm/exynos: disable unused windows on apply
  2014-06-23 12:12               ` Andrzej Hajda
@ 2014-06-23 12:43                 ` Inki Dae
  0 siblings, 0 replies; 12+ messages in thread
From: Inki Dae @ 2014-06-23 12:43 UTC (permalink / raw)
  To: Andrzej Hajda; +Cc: dri-devel

On 2014년 06월 23일 21:12, Andrzej Hajda wrote:
> Gently ping.


Oops, sorry. Applied.

Thanks,
Inki Dae

> 
> Regards
> Andrzej
> 
> On 06/09/2014 04:10 PM, Andrzej Hajda wrote:
>> The patch disables non-enabled HW windows on applying
>> configuration, it will allow to clear windows enabled
>> by bootloader.
>>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> ---
>>  drivers/gpu/drm/exynos/exynos_drm_fimd.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> index bb45ab2..33161ad 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> @@ -741,6 +741,8 @@ static void fimd_apply(struct exynos_drm_manager *mgr)
>>  		win_data = &ctx->win_data[i];
>>  		if (win_data->enabled)
>>  			fimd_win_commit(mgr, i);
>> +		else
>> +			fimd_win_disable(mgr, i);
>>  	}
>>  
>>  	fimd_commit(mgr);
> 
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2014-06-23 12:43 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-05-28  6:11 [PATCH] drm/exynos: remove hardware overlays disable from fimd probe Rahul Sharma
2014-06-02  9:11 ` Andrzej Hajda
2014-06-02  9:42   ` Rahul Sharma
2014-06-02 10:11     ` Tomasz Figa
2014-06-02 10:42       ` Andrzej Hajda
2014-06-02 11:48         ` Andrzej Hajda
2014-06-09  4:13           ` Inki Dae
2014-06-09 14:10             ` [PATCH] drm/exynos: disable unused windows on apply Andrzej Hajda
2014-06-23 12:12               ` Andrzej Hajda
2014-06-23 12:43                 ` Inki Dae
2014-06-02 10:32     ` [PATCH] drm/exynos: remove hardware overlays disable from fimd probe YoungJun Cho
2014-06-02 11:06     ` Inki Dae

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.