[PATCH-V2 1/2] drm/i915: add callback to enable/disable codec wakeup

[PATCH-V2 1/2] drm/i915: add callback to enable/disable codec wakeup

[han.lu]

From: "Lu, Han" <han.lu@intel.com>

In SKL, HDMI/DP codec and PCH HD Audio Controller are in different
power wells, so it's necessary to reset display audio codecs when
power well on, otherwise display audio codecs will disappear when
resume from low power state.
The reset step when power on is:
    enable codec wakeup -> azx_init_chip() -> disable codec wakeup

The callback is defined in drivers/gpu/drm/i915/.
The caller is in sound/pci/hda/.

Signed-off-by: Lu, Han <han.lu@intel.com>

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f3c77ca..efcf900 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1143,6 +1143,11 @@ struct i915_power_domains {
 	struct i915_power_well *power_wells;
 };
 
+struct i915_audio_priv {
+	struct mutex lock;
+	int audio_power_cnt;
+};
+
 #define MAX_L3_SLICES 2
 struct intel_l3_parity {
 	u32 *remap_info[MAX_L3_SLICES];
@@ -1757,6 +1762,8 @@ struct drm_i915_private {
 	/* hda/i915 audio component */
 	bool audio_component_registered;
 
+	struct i915_audio_priv audio_priv;
+
 	uint32_t hw_context_size;
 	struct list_head context_list;
 
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 36805b6..efdccaf 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6881,6 +6881,9 @@ enum skl_disp_power_wells {
 #define   AUDIO_CP_READY(trans)		((1 << 1) << ((trans) * 4))
 #define   AUDIO_ELD_VALID(trans)	((1 << 0) << ((trans) * 4))
 
+#define HSW_AUD_CHICKENBIT			0x65f10
+#define   AUD_CODEC_WAKE_SIGNAL			(1 << 15)
+
 /* HSW Power Wells */
 #define HSW_PWR_WELL_BIOS			0x45400 /* CTL1 */
 #define HSW_PWR_WELL_DRIVER			0x45404 /* CTL2 */
diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index f72e93a..e0cf2b9 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -448,6 +448,9 @@ void intel_audio_codec_disable(struct intel_encoder *encoder)
 void intel_init_audio(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct i915_audio_priv *audio_priv = &dev_priv->audio_priv;
+
+	mutex_init(&audio_priv->lock);
 
 	if (IS_G4X(dev)) {
 		dev_priv->display.audio_codec_enable = g4x_audio_codec_enable;
@@ -474,6 +477,60 @@ static void i915_audio_component_put_power(struct device *dev)
 	intel_display_power_put(dev_to_i915(dev), POWER_DOMAIN_AUDIO);
 }
 
+static void i915_audio_component_enable_codec_wakeup(struct device *dev)
+{
+	struct drm_i915_private *dev_priv = dev_to_i915(dev);
+	struct i915_audio_priv *audio_priv = &dev_priv->audio_priv;
+	u32 tmp;
+
+	if (!IS_SKYLAKE(dev_priv->dev))
+		return;
+
+	mutex_lock(&audio_priv->lock);
+
+	/* SKL need reset the CHICKENBIT bit 15 after power on codec */
+	if (audio_priv->audio_power_cnt == 0) {
+		tmp = I915_READ(HSW_AUD_CHICKENBIT);
+		tmp &= ~AUD_CODEC_WAKE_SIGNAL;
+		I915_WRITE(HSW_AUD_CHICKENBIT, tmp);
+		mdelay(1);
+
+		tmp = I915_READ(HSW_AUD_CHICKENBIT);
+		tmp |= AUD_CODEC_WAKE_SIGNAL;
+		I915_WRITE(HSW_AUD_CHICKENBIT, tmp);
+		mdelay(1);
+	}
+
+	audio_priv->audio_power_cnt++;
+
+	mutex_unlock(&audio_priv->lock);
+}
+
+static void i915_audio_component_disable_codec_wakeup(struct device *dev)
+{
+	struct drm_i915_private *dev_priv = dev_to_i915(dev);
+	struct i915_audio_priv *audio_priv = &dev_priv->audio_priv;
+	u32 tmp;
+
+	if (!IS_SKYLAKE(dev_priv->dev))
+		return;
+
+	mutex_lock(&audio_priv->lock);
+
+	audio_priv->audio_power_cnt--;
+	if (audio_priv->audio_power_cnt < 0)
+		dev_warn(dev, "audio_power_cnt(%d) < 0\n",
+				audio_priv->audio_power_cnt);
+
+	/* SKL need clear the CHICKENBIT bit 15 after codec detected */
+	tmp = I915_READ(HSW_AUD_CHICKENBIT);
+	tmp &= ~AUD_CODEC_WAKE_SIGNAL;
+	I915_WRITE(HSW_AUD_CHICKENBIT, tmp);
+	mdelay(1);
+
+	mutex_unlock(&audio_priv->lock);
+}
+
 /* Get CDCLK in kHz  */
 static int i915_audio_component_get_cdclk_freq(struct device *dev)
 {
@@ -495,6 +552,8 @@ static const struct i915_audio_component_ops i915_audio_component_ops = {
 	.owner		= THIS_MODULE,
 	.get_power	= i915_audio_component_get_power,
 	.put_power	= i915_audio_component_put_power,
+	.enable_codec_wakeup	= i915_audio_component_enable_codec_wakeup,
+	.disable_codec_wakeup	= i915_audio_component_disable_codec_wakeup,
 	.get_cdclk_freq	= i915_audio_component_get_cdclk_freq,
 };
 
diff --git a/include/drm/i915_component.h b/include/drm/i915_component.h
index 3e2f22e..1d57aae 100644
--- a/include/drm/i915_component.h
+++ b/include/drm/i915_component.h
@@ -31,6 +31,8 @@ struct i915_audio_component {
 		struct module *owner;
 		void (*get_power)(struct device *);
 		void (*put_power)(struct device *);
+		void (*enable_codec_wakeup)(struct device *);
+		void (*disable_codec_wakeup)(struct device *);
 		int (*get_cdclk_freq)(struct device *);
 	} *ops;
 };
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH-V2 2/2] ALSA:hda - reset display codec when power on

[han.lu]

From: "Lu, Han" <han.lu@intel.com>

In SKL, HDMI/DP codec and PCH HD Audio Controller are in different
power wells, so it's necessary to reset display audio codecs when
power well on, otherwise display audio codecs will disappear when
resume from low power state.
The reset step when power on is:
    enable codec wakeup -> azx_init_chip() -> disable codec wakeup

The callback is defined in drivers/gpu/drm/i915/.
The caller is in sound/pci/hda/.

Signed-off-by: Lu, Han <han.lu@intel.com>

diff --git a/sound/pci/hda/hda_i915.c b/sound/pci/hda/hda_i915.c
index 3052a2b..e5ebedc 100644
--- a/sound/pci/hda/hda_i915.c
+++ b/sound/pci/hda/hda_i915.c
@@ -33,6 +33,23 @@
 #define AZX_REG_EM4			0x100c
 #define AZX_REG_EM5			0x1010
 
+int hda_set_codec_wakeup(struct hda_intel *hda, bool enable)
+{
+	struct i915_audio_component *acomp = &hda->audio_component;
+
+	if (!acomp->ops)
+		return -ENODEV;
+
+	dev_dbg(&hda->chip.pci->dev, "%s codec wakeup\n",
+		enable ? "enable" : "disable");
+	if (enable)
+		acomp->ops->enable_codec_wakeup(acomp->dev);
+	else
+		acomp->ops->disable_codec_wakeup(acomp->dev);
+
+	return 0;
+}
+
 int hda_display_power(struct hda_intel *hda, bool enable)
 {
 	struct i915_audio_component *acomp = &hda->audio_component;
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index 34040d2..b0f2490 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -819,6 +819,8 @@ static int azx_resume(struct device *dev)
 	if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL) {
 		hda_display_power(hda, true);
 		haswell_set_bclk(hda);
+		/* generate codec wakeup signal */
+		hda_set_codec_wakeup(hda, true);
 	}
 	if (chip->msi)
 		if (pci_enable_msi(pci) < 0)
@@ -829,6 +831,10 @@ static int azx_resume(struct device *dev)
 
 	azx_init_chip(chip, true);
 
+	/* disable codec wakeup signal */
+	if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL)
+		hda_set_codec_wakeup(hda, false);
+
 	snd_power_change_state(card, SNDRV_CTL_POWER_D0);
 	return 0;
 }
@@ -888,6 +894,8 @@ static int azx_runtime_resume(struct device *dev)
 	if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL) {
 		hda_display_power(hda, true);
 		haswell_set_bclk(hda);
+		/* generate codec wakeup signal */
+		hda_set_codec_wakeup(hda, true);
 	}
 
 	/* Read STATESTS before controller reset */
