| Message ID | 1393540010-1582-11-git-send-email-przanoni@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 2014-02-27 at 19:26 -0300, Paulo Zanoni wrote: > From: Paulo Zanoni <paulo.r.zanoni@intel.com> > > Any power domain will require the HW to be in PCI D0 state, so just do > the simple thing. > > Dear maintainer: since intel_display_power_put() and > intel_display_power_get() are almost identical, git-am has failed to > apply the patch on my local machine once: it added both chunks to > put(), instead of one chunk to get() and another to put(). When you > apply this patch to your tree, please check if it is correct. > > v2: - Add the warning above. > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > --- > drivers/gpu/drm/i915/intel_pm.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index d68fee2..772aa678 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -5341,6 +5341,8 @@ void intel_display_power_get(struct drm_device *dev, > struct i915_power_well *power_well; > int i; > > + intel_runtime_pm_get(dev_priv); > + > power_domains = &dev_priv->power_domains; > > mutex_lock(&power_domains->lock); > @@ -5372,6 +5374,8 @@ void intel_display_power_put(struct drm_device *dev, > __intel_power_well_put(dev, power_well); > > mutex_unlock(&power_domains->lock); > + > + intel_runtime_pm_put(dev_priv); > } I'd prefer to have these in the power_well->enable/disable handlers after applying the VLV power domains patchset. That way we would have the whole power well enable/disable sequence laid out in the same function and avoid going towards a middle-ware like approach. But this is ok, if we apply this patchset first. In that case: Reviewed-by: Imre Deak <imre.deak@intel.com> > > static struct i915_power_domains *hsw_pwr;
2014-02-28 12:45 GMT-03:00 Imre Deak <imre.deak@intel.com>: > On Thu, 2014-02-27 at 19:26 -0300, Paulo Zanoni wrote: >> From: Paulo Zanoni <paulo.r.zanoni@intel.com> >> >> Any power domain will require the HW to be in PCI D0 state, so just do >> the simple thing. >> >> Dear maintainer: since intel_display_power_put() and >> intel_display_power_get() are almost identical, git-am has failed to >> apply the patch on my local machine once: it added both chunks to >> put(), instead of one chunk to get() and another to put(). When you >> apply this patch to your tree, please check if it is correct. >> >> v2: - Add the warning above. >> >> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> >> --- >> drivers/gpu/drm/i915/intel_pm.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c >> index d68fee2..772aa678 100644 >> --- a/drivers/gpu/drm/i915/intel_pm.c >> +++ b/drivers/gpu/drm/i915/intel_pm.c >> @@ -5341,6 +5341,8 @@ void intel_display_power_get(struct drm_device *dev, >> struct i915_power_well *power_well; >> int i; >> >> + intel_runtime_pm_get(dev_priv); >> + >> power_domains = &dev_priv->power_domains; >> >> mutex_lock(&power_domains->lock); >> @@ -5372,6 +5374,8 @@ void intel_display_power_put(struct drm_device *dev, >> __intel_power_well_put(dev, power_well); >> >> mutex_unlock(&power_domains->lock); >> + >> + intel_runtime_pm_put(dev_priv); >> } > > I'd prefer to have these in the power_well->enable/disable handlers > after applying the VLV power domains patchset. That way we would have > the whole power well enable/disable sequence laid out in the same > function and avoid going towards a middle-ware like approach. As long as we get/put runtime PM for every single power domain, including the ones that don't have a corresponding power well, I'm fine your suggestion. But IMHO the current approach is the simplest, and I don't see why we would want do change the order here (notice that I didn't read the VLV docs). > > But this is ok, if we apply this patchset first. In that case: > Reviewed-by: Imre Deak <imre.deak@intel.com> Thanks for the reviews! > >> >> static struct i915_power_domains *hsw_pwr; >
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index d68fee2..772aa678 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5341,6 +5341,8 @@ void intel_display_power_get(struct drm_device *dev, struct i915_power_well *power_well; int i; + intel_runtime_pm_get(dev_priv); + power_domains = &dev_priv->power_domains; mutex_lock(&power_domains->lock); @@ -5372,6 +5374,8 @@ void intel_display_power_put(struct drm_device *dev, __intel_power_well_put(dev, power_well); mutex_unlock(&power_domains->lock); + + intel_runtime_pm_put(dev_priv); } static struct i915_power_domains *hsw_pwr;