diff mbox

[v2,1/2] drm/i915: Generalize drain latency computation

Message ID 1407260754-18316-1-git-send-email-gajanan.bhat@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Gajanan Bhat Aug. 5, 2014, 5:45 p.m. UTC
Modify drain latency computation to use it for any plane. Same function can be
used for primary, cursor and sprite planes.

v2: Adressed review comments by Imre and Ville.
    - Moved clock round up in separate patch
    - Added WARN check for clock and pixel size
    - Simplified bit masking
    - Use cursor_base instead of reg read

Signed-off-by: Gajanan Bhat <gajanan.bhat@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h |    1 +
 drivers/gpu/drm/i915/intel_pm.c |   88 +++++++++++++++++++++++----------------
 2 files changed, 52 insertions(+), 37 deletions(-)

Comments

Ville Syrjälä Aug. 5, 2014, 12:56 p.m. UTC | #1
On Tue, Aug 05, 2014 at 11:15:53PM +0530, Gajanan Bhat wrote:
> Modify drain latency computation to use it for any plane. Same function can be
> used for primary, cursor and sprite planes.
> 
> v2: Adressed review comments by Imre and Ville.
>     - Moved clock round up in separate patch
>     - Added WARN check for clock and pixel size
>     - Simplified bit masking
>     - Use cursor_base instead of reg read
> 
> Signed-off-by: Gajanan Bhat <gajanan.bhat@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h |    1 +
>  drivers/gpu/drm/i915/intel_pm.c |   88 +++++++++++++++++++++++----------------
>  2 files changed, 52 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index d75bed8..55f159d 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3904,6 +3904,7 @@ enum punit_power_well {
>  #define DDL_PLANE_PRECISION_64		(1<<7)
>  #define DDL_PLANE_PRECISION_32		(0<<7)
>  #define DDL_PLANE_SHIFT			0
> +#define DRAIN_LATENCY_MASK		0x7f
>  
>  /* FIFO watermark sizes etc */
>  #define G4X_FIFO_LINE_SIZE	64
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 3544de0..ea64675 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1271,33 +1271,24 @@ static bool g4x_compute_srwm(struct drm_device *dev,
>  			      display, cursor);
>  }
>  
> -static bool vlv_compute_drain_latency(struct drm_device *dev,
> -				     int plane,
> -				     int *plane_prec_mult,
> -				     int *plane_dl,
> -				     int *cursor_prec_mult,
> -				     int *cursor_dl)
> +static bool vlv_compute_drain_latency(struct drm_crtc *crtc,
> +				      int pixel_size,
> +				      int *prec_mult,
> +				      int *drain_latency)
>  {
> -	struct drm_crtc *crtc;
> -	int clock, pixel_size;
>  	int entries;
> +	int clock = to_intel_crtc(crtc)->config.adjusted_mode.crtc_clock;
>  
> -	crtc = intel_get_crtc_for_plane(dev, plane);
> -	if (!intel_crtc_active(crtc))
> +	if (WARN(clock == 0, "Pixel clock is zero!\n"))
>  		return false;
>  
> -	clock = to_intel_crtc(crtc)->config.adjusted_mode.crtc_clock;
> -	pixel_size = crtc->primary->fb->bits_per_pixel / 8;	/* BPP */
> +	if (WARN(pixel_size == 0, "Pixel size is zero!\n"))
> +		return false;
>  
>  	entries = (clock / 1000) * pixel_size;
> -	*plane_prec_mult = (entries > 128) ?
> -		DRAIN_LATENCY_PRECISION_64 : DRAIN_LATENCY_PRECISION_32;
> -	*plane_dl = (64 * (*plane_prec_mult) * 4) / entries;
> -
> -	entries = (clock / 1000) * 4;	/* BPP is always 4 for cursor */
> -	*cursor_prec_mult = (entries > 128) ?
> -		DRAIN_LATENCY_PRECISION_64 : DRAIN_LATENCY_PRECISION_32;
> -	*cursor_dl = (64 * (*cursor_prec_mult) * 4) / entries;
> +	*prec_mult = (entries > 128) ? DRAIN_LATENCY_PRECISION_64 :
> +				       DRAIN_LATENCY_PRECISION_32;
> +	*drain_latency = (64 * (*prec_mult) * 4) / entries;
>  
>  	return true;
>  }
> @@ -1312,24 +1303,47 @@ static bool vlv_compute_drain_latency(struct drm_device *dev,
>  
>  static void vlv_update_drain_latency(struct drm_crtc *crtc)
>  {
> -	struct drm_device *dev = crtc->dev;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	enum pipe pipe = to_intel_crtc(crtc)->pipe;
> -	int plane_prec, plane_dl;
> -	int cursor_prec, cursor_dl;
> -	int plane_prec_mult, cursor_prec_mult;
> -
> -	if (vlv_compute_drain_latency(dev, pipe, &plane_prec_mult, &plane_dl,
> -				      &cursor_prec_mult, &cursor_dl)) {
> -		cursor_prec = (cursor_prec_mult == DRAIN_LATENCY_PRECISION_64) ?
> -			DDL_CURSOR_PRECISION_64 : DDL_CURSOR_PRECISION_32;
> -		plane_prec = (plane_prec_mult == DRAIN_LATENCY_PRECISION_64) ?
> -			DDL_PLANE_PRECISION_64 : DDL_PLANE_PRECISION_32;
> -
> -		I915_WRITE(VLV_DDL(pipe), cursor_prec |
> -			   (cursor_dl << DDL_CURSOR_SHIFT) |
> -			   plane_prec | (plane_dl << DDL_PLANE_SHIFT));
> +	struct drm_i915_private *dev_priv = crtc->dev->dev_private;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	int pixel_size;
> +	int drain_latency;
> +	enum pipe pipe = intel_crtc->pipe;
> +	int plane_prec, prec_mult, plane_dl;
> +
> +	plane_dl = I915_READ(VLV_DDL(pipe)) & ~(DDL_PLANE_PRECISION_64 |
> +		   DRAIN_LATENCY_MASK | DDL_CURSOR_PRECISION_64 |
> +		   (DRAIN_LATENCY_MASK << DDL_CURSOR_SHIFT));
> +
> +	if (!intel_crtc_active(crtc)) {
> +		I915_WRITE(VLV_DDL(pipe), plane_dl);
> +		return;
> +	}
> +
> +	/* Primary plane Drain Latency */
> +	pixel_size = crtc->primary->fb->bits_per_pixel / 8;	/* BPP */
> +	if (vlv_compute_drain_latency(crtc, pixel_size, &prec_mult, &drain_latency)) {
> +		plane_prec = (prec_mult == DRAIN_LATENCY_PRECISION_64) ?
> +					   DDL_PLANE_PRECISION_64 :
> +					   DDL_PLANE_PRECISION_32;
> +		plane_dl = plane_dl | plane_prec | drain_latency;

Could use the |= operator here. Using = and | makes my brain think
there's something more subtle going on when there isn't.

>  	}
> +
> +	/* Cursor Drain Latency
> +	 * BPP is always 4 for cursor
> +	 */
> +	pixel_size = 4;
> +
> +	/* Program cursor DL only if it is enabled */
> +	if (intel_crtc->cursor_base &&
> +	    vlv_compute_drain_latency(crtc, pixel_size, &prec_mult, &drain_latency)) {
> +		plane_prec = (prec_mult == DRAIN_LATENCY_PRECISION_64) ?
> +					   DDL_CURSOR_PRECISION_64 :
> +					   DDL_CURSOR_PRECISION_32;
> +		plane_dl = plane_dl | plane_prec |
> +			   drain_latency << DDL_CURSOR_SHIFT;

Same here.

Also doesn't gcc warn about adding parenthesis here with | and << ?
The code does the right thing here, but IIRC gcc likes to warn about
these. We like to keep the code reasonably free of compiler and
sparse warnings.

> +	}
> +
> +	I915_WRITE(VLV_DDL(pipe), plane_dl);
>  }
>  
>  #define single_plane_enabled(mask) is_power_of_2(mask)
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index d75bed8..55f159d 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3904,6 +3904,7 @@  enum punit_power_well {
 #define DDL_PLANE_PRECISION_64		(1<<7)
 #define DDL_PLANE_PRECISION_32		(0<<7)
 #define DDL_PLANE_SHIFT			0
+#define DRAIN_LATENCY_MASK		0x7f
 
 /* FIFO watermark sizes etc */
 #define G4X_FIFO_LINE_SIZE	64
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 3544de0..ea64675 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1271,33 +1271,24 @@  static bool g4x_compute_srwm(struct drm_device *dev,
 			      display, cursor);
 }
 
-static bool vlv_compute_drain_latency(struct drm_device *dev,
-				     int plane,
-				     int *plane_prec_mult,
-				     int *plane_dl,
-				     int *cursor_prec_mult,
-				     int *cursor_dl)
+static bool vlv_compute_drain_latency(struct drm_crtc *crtc,
+				      int pixel_size,
+				      int *prec_mult,
+				      int *drain_latency)
 {
-	struct drm_crtc *crtc;
-	int clock, pixel_size;
 	int entries;
+	int clock = to_intel_crtc(crtc)->config.adjusted_mode.crtc_clock;
 
-	crtc = intel_get_crtc_for_plane(dev, plane);
-	if (!intel_crtc_active(crtc))
+	if (WARN(clock == 0, "Pixel clock is zero!\n"))
 		return false;
 
-	clock = to_intel_crtc(crtc)->config.adjusted_mode.crtc_clock;
-	pixel_size = crtc->primary->fb->bits_per_pixel / 8;	/* BPP */
+	if (WARN(pixel_size == 0, "Pixel size is zero!\n"))
+		return false;
 
 	entries = (clock / 1000) * pixel_size;
-	*plane_prec_mult = (entries > 128) ?
-		DRAIN_LATENCY_PRECISION_64 : DRAIN_LATENCY_PRECISION_32;
-	*plane_dl = (64 * (*plane_prec_mult) * 4) / entries;
-
-	entries = (clock / 1000) * 4;	/* BPP is always 4 for cursor */
-	*cursor_prec_mult = (entries > 128) ?
-		DRAIN_LATENCY_PRECISION_64 : DRAIN_LATENCY_PRECISION_32;
-	*cursor_dl = (64 * (*cursor_prec_mult) * 4) / entries;
+	*prec_mult = (entries > 128) ? DRAIN_LATENCY_PRECISION_64 :
+				       DRAIN_LATENCY_PRECISION_32;
+	*drain_latency = (64 * (*prec_mult) * 4) / entries;
 
 	return true;
 }
@@ -1312,24 +1303,47 @@  static bool vlv_compute_drain_latency(struct drm_device *dev,
 
 static void vlv_update_drain_latency(struct drm_crtc *crtc)
 {
-	struct drm_device *dev = crtc->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	enum pipe pipe = to_intel_crtc(crtc)->pipe;
-	int plane_prec, plane_dl;
-	int cursor_prec, cursor_dl;
-	int plane_prec_mult, cursor_prec_mult;
-
-	if (vlv_compute_drain_latency(dev, pipe, &plane_prec_mult, &plane_dl,
-				      &cursor_prec_mult, &cursor_dl)) {
-		cursor_prec = (cursor_prec_mult == DRAIN_LATENCY_PRECISION_64) ?
-			DDL_CURSOR_PRECISION_64 : DDL_CURSOR_PRECISION_32;
-		plane_prec = (plane_prec_mult == DRAIN_LATENCY_PRECISION_64) ?
-			DDL_PLANE_PRECISION_64 : DDL_PLANE_PRECISION_32;
-
-		I915_WRITE(VLV_DDL(pipe), cursor_prec |
-			   (cursor_dl << DDL_CURSOR_SHIFT) |
-			   plane_prec | (plane_dl << DDL_PLANE_SHIFT));
+	struct drm_i915_private *dev_priv = crtc->dev->dev_private;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	int pixel_size;
+	int drain_latency;
+	enum pipe pipe = intel_crtc->pipe;
+	int plane_prec, prec_mult, plane_dl;
+
+	plane_dl = I915_READ(VLV_DDL(pipe)) & ~(DDL_PLANE_PRECISION_64 |
+		   DRAIN_LATENCY_MASK | DDL_CURSOR_PRECISION_64 |
+		   (DRAIN_LATENCY_MASK << DDL_CURSOR_SHIFT));
+
+	if (!intel_crtc_active(crtc)) {
+		I915_WRITE(VLV_DDL(pipe), plane_dl);
+		return;
+	}
+
+	/* Primary plane Drain Latency */
+	pixel_size = crtc->primary->fb->bits_per_pixel / 8;	/* BPP */
+	if (vlv_compute_drain_latency(crtc, pixel_size, &prec_mult, &drain_latency)) {
+		plane_prec = (prec_mult == DRAIN_LATENCY_PRECISION_64) ?
+					   DDL_PLANE_PRECISION_64 :
+					   DDL_PLANE_PRECISION_32;
+		plane_dl = plane_dl | plane_prec | drain_latency;
 	}
+
+	/* Cursor Drain Latency
+	 * BPP is always 4 for cursor
+	 */
+	pixel_size = 4;
+
+	/* Program cursor DL only if it is enabled */
+	if (intel_crtc->cursor_base &&
+	    vlv_compute_drain_latency(crtc, pixel_size, &prec_mult, &drain_latency)) {
+		plane_prec = (prec_mult == DRAIN_LATENCY_PRECISION_64) ?
+					   DDL_CURSOR_PRECISION_64 :
+					   DDL_CURSOR_PRECISION_32;
+		plane_dl = plane_dl | plane_prec |
+			   drain_latency << DDL_CURSOR_SHIFT;
+	}
+
+	I915_WRITE(VLV_DDL(pipe), plane_dl);
 }
 
 #define single_plane_enabled(mask) is_power_of_2(mask)