On Wed, Apr 28, 2021 at 11:42:31AM +0100, Tvrtko Ursulin wrote: > > On 28/04/2021 11:16, Daniel Vetter wrote: > > On Fri, Apr 23, 2021 at 05:31:19PM -0500, Jason Ekstrand wrote: > > > There's no sense in allowing userspace to create more engines than it > > > can possibly access via execbuf. > > > > > > Signed-off-by: Jason Ekstrand <jason@xxxxxxxxxxxxxx> > > > --- > > > drivers/gpu/drm/i915/gem/i915_gem_context.c | 7 +++---- > > > 1 file changed, 3 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c > > > index 5f8d0faf783aa..ecb3bf5369857 100644 > > > --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c > > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c > > > @@ -1640,11 +1640,10 @@ set_engines(struct i915_gem_context *ctx, > > > return -EINVAL; > > > } > > > - /* > > > - * Note that I915_EXEC_RING_MASK limits execbuf to only using the > > > - * first 64 engines defined here. > > > - */ > > > num_engines = (args->size - sizeof(*user)) / sizeof(*user->engines); > > > > Maybe add a comment like /* RING_MASK has not shift, so can be used > > directly here */ since I had to check that :-) > > > > Same story about igt testcases needed, just to be sure. > > > > Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx> > > I am not sure about the churn vs benefit ratio here. There are also patches > which extend the engine selection field in execbuf2 over the unused > constants bits (with an explicit flag). So churn upstream and churn in > internal (if interesting) for not much benefit. This isn't churn. This is "lock done uapi properly". -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
