All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/i915: Eliminate rmw from .update_primary_plane()
@ 2014-08-08 18:51 ville.syrjala
  2014-08-08 18:51 ` [PATCH 2/2] drm/i915: Call .update_primary_plane in intel_{enable, disable}_primary_hw_plane() ville.syrjala
  2014-08-08 18:53 ` [PATCH 1/2] drm/i915: Eliminate rmw from .update_primary_plane() Daniel Vetter
  0 siblings, 2 replies; 4+ messages in thread
From: ville.syrjala @ 2014-08-08 18:51 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Move the entire DSPCNTR register setup into the .update_primary_plane()
functions. That's where it belongs anyway and it'll also help 830M which
has the extra problem that plane registers reads will return the value
latched at the last vblank, not the value that was last written.

Also move DSPPOS and DSPSIZE setup there.

v2: Don't move variable initialization to avoid churn later

Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 100 +++++++++++------------------------
 1 file changed, 32 insertions(+), 68 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 89e0ac5..4158257 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2394,12 +2394,26 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
 	int plane = intel_crtc->plane;
 	unsigned long linear_offset;
 	u32 dspcntr;
-	u32 reg;
+	u32 reg = DSPCNTR(plane);
+
+	dspcntr = DISPPLANE_GAMMA_ENABLE;
+
+	if (intel_crtc->primary_enabled)
+		dspcntr |= DISPLAY_PLANE_ENABLE;
+
+	if (INTEL_INFO(dev)->gen < 4) {
+		if (intel_crtc->pipe == PIPE_B)
+			dspcntr |= DISPPLANE_SEL_PIPE_B;
+
+		/* pipesrc and dspsize control the size that is scaled from,
+		 * which should always be the user's requested size.
+		 */
+		I915_WRITE(DSPSIZE(plane),
+			   ((intel_crtc->config.pipe_src_h - 1) << 16) |
+			   (intel_crtc->config.pipe_src_w - 1));
+		I915_WRITE(DSPPOS(plane), 0);
+	}
 
-	reg = DSPCNTR(plane);
-	dspcntr = I915_READ(reg);
-	/* Mask out pixel format bits in case we change it */
-	dspcntr &= ~DISPPLANE_PIXFORMAT_MASK;
 	switch (fb->pixel_format) {
 	case DRM_FORMAT_C8:
 		dspcntr |= DISPPLANE_8BPP;
@@ -2431,12 +2445,9 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
 		BUG();
 	}
 
-	if (INTEL_INFO(dev)->gen >= 4) {
-		if (obj->tiling_mode != I915_TILING_NONE)
-			dspcntr |= DISPPLANE_TILED;
-		else
-			dspcntr &= ~DISPPLANE_TILED;
-	}
+	if (INTEL_INFO(dev)->gen >= 4 &&
+	    obj->tiling_mode != I915_TILING_NONE)
+		dspcntr |= DISPPLANE_TILED;
 
 	if (IS_G4X(dev))
 		dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
@@ -2480,12 +2491,16 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
 	int plane = intel_crtc->plane;
 	unsigned long linear_offset;
 	u32 dspcntr;
-	u32 reg;
+	u32 reg = DSPCNTR(plane);
+
+	dspcntr = DISPPLANE_GAMMA_ENABLE;
+
+	if (intel_crtc->primary_enabled)
+		dspcntr |= DISPLAY_PLANE_ENABLE;
+
+	if (IS_HASWELL(dev) || IS_BROADWELL(dev))
+		dspcntr |= DISPPLANE_PIPE_CSC_ENABLE;
 
-	reg = DSPCNTR(plane);
-	dspcntr = I915_READ(reg);
-	/* Mask out pixel format bits in case we change it */
-	dspcntr &= ~DISPPLANE_PIXFORMAT_MASK;
 	switch (fb->pixel_format) {
 	case DRM_FORMAT_C8:
 		dspcntr |= DISPPLANE_8BPP;
@@ -2515,12 +2530,8 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
 
 	if (obj->tiling_mode != I915_TILING_NONE)
 		dspcntr |= DISPPLANE_TILED;
-	else
-		dspcntr &= ~DISPPLANE_TILED;
 
-	if (IS_HASWELL(dev) || IS_BROADWELL(dev))
-		dspcntr &= ~DISPPLANE_TRICKLE_FEED_DISABLE;
-	else
+	if (!IS_HASWELL(dev) && !IS_BROADWELL(dev))
 		dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
 
 	I915_WRITE(reg, dspcntr);
@@ -3946,7 +3957,6 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct intel_encoder *encoder;
 	int pipe = intel_crtc->pipe;
-	enum plane plane = intel_crtc->plane;
 
 	WARN_ON(!crtc->enabled);
 
@@ -3968,10 +3978,6 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
 
 	ironlake_set_pipeconf(crtc);
 
-	/* Set up the display plane register */
-	I915_WRITE(DSPCNTR(plane), DISPPLANE_GAMMA_ENABLE);
-	POSTING_READ(DSPCNTR(plane));
-
 	dev_priv->display.update_primary_plane(crtc, crtc->primary->fb,
 					       crtc->x, crtc->y);
 
@@ -4059,7 +4065,6 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct intel_encoder *encoder;
 	int pipe = intel_crtc->pipe;
-	enum plane plane = intel_crtc->plane;
 
 	WARN_ON(!crtc->enabled);
 
@@ -4083,10 +4088,6 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
 
 	intel_set_pipe_csc(crtc);
 
-	/* Set up the display plane register */
-	I915_WRITE(DSPCNTR(plane), DISPPLANE_GAMMA_ENABLE | DISPPLANE_PIPE_CSC_ENABLE);
-	POSTING_READ(DSPCNTR(plane));
-
 	dev_priv->display.update_primary_plane(crtc, crtc->primary->fb,
 					       crtc->x, crtc->y);
 
@@ -4642,9 +4643,7 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct intel_encoder *encoder;
 	int pipe = intel_crtc->pipe;
-	int plane = intel_crtc->plane;
 	bool is_dsi;
-	u32 dspcntr;
 
 	WARN_ON(!crtc->enabled);
 
@@ -4660,27 +4659,13 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
 			vlv_prepare_pll(intel_crtc);
 	}
 
-	/* Set up the display plane register */
-	dspcntr = DISPPLANE_GAMMA_ENABLE;
-
 	if (intel_crtc->config.has_dp_encoder)
 		intel_dp_set_m_n(intel_crtc);
 
 	intel_set_pipe_timings(intel_crtc);
 
-	/* pipesrc and dspsize control the size that is scaled from,
-	 * which should always be the user's requested size.
-	 */
-	I915_WRITE(DSPSIZE(plane),
-		   ((intel_crtc->config.pipe_src_h - 1) << 16) |
-		   (intel_crtc->config.pipe_src_w - 1));
-	I915_WRITE(DSPPOS(plane), 0);
-
 	i9xx_set_pipeconf(intel_crtc);
 
-	I915_WRITE(DSPCNTR(plane), dspcntr);
-	POSTING_READ(DSPCNTR(plane));
-
 	dev_priv->display.update_primary_plane(crtc, crtc->primary->fb,
 					       crtc->x, crtc->y);
 
@@ -4735,8 +4720,6 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct intel_encoder *encoder;
 	int pipe = intel_crtc->pipe;
-	int plane = intel_crtc->plane;
-	u32 dspcntr;
 
 	WARN_ON(!crtc->enabled);
 
@@ -4745,32 +4728,13 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
 
 	i9xx_set_pll_dividers(intel_crtc);
 
-	/* Set up the display plane register */
-	dspcntr = DISPPLANE_GAMMA_ENABLE;
-
-	if (pipe == 0)
-		dspcntr &= ~DISPPLANE_SEL_PIPE_MASK;
-	else
-		dspcntr |= DISPPLANE_SEL_PIPE_B;
-
 	if (intel_crtc->config.has_dp_encoder)
 		intel_dp_set_m_n(intel_crtc);
 
 	intel_set_pipe_timings(intel_crtc);
 
-	/* pipesrc and dspsize control the size that is scaled from,
-	 * which should always be the user's requested size.
-	 */
-	I915_WRITE(DSPSIZE(plane),
-		   ((intel_crtc->config.pipe_src_h - 1) << 16) |
-		   (intel_crtc->config.pipe_src_w - 1));
-	I915_WRITE(DSPPOS(plane), 0);
-
 	i9xx_set_pipeconf(intel_crtc);
 
-	I915_WRITE(DSPCNTR(plane), dspcntr);
-	POSTING_READ(DSPCNTR(plane));
-
 	dev_priv->display.update_primary_plane(crtc, crtc->primary->fb,
 					       crtc->x, crtc->y);
 
-- 
1.8.5.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/2] drm/i915: Call .update_primary_plane in intel_{enable, disable}_primary_hw_plane()
  2014-08-08 18:51 [PATCH 1/2] drm/i915: Eliminate rmw from .update_primary_plane() ville.syrjala
@ 2014-08-08 18:51 ` ville.syrjala
  2014-08-08 19:00   ` Daniel Vetter
  2014-08-08 18:53 ` [PATCH 1/2] drm/i915: Eliminate rmw from .update_primary_plane() Daniel Vetter
  1 sibling, 1 reply; 4+ messages in thread
From: ville.syrjala @ 2014-08-08 18:51 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Make the intel_{enable,disable}_primary_hw_plane() simply call
.update_primary_plane(), thus eliminating the rmw from these functions
which should help the poor old 830M.

Now we can also remove the .update_primary_plane() from the
.crtc_enable() hooks because we end up calling it via
intel_crtc_enable_planes()->intel_enable_primary_hw_plane().

This also has the nice benefit of making primary planes a bit closer to
the way we handle sprite planes during modesets.

v2: Just write 0 to DSPCNTR and DSPSURF/DSPADDR if the plane is (to be)
    disabled. Quicker, and more importantly avoids an oops when fb==NULL
    due to BIOS fb takeover failure.
    Pimp the commit message a bit (Matt)
v3: Drop useless primary_enabled checks when setting DISPLAY_PLANE_ENABLE

Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 119 +++++++++++++++--------------------
 1 file changed, 49 insertions(+), 70 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4158257..ca4f8e6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2088,35 +2088,28 @@ void intel_flush_primary_plane(struct drm_i915_private *dev_priv,
 
 /**
  * intel_enable_primary_hw_plane - enable the primary plane on a given pipe
- * @dev_priv: i915 private structure
- * @plane: plane to enable
- * @pipe: pipe being fed
+ * @plane:  plane to be enabled
+ * @crtc: crtc for the plane
  *
- * Enable @plane on @pipe, making sure that @pipe is running first.
+ * Enable @plane on @crtc, making sure that the pipe is running first.
  */
-static void intel_enable_primary_hw_plane(struct drm_i915_private *dev_priv,
-					  enum plane plane, enum pipe pipe)
+static void intel_enable_primary_hw_plane(struct drm_plane *plane,
+					  struct drm_crtc *crtc)
 {
-	struct drm_device *dev = dev_priv->dev;
-	struct intel_crtc *intel_crtc =
-		to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
-	int reg;
-	u32 val;
+	struct drm_device *dev = plane->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 
 	/* If the pipe isn't enabled, we can't pump pixels and may hang */
-	assert_pipe_enabled(dev_priv, pipe);
+	assert_pipe_enabled(dev_priv, intel_crtc->pipe);
 
 	if (intel_crtc->primary_enabled)
 		return;
 
 	intel_crtc->primary_enabled = true;
 
-	reg = DSPCNTR(plane);
-	val = I915_READ(reg);
-	WARN_ON(val & DISPLAY_PLANE_ENABLE);
-
-	I915_WRITE(reg, val | DISPLAY_PLANE_ENABLE);
-	intel_flush_primary_plane(dev_priv, plane);
+	dev_priv->display.update_primary_plane(crtc, plane->fb,
+					       crtc->x, crtc->y);
 
 	/*
 	 * BDW signals flip done immediately if the plane
@@ -2129,31 +2122,27 @@ static void intel_enable_primary_hw_plane(struct drm_i915_private *dev_priv,
 
 /**
  * intel_disable_primary_hw_plane - disable the primary hardware plane
- * @dev_priv: i915 private structure
- * @plane: plane to disable
- * @pipe: pipe consuming the data
+ * @plane: plane to be disabled
+ * @crtc: crtc for the plane
  *
- * Disable @plane; should be an independent operation.
+ * Disable @plane on @crtc, making sure that the pipe is running first.
  */
-static void intel_disable_primary_hw_plane(struct drm_i915_private *dev_priv,
-					   enum plane plane, enum pipe pipe)
+static void intel_disable_primary_hw_plane(struct drm_plane *plane,
+					   struct drm_crtc *crtc)
 {
-	struct intel_crtc *intel_crtc =
-		to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
-	int reg;
-	u32 val;
+	struct drm_device *dev = plane->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+
+	assert_pipe_enabled(dev_priv, intel_crtc->pipe);
 
 	if (!intel_crtc->primary_enabled)
 		return;
 
 	intel_crtc->primary_enabled = false;
 
-	reg = DSPCNTR(plane);
-	val = I915_READ(reg);
-	WARN_ON((val & DISPLAY_PLANE_ENABLE) == 0);
-
-	I915_WRITE(reg, val & ~DISPLAY_PLANE_ENABLE);
-	intel_flush_primary_plane(dev_priv, plane);
+	dev_priv->display.update_primary_plane(crtc, plane->fb,
+					       crtc->x, crtc->y);
 }
 
 static bool need_vtd_wa(struct drm_device *dev)
@@ -2396,10 +2385,19 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
 	u32 dspcntr;
 	u32 reg = DSPCNTR(plane);
 
+	if (!intel_crtc->primary_enabled) {
+		I915_WRITE(reg, 0);
+		if (INTEL_INFO(dev)->gen >= 4)
+			I915_WRITE(DSPSURF(plane), 0);
+		else
+			I915_WRITE(DSPADDR(plane), 0);
+		POSTING_READ(reg);
+		return;
+	}
+
 	dspcntr = DISPPLANE_GAMMA_ENABLE;
 
-	if (intel_crtc->primary_enabled)
-		dspcntr |= DISPLAY_PLANE_ENABLE;
+	dspcntr |= DISPLAY_PLANE_ENABLE;
 
 	if (INTEL_INFO(dev)->gen < 4) {
 		if (intel_crtc->pipe == PIPE_B)
@@ -2493,10 +2491,16 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
 	u32 dspcntr;
 	u32 reg = DSPCNTR(plane);
 
+	if (!intel_crtc->primary_enabled) {
+		I915_WRITE(reg, 0);
+		I915_WRITE(DSPSURF(plane), 0);
+		POSTING_READ(reg);
+		return;
+	}
+
 	dspcntr = DISPPLANE_GAMMA_ENABLE;
 
-	if (intel_crtc->primary_enabled)
-		dspcntr |= DISPLAY_PLANE_ENABLE;
+	dspcntr |= DISPLAY_PLANE_ENABLE;
 
 	if (IS_HASWELL(dev) || IS_BROADWELL(dev))
 		dspcntr |= DISPPLANE_PIPE_CSC_ENABLE;
@@ -3890,16 +3894,14 @@ static void intel_crtc_dpms_overlay(struct intel_crtc *intel_crtc, bool enable)
 static void intel_crtc_enable_planes(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 pipe = intel_crtc->pipe;
-	int plane = intel_crtc->plane;
 
 	assert_vblank_disabled(crtc);
 
 	drm_vblank_on(dev, pipe);
 
-	intel_enable_primary_hw_plane(dev_priv, plane, pipe);
+	intel_enable_primary_hw_plane(crtc->primary, crtc);
 	intel_enable_planes(crtc);
 	intel_crtc_update_cursor(crtc, true);
 	intel_crtc_dpms_overlay(intel_crtc, true);
@@ -3936,7 +3938,7 @@ static void intel_crtc_disable_planes(struct drm_crtc *crtc)
 	intel_crtc_dpms_overlay(intel_crtc, false);
 	intel_crtc_update_cursor(crtc, false);
 	intel_disable_planes(crtc);
-	intel_disable_primary_hw_plane(dev_priv, plane, pipe);
+	intel_disable_primary_hw_plane(crtc->primary, crtc);
 
 	/*
 	 * FIXME: Once we grow proper nuclear flip support out of this we need
@@ -3978,9 +3980,6 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
 
 	ironlake_set_pipeconf(crtc);
 
-	dev_priv->display.update_primary_plane(crtc, crtc->primary->fb,
-					       crtc->x, crtc->y);
-
 	intel_crtc->active = true;
 
 	intel_set_cpu_fifo_underrun_reporting(dev, pipe, true);
@@ -4088,9 +4087,6 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
 
 	intel_set_pipe_csc(crtc);
 
-	dev_priv->display.update_primary_plane(crtc, crtc->primary->fb,
-					       crtc->x, crtc->y);
-
 	intel_crtc->active = true;
 
 	intel_set_cpu_fifo_underrun_reporting(dev, pipe, true);
@@ -4639,7 +4635,6 @@ static void valleyview_modeset_global_resources(struct drm_device *dev)
 static void valleyview_crtc_enable(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);
 	struct intel_encoder *encoder;
 	int pipe = intel_crtc->pipe;
@@ -4666,9 +4661,6 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
 
 	i9xx_set_pipeconf(intel_crtc);
 
-	dev_priv->display.update_primary_plane(crtc, crtc->primary->fb,
-					       crtc->x, crtc->y);
-
 	intel_crtc->active = true;
 
 	intel_set_cpu_fifo_underrun_reporting(dev, pipe, true);
@@ -4716,7 +4708,6 @@ static void i9xx_set_pll_dividers(struct intel_crtc *crtc)
 static void i9xx_crtc_enable(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);
 	struct intel_encoder *encoder;
 	int pipe = intel_crtc->pipe;
@@ -4735,9 +4726,6 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
 
 	i9xx_set_pipeconf(intel_crtc);
 
-	dev_priv->display.update_primary_plane(crtc, crtc->primary->fb,
-					       crtc->x, crtc->y);
-
 	intel_crtc->active = true;
 
 	if (!IS_GEN2(dev))
@@ -11361,7 +11349,6 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
 		ret = intel_set_mode(set->crtc, set->mode,
 				     set->x, set->y, set->fb);
 	} else if (config->fb_changed) {
-		struct drm_i915_private *dev_priv = dev->dev_private;
 		struct intel_crtc *intel_crtc = to_intel_crtc(set->crtc);
 
 		intel_crtc_wait_for_pending_flips(set->crtc);
@@ -11375,8 +11362,7 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
 		 */
 		if (!intel_crtc->primary_enabled && ret == 0) {
 			WARN_ON(!intel_crtc->active);
-			intel_enable_primary_hw_plane(dev_priv, intel_crtc->plane,
-						      intel_crtc->pipe);
+			intel_enable_primary_hw_plane(set->crtc->primary, set->crtc);
 		}
 
 		/*
@@ -11529,8 +11515,6 @@ static int
 intel_primary_plane_disable(struct drm_plane *plane)
 {
 	struct drm_device *dev = plane->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_plane *intel_plane = to_intel_plane(plane);
 	struct intel_crtc *intel_crtc;
 
 	if (!plane->fb)
@@ -11553,8 +11537,8 @@ intel_primary_plane_disable(struct drm_plane *plane)
 		goto disable_unpin;
 
 	intel_crtc_wait_for_pending_flips(plane->crtc);
-	intel_disable_primary_hw_plane(dev_priv, intel_plane->plane,
-				       intel_plane->pipe);
+	intel_disable_primary_hw_plane(plane, plane->crtc);
+
 disable_unpin:
 	mutex_lock(&dev->struct_mutex);
 	i915_gem_track_fb(intel_fb_obj(plane->fb), NULL,
@@ -11574,9 +11558,7 @@ intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc,
 			     uint32_t src_w, uint32_t src_h)
 {
 	struct drm_device *dev = crtc->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	struct intel_plane *intel_plane = to_intel_plane(plane);
 	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
 	struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->fb);
 	struct drm_rect dest = {
@@ -11663,9 +11645,7 @@ intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc,
 				  INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe));
 
 		if (intel_crtc->primary_enabled)
-			intel_disable_primary_hw_plane(dev_priv,
-						       intel_plane->plane,
-						       intel_plane->pipe);
+			intel_disable_primary_hw_plane(plane, crtc);
 
 
 		if (plane->fb != fb)
@@ -11682,8 +11662,7 @@ intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc,
 		return ret;
 
 	if (!intel_crtc->primary_enabled)
-		intel_enable_primary_hw_plane(dev_priv, intel_crtc->plane,
-					      intel_crtc->pipe);
+		intel_enable_primary_hw_plane(plane, crtc);
 
 	return 0;
 }
-- 
1.8.5.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/2] drm/i915: Eliminate rmw from .update_primary_plane()
  2014-08-08 18:51 [PATCH 1/2] drm/i915: Eliminate rmw from .update_primary_plane() ville.syrjala
  2014-08-08 18:51 ` [PATCH 2/2] drm/i915: Call .update_primary_plane in intel_{enable, disable}_primary_hw_plane() ville.syrjala
