On Fri, Oct 11, 2024 at 03:54:27PM -0700, Lucas De Marchi wrote: > Setting event_init to NULL is mostly done to detect when the driver is > partially working: i915 probed, but pmu is not registered. However, > checking for event_init is odd as it was supposed to always be set and > kernel/events/ would just crash if it found it set to NULL. > > Since there's already a "closed" boolean, use that instead and extend > it's meaning to unregistered/unregistering. > > Signed-off-by: Lucas De Marchi <[email protected]>
Reviewed-by: Matt Roper <[email protected]> > --- > drivers/gpu/drm/i915/i915_pmu.c | 19 ++++++------------- > 1 file changed, 6 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c > index 2e435f51867db..409e10d8190a8 100644 > --- a/drivers/gpu/drm/i915/i915_pmu.c > +++ b/drivers/gpu/drm/i915/i915_pmu.c > @@ -303,7 +303,7 @@ void i915_pmu_gt_parked(struct intel_gt *gt) > { > struct i915_pmu *pmu = >->i915->pmu; > > - if (!pmu->base.event_init) > + if (pmu->closed) > return; > > spin_lock_irq(&pmu->lock); > @@ -325,7 +325,7 @@ void i915_pmu_gt_unparked(struct intel_gt *gt) > { > struct i915_pmu *pmu = >->i915->pmu; > > - if (!pmu->base.event_init) > + if (pmu->closed) > return; > > spin_lock_irq(&pmu->lock); > @@ -1170,8 +1170,6 @@ static int i915_pmu_cpu_online(unsigned int cpu, struct > hlist_node *node) > { > struct i915_pmu *pmu = hlist_entry_safe(node, typeof(*pmu), cpuhp.node); > > - GEM_BUG_ON(!pmu->base.event_init); > - > /* Select the first online CPU as a designated reader. */ > if (cpumask_empty(&i915_pmu_cpumask)) > cpumask_set_cpu(cpu, &i915_pmu_cpumask); > @@ -1184,8 +1182,6 @@ static int i915_pmu_cpu_offline(unsigned int cpu, > struct hlist_node *node) > struct i915_pmu *pmu = hlist_entry_safe(node, typeof(*pmu), cpuhp.node); > unsigned int target = i915_pmu_target_cpu; > > - GEM_BUG_ON(!pmu->base.event_init); > - > /* > * Unregistering an instance generates a CPU offline event which we must > * ignore to avoid incorrectly modifying the shared i915_pmu_cpumask. > @@ -1258,9 +1254,10 @@ void i915_pmu_register(struct drm_i915_private *i915) > &i915_pmu_cpumask_attr_group, > NULL > }; > - > int ret = -ENOMEM; > > + pmu->closed = true; > + > if (GRAPHICS_VER(i915) <= 2) { > drm_info(&i915->drm, "PMU not supported for this GPU."); > return; > @@ -1319,6 +1316,8 @@ void i915_pmu_register(struct drm_i915_private *i915) > if (drmm_add_action(&i915->drm, free_pmu, pmu)) > goto err_unreg; > > + pmu->closed = false; > + > return; > > err_unreg: > @@ -1326,7 +1325,6 @@ void i915_pmu_register(struct drm_i915_private *i915) > err_groups: > kfree(pmu->base.attr_groups); > err_attr: > - pmu->base.event_init = NULL; > free_event_attributes(pmu); > err_name: > if (IS_DGFX(i915)) > @@ -1339,9 +1337,6 @@ void i915_pmu_unregister(struct drm_i915_private *i915) > { > struct i915_pmu *pmu = &i915->pmu; > > - if (!pmu->base.event_init) > - return; > - > /* > * "Disconnect" the PMU callbacks - since all are atomic synchronize_rcu > * ensures all currently executing ones will have exited before we > @@ -1354,6 +1349,4 @@ void i915_pmu_unregister(struct drm_i915_private *i915) > > i915_pmu_unregister_cpuhp_state(pmu); > perf_pmu_unregister(&pmu->base); > - > - pmu->base.event_init = NULL; > } > -- > 2.47.0 > -- Matt Roper Graphics Software Engineer Linux GPU Platform Enablement Intel Corporation
