On 28/04/2021 15:02, Daniel Vetter wrote:
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".
IMO it is a "meh" patch. Doesn't fix any problems and will create work
for other people and man hours spent which no one will ever properly
account against.
Number of contexts in the engine map should not really be tied to
execbuf2. As is demonstrated by the incoming work to address more than
63 engines, either as an extension to execbuf2 or future execbuf3.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
