>>> Are you talking about the change in this commit acbf27746ecfa96b
>>> "ASoC: pcm: update FE/BE trigger order based on the command"?
>>>
>>> We made this change to handle xruns during pause/release particularly on
>>> the Intel HDA platforms.
>>
>> this change was just to mirror the behavior between start/stop, I
>> thought there was a patch where we moved to TRIGGER_POST by default?
>>
>> What I am trying to figure out if whether using TRIGGER_PRE is ok or not
>> for the SOF firmware.
>>
>
> Ahh yes, it was part of the same series as this one. fd274c2b7267b "ASoC:
> SOF: topology: set trigger order for FE DAI link"
>
> TRIGGER_PRE won't really work in the case of SOF. We need the BE DAI to be
> triggered before the FE DAI during start to prevent the xruns during
> pause/release.
Thanks Ranjani. That information closes the door on the idea of playing
with the trigger order suggested earlier in the thread, so my guess is
that we really need to expose the MCLK/BCLK with the clk API and turn
them on/off from the machine driver as needed. I hope is that we don't
need the FSYNC as well, that would be rather painful to implement.
On 11 February 2020 21:49, Pierre-Louis Bossart wrote:
> -----Original Message-----
> From: Pierre-Louis Bossart <[email protected]>
> Sent: 11 February 2020 21:49
> To: Sridharan, Ranjani <[email protected]>
> Cc: [email protected]; Support Opensource
> <[email protected]>; Takashi Iwai <[email protected]>; Liam
> Girdwood <[email protected]>; [email protected]; Chiang, Mac
> <[email protected]>; Mark Brown <[email protected]>; Ranjani Sridharan
> <[email protected]>; Adam Thomson
> <[email protected]>; Lu, Brent <[email protected]>;
> [email protected]
> Subject: Re: [alsa-devel] [PATCH] ASoC: da7219: check SRM lock in trigger callback
> Importance: High
>
>
>
> >>> Are you talking about the change in this commit acbf27746ecfa96b
> >>> "ASoC: pcm: update FE/BE trigger order based on the command"?
> >>>
> >>> We made this change to handle xruns during pause/release particularly on
> >>> the Intel HDA platforms.
> >>
> >> this change was just to mirror the behavior between start/stop, I
> >> thought there was a patch where we moved to TRIGGER_POST by default?
> >>
> >> What I am trying to figure out if whether using TRIGGER_PRE is ok or not
> >> for the SOF firmware.
> >>
> >
> > Ahh yes, it was part of the same series as this one. fd274c2b7267b "ASoC:
> > SOF: topology: set trigger order for FE DAI link"
> >
> > TRIGGER_PRE won't really work in the case of SOF. We need the BE DAI to be
> > triggered before the FE DAI during start to prevent the xruns during
> > pause/release.
>
> Thanks Ranjani. That information closes the door on the idea of playing
> with the trigger order suggested earlier in the thread, so my guess is
> that we really need to expose the MCLK/BCLK with the clk API and turn
> them on/off from the machine driver as needed. I hope is that we don't
> need the FSYNC as well, that would be rather painful to implement.
Am not going to make myself popular here. It's MCLK and FSYNC (or WCLK as it's
termed for our device) that is required for SRM to lock in the PLL.
So far I've not found a way in the codec driver to be able to get around this.
I spent a very long time with Sathya in the early days (Apollo Lake) looking at
options but nothing would fit which is why I have the solution that's in place
right now. We could probably reduce the number of rechecks before timeout in the
driver but that's really just papering over the crack and there's still the
possibility of noise later when SRM finally does lock.
On Wed, Feb 12, 2020 at 10:16:54AM +0000, Adam Thomson wrote:
> So far I've not found a way in the codec driver to be able to get around this.
> I spent a very long time with Sathya in the early days (Apollo Lake) looking at
> options but nothing would fit which is why I have the solution that's in place
> right now. We could probably reduce the number of rechecks before timeout in the
> driver but that's really just papering over the crack and there's still the
> possibility of noise later when SRM finally does lock.
This really needs the componentisation refactoring I think, that way we
can annotate individual devices and links with what they need rather
than essentially guessing about what works most of the time which is
more or less what we do at the minute. Like you say as things are at
the minute there's a lot of crack papering going on.
>> Thanks Ranjani. That information closes the door on the idea of playing
>> with the trigger order suggested earlier in the thread, so my guess is
>> that we really need to expose the MCLK/BCLK with the clk API and turn
>> them on/off from the machine driver as needed. I hope is that we don't
>> need the FSYNC as well, that would be rather painful to implement.
>
> Am not going to make myself popular here. It's MCLK and FSYNC (or WCLK as it's
> termed for our device) that is required for SRM to lock in the PLL.
>
> So far I've not found a way in the codec driver to be able to get around this.
> I spent a very long time with Sathya in the early days (Apollo Lake) looking at
> options but nothing would fit which is why I have the solution that's in place
> right now. We could probably reduce the number of rechecks before timeout in the
> driver but that's really just papering over the crack and there's still the
> possibility of noise later when SRM finally does lock.
Sorry, you lost me at "the solution that's in place right now". There is
nothing in the bxt_da7219_max98357a.c code that deals with clocks or
defines a trigger order?
On 12 February 2020 15:48, Pierre-Louis Bossart wrote:
> >> Thanks Ranjani. That information closes the door on the idea of playing
> >> with the trigger order suggested earlier in the thread, so my guess is
> >> that we really need to expose the MCLK/BCLK with the clk API and turn
> >> them on/off from the machine driver as needed. I hope is that we don't
> >> need the FSYNC as well, that would be rather painful to implement.
> >
> > Am not going to make myself popular here. It's MCLK and FSYNC (or WCLK as
> it's
> > termed for our device) that is required for SRM to lock in the PLL.
> >
> > So far I've not found a way in the codec driver to be able to get around this.
> > I spent a very long time with Sathya in the early days (Apollo Lake) looking at
> > options but nothing would fit which is why I have the solution that's in place
> > right now. We could probably reduce the number of rechecks before timeout in
> the
> > driver but that's really just papering over the crack and there's still the
> > possibility of noise later when SRM finally does lock.
>
> Sorry, you lost me at "the solution that's in place right now". There is
> nothing in the bxt_da7219_max98357a.c code that deals with clocks or
> defines a trigger order?
I meant solution in the context of the codec driver. The approach or
expectation (maybe more suitable wording) for the driver is that the required
clocking can be enabled prior to the DAI widget for the codec being powered via
DAPM as part of an audio path. This approach has been there since the beginning,
for want of a better option, and I've always highlighted this need when
platforms are using our device with SRM.
You're right though that this isn't taken care of in existing mainline Linux
machine files that use this device with SRM.
>
> Am not going to make myself popular here. It's MCLK and FSYNC (or WCLK as
> it's termed for our device) that is required for SRM to lock in the PLL.
>
> So far I've not found a way in the codec driver to be able to get around this.
> I spent a very long time with Sathya in the early days (Apollo Lake) looking at
> options but nothing would fit which is why I have the solution that's in place
> right now. We could probably reduce the number of rechecks before
> timeout in the driver but that's really just papering over the crack and there's
> still the possibility of noise later when SRM finally does lock.
Hi Adam,
For Google CTS requirement (200ms cold output latency), we plan to upload a
patch which reduces the recheck number to 4 and interval to 20ms so the total
delay here would be 80ms for our platform. We think the time is still sufficient
for other platforms to generate a stable WCLK and for the codec SRM to lock but
still needs your confirmation. How do you think?
Regards,
Brent
On 19 February 2020 05:57, Lu, Brent wrote:
> > Am not going to make myself popular here. It's MCLK and FSYNC (or WCLK as
> > it's termed for our device) that is required for SRM to lock in the PLL.
> >
> > So far I've not found a way in the codec driver to be able to get around this.
> > I spent a very long time with Sathya in the early days (Apollo Lake) looking at
> > options but nothing would fit which is why I have the solution that's in place
> > right now. We could probably reduce the number of rechecks before
> > timeout in the driver but that's really just papering over the crack and there's
> > still the possibility of noise later when SRM finally does lock.
>
> Hi Adam,
>
> For Google CTS requirement (200ms cold output latency), we plan to upload a
> patch which reduces the recheck number to 4 and interval to 20ms so the total
> delay here would be 80ms for our platform. We think the time is still sufficient
> for other platforms to generate a stable WCLK and for the codec SRM to lock but
> still needs your confirmation. How do you think?
Hi Brent,
I'm concerned that just setting a timeout to suit the Google CTS requirement
isn't necessarily suitable for all targets, and this doesn't actually fix the
real problem here.
How long do you determine platforms will take to generate a stable WCLK? Do we
have an idea of how long that might be in a worst case scenario? If so then we
can look at adjusting this down, but I'd like to be clear.
>
>
> Regards,
> Brent