[PATCH 3/3 v2] drm/i915: hot plug/unplug notification to HDMI audio driver

[PATCH 3/3 v2] drm/i915: hot plug/unplug notification to HDMI audio driver

[Wu Fengguang]

On monitor hot plug/unplug, update ELD and set/clear SDVO_AUDIO_ENABLE
or DP_AUDIO_OUTPUT_ENABLE accordingly.  So that the audio driver will
receive hot plug events and take action to refresh its device state and
ELD contents.

A new callback ->hotplug() is added to struct drm_connector_funcs which
will be called immediately after ->detect() knowing that the monitor is
either plugged or unplugged.

It's noticed that X may not call ->mode_set() at monitor hot plug, so it's
necessary to call drm_edid_to_eld() earlier at ->detect() time rather than
in intel_ddc_get_modes(), so that intel_write_eld() can see the new ELD
in ->hotplug.

The call sequence on hot plug is
(the difference part lies in ->mode_set vs ->hotplug)

- KMS console
	->detect
	  drm_edid_to_eld
	->mode_set
	  intel_write_eld
	  set SDVO_AUDIO_ENABLE/DP_AUDIO_OUTPUT_ENABLE
- X
	->detect
	  drm_edid_to_eld
	->hotplug
	  intel_write_eld
	  set SDVO_AUDIO_ENABLE/DP_AUDIO_OUTPUT_ENABLE

On hot remove, the call sequence is

	->hotplug
	  clear SDVO_AUDIO_ENABLE/DP_AUDIO_OUTPUT_ENABLE

cc: Wang Zhenyu <zhenyu.z.wang@intel.com>
Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
---
 drivers/gpu/drm/drm_crtc_helper.c  |    5 ++-
 drivers/gpu/drm/i915/intel_dp.c    |   44 +++++++++++++++++++++------
 drivers/gpu/drm/i915/intel_hdmi.c  |   31 +++++++++++++++++++
 drivers/gpu/drm/i915/intel_modes.c |    2 -
 include/drm/drm_crtc.h             |    1 
 5 files changed, 71 insertions(+), 12 deletions(-)

--- linux.orig/drivers/gpu/drm/i915/intel_dp.c	2011-11-21 11:26:09.000000000 +0800
+++ linux/drivers/gpu/drm/i915/intel_dp.c	2011-11-21 14:12:21.000000000 +0800
@@ -28,6 +28,7 @@
 #include <linux/i2c.h>
 #include <linux/slab.h>
 #include <linux/export.h>
+#include <drm/drm_edid.h>
 #include "drmP.h"
 #include "drm.h"
 #include "drm_crtc.h"
@@ -1970,20 +1971,44 @@ intel_dp_detect(struct drm_connector *co
 	if (status != connector_status_connected)
 		return status;
 
-	if (intel_dp->force_audio) {
-		intel_dp->has_audio = intel_dp->force_audio > 0;
-	} else {
-		edid = intel_dp_get_edid(connector, &intel_dp->adapter);
-		if (edid) {
-			intel_dp->has_audio = drm_detect_monitor_audio(edid);
-			connector->display_info.raw_edid = NULL;
-			kfree(edid);
-		}
+	edid = intel_dp_get_edid(connector, &intel_dp->adapter);
+	if (edid) {
+		intel_dp->has_audio = drm_detect_monitor_audio(edid);
+		drm_edid_to_eld(connector, edid);
+		connector->display_info.raw_edid = NULL;
+		kfree(edid);
 	}
 
+	if (intel_dp->force_audio)
+		intel_dp->has_audio = intel_dp->force_audio > 0;
+
 	return connector_status_connected;
 }
 
+static void intel_dp_hotplug(struct drm_connector *connector)
+{
+	struct intel_dp *intel_dp = intel_attached_dp(connector);
+	struct drm_device *dev = intel_dp->base.base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_crtc *crtc = intel_dp->base.base.crtc;
+
+	DRM_DEBUG_DRIVER("DisplayPort hotplug status %d crtc %p eld %#x\n",
+			 connector->status,
+			 crtc,
+			 (unsigned int)connector->eld[0]);
+
+	if (connector->status == connector_status_disconnected) {
+		intel_dp->DP &= ~DP_AUDIO_OUTPUT_ENABLE;
+	} else if (connector->status == connector_status_connected && crtc) {
+		intel_write_eld(&intel_dp->base.base, &crtc->mode);
+		intel_dp->DP |= DP_AUDIO_OUTPUT_ENABLE;
+	} else {
+		return;
+	}
+	I915_WRITE(intel_dp->output_reg, intel_dp->DP);
+	POSTING_READ(intel_dp->output_reg);
+}
+
 static int intel_dp_get_modes(struct drm_connector *connector)
 {
 	struct intel_dp *intel_dp = intel_attached_dp(connector);
@@ -2143,6 +2168,7 @@ static const struct drm_connector_funcs 
 	.detect = intel_dp_detect,
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.set_property = intel_dp_set_property,
+	.hotplug = intel_dp_hotplug,
 	.destroy = intel_dp_destroy,
 };
 
--- linux.orig/drivers/gpu/drm/i915/intel_hdmi.c	2011-11-21 11:26:09.000000000 +0800
+++ linux/drivers/gpu/drm/i915/intel_hdmi.c	2011-11-21 14:12:26.000000000 +0800
@@ -337,6 +337,7 @@ intel_hdmi_detect(struct drm_connector *
 			status = connector_status_connected;
 			intel_hdmi->has_hdmi_sink = drm_detect_hdmi_monitor(edid);
 			intel_hdmi->has_audio = drm_detect_monitor_audio(edid);
+			drm_edid_to_eld(connector, edid);
 		}
 		connector->display_info.raw_edid = NULL;
 		kfree(edid);
@@ -350,6 +351,35 @@ intel_hdmi_detect(struct drm_connector *
 	return status;
 }
 
+static void intel_hdmi_hotplug(struct drm_connector *connector)
+{
+	struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
+	struct drm_i915_private *dev_priv = connector->dev->dev_private;
+	struct drm_crtc *crtc = intel_hdmi->base.base.crtc;
+	u32 temp;
+
+	DRM_DEBUG_DRIVER("HDMI hotplug status %d crtc %p eld %#x\n",
+			 connector->status,
+			 crtc,
+			 (unsigned int)connector->eld[0]);
+
+	if (connector->status == connector_status_disconnected) {
+		temp = I915_READ(intel_hdmi->sdvox_reg) & ~SDVO_AUDIO_ENABLE;
+	} else if (connector->status == connector_status_connected && crtc) {
+		/*
+		 * It's for X only, which may not call ->mode_set on hot plug.
+		 * KMS console has NULL crtc at this time and will
+		 * call ->mode_set to enable HDMI audio a bit later.
+		 */
+		intel_write_eld(&intel_hdmi->base.base, &crtc->mode);
+		temp = I915_READ(intel_hdmi->sdvox_reg) | SDVO_AUDIO_ENABLE;
+	} else {
+		return;
+	}
+	I915_WRITE(intel_hdmi->sdvox_reg, temp);
+	POSTING_READ(intel_hdmi->sdvox_reg);
+}
+
 static int intel_hdmi_get_modes(struct drm_connector *connector)
 {
 	struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
@@ -459,6 +489,7 @@ static const struct drm_connector_funcs 
 	.detect = intel_hdmi_detect,
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.set_property = intel_hdmi_set_property,
+	.hotplug = intel_hdmi_hotplug,
 	.destroy = intel_hdmi_destroy,
 };
 
--- linux.orig/drivers/gpu/drm/drm_crtc_helper.c	2011-11-21 11:26:09.000000000 +0800
+++ linux/drivers/gpu/drm/drm_crtc_helper.c	2011-11-21 11:26:14.000000000 +0800
@@ -903,8 +903,11 @@ static void output_poll_execute(struct w
 			      connector->base.id,
 			      drm_get_connector_name(connector),
 			      old_status, connector->status);
-		if (old_status != connector->status)
+		if (old_status != connector->status) {
 			changed = true;
+			connector->funcs->hotplug(connector);
+		}
+
 	}
 
 	mutex_unlock(&dev->mode_config.mutex);
