* [PATCH v3 01/14] dt-bindings: vendor-prefixes: Add Dalian Good Display Co., Ltd.
2026-10-19 21:37 [PATCH v3 00/14] Add DRM driver for Solomon SSD16xx e-paper display controllers Devarsh Thakkar
@ 2026-10-19 21:37 ` Devarsh Thakkar
2026-10-19 21:37 ` [PATCH v3 02/14] dt-bindings: display: Add Solomon SSD16xx e-paper controller Devarsh Thakkar
` (12 subsequent siblings)
13 siblings, 0 replies; 25+ messages in thread
From: Devarsh Thakkar @ 2026-10-19 21:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Nishanth Menon, Vignesh Raghavendra, Tero Kristo
Cc: dri-devel, devicetree, linux-kernel, linux-arm-kernel, devarsht,
LiangCheng Wang
Add vendor prefix 'gooddisplay' for Dalian Good Display Co., Ltd., a
manufacturer of e-paper display panels and modules.
This prefix will be used for the GDEY042T81 e-paper display panel in
the solomon,ssd16xx binding.
Signed-off-by: Devarsh Thakkar <devarsht@ti.com>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
V3: No change
V2: No change
---
Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index ba2002969373..3e8062aa92f7 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -686,6 +686,8 @@ patternProperties:
description: GOcontroll Modular Embedded Electronics B.V.
"^goldelico,.*":
description: Golden Delicious Computers GmbH & Co. KG
+ "^gooddisplay,.*":
+ description: Dalian Good Display Co., Ltd.
"^goodix,.*":
description: Shenzhen Huiding Technology Co., Ltd.
"^google,.*":
--
2.39.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v3 02/14] dt-bindings: display: Add Solomon SSD16xx e-paper controller
2026-10-19 21:37 [PATCH v3 00/14] Add DRM driver for Solomon SSD16xx e-paper display controllers Devarsh Thakkar
2026-10-19 21:37 ` [PATCH v3 01/14] dt-bindings: vendor-prefixes: Add Dalian Good Display Co., Ltd Devarsh Thakkar
@ 2026-10-19 21:37 ` Devarsh Thakkar
2026-10-20 8:34 ` Krzysztof Kozlowski
2026-10-19 21:37 ` [PATCH v3 03/14] dt-bindings: display: solomon, ssd16xx: Add Solomon SSD1677 controller Devarsh Thakkar
` (11 subsequent siblings)
13 siblings, 1 reply; 25+ messages in thread
From: Devarsh Thakkar @ 2026-10-19 21:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Nishanth Menon, Vignesh Raghavendra, Tero Kristo
Cc: dri-devel, devicetree, linux-kernel, linux-arm-kernel, devarsht,
LiangCheng Wang
Add device tree binding for the Solomon Systech SSD16xx family of e-Paper
display controllers (SSD1683 [1], SSD1673 [3], SSD1680 [4], SSD1681 [5]).
The binding covers the 4-wire SPI interface with required GPIO pins for
reset (active-low), busy status (active-high), and data/command selection.
The spi-max-frequency is capped at 20 MHz per the SSD1683 datasheet [1]
which is also the maximum supported frequency amongst the family.
An optional rotation property (as defined by panel-common, in degrees
counter clockwise) allows static display orientation to be configured from
device tree.
The data/command selection GPIO pin is also kept as optional since various
SSD16XX controllers such as SSD1683 [1] support 3-wire mode too with
data/command selection bit transmitted as first bit before sending the data
sequence.
The first supported compatible is for Gooddisplay GDEY042T81 [2] which is
4.2", 400x300 resolution black/white e-paper display using SSD1683
controller.
Links:
[1] : https://www.crystalfontz.com/controllers/SolomonSystech/SSD1683
[2] : https://files.seeedstudio.com/wiki/Other_Display/42-epaper/GDEY042T81.pdf
[3] : https://www.crystalfontz.com/controllers/SolomonSystech/SSD1673
[4] : https://www.crystalfontz.com/controllers/SolomonSystech/SSD1680
[5] : https://www.crystalfontz.com/controllers/SolomonSystech/SSD1681
Signed-off-by: Devarsh Thakkar <devarsht@ti.com>
---
Changes in v3:
- Rename the file to solomon,ssd1683.yaml so that it matches an actual
compatible string
- Drop the redundant "binding" from the subject
- Describe the compatible as oneOf/items from the start so that the next
patch only adds a branch instead of rewriting these lines
- Reference panel/panel-common.yaml in allOf
- Add spi-3wire and document that omitting dc-gpios selects 3-wire mode
- Use the common 'rotation' property from panel-common instead of a local
definition that said "clockwise"
- Remove rotation as it is already inherited by panel-common
- Set spi-3wire as true since supported by panel
---
.../bindings/display/solomon,ssd1683.yaml | 85 +++++++++++++++++++
1 file changed, 85 insertions(+)
create mode 100644 Documentation/devicetree/bindings/display/solomon,ssd1683.yaml
diff --git a/Documentation/devicetree/bindings/display/solomon,ssd1683.yaml b/Documentation/devicetree/bindings/display/solomon,ssd1683.yaml
new file mode 100644
index 000000000000..2e2aa195fb1e
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/solomon,ssd1683.yaml
@@ -0,0 +1,85 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/solomon,ssd1683.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Solomon Systech SSD16xx-based e-paper display panels
+
+maintainers:
+ - Devarsh Thakkar <devarsht@ti.com>
+
+description:
+ The SSD16xx family includes e-paper display controllers (SSD1673,
+ SSD1680, SSD1681, SSD1683, SSD1677) with integrated gate driver,
+ source driver and timing controller. Different panels use these
+ controllers in monochrome or 3-color (black/white/red) configurations.
+ The controller uses a 4-wire SPI interface and requires GPIO pins for
+ reset, busy status, and data/command control. A 3-wire mode is also
+ available where the D/C bit is transmitted along the SPI data line
+ before each byte.
+
+allOf:
+ - $ref: panel/panel-common.yaml#
+ - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+ compatible:
+ oneOf:
+ - items:
+ - enum:
+ # Good Display 4.2" 400x300 monochrome e-paper panel
+ - gooddisplay,gdey042t81
+ - const: solomon,ssd1683
+
+ reg:
+ maxItems: 1
+
+ spi-max-frequency:
+ maximum: 20000000
+
+ spi-3wire: true
+
+ reset-gpios:
+ maxItems: 1
+ description:
+ GPIO connected to the RSTB (reset) pin. Active low.
+
+ busy-gpios:
+ maxItems: 1
+ description:
+ GPIO connected to the BUSY pin. Active high when the controller
+ is busy updating the display.
+
+ dc-gpios:
+ maxItems: 1
+ description:
+ GPIO connected to the D/C (Data/Command) pin. Low for command,
+ high for data. If omitted, the controller is driven in 3-wire
+ (9-bit) SPI mode, where the D/C bit is sent along the data line.
+
+required:
+ - compatible
+ - reg
+ - reset-gpios
+ - busy-gpios
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ display@0 {
+ compatible = "gooddisplay,gdey042t81", "solomon,ssd1683";
+ reg = <0>;
+ spi-max-frequency = <2000000>;
+ reset-gpios = <&gpio1 17 GPIO_ACTIVE_LOW>;
+ busy-gpios = <&gpio1 18 GPIO_ACTIVE_HIGH>;
+ dc-gpios = <&gpio1 19 GPIO_ACTIVE_HIGH>;
+ };
+ };
--
2.39.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v3 02/14] dt-bindings: display: Add Solomon SSD16xx e-paper controller
2026-10-19 21:37 ` [PATCH v3 02/14] dt-bindings: display: Add Solomon SSD16xx e-paper controller Devarsh Thakkar
@ 2026-10-20 8:34 ` Krzysztof Kozlowski
0 siblings, 0 replies; 25+ messages in thread
From: Krzysztof Kozlowski @ 2026-10-20 8:34 UTC (permalink / raw)
To: Devarsh Thakkar
Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Nishanth Menon, Vignesh Raghavendra, Tero Kristo, dri-devel,
devicetree, linux-kernel, linux-arm-kernel, LiangCheng Wang
On Tue, Oct 20, 2026 at 03:07:14AM +0530, Devarsh Thakkar wrote:
> Add device tree binding for the Solomon Systech SSD16xx family of e-Paper
> display controllers (SSD1683 [1], SSD1673 [3], SSD1680 [4], SSD1681 [5]).
>
> The binding covers the 4-wire SPI interface with required GPIO pins for
> reset (active-low), busy status (active-high), and data/command selection.
> The spi-max-frequency is capped at 20 MHz per the SSD1683 datasheet [1]
> which is also the maximum supported frequency amongst the family.
>
> An optional rotation property (as defined by panel-common, in degrees
> counter clockwise) allows static display orientation to be configured from
> device tree.
>
> The data/command selection GPIO pin is also kept as optional since various
> SSD16XX controllers such as SSD1683 [1] support 3-wire mode too with
> data/command selection bit transmitted as first bit before sending the data
> sequence.
>
> The first supported compatible is for Gooddisplay GDEY042T81 [2] which is
> 4.2", 400x300 resolution black/white e-paper display using SSD1683
> controller.
>
> Links:
> [1] : https://www.crystalfontz.com/controllers/SolomonSystech/SSD1683
> [2] : https://files.seeedstudio.com/wiki/Other_Display/42-epaper/GDEY042T81.pdf
> [3] : https://www.crystalfontz.com/controllers/SolomonSystech/SSD1673
> [4] : https://www.crystalfontz.com/controllers/SolomonSystech/SSD1680
> [5] : https://www.crystalfontz.com/controllers/SolomonSystech/SSD1681
>
> Signed-off-by: Devarsh Thakkar <devarsht@ti.com>
> ---
> Changes in v3:
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v3 03/14] dt-bindings: display: solomon, ssd16xx: Add Solomon SSD1677 controller
2026-10-19 21:37 [PATCH v3 00/14] Add DRM driver for Solomon SSD16xx e-paper display controllers Devarsh Thakkar
2026-10-19 21:37 ` [PATCH v3 01/14] dt-bindings: vendor-prefixes: Add Dalian Good Display Co., Ltd Devarsh Thakkar
2026-10-19 21:37 ` [PATCH v3 02/14] dt-bindings: display: Add Solomon SSD16xx e-paper controller Devarsh Thakkar
@ 2026-10-19 21:37 ` Devarsh Thakkar
2026-10-19 21:44 ` [PATCH v3 03/14] dt-bindings: display: solomon,ssd16xx: " sashiko-bot
2026-10-19 21:37 ` [PATCH v3 04/14] drm/solomon: Add DRM driver for Solomon SSD16xx e-paper display controllers Devarsh Thakkar
` (10 subsequent siblings)
13 siblings, 1 reply; 25+ messages in thread
From: Devarsh Thakkar @ 2026-10-19 21:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Nishanth Menon, Vignesh Raghavendra, Tero Kristo
Cc: dri-devel, devicetree, linux-kernel, linux-arm-kernel, devarsht,
LiangCheng Wang
Add solomon,ssd1677 as a compatible controller string.
The SSD1677 supports displays up to 960x680 pixels with 10-bit
pixel-level X/Y RAM addressing. It shares the same command address
space and GPIO interface as the SSD1683 but differs in addressing
model, deep sleep behaviour, and initialisation sequence.
Signed-off-by: Devarsh Thakkar <devarsht@ti.com>
---
Changes in v3:
- Only add the solomon,ssd1677 branch, as the previous patch now already
uses oneOf for compatible
---
Documentation/devicetree/bindings/display/solomon,ssd1683.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/display/solomon,ssd1683.yaml b/Documentation/devicetree/bindings/display/solomon,ssd1683.yaml
index 2e2aa195fb1e..df8d9ff821a7 100644
--- a/Documentation/devicetree/bindings/display/solomon,ssd1683.yaml
+++ b/Documentation/devicetree/bindings/display/solomon,ssd1683.yaml
@@ -31,6 +31,7 @@ properties:
# Good Display 4.2" 400x300 monochrome e-paper panel
- gooddisplay,gdey042t81
- const: solomon,ssd1683
+ - const: solomon,ssd1677
reg:
maxItems: 1
--
2.39.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v3 03/14] dt-bindings: display: solomon,ssd16xx: Add Solomon SSD1677 controller
2026-10-19 21:37 ` [PATCH v3 03/14] dt-bindings: display: solomon, ssd16xx: Add Solomon SSD1677 controller Devarsh Thakkar
@ 2026-10-19 21:44 ` sashiko-bot
0 siblings, 0 replies; 25+ messages in thread
From: sashiko-bot @ 2026-10-19 21:44 UTC (permalink / raw)
To: Devarsh Thakkar; +Cc: dri-devel, devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dt-bindings: display: ssd1683: bare solomon,ssd1677 compatible without panel
--
commit 99bb2813cd624693670923f366680d97f3d2c796
Author: Devarsh Thakkar <devarsht@ti.com>
dt-bindings: display: solomon,ssd16xx: Add Solomon SSD1677 controller
This commit adds solomon,ssd1677 as a compatible string to the Solomon
SSD1683 e-paper display binding.
> diff --git a/Documentation/devicetree/bindings/display/solomon,ssd1683.yaml b/Documentation/devicetree/bindings/display/solomon,ssd1683.yaml
> index 2e2aa195fb1e6..df8d9ff821a78 100644
> --- a/Documentation/devicetree/bindings/display/solomon,ssd1683.yaml
> +++ b/Documentation/devicetree/bindings/display/solomon,ssd1683.yaml
> @@ -31,6 +31,7 @@ properties:
> # Good Display 4.2" 400x300 monochrome e-paper panel
> - gooddisplay,gdey042t81
> - const: solomon,ssd1683
> + - const: solomon,ssd1677
>
> reg:
> maxItems: 1
[Severity: Medium]
Should solomon,ssd1677 be used as a fallback behind a panel-specific
compatible in an items list rather than as a standalone const?
The SSD1677 is a display controller IC that supports panels of varying
resolutions (up to 960x680 pixels), physical dimensions, booster soft-start
tunings, and color configurations. Because this binding does not define
Device Tree properties for panel resolution, timings, or color mode, panel
parameters must be determined by a specific panel compatible string (just as
gooddisplay,gdey042t81 is paired with solomon,ssd1683 above, and as
ssd16xx_of_match in drivers/gpu/drm/solomon/ssd16xx.c expects when
ssd16xx_probe() looks up ssd16xx_device_configs[model]).
Defining solomon,ssd1677 as a standalone const under properties:compatible
allows a bare compatible = "solomon,ssd1677" node that lacks panel geometry
and cannot bind to the driver, while rejecting valid two-entry
panel-plus-controller compatible tuples during dtbs_check validation.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261019213726.4145532-1-devarsht@ti.com?part=3
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v3 04/14] drm/solomon: Add DRM driver for Solomon SSD16xx e-paper display controllers
2026-10-19 21:37 [PATCH v3 00/14] Add DRM driver for Solomon SSD16xx e-paper display controllers Devarsh Thakkar
` (2 preceding siblings ...)
2026-10-19 21:37 ` [PATCH v3 03/14] dt-bindings: display: solomon, ssd16xx: Add Solomon SSD1677 controller Devarsh Thakkar
@ 2026-10-19 21:37 ` Devarsh Thakkar
2026-10-19 22:02 ` sashiko-bot
2026-10-19 21:37 ` [PATCH v3 05/14] drm/solomon: ssd16xx: Add clear_on_init/close/disable session management Devarsh Thakkar
` (9 subsequent siblings)
13 siblings, 1 reply; 25+ messages in thread
From: Devarsh Thakkar @ 2026-10-19 21:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Nishanth Menon, Vignesh Raghavendra, Tero Kristo
Cc: dri-devel, devicetree, linux-kernel, linux-arm-kernel, devarsht,
LiangCheng Wang
Add a DRM driver for the Solomon Systech SSD16xx family of e-paper display
controllers (SSD1680 [4], SSD1673 [3], SSD1681 [5], SSD1683 [1]) based
e-paper display panels, starting with support for the Gooddisplay
GDEY042T81 which is 4.2", 400x300 resolution black/white e-paper display
using SSD1683 controller.
Hardware features supported:
- SPI communication with optional D/C GPIO (4-wire mode) or 9-bit SPI
with embedded D/C# bit (3-wire mode) command/data signaling
- BUSY GPIO polling with configurable timeout
- Hardware reset via dedicated reset GPIO
- Internal temperature sensor readout
- Border waveform control with 10 configurable modes:
LUT0-3 (GS transition waveforms), fixed voltage levels (VSS/VSH1/VSL/
VSH2), VCOM follow, and HiZ (floating, default)
- Deep sleep modes: Mode 1 (RAM retained) and Mode 2 (RAM lost)
- Data entry modes for all four orientations
- Dual display RAM planes: BW RAM (primary image) and RED RAM
(partial refresh baseline / 3-color BWR red pixels)
Refresh modes:
Three independently selectable refresh waveforms:
- Partial (~300-500 ms): low-latency BW update using RED RAM as
waveform transition baseline
- Full (~1.5-2 s): complete pixel-cycle reset with temperature-
compensated LUT reload
- Fast (~1.0-1.5 s): abbreviated waveform using a pre-loaded LUT
Pixel format conversions:
The driver accepts all common DRM formats and converts to 1bpp:
- R1 (native 1bpp), color formats (XRGB8888),
- 3-color BWR: classifies pixels as black, white, or red by dominant
channel; writes separate 1bpp BW and RED buffers
Orientation / rotation:
- Full 4-way rotation: 0° (landscape), 90° (portrait CW),
180° (landscape inverted), 270° (portrait CCW)
- Implemented via the Data Entry Mode register — no GPU or memory
rotation overhead
- Configurable via device tree 'rotation' property
DRM framework integration:
- drm_atomic_helper-based modesetting with damage tracking
- Single primary plane, CRTC, simple encoder, SPI connector
- drm_gem_shmem backend for display buffers
- fbdev fallback via drm_fbdev_shmem_setup
Links:
[1] : https://www.crystalfontz.com/controllers/SolomonSystech/SSD1683
[2] : https://files.seeedstudio.com/wiki/Other_Display/42-epaper/GDEY042T81.pdf
[3] : https://www.crystalfontz.com/controllers/SolomonSystech/SSD1673
[4] : https://www.crystalfontz.com/controllers/SolomonSystech/SSD1680
[5] : https://www.crystalfontz.com/controllers/SolomonSystech/SSD1681
Signed-off-by: Devarsh Thakkar <devarsht@ti.com>
---
Changes in v3:
- Keep lazy hardware initialization: ssd16xx_hw_init() is deferred to
crtc_atomic_enable(), not probe(). probe() performs only a GPIO reset
(ssd16xx_hw_reset()). This avoids unnecessary hw_init in case no app
using the epaper display yet.
- Add drm_dev_enter()/drm_dev_exit() to plane_atomic_update (Thomas,
sashiko-bot)
- Wrap the framebuffer read in drm_gem_fb_begin_cpu_access() and
drm_gem_fb_end_cpu_access() (Thomas)
- Skip the update when drm_atomic_helper_damage_merged() reports no damage
instead of making up a full-screen rectangle (Thomas)
- Remove ssd16xx_crtc_atomic_flush() and last_fb. The orientation is
static, so flush did a full hardware re-init and a second redraw on
every commit, and last_fb had no remaining users (Thomas, sashiko-bot)
- Remove the empty atomic_disable helper; it is re-added with real
content in the clear_on_init/close/disable patch (Thomas)
- Use drm_crtc_cleanup() directly as the CRTC .destroy callback (Thomas)
- Call drm_atomic_helper_check_crtc_primary_plane() from the CRTC
atomic_check (Thomas)
- Send SPI data from a DMA-safe buffer in struct ssd16xx_device instead of
the stack (sashiko-bot)
- Read XRGB8888 pixels with le32_to_cpup()
- Size the transfer buffers per row, as the packing code does
(sashiko-bot)
- Make Kconfig depend on MMU
- GDEY042T81: use the HIZ border waveform for updates and do not refresh
the border on every update, to avoid border flicker
---
drivers/gpu/drm/solomon/Kconfig | 13 +
drivers/gpu/drm/solomon/Makefile | 1 +
drivers/gpu/drm/solomon/ssd16xx.c | 1801 +++++++++++++++++++++++++++++
3 files changed, 1815 insertions(+)
create mode 100644 drivers/gpu/drm/solomon/ssd16xx.c
diff --git a/drivers/gpu/drm/solomon/Kconfig b/drivers/gpu/drm/solomon/Kconfig
index 400a6cab3a67..53120ea6296e 100644
--- a/drivers/gpu/drm/solomon/Kconfig
+++ b/drivers/gpu/drm/solomon/Kconfig
@@ -30,3 +30,16 @@ config DRM_SSD130X_SPI
Say Y here if the SSD13xx OLED display is connected via SPI bus.
If M is selected the module will be called ssd130x-spi.
+
+config DRM_SSD16XX
+ tristate "DRM support for Solomon SSD16xx e-ink display controllers"
+ depends on DRM && SPI && MMU
+ select DRM_CLIENT_SELECTION
+ select DRM_KMS_HELPER
+ select DRM_GEM_SHMEM_HELPER
+ help
+ DRM driver for Solomon SSD16xx family e-ink display controllers
+ (SSD1673, SSD1680, SSD1681, SSD1683, SSD1677). Supports panels
+ such as the GDEY042T81 4.2" 400x300.
+
+ If M is selected the module will be called ssd16xx.
diff --git a/drivers/gpu/drm/solomon/Makefile b/drivers/gpu/drm/solomon/Makefile
index b5fc792257d7..72384620785b 100644
--- a/drivers/gpu/drm/solomon/Makefile
+++ b/drivers/gpu/drm/solomon/Makefile
@@ -1,3 +1,4 @@
obj-$(CONFIG_DRM_SSD130X) += ssd130x.o
obj-$(CONFIG_DRM_SSD130X_I2C) += ssd130x-i2c.o
obj-$(CONFIG_DRM_SSD130X_SPI) += ssd130x-spi.o
+obj-$(CONFIG_DRM_SSD16XX) += ssd16xx.o
diff --git a/drivers/gpu/drm/solomon/ssd16xx.c b/drivers/gpu/drm/solomon/ssd16xx.c
new file mode 100644
index 000000000000..ca6550b5dd5b
--- /dev/null
+++ b/drivers/gpu/drm/solomon/ssd16xx.c
@@ -0,0 +1,1801 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * DRM driver for e-paper display panels using Solomon SSD16xx family controllers
+ *
+ * Copyright (C) 2026 Texas Instruments Incorporated - https://www.ti.com/
+ *
+ * Author: Devarsh Thakkar <devarsht@ti.com>
+ *
+ * References: https://github.com/Lesords/epaper
+ */
+
+#include <linux/delay.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/property.h>
+#include <linux/spi/spi.h>
+
+#include <drm/clients/drm_client_setup.h>
+#include <drm/drm_atomic.h>
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_damage_helper.h>
+#include <drm/drm_drv.h>
+#include <drm/drm_encoder.h>
+#include <drm/drm_fb_helper.h>
+#include <drm/drm_fbdev_shmem.h>
+#include <drm/drm_framebuffer.h>
+#include <drm/drm_gem_atomic_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
+#include <drm/drm_gem_shmem_helper.h>
+#include <drm/drm_managed.h>
+#include <drm/drm_probe_helper.h>
+#include <drm/drm_print.h>
+
+/* -----------------------------------------------------------------------
+ * SSD16xx family common: commands, data values, and bit definitions.
+ * These apply equally to SSD1673, SSD1680, and SSD1683.
+ * -----------------------------------------------------------------------
+ */
+
+/* SPI command codes (common) */
+#define SSD16XX_CMD_DRIVER_OUTPUT_CONTROL 0x01
+#define SSD16XX_CMD_DATA_ENTRY_MODE 0x11
+#define SSD16XX_CMD_SW_RESET 0x12
+#define SSD16XX_CMD_MASTER_ACTIVATION 0x20
+#define SSD16XX_CMD_DISPLAY_UPDATE_CONTROL1 0x21
+#define SSD16XX_CMD_DISPLAY_UPDATE_CONTROL2 0x22
+#define SSD16XX_CMD_WRITE_RAM_BW 0x24
+#define SSD16XX_CMD_BORDER_WAVEFORM_CONTROL 0x3C
+#define SSD16XX_CMD_SET_RAM_X_ADDRESS_START_END 0x44
+#define SSD16XX_CMD_SET_RAM_Y_ADDRESS_START_END 0x45
+#define SSD16XX_CMD_SET_RAM_X_ADDRESS_COUNTER 0x4E
+#define SSD16XX_CMD_SET_RAM_Y_ADDRESS_COUNTER 0x4F
+
+/*
+ * Data Entry Mode (command 0x11) AM/IDY/IDX bit encoding (common).
+ *
+ * Bit 2 (AM): Address update direction: 0 = X direction, 1 = Y direction
+ * ID[1:0] when AM=0 (X-direction modes, address counter advances in X):
+ * 00 = X decrement, Y decrement 01 = X increment, Y decrement
+ * 10 = X decrement, Y increment 11 = X increment, Y increment (default)
+ *
+ * Rotation to data entry mode mapping (actual implementation uses two modes,
+ * with scan direction controlled via RAM cursor positioning and manual tweaking):
+ * 0°/270° → 0x03 (X++, Y++) Landscape/Portrait-CW: cursor at (0, 0)
+ * 90°/180° → 0x00 (X--, Y--) Portrait-CCW/Upside-down: cursor at (max, max)
+ *
+ * The pixel packing in convert_fb_to_1bpp is grouped by physical layout:
+ * - Portrait (90°/270°): column-major packing, rightmost column first
+ * - Landscape (0°/180°): row-major packing, top to bottom, left to right
+ * Hardware cursor position and scan mode handle the final orientation.
+ */
+#define SSD16XX_DATA_ENTRY_XDEC_YDEC 0x00 /* X--, Y-- (X-mode) */
+#define SSD16XX_DATA_ENTRY_XINC_YINC 0x03 /* X++, Y++ (X-mode, default) */
+
+/* POR reset value: GD=0 (G0 first), SM=0 (interlaced), TB=0 (G0->G299) */
+#define SSD16XX_DRIVER_OUTPUT_CTRL_DEFAULT 0x00
+
+/* Display Update Control 1 (0x21) byte 2 default (common) */
+#define SSD16XX_CTRL1_BYTE2_DEFAULT 0x00
+
+/*
+ * Display Update Control 2 (0x22) individual bit definitions (common).
+ * NOTE: BIT(3) is NOT common — see SSD1683_CTRL2_MODE2 in the SSD1683
+ * section below; it has a completely different meaning in SSD1673.
+ */
+#define SSD16XX_CTRL2_ENABLE_CLK BIT(7)
+#define SSD16XX_CTRL2_ENABLE_ANALOG BIT(6)
+#define SSD16XX_CTRL2_LOAD_TEMPERATURE BIT(5)
+#define SSD16XX_CTRL2_LOAD_LUT BIT(4)
+#define SSD16XX_CTRL2_DISPLAY BIT(2)
+#define SSD16XX_CTRL2_DISABLE_ANALOG BIT(1)
+#define SSD16XX_CTRL2_DISABLE_CLK BIT(0)
+
+#define SSD16XX_SPI_BITS_PER_WORD 8
+#define SSD16XX_SPI_SPEED_DEFAULT 1000000
+
+/* Maximum time to wait for the BUSY pin to deassert after a display update */
+#define SSD16XX_BUSY_WAIT_TIMEOUT_MS 6000
+
+/* -----------------------------------------------------------------------
+ * SSD1683 / SSD1680 specific: commands, data values, and bit definitions.
+ * -----------------------------------------------------------------------
+ */
+
+/*
+ * Deep Sleep Mode values (command 0x10).
+ */
+#define SSD1683_DEEP_SLEEP_MODE_1 0x01 /* RAM retained */
+#define SSD1683_DEEP_SLEEP_MODE_2 0x03 /* RAM lost (max power) */
+
+/*
+ * Temperature Sensor Selection (command 0x18).
+ */
+#define SSD1683_CMD_TEMPERATURE_SENSOR_CONTROL 0x18
+#define SSD1683_TEMP_SENSOR_INTERNAL 0x80 /* Bit 7: use internal sensor */
+
+/*
+ * Write RED RAM (command 0x26).
+ */
+#define SSD1683_CMD_WRITE_RAM_RED 0x26
+
+/*
+ * Border Waveform Control (command 0x3C) byte values.
+ */
+#define SSD1683_BORDER_WAVEFORM_LUT0 0x00 /* GS Transition LUT0 (black) */
+#define SSD1683_BORDER_WAVEFORM_LUT1 0x01 /* GS Transition LUT1 (white) */
+#define SSD1683_BORDER_WAVEFORM_LUT2 0x02 /* GS Transition LUT2 (black) */
+#define SSD1683_BORDER_WAVEFORM_LUT3 0x03 /* GS Transition LUT3 (gray) */
+#define SSD1683_BORDER_WAVEFORM_FIXLVL_VSS 0x40 /* Fix Level VSS (0V, black) */
+#define SSD1683_BORDER_WAVEFORM_FIXLVL_VSH1 0x50 /* Fix Level VSH1 (+15V, black) */
+#define SSD1683_BORDER_WAVEFORM_FIXLVL_VSL 0x60 /* Fix Level VSL (-15V, white) */
+#define SSD1683_BORDER_WAVEFORM_FIXLVL_VSH2 0x70 /* Fix Level VSH2 (+15V alt, black) */
+#define SSD1683_BORDER_WAVEFORM_VCOM 0x80 /* Follow VCOM (-2V~-3V, preserve) */
+#define SSD1683_BORDER_WAVEFORM_HIZ 0xC0 /* HiZ (floating, default) */
+
+/*
+ * Display Update Control 1 (0x21) byte 1 — RED RAM control.
+ */
+#define SSD1683_CTRL1_NORMAL 0x00 /* Both BW and RED RAMs enabled */
+#define SSD1683_CTRL1_BYPASS_RED_RAM 0x40 /* Bypass RED RAM (force RED=0) */
+
+/*
+ * Display Update Control 2 (0x22) BIT(3) — "Display Mode 2" (partial/BW).
+ */
+#define SSD1683_CTRL2_MODE2 BIT(3)
+
+/* Composite CTRL2 sequences for each refresh mode */
+#define SSD1683_CTRL2_FULL_REFRESH (SSD16XX_CTRL2_ENABLE_CLK | \
+ SSD16XX_CTRL2_ENABLE_ANALOG | \
+ SSD16XX_CTRL2_LOAD_TEMPERATURE | \
+ SSD16XX_CTRL2_LOAD_LUT | \
+ SSD16XX_CTRL2_DISPLAY | \
+ SSD16XX_CTRL2_DISABLE_ANALOG | \
+ SSD16XX_CTRL2_DISABLE_CLK) /* 0xF7, ~1.5-2s */
+
+#define SSD1683_CTRL2_FAST_REFRESH (SSD16XX_CTRL2_ENABLE_CLK | \
+ SSD16XX_CTRL2_ENABLE_ANALOG | \
+ SSD16XX_CTRL2_DISPLAY | \
+ SSD16XX_CTRL2_DISABLE_ANALOG | \
+ SSD16XX_CTRL2_DISABLE_CLK) /* 0xC7, ~1.0-1.5s */
+
+#define SSD1683_CTRL2_PARTIAL_REFRESH (SSD16XX_CTRL2_ENABLE_CLK | \
+ SSD16XX_CTRL2_ENABLE_ANALOG | \
+ SSD16XX_CTRL2_LOAD_TEMPERATURE | \
+ SSD16XX_CTRL2_LOAD_LUT | \
+ SSD1683_CTRL2_MODE2 | \
+ SSD16XX_CTRL2_DISPLAY | \
+ SSD16XX_CTRL2_DISABLE_ANALOG | \
+ SSD16XX_CTRL2_DISABLE_CLK) /* 0xFF, ~300-500ms */
+
+/*
+ * Standalone LUT pre-load sequence (0x91 = ENABLE_CLK | LOAD_LUT | LOAD_TEMPERATURE |
+ * DISABLE_CLK).
+ * Pre-loads the OTP LUT without triggering a display update. Required for
+ * FAST refresh mode (0xC7) which omits LOAD_LUT from each update cycle.
+ */
+#define SSD1683_CTRL2_LOAD_TEMP_LUT (SSD16XX_CTRL2_ENABLE_CLK | \
+ SSD16XX_CTRL2_LOAD_LUT | \
+ SSD16XX_CTRL2_LOAD_TEMPERATURE | \
+ SSD16XX_CTRL2_DISABLE_CLK) /* 0xB1 */
+
+enum ssd16xx_controller {
+ SSD1683 = 1,
+};
+
+enum ssd16xx_model {
+ GDEY042T81 = 1,
+};
+
+enum ssd16xx_refresh_mode {
+ SSD16XX_REFRESH_PARTIAL = 0, /* Partial refresh (~300-500ms) */
+ SSD16XX_REFRESH_FULL, /* Full refresh (~1.5-2s) */
+ SSD16XX_REFRESH_FAST, /* Fast refresh, skip temp load (~1.0-1.5s) */
+};
+
+enum ssd16xx_color_mode {
+ SSD16XX_COLOR_MODE_BW = 0, /* Black/white only; RED RAM always bypassed */
+ SSD16XX_COLOR_MODE_3COLOR = 1, /* 3-colour BWR; RED RAM used for red pixels */
+};
+
+/* Border waveform enum indices (0-9); mapped to HW bytes via
+ * controller_cfg->border_waveform_table[]
+ */
+enum ssd16xx_border_waveform {
+ SSD16XX_BORDER_LUT0 = 0, /* GS Transition LUT0 (black) */
+ SSD16XX_BORDER_LUT1, /* GS Transition LUT1 (white) */
+ SSD16XX_BORDER_LUT2, /* GS Transition LUT2 (black) */
+ SSD16XX_BORDER_LUT3, /* GS Transition LUT3 (gray) */
+ SSD16XX_BORDER_VSS, /* Fix Level VSS (black) */
+ SSD16XX_BORDER_VSH1, /* Fix Level VSH1 (black) */
+ SSD16XX_BORDER_VSL, /* Fix Level VSL (white) */
+ SSD16XX_BORDER_VSH2, /* Fix Level VSH2 (black) */
+ SSD16XX_BORDER_VCOM, /* Follow VCOM (preserve) */
+ SSD16XX_BORDER_HIZ, /* HiZ (floating, default) */
+};
+
+/* SSD1683/SSD1680 border waveform byte encoding for command 0x3C */
+static const u8 ssd1683_border_waveform_table[] = {
+ [SSD16XX_BORDER_LUT0] = SSD1683_BORDER_WAVEFORM_LUT0,
+ [SSD16XX_BORDER_LUT1] = SSD1683_BORDER_WAVEFORM_LUT1,
+ [SSD16XX_BORDER_LUT2] = SSD1683_BORDER_WAVEFORM_LUT2,
+ [SSD16XX_BORDER_LUT3] = SSD1683_BORDER_WAVEFORM_LUT3,
+ [SSD16XX_BORDER_VSS] = SSD1683_BORDER_WAVEFORM_FIXLVL_VSS,
+ [SSD16XX_BORDER_VSH1] = SSD1683_BORDER_WAVEFORM_FIXLVL_VSH1,
+ [SSD16XX_BORDER_VSL] = SSD1683_BORDER_WAVEFORM_FIXLVL_VSL,
+ [SSD16XX_BORDER_VSH2] = SSD1683_BORDER_WAVEFORM_FIXLVL_VSH2,
+ [SSD16XX_BORDER_VCOM] = SSD1683_BORDER_WAVEFORM_VCOM,
+ [SSD16XX_BORDER_HIZ] = SSD1683_BORDER_WAVEFORM_HIZ,
+};
+
+struct ssd16xx_controller_config {
+ u16 max_width;
+ u16 max_height;
+ u8 ram_x_address_bits;
+ u8 ram_y_address_bits;
+
+ /*
+ * has_temp_sensor_ctrl: controller supports command 0x18 (Temperature
+ * Sensor Selection). Present in SSD1683/SSD1680; absent in SSD1673
+ * which uses command 0x1A (direct temperature write) instead.
+ */
+ bool has_temp_sensor_ctrl;
+
+ /*
+ * Deep sleep mode byte values for command 0x10.
+ * deep_sleep_mode_level1: lower-power sleep, RAM content retained
+ * (MODE_1 on SSD1683/SSD1680; used for runtime idle / app-close).
+ * deep_sleep_mode_level2: maximum power savings, RAM may be lost
+ * (MODE_2 on SSD1683/SSD1680; used for system suspend).
+ * Chips with a single sleep mode set both fields to the same value.
+ */
+ u8 deep_sleep_mode_level1;
+ u8 deep_sleep_mode_level2;
+
+ /*
+ * border_waveform_table: chip-specific byte values for the 10 logical
+ * border waveform modes (indexed by enum ssd16xx_border_waveform).
+ * The encoding of command 0x3C differs between SSD1683/SSD1680 and
+ * SSD1673, so each controller provides its own translation table.
+ */
+ const u8 *border_waveform_table;
+
+ /*
+ * Display Update Control 1 (cmd 0x21) byte 1 values.
+ * ctrl1_normal: both BW and RED RAMs participate in the waveform.
+ * ctrl1_bypass_red_ram: RED RAM bypassed; waveform driven from BW RAM only.
+ * SSD1673 has no RED RAM so both fields carry the same value.
+ */
+ u8 ctrl1_normal;
+ u8 ctrl1_bypass_red_ram;
+
+ /*
+ * Display Update Control 2 (cmd 0x22) composite sequences for each
+ * refresh mode (indexed by enum ssd16xx_refresh_mode) and the
+ * standalone LUT pre-load sequence used before fast refresh.
+ * Values differ between SSD1683/SSD1680 and SSD1673 (MODE2 bit, etc.).
+ */
+ u8 ctrl2_refresh[3]; /* indexed by SSD16XX_REFRESH_PARTIAL/FULL/FAST */
+ u8 ctrl2_load_temp_lut; /* standalone LUT pre-load (no display update) */
+};
+
+struct ssd16xx_device_config {
+ /* Data Entry Mode - controls X/Y increment direction for landscape (0°) */
+ u8 data_entry_mode;
+
+ /* Driver Output Control - third byte (scan direction) */
+ u8 driver_output_ctrl_byte3;
+
+ /* Default refresh mode for this panel */
+ enum ssd16xx_refresh_mode default_refresh_mode;
+
+ /* Default border waveform during clear/init (enum index 0-9) */
+ enum ssd16xx_border_waveform default_border_waveform_init;
+
+ /* Default border waveform during display updates (enum index 0-9) */
+ enum ssd16xx_border_waveform default_border_waveform_update;
+
+ /* Whether to re-send border waveform command before each display update */
+ bool default_border_refresh_on_every_update;
+
+ /*
+ * Default refresh-mode-init: -1=disabled, else skip baseline establishment
+ * and start directly in this refresh mode.
+ */
+ int default_refresh_mode_init;
+
+ /*
+ * Whether this panel has a physical red colour plane (3-colour BWR).
+ * false: 2-colour black/white only; the RED RAM is always bypassed.
+ * true: 3-colour panel; full-refresh writes to the RED RAM so that
+ * red pixels are driven through the red waveform.
+ */
+ bool red_supported;
+
+ /*
+ * Default colour mode for this panel.
+ * For BW-only panels this must be SSD16XX_COLOR_MODE_BW.
+ * For BWR panels this can be set to SSD16XX_COLOR_MODE_3COLOR to
+ * enable red ink by default;
+ */
+ enum ssd16xx_color_mode default_color_mode;
+
+ /* Panel-specific display mode (resolution and physical dimensions) */
+ const struct drm_display_mode *mode;
+};
+
+struct ssd16xx_device {
+ struct drm_device drm;
+
+ struct drm_plane primary_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;
+
+ enum ssd16xx_model model;
+ enum ssd16xx_controller controller;
+ const struct ssd16xx_controller_config *controller_cfg;
+ const struct ssd16xx_device_config *device_cfg;
+ struct drm_display_mode *mode;
+ u32 width;
+ u32 height;
+
+ bool initialized;
+ bool init_refresh_pending; /* First frame after refresh_mode_init enable */
+
+ int orientation; /* Display orientation in degrees: 0/90/180/270 */
+ enum ssd16xx_refresh_mode refresh_mode; /* Active refresh mode */
+ enum ssd16xx_color_mode color_mode; /* Active color mode (BW or 3-color) */
+ bool fast_lut_pending; /* LUT pre-load needed before next fast refresh */
+
+ /* Border waveform (as enum indices) */
+ int border_waveform_init_idx; /* Border waveform during clear/init */
+ int border_waveform_update_idx; /* Border waveform during display updates */
+ bool border_refresh_on_every_update; /* Re-send border cmd each display update */
+ bool border_waveform_pending; /* One-shot: send border cmd on next update */
+
+ /* Display control */
+ int refresh_mode_init; /* -1=disabled, else use this mode for the first frame */
+
+ u8 *tx_buf; /* 1bpp frame buffer (mono + white) */
+ u8 *tx_red_buf; /* 1bpp red-channel buffer (3-color panels only) */
+ u16 *tx_buf9; /* 9-bit SPI expansion buffer (3-wire mode only) */
+ u8 spi_buf[64] ____cacheline_aligned; /* DMA-safe buffer for SPI transfers */
+};
+
+static inline struct ssd16xx_device *to_ssd16xx_device(struct drm_device *drm)
+{
+ return container_of(drm, struct ssd16xx_device, drm);
+}
+
+static inline struct ssd16xx_device *crtc_to_ssd16xx_device(struct drm_crtc *crtc)
+{
+ return container_of(crtc, struct ssd16xx_device, crtc);
+}
+
+static inline struct ssd16xx_device *plane_to_ssd16xx_device(struct drm_plane *plane)
+{
+ return container_of(plane, struct ssd16xx_device, primary_plane);
+}
+
+static const struct ssd16xx_controller_config ssd16xx_controller_configs[] = {
+ [SSD1683] = {
+ .max_width = 400,
+ .max_height = 300,
+ .ram_x_address_bits = 8,
+ .ram_y_address_bits = 16,
+ .has_temp_sensor_ctrl = true,
+ .deep_sleep_mode_level1 = SSD1683_DEEP_SLEEP_MODE_1,
+ .deep_sleep_mode_level2 = SSD1683_DEEP_SLEEP_MODE_2,
+ .border_waveform_table = ssd1683_border_waveform_table,
+ .ctrl1_normal = SSD1683_CTRL1_NORMAL,
+ .ctrl1_bypass_red_ram = SSD1683_CTRL1_BYPASS_RED_RAM,
+ .ctrl2_refresh = {
+ [SSD16XX_REFRESH_PARTIAL] = SSD1683_CTRL2_PARTIAL_REFRESH,
+ [SSD16XX_REFRESH_FULL] = SSD1683_CTRL2_FULL_REFRESH,
+ [SSD16XX_REFRESH_FAST] = SSD1683_CTRL2_FAST_REFRESH,
+ },
+ .ctrl2_load_temp_lut = SSD1683_CTRL2_LOAD_TEMP_LUT,
+ },
+};
+
+/* GDEY042T81: 4.2" 400x300 panel, 84.8x63.6mm active area */
+static const struct drm_display_mode gdey042t81_mode = {
+ DRM_SIMPLE_MODE(400, 300, 85, 64),
+};
+
+static const struct ssd16xx_device_config ssd16xx_device_configs[] = {
+ [GDEY042T81] = {
+ .data_entry_mode = SSD16XX_DATA_ENTRY_XINC_YINC,
+ .driver_output_ctrl_byte3 = SSD16XX_DRIVER_OUTPUT_CTRL_DEFAULT,
+ .default_refresh_mode = SSD16XX_REFRESH_PARTIAL,
+ .default_border_waveform_init = SSD16XX_BORDER_LUT1, /* white, clean clear */
+ .default_border_waveform_update = SSD16XX_BORDER_HIZ, /* floating, preserve */
+ .default_border_refresh_on_every_update = false,
+ .default_refresh_mode_init = SSD16XX_REFRESH_FULL,
+ .red_supported = false, /* 2-colour black/white panel */
+ .default_color_mode = SSD16XX_COLOR_MODE_BW,
+ .mode = &gdey042t81_mode,
+ },
+};
+
+static void ssd16xx_wait_for_device(struct ssd16xx_device *device,
+ int *err)
+{
+ unsigned long timeout_jiffies = jiffies +
+ msecs_to_jiffies(SSD16XX_BUSY_WAIT_TIMEOUT_MS);
+ unsigned long start_ms = jiffies_to_msecs(jiffies);
+ int busy_val;
+
+ if (*err)
+ return;
+
+ busy_val = gpiod_get_value_cansleep(device->busy);
+ drm_dbg(&device->drm, "BUSY initial value: %d\n", busy_val);
+
+ while (gpiod_get_value_cansleep(device->busy) == 1) {
+ if (time_after(jiffies, timeout_jiffies)) {
+ drm_err(&device->drm, "Busy wait timed out after %lums\n",
+ jiffies_to_msecs(jiffies) - start_ms);
+ *err = -ETIMEDOUT;
+ return;
+ }
+ usleep_range(1000, 2000);
+ }
+
+ drm_dbg(&device->drm, "BUSY became ready after %lums\n",
+ jiffies_to_msecs(jiffies) - start_ms);
+}
+
+static void ssd16xx_spi_sync(struct spi_device *spi, struct spi_message *msg,
+ int *err)
+{
+ int ret;
+
+ if (*err)
+ return;
+
+ ret = spi_sync(spi, msg);
+ if (ret < 0)
+ *err = ret;
+}
+
+static void ssd16xx_send_cmd(struct ssd16xx_device *device, u8 cmd,
+ int *err)
+{
+ struct spi_transfer xfer = {};
+ struct spi_message msg;
+
+ if (*err)
+ return;
+
+ spi_message_init(&msg);
+ spi_message_add_tail(&xfer, &msg);
+
+ if (device->dc) {
+ /* 4-wire SPI: D/C# GPIO low selects command mode */
+ device->spi_buf[0] = cmd;
+ xfer.tx_buf = device->spi_buf;
+ xfer.len = 1;
+ gpiod_set_value_cansleep(device->dc, 0);
+ } else {
+ /*
+ * 3-wire SPI (9-bit): bit 8 is the D/C# bit.
+ * D/C# = 0 means the following 8 bits are a command.
+ */
+ u16 *word = (u16 *)device->spi_buf;
+
+ *word = cmd; /* bit 8 = 0 for command */
+ xfer.tx_buf = device->spi_buf;
+ xfer.len = sizeof(u16);
+ xfer.bits_per_word = 9;
+ }
+
+ ssd16xx_spi_sync(device->spi, &msg, err);
+}
+
+static void ssd16xx_send_data(struct ssd16xx_device *device, u8 data,
+ int *err)
+{
+ struct spi_transfer xfer = {};
+ struct spi_message msg;
+
+ if (*err)
+ return;
+
+ spi_message_init(&msg);
+ spi_message_add_tail(&xfer, &msg);
+
+ if (device->dc) {
+ /* 4-wire SPI: D/C# GPIO high selects data mode */
+ device->spi_buf[0] = data;
+ xfer.tx_buf = device->spi_buf;
+ xfer.len = 1;
+ gpiod_set_value_cansleep(device->dc, 1);
+ } else {
+ /*
+ * 3-wire SPI (9-bit): bit 8 is the D/C# bit.
+ * D/C# = 1 means the following 8 bits are data.
+ */
+ u16 *word = (u16 *)device->spi_buf;
+
+ *word = 0x100 | data;
+ xfer.tx_buf = device->spi_buf;
+ xfer.len = sizeof(u16);
+ xfer.bits_per_word = 9;
+ }
+
+ ssd16xx_spi_sync(device->spi, &msg, err);
+}
+
+static void ssd16xx_send_x_param(struct ssd16xx_device *device, u16 x,
+ int *err)
+{
+ if (*err)
+ return;
+
+ if (device->controller_cfg->ram_x_address_bits == 8) {
+ ssd16xx_send_data(device, (u8)x, err);
+ } else {
+ ssd16xx_send_data(device, x & 0xFF, err);
+ ssd16xx_send_data(device, (x >> 8) & 0xFF, err);
+ }
+}
+
+static void ssd16xx_send_y_param(struct ssd16xx_device *device, u16 y,
+ int *err)
+{
+ if (*err)
+ return;
+
+ if (device->controller_cfg->ram_y_address_bits == 8) {
+ ssd16xx_send_data(device, (u8)y, err);
+ } else {
+ ssd16xx_send_data(device, y & 0xFF, err);
+ ssd16xx_send_data(device, (y >> 8) & 0xFF, err);
+ }
+}
+
+static void ssd16xx_send_data_bulk(struct ssd16xx_device *device,
+ const u8 *data, size_t len,
+ int *err)
+{
+ struct spi_transfer xfer = {};
+ struct spi_message msg;
+
+ if (*err)
+ return;
+
+ if (!data || !len)
+ return;
+
+ spi_message_init(&msg);
+ spi_message_add_tail(&xfer, &msg);
+
+ if (device->dc) {
+ /* 4-wire SPI: D/C# GPIO high selects data mode */
+ if (len <= sizeof(device->spi_buf)) {
+ memcpy(device->spi_buf, data, len);
+ xfer.tx_buf = device->spi_buf;
+ } else {
+ xfer.tx_buf = data;
+ }
+ xfer.len = len;
+ gpiod_set_value_cansleep(device->dc, 1);
+ ssd16xx_spi_sync(device->spi, &msg, err);
+ } else {
+ /* 3-wire (9-bit): expand u8 → u16 with D/C#=1 in bit 8. */
+ size_t i;
+ u16 *buf = device->tx_buf9;
+
+ for (i = 0; i < len; i++)
+ buf[i] = 0x100 | data[i];
+
+ xfer.tx_buf = buf;
+ xfer.len = len * sizeof(u16);
+ xfer.bits_per_word = 9;
+ ssd16xx_spi_sync(device->spi, &msg, err);
+ }
+}
+
+static void ssd16xx_display_update(struct ssd16xx_device *device,
+ u8 ctrl1_byte1, u8 ctrl1_byte2, u8 ctrl2_mode,
+ int *err)
+{
+ if (*err)
+ return;
+
+ drm_dbg(&device->drm,
+ "display_update: Setting ctrl1=0x%02x,0x%02x mode=0x%02x\n",
+ ctrl1_byte1, ctrl1_byte2, ctrl2_mode);
+
+ ssd16xx_send_cmd(device, SSD16XX_CMD_DISPLAY_UPDATE_CONTROL1, err);
+ ssd16xx_send_data(device, ctrl1_byte1, err);
+ ssd16xx_send_data(device, ctrl1_byte2, err);
+
+ ssd16xx_send_cmd(device, SSD16XX_CMD_DISPLAY_UPDATE_CONTROL2, err);
+ ssd16xx_send_data(device, ctrl2_mode, err);
+ ssd16xx_send_cmd(device, SSD16XX_CMD_MASTER_ACTIVATION, err);
+
+ drm_dbg(&device->drm,
+ "display_update: Master activation sent, waiting...\n");
+
+ ssd16xx_wait_for_device(device, err);
+}
+
+static void ssd16xx_hw_reset(struct ssd16xx_device *device)
+{
+ gpiod_set_value_cansleep(device->reset, 1);
+ usleep_range(10000, 11000);
+ gpiod_set_value_cansleep(device->reset, 0);
+ usleep_range(10000, 11000);
+}
+
+/*
+ * ssd16xx_preload_fast_lut() - pre-load the OTP LUT for fast refresh mode.
+ *
+ * Fast refresh (CTRL2 = 0xC7) omits the LOAD_LUT step on every update to save
+ * time. It relies on the LUT being loaded upfront via this standalone sequence
+ * (CTRL2 = 0xB1: ENABLE_CLK | LOAD_LUT | SSD16XX_CTRL2_LOAD_TEMPERATURE | DISABLE_CLK,
+ * no display update).
+ *
+ * Must be called when:
+ * a) hw_init runs with refresh_mode == FAST, and
+ * b) switching to fast refresh from a mode that did not leave a valid Mode1
+ * LUT in the controller (i.e. previous mode was not FULL refresh, which
+ * carries LOAD_LUT in its own CTRL2 sequence).
+ */
+static int ssd16xx_preload_fast_lut(struct ssd16xx_device *device)
+{
+ int err = 0;
+
+ ssd16xx_send_cmd(device, SSD16XX_CMD_DISPLAY_UPDATE_CONTROL1, &err);
+ ssd16xx_send_data(device, device->controller_cfg->ctrl1_bypass_red_ram, &err);
+ ssd16xx_send_data(device, SSD16XX_CTRL1_BYTE2_DEFAULT, &err);
+
+ ssd16xx_send_cmd(device, SSD16XX_CMD_DISPLAY_UPDATE_CONTROL2, &err);
+ ssd16xx_send_data(device, device->controller_cfg->ctrl2_load_temp_lut, &err);
+
+ ssd16xx_send_cmd(device, SSD16XX_CMD_MASTER_ACTIVATION, &err);
+ ssd16xx_wait_for_device(device, &err);
+
+ return err;
+}
+
+static int ssd16xx_hw_init(struct ssd16xx_device *device)
+{
+ int err = 0;
+ u8 data_entry_mode;
+ /*
+ * Driver Output Control MUX ratio = (gate lines - 1).
+ * Use the actual device height, not the controller maximum —
+ * a smaller device must only drive its own gate lines.
+ */
+
+ ssd16xx_hw_reset(device);
+
+ /* Software reset */
+ ssd16xx_send_cmd(device, SSD16XX_CMD_SW_RESET, &err);
+ ssd16xx_wait_for_device(device, &err);
+
+ /* Driver output control (0x01): MUX ratio and scan direction. */
+ ssd16xx_send_cmd(device, SSD16XX_CMD_DRIVER_OUTPUT_CONTROL, &err);
+ ssd16xx_send_y_param(device, device->height - 1, &err);
+ ssd16xx_send_data(device, device->device_cfg->driver_output_ctrl_byte3, &err);
+
+ /* Internal temperature sensor (SSD1683/SSD1680 only; not present in SSD1673) */
+ if (device->controller_cfg->has_temp_sensor_ctrl) {
+ ssd16xx_send_cmd(device, SSD1683_CMD_TEMPERATURE_SENSOR_CONTROL, &err);
+ ssd16xx_send_data(device, SSD1683_TEMP_SENSOR_INTERNAL, &err);
+ }
+
+ /*
+ * For FAST refresh mode, pre-load the LUT once here during initialization.
+ * FAST mode ctrl2 (0xC7) omits LOAD_LUT on every update for speed, so the
+ * LUT must be loaded upfront. FULL (0xF7) and PARTIAL (0xFF) load LUT on
+ * every update, so no preload is needed for those modes.
+ */
+ if (device->refresh_mode == SSD16XX_REFRESH_FAST) {
+ ssd16xx_send_cmd(device, SSD16XX_CMD_DISPLAY_UPDATE_CONTROL1, &err);
+ ssd16xx_send_data(device, device->controller_cfg->ctrl1_bypass_red_ram, &err);
+ ssd16xx_send_data(device, SSD16XX_CTRL1_BYTE2_DEFAULT, &err);
+
+ ssd16xx_send_cmd(device, SSD16XX_CMD_DISPLAY_UPDATE_CONTROL2, &err);
+ ssd16xx_send_data(device, device->controller_cfg->ctrl2_load_temp_lut, &err);
+
+ ssd16xx_send_cmd(device, SSD16XX_CMD_MASTER_ACTIVATION, &err);
+ ssd16xx_wait_for_device(device, &err);
+ }
+
+ /*
+ * Set Data Entry Mode (0x11) based on orientation. This controls
+ * how the RAM address counter auto-advances after each byte write.
+ *
+ * Implementation uses two data entry modes:
+ * - 90°/180° use XDEC_YDEC (0x00): X--, Y-- with cursor at (max, max)
+ * - 0°/270° use XINC_YINC (0x03): X++, Y++ with cursor at (0, 0)
+ *
+ * The convert_fb_to_1bpp packing is grouped by physical layout:
+ * - Portrait orientations (90°/270°): column-major packing
+ * - Landscape orientations (0°/180°): row-major packing
+ *
+ * Final scan direction and image orientation are controlled by the
+ * combination of data entry mode and RAM cursor position set in fb_dirty.
+ *
+ * The RAM address window and cursor are NOT set here; fb_dirty
+ * always programmes them (with the correct end-before-start order
+ * for decrement modes) immediately before writing frame data.
+ */
+ switch (device->orientation) {
+ case 90:
+ case 180:
+ data_entry_mode = SSD16XX_DATA_ENTRY_XDEC_YDEC;
+ break;
+ default: /* 0°/270° */
+ data_entry_mode = SSD16XX_DATA_ENTRY_XINC_YINC;
+ break;
+ }
+
+ ssd16xx_send_cmd(device, SSD16XX_CMD_DATA_ENTRY_MODE, &err);
+ ssd16xx_send_data(device, data_entry_mode, &err);
+ drm_dbg(&device->drm, "hw_init: orientation=%u° data_entry=0x%02x\n",
+ device->orientation, data_entry_mode);
+
+ ssd16xx_wait_for_device(device, &err);
+
+ if (err)
+ drm_err(&device->drm, "Hardware initialization failed: %d\n", err);
+
+ return err;
+}
+
+/*
+ * ssd16xx_pixel_luma() - return ITU-R BT.601 luminance (0-255) for one pixel.
+ *
+ * Currently only XRGB8888 is supported. The function is retained as a named
+ * helper to make it straightforward to add further formats in the future.
+ * R1 is never passed here — it is already 1bpp and handled directly by callers.
+ */
+static u8 ssd16xx_pixel_luma(struct iosys_map *src,
+ struct drm_framebuffer *fb,
+ unsigned int x, unsigned int y)
+{
+ u32 *line = (u32 *)(src->vaddr + y * fb->pitches[0]);
+ u32 px = le32_to_cpup(&line[x]);
+ u8 r = (px >> 16) & 0xFF, g = (px >> 8) & 0xFF, b = px & 0xFF;
+
+ return (u8)((299u * r + 587u * g + 114u * b) / 1000u);
+}
+
+/*
+ * ssd16xx_pixel_is_white() - test whether an XRGB8888 or R1 pixel is white.
+ *
+ * For XRGB8888: ITU-R BT.601 luminance > 127 maps to white.
+ * For R1: the pixel's bit value directly encodes white (1) or black (0).
+ */
+static bool ssd16xx_pixel_is_white(struct iosys_map *src,
+ struct drm_framebuffer *fb,
+ unsigned int x, unsigned int y)
+{
+ if (fb->format->format == DRM_FORMAT_R1) {
+ u8 *line = (u8 *)(src->vaddr + y * fb->pitches[0]);
+
+ return !!(line[x / 8] & (1 << (7 - (x % 8))));
+ }
+ return ssd16xx_pixel_luma(src, fb, x, y) > 127;
+}
+
+/*
+ * ssd16xx_pixel_is_red() - test whether an XRGB8888 pixel is dominated by red.
+ *
+ * Returns true when red exceeds 50% intensity and is strictly greater than
+ * both green and blue (dominant red hue). R1 carries no colour information
+ * and always returns false.
+ */
+static bool ssd16xx_pixel_is_red(struct iosys_map *src,
+ struct drm_framebuffer *fb,
+ unsigned int x, unsigned int y)
+{
+ u32 *line;
+ u32 px;
+ u8 r, g, b;
+
+ if (fb->format->format != DRM_FORMAT_XRGB8888)
+ return false;
+
+ line = (u32 *)(src->vaddr + y * fb->pitches[0]);
+ px = le32_to_cpup(&line[x]);
+ r = (px >> 16) & 0xFF;
+ g = (px >> 8) & 0xFF;
+ b = px & 0xFF;
+
+ return r > 127 && r > g && r > b;
+}
+
+/*
+ * ssd16xx_convert_fb_to_3color() - split a framebuffer into BW and RED planes.
+ * @bw_dst: output buffer for the black/white RAM plane (1=white, 0=black)
+ * @red_dst: output buffer for the red RAM plane (1=red, 0=not red)
+ *
+ * Supports XRGB8888 and R1 formats.
+ *
+ * XRGB8888: pixels with a dominant red channel (r > 127 && r > g && r > b)
+ * map to red ink; remaining pixels threshold to white/black via BT.601 luma.
+ *
+ * R1 on a 3-color panel: the 1-bits are interpreted as red ink on a white
+ * background. BW RAM is set to all-white and RED RAM receives the R1 data
+ * directly (1=red, 0=no red). This matches the behaviour of a user who
+ * explicitly selects color_mode=3-color and submits a 1-bit mask to place
+ * red ink.
+ */
+static void ssd16xx_convert_fb_to_3color(u8 *bw_dst, u8 *red_dst,
+ struct iosys_map *src,
+ struct drm_framebuffer *fb,
+ struct drm_rect *rect)
+{
+ unsigned int x, y;
+ u8 bw_byte = 0, red_byte = 0;
+ unsigned int bit_pos = 0;
+ unsigned int dst_idx = 0;
+
+ if (fb->format->format == DRM_FORMAT_R1) {
+ unsigned int src_pitch = fb->pitches[0];
+ unsigned int width_bytes = drm_rect_width(rect) / 8;
+ unsigned int data_size = width_bytes * drm_rect_height(rect);
+
+ /* White background — no black pixels, only red ink shows */
+ memset(bw_dst, 0xFF, data_size);
+ /* RED RAM: copy R1 data directly (1=red ink, 0=no red) */
+ for (y = rect->y1; y < rect->y2; y++) {
+ u8 *line = src->vaddr + y * src_pitch + (rect->x1 / 8);
+
+ memcpy(red_dst + dst_idx, line, width_bytes);
+ dst_idx += width_bytes;
+ }
+ return;
+ }
+
+ /* XRGB8888 */
+ for (y = rect->y1; y < rect->y2; y++) {
+ for (x = rect->x1; x < rect->x2; x++) {
+ bool is_red = ssd16xx_pixel_is_red(src, fb, x, y);
+
+ if (is_red)
+ red_byte |= (1 << (7 - bit_pos));
+ else if (ssd16xx_pixel_is_white(src, fb, x, y))
+ bw_byte |= (1 << (7 - bit_pos));
+ if (++bit_pos == 8) {
+ bw_dst[dst_idx] = bw_byte;
+ red_dst[dst_idx] = red_byte;
+ dst_idx++;
+ bw_byte = 0;
+ red_byte = 0;
+ bit_pos = 0;
+ }
+ }
+ if (bit_pos > 0) {
+ bw_dst[dst_idx] = bw_byte;
+ red_dst[dst_idx] = red_byte;
+ dst_idx++;
+ bw_byte = 0;
+ red_byte = 0;
+ bit_pos = 0;
+ }
+ }
+}
+
+/*
+ * Convert framebuffer to 1-bit monochrome for e-paper display.
+ *
+ * Supports XRGB8888 (thresholded via ITU-R BT.601 luma at 127) and R1
+ * (native 1bpp, zero-copy fast path for aligned landscape frames).
+ *
+ * Output layout:
+ * 0°/180° landscape: row-major, left-to-right, top-to-bottom
+ * 90°/270° CW portrait: column-major, rightmost column first
+ */
+static void ssd16xx_convert_fb_to_1bpp(u8 *dst, struct iosys_map *src,
+ struct drm_framebuffer *fb,
+ struct drm_rect *rect,
+ unsigned int orientation)
+{
+ u32 format = fb->format->format;
+ int x, y;
+ u8 byte = 0;
+ unsigned int bit_pos = 0;
+ unsigned int dst_idx = 0;
+
+ /* Use fixed threshold of 127 for grayscale to monochrome conversion. */
+ drm_dbg(fb->dev,
+ "convert_1bpp: fmt=%p4cc rect=(%d,%d)-(%d,%d) orient=%u° path=%s\n",
+ &fb->format->format,
+ rect->x1, rect->y1, rect->x2, rect->y2,
+ orientation,
+ (format == DRM_FORMAT_R1 && orientation == 0 && rect->x1 % 8 == 0) ? "R1-fast" :
+ (orientation == 90 || orientation == 270) ? "portrait" : "landscape");
+
+ /*
+ * R1 fast path: 0° landscape with byte-aligned rect.
+ * R1 is already 1bpp so landscape rows map directly to output bytes via
+ * memcpy — no per-pixel computation needed. rect->x1 must be a
+ * multiple of 8 so that (rect->x1 / 8) gives the correct byte offset;
+ * if not, the generic pixel-by-pixel loop below handles non-aligned
+ * rects safely.
+ */
+ if (format == DRM_FORMAT_R1 && orientation == 0 && rect->x1 % 8 == 0) {
+ unsigned int src_pitch = fb->pitches[0];
+ unsigned int width_bytes = drm_rect_width(rect) / 8;
+
+ for (y = rect->y1; y < rect->y2; y++) {
+ u8 *src_line = src->vaddr + y * src_pitch + (rect->x1 / 8);
+
+ memcpy(dst + dst_idx, src_line, width_bytes);
+ dst_idx += width_bytes;
+ }
+ return;
+ }
+
+ switch (orientation) {
+ case 90:
+ case 270:
+ /*
+ * Portrait (90° or 270°): column-major packing.
+ * Each portrait source column becomes one physical RAM row.
+ * The data entry mode and cursor position control scan direction.
+ */
+ for (x = rect->x2 - 1; x >= (int)rect->x1; x--) {
+ for (y = rect->y1; y < rect->y2; y++) {
+ if (ssd16xx_pixel_is_white(src, fb, x, y))
+ byte |= (1 << (7 - bit_pos));
+ if (++bit_pos == 8) {
+ dst[dst_idx++] = byte;
+ byte = 0;
+ bit_pos = 0;
+ }
+ }
+ if (bit_pos > 0) {
+ dst[dst_idx++] = byte;
+ byte = 0;
+ bit_pos = 0;
+ }
+ }
+ break;
+
+ case 0:
+ case 180:
+ default:
+ /*
+ * Landscape (0° or 180°): row-major packing.
+ * Each landscape source row becomes one physical RAM row.
+ * The data entry mode and cursor position control scan direction.
+ */
+ for (y = rect->y1; y < rect->y2; y++) {
+ for (x = rect->x1; x < rect->x2; x++) {
+ if (ssd16xx_pixel_is_white(src, fb, x, y))
+ byte |= (1 << (7 - bit_pos));
+ if (++bit_pos == 8) {
+ dst[dst_idx++] = byte;
+ byte = 0;
+ bit_pos = 0;
+ }
+ }
+ if (bit_pos > 0) {
+ dst[dst_idx++] = byte;
+ byte = 0;
+ bit_pos = 0;
+ }
+ }
+ break;
+ }
+}
+
+static int ssd16xx_fb_dirty(struct drm_framebuffer *fb, struct drm_rect *rect,
+ struct ssd16xx_device *device,
+ const struct iosys_map *src_map)
+{
+ const u8 *ctrl2_tbl = device->controller_cfg->ctrl2_refresh;
+ struct iosys_map map = *src_map;
+ int err = 0;
+ unsigned int data_size = (device->width * device->height) / 8;
+ u8 *mono_buffer = NULL;
+ u8 *red_buffer = NULL;
+ u16 ram_x_start, ram_x_end, ram_y_start, ram_y_end;
+
+ /*
+ * Process full display area. The rect for convert_fb_to_1bpp uses
+ * the framebuffer (logical) dimensions - the pixel iteration walks
+ * the source fb coordinate space. RAM window registers below use
+ * the physical panel dimensions (device->width/height).
+ */
+ rect->x1 = 0;
+ rect->y1 = 0;
+ rect->x2 = fb->width;
+ rect->y2 = fb->height;
+
+ drm_dbg(&device->drm,
+ "fb_dirty: fb=%dx%d, refresh_mode=%d, orientation=%d\n",
+ fb->width, fb->height, device->refresh_mode, device->orientation);
+
+ mono_buffer = device->tx_buf;
+ memset(mono_buffer, 0, data_size);
+
+ /* 3-colour FULL/FAST: populate red channel. */
+ if (device->color_mode == SSD16XX_COLOR_MODE_3COLOR &&
+ (device->refresh_mode == SSD16XX_REFRESH_FULL ||
+ device->refresh_mode == SSD16XX_REFRESH_FAST)) {
+ red_buffer = device->tx_red_buf;
+ memset(red_buffer, 0, data_size);
+ }
+
+ /*
+ * R1 format interpretation depends on color_mode:
+ *
+ * color_mode = 3-color (red_buffer != NULL):
+ * R1 bits are treated as red ink — 1 = red pixel on a white
+ * background, 0 = no red. This allows applications to submit a
+ * 1-bit mask to place red ink on a BWR panel.
+ * Handled by the R1 path in ssd16xx_convert_fb_to_3color().
+ *
+ * color_mode = BW (red_buffer == NULL):
+ * R1 bits are treated as luma — 1 = white, 0 = black.
+ * Handled by the native fast path in ssd16xx_convert_fb_to_1bpp().
+ */
+ if (red_buffer)
+ ssd16xx_convert_fb_to_3color(mono_buffer, red_buffer, &map, fb, rect);
+ else
+ ssd16xx_convert_fb_to_1bpp(mono_buffer, &map, fb, rect, device->orientation);
+
+ drm_dbg(&device->drm,
+ "fb_dirty: mono[0..3]=0x%02x 0x%02x 0x%02x 0x%02x (data_size=%u)\n",
+ mono_buffer[0], mono_buffer[1], mono_buffer[2], mono_buffer[3],
+ data_size);
+
+ /* Set RAM window and cursor for current orientation. */
+ ram_x_start = 0;
+ /*
+ * X end depends on the controller's addressing model:
+ * Byte-addressed (ram_x_address_bits == 8, e.g. SSD1683):
+ * XEnd = device_width/8 - 1 (byte offset into RAM row)
+ * Pixel-addressed (ram_x_address_bits != 8, e.g. SSD1677):
+ * XEnd = device_width - 1 (direct pixel index)
+ * The model is controller-specific; the value is device-specific.
+ */
+ if (device->controller_cfg->ram_x_address_bits == 8)
+ ram_x_end = (device->width / 8) - 1;
+ else
+ ram_x_end = device->width - 1;
+ ram_y_start = 0;
+ ram_y_end = device->height - 1;
+
+ switch (device->orientation) {
+ case 90:
+ case 180:
+ /* 90°/180°: XDEC_YDEC mode, send end-before-start; cursor at (max, max). */
+ ssd16xx_send_cmd(device, SSD16XX_CMD_SET_RAM_X_ADDRESS_START_END, &err);
+ ssd16xx_send_x_param(device, ram_x_end, &err);
+ ssd16xx_send_x_param(device, ram_x_start, &err);
+
+ ssd16xx_send_cmd(device, SSD16XX_CMD_SET_RAM_Y_ADDRESS_START_END, &err);
+ ssd16xx_send_y_param(device, ram_y_end, &err);
+ ssd16xx_send_y_param(device, ram_y_start, &err);
+
+ ssd16xx_send_cmd(device, SSD16XX_CMD_SET_RAM_X_ADDRESS_COUNTER, &err);
+ ssd16xx_send_x_param(device, ram_x_end, &err);
+
+ ssd16xx_send_cmd(device, SSD16XX_CMD_SET_RAM_Y_ADDRESS_COUNTER, &err);
+ ssd16xx_send_y_param(device, ram_y_end, &err);
+ break;
+
+ default: /* 0°/270° */
+ /* 0°/270°: XINC_YINC mode, cursor at (0, 0). */
+ ssd16xx_send_cmd(device, SSD16XX_CMD_SET_RAM_X_ADDRESS_START_END, &err);
+ ssd16xx_send_x_param(device, ram_x_start, &err);
+ ssd16xx_send_x_param(device, ram_x_end, &err);
+
+ ssd16xx_send_cmd(device, SSD16XX_CMD_SET_RAM_Y_ADDRESS_START_END, &err);
+ ssd16xx_send_y_param(device, ram_y_start, &err);
+ ssd16xx_send_y_param(device, ram_y_end, &err);
+
+ ssd16xx_send_cmd(device, SSD16XX_CMD_SET_RAM_X_ADDRESS_COUNTER, &err);
+ ssd16xx_send_x_param(device, ram_x_start, &err);
+
+ ssd16xx_send_cmd(device, SSD16XX_CMD_SET_RAM_Y_ADDRESS_COUNTER, &err);
+ ssd16xx_send_y_param(device, ram_y_start, &err);
+ break;
+ }
+
+ ssd16xx_send_cmd(device, SSD16XX_CMD_WRITE_RAM_BW, &err);
+ ssd16xx_send_data_bulk(device, mono_buffer, data_size, &err);
+
+ /* Re-send border waveform when: every-update mode, init frame
+ */
+ drm_dbg(&device->drm,
+ "fb_dirty: border check: every_update=%d init_pending=%d border_pending=%d idx=%d hw=0x%02x\n",
+ device->border_refresh_on_every_update, device->init_refresh_pending,
+ device->border_waveform_pending, device->border_waveform_update_idx,
+ device->controller_cfg->border_waveform_table[device->border_waveform_update_idx]);
+ if (device->border_refresh_on_every_update || device->init_refresh_pending ||
+ device->border_waveform_pending) {
+ u8 idx = device->border_waveform_update_idx;
+ u8 border = device->controller_cfg->border_waveform_table[idx];
+
+ drm_dbg(&device->drm, "fb_dirty: Sending border waveform: 0x%02x\n",
+ border);
+ ssd16xx_send_cmd(device, SSD16XX_CMD_BORDER_WAVEFORM_CONTROL, &err);
+ ssd16xx_send_data(device, border, &err);
+ device->border_waveform_pending = false;
+ }
+
+ switch (device->refresh_mode) {
+ case SSD16XX_REFRESH_FULL:
+ /*
+ * BW full refresh: write RED RAM BEFORE display_update
+ * to avoid a post-BUSY write timing issue on some
+ * controller revisions that silently corrupts RED RAM.
+ * RED RAM is then bypassed (CTRL1_BYPASS_RED_RAM) so
+ * stale RED RAM content does not affect the output.
+ */
+ ssd16xx_send_cmd(device, SSD1683_CMD_WRITE_RAM_RED, &err);
+ if (red_buffer) {
+ /* 3-colour: write red channel before activating */
+ ssd16xx_send_data_bulk(device, red_buffer, data_size, &err);
+ ssd16xx_display_update(device, device->controller_cfg->ctrl1_normal,
+ SSD16XX_CTRL1_BYTE2_DEFAULT,
+ ctrl2_tbl[SSD16XX_REFRESH_FULL], &err);
+ } else {
+ ssd16xx_send_data_bulk(device, mono_buffer, data_size, &err);
+ ssd16xx_display_update(device, device->controller_cfg->ctrl1_bypass_red_ram,
+ SSD16XX_CTRL1_BYTE2_DEFAULT,
+ ctrl2_tbl[SSD16XX_REFRESH_FULL], &err);
+ }
+ break;
+ case SSD16XX_REFRESH_FAST:
+ /*
+ * Fast refresh: LUT pre-loaded during hw_init; BYPASS_RED_RAM
+ * so RED RAM does not affect the current output.
+ * Write RED RAM BEFORE display_update (same reasoning as FULL)
+ * so it holds the just-displayed frame as a valid reference for
+ * any subsequent PARTIAL refresh.
+ */
+
+ ssd16xx_send_cmd(device, SSD1683_CMD_WRITE_RAM_RED, &err);
+ if (red_buffer) {
+ /* 3-colour: write red channel before activating */
+ ssd16xx_send_data_bulk(device, red_buffer, data_size, &err);
+ ssd16xx_display_update(device, device->controller_cfg->ctrl1_normal,
+ SSD16XX_CTRL1_BYTE2_DEFAULT,
+ ctrl2_tbl[SSD16XX_REFRESH_FAST], &err);
+ } else {
+ ssd16xx_send_data_bulk(device, mono_buffer, data_size, &err);
+ ssd16xx_display_update(device, device->controller_cfg->ctrl1_bypass_red_ram,
+ SSD16XX_CTRL1_BYTE2_DEFAULT,
+ ctrl2_tbl[SSD16XX_REFRESH_FAST], &err);
+ }
+ break;
+ case SSD16XX_REFRESH_PARTIAL:
+ default:
+ /*
+ * Partial refresh: both RAMs used for transition waveforms.
+ * RED RAM must hold the PREVIOUS frame (= current display
+ * content) so the controller can compute pixel transitions.
+ * Write RED RAM AFTER display_update so it captures the
+ * just-displayed frame as the reference for the next partial.
+ */
+ drm_dbg(&device->drm,
+ "fb_dirty: partial pre-update: mono[0]=0x%02x (BW=new, RED=prev)\n",
+ mono_buffer[0]);
+ ssd16xx_display_update(device, device->controller_cfg->ctrl1_normal,
+ SSD16XX_CTRL1_BYTE2_DEFAULT,
+ ctrl2_tbl[SSD16XX_REFRESH_PARTIAL], &err);
+ ssd16xx_send_cmd(device, SSD1683_CMD_WRITE_RAM_RED, &err);
+ ssd16xx_send_data_bulk(device, mono_buffer, data_size, &err);
+ drm_dbg(&device->drm,
+ "fb_dirty: partial post-update: wrote RED baseline mono[0]=0x%02x\n",
+ mono_buffer[0]);
+ break;
+ }
+
+ return err;
+}
+
+/* -----------------------------------------------------------------------------
+ * Plane Functions
+ */
+
+static void ssd16xx_plane_destroy(struct drm_plane *plane)
+{
+ drm_plane_cleanup(plane);
+}
+
+static const struct drm_plane_funcs ssd16xx_plane_funcs = {
+ .update_plane = drm_atomic_helper_update_plane,
+ .disable_plane = drm_atomic_helper_disable_plane,
+ .destroy = ssd16xx_plane_destroy,
+ DRM_GEM_SHADOW_PLANE_FUNCS,
+};
+
+static int ssd16xx_plane_atomic_check(struct drm_plane *plane,
+ struct drm_atomic_commit *state)
+{
+ struct drm_plane_state *new_plane_state =
+ drm_atomic_get_new_plane_state(state, plane);
+ struct drm_crtc_state *crtc_state;
+
+ if (!new_plane_state->crtc)
+ return 0;
+
+ crtc_state = drm_atomic_get_new_crtc_state(state, new_plane_state->crtc);
+
+ return drm_atomic_helper_check_plane_state(new_plane_state, crtc_state,
+ DRM_PLANE_NO_SCALING,
+ DRM_PLANE_NO_SCALING,
+ false, false);
+}
+
+static void ssd16xx_plane_atomic_update(struct drm_plane *plane,
+ struct drm_atomic_commit *state)
+{
+ struct drm_plane_state *old_state = drm_atomic_get_old_plane_state(state, plane);
+ struct drm_plane_state *new_state = drm_atomic_get_new_plane_state(state, plane);
+ struct drm_shadow_plane_state *shadow_state = to_drm_shadow_plane_state(new_state);
+ struct ssd16xx_device *device = plane_to_ssd16xx_device(plane);
+ enum ssd16xx_refresh_mode saved_mode;
+ u8 saved_border_waveform_idx;
+ struct drm_framebuffer *fb = new_state->fb;
+ struct drm_rect rect;
+ int ret, idx;
+
+ if (!drm_dev_enter(&device->drm, &idx))
+ return;
+
+ drm_dbg(&device->drm, "plane_atomic_update: fb=%p\n", fb);
+
+ if (!fb || !device->initialized)
+ goto out_exit;
+
+ if (!drm_atomic_helper_damage_merged(old_state, new_state, &rect))
+ goto out_exit;
+
+ drm_dbg(&device->drm, "plane_atomic_update: calling fb_dirty rect=(%d,%d)-(%d,%d)\n",
+ rect.x1, rect.y1, rect.x2, rect.y2);
+ /*
+ * When refresh_mode_init was set, use the specified mode for this first
+ * frame only, then restore the user-configured refresh_mode so
+ * subsequent updates continue with the configured mode.
+ */
+ saved_mode = device->refresh_mode;
+ saved_border_waveform_idx = device->border_waveform_update_idx;
+ if (device->init_refresh_pending) {
+ device->refresh_mode = device->refresh_mode_init;
+ device->border_waveform_update_idx = device->border_waveform_init_idx;
+ }
+
+ /*
+ * Fast refresh (0xC7) omits LOAD_LUT on every update cycle and relies
+ * on the LUT being pre-loaded upfront. The property setter arms
+ * fast_lut_pending whenever the user switches into fast mode. Consume
+ * the flag here (once) before the first fast-refresh frame so the
+ * controller's LUT is in the correct state.
+ */
+ if (device->fast_lut_pending) {
+ ret = ssd16xx_preload_fast_lut(device);
+ if (ret) {
+ drm_err(&device->drm,
+ "plane_atomic_update: fast LUT preload failed: %d\n", ret);
+ }
+
+ device->fast_lut_pending = false;
+ }
+
+ ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
+ if (ret) {
+ drm_err(&device->drm, "plane_atomic_update: begin_cpu_access failed: %d\n", ret);
+ goto out_restore;
+ }
+
+ ret = ssd16xx_fb_dirty(fb, &rect, device, &shadow_state->data[0]);
+ if (ret)
+ drm_err(&device->drm, "plane_atomic_update: display update failed: %d\n", ret);
+
+ drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
+
+out_restore:
+ device->refresh_mode = saved_mode;
+ device->border_waveform_update_idx = saved_border_waveform_idx;
+
+ /*
+ * If this was the init frame (which used border_waveform_init_idx
+ * inside fb_dirty), arm border_waveform_pending so the normal
+ * (non-init) border value is sent at the start of the next update.
+ */
+ if (device->init_refresh_pending) {
+ device->init_refresh_pending = false;
+ device->border_waveform_pending = true;
+ }
+
+out_exit:
+ drm_dev_exit(idx);
+}
+
+static const struct drm_plane_helper_funcs ssd16xx_plane_helper_funcs = {
+ DRM_GEM_SHADOW_PLANE_HELPER_FUNCS,
+ .atomic_check = ssd16xx_plane_atomic_check,
+ .atomic_update = ssd16xx_plane_atomic_update,
+};
+
+/* -----------------------------------------------------------------------------
+ * CRTC Functions
+ */
+
+static const struct drm_crtc_funcs ssd16xx_crtc_funcs = {
+ .atomic_create_state = drm_atomic_helper_crtc_create_state,
+ .destroy = drm_crtc_cleanup,
+ .set_config = drm_atomic_helper_set_config,
+ .page_flip = drm_atomic_helper_page_flip,
+ .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
+ .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
+};
+
+static enum drm_mode_status ssd16xx_crtc_mode_valid(struct drm_crtc *crtc,
+ const struct drm_display_mode *mode)
+{
+ struct ssd16xx_device *device = crtc_to_ssd16xx_device(crtc);
+
+ /* Accept only our device's native mode (landscape or portrait) */
+ if ((mode->hdisplay == device->mode->hdisplay &&
+ mode->vdisplay == device->mode->vdisplay) ||
+ (mode->hdisplay == device->mode->vdisplay &&
+ mode->vdisplay == device->mode->hdisplay))
+ return MODE_OK;
+
+ return MODE_BAD;
+}
+
+static int ssd16xx_crtc_atomic_check(struct drm_crtc *crtc,
+ struct drm_atomic_commit *state)
+{
+ struct drm_crtc_state *new_crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
+
+ if (!new_crtc_state->enable)
+ return 0;
+
+ return drm_atomic_helper_check_crtc_primary_plane(new_crtc_state);
+}
+
+static void ssd16xx_crtc_atomic_enable(struct drm_crtc *crtc,
+ struct drm_atomic_commit *state)
+{
+ struct ssd16xx_device *device = crtc_to_ssd16xx_device(crtc);
+ int ret, idx;
+
+ if (!drm_dev_enter(&device->drm, &idx))
+ return;
+
+ drm_dbg(&device->drm, "atomic_enable: %dx%d orientation=%u°\n",
+ device->width, device->height, device->orientation);
+
+ if (!device->initialized) {
+ ret = ssd16xx_hw_init(device);
+ if (ret) {
+ drm_err(&device->drm, "atomic_enable: hw_init failed: %d\n", ret);
+ drm_dev_exit(idx);
+ return;
+ }
+ device->initialized = true;
+ }
+
+ /*
+ * If refresh_mode_init is set, arm init_refresh_pending so
+ * plane_atomic_update uses the specified mode for the first frame
+ * then restores the user-configured or device default refresh_mode.
+ */
+ if (device->refresh_mode_init >= 0) {
+ drm_dbg(&device->drm,
+ "atomic_enable: refresh_mode_init=%d, using for first frame\n",
+ device->refresh_mode_init);
+ device->init_refresh_pending = true;
+ }
+
+ drm_dev_exit(idx);
+}
+
+static const struct drm_crtc_helper_funcs ssd16xx_crtc_helper_funcs = {
+ .mode_valid = ssd16xx_crtc_mode_valid,
+ .atomic_check = ssd16xx_crtc_atomic_check,
+ .atomic_enable = ssd16xx_crtc_atomic_enable,
+};
+
+/* -----------------------------------------------------------------------------
+ * Connector Functions
+ */
+
+static int ssd16xx_connector_get_modes(struct drm_connector *connector)
+{
+ struct ssd16xx_device *device = to_ssd16xx_device(connector->dev);
+ bool mode_is_portrait = (device->mode->hdisplay < device->mode->vdisplay);
+ bool orient_is_portrait = (device->orientation == 90 || device->orientation == 270);
+
+ drm_dbg(&device->drm,
+ "connector_get_modes: orientation=%u° mode=%ux%u mode_portrait=%d orient_portrait=%d\n",
+ device->orientation, device->mode->hdisplay, device->mode->vdisplay,
+ mode_is_portrait, orient_is_portrait);
+
+ /* For portrait, swap dimensions so clients see logical size. */
+ if (mode_is_portrait != orient_is_portrait) {
+ struct drm_display_mode *mode;
+
+ mode = drm_mode_duplicate(&device->drm, device->mode);
+ if (!mode)
+ return 0;
+ swap(mode->hdisplay, mode->vdisplay);
+ swap(mode->hsync_start, mode->vsync_start);
+ swap(mode->hsync_end, mode->vsync_end);
+ swap(mode->htotal, mode->vtotal);
+ swap(mode->width_mm, mode->height_mm);
+ mode->type |= DRM_MODE_TYPE_PREFERRED;
+ drm_mode_set_name(mode);
+ drm_mode_probed_add(connector, mode);
+ return 1;
+ }
+
+ return drm_connector_helper_get_modes_fixed(connector, device->mode);
+}
+
+static const struct drm_connector_helper_funcs ssd16xx_connector_helper_funcs = {
+ .get_modes = ssd16xx_connector_get_modes,
+};
+
+static const struct drm_connector_funcs ssd16xx_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 u32 ssd16xx_formats[] = {
+ DRM_FORMAT_XRGB8888, /* 32-bit RGB with padding (preferred) */
+ DRM_FORMAT_R1, /* 1-bit monochrome (native, zero-copy path) */
+};
+
+DEFINE_DRM_GEM_FOPS(ssd16xx_fops);
+
+/*
+ * ssd16xx_drm_master_set - arm init refresh when a new master takes control.
+ */
+static void ssd16xx_drm_master_set(struct drm_device *drm,
+ struct drm_file *file, bool from_open)
+{
+ struct ssd16xx_device *device = to_ssd16xx_device(drm);
+
+ if (device->refresh_mode_init >= 0)
+ device->init_refresh_pending = true;
+}
+
+/*
+ * ssd16xx_drm_master_drop - clear display and disarm init refresh when the
+ * master client exits.
+ */
+static void ssd16xx_drm_master_drop(struct drm_device *drm,
+ struct drm_file *file)
+{
+ struct ssd16xx_device *device = to_ssd16xx_device(drm);
+
+ device->init_refresh_pending = false;
+}
+
+static struct drm_driver ssd16xx_drm_driver = {
+ .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
+ .fops = &ssd16xx_fops,
+ .name = "ssd16xx",
+ .desc = "DRM driver for SSD16xx e-paper controller family",
+ .major = 1,
+ .minor = 0,
+ .master_set = ssd16xx_drm_master_set,
+ .master_drop = ssd16xx_drm_master_drop,
+ DRM_GEM_SHMEM_DRIVER_OPS,
+ DRM_FBDEV_SHMEM_DRIVER_OPS,
+};
+
+static const struct drm_mode_config_funcs ssd16xx_mode_config_funcs = {
+ .fb_create = drm_gem_fb_create_with_dirty,
+ .atomic_check = drm_atomic_helper_check,
+ .atomic_commit = drm_atomic_helper_commit,
+};
+
+/*
+ * Use the RPM commit-tail variant so that drm_atomic_helper_commit_modeset_enables
+ * (which calls crtc_atomic_enable) runs before drm_atomic_helper_commit_planes.
+ * Without this, the standard commit_tail calls commit_planes before
+ * modeset_enables, so plane_atomic_update would see initialized == false on the
+ * first commit and silently drop the frame.
+ */
+static const struct drm_mode_config_helper_funcs ssd16xx_mode_config_helper_funcs = {
+ .atomic_commit_tail = drm_atomic_helper_commit_tail_rpm,
+};
+
+static int ssd16xx_alloc_tx_bufs(struct ssd16xx_device *device)
+{
+ /*
+ * Allocate for the maximum buffer needed across orientations: each row
+ * is padded to a byte boundary (DIV_ROUND_UP(dimension, 8)), so
+ * take the maximum of row-major landscape and column-major portrait.
+ */
+ size_t frame_size = max(DIV_ROUND_UP(device->width, 8) * device->height,
+ DIV_ROUND_UP(device->height, 8) * device->width);
+
+ device->tx_buf = drmm_kmalloc(&device->drm, frame_size, GFP_KERNEL);
+ if (!device->tx_buf)
+ return -ENOMEM;
+
+ if (device->device_cfg->red_supported) {
+ device->tx_red_buf = drmm_kmalloc(&device->drm, frame_size, GFP_KERNEL);
+ if (!device->tx_red_buf)
+ return -ENOMEM;
+ }
+
+ if (!device->dc) {
+ device->tx_buf9 = drmm_kmalloc_array(&device->drm, frame_size,
+ sizeof(u16), GFP_KERNEL);
+ if (!device->tx_buf9)
+ return -ENOMEM;
+ }
+
+ return 0;
+}
+
+static int ssd16xx_probe(struct spi_device *spi)
+{
+ struct device *dev = &spi->dev;
+ struct ssd16xx_device *device;
+ struct drm_device *drm;
+ const struct spi_device_id *spi_id;
+ struct drm_display_mode *mode;
+ const void *match;
+ enum ssd16xx_model model;
+ u32 dt_rotation = 0;
+ int ret;
+
+ match = device_get_match_data(dev);
+ if (match) {
+ model = (enum ssd16xx_model)(uintptr_t)match;
+ } else {
+ spi_id = spi_get_device_id(spi);
+ model = (enum ssd16xx_model)spi_id->driver_data;
+ }
+
+ device = devm_drm_dev_alloc(dev, &ssd16xx_drm_driver,
+ struct ssd16xx_device, drm);
+ if (IS_ERR(device))
+ return PTR_ERR(device);
+
+ drm = &device->drm;
+ device->spi = spi;
+ device->model = model;
+ spi_set_drvdata(spi, device);
+
+ spi->mode = SPI_MODE_0;
+ spi->bits_per_word = SSD16XX_SPI_BITS_PER_WORD;
+
+ if (!spi->max_speed_hz) {
+ drm_warn(drm, "spi-max-frequency not specified, using %u Hz\n",
+ SSD16XX_SPI_SPEED_DEFAULT);
+ spi->max_speed_hz = SSD16XX_SPI_SPEED_DEFAULT;
+ }
+
+ ret = spi_setup(spi);
+ if (ret < 0) {
+ drm_err(drm, "SPI setup failed: %d\n", ret);
+ return ret;
+ }
+
+ switch (model) {
+ case GDEY042T81:
+ device->controller = SSD1683;
+ break;
+ default:
+ drm_err(drm, "Unknown panel model: %d\n", model);
+ return -EINVAL;
+ }
+
+ if (device->controller >= ARRAY_SIZE(ssd16xx_controller_configs) ||
+ !ssd16xx_controller_configs[device->controller].max_width)
+ return -EINVAL;
+ device->controller_cfg = &ssd16xx_controller_configs[device->controller];
+
+ if (model >= ARRAY_SIZE(ssd16xx_device_configs))
+ return -EINVAL;
+ device->device_cfg = &ssd16xx_device_configs[model];
+
+ mode = devm_kmemdup(dev, device->device_cfg->mode,
+ sizeof(*device->device_cfg->mode), GFP_KERNEL);
+ if (!mode)
+ return -ENOMEM;
+
+ device->refresh_mode = device->device_cfg->default_refresh_mode;
+ device->color_mode = device->device_cfg->default_color_mode;
+ device->border_waveform_init_idx = device->device_cfg->default_border_waveform_init;
+ device->border_waveform_update_idx = device->device_cfg->default_border_waveform_update;
+ device->border_refresh_on_every_update =
+ device->device_cfg->default_border_refresh_on_every_update;
+ device->refresh_mode_init = device->device_cfg->default_refresh_mode_init;
+
+ /* Parse "rotation" DT property; swap mode dimensions for portrait. */
+ device_property_read_u32(dev, "rotation", &dt_rotation);
+ if (dt_rotation != 0 && dt_rotation != 90 && dt_rotation != 180 && dt_rotation != 270) {
+ drm_warn(drm, "Invalid DT rotation %u, defaulting to 0°\n", dt_rotation);
+ dt_rotation = 0;
+ }
+ device->orientation = dt_rotation;
+
+ device->width = mode->hdisplay;
+ device->height = mode->vdisplay;
+
+ drm_dbg(drm, "Using %s orientation (%u°, physical %ux%u)\n",
+ (device->orientation == 90 || device->orientation == 270) ?
+ "portrait" : "landscape", device->orientation, device->width, device->height);
+
+ /* Swap mode dimensions for portrait so clients see logical size. */
+ if (device->orientation == 90 || device->orientation == 270) {
+ swap(mode->hdisplay, mode->vdisplay);
+ swap(mode->hsync_start, mode->vsync_start);
+ swap(mode->hsync_end, mode->vsync_end);
+ swap(mode->htotal, mode->vtotal);
+ swap(mode->width_mm, mode->height_mm);
+ drm_dbg(drm, "Mode dimensions swapped for portrait: %ux%u\n",
+ mode->hdisplay, mode->vdisplay);
+ } else {
+ drm_dbg(drm, "Mode dimensions unchanged: %ux%u\n",
+ mode->hdisplay, mode->vdisplay);
+ }
+ device->mode = mode;
+
+ /* Validate panel dimensions against controller hardware limits. */
+ if (device->width > device->controller_cfg->max_width ||
+ device->height > device->controller_cfg->max_height) {
+ drm_err(drm, "panel %ux%u exceeds controller max %ux%u\n",
+ device->width, device->height,
+ device->controller_cfg->max_width,
+ device->controller_cfg->max_height);
+ return -EINVAL;
+ }
+
+ /*
+ * For byte-addressed X (SSD1683, ram_x_address_bits == 8), each X
+ * address covers 8 pixels. The panel width must be a multiple of 8
+ * so that (width/8 - 1) gives the correct byte-aligned window end.
+ */
+ if (device->controller_cfg->ram_x_address_bits == 8 &&
+ device->width % 8 != 0) {
+ drm_err(drm, "panel width %u not a multiple of 8 required for byte-addressed X controller)\n",
+ device->width);
+ return -EINVAL;
+ }
+
+ /* Acquire GPIOs. */
+ device->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+ if (IS_ERR(device->reset))
+ return dev_err_probe(dev, PTR_ERR(device->reset), "Failed to get RESET GPIO\n");
+
+ device->busy = devm_gpiod_get(dev, "busy", GPIOD_IN);
+ if (IS_ERR(device->busy))
+ return dev_err_probe(dev, PTR_ERR(device->busy), "Failed to get BUSY GPIO\n");
+
+ device->dc = devm_gpiod_get_optional(dev, "dc", GPIOD_OUT_LOW);
+ if (IS_ERR(device->dc))
+ return dev_err_probe(dev, PTR_ERR(device->dc), "Failed to get DC GPIO\n");
+ if (!device->dc) {
+ if (!spi_is_bpw_supported(spi, 9))
+ return dev_err_probe(dev, -EINVAL,
+ "3-wire SPI mode requires 9-bit word support\n");
+ drm_dbg(drm, "dc-gpios not specified, using 3-wire (9-bit) SPI mode\n");
+ }
+
+ ret = ssd16xx_alloc_tx_bufs(device);
+ if (ret)
+ return ret;
+
+ ssd16xx_hw_reset(device);
+
+ ret = drmm_mode_config_init(drm);
+ if (ret)
+ return ret;
+
+ drm->mode_config.funcs = &ssd16xx_mode_config_funcs;
+ drm->mode_config.helper_private = &ssd16xx_mode_config_helper_funcs;
+ drm->mode_config.min_width = min(device->width, device->height);
+ drm->mode_config.max_width = max(device->width, device->height);
+ drm->mode_config.min_height = min(device->width, device->height);
+ drm->mode_config.max_height = max(device->width, device->height);
+
+ drm_connector_helper_add(&device->connector, &ssd16xx_connector_helper_funcs);
+ ret = drm_connector_init(drm, &device->connector, &ssd16xx_connector_funcs,
+ DRM_MODE_CONNECTOR_SPI);
+ if (ret)
+ return ret;
+
+ ret = drm_universal_plane_init(drm, &device->primary_plane, 0,
+ &ssd16xx_plane_funcs,
+ ssd16xx_formats, ARRAY_SIZE(ssd16xx_formats),
+ NULL, DRM_PLANE_TYPE_PRIMARY, NULL);
+ if (ret)
+ return ret;
+ drm_plane_helper_add(&device->primary_plane, &ssd16xx_plane_helper_funcs);
+ drm_plane_enable_fb_damage_clips(&device->primary_plane);
+
+ ret = drm_crtc_init_with_planes(drm, &device->crtc, &device->primary_plane,
+ NULL, &ssd16xx_crtc_funcs, NULL);
+ if (ret)
+ return ret;
+ drm_crtc_helper_add(&device->crtc, &ssd16xx_crtc_helper_funcs);
+
+ ret = drmm_encoder_init(drm, &device->encoder, NULL, DRM_MODE_ENCODER_NONE, NULL);
+ if (ret)
+ return ret;
+ device->encoder.possible_crtcs = drm_crtc_mask(&device->crtc);
+
+ ret = drm_connector_attach_encoder(&device->connector, &device->encoder);
+ if (ret)
+ return ret;
+
+ drm_mode_config_reset(drm);
+
+ ret = drm_dev_register(drm, 0);
+ if (ret)
+ return ret;
+
+ drm_dbg(drm, "SSD16xx e-paper display initialized (%dx%d, %d° rotation)\n",
+ device->width, device->height, device->orientation);
+
+ drm_client_setup(drm, NULL);
+
+ return 0;
+}
+
+static void ssd16xx_remove(struct spi_device *spi)
+{
+ struct ssd16xx_device *device = spi_get_drvdata(spi);
+
+ drm_dev_unplug(&device->drm);
+ drm_atomic_helper_shutdown(&device->drm);
+}
+
+static void ssd16xx_shutdown(struct spi_device *spi)
+{
+ struct ssd16xx_device *device = spi_get_drvdata(spi);
+
+ drm_atomic_helper_shutdown(&device->drm);
+}
+
+static const struct of_device_id ssd16xx_of_match[] = {
+ { .compatible = "gooddisplay,gdey042t81", .data = (void *)GDEY042T81 },
+ { }
+};
+MODULE_DEVICE_TABLE(of, ssd16xx_of_match);
+
+static const struct spi_device_id ssd16xx_id[] = {
+ { "gdey042t81", GDEY042T81 },
+ { }
+};
+MODULE_DEVICE_TABLE(spi, ssd16xx_id);
+
+static struct spi_driver ssd16xx_spi_driver = {
+ .driver = {
+ .name = "ssd16xx",
+ .of_match_table = ssd16xx_of_match,
+ },
+ .probe = ssd16xx_probe,
+ .remove = ssd16xx_remove,
+ .shutdown = ssd16xx_shutdown,
+ .id_table = ssd16xx_id,
+};
+module_spi_driver(ssd16xx_spi_driver);
+
+MODULE_AUTHOR("Devarsh Thakkar <devarsht@ti.com>");
+MODULE_DESCRIPTION("DRM driver for Solomon SSD16xx e-paper display controller family");
+MODULE_LICENSE("GPL");
--
2.39.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v3 04/14] drm/solomon: Add DRM driver for Solomon SSD16xx e-paper display controllers
2026-10-19 21:37 ` [PATCH v3 04/14] drm/solomon: Add DRM driver for Solomon SSD16xx e-paper display controllers Devarsh Thakkar
@ 2026-10-19 22:02 ` sashiko-bot
0 siblings, 0 replies; 25+ messages in thread
From: sashiko-bot @ 2026-10-19 22:02 UTC (permalink / raw)
To: Devarsh Thakkar; +Cc: dri-devel, devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [Critical] drm/solomon: heap buffer overflow of tx_buf in ssd16xx_fb_dirty()
- [Medium] drm/solomon: bit-reversed pixels in ssd16xx_convert_fb_to_1bpp()
- [Medium] drm/solomon: ssd16xx_convert_fb_to_3color() ignores orientation
- [Medium] drm/solomon: init_refresh_pending cleared on error in atomic_update
- [Medium] drm/solomon: ssd16xx: use-after-free of devm-allocated mode after unbind
- [Low] drm/solomon: broken portrait check in ssd16xx_connector_get_modes()
--
commit 2fb6265a2051abb85583fcfb0ff873962f7bd4d7
Author: Devarsh Thakkar <devarsht@ti.com>
drm/solomon: Add DRM driver for Solomon SSD16xx e-paper display controllers
This commit adds a DRM driver for the Solomon Systech SSD16xx family of
e-paper display controllers over SPI, starting with support for the
Gooddisplay GDEY042T81 400x300 black/white panel using the SSD1683
controller.
[1] : https://www.crystalfontz.com/controllers/SolomonSystech/SSD1683
[2] : https://files.seeedstudio.com/wiki/Other_Display/42-epaper/GDEY042T81.pdf
[3] : https://www.crystalfontz.com/controllers/SolomonSystech/SSD1673
[4] : https://www.crystalfontz.com/controllers/SolomonSystech/SSD1680
[5] : https://www.crystalfontz.com/controllers/SolomonSystech/SSD1681
> diff --git a/drivers/gpu/drm/solomon/ssd16xx.c b/drivers/gpu/drm/solomon/ssd16xx.c
> new file mode 100644
> index 0000000000000..ca6550b5dd5bd
> --- /dev/null
> +++ b/drivers/gpu/drm/solomon/ssd16xx.c
> @@ -0,0 +1,1801 @@
[ ... ]
> +static int ssd16xx_hw_init(struct ssd16xx_device *device)
> +{
[ ... ]
> + switch (device->orientation) {
> + case 90:
> + case 180:
> + data_entry_mode = SSD16XX_DATA_ENTRY_XDEC_YDEC;
> + break;
> + default: /* 0°/270° */
> + data_entry_mode = SSD16XX_DATA_ENTRY_XINC_YINC;
> + break;
> + }
[ ... ]
> +static void ssd16xx_convert_fb_to_3color(u8 *bw_dst, u8 *red_dst,
> + struct iosys_map *src,
> + struct drm_framebuffer *fb,
> + struct drm_rect *rect)
> +{
> + unsigned int x, y;
> + u8 bw_byte = 0, red_byte = 0;
> + unsigned int bit_pos = 0;
> + unsigned int dst_idx = 0;
> +
> + if (fb->format->format == DRM_FORMAT_R1) {
> + unsigned int src_pitch = fb->pitches[0];
> + unsigned int width_bytes = drm_rect_width(rect) / 8;
> + unsigned int data_size = width_bytes * drm_rect_height(rect);
> +
> + /* White background — no black pixels, only red ink shows */
> + memset(bw_dst, 0xFF, data_size);
> + /* RED RAM: copy R1 data directly (1=red ink, 0=no red) */
> + for (y = rect->y1; y < rect->y2; y++) {
> + u8 *line = src->vaddr + y * src_pitch + (rect->x1 / 8);
> +
> + memcpy(red_dst + dst_idx, line, width_bytes);
> + dst_idx += width_bytes;
> + }
> + return;
> + }
> +
> + /* XRGB8888 */
> + for (y = rect->y1; y < rect->y2; y++) {
> + for (x = rect->x1; x < rect->x2; x++) {
> + bool is_red = ssd16xx_pixel_is_red(src, fb, x, y);
> +
> + if (is_red)
> + red_byte |= (1 << (7 - bit_pos));
> + else if (ssd16xx_pixel_is_white(src, fb, x, y))
> + bw_byte |= (1 << (7 - bit_pos));
> + if (++bit_pos == 8) {
> + bw_dst[dst_idx] = bw_byte;
> + red_dst[dst_idx] = red_byte;
> + dst_idx++;
> + bw_byte = 0;
> + red_byte = 0;
> + bit_pos = 0;
> + }
> + }
> + if (bit_pos > 0) {
> + bw_dst[dst_idx] = bw_byte;
> + red_dst[dst_idx] = red_byte;
> + dst_idx++;
> + bw_byte = 0;
> + red_byte = 0;
> + bit_pos = 0;
> + }
> + }
> +}
[Severity: Medium]
Should ssd16xx_convert_fb_to_3color() also handle device->orientation like
ssd16xx_convert_fb_to_1bpp() does?
When ssd16xx_fb_dirty() runs with device->color_mode ==
SSD16XX_COLOR_MODE_3COLOR and FULL or FAST refresh, it calls
ssd16xx_convert_fb_to_3color() without passing device->orientation.
Because ssd16xx_convert_fb_to_3color() always packs in 0-degree landscape
row-major order, 90, 180, and 270 degree rotations are ignored. In portrait
orientations (90 or 270 degrees) where fb->width is 300 and fb->height is
400, the XRGB8888 path packs 38 bytes per row across 400 rows (15,200 bytes
in row-major order instead of 300 column-major rows of 50 bytes, while
ssd16xx_fb_dirty() only sends data_size = 15,000 bytes), and the
DRM_FORMAT_R1 path computes width_bytes = 300 / 8 = 37 bytes per row
(14,800 bytes), truncating 4 pixels per row without transposing for
portrait orientation.
> +
> +/*
> + * Convert framebuffer to 1-bit monochrome for e-paper display.
> + *
> + * Supports XRGB8888 (thresholded via ITU-R BT.601 luma at 127) and R1
> + * (native 1bpp, zero-copy fast path for aligned landscape frames).
> + *
> + * Output layout:
> + * 0°/180° landscape: row-major, left-to-right, top-to-bottom
> + * 90°/270° CW portrait: column-major, rightmost column first
> + */
> +static void ssd16xx_convert_fb_to_1bpp(u8 *dst, struct iosys_map *src,
> + struct drm_framebuffer *fb,
> + struct drm_rect *rect,
> + unsigned int orientation)
> +{
[ ... ]
> + switch (orientation) {
> + case 90:
> + case 270:
> + /*
> + * Portrait (90° or 270°): column-major packing.
> + * Each portrait source column becomes one physical RAM row.
> + * The data entry mode and cursor position control scan direction.
> + */
> + for (x = rect->x2 - 1; x >= (int)rect->x1; x--) {
> + for (y = rect->y1; y < rect->y2; y++) {
> + if (ssd16xx_pixel_is_white(src, fb, x, y))
> + byte |= (1 << (7 - bit_pos));
> + if (++bit_pos == 8) {
> + dst[dst_idx++] = byte;
> + byte = 0;
> + bit_pos = 0;
> + }
> + }
> + if (bit_pos > 0) {
> + dst[dst_idx++] = byte;
> + byte = 0;
> + bit_pos = 0;
> + }
> + }
> + break;
> +
> + case 0:
> + case 180:
> + default:
> + /*
> + * Landscape (0° or 180°): row-major packing.
> + * Each landscape source row becomes one physical RAM row.
> + * The data entry mode and cursor position control scan direction.
> + */
> + for (y = rect->y1; y < rect->y2; y++) {
> + for (x = rect->x1; x < rect->x2; x++) {
> + if (ssd16xx_pixel_is_white(src, fb, x, y))
> + byte |= (1 << (7 - bit_pos));
> + if (++bit_pos == 8) {
> + dst[dst_idx++] = byte;
> + byte = 0;
> + bit_pos = 0;
> + }
> + }
> + if (bit_pos > 0) {
> + dst[dst_idx++] = byte;
> + byte = 0;
> + bit_pos = 0;
> + }
> + }
> + break;
> + }
> +}
[Severity: Medium]
Are pixels within each byte bit-reversed when orientation is 90 or 180
degrees?
When device->orientation is 90 or 180, ssd16xx_hw_init() configures Data
Entry Mode (command 0x11) to SSD16XX_DATA_ENTRY_XDEC_YDEC (0x00), and
ssd16xx_fb_dirty() sets the RAM X address counter to ram_x_end (49) so that
it decrements down to 0 across each scanline.
In SSD16xx byte-addressed RAM, each byte represents 8 horizontal pixels
along the physical X axis where bit 7 (MSB) drives source line 8 * X_addr +
0 and bit 0 (LSB) drives source line 8 * X_addr + 7. Decrementing the X
byte counter reverses the byte order across a scanline (49, 48, ..., 0),
but does not reverse the bit order within each byte.
Because ssd16xx_convert_fb_to_1bpp() uses the same MSB-first packing
expression:
byte |= (1 << (7 - bit_pos));
for 90 and 180 degrees as it does for 270 and 0 degrees, pixel 0 is placed
at physical pixel 392 and pixel 7 at physical pixel 399 in X_addr = 49,
followed by pixel 8 at physical pixel 384 and pixel 15 at physical pixel
391 in X_addr = 48, mirroring each 8-pixel group horizontally.
> +
> +static int ssd16xx_fb_dirty(struct drm_framebuffer *fb, struct drm_rect *rect,
> + struct ssd16xx_device *device,
> + const struct iosys_map *src_map)
> +{
> + const u8 *ctrl2_tbl = device->controller_cfg->ctrl2_refresh;
> + struct iosys_map map = *src_map;
> + int err = 0;
> + unsigned int data_size = (device->width * device->height) / 8;
> + u8 *mono_buffer = NULL;
> + u8 *red_buffer = NULL;
> + u16 ram_x_start, ram_x_end, ram_y_start, ram_y_end;
> +
> + /*
> + * Process full display area. The rect for convert_fb_to_1bpp uses
> + * the framebuffer (logical) dimensions - the pixel iteration walks
> + * the source fb coordinate space. RAM window registers below use
> + * the physical panel dimensions (device->width/height).
> + */
> + rect->x1 = 0;
> + rect->y1 = 0;
> + rect->x2 = fb->width;
> + rect->y2 = fb->height;
[Severity: Critical]
Can setting rect->x2 = fb->width and rect->y2 = fb->height here overflow
device->tx_buf and device->tx_red_buf?
In ssd16xx_probe(), drm->mode_config is configured with:
drm->mode_config.min_width = min(device->width, device->height);
drm->mode_config.max_width = max(device->width, device->height);
drm->mode_config.min_height = min(device->width, device->height);
drm->mode_config.max_height = max(device->width, device->height);
This allows userspace via DRM_IOCTL_MODE_ADDFB2 to create a 400x400
framebuffer and attach it to the primary plane with a 400x300 or 300x400
source rectangle matching the CRTC mode, which passes both
drm_atomic_plane_check() and ssd16xx_plane_atomic_check().
However, ssd16xx_alloc_tx_bufs() allocates device->tx_buf and
device->tx_red_buf with:
size_t frame_size = max(DIV_ROUND_UP(device->width, 8) * device->height,
DIV_ROUND_UP(device->height, 8) * device->width);
which is 15,200 bytes for a 400x300 panel. Because ssd16xx_fb_dirty()
overwrites rect with fb->width and fb->height (400x400) rather than the
active mode/clipped plane dimensions, ssd16xx_convert_fb_to_1bpp() and
ssd16xx_convert_fb_to_3color() will write (400 / 8) * 400 = 20,000 bytes
into the 15,200-byte heap buffer.
In addition, ssd16xx_crtc_mode_valid() accepts both 400x300 and 300x400
modes regardless of the static device->orientation configured in
ssd16xx_probe():
if ((mode->hdisplay == device->mode->hdisplay &&
mode->vdisplay == device->mode->vdisplay) ||
(mode->hdisplay == device->mode->vdisplay &&
mode->vdisplay == device->mode->hdisplay))
return MODE_OK;
If userspace commits a 300x400 mode when device->orientation is 0 or 180
(or 400x300 when orientation is 90 or 270), ssd16xx_convert_fb_to_1bpp()
packs 38-byte padded rows across 400 rows (15,200 bytes, or 37-byte rows
totaling 14,800 bytes on the DRM_FORMAT_R1 fast path), while
ssd16xx_fb_dirty() only zeroes and transmits data_size = 15,000 bytes to a
hardware RAM window programmed for 50-byte rows.
[ ... ]
> +static void ssd16xx_plane_atomic_update(struct drm_plane *plane,
> + struct drm_atomic_commit *state)
> +{
[ ... ]
> + ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
> + if (ret) {
> + drm_err(&device->drm, "plane_atomic_update: begin_cpu_access failed: %d\n", ret);
> + goto out_restore;
> + }
> +
> + ret = ssd16xx_fb_dirty(fb, &rect, device, &shadow_state->data[0]);
> + if (ret)
> + drm_err(&device->drm, "plane_atomic_update: display update failed: %d\n", ret);
> +
> + drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
> +
> +out_restore:
> + device->refresh_mode = saved_mode;
> + device->border_waveform_update_idx = saved_border_waveform_idx;
> +
> + /*
> + * If this was the init frame (which used border_waveform_init_idx
> + * inside fb_dirty), arm border_waveform_pending so the normal
> + * (non-init) border value is sent at the start of the next update.
> + */
> + if (device->init_refresh_pending) {
> + device->init_refresh_pending = false;
> + device->border_waveform_pending = true;
> + }
[Severity: Medium]
If drm_gem_fb_begin_cpu_access() or ssd16xx_fb_dirty() fails during the
initial frame, should device->init_refresh_pending remain set instead of
being cleared at out_restore?
Clearing device->init_refresh_pending on the error path causes subsequent
frames to skip the initial full refresh and perform partial refreshes
without a valid RED RAM baseline.
[ ... ]
> +static int ssd16xx_connector_get_modes(struct drm_connector *connector)
> +{
> + struct ssd16xx_device *device = to_ssd16xx_device(connector->dev);
> + bool mode_is_portrait = (device->mode->hdisplay < device->mode->vdisplay);
> + bool orient_is_portrait = (device->orientation == 90 || device->orientation == 270);
> +
> + drm_dbg(&device->drm,
> + "connector_get_modes: orientation=%u° mode=%ux%u mode_portrait=%d orient_portrait=%d\n",
> + device->orientation, device->mode->hdisplay, device->mode->vdisplay,
> + mode_is_portrait, orient_is_portrait);
> +
> + /* For portrait, swap dimensions so clients see logical size. */
> + if (mode_is_portrait != orient_is_portrait) {
> + struct drm_display_mode *mode;
> +
> + mode = drm_mode_duplicate(&device->drm, device->mode);
> + if (!mode)
> + return 0;
> + swap(mode->hdisplay, mode->vdisplay);
> + swap(mode->hsync_start, mode->vsync_start);
> + swap(mode->hsync_end, mode->vsync_end);
> + swap(mode->htotal, mode->vtotal);
> + swap(mode->width_mm, mode->height_mm);
> + mode->type |= DRM_MODE_TYPE_PREFERRED;
> + drm_mode_set_name(mode);
> + drm_mode_probed_add(connector, mode);
> + return 1;
> + }
> +
> + return drm_connector_helper_get_modes_fixed(connector, device->mode);
> +}
[Severity: Low]
Since ssd16xx_probe() already swaps device->mode in-place when
device->orientation is 90 or 270, is the swap check in
ssd16xx_connector_get_modes() redundant?
For natively landscape panels like GDEY042T81, device->mode->hdisplay <
device->mode->vdisplay is already true when orient_is_portrait is true, so
mode_is_portrait != orient_is_portrait is never taken. Conversely, for any
natively portrait panel, mode_is_portrait != orient_is_portrait would be
true in all orientations and undo the swap performed in ssd16xx_probe()
(while also skipping the connector->display_info.width_mm and height_mm
assignments done by drm_connector_helper_get_modes_fixed()).
[ ... ]
> +static int ssd16xx_probe(struct spi_device *spi)
> +{
[ ... ]
> + mode = devm_kmemdup(dev, device->device_cfg->mode,
> + sizeof(*device->device_cfg->mode), GFP_KERNEL);
> + if (!mode)
> + return -ENOMEM;
[Severity: Medium]
Can allocating device->mode with devm_kmemdup() lead to a use-after-free
after the SPI device is unbound?
While struct ssd16xx_device is allocated via devm_drm_dev_alloc() and stays
alive until the last open DRM file descriptor is closed, devm_kmemdup()
ties device->mode to the SPI struct device devres list, which is freed as
soon as ssd16xx_remove() returns.
If an in-flight KMS ioctl (such as DRM_IOCTL_MODE_GETCONNECTOR or
DRM_IOCTL_MODE_SETCRTC) passes the initial unplug check in
drm_ioctl_kernel() and waits on modeset locks while ssd16xx_remove() runs,
it can subsequently invoke ssd16xx_connector_get_modes() or
ssd16xx_crtc_mode_valid() (neither of which is guarded by drm_dev_enter())
and dereference device->mode after devres_release_all() has freed it.
Would embedding struct drm_display_mode directly in struct ssd16xx_device
(or using drmm_kmemdup()) avoid this lifetime mismatch?
> +
> + device->refresh_mode = device->device_cfg->default_refresh_mode;
[ ... ]
> + /* Swap mode dimensions for portrait so clients see logical size. */
> + if (device->orientation == 90 || device->orientation == 270) {
> + swap(mode->hdisplay, mode->vdisplay);
> + swap(mode->hsync_start, mode->vsync_start);
> + swap(mode->hsync_end, mode->vsync_end);
> + swap(mode->htotal, mode->vtotal);
> + swap(mode->width_mm, mode->height_mm);
> + drm_dbg(drm, "Mode dimensions swapped for portrait: %ux%u\n",
> + mode->hdisplay, mode->vdisplay);
> + } else {
> + drm_dbg(drm, "Mode dimensions unchanged: %ux%u\n",
> + mode->hdisplay, mode->vdisplay);
> + }
> + device->mode = mode;
[ ... ]
> +static void ssd16xx_remove(struct spi_device *spi)
> +{
> + struct ssd16xx_device *device = spi_get_drvdata(spi);
> +
> + drm_dev_unplug(&device->drm);
> + drm_atomic_helper_shutdown(&device->drm);
> +}
[ ... ]
> +
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261019213726.4145532-1-devarsht@ti.com?part=4
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v3 05/14] drm/solomon: ssd16xx: Add clear_on_init/close/disable session management
2026-10-19 21:37 [PATCH v3 00/14] Add DRM driver for Solomon SSD16xx e-paper display controllers Devarsh Thakkar
` (3 preceding siblings ...)
2026-10-19 21:37 ` [PATCH v3 04/14] drm/solomon: Add DRM driver for Solomon SSD16xx e-paper display controllers Devarsh Thakkar
@ 2026-10-19 21:37 ` Devarsh Thakkar
2026-10-19 21:59 ` sashiko-bot
2026-10-19 21:37 ` [PATCH v3 06/14] drm/solomon: ssd16xx: Add support for Solomon SSD1677 controller Devarsh Thakkar
` (8 subsequent siblings)
13 siblings, 1 reply; 25+ messages in thread
From: Devarsh Thakkar @ 2026-10-19 21:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Nishanth Menon, Vignesh Raghavendra, Tero Kristo
Cc: dri-devel, devicetree, linux-kernel, linux-arm-kernel, devarsht,
LiangCheng Wang
E-paper displays are bistable: the last rendered image persists
indefinitely across sessions and power cycles. This creates a session
management choice with no LCD/OLED equivalent since unlike volatile
displays, content visibility outlives the rendering process. Whether this
persistence is a feature or a liability depends entirely on the
application.
Add session-lifetime display clearing with three configurable hooks:
clear_on_init: blank the display when a new DRM master opens the device.
Useful for kiosks and applications that need a clean slate on startup.
Example: a hospital patient room sign must start with a clean slate when a
new patient is admitted, discarding the previous patient's name, dietary
restrictions, and risk warnings.
clear_on_close: blank the display when the DRM master exits (fd close).
Useful for applications that should not leave content visible after exit.
Example: a transit departure board at a remote bus stop must erase stale
schedule data when the service daemon is stopped — displaying yesterday's
departure times actively misleads commuters.
clear_on_disable: blank the display on CRTC disable / DPMS off. Provides
an independent clear point separate from master lifecycle. Example: When
userspace trigger DPMS off, similar to LCD screen, here with the epaper
too, the display would be cleared.
These hooks help enabling diverse use cases:
- A conference badge sets clear_on_init but leaves clear_on_close
disabled, leveraging e-paper's bistability to keep the attendee's name
visible indefinitely after the rendering app exits — saving power while
remaining useful.
- A hospital patient room sign sets both clear_on_init and
clear_on_close: a clean slate for the new patient and automatic erasure
on discharge so protected health information does not persist on the
display outside the door.
- An airport boarding pass kiosk sets clear_on_close so the previous
passenger's name, booking reference, and seat assignment are erased the
moment the session ends — preventing the next person in the queue from
seeing personal travel information.
- A meeting room booking display sets clear_on_disable so the schedule is
wiped when the room session ends via DPMS off, independent of whether
the calendar sync daemon continues running.
Each is an integer selecting the refresh waveform for the clear (-1 =
disabled, 0 = partial, 1 = full, 2 = fast), defaulting to -1 (disabled) in
the panel configuration so the behaviour is opt-in. A
display_cleared_on_deinit guard prevents redundant double-clears when
multiple paths fire for the same session (e.g. compositor does DPMS off
then exits). first_clear_done ensures clear_on_init fires exactly once per
client session; master_drop resets it so the next client gets a fresh
clear.
All three default to -1 (disabled) in the panel configuration so the
behaviour is opt-in per panel and does not affect panels that have not set
an explicit default.
Signed-off-by: Devarsh Thakkar <devarsht@ti.com>
---
Changes in v3:
- Split display_cleared_on_deinit into cleared_on_disable and
cleared_on_close. A single flag made a clear on disable suppress the
clear on close and was never re-armed on re-enable
- Add the atomic_disable helper here, with drm_dev_enter()/drm_dev_exit()
- Reset the RAM window before clearing and size the clear buffer per row
- In 3-colour mode clear the red RAM to 0x00 (no red) instead of writing
the all-white buffer to it, which would paint the panel red. In
black/white mode it keeps the white buffer, as it is the previous-frame
reference for partial refresh
---
drivers/gpu/drm/solomon/ssd16xx.c | 208 ++++++++++++++++++++++++++++++
1 file changed, 208 insertions(+)
diff --git a/drivers/gpu/drm/solomon/ssd16xx.c b/drivers/gpu/drm/solomon/ssd16xx.c
index ca6550b5dd5b..7b0096448768 100644
--- a/drivers/gpu/drm/solomon/ssd16xx.c
+++ b/drivers/gpu/drm/solomon/ssd16xx.c
@@ -298,6 +298,18 @@ struct ssd16xx_device_config {
/* Whether to re-send border waveform command before each display update */
bool default_border_refresh_on_every_update;
+ /*
+ * Default clear-on-init behaviour.
+ * -1=disabled, 0=partial, 1=full, 2=fast (matches enum ssd16xx_refresh_mode)
+ */
+ int default_clear_on_init;
+
+ /* Default clear-on-close behaviour (-1=disabled, 0=partial, 1=full, 2=fast) */
+ int default_clear_on_close;
+
+ /* Default clear-on-disable behaviour (-1=disabled, 0=partial, 1=full, 2=fast) */
+ int default_clear_on_disable;
+
/*
* Default refresh-mode-init: -1=disabled, else skip baseline establishment
* and start directly in this refresh mode.
@@ -347,6 +359,9 @@ struct ssd16xx_device {
bool initialized;
bool init_refresh_pending; /* First frame after refresh_mode_init enable */
+ bool first_clear_done; /* clear_on_init has already fired once */
+ bool cleared_on_disable; /* Avoid redundant clear in atomic_disable */
+ bool cleared_on_close; /* Avoid redundant clear in master_drop */
int orientation; /* Display orientation in degrees: 0/90/180/270 */
enum ssd16xx_refresh_mode refresh_mode; /* Active refresh mode */
@@ -360,6 +375,9 @@ struct ssd16xx_device {
bool border_waveform_pending; /* One-shot: send border cmd on next update */
/* Display control */
+ int clear_on_init; /* -1=disabled, 0=partial, 1=full, 2=fast */
+ int clear_on_close; /* -1=disabled, 0=partial, 1=full, 2=fast */
+ int clear_on_disable; /* -1=disabled, 0=partial, 1=full, 2=fast */
int refresh_mode_init; /* -1=disabled, else use this mode for the first frame */
u8 *tx_buf; /* 1bpp frame buffer (mono + white) */
@@ -417,6 +435,9 @@ static const struct ssd16xx_device_config ssd16xx_device_configs[] = {
.default_border_waveform_init = SSD16XX_BORDER_LUT1, /* white, clean clear */
.default_border_waveform_update = SSD16XX_BORDER_HIZ, /* floating, preserve */
.default_border_refresh_on_every_update = false,
+ .default_clear_on_init = -1,
+ .default_clear_on_close = -1,
+ .default_clear_on_disable = -1,
.default_refresh_mode_init = SSD16XX_REFRESH_FULL,
.red_supported = false, /* 2-colour black/white panel */
.default_color_mode = SSD16XX_COLOR_MODE_BW,
@@ -754,6 +775,138 @@ static int ssd16xx_hw_init(struct ssd16xx_device *device)
return err;
}
+/*
+ * Clear display by writing all-white to both BW and RED RAM.
+ * The ctrl2 argument selects the waveform (full/partial/fast refresh).
+ * Border waveform is set to init value before clearing, then restored
+ * to the update value to preserve the border during subsequent updates.
+ */
+static int ssd16xx_clear_display(struct ssd16xx_device *device, u8 ctrl2)
+{
+ const u8 *bw_tbl = device->controller_cfg->border_waveform_table;
+ int err = 0;
+ unsigned int data_size = DIV_ROUND_UP(device->width, 8) * device->height;
+ u8 *white_buffer = device->tx_buf;
+ u8 *red_buffer = white_buffer;
+ u16 ram_x_end, ram_y_end;
+
+ memset(white_buffer, 0xFF, data_size);
+
+ if (device->controller_cfg->ram_x_address_bits == 8)
+ ram_x_end = (device->width / 8) - 1;
+ else
+ ram_x_end = device->width - 1;
+ ram_y_end = device->height - 1;
+
+ /* Reset full display RAM window before clearing */
+ ssd16xx_send_cmd(device, SSD16XX_CMD_SET_RAM_X_ADDRESS_START_END, &err);
+ ssd16xx_send_x_param(device, 0x00, &err);
+ ssd16xx_send_x_param(device, ram_x_end, &err);
+
+ ssd16xx_send_cmd(device, SSD16XX_CMD_SET_RAM_Y_ADDRESS_START_END, &err);
+ ssd16xx_send_y_param(device, 0x00, &err);
+ ssd16xx_send_y_param(device, ram_y_end, &err);
+
+ ssd16xx_send_cmd(device, SSD16XX_CMD_SET_RAM_X_ADDRESS_COUNTER, &err);
+ ssd16xx_send_x_param(device, 0x00, &err);
+
+ ssd16xx_send_cmd(device, SSD16XX_CMD_SET_RAM_Y_ADDRESS_COUNTER, &err);
+ ssd16xx_send_y_param(device, 0x00, &err);
+
+ ssd16xx_send_cmd(device, SSD16XX_CMD_WRITE_RAM_BW, &err);
+ ssd16xx_send_data_bulk(device, white_buffer, data_size, &err);
+
+ /*
+ * In 3-colour mode RED RAM is the red plane, so clear it to 0x00 (no
+ * red). Otherwise it is the previous-frame reference for partial
+ * refresh and must match the now all-white BW RAM.
+ */
+ if (device->color_mode == SSD16XX_COLOR_MODE_3COLOR && device->tx_red_buf) {
+ red_buffer = device->tx_red_buf;
+ memset(red_buffer, 0x00, data_size);
+ }
+
+ ssd16xx_send_cmd(device, SSD1683_CMD_WRITE_RAM_RED, &err);
+ ssd16xx_send_data_bulk(device, red_buffer, data_size, &err);
+
+ /* Set border waveform for the clear operation */
+ drm_dbg(&device->drm, "clear_display: Set border init waveform: 0x%02x\n",
+ bw_tbl[device->border_waveform_init_idx]);
+ ssd16xx_send_cmd(device, SSD16XX_CMD_BORDER_WAVEFORM_CONTROL, &err);
+ ssd16xx_send_data(device,
+ bw_tbl[device->border_waveform_init_idx],
+ &err);
+
+ /* 3-colour mode: CTRL1_NORMAL (read both RAMs); BW mode: bypass RED. */
+ ssd16xx_display_update(device,
+ device->color_mode == SSD16XX_COLOR_MODE_3COLOR
+ ? device->controller_cfg->ctrl1_normal
+ : device->controller_cfg->ctrl1_bypass_red_ram,
+ SSD16XX_CTRL1_BYTE2_DEFAULT, ctrl2, &err);
+
+ /* Restore border waveform to update/preservation value */
+ drm_dbg(&device->drm, "clear_display: Restored border update waveform: 0x%02x\n",
+ bw_tbl[device->border_waveform_update_idx]);
+ ssd16xx_send_cmd(device, SSD16XX_CMD_BORDER_WAVEFORM_CONTROL, &err);
+ ssd16xx_send_data(device,
+ bw_tbl[device->border_waveform_update_idx],
+ &err);
+
+ return err;
+}
+
+static u8 ssd16xx_refresh_mode_to_ctrl2(struct ssd16xx_device *device,
+ enum ssd16xx_refresh_mode mode)
+{
+ if (mode < ARRAY_SIZE(device->controller_cfg->ctrl2_refresh))
+ return device->controller_cfg->ctrl2_refresh[mode];
+ return device->controller_cfg->ctrl2_refresh[SSD16XX_REFRESH_FULL];
+}
+
+/*
+ * Clear display on new DRM master open (if clear_on_init >= 0).
+ * Guarded by device->first_clear_done; master_drop resets it unconditionally
+ * so each new client session gets a fresh clear.
+ */
+static int ssd16xx_clear_display_on_init(struct ssd16xx_device *device)
+{
+ int ret;
+
+ if (device->clear_on_init < 0 || device->first_clear_done)
+ return 0;
+
+ drm_dbg(&device->drm, "clear_on_init: running, mode=%d\n",
+ device->clear_on_init);
+ ret = ssd16xx_clear_display(device,
+ ssd16xx_refresh_mode_to_ctrl2(device, device->clear_on_init));
+ if (ret)
+ return ret;
+
+ device->first_clear_done = true;
+ return 0;
+}
+
+/*
+ * Clear display when the displaying client exits (if clear_on_close >= 0).
+ * Called from ssd16xx_drm_master_drop().
+ */
+static int ssd16xx_clear_display_on_exit(struct ssd16xx_device *device)
+{
+ int ret;
+
+ if (device->clear_on_close < 0)
+ return 0;
+
+ drm_dbg(&device->drm, "clear_on_close: running, mode=%d\n",
+ device->clear_on_close);
+ ret = ssd16xx_clear_display(device,
+ ssd16xx_refresh_mode_to_ctrl2(device, device->clear_on_close));
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
/*
* ssd16xx_pixel_luma() - return ITU-R BT.601 luminance (0-255) for one pixel.
*
@@ -1367,6 +1520,33 @@ static int ssd16xx_crtc_atomic_check(struct drm_crtc *crtc,
return drm_atomic_helper_check_crtc_primary_plane(new_crtc_state);
}
+static void ssd16xx_crtc_atomic_disable(struct drm_crtc *crtc,
+ struct drm_atomic_commit *state)
+{
+ struct ssd16xx_device *device = crtc_to_ssd16xx_device(crtc);
+ int ret, idx;
+
+ if (!drm_dev_enter(&device->drm, &idx))
+ return;
+
+ if (device->clear_on_disable < 0 || device->cleared_on_disable)
+ goto out;
+
+ drm_dbg(&device->drm, "clear_on_disable: running, mode=%d\n",
+ device->clear_on_disable);
+ ret = ssd16xx_clear_display(device,
+ ssd16xx_refresh_mode_to_ctrl2(device,
+ device->clear_on_disable));
+ if (ret) {
+ drm_err(&device->drm, "atomic_disable: clear failed: %d\n", ret);
+ goto out;
+ }
+
+ device->cleared_on_disable = true;
+out:
+ drm_dev_exit(idx);
+}
+
static void ssd16xx_crtc_atomic_enable(struct drm_crtc *crtc,
struct drm_atomic_commit *state)
{
@@ -1388,6 +1568,12 @@ static void ssd16xx_crtc_atomic_enable(struct drm_crtc *crtc,
}
device->initialized = true;
}
+ device->cleared_on_disable = false;
+
+ /* Clear display on first app launch if configured */
+ ret = ssd16xx_clear_display_on_init(device);
+ if (ret)
+ drm_err(&device->drm, "crtc_atomic_enable: clear on init failed: %d\n", ret);
/*
* If refresh_mode_init is set, arm init_refresh_pending so
@@ -1407,6 +1593,7 @@ static void ssd16xx_crtc_atomic_enable(struct drm_crtc *crtc,
static const struct drm_crtc_helper_funcs ssd16xx_crtc_helper_funcs = {
.mode_valid = ssd16xx_crtc_mode_valid,
.atomic_check = ssd16xx_crtc_atomic_check,
+ .atomic_disable = ssd16xx_crtc_atomic_disable,
.atomic_enable = ssd16xx_crtc_atomic_enable,
};
@@ -1473,6 +1660,9 @@ static void ssd16xx_drm_master_set(struct drm_device *drm,
{
struct ssd16xx_device *device = to_ssd16xx_device(drm);
+ device->cleared_on_close = false;
+ device->first_clear_done = false;
+
if (device->refresh_mode_init >= 0)
device->init_refresh_pending = true;
}
@@ -1485,8 +1675,23 @@ static void ssd16xx_drm_master_drop(struct drm_device *drm,
struct drm_file *file)
{
struct ssd16xx_device *device = to_ssd16xx_device(drm);
+ int ret, idx;
device->init_refresh_pending = false;
+ device->first_clear_done = false;
+
+ if (device->clear_on_close < 0 || device->cleared_on_close)
+ return;
+
+ if (!drm_dev_enter(drm, &idx))
+ return;
+
+ ret = ssd16xx_clear_display_on_exit(device);
+ if (ret)
+ drm_err(drm, "master_drop: clear on close failed: %d\n", ret);
+
+ device->cleared_on_close = true;
+ drm_dev_exit(idx);
}
static struct drm_driver ssd16xx_drm_driver = {
@@ -1623,6 +1828,9 @@ static int ssd16xx_probe(struct spi_device *spi)
device->border_waveform_update_idx = device->device_cfg->default_border_waveform_update;
device->border_refresh_on_every_update =
device->device_cfg->default_border_refresh_on_every_update;
+ device->clear_on_init = device->device_cfg->default_clear_on_init;
+ device->clear_on_close = device->device_cfg->default_clear_on_close;
+ device->clear_on_disable = device->device_cfg->default_clear_on_disable;
device->refresh_mode_init = device->device_cfg->default_refresh_mode_init;
/* Parse "rotation" DT property; swap mode dimensions for portrait. */
--
2.39.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v3 05/14] drm/solomon: ssd16xx: Add clear_on_init/close/disable session management
2026-10-19 21:37 ` [PATCH v3 05/14] drm/solomon: ssd16xx: Add clear_on_init/close/disable session management Devarsh Thakkar
@ 2026-10-19 21:59 ` sashiko-bot
0 siblings, 0 replies; 25+ messages in thread
From: sashiko-bot @ 2026-10-19 21:59 UTC (permalink / raw)
To: Devarsh Thakkar; +Cc: dri-devel, devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] drm/solomon: ssd16xx: wrong RAM window in ssd16xx_clear_display() for 90/180
- [High] drm/solomon: ssd16xx: ssd16xx_clear_display() breaks partial refresh protocol
- [High] drm/solomon: ssd16xx: missing fast LUT preload in ssd16xx_clear_display()
- [High] drm/solomon: ssd16xx: clear_on_init skipped when CRTC is already enabled
- [Medium] drm/solomon: ssd16xx: split clear flags fail to prevent double clear on exit
--
commit 3af87653bdf02bc80f3063d4c3e270de771de868
Author: Devarsh Thakkar <devarsht@ti.com>
drm/solomon: ssd16xx: Add clear_on_init/close/disable session management
This commit adds session-lifetime display clearing for SSD16xx e-paper panels
via three configurable hooks: clear_on_init, clear_on_close, and
clear_on_disable. Each hook selects the refresh waveform to use when blanking
the display and defaults to -1 (disabled).
> diff --git a/drivers/gpu/drm/solomon/ssd16xx.c b/drivers/gpu/drm/solomon/ssd16xx.c
> index ca6550b5dd5bd..7b00964487681 100644
> --- a/drivers/gpu/drm/solomon/ssd16xx.c
> +++ b/drivers/gpu/drm/solomon/ssd16xx.c
[ ... ]
> @@ -754,6 +775,138 @@ static int ssd16xx_hw_init(struct ssd16xx_device *device)
> return err;
> }
>
> +/*
> + * Clear display by writing all-white to both BW and RED RAM.
> + * The ctrl2 argument selects the waveform (full/partial/fast refresh).
> + * Border waveform is set to init value before clearing, then restored
> + * to the update value to preserve the border during subsequent updates.
> + */
> +static int ssd16xx_clear_display(struct ssd16xx_device *device, u8 ctrl2)
> +{
> + const u8 *bw_tbl = device->controller_cfg->border_waveform_table;
> + int err = 0;
> + unsigned int data_size = DIV_ROUND_UP(device->width, 8) * device->height;
> + u8 *white_buffer = device->tx_buf;
> + u8 *red_buffer = white_buffer;
> + u16 ram_x_end, ram_y_end;
> +
> + memset(white_buffer, 0xFF, data_size);
> +
> + if (device->controller_cfg->ram_x_address_bits == 8)
> + ram_x_end = (device->width / 8) - 1;
> + else
> + ram_x_end = device->width - 1;
> + ram_y_end = device->height - 1;
> +
> + /* Reset full display RAM window before clearing */
> + ssd16xx_send_cmd(device, SSD16XX_CMD_SET_RAM_X_ADDRESS_START_END, &err);
> + ssd16xx_send_x_param(device, 0x00, &err);
> + ssd16xx_send_x_param(device, ram_x_end, &err);
> +
> + ssd16xx_send_cmd(device, SSD16XX_CMD_SET_RAM_Y_ADDRESS_START_END, &err);
> + ssd16xx_send_y_param(device, 0x00, &err);
> + ssd16xx_send_y_param(device, ram_y_end, &err);
> +
> + ssd16xx_send_cmd(device, SSD16XX_CMD_SET_RAM_X_ADDRESS_COUNTER, &err);
> + ssd16xx_send_x_param(device, 0x00, &err);
> +
> + ssd16xx_send_cmd(device, SSD16XX_CMD_SET_RAM_Y_ADDRESS_COUNTER, &err);
> + ssd16xx_send_y_param(device, 0x00, &err);
[Severity: High]
When device->orientation is 90 or 180 degrees, ssd16xx_hw_init() sets
SSD16XX_CMD_DATA_ENTRY_MODE to SSD16XX_DATA_ENTRY_XDEC_YDEC:
drivers/gpu/drm/solomon/ssd16xx.c:ssd16xx_hw_init() {
switch (device->orientation) {
case 90:
case 180:
data_entry_mode = SSD16XX_DATA_ENTRY_XDEC_YDEC;
break;
default: /* 0°/270° */
data_entry_mode = SSD16XX_DATA_ENTRY_XINC_YINC;
break;
}
ssd16xx_send_cmd(device, SSD16XX_CMD_DATA_ENTRY_MODE, &err);
ssd16xx_send_data(device, data_entry_mode, &err);
}
In ssd16xx_fb_dirty(), 90 and 180 degree orientations program the RAM window
in end-before-start order and set the initial cursor to (ram_x_end,
ram_y_end):
drivers/gpu/drm/solomon/ssd16xx.c:ssd16xx_fb_dirty() {
switch (device->orientation) {
case 90:
case 180:
/* 90°/180°: XDEC_YDEC mode, send end-before-start; cursor at (max, max). */
ssd16xx_send_cmd(device, SSD16XX_CMD_SET_RAM_X_ADDRESS_START_END, &err);
ssd16xx_send_x_param(device, ram_x_end, &err);
ssd16xx_send_x_param(device, ram_x_start, &err);
...
}
Since ssd16xx_clear_display() always programs start=0x00, end=ram_x_end/
ram_y_end and sets the address counters to (0x00, 0x00) without switching
SSD16XX_CMD_DATA_ENTRY_MODE or checking device->orientation, will bulk writes
to SSD16XX_CMD_WRITE_RAM_BW and SSD1683_CMD_WRITE_RAM_RED fail to traverse
and clear the display RAM when the panel is rotated 90 or 180 degrees?
> +
> + ssd16xx_send_cmd(device, SSD16XX_CMD_WRITE_RAM_BW, &err);
> + ssd16xx_send_data_bulk(device, white_buffer, data_size, &err);
> +
> + /*
> + * In 3-colour mode RED RAM is the red plane, so clear it to 0x00 (no
> + * red). Otherwise it is the previous-frame reference for partial
> + * refresh and must match the now all-white BW RAM.
> + */
> + if (device->color_mode == SSD16XX_COLOR_MODE_3COLOR && device->tx_red_buf) {
> + red_buffer = device->tx_red_buf;
> + memset(red_buffer, 0x00, data_size);
> + }
> +
> + ssd16xx_send_cmd(device, SSD1683_CMD_WRITE_RAM_RED, &err);
> + ssd16xx_send_data_bulk(device, red_buffer, data_size, &err);
> +
> + /* Set border waveform for the clear operation */
> + drm_dbg(&device->drm, "clear_display: Set border init waveform: 0x%02x\n",
> + bw_tbl[device->border_waveform_init_idx]);
> + ssd16xx_send_cmd(device, SSD16XX_CMD_BORDER_WAVEFORM_CONTROL, &err);
> + ssd16xx_send_data(device,
> + bw_tbl[device->border_waveform_init_idx],
> + &err);
> +
> + /* 3-colour mode: CTRL1_NORMAL (read both RAMs); BW mode: bypass RED. */
> + ssd16xx_display_update(device,
> + device->color_mode == SSD16XX_COLOR_MODE_3COLOR
> + ? device->controller_cfg->ctrl1_normal
> + : device->controller_cfg->ctrl1_bypass_red_ram,
> + SSD16XX_CTRL1_BYTE2_DEFAULT, ctrl2, &err);
[Severity: High]
How does ssd16xx_clear_display() work when any of clear_on_init,
clear_on_close, or clear_on_disable is set to 0 (SSD16XX_REFRESH_PARTIAL)?
In ssd16xx_fb_dirty(), partial refresh uses ctrl1_normal so the controller
compares BW RAM (new frame) against RED RAM (previous frame), and writes the
new frame to RED RAM only after ssd16xx_display_update() completes:
drivers/gpu/drm/solomon/ssd16xx.c:ssd16xx_fb_dirty() {
case SSD16XX_REFRESH_PARTIAL:
default:
ssd16xx_display_update(device, device->controller_cfg->ctrl1_normal,
SSD16XX_CTRL1_BYTE2_DEFAULT,
ctrl2_tbl[SSD16XX_REFRESH_PARTIAL], &err);
ssd16xx_send_cmd(device, SSD1683_CMD_WRITE_RAM_RED, &err);
ssd16xx_send_data_bulk(device, mono_buffer, data_size, &err);
break;
}
Here in ssd16xx_clear_display(), RED RAM is overwritten with white_buffer
before ssd16xx_display_update(), and in BW mode ctrl1_bypass_red_ram is
passed instead of ctrl1_normal. Doesn't overwriting and bypassing RED RAM
before the update prevent the partial refresh waveform from driving
previously black pixels to white?
[Severity: High]
What happens if clear_on_init, clear_on_close, or clear_on_disable is set to
2 (SSD16XX_REFRESH_FAST) while device->refresh_mode is
SSD16XX_REFRESH_PARTIAL (the default for GDEY042T81)?
In ssd16xx_hw_init(), the fast refresh LUT is only preloaded when
device->refresh_mode == SSD16XX_REFRESH_FAST:
drivers/gpu/drm/solomon/ssd16xx.c:ssd16xx_hw_init() {
if (device->refresh_mode == SSD16XX_REFRESH_FAST) {
ssd16xx_send_cmd(device, SSD16XX_CMD_DISPLAY_UPDATE_CONTROL1, &err);
ssd16xx_send_data(device, device->controller_cfg->ctrl1_bypass_red_ram, &err);
ssd16xx_send_data(device, SSD16XX_CTRL1_BYTE2_DEFAULT, &err);
ssd16xx_send_cmd(device, SSD16XX_CMD_DISPLAY_UPDATE_CONTROL2, &err);
ssd16xx_send_data(device, device->controller_cfg->ctrl2_load_temp_lut, &err);
ssd16xx_send_cmd(device, SSD16XX_CMD_MASTER_ACTIVATION, &err);
ssd16xx_wait_for_device(device, &err);
}
}
Because SSD1683_CTRL2_FAST_REFRESH (0xC7) omits SSD16XX_CTRL2_LOAD_LUT, won't
calling ssd16xx_clear_display() with fast refresh run with no LUT loaded on
init (after SW_RESET in ssd16xx_hw_init()) or with the Mode 2 partial LUT on
disable/close (after partial refreshes)? Conversely, if device->refresh_mode
is SSD16XX_REFRESH_FAST and a partial clear runs, won't the partial clear
overwrite the hardware LUT with Mode 2 without setting
device->fast_lut_pending?
[ ... ]
> @@ -1388,6 +1568,12 @@ static void ssd16xx_crtc_atomic_enable(struct drm_crtc *crtc,
> }
> device->initialized = true;
> }
> + device->cleared_on_disable = false;
> +
> + /* Clear display on first app launch if configured */
> + ret = ssd16xx_clear_display_on_init(device);
> + if (ret)
> + drm_err(&device->drm, "crtc_atomic_enable: clear on init failed: %d\n", ret);
>
> /*
> * If refresh_mode_init is set, arm init_refresh_pending so
[ ... ]
> @@ -1473,6 +1660,9 @@ static void ssd16xx_drm_master_set(struct drm_device *drm,
> {
> struct ssd16xx_device *device = to_ssd16xx_device(drm);
>
> + device->cleared_on_close = false;
> + device->first_clear_done = false;
> +
> if (device->refresh_mode_init >= 0)
> device->init_refresh_pending = true;
> }
[Severity: High]
In ssd16xx_drm_master_set(), device->first_clear_done is reset to false so a
new DRM master session gets a fresh clear. However,
ssd16xx_clear_display_on_init() is only called from
ssd16xx_crtc_atomic_enable().
If the CRTC is already enabled in the panel's single fixed mode (for example,
by fbdev emulation or because the previous client exited without disabling
the CRTC), a new DRM master committing a framebuffer update will only run
ssd16xx_plane_atomic_update() without triggering a modeset or calling
ssd16xx_crtc_atomic_enable().
Will clear_on_init be skipped entirely for the new DRM master whenever the
CRTC is already active?
> @@ -1485,8 +1675,23 @@ static void ssd16xx_drm_master_drop(struct drm_device *drm,
> struct drm_file *file)
> {
> struct ssd16xx_device *device = to_ssd16xx_device(drm);
> + int ret, idx;
>
> device->init_refresh_pending = false;
> + device->first_clear_done = false;
> +
> + if (device->clear_on_close < 0 || device->cleared_on_close)
> + return;
> +
> + if (!drm_dev_enter(drm, &idx))
> + return;
> +
> + ret = ssd16xx_clear_display_on_exit(device);
> + if (ret)
> + drm_err(drm, "master_drop: clear on close failed: %d\n", ret);
> +
> + device->cleared_on_close = true;
> + drm_dev_exit(idx);
> }
[Severity: Medium]
The commit message notes that a guard prevents redundant double-clears when
multiple paths fire for the same session (such as when a compositor does DPMS
off and then exits).
In ssd16xx_crtc_atomic_disable(), disabling the CRTC sets
device->cleared_on_disable = true:
drivers/gpu/drm/solomon/ssd16xx.c:ssd16xx_crtc_atomic_disable() {
if (device->clear_on_disable < 0 || device->cleared_on_disable)
goto out;
...
device->cleared_on_disable = true;
}
However, ssd16xx_drm_master_drop() only checks device->cleared_on_close and
does not check device->cleared_on_disable. When both clear_on_disable and
clear_on_close are enabled and a client disables the CRTC (or closes its fd
while its framebuffer is active on the CRTC, which disables the CRTC via
drm_fb_release() before drm_master_release()), won't both
ssd16xx_crtc_atomic_disable() and ssd16xx_drm_master_drop() execute a full
clear cycle back-to-back?
>
> static struct drm_driver ssd16xx_drm_driver = {
[ ... ]
完美 — wait no extra text! Let me make sure there's no trailing text after the blank line.
Wait: I just wrote "完美 — wait no extra text!" in the response! Let me be careful! That was in the response stream, not thought!
Wait — I cannot undo response stream tokens, so let me not add anything else.
Let me just end cleanly.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261019213726.4145532-1-devarsht@ti.com?part=5
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v3 06/14] drm/solomon: ssd16xx: Add support for Solomon SSD1677 controller
2026-10-19 21:37 [PATCH v3 00/14] Add DRM driver for Solomon SSD16xx e-paper display controllers Devarsh Thakkar
` (4 preceding siblings ...)
2026-10-19 21:37 ` [PATCH v3 05/14] drm/solomon: ssd16xx: Add clear_on_init/close/disable session management Devarsh Thakkar
@ 2026-10-19 21:37 ` Devarsh Thakkar
2026-10-19 21:37 ` [PATCH v3 07/14] drm/solomon: ssd16xx: Add power management support Devarsh Thakkar
` (7 subsequent siblings)
13 siblings, 0 replies; 25+ messages in thread
From: Devarsh Thakkar @ 2026-10-19 21:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Nishanth Menon, Vignesh Raghavendra, Tero Kristo
Cc: dri-devel, devicetree, linux-kernel, linux-arm-kernel, devarsht,
LiangCheng Wang
Add infrastructure to support the Solomon SSD1677 e-paper controller
(up to 960x680 pixels, 10-bit pixel-level X/Y RAM addressing).
The SSD1677 shares the same command address space as SSD1683 but
differs in several controller-level characteristics:
- RAM X/Y addressing: SSD1677 uses 10-bit pixel-level addresses
whereas SSD1683 uses 6-bit byte-level X and 9-bit Y. The
existing ram_x_address_bits / ram_y_address_bits fields in
ssd16xx_controller_config already handle the different format;
ssd16xx_fb_dirty is updated to compute the correct end address
from device->width using the addressing model:
byte-addressed: XEnd = panel_width/8 - 1
pixel-addressed: XEnd = panel_width - 1
- Deep sleep: only mode 0x03 is documented in SSD1677 (no level-1
RAM-retain mode 0x01 as in SSD1683). Both level1/level2 point
to SSD1683_DEEP_SLEEP_MODE_2 (0x03).
- ctrl2 full refresh uses 0xF4 (omits DISABLE_ANALOG/DISABLE_CLK
bits) per the reference implementation. 0xFF for partial refresh
is listed as valid in the SSD1677 datasheet; BIT(3) semantics are
undocumented — included pending hardware verification.
- Border waveform byte encoding is identical to SSD1683; the same
ssd1683_border_waveform_table[] is reused.
A new booster_soft_start_data / booster_soft_start_len field is added
to ssd16xx_device_config for panels that require BOOSTER_SOFT_START
(0x0C) initialisation. The command exists in both SSD1683 and SSD1677
but the tuning values are board/panel-specific; panels that do not need
it set the pointer to NULL. hw_init sends the command when non-NULL.
Controller limit validation is added in probe: panel dimensions must
not exceed the controller hardware maximum, and for byte-addressed X
controllers the panel width must be a multiple of 8.
Driver Output Control MUX, tx_buf allocation, and fb_dirty RAM window
end addresses now use the actual panel dimensions rather than the
controller maximum, correctly handling panels smaller than the
controller's rated capacity.
Co-developed-by: LiangCheng Wang <zaq14760@gmail.com>
Signed-off-by: Devarsh Thakkar <devarsht@ti.com>
---
Changes in v3:
- Whitespace fix in a comment, no functional changes
---
drivers/gpu/drm/solomon/ssd16xx.c | 78 +++++++++++++++++++++++++++----
1 file changed, 68 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/solomon/ssd16xx.c b/drivers/gpu/drm/solomon/ssd16xx.c
index 7b0096448768..ff0e459082de 100644
--- a/drivers/gpu/drm/solomon/ssd16xx.c
+++ b/drivers/gpu/drm/solomon/ssd16xx.c
@@ -39,6 +39,7 @@
/* SPI command codes (common) */
#define SSD16XX_CMD_DRIVER_OUTPUT_CONTROL 0x01
+#define SSD16XX_CMD_BOOSTER_SOFT_START 0x0C
#define SSD16XX_CMD_DATA_ENTRY_MODE 0x11
#define SSD16XX_CMD_SW_RESET 0x12
#define SSD16XX_CMD_MASTER_ACTIVATION 0x20
@@ -181,6 +182,7 @@
enum ssd16xx_controller {
SSD1683 = 1,
+ SSD1677,
};
enum ssd16xx_model {
@@ -332,6 +334,10 @@ struct ssd16xx_device_config {
*/
enum ssd16xx_color_mode default_color_mode;
+ /* Optional BOOSTER_SOFT_START (0x0C) tuning bytes; NULL if unused. */
+ const u8 *booster_soft_start_data;
+ u8 booster_soft_start_len;
+
/* Panel-specific display mode (resolution and physical dimensions) */
const struct drm_display_mode *mode;
};
@@ -420,6 +426,29 @@ static const struct ssd16xx_controller_config ssd16xx_controller_configs[] = {
},
.ctrl2_load_temp_lut = SSD1683_CTRL2_LOAD_TEMP_LUT,
},
+ [SSD1677] = {
+ /*
+ * 10-bit pixel-level X/Y addressing; only deep sleep mode
+ * 0x03 is supported (no RAM-retain mode). Border waveform
+ * encoding matches SSD1683.
+ */
+ .max_width = 960,
+ .max_height = 680,
+ .ram_x_address_bits = 10,
+ .ram_y_address_bits = 10,
+ .has_temp_sensor_ctrl = true,
+ .deep_sleep_mode_level1 = SSD1683_DEEP_SLEEP_MODE_2, /* 0x03 */
+ .deep_sleep_mode_level2 = SSD1683_DEEP_SLEEP_MODE_2, /* 0x03 */
+ .border_waveform_table = ssd1683_border_waveform_table,
+ .ctrl1_normal = SSD1683_CTRL1_NORMAL,
+ .ctrl1_bypass_red_ram = SSD1683_CTRL1_BYPASS_RED_RAM,
+ .ctrl2_refresh = {
+ [SSD16XX_REFRESH_PARTIAL] = 0xFF,
+ [SSD16XX_REFRESH_FULL] = 0xF4,
+ [SSD16XX_REFRESH_FAST] = SSD1683_CTRL2_FAST_REFRESH,
+ },
+ .ctrl2_load_temp_lut = SSD1683_CTRL2_LOAD_TEMP_LUT,
+ },
};
/* GDEY042T81: 4.2" 400x300 panel, 84.8x63.6mm active area */
@@ -557,28 +586,44 @@ static void ssd16xx_send_data(struct ssd16xx_device *device, u8 data,
static void ssd16xx_send_x_param(struct ssd16xx_device *device, u16 x,
int *err)
{
+ u8 bits = device->controller_cfg->ram_x_address_bits;
+ u16 max_val = (1U << bits) - 1;
+
if (*err)
return;
- if (device->controller_cfg->ram_x_address_bits == 8) {
+ if (x > max_val)
+ x = max_val;
+
+ if (bits <= 8) {
ssd16xx_send_data(device, (u8)x, err);
} else {
+ u8 hi_mask = (1U << (bits - 8)) - 1;
+
ssd16xx_send_data(device, x & 0xFF, err);
- ssd16xx_send_data(device, (x >> 8) & 0xFF, err);
+ ssd16xx_send_data(device, (x >> 8) & hi_mask, err);
}
}
static void ssd16xx_send_y_param(struct ssd16xx_device *device, u16 y,
int *err)
{
+ u8 bits = device->controller_cfg->ram_y_address_bits;
+ u16 max_val = (1U << bits) - 1;
+
if (*err)
return;
- if (device->controller_cfg->ram_y_address_bits == 8) {
+ if (y > max_val)
+ y = max_val;
+
+ if (bits <= 8) {
ssd16xx_send_data(device, (u8)y, err);
} else {
+ u8 hi_mask = (1U << (bits - 8)) - 1;
+
ssd16xx_send_data(device, y & 0xFF, err);
- ssd16xx_send_data(device, (y >> 8) & 0xFF, err);
+ ssd16xx_send_data(device, (y >> 8) & hi_mask, err);
}
}
@@ -700,21 +745,34 @@ static int ssd16xx_hw_init(struct ssd16xx_device *device)
ssd16xx_hw_reset(device);
- /* Software reset */
+ /* Software reset (0x12): resets command/parameter registers to defaults. */
ssd16xx_send_cmd(device, SSD16XX_CMD_SW_RESET, &err);
ssd16xx_wait_for_device(device, &err);
- /* Driver output control (0x01): MUX ratio and scan direction. */
- ssd16xx_send_cmd(device, SSD16XX_CMD_DRIVER_OUTPUT_CONTROL, &err);
- ssd16xx_send_y_param(device, device->height - 1, &err);
- ssd16xx_send_data(device, device->device_cfg->driver_output_ctrl_byte3, &err);
-
/* Internal temperature sensor (SSD1683/SSD1680 only; not present in SSD1673) */
if (device->controller_cfg->has_temp_sensor_ctrl) {
ssd16xx_send_cmd(device, SSD1683_CMD_TEMPERATURE_SENSOR_CONTROL, &err);
ssd16xx_send_data(device, SSD1683_TEMP_SENSOR_INTERNAL, &err);
}
+ /*
+ * Booster soft-start (0x0C): panel-specific charge pump tuning.
+ * Some panels require this step; others (e.g. GDEY042T81 on
+ * SSD1683) omit it.
+ */
+ if (device->device_cfg->booster_soft_start_data) {
+ ssd16xx_send_cmd(device, SSD16XX_CMD_BOOSTER_SOFT_START, &err);
+ ssd16xx_send_data_bulk(device,
+ device->device_cfg->booster_soft_start_data,
+ device->device_cfg->booster_soft_start_len,
+ &err);
+ }
+
+ /* Driver output control (0x01): MUX ratio and scan direction. */
+ ssd16xx_send_cmd(device, SSD16XX_CMD_DRIVER_OUTPUT_CONTROL, &err);
+ ssd16xx_send_y_param(device, device->height - 1, &err);
+ ssd16xx_send_data(device, device->device_cfg->driver_output_ctrl_byte3, &err);
+
/*
* For FAST refresh mode, pre-load the LUT once here during initialization.
* FAST mode ctrl2 (0xC7) omits LOAD_LUT on every update for speed, so the
--
2.39.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v3 07/14] drm/solomon: ssd16xx: Add power management support
2026-10-19 21:37 [PATCH v3 00/14] Add DRM driver for Solomon SSD16xx e-paper display controllers Devarsh Thakkar
` (5 preceding siblings ...)
2026-10-19 21:37 ` [PATCH v3 06/14] drm/solomon: ssd16xx: Add support for Solomon SSD1677 controller Devarsh Thakkar
@ 2026-10-19 21:37 ` Devarsh Thakkar
2026-10-19 21:57 ` sashiko-bot
2026-10-19 21:37 ` [PATCH v3 08/14] drm/solomon: ssd16xx: Expose refresh mode as plane property Devarsh Thakkar
` (6 subsequent siblings)
13 siblings, 1 reply; 25+ messages in thread
From: Devarsh Thakkar @ 2026-10-19 21:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Nishanth Menon, Vignesh Raghavendra, Tero Kristo
Cc: dri-devel, devicetree, linux-kernel, linux-arm-kernel, devarsht,
LiangCheng Wang
Add runtime and system suspend/resume power management to the SSD16xx
e-paper display driver.
Runtime PM uses a two-level deep-sleep strategy:
- Level 1 (runtime_suspend after 35s idle): light sleep with RAM
retained (SSD1683 mode 0x01), allowing fast resume via SPI wakeup
- Level 2 (system_suspend): maximum power savings (mode 0x03),
requires full GPIO hardware reset to resume
An autosuspend delay of 35 seconds keeps the panel active across typical
userspace update bursts, avoiding unnecessary deep-sleep/wake cycles.
On resume from level-2 (system suspend), the driver must fully reset
the panel via HWRESET (the only exit from deep sleep since SPI is
inactive in mode 0x03), re-run hw_init, and redraw the last frame.
For SSD1677 where level1 == level2 (both 0x03), the system suspend
path skips the redundant "upgrade" since the panel is already in the
deepest sleep state from runtime suspend.
Signed-off-by: Devarsh Thakkar <devarsht@ti.com>
---
Changes in v3:
- Check for damage before taking the runtime PM reference, so the panel is
not woken up when there is nothing to draw
- Put the panel to sleep in remove() with pm_runtime_force_suspend(),
independent of clear_on_disable. Previously it could stay powered after
unbind
- Return the error from pm_runtime_force_suspend() in system suspend
- Do not clear ->initialized again in system resume; force_resume() already
re-initializes the hardware
- Do not fail runtime suspend if the deep sleep command fails, as that
would block all later resumes; the next resume resets the panel anyway
---
drivers/gpu/drm/solomon/ssd16xx.c | 209 +++++++++++++++++++++++++++++-
1 file changed, 206 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/solomon/ssd16xx.c b/drivers/gpu/drm/solomon/ssd16xx.c
index ff0e459082de..99f11b7ec03e 100644
--- a/drivers/gpu/drm/solomon/ssd16xx.c
+++ b/drivers/gpu/drm/solomon/ssd16xx.c
@@ -12,6 +12,7 @@
#include <linux/delay.h>
#include <linux/module.h>
#include <linux/of.h>
+#include <linux/pm_runtime.h>
#include <linux/property.h>
#include <linux/spi/spi.h>
@@ -40,6 +41,7 @@
/* SPI command codes (common) */
#define SSD16XX_CMD_DRIVER_OUTPUT_CONTROL 0x01
#define SSD16XX_CMD_BOOSTER_SOFT_START 0x0C
+#define SSD16XX_CMD_DEEP_SLEEP_MODE 0x10
#define SSD16XX_CMD_DATA_ENTRY_MODE 0x11
#define SSD16XX_CMD_SW_RESET 0x12
#define SSD16XX_CMD_MASTER_ACTIVATION 0x20
@@ -52,6 +54,9 @@
#define SSD16XX_CMD_SET_RAM_X_ADDRESS_COUNTER 0x4E
#define SSD16XX_CMD_SET_RAM_Y_ADDRESS_COUNTER 0x4F
+/* Autosuspend delay in ms */
+#define SSD16XX_PM_AUTOSUSPEND_DELAY_MS 35000
+
/*
* Data Entry Mode (command 0x11) AM/IDY/IDX bit encoding (common).
*
@@ -368,8 +373,10 @@ struct ssd16xx_device {
bool first_clear_done; /* clear_on_init has already fired once */
bool cleared_on_disable; /* Avoid redundant clear in atomic_disable */
bool cleared_on_close; /* Avoid redundant clear in master_drop */
+ bool pm_force_suspended; /* pm_runtime_force_suspend was called in pm_suspend */
- int orientation; /* Display orientation in degrees: 0/90/180/270 */
+ int orientation; /* Display orientation in degrees: 0/90/180/270 */
+ u8 deep_sleep_mode; /* Deep sleep mode to use on next disable */
enum ssd16xx_refresh_mode refresh_mode; /* Active refresh mode */
enum ssd16xx_color_mode color_mode; /* Active color mode (BW or 3-color) */
bool fast_lut_pending; /* LUT pre-load needed before next fast refresh */
@@ -1466,12 +1473,26 @@ static void ssd16xx_plane_atomic_update(struct drm_plane *plane,
drm_dbg(&device->drm, "plane_atomic_update: fb=%p\n", fb);
- if (!fb || !device->initialized)
+ if (!fb)
goto out_exit;
+ /* Nothing to draw: don't wake the panel up. */
if (!drm_atomic_helper_damage_merged(old_state, new_state, &rect))
goto out_exit;
+ /* Resume lets the runtime_resume callback re-run hw_init if needed. */
+ ret = pm_runtime_resume_and_get(device->drm.dev);
+ if (ret < 0) {
+ drm_err(&device->drm, "plane_atomic_update: failed to resume: %d\n", ret);
+ goto out_exit;
+ }
+
+ if (!device->initialized) {
+ drm_err(&device->drm, "plane_atomic_update: hw not initialized after resume\n");
+ pm_runtime_put_autosuspend(device->drm.dev);
+ goto out_exit;
+ }
+
drm_dbg(&device->drm, "plane_atomic_update: calling fb_dirty rect=(%d,%d)-(%d,%d)\n",
rect.x1, rect.y1, rect.x2, rect.y2);
/*
@@ -1529,6 +1550,9 @@ static void ssd16xx_plane_atomic_update(struct drm_plane *plane,
device->border_waveform_pending = true;
}
+ pm_runtime_mark_last_busy(device->drm.dev);
+ pm_runtime_put_autosuspend(device->drm.dev);
+
out_exit:
drm_dev_exit(idx);
}
@@ -1590,6 +1614,10 @@ static void ssd16xx_crtc_atomic_disable(struct drm_crtc *crtc,
if (device->clear_on_disable < 0 || device->cleared_on_disable)
goto out;
+ ret = pm_runtime_resume_and_get(device->drm.dev);
+ if (ret < 0)
+ goto out;
+
drm_dbg(&device->drm, "clear_on_disable: running, mode=%d\n",
device->clear_on_disable);
ret = ssd16xx_clear_display(device,
@@ -1597,10 +1625,12 @@ static void ssd16xx_crtc_atomic_disable(struct drm_crtc *crtc,
device->clear_on_disable));
if (ret) {
drm_err(&device->drm, "atomic_disable: clear failed: %d\n", ret);
+ pm_runtime_put_autosuspend(device->drm.dev);
goto out;
}
device->cleared_on_disable = true;
+ pm_runtime_put_sync_suspend(device->drm.dev);
out:
drm_dev_exit(idx);
}
@@ -1614,6 +1644,13 @@ static void ssd16xx_crtc_atomic_enable(struct drm_crtc *crtc,
if (!drm_dev_enter(&device->drm, &idx))
return;
+ ret = pm_runtime_resume_and_get(device->drm.dev);
+ if (ret < 0) {
+ drm_err(&device->drm, "crtc_atomic_enable: failed to resume: %d\n", ret);
+ drm_dev_exit(idx);
+ return;
+ }
+
drm_dbg(&device->drm, "atomic_enable: %dx%d orientation=%u°\n",
device->width, device->height, device->orientation);
@@ -1621,6 +1658,7 @@ static void ssd16xx_crtc_atomic_enable(struct drm_crtc *crtc,
ret = ssd16xx_hw_init(device);
if (ret) {
drm_err(&device->drm, "atomic_enable: hw_init failed: %d\n", ret);
+ pm_runtime_put_autosuspend(device->drm.dev);
drm_dev_exit(idx);
return;
}
@@ -1645,6 +1683,8 @@ static void ssd16xx_crtc_atomic_enable(struct drm_crtc *crtc,
device->init_refresh_pending = true;
}
+ pm_runtime_mark_last_busy(device->drm.dev);
+ pm_runtime_put_autosuspend(device->drm.dev);
drm_dev_exit(idx);
}
@@ -1744,11 +1784,19 @@ static void ssd16xx_drm_master_drop(struct drm_device *drm,
if (!drm_dev_enter(drm, &idx))
return;
+ ret = pm_runtime_resume_and_get(drm->dev);
+ if (ret < 0) {
+ drm_dev_exit(idx);
+ return;
+ }
+
ret = ssd16xx_clear_display_on_exit(device);
if (ret)
drm_err(drm, "master_drop: clear on close failed: %d\n", ret);
device->cleared_on_close = true;
+ /* sync suspend — bypass autosuspend, sleep immediately after clear */
+ pm_runtime_put_sync_suspend(drm->dev);
drm_dev_exit(idx);
}
@@ -2011,9 +2059,26 @@ static int ssd16xx_probe(struct spi_device *spi)
drm_mode_config_reset(drm);
+ /*
+ * Mark the device active before enabling runtime PM. The SPI device
+ * persists across module reload cycles; pm_runtime_enable() alone
+ * does not clear a stale dev->power.runtime_error from a previous
+ * module load, which would cause pm_runtime_resume_and_get() to
+ * return -EINVAL on every subsequent call.
+ */
+ /* Use level1 (RAM retained) for runtime PM idle. */
+ device->deep_sleep_mode = device->controller_cfg->deep_sleep_mode_level1;
+
+ pm_runtime_set_active(dev);
+ pm_runtime_enable(dev);
+ pm_runtime_set_autosuspend_delay(dev, SSD16XX_PM_AUTOSUSPEND_DELAY_MS);
+ pm_runtime_use_autosuspend(dev);
+
ret = drm_dev_register(drm, 0);
- if (ret)
+ if (ret) {
+ pm_runtime_disable(dev);
return ret;
+ }
drm_dbg(drm, "SSD16xx e-paper display initialized (%dx%d, %d° rotation)\n",
device->width, device->height, device->orientation);
@@ -2029,6 +2094,12 @@ static void ssd16xx_remove(struct spi_device *spi)
drm_dev_unplug(&device->drm);
drm_atomic_helper_shutdown(&device->drm);
+ pm_runtime_dont_use_autosuspend(&spi->dev);
+ /*
+ * Put the panel to sleep regardless of clear_on_disable, which can make
+ * atomic_disable skip touching the hardware.
+ */
+ pm_runtime_force_suspend(&spi->dev);
}
static void ssd16xx_shutdown(struct spi_device *spi)
@@ -2036,8 +2107,139 @@ static void ssd16xx_shutdown(struct spi_device *spi)
struct ssd16xx_device *device = spi_get_drvdata(spi);
drm_atomic_helper_shutdown(&device->drm);
+ pm_runtime_dont_use_autosuspend(&spi->dev);
+ pm_runtime_force_suspend(&spi->dev);
}
+static int ssd16xx_pm_suspend(struct device *dev)
+{
+ struct ssd16xx_device *device = dev_get_drvdata(dev);
+ struct drm_device *drm = &device->drm;
+ int ret;
+
+ /* Use level2 deep sleep for system suspend; SSD1683 loses RAM but saves more power. */
+ device->deep_sleep_mode = device->controller_cfg->deep_sleep_mode_level2;
+ dev_dbg(dev, "system suspend: entering deep sleep mode 0x%02x\n",
+ device->deep_sleep_mode);
+
+ ret = drm_mode_config_helper_suspend(drm);
+ if (ret)
+ return ret;
+
+ /*
+ * If the autosuspend timer already fired the device is RPM_SUSPENDED
+ * at level1. pm_runtime_force_suspend() would skip its callback, so
+ * level2 would never reach the hardware.
+ *
+ * SPI is unresponsive in deep sleep; HWRESET is the only exit.
+ * After the reset, level2 can be sent directly without a full hw_init.
+ *
+ * Skip this for controllers where level1 == level2 (e.g. SSD1677 which
+ * has only one deep sleep mode): the device is already in the deepest
+ * available state and no upgrade is needed.
+ */
+ if (pm_runtime_status_suspended(dev)) {
+ int err = 0;
+
+ if (device->controller_cfg->deep_sleep_mode_level1 !=
+ device->controller_cfg->deep_sleep_mode_level2) {
+ dev_dbg(dev, "system suspend: upgrading level1->level2 via HWRESET\n");
+ ssd16xx_hw_reset(device);
+ ssd16xx_wait_for_device(device, &err);
+ ssd16xx_send_cmd(device, SSD16XX_CMD_DEEP_SLEEP_MODE, &err);
+ ssd16xx_send_data(device,
+ device->controller_cfg->deep_sleep_mode_level2,
+ &err);
+ } else {
+ dev_dbg(dev, "system suspend: already at deepest sleep mode\n");
+ }
+ device->initialized = false;
+ device->pm_force_suspended = false; /* Skip force_resume on resume */
+ return err;
+ }
+
+ /* Force runtime-suspended state for clean resume. */
+ ret = pm_runtime_force_suspend(dev);
+ if (ret) {
+ drm_mode_config_helper_resume(drm);
+ return ret;
+ }
+ device->pm_force_suspended = true; /* Resume must call force_resume */
+
+ return 0;
+}
+
+static int ssd16xx_pm_resume(struct device *dev)
+{
+ struct ssd16xx_device *device = dev_get_drvdata(dev);
+ struct drm_device *drm = &device->drm;
+
+ dev_dbg(dev, "system resume: restoring state\n");
+
+ /* Only force_resume if we force_suspended; the level1→level2 upgrade path did not. */
+ if (device->pm_force_suspended) {
+ pm_runtime_force_resume(dev);
+ device->pm_force_suspended = false;
+ }
+
+ /* Restore level1 for subsequent runtime PM cycles. */
+ device->deep_sleep_mode = device->controller_cfg->deep_sleep_mode_level1;
+
+ return drm_mode_config_helper_resume(drm);
+}
+
+static int ssd16xx_pm_runtime_suspend(struct device *dev)
+{
+ struct ssd16xx_device *device = dev_get_drvdata(dev);
+ int err = 0;
+
+ dev_dbg(dev, "runtime suspend: entering deep sleep mode 0x%02x\n",
+ device->deep_sleep_mode);
+
+ ssd16xx_send_cmd(device, SSD16XX_CMD_DEEP_SLEEP_MODE, &err);
+ ssd16xx_send_data(device, device->deep_sleep_mode, &err);
+ device->initialized = false;
+
+ /*
+ * Don't propagate SPI errors: a failure here would latch runtime_error
+ * and block all future resumes; the next resume resets the panel anyway.
+ */
+ if (err)
+ dev_warn(dev, "failed to enter deep sleep: %d\n", err);
+
+ return 0;
+}
+
+static int ssd16xx_pm_runtime_resume(struct device *dev)
+{
+ struct ssd16xx_device *device = dev_get_drvdata(dev);
+ int ret;
+
+ dev_dbg(dev, "runtime resume: initialized=%d%s\n",
+ device->initialized,
+ !device->initialized ? " (running hw_init)" : "");
+
+ /*
+ * pm_runtime_suspend clears initialized after sending the deep sleep
+ * command. If a new app opens before the suspend fires (the put was
+ * cancelled), initialized is still true and hw_init is unnecessary.
+ */
+ if (!device->initialized) {
+ ret = ssd16xx_hw_init(device);
+ if (ret)
+ return ret;
+
+ device->initialized = true;
+ }
+
+ return 0;
+}
+
+static const struct dev_pm_ops ssd16xx_pm_ops = {
+ SYSTEM_SLEEP_PM_OPS(ssd16xx_pm_suspend, ssd16xx_pm_resume)
+ RUNTIME_PM_OPS(ssd16xx_pm_runtime_suspend, ssd16xx_pm_runtime_resume, NULL)
+};
+
static const struct of_device_id ssd16xx_of_match[] = {
{ .compatible = "gooddisplay,gdey042t81", .data = (void *)GDEY042T81 },
{ }
@@ -2054,6 +2256,7 @@ static struct spi_driver ssd16xx_spi_driver = {
.driver = {
.name = "ssd16xx",
.of_match_table = ssd16xx_of_match,
+ .pm = pm_ptr(&ssd16xx_pm_ops),
},
.probe = ssd16xx_probe,
.remove = ssd16xx_remove,
--
2.39.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v3 07/14] drm/solomon: ssd16xx: Add power management support
2026-10-19 21:37 ` [PATCH v3 07/14] drm/solomon: ssd16xx: Add power management support Devarsh Thakkar
@ 2026-10-19 21:57 ` sashiko-bot
0 siblings, 0 replies; 25+ messages in thread
From: sashiko-bot @ 2026-10-19 21:57 UTC (permalink / raw)
To: Devarsh Thakkar; +Cc: dri-devel, devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] drm/solomon: ssd16xx: missing error rollback in ssd16xx_pm_suspend()
- [Medium] drm/solomon: ssd16xx: missing state restore in ssd16xx_pm_runtime_resume()
- [Medium] drm/solomon: ssd16xx: premature runtime suspend in ssd16xx_probe()
- [Medium] drm/solomon: ssd16xx: incomplete PM error cleanup in ssd16xx_probe()
--
commit 7bb59d973ba345233d1093837dd50809ed285f26
Author: Devarsh Thakkar <devarsht@ti.com>
drm/solomon: ssd16xx: Add power management support
This patch adds runtime PM with a 35-second autosuspend delay and system
suspend/resume support to the SSD16xx e-paper display driver.
> diff --git a/drivers/gpu/drm/solomon/ssd16xx.c b/drivers/gpu/drm/solomon/ssd16xx.c
> index ff0e459082dea..99f11b7ec03e9 100644
> --- a/drivers/gpu/drm/solomon/ssd16xx.c
> +++ b/drivers/gpu/drm/solomon/ssd16xx.c
[ ... ]
> @@ -2011,9 +2059,26 @@ static int ssd16xx_probe(struct spi_device *spi)
>
> drm_mode_config_reset(drm);
>
> + /*
> + * Mark the device active before enabling runtime PM. The SPI device
> + * persists across module reload cycles; pm_runtime_enable() alone
> + * does not clear a stale dev->power.runtime_error from a previous
> + * module load, which would cause pm_runtime_resume_and_get() to
> + * return -EINVAL on every subsequent call.
> + */
> + /* Use level1 (RAM retained) for runtime PM idle. */
> + device->deep_sleep_mode = device->controller_cfg->deep_sleep_mode_level1;
> +
> + pm_runtime_set_active(dev);
> + pm_runtime_enable(dev);
> + pm_runtime_set_autosuspend_delay(dev, SSD16XX_PM_AUTOSUSPEND_DELAY_MS);
> + pm_runtime_use_autosuspend(dev);
[Severity: Medium]
Does calling pm_runtime_set_autosuspend_delay() after pm_runtime_enable()
here cause an immediate synchronous runtime suspend during ssd16xx_probe()?
At the point pm_runtime_set_autosuspend_delay() runs, the device is enabled
with usage_count == 0, dev->power.use_autosuspend is still false, and
dev->power.last_busy has not been marked. Inside the runtime PM core:
pm_runtime_set_autosuspend_delay()
-> update_autosuspend()
-> rpm_idle(dev, RPM_AUTO)
-> rpm_suspend(dev, RPM_AUTO)
-> pm_runtime_autosuspend_expiration()
Because use_autosuspend is false (and last_busy is 0),
pm_runtime_autosuspend_expiration() returns 0, so rpm_suspend() invokes
ssd16xx_pm_runtime_suspend() synchronously and puts the panel into deep
sleep right after ssd16xx_hw_reset(), only for drm_client_setup() to
immediately wake and hardware-reset the panel a second time.
Should pm_runtime_set_autosuspend_delay(), pm_runtime_use_autosuspend(), and
pm_runtime_mark_last_busy() be called before pm_runtime_enable()?
> +
> ret = drm_dev_register(drm, 0);
> - if (ret)
> + if (ret) {
> + pm_runtime_disable(dev);
> return ret;
> + }
[Severity: Medium]
If drm_dev_register() fails in ssd16xx_probe(), is pm_runtime_disable(dev)
alone sufficient to clean up the runtime PM state?
Because pm_runtime_reinit() on unbind does not clear
dev->power.use_autosuspend or run the runtime_suspend callback, only calling
pm_runtime_disable(dev) leaves use_autosuspend enabled on the persistent
spi_device and leaves the panel powered if it is still active. Should this
error path call pm_runtime_dont_use_autosuspend(dev) and
pm_runtime_force_suspend(dev) to match ssd16xx_remove() and
ssd16xx_shutdown()?
>
> drm_dbg(drm, "SSD16xx e-paper display initialized (%dx%d, %d° rotation)\n",
> device->width, device->height, device->orientation);
> @@ -2029,6 +2094,12 @@ static void ssd16xx_remove(struct spi_device *spi)
>
> drm_dev_unplug(&device->drm);
> drm_atomic_helper_shutdown(&device->drm);
> + pm_runtime_dont_use_autosuspend(&spi->dev);
> + /*
> + * Put the panel to sleep regardless of clear_on_disable, which can make
> + * atomic_disable skip touching the hardware.
> + */
> + pm_runtime_force_suspend(&spi->dev);
> }
>
> static void ssd16xx_shutdown(struct spi_device *spi)
> @@ -2036,8 +2107,139 @@ static void ssd16xx_shutdown(struct spi_device *spi)
> struct ssd16xx_device *device = spi_get_drvdata(spi);
>
> drm_atomic_helper_shutdown(&device->drm);
> + pm_runtime_dont_use_autosuspend(&spi->dev);
> + pm_runtime_force_suspend(&spi->dev);
> }
>
> +static int ssd16xx_pm_suspend(struct device *dev)
> +{
> + struct ssd16xx_device *device = dev_get_drvdata(dev);
> + struct drm_device *drm = &device->drm;
> + int ret;
> +
> + /* Use level2 deep sleep for system suspend; SSD1683 loses RAM but saves more power. */
> + device->deep_sleep_mode = device->controller_cfg->deep_sleep_mode_level2;
> + dev_dbg(dev, "system suspend: entering deep sleep mode 0x%02x\n",
> + device->deep_sleep_mode);
> +
> + ret = drm_mode_config_helper_suspend(drm);
> + if (ret)
> + return ret;
> +
> + /*
> + * If the autosuspend timer already fired the device is RPM_SUSPENDED
> + * at level1. pm_runtime_force_suspend() would skip its callback, so
> + * level2 would never reach the hardware.
> + *
> + * SPI is unresponsive in deep sleep; HWRESET is the only exit.
> + * After the reset, level2 can be sent directly without a full hw_init.
> + *
> + * Skip this for controllers where level1 == level2 (e.g. SSD1677 which
> + * has only one deep sleep mode): the device is already in the deepest
> + * available state and no upgrade is needed.
> + */
> + if (pm_runtime_status_suspended(dev)) {
> + int err = 0;
> +
> + if (device->controller_cfg->deep_sleep_mode_level1 !=
> + device->controller_cfg->deep_sleep_mode_level2) {
> + dev_dbg(dev, "system suspend: upgrading level1->level2 via HWRESET\n");
> + ssd16xx_hw_reset(device);
> + ssd16xx_wait_for_device(device, &err);
> + ssd16xx_send_cmd(device, SSD16XX_CMD_DEEP_SLEEP_MODE, &err);
> + ssd16xx_send_data(device,
> + device->controller_cfg->deep_sleep_mode_level2,
> + &err);
> + } else {
> + dev_dbg(dev, "system suspend: already at deepest sleep mode\n");
> + }
> + device->initialized = false;
> + device->pm_force_suspended = false; /* Skip force_resume on resume */
> + return err;
> + }
> +
> + /* Force runtime-suspended state for clean resume. */
> + ret = pm_runtime_force_suspend(dev);
> + if (ret) {
> + drm_mode_config_helper_resume(drm);
> + return ret;
> + }
> + device->pm_force_suspended = true; /* Resume must call force_resume */
> +
> + return 0;
> +}
[Severity: High]
What happens in ssd16xx_pm_suspend() if an error occurs during the
pm_runtime_status_suspended(dev) upgrade path, or if
drm_mode_config_helper_suspend() or pm_runtime_force_suspend() fails?
In the pm_runtime_status_suspended(dev) branch, if ssd16xx_wait_for_device()
times out or ssd16xx_send_cmd()/ssd16xx_send_data() fails,
ssd16xx_pm_suspend() returns err directly without calling
drm_mode_config_helper_resume(drm). Because the suspend callback returns an
error, the PM core aborts system suspend and will not call
ssd16xx_pm_resume(), which leaves the DRM modeset pipeline and in-kernel
clients suspended, leaks drm->mode_config.suspend_state, and leaves the
panel awake after ssd16xx_hw_reset() while runtime PM status remains
RPM_SUSPENDED.
In addition, device->deep_sleep_mode is overwritten with
deep_sleep_mode_level2 at the start of ssd16xx_pm_suspend() and is never
restored to deep_sleep_mode_level1 on any of the three failure paths
(drm_mode_config_helper_suspend() failure, the level1->level2 upgrade
failure, or pm_runtime_force_suspend() failure). Wouldn't that cause all
subsequent runtime PM idle suspends to enter level-2 sleep (0x03) and lose
display RAM?
[ ... ]
> +static int ssd16xx_pm_runtime_resume(struct device *dev)
> +{
> + struct ssd16xx_device *device = dev_get_drvdata(dev);
> + int ret;
> +
> + dev_dbg(dev, "runtime resume: initialized=%d%s\n",
> + device->initialized,
> + !device->initialized ? " (running hw_init)" : "");
> +
> + /*
> + * pm_runtime_suspend clears initialized after sending the deep sleep
> + * command. If a new app opens before the suspend fires (the put was
> + * cancelled), initialized is still true and hw_init is unnecessary.
> + */
> + if (!device->initialized) {
> + ret = ssd16xx_hw_init(device);
> + if (ret)
> + return ret;
> +
> + device->initialized = true;
> + }
> +
> + return 0;
> +}
[Severity: Medium]
When ssd16xx_pm_runtime_resume() runs ssd16xx_hw_init() while the CRTC is
already enabled, does it leave the border waveform register and refresh
state out of sync with the driver state?
ssd16xx_hw_init() issues both ssd16xx_hw_reset() and SSD16XX_CMD_SW_RESET,
which resets SSD16XX_CMD_BORDER_WAVEFORM_CONTROL (0x3C) to its power-on
default without reprogramming it. In ssd16xx_fb_dirty(), register 0x3C is
only written when border_refresh_on_every_update, init_refresh_pending, or
border_waveform_pending is set:
if (device->border_refresh_on_every_update || device->init_refresh_pending ||
device->border_waveform_pending) {
u8 idx = device->border_waveform_update_idx;
u8 border = device->controller_cfg->border_waveform_table[idx];
...
ssd16xx_send_cmd(device, SSD16XX_CMD_BORDER_WAVEFORM_CONTROL, &err);
ssd16xx_send_data(device, border, &err);
device->border_waveform_pending = false;
}
When waking from runtime suspend in ssd16xx_plane_atomic_update(),
ssd16xx_crtc_atomic_enable() does not run, so if
border_refresh_on_every_update is false and the pending flags were already
consumed before autosuspend, register 0x3C is never restored to
border_waveform_update_idx.
Also, on controllers where deep_sleep_mode_level1 is 0x03 (such as SSD1677),
controller RAM is lost during runtime suspend. Without re-arming
init_refresh_pending (or forcing a full refresh), wouldn't the next update
in SSD16XX_REFRESH_PARTIAL mode compare the new BW RAM against lost RED RAM?
Finally, the commit message mentions that level-1 sleep allows fast resume
via SPI wakeup, whereas ssd16xx_pm_runtime_resume() always calls
ssd16xx_hw_init() which performs a full GPIO hardware reset.
[ ... ]
>
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261019213726.4145532-1-devarsht@ti.com?part=7
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v3 08/14] drm/solomon: ssd16xx: Expose refresh mode as plane property
2026-10-19 21:37 [PATCH v3 00/14] Add DRM driver for Solomon SSD16xx e-paper display controllers Devarsh Thakkar
` (6 preceding siblings ...)
2026-10-19 21:37 ` [PATCH v3 07/14] drm/solomon: ssd16xx: Add power management support Devarsh Thakkar
@ 2026-10-19 21:37 ` Devarsh Thakkar
2026-10-19 22:02 ` sashiko-bot
2026-10-19 21:37 ` [PATCH v3 09/14] drm/solomon: ssd16xx: Expose color " Devarsh Thakkar
` (5 subsequent siblings)
13 siblings, 1 reply; 25+ messages in thread
From: Devarsh Thakkar @ 2026-10-19 21:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Nishanth Menon, Vignesh Raghavendra, Tero Kristo
Cc: dri-devel, devicetree, linux-kernel, linux-arm-kernel, devarsht,
LiangCheng Wang
Add a DRM plane property "refresh_mode" allowing userspace to
select between partial, full and fast display update modes at runtime.
E-paper displays support multiple refresh strategies with different
tradeoffs:
- partial: fastest updates (~300-500ms), minimal flicker, ghosting
accumulates over time
- full: cleanest image (~1.5-2s), full black-white cycle eliminates
ghosting, most flicker
- fast: intermediate (~1s), skips LUT reload per frame (loaded once
at init)
The property uses an enum with values matching the internal
ssd16xx_refresh_mode enum. The default is set from the panel
configuration (typically "partial" for interactive use).
Signed-off-by: Devarsh Thakkar <devarsht@ti.com>
---
Changes in v3:
- Use kzalloc_obj() and fix formatting, no functional changes
---
drivers/gpu/drm/solomon/ssd16xx.c | 142 +++++++++++++++++++++++++++++-
1 file changed, 141 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/solomon/ssd16xx.c b/drivers/gpu/drm/solomon/ssd16xx.c
index 99f11b7ec03e..f53c67ae7d31 100644
--- a/drivers/gpu/drm/solomon/ssd16xx.c
+++ b/drivers/gpu/drm/solomon/ssd16xx.c
@@ -397,8 +397,21 @@ struct ssd16xx_device {
u8 *tx_red_buf; /* 1bpp red-channel buffer (3-color panels only) */
u16 *tx_buf9; /* 9-bit SPI expansion buffer (3-wire mode only) */
u8 spi_buf[64] ____cacheline_aligned; /* DMA-safe buffer for SPI transfers */
+
+ struct drm_property *refresh_mode_property;
+};
+
+/* Plane state subclass: refresh_mode lives on the plane, not the connector,
+ * so plane_atomic_update() can diff against its own old/new state.
+ */
+struct ssd16xx_plane_state {
+ struct drm_shadow_plane_state base;
+ enum ssd16xx_refresh_mode refresh_mode; /* Pending refresh mode */
};
+#define to_ssd16xx_plane_state(s) \
+ container_of((s), struct ssd16xx_plane_state, base.base)
+
static inline struct ssd16xx_device *to_ssd16xx_device(struct drm_device *drm)
{
return container_of(drm, struct ssd16xx_device, drm);
@@ -1430,11 +1443,91 @@ static void ssd16xx_plane_destroy(struct drm_plane *plane)
drm_plane_cleanup(plane);
}
+static void ssd16xx_plane_atomic_destroy_state(struct drm_plane *plane,
+ struct drm_plane_state *state);
+
+static struct drm_plane_state *ssd16xx_plane_atomic_create_state(struct drm_plane *plane)
+{
+ struct ssd16xx_device *device = plane_to_ssd16xx_device(plane);
+ struct ssd16xx_plane_state *ssd_state;
+
+ ssd_state = kzalloc_obj(*ssd_state, GFP_KERNEL);
+ if (!ssd_state)
+ return ERR_PTR(-ENOMEM);
+
+ __drm_gem_shadow_plane_state_init(plane, &ssd_state->base);
+ ssd_state->refresh_mode = device->refresh_mode;
+
+ return &ssd_state->base.base;
+}
+
+static struct drm_plane_state *
+ssd16xx_plane_atomic_duplicate_state(struct drm_plane *plane)
+{
+ struct ssd16xx_plane_state *old_ssd_state, *ssd_state;
+
+ if (WARN_ON(!plane->state))
+ return NULL;
+
+ old_ssd_state = to_ssd16xx_plane_state(plane->state);
+ ssd_state = kmemdup(old_ssd_state, sizeof(*ssd_state), GFP_KERNEL);
+ if (!ssd_state)
+ return NULL;
+
+ __drm_gem_duplicate_shadow_plane_state(plane, &ssd_state->base);
+ return &ssd_state->base.base;
+}
+
+static void ssd16xx_plane_atomic_destroy_state(struct drm_plane *plane,
+ struct drm_plane_state *state)
+{
+ struct ssd16xx_plane_state *ssd_state = to_ssd16xx_plane_state(state);
+
+ __drm_gem_destroy_shadow_plane_state(&ssd_state->base);
+ kfree(ssd_state);
+}
+
+static int ssd16xx_plane_atomic_get_property(struct drm_plane *plane,
+ const struct drm_plane_state *state,
+ struct drm_property *property,
+ uint64_t *val)
+{
+ struct ssd16xx_device *device = plane_to_ssd16xx_device(plane);
+ const struct ssd16xx_plane_state *ssd_state = to_ssd16xx_plane_state(state);
+
+ if (property == device->refresh_mode_property) {
+ *val = ssd_state->refresh_mode;
+ return 0;
+ }
+ return -EINVAL;
+}
+
+static int ssd16xx_plane_atomic_set_property(struct drm_plane *plane,
+ struct drm_plane_state *state,
+ struct drm_property *property,
+ uint64_t val)
+{
+ struct ssd16xx_device *device = plane_to_ssd16xx_device(plane);
+ struct ssd16xx_plane_state *ssd_state = to_ssd16xx_plane_state(state);
+
+ if (property == device->refresh_mode_property) {
+ if (val > SSD16XX_REFRESH_FAST)
+ return -EINVAL;
+ ssd_state->refresh_mode = val;
+ return 0;
+ }
+ return -EINVAL;
+}
+
static const struct drm_plane_funcs ssd16xx_plane_funcs = {
.update_plane = drm_atomic_helper_update_plane,
.disable_plane = drm_atomic_helper_disable_plane,
.destroy = ssd16xx_plane_destroy,
- DRM_GEM_SHADOW_PLANE_FUNCS,
+ .atomic_create_state = ssd16xx_plane_atomic_create_state,
+ .atomic_duplicate_state = ssd16xx_plane_atomic_duplicate_state,
+ .atomic_destroy_state = ssd16xx_plane_atomic_destroy_state,
+ .atomic_get_property = ssd16xx_plane_atomic_get_property,
+ .atomic_set_property = ssd16xx_plane_atomic_set_property,
};
static int ssd16xx_plane_atomic_check(struct drm_plane *plane,
@@ -1462,6 +1555,8 @@ static void ssd16xx_plane_atomic_update(struct drm_plane *plane,
struct drm_plane_state *new_state = drm_atomic_get_new_plane_state(state, plane);
struct drm_shadow_plane_state *shadow_state = to_drm_shadow_plane_state(new_state);
struct ssd16xx_device *device = plane_to_ssd16xx_device(plane);
+ struct ssd16xx_plane_state *new_ssd = to_ssd16xx_plane_state(new_state);
+ struct ssd16xx_plane_state *old_ssd = to_ssd16xx_plane_state(old_state);
enum ssd16xx_refresh_mode saved_mode;
u8 saved_border_waveform_idx;
struct drm_framebuffer *fb = new_state->fb;
@@ -1495,6 +1590,25 @@ static void ssd16xx_plane_atomic_update(struct drm_plane *plane,
drm_dbg(&device->drm, "plane_atomic_update: calling fb_dirty rect=(%d,%d)-(%d,%d)\n",
rect.x1, rect.y1, rect.x2, rect.y2);
+
+ /* Diff refresh_mode against the plane's own previous state to see
+ * whether fast LUT preload is needed.
+ */
+ if (old_ssd->refresh_mode != new_ssd->refresh_mode) {
+ /*
+ * Fast refresh omits LOAD_LUT each update; arm the
+ * one-shot flag so LUT is pre-loaded before the first
+ * fast frame.
+ */
+ if (new_ssd->refresh_mode == SSD16XX_REFRESH_FAST &&
+ old_ssd->refresh_mode != SSD16XX_REFRESH_FULL)
+ device->fast_lut_pending = true;
+ else
+ device->fast_lut_pending = false;
+ }
+
+ device->refresh_mode = new_ssd->refresh_mode;
+
/*
* When refresh_mode_init was set, use the specified mode for this first
* frame only, then restore the user-configured refresh_mode so
@@ -1735,6 +1849,29 @@ static const struct drm_connector_helper_funcs ssd16xx_connector_helper_funcs =
.get_modes = ssd16xx_connector_get_modes,
};
+static const struct drm_prop_enum_list ssd16xx_refresh_mode_enum[] = {
+ { SSD16XX_REFRESH_PARTIAL, "partial" },
+ { SSD16XX_REFRESH_FULL, "full" },
+ { SSD16XX_REFRESH_FAST, "fast" },
+};
+
+static int ssd16xx_plane_create_properties(struct ssd16xx_device *device)
+{
+ struct drm_device *drm = &device->drm;
+ struct drm_mode_object *plane_obj = &device->primary_plane.base;
+
+ device->refresh_mode_property =
+ drm_property_create_enum(drm, 0, "refresh_mode",
+ ssd16xx_refresh_mode_enum,
+ ARRAY_SIZE(ssd16xx_refresh_mode_enum));
+ if (!device->refresh_mode_property)
+ return -ENOMEM;
+ drm_object_attach_property(plane_obj,
+ device->refresh_mode_property,
+ device->refresh_mode);
+ return 0;
+}
+
static const struct drm_connector_funcs ssd16xx_connector_funcs = {
.reset = drm_atomic_helper_connector_reset,
.fill_modes = drm_helper_probe_single_connector_modes,
@@ -2057,6 +2194,9 @@ static int ssd16xx_probe(struct spi_device *spi)
if (ret)
return ret;
+ ret = ssd16xx_plane_create_properties(device);
+ if (ret)
+ return ret;
drm_mode_config_reset(drm);
/*
--
2.39.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v3 08/14] drm/solomon: ssd16xx: Expose refresh mode as plane property
2026-10-19 21:37 ` [PATCH v3 08/14] drm/solomon: ssd16xx: Expose refresh mode as plane property Devarsh Thakkar
@ 2026-10-19 22:02 ` sashiko-bot
0 siblings, 0 replies; 25+ messages in thread
From: sashiko-bot @ 2026-10-19 22:02 UTC (permalink / raw)
To: Devarsh Thakkar; +Cc: dri-devel, devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] drm/solomon: ssd16xx: missing helper in ssd16xx_plane_atomic_create_state()
- [High] drm/solomon: ssd16xx: missed fast LUT preload in ssd16xx_plane_atomic_update()
--
commit 5297d32536b98e77de269756d1d61d02dc735b4b
Author: Devarsh Thakkar <devarsht@ti.com>
drm/solomon: ssd16xx: Expose refresh mode as plane property
This commit exposes a "refresh_mode" enum property on the primary plane to
allow userspace to select between partial, full, and fast e-paper display
update modes at runtime. It subclasses drm_shadow_plane_state into
ssd16xx_plane_state and tracks refresh mode transitions in
ssd16xx_plane_atomic_update().
> diff --git a/drivers/gpu/drm/solomon/ssd16xx.c b/drivers/gpu/drm/solomon/ssd16xx.c
> index 99f11b7ec03e9..f53c67ae7d317 100644
> --- a/drivers/gpu/drm/solomon/ssd16xx.c
> +++ b/drivers/gpu/drm/solomon/ssd16xx.c
> @@ -397,8 +397,21 @@ struct ssd16xx_device {
> u8 *tx_red_buf; /* 1bpp red-channel buffer (3-color panels only) */
> u16 *tx_buf9; /* 9-bit SPI expansion buffer (3-wire mode only) */
> u8 spi_buf[64] ____cacheline_aligned; /* DMA-safe buffer for SPI transfers */
> +
> + struct drm_property *refresh_mode_property;
> +};
> +
> +/* Plane state subclass: refresh_mode lives on the plane, not the connector,
> + * so plane_atomic_update() can diff against its own old/new state.
> + */
> +struct ssd16xx_plane_state {
> + struct drm_shadow_plane_state base;
> + enum ssd16xx_refresh_mode refresh_mode; /* Pending refresh mode */
> };
>
> +#define to_ssd16xx_plane_state(s) \
> + container_of((s), struct ssd16xx_plane_state, base.base)
> +
> static inline struct ssd16xx_device *to_ssd16xx_device(struct drm_device *drm)
> {
> return container_of(drm, struct ssd16xx_device, drm);
> @@ -1430,11 +1443,91 @@ static void ssd16xx_plane_destroy(struct drm_plane *plane)
> drm_plane_cleanup(plane);
> }
>
> +static void ssd16xx_plane_atomic_destroy_state(struct drm_plane *plane,
> + struct drm_plane_state *state);
> +
> +static struct drm_plane_state *ssd16xx_plane_atomic_create_state(struct drm_plane *plane)
> +{
> + struct ssd16xx_device *device = plane_to_ssd16xx_device(plane);
> + struct ssd16xx_plane_state *ssd_state;
> +
> + ssd_state = kzalloc_obj(*ssd_state, GFP_KERNEL);
> + if (!ssd_state)
> + return ERR_PTR(-ENOMEM);
> +
> + __drm_gem_shadow_plane_state_init(plane, &ssd_state->base);
> + ssd_state->refresh_mode = device->refresh_mode;
> +
> + return &ssd_state->base.base;
> +}
[Severity: High]
Does this compile when CONFIG_DRM_SSD16XX is enabled?
In drivers/gpu/drm/solomon/ssd16xx.c, ssd16xx_plane_atomic_create_state()
calls __drm_gem_shadow_plane_state_init(), which does not exist in
include/drm/drm_gem_atomic_helper.h or anywhere else in the tree:
include/drm/drm_gem_atomic_helper.h:
void __drm_gem_duplicate_shadow_plane_state(struct drm_plane *plane,
struct drm_shadow_plane_state *new_shadow_plane_state);
void __drm_gem_destroy_shadow_plane_state(struct drm_shadow_plane_state *shadow_plane_state);
void __drm_gem_reset_shadow_plane(struct drm_plane *plane,
struct drm_shadow_plane_state *shadow_plane_state);
Should this implement a plane .reset callback calling
__drm_gem_reset_shadow_plane() instead of .atomic_create_state (and drop the
unused forward declaration of ssd16xx_plane_atomic_destroy_state() above)?
[ ... ]
> @@ -1462,6 +1555,8 @@ static void ssd16xx_plane_atomic_update(struct drm_plane *plane,
> struct drm_plane_state *new_state = drm_atomic_get_new_plane_state(state, plane);
> struct drm_shadow_plane_state *shadow_state = to_drm_shadow_plane_state(new_state);
> struct ssd16xx_device *device = plane_to_ssd16xx_device(plane);
> + struct ssd16xx_plane_state *new_ssd = to_ssd16xx_plane_state(new_state);
> + struct ssd16xx_plane_state *old_ssd = to_ssd16xx_plane_state(old_state);
> enum ssd16xx_refresh_mode saved_mode;
> u8 saved_border_waveform_idx;
> struct drm_framebuffer *fb = new_state->fb;
> @@ -1495,6 +1590,25 @@ static void ssd16xx_plane_atomic_update(struct drm_plane *plane,
>
> drm_dbg(&device->drm, "plane_atomic_update: calling fb_dirty rect=(%d,%d)-(%d,%d)\n",
> rect.x1, rect.y1, rect.x2, rect.y2);
> +
> + /* Diff refresh_mode against the plane's own previous state to see
> + * whether fast LUT preload is needed.
> + */
> + if (old_ssd->refresh_mode != new_ssd->refresh_mode) {
> + /*
> + * Fast refresh omits LOAD_LUT each update; arm the
> + * one-shot flag so LUT is pre-loaded before the first
> + * fast frame.
> + */
> + if (new_ssd->refresh_mode == SSD16XX_REFRESH_FAST &&
> + old_ssd->refresh_mode != SSD16XX_REFRESH_FULL)
> + device->fast_lut_pending = true;
> + else
> + device->fast_lut_pending = false;
> + }
> +
> + device->refresh_mode = new_ssd->refresh_mode;
> +
> /*
> * When refresh_mode_init was set, use the specified mode for this first
> * frame only, then restore the user-configured refresh_mode so
[Severity: High]
Can ssd16xx_plane_atomic_update() in drivers/gpu/drm/solomon/ssd16xx.c miss
preloading the Mode 1 LUT before executing SSD16XX_REFRESH_FAST?
There appear to be three scenarios where the LUT state and plane state get out
of sync:
1. If userspace sets refresh_mode to SSD16XX_REFRESH_FAST while the CRTC is
inactive (where drm_atomic_helper_commit_tail_rpm() passes
DRM_PLANE_COMMIT_ACTIVE_ONLY and skips ssd16xx_plane_atomic_update()), or when
ssd16xx_plane_atomic_update() returns early before this diff check:
drivers/gpu/drm/solomon/ssd16xx.c:ssd16xx_plane_atomic_update() {
...
if (!fb)
goto out_exit;
/* Nothing to draw: don't wake the panel up. */
if (!drm_atomic_helper_damage_merged(old_state, new_state, &rect))
goto out_exit;
...
}
drm_atomic_helper_swap_state() has already updated plane->state to new_state,
while device->refresh_mode and device->fast_lut_pending remain unchanged. On
the next commit that actually draws a frame,
ssd16xx_plane_atomic_duplicate_state() copies plane->state, so both
old_ssd->refresh_mode and new_ssd->refresh_mode are SSD16XX_REFRESH_FAST. As a
result, neither ssd16xx_crtc_atomic_enable() -> ssd16xx_hw_init() (which still
sees the stale device->refresh_mode) nor ssd16xx_plane_atomic_update() (which
sees old_ssd->refresh_mode == new_ssd->refresh_mode) will preload the fast
LUT.
2. In ssd16xx_plane_atomic_update(), pm_runtime_resume_and_get() is called
before device->refresh_mode is updated from new_ssd->refresh_mode. If the
panel was in SSD16XX_REFRESH_FULL mode and entered runtime suspend, waking the
device on a commit that switches from SSD16XX_REFRESH_FULL to
SSD16XX_REFRESH_FAST runs:
pm_runtime_resume_and_get()
-> ssd16xx_pm_runtime_resume()
-> ssd16xx_hw_init()
ssd16xx_hw_init() resets the controller (wiping the LUT) and checks
device->refresh_mode == SSD16XX_REFRESH_FAST, which is still
SSD16XX_REFRESH_FULL, so it skips the LUT preload. Then
ssd16xx_plane_atomic_update() checks old_ssd->refresh_mode !=
SSD16XX_REFRESH_FULL, which is false, and sets device->fast_lut_pending =
false even though the controller was just reset.
3. Right below this hunk in ssd16xx_plane_atomic_update(), when
device->init_refresh_pending is true on the first frame, device->refresh_mode
is temporarily overridden with device->refresh_mode_init, but
device->fast_lut_pending is still consumed and cleared before
ssd16xx_fb_dirty() runs:
drivers/gpu/drm/solomon/ssd16xx.c:ssd16xx_plane_atomic_update() {
...
if (device->init_refresh_pending) {
device->refresh_mode = device->refresh_mode_init;
device->border_waveform_update_idx = device->border_waveform_init_idx;
}
if (device->fast_lut_pending) {
ret = ssd16xx_preload_fast_lut(device);
...
device->fast_lut_pending = false;
}
...
}
If refresh_mode_init is SSD16XX_REFRESH_PARTIAL, ssd16xx_fb_dirty() will load
the Mode 2 partial LUT for that first frame, overwriting the preloaded Mode 1
LUT before subsequent SSD16XX_REFRESH_FAST frames execute.
Would it be more reliable to track what LUT mode is currently loaded in the
hardware (resetting that tracking in ssd16xx_hw_init() and updating it when
ssd16xx_fb_dirty() or ssd16xx_preload_fast_lut() runs), rather than diffing
old_ssd->refresh_mode against new_ssd->refresh_mode?
[ ... ]
> @@ -2057,6 +2194,9 @@ static int ssd16xx_probe(struct spi_device *spi)
> if (ret)
> return ret;
>
> + ret = ssd16xx_plane_create_properties(device);
> + if (ret)
> + return ret;
> drm_mode_config_reset(drm);
>
> /*
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261019213726.4145532-1-devarsht@ti.com?part=8
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v3 09/14] drm/solomon: ssd16xx: Expose color mode as plane property
2026-10-19 21:37 [PATCH v3 00/14] Add DRM driver for Solomon SSD16xx e-paper display controllers Devarsh Thakkar
` (7 preceding siblings ...)
2026-10-19 21:37 ` [PATCH v3 08/14] drm/solomon: ssd16xx: Expose refresh mode as plane property Devarsh Thakkar
@ 2026-10-19 21:37 ` Devarsh Thakkar
2026-10-19 21:52 ` sashiko-bot
2026-10-19 21:37 ` [PATCH v3 10/14] drm/solomon: ssd16xx: Expose session management as plane properties Devarsh Thakkar
` (4 subsequent siblings)
13 siblings, 1 reply; 25+ messages in thread
From: Devarsh Thakkar @ 2026-10-19 21:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Nishanth Menon, Vignesh Raghavendra, Tero Kristo
Cc: dri-devel, devicetree, linux-kernel, linux-arm-kernel, devarsht,
LiangCheng Wang
Add a DRM plane property "color_mode" for panels that support
3-color (black/white/red) display modes.
The property allows runtime selection between:
- bw (black/white): R1 format represents black (0) and white (1)
- 3color: R1 format represents red ink (1) on white background
The property is created on all panels with "black-white" selectable;
panels with red_supported = true also expose the "3-color" option.
The default is set from the panel configuration's default_color_mode
field (typically BW for monochrome panels).
Signed-off-by: Devarsh Thakkar <devarsht@ti.com>
---
Changes in v3:
- Reject 3-color mode together with partial refresh in the plane
atomic_check, as partial refresh does not update the red channel
- Only create the color_mode property on panels with a red plane instead
of exposing a property with a single possible value
---
drivers/gpu/drm/solomon/ssd16xx.c | 51 +++++++++++++++++++++++++++++++
1 file changed, 51 insertions(+)
diff --git a/drivers/gpu/drm/solomon/ssd16xx.c b/drivers/gpu/drm/solomon/ssd16xx.c
index f53c67ae7d31..6037717635a4 100644
--- a/drivers/gpu/drm/solomon/ssd16xx.c
+++ b/drivers/gpu/drm/solomon/ssd16xx.c
@@ -399,6 +399,7 @@ struct ssd16xx_device {
u8 spi_buf[64] ____cacheline_aligned; /* DMA-safe buffer for SPI transfers */
struct drm_property *refresh_mode_property;
+ struct drm_property *color_mode_property;
};
/* Plane state subclass: refresh_mode lives on the plane, not the connector,
@@ -407,6 +408,7 @@ struct ssd16xx_device {
struct ssd16xx_plane_state {
struct drm_shadow_plane_state base;
enum ssd16xx_refresh_mode refresh_mode; /* Pending refresh mode */
+ enum ssd16xx_color_mode color_mode; /* Pending color mode */
};
#define to_ssd16xx_plane_state(s) \
@@ -1457,6 +1459,7 @@ static struct drm_plane_state *ssd16xx_plane_atomic_create_state(struct drm_plan
__drm_gem_shadow_plane_state_init(plane, &ssd_state->base);
ssd_state->refresh_mode = device->refresh_mode;
+ ssd_state->color_mode = device->color_mode;
return &ssd_state->base.base;
}
@@ -1499,6 +1502,10 @@ static int ssd16xx_plane_atomic_get_property(struct drm_plane *plane,
*val = ssd_state->refresh_mode;
return 0;
}
+ if (property == device->color_mode_property) {
+ *val = ssd_state->color_mode;
+ return 0;
+ }
return -EINVAL;
}
@@ -1516,6 +1523,18 @@ static int ssd16xx_plane_atomic_set_property(struct drm_plane *plane,
ssd_state->refresh_mode = val;
return 0;
}
+ if (property == device->color_mode_property) {
+ if (val > SSD16XX_COLOR_MODE_3COLOR)
+ return -EINVAL;
+ if (val == SSD16XX_COLOR_MODE_3COLOR &&
+ !device->device_cfg->red_supported) {
+ drm_dbg(&device->drm,
+ "3-color not supported by this panel\n");
+ return -EINVAL;
+ }
+ ssd_state->color_mode = val;
+ return 0;
+ }
return -EINVAL;
}
@@ -1535,11 +1554,20 @@ static int ssd16xx_plane_atomic_check(struct drm_plane *plane,
{
struct drm_plane_state *new_plane_state =
drm_atomic_get_new_plane_state(state, plane);
+ struct ssd16xx_plane_state *ssd_state =
+ to_ssd16xx_plane_state(new_plane_state);
struct drm_crtc_state *crtc_state;
if (!new_plane_state->crtc)
return 0;
+ if (ssd_state->color_mode == SSD16XX_COLOR_MODE_3COLOR &&
+ ssd_state->refresh_mode == SSD16XX_REFRESH_PARTIAL) {
+ drm_dbg_atomic(plane->dev,
+ "3-color mode does not support partial refresh\n");
+ return -EINVAL;
+ }
+
crtc_state = drm_atomic_get_new_crtc_state(state, new_plane_state->crtc);
return drm_atomic_helper_check_plane_state(new_plane_state, crtc_state,
@@ -1608,6 +1636,7 @@ static void ssd16xx_plane_atomic_update(struct drm_plane *plane,
}
device->refresh_mode = new_ssd->refresh_mode;
+ device->color_mode = new_ssd->color_mode;
/*
* When refresh_mode_init was set, use the specified mode for this first
@@ -1855,6 +1884,11 @@ static const struct drm_prop_enum_list ssd16xx_refresh_mode_enum[] = {
{ SSD16XX_REFRESH_FAST, "fast" },
};
+static const struct drm_prop_enum_list ssd16xx_color_mode_enum[] = {
+ { SSD16XX_COLOR_MODE_BW, "black-white" },
+ { SSD16XX_COLOR_MODE_3COLOR, "3-color" },
+};
+
static int ssd16xx_plane_create_properties(struct ssd16xx_device *device)
{
struct drm_device *drm = &device->drm;
@@ -1869,6 +1903,23 @@ static int ssd16xx_plane_create_properties(struct ssd16xx_device *device)
drm_object_attach_property(plane_obj,
device->refresh_mode_property,
device->refresh_mode);
+
+ /*
+ * Color mode: only meaningful on panels with a physical red plane.
+ * BW-only panels get no property at all rather than a one-value knob.
+ */
+ if (!device->device_cfg->red_supported)
+ return 0;
+
+ device->color_mode_property =
+ drm_property_create_enum(drm, 0, "color_mode",
+ ssd16xx_color_mode_enum,
+ ARRAY_SIZE(ssd16xx_color_mode_enum));
+ if (!device->color_mode_property)
+ return -ENOMEM;
+ drm_object_attach_property(plane_obj,
+ device->color_mode_property,
+ device->color_mode);
return 0;
}
--
2.39.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v3 09/14] drm/solomon: ssd16xx: Expose color mode as plane property
2026-10-19 21:37 ` [PATCH v3 09/14] drm/solomon: ssd16xx: Expose color " Devarsh Thakkar
@ 2026-10-19 21:52 ` sashiko-bot
0 siblings, 0 replies; 25+ messages in thread
From: sashiko-bot @ 2026-10-19 21:52 UTC (permalink / raw)
To: Devarsh Thakkar; +Cc: dri-devel, devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] drm/solomon: ssd16xx: commit message contradicts v3 color_mode behavior
- [Low] drm/solomon: ssd16xx: dead red_supported check in atomic_set_property()
--
commit 52cffe8373e1c5de5f3a1dd257828d014890e4a4
Author: Devarsh Thakkar <devarsht@ti.com>
drm/solomon: ssd16xx: Expose color mode as plane property
This commit exposes a color_mode plane property on ssd16xx panels that
support 3-color display modes, allowing runtime switching between
black-white and 3-color operation while rejecting partial refresh when
3-color mode is selected.
> drm/solomon: ssd16xx: Expose color mode as plane property
>
> Add a DRM plane property "color_mode" for panels that support
> 3-color (black/white/red) display modes.
>
> The property allows runtime selection between:
> - bw (black/white): R1 format represents black (0) and white (1)
> - 3color: R1 format represents red ink (1) on white background
>
> The property is created on all panels with "black-white" selectable;
> panels with red_supported = true also expose the "3-color" option.
> The default is set from the panel configuration's default_color_mode
> field (typically BW for monochrome panels).
>
> Signed-off-by: Devarsh Thakkar <devarsht@ti.com>
[Severity: Medium]
Does the commit message still match the v3 implementation?
In drivers/gpu/drm/solomon/ssd16xx.c, ssd16xx_plane_create_properties()
returns early when !device->device_cfg->red_supported, so monochrome panels do
not get the color_mode property at all, whereas the commit message states that
the property is created on all panels.
In addition, ssd16xx_color_mode_enum defines the enum names as "black-white"
and "3-color", while the list in the commit message refers to them as "bw" and
"3color".
> diff --git a/drivers/gpu/drm/solomon/ssd16xx.c b/drivers/gpu/drm/solomon/ssd16xx.c
> index f53c67ae7d317..6037717635a48 100644
> --- a/drivers/gpu/drm/solomon/ssd16xx.c
> +++ b/drivers/gpu/drm/solomon/ssd16xx.c
[ ... ]
> @@ -1516,6 +1523,18 @@ static int ssd16xx_plane_atomic_set_property(struct drm_plane *plane,
> ssd_state->refresh_mode = val;
> return 0;
> }
> + if (property == device->color_mode_property) {
> + if (val > SSD16XX_COLOR_MODE_3COLOR)
> + return -EINVAL;
> + if (val == SSD16XX_COLOR_MODE_3COLOR &&
> + !device->device_cfg->red_supported) {
> + drm_dbg(&device->drm,
> + "3-color not supported by this panel\n");
> + return -EINVAL;
> + }
> + ssd_state->color_mode = val;
> + return 0;
> + }
> return -EINVAL;
> }
[Severity: Low]
Can this !device->device_cfg->red_supported check in
ssd16xx_plane_atomic_set_property() ever be reached?
In ssd16xx_plane_create_properties(), if !device->device_cfg->red_supported is
true, the function returns early and leaves device->color_mode_property as
NULL:
if (!device->device_cfg->red_supported)
return 0;
device->color_mode_property =
drm_property_create_enum(drm, 0, "color_mode",
ssd16xx_color_mode_enum,
ARRAY_SIZE(ssd16xx_color_mode_enum));
Since drm_atomic_plane_set_property() only invokes
ssd16xx_plane_atomic_set_property() for an attached, non-NULL property,
entering the property == device->color_mode_property branch already guarantees
that device->device_cfg->red_supported is true.
[ ... ]
> @@ -1869,6 +1903,23 @@ static int ssd16xx_plane_create_properties(struct ssd16xx_device *device)
> drm_object_attach_property(plane_obj,
> device->refresh_mode_property,
> device->refresh_mode);
> +
> + /*
> + * Color mode: only meaningful on panels with a physical red plane.
> + * BW-only panels get no property at all rather than a one-value knob.
> + */
> + if (!device->device_cfg->red_supported)
> + return 0;
> +
> + device->color_mode_property =
> + drm_property_create_enum(drm, 0, "color_mode",
> + ssd16xx_color_mode_enum,
> + ARRAY_SIZE(ssd16xx_color_mode_enum));
> + if (!device->color_mode_property)
> + return -ENOMEM;
> + drm_object_attach_property(plane_obj,
> + device->color_mode_property,
> + device->color_mode);
> return 0;
> }
base-commit: d0de3a73194a250a2e5ffbf1dc604c0bcbb16e8a
--
2.52.0
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261019213726.4145532-1-devarsht@ti.com?part=9
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v3 10/14] drm/solomon: ssd16xx: Expose session management as plane properties
2026-10-19 21:37 [PATCH v3 00/14] Add DRM driver for Solomon SSD16xx e-paper display controllers Devarsh Thakkar
` (8 preceding siblings ...)
2026-10-19 21:37 ` [PATCH v3 09/14] drm/solomon: ssd16xx: Expose color " Devarsh Thakkar
@ 2026-10-19 21:37 ` Devarsh Thakkar
2026-10-19 21:58 ` sashiko-bot
2026-10-20 10:32 ` Markus Elfring
2026-10-19 21:37 ` [PATCH v3 11/14] drm/solomon: ssd16xx: support panels whose RAM X order is reversed Devarsh Thakkar
` (3 subsequent siblings)
13 siblings, 2 replies; 25+ messages in thread
From: Devarsh Thakkar @ 2026-10-19 21:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Nishanth Menon, Vignesh Raghavendra, Tero Kristo
Cc: dri-devel, devicetree, linux-kernel, linux-arm-kernel, devarsht,
LiangCheng Wang
Add DRM plane properties for the clear_on_* and refresh_mode_init
session management flags, allowing userspace to control automatic
display clear/refresh behaviour at lifecycle transitions:
- clear_on_init: clear display when a new DRM master opens
- clear_on_close: clear display when the displaying client exits
- clear_on_disable: clear display when CRTC is disabled
- refresh_mode_init: override refresh mode for the first frame
after a new master opens (e.g. force full refresh to clear
ghosting before normal partial updates)
Each property is an enum with values:
disabled / partial / full / fast
These map to the internal representation (-1 for disabled, 0/1/2 for
partial/full/fast refresh modes respectively).
Defaults are seeded from the panel configuration at probe time and
can be adjusted at runtime via standard DRM property interfaces
(e.g. modetest, drm_info, or compositor property APIs).
Signed-off-by: Devarsh Thakkar <devarsht@ti.com>
---
Changes in v3:
- Add a mutex serializing the session state and the panel access between
atomic commits and master_set()/master_drop(). master_drop() can run
concurrently with a nonblocking commit (e.g. on DROP_MASTER) and both
drive the SPI bus with the same transfer buffers
- Sync the session properties from the new plane state in atomic_enable as
well, since it runs before plane_atomic_update and used the values of
the previous commit
---
drivers/gpu/drm/solomon/ssd16xx.c | 209 ++++++++++++++++++++++++++++--
1 file changed, 198 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/solomon/ssd16xx.c b/drivers/gpu/drm/solomon/ssd16xx.c
index 6037717635a4..13bd825bf3f1 100644
--- a/drivers/gpu/drm/solomon/ssd16xx.c
+++ b/drivers/gpu/drm/solomon/ssd16xx.c
@@ -205,6 +205,18 @@ enum ssd16xx_color_mode {
SSD16XX_COLOR_MODE_3COLOR = 1, /* 3-colour BWR; RED RAM used for red pixels */
};
+/*
+ * Clear/refresh-mode-init session property values exposed to userspace.
+ * "disabled" means the action is skipped; the others select which refresh
+ * mode to use for the clear/init operation.
+ */
+enum ssd16xx_session_mode {
+ SSD16XX_SESSION_DISABLED = 0,
+ SSD16XX_SESSION_PARTIAL,
+ SSD16XX_SESSION_FULL,
+ SSD16XX_SESSION_FAST,
+};
+
/* Border waveform enum indices (0-9); mapped to HW bytes via
* controller_cfg->border_waveform_table[]
*/
@@ -373,6 +385,12 @@ struct ssd16xx_device {
bool first_clear_done; /* clear_on_init has already fired once */
bool cleared_on_disable; /* Avoid redundant clear in atomic_disable */
bool cleared_on_close; /* Avoid redundant clear in master_drop */
+
+ /*
+ * Serializes panel access and the session state between atomic commits
+ * and master_set()/master_drop(), which run outside the commit machinery.
+ */
+ struct mutex lock;
bool pm_force_suspended; /* pm_runtime_force_suspend was called in pm_suspend */
int orientation; /* Display orientation in degrees: 0/90/180/270 */
@@ -400,6 +418,10 @@ struct ssd16xx_device {
struct drm_property *refresh_mode_property;
struct drm_property *color_mode_property;
+ struct drm_property *clear_on_init_property;
+ struct drm_property *clear_on_close_property;
+ struct drm_property *clear_on_disable_property;
+ struct drm_property *refresh_mode_init_property;
};
/* Plane state subclass: refresh_mode lives on the plane, not the connector,
@@ -409,6 +431,10 @@ struct ssd16xx_plane_state {
struct drm_shadow_plane_state base;
enum ssd16xx_refresh_mode refresh_mode; /* Pending refresh mode */
enum ssd16xx_color_mode color_mode; /* Pending color mode */
+ int clear_on_init; /* Session: -1=disabled, 0=partial, 1=full, 2=fast */
+ int clear_on_close;
+ int clear_on_disable;
+ int refresh_mode_init;
};
#define to_ssd16xx_plane_state(s) \
@@ -1460,6 +1486,10 @@ static struct drm_plane_state *ssd16xx_plane_atomic_create_state(struct drm_plan
__drm_gem_shadow_plane_state_init(plane, &ssd_state->base);
ssd_state->refresh_mode = device->refresh_mode;
ssd_state->color_mode = device->color_mode;
+ ssd_state->clear_on_init = device->clear_on_init;
+ ssd_state->clear_on_close = device->clear_on_close;
+ ssd_state->clear_on_disable = device->clear_on_disable;
+ ssd_state->refresh_mode_init = device->refresh_mode_init;
return &ssd_state->base.base;
}
@@ -1490,6 +1520,24 @@ static void ssd16xx_plane_atomic_destroy_state(struct drm_plane *plane,
kfree(ssd_state);
}
+/*
+ * Convert internal clear_on / refresh_mode_init value (-1=disabled, 0/1/2=mode)
+ * to the userspace-facing enum (0=disabled, 1=partial, 2=full, 3=fast).
+ */
+static inline unsigned int ssd16xx_internal_to_session_prop(int internal_val)
+{
+ if (internal_val < 0)
+ return SSD16XX_SESSION_DISABLED;
+ return (unsigned int)internal_val + 1;
+}
+
+static inline int ssd16xx_session_prop_to_internal(uint64_t prop_val)
+{
+ if (prop_val == SSD16XX_SESSION_DISABLED)
+ return -1;
+ return (int)prop_val - 1;
+}
+
static int ssd16xx_plane_atomic_get_property(struct drm_plane *plane,
const struct drm_plane_state *state,
struct drm_property *property,
@@ -1506,6 +1554,22 @@ static int ssd16xx_plane_atomic_get_property(struct drm_plane *plane,
*val = ssd_state->color_mode;
return 0;
}
+ if (property == device->clear_on_init_property) {
+ *val = ssd16xx_internal_to_session_prop(ssd_state->clear_on_init);
+ return 0;
+ }
+ if (property == device->clear_on_close_property) {
+ *val = ssd16xx_internal_to_session_prop(ssd_state->clear_on_close);
+ return 0;
+ }
+ if (property == device->clear_on_disable_property) {
+ *val = ssd16xx_internal_to_session_prop(ssd_state->clear_on_disable);
+ return 0;
+ }
+ if (property == device->refresh_mode_init_property) {
+ *val = ssd16xx_internal_to_session_prop(ssd_state->refresh_mode_init);
+ return 0;
+ }
return -EINVAL;
}
@@ -1535,6 +1599,30 @@ static int ssd16xx_plane_atomic_set_property(struct drm_plane *plane,
ssd_state->color_mode = val;
return 0;
}
+ if (property == device->clear_on_init_property) {
+ if (val > SSD16XX_SESSION_FAST)
+ return -EINVAL;
+ ssd_state->clear_on_init = ssd16xx_session_prop_to_internal(val);
+ return 0;
+ }
+ if (property == device->clear_on_close_property) {
+ if (val > SSD16XX_SESSION_FAST)
+ return -EINVAL;
+ ssd_state->clear_on_close = ssd16xx_session_prop_to_internal(val);
+ return 0;
+ }
+ if (property == device->clear_on_disable_property) {
+ if (val > SSD16XX_SESSION_FAST)
+ return -EINVAL;
+ ssd_state->clear_on_disable = ssd16xx_session_prop_to_internal(val);
+ return 0;
+ }
+ if (property == device->refresh_mode_init_property) {
+ if (val > SSD16XX_SESSION_FAST)
+ return -EINVAL;
+ ssd_state->refresh_mode_init = ssd16xx_session_prop_to_internal(val);
+ return 0;
+ }
return -EINVAL;
}
@@ -1576,6 +1664,24 @@ static int ssd16xx_plane_atomic_check(struct drm_plane *plane,
false, false);
}
+/*
+ * Copy the session properties of a plane state into the device. Called from
+ * both crtc_atomic_enable() and plane_atomic_update(): the former runs first
+ * in a commit, so it must not rely on values only synced by the latter.
+ */
+static void ssd16xx_sync_session_props(struct ssd16xx_device *device,
+ const struct ssd16xx_plane_state *ssd_state)
+{
+ lockdep_assert_held(&device->lock);
+
+ device->refresh_mode = ssd_state->refresh_mode;
+ device->color_mode = ssd_state->color_mode;
+ device->clear_on_init = ssd_state->clear_on_init;
+ device->clear_on_close = ssd_state->clear_on_close;
+ device->clear_on_disable = ssd_state->clear_on_disable;
+ device->refresh_mode_init = ssd_state->refresh_mode_init;
+}
+
static void ssd16xx_plane_atomic_update(struct drm_plane *plane,
struct drm_atomic_commit *state)
{
@@ -1619,6 +1725,8 @@ static void ssd16xx_plane_atomic_update(struct drm_plane *plane,
drm_dbg(&device->drm, "plane_atomic_update: calling fb_dirty rect=(%d,%d)-(%d,%d)\n",
rect.x1, rect.y1, rect.x2, rect.y2);
+ mutex_lock(&device->lock);
+
/* Diff refresh_mode against the plane's own previous state to see
* whether fast LUT preload is needed.
*/
@@ -1635,8 +1743,7 @@ static void ssd16xx_plane_atomic_update(struct drm_plane *plane,
device->fast_lut_pending = false;
}
- device->refresh_mode = new_ssd->refresh_mode;
- device->color_mode = new_ssd->color_mode;
+ ssd16xx_sync_session_props(device, new_ssd);
/*
* When refresh_mode_init was set, use the specified mode for this first
@@ -1693,6 +1800,8 @@ static void ssd16xx_plane_atomic_update(struct drm_plane *plane,
device->border_waveform_pending = true;
}
+ mutex_unlock(&device->lock);
+
pm_runtime_mark_last_busy(device->drm.dev);
pm_runtime_put_autosuspend(device->drm.dev);
@@ -1754,6 +1863,8 @@ static void ssd16xx_crtc_atomic_disable(struct drm_crtc *crtc,
if (!drm_dev_enter(&device->drm, &idx))
return;
+ mutex_lock(&device->lock);
+
if (device->clear_on_disable < 0 || device->cleared_on_disable)
goto out;
@@ -1775,6 +1886,7 @@ static void ssd16xx_crtc_atomic_disable(struct drm_crtc *crtc,
device->cleared_on_disable = true;
pm_runtime_put_sync_suspend(device->drm.dev);
out:
+ mutex_unlock(&device->lock);
drm_dev_exit(idx);
}
@@ -1782,6 +1894,8 @@ static void ssd16xx_crtc_atomic_enable(struct drm_crtc *crtc,
struct drm_atomic_commit *state)
{
struct ssd16xx_device *device = crtc_to_ssd16xx_device(crtc);
+ struct drm_plane_state *new_plane_state =
+ drm_atomic_get_new_plane_state(state, &device->primary_plane);
int ret, idx;
if (!drm_dev_enter(&device->drm, &idx))
@@ -1797,16 +1911,28 @@ static void ssd16xx_crtc_atomic_enable(struct drm_crtc *crtc,
drm_dbg(&device->drm, "atomic_enable: %dx%d orientation=%u°\n",
device->width, device->height, device->orientation);
+ /* Serialise hw_init and session-prop setup against master_drop(). */
+ mutex_lock(&device->lock);
+
if (!device->initialized) {
ret = ssd16xx_hw_init(device);
if (ret) {
drm_err(&device->drm, "atomic_enable: hw_init failed: %d\n", ret);
+ mutex_unlock(&device->lock);
pm_runtime_put_autosuspend(device->drm.dev);
drm_dev_exit(idx);
return;
}
device->initialized = true;
}
+
+ /*
+ * atomic_enable runs before plane_atomic_update, so pick up the session
+ * properties of this very commit instead of the previous one's.
+ */
+ if (new_plane_state)
+ ssd16xx_sync_session_props(device, to_ssd16xx_plane_state(new_plane_state));
+
device->cleared_on_disable = false;
/* Clear display on first app launch if configured */
@@ -1826,6 +1952,8 @@ static void ssd16xx_crtc_atomic_enable(struct drm_crtc *crtc,
device->init_refresh_pending = true;
}
+ mutex_unlock(&device->lock);
+
pm_runtime_mark_last_busy(device->drm.dev);
pm_runtime_put_autosuspend(device->drm.dev);
drm_dev_exit(idx);
@@ -1889,6 +2017,13 @@ static const struct drm_prop_enum_list ssd16xx_color_mode_enum[] = {
{ SSD16XX_COLOR_MODE_3COLOR, "3-color" },
};
+static const struct drm_prop_enum_list ssd16xx_session_mode_enum[] = {
+ { SSD16XX_SESSION_DISABLED, "disabled" },
+ { SSD16XX_SESSION_PARTIAL, "partial" },
+ { SSD16XX_SESSION_FULL, "full" },
+ { SSD16XX_SESSION_FAST, "fast" },
+};
+
static int ssd16xx_plane_create_properties(struct ssd16xx_device *device)
{
struct drm_device *drm = &device->drm;
@@ -1908,18 +2043,58 @@ static int ssd16xx_plane_create_properties(struct ssd16xx_device *device)
* Color mode: only meaningful on panels with a physical red plane.
* BW-only panels get no property at all rather than a one-value knob.
*/
- if (!device->device_cfg->red_supported)
- return 0;
+ if (device->device_cfg->red_supported) {
+ device->color_mode_property =
+ drm_property_create_enum(drm, 0, "color_mode",
+ ssd16xx_color_mode_enum,
+ ARRAY_SIZE(ssd16xx_color_mode_enum));
+ if (!device->color_mode_property)
+ return -ENOMEM;
+ drm_object_attach_property(plane_obj,
+ device->color_mode_property,
+ device->color_mode);
+ }
- device->color_mode_property =
- drm_property_create_enum(drm, 0, "color_mode",
- ssd16xx_color_mode_enum,
- ARRAY_SIZE(ssd16xx_color_mode_enum));
- if (!device->color_mode_property)
+ device->clear_on_init_property =
+ drm_property_create_enum(drm, 0, "clear_on_init",
+ ssd16xx_session_mode_enum,
+ ARRAY_SIZE(ssd16xx_session_mode_enum));
+ if (!device->clear_on_init_property)
+ return -ENOMEM;
+ drm_object_attach_property(plane_obj,
+ device->clear_on_init_property,
+ ssd16xx_internal_to_session_prop(device->clear_on_init));
+
+ device->clear_on_close_property =
+ drm_property_create_enum(drm, 0, "clear_on_close",
+ ssd16xx_session_mode_enum,
+ ARRAY_SIZE(ssd16xx_session_mode_enum));
+ if (!device->clear_on_close_property)
+ return -ENOMEM;
+ drm_object_attach_property(plane_obj,
+ device->clear_on_close_property,
+ ssd16xx_internal_to_session_prop(device->clear_on_close));
+
+ device->clear_on_disable_property =
+ drm_property_create_enum(drm, 0, "clear_on_disable",
+ ssd16xx_session_mode_enum,
+ ARRAY_SIZE(ssd16xx_session_mode_enum));
+ if (!device->clear_on_disable_property)
+ return -ENOMEM;
+ drm_object_attach_property(plane_obj,
+ device->clear_on_disable_property,
+ ssd16xx_internal_to_session_prop(device->clear_on_disable));
+
+ device->refresh_mode_init_property =
+ drm_property_create_enum(drm, 0, "refresh_mode_init",
+ ssd16xx_session_mode_enum,
+ ARRAY_SIZE(ssd16xx_session_mode_enum));
+ if (!device->refresh_mode_init_property)
return -ENOMEM;
drm_object_attach_property(plane_obj,
- device->color_mode_property,
- device->color_mode);
+ device->refresh_mode_init_property,
+ ssd16xx_internal_to_session_prop(device->refresh_mode_init));
+
return 0;
}
@@ -1946,6 +2121,8 @@ static void ssd16xx_drm_master_set(struct drm_device *drm,
{
struct ssd16xx_device *device = to_ssd16xx_device(drm);
+ guard(mutex)(&device->lock);
+
device->cleared_on_close = false;
device->first_clear_done = false;
@@ -1963,6 +2140,12 @@ static void ssd16xx_drm_master_drop(struct drm_device *drm,
struct ssd16xx_device *device = to_ssd16xx_device(drm);
int ret, idx;
+ /*
+ * Not serialized against nonblocking commits by the DRM core (e.g. on
+ * DROP_MASTER), so hold the lock across the panel clear too.
+ */
+ guard(mutex)(&device->lock);
+
device->init_refresh_pending = false;
device->first_clear_done = false;
@@ -2074,6 +2257,10 @@ static int ssd16xx_probe(struct spi_device *spi)
return PTR_ERR(device);
drm = &device->drm;
+ ret = drmm_mutex_init(drm, &device->lock);
+ if (ret)
+ return ret;
+
device->spi = spi;
device->model = model;
spi_set_drvdata(spi, device);
--
2.39.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v3 10/14] drm/solomon: ssd16xx: Expose session management as plane properties
2026-10-19 21:37 ` [PATCH v3 10/14] drm/solomon: ssd16xx: Expose session management as plane properties Devarsh Thakkar
@ 2026-10-19 21:58 ` sashiko-bot
2026-10-20 10:32 ` Markus Elfring
1 sibling, 0 replies; 25+ messages in thread
From: sashiko-bot @ 2026-10-19 21:58 UTC (permalink / raw)
To: Devarsh Thakkar; +Cc: dri-devel, devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] drm/solomon: ssd16xx: lock inversion between device->lock and drm_dev_enter()
- [Medium] drm/solomon: ssd16xx: session properties not synced on disable or idle commits
- [Medium] drm/solomon: ssd16xx: disabling refresh_mode_init sets refresh_mode to -1
- [Medium] drm/solomon: ssd16xx: fast LUT preload missed or clobbered by session modes
- [Medium] drm/solomon: ssd16xx: runtime PM calls outside device->lock race with master_drop()
--
commit 96a2ccec634e73c5b82d2f3b3b5d2292c57d3429
Author: Devarsh Thakkar <devarsht@ti.com>
drm/solomon: ssd16xx: Expose session management as plane properties
This commit exposes the clear_on_init, clear_on_close, clear_on_disable, and
refresh_mode_init session management settings as DRM plane enum properties. It
also introduces a device mutex to serialize session state and panel access
between atomic commits and DRM master callbacks.
> diff --git a/drivers/gpu/drm/solomon/ssd16xx.c b/drivers/gpu/drm/solomon/ssd16xx.c
> index 6037717635a48..13bd825bf3f17 100644
> --- a/drivers/gpu/drm/solomon/ssd16xx.c
> +++ b/drivers/gpu/drm/solomon/ssd16xx.c
[ ... ]
> @@ -1576,6 +1664,24 @@ static int ssd16xx_plane_atomic_check(struct drm_plane *plane,
> false, false);
> }
>
> +/*
> + * Copy the session properties of a plane state into the device. Called from
> + * both crtc_atomic_enable() and plane_atomic_update(): the former runs first
> + * in a commit, so it must not rely on values only synced by the latter.
> + */
> +static void ssd16xx_sync_session_props(struct ssd16xx_device *device,
> + const struct ssd16xx_plane_state *ssd_state)
> +{
> + lockdep_assert_held(&device->lock);
> +
> + device->refresh_mode = ssd_state->refresh_mode;
> + device->color_mode = ssd_state->color_mode;
> + device->clear_on_init = ssd_state->clear_on_init;
> + device->clear_on_close = ssd_state->clear_on_close;
> + device->clear_on_disable = ssd_state->clear_on_disable;
> + device->refresh_mode_init = ssd_state->refresh_mode_init;
> +}
> +
> static void ssd16xx_plane_atomic_update(struct drm_plane *plane,
> struct drm_atomic_commit *state)
> {
> @@ -1619,6 +1725,8 @@ static void ssd16xx_plane_atomic_update(struct drm_plane *plane,
> drm_dbg(&device->drm, "plane_atomic_update: calling fb_dirty rect=(%d,%d)-(%d,%d)\n",
> rect.x1, rect.y1, rect.x2, rect.y2);
>
> + mutex_lock(&device->lock);
> +
> /* Diff refresh_mode against the plane's own previous state to see
> * whether fast LUT preload is needed.
> */
> @@ -1635,8 +1743,7 @@ static void ssd16xx_plane_atomic_update(struct drm_plane *plane,
> device->fast_lut_pending = false;
> }
>
> - device->refresh_mode = new_ssd->refresh_mode;
> - device->color_mode = new_ssd->color_mode;
> + ssd16xx_sync_session_props(device, new_ssd);
[Severity: Medium]
Can session property updates be missed when a commit has no framebuffer
damage, detaches the framebuffer, or occurs while the CRTC is inactive?
In ssd16xx_plane_atomic_update(), ssd16xx_sync_session_props() is called
after the early exits for a NULL framebuffer and empty damage:
if (!fb)
goto out_exit;
/* Nothing to draw: don't wake the panel up. */
if (!drm_atomic_helper_damage_merged(old_state, new_state, &rect))
goto out_exit;
In addition, drm_atomic_helper_commit_tail_rpm() passes
DRM_PLANE_COMMIT_ACTIVE_ONLY to drm_atomic_helper_commit_planes(), which
skips ssd16xx_plane_atomic_update() whenever the CRTC is inactive.
If userspace updates clear_on_close or clear_on_disable in a property-only
commit with no damage, while fb is NULL, or while the CRTC is disabled, will
ssd16xx_device retain the stale values when ssd16xx_drm_master_drop() or
ssd16xx_crtc_atomic_disable() later runs?
[Severity: Medium]
Does fast_lut_pending account for refresh_mode_init and the clear_on_*
session modes?
In ssd16xx_plane_atomic_update(), device->fast_lut_pending is computed from
old_ssd->refresh_mode and new_ssd->refresh_mode before device->refresh_mode
is overridden by device->refresh_mode_init:
if (old_ssd->refresh_mode != new_ssd->refresh_mode) {
if (new_ssd->refresh_mode == SSD16XX_REFRESH_FAST &&
old_ssd->refresh_mode != SSD16XX_REFRESH_FULL)
device->fast_lut_pending = true;
else
device->fast_lut_pending = false;
}
ssd16xx_sync_session_props(device, new_ssd);
saved_mode = device->refresh_mode;
saved_border_waveform_idx = device->border_waveform_update_idx;
if (device->init_refresh_pending) {
device->refresh_mode = device->refresh_mode_init;
device->border_waveform_update_idx = device->border_waveform_init_idx;
}
if (device->fast_lut_pending) {
ret = ssd16xx_preload_fast_lut(device);
...
device->fast_lut_pending = false;
}
If new_ssd->refresh_mode is SSD16XX_REFRESH_PARTIAL and refresh_mode_init is
set to fast, fast_lut_pending remains false and the first frame runs a fast
refresh (0xC7) without preloading the fast LUT. Conversely, if
new_ssd->refresh_mode transitions to SSD16XX_REFRESH_FAST while
refresh_mode_init is set to partial, ssd16xx_preload_fast_lut() runs and
clears fast_lut_pending before the init frame, which then runs a partial
refresh (0xFF) that overwrites the LUT in the controller, leaving subsequent
fast frames without the fast LUT.
Similarly, if clear_on_init, clear_on_disable, or clear_on_close is set to
fast, ssd16xx_clear_display() sends SSD1683_CTRL2_FAST_REFRESH without
preloading the fast LUT, and if clear_on_init is set to partial while
refresh_mode is fast, ssd16xx_clear_display_on_init() overwrites the LUT
loaded by ssd16xx_hw_init() without re-arming fast_lut_pending.
>
> /*
> * When refresh_mode_init was set, use the specified mode for this first
> @@ -1693,6 +1800,8 @@ static void ssd16xx_plane_atomic_update(struct drm_plane *plane,
> device->border_waveform_pending = true;
> }
>
> + mutex_unlock(&device->lock);
> +
> pm_runtime_mark_last_busy(device->drm.dev);
> pm_runtime_put_autosuspend(device->drm.dev);
>
> @@ -1754,6 +1863,8 @@ static void ssd16xx_crtc_atomic_disable(struct drm_crtc *crtc,
> if (!drm_dev_enter(&device->drm, &idx))
> return;
>
> + mutex_lock(&device->lock);
> +
> if (device->clear_on_disable < 0 || device->cleared_on_disable)
> goto out;
[Severity: Medium]
Should ssd16xx_crtc_atomic_disable() also sync the session properties from
the commit's new primary plane state before checking
device->clear_on_disable?
Because the driver uses drm_atomic_helper_commit_tail_rpm(),
drm_atomic_helper_commit_modeset_disables() runs
ssd16xx_crtc_atomic_disable() before drm_atomic_helper_commit_planes(), and
drm_atomic_helper_commit_planes() skips disabled CRTCs due to
DRM_PLANE_COMMIT_ACTIVE_ONLY. If userspace sets clear_on_disable or
clear_on_close in the same atomic commit that disables the CRTC,
ssd16xx_crtc_atomic_disable() and ssd16xx_drm_master_drop() will not see the
updated plane state.
>
> @@ -1775,6 +1886,7 @@ static void ssd16xx_crtc_atomic_disable(struct drm_crtc *crtc,
> device->cleared_on_disable = true;
> pm_runtime_put_sync_suspend(device->drm.dev);
> out:
> + mutex_unlock(&device->lock);
> drm_dev_exit(idx);
> }
>
> @@ -1782,6 +1894,8 @@ static void ssd16xx_crtc_atomic_enable(struct drm_crtc *crtc,
> struct drm_atomic_commit *state)
> {
> struct ssd16xx_device *device = crtc_to_ssd16xx_device(crtc);
> + struct drm_plane_state *new_plane_state =
> + drm_atomic_get_new_plane_state(state, &device->primary_plane);
> int ret, idx;
>
> if (!drm_dev_enter(&device->drm, &idx))
> @@ -1797,16 +1911,28 @@ static void ssd16xx_crtc_atomic_enable(struct drm_crtc *crtc,
> drm_dbg(&device->drm, "atomic_enable: %dx%d orientation=%u°\n",
> device->width, device->height, device->orientation);
>
> + /* Serialise hw_init and session-prop setup against master_drop(). */
> + mutex_lock(&device->lock);
> +
> if (!device->initialized) {
> ret = ssd16xx_hw_init(device);
> if (ret) {
> drm_err(&device->drm, "atomic_enable: hw_init failed: %d\n", ret);
> + mutex_unlock(&device->lock);
> pm_runtime_put_autosuspend(device->drm.dev);
> drm_dev_exit(idx);
> return;
> }
> device->initialized = true;
> }
> +
> + /*
> + * atomic_enable runs before plane_atomic_update, so pick up the session
> + * properties of this very commit instead of the previous one's.
> + */
> + if (new_plane_state)
> + ssd16xx_sync_session_props(device, to_ssd16xx_plane_state(new_plane_state));
[Severity: Medium]
Should ssd16xx_sync_session_props() be called before ssd16xx_hw_init() and
pm_runtime_resume_and_get()?
Inside ssd16xx_hw_init(), device->refresh_mode is checked to decide whether
to preload the fast LUT:
if (device->refresh_mode == SSD16XX_REFRESH_FAST) {
...
}
Because ssd16xx_hw_init() runs before ssd16xx_sync_session_props(), it reads
the previous commit's refresh_mode rather than the new plane state's
refresh_mode.
[Severity: Medium]
Additionally, in both ssd16xx_crtc_atomic_enable() and
ssd16xx_plane_atomic_update(), pm_runtime_resume_and_get() is called before
mutex_lock(&device->lock), and pm_runtime_put_autosuspend() is called after
mutex_unlock(&device->lock):
ret = pm_runtime_resume_and_get(device->drm.dev);
...
mutex_lock(&device->lock);
...
mutex_unlock(&device->lock);
pm_runtime_mark_last_busy(device->drm.dev);
pm_runtime_put_autosuspend(device->drm.dev);
When the panel is runtime-suspended, pm_runtime_resume_and_get()
synchronously runs ssd16xx_pm_runtime_resume() -> ssd16xx_hw_init() and sets
device->initialized = true without holding device->lock, racing with
ssd16xx_drm_master_drop(). Furthermore, if a nonblocking commit in
ssd16xx_plane_atomic_update() holds the runtime PM reference while waiting on
mutex_lock(&device->lock), ssd16xx_drm_master_drop()'s call to
pm_runtime_put_sync_suspend() will see usage_count > 0 and fail to suspend
the panel, and once ssd16xx_drm_master_drop() releases device->lock, the
unblocked ssd16xx_plane_atomic_update() will draw over the screen that
clear_on_close just cleared.
> +
> device->cleared_on_disable = false;
>
> /* Clear display on first app launch if configured */
> @@ -1826,6 +1952,8 @@ static void ssd16xx_crtc_atomic_enable(struct drm_crtc *crtc,
> device->init_refresh_pending = true;
> }
[Severity: Medium]
What happens when a client opens the DRM device and then sets the
refresh_mode_init plane property to disabled?
When the device is opened on a panel where refresh_mode_init >= 0 by default
(such as GDEY042T81, where default_refresh_mode_init is
SSD16XX_REFRESH_FULL), ssd16xx_drm_master_set() sets
device->init_refresh_pending = true before the client can change plane
properties:
if (device->refresh_mode_init >= 0)
device->init_refresh_pending = true;
If the client then sets refresh_mode_init to SSD16XX_SESSION_DISABLED (which
maps to internal value -1), ssd16xx_sync_session_props() sets
device->refresh_mode_init = -1, but neither ssd16xx_sync_session_props() nor
ssd16xx_crtc_atomic_enable() clears device->init_refresh_pending when
device->refresh_mode_init < 0.
When ssd16xx_plane_atomic_update() subsequently runs:
if (device->init_refresh_pending) {
device->refresh_mode = device->refresh_mode_init;
device->border_waveform_update_idx = device->border_waveform_init_idx;
}
device->refresh_mode is set to -1, causing ssd16xx_fb_dirty() to fall through
to the default partial refresh case with the init border waveform (and skip
3-color red buffer setup) instead of using the configured refresh_mode.
Conversely, if device->refresh_mode_init was -1 when ssd16xx_drm_master_set()
ran and the client enables refresh_mode_init in a plane update on an
already-active CRTC where ssd16xx_crtc_atomic_enable() does not run,
device->init_refresh_pending is never armed.
>
> + mutex_unlock(&device->lock);
> +
> pm_runtime_mark_last_busy(device->drm.dev);
> pm_runtime_put_autosuspend(device->drm.dev);
> drm_dev_exit(idx);
[ ... ]
> @@ -1963,6 +2140,12 @@ static void ssd16xx_drm_master_drop(struct drm_device *drm,
> struct ssd16xx_device *device = to_ssd16xx_device(drm);
> int ret, idx;
>
> + /*
> + * Not serialized against nonblocking commits by the DRM core (e.g. on
> + * DROP_MASTER), so hold the lock across the panel clear too.
> + */
> + guard(mutex)(&device->lock);
> +
> device->init_refresh_pending = false;
> device->first_clear_done = false;
[Severity: High]
Does acquiring device->lock before drm_dev_enter() in
ssd16xx_drm_master_drop() create a lock inversion with the atomic commit
callbacks?
In ssd16xx_drm_master_drop(), guard(mutex)(&device->lock) acquires
device->lock before calling drm_dev_enter(drm, &idx) (which acquires
drm_unplug_srcu):
guard(mutex)(&device->lock);
...
if (!drm_dev_enter(drm, &idx))
return;
However, ssd16xx_plane_atomic_update(), ssd16xx_crtc_atomic_disable(), and
ssd16xx_crtc_atomic_enable() all call drm_dev_enter(&device->drm, &idx)
before acquiring mutex_lock(&device->lock):
if (!drm_dev_enter(&device->drm, &idx))
return;
mutex_lock(&device->lock);
Will this reverse locking order between device->lock and drm_unplug_srcu
trigger a lockdep circular locking dependency warning when a client commits a
frame and then drops master with clear_on_close enabled?
>
[ ... ]
>
> --
> 2.34.1
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261019213726.4145532-1-devarsht@ti.com?part=10
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v3 10/14] drm/solomon: ssd16xx: Expose session management as plane properties
2026-10-19 21:37 ` [PATCH v3 10/14] drm/solomon: ssd16xx: Expose session management as plane properties Devarsh Thakkar
2026-10-19 21:58 ` sashiko-bot
@ 2026-10-20 10:32 ` Markus Elfring
1 sibling, 0 replies; 25+ messages in thread
From: Markus Elfring @ 2026-10-20 10:32 UTC (permalink / raw)
To: Devarsh Thakkar, dri-devel, devicetree, linux-arm-kernel,
Conor Dooley, David Airlie, Krzysztof Kozlowski,
Maarten Lankhorst, Maxime Ripard, Nishanth Menon, Rob Herring,
Simona Vetter, Tero Kristo, Thomas Zimmermann,
Vignesh Raghavendra
Cc: LKML, LiangCheng Wang
…
> +++ b/drivers/gpu/drm/solomon/ssd16xx.c
…
> @@ -1619,6 +1725,8 @@ static void ssd16xx_plane_atomic_update(struct drm_plane *plane,
> drm_dbg(&device->drm, "plane_atomic_update: calling fb_dirty rect=(%d,%d)-(%d,%d)\n",
> rect.x1, rect.y1, rect.x2, rect.y2);
>
> + mutex_lock(&device->lock);
> +
…
@@ -1693,6 +1800,8 @@ static void ssd16xx_plane_atomic_update(struct drm_plane *plane,
> device->border_waveform_pending = true;
> }
>
> + mutex_unlock(&device->lock);
> +
> pm_runtime_mark_last_busy(device->drm.dev);
…
How do you think about to apply scoped lock guards?
https://elixir.bootlin.com/linux/v7.3/source/include/linux/cleanup.h#L332-L335
Regards,
Markus
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v3 11/14] drm/solomon: ssd16xx: support panels whose RAM X order is reversed
2026-10-19 21:37 [PATCH v3 00/14] Add DRM driver for Solomon SSD16xx e-paper display controllers Devarsh Thakkar
` (9 preceding siblings ...)
2026-10-19 21:37 ` [PATCH v3 10/14] drm/solomon: ssd16xx: Expose session management as plane properties Devarsh Thakkar
@ 2026-10-19 21:37 ` Devarsh Thakkar
2026-10-19 21:37 ` [PATCH v3 12/14] MAINTAINERS: Add entry for Solomon SSD16xx DRM driver Devarsh Thakkar
` (2 subsequent siblings)
13 siblings, 0 replies; 25+ messages in thread
From: Devarsh Thakkar @ 2026-10-19 21:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Nishanth Menon, Vignesh Raghavendra, Tero Kristo
Cc: dri-devel, devicetree, linux-kernel, linux-arm-kernel, devarsht,
LiangCheng Wang
From: LiangCheng Wang <zaq14760@gmail.com>
Some panels have their first column wired to the controller's last source
output, so the controller scans RAM X in the opposite direction and the
image comes out horizontally mirrored. SSD1677 offers no source-direction
control that I could find - command 0x01 only affects the gate side - so it
has to be handled while packing the framebuffer.
Add an x_mirror flag to struct ssd16xx_device_config and honour it in both
branches of ssd16xx_convert_fb_to_1bpp(): the scanline order in the
landscape path, and the inner loop in the portrait one, which is what maps
to RAM X there. The DRM_FORMAT_R1 memcpy fast path cannot mirror while
copying, so it is skipped when the flag is set. Enable it for the
pixpaper-426m entry.
Tested on a Mayqueen PIXPAPER 4.26m (SSD1677, 800x480): a pattern with four
differently sized corner blocks renders correctly at rotation 0, and a DT
rotation of 90 gives a clean clockwise portrait. Doing only the landscape
path leaves the portrait output transposed.
Signed-off-by: LiangCheng Wang <zaq14760@gmail.com>
Signed-off-by: Devarsh Thakkar <devarsht@ti.com>
---
Changes in v3:
- No changes since v2.
---
drivers/gpu/drm/solomon/ssd16xx.c | 22 ++++++++++++++++------
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/solomon/ssd16xx.c b/drivers/gpu/drm/solomon/ssd16xx.c
index 13bd825bf3f1..29dca237378f 100644
--- a/drivers/gpu/drm/solomon/ssd16xx.c
+++ b/drivers/gpu/drm/solomon/ssd16xx.c
@@ -355,6 +355,9 @@ struct ssd16xx_device_config {
const u8 *booster_soft_start_data;
u8 booster_soft_start_len;
+ /* Controller scans this panel's RAM X in the reverse direction. */
+ bool x_mirror;
+
/* Panel-specific display mode (resolution and physical dimensions) */
const struct drm_display_mode *mode;
};
@@ -517,6 +520,7 @@ static const struct ssd16xx_device_config ssd16xx_device_configs[] = {
.default_clear_on_disable = -1,
.default_refresh_mode_init = SSD16XX_REFRESH_FULL,
.red_supported = false, /* 2-colour black/white panel */
+ .x_mirror = false,
.default_color_mode = SSD16XX_COLOR_MODE_BW,
.mode = &gdey042t81_mode,
},
@@ -1161,10 +1165,11 @@ static void ssd16xx_convert_fb_to_3color(u8 *bw_dst, u8 *red_dst,
static void ssd16xx_convert_fb_to_1bpp(u8 *dst, struct iosys_map *src,
struct drm_framebuffer *fb,
struct drm_rect *rect,
- unsigned int orientation)
+ unsigned int orientation,
+ bool x_mirror)
{
u32 format = fb->format->format;
- int x, y;
+ int x, y, i;
u8 byte = 0;
unsigned int bit_pos = 0;
unsigned int dst_idx = 0;
@@ -1186,7 +1191,8 @@ static void ssd16xx_convert_fb_to_1bpp(u8 *dst, struct iosys_map *src,
* if not, the generic pixel-by-pixel loop below handles non-aligned
* rects safely.
*/
- if (format == DRM_FORMAT_R1 && orientation == 0 && rect->x1 % 8 == 0) {
+ if (format == DRM_FORMAT_R1 && orientation == 0 && rect->x1 % 8 == 0 &&
+ !x_mirror) {
unsigned int src_pitch = fb->pitches[0];
unsigned int width_bytes = drm_rect_width(rect) / 8;
@@ -1208,7 +1214,8 @@ static void ssd16xx_convert_fb_to_1bpp(u8 *dst, struct iosys_map *src,
* The data entry mode and cursor position control scan direction.
*/
for (x = rect->x2 - 1; x >= (int)rect->x1; x--) {
- for (y = rect->y1; y < rect->y2; y++) {
+ for (i = 0; i < drm_rect_height(rect); i++) {
+ y = x_mirror ? rect->y2 - 1 - i : rect->y1 + i;
if (ssd16xx_pixel_is_white(src, fb, x, y))
byte |= (1 << (7 - bit_pos));
if (++bit_pos == 8) {
@@ -1234,7 +1241,8 @@ static void ssd16xx_convert_fb_to_1bpp(u8 *dst, struct iosys_map *src,
* The data entry mode and cursor position control scan direction.
*/
for (y = rect->y1; y < rect->y2; y++) {
- for (x = rect->x1; x < rect->x2; x++) {
+ for (i = 0; i < drm_rect_width(rect); i++) {
+ x = x_mirror ? rect->x2 - 1 - i : rect->x1 + i;
if (ssd16xx_pixel_is_white(src, fb, x, y))
byte |= (1 << (7 - bit_pos));
if (++bit_pos == 8) {
@@ -1307,7 +1315,9 @@ static int ssd16xx_fb_dirty(struct drm_framebuffer *fb, struct drm_rect *rect,
if (red_buffer)
ssd16xx_convert_fb_to_3color(mono_buffer, red_buffer, &map, fb, rect);
else
- ssd16xx_convert_fb_to_1bpp(mono_buffer, &map, fb, rect, device->orientation);
+ ssd16xx_convert_fb_to_1bpp(mono_buffer, &map, fb, rect,
+ device->orientation,
+ device->device_cfg->x_mirror);
drm_dbg(&device->drm,
"fb_dirty: mono[0..3]=0x%02x 0x%02x 0x%02x 0x%02x (data_size=%u)\n",
--
2.39.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v3 12/14] MAINTAINERS: Add entry for Solomon SSD16xx DRM driver
2026-10-19 21:37 [PATCH v3 00/14] Add DRM driver for Solomon SSD16xx e-paper display controllers Devarsh Thakkar
` (10 preceding siblings ...)
2026-10-19 21:37 ` [PATCH v3 11/14] drm/solomon: ssd16xx: support panels whose RAM X order is reversed Devarsh Thakkar
@ 2026-10-19 21:37 ` Devarsh Thakkar
2026-10-19 21:37 ` [PATCH v3 13/14] arm64: defconfig: Enable DRM_SSD16XX for AM62L3 EVM Devarsh Thakkar
2026-10-19 21:37 ` [PATCH v3 14/14] arm64: dts: ti: [DO_NOT_MERGE] Add AM62L3 EVM overlay for GDEY042T81 e-paper display Devarsh Thakkar
13 siblings, 0 replies; 25+ messages in thread
From: Devarsh Thakkar @ 2026-10-19 21:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Nishanth Menon, Vignesh Raghavendra, Tero Kristo
Cc: dri-devel, devicetree, linux-kernel, linux-arm-kernel, devarsht,
LiangCheng Wang
Add maintainer entry for the ssd16xx DRM driver covering the SSD16xx
e-paper controller family (SSD1673/SSD1680/SSD1681/SSD1683/SSD1677).
Also list LiangCheng Wang as reviewer for the SSD1677 variant.
Signed-off-by: Devarsh Thakkar <devarsht@ti.com>
---
Changes in v3:
- Follow the rename of the binding file to solomon,ssd1683.yaml
- Add the GDEY042T81 EVM overlay to the file list
- Drop PixPaper reference from the commit message (not in v3)
---
MAINTAINERS | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 4d108339f44e..1b17fba57307 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8566,6 +8566,15 @@ F: Documentation/devicetree/bindings/display/solomon,ssd-common.yaml
F: Documentation/devicetree/bindings/display/solomon,ssd13*.yaml
F: drivers/gpu/drm/solomon/ssd130x*
+DRM DRIVER FOR SOLOMON SSD16XX E-PAPER DISPLAY CONTROLLERS
+M: Devarsh Thakkar <devarsht@ti.com>
+R: LiangCheng Wang <zaq14760@gmail.com>
+S: Maintained
+T: git https://gitlab.freedesktop.org/drm/misc/kernel.git
+F: Documentation/devicetree/bindings/display/solomon,ssd1683.yaml
+F: arch/arm64/boot/dts/ti/k3-am62l3-evm-epaper-gdey042t81.dtso
+F: drivers/gpu/drm/solomon/ssd16xx.c
+
DRM DRIVER FOR ST-ERICSSON MCDE
M: Linus Walleij <linusw@kernel.org>
S: Maintained
--
2.39.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v3 13/14] arm64: defconfig: Enable DRM_SSD16XX for AM62L3 EVM
2026-10-19 21:37 [PATCH v3 00/14] Add DRM driver for Solomon SSD16xx e-paper display controllers Devarsh Thakkar
` (11 preceding siblings ...)
2026-10-19 21:37 ` [PATCH v3 12/14] MAINTAINERS: Add entry for Solomon SSD16xx DRM driver Devarsh Thakkar
@ 2026-10-19 21:37 ` Devarsh Thakkar
2026-10-19 21:37 ` [PATCH v3 14/14] arm64: dts: ti: [DO_NOT_MERGE] Add AM62L3 EVM overlay for GDEY042T81 e-paper display Devarsh Thakkar
13 siblings, 0 replies; 25+ messages in thread
From: Devarsh Thakkar @ 2026-10-19 21:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Nishanth Menon, Vignesh Raghavendra, Tero Kristo
Cc: dri-devel, devicetree, linux-kernel, linux-arm-kernel, devarsht,
LiangCheng Wang
Enable the SSD16xx e-paper DRM driver as a module in the arm64
defconfig for use with the AM62L3 EVM and BeagleBadge-EPD boards.
Signed-off-by: Devarsh Thakkar <devarsht@ti.com>
---
Changes in v3:
- No changes since v2.
---
arch/arm64/configs/defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 6858b75a05f4..f10e295d5aae 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -1050,6 +1050,7 @@ CONFIG_ROCKCHIP_DW_HDMI_QP=y
CONFIG_ROCKCHIP_DW_MIPI_DSI=y
CONFIG_ROCKCHIP_INNO_HDMI=y
CONFIG_ROCKCHIP_LVDS=y
+CONFIG_DRM_SSD16XX=m
CONFIG_DRM_STM=m
CONFIG_DRM_STM_LVDS=m
CONFIG_DRM_SUN4I=m
--
2.39.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v3 14/14] arm64: dts: ti: [DO_NOT_MERGE] Add AM62L3 EVM overlay for GDEY042T81 e-paper display
2026-10-19 21:37 [PATCH v3 00/14] Add DRM driver for Solomon SSD16xx e-paper display controllers Devarsh Thakkar
` (12 preceding siblings ...)
2026-10-19 21:37 ` [PATCH v3 13/14] arm64: defconfig: Enable DRM_SSD16XX for AM62L3 EVM Devarsh Thakkar
@ 2026-10-19 21:37 ` Devarsh Thakkar
2026-10-19 21:59 ` sashiko-bot
13 siblings, 1 reply; 25+ messages in thread
From: Devarsh Thakkar @ 2026-10-19 21:37 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Nishanth Menon, Vignesh Raghavendra, Tero Kristo
Cc: dri-devel, devicetree, linux-kernel, linux-arm-kernel, devarsht,
LiangCheng Wang
Add device tree overlay for the Good Display GDEY042T81 4.2" e-paper
panel connected to the AM62L3 EVM via SPI.
The overlay configures SPI1 with the SSD1683-based GDEY042T81 panel
at 2MHz, with reset, busy, and DC GPIOs connected to Main GPIO0 (&gpio0).
Signed-off-by: Devarsh Thakkar <devarsht@ti.com>
---
arch/arm64/boot/dts/ti/Makefile | 6 ++
.../ti/k3-am62l3-evm-epaper-gdey042t81.dtso | 96 +++++++++++++++++++
2 files changed, 102 insertions(+)
create mode 100644 arch/arm64/boot/dts/ti/k3-am62l3-evm-epaper-gdey042t81.dtso
diff --git a/arch/arm64/boot/dts/ti/Makefile b/arch/arm64/boot/dts/ti/Makefile
index 795978e962ee..90e7a687ca86 100644
--- a/arch/arm64/boot/dts/ti/Makefile
+++ b/arch/arm64/boot/dts/ti/Makefile
@@ -82,6 +82,9 @@ dtb-$(CONFIG_ARCH_K3) += k3-am62d2-evm.dtb
dtb-$(CONFIG_ARCH_K3) += k3-am62l3-beaglebadge.dtb
dtb-$(CONFIG_ARCH_K3) += k3-am62l3-evm.dtb
+# Overlays for AM62Lx EVM
+dtb-$(CONFIG_ARCH_K3) += k3-am62l3-evm-epaper-gdey042t81.dtbo
+
# Boards with AM62Px SoC
dtb-$(CONFIG_ARCH_K3) += k3-am62p5-sk.dtb
dtb-$(CONFIG_ARCH_K3) += k3-am62p5-var-som-symphony.dtb
@@ -319,6 +322,8 @@ k3-am62a7-sk-csi2-ov5640-dtbs := k3-am62a7-sk.dtb \
k3-am62a7-sk-csi2-tevi-ov5640-dtbs := k3-am62a7-sk.dtb \
k3-am62x-sk-csi2-tevi-ov5640.dtbo
k3-am62a7-sk-hdmi-audio-dtbs := k3-am62a7-sk.dtb k3-am62x-sk-hdmi-audio.dtbo
+k3-am62l3-evm-epaper-gdey042t81-dtbs := k3-am62l3-evm.dtb \
+ k3-am62l3-evm-epaper-gdey042t81.dtbo
k3-am62p5-sk-csi2-imx219-dtbs := k3-am62p5-sk.dtb \
k3-am62x-sk-csi2-imx219.dtbo
k3-am62p5-sk-csi2-ov5640-dtbs := k3-am62p5-sk.dtb \
@@ -458,6 +463,7 @@ dtb- += k3-am625-beagleplay-csi2-ov5640.dtb \
k3-am62a7-sk-csi2-imx219.dtb \
k3-am62a7-sk-csi2-ov5640.dtb \
k3-am62a7-sk-hdmi-audio.dtb \
+ k3-am62l3-evm-epaper-gdey042t81.dtb \
k3-am62p5-sk-csi2-imx219.dtb \
k3-am62p5-sk-csi2-ov5640.dtb \
k3-am62p5-sk-csi2-tevi-ov5640.dtb \
diff --git a/arch/arm64/boot/dts/ti/k3-am62l3-evm-epaper-gdey042t81.dtso b/arch/arm64/boot/dts/ti/k3-am62l3-evm-epaper-gdey042t81.dtso
new file mode 100644
index 000000000000..1ff43fb54df3
--- /dev/null
+++ b/arch/arm64/boot/dts/ti/k3-am62l3-evm-epaper-gdey042t81.dtso
@@ -0,0 +1,96 @@
+// SPDX-License-Identifier: GPL-2.0-only OR MIT
+/*
+ * DT Overlay for GoodDisplay GDEY042T81 4.2" E-Paper Display on AM62L3 EVM
+ *
+ * The GDEY042T81 is a 400x300 black-and-white e-paper panel driven by a
+ * Solomon SSD1683 controller, connected via SPI1 on the AM62L3 EVM
+ * expansion header.
+ *
+ * Pin mapping (AM62L3 EVM expansion header):
+ * P5 --> BUSY (GPMC0_CLK.GPIO0_31)
+ * P26 --> RST (GPMC0_AD1.GPIO0_16)
+ * P27 --> CLK (GPMC0_AD5.SPI1_CLK)
+ * P28 --> DC (GPMC0_AD2.GPIO0_17)
+ * P29 --> DAT (GPMC0_AD3.SPI1_D1 / MOSI)
+ * P30 --> CS (GPMC0_AD4.SPI1_CS0)
+ *
+ * The GPMC0_AD[0:5] pins are multiplexed for SPI1 and GPIO use; GPMC0 is
+ * already disabled by default in the SoC DTSI.
+ *
+ * Copyright (C) 2025 Texas Instruments Incorporated - https://www.ti.com/
+ */
+
+/dts-v1/;
+/plugin/;
+
+#include <dt-bindings/gpio/gpio.h>
+#include "k3-pinctrl.h"
+
+&pmx0 {
+ dc_pins_default: epaper-dc-default-pins {
+ pinctrl-single,pins = <
+ AM62LX_IOPAD(0x0080, PIN_OUTPUT, 7) /* (K22) GPMC0_AD2.GPIO0_17 */
+ >;
+ };
+
+ rst_pins_default: epaper-rst-default-pins {
+ pinctrl-single,pins = <
+ AM62LX_IOPAD(0x007c, PIN_OUTPUT, 7) /* (L23) GPMC0_AD1.GPIO0_16 */
+ >;
+ };
+
+ busy_pins_default: epaper-busy-default-pins {
+ pinctrl-single,pins = <
+ AM62LX_IOPAD(0x00b8, PIN_INPUT, 7) /* (L21) GPMC0_CLK.GPIO0_31 */
+ >;
+ };
+
+ main_spi1_pins_default: main-spi1-default-pins {
+ pinctrl-single,pins = <
+ AM62LX_IOPAD(0x008c, PIN_OUTPUT, 4) /* (H22) GPMC0_AD5.SPI1_CLK */
+ AM62LX_IOPAD(0x0088, PIN_OUTPUT, 4) /* (K23) GPMC0_AD4.SPI1_CS0 */
+ AM62LX_IOPAD(0x0084, PIN_OUTPUT, 4) /* (J23) GPMC0_AD3.SPI1_D1 */
+ >;
+ };
+
+ fet_sel_pins_default: epaper-fet-sel-default-pins {
+ pinctrl-single,pins = <
+ AM62LX_IOPAD(0x01a8, PIN_OUTPUT, 7) /* (E13) SPI0_CLK.GPIO0_89 */
+ >;
+ };
+};
+
+/* Drive GPIO0_89 high to select the SPI1 path on the VOUT0 FET switch */
+&gpio0 {
+ fet-sel-hog {
+ gpio-hog;
+ gpios = <89 GPIO_ACTIVE_HIGH>;
+ output-high;
+ line-name = "GPIO_EXP_CONN_FET_SEL";
+ };
+};
+
+&gpmc0 {
+ status = "disabled";
+};
+
+&spi1 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&main_spi1_pins_default>, <&fet_sel_pins_default>;
+ status = "okay";
+
+ gdey042t81: display@0 {
+ compatible = "gooddisplay,gdey042t81", "solomon,ssd1683";
+ reg = <0>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&dc_pins_default>,
+ <&rst_pins_default>,
+ <&busy_pins_default>;
+ spi-max-frequency = <2000000>;
+ reset-gpios = <&gpio0 16 GPIO_ACTIVE_LOW>;
+ dc-gpios = <&gpio0 17 GPIO_ACTIVE_HIGH>;
+ busy-gpios = <&gpio0 31 GPIO_ACTIVE_HIGH>;
+ };
+};
--
2.39.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v3 14/14] arm64: dts: ti: [DO_NOT_MERGE] Add AM62L3 EVM overlay for GDEY042T81 e-paper display
2026-10-19 21:37 ` [PATCH v3 14/14] arm64: dts: ti: [DO_NOT_MERGE] Add AM62L3 EVM overlay for GDEY042T81 e-paper display Devarsh Thakkar
@ 2026-10-19 21:59 ` sashiko-bot
0 siblings, 0 replies; 25+ messages in thread
From: sashiko-bot @ 2026-10-19 21:59 UTC (permalink / raw)
To: Devarsh Thakkar; +Cc: dri-devel, devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] drm/solomon: ssd16xx: broken RAM window in ssd16xx_clear_display() at 90/180 deg
- [Medium] drm/solomon: ssd16xx: missing fast refresh LUT preload on init and resume
- [Medium] drm/solomon: ssd16xx: broken deep_sleep_mode state and error rollback in ssd16xx_pm_suspend()
- [Medium] drm/solomon: ssd16xx: missing RED RAM baseline restore after mode 0x03 sleep
--
commit a52cd0ebb1dc317dd86b82ae1b73fbcb5d5fa3b6
Author: Devarsh Thakkar <devarsht@ti.com>
arm64: dts: ti: [DO_NOT_MERGE] Add AM62L3 EVM overlay for GDEY042T81 e-paper display
This series adds a DRM driver for the Solomon SSD16xx family of e-paper
display controllers (including SSD1683 and SSD1677) with runtime and system
power management, session management, and plane properties, along with the
AM62L3 EVM device tree overlay for the GDEY042T81 panel.
> diff --git a/drivers/gpu/drm/solomon/ssd16xx.c b/drivers/gpu/drm/solomon/ssd16xx.c
> new file mode 100644
> index 0000000000000..29dca237378f8
> --- /dev/null
> +++ b/drivers/gpu/drm/solomon/ssd16xx.c
[ ... ]
> + [SSD1677] = {
> + /*
> + * 10-bit pixel-level X/Y addressing; only deep sleep mode
> + * 0x03 is supported (no RAM-retain mode). Border waveform
> + * encoding matches SSD1683.
> + */
> + .max_width = 960,
> + .max_height = 680,
> + .ram_x_address_bits = 10,
> + .ram_y_address_bits = 10,
> + .has_temp_sensor_ctrl = true,
> + .deep_sleep_mode_level1 = SSD1683_DEEP_SLEEP_MODE_2, /* 0x03 */
> + .deep_sleep_mode_level2 = SSD1683_DEEP_SLEEP_MODE_2, /* 0x03 */
[ ... ]
> + switch (device->orientation) {
> + case 90:
> + case 180:
> + data_entry_mode = SSD16XX_DATA_ENTRY_XDEC_YDEC;
> + break;
> + default: /* 0°/270° */
> + data_entry_mode = SSD16XX_DATA_ENTRY_XINC_YINC;
> + break;
> + }
> +
> + ssd16xx_send_cmd(device, SSD16XX_CMD_DATA_ENTRY_MODE, &err);
> + ssd16xx_send_data(device, data_entry_mode, &err);
> + drm_dbg(&device->drm, "hw_init: orientation=%u° data_entry=0x%02x\n",
> + device->orientation, data_entry_mode);
> +
> + ssd16xx_wait_for_device(device, &err);
> +
> + if (err)
> + drm_err(&device->drm, "Hardware initialization failed: %d\n", err);
> +
> + return err;
> +}
> +
> +/*
> + * Clear display by writing all-white to both BW and RED RAM.
> + * The ctrl2 argument selects the waveform (full/partial/fast refresh).
> + * Border waveform is set to init value before clearing, then restored
> + * to the update value to preserve the border during subsequent updates.
> + */
> +static int ssd16xx_clear_display(struct ssd16xx_device *device, u8 ctrl2)
> +{
> + const u8 *bw_tbl = device->controller_cfg->border_waveform_table;
> + int err = 0;
> + unsigned int data_size = DIV_ROUND_UP(device->width, 8) * device->height;
> + u8 *white_buffer = device->tx_buf;
> + u8 *red_buffer = white_buffer;
> + u16 ram_x_end, ram_y_end;
> +
> + memset(white_buffer, 0xFF, data_size);
> +
> + if (device->controller_cfg->ram_x_address_bits == 8)
> + ram_x_end = (device->width / 8) - 1;
> + else
> + ram_x_end = device->width - 1;
> + ram_y_end = device->height - 1;
> +
> + /* Reset full display RAM window before clearing */
> + ssd16xx_send_cmd(device, SSD16XX_CMD_SET_RAM_X_ADDRESS_START_END, &err);
> + ssd16xx_send_x_param(device, 0x00, &err);
> + ssd16xx_send_x_param(device, ram_x_end, &err);
> +
> + ssd16xx_send_cmd(device, SSD16XX_CMD_SET_RAM_Y_ADDRESS_START_END, &err);
> + ssd16xx_send_y_param(device, 0x00, &err);
> + ssd16xx_send_y_param(device, ram_y_end, &err);
> +
> + ssd16xx_send_cmd(device, SSD16XX_CMD_SET_RAM_X_ADDRESS_COUNTER, &err);
> + ssd16xx_send_x_param(device, 0x00, &err);
> +
> + ssd16xx_send_cmd(device, SSD16XX_CMD_SET_RAM_Y_ADDRESS_COUNTER, &err);
> + ssd16xx_send_y_param(device, 0x00, &err);
[Severity: Medium]
When device->orientation is 90 or 180, ssd16xx_hw_init() sets
SSD16XX_CMD_DATA_ENTRY_MODE to SSD16XX_DATA_ENTRY_XDEC_YDEC, where the
controller decrements X and Y on every byte written.
In ssd16xx_fb_dirty(), 90 and 180 degree orientations program the RAM window
with end-before-start (ram_x_end to ram_x_start, ram_y_end to ram_y_start)
and place the initial address counters at (ram_x_end, ram_y_end).
Here in ssd16xx_clear_display(), the window is unconditionally programmed
from 0x00 to ram_x_end and 0x00 to ram_y_end with the counters starting at
(0x00, 0x00). When invoked via ssd16xx_clear_display_on_init(),
ssd16xx_clear_display_on_exit(), or ssd16xx_crtc_atomic_disable() on a panel
rotated 90 or 180 degrees, won't this fail to clear the display RAM and
leave RED RAM out of sync for subsequent partial updates?
[ ... ]
> + /* Resume lets the runtime_resume callback re-run hw_init if needed. */
> + ret = pm_runtime_resume_and_get(device->drm.dev);
> + if (ret < 0) {
> + drm_err(&device->drm, "plane_atomic_update: failed to resume: %d\n", ret);
> + goto out_exit;
> + }
> +
> + if (!device->initialized) {
> + drm_err(&device->drm, "plane_atomic_update: hw not initialized after resume\n");
> + pm_runtime_put_autosuspend(device->drm.dev);
> + goto out_exit;
> + }
> +
> + drm_dbg(&device->drm, "plane_atomic_update: calling fb_dirty rect=(%d,%d)-(%d,%d)\n",
> + rect.x1, rect.y1, rect.x2, rect.y2);
> +
> + mutex_lock(&device->lock);
> +
> + /* Diff refresh_mode against the plane's own previous state to see
> + * whether fast LUT preload is needed.
> + */
> + if (old_ssd->refresh_mode != new_ssd->refresh_mode) {
> + /*
> + * Fast refresh omits LOAD_LUT each update; arm the
> + * one-shot flag so LUT is pre-loaded before the first
> + * fast frame.
> + */
> + if (new_ssd->refresh_mode == SSD16XX_REFRESH_FAST &&
> + old_ssd->refresh_mode != SSD16XX_REFRESH_FULL)
> + device->fast_lut_pending = true;
> + else
> + device->fast_lut_pending = false;
> + }
> +
> + ssd16xx_sync_session_props(device, new_ssd);
> +
> + /*
> + * When refresh_mode_init was set, use the specified mode for this first
> + * frame only, then restore the user-configured refresh_mode so
> + * subsequent updates continue with the configured mode.
> + */
> + saved_mode = device->refresh_mode;
> + saved_border_waveform_idx = device->border_waveform_update_idx;
> + if (device->init_refresh_pending) {
> + device->refresh_mode = device->refresh_mode_init;
> + device->border_waveform_update_idx = device->border_waveform_init_idx;
> + }
> +
> + /*
> + * Fast refresh (0xC7) omits LOAD_LUT on every update cycle and relies
> + * on the LUT being pre-loaded upfront. The property setter arms
> + * fast_lut_pending whenever the user switches into fast mode. Consume
> + * the flag here (once) before the first fast-refresh frame so the
> + * controller's LUT is in the correct state.
> + */
> + if (device->fast_lut_pending) {
> + ret = ssd16xx_preload_fast_lut(device);
> + if (ret) {
> + drm_err(&device->drm,
> + "plane_atomic_update: fast LUT preload failed: %d\n", ret);
> + }
> +
> + device->fast_lut_pending = false;
> + }
[Severity: Medium]
Can SSD16XX_REFRESH_FAST run without pre-loading the LUT in a few paths
here and in ssd16xx_crtc_atomic_enable()?
First, in ssd16xx_plane_atomic_update(), pm_runtime_resume_and_get() runs
ssd16xx_pm_runtime_resume() -> ssd16xx_hw_init() before
ssd16xx_sync_session_props() updates device->refresh_mode. If the device
resumes from runtime suspend when old_ssd->refresh_mode was
SSD16XX_REFRESH_FULL and new_ssd->refresh_mode is SSD16XX_REFRESH_FAST,
ssd16xx_hw_init() resets the controller (wiping the LUT) and sees the stale
FULL mode, while the check above also skips setting device->fast_lut_pending
because old_ssd->refresh_mode == SSD16XX_REFRESH_FULL.
Second, in ssd16xx_crtc_atomic_enable(), both pm_runtime_resume_and_get()
and ssd16xx_hw_init() run before ssd16xx_sync_session_props(), so
ssd16xx_hw_init() checks the stale device->refresh_mode rather than the new
plane state's mode (and ssd16xx_clear_display_on_init() can also run
SSD16XX_REFRESH_FAST without a pre-loaded LUT).
Third, when device->init_refresh_pending is true, device->refresh_mode is
overridden with device->refresh_mode_init after device->fast_lut_pending is
computed from old_ssd and new_ssd. If refresh_mode_init is
SSD16XX_REFRESH_FAST while new_ssd->refresh_mode is SSD16XX_REFRESH_PARTIAL,
fast_lut_pending is not armed for the init frame. Conversely, if
new_ssd->refresh_mode switches from PARTIAL to FAST while refresh_mode_init
is PARTIAL, fast_lut_pending is consumed before the PARTIAL init frame
(which loads the Mode 2 partial LUT), leaving subsequent FAST frames without
a Mode 1 LUT preload.
[ ... ]
> +static void ssd16xx_crtc_atomic_disable(struct drm_crtc *crtc,
> + struct drm_atomic_commit *state)
> +{
> + struct ssd16xx_device *device = crtc_to_ssd16xx_device(crtc);
> + int ret, idx;
> +
> + if (!drm_dev_enter(&device->drm, &idx))
> + return;
> +
> + mutex_lock(&device->lock);
> +
> + if (device->clear_on_disable < 0 || device->cleared_on_disable)
> + goto out;
> +
> + ret = pm_runtime_resume_and_get(device->drm.dev);
> + if (ret < 0)
> + goto out;
> +
> + drm_dbg(&device->drm, "clear_on_disable: running, mode=%d\n",
> + device->clear_on_disable);
> + ret = ssd16xx_clear_display(device,
> + ssd16xx_refresh_mode_to_ctrl2(device,
> + device->clear_on_disable));
> + if (ret) {
> + drm_err(&device->drm, "atomic_disable: clear failed: %d\n", ret);
> + pm_runtime_put_autosuspend(device->drm.dev);
> + goto out;
> + }
> +
> + device->cleared_on_disable = true;
> + pm_runtime_put_sync_suspend(device->drm.dev);
> +out:
> + mutex_unlock(&device->lock);
> + drm_dev_exit(idx);
> +}
[ ... ]
> +static int ssd16xx_pm_suspend(struct device *dev)
> +{
> + struct ssd16xx_device *device = dev_get_drvdata(dev);
> + struct drm_device *drm = &device->drm;
> + int ret;
> +
> + /* Use level2 deep sleep for system suspend; SSD1683 loses RAM but saves more power. */
> + device->deep_sleep_mode = device->controller_cfg->deep_sleep_mode_level2;
> + dev_dbg(dev, "system suspend: entering deep sleep mode 0x%02x\n",
> + device->deep_sleep_mode);
> +
> + ret = drm_mode_config_helper_suspend(drm);
> + if (ret)
> + return ret;
> +
> + /*
> + * If the autosuspend timer already fired the device is RPM_SUSPENDED
> + * at level1. pm_runtime_force_suspend() would skip its callback, so
> + * level2 would never reach the hardware.
> + *
> + * SPI is unresponsive in deep sleep; HWRESET is the only exit.
> + * After the reset, level2 can be sent directly without a full hw_init.
> + *
> + * Skip this for controllers where level1 == level2 (e.g. SSD1677 which
> + * has only one deep sleep mode): the device is already in the deepest
> + * available state and no upgrade is needed.
> + */
> + if (pm_runtime_status_suspended(dev)) {
> + int err = 0;
> +
> + if (device->controller_cfg->deep_sleep_mode_level1 !=
> + device->controller_cfg->deep_sleep_mode_level2) {
> + dev_dbg(dev, "system suspend: upgrading level1->level2 via HWRESET\n");
> + ssd16xx_hw_reset(device);
> + ssd16xx_wait_for_device(device, &err);
> + ssd16xx_send_cmd(device, SSD16XX_CMD_DEEP_SLEEP_MODE, &err);
> + ssd16xx_send_data(device,
> + device->controller_cfg->deep_sleep_mode_level2,
> + &err);
> + } else {
> + dev_dbg(dev, "system suspend: already at deepest sleep mode\n");
> + }
> + device->initialized = false;
> + device->pm_force_suspended = false; /* Skip force_resume on resume */
> + return err;
> + }
> +
> + /* Force runtime-suspended state for clean resume. */
> + ret = pm_runtime_force_suspend(dev);
> + if (ret) {
> + drm_mode_config_helper_resume(drm);
> + return ret;
> + }
> + device->pm_force_suspended = true; /* Resume must call force_resume */
> +
> + return 0;
> +}
[Severity: Medium]
In ssd16xx_pm_suspend(), device->deep_sleep_mode is updated to
deep_sleep_mode_level2 before calling drm_mode_config_helper_suspend(drm).
Could this cause three issues during system suspend?
1. When clear_on_disable >= 0, drm_mode_config_helper_suspend(drm) calls
ssd16xx_crtc_atomic_disable(), which clears the display and invokes
pm_runtime_put_sync_suspend(). Because device->deep_sleep_mode was already
set to level2, ssd16xx_pm_runtime_suspend() puts the panel into level2 sleep
and marks the device RPM_SUSPENDED. When control returns to
ssd16xx_pm_suspend(), pm_runtime_status_suspended(dev) is true, so it
assumes the panel was in level1 sleep and performs a second hardware reset
and level2 sleep command while skipping pm_runtime_force_suspend(dev).
2. If drm_mode_config_helper_suspend(drm) or pm_runtime_force_suspend(dev)
returns an error, ssd16xx_pm_suspend() exits without restoring
device->deep_sleep_mode to deep_sleep_mode_level1, leaving subsequent
runtime PM autosuspends using level2 sleep (0x03) and wiping controller RAM.
3. In the pm_runtime_status_suspended(dev) branch, if
ssd16xx_wait_for_device(), ssd16xx_send_cmd(), or ssd16xx_send_data() fails,
ssd16xx_pm_suspend() returns err without calling
drm_mode_config_helper_resume(drm) or restoring device->deep_sleep_mode,
leaving the DRM mode config suspended and leaking
drm->mode_config.suspend_state.
> +static int ssd16xx_pm_resume(struct device *dev)
> +{
> + struct ssd16xx_device *device = dev_get_drvdata(dev);
> + struct drm_device *drm = &device->drm;
> +
> + dev_dbg(dev, "system resume: restoring state\n");
> +
> + /* Only force_resume if we force_suspended; the level1→level2 upgrade path did not. */
> + if (device->pm_force_suspended) {
> + pm_runtime_force_resume(dev);
> + device->pm_force_suspended = false;
> + }
> +
> + /* Restore level1 for subsequent runtime PM cycles. */
> + device->deep_sleep_mode = device->controller_cfg->deep_sleep_mode_level1;
> +
> + return drm_mode_config_helper_resume(drm);
> +}
> +
> +static int ssd16xx_pm_runtime_suspend(struct device *dev)
> +{
> + struct ssd16xx_device *device = dev_get_drvdata(dev);
> + int err = 0;
> +
> + dev_dbg(dev, "runtime suspend: entering deep sleep mode 0x%02x\n",
> + device->deep_sleep_mode);
> +
> + ssd16xx_send_cmd(device, SSD16XX_CMD_DEEP_SLEEP_MODE, &err);
> + ssd16xx_send_data(device, device->deep_sleep_mode, &err);
> + device->initialized = false;
> +
> + /*
> + * Don't propagate SPI errors: a failure here would latch runtime_error
> + * and block all future resumes; the next resume resets the panel anyway.
> + */
> + if (err)
> + dev_warn(dev, "failed to enter deep sleep: %d\n", err);
> +
> + return 0;
> +}
> +
> +static int ssd16xx_pm_runtime_resume(struct device *dev)
> +{
> + struct ssd16xx_device *device = dev_get_drvdata(dev);
> + int ret;
> +
> + dev_dbg(dev, "runtime resume: initialized=%d%s\n",
> + device->initialized,
> + !device->initialized ? " (running hw_init)" : "");
> +
> + /*
> + * pm_runtime_suspend clears initialized after sending the deep sleep
> + * command. If a new app opens before the suspend fires (the put was
> + * cancelled), initialized is still true and hw_init is unnecessary.
> + */
> + if (!device->initialized) {
> + ret = ssd16xx_hw_init(device);
> + if (ret)
> + return ret;
> +
> + device->initialized = true;
> + }
> +
> + return 0;
> +}
[Severity: Medium]
When resuming from SSD1683_DEEP_SLEEP_MODE_2 (0x03), the controller loses
its RAM contents, including the RED RAM reference frame required by
SSD16XX_REFRESH_PARTIAL in ssd16xx_fb_dirty().
On SSD1677, deep_sleep_mode_level1 is SSD1683_DEEP_SLEEP_MODE_2 (0x03).
After runtime autosuspend fires while the CRTC remains enabled, the next
plane update calls ssd16xx_plane_atomic_update() ->
pm_runtime_resume_and_get() -> ssd16xx_pm_runtime_resume() ->
ssd16xx_hw_init() without running ssd16xx_crtc_atomic_enable(), so
init_refresh_pending remains false and ssd16xx_fb_dirty() performs a partial
refresh with ctrl1_normal against lost RED RAM.
Similarly, across system suspend and resume (which uses mode 0x03 on SSD1683
as well), if the userspace refresh_mode_init plane property is set to
disabled (-1) or partial (0) while refresh_mode is partial,
ssd16xx_pm_resume() -> drm_mode_config_helper_resume() ->
ssd16xx_crtc_atomic_enable() -> ssd16xx_plane_atomic_update() will also
execute a partial refresh with ctrl1_normal without re-establishing the RED
RAM baseline.
Should resuming from mode 0x03 sleep force a baseline full/fast refresh or
restore RED RAM before the next partial update?
[ ... ]
> --
> 2.34.1
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261019213726.4145532-1-devarsht@ti.com?part=14
^ permalink raw reply [flat|nested] 25+ messages in thread