All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/10] drm/tinydrm: Remove tinydrm.ko
@ 2019-07-17 11:58 Noralf Trønnes
  2019-07-17 11:58 ` [PATCH 01/10] drm: Add SPI connector type Noralf Trønnes
                   ` (10 more replies)
  0 siblings, 11 replies; 33+ messages in thread
From: Noralf Trønnes @ 2019-07-17 11:58 UTC (permalink / raw)
  To: dri-devel; +Cc: david

This series removes the remaining bits of tinydrm.ko.

There's only one item left on the tinydrm todo list and that is moving
out mipi_dbi.

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 (10):
  drm: Add SPI connector type
  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              |  32 +--
 drivers/gpu/drm/tinydrm/st7735r.c             |   1 -
 include/drm/tinydrm/mipi-dbi.h                |  22 +-
 include/drm/tinydrm/tinydrm-helpers.h         |  75 ------
 include/uapi/drm/drm_mode.h                   |   1 +
 18 files changed, 285 insertions(+), 573 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

-- 
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] 33+ messages in thread
* [PATCH 01/10] drm: Add SPI connector type
  2019-07-17 11:58 [PATCH 00/10] drm/tinydrm: Remove tinydrm.ko Noralf Trønnes
@ 2019-07-17 11:58 ` Noralf Trønnes
  2019-07-17 19:24   ` David Lechner
  2019-07-19  9:17   ` Daniel Vetter
  2019-07-17 11:58 ` [PATCH 02/10] drm/tinydrm: Use spi_is_bpw_supported() Noralf Trønnes
                   ` (9 subsequent siblings)
  10 siblings, 2 replies; 33+ messages in thread
From: Noralf Trønnes @ 2019-07-17 11:58 UTC (permalink / raw)
  To: dri-devel; +Cc: david

tinydrm drivers announce DRM_MODE_CONNECTOR_VIRTUAL for its SPI drivers.
Stop lying and add a SPI connector type

Cc: David Lechner <david@lechnology.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/drm_connector.c    | 1 +
 drivers/gpu/drm/tinydrm/mipi-dbi.c | 3 +--
 drivers/gpu/drm/tinydrm/repaper.c  | 2 +-
 drivers/gpu/drm/tinydrm/st7586.c   | 2 +-
 include/uapi/drm/drm_mode.h        | 1 +
 5 files changed, 5 insertions(+), 4 deletions(-)

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/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)
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 {
 
-- 
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] 33+ messages in thread
* [PATCH 02/10] drm/tinydrm: Use spi_is_bpw_supported()
  2019-07-17 11:58 [PATCH 00/10] drm/tinydrm: Remove tinydrm.ko Noralf Trønnes
  2019-07-17 11:58 ` [PATCH 01/10] drm: Add SPI connector type Noralf Trønnes
@ 2019-07-17 11:58 ` Noralf Trønnes
  2019-07-17 11:58 ` [PATCH 03/10] drm/tinydrm: Remove spi debug buffer dumping Noralf Trønnes
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 33+ messages in thread
From: Noralf Trønnes @ 2019-07-17 11:58 UTC (permalink / raw)
  To: dri-devel; +Cc: david

This means that tinydrm_spi_bpw_supported() can be removed.

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);
-- 
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] 33+ messages in thread
* [PATCH 03/10] drm/tinydrm: Remove spi debug buffer dumping
  2019-07-17 11:58 [PATCH 00/10] drm/tinydrm: Remove tinydrm.ko Noralf Trønnes
  2019-07-17 11:58 ` [PATCH 01/10] drm: Add SPI connector type Noralf Trønnes
  2019-07-17 11:58 ` [PATCH 02/10] drm/tinydrm: Use spi_is_bpw_supported() Noralf Trønnes
@ 2019-07-17 11:58 ` Noralf Trønnes
  2019-07-17 11:58 ` [PATCH 04/10] drm/tinydrm: Remove tinydrm_spi_max_transfer_size() Noralf Trønnes
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 33+ messages in thread
From: Noralf Trønnes @ 2019-07-17 11:58 UTC (permalink / raw)
  To: dri-devel; +Cc: david

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.

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 */
-- 
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] 33+ messages in thread
* [PATCH 04/10] drm/tinydrm: Remove tinydrm_spi_max_transfer_size()
  2019-07-17 11:58 [PATCH 00/10] drm/tinydrm: Remove tinydrm.ko Noralf Trønnes
                   ` (2 preceding siblings ...)
  2019-07-17 11:58 ` [PATCH 03/10] drm/tinydrm: Remove spi debug buffer dumping Noralf Trønnes
@ 2019-07-17 11:58 ` Noralf Trønnes
  2019-07-17 11:58 ` [PATCH 05/10] drm/tinydrm: Clean up tinydrm_spi_transfer() Noralf Trønnes
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 33+ messages in thread
From: Noralf Trønnes @ 2019-07-17 11:58 UTC (permalink / raw)
  To: dri-devel; +Cc: david

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.

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);
-- 
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] 33+ messages in thread
* [PATCH 05/10] drm/tinydrm: Clean up tinydrm_spi_transfer()
  2019-07-17 11:58 [PATCH 00/10] drm/tinydrm: Remove tinydrm.ko Noralf Trønnes
                   ` (3 preceding siblings ...)
  2019-07-17 11:58 ` [PATCH 04/10] drm/tinydrm: Remove tinydrm_spi_max_transfer_size() Noralf Trønnes
@ 2019-07-17 11:58 ` Noralf Trønnes
  2019-07-17 13:09   ` Sam Ravnborg
  2019-07-17 19:37   ` David Lechner
  2019-07-17 11:58 ` [PATCH 06/10] drm/tinydrm: Move tinydrm_spi_transfer() Noralf Trønnes
                   ` (5 subsequent siblings)
  10 siblings, 2 replies; 33+ messages in thread
From: Noralf Trønnes @ 2019-07-17 11:58 UTC (permalink / raw)
  To: dri-devel; +Cc: david

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>
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 */
-- 
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] 33+ messages in thread
* [PATCH 06/10] drm/tinydrm: Move tinydrm_spi_transfer()
  2019-07-17 11:58 [PATCH 00/10] drm/tinydrm: Remove tinydrm.ko Noralf Trønnes
                   ` (4 preceding siblings ...)
  2019-07-17 11:58 ` [PATCH 05/10] drm/tinydrm: Clean up tinydrm_spi_transfer() Noralf Trønnes
@ 2019-07-17 11:58 ` Noralf Trønnes
  2019-07-17 13:15   ` Sam Ravnborg
  2019-07-17 19:48   ` David Lechner
  2019-07-17 11:58 ` [PATCH 07/10] drm/tinydrm: Move tinydrm_machine_little_endian() Noralf Trønnes
                   ` (4 subsequent siblings)
  10 siblings, 2 replies; 33+ messages in thread
From: Noralf Trønnes @ 2019-07-17 11:58 UTC (permalink / raw)
  To: dri-devel; +Cc: david

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.

Additionally move the mipi_dbi_spi_init() declaration to the other SPI
functions.

Cc: 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                |  7 +-
 include/drm/tinydrm/tinydrm-helpers.h         |  5 --
 9 files changed, 59 insertions(+), 89 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 3f6ecf846263..384199325304 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -456,9 +456,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..576e9a7349ab 100644
--- a/include/drm/tinydrm/mipi-dbi.h
+++ b/include/drm/tinydrm/mipi-dbi.h
@@ -67,8 +67,6 @@ static inline struct mipi_dbi *drm_to_mipi_dbi(struct drm_device *drm)
 	return container_of(drm, struct mipi_dbi, drm);
 }
 
-int mipi_dbi_spi_init(struct spi_device *spi, struct mipi_dbi *mipi,
-		      struct gpio_desc *dc);
 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);
@@ -83,7 +81,12 @@ 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_init(struct spi_device *spi, struct mipi_dbi *mipi,
+		      struct gpio_desc *dc);
+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 */
-- 
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] 33+ messages in thread
* [PATCH 07/10] drm/tinydrm: Move tinydrm_machine_little_endian()
  2019-07-17 11:58 [PATCH 00/10] drm/tinydrm: Remove tinydrm.ko Noralf Trønnes
                   ` (5 preceding siblings ...)
  2019-07-17 11:58 ` [PATCH 06/10] drm/tinydrm: Move tinydrm_spi_transfer() Noralf Trønnes
@ 2019-07-17 11:58 ` Noralf Trønnes
  2019-07-17 20:09   ` David Lechner
  2019-07-17 11:58 ` [PATCH 08/10] drm/tinydrm/repaper: Don't use tinydrm_display_pipe_init() Noralf Trønnes
                   ` (3 subsequent siblings)
  10 siblings, 1 reply; 33+ messages in thread
From: Noralf Trønnes @ 2019-07-17 11:58 UTC (permalink / raw)
  To: dri-devel; +Cc: david

The tinydrm helper is going away so move it into the only user mipi-dbi.

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,
-- 
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] 33+ messages in thread
* [PATCH 08/10] drm/tinydrm/repaper: Don't use tinydrm_display_pipe_init()
  2019-07-17 11:58 [PATCH 00/10] drm/tinydrm: Remove tinydrm.ko Noralf Trønnes
                   ` (6 preceding siblings ...)
  2019-07-17 11:58 ` [PATCH 07/10] drm/tinydrm: Move tinydrm_machine_little_endian() Noralf Trønnes
@ 2019-07-17 11:58 ` Noralf Trønnes
  2019-07-17 11:58 ` [PATCH 09/10] drm/tinydrm/mipi-dbi: Add mipi_dbi_init_with_formats() Noralf Trønnes
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 33+ messages in thread
From: Noralf Trønnes @ 2019-07-17 11:58 UTC (permalink / raw)
  To: dri-devel; +Cc: david

tinydrm.ko is going away so let's implement a connector.

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;
 
-- 
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] 33+ messages in thread
* [PATCH 09/10] drm/tinydrm/mipi-dbi: Add mipi_dbi_init_with_formats()
  2019-07-17 11:58 [PATCH 00/10] drm/tinydrm: Remove tinydrm.ko Noralf Trønnes
                   ` (7 preceding siblings ...)
  2019-07-17 11:58 ` [PATCH 08/10] drm/tinydrm/repaper: Don't use tinydrm_display_pipe_init() Noralf Trønnes
@ 2019-07-17 11:58 ` Noralf Trønnes
  2019-07-17 20:38   ` David Lechner
  2019-07-17 11:58 ` [PATCH 10/10] drm/tinydrm: Move tinydrm_display_pipe_init() to mipi-dbi Noralf Trønnes
  2019-07-17 13:31 ` [PATCH 00/10] drm/tinydrm: Remove tinydrm.ko Sam Ravnborg
  10 siblings, 1 reply; 33+ messages in thread
From: Noralf Trønnes @ 2019-07-17 11:58 UTC (permalink / raw)
  To: dri-devel; +Cc: david

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.

Cc: 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   | 32 ++---------
 include/drm/tinydrm/mipi-dbi.h     |  5 ++
 3 files changed, 74 insertions(+), 54 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..650ca8d4732e 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)) {
@@ -374,6 +360,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 +375,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 +383,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 576e9a7349ab..2f0119e2c47e 100644
--- a/include/drm/tinydrm/mipi-dbi.h
+++ b/include/drm/tinydrm/mipi-dbi.h
@@ -67,6 +67,11 @@ static inline struct mipi_dbi *drm_to_mipi_dbi(struct drm_device *drm)
 	return container_of(drm, struct mipi_dbi, drm);
 }
 
+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);
-- 
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] 33+ messages in thread
* [PATCH 10/10] drm/tinydrm: Move tinydrm_display_pipe_init() to mipi-dbi
  2019-07-17 11:58 [PATCH 00/10] drm/tinydrm: Remove tinydrm.ko Noralf Trønnes
                   ` (8 preceding siblings ...)
  2019-07-17 11:58 ` [PATCH 09/10] drm/tinydrm/mipi-dbi: Add mipi_dbi_init_with_formats() Noralf Trønnes
