Re: [Intel-gfx] [PATCH v6 7/8] drm/i915/pxp: On MTL, KCR enabling doesn't wait on tee component

2023-04-05 Thread Teres Alexis, Alan Previn
> 
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(>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

alan: will make that change too - thanks for the R-b.
alan:snip


Re: [Intel-gfx] [PATCH v6 7/8] drm/i915/pxp: On MTL, KCR enabling doesn't wait on tee component

2023-03-03 Thread Ceraolo Spurio, Daniele




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 
---
  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(>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


+   }
+   } 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(>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);




[Intel-gfx] [PATCH v6 7/8] drm/i915/pxp: On MTL, KCR enabling doesn't wait on tee component

2023-02-27 Thread Alan Previn
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 
---
 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(>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(>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);
-- 
2.39.0