* [PATCH] tinydrm/mipi-dbi: Use dma-safe buffers for all SPI transfers
@ 2019-02-22 12:43 Noralf Trønnes
2019-02-22 15:58 ` Andy Shevchenko
0 siblings, 1 reply; 6+ messages in thread
From: Noralf Trønnes @ 2019-02-22 12:43 UTC (permalink / raw)
To: dri-devel; +Cc: andriy.shevchenko, david
Buffers passed to spi_sync() must be dma-safe even for tiny buffers since
some SPI controllers use DMA for all transfers.
Example splat with CONFIG_DMA_API_DEBUG enabled:
[ 23.750467] DMA-API: dw_dmac_pci 0000:00:15.0: device driver maps memory from stack [probable addr=000000001e49185d]
[ 23.750529] WARNING: CPU: 1 PID: 1296 at kernel/dma/debug.c:1161 check_for_stack+0xb7/0x190
[ 23.750533] Modules linked in: mmc_block(+) spi_pxa2xx_platform(+) pwm_lpss_pci pwm_lpss spi_pxa2xx_pci sdhci_pci cqhci intel_mrfld_pwrbtn extcon_intel_mrfld sdhci intel_mrfld_adc led_class mmc_core ili9341 mipi_dbi tinydrm backlight ti_ads7950 industrialio_triggered_buffer kfifo_buf intel_soc_pmic_mrfld hci_uart btbcm
[ 23.750599] CPU: 1 PID: 1296 Comm: modprobe Not tainted 5.0.0-rc7+ #236
[ 23.750605] Hardware name: Intel Corporation Merrifield/BODEGA BAY, BIOS 542 2015.01.21:18.19.48
[ 23.750620] RIP: 0010:check_for_stack+0xb7/0x190
[ 23.750630] Code: 8b 6d 50 4d 85 ed 75 04 4c 8b 6d 10 48 89 ef e8 2f 8b 44 00 48 89 c6 4a 8d 0c 23 4c 89 ea 48 c7 c7 88 d0 82 b4 e8 40 7c f9 ff <0f> 0b 8b 05 79 00 4b 01 85 c0 74 07 5b 5d 41 5c 41 5d c3 8b 05 54
[ 23.750637] RSP: 0000:ffff97bbc0292fa0 EFLAGS: 00010286
[ 23.750646] RAX: 0000000000000000 RBX: ffff97bbc0290000 RCX: 0000000000000006
[ 23.750652] RDX: 0000000000000007 RSI: 0000000000000002 RDI: ffff94b33e115450
[ 23.750658] RBP: ffff94b33c8578b0 R08: 0000000000000002 R09: 00000000000201c0
[ 23.750664] R10: 00000006ecb0ccc6 R11: 0000000000034f38 R12: 000000000000316c
[ 23.750670] R13: ffff94b33c84b250 R14: ffff94b33dedd5a0 R15: 0000000000000001
[ 23.750679] FS: 0000000000000000(0000) GS:ffff94b33e100000(0063) knlGS:00000000f7faf690
[ 23.750686] CS: 0010 DS: 002b ES: 002b CR0: 0000000080050033
[ 23.750691] CR2: 00000000f7f54faf CR3: 000000000722c000 CR4: 00000000001006e0
[ 23.750696] Call Trace:
[ 23.750713] debug_dma_map_sg+0x100/0x340
[ 23.750727] ? dma_direct_map_sg+0x3b/0xb0
[ 23.750739] spi_map_buf+0x25a/0x300
[ 23.750751] __spi_pump_messages+0x2a4/0x680
[ 23.750762] __spi_sync+0x1dd/0x1f0
[ 23.750773] spi_sync+0x26/0x40
[ 23.750790] mipi_dbi_typec3_command_read+0x14d/0x240 [mipi_dbi]
[ 23.750802] ? spi_finalize_current_transfer+0x10/0x10
[ 23.750821] mipi_dbi_typec3_command+0x1bc/0x1d0 [mipi_dbi]
Reported-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
drivers/gpu/drm/tinydrm/ili9225.c | 6 ++--
drivers/gpu/drm/tinydrm/mipi-dbi.c | 58 +++++++++++++++++++++---------
include/drm/tinydrm/mipi-dbi.h | 5 +--
3 files changed, 48 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/tinydrm/ili9225.c b/drivers/gpu/drm/tinydrm/ili9225.c
index 3f59cfbd31ba..a87078667e04 100644
--- a/drivers/gpu/drm/tinydrm/ili9225.c
+++ b/drivers/gpu/drm/tinydrm/ili9225.c
@@ -299,7 +299,7 @@ static void ili9225_pipe_disable(struct drm_simple_display_pipe *pipe)
mipi->enabled = false;
}
-static int ili9225_dbi_command(struct mipi_dbi *mipi, u8 cmd, u8 *par,
+static int ili9225_dbi_command(struct mipi_dbi *mipi, u8 *cmd, u8 *par,
size_t num)
{
struct spi_device *spi = mipi->spi;
@@ -309,11 +309,11 @@ static int ili9225_dbi_command(struct mipi_dbi *mipi, u8 cmd, u8 *par,
gpiod_set_value_cansleep(mipi->dc, 0);
speed_hz = mipi_dbi_spi_cmd_max_speed(spi, 1);
- ret = tinydrm_spi_transfer(spi, speed_hz, NULL, 8, &cmd, 1);
+ ret = tinydrm_spi_transfer(spi, speed_hz, NULL, 8, cmd, 1);
if (ret || !num)
return ret;
- if (cmd == ILI9225_WRITE_DATA_TO_GRAM && !mipi->swap_bytes)
+ if (*cmd == ILI9225_WRITE_DATA_TO_GRAM && !mipi->swap_bytes)
bpw = 16;
gpiod_set_value_cansleep(mipi->dc, 1);
diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c
index 246e708a9ff7..4a4cd7e72938 100644
--- a/drivers/gpu/drm/tinydrm/mipi-dbi.c
+++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c
@@ -153,16 +153,42 @@ EXPORT_SYMBOL(mipi_dbi_command_read);
*/
int mipi_dbi_command_buf(struct mipi_dbi *mipi, u8 cmd, u8 *data, size_t len)
{
+ u8 *cmdbuf;
int ret;
+ /* SPI requires dma-safe buffers */
+ cmdbuf = kmemdup(&cmd, 1, GFP_KERNEL);
+ if (!cmdbuf)
+ return -ENOMEM;
+
mutex_lock(&mipi->cmdlock);
- ret = mipi->command(mipi, cmd, data, len);
+ ret = mipi->command(mipi, cmdbuf, data, len);
mutex_unlock(&mipi->cmdlock);
+ kfree(cmdbuf);
+
return ret;
}
EXPORT_SYMBOL(mipi_dbi_command_buf);
+/* This should only be used by mipi_dbi_command() */
+int mipi_dbi_command_stackbuf(struct mipi_dbi *mipi, u8 cmd, u8 *data, size_t len)
+{
+ u8 *buf;
+ int ret;
+
+ buf = kmemdup(data, len, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ ret = mipi_dbi_command_buf(mipi, cmd, buf, len);
+
+ kfree(buf);
+
+ return ret;
+}
+EXPORT_SYMBOL(mipi_dbi_command_stackbuf);
+
/**
* mipi_dbi_buf_copy - Copy a framebuffer, transforming it if necessary
* @dst: The destination buffer
@@ -772,18 +798,18 @@ static int mipi_dbi_spi1_transfer(struct mipi_dbi *mipi, int dc,
return 0;
}
-static int mipi_dbi_typec1_command(struct mipi_dbi *mipi, u8 cmd,
+static int mipi_dbi_typec1_command(struct mipi_dbi *mipi, u8 *cmd,
u8 *parameters, size_t num)
{
- unsigned int bpw = (cmd == MIPI_DCS_WRITE_MEMORY_START) ? 16 : 8;
+ unsigned int bpw = (*cmd == MIPI_DCS_WRITE_MEMORY_START) ? 16 : 8;
int ret;
- if (mipi_dbi_command_is_read(mipi, cmd))
+ if (mipi_dbi_command_is_read(mipi, *cmd))
return -ENOTSUPP;
- MIPI_DBI_DEBUG_COMMAND(cmd, parameters, num);
+ MIPI_DBI_DEBUG_COMMAND(*cmd, parameters, num);
- ret = mipi_dbi_spi1_transfer(mipi, 0, &cmd, 1, 8);
+ ret = mipi_dbi_spi1_transfer(mipi, 0, cmd, 1, 8);
if (ret || !num)
return ret;
@@ -792,7 +818,7 @@ static int mipi_dbi_typec1_command(struct mipi_dbi *mipi, u8 cmd,
/* MIPI DBI Type C Option 3 */
-static int mipi_dbi_typec3_command_read(struct mipi_dbi *mipi, u8 cmd,
+static int mipi_dbi_typec3_command_read(struct mipi_dbi *mipi, u8 *cmd,
u8 *data, size_t len)
{
struct spi_device *spi = mipi->spi;
@@ -801,7 +827,7 @@ static int mipi_dbi_typec3_command_read(struct mipi_dbi *mipi, u8 cmd,
struct spi_transfer tr[2] = {
{
.speed_hz = speed_hz,
- .tx_buf = &cmd,
+ .tx_buf = cmd,
.len = 1,
}, {
.speed_hz = speed_hz,
@@ -819,8 +845,8 @@ static int mipi_dbi_typec3_command_read(struct mipi_dbi *mipi, u8 cmd,
* Support non-standard 24-bit and 32-bit Nokia read commands which
* start with a dummy clock, so we need to read an extra byte.
*/
- if (cmd == MIPI_DCS_GET_DISPLAY_ID ||
- cmd == MIPI_DCS_GET_DISPLAY_STATUS) {
+ if (*cmd == MIPI_DCS_GET_DISPLAY_ID ||
+ *cmd == MIPI_DCS_GET_DISPLAY_STATUS) {
if (!(len == 3 || len == 4))
return -EINVAL;
@@ -850,7 +876,7 @@ static int mipi_dbi_typec3_command_read(struct mipi_dbi *mipi, u8 cmd,
data[i] = (buf[i] << 1) | !!(buf[i + 1] & BIT(7));
}
- MIPI_DBI_DEBUG_COMMAND(cmd, data, len);
+ MIPI_DBI_DEBUG_COMMAND(*cmd, data, len);
err_free:
kfree(buf);
@@ -858,7 +884,7 @@ static int mipi_dbi_typec3_command_read(struct mipi_dbi *mipi, u8 cmd,
return ret;
}
-static int mipi_dbi_typec3_command(struct mipi_dbi *mipi, u8 cmd,
+static int mipi_dbi_typec3_command(struct mipi_dbi *mipi, u8 *cmd,
u8 *par, size_t num)
{
struct spi_device *spi = mipi->spi;
@@ -866,18 +892,18 @@ static int mipi_dbi_typec3_command(struct mipi_dbi *mipi, u8 cmd,
u32 speed_hz;
int ret;
- if (mipi_dbi_command_is_read(mipi, cmd))
+ if (mipi_dbi_command_is_read(mipi, *cmd))
return mipi_dbi_typec3_command_read(mipi, cmd, par, num);
- MIPI_DBI_DEBUG_COMMAND(cmd, par, num);
+ MIPI_DBI_DEBUG_COMMAND(*cmd, par, num);
gpiod_set_value_cansleep(mipi->dc, 0);
speed_hz = mipi_dbi_spi_cmd_max_speed(spi, 1);
- ret = tinydrm_spi_transfer(spi, speed_hz, NULL, 8, &cmd, 1);
+ ret = tinydrm_spi_transfer(spi, speed_hz, NULL, 8, cmd, 1);
if (ret || !num)
return ret;
- if (cmd == MIPI_DCS_WRITE_MEMORY_START && !mipi->swap_bytes)
+ if (*cmd == MIPI_DCS_WRITE_MEMORY_START && !mipi->swap_bytes)
bpw = 16;
gpiod_set_value_cansleep(mipi->dc, 1);
diff --git a/include/drm/tinydrm/mipi-dbi.h b/include/drm/tinydrm/mipi-dbi.h
index ad7e6bedab5f..464a47d1013d 100644
--- a/include/drm/tinydrm/mipi-dbi.h
+++ b/include/drm/tinydrm/mipi-dbi.h
@@ -43,7 +43,7 @@ struct mipi_dbi {
struct spi_device *spi;
bool enabled;
struct mutex cmdlock;
- int (*command)(struct mipi_dbi *mipi, u8 cmd, u8 *param, size_t num);
+ int (*command)(struct mipi_dbi *mipi, u8 *cmd, u8 *param, size_t num);
const u8 *read_commands;
struct gpio_desc *dc;
u16 *tx_buf;
@@ -83,6 +83,7 @@ u32 mipi_dbi_spi_cmd_max_speed(struct spi_device *spi, size_t len);
int mipi_dbi_command_read(struct mipi_dbi *mipi, u8 cmd, u8 *val);
int mipi_dbi_command_buf(struct mipi_dbi *mipi, u8 cmd, u8 *data, size_t len);
+int mipi_dbi_command_stackbuf(struct mipi_dbi *mipi, u8 cmd, u8 *data, size_t len);
int mipi_dbi_buf_copy(void *dst, struct drm_framebuffer *fb,
struct drm_rect *clip, bool swap);
/**
@@ -100,7 +101,7 @@ int mipi_dbi_buf_copy(void *dst, struct drm_framebuffer *fb,
#define mipi_dbi_command(mipi, cmd, seq...) \
({ \
u8 d[] = { seq }; \
- mipi_dbi_command_buf(mipi, cmd, d, ARRAY_SIZE(d)); \
+ mipi_dbi_command_stackbuf(mipi, cmd, d, ARRAY_SIZE(d)); \
})
#ifdef CONFIG_DEBUG_FS
--
2.20.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] tinydrm/mipi-dbi: Use dma-safe buffers for all SPI transfers
2019-02-22 12:43 [PATCH] tinydrm/mipi-dbi: Use dma-safe buffers for all SPI transfers Noralf Trønnes
@ 2019-02-22 15:58 ` Andy Shevchenko
2019-03-04 14:45 ` Noralf Trønnes
0 siblings, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2019-02-22 15:58 UTC (permalink / raw)
To: Noralf Trønnes; +Cc: david, dri-devel
On Fri, Feb 22, 2019 at 01:43:29PM +0100, Noralf Trønnes wrote:
> Buffers passed to spi_sync() must be dma-safe even for tiny buffers since
> some SPI controllers use DMA for all transfers.
>
> Example splat with CONFIG_DMA_API_DEBUG enabled:
>
> [ 23.750467] DMA-API: dw_dmac_pci 0000:00:15.0: device driver maps memory from stack [probable addr=000000001e49185d]
> [ 23.750529] WARNING: CPU: 1 PID: 1296 at kernel/dma/debug.c:1161 check_for_stack+0xb7/0x190
> [ 23.750533] Modules linked in: mmc_block(+) spi_pxa2xx_platform(+) pwm_lpss_pci pwm_lpss spi_pxa2xx_pci sdhci_pci cqhci intel_mrfld_pwrbtn extcon_intel_mrfld sdhci intel_mrfld_adc led_class mmc_core ili9341 mipi_dbi tinydrm backlight ti_ads7950 industrialio_triggered_buffer kfifo_buf intel_soc_pmic_mrfld hci_uart btbcm
> [ 23.750599] CPU: 1 PID: 1296 Comm: modprobe Not tainted 5.0.0-rc7+ #236
> [ 23.750605] Hardware name: Intel Corporation Merrifield/BODEGA BAY, BIOS 542 2015.01.21:18.19.48
> [ 23.750620] RIP: 0010:check_for_stack+0xb7/0x190
> [ 23.750630] Code: 8b 6d 50 4d 85 ed 75 04 4c 8b 6d 10 48 89 ef e8 2f 8b 44 00 48 89 c6 4a 8d 0c 23 4c 89 ea 48 c7 c7 88 d0 82 b4 e8 40 7c f9 ff <0f> 0b 8b 05 79 00 4b 01 85 c0 74 07 5b 5d 41 5c 41 5d c3 8b 05 54
> [ 23.750637] RSP: 0000:ffff97bbc0292fa0 EFLAGS: 00010286
> [ 23.750646] RAX: 0000000000000000 RBX: ffff97bbc0290000 RCX: 0000000000000006
> [ 23.750652] RDX: 0000000000000007 RSI: 0000000000000002 RDI: ffff94b33e115450
> [ 23.750658] RBP: ffff94b33c8578b0 R08: 0000000000000002 R09: 00000000000201c0
> [ 23.750664] R10: 00000006ecb0ccc6 R11: 0000000000034f38 R12: 000000000000316c
> [ 23.750670] R13: ffff94b33c84b250 R14: ffff94b33dedd5a0 R15: 0000000000000001
> [ 23.750679] FS: 0000000000000000(0000) GS:ffff94b33e100000(0063) knlGS:00000000f7faf690
> [ 23.750686] CS: 0010 DS: 002b ES: 002b CR0: 0000000080050033
> [ 23.750691] CR2: 00000000f7f54faf CR3: 000000000722c000 CR4: 00000000001006e0
> [ 23.750696] Call Trace:
> [ 23.750713] debug_dma_map_sg+0x100/0x340
> [ 23.750727] ? dma_direct_map_sg+0x3b/0xb0
> [ 23.750739] spi_map_buf+0x25a/0x300
> [ 23.750751] __spi_pump_messages+0x2a4/0x680
> [ 23.750762] __spi_sync+0x1dd/0x1f0
> [ 23.750773] spi_sync+0x26/0x40
> [ 23.750790] mipi_dbi_typec3_command_read+0x14d/0x240 [mipi_dbi]
> [ 23.750802] ? spi_finalize_current_transfer+0x10/0x10
> [ 23.750821] mipi_dbi_typec3_command+0x1bc/0x1d0 [mipi_dbi]
>
After few runs I don't see the warning anymore. So,
Tested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Though I would like to give few more days to monitor the state.
(I believe it's fixed)
> Reported-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
> drivers/gpu/drm/tinydrm/ili9225.c | 6 ++--
> drivers/gpu/drm/tinydrm/mipi-dbi.c | 58 +++++++++++++++++++++---------
> include/drm/tinydrm/mipi-dbi.h | 5 +--
> 3 files changed, 48 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/tinydrm/ili9225.c b/drivers/gpu/drm/tinydrm/ili9225.c
> index 3f59cfbd31ba..a87078667e04 100644
> --- a/drivers/gpu/drm/tinydrm/ili9225.c
> +++ b/drivers/gpu/drm/tinydrm/ili9225.c
> @@ -299,7 +299,7 @@ static void ili9225_pipe_disable(struct drm_simple_display_pipe *pipe)
> mipi->enabled = false;
> }
>
> -static int ili9225_dbi_command(struct mipi_dbi *mipi, u8 cmd, u8 *par,
> +static int ili9225_dbi_command(struct mipi_dbi *mipi, u8 *cmd, u8 *par,
> size_t num)
> {
> struct spi_device *spi = mipi->spi;
> @@ -309,11 +309,11 @@ static int ili9225_dbi_command(struct mipi_dbi *mipi, u8 cmd, u8 *par,
>
> gpiod_set_value_cansleep(mipi->dc, 0);
> speed_hz = mipi_dbi_spi_cmd_max_speed(spi, 1);
> - ret = tinydrm_spi_transfer(spi, speed_hz, NULL, 8, &cmd, 1);
> + ret = tinydrm_spi_transfer(spi, speed_hz, NULL, 8, cmd, 1);
> if (ret || !num)
> return ret;
>
> - if (cmd == ILI9225_WRITE_DATA_TO_GRAM && !mipi->swap_bytes)
> + if (*cmd == ILI9225_WRITE_DATA_TO_GRAM && !mipi->swap_bytes)
> bpw = 16;
>
> gpiod_set_value_cansleep(mipi->dc, 1);
> diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c
> index 246e708a9ff7..4a4cd7e72938 100644
> --- a/drivers/gpu/drm/tinydrm/mipi-dbi.c
> +++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c
> @@ -153,16 +153,42 @@ EXPORT_SYMBOL(mipi_dbi_command_read);
> */
> int mipi_dbi_command_buf(struct mipi_dbi *mipi, u8 cmd, u8 *data, size_t len)
> {
> + u8 *cmdbuf;
> int ret;
>
> + /* SPI requires dma-safe buffers */
> + cmdbuf = kmemdup(&cmd, 1, GFP_KERNEL);
> + if (!cmdbuf)
> + return -ENOMEM;
> +
> mutex_lock(&mipi->cmdlock);
> - ret = mipi->command(mipi, cmd, data, len);
> + ret = mipi->command(mipi, cmdbuf, data, len);
> mutex_unlock(&mipi->cmdlock);
>
> + kfree(cmdbuf);
> +
> return ret;
> }
> EXPORT_SYMBOL(mipi_dbi_command_buf);
>
> +/* This should only be used by mipi_dbi_command() */
> +int mipi_dbi_command_stackbuf(struct mipi_dbi *mipi, u8 cmd, u8 *data, size_t len)
> +{
> + u8 *buf;
> + int ret;
> +
> + buf = kmemdup(data, len, GFP_KERNEL);
> + if (!buf)
> + return -ENOMEM;
> +
> + ret = mipi_dbi_command_buf(mipi, cmd, buf, len);
> +
> + kfree(buf);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(mipi_dbi_command_stackbuf);
> +
> /**
> * mipi_dbi_buf_copy - Copy a framebuffer, transforming it if necessary
> * @dst: The destination buffer
> @@ -772,18 +798,18 @@ static int mipi_dbi_spi1_transfer(struct mipi_dbi *mipi, int dc,
> return 0;
> }
>
> -static int mipi_dbi_typec1_command(struct mipi_dbi *mipi, u8 cmd,
> +static int mipi_dbi_typec1_command(struct mipi_dbi *mipi, u8 *cmd,
> u8 *parameters, size_t num)
> {
> - unsigned int bpw = (cmd == MIPI_DCS_WRITE_MEMORY_START) ? 16 : 8;
> + unsigned int bpw = (*cmd == MIPI_DCS_WRITE_MEMORY_START) ? 16 : 8;
> int ret;
>
> - if (mipi_dbi_command_is_read(mipi, cmd))
> + if (mipi_dbi_command_is_read(mipi, *cmd))
> return -ENOTSUPP;
>
> - MIPI_DBI_DEBUG_COMMAND(cmd, parameters, num);
> + MIPI_DBI_DEBUG_COMMAND(*cmd, parameters, num);
>
> - ret = mipi_dbi_spi1_transfer(mipi, 0, &cmd, 1, 8);
> + ret = mipi_dbi_spi1_transfer(mipi, 0, cmd, 1, 8);
> if (ret || !num)
> return ret;
>
> @@ -792,7 +818,7 @@ static int mipi_dbi_typec1_command(struct mipi_dbi *mipi, u8 cmd,
>
> /* MIPI DBI Type C Option 3 */
>
> -static int mipi_dbi_typec3_command_read(struct mipi_dbi *mipi, u8 cmd,
> +static int mipi_dbi_typec3_command_read(struct mipi_dbi *mipi, u8 *cmd,
> u8 *data, size_t len)
> {
> struct spi_device *spi = mipi->spi;
> @@ -801,7 +827,7 @@ static int mipi_dbi_typec3_command_read(struct mipi_dbi *mipi, u8 cmd,
> struct spi_transfer tr[2] = {
> {
> .speed_hz = speed_hz,
> - .tx_buf = &cmd,
> + .tx_buf = cmd,
> .len = 1,
> }, {
> .speed_hz = speed_hz,
> @@ -819,8 +845,8 @@ static int mipi_dbi_typec3_command_read(struct mipi_dbi *mipi, u8 cmd,
> * Support non-standard 24-bit and 32-bit Nokia read commands which
> * start with a dummy clock, so we need to read an extra byte.
> */
> - if (cmd == MIPI_DCS_GET_DISPLAY_ID ||
> - cmd == MIPI_DCS_GET_DISPLAY_STATUS) {
> + if (*cmd == MIPI_DCS_GET_DISPLAY_ID ||
> + *cmd == MIPI_DCS_GET_DISPLAY_STATUS) {
> if (!(len == 3 || len == 4))
> return -EINVAL;
>
> @@ -850,7 +876,7 @@ static int mipi_dbi_typec3_command_read(struct mipi_dbi *mipi, u8 cmd,
> data[i] = (buf[i] << 1) | !!(buf[i + 1] & BIT(7));
> }
>
> - MIPI_DBI_DEBUG_COMMAND(cmd, data, len);
> + MIPI_DBI_DEBUG_COMMAND(*cmd, data, len);
>
> err_free:
> kfree(buf);
> @@ -858,7 +884,7 @@ static int mipi_dbi_typec3_command_read(struct mipi_dbi *mipi, u8 cmd,
> return ret;
> }
>
> -static int mipi_dbi_typec3_command(struct mipi_dbi *mipi, u8 cmd,
> +static int mipi_dbi_typec3_command(struct mipi_dbi *mipi, u8 *cmd,
> u8 *par, size_t num)
> {
> struct spi_device *spi = mipi->spi;
> @@ -866,18 +892,18 @@ static int mipi_dbi_typec3_command(struct mipi_dbi *mipi, u8 cmd,
> u32 speed_hz;
> int ret;
>
> - if (mipi_dbi_command_is_read(mipi, cmd))
> + if (mipi_dbi_command_is_read(mipi, *cmd))
> return mipi_dbi_typec3_command_read(mipi, cmd, par, num);
>
> - MIPI_DBI_DEBUG_COMMAND(cmd, par, num);
> + MIPI_DBI_DEBUG_COMMAND(*cmd, par, num);
>
> gpiod_set_value_cansleep(mipi->dc, 0);
> speed_hz = mipi_dbi_spi_cmd_max_speed(spi, 1);
> - ret = tinydrm_spi_transfer(spi, speed_hz, NULL, 8, &cmd, 1);
> + ret = tinydrm_spi_transfer(spi, speed_hz, NULL, 8, cmd, 1);
> if (ret || !num)
> return ret;
>
> - if (cmd == MIPI_DCS_WRITE_MEMORY_START && !mipi->swap_bytes)
> + if (*cmd == MIPI_DCS_WRITE_MEMORY_START && !mipi->swap_bytes)
> bpw = 16;
>
> gpiod_set_value_cansleep(mipi->dc, 1);
> diff --git a/include/drm/tinydrm/mipi-dbi.h b/include/drm/tinydrm/mipi-dbi.h
> index ad7e6bedab5f..464a47d1013d 100644
> --- a/include/drm/tinydrm/mipi-dbi.h
> +++ b/include/drm/tinydrm/mipi-dbi.h
> @@ -43,7 +43,7 @@ struct mipi_dbi {
> struct spi_device *spi;
> bool enabled;
> struct mutex cmdlock;
> - int (*command)(struct mipi_dbi *mipi, u8 cmd, u8 *param, size_t num);
> + int (*command)(struct mipi_dbi *mipi, u8 *cmd, u8 *param, size_t num);
> const u8 *read_commands;
> struct gpio_desc *dc;
> u16 *tx_buf;
> @@ -83,6 +83,7 @@ u32 mipi_dbi_spi_cmd_max_speed(struct spi_device *spi, size_t len);
>
> int mipi_dbi_command_read(struct mipi_dbi *mipi, u8 cmd, u8 *val);
> int mipi_dbi_command_buf(struct mipi_dbi *mipi, u8 cmd, u8 *data, size_t len);
> +int mipi_dbi_command_stackbuf(struct mipi_dbi *mipi, u8 cmd, u8 *data, size_t len);
> int mipi_dbi_buf_copy(void *dst, struct drm_framebuffer *fb,
> struct drm_rect *clip, bool swap);
> /**
> @@ -100,7 +101,7 @@ int mipi_dbi_buf_copy(void *dst, struct drm_framebuffer *fb,
> #define mipi_dbi_command(mipi, cmd, seq...) \
> ({ \
> u8 d[] = { seq }; \
> - mipi_dbi_command_buf(mipi, cmd, d, ARRAY_SIZE(d)); \
> + mipi_dbi_command_stackbuf(mipi, cmd, d, ARRAY_SIZE(d)); \
> })
>
> #ifdef CONFIG_DEBUG_FS
> --
> 2.20.1
>
--
With Best Regards,
Andy Shevchenko
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tinydrm/mipi-dbi: Use dma-safe buffers for all SPI transfers
2019-02-22 15:58 ` Andy Shevchenko
@ 2019-03-04 14:45 ` Noralf Trønnes
2019-03-04 15:10 ` Andy Shevchenko
0 siblings, 1 reply; 6+ messages in thread
From: Noralf Trønnes @ 2019-03-04 14:45 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: david, dri-devel
Den 22.02.2019 16.58, skrev Andy Shevchenko:
> On Fri, Feb 22, 2019 at 01:43:29PM +0100, Noralf Trønnes wrote:
>> Buffers passed to spi_sync() must be dma-safe even for tiny buffers since
>> some SPI controllers use DMA for all transfers.
>>
>> Example splat with CONFIG_DMA_API_DEBUG enabled:
>>
>> [ 23.750467] DMA-API: dw_dmac_pci 0000:00:15.0: device driver maps memory from stack [probable addr=000000001e49185d]
>> [ 23.750529] WARNING: CPU: 1 PID: 1296 at kernel/dma/debug.c:1161 check_for_stack+0xb7/0x190
>> [ 23.750533] Modules linked in: mmc_block(+) spi_pxa2xx_platform(+) pwm_lpss_pci pwm_lpss spi_pxa2xx_pci sdhci_pci cqhci intel_mrfld_pwrbtn extcon_intel_mrfld sdhci intel_mrfld_adc led_class mmc_core ili9341 mipi_dbi tinydrm backlight ti_ads7950 industrialio_triggered_buffer kfifo_buf intel_soc_pmic_mrfld hci_uart btbcm
>> [ 23.750599] CPU: 1 PID: 1296 Comm: modprobe Not tainted 5.0.0-rc7+ #236
>> [ 23.750605] Hardware name: Intel Corporation Merrifield/BODEGA BAY, BIOS 542 2015.01.21:18.19.48
>> [ 23.750620] RIP: 0010:check_for_stack+0xb7/0x190
>> [ 23.750630] Code: 8b 6d 50 4d 85 ed 75 04 4c 8b 6d 10 48 89 ef e8 2f 8b 44 00 48 89 c6 4a 8d 0c 23 4c 89 ea 48 c7 c7 88 d0 82 b4 e8 40 7c f9 ff <0f> 0b 8b 05 79 00 4b 01 85 c0 74 07 5b 5d 41 5c 41 5d c3 8b 05 54
>> [ 23.750637] RSP: 0000:ffff97bbc0292fa0 EFLAGS: 00010286
>> [ 23.750646] RAX: 0000000000000000 RBX: ffff97bbc0290000 RCX: 0000000000000006
>> [ 23.750652] RDX: 0000000000000007 RSI: 0000000000000002 RDI: ffff94b33e115450
>> [ 23.750658] RBP: ffff94b33c8578b0 R08: 0000000000000002 R09: 00000000000201c0
>> [ 23.750664] R10: 00000006ecb0ccc6 R11: 0000000000034f38 R12: 000000000000316c
>> [ 23.750670] R13: ffff94b33c84b250 R14: ffff94b33dedd5a0 R15: 0000000000000001
>> [ 23.750679] FS: 0000000000000000(0000) GS:ffff94b33e100000(0063) knlGS:00000000f7faf690
>> [ 23.750686] CS: 0010 DS: 002b ES: 002b CR0: 0000000080050033
>> [ 23.750691] CR2: 00000000f7f54faf CR3: 000000000722c000 CR4: 00000000001006e0
>> [ 23.750696] Call Trace:
>> [ 23.750713] debug_dma_map_sg+0x100/0x340
>> [ 23.750727] ? dma_direct_map_sg+0x3b/0xb0
>> [ 23.750739] spi_map_buf+0x25a/0x300
>> [ 23.750751] __spi_pump_messages+0x2a4/0x680
>> [ 23.750762] __spi_sync+0x1dd/0x1f0
>> [ 23.750773] spi_sync+0x26/0x40
>> [ 23.750790] mipi_dbi_typec3_command_read+0x14d/0x240 [mipi_dbi]
>> [ 23.750802] ? spi_finalize_current_transfer+0x10/0x10
>> [ 23.750821] mipi_dbi_typec3_command+0x1bc/0x1d0 [mipi_dbi]
>>
>
> After few runs I don't see the warning anymore. So,
> Tested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>
Can I have an ack as well if you're satisfied with it?
Noralf.
> Though I would like to give few more days to monitor the state.
> (I believe it's fixed)
>
>> Reported-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>> ---
>> drivers/gpu/drm/tinydrm/ili9225.c | 6 ++--
>> drivers/gpu/drm/tinydrm/mipi-dbi.c | 58 +++++++++++++++++++++---------
>> include/drm/tinydrm/mipi-dbi.h | 5 +--
>> 3 files changed, 48 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/tinydrm/ili9225.c b/drivers/gpu/drm/tinydrm/ili9225.c
>> index 3f59cfbd31ba..a87078667e04 100644
>> --- a/drivers/gpu/drm/tinydrm/ili9225.c
>> +++ b/drivers/gpu/drm/tinydrm/ili9225.c
>> @@ -299,7 +299,7 @@ static void ili9225_pipe_disable(struct drm_simple_display_pipe *pipe)
>> mipi->enabled = false;
>> }
>>
>> -static int ili9225_dbi_command(struct mipi_dbi *mipi, u8 cmd, u8 *par,
>> +static int ili9225_dbi_command(struct mipi_dbi *mipi, u8 *cmd, u8 *par,
>> size_t num)
>> {
>> struct spi_device *spi = mipi->spi;
>> @@ -309,11 +309,11 @@ static int ili9225_dbi_command(struct mipi_dbi *mipi, u8 cmd, u8 *par,
>>
>> gpiod_set_value_cansleep(mipi->dc, 0);
>> speed_hz = mipi_dbi_spi_cmd_max_speed(spi, 1);
>> - ret = tinydrm_spi_transfer(spi, speed_hz, NULL, 8, &cmd, 1);
>> + ret = tinydrm_spi_transfer(spi, speed_hz, NULL, 8, cmd, 1);
>> if (ret || !num)
>> return ret;
>>
>> - if (cmd == ILI9225_WRITE_DATA_TO_GRAM && !mipi->swap_bytes)
>> + if (*cmd == ILI9225_WRITE_DATA_TO_GRAM && !mipi->swap_bytes)
>> bpw = 16;
>>
>> gpiod_set_value_cansleep(mipi->dc, 1);
>> diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c
>> index 246e708a9ff7..4a4cd7e72938 100644
>> --- a/drivers/gpu/drm/tinydrm/mipi-dbi.c
>> +++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c
>> @@ -153,16 +153,42 @@ EXPORT_SYMBOL(mipi_dbi_command_read);
>> */
>> int mipi_dbi_command_buf(struct mipi_dbi *mipi, u8 cmd, u8 *data, size_t len)
>> {
>> + u8 *cmdbuf;
>> int ret;
>>
>> + /* SPI requires dma-safe buffers */
>> + cmdbuf = kmemdup(&cmd, 1, GFP_KERNEL);
>> + if (!cmdbuf)
>> + return -ENOMEM;
>> +
>> mutex_lock(&mipi->cmdlock);
>> - ret = mipi->command(mipi, cmd, data, len);
>> + ret = mipi->command(mipi, cmdbuf, data, len);
>> mutex_unlock(&mipi->cmdlock);
>>
>> + kfree(cmdbuf);
>> +
>> return ret;
>> }
>> EXPORT_SYMBOL(mipi_dbi_command_buf);
>>
>> +/* This should only be used by mipi_dbi_command() */
>> +int mipi_dbi_command_stackbuf(struct mipi_dbi *mipi, u8 cmd, u8 *data, size_t len)
>> +{
>> + u8 *buf;
>> + int ret;
>> +
>> + buf = kmemdup(data, len, GFP_KERNEL);
>> + if (!buf)
>> + return -ENOMEM;
>> +
>> + ret = mipi_dbi_command_buf(mipi, cmd, buf, len);
>> +
>> + kfree(buf);
>> +
>> + return ret;
>> +}
>> +EXPORT_SYMBOL(mipi_dbi_command_stackbuf);
>> +
>> /**
>> * mipi_dbi_buf_copy - Copy a framebuffer, transforming it if necessary
>> * @dst: The destination buffer
>> @@ -772,18 +798,18 @@ static int mipi_dbi_spi1_transfer(struct mipi_dbi *mipi, int dc,
>> return 0;
>> }
>>
>> -static int mipi_dbi_typec1_command(struct mipi_dbi *mipi, u8 cmd,
>> +static int mipi_dbi_typec1_command(struct mipi_dbi *mipi, u8 *cmd,
>> u8 *parameters, size_t num)
>> {
>> - unsigned int bpw = (cmd == MIPI_DCS_WRITE_MEMORY_START) ? 16 : 8;
>> + unsigned int bpw = (*cmd == MIPI_DCS_WRITE_MEMORY_START) ? 16 : 8;
>> int ret;
>>
>> - if (mipi_dbi_command_is_read(mipi, cmd))
>> + if (mipi_dbi_command_is_read(mipi, *cmd))
>> return -ENOTSUPP;
>>
>> - MIPI_DBI_DEBUG_COMMAND(cmd, parameters, num);
>> + MIPI_DBI_DEBUG_COMMAND(*cmd, parameters, num);
>>
>> - ret = mipi_dbi_spi1_transfer(mipi, 0, &cmd, 1, 8);
>> + ret = mipi_dbi_spi1_transfer(mipi, 0, cmd, 1, 8);
>> if (ret || !num)
>> return ret;
>>
>> @@ -792,7 +818,7 @@ static int mipi_dbi_typec1_command(struct mipi_dbi *mipi, u8 cmd,
>>
>> /* MIPI DBI Type C Option 3 */
>>
>> -static int mipi_dbi_typec3_command_read(struct mipi_dbi *mipi, u8 cmd,
>> +static int mipi_dbi_typec3_command_read(struct mipi_dbi *mipi, u8 *cmd,
>> u8 *data, size_t len)
>> {
>> struct spi_device *spi = mipi->spi;
>> @@ -801,7 +827,7 @@ static int mipi_dbi_typec3_command_read(struct mipi_dbi *mipi, u8 cmd,
>> struct spi_transfer tr[2] = {
>> {
>> .speed_hz = speed_hz,
>> - .tx_buf = &cmd,
>> + .tx_buf = cmd,
>> .len = 1,
>> }, {
>> .speed_hz = speed_hz,
>> @@ -819,8 +845,8 @@ static int mipi_dbi_typec3_command_read(struct mipi_dbi *mipi, u8 cmd,
>> * Support non-standard 24-bit and 32-bit Nokia read commands which
>> * start with a dummy clock, so we need to read an extra byte.
>> */
>> - if (cmd == MIPI_DCS_GET_DISPLAY_ID ||
>> - cmd == MIPI_DCS_GET_DISPLAY_STATUS) {
>> + if (*cmd == MIPI_DCS_GET_DISPLAY_ID ||
>> + *cmd == MIPI_DCS_GET_DISPLAY_STATUS) {
>> if (!(len == 3 || len == 4))
>> return -EINVAL;
>>
>> @@ -850,7 +876,7 @@ static int mipi_dbi_typec3_command_read(struct mipi_dbi *mipi, u8 cmd,
>> data[i] = (buf[i] << 1) | !!(buf[i + 1] & BIT(7));
>> }
>>
>> - MIPI_DBI_DEBUG_COMMAND(cmd, data, len);
>> + MIPI_DBI_DEBUG_COMMAND(*cmd, data, len);
>>
>> err_free:
>> kfree(buf);
>> @@ -858,7 +884,7 @@ static int mipi_dbi_typec3_command_read(struct mipi_dbi *mipi, u8 cmd,
>> return ret;
>> }
>>
>> -static int mipi_dbi_typec3_command(struct mipi_dbi *mipi, u8 cmd,
>> +static int mipi_dbi_typec3_command(struct mipi_dbi *mipi, u8 *cmd,
>> u8 *par, size_t num)
>> {
>> struct spi_device *spi = mipi->spi;
>> @@ -866,18 +892,18 @@ static int mipi_dbi_typec3_command(struct mipi_dbi *mipi, u8 cmd,
>> u32 speed_hz;
>> int ret;
>>
>> - if (mipi_dbi_command_is_read(mipi, cmd))
>> + if (mipi_dbi_command_is_read(mipi, *cmd))
>> return mipi_dbi_typec3_command_read(mipi, cmd, par, num);
>>
>> - MIPI_DBI_DEBUG_COMMAND(cmd, par, num);
>> + MIPI_DBI_DEBUG_COMMAND(*cmd, par, num);
>>
>> gpiod_set_value_cansleep(mipi->dc, 0);
>> speed_hz = mipi_dbi_spi_cmd_max_speed(spi, 1);
>> - ret = tinydrm_spi_transfer(spi, speed_hz, NULL, 8, &cmd, 1);
>> + ret = tinydrm_spi_transfer(spi, speed_hz, NULL, 8, cmd, 1);
>> if (ret || !num)
>> return ret;
>>
>> - if (cmd == MIPI_DCS_WRITE_MEMORY_START && !mipi->swap_bytes)
>> + if (*cmd == MIPI_DCS_WRITE_MEMORY_START && !mipi->swap_bytes)
>> bpw = 16;
>>
>> gpiod_set_value_cansleep(mipi->dc, 1);
>> diff --git a/include/drm/tinydrm/mipi-dbi.h b/include/drm/tinydrm/mipi-dbi.h
>> index ad7e6bedab5f..464a47d1013d 100644
>> --- a/include/drm/tinydrm/mipi-dbi.h
>> +++ b/include/drm/tinydrm/mipi-dbi.h
>> @@ -43,7 +43,7 @@ struct mipi_dbi {
>> struct spi_device *spi;
>> bool enabled;
>> struct mutex cmdlock;
>> - int (*command)(struct mipi_dbi *mipi, u8 cmd, u8 *param, size_t num);
>> + int (*command)(struct mipi_dbi *mipi, u8 *cmd, u8 *param, size_t num);
>> const u8 *read_commands;
>> struct gpio_desc *dc;
>> u16 *tx_buf;
>> @@ -83,6 +83,7 @@ u32 mipi_dbi_spi_cmd_max_speed(struct spi_device *spi, size_t len);
>>
>> int mipi_dbi_command_read(struct mipi_dbi *mipi, u8 cmd, u8 *val);
>> int mipi_dbi_command_buf(struct mipi_dbi *mipi, u8 cmd, u8 *data, size_t len);
>> +int mipi_dbi_command_stackbuf(struct mipi_dbi *mipi, u8 cmd, u8 *data, size_t len);
>> int mipi_dbi_buf_copy(void *dst, struct drm_framebuffer *fb,
>> struct drm_rect *clip, bool swap);
>> /**
>> @@ -100,7 +101,7 @@ int mipi_dbi_buf_copy(void *dst, struct drm_framebuffer *fb,
>> #define mipi_dbi_command(mipi, cmd, seq...) \
>> ({ \
>> u8 d[] = { seq }; \
>> - mipi_dbi_command_buf(mipi, cmd, d, ARRAY_SIZE(d)); \
>> + mipi_dbi_command_stackbuf(mipi, cmd, d, ARRAY_SIZE(d)); \
>> })
>>
>> #ifdef CONFIG_DEBUG_FS
>> --
>> 2.20.1
>>
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tinydrm/mipi-dbi: Use dma-safe buffers for all SPI transfers
2019-03-04 14:45 ` Noralf Trønnes
@ 2019-03-04 15:10 ` Andy Shevchenko
2019-03-04 17:51 ` Noralf Trønnes
0 siblings, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2019-03-04 15:10 UTC (permalink / raw)
To: Noralf Trønnes; +Cc: david, dri-devel
On Mon, Mar 04, 2019 at 03:45:56PM +0100, Noralf Trønnes wrote:
>
>
> Den 22.02.2019 16.58, skrev Andy Shevchenko:
> > On Fri, Feb 22, 2019 at 01:43:29PM +0100, Noralf Trønnes wrote:
> >> Buffers passed to spi_sync() must be dma-safe even for tiny buffers since
> >> some SPI controllers use DMA for all transfers.
> >>
> >> Example splat with CONFIG_DMA_API_DEBUG enabled:
> >>
> >> [ 23.750467] DMA-API: dw_dmac_pci 0000:00:15.0: device driver maps memory from stack [probable addr=000000001e49185d]
> >> [ 23.750529] WARNING: CPU: 1 PID: 1296 at kernel/dma/debug.c:1161 check_for_stack+0xb7/0x190
> >> [ 23.750533] Modules linked in: mmc_block(+) spi_pxa2xx_platform(+) pwm_lpss_pci pwm_lpss spi_pxa2xx_pci sdhci_pci cqhci intel_mrfld_pwrbtn extcon_intel_mrfld sdhci intel_mrfld_adc led_class mmc_core ili9341 mipi_dbi tinydrm backlight ti_ads7950 industrialio_triggered_buffer kfifo_buf intel_soc_pmic_mrfld hci_uart btbcm
> >> [ 23.750599] CPU: 1 PID: 1296 Comm: modprobe Not tainted 5.0.0-rc7+ #236
> >> [ 23.750605] Hardware name: Intel Corporation Merrifield/BODEGA BAY, BIOS 542 2015.01.21:18.19.48
> >> [ 23.750620] RIP: 0010:check_for_stack+0xb7/0x190
> >> [ 23.750630] Code: 8b 6d 50 4d 85 ed 75 04 4c 8b 6d 10 48 89 ef e8 2f 8b 44 00 48 89 c6 4a 8d 0c 23 4c 89 ea 48 c7 c7 88 d0 82 b4 e8 40 7c f9 ff <0f> 0b 8b 05 79 00 4b 01 85 c0 74 07 5b 5d 41 5c 41 5d c3 8b 05 54
> >> [ 23.750637] RSP: 0000:ffff97bbc0292fa0 EFLAGS: 00010286
> >> [ 23.750646] RAX: 0000000000000000 RBX: ffff97bbc0290000 RCX: 0000000000000006
> >> [ 23.750652] RDX: 0000000000000007 RSI: 0000000000000002 RDI: ffff94b33e115450
> >> [ 23.750658] RBP: ffff94b33c8578b0 R08: 0000000000000002 R09: 00000000000201c0
> >> [ 23.750664] R10: 00000006ecb0ccc6 R11: 0000000000034f38 R12: 000000000000316c
> >> [ 23.750670] R13: ffff94b33c84b250 R14: ffff94b33dedd5a0 R15: 0000000000000001
> >> [ 23.750679] FS: 0000000000000000(0000) GS:ffff94b33e100000(0063) knlGS:00000000f7faf690
> >> [ 23.750686] CS: 0010 DS: 002b ES: 002b CR0: 0000000080050033
> >> [ 23.750691] CR2: 00000000f7f54faf CR3: 000000000722c000 CR4: 00000000001006e0
> >> [ 23.750696] Call Trace:
> >> [ 23.750713] debug_dma_map_sg+0x100/0x340
> >> [ 23.750727] ? dma_direct_map_sg+0x3b/0xb0
> >> [ 23.750739] spi_map_buf+0x25a/0x300
> >> [ 23.750751] __spi_pump_messages+0x2a4/0x680
> >> [ 23.750762] __spi_sync+0x1dd/0x1f0
> >> [ 23.750773] spi_sync+0x26/0x40
> >> [ 23.750790] mipi_dbi_typec3_command_read+0x14d/0x240 [mipi_dbi]
> >> [ 23.750802] ? spi_finalize_current_transfer+0x10/0x10
> >> [ 23.750821] mipi_dbi_typec3_command+0x1bc/0x1d0 [mipi_dbi]
> >>
> >
> > After few runs I don't see the warning anymore. So,
> > Tested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> >
>
> Can I have an ack as well if you're satisfied with it?
Sure.
Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
I hope to see this in v5.1-rc1.
>
> Noralf.
>
> > Though I would like to give few more days to monitor the state.
> > (I believe it's fixed)
> >
> >> Reported-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> >> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> >> ---
> >> drivers/gpu/drm/tinydrm/ili9225.c | 6 ++--
> >> drivers/gpu/drm/tinydrm/mipi-dbi.c | 58 +++++++++++++++++++++---------
> >> include/drm/tinydrm/mipi-dbi.h | 5 +--
> >> 3 files changed, 48 insertions(+), 21 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/tinydrm/ili9225.c b/drivers/gpu/drm/tinydrm/ili9225.c
> >> index 3f59cfbd31ba..a87078667e04 100644
> >> --- a/drivers/gpu/drm/tinydrm/ili9225.c
> >> +++ b/drivers/gpu/drm/tinydrm/ili9225.c
> >> @@ -299,7 +299,7 @@ static void ili9225_pipe_disable(struct drm_simple_display_pipe *pipe)
> >> mipi->enabled = false;
> >> }
> >>
> >> -static int ili9225_dbi_command(struct mipi_dbi *mipi, u8 cmd, u8 *par,
> >> +static int ili9225_dbi_command(struct mipi_dbi *mipi, u8 *cmd, u8 *par,
> >> size_t num)
> >> {
> >> struct spi_device *spi = mipi->spi;
> >> @@ -309,11 +309,11 @@ static int ili9225_dbi_command(struct mipi_dbi *mipi, u8 cmd, u8 *par,
> >>
> >> gpiod_set_value_cansleep(mipi->dc, 0);
> >> speed_hz = mipi_dbi_spi_cmd_max_speed(spi, 1);
> >> - ret = tinydrm_spi_transfer(spi, speed_hz, NULL, 8, &cmd, 1);
> >> + ret = tinydrm_spi_transfer(spi, speed_hz, NULL, 8, cmd, 1);
> >> if (ret || !num)
> >> return ret;
> >>
> >> - if (cmd == ILI9225_WRITE_DATA_TO_GRAM && !mipi->swap_bytes)
> >> + if (*cmd == ILI9225_WRITE_DATA_TO_GRAM && !mipi->swap_bytes)
> >> bpw = 16;
> >>
> >> gpiod_set_value_cansleep(mipi->dc, 1);
> >> diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c
> >> index 246e708a9ff7..4a4cd7e72938 100644
> >> --- a/drivers/gpu/drm/tinydrm/mipi-dbi.c
> >> +++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c
> >> @@ -153,16 +153,42 @@ EXPORT_SYMBOL(mipi_dbi_command_read);
> >> */
> >> int mipi_dbi_command_buf(struct mipi_dbi *mipi, u8 cmd, u8 *data, size_t len)
> >> {
> >> + u8 *cmdbuf;
> >> int ret;
> >>
> >> + /* SPI requires dma-safe buffers */
> >> + cmdbuf = kmemdup(&cmd, 1, GFP_KERNEL);
> >> + if (!cmdbuf)
> >> + return -ENOMEM;
> >> +
> >> mutex_lock(&mipi->cmdlock);
> >> - ret = mipi->command(mipi, cmd, data, len);
> >> + ret = mipi->command(mipi, cmdbuf, data, len);
> >> mutex_unlock(&mipi->cmdlock);
> >>
> >> + kfree(cmdbuf);
> >> +
> >> return ret;
> >> }
> >> EXPORT_SYMBOL(mipi_dbi_command_buf);
> >>
> >> +/* This should only be used by mipi_dbi_command() */
> >> +int mipi_dbi_command_stackbuf(struct mipi_dbi *mipi, u8 cmd, u8 *data, size_t len)
> >> +{
> >> + u8 *buf;
> >> + int ret;
> >> +
> >> + buf = kmemdup(data, len, GFP_KERNEL);
> >> + if (!buf)
> >> + return -ENOMEM;
> >> +
> >> + ret = mipi_dbi_command_buf(mipi, cmd, buf, len);
> >> +
> >> + kfree(buf);
> >> +
> >> + return ret;
> >> +}
> >> +EXPORT_SYMBOL(mipi_dbi_command_stackbuf);
> >> +
> >> /**
> >> * mipi_dbi_buf_copy - Copy a framebuffer, transforming it if necessary
> >> * @dst: The destination buffer
> >> @@ -772,18 +798,18 @@ static int mipi_dbi_spi1_transfer(struct mipi_dbi *mipi, int dc,
> >> return 0;
> >> }
> >>
> >> -static int mipi_dbi_typec1_command(struct mipi_dbi *mipi, u8 cmd,
> >> +static int mipi_dbi_typec1_command(struct mipi_dbi *mipi, u8 *cmd,
> >> u8 *parameters, size_t num)
> >> {
> >> - unsigned int bpw = (cmd == MIPI_DCS_WRITE_MEMORY_START) ? 16 : 8;
> >> + unsigned int bpw = (*cmd == MIPI_DCS_WRITE_MEMORY_START) ? 16 : 8;
> >> int ret;
> >>
> >> - if (mipi_dbi_command_is_read(mipi, cmd))
> >> + if (mipi_dbi_command_is_read(mipi, *cmd))
> >> return -ENOTSUPP;
> >>
> >> - MIPI_DBI_DEBUG_COMMAND(cmd, parameters, num);
> >> + MIPI_DBI_DEBUG_COMMAND(*cmd, parameters, num);
> >>
> >> - ret = mipi_dbi_spi1_transfer(mipi, 0, &cmd, 1, 8);
> >> + ret = mipi_dbi_spi1_transfer(mipi, 0, cmd, 1, 8);
> >> if (ret || !num)
> >> return ret;
> >>
> >> @@ -792,7 +818,7 @@ static int mipi_dbi_typec1_command(struct mipi_dbi *mipi, u8 cmd,
> >>
> >> /* MIPI DBI Type C Option 3 */
> >>
> >> -static int mipi_dbi_typec3_command_read(struct mipi_dbi *mipi, u8 cmd,
> >> +static int mipi_dbi_typec3_command_read(struct mipi_dbi *mipi, u8 *cmd,
> >> u8 *data, size_t len)
> >> {
> >> struct spi_device *spi = mipi->spi;
> >> @@ -801,7 +827,7 @@ static int mipi_dbi_typec3_command_read(struct mipi_dbi *mipi, u8 cmd,
> >> struct spi_transfer tr[2] = {
> >> {
> >> .speed_hz = speed_hz,
> >> - .tx_buf = &cmd,
> >> + .tx_buf = cmd,
> >> .len = 1,
> >> }, {
> >> .speed_hz = speed_hz,
> >> @@ -819,8 +845,8 @@ static int mipi_dbi_typec3_command_read(struct mipi_dbi *mipi, u8 cmd,
> >> * Support non-standard 24-bit and 32-bit Nokia read commands which
> >> * start with a dummy clock, so we need to read an extra byte.
> >> */
> >> - if (cmd == MIPI_DCS_GET_DISPLAY_ID ||
> >> - cmd == MIPI_DCS_GET_DISPLAY_STATUS) {
> >> + if (*cmd == MIPI_DCS_GET_DISPLAY_ID ||
> >> + *cmd == MIPI_DCS_GET_DISPLAY_STATUS) {
> >> if (!(len == 3 || len == 4))
> >> return -EINVAL;
> >>
> >> @@ -850,7 +876,7 @@ static int mipi_dbi_typec3_command_read(struct mipi_dbi *mipi, u8 cmd,
> >> data[i] = (buf[i] << 1) | !!(buf[i + 1] & BIT(7));
> >> }
> >>
> >> - MIPI_DBI_DEBUG_COMMAND(cmd, data, len);
> >> + MIPI_DBI_DEBUG_COMMAND(*cmd, data, len);
> >>
> >> err_free:
> >> kfree(buf);
> >> @@ -858,7 +884,7 @@ static int mipi_dbi_typec3_command_read(struct mipi_dbi *mipi, u8 cmd,
> >> return ret;
> >> }
> >>
> >> -static int mipi_dbi_typec3_command(struct mipi_dbi *mipi, u8 cmd,
> >> +static int mipi_dbi_typec3_command(struct mipi_dbi *mipi, u8 *cmd,
> >> u8 *par, size_t num)
> >> {
> >> struct spi_device *spi = mipi->spi;
> >> @@ -866,18 +892,18 @@ static int mipi_dbi_typec3_command(struct mipi_dbi *mipi, u8 cmd,
> >> u32 speed_hz;
> >> int ret;
> >>
> >> - if (mipi_dbi_command_is_read(mipi, cmd))
> >> + if (mipi_dbi_command_is_read(mipi, *cmd))
> >> return mipi_dbi_typec3_command_read(mipi, cmd, par, num);
> >>
> >> - MIPI_DBI_DEBUG_COMMAND(cmd, par, num);
> >> + MIPI_DBI_DEBUG_COMMAND(*cmd, par, num);
> >>
> >> gpiod_set_value_cansleep(mipi->dc, 0);
> >> speed_hz = mipi_dbi_spi_cmd_max_speed(spi, 1);
> >> - ret = tinydrm_spi_transfer(spi, speed_hz, NULL, 8, &cmd, 1);
> >> + ret = tinydrm_spi_transfer(spi, speed_hz, NULL, 8, cmd, 1);
> >> if (ret || !num)
> >> return ret;
> >>
> >> - if (cmd == MIPI_DCS_WRITE_MEMORY_START && !mipi->swap_bytes)
> >> + if (*cmd == MIPI_DCS_WRITE_MEMORY_START && !mipi->swap_bytes)
> >> bpw = 16;
> >>
> >> gpiod_set_value_cansleep(mipi->dc, 1);
> >> diff --git a/include/drm/tinydrm/mipi-dbi.h b/include/drm/tinydrm/mipi-dbi.h
> >> index ad7e6bedab5f..464a47d1013d 100644
> >> --- a/include/drm/tinydrm/mipi-dbi.h
> >> +++ b/include/drm/tinydrm/mipi-dbi.h
> >> @@ -43,7 +43,7 @@ struct mipi_dbi {
> >> struct spi_device *spi;
> >> bool enabled;
> >> struct mutex cmdlock;
> >> - int (*command)(struct mipi_dbi *mipi, u8 cmd, u8 *param, size_t num);
> >> + int (*command)(struct mipi_dbi *mipi, u8 *cmd, u8 *param, size_t num);
> >> const u8 *read_commands;
> >> struct gpio_desc *dc;
> >> u16 *tx_buf;
> >> @@ -83,6 +83,7 @@ u32 mipi_dbi_spi_cmd_max_speed(struct spi_device *spi, size_t len);
> >>
> >> int mipi_dbi_command_read(struct mipi_dbi *mipi, u8 cmd, u8 *val);
> >> int mipi_dbi_command_buf(struct mipi_dbi *mipi, u8 cmd, u8 *data, size_t len);
> >> +int mipi_dbi_command_stackbuf(struct mipi_dbi *mipi, u8 cmd, u8 *data, size_t len);
> >> int mipi_dbi_buf_copy(void *dst, struct drm_framebuffer *fb,
> >> struct drm_rect *clip, bool swap);
> >> /**
> >> @@ -100,7 +101,7 @@ int mipi_dbi_buf_copy(void *dst, struct drm_framebuffer *fb,
> >> #define mipi_dbi_command(mipi, cmd, seq...) \
> >> ({ \
> >> u8 d[] = { seq }; \
> >> - mipi_dbi_command_buf(mipi, cmd, d, ARRAY_SIZE(d)); \
> >> + mipi_dbi_command_stackbuf(mipi, cmd, d, ARRAY_SIZE(d)); \
> >> })
> >>
> >> #ifdef CONFIG_DEBUG_FS
> >> --
> >> 2.20.1
> >>
> >
--
With Best Regards,
Andy Shevchenko
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tinydrm/mipi-dbi: Use dma-safe buffers for all SPI transfers
2019-03-04 15:10 ` Andy Shevchenko
@ 2019-03-04 17:51 ` Noralf Trønnes
2019-03-05 7:32 ` Andy Shevchenko
0 siblings, 1 reply; 6+ messages in thread
From: Noralf Trønnes @ 2019-03-04 17:51 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: david, dri-devel
Den 04.03.2019 16.10, skrev Andy Shevchenko:
> On Mon, Mar 04, 2019 at 03:45:56PM +0100, Noralf Trønnes wrote:
>>
>>
>> Den 22.02.2019 16.58, skrev Andy Shevchenko:
>>> On Fri, Feb 22, 2019 at 01:43:29PM +0100, Noralf Trønnes wrote:
>>>> Buffers passed to spi_sync() must be dma-safe even for tiny buffers since
>>>> some SPI controllers use DMA for all transfers.
>>>>
>>>> Example splat with CONFIG_DMA_API_DEBUG enabled:
>>>>
>>>> [ 23.750467] DMA-API: dw_dmac_pci 0000:00:15.0: device driver maps memory from stack [probable addr=000000001e49185d]
>>>> [ 23.750529] WARNING: CPU: 1 PID: 1296 at kernel/dma/debug.c:1161 check_for_stack+0xb7/0x190
>>>> [ 23.750533] Modules linked in: mmc_block(+) spi_pxa2xx_platform(+) pwm_lpss_pci pwm_lpss spi_pxa2xx_pci sdhci_pci cqhci intel_mrfld_pwrbtn extcon_intel_mrfld sdhci intel_mrfld_adc led_class mmc_core ili9341 mipi_dbi tinydrm backlight ti_ads7950 industrialio_triggered_buffer kfifo_buf intel_soc_pmic_mrfld hci_uart btbcm
>>>> [ 23.750599] CPU: 1 PID: 1296 Comm: modprobe Not tainted 5.0.0-rc7+ #236
>>>> [ 23.750605] Hardware name: Intel Corporation Merrifield/BODEGA BAY, BIOS 542 2015.01.21:18.19.48
>>>> [ 23.750620] RIP: 0010:check_for_stack+0xb7/0x190
>>>> [ 23.750630] Code: 8b 6d 50 4d 85 ed 75 04 4c 8b 6d 10 48 89 ef e8 2f 8b 44 00 48 89 c6 4a 8d 0c 23 4c 89 ea 48 c7 c7 88 d0 82 b4 e8 40 7c f9 ff <0f> 0b 8b 05 79 00 4b 01 85 c0 74 07 5b 5d 41 5c 41 5d c3 8b 05 54
>>>> [ 23.750637] RSP: 0000:ffff97bbc0292fa0 EFLAGS: 00010286
>>>> [ 23.750646] RAX: 0000000000000000 RBX: ffff97bbc0290000 RCX: 0000000000000006
>>>> [ 23.750652] RDX: 0000000000000007 RSI: 0000000000000002 RDI: ffff94b33e115450
>>>> [ 23.750658] RBP: ffff94b33c8578b0 R08: 0000000000000002 R09: 00000000000201c0
>>>> [ 23.750664] R10: 00000006ecb0ccc6 R11: 0000000000034f38 R12: 000000000000316c
>>>> [ 23.750670] R13: ffff94b33c84b250 R14: ffff94b33dedd5a0 R15: 0000000000000001
>>>> [ 23.750679] FS: 0000000000000000(0000) GS:ffff94b33e100000(0063) knlGS:00000000f7faf690
>>>> [ 23.750686] CS: 0010 DS: 002b ES: 002b CR0: 0000000080050033
>>>> [ 23.750691] CR2: 00000000f7f54faf CR3: 000000000722c000 CR4: 00000000001006e0
>>>> [ 23.750696] Call Trace:
>>>> [ 23.750713] debug_dma_map_sg+0x100/0x340
>>>> [ 23.750727] ? dma_direct_map_sg+0x3b/0xb0
>>>> [ 23.750739] spi_map_buf+0x25a/0x300
>>>> [ 23.750751] __spi_pump_messages+0x2a4/0x680
>>>> [ 23.750762] __spi_sync+0x1dd/0x1f0
>>>> [ 23.750773] spi_sync+0x26/0x40
>>>> [ 23.750790] mipi_dbi_typec3_command_read+0x14d/0x240 [mipi_dbi]
>>>> [ 23.750802] ? spi_finalize_current_transfer+0x10/0x10
>>>> [ 23.750821] mipi_dbi_typec3_command+0x1bc/0x1d0 [mipi_dbi]
>>>>
>>>
>>> After few runs I don't see the warning anymore. So,
>>> Tested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>>>
>>
>> Can I have an ack as well if you're satisfied with it?
>
> Sure.
>
> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>
> I hope to see this in v5.1-rc1.
The patch doesn't apply cleanly on (to be) 5.1 due to this one:
drm/tinydrm: Use struct drm_rect
https://cgit.freedesktop.org/drm/drm/commit/?id=b051b3459bbae907ef068bcd8b62f73f09ea5016
If it's just a debug warning but no ill effects, it'll show up in 5.2.
Otherwise I will have to backport it.
Noralf.
>>
>> Noralf.
>>
>>> Though I would like to give few more days to monitor the state.
>>> (I believe it's fixed)
>>>
>>>> Reported-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>>>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>>>> ---
>>>> drivers/gpu/drm/tinydrm/ili9225.c | 6 ++--
>>>> drivers/gpu/drm/tinydrm/mipi-dbi.c | 58 +++++++++++++++++++++---------
>>>> include/drm/tinydrm/mipi-dbi.h | 5 +--
>>>> 3 files changed, 48 insertions(+), 21 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/tinydrm/ili9225.c b/drivers/gpu/drm/tinydrm/ili9225.c
>>>> index 3f59cfbd31ba..a87078667e04 100644
>>>> --- a/drivers/gpu/drm/tinydrm/ili9225.c
>>>> +++ b/drivers/gpu/drm/tinydrm/ili9225.c
>>>> @@ -299,7 +299,7 @@ static void ili9225_pipe_disable(struct drm_simple_display_pipe *pipe)
>>>> mipi->enabled = false;
>>>> }
>>>>
>>>> -static int ili9225_dbi_command(struct mipi_dbi *mipi, u8 cmd, u8 *par,
>>>> +static int ili9225_dbi_command(struct mipi_dbi *mipi, u8 *cmd, u8 *par,
>>>> size_t num)
>>>> {
>>>> struct spi_device *spi = mipi->spi;
>>>> @@ -309,11 +309,11 @@ static int ili9225_dbi_command(struct mipi_dbi *mipi, u8 cmd, u8 *par,
>>>>
>>>> gpiod_set_value_cansleep(mipi->dc, 0);
>>>> speed_hz = mipi_dbi_spi_cmd_max_speed(spi, 1);
>>>> - ret = tinydrm_spi_transfer(spi, speed_hz, NULL, 8, &cmd, 1);
>>>> + ret = tinydrm_spi_transfer(spi, speed_hz, NULL, 8, cmd, 1);
>>>> if (ret || !num)
>>>> return ret;
>>>>
>>>> - if (cmd == ILI9225_WRITE_DATA_TO_GRAM && !mipi->swap_bytes)
>>>> + if (*cmd == ILI9225_WRITE_DATA_TO_GRAM && !mipi->swap_bytes)
>>>> bpw = 16;
>>>>
>>>> gpiod_set_value_cansleep(mipi->dc, 1);
>>>> diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c
>>>> index 246e708a9ff7..4a4cd7e72938 100644
>>>> --- a/drivers/gpu/drm/tinydrm/mipi-dbi.c
>>>> +++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c
>>>> @@ -153,16 +153,42 @@ EXPORT_SYMBOL(mipi_dbi_command_read);
>>>> */
>>>> int mipi_dbi_command_buf(struct mipi_dbi *mipi, u8 cmd, u8 *data, size_t len)
>>>> {
>>>> + u8 *cmdbuf;
>>>> int ret;
>>>>
>>>> + /* SPI requires dma-safe buffers */
>>>> + cmdbuf = kmemdup(&cmd, 1, GFP_KERNEL);
>>>> + if (!cmdbuf)
>>>> + return -ENOMEM;
>>>> +
>>>> mutex_lock(&mipi->cmdlock);
>>>> - ret = mipi->command(mipi, cmd, data, len);
>>>> + ret = mipi->command(mipi, cmdbuf, data, len);
>>>> mutex_unlock(&mipi->cmdlock);
>>>>
>>>> + kfree(cmdbuf);
>>>> +
>>>> return ret;
>>>> }
>>>> EXPORT_SYMBOL(mipi_dbi_command_buf);
>>>>
>>>> +/* This should only be used by mipi_dbi_command() */
>>>> +int mipi_dbi_command_stackbuf(struct mipi_dbi *mipi, u8 cmd, u8 *data, size_t len)
>>>> +{
>>>> + u8 *buf;
>>>> + int ret;
>>>> +
>>>> + buf = kmemdup(data, len, GFP_KERNEL);
>>>> + if (!buf)
>>>> + return -ENOMEM;
>>>> +
>>>> + ret = mipi_dbi_command_buf(mipi, cmd, buf, len);
>>>> +
>>>> + kfree(buf);
>>>> +
>>>> + return ret;
>>>> +}
>>>> +EXPORT_SYMBOL(mipi_dbi_command_stackbuf);
>>>> +
>>>> /**
>>>> * mipi_dbi_buf_copy - Copy a framebuffer, transforming it if necessary
>>>> * @dst: The destination buffer
>>>> @@ -772,18 +798,18 @@ static int mipi_dbi_spi1_transfer(struct mipi_dbi *mipi, int dc,
>>>> return 0;
>>>> }
>>>>
>>>> -static int mipi_dbi_typec1_command(struct mipi_dbi *mipi, u8 cmd,
>>>> +static int mipi_dbi_typec1_command(struct mipi_dbi *mipi, u8 *cmd,
>>>> u8 *parameters, size_t num)
>>>> {
>>>> - unsigned int bpw = (cmd == MIPI_DCS_WRITE_MEMORY_START) ? 16 : 8;
>>>> + unsigned int bpw = (*cmd == MIPI_DCS_WRITE_MEMORY_START) ? 16 : 8;
>>>> int ret;
>>>>
>>>> - if (mipi_dbi_command_is_read(mipi, cmd))
>>>> + if (mipi_dbi_command_is_read(mipi, *cmd))
>>>> return -ENOTSUPP;
>>>>
>>>> - MIPI_DBI_DEBUG_COMMAND(cmd, parameters, num);
>>>> + MIPI_DBI_DEBUG_COMMAND(*cmd, parameters, num);
>>>>
>>>> - ret = mipi_dbi_spi1_transfer(mipi, 0, &cmd, 1, 8);
>>>> + ret = mipi_dbi_spi1_transfer(mipi, 0, cmd, 1, 8);
>>>> if (ret || !num)
>>>> return ret;
>>>>
>>>> @@ -792,7 +818,7 @@ static int mipi_dbi_typec1_command(struct mipi_dbi *mipi, u8 cmd,
>>>>
>>>> /* MIPI DBI Type C Option 3 */
>>>>
>>>> -static int mipi_dbi_typec3_command_read(struct mipi_dbi *mipi, u8 cmd,
>>>> +static int mipi_dbi_typec3_command_read(struct mipi_dbi *mipi, u8 *cmd,
>>>> u8 *data, size_t len)
>>>> {
>>>> struct spi_device *spi = mipi->spi;
>>>> @@ -801,7 +827,7 @@ static int mipi_dbi_typec3_command_read(struct mipi_dbi *mipi, u8 cmd,
>>>> struct spi_transfer tr[2] = {
>>>> {
>>>> .speed_hz = speed_hz,
>>>> - .tx_buf = &cmd,
>>>> + .tx_buf = cmd,
>>>> .len = 1,
>>>> }, {
>>>> .speed_hz = speed_hz,
>>>> @@ -819,8 +845,8 @@ static int mipi_dbi_typec3_command_read(struct mipi_dbi *mipi, u8 cmd,
>>>> * Support non-standard 24-bit and 32-bit Nokia read commands which
>>>> * start with a dummy clock, so we need to read an extra byte.
>>>> */
>>>> - if (cmd == MIPI_DCS_GET_DISPLAY_ID ||
>>>> - cmd == MIPI_DCS_GET_DISPLAY_STATUS) {
>>>> + if (*cmd == MIPI_DCS_GET_DISPLAY_ID ||
>>>> + *cmd == MIPI_DCS_GET_DISPLAY_STATUS) {
>>>> if (!(len == 3 || len == 4))
>>>> return -EINVAL;
>>>>
>>>> @@ -850,7 +876,7 @@ static int mipi_dbi_typec3_command_read(struct mipi_dbi *mipi, u8 cmd,
>>>> data[i] = (buf[i] << 1) | !!(buf[i + 1] & BIT(7));
>>>> }
>>>>
>>>> - MIPI_DBI_DEBUG_COMMAND(cmd, data, len);
>>>> + MIPI_DBI_DEBUG_COMMAND(*cmd, data, len);
>>>>
>>>> err_free:
>>>> kfree(buf);
>>>> @@ -858,7 +884,7 @@ static int mipi_dbi_typec3_command_read(struct mipi_dbi *mipi, u8 cmd,
>>>> return ret;
>>>> }
>>>>
>>>> -static int mipi_dbi_typec3_command(struct mipi_dbi *mipi, u8 cmd,
>>>> +static int mipi_dbi_typec3_command(struct mipi_dbi *mipi, u8 *cmd,
>>>> u8 *par, size_t num)
>>>> {
>>>> struct spi_device *spi = mipi->spi;
>>>> @@ -866,18 +892,18 @@ static int mipi_dbi_typec3_command(struct mipi_dbi *mipi, u8 cmd,
>>>> u32 speed_hz;
>>>> int ret;
>>>>
>>>> - if (mipi_dbi_command_is_read(mipi, cmd))
>>>> + if (mipi_dbi_command_is_read(mipi, *cmd))
>>>> return mipi_dbi_typec3_command_read(mipi, cmd, par, num);
>>>>
>>>> - MIPI_DBI_DEBUG_COMMAND(cmd, par, num);
>>>> + MIPI_DBI_DEBUG_COMMAND(*cmd, par, num);
>>>>
>>>> gpiod_set_value_cansleep(mipi->dc, 0);
>>>> speed_hz = mipi_dbi_spi_cmd_max_speed(spi, 1);
>>>> - ret = tinydrm_spi_transfer(spi, speed_hz, NULL, 8, &cmd, 1);
>>>> + ret = tinydrm_spi_transfer(spi, speed_hz, NULL, 8, cmd, 1);
>>>> if (ret || !num)
>>>> return ret;
>>>>
>>>> - if (cmd == MIPI_DCS_WRITE_MEMORY_START && !mipi->swap_bytes)
>>>> + if (*cmd == MIPI_DCS_WRITE_MEMORY_START && !mipi->swap_bytes)
>>>> bpw = 16;
>>>>
>>>> gpiod_set_value_cansleep(mipi->dc, 1);
>>>> diff --git a/include/drm/tinydrm/mipi-dbi.h b/include/drm/tinydrm/mipi-dbi.h
>>>> index ad7e6bedab5f..464a47d1013d 100644
>>>> --- a/include/drm/tinydrm/mipi-dbi.h
>>>> +++ b/include/drm/tinydrm/mipi-dbi.h
>>>> @@ -43,7 +43,7 @@ struct mipi_dbi {
>>>> struct spi_device *spi;
>>>> bool enabled;
>>>> struct mutex cmdlock;
>>>> - int (*command)(struct mipi_dbi *mipi, u8 cmd, u8 *param, size_t num);
>>>> + int (*command)(struct mipi_dbi *mipi, u8 *cmd, u8 *param, size_t num);
>>>> const u8 *read_commands;
>>>> struct gpio_desc *dc;
>>>> u16 *tx_buf;
>>>> @@ -83,6 +83,7 @@ u32 mipi_dbi_spi_cmd_max_speed(struct spi_device *spi, size_t len);
>>>>
>>>> int mipi_dbi_command_read(struct mipi_dbi *mipi, u8 cmd, u8 *val);
>>>> int mipi_dbi_command_buf(struct mipi_dbi *mipi, u8 cmd, u8 *data, size_t len);
>>>> +int mipi_dbi_command_stackbuf(struct mipi_dbi *mipi, u8 cmd, u8 *data, size_t len);
>>>> int mipi_dbi_buf_copy(void *dst, struct drm_framebuffer *fb,
>>>> struct drm_rect *clip, bool swap);
>>>> /**
>>>> @@ -100,7 +101,7 @@ int mipi_dbi_buf_copy(void *dst, struct drm_framebuffer *fb,
>>>> #define mipi_dbi_command(mipi, cmd, seq...) \
>>>> ({ \
>>>> u8 d[] = { seq }; \
>>>> - mipi_dbi_command_buf(mipi, cmd, d, ARRAY_SIZE(d)); \
>>>> + mipi_dbi_command_stackbuf(mipi, cmd, d, ARRAY_SIZE(d)); \
>>>> })
>>>>
>>>> #ifdef CONFIG_DEBUG_FS
>>>> --
>>>> 2.20.1
>>>>
>>>
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tinydrm/mipi-dbi: Use dma-safe buffers for all SPI transfers
2019-03-04 17:51 ` Noralf Trønnes
@ 2019-03-05 7:32 ` Andy Shevchenko
0 siblings, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2019-03-05 7:32 UTC (permalink / raw)
To: Noralf Trønnes; +Cc: david, dri-devel
On Mon, Mar 04, 2019 at 06:51:33PM +0100, Noralf Trønnes wrote:
> Den 04.03.2019 16.10, skrev Andy Shevchenko:
> > On Mon, Mar 04, 2019 at 03:45:56PM +0100, Noralf Trønnes wrote:
> >> Den 22.02.2019 16.58, skrev Andy Shevchenko:
> >>> On Fri, Feb 22, 2019 at 01:43:29PM +0100, Noralf Trønnes wrote:
> > Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> >
> > I hope to see this in v5.1-rc1.
>
> The patch doesn't apply cleanly on (to be) 5.1 due to this one:
>
> drm/tinydrm: Use struct drm_rect
> https://cgit.freedesktop.org/drm/drm/commit/?id=b051b3459bbae907ef068bcd8b62f73f09ea5016
>
> If it's just a debug warning but no ill effects, it'll show up in 5.2.
> Otherwise I will have to backport it.
For x86 it's just a debug message, since on x86 stack memory is DMA capable.
But in general this should be fixed for any possible scenarios.
--
With Best Regards,
Andy Shevchenko
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-03-05 7:32 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-22 12:43 [PATCH] tinydrm/mipi-dbi: Use dma-safe buffers for all SPI transfers Noralf Trønnes
2019-02-22 15:58 ` Andy Shevchenko
2019-03-04 14:45 ` Noralf Trønnes
2019-03-04 15:10 ` Andy Shevchenko
2019-03-04 17:51 ` Noralf Trønnes
2019-03-05 7:32 ` Andy Shevchenko
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.