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 at 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 at intel.com> Signed-off-by: Wu Fengguang <fengguang.wu at 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); }