[Intel-gfx] [PATCH 10/18] drm/i915: introduce is_active/activate/deactivate to the FBC terminology

chris at chris-wilson.co.uk chris at chris-wilson.co.uk
Wed Oct 21 10:55:22 PDT 2015


On Wed, Oct 21, 2015 at 05:45:33PM +0000, Zanoni, Paulo R wrote:
> Em Qua, 2015-10-21 às 13:34 +0100, Chris Wilson escreveu:
> > On Tue, Oct 20, 2015 at 11:49:56AM -0200, Paulo Zanoni wrote:
> > > The long term goal is to have enable/disable as the higher level
> > > functions and activate/deactivate as the lower level functions,
> > > just
> > > like we do for PSR and for the CRTC. Let's start this by renaming
> > > the
> > > functions that touch the hardware state and their wrappers.
> > 
> > So enable() calls activate() and disable() calls deactivate(). So
> > what's the
> > benefit? 
> 
> I explained each individual change on its own patch, but I guess I
> should have put a higher level description here. Will fix this in v2.
> 
> With just this patch there's really no benefit. The main benefit is patch 12, when we actually have separate enable/disable and activate/deactivate functions.
> 
> One of the main points is that enable/disable are called once per modeset, while update/activate/deactivate can be called tons of times during normal operation, so moving code to enable() when possible makes sure it is not ran over and over again unnecessarily.
> 
> 
> > What mistakes and confusion are made right now 
> 
> The confusion right now is that we don't have the real higher level
> enable/disable that we get on patch 12.
> 
> > and is the
> > mismatch between low/high worth it? This is your chance to justify
> > the
> > churn and sell us on the new naming scheme, and explain your long
> > term
> > vision in making the driver consistent everywhere.
> 
> Maybe I should just redirect users to patch 12 on the commit, since
> this patch does not add any value by itself. I could have squashed this
> and 12, but I don't like huge patches: they're not easy to review and
> are a pain to rebase.
> 
> Anyway, v2 will hopefully have a better commit message.

Yes, just explain how you intend to separate the levels should be enough
to justify the change, and the overview in how they are intended to be
used invaluable.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre


More information about the Intel-gfx mailing list