[Intel-gfx] [PATCH 14/15] drm/i915: skl nv12 workarounds

[Ville Syrjälä]

On Wed, Sep 09, 2015 at 02:51:58AM +0300, Konduru, Chandra wrote:
> > > > > +static void skl_wa_clkgate(struct drm_i915_private *dev_priv,
> > > > > +	int pipe, int enable)
> > > > > +{
> > > > > +	if (pipe == PIPE_A || pipe == PIPE_B) {
> > > > > +		if (enable)
> > > > > +			I915_WRITE(CLKGATE_DIS_PSL(pipe),
> > > > > +				DUPS1_GATING_DIS | DUPS2_GATING_DIS);
> > > > > +		else
> > > > > +			I915_WRITE(CLKGATE_DIS_PSL(pipe),
> > > > > +				I915_READ(CLKGATE_DIS_PSL(pipe) &
> > > > > +				~(DUPS1_GATING_DIS|DUPS2_GATING_DIS)));
> > > > > +	}
> > > > > +}
> > > > > +
> > > > >  static void haswell_crtc_enable(struct drm_crtc *crtc)
> > > > >  {
> > > > >  	struct drm_device *dev = crtc->dev;
> > > > > @@ -5094,6 +5119,9 @@ static void haswell_crtc_enable(struct drm_crtc
> > > > *crtc)
> > > > >  		intel_wait_for_vblank(dev, hsw_workaround_pipe);
> > > > >  		intel_wait_for_vblank(dev, hsw_workaround_pipe);
> > > > >  	}
> > > > > +
> > > > > +	/* workaround for NV12 */
> > > > > +	skl_wa_clkgate(dev_priv, pipe, 1);
> > > >
> > > > I wonder what's the cost of having this
> > > > a) always enabled
> > > > b) enabled when the pipe is enabled
> > > > c) enabled only when NV12 is used
> > > > ?
> > >
> > > Initially optimized to enable only when nv12 is used,
> > > but there are some corner cases when planes switch to and
> > > from nv12 to non-nv12 and SV recommendation is to enable
> > > always; and SV evaluated cost, and it isn't a big concern.
> > 
> > So, based on that we could just stuff it into init_clock_gating and
> > forget about it.
> 
> Couldn't include into init_clock_gating because this requires
> a pipe based check.

init_clock_gating()
{
	...
	enable for pipe A
	enable for pipe B
	...
}

or

for_each_pipe(pipe)
	if (pipe != C)
		enable w/a


> 
> By the way, so far 4 patches got RB tags.
> In the respun series (http://lists.freedesktop.org/archives/intel-gfx/2015-September/075235.html
> addressed your feedback), those 4 tags goes to 1, 3, 5 and 8 of 15.
> Can you check updated patches and issue R-B tags for remaining ones?

I think your trigger finger is a bit overly sensitive :) We still had
open issues in this series so posting another one makes things somewhat
messy.

-- 
Ville Syrjälä
Intel OTC
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.