Quoting Daniele Ceraolo Spurio (2021-02-06 02:09:15) > The context is required to send the session termination commands to the > VCS, which will be implemented in a follow-up patch. We can also use the > presence of the context as a check of pxp initialization completion. > > Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx> > --- > drivers/gpu/drm/i915/Makefile | 4 ++ > drivers/gpu/drm/i915/gt/intel_gt.c | 5 ++ > drivers/gpu/drm/i915/gt/intel_gt_types.h | 3 ++ > drivers/gpu/drm/i915/pxp/intel_pxp.c | 61 ++++++++++++++++++++++ > drivers/gpu/drm/i915/pxp/intel_pxp.h | 35 +++++++++++++ > drivers/gpu/drm/i915/pxp/intel_pxp_types.h | 15 ++++++ > 6 files changed, 123 insertions(+) > create mode 100644 drivers/gpu/drm/i915/pxp/intel_pxp.c > create mode 100644 drivers/gpu/drm/i915/pxp/intel_pxp.h > create mode 100644 drivers/gpu/drm/i915/pxp/intel_pxp_types.h > > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile > index ce01634d4ea7..e2677e8c03e8 100644 > --- a/drivers/gpu/drm/i915/Makefile > +++ b/drivers/gpu/drm/i915/Makefile > @@ -268,6 +268,10 @@ i915-y += \ > > i915-y += i915_perf.o > > +# Protected execution platform (PXP) support > +i915-$(CONFIG_DRM_I915_PXP) += \ > + pxp/intel_pxp.o > + > # Post-mortem debug and GPU hang state capture > i915-$(CONFIG_DRM_I915_CAPTURE_ERROR) += i915_gpu_error.o > i915-$(CONFIG_DRM_I915_SELFTEST) += \ > diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c > index ca76f93bc03d..daf61db620d6 100644 > --- a/drivers/gpu/drm/i915/gt/intel_gt.c > +++ b/drivers/gpu/drm/i915/gt/intel_gt.c > @@ -20,6 +20,7 @@ > #include "intel_uncore.h" > #include "intel_pm.h" > #include "shmem_utils.h" > +#include "pxp/intel_pxp.h" > > void intel_gt_init_early(struct intel_gt *gt, struct drm_i915_private *i915) > { > @@ -624,6 +625,8 @@ int intel_gt_init(struct intel_gt *gt) > if (err) > goto err_gt; > > + intel_pxp_init(>->pxp); > + > goto out_fw; > err_gt: > __intel_gt_disable(gt); > @@ -658,6 +661,8 @@ void intel_gt_driver_unregister(struct intel_gt *gt) > > intel_rps_driver_unregister(>->rps); > > + intel_pxp_fini(>->pxp); > + > /* > * Upon unregistering the device to prevent any new users, cancel > * all in-flight requests so that we can quickly unbind the active > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h > index 626af37c7790..324d267eee15 100644 > --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h > +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h > @@ -23,6 +23,7 @@ > #include "intel_rc6_types.h" > #include "intel_rps_types.h" > #include "intel_wakeref.h" > +#include "pxp/intel_pxp_types.h" > > struct drm_i915_private; > struct i915_ggtt; > @@ -145,6 +146,8 @@ struct intel_gt { > /* Slice/subslice/EU info */ > struct sseu_dev_info sseu; > } info; > + > + struct intel_pxp pxp; > }; > > enum intel_gt_scratch_field { > diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.c b/drivers/gpu/drm/i915/pxp/intel_pxp.c > new file mode 100644 > index 000000000000..4ddc8a71a3e7 > --- /dev/null > +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.c > @@ -0,0 +1,61 @@ > +// SPDX-License-Identifier: MIT > +/* > + * Copyright(c) 2020 Intel Corporation. > + */ > +#include "intel_pxp.h" > +#include "gt/intel_context.h" > +#include "i915_drv.h" > + > +static int create_vcs_context(struct intel_pxp *pxp) > +{ > + struct intel_gt *gt = pxp_to_gt(pxp); > + struct intel_context *ce = NULL; > + int i; > + > + /* > + * Find the first VCS engine present. We're guaranteed there is one > + * if we're in this function due to the check in has_pxp > + */ > + for (i = 0; i < I915_MAX_VCS && !ce; i++) > + if (HAS_ENGINE(gt, _VCS(i))) > + ce = intel_context_create(gt->engine[_VCS(i)]); Just wondering if struct intel_engine_cs **vcs_engines = gt->engine_class[CLASS_VIDEO_DECODE]; for (i = 0; i < ARRAY_SIZE(gt->engine_class[CLASS_VIDEO_DECODE]); i++) { if (!vcs_engines[i]) continue; ce = intel_context_create(vcs_engines[i]); break; } is a better iterator as it only checks one place of truth about whether or not the engine exists. for_each_engine_class(engine, gt, class, i) A couple of places could use that. > + if (IS_ERR(ce)) { > + drm_err(>->i915->drm, "failed to create VCS ctx for PXP\n"); > + return PTR_ERR(ce); Is the lack of this feature enough to prevent module loading? Surely userspace will notice and report the lack of the feature? > + } > + > + pxp->ce = ce; Is protected context then implicitly tried to one engine? i.e. userspace has to use the same engine as we control invalidation? Otherwise, everytime we use pxp->ce we must impose barriers across all gt->vcs. > + > + return 0; > +} > + > +static void destroy_vcs_context(struct intel_pxp *pxp) > +{ > + intel_context_put(fetch_and_zero(&pxp->ce)); > +} > + > +void intel_pxp_init(struct intel_pxp *pxp) > +{ > + struct intel_gt *gt = pxp_to_gt(pxp); > + int ret; > + > + if (!HAS_PXP(gt->i915)) > + return; > + > + ret = create_vcs_context(pxp); > + if (ret) > + return; > + > + drm_info(>->i915->drm, "Protected Xe Path (PXP) protected content support initialized\n"); > + > + return; > +} > + > +void intel_pxp_fini(struct intel_pxp *pxp) > +{ > + if (!intel_pxp_is_enabled(pxp)) > + return; > + > + destroy_vcs_context(pxp); > +} > diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.h b/drivers/gpu/drm/i915/pxp/intel_pxp.h > new file mode 100644 > index 000000000000..e2acd06402cd > --- /dev/null > +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.h > @@ -0,0 +1,35 @@ > +/* SPDX-License-Identifier: MIT */ > +/* > + * Copyright(c) 2020, Intel Corporation. All rights reserved. > + */ > + > +#ifndef __INTEL_PXP_H__ > +#define __INTEL_PXP_H__ > + > +#include "gt/intel_gt_types.h" > +#include "intel_pxp_types.h" > + > +static inline struct intel_gt *pxp_to_gt(struct intel_pxp *pxp) > +{ > + return container_of(pxp, struct intel_gt, pxp); > +} > + > +static inline bool intel_pxp_is_enabled(struct intel_pxp *pxp) const struct intel_pxp *pxp -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx