On 9/22/23 13:58, Boris Brezillon wrote:
On Wed, 20 Sep 2023 16:42:39 +0200 Danilo Krummrich <dakr@xxxxxxxxxx> wrote:+/** + * enum drm_gpuvm_flags - flags for struct drm_gpuvm + */ +enum drm_gpuvm_flags { + /** + * @DRM_GPUVM_USERBITS: user defined bits + */ + DRM_GPUVM_USERBITS = (1 << 0),Nit: I tried declaring driver-specific flags, and I find this counter-intuitive. You basically end up with something like: enum my_gpuvm_flags { MY_FLAG_X = DRM_GPUVM_USERBITS, MY_FLAG_Y = DRM_GPUVM_USERBITS << 1, }; instead of the usual enum my_gpuvm_flags { MY_FLAG_X = BIT(0), MY_FLAG_Y = BIT(1), }; pattern.
Right, same as with dma_fence flags.
Another issue I see coming is if we end up adding more core flags and drivers start falling short of bits for their own flags. This makes me wonder if we shouldn't kill this notion of USER flags and let drivers store their flags in some dedicated field, given they're likely to derive drm_gpuvm and drm_gpuva with their own object anyway.
The only reason I have this in the code is that Xe asked for this with drm_gpuva_flags. Hence, for consistency reasons I added it for drm_gpuvm_flags too. Drivers can still have their own flag fields if needed, otherwise I guess it doesn't really hurt to keep DRM_GPUVM_USERBITS in case someone wants to use it.
+}; +
