diff mbox

[07/16] drm/i915: remove an indirection level on PC8 functions

Message ID 1394233699-3741-8-git-send-email-przanoni@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Paulo Zanoni March 7, 2014, 11:08 p.m. UTC 
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(-)

Comments

Chris Wilson March 11, 2014, 8:20 a.m. UTC | #1 
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
Daniel Vetter March 11, 2014, 2:56 p.m. UTC | #2 
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 mbox

Patch

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);
 }