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@xxxxxxxxx>
> >
> > 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@xxxxxxxxx>
> > ---
> > 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
> >
