>> +u32 *gen12_emit_aux_table_inv(const i915_reg_t inv_reg, u32 *cs)
>> -static u32 *gen12_emit_aux_table_inv(const i915_reg_t inv_reg, u32 *cs)
>
> I think all helpers which emit to ring take cs as the first argument so it would be good to make this consistent.
Updated the patch, please review rev10.
This helper function has been there for a long while, I was hesitant to change. But I agree cs should be the first argument. Since I removed the "static" anyway, so might just change the order all together.
>> @@ -329,15 +306,10 @@ int gen12_emit_flush_xcs(struct i915_request *rq, u32 mode)
>> *cs++ = 0; /* value */
>>
>> if (aux_inv) { /* hsdes: 1809175790 */
>> - struct intel_engine_cs *engine;
>> - unsigned int tmp;
>> -
>> - *cs++ = MI_LOAD_REGISTER_IMM(hweight32(aux_inv));
>> - for_each_engine_masked(engine, rq->engine->gt, aux_inv, tmp) {
>> - *cs++ = i915_mmio_reg_offset(aux_inv_reg(engine));
>> - *cs++ = AUX_INV;
>> - }
>> - *cs++ = MI_NOOP;
>> + if (rq->engine->class == VIDEO_DECODE_CLASS)
>> + cs = gen12_emit_aux_table_inv(GEN12_VD0_AUX_NV, cs);
>> + else
>> + cs = gen12_emit_aux_table_inv(GEN12_VE0_AUX_NV, cs);
>
> Not sure if, here and below, it would be worth to put register in a local and then have a single function call - up to you.
I feel it's easier to check the code correctness in the *_rcs/*_xcs functions and leave the helper function as simple as possible.
>
> Apart from the cs re-order looks good to me.
If no other problems with rev10, would you please help push it upstream? I don't have the commit right, will need to find someone to help take it further.
Thanks,
-Fei
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
