| Message ID | 20190726131458.8310-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] drm/i915/perf: Initialise err to 0 before looping over ce->engines | expand |
On 26/07/2019 16:14, Chris Wilson wrote: > Smatch warning that the loop may be empty causing us to check err before > it had been set. Ensure that it is initialised to 0, just in case. > > v2: Refactor the inner loop for better scooping and clarity > > Fixes: a9877da2d629 ("drm/i915/oa: Reconfigure contexts on the fly") > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > --- > drivers/gpu/drm/i915/i915_perf.c | 61 ++++++++++++++++++-------------- > 1 file changed, 35 insertions(+), 26 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c > index 79be9d8ea628..988a4092164e 100644 > --- a/drivers/gpu/drm/i915/i915_perf.c > +++ b/drivers/gpu/drm/i915/i915_perf.c > @@ -1790,6 +1790,38 @@ static int gen8_modify_self(struct intel_context *ce, > return err; > } > > +static int gen8_configure_context(struct i915_gem_context *ctx, > + struct flex *flex, unsigned int count) > +{ > + struct i915_gem_engines_iter it; > + struct intel_context *ce; > + int err = 0; > + > + for_each_gem_engine(ce, i915_gem_context_lock_engines(ctx), it) { > + GEM_BUG_ON(ce == ce->engine->kernel_context); > + > + if (ce->engine->class != RENDER_CLASS) > + continue; > + > + err = intel_context_lock_pinned(ce); > + if (err) > + break; > + > + flex->value = intel_sseu_make_rpcs(ctx->i915, &ce->sseu); > + > + /* Otherwise OA settings will be set upon first use */ > + if (intel_context_is_pinned(ce)) > + err = gen8_modify_context(ce, flex, count); > + > + intel_context_unlock_pinned(ce); > + if (err) > + break; > + } > + i915_gem_context_unlock_engines(ctx); > + > + return err; > +} > + > /* > * Manages updating the per-context aspects of the OA stream > * configuration across all contexts. > @@ -1844,7 +1876,6 @@ static int gen8_configure_all_contexts(struct drm_i915_private *i915, > struct intel_engine_cs *engine; > struct i915_gem_context *ctx; > enum intel_engine_id id; > - int err; > int i; > > for (i = 2; i < ARRAY_SIZE(regs); i++) > @@ -1869,35 +1900,12 @@ static int gen8_configure_all_contexts(struct drm_i915_private *i915, > * trapped behind the barrier. > */ > list_for_each_entry(ctx, &i915->contexts.list, link) { > - struct i915_gem_engines_iter it; > - struct intel_context *ce; > + int err; > > if (ctx == i915->kernel_context) > continue; > > - for_each_gem_engine(ce, > - i915_gem_context_lock_engines(ctx), > - it) { > - GEM_BUG_ON(ce == ce->engine->kernel_context); > - > - if (ce->engine->class != RENDER_CLASS) > - continue; > - > - err = intel_context_lock_pinned(ce); > - if (err) > - break; > - > - regs[0].value = intel_sseu_make_rpcs(i915, &ce->sseu); > - > - /* Otherwise OA settings will be set upon first use */ > - if (intel_context_is_pinned(ce)) > - err = gen8_modify_context(ce, regs, ARRAY_SIZE(regs)); > - > - intel_context_unlock_pinned(ce); > - if (err) > - break; > - } > - i915_gem_context_unlock_engines(ctx); > + err = gen8_configure_context(ctx, regs, ARRAY_SIZE(regs)); > if (err) > return err; > } > @@ -1909,6 +1917,7 @@ static int gen8_configure_all_contexts(struct drm_i915_private *i915, > */ > for_each_engine(engine, i915, id) { > struct intel_context *ce = engine->kernel_context; > + int err; > > if (engine->class != RENDER_CLASS) > continue;
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index 79be9d8ea628..988a4092164e 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -1790,6 +1790,38 @@ static int gen8_modify_self(struct intel_context *ce, return err; } +static int gen8_configure_context(struct i915_gem_context *ctx, + struct flex *flex, unsigned int count) +{ + struct i915_gem_engines_iter it; + struct intel_context *ce; + int err = 0; + + for_each_gem_engine(ce, i915_gem_context_lock_engines(ctx), it) { + GEM_BUG_ON(ce == ce->engine->kernel_context); + + if (ce->engine->class != RENDER_CLASS) + continue; + + err = intel_context_lock_pinned(ce); + if (err) + break; + + flex->value = intel_sseu_make_rpcs(ctx->i915, &ce->sseu); + + /* Otherwise OA settings will be set upon first use */ + if (intel_context_is_pinned(ce)) + err = gen8_modify_context(ce, flex, count); + + intel_context_unlock_pinned(ce); + if (err) + break; + } + i915_gem_context_unlock_engines(ctx); + + return err; +} + /* * Manages updating the per-context aspects of the OA stream * configuration across all contexts. @@ -1844,7 +1876,6 @@ static int gen8_configure_all_contexts(struct drm_i915_private *i915, struct intel_engine_cs *engine; struct i915_gem_context *ctx; enum intel_engine_id id; - int err; int i; for (i = 2; i < ARRAY_SIZE(regs); i++) @@ -1869,35 +1900,12 @@ static int gen8_configure_all_contexts(struct drm_i915_private *i915, * trapped behind the barrier. */ list_for_each_entry(ctx, &i915->contexts.list, link) { - struct i915_gem_engines_iter it; - struct intel_context *ce; + int err; if (ctx == i915->kernel_context) continue; - for_each_gem_engine(ce, - i915_gem_context_lock_engines(ctx), - it) { - GEM_BUG_ON(ce == ce->engine->kernel_context); - - if (ce->engine->class != RENDER_CLASS) - continue; - - err = intel_context_lock_pinned(ce); - if (err) - break; - - regs[0].value = intel_sseu_make_rpcs(i915, &ce->sseu); - - /* Otherwise OA settings will be set upon first use */ - if (intel_context_is_pinned(ce)) - err = gen8_modify_context(ce, regs, ARRAY_SIZE(regs)); - - intel_context_unlock_pinned(ce); - if (err) - break; - } - i915_gem_context_unlock_engines(ctx); + err = gen8_configure_context(ctx, regs, ARRAY_SIZE(regs)); if (err) return err; } @@ -1909,6 +1917,7 @@ static int gen8_configure_all_contexts(struct drm_i915_private *i915, */ for_each_engine(engine, i915, id) { struct intel_context *ce = engine->kernel_context; + int err; if (engine->class != RENDER_CLASS) continue;
Smatch warning that the loop may be empty causing us to check err before it had been set. Ensure that it is initialised to 0, just in case. v2: Refactor the inner loop for better scooping and clarity Fixes: a9877da2d629 ("drm/i915/oa: Reconfigure contexts on the fly") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> --- drivers/gpu/drm/i915/i915_perf.c | 61 ++++++++++++++++++-------------- 1 file changed, 35 insertions(+), 26 deletions(-)