On Thu, Feb 16, 2017 at 11:31:01AM +0200, Ander Conselvan De Oliveira wrote: > On Wed, 2017-02-15 at 13:59 +0200, Imre Deak wrote: > > So far the sync_hw hook wasn't called for power wells not belonging to > > any power domain, that is the GEN9 PW1 and MISC_IO power wells. This > > wasn't a problem so far since the goal of the sync_hw hook - to clear > > the corresponding BIOS request bit - was guaranteed by clearing the > > whole BIOS request register elsewhere. This will change with the next > > patch, so fix up the inconsistency. > > > > Cc: Ander Conselvan de Oliveira <[email protected]> > > Cc: David Weinehall <[email protected]> > > Cc: Ville Syrjälä <[email protected]> > > Signed-off-by: Imre Deak <[email protected]> > > --- > > drivers/gpu/drm/i915/intel_runtime_pm.c | 34 > > +++++++++++++++++++++------------ > > 1 file changed, 22 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c > > b/drivers/gpu/drm/i915/intel_runtime_pm.c > > index 0f64bc1..f9aff26 100644 > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > > @@ -49,17 +49,24 @@ > > * present for a given platform. > > */ > > > > -#define for_each_power_well(i, power_well, domain_mask, power_domains) > > \ > > - for (i = 0; \ > > - i < (power_domains)->power_well_count && \ > > +#define for_each_power_well(i, power_well) \ > > + for ((i) = 0; \ > > + (i) < (power_domains)->power_well_count && \ > > This now requires that power_domains is in scope to work properly. Would it > make > sense to still pass that as argument to the macro or, alternatively, pass > dev_priv?
Yes, that was a copy/paste fail. Yep, dev_priv simplifies things, will change to that. > Could probably nuke the i parameter too, since no caller uses it? Ok. I also moved these to i915_dev.h. I'll resend the patchset after a trybot round. > > But either way, > > Reviewed-by: Ander Conselvan de Oliveira <[email protected]> Thanks, Imre > > > ((power_well) = &(power_domains)->power_wells[i]); \ > > - i++) \ > > + (i)++) > > + > > +#define for_each_power_well_rev(i, power_well) > > \ > > + for ((i) = (power_domains)->power_well_count - 1; \ > > + (i) >= 0 && ((power_well) = &(power_domains)->power_wells[i]);\ > > + (i)--) > > + > > +#define for_each_power_domain_well(i, power_well, domain_mask, > > power_domains) \ > > + for_each_power_well(i, power_well) \ > > for_each_if ((power_well)->domains & (domain_mask)) > > > > -#define for_each_power_well_rev(i, power_well, domain_mask, power_domains) > > \ > > - for (i = (power_domains)->power_well_count - 1; \ > > - i >= 0 && ((power_well) = &(power_domains)->power_wells[i]);\ > > - i--) \ > > +#define for_each_power_domain_well_rev(i, power_well, domain_mask, \ > > + power_domains) \ > > + for_each_power_well_rev(i, power_well) \ > > for_each_if ((power_well)->domains & (domain_mask)) > > > > bool intel_display_power_well_is_enabled(struct drm_i915_private *dev_priv, > > @@ -210,7 +217,8 @@ bool __intel_display_power_is_enabled(struct > > drm_i915_private *dev_priv, > > > > is_enabled = true; > > > > - for_each_power_well_rev(i, power_well, BIT_ULL(domain), power_domains) { > > + for_each_power_domain_well_rev(i, power_well, BIT_ULL(domain), > > + power_domains) { > > if (power_well->always_on) > > continue; > > > > @@ -1665,7 +1673,8 @@ __intel_display_power_get_domain(struct > > drm_i915_private *dev_priv, > > struct i915_power_well *power_well; > > int i; > > > > - for_each_power_well(i, power_well, BIT_ULL(domain), power_domains) > > + for_each_power_domain_well(i, power_well, BIT_ULL(domain), > > + power_domains) > > intel_power_well_get(dev_priv, power_well); > > > > power_domains->domain_use_count[domain]++; > > @@ -1760,7 +1769,8 @@ void intel_display_power_put(struct drm_i915_private > > *dev_priv, > > intel_display_power_domain_str(domain)); > > power_domains->domain_use_count[domain]--; > > > > - for_each_power_well_rev(i, power_well, BIT_ULL(domain), power_domains) > > + for_each_power_domain_well_rev(i, power_well, BIT_ULL(domain), > > + power_domains) > > intel_power_well_put(dev_priv, power_well); > > > > mutex_unlock(&power_domains->lock); > > @@ -2427,7 +2437,7 @@ static void intel_power_domains_sync_hw(struct > > drm_i915_private *dev_priv) > > int i; > > > > mutex_lock(&power_domains->lock); > > - for_each_power_well(i, power_well, POWER_DOMAIN_MASK, power_domains) { > > + for_each_power_well(i, power_well) { > > power_well->ops->sync_hw(dev_priv, power_well); > > power_well->hw_enabled = power_well->ops->is_enabled(dev_priv, > > > > power_well); _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
