[Intel-gfx] [PATCH v2 1/7] drm/i915/pxp: Make gt and pxp init/fini aware of PXP-owning-GT
Ceraolo Spurio, Daniele
daniele.ceraolospurio at intel.com
Thu Oct 13 20:48:11 UTC 2022
On 10/5/2022 9:38 PM, Alan Previn wrote:
> In preparation for future MTL-PXP feature support, PXP control
> context should only valid on the correct gt tile. Depending on the
> device-info this mat not necessarily be the root GT tile and
> depends on which tile owns the VEBOX and KCR.
>
> PXP is still a global feature (despite the control-context being
> accessed via the owning GT structure) so let's also update HAS_PXP
> macro be called with the i915 handle instead of the gt handle.
> the correct gt-ptr access to grab the pxp handle.
>
> Update intel_pxp_init/fini aware of PXP-owning-GT to only initialize
> the PXP control-context of the correct GT structure.
>
> Signed-off-by: Alan Previn <alan.previn.teres.alexis at intel.com>
> ---
> drivers/gpu/drm/i915/gt/intel_gt.c | 4 ++++
> drivers/gpu/drm/i915/gt/intel_gt_types.h | 5 +++++
> drivers/gpu/drm/i915/gt/intel_sa_media.c | 4 ++++
> drivers/gpu/drm/i915/i915_drv.h | 6 +++---
> drivers/gpu/drm/i915/i915_pci.c | 1 +
> drivers/gpu/drm/i915/intel_device_info.h | 1 +
> drivers/gpu/drm/i915/pxp/intel_pxp.c | 22 +++++++++++++++++---
> drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.c | 2 +-
> 8 files changed, 38 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
> index b367cfff48d5..e61f6c5ed440 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> @@ -850,6 +850,10 @@ int intel_gt_probe_all(struct drm_i915_private *i915)
> gt->name = "Primary GT";
> gt->info.engine_mask = RUNTIME_INFO(i915)->platform_engine_mask;
>
> + /* device config determines which GT owns the global pxp-tee context */
> + if (VDBOX_MASK(gt) && !INTEL_INFO(i915)->has_nonroot_pxpgt)
> + gt->pxptee_iface_owner = true;
> +
I'm not convinced that we need dedicated has_nonroot_pxpgt and
pxptee_iface_owner flags. MTL moves the GSC inside a GT and the owner of
PXP is the GT where the GSC engine resides. So we could have a checker like:
bool intel_pxp_supported(struct intel_gt *gt)
{
/* we only support HECI PXP from the root GT */
if (HAS_HECI_PXP(gt->i915))
return gt_is_root(gt);
return HAS_ENGINE(gt, GSC);
}
I'm aware that the GSC engine code is still not available, but we can
special case for MTL for now and then replace it when the GSC code lands:
bool intel_pxp_supported(struct intel_gt *gt)
{
/* we only support HECI PXP from the root GT */
if (HAS_HECI_PXP(gt->i915))
return gt_is_root(gt);
/* TODO: replace with GSC check */
return IS_METEORLAKE(gt->i915) && !gt_is_root(gt);
}
Then we can use intel_pxp_supported(gt) instead of
gt->pxptee_iface_owner and we can drop has_nonroot_pxpgt. Might also be
worth merging this with HAS_PXP for a unified check, but that can come
later.
Daniele
> drm_dbg(&i915->drm, "Setting up %s\n", gt->name);
> ret = intel_gt_tile_setup(gt, phys_addr);
> if (ret)
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h
> index 30003d68fd51..fd554ec415cd 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
> @@ -279,6 +279,11 @@ struct intel_gt {
> u8 wb_index; /* Only used on HAS_L3_CCS_READ() platforms */
> } mocs;
>
> + /*
> + * In a multi-tile GPU, only one GT-tile can contain
> + * the single valid global pxp + tee context.
> + */
> + bool pxptee_iface_owner;
> struct intel_pxp pxp;
>
> /* gt/gtN sysfs */
> diff --git a/drivers/gpu/drm/i915/gt/intel_sa_media.c b/drivers/gpu/drm/i915/gt/intel_sa_media.c
> index e8f3d18c12b8..038344b48760 100644
> --- a/drivers/gpu/drm/i915/gt/intel_sa_media.c
> +++ b/drivers/gpu/drm/i915/gt/intel_sa_media.c
> @@ -36,6 +36,10 @@ int intel_sa_mediagt_setup(struct intel_gt *gt, phys_addr_t phys_addr,
> gt->uncore = uncore;
> gt->phys_addr = phys_addr;
>
> + /* On MTL, the standalone media owns the global PXP/TEE context. */
> + if (HAS_PXP(gt) && gt->info.id == 1)
> + gt->pxptee_iface_owner = true;
> +
> /*
> * For current platforms we can assume there's only a single
> * media GT and cache it for quick lookup.
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 90ed8e6db2fe..9fd0c065aa23 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -928,9 +928,9 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
>
> #define HAS_GLOBAL_MOCS_REGISTERS(dev_priv) (INTEL_INFO(dev_priv)->has_global_mocs)
>
> -#define HAS_PXP(dev_priv) ((IS_ENABLED(CONFIG_DRM_I915_PXP) && \
> - INTEL_INFO(dev_priv)->has_pxp) && \
> - VDBOX_MASK(to_gt(dev_priv)))
> +#define HAS_PXP(gt) (IS_ENABLED(CONFIG_DRM_I915_PXP) && \
> + (INTEL_INFO((gt)->i915)->has_pxp) && \
> + VDBOX_MASK(gt))
>
> #define HAS_GMCH(dev_priv) (INTEL_INFO(dev_priv)->display.has_gmch)
>
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index 38460a0bd7cb..6ee1cd6f1194 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -1149,6 +1149,7 @@ static const struct intel_device_info mtl_info = {
> .__runtime.memory_regions = REGION_SMEM | REGION_STOLEN_LMEM,
> .__runtime.platform_engine_mask = BIT(RCS0) | BIT(BCS0) | BIT(CCS0),
> .require_force_probe = 1,
> + .has_nonroot_pxpgt = 1,
> };
>
> #undef PLATFORM
> diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
> index bc87d3156b14..8508d3795593 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.h
> +++ b/drivers/gpu/drm/i915/intel_device_info.h
> @@ -167,6 +167,7 @@ enum intel_ppgtt_type {
> func(has_mslice_steering); \
> func(has_one_eu_per_fuse_bit); \
> func(has_pxp); \
> + func(has_nonroot_pxpgt); \
> func(has_rc6); \
> func(has_rc6p); \
> func(has_rps); \
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.c b/drivers/gpu/drm/i915/pxp/intel_pxp.c
> index 5efe61f67546..a18dfeca919b 100644
> --- a/drivers/gpu/drm/i915/pxp/intel_pxp.c
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.c
> @@ -138,11 +138,22 @@ static void pxp_init_full(struct intel_pxp *pxp)
> destroy_vcs_context(pxp);
> }
>
> +static bool _gt_needs_teelink(struct intel_gt *gt)
> +{
> + return intel_huc_is_loaded_by_gsc(>->uc.huc) && intel_uc_uses_huc(>->uc);
> +}
> +
> void intel_pxp_init(struct intel_pxp *pxp)
> {
> struct intel_gt *gt = pxp_to_gt(pxp);
>
> - /* we rely on the mei PXP module */
> + /*
> + * In current platforms we only need a single pxp component but must reside
> + * within the owner gt.
> + */
> + if (!gt->pxptee_iface_owner)
> + return;
> +
> if (!IS_ENABLED(CONFIG_INTEL_MEI_PXP))
> return;
>
> @@ -150,14 +161,19 @@ void intel_pxp_init(struct intel_pxp *pxp)
> * If HuC is loaded by GSC but PXP is disabled, we can skip the init of
> * the full PXP session/object management and just init the tee channel.
> */
> - if (HAS_PXP(gt->i915))
> + if (HAS_PXP(gt))
> pxp_init_full(pxp);
> - else if (intel_huc_is_loaded_by_gsc(>->uc.huc) && intel_uc_uses_huc(>->uc))
> + else if (_gt_needs_teelink(gt))
> intel_pxp_tee_component_init(pxp);
> }
>
> void intel_pxp_fini(struct intel_pxp *pxp)
> {
> + struct intel_gt *gt = pxp_to_gt(pxp);
> +
> + if (!gt->pxptee_iface_owner)
> + return;
> +
> pxp->arb_is_valid = false;
>
> intel_pxp_tee_component_fini(pxp);
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.c b/drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.c
> index 4359e8be4101..7b37f061044d 100644
> --- a/drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.c
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_debugfs.c
> @@ -70,7 +70,7 @@ void intel_pxp_debugfs_register(struct intel_pxp *pxp, struct dentry *gt_root)
> if (!gt_root)
> return;
>
> - if (!HAS_PXP((pxp_to_gt(pxp)->i915)))
> + if (!HAS_PXP((pxp_to_gt(pxp))))
> return;
>
> root = debugfs_create_dir("pxp", gt_root);
More information about the Intel-gfx
mailing list