Re: [Intel-gfx] [PATCH 23/27] drm/i915/guc: Move GuC priority fields in context under guc_active
On 8/25/2021 8:23 PM, Matthew Brost wrote:
Move GuC management fields in context under guc_active struct as this is
where the lock that protects theses fields lives. Also only set guc_prio
field once during context init.
v2:
(Daniele)
- set CONTEXT_SET_INIT
Signed-off-by: Matthew Brost
Reviewed-by: Daniele Ceraolo Spurio
Daniele
---
drivers/gpu/drm/i915/gt/intel_context_types.h | 12 ++--
.../gpu/drm/i915/gt/uc/intel_guc_submission.c | 69 +++
drivers/gpu/drm/i915/i915_trace.h | 2 +-
3 files changed, 46 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h
b/drivers/gpu/drm/i915/gt/intel_context_types.h
index 3a5d98e908f4..b56960a781da 100644
--- a/drivers/gpu/drm/i915/gt/intel_context_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
@@ -112,6 +112,7 @@ struct intel_context {
#define CONTEXT_FORCE_SINGLE_SUBMISSION 7
#define CONTEXT_NOPREEMPT 8
#define CONTEXT_LRCA_DIRTY9
+#define CONTEXT_GUC_INIT 10
struct {
u64 timeout_us;
@@ -178,6 +179,11 @@ struct intel_context {
spinlock_t lock;
/** requests: active requests on this context */
struct list_head requests;
+ /*
+* GuC priority management
+*/
+ u8 prio;
+ u32 prio_count[GUC_CLIENT_PRIORITY_NUM];
} guc_active;
/* GuC LRC descriptor ID */
@@ -191,12 +197,6 @@ struct intel_context {
*/
struct list_head guc_id_link;
- /*
-* GuC priority management
-*/
- u8 guc_prio;
- u32 guc_prio_count[GUC_CLIENT_PRIORITY_NUM];
-
#ifdef CONFIG_DRM_I915_SELFTEST
/**
* @drop_schedule_enable: Force drop of schedule enable G2H for selftest
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 14a512533c39..bc68c0122be4 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -1367,8 +1367,6 @@ static void guc_context_policy_init(struct
intel_engine_cs *engine,
desc->preemption_timeout = engine->props.preempt_timeout_ms * 1000;
}
-static inline u8 map_i915_prio_to_guc_prio(int prio);
-
static int guc_lrc_desc_pin(struct intel_context *ce, bool loop)
{
struct intel_engine_cs *engine = ce->engine;
@@ -1376,8 +1374,6 @@ static int guc_lrc_desc_pin(struct intel_context *ce,
bool loop)
struct intel_guc *guc = >gt->uc.guc;
u32 desc_idx = ce->guc_id;
struct guc_lrc_desc *desc;
- const struct i915_gem_context *ctx;
- int prio = I915_CONTEXT_DEFAULT_PRIORITY;
bool context_registered;
intel_wakeref_t wakeref;
int ret = 0;
@@ -1394,12 +1390,6 @@ static int guc_lrc_desc_pin(struct intel_context *ce,
bool loop)
context_registered = lrc_desc_registered(guc, desc_idx);
- rcu_read_lock();
- ctx = rcu_dereference(ce->gem_context);
- if (ctx)
- prio = ctx->sched.priority;
- rcu_read_unlock();
-
reset_lrc_desc(guc, desc_idx);
set_lrc_desc_registered(guc, desc_idx, ce);
@@ -1408,8 +1398,7 @@ static int guc_lrc_desc_pin(struct intel_context *ce, bool loop)
desc->engine_submit_mask = adjust_engine_mask(engine->class,
engine->mask);
desc->hw_context_desc = ce->lrc.lrca;
- ce->guc_prio = map_i915_prio_to_guc_prio(prio);
- desc->priority = ce->guc_prio;
+ desc->priority = ce->guc_active.prio;
desc->context_flags = CONTEXT_REGISTRATION_FLAG_KMD;
guc_context_policy_init(engine, desc);
@@ -1805,10 +1794,10 @@ static inline void guc_lrc_desc_unpin(struct intel_context *ce)
static void __guc_context_destroy(struct intel_context *ce)
{
- GEM_BUG_ON(ce->guc_prio_count[GUC_CLIENT_PRIORITY_KMD_HIGH] ||
- ce->guc_prio_count[GUC_CLIENT_PRIORITY_HIGH] ||
- ce->guc_prio_count[GUC_CLIENT_PRIORITY_KMD_NORMAL] ||
- ce->guc_prio_count[GUC_CLIENT_PRIORITY_NORMAL]);
+ GEM_BUG_ON(ce->guc_active.prio_count[GUC_CLIENT_PRIORITY_KMD_HIGH] ||
+ ce->guc_active.prio_count[GUC_CLIENT_PRIORITY_HIGH] ||
+ ce->guc_active.prio_count[GUC_CLIENT_PRIORITY_KMD_NORMAL] ||
+ ce->guc_active.prio_count[GUC_CLIENT_PRIORITY_NORMAL]);
GEM_BUG_ON(ce->guc_state.number_committed_requests);
lrc_fini(ce);
@@ -1918,14 +1907,17 @@ static void guc_context_set_prio(struct intel_guc *guc,
GEM_BUG_ON(prio < GUC_CLIENT_PRIORITY_KMD_HIGH ||
prio > GUC_CLIENT_PRIORITY_NORMAL);
+ lockdep_assert_held(>guc_active.lock);
- if (ce->guc_prio == prio || submission_disabled(guc) ||
- !context_registered(ce))
+ if
[Intel-gfx] [PATCH 23/27] drm/i915/guc: Move GuC priority fields in context under guc_active
Move GuC management fields in context under guc_active struct as this is
where the lock that protects theses fields lives. Also only set guc_prio
field once during context init.
v2:
(Daniele)
- set CONTEXT_SET_INIT
Signed-off-by: Matthew Brost
---
drivers/gpu/drm/i915/gt/intel_context_types.h | 12 ++--
.../gpu/drm/i915/gt/uc/intel_guc_submission.c | 69 +++
drivers/gpu/drm/i915/i915_trace.h | 2 +-
3 files changed, 46 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h
b/drivers/gpu/drm/i915/gt/intel_context_types.h
index 3a5d98e908f4..b56960a781da 100644
--- a/drivers/gpu/drm/i915/gt/intel_context_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
@@ -112,6 +112,7 @@ struct intel_context {
#define CONTEXT_FORCE_SINGLE_SUBMISSION7
#define CONTEXT_NOPREEMPT 8
#define CONTEXT_LRCA_DIRTY 9
+#define CONTEXT_GUC_INIT 10
struct {
u64 timeout_us;
@@ -178,6 +179,11 @@ struct intel_context {
spinlock_t lock;
/** requests: active requests on this context */
struct list_head requests;
+ /*
+* GuC priority management
+*/
+ u8 prio;
+ u32 prio_count[GUC_CLIENT_PRIORITY_NUM];
} guc_active;
/* GuC LRC descriptor ID */
@@ -191,12 +197,6 @@ struct intel_context {
*/
struct list_head guc_id_link;
- /*
-* GuC priority management
-*/
- u8 guc_prio;
- u32 guc_prio_count[GUC_CLIENT_PRIORITY_NUM];
-
#ifdef CONFIG_DRM_I915_SELFTEST
/**
* @drop_schedule_enable: Force drop of schedule enable G2H for selftest
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 14a512533c39..bc68c0122be4 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -1367,8 +1367,6 @@ static void guc_context_policy_init(struct
intel_engine_cs *engine,
desc->preemption_timeout = engine->props.preempt_timeout_ms * 1000;
}
-static inline u8 map_i915_prio_to_guc_prio(int prio);
-
static int guc_lrc_desc_pin(struct intel_context *ce, bool loop)
{
struct intel_engine_cs *engine = ce->engine;
@@ -1376,8 +1374,6 @@ static int guc_lrc_desc_pin(struct intel_context *ce,
bool loop)
struct intel_guc *guc = >gt->uc.guc;
u32 desc_idx = ce->guc_id;
struct guc_lrc_desc *desc;
- const struct i915_gem_context *ctx;
- int prio = I915_CONTEXT_DEFAULT_PRIORITY;
bool context_registered;
intel_wakeref_t wakeref;
int ret = 0;
@@ -1394,12 +1390,6 @@ static int guc_lrc_desc_pin(struct intel_context *ce,
bool loop)
context_registered = lrc_desc_registered(guc, desc_idx);
- rcu_read_lock();
- ctx = rcu_dereference(ce->gem_context);
- if (ctx)
- prio = ctx->sched.priority;
- rcu_read_unlock();
-
reset_lrc_desc(guc, desc_idx);
set_lrc_desc_registered(guc, desc_idx, ce);
@@ -1408,8 +1398,7 @@ static int guc_lrc_desc_pin(struct intel_context *ce,
bool loop)
desc->engine_submit_mask = adjust_engine_mask(engine->class,
engine->mask);
desc->hw_context_desc = ce->lrc.lrca;
- ce->guc_prio = map_i915_prio_to_guc_prio(prio);
- desc->priority = ce->guc_prio;
+ desc->priority = ce->guc_active.prio;
desc->context_flags = CONTEXT_REGISTRATION_FLAG_KMD;
guc_context_policy_init(engine, desc);
@@ -1805,10 +1794,10 @@ static inline void guc_lrc_desc_unpin(struct
intel_context *ce)
static void __guc_context_destroy(struct intel_context *ce)
{
- GEM_BUG_ON(ce->guc_prio_count[GUC_CLIENT_PRIORITY_KMD_HIGH] ||
- ce->guc_prio_count[GUC_CLIENT_PRIORITY_HIGH] ||
- ce->guc_prio_count[GUC_CLIENT_PRIORITY_KMD_NORMAL] ||
- ce->guc_prio_count[GUC_CLIENT_PRIORITY_NORMAL]);
+ GEM_BUG_ON(ce->guc_active.prio_count[GUC_CLIENT_PRIORITY_KMD_HIGH] ||
+ ce->guc_active.prio_count[GUC_CLIENT_PRIORITY_HIGH] ||
+ ce->guc_active.prio_count[GUC_CLIENT_PRIORITY_KMD_NORMAL] ||
+ ce->guc_active.prio_count[GUC_CLIENT_PRIORITY_NORMAL]);
GEM_BUG_ON(ce->guc_state.number_committed_requests);
lrc_fini(ce);
@@ -1918,14 +1907,17 @@ static void guc_context_set_prio(struct intel_guc *guc,
GEM_BUG_ON(prio < GUC_CLIENT_PRIORITY_KMD_HIGH ||
prio > GUC_CLIENT_PRIORITY_NORMAL);
+ lockdep_assert_held(>guc_active.lock);
- if (ce->guc_prio == prio || submission_disabled(guc) ||
- !context_registered(ce))
+ if (ce->guc_active.prio == prio || submission_disabled(guc) ||
+ !context_registered(ce))
Re: [Intel-gfx] [PATCH 23/27] drm/i915/guc: Move GuC priority fields in context under guc_active
On Wed, Aug 25, 2021 at 02:51:11PM -0700, Daniele Ceraolo Spurio wrote:
>
>
> On 8/18/2021 11:16 PM, Matthew Brost wrote:
> > Move GuC management fields in context under guc_active struct as this is
> > where the lock that protects theses fields lives. Also only set guc_prio
> > field once during context init.
>
> Can you explain what we gain by setting that only on first pin? AFAICS
> re-setting it doesn't hurt and we would cover the case where a context
> priority gets updated while the context is idle. I know the request
> submission would eventually update the prio so there is no bug, but that
> then requires an extra H2G.
>
> >
> > Fixes: ee242ca704d3 ("drm/i915/guc: Implement GuC priority management")
> > Signed-off-by: Matthew Brost
> > Cc:
> > ---
> > drivers/gpu/drm/i915/gt/intel_context_types.h | 12 ++--
> > .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 68 +++
> > drivers/gpu/drm/i915/i915_trace.h | 2 +-
> > 3 files changed, 45 insertions(+), 37 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h
> > b/drivers/gpu/drm/i915/gt/intel_context_types.h
> > index 524a35a78bf4..9fb0480ccf3b 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_context_types.h
> > +++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
> > @@ -112,6 +112,7 @@ struct intel_context {
> > #define CONTEXT_FORCE_SINGLE_SUBMISSION 7
> > #define CONTEXT_NOPREEMPT 8
> > #define CONTEXT_LRCA_DIRTY9
> > +#define CONTEXT_GUC_INIT 10
> > struct {
> > u64 timeout_us;
> > @@ -178,6 +179,11 @@ struct intel_context {
> > spinlock_t lock;
> > /** requests: active requests on this context */
> > struct list_head requests;
> > + /*
> > +* GuC priority management
> > +*/
> > + u8 prio;
> > + u32 prio_count[GUC_CLIENT_PRIORITY_NUM];
> > } guc_active;
> > /* GuC LRC descriptor ID */
> > @@ -191,12 +197,6 @@ struct intel_context {
> > */
> > struct list_head guc_id_link;
> > - /*
> > -* GuC priority management
> > -*/
> > - u8 guc_prio;
> > - u32 guc_prio_count[GUC_CLIENT_PRIORITY_NUM];
> > -
> > #ifdef CONFIG_DRM_I915_SELFTEST
> > /**
> > * @drop_schedule_enable: Force drop of schedule enable G2H for selftest
> > 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 3e90985b0c1b..bb90bedb1305 100644
> > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> > @@ -1369,8 +1369,6 @@ static void guc_context_policy_init(struct
> > intel_engine_cs *engine,
> > desc->preemption_timeout = engine->props.preempt_timeout_ms * 1000;
> > }
> > -static inline u8 map_i915_prio_to_guc_prio(int prio);
> > -
> > static int guc_lrc_desc_pin(struct intel_context *ce, bool loop)
> > {
> > struct intel_engine_cs *engine = ce->engine;
> > @@ -1378,8 +1376,6 @@ static int guc_lrc_desc_pin(struct intel_context *ce,
> > bool loop)
> > struct intel_guc *guc = >gt->uc.guc;
> > u32 desc_idx = ce->guc_id;
> > struct guc_lrc_desc *desc;
> > - const struct i915_gem_context *ctx;
> > - int prio = I915_CONTEXT_DEFAULT_PRIORITY;
> > bool context_registered;
> > intel_wakeref_t wakeref;
> > int ret = 0;
> > @@ -1396,12 +1392,6 @@ static int guc_lrc_desc_pin(struct intel_context
> > *ce, bool loop)
> > context_registered = lrc_desc_registered(guc, desc_idx);
> > - rcu_read_lock();
> > - ctx = rcu_dereference(ce->gem_context);
> > - if (ctx)
> > - prio = ctx->sched.priority;
> > - rcu_read_unlock();
> > -
> > reset_lrc_desc(guc, desc_idx);
> > set_lrc_desc_registered(guc, desc_idx, ce);
> > @@ -1410,8 +1400,7 @@ static int guc_lrc_desc_pin(struct intel_context *ce,
> > bool loop)
> > desc->engine_submit_mask = adjust_engine_mask(engine->class,
> > engine->mask);
> > desc->hw_context_desc = ce->lrc.lrca;
> > - ce->guc_prio = map_i915_prio_to_guc_prio(prio);
> > - desc->priority = ce->guc_prio;
> > + desc->priority = ce->guc_active.prio;
> > desc->context_flags = CONTEXT_REGISTRATION_FLAG_KMD;
> > guc_context_policy_init(engine, desc);
> > @@ -1813,10 +1802,10 @@ static inline void guc_lrc_desc_unpin(struct
> > intel_context *ce)
> > static void __guc_context_destroy(struct intel_context *ce)
> > {
> > - GEM_BUG_ON(ce->guc_prio_count[GUC_CLIENT_PRIORITY_KMD_HIGH] ||
> > - ce->guc_prio_count[GUC_CLIENT_PRIORITY_HIGH] ||
> > - ce->guc_prio_count[GUC_CLIENT_PRIORITY_KMD_NORMAL] ||
> > - ce->guc_prio_count[GUC_CLIENT_PRIORITY_NORMAL]);
> > + GEM_BUG_ON(ce->guc_active.prio_count[GUC_CLIENT_PRIORITY_KMD_HIGH] ||
> > + ce->guc_active.prio_count[GUC_CLIENT_PRIORITY_HIGH] ||
> > +
Re: [Intel-gfx] [PATCH 23/27] drm/i915/guc: Move GuC priority fields in context under guc_active
On Wed, Aug 25, 2021 at 02:51:11PM -0700, Daniele Ceraolo Spurio wrote:
>
>
> On 8/18/2021 11:16 PM, Matthew Brost wrote:
> > Move GuC management fields in context under guc_active struct as this is
> > where the lock that protects theses fields lives. Also only set guc_prio
> > field once during context init.
>
> Can you explain what we gain by setting that only on first pin? AFAICS
> re-setting it doesn't hurt and we would cover the case where a context
> priority gets updated while the context is idle. I know the request
> submission would eventually update the prio so there is no bug, but that
> then requires an extra H2G.
>
Contexts really shouldn't be getting registred and deregistered, so
real need to set this field on each register. Also the priority really
shouldn't be getting all the regularly. IMO this is the correct place,
so I moved it. Lastly, a subsequent patch will also use
guc_context_init() so the helper makes a bit more sense.
Matt
> >
> > Fixes: ee242ca704d3 ("drm/i915/guc: Implement GuC priority management")
> > Signed-off-by: Matthew Brost
> > Cc:
> > ---
> > drivers/gpu/drm/i915/gt/intel_context_types.h | 12 ++--
> > .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 68 +++
> > drivers/gpu/drm/i915/i915_trace.h | 2 +-
> > 3 files changed, 45 insertions(+), 37 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h
> > b/drivers/gpu/drm/i915/gt/intel_context_types.h
> > index 524a35a78bf4..9fb0480ccf3b 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_context_types.h
> > +++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
> > @@ -112,6 +112,7 @@ struct intel_context {
> > #define CONTEXT_FORCE_SINGLE_SUBMISSION 7
> > #define CONTEXT_NOPREEMPT 8
> > #define CONTEXT_LRCA_DIRTY9
> > +#define CONTEXT_GUC_INIT 10
> > struct {
> > u64 timeout_us;
> > @@ -178,6 +179,11 @@ struct intel_context {
> > spinlock_t lock;
> > /** requests: active requests on this context */
> > struct list_head requests;
> > + /*
> > +* GuC priority management
> > +*/
> > + u8 prio;
> > + u32 prio_count[GUC_CLIENT_PRIORITY_NUM];
> > } guc_active;
> > /* GuC LRC descriptor ID */
> > @@ -191,12 +197,6 @@ struct intel_context {
> > */
> > struct list_head guc_id_link;
> > - /*
> > -* GuC priority management
> > -*/
> > - u8 guc_prio;
> > - u32 guc_prio_count[GUC_CLIENT_PRIORITY_NUM];
> > -
> > #ifdef CONFIG_DRM_I915_SELFTEST
> > /**
> > * @drop_schedule_enable: Force drop of schedule enable G2H for selftest
> > 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 3e90985b0c1b..bb90bedb1305 100644
> > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> > @@ -1369,8 +1369,6 @@ static void guc_context_policy_init(struct
> > intel_engine_cs *engine,
> > desc->preemption_timeout = engine->props.preempt_timeout_ms * 1000;
> > }
> > -static inline u8 map_i915_prio_to_guc_prio(int prio);
> > -
> > static int guc_lrc_desc_pin(struct intel_context *ce, bool loop)
> > {
> > struct intel_engine_cs *engine = ce->engine;
> > @@ -1378,8 +1376,6 @@ static int guc_lrc_desc_pin(struct intel_context *ce,
> > bool loop)
> > struct intel_guc *guc = >gt->uc.guc;
> > u32 desc_idx = ce->guc_id;
> > struct guc_lrc_desc *desc;
> > - const struct i915_gem_context *ctx;
> > - int prio = I915_CONTEXT_DEFAULT_PRIORITY;
> > bool context_registered;
> > intel_wakeref_t wakeref;
> > int ret = 0;
> > @@ -1396,12 +1392,6 @@ static int guc_lrc_desc_pin(struct intel_context
> > *ce, bool loop)
> > context_registered = lrc_desc_registered(guc, desc_idx);
> > - rcu_read_lock();
> > - ctx = rcu_dereference(ce->gem_context);
> > - if (ctx)
> > - prio = ctx->sched.priority;
> > - rcu_read_unlock();
> > -
> > reset_lrc_desc(guc, desc_idx);
> > set_lrc_desc_registered(guc, desc_idx, ce);
> > @@ -1410,8 +1400,7 @@ static int guc_lrc_desc_pin(struct intel_context *ce,
> > bool loop)
> > desc->engine_submit_mask = adjust_engine_mask(engine->class,
> > engine->mask);
> > desc->hw_context_desc = ce->lrc.lrca;
> > - ce->guc_prio = map_i915_prio_to_guc_prio(prio);
> > - desc->priority = ce->guc_prio;
> > + desc->priority = ce->guc_active.prio;
> > desc->context_flags = CONTEXT_REGISTRATION_FLAG_KMD;
> > guc_context_policy_init(engine, desc);
> > @@ -1813,10 +1802,10 @@ static inline void guc_lrc_desc_unpin(struct
> > intel_context *ce)
> > static void __guc_context_destroy(struct intel_context *ce)
> > {
> > - GEM_BUG_ON(ce->guc_prio_count[GUC_CLIENT_PRIORITY_KMD_HIGH] ||
> > -
Re: [Intel-gfx] [PATCH 23/27] drm/i915/guc: Move GuC priority fields in context under guc_active
On 8/18/2021 11:16 PM, Matthew Brost wrote:
Move GuC management fields in context under guc_active struct as this is
where the lock that protects theses fields lives. Also only set guc_prio
field once during context init.
Can you explain what we gain by setting that only on first pin? AFAICS
re-setting it doesn't hurt and we would cover the case where a context
priority gets updated while the context is idle. I know the request
submission would eventually update the prio so there is no bug, but that
then requires an extra H2G.
Fixes: ee242ca704d3 ("drm/i915/guc: Implement GuC priority management")
Signed-off-by: Matthew Brost
Cc:
---
drivers/gpu/drm/i915/gt/intel_context_types.h | 12 ++--
.../gpu/drm/i915/gt/uc/intel_guc_submission.c | 68 +++
drivers/gpu/drm/i915/i915_trace.h | 2 +-
3 files changed, 45 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h
b/drivers/gpu/drm/i915/gt/intel_context_types.h
index 524a35a78bf4..9fb0480ccf3b 100644
--- a/drivers/gpu/drm/i915/gt/intel_context_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
@@ -112,6 +112,7 @@ struct intel_context {
#define CONTEXT_FORCE_SINGLE_SUBMISSION 7
#define CONTEXT_NOPREEMPT 8
#define CONTEXT_LRCA_DIRTY9
+#define CONTEXT_GUC_INIT 10
struct {
u64 timeout_us;
@@ -178,6 +179,11 @@ struct intel_context {
spinlock_t lock;
/** requests: active requests on this context */
struct list_head requests;
+ /*
+* GuC priority management
+*/
+ u8 prio;
+ u32 prio_count[GUC_CLIENT_PRIORITY_NUM];
} guc_active;
/* GuC LRC descriptor ID */
@@ -191,12 +197,6 @@ struct intel_context {
*/
struct list_head guc_id_link;
- /*
-* GuC priority management
-*/
- u8 guc_prio;
- u32 guc_prio_count[GUC_CLIENT_PRIORITY_NUM];
-
#ifdef CONFIG_DRM_I915_SELFTEST
/**
* @drop_schedule_enable: Force drop of schedule enable G2H for selftest
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 3e90985b0c1b..bb90bedb1305 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -1369,8 +1369,6 @@ static void guc_context_policy_init(struct
intel_engine_cs *engine,
desc->preemption_timeout = engine->props.preempt_timeout_ms * 1000;
}
-static inline u8 map_i915_prio_to_guc_prio(int prio);
-
static int guc_lrc_desc_pin(struct intel_context *ce, bool loop)
{
struct intel_engine_cs *engine = ce->engine;
@@ -1378,8 +1376,6 @@ static int guc_lrc_desc_pin(struct intel_context *ce,
bool loop)
struct intel_guc *guc = >gt->uc.guc;
u32 desc_idx = ce->guc_id;
struct guc_lrc_desc *desc;
- const struct i915_gem_context *ctx;
- int prio = I915_CONTEXT_DEFAULT_PRIORITY;
bool context_registered;
intel_wakeref_t wakeref;
int ret = 0;
@@ -1396,12 +1392,6 @@ static int guc_lrc_desc_pin(struct intel_context *ce,
bool loop)
context_registered = lrc_desc_registered(guc, desc_idx);
- rcu_read_lock();
- ctx = rcu_dereference(ce->gem_context);
- if (ctx)
- prio = ctx->sched.priority;
- rcu_read_unlock();
-
reset_lrc_desc(guc, desc_idx);
set_lrc_desc_registered(guc, desc_idx, ce);
@@ -1410,8 +1400,7 @@ static int guc_lrc_desc_pin(struct intel_context *ce, bool loop)
desc->engine_submit_mask = adjust_engine_mask(engine->class,
engine->mask);
desc->hw_context_desc = ce->lrc.lrca;
- ce->guc_prio = map_i915_prio_to_guc_prio(prio);
- desc->priority = ce->guc_prio;
+ desc->priority = ce->guc_active.prio;
desc->context_flags = CONTEXT_REGISTRATION_FLAG_KMD;
guc_context_policy_init(engine, desc);
@@ -1813,10 +1802,10 @@ static inline void guc_lrc_desc_unpin(struct intel_context *ce)
static void __guc_context_destroy(struct intel_context *ce)
{
- GEM_BUG_ON(ce->guc_prio_count[GUC_CLIENT_PRIORITY_KMD_HIGH] ||
- ce->guc_prio_count[GUC_CLIENT_PRIORITY_HIGH] ||
- ce->guc_prio_count[GUC_CLIENT_PRIORITY_KMD_NORMAL] ||
- ce->guc_prio_count[GUC_CLIENT_PRIORITY_NORMAL]);
+ GEM_BUG_ON(ce->guc_active.prio_count[GUC_CLIENT_PRIORITY_KMD_HIGH] ||
+ ce->guc_active.prio_count[GUC_CLIENT_PRIORITY_HIGH] ||
+ ce->guc_active.prio_count[GUC_CLIENT_PRIORITY_KMD_NORMAL] ||
+ ce->guc_active.prio_count[GUC_CLIENT_PRIORITY_NORMAL]);
GEM_BUG_ON(ce->guc_state.number_committed_requests);
lrc_fini(ce);
@@ -1926,14 +1915,17 @@ static void
[Intel-gfx] [PATCH 23/27] drm/i915/guc: Move GuC priority fields in context under guc_active
Move GuC management fields in context under guc_active struct as this is
where the lock that protects theses fields lives. Also only set guc_prio
field once during context init.
Fixes: ee242ca704d3 ("drm/i915/guc: Implement GuC priority management")
Signed-off-by: Matthew Brost
Cc:
---
drivers/gpu/drm/i915/gt/intel_context_types.h | 12 ++--
.../gpu/drm/i915/gt/uc/intel_guc_submission.c | 68 +++
drivers/gpu/drm/i915/i915_trace.h | 2 +-
3 files changed, 45 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h
b/drivers/gpu/drm/i915/gt/intel_context_types.h
index 524a35a78bf4..9fb0480ccf3b 100644
--- a/drivers/gpu/drm/i915/gt/intel_context_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
@@ -112,6 +112,7 @@ struct intel_context {
#define CONTEXT_FORCE_SINGLE_SUBMISSION7
#define CONTEXT_NOPREEMPT 8
#define CONTEXT_LRCA_DIRTY 9
+#define CONTEXT_GUC_INIT 10
struct {
u64 timeout_us;
@@ -178,6 +179,11 @@ struct intel_context {
spinlock_t lock;
/** requests: active requests on this context */
struct list_head requests;
+ /*
+* GuC priority management
+*/
+ u8 prio;
+ u32 prio_count[GUC_CLIENT_PRIORITY_NUM];
} guc_active;
/* GuC LRC descriptor ID */
@@ -191,12 +197,6 @@ struct intel_context {
*/
struct list_head guc_id_link;
- /*
-* GuC priority management
-*/
- u8 guc_prio;
- u32 guc_prio_count[GUC_CLIENT_PRIORITY_NUM];
-
#ifdef CONFIG_DRM_I915_SELFTEST
/**
* @drop_schedule_enable: Force drop of schedule enable G2H for selftest
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 3e90985b0c1b..bb90bedb1305 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -1369,8 +1369,6 @@ static void guc_context_policy_init(struct
intel_engine_cs *engine,
desc->preemption_timeout = engine->props.preempt_timeout_ms * 1000;
}
-static inline u8 map_i915_prio_to_guc_prio(int prio);
-
static int guc_lrc_desc_pin(struct intel_context *ce, bool loop)
{
struct intel_engine_cs *engine = ce->engine;
@@ -1378,8 +1376,6 @@ static int guc_lrc_desc_pin(struct intel_context *ce,
bool loop)
struct intel_guc *guc = >gt->uc.guc;
u32 desc_idx = ce->guc_id;
struct guc_lrc_desc *desc;
- const struct i915_gem_context *ctx;
- int prio = I915_CONTEXT_DEFAULT_PRIORITY;
bool context_registered;
intel_wakeref_t wakeref;
int ret = 0;
@@ -1396,12 +1392,6 @@ static int guc_lrc_desc_pin(struct intel_context *ce,
bool loop)
context_registered = lrc_desc_registered(guc, desc_idx);
- rcu_read_lock();
- ctx = rcu_dereference(ce->gem_context);
- if (ctx)
- prio = ctx->sched.priority;
- rcu_read_unlock();
-
reset_lrc_desc(guc, desc_idx);
set_lrc_desc_registered(guc, desc_idx, ce);
@@ -1410,8 +1400,7 @@ static int guc_lrc_desc_pin(struct intel_context *ce,
bool loop)
desc->engine_submit_mask = adjust_engine_mask(engine->class,
engine->mask);
desc->hw_context_desc = ce->lrc.lrca;
- ce->guc_prio = map_i915_prio_to_guc_prio(prio);
- desc->priority = ce->guc_prio;
+ desc->priority = ce->guc_active.prio;
desc->context_flags = CONTEXT_REGISTRATION_FLAG_KMD;
guc_context_policy_init(engine, desc);
@@ -1813,10 +1802,10 @@ static inline void guc_lrc_desc_unpin(struct
intel_context *ce)
static void __guc_context_destroy(struct intel_context *ce)
{
- GEM_BUG_ON(ce->guc_prio_count[GUC_CLIENT_PRIORITY_KMD_HIGH] ||
- ce->guc_prio_count[GUC_CLIENT_PRIORITY_HIGH] ||
- ce->guc_prio_count[GUC_CLIENT_PRIORITY_KMD_NORMAL] ||
- ce->guc_prio_count[GUC_CLIENT_PRIORITY_NORMAL]);
+ GEM_BUG_ON(ce->guc_active.prio_count[GUC_CLIENT_PRIORITY_KMD_HIGH] ||
+ ce->guc_active.prio_count[GUC_CLIENT_PRIORITY_HIGH] ||
+ ce->guc_active.prio_count[GUC_CLIENT_PRIORITY_KMD_NORMAL] ||
+ ce->guc_active.prio_count[GUC_CLIENT_PRIORITY_NORMAL]);
GEM_BUG_ON(ce->guc_state.number_committed_requests);
lrc_fini(ce);
@@ -1926,14 +1915,17 @@ static void guc_context_set_prio(struct intel_guc *guc,
GEM_BUG_ON(prio < GUC_CLIENT_PRIORITY_KMD_HIGH ||
prio > GUC_CLIENT_PRIORITY_NORMAL);
+ lockdep_assert_held(>guc_active.lock);
- if (ce->guc_prio == prio || submission_disabled(guc) ||
- !context_registered(ce))
+ if (ce->guc_active.prio == prio || submission_disabled(guc) ||
+