@ 2019-07-17 11:58 ` Noralf Trønnes
  2019-07-17 13:34   ` Sam Ravnborg
  2019-07-17 20:46   ` David Lechner
  2019-07-17 13:31 ` [PATCH 00/10] drm/tinydrm: Remove tinydrm.ko Sam Ravnborg
  10 siblings, 2 replies; 33+ messages in thread
From: Noralf Trønnes @ 2019-07-17 11:58 UTC (permalink / raw)
  To: dri-devel; +Cc: david

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>
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 2f0119e2c47e..8ab3bde78723 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 */
-- 
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] 33+ messages in thread
* Re: [PATCH 05/10] drm/tinydrm: Clean up tinydrm_spi_transfer()
  2019-07-17 11:58 ` [PATCH 05/10] drm/tinydrm: Clean up tinydrm_spi_transfer() Noralf Trønnes
@ 2019-07-17 13:09   ` Sam Ravnborg
  2019-07-17 16:18     ` Noralf Trønnes
  2019-07-17 19:37   ` David Lechner
  1 sibling, 1 reply; 33+ messages in thread
From: Sam Ravnborg @ 2019-07-17 13:09 UTC (permalink / raw)
  To: Noralf Trønnes; +Cc: david, dri-devel

On Wed, Jul 17, 2019 at 01:58:12PM +0200, Noralf Trønnes wrote:
> 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.

Keeping unused code around is never a good idea.
On the other hand, should we not try to get the driver in questions
ported so we have a user and we do not need to re-add this later?
What driver/display needs this?

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

^ permalink raw reply	[flat|nested] 33+ messages in thread
* Re: [PATCH 06/10] drm/tinydrm: Move tinydrm_spi_transfer()
  2019-07-17 11:58 ` [PATCH 06/10] drm/tinydrm: Move tinydrm_spi_transfer() Noralf Trønnes
@ 2019-07-17 13:15   ` Sam Ravnborg
  2019-07-17 16:20     ` Noralf Trønnes
  2019-07-17 19:48   ` David Lechner
  1 sibling, 1 reply; 33+ messages in thread
From: Sam Ravnborg @ 2019-07-17 13:15 UTC (permalink / raw)
  To: Noralf Trønnes; +Cc: david, dri-devel

Hi Noralf.

On Wed, Jul 17, 2019 at 01:58:13PM +0200, Noralf Trønnes wrote:
> 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.
> 
> Additionally move the mipi_dbi_spi_init() declaration to the other SPI
> functions.
> 
> Cc: David Lechner <david@lechnology.com>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
With the few nitpics considered:
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>


> diff --git a/include/drm/tinydrm/mipi-dbi.h b/include/drm/tinydrm/mipi-dbi.h
> index 51fc667beef7..576e9a7349ab 100644
> --- a/include/drm/tinydrm/mipi-dbi.h
> +++ b/include/drm/tinydrm/mipi-dbi.h
> @@ -67,8 +67,6 @@ static inline struct mipi_dbi *drm_to_mipi_dbi(struct drm_device *drm)
>  	return container_of(drm, struct mipi_dbi, drm);
>  }
>  
> -int mipi_dbi_spi_init(struct spi_device *spi, struct mipi_dbi *mipi,
> -		      struct gpio_desc *dc);
Moving this prototype looks like it belongs in another patch?

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

^ permalink raw reply	[flat|nested] 33+ messages in thread
* Re: [PATCH 00/10] drm/tinydrm: Remove tinydrm.ko
  2019-07-17 11:58 [PATCH 00/10] drm/tinydrm: Remove tinydrm.ko Noralf Trønnes
                   ` (9 preceding siblings ...)
  2019-07-17 11:58 ` [PATCH 10/10] drm/tinydrm: Move tinydrm_display_pipe_init() to mipi-dbi Noralf Trønnes
@ 2019-07-17 13:31 ` Sam Ravnborg
  2019-07-17 16:22   ` Noralf Trønnes
  10 siblings, 1 reply; 33+ messages in thread
From: Sam Ravnborg @ 2019-07-17 13:31 UTC (permalink / raw)
  To: Noralf Trønnes; +Cc: david, dri-devel

Hi Noralf.

Nice series of patches!



On Wed, Jul 17, 2019 at 01:58:07PM +0200, Noralf Trønnes wrote:
> This series removes the remaining bits of tinydrm.ko.
> 
> There's only one item left on the tinydrm todo list and that is moving
> out mipi_dbi.
> 
> 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 (10):
>   drm: Add SPI connector type
>   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
> 

Patch 1-3, 7, 8, 9 are:
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>

Patch 4 are:
Acked-by: Sam Ravnborg <sam@ravnborg.org>
(Did not feel I had enough background to say r-b).

Individual comments sent for the other patches.

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

^ permalink raw reply	[flat|nested] 33+ messages in thread
* Re: [PATCH 10/10] drm/tinydrm: Move tinydrm_display_pipe_init() to mipi-dbi
  2019-07-17 11:58 ` [PATCH 10/10] drm/tinydrm: Move tinydrm_display_pipe_init() to mipi-dbi Noralf Trønnes
@ 2019-07-17 13:34   ` Sam Ravnborg
  2019-07-17 20:46   ` David Lechner
  1 sibling, 0 replies; 33+ messages in thread
From: Sam Ravnborg @ 2019-07-17 13:34 UTC (permalink / raw)
  To: Noralf Trønnes; +Cc: david, dri-devel

On Wed, Jul 17, 2019 at 01:58:17PM +0200, Noralf Trønnes wrote:
> 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.

For review purposes it would have been easier had this been split in a
few more patches.
But anyway, I think I managed to follow all changes.
So it has my:
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>

> 
> Cc: Eric Anholt <eric@anholt.net>
> Cc: David Lechner <david@lechnology.com>
> 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 2f0119e2c47e..8ab3bde78723 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 */
> -- 
> 2.20.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 33+ messages in thread
* Re: [PATCH 05/10] drm/tinydrm: Clean up tinydrm_spi_transfer()
  2019-07-17 13:09   ` Sam Ravnborg
@ 2019-07-17 16:18     ` Noralf Trønnes
  2019-07-17 18:13       ` Sam Ravnborg
  0 siblings, 1 reply; 33+ messages in thread
From: Noralf Trønnes @ 2019-07-17 16:18 UTC (permalink / raw)
  To: Sam Ravnborg; +Cc: david, dri-devel



