[PATCH 0/3] Update DDL code to support sprite watermarks

[PATCH 0/3] Update DDL code to support sprite watermarks

[Gajanan Bhat]

Hi,
These patches depend on few patches from Ville and have been rebased on top of them.
Those pathes also need review and merge. Following are Ville's patches:
http://lists.freedesktop.org/archives/intel-gfx/2014-June/048150.html
http://lists.freedesktop.org/archives/intel-gfx/2014-June/048152.html
http://lists.freedesktop.org/archives/intel-gfx/2014-June/048153.html

Gajanan Bhat (3):
  drm/i915: Update DDL only for current CRTC
  drm/i915: Generalize drain latency computation
  drm/i915: Add sprite watermark programming for VLV and CHV

 drivers/gpu/drm/i915/i915_reg.h |    1 +
 drivers/gpu/drm/i915/intel_pm.c |  135 ++++++++++++++++++++++++++-------------
 2 files changed, 92 insertions(+), 44 deletions(-)

-- 
1.7.9.5

[PATCH 1/3] drm/i915: Update DDL only for current CRTC

[Gajanan Bhat]

Instead of looping through all CRTCs, update DDL for current CRTC for which
watermark is being updated.
CHV is confirmed to have precision of 32/64 which is same as VLV.

Signed-off-by: Gajanan Bhat <gajanan.bhat@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c |   25 +++++++++----------------
 1 file changed, 9 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index b881639..90df1e8 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1307,24 +1307,17 @@ static bool vlv_compute_drain_latency(struct drm_device *dev,
  * latency value.
  */
 
-static void vlv_update_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;
-
-	for_each_pipe(pipe) {
-		int plane_prec, plane_dl;
-		int cursor_prec, cursor_dl;
-		int plane_prec_mult, cursor_prec_mult;
+	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))
-			continue;
-
-		/*
-		 * FIXME CHV spec still lists 16 and 32 as the precision
-		 * values. Need to figure out if spec is outdated or what.
-		 */
+	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) ?
@@ -1349,7 +1342,7 @@ static void valleyview_update_wm(struct drm_crtc *crtc)
 	unsigned int enabled = 0;
 	bool cxsr_enabled;
 