@@ -896,6 +904,10 @@ static int azx_runtime_resume(struct device *dev)
 	azx_init_pci(chip);
 	azx_init_chip(chip, true);
 
+	/* disable codec wakeup signal */
+	if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL)
+		hda_set_codec_wakeup(hda, false);
+
 	bus = chip->bus;
 	if (status && bus) {
 		list_for_each_codec(codec, bus)
@@ -1450,6 +1462,7 @@ static int azx_first_init(struct azx *chip)
 	int dev = chip->dev_index;
 	struct pci_dev *pci = chip->pci;
 	struct snd_card *card = chip->card;
+	struct hda_intel *hda = container_of(chip, struct hda_intel, chip);
 	int err;
 	unsigned short gcap;
 	unsigned int dma_bits = 64;
@@ -1582,14 +1595,17 @@ static int azx_first_init(struct azx *chip)
 	azx_init_pci(chip);
 
 	if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL) {
-		struct hda_intel *hda;
-
-		hda = container_of(chip, struct hda_intel, chip);
 		haswell_set_bclk(hda);
+		/* generate codec wakeup signal */
+		hda_set_codec_wakeup(hda, true);
 	}
 
 	azx_init_chip(chip, (probe_only[dev] & 2) == 0);
 
+	/* disable codec wakeup signal */
+	if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL)
+		hda_set_codec_wakeup(hda, false);
+
 	/* codec detection */
 	if (!chip->codec_mask) {
 		dev_err(card->dev, "no codecs found!\n");
diff --git a/sound/pci/hda/hda_intel.h b/sound/pci/hda/hda_intel.h
index d5231f7..5d2bb07 100644
--- a/sound/pci/hda/hda_intel.h
+++ b/sound/pci/hda/hda_intel.h
@@ -48,11 +48,16 @@ struct hda_intel {
 };
 
 #ifdef CONFIG_SND_HDA_I915
+int hda_set_codec_wakeup(struct hda_intel *hda, bool enable);
 int hda_display_power(struct hda_intel *hda, bool enable);
 void haswell_set_bclk(struct hda_intel *hda);
 int hda_i915_init(struct hda_intel *hda);
 int hda_i915_exit(struct hda_intel *hda);
 #else
+static inline int hda_set_codec_wakeup(struct hda_intel *hda, bool enable)
+{
+	return 0;
+}
 static inline int hda_display_power(struct hda_intel *hda, bool enable)
 {
 	return 0;
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH-V2 2/2] ALSA:hda - reset display codec when power on

[shuang.he]

Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6277
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  276/276              276/276
ILK                                  302/302              302/302
SNB                                  318/318              318/318
IVB                                  341/341              341/341
BYT                                  287/287              287/287
BDW                                  318/318              318/318
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH-V2 1/2] drm/i915: add callback to enable/disable codec wakeup

[Jani Nikula]

On Tue, 28 Apr 2015, han.lu@intel.com wrote:
> From: "Lu, Han" <han.lu@intel.com>
>
> In SKL, HDMI/DP codec and PCH HD Audio Controller are in different
> power wells, so it's necessary to reset display audio codecs when
> power well on, otherwise display audio codecs will disappear when
> resume from low power state.
> The reset step when power on is:
>     enable codec wakeup -> azx_init_chip() -> disable codec wakeup
>
> The callback is defined in drivers/gpu/drm/i915/.
> The caller is in sound/pci/hda/.

Han Lu, two more things:

First, is there any chance the chicken bit register is mirrored in the
audio device mmio bar? If yes, you could use it directly in the audio
driver, and we could drop patch 1/2...

Second, looking at the audio driver code for the usage of this callback
in patch 2/2, I don't think the locking and refcounting add any
value. Especially since you ignore the refcounting for disable. You're
the only user, and anything beyond that would be broken anyway, with or
without refcounting or locking. In fact, I'd like to push back the
locking and refcounting to audio driver side, where you may realize they
are not needed at all. Thus, I propose this as patch 1/2:

From b06bf55b94cdbfad8ec35923b2b657674472cce5 Mon Sep 17 00:00:00 2001
From: "Lu, Han" <han.lu@intel.com>
Date: Tue, 28 Apr 2015 17:03:08 +0300
Subject: [PATCH] drm/i915/audio: add codec wakeup override enabled/disable
 callback
Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
Cc: Jani Nikula <jani.nikula@intel.com>

