| Message ID | 20220721095955.3986943-6-tilak.tangudu@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Add D3Cold-Off support for runtime-pm | expand |
On Thu, Jul 21, 2022 at 03:29:52PM +0530, tilak.tangudu@intel.com wrote: > From: Anshuman Gupta <anshuman.gupta@intel.com> > > Adding intel_runtime_idle (runtime_idle callback) to prepare the > tageted D3 state. > > Since we have introduced i915 runtime_idle callback. > It need to be warranted that Runtime PM Core invokes runtime_idle > callback when runtime usages count becomes zero. That requires > to use pm_runtime_put instead of pm_runtime_put_autosuspend. > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Cc: Chris Wilson <chris.p.wilson@intel.com> > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com> > --- > drivers/gpu/drm/i915/i915_driver.c | 12 ++++++++++++ > drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +-- > 2 files changed, 13 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c > index deb8a8b76965..4c36554567fd 100644 > --- a/drivers/gpu/drm/i915/i915_driver.c > +++ b/drivers/gpu/drm/i915/i915_driver.c > @@ -1576,6 +1576,17 @@ static int i915_pm_restore(struct device *kdev) > return i915_pm_resume(kdev); > } > > +static int intel_runtime_idle(struct device *kdev) > +{ > + struct drm_i915_private *i915 = kdev_to_i915(kdev); > + int ret = 1; > + > + pm_runtime_mark_last_busy(kdev); > + pm_runtime_autosuspend(kdev); oh, I see the ret = 1 like the other drm drivers.. do we really know why this flow and not use the runtime_idle like the rest of the kernel? I believe we could use more the runtime_idle to block the runtime_pm, but following the original documented design of it. > + > + return ret; > +} > + > static int intel_runtime_suspend(struct device *kdev) > { > struct drm_i915_private *dev_priv = kdev_to_i915(kdev); > @@ -1752,6 +1763,7 @@ const struct dev_pm_ops i915_pm_ops = { > .restore = i915_pm_restore, > > /* S0ix (via runtime suspend) event handlers */ > + .runtime_idle = intel_runtime_idle, > .runtime_suspend = intel_runtime_suspend, > .runtime_resume = intel_runtime_resume, > }; > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c > index 704beeeb560b..1c3ed0c29330 100644 > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > @@ -513,8 +513,7 @@ static void __intel_runtime_pm_put(struct intel_runtime_pm *rpm, > > intel_runtime_pm_release(rpm, wakelock); > > - pm_runtime_mark_last_busy(kdev); > - pm_runtime_put_autosuspend(kdev); > + pm_runtime_put(kdev); > } > > /** > -- > 2.25.1 >
> -----Original Message----- > From: Vivi, Rodrigo <rodrigo.vivi@intel.com> > Sent: Thursday, August 4, 2022 2:22 AM > To: Tangudu, Tilak <tilak.tangudu@intel.com> > Cc: Ewins, Jon <jon.ewins@intel.com>; Belgaumkar, Vinay > <vinay.belgaumkar@intel.com>; Roper, Matthew D > <matthew.d.roper@intel.com>; Wilson, Chris P <chris.p.wilson@intel.com>; > Nikula, Jani <jani.nikula@intel.com>; Gupta, saurabhg > <saurabhg.gupta@intel.com>; Gupta, Anshuman > <anshuman.gupta@intel.com>; Nilawar, Badal <badal.nilawar@intel.com>; > Deak, Imre <imre.deak@intel.com>; Iddamsetty, Aravind > <aravind.iddamsetty@intel.com>; intel-gfx@lists.freedesktop.org > Subject: Re: [PATCH 5/8] Drm/i915/rpm: Add intel_runtime_idle > > On Thu, Jul 21, 2022 at 03:29:52PM +0530, tilak.tangudu@intel.com wrote: > > From: Anshuman Gupta <anshuman.gupta@intel.com> > > > > Adding intel_runtime_idle (runtime_idle callback) to prepare the > > tageted D3 state. > > > > Since we have introduced i915 runtime_idle callback. > > It need to be warranted that Runtime PM Core invokes runtime_idle > > callback when runtime usages count becomes zero. That requires to use > > pm_runtime_put instead of pm_runtime_put_autosuspend. > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > > Cc: Chris Wilson <chris.p.wilson@intel.com> > > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com> > > --- > > drivers/gpu/drm/i915/i915_driver.c | 12 ++++++++++++ > > drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +-- > > 2 files changed, 13 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_driver.c > > b/drivers/gpu/drm/i915/i915_driver.c > > index deb8a8b76965..4c36554567fd 100644 > > --- a/drivers/gpu/drm/i915/i915_driver.c > > +++ b/drivers/gpu/drm/i915/i915_driver.c > > @@ -1576,6 +1576,17 @@ static int i915_pm_restore(struct device *kdev) > > return i915_pm_resume(kdev); > > } > > > > +static int intel_runtime_idle(struct device *kdev) { > > + struct drm_i915_private *i915 = kdev_to_i915(kdev); > > + int ret = 1; > > + > > + pm_runtime_mark_last_busy(kdev); > > + pm_runtime_autosuspend(kdev); > > oh, I see the ret = 1 like the other drm drivers.. > do we really know why this flow and not use the runtime_idle like the rest of the > kernel? AFAIU it is the mixed type of usages (some drivers like USB use -EBUSY), the crux of it to use the pm_runtime_mark_last_busy() and pm_runtime_autosuspend correctly , PM Core autosuspend timer start ticking since it was last busy. int usb_runtime_idle(struct device *dev) { struct usb_device *udev = to_usb_device(dev); /* An idle USB device can be suspended if it passes the various * autosuspend checks. */ if (autosuspend_check(udev) == 0) pm_runtime_autosuspend(dev); /* Tell the core not to suspend it, though. */ return -EBUSY; } Intel snd driver uses -EBUSY and 0 as a return value. static int azx_runtime_idle(struct device *dev) { struct snd_card *card = dev_get_drvdata(dev); struct azx *chip; struct hda_intel *hda; if (!card) return 0; chip = card->private_data; hda = container_of(chip, struct hda_intel, chip); if (chip->disabled || hda->init_failed) return 0; if (!power_save_controller || !azx_has_pm_runtime(chip) || azx_bus(chip)->codec_powered || !chip->running) return -EBUSY; /* ELD notification gets broken when HD-audio bus is off */ if (needs_eld_notify_link(chip)) return -EBUSY; return 0; } When rutime_idle callback returns 0, then only PM core will call rpm_suspend with RPM_AUTO flag. return retval ? retval : rpm_suspend(dev, rpmflags | RPM_AUTO); > > I believe we could use more the runtime_idle to block the runtime_pm, but > following the original documented design of it. Do you mean it is not correct to use runtime_idle callback for d3cold policy ? Yes it is being used to block the runtime pm, if device is busy for other kernel drivers. Thanks, Anshuman Gupta. > > > + > > + return ret; > > +} > > + > > static int intel_runtime_suspend(struct device *kdev) { > > struct drm_i915_private *dev_priv = kdev_to_i915(kdev); @@ -1752,6 > > +1763,7 @@ const struct dev_pm_ops i915_pm_ops = { > > .restore = i915_pm_restore, > > > > /* S0ix (via runtime suspend) event handlers */ > > + .runtime_idle = intel_runtime_idle, > > .runtime_suspend = intel_runtime_suspend, > > .runtime_resume = intel_runtime_resume, }; diff --git > > a/drivers/gpu/drm/i915/intel_runtime_pm.c > > b/drivers/gpu/drm/i915/intel_runtime_pm.c > > index 704beeeb560b..1c3ed0c29330 100644 > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > > @@ -513,8 +513,7 @@ static void __intel_runtime_pm_put(struct > > intel_runtime_pm *rpm, > > > > intel_runtime_pm_release(rpm, wakelock); > > > > - pm_runtime_mark_last_busy(kdev); > > - pm_runtime_put_autosuspend(kdev); > > + pm_runtime_put(kdev); > > } > > > > /** > > -- > > 2.25.1 > >
diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c index deb8a8b76965..4c36554567fd 100644 --- a/drivers/gpu/drm/i915/i915_driver.c +++ b/drivers/gpu/drm/i915/i915_driver.c @@ -1576,6 +1576,17 @@ static int i915_pm_restore(struct device *kdev) return i915_pm_resume(kdev); } +static int intel_runtime_idle(struct device *kdev) +{ + struct drm_i915_private *i915 = kdev_to_i915(kdev); + int ret = 1; + + pm_runtime_mark_last_busy(kdev); + pm_runtime_autosuspend(kdev); + + return ret; +} + static int intel_runtime_suspend(struct device *kdev) { struct drm_i915_private *dev_priv = kdev_to_i915(kdev); @@ -1752,6 +1763,7 @@ const struct dev_pm_ops i915_pm_ops = { .restore = i915_pm_restore, /* S0ix (via runtime suspend) event handlers */ + .runtime_idle = intel_runtime_idle, .runtime_suspend = intel_runtime_suspend, .runtime_resume = intel_runtime_resume, }; diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c index 704beeeb560b..1c3ed0c29330 100644 --- a/drivers/gpu/drm/i915/intel_runtime_pm.c +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c @@ -513,8 +513,7 @@ static void __intel_runtime_pm_put(struct intel_runtime_pm *rpm, intel_runtime_pm_release(rpm, wakelock); - pm_runtime_mark_last_busy(kdev); - pm_runtime_put_autosuspend(kdev); + pm_runtime_put(kdev); } /**