On Thu, Aug 08, 2019 at 02:29:34PM -0700, Ralph Campbell wrote: >>> { >>> struct nouveau_fence *fence; >>> unsigned long addr = args->start, nr_dma = 0, i; >>> for (i = 0; addr < args->end; i++) { >>> args->dst[i] = nouveau_dmem_migrate_copy_one(drm, args->vma, >>> - addr, args->src[i], &dma_addrs[nr_dma]); >>> + args->src[i], &dma_addrs[nr_dma], &pfns[i]); >> >> Nit: I find the &pfns[i] way to pass the argument a little weird to read. >> Why not "pfns + i"? > > OK, will do in v2. > Should I convert to "dma_addrs + nr_dma" too? I'll fix it up for v3 of the migrate_vma series. This is a leftover from passing an args structure. On something vaguely related to this patch: You use the NVIF_VMM_PFNMAP_V0_V* defines from nvif/if000c.h, which are a little odd as we only ever set these bits, but they also don't seem to appear to be in values that are directly fed to the hardware. On the other hand mmu/vmm.h defines a set of NVIF_VMM_PFNMAP_V0_* constants with similar names and identical values, and those are used in mmu/vmmgp100.c and what appears to finally do the low-level dma mapping and talking to the hardware. Are these two sets of constants supposed to be the same? Are the actual hardware values or just a driver internal interface?