Re: [PATCH RESEND] drm/sysfb: Fix atomic_disable to use dst coordinates instead of src

Hi

Am 25.08.26 um 12:41 schrieb [email protected]:
> From: Shixiong Ou <[email protected]>
>
> drm_sysfb_plane_helper_atomic_disable() constructs the clear rectangle
> from plane_state->src_* coordinates. This is inconsistent with
> drm_sysfb_plane_helper_atomic_update() which uses plane_state->dst
> — the destination rectangle already computed by the DRM core with panel
> orientation applied.
>
> Use plane_state->dst directly, matching the approach in atomic_update.

Thanks for bringing this up.  I think the issue needs to be addressed in 
a different way.  The plane's video memory is behind dst_vmap. It needs 
to be cleared to 0 entirely without considering any user-given sizes.  
The relevant size can be found in the fb_mode.

Here's some pseudo code:

   drm_rect_init(dst_clip, 0, 0, sysfb->fb_mode.width, 
sysfb->fb_mode_height).

   lines = drm_rect_height(dst_clip);
   linepixels = drm_rect_width(dst_clip);
   linesize = drm_format_info_min_pitch(sysfb->fb_format, 0, linepixels);

   dst_size = min(linesize, dst_pitch);

   for (i < lines) {
     memset_io(dst_vmap, 0, dst_size);
     dst_vmap += dst_pitch
   }

This should safely clear the visible part of the buffer without touching 
any non-visible bytes at the end of each line. It also does not depend 
on user-controlled size input.

Do you want to provide an updated patch  or shall I send it?

Best regards
Thomas

>
> Signed-off-by: Shixiong Ou <[email protected]>
> ---
>   drivers/gpu/drm/sysfb/drm_sysfb_modeset.c | 6 +-----
>   1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/sysfb/drm_sysfb_modeset.c b/drivers/gpu/drm/sysfb/drm_sysfb_modeset.c
> index d2de29caf89e..18466de886aa 100644
> --- a/drivers/gpu/drm/sysfb/drm_sysfb_modeset.c
> +++ b/drivers/gpu/drm/sysfb/drm_sysfb_modeset.c
> @@ -380,14 +380,10 @@ void drm_sysfb_plane_helper_atomic_disable(struct drm_plane *plane,
>   	void __iomem *dst_vmap = dst.vaddr_iomem; /* TODO: Use mapping abstraction */
>   	unsigned int dst_pitch = sysfb->fb_pitch;
>   	const struct drm_format_info *dst_format = sysfb->fb_format;
> -	struct drm_rect dst_clip;
> +	struct drm_rect dst_clip = plane_state->dst;
>   	unsigned long lines, linepixels, i;
>   	int idx;
>   
> -	drm_rect_init(&dst_clip,
> -		      plane_state->src_x >> 16, plane_state->src_y >> 16,
> -		      plane_state->src_w >> 16, plane_state->src_h >> 16);
> -
>   	lines = drm_rect_height(&dst_clip);
>   	linepixels = drm_rect_width(&dst_clip);
>   
>
> Content-type: Text/plain
>
> No virus found
> 		Checked by Hillstone Network AntiVirus

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, (HRB 36809, AG Nürnberg)