[Intel-gfx] [PATCH v5] drm/i915/pxp: Add drm_dbgs for critical PXP events.
Tvrtko Ursulin
tvrtko.ursulin at linux.intel.com
Fri Nov 24 08:30:35 UTC 2023
On 22/11/2023 19:15, Alan Previn wrote:
> Debugging PXP issues can't even begin without understanding precedding
> sequence of important events. Add drm_dbg into the most important PXP
> events.
>
> v5 : - rebase.
> v4 : - rebase.
> v3 : - move gt_dbg to after mutex block in function
> i915_gsc_proxy_component_bind. (Vivaik)
> v2 : - remove __func__ since drm_dbg covers that (Jani).
> - add timeout dbg of the restart from front-end (Alan).
>
> Signed-off-by: Alan Previn <alan.previn.teres.alexis at intel.com>
> Reviewed-by: Vivaik Balasubrawmanian <vivaik.balasubrawmanian at intel.com>
> ---
> drivers/gpu/drm/i915/gt/uc/intel_gsc_proxy.c | 2 ++
> drivers/gpu/drm/i915/pxp/intel_pxp.c | 15 ++++++++++++---
> drivers/gpu/drm/i915/pxp/intel_pxp_irq.c | 5 +++--
> drivers/gpu/drm/i915/pxp/intel_pxp_session.c | 6 +++++-
> drivers/gpu/drm/i915/pxp/intel_pxp_types.h | 1 +
> 5 files changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_gsc_proxy.c b/drivers/gpu/drm/i915/gt/uc/intel_gsc_proxy.c
> index 5f138de3c14f..40817ebcca71 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_gsc_proxy.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_gsc_proxy.c
> @@ -322,6 +322,7 @@ static int i915_gsc_proxy_component_bind(struct device *i915_kdev,
> gsc->proxy.component = data;
> gsc->proxy.component->mei_dev = mei_kdev;
> mutex_unlock(&gsc->proxy.mutex);
> + gt_dbg(gt, "GSC proxy mei component bound\n");
>
> return 0;
> }
> @@ -342,6 +343,7 @@ static void i915_gsc_proxy_component_unbind(struct device *i915_kdev,
> with_intel_runtime_pm(&i915->runtime_pm, wakeref)
> intel_uncore_rmw(gt->uncore, HECI_H_CSR(MTL_GSC_HECI2_BASE),
> HECI_H_CSR_IE | HECI_H_CSR_RST, 0);
> + gt_dbg(gt, "GSC proxy mei component unbound\n");
> }
>
> static const struct component_ops i915_gsc_proxy_component_ops = {
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.c b/drivers/gpu/drm/i915/pxp/intel_pxp.c
> index dc327cf40b5a..e11f562b1876 100644
> --- a/drivers/gpu/drm/i915/pxp/intel_pxp.c
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp.c
> @@ -303,6 +303,8 @@ static int __pxp_global_teardown_final(struct intel_pxp *pxp)
>
> if (!pxp->arb_is_valid)
> return 0;
> +
> + drm_dbg(&pxp->ctrl_gt->i915->drm, "PXP: teardown for suspend/fini");
> /*
> * To ensure synchronous and coherent session teardown completion
> * in response to suspend or shutdown triggers, don't use a worker.
> @@ -324,6 +326,8 @@ static int __pxp_global_teardown_restart(struct intel_pxp *pxp)
>
> if (pxp->arb_is_valid)
> return 0;
> +
> + drm_dbg(&pxp->ctrl_gt->i915->drm, "PXP: teardown for restart");
> /*
> * The arb-session is currently inactive and we are doing a reset and restart
> * due to a runtime event. Use the worker that was designed for this.
> @@ -332,8 +336,11 @@ static int __pxp_global_teardown_restart(struct intel_pxp *pxp)
>
> timeout = intel_pxp_get_backend_timeout_ms(pxp);
>
> - if (!wait_for_completion_timeout(&pxp->termination, msecs_to_jiffies(timeout)))
> + if (!wait_for_completion_timeout(&pxp->termination, msecs_to_jiffies(timeout))) {
> + drm_dbg(&pxp->ctrl_gt->i915->drm, "PXP: restart backend timed out (%d ms)",
> + timeout);
> return -ETIMEDOUT;
> + }
>
> return 0;
> }
> @@ -414,10 +421,12 @@ int intel_pxp_start(struct intel_pxp *pxp)
> int ret = 0;
>
> ret = intel_pxp_get_readiness_status(pxp, PXP_READINESS_TIMEOUT);
> - if (ret < 0)
> + if (ret < 0) {
> + drm_dbg(&pxp->ctrl_gt->i915->drm, "PXP: tried but not-avail (%d)", ret);
> return ret;
> - else if (ret > 1)
> + } else if (ret > 1) {
> return -EIO; /* per UAPI spec, user may retry later */
> + }
>
> mutex_lock(&pxp->arb_mutex);
>
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c b/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c
> index 91e9622c07d0..d81750b9bdda 100644
> --- a/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c
> @@ -40,11 +40,12 @@ void intel_pxp_irq_handler(struct intel_pxp *pxp, u16 iir)
> GEN12_DISPLAY_APP_TERMINATED_PER_FW_REQ_INTERRUPT)) {
> /* immediately mark PXP as inactive on termination */
> intel_pxp_mark_termination_in_progress(pxp);
> - pxp->session_events |= PXP_TERMINATION_REQUEST | PXP_INVAL_REQUIRED;
> + pxp->session_events |= PXP_TERMINATION_REQUEST | PXP_INVAL_REQUIRED |
> + PXP_EVENT_TYPE_IRQ;
> }
>
> if (iir & GEN12_DISPLAY_STATE_RESET_COMPLETE_INTERRUPT)
> - pxp->session_events |= PXP_TERMINATION_COMPLETE;
> + pxp->session_events |= PXP_TERMINATION_COMPLETE | PXP_EVENT_TYPE_IRQ;
This looked to be doing more than adding debug logs, but then no one is
actually consuming this new flag?!
Regards,
Tvrtko
>
> if (pxp->session_events)
> queue_work(system_unbound_wq, &pxp->session_work);
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_session.c b/drivers/gpu/drm/i915/pxp/intel_pxp_session.c
> index 0a3e66b0265e..091c86e03d1a 100644
> --- a/drivers/gpu/drm/i915/pxp/intel_pxp_session.c
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_session.c
> @@ -137,8 +137,10 @@ void intel_pxp_terminate(struct intel_pxp *pxp, bool post_invalidation_needs_res
> static void pxp_terminate_complete(struct intel_pxp *pxp)
> {
> /* Re-create the arb session after teardown handle complete */
> - if (fetch_and_zero(&pxp->hw_state_invalidated))
> + if (fetch_and_zero(&pxp->hw_state_invalidated)) {
> + drm_dbg(&pxp->ctrl_gt->i915->drm, "PXP: creating arb_session after invalidation");
> pxp_create_arb_session(pxp);
> + }
>
> complete_all(&pxp->termination);
> }
> @@ -157,6 +159,8 @@ static void pxp_session_work(struct work_struct *work)
> if (!events)
> return;
>
> + drm_dbg(>->i915->drm, "PXP: processing event-flags 0x%08x", events);
> +
> if (events & PXP_INVAL_REQUIRED)
> intel_pxp_invalidate(pxp);
>
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_types.h b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h
> index 7e11fa8034b2..07864b584cf4 100644
> --- a/drivers/gpu/drm/i915/pxp/intel_pxp_types.h
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_types.h
> @@ -124,6 +124,7 @@ struct intel_pxp {
> #define PXP_TERMINATION_REQUEST BIT(0)
> #define PXP_TERMINATION_COMPLETE BIT(1)
> #define PXP_INVAL_REQUIRED BIT(2)
> +#define PXP_EVENT_TYPE_IRQ BIT(3)
> };
>
> #endif /* __INTEL_PXP_TYPES_H__ */
>
> base-commit: 5429d55de723544dfc0630cf39d96392052b27a1
More information about the dri-devel
mailing list