| Message ID | 1394233699-3741-8-git-send-email-przanoni@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Mar 07, 2014 at 08:08:10PM -0300, Paulo Zanoni wrote: > From: Paulo Zanoni <paulo.r.zanoni@intel.com> > > After the latest changes, the indirection is useless. > > Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 16 ++-------------- > 1 file changed, 2 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 88348d6..e49c217 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -6812,12 +6812,6 @@ void __hsw_do_enable_pc8(struct drm_i915_private *dev_priv) > hsw_disable_lcpll(dev_priv, true, true); > } > > -static void __hsw_enable_package_c8(struct drm_i915_private *dev_priv) > -{ > - WARN_ON(!mutex_is_locked(&dev_priv->pc8.lock)); > - intel_runtime_pm_put(dev_priv); > -} > - > void __hsw_do_disable_pc8(struct drm_i915_private *dev_priv) > { > struct drm_device *dev = dev_priv->dev; > @@ -6845,19 +6839,13 @@ void __hsw_do_disable_pc8(struct drm_i915_private *dev_priv) > dev_priv->pc8.enabled = false; > } > > -static void __hsw_disable_package_c8(struct drm_i915_private *dev_priv) > -{ > - WARN_ON(!mutex_is_locked(&dev_priv->pc8.lock)); > - intel_runtime_pm_get(dev_priv); > -} > - > void hsw_enable_package_c8(struct drm_i915_private *dev_priv) > { > if (!HAS_PC8(dev_priv->dev)) > return; > > mutex_lock(&dev_priv->pc8.lock); > - __hsw_enable_package_c8(dev_priv); > + intel_runtime_pm_put(dev_priv); > mutex_unlock(&dev_priv->pc8.lock); > } > > @@ -6867,7 +6855,7 @@ void hsw_disable_package_c8(struct drm_i915_private *dev_priv) > return; > > mutex_lock(&dev_priv->pc8.lock); > - __hsw_disable_package_c8(dev_priv); > + intel_runtime_pm_get(dev_priv); > mutex_unlock(&dev_priv->pc8.lock); Now I am very confused about the locking. -Chris
On Tue, Mar 11, 2014 at 08:20:24AM +0000, Chris Wilson wrote: > On Fri, Mar 07, 2014 at 08:08:10PM -0300, Paulo Zanoni wrote: > > From: Paulo Zanoni <paulo.r.zanoni@intel.com> > > > > After the latest changes, the indirection is useless. > > > > Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org> > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > > --- > > drivers/gpu/drm/i915/intel_display.c | 16 ++-------------- > > 1 file changed, 2 insertions(+), 14 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index 88348d6..e49c217 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -6812,12 +6812,6 @@ void __hsw_do_enable_pc8(struct drm_i915_private *dev_priv) > > hsw_disable_lcpll(dev_priv, true, true); > > } > > > > -static void __hsw_enable_package_c8(struct drm_i915_private *dev_priv) > > -{ > > - WARN_ON(!mutex_is_locked(&dev_priv->pc8.lock)); > > - intel_runtime_pm_put(dev_priv); > > -} > > - > > void __hsw_do_disable_pc8(struct drm_i915_private *dev_priv) > > { > > struct drm_device *dev = dev_priv->dev; > > @@ -6845,19 +6839,13 @@ void __hsw_do_disable_pc8(struct drm_i915_private *dev_priv) > > dev_priv->pc8.enabled = false; > > } > > > > -static void __hsw_disable_package_c8(struct drm_i915_private *dev_priv) > > -{ > > - WARN_ON(!mutex_is_locked(&dev_priv->pc8.lock)); > > - intel_runtime_pm_get(dev_priv); > > -} > > - > > void hsw_enable_package_c8(struct drm_i915_private *dev_priv) > > { > > if (!HAS_PC8(dev_priv->dev)) > > return; > > > > mutex_lock(&dev_priv->pc8.lock); > > - __hsw_enable_package_c8(dev_priv); > > + intel_runtime_pm_put(dev_priv); > > mutex_unlock(&dev_priv->pc8.lock); > > } > > > > @@ -6867,7 +6855,7 @@ void hsw_disable_package_c8(struct drm_i915_private *dev_priv) > > return; > > > > mutex_lock(&dev_priv->pc8.lock); > > - __hsw_disable_package_c8(dev_priv); > > + intel_runtime_pm_get(dev_priv); > > mutex_unlock(&dev_priv->pc8.lock); > > Now I am very confused about the locking. Afacs this confusion all goes away a few patches later on. Otherwise I think this would indeed be ... strange. -Daniel
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 88348d6..e49c217 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -6812,12 +6812,6 @@ void __hsw_do_enable_pc8(struct drm_i915_private *dev_priv) hsw_disable_lcpll(dev_priv, true, true); } -static void __hsw_enable_package_c8(struct drm_i915_private *dev_priv) -{ - WARN_ON(!mutex_is_locked(&dev_priv->pc8.lock)); - intel_runtime_pm_put(dev_priv); -} - void __hsw_do_disable_pc8(struct drm_i915_private *dev_priv) { struct drm_device *dev = dev_priv->dev; @@ -6845,19 +6839,13 @@ void __hsw_do_disable_pc8(struct drm_i915_private *dev_priv) dev_priv->pc8.enabled = false; } -static void __hsw_disable_package_c8(struct drm_i915_private *dev_priv) -{ - WARN_ON(!mutex_is_locked(&dev_priv->pc8.lock)); - intel_runtime_pm_get(dev_priv); -} - void hsw_enable_package_c8(struct drm_i915_private *dev_priv) { if (!HAS_PC8(dev_priv->dev)) return; mutex_lock(&dev_priv->pc8.lock); - __hsw_enable_package_c8(dev_priv); + intel_runtime_pm_put(dev_priv); mutex_unlock(&dev_priv->pc8.lock); } @@ -6867,7 +6855,7 @@ void hsw_disable_package_c8(struct drm_i915_private *dev_priv) return; mutex_lock(&dev_priv->pc8.lock); - __hsw_disable_package_c8(dev_priv); + intel_runtime_pm_get(dev_priv); mutex_unlock(&dev_priv->pc8.lock); }