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

[Imre Deak <imre.deak@intel.com>]

[-- 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