On 2/3/25 3:53 AM, Danilo Krummrich wrote: > Hi Lina, > > On Sun, Feb 02, 2025 at 10:34:49PM +0900, Asahi Lina wrote: >> Some hardware requires dummy page mappings to efficiently implement >> Vulkan sparse features. These mappings consist of the same physical >> memory page, repeated for a large range of address space (e.g. 16GiB). >> >> Add support for this to drm_gpuvm. Currently, drm_gpuvm expects BO >> ranges to correspond 1:1 to virtual memory ranges that are mapped, and >> does math on the BO offset accordingly. To make single page mappings >> work, we need a way to turn off that math, keeping the BO offset always >> constant and pointing to the same page (typically BO offset 0). >> >> To make this work, we need to handle all the corner cases when these >> mappings intersect with regular mappings. The rules are simply to never >> mix or merge a "regular" mapping with a single page mapping. >> >> drm_gpuvm has support for a flags field in drm_gpuva objects. This is >> normally managed by drivers directly. We can introduce a >> DRM_GPUVA_SINGLE_PAGE flag to handle this. However, to make it work, >> sm_map and friends need to know ahead of time whether the new mapping is >> a single page mapping or not. Therefore, we need to add an argument to >> these functions so drivers can provide the flags to be filled into >> drm_gpuva.flags. >> >> These changes should not affect any existing drivers that use drm_gpuvm >> other than the API change: >> >> - imagination: Does not use flags at all >> - nouveau: Only uses drm_gpuva_invalidate(), which is only called on >> existing drm_gpuva objects (after the map steps) >> - panthor: Does not use flags at all >> - xe: Does not use drm_gpuva_init_from_op() or >> drm_gpuva_remap()/drm_gpuva_map() (which call it). This means that the >> flags field of the gpuva object is managed by the driver only, so these >> changes cannot clobber it. >> >> Note that the way this is implemented, drm_gpuvm does not need to know >> the GPU page size. It only has to never do math on the BO offset to meet >> the requirements. >> >> I suspect that after this change there could be some cleanup possible in >> the xe driver (which right now passes flags around in various >> driver-specific ways from the map step through to drm_gpuva objects), >> but I'll leave that to the Xe folks. >> >> Signed-off-by: Asahi Lina <lina@xxxxxxxxxxxxx> >> --- >> Asahi Lina (4): >> drm/gpuvm: Add a flags argument to drm_gpuvm_sm_map[_*] >> drm/gpuvm: Plumb through flags into drm_gpuva_op_map >> drm/gpuvm: Add DRM_GPUVA_SINGLE_PAGE flag and logic >> drm/gpuvm: Plumb through flags into drm_gpuva_init > > Without looking into any details yet: > > This is a bit of tricky one, since we're not even close to having a user for > this new feature upstream yet, are we? I'd hope we're at least somewhere "this year" close to upstreaming drm/asahi! > > I wonder if we could do an exception by adding some KUnit tests (which > admittedly I never got to) validating things with and without this new feature. > > Speaking of tests, how did you validate this change to validate the behavior > without DRM_GPUVA_SINGLE_PAGE? Mostly just making sure our driver passes GL/Vulkan CTS including sparse after this change. I do want to put together some low-level tests in igt-gpu-tools, but I haven't gotten around to it yet... ~~ Lina