Add support for enabling codec wakeup override signal to allow
re-enumeration of the controller on SKL after resume from low power
state.

v3 by Jani: Simplify to only support toggling the appropriate chicken
bit.

Signed-off-by: Lu, Han <han.lu@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h    |  3 +++
 drivers/gpu/drm/i915/intel_audio.c | 27 +++++++++++++++++++++++++++
 include/drm/i915_component.h       |  1 +
 3 files changed, 31 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 36805b64036b..435c372d001e 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6881,6 +6881,9 @@ enum skl_disp_power_wells {
 #define   AUDIO_CP_READY(trans)		((1 << 1) << ((trans) * 4))
 #define   AUDIO_ELD_VALID(trans)	((1 << 0) << ((trans) * 4))
 
+#define HSW_AUD_CHICKENBIT			0x65f10
+#define   SKL_AUD_CODEC_WAKE_SIGNAL		(1 << 15)
+
 /* HSW Power Wells */
 #define HSW_PWR_WELL_BIOS			0x45400 /* CTL1 */
 #define HSW_PWR_WELL_DRIVER			0x45404 /* CTL2 */
diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index f72e93a45e11..ceb232870d5a 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -474,6 +474,32 @@ static void i915_audio_component_put_power(struct device *dev)
 	intel_display_power_put(dev_to_i915(dev), POWER_DOMAIN_AUDIO);
 }
 
+static void i915_audio_component_codec_wake_override(struct device *dev,
+						     bool enable)
+{
+	struct drm_i915_private *dev_priv = dev_to_i915(dev);
+	u32 tmp;
+
+	if (!IS_SKYLAKE(dev_priv))
+		return;
+
+	/*
+	 * Enable/disable generating the codec wake signal, overriding the
+	 * internal logic to generate the codec wake to controller.
+	 */
+	tmp = I915_READ(HSW_AUD_CHICKENBIT);
+	tmp &= ~SKL_AUD_CODEC_WAKE_SIGNAL;
+	I915_WRITE(HSW_AUD_CHICKENBIT, tmp);
+	usleep_range(1000, 1500);
+
+	if (enable) {
+		tmp = I915_READ(HSW_AUD_CHICKENBIT);
+		tmp |= SKL_AUD_CODEC_WAKE_SIGNAL;
+		I915_WRITE(HSW_AUD_CHICKENBIT, tmp);
+		usleep_range(1000, 1500);
+	}
+}
+
 /* Get CDCLK in kHz  */
 static int i915_audio_component_get_cdclk_freq(struct device *dev)
 {
@@ -495,6 +521,7 @@ static const struct i915_audio_component_ops i915_audio_component_ops = {
 	.owner		= THIS_MODULE,
 	.get_power	= i915_audio_component_get_power,
 	.put_power	= i915_audio_component_put_power,
+	.codec_wake_override = i915_audio_component_codec_wake_override,
 	.get_cdclk_freq	= i915_audio_component_get_cdclk_freq,
 };
 
diff --git a/include/drm/i915_component.h b/include/drm/i915_component.h
index 3e2f22e5bf3c..c9a8b64aa33b 100644
--- a/include/drm/i915_component.h
+++ b/include/drm/i915_component.h
@@ -31,6 +31,7 @@ struct i915_audio_component {
 		struct module *owner;
 		void (*get_power)(struct device *);
 		void (*put_power)(struct device *);
+		void (*codec_wake_override)(struct device *, bool enable);
 		int (*get_cdclk_freq)(struct device *);
 	} *ops;
 };
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH-V2 1/2] drm/i915: add callback to enable/disable codec wakeup

[Lu, Han]

Hi Jani,

Sorry I didn't find method to access chicken bit registers or any other 0x65000 based registers
through audio mmio bar in ALSA side. It looks like the only way to access i915 registers from audio
side is through i915 callback (please correct me if I was wrong). So I apply your patch and it works
well. Thanks a lot!
(and yes, in my case, neither refcounting nor locking is must in ALSA side, so I will not add them this
time.)

BR,
Han Lu

> -----Original Message-----
> From: Nikula, Jani
> Sent: Tuesday, April 28, 2015 10:22 PM
> To: Lu, Han; Vetter, Daniel; tiwai@suse.de; Yang, Libin; Lin, Mengdong; intel-
> gfx@lists.freedesktop.org
> Cc: Lu, Han; Deak, Imre
> Subject: Re: [PATCH-V2 1/2] drm/i915: add callback to enable/disable codec
> wakeup
> 
> On Tue, 28 Apr 2015, han.lu@intel.com wrote:
> > From: "Lu, Han" <han.lu@intel.com>
> >
> > In SKL, HDMI/DP codec and PCH HD Audio Controller are in different
> > power wells, so it's necessary to reset display audio codecs when
> > power well on, otherwise display audio codecs will disappear when
> > resume from low power state.
> > The reset step when power on is:
> >     enable codec wakeup -> azx_init_chip() -> disable codec wakeup
> >
> > The callback is defined in drivers/gpu/drm/i915/.
> > The caller is in sound/pci/hda/.
> 
> Han Lu, two more things:
> 
> First, is there any chance the chicken bit register is mirrored in the audio
> device mmio bar? If yes, you could use it directly in the audio driver, and we
> could drop patch 1/2...
> 
> Second, looking at the audio driver code for the usage of this callback in patch
> 2/2, I don't think the locking and refcounting add any value. Especially since
> you ignore the refcounting for disable. You're the only user, and anything
> beyond that would be broken anyway, with or without refcounting or locking.
> In fact, I'd like to push back the locking and refcounting to audio driver side,
> where you may realize they are not needed at all. Thus, I propose this as
> patch 1/2:
> 
> From b06bf55b94cdbfad8ec35923b2b657674472cce5 Mon Sep 17 00:00:00
> 2001
> From: "Lu, Han" <han.lu@intel.com>
> Date: Tue, 28 Apr 2015 17:03:08 +0300
> Subject: [PATCH] drm/i915/audio: add codec wakeup override
> enabled/disable  callback
> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160
> Espoo
> Cc: Jani Nikula <jani.nikula@intel.com>
> 
> Add support for enabling codec wakeup override signal to allow re-
> enumeration of the controller on SKL after resume from low power state.
> 
> v3 by Jani: Simplify to only support toggling the appropriate chicken bit.
> 
> Signed-off-by: Lu, Han <han.lu@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h    |  3 +++
>  drivers/gpu/drm/i915/intel_audio.c | 27 +++++++++++++++++++++++++++
>  include/drm/i915_component.h       |  1 +
>  3 files changed, 31 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h index 36805b64036b..435c372d001e
> 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6881,6 +6881,9 @@ enum skl_disp_power_wells {
>  #define   AUDIO_CP_READY(trans)		((1 << 1) << ((trans) * 4))
>  #define   AUDIO_ELD_VALID(trans)	((1 << 0) << ((trans) * 4))
> 
> +#define HSW_AUD_CHICKENBIT			0x65f10
> +#define   SKL_AUD_CODEC_WAKE_SIGNAL		(1 << 15)
> +
>  /* HSW Power Wells */
>  #define HSW_PWR_WELL_BIOS			0x45400 /* CTL1 */
>  #define HSW_PWR_WELL_DRIVER			0x45404 /* CTL2 */
> diff --git a/drivers/gpu/drm/i915/intel_audio.c
> b/drivers/gpu/drm/i915/intel_audio.c
> index f72e93a45e11..ceb232870d5a 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -474,6 +474,32 @@ static void
> i915_audio_component_put_power(struct device *dev)
>  	intel_display_power_put(dev_to_i915(dev),
> POWER_DOMAIN_AUDIO);  }
> 
> +static void i915_audio_component_codec_wake_override(struct device
> *dev,
> +						     bool enable)
> +{
> +	struct drm_i915_private *dev_priv = dev_to_i915(dev);
> +	u32 tmp;
> +
> +	if (!IS_SKYLAKE(dev_priv))
> +		return;
> +
> +	/*
> +	 * Enable/disable generating the codec wake signal, overriding the
> +	 * internal logic to generate the codec wake to controller.
> +	 */
> +	tmp = I915_READ(HSW_AUD_CHICKENBIT);
> +	tmp &= ~SKL_AUD_CODEC_WAKE_SIGNAL;
> +	I915_WRITE(HSW_AUD_CHICKENBIT, tmp);
> +	usleep_range(1000, 1500);
> +
> +	if (enable) {
> +		tmp = I915_READ(HSW_AUD_CHICKENBIT);
> +		tmp |= SKL_AUD_CODEC_WAKE_SIGNAL;
> +		I915_WRITE(HSW_AUD_CHICKENBIT, tmp);
> +		usleep_range(1000, 1500);
> +	}
> +}
> +
>  /* Get CDCLK in kHz  */
>  static int i915_audio_component_get_cdclk_freq(struct device *dev)  { @@
> -495,6 +521,7 @@ static const struct i915_audio_component_ops
> i915_audio_component_ops = {
>  	.owner		= THIS_MODULE,
>  	.get_power	= i915_audio_component_get_power,
>  	.put_power	= i915_audio_component_put_power,
> +	.codec_wake_override =
> i915_audio_component_codec_wake_override,
>  	.get_cdclk_freq	= i915_audio_component_get_cdclk_freq,
>  };
> 
> diff --git a/include/drm/i915_component.h
> b/include/drm/i915_component.h index 3e2f22e5bf3c..c9a8b64aa33b 100644
> --- a/include/drm/i915_component.h
> +++ b/include/drm/i915_component.h
> @@ -31,6 +31,7 @@ struct i915_audio_component {
>  		struct module *owner;
>  		void (*get_power)(struct device *);
>  		void (*put_power)(struct device *);
> +		void (*codec_wake_override)(struct device *, bool enable);
>  		int (*get_cdclk_freq)(struct device *);
>  	} *ops;
>  };
> --
> 2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH-V2 1/2] drm/i915: add callback to enable/disable codec wakeup

