| Message ID | 20230228022150.1657843-8-alan.previn.teres.alexis@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/pxp: Add MTL PXP Support | expand |
On 2/27/2023 6:21 PM, Alan Previn wrote: > On legacy platforms, KCR HW enabling is done at the time the mei > component interface is bound. It's also disabled during unbind. > However, for MTL onwards, we don't depend on a tee component > to start sending GSC-CS firmware messages. > > Thus, immediately enable (or disable) KCR HW on PXP's init, > fini and resume. > > Signed-off-by: Alan Previn <alan.previn.teres.alexis@intel.com> > --- > drivers/gpu/drm/i915/pxp/intel_pxp.c | 19 +++++++++++++++---- > drivers/gpu/drm/i915/pxp/intel_pxp_pm.c | 3 ++- > 2 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 61041277be24..e2f2cc5f6a6e 100644 > --- a/drivers/gpu/drm/i915/pxp/intel_pxp.c > +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.c > @@ -119,6 +119,7 @@ static void destroy_vcs_context(struct intel_pxp *pxp) > static void pxp_init_full(struct intel_pxp *pxp) > { > struct intel_gt *gt = pxp->ctrl_gt; > + intel_wakeref_t wakeref; > int ret; > > /* > @@ -140,10 +141,15 @@ static void pxp_init_full(struct intel_pxp *pxp) > if (ret) > return; > > - if (HAS_ENGINE(pxp->ctrl_gt, GSC0)) > + if (HAS_ENGINE(pxp->ctrl_gt, GSC0)) { > ret = intel_pxp_gsccs_init(pxp); > - else > + if (!ret) { > + with_intel_runtime_pm(&pxp->ctrl_gt->i915->runtime_pm, wakeref) > + intel_pxp_init_hw(pxp); personal preference: I'd move this (and the matching call in fini) inside intel_pxp_gsccs_init/fini. That way we can see this as more back-end specific: the gsccs initialize everything immediately, while the tee back-end follows a 2-step approach with the component. Not a blocker since it is a personal preference, so with or without the change: Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Daniele > + } > + } else { > ret = intel_pxp_tee_component_init(pxp); > + } > if (ret) > goto out_context; > > @@ -239,15 +245,20 @@ int intel_pxp_init(struct drm_i915_private *i915) > > void intel_pxp_fini(struct drm_i915_private *i915) > { > + intel_wakeref_t wakeref; > + > if (!i915->pxp) > return; > > i915->pxp->arb_is_valid = false; > > - if (HAS_ENGINE(i915->pxp->ctrl_gt, GSC0)) > + if (HAS_ENGINE(i915->pxp->ctrl_gt, GSC0)) { > + with_intel_runtime_pm(&i915->pxp->ctrl_gt->i915->runtime_pm, wakeref) > + intel_pxp_fini_hw(i915->pxp); > intel_pxp_gsccs_fini(i915->pxp); > - else > + } else { > intel_pxp_tee_component_fini(i915->pxp); > + } > > destroy_vcs_context(i915->pxp); > > diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_pm.c b/drivers/gpu/drm/i915/pxp/intel_pxp_pm.c > index 4f836b317424..1a04067f61fc 100644 > --- a/drivers/gpu/drm/i915/pxp/intel_pxp_pm.c > +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_pm.c > @@ -43,8 +43,9 @@ void intel_pxp_resume_complete(struct intel_pxp *pxp) > * The PXP component gets automatically unbound when we go into S3 and > * re-bound after we come out, so in that scenario we can defer the > * hw init to the bind call. > + * NOTE: GSC-CS backend doesn't rely on components. > */ > - if (!pxp->pxp_component) > + if (!HAS_ENGINE(pxp->ctrl_gt, GSC0) && !pxp->pxp_component) > return; > > intel_pxp_init_hw(pxp);
> alan:snip > > @@ -140,10 +141,15 @@ static void pxp_init_full(struct intel_pxp *pxp) > > if (ret) > > return; > > > > - if (HAS_ENGINE(pxp->ctrl_gt, GSC0)) > > + if (HAS_ENGINE(pxp->ctrl_gt, GSC0)) { > > ret = intel_pxp_gsccs_init(pxp); > > - else > > + if (!ret) { > > + with_intel_runtime_pm(&pxp->ctrl_gt->i915->runtime_pm, wakeref) > > + intel_pxp_init_hw(pxp); > > personal preference: I'd move this (and the matching call in fini) > inside intel_pxp_gsccs_init/fini. That way we can see this as more > back-end specific: the gsccs initialize everything immediately, while > the tee back-end follows a 2-step approach with the component. > Not a blocker since it is a personal preference, so with or without the > change: > > Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > > Daniele alan: will make that change too - thanks for the R-b. alan:snip
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.c b/drivers/gpu/drm/i915/pxp/intel_pxp.c index 61041277be24..e2f2cc5f6a6e 100644 --- a/drivers/gpu/drm/i915/pxp/intel_pxp.c +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.c @@ -119,6 +119,7 @@ static void destroy_vcs_context(struct intel_pxp *pxp) static void pxp_init_full(struct intel_pxp *pxp) { struct intel_gt *gt = pxp->ctrl_gt; + intel_wakeref_t wakeref; int ret; /* @@ -140,10 +141,15 @@ static void pxp_init_full(struct intel_pxp *pxp) if (ret) return; - if (HAS_ENGINE(pxp->ctrl_gt, GSC0)) + if (HAS_ENGINE(pxp->ctrl_gt, GSC0)) { ret = intel_pxp_gsccs_init(pxp); - else + if (!ret) { + with_intel_runtime_pm(&pxp->ctrl_gt->i915->runtime_pm, wakeref) + intel_pxp_init_hw(pxp); + } + } else { ret = intel_pxp_tee_component_init(pxp); + } if (ret) goto out_context; @@ -239,15 +245,20 @@ int intel_pxp_init(struct drm_i915_private *i915) void intel_pxp_fini(struct drm_i915_private *i915) { + intel_wakeref_t wakeref; + if (!i915->pxp) return; i915->pxp->arb_is_valid = false; - if (HAS_ENGINE(i915->pxp->ctrl_gt, GSC0)) + if (HAS_ENGINE(i915->pxp->ctrl_gt, GSC0)) { + with_intel_runtime_pm(&i915->pxp->ctrl_gt->i915->runtime_pm, wakeref) + intel_pxp_fini_hw(i915->pxp); intel_pxp_gsccs_fini(i915->pxp); - else + } else { intel_pxp_tee_component_fini(i915->pxp); + } destroy_vcs_context(i915->pxp); diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_pm.c b/drivers/gpu/drm/i915/pxp/intel_pxp_pm.c index 4f836b317424..1a04067f61fc 100644 --- a/drivers/gpu/drm/i915/pxp/intel_pxp_pm.c +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_pm.c @@ -43,8 +43,9 @@ void intel_pxp_resume_complete(struct intel_pxp *pxp) * The PXP component gets automatically unbound when we go into S3 and * re-bound after we come out, so in that scenario we can defer the * hw init to the bind call. + * NOTE: GSC-CS backend doesn't rely on components. */ - if (!pxp->pxp_component) + if (!HAS_ENGINE(pxp->ctrl_gt, GSC0) && !pxp->pxp_component) return; intel_pxp_init_hw(pxp);
On legacy platforms, KCR HW enabling is done at the time the mei component interface is bound. It's also disabled during unbind. However, for MTL onwards, we don't depend on a tee component to start sending GSC-CS firmware messages. Thus, immediately enable (or disable) KCR HW on PXP's init, fini and resume. Signed-off-by: Alan Previn <alan.previn.teres.alexis@intel.com> --- drivers/gpu/drm/i915/pxp/intel_pxp.c | 19 +++++++++++++++---- drivers/gpu/drm/i915/pxp/intel_pxp_pm.c | 3 ++- 2 files changed, 17 insertions(+), 5 deletions(-)