Re: [PATCH v3 1/3] drm/i915/pxp/mtl: Update pxp-firmware response timeout
On Tue, 2023-08-15 at 13:29 -0700, Teres Alexis, Alan Previn wrote:
> Update the max GSC-fw response time to match updated internal
> fw specs. Because this response time is an SLA on the firmware,
> not inclusive of i915->GuC->HW handoff latency, when submitting
> requests to the GSC fw via intel_gsc_uc_heci_cmd_submit_nonpriv,
> start the count after the request hits the GSC command streamer.
>
> Signed-off-by: Alan Previn
> ---
> drivers/gpu/drm/i915/gt/uc/intel_gsc_uc_heci_cmd_submit.c | 3 +++
> drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.h| 6 +++---
> 2 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc_heci_cmd_submit.c
> b/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc_heci_cmd_submit.c
> index 89ed5ee9cded..ae45855594ac 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc_heci_cmd_submit.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc_heci_cmd_submit.c
> @@ -186,6 +186,9 @@ intel_gsc_uc_heci_cmd_submit_nonpriv(struct intel_gsc_uc
> *gsc,
> i915_request_add(rq);
>
> if (!err) {
> + if (wait_for(i915_request_started(rq), 200))
> + drm_dbg(_uc_to_gt(gsc)->i915->drm,
> + "Delay in gsc-heci-non-priv submission to
> gsccs-hw");
alan: offline discussion with Daniele, Daniele provided the following review
comments:
We should add this wait-check for both priv and non-priv but we should increase
the
timeout to be more than the guaranteed fw response time of 1 other message
(since we
have a total of 2 contexts that could be sending messages concurrently at any
time
including this one)... so maybe timeout of the new GSC_REPLY_LATENCY_MS + 150.
More importantly, he highlighted the fact that we should wait for the
request-started
AND ensure there as no error in request status.
[snip]
[PATCH v3 1/3] drm/i915/pxp/mtl: Update pxp-firmware response timeout
Update the max GSC-fw response time to match updated internal
fw specs. Because this response time is an SLA on the firmware,
not inclusive of i915->GuC->HW handoff latency, when submitting
requests to the GSC fw via intel_gsc_uc_heci_cmd_submit_nonpriv,
start the count after the request hits the GSC command streamer.
Signed-off-by: Alan Previn
---
drivers/gpu/drm/i915/gt/uc/intel_gsc_uc_heci_cmd_submit.c | 3 +++
drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.h| 6 +++---
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc_heci_cmd_submit.c
b/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc_heci_cmd_submit.c
index 89ed5ee9cded..ae45855594ac 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc_heci_cmd_submit.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc_heci_cmd_submit.c
@@ -186,6 +186,9 @@ intel_gsc_uc_heci_cmd_submit_nonpriv(struct intel_gsc_uc
*gsc,
i915_request_add(rq);
if (!err) {
+ if (wait_for(i915_request_started(rq), 200))
+ drm_dbg(_uc_to_gt(gsc)->i915->drm,
+ "Delay in gsc-heci-non-priv submission to
gsccs-hw");
if (i915_request_wait(rq, I915_WAIT_INTERRUPTIBLE,
msecs_to_jiffies(timeout_ms)) < 0)
err = -ETIME;
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.h
b/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.h
index 298ad38e6c7d..4368f010bbd3 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.h
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.h
@@ -10,10 +10,10 @@
struct intel_pxp;
-#define GSC_REPLY_LATENCY_MS 210
+#define GSC_REPLY_LATENCY_MS 350
/*
- * Max FW response time is 200ms, to which we add 10ms to account for overhead
- * such as request preparation, GuC submission to hw and pipeline completion
times.
+ * Max FW response time is 350ms, but this should be counted from the time the
+ * command has hit the GSC-CS hardware, not the preceding handoff to GuC CTB.
*/
#define GSC_PENDING_RETRY_MAXCOUNT 40
#define GSC_PENDING_RETRY_PAUSE_MS 50
--
2.39.0