[Daniel Vetter]

On Tue, Apr 28, 2015 at 05:21:30PM +0300, Jani Nikula wrote:
> On Tue, 28 Apr 2015, han.lu@intel.com wrote:
> > From: "Lu, Han" <han.lu@intel.com>
> >
> > In SKL, HDMI/DP codec and PCH HD Audio Controller are in different
> > power wells, so it's necessary to reset display audio codecs when
> > power well on, otherwise display audio codecs will disappear when
> > resume from low power state.
> > The reset step when power on is:
> >     enable codec wakeup -> azx_init_chip() -> disable codec wakeup
> >
> > The callback is defined in drivers/gpu/drm/i915/.
> > The caller is in sound/pci/hda/.
> 
> Han Lu, two more things:
> 
> First, is there any chance the chicken bit register is mirrored in the
> audio device mmio bar? If yes, you could use it directly in the audio
> driver, and we could drop patch 1/2...
> 
> Second, looking at the audio driver code for the usage of this callback
> in patch 2/2, I don't think the locking and refcounting add any
> value. Especially since you ignore the refcounting for disable. You're
> the only user, and anything beyond that would be broken anyway, with or
> without refcounting or locking. In fact, I'd like to push back the
> locking and refcounting to audio driver side, where you may realize they
> are not needed at all. Thus, I propose this as patch 1/2:
> 
> From b06bf55b94cdbfad8ec35923b2b657674472cce5 Mon Sep 17 00:00:00 2001
> From: "Lu, Han" <han.lu@intel.com>
> Date: Tue, 28 Apr 2015 17:03:08 +0300
> Subject: [PATCH] drm/i915/audio: add codec wakeup override enabled/disable
>  callback
> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
> Cc: Jani Nikula <jani.nikula@intel.com>
> 
> Add support for enabling codec wakeup override signal to allow
> re-enumeration of the controller on SKL after resume from low power
> state.

This commit message is missing the explanation and specifically the hw
sequence. Lu's original patch has a nice paragraph explaining all that.
With that re-added this is Acked-by: me for merging through the sound tree
(which seems to be the best option here I think).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx