[PATCH v6 2/6] drm/i915/pxp: add device link between i915 and mei_pxp

[Teres Alexis, Alan Previn]

Thanks Rodrigo - will do.

On Tue, 2023-01-24 at 10:10 -0500, Vivi, Rodrigo wrote:
> On Mon, Jan 23, 2023 at 09:31:46PM -0800, Alan Previn wrote:
> > From: Alexander Usyskin <alexander.usyskin at intel.com>
> > 
> > Add device link with i915 as consumer and mei_pxp as supplier
> > to ensure proper ordering of power flows.
> > 
> > V2: condition on absence of heci_pxp to filter out DG
> > 
> > Signed-off-by: Alexander Usyskin <alexander.usyskin at intel.com>
> > ---
> >  drivers/gpu/drm/i915/pxp/intel_pxp_tee.c   | 11 +++++++++++
> >  drivers/gpu/drm/i915/pxp/intel_pxp_types.h |  6 ++++++
> >  2 files changed, 17 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
> > index 73aa8015f828..cd5b86216506 100644
> > --- a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
> > +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
> > @@ -127,6 +127,12 @@ static int i915_pxp_tee_component_bind(struct device *i915_kdev,
> >         intel_wakeref_t wakeref;
> >         int ret = 0;
> >  
> > +       if (!HAS_HECI_PXP(i915)) {
> > +               pxp->component_dev_link = device_link_add(i915_kdev, tee_kdev, DL_FLAG_STATELESS);
> > +               if (drm_WARN_ON(&i915->drm, !pxp->component_dev_link))
> > +                       return -ENODEV;
> > +       }
> > +
> >         mutex_lock(&pxp->tee_mutex);
> >         pxp->pxp_component = data;
> >         pxp->pxp_component->tee_dev = tee_kdev;
> > @@ -169,6 +175,11 @@ static void i915_pxp_tee_component_unbind(struct device *i915_kdev,
> >         mutex_lock(&pxp->tee_mutex);
> >         pxp->pxp_component = NULL;
> >         mutex_unlock(&pxp->tee_mutex);
> > +
> > +       if (pxp->component_dev_link) {
> > +               device_link_remove(i915_kdev, tee_kdev);
> > +               pxp->component_dev_link = NULL;
> > +       }
> 
> use the 'del' version instead of the 'remove' one.
> 
> if (pxp->dev_link)
>         device_link_del(pxp->dev_link);
> 
> >  }
> >  
> >  static const struct component_ops i915_pxp_tee_component_ops = {
> > diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_types.h b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h
> > index 7dc5f08d1583..efd2f3915abe 100644
> > --- a/drivers/gpu/drm/i915/pxp/intel_pxp_types.h
> > +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h
> > @@ -32,6 +32,12 @@ struct intel_pxp {
> >          * which are protected by &tee_mutex.
> >          */
> >         struct i915_pxp_component *pxp_component;
> > +
> > +       /**
> > +        * @component_dev_link: device link of the pxp-component enforcing i915 as the
> > +        * consumer. This is needed for legacy platform (TGL/ADL) full-feature usage.
> 
> No need to add platform information here.
> 
> What about something shorter:
> 
> /* @dev_link: Enforce module relationship for power management ordering. */
> 
> > +        */
> > +       struct device_link *component_dev_link;
> 
> What about
>      struct device_link *dev_link;
> 
> 'component' is already part of the pxp struct.
> 
> >         /**
> >          * @pxp_component_added: track if the pxp component has been added.
> >          * Set and cleared in tee init and fini functions respectively.
> > -- 
> > 2.39.0
> >