* [PATCH 1/2] drm/nouveau/kms/nv50-: Program notifier offset before requesting disp caps [not found] <20200824183253.826343-1-lyude@redhat.com> @ 2020-08-24 18:32 ` Lyude Paul 2020-08-24 22:28 ` [Nouveau] " Ben Skeggs 2020-09-01 23:38 ` [PATCH v3] " Lyude Paul 2020-08-24 18:32 ` [PATCH 2/2] drm/nouveau/kms/nv50-: Log SOR/PIOR caps Lyude Paul 1 sibling, 2 replies; 9+ messages in thread From: Lyude Paul @ 2020-08-24 18:32 UTC (permalink / raw) To: nouveau, Sasha Levin Cc: stable, Ben Skeggs, David Airlie, Daniel Vetter, open list:DRM DRIVER FOR NVIDIA GEFORCE/QUADRO GPUS, open list 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 0000:01:00.0: disp: chid 0 mthd 008c data 00000000 0000508c 0000102b 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. Changes since v1: * Don't just program the DMA notifier offset, make sure to actually perform an update Signed-off-by: Lyude Paul <lyude@redhat.com> Fixes: 4a2cb4181b07 ("drm/nouveau/kms/nv50-: Probe SOR and PIOR caps for DP interlacing support") Cc: <stable@vger.kernel.org> # v5.8+ --- drivers/gpu/drm/nouveau/dispnv50/core507d.c | 25 ++++++++++++++++----- 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/nouveau/dispnv50/core507d.c b/drivers/gpu/drm/nouveau/dispnv50/core507d.c index e341f572c2696..5e86feec3b720 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/core507d.c +++ b/drivers/gpu/drm/nouveau/dispnv50/core507d.c @@ -65,13 +65,26 @@ core507d_ntfy_init(struct nouveau_bo *bo, u32 offset) int core507d_caps_init(struct nouveau_drm *drm, struct nv50_disp *disp) { - u32 *push = evo_wait(&disp->core->chan, 2); + struct nv50_core *core = disp->core; + u32 interlock[NV50_DISP_INTERLOCK__SIZE] = {0}; + u32 *push; - if (push) { - evo_mthd(push, 0x008c, 1); - evo_data(push, 0x0); - evo_kick(push, &disp->core->chan); - } + core->func->ntfy_init(disp->sync, NV50_DISP_CORE_NTFY); + + push = evo_wait(&core->chan, 4); + if (!push) + return 0; + + evo_mthd(push, 0x0084, 1); + evo_data(push, 0x80000000 | NV50_DISP_CORE_NTFY); + evo_mthd(push, 0x008c, 1); + evo_data(push, 0x0); + evo_kick(push, &core->chan); + + core->func->update(core, interlock, false); + if (core->func->ntfy_wait_done(disp->sync, NV50_DISP_CORE_NTFY, + core->chan.base.device)) + NV_ERROR(drm, "core notifier timeout\n"); return 0; } -- 2.26.2 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Nouveau] [PATCH 1/2] drm/nouveau/kms/nv50-: Program notifier offset before requesting disp caps 2020-08-24 18:32 ` [PATCH 1/2] drm/nouveau/kms/nv50-: Program notifier offset before requesting disp caps Lyude Paul @ 2020-08-24 22:28 ` Ben Skeggs 2020-08-25 16:52 ` Lyude Paul 2020-09-01 23:38 ` [PATCH v3] " Lyude Paul 1 sibling, 1 reply; 9+ messages in thread From: Ben Skeggs @ 2020-08-24 22:28 UTC (permalink / raw) To: Lyude Paul Cc: ML nouveau, Sasha Levin, David Airlie, open list, open list:DRM DRIVER FOR NVIDIA GEFORCE/QUADRO GPUS, Ben Skeggs, Daniel Vetter, stable On Tue, 25 Aug 2020 at 04:33, Lyude Paul <lyude@redhat.com> 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 0000:01:00.0: disp: chid 0 mthd 008c data 00000000 0000508c 0000102b > > 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. > > Changes since v1: > * Don't just program the DMA notifier offset, make sure to actually > perform an update I'm not sure there's a need to send an Update() method here, I believe GetCapabilities() is an action method on its own right? Ben. > > Signed-off-by: Lyude Paul <lyude@redhat.com> > Fixes: 4a2cb4181b07 ("drm/nouveau/kms/nv50-: Probe SOR and PIOR caps for DP interlacing support") > Cc: <stable@vger.kernel.org> # v5.8+ > --- > drivers/gpu/drm/nouveau/dispnv50/core507d.c | 25 ++++++++++++++++----- > 1 file changed, 19 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/nouveau/dispnv50/core507d.c b/drivers/gpu/drm/nouveau/dispnv50/core507d.c > index e341f572c2696..5e86feec3b720 100644 > --- a/drivers/gpu/drm/nouveau/dispnv50/core507d.c > +++ b/drivers/gpu/drm/nouveau/dispnv50/core507d.c > @@ -65,13 +65,26 @@ core507d_ntfy_init(struct nouveau_bo *bo, u32 offset) > int > core507d_caps_init(struct nouveau_drm *drm, struct nv50_disp *disp) > { > - u32 *push = evo_wait(&disp->core->chan, 2); > + struct nv50_core *core = disp->core; > + u32 interlock[NV50_DISP_INTERLOCK__SIZE] = {0}; > + u32 *push; > > - if (push) { > - evo_mthd(push, 0x008c, 1); > - evo_data(push, 0x0); > - evo_kick(push, &disp->core->chan); > - } > + core->func->ntfy_init(disp->sync, NV50_DISP_CORE_NTFY); > + > + push = evo_wait(&core->chan, 4); > + if (!push) > + return 0; > + > + evo_mthd(push, 0x0084, 1); > + evo_data(push, 0x80000000 | NV50_DISP_CORE_NTFY); > + evo_mthd(push, 0x008c, 1); > + evo_data(push, 0x0); > + evo_kick(push, &core->chan); > + > + core->func->update(core, interlock, false); > + if (core->func->ntfy_wait_done(disp->sync, NV50_DISP_CORE_NTFY, > + core->chan.base.device)) > + NV_ERROR(drm, "core notifier timeout\n"); > > return 0; > } > -- > 2.26.2 > > _______________________________________________ > Nouveau mailing list > Nouveau@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/nouveau ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Nouveau] [PATCH 1/2] drm/nouveau/kms/nv50-: Program notifier offset before requesting disp caps 2020-08-24 22:28 ` [Nouveau] " Ben Skeggs @ 2020-08-25 16:52 ` Lyude Paul 2020-08-31 4:26 ` Ben Skeggs 0 siblings, 1 reply; 9+ messages in thread From: Lyude Paul @ 2020-08-25 16:52 UTC (permalink / raw) To: Ben Skeggs Cc: ML nouveau, Sasha Levin, David Airlie, open list, open list:DRM DRIVER FOR NVIDIA GEFORCE/QUADRO GPUS, Ben Skeggs, Daniel Vetter, stable On Tue, 2020-08-25 at 08:28 +1000, Ben Skeggs wrote: > On Tue, 25 Aug 2020 at 04:33, Lyude Paul <lyude@redhat.com> 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 0000:01:00.0: disp: chid 0 mthd 008c data 00000000 0000508c 0000102b > > > > 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. > > > > Changes since v1: > > * Don't just program the DMA notifier offset, make sure to actually > > perform an update > I'm not sure there's a need to send an Update() method here, I believe > GetCapabilities() is an action method on its own right? > I'm not entirely sure about this part tbh. I do know that we need to call GetCapabilities() _after_ the DMA notifier offset is programmed. But, my assumption was that if GetCapabilities() requires a DMA notifier offset to store its results in, we'd probably want to fire an update or something to make sure that we're not reading before it finishes writing capabilities? > Ben. > > > Signed-off-by: Lyude Paul <lyude@redhat.com> > > Fixes: 4a2cb4181b07 ("drm/nouveau/kms/nv50-: Probe SOR and PIOR caps for DP > > interlacing support") > > Cc: <stable@vger.kernel.org> # v5.8+ > > --- > > drivers/gpu/drm/nouveau/dispnv50/core507d.c | 25 ++++++++++++++++----- > > 1 file changed, 19 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/nouveau/dispnv50/core507d.c > > b/drivers/gpu/drm/nouveau/dispnv50/core507d.c > > index e341f572c2696..5e86feec3b720 100644 > > --- a/drivers/gpu/drm/nouveau/dispnv50/core507d.c > > +++ b/drivers/gpu/drm/nouveau/dispnv50/core507d.c > > @@ -65,13 +65,26 @@ core507d_ntfy_init(struct nouveau_bo *bo, u32 offset) > > int > > core507d_caps_init(struct nouveau_drm *drm, struct nv50_disp *disp) > > { > > - u32 *push = evo_wait(&disp->core->chan, 2); > > + struct nv50_core *core = disp->core; > > + u32 interlock[NV50_DISP_INTERLOCK__SIZE] = {0}; > > + u32 *push; > > > > - if (push) { > > - evo_mthd(push, 0x008c, 1); > > - evo_data(push, 0x0); > > - evo_kick(push, &disp->core->chan); > > - } > > + core->func->ntfy_init(disp->sync, NV50_DISP_CORE_NTFY); > > + > > + push = evo_wait(&core->chan, 4); > > + if (!push) > > + return 0; > > + > > + evo_mthd(push, 0x0084, 1); > > + evo_data(push, 0x80000000 | NV50_DISP_CORE_NTFY); > > + evo_mthd(push, 0x008c, 1); > > + evo_data(push, 0x0); > > + evo_kick(push, &core->chan); > > + > > + core->func->update(core, interlock, false); > > + if (core->func->ntfy_wait_done(disp->sync, NV50_DISP_CORE_NTFY, > > + core->chan.base.device)) > > + NV_ERROR(drm, "core notifier timeout\n"); > > > > return 0; > > } > > -- > > 2.26.2 > > > > _______________________________________________ > > Nouveau mailing list > > Nouveau@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/nouveau -- Sincerely, Lyude Paul (she/her) Software Engineer at Red Hat ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Nouveau] [PATCH 1/2] drm/nouveau/kms/nv50-: Program notifier offset before requesting disp caps 2020-08-25 16:52 ` Lyude Paul @ 2020-08-31 4:26 ` Ben Skeggs 2020-09-01 20:43 ` Lyude Paul 0 siblings, 1 reply; 9+ messages in thread From: Ben Skeggs @ 2020-08-31 4:26 UTC (permalink / raw) To: Lyude Paul Cc: ML nouveau, Sasha Levin, David Airlie, open list, open list:DRM DRIVER FOR NVIDIA GEFORCE/QUADRO GPUS, Ben Skeggs, Daniel Vetter, stable On Wed, 26 Aug 2020 at 02:52, Lyude Paul <lyude@redhat.com> wrote: > > On Tue, 2020-08-25 at 08:28 +1000, Ben Skeggs wrote: > > On Tue, 25 Aug 2020 at 04:33, Lyude Paul <lyude@redhat.com> 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 0000:01:00.0: disp: chid 0 mthd 008c data 00000000 0000508c 0000102b > > > > > > 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. > > > > > > Changes since v1: > > > * Don't just program the DMA notifier offset, make sure to actually > > > perform an update > > I'm not sure there's a need to send an Update() method here, I believe > > GetCapabilities() is an action method on its own right? > > > > I'm not entirely sure about this part tbh. I do know that we need to call > GetCapabilities() _after_ the DMA notifier offset is programmed. But, my > assumption was that if GetCapabilities() requires a DMA notifier offset to store > its results in, we'd probably want to fire an update or something to make sure > that we're not reading before it finishes writing capabilities? We definitely want to *wait* on GetCapabilities() finishing, I believe it should also update the notifier the same (or similar) way Update() does. But I don't think we want to send an Update() here, it'll actually trigger a modeset (which, on earlier HW, will tear down the boot mode. Not sure about current HW, it might preserve state), and we may not want that to happen there. Ben. > > > Ben. > > > > > Signed-off-by: Lyude Paul <lyude@redhat.com> > > > Fixes: 4a2cb4181b07 ("drm/nouveau/kms/nv50-: Probe SOR and PIOR caps for DP > > > interlacing support") > > > Cc: <stable@vger.kernel.org> # v5.8+ > > > --- > > > drivers/gpu/drm/nouveau/dispnv50/core507d.c | 25 ++++++++++++++++----- > > > 1 file changed, 19 insertions(+), 6 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/nouveau/dispnv50/core507d.c > > > b/drivers/gpu/drm/nouveau/dispnv50/core507d.c > > > index e341f572c2696..5e86feec3b720 100644 > > > --- a/drivers/gpu/drm/nouveau/dispnv50/core507d.c > > > +++ b/drivers/gpu/drm/nouveau/dispnv50/core507d.c > > > @@ -65,13 +65,26 @@ core507d_ntfy_init(struct nouveau_bo *bo, u32 offset) > > > int > > > core507d_caps_init(struct nouveau_drm *drm, struct nv50_disp *disp) > > > { > > > - u32 *push = evo_wait(&disp->core->chan, 2); > > > + struct nv50_core *core = disp->core; > > > + u32 interlock[NV50_DISP_INTERLOCK__SIZE] = {0}; > > > + u32 *push; > > > > > > - if (push) { > > > - evo_mthd(push, 0x008c, 1); > > > - evo_data(push, 0x0); > > > - evo_kick(push, &disp->core->chan); > > > - } > > > + core->func->ntfy_init(disp->sync, NV50_DISP_CORE_NTFY); > > > + > > > + push = evo_wait(&core->chan, 4); > > > + if (!push) > > > + return 0; > > > + > > > + evo_mthd(push, 0x0084, 1); > > > + evo_data(push, 0x80000000 | NV50_DISP_CORE_NTFY); > > > + evo_mthd(push, 0x008c, 1); > > > + evo_data(push, 0x0); > > > + evo_kick(push, &core->chan); > > > + > > > + core->func->update(core, interlock, false); > > > + if (core->func->ntfy_wait_done(disp->sync, NV50_DISP_CORE_NTFY, > > > + core->chan.base.device)) > > > + NV_ERROR(drm, "core notifier timeout\n"); > > > > > > return 0; > > > } > > > -- > > > 2.26.2 > > > > > > _______________________________________________ > > > Nouveau mailing list > > > Nouveau@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/nouveau > -- > Sincerely, > Lyude Paul (she/her) > Software Engineer at Red Hat > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Nouveau] [PATCH 1/2] drm/nouveau/kms/nv50-: Program notifier offset before requesting disp caps 2020-08-31 4:26 ` Ben Skeggs @ 2020-09-01 20:43 ` Lyude Paul 0 siblings, 0 replies; 9+ messages in thread From: Lyude Paul @ 2020-09-01 20:43 UTC (permalink / raw) To: Ben Skeggs Cc: ML nouveau, Sasha Levin, David Airlie, open list, open list:DRM DRIVER FOR NVIDIA GEFORCE/QUADRO GPUS, Ben Skeggs, Daniel Vetter, stable On Mon, 2020-08-31 at 14:26 +1000, Ben Skeggs wrote: > On Wed, 26 Aug 2020 at 02:52, Lyude Paul <lyude@redhat.com> wrote: > > On Tue, 2020-08-25 at 08:28 +1000, Ben Skeggs wrote: > > > On Tue, 25 Aug 2020 at 04:33, Lyude Paul <lyude@redhat.com> 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 0000:01:00.0: disp: chid 0 mthd 008c data 00000000 0000508c > > > > 0000102b > > > > > > > > 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. > > > > > > > > Changes since v1: > > > > * Don't just program the DMA notifier offset, make sure to actually > > > > perform an update > > > I'm not sure there's a need to send an Update() method here, I believe > > > GetCapabilities() is an action method on its own right? > > > > > > > I'm not entirely sure about this part tbh. I do know that we need to call > > GetCapabilities() _after_ the DMA notifier offset is programmed. But, my > > assumption was that if GetCapabilities() requires a DMA notifier offset to > > store > > its results in, we'd probably want to fire an update or something to make > > sure > > that we're not reading before it finishes writing capabilities? > We definitely want to *wait* on GetCapabilities() finishing, I believe > it should also update the notifier the same (or similar) way Update() > does. But I don't think we want to send an Update() here, it'll > actually trigger a modeset (which, on earlier HW, will tear down the > boot mode. Not sure about current HW, it might preserve state), and > we may not want that to happen there. I'm not so sure about that, as it seems like the notifier times out without the update: [ 5.142033] nouveau 0000:1f:00.0: DRM: [DRM/00000000:kmsChanPush] 00000000: 00040088 mthd 0x0088 size 1 - core507d_init [ 5.142037] nouveau 0000:1f:00.0: DRM: [DRM/00000000:kmsChanPush] 00000004: f0000000-> NV507D_SET_CONTEXT_DMA_NOTIFIER [ 5.142041] nouveau 0000:1f:00.0: DRM: [DRM/00000000:kmsChanPush] 00000008: 00040084 mthd 0x0084 size 1 - core507d_caps_init [ 5.142044] nouveau 0000:1f:00.0: DRM: [DRM/00000000:kmsChanPush] 0000000c: 80000000-> NV507D_SET_NOTIFIER_CONTROL [ 5.142047] nouveau 0000:1f:00.0: DRM: [DRM/00000000:kmsChanPush] 00000010: 0004008c mthd 0x008c size 1 - core507d_caps_init [ 5.142050] nouveau 0000:1f:00.0: DRM: [DRM/00000000:kmsChanPush] 00000014: 00000000-> NV507D_GET_CAPABILITIES [ 7.142026] nouveau 0000:1f:00.0: DRM: core notifier timeout [ 7.142700] nouveau 0000:1f:00.0: DRM: sor-0002-0fc1 caps: dp_interlace=0 [ 7.142708] nouveau 0000:1f:00.0: DRM: sor-0002-0fc4 caps: dp_interlace=0 [ 7.142715] nouveau 0000:1f:00.0: DRM: sor-0002-0f42 caps: dp_interlace=0 [ 7.142829] nouveau 0000:1f:00.0: DRM: sor-0006-0f82 caps: dp_interlace=0 [ 7.142842] nouveau 0000:1f:00.0: DRM: sor-0002-0f82 caps: dp_interlace=0 [ 7.142849] nouveau 0000:1f:00.0: DRM: failed to create encoder 1/8/0: -19 [ 7.142851] nouveau 0000:1f:00.0: DRM: Virtual-1 has no encoders, removing Any other alternatives to UPDATE we might want to try? > > Ben. > > > > Ben. > > > > > > > Signed-off-by: Lyude Paul <lyude@redhat.com> > > > > Fixes: 4a2cb4181b07 ("drm/nouveau/kms/nv50-: Probe SOR and PIOR caps for > > > > DP > > > > interlacing support") > > > > Cc: <stable@vger.kernel.org> # v5.8+ > > > > --- > > > > drivers/gpu/drm/nouveau/dispnv50/core507d.c | 25 ++++++++++++++++----- > > > > 1 file changed, 19 insertions(+), 6 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/nouveau/dispnv50/core507d.c > > > > b/drivers/gpu/drm/nouveau/dispnv50/core507d.c > > > > index e341f572c2696..5e86feec3b720 100644 > > > > --- a/drivers/gpu/drm/nouveau/dispnv50/core507d.c > > > > +++ b/drivers/gpu/drm/nouveau/dispnv50/core507d.c > > > > @@ -65,13 +65,26 @@ core507d_ntfy_init(struct nouveau_bo *bo, u32 > > > > offset) > > > > int > > > > core507d_caps_init(struct nouveau_drm *drm, struct nv50_disp *disp) > > > > { > > > > - u32 *push = evo_wait(&disp->core->chan, 2); > > > > + struct nv50_core *core = disp->core; > > > > + u32 interlock[NV50_DISP_INTERLOCK__SIZE] = {0}; > > > > + u32 *push; > > > > > > > > - if (push) { > > > > - evo_mthd(push, 0x008c, 1); > > > > - evo_data(push, 0x0); > > > > - evo_kick(push, &disp->core->chan); > > > > - } > > > > + core->func->ntfy_init(disp->sync, NV50_DISP_CORE_NTFY); > > > > + > > > > + push = evo_wait(&core->chan, 4); > > > > + if (!push) > > > > + return 0; > > > > + > > > > + evo_mthd(push, 0x0084, 1); > > > > + evo_data(push, 0x80000000 | NV50_DISP_CORE_NTFY); > > > > + evo_mthd(push, 0x008c, 1); > > > > + evo_data(push, 0x0); > > > > + evo_kick(push, &core->chan); > > > > + > > > > + core->func->update(core, interlock, false); > > > > + if (core->func->ntfy_wait_done(disp->sync, NV50_DISP_CORE_NTFY, > > > > + core->chan.base.device)) > > > > + NV_ERROR(drm, "core notifier timeout\n"); > > > > > > > > return 0; > > > > } > > > > -- > > > > 2.26.2 > > > > > > > > _______________________________________________ > > > > Nouveau mailing list > > > > Nouveau@lists.freedesktop.org > > > > https://lists.freedesktop.org/mailman/listinfo/nouveau > > -- > > Sincerely, > > Lyude Paul (she/her) > > Software Engineer at Red Hat > > -- Sincerely, Lyude Paul (she/her) Software Engineer at Red Hat ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3] drm/nouveau/kms/nv50-: Program notifier offset before requesting disp caps 2020-08-24 18:32 ` [PATCH 1/2] drm/nouveau/kms/nv50-: Program notifier offset before requesting disp caps Lyude Paul 2020-08-24 22:28 ` [Nouveau] " Ben Skeggs @ 2020-09-01 23:38 ` Lyude Paul 2020-09-01 23:42 ` [PATCH v4] " Lyude Paul 1 sibling, 1 reply; 9+ messages in thread From: Lyude Paul @ 2020-09-01 23:38 UTC (permalink / raw) To: nouveau Cc: stable, Ben Skeggs, David Airlie, Daniel Vetter, Dave Airlie, Christian König, Nirmoy Das, Jani Nikula, James Jones, open list:DRM DRIVER FOR NVIDIA GEFORCE/QUADRO GPUS, open list 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 0000:01:00.0: disp: chid 0 mthd 008c data 00000000 0000508c 0000102b 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. Signed-off-by: Lyude Paul <lyude@redhat.com> Fixes: 4a2cb4181b07 ("drm/nouveau/kms/nv50-: Probe SOR and PIOR caps for DP interlacing support") Cc: <stable@vger.kernel.org> # v5.8+ --- drivers/gpu/drm/nouveau/dispnv50/core.h | 2 ++ drivers/gpu/drm/nouveau/dispnv50/core507d.c | 34 +++++++++++++++++-- drivers/gpu/drm/nouveau/dispnv50/core907d.c | 33 +++++++++++++++++- 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, 77 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..3ec4c3a238c41 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/core507d.c +++ b/drivers/gpu/drm/nouveau/dispnv50/core507d.c @@ -75,18 +75,48 @@ 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, 0x00000000); + 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; + + NVBO_WR32(bo, NV50_DISP_CAPS_NTFY1, NV_DISP_CORE_NOTIFIER_1, CAPABILITIES_1, + NVDEF(NV_DISP_CORE_NOTIFIER_1, CAPABILITIES_1, DONE, FALSE)); + + core507d_read_caps(disp, NV50_DISP_CAPS_NTFY1); + + 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..8a2005adb0e2f 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/core907d.c +++ b/drivers/gpu/drm/nouveau/dispnv50/core907d.c @@ -22,11 +22,42 @@ #include "core.h" #include "head.h" +#include <nvif/push507c.h> +#include <nvif/timer.h> + +#include <nvhw/class/cl907d.h> + +#include "nouveau_bo.h" + +int +core907d_caps_init(struct nouveau_drm *drm, struct nv50_disp *disp) +{ + struct nv50_core *core = disp->core; + struct nouveau_bo *bo = disp->sync; + s64 time; + + NVBO_WR32(bo, NV50_DISP_CAPS_NTFY4, NV907D_CORE_NOTIFIER_3, CAPABILITIES_4, + NVDEF(NV907D_CORE_NOTIFIER_3, CAPABILITIES_4, DONE, FALSE)); + + core507d_read_caps(disp, NV50_DISP_CAPS_NTFY4); + + time = nvif_msec(core->chan.base.device, 2000ULL, + if (NVBO_TD32(bo, NV50_DISP_CAPS_NTFY4, + NV907D_CORE_NOTIFIER_3, CAPABILITIES_4, DONE, ==, TRUE)) + break; + usleep_range(1, 2); + ); + if (time < 0) + NV_ERROR(drm, "core caps notifier timeout\n"); + + return 0; +} + static const struct nv50_core_func core907d = { .init = core507d_init, .ntfy_init = core507d_ntfy_init, - .caps_init = core507d_caps_init, + .caps_init = core907d_caps_init, .ntfy_wait_done = core507d_ntfy_wait_done, .update = core507d_update, .head = &head907d, diff --git a/drivers/gpu/drm/nouveau/dispnv50/core917d.c b/drivers/gpu/drm/nouveau/dispnv50/core917d.c index 66846f3720805..1cd3a2a35dfb7 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/core917d.c +++ b/drivers/gpu/drm/nouveau/dispnv50/core917d.c @@ -26,7 +26,7 @@ static const struct nv50_core_func core917d = { .init = core507d_init, .ntfy_init = core507d_ntfy_init, - .caps_init = core507d_caps_init, + .caps_init = core907d_caps_init, .ntfy_wait_done = core507d_ntfy_wait_done, .update = core507d_update, .head = &head917d, diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.h b/drivers/gpu/drm/nouveau/dispnv50/disp.h index 92bddc0836171..a59051bd070d7 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/disp.h +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.h @@ -16,6 +16,8 @@ struct nv50_disp { #define NV50_DISP_SYNC(c, o) ((c) * 0x040 + (o)) #define NV50_DISP_CORE_NTFY NV50_DISP_SYNC(0 , 0x00) +#define NV50_DISP_CAPS_NTFY1 NV50_DISP_SYNC(0 , 0x01) +#define NV50_DISP_CAPS_NTFY4 NV50_DISP_SYNC(0 , 0x04) #define NV50_DISP_WNDW_SEM0(c) NV50_DISP_SYNC(1 + (c), 0x00) #define NV50_DISP_WNDW_SEM1(c) NV50_DISP_SYNC(1 + (c), 0x10) #define NV50_DISP_WNDW_NTFY(c) NV50_DISP_SYNC(1 + (c), 0x20) diff --git a/drivers/gpu/drm/nouveau/include/nvhw/class/cl507d.h b/drivers/gpu/drm/nouveau/include/nvhw/class/cl507d.h index 2e444bac701dd..6a463f308b64f 100644 --- a/drivers/gpu/drm/nouveau/include/nvhw/class/cl507d.h +++ b/drivers/gpu/drm/nouveau/include/nvhw/class/cl507d.h @@ -32,7 +32,10 @@ #define NV_DISP_CORE_NOTIFIER_1_COMPLETION_0_DONE_TRUE 0x00000001 #define NV_DISP_CORE_NOTIFIER_1_COMPLETION_0_R0 15:1 #define NV_DISP_CORE_NOTIFIER_1_COMPLETION_0_TIMESTAMP 29:16 - +#define NV_DISP_CORE_NOTIFIER_1_CAPABILITIES_1 0x00000001 +#define NV_DISP_CORE_NOTIFIER_1_CAPABILITIES_1_DONE 0:0 +#define NV_DISP_CORE_NOTIFIER_1_CAPABILITIES_1_DONE_FALSE 0x00000000 +#define NV_DISP_CORE_NOTIFIER_1_CAPABILITIES_1_DONE_TRUE 0x00000001 // class methods #define NV507D_UPDATE (0x00000080) diff --git a/drivers/gpu/drm/nouveau/include/nvhw/class/cl907d.h b/drivers/gpu/drm/nouveau/include/nvhw/class/cl907d.h index 34bc3eafac7d1..79aff6ff31385 100644 --- a/drivers/gpu/drm/nouveau/include/nvhw/class/cl907d.h +++ b/drivers/gpu/drm/nouveau/include/nvhw/class/cl907d.h @@ -24,6 +24,10 @@ #ifndef _cl907d_h_ #define _cl907d_h_ +#define NV907D_CORE_NOTIFIER_3_CAPABILITIES_4 0x00000004 +#define NV907D_CORE_NOTIFIER_3_CAPABILITIES_4_DONE 0:0 +#define NV907D_CORE_NOTIFIER_3_CAPABILITIES_4_DONE_FALSE 0x00000000 +#define NV907D_CORE_NOTIFIER_3_CAPABILITIES_4_DONE_TRUE 0x00000001 #define NV907D_CORE_NOTIFIER_3_CAPABILITIES_CAP_SOR0_20 0x00000014 #define NV907D_CORE_NOTIFIER_3_CAPABILITIES_CAP_SOR0_20_SINGLE_LVDS18 0:0 #define NV907D_CORE_NOTIFIER_3_CAPABILITIES_CAP_SOR0_20_SINGLE_LVDS18_FALSE 0x00000000 -- 2.26.2 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v4] drm/nouveau/kms/nv50-: Program notifier offset before requesting disp caps 2020-09-01 23:38 ` [PATCH v3] " Lyude Paul @ 2020-09-01 23:42 ` Lyude Paul 2020-09-03 5:15 ` [Nouveau] " Ben Skeggs 0 siblings, 1 reply; 9+ messages in thread From: Lyude Paul @ 2020-09-01 23:42 UTC (permalink / raw) To: nouveau Cc: stable, Ben Skeggs, David Airlie, Daniel Vetter, Dave Airlie, Christian König, Nirmoy Das, Ville Syrjälä, James Jones, Jani Nikula, open list:DRM DRIVER FOR NVIDIA GEFORCE/QUADRO GPUS, open list 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 0000:01:00.0: disp: chid 0 mthd 008c data 00000000 0000508c 0000102b 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 <lyude@redhat.com> Fixes: 4a2cb4181b07 ("drm/nouveau/kms/nv50-: Probe SOR and PIOR caps for DP interlacing support") Cc: <stable@vger.kernel.org> # 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, 0x00000000); + 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 @@ #include "core.h" #include "head.h" +#include <nvif/push507c.h> +#include <nvif/timer.h> + +#include <nvhw/class/cl907d.h> + +#include "nouveau_bo.h" + +int +core907d_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_NTFY4, NV907D_CORE_NOTIFIER_3, CAPABILITIES_4, + NVDEF(NV907D_CORE_NOTIFIER_3, CAPABILITIES_4, DONE, FALSE)); + + ret = core507d_read_caps(disp, NV50_DISP_CAPS_NTFY4); + if (ret < 0) + return ret; + + time = nvif_msec(core->chan.base.device, 2000ULL, + if (NVBO_TD32(bo, NV50_DISP_CAPS_NTFY4, + NV907D_CORE_NOTIFIER_3, CAPABILITIES_4, DONE, ==, TRUE)) + break; + usleep_range(1, 2); + ); + if (time < 0) + NV_ERROR(drm, "core caps notifier timeout\n"); + + return 0; +} + static const struct nv50_core_func core907d = { .init = core507d_init, .ntfy_init = core507d_ntfy_init, - .caps_init = core507d_caps_init, + .caps_init = core907d_caps_init, .ntfy_wait_done = core507d_ntfy_wait_done, .update = core507d_update, .head = &head907d, diff --git a/drivers/gpu/drm/nouveau/dispnv50/core917d.c b/drivers/gpu/drm/nouveau/dispnv50/core917d.c index 66846f3720805..1cd3a2a35dfb7 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/core917d.c +++ b/drivers/gpu/drm/nouveau/dispnv50/core917d.c @@ -26,7 +26,7 @@ static const struct nv50_core_func core917d = { .init = core507d_init, .ntfy_init = core507d_ntfy_init, - .caps_init = core507d_caps_init, + .caps_init = core907d_caps_init, .ntfy_wait_done = core507d_ntfy_wait_done, .update = core507d_update, .head = &head917d, diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.h b/drivers/gpu/drm/nouveau/dispnv50/disp.h index 92bddc0836171..a59051bd070d7 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/disp.h +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.h @@ -16,6 +16,8 @@ struct nv50_disp { #define NV50_DISP_SYNC(c, o) ((c) * 0x040 + (o)) #define NV50_DISP_CORE_NTFY NV50_DISP_SYNC(0 , 0x00) +#define NV50_DISP_CAPS_NTFY1 NV50_DISP_SYNC(0 , 0x01) +#define NV50_DISP_CAPS_NTFY4 NV50_DISP_SYNC(0 , 0x04) #define NV50_DISP_WNDW_SEM0(c) NV50_DISP_SYNC(1 + (c), 0x00) #define NV50_DISP_WNDW_SEM1(c) NV50_DISP_SYNC(1 + (c), 0x10) #define NV50_DISP_WNDW_NTFY(c) NV50_DISP_SYNC(1 + (c), 0x20) diff --git a/drivers/gpu/drm/nouveau/include/nvhw/class/cl507d.h b/drivers/gpu/drm/nouveau/include/nvhw/class/cl507d.h index 2e444bac701dd..6a463f308b64f 100644 --- a/drivers/gpu/drm/nouveau/include/nvhw/class/cl507d.h +++ b/drivers/gpu/drm/nouveau/include/nvhw/class/cl507d.h @@ -32,7 +32,10 @@ #define NV_DISP_CORE_NOTIFIER_1_COMPLETION_0_DONE_TRUE 0x00000001 #define NV_DISP_CORE_NOTIFIER_1_COMPLETION_0_R0 15:1 #define NV_DISP_CORE_NOTIFIER_1_COMPLETION_0_TIMESTAMP 29:16 - +#define NV_DISP_CORE_NOTIFIER_1_CAPABILITIES_1 0x00000001 +#define NV_DISP_CORE_NOTIFIER_1_CAPABILITIES_1_DONE 0:0 +#define NV_DISP_CORE_NOTIFIER_1_CAPABILITIES_1_DONE_FALSE 0x00000000 +#define NV_DISP_CORE_NOTIFIER_1_CAPABILITIES_1_DONE_TRUE 0x00000001 // class methods #define NV507D_UPDATE (0x00000080) diff --git a/drivers/gpu/drm/nouveau/include/nvhw/class/cl907d.h b/drivers/gpu/drm/nouveau/include/nvhw/class/cl907d.h index 34bc3eafac7d1..79aff6ff31385 100644 --- a/drivers/gpu/drm/nouveau/include/nvhw/class/cl907d.h +++ b/drivers/gpu/drm/nouveau/include/nvhw/class/cl907d.h @@ -24,6 +24,10 @@ #ifndef _cl907d_h_ #define _cl907d_h_ +#define NV907D_CORE_NOTIFIER_3_CAPABILITIES_4 0x00000004 +#define NV907D_CORE_NOTIFIER_3_CAPABILITIES_4_DONE 0:0 +#define NV907D_CORE_NOTIFIER_3_CAPABILITIES_4_DONE_FALSE 0x00000000 +#define NV907D_CORE_NOTIFIER_3_CAPABILITIES_4_DONE_TRUE 0x00000001 #define NV907D_CORE_NOTIFIER_3_CAPABILITIES_CAP_SOR0_20 0x00000014 #define NV907D_CORE_NOTIFIER_3_CAPABILITIES_CAP_SOR0_20_SINGLE_LVDS18 0:0 #define NV907D_CORE_NOTIFIER_3_CAPABILITIES_CAP_SOR0_20_SINGLE_LVDS18_FALSE 0x00000000 -- 2.26.2 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Nouveau] [PATCH v4] drm/nouveau/kms/nv50-: Program notifier offset before requesting disp caps 2020-09-01 23:42 ` [PATCH v4] " Lyude Paul @ 2020-09-03 5:15 ` Ben Skeggs 0 siblings, 0 replies; 9+ messages in thread From: Ben Skeggs @ 2020-09-03 5:15 UTC (permalink / raw) To: Lyude Paul Cc: ML nouveau, open list:DRM DRIVER FOR NVIDIA GEFORCE/QUADRO GPUS, David Airlie, open list, stable, Jani Nikula, Ben Skeggs, Daniel Vetter, Christian König, Ville Syrjälä, Nirmoy Das On Wed, 2 Sep 2020 at 09:43, Lyude Paul <lyude@redhat.com> 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 0000:01:00.0: disp: chid 0 mthd 008c data 00000000 0000508c 0000102b > > 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 <lyude@redhat.com> > Fixes: 4a2cb4181b07 ("drm/nouveau/kms/nv50-: Probe SOR and PIOR caps for DP interlacing support") > Cc: <stable@vger.kernel.org> # 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, 0x00000000); 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 reading CAPABILITIES_5 (which, weirdly, seems to contain an identical value). > + > + 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 @@ > #include "core.h" > #include "head.h" > > +#include <nvif/push507c.h> > +#include <nvif/timer.h> > + > +#include <nvhw/class/cl907d.h> > + > +#include "nouveau_bo.h" > + > +int > +core907d_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_NTFY4, NV907D_CORE_NOTIFIER_3, CAPABILITIES_4, > + NVDEF(NV907D_CORE_NOTIFIER_3, CAPABILITIES_4, DONE, FALSE)); > + > + ret = core507d_read_caps(disp, NV50_DISP_CAPS_NTFY4); > + if (ret < 0) > + return ret; > + > + time = nvif_msec(core->chan.base.device, 2000ULL, > + if (NVBO_TD32(bo, NV50_DISP_CAPS_NTFY4, > + NV907D_CORE_NOTIFIER_3, CAPABILITIES_4, DONE, ==, TRUE)) > + break; > + usleep_range(1, 2); > + ); > + if (time < 0) > + NV_ERROR(drm, "core caps notifier timeout\n"); > + > + return 0; > +} > + > static const struct nv50_core_func > core907d = { > .init = core507d_init, > .ntfy_init = core507d_ntfy_init, > - .caps_init = core507d_caps_init, > + .caps_init = core907d_caps_init, > .ntfy_wait_done = core507d_ntfy_wait_done, > .update = core507d_update, > .head = &head907d, > diff --git a/drivers/gpu/drm/nouveau/dispnv50/core917d.c b/drivers/gpu/drm/nouveau/dispnv50/core917d.c > index 66846f3720805..1cd3a2a35dfb7 100644 > --- a/drivers/gpu/drm/nouveau/dispnv50/core917d.c > +++ b/drivers/gpu/drm/nouveau/dispnv50/core917d.c > @@ -26,7 +26,7 @@ static const struct nv50_core_func > core917d = { > .init = core507d_init, > .ntfy_init = core507d_ntfy_init, > - .caps_init = core507d_caps_init, > + .caps_init = core907d_caps_init, > .ntfy_wait_done = core507d_ntfy_wait_done, > .update = core507d_update, > .head = &head917d, > diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.h b/drivers/gpu/drm/nouveau/dispnv50/disp.h > index 92bddc0836171..a59051bd070d7 100644 > --- a/drivers/gpu/drm/nouveau/dispnv50/disp.h > +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.h > @@ -16,6 +16,8 @@ struct nv50_disp { > > #define NV50_DISP_SYNC(c, o) ((c) * 0x040 + (o)) > #define NV50_DISP_CORE_NTFY NV50_DISP_SYNC(0 , 0x00) > +#define NV50_DISP_CAPS_NTFY1 NV50_DISP_SYNC(0 , 0x01) > +#define NV50_DISP_CAPS_NTFY4 NV50_DISP_SYNC(0 , 0x04) > #define NV50_DISP_WNDW_SEM0(c) NV50_DISP_SYNC(1 + (c), 0x00) > #define NV50_DISP_WNDW_SEM1(c) NV50_DISP_SYNC(1 + (c), 0x10) > #define NV50_DISP_WNDW_NTFY(c) NV50_DISP_SYNC(1 + (c), 0x20) > diff --git a/drivers/gpu/drm/nouveau/include/nvhw/class/cl507d.h b/drivers/gpu/drm/nouveau/include/nvhw/class/cl507d.h > index 2e444bac701dd..6a463f308b64f 100644 > --- a/drivers/gpu/drm/nouveau/include/nvhw/class/cl507d.h > +++ b/drivers/gpu/drm/nouveau/include/nvhw/class/cl507d.h > @@ -32,7 +32,10 @@ > #define NV_DISP_CORE_NOTIFIER_1_COMPLETION_0_DONE_TRUE 0x00000001 > #define NV_DISP_CORE_NOTIFIER_1_COMPLETION_0_R0 15:1 > #define NV_DISP_CORE_NOTIFIER_1_COMPLETION_0_TIMESTAMP 29:16 > - > +#define NV_DISP_CORE_NOTIFIER_1_CAPABILITIES_1 0x00000001 > +#define NV_DISP_CORE_NOTIFIER_1_CAPABILITIES_1_DONE 0:0 > +#define NV_DISP_CORE_NOTIFIER_1_CAPABILITIES_1_DONE_FALSE 0x00000000 > +#define NV_DISP_CORE_NOTIFIER_1_CAPABILITIES_1_DONE_TRUE 0x00000001 > > // class methods > #define NV507D_UPDATE (0x00000080) > diff --git a/drivers/gpu/drm/nouveau/include/nvhw/class/cl907d.h b/drivers/gpu/drm/nouveau/include/nvhw/class/cl907d.h > index 34bc3eafac7d1..79aff6ff31385 100644 > --- a/drivers/gpu/drm/nouveau/include/nvhw/class/cl907d.h > +++ b/drivers/gpu/drm/nouveau/include/nvhw/class/cl907d.h > @@ -24,6 +24,10 @@ > #ifndef _cl907d_h_ > #define _cl907d_h_ > > +#define NV907D_CORE_NOTIFIER_3_CAPABILITIES_4 0x00000004 > +#define NV907D_CORE_NOTIFIER_3_CAPABILITIES_4_DONE 0:0 > +#define NV907D_CORE_NOTIFIER_3_CAPABILITIES_4_DONE_FALSE 0x00000000 > +#define NV907D_CORE_NOTIFIER_3_CAPABILITIES_4_DONE_TRUE 0x00000001 > #define NV907D_CORE_NOTIFIER_3_CAPABILITIES_CAP_SOR0_20 0x00000014 > #define NV907D_CORE_NOTIFIER_3_CAPABILITIES_CAP_SOR0_20_SINGLE_LVDS18 0:0 > #define NV907D_CORE_NOTIFIER_3_CAPABILITIES_CAP_SOR0_20_SINGLE_LVDS18_FALSE 0x00000000 > -- > 2.26.2 > > _______________________________________________ > Nouveau mailing list > Nouveau@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/nouveau ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] drm/nouveau/kms/nv50-: Log SOR/PIOR caps [not found] <20200824183253.826343-1-lyude@redhat.com> 2020-08-24 18:32 ` [PATCH 1/2] drm/nouveau/kms/nv50-: Program notifier offset before requesting disp caps Lyude Paul @ 2020-08-24 18:32 ` Lyude Paul 1 sibling, 0 replies; 9+ messages in thread From: Lyude Paul @ 2020-08-24 18:32 UTC (permalink / raw) To: nouveau, Sasha Levin Cc: Ben Skeggs, David Airlie, Daniel Vetter, Alex Deucher, Pankaj Bharadiya, Manasi Navare, Takashi Iwai, James Jones, open list:DRM DRIVER FOR NVIDIA GEFORCE/QUADRO GPUS, open list Since I'm almost certain I didn't get capability checking right for pre-volta chipsets, let's start logging any caps we find to make things like this obvious in the future. Signed-off-by: Lyude Paul <lyude@redhat.com> --- drivers/gpu/drm/nouveau/dispnv50/disp.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c index 800b7757252e3..6210ee1c145a2 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c @@ -258,6 +258,14 @@ evo_kick(u32 *push, struct nv50_dmac *evoc) /****************************************************************************** * Output path helpers *****************************************************************************/ +static void +nv50_outp_dump_caps(struct nouveau_drm *drm, + struct nouveau_encoder *outp) +{ + NV_DEBUG(drm, "%s caps: dp_interlace=%d\n", + outp->base.base.name, outp->caps.dp_interlace); +} + static void nv50_outp_release(struct nouveau_encoder *nv_encoder) { @@ -1715,6 +1723,7 @@ nv50_sor_create(struct drm_connector *connector, struct dcb_output *dcbe) drm_connector_attach_encoder(connector, encoder); disp->core->func->sor->get_caps(disp, nv_encoder, ffs(dcbe->or) - 1); + nv50_outp_dump_caps(drm, nv_encoder); if (dcbe->type == DCB_OUTPUT_DP) { struct nvkm_i2c_aux *aux = @@ -1875,6 +1884,7 @@ nv50_pior_create(struct drm_connector *connector, struct dcb_output *dcbe) drm_connector_attach_encoder(connector, encoder); disp->core->func->pior->get_caps(disp, nv_encoder, ffs(dcbe->or) - 1); + nv50_outp_dump_caps(drm, nv_encoder); return 0; } -- 2.26.2 ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-09-03 5:16 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20200824183253.826343-1-lyude@redhat.com>
2020-08-24 18:32 ` [PATCH 1/2] drm/nouveau/kms/nv50-: Program notifier offset before requesting disp caps Lyude Paul
2020-08-24 22:28 ` [Nouveau] " Ben Skeggs
2020-08-25 16:52 ` Lyude Paul
2020-08-31 4:26 ` Ben Skeggs
2020-09-01 20:43 ` Lyude Paul
2020-09-01 23:38 ` [PATCH v3] " Lyude Paul
2020-09-01 23:42 ` [PATCH v4] " Lyude Paul
2020-09-03 5:15 ` [Nouveau] " Ben Skeggs
2020-08-24 18:32 ` [PATCH 2/2] drm/nouveau/kms/nv50-: Log SOR/PIOR caps Lyude Paul
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®