[PATCH 0/5] drm/nouveau: GSP DP control call fixes and two KMS fixes

[PATCH 0/5] drm/nouveau: GSP DP control call fixes and two KMS fixes

[David R. Hedges via B4 Relay]

These turned up while debugging a DP monitor on a GA102 (GSP-RM
570.144) that stays dark after DPMS off [1].  The fix for that is a
separate series for drm-misc-next, based on this one.  These five
don't depend on it, or on each other:

 - 1: r535_gsp_rpc_rm_ctrl_push() reports success with a NULL reply
   when the reply it was waiting for goes missing.  That only happens
   after a WARN in r535_gsp_msgq_recv(); found by reading the code.
 - 2: r535_dp_aux_xfer() has, since v6.12, passed RM's reply fields up
   as a completed AUX transaction even when RM failed the call.  With
   Ubuntu's 7.3 kernel this machine logs bursts of 40-50 such failures
   each time the monitor fails to come back from DPMS off.
 - 3: r535_dp_train_target() dereferences a NULL reply after a GSP RPC
   timeout.  Seen once, turning a display engine hang into an oops.
 - 4: a modeset racing a probe that lost the EDID drives the output at
   0 bpc and trips the WARN in nv50_sor_dp_watermark_sst().  Seen with
   Ubuntu's 7.3 kernel.
 - 5: since v6.2, a plug or unplug event pending together with an
   IRQ_HPD, including the sink count changes nouveau_dp_irq() reports
   that way, skips connector detection.  Found by reading the code; I
   have nothing that changes its sink count to test it with.

Testing, on the GA102 above with Ubuntu's 7.3.0-9-generic kernel and
nouveau built from v7.3 plus this series and the drm-misc-next one:

 - the DPMS, cable replug, S3 suspend/resume and power button tests
   described in the other series, with drm.debug=0x106, and five days
   of normal use: no WARNs, oopses or display engine hangs.  The only
   GSP errors were AUXCH_CTRL failures (0x65) while the monitor was
   switched off, and those now reach the DP helpers as errors ("Too
   many retries, giving up. First error: -22") rather than as
   completed transfers.
 - 1, 3 and 5 deal with conditions I can't reproduce on demand, and the
   0-bpc race 4 fixes didn't recur.

Both series build with W=1 on drm-misc-fixes (this one) and on
drm-misc-next (both), with no new W=1 or sparse warnings in the
touched files.  checkpatch --strict only points out RM's camel-case
replyType field.

This series was developed with the help of Claude Code (Claude Opus
5.5).  It analysed the kernel logs, read the nouveau, DRM and NVIDIA
open-gpu-kernel-modules sources, drafted the patches and changelogs,
and ran the build and checkpatch checks.  I provided evidence and
direction, reviewed changes, and ran the hardware tests.

[1] https://gitlab.freedesktop.org/drm/nouveau/-/work_items/515

Signed-off-by: David R. Hedges <linuxdev844@thelittleman.net>
---
David R. Hedges (5):
      drm/nouveau/gsp: fail control calls whose reply went missing
      drm/nouveau/gsp: don't report failed DP AUX transfers as completed
      drm/nouveau/gsp: don't dereference a failed DP_CTRL reply
      drm/nouveau/kms/nv50-: don't drive outputs at 0 bpc
      drm/nouveau: don't let an IRQ_HPD hide a plug or unplug event

 drivers/gpu/drm/nouveau/dispnv50/disp.c                |  7 +++++--
 drivers/gpu/drm/nouveau/nouveau_display.c              |  7 ++++++-
 drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/ctrl.c |  3 +++
 drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c | 11 +++++++----
 4 files changed, 21 insertions(+), 7 deletions(-)
---
base-commit: 54a88d9afcca66221d14f438d0960472984968ec
change-id: 20261103-nouveau-dp-fixes-4e5472679f13

Best regards,
--  
David R. Hedges <linuxdev844@thelittleman.net>

[PATCH 0/5] drm/nouveau: GSP DP control call fixes and two KMS fixes

[David R. Hedges]

These turned up while debugging a DP monitor on a GA102 (GSP-RM
570.144) that stays dark after DPMS off [1].  The fix for that is a
separate series for drm-misc-next, based on this one.  These five
don't depend on it, or on each other:

 - 1: r535_gsp_rpc_rm_ctrl_push() reports success with a NULL reply
   when the reply it was waiting for goes missing.  That only happens
   after a WARN in r535_gsp_msgq_recv(); found by reading the code.
 - 2: r535_dp_aux_xfer() has, since v6.12, passed RM's reply fields up
   as a completed AUX transaction even when RM failed the call.  With
   Ubuntu's 7.3 kernel this machine logs bursts of 40-50 such failures
   each time the monitor fails to come back from DPMS off.
 - 3: r535_dp_train_target() dereferences a NULL reply after a GSP RPC
   timeout.  Seen once, turning a display engine hang into an oops.
 - 4: a modeset racing a probe that lost the EDID drives the output at
   0 bpc and trips the WARN in nv50_sor_dp_watermark_sst().  Seen with
   Ubuntu's 7.3 kernel.
 - 5: since v6.2, a plug or unplug event pending together with an
   IRQ_HPD, including the sink count changes nouveau_dp_irq() reports
   that way, skips connector detection.  Found by reading the code; I
   have nothing that changes its sink count to test it with.

Testing, on the GA102 above with Ubuntu's 7.3.0-9-generic kernel and
nouveau built from v7.3 plus this series and the drm-misc-next one:

 - the DPMS, cable replug, S3 suspend/resume and power button tests
   described in the other series, with drm.debug=0x106, and five days
   of normal use: no WARNs, oopses or display engine hangs.  The only
   GSP errors were AUXCH_CTRL failures (0x65) while the monitor was
   switched off, and those now reach the DP helpers as errors ("Too
   many retries, giving up. First error: -22") rather than as
   completed transfers.
 - 1, 3 and 5 deal with conditions I can't reproduce on demand, and the
   0-bpc race 4 fixes didn't recur.

Both series build with W=1 on drm-misc-fixes (this one) and on
drm-misc-next (both), with no new W=1 or sparse warnings in the
touched files.  checkpatch --strict only points out RM's camel-case
replyType field.

This series was developed with the help of Claude Code (Claude Opus
5.5).  It analysed the kernel logs, read the nouveau, DRM and NVIDIA
open-gpu-kernel-modules sources, drafted the patches and changelogs,
and ran the build and checkpatch checks.  I provided evidence and
direction, reviewed changes, and ran the hardware tests.

[1] https://gitlab.freedesktop.org/drm/nouveau/-/work_items/515

Signed-off-by: David R. Hedges <linuxdev844@thelittleman.net>
---
David R. Hedges (5):
      drm/nouveau/gsp: fail control calls whose reply went missing
      drm/nouveau/gsp: don't report failed DP AUX transfers as completed
      drm/nouveau/gsp: don't dereference a failed DP_CTRL reply
      drm/nouveau/kms/nv50-: don't drive outputs at 0 bpc
      drm/nouveau: don't let an IRQ_HPD hide a plug or unplug event

 drivers/gpu/drm/nouveau/dispnv50/disp.c                |  7 +++++--
 drivers/gpu/drm/nouveau/nouveau_display.c              |  7 ++++++-
 drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/ctrl.c |  3 +++
 drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c | 11 +++++++----
 4 files changed, 21 insertions(+), 7 deletions(-)
---
base-commit: 54a88d9afcca66221d14f438d0960472984968ec
change-id: 20261103-nouveau-dp-fixes-4e5472679f13

Best regards,
--  
David R. Hedges <linuxdev844@thelittleman.net>

[PATCH 1/5] drm/nouveau/gsp: fail control calls whose reply went missing

[David R. Hedges via B4 Relay]

From: "David R. Hedges" <linuxdev844@thelittleman.net>

r535_gsp_rpc_rm_ctrl_push() sets *params to NULL and returns
PTR_ERR(rpc) when nvkm_gsp_rpc_push() returns an error or NULL.  NULL
is the normal result when no reply was asked for (repc == 0), but
r535_gsp_msgq_recv() also returns NULL, after a WARN, when the message
at the head of the queue claims to be longer than a message can be.
PTR_ERR(NULL) is 0, so the caller sees success with a NULL reply:
nvkm_gsp_rm_ctrl_rd() hands the NULL back as if it were valid, and
r535_dp_aux_xfer() and r535_dp_train_target() go on to read their
reply fields.

Return -EIO when a reply was expected and none came back.

Fixes: 4ae3a20102b2 ("nouveau/gsp: don't free ctrl messages on errors")
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: David R. Hedges <linuxdev844@thelittleman.net>
---
 drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/ctrl.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/ctrl.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/ctrl.c
index 70b9ee911c5e..ea925a9d0531 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/ctrl.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/ctrl.c
@@ -44,6 +44,9 @@ r535_gsp_rpc_rm_ctrl_push(struct nvkm_gsp_object *object, void **params, u32 rep
 	rpc = nvkm_gsp_rpc_push(gsp, rpc, NVKM_GSP_RPC_REPLY_RECV, repc);
 	if (IS_ERR_OR_NULL(rpc)) {
 		*params = NULL;
+		/* NULL is only a successful reply if none was asked for. */
+		if (!rpc && repc)
+			return -EIO;
 		return PTR_ERR(rpc);
 	}
 

-- 
2.55.0

[PATCH 1/5] drm/nouveau/gsp: fail control calls whose reply went missing

[David R. Hedges]

r535_gsp_rpc_rm_ctrl_push() sets *params to NULL and returns
PTR_ERR(rpc) when nvkm_gsp_rpc_push() returns an error or NULL.  NULL
is the normal result when no reply was asked for (repc == 0), but
r535_gsp_msgq_recv() also returns NULL, after a WARN, when the message
at the head of the queue claims to be longer than a message can be.
PTR_ERR(NULL) is 0, so the caller sees success with a NULL reply:
nvkm_gsp_rm_ctrl_rd() hands the NULL back as if it were valid, and
r535_dp_aux_xfer() and r535_dp_train_target() go on to read their
reply fields.

Return -EIO when a reply was expected and none came back.

Fixes: 4ae3a20102b2 ("nouveau/gsp: don't free ctrl messages on errors")
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: David R. Hedges <linuxdev844@thelittleman.net>
---
 drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/ctrl.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/ctrl.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/ctrl.c
index 70b9ee911c5e..ea925a9d0531 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/ctrl.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/ctrl.c
@@ -44,6 +44,9 @@ r535_gsp_rpc_rm_ctrl_push(struct nvkm_gsp_object *object, void **params, u32 rep
 	rpc = nvkm_gsp_rpc_push(gsp, rpc, NVKM_GSP_RPC_REPLY_RECV, repc);
 	if (IS_ERR_OR_NULL(rpc)) {
 		*params = NULL;
+		/* NULL is only a successful reply if none was asked for. */
+		if (!rpc && repc)
+			return -EIO;
 		return PTR_ERR(rpc);
 	}
 

-- 
2.55.0

[PATCH 2/5] drm/nouveau/gsp: don't report failed DP AUX transfers as completed

[David R. Hedges via B4 Relay]

From: "David R. Hedges" <linuxdev844@thelittleman.net>

Commit 9776c0a75a1a ("nouveau/dp: handle retries for AUX CH transfers
with GSP.") put the NV0073_CTRL_CMD_DP_AUXCH_CTRL call in a retry loop
and dropped the check for the call failing.  When RM fails it with
anything other than a retry request, r535_dp_aux_xfer() ignores the
error and returns the reply fields as if a transaction had completed:
replyType becomes the AUX reply, and size and data are copied out.
NVIDIA's own DisplayPort library (EvoAuxBus::transaction() in
open-gpu-kernel-modules) treats a failed AUXCH_CTRL call as a NACK and
ignores the reply.

RM does fail these calls.  On a GA102 with GSP-RM 570.144, replugging
a DP monitor that hasn't come back from DPMS off produces bursts of

  gsp: cli:0xc1d00001 obj:0x00730000 ctrl cmd:0x00731341 failed: 0x0000ffff

(NV_ERR_GENERIC, occasionally NV_ERR_INVALID_DATA), one per AUX
transaction, and the DPCD and EDID reads made during a burst are
judged on whatever the reply fields happen to contain.

Return the error from the control call again, as r535_dp_aux_xfer()
did before the retry loop was added.

Fixes: 9776c0a75a1a ("nouveau/dp: handle retries for AUX CH transfers with GSP.")
Cc: stable@vger.kernel.org
Link: https://gitlab.freedesktop.org/drm/nouveau/-/work_items/515
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: David R. Hedges <linuxdev844@thelittleman.net>
---
 drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c
index f5f22173fc2c..953392504413 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c
@@ -1101,9 +1101,12 @@ r535_dp_aux_xfer(struct nvkm_outp *outp, u8 type, u32 addr, u8 *data, u8 *psize)
 			msleep(ctrl->retryTimeMs);
 			nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl);
 		} else {
-			memcpy(data, ctrl->data, size);
-			*psize = ctrl->size;
-			ret = ctrl->replyType;
+			/* The reply only describes a transaction if RM completed one. */
+			if (ret == 0) {
+				memcpy(data, ctrl->data, size);
+				*psize = ctrl->size;
+				ret = ctrl->replyType;
+			}
 			nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl);
 			break;
 		}

-- 
2.55.0

[PATCH 2/5] drm/nouveau/gsp: don't report failed DP AUX transfers as completed

[David R. Hedges]

Commit 9776c0a75a1a ("nouveau/dp: handle retries for AUX CH transfers
with GSP.") put the NV0073_CTRL_CMD_DP_AUXCH_CTRL call in a retry loop
and dropped the check for the call failing.  When RM fails it with
anything other than a retry request, r535_dp_aux_xfer() ignores the
error and returns the reply fields as if a transaction had completed:
replyType becomes the AUX reply, and size and data are copied out.
NVIDIA's own DisplayPort library (EvoAuxBus::transaction() in
open-gpu-kernel-modules) treats a failed AUXCH_CTRL call as a NACK and
ignores the reply.

RM does fail these calls.  On a GA102 with GSP-RM 570.144, replugging
a DP monitor that hasn't come back from DPMS off produces bursts of

  gsp: cli:0xc1d00001 obj:0x00730000 ctrl cmd:0x00731341 failed: 0x0000ffff

(NV_ERR_GENERIC, occasionally NV_ERR_INVALID_DATA), one per AUX
transaction, and the DPCD and EDID reads made during a burst are
judged on whatever the reply fields happen to contain.

Return the error from the control call again, as r535_dp_aux_xfer()
did before the retry loop was added.

Fixes: 9776c0a75a1a ("nouveau/dp: handle retries for AUX CH transfers with GSP.")
Cc: stable@vger.kernel.org
Link: https://gitlab.freedesktop.org/drm/nouveau/-/work_items/515
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: David R. Hedges <linuxdev844@thelittleman.net>
---
 drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c
index f5f22173fc2c..953392504413 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c
@@ -1101,9 +1101,12 @@ r535_dp_aux_xfer(struct nvkm_outp *outp, u8 type, u32 addr, u8 *data, u8 *psize)
 			msleep(ctrl->retryTimeMs);
 			nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl);
 		} else {
-			memcpy(data, ctrl->data, size);
-			*psize = ctrl->size;
-			ret = ctrl->replyType;
+			/* The reply only describes a transaction if RM completed one. */
+			if (ret == 0) {
+				memcpy(data, ctrl->data, size);
+				*psize = ctrl->size;
+				ret = ctrl->replyType;
+			}
 			nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl);
 			break;
 		}

-- 
2.55.0

[PATCH 3/5] drm/nouveau/gsp: don't dereference a failed DP_CTRL reply

[David R. Hedges via B4 Relay]

From: "David R. Hedges" <linuxdev844@thelittleman.net>

Commit eb284f4b3781 ("drm/nouveau/dp: Honor GSP link training retry
timeouts") put the NV0073_CTRL_CMD_DP_CTRL call in
r535_dp_train_target() in a retry loop and dropped the check for the
call failing.  The reply's err field is now read whatever
nvkm_gsp_rm_ctrl_push() returned, but when the GSP RPC itself fails,
a timeout for instance, the reply is NULL.

On a GA102 with GSP-RM 570.144 this turned a display hang into an
oops.  A modeset hit "core notifier timeout", and releasing the output
afterwards timed out waiting for GSP:

  WARNING: nvkm/subdev/gsp/rm/r535/rpc.c:164 at r535_gsp_msgq_wait+0xb1/0xc0 [nouveau]
  ...
  BUG: kernel NULL pointer dereference, address: 0000000000000010
  RIP: 0010:r535_dp_train_target+0x1db/0x220 [nouveau]
  Call Trace:
   r535_dp_release+0x73/0xe0 [nouveau]
   nvkm_uoutp_mthd+0xaf0/0xc70 [nouveau]
   ...
   nvif_outp_release+0x1b/0xb0 [nouveau]
   nv50_disp_atomic_commit_core+0x5ba/0x6b0 [nouveau]

0x10 is the offset of err in NV0073_CTRL_DP_CTRL_PARAMS.  Only look at
err if the control call succeeded.

Fixes: eb284f4b3781 ("drm/nouveau/dp: Honor GSP link training retry timeouts")
Cc: stable@vger.kernel.org
Link: https://gitlab.freedesktop.org/drm/nouveau/-/work_items/515
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: David R. Hedges <linuxdev844@thelittleman.net>
---
 drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c
index 953392504413..028f342e020b 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c
@@ -1006,7 +1006,7 @@ r535_dp_train_target(struct nvkm_outp *outp, u8 target, bool mst, u8 link_nr, u8
 			nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl);
 		} else {
 			/* GSP didn't say to retry, or we were successful */
-			if (ctrl->err)
+			if (ret == 0 && ctrl->err)
 				ret = -EIO;
 			nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl);
 			break;

