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

["Zanoni, Paulo R" <paulo.r.zanoni@intel.com>]

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.

> -Chris
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx