Re: [PATCH 0/2] soundwire: Remove redundant zeroing of page registers

[Richard Fitzgerald <rf@opensource.cirrus.com>]

On 01/12/2022 18:31, Pierre-Louis Bossart wrote:
> 
> 
> On 12/1/22 08:08, Richard Fitzgerald wrote:
>> Writing zero to the page registers after each message transaction can add
>> up to a lot of overhead for codecs that need to transfer large amount of
>> data - for example a firmware download.
>>
>> There's no spec reason I can see for this zeroing. The page registers are
>> only used for a paged address. The bus code uses a non-paged address for
>> registers in page 0. It always writes the page registers at the start of
>> a paged transaction.
>>
>> If this zeroing was a workaround for anything, let me know and I will
>> re-implement the zeroing as a quirk that can be enabled only when it is
>> necessary.
> 
> It's a feature, not a bug :-)
> 
> The page registers have to be zeroed out so that any bus-management
> command hits the page0 instead of using a value that was set by codec
> driver for vendor-specific configurations.
> 

Why would these bus management commands set bit 15 to indicate a paged
access? If they don't set bit 15 the page registers are not used and
bits 15..31 of the register address must be 0. Table 78 in the Soundwire
1.2 spec. Table 71 in the 1.0 spec. Table 43 in the 0.6 draft spec.


> The implementation is far from optimal though, and indeed if we have
> long transactions that are not interrupted by anything else we could
> avoid resetting the page registers.
> 
> I tried to implement a 'lazy approach' some time back, but at the time I
> didn't see any benefits due to the limited number of configurations.
> 
> I can't remember where the code is, but the initial enhancement was
> listed here: https://github.com/thesofproject/linux/issues/2881
> 
>>
>> Richard Fitzgerald (2):
>>    soundwire: bus: Don't zero page registers after every transaction
>>    soundwire: bus: Remove unused reset_page_addr() callback
>>
>>   drivers/soundwire/bus.c             | 23 -----------------------
>>   drivers/soundwire/cadence_master.c  | 14 --------------
>>   drivers/soundwire/cadence_master.h  |  3 ---
>>   drivers/soundwire/intel_auxdevice.c |  1 -
>>   include/linux/soundwire/sdw.h       |  3 ---
>>   5 files changed, 44 deletions(-)
>>