diff mbox series

[1/7] drm/vc4: hdmi: Check that the monitor supports HDMI audio

Message ID f0e4c1b92b2c1a98189fd891c9ce0f65031636b0.1551711042.git-series.maxime.ripard@bootlin.com (mailing list archive)
State New, archived
Headers show
Series drm/vc4: Allow for more boot-time configuration | expand

Commit Message

Maxime Ripard March 4, 2019, 2:52 p.m. UTC
The current code assumes as soon as the device is an HDMI one that it
supports an audio sink. However, strictly speaking, this is exposed as a
separate part of EDID.

This can be checked through the drm_detect_monitor_audio function, so let's
use it and make sure that we can use the HDMI monitor as an output before
sending sound.

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 drivers/gpu/drm/vc4/vc4_hdmi.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Paul Kocialkowski March 4, 2019, 3:10 p.m. UTC | #1
Hi,

On Mon, 2019-03-04 at 15:52 +0100, Maxime Ripard wrote:
> The current code assumes as soon as the device is an HDMI one that it
> supports an audio sink. However, strictly speaking, this is exposed as a
> separate part of EDID.
> 
> This can be checked through the drm_detect_monitor_audio function, so let's
> use it and make sure that we can use the HDMI monitor as an output before
> sending sound.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>

Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Cheers,

Paul

