[PATCH RFC] drm/vc4: hdmi: Avoid ASoC error messages on startup

[PATCH RFC] drm/vc4: hdmi: Avoid ASoC error messages on startup

[Stefan Wahren]

During startup of Raspberry Pi 4 there seems to be a race between
VC4 probing and Pulseaudio trying to open its PCM device:

    ASoC: error at snd_soc_dai_startup on fef05700.hdmi: -19

Avoid these errors by returning EPROBE_DEFER in this situation.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 drivers/gpu/drm/vc4/vc4_hdmi.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 5551062..e0f9357 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -909,12 +909,14 @@ static int vc4_hdmi_audio_startup(struct snd_pcm_substream *substream,
 
 	vc4_hdmi->audio.substream = substream;
 
+	if (!encoder->crtc)
+		return -EPROBE_DEFER;
+
 	/*
-	 * If the HDMI encoder hasn't probed, or the encoder is
-	 * currently in DVI mode, treat the codec dai as missing.
+	 * If the HDMI encoder is currently in DVI mode,
+	 * treat the codec dai as missing.
 	 */
-	if (!encoder->crtc || !(HDMI_READ(HDMI_RAM_PACKET_CONFIG) &
-				VC4_HDMI_RAM_PACKET_ENABLE))
+	if (!(HDMI_READ(HDMI_RAM_PACKET_CONFIG) & VC4_HDMI_RAM_PACKET_ENABLE))
 		return -ENODEV;
 
 	ret = snd_pcm_hw_constraint_eld(substream->runtime, connector->eld);
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH RFC] drm/vc4: hdmi: Avoid ASoC error messages on startup

[Stefan Wahren]

Hi Maxime,

Am 29.12.20 um 16:36 schrieb Stefan Wahren:
> During startup of Raspberry Pi 4 there seems to be a race between
> VC4 probing and Pulseaudio trying to open its PCM device:
>
>     ASoC: error at snd_soc_dai_startup on fef05700.hdmi: -19
>
> Avoid these errors by returning EPROBE_DEFER in this situation.
>
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>

should i resend without RFC?

Best regards
Stefan

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH RFC] drm/vc4: hdmi: Avoid ASoC error messages on startup

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 633 bytes --]

Hi Stefan,

On Sat, Jan 09, 2021 at 12:41:57PM +0100, Stefan Wahren wrote:
> Hi Maxime,
> 
> Am 29.12.20 um 16:36 schrieb Stefan Wahren:
> > During startup of Raspberry Pi 4 there seems to be a race between
> > VC4 probing and Pulseaudio trying to open its PCM device:
> >
> >     ASoC: error at snd_soc_dai_startup on fef05700.hdmi: -19
> >
> > Avoid these errors by returning EPROBE_DEFER in this situation.
> >
> > Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> 
> should i resend without RFC?

Yeah, it looks reasonable enough to me :)

I'd like to get Mark's opinion before merging though

Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH RFC] drm/vc4: hdmi: Avoid ASoC error messages on startup

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 290 bytes --]

On Wed, Jan 13, 2021 at 10:19:57AM +0100, Maxime Ripard wrote:

> I'd like to get Mark's opinion before merging though

I'm not sure what the question is here?  I get CCed on a bunch of not
obviously relevant DRM patches so there's a good chance I've just
deleted some relevnat discussion.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH RFC] drm/vc4: hdmi: Avoid ASoC error messages on startup

[Nicolas Saenz Julienne]

[-- Attachment #1.1: Type: text/plain, Size: 572 bytes --]

On Tue, 2020-12-29 at 16:36 +0100, Stefan Wahren wrote:
> During startup of Raspberry Pi 4 there seems to be a race between
> VC4 probing and Pulseaudio trying to open its PCM device:
> 
>     ASoC: error at snd_soc_dai_startup on fef05700.hdmi: -19
> 
> Avoid these errors by returning EPROBE_DEFER in this situation.
> 
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> ---

Seems reasonable to me:

Tested-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
Reviewed-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>

Regards,
Nicolas


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH RFC] drm/vc4: hdmi: Avoid ASoC error messages on startup

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 625 bytes --]