--- linux.orig/include/drm/drm_crtc.h	2011-11-21 11:26:09.000000000 +0800
+++ linux/include/drm/drm_crtc.h	2011-11-21 11:26:14.000000000 +0800
@@ -419,6 +419,7 @@ struct drm_connector_funcs {
 	int (*fill_modes)(struct drm_connector *connector, uint32_t max_width, uint32_t max_height);
 	int (*set_property)(struct drm_connector *connector, struct drm_property *property,
 			     uint64_t val);
+	void (*hotplug)(struct drm_connector *connector);
 	void (*destroy)(struct drm_connector *connector);
 	void (*force)(struct drm_connector *connector);
 };
--- linux.orig/drivers/gpu/drm/i915/intel_modes.c	2011-11-21 11:26:09.000000000 +0800
+++ linux/drivers/gpu/drm/i915/intel_modes.c	2011-11-21 11:26:14.000000000 +0800
@@ -26,7 +26,6 @@
 #include <linux/slab.h>
 #include <linux/i2c.h>
 #include <linux/fb.h>
-#include <drm/drm_edid.h>
 #include "drmP.h"
 #include "intel_drv.h"
 #include "i915_drv.h"
@@ -75,7 +74,6 @@ int intel_ddc_get_modes(struct drm_conne
 	if (edid) {
 		drm_mode_connector_update_edid_property(connector, edid);
 		ret = drm_add_edid_modes(connector, edid);
-		drm_edid_to_eld(connector, edid);
 		connector->display_info.raw_edid = NULL;
 		kfree(edid);
 	}

[PATCH 3/3 v3] drm/i915: hot plug/unplug notification to HDMI audio driver

[Wu Fengguang]

On monitor hot plug/unplug, update ELD and set/clear SDVO_AUDIO_ENABLE
or DP_AUDIO_OUTPUT_ENABLE accordingly.  So that the audio driver will
receive hot plug events and take action to refresh its device state and
ELD contents.

A new callback ->hotplug() is added to struct drm_connector_funcs which
will be called immediately after ->detect() knowing that the monitor is
either plugged or unplugged.

It's noticed that X may not call ->mode_set() at monitor hot plug, so it's
necessary to call drm_edid_to_eld() earlier at ->detect() time rather than
in intel_ddc_get_modes(), so that intel_write_eld() can see the new ELD
in ->hotplug.

The call sequence on hot plug is
(the difference part lies in ->mode_set vs ->hotplug)

- KMS console
	->detect
	  drm_edid_to_eld
	->mode_set
	  intel_write_eld
	  set SDVO_AUDIO_ENABLE/DP_AUDIO_OUTPUT_ENABLE
- X
	->detect
	  drm_edid_to_eld
	->hotplug
	  intel_write_eld
	  set SDVO_AUDIO_ENABLE/DP_AUDIO_OUTPUT_ENABLE

On hot remove, the call sequence is

	->hotplug
	  clear SDVO_AUDIO_ENABLE/DP_AUDIO_OUTPUT_ENABLE

cc: Wang Zhenyu <zhenyu.z.wang@intel.com>
Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
---
 drivers/gpu/drm/drm_crtc_helper.c  |    6 +++
 drivers/gpu/drm/i915/intel_dp.c    |   44 +++++++++++++++++++++------
 drivers/gpu/drm/i915/intel_hdmi.c  |   31 +++++++++++++++++++
 drivers/gpu/drm/i915/intel_modes.c |    2 -
 include/drm/drm_crtc.h             |    1 
 5 files changed, 72 insertions(+), 12 deletions(-)

Changes since v2:

Fix "kernel NULL pointer dereference" due to calling NULL ->hotplug.

--- linux.orig/drivers/gpu/drm/i915/intel_dp.c	2011-11-21 11:26:09.000000000 +0800
+++ linux/drivers/gpu/drm/i915/intel_dp.c	2011-11-21 14:12:21.000000000 +0800
@@ -28,6 +28,7 @@
 #include <linux/i2c.h>
 #include <linux/slab.h>
 #include <linux/export.h>
+#include <drm/drm_edid.h>
 #include "drmP.h"
 #include "drm.h"
 #include "drm_crtc.h"
@@ -1970,20 +1971,44 @@ intel_dp_detect(struct drm_connector *co
 	if (status != connector_status_connected)
 		return status;
 
-	if (intel_dp->force_audio) {
-		intel_dp->has_audio = intel_dp->force_audio > 0;
-	} else {
-		edid = intel_dp_get_edid(connector, &intel_dp->adapter);
-		if (edid) {
-			intel_dp->has_audio = drm_detect_monitor_audio(edid);
-			connector->display_info.raw_edid = NULL;
-			kfree(edid);
-		}
+	edid = intel_dp_get_edid(connector, &intel_dp->adapter);
+	if (edid) {
+		intel_dp->has_audio = drm_detect_monitor_audio(edid);
+		drm_edid_to_eld(connector, edid);
+		connector->display_info.raw_edid = NULL;
+		kfree(edid);
 	}
 
+	if (intel_dp->force_audio)
+		intel_dp->has_audio = intel_dp->force_audio > 0;
+
 	return connector_status_connected;
 }
 
+static void intel_dp_hotplug(struct drm_connector *connector)
+{
+	struct intel_dp *intel_dp = intel_attached_dp(connector);
+	struct drm_device *dev = intel_dp->base.base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_crtc *crtc = intel_dp->base.base.crtc;
+
+	DRM_DEBUG_DRIVER("DisplayPort hotplug status %d crtc %p eld %#x\n",
+			 connector->status,
+			 crtc,
+			 (unsigned int)connector->eld[0]);
+
+	if (connector->status == connector_status_disconnected) {
+		intel_dp->DP &= ~DP_AUDIO_OUTPUT_ENABLE;
+	} else if (connector->status == connector_status_connected && crtc) {
+		intel_write_eld(&intel_dp->base.base, &crtc->mode);
+		intel_dp->DP |= DP_AUDIO_OUTPUT_ENABLE;
+	} else {
+		return;
+	}
+	I915_WRITE(intel_dp->output_reg, intel_dp->DP);
+	POSTING_READ(intel_dp->output_reg);
+}
+
 static int intel_dp_get_modes(struct drm_connector *connector)
 {
 	struct intel_dp *intel_dp = intel_attached_dp(connector);
@@ -2143,6 +2168,7 @@ static const struct drm_connector_funcs 
 	.detect = intel_dp_detect,
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.set_property = intel_dp_set_property,
+	.hotplug = intel_dp_hotplug,
 	.destroy = intel_dp_destroy,
 };
 
--- linux.orig/drivers/gpu/drm/i915/intel_hdmi.c	2011-11-21 11:26:09.000000000 +0800
+++ linux/drivers/gpu/drm/i915/intel_hdmi.c	2011-11-21 14:12:26.000000000 +0800
@@ -337,6 +337,7 @@ intel_hdmi_detect(struct drm_connector *
 			status = connector_status_connected;
 			intel_hdmi->has_hdmi_sink = drm_detect_hdmi_monitor(edid);
 			intel_hdmi->has_audio = drm_detect_monitor_audio(edid);
+			drm_edid_to_eld(connector, edid);
 		}
 		connector->display_info.raw_edid = NULL;
 		kfree(edid);
@@ -350,6 +351,35 @@ intel_hdmi_detect(struct drm_connector *
 	return status;
 }
 
+static void intel_hdmi_hotplug(struct drm_connector *connector)
+{
+	struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
+	struct drm_i915_private *dev_priv = connector->dev->dev_private;
+	struct drm_crtc *crtc = intel_hdmi->base.base.crtc;
+	u32 temp;
+
+	DRM_DEBUG_DRIVER("HDMI hotplug status %d crtc %p eld %#x\n",
+			 connector->status,
+			 crtc,
+			 (unsigned int)connector->eld[0]);
+
+	if (connector->status == connector_status_disconnected) {
+		temp = I915_READ(intel_hdmi->sdvox_reg) & ~SDVO_AUDIO_ENABLE;
+	} else if (connector->status == connector_status_connected && crtc) {
+		/*
+		 * It's for X only, which may not call ->mode_set on hot plug.
+		 * KMS console has NULL crtc at this time and will
+		 * call ->mode_set to enable HDMI audio a bit later.
+		 */
+		intel_write_eld(&intel_hdmi->base.base, &crtc->mode);
+		temp = I915_READ(intel_hdmi->sdvox_reg) | SDVO_AUDIO_ENABLE;
+	} else {
+		return;
+	}
+	I915_WRITE(intel_hdmi->sdvox_reg, temp);
+	POSTING_READ(intel_hdmi->sdvox_reg);
+}
+
 static int intel_hdmi_get_modes(struct drm_connector *connector)
 {
 	struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
@@ -459,6 +489,7 @@ static const struct drm_connector_funcs 
 	.detect = intel_hdmi_detect,
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.set_property = intel_hdmi_set_property,
+	.hotplug = intel_hdmi_hotplug,
 	.destroy = intel_hdmi_destroy,
 };
 
--- linux.orig/drivers/gpu/drm/drm_crtc_helper.c	2011-11-21 11:26:09.000000000 +0800
+++ linux/drivers/gpu/drm/drm_crtc_helper.c	2011-11-21 16:19:11.000000000 +0800
@@ -903,8 +903,12 @@ static void output_poll_execute(struct w
 			      connector->base.id,
 			      drm_get_connector_name(connector),
 			      old_status, connector->status);
-		if (old_status != connector->status)
+		if (old_status != connector->status) {
 			changed = true;
+			if (connector->funcs->hotplug)
+				connector->funcs->hotplug(connector);
+		}
+
 	}
 
 	mutex_unlock(&dev->mode_config.mutex);
--- linux.orig/include/drm/drm_crtc.h	2011-11-21 11:26:09.000000000 +0800
+++ linux/include/drm/drm_crtc.h	2011-11-21 11:26:14.000000000 +0800
@@ -419,6 +419,7 @@ struct drm_connector_funcs {
 	int (*fill_modes)(struct drm_connector *connector, uint32_t max_width, uint32_t max_height);
 	int (*set_property)(struct drm_connector *connector, struct drm_property *property,
 			     uint64_t val);
+	void (*hotplug)(struct drm_connector *connector);
 	void (*destroy)(struct drm_connector *connector);
 	void (*force)(struct drm_connector *connector);
 };
--- linux.orig/drivers/gpu/drm/i915/intel_modes.c	2011-11-21 11:26:09.000000000 +0800
+++ linux/drivers/gpu/drm/i915/intel_modes.c	2011-11-21 11:26:14.000000000 +0800
@@ -26,7 +26,6 @@
 #include <linux/slab.h>
 #include <linux/i2c.h>
 #include <linux/fb.h>
-#include <drm/drm_edid.h>
 #include "drmP.h"
 #include "intel_drv.h"
 #include "i915_drv.h"
@@ -75,7 +74,6 @@ int intel_ddc_get_modes(struct drm_conne
 	if (edid) {
 		drm_mode_connector_update_edid_property(connector, edid);
 		ret = drm_add_edid_modes(connector, edid);
-		drm_edid_to_eld(connector, edid);
 		connector->display_info.raw_edid = NULL;
 		kfree(edid);
 	}

Re: [PATCH 3/3 v2] drm/i915: hot plug/unplug notification to HDMI audio driver

[Keith Packard]

[-- Attachment #1: Type: text/plain, Size: 1082 bytes --]

On Mon, 21 Nov 2011 14:37:49 +0800, Wu Fengguang <fengguang.wu@intel.com> wrote:
> On monitor hot plug/unplug, update ELD and set/clear SDVO_AUDIO_ENABLE
> or DP_AUDIO_OUTPUT_ENABLE accordingly.  So that the audio driver will
> receive hot plug events and take action to refresh its device state and
> ELD contents.
> 
> A new callback ->hotplug() is added to struct drm_connector_funcs which
> will be called immediately after ->detect() knowing that the monitor is
> either plugged or unplugged.
> 
> It's noticed that X may not call ->mode_set() at monitor hot plug, so it's
> necessary to call drm_edid_to_eld() earlier at ->detect() time rather than
> in intel_ddc_get_modes(), so that intel_write_eld() can see the new ELD
> in ->hotplug.

The X environment will eventually call mode_set when the user
environment decides to use the monitor. Any audio bits can, and should,
await the user's choice of a video mode before choosing the audio format
to use. We should not be adding eld information until the monitor is in
use.

-- 
keith.packard@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 827 bytes --]

Re: [PATCH 3/3 v2] drm/i915: hot plug/unplug notification to HDMI audio driver

[Wu Fengguang]

On Tue, Nov 22, 2011 at 12:55:43AM +0800, Keith Packard wrote:
> On Mon, 21 Nov 2011 14:37:49 +0800, Wu Fengguang <fengguang.wu@intel.com> wrote:
> > On monitor hot plug/unplug, update ELD and set/clear SDVO_AUDIO_ENABLE
> > or DP_AUDIO_OUTPUT_ENABLE accordingly.  So that the audio driver will
> > receive hot plug events and take action to refresh its device state and
> > ELD contents.
> > 
> > A new callback ->hotplug() is added to struct drm_connector_funcs which
> > will be called immediately after ->detect() knowing that the monitor is
> > either plugged or unplugged.
> > 
> > It's noticed that X may not call ->mode_set() at monitor hot plug, so it's
> > necessary to call drm_edid_to_eld() earlier at ->detect() time rather than
> > in intel_ddc_get_modes(), so that intel_write_eld() can see the new ELD
> > in ->hotplug.
> 
> The X environment will eventually call mode_set when the user
> environment decides to use the monitor. Any audio bits can, and should,
> await the user's choice of a video mode before choosing the audio format
> to use. We should not be adding eld information until the monitor is in
> use.

Yeah. I just tested the full gnome desktop and find it behave like the
KMS console:

1) ->mode_set will be called
2) crtc is 0 in intel_hdmi_hotplug(), hence skipping the ELD code there

