[PATCH v2 05/12] drm/xe/pxp: Handle the PXP termination interrupt
Daniele Ceraolo Spurio
daniele.ceraolospurio at intel.com
Thu Nov 7 00:33:20 UTC 2024
On 10/7/24 17:34, John Harrison wrote:
> On 8/16/2024 12:00, Daniele Ceraolo Spurio wrote:
>> When something happen to the session, the HW generates a termination
>> interrupt. In reply to this, the driver is required to submit an inline
>> session termination via the VCS, trigger the global termination and
>> notify the GSC FW that the session is now invalid.
>>
>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
>> ---
>> drivers/gpu/drm/xe/regs/xe_gt_regs.h | 8 ++
>> drivers/gpu/drm/xe/regs/xe_pxp_regs.h | 6 ++
>> drivers/gpu/drm/xe/xe_irq.c | 20 +++-
>> drivers/gpu/drm/xe/xe_pxp.c | 138 +++++++++++++++++++++++++-
>> drivers/gpu/drm/xe/xe_pxp.h | 3 +
>> drivers/gpu/drm/xe/xe_pxp_types.h | 13 +++
>> 6 files changed, 184 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/regs/xe_gt_regs.h
>> b/drivers/gpu/drm/xe/regs/xe_gt_regs.h
>> index 0d1a4a9f4e11..9e9c20f1f1f4 100644
>> --- a/drivers/gpu/drm/xe/regs/xe_gt_regs.h
>> +++ b/drivers/gpu/drm/xe/regs/xe_gt_regs.h
>> @@ -570,6 +570,7 @@
>> #define ENGINE1_MASK REG_GENMASK(31, 16)
>> #define ENGINE0_MASK REG_GENMASK(15, 0)
>> #define GPM_WGBOXPERF_INTR_ENABLE XE_REG(0x19003c,
>> XE_REG_OPTION_VF)
>> +#define CRYPTO_RSVD_INTR_ENABLE XE_REG(0x190040)
>> #define GUNIT_GSC_INTR_ENABLE XE_REG(0x190044,
>> XE_REG_OPTION_VF)
>> #define CCS_RSVD_INTR_ENABLE XE_REG(0x190048,
>> XE_REG_OPTION_VF)
>> @@ -580,6 +581,7 @@
>> #define INTR_ENGINE_INTR(x) REG_FIELD_GET(GENMASK(15, 0), x)
>> #define OTHER_GUC_INSTANCE 0
>> #define OTHER_GSC_HECI2_INSTANCE 3
>> +#define OTHER_KCR_INSTANCE 4
>> #define OTHER_GSC_INSTANCE 6
>> #define IIR_REG_SELECTOR(x) XE_REG(0x190070 + ((x) *
>> 4), XE_REG_OPTION_VF)
>> @@ -591,6 +593,7 @@
>> #define HECI2_RSVD_INTR_MASK XE_REG(0x1900e4)
>> #define GUC_SG_INTR_MASK XE_REG(0x1900e8, XE_REG_OPTION_VF)
>> #define GPM_WGBOXPERF_INTR_MASK XE_REG(0x1900ec,
>> XE_REG_OPTION_VF)
>> +#define CRYPTO_RSVD_INTR_MASK XE_REG(0x1900f0)
>> #define GUNIT_GSC_INTR_MASK XE_REG(0x1900f4,
>> XE_REG_OPTION_VF)
>> #define CCS0_CCS1_INTR_MASK XE_REG(0x190100)
>> #define CCS2_CCS3_INTR_MASK XE_REG(0x190104)
>> @@ -605,4 +608,9 @@
>> #define GT_CS_MASTER_ERROR_INTERRUPT REG_BIT(3)
>> #define GT_RENDER_USER_INTERRUPT REG_BIT(0)
>> +/* irqs for OTHER_KCR_INSTANCE */
>> +#define KCR_PXP_STATE_TERMINATED_INTERRUPT REG_BIT(1)
>> +#define KCR_APP_TERMINATED_PER_FW_REQ_INTERRUPT REG_BIT(2)
>> +#define KCR_PXP_STATE_RESET_COMPLETE_INTERRUPT REG_BIT(3)
>> +
>> #endif
>> diff --git a/drivers/gpu/drm/xe/regs/xe_pxp_regs.h
>> b/drivers/gpu/drm/xe/regs/xe_pxp_regs.h
>> index d67cf210d23d..aa158938b42e 100644
>> --- a/drivers/gpu/drm/xe/regs/xe_pxp_regs.h
>> +++ b/drivers/gpu/drm/xe/regs/xe_pxp_regs.h
>> @@ -14,4 +14,10 @@
>> #define KCR_INIT XE_REG(0x3860f0)
>> #define KCR_INIT_ALLOW_DISPLAY_ME_WRITES REG_BIT(14)
>> +/* KCR hwdrm session in play status 0-31 */
>> +#define KCR_SIP XE_REG(0x386260)
>> +
>> +/* PXP global terminate register for session termination */
>> +#define KCR_GLOBAL_TERMINATE XE_REG(0x3860f8)
>> +
>> #endif /* __XE_PXP_REGS_H__ */
>> diff --git a/drivers/gpu/drm/xe/xe_irq.c b/drivers/gpu/drm/xe/xe_irq.c
>> index 5f2c368c35ad..f11d9a740627 100644
>> --- a/drivers/gpu/drm/xe/xe_irq.c
>> +++ b/drivers/gpu/drm/xe/xe_irq.c
>> @@ -20,6 +20,7 @@
>> #include "xe_hw_engine.h"
>> #include "xe_memirq.h"
>> #include "xe_mmio.h"
>> +#include "xe_pxp.h"
>> #include "xe_sriov.h"
>> /*
>> @@ -202,6 +203,15 @@ void xe_irq_enable_hwe(struct xe_gt *gt)
>> }
>> if (heci_mask)
>> xe_mmio_write32(gt, HECI2_RSVD_INTR_MASK, ~(heci_mask
>> << 16));
>> +
>> + if (xe_pxp_is_supported(xe)) {
>> + u32 kcr_mask = KCR_PXP_STATE_TERMINATED_INTERRUPT |
>> + KCR_APP_TERMINATED_PER_FW_REQ_INTERRUPT |
>> + KCR_PXP_STATE_RESET_COMPLETE_INTERRUPT;
>> +
>> + xe_mmio_write32(gt, CRYPTO_RSVD_INTR_ENABLE, kcr_mask <<
>> 16);
>> + xe_mmio_write32(gt, CRYPTO_RSVD_INTR_MASK, ~(kcr_mask <<
>> 16));
>> + }
>> }
>> }
>> @@ -324,9 +334,15 @@ static void gt_irq_handler(struct xe_tile *tile,
>> }
>> if (class == XE_ENGINE_CLASS_OTHER) {
>> - /* HECI GSCFI interrupts come from outside of GT */
>> + /*
>> + * HECI GSCFI interrupts come from outside of GT.
>> + * KCR irqs come from inside GT but are handled
>> + * by the global PXP subsystem.
>> + */
>> if (HAS_HECI_GSCFI(xe) && instance ==
>> OTHER_GSC_INSTANCE)
>> xe_heci_gsc_irq_handler(xe, intr_vec);
>> + else if (instance == OTHER_KCR_INSTANCE)
>> + xe_pxp_irq_handler(xe, intr_vec);
>> else
>> gt_other_irq_handler(engine_gt, instance,
>> intr_vec);
>> }
>> @@ -512,6 +528,8 @@ static void gt_irq_reset(struct xe_tile *tile)
>> xe_mmio_write32(mmio, GUNIT_GSC_INTR_ENABLE, 0);
>> xe_mmio_write32(mmio, GUNIT_GSC_INTR_MASK, ~0);
>> xe_mmio_write32(mmio, HECI2_RSVD_INTR_MASK, ~0);
>> + xe_mmio_write32(mmio, CRYPTO_RSVD_INTR_ENABLE, 0);
>> + xe_mmio_write32(mmio, CRYPTO_RSVD_INTR_MASK, ~0);
>> }
>> xe_mmio_write32(mmio, GPM_WGBOXPERF_INTR_ENABLE, 0);
>> diff --git a/drivers/gpu/drm/xe/xe_pxp.c b/drivers/gpu/drm/xe/xe_pxp.c
>> index 56bb7d927c07..382eb0cb0018 100644
>> --- a/drivers/gpu/drm/xe/xe_pxp.c
>> +++ b/drivers/gpu/drm/xe/xe_pxp.c
>> @@ -12,9 +12,11 @@
>> #include "xe_gt.h"
>> #include "xe_gt_types.h"
>> #include "xe_mmio.h"
>> +#include "xe_pm.h"
>> #include "xe_pxp_submit.h"
>> #include "xe_pxp_types.h"
>> #include "xe_uc_fw.h"
>> +#include "regs/xe_gt_regs.h"
>> #include "regs/xe_pxp_regs.h"
>> /**
>> @@ -25,11 +27,133 @@
>> * integrated parts.
>> */
>> -static bool pxp_is_supported(const struct xe_device *xe)
>> +#define ARB_SESSION 0xF /* TODO: move to UAPI */
>> +
>> +bool xe_pxp_is_supported(const struct xe_device *xe)
>> {
>> return xe->info.has_pxp && IS_ENABLED(CONFIG_INTEL_MEI_GSC_PROXY);
>> }
>> +static bool pxp_is_enabled(const struct xe_pxp *pxp)
>> +{
>> + return pxp;
>> +}
>> +
>> +static int pxp_wait_for_session_state(struct xe_pxp *pxp, u32 id,
>> bool in_play)
>> +{
>> + struct xe_gt *gt = pxp->gt;
>> + u32 mask = BIT(id);
>> + int ret;
>> +
>> + ret = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT);
>> + if (ret)
>> + return ret;
>> +
>> + ret = xe_mmio_wait32(gt, KCR_SIP, mask, in_play ? mask : 0,
>> + 250, NULL, false);
>> + xe_force_wake_put(gt_to_fw(gt), XE_FW_GT);
>> +
>> + return ret;
>> +}
>> +
>> +static void pxp_terminate(struct xe_pxp *pxp)
>> +{
>> + int ret = 0;
>> + struct xe_device *xe = pxp->xe;
>> + struct xe_gt *gt = pxp->gt;
>> +
>> + drm_dbg(&xe->drm, "Terminating PXP\n");
>> +
>> + /* terminate the hw session */
>> + ret = xe_pxp_submit_session_termination(pxp, ARB_SESSION);
>> + if (ret)
>> + goto out;
>> +
>> + ret = pxp_wait_for_session_state(pxp, ARB_SESSION, false);
>> + if (ret)
>> + goto out;
>> +
>> + /* Trigger full HW cleanup */
>> + XE_WARN_ON(xe_force_wake_get(gt_to_fw(gt), XE_FW_GT));
> Why WARN here but no explicit message at all if the earlier force wake
> fails? And is it safe to keep going if the fw did fail?
The idea was that if we know that the state is good enough to terminate
we could try to do it even with a forcewake error, worst case it doesn't
work. If we don't know the state we can't attempt a termination at all.
>
> Also, given two identical, back-to-back fw get/put sets, would it not
> be more efficient to have pxp_terminate do the get and share that
> across the two register access? It would also remove the issue with
> failed fw half way through causing problems due to not wanting to abort.
will do.
>
>> + xe_mmio_write32(gt, KCR_GLOBAL_TERMINATE, 1);
> BSpec description for KCR_GLOBAL_TERMINATE says need to check
> KCR_SIP_GCD rather than KCR_SIP_MEDIA for bits 0-15 being 0. Whereas
> the KCR_SIP being checked above is KCR_SIP_MEDIA only.
That's just the spec not being super clear. The description is common
between the render and the media copies of the registers, but you need
to check the version of the registers on the actual GT you're doing the
termination on.
>
>> + xe_force_wake_put(gt_to_fw(gt), XE_FW_GT);
>> +
>> + /* now we can tell the GSC to clean up its own state */
>> + ret = xe_pxp_submit_session_invalidation(&pxp->gsc_res,
>> ARB_SESSION);
>> +
>> +out:
>> + if (ret)
>> + drm_err(&xe->drm, "PXP termination failed: %pe\n",
>> ERR_PTR(ret));
>> +}
>> +
>> +static void pxp_terminate_complete(struct xe_pxp *pxp)
>> +{
>> + /* TODO mark the session as ready to start */
>> +}
>> +
>> +static void pxp_irq_work(struct work_struct *work)
>> +{
>> + struct xe_pxp *pxp = container_of(work, typeof(*pxp), irq.work);
>> + struct xe_device *xe = pxp->xe;
>> + u32 events = 0;
>> +
>> + spin_lock_irq(&xe->irq.lock);
>> + events = pxp->irq.events;
>> + pxp->irq.events = 0;
>> + spin_unlock_irq(&xe->irq.lock);
>> +
>> + if (!events)
>> + return;
>> +
>> + /*
>> + * If we're processing a termination irq while suspending then
>> don't
>> + * bother, we're going to re-init everything on resume anyway.
>> + */
>> + if ((events & PXP_TERMINATION_REQUEST) &&
>> !xe_pm_runtime_get_if_active(xe))
>> + return;
> I assume it is not possible to have both REQUEST and COMPLETE set at
> the same time? I.e. is it possible for this early exit to cause a lost
> termination complete call?
A complete is only received in response to the submission of a
termination request, so they should never be set at the same time.
Doesn't really matter anyway since we submit a termination on resume anyway.
Daniele
>
> John.
>
>> +
>> + if (events & PXP_TERMINATION_REQUEST) {
>> + events &= ~PXP_TERMINATION_COMPLETE;
>> + pxp_terminate(pxp);
>> + }
>> +
>> + if (events & PXP_TERMINATION_COMPLETE)
>> + pxp_terminate_complete(pxp);
>> +
>> + if (events & PXP_TERMINATION_REQUEST)
>> + xe_pm_runtime_put(xe);
>> +}
>> +
>> +/**
>> + * xe_pxp_irq_handler - Handles PXP interrupts.
>> + * @pxp: pointer to pxp struct
>> + * @iir: interrupt vector
>> + */
>> +void xe_pxp_irq_handler(struct xe_device *xe, u16 iir)
>> +{
>> + struct xe_pxp *pxp = xe->pxp;
>> +
>> + if (!pxp_is_enabled(pxp)) {
>> + drm_err(&xe->drm, "PXP irq 0x%x received with PXP
>> disabled!\n", iir);
>> + return;
>> + }
>> +
>> + lockdep_assert_held(&xe->irq.lock);
>> +
>> + if (unlikely(!iir))
>> + return;
>> +
>> + if (iir & (KCR_PXP_STATE_TERMINATED_INTERRUPT |
>> + KCR_APP_TERMINATED_PER_FW_REQ_INTERRUPT))
>> + pxp->irq.events |= PXP_TERMINATION_REQUEST;
>> +
>> + if (iir & KCR_PXP_STATE_RESET_COMPLETE_INTERRUPT)
>> + pxp->irq.events |= PXP_TERMINATION_COMPLETE;
>> +
>> + if (pxp->irq.events)
>> + queue_work(pxp->irq.wq, &pxp->irq.work);
>> +}
>> +
>> static int kcr_pxp_set_status(const struct xe_pxp *pxp, bool enable)
>> {
>> u32 val = enable ?
>> _MASKED_BIT_ENABLE(KCR_INIT_ALLOW_DISPLAY_ME_WRITES) :
>> @@ -60,6 +184,7 @@ static void pxp_fini(void *arg)
>> {
>> struct xe_pxp *pxp = arg;
>> + destroy_workqueue(pxp->irq.wq);
>> xe_pxp_destroy_execution_resources(pxp);
>> /* no need to explicitly disable KCR since we're going to do
>> an FLR */
>> @@ -83,7 +208,7 @@ int xe_pxp_init(struct xe_device *xe)
>> struct xe_pxp *pxp;
>> int err;
>> - if (!pxp_is_supported(xe))
>> + if (!xe_pxp_is_supported(xe))
>> return -EOPNOTSUPP;
>> /* we only support PXP on single tile devices with a media GT */
>> @@ -105,12 +230,17 @@ int xe_pxp_init(struct xe_device *xe)
>> if (!pxp)
>> return -ENOMEM;
>> + INIT_WORK(&pxp->irq.work, pxp_irq_work);
>> pxp->xe = xe;
>> pxp->gt = gt;
>> + pxp->irq.wq = alloc_ordered_workqueue("pxp-wq", 0);
>> + if (!pxp->irq.wq)
>> + return -ENOMEM;
>> +
>> err = kcr_pxp_enable(pxp);
>> if (err)
>> - return err;
>> + goto out_wq;
>> err = xe_pxp_allocate_execution_resources(pxp);
>> if (err)
>> @@ -122,5 +252,7 @@ int xe_pxp_init(struct xe_device *xe)
>> kcr_disable:
>> kcr_pxp_disable(pxp);
>> +out_wq:
>> + destroy_workqueue(pxp->irq.wq);
>> return err;
>> }
>> diff --git a/drivers/gpu/drm/xe/xe_pxp.h b/drivers/gpu/drm/xe/xe_pxp.h
>> index 79c951667f13..81bafe2714ff 100644
>> --- a/drivers/gpu/drm/xe/xe_pxp.h
>> +++ b/drivers/gpu/drm/xe/xe_pxp.h
>> @@ -10,6 +10,9 @@
>> struct xe_device;
>> +bool xe_pxp_is_supported(const struct xe_device *xe);
>> +
>> int xe_pxp_init(struct xe_device *xe);
>> +void xe_pxp_irq_handler(struct xe_device *xe, u16 iir);
>> #endif /* __XE_PXP_H__ */
>> diff --git a/drivers/gpu/drm/xe/xe_pxp_types.h
>> b/drivers/gpu/drm/xe/xe_pxp_types.h
>> index 3463caaad101..d5cf8faed7be 100644
>> --- a/drivers/gpu/drm/xe/xe_pxp_types.h
>> +++ b/drivers/gpu/drm/xe/xe_pxp_types.h
>> @@ -8,6 +8,7 @@
>> #include <linux/iosys-map.h>
>> #include <linux/types.h>
>> +#include <linux/workqueue.h>
>> struct xe_bo;
>> struct xe_exec_queue;
>> @@ -69,6 +70,18 @@ struct xe_pxp {
>> /** @gsc_exec: kernel-owned objects for PXP submissions to
>> the GSCCS */
>> struct xe_pxp_gsc_client_resources gsc_res;
>> +
>> + /** @irq: wrapper for the worker and queue used for PXP irq
>> support */
>> + struct {
>> + /** @irq.work: worker that manages irq events. */
>> + struct work_struct work;
>> + /** @irq.wq: workqueue on which to queue the irq work. */
>> + struct workqueue_struct *wq;
>> + /** @irq.events: pending events, protected with
>> xe->irq.lock. */
>> + u32 events;
>> +#define PXP_TERMINATION_REQUEST BIT(0)
>> +#define PXP_TERMINATION_COMPLETE BIT(1)
>> + } irq;
>> };
>> #endif /* __XE_PXP_TYPES_H__ */
>
More information about the Intel-xe
mailing list