Message ID | 20230111214226.907536-4-alan.previn.teres.alexis@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | drm/i915/pxp: Add MTL PXP Support | expand |
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@intel.com> > --- > 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; > }
Thanks for reviewing. On Wed, 2023-01-18 at 16:09 -0800, Ceraolo Spurio, Daniele wrote: > > > > Alan: [snip] > > > > -/* 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. > > Alan: Yes agreed - in alignment with to your next comment, this readiblity improvement shall therefore go into pxp_init (when we initialize value for kcr_base offset) Alan: [snip] > > > > +#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. > > Alan: sounds good - will do this. Alan: [snip]
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) + 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) + +#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; }
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@intel.com> --- 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