Re: [DPU PATCH 07/19] drm/msm: dpu_encoder_phys_vid: 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_vid with either a 
> DRM_* log
> message or a linux tracepoint.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
>  .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c  | 36 ++++++++-----------
>  drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h     | 35 ++++++++++++++++++
>  2 files changed, 50 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> index 73e59382eeac..fc83745b48fa 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> @@ -458,12 +458,8 @@ static int 
> dpu_encoder_phys_vid_control_vblank_irq(
>  		goto end;
>  	}
> 
> -	DPU_DEBUG_VIDENC(vid_enc, "[%pS] enable=%d/%d\n",
> -			__builtin_return_address(0),
> -			enable, atomic_read(&phys_enc->vblank_refcount));
> -
> -	DPU_EVT32(DRMID(phys_enc->parent), enable,
> -			atomic_read(&phys_enc->vblank_refcount));
> +	DRM_DEBUG_KMS("id:%u enable=%d/%d\n", DRMID(phys_enc->parent), 
> enable,
> +		      atomic_read(&phys_enc->vblank_refcount));
please use ["%s", enable ? "true" : "false"] to be consistent with 
dpu_encoder_phys_cmd.c
Reviewed-by: Rajesh Yadav <ryadav@codeaurora.org>
> 
>  	if (enable && atomic_inc_return(&phys_enc->vblank_refcount) == 1)
>  		ret = dpu_encoder_helper_register_irq(phys_enc, INTR_IDX_VSYNC);
> @@ -473,12 +469,10 @@ static int 
> dpu_encoder_phys_vid_control_vblank_irq(
> 
>  end:
>  	if (ret) {
> -		DPU_ERROR_VIDENC(vid_enc,
> -				"control vblank irq error %d, enable %d\n",
> -				ret, enable);
> -		DPU_EVT32(DRMID(phys_enc->parent),
> -				vid_enc->hw_intf->idx - INTF_0,
> -				enable, refcount, DPU_EVTLOG_ERROR);
> +		DRM_ERROR("failed: id:%u intf:%d ret:%d enable:%d refcnt:%d\n",
> +			  DRMID(phys_enc->parent),
> +			  vid_enc->hw_intf->idx - INTF_0, ret, enable,
> +			  refcount);
>  	}
>  	return ret;
>  }
> @@ -697,11 +691,9 @@ static void dpu_encoder_phys_vid_disable(struct
> dpu_encoder_phys *phys_enc)
>  		ret = _dpu_encoder_phys_vid_wait_for_vblank(phys_enc, false);
>  		if (ret) {
>  			atomic_set(&phys_enc->pending_kickoff_cnt, 0);
> -			DPU_ERROR_VIDENC(vid_enc,
> -					"failure waiting for disable: %d\n",
> -					ret);
> -			DPU_EVT32(DRMID(phys_enc->parent),
> -					vid_enc->hw_intf->idx - INTF_0, ret);
> +			DRM_ERROR("wait disable failed: id:%u intf:%d ret:%d\n",
> +				  DRMID(phys_enc->parent),
> +				  vid_enc->hw_intf->idx - INTF_0, ret);
>  		}
>  	}
> 
> @@ -727,8 +719,8 @@ static void 
> dpu_encoder_phys_vid_handle_post_kickoff(
>  	 * Video encoders need to turn on their interfaces now
>  	 */
>  	if (phys_enc->enable_state == DPU_ENC_ENABLING) {
> -		DPU_EVT32(DRMID(phys_enc->parent),
> -				vid_enc->hw_intf->idx - INTF_0);
> +		trace_dpu_enc_phys_vid_post_kickoff(DRMID(phys_enc->parent),
> +				    vid_enc->hw_intf->idx - INTF_0);
>  		spin_lock_irqsave(phys_enc->enc_spinlock, lock_flags);
>  		vid_enc->hw_intf->ops.enable_timing(vid_enc->hw_intf, 1);
>  		spin_unlock_irqrestore(phys_enc->enc_spinlock, lock_flags);
> @@ -747,8 +739,10 @@ static void
> dpu_encoder_phys_vid_irq_control(struct dpu_encoder_phys *phys_enc,
> 
>  	vid_enc = to_dpu_encoder_phys_vid(phys_enc);
> 
> -	DPU_EVT32(DRMID(phys_enc->parent), vid_enc->hw_intf->idx - INTF_0,
> -			enable, atomic_read(&phys_enc->vblank_refcount));
> +	trace_dpu_enc_phys_vid_irq_ctrl(DRMID(phys_enc->parent),
> +			    vid_enc->hw_intf->idx - INTF_0,
> +			    enable,
> +			    atomic_read(&phys_enc->vblank_refcount));
> 
>  	if (enable) {
>  		ret = dpu_encoder_phys_vid_control_vblank_irq(phys_enc, true);
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
> index a6313c4343c8..c9041e2a7aa1 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
> @@ -672,6 +672,41 @@ TRACE_EVENT(dpu_enc_phys_cmd_pdone_timeout,
>  		  __entry->kickoff_count, __entry->event)
>  );
> 
> +TRACE_EVENT(dpu_enc_phys_vid_post_kickoff,
> +	TP_PROTO(uint32_t drm_id, enum dpu_intf intf_idx),
> +	TP_ARGS(drm_id, intf_idx),
> +	TP_STRUCT__entry(
> +		__field(	uint32_t,	drm_id			)
> +		__field(	enum dpu_intf,	intf_idx		)
> +	),
> +	TP_fast_assign(
> +		__entry->drm_id = drm_id;
> +		__entry->intf_idx = intf_idx;
> +	),
> +	TP_printk("id=%u, intf_idx=%d", __entry->drm_id, __entry->intf_idx)
> +);
> +
> +TRACE_EVENT(dpu_enc_phys_vid_irq_ctrl,
> +	TP_PROTO(uint32_t drm_id, enum dpu_intf intf_idx, bool enable,
> +		 int refcnt),
> +	TP_ARGS(drm_id, intf_idx, enable, refcnt),
> +	TP_STRUCT__entry(
> +		__field(	uint32_t,	drm_id		)
> +		__field(	enum dpu_intf,	intf_idx	)
> +		__field(	bool,		enable		)
> +		__field(	int,		refcnt		)
> +	),
> +	TP_fast_assign(
> +		__entry->drm_id = drm_id;
> +		__entry->intf_idx = intf_idx;
> +		__entry->enable = enable;
> +		__entry->refcnt = refcnt;
> +	),
> +	TP_printk("id=%u, intf_idx=%d enable=%s refcnt=%d", __entry->drm_id,
> +		  __entry->intf_idx, __entry->enable ? "true" : "false",
> +		  __entry->drm_id)
> +);
> +
>  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