| Message ID | 20220217212942.629922-1-John.C.Harrison@Intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/guc: Fix flag query helper function to not modify state | expand |
On 2/17/2022 1:29 PM, John.C.Harrison@Intel.com wrote: > From: John Harrison<John.C.Harrison@Intel.com> > > A flag query helper was actually writing to the flags word rather than > just reading. Fix that. Also update the function's comment as it was > out of date. > > NB: No need for a 'Fixes' tag. The test was only ever used inside a > BUG_ON during context registration. Rather than asserting that the > condition was true, it was making the condition true. So, in theory, > there was no consequence because we should never have hit a BUG_ON > anyway. Which means the write should always have been a no-op. > > Signed-off-by: John Harrison<John.C.Harrison@Intel.com> |Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Daniele| > --- > drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 7 ++----- > 1 file changed, 2 insertions(+), 5 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 b3a429a92c0d..d9f4218f5ef4 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > @@ -174,11 +174,8 @@ static inline void init_sched_state(struct intel_context *ce) > __maybe_unused > static bool sched_state_is_init(struct intel_context *ce) > { > - /* > - * XXX: Kernel contexts can have SCHED_STATE_NO_LOCK_REGISTERED after > - * suspend. > - */ > - return !(ce->guc_state.sched_state &= > + /* Kernel contexts can have SCHED_STATE_REGISTERED after suspend. */ > + return !(ce->guc_state.sched_state & > ~(SCHED_STATE_BLOCKED_MASK | SCHED_STATE_REGISTERED)); > } >
On 2/17/2022 1:29 PM, John.C.Harrison@Intel.com wrote: > From: John Harrison <John.C.Harrison@Intel.com> > > A flag query helper was actually writing to the flags word rather than > just reading. Fix that. Also update the function's comment as it was > out of date. > > NB: No need for a 'Fixes' tag. The test was only ever used inside a > BUG_ON during context registration. Rather than asserting that the > condition was true, it was making the condition true. So, in theory, > there was no consequence because we should never have hit a BUG_ON > anyway. Which means the write should always have been a no-op. > > Signed-off-by: John Harrison <John.C.Harrison@Intel.com> I seem to have confused patchwork by doing a cut & paste of my r-b from a different review, so here it is again: Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Daniele > --- > drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 7 ++----- > 1 file changed, 2 insertions(+), 5 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 b3a429a92c0d..d9f4218f5ef4 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > @@ -174,11 +174,8 @@ static inline void init_sched_state(struct intel_context *ce) > __maybe_unused > static bool sched_state_is_init(struct intel_context *ce) > { > - /* > - * XXX: Kernel contexts can have SCHED_STATE_NO_LOCK_REGISTERED after > - * suspend. > - */ > - return !(ce->guc_state.sched_state &= > + /* Kernel contexts can have SCHED_STATE_REGISTERED after suspend. */ > + return !(ce->guc_state.sched_state & > ~(SCHED_STATE_BLOCKED_MASK | SCHED_STATE_REGISTERED)); > } >
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 b3a429a92c0d..d9f4218f5ef4 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -174,11 +174,8 @@ static inline void init_sched_state(struct intel_context *ce) __maybe_unused static bool sched_state_is_init(struct intel_context *ce) { - /* - * XXX: Kernel contexts can have SCHED_STATE_NO_LOCK_REGISTERED after - * suspend. - */ - return !(ce->guc_state.sched_state &= + /* Kernel contexts can have SCHED_STATE_REGISTERED after suspend. */ + return !(ce->guc_state.sched_state & ~(SCHED_STATE_BLOCKED_MASK | SCHED_STATE_REGISTERED)); }