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

[Daniel Kurtz]

On Wed, May 21, 2014 at 2:28 PM, YoungJun Cho <yj44.cho at samsung.com> wrote:
> Hi Daniel
>
>
> On 05/21/2014 03:01 PM, Daniel Kurtz wrote:
>>
>> On Wed, May 21, 2014 at 12:42 PM, YoungJun Cho <yj44.cho at 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.
>>> And clears exynos_crtc->pending_flip in exynos_drm_crtc_page_flip() when
>>> exynos_drm_crtc_mode_set_commit() is failed.
>>>
>>> Signed-off-by: YoungJun Cho <yj44.cho at samsung.com>
>>> Acked-by: Inki Dae <inki.dae at samsung.com>
>>> Acked-by: Kyungmin Park <kyungmin.park at samsung.com>
>>> ---
>>>   drivers/gpu/drm/exynos/exynos_drm_crtc.c |    7 +++++--
>>>   1 file changed, 5 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..3bf091d 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
>>> @@ -69,8 +69,10 @@ 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);
>>> +               if (!wait_event_timeout(exynos_crtc->pending_flip_queue,
>>> +                               !atomic_read(&exynos_crtc->pending_flip),
>>> +                               HZ/20))
>>> +                       atomic_set(&exynos_crtc->pending_flip, 0);
>>
>>
>> I meant that changing this to wait_event_timeout() seems to be masking
>> the original problem, that pending_flip wasn't being cleared.
>
>
> Yes, I agree.
>
> The original purpose of this patch is to avoid lock-up during modetest (with
> test_vsync) with command mode panel.
> In MIPI DSI command mode interface, the display controller can not generate
> VSYNC signal and uses TE signal instead which is generated by panel.
> If there is an abnormal power off or reset condition, it is possible that
> the display controller misses the TE signal and it makes lock-up.
> So I needed this patch.

If flips are not completing on MIPI DSI, may I recommend that you
start a timer when scheduling such a flip in the MIPI DSI driver that
will expire after a timeout.  In that timer's expiration handler, you
can then go through the normal process of completing a flip.  This
would make the timeout transparent to the generic exynos_drm_crtc
layer, which can then do its normal flip complete processing: send
pending vblank events, put vblank, clear pending_flip, etc.
Does that make sense?
Is it possible?

-Dan

>> Now that you are now clearing pending_flip in the error path, you
>> don't need the timeout, right?
>>
>
> There might be my missing point. Would you explain more detail?
>
>
> Thank you.
> Best regards YJ
>
>> -Dan
>>
>>>                  drm_vblank_off(crtc->dev, exynos_crtc->pipe);
>>>          }
>>>
>>> @@ -259,6 +261,7 @@ static int exynos_drm_crtc_page_flip(struct drm_crtc
>>> *crtc,
>>>                          spin_lock_irq(&dev->event_lock);
>>>                          drm_vblank_put(dev, exynos_crtc->pipe);
>>>                          list_del(&event->base.link);
>>> +                       atomic_set(&exynos_crtc->pending_flip, 0);
>>>                          spin_unlock_irq(&dev->event_lock);
>>>
>>>                          goto out;
>>> --
>>> 1.7.9.5
>>>
>>
>