> ---
>  drivers/gpu/drm/vc4/vc4_hdmi.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index 88fd5df7e7dc..a1bdc065c47c 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -109,6 +109,7 @@ struct vc4_hdmi_encoder {
>  	struct vc4_encoder base;
>  	bool hdmi_monitor;
>  	bool limited_rgb_range;
> +	bool monitor_has_audio;
>  };
>  
>  static inline struct vc4_hdmi_encoder *
> @@ -278,6 +279,7 @@ static int vc4_hdmi_connector_get_modes(struct drm_connector *connector)
>  		return -ENODEV;
>  
>  	vc4_encoder->hdmi_monitor = drm_detect_hdmi_monitor(edid);
> +	vc4_encoder->monitor_has_audio = drm_detect_monitor_audio(edid);
>  
>  	drm_connector_update_edid_property(connector, edid);
>  	ret = drm_add_edid_modes(connector, edid);
> @@ -785,9 +787,13 @@ static int vc4_hdmi_audio_startup(struct snd_pcm_substream *substream,
>  {
>  	struct vc4_hdmi *hdmi = dai_to_hdmi(dai);
>  	struct drm_encoder *encoder = hdmi->encoder;
> +	struct vc4_hdmi_encoder *vc4_encoder = to_vc4_hdmi_encoder(encoder);
>  	struct vc4_dev *vc4 = to_vc4_dev(encoder->dev);
>  	int ret;
>  
> +	if (!vc4_encoder->monitor_has_audio)
> +		return -ENODEV;
> +
>  	if (hdmi->audio.substream && hdmi->audio.substream != substream)
>  		return -EINVAL;
>
Stefan Wahren March 4, 2019, 3:54 p.m. UTC | #2
Hi Maxime,

Am 04.03.2019 um 15:52 schrieb Maxime Ripard:
> The current code assumes as soon as the device is an HDMI one that it
> supports an audio sink. However, strictly speaking, this is exposed as a
> separate part of EDID.
>
> This can be checked through the drm_detect_monitor_audio function, so let's
> use it and make sure that we can use the HDMI monitor as an output before
> sending sound.

does the audio output work in the following setup after applying this patch?

VC4 --- HDMI Audio extractor --- Non audio capable monitor

>
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
Eric Anholt March 4, 2019, 6:28 p.m. UTC | #3
Stefan Wahren <stefan.wahren@i2se.com> writes:

> Hi Maxime,
>
> Am 04.03.2019 um 15:52 schrieb Maxime Ripard:
>> The current code assumes as soon as the device is an HDMI one that it
>> supports an audio sink. However, strictly speaking, this is exposed as a
>> separate part of EDID.
>>
>> This can be checked through the drm_detect_monitor_audio function, so let's
>> use it and make sure that we can use the HDMI monitor as an output before
>> sending sound.
>
> does the audio output work in the following setup after applying this patch?
>
> VC4 --- HDMI Audio extractor --- Non audio capable monitor

A 1-minute google of audio extractors says they manage the EDID.  Do you
have some reason to think this wouldn't work?
Stefan Wahren March 4, 2019, 8:06 p.m. UTC | #4
> Eric Anholt <eric@anholt.net> hat am 4. März 2019 um 19:28 geschrieben:
> 
> 
> Stefan Wahren <stefan.wahren@i2se.com> writes:
> 
> > Hi Maxime,
> >
> > Am 04.03.2019 um 15:52 schrieb Maxime Ripard:
> >> The current code assumes as soon as the device is an HDMI one that it
> >> supports an audio sink. However, strictly speaking, this is exposed as a
> >> separate part of EDID.
> >>
> >> This can be checked through the drm_detect_monitor_audio function, so let's
> >> use it and make sure that we can use the HDMI monitor as an output before
> >> sending sound.
> >
> > does the audio output work in the following setup after applying this patch?
> >
> > VC4 --- HDMI Audio extractor --- Non audio capable monitor
> 
> A 1-minute google of audio extractors says they manage the EDID.  Do you
> have some reason to think this wouldn't work?

My only concern is the existence of some audio extractors which doesn't care about EDID. I that case we need to provide some kind of force switch.
Eric Anholt March 4, 2019, 9:09 p.m. UTC | #5
Stefan Wahren <stefan.wahren@i2se.com> writes:

>> Eric Anholt <eric@anholt.net> hat am 4. März 2019 um 19:28 geschrieben:
>> 
>> 
>> Stefan Wahren <stefan.wahren@i2se.com> writes:
>> 
>> > Hi Maxime,
>> >
>> > Am 04.03.2019 um 15:52 schrieb Maxime Ripard:
>> >> The current code assumes as soon as the device is an HDMI one that it
>> >> supports an audio sink. However, strictly speaking, this is exposed as a
>> >> separate part of EDID.
>> >>
>> >> This can be checked through the drm_detect_monitor_audio function, so let's
>> >> use it and make sure that we can use the HDMI monitor as an output before
>> >> sending sound.
>> >
>> > does the audio output work in the following setup after applying this patch?
>> >
>> > VC4 --- HDMI Audio extractor --- Non audio capable monitor
>> 
>> A 1-minute google of audio extractors says they manage the EDID.  Do you
>> have some reason to think this wouldn't work?
>
> My only concern is the existence of some audio extractors which doesn't care about EDID. I that case we need to provide some kind of force switch.

I'm having a hard time imagining such a product existing, given that I
don't think Windows will have "ignore the EDID, force audio anyway"
switches in their UIs.
diff mbox series

Patch

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 88fd5df7e7dc..a1bdc065c47c 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -109,6 +109,7 @@  struct vc4_hdmi_encoder {
 	struct vc4_encoder base;
 	bool hdmi_monitor;
 	bool limited_rgb_range;
+	bool monitor_has_audio;
 };
 
 static inline struct vc4_hdmi_encoder *
@@ -278,6 +279,7 @@  static int vc4_hdmi_connector_get_modes(struct drm_connector *connector)
 		return -ENODEV;
 
 	vc4_encoder->hdmi_monitor = drm_detect_hdmi_monitor(edid);
+	vc4_encoder->monitor_has_audio = drm_detect_monitor_audio(edid);
 
 	drm_connector_update_edid_property(connector, edid);
 	ret = drm_add_edid_modes(connector, edid);
@@ -785,9 +787,13 @@  static int vc4_hdmi_audio_startup(struct snd_pcm_substream *substream,
 {
 	struct vc4_hdmi *hdmi = dai_to_hdmi(dai);
 	struct drm_encoder *encoder = hdmi->encoder;
+	struct vc4_hdmi_encoder *vc4_encoder = to_vc4_hdmi_encoder(encoder);
 	struct vc4_dev *vc4 = to_vc4_dev(encoder->dev);
 	int ret;
 
+	if (!vc4_encoder->monitor_has_audio)
+		return -ENODEV;
+
 	if (hdmi->audio.substream && hdmi->audio.substream != substream)
 		return -EINVAL;