Re: [PATCH v2 0/7] ASoC: Intel: avs: Soundwire support

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

>>> However, I find it easier to move from hardcodes to the basic allocation
>>> approach (seen here, in this very patchset) than the other way around.
>>> The change would consist of updating intel_hw_params() so the PDI
>>> allocation is no longer done there.
>>
>> Change is risky, especially when this touches platforms all the way to
>> CNL across at least 3 generations of Intel hardware since 2018, for no
>> tangible benefit.
> 
> Well, that's why I don't want to mix the 2 patchsets. However, I'm of
> those who is not afraid of editing code that touches older platform - my
> intention wouldn't be to level the code, obviously. From functional
> perspective it should reach status quo. And both teams have their CIs.

CI resources are limited, lab space and maintenance costs result in
older generation moved to 'community support', you'd be lucky if there
is one device per generation...

"If it ain't broke don't fix it" applies here.

There are many other topics to work on with the on-going change from
HDaudio to SoundWire in the PC space, Linux is way way behind Windows as
measured by all the recent bug reports on configurations with new
vendors/new chips - not to mention that there are very few reviewers for
SoundWire...

> Now, I assume most of the discussion touches on patch 2/7, the one with
> sdw_cdns_assign_pdi()/release_pdi(). The main suspect:
> 
> intel_hw_params(dai)
> 	alloc_pdi()
> 	config_pdi()
> 	params_hook()
> 
> is a DAI operation, represents dai->hw_params(). The params hook is
> there to give SOF some location to send IPC from. If intel_hw_params()
> is split, there is no need for hooks. Such hooks are certainly not elegant.

The reason for those hooks is not technical, it's due to the split in
the code between the Soundwire code in drivers/soundwire and the
controller in sound/soc/sof/intel. There's no real other way to send an
IPC from the SoundWire side, it is what it is now.

> I believe the way forward is to convert DAI ops present in intel.c to
> library-type functions. Let platform components call these. Frankly,
> except for our friend, intel_hw_params(), one could say they already are
> - perhaps the whole intel_pcm_dai_ops could be dropped.

No, the intel_pcm_dai_ops was introduced to have intel-bus-common define
a generic behavior for a stream and the ops hide hardware-specific
implementations between CNL and LNL. This has nothing to do with the PDI
really.

There's a second level of indirection to send an IPC with the
dai->hw_params, again the implementation is done differently depending
on generations. Note that in theory on ACE2.x SoundWire could have been
managed without any dependency on the DSP firmware and thus no IPC,
unfortunately decisions made by firmware folks result in the need for
this IPC. It is also what it is now.