Den 17.07.2019 15.09, skrev Sam Ravnborg:
> On Wed, Jul 17, 2019 at 01:58:12PM +0200, Noralf Trønnes wrote:
>> 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.
> 
> Keeping unused code around is never a good idea.
> On the other hand, should we not try to get the driver in questions
> ported so we have a user and we do not need to re-add this later?
> What driver/display needs this?

At least drivers/staging/fbtft/fb_ili932{0,5}.c and maybe another one, I
don't remember. I haven't worked on fbtft after I did tinydrm.
It looks like they still sell the hy28b:
https://www.hotmcu.com/28-touch-screen-tft-lcd-with-all-interface-p-63.html

I'm not sure what the future of fbtft is. The idea was that the drivers
should get cleaned up and move out of staging, but then fbdev was closed
for new drivers and I did tinydrm. Only two drivers have been converted
apart from mi0283qt that I did and that is hx8357 which Eric did and
st7735 that David did. Those 3 covers a lot of the tiny SPI display
marked, Adafruit sells them.
It's a chicken and egg problem, as long as the fbtft drivers are there
and working, there's no incentive to convert them.

There's another challenge with these drivers since it is possible to
override the init sequence in Device Tree, meaning they can work with
all kinds of displays without writing a new driver.
I was not allowed to keep that functionality outside of staging.

When I'm done with the tinydrm cleanup, I'm going to work on an idea I
have: turn the Raspberry Pi Zero into a $5 USB to
HDMI/SDTV/DSI/DPI/SPI-display adapter. I'm planning to write a generic
USB host display driver with a matching Linux OTG device driver.
I plan to make it easy to do the display OTG side on a microcontroller
as well. This way it will be possible for manufacturers to do USB
connected displays of (nearly) all sizes without having to write a Linux
driver.

It's difficult to predict the future, but powerful microcontrollers are
cheap nowadays so maybe these SPI displays will be fased out by cheap
USB displays. The uC can replace the touch controller cutting some of
the uC cost.

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

^ permalink raw reply	[flat|nested] 33+ messages in thread
* Re: [PATCH 06/10] drm/tinydrm: Move tinydrm_spi_transfer()
  2019-07-17 13:15   ` Sam Ravnborg
@ 2019-07-17 16:20     ` Noralf Trønnes
  0 siblings, 0 replies; 33+ messages in thread
From: Noralf Trønnes @ 2019-07-17 16:20 UTC (permalink / raw)
  To: Sam Ravnborg; +Cc: david, dri-devel



Den 17.07.2019 15.15, skrev Sam Ravnborg:
> Hi Noralf.
> 
> On Wed, Jul 17, 2019 at 01:58:13PM +0200, Noralf Trønnes wrote:
>> 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.
>>
>> Additionally move the mipi_dbi_spi_init() declaration to the other SPI
>> functions.
>>
>> Cc: David Lechner <david@lechnology.com>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> With the few nitpics considered:
> Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
> 
> 
>> diff --git a/include/drm/tinydrm/mipi-dbi.h b/include/drm/tinydrm/mipi-dbi.h
>> index 51fc667beef7..576e9a7349ab 100644
>> --- a/include/drm/tinydrm/mipi-dbi.h
>> +++ b/include/drm/tinydrm/mipi-dbi.h
>> @@ -67,8 +67,6 @@ static inline struct mipi_dbi *drm_to_mipi_dbi(struct drm_device *drm)
>>  	return container_of(drm, struct mipi_dbi, drm);
>>  }
>>  
>> -int mipi_dbi_spi_init(struct spi_device *spi, struct mipi_dbi *mipi,
>> -		      struct gpio_desc *dc);
> Moving this prototype looks like it belongs in another patch?
> 

Strictly speaking it does, if you don't like I'll just drop it.

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

^ permalink raw reply	[flat|nested] 33+ messages in thread
* Re: [PATCH 00/10] drm/tinydrm: Remove tinydrm.ko
  2019-07-17 13:31 ` [PATCH 00/10] drm/tinydrm: Remove tinydrm.ko Sam Ravnborg
@ 2019-07-17 16:22   ` Noralf Trønnes
  0 siblings, 0 replies; 33+ messages in thread
From: Noralf Trønnes @ 2019-07-17 16:22 UTC (permalink / raw)
  To: Sam Ravnborg; +Cc: david, dri-devel



Den 17.07.2019 15.31, skrev Sam Ravnborg:
> Hi Noralf.
> 
> Nice series of patches!
> 
> 
> 
> On Wed, Jul 17, 2019 at 01:58:07PM +0200, Noralf Trønnes wrote:
>> This series removes the remaining bits of tinydrm.ko.
>>
>> There's only one item left on the tinydrm todo list and that is moving
>> out mipi_dbi.
>>
>> 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 (10):
>>   drm: Add SPI connector type
>>   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
>>
> 
> Patch 1-3, 7, 8, 9 are:
> Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
> 
> Patch 4 are:
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
> (Did not feel I had enough background to say r-b).
> 
> Individual comments sent for the other patches.
> 

Thanks for your review Sam!

Noralf.

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

^ permalink raw reply	[flat|nested] 33+ messages in thread
* Re: [PATCH 05/10] drm/tinydrm: Clean up tinydrm_spi_transfer()
  2019-07-17 16:18     ` Noralf Trønnes
@ 2019-07-17 18:13       ` Sam Ravnborg
  0 siblings, 0 replies; 33+ messages in thread
From: Sam Ravnborg @ 2019-07-17 18:13 UTC (permalink / raw)
  To: Noralf Trønnes; +Cc: david, dri-devel

Hi Noralf.

On Wed, Jul 17, 2019 at 06:18:39PM +0200, Noralf Trønnes wrote:
> 
> 
> Den 17.07.2019 15.09, skrev Sam Ravnborg:
> > On Wed, Jul 17, 2019 at 01:58:12PM +0200, Noralf Trønnes wrote:
> >> 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.
> > 
> > Keeping unused code around is never a good idea.
> > On the other hand, should we not try to get the driver in questions
> > ported so we have a user and we do not need to re-add this later?
> > What driver/display needs this?
> 
> At least drivers/staging/fbtft/fb_ili932{0,5}.c and maybe another one, I
> don't remember. I haven't worked on fbtft after I did tinydrm.
> It looks like they still sell the hy28b:
> https://www.hotmcu.com/28-touch-screen-tft-lcd-with-all-interface-p-63.html

I ordered one, then we will see if I also find time to port the driver
and test it.

> I'm not sure what the future of fbtft is. The idea was that the drivers
> should get cleaned up and move out of staging, but then fbdev was closed
> for new drivers and I did tinydrm. Only two drivers have been converted
> apart from mi0283qt that I did and that is hx8357 which Eric did and
> st7735 that David did. Those 3 covers a lot of the tiny SPI display
> marked, Adafruit sells them.
> It's a chicken and egg problem, as long as the fbtft drivers are there
> and working, there's no incentive to convert them.
I follow the average joe user here. If it works then why worry.
But if I get HW and time I can at least port over a few of them.
It looks like it takes more time to test than to do the porting.

> There's another challenge with these drivers since it is possible to
> override the init sequence in Device Tree, meaning they can work with
> all kinds of displays without writing a new driver.
> I was not allowed to keep that functionality outside of staging.
> 
> When I'm done with the tinydrm cleanup, I'm going to work on an idea I
> have: turn the Raspberry Pi Zero into a $5 USB to
> HDMI/SDTV/DSI/DPI/SPI-display adapter. I'm planning to write a generic
> USB host display driver with a matching Linux OTG device driver.
> I plan to make it easy to do the display OTG side on a microcontroller
> as well. This way it will be possible for manufacturers to do USB
> connected displays of (nearly) all sizes without having to write a Linux
> driver.
It will be interesting to follow this, keep us posted.

> It's difficult to predict the future, but powerful microcontrollers are
> cheap nowadays so maybe these SPI displays will be fased out by cheap
> USB displays. The uC can replace the touch controller cutting some of
> the uC cost.

Yep, it is impressive what one can get for USD 5 these days.

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

^ permalink raw reply	[flat|nested] 33+ messages in thread
* Re: [PATCH 01/10] drm: Add SPI connector type
  2019-07-17 11:58 ` [PATCH 01/10] drm: Add SPI connector type Noralf Trønnes
@ 2019-07-17 19:24   ` David Lechner
  2019-07-19  9:17   ` Daniel Vetter
  1 sibling, 0 replies; 33+ messages in thread
From: David Lechner @ 2019-07-17 19:24 UTC (permalink / raw)
  To: Noralf Trønnes, dri-devel

On 7/17/19 6:58 AM, Noralf Trønnes wrote:
> tinydrm drivers announce DRM_MODE_CONNECTOR_VIRTUAL for its SPI drivers.
> Stop lying and add a SPI connector type
> 
> Cc: David Lechner <david@lechnology.com>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---

Acked-by: David Lechner <david@lechnology.com>

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

^ permalink raw reply	[flat|nested] 33+ messages in thread
* Re: [PATCH 05/10] drm/tinydrm: Clean up tinydrm_spi_transfer()
  2019-07-17 11:58 ` [PATCH 05/10] drm/tinydrm: Clean up tinydrm_spi_transfer() Noralf Trønnes
  2019-07-17 13:09   ` Sam Ravnborg
@ 2019-07-17 19:37   ` David Lechner
  1 sibling, 0 replies; 33+ messages in thread
From: David Lechner @ 2019-07-17 19:37 UTC (permalink / raw)
  To: Noralf Trønnes, dri-devel

On 7/17/19 6:58 AM, Noralf Trønnes wrote:
> 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>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---

Acked-by: : David Lechner <david@lechnology.com>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 33+ messages in thread
* Re: [PATCH 06/10] drm/tinydrm: Move tinydrm_spi_transfer()
  2019-07-17 11:58 ` [PATCH 06/10] drm/tinydrm: Move tinydrm_spi_transfer() Noralf Trønnes
  2019-07-17 13:15   ` Sam Ravnborg
@ 2019-07-17 19:48   ` David Lechner
  2019-07-18 12:14     ` Noralf Trønnes
  1 sibling, 1 reply; 33+ messages in thread
From: David Lechner @ 2019-07-17 19:48 UTC (permalink / raw)
  To: Noralf Trønnes, dri-devel

On 7/17/19 6:58 AM, Noralf Trønnes wrote:
> 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.
> 
> Additionally move the mipi_dbi_spi_init() declaration to the other SPI
> functions.
> 
> Cc: David Lechner <david@lechnology.com>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---

Acked-by: : David Lechner <david@lechnology.com>

I assume that the comments here might have something to do with the
issue[1] I raised a while back? Should I dust that patch off and resend
it after this series lands?

[1]: https://lore.kernel.org/lkml/1519082461-32405-1-git-send-email-david@lechnology.com/
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 33+ messages in thread
* Re: [PATCH 07/10] drm/tinydrm: Move tinydrm_machine_little_endian()
  2019-07-17 11:58 ` [PATCH 07/10] drm/tinydrm: Move tinydrm_machine_little_endian() Noralf Trønnes
@ 2019-07-17 20:09   ` David Lechner
  2019-07-18 12:20     ` Noralf Trønnes
  0 siblings, 1 reply; 33+ messages in thread
From: David Lechner @ 2019-07-17 20:09 UTC (permalink / raw)
  To: Noralf Trønnes, dri-devel

On 7/17/19 6:58 AM, Noralf Trønnes wrote:
> The tinydrm helper is going away so move it into the only user mipi-dbi.
> 
> 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
> +}
> +

I'm kind of surprised that there isn't something like this elsewhere
in the kernel already. The way this function is being used it kind of
seems like it should be static __always_inline (or a macro) so that
the compiler can do a better job optimizing the code (although it is
a very minor improvement).
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 33+ messages in thread
* Re: [PATCH 09/10] drm/tinydrm/mipi-dbi: Add mipi_dbi_init_with_formats()
  2019-07-17 11:58 ` [PATCH 09/10] drm/tinydrm/mipi-dbi: Add mipi_dbi_init_with_formats() Noralf Trønnes
