* [PATCH v5 0/3] drm/i915/pxp/mtl: Update gsc-heci cmd submission to align with fw/hw spec
@ 2023-09-09 22:38 Alan Previn
2023-09-09 22:38 ` [PATCH v5 1/3] drm/i915/pxp/mtl: Update pxp-firmware response timeout Alan Previn
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Alan Previn @ 2023-09-09 22:38 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniele Ceraolo Spurio, dri-devel, Alan Previn
For MTL, update the GSC-HECI packet size and the max firmware
response timeout to match internal fw specs. Enforce setting
run-alone bit in LRC for protected contexts.
Changes from prio revs:
v4: - PAGE_ALIGN the max heci packet size (Alan).
v3: - Patch #1. Only start counting the request completion
timeout from after the request has started (Daniele).
v2: - Patch #3: fix sparse warning reported by kernel test robot.
v1: - N/A (Re-test)
Signed-off-by: Alan Previn <alan.previn.teres.alexis@intel.com>
Alan Previn (3):
drm/i915/pxp/mtl: Update pxp-firmware response timeout
drm/i915/pxp/mtl: Update pxp-firmware packet size
drm/i915/lrc: User PXP contexts requires runalone bit in lrc
drivers/gpu/drm/i915/gt/intel_lrc.c | 23 +++++++++++++++++++
.../i915/gt/uc/intel_gsc_uc_heci_cmd_submit.c | 20 ++++++++++++++--
.../i915/gt/uc/intel_gsc_uc_heci_cmd_submit.h | 6 +++++
.../drm/i915/pxp/intel_pxp_cmd_interface_43.h | 4 ++--
drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.h | 11 +++++----
5 files changed, 56 insertions(+), 8 deletions(-)
base-commit: f8d21cb17a99b75862196036bb4bb93ee9637b74
--
2.39.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v5 1/3] drm/i915/pxp/mtl: Update pxp-firmware response timeout
2023-09-09 22:38 [PATCH v5 0/3] drm/i915/pxp/mtl: Update gsc-heci cmd submission to align with fw/hw spec Alan Previn
@ 2023-09-09 22:38 ` Alan Previn
2023-09-14 22:25 ` Balasubrawmanian, Vivaik
` (2 more replies)
2023-09-09 22:38 ` [PATCH v5 2/3] drm/i915/pxp/mtl: Update pxp-firmware packet size Alan Previn
2023-09-09 22:38 ` [PATCH v5 3/3] drm/i915/lrc: User PXP contexts requires runalone bit in lrc Alan Previn
2 siblings, 3 replies; 11+ messages in thread
From: Alan Previn @ 2023-09-09 22:38 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniele Ceraolo Spurio, dri-devel, Alan Previn
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 helpers,
start the count after the request hits the GSC command streamer.
Also, move GSC_REPLY_LATENCY_MS definition from pxp header to
intel_gsc_uc_heci_cmd_submit.h since its for any GSC HECI packet.
Signed-off-by: Alan Previn <alan.previn.teres.alexis@intel.com>
---
.../i915/gt/uc/intel_gsc_uc_heci_cmd_submit.c | 20 +++++++++++++++++--
.../i915/gt/uc/intel_gsc_uc_heci_cmd_submit.h | 6 ++++++
drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.h | 11 ++++++----
3 files changed, 31 insertions(+), 6 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..fe6a2f78cea0 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
@@ -81,8 +81,17 @@ int intel_gsc_uc_heci_cmd_submit_packet(struct intel_gsc_uc *gsc, u64 addr_in,
i915_request_add(rq);
- if (!err && i915_request_wait(rq, 0, msecs_to_jiffies(500)) < 0)
- err = -ETIME;
+ if (!err) {
+ /*
+ * Start timeout for i915_request_wait only after considering one possible
+ * pending GSC-HECI submission cycle on the other (non-privileged) path.
+ */
+ if (wait_for(i915_request_started(rq), GSC_HECI_REPLY_LATENCY_MS))
+ drm_dbg(&gsc_uc_to_gt(gsc)->i915->drm,
+ "Delay in gsc-heci-priv submission to gsccs-hw");
+ if (i915_request_wait(rq, 0, msecs_to_jiffies(500)) < 0)
+ err = -ETIME;
+ }
i915_request_put(rq);
@@ -186,6 +195,13 @@ intel_gsc_uc_heci_cmd_submit_nonpriv(struct intel_gsc_uc *gsc,
i915_request_add(rq);
if (!err) {
+ /*
+ * Start timeout for i915_request_wait only after considering one possible
+ * pending GSC-HECI submission cycle on the other (privileged) path.
+ */
+ if (wait_for(i915_request_started(rq), GSC_HECI_REPLY_LATENCY_MS))
+ drm_dbg(&gsc_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/gt/uc/intel_gsc_uc_heci_cmd_submit.h b/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc_heci_cmd_submit.h
index 09d3fbdad05a..5ae5c5d9608b 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc_heci_cmd_submit.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc_heci_cmd_submit.h
@@ -12,6 +12,12 @@ struct i915_vma;
struct intel_context;
struct intel_gsc_uc;
+#define GSC_HECI_REPLY_LATENCY_MS 350
+/*
+ * 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.
+ */
+
struct intel_gsc_mtl_header {
u32 validity_marker;
#define GSC_HECI_VALIDITY_MARKER 0xA578875A
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.h b/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.h
index 298ad38e6c7d..a4f17b3ea286 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.h
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.h
@@ -8,16 +8,19 @@
#include <linux/types.h>
+#include "gt/uc/intel_gsc_uc_heci_cmd_submit.h"
+
struct intel_pxp;
-#define GSC_REPLY_LATENCY_MS 210
+#define GSC_REPLY_LATENCY_MS GSC_HECI_REPLY_LATENCY_MS
/*
- * 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
-#define GSCFW_MAX_ROUND_TRIP_LATENCY_MS (GSC_PENDING_RETRY_MAXCOUNT * GSC_PENDING_RETRY_PAUSE_MS)
+#define GSCFW_MAX_ROUND_TRIP_LATENCY_MS (GSC_REPLY_LATENCY_MS + \
+ (GSC_PENDING_RETRY_MAXCOUNT * GSC_PENDING_RETRY_PAUSE_MS))
#ifdef CONFIG_DRM_I915_PXP
void intel_pxp_gsccs_fini(struct intel_pxp *pxp);
--
2.39.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v5 2/3] drm/i915/pxp/mtl: Update pxp-firmware packet size
2023-09-09 22:38 [PATCH v5 0/3] drm/i915/pxp/mtl: Update gsc-heci cmd submission to align with fw/hw spec Alan Previn
2023-09-09 22:38 ` [PATCH v5 1/3] drm/i915/pxp/mtl: Update pxp-firmware response timeout Alan Previn
@ 2023-09-09 22:38 ` Alan Previn
2023-09-15 17:28 ` Teres Alexis, Alan Previn
2023-09-15 18:02 ` Teres Alexis, Alan Previn
2023-09-09 22:38 ` [PATCH v5 3/3] drm/i915/lrc: User PXP contexts requires runalone bit in lrc Alan Previn
2 siblings, 2 replies; 11+ messages in thread
From: Alan Previn @ 2023-09-09 22:38 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniele Ceraolo Spurio, dri-devel, Alan Previn
Update the GSC-fw input/output HECI packet size to match
updated internal fw specs.
Signed-off-by: Alan Previn <alan.previn.teres.alexis@intel.com>
---
drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_43.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_43.h b/drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_43.h
index 0165d38fbead..e017a7d952e9 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_43.h
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_43.h
@@ -14,8 +14,8 @@
#define PXP43_CMDID_NEW_HUC_AUTH 0x0000003F /* MTL+ */
#define PXP43_CMDID_INIT_SESSION 0x00000036
-/* PXP-Packet sizes for MTL's GSCCS-HECI instruction */
-#define PXP43_MAX_HECI_INOUT_SIZE (SZ_32K)
+/* PXP-Packet sizes for MTL's GSCCS-HECI instruction is spec'd at 65K before page alignment*/
+#define PXP43_MAX_HECI_INOUT_SIZE (PAGE_ALIGNED(SZ_64K + SZ_1K))
/* PXP-Packet size for MTL's NEW_HUC_AUTH instruction */
#define PXP43_HUC_AUTH_INOUT_SIZE (SZ_4K)
--
2.39.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v5 3/3] drm/i915/lrc: User PXP contexts requires runalone bit in lrc
2023-09-09 22:38 [PATCH v5 0/3] drm/i915/pxp/mtl: Update gsc-heci cmd submission to align with fw/hw spec Alan Previn
2023-09-09 22:38 ` [PATCH v5 1/3] drm/i915/pxp/mtl: Update pxp-firmware response timeout Alan Previn
2023-09-09 22:38 ` [PATCH v5 2/3] drm/i915/pxp/mtl: Update pxp-firmware packet size Alan Previn
@ 2023-09-09 22:38 ` Alan Previn
2023-09-14 22:51 ` Balasubrawmanian, Vivaik
2023-09-15 18:04 ` Teres Alexis, Alan Previn
2 siblings, 2 replies; 11+ messages in thread
From: Alan Previn @ 2023-09-09 22:38 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniele Ceraolo Spurio, dri-devel, Alan Previn
On Meteorlake onwards, HW specs require that all user contexts that
run on render or compute engines and require PXP must enforce
run-alone bit in lrc. Add this enforcement for protected contexts.
Signed-off-by: Alan Previn <alan.previn.teres.alexis@intel.com>
---
drivers/gpu/drm/i915/gt/intel_lrc.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 967fe4d77a87..3df32177e49e 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -845,6 +845,27 @@ lrc_setup_indirect_ctx(u32 *regs,
lrc_ring_indirect_offset_default(engine) << 6;
}
+static bool ctx_needs_runalone(const struct intel_context *ce)
+{
+ struct i915_gem_context *gem_ctx;
+ bool ctx_is_protected = false;
+
+ /*
+ * On MTL and newer platforms, protected contexts require setting
+ * the LRC run-alone bit or else the encryption will not happen.
+ */
+ if (GRAPHICS_VER_FULL(ce->engine->i915) >= IP_VER(12, 70) &&
+ (ce->engine->class == COMPUTE_CLASS || ce->engine->class == RENDER_CLASS)) {
+ rcu_read_lock();
+ gem_ctx = rcu_dereference(ce->gem_context);
+ if (gem_ctx)
+ ctx_is_protected = gem_ctx->uses_protected_content;
+ rcu_read_unlock();
+ }
+
+ return ctx_is_protected;
+}
+
static void init_common_regs(u32 * const regs,
const struct intel_context *ce,
const struct intel_engine_cs *engine,
@@ -860,6 +881,8 @@ static void init_common_regs(u32 * const regs,
if (GRAPHICS_VER(engine->i915) < 11)
ctl |= _MASKED_BIT_DISABLE(CTX_CTRL_ENGINE_CTX_SAVE_INHIBIT |
CTX_CTRL_RS_CTX_ENABLE);
+ if (ctx_needs_runalone(ce))
+ ctl |= _MASKED_BIT_ENABLE(BIT(7));
regs[CTX_CONTEXT_CONTROL] = ctl;
regs[CTX_TIMESTAMP] = ce->stats.runtime.last;
--
2.39.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v5 1/3] drm/i915/pxp/mtl: Update pxp-firmware response timeout
2023-09-09 22:38 ` [PATCH v5 1/3] drm/i915/pxp/mtl: Update pxp-firmware response timeout Alan Previn
@ 2023-09-14 22:25 ` Balasubrawmanian, Vivaik
2023-09-15 17:30 ` Teres Alexis, Alan Previn
2023-09-15 17:58 ` Teres Alexis, Alan Previn
2 siblings, 0 replies; 11+ messages in thread
From: Balasubrawmanian, Vivaik @ 2023-09-14 22:25 UTC (permalink / raw)
To: dri-devel
On 9/9/2023 3:38 PM, 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 helpers,
> start the count after the request hits the GSC command streamer.
> Also, move GSC_REPLY_LATENCY_MS definition from pxp header to
> intel_gsc_uc_heci_cmd_submit.h since its for any GSC HECI packet.
>
> Signed-off-by: Alan Previn <alan.previn.teres.alexis@intel.com>
> ---
> .../i915/gt/uc/intel_gsc_uc_heci_cmd_submit.c | 20 +++++++++++++++++--
> .../i915/gt/uc/intel_gsc_uc_heci_cmd_submit.h | 6 ++++++
> drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.h | 11 ++++++----
> 3 files changed, 31 insertions(+), 6 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..fe6a2f78cea0 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
> @@ -81,8 +81,17 @@ int intel_gsc_uc_heci_cmd_submit_packet(struct intel_gsc_uc *gsc, u64 addr_in,
>
> i915_request_add(rq);
>
> - if (!err && i915_request_wait(rq, 0, msecs_to_jiffies(500)) < 0)
> - err = -ETIME;
> + if (!err) {
> + /*
> + * Start timeout for i915_request_wait only after considering one possible
> + * pending GSC-HECI submission cycle on the other (non-privileged) path.
> + */
> + if (wait_for(i915_request_started(rq), GSC_HECI_REPLY_LATENCY_MS))
> + drm_dbg(&gsc_uc_to_gt(gsc)->i915->drm,
> + "Delay in gsc-heci-priv submission to gsccs-hw");
> + if (i915_request_wait(rq, 0, msecs_to_jiffies(500)) < 0)
> + err = -ETIME;
> + }
>
> i915_request_put(rq);
>
> @@ -186,6 +195,13 @@ intel_gsc_uc_heci_cmd_submit_nonpriv(struct intel_gsc_uc *gsc,
> i915_request_add(rq);
>
> if (!err) {
> + /*
> + * Start timeout for i915_request_wait only after considering one possible
> + * pending GSC-HECI submission cycle on the other (privileged) path.
> + */
> + if (wait_for(i915_request_started(rq), GSC_HECI_REPLY_LATENCY_MS))
> + drm_dbg(&gsc_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/gt/uc/intel_gsc_uc_heci_cmd_submit.h b/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc_heci_cmd_submit.h
> index 09d3fbdad05a..5ae5c5d9608b 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc_heci_cmd_submit.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc_heci_cmd_submit.h
> @@ -12,6 +12,12 @@ struct i915_vma;
> struct intel_context;
> struct intel_gsc_uc;
>
> +#define GSC_HECI_REPLY_LATENCY_MS 350
> +/*
> + * 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.
> + */
> +
> struct intel_gsc_mtl_header {
> u32 validity_marker;
> #define GSC_HECI_VALIDITY_MARKER 0xA578875A
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.h b/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.h
> index 298ad38e6c7d..a4f17b3ea286 100644
> --- a/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.h
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.h
> @@ -8,16 +8,19 @@
>
> #include <linux/types.h>
>
> +#include "gt/uc/intel_gsc_uc_heci_cmd_submit.h"
> +
> struct intel_pxp;
>
> -#define GSC_REPLY_LATENCY_MS 210
> +#define GSC_REPLY_LATENCY_MS GSC_HECI_REPLY_LATENCY_MS
> /*
> - * 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
> -#define GSCFW_MAX_ROUND_TRIP_LATENCY_MS (GSC_PENDING_RETRY_MAXCOUNT * GSC_PENDING_RETRY_PAUSE_MS)
> +#define GSCFW_MAX_ROUND_TRIP_LATENCY_MS (GSC_REPLY_LATENCY_MS + \
> + (GSC_PENDING_RETRY_MAXCOUNT * GSC_PENDING_RETRY_PAUSE_MS))
>
> #ifdef CONFIG_DRM_I915_PXP
> void intel_pxp_gsccs_fini(struct intel_pxp *pxp);
Reviewed-by: Balasubrawmanian, Vivaik
<vivaik.balasubrawmanian@intel.com>
<mailto:vivaik.balasubrawmanian@intel.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v5 3/3] drm/i915/lrc: User PXP contexts requires runalone bit in lrc
2023-09-09 22:38 ` [PATCH v5 3/3] drm/i915/lrc: User PXP contexts requires runalone bit in lrc Alan Previn
@ 2023-09-14 22:51 ` Balasubrawmanian, Vivaik
2023-09-15 18:04 ` Teres Alexis, Alan Previn
1 sibling, 0 replies; 11+ messages in thread
From: Balasubrawmanian, Vivaik @ 2023-09-14 22:51 UTC (permalink / raw)
To: dri-devel
On 9/9/2023 3:38 PM, Alan Previn wrote:
> On Meteorlake onwards, HW specs require that all user contexts that
> run on render or compute engines and require PXP must enforce
> run-alone bit in lrc. Add this enforcement for protected contexts.
>
> Signed-off-by: Alan Previn <alan.previn.teres.alexis@intel.com>
> ---
> drivers/gpu/drm/i915/gt/intel_lrc.c | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index 967fe4d77a87..3df32177e49e 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -845,6 +845,27 @@ lrc_setup_indirect_ctx(u32 *regs,
> lrc_ring_indirect_offset_default(engine) << 6;
> }
>
> +static bool ctx_needs_runalone(const struct intel_context *ce)
> +{
> + struct i915_gem_context *gem_ctx;
> + bool ctx_is_protected = false;
> +
> + /*
> + * On MTL and newer platforms, protected contexts require setting
> + * the LRC run-alone bit or else the encryption will not happen.
> + */
> + if (GRAPHICS_VER_FULL(ce->engine->i915) >= IP_VER(12, 70) &&
> + (ce->engine->class == COMPUTE_CLASS || ce->engine->class == RENDER_CLASS)) {
> + rcu_read_lock();
> + gem_ctx = rcu_dereference(ce->gem_context);
> + if (gem_ctx)
> + ctx_is_protected = gem_ctx->uses_protected_content;
> + rcu_read_unlock();
> + }
> +
> + return ctx_is_protected;
> +}
> +
> static void init_common_regs(u32 * const regs,
> const struct intel_context *ce,
> const struct intel_engine_cs *engine,
> @@ -860,6 +881,8 @@ static void init_common_regs(u32 * const regs,
> if (GRAPHICS_VER(engine->i915) < 11)
> ctl |= _MASKED_BIT_DISABLE(CTX_CTRL_ENGINE_CTX_SAVE_INHIBIT |
> CTX_CTRL_RS_CTX_ENABLE);
> + if (ctx_needs_runalone(ce))
> + ctl |= _MASKED_BIT_ENABLE(BIT(7));
> regs[CTX_CONTEXT_CONTROL] = ctl;
>
> regs[CTX_TIMESTAMP] = ce->stats.runtime.last;
Can we please get the bit defined in intel_engine_regs.h with a define
instead of a number identification?
Review completed conditional to the above fix.
Reviewed-by: Balasubrawmanian, Vivaik
<vivaik.balasubrawmanian@intel.com>
<mailto:vivaik.balasubrawmanian@intel.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v5 2/3] drm/i915/pxp/mtl: Update pxp-firmware packet size
2023-09-09 22:38 ` [PATCH v5 2/3] drm/i915/pxp/mtl: Update pxp-firmware packet size Alan Previn
@ 2023-09-15 17:28 ` Teres Alexis, Alan Previn
2023-09-15 18:02 ` Teres Alexis, Alan Previn
1 sibling, 0 replies; 11+ messages in thread
From: Teres Alexis, Alan Previn @ 2023-09-15 17:28 UTC (permalink / raw)
To: intel-gfx@lists.freedesktop.org
Cc: Ceraolo Spurio, Daniele, dri-devel@lists.freedesktop.org
On Sat, 2023-09-09 at 15:38 -0700, Teres Alexis, Alan Previn wrote:
> Update the GSC-fw input/output HECI packet size to match
> updated internal fw specs.
>
> Signed-off-by: Alan Previn <alan.previn.teres.alexis@intel.com>
>
alan:snip
> -/* PXP-Packet sizes for MTL's GSCCS-HECI instruction */
> -#define PXP43_MAX_HECI_INOUT_SIZE (SZ_32K)
> +/* PXP-Packet sizes for MTL's GSCCS-HECI instruction is spec'd at 65K before page alignment*/
> +#define PXP43_MAX_HECI_INOUT_SIZE (PAGE_ALIGNED(SZ_64K + SZ_1K))
alan: silly ctrl-c/v bug on my part - should be PAGE_ALIGN, not ALIGNED
>
> /* PXP-Packet size for MTL's NEW_HUC_AUTH instruction */
> #define PXP43_HUC_AUTH_INOUT_SIZE (SZ_4K)
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v5 1/3] drm/i915/pxp/mtl: Update pxp-firmware response timeout
2023-09-09 22:38 ` [PATCH v5 1/3] drm/i915/pxp/mtl: Update pxp-firmware response timeout Alan Previn
2023-09-14 22:25 ` Balasubrawmanian, Vivaik
@ 2023-09-15 17:30 ` Teres Alexis, Alan Previn
2023-09-15 17:58 ` Teres Alexis, Alan Previn
2 siblings, 0 replies; 11+ messages in thread
From: Teres Alexis, Alan Previn @ 2023-09-15 17:30 UTC (permalink / raw)
To: intel-gfx@lists.freedesktop.org
Cc: Ceraolo Spurio, Daniele, dri-devel@lists.freedesktop.org
On Sat, 2023-09-09 at 15:38 -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 helpers,
> start the count after the request hits the GSC command streamer.
> Also, move GSC_REPLY_LATENCY_MS definition from pxp header to
> intel_gsc_uc_heci_cmd_submit.h since its for any GSC HECI packet.
>
> Signed-off-by: Alan Previn <alan.previn.teres.alexis@intel.com>
> ---
> .../i915/gt/uc/intel_gsc_uc_heci_cmd_submit.c | 20 +++++++++++++++++--
> .../i915/gt/uc/intel_gsc_uc_heci_cmd_submit.h | 6 ++++++
> drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.h | 11 ++++++----
> 3 files changed, 31 insertions(+), 6 deletions(-)
alan: snip
> index 09d3fbdad05a..5ae5c5d9608b 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc_heci_cmd_submit.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc_heci_cmd_submit.h
> @@ -12,6 +12,12 @@ struct i915_vma;
> struct intel_context;
> struct intel_gsc_uc;
>
> +#define GSC_HECI_REPLY_LATENCY_MS 350
> +/*
> + * 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.
> + */
alan: continue to face timeout issues - so increasing this to ~500 to absorb other hw/sw system latencies.
this also matches what the gsc-proxy code was doing - so i could use the same macro for that other code path.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v5 1/3] drm/i915/pxp/mtl: Update pxp-firmware response timeout
2023-09-09 22:38 ` [PATCH v5 1/3] drm/i915/pxp/mtl: Update pxp-firmware response timeout Alan Previn
2023-09-14 22:25 ` Balasubrawmanian, Vivaik
2023-09-15 17:30 ` Teres Alexis, Alan Previn
@ 2023-09-15 17:58 ` Teres Alexis, Alan Previn
2 siblings, 0 replies; 11+ messages in thread
From: Teres Alexis, Alan Previn @ 2023-09-15 17:58 UTC (permalink / raw)
To: intel-gfx@lists.freedesktop.org
Cc: Balasubrawmanian, Vivaik, Ceraolo Spurio, Daniele,
dri-devel@lists.freedesktop.org
On Sat, 2023-09-09 at 15:38 -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 helpers,
alan:snip
Vivaik replied with RB on dri-devel: https://lists.freedesktop.org/archives/dri-devel/2023-September/422861.html
we connected offline and agreed that his RB can remain standing on condition i fix the PAGE_ALIGNED -> PAGE_ALIGN fix.
Thanks Vivaik for reviewing.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v5 2/3] drm/i915/pxp/mtl: Update pxp-firmware packet size
2023-09-09 22:38 ` [PATCH v5 2/3] drm/i915/pxp/mtl: Update pxp-firmware packet size Alan Previn
2023-09-15 17:28 ` Teres Alexis, Alan Previn
@ 2023-09-15 18:02 ` Teres Alexis, Alan Previn
1 sibling, 0 replies; 11+ messages in thread
From: Teres Alexis, Alan Previn @ 2023-09-15 18:02 UTC (permalink / raw)
To: intel-gfx@lists.freedesktop.org
Cc: Ceraolo Spurio, Daniele, dri-devel@lists.freedesktop.org
On Sat, 2023-09-09 at 15:38 -0700, Teres Alexis, Alan Previn wrote:
> Update the GSC-fw input/output HECI packet size to match
> updated internal fw specs.
>
> Signed-off-by: Alan Previn <alan.previn.teres.alexis@intel.com>
> ---
> drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_43.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_43.h b/drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_43.h
> index 0165d38fbead..e017a7d952e9 100644
> --- a/drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_43.h
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_43.h
> @@ -14,8 +14,8 @@
> #define PXP43_CMDID_NEW_HUC_AUTH 0x0000003F /* MTL+ */
> #define PXP43_CMDID_INIT_SESSION 0x00000036
>
> -/* PXP-Packet sizes for MTL's GSCCS-HECI instruction */
> -#define PXP43_MAX_HECI_INOUT_SIZE (SZ_32K)
> +/* PXP-Packet sizes for MTL's GSCCS-HECI instruction is spec'd at 65K before page alignment*/
> +#define PXP43_MAX_HECI_INOUT_SIZE (PAGE_ALIGNED(SZ_64K + SZ_1K))
>
> /* PXP-Packet size for MTL's NEW_HUC_AUTH instruction */
> #define PXP43_HUC_AUTH_INOUT_SIZE (SZ_4K)
Vivaik replied with RB on dri-devel: https://lists.freedesktop.org/archives/dri-devel/2023-September/422862.htmll
we connected offline and agreed that his RB can remain standing on condition i fix the PAGE_ALIGNED -> PAGE_ALIGN fix.
Thanks Vivaik for reviewing.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v5 3/3] drm/i915/lrc: User PXP contexts requires runalone bit in lrc
2023-09-09 22:38 ` [PATCH v5 3/3] drm/i915/lrc: User PXP contexts requires runalone bit in lrc Alan Previn
2023-09-14 22:51 ` Balasubrawmanian, Vivaik
@ 2023-09-15 18:04 ` Teres Alexis, Alan Previn
1 sibling, 0 replies; 11+ messages in thread
From: Teres Alexis, Alan Previn @ 2023-09-15 18:04 UTC (permalink / raw)
To: intel-gfx@lists.freedesktop.org
Cc: Ceraolo Spurio, Daniele, dri-devel@lists.freedesktop.org
On Sat, 2023-09-09 at 15:38 -0700, Teres Alexis, Alan Previn wrote:
> On Meteorlake onwards, HW specs require that all user contexts that
> run on render or compute engines and require PXP must enforce
> run-alone bit in lrc. Add this enforcement for protected contexts.
alan:snip
>
> Signed-off-by: Alan Previn <alan.previn.teres.alexis@intel.com>
> @@ -860,6 +881,8 @@ static void init_common_regs(u32 * const regs,
> if (GRAPHICS_VER(engine->i915) < 11)
> ctl |= _MASKED_BIT_DISABLE(CTX_CTRL_ENGINE_CTX_SAVE_INHIBIT |
> CTX_CTRL_RS_CTX_ENABLE);
> + if (ctx_needs_runalone(ce))
> + ctl |= _MASKED_BIT_ENABLE(BIT(7));
> regs[CTX_CONTEXT_CONTROL] = ctl;
>
> regs[CTX_TIMESTAMP] = ce->stats.runtime.last;
alan: to align intel-gfx ml, Vivaik reviewed this on dri-devel at https://lists.freedesktop.org/archives/dri-devel/2023-September/422875.html - snippet:
thus, will rerev this (with the others fixes in this series).
>> Can we please get the bit defined in intel_engine_regs.h with a define
>> instead of a number identification?
>>
>> Review completed conditional to the above fix.
>>
>> Reviewed-by: Balasubrawmanian, Vivaik
>> <vivaik.balasubrawmanian at intel.com>
>> <mailto:vivaik.balasubrawmanian at intel.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-09-15 18:04 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-09 22:38 [PATCH v5 0/3] drm/i915/pxp/mtl: Update gsc-heci cmd submission to align with fw/hw spec Alan Previn
2023-09-09 22:38 ` [PATCH v5 1/3] drm/i915/pxp/mtl: Update pxp-firmware response timeout Alan Previn
2023-09-14 22:25 ` Balasubrawmanian, Vivaik
2023-09-15 17:30 ` Teres Alexis, Alan Previn
2023-09-15 17:58 ` Teres Alexis, Alan Previn
2023-09-09 22:38 ` [PATCH v5 2/3] drm/i915/pxp/mtl: Update pxp-firmware packet size Alan Previn
2023-09-15 17:28 ` Teres Alexis, Alan Previn
2023-09-15 18:02 ` Teres Alexis, Alan Previn
2023-09-09 22:38 ` [PATCH v5 3/3] drm/i915/lrc: User PXP contexts requires runalone bit in lrc Alan Previn
2023-09-14 22:51 ` Balasubrawmanian, Vivaik
2023-09-15 18:04 ` Teres Alexis, Alan Previn
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox