Re: [PATCH 34/70] drm/tiny: pixpaper: Switch to drm_atomic_helper_crtc_create_state

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the plane and CRTC atomic callbacks call sleeping functions in atomic context, violating DRM subsystem rules.
--

--- Patch [34]: [PATCH 34/70] drm/tiny: pixpaper: Switch to drm_atomic_helper_crtc_create_state ---
commit 0c05f2d46ff22e1262b7a7c99b4dd00d2c48ebfe
Author: Maxime Ripard <[email protected]>

drm/tiny: pixpaper: Switch to drm_atomic_helper_crtc_create_state

This commit switches the pixpaper crtc implementation from using the
deprecated drm_atomic_helper_crtc_reset() hook to using
drm_atomic_helper_crtc_create_state().

> diff --git a/drivers/gpu/drm/tiny/pixpaper.c b/drivers/gpu/drm/tiny/pixpaper.c
> index d02ac26d007c1..b69e7c30607e7 100644
> --- a/drivers/gpu/drm/tiny/pixpaper.c
> +++ b/drivers/gpu/drm/tiny/pixpaper.c
> @@ -963,7 +963,7 @@ static const struct drm_plane_helper_funcs pixpaper_plane_helper_funcs = {
>  static const struct drm_crtc_funcs pixpaper_crtc_funcs = {
>  	.set_config = drm_atomic_helper_set_config,
>  	.page_flip = drm_atomic_helper_page_flip,
> -	.reset = drm_atomic_helper_crtc_reset,
> +	.atomic_create_state = drm_atomic_helper_crtc_create_state,

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the atomic
state changes I noticed a pre-existing issue. Does pixpaper_wait_for_panel()
call sleeping functions while in an atomic context?

Looking at pixpaper_crtc_atomic_enable() and pixpaper_plane_atomic_update(),
these are DRM atomic callbacks and execute in atomic context where sleeping
is forbidden. However, both of them call pixpaper_wait_for_panel():

pixpaper_wait_for_panel() {
	...
	usleep_range(1000, 1500);
	while (gpiod_get_value_cansleep(panel->busy) != 1) {
	...
}

Could this trigger scheduling while atomic oopses or deadlocks during
non-blocking commits? Should this be modified to use non-sleeping delays
and GPIO reads instead?

>  	.destroy = drm_crtc_cleanup,
>  	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
>  	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=34