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

[Mark Brown]

On Mon, Feb 19, 2018 at 11:47:23AM +, Meghana Madhyastha wrote:
> -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.
> -Remove automatic byte swapping in tinydrm_spi_transfer as it doesn't have 
> users.
> -Remove the upper bound check on dma transfer length in bcm2835_spi_can_dma().

This looks like a series of related changes that should be split up into
individual commits, probably at least one per bullet point there.

> - /* 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(>dev,
> -   "transfer size of %d too big for dma-transfer\n",
> -   tfr->len);
> - return false;
> - }
> -

The changelog says we're removing this check but doesn't explain why
and...

>   /* 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 = 65528; /* 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;

...this bit of the change isn't mentioned at all?


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] 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.
-Remove automatic byte swapping in tinydrm_spi_transfer as it doesn't have 
users.
-Remove the upper bound check on dma transfer length in bcm2835_spi_can_dma().

Signed-off-by: Meghana Madhyastha 
---
 drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c | 48 --
 drivers/gpu/drm/tinydrm/mipi-dbi.c | 10 ++
 drivers/spi/spi-bcm2835.c  | 15 +---
 3 files changed, 9 insertions(+), 64 deletions(-)

diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c 
b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
index bf96072d1b97..6064099e8e63 100644
--- a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
+++ b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
@@ -452,62 +452,26 @@ 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);
+   max_chunk = SIZE_MAX;
 
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;
-   }
-   }
+   __func__, bpw, max_chunk);
 
spi_message_init();
if (header)
spi_message_add_tail(header, );
spi_message_add_tail(, );
 
-   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, );
-   ret = spi_sync(spi, );
-   if (ret)
-   return ret;
-   }
+   tinydrm_dbg_spi_message(spi, );
 
-   return 0;
+   return spi_sync(spi, );
 }
 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 = >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;
}
diff --git a/drivers/spi/spi-bcm2835.c b/drivers/spi/spi-bcm2835.c
index f35cc10772f6..2fd650891c07 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) {
-