Re: [RFC PATCH 01/16] Documentation: driver: add SoundWire BRA description

[Charles Keepax <ckeepax@opensource.cirrus.com>]

On Thu, Dec 07, 2023 at 04:29:29PM -0600, Pierre-Louis Bossart wrote:
> The Bulk Register Access protocol was left as a TODO topic since
> 2018. It's time to document this protocol and the design of its Linux
> support.
> 
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> ---
> +Concurrency between BRA and regular read/write
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +The existing 'nread/nwrite' API already relies on a notion of start
> +address and number of bytes, so it would be possible to extend this
> +API with a 'hint' requesting BPT/BRA be used.
> +
> +However BRA transfers could be quite long, and the use of a single
> +mutex for regular read/write and BRA is a show-stopper. Independent
> +operation of the control/command and BRA transfers is a fundamental
> +requirement, e.g. to change the volume level with the existing regmap
> +interface while downloading firmware.

Is this definitely a show stopper? Not saying that it wouldn't be
desirable to do both from a speed perspective, but current
systems that download firmware (I2C/SPI) typically will block the
bus for some amount of time. There are also some desirable properties
to a single lock such as not needing to worry about accessing the
same register in the bulk transfer and a normal command transfer.

> +Audio DMA support
> +-----------------
> +
> +Some DMAs, such as HDaudio, require an audio format field to be
> +set. This format is in turn used to define acceptable bursts. BPT/BRA
> +support is not fully compatible with these definitions in that the
> +format may vary between read and write commands.
> +
> +In addition, on Intel HDaudio Intel platforms the DMAs need to be
> +programmed with a PCM format matching the bandwidth of the BPT/BRA
> +transfer. The format is based on 48kHz 32-bit samples, and the number
> +of channels varies to adjust the bandwidth. The notion of channel is
> +completely notional since the data is not typical audio
> +PCM. Programming channels helps reserve enough bandwidth and adjust
> +FIFO sizes to avoid xruns. Note that the quality of service comes as a
> +cost. Since all channels need to be present as a sample block, data
> +sizes not aligned to 128-bytes are not supported.

Apologies but could you elaborate a litte on this? I am not sure
I follow the reasoning, how does the 48k 32bit DMA implementation
result in 128-byte limitation? I would have thought 1 channel would
be 4-bytes and you are varying the channels so I would have expected
4-byte aligned maybe 8-byte if the DMA expects stereo pairs.

And what exactly do we mean by aligned, are we saying the length
all transfers needs to be a multiple of 128-bytes?

I think we might have some annoying restrictions on the block
size on our hardware as well I will go dig into that and report
back.

Thanks,
Charles