On Tue, 21 Jun 2022, "Tangudu, Tilak" <tilak.tangudu@xxxxxxxxx> wrote:
>> -----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 ?
One option would be to track this in intel_wakeref_t, i.e. _get flags
the case in the returned wakeref and _put skips in that case.
BR,
Jani.
>
>> >
>> > 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
>>
>
--
Jani Nikula, Intel Open Source Graphics Center
