Re: [Nouveau] [PATCH v4] drm/nouveau/kms/nv50-: Program notifier offset before requesting disp caps
On Wed, 2 Sep 2020 at 09:43, Lyude Paul wrote: > > Not entirely sure why this never came up when I originally tested this > (maybe some BIOSes already have this setup?) but the ->caps_init vfunc > appears to cause the display engine to throw an exception on driver > init, at least on my ThinkPad P72: > > nouveau :01:00.0: disp: chid 0 mthd 008c data 508c 102b > > This is magic nvidia speak for "You need to have the DMA notifier offset > programmed before you can call NV507D_GET_CAPABILITIES." So, let's fix > this by doing that, and also perform an update afterwards to prevent > racing with the GPU when reading capabilities. > > v2: > * Don't just program the DMA notifier offset, make sure to actually > perform an update > v3: > * Don't call UPDATE() > * Actually read the correct notifier fields, as apparently the > CAPABILITIES_DONE field lives in a different location than the main > NV_DISP_CORE_NOTIFIER_1 field. As well, 907d+ use a different > CAPABILITIES_DONE field then pre-907d cards. > v4: > * Don't forget to check the return value of core507d_read_caps() > > Signed-off-by: Lyude Paul > Fixes: 4a2cb4181b07 ("drm/nouveau/kms/nv50-: Probe SOR and PIOR caps for DP > interlacing support") > Cc: # v5.8+ > --- > drivers/gpu/drm/nouveau/dispnv50/core.h | 2 + > drivers/gpu/drm/nouveau/dispnv50/core507d.c | 37 ++- > drivers/gpu/drm/nouveau/dispnv50/core907d.c | 36 +- > drivers/gpu/drm/nouveau/dispnv50/core917d.c | 2 +- > drivers/gpu/drm/nouveau/dispnv50/disp.h | 2 + > .../drm/nouveau/include/nvhw/class/cl507d.h | 5 ++- > .../drm/nouveau/include/nvhw/class/cl907d.h | 4 ++ > 7 files changed, 83 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/nouveau/dispnv50/core.h > b/drivers/gpu/drm/nouveau/dispnv50/core.h > index 498622c0c670d..b789139e5fff6 100644 > --- a/drivers/gpu/drm/nouveau/dispnv50/core.h > +++ b/drivers/gpu/drm/nouveau/dispnv50/core.h > @@ -44,6 +44,7 @@ int core507d_new_(const struct nv50_core_func *, struct > nouveau_drm *, s32, > struct nv50_core **); > int core507d_init(struct nv50_core *); > void core507d_ntfy_init(struct nouveau_bo *, u32); > +int core507d_read_caps(struct nv50_disp *disp, u32 offset); > int core507d_caps_init(struct nouveau_drm *, struct nv50_disp *); > int core507d_ntfy_wait_done(struct nouveau_bo *, u32, struct nvif_device *); > int core507d_update(struct nv50_core *, u32 *, bool); > @@ -55,6 +56,7 @@ extern const struct nv50_outp_func pior507d; > int core827d_new(struct nouveau_drm *, s32, struct nv50_core **); > > int core907d_new(struct nouveau_drm *, s32, struct nv50_core **); > +int core907d_caps_init(struct nouveau_drm *drm, struct nv50_disp *disp); > extern const struct nv50_outp_func dac907d; > extern const struct nv50_outp_func sor907d; > > diff --git a/drivers/gpu/drm/nouveau/dispnv50/core507d.c > b/drivers/gpu/drm/nouveau/dispnv50/core507d.c > index ad1f09a143aa4..d0f2b80a32103 100644 > --- a/drivers/gpu/drm/nouveau/dispnv50/core507d.c > +++ b/drivers/gpu/drm/nouveau/dispnv50/core507d.c > @@ -75,18 +75,51 @@ core507d_ntfy_init(struct nouveau_bo *bo, u32 offset) > } > > int > -core507d_caps_init(struct nouveau_drm *drm, struct nv50_disp *disp) > +core507d_read_caps(struct nv50_disp *disp, u32 offset) > { > struct nvif_push *push = disp->core->chan.push; > int ret; > > - if ((ret = PUSH_WAIT(push, 2))) > + ret = PUSH_WAIT(push, 4); > + if (ret) > return ret; > > + PUSH_MTHD(push, NV507D, SET_NOTIFIER_CONTROL, > + NVDEF(NV507D, SET_NOTIFIER_CONTROL, MODE, WRITE) | > + NVVAL(NV507D, SET_NOTIFIER_CONTROL, OFFSET, offset >> 2) | > + NVDEF(NV507D, SET_NOTIFIER_CONTROL, NOTIFY, ENABLE)); > PUSH_MTHD(push, NV507D, GET_CAPABILITIES, 0x); Can you send a SET_NOTIFIER_CONTROL_NOTIFY_DISABLE after GET_CAPABILITIES() too please :) > + > return PUSH_KICK(push); > } > > +int > +core507d_caps_init(struct nouveau_drm *drm, struct nv50_disp *disp) > +{ > + struct nv50_core *core = disp->core; > + struct nouveau_bo *bo = disp->sync; > + s64 time; > + int ret; > + > + NVBO_WR32(bo, NV50_DISP_CAPS_NTFY1, NV_DISP_CORE_NOTIFIER_1, > CAPABILITIES_1, > + NVDEF(NV_DISP_CORE_NOTIFIER_1, > CAPABILITIES_1, DONE, FALSE)); You don't need these NV50_DISP_CAPS_NTFYx thingies. These offsets are already encoded in NVIDIA's headers (NV_DISP_CORE_NOTIFIER_1_CAPABILITIES_1 is an offset), you're adding an additional offset by doing this. Just use NV50_DISP_CORE_NTFY in all these places, and let NVIDIA's headers do the rest. The additional offset in these macros are meant for when there's multiple structures packed into a single nouveau_bo at different offsets. It doesn't actually matter here, because it gets divided away. But for core907d, you're actually
Re: [Nouveau] [PATCH v4] drm/nouveau/kms/nv50-: Program notifier offset before requesting disp caps
Hi [This is an automated email] This commit has been processed because it contains a "Fixes:" tag fixing commit: 4a2cb4181b07 ("drm/nouveau/kms/nv50-: Probe SOR and PIOR caps for DP interlacing support"). The bot has tested the following trees: v5.8.5. v5.8.5: Failed to apply! Possible dependencies: 0a96099691c8 ("drm/nouveau/kms/nv50-: implement proper push buffer control logic") 0bc8ffe09771 ("drm/nouveau/kms/nv50-: Move hard-coded object handles into header") 12885ecbfe62 ("drm/nouveau/kms/nvd9-: Add CRC support") 203f6eaf4182 ("drm/nouveau/kms/nv50-: convert core update() to new push macros") 2853ccf09255 ("drm/nouveau/kms/nv50-: wrap existing command submission in nvif_push interface") 344c2e5a4796 ("drm/nouveau/kms/nv50-: use NVIDIA's headers for core or_ctrl()") 3c43c362b3a5 ("drm/nouveau/kms/nv50-: convert core caps_init() to new push macros") 5e691222eac6 ("drm/nouveau/kms/nv50-: convert core init() to new push macros") 9ec5e8204053 ("drm/nouveau/kms/nv50-: convert core or_ctrl() to new push macros") b11d8ca151d0 ("drm/nouveau/kms/nv50-: use NVIDIA's headers for core init()") b505935e56b2 ("drm/nouveau/kms/nv50-: convert core wndw_owner() to new push macros") d8b24526ef68 ("drm/nouveau/kms/nv50-: use NVIDIA's headers for core caps_init()") e79c9a0ba5e7 ("drm/nouveau/nvif: give every mem object a human-readable identifier") NOTE: The patch will not be queued to stable trees until it is upstream. How should we proceed with this patch? -- Thanks Sasha ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v4] drm/nouveau/kms/nv50-: Program notifier offset before requesting disp caps
Not entirely sure why this never came up when I originally tested this (maybe some BIOSes already have this setup?) but the ->caps_init vfunc appears to cause the display engine to throw an exception on driver init, at least on my ThinkPad P72: nouveau :01:00.0: disp: chid 0 mthd 008c data 508c 102b This is magic nvidia speak for "You need to have the DMA notifier offset programmed before you can call NV507D_GET_CAPABILITIES." So, let's fix this by doing that, and also perform an update afterwards to prevent racing with the GPU when reading capabilities. v2: * Don't just program the DMA notifier offset, make sure to actually perform an update v3: * Don't call UPDATE() * Actually read the correct notifier fields, as apparently the CAPABILITIES_DONE field lives in a different location than the main NV_DISP_CORE_NOTIFIER_1 field. As well, 907d+ use a different CAPABILITIES_DONE field then pre-907d cards. v4: * Don't forget to check the return value of core507d_read_caps() Signed-off-by: Lyude Paul Fixes: 4a2cb4181b07 ("drm/nouveau/kms/nv50-: Probe SOR and PIOR caps for DP interlacing support") Cc: # v5.8+ --- drivers/gpu/drm/nouveau/dispnv50/core.h | 2 + drivers/gpu/drm/nouveau/dispnv50/core507d.c | 37 ++- drivers/gpu/drm/nouveau/dispnv50/core907d.c | 36 +- drivers/gpu/drm/nouveau/dispnv50/core917d.c | 2 +- drivers/gpu/drm/nouveau/dispnv50/disp.h | 2 + .../drm/nouveau/include/nvhw/class/cl507d.h | 5 ++- .../drm/nouveau/include/nvhw/class/cl907d.h | 4 ++ 7 files changed, 83 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/nouveau/dispnv50/core.h b/drivers/gpu/drm/nouveau/dispnv50/core.h index 498622c0c670d..b789139e5fff6 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/core.h +++ b/drivers/gpu/drm/nouveau/dispnv50/core.h @@ -44,6 +44,7 @@ int core507d_new_(const struct nv50_core_func *, struct nouveau_drm *, s32, struct nv50_core **); int core507d_init(struct nv50_core *); void core507d_ntfy_init(struct nouveau_bo *, u32); +int core507d_read_caps(struct nv50_disp *disp, u32 offset); int core507d_caps_init(struct nouveau_drm *, struct nv50_disp *); int core507d_ntfy_wait_done(struct nouveau_bo *, u32, struct nvif_device *); int core507d_update(struct nv50_core *, u32 *, bool); @@ -55,6 +56,7 @@ extern const struct nv50_outp_func pior507d; int core827d_new(struct nouveau_drm *, s32, struct nv50_core **); int core907d_new(struct nouveau_drm *, s32, struct nv50_core **); +int core907d_caps_init(struct nouveau_drm *drm, struct nv50_disp *disp); extern const struct nv50_outp_func dac907d; extern const struct nv50_outp_func sor907d; diff --git a/drivers/gpu/drm/nouveau/dispnv50/core507d.c b/drivers/gpu/drm/nouveau/dispnv50/core507d.c index ad1f09a143aa4..d0f2b80a32103 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/core507d.c +++ b/drivers/gpu/drm/nouveau/dispnv50/core507d.c @@ -75,18 +75,51 @@ core507d_ntfy_init(struct nouveau_bo *bo, u32 offset) } int -core507d_caps_init(struct nouveau_drm *drm, struct nv50_disp *disp) +core507d_read_caps(struct nv50_disp *disp, u32 offset) { struct nvif_push *push = disp->core->chan.push; int ret; - if ((ret = PUSH_WAIT(push, 2))) + ret = PUSH_WAIT(push, 4); + if (ret) return ret; + PUSH_MTHD(push, NV507D, SET_NOTIFIER_CONTROL, + NVDEF(NV507D, SET_NOTIFIER_CONTROL, MODE, WRITE) | + NVVAL(NV507D, SET_NOTIFIER_CONTROL, OFFSET, offset >> 2) | + NVDEF(NV507D, SET_NOTIFIER_CONTROL, NOTIFY, ENABLE)); PUSH_MTHD(push, NV507D, GET_CAPABILITIES, 0x); + return PUSH_KICK(push); } +int +core507d_caps_init(struct nouveau_drm *drm, struct nv50_disp *disp) +{ + struct nv50_core *core = disp->core; + struct nouveau_bo *bo = disp->sync; + s64 time; + int ret; + + NVBO_WR32(bo, NV50_DISP_CAPS_NTFY1, NV_DISP_CORE_NOTIFIER_1, CAPABILITIES_1, + NVDEF(NV_DISP_CORE_NOTIFIER_1, CAPABILITIES_1, DONE, FALSE)); + + ret = core507d_read_caps(disp, NV50_DISP_CAPS_NTFY1); + if (ret < 0) + return ret; + + time = nvif_msec(core->chan.base.device, 2000ULL, +if (NVBO_TD32(bo, NV50_DISP_CAPS_NTFY1, + NV_DISP_CORE_NOTIFIER_1, CAPABILITIES_1, DONE, ==, TRUE)) +break; +usleep_range(1, 2); +); + if (time < 0) + NV_ERROR(drm, "core caps notifier timeout\n"); + + return 0; +} + int core507d_init(struct nv50_core *core) { diff --git a/drivers/gpu/drm/nouveau/dispnv50/core907d.c b/drivers/gpu/drm/nouveau/dispnv50/core907d.c index b17c03529c784..45505a18aca17 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/core907d.c +++ b/drivers/gpu/drm/nouveau/dispnv50/core907d.c @@ -22,11 +22,45