[RFC PATCH 0/3] drm/i915: tell bios to update audio controller em4/5 values

[RFC PATCH 0/3] drm/i915: tell bios to update audio controller em4/5 values

[Jani Nikula]

Hi Mengdong -

Here's the first drafts towards fixing [1], but unfortunately this is
still Broadwell specific. Currently patch 2 is not needed, but is
included in case we'll need that too (maybe for Haswell?). Please
review/test, I don't have access to either hsw or bdw right now.

BR,
Jani.

[1] https://bugzilla.kernel.org/show_bug.cgi?id=74861


Jani Nikula (3):
  drm/i915: add opregion function to notify BIOS of CDCLK changes
  drm/i915: add opregion function to enable/disable audio device
  drm/i915: tell BIOS to update audio controller EM4/EM5 divider values

 drivers/gpu/drm/i915/i915_drv.h       | 12 +++++++++++
 drivers/gpu/drm/i915/intel_opregion.c | 39 +++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_pm.c       | 11 ++++++++++
 3 files changed, 62 insertions(+)

-- 
2.0.0

[RFC PATCH 1/3] drm/i915: add opregion function to notify BIOS of CDCLK changes

[Jani Nikula]

Notifying the BIOS about CDCLK changes lets it program audio controller
EM4/EM5 divider values accordingly.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h       |  6 ++++++
 drivers/gpu/drm/i915/intel_opregion.c | 29 +++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8cea59649ef2..2feb8215f9fa 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2602,6 +2602,7 @@ extern int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
 					 bool enable);
 extern int intel_opregion_notify_adapter(struct drm_device *dev,
 					 pci_power_t state);
+extern int intel_opregion_notify_cdclk(struct drm_device *dev, int cdclk);
 #else
 static inline int intel_opregion_setup(struct drm_device *dev) { return 0; }
 static inline void intel_opregion_init(struct drm_device *dev) { return; }
@@ -2617,6 +2618,11 @@ intel_opregion_notify_adapter(struct drm_device *dev, pci_power_t state)
 {
 	return 0;
 }
+static inline int
+intel_opregion_notify_cdclk(struct drm_device *dev, int cdclk)
+{
+	return 0;
+}
 #endif
 
 /* intel_acpi.c */
diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
index 2e2c71fcc9ed..6450d2625624 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -219,6 +219,7 @@ struct opregion_asle {
 #define SWSCI_SBCB_SET_SPREAD_SPECTRUM	SWSCI_FUNCTION_CODE(SWSCI_SBCB, 18)
 #define SWSCI_SBCB_POST_VBE_PM		SWSCI_FUNCTION_CODE(SWSCI_SBCB, 19)
 #define SWSCI_SBCB_ENABLE_DISABLE_AUDIO	SWSCI_FUNCTION_CODE(SWSCI_SBCB, 21)
+#define SWSCI_SBCB_CD_CLOCK_CHANGE	SWSCI_FUNCTION_CODE(SWSCI_SBCB, 22)
 
 #define ACPI_OTHER_OUTPUT (0<<8)
 #define ACPI_VGA_OUTPUT (1<<8)
@@ -395,6 +396,34 @@ int intel_opregion_notify_adapter(struct drm_device *dev, pci_power_t state)
 	return -EINVAL;
 }
 
+int intel_opregion_notify_cdclk(struct drm_device *dev, int cdclk)
+{
+	u32 parm;
+
+	if (!IS_BROADWELL(dev))
+		return 0;
+
+	switch (cdclk) {
+	case 337500:
+		parm = 2;
+		break;
+	case 450000:
+		parm = 0;
+		break;
+	case 540000:
+		parm = 1;
+		break;
+	case 675000:
+		parm = 3;
+		break;
+	default:
+		WARN_ONCE(1, "unknown cdclk %d\n", cdclk);
+		return -EINVAL;
+	}
+
+	return swsci(dev, SWSCI_SBCB_CD_CLOCK_CHANGE, parm, NULL);
+}
+
 static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-- 
2.0.0

[RFC PATCH 2/3] drm/i915: add opregion function to enable/disable audio device

[Jani Nikula]

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h       |  6 ++++++
 drivers/gpu/drm/i915/intel_opregion.c | 10 ++++++++++
 2 files changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2feb8215f9fa..f8e7a74a21ff 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2603,6 +2603,7 @@ extern int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
 extern int intel_opregion_notify_adapter(struct drm_device *dev,
 					 pci_power_t state);
 extern int intel_opregion_notify_cdclk(struct drm_device *dev, int cdclk);
+extern int intel_opregion_audio_enable(struct drm_device *dev, bool enable);
 #else
 static inline int intel_opregion_setup(struct drm_device *dev) { return 0; }
 static inline void intel_opregion_init(struct drm_device *dev) { return; }
@@ -2623,6 +2624,11 @@ intel_opregion_notify_cdclk(struct drm_device *dev, int cdclk)
 {
 	return 0;
 }
+static inline int
+intel_opregion_audio_enable(struct drm_device *dev, bool enable)
+{
+	return 0;
+}
 #endif
 
 /* intel_acpi.c */
diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
index 6450d2625624..7482b687ac20 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -424,6 +424,16 @@ int intel_opregion_notify_cdclk(struct drm_device *dev, int cdclk)
 	return swsci(dev, SWSCI_SBCB_CD_CLOCK_CHANGE, parm, NULL);
 }
 
+int intel_opregion_audio_enable(struct drm_device *dev, bool enable)
+{
+	u32 parm = enable ? 1 : 0;
+
+	if (!IS_HASWELL(dev) && !IS_BROADWELL(dev))
+		return 0;
+
+	return swsci(dev, SWSCI_SBCB_ENABLE_DISABLE_AUDIO, parm, NULL);
+}
+
 static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-- 
2.0.0

[RFC PATCH 3/3] drm/i915: tell BIOS to update audio controller EM4/EM5 divider values

[Jani Nikula]

If the display power well has been disabled, the display audio
controller divider values EM4 MVALUE and EM5 NVALUE will have been
lost. Notify the BIOS about CDCLK change through opregion to make it
reprogram the values when the audio driver requests the power
well. Otherwise the audio playback speed may be wrong.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=74861
Reported-by: Neil Shepperd <nshepperd@gmail.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>

---

NOTE: This will *not* yet fix the referenced bug; this is currently for
Broadwell only.
---
 drivers/gpu/drm/i915/intel_pm.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a90fdbd30edf..b1cbb6f4ec06 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6237,6 +6237,17 @@ int i915_request_power_well(void)
 	dev_priv = container_of(hsw_pwr, struct drm_i915_private,
 				power_domains);
 	intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
+
+	/*
+	 * If the display power well has been disabled, the display audio
+	 * controller divider values EM4 MVALUE and EM5 NVALUE will have been
+	 * lost. Notify the BIOS about CDCLK change through opregion to make it
+	 * reprogram the values. Otherwise the audio playback speed will be
+	 * wrong.
+	 */
+	intel_opregion_notify_cdclk(dev_priv->dev,
+				    intel_ddi_get_cdclk_freq(dev_priv));
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(i915_request_power_well);
-- 
2.0.0

Re: [RFC PATCH 1/3] drm/i915: add opregion function to notify BIOS of CDCLK changes

[Jani Nikula]

