Re: [PATCH v2 3/8] drm/i915: add always-on power wells instead of special casing them

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, Nov 22, 2013 at 02:04:02PM -0200, Paulo Zanoni wrote:
> 2013/11/14 Imre Deak <imre.deak@intel.com>:
> > Instead of using a separate function to check whether a power domain is
> > is always on, add an always-on power well covering all these power
> > domains and do the usual get/put on these unconditionally. Since we
> > don't assign a .set handler for these the get/put won't have any effect
> > besides the adjusted refcount.
> 
> Oh, now I see why you had all those checks for the existence of the
> "set" function :)
> 
> 
> >
> > This makes the code more readable and provides debug info also on the
> > use of always-on power wells (once the relevant debugfs entry is added.)
> >
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h |  1 +
> >  drivers/gpu/drm/i915/intel_pm.c | 41 +++++++++++++----------------------------
> >  2 files changed, 14 insertions(+), 28 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index b20016c..ff3314d 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -946,6 +946,7 @@ struct intel_ilk_power_mgmt {
> >  /* Power well structure for haswell */
> >  struct i915_power_well {
> >         const char *name;
> > +       unsigned always_on:1;
> 
> On our driver we have many cases where we just use many "bool"
> variables, and we also have many cases where we use single-bit
> variables like this. On this specific case we're not gaining anything
> by using the single-bit variable, so I'm not sure if it's the most
> appropriate thing to use. I wish we had a guideline telling us which
> one is preferred on each case :)

I wish we'd use 'bool foo:1' for single bit bitfields. Otherwise stuff
like 'foo = 2' doesn't work (you get false instead of true).

-- 
Ville Syrjälä
Intel OTC