Quoting Tvrtko Ursulin (2019-02-11 18:51:09)
>
> On 06/02/2019 13:03, Chris Wilson wrote:
> > In the next patch, we are introducing a broad virtual engine to encompass
> > multiple physical engines, losing the 1:1 nature of BIT(engine->id). To
> > reflect the broader set of engines implied by the virtual instance, lets
> > store the full bitmask.
> >
> > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/i915_reset.c | 4 ++--
> > drivers/gpu/drm/i915/intel_engine_cs.c | 3 +++
> > drivers/gpu/drm/i915/intel_hangcheck.c | 8 ++++----
> > drivers/gpu/drm/i915/intel_ringbuffer.c | 4 ++--
> > drivers/gpu/drm/i915/intel_ringbuffer.h | 7 +------
> > drivers/gpu/drm/i915/selftests/intel_hangcheck.c | 2 +-
> > drivers/gpu/drm/i915/selftests/mock_engine.c | 1 +
> > 7 files changed, 14 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reset.c b/drivers/gpu/drm/i915/i915_reset.c
> > index 7051c0a43941..78c9689629a0 100644
> > --- a/drivers/gpu/drm/i915/i915_reset.c
> > +++ b/drivers/gpu/drm/i915/i915_reset.c
> > @@ -1053,7 +1053,7 @@ void i915_reset(struct drm_i915_private *i915,
> > static inline int intel_gt_reset_engine(struct drm_i915_private *i915,
> > struct intel_engine_cs *engine)
> > {
> > - return intel_gpu_reset(i915, intel_engine_flag(engine));
> > + return intel_gpu_reset(i915, engine->mask);
> > }
> >
> > /**
> > @@ -1253,7 +1253,7 @@ void i915_handle_error(struct drm_i915_private *i915,
> > continue;
> >
> > if (i915_reset_engine(engine, msg) == 0)
> > - engine_mask &= ~intel_engine_flag(engine);
> > + engine_mask &= ~engine->mask;
> >
> > clear_bit(I915_RESET_ENGINE + engine->id,
> > &error->flags);
> > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> > index ce7c19f2ae49..45e38877ab17 100644
> > --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> > @@ -313,7 +313,10 @@ intel_engine_setup(struct drm_i915_private *dev_priv,
> > if (!engine)
> > return -ENOMEM;
> >
> > + BUILD_BUG_ON(BITS_PER_TYPE(engine->mask) < I915_NUM_ENGINES);
> > +
> > engine->id = id;
> > + engine->mask = BIT(id);
> > engine->i915 = dev_priv;
> > __sprint_engine_name(engine->name, info);
> > engine->hw_id = engine->guc_id = info->hw_id;
> > diff --git a/drivers/gpu/drm/i915/intel_hangcheck.c b/drivers/gpu/drm/i915/intel_hangcheck.c
> > index e04b2560369e..58b6ff8453dc 100644
> > --- a/drivers/gpu/drm/i915/intel_hangcheck.c
> > +++ b/drivers/gpu/drm/i915/intel_hangcheck.c
> > @@ -120,7 +120,7 @@ engine_stuck(struct intel_engine_cs *engine, u64 acthd)
> > */
> > tmp = I915_READ_CTL(engine);
> > if (tmp & RING_WAIT) {
> > - i915_handle_error(dev_priv, BIT(engine->id), 0,
> > + i915_handle_error(dev_priv, engine->mask, 0,
> > "stuck wait on %s", engine->name);
> > I915_WRITE_CTL(engine, tmp);
> > return ENGINE_WAIT_KICK;
> > @@ -282,13 +282,13 @@ static void i915_hangcheck_elapsed(struct work_struct *work)
> > hangcheck_store_sample(engine, &hc);
> >
> > if (hc.stalled) {
> > - hung |= intel_engine_flag(engine);
> > + hung |= engine->mask;
> > if (hc.action != ENGINE_DEAD)
> > - stuck |= intel_engine_flag(engine);
> > + stuck |= engine->mask;
> > }
> >
> > if (hc.wedged)
> > - wedged |= intel_engine_flag(engine);
> > + wedged |= engine->mask;
> > }
> >
> > if (GEM_SHOW_DEBUG() && (hung | stuck)) {
> > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > index 1b96b0960adc..91c49f644898 100644
> > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > @@ -1859,8 +1859,8 @@ static int switch_context(struct i915_request *rq)
> > goto err;
> > } while (--loops);
> >
> > - if (intel_engine_flag(engine) & ppgtt->pd_dirty_rings) {
> > - unwind_mm = intel_engine_flag(engine);
> > + if (ppgtt->pd_dirty_rings & engine->mask) {
> > + unwind_mm = engine->mask;
> > ppgtt->pd_dirty_rings &= ~unwind_mm;
> > hw_flags = MI_FORCE_RESTORE;
> > }
> > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> > index 39a9ee7b61e2..7777d46784f9 100644
> > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> > @@ -334,6 +334,7 @@ struct intel_engine_cs {
> > enum intel_engine_id id;
> > unsigned int hw_id;
> > unsigned int guc_id;
> > + unsigned long mask;
>
> Could use intel_ring_mask_t - if we renamed it to intel_engine_mask_t -
> which is already checked with a BUILD_BUG_ON.
Only downside, it was hidden away in intel_device_info.h
[snip]
> No other suggestions.
The name is open to suggestions, engine->physmask was a possibility we
discussed, but none of the suggestions stuck over a plain old boring
engine->mask.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
