On Sat, Jul 27, 2013 at 03:13:16PM -0700, Ben Widawsky wrote:
> On Sat, Jul 27, 2013 at 05:23:55PM +0100, Chris Wilson wrote:
> > - enum {
> > + enum no_fbc_reason {
> > + FBC_OK, /* FBC is enabled */
> > + FBC_UNSUPPORTED, /* FBC is not supported by this chipset */
>
> Don't we already have FBC_CHIP_DEFAULT for this?
No CHIP_DEFAULT is used for when the module parameter is -1 and the
default behaviour is to off. (So really FBC_MODULE_PARAM). This is
another scenario where we don't even try and perform fbc as there is no
code. It should not be possible for i915_fbc_status to ever report
FBC_UNSUPPORTED, but is possible for it to report CHIP_DEFAULT. It's a
subtle distinction. :)
> > +static bool set_no_fbc_reason(struct drm_i915_private *dev_priv,
> > + enum no_fbc_reason reason)
> > +{
> > + if (dev_priv->fbc.no_fbc_reason == reason)
> > + return false;
> > +
> > + dev_priv->fbc.no_fbc_reason = reason;
> > + return true;
> > +}
> > +
>
> This should only return true if no_fbc_reason != FBC_OK, right?
We want the message on transition from FBC_OK -> error as well so that
we log when we go from enabling FBC to having to disable it for some
reason. That reason is likely to be the same over many off -> on -> off
loops, but useful info I think. You could argue that it would be useful
to refresh the no_fbc_reason message on every modeset for debugging
convenience - but I'm biased as I've found this message to always be
noise.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
