* [PATCH v3 1/2] drm/nouveau/pci: use config-space MSI rearm on MCP79/MCP7A (NVAC)
2026-06-11 12:45 [PATCH v3 0/2] drm/nouveau: NVAC (MCP79) MSI rearm + SOR-disable NULL guard Marek Czernohous
@ 2026-06-11 12:45 ` Marek Czernohous
2026-06-11 12:45 ` [PATCH v3 2/2] drm/nouveau/kms: guard NULL crtc in nv50_sor_atomic_disable() Marek Czernohous
1 sibling, 0 replies; 3+ messages in thread
From: Marek Czernohous @ 2026-06-11 12:45 UTC (permalink / raw)
To: Lyude Paul, Danilo Krummrich; +Cc: dri-devel, nouveau, linux-kernel, Fab Stz
From: Marek Czernohous <marek@czernohous.de>
NVAC (MCP79/MCP7A) uses g94_pci_func, whose .msi_rearm is
nv40_pci_msi_rearm(): a re-arm write through the MMIO mirror of PCI
config space. On this IGP that path is unreliable; when a re-arm is
missed the interrupt line stays dead, command submission times out and
the GPU appears hung until reboot. On an Apple Mac mini (early 2009,
MCP79, boot0 0x0ac080b1) this showed as sporadic fifo timeouts and GPU
hangs under load unless MSI was disabled via config=NvMSI=0.
Give NVAC its own pci func that re-arms through real PCI config space
(nv46_pci_msi_rearm) instead. This follows existing precedent: nv46.c
documents the MMIO-mirror re-arm as broken on several related parts,
and commit 5112abc6a433 ("drm/nouveau/pci/g92: Fix rearm") fixed g92
the same way while moving the remaining chipsets, NVAC included, into
the newly added shared g94 table, where NVAC stayed on the MMIO path.
This change completes that fix for NVAC. The sibling IGP NVAA
(MCP77/MCP78) has MSI disabled entirely as "reported broken" in
nvkm_pci_new_(); NVAC works correctly once the re-arm goes through
config space, so disabling MSI is not necessary.
Only NVAC is switched: that is the hardware this has been validated
on. The other users of g94_pci_func (G94/G96/G98/GT2xx and the
MCP77/MCP89 IGPs) keep their current behavior; MCP77 and MCP89
plausibly want the same treatment but were not tested.
Tested on the Mac mini as a daily driver for two months with MSI
enabled and zero fifo timeouts. Independently confirmed stable on an
iMac9,1 (MCP79) running 6.12.90 with the v1 form of this change (the
same one-line functional switch, applied to that kernel's g94
implementation).
Fixes: 5112abc6a433 ("drm/nouveau/pci/g92: Fix rearm")
Cc: <stable@vger.kernel.org> # v6.16+
Tested-by: Fab Stz <fabstz-it@yahoo.fr>
Assisted-by: Claude:claude-opus-4-7
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Marek Czernohous <marek@czernohous.de>
---
The stable tag is annotated v6.16+ because the new file uses the .cfg
member introduced there by 2f89bb3264af ("drm/nouveau/pci: add PRI
address of config space mirror to nvkm_pci_func"); on older kernels
the backport is the era-specific one-line .msi_rearm switch in the g94
implementation (nv46_pci_msi_rearm exists everywhere), happy to send
those per tree if wanted.
v2 -> v3: no code changes; annotated the stable tag, clarified the g92
precedent wording and that the iMac9,1 test used the v1 form, minor
nits (copyright line, declaration style matching the header, boot0
notation).
v1 -> v2: narrowed to NVAC via a dedicated mcp79 pci func instead of
changing the shared g94 table (only chipset the fix was validated on);
added Fixes/stable/Assisted-by tags.
v2: https://lore.kernel.org/all/c5a46ddcc38172b43bc3d7432e8114669f3dc933.1781162589.git.marek@czernohous.de/
v1: https://lore.kernel.org/nouveau/20260409172126.115441-2-marek@czernohous.de/
.../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 +++++++++++++++++++
4 files changed, 38 insertions(+), 1 deletion(-)
create mode 100644 drivers/gpu/drm/nouveau/nvkm/subdev/pci/mcp79.c
diff --git a/drivers/gpu/drm/nouveau/include/nvkm/subdev/pci.h b/drivers/gpu/drm/nouveau/include/nvkm/subdev/pci.h
index 112b674..0172e0d 100644
--- a/drivers/gpu/drm/nouveau/include/nvkm/subdev/pci.h
+++ b/drivers/gpu/drm/nouveau/include/nvkm/subdev/pci.h
@@ -46,6 +46,7 @@ int nv4c_pci_new(struct nvkm_device *, enum nvkm_subdev_type, int inst, struct n
int g84_pci_new(struct nvkm_device *, enum nvkm_subdev_type, int inst, struct nvkm_pci **);
int g92_pci_new(struct nvkm_device *, enum nvkm_subdev_type, int inst, struct nvkm_pci **);
int g94_pci_new(struct nvkm_device *, enum nvkm_subdev_type, int inst, struct nvkm_pci **);
+int mcp79_pci_new(struct nvkm_device *, enum nvkm_subdev_type, int inst, struct nvkm_pci **);
int gf100_pci_new(struct nvkm_device *, enum nvkm_subdev_type, int inst, struct nvkm_pci **);
int gf106_pci_new(struct nvkm_device *, enum nvkm_subdev_type, int inst, struct nvkm_pci **);
int gk104_pci_new(struct nvkm_device *, enum nvkm_subdev_type, int inst, struct nvkm_pci **);
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c b/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c
index b101e14..a809ec3 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c
@@ -1237,7 +1237,7 @@ nvac_chipset = {
.mc = { 0x00000001, g98_mc_new },
.mmu = { 0x00000001, mcp77_mmu_new },
.mxm = { 0x00000001, nv50_mxm_new },
- .pci = { 0x00000001, g94_pci_new },
+ .pci = { 0x00000001, mcp79_pci_new },
.therm = { 0x00000001, g84_therm_new },
.timer = { 0x00000001, nv41_timer_new },
.volt = { 0x00000001, nv40_volt_new },
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/pci/Kbuild b/drivers/gpu/drm/nouveau/nvkm/subdev/pci/Kbuild
index a14ea0f..90f03ba 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/pci/Kbuild
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/pci/Kbuild
@@ -9,6 +9,7 @@ nvkm-y += nvkm/subdev/pci/nv4c.o
nvkm-y += nvkm/subdev/pci/g84.o
nvkm-y += nvkm/subdev/pci/g92.o
nvkm-y += nvkm/subdev/pci/g94.o
+nvkm-y += nvkm/subdev/pci/mcp79.o
nvkm-y += nvkm/subdev/pci/gf100.o
nvkm-y += nvkm/subdev/pci/gf106.o
nvkm-y += nvkm/subdev/pci/gk104.o
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/pci/mcp79.c b/drivers/gpu/drm/nouveau/nvkm/subdev/pci/mcp79.c
new file mode 100644
index 0000000..e2ae242
--- /dev/null
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/pci/mcp79.c
@@ -0,0 +1,35 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright 2026 Marek Czernohous
+ *
+ * MCP79/MCP7A (NVAC): like g94, but MSI re-arm goes through real PCI
+ * config space. The MMIO-mirror re-arm is unreliable on this IGP and a
+ * missed re-arm kills the interrupt line (see the nv46 comment; g92
+ * already re-arms through config space for the same reason).
+ */
+#include "priv.h"
+
+static const struct nvkm_pci_func
+mcp79_pci_func = {
+ .cfg = { .addr = 0x088000, .size = 0x1000 },
+
+ .init = g84_pci_init,
+ .msi_rearm = nv46_pci_msi_rearm,
+
+ .pcie.init = g84_pcie_init,
+ .pcie.set_link = g84_pcie_set_link,
+
+ .pcie.max_speed = g84_pcie_max_speed,
+ .pcie.cur_speed = g84_pcie_cur_speed,
+
+ .pcie.set_version = g84_pcie_set_version,
+ .pcie.version = g84_pcie_version,
+ .pcie.version_supported = g92_pcie_version_supported,
+};
+
+int
+mcp79_pci_new(struct nvkm_device *device, enum nvkm_subdev_type type, int inst,
+ struct nvkm_pci **ppci)
+{
+ return nvkm_pci_new_(&mcp79_pci_func, device, type, inst, ppci);
+}
--
2.53.0
^ permalink raw reply [flat|nested] 3+ messages in thread* [PATCH v3 2/2] drm/nouveau/kms: guard NULL crtc in nv50_sor_atomic_disable()
2026-06-11 12:45 [PATCH v3 0/2] drm/nouveau: NVAC (MCP79) MSI rearm + SOR-disable NULL guard Marek Czernohous
2026-06-11 12:45 ` [PATCH v3 1/2] drm/nouveau/pci: use config-space MSI rearm on MCP79/MCP7A (NVAC) Marek Czernohous
@ 2026-06-11 12:45 ` Marek Czernohous
1 sibling, 0 replies; 3+ messages in thread
From: Marek Czernohous @ 2026-06-11 12:45 UTC (permalink / raw)
To: Lyude Paul, Danilo Krummrich; +Cc: dri-devel, nouveau, linux-kernel, Fab Stz
From: Marek Czernohous <marek@czernohous.de>
nv50_sor_atomic_disable() unconditionally computes
nv50_head(nv_encoder->crtc) and dereferences the result a few lines
later. nv_encoder->crtc is nouveau's own shadow pointer, set in
.atomic_enable and cleared at the end of .atomic_disable.
Commit f575f2bdb6c3 ("drm/nouveau/kms/nv50-: Remove (nv_encoder->crtc)
checks in ->disable callbacks") removed the NULL check here,
reasoning that the atomic helpers never call ->disable without a
crtc. On NVAC (MCP79) under Wayland sessions (observed with Weston's
DRM backend and with labwc/wlroots) we have hit the NULL case in
practice during session teardown and VT switches: disable runs without
(or after) its matching enable, and because nv50_head() is
container_of(), the NULL does not stay NULL but becomes a bogus
non-NULL pointer, so the subsequent head dereferences fault and the
kernel oopses.
Restore the guard, as drm_WARN_ON_ONCE() instead of a silent return:
a NULL crtc here still indicates a state-tracking inconsistency that
should stay visible. Return without touching the output; in this path
either enable never ran (nothing to tear down) or an earlier disable
already did the teardown, and the encoder release is handled by the
commit_tail release loop in both cases. (That loop then rejects the
release of a never-acquired output with -EINVAL in the nvif layer,
which is harmless; the vanilla code oopsed before ever reaching it.)
The same inconsistent-state path can also leave the encoder without an
old connector state, in which case nv50_outp_get_old_connector()
returns NULL while the backlight teardown dereferenced it
unconditionally, so the oops would only have moved there. Hoist the
guard above all of that and look at the old connector only after
checking it.
Fixes: f575f2bdb6c3 ("drm/nouveau/kms/nv50-: Remove (nv_encoder->crtc) checks in ->disable callbacks")
Cc: <stable@vger.kernel.org>
Tested-by: Fab Stz <fabstz-it@yahoo.fr>
Assisted-by: Claude:claude-opus-4-7
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Marek Czernohous <marek@czernohous.de>
---
v2 -> v3: hoisted the guard above the CONFIG_DRM_NOUVEAU_BACKLIGHT
block and made the old-connector lookup NULL-safe (reported by the
Sashiko AI review on the v2 posting: the unconditional
nv_connector->backlight initializer ran before the guard); folded
review nits (pointer wording, release-loop note, comment). Tested-by
carried over; the changes stay confined to the inconsistent-state
path, normal-path behavior is unchanged.
v1 -> v2: dropped the nvif_outp_release() call from the early return
(the release is owned by the commit_tail release loop; calling it here
released twice and detached the OR before the disable flush); turned
the silent return into drm_WARN_ON_ONCE(); reworded the commit message.
v2: https://lore.kernel.org/all/66ea307dd4fa080db27b2b9a0caa31b562d72c2b.1781162589.git.marek@czernohous.de/
v1: https://lore.kernel.org/nouveau/20260409172126.115441-3-marek@czernohous.de/
drivers/gpu/drm/nouveau/dispnv50/disp.c | 30 ++++++++++++++++++++-----
1 file changed, 25 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 6c3a871..47e15a4 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -1565,16 +1565,36 @@ static void
nv50_sor_atomic_disable(struct drm_encoder *encoder, struct drm_atomic_state *state)
{
struct nouveau_encoder *nv_encoder = nouveau_encoder(encoder);
- struct nv50_head *head = nv50_head(nv_encoder->crtc);
+ struct nv50_head *head;
#ifdef CONFIG_DRM_NOUVEAU_BACKLIGHT
- struct nouveau_connector *nv_connector = nv50_outp_get_old_connector(state, nv_encoder);
+ struct nouveau_connector *nv_connector;
struct nouveau_drm *drm = nouveau_drm(nv_encoder->base.base.dev);
- struct nouveau_backlight *backlight = nv_connector->backlight;
- struct drm_dp_aux *aux = &nv_connector->aux;
+ struct nouveau_backlight *backlight;
int ret;
+#endif
+ /* nv_encoder->crtc is the driver's shadow pointer, set in
+ * .atomic_enable (and by the boot-time hardware readback) and
+ * cleared at the end of this function. NULL here
+ * means disable-without-enable or a double disable; bail before
+ * container_of() turns it into a bogus head pointer (checking the
+ * result would not work, container_of(NULL) is never NULL). The
+ * encoder release is handled by the commit_tail release loop, so
+ * there is nothing to clean up here.
+ */
+ if (drm_WARN_ON_ONCE(encoder->dev, !nv_encoder->crtc))
+ return;
+ head = nv50_head(nv_encoder->crtc);
+
+#ifdef CONFIG_DRM_NOUVEAU_BACKLIGHT
+ /* The same inconsistent-state path can leave us without an old
+ * connector state, so check before touching it.
+ */
+ nv_connector = nv50_outp_get_old_connector(state, nv_encoder);
+ backlight = nv_connector ? nv_connector->backlight : NULL;
if (backlight && backlight->uses_dpcd) {
- ret = drm_edp_backlight_disable(aux, &backlight->edp_info);
+ ret = drm_edp_backlight_disable(&nv_connector->aux,
+ &backlight->edp_info);
if (ret < 0)
NV_ERROR(drm, "Failed to disable backlight on [CONNECTOR:%d:%s]: %d\n",
nv_connector->base.base.id, nv_connector->base.name, ret);
--
2.53.0
^ permalink raw reply [flat|nested] 3+ messages in thread