dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RESEND 0/2] drm/tiny: sharp-memory: fix TX buffer corruption on partial update
@ 2026-09-01 11:53 Tobias Johansson
  2026-09-01 11:53 ` [PATCH RESEND 1/2] drm/tiny: sharp-memory: fix line address assignment " Tobias Johansson
  2026-09-01 11:53 ` [PATCH RESEND 2/2] drm/tiny: sharp-memory: avoid transmitting stale TX buffer data Tobias Johansson
  0 siblings, 2 replies; 5+ messages in thread
From: Tobias Johansson @ 2026-09-01 11:53 UTC (permalink / raw)
  To: Alex Lanzano, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Simona Vetter, Dmitry Baryshkov, Mehdi Djait,
	Uwe Kleine-König
  Cc: dri-devel, linux-kernel, Tobias Johansson, kernel

Users running applications that submit partial framebuffer updates
(such as LVGL with dirty-region tracking) can observe persistent
flickering on the display when using animations.

The flickering is caused by two bugs that corrupt the TX buffer on
partial updates. The first patch fixes incorrect line address
assignment in sharp_memory_set_tx_buffer_addresses(). The second
patch fixes stale data transmission in
sharp_memory_update_display(). Both patches are needed to fully
eliminate the flickering.

Signed-off-by: Tobias Johansson <tobias.johansson@axis.com>
---
Tobias Johansson (2):
      drm/tiny: sharp-memory: fix line address assignment on partial update
      drm/tiny: sharp-memory: avoid transmitting stale TX buffer data

 drivers/gpu/drm/tiny/sharp-memory.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
---
base-commit: ba683f774299d89d17cde03bb1bdb13f3513cd20
change-id: 20260316-flickering-f9df09243d2f

Best regards,
-- 
Tobias Johansson <tobias.johansson@axis.com>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH RESEND 1/2] drm/tiny: sharp-memory: fix line address assignment on partial update
  2026-09-01 11:53 [PATCH RESEND 0/2] drm/tiny: sharp-memory: fix TX buffer corruption on partial update Tobias Johansson
@ 2026-09-01 11:53 ` Tobias Johansson
  2026-09-01 12:10   ` sashiko-bot
  2026-09-01 11:53 ` [PATCH RESEND 2/2] drm/tiny: sharp-memory: avoid transmitting stale TX buffer data Tobias Johansson
  1 sibling, 1 reply; 5+ messages in thread
From: Tobias Johansson @ 2026-09-01 11:53 UTC (permalink / raw)
  To: Alex Lanzano, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Simona Vetter, Dmitry Baryshkov, Mehdi Djait,
	Uwe Kleine-König
  Cc: dri-devel, linux-kernel, Tobias Johansson, kernel

When only a subset of lines is dirty, the TX buffer sent to the
panel contains incorrect line addresses, resulting in visible
flickering on the display.

sharp_memory_set_tx_buffer_addresses() iterates from line 0 to the
last damaged line, assigning addresses sequentially from 1. When
only lines 10-20 are dirty, line 10's pixel data is written to the
slot with address 1 instead of address 11, corrupting the address-
to-data mapping.

Fix sharp_memory_set_tx_buffer_addresses() to iterate over only the
damaged line count and offset assigned addresses by the clip start,
so that addresses match the pixel data that follows.

Fixes: b8f9f21716fec ("drm/tiny: Add driver for Sharp Memory LCD")
Signed-off-by: Tobias Johansson <tobias.johansson@axis.com>
---
 drivers/gpu/drm/tiny/sharp-memory.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/tiny/sharp-memory.c b/drivers/gpu/drm/tiny/sharp-memory.c
index cbf69460ebf3..595926ed660e 100644
--- a/drivers/gpu/drm/tiny/sharp-memory.c
+++ b/drivers/gpu/drm/tiny/sharp-memory.c
@@ -120,8 +120,8 @@ static inline void sharp_memory_set_tx_buffer_addresses(u8 *buffer,
 							struct drm_rect clip,
 							u32 pitch)
 {
-	for (u32 line = 0; line < clip.y2; ++line)
-		buffer[line * pitch] = line + 1;
+	for (u32 line = 0; line < drm_rect_height(&clip); ++line)
+		buffer[line * pitch] = clip.y1 + line + 1;
 }
 
 static void sharp_memory_set_tx_buffer_data(u8 *buffer,

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH RESEND 2/2] drm/tiny: sharp-memory: avoid transmitting stale TX buffer data
  2026-09-01 11:53 [PATCH RESEND 0/2] drm/tiny: sharp-memory: fix TX buffer corruption on partial update Tobias Johansson
  2026-09-01 11:53 ` [PATCH RESEND 1/2] drm/tiny: sharp-memory: fix line address assignment " Tobias Johansson