So the v3 patch will behave equally well on KMS console, gnome desktop
and bare X. Shall we just use it, or go back and use the original
->hot_remove patch?

Thanks,
Fengguang

Re: [PATCH 3/3 v2] drm/i915: hot plug/unplug notification to HDMI audio driver

[Wu Fengguang]

> > The X environment will eventually call mode_set when the user
> > environment decides to use the monitor. Any audio bits can, and should,
> > await the user's choice of a video mode before choosing the audio format
> > to use. We should not be adding eld information until the monitor is in
> > use.
> 
> Yeah. I just tested the full gnome desktop and find it behave like the
> KMS console:
> 
> 1) ->mode_set will be called
> 2) crtc is 0 in intel_hdmi_hotplug(), hence skipping the ELD code there
> 
> So the v3 patch will behave equally well on KMS console, gnome desktop
> and bare X. Shall we just use it, or go back and use the original
> ->hot_remove patch?

Here is the patch with updated comments and changelog to reflect the
new findings.

---
Subject: drm/i915: hot plug/unplug notification to HDMI audio driver
Date: Fri Nov 11 13:49:04 CST 2011

On monitor hot plug/unplug, update ELD and set/clear SDVO_AUDIO_ENABLE
or DP_AUDIO_OUTPUT_ENABLE accordingly.  So that the audio driver will
receive hot plug events and take action to refresh its device state and
ELD contents.

A new callback ->hotplug() is added to struct drm_connector_funcs which
will be called immediately after ->detect() knowing that the monitor is
either plugged or unplugged.

It's noticed that X may not call ->mode_set() at monitor hot plug, so it's
necessary to call drm_edid_to_eld() earlier at ->detect() time rather than
in intel_ddc_get_modes(), so that intel_write_eld() can see the new ELD
in ->hotplug.

The call sequence on hot plug is
(the difference part lies in ->mode_set vs ->hotplug)

- KMS console
	->detect
	  drm_edid_to_eld
	->mode_set
	  intel_write_eld
	  set SDVO_AUDIO_ENABLE/DP_AUDIO_OUTPUT_ENABLE
- X
	->detect
	  drm_edid_to_eld
	->hotplug
	  intel_write_eld
	  set SDVO_AUDIO_ENABLE/DP_AUDIO_OUTPUT_ENABLE

On hot remove, the call sequence is

	->hotplug
	  clear SDVO_AUDIO_ENABLE/DP_AUDIO_OUTPUT_ENABLE

cc: Wang Zhenyu <zhenyu.z.wang@intel.com>
Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
---
 drivers/gpu/drm/drm_crtc_helper.c  |    6 +++
 drivers/gpu/drm/i915/intel_dp.c    |   44 +++++++++++++++++++++------
 drivers/gpu/drm/i915/intel_hdmi.c  |   31 +++++++++++++++++++
 drivers/gpu/drm/i915/intel_modes.c |    2 -
 include/drm/drm_crtc.h             |    1 
 5 files changed, 72 insertions(+), 12 deletions(-)

--- linux.orig/drivers/gpu/drm/i915/intel_dp.c	2011-11-21 11:26:09.000000000 +0800
+++ linux/drivers/gpu/drm/i915/intel_dp.c	2011-11-21 14:12:21.000000000 +0800
@@ -28,6 +28,7 @@
 #include <linux/i2c.h>
 #include <linux/slab.h>
 #include <linux/export.h>
+#include <drm/drm_edid.h>
 #include "drmP.h"
 #include "drm.h"
 #include "drm_crtc.h"
@@ -1970,20 +1971,44 @@ intel_dp_detect(struct drm_connector *co
 	if (status != connector_status_connected)
 		return status;
 
-	if (intel_dp->force_audio) {
-		intel_dp->has_audio = intel_dp->force_audio > 0;
-	} else {
-		edid = intel_dp_get_edid(connector, &intel_dp->adapter);
-		if (edid) {
-			intel_dp->has_audio = drm_detect_monitor_audio(edid);
-			connector->display_info.raw_edid = NULL;
-			kfree(edid);
-		}
+	edid = intel_dp_get_edid(connector, &intel_dp->adapter);
+	if (edid) {
+		intel_dp->has_audio = drm_detect_monitor_audio(edid);
+		drm_edid_to_eld(connector, edid);
+		connector->display_info.raw_edid = NULL;
+		kfree(edid);
 	}
 
+	if (intel_dp->force_audio)
+		intel_dp->has_audio = intel_dp->force_audio > 0;
+
 	return connector_status_connected;
 }
 
+static void intel_dp_hotplug(struct drm_connector *connector)
+{
+	struct intel_dp *intel_dp = intel_attached_dp(connector);
+	struct drm_device *dev = intel_dp->base.base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_crtc *crtc = intel_dp->base.base.crtc;
+
+	DRM_DEBUG_DRIVER("DisplayPort hotplug status %d crtc %p eld %#x\n",
+			 connector->status,
+			 crtc,
+			 (unsigned int)connector->eld[0]);
+
+	if (connector->status == connector_status_disconnected) {
+		intel_dp->DP &= ~DP_AUDIO_OUTPUT_ENABLE;
+	} else if (connector->status == connector_status_connected && crtc) {
+		intel_write_eld(&intel_dp->base.base, &crtc->mode);
+		intel_dp->DP |= DP_AUDIO_OUTPUT_ENABLE;
+	} else {
+		return;
+	}
+	I915_WRITE(intel_dp->output_reg, intel_dp->DP);
+	POSTING_READ(intel_dp->output_reg);
+}
+
 static int intel_dp_get_modes(struct drm_connector *connector)
 {
 	struct intel_dp *intel_dp = intel_attached_dp(connector);
@@ -2143,6 +2168,7 @@ static const struct drm_connector_funcs 
 	.detect = intel_dp_detect,
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.set_property = intel_dp_set_property,
+	.hotplug = intel_dp_hotplug,
 	.destroy = intel_dp_destroy,
 };
 
--- linux.orig/drivers/gpu/drm/i915/intel_hdmi.c	2011-11-21 11:26:09.000000000 +0800
+++ linux/drivers/gpu/drm/i915/intel_hdmi.c	2011-11-21 14:12:26.000000000 +0800
@@ -337,6 +337,7 @@ intel_hdmi_detect(struct drm_connector *
 			status = connector_status_connected;
 			intel_hdmi->has_hdmi_sink = drm_detect_hdmi_monitor(edid);
 			intel_hdmi->has_audio = drm_detect_monitor_audio(edid);
+			drm_edid_to_eld(connector, edid);
 		}
 		connector->display_info.raw_edid = NULL;
 		kfree(edid);
@@ -350,6 +351,35 @@ intel_hdmi_detect(struct drm_connector *
 	return status;
 }
 
+static void intel_hdmi_hotplug(struct drm_connector *connector)
+{
+	struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
+	struct drm_i915_private *dev_priv = connector->dev->dev_private;
+	struct drm_crtc *crtc = intel_hdmi->base.base.crtc;
+	u32 temp;
+
+	DRM_DEBUG_DRIVER("HDMI hotplug status %d crtc %p eld %#x\n",
+			 connector->status,
+			 crtc,
+			 (unsigned int)connector->eld[0]);
+
+	if (connector->status == connector_status_disconnected) {
+		temp = I915_READ(intel_hdmi->sdvox_reg) & ~SDVO_AUDIO_ENABLE;
+	} else if (connector->status == connector_status_connected && crtc) {
+		/*
+		 * It's for X only, which may not call ->mode_set on hot plug.
+		 * KMS console has NULL crtc at this time and will
+		 * call ->mode_set to enable HDMI audio a bit later.
+		 */
+		intel_write_eld(&intel_hdmi->base.base, &crtc->mode);
+		temp = I915_READ(intel_hdmi->sdvox_reg) | SDVO_AUDIO_ENABLE;
+	} else {
+		return;
+	}
+	I915_WRITE(intel_hdmi->sdvox_reg, temp);
+	POSTING_READ(intel_hdmi->sdvox_reg);
+}
+
 static int intel_hdmi_get_modes(struct drm_connector *connector)
 {
 	struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
@@ -459,6 +489,7 @@ static const struct drm_connector_funcs 
 	.detect = intel_hdmi_detect,
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.set_property = intel_hdmi_set_property,
+	.hotplug = intel_hdmi_hotplug,
 	.destroy = intel_hdmi_destroy,
 };
 
--- linux.orig/drivers/gpu/drm/drm_crtc_helper.c	2011-11-21 11:26:09.000000000 +0800
+++ linux/drivers/gpu/drm/drm_crtc_helper.c	2011-11-21 16:19:11.000000000 +0800
@@ -903,8 +903,12 @@ static void output_poll_execute(struct w
 			      connector->base.id,
 			      drm_get_connector_name(connector),
 			      old_status, connector->status);
-		if (old_status != connector->status)
+		if (old_status != connector->status) {
 			changed = true;
+			if (connector->funcs->hotplug)
+				connector->funcs->hotplug(connector);
+		}
+
 	}
 
 	mutex_unlock(&dev->mode_config.mutex);
