S905x: No sound with error log.

S905x: No sound with error log.

[张 宁]

Hi,

Recent asoc change makes S905x audio card prob failed.

[    6.338428] gx-sound-card sound: CPU DAI I2S Encoder for rtd be.dai-link-1 does not support capture
[    6.342176] gx-sound-card sound: ASoC: can't create pcm be.dai-link-1 :-22

Good: 5.7.3
Bad: 5.7.6

After read patches, I find suspect:

https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/sound/soc?h=v5.7.8&id=1bb707fbfd5c246028d76b8f11a19dfd118d6306 

this patch doesn’t allow playback only or capture only dai-link. Why?

BR.
Ning.
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: S905x: No sound with error log.

[Pierre-Louis Bossart]

On 7/15/20 5:04 PM, 张 宁 wrote:
> Hi,
> 
> Recent asoc change makes S905x audio card prob failed.
> 
> [    6.338428] gx-sound-card sound: CPU DAI I2S Encoder for rtd be.dai-link-1 does not support capture
> [    6.342176] gx-sound-card sound: ASoC: can't create pcm be.dai-link-1 :-22
> 
> Good: 5.7.3
> Bad: 5.7.6
> 
> After read patches, I find suspect:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/sound/soc?h=v5.7.8&id=1bb707fbfd5c246028d76b8f11a19dfd118d6306
> 
> this patch doesn’t allow playback only or capture only dai-link. Why?

it does, but you need to make sure the capabilities of a dai match the 
capability of a dailink they are part of. The be-dai-link1 can support 
capture but the I2S encoder so doesn't you have an invalid configuration.

Could this be the problem be with this code:

int meson_card_set_be_link(struct snd_soc_card *card,
			   struct snd_soc_dai_link *link,
			   struct device_node *node)
{
	struct snd_soc_dai_link_component *codec;
	struct device_node *np;
	int ret, num_codecs;

