All of lore.kernel.org
 help / color / mirror / Atom feed
From: lyude@redhat.com
To: Marek Czernohous <mczernohous@gmail.com>,
	nouveau@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org, Danilo Krummrich <dakr@kernel.org>,
	David Airlie <airlied@gmail.com>, Simona Vetter <simona@ffwll.ch>
Subject: Re: [PATCH v4 0/2] drm/nouveau: NVAC (MCP79) stability: MSI rearm and a NULL crtc guard
Date: Tue, 18 Aug 2026 20:02:08 -0400	[thread overview]
Message-ID: <6244a95cc925d8b0f3c01ac000ca56dad246305b.camel@redhat.com> (raw)
In-Reply-To: <178688574400.522643.6695278742335367229@gmail.com>

First patch looks fine, I sent a new patch series to address the issue
pointed out by patch 2, as it was a bit of a bandaid around a larger
issue.

Will push the first patch in just a moment

On Sun, 2026-08-16 at 15:09 +0200, Marek Czernohous wrote:
> From: Marek Czernohous <marek@czernohous.de>
> 
> This is v4 of two NVAC (MCP79) stability fixes.  v3 went out on 11
> June
> and has had no replies:
> 
>  
> https://lore.kernel.org/all/20260611124535.527275-1-mczernohous@gmail.com/
> 
> Nothing in either patch changed.  Same diffs, same reasoning, same
> trailers.  The reason this is a v4 and not a ping is that 2/2 no
> longer
> applies to mainline, so a ping would have pointed at a series nobody
> can
> test.
> 
> What moved was somebody else's context, not this code.
> Commit 5164f7e7ff8e ("drm: Rename struct drm_atomic_state to
> drm_atomic_commit") changed the callback signature, so
> nv50_sor_atomic_disable() now takes a struct drm_atomic_commit
> *state.
> 2/2 is rebased onto that by hand.  1/2 applies unchanged and is
> byte-identical to v3.
> 
> Both bugs are still live in c21bb4193868.  dispnv50/disp.c:1568 still
> does nv50_head(nv_encoder->crtc) with no NULL check, and there is no
> guard anywhere in the function.  Immediately below it, :1570 takes
> nv50_outp_get_old_connector() and :1572 dereferences the result
> without
> checking it.
> 
> 1/2 drm/nouveau/pci: use config-space MSI rearm on MCP79/MCP7A (NVAC)
> 
>     NVAC re-arms MSI through the MMIO mirror of PCI config space,
> which
>     is unreliable on this IGP.  A missed re-arm leaves the interrupt
>     line dead, submission times out and the GPU looks hung until
>     reboot.  Give NVAC its own pci func that re-arms through real
>     config space, the way commit 5112abc6a433 ("drm/nouveau/pci/g92:
>     Fix rearm") already did for g92.  Only NVAC is switched, because
>     that is the part this was validated on.
> 
> 2/2 drm/nouveau/kms: guard NULL crtc in nv50_sor_atomic_disable()
> 
>     Restore the NULL check that
>     commit f575f2bdb6c3 ("drm/nouveau/kms/nv50-: Remove
>     (nv_encoder->crtc) checks in ->disable callbacks") removed, as a
>     drm_WARN_ON_ONCE() rather than a silent return, since a NULL crtc
>     here is a state-tracking inconsistency worth seeing.  The guard
> is
>     hoisted above the backlight teardown so the old connector is only
>     used after it has been checked; without that the oops would just
>     move a few lines down.
> 
> Testing.  The MSI change continues to run here on the Mac mini as a
> daily driver, and Fab Stz confirmed it independently on an iMac9,1.
> Both Tested-by tags are carried over unchanged from v3; the
> diffs they were given are the diffs sent here, apart from the
> mechanical
> rebase of 2/2.  For v4 I rebuilt 2/2 with
> CONFIG_DRM_NOUVEAU_BACKLIGHT
> set to y and to n, because the patch splits the #ifdef block in two
> and
> that is where a change of this shape tends to break.
> 
> If something about the shape of the series is what is holding it up,
> I
> would rather hear that than keep rebasing it.
> 
> Marek Czernohous (2):
>   drm/nouveau/pci: use config-space MSI rearm on MCP79/MCP7A (NVAC)
>   drm/nouveau/kms: guard NULL crtc in nv50_sor_atomic_disable()
> 
>  drivers/gpu/drm/nouveau/dispnv50/disp.c       | 30 +++++++++++++---
>  .../gpu/drm/nouveau/include/nvkm/subdev/pci.h |  1 +
>  .../gpu/drm/nouveau/nvkm/engine/device/base.c |  2 +-
>  .../gpu/drm/nouveau/nvkm/subdev/pci/Kbuild    |  1 +
>  .../gpu/drm/nouveau/nvkm/subdev/pci/mcp79.c   | 35
> +++++++++++++++++++
>  5 files changed, 63 insertions(+), 6 deletions(-)
>  create mode 100644 drivers/gpu/drm/nouveau/nvkm/subdev/pci/mcp79.c
> 
> 
> base-commit: c21bb4193868a8de71fc4693fa741e195fdf5d86


WARNING: multiple messages have this Message-ID (diff)
From: lyude@redhat.com
To: Marek Czernohous <mczernohous@gmail.com>,
	nouveau@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org, Danilo Krummrich <dakr@kernel.org>,
	Simona Vetter <simona@ffwll.ch>
Subject: Re: [PATCH v4 0/2] drm/nouveau: NVAC (MCP79) stability: MSI rearm and a NULL crtc guard
Date: Tue, 18 Aug 2026 20:02:08 -0400	[thread overview]
Message-ID: <6244a95cc925d8b0f3c01ac000ca56dad246305b.camel@redhat.com> (raw)
In-Reply-To: <178688574400.522643.6695278742335367229@gmail.com>

First patch looks fine, I sent a new patch series to address the issue
pointed out by patch 2, as it was a bit of a bandaid around a larger
issue.

Will push the first patch in just a moment

On Sun, 2026-08-16 at 15:09 +0200, Marek Czernohous wrote:
> From: Marek Czernohous <marek@czernohous.de>
> 
> This is v4 of two NVAC (MCP79) stability fixes.  v3 went out on 11
> June
> and has had no replies:
> 
>  
> https://lore.kernel.org/all/20260611124535.527275-1-mczernohous@gmail.com/
> 
> Nothing in either patch changed.  Same diffs, same reasoning, same
> trailers.  The reason this is a v4 and not a ping is that 2/2 no
> longer
> applies to mainline, so a ping would have pointed at a series nobody
> can
> test.
> 
> What moved was somebody else's context, not this code.
> Commit 5164f7e7ff8e ("drm: Rename struct drm_atomic_state to
> drm_atomic_commit") changed the callback signature, so
> nv50_sor_atomic_disable() now takes a struct drm_atomic_commit
> *state.
> 2/2 is rebased onto that by hand.  1/2 applies unchanged and is
> byte-identical to v3.
> 
> Both bugs are still live in c21bb4193868.  dispnv50/disp.c:1568 still
> does nv50_head(nv_encoder->crtc) with no NULL check, and there is no
> guard anywhere in the function.  Immediately below it, :1570 takes
> nv50_outp_get_old_connector() and :1572 dereferences the result
> without
> checking it.
> 
> 1/2 drm/nouveau/pci: use config-space MSI rearm on MCP79/MCP7A (NVAC)
> 
>     NVAC re-arms MSI through the MMIO mirror of PCI config space,
> which
>     is unreliable on this IGP.  A missed re-arm leaves the interrupt
>     line dead, submission times out and the GPU looks hung until
>     reboot.  Give NVAC its own pci func that re-arms through real
>     config space, the way commit 5112abc6a433 ("drm/nouveau/pci/g92:
>     Fix rearm") already did for g92.  Only NVAC is switched, because
>     that is the part this was validated on.
> 
> 2/2 drm/nouveau/kms: guard NULL crtc in nv50_sor_atomic_disable()
> 
>     Restore the NULL check that
>     commit f575f2bdb6c3 ("drm/nouveau/kms/nv50-: Remove
>     (nv_encoder->crtc) checks in ->disable callbacks") removed, as a
>     drm_WARN_ON_ONCE() rather than a silent return, since a NULL crtc
>     here is a state-tracking inconsistency worth seeing.  The guard
> is
>     hoisted above the backlight teardown so the old connector is only
>     used after it has been checked; without that the oops would just
>     move a few lines down.
> 
> Testing.  The MSI change continues to run here on the Mac mini as a
> daily driver, and Fab Stz confirmed it independently on an iMac9,1.
> Both Tested-by tags are carried over unchanged from v3; the
> diffs they were given are the diffs sent here, apart from the
> mechanical
> rebase of 2/2.  For v4 I rebuilt 2/2 with
> CONFIG_DRM_NOUVEAU_BACKLIGHT
> set to y and to n, because the patch splits the #ifdef block in two
> and
> that is where a change of this shape tends to break.
> 
> If something about the shape of the series is what is holding it up,
> I
> would rather hear that than keep rebasing it.
> 
> Marek Czernohous (2):
>   drm/nouveau/pci: use config-space MSI rearm on MCP79/MCP7A (NVAC)
>   drm/nouveau/kms: guard NULL crtc in nv50_sor_atomic_disable()
> 
>  drivers/gpu/drm/nouveau/dispnv50/disp.c       | 30 +++++++++++++---
>  .../gpu/drm/nouveau/include/nvkm/subdev/pci.h |  1 +
>  .../gpu/drm/nouveau/nvkm/engine/device/base.c |  2 +-
>  .../gpu/drm/nouveau/nvkm/subdev/pci/Kbuild    |  1 +
>  .../gpu/drm/nouveau/nvkm/subdev/pci/mcp79.c   | 35
> +++++++++++++++++++
>  5 files changed, 63 insertions(+), 6 deletions(-)
>  create mode 100644 drivers/gpu/drm/nouveau/nvkm/subdev/pci/mcp79.c
> 
> 
> base-commit: c21bb4193868a8de71fc4693fa741e195fdf5d86


  parent reply	other threads:[~2026-08-19  0:02 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-16 13:09 [PATCH v4 0/2] drm/nouveau: NVAC (MCP79) stability: MSI rearm and a NULL crtc guard Marek Czernohous
2026-08-16 13:09 ` Marek Czernohous
2026-08-16 13:09 ` [PATCH v4 2/2] drm/nouveau/kms: guard NULL crtc in nv50_sor_atomic_disable() Marek Czernohous
2026-08-16 13:09   ` Marek Czernohous
2026-08-16 13:17   ` sashiko-bot
2026-08-16 16:49     ` Marek Czernohous
2026-08-16 16:49       ` Marek Czernohous
2026-08-18 20:44   ` lyude
2026-08-18 20:44     ` lyude
2026-08-18 22:32     ` lyude
2026-08-18 22:32       ` lyude
2026-08-16 13:09 ` [PATCH v4 1/2] drm/nouveau/pci: use config-space MSI rearm on MCP79/MCP7A (NVAC) Marek Czernohous
2026-08-16 13:09   ` Marek Czernohous
2026-08-18 20:28   ` lyude
2026-08-18 20:28     ` lyude
2026-08-19  0:02 ` lyude [this message]
2026-08-19  0:02   ` [PATCH v4 0/2] drm/nouveau: NVAC (MCP79) stability: MSI rearm and a NULL crtc guard lyude

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6244a95cc925d8b0f3c01ac000ca56dad246305b.camel@redhat.com \
    --to=lyude@redhat.com \
    --cc=airlied@gmail.com \
    --cc=dakr@kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mczernohous@gmail.com \
    --cc=nouveau@lists.freedesktop.org \
    --cc=simona@ffwll.ch \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.