| Message ID | 20250304193912.5696-3-wahrenst@gmx.net (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm/vc4: hdmi: Add jack detection to HDMI audio driver | expand |
Hi, On Tue, Mar 04, 2025 at 08:39:12PM +0100, Stefan Wahren wrote: > From: David Turner <david.turner@raspberrypi.com> > > Add ALSA jack detection to the vc4-hdmi audio driver so userspace knows > when to add/remove HDMI audio devices. > > Signed-off-by: David Turner <david.turner@raspberrypi.com> > Signed-off-by: Stefan Wahren <wahrenst@gmx.net> > --- > drivers/gpu/drm/vc4/vc4_hdmi.c | 18 ++++++++++++++++++ > drivers/gpu/drm/vc4/vc4_hdmi.h | 7 +++++++ > 2 files changed, 25 insertions(+) > > diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c > index e4b027616d04..f46a135568b2 100644 > --- a/drivers/gpu/drm/vc4/vc4_hdmi.c > +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c > @@ -51,6 +51,7 @@ > #include <linux/reset.h> > #include <sound/dmaengine_pcm.h> > #include <sound/hdmi-codec.h> > +#include <sound/jack.h> > #include <sound/pcm_drm_eld.h> > #include <sound/pcm_params.h> > #include <sound/soc.h> > @@ -2203,6 +2204,22 @@ static const struct drm_connector_hdmi_audio_funcs vc4_hdmi_audio_funcs = { > .shutdown = vc4_hdmi_audio_shutdown, > }; > > +static int vc4_hdmi_codec_init(struct snd_soc_pcm_runtime *rtd) > +{ > + struct vc4_hdmi *vc4_hdmi = snd_soc_card_get_drvdata(rtd->card); > + struct snd_soc_component *component = snd_soc_rtd_to_codec(rtd, 0)->component; > + int ret; > + > + ret = snd_soc_card_jack_new(rtd->card, "HDMI Jack", SND_JACK_LINEOUT, > + &vc4_hdmi->hdmi_jack); > + if (ret) { > + dev_err(rtd->dev, "HDMI Jack creation failed: %d\n", ret); > + return ret; > + } > + > + return snd_soc_component_set_jack(component, &vc4_hdmi->hdmi_jack, NULL); > +} > + > static int vc4_hdmi_audio_init(struct vc4_hdmi *vc4_hdmi) > { > const struct vc4_hdmi_register *mai_data = > @@ -2316,6 +2333,7 @@ static int vc4_hdmi_audio_init(struct vc4_hdmi *vc4_hdmi) > dai_link->cpus->dai_name = dev_name(dev); > dai_link->codecs->name = dev_name(&vc4_hdmi->connector.hdmi_audio.codec_pdev->dev); > dai_link->platforms->name = dev_name(dev); > + dai_link->init = vc4_hdmi_codec_init; > > card->dai_link = dai_link; > card->num_links = 1; > diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h > index e3d989ca302b..a31157c99bee 100644 > --- a/drivers/gpu/drm/vc4/vc4_hdmi.h > +++ b/drivers/gpu/drm/vc4/vc4_hdmi.h > @@ -4,6 +4,7 @@ > #include <drm/drm_connector.h> > #include <media/cec.h> > #include <sound/dmaengine_pcm.h> > +#include <sound/hdmi-codec.h> > #include <sound/soc.h> > > #include "vc4_drv.h" > @@ -211,6 +212,12 @@ struct vc4_hdmi { > * KMS hooks. Protected by @mutex. > */ > enum hdmi_colorspace output_format; > + > + /** > + * @hdmi_jack: Represents the connection state of the HDMI plug, for > + * ALSA jack detection. > + */ > + struct snd_soc_jack hdmi_jack; > }; It looks fairly generic to me. Is there any reason you didn't put it in the HDMI audio helpers? Maxime
Hi all, On Thu, 6 Mar 2025 at 13:39, Maxime Ripard <mripard@kernel.org> wrote: > It looks fairly generic to me. Is there any reason you didn't put it in > the HDMI audio helpers? I originally wrote the downstream patch last year on 6.6, before the generic HDMI audio code existed. I just had a look at doing this. At the moment drm_hdmi_audio_helper doesn't manage the snd_soc_card (needed to setup the jack) at all, that still lives in vc4_hdmi_audio, so I can't see an easy way to move this over without a lot more reorganising. David
On Thu, 6 Mar 2025 at 14:39, Maxime Ripard <mripard@kernel.org> wrote: > > Hi, > > On Tue, Mar 04, 2025 at 08:39:12PM +0100, Stefan Wahren wrote: > > From: David Turner <david.turner@raspberrypi.com> > > > > Add ALSA jack detection to the vc4-hdmi audio driver so userspace knows > > when to add/remove HDMI audio devices. > > > > Signed-off-by: David Turner <david.turner@raspberrypi.com> > > Signed-off-by: Stefan Wahren <wahrenst@gmx.net> > > --- > > drivers/gpu/drm/vc4/vc4_hdmi.c | 18 ++++++++++++++++++ > > drivers/gpu/drm/vc4/vc4_hdmi.h | 7 +++++++ > > 2 files changed, 25 insertions(+) > > > > diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c > > index e4b027616d04..f46a135568b2 100644 > > --- a/drivers/gpu/drm/vc4/vc4_hdmi.c > > +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c > > @@ -51,6 +51,7 @@ > > #include <linux/reset.h> > > #include <sound/dmaengine_pcm.h> > > #include <sound/hdmi-codec.h> > > +#include <sound/jack.h> > > #include <sound/pcm_drm_eld.h> > > #include <sound/pcm_params.h> > > #include <sound/soc.h> > > @@ -2203,6 +2204,22 @@ static const struct drm_connector_hdmi_audio_funcs vc4_hdmi_audio_funcs = { > > .shutdown = vc4_hdmi_audio_shutdown, > > }; > > > > +static int vc4_hdmi_codec_init(struct snd_soc_pcm_runtime *rtd) > > +{ > > + struct vc4_hdmi *vc4_hdmi = snd_soc_card_get_drvdata(rtd->card); > > + struct snd_soc_component *component = snd_soc_rtd_to_codec(rtd, 0)->component; > > + int ret; > > + > > + ret = snd_soc_card_jack_new(rtd->card, "HDMI Jack", SND_JACK_LINEOUT, > > + &vc4_hdmi->hdmi_jack); > > + if (ret) { > > + dev_err(rtd->dev, "HDMI Jack creation failed: %d\n", ret); > > + return ret; > > + } > > + > > + return snd_soc_component_set_jack(component, &vc4_hdmi->hdmi_jack, NULL); > > +} > > + > > static int vc4_hdmi_audio_init(struct vc4_hdmi *vc4_hdmi) > > { > > const struct vc4_hdmi_register *mai_data = > > @@ -2316,6 +2333,7 @@ static int vc4_hdmi_audio_init(struct vc4_hdmi *vc4_hdmi) > > dai_link->cpus->dai_name = dev_name(dev); > > dai_link->codecs->name = dev_name(&vc4_hdmi->connector.hdmi_audio.codec_pdev->dev); > > dai_link->platforms->name = dev_name(dev); > > + dai_link->init = vc4_hdmi_codec_init; > > > > card->dai_link = dai_link; > > card->num_links = 1; > > diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h > > index e3d989ca302b..a31157c99bee 100644 > > --- a/drivers/gpu/drm/vc4/vc4_hdmi.h > > +++ b/drivers/gpu/drm/vc4/vc4_hdmi.h > > @@ -4,6 +4,7 @@ > > #include <drm/drm_connector.h> > > #include <media/cec.h> > > #include <sound/dmaengine_pcm.h> > > +#include <sound/hdmi-codec.h> > > #include <sound/soc.h> > > > > #include "vc4_drv.h" > > @@ -211,6 +212,12 @@ struct vc4_hdmi { > > * KMS hooks. Protected by @mutex. > > */ > > enum hdmi_colorspace output_format; > > + > > + /** > > + * @hdmi_jack: Represents the connection state of the HDMI plug, for > > + * ALSA jack detection. > > + */ > > + struct snd_soc_jack hdmi_jack; > > }; > > It looks fairly generic to me. Is there any reason you didn't put it in > the HDMI audio helpers? I had a similar question in my mind, but after checking I also could not find a good place for it. I had an idea of pushing this kind of code to hdmi-codec.c / hdmi_probe(), but then I understood that it might not work. The codec can be registered independently from the platform sound card device, the codec doesn't have a knowledge of the platform's data or topology. We might not be able to specify the name (even through codec platform data) if there are more than one HDMI connector / codec in play. So, I think that a sound card driver is a proper place for that (so it belongs to the vc4 driver).
On Thu, Mar 06, 2025 at 03:47:27PM +0000, David Turner wrote: > Hi all, > > On Thu, 6 Mar 2025 at 13:39, Maxime Ripard <mripard@kernel.org> wrote: > > It looks fairly generic to me. Is there any reason you didn't put it in > > the HDMI audio helpers? > > I originally wrote the downstream patch last year on 6.6, before the > generic HDMI audio code existed. > > I just had a look at doing this. At the moment drm_hdmi_audio_helper > doesn't manage the snd_soc_card (needed to setup the jack) at all, > that still lives in vc4_hdmi_audio, so I can't see an easy way to move > this over without a lot more reorganising. Ah, yes, of course. Most (all?) the other drivers just use an external I2S controller so the card is mostly likely going to be described through simple-card. I guess we can always turn that code into a helper if a similar case comes along in the future. Thanks, Maxime
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index e4b027616d04..f46a135568b2 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -51,6 +51,7 @@ #include <linux/reset.h> #include <sound/dmaengine_pcm.h> #include <sound/hdmi-codec.h> +#include <sound/jack.h> #include <sound/pcm_drm_eld.h> #include <sound/pcm_params.h> #include <sound/soc.h> @@ -2203,6 +2204,22 @@ static const struct drm_connector_hdmi_audio_funcs vc4_hdmi_audio_funcs = { .shutdown = vc4_hdmi_audio_shutdown, }; +static int vc4_hdmi_codec_init(struct snd_soc_pcm_runtime *rtd) +{ + struct vc4_hdmi *vc4_hdmi = snd_soc_card_get_drvdata(rtd->card); + struct snd_soc_component *component = snd_soc_rtd_to_codec(rtd, 0)->component; + int ret; + + ret = snd_soc_card_jack_new(rtd->card, "HDMI Jack", SND_JACK_LINEOUT, + &vc4_hdmi->hdmi_jack); + if (ret) { + dev_err(rtd->dev, "HDMI Jack creation failed: %d\n", ret); + return ret; + } + + return snd_soc_component_set_jack(component, &vc4_hdmi->hdmi_jack, NULL); +} + static int vc4_hdmi_audio_init(struct vc4_hdmi *vc4_hdmi) { const struct vc4_hdmi_register *mai_data = @@ -2316,6 +2333,7 @@ static int vc4_hdmi_audio_init(struct vc4_hdmi *vc4_hdmi) dai_link->cpus->dai_name = dev_name(dev); dai_link->codecs->name = dev_name(&vc4_hdmi->connector.hdmi_audio.codec_pdev->dev); dai_link->platforms->name = dev_name(dev); + dai_link->init = vc4_hdmi_codec_init; card->dai_link = dai_link; card->num_links = 1; diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h index e3d989ca302b..a31157c99bee 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.h +++ b/drivers/gpu/drm/vc4/vc4_hdmi.h @@ -4,6 +4,7 @@ #include <drm/drm_connector.h> #include <media/cec.h> #include <sound/dmaengine_pcm.h> +#include <sound/hdmi-codec.h> #include <sound/soc.h> #include "vc4_drv.h" @@ -211,6 +212,12 @@ struct vc4_hdmi { * KMS hooks. Protected by @mutex. */ enum hdmi_colorspace output_format; + + /** + * @hdmi_jack: Represents the connection state of the HDMI plug, for + * ALSA jack detection. + */ + struct snd_soc_jack hdmi_jack; }; #define connector_to_vc4_hdmi(_connector) \