CHROMIUM: drm/exynos: fix refcounting in hdmi (mixer) path

Previously, if two fbs were ready to be flipped during the same
vblank, the kds_callback would drop the frame which came earlier
and call apply on the later frame. This worked fine on fimd but
caused problems on hdmi (mixer). The mixer will only accept one
apply call per vblank and ignore calls after that.

Since, we already applied the earlier frame, the mixer would
ignore the later frame. So the mixer flips the earlier fb while
the refcounting code thinks we are flipping the later fb. So the
ref for the earlier fb gets dropped even though its the scanout
fb. This can cause an IOMMU page fault.

The fix is to hold the later fb in an extra slot and then flip
to it on the next vblank.

In order, to allow calling apply from finish_page_flip, I had to
re-structure mixer_irq_handler in order to avoid spinlock
recursion on reg_slock.

BUG=chrome-os-partner:15241
TEST=see below

Before:

$ DISPLAY=:0.0 xrandr --output eDP-1 --off
* crash *

After:

export DISPLAY=:0.0
for i in `seq 20`; do
  xrandr --output eDP-1 --off;
  sleep 5;
  xrandr --output eDP-1 --auto;
  sleep 5;
done

* no crash *

Change-Id: I45cdad3dea94c9018e6a5fef9f9807b225130cea
Signed-off-by: Mandeep Singh Baines <msb@chromium.org>
Reviewed-on: https://gerrit.chromium.org/gerrit/39386
Reviewed-by: Sean Paul <seanpaul@chromium.org>

diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index 97602d0..a67c5f8 100644 (file)
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -60,10 +60,12 @@ struct exynos_drm_crtc {
        struct exynos_drm_overlay       overlay;
        struct drm_framebuffer          *current_fb;
        struct drm_framebuffer          *pending_fb;
+       struct drm_framebuffer          *pending_fb_extra;
 #ifdef CONFIG_DMA_SHARED_BUFFER_USES_KDS
        struct drm_pending_vblank_event *event;
        struct kds_resource_set         *current_kds;
        struct kds_resource_set         *pending_kds;
+       struct kds_resource_set         *pending_kds_extra;
        struct kds_resource_set         *future_kds;
        struct kds_resource_set         *future_kds_extra;
 #endif
@@ -357,37 +359,39 @@ static struct drm_crtc_helper_funcs exynos_crtc_helper_funcs = {
 #ifdef CONFIG_DMA_SHARED_BUFFER_USES_KDS
 void exynos_drm_kds_callback(void *callback_parameter, void *callback_extra_parameter)
 {
-       struct drm_framebuffer *fb = callback_parameter;
-       struct exynos_drm_fb *exynos_fb = to_exynos_fb(fb);
-       struct drm_crtc *crtc =  exynos_fb->crtc;
+       struct drm_framebuffer *prev_fb, *fb = callback_parameter;
+       struct drm_crtc *crtc =  to_exynos_fb(fb)->crtc;
        struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
        struct drm_device *dev = crtc->dev;
        struct kds_resource_set **pkds = callback_extra_parameter;
-       struct kds_resource_set *prev_kds;
-       struct drm_framebuffer *prev_fb;
        unsigned long flags;
 
-       exynos_drm_crtc_update(crtc, fb);
-       exynos_drm_crtc_apply(crtc);
+       /*
+        * Under rare circumstances (modeset followed by flip) it is possible
+        * to have two fbs ready in time for the same vblank. We don't want to
+        * drop the latter fb because it is most recent. We can't drop the
+        * former either because we've already applied it and the mixer does
+        * not allow applying more than one frame on a vblank. So instead of
+        * dropping a frame, we store the latter frame in the extra slot and
+        * apply it on the next vblank.
+        */
 
        spin_lock_irqsave(&dev->event_lock, flags);
-       prev_kds = exynos_crtc->pending_kds;
        prev_fb = exynos_crtc->pending_fb;
-       exynos_crtc->pending_kds = *pkds;
-       exynos_crtc->pending_fb = fb;
+       if (unlikely(prev_fb)) {
+               exynos_crtc->pending_kds_extra = *pkds;
+               exynos_crtc->pending_fb_extra = fb;
+       } else {
+               exynos_crtc->pending_kds = *pkds;
+               exynos_crtc->pending_fb = fb;
+       }
        *pkds = NULL;
-       if (prev_fb)
-               exynos_crtc->flip_in_flight--;
        spin_unlock_irqrestore(&dev->event_lock, flags);
 
-       if (prev_fb) {
-               DRM_ERROR("previous work detected\n");
-               exynos_drm_fb_put(to_exynos_fb(prev_fb));
-               if (prev_kds)
-                       kds_resource_set_release(&prev_kds);
-       } else {
+       if (likely(!prev_fb)) {
                BUG_ON(atomic_read(&exynos_crtc->flip_pending));
-               BUG_ON(prev_kds);
+               exynos_drm_crtc_update(crtc, fb);
+               exynos_drm_crtc_apply(crtc);
                atomic_set(&exynos_crtc->flip_pending, 1);
        }
 }
@@ -532,7 +536,7 @@ void exynos_drm_crtc_finish_pageflip(struct drm_device *drm_dev, int crtc_idx)
        struct drm_crtc *crtc = dev_priv->crtc[crtc_idx];
        struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
        struct kds_resource_set *kds;
-       struct drm_framebuffer *fb;
+       struct drm_framebuffer *fb, *pending_fb;
        unsigned long flags;
 
        /* set wait vsync wake up queue. */
@@ -550,10 +554,18 @@ void exynos_drm_crtc_finish_pageflip(struct drm_device *drm_dev, int crtc_idx)
        }
        kds = exynos_crtc->current_kds;
        exynos_crtc->current_kds = exynos_crtc->pending_kds;
-       exynos_crtc->pending_kds = NULL;
        fb = exynos_crtc->current_fb;
        exynos_crtc->current_fb = exynos_crtc->pending_fb;
-       exynos_crtc->pending_fb = NULL;
+       if (unlikely(exynos_crtc->pending_fb_extra)) {
+               exynos_crtc->pending_fb = exynos_crtc->pending_fb_extra;
+               exynos_crtc->pending_kds = exynos_crtc->pending_kds_extra;
+               exynos_crtc->pending_fb_extra = NULL;
+               exynos_crtc->pending_kds_extra = NULL;
+       } else {
+               exynos_crtc->pending_fb = NULL;
+               exynos_crtc->pending_kds = NULL;
+       }
+       pending_fb = exynos_crtc->pending_fb;
        exynos_crtc->flip_in_flight--;
        spin_unlock_irqrestore(&drm_dev->event_lock, flags);
 
@@ -562,6 +574,12 @@ void exynos_drm_crtc_finish_pageflip(struct drm_device *drm_dev, int crtc_idx)
        if (kds)
                kds_resource_set_release(&kds);
 
+       if (unlikely(pending_fb)) {
+               exynos_drm_crtc_update(crtc, pending_fb);
+               exynos_drm_crtc_apply(crtc);
+               atomic_set(&exynos_crtc->flip_pending, 1);
+       }
+
        drm_vblank_put(drm_dev, crtc_idx);
 }
 

diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
index 0d5a2f0..f63d636 100644 (file)
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -901,6 +901,7 @@ static irqreturn_t mixer_irq_handler(int irq, void *arg)
        struct mixer_context *mctx = arg;
        struct mixer_resources *res = &mctx->mixer_res;
        u32 val, base, shadow;
+       bool flip_complete = false;
        int i;
 
        spin_lock(&res->reg_slock);
@@ -932,7 +933,7 @@ static irqreturn_t mixer_irq_handler(int irq, void *arg)
                for (i = 0; i < MIXER_WIN_NR; i++)
                        mctx->win_data[i].updated = false;
 
-               exynos_drm_crtc_finish_pageflip(mctx->drm_dev, mctx->pipe);
+               flip_complete = true;
 
                if (mctx->event_flags & MXR_EVENT_VSYNC) {
                        DRM_DEBUG_KMS("mctx->event_flags & MXR_EVENT_VSYNC");
@@ -953,6 +954,9 @@ out:
 
        spin_unlock(&res->reg_slock);
 
+       if (flip_complete)
+               exynos_drm_crtc_finish_pageflip(mctx->drm_dev, mctx->pipe);
+
        return IRQ_HANDLED;
 }