Em Qua, 2015-10-21 às 18:31 +0100, chris@xxxxxxxxxxxxxxxxxx escreveu:
> On Wed, Oct 21, 2015 at 05:08:42PM +0000, Zanoni, Paulo R wrote:
> > Em Ter, 2015-10-20 às 16:59 +0100, Chris Wilson escreveu:
> > > On Tue, Oct 20, 2015 at 11:49:49AM -0200, Paulo Zanoni wrote:
> > > > There's no need to stop and restart FBC: a nuke should be fine.
> > > >
> > > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
> > > > ---
> > > > drivers/gpu/drm/i915/intel_fbc.c | 6 ++++--
> > > > 1 file changed, 4 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_fbc.c
> > > > b/drivers/gpu/drm/i915/intel_fbc.c
> > > > index 9477379..b9cfd16 100644
> > > > --- a/drivers/gpu/drm/i915/intel_fbc.c
> > > > +++ b/drivers/gpu/drm/i915/intel_fbc.c
> > > > @@ -1088,8 +1088,10 @@ void intel_fbc_flush(struct
> > > > drm_i915_private
> > > > *dev_priv,
> > > > if (origin == ORIGIN_FLIP) {
> > > > __intel_fbc_update(dev_priv);
> > > > } else {
> > > > - __intel_fbc_disable(dev_priv);
> > > > - __intel_fbc_update(dev_priv);
> > > > + if (dev_priv->fbc.enabled)
> > > > + intel_fbc_nuke(dev_priv);
> > >
> > > Ok, what does nuke actually do? From the name, I would expect FBC
> > > to
> > > be
> > > left in an unusable state.
> >
> > As far as I understand, it triggers a full recompression of the
> > CFB. It
> > should be equivalent to disable+reenable.
>
> Maybe intel_fbc_recompress(), that seems a little more obvious than
> nuke?
Sure. I guess I just got used to seeing 'nuke' on the specs and forgot
that people without the specs would not know what it means.
> > >
> > > > + else
> > > > + __intel_fbc_update(dev_priv);
> > > > }
> > > > }
> > >
> > > This becomes
> > >
> > > if (enabled && origin != ORIGIN_FLIP)
> > > intel_fbc_nuke();
> > > else
> > > __intel_fbc_update();
> >
> > Now I see this code could definitely have been made simpler...
> > Fixing
> > this here would require me to redo many of the next patches. I hope
> > you
> > accept patch 19/18 as a possible "fix".
>
> Sure.
>
> > >
> > > It seems a little odd that anything is done if disabled, so care
> > > to
> > > elaborate that reason
> >
> > When we're drawing on the frontbuffer we may get an invalidate()
> > call
> > first, which will trigger an FBC deactivation. Then later we'll get
> > a
> > flush() and will have to reenable. Sometimes we may just get the
> > flush() without the previous invalidate(), and for this case a nuke
> > is
> > the easiest thing to do. That's all just the normal frontbuffer
> > tracking mechanism.
> >
> >
> > > , and I presume there is an equally good comment
> > > before the context that explains why FLIP is special?
> >
> > It's just that we ignore flushes() for the FLIP case if FBC is
> > active
> > due to the hardware tracking, which automatically does a nuke.
> > There's
> > a check for this earlier on this function, which you can't see on
> > this
> > diff context but you can see on patch 02/18. So if origin is FLIP,
> > and
> > FBC is active, we return early.
>
> I like this comment. Care to add it to the function?
Will do.
> -Chris
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
