On Mon, Feb 26, 2018 at 09:00:50PM +0000, Chris Wilson wrote:
> Quoting Rodrigo Vivi (2018-02-26 20:53:08)
> > -void intel_fbc_handle_fifo_underrun_irq(struct drm_i915_private *dev_priv)
> > -{
> > - struct intel_fbc *fbc = &dev_priv->fbc;
> > -
> > - if (!fbc_supported(dev_priv))
> > - return;
> > -
> > - /* There's no guarantee that underrun_detected won't be set to true
> > - * right after this check and before the work is scheduled, but that's
> > - * not a problem since we'll check it again under the work function
> > - * while FBC is locked. This check here is just to prevent us from
> > - * unnecessarily scheduling the work, and it relies on the fact that we
> > - * never switch underrun_detect back to false after it's true. */
> > - if (READ_ONCE(fbc->underrun_detected))
> > - return;
> > -
> > - schedule_work(&fbc->underrun_work);
> > -}
>
> > +static void intel_handle_fifo_underrun_irq(struct drm_i915_private *dev_priv)
> > +{
> > + if (!HAS_FBC(dev_priv))
> > + return;
> > +
> > + spin_lock(&dev_priv->underrun.lock);
> > +
> > + if (dev_priv->underrun.detected)
> > + goto out;
> > + dev_priv->underrun.detected = true;
> > +
> > + schedule_work(&dev_priv->underrun.work);
> > +out:
> > + spin_unlock(&dev_priv->underrun.lock);
>
> This locking (or bool) isn't required by the following patch either. But
> I presume the boolean is printed at some point, although that's probably
> less useful than whatever FBC/SAGV would say about being disabled.
I honestly didn't fully followed the initial goal of checking the detection
before scheduling the work.... So I assumed it was some underrun storm
and a mechanism to avoid multiple calls to fbc disable...
But yeap... I think we could live without it if no storm is possible
or if internal sagv and fbc are already handling those properly.
> -Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
