On 12/2/22 05:26, Richard Fitzgerald wrote: > 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. I forgot about this magic BIT(15) and indeed the Addr_page1/2 values are ignored when issuing non-paged register access. There's really no need to zero-out the page registers, it's completely unnecessary. Nice catch! For the series: Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx> >> 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(-) >>>
