Danilo Krummrich
2023-Sep-27 16:52 UTC
[Nouveau] [PATCH drm-misc-next v4 6/8] drm/gpuvm: add drm_gpuvm_flags to drm_gpuvm
On 9/22/23 13:58, Boris Brezillon wrote:> On Wed, 20 Sep 2023 16:42:39 +0200 > Danilo Krummrich <dakr at redhat.com> 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.> >> +}; >> + >