@ 2026-09-01 11:53 ` Tobias Johansson
  2026-09-01 12:08   ` sashiko-bot
  1 sibling, 1 reply; 5+ messages in thread
From: Tobias Johansson @ 2026-09-01 11:53 UTC (permalink / raw)
  To: Alex Lanzano, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Simona Vetter, Dmitry Baryshkov, Mehdi Djait,
	Uwe Kleine-König
  Cc: dri-devel, linux-kernel, Tobias Johansson, kernel

When only a subset of lines is dirty, the TX buffer sent to the
panel contains stale line data from previous updates, resulting
in visible flickering on the display.

sharp_memory_update_display() transmits the entire TX buffer
regardless of how many lines were updated. Entries written by a
previous larger update linger in the buffer and
are retransmitted on every subsequent smaller update, overwriting
the newly written data with stale content.

Fix sharp_memory_update_display() to transmit only the buffer
entries populated by the current update.

Fixes: b8f9f21716fec ("drm/tiny: Add driver for Sharp Memory LCD")
Signed-off-by: Tobias Johansson <tobias.johansson@axis.com>
---
 drivers/gpu/drm/tiny/sharp-memory.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/tiny/sharp-memory.c b/drivers/gpu/drm/tiny/sharp-memory.c
index 595926ed660e..e7521fc6a010 100644
--- a/drivers/gpu/drm/tiny/sharp-memory.c
+++ b/drivers/gpu/drm/tiny/sharp-memory.c
@@ -155,7 +155,7 @@ static int sharp_memory_update_display(struct sharp_memory_device *smd,
 	u32 pitch = smd->pitch;
 	u8 vcom = smd->vcom;
 	u8 *tx_buffer = smd->tx_buffer;
-	u32 tx_buffer_size = smd->tx_buffer_size;
+	u32 tx_len = 1 + (drm_rect_height(&clip) * pitch);
 
 	mutex_lock(&smd->tx_mutex);
 
@@ -165,7 +165,7 @@ static int sharp_memory_update_display(struct sharp_memory_device *smd,
 	sharp_memory_set_tx_buffer_addresses(&tx_buffer[1], clip, pitch);
 	sharp_memory_set_tx_buffer_data(&tx_buffer[2], fb, vmap, clip, pitch, fmtcnv_state);
 
-	ret = sharp_memory_spi_write(smd->spi, tx_buffer, tx_buffer_size);
+	ret = sharp_memory_spi_write(smd->spi, tx_buffer, tx_len);
 
 	mutex_unlock(&smd->tx_mutex);
 

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH RESEND 2/2] drm/tiny: sharp-memory: avoid transmitting stale TX buffer data
  2026-09-01 11:53 ` [PATCH RESEND 2/2] drm/tiny: sharp-memory: avoid transmitting stale TX buffer data Tobias Johansson
@ 2026-09-01 12:08   ` sashiko-bot
  0 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-09-01 12:08 UTC (permalink / raw)
  To: Tobias Johansson; +Cc: dri-devel

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

Pre-existing issues:
- [High] Missing error check for kthread_run() and failure to stop kthread on probe error paths leads to crashes and Use-After-Free.
--

commit 2cf47adb5e86fa285b44ad0bec11eb5287e32f73
Author: Tobias Johansson <tobias.johansson@axis.com>

drm/tiny: sharp-memory: avoid transmitting stale TX buffer data

When only a subset of lines is dirty, the TX buffer sent to the panel contains
stale line data from previous updates, resulting in visible flickering. This
commit fixes sharp_memory_update_display() to transmit only the buffer
entries populated by the current update.

> diff --git a/drivers/gpu/drm/tiny/sharp-memory.c b/drivers/gpu/drm/tiny/sharp-memory.c
> index 595926ed660e5..e7521fc6a0104 100644
> --- a/drivers/gpu/drm/tiny/sharp-memory.c
> +++ b/drivers/gpu/drm/tiny/sharp-memory.c
[ ... ]

This isn't a bug introduced by this patch, but I noticed a pre-existing
issue in sharp_memory_probe():

drivers/gpu/drm/tiny/sharp-memory.c:sharp_memory_probe() {
    ...
		smd->sw_vcom_signal = kthread_run(sharp_memory_sw_vcom_signal_thread,
						  smd, "sw_vcom_signal");
    ...
	ret = drm_dev_register(drm, 0);
	if (ret)
		return dev_err_probe(dev, ret, "Failed to register drm device.\n");
    ...
}

[Severity: High]
Can this lead to a use-after-free if the probe function fails?

If drm_dev_register() fails, the function returns an error without stopping
the running kthread. Devres automatically frees the smd structure, but the
kthread continues executing and may access freed memory.

Additionally, if kthread_run() fails, an ERR_PTR is stored. Will this ERR_PTR
be dereferenced by kthread_stop() during module removal?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901-flickering-v1-0-eaf7bbf9c000@axis.com?part=2

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH RESEND 1/2] drm/tiny: sharp-memory: fix line address assignment on partial update
  2026-09-01 11:53 ` [PATCH RESEND 1/2] drm/tiny: sharp-memory: fix line address assignment " Tobias Johansson
@ 2026-09-01 12:10   ` sashiko-bot
  0 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-09-01 12:10 UTC (permalink / raw)
  To: Tobias Johansson; +Cc: dri-devel

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

