[PATCH 2/2] drm/i915/gvt: Add GVT suspend/resume routine to i915.

[Colin Xu]

On 2020-05-21 11:51, Zhenyu Wang wrote:
> On 2020.05.21 08:50:32 +0800, Colin Xu wrote:
>> On 2020-05-21 08:37, Colin Xu wrote:
>>> On 2020-05-18 18:45, Zhenyu Wang wrote:
>>>> On 2020.05.18 14:28:52 +0800, Colin Xu wrote:
>>>>> Add GVT suspend/resume routine to i915.
>>>>> GVT relies on i915 so suspend ahead of other i915 routine, and resume
>>>>> at last vice versa.
>>>>>
>>>>> Signed-off-by: Hang Yuan <hang.yuan at linux.intel.com>
>>>>> Signed-off-by: Colin Xu <colin.xu at intel.com>
>>>>> ---
>>>>> ?? drivers/gpu/drm/i915/i915_drv.c | 15 +++++++++++++++
>>>>> ?? 1 file changed, 15 insertions(+)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.c
>>>>> b/drivers/gpu/drm/i915/i915_drv.c
>>>>> index 34ee12f3f02d..c5d7d58df67e 100644
>>>>> --- a/drivers/gpu/drm/i915/i915_drv.c
>>>>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>>>>> @@ -87,6 +87,10 @@
>>>>> ?? #include "intel_pm.h"
>>>>> ?? #include "vlv_suspend.h"
>>>>> ?? +#if IS_ENABLED(CONFIG_DRM_I915_GVT)
>>>>> +#include "gvt.h"
>>>>> +#endif
>>>>> +
>>>>> ?? static struct drm_driver driver;
>>>>> ?? ?? static int i915_get_bridge_dev(struct drm_i915_private *dev_priv)
>>>>> @@ -1177,6 +1181,11 @@ static int i915_drm_suspend(struct
>>>>> drm_device *dev)
>>>>> ?? ?????????? drm_kms_helper_poll_disable(dev);
>>>>> ?? +#if IS_ENABLED(CONFIG_DRM_I915_GVT)
>>>>> +?????? if (dev_priv->gvt)
>>>>> +?????????????? intel_gvt_pm_suspend(dev_priv->gvt);
>>>>> +#endif
>>>>> +
>>>>> ?????????? pci_save_state(pdev);
>>>>> ?? ?????????? intel_display_suspend(dev);
>>>>> @@ -1354,6 +1363,12 @@ static int i915_drm_resume(struct
>>>>> drm_device *dev)
>>>>> ?? ?????????? intel_power_domains_enable(dev_priv);
>>>>> ?? +#if IS_ENABLED(CONFIG_DRM_I915_GVT)
>>>>> +?????? if (dev_priv->gvt) {
>>>> intel_gvt_active()
>>> Almost forgot we have this function can use. Thanks.
>>>>> +?????????????? return intel_gvt_pm_resume(dev_priv->gvt);
>>>>> +?????? }
>>>>> +#endif
>>>>> +
>>>>> enable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
>>>>>
>>>> Just wrap this in intel_gvt.c/.h which is i915 gvt host side caller,
>>>> then
>>>> i915 code just call intel_gvt_pm_suspend/resume().
>>> It makes the integration more clean.
>>>
>>> I'll move the code and update in V2 and include intel-gfx. Thanks.
>> BTW. Is it necessary to implement pm suspend/resume?? body in gvt.c instead
>> of intel_gvt?
>>
>> i915_drv.c -> intel_gvt.c -> gvt.c
>>
>> or
>>
>> i915_drv.c ->intel_gvt.c
>>
>> Which is better?
>>
>> Current intel_gvt.c looks only a wrapper code while all implementation
>> actually in gvt.c.
>>
> Yeah, we want to keep GVT device model in gvt directory as much as possible,
> and intel_gvt.c provides interface required to be used by i915. So first option
> is better, e.g you have gvt_pm_suspend/resume() function in gvt/gvt.c and
> intel_gvt.c will provide a wrapper to call them by i915 pm.
Got it. Will update and include intel-gfx in V2.

-- 
Best Regards,
Colin Xu