>> +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 <[email protected]>
