[PATCH v3 0/2] Chunk splitting of spi transfers

[PATCH v3 0/2] Chunk splitting of spi transfers

[Meghana Madhyastha]

-Call spi_split_transfers_maxsize in __spi_pump_messages
to split large chunks for spi dma transfers.
-Remove chunk splitting in the tinydrm spi helper (as now the core is
handling the chunk splitting).

Changes in v3:
-Remove max_chunk 
-Remove tinydrm_spi_max_transfer_size()                        
-Change master->max_dma_len in patch 0/2 and explain why it is changed

Meghana Madhyastha (2):
  spi: Split spi message into chunks of <65535 in the spi subsystem
  drm/tinydrm: Remove chunk splitting in tinydrm_spi_transfer

 drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c | 80 ++------------------------
 drivers/gpu/drm/tinydrm/mipi-dbi.c             | 10 +---
 drivers/spi/spi-bcm2835.c                      | 22 +++----
 3 files changed, 16 insertions(+), 96 deletions(-)

-- 
2.11.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3 1/2] spi: Split spi message into chunks of <65535 in the spi subsystem

[Meghana Madhyastha]

Split spi messages into chunks of <65535 in the spi subsystem and remove the message
length warning in bcm2835_spi_can_dma. This is so that the messages can be transferred
via dma and that the tinydrm drivers need not split it. The current chunk size is 65532.
This is because the scatter gather segment is required to be multiple of 4. The size
has to be <65535 due to hardware limitations.

Signed-off-by: Meghana Madhyastha <meghana.madhyastha@gmail.com>
---
 drivers/spi/spi-bcm2835.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/spi/spi-bcm2835.c b/drivers/spi/spi-bcm2835.c
index f35cc10772f6..280ffa5aef7e 100644
--- a/drivers/spi/spi-bcm2835.c
+++ b/drivers/spi/spi-bcm2835.c
@@ -365,19 +365,6 @@ static bool bcm2835_spi_can_dma(struct spi_master *master,
 	if (tfr->len < BCM2835_SPI_DMA_MIN_LENGTH)
 		return false;
 
-	/* BCM2835_SPI_DLEN has defined a max transfer size as
-	 * 16 bit, so max is 65535
-	 * we can revisit this by using an alternative transfer
-	 * method - ideally this would get done without any more
-	 * interaction...
-	 */
-	if (tfr->len > 65535) {
-		dev_warn_once(&spi->dev,
-			      "transfer size of %d too big for dma-transfer\n",
-			      tfr->len);
-		return false;
-	}
-
 	/* if we run rx/tx_buf with word aligned addresses then we are OK */
 	if ((((size_t)tfr->rx_buf & 3) == 0) &&
 	    (((size_t)tfr->tx_buf & 3) == 0))
@@ -461,7 +448,7 @@ static void bcm2835_dma_init(struct spi_master *master, struct device *dev)
 
 	/* all went well, so set can_dma */
 	master->can_dma = bcm2835_spi_can_dma;
-	master->max_dma_len = 65535; /* limitation by BCM2835_SPI_DLEN */
+	master->max_dma_len = 65532; /* limitation by BCM2835_SPI_DLEN */
 	/* need to do TX AND RX DMA, so we need dummy buffers */
 	master->flags = SPI_MASTER_MUST_RX | SPI_MASTER_MUST_TX;
 
@@ -597,7 +584,14 @@ static int bcm2835_spi_prepare_message(struct spi_master *master,
 {
 	struct spi_device *spi = msg->spi;
 	struct bcm2835_spi *bs = spi_master_get_devdata(master);
+	gfp_t gfp_flags = GFP_KERNEL | GFP_DMA;
 	u32 cs = bcm2835_rd(bs, BCM2835_SPI_CS);
+	size_t max_transfer_size = 65532;
+	int ret;
+
+	ret = spi_split_transfers_maxsize(master, msg, max_transfer_size, gfp_flags);
+	if (ret)
+	       return ret;
 
 	cs &= ~(BCM2835_SPI_CS_CPOL | BCM2835_SPI_CS_CPHA);
 
-- 
2.11.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3 2/2] drm/tinydrm: Remove chunk splitting in tinydrm_spi_transfer

[Meghana Madhyastha]

Remove chunk splitting in tinydrm_spi_transfer in tinydrm-helpers as the spi core will split
a buffer into max_dma_len chunks for the spi controller driver to handle, automatic byte
swapping in tinydrm_spi_transfer as it doesn't have users.

Signed-off-by: Meghana Madhyastha <meghana.madhyastha@gmail.com>
---
 drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c | 80 ++------------------------
 drivers/gpu/drm/tinydrm/mipi-dbi.c             | 10 +---
 2 files changed, 8 insertions(+), 82 deletions(-)

diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
index bf96072d1b97..d7cbbbe2fda0 100644
--- a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
+++ b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
@@ -334,35 +334,6 @@ EXPORT_SYMBOL(tinydrm_disable_backlight);
 #if IS_ENABLED(CONFIG_SPI)
 
 /**
- * tinydrm_spi_max_transfer_size - Determine max SPI transfer size
- * @spi: SPI device
- * @max_len: Maximum buffer size needed (optional)
- *
- * This function returns the maximum size to use for SPI transfers. It checks
- * the SPI master, the optional @max_len and the module parameter spi_max and
- * returns the smallest.
- *
- * Returns:
- * Maximum size for SPI transfers
- */
-size_t tinydrm_spi_max_transfer_size(struct spi_device *spi, size_t max_len)
-{
-	size_t ret;
-
-	ret = min(spi_max_transfer_size(spi), spi->master->max_dma_len);
-	if (max_len)
-		ret = min(ret, max_len);
-	if (spi_max)
-		ret = min_t(size_t, ret, spi_max);
-	ret &= ~0x3;
-	if (ret < 4)
-		ret = 4;
-
-	return ret;
-}
-EXPORT_SYMBOL(tinydrm_spi_max_transfer_size);
-
-/**
  * tinydrm_spi_bpw_supported - Check if bits per word is supported
  * @spi: SPI device
  * @bpw: Bits per word
@@ -452,62 +423,23 @@ int tinydrm_spi_transfer(struct spi_device *spi, u32 speed_hz,
 	struct spi_transfer tr = {
 		.bits_per_word = bpw,
 		.speed_hz = speed_hz,
+		.tx_buf = buf,
+		.len = len
 	};
 	struct spi_message m;
-	u16 *swap_buf = NULL;
-	size_t max_chunk;
-	size_t chunk;
-	int ret = 0;
-
-	if (WARN_ON_ONCE(bpw != 8 && bpw != 16))
-		return -EINVAL;
-
-	max_chunk = tinydrm_spi_max_transfer_size(spi, 0);
 
 	if (drm_debug & DRM_UT_DRIVER)
-		pr_debug("[drm:%s] bpw=%u, max_chunk=%zu, transfers:\n",
-			 __func__, bpw, max_chunk);
-
-	if (bpw == 16 && !tinydrm_spi_bpw_supported(spi, 16)) {
-		tr.bits_per_word = 8;
-		if (tinydrm_machine_little_endian()) {
-			swap_buf = kmalloc(min(len, max_chunk), GFP_KERNEL);
-			if (!swap_buf)
-				return -ENOMEM;
-		}
-	}
+		pr_debug("[drm:%s] bpw=%u, transfers:\n",
+			__func__, bpw);
 
 	spi_message_init(&m);
 	if (header)
 		spi_message_add_tail(header, &m);
 	spi_message_add_tail(&tr, &m);
 
-	while (len) {
-		chunk = min(len, max_chunk);
-
-		tr.tx_buf = buf;
-		tr.len = chunk;
-
-		if (swap_buf) {
-			const u16 *buf16 = buf;
-			unsigned int i;
-
-			for (i = 0; i < chunk / 2; i++)
-				swap_buf[i] = swab16(buf16[i]);
-
-			tr.tx_buf = swap_buf;
-		}
-
-		buf += chunk;
-		len -= chunk;
-
-		tinydrm_dbg_spi_message(spi, &m);
-		ret = spi_sync(spi, &m);
-		if (ret)
-			return ret;
-	}
+	tinydrm_dbg_spi_message(spi, &m);
 
-	return 0;
+	return spi_sync(spi, &m);
 }
 EXPORT_SYMBOL(tinydrm_spi_transfer);
 
diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c
index 75dd65c57e74..c8af2d65c2ad 100644
--- a/drivers/gpu/drm/tinydrm/mipi-dbi.c
+++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c
@@ -886,15 +886,9 @@ static int mipi_dbi_typec3_command(struct mipi_dbi *mipi, u8 cmd,
 int mipi_dbi_spi_init(struct spi_device *spi, struct mipi_dbi *mipi,
 		      struct gpio_desc *dc)
 {
-	size_t tx_size = tinydrm_spi_max_transfer_size(spi, 0);
 	struct device *dev = &spi->dev;
 	int ret;
 
-	if (tx_size < 16) {
-		DRM_ERROR("SPI transmit buffer too small: %zu\n", tx_size);
-		return -EINVAL;
-	}
-
 	/*
 	 * Even though it's not the SPI device that does DMA (the master does),
 	 * the dma mask is necessary for the dma_alloc_wc() in
@@ -924,8 +918,8 @@ int mipi_dbi_spi_init(struct spi_device *spi, struct mipi_dbi *mipi,
 			mipi->swap_bytes = true;
 	} else {
 		mipi->command = mipi_dbi_typec1_command;
-		mipi->tx_buf9_len = tx_size;
-		mipi->tx_buf9 = devm_kmalloc(dev, tx_size, GFP_KERNEL);
+		mipi->tx_buf9_len = SZ_16K;
+		mipi->tx_buf9 = devm_kmalloc(dev, mipi->tx_buf9_len, GFP_KERNEL);
 		if (!mipi->tx_buf9)
 			return -ENOMEM;
 	}
-- 
2.11.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 1/2] spi: Split spi message into chunks of <65535 in the spi subsystem

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 2715 bytes --]

Meghana Madhyastha <meghana.madhyastha@gmail.com> writes:

> Split spi messages into chunks of <65535 in the spi subsystem and remove the message
> length warning in bcm2835_spi_can_dma. This is so that the messages can be transferred
> via dma and that the tinydrm drivers need not split it. The current chunk size is 65532.
> This is because the scatter gather segment is required to be multiple of 4. The size
> has to be <65535 due to hardware limitations.
>
> Signed-off-by: Meghana Madhyastha <meghana.madhyastha@gmail.com>
> ---
>  drivers/spi/spi-bcm2835.c | 22 ++++++++--------------
>  1 file changed, 8 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/spi/spi-bcm2835.c b/drivers/spi/spi-bcm2835.c
> index f35cc10772f6..280ffa5aef7e 100644
> --- a/drivers/spi/spi-bcm2835.c
> +++ b/drivers/spi/spi-bcm2835.c
> @@ -365,19 +365,6 @@ static bool bcm2835_spi_can_dma(struct spi_master *master,
>  	if (tfr->len < BCM2835_SPI_DMA_MIN_LENGTH)
>  		return false;
>  
> -	/* BCM2835_SPI_DLEN has defined a max transfer size as
> -	 * 16 bit, so max is 65535
> -	 * we can revisit this by using an alternative transfer
> -	 * method - ideally this would get done without any more
> -	 * interaction...
> -	 */
> -	if (tfr->len > 65535) {
> -		dev_warn_once(&spi->dev,
> -			      "transfer size of %d too big for dma-transfer\n",
> -			      tfr->len);
> -		return false;
> -	}
> -
>  	/* if we run rx/tx_buf with word aligned addresses then we are OK */
>  	if ((((size_t)tfr->rx_buf & 3) == 0) &&
>  	    (((size_t)tfr->tx_buf & 3) == 0))
> @@ -461,7 +448,7 @@ static void bcm2835_dma_init(struct spi_master *master, struct device *dev)
>  
>  	/* all went well, so set can_dma */
>  	master->can_dma = bcm2835_spi_can_dma;
> -	master->max_dma_len = 65535; /* limitation by BCM2835_SPI_DLEN */
> +	master->max_dma_len = 65532; /* limitation by BCM2835_SPI_DLEN */
>  	/* need to do TX AND RX DMA, so we need dummy buffers */
>  	master->flags = SPI_MASTER_MUST_RX | SPI_MASTER_MUST_TX;
>  
> @@ -597,7 +584,14 @@ static int bcm2835_spi_prepare_message(struct spi_master *master,
>  {
>  	struct spi_device *spi = msg->spi;
>  	struct bcm2835_spi *bs = spi_master_get_devdata(master);
> +	gfp_t gfp_flags = GFP_KERNEL | GFP_DMA;
>  	u32 cs = bcm2835_rd(bs, BCM2835_SPI_CS);
> +	size_t max_transfer_size = 65532;
> +	int ret;
> +
> +	ret = spi_split_transfers_maxsize(master, msg, max_transfer_size, gfp_flags);
> +	if (ret)
> +	       return ret;

Mark: is this how spi_split_transfers_maxsize() is supposed to be used?
If so, I'm happy to see our hardware have fewer limitations, and it gets
my:

Reviewed-by: Eric Anholt <eric@anholt.net>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel