On 2/21/19 12:47 PM, Mark Brown wrote:

On Wed, Feb 20, 2019 at 03:32:54PM -0600, Pierre-Louis Bossart wrote:

...

The only reason for extending the struct snd_soc_pcm_runtime was to enable the use of a 'context' in PCM operations, see e.g in patch 5/14 the repeated pattern to get an 'SOF PCM' context: struct snd_sof_pcm *spcm = rtd->private;

The reason you're having trouble with this is that the DSP isn't really properly visible in the system, it's pushed in on the side with DPCM coexisting with multiple other drivers so as far as I can see it's hard for it to fit in anywhere in Intel systems.

...

So ironically in ASoC we don't have a means to use a private_data field for the substreams since it's used for a 'standard' mechanism.

This is why there's things like snd_soc_dai_get_dma_data() and snd_soc_component_get_drvdata() - if you're a fully visible component in the system there's ways to get to your ASoC specific driver data. For something like SoF that'd mean cooperating with the drivers for the devices with the SoF using DSPs in some way.

...

The extension suggested in this patch 1/14 was really trying to come back to a practical means to store a context in a substream, and make the PCM operations manageable. I totally agree that extending the PCM runtime might not recommended or safe, but at this point I don't have any slack in the existing data structures to add a context in a substream. Does this help clarify the problem statement?

It is fairly clear what driver data is useful for. The issue is that instead of getting to it through the relevant hardware component you're adding something directly in the core data structure, whatever way we look at it that's going to be awkward. Consider for example what happens if someone integrates a system with a SoF DSP in the SoC and another SoF DSP in the CODEC - there will be two separate copies of SoF on a single DAI so a SoF specific value directly in the runtime isn't going to work.

If there were a component for the DSP (or the DSP was fully part of some other existing component) then it should be possible to arrange to get to the DSP data using that but with it just floating there unattached to anything it's harder.

After spending a bit more time on this, I agree it's possible to retrieve the required information without any change to the rtd structure. There are existing precedents using substream->runtime->private_data, e.g. to store an HDAudio stream tag, and SOF already has a list of contexts that can be searched for with a simple list walk.

Modeling the DSP differently is something I'll add to my growing-by-the-day TODO list.