* [PATCH] drm/i915/chv: Use 16 and 32 for low and high drain latency precision.
@ 2014-10-17 15:05 Rodrigo Vivi
2014-10-21 22:42 ` Boyer, Wayne
2014-10-22 6:57 ` Ville Syrjälä
0 siblings, 2 replies; 7+ messages in thread
From: Rodrigo Vivi @ 2014-10-17 15:05 UTC (permalink / raw)
To: intel-gfx; +Cc: Rodrigo Vivi
Current chv spec teels we can only use either 16 or 32 bits as precision.
Although in the past VLV went from 16/32 to 32/64 and spec might not be updated,
these precision values brings stability and fixes some issues Wayne was facing.
Cc: Wayne Boyer <wayne.boyer@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 13 +++++++------
drivers/gpu/drm/i915/intel_pm.c | 40 +++++++++++++++++++++++++---------------
2 files changed, 32 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 6db369a..dcd5650 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4054,17 +4054,18 @@ enum punit_power_well {
#define DSPFW_PLANEA_WM1_HI_MASK (1<<0)
/* drain latency register values*/
+#define DRAIN_LATENCY_PRECISION_16 16
#define DRAIN_LATENCY_PRECISION_32 32
#define DRAIN_LATENCY_PRECISION_64 64
#define VLV_DDL(pipe) (VLV_DISPLAY_BASE + 0x70050 + 4 * (pipe))
-#define DDL_CURSOR_PRECISION_64 (1<<31)
-#define DDL_CURSOR_PRECISION_32 (0<<31)
+#define DDL_CURSOR_PRECISION_HIGH (1<<31)
+#define DDL_CURSOR_PRECISION_LOW (0<<31)
#define DDL_CURSOR_SHIFT 24
-#define DDL_SPRITE_PRECISION_64(sprite) (1<<(15+8*(sprite)))
-#define DDL_SPRITE_PRECISION_32(sprite) (0<<(15+8*(sprite)))
+#define DDL_SPRITE_PRECISION_HIGH(sprite) (1<<(15+8*(sprite)))
+#define DDL_SPRITE_PRECISION_LOW(sprite) (0<<(15+8*(sprite)))
#define DDL_SPRITE_SHIFT(sprite) (8+8*(sprite))
-#define DDL_PLANE_PRECISION_64 (1<<7)
-#define DDL_PLANE_PRECISION_32 (0<<7)
+#define DDL_PLANE_PRECISION_HIGH (1<<7)
+#define DDL_PLANE_PRECISION_LOW (0<<7)
#define DDL_PLANE_SHIFT 0
#define DRAIN_LATENCY_MASK 0x7f
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index daa99e7..50c3512 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1345,6 +1345,7 @@ static bool vlv_compute_drain_latency(struct drm_crtc *crtc,
int *prec_mult,
int *drain_latency)
{
+ struct drm_device *dev = crtc->dev;
int entries;
int clock = to_intel_crtc(crtc)->config.adjusted_mode.crtc_clock;
@@ -1355,8 +1356,12 @@ static bool vlv_compute_drain_latency(struct drm_crtc *crtc,
return false;
entries = DIV_ROUND_UP(clock, 1000) * pixel_size;
- *prec_mult = (entries > 128) ? DRAIN_LATENCY_PRECISION_64 :
- DRAIN_LATENCY_PRECISION_32;
+ if (IS_CHERRYVIEW(dev))
+ *prec_mult = (entries > 128) ? DRAIN_LATENCY_PRECISION_32 :
+ DRAIN_LATENCY_PRECISION_16;
+ else
+ *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)
@@ -1375,15 +1380,18 @@ static bool vlv_compute_drain_latency(struct drm_crtc *crtc,
static void vlv_update_drain_latency(struct drm_crtc *crtc)
{
- struct drm_i915_private *dev_priv = crtc->dev->dev_private;
+ 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 pixel_size;
int drain_latency;
enum pipe pipe = intel_crtc->pipe;
int plane_prec, prec_mult, plane_dl;
+ int high_precision = IS_CHERRYVIEW(dev) ? DRAIN_LATENCY_PRECISION_32 :
+ DRAIN_LATENCY_PRECISION_64;
- plane_dl = I915_READ(VLV_DDL(pipe)) & ~(DDL_PLANE_PRECISION_64 |
- DRAIN_LATENCY_MASK | DDL_CURSOR_PRECISION_64 |
+ plane_dl = I915_READ(VLV_DDL(pipe)) & ~(DDL_PLANE_PRECISION_HIGH |
+ DRAIN_LATENCY_MASK | DDL_CURSOR_PRECISION_HIGH |
(DRAIN_LATENCY_MASK << DDL_CURSOR_SHIFT));
if (!intel_crtc_active(crtc)) {
@@ -1394,9 +1402,9 @@ static void vlv_update_drain_latency(struct drm_crtc *crtc)
/* 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_prec = (prec_mult == high_precision) ?
+ DDL_PLANE_PRECISION_HIGH :
+ DDL_PLANE_PRECISION_LOW;
plane_dl |= plane_prec | drain_latency;
}
@@ -1408,9 +1416,9 @@ static void vlv_update_drain_latency(struct drm_crtc *crtc)
/* 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_prec = (prec_mult == high_precision) ?
+ DDL_CURSOR_PRECISION_HIGH :
+ DDL_CURSOR_PRECISION_LOW;
plane_dl |= plane_prec | (drain_latency << DDL_CURSOR_SHIFT);
}
@@ -1578,15 +1586,17 @@ static void valleyview_update_sprite_wm(struct drm_plane *plane,
int plane_prec;
int sprite_dl;
int prec_mult;
+ int high_precision = IS_CHERRYVIEW(dev) ? DRAIN_LATENCY_PRECISION_32 :
+ DRAIN_LATENCY_PRECISION_64;
- sprite_dl = I915_READ(VLV_DDL(pipe)) & ~(DDL_SPRITE_PRECISION_64(sprite) |
+ sprite_dl = I915_READ(VLV_DDL(pipe)) & ~(DDL_SPRITE_PRECISION_HIGH(sprite) |
(DRAIN_LATENCY_MASK << DDL_SPRITE_SHIFT(sprite)));
if (enabled && vlv_compute_drain_latency(crtc, pixel_size, &prec_mult,
&drain_latency)) {
- plane_prec = (prec_mult == DRAIN_LATENCY_PRECISION_64) ?
- DDL_SPRITE_PRECISION_64(sprite) :
- DDL_SPRITE_PRECISION_32(sprite);
+ plane_prec = (prec_mult == high_precision) ?
+ DDL_SPRITE_PRECISION_HIGH(sprite) :
+ DDL_SPRITE_PRECISION_LOW(sprite);
sprite_dl |= plane_prec |
(drain_latency << DDL_SPRITE_SHIFT(sprite));
}
--
1.9.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915/chv: Use 16 and 32 for low and high drain latency precision.
2014-10-17 15:05 [PATCH] drm/i915/chv: Use 16 and 32 for low and high drain latency precision Rodrigo Vivi
@ 2014-10-21 22:42 ` Boyer, Wayne
2014-10-22 6:57 ` Ville Syrjälä
1 sibling, 0 replies; 7+ messages in thread
From: Boyer, Wayne @ 2014-10-21 22:42 UTC (permalink / raw)
To: Vivi, Rodrigo, intel-gfx@lists.freedesktop.org
Tested-by: Wayne Boyer <wayne.boyer@intel.com>
Before this patch I was getting pipe underrun errors on pipe A and pipe C
when
running various workloads. Shortly after the errors, the screens would go
black
and could not be recovered without rebooting.
With this patch I don't get the underrun errors and the machine has been
stable.
On 10/17/14, 8:05 AM, "Vivi, Rodrigo" <rodrigo.vivi@intel.com> wrote:
>Current chv spec teels we can only use either 16 or 32 bits as precision.
>
>Although in the past VLV went from 16/32 to 32/64 and spec might not be
>updated,
>these precision values brings stability and fixes some issues Wayne was
>facing.
>
>Cc: Wayne Boyer <wayne.boyer@intel.com>
>Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>---
> drivers/gpu/drm/i915/i915_reg.h | 13 +++++++------
> drivers/gpu/drm/i915/intel_pm.c | 40
>+++++++++++++++++++++++++---------------
> 2 files changed, 32 insertions(+), 21 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/i915_reg.h
>b/drivers/gpu/drm/i915/i915_reg.h
>index 6db369a..dcd5650 100644
>--- a/drivers/gpu/drm/i915/i915_reg.h
>+++ b/drivers/gpu/drm/i915/i915_reg.h
>@@ -4054,17 +4054,18 @@ enum punit_power_well {
> #define DSPFW_PLANEA_WM1_HI_MASK (1<<0)
>
> /* drain latency register values*/
>+#define DRAIN_LATENCY_PRECISION_16 16
> #define DRAIN_LATENCY_PRECISION_32 32
> #define DRAIN_LATENCY_PRECISION_64 64
> #define VLV_DDL(pipe) (VLV_DISPLAY_BASE + 0x70050 + 4 * (pipe))
>-#define DDL_CURSOR_PRECISION_64 (1<<31)
>-#define DDL_CURSOR_PRECISION_32 (0<<31)
>+#define DDL_CURSOR_PRECISION_HIGH (1<<31)
>+#define DDL_CURSOR_PRECISION_LOW (0<<31)
> #define DDL_CURSOR_SHIFT 24
>-#define DDL_SPRITE_PRECISION_64(sprite) (1<<(15+8*(sprite)))
>-#define DDL_SPRITE_PRECISION_32(sprite) (0<<(15+8*(sprite)))
>+#define DDL_SPRITE_PRECISION_HIGH(sprite) (1<<(15+8*(sprite)))
>+#define DDL_SPRITE_PRECISION_LOW(sprite) (0<<(15+8*(sprite)))
> #define DDL_SPRITE_SHIFT(sprite) (8+8*(sprite))
>-#define DDL_PLANE_PRECISION_64 (1<<7)
>-#define DDL_PLANE_PRECISION_32 (0<<7)
>+#define DDL_PLANE_PRECISION_HIGH (1<<7)
>+#define DDL_PLANE_PRECISION_LOW (0<<7)
> #define DDL_PLANE_SHIFT 0
> #define DRAIN_LATENCY_MASK 0x7f
>
>diff --git a/drivers/gpu/drm/i915/intel_pm.c
>b/drivers/gpu/drm/i915/intel_pm.c
>index daa99e7..50c3512 100644
>--- a/drivers/gpu/drm/i915/intel_pm.c
>+++ b/drivers/gpu/drm/i915/intel_pm.c
>@@ -1345,6 +1345,7 @@ static bool vlv_compute_drain_latency(struct
>drm_crtc *crtc,
> int *prec_mult,
> int *drain_latency)
> {
>+ struct drm_device *dev = crtc->dev;
> int entries;
> int clock = to_intel_crtc(crtc)->config.adjusted_mode.crtc_clock;
>
>@@ -1355,8 +1356,12 @@ static bool vlv_compute_drain_latency(struct
>drm_crtc *crtc,
> return false;
>
> entries = DIV_ROUND_UP(clock, 1000) * pixel_size;
>- *prec_mult = (entries > 128) ? DRAIN_LATENCY_PRECISION_64 :
>- DRAIN_LATENCY_PRECISION_32;
>+ if (IS_CHERRYVIEW(dev))
>+ *prec_mult = (entries > 128) ? DRAIN_LATENCY_PRECISION_32 :
>+ DRAIN_LATENCY_PRECISION_16;
>+ else
>+ *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)
>@@ -1375,15 +1380,18 @@ static bool vlv_compute_drain_latency(struct
>drm_crtc *crtc,
>
> static void vlv_update_drain_latency(struct drm_crtc *crtc)
> {
>- struct drm_i915_private *dev_priv = crtc->dev->dev_private;
>+ 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 pixel_size;
> int drain_latency;
> enum pipe pipe = intel_crtc->pipe;
> int plane_prec, prec_mult, plane_dl;
>+ int high_precision = IS_CHERRYVIEW(dev) ? DRAIN_LATENCY_PRECISION_32 :
>+ DRAIN_LATENCY_PRECISION_64;
>
>- plane_dl = I915_READ(VLV_DDL(pipe)) & ~(DDL_PLANE_PRECISION_64 |
>- DRAIN_LATENCY_MASK | DDL_CURSOR_PRECISION_64 |
>+ plane_dl = I915_READ(VLV_DDL(pipe)) & ~(DDL_PLANE_PRECISION_HIGH |
>+ DRAIN_LATENCY_MASK | DDL_CURSOR_PRECISION_HIGH |
> (DRAIN_LATENCY_MASK << DDL_CURSOR_SHIFT));
>
> if (!intel_crtc_active(crtc)) {
>@@ -1394,9 +1402,9 @@ static void vlv_update_drain_latency(struct
>drm_crtc *crtc)
> /* 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_prec = (prec_mult == high_precision) ?
>+ DDL_PLANE_PRECISION_HIGH :
>+ DDL_PLANE_PRECISION_LOW;
> plane_dl |= plane_prec | drain_latency;
> }
>
>@@ -1408,9 +1416,9 @@ static void vlv_update_drain_latency(struct
>drm_crtc *crtc)
> /* 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_prec = (prec_mult == high_precision) ?
>+ DDL_CURSOR_PRECISION_HIGH :
>+ DDL_CURSOR_PRECISION_LOW;
> plane_dl |= plane_prec | (drain_latency << DDL_CURSOR_SHIFT);
> }
>
>@@ -1578,15 +1586,17 @@ static void valleyview_update_sprite_wm(struct
>drm_plane *plane,
> int plane_prec;
> int sprite_dl;
> int prec_mult;
>+ int high_precision = IS_CHERRYVIEW(dev) ? DRAIN_LATENCY_PRECISION_32 :
>+ DRAIN_LATENCY_PRECISION_64;
>
>- sprite_dl = I915_READ(VLV_DDL(pipe)) &
>~(DDL_SPRITE_PRECISION_64(sprite) |
>+ sprite_dl = I915_READ(VLV_DDL(pipe)) &
>~(DDL_SPRITE_PRECISION_HIGH(sprite) |
> (DRAIN_LATENCY_MASK << DDL_SPRITE_SHIFT(sprite)));
>
> if (enabled && vlv_compute_drain_latency(crtc, pixel_size, &prec_mult,
> &drain_latency)) {
>- plane_prec = (prec_mult == DRAIN_LATENCY_PRECISION_64) ?
>- DDL_SPRITE_PRECISION_64(sprite) :
>- DDL_SPRITE_PRECISION_32(sprite);
>+ plane_prec = (prec_mult == high_precision) ?
>+ DDL_SPRITE_PRECISION_HIGH(sprite) :
>+ DDL_SPRITE_PRECISION_LOW(sprite);
> sprite_dl |= plane_prec |
> (drain_latency << DDL_SPRITE_SHIFT(sprite));
> }
>--
>1.9.3
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915/chv: Use 16 and 32 for low and high drain latency precision.
2014-10-17 15:05 [PATCH] drm/i915/chv: Use 16 and 32 for low and high drain latency precision Rodrigo Vivi
2014-10-21 22:42 ` Boyer, Wayne
@ 2014-10-22 6:57 ` Ville Syrjälä
2014-10-22 11:22 ` Daniel Vetter
2014-10-27 12:17 ` Ville Syrjälä
1 sibling, 2 replies; 7+ messages in thread
From: Ville Syrjälä @ 2014-10-22 6:57 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx
On Fri, Oct 17, 2014 at 08:05:08AM -0700, Rodrigo Vivi wrote:
> Current chv spec teels we can only use either 16 or 32 bits as precision.
>
> Although in the past VLV went from 16/32 to 32/64 and spec might not be updated,
> these precision values brings stability and fixes some issues Wayne was facing.
Lies, damned lies, specs!
Well in this case I guess the spec might be correct since it helps
Wayne.
>
> Cc: Wayne Boyer <wayne.boyer@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 13 +++++++------
> drivers/gpu/drm/i915/intel_pm.c | 40 +++++++++++++++++++++++++---------------
> 2 files changed, 32 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 6db369a..dcd5650 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4054,17 +4054,18 @@ enum punit_power_well {
> #define DSPFW_PLANEA_WM1_HI_MASK (1<<0)
>
> /* drain latency register values*/
> +#define DRAIN_LATENCY_PRECISION_16 16
> #define DRAIN_LATENCY_PRECISION_32 32
> #define DRAIN_LATENCY_PRECISION_64 64
While you're poking at this stuff, could I trouble you to remove these
defines and just use the values directly? Could be a separate patch if
you prefer.
> #define VLV_DDL(pipe) (VLV_DISPLAY_BASE + 0x70050 + 4 * (pipe))
> -#define DDL_CURSOR_PRECISION_64 (1<<31)
> -#define DDL_CURSOR_PRECISION_32 (0<<31)
> +#define DDL_CURSOR_PRECISION_HIGH (1<<31)
> +#define DDL_CURSOR_PRECISION_LOW (0<<31)
> #define DDL_CURSOR_SHIFT 24
> -#define DDL_SPRITE_PRECISION_64(sprite) (1<<(15+8*(sprite)))
> -#define DDL_SPRITE_PRECISION_32(sprite) (0<<(15+8*(sprite)))
> +#define DDL_SPRITE_PRECISION_HIGH(sprite) (1<<(15+8*(sprite)))
> +#define DDL_SPRITE_PRECISION_LOW(sprite) (0<<(15+8*(sprite)))
> #define DDL_SPRITE_SHIFT(sprite) (8+8*(sprite))
> -#define DDL_PLANE_PRECISION_64 (1<<7)
> -#define DDL_PLANE_PRECISION_32 (0<<7)
> +#define DDL_PLANE_PRECISION_HIGH (1<<7)
> +#define DDL_PLANE_PRECISION_LOW (0<<7)
> #define DDL_PLANE_SHIFT 0
> #define DRAIN_LATENCY_MASK 0x7f
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index daa99e7..50c3512 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1345,6 +1345,7 @@ static bool vlv_compute_drain_latency(struct drm_crtc *crtc,
> int *prec_mult,
> int *drain_latency)
> {
> + struct drm_device *dev = crtc->dev;
> int entries;
> int clock = to_intel_crtc(crtc)->config.adjusted_mode.crtc_clock;
>
> @@ -1355,8 +1356,12 @@ static bool vlv_compute_drain_latency(struct drm_crtc *crtc,
> return false;
>
> entries = DIV_ROUND_UP(clock, 1000) * pixel_size;
> - *prec_mult = (entries > 128) ? DRAIN_LATENCY_PRECISION_64 :
> - DRAIN_LATENCY_PRECISION_32;
> + if (IS_CHERRYVIEW(dev))
> + *prec_mult = (entries > 128) ? DRAIN_LATENCY_PRECISION_32 :
> + DRAIN_LATENCY_PRECISION_16;
> + else
> + *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)
> @@ -1375,15 +1380,18 @@ static bool vlv_compute_drain_latency(struct drm_crtc *crtc,
>
> static void vlv_update_drain_latency(struct drm_crtc *crtc)
> {
> - struct drm_i915_private *dev_priv = crtc->dev->dev_private;
> + 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 pixel_size;
> int drain_latency;
> enum pipe pipe = intel_crtc->pipe;
> int plane_prec, prec_mult, plane_dl;
> + int high_precision = IS_CHERRYVIEW(dev) ? DRAIN_LATENCY_PRECISION_32 :
> + DRAIN_LATENCY_PRECISION_64;
Maybe const just help the reader remember that it's a constant.
>
> - plane_dl = I915_READ(VLV_DDL(pipe)) & ~(DDL_PLANE_PRECISION_64 |
> - DRAIN_LATENCY_MASK | DDL_CURSOR_PRECISION_64 |
> + plane_dl = I915_READ(VLV_DDL(pipe)) & ~(DDL_PLANE_PRECISION_HIGH |
> + DRAIN_LATENCY_MASK | DDL_CURSOR_PRECISION_HIGH |
> (DRAIN_LATENCY_MASK << DDL_CURSOR_SHIFT));
>
> if (!intel_crtc_active(crtc)) {
> @@ -1394,9 +1402,9 @@ static void vlv_update_drain_latency(struct drm_crtc *crtc)
> /* 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_prec = (prec_mult == high_precision) ?
> + DDL_PLANE_PRECISION_HIGH :
> + DDL_PLANE_PRECISION_LOW;
> plane_dl |= plane_prec | drain_latency;
> }
>
> @@ -1408,9 +1416,9 @@ static void vlv_update_drain_latency(struct drm_crtc *crtc)
> /* 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_prec = (prec_mult == high_precision) ?
> + DDL_CURSOR_PRECISION_HIGH :
> + DDL_CURSOR_PRECISION_LOW;
> plane_dl |= plane_prec | (drain_latency << DDL_CURSOR_SHIFT);
> }
>
> @@ -1578,15 +1586,17 @@ static void valleyview_update_sprite_wm(struct drm_plane *plane,
> int plane_prec;
> int sprite_dl;
> int prec_mult;
> + int high_precision = IS_CHERRYVIEW(dev) ? DRAIN_LATENCY_PRECISION_32 :
> + DRAIN_LATENCY_PRECISION_64;
Could also be const.
Otherwise looks good, so:
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> - sprite_dl = I915_READ(VLV_DDL(pipe)) & ~(DDL_SPRITE_PRECISION_64(sprite) |
> + sprite_dl = I915_READ(VLV_DDL(pipe)) & ~(DDL_SPRITE_PRECISION_HIGH(sprite) |
> (DRAIN_LATENCY_MASK << DDL_SPRITE_SHIFT(sprite)));
>
> if (enabled && vlv_compute_drain_latency(crtc, pixel_size, &prec_mult,
> &drain_latency)) {
> - plane_prec = (prec_mult == DRAIN_LATENCY_PRECISION_64) ?
> - DDL_SPRITE_PRECISION_64(sprite) :
> - DDL_SPRITE_PRECISION_32(sprite);
> + plane_prec = (prec_mult == high_precision) ?
> + DDL_SPRITE_PRECISION_HIGH(sprite) :
> + DDL_SPRITE_PRECISION_LOW(sprite);
> sprite_dl |= plane_prec |
> (drain_latency << DDL_SPRITE_SHIFT(sprite));
> }
> --
> 1.9.3
--
Ville Syrjälä
Intel OTC
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915/chv: Use 16 and 32 for low and high drain latency precision.
2014-10-22 6:57 ` Ville Syrjälä
@ 2014-10-22 11:22 ` Daniel Vetter
2014-10-27 12:17 ` Ville Syrjälä
1 sibling, 0 replies; 7+ messages in thread
From: Daniel Vetter @ 2014-10-22 11:22 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx, Rodrigo Vivi
On Wed, Oct 22, 2014 at 09:57:06AM +0300, Ville Syrjälä wrote:
> On Fri, Oct 17, 2014 at 08:05:08AM -0700, Rodrigo Vivi wrote:
> > Current chv spec teels we can only use either 16 or 32 bits as precision.
> >
> > Although in the past VLV went from 16/32 to 32/64 and spec might not be updated,
> > these precision values brings stability and fixes some issues Wayne was facing.
>
> Lies, damned lies, specs!
>
> Well in this case I guess the spec might be correct since it helps
> Wayne.
>
> >
> > Cc: Wayne Boyer <wayne.boyer@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_reg.h | 13 +++++++------
> > drivers/gpu/drm/i915/intel_pm.c | 40 +++++++++++++++++++++++++---------------
> > 2 files changed, 32 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 6db369a..dcd5650 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -4054,17 +4054,18 @@ enum punit_power_well {
> > #define DSPFW_PLANEA_WM1_HI_MASK (1<<0)
> >
> > /* drain latency register values*/
> > +#define DRAIN_LATENCY_PRECISION_16 16
> > #define DRAIN_LATENCY_PRECISION_32 32
> > #define DRAIN_LATENCY_PRECISION_64 64
>
> While you're poking at this stuff, could I trouble you to remove these
> defines and just use the values directly? Could be a separate patch if
> you prefer.
Separate patch imo.
>
> > #define VLV_DDL(pipe) (VLV_DISPLAY_BASE + 0x70050 + 4 * (pipe))
> > -#define DDL_CURSOR_PRECISION_64 (1<<31)
> > -#define DDL_CURSOR_PRECISION_32 (0<<31)
> > +#define DDL_CURSOR_PRECISION_HIGH (1<<31)
> > +#define DDL_CURSOR_PRECISION_LOW (0<<31)
> > #define DDL_CURSOR_SHIFT 24
> > -#define DDL_SPRITE_PRECISION_64(sprite) (1<<(15+8*(sprite)))
> > -#define DDL_SPRITE_PRECISION_32(sprite) (0<<(15+8*(sprite)))
> > +#define DDL_SPRITE_PRECISION_HIGH(sprite) (1<<(15+8*(sprite)))
> > +#define DDL_SPRITE_PRECISION_LOW(sprite) (0<<(15+8*(sprite)))
> > #define DDL_SPRITE_SHIFT(sprite) (8+8*(sprite))
> > -#define DDL_PLANE_PRECISION_64 (1<<7)
> > -#define DDL_PLANE_PRECISION_32 (0<<7)
> > +#define DDL_PLANE_PRECISION_HIGH (1<<7)
> > +#define DDL_PLANE_PRECISION_LOW (0<<7)
> > #define DDL_PLANE_SHIFT 0
> > #define DRAIN_LATENCY_MASK 0x7f
> >
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index daa99e7..50c3512 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -1345,6 +1345,7 @@ static bool vlv_compute_drain_latency(struct drm_crtc *crtc,
> > int *prec_mult,
> > int *drain_latency)
> > {
> > + struct drm_device *dev = crtc->dev;
> > int entries;
> > int clock = to_intel_crtc(crtc)->config.adjusted_mode.crtc_clock;
> >
> > @@ -1355,8 +1356,12 @@ static bool vlv_compute_drain_latency(struct drm_crtc *crtc,
> > return false;
> >
> > entries = DIV_ROUND_UP(clock, 1000) * pixel_size;
> > - *prec_mult = (entries > 128) ? DRAIN_LATENCY_PRECISION_64 :
> > - DRAIN_LATENCY_PRECISION_32;
> > + if (IS_CHERRYVIEW(dev))
> > + *prec_mult = (entries > 128) ? DRAIN_LATENCY_PRECISION_32 :
> > + DRAIN_LATENCY_PRECISION_16;
> > + else
> > + *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)
> > @@ -1375,15 +1380,18 @@ static bool vlv_compute_drain_latency(struct drm_crtc *crtc,
> >
> > static void vlv_update_drain_latency(struct drm_crtc *crtc)
> > {
> > - struct drm_i915_private *dev_priv = crtc->dev->dev_private;
> > + 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 pixel_size;
> > int drain_latency;
> > enum pipe pipe = intel_crtc->pipe;
> > int plane_prec, prec_mult, plane_dl;
> > + int high_precision = IS_CHERRYVIEW(dev) ? DRAIN_LATENCY_PRECISION_32 :
> > + DRAIN_LATENCY_PRECISION_64;
>
> Maybe const just help the reader remember that it's a constant.
>
> >
> > - plane_dl = I915_READ(VLV_DDL(pipe)) & ~(DDL_PLANE_PRECISION_64 |
> > - DRAIN_LATENCY_MASK | DDL_CURSOR_PRECISION_64 |
> > + plane_dl = I915_READ(VLV_DDL(pipe)) & ~(DDL_PLANE_PRECISION_HIGH |
> > + DRAIN_LATENCY_MASK | DDL_CURSOR_PRECISION_HIGH |
> > (DRAIN_LATENCY_MASK << DDL_CURSOR_SHIFT));
> >
> > if (!intel_crtc_active(crtc)) {
> > @@ -1394,9 +1402,9 @@ static void vlv_update_drain_latency(struct drm_crtc *crtc)
> > /* 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_prec = (prec_mult == high_precision) ?
> > + DDL_PLANE_PRECISION_HIGH :
> > + DDL_PLANE_PRECISION_LOW;
> > plane_dl |= plane_prec | drain_latency;
> > }
> >
> > @@ -1408,9 +1416,9 @@ static void vlv_update_drain_latency(struct drm_crtc *crtc)
> > /* 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_prec = (prec_mult == high_precision) ?
> > + DDL_CURSOR_PRECISION_HIGH :
> > + DDL_CURSOR_PRECISION_LOW;
> > plane_dl |= plane_prec | (drain_latency << DDL_CURSOR_SHIFT);
> > }
> >
> > @@ -1578,15 +1586,17 @@ static void valleyview_update_sprite_wm(struct drm_plane *plane,
> > int plane_prec;
> > int sprite_dl;
> > int prec_mult;
> > + int high_precision = IS_CHERRYVIEW(dev) ? DRAIN_LATENCY_PRECISION_32 :
> > + DRAIN_LATENCY_PRECISION_64;
>
> Could also be const.
Both const modifiers added while applying.
>
> Otherwise looks good, so:
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Queued for -next, thanks for the patch.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915/chv: Use 16 and 32 for low and high drain latency precision.
2014-10-22 6:57 ` Ville Syrjälä
2014-10-22 11:22 ` Daniel Vetter
@ 2014-10-27 12:17 ` Ville Syrjälä
2014-10-27 14:20 ` Daniel Vetter
1 sibling, 1 reply; 7+ messages in thread
From: Ville Syrjälä @ 2014-10-27 12:17 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx
On Wed, Oct 22, 2014 at 09:57:06AM +0300, Ville Syrjälä wrote:
> On Fri, Oct 17, 2014 at 08:05:08AM -0700, Rodrigo Vivi wrote:
> > Current chv spec teels we can only use either 16 or 32 bits as precision.
> >
> > Although in the past VLV went from 16/32 to 32/64 and spec might not be updated,
> > these precision values brings stability and fixes some issues Wayne was facing.
>
> Lies, damned lies, specs!
>
> Well in this case I guess the spec might be correct since it helps
> Wayne.
>
> >
> > Cc: Wayne Boyer <wayne.boyer@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_reg.h | 13 +++++++------
> > drivers/gpu/drm/i915/intel_pm.c | 40 +++++++++++++++++++++++++---------------
> > 2 files changed, 32 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 6db369a..dcd5650 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -4054,17 +4054,18 @@ enum punit_power_well {
> > #define DSPFW_PLANEA_WM1_HI_MASK (1<<0)
> >
> > /* drain latency register values*/
> > +#define DRAIN_LATENCY_PRECISION_16 16
> > #define DRAIN_LATENCY_PRECISION_32 32
> > #define DRAIN_LATENCY_PRECISION_64 64
>
> While you're poking at this stuff, could I trouble you to remove these
> defines and just use the values directly? Could be a separate patch if
> you prefer.
>
> > #define VLV_DDL(pipe) (VLV_DISPLAY_BASE + 0x70050 + 4 * (pipe))
> > -#define DDL_CURSOR_PRECISION_64 (1<<31)
> > -#define DDL_CURSOR_PRECISION_32 (0<<31)
> > +#define DDL_CURSOR_PRECISION_HIGH (1<<31)
> > +#define DDL_CURSOR_PRECISION_LOW (0<<31)
> > #define DDL_CURSOR_SHIFT 24
> > -#define DDL_SPRITE_PRECISION_64(sprite) (1<<(15+8*(sprite)))
> > -#define DDL_SPRITE_PRECISION_32(sprite) (0<<(15+8*(sprite)))
> > +#define DDL_SPRITE_PRECISION_HIGH(sprite) (1<<(15+8*(sprite)))
> > +#define DDL_SPRITE_PRECISION_LOW(sprite) (0<<(15+8*(sprite)))
> > #define DDL_SPRITE_SHIFT(sprite) (8+8*(sprite))
> > -#define DDL_PLANE_PRECISION_64 (1<<7)
> > -#define DDL_PLANE_PRECISION_32 (0<<7)
> > +#define DDL_PLANE_PRECISION_HIGH (1<<7)
> > +#define DDL_PLANE_PRECISION_LOW (0<<7)
> > #define DDL_PLANE_SHIFT 0
> > #define DRAIN_LATENCY_MASK 0x7f
> >
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index daa99e7..50c3512 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -1345,6 +1345,7 @@ static bool vlv_compute_drain_latency(struct drm_crtc *crtc,
> > int *prec_mult,
> > int *drain_latency)
> > {
> > + struct drm_device *dev = crtc->dev;
> > int entries;
> > int clock = to_intel_crtc(crtc)->config.adjusted_mode.crtc_clock;
> >
> > @@ -1355,8 +1356,12 @@ static bool vlv_compute_drain_latency(struct drm_crtc *crtc,
> > return false;
> >
> > entries = DIV_ROUND_UP(clock, 1000) * pixel_size;
> > - *prec_mult = (entries > 128) ? DRAIN_LATENCY_PRECISION_64 :
> > - DRAIN_LATENCY_PRECISION_32;
> > + if (IS_CHERRYVIEW(dev))
> > + *prec_mult = (entries > 128) ? DRAIN_LATENCY_PRECISION_32 :
> > + DRAIN_LATENCY_PRECISION_16;
> > + else
> > + *prec_mult = (entries > 128) ? DRAIN_LATENCY_PRECISION_64 :
> > + DRAIN_LATENCY_PRECISION_32;
Actually let me take that r-b back a bit. That 128 needs to be changed for CHV
as well.
I think we want to rearrange this code a bit to kill that magic constant
and do things in the obvious way. So something like this would be fairly
straightforward I think:
prec_mult = IS_CHV ? 32 : 64;
drain_latency = 64 * prec_mult * 4 / entries;
if (drain_latency > DRAIN_LATENCY_MASK) {
prec_mult /= 2;
drain_latency = 64 * prec_mult * 4 / entries;
}
if (drain_latency > DRAIN_LATENCY_MASK)
drain_latency = DRAIN_LATENCY_MASK;
> > *drain_latency = (64 * (*prec_mult) * 4) / entries;
> >
> > if (*drain_latency > DRAIN_LATENCY_MASK)
> > @@ -1375,15 +1380,18 @@ static bool vlv_compute_drain_latency(struct drm_crtc *crtc,
> >
> > static void vlv_update_drain_latency(struct drm_crtc *crtc)
> > {
> > - struct drm_i915_private *dev_priv = crtc->dev->dev_private;
> > + 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 pixel_size;
> > int drain_latency;
> > enum pipe pipe = intel_crtc->pipe;
> > int plane_prec, prec_mult, plane_dl;
> > + int high_precision = IS_CHERRYVIEW(dev) ? DRAIN_LATENCY_PRECISION_32 :
> > + DRAIN_LATENCY_PRECISION_64;
>
> Maybe const just help the reader remember that it's a constant.
>
> >
> > - plane_dl = I915_READ(VLV_DDL(pipe)) & ~(DDL_PLANE_PRECISION_64 |
> > - DRAIN_LATENCY_MASK | DDL_CURSOR_PRECISION_64 |
> > + plane_dl = I915_READ(VLV_DDL(pipe)) & ~(DDL_PLANE_PRECISION_HIGH |
> > + DRAIN_LATENCY_MASK | DDL_CURSOR_PRECISION_HIGH |
> > (DRAIN_LATENCY_MASK << DDL_CURSOR_SHIFT));
> >
> > if (!intel_crtc_active(crtc)) {
> > @@ -1394,9 +1402,9 @@ static void vlv_update_drain_latency(struct drm_crtc *crtc)
> > /* 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_prec = (prec_mult == high_precision) ?
> > + DDL_PLANE_PRECISION_HIGH :
> > + DDL_PLANE_PRECISION_LOW;
> > plane_dl |= plane_prec | drain_latency;
> > }
> >
> > @@ -1408,9 +1416,9 @@ static void vlv_update_drain_latency(struct drm_crtc *crtc)
> > /* 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_prec = (prec_mult == high_precision) ?
> > + DDL_CURSOR_PRECISION_HIGH :
> > + DDL_CURSOR_PRECISION_LOW;
> > plane_dl |= plane_prec | (drain_latency << DDL_CURSOR_SHIFT);
> > }
> >
> > @@ -1578,15 +1586,17 @@ static void valleyview_update_sprite_wm(struct drm_plane *plane,
> > int plane_prec;
> > int sprite_dl;
> > int prec_mult;
> > + int high_precision = IS_CHERRYVIEW(dev) ? DRAIN_LATENCY_PRECISION_32 :
> > + DRAIN_LATENCY_PRECISION_64;
>
> Could also be const.
>
> Otherwise looks good, so:
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> >
> > - sprite_dl = I915_READ(VLV_DDL(pipe)) & ~(DDL_SPRITE_PRECISION_64(sprite) |
> > + sprite_dl = I915_READ(VLV_DDL(pipe)) & ~(DDL_SPRITE_PRECISION_HIGH(sprite) |
> > (DRAIN_LATENCY_MASK << DDL_SPRITE_SHIFT(sprite)));
> >
> > if (enabled && vlv_compute_drain_latency(crtc, pixel_size, &prec_mult,
> > &drain_latency)) {
> > - plane_prec = (prec_mult == DRAIN_LATENCY_PRECISION_64) ?
> > - DDL_SPRITE_PRECISION_64(sprite) :
> > - DDL_SPRITE_PRECISION_32(sprite);
> > + plane_prec = (prec_mult == high_precision) ?
> > + DDL_SPRITE_PRECISION_HIGH(sprite) :
> > + DDL_SPRITE_PRECISION_LOW(sprite);
> > sprite_dl |= plane_prec |
> > (drain_latency << DDL_SPRITE_SHIFT(sprite));
> > }
> > --
> > 1.9.3
>
> --
> Ville Syrjälä
> Intel OTC
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915/chv: Use 16 and 32 for low and high drain latency precision.
2014-10-27 12:17 ` Ville Syrjälä
@ 2014-10-27 14:20 ` Daniel Vetter
2014-10-27 14:35 ` Ville Syrjälä
0 siblings, 1 reply; 7+ messages in thread
From: Daniel Vetter @ 2014-10-27 14:20 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx, Rodrigo Vivi
On Mon, Oct 27, 2014 at 02:17:34PM +0200, Ville Syrjälä wrote:
> On Wed, Oct 22, 2014 at 09:57:06AM +0300, Ville Syrjälä wrote:
> > On Fri, Oct 17, 2014 at 08:05:08AM -0700, Rodrigo Vivi wrote:
> > > Current chv spec teels we can only use either 16 or 32 bits as precision.
> > >
> > > Although in the past VLV went from 16/32 to 32/64 and spec might not be updated,
> > > these precision values brings stability and fixes some issues Wayne was facing.
> >
> > Lies, damned lies, specs!
> >
> > Well in this case I guess the spec might be correct since it helps
> > Wayne.
> >
> > >
> > > Cc: Wayne Boyer <wayne.boyer@intel.com>
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > ---
> > > drivers/gpu/drm/i915/i915_reg.h | 13 +++++++------
> > > drivers/gpu/drm/i915/intel_pm.c | 40 +++++++++++++++++++++++++---------------
> > > 2 files changed, 32 insertions(+), 21 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > > index 6db369a..dcd5650 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -4054,17 +4054,18 @@ enum punit_power_well {
> > > #define DSPFW_PLANEA_WM1_HI_MASK (1<<0)
> > >
> > > /* drain latency register values*/
> > > +#define DRAIN_LATENCY_PRECISION_16 16
> > > #define DRAIN_LATENCY_PRECISION_32 32
> > > #define DRAIN_LATENCY_PRECISION_64 64
> >
> > While you're poking at this stuff, could I trouble you to remove these
> > defines and just use the values directly? Could be a separate patch if
> > you prefer.
> >
> > > #define VLV_DDL(pipe) (VLV_DISPLAY_BASE + 0x70050 + 4 * (pipe))
> > > -#define DDL_CURSOR_PRECISION_64 (1<<31)
> > > -#define DDL_CURSOR_PRECISION_32 (0<<31)
> > > +#define DDL_CURSOR_PRECISION_HIGH (1<<31)
> > > +#define DDL_CURSOR_PRECISION_LOW (0<<31)
> > > #define DDL_CURSOR_SHIFT 24
> > > -#define DDL_SPRITE_PRECISION_64(sprite) (1<<(15+8*(sprite)))
> > > -#define DDL_SPRITE_PRECISION_32(sprite) (0<<(15+8*(sprite)))
> > > +#define DDL_SPRITE_PRECISION_HIGH(sprite) (1<<(15+8*(sprite)))
> > > +#define DDL_SPRITE_PRECISION_LOW(sprite) (0<<(15+8*(sprite)))
> > > #define DDL_SPRITE_SHIFT(sprite) (8+8*(sprite))
> > > -#define DDL_PLANE_PRECISION_64 (1<<7)
> > > -#define DDL_PLANE_PRECISION_32 (0<<7)
> > > +#define DDL_PLANE_PRECISION_HIGH (1<<7)
> > > +#define DDL_PLANE_PRECISION_LOW (0<<7)
> > > #define DDL_PLANE_SHIFT 0
> > > #define DRAIN_LATENCY_MASK 0x7f
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > > index daa99e7..50c3512 100644
> > > --- a/drivers/gpu/drm/i915/intel_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > > @@ -1345,6 +1345,7 @@ static bool vlv_compute_drain_latency(struct drm_crtc *crtc,
> > > int *prec_mult,
> > > int *drain_latency)
> > > {
> > > + struct drm_device *dev = crtc->dev;
> > > int entries;
> > > int clock = to_intel_crtc(crtc)->config.adjusted_mode.crtc_clock;
> > >
> > > @@ -1355,8 +1356,12 @@ static bool vlv_compute_drain_latency(struct drm_crtc *crtc,
> > > return false;
> > >
> > > entries = DIV_ROUND_UP(clock, 1000) * pixel_size;
> > > - *prec_mult = (entries > 128) ? DRAIN_LATENCY_PRECISION_64 :
> > > - DRAIN_LATENCY_PRECISION_32;
> > > + if (IS_CHERRYVIEW(dev))
> > > + *prec_mult = (entries > 128) ? DRAIN_LATENCY_PRECISION_32 :
> > > + DRAIN_LATENCY_PRECISION_16;
> > > + else
> > > + *prec_mult = (entries > 128) ? DRAIN_LATENCY_PRECISION_64 :
> > > + DRAIN_LATENCY_PRECISION_32;
>
> Actually let me take that r-b back a bit. That 128 needs to be changed for CHV
> as well.
Well it's already backed into history, so no taking back. Bad enough to
warrant a revert or fixup patch ok?
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915/chv: Use 16 and 32 for low and high drain latency precision.
2014-10-27 14:20 ` Daniel Vetter
@ 2014-10-27 14:35 ` Ville Syrjälä
0 siblings, 0 replies; 7+ messages in thread
From: Ville Syrjälä @ 2014-10-27 14:35 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx, Rodrigo Vivi
On Mon, Oct 27, 2014 at 03:20:28PM +0100, Daniel Vetter wrote:
> On Mon, Oct 27, 2014 at 02:17:34PM +0200, Ville Syrjälä wrote:
> > On Wed, Oct 22, 2014 at 09:57:06AM +0300, Ville Syrjälä wrote:
> > > On Fri, Oct 17, 2014 at 08:05:08AM -0700, Rodrigo Vivi wrote:
> > > > Current chv spec teels we can only use either 16 or 32 bits as precision.
> > > >
> > > > Although in the past VLV went from 16/32 to 32/64 and spec might not be updated,
> > > > these precision values brings stability and fixes some issues Wayne was facing.
> > >
> > > Lies, damned lies, specs!
> > >
> > > Well in this case I guess the spec might be correct since it helps
> > > Wayne.
> > >
> > > >
> > > > Cc: Wayne Boyer <wayne.boyer@intel.com>
> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > ---
> > > > drivers/gpu/drm/i915/i915_reg.h | 13 +++++++------
> > > > drivers/gpu/drm/i915/intel_pm.c | 40 +++++++++++++++++++++++++---------------
> > > > 2 files changed, 32 insertions(+), 21 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > > > index 6db369a..dcd5650 100644
> > > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > > @@ -4054,17 +4054,18 @@ enum punit_power_well {
> > > > #define DSPFW_PLANEA_WM1_HI_MASK (1<<0)
> > > >
> > > > /* drain latency register values*/
> > > > +#define DRAIN_LATENCY_PRECISION_16 16
> > > > #define DRAIN_LATENCY_PRECISION_32 32
> > > > #define DRAIN_LATENCY_PRECISION_64 64
> > >
> > > While you're poking at this stuff, could I trouble you to remove these
> > > defines and just use the values directly? Could be a separate patch if
> > > you prefer.
> > >
> > > > #define VLV_DDL(pipe) (VLV_DISPLAY_BASE + 0x70050 + 4 * (pipe))
> > > > -#define DDL_CURSOR_PRECISION_64 (1<<31)
> > > > -#define DDL_CURSOR_PRECISION_32 (0<<31)
> > > > +#define DDL_CURSOR_PRECISION_HIGH (1<<31)
> > > > +#define DDL_CURSOR_PRECISION_LOW (0<<31)
> > > > #define DDL_CURSOR_SHIFT 24
> > > > -#define DDL_SPRITE_PRECISION_64(sprite) (1<<(15+8*(sprite)))
> > > > -#define DDL_SPRITE_PRECISION_32(sprite) (0<<(15+8*(sprite)))
> > > > +#define DDL_SPRITE_PRECISION_HIGH(sprite) (1<<(15+8*(sprite)))
> > > > +#define DDL_SPRITE_PRECISION_LOW(sprite) (0<<(15+8*(sprite)))
> > > > #define DDL_SPRITE_SHIFT(sprite) (8+8*(sprite))
> > > > -#define DDL_PLANE_PRECISION_64 (1<<7)
> > > > -#define DDL_PLANE_PRECISION_32 (0<<7)
> > > > +#define DDL_PLANE_PRECISION_HIGH (1<<7)
> > > > +#define DDL_PLANE_PRECISION_LOW (0<<7)
> > > > #define DDL_PLANE_SHIFT 0
> > > > #define DRAIN_LATENCY_MASK 0x7f
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > > > index daa99e7..50c3512 100644
> > > > --- a/drivers/gpu/drm/i915/intel_pm.c
> > > > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > > > @@ -1345,6 +1345,7 @@ static bool vlv_compute_drain_latency(struct drm_crtc *crtc,
> > > > int *prec_mult,
> > > > int *drain_latency)
> > > > {
> > > > + struct drm_device *dev = crtc->dev;
> > > > int entries;
> > > > int clock = to_intel_crtc(crtc)->config.adjusted_mode.crtc_clock;
> > > >
> > > > @@ -1355,8 +1356,12 @@ static bool vlv_compute_drain_latency(struct drm_crtc *crtc,
> > > > return false;
> > > >
> > > > entries = DIV_ROUND_UP(clock, 1000) * pixel_size;
> > > > - *prec_mult = (entries > 128) ? DRAIN_LATENCY_PRECISION_64 :
> > > > - DRAIN_LATENCY_PRECISION_32;
> > > > + if (IS_CHERRYVIEW(dev))
> > > > + *prec_mult = (entries > 128) ? DRAIN_LATENCY_PRECISION_32 :
> > > > + DRAIN_LATENCY_PRECISION_16;
> > > > + else
> > > > + *prec_mult = (entries > 128) ? DRAIN_LATENCY_PRECISION_64 :
> > > > + DRAIN_LATENCY_PRECISION_32;
> >
> > Actually let me take that r-b back a bit. That 128 needs to be changed for CHV
> > as well.
>
> Well it's already backed into history, so no taking back. Bad enough to
> warrant a revert or fixup patch ok?
A fixup is perfectly fine here. With the error this way we just end up
not taking full advantage of the available 7 bits. So things should
still work just fine, albeit not exactly optimally.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-10-27 14:35 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-10-17 15:05 [PATCH] drm/i915/chv: Use 16 and 32 for low and high drain latency precision Rodrigo Vivi
2014-10-21 22:42 ` Boyer, Wayne
2014-10-22 6:57 ` Ville Syrjälä
2014-10-22 11:22 ` Daniel Vetter
2014-10-27 12:17 ` Ville Syrjälä
2014-10-27 14:20 ` Daniel Vetter
2014-10-27 14:35 ` Ville Syrjälä
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox