On 2/22/23 11:25, Christian König wrote:
Am 17.02.23 um 14:44 schrieb Danilo Krummrich:
<snip>
+/**
+ * DOC: Overview
+ *
+ * The DRM GPU VA Manager, represented by struct drm_gpuva_manager
keeps track
+ * of a GPU's virtual address (VA) space and manages the
corresponding virtual
+ * mappings represented by &drm_gpuva objects. It also keeps track of
the
+ * mapping's backing &drm_gem_object buffers.
+ *
+ * &drm_gem_object buffers maintain a list (and a corresponding list
lock) of
+ * &drm_gpuva objects representing all existent GPU VA mappings using
this
+ * &drm_gem_object as backing buffer.
+ *
+ * If the &DRM_GPUVA_MANAGER_REGIONS feature is enabled, a GPU VA
mapping can
+ * only be created within a previously allocated &drm_gpuva_region,
which
+ * represents a reserved portion of the GPU VA space. GPU VA mappings
are not
+ * allowed to span over a &drm_gpuva_region's boundary.
+ *
+ * GPU VA regions can also be flagged as sparse, which allows drivers
to create
+ * sparse mappings for a whole GPU VA region in order to support Vulkan
+ * 'Sparse Resources'.
Well since we have now found that there is absolutely no technical
reason for having those regions could we please drop them?
I disagree this was the outcome of our previous discussion.
In nouveau I still need them to track the separate sparse page tables
and, as you confirmed previously, Nvidia cards are not the only cards
supporting this feature.
The second reason is that with regions we can avoid merging between
buffers, which saves some effort. However, I agree that this argument by
itself probably doesn't hold too much, since you've pointed out in a
previous mail that:
<cite>
1) If we merge and decide to only do that inside certain boundaries then
those boundaries needs to be provided and checked against. This burns
quite some CPU cycles
2) If we just merge what we can we might have extra page table updates
which cost time and could result in undesired side effects.
3) If we don't merge at all we have additional housekeeping for the
mappings and maybe hw restrictions.
</cite>
However, if a driver uses regions to track its separate sparse page
tables anyway it gets 1) for free, which is a nice synergy.
I totally agree that regions aren't for everyone though. Hence, I made
them an optional feature and by default regions are disabled. In order
to use them drm_gpuva_manager_init() must be called with the
DRM_GPUVA_MANAGER_REGIONS feature flag.
I really would not want to open code regions or have two GPUVA manager
instances in nouveau to track sparse page tables. That would be really
messy, hence I hope we can agree on this to be an optional feature.
I don't really see a need for them any more.
Regards,
Christian.
