On Thu, Sep 18, 2014 at 4:39 AM, Daniel Vetter <[email protected]> wrote:
> On Thu, Sep 18, 2014 at 07:28:48AM +0100, Chris Wilson wrote: > > On Wed, Sep 17, 2014 at 04:59:20PM -0400, Rodrigo Vivi wrote: > > > If it wasn't never enabled by kernel parameter or platform default > > > we can avoid reading registers so many times in vain > > > > Nak. > > Well I've merged this for now to reduce fbc impact. > Uhm, unfortunatelly I'm afraid Chris was right. Paulo also nacked it. Because it just helps when it was explicitly disabled by setting i915.enable_fbc=0 while the default is -1. I though about returning on <= 0, but Paulo is afraid that when enabling back for some platform people would forget to fix this part here and I agree. > > > > Cc: Paulo Zanoni <[email protected]> > > > Signed-off-by: Rodrigo Vivi <[email protected]> > > > --- > > > drivers/gpu/drm/i915/intel_pm.c | 6 ++++++ > > > 1 file changed, 6 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > b/drivers/gpu/drm/i915/intel_pm.c > > > index a988862..afcc284 100644 > > > --- a/drivers/gpu/drm/i915/intel_pm.c > > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > > @@ -339,6 +339,12 @@ bool intel_fbc_enabled(struct drm_device *dev) > > > { > > > struct drm_i915_private *dev_priv = dev->dev_private; > > > > > > + /* If it wasn't never enabled by kernel parameter or platform > default > > > + * we can avoid reading registers so many times in vain > > > + */ > > > + if (!i915.enable_fbc) > > > + return false; > > > + > > > if (!dev_priv->display.fbc_enabled) > > > return false; > > > > The correct state to check here is whether we have enabled fbc, not the > > module parameter which just rules whether we should turn it on. > > Look at making dev_priv->fbc.no_fbc_reason always correct instead. > > Well we need to fix this all up anyway, since pretty much everything on > the software side of fbc is busted (locking, tracking, piles of rechecks > and other hilarious stuff). > I absolutely agree with you Daniel. We need a full fbc rework and simplification. But for now we need a quick stuff to protect the current code. Maybe an extra variable dev_priv.fbc_ever_enabled... This is ugly enough that someone wokirng to get fbc enabled by default would never forget as he can forget about i915.enable_fbc <= 0 at some random point of the code. > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch > _______________________________________________ > Intel-gfx mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > -- Rodrigo Vivi Blog: http://blog.vivi.eng.br
_______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