-- 
2.55.0

[PATCH 3/5] drm/nouveau/gsp: don't dereference a failed DP_CTRL reply

[David R. Hedges]

Commit eb284f4b3781 ("drm/nouveau/dp: Honor GSP link training retry
timeouts") put the NV0073_CTRL_CMD_DP_CTRL call in
r535_dp_train_target() in a retry loop and dropped the check for the
call failing.  The reply's err field is now read whatever
nvkm_gsp_rm_ctrl_push() returned, but when the GSP RPC itself fails,
a timeout for instance, the reply is NULL.

On a GA102 with GSP-RM 570.144 this turned a display hang into an
oops.  A modeset hit "core notifier timeout", and releasing the output
afterwards timed out waiting for GSP:

  WARNING: nvkm/subdev/gsp/rm/r535/rpc.c:164 at r535_gsp_msgq_wait+0xb1/0xc0 [nouveau]
  ...
  BUG: kernel NULL pointer dereference, address: 0000000000000010
  RIP: 0010:r535_dp_train_target+0x1db/0x220 [nouveau]
  Call Trace:
   r535_dp_release+0x73/0xe0 [nouveau]
   nvkm_uoutp_mthd+0xaf0/0xc70 [nouveau]
   ...
   nvif_outp_release+0x1b/0xb0 [nouveau]
   nv50_disp_atomic_commit_core+0x5ba/0x6b0 [nouveau]

0x10 is the offset of err in NV0073_CTRL_DP_CTRL_PARAMS.  Only look at
err if the control call succeeded.

Fixes: eb284f4b3781 ("drm/nouveau/dp: Honor GSP link training retry timeouts")
Cc: stable@vger.kernel.org
Link: https://gitlab.freedesktop.org/drm/nouveau/-/work_items/515
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: David R. Hedges <linuxdev844@thelittleman.net>
---
 drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c
index 953392504413..028f342e020b 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c
@@ -1006,7 +1006,7 @@ r535_dp_train_target(struct nvkm_outp *outp, u8 target, bool mst, u8 link_nr, u8
 			nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl);
 		} else {
 			/* GSP didn't say to retry, or we were successful */
-			if (ctrl->err)
+			if (ret == 0 && ctrl->err)
 				ret = -EIO;
 			nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl);
 			break;

-- 
2.55.0

[PATCH 4/5] drm/nouveau/kms/nv50-: don't drive outputs at 0 bpc

[David R. Hedges via B4 Relay]

From: "David R. Hedges" <linuxdev844@thelittleman.net>

When nouveau_connector_detect() can't read an EDID it clears the
connector's EDID, which resets display_info.bpc to 0.  A default is
only filled back in by nouveau_connector_detect_depth(), from
->get_modes(), so if the HPD worker's probe loses the EDID and
userspace commits a modeset before it next fetches the mode list,
nv50_outp_atomic_check() sets or.bpc to 0.

For DP, nouveau_dp_train() then computes a minimum link rate of 0, and
nv50_sor_dp_watermark_sst() computes 0 symbols per line and fires

  WARNING: drivers/gpu/drm/nouveau/dispnv50/disp.c:1678 at nv50_sor_dp_watermark_sst.isra.0+0x2ab/0x2c0 [nouveau]

This was seen on a GA102 right after "DDC responded, but no EDID for
DP-3" from the HPD worker, while replugging a DP monitor.

Fall back to 8 bpc when the connector doesn't provide a value, which is
nouveau_connector_detect_depth()'s own default for outputs that are
neither eDP nor LVDS, and do the same in nv50_msto_atomic_check(),
which takes or.bpc from the MST connector the same way.  Calling
nouveau_connector_detect_depth() from ->detect() instead isn't an
option, as for LVDS it depends on the native mode that ->get_modes()
sets up.

