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

[Xianwei Zhao <xianwei.zhao@amlogic.com>]

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

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

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic