[PATCH] drm/i915/guc: Don't send policy update for child contexts.

[John Harrison]

On 7/27/2022 18:50, Ceraolo Spurio, Daniele wrote:
> On 7/27/2022 6:44 PM, John Harrison wrote:
>> On 7/27/2022 17:33, Daniele Ceraolo Spurio wrote:
>>> The GuC FW applies the parent context policy to all the children,
>>> so individual updates to the children are not supported and we
>>> should not send them.
>>>
>>> Note that sending the message did not have any functional consequences,
>>> because the GuC just drops it and logs an error; since we were trying
>>> to set the child policy to match the parent anyway the message being
>>> dropped was not a problem.
>>>
>>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
>>> Cc: John Harrison <john.c.harrison at intel.com>
>> Needs a Fixes tag for the original v70 update patch?
>
> I don't think so. I added the explanation about it not being a 
> functional issue to make it clear that everything still works as 
> expected without this patch, just with a bit of extra noise in the GuC 
> logs. If you think it is still worth applying to older kernels I'll 
> add the tag in.
>
> Daniele
Hmm. It is strictly speaking a bug fix. But yes, the only impact is 
extra H2G traffic on context registration and in the GuC log. So maybe 
not worth worrying about.

Reviewed-by: John Harrison <John.C.Harrison at Intel.com>

>
>>
>> John.
>>
>>> ---
>>>   .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 26 
>>> +------------------
>>>   1 file changed, 1 insertion(+), 25 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c 
>>> b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
>>> index 76916aed897a..5e31e2540297 100644
>>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
>>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
>>> @@ -2420,7 +2420,6 @@ static int guc_context_policy_init_v70(struct 
>>> intel_context *ce, bool loop)
>>>       struct context_policy policy;
>>>       u32 execution_quantum;
>>>       u32 preemption_timeout;
>>> -    bool missing = false;
>>>       unsigned long flags;
>>>       int ret;
>>>   @@ -2438,32 +2437,9 @@ static int 
>>> guc_context_policy_init_v70(struct intel_context *ce, bool loop)
>>> __guc_context_policy_add_preempt_to_idle(&policy, 1);
>>>         ret = __guc_context_set_context_policies(guc, &policy, loop);
>>> -    missing = ret != 0;
>>> -
>>> -    if (!missing && intel_context_is_parent(ce)) {
>>> -        struct intel_context *child;
>>> -
>>> -        for_each_child(ce, child) {
>>> -            __guc_context_policy_start_klv(&policy, child->guc_id.id);
>>> -
>>> -            if (engine->flags & I915_ENGINE_WANT_FORCED_PREEMPTION)
>>> - __guc_context_policy_add_preempt_to_idle(&policy, 1);
>>> -
>>> -            child->guc_state.prio = ce->guc_state.prio;
>>> -            __guc_context_policy_add_priority(&policy, 
>>> ce->guc_state.prio);
>>> - __guc_context_policy_add_execution_quantum(&policy, 
>>> execution_quantum);
>>> - __guc_context_policy_add_preemption_timeout(&policy, 
>>> preemption_timeout);
>>> -
>>> -            ret = __guc_context_set_context_policies(guc, &policy, 
>>> loop);
>>> -            if (ret) {
>>> -                missing = true;
>>> -                break;
>>> -            }
>>> -        }
>>> -    }
>>>         spin_lock_irqsave(&ce->guc_state.lock, flags);
>>> -    if (missing)
>>> +    if (ret != 0)
>>>           set_context_policy_required(ce);
>>>       else
>>>           clr_context_policy_required(ce);
>>
>