Re: [DPU PATCH 06/19] drm/msm: dpu_encoder_phys_cmd: Replace DPU_EVT with tracepoints

ryadav-sgV2jX0FEOL9JmXXK+q4OQ
2018-06-26T10:37:27Z

On 2018-06-21 02:18, Sean Paul wrote:
> This patch converts all DPU_EVTs in dpu_encoder_phys_cmd with either a
> DRM_* log
> message or a linux tracepoint.
>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
Reviewed-by: Rajesh Yadav <ryadav@codeaurora.org>
> ---
> .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 79 +++++++++----------
> drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 68 ++++++++++++++++
> 2 files changed, 104 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
> index 388de384e2cf..eb9314aaa85f 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
> @@ -108,8 +108,9 @@ static void
> dpu_encoder_phys_cmd_pp_tx_done_irq(void *arg, int irq_idx)
> new_cnt = atomic_add_unless(&phys_enc->pending_kickoff_cnt, -1, 0);
> spin_unlock_irqrestore(phys_enc->enc_spinlock, lock_flags);
>
> - DPU_EVT32_IRQ(DRMID(phys_enc->parent),
> - phys_enc->hw_pp->idx - PINGPONG_0, new_cnt, event);
> + trace_dpu_enc_phys_cmd_pp_tx_done(DRMID(phys_enc->parent),
> + phys_enc->hw_pp->idx - PINGPONG_0,
> + new_cnt, event);
>
> /* Signal any waiting atomic commit thread */
> wake_up_all(&phys_enc->pending_kickoff_wq);
> @@ -245,21 +246,20 @@ static int
> _dpu_encoder_phys_cmd_handle_ppdone_timeout(
> do_log = true;
> }
>
> - DPU_EVT32(DRMID(phys_enc->parent), phys_enc->hw_pp->idx - PINGPONG_0,
> - cmd_enc->pp_timeout_report_cnt,
> - atomic_read(&phys_enc->pending_kickoff_cnt),
> - frame_event);
> + trace_dpu_enc_phys_cmd_pdone_timeout(DRMID(phys_enc->parent),
> + phys_enc->hw_pp->idx - PINGPONG_0,
> + cmd_enc->pp_timeout_report_cnt,
> + atomic_read(&phys_enc->pending_kickoff_cnt),
> + frame_event);
>
> /* to avoid flooding, only log first time, and "dead" time */
> if (do_log) {
> - DPU_ERROR_CMDENC(cmd_enc,
> - "pp:%d kickoff timed out ctl %d cnt %d koff_cnt %d\n",
> - phys_enc->hw_pp->idx - PINGPONG_0,
> - phys_enc->hw_ctl->idx - CTL_0,
> - cmd_enc->pp_timeout_report_cnt,
> - atomic_read(&phys_enc->pending_kickoff_cnt));
> -
> - DPU_EVT32(DRMID(phys_enc->parent), DPU_EVTLOG_FATAL);
> + DRM_ERROR("id:%d pp:%d kickoff timeout %d cnt %d koff_cnt %d\n",
> + DRMID(phys_enc->parent),
> + phys_enc->hw_pp->idx - PINGPONG_0,
> + phys_enc->hw_ctl->idx - CTL_0,
> + cmd_enc->pp_timeout_report_cnt,
> + atomic_read(&phys_enc->pending_kickoff_cnt));
>
> dpu_encoder_helper_unregister_irq(phys_enc, INTR_IDX_RDPTR);
> DPU_DBG_DUMP("all", "dbg_bus", "vbif_dbg_bus", "panic");
> @@ -308,8 +308,6 @@ static int dpu_encoder_phys_cmd_control_vblank_irq(
> struct dpu_encoder_phys *phys_enc,
> bool enable)
> {
> - struct dpu_encoder_phys_cmd *cmd_enc =
> - to_dpu_encoder_phys_cmd(phys_enc);
> int ret = 0;
> int refcount;
>
> @@ -330,10 +328,9 @@ static int
> dpu_encoder_phys_cmd_control_vblank_irq(
> goto end;
> }
>
> - DPU_DEBUG_CMDENC(cmd_enc, "[%pS] enable=%d/%d\n",
> - __builtin_return_address(0), enable, refcount);
> - DPU_EVT32(DRMID(phys_enc->parent), phys_enc->hw_pp->idx - PINGPONG_0,
> - enable, refcount);
> + DRM_DEBUG_KMS("id:%u pp:%d enable=%s/%d\n", DRMID(phys_enc->parent),
> + phys_enc->hw_pp->idx - PINGPONG_0,
> + enable ? "true" : "false", refcount);
>
> if (enable && atomic_inc_return(&phys_enc->vblank_refcount) == 1)
> ret = dpu_encoder_helper_register_irq(phys_enc, INTR_IDX_RDPTR);
> @@ -343,12 +340,10 @@ static int
> dpu_encoder_phys_cmd_control_vblank_irq(
>
> end:
> if (ret) {
> - DPU_ERROR_CMDENC(cmd_enc,
> - "control vblank irq error %d, enable %d, refcount %d\n",
> - ret, enable, refcount);
> - DPU_EVT32(DRMID(phys_enc->parent),
> - phys_enc->hw_pp->idx - PINGPONG_0,
> - enable, refcount, DPU_EVTLOG_ERROR);
> + DRM_ERROR("vblank irq err id:%u pp:%d ret:%d, enable %s/%d\n",
> + DRMID(phys_enc->parent),
> + phys_enc->hw_pp->idx - PINGPONG_0, ret,
> + enable ? "true" : "false", refcount);
> }
>
> return ret;
> @@ -364,7 +359,8 @@ void dpu_encoder_phys_cmd_irq_control(struct
> dpu_encoder_phys *phys_enc,
>
> cmd_enc = to_dpu_encoder_phys_cmd(phys_enc);
>
> - DPU_EVT32(DRMID(phys_enc->parent), phys_enc->hw_pp->idx - PINGPONG_0,
> + trace_dpu_enc_phys_cmd_irq_ctrl(DRMID(phys_enc->parent),
> + phys_enc->hw_pp->idx - PINGPONG_0,
> enable, atomic_read(&phys_enc->vblank_refcount));
>
> if (enable) {
> @@ -557,7 +553,7 @@ static void _dpu_encoder_phys_cmd_connect_te(
> !phys_enc->hw_pp->ops.connect_external_te)
> return;
>
> - DPU_EVT32(DRMID(phys_enc->parent), enable);
> + trace_dpu_enc_phys_cmd_connect_te(DRMID(phys_enc->parent), enable);
> phys_enc->hw_pp->ops.connect_external_te(phys_enc->hw_pp, enable);
> }
>
> @@ -594,11 +590,9 @@ static void dpu_encoder_phys_cmd_disable(struct
> dpu_encoder_phys *phys_enc)
> DPU_ERROR("invalid encoder\n");
> return;
> }
> - DPU_DEBUG_CMDENC(cmd_enc, "pp %d state %d\n",
> - phys_enc->hw_pp->idx - PINGPONG_0,
> - phys_enc->enable_state);
> - DPU_EVT32(DRMID(phys_enc->parent), phys_enc->hw_pp->idx - PINGPONG_0,
> - phys_enc->enable_state);
> + DRM_DEBUG_KMS("id:%u pp:%d state:%d\n", DRMID(phys_enc->parent),
> + phys_enc->hw_pp->idx - PINGPONG_0,
> + phys_enc->enable_state);
>
> if (phys_enc->enable_state == DPU_ENC_DISABLED) {
> DPU_ERROR_CMDENC(cmd_enc, "already disabled\n");
> @@ -656,10 +650,9 @@ static void
> dpu_encoder_phys_cmd_prepare_for_kickoff(
> DPU_ERROR("invalid encoder\n");
> return;
> }
> - DPU_DEBUG_CMDENC(cmd_enc, "pp %d\n", phys_enc->hw_pp->idx -
> PINGPONG_0);
> -
> - DPU_EVT32(DRMID(phys_enc->parent), phys_enc->hw_pp->idx - PINGPONG_0,
> - atomic_read(&phys_enc->pending_kickoff_cnt));
> + DRM_DEBUG_KMS("id:%u pp:%d pending_cnt:%d\n",
> DRMID(phys_enc->parent),
> + phys_enc->hw_pp->idx - PINGPONG_0,
> + atomic_read(&phys_enc->pending_kickoff_cnt));
>
> /*
> * Mark kickoff request as outstanding. If there are more than one,
> @@ -669,9 +662,9 @@ static void
> dpu_encoder_phys_cmd_prepare_for_kickoff(
> if (ret) {
> /* force pending_kickoff_cnt 0 to discard failed kickoff */
> atomic_set(&phys_enc->pending_kickoff_cnt, 0);
> - DPU_EVT32(DRMID(phys_enc->parent),
> - phys_enc->hw_pp->idx - PINGPONG_0);
> - DPU_ERROR("failed wait_for_idle: %d\n", ret);
> + DRM_ERROR("failed wait_for_idle: id:%u ret:%d pp:%d\n",
> + DRMID(phys_enc->parent), ret,
> + phys_enc->hw_pp->idx - PINGPONG_0);
> }
>
> DPU_DEBUG_CMDENC(cmd_enc, "pp:%d pending_cnt %d\n",
> @@ -720,9 +713,9 @@ static int
> dpu_encoder_phys_cmd_wait_for_tx_complete(
>
> rc = _dpu_encoder_phys_cmd_wait_for_idle(phys_enc);
> if (rc) {
> - DPU_EVT32(DRMID(phys_enc->parent),
> - phys_enc->intf_idx - INTF_0);
> - DPU_ERROR("failed wait_for_idle: %d\n", rc);
> + DRM_ERROR("failed wait_for_idle: id:%u ret:%d intf:%d\n",
> + DRMID(phys_enc->parent), rc,
> + phys_enc->intf_idx - INTF_0);
> }
>
> return rc;
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
> index 7169ff3a9805..a6313c4343c8 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
> @@ -437,6 +437,10 @@ DEFINE_EVENT(dpu_enc_id_enable_template,
> dpu_enc_frame_event_cb,
> TP_PROTO(uint32_t drm_id, bool enable),
> TP_ARGS(drm_id, enable)
> );
> +DEFINE_EVENT(dpu_enc_id_enable_template, dpu_enc_phys_cmd_connect_te,
> + TP_PROTO(uint32_t drm_id, bool enable),
> + TP_ARGS(drm_id, enable)
> +);
>
> TRACE_EVENT(dpu_enc_rc,
> TP_PROTO(uint32_t drm_id, u32 sw_event, bool idle_pc_supported,
> @@ -604,6 +608,70 @@ TRACE_EVENT(dpu_enc_wait_event_timeout,
> __entry->expected_time, __entry->atomic_cnt)
> );
>
> +TRACE_EVENT(dpu_enc_phys_cmd_irq_ctrl,
> + TP_PROTO(uint32_t drm_id, enum dpu_pingpong pp, bool enable,
> + int refcnt),
> + TP_ARGS(drm_id, pp, enable, refcnt),
> + TP_STRUCT__entry(
> + __field( uint32_t, drm_id )
> + __field( enum dpu_pingpong, pp )
> + __field( bool, enable )
> + __field( int, refcnt )
> + ),
> + TP_fast_assign(
> + __entry->drm_id = drm_id;
> + __entry->pp = pp;
> + __entry->enable = enable;
> + __entry->refcnt = refcnt;
> + ),
> + TP_printk("id=%u, pp=%d, enable=%s, refcnt=%d", __entry->drm_id,
> + __entry->pp, __entry->enable ? "true" : "false",
> + __entry->refcnt)
> +);
> +
> +TRACE_EVENT(dpu_enc_phys_cmd_pp_tx_done,
> + TP_PROTO(uint32_t drm_id, enum dpu_pingpong pp, int new_count,
> + u32 event),
> + TP_ARGS(drm_id, pp, new_count, event),
> + TP_STRUCT__entry(
> + __field( uint32_t, drm_id )
> + __field( enum dpu_pingpong, pp )
> + __field( int, new_count )
> + __field( u32, event )
> + ),
> + TP_fast_assign(
> + __entry->drm_id = drm_id;
> + __entry->pp = pp;
> + __entry->new_count = new_count;
> + __entry->event = event;
> + ),
> + TP_printk("id=%u, pp=%d, new_count=%d, event=%u", __entry->drm_id,
> + __entry->pp, __entry->new_count, __entry->event)
> +);
> +
> +TRACE_EVENT(dpu_enc_phys_cmd_pdone_timeout,
> + TP_PROTO(uint32_t drm_id, enum dpu_pingpong pp, int timeout_count,
> + int kickoff_count, u32 event),
> + TP_ARGS(drm_id, pp, timeout_count, kickoff_count, event),
> + TP_STRUCT__entry(
> + __field( uint32_t, drm_id )
> + __field( enum dpu_pingpong, pp )
> + __field( int, timeout_count )
> + __field( int, kickoff_count )
> + __field( u32, event )
> + ),
> + TP_fast_assign(
> + __entry->drm_id = drm_id;
> + __entry->pp = pp;
> + __entry->timeout_count = timeout_count;
> + __entry->kickoff_count = kickoff_count;
> + __entry->event = event;
> + ),
> + TP_printk("id=%u, pp=%d, timeout_count=%d, kickoff_count=%d,
> event=%u",
> + __entry->drm_id, __entry->pp, __entry->timeout_count,
> + __entry->kickoff_count, __entry->event)
> +);
> +
> TRACE_EVENT(dpu_crtc_setup_mixer,
> TP_PROTO(uint32_t crtc_id, uint32_t plane_id,
> struct drm_plane_state *state, struct dpu_plane_state *pstate,