Re: [Sound-open-firmware] [PATCH v4 01/14] ASoC: SOF: Add Sound Open Firmware driver core

[Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>]

On 2/20/19 10:26 AM, Mark Brown wrote:
> On Wed, Feb 20, 2019 at 08:35:16AM -0600, Pierre-Louis Bossart wrote:
>
>> I see your point and agree the current direction isn't scalable, but I am
>> not sure about the definition of 'first class device'.
>> Did you mean 'component' at the same level as the 'platforms' and 'codecs',
>> with all the DSP stuff nicely packaged and maintained in a self-contained
>> way?
>> Or a subcomponent that might be part of those platforms and codecs?
> As far as I can see for the Intel systems it's looking like a first
> class component - there will be some systems where the DSP is embedded
> in something (like the big CODECs) but even there it's easier as it's
> contained within the device rather than affecting multiple devices which
> is the big issue here.

Sorry Mark, I am struggling with the 'first class' idea.

Allow me to go back to the practical issue we faced here, and 
maybe/hopefully this will help us converge on a consensus.

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;

Managing a context for PCM ops is typically done in other drivers by 
using the substream->private_data aka snd_pcm_substream_chip(substream).

In ASoC/DPCM, unfortunately this substream->private_data field was 
reclaimed to store a pointer to a 'struct snd_soc_pcm_runtime', see e.g. 
in soc-pcm.c:

pcm->private_data = rtd;

         if (playback)
pcm->streams[SNDRV_PCM_STREAM_PLAYBACK].substream->private_data = rtd;
         if (capture)
pcm->streams[SNDRV_PCM_STREAM_CAPTURE].substream->private_data = rtd;

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.

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?