This series removes the remaining bits of tinydrm.ko.
Changes: - Split SPI connector type patch in core and driver changes, expand commit message (Daniel) - Drop moving the mipi_dbi_spi_init() declaration (Sam) - st7586: Forgot to remove the mipi->rotation assignment
Note: This depends on a commit that just entered Linus' tree: e6f3f7e4dc76 ("spi: Add spi_is_bpw_supported()")
Series is also available here: https://github.com/notro/linux/tree/remove_tinydrm_ko
Noralf.
Noralf Trønnes (11): drm: Add SPI connector type drm/tinydrm: Use DRM_MODE_CONNECTOR_SPI drm/tinydrm: Use spi_is_bpw_supported() drm/tinydrm: Remove spi debug buffer dumping drm/tinydrm: Remove tinydrm_spi_max_transfer_size() drm/tinydrm: Clean up tinydrm_spi_transfer() drm/tinydrm: Move tinydrm_spi_transfer() drm/tinydrm: Move tinydrm_machine_little_endian() drm/tinydrm/repaper: Don't use tinydrm_display_pipe_init() drm/tinydrm/mipi-dbi: Add mipi_dbi_init_with_formats() drm/tinydrm: Move tinydrm_display_pipe_init() to mipi-dbi
Documentation/gpu/tinydrm.rst | 12 - Documentation/gpu/todo.rst | 3 - drivers/gpu/drm/drm_connector.c | 1 + drivers/gpu/drm/tinydrm/Makefile | 1 - drivers/gpu/drm/tinydrm/core/Makefile | 4 - .../gpu/drm/tinydrm/core/tinydrm-helpers.c | 207 -------------- drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c | 179 ------------ drivers/gpu/drm/tinydrm/hx8357d.c | 1 - drivers/gpu/drm/tinydrm/ili9225.c | 5 +- drivers/gpu/drm/tinydrm/ili9341.c | 1 - drivers/gpu/drm/tinydrm/mi0283qt.c | 1 - drivers/gpu/drm/tinydrm/mipi-dbi.c | 254 ++++++++++++++---- drivers/gpu/drm/tinydrm/repaper.c | 58 +++- drivers/gpu/drm/tinydrm/st7586.c | 33 +-- drivers/gpu/drm/tinydrm/st7735r.c | 1 - include/drm/tinydrm/mipi-dbi.h | 18 ++ include/drm/tinydrm/tinydrm-helpers.h | 75 ------ include/uapi/drm/drm_mode.h | 1 + 18 files changed, 283 insertions(+), 572 deletions(-) delete mode 100644 drivers/gpu/drm/tinydrm/core/Makefile delete mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c delete mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c delete mode 100644 include/drm/tinydrm/tinydrm-helpers.h
tinydrm drivers announce DRM_MODE_CONNECTOR_VIRTUAL for its SPI drivers. Add a SPI connector type to match the actual connector.
X will list the connector as Unknown:
X.Org X Server 1.19.2 Release Date: 2017-03-02 <...> [ 53523.905] (II) modeset(0): Output Unknown19-1 has no monitor section [ 53523.908] (II) modeset(0): EDID for output Unknown19-1 [ 53523.910] (II) modeset(0): Printing probed modes for output Unknown19-1 [ 53523.911] (II) modeset(0): Modeline "320x240"x0.0 0.00 320 320 320 320 240 240 240 240 (0.0 kHz eP) [ 53523.911] (II) modeset(0): Output Unknown19-1 connected [ 53523.912] (II) modeset(0): Using exact sizes for initial modes [ 53523.912] (II) modeset(0): Output Unknown19-1 using initial mode 320x240 +0+0
The weston source shows that it will be listed as UNNAMED.
v2: Split patch in core and driver changes, expand commit message (Daniel)
Cc: Daniel Vetter daniel.vetter@ffwll.ch Reviewed-by: Sam Ravnborg sam@ravnborg.org Acked-by: David Lechner david@lechnology.com Signed-off-by: Noralf Trønnes noralf@tronnes.org --- drivers/gpu/drm/drm_connector.c | 1 + include/uapi/drm/drm_mode.h | 1 + 2 files changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 068d4b05f1be..cbb548b3708f 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -92,6 +92,7 @@ static struct drm_conn_prop_enum_list drm_connector_enum_list[] = { { DRM_MODE_CONNECTOR_DSI, "DSI" }, { DRM_MODE_CONNECTOR_DPI, "DPI" }, { DRM_MODE_CONNECTOR_WRITEBACK, "Writeback" }, + { DRM_MODE_CONNECTOR_SPI, "SPI" }, };
void drm_connector_ida_init(void) diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index 5ab331e5dc23..735c8cfdaaa1 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -361,6 +361,7 @@ enum drm_mode_subconnector { #define DRM_MODE_CONNECTOR_DSI 16 #define DRM_MODE_CONNECTOR_DPI 17 #define DRM_MODE_CONNECTOR_WRITEBACK 18 +#define DRM_MODE_CONNECTOR_SPI 19
struct drm_mode_get_connector {
tinydrm drivers announce DRM_MODE_CONNECTOR_VIRTUAL for its SPI drivers. Use the new SPI connector type instead.
X server will now list the connector as Unknown instead of Virtual:
X.Org X Server 1.19.2 Release Date: 2017-03-02 <...> [ 53523.905] (II) modeset(0): Output Unknown19-1 has no monitor section [ 53523.908] (II) modeset(0): EDID for output Unknown19-1 [ 53523.910] (II) modeset(0): Printing probed modes for output Unknown19-1 [ 53523.911] (II) modeset(0): Modeline "320x240"x0.0 0.00 320 320 320 320 240 240 240 240 (0.0 kHz eP) [ 53523.911] (II) modeset(0): Output Unknown19-1 connected [ 53523.912] (II) modeset(0): Using exact sizes for initial modes [ 53523.912] (II) modeset(0): Output Unknown19-1 using initial mode 320x240 +0+0
I won't chase down and fix userspace, but the new connector type will trickle out to userspace eventually.
v2: Split patch in core and driver changes, expand commit message (Daniel)
Cc: David Lechner david@lechnology.com Reviewed-by: Sam Ravnborg sam@ravnborg.org Acked-by: David Lechner david@lechnology.com Signed-off-by: Noralf Trønnes noralf@tronnes.org --- drivers/gpu/drm/tinydrm/mipi-dbi.c | 3 +-- drivers/gpu/drm/tinydrm/repaper.c | 2 +- drivers/gpu/drm/tinydrm/st7586.c | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c index ca9da654fc6f..791a0b43beef 100644 --- a/drivers/gpu/drm/tinydrm/mipi-dbi.c +++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c @@ -445,9 +445,8 @@ int mipi_dbi_init(struct mipi_dbi *mipi, if (!mipi->tx_buf) return -ENOMEM;
- /* TODO: Maybe add DRM_MODE_CONNECTOR_SPI */ ret = tinydrm_display_pipe_init(drm, &mipi->pipe, funcs, - DRM_MODE_CONNECTOR_VIRTUAL, + DRM_MODE_CONNECTOR_SPI, mipi_dbi_formats, ARRAY_SIZE(mipi_dbi_formats), mode, rotation); diff --git a/drivers/gpu/drm/tinydrm/repaper.c b/drivers/gpu/drm/tinydrm/repaper.c index 85acfccefcdb..40afa66107e5 100644 --- a/drivers/gpu/drm/tinydrm/repaper.c +++ b/drivers/gpu/drm/tinydrm/repaper.c @@ -1110,7 +1110,7 @@ static int repaper_probe(struct spi_device *spi) return -ENOMEM;
ret = tinydrm_display_pipe_init(drm, &epd->pipe, &repaper_pipe_funcs, - DRM_MODE_CONNECTOR_VIRTUAL, + DRM_MODE_CONNECTOR_SPI, repaper_formats, ARRAY_SIZE(repaper_formats), mode, 0); if (ret) diff --git a/drivers/gpu/drm/tinydrm/st7586.c b/drivers/gpu/drm/tinydrm/st7586.c index 204face7b311..7ae39004aa88 100644 --- a/drivers/gpu/drm/tinydrm/st7586.c +++ b/drivers/gpu/drm/tinydrm/st7586.c @@ -384,7 +384,7 @@ static int st7586_probe(struct spi_device *spi) mipi->swap_bytes = true;
ret = tinydrm_display_pipe_init(drm, &mipi->pipe, &st7586_pipe_funcs, - DRM_MODE_CONNECTOR_VIRTUAL, + DRM_MODE_CONNECTOR_SPI, st7586_formats, ARRAY_SIZE(st7586_formats), &st7586_mode, rotation); if (ret)
This means that tinydrm_spi_bpw_supported() can be removed.
Reviewed-by: Sam Ravnborg sam@ravnborg.org Signed-off-by: Noralf Trønnes noralf@tronnes.org --- .../gpu/drm/tinydrm/core/tinydrm-helpers.c | 32 +------------------ drivers/gpu/drm/tinydrm/mipi-dbi.c | 5 ++- include/drm/tinydrm/tinydrm-helpers.h | 1 - 3 files changed, 3 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c index dfeafac4c656..aeb49cefed25 100644 --- a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c +++ b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c @@ -53,36 +53,6 @@ size_t tinydrm_spi_max_transfer_size(struct spi_device *spi, size_t max_len) } 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 - * - * This function checks to see if the SPI master driver supports @bpw. - * - * Returns: - * True if @bpw is supported, false otherwise. - */ -bool tinydrm_spi_bpw_supported(struct spi_device *spi, u8 bpw) -{ - u32 bpw_mask = spi->master->bits_per_word_mask; - - if (bpw == 8) - return true; - - if (!bpw_mask) { - dev_warn_once(&spi->dev, - "bits_per_word_mask not set, assume 8-bit only\n"); - return false; - } - - if (bpw_mask & SPI_BPW_MASK(bpw)) - return true; - - return false; -} -EXPORT_SYMBOL(tinydrm_spi_bpw_supported); - static void tinydrm_dbg_spi_print(struct spi_device *spi, struct spi_transfer *tr, const void *buf, int idx, bool tx) @@ -159,7 +129,7 @@ int tinydrm_spi_transfer(struct spi_device *spi, u32 speed_hz, pr_debug("[drm:%s] bpw=%u, max_chunk=%zu, transfers:\n", __func__, bpw, max_chunk);
- if (bpw == 16 && !tinydrm_spi_bpw_supported(spi, 16)) { + if (bpw == 16 && !spi_is_bpw_supported(spi, 16)) { tr.bits_per_word = 8; if (tinydrm_machine_little_endian()) { swap_buf = kmalloc(min(len, max_chunk), GFP_KERNEL); diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c index 791a0b43beef..b6c46453e904 100644 --- a/drivers/gpu/drm/tinydrm/mipi-dbi.c +++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c @@ -782,7 +782,7 @@ static int mipi_dbi_spi1_transfer(struct mipi_dbi *mipi, int dc, u16 *dst16; int ret;
- if (!tinydrm_spi_bpw_supported(spi, 9)) + if (!spi_is_bpw_supported(spi, 9)) return mipi_dbi_spi1e_transfer(mipi, dc, buf, len, bpw);
tr.speed_hz = mipi_dbi_spi_cmd_max_speed(spi, len); @@ -1003,8 +1003,7 @@ int mipi_dbi_spi_init(struct spi_device *spi, struct mipi_dbi *mipi, if (dc) { mipi->command = mipi_dbi_typec3_command; mipi->dc = dc; - if (tinydrm_machine_little_endian() && - !tinydrm_spi_bpw_supported(spi, 16)) + if (tinydrm_machine_little_endian() && !spi_is_bpw_supported(spi, 16)) mipi->swap_bytes = true; } else { mipi->command = mipi_dbi_typec1_command; diff --git a/include/drm/tinydrm/tinydrm-helpers.h b/include/drm/tinydrm/tinydrm-helpers.h index f8bcadf48cb1..146bc383297c 100644 --- a/include/drm/tinydrm/tinydrm-helpers.h +++ b/include/drm/tinydrm/tinydrm-helpers.h @@ -43,7 +43,6 @@ int tinydrm_display_pipe_init(struct drm_device *drm, unsigned int rotation);
size_t tinydrm_spi_max_transfer_size(struct spi_device *spi, size_t max_len); -bool tinydrm_spi_bpw_supported(struct spi_device *spi, u8 bpw); int tinydrm_spi_transfer(struct spi_device *spi, u32 speed_hz, struct spi_transfer *header, u8 bpw, const void *buf, size_t len);
The SPI event tracing can dump the buffer now so no need for this. Remove the debug print from tinydrm_spi_transfer() since this info can be gleaned from the trace event.
Reviewed-by: Sam Ravnborg sam@ravnborg.org Signed-off-by: Noralf Trønnes noralf@tronnes.org --- .../gpu/drm/tinydrm/core/tinydrm-helpers.c | 40 ------------------- drivers/gpu/drm/tinydrm/mipi-dbi.c | 6 --- include/drm/tinydrm/tinydrm-helpers.h | 25 ------------ 3 files changed, 71 deletions(-)
diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c index aeb49cefed25..272616a246cd 100644 --- a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c +++ b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c @@ -53,41 +53,6 @@ size_t tinydrm_spi_max_transfer_size(struct spi_device *spi, size_t max_len) } EXPORT_SYMBOL(tinydrm_spi_max_transfer_size);
-static void -tinydrm_dbg_spi_print(struct spi_device *spi, struct spi_transfer *tr, - const void *buf, int idx, bool tx) -{ - u32 speed_hz = tr->speed_hz ? tr->speed_hz : spi->max_speed_hz; - char linebuf[3 * 32]; - - hex_dump_to_buffer(buf, tr->len, 16, - DIV_ROUND_UP(tr->bits_per_word, 8), - linebuf, sizeof(linebuf), false); - - printk(KERN_DEBUG - " tr(%i): speed=%u%s, bpw=%i, len=%u, %s_buf=[%s%s]\n", idx, - speed_hz > 1000000 ? speed_hz / 1000000 : speed_hz / 1000, - speed_hz > 1000000 ? "MHz" : "kHz", tr->bits_per_word, tr->len, - tx ? "tx" : "rx", linebuf, tr->len > 16 ? " ..." : ""); -} - -/* called through tinydrm_dbg_spi_message() */ -void _tinydrm_dbg_spi_message(struct spi_device *spi, struct spi_message *m) -{ - struct spi_transfer *tmp; - int i = 0; - - list_for_each_entry(tmp, &m->transfers, transfer_list) { - - if (tmp->tx_buf) - tinydrm_dbg_spi_print(spi, tmp, tmp->tx_buf, i, true); - if (tmp->rx_buf) - tinydrm_dbg_spi_print(spi, tmp, tmp->rx_buf, i, false); - i++; - } -} -EXPORT_SYMBOL(_tinydrm_dbg_spi_message); - /** * tinydrm_spi_transfer - SPI transfer helper * @spi: SPI device @@ -125,10 +90,6 @@ int tinydrm_spi_transfer(struct spi_device *spi, u32 speed_hz,
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 && !spi_is_bpw_supported(spi, 16)) { tr.bits_per_word = 8; if (tinydrm_machine_little_endian()) { @@ -162,7 +123,6 @@ int tinydrm_spi_transfer(struct spi_device *spi, u32 speed_hz, buf += chunk; len -= chunk;
- tinydrm_dbg_spi_message(spi, &m); ret = spi_sync(spi, &m); if (ret) return ret; diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c index b6c46453e904..99509d16b037 100644 --- a/drivers/gpu/drm/tinydrm/mipi-dbi.c +++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c @@ -679,8 +679,6 @@ static int mipi_dbi_spi1e_transfer(struct mipi_dbi *mipi, int dc, dst[8] = *src; tr.len = 9;
- tinydrm_dbg_spi_message(spi, &m); - return spi_sync(spi, &m); }
@@ -758,7 +756,6 @@ static int mipi_dbi_spi1e_transfer(struct mipi_dbi *mipi, int dc,
tr.len = chunk + added;
- tinydrm_dbg_spi_message(spi, &m); ret = spi_sync(spi, &m); if (ret) return ret; @@ -822,7 +819,6 @@ static int mipi_dbi_spi1_transfer(struct mipi_dbi *mipi, int dc, tr.len = chunk; len -= chunk;
- tinydrm_dbg_spi_message(spi, &m); ret = spi_sync(spi, &m); if (ret) return ret; @@ -898,8 +894,6 @@ static int mipi_dbi_typec3_command_read(struct mipi_dbi *mipi, u8 *cmd, if (ret) goto err_free;
- tinydrm_dbg_spi_message(spi, &m); - if (tr[1].len == len) { memcpy(data, buf, len); } else { diff --git a/include/drm/tinydrm/tinydrm-helpers.h b/include/drm/tinydrm/tinydrm-helpers.h index 146bc383297c..dca75de3a359 100644 --- a/include/drm/tinydrm/tinydrm-helpers.h +++ b/include/drm/tinydrm/tinydrm-helpers.h @@ -14,7 +14,6 @@ struct drm_rect; struct drm_simple_display_pipe; struct drm_simple_display_pipe_funcs; struct spi_transfer; -struct spi_message; struct spi_device; struct device;
@@ -46,29 +45,5 @@ size_t tinydrm_spi_max_transfer_size(struct spi_device *spi, size_t max_len); int tinydrm_spi_transfer(struct spi_device *spi, u32 speed_hz, struct spi_transfer *header, u8 bpw, const void *buf, size_t len); -void _tinydrm_dbg_spi_message(struct spi_device *spi, struct spi_message *m); - -#ifdef DEBUG -/** - * tinydrm_dbg_spi_message - Dump SPI message - * @spi: SPI device - * @m: SPI message - * - * Dumps info about the transfers in a SPI message including buffer content. - * DEBUG has to be defined for this function to be enabled alongside setting - * the DRM_UT_DRIVER bit of &drm_debug. - */ -static inline void tinydrm_dbg_spi_message(struct spi_device *spi, - struct spi_message *m) -{ - if (drm_debug & DRM_UT_DRIVER) - _tinydrm_dbg_spi_message(spi, m); -} -#else -static inline void tinydrm_dbg_spi_message(struct spi_device *spi, - struct spi_message *m) -{ -} -#endif /* DEBUG */
#endif /* __LINUX_TINYDRM_HELPERS_H */
spi-bcm2835 can handle >64kB buffers now so there is no need to check ->max_dma_len. The tinydrm_spi_max_transfer_size() max_len argument is not used by any callers, so not needed.
Then we have the spi_max module parameter. It was added because staging/fbtft has support for it and there was a report that someone used it to set a small buffer size to avoid popping on a USB soundcard on a Raspberry Pi. In hindsight it shouldn't have been added, I should have waited for it to become a problem first. I don't know it anyone is actually using it, but since tinydrm_spi_transfer() is being moved to mipi-dbi, I'm taking the opportunity to remove it. I'll add it back to mipi-dbi if someone complains.
With that out of the way, spi_max_transfer_size() can be used instead.
The chosen 16kB buffer size for Type C Option 1 (9-bit) interface is somewhat arbitrary, but a bigger buffer will have a miniscule impact on transfer speed, so it's probably fine.
Acked-by: Sam Ravnborg sam@ravnborg.org Signed-off-by: Noralf Trønnes noralf@tronnes.org --- .../gpu/drm/tinydrm/core/tinydrm-helpers.c | 37 +------------------ drivers/gpu/drm/tinydrm/mipi-dbi.c | 10 +---- include/drm/tinydrm/tinydrm-helpers.h | 1 - 3 files changed, 3 insertions(+), 45 deletions(-)
diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c index 272616a246cd..af5bec8861de 100644 --- a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c +++ b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c @@ -18,41 +18,8 @@ #include <drm/drm_rect.h> #include <drm/tinydrm/tinydrm-helpers.h>
-static unsigned int spi_max; -module_param(spi_max, uint, 0400); -MODULE_PARM_DESC(spi_max, "Set a lower SPI max transfer size"); - #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_transfer - SPI transfer helper * @spi: SPI device @@ -75,21 +42,19 @@ int tinydrm_spi_transfer(struct spi_device *spi, u32 speed_hz, struct spi_transfer *header, u8 bpw, const void *buf, size_t len) { + size_t max_chunk = spi_max_transfer_size(spi); struct spi_transfer tr = { .bits_per_word = bpw, .speed_hz = speed_hz, }; 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 (bpw == 16 && !spi_is_bpw_supported(spi, 16)) { tr.bits_per_word = 8; if (tinydrm_machine_little_endian()) { diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c index 99509d16b037..ae31a5c9aa1b 100644 --- a/drivers/gpu/drm/tinydrm/mipi-dbi.c +++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c @@ -964,15 +964,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 @@ -1001,8 +995,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/include/drm/tinydrm/tinydrm-helpers.h b/include/drm/tinydrm/tinydrm-helpers.h index dca75de3a359..10b35375a009 100644 --- a/include/drm/tinydrm/tinydrm-helpers.h +++ b/include/drm/tinydrm/tinydrm-helpers.h @@ -41,7 +41,6 @@ int tinydrm_display_pipe_init(struct drm_device *drm, const struct drm_display_mode *mode, unsigned int rotation);
-size_t tinydrm_spi_max_transfer_size(struct spi_device *spi, size_t max_len); int tinydrm_spi_transfer(struct spi_device *spi, u32 speed_hz, struct spi_transfer *header, u8 bpw, const void *buf, size_t len);
On Fri, Jul 19, 2019 at 05:59:10PM +0200, Noralf Trønnes wrote:
spi-bcm2835 can handle >64kB buffers now so there is no need to check ->max_dma_len. The tinydrm_spi_max_transfer_size() max_len argument is not used by any callers, so not needed.
Then we have the spi_max module parameter. It was added because staging/fbtft has support for it and there was a report that someone used it to set a small buffer size to avoid popping on a USB soundcard on a Raspberry Pi. In hindsight it shouldn't have been added, I should have waited for it to become a problem first. I don't know it anyone is actually using it, but since tinydrm_spi_transfer() is being moved to mipi-dbi, I'm taking the opportunity to remove it. I'll add it back to mipi-dbi if someone complains.
With that out of the way, spi_max_transfer_size() can be used instead.
The chosen 16kB buffer size for Type C Option 1 (9-bit) interface is somewhat arbitrary, but a bigger buffer will have a miniscule impact on transfer speed, so it's probably fine.
This breaks the SPI PXA2xx case I'm using. The world is not a Pi:e.
[ 388.445752] mi0283qt spi-PRP0001:01: DMA disabled for transfer length 153600 greater than 65536 [ 388.634437] mi0283qt spi-PRP0001:01: DMA disabled for transfer length 153600 greater than 65536 [ 388.822933] mi0283qt spi-PRP0001:01: DMA disabled for transfer length 153600 greater than 65536
The crucial thing is to check the transfer size against maximum DMA length of the master.
Please, fix.
On Tue, Oct 15, 2019 at 05:32:36PM +0300, Andy Shevchenko wrote:
On Fri, Jul 19, 2019 at 05:59:10PM +0200, Noralf Trønnes wrote:
spi-bcm2835 can handle >64kB buffers now so there is no need to check ->max_dma_len. The tinydrm_spi_max_transfer_size() max_len argument is not used by any callers, so not needed.
Then we have the spi_max module parameter. It was added because staging/fbtft has support for it and there was a report that someone used it to set a small buffer size to avoid popping on a USB soundcard on a Raspberry Pi. In hindsight it shouldn't have been added, I should have waited for it to become a problem first. I don't know it anyone is actually using it, but since tinydrm_spi_transfer() is being moved to mipi-dbi, I'm taking the opportunity to remove it. I'll add it back to mipi-dbi if someone complains.
With that out of the way, spi_max_transfer_size() can be used instead.
The chosen 16kB buffer size for Type C Option 1 (9-bit) interface is somewhat arbitrary, but a bigger buffer will have a miniscule impact on transfer speed, so it's probably fine.
This breaks the SPI PXA2xx case I'm using. The world is not a Pi:e.
[ 388.445752] mi0283qt spi-PRP0001:01: DMA disabled for transfer length 153600 greater than 65536 [ 388.634437] mi0283qt spi-PRP0001:01: DMA disabled for transfer length 153600 greater than 65536 [ 388.822933] mi0283qt spi-PRP0001:01: DMA disabled for transfer length 153600 greater than 65536
The crucial thing is to check the transfer size against maximum DMA length of the master.
Please, fix.
Partial revert fixes the issue, though I'm not sure it's the best approach.
--- a/drivers/gpu/drm/drm_mipi_dbi.c +++ b/drivers/gpu/drm/drm_mipi_dbi.c @@ -1147,7 +1147,7 @@ EXPORT_SYMBOL(mipi_dbi_spi_init); int mipi_dbi_spi_transfer(struct spi_device *spi, u32 speed_hz, u8 bpw, const void *buf, size_t len) { - size_t max_chunk = spi_max_transfer_size(spi); + size_t max_chunk = min(spi_max_transfer_size(spi), spi->master->max_dma_len); struct spi_transfer tr = { .bits_per_word = bpw, .speed_hz = speed_hz,
Den 15.10.2019 16.32, skrev Andy Shevchenko:
On Fri, Jul 19, 2019 at 05:59:10PM +0200, Noralf Trønnes wrote:
spi-bcm2835 can handle >64kB buffers now so there is no need to check ->max_dma_len. The tinydrm_spi_max_transfer_size() max_len argument is not used by any callers, so not needed.
Then we have the spi_max module parameter. It was added because staging/fbtft has support for it and there was a report that someone used it to set a small buffer size to avoid popping on a USB soundcard on a Raspberry Pi. In hindsight it shouldn't have been added, I should have waited for it to become a problem first. I don't know it anyone is actually using it, but since tinydrm_spi_transfer() is being moved to mipi-dbi, I'm taking the opportunity to remove it. I'll add it back to mipi-dbi if someone complains.
With that out of the way, spi_max_transfer_size() can be used instead.
The chosen 16kB buffer size for Type C Option 1 (9-bit) interface is somewhat arbitrary, but a bigger buffer will have a miniscule impact on transfer speed, so it's probably fine.
This breaks the SPI PXA2xx case I'm using. The world is not a Pi:e.
[ 388.445752] mi0283qt spi-PRP0001:01: DMA disabled for transfer length 153600 greater than 65536 [ 388.634437] mi0283qt spi-PRP0001:01: DMA disabled for transfer length 153600 greater than 65536 [ 388.822933] mi0283qt spi-PRP0001:01: DMA disabled for transfer length 153600 greater than 65536
The crucial thing is to check the transfer size against maximum DMA length of the master.
Isn't this a spi controller driver problem? spi_max_transfer_size() tells the client what the maximum transfer length is. The controller driver can use ctlr->max_transfer_size if it has restrictions. AFAIUI max_dma_len is used when splitting up the buffer for the sg table in spi_map_buf().
Noralf.
Please, fix.
On Tue, Oct 15, 2019 at 05:41:53PM +0200, Noralf Trønnes wrote:
Den 15.10.2019 16.32, skrev Andy Shevchenko:
On Fri, Jul 19, 2019 at 05:59:10PM +0200, Noralf Trønnes wrote:
spi-bcm2835 can handle >64kB buffers now so there is no need to check ->max_dma_len. The tinydrm_spi_max_transfer_size() max_len argument is not used by any callers, so not needed.
Then we have the spi_max module parameter. It was added because staging/fbtft has support for it and there was a report that someone used it to set a small buffer size to avoid popping on a USB soundcard on a Raspberry Pi. In hindsight it shouldn't have been added, I should have waited for it to become a problem first. I don't know it anyone is actually using it, but since tinydrm_spi_transfer() is being moved to mipi-dbi, I'm taking the opportunity to remove it. I'll add it back to mipi-dbi if someone complains.
With that out of the way, spi_max_transfer_size() can be used instead.
The chosen 16kB buffer size for Type C Option 1 (9-bit) interface is somewhat arbitrary, but a bigger buffer will have a miniscule impact on transfer speed, so it's probably fine.
This breaks the SPI PXA2xx case I'm using. The world is not a Pi:e.
[ 388.445752] mi0283qt spi-PRP0001:01: DMA disabled for transfer length 153600 greater than 65536 [ 388.634437] mi0283qt spi-PRP0001:01: DMA disabled for transfer length 153600 greater than 65536 [ 388.822933] mi0283qt spi-PRP0001:01: DMA disabled for transfer length 153600 greater than 65536
The crucial thing is to check the transfer size against maximum DMA length of the master.
Isn't this a spi controller driver problem? spi_max_transfer_size() tells the client what the maximum transfer length is. The controller driver can use ctlr->max_transfer_size if it has restrictions. AFAIUI max_dma_len is used when splitting up the buffer for the sg table in spi_map_buf().
Something like this, as a test patch.
Cheers, Daniel
diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c index bb6a14d1ab0f..f77201915033 100644 --- a/drivers/spi/spi-pxa2xx.c +++ b/drivers/spi/spi-pxa2xx.c @@ -1707,6 +1707,7 @@ static int pxa2xx_spi_probe(struct platform_device *pdev) } else { controller->can_dma = pxa2xx_spi_can_dma; controller->max_dma_len = MAX_DMA_LEN; + controller->max_transfer_size = MAX_DMA_LEN; } }
On Tue, Oct 15, 2019 at 05:57:20PM +0200, Daniel Vetter wrote:
On Tue, Oct 15, 2019 at 05:41:53PM +0200, Noralf Trønnes wrote:
Den 15.10.2019 16.32, skrev Andy Shevchenko:
On Fri, Jul 19, 2019 at 05:59:10PM +0200, Noralf Trønnes wrote:
spi-bcm2835 can handle >64kB buffers now so there is no need to check ->max_dma_len. The tinydrm_spi_max_transfer_size() max_len argument is not used by any callers, so not needed.
Then we have the spi_max module parameter. It was added because staging/fbtft has support for it and there was a report that someone used it to set a small buffer size to avoid popping on a USB soundcard on a Raspberry Pi. In hindsight it shouldn't have been added, I should have waited for it to become a problem first. I don't know it anyone is actually using it, but since tinydrm_spi_transfer() is being moved to mipi-dbi, I'm taking the opportunity to remove it. I'll add it back to mipi-dbi if someone complains.
With that out of the way, spi_max_transfer_size() can be used instead.
The chosen 16kB buffer size for Type C Option 1 (9-bit) interface is somewhat arbitrary, but a bigger buffer will have a miniscule impact on transfer speed, so it's probably fine.
This breaks the SPI PXA2xx case I'm using. The world is not a Pi:e.
[ 388.445752] mi0283qt spi-PRP0001:01: DMA disabled for transfer length 153600 greater than 65536 [ 388.634437] mi0283qt spi-PRP0001:01: DMA disabled for transfer length 153600 greater than 65536 [ 388.822933] mi0283qt spi-PRP0001:01: DMA disabled for transfer length 153600 greater than 65536
The crucial thing is to check the transfer size against maximum DMA length of the master.
Isn't this a spi controller driver problem? spi_max_transfer_size() tells the client what the maximum transfer length is. The controller driver can use ctlr->max_transfer_size if it has restrictions. AFAIUI max_dma_len is used when splitting up the buffer for the sg table in spi_map_buf().
Something like this, as a test patch.
max_transfer_size should be a function. In that case it works. However I'm not sure it's the best approach, thus, Cc to SPI PXA people.
diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c index bb6a14d1ab0f..f77201915033 100644 --- a/drivers/spi/spi-pxa2xx.c +++ b/drivers/spi/spi-pxa2xx.c @@ -1707,6 +1707,7 @@ static int pxa2xx_spi_probe(struct platform_device *pdev) } else { controller->can_dma = pxa2xx_spi_can_dma; controller->max_dma_len = MAX_DMA_LEN;
controller->max_transfer_size = MAX_DMA_LEN;
On Wed, Oct 16, 2019 at 6:13 PM Andy Shevchenko andriy.shevchenko@intel.com wrote:
On Tue, Oct 15, 2019 at 05:57:20PM +0200, Daniel Vetter wrote:
On Tue, Oct 15, 2019 at 05:41:53PM +0200, Noralf Trønnes wrote:
Den 15.10.2019 16.32, skrev Andy Shevchenko:
On Fri, Jul 19, 2019 at 05:59:10PM +0200, Noralf Trønnes wrote:
spi-bcm2835 can handle >64kB buffers now so there is no need to check ->max_dma_len. The tinydrm_spi_max_transfer_size() max_len argument is not used by any callers, so not needed.
Then we have the spi_max module parameter. It was added because staging/fbtft has support for it and there was a report that someone used it to set a small buffer size to avoid popping on a USB soundcard on a Raspberry Pi. In hindsight it shouldn't have been added, I should have waited for it to become a problem first. I don't know it anyone is actually using it, but since tinydrm_spi_transfer() is being moved to mipi-dbi, I'm taking the opportunity to remove it. I'll add it back to mipi-dbi if someone complains.
With that out of the way, spi_max_transfer_size() can be used instead.
The chosen 16kB buffer size for Type C Option 1 (9-bit) interface is somewhat arbitrary, but a bigger buffer will have a miniscule impact on transfer speed, so it's probably fine.
This breaks the SPI PXA2xx case I'm using. The world is not a Pi:e.
[ 388.445752] mi0283qt spi-PRP0001:01: DMA disabled for transfer length 153600 greater than 65536 [ 388.634437] mi0283qt spi-PRP0001:01: DMA disabled for transfer length 153600 greater than 65536 [ 388.822933] mi0283qt spi-PRP0001:01: DMA disabled for transfer length 153600 greater than 65536
The crucial thing is to check the transfer size against maximum DMA length of the master.
Isn't this a spi controller driver problem? spi_max_transfer_size() tells the client what the maximum transfer length is. The controller driver can use ctlr->max_transfer_size if it has restrictions. AFAIUI max_dma_len is used when splitting up the buffer for the sg table in spi_map_buf().
Something like this, as a test patch.
max_transfer_size should be a function. In that case it works.
Why do you want to make it a function? At least from my reading of the code, the dma vs pio decision seems to be done once. So no need to change this at runtime. Changing at runtime would also be a pretty big surprise I think for users of spi.
However I'm not sure it's the best approach, thus, Cc to SPI PXA people.
Hm didn't spot the pxa people, added them. Mark, should I just go ahead and bake this into a proper patch for discussion? Or fundamentally wrong approach? -Daniel
diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c index bb6a14d1ab0f..f77201915033 100644 --- a/drivers/spi/spi-pxa2xx.c +++ b/drivers/spi/spi-pxa2xx.c @@ -1707,6 +1707,7 @@ static int pxa2xx_spi_probe(struct platform_device *pdev) } else { controller->can_dma = pxa2xx_spi_can_dma; controller->max_dma_len = MAX_DMA_LEN;
controller->max_transfer_size = MAX_DMA_LEN; } }-- With Best Regards, Andy Shevchenko
On Wed, Oct 16, 2019 at 07:44:51PM +0200, Daniel Vetter wrote:
On Wed, Oct 16, 2019 at 6:13 PM Andy Shevchenko
On Tue, Oct 15, 2019 at 05:57:20PM +0200, Daniel Vetter wrote:
Something like this, as a test patch.
max_transfer_size should be a function. In that case it works.
Why do you want to make it a function? At least from my reading of the code, the dma vs pio decision seems to be done once. So no need to change this at runtime. Changing at runtime would also be a pretty big surprise I think for users of spi.
Yeah, I'd expect it to be a fixed property of the hardware that doesn't vary at runtime though I'm sure there must be some innovation out there which challenges that assumption.
However I'm not sure it's the best approach, thus, Cc to SPI PXA people.
Hm didn't spot the pxa people, added them. Mark, should I just go ahead and bake this into a proper patch for discussion? Or fundamentally wrong approach?
That seems sensible enough, it should certainly fix the immediate issue.
On Wed, Oct 16, 2019 at 07:44:51PM +0200, Daniel Vetter wrote:
On Wed, Oct 16, 2019 at 6:13 PM Andy Shevchenko andriy.shevchenko@intel.com wrote:
On Tue, Oct 15, 2019 at 05:57:20PM +0200, Daniel Vetter wrote:
max_transfer_size should be a function. In that case it works.
Why do you want to make it a function?
Me?!
commit 4acad4aae10d1fa79a075b38b5c73772c44f576c Author: Michal Suchanek hramrach@gmail.com Date: Wed Dec 2 10:38:21 2015 +0000
spi: expose master transfer size limitation.
On Tue, Oct 15, 2019 at 05:41:53PM +0200, Noralf Trønnes wrote:
Den 15.10.2019 16.32, skrev Andy Shevchenko:
On Fri, Jul 19, 2019 at 05:59:10PM +0200, Noralf Trønnes wrote:
spi-bcm2835 can handle >64kB buffers now so there is no need to check ->max_dma_len. The tinydrm_spi_max_transfer_size() max_len argument is not used by any callers, so not needed.
Then we have the spi_max module parameter. It was added because staging/fbtft has support for it and there was a report that someone used it to set a small buffer size to avoid popping on a USB soundcard on a Raspberry Pi. In hindsight it shouldn't have been added, I should have waited for it to become a problem first. I don't know it anyone is actually using it, but since tinydrm_spi_transfer() is being moved to mipi-dbi, I'm taking the opportunity to remove it. I'll add it back to mipi-dbi if someone complains.
With that out of the way, spi_max_transfer_size() can be used instead.
The chosen 16kB buffer size for Type C Option 1 (9-bit) interface is somewhat arbitrary, but a bigger buffer will have a miniscule impact on transfer speed, so it's probably fine.
This breaks the SPI PXA2xx case I'm using. The world is not a Pi:e.
[ 388.445752] mi0283qt spi-PRP0001:01: DMA disabled for transfer length 153600 greater than 65536 [ 388.634437] mi0283qt spi-PRP0001:01: DMA disabled for transfer length 153600 greater than 65536 [ 388.822933] mi0283qt spi-PRP0001:01: DMA disabled for transfer length 153600 greater than 65536
The crucial thing is to check the transfer size against maximum DMA length of the master.
Isn't this a spi controller driver problem?
It doesn't matter. This patch made a regression. Before it worked, now it doesn't.
spi_max_transfer_size() tells the client what the maximum transfer length is. The controller driver can use ctlr->max_transfer_size if it has restrictions.
It might be a problem of the SPI core.
AFAIUI max_dma_len is used when splitting up the buffer for the sg table in spi_map_buf().
Please, fix.
Should I send the revert?
On Tue, Oct 15, 2019 at 5:59 PM Andy Shevchenko andriy.shevchenko@intel.com wrote:
On Tue, Oct 15, 2019 at 05:41:53PM +0200, Noralf Trønnes wrote:
Den 15.10.2019 16.32, skrev Andy Shevchenko:
On Fri, Jul 19, 2019 at 05:59:10PM +0200, Noralf Trønnes wrote:
spi-bcm2835 can handle >64kB buffers now so there is no need to check ->max_dma_len. The tinydrm_spi_max_transfer_size() max_len argument is not used by any callers, so not needed.
Then we have the spi_max module parameter. It was added because staging/fbtft has support for it and there was a report that someone used it to set a small buffer size to avoid popping on a USB soundcard on a Raspberry Pi. In hindsight it shouldn't have been added, I should have waited for it to become a problem first. I don't know it anyone is actually using it, but since tinydrm_spi_transfer() is being moved to mipi-dbi, I'm taking the opportunity to remove it. I'll add it back to mipi-dbi if someone complains.
With that out of the way, spi_max_transfer_size() can be used instead.
The chosen 16kB buffer size for Type C Option 1 (9-bit) interface is somewhat arbitrary, but a bigger buffer will have a miniscule impact on transfer speed, so it's probably fine.
This breaks the SPI PXA2xx case I'm using. The world is not a Pi:e.
[ 388.445752] mi0283qt spi-PRP0001:01: DMA disabled for transfer length 153600 greater than 65536 [ 388.634437] mi0283qt spi-PRP0001:01: DMA disabled for transfer length 153600 greater than 65536 [ 388.822933] mi0283qt spi-PRP0001:01: DMA disabled for transfer length 153600 greater than 65536
The crucial thing is to check the transfer size against maximum DMA length of the master.
Isn't this a spi controller driver problem?
It doesn't matter. This patch made a regression. Before it worked, now it doesn't.
Yes this is clear.
spi_max_transfer_size() tells the client what the maximum transfer length is. The controller driver can use ctlr->max_transfer_size if it has restrictions.
It might be a problem of the SPI core.
AFAIUI max_dma_len is used when splitting up the buffer for the sg table in spi_map_buf().
Please, fix.
Should I send the revert?
Can we please not roll in with the cavalry before everyone had a chance to wake up and look at this properly? It's less than 1h since your initial bug report.
Also, you _do_ have a test patch in your inbox ...
Seriously. -Daniel
On Tue, Oct 15, 2019 at 06:05:16PM +0200, Daniel Vetter wrote:
On Tue, Oct 15, 2019 at 5:59 PM Andy Shevchenko andriy.shevchenko@intel.com wrote:
On Tue, Oct 15, 2019 at 05:41:53PM +0200, Noralf Trønnes wrote:
Den 15.10.2019 16.32, skrev Andy Shevchenko:
On Fri, Jul 19, 2019 at 05:59:10PM +0200, Noralf Trønnes wrote:
spi-bcm2835 can handle >64kB buffers now so there is no need to check ->max_dma_len. The tinydrm_spi_max_transfer_size() max_len argument is not used by any callers, so not needed.
Then we have the spi_max module parameter. It was added because staging/fbtft has support for it and there was a report that someone used it to set a small buffer size to avoid popping on a USB soundcard on a Raspberry Pi. In hindsight it shouldn't have been added, I should have waited for it to become a problem first. I don't know it anyone is actually using it, but since tinydrm_spi_transfer() is being moved to mipi-dbi, I'm taking the opportunity to remove it. I'll add it back to mipi-dbi if someone complains.
With that out of the way, spi_max_transfer_size() can be used instead.
The chosen 16kB buffer size for Type C Option 1 (9-bit) interface is somewhat arbitrary, but a bigger buffer will have a miniscule impact on transfer speed, so it's probably fine.
This breaks the SPI PXA2xx case I'm using. The world is not a Pi:e.
[ 388.445752] mi0283qt spi-PRP0001:01: DMA disabled for transfer length 153600 greater than 65536 [ 388.634437] mi0283qt spi-PRP0001:01: DMA disabled for transfer length 153600 greater than 65536 [ 388.822933] mi0283qt spi-PRP0001:01: DMA disabled for transfer length 153600 greater than 65536
The crucial thing is to check the transfer size against maximum DMA length of the master.
Isn't this a spi controller driver problem?
It doesn't matter. This patch made a regression. Before it worked, now it doesn't.
Yes this is clear.
spi_max_transfer_size() tells the client what the maximum transfer length is. The controller driver can use ctlr->max_transfer_size if it has restrictions.
It might be a problem of the SPI core.
AFAIUI max_dma_len is used when splitting up the buffer for the sg table in spi_map_buf().
Please, fix.
Should I send the revert?
Can we please not roll in with the cavalry before everyone had a chance to wake up and look at this properly? It's less than 1h since your initial bug report.
It was a simple question to the 1st level maintainer of the subsystem, who already read the report and answered to it.
Also, you _do_ have a test patch in your inbox ...
Yes, thanks for it, I'm going to test right now.
Den 15.10.2019 17.59, skrev Andy Shevchenko:
On Tue, Oct 15, 2019 at 05:41:53PM +0200, Noralf Trønnes wrote:
Den 15.10.2019 16.32, skrev Andy Shevchenko:
On Fri, Jul 19, 2019 at 05:59:10PM +0200, Noralf Trønnes wrote:
spi-bcm2835 can handle >64kB buffers now so there is no need to check ->max_dma_len. The tinydrm_spi_max_transfer_size() max_len argument is not used by any callers, so not needed.
Then we have the spi_max module parameter. It was added because staging/fbtft has support for it and there was a report that someone used it to set a small buffer size to avoid popping on a USB soundcard on a Raspberry Pi. In hindsight it shouldn't have been added, I should have waited for it to become a problem first. I don't know it anyone is actually using it, but since tinydrm_spi_transfer() is being moved to mipi-dbi, I'm taking the opportunity to remove it. I'll add it back to mipi-dbi if someone complains.
With that out of the way, spi_max_transfer_size() can be used instead.
The chosen 16kB buffer size for Type C Option 1 (9-bit) interface is somewhat arbitrary, but a bigger buffer will have a miniscule impact on transfer speed, so it's probably fine.
This breaks the SPI PXA2xx case I'm using. The world is not a Pi:e.
[ 388.445752] mi0283qt spi-PRP0001:01: DMA disabled for transfer length 153600 greater than 65536 [ 388.634437] mi0283qt spi-PRP0001:01: DMA disabled for transfer length 153600 greater than 65536 [ 388.822933] mi0283qt spi-PRP0001:01: DMA disabled for transfer length 153600 greater than 65536
The crucial thing is to check the transfer size against maximum DMA length of the master.
Isn't this a spi controller driver problem?
It doesn't matter. This patch made a regression. Before it worked, now it doesn't.
spi_max_transfer_size() tells the client what the maximum transfer length is. The controller driver can use ctlr->max_transfer_size if it has restrictions.
It might be a problem of the SPI core.
AFAIUI max_dma_len is used when splitting up the buffer for the sg table in spi_map_buf().
Please, fix.
Should I send the revert?
Please do.
I still believe that this papers over a controller driver bug/shortcoming, but there could be more drivers that have the same problem so it's better to revert to the old behaviour. I guess the problem is that not many controller drivers are tested with buffers bigger than max_dma_len (64kB in this case, same as for the Pi).
If I have the revert by tomorrow, then I can apply it before Maxime sends the -rc PR to Dave on Thursday.
Noralf.
Prep work before moving the function to mipi-dbi.
tinydrm_spi_transfer() was made to support one class of drivers in drivers/staging/fbtft that has not been converted to DRM yet, so strip away the unused functionality: - Start byte (header) is not used. - No driver relies on the automatic 16-bit byte swapping on little endian machines with SPI controllers only supporting 8 bits per word.
Other changes: - No need to initialize ret - No need for the WARN since mipi-dbi only uses 8 and 16 bpw. - Use spi_message_init_with_transfers()
Cc: David Lechner david@lechnology.com Acked-by: : David Lechner david@lechnology.com Signed-off-by: Noralf Trønnes noralf@tronnes.org --- .../gpu/drm/tinydrm/core/tinydrm-helpers.c | 40 ++----------------- drivers/gpu/drm/tinydrm/ili9225.c | 4 +- drivers/gpu/drm/tinydrm/mipi-dbi.c | 4 +- include/drm/tinydrm/tinydrm-helpers.h | 3 +- 4 files changed, 9 insertions(+), 42 deletions(-)
diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c index af5bec8861de..d95eb50fa9d4 100644 --- a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c +++ b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c @@ -24,23 +24,18 @@ * tinydrm_spi_transfer - SPI transfer helper * @spi: SPI device * @speed_hz: Override speed (optional) - * @header: Optional header transfer * @bpw: Bits per word * @buf: Buffer to transfer * @len: Buffer length * * This SPI transfer helper breaks up the transfer of @buf into chunks which - * the SPI master driver can handle. If the machine is Little Endian and the - * SPI master driver doesn't support 16 bits per word, it swaps the bytes and - * does a 8-bit transfer. - * If @header is set, it is prepended to each SPI message. + * the SPI controller driver can handle. * * Returns: * Zero on success, negative error code on failure. */ int tinydrm_spi_transfer(struct spi_device *spi, u32 speed_hz, - struct spi_transfer *header, u8 bpw, const void *buf, - size_t len) + u8 bpw, const void *buf, size_t len) { size_t max_chunk = spi_max_transfer_size(spi); struct spi_transfer tr = { @@ -48,43 +43,16 @@ int tinydrm_spi_transfer(struct spi_device *spi, u32 speed_hz, .speed_hz = speed_hz, }; struct spi_message m; - u16 *swap_buf = NULL; size_t chunk; - int ret = 0; + int ret;
- if (WARN_ON_ONCE(bpw != 8 && bpw != 16)) - return -EINVAL; - - if (bpw == 16 && !spi_is_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; - } - } - - spi_message_init(&m); - if (header) - spi_message_add_tail(header, &m); - spi_message_add_tail(&tr, &m); + spi_message_init_with_transfers(&m, &tr, 1);
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;
diff --git a/drivers/gpu/drm/tinydrm/ili9225.c b/drivers/gpu/drm/tinydrm/ili9225.c index 7a8e1b4a37ee..21677e3ed38b 100644 --- a/drivers/gpu/drm/tinydrm/ili9225.c +++ b/drivers/gpu/drm/tinydrm/ili9225.c @@ -323,7 +323,7 @@ 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, 8, cmd, 1); if (ret || !num) return ret;
@@ -333,7 +333,7 @@ static int ili9225_dbi_command(struct mipi_dbi *mipi, u8 *cmd, u8 *par, gpiod_set_value_cansleep(mipi->dc, 1); speed_hz = mipi_dbi_spi_cmd_max_speed(spi, num);
- return tinydrm_spi_transfer(spi, speed_hz, NULL, bpw, par, num); + return tinydrm_spi_transfer(spi, speed_hz, bpw, par, num); }
static const struct drm_simple_display_pipe_funcs ili9225_pipe_funcs = { diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c index ae31a5c9aa1b..8fb6ce4ca6fc 100644 --- a/drivers/gpu/drm/tinydrm/mipi-dbi.c +++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c @@ -926,7 +926,7 @@ static int mipi_dbi_typec3_command(struct mipi_dbi *mipi, u8 *cmd,
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, 8, cmd, 1); if (ret || !num) return ret;
@@ -936,7 +936,7 @@ static int mipi_dbi_typec3_command(struct mipi_dbi *mipi, u8 *cmd, gpiod_set_value_cansleep(mipi->dc, 1); speed_hz = mipi_dbi_spi_cmd_max_speed(spi, num);
- return tinydrm_spi_transfer(spi, speed_hz, NULL, bpw, par, num); + return tinydrm_spi_transfer(spi, speed_hz, bpw, par, num); }
/** diff --git a/include/drm/tinydrm/tinydrm-helpers.h b/include/drm/tinydrm/tinydrm-helpers.h index 10b35375a009..708c5a7d51e0 100644 --- a/include/drm/tinydrm/tinydrm-helpers.h +++ b/include/drm/tinydrm/tinydrm-helpers.h @@ -42,7 +42,6 @@ int tinydrm_display_pipe_init(struct drm_device *drm, unsigned int rotation);
int tinydrm_spi_transfer(struct spi_device *spi, u32 speed_hz, - struct spi_transfer *header, u8 bpw, const void *buf, - size_t len); + u8 bpw, const void *buf, size_t len);
#endif /* __LINUX_TINYDRM_HELPERS_H */
This is only used by mipi-dbi drivers so move it there.
The reason this isn't moved to the SPI subsystem is that it will in a later patch pass a dummy rx buffer for SPI controllers that need this. Low memory boards (64MB) can run into a problem allocating such a "large" contiguous buffer on every transfer after a long up time. This leaves a very specific use case, so we'll keep the function here. mipi-dbi will first go through a refactoring though, before this will be done.
Remove SPI todo entry now that we're done with the tinydrm.ko SPI code.
v2: Drop moving the mipi_dbi_spi_init() declaration (Sam)
Cc: David Lechner david@lechnology.com Reviewed-by: Sam Ravnborg sam@ravnborg.org Acked-by: : David Lechner david@lechnology.com Signed-off-by: Noralf Trønnes noralf@tronnes.org --- Documentation/gpu/tinydrm.rst | 3 - Documentation/gpu/todo.rst | 3 - drivers/gpu/drm/tinydrm/core/Makefile | 2 +- .../gpu/drm/tinydrm/core/tinydrm-helpers.c | 70 ------------------- drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c | 4 ++ drivers/gpu/drm/tinydrm/ili9225.c | 5 +- drivers/gpu/drm/tinydrm/mipi-dbi.c | 49 ++++++++++++- include/drm/tinydrm/mipi-dbi.h | 3 + include/drm/tinydrm/tinydrm-helpers.h | 5 -- 9 files changed, 57 insertions(+), 87 deletions(-) delete mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
diff --git a/Documentation/gpu/tinydrm.rst b/Documentation/gpu/tinydrm.rst index 33a41544f659..2c2860fa1510 100644 --- a/Documentation/gpu/tinydrm.rst +++ b/Documentation/gpu/tinydrm.rst @@ -11,9 +11,6 @@ Helpers .. kernel-doc:: include/drm/tinydrm/tinydrm-helpers.h :internal:
-.. kernel-doc:: drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c - :export: - .. kernel-doc:: drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c :export:
diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst index a38639e96e8b..688b4adbbf62 100644 --- a/Documentation/gpu/todo.rst +++ b/Documentation/gpu/todo.rst @@ -443,9 +443,6 @@ tinydrm Tinydrm is the helper driver for really simple fb drivers. The goal is to make those drivers as simple as possible, so lots of room for refactoring:
-- spi helpers, probably best put into spi core/helper code. Thierry said - the spi maintainer is fast&reactive, so shouldn't be a big issue. - - extract the mipi-dbi helper (well, the non-tinydrm specific parts at least) into a separate helper, like we have for mipi-dsi already. Or follow one of the ideas for having a shared dsi/dbi helper, abstracting away the diff --git a/drivers/gpu/drm/tinydrm/core/Makefile b/drivers/gpu/drm/tinydrm/core/Makefile index 01065e920aea..78e179127e55 100644 --- a/drivers/gpu/drm/tinydrm/core/Makefile +++ b/drivers/gpu/drm/tinydrm/core/Makefile @@ -1,4 +1,4 @@ # SPDX-License-Identifier: GPL-2.0-only -tinydrm-y := tinydrm-pipe.o tinydrm-helpers.o +tinydrm-y := tinydrm-pipe.o
obj-$(CONFIG_DRM_TINYDRM) += tinydrm.o diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c deleted file mode 100644 index d95eb50fa9d4..000000000000 --- a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c +++ /dev/null @@ -1,70 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0-or-later -/* - * Copyright (C) 2016 Noralf Trønnes - */ - -#include <linux/backlight.h> -#include <linux/dma-buf.h> -#include <linux/module.h> -#include <linux/pm.h> -#include <linux/spi/spi.h> -#include <linux/swab.h> - -#include <drm/drm_device.h> -#include <drm/drm_drv.h> -#include <drm/drm_fourcc.h> -#include <drm/drm_framebuffer.h> -#include <drm/drm_print.h> -#include <drm/drm_rect.h> -#include <drm/tinydrm/tinydrm-helpers.h> - -#if IS_ENABLED(CONFIG_SPI) - -/** - * tinydrm_spi_transfer - SPI transfer helper - * @spi: SPI device - * @speed_hz: Override speed (optional) - * @bpw: Bits per word - * @buf: Buffer to transfer - * @len: Buffer length - * - * This SPI transfer helper breaks up the transfer of @buf into chunks which - * the SPI controller driver can handle. - * - * Returns: - * Zero on success, negative error code on failure. - */ -int tinydrm_spi_transfer(struct spi_device *spi, u32 speed_hz, - u8 bpw, const void *buf, size_t len) -{ - size_t max_chunk = spi_max_transfer_size(spi); - struct spi_transfer tr = { - .bits_per_word = bpw, - .speed_hz = speed_hz, - }; - struct spi_message m; - size_t chunk; - int ret; - - spi_message_init_with_transfers(&m, &tr, 1); - - while (len) { - chunk = min(len, max_chunk); - - tr.tx_buf = buf; - tr.len = chunk; - buf += chunk; - len -= chunk; - - ret = spi_sync(spi, &m); - if (ret) - return ret; - } - - return 0; -} -EXPORT_SYMBOL(tinydrm_spi_transfer); - -#endif /* CONFIG_SPI */ - -MODULE_LICENSE("GPL"); diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c b/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c index ed798fd95152..a62d1dfe87f8 100644 --- a/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c +++ b/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c @@ -3,6 +3,8 @@ * Copyright (C) 2016 Noralf Trønnes */
+#include <linux/module.h> + #include <drm/drm_atomic_helper.h> #include <drm/drm_drv.h> #include <drm/drm_gem_framebuffer_helper.h> @@ -177,3 +179,5 @@ int tinydrm_display_pipe_init(struct drm_device *drm, format_count, modifiers, connector); } EXPORT_SYMBOL(tinydrm_display_pipe_init); + +MODULE_LICENSE("GPL"); diff --git a/drivers/gpu/drm/tinydrm/ili9225.c b/drivers/gpu/drm/tinydrm/ili9225.c index 21677e3ed38b..62f29b2faf74 100644 --- a/drivers/gpu/drm/tinydrm/ili9225.c +++ b/drivers/gpu/drm/tinydrm/ili9225.c @@ -27,7 +27,6 @@ #include <drm/drm_rect.h> #include <drm/drm_vblank.h> #include <drm/tinydrm/mipi-dbi.h> -#include <drm/tinydrm/tinydrm-helpers.h>
#define ILI9225_DRIVER_READ_CODE 0x00 #define ILI9225_DRIVER_OUTPUT_CONTROL 0x01 @@ -323,7 +322,7 @@ 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, 8, cmd, 1); + ret = mipi_dbi_spi_transfer(spi, speed_hz, 8, cmd, 1); if (ret || !num) return ret;
@@ -333,7 +332,7 @@ static int ili9225_dbi_command(struct mipi_dbi *mipi, u8 *cmd, u8 *par, gpiod_set_value_cansleep(mipi->dc, 1); speed_hz = mipi_dbi_spi_cmd_max_speed(spi, num);
- return tinydrm_spi_transfer(spi, speed_hz, bpw, par, num); + return mipi_dbi_spi_transfer(spi, speed_hz, bpw, par, num); }
static const struct drm_simple_display_pipe_funcs ili9225_pipe_funcs = { diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c index 8fb6ce4ca6fc..6a8f2d66377f 100644 --- a/drivers/gpu/drm/tinydrm/mipi-dbi.c +++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c @@ -926,7 +926,7 @@ static int mipi_dbi_typec3_command(struct mipi_dbi *mipi, u8 *cmd,
gpiod_set_value_cansleep(mipi->dc, 0); speed_hz = mipi_dbi_spi_cmd_max_speed(spi, 1); - ret = tinydrm_spi_transfer(spi, speed_hz, 8, cmd, 1); + ret = mipi_dbi_spi_transfer(spi, speed_hz, 8, cmd, 1); if (ret || !num) return ret;
@@ -936,7 +936,7 @@ static int mipi_dbi_typec3_command(struct mipi_dbi *mipi, u8 *cmd, gpiod_set_value_cansleep(mipi->dc, 1); speed_hz = mipi_dbi_spi_cmd_max_speed(spi, num);
- return tinydrm_spi_transfer(spi, speed_hz, bpw, par, num); + return mipi_dbi_spi_transfer(spi, speed_hz, bpw, par, num); }
/** @@ -1007,6 +1007,51 @@ int mipi_dbi_spi_init(struct spi_device *spi, struct mipi_dbi *mipi, } EXPORT_SYMBOL(mipi_dbi_spi_init);
+/** + * mipi_dbi_spi_transfer - SPI transfer helper + * @spi: SPI device + * @speed_hz: Override speed (optional) + * @bpw: Bits per word + * @buf: Buffer to transfer + * @len: Buffer length + * + * This SPI transfer helper breaks up the transfer of @buf into chunks which + * the SPI controller driver can handle. + * + * Returns: + * Zero on success, negative error code on failure. + */ +int mipi_dbi_spi_transfer(struct spi_device *spi, u32 speed_hz, + u8 bpw, const void *buf, size_t len) +{ + size_t max_chunk = spi_max_transfer_size(spi); + struct spi_transfer tr = { + .bits_per_word = bpw, + .speed_hz = speed_hz, + }; + struct spi_message m; + size_t chunk; + int ret; + + spi_message_init_with_transfers(&m, &tr, 1); + + while (len) { + chunk = min(len, max_chunk); + + tr.tx_buf = buf; + tr.len = chunk; + buf += chunk; + len -= chunk; + + ret = spi_sync(spi, &m); + if (ret) + return ret; + } + + return 0; +} +EXPORT_SYMBOL(mipi_dbi_spi_transfer); + #endif /* CONFIG_SPI */
#ifdef CONFIG_DEBUG_FS diff --git a/include/drm/tinydrm/mipi-dbi.h b/include/drm/tinydrm/mipi-dbi.h index 51fc667beef7..35a1413f2122 100644 --- a/include/drm/tinydrm/mipi-dbi.h +++ b/include/drm/tinydrm/mipi-dbi.h @@ -83,7 +83,10 @@ void mipi_dbi_hw_reset(struct mipi_dbi *mipi); bool mipi_dbi_display_is_on(struct mipi_dbi *mipi); int mipi_dbi_poweron_reset(struct mipi_dbi *mipi); int mipi_dbi_poweron_conditional_reset(struct mipi_dbi *mipi); + u32 mipi_dbi_spi_cmd_max_speed(struct spi_device *spi, size_t len); +int mipi_dbi_spi_transfer(struct spi_device *spi, u32 speed_hz, + u8 bpw, const void *buf, 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); diff --git a/include/drm/tinydrm/tinydrm-helpers.h b/include/drm/tinydrm/tinydrm-helpers.h index 708c5a7d51e0..8c5d20efeaa1 100644 --- a/include/drm/tinydrm/tinydrm-helpers.h +++ b/include/drm/tinydrm/tinydrm-helpers.h @@ -13,8 +13,6 @@ struct drm_framebuffer; struct drm_rect; struct drm_simple_display_pipe; struct drm_simple_display_pipe_funcs; -struct spi_transfer; -struct spi_device; struct device;
/** @@ -41,7 +39,4 @@ int tinydrm_display_pipe_init(struct drm_device *drm, const struct drm_display_mode *mode, unsigned int rotation);
-int tinydrm_spi_transfer(struct spi_device *spi, u32 speed_hz, - u8 bpw, const void *buf, size_t len); - #endif /* __LINUX_TINYDRM_HELPERS_H */
The tinydrm helper is going away so move it into the only user mipi-dbi.
Reviewed-by: Sam Ravnborg sam@ravnborg.org Signed-off-by: Noralf Trønnes noralf@tronnes.org --- drivers/gpu/drm/tinydrm/mipi-dbi.c | 15 ++++++++++++--- include/drm/tinydrm/tinydrm-helpers.h | 15 --------------- 2 files changed, 12 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c index 6a8f2d66377f..73db287e5c52 100644 --- a/drivers/gpu/drm/tinydrm/mipi-dbi.c +++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c @@ -628,6 +628,15 @@ u32 mipi_dbi_spi_cmd_max_speed(struct spi_device *spi, size_t len) } EXPORT_SYMBOL(mipi_dbi_spi_cmd_max_speed);
+static bool mipi_dbi_machine_little_endian(void) +{ +#if defined(__LITTLE_ENDIAN) + return true; +#else + return false; +#endif +} + /* * MIPI DBI Type C Option 1 * @@ -650,7 +659,7 @@ static int mipi_dbi_spi1e_transfer(struct mipi_dbi *mipi, int dc, const void *buf, size_t len, unsigned int bpw) { - bool swap_bytes = (bpw == 16 && tinydrm_machine_little_endian()); + bool swap_bytes = (bpw == 16 && mipi_dbi_machine_little_endian()); size_t chunk, max_chunk = mipi->tx_buf9_len; struct spi_device *spi = mipi->spi; struct spi_transfer tr = { @@ -799,7 +808,7 @@ static int mipi_dbi_spi1_transfer(struct mipi_dbi *mipi, int dc, size_t chunk = min(len, max_chunk); unsigned int i;
- if (bpw == 16 && tinydrm_machine_little_endian()) { + if (bpw == 16 && mipi_dbi_machine_little_endian()) { for (i = 0; i < (chunk * 2); i += 2) { dst16[i] = *src16 >> 8; dst16[i + 1] = *src16++ & 0xFF; @@ -991,7 +1000,7 @@ int mipi_dbi_spi_init(struct spi_device *spi, struct mipi_dbi *mipi, if (dc) { mipi->command = mipi_dbi_typec3_command; mipi->dc = dc; - if (tinydrm_machine_little_endian() && !spi_is_bpw_supported(spi, 16)) + if (mipi_dbi_machine_little_endian() && !spi_is_bpw_supported(spi, 16)) mipi->swap_bytes = true; } else { mipi->command = mipi_dbi_typec1_command; diff --git a/include/drm/tinydrm/tinydrm-helpers.h b/include/drm/tinydrm/tinydrm-helpers.h index 8c5d20efeaa1..0e7470771c5e 100644 --- a/include/drm/tinydrm/tinydrm-helpers.h +++ b/include/drm/tinydrm/tinydrm-helpers.h @@ -15,21 +15,6 @@ struct drm_simple_display_pipe; struct drm_simple_display_pipe_funcs; struct device;
-/** - * tinydrm_machine_little_endian - Machine is little endian - * - * Returns: - * true if *defined(__LITTLE_ENDIAN)*, false otherwise - */ -static inline bool tinydrm_machine_little_endian(void) -{ -#if defined(__LITTLE_ENDIAN) - return true; -#else - return false; -#endif -} - int tinydrm_display_pipe_init(struct drm_device *drm, struct drm_simple_display_pipe *pipe, const struct drm_simple_display_pipe_funcs *funcs,
tinydrm.ko is going away so let's implement a connector.
Reviewed-by: Sam Ravnborg sam@ravnborg.org Signed-off-by: Noralf Trønnes noralf@tronnes.org --- drivers/gpu/drm/tinydrm/repaper.c | 58 ++++++++++++++++++++++++++++--- 1 file changed, 53 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/tinydrm/repaper.c b/drivers/gpu/drm/tinydrm/repaper.c index 40afa66107e5..76d179200775 100644 --- a/drivers/gpu/drm/tinydrm/repaper.c +++ b/drivers/gpu/drm/tinydrm/repaper.c @@ -23,6 +23,7 @@ #include <linux/thermal.h>
#include <drm/drm_atomic_helper.h> +#include <drm/drm_connector.h> #include <drm/drm_damage_helper.h> #include <drm/drm_drv.h> #include <drm/drm_fb_cma_helper.h> @@ -30,10 +31,11 @@ #include <drm/drm_format_helper.h> #include <drm/drm_gem_cma_helper.h> #include <drm/drm_gem_framebuffer_helper.h> +#include <drm/drm_modes.h> #include <drm/drm_rect.h> #include <drm/drm_vblank.h> +#include <drm/drm_probe_helper.h> #include <drm/drm_simple_kms_helper.h> -#include <drm/tinydrm/tinydrm-helpers.h>
#define REPAPER_RID_G2_COG_ID 0x12
@@ -60,6 +62,8 @@ enum repaper_epd_border_byte { struct repaper_epd { struct drm_device drm; struct drm_simple_display_pipe pipe; + const struct drm_display_mode *mode; + struct drm_connector connector; struct spi_device *spi;
struct gpio_desc *panel_on; @@ -873,6 +877,39 @@ static const struct drm_simple_display_pipe_funcs repaper_pipe_funcs = { .prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb, };
+static int repaper_connector_get_modes(struct drm_connector *connector) +{ + struct repaper_epd *epd = drm_to_epd(connector->dev); + struct drm_display_mode *mode; + + mode = drm_mode_duplicate(connector->dev, epd->mode); + if (!mode) { + DRM_ERROR("Failed to duplicate mode\n"); + return 0; + } + + drm_mode_set_name(mode); + mode->type |= DRM_MODE_TYPE_PREFERRED; + drm_mode_probed_add(connector, mode); + + connector->display_info.width_mm = mode->width_mm; + connector->display_info.height_mm = mode->height_mm; + + return 1; +} + +static const struct drm_connector_helper_funcs repaper_connector_hfuncs = { + .get_modes = repaper_connector_get_modes, +}; + +static const struct drm_connector_funcs repaper_connector_funcs = { + .reset = drm_atomic_helper_connector_reset, + .fill_modes = drm_helper_probe_single_connector_modes, + .destroy = drm_connector_cleanup, + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, +}; + static const struct drm_mode_config_funcs repaper_mode_config_funcs = { .fb_create = drm_gem_fb_create_with_dirty, .atomic_check = drm_atomic_helper_check, @@ -1095,6 +1132,7 @@ static int repaper_probe(struct spi_device *spi) return -ENODEV; }
+ epd->mode = mode; epd->width = mode->hdisplay; epd->height = mode->vdisplay; epd->factored_stage_time = epd->stage_time; @@ -1109,10 +1147,20 @@ static int repaper_probe(struct spi_device *spi) if (!epd->current_frame) return -ENOMEM;
- ret = tinydrm_display_pipe_init(drm, &epd->pipe, &repaper_pipe_funcs, - DRM_MODE_CONNECTOR_SPI, - repaper_formats, - ARRAY_SIZE(repaper_formats), mode, 0); + drm->mode_config.min_width = mode->hdisplay; + drm->mode_config.max_width = mode->hdisplay; + drm->mode_config.min_height = mode->vdisplay; + drm->mode_config.max_height = mode->vdisplay; + + drm_connector_helper_add(&epd->connector, &repaper_connector_hfuncs); + ret = drm_connector_init(drm, &epd->connector, &repaper_connector_funcs, + DRM_MODE_CONNECTOR_SPI); + if (ret) + return ret; + + ret = drm_simple_display_pipe_init(drm, &epd->pipe, &repaper_pipe_funcs, + repaper_formats, ARRAY_SIZE(repaper_formats), + NULL, &epd->connector); if (ret) return ret;
The MIPI DBI standard support more pixel formats than what this helper supports. Add an init function that lets the driver use different format(s). This avoids open coding mipi_dbi_init() in st7586.
st7586 sets preferred_depth but this is not necessary since it only supports one format.
v2: Forgot to remove the mipi->rotation assignment in st7586, mipi_dbi_init_with_formats() handles it.
Cc: David Lechner david@lechnology.com Acked-by: David Lechner david@lechnology.com Signed-off-by: Noralf Trønnes noralf@tronnes.org --- drivers/gpu/drm/tinydrm/mipi-dbi.c | 91 +++++++++++++++++++++--------- drivers/gpu/drm/tinydrm/st7586.c | 33 ++--------- include/drm/tinydrm/mipi-dbi.h | 5 ++ 3 files changed, 74 insertions(+), 55 deletions(-)
diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c index 73db287e5c52..a264c0bb74b0 100644 --- a/drivers/gpu/drm/tinydrm/mipi-dbi.c +++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c @@ -411,6 +411,65 @@ static const uint32_t mipi_dbi_formats[] = { DRM_FORMAT_XRGB8888, };
+/** + * mipi_dbi_init_with_formats - MIPI DBI initialization with custom formats + * @mipi: &mipi_dbi structure to initialize + * @funcs: Display pipe functions + * @formats: Array of supported formats (DRM_FORMAT_*). + * @format_count: Number of elements in @formats + * @mode: Display mode + * @rotation: Initial rotation in degrees Counter Clock Wise + * @tx_buf_size: Allocate a transmit buffer of this size. + * + * This function sets up a &drm_simple_display_pipe with a &drm_connector that + * has one fixed &drm_display_mode which is rotated according to @rotation. + * This mode is used to set the mode config min/max width/height properties. + * + * Use mipi_dbi_init() if you don't need custom formats. + * + * Note: + * Some of the helper functions expects RGB565 to be the default format and the + * transmit buffer sized to fit that. + * + * Returns: + * Zero on success, negative error code on failure. + */ +int mipi_dbi_init_with_formats(struct mipi_dbi *mipi, + const struct drm_simple_display_pipe_funcs *funcs, + const uint32_t *formats, unsigned int format_count, + const struct drm_display_mode *mode, + unsigned int rotation, size_t tx_buf_size) +{ + struct drm_device *drm = &mipi->drm; + int ret; + + if (!mipi->command) + return -EINVAL; + + mutex_init(&mipi->cmdlock); + + mipi->tx_buf = devm_kmalloc(drm->dev, tx_buf_size, GFP_KERNEL); + if (!mipi->tx_buf) + return -ENOMEM; + + ret = tinydrm_display_pipe_init(drm, &mipi->pipe, funcs, + DRM_MODE_CONNECTOR_SPI, + formats, format_count, mode, + rotation); + if (ret) + return ret; + + drm_plane_enable_fb_damage_clips(&mipi->pipe.plane); + + drm->mode_config.funcs = &mipi_dbi_mode_config_funcs; + mipi->rotation = rotation; + + DRM_DEBUG_KMS("rotation = %u\n", rotation); + + return 0; +} +EXPORT_SYMBOL(mipi_dbi_init_with_formats); + /** * mipi_dbi_init - MIPI DBI initialization * @mipi: &mipi_dbi structure to initialize @@ -433,36 +492,12 @@ int mipi_dbi_init(struct mipi_dbi *mipi, const struct drm_display_mode *mode, unsigned int rotation) { size_t bufsize = mode->vdisplay * mode->hdisplay * sizeof(u16); - struct drm_device *drm = &mipi->drm; - int ret;
- if (!mipi->command) - return -EINVAL; + mipi->drm.mode_config.preferred_depth = 16;
- mutex_init(&mipi->cmdlock); - - mipi->tx_buf = devm_kmalloc(drm->dev, bufsize, GFP_KERNEL); - if (!mipi->tx_buf) - return -ENOMEM; - - ret = tinydrm_display_pipe_init(drm, &mipi->pipe, funcs, - DRM_MODE_CONNECTOR_SPI, - mipi_dbi_formats, - ARRAY_SIZE(mipi_dbi_formats), mode, - rotation); - if (ret) - return ret; - - drm_plane_enable_fb_damage_clips(&mipi->pipe.plane); - - drm->mode_config.funcs = &mipi_dbi_mode_config_funcs; - drm->mode_config.preferred_depth = 16; - mipi->rotation = rotation; - - DRM_DEBUG_KMS("preferred_depth=%u, rotation = %u\n", - drm->mode_config.preferred_depth, rotation); - - return 0; + return mipi_dbi_init_with_formats(mipi, funcs, mipi_dbi_formats, + ARRAY_SIZE(mipi_dbi_formats), mode, + rotation, bufsize); } EXPORT_SYMBOL(mipi_dbi_init);
diff --git a/drivers/gpu/drm/tinydrm/st7586.c b/drivers/gpu/drm/tinydrm/st7586.c index 7ae39004aa88..4046b0fc3f7a 100644 --- a/drivers/gpu/drm/tinydrm/st7586.c +++ b/drivers/gpu/drm/tinydrm/st7586.c @@ -24,7 +24,6 @@ #include <drm/drm_rect.h> #include <drm/drm_vblank.h> #include <drm/tinydrm/mipi-dbi.h> -#include <drm/tinydrm/tinydrm-helpers.h>
/* controller-specific commands */ #define ST7586_DISP_MODE_GRAY 0x38 @@ -283,12 +282,6 @@ static const struct drm_simple_display_pipe_funcs st7586_pipe_funcs = { .prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb, };
-static const struct drm_mode_config_funcs st7586_mode_config_funcs = { - .fb_create = drm_gem_fb_create_with_dirty, - .atomic_check = drm_atomic_helper_check, - .atomic_commit = drm_atomic_helper_commit, -}; - static const struct drm_display_mode st7586_mode = { DRM_SIMPLE_MODE(178, 128, 37, 27), }; @@ -342,15 +335,8 @@ static int st7586_probe(struct spi_device *spi) }
drm_mode_config_init(drm); - drm->mode_config.preferred_depth = 32; - drm->mode_config.funcs = &st7586_mode_config_funcs; - - mutex_init(&mipi->cmdlock);
bufsize = (st7586_mode.vdisplay + 2) / 3 * st7586_mode.hdisplay; - mipi->tx_buf = devm_kmalloc(dev, bufsize, GFP_KERNEL); - if (!mipi->tx_buf) - return -ENOMEM;
mipi->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH); if (IS_ERR(mipi->reset)) { @@ -365,7 +351,6 @@ static int st7586_probe(struct spi_device *spi) }
device_property_read_u32(dev, "rotation", &rotation); - mipi->rotation = rotation;
ret = mipi_dbi_spi_init(spi, mipi, a0); if (ret) @@ -374,6 +359,12 @@ static int st7586_probe(struct spi_device *spi) /* Cannot read from this controller via SPI */ mipi->read_commands = NULL;
+ ret = mipi_dbi_init_with_formats(mipi, &st7586_pipe_funcs, + st7586_formats, ARRAY_SIZE(st7586_formats), + &st7586_mode, rotation, bufsize); + if (ret) + return ret; + /* * we are using 8-bit data, so we are not actually swapping anything, * but setting mipi->swap_bytes makes mipi_dbi_typec3_command() do the @@ -383,15 +374,6 @@ static int st7586_probe(struct spi_device *spi) */ mipi->swap_bytes = true;
- ret = tinydrm_display_pipe_init(drm, &mipi->pipe, &st7586_pipe_funcs, - DRM_MODE_CONNECTOR_SPI, - st7586_formats, ARRAY_SIZE(st7586_formats), - &st7586_mode, rotation); - if (ret) - return ret; - - drm_plane_enable_fb_damage_clips(&mipi->pipe.plane); - drm_mode_config_reset(drm);
ret = drm_dev_register(drm, 0); @@ -400,9 +382,6 @@ static int st7586_probe(struct spi_device *spi)
spi_set_drvdata(spi, drm);
- DRM_DEBUG_KMS("preferred_depth=%u, rotation = %u\n", - drm->mode_config.preferred_depth, rotation); - drm_fbdev_generic_setup(drm, 0);
return 0; diff --git a/include/drm/tinydrm/mipi-dbi.h b/include/drm/tinydrm/mipi-dbi.h index 35a1413f2122..c4b04789bf84 100644 --- a/include/drm/tinydrm/mipi-dbi.h +++ b/include/drm/tinydrm/mipi-dbi.h @@ -69,6 +69,11 @@ static inline struct mipi_dbi *drm_to_mipi_dbi(struct drm_device *drm)
int mipi_dbi_spi_init(struct spi_device *spi, struct mipi_dbi *mipi, struct gpio_desc *dc); +int mipi_dbi_init_with_formats(struct mipi_dbi *mipi, + const struct drm_simple_display_pipe_funcs *funcs, + const uint32_t *formats, unsigned int format_count, + const struct drm_display_mode *mode, + unsigned int rotation, size_t tx_buf_size); int mipi_dbi_init(struct mipi_dbi *mipi, const struct drm_simple_display_pipe_funcs *funcs, const struct drm_display_mode *mode, unsigned int rotation);
On 7/19/19 10:59 AM, Noralf Trønnes wrote:
The MIPI DBI standard support more pixel formats than what this helper supports. Add an init function that lets the driver use different format(s). This avoids open coding mipi_dbi_init() in st7586.
st7586 sets preferred_depth but this is not necessary since it only supports one format.
v2: Forgot to remove the mipi->rotation assignment in st7586, mipi_dbi_init_with_formats() handles it.
Cc: David Lechner david@lechnology.com Acked-by: David Lechner david@lechnology.com Signed-off-by: Noralf Trønnes noralf@tronnes.org
drivers/gpu/drm/tinydrm/mipi-dbi.c | 91 +++++++++++++++++++++--------- drivers/gpu/drm/tinydrm/st7586.c | 33 ++--------- include/drm/tinydrm/mipi-dbi.h | 5 ++ 3 files changed, 74 insertions(+), 55 deletions(-)
Tested whole series on st7586. Seems to be still working.
Just putting Tested-by here since this patch is the only one that changed st7586 significantly.
Tested-by: David Lechner david@lechnology.com
tinydrm_display_pipe_init() has only one user now, so move it to mipi-dbi.
Changes: - Remove drm_connector_helper_funcs.detect, it's always connected. - Store the connector and mode in mipi_dbi instead of it's own struct.
Otherwise remove some leftover tinydrm-helpers.h inclusions.
Cc: Eric Anholt eric@anholt.net Cc: David Lechner david@lechnology.com Reviewed-by: Sam Ravnborg sam@ravnborg.org Signed-off-by: Noralf Trønnes noralf@tronnes.org --- Documentation/gpu/tinydrm.rst | 9 - drivers/gpu/drm/tinydrm/Makefile | 1 - drivers/gpu/drm/tinydrm/core/Makefile | 4 - drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c | 183 -------------------- drivers/gpu/drm/tinydrm/hx8357d.c | 1 - drivers/gpu/drm/tinydrm/ili9341.c | 1 - drivers/gpu/drm/tinydrm/mi0283qt.c | 1 - drivers/gpu/drm/tinydrm/mipi-dbi.c | 87 +++++++++- drivers/gpu/drm/tinydrm/st7735r.c | 1 - include/drm/tinydrm/mipi-dbi.h | 10 ++ include/drm/tinydrm/tinydrm-helpers.h | 27 --- 11 files changed, 91 insertions(+), 234 deletions(-) delete mode 100644 drivers/gpu/drm/tinydrm/core/Makefile delete mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c delete mode 100644 include/drm/tinydrm/tinydrm-helpers.h
diff --git a/Documentation/gpu/tinydrm.rst b/Documentation/gpu/tinydrm.rst index 2c2860fa1510..64bdf6356024 100644 --- a/Documentation/gpu/tinydrm.rst +++ b/Documentation/gpu/tinydrm.rst @@ -5,15 +5,6 @@ drm/tinydrm Tiny DRM drivers tinydrm is a collection of DRM drivers that are so small they can fit in a single source file.
-Helpers -======= - -.. kernel-doc:: include/drm/tinydrm/tinydrm-helpers.h - :internal: - -.. kernel-doc:: drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c - :export: - MIPI DBI Compatible Controllers ===============================
diff --git a/drivers/gpu/drm/tinydrm/Makefile b/drivers/gpu/drm/tinydrm/Makefile index 48ec8ed9dc16..ab6b9bebf321 100644 --- a/drivers/gpu/drm/tinydrm/Makefile +++ b/drivers/gpu/drm/tinydrm/Makefile @@ -1,5 +1,4 @@ # SPDX-License-Identifier: GPL-2.0-only -obj-$(CONFIG_DRM_TINYDRM) += core/
# Controllers obj-$(CONFIG_TINYDRM_MIPI_DBI) += mipi-dbi.o diff --git a/drivers/gpu/drm/tinydrm/core/Makefile b/drivers/gpu/drm/tinydrm/core/Makefile deleted file mode 100644 index 78e179127e55..000000000000 --- a/drivers/gpu/drm/tinydrm/core/Makefile +++ /dev/null @@ -1,4 +0,0 @@ -# SPDX-License-Identifier: GPL-2.0-only -tinydrm-y := tinydrm-pipe.o - -obj-$(CONFIG_DRM_TINYDRM) += tinydrm.o diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c b/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c deleted file mode 100644 index a62d1dfe87f8..000000000000 --- a/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c +++ /dev/null @@ -1,183 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0-or-later -/* - * Copyright (C) 2016 Noralf Trønnes - */ - -#include <linux/module.h> - -#include <drm/drm_atomic_helper.h> -#include <drm/drm_drv.h> -#include <drm/drm_gem_framebuffer_helper.h> -#include <drm/drm_modes.h> -#include <drm/drm_probe_helper.h> -#include <drm/drm_print.h> -#include <drm/drm_simple_kms_helper.h> - -struct tinydrm_connector { - struct drm_connector base; - struct drm_display_mode mode; -}; - -static inline struct tinydrm_connector * -to_tinydrm_connector(struct drm_connector *connector) -{ - return container_of(connector, struct tinydrm_connector, base); -} - -static int tinydrm_connector_get_modes(struct drm_connector *connector) -{ - struct tinydrm_connector *tconn = to_tinydrm_connector(connector); - struct drm_display_mode *mode; - - mode = drm_mode_duplicate(connector->dev, &tconn->mode); - if (!mode) { - DRM_ERROR("Failed to duplicate mode\n"); - return 0; - } - - if (mode->name[0] == '\0') - drm_mode_set_name(mode); - - mode->type |= DRM_MODE_TYPE_PREFERRED; - drm_mode_probed_add(connector, mode); - - if (mode->width_mm) { - connector->display_info.width_mm = mode->width_mm; - connector->display_info.height_mm = mode->height_mm; - } - - return 1; -} - -static const struct drm_connector_helper_funcs tinydrm_connector_hfuncs = { - .get_modes = tinydrm_connector_get_modes, -}; - -static enum drm_connector_status -tinydrm_connector_detect(struct drm_connector *connector, bool force) -{ - if (drm_dev_is_unplugged(connector->dev)) - return connector_status_disconnected; - - return connector->status; -} - -static void tinydrm_connector_destroy(struct drm_connector *connector) -{ - struct tinydrm_connector *tconn = to_tinydrm_connector(connector); - - drm_connector_cleanup(connector); - kfree(tconn); -} - -static const struct drm_connector_funcs tinydrm_connector_funcs = { - .reset = drm_atomic_helper_connector_reset, - .detect = tinydrm_connector_detect, - .fill_modes = drm_helper_probe_single_connector_modes, - .destroy = tinydrm_connector_destroy, - .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, - .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, -}; - -struct drm_connector * -tinydrm_connector_create(struct drm_device *drm, - const struct drm_display_mode *mode, - int connector_type) -{ - struct tinydrm_connector *tconn; - struct drm_connector *connector; - int ret; - - tconn = kzalloc(sizeof(*tconn), GFP_KERNEL); - if (!tconn) - return ERR_PTR(-ENOMEM); - - drm_mode_copy(&tconn->mode, mode); - connector = &tconn->base; - - drm_connector_helper_add(connector, &tinydrm_connector_hfuncs); - ret = drm_connector_init(drm, connector, &tinydrm_connector_funcs, - connector_type); - if (ret) { - kfree(tconn); - return ERR_PTR(ret); - } - - connector->status = connector_status_connected; - - return connector; -} - -static int tinydrm_rotate_mode(struct drm_display_mode *mode, - unsigned int rotation) -{ - if (rotation == 0 || rotation == 180) { - return 0; - } else if (rotation == 90 || rotation == 270) { - swap(mode->hdisplay, mode->vdisplay); - swap(mode->hsync_start, mode->vsync_start); - swap(mode->hsync_end, mode->vsync_end); - swap(mode->htotal, mode->vtotal); - swap(mode->width_mm, mode->height_mm); - return 0; - } else { - return -EINVAL; - } -} - -/** - * tinydrm_display_pipe_init - Initialize display pipe - * @drm: DRM device - * @pipe: Display pipe - * @funcs: Display pipe functions - * @connector_type: Connector type - * @formats: Array of supported formats (DRM_FORMAT_*) - * @format_count: Number of elements in @formats - * @mode: Supported mode - * @rotation: Initial @mode rotation in degrees Counter Clock Wise - * - * This function sets up a &drm_simple_display_pipe with a &drm_connector that - * has one fixed &drm_display_mode which is rotated according to @rotation. - * - * Returns: - * Zero on success, negative error code on failure. - */ -int tinydrm_display_pipe_init(struct drm_device *drm, - struct drm_simple_display_pipe *pipe, - const struct drm_simple_display_pipe_funcs *funcs, - int connector_type, - const uint32_t *formats, - unsigned int format_count, - const struct drm_display_mode *mode, - unsigned int rotation) -{ - struct drm_display_mode mode_copy; - struct drm_connector *connector; - int ret; - static const uint64_t modifiers[] = { - DRM_FORMAT_MOD_LINEAR, - DRM_FORMAT_MOD_INVALID - }; - - drm_mode_copy(&mode_copy, mode); - ret = tinydrm_rotate_mode(&mode_copy, rotation); - if (ret) { - DRM_ERROR("Illegal rotation value %u\n", rotation); - return -EINVAL; - } - - drm->mode_config.min_width = mode_copy.hdisplay; - drm->mode_config.max_width = mode_copy.hdisplay; - drm->mode_config.min_height = mode_copy.vdisplay; - drm->mode_config.max_height = mode_copy.vdisplay; - - connector = tinydrm_connector_create(drm, &mode_copy, connector_type); - if (IS_ERR(connector)) - return PTR_ERR(connector); - - return drm_simple_display_pipe_init(drm, pipe, funcs, formats, - format_count, modifiers, connector); -} -EXPORT_SYMBOL(tinydrm_display_pipe_init); - -MODULE_LICENSE("GPL"); diff --git a/drivers/gpu/drm/tinydrm/hx8357d.c b/drivers/gpu/drm/tinydrm/hx8357d.c index be197c5c3211..7d2dfa92b661 100644 --- a/drivers/gpu/drm/tinydrm/hx8357d.c +++ b/drivers/gpu/drm/tinydrm/hx8357d.c @@ -23,7 +23,6 @@ #include <drm/drm_gem_framebuffer_helper.h> #include <drm/drm_modeset_helper.h> #include <drm/tinydrm/mipi-dbi.h> -#include <drm/tinydrm/tinydrm-helpers.h> #include <video/mipi_display.h>
#define HX8357D_SETOSC 0xb0 diff --git a/drivers/gpu/drm/tinydrm/ili9341.c b/drivers/gpu/drm/tinydrm/ili9341.c index 00f28b8e4345..cb0a0ddbc090 100644 --- a/drivers/gpu/drm/tinydrm/ili9341.c +++ b/drivers/gpu/drm/tinydrm/ili9341.c @@ -22,7 +22,6 @@ #include <drm/drm_gem_framebuffer_helper.h> #include <drm/drm_modeset_helper.h> #include <drm/tinydrm/mipi-dbi.h> -#include <drm/tinydrm/tinydrm-helpers.h> #include <video/mipi_display.h>
#define ILI9341_FRMCTR1 0xb1 diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c b/drivers/gpu/drm/tinydrm/mi0283qt.c index 7a14d6b355f2..f21d58dcaa5a 100644 --- a/drivers/gpu/drm/tinydrm/mi0283qt.c +++ b/drivers/gpu/drm/tinydrm/mi0283qt.c @@ -20,7 +20,6 @@ #include <drm/drm_gem_framebuffer_helper.h> #include <drm/drm_modeset_helper.h> #include <drm/tinydrm/mipi-dbi.h> -#include <drm/tinydrm/tinydrm-helpers.h> #include <video/mipi_display.h>
#define ILI9341_FRMCTR1 0xb1 diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c index a264c0bb74b0..42465228129c 100644 --- a/drivers/gpu/drm/tinydrm/mipi-dbi.c +++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c @@ -13,6 +13,7 @@ #include <linux/regulator/consumer.h> #include <linux/spi/spi.h>
+#include <drm/drm_connector.h> #include <drm/drm_damage_helper.h> #include <drm/drm_drv.h> #include <drm/drm_fb_cma_helper.h> @@ -20,10 +21,11 @@ #include <drm/drm_format_helper.h> #include <drm/drm_fourcc.h> #include <drm/drm_gem_framebuffer_helper.h> -#include <drm/drm_vblank.h> +#include <drm/drm_modes.h> +#include <drm/drm_probe_helper.h> #include <drm/drm_rect.h> +#include <drm/drm_vblank.h> #include <drm/tinydrm/mipi-dbi.h> -#include <drm/tinydrm/tinydrm-helpers.h> #include <video/mipi_display.h>
#define MIPI_DBI_MAX_SPI_READ_SPEED 2000000 /* 2MHz */ @@ -400,6 +402,60 @@ void mipi_dbi_pipe_disable(struct drm_simple_display_pipe *pipe) } EXPORT_SYMBOL(mipi_dbi_pipe_disable);
+static int mipi_dbi_connector_get_modes(struct drm_connector *connector) +{ + struct mipi_dbi *mipi = drm_to_mipi_dbi(connector->dev); + struct drm_display_mode *mode; + + mode = drm_mode_duplicate(connector->dev, &mipi->mode); + if (!mode) { + DRM_ERROR("Failed to duplicate mode\n"); + return 0; + } + + if (mode->name[0] == '\0') + drm_mode_set_name(mode); + + mode->type |= DRM_MODE_TYPE_PREFERRED; + drm_mode_probed_add(connector, mode); + + if (mode->width_mm) { + connector->display_info.width_mm = mode->width_mm; + connector->display_info.height_mm = mode->height_mm; + } + + return 1; +} + +static const struct drm_connector_helper_funcs mipi_dbi_connector_hfuncs = { + .get_modes = mipi_dbi_connector_get_modes, +}; + +static const struct drm_connector_funcs mipi_dbi_connector_funcs = { + .reset = drm_atomic_helper_connector_reset, + .fill_modes = drm_helper_probe_single_connector_modes, + .destroy = drm_connector_cleanup, + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, +}; + +static int mipi_dbi_rotate_mode(struct drm_display_mode *mode, + unsigned int rotation) +{ + if (rotation == 0 || rotation == 180) { + return 0; + } else if (rotation == 90 || rotation == 270) { + swap(mode->hdisplay, mode->vdisplay); + swap(mode->hsync_start, mode->vsync_start); + swap(mode->hsync_end, mode->vsync_end); + swap(mode->htotal, mode->vtotal); + swap(mode->width_mm, mode->height_mm); + return 0; + } else { + return -EINVAL; + } +} + static const struct drm_mode_config_funcs mipi_dbi_mode_config_funcs = { .fb_create = drm_gem_fb_create_with_dirty, .atomic_check = drm_atomic_helper_check, @@ -440,6 +496,10 @@ int mipi_dbi_init_with_formats(struct mipi_dbi *mipi, const struct drm_display_mode *mode, unsigned int rotation, size_t tx_buf_size) { + static const uint64_t modifiers[] = { + DRM_FORMAT_MOD_LINEAR, + DRM_FORMAT_MOD_INVALID + }; struct drm_device *drm = &mipi->drm; int ret;
@@ -452,16 +512,31 @@ int mipi_dbi_init_with_formats(struct mipi_dbi *mipi, if (!mipi->tx_buf) return -ENOMEM;
- ret = tinydrm_display_pipe_init(drm, &mipi->pipe, funcs, - DRM_MODE_CONNECTOR_SPI, - formats, format_count, mode, - rotation); + drm_mode_copy(&mipi->mode, mode); + ret = mipi_dbi_rotate_mode(&mipi->mode, rotation); + if (ret) { + DRM_ERROR("Illegal rotation value %u\n", rotation); + return -EINVAL; + } + + drm_connector_helper_add(&mipi->connector, &mipi_dbi_connector_hfuncs); + ret = drm_connector_init(drm, &mipi->connector, &mipi_dbi_connector_funcs, + DRM_MODE_CONNECTOR_SPI); + if (ret) + return ret; + + ret = drm_simple_display_pipe_init(drm, &mipi->pipe, funcs, formats, format_count, + modifiers, &mipi->connector); if (ret) return ret;
drm_plane_enable_fb_damage_clips(&mipi->pipe.plane);
drm->mode_config.funcs = &mipi_dbi_mode_config_funcs; + drm->mode_config.min_width = mipi->mode.hdisplay; + drm->mode_config.max_width = mipi->mode.hdisplay; + drm->mode_config.min_height = mipi->mode.vdisplay; + drm->mode_config.max_height = mipi->mode.vdisplay; mipi->rotation = rotation;
DRM_DEBUG_KMS("rotation = %u\n", rotation); diff --git a/drivers/gpu/drm/tinydrm/st7735r.c b/drivers/gpu/drm/tinydrm/st7735r.c index b23899788f5b..151749035a19 100644 --- a/drivers/gpu/drm/tinydrm/st7735r.c +++ b/drivers/gpu/drm/tinydrm/st7735r.c @@ -20,7 +20,6 @@ #include <drm/drm_gem_cma_helper.h> #include <drm/drm_gem_framebuffer_helper.h> #include <drm/tinydrm/mipi-dbi.h> -#include <drm/tinydrm/tinydrm-helpers.h>
#define ST7735R_FRMCTR1 0xb1 #define ST7735R_FRMCTR2 0xb2 diff --git a/include/drm/tinydrm/mipi-dbi.h b/include/drm/tinydrm/mipi-dbi.h index c4b04789bf84..393323f87e12 100644 --- a/include/drm/tinydrm/mipi-dbi.h +++ b/include/drm/tinydrm/mipi-dbi.h @@ -46,6 +46,16 @@ struct mipi_dbi { */ struct drm_simple_display_pipe pipe;
+ /** + * @connector: Connector + */ + struct drm_connector connector; + + /** + * @mode: Fixed display mode + */ + struct drm_display_mode mode; + struct spi_device *spi; bool enabled; struct mutex cmdlock; diff --git a/include/drm/tinydrm/tinydrm-helpers.h b/include/drm/tinydrm/tinydrm-helpers.h deleted file mode 100644 index 0e7470771c5e..000000000000 --- a/include/drm/tinydrm/tinydrm-helpers.h +++ /dev/null @@ -1,27 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-or-later */ -/* - * Copyright (C) 2016 Noralf Trønnes - */ - -#ifndef __LINUX_TINYDRM_HELPERS_H -#define __LINUX_TINYDRM_HELPERS_H - -struct backlight_device; -struct drm_device; -struct drm_display_mode; -struct drm_framebuffer; -struct drm_rect; -struct drm_simple_display_pipe; -struct drm_simple_display_pipe_funcs; -struct device; - -int tinydrm_display_pipe_init(struct drm_device *drm, - struct drm_simple_display_pipe *pipe, - const struct drm_simple_display_pipe_funcs *funcs, - int connector_type, - const uint32_t *formats, - unsigned int format_count, - const struct drm_display_mode *mode, - unsigned int rotation); - -#endif /* __LINUX_TINYDRM_HELPERS_H */
Den 19.07.2019 17.59, skrev Noralf Trønnes:
This series removes the remaining bits of tinydrm.ko.
Changes:
- Split SPI connector type patch in core and driver changes, expand commit message (Daniel)
- Drop moving the mipi_dbi_spi_init() declaration (Sam)
- st7586: Forgot to remove the mipi->rotation assignment
Note: This depends on a commit that just entered Linus' tree: e6f3f7e4dc76 ("spi: Add spi_is_bpw_supported()")
Series is also available here: https://github.com/notro/linux/tree/remove_tinydrm_ko
Noralf.
Noralf Trønnes (11): drm: Add SPI connector type drm/tinydrm: Use DRM_MODE_CONNECTOR_SPI drm/tinydrm: Use spi_is_bpw_supported() drm/tinydrm: Remove spi debug buffer dumping drm/tinydrm: Remove tinydrm_spi_max_transfer_size() drm/tinydrm: Clean up tinydrm_spi_transfer() drm/tinydrm: Move tinydrm_spi_transfer() drm/tinydrm: Move tinydrm_machine_little_endian() drm/tinydrm/repaper: Don't use tinydrm_display_pipe_init() drm/tinydrm/mipi-dbi: Add mipi_dbi_init_with_formats() drm/tinydrm: Move tinydrm_display_pipe_init() to mipi-dbi
Series applied to drm-misc-next, thanks for review and testing!
Noralf.
Documentation/gpu/tinydrm.rst | 12 - Documentation/gpu/todo.rst | 3 - drivers/gpu/drm/drm_connector.c | 1 + drivers/gpu/drm/tinydrm/Makefile | 1 - drivers/gpu/drm/tinydrm/core/Makefile | 4 - .../gpu/drm/tinydrm/core/tinydrm-helpers.c | 207 -------------- drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c | 179 ------------ drivers/gpu/drm/tinydrm/hx8357d.c | 1 - drivers/gpu/drm/tinydrm/ili9225.c | 5 +- drivers/gpu/drm/tinydrm/ili9341.c | 1 - drivers/gpu/drm/tinydrm/mi0283qt.c | 1 - drivers/gpu/drm/tinydrm/mipi-dbi.c | 254 ++++++++++++++---- drivers/gpu/drm/tinydrm/repaper.c | 58 +++- drivers/gpu/drm/tinydrm/st7586.c | 33 +-- drivers/gpu/drm/tinydrm/st7735r.c | 1 - include/drm/tinydrm/mipi-dbi.h | 18 ++ include/drm/tinydrm/tinydrm-helpers.h | 75 ------ include/uapi/drm/drm_mode.h | 1 + 18 files changed, 283 insertions(+), 572 deletions(-) delete mode 100644 drivers/gpu/drm/tinydrm/core/Makefile delete mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c delete mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c delete mode 100644 include/drm/tinydrm/tinydrm-helpers.h
dri-devel@lists.freedesktop.org
-
Andy Shevchenko -
Daniel Vetter -
David Lechner -
Mark Brown -
Noralf Trønnes