Hi,

On Wed, Jan 13, 2021 at 11:42:23AM +0000, Mark Brown wrote:
> On Wed, Jan 13, 2021 at 10:19:57AM +0100, Maxime Ripard wrote:
> 
> > I'd like to get Mark's opinion before merging though
> 
> I'm not sure what the question is here?  I get CCed on a bunch of not
> obviously relevant DRM patches so there's a good chance I've just
> deleted some relevnat discussion.

The patch is question is here:
https://lore.kernel.org/dri-devel/1609256210-19863-1-git-send-email-stefan.wahren@i2se.com/

In particular, I'm not so sure whether it's fine to return -EPROBE_DEFER
in the startup hook.

Thanks!
Maxime


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH RFC] drm/vc4: hdmi: Avoid ASoC error messages on startup

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1013 bytes --]

On Fri, Jan 15, 2021 at 07:14:37PM +0100, Maxime Ripard wrote:
> On Wed, Jan 13, 2021 at 11:42:23AM +0000, Mark Brown wrote:
> > On Wed, Jan 13, 2021 at 10:19:57AM +0100, Maxime Ripard wrote:

> > > I'd like to get Mark's opinion before merging though

> > I'm not sure what the question is here?  I get CCed on a bunch of not
> > obviously relevant DRM patches so there's a good chance I've just
> > deleted some relevnat discussion.

> The patch is question is here:
> https://lore.kernel.org/dri-devel/1609256210-19863-1-git-send-email-stefan.wahren@i2se.com/

> In particular, I'm not so sure whether it's fine to return -EPROBE_DEFER
> in the startup hook.

I wouldn't expect that to do anything useful and IIRC that error code
will end up in userspace which isn't good.  If the driver wants to defer
probe it should defer it from the probe() routine, after the driver has
been instantiated I'm not sure what the expectation would be.  In
general a driver should acquire all resources it needs when probing.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH RFC] drm/vc4: hdmi: Avoid ASoC error messages on startup

[Stefan Wahren]

Hi,

Am 15.01.21 um 19:39 schrieb Mark Brown:
> On Fri, Jan 15, 2021 at 07:14:37PM +0100, Maxime Ripard wrote:
>> On Wed, Jan 13, 2021 at 11:42:23AM +0000, Mark Brown wrote:
>>> On Wed, Jan 13, 2021 at 10:19:57AM +0100, Maxime Ripard wrote:
>>>> I'd like to get Mark's opinion before merging though
>>> I'm not sure what the question is here?  I get CCed on a bunch of not
>>> obviously relevant DRM patches so there's a good chance I've just
>>> deleted some relevnat discussion.
>> The patch is question is here:
>> https://lore.kernel.org/dri-devel/1609256210-19863-1-git-send-email-stefan.wahren@i2se.com/
>> In particular, I'm not so sure whether it's fine to return -EPROBE_DEFER
>> in the startup hook.
> I wouldn't expect that to do anything useful and IIRC that error code
> will end up in userspace which isn't good.  If the driver wants to defer
> probe it should defer it from the probe() routine, after the driver has
> been instantiated I'm not sure what the expectation would be.  In
> general a driver should acquire all resources it needs when probing.

understand. Unfortunately, currently i don't have the time to
investigate how we can achieve this with this drm driver.

Maybe some else can take over?

Regards
Stefan

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH RFC] drm/vc4: hdmi: Avoid ASoC error messages on startup

[Nicolas Saenz Julienne]

