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

[Zhenyu Wang]

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.

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/intel-gvt-dev/attachments/20200521/0fb1e8c2/attachment.sig>