From patchwork Mon May 19 10:13:05 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: "G, Pallavi" X-Patchwork-Id: 4200851 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 6A3019F32A for ; Mon, 19 May 2014 10:13:44 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 434D820221 for ; Mon, 19 May 2014 10:13:43 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id F14632028D for ; Mon, 19 May 2014 10:13:41 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id D7AF26E6AF; Mon, 19 May 2014 03:13:40 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTP id A8AAA6E6AF for ; Mon, 19 May 2014 03:13:39 -0700 (PDT) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga101.jf.intel.com with ESMTP; 19 May 2014 03:13:38 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.98,866,1392192000"; d="scan'208";a="542746567" Received: from fmsmsx103.amr.corp.intel.com ([10.19.9.34]) by orsmga002.jf.intel.com with ESMTP; 19 May 2014 03:13:10 -0700 Received: from fmsmsx117.amr.corp.intel.com (10.18.116.17) by FMSMSX103.amr.corp.intel.com (10.19.9.34) with Microsoft SMTP Server (TLS) id 14.3.123.3; Mon, 19 May 2014 03:13:10 -0700 Received: from bgsmsx101.gar.corp.intel.com (10.223.4.170) by fmsmsx117.amr.corp.intel.com (10.18.116.17) with Microsoft SMTP Server (TLS) id 14.3.123.3; Mon, 19 May 2014 03:13:10 -0700 Received: from bgsmsx103.gar.corp.intel.com ([169.254.4.124]) by BGSMSX101.gar.corp.intel.com ([169.254.1.207]) with mapi id 14.03.0123.003; Mon, 19 May 2014 15:43:06 +0530 From: "G, Pallavi" To: "ville.syrjala@linux.intel.com" , "intel-gfx@lists.freedesktop.org" Thread-Topic: [Intel-gfx] [PATCH v2 4/9] drm/i915: Perform primary enable/disable atomically with sprite updates Thread-Index: AQHPY5bg3t15kwzz90yRYSOr6hFnQJtHy6yg Date: Mon, 19 May 2014 10:13:05 +0000 Message-ID: <288F2AD7A5BB644A8E17E7267429B13C10C40261@BGSMSX103.gar.corp.intel.com> References: <1398767752-619-1-git-send-email-ville.syrjala@linux.intel.com> <1398767752-619-5-git-send-email-ville.syrjala@linux.intel.com> In-Reply-To: <1398767752-619-5-git-send-email-ville.syrjala@linux.intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.223.10.10] MIME-Version: 1.0 Cc: "Murthy, Arun R" Subject: Re: [Intel-gfx] [PATCH v2 4/9] drm/i915: Perform primary enable/disable atomically with sprite updates X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Spam-Status: No, score=-4.8 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP -----Original Message----- From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of ville.syrjala@linux.intel.com Sent: Tuesday, April 29, 2014 4:06 PM To: intel-gfx@lists.freedesktop.org Subject: [Intel-gfx] [PATCH v2 4/9] drm/i915: Perform primary enable/disable atomically with sprite updates From: Ville Syrjälä Move the primary plane enable/disable to occur atomically with the sprite update that caused the primary plane visibility to change. FBC and IPS enable/disable is left to happen well before or after the primary plane change. v2: Pass intel_crtc instead of drm_crtc (Daniel) Reviewed-by: Jesse Barnes Reviewed-by: Sourab Gupta Reviewed-by: Akash Goel Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_sprite.c | 94 ++++++++++++++++++++++--------------- 1 file changed, 55 insertions(+), 39 deletions(-) + intel_update_primary_plane(intel_crtc); + I915_WRITE(SPSTRIDE(pipe, plane), fb->pitches[0]); I915_WRITE(SPPOS(pipe, plane), (crtc_y << 16) | crtc_x); @@ -231,7 +244,8 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc, I915_WRITE(SPCNTR(pipe, plane), sprctl); I915_WRITE(SPSURF(pipe, plane), i915_gem_obj_ggtt_offset(obj) + sprsurf_offset); - POSTING_READ(SPSURF(pipe, plane)); + + intel_flush_primary_plane(dev_priv, intel_crtc->plane); if (atomic_update) intel_pipe_update_end(intel_crtc, start_vbl_count); @@ -251,11 +265,14 @@ vlv_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc) atomic_update = intel_pipe_update_start(intel_crtc, &start_vbl_count); + intel_update_primary_plane(intel_crtc); + I915_WRITE(SPCNTR(pipe, plane), I915_READ(SPCNTR(pipe, plane)) & ~SP_ENABLE); /* Activate double buffered register update */ I915_WRITE(SPSURF(pipe, plane), 0); - POSTING_READ(SPSURF(pipe, plane)); + + intel_flush_primary_plane(dev_priv, intel_crtc->plane); if (atomic_update) intel_pipe_update_end(intel_crtc, start_vbl_count); @@ -403,6 +420,8 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, atomic_update = intel_pipe_update_start(intel_crtc, &start_vbl_count); + intel_update_primary_plane(intel_crtc); + I915_WRITE(SPRSTRIDE(pipe), fb->pitches[0]); I915_WRITE(SPRPOS(pipe), (crtc_y << 16) | crtc_x); @@ -421,7 +440,8 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, I915_WRITE(SPRCTL(pipe), sprctl); I915_WRITE(SPRSURF(pipe), i915_gem_obj_ggtt_offset(obj) + sprsurf_offset); - POSTING_READ(SPRSURF(pipe)); + + intel_flush_primary_plane(dev_priv, intel_crtc->plane); if (atomic_update) intel_pipe_update_end(intel_crtc, start_vbl_count); @@ -440,13 +460,16 @@ ivb_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc) atomic_update = intel_pipe_update_start(intel_crtc, &start_vbl_count); + intel_update_primary_plane(intel_crtc); + I915_WRITE(SPRCTL(pipe), I915_READ(SPRCTL(pipe)) & ~SPRITE_ENABLE); /* Can't leave the scaler enabled... */ if (intel_plane->can_scale) I915_WRITE(SPRSCALE(pipe), 0); /* Activate double buffered register update */ I915_WRITE(SPRSURF(pipe), 0); - POSTING_READ(SPRSURF(pipe)); + + intel_flush_primary_plane(dev_priv, intel_crtc->plane); if (atomic_update) intel_pipe_update_end(intel_crtc, start_vbl_count); @@ -598,6 +621,8 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, atomic_update = intel_pipe_update_start(intel_crtc, &start_vbl_count); + intel_update_primary_plane(intel_crtc); + I915_WRITE(DVSSTRIDE(pipe), fb->pitches[0]); I915_WRITE(DVSPOS(pipe), (crtc_y << 16) | crtc_x); @@ -611,7 +636,8 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, I915_WRITE(DVSCNTR(pipe), dvscntr); I915_WRITE(DVSSURF(pipe), i915_gem_obj_ggtt_offset(obj) + dvssurf_offset); - POSTING_READ(DVSSURF(pipe)); + + intel_flush_primary_plane(dev_priv, intel_crtc->plane); if (atomic_update) intel_pipe_update_end(intel_crtc, start_vbl_count); @@ -630,12 +656,15 @@ ilk_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc) atomic_update = intel_pipe_update_start(intel_crtc, &start_vbl_count); + intel_update_primary_plane(intel_crtc); + I915_WRITE(DVSCNTR(pipe), I915_READ(DVSCNTR(pipe)) & ~DVS_ENABLE); /* Disable the scaler */ I915_WRITE(DVSSCALE(pipe), 0); /* Flush double buffered register updates */ I915_WRITE(DVSSURF(pipe), 0); - POSTING_READ(DVSSURF(pipe)); + + intel_flush_primary_plane(dev_priv, intel_crtc->plane); if (atomic_update) intel_pipe_update_end(intel_crtc, start_vbl_count); @@ -650,20 +679,10 @@ ilk_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc) } static void -intel_enable_primary(struct drm_crtc *crtc) +intel_post_enable_primary(struct drm_crtc *crtc) { struct drm_device *dev = crtc->dev; - struct drm_i915_private *dev_priv = dev->dev_private; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - int reg = DSPCNTR(intel_crtc->plane); - - if (intel_crtc->primary_enabled) - return; - - intel_crtc->primary_enabled = true; - - I915_WRITE(reg, I915_READ(reg) | DISPLAY_PLANE_ENABLE); - intel_flush_primary_plane(dev_priv, intel_crtc->plane); /* * FIXME IPS should be fine as long as one plane is @@ -682,17 +701,11 @@ intel_enable_primary(struct drm_crtc *crtc) } static void -intel_disable_primary(struct drm_crtc *crtc) +intel_pre_disable_primary(struct drm_crtc *crtc) { struct drm_device *dev = crtc->dev; struct drm_i915_private *dev_priv = dev->dev_private; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - int reg = DSPCNTR(intel_crtc->plane); - - if (!intel_crtc->primary_enabled) - return; - - intel_crtc->primary_enabled = false; mutex_lock(&dev->struct_mutex); if (dev_priv->fbc.plane == intel_crtc->plane) @@ -706,9 +719,6 @@ intel_disable_primary(struct drm_crtc *crtc) * versa. */ hsw_disable_ips(intel_crtc); - - I915_WRITE(reg, I915_READ(reg) & ~DISPLAY_PLANE_ENABLE); - intel_flush_primary_plane(dev_priv, intel_crtc->plane); } static int @@ -802,7 +812,7 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, struct drm_i915_gem_object *obj = intel_fb->obj; struct drm_i915_gem_object *old_obj = intel_plane->obj; int ret; - bool disable_primary = false; + bool primary_enabled; bool visible; int hscale, vscale; int max_scale, min_scale; @@ -973,8 +983,8 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, * If the sprite is completely covering the primary plane, * we can disable the primary and save power. */ - disable_primary = drm_rect_equals(&dst, &clip) && !colorkey_enabled(intel_plane); - WARN_ON(disable_primary && !visible && intel_crtc->active); + primary_enabled = !drm_rect_equals(&dst, &clip) || colorkey_enabled(intel_plane); + WARN_ON(!primary_enabled && !visible && intel_crtc->active); mutex_lock(&dev->struct_mutex); @@ -1001,12 +1011,12 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, intel_plane->obj = obj; if (intel_crtc->active) { - /* - * Be sure to re-enable the primary before the sprite is no longer - * covering it fully. - */ - if (!disable_primary) - intel_enable_primary(crtc); + bool primary_was_enabled = intel_crtc->primary_enabled; + + intel_crtc->primary_enabled = primary_enabled; + + if (primary_was_enabled && !primary_enabled) + intel_pre_disable_primary(crtc); if (visible) intel_plane->update_plane(plane, crtc, fb, obj, @@ -1015,8 +1025,8 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, else intel_plane->disable_plane(plane, crtc); - if (disable_primary) - intel_disable_primary(crtc); + if (!primary_was_enabled && primary_enabled) + intel_post_enable_primary(crtc); } /* Unpin old obj after new one is active to avoid ugliness */ @@ -1054,8 +1064,14 @@ intel_disable_plane(struct drm_plane *plane) intel_crtc = to_intel_crtc(plane->crtc); if (intel_crtc->active) { - intel_enable_primary(plane->crtc); + bool primary_was_enabled = intel_crtc->primary_enabled; + + intel_crtc->primary_enabled = true; + intel_plane->disable_plane(plane, plane->crtc); + + if (!primary_was_enabled && intel_crtc->primary_enabled) + intel_post_enable_primary(plane->crtc); } if (intel_plane->obj) { -- 1.8.3.2 Hi Ville, We are not considering the transparency and the variable Z-order stuffs. In case of the VLV the Z-order is not fixed we can program any plane on any order PASASBCA or SAPASBCA and even if the sprite covers the entire primary plane, part of the content on the sprite plane can be transparent/opaque. This won't work in case of Android scenario where we faced the similar problem and disabled this primary enable/disable as a temp WA. Apart from this rest of the code looks fine. Thanks, Pallavi _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index a3ed840..6192e58 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -120,6 +120,17 @@ static void intel_pipe_update_end(struct intel_crtc *crtc, u32 start_vbl_count) pipe_name(pipe), start_vbl_count, end_vbl_count); } +static void intel_update_primary_plane(struct intel_crtc *crtc) { + struct drm_i915_private *dev_priv = crtc->base.dev->dev_private; + int reg = DSPCNTR(crtc->plane); + + if (crtc->primary_enabled) + I915_WRITE(reg, I915_READ(reg) | DISPLAY_PLANE_ENABLE); + else + I915_WRITE(reg, I915_READ(reg) & ~DISPLAY_PLANE_ENABLE); } + static void vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc, struct drm_framebuffer *fb, @@ -219,6 +230,8 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc, atomic_update = intel_pipe_update_start(intel_crtc, &start_vbl_count);