-----Original Message-----
From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of
ville.syrj...@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ä <ville.syrj...@linux.intel.com>
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 <jbar...@virtuousgeek.org>
Reviewed-by: Sourab Gupta <sourabgu...@gmail.com>
Reviewed-by: Akash Goel <akash.go...@gmail.com>
Signed-off-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
---
drivers/gpu/drm/i915/intel_sprite.c | 94 ++++++++++++++++++++++---------------
1 file changed, 55 insertions(+), 39 deletions(-)
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);
+ 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