Fixes: ac2d9275f371 ("drm/nouveau/kms/nv50-: Store the bpc we're using in nv50_head_atom")
Cc: stable@vger.kernel.org
Link: https://gitlab.freedesktop.org/drm/nouveau/-/work_items/515
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: David R. Hedges <linuxdev844@thelittleman.net>
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 2c66e480b511..a265fd971c99 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -418,8 +418,11 @@ nv50_outp_atomic_check(struct drm_encoder *encoder,
 	if (ret)
 		return ret;
 
+	/* display_info.bpc is reset to 0 when a probe loses the EDID, and
+	 * isn't given a default again until ->get_modes() runs.
+	 */
 	if (crtc_state->mode_changed || crtc_state->connectors_changed)
-		asyh->or.bpc = connector->display_info.bpc;
+		asyh->or.bpc = connector->display_info.bpc ?: 8;
 
 	/* We might have to reduce the bpc */
 	nv50_outp_atomic_fix_depth(encoder, crtc_state);
@@ -989,7 +992,7 @@ nv50_msto_atomic_check(struct drm_encoder *encoder,
 	if (!state->duplicated) {
 		const int clock = crtc_state->adjusted_mode.clock;
 
-		asyh->or.bpc = connector->display_info.bpc;
+		asyh->or.bpc = connector->display_info.bpc ?: 8;
 		asyh->dp.pbn = drm_dp_calc_pbn_mode(clock, asyh->or.bpc * 3 << 4);
 	}
 

-- 
2.55.0

[PATCH 4/5] drm/nouveau/kms/nv50-: don't drive outputs at 0 bpc

[David R. Hedges]

When nouveau_connector_detect() can't read an EDID it clears the
connector's EDID, which resets display_info.bpc to 0.  A default is
only filled back in by nouveau_connector_detect_depth(), from
->get_modes(), so if the HPD worker's probe loses the EDID and
userspace commits a modeset before it next fetches the mode list,
nv50_outp_atomic_check() sets or.bpc to 0.

For DP, nouveau_dp_train() then computes a minimum link rate of 0, and
nv50_sor_dp_watermark_sst() computes 0 symbols per line and fires

  WARNING: drivers/gpu/drm/nouveau/dispnv50/disp.c:1678 at nv50_sor_dp_watermark_sst.isra.0+0x2ab/0x2c0 [nouveau]

This was seen on a GA102 right after "DDC responded, but no EDID for
DP-3" from the HPD worker, while replugging a DP monitor.

Fall back to 8 bpc when the connector doesn't provide a value, which is
nouveau_connector_detect_depth()'s own default for outputs that are
neither eDP nor LVDS, and do the same in nv50_msto_atomic_check(),
which takes or.bpc from the MST connector the same way.  Calling
nouveau_connector_detect_depth() from ->detect() instead isn't an
option, as for LVDS it depends on the native mode that ->get_modes()
sets up.

Fixes: ac2d9275f371 ("drm/nouveau/kms/nv50-: Store the bpc we're using in nv50_head_atom")
Cc: stable@vger.kernel.org
Link: https://gitlab.freedesktop.org/drm/nouveau/-/work_items/515
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: David R. Hedges <linuxdev844@thelittleman.net>
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 2c66e480b511..a265fd971c99 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -418,8 +418,11 @@ nv50_outp_atomic_check(struct drm_encoder *encoder,
 	if (ret)
 		return ret;
 
+	/* display_info.bpc is reset to 0 when a probe loses the EDID, and
+	 * isn't given a default again until ->get_modes() runs.
+	 */
 	if (crtc_state->mode_changed || crtc_state->connectors_changed)
-		asyh->or.bpc = connector->display_info.bpc;
+		asyh->or.bpc = connector->display_info.bpc ?: 8;
 
 	/* We might have to reduce the bpc */
 	nv50_outp_atomic_fix_depth(encoder, crtc_state);
@@ -989,7 +992,7 @@ nv50_msto_atomic_check(struct drm_encoder *encoder,
 	if (!state->duplicated) {
 		const int clock = crtc_state->adjusted_mode.clock;
 
-		asyh->or.bpc = connector->display_info.bpc;
+		asyh->or.bpc = connector->display_info.bpc ?: 8;
 		asyh->dp.pbn = drm_dp_calc_pbn_mode(clock, asyh->or.bpc * 3 << 4);
 	}
 

-- 
2.55.0

Re: [PATCH 4/5] drm/nouveau/kms/nv50-: don't drive outputs at 0 bpc

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/nouveau/kms/nv50-: 8 bpc fallback in nv50_outp_atomic_check() breaks 6 bpc eDP and LVDS
--

commit 7d97154d2ddf364be90f072cc11c87de1735db87
Author: David R. Hedges <linuxdev844@thelittleman.net>

drm/nouveau/kms/nv50-: don't drive outputs at 0 bpc

This commit falls back to 8 bpc in nv50_outp_atomic_check() and
nv50_msto_atomic_check() when connector->display_info.bpc is 0 so that outputs
are not driven at 0 bpc if a modeset happens after a probe clears or resets
display_info.bpc before ->get_modes() runs.

Link: https://gitlab.freedesktop.org/drm/nouveau/-/work_items/515

> diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> index 2c66e480b5116..a265fd971c99c 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> @@ -418,8 +418,11 @@ nv50_outp_atomic_check(struct drm_encoder *encoder,
>  	if (ret)
>  		return ret;
>  
> +	/* display_info.bpc is reset to 0 when a probe loses the EDID, and
> +	 * isn't given a default again until ->get_modes() runs.
> +	 */
>  	if (crtc_state->mode_changed || crtc_state->connectors_changed)
> -		asyh->or.bpc = connector->display_info.bpc;
> +		asyh->or.bpc = connector->display_info.bpc ?: 8;

[Severity: Medium]
Could unconditionally defaulting asyh->or.bpc to 8 here break 6 bpc eDP and
18-bit (6 bpc) LVDS panels?

Whenever nouveau_connector_detect() or nouveau_connector_detect_lvds() runs
without a subsequent ->get_modes() call, drm_connector_update_edid_property()
calls drm_edid_connector_update() -> update_display_info() ->
drm_reset_display_info(), which resets connector->display_info.bpc to 0.
For example, on system or runtime resume, nouveau_display_hpd_resume()
schedules nouveau_display_hpd_work() which calls drm_helper_probe_detect() on
every connector; when an eDP or LVDS panel stays connected, its epoch_counter
does not change, so no hotplug event is sent and ->get_modes() does not run.
For EDID 1.3 panels (standard on LVDS and Apple eDP panels), EDID 1.4 panels
with undefined depth, or failed EDID reads, display_info.bpc remains 0 after
->detect().

In drivers/gpu/drm/nouveau/nouveau_connector.c,
nouveau_connector_detect_depth() defaults eDP and LVDS to 6 bpc (unless LVDS
VBIOS flags specify 24-bit):

	/* EDID 1.4 is *supposed* to be supported on eDP, but, Apple... */
	if (nv_connector->type == DCB_CONNECTOR_eDP) {
		connector->display_info.bpc = 6;
		return;
	}

	/* we're out of options unless we're LVDS, default to 8bpc */
	if (nv_encoder->dcb->type != DCB_OUTPUT_LVDS) {
		connector->display_info.bpc = 8;
		return;
	}

	connector->display_info.bpc = 6;

When a modeset is committed while display_info.bpc is 0, setting asyh->or.bpc
to 8 in nv50_outp_atomic_check() causes nv50_sor_atomic_enable() in
drivers/gpu/drm/nouveau/dispnv50/disp.c to override
bios->fp.strapless_is_24bit and force 24-bit LVDS mode on 18-bit LVDS panels:

			if (lvds_dual) {
				if (bios->fp.strapless_is_24bit & 2)
					lvds_8bpc = true;
			} else {
				if (bios->fp.strapless_is_24bit & 1)
					lvds_8bpc = true;
			}

			if (asyh->or.bpc == 8)
				lvds_8bpc = true;
		}

		nvif_outp_lvds(&nv_encoder->outp, lvds_dual, lvds_8bpc);

It also causes eDP links to be trained and driven at 8 bpc instead of 6 bpc,
and prevents nv50_head_atomic_check_dither() in
drivers/gpu/drm/nouveau/dispnv50/head.c from enabling 6 bpc dithering on
24-bit framebuffers:

		if (asyc->dither.mode == DITHERING_MODE_AUTO) {
			if (asyh->base.depth > asyh->or.bpc * 3)
				mode = DITHERING_MODE_DYNAMIC2X2;
		} else {
			mode = asyc->dither.mode;
		}

		if (asyc->dither.depth == DITHERING_DEPTH_AUTO) {
			if (asyh->or.bpc >= 8)
				mode |= DITHERING_DEPTH_8BPC;
		}

Should the fallback in nv50_outp_atomic_check() account for eDP and LVDS
connectors, or should nouveau_connector_detect_depth() also be invoked once
nv_connector->native_mode is available?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103-nouveau-dp-fixes-v1-0-892f4bd357cd@thelittleman.net?part=4

Re: [PATCH 4/5] drm/nouveau/kms/nv50-: don't drive outputs at 0 bpc

[David R. Hedges]

On 2026-11-03 13:41 -0600, sashiko-bot@kernel.org wrote:
> Could unconditionally defaulting asyh->or.bpc to 8 here break 6 bpc eDP 
> and
> 18-bit (6 bpc) LVDS panels?

Yes. ->detect() replaces the EDID on every probe, which resets
display_info.bpc, and for a panel whose EDID doesn't give a color depth
it stays 0 until ->get_modes() runs, so this isn't limited to a lost
EDID. For those panels, nouveau_connector_detect_depth() picks 6 bpc on
eDP and starts from 6 bpc on LVDS. Forcing 8 would drive eDP panels at
24 bpp and make nv50_sor_atomic_enable() override the VBIOS straps on
18-bit LVDS panels.

v2 will fall back to 6 bpc for eDP and LVDS and 8 bpc for everything
else, the same defaults nouveau_connector_detect_depth() uses before it
looks at the straps. On LVDS, that leaves the 18/24-bit choice to the
straps as 0 did, and eDP gets the depth ->get_modes() would give it
instead of 0, which trips the same watermark WARN as on DP. Calling
nouveau_connector_detect_depth() from ->detect() doesn't work for LVDS,
which needs the native mode that ->get_modes() sets up.

Re: [PATCH 4/5] drm/nouveau/kms/nv50-: don't drive outputs at 0 bpc

[David R. Hedges]

On 2026-11-03 13:41 -0600, sashiko-bot@kernel.org wrote:
> Could unconditionally defaulting asyh->or.bpc to 8 here break 6 bpc eDP 
> and
> 18-bit (6 bpc) LVDS panels?

Yes. ->detect() replaces the EDID on every probe, which resets
display_info.bpc, and for a panel whose EDID doesn't give a color depth
it stays 0 until ->get_modes() runs, so this isn't limited to a lost
EDID. For those panels, nouveau_connector_detect_depth() picks 6 bpc on
eDP and starts from 6 bpc on LVDS. Forcing 8 would drive eDP panels at
24 bpp and make nv50_sor_atomic_enable() override the VBIOS straps on
18-bit LVDS panels.

v2 will fall back to 6 bpc for eDP and LVDS and 8 bpc for everything
else, the same defaults nouveau_connector_detect_depth() uses before it
looks at the straps. On LVDS, that leaves the 18/24-bit choice to the
straps as 0 did, and eDP gets the depth ->get_modes() would give it
instead of 0, which trips the same watermark WARN as on DP. Calling
nouveau_connector_detect_depth() from ->detect() doesn't work for LVDS,
which needs the native mode that ->get_modes() sets up.

[PATCH 5/5] drm/nouveau: don't let an IRQ_HPD hide a plug or unplug event

[David R. Hedges via B4 Relay]

From: "David R. Hedges" <linuxdev844@thelittleman.net>

nouveau_dp_irq() passes an IRQ_HPD to the HPD worker as
NVIF_CONN_EVENT_V0_IRQ, adding NVIF_CONN_EVENT_V0_PLUG when the sink
count has changed, or NVIF_CONN_EVENT_V0_UNPLUG when servicing an MST
hub fails.  A real plug or unplug can also be pending in the same bits
by the time the worker runs.  nouveau_display_hpd_work() skips
connector detection for anything with the IRQ bit set, though, as long
as nouveau_dp_link_check() reports the link as fine, which it does
whenever there's no trained link to look at, so those plug and unplug
events are dropped.

Only take the link check shortcut for an IRQ_HPD on its own.

Fixes: 016dacb60e6d ("drm/nouveau/kms: pass event mask to hpd handler")
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: David R. Hedges <linuxdev844@thelittleman.net>
---
 drivers/gpu/drm/nouveau/nouveau_display.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
index d71dcfc6ee66..4398cf0fc541 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -462,7 +462,12 @@ nouveau_display_hpd_work(struct work_struct *work)
 			    !!(bits & NVIF_CONN_EVENT_V0_UNPLUG),
 			    !!(bits & NVIF_CONN_EVENT_V0_IRQ));
 
-		if (bits & NVIF_CONN_EVENT_V0_IRQ) {
+		/* A plug or unplug event pending alongside the IRQ, such as a
+		 * sink count change reported by nouveau_dp_irq(), still needs
+		 * connector detection.
+		 */
+		if ((bits & NVIF_CONN_EVENT_V0_IRQ) &&
+		    !(bits & (NVIF_CONN_EVENT_V0_PLUG | NVIF_CONN_EVENT_V0_UNPLUG))) {
 			if (nouveau_dp_link_check(nv_connector))
 				continue;
 		}

-- 
2.55.0

[PATCH 5/5] drm/nouveau: don't let an IRQ_HPD hide a plug or unplug event

[David R. Hedges]

nouveau_dp_irq() passes an IRQ_HPD to the HPD worker as
NVIF_CONN_EVENT_V0_IRQ, adding NVIF_CONN_EVENT_V0_PLUG when the sink
count has changed, or NVIF_CONN_EVENT_V0_UNPLUG when servicing an MST
hub fails.  A real plug or unplug can also be pending in the same bits
by the time the worker runs.  nouveau_display_hpd_work() skips
connector detection for anything with the IRQ bit set, though, as long
as nouveau_dp_link_check() reports the link as fine, which it does
whenever there's no trained link to look at, so those plug and unplug
events are dropped.

Only take the link check shortcut for an IRQ_HPD on its own.

Fixes: 016dacb60e6d ("drm/nouveau/kms: pass event mask to hpd handler")
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: David R. Hedges <linuxdev844@thelittleman.net>
---
 drivers/gpu/drm/nouveau/nouveau_display.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
index d71dcfc6ee66..4398cf0fc541 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -462,7 +462,12 @@ nouveau_display_hpd_work(struct work_struct *work)
 			    !!(bits & NVIF_CONN_EVENT_V0_UNPLUG),
 			    !!(bits & NVIF_CONN_EVENT_V0_IRQ));
 
-		if (bits & NVIF_CONN_EVENT_V0_IRQ) {
+		/* A plug or unplug event pending alongside the IRQ, such as a
+		 * sink count change reported by nouveau_dp_irq(), still needs
+		 * connector detection.
+		 */
+		if ((bits & NVIF_CONN_EVENT_V0_IRQ) &&
+		    !(bits & (NVIF_CONN_EVENT_V0_PLUG | NVIF_CONN_EVENT_V0_UNPLUG))) {
 			if (nouveau_dp_link_check(nv_connector))
 				continue;
 		}

-- 
2.55.0

Re: [PATCH 5/5] drm/nouveau: don't let an IRQ_HPD hide a plug or unplug event

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] drm/nouveau: lost plug/unplug bits in nouveau_connector_hpd() when pending
- [Medium] drm/nouveau: nouveau_display_hpd_work() skips link check on IRQ with plug
--

commit 8aa36f0d839e814a361fc038f601698183c285f9
Author: David R. Hedges <linuxdev844@thelittleman.net>

drm/nouveau: don't let an IRQ_HPD hide a plug or unplug event

This commit updates nouveau_display_hpd_work() so that the
nouveau_dp_link_check() shortcut is only taken when NVIF_CONN_EVENT_V0_IRQ is
set without NVIF_CONN_EVENT_V0_PLUG or NVIF_CONN_EVENT_V0_UNPLUG. This
prevents plug or unplug events queued alongside an IRQ_HPD from skipping
connector detection.

> diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
> index d71dcfc6ee66b..4398cf0fc541c 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_display.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_display.c
> @@ -462,7 +462,12 @@ nouveau_display_hpd_work(struct work_struct *work)
>  			    !!(bits & NVIF_CONN_EVENT_V0_UNPLUG),
>  			    !!(bits & NVIF_CONN_EVENT_V0_IRQ));
>  
> -		if (bits & NVIF_CONN_EVENT_V0_IRQ) {
> +		/* A plug or unplug event pending alongside the IRQ, such as a
> +		 * sink count change reported by nouveau_dp_irq(), still needs
> +		 * connector detection.
> +		 */
> +		if ((bits & NVIF_CONN_EVENT_V0_IRQ) &&
> +		    !(bits & (NVIF_CONN_EVENT_V0_PLUG | NVIF_CONN_EVENT_V0_UNPLUG))) {

[Severity: Medium]
Can a separate plug or unplug event still be dropped if it arrives while an
earlier NVIF_CONN_EVENT_V0_IRQ is already pending for the same connector?

In drivers/gpu/drm/nouveau/nouveau_connector.c:nouveau_connector_hpd(),
updating nv_connector->hpd_pending is guarded by the drm->hpd_pending check:

	spin_lock_irqsave(&drm->hpd_lock, flags);
	if (!(drm->hpd_pending & mask)) {
		nv_connector->hpd_pending |= bits;
		drm->hpd_pending |= mask;
		schedule_work(&drm->hpd_work);
	}
	spin_unlock_irqrestore(&drm->hpd_lock, flags);

If nouveau_connector_irq() -> nouveau_dp_irq() first queues
NVIF_CONN_EVENT_V0_IRQ (before a sink count change is visible), and a plug or
unplug event then arrives via nouveau_connector_hotplug() or a follow-up
nouveau_dp_irq() before nouveau_display_hpd_work() finishes
pm_runtime_get_sync() and clears drm->hpd_pending, drm->hpd_pending & mask is
already set. The new NVIF_CONN_EVENT_V0_PLUG or NVIF_CONN_EVENT_V0_UNPLUG bit
is never ORed into nv_connector->hpd_pending.

When nouveau_display_hpd_work() then reads nv_connector->hpd_pending, bits
still only contains NVIF_CONN_EVENT_V0_IRQ, so nouveau_dp_link_check() returns
true when no link is trained and connector detection is still skipped.

Should nv_connector->hpd_pending |= bits be moved outside the
if (!(drm->hpd_pending & mask)) check in nouveau_connector_hpd()?

>  			if (nouveau_dp_link_check(nv_connector))
>  				continue;
>  		}

[Severity: Medium]
Does guarding the outer if statement instead of the continue statement skip
DP link retraining when NVIF_CONN_EVENT_V0_IRQ is set alongside
NVIF_CONN_EVENT_V0_PLUG or NVIF_CONN_EVENT_V0_UNPLUG?

In drivers/gpu/drm/nouveau/nouveau_dp.c, nouveau_dp_link_check() ->
nouveau_dp_link_check_locked() is the only HPD path that checks channel
equalization on an active link (outp->dp.lt.nr != 0) and retrains it via
nouveau_dp_train_link(outp, true):

	if (drm_dp_dpcd_read_phy_link_status(&outp->conn->aux, DP_PHY_DPRX, link_status) < 0)
		return false;

	if (drm_dp_channel_eq_ok(link_status, outp->dp.lt.nr))
		return true;

	return nouveau_dp_train_link(outp, true);

If a plug event runs first and nouveau_display_hpd_work() clears
drm->hpd_pending = 0 before acquiring dev->mode_config.mutex, a subsequent
IRQ_HPD in nouveau_dp_irq() -> nouveau_connector_hpd() sees
!(drm->hpd_pending & mask), ORs NVIF_CONN_EVENT_V0_IRQ into
nv_connector->hpd_pending alongside NVIF_CONN_EVENT_V0_PLUG, and reschedules
drm->hpd_work. The same combination also occurs directly when nouveau_dp_irq()
observes a sink count change on an active SST branch device while the link
needs retraining.

The first worker run sees both NVIF_CONN_EVENT_V0_IRQ and
NVIF_CONN_EVENT_V0_PLUG in bits, clears nv_connector->hpd_pending = 0, and
skips nouveau_dp_link_check(). Because drm_helper_probe_detect() ->
nouveau_connector_detect() -> nouveau_dp_detect() does not check link status
or retrain the link, and any rescheduled worker run sees bits == 0, the link
check is lost.

Would it be better to always run nouveau_dp_link_check(nv_connector) when
NVIF_CONN_EVENT_V0_IRQ is set, and only guard the continue statement with
!(bits & (NVIF_CONN_EVENT_V0_PLUG | NVIF_CONN_EVENT_V0_UNPLUG))?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103-nouveau-dp-fixes-v1-0-892f4bd357cd@thelittleman.net?part=5

Re: [PATCH 5/5] drm/nouveau: don't let an IRQ_HPD hide a plug or unplug event

[David R. Hedges]

On 2026-11-03 13:45 -0600, sashiko-bot@kernel.org wrote:
> Can a separate plug or unplug event still be dropped if it arrives 
> while an
> earlier NVIF_CONN_EVENT_V0_IRQ is already pending for the same 
> connector?

[...]

> Should nv_connector->hpd_pending |= bits be moved outside the
> if (!(drm->hpd_pending & mask)) check in nouveau_connector_hpd()?

Yes. Since 016dacb60e6d ("drm/nouveau/kms: pass event mask to hpd
handler") put the event bits under that check, any event that arrives
between scheduling the HPD worker and the worker clearing
drm->hpd_pending is lost, and the worker can sit in
pm_runtime_get_sync() for some time first. That predates this patch,
but it's the same problem from the other side, so v2 will add a patch
that always records the bits and only schedules the worker under the
check.

> Would it be better to always run nouveau_dp_link_check(nv_connector) 
> when
> NVIF_CONN_EVENT_V0_IRQ is set, and only guard the continue statement 
> with
> !(bits & (NVIF_CONN_EVENT_V0_PLUG | NVIF_CONN_EVENT_V0_UNPLUG))?

I'd rather keep it as it is, but I welcome feedback from the
maintainers. With only this series applied, skipping the link check
loses nothing: nv_connector->dp_encoder is never set on nv50+, so
nouveau_dp_link_check() returns true without looking at the link. The
follow-up series that makes the link check work again [1] also restarts
an active DP link with a full modeset whenever the sink drops or
reasserts HPD, which covers a plug or unplug that comes in with an
IRQ_HPD. Running the link check as well would only add a second
retrain, or AUX reads from a sink that has just gone away.

[1] 
https://lore.kernel.org/r/20261103-nouveau-dp-relink-v1-0-7ee88c7f1e8a@thelittleman.net

Re: [PATCH 5/5] drm/nouveau: don't let an IRQ_HPD hide a plug or unplug event

[David R. Hedges]

On 2026-11-03 13:45 -0600, sashiko-bot@kernel.org wrote:
> Can a separate plug or unplug event still be dropped if it arrives 
> while an
> earlier NVIF_CONN_EVENT_V0_IRQ is already pending for the same 
> connector?

[...]

> Should nv_connector->hpd_pending |= bits be moved outside the
> if (!(drm->hpd_pending & mask)) check in nouveau_connector_hpd()?

Yes. Since 016dacb60e6d ("drm/nouveau/kms: pass event mask to hpd
handler") put the event bits under that check, any event that arrives
between scheduling the HPD worker and the worker clearing
drm->hpd_pending is lost, and the worker can sit in
pm_runtime_get_sync() for some time first. That predates this patch,
but it's the same problem from the other side, so v2 will add a patch
that always records the bits and only schedules the worker under the
check.

> Would it be better to always run nouveau_dp_link_check(nv_connector) 
> when
> NVIF_CONN_EVENT_V0_IRQ is set, and only guard the continue statement 
> with
> !(bits & (NVIF_CONN_EVENT_V0_PLUG | NVIF_CONN_EVENT_V0_UNPLUG))?

I'd rather keep it as it is, but I welcome feedback from the
maintainers. With only this series applied, skipping the link check
loses nothing: nv_connector->dp_encoder is never set on nv50+, so
nouveau_dp_link_check() returns true without looking at the link. The
follow-up series that makes the link check work again [1] also restarts
an active DP link with a full modeset whenever the sink drops or
reasserts HPD, which covers a plug or unplug that comes in with an
IRQ_HPD. Running the link check as well would only add a second
retrain, or AUX reads from a sink that has just gone away.

[1] 
https://lore.kernel.org/r/20261103-nouveau-dp-relink-v1-0-7ee88c7f1e8a@thelittleman.net