Re: [PATCH 04/26] drm/i915: ValleyView watermark support

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Mar 22, 2012 at 08:29:30PM -0700, Ben Widawsky wrote:
> On Thu, Mar 22, 2012 at 02:38:46PM -0700, Jesse Barnes wrote:
> > Add support for ValleyView watermark handling.  It's like Cantiga with a
> > few small differences (big FIFO mode and different WM limits).
> > 
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h      |    9 +++++
> >  drivers/gpu/drm/i915/intel_display.c |   65 ++++++++++++++++++++++++++++++++++
> >  2 files changed, 74 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index f3609f2..0540099 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -1043,6 +1043,12 @@
> >  #define RAMCLK_GATE_D		0x6210		/* CRL only */
> >  #define DEUC			0x6214          /* CRL only */
> >  
> > +#define FW_BLC_SELF_VLV		0x6500
> > +#define  FW_CSPWRDWNEN		(1<<15)
> > +#define MI_ARB_VLV		0x6504
> > +#define  DISP_TRICKLE_FEED_DIS	(1<<2)
> > +#define CZCLK_CDCLK_FREQ_RATIO_VLV	0x6508
> > +
> >  /*
> >   * Palette regs
> >   */
> 
> Seems like the bottom 3 of these aren't used anywhere.

I honestly don't mind a few extra #defines, as long as someone has
bothered to cross check them with bspec (to avoid another MI_WTF). At
least if they're sounding somewhat relevant.
-Daniel

> 
> > @@ -2495,6 +2501,7 @@
> >  #define I915_FIFO_LINE_SIZE	64
> >  #define I830_FIFO_LINE_SIZE	32
> >  
> > +#define VALLEYVIEW_FIFO_SIZE	255
> >  #define G4X_FIFO_SIZE		127
> >  #define I965_FIFO_SIZE		512
> >  #define I945_FIFO_SIZE		127
> > @@ -2502,6 +2509,7 @@
> >  #define I855GM_FIFO_SIZE	127 /* In cachelines */
> >  #define I830_FIFO_SIZE		95
> >  
> > +#define VALLEYVIEW_MAX_WM	0xff
> >  #define G4X_MAX_WM		0x3f
> >  #define I915_MAX_WM		0x3f
> >  
> > @@ -2516,6 +2524,7 @@
> >  #define PINEVIEW_CURSOR_DFT_WM	0
> >  #define PINEVIEW_CURSOR_GUARD_WM	5
> >  
> > +#define VALLEYVIEW_CURSOR_MAX_WM 64
> >  #define I965_CURSOR_FIFO	64
> >  #define I965_CURSOR_MAX_WM	32
> >  #define I965_CURSOR_DFT_WM	8
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index de1ba19..daa8853 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -3606,6 +3606,20 @@ static const struct intel_watermark_params g4x_cursor_wm_info = {
> >  	2,
> >  	G4X_FIFO_LINE_SIZE,
> >  };
> > +static const struct intel_watermark_params valleyview_wm_info = {
> > +	VALLEYVIEW_FIFO_SIZE,
> > +	VALLEYVIEW_MAX_WM,
> > +	VALLEYVIEW_MAX_WM,
> > +	2,
> > +	G4X_FIFO_LINE_SIZE,
> > +};
> > +static const struct intel_watermark_params valleyview_cursor_wm_info = {
> > +	I965_CURSOR_FIFO,
> > +	VALLEYVIEW_CURSOR_MAX_WM,
> > +	I965_CURSOR_DFT_WM,
> > +	2,
> > +	G4X_FIFO_LINE_SIZE,
> > +};
> >  static const struct intel_watermark_params i965_cursor_wm_info = {
> >  	I965_CURSOR_FIFO,
> >  	I965_CURSOR_MAX_WM,
> > @@ -4130,6 +4144,55 @@ static bool g4x_compute_srwm(struct drm_device *dev,
> >  
> >  #define single_plane_enabled(mask) is_power_of_2(mask)
> >  
> > +static void valleyview_update_wm(struct drm_device *dev)
> > +{
> > +	static const int sr_latency_ns = 12000;
> > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > +	int planea_wm, planeb_wm, cursora_wm, cursorb_wm;
> > +	int plane_sr, cursor_sr;
> > +	unsigned int enabled = 0;
> > +
> > +	if (g4x_compute_wm0(dev, 0,
> > +			    &valleyview_wm_info, latency_ns,
> > +			    &valleyview_cursor_wm_info, latency_ns,
> > +			    &planea_wm, &cursora_wm))
> > +		enabled |= 1;
> > +
> > +	if (g4x_compute_wm0(dev, 1,
> > +			    &valleyview_wm_info, latency_ns,
> > +			    &valleyview_cursor_wm_info, latency_ns,
> > +			    &planeb_wm, &cursorb_wm))
> > +		enabled |= 2;
> > +
> > +	plane_sr = cursor_sr = 0;
> > +	if (single_plane_enabled(enabled) &&
> > +	    g4x_compute_srwm(dev, ffs(enabled) - 1,
> > +			     sr_latency_ns,
> > +			     &valleyview_wm_info,
> > +			     &valleyview_cursor_wm_info,
> > +			     &plane_sr, &cursor_sr))
> > +		I915_WRITE(FW_BLC_SELF_VLV, FW_CSPWRDWNEN);
> > +	else
> > +		I915_WRITE(FW_BLC_SELF_VLV,
> > +			   I915_READ(FW_BLC_SELF_VLV) & ~FW_CSPWRDWNEN);
> > +
> > +	DRM_DEBUG_KMS("Setting FIFO watermarks - A: plane=%d, cursor=%d, B: plane=%d, cursor=%d, SR: plane=%d, cursor=%d\n",
> > +		      planea_wm, cursora_wm,
> > +		      planeb_wm, cursorb_wm,
> > +		      plane_sr, cursor_sr);
> > +
> > +	I915_WRITE(DSPFW1,
> > +		   (plane_sr << DSPFW_SR_SHIFT) |
> > +		   (cursorb_wm << DSPFW_CURSORB_SHIFT) |
> > +		   (planeb_wm << DSPFW_PLANEB_SHIFT) |
> > +		   planea_wm);
> > +	I915_WRITE(DSPFW2,
> > +		   (I915_READ(DSPFW2) & DSPFW_CURSORA_MASK) |
> > +		   (cursora_wm << DSPFW_CURSORA_SHIFT));
> > +	I915_WRITE(DSPFW3,
> > +		   (I915_READ(DSPFW3) | (cursor_sr << DSPFW_CURSOR_SR_SHIFT)));
> > +}
> > +
> >  static void g4x_update_wm(struct drm_device *dev)
> >  {
> >  	static const int sr_latency_ns = 12000;
> > @@ -8917,6 +8980,8 @@ static void intel_init_display(struct drm_device *dev)
> >  			dev_priv->display.write_eld = ironlake_write_eld;
> >  		} else
> >  			dev_priv->display.update_wm = NULL;
> > +	} else if (IS_VALLEYVIEW(dev)) {
> > +		dev_priv->display.update_wm = valleyview_update_wm;
> >  	} else if (IS_PINEVIEW(dev)) {
> >  		if (!intel_get_cxsr_latency(IS_PINEVIEW_G(dev),
> >  					    dev_priv->is_ddr3,
> 
> Aside from the extraneous #defines
> Acked-by: Ben Widawsky <ben@bwidawsk.net>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48