On Mon, 16 Nov 2020, "Souza, Jose" <jose.so...@intel.com> wrote: > On Fri, 2020-11-13 at 16:36 -0800, Sean Z Huang wrote: >> From: "Huang, Sean Z" <sean.z.hu...@intel.com> >> >> Create the irq worker that serves as callback handler, those >> callback stubs should be called while the hardware key teardown >> occurs. >> >> Signed-off-by: Huang, Sean Z <sean.z.hu...@intel.com> >> --- >> drivers/gpu/drm/i915/gt/intel_gt_irq.c | 4 ++ >> drivers/gpu/drm/i915/i915_reg.h | 1 + >> drivers/gpu/drm/i915/pxp/intel_pxp.c | 95 ++++++++++++++++++++++++++ >> drivers/gpu/drm/i915/pxp/intel_pxp.h | 22 ++++++ >> 4 files changed, 122 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_irq.c >> b/drivers/gpu/drm/i915/gt/intel_gt_irq.c >> index 257063a57101..d64013d0afb5 100644 >> --- a/drivers/gpu/drm/i915/gt/intel_gt_irq.c >> +++ b/drivers/gpu/drm/i915/gt/intel_gt_irq.c >> @@ -13,6 +13,7 @@ >> #include "intel_gt_irq.h" >> #include "intel_uncore.h" >> #include "intel_rps.h" >> +#include "pxp/intel_pxp.h" >> >> >> >> >> static void guc_irq_handler(struct intel_guc *guc, u16 iir) >> { >> @@ -106,6 +107,9 @@ gen11_other_irq_handler(struct intel_gt *gt, const u8 >> instance, >> if (instance == OTHER_GTPM_INSTANCE) >> return gen11_rps_irq_handler(>->rps, iir); >> >> >> >> >> + if (instance == OTHER_KCR_INSTANCE) >> + return intel_pxp_irq_handler(gt, iir); >> + >> WARN_ONCE(1, "unhandled other interrupt instance=0x%x, iir=0x%x\n", >> instance, iir); >> } >> diff --git a/drivers/gpu/drm/i915/i915_reg.h >> b/drivers/gpu/drm/i915/i915_reg.h >> index 7ea70b7ffcc6..faf6b06145fa 100644 >> --- a/drivers/gpu/drm/i915/i915_reg.h >> +++ b/drivers/gpu/drm/i915/i915_reg.h >> @@ -7941,6 +7941,7 @@ enum { >> /* irq instances for OTHER_CLASS */ >> #define OTHER_GUC_INSTANCE 0 >> #define OTHER_GTPM_INSTANCE 1 >> +#define OTHER_KCR_INSTANCE 4 >> >> >> >> >> #define GEN11_INTR_IDENTITY_REG(x) _MMIO(0x190060 + ((x) * 4)) >> >> >> >> >> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.c >> b/drivers/gpu/drm/i915/pxp/intel_pxp.c >> index a469c55e3e54..d98bff4a0fde 100644 >> --- a/drivers/gpu/drm/i915/pxp/intel_pxp.c >> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.c >> @@ -6,15 +6,110 @@ >> #include "i915_drv.h" >> #include "intel_pxp.h" >> >> >> >> >> +static void intel_pxp_write_irq_mask_reg(struct drm_i915_private *i915, u32 >> mask) >> +{ >> + WARN_ON(INTEL_GEN(i915) < 11); > > > should be drm_warn_on(), also I don't see any value in this warning. > >> + >> + /* crypto mask is in bit31-16 (Engine1 Interrupt Mask) */ >> + intel_uncore_write(&i915->uncore, GEN11_CRYPTO_RSVD_INTR_MASK, mask << >> 16); >> +} >> + >> +static void intel_pxp_unmask_irq(struct intel_gt *gt) >> +{ >> + lockdep_assert_held(>->irq_lock); >> + >> + intel_pxp_write_irq_mask_reg(gt->i915, 0); >> +} >> + >> +static void intel_pxp_mask_irq(struct intel_gt *gt, u32 mask) >> +{ >> + lockdep_assert_held(>->irq_lock); >> + >> + intel_pxp_write_irq_mask_reg(gt->i915, mask); >> +} >> + >> +static int intel_pxp_teardown_required_callback(struct drm_i915_private >> *i915) >> +{ >> + drm_dbg(&i915->drm, "%s was called\n", __func__); > > Looks like something used for debug, should not be in the upstream version. > Saw a lot more of debug like this that should be removed.
Absolutely! > >> + >> + return 0; >> +} >> + >> +static int intel_pxp_global_terminate_complete_callback(struct >> drm_i915_private *i915) >> +{ >> + drm_dbg(&i915->drm, ">>> %s\n", __func__); >> + >> + return 0; >> +} >> + >> +static void intel_pxp_irq_work(struct work_struct *work) >> +{ >> + struct intel_pxp *pxp_ptr = container_of(work, typeof(*pxp_ptr), >> irq_work); >> + struct drm_i915_private *i915 = container_of(pxp_ptr, typeof(*i915), >> pxp); >> + u32 events = 0; >> + >> + spin_lock_irq(&i915->gt.irq_lock); >> + events = fetch_and_zero(&pxp_ptr->current_events); >> + spin_unlock_irq(&i915->gt.irq_lock); >> + >> + drm_dbg(&i915->drm, "%s was called with events=[%d]\n", __func__, >> events); >> + >> + if (events & PXP_IRQ_VECTOR_DISPLAY_PXP_STATE_TERMINATED || >> + events & PXP_IRQ_VECTOR_DISPLAY_APP_TERM_PER_FW_REQ) >> + intel_pxp_teardown_required_callback(i915); >> + >> + if (events & PXP_IRQ_VECTOR_PXP_DISP_STATE_RESET_COMPLETE) >> + intel_pxp_global_terminate_complete_callback(i915); >> + >> + spin_lock_irq(&i915->gt.irq_lock); >> + intel_pxp_unmask_irq(&i915->gt); >> + spin_unlock_irq(&i915->gt.irq_lock); >> +} >> + >> int intel_pxp_init(struct drm_i915_private *i915) >> { >> int ret; >> >> >> >> >> >> >> >> >> drm_info(&i915->drm, "i915_pxp_init\n"); >> >> >> >> >> >> >> >> >> + INIT_WORK(&i915->pxp.irq_work, intel_pxp_irq_work); >> + >> + i915->pxp.handled_irr = (PXP_IRQ_VECTOR_DISPLAY_PXP_STATE_TERMINATED | >> + PXP_IRQ_VECTOR_DISPLAY_APP_TERM_PER_FW_REQ | >> + PXP_IRQ_VECTOR_PXP_DISP_STATE_RESET_COMPLETE); >> + >> return ret; >> } >> >> >> >> >> >> >> >> >> void intel_pxp_uninit(struct drm_i915_private *i915) >> { >> } >> + >> +/** >> + * intel_pxp_irq_handler - Proxies KCR interrupts to PXP. >> + * @gt: valid GT instance >> + * @iir: GT interrupt vector associated with the interrupt >> + * >> + * Dispatches each vector element into an IRQ to PXP. >> + */ >> +void intel_pxp_irq_handler(struct intel_gt *gt, u16 iir) >> +{ >> + struct drm_i915_private *i915 = gt->i915; >> + const u32 events = iir & i915->pxp.handled_irr; >> + >> + drm_dbg(&i915->drm, "%s was called with iir=[0x%04x]\n", __func__, iir); >> + >> + lockdep_assert_held(>->irq_lock); >> + >> + if (unlikely(!events)) { >> + drm_dbg(&i915->drm, "%s returned due to iir=[0x%04x]\n", >> __func__, iir); >> + goto end; Please just return instead of goto. >> + } >> + >> + intel_pxp_mask_irq(gt, i915->pxp.handled_irr); >> + >> + i915->pxp.current_events |= events; >> + schedule_work(&i915->pxp.irq_work); >> +end: >> + return; No need for return in the end of void functions. >> +} >> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.h >> b/drivers/gpu/drm/i915/pxp/intel_pxp.h >> index 578f1126bada..620774fc32e2 100644 >> --- a/drivers/gpu/drm/i915/pxp/intel_pxp.h >> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.h >> @@ -8,14 +8,36 @@ >> >> >> >> >> >> >> >> >> #include <drm/drm_file.h> >> >> >> >> >> >> >> >> >> +#define PXP_IRQ_VECTOR_DISPLAY_PXP_STATE_TERMINATED BIT(1) >> +#define PXP_IRQ_VECTOR_DISPLAY_APP_TERM_PER_FW_REQ BIT(2) >> +#define PXP_IRQ_VECTOR_PXP_DISP_STATE_RESET_COMPLETE BIT(3) >> + >> +enum pxp_sm_session_req { >> + /* Request KMD to allocate session id and move it to IN INIT */ >> + PXP_SM_REQ_SESSION_ID_INIT = 0x0, >> + /* Inform KMD that UMD has completed the initialization */ >> + PXP_SM_REQ_SESSION_IN_PLAY, >> + /* Request KMD to terminate the session */ >> + PXP_SM_REQ_SESSION_TERMINATE >> +}; >> + >> struct pxp_context; >> >> >> >> >> >> >> >> >> struct intel_pxp { >> + struct work_struct irq_work; >> + u32 handled_irr; >> + u32 current_events; >> + >> struct pxp_context *r0ctx; >> }; >> >> >> >> >> >> >> >> >> +struct intel_gt; >> struct drm_i915_private; >> >> >> >> >> >> >> >> >> +void intel_pxp_irq_handler(struct intel_gt *gt, u16 iir); >> +int i915_pxp_teardown_required_callback(struct drm_i915_private *i915); >> +int i915_pxp_global_terminate_complete_callback(struct drm_i915_private >> *i915); >> + >> int intel_pxp_init(struct drm_i915_private *i915); >> void intel_pxp_uninit(struct drm_i915_private *i915); >> >> >> >> >> >> >> >> > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Graphics Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx