Messages in this thread | | | Date | Tue, 20 Jun 2023 10:00:56 +0200 | Subject | Re: [PATCH 2/2] media: verisilicon: change confusingly named relaxed register access | From | Benjamin Gaignard <> |
| |
Le 19/06/2023 à 21:26, Arnd Bergmann a écrit : > On Mon, Jun 19, 2023, at 20:29, Nicolas Dufresne wrote: >> Le lundi 19 juin 2023 à 16:49 +0200, Arnd Bergmann a écrit : >>>> In this text you spoke about potential performance side effects of existing code >>>> and your changes, but its left all very vague and theoretical. Have you done any >>>> measurement ? Do you need help with the manner ? >>> I don't have this hardware and have not done any measurements. >>> Obviously the only point of using relaxed accessors is to >>> improve performance in critical code paths, but from the way they >>> are used here it seems that this was instead just an accident >>> and nobody else did any comparisons either. >>> >>> My guess would be that if one wanted to speed up the register >>> access, a better way would be to use a regmap cache to avoid >>> reading registers when the contents are already known. >> All I know is that for the majority of registers when programming stateless >> codecs, each 32bit word of registers are fully written too, the read value is >> not always meaningful (its a value from last time the HW has been triggered) and >> should be ignored, so better to not do that. As for regmap, there is folks that >> have reported regmap to be completely overkill for this type of hardware. > Right, most likely neither the cache nor avoiding the readl() is necessary, > and that was exactly my point to start with: don't add potentially dangerous > microoptimizations like relaxed accessors unless the obvious optimizations > are also needed and used. > > Obviously, testing my patch would still be a good idea before applying it.
I have test the patches on IMX8M (HEVC decoder) and RK3588 (AV1 decoder). I notice not regression or problems, conformance tests scores remain identical.
For the both patches: Tested-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
Thanks for the patches, Benjamin
> > Arnd
| |