Re: [PATCH] sound: sof: ioc4-topology: avoid extra dai_params copy

[Ranjani Sridharan <ranjani.sridharan@xxxxxxxxxxxxxxx>]

On Wed, 2024-08-07 at 17:18 +0200, Arnd Bergmann wrote:
> On Wed, Aug 7, 2024, at 17:09, Mark Brown wrote:
> > On Wed, Aug 07, 2024 at 10:02:27AM +0200, Arnd Bergmann wrote:
> > 
> > > From what I can tell, this was unintentional, as both
> > > sof_ipc4_prepare_dai_copier() and
> > > sof_ipc4_prepare_copier_module() make a
> > > copy for the same purpose, but copying it once has the exact same
> > > effect.
> > 
> > > Remove the extra copy and change the direct struct assignment to
> > > an explicit memcpy() call to make it clearer to the reader that
> > > this
> > > is what happens. Note that gcc treats struct assignment as a
> > > memcpy()
> > > that may be inlined anyway, so the resulting object code is the
> > > same.
> > 
> > The effect of the copy is to ensure that if the function fails the
> > argument is unmodified - did you do the analysis to check that it's
> > OK
> > to modify on error?  Your commit log says "the same purpose" but
> > never
> > specifies what that purpose is.
> 
> There is always a chance that I misunderstood the code, but
> yes, I did understand that the idea is to not modify the
> parameters inside of sof_ipc4_prepare_dai_copier.
Hi Arnd,

The idea behind the local copy is that the DAI widget needs to handle
its audio formats in topology differently from the other widgets in the
pipeline. So, locally the sof_ipc4_prepare_dai_copier() modifies the
params to make sure the right NHLT blobs are chosen based on what's
available in topology and the information passed in the params. But
when the params variable is passed on to the next widget in the
pipeline, any local modifications done by the DAI widget should be
carried forward.

For your reference, this is code that does the propagation of the
prepare callback for each widget in the playback/capture path in the
pipeline.
https://github.com/thesofproject/linux/blob/bc47b82db6e03d540061964d4540a371e7d344c8/sound/soc/sof/sof-audio.c#L442

Thanks,
Ranjani