Re: [PATCH v3 01/23] ASoC: soc-pcm: cleanup soc_get_playback_capture()

[Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>]

Hi Pierre-Louis

Thank you for your feedback and report

> On some Chromebooks, we tag a dailink as supporting capture for echo
> reference, but that echo reference is generated by the Intel firmware.
> The amplifiers only support playback.
> 
> see https://github.com/thesofproject/linux/pull/4937 for details, I
> added a clear log:
> 
> [  807.304740] kernel:  SSP1-Codec: CPU dai SSP1 Pin supports capture
> but codec DAI rt1011-aif does not
> 
> So I think for now we probably want to avoid this stricter criterion and
> only check the supported direction with the cpu-dais. Or we add an
> escape mechanism to let the core know that the capture support was
> intentional.

I think my patch have escape mechanism, but it was for BE Codec.
If my understand was correct, above is FE Codec ?

One question is that is it just a mistake (no one noticed) ? or is there
some reasons it can't support capture (but use it ?). If it was just a
mistake, is it possible to update driver during the grace time ?
Or do you think we need "escape mechanism" permanently ?

> I agree with your analysis. We don't have a clear memory/understanding
> of which "no_chan_DAI" platforms (B) was supposed to handle, and why no
> one reported them as broken by (C).
(snip)
> I am good with that plan, but I'll need to investigate first why we had
> a failure with one of the Chromebooks on this v3 patchset. That may give
> us some insights on "special" uses of those flags.

Thanks.
It seems the code was just complicated, and we have been missing
important check. Let's break out my patch-set into small pieces,
and go more slowly.

I think it will be like below. Can you agree about this ?

Step1
	dpcm_xxx flag will be "option flag" instead of "mandatory flag"
	for a while to keep compatibility and avoide confusion.
	But it will be removed in Step3. To indicate such things,
	it will have dev_warn() if dpcm_xxx flag was used. like below

	if (rtd->dai_link->dynamic || rtd->dai_link->no_pcm) {
		has_playback = /* at least one of CPU DAI supports playback */
		has_capture  = ...

		if (!playback && rtd->dai_link->dpcm_playback) {
			dev_warn(dev, "Playback is requested, but CPU doesn't support it\n")
			has_playback = 1;
		}
		...

Step2 (In case of "escape mechanism" is not needed)

	Current DPCM is checking CPU side only. Indicate warning until
	Step4 if Codec is not match . warning only, not error for a while.
	
	if (rtd->dai_link->dynamic || rtd->dai_link->no_pcm) {
		...

		if (has_playback && /* no Codec DAI supports playback */)
			dev_warn(dev, "Playback is requested, but Codec doesn't support it\n")
		...

Step3

	Step1 deadline
	remove dpcm_xxx flag

Step4
	Step2 deadline
	CPU / Codec mismatch will be error.
	It will be "at least one of CPU/Codec pair supports playback/capture"

Step5

	There is no big diff between DPCM / non-DPCM check.
	Merge these, and clean-up code (soc_get_playback_capture())


Thank you for your help !!

Best regards
---
Renesas Electronics
Ph.D. Kuninori Morimoto