public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Ludovic Desroches <ludovic.desroches@microchip.com>,
	Manikandan Muralidharan <manikandan.m@microchip.com>,
	Sasha Levin <sashal@kernel.org>,
	dharma.b@microchip.com, nicolas.ferre@microchip.com,
	alexandre.belloni@bootlin.com, claudiu.beznea@tuxon.dev,
	dri-devel@lists.freedesktop.org,
	linux-arm-kernel@lists.infradead.org
Subject: [PATCH AUTOSEL 6.19-5.15] drm/atmel-hlcdc: don't reject the commit if the src rect has fractional parts
Date: Fri, 13 Feb 2026 19:58:54 -0500	[thread overview]
Message-ID: <20260214010245.3671907-54-sashal@kernel.org> (raw)
In-Reply-To: <20260214010245.3671907-1-sashal@kernel.org>

From: Ludovic Desroches <ludovic.desroches@microchip.com>

[ Upstream commit 06682206e2a1883354ed758c09efeb51f435adbd ]

Don’t reject the commit when the source rectangle has fractional parts.
This can occur due to scaling: drm_atomic_helper_check_plane_state() calls
drm_rect_clip_scaled(), which may introduce fractional parts while
computing the clipped source rectangle. This does not imply the commit is
invalid, so we should accept it instead of discarding it.

Signed-off-by: Ludovic Desroches <ludovic.desroches@microchip.com>
Reviewed-by: Manikandan Muralidharan <manikandan.m@microchip.com>
Link: https://patch.msgid.link/20251120-lcd_scaling_fix-v1-1-5ffc98557923@microchip.com
Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

## Analysis of drm/atmel-hlcdc: don't reject the commit if the src rect
has fractional parts

### 1. COMMIT MESSAGE ANALYSIS

The commit message clearly describes a bug fix: the driver was
incorrectly rejecting valid plane configurations when the source
rectangle had fractional (subpixel) parts. The fractional parts are
legitimately introduced by `drm_rect_clip_scaled()` during the atomic
check process, but the driver was treating them as invalid input and
returning `-EINVAL`.

Keywords: "don't reject" — this is fixing a false rejection of valid
display configurations.

### 2. CODE CHANGE ANALYSIS

The change is straightforward and well-contained:

**What was happening before:**
1. `hstate->src_x/y/w/h` were assigned the raw 16.16 fixed-point values
   from `s->src`
2. A check was performed: if any of these values had bits set in the
   lower 16 bits (fractional part, via `SUBPIXEL_MASK = 0xffff`), the
   function returned `-EINVAL`
3. Only after passing that check were the values right-shifted by 16 to
   get integer pixel coordinates

**What happens now:**
1. `hstate->src_x/y/w/h` are assigned the values right-shifted by 16
   immediately (converting to integer pixels)
2. The subpixel mask check is completely removed
3. The `SUBPIXEL_MASK` macro definition is removed as it's no longer
   needed

**The bug:** `drm_atomic_helper_check_plane_state()` calls
`drm_rect_clip_scaled()`, which can introduce fractional parts when
computing clipped source rectangles during scaling operations. This is
normal DRM behavior — the fractional parts represent subpixel precision
in the scaling calculation. The Atmel HLCDC hardware doesn't support
subpixel addressing, so the correct behavior is to truncate (>> 16) and
use integer coordinates, not reject the entire configuration.

**Impact of the bug:** When scaling is involved and clipping produces
fractional coordinates, display operations would fail with `-EINVAL`.
This means users would see display failures (plane updates rejected) in
legitimate scaling scenarios. This is a real user-visible bug — display
output fails when it shouldn't.

### 3. CLASSIFICATION

This is a **bug fix**. The driver was incorrectly rejecting valid atomic
commits, causing display failures during scaling operations. No new
features are added, no new APIs introduced.

### 4. SCOPE AND RISK ASSESSMENT

- **Lines changed:** ~15 lines removed, ~4 lines modified — very small
- **Files touched:** 1 file (`atmel_hlcdc_plane.c`)
- **Subsystem:** DRM driver for Atmel HLCDC (embedded ARM display
  controller)
- **Risk:** Very low. The change simply truncates subpixel precision
  instead of rejecting it. The hardware can only address whole pixels
  anyway, so truncation is the correct behavior. The integer parts of
  the coordinates are unchanged.
- **Could it break something?** Extremely unlikely. The only behavioral
  change is accepting configurations that were previously rejected. The
  pixel coordinates used are identical (same >> 16 operation, just done
  earlier).

### 5. USER IMPACT

This affects users of Atmel/Microchip SAM9/SAMA5 SoC-based embedded
systems that use the HLCDC display controller with plane scaling. When
scaling is active and clipping occurs, display updates would fail. This
is a real-world scenario for embedded display applications.

### 6. STABILITY INDICATORS

- **Reviewed-by:** Manikandan Muralidharan (subsystem maintainer)
- **Author:** Ludovic Desroches (Microchip engineer, familiar with the
  hardware)
- The fix is logically sound — truncating subpixel coordinates is
  standard practice in DRM drivers that don't support subpixel precision

### 7. DEPENDENCY CHECK

The commit is self-contained. It doesn't depend on any other changes.
The code it modifies (`atmel_hlcdc_plane_atomic_check`) has been present
in stable trees for a long time. The functions it interacts with
(`drm_atomic_helper_check_plane_state`, `drm_rect_width/height`) are
standard DRM helpers available in all stable trees.

### Summary

This is a small, well-understood bug fix for an incorrect rejection of
valid display configurations in the Atmel HLCDC driver. The fix is
surgical (removes an overly strict validation check and shifts the
coordinate conversion earlier), has been reviewed by the subsystem
maintainer, and carries minimal risk of regression. It fixes a real
user-visible bug (display failures during scaling with clipping) that
affects embedded systems using this display controller.

**YES**

 .../gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c   | 19 ++++---------------
 1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
index c0075894dc422..ec1fb5f9549a2 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
@@ -79,8 +79,6 @@ drm_plane_state_to_atmel_hlcdc_plane_state(struct drm_plane_state *s)
 	return container_of(s, struct atmel_hlcdc_plane_state, base);
 }
 
-#define SUBPIXEL_MASK			0xffff
-
 static uint32_t rgb_formats[] = {
 	DRM_FORMAT_C8,
 	DRM_FORMAT_XRGB4444,
@@ -745,24 +743,15 @@ static int atmel_hlcdc_plane_atomic_check(struct drm_plane *p,
 	if (ret || !s->visible)
 		return ret;
 
-	hstate->src_x = s->src.x1;
-	hstate->src_y = s->src.y1;
-	hstate->src_w = drm_rect_width(&s->src);
-	hstate->src_h = drm_rect_height(&s->src);
+	hstate->src_x = s->src.x1 >> 16;
+	hstate->src_y = s->src.y1 >> 16;
+	hstate->src_w = drm_rect_width(&s->src) >> 16;
+	hstate->src_h = drm_rect_height(&s->src) >> 16;
 	hstate->crtc_x = s->dst.x1;
 	hstate->crtc_y = s->dst.y1;
 	hstate->crtc_w = drm_rect_width(&s->dst);
 	hstate->crtc_h = drm_rect_height(&s->dst);
 
-	if ((hstate->src_x | hstate->src_y | hstate->src_w | hstate->src_h) &
-	    SUBPIXEL_MASK)
-		return -EINVAL;
-
-	hstate->src_x >>= 16;
-	hstate->src_y >>= 16;
-	hstate->src_w >>= 16;
-	hstate->src_h >>= 16;
-
 	hstate->nplanes = fb->format->num_planes;
 	if (hstate->nplanes > ATMEL_HLCDC_LAYER_MAX_PLANES)
 		return -EINVAL;
-- 
2.51.0



  parent reply	other threads:[~2026-02-14  1:04 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20260214010245.3671907-1-sashal@kernel.org>
2026-02-14  0:58 ` [PATCH AUTOSEL 6.19-5.15] drm/v3d: Set DMA segment size to avoid debug warnings Sasha Levin
2026-02-14  0:58 ` [PATCH AUTOSEL 6.19-6.12] ASoC: fsl: imx-rpmsg: use snd_soc_find_dai_with_mutex() in probe Sasha Levin
2026-02-14  0:58 ` [PATCH AUTOSEL 6.19-5.10] drm/atmel-hlcdc: fix use-after-free of drm_crtc_commit after release Sasha Levin
2026-02-14  0:58 ` [PATCH AUTOSEL 6.19-5.15] spi: stm32: fix Overrun issue at < 8bpw Sasha Levin
2026-02-14  0:58 ` [PATCH AUTOSEL 6.19-5.10] drm/atmel-hlcdc: fix memory leak from the atomic_destroy_state callback Sasha Levin
2026-02-14  0:58 ` [PATCH AUTOSEL 6.19-6.1] media: rkisp1: Fix filter mode register configuration Sasha Levin
2026-02-14  0:58 ` Sasha Levin [this message]
2026-02-14  0:59 ` [PATCH AUTOSEL 6.19-5.10] gpio: aspeed-sgpio: Change the macro to support deferred probe Sasha Levin
2026-02-14  0:59 ` [PATCH AUTOSEL 6.19] drm/atmel-hlcdc: destroy properly the plane state in the reset callback Sasha Levin
2026-02-14  0:59 ` [PATCH AUTOSEL 6.19-6.6] media: mediatek: vcodec: Don't try to decode 422/444 VP9 Sasha Levin
2026-02-14  1:00 ` [PATCH AUTOSEL 6.19-6.1] ASoC: sunxi: sun50i-dmic: Add missing check for devm_regmap_init_mmio Sasha Levin

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=20260214010245.3671907-54-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=alexandre.belloni@bootlin.com \
    --cc=claudiu.beznea@tuxon.dev \
    --cc=dharma.b@microchip.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=ludovic.desroches@microchip.com \
    --cc=manikandan.m@microchip.com \
    --cc=nicolas.ferre@microchip.com \
    --cc=patches@lists.linux.dev \
    --cc=stable@vger.kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox