Re: [RFC PATCH 08/16] soundwire: bus: add bpt_stream pointer

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

On 12/21/23 15:39, Vinod Koul wrote:
> On 18-12-23, 14:20, Pierre-Louis Bossart wrote:
>>
>>
>> On 12/18/23 05:55, Vinod Koul wrote:
>>> On 07-12-23, 16:29, Pierre-Louis Bossart wrote:
>>>> Add a convenience pointer to the 'sdw_bus' structure. BPT is a
>>>> dedicated stream which will typically not be handled by DAIs or
>>>> dailinks. Since there's only one BPT stream per link, storing the
>>>> pointer at the link level seems rather natural.
>>>>
>>>> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>
>>>> ---
>>>>  include/linux/soundwire/sdw.h | 2 ++
>>>>  1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
>>>> index e54c5bbd2b91..8db0cd7d0d89 100644
>>>> --- a/include/linux/soundwire/sdw.h
>>>> +++ b/include/linux/soundwire/sdw.h
>>>> @@ -965,6 +965,7 @@ struct sdw_master_ops {
>>>>   * @stream_refcount: number of streams currently using this bus
>>>>   * @btp_stream_refcount: number of BTP streams currently using this bus (should
>>>>   * be zero or one, multiple streams per link is not supported).
>>>> + * @bpt_stream: pointer stored for convenience.
>>>>   */
>>>>  struct sdw_bus {
>>>>  	struct device *dev;
>>>> @@ -996,6 +997,7 @@ struct sdw_bus {
>>>>  	int hw_sync_min_links;
>>>>  	int stream_refcount;
>>>>  	int bpt_stream_refcount;
>>>> +	struct sdw_stream_runtime *bpt_stream;
>>>
>>> So we are limiting to single stream? Can we have multiple transfers
>>> queued up, which I guess might imply multiple streams?
>>
>>
>> Yes for now there is a BTP/BRA single stream active when there are no
>> audio transfers taking place. This is the only way to guarantee
>> predictable download/resume times.
>>
>> There is no mechanism to queue up transfers, be it from the same
>> peripheral device or different ones. It would be up to the peripheral
>> driver to wait for the BTP stream to be available.
>>
>> Adding a queuing mechanism is a bridge too far for now, most platforms
>> only have 1 or 2 devices only, and a peripheral driver may or may not be
>> ok with delayed downloads. For now the main ask is to reduce download
>> times, a single stream is already a good start. There are other
>> refinements we need to add as well, such as changing clocks to use the
>> fastest gear. I'd like to proceed with baby steps...
> 
> Since the API is async in nature, I think it is very reasonable that we
> add the queue support (a simple list would do) and notify when the
> transfer is complete..

On paper queueing is nice/elegant.

In practice it's likely that there's a bit of configuration needed
before/after each download, e.g. restart the DSP.

It would also be an absolute horror to deal with error flow if one of
the queued transfers did not succeed.

I would be more than happy if we successfully enable a single transfer
across multiple platforms, and look at queueing as a optimization.

BTW even if there are multiple transfers queued, there's still only one
stream active at a given time.