Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Feedback-ID: i56a14606:Fastmail
User-Agent: Cyrus-JMAP/3.9.0-alpha0-496-g8c46984af0-fm-20230615.001-g8c46984a
Mime-Version: 1.0
Message-Id: <063a8886-fd31-425f-901c-fc830512eca3@app.fastmail.com>
In-Reply-To: <e264ac3a15e0f115aa7e941a77eb312429b8f65e.camel@collabora.com>
References: <20230616144854.3818934-1-arnd@kernel.org>
 <20230616144854.3818934-2-arnd@kernel.org>
 <e264ac3a15e0f115aa7e941a77eb312429b8f65e.camel@collabora.com>
Date:   Mon, 19 Jun 2023 16:49:41 +0200
From:   "Arnd Bergmann" <arnd@arndb.de>
To:     "Nicolas Dufresne" <nicolas.dufresne@collabora.com>,
        "Arnd Bergmann" <arnd@kernel.org>,
        "Ezequiel Garcia" <ezequiel@vanguardiasur.com.ar>,
        "Philipp Zabel" <p.zabel@pengutronix.de>,
        "Mauro Carvalho Chehab" <mchehab@kernel.org>
Cc:     "Hans Verkuil" <hverkuil-cisco@xs4all.nl>,
        "Benjamin Gaignard" <benjamin.gaignard@collabora.com>,
        "Jernej Skrabec" <jernej.skrabec@gmail.com>,
        linux-media@vger.kernel.org, linux-rockchip@lists.infradead.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] media: verisilicon: change confusingly named relaxed register
 access
Content-Type: text/plain;charset=utf-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Mon, Jun 19, 2023, at 16:41, Nicolas Dufresne wrote:
> Le vendredi 16 juin 2023 =C3=A0 16:48 +0200, Arnd Bergmann a =C3=A9cri=
t=C2=A0:
>> From: Arnd Bergmann <arnd@arndb.de>
>>=20
>> The register abstraction has wrappers around both the normal writel()
>> and its writel_relaxed() counterpart, but this has led to a lot of us=
ers
>> ending up with the relaxed version.
>>=20
>> There is sometimes a need to intentionally pick the relaxed accessor =
for
>> performance critical functions, but I noticed that each hantro_reg_wr=
ite()
>> call also contains a non-relaxed readl(), which is typically much more
>> expensive than a writel, so there is little benefit here but an added
>> risk of missing a serialization against DMA.
>>=20
>> To make this behave like other interfaces, use the normal accessor by
>> default and only provide the relaxed version as an alternative for
>> performance critical code. hantro_postproc.c is the only place that
>> used both the relaxed and normal writel, but this does not seem
>> cricital either, so change it all to the normal ones.
>
> In this text you spoke about potential performance side effects of exi=
sting code
> and your changes, but its left all very vague and theoretical. Have yo=
u 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.

     Arnd