| Message ID | 20231026135839.486894-1-gilbertadikankwu@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/gt: Remove assignment from if condition | expand |
On 26/10/2023 14:58, Gilbert Adikankwu wrote: > Initialize variable "entry" during declaration. Remove assignment from if > condition. > > Fix checkpatch.pl error: > ERROR: do not use assignment in if condition > > Signed-off-by: Gilbert Adikankwu <gilbertadikankwu@gmail.com> > --- > drivers/gpu/drm/i915/gt/intel_execlists_submission.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c > index e8f42ec6b1b4..cbc4ecf26d8b 100644 > --- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c > +++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c > @@ -1751,7 +1751,7 @@ static bool gen8_csb_parse(const u64 csb) > static noinline u64 > wa_csb_read(const struct intel_engine_cs *engine, u64 * const csb) > { > - u64 entry; > + u64 entry = READ_ONCE(*csb); > > /* > * Reading from the HWSP has one particular advantage: we can detect > @@ -1763,7 +1763,7 @@ wa_csb_read(const struct intel_engine_cs *engine, u64 * const csb) > * tgl,dg1:HSDES#22011327657 > */ > preempt_disable(); > - if (wait_for_atomic_us((entry = READ_ONCE(*csb)) != -1, 10)) { > + if (wait_for_atomic_us(entry != -1, 10)) { Wait_for_atomic_us is a macro which needs to keep reading the csb until it changes so this will not work. Regards, Tvrtko > int idx = csb - engine->execlists.csb_status; > int status; >
Hi Gilbert, > diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c > index e8f42ec6b1b4..cbc4ecf26d8b 100644 > --- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c > +++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c > @@ -1751,7 +1751,7 @@ static bool gen8_csb_parse(const u64 csb) > static noinline u64 > wa_csb_read(const struct intel_engine_cs *engine, u64 * const csb) > { > - u64 entry; > + u64 entry = READ_ONCE(*csb); > > /* > * Reading from the HWSP has one particular advantage: we can detect > @@ -1763,7 +1763,7 @@ wa_csb_read(const struct intel_engine_cs *engine, u64 * const csb) > * tgl,dg1:HSDES#22011327657 > */ > preempt_disable(); > - if (wait_for_atomic_us((entry = READ_ONCE(*csb)) != -1, 10)) { > + if (wait_for_atomic_us(entry != -1, 10)) { this doesn't work. Hav you looked inside the wait_for_atomic_us? It's busy looping for 10us waiting for *csb to be different from -1. Because the entry value won't have a chance to change, then we would be waiting 10us for nothing. Not all the checkpatch errors need to be fixed and we can live with this one. Andi > int idx = csb - engine->execlists.csb_status; > int status; > > -- > 2.34.1
diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c index e8f42ec6b1b4..cbc4ecf26d8b 100644 --- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c +++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c @@ -1751,7 +1751,7 @@ static bool gen8_csb_parse(const u64 csb) static noinline u64 wa_csb_read(const struct intel_engine_cs *engine, u64 * const csb) { - u64 entry; + u64 entry = READ_ONCE(*csb); /* * Reading from the HWSP has one particular advantage: we can detect @@ -1763,7 +1763,7 @@ wa_csb_read(const struct intel_engine_cs *engine, u64 * const csb) * tgl,dg1:HSDES#22011327657 */ preempt_disable(); - if (wait_for_atomic_us((entry = READ_ONCE(*csb)) != -1, 10)) { + if (wait_for_atomic_us(entry != -1, 10)) { int idx = csb - engine->execlists.csb_status; int status;
Initialize variable "entry" during declaration. Remove assignment from if condition. Fix checkpatch.pl error: ERROR: do not use assignment in if condition Signed-off-by: Gilbert Adikankwu <gilbertadikankwu@gmail.com> --- drivers/gpu/drm/i915/gt/intel_execlists_submission.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)