[Intel-gfx] [PATCH v10 1/9] drm/i915: Create GEM runtime resume helper and handle GEM suspend/resume errors
Sagar Arun Kamble
sagar.a.kamble at intel.com
Wed Sep 27 17:02:14 UTC 2017
On 9/27/2017 9:11 PM, Michal Wajdeczko wrote:
> On Wed, 27 Sep 2017 11:30:31 +0200, Sagar Arun Kamble
> <sagar.a.kamble at intel.com> wrote:
>
>> These changes are preparation to handle GuC suspend/resume. Prepared
>> helper i915_gem_runtime_resume to reinitialize suspended gem setup.
>> Returning status from i915_gem_runtime_suspend and i915_gem_resume.
>> This will be placeholder for handling any errors from uC suspend/resume
>> in upcoming patches. Restructured the suspend/resume routines w.r.t
>> setup
>> creation and rollback order.
>> This also fixes issue of ordering of i915_gem_runtime_resume with
>> intel_runtime_pm_enable_interrupts.
>>
>> v2: Fixed return from intel_runtime_resume. (Michał Winiarski)
>>
>> v3: Not returning status from gem_runtime_resume. (Chris)
>>
>> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble at intel.com>
>> Cc: Chris Wilson <chris at chris-wilson.co.uk>
>> Cc: Imre Deak <imre.deak at intel.com>
>> Cc: Paulo Zanoni <paulo.r.zanoni at intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
>> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
>> Cc: Michał Winiarski <michal.winiarski at intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_drv.c | 22 +++++++++++++---------
>> drivers/gpu/drm/i915/i915_drv.h | 5 +++--
>> drivers/gpu/drm/i915/i915_gem.c | 18 ++++++++++++++++--
>> 3 files changed, 32 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c
>> b/drivers/gpu/drm/i915/i915_drv.c
>> index 7056bb2..d0a710d 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -1655,6 +1655,7 @@ static int i915_suspend_switcheroo(struct
>> drm_device *dev, pm_message_t state)
>> static int i915_drm_resume(struct drm_device *dev)
>> {
>> struct drm_i915_private *dev_priv = to_i915(dev);
>> + struct pci_dev *pdev = dev_priv->drm.pdev;
>> int ret;
>> disable_rpm_wakeref_asserts(dev_priv);
>> @@ -1666,7 +1667,9 @@ static int i915_drm_resume(struct drm_device *dev)
>> intel_csr_ucode_resume(dev_priv);
>> - i915_gem_resume(dev_priv);
>> + ret = i915_gem_resume(dev_priv);
>> + if (ret)
>> + dev_err(&pdev->dev, "GEM resume failed\n");
>
> btw, code above/below uses DRM_ERROR()
This was in symmetry with i915_gem_suspend return handling.
>
>> i915_restore_state(dev_priv);
>> intel_pps_unlock_regs_wa(dev_priv);
>> @@ -2495,7 +2498,11 @@ static int intel_runtime_suspend(struct device
>> *kdev)
>> * We are safe here against re-faults, since the fault handler
>> takes
>> * an RPM reference.
>> */
>> - i915_gem_runtime_suspend(dev_priv);
>> + ret = i915_gem_runtime_suspend(dev_priv);
>> + if (ret) {
>> + enable_rpm_wakeref_asserts(dev_priv);
>> + return ret;
>
> as this is second place where we exit, maybe it's time for
>
> if (ret)
> goto fail;
> fail:
> enable_rpm_wakeref_asserts(dev_priv);
> return ret;
> }
>
Yes. Will update.
>> + }
>> intel_guc_suspend(dev_priv);
>> @@ -2515,6 +2522,8 @@ static int intel_runtime_suspend(struct device
>> *kdev)
>> DRM_ERROR("Runtime suspend failed, disabling it (%d)\n", ret);
>> intel_runtime_pm_enable_interrupts(dev_priv);
>> + intel_guc_resume(dev_priv);
>> + i915_gem_runtime_resume(dev_priv);
>> enable_rpm_wakeref_asserts(dev_priv);
>> return ret;
>> @@ -2596,13 +2605,6 @@ static int intel_runtime_resume(struct device
>> *kdev)
>> ret = vlv_resume_prepare(dev_priv, true);
>> }
>> - /*
>> - * No point of rolling back things in case of an error, as the best
>> - * we can do is to hope that things will still work (and disable
>> RPM).
>> - */
>> - i915_gem_init_swizzling(dev_priv);
>> - i915_gem_restore_fences(dev_priv);
>> -
>> intel_runtime_pm_enable_interrupts(dev_priv);
>> /*
>> @@ -2615,6 +2617,8 @@ static int intel_runtime_resume(struct device
>> *kdev)
>> intel_enable_ipc(dev_priv);
>> + i915_gem_runtime_resume(dev_priv);
>> +
>> enable_rpm_wakeref_asserts(dev_priv);
>> if (ret)
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>> b/drivers/gpu/drm/i915/i915_drv.h
>> index b7cba89..42f3d89 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -3479,7 +3479,8 @@ struct i915_vma * __must_check
>> int i915_gem_object_unbind(struct drm_i915_gem_object *obj);
>> void i915_gem_release_mmap(struct drm_i915_gem_object *obj);
>> -void i915_gem_runtime_suspend(struct drm_i915_private *dev_priv);
>> +int i915_gem_runtime_suspend(struct drm_i915_private *dev_priv);
>> +void i915_gem_runtime_resume(struct drm_i915_private *dev_priv);
>> static inline int __sg_page_count(const struct scatterlist *sg)
>> {
>> @@ -3682,7 +3683,7 @@ void i915_gem_reset_engine(struct
>> intel_engine_cs *engine,
>> int i915_gem_wait_for_idle(struct drm_i915_private *dev_priv,
>> unsigned int flags);
>> int __must_check i915_gem_suspend(struct drm_i915_private *dev_priv);
>> -void i915_gem_resume(struct drm_i915_private *dev_priv);
>> +int i915_gem_resume(struct drm_i915_private *dev_priv);
>> int i915_gem_fault(struct vm_fault *vmf);
>> int i915_gem_object_wait(struct drm_i915_gem_object *obj,
>> unsigned int flags,
>> diff --git a/drivers/gpu/drm/i915/i915_gem.c
>> b/drivers/gpu/drm/i915/i915_gem.c
>> index 73eeb6b..59a88f2 100644
>> --- a/drivers/gpu/drm/i915/i915_gem.c
>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>> @@ -2022,7 +2022,7 @@ int i915_gem_fault(struct vm_fault *vmf)
>> intel_runtime_pm_put(i915);
>> }
>> -void i915_gem_runtime_suspend(struct drm_i915_private *dev_priv)
>> +int i915_gem_runtime_suspend(struct drm_i915_private *dev_priv)
>> {
>> struct drm_i915_gem_object *obj, *on;
>> int i;
>> @@ -2065,6 +2065,18 @@ void i915_gem_runtime_suspend(struct
>> drm_i915_private *dev_priv)
>> GEM_BUG_ON(!list_empty(®->vma->obj->userfault_link));
>> reg->dirty = true;
>> }
>> +
>> + return 0;
>> +}
>> +
>> +void i915_gem_runtime_resume(struct drm_i915_private *dev_priv)
>> +{
>> + /*
>> + * No point of rolling back things in case of an error, as the best
>> + * we can do is to hope that things will still work (and disable
>> RPM).
>> + */
>> + i915_gem_init_swizzling(dev_priv);
>> + i915_gem_restore_fences(dev_priv);
>> }
>> static int i915_gem_object_create_mmap_offset(struct
>> drm_i915_gem_object *obj)
>> @@ -4587,7 +4599,7 @@ int i915_gem_suspend(struct drm_i915_private
>> *dev_priv)
>> return ret;
>> }
>> -void i915_gem_resume(struct drm_i915_private *dev_priv)
>> +int i915_gem_resume(struct drm_i915_private *dev_priv)
>> {
>> struct drm_device *dev = &dev_priv->drm;
>> @@ -4603,6 +4615,8 @@ void i915_gem_resume(struct drm_i915_private
>> *dev_priv)
>> dev_priv->gt.resume(dev_priv);
>> mutex_unlock(&dev->struct_mutex);
>> +
>> + return 0;
>> }
>> void i915_gem_init_swizzling(struct drm_i915_private *dev_priv)
More information about the Intel-gfx
mailing list