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

[ryadav-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org]

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,
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno