Re: [PATCH v1 1/1] drm/i915/pxp: Bail early in pxp tee backend on first teardown error

[Teres Alexis, Alan Previn]

On Thu, 2023-11-16 at 15:20 -0800, Teres Alexis, Alan Previn wrote:
> For Gen12 when using mei-pxp tee backend tranport, if we are coming
> up from a cold boot or from a resume (not runtime resume), we can
> optionally quicken the very first session cleanup that would occur
> as part of starting up a default PXP session. Typically a cleanup
> from a cold-start is expected to be quick so we can use a shorter
> timeout and skip retries (when getting non-success on calling
> backend transport for intel_pxp_tee_end_arb_fw_session).
alan:snip

> @@ -387,10 +389,14 @@ void intel_pxp_tee_end_arb_fw_session(struct intel_pxp 
> *pxp, u32 session_id)
>   ret = intel_pxp_tee_io_message(pxp,
>  _in, sizeof(msg_in),
>  _out, sizeof(msg_out),
> -NULL);
> +NULL, pxp->hw_state_coldstart ?
> +PXP_TRANSPORT_TIMEOUT_FAST_MS : 
> PXP_TRANSPORT_TIMEOUT_MS);
>  
> - /* Cleanup coherency between GT and Firmware is critical, so try again 
> if it fails */
> - if ((ret || msg_out.header.status != 0x0) && ++trials < 3)
> + /*
> +  * Cleanup coherency between GT and Firmware is critical, so try again 
> if it
> +  * fails, unless we are performing a cold-start reset
> +  */
> + if ((ret || msg_out.header.status != 0x0) && !pxp->hw_state_coldstart 
> &&  ++trials < 3)
alan: Take note I am working offline with sister teams to perform some end to
end conformance testing with more comprehensive OS stacks before we can verify
that this optimization doesnt break any existing use-cases.

[PATCH v1 1/1] drm/i915/pxp: Bail early in pxp tee backend on first teardown error

[Alan Previn]

For Gen12 when using mei-pxp tee backend tranport, if we are coming
up from a cold boot or from a resume (not runtime resume), we can
optionally quicken the very first session cleanup that would occur
as part of starting up a default PXP session. Typically a cleanup
from a cold-start is expected to be quick so we can use a shorter
timeout and skip retries (when getting non-success on calling
backend transport for intel_pxp_tee_end_arb_fw_session).

While we are touching this area of code, lets not update
pxp->platform_cfg_is_bad so its not done inside an "is_foo"
helper, move that to the helper's caller.

Signed-off-by: Alan Previn 
---
 drivers/gpu/drm/i915/pxp/intel_pxp.c |  1 +
 drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c   |  3 ++-
 drivers/gpu/drm/i915/pxp/intel_pxp_pm.c  |  1 +
 drivers/gpu/drm/i915/pxp/intel_pxp_session.c |  1 +
 drivers/gpu/drm/i915/pxp/intel_pxp_tee.c | 23 +---
 drivers/gpu/drm/i915/pxp/intel_pxp_types.h   | 10 +
 6 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.c 
b/drivers/gpu/drm/i915/pxp/intel_pxp.c
index dc327cf40b5a..b4de34e6ad01 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp.c
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp.c
@@ -140,6 +140,7 @@ static void pxp_init_full(struct intel_pxp *pxp)
if (ret)
return;
 
+   pxp->hw_state_coldstart = true;
if (HAS_ENGINE(pxp->ctrl_gt, GSC0))
ret = intel_pxp_gsccs_init(pxp);
else
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c 
b/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c
index 75df959b0aa0..94a26faac14f 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c
@@ -24,7 +24,6 @@ is_fw_err_platform_config(struct intel_pxp *pxp, u32 type)
case PXP_STATUS_ERROR_API_VERSION:
case PXP_STATUS_PLATFCONFIG_KF1_NOVERIF:
case PXP_STATUS_PLATFCONFIG_KF1_BAD:
-   pxp->platform_cfg_is_bad = true;
return true;
default:
break;
@@ -228,6 +227,7 @@ int intel_pxp_gsccs_create_session(struct intel_pxp *pxp,
drm_err(>drm, "Failed to init session %d, ret=[%d]\n", 
arb_session_id, ret);
} else if (msg_out.header.status != 0) {
if (is_fw_err_platform_config(pxp, msg_out.header.status)) {
+   pxp->platform_cfg_is_bad = true;
drm_info_once(>drm,
  "PXP init-session-%d failed due to 
BIOS/SOC:0x%08x:%s\n",
  arb_session_id, msg_out.header.status,
@@ -271,6 +271,7 @@ void intel_pxp_gsccs_end_arb_fw_session(struct intel_pxp 
*pxp, u32 session_id)
session_id, ret);
} else if (msg_out.header.status != 0) {
if (is_fw_err_platform_config(pxp, msg_out.header.status)) {
+   pxp->platform_cfg_is_bad = true;
drm_info_once(>drm,
  "PXP inv-stream-key-%u failed due to 
BIOS/SOC :0x%08x:%s\n",
  session_id, msg_out.header.status,
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_pm.c 
b/drivers/gpu/drm/i915/pxp/intel_pxp_pm.c
index 6dfd24918953..fd53b4fd7bac 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_pm.c
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_pm.c
@@ -60,6 +60,7 @@ static void _pxp_resume(struct intel_pxp *pxp, bool 
take_wakeref)
 void intel_pxp_resume_complete(struct intel_pxp *pxp)
 {
_pxp_resume(pxp, true);
+   pxp->hw_state_coldstart = true;
 }
 
 void intel_pxp_runtime_resume(struct intel_pxp *pxp)
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_session.c 
b/drivers/gpu/drm/i915/pxp/intel_pxp_session.c
index 0a3e66b0265e..7979648c6a2c 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_session.c
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_session.c
@@ -114,6 +114,7 @@ static int pxp_terminate_arb_session_and_global(struct 
intel_pxp *pxp)
intel_pxp_gsccs_end_arb_fw_session(pxp, ARB_SESSION);
else
intel_pxp_tee_end_arb_fw_session(pxp, ARB_SESSION);
+   pxp->hw_state_coldstart = false;
 
return ret;
 }
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c 
b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
index b00d6c280159..07696738d31b 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c
@@ -22,6 +22,7 @@
 #include "intel_pxp_types.h"
 
 #define PXP_TRANSPORT_TIMEOUT_MS 5000 /* 5 sec */
+#define PXP_TRANSPORT_TIMEOUT_FAST_MS 1000 /* 1 sec */
 
 static bool
 is_fw_err_platform_config(struct intel_pxp *pxp, u32 type)
@@ -30,7 +31,6 @@ is_fw_err_platform_config(struct intel_pxp *pxp, u32 type)
case PXP_STATUS_ERROR_API_VERSION:
case PXP_STATUS_PLATFCONFIG_KF1_NOVERIF:
case PXP_STATUS_PLATFCONFIG_KF1_BAD:
-   pxp->platform_cfg_is_bad = true;