[PATCH AUTOSEL 6.19-5.15] drm/atmel-hlcdc: don't reject the commit if the src rect has fractional parts

Sasha Levin
2026-02-14T01:04:42Z

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