| Message ID | 20230124053150.3543928-3-alan.previn.teres.alexis@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/pxp: Add missing cleanup steps for PXP global-teardown | expand |
On Mon, Jan 23, 2023 at 09:31:46PM -0800, Alan Previn wrote: > From: Alexander Usyskin <alexander.usyskin@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@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 >
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@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@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 > >
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; + } } 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. + */ + struct device_link *component_dev_link; /** * @pxp_component_added: track if the pxp component has been added. * Set and cleared in tee init and fini functions respectively.