On Tue, 01 Jul 2014, Jani Nikula <jani.nikula@intel.com> wrote:
> Notifying the BIOS about CDCLK changes lets it program audio controller
> EM4/EM5 divider values accordingly.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h       |  6 ++++++
>  drivers/gpu/drm/i915/intel_opregion.c | 29 +++++++++++++++++++++++++++++
>  2 files changed, 35 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 8cea59649ef2..2feb8215f9fa 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2602,6 +2602,7 @@ extern int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
>  					 bool enable);
>  extern int intel_opregion_notify_adapter(struct drm_device *dev,
>  					 pci_power_t state);
> +extern int intel_opregion_notify_cdclk(struct drm_device *dev, int cdclk);
>  #else
>  static inline int intel_opregion_setup(struct drm_device *dev) { return 0; }
>  static inline void intel_opregion_init(struct drm_device *dev) { return; }
> @@ -2617,6 +2618,11 @@ intel_opregion_notify_adapter(struct drm_device *dev, pci_power_t state)
>  {
>  	return 0;
>  }
> +static inline int
> +intel_opregion_notify_cdclk(struct drm_device *dev, int cdclk)
> +{
> +	return 0;
> +}
>  #endif
>  
>  /* intel_acpi.c */
> diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
> index 2e2c71fcc9ed..6450d2625624 100644
> --- a/drivers/gpu/drm/i915/intel_opregion.c
> +++ b/drivers/gpu/drm/i915/intel_opregion.c
> @@ -219,6 +219,7 @@ struct opregion_asle {
>  #define SWSCI_SBCB_SET_SPREAD_SPECTRUM	SWSCI_FUNCTION_CODE(SWSCI_SBCB, 18)
>  #define SWSCI_SBCB_POST_VBE_PM		SWSCI_FUNCTION_CODE(SWSCI_SBCB, 19)
>  #define SWSCI_SBCB_ENABLE_DISABLE_AUDIO	SWSCI_FUNCTION_CODE(SWSCI_SBCB, 21)
> +#define SWSCI_SBCB_CD_CLOCK_CHANGE	SWSCI_FUNCTION_CODE(SWSCI_SBCB, 22)
>  
>  #define ACPI_OTHER_OUTPUT (0<<8)
>  #define ACPI_VGA_OUTPUT (1<<8)
> @@ -395,6 +396,34 @@ int intel_opregion_notify_adapter(struct drm_device *dev, pci_power_t state)
>  	return -EINVAL;
>  }
>  
> +int intel_opregion_notify_cdclk(struct drm_device *dev, int cdclk)
> +{
> +	u32 parm;
> +
> +	if (!IS_BROADWELL(dev))
> +		return 0;

To clarify, the spec I have states this call is just for Broadwell, so
it seems we can't just call this on Haswell to fix the bug.

BR,
Jani.

> +
> +	switch (cdclk) {
> +	case 337500:
> +		parm = 2;
> +		break;
> +	case 450000:
> +		parm = 0;
> +		break;
> +	case 540000:
> +		parm = 1;
> +		break;
> +	case 675000:
> +		parm = 3;
> +		break;
> +	default:
> +		WARN_ONCE(1, "unknown cdclk %d\n", cdclk);
> +		return -EINVAL;
> +	}
> +
> +	return swsci(dev, SWSCI_SBCB_CD_CLOCK_CHANGE, parm, NULL);
> +}
> +
>  static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -- 
> 2.0.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center

[RFC PATCH 4/3 :] drm/i915: tell BIOS to update controller EM4/EM5 divider values

[Jani Nikula]

This may do what's required on Haswell to achieve the same as the
previous patch does on Broadwell...

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=74861
Signed-off-by: Jani Nikula <jani.nikula@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 b1cbb6f4ec06..504c8675e8b4 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6248,6 +6248,8 @@ int i915_request_power_well(void)
 	intel_opregion_notify_cdclk(dev_priv->dev,
 				    intel_ddi_get_cdclk_freq(dev_priv));
 
+	intel_opregion_audio_enable(dev_priv->dev, true);
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(i915_request_power_well);
@@ -6260,6 +6262,8 @@ int i915_release_power_well(void)
 	if (!hsw_pwr)
 		return -ENODEV;
 
+	intel_opregion_audio_enable(dev_priv->dev, false);
+
 	dev_priv = container_of(hsw_pwr, struct drm_i915_private,
 				power_domains);
 	intel_display_power_put(dev_priv, POWER_DOMAIN_AUDIO);
-- 
2.0.0