	link->no_pcm = 1;
	link->dpcm_playback = 1;
	link->dpcm_capture = 1;

Setting the flags blindly is not so good. A helper was added recently, 
see 25612477d20b ('ASoC: soc-dai: set dai_link dpcm_ flags with a helper')


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: S905x: No sound with error log.

[Jerome Brunet]

On Thu 16 Jul 2020 at 04:37, Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> wrote:

> On 7/15/20 5:04 PM, 张 宁 wrote:
>> Hi,
>> 
>> Recent asoc change makes S905x audio card prob failed.
>> 
>> [    6.338428] gx-sound-card sound: CPU DAI I2S Encoder for rtd be.dai-link-1 does not support capture
>> [    6.342176] gx-sound-card sound: ASoC: can't create pcm be.dai-link-1 :-22
>> 
>> Good: 5.7.3
>> Bad: 5.7.6
>> 
>> After read patches, I find suspect:
>> 
>> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/sound/soc?h=v5.7.8&id=1bb707fbfd5c246028d76b8f11a19dfd118d6306
>> 
>> this patch doesn’t allow playback only or capture only dai-link. Why?
>
> it does, but you need to make sure the capabilities of a dai match the 
> capability of a dailink they are part of. The be-dai-link1 can support 
> capture but the I2S encoder so doesn't you have an invalid configuration.
>
> Could this be the problem be with this code:
>
> int meson_card_set_be_link(struct snd_soc_card *card,
> 			   struct snd_soc_dai_link *link,
> 			   struct device_node *node)
> {
> 	struct snd_soc_dai_link_component *codec;
> 	struct device_node *np;
> 	int ret, num_codecs;
>
> 	link->no_pcm = 1;
> 	link->dpcm_playback = 1;
> 	link->dpcm_capture = 1;
>

Hi Pierre-Louis,

actually this patch changed the meaning of the dpcm_playback and
dpcm_capture from "is_allowed" to "is_required" and it broke this card
and another one for me.

Yes the flag are "blindly" set because with the old meanning, there was
no reason to disallow any direction. Also the card driver only handles
DT phandle and does not know much about the DAI (BE) caps.

Your change in ASoC core was meant to add support for multi cpu BE link
but:
 - it changed the meaning of the flags
 - it forces all CPU on the BE link to share the same direction
 capability. 

> Setting the flags blindly is not so good. A helper was added recently, 
> see 25612477d20b ('ASoC: soc-dai: set dai_link dpcm_ flags with a
> helper')

Same with this change, if any codec does not support a stream direction,
it gets disabled. But it is uncommon to have link with multiple codec,
one for each stream direction. The commit above would break those cards

Could you tell why it was not possible to retain the previous logic
which was : disable the stream direction if
 - dpcm_{capture,playback) is false OR
 - no codec support the direction (same would apply to cpu)

??

>
>
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: 回复: S905x: No sound with error log.

[Jerome Brunet]

On Wed 29 Jul 2020 at 04:16, 张 宁 <zhangn1985@outlook.com> wrote:

> any progress on this issue?
>
> I have tried to remove link->dpcm_capture = 1; this fix the issue, but I don't know whether there any side effect.
>
> my patch: https://github.com/zhangn1985/linux/commit/b88a7df8adfdd80bc6a0802ce64797f17c0a7d32
>

As you may have seen, there is some discussion around this topic ATM.
Until something concrete lands, the easiest thing to do is to revert
b73287f0b074 ("ASoC: soc-pcm: dpcm: fix playback/capture checks")

The card should work again with that. Sorry for the inconvenience.

Jerome

> ________________________________
> 发件人: Jerome Brunet <jbrunet@baylibre.com>
> 发送时间: 2020年7月17日 10:12
> 收件人: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>; 张 宁 <zhangn1985@outlook.com>
> 抄送: linux-amlogic@lists.infradead.org <linux-amlogic@lists.infradead.org>
> 主题: Re: S905x: No sound with error log.
>
>
> On Thu 16 Jul 2020 at 04:37, Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> wrote:
>
>> On 7/15/20 5:04 PM, 张 宁 wrote:
>>> Hi,
>>>
>>> Recent asoc change makes S905x audio card prob failed.
>>>
>>> [    6.338428] gx-sound-card sound: CPU DAI I2S Encoder for rtd be.dai-link-1 does not support capture
>>> [    6.342176] gx-sound-card sound: ASoC: can't create pcm be.dai-link-1 :-22
>>>
>>> Good: 5.7.3
>>> Bad: 5.7.6
>>>
>>> After read patches, I find suspect:
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/sound/soc?h=v5.7.8&id=1bb707fbfd5c246028d76b8f11a19dfd118d6306
>>>
>>> this patch doesn’t allow playback only or capture only dai-link. Why?
>>
>> it does, but you need to make sure the capabilities of a dai match the
>> capability of a dailink they are part of. The be-dai-link1 can support
>> capture but the I2S encoder so doesn't you have an invalid configuration.
>>
>> Could this be the problem be with this code:
>>
>> int meson_card_set_be_link(struct snd_soc_card *card,
>>                           struct snd_soc_dai_link *link,
>>                           struct device_node *node)
>> {
>>        struct snd_soc_dai_link_component *codec;
>>        struct device_node *np;
>>        int ret, num_codecs;
>>
>>        link->no_pcm = 1;
>>        link->dpcm_playback = 1;
>>        link->dpcm_capture = 1;
>>
>
> Hi Pierre-Louis,
>
> actually this patch changed the meaning of the dpcm_playback and
> dpcm_capture from "is_allowed" to "is_required" and it broke this card
> and another one for me.
>
> Yes the flag are "blindly" set because with the old meanning, there was
> no reason to disallow any direction. Also the card driver only handles
> DT phandle and does not know much about the DAI (BE) caps.
>
> Your change in ASoC core was meant to add support for multi cpu BE link
> but:
>  - it changed the meaning of the flags
>  - it forces all CPU on the BE link to share the same direction
>  capability.
>
>> Setting the flags blindly is not so good. A helper was added recently,
>> see 25612477d20b ('ASoC: soc-dai: set dai_link dpcm_ flags with a
>> helper')
>
> Same with this change, if any codec does not support a stream direction,
> it gets disabled. But it is uncommon to have link with multiple codec,
> one for each stream direction. The commit above would break those cards
>
> Could you tell why it was not possible to retain the previous logic
> which was : disable the stream direction if
>  - dpcm_{capture,playback) is false OR
>  - no codec support the direction (same would apply to cpu)
>
> ??
>
>>
>>
>> _______________________________________________
>> linux-amlogic mailing list
>> linux-amlogic@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-amlogic


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic