Re: [PATCH v2 3/4] drm/amd/display: Add pipe_state tracepoint

On 09/11, Kazlauskas, Nicholas wrote:

On 2020-09-11 10:59 a.m., Rodrigo Siqueira wrote:

...

This commit introduces a trace mechanism for struct pipe_ctx by adding a middle layer struct in the amdgpu_dm_trace.h for capturing the most important data from struct pipe_ctx and showing its data via tracepoint. This tracepoint was added to dc.c and dcn10_hw_sequencer, however, it can be added to other DCN architecture.

Co-developed-by: Nicholas Kazlauskas nicholas.kazlauskas@amd.com Signed-off-by: Nicholas Kazlauskas nicholas.kazlauskas@amd.com Signed-off-by: Rodrigo Siqueira Rodrigo.Siqueira@amd.com

This patch, while very useful, unfortunately pulls in a lot of DM code into DC so I would prefer to hold off on this one for now.

Hi Nicholas, first of all, thanks for your feedback.

By "pulls in a lot of DM code into DC" do you mean all references to plane_state and plane_res? Or the data inserted in the struct?

It would be better if this had a proper DC interface for tracing/logging these states. If the API was more like how we handle tracing register reads/writes this would be cleaner.

Could you elaborate a little bit more about "a proper DC interface"? What is your view of this sort of interface?

Also, how about Patch 04? Same problems?

Best Regards Rodrigo Siqueira

Regards, Nicholas Kazlauskas

...

---

.../amd/display/amdgpu_dm/amdgpu_dm_trace.h | 172 ++++++++++++++++++ drivers/gpu/drm/amd/display/dc/core/dc.c | 11 ++ .../amd/display/dc/dcn10/dcn10_hw_sequencer.c | 17 +- 3 files changed, 195 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h index 5fb4c4a5c349..53f62506e17c 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h @@ -376,6 +376,178 @@ TRACE_EVENT(amdgpu_dm_atomic_check_finish, __entry->async_update, __entry->allow_modeset) ); +#ifndef _AMDGPU_DM_TRACE_STRUCTS_DEFINED_ +#define _AMDGPU_DM_TRACE_STRUCTS_DEFINED_

+struct amdgpu_dm_trace_pipe_state {

• int pipe_idx;
• const void *stream;
• int stream_w;
• int stream_h;
• int dst_x;
• int dst_y;
• int dst_w;
• int dst_h;
• int src_x;
• int src_y;
• int src_w;
• int src_h;
• int clip_x;
• int clip_y;
• int clip_w;
• int clip_h;
• int recout_x;
• int recout_y;
• int recout_w;
• int recout_h;
• int viewport_x;
• int viewport_y;
• int viewport_w;
• int viewport_h;
• int flip_immediate;
• int surface_pitch;
• int format;
• int swizzle;
• unsigned int update_flags;

+};

+#define fill_out_trace_pipe_state(trace_pipe_state, pipe_ctx) \

