Re: [alsa-devel] [PATCH 2/7] drm/edid: Allow to ignore the audio EDID data
On Tue, Mar 05, 2019 at 10:12:40AM +0100, Maxime Ripard wrote:
On Mon, Mar 04, 2019 at 03:05:31PM -0500, Alex Deucher wrote:
On Mon, Mar 4, 2019 at 2:53 PM Eric Anholt eric@anholt.net wrote:
Maxime Ripard maxime.ripard@bootlin.com writes:
In some cases, in order to accomodate with displays with poor EDIDs, we need to ignore that the monitor alledgedly supports audio output and disable the audio output.
Signed-off-by: Maxime Ripard maxime.ripard@bootlin.com
drivers/gpu/drm/drm_edid.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 990b1909f9d7..c0258b011bb2 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4190,6 +4190,11 @@ bool drm_detect_hdmi_monitor(struct edid *edid) } EXPORT_SYMBOL(drm_detect_hdmi_monitor);
+static bool ignore_edid_audio = false; +module_param(ignore_edid_audio, bool, 0644); +MODULE_PARM_DESC(ignore_edid_audio,
"Ignore the EDID and always consider that a monitor doesn't have audio capabilities");/**
- drm_detect_monitor_audio - check monitor audio capability
- @edid: EDID block to scan
@@ -4209,6 +4214,9 @@ bool drm_detect_monitor_audio(struct edid *edid) bool has_audio = false; int start_offset, end_offset;
if (ignore_edid_audio)goto end;edid_ext = drm_find_cea_extension(edid); if (!edid_ext) goto end;It looks like the motivation for the original flag on Raspberry Pi was "I've got a non-audio monitor, but the system comes up trying to play audio to HDMI instead of the analog jack". Do we have some way for DRM to communicate to ALSA that this is not the right place to try to play audio by default?
Apparently not. We have users using debug knobs in our drivers to disable display audio because ALSA defaults to that rather than other audio.
I guess one way to do this would be to register the card only when an audio-capable monitor is connected instead of doing this at probe time. I'm not sure how convenient it is for userspace though.
We already provide the ELD to alsa. I'm pretty sure pulseaudio uses that stuff somehow to figure out whether to play audio over HDMI. But since I don't use pulseaudio myself I can't be 100% sure.
Cc:ing Takashi/alsa folks for confirmation.
On Tue, Mar 05, 2019 at 05:24:13PM +0200, Ville Syrjälä wrote:
On Tue, Mar 05, 2019 at 10:12:40AM +0100, Maxime Ripard wrote:
On Mon, Mar 04, 2019 at 03:05:31PM -0500, Alex Deucher wrote:
On Mon, Mar 4, 2019 at 2:53 PM Eric Anholt eric@anholt.net wrote:
Maxime Ripard maxime.ripard@bootlin.com writes:
In some cases, in order to accomodate with displays with poor EDIDs, we need to ignore that the monitor alledgedly supports audio output and disable the audio output.
Signed-off-by: Maxime Ripard maxime.ripard@bootlin.com
drivers/gpu/drm/drm_edid.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 990b1909f9d7..c0258b011bb2 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4190,6 +4190,11 @@ bool drm_detect_hdmi_monitor(struct edid *edid) } EXPORT_SYMBOL(drm_detect_hdmi_monitor);
+static bool ignore_edid_audio = false; +module_param(ignore_edid_audio, bool, 0644); +MODULE_PARM_DESC(ignore_edid_audio,
"Ignore the EDID and always consider that a monitor doesn't have audio capabilities");/**
- drm_detect_monitor_audio - check monitor audio capability
- @edid: EDID block to scan
@@ -4209,6 +4214,9 @@ bool drm_detect_monitor_audio(struct edid *edid) bool has_audio = false; int start_offset, end_offset;
if (ignore_edid_audio)goto end;edid_ext = drm_find_cea_extension(edid); if (!edid_ext) goto end;It looks like the motivation for the original flag on Raspberry Pi was "I've got a non-audio monitor, but the system comes up trying to play audio to HDMI instead of the analog jack". Do we have some way for DRM to communicate to ALSA that this is not the right place to try to play audio by default?
Apparently not. We have users using debug knobs in our drivers to disable display audio because ALSA defaults to that rather than other audio.
I guess one way to do this would be to register the card only when an audio-capable monitor is connected instead of doing this at probe time. I'm not sure how convenient it is for userspace though.
We already provide the ELD to alsa. I'm pretty sure pulseaudio uses that stuff somehow to figure out whether to play audio over HDMI. But since I don't use pulseaudio myself I can't be 100% sure.
Cc:ing Takashi/alsa folks for confirmation.
I forgot that the .pin_eld_notify() stuff is i915 specific. But I see some kind of hdmi_codec_ops thing used by some other drivers. I guess that is supposed to achieve the same thing more or less? I'm not immediately seeing any kind of drm->alsa notification hook in there though.
On Tue, Mar 5, 2019 at 2:15 PM Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Tue, Mar 05, 2019 at 05:24:13PM +0200, Ville Syrjälä wrote:
On Tue, Mar 05, 2019 at 10:12:40AM +0100, Maxime Ripard wrote:
On Mon, Mar 04, 2019 at 03:05:31PM -0500, Alex Deucher wrote:
On Mon, Mar 4, 2019 at 2:53 PM Eric Anholt eric@anholt.net wrote:
Maxime Ripard maxime.ripard@bootlin.com writes:
In some cases, in order to accomodate with displays with poor EDIDs, we need to ignore that the monitor alledgedly supports audio output and disable the audio output.
Signed-off-by: Maxime Ripard maxime.ripard@bootlin.com
drivers/gpu/drm/drm_edid.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 990b1909f9d7..c0258b011bb2 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4190,6 +4190,11 @@ bool drm_detect_hdmi_monitor(struct edid *edid) } EXPORT_SYMBOL(drm_detect_hdmi_monitor);
+static bool ignore_edid_audio = false; +module_param(ignore_edid_audio, bool, 0644); +MODULE_PARM_DESC(ignore_edid_audio,
"Ignore the EDID and always consider that a monitor doesn't have audio capabilities");/**
- drm_detect_monitor_audio - check monitor audio capability
- @edid: EDID block to scan
@@ -4209,6 +4214,9 @@ bool drm_detect_monitor_audio(struct edid *edid) bool has_audio = false; int start_offset, end_offset;
if (ignore_edid_audio)goto end;edid_ext = drm_find_cea_extension(edid); if (!edid_ext) goto end;It looks like the motivation for the original flag on Raspberry Pi was "I've got a non-audio monitor, but the system comes up trying to play audio to HDMI instead of the analog jack". Do we have some way for DRM to communicate to ALSA that this is not the right place to try to play audio by default?
Apparently not. We have users using debug knobs in our drivers to disable display audio because ALSA defaults to that rather than other audio.
I guess one way to do this would be to register the card only when an audio-capable monitor is connected instead of doing this at probe time. I'm not sure how convenient it is for userspace though.
We already provide the ELD to alsa. I'm pretty sure pulseaudio uses that stuff somehow to figure out whether to play audio over HDMI. But since I don't use pulseaudio myself I can't be 100% sure.
Cc:ing Takashi/alsa folks for confirmation.
I forgot that the .pin_eld_notify() stuff is i915 specific. But I see some kind of hdmi_codec_ops thing used by some other drivers. I guess that is supposed to achieve the same thing more or less? I'm not immediately seeing any kind of drm->alsa notification hook in there though.
On AMD hw, the GPU has backdoor access to some of the audio state, so when stuff happens on the GPU side, it's reflected on the audio side automatically.
Alex
On Tue, Mar 05, 2019 at 02:21:04PM -0500, Alex Deucher wrote:
On Tue, Mar 5, 2019 at 2:15 PM Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Tue, Mar 05, 2019 at 05:24:13PM +0200, Ville Syrjälä wrote:
On Tue, Mar 05, 2019 at 10:12:40AM +0100, Maxime Ripard wrote:
On Mon, Mar 04, 2019 at 03:05:31PM -0500, Alex Deucher wrote:
On Mon, Mar 4, 2019 at 2:53 PM Eric Anholt eric@anholt.net wrote:
Maxime Ripard maxime.ripard@bootlin.com writes:
> In some cases, in order to accomodate with displays with poor EDIDs, we > need to ignore that the monitor alledgedly supports audio output and > disable the audio output. > > Signed-off-by: Maxime Ripard maxime.ripard@bootlin.com > --- > drivers/gpu/drm/drm_edid.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index 990b1909f9d7..c0258b011bb2 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -4190,6 +4190,11 @@ bool drm_detect_hdmi_monitor(struct edid *edid) > } > EXPORT_SYMBOL(drm_detect_hdmi_monitor); > > +static bool ignore_edid_audio = false; > +module_param(ignore_edid_audio, bool, 0644); > +MODULE_PARM_DESC(ignore_edid_audio, > + "Ignore the EDID and always consider that a monitor doesn't have audio capabilities"); > + > /** > * drm_detect_monitor_audio - check monitor audio capability > * @edid: EDID block to scan > @@ -4209,6 +4214,9 @@ bool drm_detect_monitor_audio(struct edid *edid) > bool has_audio = false; > int start_offset, end_offset; > > + if (ignore_edid_audio) > + goto end; > + > edid_ext = drm_find_cea_extension(edid); > if (!edid_ext) > goto end;
It looks like the motivation for the original flag on Raspberry Pi was "I've got a non-audio monitor, but the system comes up trying to play audio to HDMI instead of the analog jack". Do we have some way for DRM to communicate to ALSA that this is not the right place to try to play audio by default?
Apparently not. We have users using debug knobs in our drivers to disable display audio because ALSA defaults to that rather than other audio.
I guess one way to do this would be to register the card only when an audio-capable monitor is connected instead of doing this at probe time. I'm not sure how convenient it is for userspace though.
We already provide the ELD to alsa. I'm pretty sure pulseaudio uses that stuff somehow to figure out whether to play audio over HDMI. But since I don't use pulseaudio myself I can't be 100% sure.
Cc:ing Takashi/alsa folks for confirmation.
I forgot that the .pin_eld_notify() stuff is i915 specific. But I see some kind of hdmi_codec_ops thing used by some other drivers. I guess that is supposed to achieve the same thing more or less? I'm not immediately seeing any kind of drm->alsa notification hook in there though.
On AMD hw, the GPU has backdoor access to some of the audio state, so when stuff happens on the GPU side, it's reflected on the audio side automatically.
Right. i915 has a similar thing (my theory is that it's basically an industry wide hardware workaround for inflexible Windows driver architecture). But that was problematic for some power management related reasons (IIRC) so we added a software mechanism for it. Though I believe we still write the ELD into the hardware buffer as well.
On Tue, 05 Mar 2019 20:36:38 +0100, Ville Syrjälä wrote:
On Tue, Mar 05, 2019 at 02:21:04PM -0500, Alex Deucher wrote:
On Tue, Mar 5, 2019 at 2:15 PM Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Tue, Mar 05, 2019 at 05:24:13PM +0200, Ville Syrjälä wrote:
On Tue, Mar 05, 2019 at 10:12:40AM +0100, Maxime Ripard wrote:
On Mon, Mar 04, 2019 at 03:05:31PM -0500, Alex Deucher wrote:
On Mon, Mar 4, 2019 at 2:53 PM Eric Anholt eric@anholt.net wrote: > > Maxime Ripard maxime.ripard@bootlin.com writes: > > > In some cases, in order to accomodate with displays with poor EDIDs, we > > need to ignore that the monitor alledgedly supports audio output and > > disable the audio output. > > > > Signed-off-by: Maxime Ripard maxime.ripard@bootlin.com > > --- > > drivers/gpu/drm/drm_edid.c | 8 ++++++++ > > 1 file changed, 8 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > > index 990b1909f9d7..c0258b011bb2 100644 > > --- a/drivers/gpu/drm/drm_edid.c > > +++ b/drivers/gpu/drm/drm_edid.c > > @@ -4190,6 +4190,11 @@ bool drm_detect_hdmi_monitor(struct edid *edid) > > } > > EXPORT_SYMBOL(drm_detect_hdmi_monitor); > > > > +static bool ignore_edid_audio = false; > > +module_param(ignore_edid_audio, bool, 0644); > > +MODULE_PARM_DESC(ignore_edid_audio, > > + "Ignore the EDID and always consider that a monitor doesn't have audio capabilities"); > > + > > /** > > * drm_detect_monitor_audio - check monitor audio capability > > * @edid: EDID block to scan > > @@ -4209,6 +4214,9 @@ bool drm_detect_monitor_audio(struct edid *edid) > > bool has_audio = false; > > int start_offset, end_offset; > > > > + if (ignore_edid_audio) > > + goto end; > > + > > edid_ext = drm_find_cea_extension(edid); > > if (!edid_ext) > > goto end; > > It looks like the motivation for the original flag on Raspberry Pi was > "I've got a non-audio monitor, but the system comes up trying to play > audio to HDMI instead of the analog jack". Do we have some way for DRM > to communicate to ALSA that this is not the right place to try to play > audio by default?
Apparently not. We have users using debug knobs in our drivers to disable display audio because ALSA defaults to that rather than other audio.
I guess one way to do this would be to register the card only when an audio-capable monitor is connected instead of doing this at probe time. I'm not sure how convenient it is for userspace though.
We already provide the ELD to alsa. I'm pretty sure pulseaudio uses that stuff somehow to figure out whether to play audio over HDMI. But since I don't use pulseaudio myself I can't be 100% sure.
Cc:ing Takashi/alsa folks for confirmation.
I forgot that the .pin_eld_notify() stuff is i915 specific. But I see some kind of hdmi_codec_ops thing used by some other drivers. I guess that is supposed to achieve the same thing more or less? I'm not immediately seeing any kind of drm->alsa notification hook in there though.
On AMD hw, the GPU has backdoor access to some of the audio state, so when stuff happens on the GPU side, it's reflected on the audio side automatically.
Right. i915 has a similar thing (my theory is that it's basically an industry wide hardware workaround for inflexible Windows driver architecture). But that was problematic for some power management related reasons (IIRC) so we added a software mechanism for it. Though I believe we still write the ELD into the hardware buffer as well.
I'm late to the game as I've been off in the last week, and here is just the confirmation.
The direct communication from drm to ALSA via component has been implemented currently only for i915. I had some patches to enable the feature for radeon and amdgpu, but the enablement on amdgpu DC is still missing, and the work is pending for now.
For other drivers, we'd need more or less similar mechanism. We might want to choose a better infrastructure than the component binding, but it's a thing to be discussed.
thanks,
Takashi
Hi,
On Wed, Mar 13, 2019 at 11:44:17AM +0100, Takashi Iwai wrote:
On Tue, 05 Mar 2019 20:36:38 +0100, Ville Syrjälä wrote:
On Tue, Mar 05, 2019 at 02:21:04PM -0500, Alex Deucher wrote:
On Tue, Mar 5, 2019 at 2:15 PM Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Tue, Mar 05, 2019 at 05:24:13PM +0200, Ville Syrjälä wrote:
On Tue, Mar 05, 2019 at 10:12:40AM +0100, Maxime Ripard wrote:
On Mon, Mar 04, 2019 at 03:05:31PM -0500, Alex Deucher wrote: > On Mon, Mar 4, 2019 at 2:53 PM Eric Anholt eric@anholt.net wrote: > > > > Maxime Ripard maxime.ripard@bootlin.com writes: > > > > > In some cases, in order to accomodate with displays with poor EDIDs, we > > > need to ignore that the monitor alledgedly supports audio output and > > > disable the audio output. > > > > > > Signed-off-by: Maxime Ripard maxime.ripard@bootlin.com > > > --- > > > drivers/gpu/drm/drm_edid.c | 8 ++++++++ > > > 1 file changed, 8 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > > > index 990b1909f9d7..c0258b011bb2 100644 > > > --- a/drivers/gpu/drm/drm_edid.c > > > +++ b/drivers/gpu/drm/drm_edid.c > > > @@ -4190,6 +4190,11 @@ bool drm_detect_hdmi_monitor(struct edid *edid) > > > } > > > EXPORT_SYMBOL(drm_detect_hdmi_monitor); > > > > > > +static bool ignore_edid_audio = false; > > > +module_param(ignore_edid_audio, bool, 0644); > > > +MODULE_PARM_DESC(ignore_edid_audio, > > > + "Ignore the EDID and always consider that a monitor doesn't have audio capabilities"); > > > + > > > /** > > > * drm_detect_monitor_audio - check monitor audio capability > > > * @edid: EDID block to scan > > > @@ -4209,6 +4214,9 @@ bool drm_detect_monitor_audio(struct edid *edid) > > > bool has_audio = false; > > > int start_offset, end_offset; > > > > > > + if (ignore_edid_audio) > > > + goto end; > > > + > > > edid_ext = drm_find_cea_extension(edid); > > > if (!edid_ext) > > > goto end; > > > > It looks like the motivation for the original flag on Raspberry Pi was > > "I've got a non-audio monitor, but the system comes up trying to play > > audio to HDMI instead of the analog jack". Do we have some way for DRM > > to communicate to ALSA that this is not the right place to try to play > > audio by default? > > Apparently not. We have users using debug knobs in our drivers to > disable display audio because ALSA defaults to that rather than other > audio.
I guess one way to do this would be to register the card only when an audio-capable monitor is connected instead of doing this at probe time. I'm not sure how convenient it is for userspace though.
We already provide the ELD to alsa. I'm pretty sure pulseaudio uses that stuff somehow to figure out whether to play audio over HDMI. But since I don't use pulseaudio myself I can't be 100% sure.
Cc:ing Takashi/alsa folks for confirmation.
I forgot that the .pin_eld_notify() stuff is i915 specific. But I see some kind of hdmi_codec_ops thing used by some other drivers. I guess that is supposed to achieve the same thing more or less? I'm not immediately seeing any kind of drm->alsa notification hook in there though.
On AMD hw, the GPU has backdoor access to some of the audio state, so when stuff happens on the GPU side, it's reflected on the audio side automatically.
Right. i915 has a similar thing (my theory is that it's basically an industry wide hardware workaround for inflexible Windows driver architecture). But that was problematic for some power management related reasons (IIRC) so we added a software mechanism for it. Though I believe we still write the ELD into the hardware buffer as well.
I'm late to the game as I've been off in the last week, and here is just the confirmation.
The direct communication from drm to ALSA via component has been implemented currently only for i915. I had some patches to enable the feature for radeon and amdgpu, but the enablement on amdgpu DC is still missing, and the work is pending for now.
For other drivers, we'd need more or less similar mechanism. We might want to choose a better infrastructure than the component binding, but it's a thing to be discussed.
I guess I would be interested in working on this.
I've looked at the ELD situation on vc4, and even though the control is exposed for the vc4 driver, and that content seems to be ok, pulseaudio doesn't pick it up.
My understanding is that pulseaudio waits on an ALSA event before looking at the ELD control, and only does so for the device that are considered HDMI, but I'm not quite sure what policy it has for that yet...
Maxime
participants (4)
-
Alex Deucher -
Maxime Ripard -
Takashi Iwai -
Ville Syrjälä