On Thu, Mar 02, 2017 at 07:33:26AM +0000, Tvrtko Ursulin wrote: > > On 01/03/2017 20:27, Michal Wajdeczko wrote: > > Engine related definitions are located in different files > > and it is easy to break their cross dependency. > > > > v2: compare against array size > > v3: don't use BUILD_BUG (Tvrtko) > > > > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx> > > Cc: Jani Nikula <jani.nikula@xxxxxxxxx> > > Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx> > > Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > > Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> > > --- > > drivers/gpu/drm/i915/intel_engine_cs.c | 7 ++++++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c > > index a238304..8675164 100644 > > --- a/drivers/gpu/drm/i915/intel_engine_cs.c > > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c > > @@ -89,7 +89,11 @@ intel_engine_setup(struct drm_i915_private *dev_priv, > > const struct engine_info *info = &intel_engines[id]; > > Just need to move this assignment after the assert. I was thinking about it, but decided to leave as is, as this is just pointer assignment and there is no read/write from this pointer. But I'll move it, to make this code even more robust ;) > > > struct intel_engine_cs *engine; > > > > - GEM_BUG_ON(dev_priv->engine[id]); > > + if (GEM_WARN_ON((unsigned)id >= ARRAY_SIZE(intel_engines))) > > + return -EINVAL; > > + if (GEM_WARN_ON((unsigned)id >= ARRAY_SIZE(dev_priv->engine))) > > + return -EINVAL; > > + > > engine = kzalloc(sizeof(*engine), GFP_KERNEL); > > if (!engine) > > return -ENOMEM; > > @@ -105,6 +109,7 @@ intel_engine_setup(struct drm_i915_private *dev_priv, > > /* Nothing to do here, execute in order of dependencies */ > > engine->schedule = NULL; > > > > + GEM_BUG_ON(dev_priv->engine[id]); > > It doesn't really matter but I think moving this one was not needed. Moved here just to correlate this assert with the actual critical assignment from the line below. I can revert it if you prefer old way. > > > dev_priv->engine[id] = engine; > > return 0; > > } > > > _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
