Re: [PATCH v2 6/8] drm/repaper: replace struct drm_simple_display_pipe with regular atomic helpers

[sashiko-bot@kernel.org]

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

New issues:
- [High] Missing CRTC active check in the plane's atomic_update callback causes the driver to send SPI framebuffer updates to a powered-off display panel.

Pre-existing issues:
- [High] The CRTC atomic enable/disable and plane atomic update callbacks execute sleeping functions and allocate memory with GFP_KERNEL, which is forbidden in atomic context.
--

--- Patch [6]: [PATCH v2 6/8] drm/repaper: replace struct drm_simple_display_pipe with regular atomic helpers ---
commit 3d9748839d37276a557c720301a5a848296e54b7
Author: Ze Huang <ze.huang@oss.qualcomm.com>

drm/repaper: replace struct drm_simple_display_pipe with regular atomic helpers
    
Convert repaper to explicit primary plane, CRTC and encoder objects.
Keep shadow-plane helpers, framebuffer access handling and no-scaling
plane-state validation from simple-KMS path.

> diff --git a/drivers/gpu/drm/tiny/repaper.c b/drivers/gpu/drm/tiny/repaper.c
> index c8270591afc7f..7057075161c4d 100644
> --- a/drivers/gpu/drm/tiny/repaper.c
> +++ b/drivers/gpu/drm/tiny/repaper.c

[ ... ]

> -static void repaper_pipe_enable(struct drm_simple_display_pipe *pipe,
> -				struct drm_crtc_state *crtc_state,
> -				struct drm_plane_state *plane_state)
> +static void repaper_crtc_helper_atomic_enable(struct drm_crtc *crtc,
> +					      struct drm_atomic_commit *commit)
>  {
> -	struct repaper_epd *epd = drm_to_epd(pipe->crtc.dev);
> +	struct repaper_epd *epd = drm_to_epd(crtc->dev);

[Severity: High]
This is a pre-existing issue, but do the CRTC atomic enable/disable and plane
atomic update callbacks execute sleeping functions in atomic context?

The DRM subsystem explicitly forbids sleeping in CRTC enable/disable and plane
update callbacks. However:
- repaper_crtc_helper_atomic_enable() calls usleep_range()
- repaper_crtc_helper_atomic_disable() calls msleep()
- repaper_plane_helper_atomic_update() calls repaper_fb_dirty(), which
  allocates memory using GFP_KERNEL

Since these operations are now wired directly to the standard atomic helpers
which run in atomic context (for example, during non-blocking commits), can
this cause system instability or deadlocks?

[ ... ]

> -static void repaper_pipe_update(struct drm_simple_display_pipe *pipe,
> -				struct drm_plane_state *old_state)
> +static void repaper_plane_helper_atomic_update(struct drm_plane *plane,
> +					       struct drm_atomic_commit *commit)
>  {
> -	struct drm_plane_state *state = pipe->plane.state;
> -	struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(state);
> +	struct drm_plane_state *old_pstate = drm_atomic_get_old_plane_state(commit, plane);
> +	struct drm_plane_state *pstate = drm_atomic_get_new_plane_state(commit, plane);
> +	struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(pstate);
>  	struct drm_rect rect;
>  
> -	if (!pipe->crtc.state->active)
> +	if (!pstate->fb)
>  		return;
>  
> -	if (drm_atomic_helper_damage_merged(old_state, state, &rect))
> -		repaper_fb_dirty(state->fb, shadow_plane_state->data,
> +	if (drm_atomic_helper_damage_merged(old_pstate, pstate, &rect))
> +		repaper_fb_dirty(pstate->fb, shadow_plane_state->data,
>  				 &shadow_plane_state->fmtcnv_state);

[Severity: High]
Can this cause the driver to send SPI framebuffer updates to a powered-off
display panel?

The previous code checked !pipe->crtc.state->active here to ensure we
do not update inactive displays. During a DPMS OFF request,
drm_atomic_helper_commit_modeset_disables() runs first and disables the
panel by calling repaper_crtc_helper_atomic_disable(). 

Next, drm_atomic_helper_commit_planes() will still call this update
function because active_only is not set by default and pstate->visible
can remain true.

Without the crtc_state->active check, doesn't this result in writing data
to dead hardware via repaper_fb_dirty(), potentially causing SPI timeouts
or driver hangs?

>  }

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716-drm-simple-kms-removal-v2-0-1133a8fc3785@oss.qualcomm.com?part=6