Hi
Am 14.07.25 um 04:59 schrieb LiangCheng Wang:
Introduce a DRM driver for the Mayqueen Pixpaper e-ink display panel, which is controlled via SPI. The driver supports a 122x250 resolution display with XRGB8888 format.
Also, add a MAINTAINERS entry for the Pixpaper driver.
Signed-off-by: LiangCheng Wang zaq14760@gmail.com
MAINTAINERS | 6 + drivers/gpu/drm/tiny/Kconfig | 15 + drivers/gpu/drm/tiny/Makefile | 1 + drivers/gpu/drm/tiny/pixpaper.c | 777 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 799 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS index fad6cb025a1918beec113b576cf28b76151745ef..0613f32ef8a702e508c9a2e51853f8fe6a38ba42 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -7756,6 +7756,12 @@ T: git https://gitlab.freedesktop.org/drm/misc/kernel.git F: Documentation/devicetree/bindings/display/repaper.txt F: drivers/gpu/drm/tiny/repaper.c +DRM DRIVER FOR PIXPAPER E-INK PANEL +M: LiangCheng Wang zaq14760@gmail.com +S: Maintained +F: Documentation/devicetree/bindings/display/mayqueen,pixpaper.yaml +F: drivers/gpu/drm/tiny/pixpaper.c
- DRM DRIVER FOR QEMU'S CIRRUS DEVICE M: Dave Airlie airlied@redhat.com M: Gerd Hoffmann kraxel@redhat.com
diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig index 06e54694a7f2fe1649e1886f039926b24f698e0d..f9b814e87348cad1946dd5e2ff54d304100ef264 100644 --- a/drivers/gpu/drm/tiny/Kconfig +++ b/drivers/gpu/drm/tiny/Kconfig @@ -164,6 +164,21 @@ config TINYDRM_MI0283QT DRM driver for the Multi-Inno MI0283QT display panel If M is selected the module will be called mi0283qt. +config TINYDRM_PIXPAPER
We don't use the TINYDRM_ prefix any longer. Just call it DRM_PIXPAPER.
- tristate "DRM support for PIXPAPER display panels"
- depends on DRM && SPI
- select DRM_CLIENT_SELECTION
- select DRM_KMS_HELPER
- select DRM_GEM_DMA_HELPER
Alphabetical sorting please.
- help
DRM driver for the Mayqueen Pixpaper e-ink display panel.This driver supports small e-paper displays connected over SPI,with a resolution of 122x250 and XRGB8888 framebuffer format.It is intended for low-power embedded applications.If M is selected, the module will be built as pixpaper.ko.- config TINYDRM_REPAPER tristate "DRM support for Pervasive Displays RePaper panels (V231)" depends on DRM && SPI
diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile index 4a9ff61ec25420e2c0a648c04eaab7ca25dd5407..40d613b57f5ee990dbf170d69939a39546be21b7 100644 --- a/drivers/gpu/drm/tiny/Makefile +++ b/drivers/gpu/drm/tiny/Makefile @@ -12,5 +12,6 @@ obj-$(CONFIG_TINYDRM_ILI9225) += ili9225.o obj-$(CONFIG_TINYDRM_ILI9341) += ili9341.o obj-$(CONFIG_TINYDRM_ILI9486) += ili9486.o obj-$(CONFIG_TINYDRM_MI0283QT) += mi0283qt.o +obj-$(CONFIG_TINYDRM_PIXPAPER) += pixpaper.o obj-$(CONFIG_TINYDRM_REPAPER) += repaper.o obj-$(CONFIG_TINYDRM_SHARP_MEMORY) += sharp-memory.o diff --git a/drivers/gpu/drm/tiny/pixpaper.c b/drivers/gpu/drm/tiny/pixpaper.c new file mode 100644 index 0000000000000000000000000000000000000000..f134cacbb3d2b2792b94fc37cddb0b264348f192 --- /dev/null +++ b/drivers/gpu/drm/tiny/pixpaper.c @@ -0,0 +1,777 @@ +// SPDX-License-Identifier: GPL-2.0 +/*
- DRM driver for PIXPAPER e-ink panel
- Author: LiangCheng Wang zaq14760@gmail.com,
- */
+#include <drm/clients/drm_client_setup.h>
Blank line before this include, not after.
+#include <drm/drm_atomic.h> +#include <drm/drm_atomic_helper.h> +#include <drm/drm_drv.h> +#include <drm/drm_fbdev_dma.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_probe_helper.h>
+#include <linux/delay.h> +#include <linux/module.h> +#include <linux/spi/spi.h>
The <linux/> includes go before the <drm/> ones.
+MODULE_IMPORT_NS("DMA_BUF");
+#define PIXPAPER_WIDTH 122 +#define PIXPAPER_HEIGHT 250
+#define PANEL_BUFFER_WIDTH 128 +#define PANEL_BUFFER_TWO_BYTES_PER_ROW (PANEL_BUFFER_WIDTH / 4)
+#define PIXPAPER_SPI_SPEED_DEFAULT 1000000
+#define PIXPAPER_CMD_PANEL_SETTING 0x00 +#define PIXPAPER_CMD_POWER_SETTING 0x01 +#define PIXPAPER_CMD_POWER_OFF 0x02 +#define PIXPAPER_CMD_POWER_OFF_SEQUENCE 0x03 +#define PIXPAPER_CMD_POWER_ON 0x04 +#define PIXPAPER_CMD_BOOSTER_SOFT_START 0x06 +#define PIXPAPER_CMD_DEEP_SLEEP 0x07 +#define PIXPAPER_CMD_DATA_START_TRANSMISSION 0x10 +#define PIXPAPER_CMD_DISPLAY_REFRESH 0x12 +#define PIXPAPER_CMD_PLL_CONTROL 0x30 +#define PIXPAPER_CMD_TEMP_SENSOR_CALIB 0x41 +#define PIXPAPER_CMD_UNKNOWN_4D 0x4D +#define PIXPAPER_CMD_VCOM_INTERVAL 0x50 +#define PIXPAPER_CMD_TCON_SETTING 0x60 +#define PIXPAPER_CMD_RESOLUTION_SETTING 0x61 +#define PIXPAPER_CMD_GATE_SOURCE_START 0x65 +#define PIXPAPER_CMD_UNKNOWN_B4 0xB4 +#define PIXPAPER_CMD_UNKNOWN_B5 0xB5 +#define PIXPAPER_CMD_CASCADE_SETTING 0xE0 +#define PIXPAPER_CMD_POWER_SAVING 0xE3 +#define PIXPAPER_CMD_AUTO_MEASURE_VCOM 0xE7 +#define PIXPAPER_CMD_UNKNOWN_E9 0xE9
+static int pixpaper_crtc_helper_atomic_check(struct drm_crtc *crtc,
struct drm_atomic_state *state);+static int pixpaper_plane_helper_atomic_check(struct drm_plane *plane,
struct drm_atomic_state *state);+static void pixpaper_crtc_atomic_enable(struct drm_crtc *crtc,
struct drm_atomic_state *state);+static void pixpaper_crtc_atomic_disable(struct drm_crtc *crtc,
struct drm_atomic_state *state);+static void pixpaper_plane_atomic_update(struct drm_plane *plane,
struct drm_atomic_state *state);+static int pixpaper_connector_get_modes(struct drm_connector *connector);
Please don't declare those functions. Interleave the implementation with the structs that use them.
+struct pixpaper_panel {
- struct drm_device drm;
- struct drm_plane plane;
- struct drm_crtc crtc;
- struct drm_encoder encoder;
- struct drm_connector connector;
- struct spi_device *spi;
- struct gpio_desc *reset;
- struct gpio_desc *busy;
- struct gpio_desc *dc;
+};
+static const struct drm_plane_funcs pixpaper_plane_funcs = {
- .update_plane = drm_atomic_helper_update_plane,
- .disable_plane = drm_atomic_helper_disable_plane,
- .destroy = drm_plane_cleanup,
- DRM_GEM_SHADOW_PLANE_FUNCS,
+};
+static const struct drm_plane_helper_funcs pixpaper_plane_helper_funcs = {
- DRM_GEM_SHADOW_PLANE_HELPER_FUNCS,
- .atomic_check = pixpaper_plane_helper_atomic_check,
- .atomic_update = pixpaper_plane_atomic_update,
For example, these functions should be implemented right before declaring the global variable; same goes for the others.
+};
+static const struct drm_crtc_funcs pixpaper_crtc_funcs = {
- .set_config = drm_atomic_helper_set_config,
- .page_flip = drm_atomic_helper_page_flip,
- .reset = drm_atomic_helper_crtc_reset,
- .destroy = drm_crtc_cleanup,
- .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
- .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
+};
+static const struct drm_crtc_helper_funcs pixpaper_crtc_helper_funcs = {
- .atomic_check = pixpaper_crtc_helper_atomic_check,
- .atomic_enable = pixpaper_crtc_atomic_enable,
- .atomic_disable = pixpaper_crtc_atomic_disable,
+};
+static const struct drm_encoder_funcs pixpaper_encoder_funcs = {
- .destroy = drm_encoder_cleanup,
+};
+static const struct drm_connector_funcs pixpaper_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_connector_helper_funcs pixpaper_connector_helper_funcs = {
- .get_modes = pixpaper_connector_get_modes,
+};
+static int pixpaper_plane_helper_atomic_check(struct drm_plane *plane,
struct drm_atomic_state *state)+{
- struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state, plane);
- struct drm_crtc *new_crtc = new_plane_state->crtc;
- struct drm_crtc_state *new_crtc_state = NULL;
- int ret;
- if (new_crtc)
new_crtc_state = drm_atomic_get_new_crtc_state(state, new_crtc);- ret = drm_atomic_helper_check_plane_state(new_plane_state, new_crtc_state,
DRM_PLANE_NO_SCALING,DRM_PLANE_NO_SCALING,false, false);- if (ret)
return ret;- else if (!new_plane_state->visible)
return 0;- return 0;
+}
+static int pixpaper_crtc_helper_atomic_check(struct drm_crtc *crtc,
struct drm_atomic_state *state)+{
- struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
- if (!crtc_state->enable)
return 0;- return drm_atomic_helper_check_crtc_primary_plane(crtc_state);
+}
+static void pixpaper_wait_busy(struct pixpaper_panel *panel) +{
- unsigned int delay_us = 1000;
- usleep_range(delay_us, delay_us + 500);
- while (true)
if (gpiod_get_value_cansleep(panel->busy) == 1)break;+}
This function should eventually time out.
+static int pixpaper_spi_sync(struct spi_device *spi, struct spi_message *msg) +{
- int ret;
- ret = spi_sync(spi, msg);
- if (ret < 0)
dev_err(&spi->dev, "SPI sync failed: %d\n", ret);
Please use DRM's error reporting. dev_err() becomes drm_err(), dev_dbg() becomes drm_dbg(). Further below is WARN_ON(), which should become drm_WARN_ON(). Here and everwhere else in the driver. The only exception is in places where you don't have a DRM device yet, sch as early device probing.
- return ret;
+}
+static int pixpaper_send_cmd(struct pixpaper_panel *panel, u8 cmd) +{
- struct spi_transfer xfer = {
.tx_buf = &cmd,.len = 1,- };
- struct spi_message msg;
- int ret;
- spi_message_init(&msg);
- spi_message_add_tail(&xfer, &msg);
- gpiod_set_value_cansleep(panel->dc, 0);
- usleep_range(1, 5);
- ret = pixpaper_spi_sync(panel->spi, &msg);
- return ret;
+}
+static int pixpaper_send_data(struct pixpaper_panel *panel, u8 data) +{
- struct spi_transfer xfer = {
.tx_buf = &data,.len = 1,- };
- struct spi_message msg;
- int ret;
- spi_message_init(&msg);
- spi_message_add_tail(&xfer, &msg);
- gpiod_set_value_cansleep(panel->dc, 1);
- usleep_range(1, 5);
- ret = pixpaper_spi_sync(panel->spi, &msg);
- return ret;
+}
+static int pixpaper_panel_hw_init(struct pixpaper_panel *panel) +{
- struct device *dev = &panel->spi->dev;
- int ret = 0;
- dev_info(dev, "%s: Starting hardware initialization\n", __func__);
- gpiod_set_value_cansleep(panel->reset, 0);
- msleep(50);
- gpiod_set_value_cansleep(panel->reset, 1);
- msleep(50);
- pixpaper_wait_busy(panel);
- dev_info(dev, "Hardware reset complete, panel idle.\n");
- ret |= pixpaper_send_cmd(panel, PIXPAPER_CMD_UNKNOWN_4D);
- ret |= pixpaper_send_data(panel, 0x78);
This error handling isn't working. Your code OR's multiple errno codes and the result will have a different meaning.
The easiest way to avoid excessive error checking is to add an error context as final parameter, like this
struct pixpaper_error_ctx { int errno_code; };
pixpaper_spi_sync(..., pixpaper_error_ctx *err) { if (err->errno_code) return; ret = spi_sync(...) if (ret) err->errno_code = ret; }
send_cmd(..., pix_error_ctx *err) { pixppaper_spi_sync(..., err); }
send_data(..., pix_error_ctx *err) { pixppaper_spi_sync(..., err); }
int my_pixpaper_driver_func() { sturct pixpaper_error_ctx = {}
send_cmd(..., &err); send_data(..., &err); send_data(..., &err); send_data(..., &err);
send_cmd(..., &err); send_data(..., &err); send_data(..., &err); send_data(..., &err);
if (err->errno_code) return errno_code
return 0; }
This pushes error handling down into the call tree and you only have to test for the final result.
- if (ret)
goto init_fail;- pixpaper_wait_busy(panel);
And you can extend other functions, such as pixpaper_wait_busy(), with the error context. Integrates them with the rest of the I/O code.
- ret |= pixpaper_send_cmd(panel, PIXPAPER_CMD_PANEL_SETTING);
- ret |= pixpaper_send_data(panel, 0x0F);
- ret |= pixpaper_send_data(panel, 0x09);
There are *a lot* of magic values in this function. Please document the meaning of these.
- if (ret)
goto init_fail;- pixpaper_wait_busy(panel);
- ret |= pixpaper_send_cmd(panel, PIXPAPER_CMD_POWER_SETTING);
- ret |= pixpaper_send_data(panel, 0x07);
- ret |= pixpaper_send_data(panel, 0x00);
- ret |= pixpaper_send_data(panel, 0x22);
- ret |= pixpaper_send_data(panel, 0x78);
- ret |= pixpaper_send_data(panel, 0x0A);
- ret |= pixpaper_send_data(panel, 0x22);
- if (ret)
goto init_fail;- pixpaper_wait_busy(panel);
- ret |= pixpaper_send_cmd(panel, PIXPAPER_CMD_POWER_OFF_SEQUENCE);
- ret |= pixpaper_send_data(panel, 0x10);
- ret |= pixpaper_send_data(panel, 0x54);
- ret |= pixpaper_send_data(panel, 0x44);
- if (ret)
goto init_fail;- pixpaper_wait_busy(panel);
- ret |= pixpaper_send_cmd(panel, PIXPAPER_CMD_BOOSTER_SOFT_START);
- ret |= pixpaper_send_data(panel, 0x0F);
- ret |= pixpaper_send_data(panel, 0x0A);
- ret |= pixpaper_send_data(panel, 0x2F);
- ret |= pixpaper_send_data(panel, 0x25);
- ret |= pixpaper_send_data(panel, 0x22);
- ret |= pixpaper_send_data(panel, 0x2E);
- ret |= pixpaper_send_data(panel, 0x21);
- if (ret)
goto init_fail;- pixpaper_wait_busy(panel);
- ret |= pixpaper_send_cmd(panel, PIXPAPER_CMD_PLL_CONTROL);
- ret |= pixpaper_send_data(panel, 0x02);
- if (ret)
goto init_fail;- pixpaper_wait_busy(panel);
- ret |= pixpaper_send_cmd(panel, PIXPAPER_CMD_TEMP_SENSOR_CALIB);
- ret |= pixpaper_send_data(panel, 0x00);
- if (ret)
goto init_fail;- pixpaper_wait_busy(panel);
- ret |= pixpaper_send_cmd(panel, PIXPAPER_CMD_VCOM_INTERVAL);
- ret |= pixpaper_send_data(panel, 0x37);
- if (ret)
goto init_fail;- pixpaper_wait_busy(panel);
- ret |= pixpaper_send_cmd(panel, PIXPAPER_CMD_TCON_SETTING);
- ret |= pixpaper_send_data(panel, 0x02);
- ret |= pixpaper_send_data(panel, 0x02);
- if (ret)
goto init_fail;- pixpaper_wait_busy(panel);
- ret |= pixpaper_send_cmd(panel, PIXPAPER_CMD_RESOLUTION_SETTING);
- ret |= pixpaper_send_data(panel, 0x00);
- ret |= pixpaper_send_data(panel, 0x80);
- ret |= pixpaper_send_data(panel, 0x00);
- ret |= pixpaper_send_data(panel, 0xFA);
- if (ret)
goto init_fail;- pixpaper_wait_busy(panel);
- ret |= pixpaper_send_cmd(panel, PIXPAPER_CMD_GATE_SOURCE_START);
- ret |= pixpaper_send_data(panel, 0x00);
- ret |= pixpaper_send_data(panel, 0x00);
- ret |= pixpaper_send_data(panel, 0x00);
- ret |= pixpaper_send_data(panel, 0x00);
- if (ret)
goto init_fail;- pixpaper_wait_busy(panel);
- ret |= pixpaper_send_cmd(panel, PIXPAPER_CMD_AUTO_MEASURE_VCOM);
- ret |= pixpaper_send_data(panel, 0x1C);
- if (ret)
goto init_fail;- pixpaper_wait_busy(panel);
- ret |= pixpaper_send_cmd(panel, PIXPAPER_CMD_POWER_SAVING);
- ret |= pixpaper_send_data(panel, 0x22);
- if (ret)
goto init_fail;- pixpaper_wait_busy(panel);
- ret |= pixpaper_send_cmd(panel, PIXPAPER_CMD_CASCADE_SETTING);
- ret |= pixpaper_send_data(panel, 0x00);
- if (ret)
goto init_fail;- pixpaper_wait_busy(panel);
- ret |= pixpaper_send_cmd(panel, PIXPAPER_CMD_UNKNOWN_B4);
- ret |= pixpaper_send_data(panel, 0xD0);
- if (ret)
goto init_fail;- pixpaper_wait_busy(panel);
- ret |= pixpaper_send_cmd(panel, PIXPAPER_CMD_UNKNOWN_B5);
- ret |= pixpaper_send_data(panel, 0x03);
- if (ret)
goto init_fail;- pixpaper_wait_busy(panel);
- ret |= pixpaper_send_cmd(panel, PIXPAPER_CMD_UNKNOWN_E9);
- ret |= pixpaper_send_data(panel, 0x01);
- if (ret)
goto init_fail;- pixpaper_wait_busy(panel);
- dev_info(dev, "%s: Hardware initialization successful\n", __func__);
Never print this type of status messages. If nothing exceptional happens, you driver should be quite. At most, use drm_dbg() here.
- return 0;
+init_fail:
- dev_err(dev, "%s: Hardware initialization failed (err=%d)\n", __func__,
ret);
Error messages are OK, but don't include the function name.
- return ret;
+}
+static void pixpaper_crtc_atomic_enable(struct drm_crtc *crtc,
struct drm_atomic_state *state)+{
- struct pixpaper_panel *panel =
container_of(crtc, struct pixpaper_panel, crtc);
Instead of semi-manual upcast, please write a helper that upcasts from struct drm_device
struct pixpaper_panel *to_pixpaper_panel(struct drm_device *drm) { return container_of(drm, struct pixpaper_panel, drm); }
and call it with the CRTC's device
to_pixpaper_panel(crtc->dev);
- struct drm_device *drm = &panel->drm;
- int ret;
- dev_info(drm->dev, "%s: Enabling pipe\n", __func__);
Same here. The driver should just be quite.
- ret = pixpaper_panel_hw_init(panel);
Do you really need a full hardware init for an atomic_enable? You should rather do this during device probing.
- if (ret) {
dev_err(drm->dev, "Panel HW Init failed during enable: %d\n",ret);return;- }
- dev_info(drm->dev, "Sending Power ON (PON)\n");
- ret = pixpaper_send_cmd(panel, PIXPAPER_CMD_POWER_ON);
- if (ret) {
dev_err(drm->dev, "Failed to send PON command: %d\n", ret);return;- }
- usleep_range(10000, 11000);
- pixpaper_wait_busy(panel);
- dev_info(drm->dev, "Panel enabled and powered on\n");
+}
+static void pixpaper_crtc_atomic_disable(struct drm_crtc *crtc,
struct drm_atomic_state *state)+{
- struct pixpaper_panel *panel =
container_of(crtc, struct pixpaper_panel, crtc);- struct drm_device *drm = &panel->drm;
- int ret;
- dev_dbg(drm->dev, "%s: Disabling pipe\n", __func__);
- ret = pixpaper_send_cmd(panel, PIXPAPER_CMD_POWER_OFF);
- if (!ret) {
usleep_range(10000, 11000);pixpaper_wait_busy(panel);- } else {
dev_warn(drm->dev, "Failed to send POF command: %d\n", ret);- }
- dev_info(drm->dev, "Panel disabled\n");
+}
+static u8 pack_pixels_to_byte(u32 *src_pixels, int i, int j,
struct drm_framebuffer *fb)+{
- u8 packed_byte = 0;
- int k;
- for (k = 0; k < 4; k++) {
int current_pixel_x = j * 4 + k;u8 two_bit_val;if (current_pixel_x < PIXPAPER_WIDTH) {u32 pixel_offset =(i * (fb->pitches[0] / 4)) + current_pixel_x;u32 pixel = src_pixels[pixel_offset];u32 r = (pixel >> 16) & 0xFF;u32 g = (pixel >> 8) & 0xFF;u32 b = pixel & 0xFF;u32 gray_val =(r * 299 + g * 587 + b * 114 + 500) / 1000;if (gray_val < 64)two_bit_val = 0b00;else if (gray_val < 128)two_bit_val = 0b01;else if (gray_val < 192)two_bit_val = 0b10;elsetwo_bit_val = 0b11;} else {two_bit_val = 0b11;}packed_byte |= two_bit_val << ((3 - k) * 2);- }
- return packed_byte;
+}
+static void pixpaper_plane_atomic_update(struct drm_plane *plane,
struct drm_atomic_state *state)+{
- struct drm_plane_state *plane_state =
drm_atomic_get_new_plane_state(state, plane);- struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(plane_state);
- struct drm_crtc *crtc = plane_state->crtc;
- struct pixpaper_panel *panel =
container_of(crtc, struct pixpaper_panel, crtc);- struct drm_device *drm = &panel->drm;
- struct drm_framebuffer *fb = plane_state->fb;
- struct iosys_map map = shadow_plane_state->data[0];
- void *vaddr = map.vaddr;
- int i, j, ret = 0;
- u32 *src_pixels = NULL;
- dev_info(drm->dev, "Starting frame update (phys=%dx%d, buf_w=%d)\n",
PIXPAPER_WIDTH, PIXPAPER_HEIGHT, PANEL_BUFFER_WIDTH);
Remove all these info statements please. At most, they should be drm_dbg().
- if (!fb || !plane_state->visible) {
dev_err(drm->dev,"No framebuffer or plane not visible, skipping update\n");return;- }
- if (ret) {
dev_err(drm->dev, "Failed to vmap dma_buf\n");return;- }
- src_pixels = (u32 *)vaddr;
- dev_info(drm->dev, "Sending DTM command\n");
- ret = pixpaper_send_cmd(panel, PIXPAPER_CMD_DATA_START_TRANSMISSION);
- if (ret)
goto update_cleanup;- usleep_range(10000, 11000);
- pixpaper_wait_busy(panel);
That driver does quite a bit of waiting. Is that really necessary or in case?
- dev_info(drm->dev,
"Panel idle after DTM command, starting data batch send.\n");- for (i = 0; i < PIXPAPER_HEIGHT; i++) {
for (j = 0; j < PANEL_BUFFER_TWO_BYTES_PER_ROW; j++) {u8 packed_byte =pack_pixels_to_byte(src_pixels, i, j, fb);pixpaper_wait_busy(panel);pixpaper_send_data(panel, packed_byte);}- }
You're updating the full display AFAICT. That's not optimal, but OK if the hardware can't do better.
- pixpaper_wait_busy(panel);
- dev_info(drm->dev, "Sending PON + 0x00 before DRF\n");
- ret = pixpaper_send_cmd(panel, PIXPAPER_CMD_POWER_ON);
- if (ret)
goto update_cleanup;- ret = pixpaper_send_data(panel, 0x00);
- if (ret) {
dev_err(drm->dev,"Failed sending data after PON-before-DRF: %d\n", ret);goto update_cleanup;- }
- usleep_range(10000, 11000);
- pixpaper_wait_busy(panel);
- dev_info(drm->dev, "Triggering display refresh (DRF)\n");
- ret = pixpaper_send_cmd(panel, PIXPAPER_CMD_DISPLAY_REFRESH);
- if (ret)
goto update_cleanup;- ret = pixpaper_send_data(panel, 0x00);
- if (ret) {
dev_err(drm->dev, "Failed sending data after DRF: %d\n", ret);goto update_cleanup;- }
- usleep_range(10000, 11000);
- pixpaper_wait_busy(panel);
- dev_info(drm->dev, "Frame update completed and refresh triggered\n");
+update_cleanup:
- if (ret && ret != -ETIMEDOUT)
dev_err(drm->dev,"Frame update function failed with error %d\n", ret);+}
+static int pixpaper_connector_get_modes(struct drm_connector *connector) +{
- struct drm_display_mode *mode;
- dev_info(connector->dev->dev, "%s: CALLED for connector %s (id: %d)\n",
__func__, connector->name, connector->base.id);- mode = drm_mode_create(connector->dev);
- if (!mode) {
DRM_ERROR("Failed to create mode for connector %s\n",connector->name);return 0;- }
- mode->hdisplay = PIXPAPER_WIDTH;
- mode->vdisplay = PIXPAPER_HEIGHT;
- mode->htotal = mode->hdisplay + 80;
- mode->hsync_start = mode->hdisplay + 8;
- mode->hsync_end = mode->hdisplay + 8 + 32;
- mode->vtotal = mode->vdisplay + 10;
- mode->vsync_start = mode->vdisplay + 2;
- mode->vsync_end = mode->vdisplay + 2 + 2;
- mode->clock = 6000;
- mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
- drm_mode_set_name(mode);
- if (drm_mode_validate_size(mode, connector->dev->mode_config.max_width,
connector->dev->mode_config.max_height) !=MODE_OK) {DRM_WARN("%s: Mode %s (%dx%d) failed size validation against max %dx%d\n",__func__, mode->name, mode->hdisplay, mode->vdisplay,connector->dev->mode_config.max_width,connector->dev->mode_config.max_height);drm_mode_destroy(connector->dev, mode);return 0;- }
- drm_mode_probed_add(connector, mode);
- dev_info(connector->dev->dev,
"%s: Added mode '%s' (%dx%d@%d) to connector %s\n", __func__,mode->name, mode->hdisplay, mode->vdisplay,drm_mode_vrefresh(mode), connector->name);- connector->display_info.width_mm = 30;
- connector->display_info.height_mm = 47;
- return 1;
+}
+DEFINE_DRM_GEM_DMA_FOPS(pixpaper_fops);
+static struct drm_driver pixpaper_drm_driver = {
- .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
- .fops = &pixpaper_fops,
- .name = "pixpaper",
- .desc = "DRM driver for PIXPAPER e-ink",
- .major = 1,
- .minor = 0,
- DRM_GEM_DMA_DRIVER_OPS_VMAP,
- DRM_FBDEV_DMA_DRIVER_OPS,
Is there a specific reason why you need GEM DMA? Since you're hardware doesn't use DMA, GEM SHMEM might be a better choice.
+};
+static int pixpaper_mode_valid(struct drm_device *dev,
const struct drm_display_mode *mode)+{
- if (mode->hdisplay == PIXPAPER_WIDTH &&
mode->vdisplay == PIXPAPER_HEIGHT) {return MODE_OK;- }
- dev_dbg(dev->dev, "Rejecting mode %dx%d (supports only %dx%d)\n",
mode->hdisplay, mode->vdisplay, PIXPAPER_WIDTH,PIXPAPER_HEIGHT);- return MODE_BAD;
+}
+static const struct drm_mode_config_funcs pixpaper_mode_config_funcs = {
- .fb_create = drm_gem_fb_create_with_dirty,
- .mode_valid = pixpaper_mode_valid,
- .atomic_check = drm_atomic_helper_check,
- .atomic_commit = drm_atomic_helper_commit,
+};
+static int pixpaper_probe(struct spi_device *spi) +{
- struct device *dev = &spi->dev;
- struct pixpaper_panel *panel;
- struct drm_device *drm;
- int ret;
- dev_info(dev, "Probing PIXPAPER panel driver\n");
- panel = devm_drm_dev_alloc(dev, &pixpaper_drm_driver,
struct pixpaper_panel, drm);- if (IS_ERR(panel)) {
ret = PTR_ERR(panel);dev_err(dev, "Failed to allocate DRM device: %d\n", ret);return ret;- }
- drm = &panel->drm;
- panel->spi = spi;
- spi_set_drvdata(spi, panel);
- spi->mode = SPI_MODE_0;
- spi->bits_per_word = 8;
- if (!spi->max_speed_hz) {
dev_warn(dev, "spi-max-frequency not specified in DT, using default %u Hz\n",PIXPAPER_SPI_SPEED_DEFAULT);spi->max_speed_hz = PIXPAPER_SPI_SPEED_DEFAULT;- } else {
dev_info(dev, "Using spi-max-frequency from DT: %u Hz\n",spi->max_speed_hz);- }
- ret = spi_setup(spi);
- if (ret < 0) {
dev_err(dev, "SPI setup failed: %d\n", ret);return ret;- }
- dev_info(dev, "SPI setup OK (mode=%d, speed=%u Hz, bpw=%d)\n",
spi->mode, spi->max_speed_hz, spi->bits_per_word);- if (!dev->dma_mask)
dev->dma_mask = &dev->coherent_dma_mask;- ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
- if (ret) {
dev_err(dev, "Failed to set DMA mask: %d\n", ret);return ret;- }
- dev_dbg(dev, "DMA mask set\n");
- panel->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
- if (IS_ERR(panel->reset))
return dev_err_probe(dev, PTR_ERR(panel->reset),"Failed to get 'reset' GPIO\n");- panel->busy = devm_gpiod_get(dev, "busy", GPIOD_IN);
- if (IS_ERR(panel->busy))
return dev_err_probe(dev, PTR_ERR(panel->busy),"Failed to get 'busy' GPIO\n");- panel->dc = devm_gpiod_get(dev, "dc", GPIOD_OUT_HIGH);
- if (IS_ERR(panel->dc))
return dev_err_probe(dev, PTR_ERR(panel->dc),"Failed to get 'dc' GPIO\n");- dev_info(dev, "All required GPIOs obtained successfully.\n");
- ret = drmm_mode_config_init(drm);
- if (ret)
return ret;- drm->mode_config.funcs = &pixpaper_mode_config_funcs;
- drm->mode_config.min_width = PIXPAPER_WIDTH;
- drm->mode_config.max_width = PIXPAPER_WIDTH;
- drm->mode_config.min_height = PIXPAPER_HEIGHT;
- drm->mode_config.max_height = PIXPAPER_HEIGHT;
- ret = drm_universal_plane_init(drm, &panel->plane, 1, &pixpaper_plane_funcs,
(const uint32_t[]){ DRM_FORMAT_XRGB8888 }, 1, NULL,DRM_PLANE_TYPE_PRIMARY, NULL);
- drm_plane_enable_fb_damage_clips(&panel->plane);
Since the driver always updates the full display anyway, you don't need to enable damage clipping.
- if (ret)
return ret;- drm_plane_helper_add(&panel->plane, &pixpaper_plane_helper_funcs);
- ret = drm_crtc_init_with_planes(drm, &panel->crtc, &panel->plane, NULL,
&pixpaper_crtc_funcs, NULL);- if (ret)
return ret;- drm_crtc_helper_add(&panel->crtc, &pixpaper_crtc_helper_funcs);
- ret = drm_encoder_init(drm, &panel->encoder, &pixpaper_encoder_funcs,
DRM_MODE_ENCODER_DAC, NULL);
ENCODER_DAC is for VGA connectors. Rather use ENCODER_NONE here.
- if (ret)
return ret;- panel->encoder.possible_crtcs = drm_crtc_mask(&panel->crtc);
- ret = drm_connector_init(drm, &panel->connector,
&pixpaper_connector_funcs,DRM_MODE_CONNECTOR_SPI);- if (ret)
return ret;- drm_connector_helper_add(&panel->connector,
&pixpaper_connector_helper_funcs);- drm_connector_attach_encoder(&panel->connector, &panel->encoder);
- drm_mode_config_reset(drm);
- ret = drm_dev_register(drm, 0);
- if (ret)
return ret;- drm_client_setup(drm, NULL);
- dev_info(dev, "Initialized PIXPAPER panel driver successfully\n");
- return 0;
+}
+static void pixpaper_remove(struct spi_device *spi) +{
- struct pixpaper_panel *panel = spi_get_drvdata(spi);
- if (!panel)
return;- dev_info(&spi->dev, "Removing PIXPAPER panel driver\n");
- drm_dev_unplug(&panel->drm);
This signals to DRM that the hardware device has been removed. You should put drm_dev_enter() and drm_dev_exit() to the top and bottom of each of the DRM callbacks. The driver will then not try to access the device.
Best regards Thomas
- drm_atomic_helper_shutdown(&panel->drm);
+}
+static const struct spi_device_id pixpaper_ids[] = { { "pixpaper", 0 }, {} }; +MODULE_DEVICE_TABLE(spi, pixpaper_ids);
+static const struct of_device_id pixpaper_dt_ids[] = {
- { .compatible = "mayqueen,pixpaper" },
- {}
+}; +MODULE_DEVICE_TABLE(of, pixpaper_dt_ids);
+static struct spi_driver pixpaper_spi_driver = {
- .driver = {
.name = "pixpaper",.of_match_table = pixpaper_dt_ids,- },
- .id_table = pixpaper_ids,
- .probe = pixpaper_probe,
- .remove = pixpaper_remove,
+};
+module_spi_driver(pixpaper_spi_driver);
+MODULE_AUTHOR("LiangCheng Wang"); +MODULE_DESCRIPTION("DRM SPI driver for PIXPAPER e-ink panel"); +MODULE_LICENSE("GPL");
linaro-mm-sig@lists.linaro.org
-
Thomas Zimmermann