Re: [PATCH 2/2] media: verisilicon: change confusingly named relaxed register access
From: Benjamin Gaignard
Date: Tue Jun 20 2023 - 04:01:28 EST
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@xxxxxxxxxxxxx>
Thanks for the patches,
Benjamin
Arnd