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====
...
This series depends on https://patchwork.freedesktop.org/series/119776/.
---
Changes in v5:
- Fixed indentation in refactor main block printing patch
- Fixed formatting issues to satisfy checkpatch
- Instead of passing 0 for DSC block, used actual length thanks to https://patchwork.freedesktop.org/series/119776/
- Link to v4: https://lore.kernel.org/r/[email protected]
Changes in v4:
- Added review tags
- Link to v3: https://lore.kernel.org/r/[email protected]
Changes in v3:
- Split sub-block changes and main block changes into two commits
- Corrected typo in comment located in DSC for loop block
- Eliminated variables mmio and base
- Dropped unnecessary "%s"
- Link to v2: https://lore.kernel.org/r/[email protected]
Changes in v2:
- Changed spelling "sub block" to "sub-block" or "sblk".
- Capitalized DPU.
- Eliminated multiplexer/wrapper function. Inlined instead.
- Changed if statements from feature checks to length checks.
- Squashed prefix and suffix patch into one.
- Link to v1: https://lore.kernel.org/r/[email protected]
---
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 prefix/suffix in name of sub-blocks
drm/msm/dpu: Refactor printing of main blocks in device core dump
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 | 74 +++++++++++++++----
drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c | 2 +-
3 files changed, 104 insertions(+), 62 deletions(-)
---
base-commit: a92b5625851098af521cd92e4c518429b661c8f4
change-id: 20230622-devcoredump_patch-df7e8f6fd632
Best regards,
--
Ryan McCann <[email protected]>
Device core dump add block method adds hardware blocks to dumping queue
with stack behavior which causes the hardware blocks to be printed in
reverse order. Change the addition to dumping queue data structure
from "list_add" to "list_add_tail" for FIFO queue behavior.
Fixes: 98659487b845 ("drm/msm: add support to take dpu snapshot")
Reviewed-by: Dmitry Baryshkov <[email protected]>
Reviewed-by: Abhinav Kumar <[email protected]>
Signed-off-by: Ryan McCann <[email protected]>
---
drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c b/drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c
index acfe1b31e079..add72bbc28b1 100644
--- a/drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c
+++ b/drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c
@@ -192,5 +192,5 @@ void msm_disp_snapshot_add_block(struct msm_disp_state *disp_state, u32 len,
new_blk->base_addr = base_addr;
msm_disp_state_dump_regs(&new_blk->state, new_blk->size, base_addr);
- list_add(&new_blk->node, &disp_state->blocks);
+ list_add_tail(&new_blk->node, &disp_state->blocks);
}
--
2.25.1
Currently, the device core dump mechanism does not dump registers of
sub-blocks within the DSPP, SSPP, DSC, and PINGPONG blocks. Edit
dpu_kms_mdp_snapshot function to account for sub-blocks.
Signed-off-by: Ryan McCann <[email protected]>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 66 +++++++++++++++++++++++++++------
1 file changed, 54 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 7a2787279ba0..f7199a5c45ab 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -890,6 +890,7 @@ static void dpu_kms_mdp_snapshot(struct msm_disp_state *disp_state, struct msm_k
int i;
struct dpu_kms *dpu_kms;
const struct dpu_mdss_cfg *cat;
+ void __iomem *base;
dpu_kms = to_dpu_kms(kms);
@@ -903,9 +904,16 @@ static void dpu_kms_mdp_snapshot(struct msm_disp_state *disp_state, struct msm_k
dpu_kms->mmio + cat->ctl[i].base, cat->ctl[i].name);
/* dump DSPP sub-blocks HW regs info */
- for (i = 0; i < cat->dspp_count; i++)
- msm_disp_snapshot_add_block(disp_state, cat->dspp[i].len,
- dpu_kms->mmio + cat->dspp[i].base, cat->dspp[i].name);
+ for (i = 0; i < cat->dspp_count; i++) {
+ base = dpu_kms->mmio + cat->dspp[i].base;
+ msm_disp_snapshot_add_block(disp_state, cat->dspp[i].len, base, cat->dspp[i].name);
+
+ if (cat->dspp[i].sblk && cat->dspp[i].sblk->pcc.len > 0)
+ msm_disp_snapshot_add_block(disp_state, cat->dspp[i].sblk->pcc.len,
+ base + cat->dspp[i].sblk->pcc.base, "%s_%s",
+ cat->dspp[i].name,
+ cat->dspp[i].sblk->pcc.name);
+ }
/* dump INTF sub-blocks HW regs info */
for (i = 0; i < cat->intf_count; i++)
@@ -913,14 +921,37 @@ static void dpu_kms_mdp_snapshot(struct msm_disp_state *disp_state, struct msm_k
dpu_kms->mmio + cat->intf[i].base, cat->intf[i].name);
/* dump PP sub-blocks HW regs info */
- for (i = 0; i < cat->pingpong_count; i++)
- msm_disp_snapshot_add_block(disp_state, cat->pingpong[i].len,
- dpu_kms->mmio + cat->pingpong[i].base, cat->pingpong[i].name);
+ for (i = 0; i < cat->pingpong_count; i++) {
+ base = dpu_kms->mmio + cat->pingpong[i].base;
+ msm_disp_snapshot_add_block(disp_state, cat->pingpong[i].len, base,
+ cat->pingpong[i].name);
+
+ /* TE2 sub-block has length of 0, so will not print it */
+
+ if (cat->pingpong[i].sblk && cat->pingpong[i].sblk->dither.len > 0)
+ msm_disp_snapshot_add_block(disp_state, cat->pingpong[i].sblk->dither.len,
+ base + cat->pingpong[i].sblk->dither.base,
+ "%s_%s", cat->pingpong[i].name,
+ cat->pingpong[i].sblk->dither.name);
+ }
/* dump SSPP sub-blocks HW regs info */
- for (i = 0; i < cat->sspp_count; i++)
- msm_disp_snapshot_add_block(disp_state, cat->sspp[i].len,
- dpu_kms->mmio + cat->sspp[i].base, cat->sspp[i].name);
+ for (i = 0; i < cat->sspp_count; i++) {
+ base = dpu_kms->mmio + cat->sspp[i].base;
+ msm_disp_snapshot_add_block(disp_state, cat->sspp[i].len, base, cat->sspp[i].name);
+
+ if (cat->sspp[i].sblk && cat->sspp[i].sblk->scaler_blk.len > 0)
+ msm_disp_snapshot_add_block(disp_state, cat->sspp[i].sblk->scaler_blk.len,
+ base + cat->sspp[i].sblk->scaler_blk.base,
+ "%s_%s", cat->sspp[i].name,
+ cat->sspp[i].sblk->scaler_blk.name);
+
+ if (cat->sspp[i].sblk && cat->sspp[i].sblk->csc_blk.len > 0)
+ msm_disp_snapshot_add_block(disp_state, cat->sspp[i].sblk->csc_blk.len,
+ base + cat->sspp[i].sblk->csc_blk.base,
+ "%s_%s", cat->sspp[i].name,
+ cat->sspp[i].sblk->csc_blk.name);
+ }
/* dump LM sub-blocks HW regs info */
for (i = 0; i < cat->mixer_count; i++)
@@ -943,9 +974,20 @@ static void dpu_kms_mdp_snapshot(struct msm_disp_state *disp_state, struct msm_k
}
/* dump DSC sub-blocks HW regs info */
- for (i = 0; i < cat->dsc_count; i++)
- msm_disp_snapshot_add_block(disp_state, cat->dsc[i].len,
- dpu_kms->mmio + cat->dsc[i].base, cat->dsc[i].name);
+ for (i = 0; i < cat->dsc_count; i++) {
+ base = dpu_kms->mmio + cat->dsc[i].base;
+ msm_disp_snapshot_add_block(disp_state, cat->dsc[i].len, base, cat->dsc[i].name);
+
+ if (cat->dsc[i].features & BIT(DPU_DSC_HW_REV_1_2)) {
+ struct dpu_dsc_blk enc = cat->dsc[i].sblk->enc;
+ struct dpu_dsc_blk ctl = cat->dsc[i].sblk->ctl;
+
+ msm_disp_snapshot_add_block(disp_state, enc.len, base + enc.base, "%s_%s",
+ cat->dsc[i].name, enc.name);
+ msm_disp_snapshot_add_block(disp_state, ctl.len, base + ctl.base, "%s_%s",
+ cat->dsc[i].name, ctl.name);
+ }
+ }
pm_runtime_put_sync(&dpu_kms->pdev->dev);
}
--
2.25.1
For a device core dump, the registers of sub-blocks are printed under a
title formatted as <mainBlkName_sblkName>. For example, the csc sub-block
for an SSPP main block "sspp_0" would be printed "sspp_0_sspp_csc0". The
title is clearly redundant due to the duplicate "sspp" and "0" that exist
in both the mainBlkName and sblkName. To eliminate this redundancy, remove
the secondary "sspp" and "0" that exist in the sub-block name by
elimanting the "sspp_" prefix and the concatenation of "num" that results
in the redundant "0" suffix. Remove num parameter altogether from relevant
macros as a consequence of it no longer being used.
Reviewed-by: Abhinav Kumar <[email protected]>
Reviewed-by: Dmitry Baryshkov <[email protected]>
Signed-off-by: Ryan McCann <[email protected]>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 50 +++++++++++++-------------
1 file changed, 25 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index e2879cc84ee0..2a903ab29964 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -252,15 +252,15 @@ static const uint32_t wb2_formats[] = {
*************************************************************/
/* SSPP common configuration */
-#define _VIG_SBLK(num, sdma_pri, qseed_ver) \
+#define _VIG_SBLK(sdma_pri, qseed_ver) \
{ \
.maxdwnscale = MAX_DOWNSCALE_RATIO, \
.maxupscale = MAX_UPSCALE_RATIO, \
.smart_dma_priority = sdma_pri, \
- .scaler_blk = {.name = STRCAT("sspp_scaler", num), \
+ .scaler_blk = {.name = "scaler", \
.id = qseed_ver, \
.base = 0xa00, .len = 0xa0,}, \
- .csc_blk = {.name = STRCAT("sspp_csc", num), \
+ .csc_blk = {.name = "csc", \
.id = DPU_SSPP_CSC_10BIT, \
.base = 0x1a00, .len = 0x100,}, \
.format_list = plane_formats_yuv, \
@@ -270,15 +270,15 @@ static const uint32_t wb2_formats[] = {
.rotation_cfg = NULL, \
}
-#define _VIG_SBLK_ROT(num, sdma_pri, qseed_ver, rot_cfg) \
+#define _VIG_SBLK_ROT(sdma_pri, qseed_ver, rot_cfg) \
{ \
.maxdwnscale = MAX_DOWNSCALE_RATIO, \
.maxupscale = MAX_UPSCALE_RATIO, \
.smart_dma_priority = sdma_pri, \
- .scaler_blk = {.name = STRCAT("sspp_scaler", num), \
+ .scaler_blk = {.name = "scaler", \
.id = qseed_ver, \
.base = 0xa00, .len = 0xa0,}, \
- .csc_blk = {.name = STRCAT("sspp_csc", num), \
+ .csc_blk = {.name = "csc", \
.id = DPU_SSPP_CSC_10BIT, \
.base = 0x1a00, .len = 0x100,}, \
.format_list = plane_formats_yuv, \
@@ -300,13 +300,13 @@ static const uint32_t wb2_formats[] = {
}
static const struct dpu_sspp_sub_blks msm8998_vig_sblk_0 =
- _VIG_SBLK("0", 0, DPU_SSPP_SCALER_QSEED3);
+ _VIG_SBLK(0, DPU_SSPP_SCALER_QSEED3);
static const struct dpu_sspp_sub_blks msm8998_vig_sblk_1 =
- _VIG_SBLK("1", 0, DPU_SSPP_SCALER_QSEED3);
+ _VIG_SBLK(0, DPU_SSPP_SCALER_QSEED3);
static const struct dpu_sspp_sub_blks msm8998_vig_sblk_2 =
- _VIG_SBLK("2", 0, DPU_SSPP_SCALER_QSEED3);
+ _VIG_SBLK(0, DPU_SSPP_SCALER_QSEED3);
static const struct dpu_sspp_sub_blks msm8998_vig_sblk_3 =
- _VIG_SBLK("3", 0, DPU_SSPP_SCALER_QSEED3);
+ _VIG_SBLK(0, DPU_SSPP_SCALER_QSEED3);
static const struct dpu_rotation_cfg dpu_rot_sc7280_cfg_v2 = {
.rot_maxheight = 1088,
@@ -315,13 +315,13 @@ static const struct dpu_rotation_cfg dpu_rot_sc7280_cfg_v2 = {
};
static const struct dpu_sspp_sub_blks sdm845_vig_sblk_0 =
- _VIG_SBLK("0", 5, DPU_SSPP_SCALER_QSEED3);
+ _VIG_SBLK(5, DPU_SSPP_SCALER_QSEED3);
static const struct dpu_sspp_sub_blks sdm845_vig_sblk_1 =
- _VIG_SBLK("1", 6, DPU_SSPP_SCALER_QSEED3);
+ _VIG_SBLK(6, DPU_SSPP_SCALER_QSEED3);
static const struct dpu_sspp_sub_blks sdm845_vig_sblk_2 =
- _VIG_SBLK("2", 7, DPU_SSPP_SCALER_QSEED3);
+ _VIG_SBLK(7, DPU_SSPP_SCALER_QSEED3);
static const struct dpu_sspp_sub_blks sdm845_vig_sblk_3 =
- _VIG_SBLK("3", 8, DPU_SSPP_SCALER_QSEED3);
+ _VIG_SBLK(8, DPU_SSPP_SCALER_QSEED3);
static const struct dpu_sspp_sub_blks sdm845_dma_sblk_0 = _DMA_SBLK(1);
static const struct dpu_sspp_sub_blks sdm845_dma_sblk_1 = _DMA_SBLK(2);
@@ -341,31 +341,31 @@ static const struct dpu_sspp_sub_blks sdm845_dma_sblk_3 = _DMA_SBLK(4);
}
static const struct dpu_sspp_sub_blks sc7180_vig_sblk_0 =
- _VIG_SBLK("0", 4, DPU_SSPP_SCALER_QSEED4);
+ _VIG_SBLK(4, DPU_SSPP_SCALER_QSEED4);
static const struct dpu_sspp_sub_blks sc7280_vig_sblk_0 =
- _VIG_SBLK_ROT("0", 4, DPU_SSPP_SCALER_QSEED4, &dpu_rot_sc7280_cfg_v2);
+ _VIG_SBLK_ROT(4, DPU_SSPP_SCALER_QSEED4, &dpu_rot_sc7280_cfg_v2);
static const struct dpu_sspp_sub_blks sm6115_vig_sblk_0 =
- _VIG_SBLK("0", 2, DPU_SSPP_SCALER_QSEED4);
+ _VIG_SBLK(2, DPU_SSPP_SCALER_QSEED4);
static const struct dpu_sspp_sub_blks sm8250_vig_sblk_0 =
- _VIG_SBLK("0", 5, DPU_SSPP_SCALER_QSEED4);
+ _VIG_SBLK(5, DPU_SSPP_SCALER_QSEED4);
static const struct dpu_sspp_sub_blks sm8250_vig_sblk_1 =
- _VIG_SBLK("1", 6, DPU_SSPP_SCALER_QSEED4);
+ _VIG_SBLK(6, DPU_SSPP_SCALER_QSEED4);
static const struct dpu_sspp_sub_blks sm8250_vig_sblk_2 =
- _VIG_SBLK("2", 7, DPU_SSPP_SCALER_QSEED4);
+ _VIG_SBLK(7, DPU_SSPP_SCALER_QSEED4);
static const struct dpu_sspp_sub_blks sm8250_vig_sblk_3 =
- _VIG_SBLK("3", 8, DPU_SSPP_SCALER_QSEED4);
+ _VIG_SBLK(8, DPU_SSPP_SCALER_QSEED4);
static const struct dpu_sspp_sub_blks sm8550_vig_sblk_0 =
- _VIG_SBLK("0", 7, DPU_SSPP_SCALER_QSEED4);
+ _VIG_SBLK(7, DPU_SSPP_SCALER_QSEED4);
static const struct dpu_sspp_sub_blks sm8550_vig_sblk_1 =
- _VIG_SBLK("1", 8, DPU_SSPP_SCALER_QSEED4);
+ _VIG_SBLK(8, DPU_SSPP_SCALER_QSEED4);
static const struct dpu_sspp_sub_blks sm8550_vig_sblk_2 =
- _VIG_SBLK("2", 9, DPU_SSPP_SCALER_QSEED4);
+ _VIG_SBLK(9, DPU_SSPP_SCALER_QSEED4);
static const struct dpu_sspp_sub_blks sm8550_vig_sblk_3 =
- _VIG_SBLK("3", 10, DPU_SSPP_SCALER_QSEED4);
+ _VIG_SBLK(10, DPU_SSPP_SCALER_QSEED4);
static const struct dpu_sspp_sub_blks sm8550_dma_sblk_4 = _DMA_SBLK(5);
static const struct dpu_sspp_sub_blks sm8550_dma_sblk_5 = _DMA_SBLK(6);
--
2.25.1
Currently, the names of main blocks are hardcoded into the
msm_disp_snapshot_add_block function rather than using the name that
already exists in the catalog. Change this to take the name directly from
the catalog instead of hardcoding it.
Signed-off-by: Ryan McCann <[email protected]>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index aa8499de1b9f..7a2787279ba0 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -900,37 +900,37 @@ static void dpu_kms_mdp_snapshot(struct msm_disp_state *disp_state, struct msm_k
/* dump CTL sub-blocks HW regs info */
for (i = 0; i < cat->ctl_count; i++)
msm_disp_snapshot_add_block(disp_state, cat->ctl[i].len,
- dpu_kms->mmio + cat->ctl[i].base, "ctl_%d", i);
+ dpu_kms->mmio + cat->ctl[i].base, cat->ctl[i].name);
/* dump DSPP sub-blocks HW regs info */
for (i = 0; i < cat->dspp_count; i++)
msm_disp_snapshot_add_block(disp_state, cat->dspp[i].len,
- dpu_kms->mmio + cat->dspp[i].base, "dspp_%d", i);
+ dpu_kms->mmio + cat->dspp[i].base, cat->dspp[i].name);
/* dump INTF sub-blocks HW regs info */
for (i = 0; i < cat->intf_count; i++)
msm_disp_snapshot_add_block(disp_state, cat->intf[i].len,
- dpu_kms->mmio + cat->intf[i].base, "intf_%d", i);
+ dpu_kms->mmio + cat->intf[i].base, cat->intf[i].name);
/* dump PP sub-blocks HW regs info */
for (i = 0; i < cat->pingpong_count; i++)
msm_disp_snapshot_add_block(disp_state, cat->pingpong[i].len,
- dpu_kms->mmio + cat->pingpong[i].base, "pingpong_%d", i);
+ dpu_kms->mmio + cat->pingpong[i].base, cat->pingpong[i].name);
/* dump SSPP sub-blocks HW regs info */
for (i = 0; i < cat->sspp_count; i++)
msm_disp_snapshot_add_block(disp_state, cat->sspp[i].len,
- dpu_kms->mmio + cat->sspp[i].base, "sspp_%d", i);
+ dpu_kms->mmio + cat->sspp[i].base, cat->sspp[i].name);
/* dump LM sub-blocks HW regs info */
for (i = 0; i < cat->mixer_count; i++)
msm_disp_snapshot_add_block(disp_state, cat->mixer[i].len,
- dpu_kms->mmio + cat->mixer[i].base, "lm_%d", i);
+ dpu_kms->mmio + cat->mixer[i].base, cat->mixer[i].name);
/* dump WB sub-blocks HW regs info */
for (i = 0; i < cat->wb_count; i++)
msm_disp_snapshot_add_block(disp_state, cat->wb[i].len,
- dpu_kms->mmio + cat->wb[i].base, "wb_%d", i);
+ dpu_kms->mmio + cat->wb[i].base, cat->wb[i].name);
if (cat->mdp[0].features & BIT(DPU_MDP_PERIPH_0_REMOVED)) {
msm_disp_snapshot_add_block(disp_state, MDP_PERIPH_TOP0,
@@ -945,7 +945,7 @@ static void dpu_kms_mdp_snapshot(struct msm_disp_state *disp_state, struct msm_k
/* dump DSC sub-blocks HW regs info */
for (i = 0; i < cat->dsc_count; i++)
msm_disp_snapshot_add_block(disp_state, cat->dsc[i].len,
- dpu_kms->mmio + cat->dsc[i].base, "dsc_%d", i);
+ dpu_kms->mmio + cat->dsc[i].base, cat->dsc[i].name);
pm_runtime_put_sync(&dpu_kms->pdev->dev);
}
--
2.25.1
Drop unused parameter "num" from VIG_SBLK_NOSCALE and DMA sub-block
macros. Update calls to relevant macros to reflect change.
Reviewed-by: Abhinav Kumar <[email protected]>
Reviewed-by: Dmitry Baryshkov <[email protected]>
Signed-off-by: Ryan McCann <[email protected]>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index dd2f89ada043..1291251e4c90 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -288,7 +288,7 @@ static const uint32_t wb2_formats[] = {
.rotation_cfg = rot_cfg, \
}
-#define _DMA_SBLK(num, sdma_pri) \
+#define _DMA_SBLK(sdma_pri) \
{ \
.maxdwnscale = SSPP_UNITY_SCALE, \
.maxupscale = SSPP_UNITY_SCALE, \
@@ -323,10 +323,10 @@ static const struct dpu_sspp_sub_blks sdm845_vig_sblk_2 =
static const struct dpu_sspp_sub_blks sdm845_vig_sblk_3 =
_VIG_SBLK("3", 8, DPU_SSPP_SCALER_QSEED3);
-static const struct dpu_sspp_sub_blks sdm845_dma_sblk_0 = _DMA_SBLK("8", 1);
-static const struct dpu_sspp_sub_blks sdm845_dma_sblk_1 = _DMA_SBLK("9", 2);
-static const struct dpu_sspp_sub_blks sdm845_dma_sblk_2 = _DMA_SBLK("10", 3);
-static const struct dpu_sspp_sub_blks sdm845_dma_sblk_3 = _DMA_SBLK("11", 4);
+static const struct dpu_sspp_sub_blks sdm845_dma_sblk_0 = _DMA_SBLK(1);
+static const struct dpu_sspp_sub_blks sdm845_dma_sblk_1 = _DMA_SBLK(2);
+static const struct dpu_sspp_sub_blks sdm845_dma_sblk_2 = _DMA_SBLK(3);
+static const struct dpu_sspp_sub_blks sdm845_dma_sblk_3 = _DMA_SBLK(4);
#define SSPP_BLK(_name, _id, _base, _len, _features, \
_sblk, _xinid, _type, _clkctrl) \
@@ -366,10 +366,10 @@ static const struct dpu_sspp_sub_blks sm8550_vig_sblk_2 =
_VIG_SBLK("2", 9, DPU_SSPP_SCALER_QSEED4);
static const struct dpu_sspp_sub_blks sm8550_vig_sblk_3 =
_VIG_SBLK("3", 10, DPU_SSPP_SCALER_QSEED4);
-static const struct dpu_sspp_sub_blks sm8550_dma_sblk_4 = _DMA_SBLK("12", 5);
-static const struct dpu_sspp_sub_blks sm8550_dma_sblk_5 = _DMA_SBLK("13", 6);
+static const struct dpu_sspp_sub_blks sm8550_dma_sblk_4 = _DMA_SBLK(5);
+static const struct dpu_sspp_sub_blks sm8550_dma_sblk_5 = _DMA_SBLK(6);
-#define _VIG_SBLK_NOSCALE(num, sdma_pri) \
+#define _VIG_SBLK_NOSCALE(sdma_pri) \
{ \
.maxdwnscale = SSPP_UNITY_SCALE, \
.maxupscale = SSPP_UNITY_SCALE, \
@@ -380,8 +380,8 @@ static const struct dpu_sspp_sub_blks sm8550_dma_sblk_5 = _DMA_SBLK("13", 6);
.virt_num_formats = ARRAY_SIZE(plane_formats), \
}
-static const struct dpu_sspp_sub_blks qcm2290_vig_sblk_0 = _VIG_SBLK_NOSCALE("0", 2);
-static const struct dpu_sspp_sub_blks qcm2290_dma_sblk_0 = _DMA_SBLK("8", 1);
+static const struct dpu_sspp_sub_blks qcm2290_vig_sblk_0 = _VIG_SBLK_NOSCALE(2);
+static const struct dpu_sspp_sub_blks qcm2290_dma_sblk_0 = _DMA_SBLK(1);
/*************************************************************
* MIXER sub blocks config
--
2.25.1
Some sub-blocks in the hw catalog have not been given a name, so when the
registers from that block are dumped, there is no name to reference.
Define names for relevant sub-blocks to fix this.
Reviewed-by: Abhinav Kumar <[email protected]>
Reviewed-by: Dmitry Baryshkov <[email protected]>
Signed-off-by: Ryan McCann <[email protected]>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index 1291251e4c90..e2879cc84ee0 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -444,12 +444,12 @@ static const struct dpu_lm_sub_blks qcm2290_lm_sblk = {
* DSPP sub blocks config
*************************************************************/
static const struct dpu_dspp_sub_blks msm8998_dspp_sblk = {
- .pcc = {.id = DPU_DSPP_PCC, .base = 0x1700,
+ .pcc = {.name = "pcc", .id = DPU_DSPP_PCC, .base = 0x1700,
.len = 0x90, .version = 0x10007},
};
static const struct dpu_dspp_sub_blks sm8150_dspp_sblk = {
- .pcc = {.id = DPU_DSPP_PCC, .base = 0x1700,
+ .pcc = {.name = "pcc", .id = DPU_DSPP_PCC, .base = 0x1700,
.len = 0x90, .version = 0x40000},
};
@@ -465,19 +465,19 @@ static const struct dpu_dspp_sub_blks sm8150_dspp_sblk = {
* PINGPONG sub blocks config
*************************************************************/
static const struct dpu_pingpong_sub_blks sdm845_pp_sblk_te = {
- .te2 = {.id = DPU_PINGPONG_TE2, .base = 0x2000, .len = 0x0,
+ .te2 = {.name = "te2", .id = DPU_PINGPONG_TE2, .base = 0x2000, .len = 0x0,
.version = 0x1},
- .dither = {.id = DPU_PINGPONG_DITHER, .base = 0x30e0,
+ .dither = {.name = "dither", .id = DPU_PINGPONG_DITHER, .base = 0x30e0,
.len = 0x20, .version = 0x10000},
};
static const struct dpu_pingpong_sub_blks sdm845_pp_sblk = {
- .dither = {.id = DPU_PINGPONG_DITHER, .base = 0x30e0,
+ .dither = {.name = "dither", .id = DPU_PINGPONG_DITHER, .base = 0x30e0,
.len = 0x20, .version = 0x10000},
};
static const struct dpu_pingpong_sub_blks sc7280_pp_sblk = {
- .dither = {.id = DPU_PINGPONG_DITHER, .base = 0xe0,
+ .dither = {.name = "dither", .id = DPU_PINGPONG_DITHER, .base = 0xe0,
.len = 0x20, .version = 0x20000},
};
@@ -517,13 +517,13 @@ static const struct dpu_pingpong_sub_blks sc7280_pp_sblk = {
* DSC sub blocks config
*************************************************************/
static const struct dpu_dsc_sub_blks dsc_sblk_0 = {
- .enc = {.base = 0x100, .len = 0x9c},
- .ctl = {.base = 0xF00, .len = 0x10},
+ .enc = {.name = "enc", .base = 0x100, .len = 0x9c},
+ .ctl = {.name = "ctl", .base = 0xF00, .len = 0x10},
};
static const struct dpu_dsc_sub_blks dsc_sblk_1 = {
- .enc = {.base = 0x200, .len = 0x9c},
- .ctl = {.base = 0xF80, .len = 0x10},
+ .enc = {.name = "enc", .base = 0x200, .len = 0x9c},
+ .ctl = {.name = "ctl", .base = 0xF80, .len = 0x10},
};
#define DSC_BLK(_name, _id, _base, _features) \
--
2.25.1
On 08/07/2023 04:24, Ryan McCann wrote:
> Currently, the names of main blocks are hardcoded into the
> msm_disp_snapshot_add_block function rather than using the name that
> already exists in the catalog. Change this to take the name directly from
> the catalog instead of hardcoding it.
>
> Signed-off-by: Ryan McCann <[email protected]>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
Reviewed-by: Dmitry Baryshkov <[email protected]>
--
With best wishes
Dmitry
On 08/07/2023 04:24, Ryan McCann wrote:
> Currently, the device core dump mechanism does not dump registers of
> sub-blocks within the DSPP, SSPP, DSC, and PINGPONG blocks. Edit
> dpu_kms_mdp_snapshot function to account for sub-blocks.
>
> Signed-off-by: Ryan McCann <[email protected]>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 66 +++++++++++++++++++++++++++------
> 1 file changed, 54 insertions(+), 12 deletions(-)
Reviewed-by: Dmitry Baryshkov <[email protected]>
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index 7a2787279ba0..f7199a5c45ab 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -890,6 +890,7 @@ static void dpu_kms_mdp_snapshot(struct msm_disp_state *disp_state, struct msm_k
> int i;
> struct dpu_kms *dpu_kms;
> const struct dpu_mdss_cfg *cat;
> + void __iomem *base;
>
> dpu_kms = to_dpu_kms(kms);
>
> @@ -903,9 +904,16 @@ static void dpu_kms_mdp_snapshot(struct msm_disp_state *disp_state, struct msm_k
> dpu_kms->mmio + cat->ctl[i].base, cat->ctl[i].name);
>
> /* dump DSPP sub-blocks HW regs info */
> - for (i = 0; i < cat->dspp_count; i++)
> - msm_disp_snapshot_add_block(disp_state, cat->dspp[i].len,
> - dpu_kms->mmio + cat->dspp[i].base, cat->dspp[i].name);
> + for (i = 0; i < cat->dspp_count; i++) {
> + base = dpu_kms->mmio + cat->dspp[i].base;
> + msm_disp_snapshot_add_block(disp_state, cat->dspp[i].len, base, cat->dspp[i].name);
> +
> + if (cat->dspp[i].sblk && cat->dspp[i].sblk->pcc.len > 0)
> + msm_disp_snapshot_add_block(disp_state, cat->dspp[i].sblk->pcc.len,
> + base + cat->dspp[i].sblk->pcc.base, "%s_%s",
> + cat->dspp[i].name,
> + cat->dspp[i].sblk->pcc.name);
Nit (no need to resend to correct this): the "%s_%s" logically fits the
next line, as it it related to the names rather than base address.
> + }
>
> /* dump INTF sub-blocks HW regs info */
> for (i = 0; i < cat->intf_count; i++)
--
With best wishes
Dmitry
On 7/7/2023 6:24 PM, Ryan McCann wrote:
> Currently, the names of main blocks are hardcoded into the
> msm_disp_snapshot_add_block function rather than using the name that
> already exists in the catalog. Change this to take the name directly from
> the catalog instead of hardcoding it.
>
> Signed-off-by: Ryan McCann <[email protected]>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
Reviewed-by: Abhinav Kumar <[email protected]>
On 7/7/2023 6:24 PM, Ryan McCann wrote:
> Currently, the device core dump mechanism does not dump registers of
> sub-blocks within the DSPP, SSPP, DSC, and PINGPONG blocks. Edit
> dpu_kms_mdp_snapshot function to account for sub-blocks.
>
> Signed-off-by: Ryan McCann <[email protected]>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 66 +++++++++++++++++++++++++++------
> 1 file changed, 54 insertions(+), 12 deletions(-)
>
Overall, I like this one.
Reviewed-by: Abhinav Kumar <[email protected]>
On 7/7/2023 6:24 PM, 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
> hardware blocks are dumped for a device core dump. This involves:
>
Nice work on completing this feature and working patiently through all
the comments !
> 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====
> ...
>
> This series depends on https://patchwork.freedesktop.org/series/119776/.
>
> ---
> Changes in v5:
> - Fixed indentation in refactor main block printing patch
> - Fixed formatting issues to satisfy checkpatch
> - Instead of passing 0 for DSC block, used actual length thanks to https://patchwork.freedesktop.org/series/119776/
> - Link to v4: https://lore.kernel.org/r/[email protected]
>
> Changes in v4:
> - Added review tags
> - Link to v3: https://lore.kernel.org/r/[email protected]
>
> Changes in v3:
> - Split sub-block changes and main block changes into two commits
> - Corrected typo in comment located in DSC for loop block
> - Eliminated variables mmio and base
> - Dropped unnecessary "%s"
> - Link to v2: https://lore.kernel.org/r/[email protected]
>
> Changes in v2:
> - Changed spelling "sub block" to "sub-block" or "sblk".
> - Capitalized DPU.
> - Eliminated multiplexer/wrapper function. Inlined instead.
> - Changed if statements from feature checks to length checks.
> - Squashed prefix and suffix patch into one.
> - Link to v1: https://lore.kernel.org/r/[email protected]
>
> ---
> 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 prefix/suffix in name of sub-blocks
> drm/msm/dpu: Refactor printing of main blocks in device core dump
> 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 | 74 +++++++++++++++----
> drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c | 2 +-
> 3 files changed, 104 insertions(+), 62 deletions(-)
> ---
> base-commit: a92b5625851098af521cd92e4c518429b661c8f4
> change-id: 20230622-devcoredump_patch-df7e8f6fd632
>
> Best regards,
On Fri, 07 Jul 2023 18:24:39 -0700, 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
> 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.
>
> [...]
Applied, thanks!
[1/6] drm/msm: Update dev core dump to not print backwards
https://gitlab.freedesktop.org/lumag/msm/-/commit/beef5b6c4050
[2/6] drm/msm/dpu: Drop unused num argument from relevant macros
https://gitlab.freedesktop.org/lumag/msm/-/commit/c8c931fb7426
[3/6] drm/msm/dpu: Define names for unnamed sblks
https://gitlab.freedesktop.org/lumag/msm/-/commit/785150043166
[4/6] drm/msm/dpu: Remove redundant prefix/suffix in name of sub-blocks
https://gitlab.freedesktop.org/lumag/msm/-/commit/8875840d60a8
[5/6] drm/msm/dpu: Refactor printing of main blocks in device core dump
https://gitlab.freedesktop.org/lumag/msm/-/commit/f63f29b1d2d5
[6/6] drm/msm/dpu: Update dev core dump to dump registers of sub-blocks
https://gitlab.freedesktop.org/lumag/msm/-/commit/4183857e5747
Best regards,
--
Dmitry Baryshkov <[email protected]>