Re: [PATCH 2/3] ASoC: soc-pcm: add soc_pcm_hw_update_chan()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hey,

On Mon, 15 Feb 2021, Mark Brown wrote:

> On Sun, Feb 14, 2021 at 11:17:03PM +0200, Kai Vehmanen wrote:
> > On Fri, 12 Feb 2021, Pierre-Louis Bossart wrote:
> > > This patch seems to break all SOF platforms. I tested manually to try and
> > > reproduce the CI results and it's indeed not so good:
> > The new helper functions seem all correct, but the problem would seem to 
> > be in the dpcm_init_runtime_hw() as some of the inputs are not initialized 
> 
> I've applied the fixup patch, if someone could confirm that the CI looks
> good I'll send the pull request for this release.

SOF CI now has had multiple all-passed runs with the fixup included while 
it fails systematically without it:
https://sof-ci.01.org/linuxpr/PR2756/build5292/devicetest/

Morimoto-san, did you have a look at the code changes? 

I think we might still have issues if we have multiple CPU DAIs per
runtime and dpcm_init_runtime_hw() is called multiple times. With the 
fixup, the limits are taken from the last CPU DAI.

But if you look at original code, the same issues seems to be there
(rate and channels taken from the stream directly with no consideration
of already set values). Only exception is runtime->hw.formats, which
is handled differently:

»       runtime->hw.rate_min = stream->rate_min;
»       runtime->hw.rate_max = min_not_zero(stream->rate_max, UINT_MAX);
»       runtime->hw.channels_min = stream->channels_min;
»       runtime->hw.channels_max = stream->channels_max;
»       if (runtime->hw.formats)
»       »       runtime->hw.formats &= stream->formats;
»       else
»       »       runtime->hw.formats = stream->formats;
»       runtime->hw.rates = stream->rates;

... so the fixup also resets hw.formats, but it does not seem logical to 
have different treatment only for hw.formats in the mult-CPU-DAI case.

But yes, more reviewers are welcome. 

Br, Kai



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux