Re: [PATCH] [RFC v3]: drm/i915/display: Use ceiling division for NV12 UV surface offset calculation

["Juha-Pekka Heikkilä" <juhapekka.heikkila@gmail.com>]

Look ok to me. I tested this make related failing test pass and I didn't 
spot planar formats or scaler related failures in results for this patch.

Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>

On 15/04/2026 19.58, Vidya Srinivas wrote:
> For LNL+, odd source size and panning for YUV 422/420 surfaces is
> supported. However, it requires the UV (chroma) surface Start X/Y and
> width/height to be calculated as ceiling(half of Y plane value) rather
> than floor.
> 
> The current code uses (>> 17) which combines the U16.16 fixed-point to
> integer conversion (>> 16) with a divide-by-2 for chroma subsampling
> (>> 1) into a single floor division. For odd Y plane values this
> produces an off-by-one error in the UV plane offset.
> 
> On Android systems we see PLANE ATS fault when NV12 overlays are
> used with odd source dimensions:
> 
> [  126.854200] xe 0000:00:02.0: [drm:intel_atomic_setup_scaler [xe]] [CRTC:148:pipe A] attached scaler id 0.0 to PLANE:33
> [  126.854617] xe 0000:00:02.0: [drm:skl_update_scaler [xe]] [CRTC:148:pipe A] scaler_user index 0.0: staged scaling request for 1279x719->1340x753
> [  126.854837] xe 0000:00:02.0: [drm:intel_plane_atomic_check [xe]] UV plane [PLANE:33:plane 1A] using Y plane [PLANE:123:plane 4A]
> [  126.854926] xe 0000:00:02.0: [drm] *ERROR* [CRTC:148:pipe A] PLANE ATS fault
> 
> With Y plane width 1279:
>    floor(1279/2) = 639 (current)
>    ceil(1279/2)  = 640 (required)
> 
> Introduce fp_16_16_div2() and fp_16_16_to_int_ceil() helpers to cleanly
> separate the two operations: first halve the U16.16 fixed-point value
> for chroma subsampling (staying in fixed-point domain), then convert
> to integer with ceiling rounding.
> 
> v2: Use DIV_ROUND_UP(value, 1 << 17) to preserve sub-pixel precision
>      while making the ceiling division readable (Jani, Uma)
> 
> v3: Split into two helpers - fp_16_16_div2() for fixed-point division
>      by 2 and fp_16_16_to_int_ceil() for ceiling conversion to integer,
>      cleanly separating chroma subsampling from fixed-point to integer
>      conversion (Jani)
> 
> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> ---
>   .../drm/i915/display/skl_universal_plane.c    | 27 ++++++++++++++++---
>   1 file changed, 23 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> index 7a9d494334b5..e772b0d716c7 100644
> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> @@ -2126,6 +2126,19 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
>   	return 0;
>   }
>   
> +
> +/* Divide a U16.16 fixed-point value by 2, staying in fixed-point domain */
> +static inline u32 fp_16_16_div2(u32 fp)
> +{
> +	return fp >> 1;
> +}
> +
> +/* Convert a U16.16 fixed-point value to integer, rounding up */
> +static inline int fp_16_16_to_int_ceil(u32 fp)
> +{
> +	return DIV_ROUND_UP(fp, 1 << 16);
> +}
> +
>   static int skl_check_nv12_aux_surface(struct intel_plane_state *plane_state)
>   {
>   	struct intel_display *display = to_intel_display(plane_state);
> @@ -2139,10 +2152,16 @@ static int skl_check_nv12_aux_surface(struct intel_plane_state *plane_state)
>   	int min_height = intel_plane_min_height(plane, fb, uv_plane, rotation);
>   	int max_width = intel_plane_max_width(plane, fb, uv_plane, rotation);
>   	int max_height = intel_plane_max_height(plane, fb, uv_plane, rotation);
> -	int x = plane_state->uapi.src.x1 >> 17;
> -	int y = plane_state->uapi.src.y1 >> 17;
> -	int w = drm_rect_width(&plane_state->uapi.src) >> 17;
> -	int h = drm_rect_height(&plane_state->uapi.src) >> 17;
> +
> +	/*
> +	 * LNL+ UV surface start/size =
> +	 * ceiling(half of Y plane start/size). Use ceiling division
> +	 * unconditionally; it is a no-op for even values.
> +	 */
> +	int x = fp_16_16_to_int_ceil(fp_16_16_div2(plane_state->uapi.src.x1));
> +	int y = fp_16_16_to_int_ceil(fp_16_16_div2(plane_state->uapi.src.y1));
> +	int w = fp_16_16_to_int_ceil(fp_16_16_div2(drm_rect_width(&plane_state->uapi.src)));
> +	int h = fp_16_16_to_int_ceil(fp_16_16_div2(drm_rect_height(&plane_state->uapi.src)));
>   	u32 offset;
>   
>   	/* FIXME not quite sure how/if these apply to the chroma plane */