> -----Original Message-----
> From: Shankar, Uma <[email protected]>
> Sent: 14 July 2026 03:35
> To: Srinivas, Vidya <[email protected]>; Juha-Pekka Heikkilä
> <[email protected]>; [email protected]
> Cc: [email protected]
> Subject: RE: [PATCH] [RFC]: drm/i915/display: Fix NV12 ceiling division for
> bigjoiner case
>
>
>
> > -----Original Message-----
> > From: Intel-xe <[email protected]> On Behalf Of
> > Srinivas, Vidya
> > Sent: Friday, July 3, 2026 9:46 PM
> > To: Juha-Pekka Heikkilä <[email protected]>; intel-
> > [email protected]
> > Cc: [email protected]
> > Subject: RE: [PATCH] [RFC]: drm/i915/display: Fix NV12 ceiling
> > division for bigjoiner case
> >
> >
> >
> > > -----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.
>
> Pushed the change to drm-intel-next. Thanks for the patch and review.
Thank you very much Uma.
Regards
Vidya
>
> Regards,
> Uma Shankar
>
> > 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
> > > > */
