[Intel-gfx] [PATCH 2/2] drm/i915: Call .update_primary_plane in intel_{enable, disable}_primary_hw_plane()

2014-08-08 Thread ville . syrjala 
From: Ville Syrjälä ville.syrj...@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.ro...@intel.com
Signed-off-by: Ville Syrjälä ville.syrj...@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

Re: [Intel-gfx] [PATCH 2/2] drm/i915: Call .update_primary_plane in intel_{enable, disable}_primary_hw_plane()

2014-08-08 Thread Daniel Vetter 
On Fri, Aug 08, 2014 at 09:51:11PM +0300, ville.syrj...@linux.intel.com wrote:
 From: Ville Syrjälä ville.syrj...@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.ro...@intel.com
 Signed-off-by: Ville Syrjälä ville.syrj...@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