-	vlv_update_drain_latency(dev);
+	vlv_update_drain_latency(crtc);
 
 	if (g4x_compute_wm0(dev, PIPE_A,
 			    &valleyview_wm_info, latency_ns,
-- 
1.7.9.5

Re: [PATCH 1/3] drm/i915: Update DDL only for current CRTC

[Imre Deak]

[-- Attachment #1.1: Type: text/plain, Size: 2295 bytes --]

On Wed, 2014-07-16 at 18:24 +0530, Gajanan Bhat wrote:
> Instead of looping through all CRTCs, update DDL for current CRTC for which
> watermark is being updated.
> CHV is confirmed to have precision of 32/64 which is same as VLV.
> 
> Signed-off-by: Gajanan Bhat <gajanan.bhat@intel.com>

Looks ok to me:
Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_pm.c |   25 +++++++++----------------
>  1 file changed, 9 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index b881639..90df1e8 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1307,24 +1307,17 @@ static bool vlv_compute_drain_latency(struct drm_device *dev,
>   * latency value.
>   */
>  
> -static void vlv_update_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;
> -
> -	for_each_pipe(pipe) {
> -		int plane_prec, plane_dl;
> -		int cursor_prec, cursor_dl;
> -		int plane_prec_mult, cursor_prec_mult;
> +	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))
> -			continue;
> -
> -		/*
> -		 * FIXME CHV spec still lists 16 and 32 as the precision
> -		 * values. Need to figure out if spec is outdated or what.
> -		 */
> +	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) ?
> @@ -1349,7 +1342,7 @@ static void valleyview_update_wm(struct drm_crtc *crtc)
>  	unsigned int enabled = 0;
>  	bool cxsr_enabled;
>  
> -	vlv_update_drain_latency(dev);
> +	vlv_update_drain_latency(crtc);
>  
>  	if (g4x_compute_wm0(dev, PIPE_A,
>  			    &valleyview_wm_info, latency_ns,


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

[PATCH 2/3] drm/i915: Generalize drain latency computation

[Gajanan Bhat]

Modify drain latency computation to use it for any plane. Same function can be
used for primary, cursor and sprite planes.

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

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index d2a220b..a1260a2 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3877,6 +3877,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_MAX		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 90df1e8..f3a3e90 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1268,33 +1268,21 @@ 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 (clock == 0 || pixel_size == 0)
 		return false;
 
-	clock = to_intel_crtc(crtc)->config.adjusted_mode.crtc_clock;
-	pixel_size = crtc->primary->fb->bits_per_pixel / 8;	/* BPP */
-
-	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;
+	entries = DIV_ROUND_UP(clock, 1000) * pixel_size;
+	*prec_mult = (entries > 128) ? DRAIN_LATENCY_PRECISION_64 :
+				       DRAIN_LATENCY_PRECISION_32;
+	*drain_latency = (64 * (*prec_mult) * 4) / entries;
 
 	return true;
 }
@@ -1309,24 +1297,46 @@ 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;
+	struct drm_i915_private *dev_priv = crtc->dev->dev_private;
+	int pixel_size;
+	int drain_latency;
 	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;
+	int plane_prec, prec_mult, plane_dl;
 
-	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;
+	plane_dl = I915_READ(VLV_DDL(pipe)) & ~DDL_PLANE_PRECISION_64 &
+		   ~DRAIN_LATENCY_MAX & ~DDL_CURSOR_PRECISION_64 &
+		   ~(DRAIN_LATENCY_MAX << DDL_CURSOR_SHIFT);
 
-		I915_WRITE(VLV_DDL(pipe), cursor_prec |
-			   (cursor_dl << DDL_CURSOR_SHIFT) |
-			   plane_prec | (plane_dl << DDL_PLANE_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 ((I915_READ(CURCNTR(pipe)) & CURSOR_MODE) &&
+	    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)
-- 
1.7.9.5

Re: [PATCH 2/3] drm/i915: Generalize drain latency computation

[Imre Deak]

[-- Attachment #1.1: Type: text/plain, Size: 5471 bytes --]

On Wed, 2014-07-16 at 18:24 +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.
> 
> Signed-off-by: Gajanan Bhat <gajanan.bhat@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h |    1 +
>  drivers/gpu/drm/i915/intel_pm.c |   82 ++++++++++++++++++++++-----------------
>  2 files changed, 47 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index d2a220b..a1260a2 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3877,6 +3877,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_MAX		0x7f

_MASK is the standard postfix in the driver.

>  
>  /* 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 90df1e8..f3a3e90 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1268,33 +1268,21 @@ 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 (clock == 0 || pixel_size == 0)

This would mean a driver bug, so needs to be wrapped in a WARN().

>  		return false;
>  
> -	clock = to_intel_crtc(crtc)->config.adjusted_mode.crtc_clock;
> -	pixel_size = crtc->primary->fb->bits_per_pixel / 8;	/* BPP */
> -
> -	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;
> +	entries = DIV_ROUND_UP(clock, 1000) * pixel_size;
> +	*prec_mult = (entries > 128) ? DRAIN_LATENCY_PRECISION_64 :
> +				       DRAIN_LATENCY_PRECISION_32;
> +	*drain_latency = (64 * (*prec_mult) * 4) / entries;
>  
>  	return true;
>  }
> @@ -1309,24 +1297,46 @@ 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;
> +	struct drm_i915_private *dev_priv = crtc->dev->dev_private;
> +	int pixel_size;
> +	int drain_latency;
>  	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;
> +	int plane_prec, prec_mult, plane_dl;
>  
> -	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;
> +	plane_dl = I915_READ(VLV_DDL(pipe)) & ~DDL_PLANE_PRECISION_64 &
> +		   ~DRAIN_LATENCY_MAX & ~DDL_CURSOR_PRECISION_64 &
> +		   ~(DRAIN_LATENCY_MAX << DDL_CURSOR_SHIFT);

A tad simpler and more standard way for masking is ~(X | Y | Z).
 
> -		I915_WRITE(VLV_DDL(pipe), cursor_prec |
> -			   (cursor_dl << DDL_CURSOR_SHIFT) |
> -			   plane_prec | (plane_dl << DDL_PLANE_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 ((I915_READ(CURCNTR(pipe)) & CURSOR_MODE) &&

Checking intel_crtc->cursor_base would be cheaper.

> +	    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;

This would overwrite the above assignment of plane_dl.

> +	}
> +
> +	I915_WRITE(VLV_DDL(pipe), plane_dl);
>  }
>  
>  #define single_plane_enabled(mask) is_power_of_2(mask)


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH 2/3] drm/i915: Generalize drain latency computation

[Ville Syrjälä]

On Wed, Jul 16, 2014 at 06:24:04PM +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.
> 
> Signed-off-by: Gajanan Bhat <gajanan.bhat@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h |    1 +
>  drivers/gpu/drm/i915/intel_pm.c |   82 ++++++++++++++++++++++-----------------
>  2 files changed, 47 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index d2a220b..a1260a2 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3877,6 +3877,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_MAX		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 90df1e8..f3a3e90 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1268,33 +1268,21 @@ 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 (clock == 0 || pixel_size == 0)
>  		return false;
>  
> -	clock = to_intel_crtc(crtc)->config.adjusted_mode.crtc_clock;
> -	pixel_size = crtc->primary->fb->bits_per_pixel / 8;	/* BPP */
> -
> -	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;
> +	entries = DIV_ROUND_UP(clock, 1000) * pixel_size;

Yeah rounding the clock up should be safe, rounding down not so much.
But this change should really be a separate patch.

> +	*prec_mult = (entries > 128) ? DRAIN_LATENCY_PRECISION_64 :
> +				       DRAIN_LATENCY_PRECISION_32;
> +	*drain_latency = (64 * (*prec_mult) * 4) / entries;

Another thing you could fix is making sure the result is <=0x7f. It's
quite possible we'd exceed that (all that's needed is a 16bpp fb and
<32MHz pixel clock, which isn't all that far fetched). This could
be part of the rounding fix patch since it's somewhat related.

>  
>  	return true;
>  }
> @@ -1309,24 +1297,46 @@ 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;
> +	struct drm_i915_private *dev_priv = crtc->dev->dev_private;
> +	int pixel_size;
> +	int drain_latency;
>  	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;
> +	int plane_prec, prec_mult, plane_dl;
>  
> -	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;
> +	plane_dl = I915_READ(VLV_DDL(pipe)) & ~DDL_PLANE_PRECISION_64 &
> +		   ~DRAIN_LATENCY_MAX & ~DDL_CURSOR_PRECISION_64 &
> +		   ~(DRAIN_LATENCY_MAX << DDL_CURSOR_SHIFT);
>  
> -		I915_WRITE(VLV_DDL(pipe), cursor_prec |
> -			   (cursor_dl << DDL_CURSOR_SHIFT) |
> -			   plane_prec | (plane_dl << DDL_PLANE_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 ((I915_READ(CURCNTR(pipe)) & CURSOR_MODE) &&
> +	    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)
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

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

[Gajanan Bhat]

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;
 	}
+
+	/* 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)
-- 
1.7.9.5

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

[Ville Syrjälä]

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

-- 
Ville Syrjälä
Intel OTC

[PATCH v3] drm/i915: Generalize drain latency computation

[Gajanan Bhat]

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

v3: Changed to bitwise shorthand operator for plane_dl assignment.

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

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 187f862..08916df 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4009,6 +4009,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 ba8dfeb..cabea4b 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1277,33 +1277,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;
 }
@@ -1318,24 +1309,46 @@ 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_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_prec | (drain_latency << DDL_CURSOR_SHIFT);
+	}
+
+	I915_WRITE(VLV_DDL(pipe), plane_dl);
 }
 
 #define single_plane_enabled(mask) is_power_of_2(mask)
-- 
1.7.9.5

Re: [PATCH v3] drm/i915: Generalize drain latency computation

[Ville Syrjälä]

On Thu, Aug 07, 2014 at 01:58:24AM +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
> 
> v3: Changed to bitwise shorthand operator for plane_dl assignment.
> 
> Signed-off-by: Gajanan Bhat <gajanan.bhat@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/i915_reg.h |    1 +
>  drivers/gpu/drm/i915/intel_pm.c |   87 ++++++++++++++++++++++-----------------
>  2 files changed, 51 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 187f862..08916df 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4009,6 +4009,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 ba8dfeb..cabea4b 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1277,33 +1277,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;
>  }
> @@ -1318,24 +1309,46 @@ 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_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_prec | (drain_latency << DDL_CURSOR_SHIFT);
> +	}
> +
> +	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

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH v3] drm/i915: Generalize drain latency computation

[Daniel Vetter]

On Wed, Aug 06, 2014 at 06:07:12PM +0300, Ville Syrjälä wrote:
> On Thu, Aug 07, 2014 at 01:58:24AM +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
> > 
> > v3: Changed to bitwise shorthand operator for plane_dl assignment.
> > 
> > Signed-off-by: Gajanan Bhat <gajanan.bhat@intel.com>
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Pulled in these two patches.
-Danil

> 
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h |    1 +
> >  drivers/gpu/drm/i915/intel_pm.c |   87 ++++++++++++++++++++++-----------------
> >  2 files changed, 51 insertions(+), 37 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 187f862..08916df 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -4009,6 +4009,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 ba8dfeb..cabea4b 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -1277,33 +1277,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;
> >  }
> > @@ -1318,24 +1309,46 @@ 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_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_prec | (drain_latency << DDL_CURSOR_SHIFT);
> > +	}
> > +
> > +	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
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> 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

[PATCH 2/2] drm/i915: Round-up clock and limit drain latency

[Gajanan Bhat]

Round up clock computation and limit drain latency to maximum of 0x7F.

Signed-off-by: Gajanan Bhat <gajanan.bhat@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index ea64675..5e81c49 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1285,11 +1285,14 @@ static bool vlv_compute_drain_latency(struct drm_crtc *crtc,
 	if (WARN(pixel_size == 0, "Pixel size is zero!\n"))
 		return false;
 
-	entries = (clock / 1000) * pixel_size;
+	entries = DIV_ROUND_UP(clock, 1000) * pixel_size;
 	*prec_mult = (entries > 128) ? DRAIN_LATENCY_PRECISION_64 :
 				       DRAIN_LATENCY_PRECISION_32;
 	*drain_latency = (64 * (*prec_mult) * 4) / entries;
 
+	if (*drain_latency > DRAIN_LATENCY_MASK)
+		*drain_latency = DRAIN_LATENCY_MASK;
+
 	return true;
 }
 
-- 
1.7.9.5

Re: [PATCH 2/2] drm/i915: Round-up clock and limit drain latency

[Ville Syrjälä]

On Tue, Aug 05, 2014 at 11:15:54PM +0530, Gajanan Bhat wrote:
> Round up clock computation and limit drain latency to maximum of 0x7F.
> 
> Signed-off-by: Gajanan Bhat <gajanan.bhat@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index ea64675..5e81c49 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1285,11 +1285,14 @@ static bool vlv_compute_drain_latency(struct drm_crtc *crtc,
>  	if (WARN(pixel_size == 0, "Pixel size is zero!\n"))
>  		return false;
>  
> -	entries = (clock / 1000) * pixel_size;
> +	entries = DIV_ROUND_UP(clock, 1000) * pixel_size;
>  	*prec_mult = (entries > 128) ? DRAIN_LATENCY_PRECISION_64 :
>  				       DRAIN_LATENCY_PRECISION_32;
>  	*drain_latency = (64 * (*prec_mult) * 4) / entries;
>  
> +	if (*drain_latency > DRAIN_LATENCY_MASK)
> +		*drain_latency = DRAIN_LATENCY_MASK;

I would have probably written this as:
*drain_latency = min(..., DRAIN_LATENCY_MASK);

But that's just a style I like. The patch looks fine to me so:
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +
>  	return true;
>  }
>  
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

[PATCH 3/3] drm/i915: Add sprite watermark programming for VLV and CHV

[Gajanan Bhat]

Program DDL register as part sprite watermark programming for CHV and VLV.

Signed-off-by: Gajanan Bhat <gajanan.bhat@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c |   44 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index f3a3e90..0f439f7 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1405,6 +1405,48 @@ static void valleyview_update_wm(struct drm_crtc *crtc)
 		intel_set_memory_cxsr(dev_priv, true);
 }
 
+static void valleyview_update_sprite_wm(struct drm_plane *plane,
+					struct drm_crtc *crtc,
+					uint32_t sprite_width,
+					uint32_t sprite_height,
+					int pixel_size,
+					bool enabled, bool scaled)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int pipe = to_intel_plane(plane)->pipe;
+	int drain_latency;
+	int plane_prec;
+	int sprite_dl;
+	int prec_mult;
+
+	if (to_intel_plane(plane)->plane == 0)
+		sprite_dl = I915_READ(VLV_DDL(pipe)) & ~DDL_SPRITE0_PRECISION_64 &
+			    ~(DRAIN_LATENCY_MAX << DDL_SPRITE0_SHIFT);
+	else
+		sprite_dl = I915_READ(VLV_DDL(pipe)) & ~DDL_SPRITE1_PRECISION_64 &
+			    ~(DRAIN_LATENCY_MAX << DDL_SPRITE1_SHIFT);
+
+	if (enabled && vlv_compute_drain_latency(crtc, pixel_size, &prec_mult,
+						 &drain_latency)) {
+		if (to_intel_plane(plane)->plane == 0) {
+			plane_prec = (prec_mult == DRAIN_LATENCY_PRECISION_64) ?
+						   DDL_SPRITE0_PRECISION_64 :
+						   DDL_SPRITE0_PRECISION_32;
+			sprite_dl = sprite_dl | plane_prec |
+				    drain_latency << DDL_SPRITE0_SHIFT;
+		} else {
+			plane_prec = (prec_mult == DRAIN_LATENCY_PRECISION_64) ?
+						   DDL_SPRITE1_PRECISION_64 :
+						   DDL_SPRITE1_PRECISION_32;
+			sprite_dl = sprite_dl | plane_prec |
+				    drain_latency << DDL_SPRITE1_SHIFT;
+		}
+	}
+
+	I915_WRITE(VLV_DDL(pipe), sprite_dl);
+}
+
 static void g4x_update_wm(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
@@ -6851,10 +6893,12 @@ void intel_init_pm(struct drm_device *dev)
 			dev_priv->display.init_clock_gating = gen8_init_clock_gating;
 	} else if (IS_CHERRYVIEW(dev)) {
 		dev_priv->display.update_wm = valleyview_update_wm;
+		dev_priv->display.update_sprite_wm = valleyview_update_sprite_wm;
 		dev_priv->display.init_clock_gating =
 			cherryview_init_clock_gating;
 	} else if (IS_VALLEYVIEW(dev)) {
 		dev_priv->display.update_wm = valleyview_update_wm;
+		dev_priv->display.update_sprite_wm = valleyview_update_sprite_wm;
 		dev_priv->display.init_clock_gating =
 			valleyview_init_clock_gating;
 	} else if (IS_PINEVIEW(dev)) {
-- 
1.7.9.5

Re: [PATCH 3/3] drm/i915: Add sprite watermark programming for VLV and CHV

[Imre Deak]

[-- Attachment #1.1: Type: text/plain, Size: 3164 bytes --]

On Wed, 2014-07-16 at 18:24 +0530, Gajanan Bhat wrote:
> Program DDL register as part sprite watermark programming for CHV and VLV.
> 
> Signed-off-by: Gajanan Bhat <gajanan.bhat@intel.com>

This looks ok, but could you confirm, ideally referencing some document,
that we don't need to program any of the sprite watermark level
registers along with the DDL values? Specifically I mean the FW7, FW8
registers.

--Imre

> ---
>  drivers/gpu/drm/i915/intel_pm.c |   44 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index f3a3e90..0f439f7 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1405,6 +1405,48 @@ static void valleyview_update_wm(struct drm_crtc *crtc)
>  		intel_set_memory_cxsr(dev_priv, true);
>  }
>  
> +static void valleyview_update_sprite_wm(struct drm_plane *plane,
> +					struct drm_crtc *crtc,
> +					uint32_t sprite_width,
> +					uint32_t sprite_height,
> +					int pixel_size,
> +					bool enabled, bool scaled)
> +{
> +	struct drm_device *dev = crtc->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	int pipe = to_intel_plane(plane)->pipe;
> +	int drain_latency;
> +	int plane_prec;
> +	int sprite_dl;
> +	int prec_mult;
> +
> +	if (to_intel_plane(plane)->plane == 0)
> +		sprite_dl = I915_READ(VLV_DDL(pipe)) & ~DDL_SPRITE0_PRECISION_64 &
> +			    ~(DRAIN_LATENCY_MAX << DDL_SPRITE0_SHIFT);
> +	else
> +		sprite_dl = I915_READ(VLV_DDL(pipe)) & ~DDL_SPRITE1_PRECISION_64 &
> +			    ~(DRAIN_LATENCY_MAX << DDL_SPRITE1_SHIFT);
> +
> +	if (enabled && vlv_compute_drain_latency(crtc, pixel_size, &prec_mult,
> +						 &drain_latency)) {
> +		if (to_intel_plane(plane)->plane == 0) {
> +			plane_prec = (prec_mult == DRAIN_LATENCY_PRECISION_64) ?
> +						   DDL_SPRITE0_PRECISION_64 :
> +						   DDL_SPRITE0_PRECISION_32;
> +			sprite_dl = sprite_dl | plane_prec |
> +				    drain_latency << DDL_SPRITE0_SHIFT;
> +		} else {
> +			plane_prec = (prec_mult == DRAIN_LATENCY_PRECISION_64) ?
> +						   DDL_SPRITE1_PRECISION_64 :
> +						   DDL_SPRITE1_PRECISION_32;
> +			sprite_dl = sprite_dl | plane_prec |
> +				    drain_latency << DDL_SPRITE1_SHIFT;
> +		}
> +	}
> +
> +	I915_WRITE(VLV_DDL(pipe), sprite_dl);
> +}
> +
>  static void g4x_update_wm(struct drm_crtc *crtc)
>  {
>  	struct drm_device *dev = crtc->dev;
> @@ -6851,10 +6893,12 @@ void intel_init_pm(struct drm_device *dev)
>  			dev_priv->display.init_clock_gating = gen8_init_clock_gating;
>  	} else if (IS_CHERRYVIEW(dev)) {
>  		dev_priv->display.update_wm = valleyview_update_wm;
> +		dev_priv->display.update_sprite_wm = valleyview_update_sprite_wm;
>  		dev_priv->display.init_clock_gating =
>  			cherryview_init_clock_gating;
>  	} else if (IS_VALLEYVIEW(dev)) {
>  		dev_priv->display.update_wm = valleyview_update_wm;
> +		dev_priv->display.update_sprite_wm = valleyview_update_sprite_wm;
>  		dev_priv->display.init_clock_gating =
>  			valleyview_init_clock_gating;
>  	} else if (IS_PINEVIEW(dev)) {


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH 3/3] drm/i915: Add sprite watermark programming for VLV and CHV

[Bhat, Gajanan]

On 7/31/2014 7:14 PM, Imre Deak wrote:
> On Wed, 2014-07-16 at 18:24 +0530, Gajanan Bhat wrote:
>> Program DDL register as part sprite watermark programming for CHV and VLV.
>>
>> Signed-off-by: Gajanan Bhat <gajanan.bhat@intel.com>
> This looks ok, but could you confirm, ideally referencing some document,
> that we don't need to program any of the sprite watermark level
> registers along with the DDL values? Specifically I mean the FW7, FW8
> registers.
>
> --Imre
I have looked at the B-spec to again confirm my understanding. The 
chicken bit in CBR1
specifies which mechanism (PND or watermark) to be used. In our case we 
are using only the PND with DDL.
I couldn't find any reference which explicitly tells that both DDL and 
watermarks
should be programmed together. Also we have tested this code and so far 
it has not caused any problem.
-Gajanan

>> ---
>>   drivers/gpu/drm/i915/intel_pm.c |   44 +++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 44 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> index f3a3e90..0f439f7 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -1405,6 +1405,48 @@ static void valleyview_update_wm(struct drm_crtc *crtc)
>>   		intel_set_memory_cxsr(dev_priv, true);
>>   }
>>   
>> +static void valleyview_update_sprite_wm(struct drm_plane *plane,
>> +					struct drm_crtc *crtc,
>> +					uint32_t sprite_width,
>> +					uint32_t sprite_height,
>> +					int pixel_size,
>> +					bool enabled, bool scaled)
>> +{
>> +	struct drm_device *dev = crtc->dev;
>> +	struct drm_i915_private *dev_priv = dev->dev_private;
>> +	int pipe = to_intel_plane(plane)->pipe;
>> +	int drain_latency;
>> +	int plane_prec;
>> +	int sprite_dl;
>> +	int prec_mult;
>> +
>> +	if (to_intel_plane(plane)->plane == 0)
>> +		sprite_dl = I915_READ(VLV_DDL(pipe)) & ~DDL_SPRITE0_PRECISION_64 &
>> +			    ~(DRAIN_LATENCY_MAX << DDL_SPRITE0_SHIFT);
>> +	else
>> +		sprite_dl = I915_READ(VLV_DDL(pipe)) & ~DDL_SPRITE1_PRECISION_64 &
>> +			    ~(DRAIN_LATENCY_MAX << DDL_SPRITE1_SHIFT);
>> +
>> +	if (enabled && vlv_compute_drain_latency(crtc, pixel_size, &prec_mult,
>> +						 &drain_latency)) {
>> +		if (to_intel_plane(plane)->plane == 0) {
>> +			plane_prec = (prec_mult == DRAIN_LATENCY_PRECISION_64) ?
>> +						   DDL_SPRITE0_PRECISION_64 :
>> +						   DDL_SPRITE0_PRECISION_32;
>> +			sprite_dl = sprite_dl | plane_prec |
>> +				    drain_latency << DDL_SPRITE0_SHIFT;
>> +		} else {
>> +			plane_prec = (prec_mult == DRAIN_LATENCY_PRECISION_64) ?
>> +						   DDL_SPRITE1_PRECISION_64 :
>> +						   DDL_SPRITE1_PRECISION_32;
>> +			sprite_dl = sprite_dl | plane_prec |
>> +				    drain_latency << DDL_SPRITE1_SHIFT;
>> +		}
>> +	}
>> +
>> +	I915_WRITE(VLV_DDL(pipe), sprite_dl);
>> +}
>> +
>>   static void g4x_update_wm(struct drm_crtc *crtc)
>>   {
>>   	struct drm_device *dev = crtc->dev;
>> @@ -6851,10 +6893,12 @@ void intel_init_pm(struct drm_device *dev)
>>   			dev_priv->display.init_clock_gating = gen8_init_clock_gating;
>>   	} else if (IS_CHERRYVIEW(dev)) {
>>   		dev_priv->display.update_wm = valleyview_update_wm;
>> +		dev_priv->display.update_sprite_wm = valleyview_update_sprite_wm;
>>   		dev_priv->display.init_clock_gating =
>>   			cherryview_init_clock_gating;
>>   	} else if (IS_VALLEYVIEW(dev)) {
>>   		dev_priv->display.update_wm = valleyview_update_wm;
>> +		dev_priv->display.update_sprite_wm = valleyview_update_sprite_wm;
>>   		dev_priv->display.init_clock_gating =
>>   			valleyview_init_clock_gating;
>>   	} else if (IS_PINEVIEW(dev)) {

Re: [PATCH 3/3] drm/i915: Add sprite watermark programming for VLV and CHV

[Imre Deak]

[-- Attachment #1.1: Type: text/plain, Size: 4329 bytes --]

On Thu, 2014-08-07 at 11:26 +0530, Bhat, Gajanan wrote:
> On 7/31/2014 7:14 PM, Imre Deak wrote:
> > On Wed, 2014-07-16 at 18:24 +0530, Gajanan Bhat wrote:
> >> Program DDL register as part sprite watermark programming for CHV and VLV.
> >>
> >> Signed-off-by: Gajanan Bhat <gajanan.bhat@intel.com>
> > This looks ok, but could you confirm, ideally referencing some document,
> > that we don't need to program any of the sprite watermark level
> > registers along with the DDL values? Specifically I mean the FW7, FW8
> > registers.
> >
> I have looked at the B-spec to again confirm my understanding. The
> chicken bit in CBR1 specifies which mechanism (PND or watermark) to be
> used. In our case we are using only the PND with DDL. I couldn't find
> any reference which explicitly tells that both DDL and watermarks
> should be programmed together. Also we have tested this code and so far
> it has not caused any problem.

Thanks for clarifying this, the docs on this are fuzzy to me. Earlier
Cesar and Ville also pointed out that the PND mechanism depends on the
MI_ARB[trickle feed] flag being enabled, which is afaics disabled on
VLV. But based on what you say I understand that the DDL values are not
used in the 'watermark mode', so I'm ok with your change. Perhaps a
comment about this could be added.

--Imre

> -Gajanan
> 
> >> ---
> >>   drivers/gpu/drm/i915/intel_pm.c |   44 +++++++++++++++++++++++++++++++++++++++
> >>   1 file changed, 44 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> >> index f3a3e90..0f439f7 100644
> >> --- a/drivers/gpu/drm/i915/intel_pm.c
> >> +++ b/drivers/gpu/drm/i915/intel_pm.c
> >> @@ -1405,6 +1405,48 @@ static void valleyview_update_wm(struct drm_crtc *crtc)
> >>   		intel_set_memory_cxsr(dev_priv, true);
> >>   }
> >>   
> >> +static void valleyview_update_sprite_wm(struct drm_plane *plane,
> >> +					struct drm_crtc *crtc,
> >> +					uint32_t sprite_width,
> >> +					uint32_t sprite_height,
> >> +					int pixel_size,
> >> +					bool enabled, bool scaled)
> >> +{
> >> +	struct drm_device *dev = crtc->dev;
> >> +	struct drm_i915_private *dev_priv = dev->dev_private;
> >> +	int pipe = to_intel_plane(plane)->pipe;
> >> +	int drain_latency;
> >> +	int plane_prec;
> >> +	int sprite_dl;
> >> +	int prec_mult;
> >> +
> >> +	if (to_intel_plane(plane)->plane == 0)
> >> +		sprite_dl = I915_READ(VLV_DDL(pipe)) & ~DDL_SPRITE0_PRECISION_64 &
> >> +			    ~(DRAIN_LATENCY_MAX << DDL_SPRITE0_SHIFT);
> >> +	else
> >> +		sprite_dl = I915_READ(VLV_DDL(pipe)) & ~DDL_SPRITE1_PRECISION_64 &
> >> +			    ~(DRAIN_LATENCY_MAX << DDL_SPRITE1_SHIFT);
> >> +
> >> +	if (enabled && vlv_compute_drain_latency(crtc, pixel_size, &prec_mult,
> >> +						 &drain_latency)) {
> >> +		if (to_intel_plane(plane)->plane == 0) {
> >> +			plane_prec = (prec_mult == DRAIN_LATENCY_PRECISION_64) ?
> >> +						   DDL_SPRITE0_PRECISION_64 :
> >> +						   DDL_SPRITE0_PRECISION_32;
> >> +			sprite_dl = sprite_dl | plane_prec |
> >> +				    drain_latency << DDL_SPRITE0_SHIFT;
> >> +		} else {
> >> +			plane_prec = (prec_mult == DRAIN_LATENCY_PRECISION_64) ?
> >> +						   DDL_SPRITE1_PRECISION_64 :
> >> +						   DDL_SPRITE1_PRECISION_32;
> >> +			sprite_dl = sprite_dl | plane_prec |
> >> +				    drain_latency << DDL_SPRITE1_SHIFT;
> >> +		}
> >> +	}
> >> +
> >> +	I915_WRITE(VLV_DDL(pipe), sprite_dl);
> >> +}
> >> +
> >>   static void g4x_update_wm(struct drm_crtc *crtc)
> >>   {
> >>   	struct drm_device *dev = crtc->dev;
> >> @@ -6851,10 +6893,12 @@ void intel_init_pm(struct drm_device *dev)
> >>   			dev_priv->display.init_clock_gating = gen8_init_clock_gating;
> >>   	} else if (IS_CHERRYVIEW(dev)) {
> >>   		dev_priv->display.update_wm = valleyview_update_wm;
> >> +		dev_priv->display.update_sprite_wm = valleyview_update_sprite_wm;
> >>   		dev_priv->display.init_clock_gating =
> >>   			cherryview_init_clock_gating;
> >>   	} else if (IS_VALLEYVIEW(dev)) {
> >>   		dev_priv->display.update_wm = valleyview_update_wm;
> >> +		dev_priv->display.update_sprite_wm = valleyview_update_sprite_wm;
> >>   		dev_priv->display.init_clock_gating =
> >>   			valleyview_init_clock_gating;
> >>   	} else if (IS_PINEVIEW(dev)) {
> 


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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