| Message ID | 1456224746-5956-1-git-send-email-gabriel.feceoru@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On 23/02/16 10:52, Gabriel Feceoru wrote: > Return error when I915_EXEC_BSD_RING2 flag is set but BSD2 ring > is not available in the HW. What is the reasoning behind this? So far kernel was allowing userspace to select these bits and execute on the first engine. With this patch it would start failing potentially breaking userspace. Is it not too late to make such change? Regards, Tvrtko > v2: Reworked > Signed-off-by: Gabriel Feceoru <gabriel.feceoru@intel.com> > --- > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > index 8fd00d2..9fbd023 100644 > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > @@ -1394,6 +1394,13 @@ eb_select_ring(struct drm_i915_private *dev_priv, > return -EINVAL; > } > > + if ((user_ring_id == I915_EXEC_BSD) && !HAS_BSD2(dev_priv) && > + ((args->flags & I915_EXEC_BSD_MASK) != 0)) { > + DRM_DEBUG("execbuf with bsd ring but with invalid " > + "bsd dispatch flags: %d\n", (int)(args->flags)); > + return -EINVAL; > + } > + > if (user_ring_id == I915_EXEC_BSD && HAS_BSD2(dev_priv)) { > unsigned int bsd_idx = args->flags & I915_EXEC_BSD_MASK; > >
On 23.02.2016 13:05, Tvrtko Ursulin wrote: > > Hi, > > On 23/02/16 10:52, Gabriel Feceoru wrote: >> Return error when I915_EXEC_BSD_RING2 flag is set but BSD2 ring >> is not available in the HW. > > What is the reasoning behind this? So far kernel was allowing userspace > to select these bits and execute on the first engine. With this patch it > would start failing potentially breaking userspace. Is it not too late > to make such change? I noticed some inconsistencies in igt with regards to bsd and bsd1. For instance, if bsd2 is not available, gem_sync@basic-bsd1 is skipped, but it's skipped because of the 2nd check gem_has_bsd2 (see gem_require_ring). Surprisingly gem_has_ring() didn't complain about bsd1. This fix will make gem_has_ring() return false. I'm not aware about legacy/compatibility issue - if that's the case, please disregard this. Regards, Gabriel > > Regards, > > Tvrtko > >> v2: Reworked >> Signed-off-by: Gabriel Feceoru <gabriel.feceoru@intel.com> >> --- >> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c >> b/drivers/gpu/drm/i915/i915_gem_execbuffer.c >> index 8fd00d2..9fbd023 100644 >> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c >> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c >> @@ -1394,6 +1394,13 @@ eb_select_ring(struct drm_i915_private *dev_priv, >> return -EINVAL; >> } >> >> + if ((user_ring_id == I915_EXEC_BSD) && !HAS_BSD2(dev_priv) && >> + ((args->flags & I915_EXEC_BSD_MASK) != 0)) { >> + DRM_DEBUG("execbuf with bsd ring but with invalid " >> + "bsd dispatch flags: %d\n", (int)(args->flags)); >> + return -EINVAL; >> + } >> + >> if (user_ring_id == I915_EXEC_BSD && HAS_BSD2(dev_priv)) { >> unsigned int bsd_idx = args->flags & I915_EXEC_BSD_MASK; >> >>
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 8fd00d2..9fbd023 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1394,6 +1394,13 @@ eb_select_ring(struct drm_i915_private *dev_priv, return -EINVAL; } + if ((user_ring_id == I915_EXEC_BSD) && !HAS_BSD2(dev_priv) && + ((args->flags & I915_EXEC_BSD_MASK) != 0)) { + DRM_DEBUG("execbuf with bsd ring but with invalid " + "bsd dispatch flags: %d\n", (int)(args->flags)); + return -EINVAL; + } + if (user_ring_id == I915_EXEC_BSD && HAS_BSD2(dev_priv)) { unsigned int bsd_idx = args->flags & I915_EXEC_BSD_MASK;
Return error when I915_EXEC_BSD_RING2 flag is set but BSD2 ring is not available in the HW. v2: Reworked Signed-off-by: Gabriel Feceoru <gabriel.feceoru@intel.com> --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 7 +++++++ 1 file changed, 7 insertions(+)