Thread (20 messages) flat view 20 messages, 4 authors, 12d ago

Re: [PATCH v5 6/6] drm/ssd130x: Add SSD135X_FAMILY and SSD1351 support

From: sashiko-bot@kernel.org
Date: 2026-08-23 11:34:07
Also in: dri-devel

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

New issues:
- [High] DMA on stack memory via regmap_bulk_write in ssd130x_write_cmds.

Pre-existing issues:
- [High] drm_gem_fb_begin_cpu_access sleeps in atomic context.
- [High] regulator_enable sleeps in atomic context.
- [High] Backlight and DRM atomic commit unprotected SPI updates interleave.
--
--- Patch [6]: [PATCH v5 6/6] drm/ssd130x: Add SSD135X_FAMILY and SSD1351 support ---
commit c25fd0a18b10199afa7a08a23e60b6d45a463381
Author: Amit Barzilai [off-list ref]

drm/ssd130x: Add SSD135X_FAMILY and SSD1351 support

This commit adds support for the SSD1351 controller by introducing a new
SSD135X_FAMILY to the ssd130x driver. It adds a new init sequence, custom
command protocols that pass parameters on the data path, and family-specific
plane and encoder callbacks.
quoted hunk ↗ jump to hunk
diff --git a/drivers/gpu/drm/solomon/ssd130x.c b/drivers/gpu/drm/solomon/ssd130x.c
--- a/drivers/gpu/drm/solomon/ssd130x.c
+++ b/drivers/gpu/drm/solomon/ssd130x.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -271,6 +310,17 @@ static int ssd130x_write_cmds(struct ssd130x_device *ssd130x, const u8 *cmd,
 	unsigned int i;
 	int ret;
 
+	if (ssd130x->device_info->cmd_params_are_data) {
+		if (!len)
+			return 0;
+
+		ret = regmap_write(ssd130x->regmap, SSD13XX_COMMAND, cmd[0]);
+		if (ret || len == 1)
+			return ret;
+
+		return ssd130x_write_data(ssd130x, cmd + 1, len - 1);
+	}
[Severity: High]
Will this cause a DMA fault on stack memory? Callers like ssd130x_write_cmd()
allocate the command array on the kernel stack (e.g. u8 buf[8]). Passing
cmd + 1 to ssd130x_write_data() results in a regmap_bulk_write(). On SPI
devices, this passes the buffer directly to the SPI DMA engine. Because
vmalloc-backed kernel stacks forbid DMA, this might cause an immediate
kernel panic or fault during device probe and runtime updates.

[Severity: High]
This is a pre-existing issue, but does this split transaction allow
concurrent updates to interleave on the SPI bus? If a user-space write to
the sysfs backlight interface triggers ssd135x_update_bl() concurrently with
a display update from ssd135x_update_rect(), another thread's regmap_write()
could execute in between this regmap_write() and ssd130x_write_data(). This
could cause the controller to misinterpret parameters as opcodes or vice
versa.

[ ... ]
quoted hunk ↗ jump to hunk
@@ -1247,6 +1431,45 @@ static void ssd133x_primary_plane_atomic_update(struct drm_plane *plane,
 	drm_dev_exit(idx);
 }
 
+static void ssd135x_primary_plane_atomic_update(struct drm_plane *plane,
+						struct drm_atomic_commit *state)
+{
[ ... ]
+	if (!drm_dev_enter(drm, &idx))
+		return;
+
+	if (drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE))
+		goto out_drm_dev_exit;
[Severity: High]
This isn't a bug introduced by this patch, but does this call to
drm_gem_fb_begin_cpu_access() sleep in an atomic context? Plane atomic
update callbacks run in atomic context during non-blocking commit tails.
Since drm_gem_fb_begin_cpu_access() can sleep while waiting on dma-buf
fences or acquiring locks, this might cause kernel warnings or deadlocks
during screen updates.

[ ... ]
quoted hunk ↗ jump to hunk
@@ -1629,6 +1887,31 @@ static void ssd133x_encoder_atomic_enable(struct drm_encoder *encoder,
 	ssd130x_power_off(ssd130x);
 }
 
+static void ssd135x_encoder_atomic_enable(struct drm_encoder *encoder,
+					  struct drm_atomic_commit *state)
+{
+	struct drm_device *drm = encoder->dev;
+	struct ssd130x_device *ssd130x = drm_to_ssd130x(drm);
+	int ret;
+
+	ret = ssd130x_power_on(ssd130x);
[Severity: High]
This is a pre-existing issue, but can this power management sequence sleep
in an atomic context? Encoder atomic enable callbacks must not sleep, yet
ssd130x_power_on() calls regulator_enable() which uses mutexes internally.
Could this lead to deadlocks or crashes when the display is enabled?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260823111959.17029-1-amit.barzilai22@gmail.com?part=6
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help