Re: [Intel-gfx] [PATCH] drm/i915/pmu: Atomically acquire the gt_pm wakeref
Quoting Tvrtko Ursulin (2019-08-01 15:55:29) > > On 01/08/2019 15:39, Chris Wilson wrote: > > Quoting Tvrtko Ursulin (2019-08-01 15:21:17) > >> I guess I'll be rebasing mine, at some point. :) > > > > I anticipated you merging it at some point. > > This patch to my series or what? Your series to update the locals, those first patches should be non-controversial. The fixup is trivial either way. > > Had a thought, and we don't need gt_pm for the engine sampling, but > > rather intel_engine_pm_get_if_awake. > > I thought about it, but since we iterate all engines did not see a real > benefit. Wouldn't harm either, only some more CPU cycles in the loop, so > up to you. But also likely to save some work on larger devices if they are mostly idle, which I hope they typically are. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/pmu: Atomically acquire the gt_pm wakeref
On 01/08/2019 15:39, Chris Wilson wrote: Quoting Tvrtko Ursulin (2019-08-01 15:21:17) On 01/08/2019 13:20, Chris Wilson wrote: Currently, we only sample if the intel_gt is awake, but we acquire our own runtime_pm wakeref. Since intel_gt has transitioned to tracking its own wakeref, we can atomically test and acquire that wakeref instead. Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin --- No automagic drop! --- drivers/gpu/drm/i915/gt/intel_gt_pm.h | 8 +++- drivers/gpu/drm/i915/i915_pmu.c | 23 --- 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.h b/drivers/gpu/drm/i915/gt/intel_gt_pm.h index ba960e1fc209..016298483de7 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.h +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.h @@ -9,7 +9,8 @@ #include -struct intel_gt; +#include "intel_gt_types.h" +#include "intel_wakeref.h" enum { INTEL_GT_UNPARK, @@ -19,6 +20,11 @@ enum { void intel_gt_pm_get(struct intel_gt *gt); void intel_gt_pm_put(struct intel_gt *gt); +static inline bool intel_gt_pm_get_if_awake(struct intel_gt *gt) +{ + return intel_wakeref_get_if_active(>wakeref); +} + void intel_gt_pm_init_early(struct intel_gt *gt); void intel_gt_sanitize(struct intel_gt *gt, bool force); diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c index eff86483bec0..e71192804996 100644 --- a/drivers/gpu/drm/i915/i915_pmu.c +++ b/drivers/gpu/drm/i915/i915_pmu.c @@ -8,6 +8,7 @@ #include #include "gt/intel_engine.h" +#include "gt/intel_gt_pm.h" #include "i915_drv.h" #include "i915_pmu.h" @@ -161,16 +162,12 @@ engines_sample(struct drm_i915_private *dev_priv, unsigned int period_ns) { struct intel_engine_cs *engine; enum intel_engine_id id; - intel_wakeref_t wakeref; unsigned long flags; if ((dev_priv->pmu.enable & ENGINE_SAMPLE_MASK) == 0) return; - wakeref = 0; - if (READ_ONCE(dev_priv->gt.awake)) - wakeref = intel_runtime_pm_get_if_in_use(_priv->runtime_pm); - if (!wakeref) + if (!intel_gt_pm_get_if_awake(_priv->gt)) return; spin_lock_irqsave(_priv->uncore.lock, flags); @@ -205,7 +202,7 @@ engines_sample(struct drm_i915_private *dev_priv, unsigned int period_ns) } spin_unlock_irqrestore(_priv->uncore.lock, flags); - intel_runtime_pm_put(_priv->runtime_pm, wakeref); + intel_gt_pm_put(_priv->gt); } static void @@ -222,15 +219,11 @@ frequency_sample(struct drm_i915_private *dev_priv, unsigned int period_ns) u32 val; val = dev_priv->gt_pm.rps.cur_freq; - if (dev_priv->gt.awake) { - intel_wakeref_t wakeref; - - with_intel_runtime_pm_if_in_use(_priv->runtime_pm, - wakeref) { - val = intel_uncore_read_notrace(_priv->uncore, - GEN6_RPSTAT1); - val = intel_get_cagf(dev_priv, val); - } + if (intel_gt_pm_get_if_awake(_priv->gt)) { + val = intel_uncore_read_notrace(_priv->uncore, + GEN6_RPSTAT1); + val = intel_get_cagf(dev_priv, val); + intel_gt_pm_put(_priv->gt); } add_sample_mult(_priv->pmu.sample[__I915_SAMPLE_FREQ_ACT], I guess I'll be rebasing mine, at some point. :) I anticipated you merging it at some point. This patch to my series or what? Had a thought, and we don't need gt_pm for the engine sampling, but rather intel_engine_pm_get_if_awake. I thought about it, but since we iterate all engines did not see a real benefit. Wouldn't harm either, only some more CPU cycles in the loop, so up to you. Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/pmu: Atomically acquire the gt_pm wakeref
Quoting Tvrtko Ursulin (2019-08-01 15:21:17) > > On 01/08/2019 13:20, Chris Wilson wrote: > > Currently, we only sample if the intel_gt is awake, but we acquire our > > own runtime_pm wakeref. Since intel_gt has transitioned to tracking its > > own wakeref, we can atomically test and acquire that wakeref instead. > > > > Signed-off-by: Chris Wilson > > Cc: Tvrtko Ursulin > > --- > > No automagic drop! > > --- > > drivers/gpu/drm/i915/gt/intel_gt_pm.h | 8 +++- > > drivers/gpu/drm/i915/i915_pmu.c | 23 --- > > 2 files changed, 15 insertions(+), 16 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.h > > b/drivers/gpu/drm/i915/gt/intel_gt_pm.h > > index ba960e1fc209..016298483de7 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.h > > +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.h > > @@ -9,7 +9,8 @@ > > > > #include > > > > -struct intel_gt; > > +#include "intel_gt_types.h" > > +#include "intel_wakeref.h" > > > > enum { > > INTEL_GT_UNPARK, > > @@ -19,6 +20,11 @@ enum { > > void intel_gt_pm_get(struct intel_gt *gt); > > void intel_gt_pm_put(struct intel_gt *gt); > > > > +static inline bool intel_gt_pm_get_if_awake(struct intel_gt *gt) > > +{ > > + return intel_wakeref_get_if_active(>wakeref); > > +} > > + > > void intel_gt_pm_init_early(struct intel_gt *gt); > > > > void intel_gt_sanitize(struct intel_gt *gt, bool force); > > diff --git a/drivers/gpu/drm/i915/i915_pmu.c > > b/drivers/gpu/drm/i915/i915_pmu.c > > index eff86483bec0..e71192804996 100644 > > --- a/drivers/gpu/drm/i915/i915_pmu.c > > +++ b/drivers/gpu/drm/i915/i915_pmu.c > > @@ -8,6 +8,7 @@ > > #include > > > > #include "gt/intel_engine.h" > > +#include "gt/intel_gt_pm.h" > > > > #include "i915_drv.h" > > #include "i915_pmu.h" > > @@ -161,16 +162,12 @@ engines_sample(struct drm_i915_private *dev_priv, > > unsigned int period_ns) > > { > > struct intel_engine_cs *engine; > > enum intel_engine_id id; > > - intel_wakeref_t wakeref; > > unsigned long flags; > > > > if ((dev_priv->pmu.enable & ENGINE_SAMPLE_MASK) == 0) > > return; > > > > - wakeref = 0; > > - if (READ_ONCE(dev_priv->gt.awake)) > > - wakeref = > > intel_runtime_pm_get_if_in_use(_priv->runtime_pm); > > - if (!wakeref) > > + if (!intel_gt_pm_get_if_awake(_priv->gt)) > > return; > > > > spin_lock_irqsave(_priv->uncore.lock, flags); > > @@ -205,7 +202,7 @@ engines_sample(struct drm_i915_private *dev_priv, > > unsigned int period_ns) > > } > > spin_unlock_irqrestore(_priv->uncore.lock, flags); > > > > - intel_runtime_pm_put(_priv->runtime_pm, wakeref); > > + intel_gt_pm_put(_priv->gt); > > } > > > > static void > > @@ -222,15 +219,11 @@ frequency_sample(struct drm_i915_private *dev_priv, > > unsigned int period_ns) > > u32 val; > > > > val = dev_priv->gt_pm.rps.cur_freq; > > - if (dev_priv->gt.awake) { > > - intel_wakeref_t wakeref; > > - > > - with_intel_runtime_pm_if_in_use(_priv->runtime_pm, > > - wakeref) { > > - val = > > intel_uncore_read_notrace(_priv->uncore, > > - GEN6_RPSTAT1); > > - val = intel_get_cagf(dev_priv, val); > > - } > > + if (intel_gt_pm_get_if_awake(_priv->gt)) { > > + val = intel_uncore_read_notrace(_priv->uncore, > > + GEN6_RPSTAT1); > > + val = intel_get_cagf(dev_priv, val); > > + intel_gt_pm_put(_priv->gt); > > } > > > > add_sample_mult(_priv->pmu.sample[__I915_SAMPLE_FREQ_ACT], > > > > I guess I'll be rebasing mine, at some point. :) I anticipated you merging it at some point. Had a thought, and we don't need gt_pm for the engine sampling, but rather intel_engine_pm_get_if_awake. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/pmu: Atomically acquire the gt_pm wakeref
On 01/08/2019 13:20, Chris Wilson wrote: Currently, we only sample if the intel_gt is awake, but we acquire our own runtime_pm wakeref. Since intel_gt has transitioned to tracking its own wakeref, we can atomically test and acquire that wakeref instead. Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin --- No automagic drop! --- drivers/gpu/drm/i915/gt/intel_gt_pm.h | 8 +++- drivers/gpu/drm/i915/i915_pmu.c | 23 --- 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.h b/drivers/gpu/drm/i915/gt/intel_gt_pm.h index ba960e1fc209..016298483de7 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.h +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.h @@ -9,7 +9,8 @@ #include -struct intel_gt; +#include "intel_gt_types.h" +#include "intel_wakeref.h" enum { INTEL_GT_UNPARK, @@ -19,6 +20,11 @@ enum { void intel_gt_pm_get(struct intel_gt *gt); void intel_gt_pm_put(struct intel_gt *gt); +static inline bool intel_gt_pm_get_if_awake(struct intel_gt *gt) +{ + return intel_wakeref_get_if_active(>wakeref); +} + void intel_gt_pm_init_early(struct intel_gt *gt); void intel_gt_sanitize(struct intel_gt *gt, bool force); diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c index eff86483bec0..e71192804996 100644 --- a/drivers/gpu/drm/i915/i915_pmu.c +++ b/drivers/gpu/drm/i915/i915_pmu.c @@ -8,6 +8,7 @@ #include #include "gt/intel_engine.h" +#include "gt/intel_gt_pm.h" #include "i915_drv.h" #include "i915_pmu.h" @@ -161,16 +162,12 @@ engines_sample(struct drm_i915_private *dev_priv, unsigned int period_ns) { struct intel_engine_cs *engine; enum intel_engine_id id; - intel_wakeref_t wakeref; unsigned long flags; if ((dev_priv->pmu.enable & ENGINE_SAMPLE_MASK) == 0) return; - wakeref = 0; - if (READ_ONCE(dev_priv->gt.awake)) - wakeref = intel_runtime_pm_get_if_in_use(_priv->runtime_pm); - if (!wakeref) + if (!intel_gt_pm_get_if_awake(_priv->gt)) return; spin_lock_irqsave(_priv->uncore.lock, flags); @@ -205,7 +202,7 @@ engines_sample(struct drm_i915_private *dev_priv, unsigned int period_ns) } spin_unlock_irqrestore(_priv->uncore.lock, flags); - intel_runtime_pm_put(_priv->runtime_pm, wakeref); + intel_gt_pm_put(_priv->gt); } static void @@ -222,15 +219,11 @@ frequency_sample(struct drm_i915_private *dev_priv, unsigned int period_ns) u32 val; val = dev_priv->gt_pm.rps.cur_freq; - if (dev_priv->gt.awake) { - intel_wakeref_t wakeref; - - with_intel_runtime_pm_if_in_use(_priv->runtime_pm, - wakeref) { - val = intel_uncore_read_notrace(_priv->uncore, - GEN6_RPSTAT1); - val = intel_get_cagf(dev_priv, val); - } + if (intel_gt_pm_get_if_awake(_priv->gt)) { + val = intel_uncore_read_notrace(_priv->uncore, + GEN6_RPSTAT1); + val = intel_get_cagf(dev_priv, val); + intel_gt_pm_put(_priv->gt); } add_sample_mult(_priv->pmu.sample[__I915_SAMPLE_FREQ_ACT], I guess I'll be rebasing mine, at some point. :) Reviewed-by: Tvrtko Ursulin Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/pmu: Atomically acquire the gt_pm wakeref
Currently, we only sample if the intel_gt is awake, but we acquire our own runtime_pm wakeref. Since intel_gt has transitioned to tracking its own wakeref, we can atomically test and acquire that wakeref instead. Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin --- No automagic drop! --- drivers/gpu/drm/i915/gt/intel_gt_pm.h | 8 +++- drivers/gpu/drm/i915/i915_pmu.c | 23 --- 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.h b/drivers/gpu/drm/i915/gt/intel_gt_pm.h index ba960e1fc209..016298483de7 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.h +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.h @@ -9,7 +9,8 @@ #include -struct intel_gt; +#include "intel_gt_types.h" +#include "intel_wakeref.h" enum { INTEL_GT_UNPARK, @@ -19,6 +20,11 @@ enum { void intel_gt_pm_get(struct intel_gt *gt); void intel_gt_pm_put(struct intel_gt *gt); +static inline bool intel_gt_pm_get_if_awake(struct intel_gt *gt) +{ + return intel_wakeref_get_if_active(>wakeref); +} + void intel_gt_pm_init_early(struct intel_gt *gt); void intel_gt_sanitize(struct intel_gt *gt, bool force); diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c index eff86483bec0..e71192804996 100644 --- a/drivers/gpu/drm/i915/i915_pmu.c +++ b/drivers/gpu/drm/i915/i915_pmu.c @@ -8,6 +8,7 @@ #include #include "gt/intel_engine.h" +#include "gt/intel_gt_pm.h" #include "i915_drv.h" #include "i915_pmu.h" @@ -161,16 +162,12 @@ engines_sample(struct drm_i915_private *dev_priv, unsigned int period_ns) { struct intel_engine_cs *engine; enum intel_engine_id id; - intel_wakeref_t wakeref; unsigned long flags; if ((dev_priv->pmu.enable & ENGINE_SAMPLE_MASK) == 0) return; - wakeref = 0; - if (READ_ONCE(dev_priv->gt.awake)) - wakeref = intel_runtime_pm_get_if_in_use(_priv->runtime_pm); - if (!wakeref) + if (!intel_gt_pm_get_if_awake(_priv->gt)) return; spin_lock_irqsave(_priv->uncore.lock, flags); @@ -205,7 +202,7 @@ engines_sample(struct drm_i915_private *dev_priv, unsigned int period_ns) } spin_unlock_irqrestore(_priv->uncore.lock, flags); - intel_runtime_pm_put(_priv->runtime_pm, wakeref); + intel_gt_pm_put(_priv->gt); } static void @@ -222,15 +219,11 @@ frequency_sample(struct drm_i915_private *dev_priv, unsigned int period_ns) u32 val; val = dev_priv->gt_pm.rps.cur_freq; - if (dev_priv->gt.awake) { - intel_wakeref_t wakeref; - - with_intel_runtime_pm_if_in_use(_priv->runtime_pm, - wakeref) { - val = intel_uncore_read_notrace(_priv->uncore, - GEN6_RPSTAT1); - val = intel_get_cagf(dev_priv, val); - } + if (intel_gt_pm_get_if_awake(_priv->gt)) { + val = intel_uncore_read_notrace(_priv->uncore, + GEN6_RPSTAT1); + val = intel_get_cagf(dev_priv, val); + intel_gt_pm_put(_priv->gt); } add_sample_mult(_priv->pmu.sample[__I915_SAMPLE_FREQ_ACT], -- 2.23.0.rc0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/pmu: Atomically acquire the gt_pm wakeref
Currently, we only sample if the intel_gt is awake, but we acquire our own runtime_pm wakeref. Since intel_gt has transitioned to tracking its own wakeref, we can atomically test and acquire that wakeref instead. Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin --- drivers/gpu/drm/i915/gt/intel_gt_pm.h | 8 +++- drivers/gpu/drm/i915/i915_pmu.c | 22 +++--- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.h b/drivers/gpu/drm/i915/gt/intel_gt_pm.h index ba960e1fc209..016298483de7 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.h +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.h @@ -9,7 +9,8 @@ #include -struct intel_gt; +#include "intel_gt_types.h" +#include "intel_wakeref.h" enum { INTEL_GT_UNPARK, @@ -19,6 +20,11 @@ enum { void intel_gt_pm_get(struct intel_gt *gt); void intel_gt_pm_put(struct intel_gt *gt); +static inline bool intel_gt_pm_get_if_awake(struct intel_gt *gt) +{ + return intel_wakeref_get_if_active(>wakeref); +} + void intel_gt_pm_init_early(struct intel_gt *gt); void intel_gt_sanitize(struct intel_gt *gt, bool force); diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c index eff86483bec0..a0278efe7b33 100644 --- a/drivers/gpu/drm/i915/i915_pmu.c +++ b/drivers/gpu/drm/i915/i915_pmu.c @@ -8,6 +8,7 @@ #include #include "gt/intel_engine.h" +#include "gt/intel_gt_pm.h" #include "i915_drv.h" #include "i915_pmu.h" @@ -161,16 +162,12 @@ engines_sample(struct drm_i915_private *dev_priv, unsigned int period_ns) { struct intel_engine_cs *engine; enum intel_engine_id id; - intel_wakeref_t wakeref; unsigned long flags; if ((dev_priv->pmu.enable & ENGINE_SAMPLE_MASK) == 0) return; - wakeref = 0; - if (READ_ONCE(dev_priv->gt.awake)) - wakeref = intel_runtime_pm_get_if_in_use(_priv->runtime_pm); - if (!wakeref) + if (!intel_gt_pm_get_if_awake(_priv->gt)) return; spin_lock_irqsave(_priv->uncore.lock, flags); @@ -205,7 +202,7 @@ engines_sample(struct drm_i915_private *dev_priv, unsigned int period_ns) } spin_unlock_irqrestore(_priv->uncore.lock, flags); - intel_runtime_pm_put(_priv->runtime_pm, wakeref); + intel_gt_pm_put(_priv->gt); } static void @@ -222,15 +219,10 @@ frequency_sample(struct drm_i915_private *dev_priv, unsigned int period_ns) u32 val; val = dev_priv->gt_pm.rps.cur_freq; - if (dev_priv->gt.awake) { - intel_wakeref_t wakeref; - - with_intel_runtime_pm_if_in_use(_priv->runtime_pm, - wakeref) { - val = intel_uncore_read_notrace(_priv->uncore, - GEN6_RPSTAT1); - val = intel_get_cagf(dev_priv, val); - } + if (intel_gt_pm_get_if_awake(_priv->gt)) { + val = intel_uncore_read_notrace(_priv->uncore, + GEN6_RPSTAT1); + val = intel_get_cagf(dev_priv, val); } add_sample_mult(_priv->pmu.sample[__I915_SAMPLE_FREQ_ACT], -- 2.23.0.rc0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx