Re: [PATCH 02/18] drm/exynos: use wait_event_timeout() for safety usage

[YoungJun Cho <yj44.cho@samsung.com>]

Hi Daniel

Thank you for comments.

On 05/20/2014 07:02 PM, Daniel Kurtz wrote:
> On Wed, May 14, 2014 at 2:26 PM, YoungJun Cho <yj44.cho@samsung.com> wrote:
>>
>> There could be the case that the page flip operation isn't finished correctly
>> with some abnormal condition such as panel reset. So this patch replaces
>> wait_event() with wait_event_timeout() to avoid waiting for page flip completion
>> infinitely.
>
>
> This solution looks like we aren't really handling some conditions
> completely (such as "panel reset"?). In cases where this timeout can
> occur, shouldn't we be clearing pending_flip when we detect that the
> flip will never complete?
>

Nice catch!.

The others except pending_flip issue are done in drm_vblank_off() 
instead of exynos_drm_crtc_finish_pageflip() at this case.

I'll do it for this routine and exynos_drm_crtc_page_flip() also.
There is a leak point in exynos_drm_crtc_page_flip() when 
exynos_drm_crtc_mode_set_commit() is failed.

Thank you.
Best regards YJ

> -Dan
>
>>
>>
>> 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_crtc.c |    5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
>> index 95c9435..485fa26 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
>> @@ -69,8 +69,9 @@ static void exynos_drm_crtc_dpms(struct drm_crtc *crtc, int mode)
>>
>>          if (mode > DRM_MODE_DPMS_ON) {
>>                  /* wait for the completion of page flip. */
>> -               wait_event(exynos_crtc->pending_flip_queue,
>> -                               atomic_read(&exynos_crtc->pending_flip) == 0);
>> +               wait_event_timeout(exynos_crtc->pending_flip_queue,
>> +                               !atomic_read(&exynos_crtc->pending_flip),
>> +                               HZ/20);
>>                  drm_vblank_off(crtc->dev, exynos_crtc->pipe);
>>          }
>>
>> --
>> 1.7.9.5
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>