Pre-existing issues:
- [Critical] Background kthread is not stopped on probe error paths, leading to a critical Use-After-Free.
- [High] Calling sleeping functions from a plane atomic_update callback violates DRM atomic context rules.
- [High] Line addresses are truncated to 8 bits, breaking hardware support for panels with more than 255 lines.
- [High] Missing IS_ERR check for kthread_run leads to a kernel panic during device removal.
- [High] Truncation in pitch calculation for non-byte-aligned widths causes missing hardware dummy bits.
--

commit c42f51bc1dfa5c55a57f8a64e3aa97d5ec5bc008
Author: Tobias Johansson <tobias.johansson@axis.com>
Subject: drm/tiny: sharp-memory: fix line address assignment on partial update

This patch modifies sharp_memory_set_tx_buffer_addresses() to only iterate
over the damaged line count and offset the assigned addresses by the clip
start. This fixes corrupted address-to-data mapping on partial updates where
incorrect line addresses were being sent to the panel.

> diff --git a/drivers/gpu/drm/tiny/sharp-memory.c b/drivers/gpu/drm/tiny/sharp-memory.c
> index cbf69460ebf32..595926ed660e5 100644
> --- a/drivers/gpu/drm/tiny/sharp-memory.c
> +++ b/drivers/gpu/drm/tiny/sharp-memory.c
> @@ -120,8 +120,8 @@ static inline void sharp_memory_set_tx_buffer_addresses(u8 *buffer,
>  							struct drm_rect clip,
>  							u32 pitch)
>  {
> -	for (u32 line = 0; line < clip.y2; ++line)
> -		buffer[line * pitch] = line + 1;
> +	for (u32 line = 0; line < drm_rect_height(&clip); ++line)
> +		buffer[line * pitch] = clip.y1 + line + 1;

[Severity: High]
This isn't a bug introduced by this patch, but does the computed line address
get truncated?

Since buffer is an 8-bit u8 array and SHARP_ADDR_PERIOD is hardcoded to
8 bits, addresses > 255 might overflow and be truncated. Could this break
hardware support for panels with more than 255 lines, like ls032b7dd02 which
can have up to 536 lines?

>  }

I also noticed some other pre-existing issues in this file:

[Severity: High]
This is a pre-existing issue, but does this violate DRM atomic context rules?

drivers/gpu/drm/tiny/sharp-memory.c:sharp_memory_plane_atomic_update() {
    ...
	if (drm_atomic_helper_damage_merged(old_state, plane_state, &rect))
		sharp_memory_fb_dirty(plane_state->fb, shadow_plane_state->data,
				      &rect, &shadow_plane_state->fmtcnv_state);
    ...
}

sharp_memory_plane_atomic_update() executes in an atomic context where
sleeping is forbidden. However, it delegates to
sharp_memory_update_display() which acquires a mutex:

drivers/gpu/drm/tiny/sharp-memory.c:sharp_memory_update_display() {
    ...
	mutex_lock(&smd->tx_mutex);
    ...
}

Additionally, it calls drm_fb_xrgb8888_to_mono() which allocates memory with
GFP_KERNEL:

drivers/gpu/drm/drm_format_helper.c:drm_fb_xrgb8888_to_mono() {
    ...
	src32 = drm_format_conv_state_reserve(state, len_src32 + linepixels,
					      GFP_KERNEL);
    ...
}

Could this lead to system instability, kernel warnings, and potential deadlocks
during non-blocking atomic commits?


[Severity: Critical]
This is a pre-existing issue, but is there a use-after-free on the probe error
path if initialization fails?

drivers/gpu/drm/tiny/sharp-memory.c:sharp_memory_probe() {
    ...
	smd->sw_vcom_signal = kthread_run(sharp_memory_sw_vcom_signal_thread,
					  smd, "sw_vcom_signal");
    ...
	return dev_err_probe(dev, ret, "Failed to initialize display pipeline.\n");
    ...
}

If subsequent steps like sharp_memory_pipe_init() fail, the probe function
returns directly without stopping the sw_vcom_signal kthread. The devres
framework automatically frees the smd structure, but the still-running kthread
might dereference it to toggle vcom and update the display.


[Severity: High]
This is a pre-existing issue, but what happens if kthread_run() fails and
returns an ERR_PTR?

drivers/gpu/drm/tiny/sharp-memory.c:sharp_memory_probe() {
    ...
	smd->sw_vcom_signal = kthread_run(sharp_memory_sw_vcom_signal_thread,
					  smd, "sw_vcom_signal");
    ...
}

There doesn't appear to be an IS_ERR() check on the return value. If thread
creation fails due to memory limits, the ERR_PTR is stored in
smd->sw_vcom_signal.

When the device is later removed, it seems to unconditionally call
kthread_stop() on that unvalidated error pointer, which could cause a kernel
panic:

drivers/gpu/drm/tiny/sharp-memory.c:sharp_memory_remove() {
    ...
	kthread_stop(smd->sw_vcom_signal);
    ...
}


[Severity: High]
This is a pre-existing issue, but does this pitch calculation truncate instead
of rounding up?

drivers/gpu/drm/tiny/sharp-memory.c:sharp_memory_probe() {
    ...
	smd->pitch = (SHARP_ADDR_PERIOD + smd->mode->hdisplay + SHARP_DUMMY_PERIOD) / 8;
    ...
}

For non-byte-aligned widths (e.g., the ls018b7dh02 panel with hdisplay = 230),
this integer division evaluates to 246 / 8 = 30 bytes (240 bits).

However, the panel requires 230 bits of data, which drm_fb_xrgb8888_to_mono()
expands to 29 bytes. With 1 byte for the address and 29 bytes for data,
the 30 bytes of pitch are fully consumed. This appears to entirely omit
the required 8-bit dummy period.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901-flickering-v1-0-eaf7bbf9c000@axis.com?part=1

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-09-03  7:33 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-01 11:53 [PATCH RESEND 0/2] drm/tiny: sharp-memory: fix TX buffer corruption on partial update Tobias Johansson
2026-09-01 11:53 ` [PATCH RESEND 1/2] drm/tiny: sharp-memory: fix line address assignment " Tobias Johansson
2026-09-01 12:10   ` sashiko-bot
2026-09-01 11:53 ` [PATCH RESEND 2/2] drm/tiny: sharp-memory: avoid transmitting stale TX buffer data Tobias Johansson
2026-09-01 12:08   ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox