diff mbox series

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

Message ID 20231116232041.25534-1-alan.previn.teres.alexis@intel.com (mailing list archive)
State New, archived
Headers show
Series [v1,1/1] drm/i915/pxp: Bail early in pxp tee backend on first teardown error | expand

Commit Message

Alan Previn Nov. 16, 2023, 11:20 p.m. UTC
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 <alan.previn.teres.alexis@intel.com>
---
 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(-)


base-commit: 346f47e69d27a4b3177c2939b1f6f26d093ad8c4

Comments

Alan Previn Nov. 16, 2023, 11:28 p.m. UTC | #1
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,
>  				       &msg_in, sizeof(msg_in),
>  				       &msg_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.
diff mbox series

Patch

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(&i915->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(&i915->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(&i915->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;
 		return true;
 	default:
 		break;
@@ -58,7 +58,8 @@  fw_err_to_string(u32 type)
 static int intel_pxp_tee_io_message(struct intel_pxp *pxp,
 				    void *msg_in, u32 msg_in_size,
 				    void *msg_out, u32 msg_out_max_size,
-				    u32 *msg_out_rcv_size)
+				    u32 *msg_out_rcv_size,
+				    unsigned long timeout_ms)
 {
 	struct drm_i915_private *i915 = pxp->ctrl_gt->i915;
 	struct i915_pxp_component *pxp_component = pxp->pxp_component;
@@ -76,14 +77,14 @@  static int intel_pxp_tee_io_message(struct intel_pxp *pxp,
 	}
 
 	ret = pxp_component->ops->send(pxp_component->tee_dev, msg_in, msg_in_size,
-				       PXP_TRANSPORT_TIMEOUT_MS);
+				       timeout_ms);
 	if (ret) {
 		drm_err(&i915->drm, "Failed to send PXP TEE message\n");
 		goto unlock;
 	}
 
 	ret = pxp_component->ops->recv(pxp_component->tee_dev, msg_out, msg_out_max_size,
-				       PXP_TRANSPORT_TIMEOUT_MS);
+				       timeout_ms);
 	if (ret < 0) {
 		drm_err(&i915->drm, "Failed to receive PXP TEE message\n");
 		goto unlock;
@@ -344,12 +345,13 @@  int intel_pxp_tee_cmd_create_arb_session(struct intel_pxp *pxp,
 	ret = intel_pxp_tee_io_message(pxp,
 				       &msg_in, sizeof(msg_in),
 				       &msg_out, sizeof(msg_out),
-				       NULL);
+				       NULL, PXP_TRANSPORT_TIMEOUT_MS);
 
 	if (ret) {
 		drm_err(&i915->drm, "Failed to send tee msg init arb session, ret=[%d]\n", 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(&i915->drm,
 				      "PXP init-arb-session-%d failed due to BIOS/SOC:0x%08x:%s\n",
 				      arb_session_id, msg_out.header.status,
@@ -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,
 				       &msg_in, sizeof(msg_in),
 				       &msg_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)
 		goto try_again;
 
 	if (ret) {
@@ -398,6 +404,7 @@  void intel_pxp_tee_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(&i915->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_types.h b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h
index 7e11fa8034b2..acafd5dfe12f 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_types.h
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h
@@ -108,6 +108,16 @@  struct intel_pxp {
 	 * we only re-start the arb session when required.
 	 */
 	bool hw_state_invalidated;
+	/**
+	 * @hw_state_coldstart: If we are coming up from a cold boot or from
+	 * resume (not runtime resume) condition, 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).
+	 */
+	bool hw_state_coldstart;
 
 	/** @irq_enabled: tracks the status of the kcr irqs */
 	bool irq_enabled;