| Message ID | 20190206130356.18771-26-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/46] drm/i915: Hack and slash, throttle execbuffer hogs | expand |
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@chris-wilson.co.uk> > --- > 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. > > u8 uabi_id; > u8 uabi_class; > @@ -668,12 +669,6 @@ execlists_port_complete(struct intel_engine_execlists * const execlists, > return port; > } > > -static inline unsigned int > -intel_engine_flag(const struct intel_engine_cs *engine) > -{ > - return BIT(engine->id); > -} > - > static inline u32 > intel_read_status_page(const struct intel_engine_cs *engine, int reg) > { > diff --git a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c > index 4aa57d0d1b92..50a7f57a00a4 100644 > --- a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c > +++ b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c > @@ -1142,7 +1142,7 @@ static int __igt_reset_evict_vma(struct drm_i915_private *i915, > > out_reset: > igt_global_reset_lock(i915); > - fake_hangcheck(rq->i915, intel_engine_flag(rq->engine)); > + fake_hangcheck(rq->i915, rq->engine->mask); > igt_global_reset_unlock(i915); > > if (tsk) { > diff --git a/drivers/gpu/drm/i915/selftests/mock_engine.c b/drivers/gpu/drm/i915/selftests/mock_engine.c > index ec1ae948954c..c2c954f64226 100644 > --- a/drivers/gpu/drm/i915/selftests/mock_engine.c > +++ b/drivers/gpu/drm/i915/selftests/mock_engine.c > @@ -223,6 +223,7 @@ struct intel_engine_cs *mock_engine(struct drm_i915_private *i915, > engine->base.i915 = i915; > snprintf(engine->base.name, sizeof(engine->base.name), "%s", name); > engine->base.id = id; > + engine->base.mask = BIT(id); > engine->base.status_page.addr = (void *)(engine + 1); > > engine->base.context_pin = mock_context_pin; > No other suggestions. Regards, Tvrtko
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@chris-wilson.co.uk> > > --- > > 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
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; u8 uabi_id; u8 uabi_class; @@ -668,12 +669,6 @@ execlists_port_complete(struct intel_engine_execlists * const execlists, return port; } -static inline unsigned int -intel_engine_flag(const struct intel_engine_cs *engine) -{ - return BIT(engine->id); -} - static inline u32 intel_read_status_page(const struct intel_engine_cs *engine, int reg) { diff --git a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c index 4aa57d0d1b92..50a7f57a00a4 100644 --- a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c +++ b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c @@ -1142,7 +1142,7 @@ static int __igt_reset_evict_vma(struct drm_i915_private *i915, out_reset: igt_global_reset_lock(i915); - fake_hangcheck(rq->i915, intel_engine_flag(rq->engine)); + fake_hangcheck(rq->i915, rq->engine->mask); igt_global_reset_unlock(i915); if (tsk) { diff --git a/drivers/gpu/drm/i915/selftests/mock_engine.c b/drivers/gpu/drm/i915/selftests/mock_engine.c index ec1ae948954c..c2c954f64226 100644 --- a/drivers/gpu/drm/i915/selftests/mock_engine.c +++ b/drivers/gpu/drm/i915/selftests/mock_engine.c @@ -223,6 +223,7 @@ struct intel_engine_cs *mock_engine(struct drm_i915_private *i915, engine->base.i915 = i915; snprintf(engine->base.name, sizeof(engine->base.name), "%s", name); engine->base.id = id; + engine->base.mask = BIT(id); engine->base.status_page.addr = (void *)(engine + 1); engine->base.context_pin = mock_context_pin;
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@chris-wilson.co.uk> --- 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(-)