drm/i915: add a "force-dvi" HDMI audio mode
Subsystems (2)
PATCH 1/1 drm/i915: add a "force-dvi" HDMI audio mode
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.
--- 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(-)
drivers/gpu/drm/i915/i915_drv.h +7 -0
@@ -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 ,
drivers/gpu/drm/i915/intel_hdmi.c +5 -3
@@ -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_conneelse has_audio = i >0 ; -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;
drivers/gpu/drm/i915/intel_modes.c +3 -1
@@ -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(structreturn ;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; }
Quoted 5 lines · Wu Fengguang
When HDMI-DVI converter is used, it's not only necessary to turn off audio, but also to disable
HDMI_MODE_SELECTand video infoframe. Since the DVI mode is mainly tied to audio functionality from end user POV, add a new "force-dvi" audio mode:
Quoted 7 lines
xrandr --output HDMI1 --set audio force-dviNote that most users won't need to set this and happily rely on the EDID based DVI auto detection.
Reported-by Andrea Arcangeli <[email protected]>Signed-off-by Wu Fengguang <[email protected]>
Jesse acked this patch on irc, but I've noticed a few little things. See below for the details.
-Daniel
Quoted 36 lines
--- 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(-)drivers/gpu/drm/i915/i915_drv.h +7
-0@@ -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 ,drivers/gpu/drm/i915/intel_hdmi.c +3
-1@@ -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 ;
drivers/gpu/drm/i915/intel_hdmi.c intel_hdmi_set_property(struct drm_conne · line 418
Quoted 2 lines
drivers/gpu/drm/i915/intel_hdmi.c +0
-0@@ -415,8 +417,8 @@ intel_hdmi_set_property(struct drm_conneelse 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.
Quoted 32 lines
drivers/gpu/drm/i915/intel_hdmi.c +2
-2-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;drivers/gpu/drm/i915/intel_modes.c +3
-1@@ -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(structreturn ;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
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 3 lines
Daniel Vetter Mail: [email protected] Mobile: +41 (0)79 365 57 48
Quoted 8 lines · Daniel Vetter
Quoted 7 lines · Wu Fengguang
When HDMI-DVI converter is used, it's not only necessary to turn off audio, but also to disable
HDMI_MODE_SELECTand 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
Quoted 8 lines
Quoted 5 lines
Note that most users won't need to set this and happily rely on the EDID based DVI auto detection.
Reported-by Andrea Arcangeli <[email protected]>Signed-off-by Wu Fengguang <[email protected]>Jesse acked this patch on irc, but I've noticed a few little things. See below for the details.
OK, thanks!
Quoted 21 lines
Quoted 14 lines
drivers/gpu/drm/i915/intel_hdmi.c +3
-1@@ -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 ;drivers/gpu/drm/i915/intel_hdmi.c intel_hdmi_set_property(struct drm_conne · line 418
Quoted 2 lines
drivers/gpu/drm/i915/intel_hdmi.c +0
-0@@ -415,8 +417,8 @@ intel_hdmi_set_property(struct drm_conneelse 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:
Quoted 4 lines
if (i == HDMI_AUDIO_AUOT) ... else if (i == HDMI_AUDIO_ON) has_audio = true;
Done.
Quoted 3 lines
intel_hdmi_detectis also a place that checks these values (withintel_hdmi->force_audio) - please use the enum values there, too. Also please change the type ofintel_hdmi->force_audiofrom int to the enum.
Done.
Quoted 2 lines
Another thing I've noticed is that in
intel_hdmi_detectyou don't forcehas_hdmi_sinkto false if theforce_dvioption is set.
It's implicitly initialized to false at the beginning. Isn't that enough?
Updated patch follows. Compile tested.
Thanks,
Fengguang
---
Subject: drm/i915: add an "force-dvi" HDMI audio mode
Date: Fri Jan 06 11:04:00 CST 2012
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.
--- drivers/gpu/drm/i915/i915_drv.h | 7 +++++++ drivers/gpu/drm/i915/intel_hdmi.c | 21 ++++++++++++--------- drivers/gpu/drm/i915/intel_modes.c | 4 +++- 3 files changed, 22 insertions(+), 10 deletions(-)
drivers/gpu/drm/i915/i915_drv.h +7 -0
@@ -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 ,
drivers/gpu/drm/i915/intel_hdmi.c +12 -9
@@ -44,7 +44,7 @@ struct intel_hdmi {uint32_t color_range;bool has_hdmi_sink;bool has_audio; -int force_audio; +enum hdmi_force_audio force_audio;void (*write_infoframe)(struct drm_encoder *encoder,struct dip_infoframe *frame); };
@@ -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 ;
@@ -347,8 +349,9 @@ intel_hdmi_detect(struct drm_connector * }if (status == connector_status_connected) { -if (intel_hdmi->force_audio) - intel_hdmi->has_audio = intel_hdmi->force_audio >0 ; +if (intel_hdmi->force_audio !=HDMI_AUDIO_AUTO ) + intel_hdmi->has_audio = + (intel_hdmi->force_audio ==HDMI_AUDIO_ON ); }return status;
@@ -402,7 +405,7 @@ intel_hdmi_set_property(struct drm_connereturn ret;if (property == dev_priv->force_audio_property) { -int i = val; +enum hdmi_force_audio i = val;bool has_audio;if (i == intel_hdmi->force_audio)
@@ -410,13 +413,13 @@ intel_hdmi_set_property(struct drm_conne intel_hdmi->force_audio = i; -if (i ==0 ) +if (i ==HDMI_AUDIO_AUTO ) has_audio =intel_hdmi_detect_audio (connector);else - has_audio = i >0 ; + has_audio = (i ==HDMI_AUDIO_ON ); -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;
drivers/gpu/drm/i915/intel_modes.c +3 -1
@@ -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(structreturn ;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; }
Daniel Vetter
AppliedQuoted 52 lines · Wu Fengguang
Quoted 8 lines · Daniel Vetter
Quoted 7 lines · Wu Fengguang
When HDMI-DVI converter is used, it's not only necessary to turn off audio, but also to disable
HDMI_MODE_SELECTand 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-dviQuoted 8 lines
Quoted 5 lines
Note that most users won't need to set this and happily rely on the EDID based DVI auto detection.
Reported-by Andrea Arcangeli <[email protected]>Signed-off-by Wu Fengguang <[email protected]>Jesse acked this patch on irc, but I've noticed a few little things. See below for the details.
OK, thanks!
Quoted 21 lines
Quoted 14 lines
drivers/gpu/drm/i915/intel_hdmi.c +3
-1@@ -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 ;drivers/gpu/drm/i915/intel_hdmi.c intel_hdmi_set_property(struct drm_conne · line 418
Quoted 2 lines
drivers/gpu/drm/i915/intel_hdmi.c +0
-0@@ -415,8 +417,8 @@ intel_hdmi_set_property(struct drm_conneelse 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:
Quoted 4 lines
if (i == HDMI_AUDIO_AUOT) ... else if (i == HDMI_AUDIO_ON) has_audio = true;Done.
Quoted 3 lines
intel_hdmi_detectis also a place that checks these values (withintel_hdmi->force_audio) - please use the enum values there, too. Also please change the type ofintel_hdmi->force_audiofrom int to the enum.
Quoted 6 lines
Done.
Quoted 2 lines
Another thing I've noticed is that in
intel_hdmi_detectyou don't forcehas_hdmi_sinkto false if theforce_dvioption is set.It's implicitly initialized to false at the beginning. Isn't that enough?
Yep, you're right, should be enough.
Quoted 1 line
Updated patch follows. Compile tested.
Applied. You owe me a beer when it blows up because it's only compile-tested ;-)
Cheers, Daniel
Signature · 3 lines
Daniel Vetter Mail: [email protected] Mobile: +41 (0)79 365 57 48