> -----Original Message-----
> From: Juha-Pekka Heikkilä <[email protected]>
> Sent: 03 July 2026 19:21
> To: Srinivas, Vidya <[email protected]>;
> [email protected]
> Cc: [email protected]
> Subject: Re: [PATCH] [RFC]: drm/i915/display: Fix NV12 ceiling division for
> bigjoiner case
>
> Hi Vidya,
>
> apologies for the delayed reply. I tried to do the math and seems I got
> correct
> numbers as expected what you say below. I can't test this anywhere but it
> seems correct on code and idea level.
>
> Reviewed-by: Juha-Pekka Heikkila <[email protected]>
Hello Juha
Thank you very much.
Regards
Vidya
>
> On 18/06/2026 21.18, Vidya Srinivas wrote:
> > Commit 16df4cc63c58 ("drm/i915/display: Use ceiling division for NV12
> > UV surface offset calculation") computes the UV (chroma) surface
> > start/size as ceiling(half of Y plane start/size) directly from the
> > U16.16 fixed-point source rectangle:
> >
> > x = fp_16_16_to_int_ceil(fp_16_16_div2(src.x1));
> >
> > For a single pipe the source coordinates are integers, so this is
> > correct.
> > (UV start = ceiling(half of Y plane start)).
> >
> > With bigjoiner + a plane scaler the picture changes. The pipe boundary
> > is a fixed integer destination pixel, but the plane's position and the
> > scaler ratio are arbitrary, so drm_rect_clip_scaled() maps the seam
> > back to a *fractional* per-pipe source. For a 1280->2407 upscaled NV12
> > plane crossing the seam:
> >
> > master src: width = 1204 * 1280/2407 = 640.265899, x1 = 0
> > joiner src: width = 1203 * 1280/2407 = 639.734115, x1 =
> > 640.265884
> >
> > The luma path floors this to an integer (src.x1 >> 16 = 640), but the
> > UV path takes ceiling(640.265884 / 2) = ceil(320.13) = 321. The Y
> > plane then starts at column 640 while the UV plane starts at 321*2 =
> > 642, pushing the chroma read one column past the 640-wide chroma
> > surface on the joiner secondary:
> >
> > [CRTC:382:pipe C] PLANE ATS fault
> > [CRTC:382:pipe C][PLANE:267:plane 1C] fault (CTL=0x81009400,
> > ...)
> >
> > The spec "Y plane start" is the integer pixel the luma surface
> > actually programs (640), not the pre-floor fixed-point value (640.27).
> > Convert the Y plane start/size to integer first - matching
> > skl_check_main_surface()
> > - and then apply the ceiling. This is a no-op for the integer
> > (non-joiner) case and yields the correct, in-bounds chroma offset for
> > the fractional joiner seam:
> >
> > before fix after fix
> > master 1B: x=0 w=321 x=0 w=320 -> [0, 320)
> > slave 1C: x=321 w=320 x=320 w=320 -> [320, 640)
> >
> > The two halves now tile the 640-wide chroma plane exactly and the ATS
> > fault is gone.
> >
> > Assisted-by: GitHub-Copilot:Claude-Opus-4.8
> > Fixes: 16df4cc63c58 ("drm/i915/display: Use ceiling division for NV12
> > UV surface offset calculation")
> > Signed-off-by: Vidya Srinivas <[email protected]>
> > ---
> > .../drm/i915/display/skl_universal_plane.c | 33 ++++++++-----------
> > 1 file changed, 13 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > index ad4bfff6903d..164b7d61c9a3 100644
> > --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > @@ -2126,19 +2126,6 @@ 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); @@
> > -2154,14 +2141,20 @@ static int skl_check_nv12_aux_surface(struct
> intel_plane_state *plane_state)
> > int max_height = intel_plane_max_height(plane, fb, uv_plane,
> > rotation);
> >
> > /*
> > - * LNL+ UV surface start/size =
> > - * ceiling(half of Y plane start/size). Use ceiling division
> > - * unconditionally; it is a no-op for even values.
> > + * UV (chroma) start/size = ceiling(half of the *integer* Y plane
> > + * start/size), i.e. the value the luma surface programs (src >> 16),
> > + * not the raw U16.16. A bigjoiner seam mapped through the scaler
> can
> > + * give a fractional luma src; ceiling that directly would round the
> > + * chroma one column too far and read past the chroma surface.
> > */
> > - 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)));
> > + int luma_x = plane_state->uapi.src.x1 >> 16;
> > + int luma_y = plane_state->uapi.src.y1 >> 16;
> > + int luma_w = drm_rect_width(&plane_state->uapi.src) >> 16;
> > + int luma_h = drm_rect_height(&plane_state->uapi.src) >> 16;
> > + int x = DIV_ROUND_UP(luma_x, 2);
> > + int y = DIV_ROUND_UP(luma_y, 2);
> > + int w = DIV_ROUND_UP(luma_x + luma_w, 2) - x;
> > + int h = DIV_ROUND_UP(luma_y + luma_h, 2) - y;
> > u32 offset;
> >
> > /* FIXME not quite sure how/if these apply to the chroma plane */
