mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] drm/nouveau/kms: defer window LUT disable until the image update
@ 2026-10-04  0:41 Solom Tamawy
  2026-10-09 22:25 ` lyude
  0 siblings, 1 reply; 2+ messages in thread
From: Solom Tamawy @ 2026-10-04  0:41 UTC (permalink / raw)
  To: nouveau
  Cc: Solom Tamawy, Lyude Paul, Danilo Krummrich, dri-devel, linux-kernel

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@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Solom Tamawy <solom@solom.dev>
---
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

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] drm/nouveau/kms: defer window LUT disable until the image update
  2026-10-04  0:41 [PATCH] drm/nouveau/kms: defer window LUT disable until the image update Solom Tamawy
@ 2026-10-09 22:25 ` lyude
  0 siblings, 0 replies; 2+ messages in thread
From: lyude @ 2026-10-09 22:25 UTC (permalink / raw)
  To: Solom Tamawy, nouveau; +Cc: Danilo Krummrich, dri-devel, linux-kernel

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@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Solom Tamawy <solom@solom.dev>
> ---
> 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


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-10-09 22:25 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-04  0:41 [PATCH] drm/nouveau/kms: defer window LUT disable until the image update Solom Tamawy
2026-10-09 22:25 ` lyude

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®