On 1/11/2023 1:42 PM, Alan Previn wrote:
Add MTL hw-plumbing enabling for KCR operation under PXP
which includes:
1. Updating 'pick-gt' to get the media tile for
KCR interrupt handling
2. Adding MTL's KCR registers for PXP operation
(init, status-checking, etc.).
While doing #2, lets create a separate registers header file for PXP
to be consistent with other i915 global subsystems.
Signed-off-by: Alan Previn <alan.previn.teres.alexis@xxxxxxxxx>
---
drivers/gpu/drm/i915/gt/intel_gt_irq.c | 3 +-
drivers/gpu/drm/i915/pxp/intel_pxp.c | 35 ++++++++++++--------
drivers/gpu/drm/i915/pxp/intel_pxp_regs.h | 26 +++++++++++++++
drivers/gpu/drm/i915/pxp/intel_pxp_session.c | 29 +++++++++++-----
4 files changed, 70 insertions(+), 23 deletions(-)
create mode 100644 drivers/gpu/drm/i915/pxp/intel_pxp_regs.h
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_irq.c b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
index 8fac2660e16b..957fa11373fc 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_irq.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
@@ -100,7 +100,8 @@ static struct intel_gt *pick_gt(struct intel_gt *gt, u8 class, u8 instance)
case VIDEO_ENHANCEMENT_CLASS:
return media_gt;
case OTHER_CLASS:
- if (instance == OTHER_GSC_INSTANCE && HAS_ENGINE(media_gt, GSC0))
+ if ((instance == OTHER_GSC_INSTANCE || instance == OTHER_KCR_INSTANCE) &&
+ HAS_ENGINE(media_gt, GSC0))
return media_gt;
fallthrough;
default:
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.c b/drivers/gpu/drm/i915/pxp/intel_pxp.c
index be52bf92e847..809b49f59594 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp.c
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp.c
@@ -14,6 +14,7 @@
#include "intel_pxp.h"
#include "intel_pxp_gsccs.h"
#include "intel_pxp_irq.h"
+#include "intel_pxp_regs.h"
#include "intel_pxp_session.h"
#include "intel_pxp_tee.h"
#include "intel_pxp_types.h"
@@ -61,21 +62,30 @@ bool intel_pxp_is_active(const struct intel_pxp *pxp)
return IS_ENABLED(CONFIG_DRM_I915_PXP) && pxp && pxp->arb_is_valid;
}
-/* KCR register definitions */
-#define KCR_INIT _MMIO(0x320f0)
-/* Setting KCR Init bit is required after system boot */
-#define KCR_INIT_ALLOW_DISPLAY_ME_WRITES REG_BIT(14)
+static i915_reg_t get_kcr_reg(const struct intel_pxp *pxp)
+{
+ if (pxp->gsccs_priv)
IMO here a better check would be:
if (pxp->ctrl_gt->type == GT_MEDIA)
It's equivalent, but IMO from a readability perspective it highlights
the fact that this is a change due to us moving to the media GT model
and has nothing to do with the GSC CS itself.
+ return MTL_KCR_INIT;
+ return GEN12_KCR_INIT;
+}
-static void kcr_pxp_enable(struct intel_gt *gt)
+static void kcr_pxp_set_status(const struct intel_pxp *pxp, bool enable)
{
- intel_uncore_write(gt->uncore, KCR_INIT,
- _MASKED_BIT_ENABLE(KCR_INIT_ALLOW_DISPLAY_ME_WRITES));
+ i915_reg_t reg = get_kcr_reg(pxp);
+ u32 val = enable ? _MASKED_BIT_ENABLE(KCR_INIT_ALLOW_DISPLAY_ME_WRITES) :
+ _MASKED_BIT_DISABLE(KCR_INIT_ALLOW_DISPLAY_ME_WRITES);
+
+ intel_uncore_write(pxp->ctrl_gt->uncore, reg, val);
}
-static void kcr_pxp_disable(struct intel_gt *gt)
+static void kcr_pxp_enable(const struct intel_pxp *pxp)
{
- intel_uncore_write(gt->uncore, KCR_INIT,
- _MASKED_BIT_DISABLE(KCR_INIT_ALLOW_DISPLAY_ME_WRITES));
+ kcr_pxp_set_status(pxp, true);
+}
+
+static void kcr_pxp_disable(const struct intel_pxp *pxp)
+{
+ kcr_pxp_set_status(pxp, false);
}
static int create_vcs_context(struct intel_pxp *pxp)
@@ -323,14 +333,13 @@ int intel_pxp_start(struct intel_pxp *pxp)
void intel_pxp_init_hw(struct intel_pxp *pxp)
{
- kcr_pxp_enable(pxp->ctrl_gt);
+ kcr_pxp_enable(pxp);
intel_pxp_irq_enable(pxp);
}
void intel_pxp_fini_hw(struct intel_pxp *pxp)
{
- kcr_pxp_disable(pxp->ctrl_gt);
-
+ kcr_pxp_disable(pxp);
intel_pxp_irq_disable(pxp);
}
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_regs.h b/drivers/gpu/drm/i915/pxp/intel_pxp_regs.h
new file mode 100644
index 000000000000..dd4131903d4e
--- /dev/null
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_regs.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright(c) 2023, Intel Corporation. All rights reserved.
+ */
+
+#ifndef __INTEL_PXP_REGS_H__
+#define __INTEL_PXP_REGS_H__
+
+#include "i915_reg_defs.h"
+
+/* KCR enable/disable control */
+#define GEN12_KCR_INIT _MMIO(0x320f0)
+#define MTL_KCR_INIT _MMIO(0x3860f0)
+
+/* Setting KCR Init bit is required after system boot */
+#define KCR_INIT_ALLOW_DISPLAY_ME_WRITES REG_BIT(14)
+
+/* KCR hwdrm session in play status 0-31 */
+#define GEN12_KCR_SIP _MMIO(0x32260)
+#define MTL_KCR_SIP _MMIO(0x386260)
+
+/* PXP global terminate register for session termination */
+#define GEN12_KCR_GLOBAL_TERMINATE _MMIO(0x320f8)
+#define MTL_KCR_GLOBAL_TERMINATE _MMIO(0x3860f8)
Non blocking suggestion:
it looks like all KCR regs are at the same relative offsets, just from a
different base (which makes, sense, because the HW just got moved to the
media tile as-is). So we could possibly have something like what we do
for the engines:
#define GEN12_KCR_BASE 0x32000
#define MTL_KCR_BASE 0x386000
#define KCR_INIT(base) _MMIO(base + 0xf0)
#define KCR_GLOBAL_TERMINATE(base) _MMIO(base + 0xf8)
#define KCR_SIP(base) _MMIO(base + 0x260)
Then if we store the correct base in the pxp structure we can just pass
it in the define when we need to access a register and remove the if
conditions at each access.
Daniele
+
+#endif /* __INTEL_PXP_REGS_H__ */
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_session.c b/drivers/gpu/drm/i915/pxp/intel_pxp_session.c
index 080aa2209c5b..7bb06e67b155 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_session.c
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_session.c
@@ -11,23 +11,25 @@
#include "intel_pxp_session.h"
#include "intel_pxp_tee.h"
#include "intel_pxp_types.h"
+#include "intel_pxp_regs.h"
#define ARB_SESSION I915_PROTECTED_CONTENT_DEFAULT_SESSION /* shorter define */
-#define GEN12_KCR_SIP _MMIO(0x32260) /* KCR hwdrm session in play 0-31 */
-
-/* PXP global terminate register for session termination */
-#define PXP_GLOBAL_TERMINATE _MMIO(0x320f8)
-
static bool intel_pxp_session_is_in_play(struct intel_pxp *pxp, u32 id)
{
struct intel_uncore *uncore = pxp->ctrl_gt->uncore;
intel_wakeref_t wakeref;
+ i915_reg_t reg;
u32 sip = 0;
/* if we're suspended the session is considered off */
- with_intel_runtime_pm_if_in_use(uncore->rpm, wakeref)
- sip = intel_uncore_read(uncore, GEN12_KCR_SIP);
+ with_intel_runtime_pm_if_in_use(uncore->rpm, wakeref) {
+ if (pxp->gsccs_priv)
+ reg = MTL_KCR_SIP;
+ else
+ reg = GEN12_KCR_SIP;
+ sip = intel_uncore_read(uncore, reg);
+ }
return sip & BIT(id);
}
@@ -37,6 +39,7 @@ static int pxp_wait_for_session_state(struct intel_pxp *pxp, u32 id, bool in_pla
struct intel_uncore *uncore = pxp->ctrl_gt->uncore;
intel_wakeref_t wakeref;
u32 mask = BIT(id);
+ i915_reg_t reg;
int ret;
/* if we're suspended the session is considered off */
@@ -44,8 +47,13 @@ static int pxp_wait_for_session_state(struct intel_pxp *pxp, u32 id, bool in_pla
if (!wakeref)
return in_play ? -ENODEV : 0;
+ if (pxp->gsccs_priv)
+ reg = MTL_KCR_SIP;
+ else
+ reg = GEN12_KCR_SIP;
+
ret = intel_wait_for_register(uncore,
- GEN12_KCR_SIP,
+ reg,
mask,
in_play ? mask : 0,
100);
@@ -112,7 +120,10 @@ static int pxp_terminate_arb_session_and_global(struct intel_pxp *pxp)
return ret;
}
- intel_uncore_write(gt->uncore, PXP_GLOBAL_TERMINATE, 1);
+ if (pxp->gsccs_priv)
+ intel_uncore_write(gt->uncore, MTL_KCR_GLOBAL_TERMINATE, 1);
+ else
+ intel_uncore_write(gt->uncore, GEN12_KCR_GLOBAL_TERMINATE, 1);
return ret;
}
