Re: [PATCH v4 2/4] spi: Split spi message into max_dma_len size chunks

[Lukas Wunner <lukas@xxxxxxxxx>]

On Thu, Apr 11, 2019 at 06:42:33PM +0200, Noralf Trønnes wrote:
> @@ -1299,6 +1299,11 @@ static void __spi_pump_messages(struct spi_controller *ctlr, bool in_kthread)
>  
>  	trace_spi_message_start(ctlr->cur_msg);
>  
> +	ret = spi_split_transfers_maxsize(ctlr, ctlr->cur_msg, ctlr->max_dma_len,
> +					  GFP_KERNEL | GFP_DMA);

Um, shouldn't this be ifdef'ed to CONFIG_HAS_DMA?


> Some drivers like spi_bcm2835 have a max size on DMA transfers. Work
> around this by splitting up the transfer if necessary.

This feels like a very expensive solution to the problem:  Large transfers
are split into multiple smaller transfers (requiring lots of overhead to
allocate and populate the structures) and the split transfers seem to be
cached for later reuse.

I'm wondering, for a frame buffer, doesn't the buffer (and hence the SPI
message) change on every transmission?  In other words, does a frame
buffer benefit from the caching?  If not, then the caching seems to incur
unnecessary overhead.

Even the normal case when no transfers exceed the limit is affected by
additional overhead, namely an iteration over the transfer list to
check each transfers' length. :-(

Note that spi_map_buf() already splits every transfer's sglist into
segments that are smaller than ctlr->max_dma_len.  Now all that needs
to be done is to amend spi-bcm2835.c to iterate over the sglist
and transmit it in portions which do not exceed 65535.  Addressing the
problem at this lower level would drastically reduce the overhead
compared to the approach in the present patch and hence appears to be
more recommendable.

Thanks,

Lukas