Re: [PATCH 5/7] drm/exynos/decon5433: fix DECON standalone update

[Inki Dae <inki.dae@samsung.com>]

2016년 04월 12일 16:33에 Andrzej Hajda 이(가) 쓴 글:
> On 04/12/2016 04:25 AM, Inki Dae wrote:
>> Hi Andrzej,
>>
>> 2016년 03월 23일 22:15에 Andrzej Hajda 이(가) 쓴 글:
>>> DECON should be updated after un-protecting windows and after changing
>>> output parameters, otherwise image is not displayed in case of HDMI path.
>> I'm not sure why STANDALONE_UPDATE_F bit should be updated after un-protecting windows and changing output parameters.
>> The fields with _F prefix mean that these will be applied after vsync so we use protection window in case of all registers which affect display output so that they can be updated together.
>>
>> Wouldn't there be other thing which affects HDMI output?
>>
>> In addition, we would need another patch which updates TV relevant registers only in case of DECON-TV. DECON_UPDATE is a register for DECON-TV.
> 
> DECON_UPDATE is present in both DECON and DECON-TV and in both cases
> have the same field STANDALONE_UPDATE_F.
> 
> Documentation for 5433 says:
>> When you modify the shadow attributed registers, set
>> STANDALONE_UPDATE_F.
>>
> So it should be set after setting registers with _F suffix,
> but it has also _F suffix - contradiction. So I guess this _F suffix
> should not be treated too strictly in this case. Moreover the suffix
> has been removed in Exynos7420 - it is called just STANDALONE_UPDATE.

Indeed. I thought the register name has TV suffix so this register is for DECON-TV. However, without the register setting, DECON didn't work correctly - display not updated.


> 
> Anyway I am not sure what is exact purpose of this register and
> the changes proposed in the patch are rather results of multiple tests
> with hardware - documentation is rather poor on this subject.
> 
> I am also not sure why DECON works without it but DECON-TV does not,
> anyway I set it in both variants because documentation says so.

Ok, picked them up.

Thanks,
Inki Dae

> 
> Regards
> Andrzej
> 
>>
>> Thanks,
>> Inki Dae
>>
>>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>>> ---
>>>  drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 15 ++++++++-------
>>>  1 file changed, 8 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
>>> index ab9154e..7fec656 100644
>>> --- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
>>> +++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
>>> @@ -191,6 +191,8 @@ static void decon_commit(struct exynos_drm_crtc *crtc)
>>>  
>>>  	/* enable output and display signal */
>>>  	decon_set_bits(ctx, DECON_VIDCON0, VIDCON0_ENVID | VIDCON0_ENVID_F, ~0);
>>> +
>>> +	decon_set_bits(ctx, DECON_UPDATE, STANDALONE_UPDATE_F, ~0);
>>>  }
>>>  
>>>  static void decon_win_set_pixfmt(struct decon_context *ctx, unsigned int win,
>>> @@ -316,9 +318,6 @@ static void decon_update_plane(struct exynos_drm_crtc *crtc,
>>>  
>>>  	/* window enable */
>>>  	decon_set_bits(ctx, DECON_WINCONx(win), WINCONx_ENWIN_F, ~0);
>>> -
>>> -	/* standalone update */
>>> -	decon_set_bits(ctx, DECON_UPDATE, STANDALONE_UPDATE_F, ~0);
>>>  }
>>>  
>>>  static void decon_disable_plane(struct exynos_drm_crtc *crtc,
>>> @@ -336,9 +335,6 @@ static void decon_disable_plane(struct exynos_drm_crtc *crtc,
>>>  	decon_set_bits(ctx, DECON_WINCONx(win), WINCONx_ENWIN_F, 0);
>>>  
>>>  	decon_shadow_protect_win(ctx, win, false);
>>> -
>>> -	/* standalone update */
>>> -	decon_set_bits(ctx, DECON_UPDATE, STANDALONE_UPDATE_F, ~0);
>>>  }
>>>  
>>>  static void decon_atomic_flush(struct exynos_drm_crtc *crtc)
>>> @@ -352,6 +348,9 @@ static void decon_atomic_flush(struct exynos_drm_crtc *crtc)
>>>  	for (i = ctx->first_win; i < WINDOWS_NR; i++)
>>>  		decon_shadow_protect_win(ctx, i, false);
>>>  
>>> +	/* standalone update */
>>> +	decon_set_bits(ctx, DECON_UPDATE, STANDALONE_UPDATE_F, ~0);
>>> +
>>>  	if (ctx->out_type == IFTYPE_I80)
>>>  		set_bit(BIT_WIN_UPDATED, &ctx->flags);
>>>  }
>>> @@ -463,8 +462,10 @@ static void decon_clear_channels(struct exynos_drm_crtc *crtc)
>>>  		decon_shadow_protect_win(ctx, win, true);
>>>  		decon_set_bits(ctx, DECON_WINCONx(win), WINCONx_ENWIN_F, 0);
>>>  		decon_shadow_protect_win(ctx, win, false);
>>> -		decon_set_bits(ctx, DECON_UPDATE, STANDALONE_UPDATE_F, ~0);
>>>  	}
>>> +
>>> +	decon_set_bits(ctx, DECON_UPDATE, STANDALONE_UPDATE_F, ~0);
>>> +
>>>  	/* TODO: wait for possible vsync */
>>>  	msleep(50);
>>>  
>>>
> 
>