[v3,2/3] ASoC: mediatek: mt8186: correct the HDMI widgets

  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(-)
  

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