All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Jonathan Frazin" <frazinjonathan@gmail.com>
Cc: dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v2 1/2] drm/mipi-dbi: honour the plane source offset when flushing
Date: Wed, 23 Sep 2026 20:18:08 +0000	[thread overview]
Message-ID: <20260923201808.D24301F000FF@smtp.kernel.org> (raw)
In-Reply-To: <20260923195713.88-2-frazinjonathan@gmail.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/mipi-dbi: hardware protocol violation for 0-width damage rects
--

commit f18906cb9ed3368767cdf7495a3f7b34e33a50c0
Author: Jonathan Frazin <frazinjonathan@gmail.com>
Date:   Wed Sep 23 19:57:11 2026 +0000

    drm/mipi-dbi: honour the plane source offset when flushing

This commit updates mipi_dbi_fb_dirty() to account for the plane's source
origin when calculating the display window address. It also clamps the damage
rectangle to the panel's fixed mode to prevent overshoots caused by
fractional scaling.

> diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
> index 25cf04d029f77..1263dce91c80a 100644
> --- a/drivers/gpu/drm/drm_mipi_dbi.c
> +++ b/drivers/gpu/drm/drm_mipi_dbi.c
[ ... ]
> @@ -271,19 +271,35 @@ static void mipi_dbi_set_window_address(struct mipi_dbi_dev *dbidev,
>  }
>  
>  static void mipi_dbi_fb_dirty(struct iosys_map *src, struct drm_framebuffer *fb,
> -			      struct drm_rect *rect, struct drm_format_conv_state *fmtcnv_state)
> +			      struct drm_rect *rect, unsigned int src_x, unsigned int src_y,
> +			      struct drm_format_conv_state *fmtcnv_state)
>  {
[ ... ]
> +	/*
> +	 * @rect is in framebuffer coordinates, clipped to the plane's src
> +	 * rectangle by the damage iterator against that rectangle's exact
> +	 * 16.16 fixed-point bounds. @src_x/@src_y are that same origin
> +	 * truncated to whole pixels. When the origin has a fractional part,
> +	 * that truncation can leave @rect's far edge up to a pixel past
> +	 * where a whole-pixel @src_x/@src_y would place the panel's own
> +	 * width/height -- and tx_buf is sized for exactly the panel, with no
> +	 * slack for that overshoot. Clamp before using @rect for anything.
> +	 */
> +	rect->x2 = min_t(int, rect->x2, src_x + dbidev->mode.hdisplay);
> +	rect->y2 = min_t(int, rect->y2, src_y + dbidev->mode.vdisplay);
> +
> +	height = rect->y2 - rect->y1;
> +	width = rect->x2 - rect->x1;

[Severity: Medium]
If drm_atomic_helper_damage_merged() clips a damage rectangle entirely to the
fractional overshoot region (e.g., where x1 is at src_x + hdisplay and x2
extends 1 pixel beyond it), does clamping only the far edge result in
rect->x2 becoming equal to rect->x1, leaving width or height as 0?

>  
>  	full = width == fb->width && height == fb->height;
>  
>  	DRM_DEBUG_KMS("Flushing [FB:%d] " DRM_RECT_FMT "\n", fb->base.id, DRM_RECT_ARG(rect));
> @@ -298,8 +314,13 @@ static void mipi_dbi_fb_dirty(struct iosys_map *src, struct drm_framebuffer *fb,
>  		tr = src->vaddr; /* TODO: Use mapping abstraction properly */
>  	}
>  
> -	mipi_dbi_set_window_address(dbidev, rect->x1, rect->x2 - 1, rect->y1,
> -				    rect->y2 - 1);
> +	/*
> +	 * @rect is in framebuffer coordinates and has been clipped to the plane
> +	 * src rectangle by the damage iterator. The panel is addressed relative
> +	 * to the src origin, so subtract it here.
> +	 */
> +	mipi_dbi_set_window_address(dbidev, rect->x1 - src_x, rect->x2 - 1 - src_x,
> +				    rect->y1 - src_y, rect->y2 - 1 - src_y);

[Severity: Medium]
If width or height evaluates to 0 (as questioned above), will this calculate
an underflowed end window coordinate where the end is strictly less than the
start (e.g. xe < xs, due to rect->x2 - 1)?

Since this is followed by a 0-length data transfer, does sending an inverted
window address violate the MIPI DCS specification and potentially cause the
display controller to lock up or corrupt the display state?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260923195713.88-1-frazinjonathan@gmail.com?part=1

  reply	other threads:[~2026-09-23 20:18 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-23 19:57 [PATCH v2 0/2] drm/mipi-dbi: display a cropped region of an oversized framebuffer Jonathan Frazin
2026-09-23 19:57 ` [PATCH v2 1/2] drm/mipi-dbi: honour the plane source offset when flushing Jonathan Frazin
2026-09-23 20:18   ` sashiko-bot [this message]
2026-09-23 19:57 ` [PATCH v2 2/2] drm/tiny: allow a framebuffer larger than the panel on MIPI DBI drivers Jonathan Frazin
2026-09-23 20:25   ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260923201808.D24301F000FF@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=frazinjonathan@gmail.com \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.