| Message ID | 20240422134354.89291-3-srinivas.kandagatla@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ASoC: qcom: display port changes | expand |
On Mon, Apr 22, 2024 at 02:43:52PM +0100, Srinivas Kandagatla wrote: > From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > > Add a common function to add Display port jack, this can be used by > multiple board files and avoid any code duplication. > > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > --- > sound/soc/qcom/common.c | 29 +++++++++++++++++++++++++++++ > sound/soc/qcom/common.h | 3 +++ > 2 files changed, 32 insertions(+) > > diff --git a/sound/soc/qcom/common.c b/sound/soc/qcom/common.c > index 747041fa7866..3bfe618e7bd7 100644 > --- a/sound/soc/qcom/common.c > +++ b/sound/soc/qcom/common.c > @@ -7,10 +7,14 @@ > #include <sound/jack.h> > #include <linux/input-event-codes.h> > #include "common.h" Missing newline. > +#define NAME_SIZE 32 > > static const struct snd_soc_dapm_widget qcom_jack_snd_widgets[] = { > SND_SOC_DAPM_HP("Headphone Jack", NULL), > SND_SOC_DAPM_MIC("Mic Jack", NULL), > + SND_SOC_DAPM_SPK("HDMI/DP0 Jack", NULL), > + SND_SOC_DAPM_SPK("HDMI/DP1 Jack", NULL), > + SND_SOC_DAPM_SPK("HDMI/DP2 Jack", NULL), > }; > > int qcom_snd_parse_of(struct snd_soc_card *card) > @@ -239,4 +243,29 @@ int qcom_snd_wcd_jack_setup(struct snd_soc_pcm_runtime *rtd, > return 0; > } > EXPORT_SYMBOL_GPL(qcom_snd_wcd_jack_setup); > + > +int qcom_snd_dp_jack_setup(struct snd_soc_pcm_runtime *rtd, > + struct snd_soc_jack *hdmi_jack, int hdmi_pcm_id) The function is called dp_jack_setup() so shouldn't the parameters reflect that and be called dp_jack etc. for consistency (i.e. even if you plan on using this interface also for hdmi)? Johan
On Mon, Apr 22, 2024 at 02:43:52PM +0100, Srinivas Kandagatla wrote: > static const struct snd_soc_dapm_widget qcom_jack_snd_widgets[] = { > SND_SOC_DAPM_HP("Headphone Jack", NULL), > SND_SOC_DAPM_MIC("Mic Jack", NULL), > + SND_SOC_DAPM_SPK("HDMI/DP0 Jack", NULL), > + SND_SOC_DAPM_SPK("HDMI/DP1 Jack", NULL), > + SND_SOC_DAPM_SPK("HDMI/DP2 Jack", NULL), Shouldn't these be split in dedicated HDMI and DP jacks too? What if you have a machine with HDMI and DP jacks which would otherwise both claim "HDMI/DP0"? Johan
On 23/04/2024 13:02, Johan Hovold wrote: > On Mon, Apr 22, 2024 at 02:43:52PM +0100, Srinivas Kandagatla wrote: > >> static const struct snd_soc_dapm_widget qcom_jack_snd_widgets[] = { >> SND_SOC_DAPM_HP("Headphone Jack", NULL), >> SND_SOC_DAPM_MIC("Mic Jack", NULL), >> + SND_SOC_DAPM_SPK("HDMI/DP0 Jack", NULL), >> + SND_SOC_DAPM_SPK("HDMI/DP1 Jack", NULL), >> + SND_SOC_DAPM_SPK("HDMI/DP2 Jack", NULL), > > Shouldn't these be split in dedicated HDMI and DP jacks too? What if you > have a machine with HDMI and DP jacks which would otherwise both claim > "HDMI/DP0"? These map to the Jack's added as part of qcom_snd_dp_jack_setup and belong to DISPLAY_PORT_RX_0, DISPLAY_PORT_RX_1, DISPLAY_PORT_RX_2. If its going via USB-C DP controller it will be either DP or an HDMI ? This is the most common naming for the USB-C DP/HDMI jack events. Qualcomm LPASS in some older SoCs had a dedicated HDMI interface which is different to this one. Usual Other ways to connect HDMI is via external HDMI Bridge using I2S interface which totally different to this DP interface. So none of these will conflict. hope this clarifies. thanks, Srini > > Johan
On Tue, Apr 23, 2024 at 04:55:32PM +0100, Srinivas Kandagatla wrote: > On 23/04/2024 13:02, Johan Hovold wrote: > > On Mon, Apr 22, 2024 at 02:43:52PM +0100, Srinivas Kandagatla wrote: > > > >> static const struct snd_soc_dapm_widget qcom_jack_snd_widgets[] = { > >> SND_SOC_DAPM_HP("Headphone Jack", NULL), > >> SND_SOC_DAPM_MIC("Mic Jack", NULL), > >> + SND_SOC_DAPM_SPK("HDMI/DP0 Jack", NULL), > >> + SND_SOC_DAPM_SPK("HDMI/DP1 Jack", NULL), > >> + SND_SOC_DAPM_SPK("HDMI/DP2 Jack", NULL), > > > > Shouldn't these be split in dedicated HDMI and DP jacks too? What if you > > have a machine with HDMI and DP jacks which would otherwise both claim > > "HDMI/DP0"? > > These map to the Jack's added as part of qcom_snd_dp_jack_setup and > belong to DISPLAY_PORT_RX_0, DISPLAY_PORT_RX_1, DISPLAY_PORT_RX_2. > > If its going via USB-C DP controller it will be either DP or an HDMI ? It will always be DP out of the machine even if an adapter can convert to HDMI internally. The DRM ports are called "DP-1" and "DP-2" so it seems we should match that. > This is the most common naming for the USB-C DP/HDMI jack events. It looks like some Intel machines use names like "HDMI/DP, pcm=%d Jack" (with a pcm device number), but we also have "DP Jack". Not sure which are are used with USB-C, though. (Or if the former actually support HDMI altmode.) > Qualcomm LPASS in some older SoCs had a dedicated HDMI interface which > is different to this one. > > Usual Other ways to connect HDMI is via external HDMI Bridge using I2S > interface which totally different to this DP interface. Sure, but if there's ever a design with such a port then it will be called "HDMI Jack" and then the "HDMI in "HDMI/DP0 Jack" is unnecessary and confusing when it is always DP out. Johan
On 29/04/2024 15:54, Johan Hovold wrote: > On Tue, Apr 23, 2024 at 04:55:32PM +0100, Srinivas Kandagatla wrote: >> On 23/04/2024 13:02, Johan Hovold wrote: >>> On Mon, Apr 22, 2024 at 02:43:52PM +0100, Srinivas Kandagatla wrote: >>> >>>> static const struct snd_soc_dapm_widget qcom_jack_snd_widgets[] = { >>>> SND_SOC_DAPM_HP("Headphone Jack", NULL), >>>> SND_SOC_DAPM_MIC("Mic Jack", NULL), >>>> + SND_SOC_DAPM_SPK("HDMI/DP0 Jack", NULL), >>>> + SND_SOC_DAPM_SPK("HDMI/DP1 Jack", NULL), >>>> + SND_SOC_DAPM_SPK("HDMI/DP2 Jack", NULL), >>> >>> Shouldn't these be split in dedicated HDMI and DP jacks too? What if you >>> have a machine with HDMI and DP jacks which would otherwise both claim >>> "HDMI/DP0"? >> >> These map to the Jack's added as part of qcom_snd_dp_jack_setup and >> belong to DISPLAY_PORT_RX_0, DISPLAY_PORT_RX_1, DISPLAY_PORT_RX_2. >> >> If its going via USB-C DP controller it will be either DP or an HDMI ? > > It will always be DP out of the machine even if an adapter can convert > to HDMI internally. > > The DRM ports are called "DP-1" and "DP-2" so it seems we should match > that. > >> This is the most common naming for the USB-C DP/HDMI jack events. > > It looks like some Intel machines use names like "HDMI/DP, pcm=%d Jack" > (with a pcm device number), but we also have "DP Jack". Not sure which > are are used with USB-C, though. (Or if the former actually support HDMI > altmode.) I checked this on my machine which has usb-c and I can confirm using HDMI/DP naming for these jack. Either way I don't mind having any names, but my point here is to be more consistent across. --srini > >> Qualcomm LPASS in some older SoCs had a dedicated HDMI interface which >> is different to this one. >> >> Usual Other ways to connect HDMI is via external HDMI Bridge using I2S >> interface which totally different to this DP interface. > > Sure, but if there's ever a design with such a port then it will be > called "HDMI Jack" and then the "HDMI in "HDMI/DP0 Jack" is unnecessary > and confusing when it is always DP out. > > Johan
On Thu, May 09, 2024 at 09:59:40AM +0100, Srinivas Kandagatla wrote: > > > On 29/04/2024 15:54, Johan Hovold wrote: > > On Tue, Apr 23, 2024 at 04:55:32PM +0100, Srinivas Kandagatla wrote: > > > On 23/04/2024 13:02, Johan Hovold wrote: > > > > On Mon, Apr 22, 2024 at 02:43:52PM +0100, Srinivas Kandagatla wrote: > > > > > static const struct snd_soc_dapm_widget qcom_jack_snd_widgets[] = { > > > > > SND_SOC_DAPM_HP("Headphone Jack", NULL), > > > > > SND_SOC_DAPM_MIC("Mic Jack", NULL), > > > > > + SND_SOC_DAPM_SPK("HDMI/DP0 Jack", NULL), > > > > > + SND_SOC_DAPM_SPK("HDMI/DP1 Jack", NULL), > > > > > + SND_SOC_DAPM_SPK("HDMI/DP2 Jack", NULL), > > > > > > > > Shouldn't these be split in dedicated HDMI and DP jacks too? What if you > > > > have a machine with HDMI and DP jacks which would otherwise both claim > > > > "HDMI/DP0"? > > > > > > These map to the Jack's added as part of qcom_snd_dp_jack_setup and > > > belong to DISPLAY_PORT_RX_0, DISPLAY_PORT_RX_1, DISPLAY_PORT_RX_2. > > > > > > If its going via USB-C DP controller it will be either DP or an HDMI ? > > > > It will always be DP out of the machine even if an adapter can convert > > to HDMI internally. > > > > The DRM ports are called "DP-1" and "DP-2" so it seems we should match > > that. > > > > > This is the most common naming for the USB-C DP/HDMI jack events. > > > > It looks like some Intel machines use names like "HDMI/DP, pcm=%d Jack" > > (with a pcm device number), but we also have "DP Jack". Not sure which > > are are used with USB-C, though. (Or if the former actually support HDMI > > altmode.) > > I checked this on my machine which has usb-c and I can confirm using HDMI/DP > naming for these jack. > > Either way I don't mind having any names, but my point here is to be more > consistent across. I fear it is till not consistent. On the Intel laptop I see following jacks: numid=18,iface=CARD,name='HDMI/DP,pcm=3 Jack' numid=24,iface=CARD,name='HDMI/DP,pcm=7 Jack' numid=30,iface=CARD,name='HDMI/DP,pcm=8 Jack' On the other hand Mediatek and RockChip use just 'DP Jack'. I'd suggest settling on the latter option. We are closer to MTK and RockChip rather than Intel. BTW: a platform can easily have 4 (x1e8100) or even 8 (sc8280xp) DP outputs. Could you please point out why there are just 3 jacks? > --srini > > > > > Qualcomm LPASS in some older SoCs had a dedicated HDMI interface which > > > is different to this one. > > > > > > Usual Other ways to connect HDMI is via external HDMI Bridge using I2S > > > interface which totally different to this DP interface. > > > > Sure, but if there's ever a design with such a port then it will be > > called "HDMI Jack" and then the "HDMI in "HDMI/DP0 Jack" is unnecessary > > and confusing when it is always DP out. > > > > Johan
On 30/05/2024 23:38, Dmitry Baryshkov wrote: >>> It will always be DP out of the machine even if an adapter can convert >>> to HDMI internally. >>> >>> The DRM ports are called "DP-1" and "DP-2" so it seems we should match >>> that. >>> >>>> This is the most common naming for the USB-C DP/HDMI jack events. >>> It looks like some Intel machines use names like "HDMI/DP, pcm=%d Jack" >>> (with a pcm device number), but we also have "DP Jack". Not sure which >>> are are used with USB-C, though. (Or if the former actually support HDMI >>> altmode.) >> I checked this on my machine which has usb-c and I can confirm using HDMI/DP >> naming for these jack. >> >> Either way I don't mind having any names, but my point here is to be more >> consistent across. > I fear it is till not consistent. On the Intel laptop I see following > jacks: > > numid=18,iface=CARD,name='HDMI/DP,pcm=3 Jack' > numid=24,iface=CARD,name='HDMI/DP,pcm=7 Jack' > numid=30,iface=CARD,name='HDMI/DP,pcm=8 Jack' > > On the other hand Mediatek and RockChip use just 'DP Jack'. > > I'd suggest settling on the latter option. We are closer to MTK and > RockChip rather than Intel. that is fine with me. > > BTW: a platform can easily have 4 (x1e8100) or even 8 (sc8280xp) DP > outputs. Could you please point out why there are just 3 jacks? The CRD platform that I have access to has 3 ports which is why I started with 3 ports, but we can add more ports as and when we can really test them. --srini
diff --git a/sound/soc/qcom/common.c b/sound/soc/qcom/common.c index 747041fa7866..3bfe618e7bd7 100644 --- a/sound/soc/qcom/common.c +++ b/sound/soc/qcom/common.c @@ -7,10 +7,14 @@ #include <sound/jack.h> #include <linux/input-event-codes.h> #include "common.h" +#define NAME_SIZE 32 static const struct snd_soc_dapm_widget qcom_jack_snd_widgets[] = { SND_SOC_DAPM_HP("Headphone Jack", NULL), SND_SOC_DAPM_MIC("Mic Jack", NULL), + SND_SOC_DAPM_SPK("HDMI/DP0 Jack", NULL), + SND_SOC_DAPM_SPK("HDMI/DP1 Jack", NULL), + SND_SOC_DAPM_SPK("HDMI/DP2 Jack", NULL), }; int qcom_snd_parse_of(struct snd_soc_card *card) @@ -239,4 +243,29 @@ int qcom_snd_wcd_jack_setup(struct snd_soc_pcm_runtime *rtd, return 0; } EXPORT_SYMBOL_GPL(qcom_snd_wcd_jack_setup); + +int qcom_snd_dp_jack_setup(struct snd_soc_pcm_runtime *rtd, + struct snd_soc_jack *hdmi_jack, int hdmi_pcm_id) +{ + struct snd_soc_dai *codec_dai = snd_soc_rtd_to_codec(rtd, 0); + struct snd_soc_card *card = rtd->card; + char jack_name[NAME_SIZE]; + int rval, i; + + snprintf(jack_name, sizeof(jack_name), "HDMI/DP%d Jack", hdmi_pcm_id); + rval = snd_soc_card_jack_new(card, jack_name, SND_JACK_AVOUT, hdmi_jack); + if (rval) + return rval; + + for_each_rtd_codec_dais(rtd, i, codec_dai) { + rval = snd_soc_component_set_jack(codec_dai->component, hdmi_jack, NULL); + if (rval != 0 && rval != -ENOTSUPP) { + dev_warn(card->dev, "Failed to set jack: %d\n", rval); + return rval; + } + } + + return 0; +} +EXPORT_SYMBOL_GPL(qcom_snd_dp_jack_setup); MODULE_LICENSE("GPL"); diff --git a/sound/soc/qcom/common.h b/sound/soc/qcom/common.h index d7f80ee5ae26..3675d72c5285 100644 --- a/sound/soc/qcom/common.h +++ b/sound/soc/qcom/common.h @@ -9,5 +9,8 @@ int qcom_snd_parse_of(struct snd_soc_card *card); int qcom_snd_wcd_jack_setup(struct snd_soc_pcm_runtime *rtd, struct snd_soc_jack *jack, bool *jack_setup); +int qcom_snd_dp_jack_setup(struct snd_soc_pcm_runtime *rtd, + struct snd_soc_jack *jack, int id); + #endif