2021-07-05 17:44:40

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH] ASoC: add dai_reoder flag to reverse the stop sequence

On Mon, Jul 05, 2021 at 09:28:28PM +0530, Vijendar Mukunda wrote:

> @@ -982,6 +982,7 @@ struct snd_soc_card {
> unsigned int disable_route_checks:1;
> unsigned int probed:1;
> unsigned int component_chaining:1;
> + unsigned int dai_reorder:1;

This feels like it should be a per dai_link option rather than a card
wide option - the system could have a mix of links that do and don't
want this depending on why it's an issue. The name probably also wants
to be more specific to what's being reordered, something like
stop_dma_first for example since it's only for stops and moves the DMA
first.


Attachments:
(No filename) (616.00 B)
signature.asc (499.00 B)
Download all attachments

2021-07-05 18:46:55

by Vijendar Mukunda

[permalink] [raw]
Subject: Re: [PATCH] ASoC: add dai_reoder flag to reverse the stop sequence

On 7/5/21 11:12 PM, Mark Brown wrote:
> On Mon, Jul 05, 2021 at 09:28:28PM +0530, Vijendar Mukunda wrote:
>
>> @@ -982,6 +982,7 @@ struct snd_soc_card {
>> unsigned int disable_route_checks:1;
>> unsigned int probed:1;
>> unsigned int component_chaining:1;
>> + unsigned int dai_reorder:1;
>
> This feels like it should be a per dai_link option rather than a card
> wide option - the system could have a mix of links that do and don't
> want this depending on why it's an issue. The name probably also wants
> to be more specific to what's being reordered, something like
> stop_dma_first for example since it's only for stops and moves the DMA
> first.
>
As per our understanding by going with card wide option is easier rather
than checking dai link name for re-ordering the stop sequence for
specific platforms.
We will rename the flag as "stop_dma_fist" and will post the new version.

2021-07-05 19:31:44

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH] ASoC: add dai_reoder flag to reverse the stop sequence

On Tue, Jul 06, 2021 at 12:30:10AM +0530, Mukunda,Vijendar wrote:

> As per our understanding by going with card wide option is easier rather
> than checking dai link name for re-ordering the stop sequence for
> specific platforms.
> We will rename the flag as "stop_dma_fist" and will post the new version.

Why would we need to check the name for the link? Presumably all
affected AMD cards would just unconditionally set the flag, and other
systems could do what they like?


Attachments:
(No filename) (488.00 B)
signature.asc (499.00 B)
Download all attachments

2021-07-05 19:57:51

by Vijendar Mukunda

[permalink] [raw]
Subject: Re: [PATCH] ASoC: add dai_reoder flag to reverse the stop sequence

On 7/6/21 12:59 AM, Mark Brown wrote:
> On Tue, Jul 06, 2021 at 12:30:10AM +0530, Mukunda,Vijendar wrote:
>
>> As per our understanding by going with card wide option is easier rather
>> than checking dai link name for re-ordering the stop sequence for
>> specific platforms.
>> We will rename the flag as "stop_dma_fist" and will post the new version.
>
> Why would we need to check the name for the link? Presumably all
> affected AMD cards would just unconditionally set the flag, and other
> systems could do what they like?
>

This issue is only observed with older ACP2.x AMD platforms.
To make AMD specific platform change(which uses ACP 2.x IP), As per our
understanding we should only update the flag in ACP DMA driver by adding
flag in snd_pcm_substream structure rather than adding flag in card
structure.
Please suggest us, if there is any better place holder to add
"stop_dma_first" flag.

Due to some problem with mail client, i have received mail response
late. I have posted v2 version already.

Based on your suggestion, We will make the changes and post the new version.

2021-07-06 12:41:42

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH] ASoC: add dai_reoder flag to reverse the stop sequence

On Tue, Jul 06, 2021 at 01:40:59AM +0530, Mukunda,Vijendar wrote:

> To make AMD specific platform change(which uses ACP 2.x IP), As per our
> understanding we should only update the flag in ACP DMA driver by adding
> flag in snd_pcm_substream structure rather than adding flag in card
> structure.
> Please suggest us, if there is any better place holder to add
> "stop_dma_first" flag.

I'd expect this to be configured by the machine driver in the dai_link.
It might need copying over to the substream for runtime use but the core
should do that.


Attachments:
(No filename) (562.00 B)
signature.asc (499.00 B)
Download all attachments