diff mbox

[v2,3/3] ALSA: hda - implement a refcount for i915 power well switch

Message ID 1430224999-22271-1-git-send-email-mengdong.lin@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Lin, Mengdong April 28, 2015, 12:43 p.m. UTC 
From: Mengdong Lin <mengdong.lin@intel.com>

This is to check the refcount of audio driver and reduce calling to i915.

Signed-off-by: Mengdong Lin <mengdong.lin@intel.com>

Comments

Takashi Iwai April 28, 2015, 1:01 p.m. UTC | #1 
At Tue, 28 Apr 2015 20:43:19 +0800,
mengdong.lin@intel.com wrote:
> 
> From: Mengdong Lin <mengdong.lin@intel.com>
> 
> This is to check the refcount of audio driver and reduce calling to i915.

This has to be implemented before patch 1 & 2, I suppose.

But, you don't mention about the new mutex at all.  Why do you need
it?


Takashi

> 
> Signed-off-by: Mengdong Lin <mengdong.lin@intel.com>
> 
> diff --git a/sound/pci/hda/hda_i915.c b/sound/pci/hda/hda_i915.c
> index 52a85d8..e831691 100644
> --- a/sound/pci/hda/hda_i915.c
> +++ b/sound/pci/hda/hda_i915.c
> @@ -42,11 +42,16 @@ int hda_display_power(struct hda_intel *hda, bool enable)
>  
>  	dev_dbg(&hda->chip.pci->dev, "display power %s\n",
>  		enable ? "enable" : "disable");
> -	if (enable)
> -		acomp->ops->get_power(acomp->dev);
> -	else
> -		acomp->ops->put_power(acomp->dev);
> -
> +	mutex_lock(&hda->display_power.lock);
> +	if (enable) {
> +		if (!hda->display_power.use_count++)
> +			acomp->ops->get_power(acomp->dev);
> +	} else {
> +		WARN_ON(!hda->display_power.use_count);
> +		if (!--hda->display_power.use_count)
> +			acomp->ops->put_power(acomp->dev);
> +	}
> +	mutex_unlock(&hda->display_power.lock);
>  	return 0;
>  }
>  
> @@ -171,6 +176,8 @@ int hda_i915_init(struct hda_intel *hda)
>  
>  	dev_dbg(dev, "bound to i915 component master\n");
>  
> +	mutex_init(&hda->display_power.lock);
> +
>  	return 0;
>  out_master_del:
>  	component_master_del(dev, &hda_component_master_ops);
> diff --git a/sound/pci/hda/hda_intel.h b/sound/pci/hda/hda_intel.h
> index 2069898..3ca000a 100644
> --- a/sound/pci/hda/hda_intel.h
> +++ b/sound/pci/hda/hda_intel.h
> @@ -19,6 +19,11 @@
>  #include <drm/i915_component.h>
>  #include "hda_controller.h"
>  
> +struct i915_display_power {
> +	struct mutex lock;
> +	int use_count;
> +};
> +
>  struct hda_intel {
>  	struct azx chip;
>  
> @@ -46,6 +51,7 @@ struct hda_intel {
>  
>  	/* i915 component interface */
>  	struct i915_audio_component audio_component;
> +	struct i915_display_power display_power;
>  };
>  
>  #ifdef CONFIG_SND_HDA_I915
> -- 
> 1.9.1
>
Lin, Mengdong April 28, 2015, 3:09 p.m. UTC | #2 
> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Tuesday, April 28, 2015 9:01 PM

> > This is to check the refcount of audio driver and reduce calling to i915.
> 
> This has to be implemented before patch 1 & 2, I suppose.

You're right. I'll change the patch order.

> But, you don't mention about the new mutex at all.  Why do you need it?

I'll remove the mutex. 
Actually we need not this mutex for legacy HD-A at all. I'm not sure about the new SKL driver and so add a mutex. 
But since the HW power dependency is same no matter which driver is used on SKL, we could assume the new driver will also request/release power in probe and codec runtime PM ops.

Thanks
Mengdong
 
> 
> Takashi
> 
> >
> > Signed-off-by: Mengdong Lin <mengdong.lin@intel.com>
> >
> > diff --git a/sound/pci/hda/hda_i915.c b/sound/pci/hda/hda_i915.c index
> > 52a85d8..e831691 100644
> > --- a/sound/pci/hda/hda_i915.c
> > +++ b/sound/pci/hda/hda_i915.c
> > @@ -42,11 +42,16 @@ int hda_display_power(struct hda_intel *hda, bool
> > enable)
> >
> >  	dev_dbg(&hda->chip.pci->dev, "display power %s\n",
> >  		enable ? "enable" : "disable");
> > -	if (enable)
> > -		acomp->ops->get_power(acomp->dev);
> > -	else
> > -		acomp->ops->put_power(acomp->dev);
> > -
> > +	mutex_lock(&hda->display_power.lock);
> > +	if (enable) {
> > +		if (!hda->display_power.use_count++)
> > +			acomp->ops->get_power(acomp->dev);
> > +	} else {
> > +		WARN_ON(!hda->display_power.use_count);
> > +		if (!--hda->display_power.use_count)
> > +			acomp->ops->put_power(acomp->dev);
> > +	}
> > +	mutex_unlock(&hda->display_power.lock);
> >  	return 0;
> >  }
> >
> > @@ -171,6 +176,8 @@ int hda_i915_init(struct hda_intel *hda)
> >
> >  	dev_dbg(dev, "bound to i915 component master\n");
> >
> > +	mutex_init(&hda->display_power.lock);
> > +
> >  	return 0;
> >  out_master_del:
> >  	component_master_del(dev, &hda_component_master_ops); diff --git
> > a/sound/pci/hda/hda_intel.h b/sound/pci/hda/hda_intel.h index
> > 2069898..3ca000a 100644
> > --- a/sound/pci/hda/hda_intel.h
> > +++ b/sound/pci/hda/hda_intel.h
> > @@ -19,6 +19,11 @@
> >  #include <drm/i915_component.h>
> >  #include "hda_controller.h"
> >
> > +struct i915_display_power {
> > +	struct mutex lock;
> > +	int use_count;
> > +};
> > +
> >  struct hda_intel {
> >  	struct azx chip;
> >
> > @@ -46,6 +51,7 @@ struct hda_intel {
> >
> >  	/* i915 component interface */
> >  	struct i915_audio_component audio_component;
> > +	struct i915_display_power display_power;
> >  };
> >
> >  #ifdef CONFIG_SND_HDA_I915
> > --
> > 1.9.1
> >
diff mbox

Patch

diff --git a/sound/pci/hda/hda_i915.c b/sound/pci/hda/hda_i915.c
index 52a85d8..e831691 100644
--- a/sound/pci/hda/hda_i915.c
+++ b/sound/pci/hda/hda_i915.c
@@ -42,11 +42,16 @@  int hda_display_power(struct hda_intel *hda, bool enable)
 
 	dev_dbg(&hda->chip.pci->dev, "display power %s\n",
 		enable ? "enable" : "disable");
-	if (enable)
-		acomp->ops->get_power(acomp->dev);
-	else
-		acomp->ops->put_power(acomp->dev);
-
+	mutex_lock(&hda->display_power.lock);
+	if (enable) {
+		if (!hda->display_power.use_count++)
+			acomp->ops->get_power(acomp->dev);
+	} else {
+		WARN_ON(!hda->display_power.use_count);
+		if (!--hda->display_power.use_count)
+			acomp->ops->put_power(acomp->dev);
+	}
+	mutex_unlock(&hda->display_power.lock);
 	return 0;
 }
 
@@ -171,6 +176,8 @@  int hda_i915_init(struct hda_intel *hda)
 
 	dev_dbg(dev, "bound to i915 component master\n");
 
+	mutex_init(&hda->display_power.lock);
+
 	return 0;
 out_master_del:
 	component_master_del(dev, &hda_component_master_ops);
diff --git a/sound/pci/hda/hda_intel.h b/sound/pci/hda/hda_intel.h
index 2069898..3ca000a 100644
--- a/sound/pci/hda/hda_intel.h
+++ b/sound/pci/hda/hda_intel.h
@@ -19,6 +19,11 @@ 
 #include <drm/i915_component.h>
 #include "hda_controller.h"
 
+struct i915_display_power {
+	struct mutex lock;
+	int use_count;
+};
+
 struct hda_intel {
 	struct azx chip;
 
@@ -46,6 +51,7 @@  struct hda_intel {
 
 	/* i915 component interface */
 	struct i915_audio_component audio_component;
+	struct i915_display_power display_power;
 };
 
 #ifdef CONFIG_SND_HDA_I915