From: Sean Paul seanpaul@chromium.org
We're printing the frame_busy_mask in a trace, but after it's been cleared. This, as it turns out, is pretty pointless.
Signed-off-by: Sean Paul seanpaul@chromium.org --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index c2e8985b4c54..e56f29190121 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -1361,9 +1361,9 @@ static void dpu_encoder_frame_done_callback( /* One of the physical encoders has become idle */ for (i = 0; i < dpu_enc->num_phys_encs; i++) { if (dpu_enc->phys_encs[i] == ready_phys) { - clear_bit(i, dpu_enc->frame_busy_mask); trace_dpu_enc_frame_done_cb(DRMID(drm_enc), i, dpu_enc->frame_busy_mask[0]); + clear_bit(i, dpu_enc->frame_busy_mask); } }
From: Sean Paul seanpaul@chromium.org
It's useful to know which bits of the flush come from "extra_flush_bits"
Signed-off-by: Sean Paul seanpaul@chromium.org --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 3 ++- drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 14 +++++++++----- 2 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index e56f29190121..8f6880db5c99 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -1444,7 +1444,8 @@ static inline void _dpu_encoder_trigger_flush(struct drm_encoder *drm_enc, ret = ctl->ops.get_pending_flush(ctl);
trace_dpu_enc_trigger_flush(DRMID(drm_enc), phys->intf_idx, - pending_kickoff_cnt, ctl->idx, ret); + pending_kickoff_cnt, ctl->idx, + extra_flush_bits, ret); }
/** diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h index 0be51db02f2e..ae6b0c51ba52 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h @@ -468,14 +468,16 @@ TRACE_EVENT(dpu_enc_frame_done_cb,
TRACE_EVENT(dpu_enc_trigger_flush, TP_PROTO(uint32_t drm_id, enum dpu_intf intf_idx, - int pending_kickoff_cnt, int ctl_idx, u32 pending_flush_ret), + int pending_kickoff_cnt, int ctl_idx, u32 extra_flush_bits, + u32 pending_flush_ret), TP_ARGS(drm_id, intf_idx, pending_kickoff_cnt, ctl_idx, - pending_flush_ret), + extra_flush_bits, pending_flush_ret), TP_STRUCT__entry( __field( uint32_t, drm_id ) __field( enum dpu_intf, intf_idx ) __field( int, pending_kickoff_cnt ) __field( int, ctl_idx ) + __field( u32, extra_flush_bits ) __field( u32, pending_flush_ret ) ), TP_fast_assign( @@ -483,12 +485,14 @@ TRACE_EVENT(dpu_enc_trigger_flush, __entry->intf_idx = intf_idx; __entry->pending_kickoff_cnt = pending_kickoff_cnt; __entry->ctl_idx = ctl_idx; + __entry->extra_flush_bits = extra_flush_bits; __entry->pending_flush_ret = pending_flush_ret; ), TP_printk("id=%u, intf_idx=%d, pending_kickoff_cnt=%d ctl_idx=%d " - "pending_flush_ret=%u", __entry->drm_id, - __entry->intf_idx, __entry->pending_kickoff_cnt, - __entry->ctl_idx, __entry->pending_flush_ret) + "extra_flush_bits=0x%x pending_flush_ret=0x%x", + __entry->drm_id, __entry->intf_idx, + __entry->pending_kickoff_cnt, __entry->ctl_idx, + __entry->extra_flush_bits, __entry->pending_flush_ret) );
DECLARE_EVENT_CLASS(dpu_enc_ktime_template,
On 2018-09-19 11:33, Sean Paul wrote:
From: Sean Paul seanpaul@chromium.org
It's useful to know which bits of the flush come from "extra_flush_bits"
Signed-off-by: Sean Paul seanpaul@chromium.org
Reviewed-by: Abhinav Kumar abhinavk@codeaurora.org
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 3 ++- drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 14 +++++++++----- 2 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index e56f29190121..8f6880db5c99 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -1444,7 +1444,8 @@ static inline void _dpu_encoder_trigger_flush(struct drm_encoder *drm_enc, ret = ctl->ops.get_pending_flush(ctl);
trace_dpu_enc_trigger_flush(DRMID(drm_enc), phys->intf_idx,
pending_kickoff_cnt, ctl->idx, ret);
pending_kickoff_cnt, ctl->idx,extra_flush_bits, ret);}
/** diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h index 0be51db02f2e..ae6b0c51ba52 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h @@ -468,14 +468,16 @@ TRACE_EVENT(dpu_enc_frame_done_cb,
TRACE_EVENT(dpu_enc_trigger_flush, TP_PROTO(uint32_t drm_id, enum dpu_intf intf_idx,
int pending_kickoff_cnt, int ctl_idx, u32 pending_flush_ret),
int pending_kickoff_cnt, int ctl_idx, u32 extra_flush_bits,u32 pending_flush_ret),
pending_flush_ret),
extra_flush_bits, pending_flush_ret),__field( u32, extra_flush_bits )@@ -483,12 +485,14 @@ TRACE_EVENT(dpu_enc_trigger_flush, __entry->intf_idx = intf_idx; __entry->pending_kickoff_cnt = pending_kickoff_cnt; __entry->ctl_idx = ctl_idx;
__entry->extra_flush_bits = extra_flush_bits;
"pending_flush_ret=%u", __entry->drm_id,__entry->intf_idx, __entry->pending_kickoff_cnt,__entry->ctl_idx, __entry->pending_flush_ret)
"extra_flush_bits=0x%x pending_flush_ret=0x%x",__entry->drm_id, __entry->intf_idx,__entry->pending_kickoff_cnt, __entry->ctl_idx,__entry->extra_flush_bits, __entry->pending_flush_ret));
DECLARE_EVENT_CLASS(dpu_enc_ktime_template,
From: Sean Paul seanpaul@chromium.org
TP_printk is not synchronous, so storing pointers and then later derferencing them is a Bad Idea. This patch stores everything locally to avoid display stomped memory.
Signed-off-by: Sean Paul seanpaul@chromium.org --- drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 98 +++++++++++++---------- 1 file changed, 55 insertions(+), 43 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h index ae6b0c51ba52..e12c4cefb742 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h @@ -686,37 +686,41 @@ TRACE_EVENT(dpu_crtc_setup_mixer, TP_STRUCT__entry( __field( uint32_t, crtc_id ) __field( uint32_t, plane_id ) - __field( struct drm_plane_state*,state ) - __field( struct dpu_plane_state*,pstate ) + __field( uint32_t, fb_id ) + __field_struct( struct drm_rect, src_rect ) + __field_struct( struct drm_rect, dst_rect ) __field( uint32_t, stage_idx ) + __field( enum dpu_stage, stage ) __field( enum dpu_sspp, sspp ) + __field( uint32_t, multirect_idx ) + __field( uint32_t, multirect_mode ) __field( uint32_t, pixel_format ) __field( uint64_t, modifier ) ), TP_fast_assign( __entry->crtc_id = crtc_id; __entry->plane_id = plane_id; - __entry->state = state; - __entry->pstate = pstate; + __entry->fb_id = state ? state->fb->base.id : 0; + __entry->src_rect = drm_plane_state_src(state); + __entry->dst_rect = drm_plane_state_dest(state); __entry->stage_idx = stage_idx; + __entry->stage = pstate->stage; __entry->sspp = sspp; + __entry->multirect_idx = pstate->multirect_index; + __entry->multirect_mode = pstate->multirect_mode; __entry->pixel_format = pixel_format; __entry->modifier = modifier; ), - TP_printk("crtc_id:%u plane_id:%u fb_id:%u src:{%ux%u+%ux%u} " - "dst:{%ux%u+%ux%u} stage_idx:%u stage:%d, sspp:%d " + TP_printk("crtc_id:%u plane_id:%u fb_id:%u src:" DRM_RECT_FP_FMT + " dst:" DRM_RECT_FMT " stage_idx:%u stage:%d, sspp:%d " "multirect_index:%d multirect_mode:%u pix_format:%u " "modifier:%llu", - __entry->crtc_id, __entry->plane_id, - __entry->state->fb ? __entry->state->fb->base.id : -1, - __entry->state->src_w >> 16, __entry->state->src_h >> 16, - __entry->state->src_x >> 16, __entry->state->src_y >> 16, - __entry->state->crtc_w, __entry->state->crtc_h, - __entry->state->crtc_x, __entry->state->crtc_y, - __entry->stage_idx, __entry->pstate->stage, __entry->sspp, - __entry->pstate->multirect_index, - __entry->pstate->multirect_mode, __entry->pixel_format, - __entry->modifier) + __entry->crtc_id, __entry->plane_id, __entry->fb_id, + DRM_RECT_FP_ARG(&__entry->src_rect), + DRM_RECT_ARG(&__entry->dst_rect), + __entry->stage_idx, __entry->stage, __entry->sspp, + __entry->multirect_idx, __entry->multirect_mode, + __entry->pixel_format, __entry->modifier) );
TRACE_EVENT(dpu_crtc_setup_lm_bounds, @@ -725,15 +729,15 @@ TRACE_EVENT(dpu_crtc_setup_lm_bounds, TP_STRUCT__entry( __field( uint32_t, drm_id ) __field( int, mixer ) - __field( struct drm_rect *, bounds ) + __field_struct( struct drm_rect, bounds ) ), TP_fast_assign( __entry->drm_id = drm_id; __entry->mixer = mixer; - __entry->bounds = bounds; + __entry->bounds = *bounds; ), TP_printk("id:%u mixer:%d bounds:" DRM_RECT_FMT, __entry->drm_id, - __entry->mixer, DRM_RECT_ARG(__entry->bounds)) + __entry->mixer, DRM_RECT_ARG(&__entry->bounds)) );
TRACE_EVENT(dpu_crtc_vblank_enable, @@ -744,21 +748,25 @@ TRACE_EVENT(dpu_crtc_vblank_enable, __field( uint32_t, drm_id ) __field( uint32_t, enc_id ) __field( bool, enable ) - __field( struct dpu_crtc *, crtc ) + __field( bool, enabled ) + __field( bool, suspend ) + __field( bool, vblank_requested ) ), TP_fast_assign( __entry->drm_id = drm_id; __entry->enc_id = enc_id; __entry->enable = enable; - __entry->crtc = crtc; + __entry->enabled = crtc->enabled; + __entry->suspend = crtc->suspend; + __entry->vblank_requested = crtc->vblank_requested; ), TP_printk("id:%u encoder:%u enable:%s state{enabled:%s suspend:%s " "vblank_req:%s}", __entry->drm_id, __entry->enc_id, __entry->enable ? "true" : "false", - __entry->crtc->enabled ? "true" : "false", - __entry->crtc->suspend ? "true" : "false", - __entry->crtc->vblank_requested ? "true" : "false") + __entry->enabled ? "true" : "false", + __entry->suspend ? "true" : "false", + __entry->vblank_requested ? "true" : "false") );
DECLARE_EVENT_CLASS(dpu_crtc_enable_template, @@ -767,18 +775,22 @@ DECLARE_EVENT_CLASS(dpu_crtc_enable_template, TP_STRUCT__entry( __field( uint32_t, drm_id ) __field( bool, enable ) - __field( struct dpu_crtc *, crtc ) + __field( bool, enabled ) + __field( bool, suspend ) + __field( bool, vblank_requested ) ), TP_fast_assign( __entry->drm_id = drm_id; __entry->enable = enable; - __entry->crtc = crtc; + __entry->enabled = crtc->enabled; + __entry->suspend = crtc->suspend; + __entry->vblank_requested = crtc->vblank_requested; ), TP_printk("id:%u enable:%s state{enabled:%s suspend:%s vblank_req:%s}", __entry->drm_id, __entry->enable ? "true" : "false", - __entry->crtc->enabled ? "true" : "false", - __entry->crtc->suspend ? "true" : "false", - __entry->crtc->vblank_requested ? "true" : "false") + __entry->enabled ? "true" : "false", + __entry->suspend ? "true" : "false", + __entry->vblank_requested ? "true" : "false") ); DEFINE_EVENT(dpu_crtc_enable_template, dpu_crtc_set_suspend, TP_PROTO(uint32_t drm_id, bool enable, struct dpu_crtc *crtc), @@ -818,24 +830,24 @@ TRACE_EVENT(dpu_plane_set_scanout, TP_ARGS(index, layout, multirect_index), TP_STRUCT__entry( __field( enum dpu_sspp, index ) - __field( struct dpu_hw_fmt_layout*, layout ) + __field_struct( struct dpu_hw_fmt_layout, layout ) __field( enum dpu_sspp_multirect_index, multirect_index) ), TP_fast_assign( __entry->index = index; - __entry->layout = layout; + __entry->layout = *layout; __entry->multirect_index = multirect_index; ), TP_printk("index:%d layout:{%ux%u @ [%u/%u, %u/%u, %u/%u, %u/%u]} " - "multirect_index:%d", __entry->index, __entry->layout->width, - __entry->layout->height, __entry->layout->plane_addr[0], - __entry->layout->plane_size[0], - __entry->layout->plane_addr[1], - __entry->layout->plane_size[1], - __entry->layout->plane_addr[2], - __entry->layout->plane_size[2], - __entry->layout->plane_addr[3], - __entry->layout->plane_size[3], __entry->multirect_index) + "multirect_index:%d", __entry->index, __entry->layout.width, + __entry->layout.height, __entry->layout.plane_addr[0], + __entry->layout.plane_size[0], + __entry->layout.plane_addr[1], + __entry->layout.plane_size[1], + __entry->layout.plane_addr[2], + __entry->layout.plane_size[2], + __entry->layout.plane_addr[3], + __entry->layout.plane_size[3], __entry->multirect_index) );
TRACE_EVENT(dpu_plane_disable, @@ -979,16 +991,16 @@ TRACE_EVENT(dpu_core_perf_update_clk, TP_PROTO(struct drm_device *dev, bool stop_req, u64 clk_rate), TP_ARGS(dev, stop_req, clk_rate), TP_STRUCT__entry( - __field( struct drm_device *, dev ) + __string( dev_name, dev->unique ) __field( bool, stop_req ) __field( u64, clk_rate ) ), TP_fast_assign( - __entry->dev = dev; + __assign_str(dev_name, dev->unique); __entry->stop_req = stop_req; __entry->clk_rate = clk_rate; ), - TP_printk("dev:%s stop_req:%s clk_rate:%llu", __entry->dev->unique, + TP_printk("dev:%s stop_req:%s clk_rate:%llu", __get_str(dev_name), __entry->stop_req ? "true" : "false", __entry->clk_rate) );
On 2018-09-19 11:33, Sean Paul wrote:
From: Sean Paul seanpaul@chromium.org
TP_printk is not synchronous, so storing pointers and then later derferencing them is a Bad Idea. This patch stores everything locally to
minor typo "dereferencing",
avoid display stomped memory.
Signed-off-by: Sean Paul seanpaul@chromium.org
After fixing the minor nit please add, Reviewed-by: Abhinav Kumar abhinavk@codeaurora.org
drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 98 +++++++++++++---------- 1 file changed, 55 insertions(+), 43 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h index ae6b0c51ba52..e12c4cefb742 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h @@ -686,37 +686,41 @@ TRACE_EVENT(dpu_crtc_setup_mixer, TP_STRUCT__entry( __field( uint32_t, crtc_id ) __field( uint32_t, plane_id )
__field( struct drm_plane_state*,state )__field( struct dpu_plane_state*,pstate )
__field( uint32_t, fb_id )__field_struct( struct drm_rect, src_rect )__field_struct( struct drm_rect, dst_rect )__field( enum dpu_stage, stage )__field( uint32_t, multirect_idx )__field( uint32_t, multirect_mode )
__entry->state = state;__entry->pstate = pstate;
__entry->fb_id = state ? state->fb->base.id : 0;__entry->src_rect = drm_plane_state_src(state);__entry->dst_rect = drm_plane_state_dest(state);__entry->stage = pstate->stage;__entry->multirect_idx = pstate->multirect_index;__entry->multirect_mode = pstate->multirect_mode;
- TP_printk("crtc_id:%u plane_id:%u fb_id:%u src:{%ux%u+%ux%u} "
"dst:{%ux%u+%ux%u} stage_idx:%u stage:%d, sspp:%d "
- TP_printk("crtc_id:%u plane_id:%u fb_id:%u src:" DRM_RECT_FP_FMT
" dst:" DRM_RECT_FMT " stage_idx:%u stage:%d, sspp:%d "
__entry->crtc_id, __entry->plane_id,__entry->state->fb ? __entry->state->fb->base.id : -1,__entry->state->src_w >> 16, __entry->state->src_h >> 16,__entry->state->src_x >> 16, __entry->state->src_y >> 16,__entry->state->crtc_w, __entry->state->crtc_h,__entry->state->crtc_x, __entry->state->crtc_y,__entry->stage_idx, __entry->pstate->stage, __entry->sspp,__entry->pstate->multirect_index,__entry->pstate->multirect_mode, __entry->pixel_format,__entry->modifier)
__entry->crtc_id, __entry->plane_id, __entry->fb_id,DRM_RECT_FP_ARG(&__entry->src_rect),DRM_RECT_ARG(&__entry->dst_rect),__entry->stage_idx, __entry->stage, __entry->sspp,__entry->multirect_idx, __entry->multirect_mode,__entry->pixel_format, __entry->modifier));
TRACE_EVENT(dpu_crtc_setup_lm_bounds, @@ -725,15 +729,15 @@ TRACE_EVENT(dpu_crtc_setup_lm_bounds, TP_STRUCT__entry( __field( uint32_t, drm_id ) __field( int, mixer )
__field( struct drm_rect *, bounds )
__field_struct( struct drm_rect, bounds )
__entry->bounds = bounds;
__entry->bounds = *bounds;
__entry->mixer, DRM_RECT_ARG(__entry->bounds))
__entry->mixer, DRM_RECT_ARG(&__entry->bounds)));
TRACE_EVENT(dpu_crtc_vblank_enable, @@ -744,21 +748,25 @@ TRACE_EVENT(dpu_crtc_vblank_enable, __field( uint32_t, drm_id ) __field( uint32_t, enc_id ) __field( bool, enable )
__field( struct dpu_crtc *, crtc )
__field( bool, enabled )__field( bool, suspend )__field( bool, vblank_requested )
__entry->crtc = crtc;
__entry->enabled = crtc->enabled;__entry->suspend = crtc->suspend;__entry->vblank_requested = crtc->vblank_requested;
__entry->crtc->enabled ? "true" : "false",__entry->crtc->suspend ? "true" : "false",__entry->crtc->vblank_requested ? "true" : "false")
__entry->enabled ? "true" : "false",__entry->suspend ? "true" : "false",__entry->vblank_requested ? "true" : "false"));
DECLARE_EVENT_CLASS(dpu_crtc_enable_template, @@ -767,18 +775,22 @@ DECLARE_EVENT_CLASS(dpu_crtc_enable_template, TP_STRUCT__entry( __field( uint32_t, drm_id ) __field( bool, enable )
__field( struct dpu_crtc *, crtc )
__field( bool, enabled )__field( bool, suspend )__field( bool, vblank_requested )
__entry->crtc = crtc;
__entry->enabled = crtc->enabled;__entry->suspend = crtc->suspend;__entry->vblank_requested = crtc->vblank_requested;vblank_req:%s}", __entry->drm_id, __entry->enable ? "true" : "false",
__entry->crtc->enabled ? "true" : "false",__entry->crtc->suspend ? "true" : "false",__entry->crtc->vblank_requested ? "true" : "false")
__entry->enabled ? "true" : "false",__entry->suspend ? "true" : "false",__entry->vblank_requested ? "true" : "false")); DEFINE_EVENT(dpu_crtc_enable_template, dpu_crtc_set_suspend, TP_PROTO(uint32_t drm_id, bool enable, struct dpu_crtc *crtc), @@ -818,24 +830,24 @@ TRACE_EVENT(dpu_plane_set_scanout, TP_ARGS(index, layout, multirect_index), TP_STRUCT__entry( __field( enum dpu_sspp, index )
__field( struct dpu_hw_fmt_layout*, layout )
__field_struct( struct dpu_hw_fmt_layout, layout )
__entry->layout = layout;
__entry->layout = *layout;
"multirect_index:%d", __entry->index, __entry->layout->width,__entry->layout->height, __entry->layout->plane_addr[0],__entry->layout->plane_size[0],__entry->layout->plane_addr[1],__entry->layout->plane_size[1],__entry->layout->plane_addr[2],__entry->layout->plane_size[2],__entry->layout->plane_addr[3],__entry->layout->plane_size[3], __entry->multirect_index)
"multirect_index:%d", __entry->index, __entry->layout.width,__entry->layout.height, __entry->layout.plane_addr[0],__entry->layout.plane_size[0],__entry->layout.plane_addr[1],__entry->layout.plane_size[1],__entry->layout.plane_addr[2],__entry->layout.plane_size[2],__entry->layout.plane_addr[3],__entry->layout.plane_size[3], __entry->multirect_index));
TRACE_EVENT(dpu_plane_disable, @@ -979,16 +991,16 @@ TRACE_EVENT(dpu_core_perf_update_clk, TP_PROTO(struct drm_device *dev, bool stop_req, u64 clk_rate), TP_ARGS(dev, stop_req, clk_rate), TP_STRUCT__entry(
__field( struct drm_device *, dev )
__string( dev_name, dev->unique )
__entry->dev = dev;
__assign_str(dev_name, dev->unique);
- TP_printk("dev:%s stop_req:%s clk_rate:%llu", __entry->dev->unique,
- TP_printk("dev:%s stop_req:%s clk_rate:%llu", __get_str(dev_name), __entry->stop_req ? "true" : "false", __entry->clk_rate)
);
On Wed, Sep 19, 2018 at 01:04:42PM -0700, Abhinav Kumar wrote:
On 2018-09-19 11:33, Sean Paul wrote:
From: Sean Paul seanpaul@chromium.org
TP_printk is not synchronous, so storing pointers and then later derferencing them is a Bad Idea. This patch stores everything locally to
minor typo "dereferencing",
avoid display stomped memory.
Signed-off-by: Sean Paul seanpaul@chromium.org
After fixing the minor nit please add, Reviewed-by: Abhinav Kumar abhinavk@codeaurora.org
Thanks for the reviews, Abhinav, I've stuffed the set in dpu-staging/for-next.
Sean
drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 98 +++++++++++++---------- 1 file changed, 55 insertions(+), 43 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h index ae6b0c51ba52..e12c4cefb742 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h @@ -686,37 +686,41 @@ TRACE_EVENT(dpu_crtc_setup_mixer, TP_STRUCT__entry( __field( uint32_t, crtc_id ) __field( uint32_t, plane_id )
__field( struct drm_plane_state*,state )__field( struct dpu_plane_state*,pstate )
__field( uint32_t, fb_id )__field_struct( struct drm_rect, src_rect )__field_struct( struct drm_rect, dst_rect )__field( enum dpu_stage, stage )__field( uint32_t, multirect_idx )__field( uint32_t, multirect_mode )
__entry->state = state;__entry->pstate = pstate;
__entry->fb_id = state ? state->fb->base.id : 0;__entry->src_rect = drm_plane_state_src(state);__entry->dst_rect = drm_plane_state_dest(state);__entry->stage = pstate->stage;__entry->multirect_idx = pstate->multirect_index;__entry->multirect_mode = pstate->multirect_mode;
- TP_printk("crtc_id:%u plane_id:%u fb_id:%u src:{%ux%u+%ux%u} "
"dst:{%ux%u+%ux%u} stage_idx:%u stage:%d, sspp:%d "
- TP_printk("crtc_id:%u plane_id:%u fb_id:%u src:" DRM_RECT_FP_FMT
" dst:" DRM_RECT_FMT " stage_idx:%u stage:%d, sspp:%d "
__entry->crtc_id, __entry->plane_id,__entry->state->fb ? __entry->state->fb->base.id : -1,__entry->state->src_w >> 16, __entry->state->src_h >> 16,__entry->state->src_x >> 16, __entry->state->src_y >> 16,__entry->state->crtc_w, __entry->state->crtc_h,__entry->state->crtc_x, __entry->state->crtc_y,__entry->stage_idx, __entry->pstate->stage, __entry->sspp,__entry->pstate->multirect_index,__entry->pstate->multirect_mode, __entry->pixel_format,__entry->modifier)
__entry->crtc_id, __entry->plane_id, __entry->fb_id,DRM_RECT_FP_ARG(&__entry->src_rect),DRM_RECT_ARG(&__entry->dst_rect),__entry->stage_idx, __entry->stage, __entry->sspp,__entry->multirect_idx, __entry->multirect_mode,__entry->pixel_format, __entry->modifier));
TRACE_EVENT(dpu_crtc_setup_lm_bounds, @@ -725,15 +729,15 @@ TRACE_EVENT(dpu_crtc_setup_lm_bounds, TP_STRUCT__entry( __field( uint32_t, drm_id ) __field( int, mixer )
__field( struct drm_rect *, bounds )
__field_struct( struct drm_rect, bounds )
__entry->bounds = bounds;
__entry->bounds = *bounds;
__entry->mixer, DRM_RECT_ARG(__entry->bounds))
__entry->mixer, DRM_RECT_ARG(&__entry->bounds)));
TRACE_EVENT(dpu_crtc_vblank_enable, @@ -744,21 +748,25 @@ TRACE_EVENT(dpu_crtc_vblank_enable, __field( uint32_t, drm_id ) __field( uint32_t, enc_id ) __field( bool, enable )
__field( struct dpu_crtc *, crtc )
__field( bool, enabled )__field( bool, suspend )__field( bool, vblank_requested )
__entry->crtc = crtc;
__entry->enabled = crtc->enabled;__entry->suspend = crtc->suspend;__entry->vblank_requested = crtc->vblank_requested;
__entry->crtc->enabled ? "true" : "false",__entry->crtc->suspend ? "true" : "false",__entry->crtc->vblank_requested ? "true" : "false")
__entry->enabled ? "true" : "false",__entry->suspend ? "true" : "false",__entry->vblank_requested ? "true" : "false"));
DECLARE_EVENT_CLASS(dpu_crtc_enable_template, @@ -767,18 +775,22 @@ DECLARE_EVENT_CLASS(dpu_crtc_enable_template, TP_STRUCT__entry( __field( uint32_t, drm_id ) __field( bool, enable )
__field( struct dpu_crtc *, crtc )
__field( bool, enabled )__field( bool, suspend )__field( bool, vblank_requested )
__entry->crtc = crtc;
__entry->enabled = crtc->enabled;__entry->suspend = crtc->suspend;__entry->vblank_requested = crtc->vblank_requested;vblank_req:%s}", __entry->drm_id, __entry->enable ? "true" : "false",
__entry->crtc->enabled ? "true" : "false",__entry->crtc->suspend ? "true" : "false",__entry->crtc->vblank_requested ? "true" : "false")
__entry->enabled ? "true" : "false",__entry->suspend ? "true" : "false",__entry->vblank_requested ? "true" : "false")); DEFINE_EVENT(dpu_crtc_enable_template, dpu_crtc_set_suspend, TP_PROTO(uint32_t drm_id, bool enable, struct dpu_crtc *crtc), @@ -818,24 +830,24 @@ TRACE_EVENT(dpu_plane_set_scanout, TP_ARGS(index, layout, multirect_index), TP_STRUCT__entry( __field( enum dpu_sspp, index )
__field( struct dpu_hw_fmt_layout*, layout )
__field_struct( struct dpu_hw_fmt_layout, layout )
__entry->layout = layout;
__entry->layout = *layout;
"multirect_index:%d", __entry->index, __entry->layout->width,__entry->layout->height, __entry->layout->plane_addr[0],__entry->layout->plane_size[0],__entry->layout->plane_addr[1],__entry->layout->plane_size[1],__entry->layout->plane_addr[2],__entry->layout->plane_size[2],__entry->layout->plane_addr[3],__entry->layout->plane_size[3], __entry->multirect_index)
"multirect_index:%d", __entry->index, __entry->layout.width,__entry->layout.height, __entry->layout.plane_addr[0],__entry->layout.plane_size[0],__entry->layout.plane_addr[1],__entry->layout.plane_size[1],__entry->layout.plane_addr[2],__entry->layout.plane_size[2],__entry->layout.plane_addr[3],__entry->layout.plane_size[3], __entry->multirect_index));
TRACE_EVENT(dpu_plane_disable, @@ -979,16 +991,16 @@ TRACE_EVENT(dpu_core_perf_update_clk, TP_PROTO(struct drm_device *dev, bool stop_req, u64 clk_rate), TP_ARGS(dev, stop_req, clk_rate), TP_STRUCT__entry(
__field( struct drm_device *, dev )
__string( dev_name, dev->unique )
__entry->dev = dev;
__assign_str(dev_name, dev->unique);
- TP_printk("dev:%s stop_req:%s clk_rate:%llu", __entry->dev->unique,
- TP_printk("dev:%s stop_req:%s clk_rate:%llu", __get_str(dev_name), __entry->stop_req ? "true" : "false", __entry->clk_rate)
);
On 2018-09-19 11:33, Sean Paul wrote:
From: Sean Paul seanpaul@chromium.org
We're printing the frame_busy_mask in a trace, but after it's been cleared. This, as it turns out, is pretty pointless.
Signed-off-by: Sean Paul seanpaul@chromium.org
Reviewed-by: Abhinav Kumar abhinavk@codeaurora.org
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index c2e8985b4c54..e56f29190121 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -1361,9 +1361,9 @@ static void dpu_encoder_frame_done_callback( /* One of the physical encoders has become idle */ for (i = 0; i < dpu_enc->num_phys_encs; i++) { if (dpu_enc->phys_encs[i] == ready_phys) {
clear_bit(i, dpu_enc->frame_busy_mask); trace_dpu_enc_frame_done_cb(DRMID(drm_enc), i, dpu_enc->frame_busy_mask[0]);
clear_bit(i, dpu_enc->frame_busy_mask); }
dri-devel@lists.freedesktop.org
-
Abhinav Kumar -
Sean Paul