• do { \

• trace_pipe_state.pipe_idx	= (pipe_ctx)->pipe_idx; \
  

• trace_pipe_state.stream		= (pipe_ctx)->stream; \
  

• trace_pipe_state.stream_w	= (pipe_ctx)->stream->timing.h_addressable; \
  

• trace_pipe_state.stream_h	= (pipe_ctx)->stream->timing.v_addressable; \
  

• trace_pipe_state.dst_x		= (pipe_ctx)->plane_state->dst_rect.x; \
  

• trace_pipe_state.dst_y		= (pipe_ctx)->plane_state->dst_rect.y; \
  

• trace_pipe_state.dst_w		= (pipe_ctx)->plane_state->dst_rect.width; \
  

• trace_pipe_state.dst_h		= (pipe_ctx)->plane_state->dst_rect.height; \
  

• trace_pipe_state.src_x		= (pipe_ctx)->plane_state->src_rect.x; \
  

• trace_pipe_state.src_y		= (pipe_ctx)->plane_state->src_rect.y; \
  

• trace_pipe_state.src_w		= (pipe_ctx)->plane_state->src_rect.width; \
  

• trace_pipe_state.src_h		= (pipe_ctx)->plane_state->src_rect.height; \
  

• trace_pipe_state.clip_x		= (pipe_ctx)->plane_state->clip_rect.x; \
  

• trace_pipe_state.clip_y		= (pipe_ctx)->plane_state->clip_rect.y; \
  

• trace_pipe_state.clip_w		= (pipe_ctx)->plane_state->clip_rect.width; \
  

• trace_pipe_state.clip_h		= (pipe_ctx)->plane_state->clip_rect.height; \
  

• trace_pipe_state.recout_x	= (pipe_ctx)->plane_res.scl_data.recout.x; \
  

• trace_pipe_state.recout_y	= (pipe_ctx)->plane_res.scl_data.recout.y; \
  

• trace_pipe_state.recout_w	= (pipe_ctx)->plane_res.scl_data.recout.width; \
  

• trace_pipe_state.recout_h	= (pipe_ctx)->plane_res.scl_data.recout.height; \
  

• trace_pipe_state.viewport_x	= (pipe_ctx)->plane_res.scl_data.viewport.x; \
  

• trace_pipe_state.viewport_y	= (pipe_ctx)->plane_res.scl_data.viewport.y; \
  

• trace_pipe_state.viewport_w	= (pipe_ctx)->plane_res.scl_data.viewport.width; \
  

• trace_pipe_state.viewport_h	= (pipe_ctx)->plane_res.scl_data.viewport.height; \
  

• trace_pipe_state.flip_immediate = (pipe_ctx)->plane_state->flip_immediate; \
  

• trace_pipe_state.surface_pitch	= (pipe_ctx)->plane_state->plane_size.surface_pitch; \
  

• trace_pipe_state.format		= (pipe_ctx)->plane_state->format; \
  

• trace_pipe_state.swizzle	= (pipe_ctx)->plane_state->tiling_info.gfx9.swizzle; \
  

• trace_pipe_state.update_flags	= (pipe_ctx)->update_flags.raw; \
  

• } while (0)

+#endif /* _AMDGPU_DM_TRACE_STRUCTS_DEFINED_ */

+TRACE_EVENT(amdgpu_dm_dc_pipe_state,

•    TP_PROTO(const struct amdgpu_dm_trace_pipe_state *pipe_state),
  

•    TP_ARGS(pipe_state),
  

•    TP_STRUCT__entry(
  

• 	     __field(int, pipe_idx)
  

• 	     __field(const void *, stream)
  

• 	     __field(int, stream_w)
  

• 	     __field(int, stream_h)
  

• 	     __field(int, dst_x)
  

• 	     __field(int, dst_y)
  

• 	     __field(int, dst_w)
  

• 	     __field(int, dst_h)
  

• 	     __field(int, src_x)
  

• 	     __field(int, src_y)
  

• 	     __field(int, src_w)
  

• 	     __field(int, src_h)
  

• 	     __field(int, clip_x)
  

• 	     __field(int, clip_y)
  

• 	     __field(int, clip_w)
  

• 	     __field(int, clip_h)
  

• 	     __field(int, recout_x)
  

• 	     __field(int, recout_y)
  

• 	     __field(int, recout_w)
  

• 	     __field(int, recout_h)
  

• 	     __field(int, viewport_x)
  

• 	     __field(int, viewport_y)
  

• 	     __field(int, viewport_w)
  

• 	     __field(int, viewport_h)
  

• 	     __field(int, flip_immediate)
  

• 	     __field(int, surface_pitch)
  

• 	     __field(int, format)
  

• 	     __field(int, swizzle)
  

• 	     __field(unsigned int, update_flags)
  

• ),
• TP_fast_assign(

•        __entry->pipe_idx = pipe_state->pipe_idx;
  

•        __entry->stream = pipe_state->stream;
  

•        __entry->stream_w = pipe_state->stream_w;
  

•        __entry->stream_h = pipe_state->stream_h;
  

•        __entry->dst_x = pipe_state->dst_x;
  

•        __entry->dst_y = pipe_state->dst_y;
  

•        __entry->dst_w = pipe_state->dst_w;
  

•        __entry->dst_h = pipe_state->dst_h;
  

•        __entry->src_x = pipe_state->src_x;
  

•        __entry->src_y = pipe_state->src_y;
  

•        __entry->src_w = pipe_state->src_w;
  

•        __entry->src_h = pipe_state->src_h;
  

•        __entry->clip_x = pipe_state->clip_x;
  

•        __entry->clip_y = pipe_state->clip_y;
  

•        __entry->clip_w = pipe_state->clip_w;
  

•        __entry->clip_h = pipe_state->clip_h;
  

•        __entry->recout_x = pipe_state->recout_x;
  

•        __entry->recout_y = pipe_state->recout_y;
  

•        __entry->recout_w = pipe_state->recout_w;
  

•        __entry->recout_h = pipe_state->recout_h;
  

•        __entry->viewport_x = pipe_state->viewport_x;
  

•        __entry->viewport_y = pipe_state->viewport_y;
  

•        __entry->viewport_w = pipe_state->viewport_w;
  

•        __entry->viewport_h = pipe_state->viewport_h;
  

•        __entry->flip_immediate = pipe_state->flip_immediate;
  

•        __entry->surface_pitch = pipe_state->surface_pitch;
  

•        __entry->format = pipe_state->format;
  

•        __entry->swizzle = pipe_state->swizzle;
  

•        __entry->update_flags = pipe_state->update_flags;
  

• ),
• TP_printk("pipe_idx=%d stream=%p rct(%d,%d) dst=(%d,%d,%d,%d) "

•   "src=(%d,%d,%d,%d) clip=(%d,%d,%d,%d) recout=(%d,%d,%d,%d) "
  

•   "viewport=(%d,%d,%d,%d) flip_immediate=%d pitch=%d "
  

•   "format=%d swizzle=%d update_flags=%x",
  

•   __entry->pipe_idx,
  

•   __entry->stream,
  

•   __entry->stream_w,
  

•   __entry->stream_h,
  

•   __entry->dst_x,
  

•   __entry->dst_y,
  

•   __entry->dst_w,
  

•   __entry->dst_h,
  

•   __entry->src_x,
  

•   __entry->src_y,
  

•   __entry->src_w,
  

•   __entry->src_h,
  

•   __entry->clip_x,
  

•   __entry->clip_y,
  

•   __entry->clip_w,
  

•   __entry->clip_h,
  

•   __entry->recout_x,
  

•   __entry->recout_y,
  

•   __entry->recout_w,
  

•   __entry->recout_h,
  

•   __entry->viewport_x,
  

•   __entry->viewport_y,
  

•   __entry->viewport_w,
  

•   __entry->viewport_h,
  

•   __entry->flip_immediate,
  

•   __entry->surface_pitch,
  

•   __entry->format,
  

•   __entry->swizzle,
  

•   __entry->update_flags
  

• )

+);

