> -----Original Message----- > From: Gupta, Anshuman <anshuman.gupta@xxxxxxxxx> > Sent: Tuesday, June 21, 2022 7:47 PM > To: Tangudu, Tilak <tilak.tangudu@xxxxxxxxx>; intel-gfx@xxxxxxxxxxxxxxxxxxxxx; > Ewins, Jon <jon.ewins@xxxxxxxxx>; Vivi, Rodrigo <rodrigo.vivi@xxxxxxxxx>; > Belgaumkar, Vinay <vinay.belgaumkar@xxxxxxxxx>; Wilson, Chris P > <chris.p.wilson@xxxxxxxxx>; Dixit, Ashutosh <ashutosh.dixit@xxxxxxxxx>; > Nilawar, Badal <badal.nilawar@xxxxxxxxx>; Roper, Matthew D > <matthew.d.roper@xxxxxxxxx>; Gupta, saurabhg > <saurabhg.gupta@xxxxxxxxx>; Iddamsetty, Aravind > <aravind.iddamsetty@xxxxxxxxx>; Sundaresan, Sujaritha > <sujaritha.sundaresan@xxxxxxxxx> > Subject: RE: [PATCH 04/11] drm/i915: Added is_intel_rpm_allowed helper > > > > > -----Original Message----- > > From: Tangudu, Tilak <tilak.tangudu@xxxxxxxxx> > > Sent: Tuesday, June 21, 2022 6:05 PM > > To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Ewins, Jon <jon.ewins@xxxxxxxxx>; > > Vivi, Rodrigo <rodrigo.vivi@xxxxxxxxx>; Belgaumkar, Vinay > > <vinay.belgaumkar@xxxxxxxxx>; Wilson, Chris P > > <chris.p.wilson@xxxxxxxxx>; Dixit, Ashutosh > > <ashutosh.dixit@xxxxxxxxx>; Nilawar, Badal <badal.nilawar@xxxxxxxxx>; > > Gupta, Anshuman <anshuman.gupta@xxxxxxxxx>; Tangudu, Tilak > > <tilak.tangudu@xxxxxxxxx>; Roper, Matthew D > > <matthew.d.roper@xxxxxxxxx>; Gupta, saurabhg > > <saurabhg.gupta@xxxxxxxxx>; Iddamsetty, Aravind > > <aravind.iddamsetty@xxxxxxxxx>; Sundaresan, Sujaritha > > <sujaritha.sundaresan@xxxxxxxxx> > > Subject: [PATCH 04/11] drm/i915: Added is_intel_rpm_allowed helper > > > > Added is_intel_rpm_allowed function to query the runtime_pm status and > > disllow during suspending and resuming. > This seems a hack, > Not sure if we have better way to handle it. > May be check this in intel_pm_runtime_{get,put} to keep entire code simple ? Yes, that would be simple without code refactoring. Checked the same with Chris, he suggested unbalancing of wakeref might popup If used at intel_pm_runtime_{get,put} . So used like this, @Wilson, Chris P , Please comment . @Vivi, Rodrigo , Any suggestion ? > > > > Signed-off-by: Tilak Tangudu <tilak.tangudu@xxxxxxxxx> > > --- > > drivers/gpu/drm/i915/intel_runtime_pm.c | 15 +++++++++++++++ > > drivers/gpu/drm/i915/intel_runtime_pm.h | 1 + > > 2 files changed, 16 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c > > b/drivers/gpu/drm/i915/intel_runtime_pm.c > > index 6ed5786bcd29..3759a8596084 100644 > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > > @@ -320,6 +320,21 @@ untrack_all_intel_runtime_pm_wakerefs(struct > > intel_runtime_pm *rpm) } > > > > #endif > > +static int intel_runtime_pm_status(struct intel_runtime_pm *rpm) { > > +return rpm->kdev->power.runtime_status; } > This is racy in principal, we need a kdev->power lock here. > Regards, > Anshuman Gupta. > > + > > +bool is_intel_rpm_allowed(struct intel_runtime_pm *rpm) { int > > +rpm_status; > > + > > +rpm_status = intel_runtime_pm_status(rpm); if (rpm_status == > > +RPM_RESUMING || rpm_status == > > RPM_SUSPENDING) > > +return false; > > +else > > +return true; > > +} > > > > static void > > intel_runtime_pm_acquire(struct intel_runtime_pm *rpm, bool wakelock) > > diff -- git a/drivers/gpu/drm/i915/intel_runtime_pm.h > > b/drivers/gpu/drm/i915/intel_runtime_pm.h > > index d9160e3ff4af..99418c3a934a 100644 > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.h > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.h > > @@ -173,6 +173,7 @@ void intel_runtime_pm_init_early(struct > > intel_runtime_pm *rpm); void intel_runtime_pm_enable(struct > > intel_runtime_pm *rpm); void intel_runtime_pm_disable(struct > > intel_runtime_pm *rpm); void intel_runtime_pm_driver_release(struct > > intel_runtime_pm *rpm); > > +bool is_intel_rpm_allowed(struct intel_runtime_pm *rpm); > > > > intel_wakeref_t intel_runtime_pm_get(struct intel_runtime_pm *rpm); > > intel_wakeref_t intel_runtime_pm_get_if_in_use(struct intel_runtime_pm > > *rpm); > > -- > > 2.25.1 >