[Intel-gfx] [PATCH 4/4] drm/i915/fbc: enable FBC on SKL too

[Daniel Vetter]

On Tue, Mar 22, 2016 at 09:51:34PM +0000, Zanoni, Paulo R wrote:
> Em Ter, 2016-03-22 às 12:16 +0100, Daniel Vetter escreveu:
> > On Mon, Mar 21, 2016 at 04:26:58PM -0300, Paulo Zanoni wrote:
> > > 
> > > Now that we're more protected against user space doing frontbuffer
> > > mmap rendering, the last - how many times did I say this before? -
> > > SKL problem seems to be solved. So let's give it a try.
> > > 
> > > If you reached this commit through git bisect or if you just want
> > > more
> > > information about FBC, please see:
> > >     commit a98ee79317b4091cafb502b4ffdbbbe1335e298c
> > >     Author: Paulo Zanoni <paulo.r.zanoni at intel.com>
> > >     Date:   Tue Feb 16 18:47:21 2016 -0200
> > >         drm/i915/fbc: enable FBC by default on HSW and BDW
> > > 
> > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_fbc.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_fbc.c
> > > b/drivers/gpu/drm/i915/intel_fbc.c
> > > index 718ac38..67f8810 100644
> > > --- a/drivers/gpu/drm/i915/intel_fbc.c
> > > +++ b/drivers/gpu/drm/i915/intel_fbc.c
> > > @@ -1270,7 +1270,8 @@ void intel_fbc_init(struct drm_i915_private
> > > *dev_priv)
> > >  	 * know what's going on. */
> > >  	if (i915.enable_fbc < 0) {
> > >  		i915.enable_fbc = IS_HASWELL(dev_priv) ||
> > > -				  IS_BROADWELL(dev_priv);
> > > +				  IS_BROADWELL(dev_priv) ||
> > > +				  IS_SKYLAKE(dev_priv);
> > Can we just future-proof this and enable on everything gen8+ where we
> > have
> > fbc? Apparently bsw/bxt simply lack this ...
> 
> This can be done, but I'm not sure if it's a good idea, given FBC's
> never-ending history of platform-specific workarounds. We'd force
> people to have to have FBC working right from the start. Hmmm, that
> could actually be a good thing, enforcing people to make features work.

Well we generally do the same in all other places too - we just enable
everything and smash all the patches in. Then testing/power-on happens,
and more fixes pile on top.

We can still disable something if it's completely broken. But I'd really
prefer if this is a concious decision that requires explicit action.
Otherwise I fear we'll "forget" FBC again :( And e.g. modesetting or
rendering also require tons of special cases on each new platform, we
don't disable them either. We just have one overall flag for the entire
driver, which I think should be good enough.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch