Re: [DPU PATCH 07/19] drm/msm: dpu_encoder_phys_vid: Replace DPU_EVT with tracepoints

ryadav-sgV2jX0FEOL9JmXXK+q4OQ
2018-06-26T10:47:26Z

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,