Linux-Amlogic Archive on lore.kernel.org
 help / color / Atom feed
* S905x: No sound with error log.
@ 2020-07-15 22:04 张 宁
  2020-07-16  2:37 ` Pierre-Louis Bossart
  0 siblings, 1 reply; 4+ messages in thread
From: 张 宁 @ 2020-07-15 22:04 UTC (permalink / raw)
  To: pierre-louis.bossart; +Cc: linux-amlogic

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: S905x: No sound with error log.
  2020-07-15 22:04 S905x: No sound with error log 张 宁
@ 2020-07-16  2:37 ` Pierre-Louis Bossart
  2020-07-17 10:12   ` Jerome Brunet
  0 siblings, 1 reply; 4+ messages in thread
From: Pierre-Louis Bossart @ 2020-07-16  2:37 UTC (permalink / raw)
  To: 张 宁; +Cc: linux-amlogic



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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: S905x: No sound with error log.
  2020-07-16  2:37 ` Pierre-Louis Bossart
@ 2020-07-17 10:12   ` Jerome Brunet
       [not found]     ` <MWHPR1601MB1343831C2913ECB7D77DB04DCD700@MWHPR1601MB1343.namprd16.prod.outlook.com>
  0 siblings, 1 reply; 4+ messages in thread
From: Jerome Brunet @ 2020-07-17 10:12 UTC (permalink / raw)
  To: Pierre-Louis Bossart, 张 宁; +Cc: linux-amlogic


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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: 回复: S905x: No sound with error log.
       [not found]     ` <MWHPR1601MB1343831C2913ECB7D77DB04DCD700@MWHPR1601MB1343.namprd16.prod.outlook.com>
@ 2020-07-31  8:38       ` Jerome Brunet
  0 siblings, 0 replies; 4+ messages in thread
From: Jerome Brunet @ 2020-07-31  8:38 UTC (permalink / raw)
  To: 张 宁, Pierre-Louis Bossart, Kevin Hilman
  Cc: linux-amlogic, Mark Brown


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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-15 22:04 S905x: No sound with error log 张 宁
2020-07-16  2:37 ` Pierre-Louis Bossart
2020-07-17 10:12   ` Jerome Brunet
     [not found]     ` <MWHPR1601MB1343831C2913ECB7D77DB04DCD700@MWHPR1601MB1343.namprd16.prod.outlook.com>
2020-07-31  8:38       ` 回复: " Jerome Brunet

Linux-Amlogic Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-amlogic/0 linux-amlogic/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-amlogic linux-amlogic/ https://lore.kernel.org/linux-amlogic \
		linux-amlogic@lists.infradead.org
	public-inbox-index linux-amlogic

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-amlogic


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git