On Thu, Sep 18, 2014 at 4:39 AM, Daniel Vetter <daniel@xxxxxxxx> 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 <paulo.r.zanoni@xxxxxxxxx>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> > ---
> > 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
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
