| Message ID | 20230622-devcoredump_patch-v1-0-3b2cdcc6a576@quicinc.com |
|---|---|
| Headers | show |
| Series | Add support to print sub block registers in dpu hw catalog | expand |
On 6/23/2023 12:10 AM, Marijn Suijten wrote: > It is nice if cover letters also include the subsystem: > > drm/msm: Add support to print DPU sub-block registers > > On 2023-06-22 16:48:52, Ryan McCann wrote: >> The purpose of this patch series is to add support to print the registers >> of sub blocks in the dpu hardware catalog and fix the order in which all Hey Marijn, Sorry for the late response -- had to shift focus onto another feature for a bit. > > Global nit: I think we previously settled on "sblk" or "sub-block(s)", > not "sub blocks". > > And capitalize DPU. Acked. > >> hardware blocks are dumped for a device core dump. This involves: >> >> 1. Changing data structure from stack to queue to fix the printing order >> of the device core dump. >> >> 2. Removing redundant suffix of sub block names. >> >> 3. Removing redundant prefix of sub block names. >> >> 4. Eliminating unused variable from relevant macros. > > Dmitry has been doing that in one of his DPU catalog reworks. Got it. Is there a specific one that's making similar changes? IIRC, I didn't see one that was changing the *_SBLK macro. > >> 5. Defining names for sub blocks that have not yet been defined. > > Let's see what this means, because the code logic should already be able > to figure this out (and in some places we can perhaps delete the name > entirely). > >> 6. Implementing wrapper function that prints the registers of sub blocks >> when there is a need. > > Thought this could be rather automated, but let me see what it means in > the patches. > >> Sample Output of the sspp_0 block and its sub blocks for devcore dump: >> ======sspp_0====== >> ...registers >> ... >> ====sspp_0_scaler==== > > My suggestion would be to put less emphasis on this header with: > > ----sspp_0_scaler---- > > So that it becomes obvious that this is a sblk of the ====sspp_0==== > above. FWIW, I think having the main block name prefix in the sblk header makes it clear which blocks are main block and which ones are sblks. In addition, I'd like to keep this change within DPU (aside from the fix in the first patch) since implementing this change would require changing the *_add_block() parameters, and DSI/DP don't seem to have a need for conditional header formatting. Thanks, Jessica Zhang > >> ... >> ... >> ====sspp_0_csc==== >> ... >> ... >> ====next_block==== >> ... >> >> Signed-off-by: Ryan McCann <quic_rmccann@quicinc.com> > > No need for sign-off in cover letters. > > - Marijn > >> --- >> Ryan McCann (6): >> drm/msm: Update dev core dump to not print backwards >> drm/msm/dpu: Drop unused num argument from relevant macros >> drm/msm/dpu: Define names for unnamed sblks >> drm/msm/dpu: Remove redundant suffix in name of sub blocks >> drm/msm/disp: Remove redundant prefix in name of sub blocks >> drm/msm/dpu: Update dev core dump to dump registers of sub blocks >> >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 90 +++++----- >> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 194 +++++++++++++++++++--- >> drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c | 2 +- >> 3 files changed, 214 insertions(+), 72 deletions(-) >> --- >> base-commit: 710025fdedb3767655823c3a12d27d404d209f75 >> change-id: 20230622-devcoredump_patch-df7e8f6fd632 >> >> Best regards, >> -- >> Ryan McCann <quic_rmccann@quicinc.com> >>
The purpose of this patch series is to add support to print the registers of sub blocks in the dpu hardware catalog and fix the order in which all hardware blocks are dumped for a device core dump. This involves: 1. Changing data structure from stack to queue to fix the printing order of the device core dump. 2. Removing redundant suffix of sub block names. 3. Removing redundant prefix of sub block names. 4. Eliminating unused variable from relevant macros. 5. Defining names for sub blocks that have not yet been defined. 6. Implementing wrapper function that prints the registers of sub blocks when there is a need. Sample Output of the sspp_0 block and its sub blocks for devcore dump: ======sspp_0====== ...registers ... ====sspp_0_scaler==== ... ... ====sspp_0_csc==== ... ... ====next_block==== ... Signed-off-by: Ryan McCann <quic_rmccann@quicinc.com> --- Ryan McCann (6): drm/msm: Update dev core dump to not print backwards drm/msm/dpu: Drop unused num argument from relevant macros drm/msm/dpu: Define names for unnamed sblks drm/msm/dpu: Remove redundant suffix in name of sub blocks drm/msm/disp: Remove redundant prefix in name of sub blocks drm/msm/dpu: Update dev core dump to dump registers of sub blocks drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 90 +++++----- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 194 +++++++++++++++++++--- drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c | 2 +- 3 files changed, 214 insertions(+), 72 deletions(-) --- base-commit: 710025fdedb3767655823c3a12d27d404d209f75 change-id: 20230622-devcoredump_patch-df7e8f6fd632 Best regards,