[PATCH 0/2] drm/tiny: Add driver for the sharp LS027B7DH01 Memory LCD

[PATCH 0/2] drm/tiny: Add driver for the sharp LS027B7DH01 Memory LCD

[Mehdi Djait]

This patch series adds a DRM driver for the sharp LS027B7DH01 memory display:
a 2.7" 400x240 monochrome 1 bit per pixel display SPI device.

This controller uses SPI both for control and for pixel data. Pixel data can be
sent either as one line per SPI frame or multiple lines (up to the entire picture)
in a single SPI frame. This driver implements the latter.

The Sharp Memory LCD requires an alternating signal to prevent the buildup of
a DC bias that would result in a Display that no longer can be updated. Two
modes for the generation of this signal are supported:

- Software, EXTMODE = Low: toggling the BIT(1) of the Command and writing it at
  least once a second to the display.

- Hardware, EXTMODE = High: the alternating signal should be supplied on the
  EXTCOMIN pin.

the Hardware mode is implemented with a PWM signal.

The driver announces the commonly supported XRGB8888 to userspace and
uses the drm_fb_xrgb8888_to_mono() function to convert the format.

Mehdi Djait (2):
  dt-bindings: display: Add Sharp LS027B7DH01 Memory LCD
  drm/tiny: Add driver for the sharp LS027B7DH01 Memory LCD

 .../bindings/display/sharp,ls027b7dh01.yaml   |  71 +++
 MAINTAINERS                                   |   7 +
 drivers/gpu/drm/tiny/Kconfig                  |   8 +
 drivers/gpu/drm/tiny/Makefile                 |   1 +
 drivers/gpu/drm/tiny/sharp-ls027b7dh01.c      | 411 ++++++++++++++++++
 5 files changed, 498 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/sharp,ls027b7dh01.yaml
 create mode 100644 drivers/gpu/drm/tiny/sharp-ls027b7dh01.c

-- 
2.41.0

[PATCH 0/2] drm/tiny: Add driver for the sharp LS027B7DH01 Memory LCD

[Mehdi Djait]

This patch series adds a DRM driver for the sharp LS027B7DH01 memory display:
a 2.7" 400x240 monochrome 1 bit per pixel display SPI device.

This controller uses SPI both for control and for pixel data. Pixel data can be
sent either as one line per SPI frame or multiple lines (up to the entire picture)
in a single SPI frame. This driver implements the latter.

The Sharp Memory LCD requires an alternating signal to prevent the buildup of
a DC bias that would result in a Display that no longer can be updated. Two
modes for the generation of this signal are supported:

- Software, EXTMODE = Low: toggling the BIT(1) of the Command and writing it at
  least once a second to the display.

- Hardware, EXTMODE = High: the alternating signal should be supplied on the
  EXTCOMIN pin.

the Hardware mode is implemented with a PWM signal.

The driver announces the commonly supported XRGB8888 to userspace and
uses the drm_fb_xrgb8888_to_mono() function to convert the format.

Mehdi Djait (2):
  dt-bindings: display: Add Sharp LS027B7DH01 Memory LCD
  drm/tiny: Add driver for the sharp LS027B7DH01 Memory LCD

 .../bindings/display/sharp,ls027b7dh01.yaml   |  71 +++
 MAINTAINERS                                   |   7 +
 drivers/gpu/drm/tiny/Kconfig                  |   8 +
 drivers/gpu/drm/tiny/Makefile                 |   1 +
 drivers/gpu/drm/tiny/sharp-ls027b7dh01.c      | 411 ++++++++++++++++++
 5 files changed, 498 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/sharp,ls027b7dh01.yaml
 create mode 100644 drivers/gpu/drm/tiny/sharp-ls027b7dh01.c

-- 
2.41.0

[PATCH 1/2] dt-bindings: display: Add Sharp LS027B7DH01 Memory LCD

[Mehdi Djait]

Add device tree bindings for the Sharp LS027B7DH01: a 2.7" 400x240
monochrome display connected over SPI.

Signed-off-by: Mehdi Djait <mehdi.djait@bootlin.com>
---
 .../bindings/display/sharp,ls027b7dh01.yaml   | 71 +++++++++++++++++++
 1 file changed, 71 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/sharp,ls027b7dh01.yaml

diff --git a/Documentation/devicetree/bindings/display/sharp,ls027b7dh01.yaml b/Documentation/devicetree/bindings/display/sharp,ls027b7dh01.yaml
new file mode 100644
index 000000000000..d0a4efae2827
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/sharp,ls027b7dh01.yaml
@@ -0,0 +1,71 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/sharp,ls027b7dh01.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sharp LS027B7DH01 Memory LCD Display
+
+maintainers:
+  - Mehdi Djait <mehdi.djait@bootlin.com>
+
+description:
+  The Sharp LS027B7DH01 is a 2.7" 400x240 monochrome display connected over a
+  SPI bus. The display requires an alternating signal to prevent the buildup of
+  a DC bias that will stop any update. Two modes can be used for the generation
+  of this signal Software by writing to the display or Hardware by supplying the
+  External COM inversion signal.
+
+allOf:
+  - $ref: panel/panel-common.yaml#
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+  compatible:
+    const: sharp,ls027b7dh01
+
+  reg:
+    maxItems: 1
+
+  spi-cs-high: true
+
+  spi-lsb-first: true
+
+  spi-max-frequency:
+    maximum: 2000000
+
+  enable-gpios:
+    maxItems: 1
+
+  pwms:
+    maxItems: 1
+    description: External COM inversion signal
+
+required:
+  - compatible
+  - reg
+  - spi-lsb-first
+  - enable-gpios
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    spi {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            display@0{
+                    compatible = "sharp,ls027b7dh01";
+                    reg = <0>;
+                    spi-cs-high;
+                    spi-lsb-first;
+                    spi-max-frequency = <2000000>;
+                    enable-gpios = <&gpiof 3 GPIO_ACTIVE_HIGH>;
+                    pwms = <&pwm 1 1000000000 0>;
+            };
+    };
+
+...
-- 
2.41.0

[PATCH 1/2] dt-bindings: display: Add Sharp LS027B7DH01 Memory LCD

[Mehdi Djait]

Add device tree bindings for the Sharp LS027B7DH01: a 2.7" 400x240
monochrome display connected over SPI.

Signed-off-by: Mehdi Djait <mehdi.djait@bootlin.com>
---
 .../bindings/display/sharp,ls027b7dh01.yaml   | 71 +++++++++++++++++++
 1 file changed, 71 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/sharp,ls027b7dh01.yaml

diff --git a/Documentation/devicetree/bindings/display/sharp,ls027b7dh01.yaml b/Documentation/devicetree/bindings/display/sharp,ls027b7dh01.yaml
new file mode 100644
index 000000000000..d0a4efae2827
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/sharp,ls027b7dh01.yaml
@@ -0,0 +1,71 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/sharp,ls027b7dh01.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sharp LS027B7DH01 Memory LCD Display
+
+maintainers:
+  - Mehdi Djait <mehdi.djait@bootlin.com>
+
+description:
+  The Sharp LS027B7DH01 is a 2.7" 400x240 monochrome display connected over a
+  SPI bus. The display requires an alternating signal to prevent the buildup of
+  a DC bias that will stop any update. Two modes can be used for the generation
+  of this signal Software by writing to the display or Hardware by supplying the
+  External COM inversion signal.
+
+allOf:
+  - $ref: panel/panel-common.yaml#
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+  compatible:
+    const: sharp,ls027b7dh01
+
+  reg:
+    maxItems: 1
+
+  spi-cs-high: true
+
+  spi-lsb-first: true
+
+  spi-max-frequency:
+    maximum: 2000000
+
+  enable-gpios:
+    maxItems: 1
+
+  pwms:
+    maxItems: 1
+    description: External COM inversion signal
+
+required:
+  - compatible
+  - reg
+  - spi-lsb-first
+  - enable-gpios
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    spi {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            display@0{
+                    compatible = "sharp,ls027b7dh01";
+                    reg = <0>;
+                    spi-cs-high;
+                    spi-lsb-first;
+                    spi-max-frequency = <2000000>;
+                    enable-gpios = <&gpiof 3 GPIO_ACTIVE_HIGH>;
+                    pwms = <&pwm 1 1000000000 0>;
+            };
+    };
+
+...
-- 
2.41.0

[PATCH 2/2] drm/tiny: Add driver for the sharp LS027B7DH01 Memory LCD

[Mehdi Djait]

Introduce a DRM driver for the sharp LS027B7DH01 Memory LCD.

LS027B7DH01 is a 2.7" 400x240 monochrome display connected to a SPI bus.
The drivers implements the Multiple Lines Data Update Mode.
External COM inversion is enabled using a PWM signal as input.

Signed-off-by: Mehdi Djait <mehdi.djait@bootlin.com>
---
 MAINTAINERS                              |   7 +
 drivers/gpu/drm/tiny/Kconfig             |   8 +
 drivers/gpu/drm/tiny/Makefile            |   1 +
 drivers/gpu/drm/tiny/sharp-ls027b7dh01.c | 411 +++++++++++++++++++++++
 4 files changed, 427 insertions(+)
 create mode 100644 drivers/gpu/drm/tiny/sharp-ls027b7dh01.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 012df8ccf34e..fb859698bd3d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6832,6 +6832,13 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/display/panel/samsung,s6d7aa0.yaml
 F:	drivers/gpu/drm/panel/panel-samsung-s6d7aa0.c
 
+DRM DRIVER FOR SHARP LS027B7DH01 Memory LCD
+M:	Mehdi Djait <mehdi.djait@bootlin.com>
+S:	Maintained
+T:	git git://anongit.freedesktop.org/drm/drm-misc
+F:	Documentation/devicetree/bindings/display/sharp,ls027b7dh01.yaml
+F:	drivers/gpu/drm/tiny/sharp-ls027b7dh01.c
+
 DRM DRIVER FOR SITRONIX ST7586 PANELS
 M:	David Lechner <david@lechnology.com>
 S:	Maintained
diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig
index f6889f649bc1..a2ade06403ca 100644
--- a/drivers/gpu/drm/tiny/Kconfig
+++ b/drivers/gpu/drm/tiny/Kconfig
@@ -186,6 +186,14 @@ config TINYDRM_REPAPER
 
 	  If M is selected the module will be called repaper.
 
+config TINYDRM_SHARP_LS027B7DH01
+	tristate "DRM support for SHARP LS027B7DH01 display"
+	depends on DRM && SPI
+	select DRM_KMS_HELPER
+	select DRM_GEM_DMA_HELPER
+	help
+	  DRM driver for the SHARP LS027B7DD01 LCD display.
+
 config TINYDRM_ST7586
 	tristate "DRM support for Sitronix ST7586 display panels"
 	depends on DRM && SPI
diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile
index 76dde89a044b..b05df3afb231 100644
--- a/drivers/gpu/drm/tiny/Makefile
+++ b/drivers/gpu/drm/tiny/Makefile
@@ -14,5 +14,6 @@ obj-$(CONFIG_TINYDRM_ILI9341)		+= ili9341.o
 obj-$(CONFIG_TINYDRM_ILI9486)		+= ili9486.o
 obj-$(CONFIG_TINYDRM_MI0283QT)		+= mi0283qt.o
 obj-$(CONFIG_TINYDRM_REPAPER)		+= repaper.o
+obj-$(CONFIG_TINYDRM_SHARP_LS027B7DH01)	+= sharp-ls027b7dh01.o
 obj-$(CONFIG_TINYDRM_ST7586)		+= st7586.o
 obj-$(CONFIG_TINYDRM_ST7735R)		+= st7735r.o
diff --git a/drivers/gpu/drm/tiny/sharp-ls027b7dh01.c b/drivers/gpu/drm/tiny/sharp-ls027b7dh01.c
new file mode 100644
index 000000000000..2f58865a5c78
--- /dev/null
+++ b/drivers/gpu/drm/tiny/sharp-ls027b7dh01.c
@@ -0,0 +1,411 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Sharp LS027B7DH01 Memory Display Driver
+ *
+ * Copyright (C) 2023 Andrew D'Angelo
+ * Copyright (C) 2023 Mehdi Djait <mehdi.djait@bootlin.com>
+ *
+ * The Sharp Memory LCD requires an alternating signal to prevent the buildup of
+ * a DC bias that would result in a Display that no longer can be updated. Two
+ * modes for the generation of this signal are supported:
+ *
+ * Software, EXTMODE = Low: toggling the BIT(1) of the Command and writing it at
+ * least once a second to the display.
+ *
+ * Hardware, EXTMODE = High: the alternating signal should be supplied on the
+ * EXTCOMIN pin.
+ *
+ * In this driver the Hardware mode is implemented with a PWM signal.
+ */
+
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/pwm.h>
+#include <linux/spi/spi.h>
+
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_damage_helper.h>
+#include <drm/drm_drv.h>
+#include <drm/drm_fbdev_generic.h>
+#include <drm/drm_fb_dma_helper.h>
+#include <drm/drm_format_helper.h>
+#include <drm/drm_framebuffer.h>
+#include <drm/drm_gem_atomic_helper.h>
+#include <drm/drm_gem_dma_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
+#include <drm/drm_modes.h>
+#include <drm/drm_probe_helper.h>
+#include <drm/drm_simple_kms_helper.h>
+
+#define CMD_WRITE BIT(0)
+#define CMD_CLEAR BIT(2)
+
+struct sharp_ls027b7dh01 {
+	struct spi_device *spi;
+
+	struct drm_device drm;
+	struct drm_connector connector;
+	struct drm_simple_display_pipe pipe;
+	const struct drm_display_mode *display_mode;
+
+	struct gpio_desc *enable_gpio;
+	struct pwm_device *extcomin_pwm;
+
+	u8 *write_buf;
+};
+
+static inline struct sharp_ls027b7dh01 *drm_to_priv(struct drm_device *drm)
+{
+	return container_of(drm, struct sharp_ls027b7dh01, drm);
+}
+
+/**
+ * sharp_ls027b7dh01_add_headers - Add the Sharp LS027B7DH01 specific headers
+ * @write_buf: Buffer to write
+ * @clip: DRM clip rectangle area to write
+ * @dst_pitch: Pitch of the write buffer
+ *
+ * This function adds the SHARP LS027B7DH01 specific headers to the buffer for
+ * the Multiple Lines Write Mode:
+ * - The first byte will contain the write command.
+ * - Every line data starts with the line number and ends with a dummy zero
+ *   trailer byte. It should be noted here that the line numbers are indexed
+ *   from 1.
+ *
+ * Returns the size of the buffer to write to the display.
+ */
+static size_t sharp_ls027b7dh01_add_headers(u8 *write_buf,
+					    const struct drm_rect *clip,
+					    const unsigned int dst_pitch)
+{
+	u8 line_num = clip->y1 + 1;
+	unsigned int lines = drm_rect_height(clip);
+	unsigned int y;
+
+	write_buf[0] = CMD_WRITE;
+	write_buf[1] = line_num++;
+
+	for (y = 1; y < lines; y++) {
+		write_buf[y * dst_pitch] = 0;
+		write_buf[y * dst_pitch + 1] = line_num++;
+	}
+
+	write_buf[lines * dst_pitch] = 0;
+	write_buf[lines * dst_pitch + 1] = 0;
+
+	return lines * dst_pitch + 2;
+}
+
+static int sharp_ls027b7dh01_prepare_buf(struct sharp_ls027b7dh01 *priv,
+					 u8 *write_buf,
+					 size_t *data_len,
+					 struct drm_framebuffer *fb,
+					 const struct drm_rect *clip)
+{
+	struct drm_gem_dma_object *dma_obj;
+	struct iosys_map dst, vmap;
+	unsigned int dst_pitch;
+	int ret;
+
+	/* Leave 2 bytes to hold the line number and the trailer dummy byte. */
+	dst_pitch = (drm_rect_width(clip) / 8) + 2;
+
+	dma_obj = drm_fb_dma_get_gem_obj(fb, 0);
+
+	ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
+	if (ret)
+		return ret;
+
+	iosys_map_set_vaddr(&dst, &write_buf[2]);
+	iosys_map_set_vaddr(&vmap, dma_obj->vaddr);
+
+	drm_fb_xrgb8888_to_mono(&dst, &dst_pitch, &vmap, fb, clip);
+
+	drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
+
+	*data_len = sharp_ls027b7dh01_add_headers(write_buf, clip, dst_pitch);
+
+	return 0;
+}
+
+static int sharp_ls027b7dh01_fb_damaged(struct drm_framebuffer *fb,
+					const struct drm_rect *rect)
+{
+	struct drm_rect clip;
+	struct sharp_ls027b7dh01 *priv;
+	size_t data_len;
+	int drm_index;
+	int ret;
+
+	clip.x1 = 0;
+	clip.x2 = fb->width;
+	clip.y1 = rect->y1;
+	clip.y2 = rect->y2;
+
+	priv = drm_to_priv(fb->dev);
+
+	if (!drm_dev_enter(fb->dev, &drm_index))
+		return -ENODEV;
+
+	ret = sharp_ls027b7dh01_prepare_buf(priv, priv->write_buf, &data_len, fb, &clip);
+	if (ret)
+		goto exit;
+
+	ret = spi_write(priv->spi, priv->write_buf, data_len);
+
+exit:
+	drm_dev_exit(drm_index);
+
+	return ret;
+}
+
+static void sharp_ls027b7dh01_pipe_update(struct drm_simple_display_pipe *pipe,
+					  struct drm_plane_state *old_state)
+{
+	struct drm_plane_state *state = pipe->plane.state;
+	struct drm_rect rect;
+
+	if (!pipe->crtc.state->active)
+		return;
+
+	if (drm_atomic_helper_damage_merged(old_state, state, &rect))
+		sharp_ls027b7dh01_fb_damaged(state->fb, &rect);
+}
+
+static void sharp_ls027b7dh01_pipe_disable(struct drm_simple_display_pipe *pipe)
+{
+	struct sharp_ls027b7dh01 *priv;
+
+	priv = drm_to_priv(pipe->crtc.dev);
+	gpiod_set_value(priv->enable_gpio, 0);
+}
+
+static int sharp_ls027b7dh01_clear_display(struct sharp_ls027b7dh01 *priv)
+{
+	u8 clear_buf[2] = { CMD_CLEAR, 0 };
+
+	return spi_write(priv->spi, clear_buf, sizeof(clear_buf));
+}
+
+static int sharp_ls027b7dh01_pwm_enable(struct sharp_ls027b7dh01 *priv)
+{
+	struct device *dev = &priv->spi->dev;
+	struct pwm_state pwmstate;
+
+	priv->extcomin_pwm = devm_pwm_get(dev, NULL);
+	if (IS_ERR(priv->extcomin_pwm)) {
+		dev_err(dev, "Could not get EXTCOMIN pwm\n");
+		return PTR_ERR(priv->extcomin_pwm);
+	}
+
+	pwm_init_state(priv->extcomin_pwm, &pwmstate);
+	pwm_set_relative_duty_cycle(&pwmstate, 50, 100);
+	pwm_apply_state(priv->extcomin_pwm, &pwmstate);
+
+	pwm_enable(priv->extcomin_pwm);
+
+	return 0;
+}
+
+static void sharp_ls027b7dh01_pipe_enable(struct drm_simple_display_pipe *pipe,
+					  struct drm_crtc_state *crtc_state,
+					  struct drm_plane_state *plane_state)
+{
+	struct sharp_ls027b7dh01 *priv;
+	int ret, drm_idx;
+
+	priv = drm_to_priv(pipe->crtc.dev);
+
+	if (!drm_dev_enter(pipe->crtc.dev, &drm_idx))
+		return;
+
+	gpiod_set_value(priv->enable_gpio, 1);
+
+	ret = sharp_ls027b7dh01_clear_display(priv);
+	if (ret)
+		goto exit;
+
+	sharp_ls027b7dh01_pwm_enable(priv);
+
+exit:
+	drm_dev_exit(drm_idx);
+}
+
+static const struct drm_simple_display_pipe_funcs sharp_ls027b7dh01_pipe_funcs = {
+	.enable = sharp_ls027b7dh01_pipe_enable,
+	.disable = sharp_ls027b7dh01_pipe_disable,
+	.update = sharp_ls027b7dh01_pipe_update,
+};
+
+static int sharp_ls027b7dh01_connector_get_modes(struct drm_connector *connector)
+{
+	struct sharp_ls027b7dh01 *priv = drm_to_priv(connector->dev);
+
+	return drm_connector_helper_get_modes_fixed(connector, priv->display_mode);
+}
+
+static const struct drm_connector_helper_funcs sharp_ls027b7dh01_connector_hfuncs = {
+	.get_modes = sharp_ls027b7dh01_connector_get_modes,
+};
+
+static const struct drm_connector_funcs sharp_ls027b7dh01_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 sharp_ls027b7dh01_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 uint32_t sharp_ls027b7dh01_formats[] = {
+	DRM_FORMAT_XRGB8888,
+};
+
+static const struct drm_display_mode sharp_ls027b7dh01_mode = {
+	DRM_SIMPLE_MODE(400, 240, 59, 35),
+};
+
+DEFINE_DRM_GEM_DMA_FOPS(sharp_ls027b7dh01_fops);
+
+static const struct drm_driver sharp_ls027b7dh01_drm_driver = {
+	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
+	.fops			= &sharp_ls027b7dh01_fops,
+	DRM_GEM_DMA_DRIVER_OPS_VMAP,
+	.name			= "sharp_ls027b7dh01",
+	.desc			= "Sharp ls027b7dh01 Memory LCD",
+	.date			= "20231129",
+	.major			= 1,
+	.minor			= 0,
+};
+
+static int sharp_ls027b7dh01_probe(struct spi_device *spi)
+{
+	struct device *dev = &spi->dev;
+	struct sharp_ls027b7dh01 *priv;
+	struct drm_device *drm;
+	unsigned int write_buf_size;
+	int ret;
+
+	if (!dev->coherent_dma_mask) {
+		ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(32));
+		if (ret)
+			return dev_err_probe(dev, ret, "Failed to set dma mask\n");
+	}
+
+	priv = devm_drm_dev_alloc(dev, &sharp_ls027b7dh01_drm_driver,
+				  struct sharp_ls027b7dh01, drm);
+	if (IS_ERR(priv))
+		return PTR_ERR(priv);
+
+	spi_set_drvdata(spi, priv);
+	priv->spi = spi;
+
+	priv->enable_gpio = devm_gpiod_get(dev, "enable", GPIOD_OUT_HIGH);
+	if (IS_ERR(priv->enable_gpio))
+		return dev_err_probe(dev, PTR_ERR(priv->enable_gpio),
+				     "Failed to get GPIO 'enable'\n");
+
+	drm = &priv->drm;
+	ret = drmm_mode_config_init(drm);
+	if (ret)
+		return ret;
+
+	drm->mode_config.funcs = &sharp_ls027b7dh01_mode_config_funcs;
+	priv->display_mode = &sharp_ls027b7dh01_mode;
+
+	/*
+	 * write_buf_size:
+	 *
+	 * hdisplay * vdisplay / 8 => 1 bit per Pixel.
+	 * 2 * vdisplay => line number byte + trailer dummy byte for every line.
+	 * 2 => write command byte + final trailer dummy byte.
+	 */
+	write_buf_size = priv->display_mode->hdisplay * priv->display_mode->vdisplay / 8
+			 + 2 * priv->display_mode->vdisplay + 2;
+
+	priv->write_buf = devm_kzalloc(dev, write_buf_size, GFP_KERNEL);
+	if (!priv->write_buf)
+		return -ENOMEM;
+
+	drm->mode_config.min_width = priv->display_mode->hdisplay;
+	drm->mode_config.max_width = priv->display_mode->hdisplay;
+	drm->mode_config.min_height = priv->display_mode->vdisplay;
+	drm->mode_config.max_height = priv->display_mode->vdisplay;
+
+	ret = drm_connector_init(drm, &priv->connector,
+				 &sharp_ls027b7dh01_connector_funcs,
+				 DRM_MODE_CONNECTOR_SPI);
+	if (ret)
+		return ret;
+
+	drm_connector_helper_add(&priv->connector,
+				 &sharp_ls027b7dh01_connector_hfuncs);
+
+	ret = drm_simple_display_pipe_init(drm, &priv->pipe,
+					   &sharp_ls027b7dh01_pipe_funcs,
+					   sharp_ls027b7dh01_formats,
+					   ARRAY_SIZE(sharp_ls027b7dh01_formats),
+					   NULL, &priv->connector);
+	if (ret)
+		return ret;
+
+	drm_plane_enable_fb_damage_clips(&priv->pipe.plane);
+	drm_mode_config_reset(drm);
+
+	ret = drm_dev_register(drm, 0);
+	if (ret)
+		return ret;
+
+	drm_fbdev_generic_setup(drm, 0);
+
+	return 0;
+}
+
+static void sharp_ls027b7dh01_remove(struct spi_device *spi)
+{
+	struct sharp_ls027b7dh01 *priv = spi_get_drvdata(spi);
+
+	drm_dev_unplug(&priv->drm);
+	drm_atomic_helper_shutdown(&priv->drm);
+}
+
+static void sharp_ls027b7dh01_shutdown(struct spi_device *spi)
+{
+	struct sharp_ls027b7dh01 *priv = spi_get_drvdata(spi);
+
+	drm_atomic_helper_shutdown(&priv->drm);
+}
+
+static const struct spi_device_id sharp_ls027b7dh01_ids[] = {
+	{ "ls027b7dh01" },
+	{ },
+};
+MODULE_DEVICE_TABLE(spi, sharp_ls027b7dh01_ids);
+
+static const struct of_device_id sharp_ls027b7dh01_of_match[] = {
+	{ .compatible = "sharp,ls027b7dh01", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, sharp_ls027b7dh01_of_match);
+
+static struct spi_driver sharp_ls027b7dh01_spi_driver = {
+	.probe = sharp_ls027b7dh01_probe,
+	.remove = sharp_ls027b7dh01_remove,
+	.shutdown = sharp_ls027b7dh01_shutdown,
+	.id_table = sharp_ls027b7dh01_ids,
+	.driver = {
+		.name = "sharp-ls027b7dh01",
+		.of_match_table = sharp_ls027b7dh01_of_match,
+	},
+};
+module_spi_driver(sharp_ls027b7dh01_spi_driver);
+
+MODULE_AUTHOR("Andrew D'Angelo");
+MODULE_AUTHOR("Mehdi Djait <mehdi.djait@bootlin.com>");
+MODULE_DESCRIPTION("Sharp LS027B7DH01 Driver");
+MODULE_LICENSE("GPL");
-- 
2.41.0

[PATCH 2/2] drm/tiny: Add driver for the sharp LS027B7DH01 Memory LCD

[Mehdi Djait]

Introduce a DRM driver for the sharp LS027B7DH01 Memory LCD.

LS027B7DH01 is a 2.7" 400x240 monochrome display connected to a SPI bus.
The drivers implements the Multiple Lines Data Update Mode.
External COM inversion is enabled using a PWM signal as input.

Signed-off-by: Mehdi Djait <mehdi.djait@bootlin.com>
---
 MAINTAINERS                              |   7 +
 drivers/gpu/drm/tiny/Kconfig             |   8 +
 drivers/gpu/drm/tiny/Makefile            |   1 +
 drivers/gpu/drm/tiny/sharp-ls027b7dh01.c | 411 +++++++++++++++++++++++
 4 files changed, 427 insertions(+)
 create mode 100644 drivers/gpu/drm/tiny/sharp-ls027b7dh01.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 012df8ccf34e..fb859698bd3d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6832,6 +6832,13 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/display/panel/samsung,s6d7aa0.yaml
 F:	drivers/gpu/drm/panel/panel-samsung-s6d7aa0.c
 
+DRM DRIVER FOR SHARP LS027B7DH01 Memory LCD
+M:	Mehdi Djait <mehdi.djait@bootlin.com>
+S:	Maintained
+T:	git git://anongit.freedesktop.org/drm/drm-misc
+F:	Documentation/devicetree/bindings/display/sharp,ls027b7dh01.yaml
+F:	drivers/gpu/drm/tiny/sharp-ls027b7dh01.c
+
 DRM DRIVER FOR SITRONIX ST7586 PANELS
 M:	David Lechner <david@lechnology.com>
 S:	Maintained
diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig
index f6889f649bc1..a2ade06403ca 100644
--- a/drivers/gpu/drm/tiny/Kconfig
+++ b/drivers/gpu/drm/tiny/Kconfig
@@ -186,6 +186,14 @@ config TINYDRM_REPAPER
 
 	  If M is selected the module will be called repaper.
 
+config TINYDRM_SHARP_LS027B7DH01
+	tristate "DRM support for SHARP LS027B7DH01 display"
+	depends on DRM && SPI
+	select DRM_KMS_HELPER
+	select DRM_GEM_DMA_HELPER
+	help
+	  DRM driver for the SHARP LS027B7DD01 LCD display.
+
 config TINYDRM_ST7586
 	tristate "DRM support for Sitronix ST7586 display panels"
 	depends on DRM && SPI
diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile
index 76dde89a044b..b05df3afb231 100644
--- a/drivers/gpu/drm/tiny/Makefile
+++ b/drivers/gpu/drm/tiny/Makefile
@@ -14,5 +14,6 @@ obj-$(CONFIG_TINYDRM_ILI9341)		+= ili9341.o
 obj-$(CONFIG_TINYDRM_ILI9486)		+= ili9486.o
 obj-$(CONFIG_TINYDRM_MI0283QT)		+= mi0283qt.o
 obj-$(CONFIG_TINYDRM_REPAPER)		+= repaper.o
+obj-$(CONFIG_TINYDRM_SHARP_LS027B7DH01)	+= sharp-ls027b7dh01.o
 obj-$(CONFIG_TINYDRM_ST7586)		+= st7586.o
 obj-$(CONFIG_TINYDRM_ST7735R)		+= st7735r.o
diff --git a/drivers/gpu/drm/tiny/sharp-ls027b7dh01.c b/drivers/gpu/drm/tiny/sharp-ls027b7dh01.c
new file mode 100644
index 000000000000..2f58865a5c78
--- /dev/null
+++ b/drivers/gpu/drm/tiny/sharp-ls027b7dh01.c
@@ -0,0 +1,411 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Sharp LS027B7DH01 Memory Display Driver
+ *
+ * Copyright (C) 2023 Andrew D'Angelo
+ * Copyright (C) 2023 Mehdi Djait <mehdi.djait@bootlin.com>
+ *
+ * The Sharp Memory LCD requires an alternating signal to prevent the buildup of
+ * a DC bias that would result in a Display that no longer can be updated. Two
+ * modes for the generation of this signal are supported:
+ *
+ * Software, EXTMODE = Low: toggling the BIT(1) of the Command and writing it at
+ * least once a second to the display.
+ *
+ * Hardware, EXTMODE = High: the alternating signal should be supplied on the
+ * EXTCOMIN pin.
+ *
+ * In this driver the Hardware mode is implemented with a PWM signal.
+ */
+
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/pwm.h>
+#include <linux/spi/spi.h>
+
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_damage_helper.h>
+#include <drm/drm_drv.h>
+#include <drm/drm_fbdev_generic.h>
+#include <drm/drm_fb_dma_helper.h>
+#include <drm/drm_format_helper.h>
+#include <drm/drm_framebuffer.h>
+#include <drm/drm_gem_atomic_helper.h>
+#include <drm/drm_gem_dma_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
+#include <drm/drm_modes.h>
+#include <drm/drm_probe_helper.h>
+#include <drm/drm_simple_kms_helper.h>
+
+#define CMD_WRITE BIT(0)
+#define CMD_CLEAR BIT(2)
+
+struct sharp_ls027b7dh01 {
+	struct spi_device *spi;
+
+	struct drm_device drm;
+	struct drm_connector connector;
+	struct drm_simple_display_pipe pipe;
+	const struct drm_display_mode *display_mode;
+
+	struct gpio_desc *enable_gpio;
+	struct pwm_device *extcomin_pwm;
+
+	u8 *write_buf;
+};
+
+static inline struct sharp_ls027b7dh01 *drm_to_priv(struct drm_device *drm)
+{
+	return container_of(drm, struct sharp_ls027b7dh01, drm);
+}
+
+/**
+ * sharp_ls027b7dh01_add_headers - Add the Sharp LS027B7DH01 specific headers
+ * @write_buf: Buffer to write
+ * @clip: DRM clip rectangle area to write
+ * @dst_pitch: Pitch of the write buffer
+ *
+ * This function adds the SHARP LS027B7DH01 specific headers to the buffer for
+ * the Multiple Lines Write Mode:
+ * - The first byte will contain the write command.
+ * - Every line data starts with the line number and ends with a dummy zero
+ *   trailer byte. It should be noted here that the line numbers are indexed
+ *   from 1.
+ *
+ * Returns the size of the buffer to write to the display.
+ */
+static size_t sharp_ls027b7dh01_add_headers(u8 *write_buf,
+					    const struct drm_rect *clip,
+					    const unsigned int dst_pitch)
+{
+	u8 line_num = clip->y1 + 1;
+	unsigned int lines = drm_rect_height(clip);
+	unsigned int y;
+
+	write_buf[0] = CMD_WRITE;
+	write_buf[1] = line_num++;
+
+	for (y = 1; y < lines; y++) {
+		write_buf[y * dst_pitch] = 0;
+		write_buf[y * dst_pitch + 1] = line_num++;
+	}
+
+	write_buf[lines * dst_pitch] = 0;
+	write_buf[lines * dst_pitch + 1] = 0;
+
+	return lines * dst_pitch + 2;
+}
+
+static int sharp_ls027b7dh01_prepare_buf(struct sharp_ls027b7dh01 *priv,
+					 u8 *write_buf,
+					 size_t *data_len,
+					 struct drm_framebuffer *fb,
+					 const struct drm_rect *clip)
+{
+	struct drm_gem_dma_object *dma_obj;
+	struct iosys_map dst, vmap;
+	unsigned int dst_pitch;
+	int ret;
+
+	/* Leave 2 bytes to hold the line number and the trailer dummy byte. */
+	dst_pitch = (drm_rect_width(clip) / 8) + 2;
+
+	dma_obj = drm_fb_dma_get_gem_obj(fb, 0);
+
+	ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
+	if (ret)
+		return ret;
+
+	iosys_map_set_vaddr(&dst, &write_buf[2]);
+	iosys_map_set_vaddr(&vmap, dma_obj->vaddr);
+
+	drm_fb_xrgb8888_to_mono(&dst, &dst_pitch, &vmap, fb, clip);
+
+	drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
+
+	*data_len = sharp_ls027b7dh01_add_headers(write_buf, clip, dst_pitch);
+
+	return 0;
+}
+
+static int sharp_ls027b7dh01_fb_damaged(struct drm_framebuffer *fb,
+					const struct drm_rect *rect)
+{
+	struct drm_rect clip;
+	struct sharp_ls027b7dh01 *priv;
+	size_t data_len;
+	int drm_index;
+	int ret;
+
+	clip.x1 = 0;
+	clip.x2 = fb->width;
+	clip.y1 = rect->y1;
+	clip.y2 = rect->y2;
+
+	priv = drm_to_priv(fb->dev);
+
+	if (!drm_dev_enter(fb->dev, &drm_index))
+		return -ENODEV;
+
+	ret = sharp_ls027b7dh01_prepare_buf(priv, priv->write_buf, &data_len, fb, &clip);
+	if (ret)
+		goto exit;
+
+	ret = spi_write(priv->spi, priv->write_buf, data_len);
+
+exit:
+	drm_dev_exit(drm_index);
+
+	return ret;
+}
+
+static void sharp_ls027b7dh01_pipe_update(struct drm_simple_display_pipe *pipe,
+					  struct drm_plane_state *old_state)
+{
+	struct drm_plane_state *state = pipe->plane.state;
+	struct drm_rect rect;
+
+	if (!pipe->crtc.state->active)
+		return;
+
+	if (drm_atomic_helper_damage_merged(old_state, state, &rect))
+		sharp_ls027b7dh01_fb_damaged(state->fb, &rect);
+}
+
+static void sharp_ls027b7dh01_pipe_disable(struct drm_simple_display_pipe *pipe)
+{
+	struct sharp_ls027b7dh01 *priv;
+
+	priv = drm_to_priv(pipe->crtc.dev);
+	gpiod_set_value(priv->enable_gpio, 0);
+}
+
+static int sharp_ls027b7dh01_clear_display(struct sharp_ls027b7dh01 *priv)
+{
+	u8 clear_buf[2] = { CMD_CLEAR, 0 };
+
+	return spi_write(priv->spi, clear_buf, sizeof(clear_buf));
+}
+
+static int sharp_ls027b7dh01_pwm_enable(struct sharp_ls027b7dh01 *priv)
+{
+	struct device *dev = &priv->spi->dev;
+	struct pwm_state pwmstate;
+
+	priv->extcomin_pwm = devm_pwm_get(dev, NULL);
+	if (IS_ERR(priv->extcomin_pwm)) {
+		dev_err(dev, "Could not get EXTCOMIN pwm\n");
+		return PTR_ERR(priv->extcomin_pwm);
+	}
+
+	pwm_init_state(priv->extcomin_pwm, &pwmstate);
+	pwm_set_relative_duty_cycle(&pwmstate, 50, 100);
+	pwm_apply_state(priv->extcomin_pwm, &pwmstate);
+
+	pwm_enable(priv->extcomin_pwm);
+
+	return 0;
+}
+
+static void sharp_ls027b7dh01_pipe_enable(struct drm_simple_display_pipe *pipe,
+					  struct drm_crtc_state *crtc_state,
+					  struct drm_plane_state *plane_state)
+{
+	struct sharp_ls027b7dh01 *priv;
+	int ret, drm_idx;
+
+	priv = drm_to_priv(pipe->crtc.dev);
+
+	if (!drm_dev_enter(pipe->crtc.dev, &drm_idx))
+		return;
+
+	gpiod_set_value(priv->enable_gpio, 1);
+
+	ret = sharp_ls027b7dh01_clear_display(priv);
+	if (ret)
+		goto exit;
+
+	sharp_ls027b7dh01_pwm_enable(priv);
+
+exit:
+	drm_dev_exit(drm_idx);
+}
+
+static const struct drm_simple_display_pipe_funcs sharp_ls027b7dh01_pipe_funcs = {
+	.enable = sharp_ls027b7dh01_pipe_enable,
+	.disable = sharp_ls027b7dh01_pipe_disable,
+	.update = sharp_ls027b7dh01_pipe_update,
+};
+
+static int sharp_ls027b7dh01_connector_get_modes(struct drm_connector *connector)
+{
+	struct sharp_ls027b7dh01 *priv = drm_to_priv(connector->dev);
+
+	return drm_connector_helper_get_modes_fixed(connector, priv->display_mode);
+}
+
+static const struct drm_connector_helper_funcs sharp_ls027b7dh01_connector_hfuncs = {
+	.get_modes = sharp_ls027b7dh01_connector_get_modes,
+};
+
+static const struct drm_connector_funcs sharp_ls027b7dh01_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 sharp_ls027b7dh01_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 uint32_t sharp_ls027b7dh01_formats[] = {
+	DRM_FORMAT_XRGB8888,
+};
+
+static const struct drm_display_mode sharp_ls027b7dh01_mode = {
+	DRM_SIMPLE_MODE(400, 240, 59, 35),
+};
+
+DEFINE_DRM_GEM_DMA_FOPS(sharp_ls027b7dh01_fops);
+
+static const struct drm_driver sharp_ls027b7dh01_drm_driver = {
+	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
+	.fops			= &sharp_ls027b7dh01_fops,
+	DRM_GEM_DMA_DRIVER_OPS_VMAP,
+	.name			= "sharp_ls027b7dh01",
+	.desc			= "Sharp ls027b7dh01 Memory LCD",
+	.date			= "20231129",
+	.major			= 1,
+	.minor			= 0,
+};
+
+static int sharp_ls027b7dh01_probe(struct spi_device *spi)
+{
+	struct device *dev = &spi->dev;
+	struct sharp_ls027b7dh01 *priv;
+	struct drm_device *drm;
+	unsigned int write_buf_size;
+	int ret;
+
+	if (!dev->coherent_dma_mask) {
+		ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(32));
+		if (ret)
+			return dev_err_probe(dev, ret, "Failed to set dma mask\n");
+	}
+
+	priv = devm_drm_dev_alloc(dev, &sharp_ls027b7dh01_drm_driver,
+				  struct sharp_ls027b7dh01, drm);
+	if (IS_ERR(priv))
+		return PTR_ERR(priv);
+
+	spi_set_drvdata(spi, priv);
+	priv->spi = spi;
+
+	priv->enable_gpio = devm_gpiod_get(dev, "enable", GPIOD_OUT_HIGH);
+	if (IS_ERR(priv->enable_gpio))
+		return dev_err_probe(dev, PTR_ERR(priv->enable_gpio),
+				     "Failed to get GPIO 'enable'\n");
+
+	drm = &priv->drm;
+	ret = drmm_mode_config_init(drm);
+	if (ret)
+		return ret;
+
+	drm->mode_config.funcs = &sharp_ls027b7dh01_mode_config_funcs;
+	priv->display_mode = &sharp_ls027b7dh01_mode;
+
+	/*
+	 * write_buf_size:
+	 *
+	 * hdisplay * vdisplay / 8 => 1 bit per Pixel.
+	 * 2 * vdisplay => line number byte + trailer dummy byte for every line.
+	 * 2 => write command byte + final trailer dummy byte.
+	 */
+	write_buf_size = priv->display_mode->hdisplay * priv->display_mode->vdisplay / 8
+			 + 2 * priv->display_mode->vdisplay + 2;
+
+	priv->write_buf = devm_kzalloc(dev, write_buf_size, GFP_KERNEL);
+	if (!priv->write_buf)
+		return -ENOMEM;
+
+	drm->mode_config.min_width = priv->display_mode->hdisplay;
+	drm->mode_config.max_width = priv->display_mode->hdisplay;
+	drm->mode_config.min_height = priv->display_mode->vdisplay;
+	drm->mode_config.max_height = priv->display_mode->vdisplay;
+
+	ret = drm_connector_init(drm, &priv->connector,
+				 &sharp_ls027b7dh01_connector_funcs,
+				 DRM_MODE_CONNECTOR_SPI);
+	if (ret)
+		return ret;
+
+	drm_connector_helper_add(&priv->connector,
+				 &sharp_ls027b7dh01_connector_hfuncs);
+
+	ret = drm_simple_display_pipe_init(drm, &priv->pipe,
+					   &sharp_ls027b7dh01_pipe_funcs,
+					   sharp_ls027b7dh01_formats,
+					   ARRAY_SIZE(sharp_ls027b7dh01_formats),
+					   NULL, &priv->connector);
+	if (ret)
+		return ret;
+
+	drm_plane_enable_fb_damage_clips(&priv->pipe.plane);
+	drm_mode_config_reset(drm);
+
+	ret = drm_dev_register(drm, 0);
+	if (ret)
+		return ret;
+
+	drm_fbdev_generic_setup(drm, 0);
+
+	return 0;
+}
+
+static void sharp_ls027b7dh01_remove(struct spi_device *spi)
+{
+	struct sharp_ls027b7dh01 *priv = spi_get_drvdata(spi);
+
+	drm_dev_unplug(&priv->drm);
+	drm_atomic_helper_shutdown(&priv->drm);
+}
+
+static void sharp_ls027b7dh01_shutdown(struct spi_device *spi)
+{
+	struct sharp_ls027b7dh01 *priv = spi_get_drvdata(spi);
+
+	drm_atomic_helper_shutdown(&priv->drm);
+}
+
+static const struct spi_device_id sharp_ls027b7dh01_ids[] = {
+	{ "ls027b7dh01" },
+	{ },
+};
+MODULE_DEVICE_TABLE(spi, sharp_ls027b7dh01_ids);
+
+static const struct of_device_id sharp_ls027b7dh01_of_match[] = {
+	{ .compatible = "sharp,ls027b7dh01", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, sharp_ls027b7dh01_of_match);
+
+static struct spi_driver sharp_ls027b7dh01_spi_driver = {
+	.probe = sharp_ls027b7dh01_probe,
+	.remove = sharp_ls027b7dh01_remove,
+	.shutdown = sharp_ls027b7dh01_shutdown,
+	.id_table = sharp_ls027b7dh01_ids,
+	.driver = {
+		.name = "sharp-ls027b7dh01",
+		.of_match_table = sharp_ls027b7dh01_of_match,
+	},
+};
+module_spi_driver(sharp_ls027b7dh01_spi_driver);
+
+MODULE_AUTHOR("Andrew D'Angelo");
+MODULE_AUTHOR("Mehdi Djait <mehdi.djait@bootlin.com>");
+MODULE_DESCRIPTION("Sharp LS027B7DH01 Driver");
+MODULE_LICENSE("GPL");
-- 
2.41.0

Re: [PATCH 2/2] drm/tiny: Add driver for the sharp LS027B7DH01 Memory LCD

[Thomas Zimmermann]

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

Hi,

thanks for the patches.

Am 29.11.23 um 15:29 schrieb Mehdi Djait:
> Introduce a DRM driver for the sharp LS027B7DH01 Memory LCD.
> 
> LS027B7DH01 is a 2.7" 400x240 monochrome display connected to a SPI bus.
> The drivers implements the Multiple Lines Data Update Mode.
> External COM inversion is enabled using a PWM signal as input.
> 
> Signed-off-by: Mehdi Djait <mehdi.djait@bootlin.com>
> ---
>   MAINTAINERS                              |   7 +
>   drivers/gpu/drm/tiny/Kconfig             |   8 +
>   drivers/gpu/drm/tiny/Makefile            |   1 +
>   drivers/gpu/drm/tiny/sharp-ls027b7dh01.c | 411 +++++++++++++++++++++++
>   4 files changed, 427 insertions(+)
>   create mode 100644 drivers/gpu/drm/tiny/sharp-ls027b7dh01.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 012df8ccf34e..fb859698bd3d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6832,6 +6832,13 @@ S:	Maintained
>   F:	Documentation/devicetree/bindings/display/panel/samsung,s6d7aa0.yaml
>   F:	drivers/gpu/drm/panel/panel-samsung-s6d7aa0.c
>   
> +DRM DRIVER FOR SHARP LS027B7DH01 Memory LCD
> +M:	Mehdi Djait <mehdi.djait@bootlin.com>
> +S:	Maintained
> +T:	git git://anongit.freedesktop.org/drm/drm-misc
> +F:	Documentation/devicetree/bindings/display/sharp,ls027b7dh01.yaml
> +F:	drivers/gpu/drm/tiny/sharp-ls027b7dh01.c
> +
>   DRM DRIVER FOR SITRONIX ST7586 PANELS
>   M:	David Lechner <david@lechnology.com>
>   S:	Maintained
> diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig
> index f6889f649bc1..a2ade06403ca 100644
> --- a/drivers/gpu/drm/tiny/Kconfig
> +++ b/drivers/gpu/drm/tiny/Kconfig
> @@ -186,6 +186,14 @@ config TINYDRM_REPAPER
>   
>   	  If M is selected the module will be called repaper.
>   
> +config TINYDRM_SHARP_LS027B7DH01
> +	tristate "DRM support for SHARP LS027B7DH01 display"
> +	depends on DRM && SPI
> +	select DRM_KMS_HELPER
> +	select DRM_GEM_DMA_HELPER
> +	help
> +	  DRM driver for the SHARP LS027B7DD01 LCD display.
> +
>   config TINYDRM_ST7586
>   	tristate "DRM support for Sitronix ST7586 display panels"
>   	depends on DRM && SPI
> diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile
> index 76dde89a044b..b05df3afb231 100644
> --- a/drivers/gpu/drm/tiny/Makefile
> +++ b/drivers/gpu/drm/tiny/Makefile
> @@ -14,5 +14,6 @@ obj-$(CONFIG_TINYDRM_ILI9341)		+= ili9341.o
>   obj-$(CONFIG_TINYDRM_ILI9486)		+= ili9486.o
>   obj-$(CONFIG_TINYDRM_MI0283QT)		+= mi0283qt.o
>   obj-$(CONFIG_TINYDRM_REPAPER)		+= repaper.o
> +obj-$(CONFIG_TINYDRM_SHARP_LS027B7DH01)	+= sharp-ls027b7dh01.o
>   obj-$(CONFIG_TINYDRM_ST7586)		+= st7586.o
>   obj-$(CONFIG_TINYDRM_ST7735R)		+= st7735r.o
> diff --git a/drivers/gpu/drm/tiny/sharp-ls027b7dh01.c b/drivers/gpu/drm/tiny/sharp-ls027b7dh01.c
> new file mode 100644
> index 000000000000..2f58865a5c78
> --- /dev/null
> +++ b/drivers/gpu/drm/tiny/sharp-ls027b7dh01.c
> @@ -0,0 +1,411 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Sharp LS027B7DH01 Memory Display Driver
> + *
> + * Copyright (C) 2023 Andrew D'Angelo
> + * Copyright (C) 2023 Mehdi Djait <mehdi.djait@bootlin.com>
> + *
> + * The Sharp Memory LCD requires an alternating signal to prevent the buildup of
> + * a DC bias that would result in a Display that no longer can be updated. Two
> + * modes for the generation of this signal are supported:
> + *
> + * Software, EXTMODE = Low: toggling the BIT(1) of the Command and writing it at
> + * least once a second to the display.
> + *
> + * Hardware, EXTMODE = High: the alternating signal should be supplied on the
> + * EXTCOMIN pin.
> + *
> + * In this driver the Hardware mode is implemented with a PWM signal.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/pwm.h>
> +#include <linux/spi/spi.h>
> +
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_damage_helper.h>
> +#include <drm/drm_drv.h>
> +#include <drm/drm_fbdev_generic.h>
> +#include <drm/drm_fb_dma_helper.h>
> +#include <drm/drm_format_helper.h>
> +#include <drm/drm_framebuffer.h>
> +#include <drm/drm_gem_atomic_helper.h>
> +#include <drm/drm_gem_dma_helper.h>
> +#include <drm/drm_gem_framebuffer_helper.h>
> +#include <drm/drm_modes.h>
> +#include <drm/drm_probe_helper.h>
> +#include <drm/drm_simple_kms_helper.h>
> +
> +#define CMD_WRITE BIT(0)
> +#define CMD_CLEAR BIT(2)
> +
> +struct sharp_ls027b7dh01 {
> +	struct spi_device *spi;
> +
> +	struct drm_device drm;
> +	struct drm_connector connector;
> +	struct drm_simple_display_pipe pipe;

Could you please replace the simple pipe and its helpers with regular 
DRMhelpers. It should no tbe used in new drivers. It's an unnecessary 
indirection. Replacing is simple: copy the content of the data structure 
and its helpers into the driver. Maybe clean up the result, if necessary.

Best regards
Thomas

> +	const struct drm_display_mode *display_mode;
> +
> +	struct gpio_desc *enable_gpio;
> +	struct pwm_device *extcomin_pwm;
> +
> +	u8 *write_buf;
> +};
> +
> +static inline struct sharp_ls027b7dh01 *drm_to_priv(struct drm_device *drm)
> +{
> +	return container_of(drm, struct sharp_ls027b7dh01, drm);
> +}
> +
> +/**
> + * sharp_ls027b7dh01_add_headers - Add the Sharp LS027B7DH01 specific headers
> + * @write_buf: Buffer to write
> + * @clip: DRM clip rectangle area to write
> + * @dst_pitch: Pitch of the write buffer
> + *
> + * This function adds the SHARP LS027B7DH01 specific headers to the buffer for
> + * the Multiple Lines Write Mode:
> + * - The first byte will contain the write command.
> + * - Every line data starts with the line number and ends with a dummy zero
> + *   trailer byte. It should be noted here that the line numbers are indexed
> + *   from 1.
> + *
> + * Returns the size of the buffer to write to the display.
> + */
> +static size_t sharp_ls027b7dh01_add_headers(u8 *write_buf,
> +					    const struct drm_rect *clip,
> +					    const unsigned int dst_pitch)
> +{
> +	u8 line_num = clip->y1 + 1;
> +	unsigned int lines = drm_rect_height(clip);
> +	unsigned int y;
> +
> +	write_buf[0] = CMD_WRITE;
> +	write_buf[1] = line_num++;
> +
> +	for (y = 1; y < lines; y++) {
> +		write_buf[y * dst_pitch] = 0;
> +		write_buf[y * dst_pitch + 1] = line_num++;
> +	}
> +
> +	write_buf[lines * dst_pitch] = 0;
> +	write_buf[lines * dst_pitch + 1] = 0;
> +
> +	return lines * dst_pitch + 2;
> +}
> +
> +static int sharp_ls027b7dh01_prepare_buf(struct sharp_ls027b7dh01 *priv,
> +					 u8 *write_buf,
> +					 size_t *data_len,
> +					 struct drm_framebuffer *fb,
> +					 const struct drm_rect *clip)
> +{
> +	struct drm_gem_dma_object *dma_obj;
> +	struct iosys_map dst, vmap;
> +	unsigned int dst_pitch;
> +	int ret;
> +
> +	/* Leave 2 bytes to hold the line number and the trailer dummy byte. */
> +	dst_pitch = (drm_rect_width(clip) / 8) + 2;
> +
> +	dma_obj = drm_fb_dma_get_gem_obj(fb, 0);
> +
> +	ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
> +	if (ret)
> +		return ret;
> +
> +	iosys_map_set_vaddr(&dst, &write_buf[2]);
> +	iosys_map_set_vaddr(&vmap, dma_obj->vaddr);
> +
> +	drm_fb_xrgb8888_to_mono(&dst, &dst_pitch, &vmap, fb, clip);
> +
> +	drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
> +
> +	*data_len = sharp_ls027b7dh01_add_headers(write_buf, clip, dst_pitch);
> +
> +	return 0;
> +}
> +
> +static int sharp_ls027b7dh01_fb_damaged(struct drm_framebuffer *fb,
> +					const struct drm_rect *rect)
> +{
> +	struct drm_rect clip;
> +	struct sharp_ls027b7dh01 *priv;
> +	size_t data_len;
> +	int drm_index;
> +	int ret;
> +
> +	clip.x1 = 0;
> +	clip.x2 = fb->width;
> +	clip.y1 = rect->y1;
> +	clip.y2 = rect->y2;
> +
> +	priv = drm_to_priv(fb->dev);
> +
> +	if (!drm_dev_enter(fb->dev, &drm_index))
> +		return -ENODEV;
> +
> +	ret = sharp_ls027b7dh01_prepare_buf(priv, priv->write_buf, &data_len, fb, &clip);
> +	if (ret)
> +		goto exit;
> +
> +	ret = spi_write(priv->spi, priv->write_buf, data_len);
> +
> +exit:
> +	drm_dev_exit(drm_index);
> +
> +	return ret;
> +}
> +
> +static void sharp_ls027b7dh01_pipe_update(struct drm_simple_display_pipe *pipe,
> +					  struct drm_plane_state *old_state)
> +{
> +	struct drm_plane_state *state = pipe->plane.state;
> +	struct drm_rect rect;
> +
> +	if (!pipe->crtc.state->active)
> +		return;
> +
> +	if (drm_atomic_helper_damage_merged(old_state, state, &rect))
> +		sharp_ls027b7dh01_fb_damaged(state->fb, &rect);
> +}
> +
> +static void sharp_ls027b7dh01_pipe_disable(struct drm_simple_display_pipe *pipe)
> +{
> +	struct sharp_ls027b7dh01 *priv;
> +
> +	priv = drm_to_priv(pipe->crtc.dev);
> +	gpiod_set_value(priv->enable_gpio, 0);
> +}
> +
> +static int sharp_ls027b7dh01_clear_display(struct sharp_ls027b7dh01 *priv)
> +{
> +	u8 clear_buf[2] = { CMD_CLEAR, 0 };
> +
> +	return spi_write(priv->spi, clear_buf, sizeof(clear_buf));
> +}
> +
> +static int sharp_ls027b7dh01_pwm_enable(struct sharp_ls027b7dh01 *priv)
> +{
> +	struct device *dev = &priv->spi->dev;
> +	struct pwm_state pwmstate;
> +
> +	priv->extcomin_pwm = devm_pwm_get(dev, NULL);
> +	if (IS_ERR(priv->extcomin_pwm)) {
> +		dev_err(dev, "Could not get EXTCOMIN pwm\n");
> +		return PTR_ERR(priv->extcomin_pwm);
> +	}
> +
> +	pwm_init_state(priv->extcomin_pwm, &pwmstate);
> +	pwm_set_relative_duty_cycle(&pwmstate, 50, 100);
> +	pwm_apply_state(priv->extcomin_pwm, &pwmstate);
> +
> +	pwm_enable(priv->extcomin_pwm);
> +
> +	return 0;
> +}
> +
> +static void sharp_ls027b7dh01_pipe_enable(struct drm_simple_display_pipe *pipe,
> +					  struct drm_crtc_state *crtc_state,
> +					  struct drm_plane_state *plane_state)
> +{
> +	struct sharp_ls027b7dh01 *priv;
> +	int ret, drm_idx;
> +
> +	priv = drm_to_priv(pipe->crtc.dev);
> +
> +	if (!drm_dev_enter(pipe->crtc.dev, &drm_idx))
> +		return;
> +
> +	gpiod_set_value(priv->enable_gpio, 1);
> +
> +	ret = sharp_ls027b7dh01_clear_display(priv);
> +	if (ret)
> +		goto exit;
> +
> +	sharp_ls027b7dh01_pwm_enable(priv);
> +
> +exit:
> +	drm_dev_exit(drm_idx);
> +}
> +
> +static const struct drm_simple_display_pipe_funcs sharp_ls027b7dh01_pipe_funcs = {
> +	.enable = sharp_ls027b7dh01_pipe_enable,
> +	.disable = sharp_ls027b7dh01_pipe_disable,
> +	.update = sharp_ls027b7dh01_pipe_update,
> +};
> +
> +static int sharp_ls027b7dh01_connector_get_modes(struct drm_connector *connector)
> +{
> +	struct sharp_ls027b7dh01 *priv = drm_to_priv(connector->dev);
> +
> +	return drm_connector_helper_get_modes_fixed(connector, priv->display_mode);
> +}
> +
> +static const struct drm_connector_helper_funcs sharp_ls027b7dh01_connector_hfuncs = {
> +	.get_modes = sharp_ls027b7dh01_connector_get_modes,
> +};
> +
> +static const struct drm_connector_funcs sharp_ls027b7dh01_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 sharp_ls027b7dh01_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 uint32_t sharp_ls027b7dh01_formats[] = {
> +	DRM_FORMAT_XRGB8888,
> +};
> +
> +static const struct drm_display_mode sharp_ls027b7dh01_mode = {
> +	DRM_SIMPLE_MODE(400, 240, 59, 35),
> +};
> +
> +DEFINE_DRM_GEM_DMA_FOPS(sharp_ls027b7dh01_fops);
> +
> +static const struct drm_driver sharp_ls027b7dh01_drm_driver = {
> +	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
> +	.fops			= &sharp_ls027b7dh01_fops,
> +	DRM_GEM_DMA_DRIVER_OPS_VMAP,
> +	.name			= "sharp_ls027b7dh01",
> +	.desc			= "Sharp ls027b7dh01 Memory LCD",
> +	.date			= "20231129",
> +	.major			= 1,
> +	.minor			= 0,
> +};
> +
> +static int sharp_ls027b7dh01_probe(struct spi_device *spi)
> +{
> +	struct device *dev = &spi->dev;
> +	struct sharp_ls027b7dh01 *priv;
> +	struct drm_device *drm;
> +	unsigned int write_buf_size;
> +	int ret;
> +
> +	if (!dev->coherent_dma_mask) {
> +		ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(32));
> +		if (ret)
> +			return dev_err_probe(dev, ret, "Failed to set dma mask\n");
> +	}
> +
> +	priv = devm_drm_dev_alloc(dev, &sharp_ls027b7dh01_drm_driver,
> +				  struct sharp_ls027b7dh01, drm);
> +	if (IS_ERR(priv))
> +		return PTR_ERR(priv);
> +
> +	spi_set_drvdata(spi, priv);
> +	priv->spi = spi;
> +
> +	priv->enable_gpio = devm_gpiod_get(dev, "enable", GPIOD_OUT_HIGH);
> +	if (IS_ERR(priv->enable_gpio))
> +		return dev_err_probe(dev, PTR_ERR(priv->enable_gpio),
> +				     "Failed to get GPIO 'enable'\n");
> +
> +	drm = &priv->drm;
> +	ret = drmm_mode_config_init(drm);
> +	if (ret)
> +		return ret;
> +
> +	drm->mode_config.funcs = &sharp_ls027b7dh01_mode_config_funcs;
> +	priv->display_mode = &sharp_ls027b7dh01_mode;
> +
> +	/*
> +	 * write_buf_size:
> +	 *
> +	 * hdisplay * vdisplay / 8 => 1 bit per Pixel.
> +	 * 2 * vdisplay => line number byte + trailer dummy byte for every line.
> +	 * 2 => write command byte + final trailer dummy byte.
> +	 */
> +	write_buf_size = priv->display_mode->hdisplay * priv->display_mode->vdisplay / 8
> +			 + 2 * priv->display_mode->vdisplay + 2;
> +
> +	priv->write_buf = devm_kzalloc(dev, write_buf_size, GFP_KERNEL);
> +	if (!priv->write_buf)
> +		return -ENOMEM;
> +
> +	drm->mode_config.min_width = priv->display_mode->hdisplay;
> +	drm->mode_config.max_width = priv->display_mode->hdisplay;
> +	drm->mode_config.min_height = priv->display_mode->vdisplay;
> +	drm->mode_config.max_height = priv->display_mode->vdisplay;
> +
> +	ret = drm_connector_init(drm, &priv->connector,
> +				 &sharp_ls027b7dh01_connector_funcs,
> +				 DRM_MODE_CONNECTOR_SPI);
> +	if (ret)
> +		return ret;
> +
> +	drm_connector_helper_add(&priv->connector,
> +				 &sharp_ls027b7dh01_connector_hfuncs);
> +
> +	ret = drm_simple_display_pipe_init(drm, &priv->pipe,
> +					   &sharp_ls027b7dh01_pipe_funcs,
> +					   sharp_ls027b7dh01_formats,
> +					   ARRAY_SIZE(sharp_ls027b7dh01_formats),
> +					   NULL, &priv->connector);
> +	if (ret)
> +		return ret;
> +
> +	drm_plane_enable_fb_damage_clips(&priv->pipe.plane);
> +	drm_mode_config_reset(drm);
> +
> +	ret = drm_dev_register(drm, 0);
> +	if (ret)
> +		return ret;
> +
> +	drm_fbdev_generic_setup(drm, 0);
> +
> +	return 0;
> +}
> +
> +static void sharp_ls027b7dh01_remove(struct spi_device *spi)
> +{
> +	struct sharp_ls027b7dh01 *priv = spi_get_drvdata(spi);
> +
> +	drm_dev_unplug(&priv->drm);
> +	drm_atomic_helper_shutdown(&priv->drm);
> +}
> +
> +static void sharp_ls027b7dh01_shutdown(struct spi_device *spi)
> +{
> +	struct sharp_ls027b7dh01 *priv = spi_get_drvdata(spi);
> +
> +	drm_atomic_helper_shutdown(&priv->drm);
> +}
> +
> +static const struct spi_device_id sharp_ls027b7dh01_ids[] = {
> +	{ "ls027b7dh01" },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(spi, sharp_ls027b7dh01_ids);
> +
> +static const struct of_device_id sharp_ls027b7dh01_of_match[] = {
> +	{ .compatible = "sharp,ls027b7dh01", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, sharp_ls027b7dh01_of_match);
> +
> +static struct spi_driver sharp_ls027b7dh01_spi_driver = {
> +	.probe = sharp_ls027b7dh01_probe,
> +	.remove = sharp_ls027b7dh01_remove,
> +	.shutdown = sharp_ls027b7dh01_shutdown,
> +	.id_table = sharp_ls027b7dh01_ids,
> +	.driver = {
> +		.name = "sharp-ls027b7dh01",
> +		.of_match_table = sharp_ls027b7dh01_of_match,
> +	},
> +};
> +module_spi_driver(sharp_ls027b7dh01_spi_driver);
> +
> +MODULE_AUTHOR("Andrew D'Angelo");
> +MODULE_AUTHOR("Mehdi Djait <mehdi.djait@bootlin.com>");
> +MODULE_DESCRIPTION("Sharp LS027B7DH01 Driver");
> +MODULE_LICENSE("GPL");

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH 2/2] drm/tiny: Add driver for the sharp LS027B7DH01 Memory LCD

[Thomas Zimmermann]

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

Hi,

thanks for the patches.

Am 29.11.23 um 15:29 schrieb Mehdi Djait:
> Introduce a DRM driver for the sharp LS027B7DH01 Memory LCD.
> 
> LS027B7DH01 is a 2.7" 400x240 monochrome display connected to a SPI bus.
> The drivers implements the Multiple Lines Data Update Mode.
> External COM inversion is enabled using a PWM signal as input.
> 
> Signed-off-by: Mehdi Djait <mehdi.djait@bootlin.com>
> ---
>   MAINTAINERS                              |   7 +
>   drivers/gpu/drm/tiny/Kconfig             |   8 +
>   drivers/gpu/drm/tiny/Makefile            |   1 +
>   drivers/gpu/drm/tiny/sharp-ls027b7dh01.c | 411 +++++++++++++++++++++++
>   4 files changed, 427 insertions(+)
>   create mode 100644 drivers/gpu/drm/tiny/sharp-ls027b7dh01.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 012df8ccf34e..fb859698bd3d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6832,6 +6832,13 @@ S:	Maintained
>   F:	Documentation/devicetree/bindings/display/panel/samsung,s6d7aa0.yaml
>   F:	drivers/gpu/drm/panel/panel-samsung-s6d7aa0.c
>   
> +DRM DRIVER FOR SHARP LS027B7DH01 Memory LCD
> +M:	Mehdi Djait <mehdi.djait@bootlin.com>
> +S:	Maintained
> +T:	git git://anongit.freedesktop.org/drm/drm-misc
> +F:	Documentation/devicetree/bindings/display/sharp,ls027b7dh01.yaml
> +F:	drivers/gpu/drm/tiny/sharp-ls027b7dh01.c
> +
>   DRM DRIVER FOR SITRONIX ST7586 PANELS
>   M:	David Lechner <david@lechnology.com>
>   S:	Maintained
> diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig
> index f6889f649bc1..a2ade06403ca 100644
> --- a/drivers/gpu/drm/tiny/Kconfig
> +++ b/drivers/gpu/drm/tiny/Kconfig
> @@ -186,6 +186,14 @@ config TINYDRM_REPAPER
>   
>   	  If M is selected the module will be called repaper.
>   
> +config TINYDRM_SHARP_LS027B7DH01
> +	tristate "DRM support for SHARP LS027B7DH01 display"
> +	depends on DRM && SPI
> +	select DRM_KMS_HELPER
> +	select DRM_GEM_DMA_HELPER
> +	help
> +	  DRM driver for the SHARP LS027B7DD01 LCD display.
> +
>   config TINYDRM_ST7586
>   	tristate "DRM support for Sitronix ST7586 display panels"
>   	depends on DRM && SPI
> diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile
> index 76dde89a044b..b05df3afb231 100644
> --- a/drivers/gpu/drm/tiny/Makefile
> +++ b/drivers/gpu/drm/tiny/Makefile
> @@ -14,5 +14,6 @@ obj-$(CONFIG_TINYDRM_ILI9341)		+= ili9341.o
>   obj-$(CONFIG_TINYDRM_ILI9486)		+= ili9486.o
>   obj-$(CONFIG_TINYDRM_MI0283QT)		+= mi0283qt.o
>   obj-$(CONFIG_TINYDRM_REPAPER)		+= repaper.o
> +obj-$(CONFIG_TINYDRM_SHARP_LS027B7DH01)	+= sharp-ls027b7dh01.o
>   obj-$(CONFIG_TINYDRM_ST7586)		+= st7586.o
>   obj-$(CONFIG_TINYDRM_ST7735R)		+= st7735r.o
> diff --git a/drivers/gpu/drm/tiny/sharp-ls027b7dh01.c b/drivers/gpu/drm/tiny/sharp-ls027b7dh01.c
> new file mode 100644
> index 000000000000..2f58865a5c78
> --- /dev/null
> +++ b/drivers/gpu/drm/tiny/sharp-ls027b7dh01.c
> @@ -0,0 +1,411 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Sharp LS027B7DH01 Memory Display Driver
> + *
> + * Copyright (C) 2023 Andrew D'Angelo
> + * Copyright (C) 2023 Mehdi Djait <mehdi.djait@bootlin.com>
> + *
> + * The Sharp Memory LCD requires an alternating signal to prevent the buildup of
> + * a DC bias that would result in a Display that no longer can be updated. Two
> + * modes for the generation of this signal are supported:
> + *
> + * Software, EXTMODE = Low: toggling the BIT(1) of the Command and writing it at
> + * least once a second to the display.
> + *
> + * Hardware, EXTMODE = High: the alternating signal should be supplied on the
> + * EXTCOMIN pin.
> + *
> + * In this driver the Hardware mode is implemented with a PWM signal.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/pwm.h>
> +#include <linux/spi/spi.h>
> +
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_damage_helper.h>
> +#include <drm/drm_drv.h>
> +#include <drm/drm_fbdev_generic.h>
> +#include <drm/drm_fb_dma_helper.h>
> +#include <drm/drm_format_helper.h>
> +#include <drm/drm_framebuffer.h>
> +#include <drm/drm_gem_atomic_helper.h>
> +#include <drm/drm_gem_dma_helper.h>
> +#include <drm/drm_gem_framebuffer_helper.h>
> +#include <drm/drm_modes.h>
> +#include <drm/drm_probe_helper.h>
> +#include <drm/drm_simple_kms_helper.h>
> +
> +#define CMD_WRITE BIT(0)
> +#define CMD_CLEAR BIT(2)
> +
> +struct sharp_ls027b7dh01 {
> +	struct spi_device *spi;
> +
> +	struct drm_device drm;
> +	struct drm_connector connector;
> +	struct drm_simple_display_pipe pipe;

Could you please replace the simple pipe and its helpers with regular 
DRMhelpers. It should no tbe used in new drivers. It's an unnecessary 
indirection. Replacing is simple: copy the content of the data structure 
and its helpers into the driver. Maybe clean up the result, if necessary.

Best regards
Thomas

> +	const struct drm_display_mode *display_mode;
> +
> +	struct gpio_desc *enable_gpio;
> +	struct pwm_device *extcomin_pwm;
> +
> +	u8 *write_buf;
> +};
> +
> +static inline struct sharp_ls027b7dh01 *drm_to_priv(struct drm_device *drm)
> +{
> +	return container_of(drm, struct sharp_ls027b7dh01, drm);
> +}
> +
> +/**
> + * sharp_ls027b7dh01_add_headers - Add the Sharp LS027B7DH01 specific headers
> + * @write_buf: Buffer to write
> + * @clip: DRM clip rectangle area to write
> + * @dst_pitch: Pitch of the write buffer
> + *
> + * This function adds the SHARP LS027B7DH01 specific headers to the buffer for
> + * the Multiple Lines Write Mode:
> + * - The first byte will contain the write command.
> + * - Every line data starts with the line number and ends with a dummy zero
> + *   trailer byte. It should be noted here that the line numbers are indexed
> + *   from 1.
> + *
> + * Returns the size of the buffer to write to the display.
> + */
> +static size_t sharp_ls027b7dh01_add_headers(u8 *write_buf,
> +					    const struct drm_rect *clip,
> +					    const unsigned int dst_pitch)
> +{
> +	u8 line_num = clip->y1 + 1;
> +	unsigned int lines = drm_rect_height(clip);
> +	unsigned int y;
> +
> +	write_buf[0] = CMD_WRITE;
> +	write_buf[1] = line_num++;
> +
> +	for (y = 1; y < lines; y++) {
> +		write_buf[y * dst_pitch] = 0;
> +		write_buf[y * dst_pitch + 1] = line_num++;
> +	}
> +
> +	write_buf[lines * dst_pitch] = 0;
> +	write_buf[lines * dst_pitch + 1] = 0;
> +
> +	return lines * dst_pitch + 2;
> +}
> +
> +static int sharp_ls027b7dh01_prepare_buf(struct sharp_ls027b7dh01 *priv,
> +					 u8 *write_buf,
> +					 size_t *data_len,
> +					 struct drm_framebuffer *fb,
> +					 const struct drm_rect *clip)
> +{
> +	struct drm_gem_dma_object *dma_obj;
> +	struct iosys_map dst, vmap;
> +	unsigned int dst_pitch;
> +	int ret;
> +
> +	/* Leave 2 bytes to hold the line number and the trailer dummy byte. */
> +	dst_pitch = (drm_rect_width(clip) / 8) + 2;
> +
> +	dma_obj = drm_fb_dma_get_gem_obj(fb, 0);
> +
> +	ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
> +	if (ret)
> +		return ret;
> +
> +	iosys_map_set_vaddr(&dst, &write_buf[2]);
> +	iosys_map_set_vaddr(&vmap, dma_obj->vaddr);
> +
> +	drm_fb_xrgb8888_to_mono(&dst, &dst_pitch, &vmap, fb, clip);
> +
> +	drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
> +
> +	*data_len = sharp_ls027b7dh01_add_headers(write_buf, clip, dst_pitch);
> +
> +	return 0;
> +}
> +
> +static int sharp_ls027b7dh01_fb_damaged(struct drm_framebuffer *fb,
> +					const struct drm_rect *rect)
> +{
> +	struct drm_rect clip;
> +	struct sharp_ls027b7dh01 *priv;
> +	size_t data_len;
> +	int drm_index;
> +	int ret;
> +
> +	clip.x1 = 0;
> +	clip.x2 = fb->width;
> +	clip.y1 = rect->y1;
> +	clip.y2 = rect->y2;
> +
> +	priv = drm_to_priv(fb->dev);
> +
> +	if (!drm_dev_enter(fb->dev, &drm_index))
> +		return -ENODEV;
> +
> +	ret = sharp_ls027b7dh01_prepare_buf(priv, priv->write_buf, &data_len, fb, &clip);
> +	if (ret)
> +		goto exit;
> +
> +	ret = spi_write(priv->spi, priv->write_buf, data_len);
> +
> +exit:
> +	drm_dev_exit(drm_index);
> +
> +	return ret;
> +}
> +
> +static void sharp_ls027b7dh01_pipe_update(struct drm_simple_display_pipe *pipe,
> +					  struct drm_plane_state *old_state)
> +{
> +	struct drm_plane_state *state = pipe->plane.state;
> +	struct drm_rect rect;
> +
> +	if (!pipe->crtc.state->active)
> +		return;
> +
> +	if (drm_atomic_helper_damage_merged(old_state, state, &rect))
> +		sharp_ls027b7dh01_fb_damaged(state->fb, &rect);
> +}
> +
> +static void sharp_ls027b7dh01_pipe_disable(struct drm_simple_display_pipe *pipe)
> +{
> +	struct sharp_ls027b7dh01 *priv;
> +
> +	priv = drm_to_priv(pipe->crtc.dev);
> +	gpiod_set_value(priv->enable_gpio, 0);
> +}
> +
> +static int sharp_ls027b7dh01_clear_display(struct sharp_ls027b7dh01 *priv)
> +{
> +	u8 clear_buf[2] = { CMD_CLEAR, 0 };
> +
> +	return spi_write(priv->spi, clear_buf, sizeof(clear_buf));
> +}
> +
> +static int sharp_ls027b7dh01_pwm_enable(struct sharp_ls027b7dh01 *priv)
> +{
> +	struct device *dev = &priv->spi->dev;
> +	struct pwm_state pwmstate;
> +
> +	priv->extcomin_pwm = devm_pwm_get(dev, NULL);
> +	if (IS_ERR(priv->extcomin_pwm)) {
> +		dev_err(dev, "Could not get EXTCOMIN pwm\n");
> +		return PTR_ERR(priv->extcomin_pwm);
> +	}
> +
> +	pwm_init_state(priv->extcomin_pwm, &pwmstate);
> +	pwm_set_relative_duty_cycle(&pwmstate, 50, 100);
> +	pwm_apply_state(priv->extcomin_pwm, &pwmstate);
> +
> +	pwm_enable(priv->extcomin_pwm);
> +
> +	return 0;
> +}
> +
> +static void sharp_ls027b7dh01_pipe_enable(struct drm_simple_display_pipe *pipe,
> +					  struct drm_crtc_state *crtc_state,
> +					  struct drm_plane_state *plane_state)
> +{
> +	struct sharp_ls027b7dh01 *priv;
> +	int ret, drm_idx;
> +
> +	priv = drm_to_priv(pipe->crtc.dev);
> +
> +	if (!drm_dev_enter(pipe->crtc.dev, &drm_idx))
> +		return;
> +
> +	gpiod_set_value(priv->enable_gpio, 1);
> +
> +	ret = sharp_ls027b7dh01_clear_display(priv);
> +	if (ret)
> +		goto exit;
> +
> +	sharp_ls027b7dh01_pwm_enable(priv);
> +
> +exit:
> +	drm_dev_exit(drm_idx);
> +}
> +
> +static const struct drm_simple_display_pipe_funcs sharp_ls027b7dh01_pipe_funcs = {
> +	.enable = sharp_ls027b7dh01_pipe_enable,
> +	.disable = sharp_ls027b7dh01_pipe_disable,
> +	.update = sharp_ls027b7dh01_pipe_update,
> +};
> +
> +static int sharp_ls027b7dh01_connector_get_modes(struct drm_connector *connector)
> +{
> +	struct sharp_ls027b7dh01 *priv = drm_to_priv(connector->dev);
> +
> +	return drm_connector_helper_get_modes_fixed(connector, priv->display_mode);
> +}
> +
> +static const struct drm_connector_helper_funcs sharp_ls027b7dh01_connector_hfuncs = {
> +	.get_modes = sharp_ls027b7dh01_connector_get_modes,
> +};
> +
> +static const struct drm_connector_funcs sharp_ls027b7dh01_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 sharp_ls027b7dh01_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 uint32_t sharp_ls027b7dh01_formats[] = {
> +	DRM_FORMAT_XRGB8888,
> +};
> +
> +static const struct drm_display_mode sharp_ls027b7dh01_mode = {
> +	DRM_SIMPLE_MODE(400, 240, 59, 35),
> +};
> +
> +DEFINE_DRM_GEM_DMA_FOPS(sharp_ls027b7dh01_fops);
> +
> +static const struct drm_driver sharp_ls027b7dh01_drm_driver = {
> +	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
> +	.fops			= &sharp_ls027b7dh01_fops,
> +	DRM_GEM_DMA_DRIVER_OPS_VMAP,
> +	.name			= "sharp_ls027b7dh01",
> +	.desc			= "Sharp ls027b7dh01 Memory LCD",
> +	.date			= "20231129",
> +	.major			= 1,
> +	.minor			= 0,
> +};
> +
> +static int sharp_ls027b7dh01_probe(struct spi_device *spi)
> +{
> +	struct device *dev = &spi->dev;
> +	struct sharp_ls027b7dh01 *priv;
> +	struct drm_device *drm;
> +	unsigned int write_buf_size;
> +	int ret;
> +
> +	if (!dev->coherent_dma_mask) {
> +		ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(32));
> +		if (ret)
> +			return dev_err_probe(dev, ret, "Failed to set dma mask\n");
> +	}
> +
> +	priv = devm_drm_dev_alloc(dev, &sharp_ls027b7dh01_drm_driver,
> +				  struct sharp_ls027b7dh01, drm);
> +	if (IS_ERR(priv))
> +		return PTR_ERR(priv);
> +
> +	spi_set_drvdata(spi, priv);
> +	priv->spi = spi;
> +
> +	priv->enable_gpio = devm_gpiod_get(dev, "enable", GPIOD_OUT_HIGH);
> +	if (IS_ERR(priv->enable_gpio))
> +		return dev_err_probe(dev, PTR_ERR(priv->enable_gpio),
> +				     "Failed to get GPIO 'enable'\n");
> +
> +	drm = &priv->drm;
> +	ret = drmm_mode_config_init(drm);
> +	if (ret)
> +		return ret;
> +
> +	drm->mode_config.funcs = &sharp_ls027b7dh01_mode_config_funcs;
> +	priv->display_mode = &sharp_ls027b7dh01_mode;
> +
> +	/*
> +	 * write_buf_size:
> +	 *
> +	 * hdisplay * vdisplay / 8 => 1 bit per Pixel.
> +	 * 2 * vdisplay => line number byte + trailer dummy byte for every line.
> +	 * 2 => write command byte + final trailer dummy byte.
> +	 */
> +	write_buf_size = priv->display_mode->hdisplay * priv->display_mode->vdisplay / 8
> +			 + 2 * priv->display_mode->vdisplay + 2;
> +
> +	priv->write_buf = devm_kzalloc(dev, write_buf_size, GFP_KERNEL);
> +	if (!priv->write_buf)
> +		return -ENOMEM;
> +
> +	drm->mode_config.min_width = priv->display_mode->hdisplay;
> +	drm->mode_config.max_width = priv->display_mode->hdisplay;
> +	drm->mode_config.min_height = priv->display_mode->vdisplay;
> +	drm->mode_config.max_height = priv->display_mode->vdisplay;
> +
> +	ret = drm_connector_init(drm, &priv->connector,
> +				 &sharp_ls027b7dh01_connector_funcs,
> +				 DRM_MODE_CONNECTOR_SPI);
> +	if (ret)
> +		return ret;
> +
> +	drm_connector_helper_add(&priv->connector,
> +				 &sharp_ls027b7dh01_connector_hfuncs);
> +
> +	ret = drm_simple_display_pipe_init(drm, &priv->pipe,
> +					   &sharp_ls027b7dh01_pipe_funcs,
> +					   sharp_ls027b7dh01_formats,
> +					   ARRAY_SIZE(sharp_ls027b7dh01_formats),
> +					   NULL, &priv->connector);
> +	if (ret)
> +		return ret;
> +
> +	drm_plane_enable_fb_damage_clips(&priv->pipe.plane);
> +	drm_mode_config_reset(drm);
> +
> +	ret = drm_dev_register(drm, 0);
> +	if (ret)
> +		return ret;
> +
> +	drm_fbdev_generic_setup(drm, 0);
> +
> +	return 0;
> +}
> +
> +static void sharp_ls027b7dh01_remove(struct spi_device *spi)
> +{
> +	struct sharp_ls027b7dh01 *priv = spi_get_drvdata(spi);
> +
> +	drm_dev_unplug(&priv->drm);
> +	drm_atomic_helper_shutdown(&priv->drm);
> +}
> +
> +static void sharp_ls027b7dh01_shutdown(struct spi_device *spi)
> +{
> +	struct sharp_ls027b7dh01 *priv = spi_get_drvdata(spi);
> +
> +	drm_atomic_helper_shutdown(&priv->drm);
> +}
> +
> +static const struct spi_device_id sharp_ls027b7dh01_ids[] = {
> +	{ "ls027b7dh01" },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(spi, sharp_ls027b7dh01_ids);
> +
> +static const struct of_device_id sharp_ls027b7dh01_of_match[] = {
> +	{ .compatible = "sharp,ls027b7dh01", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, sharp_ls027b7dh01_of_match);
> +
> +static struct spi_driver sharp_ls027b7dh01_spi_driver = {
> +	.probe = sharp_ls027b7dh01_probe,
> +	.remove = sharp_ls027b7dh01_remove,
> +	.shutdown = sharp_ls027b7dh01_shutdown,
> +	.id_table = sharp_ls027b7dh01_ids,
> +	.driver = {
> +		.name = "sharp-ls027b7dh01",
> +		.of_match_table = sharp_ls027b7dh01_of_match,
> +	},
> +};
> +module_spi_driver(sharp_ls027b7dh01_spi_driver);
> +
> +MODULE_AUTHOR("Andrew D'Angelo");
> +MODULE_AUTHOR("Mehdi Djait <mehdi.djait@bootlin.com>");
> +MODULE_DESCRIPTION("Sharp LS027B7DH01 Driver");
> +MODULE_LICENSE("GPL");

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH 2/2] drm/tiny: Add driver for the sharp LS027B7DH01 Memory LCD

[Paul Kocialkowski]

[-- Attachment #1: Type: text/plain, Size: 17921 bytes --]

Hi Mehdi,

See a few comments about this new driver below.

On Wed 29 Nov 23, 15:29, Mehdi Djait wrote:
> Introduce a DRM driver for the sharp LS027B7DH01 Memory LCD.
> 
> LS027B7DH01 is a 2.7" 400x240 monochrome display connected to a SPI bus.
> The drivers

Typo here: "driver".

> implements the Multiple Lines Data Update Mode.

This sounds like a fancy vendor-specific way to say that you are updating all
the lines at once instead of a target crop rectangle. The wording could be
clarified here and you shouldn't assume people are familiar with the vendor's
terminology.

> External COM inversion is enabled using a PWM signal as input.

What is this external com inversion about and why should we care about it?

> Signed-off-by: Mehdi Djait <mehdi.djait@bootlin.com>
> ---
>  MAINTAINERS                              |   7 +
>  drivers/gpu/drm/tiny/Kconfig             |   8 +
>  drivers/gpu/drm/tiny/Makefile            |   1 +
>  drivers/gpu/drm/tiny/sharp-ls027b7dh01.c | 411 +++++++++++++++++++++++
>  4 files changed, 427 insertions(+)
>  create mode 100644 drivers/gpu/drm/tiny/sharp-ls027b7dh01.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 012df8ccf34e..fb859698bd3d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6832,6 +6832,13 @@ S:	Maintained
>  F:	Documentation/devicetree/bindings/display/panel/samsung,s6d7aa0.yaml
>  F:	drivers/gpu/drm/panel/panel-samsung-s6d7aa0.c
>  
> +DRM DRIVER FOR SHARP LS027B7DH01 Memory LCD
> +M:	Mehdi Djait <mehdi.djait@bootlin.com>
> +S:	Maintained
> +T:	git git://anongit.freedesktop.org/drm/drm-misc
> +F:	Documentation/devicetree/bindings/display/sharp,ls027b7dh01.yaml
> +F:	drivers/gpu/drm/tiny/sharp-ls027b7dh01.c
> +
>  DRM DRIVER FOR SITRONIX ST7586 PANELS
>  M:	David Lechner <david@lechnology.com>
>  S:	Maintained
> diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig
> index f6889f649bc1..a2ade06403ca 100644
> --- a/drivers/gpu/drm/tiny/Kconfig
> +++ b/drivers/gpu/drm/tiny/Kconfig
> @@ -186,6 +186,14 @@ config TINYDRM_REPAPER
>  
>  	  If M is selected the module will be called repaper.
>  
> +config TINYDRM_SHARP_LS027B7DH01
> +	tristate "DRM support for SHARP LS027B7DH01 display"
> +	depends on DRM && SPI
> +	select DRM_KMS_HELPER
> +	select DRM_GEM_DMA_HELPER
> +	help
> +	  DRM driver for the SHARP LS027B7DD01 LCD display.
> +
>  config TINYDRM_ST7586
>  	tristate "DRM support for Sitronix ST7586 display panels"
>  	depends on DRM && SPI
> diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile
> index 76dde89a044b..b05df3afb231 100644
> --- a/drivers/gpu/drm/tiny/Makefile
> +++ b/drivers/gpu/drm/tiny/Makefile
> @@ -14,5 +14,6 @@ obj-$(CONFIG_TINYDRM_ILI9341)		+= ili9341.o
>  obj-$(CONFIG_TINYDRM_ILI9486)		+= ili9486.o
>  obj-$(CONFIG_TINYDRM_MI0283QT)		+= mi0283qt.o
>  obj-$(CONFIG_TINYDRM_REPAPER)		+= repaper.o
> +obj-$(CONFIG_TINYDRM_SHARP_LS027B7DH01)	+= sharp-ls027b7dh01.o

Looks like other drivers in this directory don't include the vendor name as
a prefix so it would be best to do the same for consistency.

On the other hand it looks like drm/panel does it always. While the two are
not consistent with eachother, I think it's best to keep local consistency,
so align with what other files are doing in the same directory.

>  obj-$(CONFIG_TINYDRM_ST7586)		+= st7586.o
>  obj-$(CONFIG_TINYDRM_ST7735R)		+= st7735r.o
> diff --git a/drivers/gpu/drm/tiny/sharp-ls027b7dh01.c b/drivers/gpu/drm/tiny/sharp-ls027b7dh01.c
> new file mode 100644
> index 000000000000..2f58865a5c78
> --- /dev/null
> +++ b/drivers/gpu/drm/tiny/sharp-ls027b7dh01.c
> @@ -0,0 +1,411 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Sharp LS027B7DH01 Memory Display Driver
> + *
> + * Copyright (C) 2023 Andrew D'Angelo
> + * Copyright (C) 2023 Mehdi Djait <mehdi.djait@bootlin.com>
> + *
> + * The Sharp Memory LCD requires an alternating signal to prevent the buildup of
> + * a DC bias that would result in a Display that no longer can be updated. Two
> + * modes for the generation of this signal are supported:
> + *
> + * Software, EXTMODE = Low: toggling the BIT(1) of the Command and writing it at
> + * least once a second to the display.
> + *
> + * Hardware, EXTMODE = High: the alternating signal should be supplied on the
> + * EXTCOMIN pin.
> + *
> + * In this driver the Hardware mode is implemented with a PWM signal.
> + */

I would put this comment in a more significant place than in the first header
block. For instance around the place you are dealing with this feature.

> +
> +#include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/pwm.h>
> +#include <linux/spi/spi.h>
> +
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_damage_helper.h>
> +#include <drm/drm_drv.h>
> +#include <drm/drm_fbdev_generic.h>
> +#include <drm/drm_fb_dma_helper.h>
> +#include <drm/drm_format_helper.h>
> +#include <drm/drm_framebuffer.h>
> +#include <drm/drm_gem_atomic_helper.h>
> +#include <drm/drm_gem_dma_helper.h>
> +#include <drm/drm_gem_framebuffer_helper.h>
> +#include <drm/drm_modes.h>
> +#include <drm/drm_probe_helper.h>
> +#include <drm/drm_simple_kms_helper.h>
> +
> +#define CMD_WRITE BIT(0)
> +#define CMD_CLEAR BIT(2)
> +
> +struct sharp_ls027b7dh01 {
> +	struct spi_device *spi;
> +
> +	struct drm_device drm;
> +	struct drm_connector connector;
> +	struct drm_simple_display_pipe pipe;
> +	const struct drm_display_mode *display_mode;
> +
> +	struct gpio_desc *enable_gpio;
> +	struct pwm_device *extcomin_pwm;
> +
> +	u8 *write_buf;
> +};
> +
> +static inline struct sharp_ls027b7dh01 *drm_to_priv(struct drm_device *drm)
> +{
> +	return container_of(drm, struct sharp_ls027b7dh01, drm);
> +}
> +
> +/**
> + * sharp_ls027b7dh01_add_headers - Add the Sharp LS027B7DH01 specific headers
> + * @write_buf: Buffer to write
> + * @clip: DRM clip rectangle area to write
> + * @dst_pitch: Pitch of the write buffer
> + *

Why are you providing in-tree documentation for this function and not any other
function? The rest of the comment is important but you can drop the part above.

> + * This function adds the SHARP LS027B7DH01 specific headers to the buffer for
> + * the Multiple Lines Write Mode:
> + * - The first byte will contain the write command.
> + * - Every line data starts with the line number and ends with a dummy zero
> + *   trailer byte. It should be noted here that the line numbers are indexed
> + *   from 1.
> + *
> + * Returns the size of the buffer to write to the display.
> + */
> +static size_t sharp_ls027b7dh01_add_headers(u8 *write_buf,
> +					    const struct drm_rect *clip,
> +					    const unsigned int dst_pitch)
> +{
> +	u8 line_num = clip->y1 + 1;
> +	unsigned int lines = drm_rect_height(clip);
> +	unsigned int y;
> +
> +	write_buf[0] = CMD_WRITE;
> +	write_buf[1] = line_num++;
> +
> +	for (y = 1; y < lines; y++) {
> +		write_buf[y * dst_pitch] = 0;
> +		write_buf[y * dst_pitch + 1] = line_num++;
> +	}
> +
> +	write_buf[lines * dst_pitch] = 0;
> +	write_buf[lines * dst_pitch + 1] = 0;
> +
> +	return lines * dst_pitch + 2;
> +}
> +
> +static int sharp_ls027b7dh01_prepare_buf(struct sharp_ls027b7dh01 *priv,
> +					 u8 *write_buf,
> +					 size_t *data_len,
> +					 struct drm_framebuffer *fb,
> +					 const struct drm_rect *clip)
> +{
> +	struct drm_gem_dma_object *dma_obj;
> +	struct iosys_map dst, vmap;
> +	unsigned int dst_pitch;
> +	int ret;
> +
> +	/* Leave 2 bytes to hold the line number and the trailer dummy byte. */
> +	dst_pitch = (drm_rect_width(clip) / 8) + 2;

What happens if the drm_rect_width() is not dividable by 8?
Sounds like a case for DIV_ROUND_UP (and it should be tested to ensure that
this is what the hardware expects).

> +
> +	dma_obj = drm_fb_dma_get_gem_obj(fb, 0);
> +
> +	ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
> +	if (ret)
> +		return ret;
> +
> +	iosys_map_set_vaddr(&dst, &write_buf[2]);
> +	iosys_map_set_vaddr(&vmap, dma_obj->vaddr);
> +
> +	drm_fb_xrgb8888_to_mono(&dst, &dst_pitch, &vmap, fb, clip);
> +
> +	drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
> +
> +	*data_len = sharp_ls027b7dh01_add_headers(write_buf, clip, dst_pitch);
> +
> +	return 0;
> +}
> +
> +static int sharp_ls027b7dh01_fb_damaged(struct drm_framebuffer *fb,
> +					const struct drm_rect *rect)
> +{
> +	struct drm_rect clip;
> +	struct sharp_ls027b7dh01 *priv;
> +	size_t data_len;
> +	int drm_index;
> +	int ret;
> +
> +	clip.x1 = 0;
> +	clip.x2 = fb->width;
> +	clip.y1 = rect->y1;
> +	clip.y2 = rect->y2;
> +
> +	priv = drm_to_priv(fb->dev);
> +
> +	if (!drm_dev_enter(fb->dev, &drm_index))
> +		return -ENODEV;
> +
> +	ret = sharp_ls027b7dh01_prepare_buf(priv, priv->write_buf, &data_len, fb, &clip);
> +	if (ret)
> +		goto exit;
> +
> +	ret = spi_write(priv->spi, priv->write_buf, data_len);
> +
> +exit:
> +	drm_dev_exit(drm_index);
> +
> +	return ret;
> +}
> +
> +static void sharp_ls027b7dh01_pipe_update(struct drm_simple_display_pipe *pipe,
> +					  struct drm_plane_state *old_state)
> +{
> +	struct drm_plane_state *state = pipe->plane.state;
> +	struct drm_rect rect;
> +
> +	if (!pipe->crtc.state->active)
> +		return;
> +
> +	if (drm_atomic_helper_damage_merged(old_state, state, &rect))
> +		sharp_ls027b7dh01_fb_damaged(state->fb, &rect);
> +}
> +
> +static void sharp_ls027b7dh01_pipe_disable(struct drm_simple_display_pipe *pipe)
> +{
> +	struct sharp_ls027b7dh01 *priv;
> +
> +	priv = drm_to_priv(pipe->crtc.dev);
> +	gpiod_set_value(priv->enable_gpio, 0);
> +}
> +
> +static int sharp_ls027b7dh01_clear_display(struct sharp_ls027b7dh01 *priv)
> +{
> +	u8 clear_buf[2] = { CMD_CLEAR, 0 };
> +
> +	return spi_write(priv->spi, clear_buf, sizeof(clear_buf));
> +}
> +
> +static int sharp_ls027b7dh01_pwm_enable(struct sharp_ls027b7dh01 *priv)
> +{
> +	struct device *dev = &priv->spi->dev;
> +	struct pwm_state pwmstate;
> +
> +	priv->extcomin_pwm = devm_pwm_get(dev, NULL);

You should almost certainly do that just once in probe, not every time you want
to enable the pwm. This might increase some internal refcount which won't be
balanced.

> +	if (IS_ERR(priv->extcomin_pwm)) {
> +		dev_err(dev, "Could not get EXTCOMIN pwm\n");
> +		return PTR_ERR(priv->extcomin_pwm);
> +	}
> +
> +	pwm_init_state(priv->extcomin_pwm, &pwmstate);
> +	pwm_set_relative_duty_cycle(&pwmstate, 50, 100);
> +	pwm_apply_state(priv->extcomin_pwm, &pwmstate);
> +
> +	pwm_enable(priv->extcomin_pwm);
> +
> +	return 0;
> +}
> +
> +static void sharp_ls027b7dh01_pipe_enable(struct drm_simple_display_pipe *pipe,
> +					  struct drm_crtc_state *crtc_state,
> +					  struct drm_plane_state *plane_state)
> +{
> +	struct sharp_ls027b7dh01 *priv;
> +	int ret, drm_idx;
> +
> +	priv = drm_to_priv(pipe->crtc.dev);
> +
> +	if (!drm_dev_enter(pipe->crtc.dev, &drm_idx))
> +		return;
> +
> +	gpiod_set_value(priv->enable_gpio, 1);
> +

Does the panel documentation specify some delay between setting the enable GPIO
and communicating with it? It's rarely instantaneous.

> +	ret = sharp_ls027b7dh01_clear_display(priv);
> +	if (ret)
> +		goto exit;
> +
> +	sharp_ls027b7dh01_pwm_enable(priv);
> +
> +exit:
> +	drm_dev_exit(drm_idx);
> +}
> +
> +static const struct drm_simple_display_pipe_funcs sharp_ls027b7dh01_pipe_funcs = {
> +	.enable = sharp_ls027b7dh01_pipe_enable,
> +	.disable = sharp_ls027b7dh01_pipe_disable,
> +	.update = sharp_ls027b7dh01_pipe_update,
> +};
> +
> +static int sharp_ls027b7dh01_connector_get_modes(struct drm_connector *connector)
> +{
> +	struct sharp_ls027b7dh01 *priv = drm_to_priv(connector->dev);
> +
> +	return drm_connector_helper_get_modes_fixed(connector, priv->display_mode);
> +}
> +
> +static const struct drm_connector_helper_funcs sharp_ls027b7dh01_connector_hfuncs = {
> +	.get_modes = sharp_ls027b7dh01_connector_get_modes,
> +};
> +
> +static const struct drm_connector_funcs sharp_ls027b7dh01_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 sharp_ls027b7dh01_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 uint32_t sharp_ls027b7dh01_formats[] = {
> +	DRM_FORMAT_XRGB8888,
> +};
> +
> +static const struct drm_display_mode sharp_ls027b7dh01_mode = {
> +	DRM_SIMPLE_MODE(400, 240, 59, 35),
> +};
> +
> +DEFINE_DRM_GEM_DMA_FOPS(sharp_ls027b7dh01_fops);
> +
> +static const struct drm_driver sharp_ls027b7dh01_drm_driver = {
> +	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
> +	.fops			= &sharp_ls027b7dh01_fops,
> +	DRM_GEM_DMA_DRIVER_OPS_VMAP,
> +	.name			= "sharp_ls027b7dh01",
> +	.desc			= "Sharp ls027b7dh01 Memory LCD",
> +	.date			= "20231129",
> +	.major			= 1,
> +	.minor			= 0,
> +};
> +
> +static int sharp_ls027b7dh01_probe(struct spi_device *spi)
> +{
> +	struct device *dev = &spi->dev;
> +	struct sharp_ls027b7dh01 *priv;
> +	struct drm_device *drm;
> +	unsigned int write_buf_size;
> +	int ret;
> +
> +	if (!dev->coherent_dma_mask) {
> +		ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(32));
> +		if (ret)
> +			return dev_err_probe(dev, ret, "Failed to set dma mask\n");
> +	}

Is there a particular need for this? I don't see where you're dealing with
DMA at all.

> +
> +	priv = devm_drm_dev_alloc(dev, &sharp_ls027b7dh01_drm_driver,
> +				  struct sharp_ls027b7dh01, drm);
> +	if (IS_ERR(priv))
> +		return PTR_ERR(priv);
> +
> +	spi_set_drvdata(spi, priv);
> +	priv->spi = spi;
> +
> +	priv->enable_gpio = devm_gpiod_get(dev, "enable", GPIOD_OUT_HIGH);
> +	if (IS_ERR(priv->enable_gpio))
> +		return dev_err_probe(dev, PTR_ERR(priv->enable_gpio),
> +				     "Failed to get GPIO 'enable'\n");
> +
> +	drm = &priv->drm;
> +	ret = drmm_mode_config_init(drm);
> +	if (ret)
> +		return ret;
> +
> +	drm->mode_config.funcs = &sharp_ls027b7dh01_mode_config_funcs;
> +	priv->display_mode = &sharp_ls027b7dh01_mode;

It's a bit redundant to store it in priv if there's just one mode supported.

> +
> +	/*
> +	 * write_buf_size:
> +	 *
> +	 * hdisplay * vdisplay / 8 => 1 bit per Pixel.
> +	 * 2 * vdisplay => line number byte + trailer dummy byte for every line.
> +	 * 2 => write command byte + final trailer dummy byte.
> +	 */
> +	write_buf_size = priv->display_mode->hdisplay * priv->display_mode->vdisplay / 8
> +			 + 2 * priv->display_mode->vdisplay + 2;

Same issue about priv->display_mode->hdisplay * priv->display_mode->vdisplay
being dividable by 8. In your case you just have a single mode which doesn't
have the issue, but this code looks generic laid out like this, so it should
take care of all possible cases.
> +
> +	priv->write_buf = devm_kzalloc(dev, write_buf_size, GFP_KERNEL);
> +	if (!priv->write_buf)
> +		return -ENOMEM;
> +
> +	drm->mode_config.min_width = priv->display_mode->hdisplay;
> +	drm->mode_config.max_width = priv->display_mode->hdisplay;
> +	drm->mode_config.min_height = priv->display_mode->vdisplay;
> +	drm->mode_config.max_height = priv->display_mode->vdisplay;
> +
> +	ret = drm_connector_init(drm, &priv->connector,
> +				 &sharp_ls027b7dh01_connector_funcs,
> +				 DRM_MODE_CONNECTOR_SPI);
> +	if (ret)
> +		return ret;
> +
> +	drm_connector_helper_add(&priv->connector,
> +				 &sharp_ls027b7dh01_connector_hfuncs);
> +
> +	ret = drm_simple_display_pipe_init(drm, &priv->pipe,
> +					   &sharp_ls027b7dh01_pipe_funcs,
> +					   sharp_ls027b7dh01_formats,
> +					   ARRAY_SIZE(sharp_ls027b7dh01_formats),
> +					   NULL, &priv->connector);
> +	if (ret)
> +		return ret;
> +
> +	drm_plane_enable_fb_damage_clips(&priv->pipe.plane);
> +	drm_mode_config_reset(drm);
> +
> +	ret = drm_dev_register(drm, 0);
> +	if (ret)
> +		return ret;
> +
> +	drm_fbdev_generic_setup(drm, 0);
> +
> +	return 0;
> +}
> +
> +static void sharp_ls027b7dh01_remove(struct spi_device *spi)
> +{
> +	struct sharp_ls027b7dh01 *priv = spi_get_drvdata(spi);
> +
> +	drm_dev_unplug(&priv->drm);
> +	drm_atomic_helper_shutdown(&priv->drm);
> +}
> +
> +static void sharp_ls027b7dh01_shutdown(struct spi_device *spi)
> +{
> +	struct sharp_ls027b7dh01 *priv = spi_get_drvdata(spi);
> +
> +	drm_atomic_helper_shutdown(&priv->drm);
> +}
> +
> +static const struct spi_device_id sharp_ls027b7dh01_ids[] = {
> +	{ "ls027b7dh01" },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(spi, sharp_ls027b7dh01_ids);
> +
> +static const struct of_device_id sharp_ls027b7dh01_of_match[] = {
> +	{ .compatible = "sharp,ls027b7dh01", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, sharp_ls027b7dh01_of_match);
> +
> +static struct spi_driver sharp_ls027b7dh01_spi_driver = {
> +	.probe = sharp_ls027b7dh01_probe,
> +	.remove = sharp_ls027b7dh01_remove,
> +	.shutdown = sharp_ls027b7dh01_shutdown,
> +	.id_table = sharp_ls027b7dh01_ids,
> +	.driver = {
> +		.name = "sharp-ls027b7dh01",
> +		.of_match_table = sharp_ls027b7dh01_of_match,
> +	},
> +};
> +module_spi_driver(sharp_ls027b7dh01_spi_driver);
> +
> +MODULE_AUTHOR("Andrew D'Angelo");
> +MODULE_AUTHOR("Mehdi Djait <mehdi.djait@bootlin.com>");
> +MODULE_DESCRIPTION("Sharp LS027B7DH01 Driver");
> +MODULE_LICENSE("GPL");
> -- 
> 2.41.0
> 

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

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

Re: [PATCH 2/2] drm/tiny: Add driver for the sharp LS027B7DH01 Memory LCD

[Paul Kocialkowski]

[-- Attachment #1: Type: text/plain, Size: 17921 bytes --]

Hi Mehdi,

See a few comments about this new driver below.

On Wed 29 Nov 23, 15:29, Mehdi Djait wrote:
> Introduce a DRM driver for the sharp LS027B7DH01 Memory LCD.
> 
> LS027B7DH01 is a 2.7" 400x240 monochrome display connected to a SPI bus.
> The drivers

Typo here: "driver".

> implements the Multiple Lines Data Update Mode.

This sounds like a fancy vendor-specific way to say that you are updating all
the lines at once instead of a target crop rectangle. The wording could be
clarified here and you shouldn't assume people are familiar with the vendor's
terminology.

> External COM inversion is enabled using a PWM signal as input.

What is this external com inversion about and why should we care about it?

> Signed-off-by: Mehdi Djait <mehdi.djait@bootlin.com>
> ---
>  MAINTAINERS                              |   7 +
>  drivers/gpu/drm/tiny/Kconfig             |   8 +
>  drivers/gpu/drm/tiny/Makefile            |   1 +
>  drivers/gpu/drm/tiny/sharp-ls027b7dh01.c | 411 +++++++++++++++++++++++
>  4 files changed, 427 insertions(+)
>  create mode 100644 drivers/gpu/drm/tiny/sharp-ls027b7dh01.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 012df8ccf34e..fb859698bd3d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6832,6 +6832,13 @@ S:	Maintained
>  F:	Documentation/devicetree/bindings/display/panel/samsung,s6d7aa0.yaml
>  F:	drivers/gpu/drm/panel/panel-samsung-s6d7aa0.c
>  
> +DRM DRIVER FOR SHARP LS027B7DH01 Memory LCD
> +M:	Mehdi Djait <mehdi.djait@bootlin.com>
> +S:	Maintained
> +T:	git git://anongit.freedesktop.org/drm/drm-misc
> +F:	Documentation/devicetree/bindings/display/sharp,ls027b7dh01.yaml
> +F:	drivers/gpu/drm/tiny/sharp-ls027b7dh01.c
> +
>  DRM DRIVER FOR SITRONIX ST7586 PANELS
>  M:	David Lechner <david@lechnology.com>
>  S:	Maintained
> diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig
> index f6889f649bc1..a2ade06403ca 100644
> --- a/drivers/gpu/drm/tiny/Kconfig
> +++ b/drivers/gpu/drm/tiny/Kconfig
> @@ -186,6 +186,14 @@ config TINYDRM_REPAPER
>  
>  	  If M is selected the module will be called repaper.
>  
> +config TINYDRM_SHARP_LS027B7DH01
> +	tristate "DRM support for SHARP LS027B7DH01 display"
> +	depends on DRM && SPI
> +	select DRM_KMS_HELPER
> +	select DRM_GEM_DMA_HELPER
> +	help
> +	  DRM driver for the SHARP LS027B7DD01 LCD display.
> +
>  config TINYDRM_ST7586
>  	tristate "DRM support for Sitronix ST7586 display panels"
>  	depends on DRM && SPI
> diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile
> index 76dde89a044b..b05df3afb231 100644
> --- a/drivers/gpu/drm/tiny/Makefile
> +++ b/drivers/gpu/drm/tiny/Makefile
> @@ -14,5 +14,6 @@ obj-$(CONFIG_TINYDRM_ILI9341)		+= ili9341.o
>  obj-$(CONFIG_TINYDRM_ILI9486)		+= ili9486.o
>  obj-$(CONFIG_TINYDRM_MI0283QT)		+= mi0283qt.o
>  obj-$(CONFIG_TINYDRM_REPAPER)		+= repaper.o
> +obj-$(CONFIG_TINYDRM_SHARP_LS027B7DH01)	+= sharp-ls027b7dh01.o

Looks like other drivers in this directory don't include the vendor name as
a prefix so it would be best to do the same for consistency.

On the other hand it looks like drm/panel does it always. While the two are
not consistent with eachother, I think it's best to keep local consistency,
so align with what other files are doing in the same directory.

>  obj-$(CONFIG_TINYDRM_ST7586)		+= st7586.o
>  obj-$(CONFIG_TINYDRM_ST7735R)		+= st7735r.o
> diff --git a/drivers/gpu/drm/tiny/sharp-ls027b7dh01.c b/drivers/gpu/drm/tiny/sharp-ls027b7dh01.c
> new file mode 100644
> index 000000000000..2f58865a5c78
> --- /dev/null
> +++ b/drivers/gpu/drm/tiny/sharp-ls027b7dh01.c
> @@ -0,0 +1,411 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Sharp LS027B7DH01 Memory Display Driver
> + *
> + * Copyright (C) 2023 Andrew D'Angelo
> + * Copyright (C) 2023 Mehdi Djait <mehdi.djait@bootlin.com>
> + *
> + * The Sharp Memory LCD requires an alternating signal to prevent the buildup of
> + * a DC bias that would result in a Display that no longer can be updated. Two
> + * modes for the generation of this signal are supported:
> + *
> + * Software, EXTMODE = Low: toggling the BIT(1) of the Command and writing it at
> + * least once a second to the display.
> + *
> + * Hardware, EXTMODE = High: the alternating signal should be supplied on the
> + * EXTCOMIN pin.
> + *
> + * In this driver the Hardware mode is implemented with a PWM signal.
> + */

I would put this comment in a more significant place than in the first header
block. For instance around the place you are dealing with this feature.

> +
> +#include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/pwm.h>
> +#include <linux/spi/spi.h>
> +
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_damage_helper.h>
> +#include <drm/drm_drv.h>
> +#include <drm/drm_fbdev_generic.h>
> +#include <drm/drm_fb_dma_helper.h>
> +#include <drm/drm_format_helper.h>
> +#include <drm/drm_framebuffer.h>
> +#include <drm/drm_gem_atomic_helper.h>
> +#include <drm/drm_gem_dma_helper.h>
> +#include <drm/drm_gem_framebuffer_helper.h>
> +#include <drm/drm_modes.h>
> +#include <drm/drm_probe_helper.h>
> +#include <drm/drm_simple_kms_helper.h>
> +
> +#define CMD_WRITE BIT(0)
> +#define CMD_CLEAR BIT(2)
> +
> +struct sharp_ls027b7dh01 {
> +	struct spi_device *spi;
> +
> +	struct drm_device drm;
> +	struct drm_connector connector;
> +	struct drm_simple_display_pipe pipe;
> +	const struct drm_display_mode *display_mode;
> +
> +	struct gpio_desc *enable_gpio;
> +	struct pwm_device *extcomin_pwm;
> +
> +	u8 *write_buf;
> +};
> +
> +static inline struct sharp_ls027b7dh01 *drm_to_priv(struct drm_device *drm)
> +{
> +	return container_of(drm, struct sharp_ls027b7dh01, drm);
> +}
> +
> +/**
> + * sharp_ls027b7dh01_add_headers - Add the Sharp LS027B7DH01 specific headers
> + * @write_buf: Buffer to write
> + * @clip: DRM clip rectangle area to write
> + * @dst_pitch: Pitch of the write buffer
> + *

Why are you providing in-tree documentation for this function and not any other
function? The rest of the comment is important but you can drop the part above.

> + * This function adds the SHARP LS027B7DH01 specific headers to the buffer for
> + * the Multiple Lines Write Mode:
> + * - The first byte will contain the write command.
> + * - Every line data starts with the line number and ends with a dummy zero
> + *   trailer byte. It should be noted here that the line numbers are indexed
> + *   from 1.
> + *
> + * Returns the size of the buffer to write to the display.
> + */
> +static size_t sharp_ls027b7dh01_add_headers(u8 *write_buf,
> +					    const struct drm_rect *clip,
> +					    const unsigned int dst_pitch)
> +{
> +	u8 line_num = clip->y1 + 1;
> +	unsigned int lines = drm_rect_height(clip);
> +	unsigned int y;
> +
> +	write_buf[0] = CMD_WRITE;
> +	write_buf[1] = line_num++;
> +
> +	for (y = 1; y < lines; y++) {
> +		write_buf[y * dst_pitch] = 0;
> +		write_buf[y * dst_pitch + 1] = line_num++;
> +	}
> +
> +	write_buf[lines * dst_pitch] = 0;
> +	write_buf[lines * dst_pitch + 1] = 0;
> +
> +	return lines * dst_pitch + 2;
> +}
> +
> +static int sharp_ls027b7dh01_prepare_buf(struct sharp_ls027b7dh01 *priv,
> +					 u8 *write_buf,
> +					 size_t *data_len,
> +					 struct drm_framebuffer *fb,
> +					 const struct drm_rect *clip)
> +{
> +	struct drm_gem_dma_object *dma_obj;
> +	struct iosys_map dst, vmap;
> +	unsigned int dst_pitch;
> +	int ret;
> +
> +	/* Leave 2 bytes to hold the line number and the trailer dummy byte. */
> +	dst_pitch = (drm_rect_width(clip) / 8) + 2;

What happens if the drm_rect_width() is not dividable by 8?
Sounds like a case for DIV_ROUND_UP (and it should be tested to ensure that
this is what the hardware expects).

> +
> +	dma_obj = drm_fb_dma_get_gem_obj(fb, 0);
> +
> +	ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
> +	if (ret)
> +		return ret;
> +
> +	iosys_map_set_vaddr(&dst, &write_buf[2]);
> +	iosys_map_set_vaddr(&vmap, dma_obj->vaddr);
> +
> +	drm_fb_xrgb8888_to_mono(&dst, &dst_pitch, &vmap, fb, clip);
> +
> +	drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
> +
> +	*data_len = sharp_ls027b7dh01_add_headers(write_buf, clip, dst_pitch);
> +
> +	return 0;
> +}
> +
> +static int sharp_ls027b7dh01_fb_damaged(struct drm_framebuffer *fb,
> +					const struct drm_rect *rect)
> +{
> +	struct drm_rect clip;
> +	struct sharp_ls027b7dh01 *priv;
> +	size_t data_len;
> +	int drm_index;
> +	int ret;
> +
> +	clip.x1 = 0;
> +	clip.x2 = fb->width;
> +	clip.y1 = rect->y1;
> +	clip.y2 = rect->y2;
> +
> +	priv = drm_to_priv(fb->dev);
> +
> +	if (!drm_dev_enter(fb->dev, &drm_index))
> +		return -ENODEV;
> +
> +	ret = sharp_ls027b7dh01_prepare_buf(priv, priv->write_buf, &data_len, fb, &clip);
> +	if (ret)
> +		goto exit;
> +
> +	ret = spi_write(priv->spi, priv->write_buf, data_len);
> +
> +exit:
> +	drm_dev_exit(drm_index);
> +
> +	return ret;
> +}
> +
> +static void sharp_ls027b7dh01_pipe_update(struct drm_simple_display_pipe *pipe,
> +					  struct drm_plane_state *old_state)
> +{
> +	struct drm_plane_state *state = pipe->plane.state;
> +	struct drm_rect rect;
> +
> +	if (!pipe->crtc.state->active)
> +		return;
> +
> +	if (drm_atomic_helper_damage_merged(old_state, state, &rect))
> +		sharp_ls027b7dh01_fb_damaged(state->fb, &rect);
> +}
> +
> +static void sharp_ls027b7dh01_pipe_disable(struct drm_simple_display_pipe *pipe)
> +{
> +	struct sharp_ls027b7dh01 *priv;
> +
> +	priv = drm_to_priv(pipe->crtc.dev);
> +	gpiod_set_value(priv->enable_gpio, 0);
> +}
> +
> +static int sharp_ls027b7dh01_clear_display(struct sharp_ls027b7dh01 *priv)
> +{
> +	u8 clear_buf[2] = { CMD_CLEAR, 0 };
> +
> +	return spi_write(priv->spi, clear_buf, sizeof(clear_buf));
> +}
> +
> +static int sharp_ls027b7dh01_pwm_enable(struct sharp_ls027b7dh01 *priv)
> +{
> +	struct device *dev = &priv->spi->dev;
> +	struct pwm_state pwmstate;
> +
> +	priv->extcomin_pwm = devm_pwm_get(dev, NULL);

You should almost certainly do that just once in probe, not every time you want
to enable the pwm. This might increase some internal refcount which won't be
balanced.

> +	if (IS_ERR(priv->extcomin_pwm)) {
> +		dev_err(dev, "Could not get EXTCOMIN pwm\n");
> +		return PTR_ERR(priv->extcomin_pwm);
> +	}
> +
> +	pwm_init_state(priv->extcomin_pwm, &pwmstate);
> +	pwm_set_relative_duty_cycle(&pwmstate, 50, 100);
> +	pwm_apply_state(priv->extcomin_pwm, &pwmstate);
> +
> +	pwm_enable(priv->extcomin_pwm);
> +
> +	return 0;
> +}
> +
> +static void sharp_ls027b7dh01_pipe_enable(struct drm_simple_display_pipe *pipe,
> +					  struct drm_crtc_state *crtc_state,
> +					  struct drm_plane_state *plane_state)
> +{
> +	struct sharp_ls027b7dh01 *priv;
> +	int ret, drm_idx;
> +
> +	priv = drm_to_priv(pipe->crtc.dev);
> +
> +	if (!drm_dev_enter(pipe->crtc.dev, &drm_idx))
> +		return;
> +
> +	gpiod_set_value(priv->enable_gpio, 1);
> +

Does the panel documentation specify some delay between setting the enable GPIO
and communicating with it? It's rarely instantaneous.

> +	ret = sharp_ls027b7dh01_clear_display(priv);
> +	if (ret)
> +		goto exit;
> +
> +	sharp_ls027b7dh01_pwm_enable(priv);
> +
> +exit:
> +	drm_dev_exit(drm_idx);
> +}
> +
> +static const struct drm_simple_display_pipe_funcs sharp_ls027b7dh01_pipe_funcs = {
> +	.enable = sharp_ls027b7dh01_pipe_enable,
> +	.disable = sharp_ls027b7dh01_pipe_disable,
> +	.update = sharp_ls027b7dh01_pipe_update,
> +};
> +
> +static int sharp_ls027b7dh01_connector_get_modes(struct drm_connector *connector)
> +{
> +	struct sharp_ls027b7dh01 *priv = drm_to_priv(connector->dev);
> +
> +	return drm_connector_helper_get_modes_fixed(connector, priv->display_mode);
> +}
> +
> +static const struct drm_connector_helper_funcs sharp_ls027b7dh01_connector_hfuncs = {
> +	.get_modes = sharp_ls027b7dh01_connector_get_modes,
> +};
> +
> +static const struct drm_connector_funcs sharp_ls027b7dh01_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 sharp_ls027b7dh01_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 uint32_t sharp_ls027b7dh01_formats[] = {
> +	DRM_FORMAT_XRGB8888,
> +};
> +
> +static const struct drm_display_mode sharp_ls027b7dh01_mode = {
> +	DRM_SIMPLE_MODE(400, 240, 59, 35),
> +};
> +
> +DEFINE_DRM_GEM_DMA_FOPS(sharp_ls027b7dh01_fops);
> +
> +static const struct drm_driver sharp_ls027b7dh01_drm_driver = {
> +	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
> +	.fops			= &sharp_ls027b7dh01_fops,
> +	DRM_GEM_DMA_DRIVER_OPS_VMAP,
> +	.name			= "sharp_ls027b7dh01",
> +	.desc			= "Sharp ls027b7dh01 Memory LCD",
> +	.date			= "20231129",
> +	.major			= 1,
> +	.minor			= 0,
> +};
> +
> +static int sharp_ls027b7dh01_probe(struct spi_device *spi)
> +{
> +	struct device *dev = &spi->dev;
> +	struct sharp_ls027b7dh01 *priv;
> +	struct drm_device *drm;
> +	unsigned int write_buf_size;
> +	int ret;
> +
> +	if (!dev->coherent_dma_mask) {
> +		ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(32));
> +		if (ret)
> +			return dev_err_probe(dev, ret, "Failed to set dma mask\n");
> +	}

Is there a particular need for this? I don't see where you're dealing with
DMA at all.

> +
> +	priv = devm_drm_dev_alloc(dev, &sharp_ls027b7dh01_drm_driver,
> +				  struct sharp_ls027b7dh01, drm);
> +	if (IS_ERR(priv))
> +		return PTR_ERR(priv);
> +
> +	spi_set_drvdata(spi, priv);
> +	priv->spi = spi;
> +
> +	priv->enable_gpio = devm_gpiod_get(dev, "enable", GPIOD_OUT_HIGH);
> +	if (IS_ERR(priv->enable_gpio))
> +		return dev_err_probe(dev, PTR_ERR(priv->enable_gpio),
> +				     "Failed to get GPIO 'enable'\n");
> +
> +	drm = &priv->drm;
> +	ret = drmm_mode_config_init(drm);
> +	if (ret)
> +		return ret;
> +
> +	drm->mode_config.funcs = &sharp_ls027b7dh01_mode_config_funcs;
> +	priv->display_mode = &sharp_ls027b7dh01_mode;

It's a bit redundant to store it in priv if there's just one mode supported.

> +
> +	/*
> +	 * write_buf_size:
> +	 *
> +	 * hdisplay * vdisplay / 8 => 1 bit per Pixel.
> +	 * 2 * vdisplay => line number byte + trailer dummy byte for every line.
> +	 * 2 => write command byte + final trailer dummy byte.
> +	 */
> +	write_buf_size = priv->display_mode->hdisplay * priv->display_mode->vdisplay / 8
> +			 + 2 * priv->display_mode->vdisplay + 2;

Same issue about priv->display_mode->hdisplay * priv->display_mode->vdisplay
being dividable by 8. In your case you just have a single mode which doesn't
have the issue, but this code looks generic laid out like this, so it should
take care of all possible cases.
> +
> +	priv->write_buf = devm_kzalloc(dev, write_buf_size, GFP_KERNEL);
> +	if (!priv->write_buf)
> +		return -ENOMEM;
> +
> +	drm->mode_config.min_width = priv->display_mode->hdisplay;
> +	drm->mode_config.max_width = priv->display_mode->hdisplay;
> +	drm->mode_config.min_height = priv->display_mode->vdisplay;
> +	drm->mode_config.max_height = priv->display_mode->vdisplay;
> +
> +	ret = drm_connector_init(drm, &priv->connector,
> +				 &sharp_ls027b7dh01_connector_funcs,
> +				 DRM_MODE_CONNECTOR_SPI);
> +	if (ret)
> +		return ret;
> +
> +	drm_connector_helper_add(&priv->connector,
> +				 &sharp_ls027b7dh01_connector_hfuncs);
> +
> +	ret = drm_simple_display_pipe_init(drm, &priv->pipe,
> +					   &sharp_ls027b7dh01_pipe_funcs,
> +					   sharp_ls027b7dh01_formats,
> +					   ARRAY_SIZE(sharp_ls027b7dh01_formats),
> +					   NULL, &priv->connector);
> +	if (ret)
> +		return ret;
> +
> +	drm_plane_enable_fb_damage_clips(&priv->pipe.plane);
> +	drm_mode_config_reset(drm);
> +
> +	ret = drm_dev_register(drm, 0);
> +	if (ret)
> +		return ret;
> +
> +	drm_fbdev_generic_setup(drm, 0);
> +
> +	return 0;
> +}
> +
> +static void sharp_ls027b7dh01_remove(struct spi_device *spi)
> +{
> +	struct sharp_ls027b7dh01 *priv = spi_get_drvdata(spi);
> +
> +	drm_dev_unplug(&priv->drm);
> +	drm_atomic_helper_shutdown(&priv->drm);
> +}
> +
> +static void sharp_ls027b7dh01_shutdown(struct spi_device *spi)
> +{
> +	struct sharp_ls027b7dh01 *priv = spi_get_drvdata(spi);
> +
> +	drm_atomic_helper_shutdown(&priv->drm);
> +}
> +
> +static const struct spi_device_id sharp_ls027b7dh01_ids[] = {
> +	{ "ls027b7dh01" },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(spi, sharp_ls027b7dh01_ids);
> +
> +static const struct of_device_id sharp_ls027b7dh01_of_match[] = {
> +	{ .compatible = "sharp,ls027b7dh01", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, sharp_ls027b7dh01_of_match);
> +
> +static struct spi_driver sharp_ls027b7dh01_spi_driver = {
> +	.probe = sharp_ls027b7dh01_probe,
> +	.remove = sharp_ls027b7dh01_remove,
> +	.shutdown = sharp_ls027b7dh01_shutdown,
> +	.id_table = sharp_ls027b7dh01_ids,
> +	.driver = {
> +		.name = "sharp-ls027b7dh01",
> +		.of_match_table = sharp_ls027b7dh01_of_match,
> +	},
> +};
> +module_spi_driver(sharp_ls027b7dh01_spi_driver);
> +
> +MODULE_AUTHOR("Andrew D'Angelo");
> +MODULE_AUTHOR("Mehdi Djait <mehdi.djait@bootlin.com>");
> +MODULE_DESCRIPTION("Sharp LS027B7DH01 Driver");
> +MODULE_LICENSE("GPL");
> -- 
> 2.41.0
> 

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

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

Re: [PATCH 1/2] dt-bindings: display: Add Sharp LS027B7DH01 Memory LCD

[Krzysztof Kozlowski]

On 29/11/2023 15:29, Mehdi Djait wrote:
> +  pwms:
> +    maxItems: 1
> +    description: External COM inversion signal
> +
> +required:
> +  - compatible
> +  - reg
> +  - spi-lsb-first
> +  - enable-gpios
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    spi {
> +            #address-cells = <1>;

If there is going to be new version, then:
Use 4 spaces for example indentation.

Anyway:

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


---

This is an automated instruction, just in case, because many review tags
are being ignored. If you know the process, you can skip it (please do
not feel offended by me posting it here - no bad intentions intended).
If you do not know the process, here is a short explanation:

Please add Acked-by/Reviewed-by/Tested-by tags when posting new
versions, under or above your Signed-off-by tag. Tag is "received", when
provided in a message replied to you on the mailing list. Tools like b4
can help here. However, there's no need to repost patches *only* to add
the tags. The upstream maintainer will do that for tags received on the
version they apply.

https://elixir.bootlin.com/linux/v6.5-rc3/source/Documentation/process/submitting-patches.rst#L577

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: display: Add Sharp LS027B7DH01 Memory LCD

[Krzysztof Kozlowski]

On 29/11/2023 15:29, Mehdi Djait wrote:
> +  pwms:
> +    maxItems: 1
> +    description: External COM inversion signal
> +
> +required:
> +  - compatible
> +  - reg
> +  - spi-lsb-first
> +  - enable-gpios
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    spi {
> +            #address-cells = <1>;

If there is going to be new version, then:
Use 4 spaces for example indentation.

Anyway:

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


---

This is an automated instruction, just in case, because many review tags
are being ignored. If you know the process, you can skip it (please do
not feel offended by me posting it here - no bad intentions intended).
If you do not know the process, here is a short explanation:

Please add Acked-by/Reviewed-by/Tested-by tags when posting new
versions, under or above your Signed-off-by tag. Tag is "received", when
provided in a message replied to you on the mailing list. Tools like b4
can help here. However, there's no need to repost patches *only* to add
the tags. The upstream maintainer will do that for tags received on the
version they apply.

https://elixir.bootlin.com/linux/v6.5-rc3/source/Documentation/process/submitting-patches.rst#L577

Best regards,
Krzysztof

Re: [PATCH 2/2] drm/tiny: Add driver for the sharp LS027B7DH01 Memory LCD

[Mehdi Djait]

Hi Thomas, 

Thank you for the review.

On Wed, Nov 29, 2023 at 05:21:19PM +0100, Thomas Zimmermann wrote:
> Hi,
> 
> thanks for the patches.
> 
> Am 29.11.23 um 15:29 schrieb Mehdi Djait:
> > Introduce a DRM driver for the sharp LS027B7DH01 Memory LCD.
> > 
> > LS027B7DH01 is a 2.7" 400x240 monochrome display connected to a SPI bus.
> > The drivers implements the Multiple Lines Data Update Mode.
> > External COM inversion is enabled using a PWM signal as input.
> > 
> > Signed-off-by: Mehdi Djait <mehdi.djait@bootlin.com>
> > ---
> >   MAINTAINERS                              |   7 +
> >   drivers/gpu/drm/tiny/Kconfig             |   8 +
> >   drivers/gpu/drm/tiny/Makefile            |   1 +
> >   drivers/gpu/drm/tiny/sharp-ls027b7dh01.c | 411 +++++++++++++++++++++++
> >   4 files changed, 427 insertions(+)
> >   create mode 100644 drivers/gpu/drm/tiny/sharp-ls027b7dh01.c
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 012df8ccf34e..fb859698bd3d 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -6832,6 +6832,13 @@ S:	Maintained
> >   F:	Documentation/devicetree/bindings/display/panel/samsung,s6d7aa0.yaml
> >   F:	drivers/gpu/drm/panel/panel-samsung-s6d7aa0.c
> > +DRM DRIVER FOR SHARP LS027B7DH01 Memory LCD
> > +M:	Mehdi Djait <mehdi.djait@bootlin.com>
> > +S:	Maintained
> > +T:	git git://anongit.freedesktop.org/drm/drm-misc
> > +F:	Documentation/devicetree/bindings/display/sharp,ls027b7dh01.yaml
> > +F:	drivers/gpu/drm/tiny/sharp-ls027b7dh01.c
> > +
> >   DRM DRIVER FOR SITRONIX ST7586 PANELS
> >   M:	David Lechner <david@lechnology.com>
> >   S:	Maintained
> > diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig
> > index f6889f649bc1..a2ade06403ca 100644
> > --- a/drivers/gpu/drm/tiny/Kconfig
> > +++ b/drivers/gpu/drm/tiny/Kconfig
> > @@ -186,6 +186,14 @@ config TINYDRM_REPAPER
> >   	  If M is selected the module will be called repaper.
> > +config TINYDRM_SHARP_LS027B7DH01
> > +	tristate "DRM support for SHARP LS027B7DH01 display"
> > +	depends on DRM && SPI
> > +	select DRM_KMS_HELPER
> > +	select DRM_GEM_DMA_HELPER
> > +	help
> > +	  DRM driver for the SHARP LS027B7DD01 LCD display.
> > +
> >   config TINYDRM_ST7586
> >   	tristate "DRM support for Sitronix ST7586 display panels"
> >   	depends on DRM && SPI
> > diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile
> > index 76dde89a044b..b05df3afb231 100644
> > --- a/drivers/gpu/drm/tiny/Makefile
> > +++ b/drivers/gpu/drm/tiny/Makefile
> > @@ -14,5 +14,6 @@ obj-$(CONFIG_TINYDRM_ILI9341)		+= ili9341.o
> >   obj-$(CONFIG_TINYDRM_ILI9486)		+= ili9486.o
> >   obj-$(CONFIG_TINYDRM_MI0283QT)		+= mi0283qt.o
> >   obj-$(CONFIG_TINYDRM_REPAPER)		+= repaper.o
> > +obj-$(CONFIG_TINYDRM_SHARP_LS027B7DH01)	+= sharp-ls027b7dh01.o
> >   obj-$(CONFIG_TINYDRM_ST7586)		+= st7586.o
> >   obj-$(CONFIG_TINYDRM_ST7735R)		+= st7735r.o
> > diff --git a/drivers/gpu/drm/tiny/sharp-ls027b7dh01.c b/drivers/gpu/drm/tiny/sharp-ls027b7dh01.c
> > new file mode 100644
> > index 000000000000..2f58865a5c78
> > --- /dev/null
> > +++ b/drivers/gpu/drm/tiny/sharp-ls027b7dh01.c
> > @@ -0,0 +1,411 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Sharp LS027B7DH01 Memory Display Driver
> > + *
> > + * Copyright (C) 2023 Andrew D'Angelo
> > + * Copyright (C) 2023 Mehdi Djait <mehdi.djait@bootlin.com>
> > + *
> > + * The Sharp Memory LCD requires an alternating signal to prevent the buildup of
> > + * a DC bias that would result in a Display that no longer can be updated. Two
> > + * modes for the generation of this signal are supported:
> > + *
> > + * Software, EXTMODE = Low: toggling the BIT(1) of the Command and writing it at
> > + * least once a second to the display.
> > + *
> > + * Hardware, EXTMODE = High: the alternating signal should be supplied on the
> > + * EXTCOMIN pin.
> > + *
> > + * In this driver the Hardware mode is implemented with a PWM signal.
> > + */
> > +
> > +#include <linux/delay.h>
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/pwm.h>
> > +#include <linux/spi/spi.h>
> > +
> > +#include <drm/drm_atomic_helper.h>
> > +#include <drm/drm_damage_helper.h>
> > +#include <drm/drm_drv.h>
> > +#include <drm/drm_fbdev_generic.h>
> > +#include <drm/drm_fb_dma_helper.h>
> > +#include <drm/drm_format_helper.h>
> > +#include <drm/drm_framebuffer.h>
> > +#include <drm/drm_gem_atomic_helper.h>
> > +#include <drm/drm_gem_dma_helper.h>
> > +#include <drm/drm_gem_framebuffer_helper.h>
> > +#include <drm/drm_modes.h>
> > +#include <drm/drm_probe_helper.h>
> > +#include <drm/drm_simple_kms_helper.h>
> > +
> > +#define CMD_WRITE BIT(0)
> > +#define CMD_CLEAR BIT(2)
> > +
> > +struct sharp_ls027b7dh01 {
> > +	struct spi_device *spi;
> > +
> > +	struct drm_device drm;
> > +	struct drm_connector connector;
> > +	struct drm_simple_display_pipe pipe;
> 
> Could you please replace the simple pipe and its helpers with regular
> DRMhelpers. It should no tbe used in new drivers. It's an unnecessary
> indirection. Replacing is simple: copy the content of the data structure and
> its helpers into the driver. Maybe clean up the result, if necessary.


Could you please further explain where to copy the helper functions or give me
an example driver ? This is my first DRM driver and grepping did not help me much.

(Sorry for the delayed response)

--
Kind Regards
Mehdi Djait