On Mon, 07 Jul 2025 21:06:50 +0200
"Danilo Krummrich" <d...@kernel.org> wrote:
> On Mon Jul 7, 2025 at 9:00 PM CEST, Danilo Krummrich wrote:
> > On Mon Jul 7, 2025 at 7:04 PM CEST, Caterina Shablia wrote:
> >> diff --git a/drivers/gpu/drm/drm_gpuvm.c b/drivers/gpu/drm/drm_gpuvm.c
> >> index 05978c5c38b1..dc3c2f906400 100644
> >> --- a/drivers/gpu/drm/drm_gpuvm.c
> >> +++ b/drivers/gpu/drm/drm_gpuvm.c
> >> @@ -2098,12 +2098,48 @@ op_unmap_cb(const struct drm_gpuvm_ops *fn, void
> >> *priv,
> >> return fn->sm_step_unmap(&op, priv);
> >> }
> >>
> >> +static bool can_merge(struct drm_gpuvm *gpuvm, const struct drm_gpuva *a,
> >> + const struct drm_gpuva *b)
> >> +{
> >> + /* Only GEM-based mappings can be merged, and they must point to
> >> + * the same GEM object.
> >> + */
> >> + if (a->gem.obj != b->gem.obj || !a->gem.obj)
> >> + return false;
> >> +
> >> + /* Let's keep things simple for now and force all flags to match. */
> >> + if (a->flags != b->flags)
> >> + return false;
>
> Forgot to mention, this can include driver specific flags. How do we know from
> the generic code whether this condition makes sense? *At least* it would need
> to
> be documented.
You're right, it should have been:
if ((a->flags & DRM_GPUVA_MERGEABLE_FLAGS_MASK) !=
(b->flags & DRM_GPUVA_MERGEABLE_FLAGS_MASK))
return false;
with DRM_GPUVA_COMMON_FLAGS_MASK set to the set of flags that matter
when merging.
>
> However, I think it would be better to provide an optional callback for
> drivers
> to check whether merge makes sense or not. This doesn't mean we need drivers
> to
> do those common checks, this can remain here in the common code.
Seems a bit premature to me. Again, if there's a need for drivers to add
extra checks we can always add a callback at this point, but until
this is the case, I'd rather stick to these common checks.
