Re: [PATCH 5/7] nouveau/gsp: fix vblank interrupts on GB20x
Mohamed Ahmed <[email protected]>
| Newsgroups | org.freedesktop.lists.nouveau,org.freedesktop.lists.dri-devel,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <CAA+WOBtbC176yVMZ1OZHQANNdNhHuGVtNMdiDLeSvHKmsD3L-A@mail.gmail.com> |
On Wed, Aug 19, 2026 at 12:06 AM <[email protected]> wrote: > > On Wed, 2026-08-19 at 00:02 +0400, Mohamed Ahmed wrote: > > On Tue, Aug 18, 2026 at 2:59 AM <[email protected]> wrote: > > > > > > On Sat, 2026-08-15 at 03:57 +0400, Mohamed Ahmed wrote: > > > > > > > > + > > > > +static const struct nvkm_head_func > > > > +gb202_head = { > > > > + .state = r535_head_state, > > > > + .vblank_get = gb202_head_vblank_get, > > > > + .vblank_put = gb202_head_vblank_put, > > > > +}; > > > > + > > > > > > You should probably add these two as well: > > > > > > .state = gv100_head_state, > > > .rgpos = gv100_head_rgpos, > > > > > > I don't think they've actually changed in blackwell, but it's > > > possible > > > maybe they do change in GB200x so you probably want to double check > > > so > > > we don't lose precise vblank timestamps again. > > > > > The head state changed actually (but RG position didn't), so adding > > this in would be a regression. I have some work to wire it up > > properly > > (part of VRR bringup) but I wasn't sure whether to include it in this > > series or not. We don't have precise vblank timestamps for anything > > GSP atm and everything falls back to the interrupt path, so it would > > technically be a new feature rather than fix ups. > > Actually we definitely do now! We didn't used to for ages, but I wrote > a patch that got upstreamed for it a few weeks ago, you can see it > here: > > static const struct nvkm_head_func > r535_head = { > .state = gv100_head_state, > .rgpos = gv100_head_rgpos, > .vblank_get = r535_head_vblank_get, > .vblank_put = r535_head_vblank_put, > }; > > If the patch for adding this to blackwell is already ready I wouldn't > mind taking it as part of this series, but that's up to you > Ohh woops, sorry. I missed that. Yes, I'll rebase on top of the latest then and re-test. > > > > static struct nvkm_conn * > > > > r535_conn_new(struct nvkm_disp *disp, u32 id) > > > > { > > > > @@ -1496,6 +1523,20 @@ r535_disp_intr(struct nvkm_inth *inth) > > > > return IRQ_HANDLED; > > > > } > > > > > > > > +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 = r535_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; > > > > +} > > > > + > > > > static void > > > > r535_disp_fini(struct nvkm_disp *disp, bool suspend) > > > > { > > > > @@ -1568,7 +1609,9 @@ r535_disp_oneinit(struct nvkm_disp *disp) > > > > struct nvkm_device *device = disp->engine.subdev.device; > > > > struct nvkm_gsp *gsp = device->gsp; > > > > const struct nvkm_rm_api *rmapi = gsp->rm->api; > > > > + const struct nvkm_rm_gpu *gpu = gsp->rm->gpu; > > > > NV2080_CTRL_INTERNAL_DISPLAY_WRITE_INST_MEM_PARAMS *ctrl; > > > > + nvkm_inth_func intr_func; > > > > unsigned long mask; > > > > int ret, i; > > > > > > > > @@ -1722,7 +1765,12 @@ r535_disp_oneinit(struct nvkm_disp *disp) > > > > nvkm_gsp_rm_ctrl_done(&disp->rm.objcom, ctrl); > > > > > > > > for_each_set_bit(i, &disp->head.mask, disp- > > > > >head.nr) > > > > { > > > > - ret = nvkm_head_new_(&r535_head, disp, i); > > > > + const struct nvkm_head_func *func = > > > > &r535_head; > > > > + > > > > + if (gpu->disp.class.root >= GB202_DISP) > > > > + func = &gb202_head; > > > > + > > > > + ret = nvkm_head_new_(func, disp, i); > > > > if (ret) > > > > return ret; > > > > } > > > > @@ -1766,12 +1814,21 @@ r535_disp_oneinit(struct nvkm_disp *disp) > > > > if (ret) > > > > return ret; > > > > > > > > - ret = nvkm_gsp_intr_stall(gsp, disp->engine.subdev.type, > > > > disp->engine.subdev.inst); > > > > + if (gpu->disp.class.root >= GB202_DISP) { > > > > + /* GB20x deliver head-timing interrupts on the > > > > display's > > > > + * separate low-latency vector (interrupt table > > > > instance 1). > > > > + */ > > > > + ret = nvkm_gsp_intr_stall(gsp, disp- > > > > > engine.subdev.type, 1); > > > > + intr_func = gb202_disp_intr; > > > > + } else { > > > > + ret = nvkm_gsp_intr_stall(gsp, disp- > > > > > engine.subdev.type, disp->engine.subdev.inst); > > > > + intr_func = r535_disp_intr; > > > > + } > > > > if (ret < 0) > > > > return ret; > > > > > > > > ret = nvkm_inth_add(&device->vfn->intr, ret, > > > > NVKM_INTR_PRIO_NORMAL, &disp->engine.subdev, > > > > - r535_disp_intr, &disp- > > > > > engine.subdev.inth); > > > > + intr_func, &disp->engine.subdev.inth); > > > > if (ret) > > > > return ret; > > > > > > > > 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 996941c668ba..2590b22663cb 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,14 @@ 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. > > > > + */ > > > > + *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; > > > >