Re: [PATCH v4 2/3] spi: Add Amlogic SPISG driver

[Xianwei Zhao]

On 2025/7/17 00:25, Da Xue wrote:
> [ EXTERNAL EMAIL ]
>
> On Wed, Jul 16, 2025 at 5:30 AM Xianwei Zhao <xianwei.zhao@xxxxxxxxxxx> 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@xxxxxxxxxx>
> > > > Date:
> > > > 2025/7/8 21:50
> > > >
> > > > To:
> > > > Xianwei Zhao <xianwei.zhao@xxxxxxxxxxx>
> > > > CC:
> > > > Sunny Luo <sunny.luo@xxxxxxxxxxx>, Rob Herring <robh@xxxxxxxxxx>,
> > > > Krzysztof Kozlowski <krzk+dt@xxxxxxxxxx>, Conor Dooley
> > > > <conor+dt@xxxxxxxxxx>, linux-amlogic@xxxxxxxxxxxxxxxxxxx,
> > > > linux-spi@xxxxxxxxxxxxxxx, devicetree@xxxxxxxxxxxxxxx,
> > > > linux-kernel@xxxxxxxxxxxxxxx
> > > >
> > > >
> > > >
> > > > 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.

For the new SPISG driver, regardless of the mode(PIO, DMA(MEM or sg), 
the configuration registers are the same; the only difference value  in 
the mode selection. see reg cfg_start.

> > 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@xxxxxxxxxxxxxxxxxxx
> > http://lists.infradead.org/mailman/listinfo/linux-amlogic