Re: [PATCH] drm/nouveau/kms/nvd9-: Use contiguous memory for CRC notifier context

[Dave Airlie <airlied@gmail.com>]

On Sat, 2 May 2026 at 07:59, Lyude Paul <lyude@redhat.com> wrote:
>
> It looks like CRC read back has been slightly broken for a while now, in
> particular on GPUs using GSP. On my test machines, it's worked normally
> when attempting to use it from fbcon. After gnome-shell gets started
> however, attempting to read /sys/kernel/debug/dri/$CARD/$CRTC/crc/data just
> returns -EINVAL.
>
> It turns out what's been happening is that since we've been using
> nvif_mem_ctor_map() to both allocate and map the CRC notifier region - we
> haven't actually asked for a contiguous allocation, and simply ask for
> whatever type of memory allocation nouveau can find first. This doesn't
> work because the CRC engine on nvidia GPUs doesn't support non-contiguous
> allocations, which also causes us to fail setting up the kmsCrcNtfyCtxDma
> object on pre-blackwell platforms since we don't have a single memory
> address we can point nvif_object_ctor() to. Instead, ctx->mem.addr gets set
> to ~0ULL.
>
> It does however, seem to work when fbcon is running. The only reason I can
> think of this is that before we start up a display environment, there is
> pretty much nothing allocated in our VRAM that wasn't allocated by nouveau
> itself - making it dramatically more likely that we end up finding a
> contiguous allocation by default.
>
> So, fix this by manually requesting a contiguous allocation when we
> allocate our context notifiers.
>
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> Fixes: 12885ecbfe62 ("drm/nouveau/kms/nvd9-: Add CRC support")
> Cc: Lyude Paul <lyude@redhat.com>
> Cc: Dave Airlie <airlied@gmail.com>
> Cc: Danilo Krummrich <dakr@kernel.org>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Timur Tabi <ttabi@nvidia.com>
> Cc: Suraj Kandpal <suraj.kandpal@intel.com>
> Cc: James Jones <jajones@nvidia.com>
> Cc: Faith Ekstrand <faith.ekstrand@collabora.com>
> Cc: dri-devel@lists.freedesktop.org
> Cc: nouveau@lists.freedesktop.org
> Cc: <stable@vger.kernel.org> # v5.9+
> ---
>  drivers/gpu/drm/nouveau/dispnv50/crc.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/crc.c b/drivers/gpu/drm/nouveau/dispnv50/crc.c
> index deb6af40ef328..5817f39934a8b 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/crc.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/crc.c
> @@ -10,6 +10,7 @@
>  #include <nvif/class.h>
>  #include <nvif/cl0002.h>
>  #include <nvif/timer.h>
> +#include <nvif/if900b.h>
>
>  #include <nvhw/class/cl907d.h>
>
> @@ -499,16 +500,24 @@ nv50_crc_raster_type(enum nv50_crc_source source)
>   * notifier needs it's own handle
>   */
>  static inline int
> -nv50_crc_ctx_init(struct nv50_head *head, struct nvif_mmu *mmu,
> +nv50_crc_ctx_init(struct drm_device *dev, struct nv50_head *head, struct nvif_mmu *mmu,
>                   struct nv50_crc_notifier_ctx *ctx, size_t len, int idx)
>  {
> -       struct nv50_core *core = nv50_disp(head->base.base.dev)->core;
> +       struct nv50_core *core = nv50_disp(dev)->core;
>         int ret;
>
> -       ret = nvif_mem_ctor_map(mmu, "kmsCrcNtfy", NVIF_MEM_VRAM, len, &ctx->mem);
> +       /* The display engine requires a contiguous region of memory for the CRC notifier context */
> +       ret = nvif_mem_ctor(mmu, "kmsCrcNtfy", mmu->mem, NVIF_MEM_VRAM | NVIF_MEM_MAPPABLE, 0, len,
> +                           &(struct gf100_mem_v0) {
> +                               .contig = true,
> +                           }, sizeof(struct gf100_mem_v0), &ctx->mem);

I don't think you can just throw gf100_mem_v0 in here like that, I
think you should maybe make nvif_mem_ctor_map deal with this since you
want a kernel mappable vram region it needs contig.

Dave.