Quoting Tvrtko Ursulin (2019-03-05 14:44:04)
>
> On 02/03/2019 10:06, 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.
> >
> > v2: Use intel_engine_mask_t (s/ring_mask/engine_mask/)
> > v3: Tvrtko voted for moah churn so teach everyone to not mention ring
> > and use $class$instance throughout.
> >
> > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> > ---
> > -static void gen6_record_semaphore_state(struct intel_engine_cs *engine,
> > - struct drm_i915_error_engine *ee)
> > -{
> > - struct drm_i915_private *dev_priv = engine->i915;
> > -
> > - ee->semaphore_mboxes[0] = I915_READ(RING_SYNC_0(engine->mmio_base));
> > - ee->semaphore_mboxes[1] = I915_READ(RING_SYNC_1(engine->mmio_base));
> > - if (HAS_VEBOX(dev_priv))
> > - ee->semaphore_mboxes[2] =
> > - I915_READ(RING_SYNC_2(engine->mmio_base));
> > -}
> > -
> > static void error_record_engine_registers(struct i915_gpu_state *error,
> > struct intel_engine_cs *engine,
> > struct drm_i915_error_engine *ee)
> > @@ -1161,7 +1142,6 @@ static void error_record_engine_registers(struct i915_gpu_state *error,
> > if (INTEL_GEN(dev_priv) >= 8) {
> > ee->fault_reg = I915_READ(GEN8_RING_FAULT_REG);
> > } else {
> > - gen6_record_semaphore_state(engine, ee);
>
> Wrong patch?
There wasn't much point updating these as they are defunct, so I removed
them instead.
> > /* Engine class */
> >
> > @@ -7250,8 +7250,8 @@ enum {
> > #define GEN8_GT_VECS_IRQ (1 << 6)
> > #define GEN8_GT_GUC_IRQ (1 << 5)
> > #define GEN8_GT_PM_IRQ (1 << 4)
> > -#define GEN8_GT_VCS2_IRQ (1 << 3)
> > -#define GEN8_GT_VCS1_IRQ (1 << 2)
> > +#define GEN8_GT_VCS1_IRQ (1 << 3)
> > +#define GEN8_GT_VCS0_IRQ (1 << 2)
>
> BSpec uses index 1. :( So I think best to leave these alone.
That does get confusing. So we either have confusing code
with VCS1 -> VCS0 and VCS2 -> VCS1, or just live with the difference
with bspec. Code wins (because I read that more often), but we can
comment that "old" bspec refers to these as 1-indexed?
[snip]
> I wasn't suggesting renaming the engine id enums, but on the other hand
> it is nicer to have VCS0, VCS1,.. instead of VCS, VCS2, VCS3,...
>
> So ack from me. And since I read it all almost r-b if not the unrelated
> error state hunk and more importantly reservations about bit definition
> names from i915_reg.h.
I'll go drop another couple of patches for the grumbles.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
