Received-SPF: pass (google.com: domain of linux-kernel+bounces-72847-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <d5fcd8b83f424bc05d8363210632bf8db4c376cb.camel@gmail.com>
Subject: Re: [PATCH v2 4/5] spi: axi-spi-engine: move message compile to
 optimize_message
From: Nuno =?ISO-8859-1?Q?S=E1?= <noname.nuno@gmail.com>
To: David Lechner <dlechner@baylibre.com>, Mark Brown <broonie@kernel.org>
Cc: Martin Sperl <kernel@martin.sperl.org>, David Jander
 <david@protonic.nl>,  Jonathan Cameron <jic23@kernel.org>, Michael
 Hennerich <michael.hennerich@analog.com>, Nuno =?ISO-8859-1?Q?S=E1?=
 <nuno.sa@analog.com>, Alain Volmat <alain.volmat@foss.st.com>, Maxime
 Coquelin <mcoquelin.stm32@gmail.com>, Alexandre Torgue
 <alexandre.torgue@foss.st.com>,  linux-spi@vger.kernel.org,
 linux-kernel@vger.kernel.org,  linux-stm32@st-md-mailman.stormreply.com,
 linux-arm-kernel@lists.infradead.org,  linux-iio@vger.kernel.org, Julien
 Stephan <jstephan@baylibre.com>
Date: Tue, 20 Feb 2024 11:45:55 +0100
In-Reply-To: <20240219-mainline-spi-precook-message-v2-4-4a762c6701b9@baylibre.com>
References: 
	<20240219-mainline-spi-precook-message-v2-0-4a762c6701b9@baylibre.com>
	 <20240219-mainline-spi-precook-message-v2-4-4a762c6701b9@baylibre.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.50.3 (3.50.3-1.fc39) 
Precedence: bulk
MIME-Version: 1.0

On Mon, 2024-02-19 at 16:33 -0600, David Lechner wrote:
> In the AXI SPI Engine driver, compiling the message is an expensive
> operation. Previously, it was done per message transfer in the
> prepare_message hook. This patch moves the message compile to the
> optimize_message hook so that it is only done once per message in
> cases where the peripheral driver calls spi_optimize_message().
>=20
> This can be a significant performance improvement for some peripherals.
> For example, the ad7380 driver saw a 13% improvement in throughput
> when using the AXI SPI Engine driver with this patch.
>=20
> Since we now need two message states, one for the optimization stage
> that doesn't change for the lifetime of the message and one that is
> reset on each transfer for managing the current transfer state, the old
> msg->state is split into msg->opt_state and spi_engine->msg_state. The
> latter is included in the driver struct now since there is only one
> current message at a time that can ever use it and it is in a hot path
> so avoiding allocating a new one on each message transfer saves a few
> cpu cycles and lets us get rid of the prepare_message callback.
>=20
> Signed-off-by: David Lechner <dlechner@baylibre.com>
> ---

Reviewed-by: Nuno Sa <nuno.sa@analog.com>

>=20
> v2 changes: none
>=20
> =C2=A0drivers/spi/spi-axi-spi-engine.c | 40 +++++++++++++++++------------=
-----------
> =C2=A01 file changed, 17 insertions(+), 23 deletions(-)
>=20
> diff --git a/drivers/spi/spi-axi-spi-engine.c b/drivers/spi/spi-axi-spi-e=
ngine.c
> index ca66d202f0e2..6177c1a8d56e 100644
> --- a/drivers/spi/spi-axi-spi-engine.c
> +++ b/drivers/spi/spi-axi-spi-engine.c
> @@ -109,6 +109,7 @@ struct spi_engine {
> =C2=A0	spinlock_t lock;
> =C2=A0
> =C2=A0	void __iomem *base;
> +	struct spi_engine_message_state msg_state;
> =C2=A0	struct completion msg_complete;
> =C2=A0	unsigned int int_enable;
> =C2=A0};
> @@ -499,17 +500,11 @@ static irqreturn_t spi_engine_irq(int irq, void *de=
vid)
> =C2=A0	return IRQ_HANDLED;
> =C2=A0}
> =C2=A0
> -static int spi_engine_prepare_message(struct spi_controller *host,
> -				=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 struct spi_message *msg)
> +static int spi_engine_optimize_message(struct spi_message *msg)
> =C2=A0{
> =C2=A0	struct spi_engine_program p_dry, *p;
> -	struct spi_engine_message_state *st;
> =C2=A0	size_t size;
> =C2=A0
> -	st =3D kzalloc(sizeof(*st), GFP_KERNEL);
> -	if (!st)
> -		return -ENOMEM;
> -
> =C2=A0	spi_engine_precompile_message(msg);
> =C2=A0
> =C2=A0	p_dry.length =3D 0;
> @@ -517,31 +512,22 @@ static int spi_engine_prepare_message(struct spi_co=
ntroller
> *host,
> =C2=A0
> =C2=A0	size =3D sizeof(*p->instructions) * (p_dry.length + 1);
> =C2=A0	p =3D kzalloc(sizeof(*p) + size, GFP_KERNEL);
> -	if (!p) {
> -		kfree(st);
> +	if (!p)
> =C2=A0		return -ENOMEM;
> -	}
> =C2=A0
> =C2=A0	spi_engine_compile_message(msg, false, p);
> =C2=A0
> =C2=A0	spi_engine_program_add_cmd(p, false, SPI_ENGINE_CMD_SYNC(
> =C2=A0						AXI_SPI_ENGINE_CUR_MSG_SYNC_ID));
> =C2=A0
> -	st->p =3D p;
> -	st->cmd_buf =3D p->instructions;
> -	st->cmd_length =3D p->length;
> -	msg->state =3D st;
> +	msg->opt_state =3D p;
> =C2=A0
> =C2=A0	return 0;
> =C2=A0}
> =C2=A0
> -static int spi_engine_unprepare_message(struct spi_controller *host,
> -					struct spi_message *msg)
> +static int spi_engine_unoptimize_message(struct spi_message *msg)
> =C2=A0{
> -	struct spi_engine_message_state *st =3D msg->state;
> -
> -	kfree(st->p);
> -	kfree(st);
> +	kfree(msg->opt_state);
> =C2=A0
> =C2=A0	return 0;
> =C2=A0}
> @@ -550,10 +536,18 @@ static int spi_engine_transfer_one_message(struct
> spi_controller *host,
> =C2=A0	struct spi_message *msg)
> =C2=A0{
> =C2=A0	struct spi_engine *spi_engine =3D spi_controller_get_devdata(host)=
;
> -	struct spi_engine_message_state *st =3D msg->state;
> +	struct spi_engine_message_state *st =3D &spi_engine->msg_state;
> +	struct spi_engine_program *p =3D msg->opt_state;
> =C2=A0	unsigned int int_enable =3D 0;
> =C2=A0	unsigned long flags;
> =C2=A0
> +	/* reinitialize message state for this transfer */
> +	memset(st, 0, sizeof(*st));
> +	st->p =3D p;
> +	st->cmd_buf =3D p->instructions;
> +	st->cmd_length =3D p->length;
> +	msg->state =3D st;
> +
> =C2=A0	reinit_completion(&spi_engine->msg_complete);
> =C2=A0
> =C2=A0	spin_lock_irqsave(&spi_engine->lock, flags);
> @@ -658,8 +652,8 @@ static int spi_engine_probe(struct platform_device *p=
dev)
> =C2=A0	host->bits_per_word_mask =3D SPI_BPW_RANGE_MASK(1, 32);
> =C2=A0	host->max_speed_hz =3D clk_get_rate(spi_engine->ref_clk) / 2;
> =C2=A0	host->transfer_one_message =3D spi_engine_transfer_one_message;
> -	host->prepare_message =3D spi_engine_prepare_message;
> -	host->unprepare_message =3D spi_engine_unprepare_message;
> +	host->optimize_message =3D spi_engine_optimize_message;
> +	host->unoptimize_message =3D spi_engine_unoptimize_message;
> =C2=A0	host->num_chipselect =3D 8;
> =C2=A0
> =C2=A0	if (host->max_speed_hz =3D=3D 0)
>=20