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.
Best regards,
--
Jocelyn
Best regards
Thomas
/*
* Some BMCs stop scanning out the video signal after the driver
* reprogrammed the offset. This stalls display output for several
base-commit: 4f9ffd2c80a2fa09dcc8dfa0482cb7e0fb6fcf6c