@ 2019-07-17 20:38   ` David Lechner
  0 siblings, 0 replies; 33+ messages in thread
From: David Lechner @ 2019-07-17 20:38 UTC (permalink / raw)
  To: Noralf Trønnes, dri-devel

On 7/17/19 6:58 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.

Although that might not always be the case. FYI, we are finding that
having XRGB8888 for a 2bpp grayscale display is not the greatest. When
we want to do direct drawing on the screen, we haven't found very good
support in existing graphics libraries for embedded systems for this
format. If I had more free time, I would like to look at adding
grayscale support to DRM.

> 
> Cc: David Lechner <david@lechnology.com>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---

Acked-by: David Lechner <david@lechnology.com>

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

^ permalink raw reply	[flat|nested] 33+ messages in thread
* Re: [PATCH 10/10] drm/tinydrm: Move tinydrm_display_pipe_init() to mipi-dbi
  2019-07-17 11:58 ` [PATCH 10/10] drm/tinydrm: Move tinydrm_display_pipe_init() to mipi-dbi Noralf Trønnes
  2019-07-17 13:34   ` Sam Ravnborg
@ 2019-07-17 20:46   ` David Lechner
  2019-07-18 12:27     ` Noralf Trønnes
  1 sibling, 1 reply; 33+ messages in thread
From: David Lechner @ 2019-07-17 20:46 UTC (permalink / raw)
  To: Noralf Trønnes, dri-devel

On 7/17/19 6:58 AM, Noralf Trønnes wrote:
> 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.

Were the uses of tinydrm-helpers.h removed in this series? If so, then th
#include should probably be removed at the same time. If not, removing the
#include lines could just be an independent patch from this series.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 33+ messages in thread
* Re: [PATCH 06/10] drm/tinydrm: Move tinydrm_spi_transfer()
  2019-07-17 19:48   ` David Lechner
@ 2019-07-18 12:14     ` Noralf Trønnes
  2019-07-25 14:16       ` Noralf Trønnes
  0 siblings, 1 reply; 33+ messages in thread
From: Noralf Trønnes @ 2019-07-18 12:14 UTC (permalink / raw)
  To: David Lechner, dri-devel



Den 17.07.2019 21.48, skrev David Lechner:
> On 7/17/19 6:58 AM, Noralf Trønnes wrote:
>> 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.
>>
>> Additionally move the mipi_dbi_spi_init() declaration to the other SPI
>> functions.
>>
>> Cc: David Lechner <david@lechnology.com>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>> ---
> 
> Acked-by: : David Lechner <david@lechnology.com>
> 
> I assume that the comments here might have something to do with the
> issue[1] I raised a while back? Should I dust that patch off and resend
> it after this series lands?
> 
> [1]:
> https://lore.kernel.org/lkml/1519082461-32405-1-git-send-email-david@lechnology.com/
> 

Yep, that's the one. I want to refactor mipi-dbi first splitting struct
mipi_dbi into an interface and display pipeline part. The helper is
going to be moved to drivers/gpu/drm with the other helpers.
Please wait until that is done, I want to see what kind of coupling I
end up between the two structs and don't want another dependency to deal
with if I can avoid it.

Noralf.

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

^ permalink raw reply	[flat|nested] 33+ messages in thread
* Re: [PATCH 07/10] drm/tinydrm: Move tinydrm_machine_little_endian()
  2019-07-17 20:09   ` David Lechner
@ 2019-07-18 12:20     ` Noralf Trønnes
  0 siblings, 0 replies; 33+ messages in thread
From: Noralf Trønnes @ 2019-07-18 12:20 UTC (permalink / raw)
  To: David Lechner, dri-devel



Den 17.07.2019 22.09, skrev David Lechner:
> On 7/17/19 6:58 AM, Noralf Trønnes wrote:
>> The tinydrm helper is going away so move it into the only user mipi-dbi.
>>
>> 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
>> +}
>> +
> 
> I'm kind of surprised that there isn't something like this elsewhere
> in the kernel already. The way this function is being used it kind of
> seems like it should be static __always_inline (or a macro) so that
> the compiler can do a better job optimizing the code (although it is
> a very minor improvement).

Ideally this should be in the core somewhere, but I don't want to spend
more time on refactoring. I have a usb driver that I want to write and
I've waited nearly 2 years now. I got sucked into a giant refactoring
hole :-)

Doing a quick scan I found virtio_legacy_is_little_endian() which does
the same, but it's clearly virtio related and I don't want to drag in that.

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

^ permalink raw reply	[flat|nested] 33+ messages in thread
* Re: [PATCH 10/10] drm/tinydrm: Move tinydrm_display_pipe_init() to mipi-dbi
  2019-07-17 20:46   ` David Lechner
@ 2019-07-18 12:27     ` Noralf Trønnes
  0 siblings, 0 replies; 33+ messages in thread
From: Noralf Trønnes @ 2019-07-18 12:27 UTC (permalink / raw)
  To: David Lechner, dri-devel



Den 17.07.2019 22.46, skrev David Lechner:
> On 7/17/19 6:58 AM, Noralf Trønnes wrote:
>> 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.
> 
> Were the uses of tinydrm-helpers.h removed in this series? If so, then th
> #include should probably be removed at the same time. If not, removing the
> #include lines could just be an independent patch from this series.

tinydrm-helpers.h has only one function declaration that is remove in
this patch, so the file is deleted. That's why I need to remove the
#include's.

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

^ permalink raw reply	[flat|nested] 33+ messages in thread
* Re: [PATCH 01/10] drm: Add SPI connector type
  2019-07-17 11:58 ` [PATCH 01/10] drm: Add SPI connector type Noralf Trønnes
  2019-07-17 19:24   ` David Lechner
@ 2019-07-19  9:17   ` Daniel Vetter
  2019-07-19 12:34     ` Noralf Trønnes
  1 sibling, 1 reply; 33+ messages in thread
From: Daniel Vetter @ 2019-07-19  9:17 UTC (permalink / raw)
  To: Noralf Trønnes; +Cc: david, dri-devel

On Wed, Jul 17, 2019 at 01:58:08PM +0200, Noralf Trønnes wrote:
> tinydrm drivers announce DRM_MODE_CONNECTOR_VIRTUAL for its SPI drivers.
> Stop lying and add a SPI connector type
> 
> Cc: David Lechner <david@lechnology.com>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>

