[Intel-gfx] drm/i915/fbc: disable FBC on FIFO underruns

[Zanoni, Paulo R]

Em Sáb, 2016-08-13 às 10:05 +0100, Chris Wilson escreveu:
> On Fri, Aug 12, 2016 at 11:24:59PM +0000, Pandiyan, Dhinakaran wrote:
> Do we know why we get black screens in this scenario?

I don't know exactly, but I can speculate that it's probably because
the display engine fails to decompress the framebuffer and gets
completely lost on when the data ends, so it gives up and shows black.

> > 
> > > 
> > > +/**
> > > + * intel_fbc_handle_fifo_underrun - disable FBC when we get a
> > > FIFO underrun
> > > + * @dev_priv: i915 device instance
> > > + *
> > > + * Without FBC, most underruns are harmless and don't really
> > > cause too many
> > > + * problems, except for an annoying message on dmesg. With FBC,
> > > underruns can
> > > + * become black screens or even worse, especially when paired
> > > with bad
> > > + * watermarks. So in order for us to be on the safe side,
> > > completely disable FBC
> > > + * in case we ever detect a FIFO underrun on any pipe. An
> > > underrun on any pipe
> > > + * already suggests that watermarks may be bad, so try to be as
> > > safe as
> > > + * possible.
> > > + */
> > > +void intel_fbc_handle_fifo_underrun(struct drm_i915_private
> > > *dev_priv)
> > > +{
> > > +	struct intel_fbc *fbc = &dev_priv->fbc;
> > > +
> > > +	if (!fbc_supported(dev_priv))
> > > +		return;
> > > +
> > 
> > Should we bail out if fbc is not enabled?

No. Even if FBC is disabled or deactivated we need to make sure the
code doesn't decide to enable/activate FBC later, and simply disabling
it now won't prevent it from being reactivated on the next modesets.

> > Also, can we just disable fbc if we see an underrun instead of
> > using a
> > new flag to prevent activation?

No, for the same reason as above: we don't want the code to decide to
enable/activate FBC later.

> 
> The information that is crucially absent in the function name, and
> its
> kerneldoc, is that this function is run from hardirq context. There
> isn't
> much you are allowed to do here but schedule work.

You're right. I'll rename the function and improve the doc.

> -Chris
>