On Wed, Nov 16, 2022 at 04:30:16PM -0800, Alan Previn wrote:
> Ensure i915_pxp_tee_component_bind / unbind implicitly sorts out
> getting the correct per-GT PXP control-context from the PXP-owning-GT
> when establishing or ending connection. Thus, replace _i915_to_pxp_gt
> with intel_pxp_get_owning_gt (also takes in i915).
>
> Signed-off-by: Alan Previn
> Reviewed-by: Daniele Ceraolo Spurio
> ---
> drivers/gpu/drm/i915/pxp/intel_pxp.c | 6 +++---
> drivers/gpu/drm/i915/pxp/intel_pxp.h | 2 ++
> drivers/gpu/drm/i915/pxp/intel_pxp_tee.c | 14 --
> 3 files changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.c
> b/drivers/gpu/drm/i915/pxp/intel_pxp.c
> index 76a924587543..6a78b6ef0235 100644
> --- a/drivers/gpu/drm/i915/pxp/intel_pxp.c
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.c
> @@ -64,7 +64,7 @@ bool intel_pxp_is_enabled_on_gt(const struct intel_pxp *pxp)
> return pxp->ce;
> }
>
> -static struct intel_gt *i915_to_pxp_gt(struct drm_i915_private *i915)
> +struct intel_gt *intel_pxp_get_owning_gt(struct drm_i915_private *i915)
> {
> struct intel_gt *gt = NULL;
> int i = 0;
> @@ -79,7 +79,7 @@ static struct intel_gt *i915_to_pxp_gt(struct
> drm_i915_private *i915)
>
> bool intel_pxp_is_enabled(struct drm_i915_private *i915)
> {
> - struct intel_gt *gt = i915_to_pxp_gt(i915);
> + struct intel_gt *gt = intel_pxp_get_owning_gt(i915);
>
> if (!gt)
> return false;
> @@ -94,7 +94,7 @@ bool intel_pxp_is_active_on_gt(const struct intel_pxp *pxp)
>
> bool intel_pxp_is_active(struct drm_i915_private *i915)
> {
> - struct intel_gt *gt = i915_to_pxp_gt(i915);
> + struct intel_gt *gt = intel_pxp_get_owning_gt(i915);
>
> if (!gt)
> return false;
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.h
> b/drivers/gpu/drm/i915/pxp/intel_pxp.h
> index fe981eebf0ec..c798c3bde957 100644
> --- a/drivers/gpu/drm/i915/pxp/intel_pxp.h
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.h
> @@ -13,6 +13,8 @@ struct intel_pxp;
> struct drm_i915_gem_object;
> struct drm_i915_private;
>
> +struct intel_gt *intel_pxp_get_owning_gt(struct drm_i915_private *i915);
> +
> struct intel_gt *pxp_to_gt(const struct intel_pxp *pxp);
>
> bool intel_pxp_supported_on_gt(const struct intel_pxp *pxp);
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
> b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
> index a5c9c692c20d..b9198e961cb6 100644
> --- a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
> @@ -20,8 +20,12 @@
> static inline struct intel_pxp *i915_dev_to_pxp(struct device *i915_kdev)
> {
> struct drm_i915_private *i915 = kdev_to_i915(i915_kdev);
> + struct intel_gt *gt = intel_pxp_get_owning_gt(i915);
>
> - return &to_gt(i915)->pxp;
> + if (!gt)
> + return NULL;
> +
> + return >->pxp;
or pxp is part of gt, what it looks like, then we use per gt and avoid on the
others...
> }
>
> static int intel_pxp_tee_io_message(struct intel_pxp *pxp,
> @@ -128,10 +132,16 @@ static int i915_pxp_tee_component_bind(struct device
> *i915_kdev,
> {
> struct drm_i915_private *i915 = kdev_to_i915(i915_kdev);
> struct intel_pxp *pxp = i915_dev_to_pxp(i915_kdev);
> - struct intel_uc *uc = &pxp_to_gt(pxp)->uc;
> + struct intel_uc *uc;
> intel_wakeref_t wakeref;
> int ret = 0;
>
> + if (!pxp) {
> + drm_warn(&i915->drm, "tee comp binding without a PXP-owner
> GT\n");
or we have a single pxp component under i915 and we associate with the gt0 only
and save the gt inside the pxp...
but this whole owning thing seems so convoluted...
> + return -ENODEV;
> + }
> + uc = &pxp_to_gt(pxp)->uc;
> +
> mutex_lock(&pxp->tee_mutex);
> pxp->pxp_component = data;
> pxp->pxp_component->tee_dev = tee_kdev;
> --
> 2.34.1
>