Note this will break X (and probably a pile of others), which is why we
tried hard to avoid adding new connector types. Or did this all get fixed
by now?
-Daniel

> ---
>  drivers/gpu/drm/drm_connector.c    | 1 +
>  drivers/gpu/drm/tinydrm/mipi-dbi.c | 3 +--
>  drivers/gpu/drm/tinydrm/repaper.c  | 2 +-
>  drivers/gpu/drm/tinydrm/st7586.c   | 2 +-
>  include/uapi/drm/drm_mode.h        | 1 +
>  5 files changed, 5 insertions(+), 4 deletions(-)
> 
> 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/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)
> 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 {
>  
> -- 
> 2.20.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 33+ messages in thread
* Re: [PATCH 01/10] drm: Add SPI connector type
  2019-07-19  9:17   ` Daniel Vetter
@ 2019-07-19 12:34     ` Noralf Trønnes
  2019-07-19 12:39       ` Daniel Vetter
  0 siblings, 1 reply; 33+ messages in thread
From: Noralf Trønnes @ 2019-07-19 12:34 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: david, dri-devel



Den 19.07.2019 11.17, skrev Daniel Vetter:
> On Wed, Jul 17, 2019 at 01:58:08PM +0200, Noralf Trønnes wrote:
>> tinydrm drivers announce DRM_MODE_CONNECTOR_VIRTUAL for its SPI drivers.
>> Stop lying and add a SPI connector type
>>
>> Cc: David Lechner <david@lechnology.com>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> 
> Note this will break X (and probably a pile of others), which is why we
> tried hard to avoid adding new connector types. Or did this all get fixed
> by now?

X lists it 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

Looking at the weston source it will list it as UNNAMED.

This is not important for me to change, I'm just ticking off todo's.
I won't chase down userspace to fix this up, but I assume that it will
trickle in eventually.

I can drop this if you're not to happy about it.

Noralf.

> -Daniel
> 
>> ---
>>  drivers/gpu/drm/drm_connector.c    | 1 +
>>  drivers/gpu/drm/tinydrm/mipi-dbi.c | 3 +--
>>  drivers/gpu/drm/tinydrm/repaper.c  | 2 +-
>>  drivers/gpu/drm/tinydrm/st7586.c   | 2 +-
>>  include/uapi/drm/drm_mode.h        | 1 +
>>  5 files changed, 5 insertions(+), 4 deletions(-)
>>
>> 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/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)
>> 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 {
>>  
>> -- 
>> 2.20.1
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 33+ messages in thread
* Re: [PATCH 01/10] drm: Add SPI connector type
  2019-07-19 12:34     ` Noralf Trønnes
@ 2019-07-19 12:39       ` Daniel Vetter
  0 siblings, 0 replies; 33+ messages in thread
From: Daniel Vetter @ 2019-07-19 12:39 UTC (permalink / raw)
  To: Noralf Trønnes; +Cc: David Lechner, dri-devel

On Fri, Jul 19, 2019 at 2:34 PM Noralf Trønnes <noralf@tronnes.org> wrote:
>
>
>
> Den 19.07.2019 11.17, skrev Daniel Vetter:
> > On Wed, Jul 17, 2019 at 01:58:08PM +0200, Noralf Trønnes wrote:
> >> tinydrm drivers announce DRM_MODE_CONNECTOR_VIRTUAL for its SPI drivers.
> >> Stop lying and add a SPI connector type
> >>
> >> Cc: David Lechner <david@lechnology.com>
> >> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> >
> > Note this will break X (and probably a pile of others), which is why we
> > tried hard to avoid adding new connector types. Or did this all get fixed
> > by now?
>
> X lists it 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
>
> Looking at the weston source it will list it as UNNAMED.
>
> This is not important for me to change, I'm just ticking off todo's.
> I won't chase down userspace to fix this up, but I assume that it will
> trickle in eventually.
>
> I can drop this if you're not to happy about it.

I'm totally fine with UNNAMED/unknown, that's not worse or better than
virtual. I was only worried about userspace blowing up because e.g.
array deref for an element that doesn't exist. If you add the above to
the commit message this is imo all fine.

Also maybe split out the addition of the core bits into a separate
prep patch would be good.
-Daniel

>
> Noralf.
>
> > -Daniel
> >
> >> ---
> >>  drivers/gpu/drm/drm_connector.c    | 1 +
> >>  drivers/gpu/drm/tinydrm/mipi-dbi.c | 3 +--
> >>  drivers/gpu/drm/tinydrm/repaper.c  | 2 +-
> >>  drivers/gpu/drm/tinydrm/st7586.c   | 2 +-
> >>  include/uapi/drm/drm_mode.h        | 1 +
> >>  5 files changed, 5 insertions(+), 4 deletions(-)
> >>
> >> 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/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)
> >> 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 {
> >>
> >> --
> >> 2.20.1
> >>
> >> _______________________________________________
> >> dri-devel mailing list
> >> dri-devel@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 33+ messages in thread
* Re: [PATCH 06/10] drm/tinydrm: Move tinydrm_spi_transfer()
  2019-07-18 12:14     ` Noralf Trønnes
@ 2019-07-25 14:16       ` Noralf Trønnes
  2019-07-25 14:29         ` David Lechner
  0 siblings, 1 reply; 33+ messages in thread
From: Noralf Trønnes @ 2019-07-25 14:16 UTC (permalink / raw)
  To: David Lechner, dri-devel



Den 18.07.2019 14.14, skrev Noralf Trønnes:
> 
> 
> Den 17.07.2019 21.48, skrev David Lechner:
>> On 7/17/19 6:58 AM, Noralf Trønnes wrote:
>>> 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.
>>>
>>> Additionally move the mipi_dbi_spi_init() declaration to the other SPI
>>> functions.
>>>
>>> Cc: David Lechner <david@lechnology.com>
>>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>>> ---
>>
>> Acked-by: : David Lechner <david@lechnology.com>
>>
>> I assume that the comments here might have something to do with the
>> issue[1] I raised a while back? Should I dust that patch off and resend
>> it after this series lands?
>>
>> [1]:
>> https://lore.kernel.org/lkml/1519082461-32405-1-git-send-email-david@lechnology.com/
>>
> 
> Yep, that's the one. I want to refactor mipi-dbi first splitting struct
> mipi_dbi into an interface and display pipeline part. The helper is
> going to be moved to drivers/gpu/drm with the other helpers.
> Please wait until that is done, I want to see what kind of coupling I
> end up between the two structs and don't want another dependency to deal
> with if I can avoid it.
> 

I've applied the series now.

Do you have this problem only on the EV3 and with only one
display/driver? If so I'm wondering if there's a way to implement this
that doesn't affect the other drivers since you need a special use case
to be hit by this.

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

^ permalink raw reply	[flat|nested] 33+ messages in thread
* Re: [PATCH 06/10] drm/tinydrm: Move tinydrm_spi_transfer()
  2019-07-25 14:16       ` Noralf Trønnes
@ 2019-07-25 14:29         ` David Lechner
  0 siblings, 0 replies; 33+ messages in thread
From: David Lechner @ 2019-07-25 14:29 UTC (permalink / raw)
  To: Noralf Trønnes, dri-devel

On 7/25/19 9:16 AM, Noralf Trønnes wrote:
> 
> 
> Den 18.07.2019 14.14, skrev Noralf Trønnes:
>>
>>
>> Den 17.07.2019 21.48, skrev David Lechner:
>>> On 7/17/19 6:58 AM, Noralf Trønnes wrote:
>>>> 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.
>>>>
>>>> Additionally move the mipi_dbi_spi_init() declaration to the other SPI
>>>> functions.
>>>>
>>>> Cc: David Lechner <david@lechnology.com>
>>>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>>>> ---
>>>
>>> Acked-by: : David Lechner <david@lechnology.com>
>>>
>>> I assume that the comments here might have something to do with the
>>> issue[1] I raised a while back? Should I dust that patch off and resend
>>> it after this series lands?
>>>
>>> [1]:
>>> https://lore.kernel.org/lkml/1519082461-32405-1-git-send-email-david@lechnology.com/
>>>
>>
>> Yep, that's the one. I want to refactor mipi-dbi first splitting struct
>> mipi_dbi into an interface and display pipeline part. The helper is
>> going to be moved to drivers/gpu/drm with the other helpers.
>> Please wait until that is done, I want to see what kind of coupling I
>> end up between the two structs and don't want another dependency to deal
>> with if I can avoid it.
>>
> 
> I've applied the series now.
> 
> Do you have this problem only on the EV3 and with only one
> display/driver? If so I'm wondering if there's a way to implement this
> that doesn't affect the other drivers since you need a special use case
> to be hit by this.
> 
> Noralf.
> 

I've let this sit running for several days and I don't see the error
any more. So perhaps something was fixed in the DMA driver? Or maybe
I just haven't seen the error because it is sitting idle and not under
heavy memory usage? (I just ran some apt commands that crashed because
of OOM and nothing happened with the display driver.)

Interestingly, I was getting a similar allocation error from zram
in the 5.2 kernel, usually triggered by starting a SSH session (totally
unrelated to DRM).

So, unless I start seeing the problem again, I think we can leave it
alone for now.

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

^ permalink raw reply	[flat|nested] 33+ messages in thread
end of thread, other threads:[~2019-07-25 14:29 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-17 11:58 [PATCH 00/10] drm/tinydrm: Remove tinydrm.ko Noralf Trønnes
2019-07-17 11:58 ` [PATCH 01/10] drm: Add SPI connector type Noralf Trønnes
2019-07-17 19:24   ` David Lechner
2019-07-19  9:17   ` Daniel Vetter
2019-07-19 12:34     ` Noralf Trønnes
2019-07-19 12:39       ` Daniel Vetter
2019-07-17 11:58 ` [PATCH 02/10] drm/tinydrm: Use spi_is_bpw_supported() Noralf Trønnes
2019-07-17 11:58 ` [PATCH 03/10] drm/tinydrm: Remove spi debug buffer dumping Noralf Trønnes
2019-07-17 11:58 ` [PATCH 04/10] drm/tinydrm: Remove tinydrm_spi_max_transfer_size() Noralf Trønnes
2019-07-17 11:58 ` [PATCH 05/10] drm/tinydrm: Clean up tinydrm_spi_transfer() Noralf Trønnes
2019-07-17 13:09   ` Sam Ravnborg
2019-07-17 16:18     ` Noralf Trønnes
2019-07-17 18:13       ` Sam Ravnborg
2019-07-17 19:37   ` David Lechner
2019-07-17 11:58 ` [PATCH 06/10] drm/tinydrm: Move tinydrm_spi_transfer() Noralf Trønnes
2019-07-17 13:15   ` Sam Ravnborg
2019-07-17 16:20     ` Noralf Trønnes
2019-07-17 19:48   ` David Lechner
2019-07-18 12:14     ` Noralf Trønnes
2019-07-25 14:16       ` Noralf Trønnes
2019-07-25 14:29         ` David Lechner
2019-07-17 11:58 ` [PATCH 07/10] drm/tinydrm: Move tinydrm_machine_little_endian() Noralf Trønnes
2019-07-17 20:09   ` David Lechner
2019-07-18 12:20     ` Noralf Trønnes
2019-07-17 11:58 ` [PATCH 08/10] drm/tinydrm/repaper: Don't use tinydrm_display_pipe_init() Noralf Trønnes
2019-07-17 11:58 ` [PATCH 09/10] drm/tinydrm/mipi-dbi: Add mipi_dbi_init_with_formats() Noralf Trønnes
2019-07-17 20:38   ` David Lechner
2019-07-17 11:58 ` [PATCH 10/10] drm/tinydrm: Move tinydrm_display_pipe_init() to mipi-dbi Noralf Trønnes
2019-07-17 13:34   ` Sam Ravnborg
2019-07-17 20:46   ` David Lechner
2019-07-18 12:27     ` Noralf Trønnes
2019-07-17 13:31 ` [PATCH 00/10] drm/tinydrm: Remove tinydrm.ko Sam Ravnborg
2019-07-17 16:22   ` Noralf Trønnes

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.