[-- Attachment #1.1: Type: text/plain, Size: 1719 bytes --]

Hi Stefan, Maxime,

On Mon, 2021-01-18 at 12:28 +0100, Stefan Wahren wrote:
> Hi,
> 
> Am 15.01.21 um 19:39 schrieb Mark Brown:
> > On Fri, Jan 15, 2021 at 07:14:37PM +0100, Maxime Ripard wrote:
> > > On Wed, Jan 13, 2021 at 11:42:23AM +0000, Mark Brown wrote:
> > > > On Wed, Jan 13, 2021 at 10:19:57AM +0100, Maxime Ripard wrote:
> > > > > I'd like to get Mark's opinion before merging though
> > > > I'm not sure what the question is here?  I get CCed on a bunch of not
> > > > obviously relevant DRM patches so there's a good chance I've just
> > > > deleted some relevnat discussion.
> > > The patch is question is here:
> > > https://lore.kernel.org/dri-devel/1609256210-19863-1-git-send-email-stefan.wahren@i2se.com/
> > > In particular, I'm not so sure whether it's fine to return -EPROBE_DEFER
> > > in the startup hook.
> > I wouldn't expect that to do anything useful and IIRC that error code
> > will end up in userspace which isn't good.  If the driver wants to defer
> > probe it should defer it from the probe() routine, after the driver has
> > been instantiated I'm not sure what the expectation would be.  In
> > general a driver should acquire all resources it needs when probing.
> 
> understand. Unfortunately, currently i don't have the time to
> investigate how we can achieve this with this drm driver.
> 
> Maybe some else can take over?

My two cents: IIUC it's a tricky one since components don't have a way to
express dependencies. Somewhat similar to what happened with the DSI
bus/display race. To what extent vc4-crtc has a dependency with vc4-hdmi?
Couldn't we move vc4-hdmi component's registration at the end of
vc4_crtc_bind()?

Regards,
Nicolas


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH RFC] drm/vc4: hdmi: Avoid ASoC error messages on startup

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 2341 bytes --]

On Mon, Jan 18, 2021 at 01:10:40PM +0100, Nicolas Saenz Julienne wrote:
> Hi Stefan, Maxime,
> 
> On Mon, 2021-01-18 at 12:28 +0100, Stefan Wahren wrote:
> > Hi,
> > 
> > Am 15.01.21 um 19:39 schrieb Mark Brown:
> > > On Fri, Jan 15, 2021 at 07:14:37PM +0100, Maxime Ripard wrote:
> > > > On Wed, Jan 13, 2021 at 11:42:23AM +0000, Mark Brown wrote:
> > > > > On Wed, Jan 13, 2021 at 10:19:57AM +0100, Maxime Ripard wrote:
> > > > > > I'd like to get Mark's opinion before merging though
> > > > > I'm not sure what the question is here?  I get CCed on a bunch of not
> > > > > obviously relevant DRM patches so there's a good chance I've just
> > > > > deleted some relevnat discussion.
> > > > The patch is question is here:
> > > > https://lore.kernel.org/dri-devel/1609256210-19863-1-git-send-email-stefan.wahren@i2se.com/
> > > > In particular, I'm not so sure whether it's fine to return -EPROBE_DEFER
> > > > in the startup hook.
> > > I wouldn't expect that to do anything useful and IIRC that error code
> > > will end up in userspace which isn't good.  If the driver wants to defer
> > > probe it should defer it from the probe() routine, after the driver has
> > > been instantiated I'm not sure what the expectation would be.  In
> > > general a driver should acquire all resources it needs when probing.
> > 
> > understand. Unfortunately, currently i don't have the time to
> > investigate how we can achieve this with this drm driver.
> > 
> > Maybe some else can take over?
> 
> My two cents: IIUC it's a tricky one since components don't have a way to
> express dependencies.

There's a way though :)

Components are bound in the order they've been added to the component
list, so as long as the component we have a dependency on is added
first, it's fine.

The issue here is that the HDMI devices are added first:
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/vc4/vc4_drv.c#L334

We could just move the HDMI driver after the CRTC one and we would solve
it. I'm not sure if it has any other side effects though.

> Somewhat similar to what happened with the DSI bus/display race.

DSI is a bit different, especially on vc4 since the panel driver doesn't
sit on the DSI bus in DT and as such isn't added by the bus code when
the controller is added.

Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel