Hi
Am 28.10.25 um 18:24 schrieb Jocelyn Falempe:
On 28/10/2025 17:01, Thomas Zimmermann wrote:
Hi Jocelyn
Am 28.10.25 um 15:49 schrieb Jocelyn Falempe:
On 28/10/2025 15:42, Jocelyn Falempe wrote:
In the atomic update callback, ast should call
drm_gem_fb_begin_cpu_access() to make sure it can read the
framebuffer from the CPU, otherwise the data might not be there due
to cache, and synchronization.
Tested on a Lenovo SE100, while rendering on the ArrowLake GPU with
i915 driver, and using ast for display.
I sent this patch a bit too fast, my mistake below:>
Suggested-by: Thomas Zimmermann <[email protected]>
Signed-off-by: Jocelyn Falempe <[email protected]>
---
drivers/gpu/drm/ast/ast_mode.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/
ast_mode.c
index 9ce874dba69c..68da4544d92d 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -556,11 +556,17 @@ static void
ast_primary_plane_helper_atomic_update(struct drm_plane *plane,
ast_set_vbios_color_reg(ast, fb->format, ast_crtc_state-
>vmode);
}
+ /* if the buffer comes from another device */
+ if (!drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE))
+ return;
+
Sorry, there is a typo here, it should be:
if (drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE))
return;
drm_atomic_helper_damage_iter_init(&iter, old_plane_state,
plane_state);
drm_atomic_for_each_plane_damage(&iter, &damage) {
ast_handle_damage(ast_plane, shadow_plane_state->data,
fb, &damage);
}
+ drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
+
Thanks for posting this. I know that you follow the common code from
other drivers, but I think we should change the pattern a bit. The
code in _begin_cpu_access() might fail for whatever reason, but that
does not affect the rest of the plane update.
How about
if (_begin_cpu_access() == 0)
for_each_damage {
handle_damage()
}
_end_cpu_access()
}
and then continue with the rest of the helper? Even if the damage
update doesn't happen, the driver would still program changes of the
framebuffer pitch.
There's no easy way of keeping the damage information across display
updates AFAIK. But the problem might be transient, while a failed
update of the framebuffer pitch can be permanent.
I'm unsure what is better, change the pitch while keeping the old
buffer content, or keeping the last frame, and risking the next update
to mess up. Anyway this shouldn't happen in practice, so I will sent
your version in a v2.
If the error is permanent, we're probably out of luck in any case. If
the error is transient, it might be fixed with the next frame if we
program the framebuffer pitch to the correct value.
Best regards
Thomas
Best regards,
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
