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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Hi Daniel,

On 05/21/2014 03:45 PM, Daniel Kurtz wrote:
On Wed, May 21, 2014 at 2:28 PM, YoungJun Cho <yj44.cho@xxxxxxxxxxx> 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@xxxxxxxxxxx>
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@xxxxxxxxxxx>
Acked-by: Inki Dae <inki.dae@xxxxxxxxxxx>
Acked-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx>
---
   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

At first, I omitted the keyword "panel" at previous reply.
The abnormal power off or reset condition could be happened in panel
which generates TE signal.

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

The MIPI DSI doesn't take care page flip operation at all.
And the exynos_drm_crtc_finish_page_flip() is called only by interrupt handler of display controllers.
So there is no suitable place to set time-out for this.

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?

This function is exynos_drm_crtc_dpms() and my patch is for DPMS OFF case.
That means all page flip operations should be finished already to
control DPMS off and in almost every case, there is no problem.

But if there is an issue like this lock-up, I think it should escape this condition.

Thank you.
Best regards YJ


-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





--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux