Quoting Mika Kuoppala (2018-01-31 14:38:02) > > @@ -1988,6 +1988,11 @@ static int logical_ring_init(struct intel_engine_cs *engine) > > engine->execlists.elsp = > > engine->i915->regs + i915_mmio_reg_offset(RING_ELSP(engine)); > > > > + engine->execlists.preempt_complete_status = ~0u; > > This is to ensure that we catch a rogue status? Atleast we will > bug on of preempting with this status as the EXECLIST_ACTIVE_PREEMPT > will be false. 0 is the value for the i915->kernel_context. ~0u is currently invalid and that looks to be the case (that at least it will never match an active context) for the near future. > > + if (engine->i915->preempt_context) > > + engine->execlists.preempt_complete_status = > > + upper_32_bits(engine->i915->preempt_context->engine[engine->id].lrc_desc); > > > We have not upgraded the descriptor yet, so just use preempt_context->hw_id; The upper_32_bits should be set; and I think it is important that the usage is consistent. If we change the upper_32_bits later, we should try to remember to update the preempt_complete_status. I.e. I feel it is vital to remember that the complete_status is the upper_32_bits(lrc_desc) that just happens to be hw_id due to our limited definition today. > I would also like duplicate, from i915_gem_context.c, the assertion that > hw_id needs to be <= INT_MAX but didn't find a good spot. For what purpose? When defining lrc_desc checking that hw_id fits within the field width would be sensible. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
