[Intel-gfx] [PATCH 3/6] drm/i915: add SNB runtime PM support
Paulo Zanoni
przanoni at gmail.com
Tue Apr 1 23:17:07 CEST 2014
2014-04-01 12:34 GMT-03:00 Imre Deak <imre.deak at intel.com>:
> On Fri, 2014-03-07 at 20:12 -0300, Paulo Zanoni wrote:
>> From: Paulo Zanoni <paulo.r.zanoni at intel.com>
>>
>> Just because I have a SNB machine and I can easily test it.
>>
>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_drv.c | 27 +++++++++++++++++++++++++--
>> drivers/gpu/drm/i915/i915_drv.h | 2 +-
>> drivers/gpu/drm/i915/intel_display.c | 7 +++++++
>> 3 files changed, 33 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>> index f75d776..2c62e0c 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -832,6 +832,13 @@ static int i915_pm_poweroff(struct device *dev)
>> return i915_drm_freeze(drm_dev);
>> }
>>
>> +static void snb_runtime_suspend(struct drm_i915_private *dev_priv)
>> +{
>> + struct drm_device *dev = dev_priv->dev;
>> +
>> + intel_runtime_pm_disable_interrupts(dev);
>> +}
>
> Afaics RPS is still enabled here, so if we need it enabled at this point
> (so that GT stays at RC6 for example) we'd at least need here
>
> cancel_delayed_work_sync(&dev_priv->rps.delayed_resume_work);
> cancel_work_sync(&dev_priv->rps.work);
Congratulations, you've found the only runtime PM bug I can still
reproduce on my local tree. But this problem also happens for HSW, not
just SNB, so IMHO its fix should go on a separate patch. And it should
go to -fixes.
>
>> +
>> static void hsw_runtime_suspend(struct drm_i915_private *dev_priv)
>> {
>> struct drm_device *dev = dev_priv->dev;
>> @@ -840,6 +847,18 @@ static void hsw_runtime_suspend(struct drm_i915_private *dev_priv)
>> hsw_enable_pc8(dev_priv);
>> }
>>
>> +static void snb_runtime_resume(struct drm_i915_private *dev_priv)
>> +{
>> + struct drm_device *dev = dev_priv->dev;
>> +
>> + intel_runtime_pm_restore_interrupts(dev);
>> + intel_init_pch_refclk(dev);
>> + i915_gem_init_swizzling(dev);
>> + mutex_lock(&dev_priv->rps.hw_lock);
>> + gen6_update_ring_freq(dev);
>> + mutex_unlock(&dev_priv->rps.hw_lock);
>> +}
>
> I haven't found any info on what exact state is lost in D3, but in any
> case documenting this in the code would be great.
Unfortunately we don't have this documentation, and that's why we have
so many different tests on the pm_pc8 test suite. This function is
just based on the HSW code, and I am kinda trusting the test suite and
my manual tests.
>
> Here there are some bits restored for GTI in i915_gem_init_swizzling(),
> but what about the clock gating registers and RPS registers. I'd be
> easier if those would be also re-inited here, or a confirmation that
> they are saved/restored by HW.
The clock gating registers were manually checked a looong time ago on
HSW, and I didn't really remember to check them on SNB. We even
discussed on the mailing list how to do this check, and I proposed an
implementation a long time ago:
http://lists.freedesktop.org/archives/intel-gfx/2013-November/036351.html
, but I never wrote this code.
Also, if you have ideas of IGT tests that could catch these possible
problems you mentioned, please feel free to contribute! Either by
writing code or by giving me a description of the code to be written
:)
I'll move this SNB patch to the end of the series.
Thanks,
Paulo
>
> --Imre
>
>> +
>> static void hsw_runtime_resume(struct drm_i915_private *dev_priv)
>> {
>> struct drm_device *dev = dev_priv->dev;
>> @@ -859,7 +878,9 @@ static int intel_runtime_suspend(struct device *device)
>>
>> DRM_DEBUG_KMS("Suspending device\n");
>>
>> - if (IS_HASWELL(dev))
>> + if (IS_GEN6(dev))
>> + snb_runtime_suspend(dev_priv);
>> + else if (IS_HASWELL(dev))
>> hsw_runtime_suspend(dev_priv);
>>
>> i915_gem_release_all_mmaps(dev_priv);
>> @@ -893,7 +914,9 @@ static int intel_runtime_resume(struct device *device)
>> intel_opregion_notify_adapter(dev, PCI_D0);
>> dev_priv->pm.suspended = false;
>>
>> - if (IS_HASWELL(dev))
>> + if (IS_GEN6(dev))
>> + snb_runtime_resume(dev_priv);
>> + else if (IS_HASWELL(dev))
>> hsw_runtime_resume(dev_priv);
>>
>> DRM_DEBUG_KMS("Device resumed\n");
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 493579d..309852d 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1998,7 +1998,7 @@ struct drm_i915_cmd_table {
>> #define HAS_FPGA_DBG_UNCLAIMED(dev) (INTEL_INFO(dev)->has_fpga_dbg)
>> #define HAS_PSR(dev) (IS_HASWELL(dev) || IS_BROADWELL(dev))
>> #define HAS_PC8(dev) (IS_HASWELL(dev)) /* XXX HSW:ULX */
>> -#define HAS_RUNTIME_PM(dev) (IS_HASWELL(dev))
>> +#define HAS_RUNTIME_PM(dev) (IS_GEN6(dev) || IS_HASWELL(dev))
>>
>> #define INTEL_PCH_DEVICE_ID_MASK 0xff00
>> #define INTEL_PCH_IBX_DEVICE_ID_TYPE 0x3b00
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index df69866..35f65c1 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -6859,6 +6859,11 @@ void hsw_disable_pc8(struct drm_i915_private *dev_priv)
>> mutex_unlock(&dev_priv->rps.hw_lock);
>> }
>>
>> +static void snb_modeset_global_resources(struct drm_device *dev)
>> +{
>> + modeset_update_crtc_power_domains(dev);
>> +}
>> +
>> static void haswell_modeset_global_resources(struct drm_device *dev)
>> {
>> modeset_update_crtc_power_domains(dev);
>> @@ -10758,6 +10763,8 @@ static void intel_init_display(struct drm_device *dev)
>> } else if (IS_GEN6(dev)) {
>> dev_priv->display.fdi_link_train = gen6_fdi_link_train;
>> dev_priv->display.write_eld = ironlake_write_eld;
>> + dev_priv->display.modeset_global_resources =
>> + snb_modeset_global_resources;
>> } else if (IS_IVYBRIDGE(dev)) {
>> /* FIXME: detect B0+ stepping and use auto training */
>> dev_priv->display.fdi_link_train = ivb_manual_fdi_link_train;
>
--
Paulo Zanoni
More information about the Intel-gfx
mailing list