From: Arnd Bergmann <arnd@arndb.de>
To: Jani Nikula <jani.nikula@linux.intel.com>,
Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
Rodrigo Vivi <rodrigo.vivi@intel.com>,
David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>,
Tvrtko Ursulin <tvrtko.ursulin@intel.com>,
Chris Wilson <chris@chris-wilson.co.uk>
Cc: "Arnd Bergmann" <arnd@arndb.de>,
"Michał Winiarski" <michal.winiarski@intel.com>,
"Andi Shyti" <andi.shyti@intel.com>,
intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
linux-kernel@vger.kernel.org
Subject: [PATCH 1/3] drm/i915/pmu: avoid an maybe-uninitialized warning
Date: Wed, 27 May 2020 16:05:08 +0200 [thread overview]
Message-ID: <20200527140526.1458215-1-arnd@arndb.de> (raw)
Conditional spinlocks make it hard for gcc and for lockdep to
follow the code flow. This one causes a warning with at least
gcc-9 and higher:
In file included from include/linux/irq.h:14,
from drivers/gpu/drm/i915/i915_pmu.c:7:
drivers/gpu/drm/i915/i915_pmu.c: In function 'i915_sample':
include/linux/spinlock.h:289:3: error: 'flags' may be used uninitialized in this function [-Werror=maybe-uninitialized]
289 | _raw_spin_unlock_irqrestore(lock, flags); \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/i915/i915_pmu.c:288:17: note: 'flags' was declared here
288 | unsigned long flags;
| ^~~~~
Split out the part between the locks into a separate function
for readability and to let the compiler figure out what the
logic actually is.
Fixes: d79e1bd676f0 ("drm/i915/pmu: Only use exclusive mmio access for gen7")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
I have no idea why I see three separate issues like this pop up in i915,
there are not a lot of them elsewhere.
drivers/gpu/drm/i915/i915_pmu.c | 84 ++++++++++++++++-----------------
1 file changed, 42 insertions(+), 42 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index e991a707bdb7..962ded9ce73f 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -269,12 +269,48 @@ static bool exclusive_mmio_access(const struct drm_i915_private *i915)
return IS_GEN(i915, 7);
}
+static void engine_sample(struct intel_engine_cs *engine, unsigned int period_ns)
+{
+ struct intel_engine_pmu *pmu = &engine->pmu;
+ bool busy;
+ u32 val;
+
+ val = ENGINE_READ_FW(engine, RING_CTL);
+ if (val == 0) /* powerwell off => engine idle */
+ return;
+
+ if (val & RING_WAIT)
+ add_sample(&pmu->sample[I915_SAMPLE_WAIT], period_ns);
+ if (val & RING_WAIT_SEMAPHORE)
+ add_sample(&pmu->sample[I915_SAMPLE_SEMA], period_ns);
+
+ /* No need to sample when busy stats are supported. */
+ if (intel_engine_supports_stats(engine))
+ return;
+
+ /*
+ * While waiting on a semaphore or event, MI_MODE reports the
+ * ring as idle. However, previously using the seqno, and with
+ * execlists sampling, we account for the ring waiting as the
+ * engine being busy. Therefore, we record the sample as being
+ * busy if either waiting or !idle.
+ */
+ busy = val & (RING_WAIT_SEMAPHORE | RING_WAIT);
+ if (!busy) {
+ val = ENGINE_READ_FW(engine, RING_MI_MODE);
+ busy = !(val & MODE_IDLE);
+ }
+ if (busy)
+ add_sample(&pmu->sample[I915_SAMPLE_BUSY], period_ns);
+}
+
static void
engines_sample(struct intel_gt *gt, unsigned int period_ns)
{
struct drm_i915_private *i915 = gt->i915;
struct intel_engine_cs *engine;
enum intel_engine_id id;
+ unsigned long flags;
if ((i915->pmu.enable & ENGINE_SAMPLE_MASK) == 0)
return;
@@ -283,53 +319,17 @@ engines_sample(struct intel_gt *gt, unsigned int period_ns)
return;
for_each_engine(engine, gt, id) {
- struct intel_engine_pmu *pmu = &engine->pmu;
- spinlock_t *mmio_lock;
- unsigned long flags;
- bool busy;
- u32 val;
-
if (!intel_engine_pm_get_if_awake(engine))
continue;
- mmio_lock = NULL;
- if (exclusive_mmio_access(i915))
- mmio_lock = &engine->uncore->lock;
-
- if (unlikely(mmio_lock))
- spin_lock_irqsave(mmio_lock, flags);
-
- val = ENGINE_READ_FW(engine, RING_CTL);
- if (val == 0) /* powerwell off => engine idle */
- goto skip;
-
- if (val & RING_WAIT)
- add_sample(&pmu->sample[I915_SAMPLE_WAIT], period_ns);
- if (val & RING_WAIT_SEMAPHORE)
- add_sample(&pmu->sample[I915_SAMPLE_SEMA], period_ns);
-
- /* No need to sample when busy stats are supported. */
- if (intel_engine_supports_stats(engine))
- goto skip;
-
- /*
- * While waiting on a semaphore or event, MI_MODE reports the
- * ring as idle. However, previously using the seqno, and with
- * execlists sampling, we account for the ring waiting as the
- * engine being busy. Therefore, we record the sample as being
- * busy if either waiting or !idle.
- */
- busy = val & (RING_WAIT_SEMAPHORE | RING_WAIT);
- if (!busy) {
- val = ENGINE_READ_FW(engine, RING_MI_MODE);
- busy = !(val & MODE_IDLE);
+ if (exclusive_mmio_access(i915)) {
+ spin_lock_irqsave(&engine->uncore->lock, flags);
+ engine_sample(engine, period_ns);
+ spin_unlock_irqrestore(&engine->uncore->lock, flags);
+ } else {
+ engine_sample(engine, period_ns);
}
- if (busy)
- add_sample(&pmu->sample[I915_SAMPLE_BUSY], period_ns);
-skip:
- if (unlikely(mmio_lock))
- spin_unlock_irqrestore(mmio_lock, flags);
intel_engine_pm_put_async(engine);
}
}
--
2.26.2
next reply other threads:[~2020-05-27 14:05 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-27 14:05 Arnd Bergmann [this message]
2020-05-27 14:05 ` [PATCH 2/3] drm/i915: work around false-positive maybe-uninitialized warning Arnd Bergmann
2020-05-27 15:43 ` Chris Wilson
2020-05-27 14:05 ` [PATCH 3/3] drm/i915/selftests: avoid bogus " Arnd Bergmann
2020-05-27 15:47 ` Chris Wilson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200527140526.1458215-1-arnd@arndb.de \
--to=arnd@arndb.de \
--cc=airlied@linux.ie \
--cc=andi.shyti@intel.com \
--cc=chris@chris-wilson.co.uk \
--cc=daniel@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=intel-gfx@lists.freedesktop.org \
--cc=jani.nikula@linux.intel.com \
--cc=joonas.lahtinen@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=michal.winiarski@intel.com \
--cc=rodrigo.vivi@intel.com \
--cc=tvrtko.ursulin@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).