From patchwork Tue Sep 15 20:57:37 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 47768 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n8FKwZuj001252 for ; Tue, 15 Sep 2009 20:58:35 GMT Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 853B09F67C; Tue, 15 Sep 2009 13:58:35 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mail.ffwll.ch (cable-static-49-187.intergga.ch [157.161.49.187]) by gabe.freedesktop.org (Postfix) with ESMTP id 052669F66D for ; Tue, 15 Sep 2009 13:58:32 -0700 (PDT) Received: by mail.ffwll.ch (Postfix, from userid 1000) id 3850320C4DA; Wed, 16 Sep 2009 06:52:40 +0200 (CEST) X-Spam-ASN: X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on orange.ffwll.ch X-Spam-Level: X-Spam-Hammy: 0.000-+--H*Ad:D*sourceforge.net, 0.000-+--H*Ad:D*lists.sourceforge.net, 0.000-+--HCc:D*net X-Spam-Status: No, score=-4.4 required=6.0 tests=ALL_TRUSTED,BAYES_00 autolearn=ham version=3.2.5 X-Spam-Spammy: Received: from biene (unknown [192.168.23.129]) by mail.ffwll.ch (Postfix) with ESMTP id 9544920C4B8; Wed, 16 Sep 2009 06:52:06 +0200 (CEST) Received: from daniel by biene with local (Exim 4.69) (envelope-from ) id 1Mnf66-0002Ax-Jv; Tue, 15 Sep 2009 22:57:58 +0200 From: Daniel Vetter To: intel-gfx@lists.freedesktop.org Date: Tue, 15 Sep 2009 22:57:37 +0200 Message-Id: <01d51e4ad2c780f680b2c0eeb289804f2878bf65.1253047594.git.daniel.vetter@ffwll.ch> X-Mailer: git-send-email 1.6.3.3 In-Reply-To: <5a82db0a8446ac3e5d15cec182504d94569521d4.1253047594.git.daniel.vetter@ffwll.ch> References: <0cdb434c339fd4257f9e19c0b9db4389c99c8e84.1253047594.git.daniel.vetter@ffwll.ch> <23ce9c607c40e8441e7c5650124782f150030e52.1253047594.git.daniel.vetter@ffwll.ch> <7e4800d3a495686bb3d4cea4dd9364f53c100bba.1253047594.git.daniel.vetter@ffwll.ch> <5a82db0a8446ac3e5d15cec182504d94569521d4.1253047594.git.daniel.vetter@ffwll.ch> In-Reply-To: References: Cc: Daniel Vetter , dri-devel@lists.sourceforge.net Subject: [Intel-gfx] [PATCH 7/8] [drm/i915] implement interruptible sleeps in the overlay code X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.9 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org At least for the common case of userspace ioctls. When doing a modeset operation, the wait is still uninterruptible. But considering that failing to turn off the overlay when switching off the crtc it's running on hangs the chip, it doesn't complicate matters _very_ much. There's just an unkillable X in addition to a black screen. BUG() about it and explain in the code. Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_display.c | 17 +++- drivers/gpu/drm/i915/intel_drv.h | 9 ++- drivers/gpu/drm/i915/intel_overlay.c | 167 +++++++++++++++++++++++++++------- 3 files changed, 159 insertions(+), 34 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 19773c9..3112bf3 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -1703,11 +1703,26 @@ static void igdng_crtc_dpms(struct drm_crtc *crtc, int mode) static void intel_crtc_dpms_overlay(struct intel_crtc *intel_crtc, bool enable) { struct intel_overlay *overlay; + int ret; if (!enable && intel_crtc->overlay) { overlay = intel_crtc->overlay; mutex_lock(&overlay->dev->struct_mutex); - intel_overlay_switch_off(overlay); + for (;;) { + ret = intel_overlay_switch_off(overlay); + if (ret == 0) + break; + + ret = intel_overlay_recover_from_interrupt(overlay, 0); + if (ret != 0) { + /* overlay doesn't react anymore. Usually + * results in a black screen and an unkillable + * X server. */ + BUG(); + overlay->hw_wedged = HW_WEDGED; + break; + } + } mutex_unlock(&overlay->dev->struct_mutex); } /* Let userspace switch the overlay on again. In most cases userspace diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 9eea5c7..8a0fddf 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -126,8 +126,13 @@ struct intel_overlay { struct drm_i915_gem_object *reg_bo; void *virt_addr; /* flip handling */ - int hw_wedged; uint32_t last_flip_req; + int hw_wedged; +#define HW_WEDGED 1 +#define NEEDS_WAIT_FOR_FLIP 2 +#define RELEASE_OLD_VID 3 +#define SWITCH_OFF_STAGE_1 4 +#define SWITCH_OFF_STAGE_2 5 }; struct intel_crtc { @@ -207,6 +212,8 @@ extern int intel_framebuffer_create(struct drm_device *dev, extern void intel_setup_overlay(struct drm_device *dev); extern void intel_cleanup_overlay(struct drm_device *dev); extern int intel_overlay_switch_off(struct intel_overlay *overlay); +extern int intel_overlay_recover_from_interrupt(struct intel_overlay *overlay, + int interruptible); extern int intel_overlay_put_image(struct drm_device *dev, void *data, struct drm_file *file_priv); extern int intel_overlay_attrs(struct drm_device *dev, void *data, diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c index 85e07e4..972d715 100644 --- a/drivers/gpu/drm/i915/intel_overlay.c +++ b/drivers/gpu/drm/i915/intel_overlay.c @@ -222,6 +222,9 @@ static int intel_overlay_on(struct intel_overlay *overlay) BUG_ON(overlay->active); + overlay->active = 1; + overlay->hw_wedged = NEEDS_WAIT_FOR_FLIP; + BEGIN_LP_RING(6); OUT_RING(MI_FLUSH); OUT_RING(MI_NOOP); @@ -231,15 +234,16 @@ static int intel_overlay_on(struct intel_overlay *overlay) OUT_RING(MI_NOOP); ADVANCE_LP_RING(); - ret = i915_lp_ring_sync(dev); - if (ret != 0) { - DRM_ERROR("intel overlay: ring sync failed, hw likely wedged\n"); - overlay->hw_wedged = 1; - return 0; - } + overlay->last_flip_req = i915_add_request(dev, NULL, 0); + if (overlay->last_flip_req == 0) + return -ENOMEM; - overlay->active = 1; + ret = i915_do_wait_request(dev, overlay->last_flip_req, 1); + if (ret != 0) + return ret; + overlay->hw_wedged = 0; + overlay->last_flip_req = 0; return 0; } @@ -283,7 +287,6 @@ static int intel_overlay_wait_flip(struct intel_overlay *overlay) if (overlay->last_flip_req != 0) { ret = i915_do_wait_request(dev, overlay->last_flip_req, 0); - if (ret != 0) return ret; @@ -296,19 +299,24 @@ static int intel_overlay_wait_flip(struct intel_overlay *overlay) } /* synchronous slowpath */ + overlay->hw_wedged = RELEASE_OLD_VID; + BEGIN_LP_RING(2); OUT_RING(MI_WAIT_FOR_EVENT | MI_WAIT_FOR_OVERLAY_FLIP); OUT_RING(MI_NOOP); ADVANCE_LP_RING(); - /* run in lockstep with the hw for easier testing */ - ret = i915_lp_ring_sync(dev); - if (ret != 0) { - DRM_ERROR("intel overlay: ring sync failed, hw likely wedged\n"); - overlay->hw_wedged = 1; - } + overlay->last_flip_req = i915_add_request(dev, NULL, 0); + if (overlay->last_flip_req == 0) + return -ENOMEM; - return ret; + ret = i915_do_wait_request(dev, overlay->last_flip_req, 1); + if (ret != 0) + return ret; + + overlay->hw_wedged = 0; + overlay->last_flip_req = 0; + return 0; } /* overlay needs to be disabled in OCMD reg */ @@ -329,6 +337,8 @@ static int intel_overlay_off(struct intel_overlay *overlay) flip_addr |= OFC_UPDATE; /* wait for overlay to go idle */ + overlay->hw_wedged = SWITCH_OFF_STAGE_1; + BEGIN_LP_RING(6); OUT_RING(MI_FLUSH); OUT_RING(MI_NOOP); @@ -338,14 +348,17 @@ static int intel_overlay_off(struct intel_overlay *overlay) OUT_RING(MI_NOOP); ADVANCE_LP_RING(); - ret = i915_lp_ring_sync(dev); - if (ret != 0) { - DRM_ERROR("intel overlay: ring sync failed, hw likely wedged\n"); - overlay->hw_wedged = 1; + overlay->last_flip_req = i915_add_request(dev, NULL, 0); + if (overlay->last_flip_req == 0) + return -ENOMEM; + + ret = i915_do_wait_request(dev, overlay->last_flip_req, 1); + if (ret != 0) return ret; - } /* turn overlay off */ + overlay->hw_wedged = SWITCH_OFF_STAGE_2; + BEGIN_LP_RING(6); OUT_RING(MI_FLUSH); OUT_RING(MI_NOOP); @@ -355,18 +368,100 @@ static int intel_overlay_off(struct intel_overlay *overlay) OUT_RING(MI_NOOP); ADVANCE_LP_RING(); - ret = i915_lp_ring_sync(dev); - if (ret != 0) { - DRM_ERROR("intel overlay: ring sync failed, hw likely wedged\n"); - overlay->hw_wedged = 1; + overlay->last_flip_req = i915_add_request(dev, NULL, 0); + if (overlay->last_flip_req == 0) + return -ENOMEM; + + ret = i915_do_wait_request(dev, overlay->last_flip_req, 1); + if (ret != 0) return ret; - } overlay->active = 0; - + overlay->hw_wedged = 0; + overlay->last_flip_req = 0; return ret; } +/* recover from an interruption due to a signal + * We have to be careful not to repeat work forever an make forward progess. */ +int intel_overlay_recover_from_interrupt(struct intel_overlay *overlay, + int interruptible) +{ + struct drm_device *dev = overlay->dev; + drm_i915_private_t *dev_priv = dev->dev_private; + struct drm_gem_object *obj; + u32 flip_addr; + int ret; + RING_LOCALS; + + if (overlay->hw_wedged == HW_WEDGED) + return -EIO; + + if (overlay->last_flip_req == 0) { + overlay->last_flip_req = i915_add_request(dev, NULL, 0); + if (overlay->last_flip_req == 0) + return -ENOMEM; + } + + ret = i915_do_wait_request(dev, overlay->last_flip_req, interruptible); + if (ret != 0) + return ret; + + switch (overlay->hw_wedged) { + case RELEASE_OLD_VID: + obj = overlay->old_vid_bo->obj; + i915_gem_object_unpin(obj); + drm_gem_object_unreference(obj); + overlay->old_vid_bo = NULL; + break; + case SWITCH_OFF_STAGE_1: + flip_addr = overlay->flip_addr; + flip_addr |= OFC_UPDATE; + + overlay->hw_wedged = SWITCH_OFF_STAGE_2; + + BEGIN_LP_RING(6); + OUT_RING(MI_FLUSH); + OUT_RING(MI_NOOP); + OUT_RING(MI_OVERLAY_FLIP | MI_OVERLAY_OFF); + OUT_RING(flip_addr); + OUT_RING(MI_WAIT_FOR_EVENT | MI_WAIT_FOR_OVERLAY_FLIP); + OUT_RING(MI_NOOP); + ADVANCE_LP_RING(); + + overlay->last_flip_req = i915_add_request(dev, NULL, 0); + if (overlay->last_flip_req == 0) + return -ENOMEM; + + ret = i915_do_wait_request(dev, overlay->last_flip_req, + interruptible); + if (ret != 0) + return ret; + + case SWITCH_OFF_STAGE_2: + printk("switch off 2\n"); + + BUG_ON(!overlay->vid_bo); + obj = overlay->vid_bo->obj; + + i915_gem_object_unpin(obj); + drm_gem_object_unreference(obj); + overlay->vid_bo = NULL; + + overlay->crtc->overlay = NULL; + overlay->crtc = NULL; + + overlay->active = 0; + break; + default: + BUG_ON(overlay->hw_wedged != NEEDS_WAIT_FOR_FLIP); + } + + overlay->hw_wedged = 0; + overlay->last_flip_req = 0; + return 0; +} + /* Wait for pending overlay flip and release old frame. * Needs to be called before the overlay register are changed * via intel_overlay_(un)map_regs_atomic */ @@ -375,13 +470,15 @@ static int intel_overlay_release_old_vid(struct intel_overlay *overlay) int ret; struct drm_gem_object *obj; + /* only wait if there is actually an old frame to release to + * guarantee forward progress */ + if (!overlay->old_vid_bo) + return 0; + ret = intel_overlay_wait_flip(overlay); if (ret != 0) return ret; - if (!overlay->old_vid_bo) - return 0; - obj = overlay->old_vid_bo->obj; i915_gem_object_unpin(obj); drm_gem_object_unreference(obj); @@ -646,9 +743,6 @@ int intel_overlay_do_put_image(struct intel_overlay *overlay, BUG_ON(!mutex_is_locked(&dev->mode_config.mutex)); BUG_ON(!overlay); - if (overlay->hw_wedged) - return -EBUSY; - ret = intel_overlay_release_old_vid(overlay); if (ret != 0) return ret; @@ -761,6 +855,9 @@ int intel_overlay_switch_off(struct intel_overlay *overlay) intel_overlay_unmap_regs_atomic(overlay); ret = intel_overlay_off(overlay); + if (ret != 0) + return ret; + /* never have the overlay hw on without showing a frame */ BUG_ON(!overlay->vid_bo); obj = overlay->vid_bo->obj; @@ -1002,6 +1099,12 @@ int intel_overlay_put_image(struct drm_device *dev, void *data, mutex_lock(&dev->mode_config.mutex); mutex_lock(&dev->struct_mutex); + if (overlay->hw_wedged) { + ret = intel_overlay_recover_from_interrupt(overlay, 1); + if (ret != 0) + goto out_unlock; + } + if (overlay->crtc != crtc) { struct drm_display_mode *mode = &crtc->base.mode; ret = intel_overlay_switch_off(overlay);