Re: [PATCH v2 1/3] ASoC: hdmi-codec: Add event handler for hdmi TX
On Tue, Dec 13, 2022 at 02:23:32PM +, Jiaxin Yu (俞家鑫) wrote:
> On Mon, 2022-12-05 at 12:07 +, Mark Brown wrote:
> > On Mon, Dec 05, 2022 at 09:34:17AM +, Jiaxin Yu (俞家鑫) wrote:
> > No, I mean that if you want to control the enable and disable of the
> > output path you should implement a DAPM widget.
> May I ask which driver file to add a new DAPM widget? Is it the bridge
> ic driver like it6505.c? Or is it linke the "SDB" added in this patch?
I would expect this to follow a similar pattern to everything else with
hdmi-codec.c and have the actual ASoC stuff in there with a callback
exposed to the rest of the world.
> Yes, I should add a new set of events, such as:
> enum {
> HDMI_CODEC_TRIGGER_EVENT_STOP,
> HDMI_CODEC_TRIGGER_EVENT_START,
> HDMI_CODEC_TRIGGER_EVENT_SUSPEND,
> HDMI_CODEC_TRIGGER_EVENT_RESUME,
> }
> Then provide handles for these events in the it6505 driver. Am I right?
I'd expect more like on/off for a DAPM widget (the DAPM callbacks are
pre/post on/off) but yes.
signature.asc
Description: PGP signature
Re: [PATCH v2 1/3] ASoC: hdmi-codec: Add event handler for hdmi TX
Re: [PATCH v2 1/3] ASoC: hdmi-codec: Add event handler for hdmi TX
On Mon, Dec 05, 2022 at 09:34:17AM +, Jiaxin Yu (俞家鑫) wrote: > 1. I have added a DAPM widget that is "SDB", when we open or close HDMI > PIN_SWITCH, the callback 'hdmi_tx_event' registered in the widget will > be triggered. Maybe you mean I shouldn't use SNDRV_PCM_TRIGGER_START > and SNDRV_PCM_TRIGGER_STOP? No, I mean that if you want to control the enable and disable of the output path you should implement a DAPM widget. > 2. If I don't use hcd.ops->trigger notifies the bridge ic driver to > switch the audio, which ops should I use? > I actually want to know hdmi-codec.c and it6505.c except > hdmi_codec_ops, is there any other way to communicate? Like I said you should use the event on the DAPM widget. This will require providing operations for the events to the drivers. signature.asc Description: PGP signature
Re: [PATCH v2 1/3] ASoC: hdmi-codec: Add event handler for hdmi TX
Re: [PATCH v2 1/3] ASoC: hdmi-codec: Add event handler for hdmi TX
On Thu, Dec 01, 2022 at 03:06:04PM +, Jiaxin Yu (俞家鑫) wrote:
> On Tue, 2022-11-29 at 17:22 +, Mark Brown wrote:
> > static const struct snd_kcontrol_new
> > mt8186_mt6366_rt1019_rt5682s_controls[] = {
> > SOC_DAPM_PIN_SWITCH("Speakers"),
> > SOC_DAPM_PIN_SWITCH("Headphone"),
> > SOC_DAPM_PIN_SWITCH("Headset Mic"),
> > SOC_DAPM_PIN_SWITCH("HDMI1"),
> > };
> Which operation should I use to inform bridge driver to control audio
> on or off? I'm curious why I don't see .trigger in the structure
> hdmi_codec_ops compared to the structure snd_soc_dai_ops?
You'd need to add a callback that the user of hdmi-codec passes in which
would be triggered by an event on a DAPM widget added in the audio path
rather than trying to shoehorn this into a PCM operation - a big part of
the problem here is that you're trying to add something that doesn't fit
into a PCM operation.
signature.asc
Description: PGP signature
Re: [PATCH v2 1/3] ASoC: hdmi-codec: Add event handler for hdmi TX
Re: [PATCH v2 1/3] ASoC: hdmi-codec: Add event handler for hdmi TX
On Mon, Nov 28, 2022 at 03:07:22PM +, Jiaxin Yu (俞家鑫) wrote: > On Fri, 2022-11-25 at 12:18 +, Mark Brown wrote: > > On Fri, Nov 25, 2022 at 05:44:11PM +0800, Jiaxin Yu wrote: > > I'm a little unclear why this is being implemented as a DAPM > > operation > > rather than having the driver forward the PCM trigger op if it's > > needed? > > Or alternatively if a DAPM callback is needed why not provide one > > directly rather than hooking into the trigger function - that's going > > to > > be called out of sequence with the rest of DAPM and be potentially > > confusing given the very different environments that trigger and DAPM > > operations run in. A quick glance at the it6505 driver suggests it'd > > be > > happier with a DAPM callback. > Let me describe the hardware connection about mt8186 with it6505(hdmi) > and rt1015p(speakers). >==>it6505 > = > DL1(FE) ==>I2S3(BE) = > = >==>rt1015p > They shared the same one i2s port, but we'd like to control them > separately. So if hdmi-codec use the PCM trigger op, whne we turn on > the speaker, hdmi-codec's PCM trigger op is also executed, resulting in > sound on both devices. > Is there another way to control them separately? Thank you. If you just need power control for one or both devices then the machine driver can add a _PIN_SWITCH() on the output of the device, that'll cause DAPM to keep the device powered down when not in use. That should work well with the suggestion to provide a DAPM callback instead of a a trigger operation. signature.asc Description: PGP signature
Re: [PATCH v2 1/3] ASoC: hdmi-codec: Add event handler for hdmi TX
Re: [PATCH v2 1/3] ASoC: hdmi-codec: Add event handler for hdmi TX
On Fri, Nov 25, 2022 at 05:44:11PM +0800, Jiaxin Yu wrote:
> + /*
> + * PCM trigger callback.
> + * Mandatory
> + */
> + int (*trigger)(struct device *dev, int cmd);
> +
Making this mandatory would break all existing users, though...
> + switch (event) {
> + case SND_SOC_DAPM_PRE_PMU:
> + if (hcp->hcd.ops->trigger)
> + hcp->hcd.ops->trigger(component->dev->parent,
> SNDRV_PCM_TRIGGER_START);
...it's not actually mandatory so it's just the comment that's wrong.
I'm a little unclear why this is being implemented as a DAPM operation
rather than having the driver forward the PCM trigger op if it's needed?
Or alternatively if a DAPM callback is needed why not provide one
directly rather than hooking into the trigger function - that's going to
be called out of sequence with the rest of DAPM and be potentially
confusing given the very different environments that trigger and DAPM
operations run in. A quick glance at the it6505 driver suggests it'd be
happier with a DAPM callback.
signature.asc
Description: PGP signature
