<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Sep 18, 2014 at 4:39 AM, Daniel Vetter <span dir="ltr"><<a href="mailto:daniel@ffwll.ch" target="_blank">daniel@ffwll.ch</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Thu, Sep 18, 2014 at 07:28:48AM +0100, Chris Wilson wrote:<br>
> On Wed, Sep 17, 2014 at 04:59:20PM -0400, Rodrigo Vivi wrote:<br>
> > If it wasn't never enabled by kernel parameter or platform default<br>
> > we can avoid reading registers so many times in vain<br>
><br>
> Nak.<br>
<br>
</span>Well I've merged this for now to reduce fbc impact.<br></blockquote><div><br></div><div>Uhm, unfortunatelly I'm afraid Chris was right.</div><div>Paulo also nacked it. Because it just helps when it was explicitly disabled by setting i915.enable_fbc=0 while the default is -1.</div><div><br></div><div>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.</div><div> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class=""><br>
> > Cc: Paulo Zanoni <<a href="mailto:paulo.r.zanoni@intel.com">paulo.r.zanoni@intel.com</a>><br>
> > Signed-off-by: Rodrigo Vivi <<a href="mailto:rodrigo.vivi@intel.com">rodrigo.vivi@intel.com</a>><br>
> > ---<br>
> >  drivers/gpu/drm/i915/intel_pm.c | 6 ++++++<br>
> >  1 file changed, 6 insertions(+)<br>
> ><br>
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c<br>
> > index a988862..afcc284 100644<br>
> > --- a/drivers/gpu/drm/i915/intel_pm.c<br>
> > +++ b/drivers/gpu/drm/i915/intel_pm.c<br>
> > @@ -339,6 +339,12 @@ bool intel_fbc_enabled(struct drm_device *dev)<br>
> >  {<br>
> >     struct drm_i915_private *dev_priv = dev->dev_private;<br>
> ><br>
> > +   /* If it wasn't never enabled by kernel parameter or platform default<br>
> > +    * we can avoid reading registers so many times in vain<br>
> > +    */<br>
> > +   if (!i915.enable_fbc)<br>
> > +           return false;<br>
> > +<br>
> >     if (!dev_priv->display.fbc_enabled)<br>
> >             return false;<br>
><br>
> The correct state to check here is whether we have enabled fbc, not the<br>
> module parameter which just rules whether we should turn it on.<br>
> Look at making dev_priv->fbc.no_fbc_reason always correct instead.<br>
<br>
</span>Well we need to fix this all up anyway, since pretty much everything on<br>
the software side of fbc is busted (locking, tracking, piles of rechecks<br>
and other hilarious stuff).<br></blockquote><div><br></div><div>I absolutely agree with you Daniel. We need a full fbc rework and simplification.</div><div><br></div><div>But for now we need a quick stuff to protect the current code.</div><div><br></div><div>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.</div><div><br></div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class="HOEnZb"><font color="#888888">-Daniel<br>
--<br>
Daniel Vetter<br>
Software Engineer, Intel Corporation<br>
<a href="tel:%2B41%20%280%29%2079%20365%2057%2048" value="+41793655748">+41 (0) 79 365 57 48</a> - <a href="http://blog.ffwll.ch" target="_blank">http://blog.ffwll.ch</a><br>
</font></span><div class="HOEnZb"><div class="h5">_______________________________________________<br>
Intel-gfx mailing list<br>
<a href="mailto:Intel-gfx@lists.freedesktop.org">Intel-gfx@lists.freedesktop.org</a><br>
<a href="http://lists.freedesktop.org/mailman/listinfo/intel-gfx" target="_blank">http://lists.freedesktop.org/mailman/listinfo/intel-gfx</a><br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div>Rodrigo Vivi</div><div>Blog: <a href="http://blog.vivi.eng.br" target="_blank">http://blog.vivi.eng.br</a></div><div> </div>
</div></div>