--- linux.orig/include/drm/drm_crtc.h	2011-11-21 11:26:09.000000000 +0800
+++ linux/include/drm/drm_crtc.h	2011-11-21 11:26:14.000000000 +0800
@@ -419,6 +419,7 @@ struct drm_connector_funcs {
 	int (*fill_modes)(struct drm_connector *connector, uint32_t max_width, uint32_t max_height);
 	int (*set_property)(struct drm_connector *connector, struct drm_property *property,
 			     uint64_t val);
+	void (*hotplug)(struct drm_connector *connector);
 	void (*destroy)(struct drm_connector *connector);
 	void (*force)(struct drm_connector *connector);
 };
--- linux.orig/drivers/gpu/drm/i915/intel_modes.c	2011-11-21 11:26:09.000000000 +0800
+++ linux/drivers/gpu/drm/i915/intel_modes.c	2011-11-21 11:26:14.000000000 +0800
@@ -26,7 +26,6 @@
 #include <linux/slab.h>
 #include <linux/i2c.h>
 #include <linux/fb.h>
-#include <drm/drm_edid.h>
 #include "drmP.h"
 #include "intel_drv.h"
 #include "i915_drv.h"
@@ -75,7 +74,6 @@ int intel_ddc_get_modes(struct drm_conne
 	if (edid) {
 		drm_mode_connector_update_edid_property(connector, edid);
 		ret = drm_add_edid_modes(connector, edid);
-		drm_edid_to_eld(connector, edid);
 		connector->display_info.raw_edid = NULL;
 		kfree(edid);
 	}

Re: [PATCH 3/3 v2] drm/i915: hot plug/unplug notification to HDMI audio driver

[Keith Packard]

[-- Attachment #1: Type: text/plain, Size: 652 bytes --]

On Tue, 22 Nov 2011 15:40:55 +0800, Wu Fengguang <fengguang.wu@intel.com> wrote:

> So the v3 patch will behave equally well on KMS console, gnome desktop
> and bare X. Shall we just use it, or go back and use the original
> ->hot_remove patch?

I'm not sure why we need any change to the core DRM code. The HDMI and
DP drivers will be told when to turn the monitors on and off, and that's
the appropriate time to control whether audio is routed to them. Hotplug
is considered simply a notification to user space that something has
changed -- user space is in charge of configuring which outputs are in
use.

-- 
keith.packard@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 827 bytes --]

Re: [PATCH 3/3 v2] drm/i915: hot plug/unplug notification to HDMI audio driver

[Wu Fengguang]

On Wed, Nov 23, 2011 at 02:25:14AM +0800, Keith Packard wrote:
> On Tue, 22 Nov 2011 15:40:55 +0800, Wu Fengguang <fengguang.wu@intel.com> wrote:
> 
> > So the v3 patch will behave equally well on KMS console, gnome desktop
> > and bare X. Shall we just use it, or go back and use the original
> > ->hot_remove patch?
> 
> I'm not sure why we need any change to the core DRM code. The HDMI and
> DP drivers will be told when to turn the monitors on and off, and that's

Yeah, The DP driver will be notified via the intel_dp_hot_plug() hook
if I understand it right.

> the appropriate time to control whether audio is routed to them.

However ->hot_plug() is called too early to be useful for this case.

What I need is a hot plug hook that knows whether the monitor is
plugged or removed, which is only possible if the hook is called
after ->detect().

Or, we can avoid adding the ->hotplug() hook and just add the call
directly to intel_hdmi_detect() and intel_dp_detect().

The below patch does this and eliminates the DRM callback :-)

> Hotplug is considered simply a notification to user space that
> something has changed -- user space is in charge of configuring
> which outputs are in use.

Yeah, and here is another notification to the audio driver demanded by
the HD audio spec.

Thanks,
Fengguang
---
Subject: drm/i915: hot plug/remove notification to HDMI audio driver
Date: Fri Nov 11 13:49:04 CST 2011

