* [PATCH] drm/i915/guc: Don't send policy update for child contexts.
@ 2022-07-28 0:33 Daniele Ceraolo Spurio
2022-07-28 1:44 ` John Harrison
0 siblings, 1 reply; 4+ messages in thread
From: Daniele Ceraolo Spurio @ 2022-07-28 0:33 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniele Ceraolo Spurio, John Harrison, dri-devel
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@intel.com>
Cc: John Harrison <john.c.harrison@intel.com>
---
.../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);
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915/guc: Don't send policy update for child contexts.
2022-07-28 0:33 [PATCH] drm/i915/guc: Don't send policy update for child contexts Daniele Ceraolo Spurio
@ 2022-07-28 1:44 ` John Harrison
2022-07-28 1:50 ` Ceraolo Spurio, Daniele
0 siblings, 1 reply; 4+ messages in thread
From: John Harrison @ 2022-07-28 1:44 UTC (permalink / raw)
To: Daniele Ceraolo Spurio, intel-gfx; +Cc: dri-devel
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@intel.com>
> Cc: John Harrison <john.c.harrison@intel.com>
Needs a Fixes tag for the original v70 update patch?
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);
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915/guc: Don't send policy update for child contexts.
2022-07-28 1:44 ` John Harrison
@ 2022-07-28 1:50 ` Ceraolo Spurio, Daniele
2022-07-28 1:54 ` John Harrison
0 siblings, 1 reply; 4+ messages in thread
From: Ceraolo Spurio, Daniele @ 2022-07-28 1:50 UTC (permalink / raw)
To: John Harrison, intel-gfx; +Cc: dri-devel
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@intel.com>
>> Cc: John Harrison <john.c.harrison@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
>
> 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);
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915/guc: Don't send policy update for child contexts.
2022-07-28 1:50 ` Ceraolo Spurio, Daniele
@ 2022-07-28 1:54 ` John Harrison
0 siblings, 0 replies; 4+ messages in thread
From: John Harrison @ 2022-07-28 1:54 UTC (permalink / raw)
To: Ceraolo Spurio, Daniele, intel-gfx; +Cc: dri-devel
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@intel.com>
>>> Cc: John Harrison <john.c.harrison@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@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);
>>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-07-28 1:54 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-28 0:33 [PATCH] drm/i915/guc: Don't send policy update for child contexts Daniele Ceraolo Spurio
2022-07-28 1:44 ` John Harrison
2022-07-28 1:50 ` Ceraolo Spurio, Daniele
2022-07-28 1:54 ` John Harrison
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).