• #endif /* _AMDGPU_DM_TRACE_H_ */ #undef TRACE_INCLUDE_PATH

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c index dc463d99ef50..0c9f177e5827 100644 --- a/drivers/gpu/drm/amd/display/dc/core/dc.c +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c @@ -2644,6 +2644,17 @@ void dc_commit_updates_for_stream(struct dc *dc, } }

• for (i = 0; i < MAX_PIPES; ++i) {

• struct pipe_ctx *pipe_ctx = &dc->current_state->res_ctx.pipe_ctx[i];
  

• if (pipe_ctx->plane_state) {
  

• 	struct amdgpu_dm_trace_pipe_state pipe_state_trace;
  

• 	fill_out_trace_pipe_state(pipe_state_trace, pipe_ctx);
  

• 	trace_amdgpu_dm_dc_pipe_state(&pipe_state_trace);
  

• }
  

• }
• commit_planes_for_stream( dc, srf_updates,

diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c index 8ca94f506195..464d0ad093b9 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c @@ -1020,15 +1020,22 @@ static bool dcn10_hw_wa_force_recovery(struct dc *dc) }

• void dcn10_verify_allow_pstate_change_high(struct dc *dc) {
• static bool should_log_hw_state; /* prevent hw state log by default */
• if (!hubbub1_verify_allow_pstate_change_high(dc->res_pool->hubbub)) {

• if (should_log_hw_state) {
  

• 	dcn10_log_hw_state(dc, NULL);
  

• int i;
  

• for (i = 0; i < MAX_PIPES; ++i) {
  

• 	struct pipe_ctx *pipe_ctx = &dc->current_state->res_ctx.pipe_ctx[i];
  

• 	if (pipe_ctx->plane_state) {
  

• 		struct amdgpu_dm_trace_pipe_state pipe_state_trace;
  

• 		fill_out_trace_pipe_state(pipe_state_trace, pipe_ctx);
  

• 		trace_amdgpu_dm_dc_pipe_state(&pipe_state_trace);
  

• 	}
  

  }
• BREAK_TO_DEBUGGER(); if (dcn10_hw_wa_force_recovery(dc)) { /*check again*/