Some comments regarding both patch #2 and this patch below:
On Thu, 2026-08-20 at 20:49 +0400, Mohamed Ahmed wrote: > The GSP path programs per-head timing (vblank) interrupts the same > way on > every generation. NVD5.0 (GB20x) reworked the FE interrupt frontend > around four message-based kernel vectors (high latency, low latency, > PMU, > and GSP) and moved RM head-timing interrupts to the dedicated low- > latency > vector: > > - The enable is NV_PDISP_FE_RM_INTR_EN1_HEAD_TIMING, 0x611ef0 + > head*4 (570.144 kernel_head_0501.c, renamed kernel_head_0502.c > from > 575.51.02 on, and v05_01 dev_disp.h). > > - The vector is reported as a separate interrupt table entry, > MC_ENGINE_IDX_DISP_LOW (intr_gb202.c, intrCacheDispIntrVectors). > > - The vector must be re-armed through NV_PDISP_FE_INTR_RETRIGGER(1) > at 0x611f34 after servicing (kdispServiceInterrupt -> > kdispIntrRetrigger_v05_01). > > The event latch (0x611800), per-head status (0x611c00), and dispatch > summary (0x611ec0) the interrupt handler uses are unchanged on GB20x > (kheadReadPendingVblank_v03_00 and kheadResetPendingLastData_v03_00 > remain for DISPv0502+). > > On GB20x the old code enables head timing onto the legacy vector, > leaves > its handler there, and never re-arms the message-based vectors. Page > flips still complete (nv50 sends those events from the commit path), > so > the desktop looks fine while DRM vblank waits and vblank sequence > queries > are affected. > > Supply GB20x vblank enables and an interrupt handler that re-arms the > vector after servicing through gb202_gsp_disp, translate the low- > latency > interrupt table entry as a second NVKM_ENGINE_DISP instance, and flag > the table so r535_disp_oneinit() attaches the handler to that > instance. > > Signed-off-by: Mohamed Ahmed <[email protected]> > --- > .../gpu/drm/nouveau/nvkm/engine/disp/gb202.c | 42 > +++++++++++++++++-- > .../nouveau/nvkm/subdev/gsp/rm/r535/disp.c | 5 ++- > .../drm/nouveau/nvkm/subdev/gsp/rm/r570/gsp.c | 9 ++++ > 3 files changed, 52 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/gb202.c > b/drivers/gpu/drm/nouveau/nvkm/engine/disp/gb202.c > index a66c820be9fe..f78669bafd64 100644 > --- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/gb202.c > +++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/gb202.c > @@ -130,6 +130,40 @@ gb202_head_state(struct nvkm_head *head, struct > nvkm_head_state *state) > } > } > > +/* NVD5.0 (GB20x and later) moved the RM head-timing interrupt > enable to > + * the low-latency vector's EN1 block. The event latch is unchanged. > + */ > +static void > +gb202_head_vblank_put(struct nvkm_head *head) > +{ > + struct nvkm_device *device = head->disp- > >engine.subdev.device; > + > + nvkm_mask(device, 0x611ef0 + (head->id * 4), 0x00000002, > 0x00000000); > +} > + > +static void > +gb202_head_vblank_get(struct nvkm_head *head) > +{ > + struct nvkm_device *device = head->disp- > >engine.subdev.device; > + > + nvkm_wr32(device, 0x611800 + (head->id * 4), 0x00000002); > + nvkm_mask(device, 0x611ef0 + (head->id * 4), 0x00000002, > 0x00000002); > +} > + > +static irqreturn_t > +gb202_disp_intr(struct nvkm_inth *inth) > +{ > + struct nvkm_disp *disp = container_of(inth, typeof(*disp), > engine.subdev.inth); > + irqreturn_t ret = tu102_disp_intr(inth); > + > + /* The FE interrupt vectors are message-based on NVD5.0. Re- > arm the > + * low-latency vector so it fires again for any event that > latched > + * while we were servicing. > + */ > + nvkm_wr32(disp->engine.subdev.device, 0x611f34, 0x00000001); > + return ret; > +} > + > /* GB20x is GSP-only. This table supplies the register programming > the > * GSP-RM display path needs from the chip. > */ > @@ -137,11 +171,13 @@ static const struct nvkm_disp_func > gb202_gsp_disp = { > .uevent = &gv100_disp_chan_uevent, > .ramht_size = 0x2000, > - .gsp.intr = tu102_disp_intr, > + /* Head timing arrives on the dedicated low-latency vector. > */ > + .gsp.intr = gb202_disp_intr, > + .gsp.intr_low_latency = true, > .gsp.head_state = gb202_head_state, > .gsp.head_rgpos = gv100_head_rgpos, > - .gsp.vblank_get = tu102_head_vblank_get, > - .gsp.vblank_put = tu102_head_vblank_put, > + .gsp.vblank_get = gb202_head_vblank_get, > + .gsp.vblank_put = gb202_head_vblank_put, > .gsp.hdmi_gcp = gb202_sor_hdmi_gcp, > /* The legacy AVI unit is unchanged on GB20x. */ > .gsp.hdmi_infoframe_avi = gv100_sor_hdmi_infoframe_avi, > 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 3a8ff621ed62..a95f78c4502f 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 > @@ -1705,7 +1705,10 @@ r535_disp_oneinit(struct nvkm_disp *disp) > > /* Chips that raise head-timing interrupts on a separate > low-latency > * vector report it as a second DISP interrupt table entry, > exposed > - * as instance 1 by the RM engine-index translation. > + * as instance 1 by the RM engine-index translation (see > + * r570_gsp_xlat_mc_engine_idx()). Their high-latency vector > + * (instance 0) is left unhandled as no event nouveau > enables is > + * routed to it, and without a handler it stays masked. > */ I didn't notice it until I got to this patch, but is it possible you mistakenly added the intr_low_latency stuff a little early with patch #2 and meant to add it here? (doesn't matter to me too much either way, whatever you intended works fine with me) Otherwise: Reviewed-by: Lyude Paul <[email protected]> > ret = nvkm_gsp_intr_stall(gsp, disp->engine.subdev.type, > disp->func->gsp.intr_low_latency ? > 1 : disp->engine.subdev.inst); > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gsp.c > b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gsp.c > index 3e391646d8f7..b45781cd0dfd 100644 > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gsp.c > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gsp.c > @@ -44,6 +44,15 @@ r570_gsp_xlat_mc_engine_idx(u32 mc_engine_idx, > enum nvkm_subdev_type *ptype, int > *ptype = NVKM_ENGINE_DISP; > *pinst = 0; > return true; > + case MC_ENGINE_IDX_DISP_LOW: > + /* GB20x+ report a separate low-latency display > vector, used > + * for head-timing interrupts. Expose it as a second > DISP > + * interrupt instance. r535_disp_oneinit() attaches > the > + * handler to it when the chip's > gsp.intr_low_latency is set. > + */ > + *ptype = NVKM_ENGINE_DISP; > + *pinst = 1; > + return true; > case MC_ENGINE_IDX_CE0 ... MC_ENGINE_IDX_CE19: > *ptype = NVKM_ENGINE_CE; > *pinst = mc_engine_idx - MC_ENGINE_IDX_CE0;
