On Wed, Feb 08, 2012 at 10:48:20AM +0800, Wu Fengguang wrote:
> When HDMI-DVI converter is used, it's not only necessary to turn off
> audio, but also to disable HDMI_MODE_SELECT and video infoframe. Since
> the DVI mode is mainly tied to audio functionality from end user POV,
> add a new "force-dvi" audio mode:
>
> xrandr --output HDMI1 --set audio force-dvi
>
> Note that most users won't need to set this and happily rely on the
> EDID based DVI auto detection.
>
> Reported-by: Andrea Arcangeli <aarcange at redhat.com>
> Signed-off-by: Wu Fengguang <fengguang.wu at intel.com>
Jesse acked this patch on irc, but I've noticed a few little things. See
below for the details.
-Daniel
> ---
> drivers/gpu/drm/i915/i915_drv.h | 7 +++++++
> drivers/gpu/drm/i915/intel_hdmi.c | 8 +++++---
> drivers/gpu/drm/i915/intel_modes.c | 4 +++-
> 3 files changed, 15 insertions(+), 4 deletions(-)
>
> --- linux-next.orig/drivers/gpu/drm/i915/i915_drv.h 2012-01-31 20:44:59.000000000 +0800
> +++ linux-next/drivers/gpu/drm/i915/i915_drv.h 2012-02-08 10:37:30.000000000 +0800
> @@ -749,6 +749,13 @@ typedef struct drm_i915_private {
> struct drm_property *force_audio_property;
> } drm_i915_private_t;
>
> +enum hdmi_force_audio {
> + HDMI_AUDIO_OFF_DVI = -2, /* no aux data for HDMI-DVI converter */
> + HDMI_AUDIO_OFF, /* force turn off HDMI audio */
> + HDMI_AUDIO_AUTO, /* trust EDID */
> + HDMI_AUDIO_ON, /* force turn on HDMI audio */
> +};
> +
> enum i915_cache_level {
> I915_CACHE_NONE,
> I915_CACHE_LLC,
> --- linux-next.orig/drivers/gpu/drm/i915/intel_hdmi.c 2012-01-07 20:47:34.000000000 +0800
> +++ linux-next/drivers/gpu/drm/i915/intel_hdmi.c 2012-02-08 10:37:30.000000000 +0800
> @@ -339,7 +339,9 @@ intel_hdmi_detect(struct drm_connector *
> if (edid) {
> if (edid->input & DRM_EDID_INPUT_DIGITAL) {
> status = connector_status_connected;
> - intel_hdmi->has_hdmi_sink = drm_detect_hdmi_monitor(edid);
> + if (intel_hdmi->force_audio != HDMI_AUDIO_OFF_DVI)
> + intel_hdmi->has_hdmi_sink =
> + drm_detect_hdmi_monitor(edid);
> intel_hdmi->has_audio = drm_detect_monitor_audio(edid);
> }
> connector->display_info.raw_edid = NULL;
> @@ -415,8 +417,8 @@ intel_hdmi_set_property(struct drm_conne
> else
> has_audio = i > 0;
All these comparison with i use magic values instead of your new enum
definitions. Can you replace these with the enums like you've done below,
i.e. for the above if block:
if (i == HDMI_AUDIO_AUOT)
...
else if (i == HDMI_AUDIO_ON)
has_audio = true;
intel_hdmi_detect is also a place that checks these values (with
intel_hdmi->force_audio) - please use the enum values there, too. Also
please change the type of intel_hdmi->force_audio from int to the enum.
Another thing I've noticed is that in intel_hdmi_detect you don't force
has_hdmi_sink to false if the force_dvi option is set.
>
> - if (has_audio == intel_hdmi->has_audio)
> - return 0;
> + if (i == HDMI_AUDIO_OFF_DVI)
> + intel_hdmi->has_hdmi_sink = 0;
>
> intel_hdmi->has_audio = has_audio;
> goto done;
> --- linux-next.orig/drivers/gpu/drm/i915/intel_modes.c 2011-10-19 11:11:20.000000000 +0800
> +++ linux-next/drivers/gpu/drm/i915/intel_modes.c 2012-02-08 10:37:30.000000000 +0800
> @@ -84,6 +84,7 @@ int intel_ddc_get_modes(struct drm_conne
> }
>
> static const char *force_audio_names[] = {
> + "force-dvi",
> "off",
> "auto",
> "on",
> @@ -106,7 +107,8 @@ intel_attach_force_audio_property(struct
> return;
>
> for (i = 0; i < ARRAY_SIZE(force_audio_names); i++)
> - drm_property_add_enum(prop, i, i-1, force_audio_names[i]);
> + drm_property_add_enum(prop, i, i-2,
> + force_audio_names[i]);
>
> dev_priv->force_audio_property = prop;
> }
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Mail: daniel at ffwll.ch
Mobile: +41 (0)79 365 57 48