On monitor hot plug/unplug, set/clear SDVO_AUDIO_ENABLE or
DP_AUDIO_OUTPUT_ENABLE accordingly, so that the audio driver will
receive hot plug events and take action to refresh its device state and
ELD contents.

After intel_dp_detect() knows whether the monitor is plugged or
removed, it will call intel_dp_hotplug_audio() to notify the audio
driver of the event.

It's noticed that bare metal X may not call ->mode_set() at monitor hot
plug, so it's necessary to call drm_edid_to_eld() earlier at ->detect()
time rather than in intel_ddc_get_modes(), so that intel_write_eld() can
see the new ELD in intel_dp_hotplug_audio().

The call sequence on hot plug is

- KMS console, full blown X (eg. gnome desktop)
	->detect
	  drm_edid_to_eld
	->mode_set
	  intel_write_eld
	  set SDVO_AUDIO_ENABLE/DP_AUDIO_OUTPUT_ENABLE
- bare metal X (eg. fluxbox)
	->detect
	  drm_edid_to_eld
	  intel_dp_hotplug_audio()
	    intel_write_eld
	    set SDVO_AUDIO_ENABLE/DP_AUDIO_OUTPUT_ENABLE

On hot remove, the call sequence is

	->detect
	  intel_dp_hotplug_audio()
	    clear SDVO_AUDIO_ENABLE/DP_AUDIO_OUTPUT_ENABLE

cc: Wang Zhenyu <zhenyu.z.wang@intel.com>
Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c    |   47 ++++++++++++++++++++-------
 drivers/gpu/drm/i915/intel_hdmi.c  |   32 ++++++++++++++++++
 drivers/gpu/drm/i915/intel_modes.c |    2 -
 3 files changed, 68 insertions(+), 13 deletions(-)

--- linux.orig/drivers/gpu/drm/i915/intel_dp.c	2011-11-23 15:16:10.000000000 +0800
+++ linux/drivers/gpu/drm/i915/intel_dp.c	2011-11-23 16:20:25.000000000 +0800
@@ -28,6 +28,7 @@
 #include <linux/i2c.h>
 #include <linux/slab.h>
 #include <linux/export.h>
+#include <drm/drm_edid.h>
 #include "drmP.h"
 #include "drm.h"
 #include "drm_crtc.h"
@@ -1940,6 +1941,27 @@ intel_dp_get_edid_modes(struct drm_conne
 	return ret;
 }
 
+static void intel_dp_hotplug_audio(struct drm_connector *connector,
+				   enum drm_connector_status status)
+{
+	struct intel_dp *intel_dp = intel_attached_dp(connector);
+	struct drm_device *dev = intel_dp->base.base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_crtc *crtc = intel_dp->base.base.crtc;
+
+	DRM_DEBUG_DRIVER("hotplug status %d crtc %p eld %#x\n",
+			 status, crtc, (unsigned int)connector->eld[0]);
+
+	if (status == connector_status_disconnected)
+		intel_dp->DP &= ~DP_AUDIO_OUTPUT_ENABLE;
+	else if (status == connector_status_connected && crtc) {
+		intel_write_eld(&intel_dp->base.base, &crtc->mode);
+		intel_dp->DP |= DP_AUDIO_OUTPUT_ENABLE;
+	} else
+		return;
+	I915_WRITE(intel_dp->output_reg, intel_dp->DP);
+	POSTING_READ(intel_dp->output_reg);
+}
 
 /**
  * Uses CRT_HOTPLUG_EN and CRT_HOTPLUG_STAT to detect DP connection.
@@ -1968,20 +1990,23 @@ intel_dp_detect(struct drm_connector *co
 		      intel_dp->dpcd[6], intel_dp->dpcd[7]);
 
 	if (status != connector_status_connected)
-		return status;
+		goto out;
 
-	if (intel_dp->force_audio) {
-		intel_dp->has_audio = intel_dp->force_audio > 0;
-	} else {
-		edid = intel_dp_get_edid(connector, &intel_dp->adapter);
-		if (edid) {
-			intel_dp->has_audio = drm_detect_monitor_audio(edid);
-			connector->display_info.raw_edid = NULL;
-			kfree(edid);
-		}
+	edid = intel_dp_get_edid(connector, &intel_dp->adapter);
+	if (edid) {
+		intel_dp->has_audio = drm_detect_monitor_audio(edid);
+		drm_edid_to_eld(connector, edid);
+		connector->display_info.raw_edid = NULL;
+		kfree(edid);
 	}
 
-	return connector_status_connected;
+	if (intel_dp->force_audio)
+		intel_dp->has_audio = intel_dp->force_audio > 0;
+out:
+	if (status != connector->status)
+		intel_dp_hotplug_audio(connector, status);
+
+	return status;
 }
 
 static int intel_dp_get_modes(struct drm_connector *connector)
--- linux.orig/drivers/gpu/drm/i915/intel_hdmi.c	2011-11-23 15:16:10.000000000 +0800
+++ linux/drivers/gpu/drm/i915/intel_hdmi.c	2011-11-23 16:20:25.000000000 +0800
@@ -319,6 +319,34 @@ static bool intel_hdmi_mode_fixup(struct
 	return true;
 }
 
+static void intel_hdmi_hotplug_audio(struct drm_connector *connector,
+				     enum drm_connector_status status)
+{
+	struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
+	struct drm_i915_private *dev_priv = connector->dev->dev_private;
+	struct drm_crtc *crtc = intel_hdmi->base.base.crtc;
+	u32 temp;
+
+	DRM_DEBUG_DRIVER("hotplug status %d crtc %p eld %#x\n",
+			 status, crtc, (unsigned int)connector->eld[0]);
+
+	if (status == connector_status_disconnected)
+		temp = I915_READ(intel_hdmi->sdvox_reg) & ~SDVO_AUDIO_ENABLE;
+	else if (status == connector_status_connected && crtc) {
+		/*
+		 * It's for bare metal X (eg. fluxbox) only, which may not call
+		 * ->mode_set on hot plug.  KMS console and full blown X (eg.
+		 * gnome desktop) will see NULL crtc at this time and call
+		 * ->mode_set to enable HDMI audio a bit later.
+		 */
+		intel_write_eld(&intel_hdmi->base.base, &crtc->mode);
+		temp = I915_READ(intel_hdmi->sdvox_reg) | SDVO_AUDIO_ENABLE;
+	} else
+		return;
+	I915_WRITE(intel_hdmi->sdvox_reg, temp);
+	POSTING_READ(intel_hdmi->sdvox_reg);
+}
+
 static enum drm_connector_status
 intel_hdmi_detect(struct drm_connector *connector, bool force)
 {
@@ -337,6 +365,7 @@ intel_hdmi_detect(struct drm_connector *
 			status = connector_status_connected;
 			intel_hdmi->has_hdmi_sink = drm_detect_hdmi_monitor(edid);
 			intel_hdmi->has_audio = drm_detect_monitor_audio(edid);
+			drm_edid_to_eld(connector, edid);
 		}
 		connector->display_info.raw_edid = NULL;
 		kfree(edid);
@@ -347,6 +376,9 @@ intel_hdmi_detect(struct drm_connector *
 			intel_hdmi->has_audio = intel_hdmi->force_audio > 0;
 	}
 
+	if (status != connector->status)
+		intel_hdmi_hotplug_audio(connector, status);
+
 	return status;
 }
 
--- linux.orig/drivers/gpu/drm/i915/intel_modes.c	2011-11-23 15:16:10.000000000 +0800
+++ linux/drivers/gpu/drm/i915/intel_modes.c	2011-11-23 15:16:11.000000000 +0800
@@ -26,7 +26,6 @@
 #include <linux/slab.h>
 #include <linux/i2c.h>
 #include <linux/fb.h>
-#include <drm/drm_edid.h>
 #include "drmP.h"
 #include "intel_drv.h"
 #include "i915_drv.h"
@@ -75,7 +74,6 @@ int intel_ddc_get_modes(struct drm_conne
 	if (edid) {
 		drm_mode_connector_update_edid_property(connector, edid);
 		ret = drm_add_edid_modes(connector, edid);
-		drm_edid_to_eld(connector, edid);
 		connector->display_info.raw_edid = NULL;
 		kfree(edid);
 	}

Re: [PATCH 3/3 v2] drm/i915: hot plug/unplug notification to HDMI audio driver

[Keith Packard]

[-- Attachment #1: Type: text/plain, Size: 571 bytes --]

On Wed, 23 Nov 2011 16:29:58 +0800, Wu Fengguang <fengguang.wu@intel.com> wrote:

> What I need is a hot plug hook that knows whether the monitor is
> plugged or removed, which is only possible if the hook is called
> after ->detect().

That would be mode_set to tell you that the monitor is in use, and the
disable function to tell you when the monitor is no longer in use.

You do not want to do anything to the hardware in the hot_plug paths;
those are strictly informative; telling user space which connectors are
present.

-- 
keith.packard@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 827 bytes --]

Re: [PATCH 3/3 v2] drm/i915: hot plug/unplug notification to HDMI audio driver

[Wu Fengguang]

On Thu, Nov 24, 2011 at 03:26:49AM +0800, Keith Packard wrote:
> On Wed, 23 Nov 2011 16:29:58 +0800, Wu Fengguang <fengguang.wu@intel.com> wrote:
> 
> > What I need is a hot plug hook that knows whether the monitor is
> > plugged or removed, which is only possible if the hook is called
> > after ->detect().
> 
> That would be mode_set to tell you that the monitor is in use, and the
> disable function to tell you when the monitor is no longer in use.
> 
> You do not want to do anything to the hardware in the hot_plug paths;
> those are strictly informative; telling user space which connectors are
> present.

Thanks a lot for the tips! When doing things in the right path, I got
a much reduced patch :-)

Due to DP being a bit more tricky than HDMI and no convenient DP test
environment, I'll have to delay the DP part to next week...

Thanks,
Fengguang
---
Subject: drm/i915: HDMI hot remove notification to audio driver
Date: Fri Nov 11 13:49:04 CST 2011

On HDMI monitor hot remove, clear SDVO_AUDIO_ENABLE accordingly, so that
the audio driver will receive hot plug events and take action to refresh
its device state and ELD contents.

The cleared SDVO_AUDIO_ENABLE bit needs to be restored to prevent losing
HDMI audio after DPMS on.

CC: Wang Zhenyu <zhenyu.z.wang@intel.com>
Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c   |    4 +++-
 drivers/gpu/drm/i915/intel_hdmi.c |    8 ++++++--
 2 files changed, 9 insertions(+), 3 deletions(-)

--- linux.orig/drivers/gpu/drm/i915/intel_hdmi.c	2011-11-24 17:11:38.000000000 +0800
+++ linux/drivers/gpu/drm/i915/intel_hdmi.c	2011-11-24 17:15:03.000000000 +0800
@@ -269,6 +269,10 @@ static void intel_hdmi_dpms(struct drm_e
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
 	u32 temp;
+	u32 enable_bits = SDVO_ENABLE;
+
+	if (intel_hdmi->has_audio)
+		enable_bits |= SDVO_AUDIO_ENABLE;
 
 	temp = I915_READ(intel_hdmi->sdvox_reg);
 
@@ -281,9 +285,9 @@ static void intel_hdmi_dpms(struct drm_e
 	}
 
 	if (mode != DRM_MODE_DPMS_ON) {
-		temp &= ~SDVO_ENABLE;
+		temp &= ~enable_bits;
 	} else {
-		temp |= SDVO_ENABLE;
+		temp |= enable_bits;
 	}
 
 	I915_WRITE(intel_hdmi->sdvox_reg, temp);