From: Ben Skeggs bskeggs@redhat.com
The code which constructs the modules for each engine present on the GPU passes -1 for 'instance' on non-instanced engines, which affects how the name for a sub-device is generated. This is then stored as 'instance 0' in nvkm_subdev.inst, so code can potentially be shared with earlier GPUs that only had a single instance of an engine.
However, GF100's CE constructor uses this value to calculate the address of its falcon before it's translated, resulting in CE0 getting the wrong address.
This slightly modifies the approach, always passing a valid instance for engines that *can* have multiple copies, and having the code for earlier GPUs explicitly ask for non-instanced name generation.
Bug: https://gitlab.freedesktop.org/drm/nouveau/-/issues/91
Fixes: 50551b15c760 ("drm/nouveau/ce: switch to instanced constructor") Cc: stable@vger.kernel.org # v5.12+ Signed-off-by: Ben Skeggs bskeggs@redhat.com --- drivers/gpu/drm/nouveau/nvkm/engine/ce/gt215.c | 2 +- drivers/gpu/drm/nouveau/nvkm/engine/device/base.c | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/ce/gt215.c b/drivers/gpu/drm/nouveau/nvkm/engine/ce/gt215.c index 704df0f2d1f1..09a112af2f89 100644 --- a/drivers/gpu/drm/nouveau/nvkm/engine/ce/gt215.c +++ b/drivers/gpu/drm/nouveau/nvkm/engine/ce/gt215.c @@ -78,6 +78,6 @@ int gt215_ce_new(struct nvkm_device *device, enum nvkm_subdev_type type, int inst, struct nvkm_engine **pengine) { - return nvkm_falcon_new_(>215_ce, device, type, inst, + return nvkm_falcon_new_(>215_ce, device, type, -1, (device->chipset != 0xaf), 0x104000, pengine); } diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c b/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c index ca75c5f6ecaf..b51d690f375f 100644 --- a/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c +++ b/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c @@ -3147,8 +3147,7 @@ nvkm_device_ctor(const struct nvkm_device_func *func, WARN_ON(device->chip->ptr.inst & ~((1 << ARRAY_SIZE(device->ptr)) - 1)); \ for (j = 0; device->chip->ptr.inst && j < ARRAY_SIZE(device->ptr); j++) { \ if ((device->chip->ptr.inst & BIT(j)) && (subdev_mask & BIT_ULL(type))) { \ - int inst = (device->chip->ptr.inst == 1) ? -1 : (j); \ - ret = device->chip->ptr.ctor(device, (type), inst, &device->ptr[j]); \ + ret = device->chip->ptr.ctor(device, (type), (j), &device->ptr[j]); \ subdev = nvkm_device_subdev(device, (type), (j)); \ if (ret) { \ nvkm_subdev_del(&subdev); \
On Wed, Nov 3, 2021 at 2:11 AM Ben Skeggs skeggsb@gmail.com wrote:
From: Ben Skeggs bskeggs@redhat.com
The code which constructs the modules for each engine present on the GPU passes -1 for 'instance' on non-instanced engines, which affects how the name for a sub-device is generated. This is then stored as 'instance 0' in nvkm_subdev.inst, so code can potentially be shared with earlier GPUs that only had a single instance of an engine.
However, GF100's CE constructor uses this value to calculate the address of its falcon before it's translated, resulting in CE0 getting the wrong address.
This slightly modifies the approach, always passing a valid instance for engines that *can* have multiple copies, and having the code for earlier GPUs explicitly ask for non-instanced name generation.
Bug: https://gitlab.freedesktop.org/drm/nouveau/-/issues/91
Fixes: 50551b15c760 ("drm/nouveau/ce: switch to instanced constructor") Cc: stable@vger.kernel.org # v5.12+ Signed-off-by: Ben Skeggs bskeggs@redhat.com
drivers/gpu/drm/nouveau/nvkm/engine/ce/gt215.c | 2 +- drivers/gpu/drm/nouveau/nvkm/engine/device/base.c | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/ce/gt215.c b/drivers/gpu/drm/nouveau/nvkm/engine/ce/gt215.c index 704df0f2d1f1..09a112af2f89 100644 --- a/drivers/gpu/drm/nouveau/nvkm/engine/ce/gt215.c +++ b/drivers/gpu/drm/nouveau/nvkm/engine/ce/gt215.c @@ -78,6 +78,6 @@ int gt215_ce_new(struct nvkm_device *device, enum nvkm_subdev_type type, int inst, struct nvkm_engine **pengine) {
return nvkm_falcon_new_(>215_ce, device, type, inst,
return nvkm_falcon_new_(>215_ce, device, type, -1, (device->chipset != 0xaf), 0x104000, pengine);} diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c b/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c index ca75c5f6ecaf..b51d690f375f 100644 --- a/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c +++ b/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c @@ -3147,8 +3147,7 @@ nvkm_device_ctor(const struct nvkm_device_func *func, WARN_ON(device->chip->ptr.inst & ~((1 << ARRAY_SIZE(device->ptr)) - 1)); \ for (j = 0; device->chip->ptr.inst && j < ARRAY_SIZE(device->ptr); j++) { \ if ((device->chip->ptr.inst & BIT(j)) && (subdev_mask & BIT_ULL(type))) { \
int inst = (device->chip->ptr.inst == 1) ? -1 : (j); \ret = device->chip->ptr.ctor(device, (type), inst, &device->ptr[j]); \
ret = device->chip->ptr.ctor(device, (type), (j), &device->ptr[j]); \ subdev = nvkm_device_subdev(device, (type), (j)); \ if (ret) { \ nvkm_subdev_del(&subdev); \-- 2.31.1
Reviewed-by: Karol Herbst kherbst@redhat.com
On Wed, Nov 3, 2021 at 8:51 AM Karol Herbst kherbst@redhat.com wrote:
On Wed, Nov 3, 2021 at 2:11 AM Ben Skeggs skeggsb@gmail.com wrote:
From: Ben Skeggs bskeggs@redhat.com
The code which constructs the modules for each engine present on the GPU passes -1 for 'instance' on non-instanced engines, which affects how the name for a sub-device is generated. This is then stored as 'instance 0' in nvkm_subdev.inst, so code can potentially be shared with earlier GPUs that only had a single instance of an engine.
However, GF100's CE constructor uses this value to calculate the address of its falcon before it's translated, resulting in CE0 getting the wrong address.
This slightly modifies the approach, always passing a valid instance for engines that *can* have multiple copies, and having the code for earlier GPUs explicitly ask for non-instanced name generation.
Bug: https://gitlab.freedesktop.org/drm/nouveau/-/issues/91
Fixes: 50551b15c760 ("drm/nouveau/ce: switch to instanced constructor") Cc: stable@vger.kernel.org # v5.12+ Signed-off-by: Ben Skeggs bskeggs@redhat.com
drivers/gpu/drm/nouveau/nvkm/engine/ce/gt215.c | 2 +- drivers/gpu/drm/nouveau/nvkm/engine/device/base.c | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/ce/gt215.c b/drivers/gpu/drm/nouveau/nvkm/engine/ce/gt215.c index 704df0f2d1f1..09a112af2f89 100644 --- a/drivers/gpu/drm/nouveau/nvkm/engine/ce/gt215.c +++ b/drivers/gpu/drm/nouveau/nvkm/engine/ce/gt215.c @@ -78,6 +78,6 @@ int gt215_ce_new(struct nvkm_device *device, enum nvkm_subdev_type type, int inst, struct nvkm_engine **pengine) {
return nvkm_falcon_new_(>215_ce, device, type, inst,
return nvkm_falcon_new_(>215_ce, device, type, -1, (device->chipset != 0xaf), 0x104000, pengine);} diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c b/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c index ca75c5f6ecaf..b51d690f375f 100644 --- a/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c +++ b/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c @@ -3147,8 +3147,7 @@ nvkm_device_ctor(const struct nvkm_device_func *func, WARN_ON(device->chip->ptr.inst & ~((1 << ARRAY_SIZE(device->ptr)) - 1)); \ for (j = 0; device->chip->ptr.inst && j < ARRAY_SIZE(device->ptr); j++) { \ if ((device->chip->ptr.inst & BIT(j)) && (subdev_mask & BIT_ULL(type))) { \
int inst = (device->chip->ptr.inst == 1) ? -1 : (j); \ret = device->chip->ptr.ctor(device, (type), inst, &device->ptr[j]); \
ret = device->chip->ptr.ctor(device, (type), (j), &device->ptr[j]); \ subdev = nvkm_device_subdev(device, (type), (j)); \ if (ret) { \ nvkm_subdev_del(&subdev); \-- 2.31.1
Reviewed-by: Karol Herbst kherbst@redhat.com
Tested that on a GF108, so
Tested-by: Karol Herbst kherbst@redhat.com
linux-stable-mirror@lists.linaro.org
-
Ben Skeggs -
Karol Herbst