Re: [PATCH 14/15] drm/i915: skl nv12 workarounds

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Sat, Sep 05, 2015 at 01:28:56AM +0000, Konduru, Chandra wrote:
> > > +
> > > +	if (((IS_SKYLAKE(dev) && intel_get_stepping(dev) == 'C') ||
> > > +		(IS_BROXTON(dev) && intel_get_stepping(dev) == 'A')) &&
> > > +		fb->pixel_format == DRM_FORMAT_NV12) {
> > > +			I915_WRITE(CHICKEN_PIPESL(pipe),
> > > +				I915_READ(CHICKEN_PIPESL(pipe)) |
> > DISABLE_STREAMER_FIX);
> > > +	}
> > 
> > According to Bspec this would need to be disabled for render
> > compression. And to do that we'd need to add some vblank waits to make
> > sure we don't disable it too soon. But since it's pre-production
> > hardware anyway I guess we might not care too much.
> 
> Render compression related checks will be coming as part of that patch.
> 
> > 
> > I would probably drop SKL from these since I'd assume almost everyone
> > has D+ by now. And maybe just stuff it in init_clock_gating for BXT
> > since we're going to eliminate it soonish anyway.
> 
> Last checked some folks are still using, so keep it intact for completeness.
> 
> > > +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.

But we'll run into problems as soon as render compression enters the
picure. But I don't have a problem leaving it up to the render
compression patches to solve.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx