On Mon, 2014-02-24 at 10:23 -0300, Paulo Zanoni wrote:
> 2014-02-17 19:02 GMT-03:00 Imre Deak <imre.deak@intel.com>:
> > These functions are used only by a single call site and are simple
> > enough to just fold them in.
> >
> > No functional change.
> >
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> 
> IMHO, this makes the code harder to read. I would prefer to not have
> this change.

I find intel_display_power_put() after patch 19/19 simple enough to not
have the separate functions. I'd say that in this case readability is
actually better with the inlined version, as you don't have to jump
between two functions when reading the code.

--Imre

> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 37 +++++++++++++------------------------
> >  1 file changed, 13 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index aa9c2df..db48d55 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -5305,27 +5305,6 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv,
> >         }
> >  }
> >
> > -static void __intel_power_well_get(struct drm_i915_private *dev_priv,
> > -                                  struct i915_power_well *power_well)
> > -{
> > -       if (!power_well->count++ && power_well->set) {
> > -               hsw_disable_package_c8(dev_priv);
> > -               power_well->set(dev_priv, power_well, true);
> > -       }
> > -}
> > -
> > -static void __intel_power_well_put(struct drm_i915_private *dev_priv,
> > -                                  struct i915_power_well *power_well)
> > -{
> > -       WARN_ON(!power_well->count);
> > -
> > -       if (!--power_well->count && power_well->set &&
> > -           i915.disable_power_well) {
> > -               power_well->set(dev_priv, power_well, false);
> > -               hsw_enable_package_c8(dev_priv);
> > -       }
> > -}
> > -
> >  void intel_display_power_get(struct drm_i915_private *dev_priv,
> >                              enum intel_display_power_domain domain)
> >  {
> > @@ -5338,7 +5317,10 @@ void intel_display_power_get(struct drm_i915_private *dev_priv,
> >         mutex_lock(&power_domains->lock);
> >
> >         for_each_power_well(i, power_well, BIT(domain), power_domains)
> > -               __intel_power_well_get(dev_priv, power_well);
> > +               if (!power_well->count++ && power_well->set) {
> > +                       hsw_disable_package_c8(dev_priv);
> > +                       power_well->set(dev_priv, power_well, true);
> > +               }
> >
> >         power_domains->domain_use_count[domain]++;
> >
> > @@ -5359,8 +5341,15 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
> >         WARN_ON(!power_domains->domain_use_count[domain]);
> >         power_domains->domain_use_count[domain]--;
> >
> > -       for_each_power_well_rev(i, power_well, BIT(domain), power_domains)
> > -               __intel_power_well_put(dev_priv, power_well);
> > +       for_each_power_well_rev(i, power_well, BIT(domain), power_domains) {
> > +               WARN_ON(!power_well->count);
> > +
> > +               if (!--power_well->count && power_well->set &&
> > +                               i915.disable_power_well) {
> > +                       power_well->set(dev_priv, power_well, false);
> > +                       hsw_enable_package_c8(dev_priv);
> > +               }
> > +       }
> >
> >         mutex_unlock(&power_domains->lock);
> >  }
> > --
> > 1.8.4
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
>