@ 2014-08-08 18:53 ` Daniel Vetter
  1 sibling, 0 replies; 4+ messages in thread
From: Daniel Vetter @ 2014-08-08 18:53 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Fri, Aug 08, 2014 at 09:51:10PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Move the entire DSPCNTR register setup into the .update_primary_plane()
> functions. That's where it belongs anyway and it'll also help 830M which
> has the extra problem that plane registers reads will return the value
> latched at the last vblank, not the value that was last written.
> 
> Also move DSPPOS and DSPSIZE setup there.
> 
> v2: Don't move variable initialization to avoid churn later
> 
> Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Queued for -next, thanks for the patch.
-Daniel
> ---
>  drivers/gpu/drm/i915/intel_display.c | 100 +++++++++++------------------------
>  1 file changed, 32 insertions(+), 68 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 89e0ac5..4158257 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2394,12 +2394,26 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
>  	int plane = intel_crtc->plane;
>  	unsigned long linear_offset;
>  	u32 dspcntr;
> -	u32 reg;
> +	u32 reg = DSPCNTR(plane);
> +
> +	dspcntr = DISPPLANE_GAMMA_ENABLE;
> +
> +	if (intel_crtc->primary_enabled)
> +		dspcntr |= DISPLAY_PLANE_ENABLE;
> +
> +	if (INTEL_INFO(dev)->gen < 4) {
> +		if (intel_crtc->pipe == PIPE_B)
> +			dspcntr |= DISPPLANE_SEL_PIPE_B;
> +
> +		/* pipesrc and dspsize control the size that is scaled from,
> +		 * which should always be the user's requested size.
> +		 */
> +		I915_WRITE(DSPSIZE(plane),
> +			   ((intel_crtc->config.pipe_src_h - 1) << 16) |
> +			   (intel_crtc->config.pipe_src_w - 1));
> +		I915_WRITE(DSPPOS(plane), 0);
> +	}
>  
> -	reg = DSPCNTR(plane);
> -	dspcntr = I915_READ(reg);
> -	/* Mask out pixel format bits in case we change it */
> -	dspcntr &= ~DISPPLANE_PIXFORMAT_MASK;
>  	switch (fb->pixel_format) {
>  	case DRM_FORMAT_C8:
>  		dspcntr |= DISPPLANE_8BPP;
> @@ -2431,12 +2445,9 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
>  		BUG();
>  	}
>  
> -	if (INTEL_INFO(dev)->gen >= 4) {
> -		if (obj->tiling_mode != I915_TILING_NONE)
> -			dspcntr |= DISPPLANE_TILED;
> -		else
> -			dspcntr &= ~DISPPLANE_TILED;
> -	}
> +	if (INTEL_INFO(dev)->gen >= 4 &&
> +	    obj->tiling_mode != I915_TILING_NONE)
> +		dspcntr |= DISPPLANE_TILED;
>  
>  	if (IS_G4X(dev))
>  		dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
> @@ -2480,12 +2491,16 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
>  	int plane = intel_crtc->plane;
>  	unsigned long linear_offset;
>  	u32 dspcntr;
> -	u32 reg;
> +	u32 reg = DSPCNTR(plane);
> +
> +	dspcntr = DISPPLANE_GAMMA_ENABLE;
> +
> +	if (intel_crtc->primary_enabled)
> +		dspcntr |= DISPLAY_PLANE_ENABLE;
> +
> +	if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> +		dspcntr |= DISPPLANE_PIPE_CSC_ENABLE;
>  
> -	reg = DSPCNTR(plane);
> -	dspcntr = I915_READ(reg);
> -	/* Mask out pixel format bits in case we change it */
> -	dspcntr &= ~DISPPLANE_PIXFORMAT_MASK;
>  	switch (fb->pixel_format) {
>  	case DRM_FORMAT_C8:
>  		dspcntr |= DISPPLANE_8BPP;
> @@ -2515,12 +2530,8 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
>  
>  	if (obj->tiling_mode != I915_TILING_NONE)
>  		dspcntr |= DISPPLANE_TILED;
> -	else
> -		dspcntr &= ~DISPPLANE_TILED;
>  
> -	if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> -		dspcntr &= ~DISPPLANE_TRICKLE_FEED_DISABLE;
> -	else
> +	if (!IS_HASWELL(dev) && !IS_BROADWELL(dev))
>  		dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
>  
>  	I915_WRITE(reg, dspcntr);
> @@ -3946,7 +3957,6 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	struct intel_encoder *encoder;
>  	int pipe = intel_crtc->pipe;
> -	enum plane plane = intel_crtc->plane;
>  
>  	WARN_ON(!crtc->enabled);
>  
> @@ -3968,10 +3978,6 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
>  
>  	ironlake_set_pipeconf(crtc);
>  
> -	/* Set up the display plane register */
> -	I915_WRITE(DSPCNTR(plane), DISPPLANE_GAMMA_ENABLE);
> -	POSTING_READ(DSPCNTR(plane));
> -
>  	dev_priv->display.update_primary_plane(crtc, crtc->primary->fb,
>  					       crtc->x, crtc->y);
>  
> @@ -4059,7 +4065,6 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	struct intel_encoder *encoder;
>  	int pipe = intel_crtc->pipe;
> -	enum plane plane = intel_crtc->plane;
>  
>  	WARN_ON(!crtc->enabled);
>  
> @@ -4083,10 +4088,6 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
>  
>  	intel_set_pipe_csc(crtc);
>  
> -	/* Set up the display plane register */
> -	I915_WRITE(DSPCNTR(plane), DISPPLANE_GAMMA_ENABLE | DISPPLANE_PIPE_CSC_ENABLE);
> -	POSTING_READ(DSPCNTR(plane));
> -
>  	dev_priv->display.update_primary_plane(crtc, crtc->primary->fb,
>  					       crtc->x, crtc->y);
>  
> @@ -4642,9 +4643,7 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	struct intel_encoder *encoder;
>  	int pipe = intel_crtc->pipe;
> -	int plane = intel_crtc->plane;
>  	bool is_dsi;
> -	u32 dspcntr;
>  
>  	WARN_ON(!crtc->enabled);
>  
> @@ -4660,27 +4659,13 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
>  			vlv_prepare_pll(intel_crtc);
>  	}
>  
> -	/* Set up the display plane register */
> -	dspcntr = DISPPLANE_GAMMA_ENABLE;
> -
>  	if (intel_crtc->config.has_dp_encoder)
>  		intel_dp_set_m_n(intel_crtc);
>  
>  	intel_set_pipe_timings(intel_crtc);
>  
> -	/* pipesrc and dspsize control the size that is scaled from,
> -	 * which should always be the user's requested size.
> -	 */
> -	I915_WRITE(DSPSIZE(plane),
> -		   ((intel_crtc->config.pipe_src_h - 1) << 16) |
> -		   (intel_crtc->config.pipe_src_w - 1));
> -	I915_WRITE(DSPPOS(plane), 0);
> -
>  	i9xx_set_pipeconf(intel_crtc);
>  
> -	I915_WRITE(DSPCNTR(plane), dspcntr);
> -	POSTING_READ(DSPCNTR(plane));
> -
>  	dev_priv->display.update_primary_plane(crtc, crtc->primary->fb,
>  					       crtc->x, crtc->y);
>  
> @@ -4735,8 +4720,6 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	struct intel_encoder *encoder;
>  	int pipe = intel_crtc->pipe;
> -	int plane = intel_crtc->plane;
> -	u32 dspcntr;
>  
>  	WARN_ON(!crtc->enabled);
>  
> @@ -4745,32 +4728,13 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
>  
>  	i9xx_set_pll_dividers(intel_crtc);
>  
> -	/* Set up the display plane register */
> -	dspcntr = DISPPLANE_GAMMA_ENABLE;
> -
> -	if (pipe == 0)
> -		dspcntr &= ~DISPPLANE_SEL_PIPE_MASK;
> -	else
> -		dspcntr |= DISPPLANE_SEL_PIPE_B;
> -
>  	if (intel_crtc->config.has_dp_encoder)
>  		intel_dp_set_m_n(intel_crtc);
>  
>  	intel_set_pipe_timings(intel_crtc);
>  
> -	/* pipesrc and dspsize control the size that is scaled from,
> -	 * which should always be the user's requested size.
> -	 */
> -	I915_WRITE(DSPSIZE(plane),
> -		   ((intel_crtc->config.pipe_src_h - 1) << 16) |
> -		   (intel_crtc->config.pipe_src_w - 1));
> -	I915_WRITE(DSPPOS(plane), 0);
> -
>  	i9xx_set_pipeconf(intel_crtc);
>  
> -	I915_WRITE(DSPCNTR(plane), dspcntr);
> -	POSTING_READ(DSPCNTR(plane));
> -
>  	dev_priv->display.update_primary_plane(crtc, crtc->primary->fb,
>  					       crtc->x, crtc->y);
>  
> -- 
> 1.8.5.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 2/2] drm/i915: Call .update_primary_plane in intel_{enable, disable}_primary_hw_plane()
  2014-08-08 18:51 ` [PATCH 2/2] drm/i915: Call .update_primary_plane in intel_{enable, disable}_primary_hw_plane() ville.syrjala
@ 2014-08-08 19:00   ` Daniel Vetter
  0 siblings, 0 replies; 4+ messages in thread
From: Daniel Vetter @ 2014-08-08 19:00 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Fri, Aug 08, 2014 at 09:51:11PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Make the intel_{enable,disable}_primary_hw_plane() simply call
> .update_primary_plane(), thus eliminating the rmw from these functions
> which should help the poor old 830M.
> 
> Now we can also remove the .update_primary_plane() from the
> .crtc_enable() hooks because we end up calling it via
> intel_crtc_enable_planes()->intel_enable_primary_hw_plane().
> 
> This also has the nice benefit of making primary planes a bit closer to
> the way we handle sprite planes during modesets.
> 
> v2: Just write 0 to DSPCNTR and DSPSURF/DSPADDR if the plane is (to be)
>     disabled. Quicker, and more importantly avoids an oops when fb==NULL
>     due to BIOS fb takeover failure.
>     Pimp the commit message a bit (Matt)
> v3: Drop useless primary_enabled checks when setting DISPLAY_PLANE_ENABLE
> 
> Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

This almost starts to look sane ... Queued for -next, thanks for the patch.
-Daniel
> ---
>  drivers/gpu/drm/i915/intel_display.c | 119 +++++++++++++++--------------------
>  1 file changed, 49 insertions(+), 70 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 4158257..ca4f8e6 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2088,35 +2088,28 @@ void intel_flush_primary_plane(struct drm_i915_private *dev_priv,
>  
>  /**
>   * intel_enable_primary_hw_plane - enable the primary plane on a given pipe
> - * @dev_priv: i915 private structure
> - * @plane: plane to enable
> - * @pipe: pipe being fed
> + * @plane:  plane to be enabled
> + * @crtc: crtc for the plane
>   *
> - * Enable @plane on @pipe, making sure that @pipe is running first.
> + * Enable @plane on @crtc, making sure that the pipe is running first.
>   */
> -static void intel_enable_primary_hw_plane(struct drm_i915_private *dev_priv,
> -					  enum plane plane, enum pipe pipe)
> +static void intel_enable_primary_hw_plane(struct drm_plane *plane,
> +					  struct drm_crtc *crtc)
>  {
> -	struct drm_device *dev = dev_priv->dev;
> -	struct intel_crtc *intel_crtc =
> -		to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
> -	int reg;
> -	u32 val;
> +	struct drm_device *dev = plane->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  
>  	/* If the pipe isn't enabled, we can't pump pixels and may hang */
> -	assert_pipe_enabled(dev_priv, pipe);
> +	assert_pipe_enabled(dev_priv, intel_crtc->pipe);
>  
>  	if (intel_crtc->primary_enabled)
>  		return;
>  
>  	intel_crtc->primary_enabled = true;
>  
> -	reg = DSPCNTR(plane);
> -	val = I915_READ(reg);
> -	WARN_ON(val & DISPLAY_PLANE_ENABLE);
> -
> -	I915_WRITE(reg, val | DISPLAY_PLANE_ENABLE);
> -	intel_flush_primary_plane(dev_priv, plane);
> +	dev_priv->display.update_primary_plane(crtc, plane->fb,
> +					       crtc->x, crtc->y);
>  
>  	/*
>  	 * BDW signals flip done immediately if the plane
> @@ -2129,31 +2122,27 @@ static void intel_enable_primary_hw_plane(struct drm_i915_private *dev_priv,
>  
>  /**
>   * intel_disable_primary_hw_plane - disable the primary hardware plane
> - * @dev_priv: i915 private structure
> - * @plane: plane to disable
> - * @pipe: pipe consuming the data
> + * @plane: plane to be disabled
> + * @crtc: crtc for the plane
>   *
> - * Disable @plane; should be an independent operation.
> + * Disable @plane on @crtc, making sure that the pipe is running first.
>   */
> -static void intel_disable_primary_hw_plane(struct drm_i915_private *dev_priv,
> -					   enum plane plane, enum pipe pipe)
> +static void intel_disable_primary_hw_plane(struct drm_plane *plane,
> +					   struct drm_crtc *crtc)
>  {
> -	struct intel_crtc *intel_crtc =
> -		to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
> -	int reg;
> -	u32 val;
> +	struct drm_device *dev = plane->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +
> +	assert_pipe_enabled(dev_priv, intel_crtc->pipe);
>  
>  	if (!intel_crtc->primary_enabled)
>  		return;
>  
>  	intel_crtc->primary_enabled = false;
>  
> -	reg = DSPCNTR(plane);
> -	val = I915_READ(reg);
> -	WARN_ON((val & DISPLAY_PLANE_ENABLE) == 0);
> -
> -	I915_WRITE(reg, val & ~DISPLAY_PLANE_ENABLE);
> -	intel_flush_primary_plane(dev_priv, plane);
> +	dev_priv->display.update_primary_plane(crtc, plane->fb,
> +					       crtc->x, crtc->y);
>  }
>  
>  static bool need_vtd_wa(struct drm_device *dev)
> @@ -2396,10 +2385,19 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
>  	u32 dspcntr;
>  	u32 reg = DSPCNTR(plane);
>  
> +	if (!intel_crtc->primary_enabled) {
> +		I915_WRITE(reg, 0);
> +		if (INTEL_INFO(dev)->gen >= 4)
> +			I915_WRITE(DSPSURF(plane), 0);
> +		else
> +			I915_WRITE(DSPADDR(plane), 0);
> +		POSTING_READ(reg);
> +		return;
> +	}
> +
>  	dspcntr = DISPPLANE_GAMMA_ENABLE;
>  
> -	if (intel_crtc->primary_enabled)
> -		dspcntr |= DISPLAY_PLANE_ENABLE;
> +	dspcntr |= DISPLAY_PLANE_ENABLE;
>  
>  	if (INTEL_INFO(dev)->gen < 4) {
>  		if (intel_crtc->pipe == PIPE_B)
> @@ -2493,10 +2491,16 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
>  	u32 dspcntr;
>  	u32 reg = DSPCNTR(plane);
>  
> +	if (!intel_crtc->primary_enabled) {
> +		I915_WRITE(reg, 0);
> +		I915_WRITE(DSPSURF(plane), 0);
> +		POSTING_READ(reg);
> +		return;
> +	}
> +
>  	dspcntr = DISPPLANE_GAMMA_ENABLE;
>  
> -	if (intel_crtc->primary_enabled)
> -		dspcntr |= DISPLAY_PLANE_ENABLE;
> +	dspcntr |= DISPLAY_PLANE_ENABLE;
>  
>  	if (IS_HASWELL(dev) || IS_BROADWELL(dev))
>  		dspcntr |= DISPPLANE_PIPE_CSC_ENABLE;
> @@ -3890,16 +3894,14 @@ static void intel_crtc_dpms_overlay(struct intel_crtc *intel_crtc, bool enable)
>  static void intel_crtc_enable_planes(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 pipe = intel_crtc->pipe;
> -	int plane = intel_crtc->plane;
>  
>  	assert_vblank_disabled(crtc);
>  
>  	drm_vblank_on(dev, pipe);
>  
> -	intel_enable_primary_hw_plane(dev_priv, plane, pipe);
> +	intel_enable_primary_hw_plane(crtc->primary, crtc);
>  	intel_enable_planes(crtc);
>  	intel_crtc_update_cursor(crtc, true);
>  	intel_crtc_dpms_overlay(intel_crtc, true);
> @@ -3936,7 +3938,7 @@ static void intel_crtc_disable_planes(struct drm_crtc *crtc)
>  	intel_crtc_dpms_overlay(intel_crtc, false);
>  	intel_crtc_update_cursor(crtc, false);
>  	intel_disable_planes(crtc);
> -	intel_disable_primary_hw_plane(dev_priv, plane, pipe);
> +	intel_disable_primary_hw_plane(crtc->primary, crtc);
>  
>  	/*
>  	 * FIXME: Once we grow proper nuclear flip support out of this we need
> @@ -3978,9 +3980,6 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
>  
>  	ironlake_set_pipeconf(crtc);
>  
> -	dev_priv->display.update_primary_plane(crtc, crtc->primary->fb,
> -					       crtc->x, crtc->y);
> -
>  	intel_crtc->active = true;
>  
>  	intel_set_cpu_fifo_underrun_reporting(dev, pipe, true);
> @@ -4088,9 +4087,6 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
>  
>  	intel_set_pipe_csc(crtc);
>  
> -	dev_priv->display.update_primary_plane(crtc, crtc->primary->fb,
> -					       crtc->x, crtc->y);
> -
>  	intel_crtc->active = true;
>  
>  	intel_set_cpu_fifo_underrun_reporting(dev, pipe, true);
> @@ -4639,7 +4635,6 @@ static void valleyview_modeset_global_resources(struct drm_device *dev)
>  static void valleyview_crtc_enable(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);
>  	struct intel_encoder *encoder;
>  	int pipe = intel_crtc->pipe;
> @@ -4666,9 +4661,6 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
>  
>  	i9xx_set_pipeconf(intel_crtc);
>  
> -	dev_priv->display.update_primary_plane(crtc, crtc->primary->fb,
> -					       crtc->x, crtc->y);
> -
>  	intel_crtc->active = true;
>  
>  	intel_set_cpu_fifo_underrun_reporting(dev, pipe, true);
> @@ -4716,7 +4708,6 @@ static void i9xx_set_pll_dividers(struct intel_crtc *crtc)
>  static void i9xx_crtc_enable(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);
>  	struct intel_encoder *encoder;
>  	int pipe = intel_crtc->pipe;
> @@ -4735,9 +4726,6 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
>  
>  	i9xx_set_pipeconf(intel_crtc);
>  
> -	dev_priv->display.update_primary_plane(crtc, crtc->primary->fb,
> -					       crtc->x, crtc->y);
> -
>  	intel_crtc->active = true;
>  
>  	if (!IS_GEN2(dev))
> @@ -11361,7 +11349,6 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
>  		ret = intel_set_mode(set->crtc, set->mode,
>  				     set->x, set->y, set->fb);
>  	} else if (config->fb_changed) {
> -		struct drm_i915_private *dev_priv = dev->dev_private;
>  		struct intel_crtc *intel_crtc = to_intel_crtc(set->crtc);
>  
>  		intel_crtc_wait_for_pending_flips(set->crtc);
> @@ -11375,8 +11362,7 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
>  		 */
>  		if (!intel_crtc->primary_enabled && ret == 0) {
>  			WARN_ON(!intel_crtc->active);
> -			intel_enable_primary_hw_plane(dev_priv, intel_crtc->plane,
> -						      intel_crtc->pipe);
> +			intel_enable_primary_hw_plane(set->crtc->primary, set->crtc);
>  		}
>  
>  		/*
> @@ -11529,8 +11515,6 @@ static int
>  intel_primary_plane_disable(struct drm_plane *plane)
>  {
>  	struct drm_device *dev = plane->dev;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_plane *intel_plane = to_intel_plane(plane);
>  	struct intel_crtc *intel_crtc;
>  
>  	if (!plane->fb)
> @@ -11553,8 +11537,8 @@ intel_primary_plane_disable(struct drm_plane *plane)
>  		goto disable_unpin;
>  
>  	intel_crtc_wait_for_pending_flips(plane->crtc);
> -	intel_disable_primary_hw_plane(dev_priv, intel_plane->plane,
> -				       intel_plane->pipe);
> +	intel_disable_primary_hw_plane(plane, plane->crtc);
> +
>  disable_unpin:
>  	mutex_lock(&dev->struct_mutex);
>  	i915_gem_track_fb(intel_fb_obj(plane->fb), NULL,
> @@ -11574,9 +11558,7 @@ intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc,
>  			     uint32_t src_w, uint32_t src_h)
>  {
>  	struct drm_device *dev = crtc->dev;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -	struct intel_plane *intel_plane = to_intel_plane(plane);
>  	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
>  	struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->fb);
>  	struct drm_rect dest = {
> @@ -11663,9 +11645,7 @@ intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc,
>  				  INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe));
>  
>  		if (intel_crtc->primary_enabled)
> -			intel_disable_primary_hw_plane(dev_priv,
> -						       intel_plane->plane,
> -						       intel_plane->pipe);
> +			intel_disable_primary_hw_plane(plane, crtc);
>  
>  
>  		if (plane->fb != fb)
> @@ -11682,8 +11662,7 @@ intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc,
>  		return ret;
>  
>  	if (!intel_crtc->primary_enabled)
> -		intel_enable_primary_hw_plane(dev_priv, intel_crtc->plane,
> -					      intel_crtc->pipe);
> +		intel_enable_primary_hw_plane(plane, crtc);
>  
>  	return 0;
>  }
> -- 
> 1.8.5.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2014-08-08 19:00 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-08 18:51 [PATCH 1/2] drm/i915: Eliminate rmw from .update_primary_plane() ville.syrjala
2014-08-08 18:51 ` [PATCH 2/2] drm/i915: Call .update_primary_plane in intel_{enable, disable}_primary_hw_plane() ville.syrjala
2014-08-08 19:00   ` Daniel Vetter
2014-08-08 18:53 ` [PATCH 1/2] drm/i915: Eliminate rmw from .update_primary_plane() Daniel Vetter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.