| Message ID | 20230730180803.22570-3-jiaxin.yu@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ASoC: mediatek:mt8186: fix both the speaker and hdmi | expand |
On Mon, Jul 31, 2023 at 02:08:02AM +0800, Jiaxin Yu wrote: > Use SND_SOC_DAPM_LINE instead of SND_SOC_DAPM_OUTPUT to trigger > DAPM events to hdmi-codec when userspace control the DPAM pin. Why?
On Mon, 2023-07-31 at 12:50 +0100, Mark Brown wrote: > On Mon, Jul 31, 2023 at 02:08:02AM +0800, Jiaxin Yu wrote: > > > Use SND_SOC_DAPM_LINE instead of SND_SOC_DAPM_OUTPUT to trigger > > DAPM events to hdmi-codec when userspace control the DPAM pin. > > Why? I have defined an SOC_DAPM_PIN_SWITCH that named as "HDMI1", if I use SND_SOC_DAPM_OUTPUT, it can't be controlled by HDMI1's PIN_SWITCH. 994 static const struct snd_kcontrol_new 995 mt8186_mt6366_da7219_max98357_controls[] = { 996 SOC_DAPM_PIN_SWITCH("Speakers"), 997 SOC_DAPM_PIN_SWITCH("Headphones"), 998 SOC_DAPM_PIN_SWITCH("Headset Mic"), 999 SOC_DAPM_PIN_SWITCH("HDMI1"), I think SND_SOC_DAPM_OUTPUT must be judged as ep, so I want to define HDMI1 as a snd_soc_dapm_spk's widget. From the perspective of hardware connection, their relationship is indeed equal, so I find SOC_SOC_DAPM_LINE to define HDMI1. ==> hdmi-codec ==> it6505(HDMI output) DL1(FE) ==> I2S3(BE) ==> rt1015p(SPEAKER output) 2738 static void dapm_update_widget_flags(struct snd_soc_dapm_widget *w) 2739 { 2740 enum snd_soc_dapm_direction dir; 2741 struct snd_soc_dapm_path *p; 2742 unsigned int ep; 2743 ... 2760 case snd_soc_dapm_output: 2761 /* On a fully routed card a output is never a sink */ 2762 if (w->dapm->card->fully_routed) 2763 return; 2764 ep = SND_SOC_DAPM_EP_SINK; 2765 snd_soc_dapm_widget_for_each_sink_path(w, p) { 2766 if (p->sink->id == snd_soc_dapm_spk || 2767 p->sink->id == snd_soc_dapm_hp || 2768 p->sink->id == snd_soc_dapm_line || 2769 p->sink->id == snd_soc_dapm_input) { 2770 ep = 0; 2771 break; 2772 } 2773 } 2774 break;
On Wed, Aug 02, 2023 at 02:52:57PM +0000, Jiaxin Yu (俞家鑫) wrote: > On Mon, 2023-07-31 at 12:50 +0100, Mark Brown wrote: > > On Mon, Jul 31, 2023 at 02:08:02AM +0800, Jiaxin Yu wrote: > > > Use SND_SOC_DAPM_LINE instead of SND_SOC_DAPM_OUTPUT to trigger > > > DAPM events to hdmi-codec when userspace control the DPAM pin. > > Why? > I have defined an SOC_DAPM_PIN_SWITCH that named as "HDMI1", if I use > SND_SOC_DAPM_OUTPUT, it can't be controlled by HDMI1's PIN_SWITCH. ... > 2762 if (w->dapm->card->fully_routed) > 2763 return; > 2764 ep = SND_SOC_DAPM_EP_SINK; > 2765 snd_soc_dapm_widget_for_each_sink_path(w, p) { > 2766 if (p->sink->id == snd_soc_dapm_spk || > 2767 p->sink->id == snd_soc_dapm_hp || > 2768 p->sink->id == snd_soc_dapm_line > || > 2769 p->sink->id == snd_soc_dapm_input) > { > 2770 ep = 0; The expectation here is that you'll connect the output to a widget that corresponds to the physical output on your board and put the pin switch on that, ideally with a label that corresponds to case markings or what the physical output is called on the board.
On Wed, 2023-08-02 at 17:38 +0100, Mark Brown wrote: > On Wed, Aug 02, 2023 at 02:52:57PM +0000, Jiaxin Yu (俞家鑫) wrote: > > On Mon, 2023-07-31 at 12:50 +0100, Mark Brown wrote: > > > On Mon, Jul 31, 2023 at 02:08:02AM +0800, Jiaxin Yu wrote: > > > > Use SND_SOC_DAPM_LINE instead of SND_SOC_DAPM_OUTPUT to trigger > > > > DAPM events to hdmi-codec when userspace control the DPAM pin. > > > Why? > > I have defined an SOC_DAPM_PIN_SWITCH that named as "HDMI1", if I > > use > > SND_SOC_DAPM_OUTPUT, it can't be controlled by HDMI1's PIN_SWITCH. > > ... > > > 2762 if (w->dapm->card->fully_routed) > > 2763 return; > > 2764 ep = SND_SOC_DAPM_EP_SINK; > > 2765 snd_soc_dapm_widget_for_each_sink_path(w, p) { > > 2766 if (p->sink->id == snd_soc_dapm_spk || > > 2767 p->sink->id == snd_soc_dapm_hp > > || > > 2768 p->sink->id == > > snd_soc_dapm_line > > > > > > > > 2769 p->sink->id == > > snd_soc_dapm_input) > > { > > 2770 ep = 0; > > The expectation here is that you'll connect the output to a widget > that > corresponds to the physical output on your board and put the pin > switch > on that, ideally with a label that corresponds to case markings or > what > the physical output is called on the board. Dear brown, I agree with you, in fact the speaker is indeed doing this way. But about the hdmi that on the board, I did not find a defination link snd_soc_dapm_hdmi, so I use snd_soc_dapm_line to replace. The purpose is to control it link speaker. Or what do you suggest I should do? Thank you very much.
On Thu, Aug 03, 2023 at 07:20:15AM +0000, Jiaxin Yu (俞家鑫) wrote: > I agree with you, in fact the speaker is indeed doing this way. But > about the hdmi that on the board, I did not find a defination link > snd_soc_dapm_hdmi, so I use snd_soc_dapm_line to replace. The purpose > is to control it link speaker. Or what do you suggest I should do? I think the sensible thing here is to define a DIGITAL_OUTPUT() which can be used for HDMI, S/PDIF and anything else that comes up and isn't clearly wrong like reusing one of the analog descriptions is.
Il 03/08/23 21:33, Mark Brown ha scritto: > On Thu, Aug 03, 2023 at 07:20:15AM +0000, Jiaxin Yu (俞家鑫) wrote: > >> I agree with you, in fact the speaker is indeed doing this way. But >> about the hdmi that on the board, I did not find a defination link >> snd_soc_dapm_hdmi, so I use snd_soc_dapm_line to replace. The purpose >> is to control it link speaker. Or what do you suggest I should do? > > I think the sensible thing here is to define a DIGITAL_OUTPUT() which > can be used for HDMI, S/PDIF and anything else that comes up and isn't > clearly wrong like reusing one of the analog descriptions is. Hello Jiaxin, the MT8186 Corsola Chromebooks are broken upstream without this series. Are you still interested in upstreaming this one? Thanks, Angelo
On Wed, 2024-01-31 at 12:42 +0100, AngeloGioacchino Del Regno wrote: > Il 03/08/23 21:33, Mark Brown ha scritto: > > On Thu, Aug 03, 2023 at 07:20:15AM +0000, Jiaxin Yu (俞家鑫) wrote: > > > > > I agree with you, in fact the speaker is indeed doing this way. > > > But > > > about the hdmi that on the board, I did not find a defination > > > link > > > snd_soc_dapm_hdmi, so I use snd_soc_dapm_line to replace. The > > > purpose > > > is to control it link speaker. Or what do you suggest I should > > > do? > > > > I think the sensible thing here is to define a DIGITAL_OUTPUT() > > which > > can be used for HDMI, S/PDIF and anything else that comes up and > > isn't > > clearly wrong like reusing one of the analog descriptions is. > > Hello Jiaxin, > > the MT8186 Corsola Chromebooks are broken upstream without this > series. > > Are you still interested in upstreaming this one? > > Thanks, > Angelo Hello Angelo, No, I'm still interesting in upstream this series. It's just that I have less time recently. I'm considering revisiting this issue next mouth. Do you have any suggestions for this? Thanks, Jiaxin.Yu
Il 31/01/24 13:25, Jiaxin Yu (俞家鑫) ha scritto: > On Wed, 2024-01-31 at 12:42 +0100, AngeloGioacchino Del Regno wrote: >> Il 03/08/23 21:33, Mark Brown ha scritto: >>> On Thu, Aug 03, 2023 at 07:20:15AM +0000, Jiaxin Yu (俞家鑫) wrote: >>> >>>> I agree with you, in fact the speaker is indeed doing this way. >>>> But >>>> about the hdmi that on the board, I did not find a defination >>>> link >>>> snd_soc_dapm_hdmi, so I use snd_soc_dapm_line to replace. The >>>> purpose >>>> is to control it link speaker. Or what do you suggest I should >>>> do? >>> >>> I think the sensible thing here is to define a DIGITAL_OUTPUT() >>> which >>> can be used for HDMI, S/PDIF and anything else that comes up and >>> isn't >>> clearly wrong like reusing one of the analog descriptions is. >> >> Hello Jiaxin, >> >> the MT8186 Corsola Chromebooks are broken upstream without this >> series. >> >> Are you still interested in upstreaming this one? >> >> Thanks, >> Angelo > > Hello Angelo, > > No, I'm still interesting in upstream this series. It's just that I > have less time recently. I'm considering revisiting this issue next > mouth. Do you have any suggestions for this? > Nothing on top of Mark's suggestions. Angelo
diff --git a/sound/soc/mediatek/mt8186/mt8186-mt6366-da7219-max98357.c b/sound/soc/mediatek/mt8186/mt8186-mt6366-da7219-max98357.c index 0432f9d89020..ae51d70e2c0b 100644 --- a/sound/soc/mediatek/mt8186/mt8186-mt6366-da7219-max98357.c +++ b/sound/soc/mediatek/mt8186/mt8186-mt6366-da7219-max98357.c @@ -964,7 +964,7 @@ mt8186_mt6366_da7219_max98357_widgets[] = { SND_SOC_DAPM_SPK("Speakers", NULL), SND_SOC_DAPM_HP("Headphones", NULL), SND_SOC_DAPM_MIC("Headset Mic", NULL), - SND_SOC_DAPM_OUTPUT("HDMI1"), + SND_SOC_DAPM_LINE("HDMI1", NULL), SND_SOC_DAPM_MIXER(SOF_DMA_DL1, SND_SOC_NOPM, 0, 0, NULL, 0), SND_SOC_DAPM_MIXER(SOF_DMA_DL2, SND_SOC_NOPM, 0, 0, NULL, 0), SND_SOC_DAPM_MIXER(SOF_DMA_UL1, SND_SOC_NOPM, 0, 0, NULL, 0), diff --git a/sound/soc/mediatek/mt8186/mt8186-mt6366-rt1019-rt5682s.c b/sound/soc/mediatek/mt8186/mt8186-mt6366-rt1019-rt5682s.c index 9c11016f032c..a39e37fa4e02 100644 --- a/sound/soc/mediatek/mt8186/mt8186-mt6366-rt1019-rt5682s.c +++ b/sound/soc/mediatek/mt8186/mt8186-mt6366-rt1019-rt5682s.c @@ -1032,7 +1032,7 @@ mt8186_mt6366_rt1019_rt5682s_widgets[] = { SND_SOC_DAPM_SPK("Speakers", NULL), SND_SOC_DAPM_HP("Headphone", NULL), SND_SOC_DAPM_MIC("Headset Mic", NULL), - SND_SOC_DAPM_OUTPUT("HDMI1"), + SND_SOC_DAPM_LINE("HDMI1", NULL), SND_SOC_DAPM_MIXER(SOF_DMA_DL1, SND_SOC_NOPM, 0, 0, NULL, 0), SND_SOC_DAPM_MIXER(SOF_DMA_DL2, SND_SOC_NOPM, 0, 0, NULL, 0), SND_SOC_DAPM_MIXER(SOF_DMA_UL1, SND_SOC_NOPM, 0, 0, NULL, 0),
Use SND_SOC_DAPM_LINE instead of SND_SOC_DAPM_OUTPUT to trigger DAPM events to hdmi-codec when userspace control the DPAM pin. Signed-off-by: Jiaxin Yu <jiaxin.yu@mediatek.com> --- sound/soc/mediatek/mt8186/mt8186-mt6366-da7219-max98357.c | 2 +- sound/soc/mediatek/mt8186/mt8186-mt6366-rt1019-rt5682s.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)