Re: [PATCH] drm/nouveau/kms: defer window LUT disable until the image update

[lyude]

JFYI - I will review this patch soon, but I'm currently waiting on
getting the information I need from elsewhere to actually decode the
NvDisplay error here just to double check that this fix makes sense.

On Sat, 2026-10-03 at 17:41 -0700, Solom Tamawy wrote:
> A modeset on one head can set flush_disable for the entire atomic
> commit while another window changes from an integer framebuffer to
> FP16 without a modeset. The latter sets clr.xlut and set.image, but
> not clr.image.
> 
> nv50_wndw_flush_clr() clears that window's ILUT before the
> intermediate
> disable UPDATE, leaving the old integer image enabled without its
> LUT.
> The new FP16 image is only programmed later in nv50_wndw_flush_set().
> On hardware that uses the ILUT to convert integer input to the
> internal
> FP16 pipeline, the intermediate state is invalid.
> 
> This ordering defect was found while investigating a Plasma login
> hang
> on GB205 after atomic modesetting became enabled by default. The
> failure
> logs contained window UPDATE exceptions followed by a core notifier
> timeout:
> 
>    gsp: Xid:56 CMDre 00000001 00000200 00000001 00000005 0000002d
>    gsp: Xid:56 CMDre 00000005 00000200 00000001 00000005 0000002d
>    drm: core notifier timeout
> 
> Defer the LUT clear until the image update when the old image remains
> enabled across a separate disable UPDATE. The LUT clear and new image
> then take effect together. Keep early clears for windows whose images
> are disabled and preserve the path without a separate disable UPDATE.
> Pass flush_disable explicitly to the set phase, since the new plane
> state's atomic-state backpointer is cleared during state swap.
> 
> Fixes: ebf8ca6b3d6d ("drm/nouveau/kms/nv50-: disable input lut
> harder")
> Cc: stable@xxxxxxxxxxxxxxx
> Assisted-by: LLM
> Signed-off-by: Solom Tamawy <solom@xxxxxxxxx>
> ---
> Based on linux-next f0406245cb9855e6318335a8a223551354291a46
> (next-20261002). This is the functional fix only; atomic modesetting
> remains enabled and the advertised formats are unchanged.
> 
> Testing:
> - A focused C harness extracts the actual pristine and patched flush
>    helpers and models two heads at each UPDATE boundary. The pristine
>    helpers fail the mixed-head integer-to-FP16 case; the patched
> helpers
>    pass all 14 cases. The model is not a full GPU simulator.
> - Nouveau built against the matching kernel configuration and headers
>    with successful modpost and BTF generation and no build warnings.
> - I successfully logged into Plasma Wayland on RTX 5070
>    (GB205), with three NVIDIA DP displays, amdgpu active on the
> integrated
>    GPU, and a CalDigit TS5 Plus dock connected before login. No
> atomic=0
>    override or USB/PCIe/Thunderbolt PM workarounds were present. The
>    expected patched module was loaded, and the I confirmed a
>    successful DRM_CLIENT_CAP_ATOMIC=1 capability probe. No Xid or
> core
>    notifier timeout appeared in this boot.
> 
>   drivers/gpu/drm/nouveau/dispnv50/disp.c |  2 +-
>   drivers/gpu/drm/nouveau/dispnv50/wndw.c | 13 ++++++++++++-
>   drivers/gpu/drm/nouveau/dispnv50/wndw.h |  4 ++--
>   3 files changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> index e91130f93db6..b7022a9b6e6f 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> @@ -2351,7 +2351,7 @@ nv50_disp_atomic_commit_tail(struct
> drm_atomic_commit *state)
>                      (!asyw->clr.mask || atom->flush_disable))
>                          continue;
> 
> -               nv50_wndw_flush_set(wndw, interlock, asyw);
> +               nv50_wndw_flush_set(wndw, interlock,
> atom->flush_disable, asyw);
>          }
> 
>          /* Flush update. */
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.c
> b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
> index 74eb1dfcc043..ac59219e162a 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/wndw.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
> @@ -137,6 +137,14 @@ nv50_wndw_flush_clr(struct nv50_wndw *wndw, u32
> *interlock, bool flush,
>          union nv50_wndw_atom_mask clr = {
>                  .mask = asyw->clr.mask & ~(flush ? 0 : asyw-
> >set.mask),
>          };
> +
> +       /* A different head can require a separate disable update
> while this
> +        * window only changes format. Keep its LUT enabled for the
> old
> image
> +        * until the new image is programmed: integer formats require
> an
> ILUT.
> +        */
> +       if (flush && !clr.image && asyw->set.image)
> +               clr.xlut = false;
> +
>          if (clr.sema ) wndw->func-> sema_clr(wndw);
>          if (clr.ntfy ) wndw->func-> ntfy_clr(wndw);
>          if (clr.xlut ) wndw->func-> xlut_clr(wndw);
> @@ -147,7 +155,7 @@ nv50_wndw_flush_clr(struct nv50_wndw *wndw, u32
> *interlock, bool flush,
>   }
> 
>   void
> -nv50_wndw_flush_set(struct nv50_wndw *wndw, u32 *interlock,
> +nv50_wndw_flush_set(struct nv50_wndw *wndw, u32 *interlock, bool
> flush,
>                      struct nv50_wndw_atom *asyw)
>   {
>          if (interlock[NV50_DISP_INTERLOCK_CORE]) {
> @@ -157,6 +165,9 @@ nv50_wndw_flush_set(struct nv50_wndw *wndw, u32
> *interlock,
> 
>          if (asyw->set.sema ) wndw->func->sema_set (wndw, asyw);
>          if (asyw->set.ntfy ) wndw->func->ntfy_set (wndw, asyw);
> +       /* Apply a deferred LUT disable together with the new image.
> */
> +       if (flush && asyw->clr.xlut && !asyw->clr.image && asyw-
> >set.image)
> +               wndw->func->xlut_clr(wndw);
>          if (asyw->set.image) wndw->func->image_set(wndw, asyw);
> 
>          if (asyw->set.xlut ) {
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.h
> b/drivers/gpu/drm/nouveau/dispnv50/wndw.h
> index 7bd8bcc199db..b0b1b10cf782 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/wndw.h
> +++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.h
> @@ -40,8 +40,8 @@ int nv50_wndw_new_(const struct nv50_wndw_func *,
> struct drm_device *,
>                     const u32 *format, u32 heads,
>                     enum nv50_disp_interlock_type, u32
> interlock_data,
>                     struct nv50_wndw **);
> -void nv50_wndw_flush_set(struct nv50_wndw *, u32 *interlock,
> -                        struct nv50_wndw_atom *);
> +void nv50_wndw_flush_set(struct nv50_wndw *wndw, u32 *interlock,
> bool
> flush,
> +                        struct nv50_wndw_atom *asyw);
>   void nv50_wndw_flush_clr(struct nv50_wndw *, u32 *interlock, bool
> flush,
>                           struct nv50_wndw_atom *);
>   void nv50_wndw_ntfy_enable(struct nv50_wndw *, struct
> nv50_wndw_atom *);
> 
> base-commit: f0406245cb9855e6318335a8a223551354291a46