Message-ID: <CACdvmAizF9OcdBi=Qw38NH+YHT=4edQzJGuQNvxm+m5xqZE0nQ@mail.gmail.com>
Date: Wed, 16 Jul 2025 12:25:58 -0400
From: Da Xue <da@...sconfused.com>
To: Xianwei Zhao <xianwei.zhao@...ogic.com>
Cc: Mark Brown <broonie@...nel.org>, Sunny Luo <sunny.luo@...ogic.com>, 
	Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	linux-amlogic@...ts.infradead.org, linux-spi@...r.kernel.org, 
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 2/3] spi: Add Amlogic SPISG driver

On Wed, Jul 16, 2025 at 5:30 AM Xianwei Zhao <xianwei.zhao@...ogic.com> wrote:
>
> Hi Mark,
>
> On 2025/7/9 15:02, Xianwei Zhao wrote:
> > Hi Mark,
> >     Thanks for your advice.
> >
> > On 2025/7/8 21:50, Mark Brown wrote:
> >> Subject:
> >> Re: [PATCH v4 2/3] spi: Add Amlogic SPISG driver
> >> From:
> >> Mark Brown <broonie@...nel.org>
> >> Date:
> >> 2025/7/8 21:50
> >>
> >> To:
> >> Xianwei Zhao <xianwei.zhao@...ogic.com>
> >> CC:
> >> Sunny Luo <sunny.luo@...ogic.com>, Rob Herring <robh@...nel.org>,
> >> Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
> >> <conor+dt@...nel.org>, linux-amlogic@...ts.infradead.org,
> >> linux-spi@...r.kernel.org, devicetree@...r.kernel.org,
> >> linux-kernel@...r.kernel.org
> >>
> >>
> >>
> >> On Tue, Jul 08, 2025 at 06:34:02PM +0800, Xianwei Zhao wrote:
> >>> On 2025/7/7 21:05, Mark Brown wrote:
> >>>> Is it worth having a copybreak such that smaller transfers are done
> >>>> using PIO?  With a lot of controllers that increases performance due to
> >>>> the extra overhead of setting up DMA, talking to the DMA and interrupt
> >>>> controllers can be as expensive as directly accessing the FIFOs.
> >>> If the data volume of a single transfer (xfer) is small, PIO mode
> >>> does offer
> >>> some advantages. However, since PIO requires the CPU to wait in a
> >>> busy loop
> >>> for the transfer to complete, it continuously occupies CPU resources.
> >>> As a
> >>> result, its advantages are not particularly significant.
> >> The CPU overhead tends to be higher (you can avoid some of it with a
> >> dead reckoning sleep), but the latency vastly improved which for many
> >> applications is a worthwhile advantage.  It tends to be things like
> >> accesses to one or two registers on a device with registers where this
> >> wins, 16 bytes or lower would be a common number off the top of my head.
> >>
> >>> If PIO is to be implemented, it can only handle one transfer at a
> >>> time (via
> >>> transfer_one), and not entire messages (which consist of multiple
> >>> transfers). In contrast, when processing messages, the SPI controller
> >>> can
> >>> handle the entire sequence in one go, which also provides certain
> >>> benefits.
> >> It's probably worth adding something to the framework to be able to take
> >> a decision at the message level, for writes this tends to all fall out
> >> naturally since the write will tend to be a single transfer anyway.
> >
> > I will try to add new API message_can_dma for framework, and implement
> > PIO for message.
> >
>
> I tried to implement PIO mode in the driver, but it turned out to be too
> slow. Due to the lack of an internal FIFO, data could only be
> transmitted one word at a time, and each transmission required
> reconfiguring the corresponding registers. As a result, the efficiency
> was quite low.

One of the use cases is for SPI-based displays and it transfers one
word via PIO and then a lot of words via DMA. I see PIO as beneficial
for this common use case for SPI.
The efficiency does not need to be high for this one word but reducing
the latency for DMA setup is a significant gain.

>
> The only simplifications provided by PIO mode were in two areas:
>
> 1. The allocation and release of the transfer descriptor
> 2. The DMA mapping and unmapping process
>
> Therefore, I suggest not implementing PIO mode in this driver. I will
> document clearly in the code that PIO mode is not supported and explain
> the reasons behind this decision.
>
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic

Powered by blists - more mailing lists