* [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers
@ 2023-05-21 17:21 ` Dmitry Baryshkov
0 siblings, 0 replies; 32+ messages in thread
From: Dmitry Baryshkov @ 2023-05-21 17:21 UTC (permalink / raw)
To: Rob Clark, Sean Paul, Abhinav Kumar
Cc: freedreno, linux-arm-msm, Bjorn Andersson, dri-devel,
Stephen Boyd, Marijn Suijten
Drop SSPP-specifig debugfs register dumps in favour of using
debugfs/dri/0/kms or devcoredump.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 25 ---------------------
1 file changed, 25 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
index bfd82c2921af..6c5ebee2f7cd 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
@@ -727,31 +727,6 @@ int _dpu_hw_sspp_init_debugfs(struct dpu_hw_sspp *hw_pipe, struct dpu_kms *kms,
debugfs_create_xul("features", 0600,
debugfs_root, (unsigned long *)&hw_pipe->cap->features);
- /* add register dump support */
- dpu_debugfs_create_regset32("src_blk", 0400,
- debugfs_root,
- sblk->src_blk.base + cfg->base,
- sblk->src_blk.len,
- kms);
-
- if (cfg->features & BIT(DPU_SSPP_SCALER_QSEED3) ||
- cfg->features & BIT(DPU_SSPP_SCALER_QSEED3LITE) ||
- cfg->features & BIT(DPU_SSPP_SCALER_QSEED2) ||
- cfg->features & BIT(DPU_SSPP_SCALER_QSEED4))
- dpu_debugfs_create_regset32("scaler_blk", 0400,
- debugfs_root,
- sblk->scaler_blk.base + cfg->base,
- sblk->scaler_blk.len,
- kms);
-
- if (cfg->features & BIT(DPU_SSPP_CSC) ||
- cfg->features & BIT(DPU_SSPP_CSC_10BIT))
- dpu_debugfs_create_regset32("csc_blk", 0400,
- debugfs_root,
- sblk->csc_blk.base + cfg->base,
- sblk->csc_blk.len,
- kms);
-
debugfs_create_u32("xin_id",
0400,
debugfs_root,
--
2.39.2
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers
@ 2023-05-21 17:21 ` Dmitry Baryshkov
0 siblings, 0 replies; 32+ messages in thread
From: Dmitry Baryshkov @ 2023-05-21 17:21 UTC (permalink / raw)
To: Rob Clark, Sean Paul, Abhinav Kumar
Cc: Marijn Suijten, Stephen Boyd, David Airlie, Daniel Vetter,
Bjorn Andersson, linux-arm-msm, dri-devel, freedreno
Drop SSPP-specifig debugfs register dumps in favour of using
debugfs/dri/0/kms or devcoredump.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 25 ---------------------
1 file changed, 25 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
index bfd82c2921af..6c5ebee2f7cd 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
@@ -727,31 +727,6 @@ int _dpu_hw_sspp_init_debugfs(struct dpu_hw_sspp *hw_pipe, struct dpu_kms *kms,
debugfs_create_xul("features", 0600,
debugfs_root, (unsigned long *)&hw_pipe->cap->features);
- /* add register dump support */
- dpu_debugfs_create_regset32("src_blk", 0400,
- debugfs_root,
- sblk->src_blk.base + cfg->base,
- sblk->src_blk.len,
- kms);
-
- if (cfg->features & BIT(DPU_SSPP_SCALER_QSEED3) ||
- cfg->features & BIT(DPU_SSPP_SCALER_QSEED3LITE) ||
- cfg->features & BIT(DPU_SSPP_SCALER_QSEED2) ||
- cfg->features & BIT(DPU_SSPP_SCALER_QSEED4))
- dpu_debugfs_create_regset32("scaler_blk", 0400,
- debugfs_root,
- sblk->scaler_blk.base + cfg->base,
- sblk->scaler_blk.len,
- kms);
-
- if (cfg->features & BIT(DPU_SSPP_CSC) ||
- cfg->features & BIT(DPU_SSPP_CSC_10BIT))
- dpu_debugfs_create_regset32("csc_blk", 0400,
- debugfs_root,
- sblk->csc_blk.base + cfg->base,
- sblk->csc_blk.len,
- kms);
-
debugfs_create_u32("xin_id",
0400,
debugfs_root,
--
2.39.2
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 2/2] drm/msm/dpu: drop debugfs regset32 support
2023-05-21 17:21 ` Dmitry Baryshkov
@ 2023-05-21 17:21 ` Dmitry Baryshkov
-1 siblings, 0 replies; 32+ messages in thread
From: Dmitry Baryshkov @ 2023-05-21 17:21 UTC (permalink / raw)
To: Rob Clark, Sean Paul, Abhinav Kumar
Cc: freedreno, linux-arm-msm, Bjorn Andersson, dri-devel,
Stephen Boyd, Marijn Suijten
Drop the custom DPU's dpu_debugfs_create_regset32() function. With the
SSPP user being gone, there is no need in this function. While we are at
it also drop unused debugfs declarations from dpu_kms.h.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 65 -------------------------
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 48 ------------------
2 files changed, 113 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index a4293dc0b61b..26597fcb2a09 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -186,71 +186,6 @@ static void dpu_debugfs_danger_init(struct dpu_kms *dpu_kms,
}
-/*
- * Companion structure for dpu_debugfs_create_regset32.
- */
-struct dpu_debugfs_regset32 {
- uint32_t offset;
- uint32_t blk_len;
- struct dpu_kms *dpu_kms;
-};
-
-static int dpu_regset32_show(struct seq_file *s, void *data)
-{
- struct dpu_debugfs_regset32 *regset = s->private;
- struct dpu_kms *dpu_kms = regset->dpu_kms;
- void __iomem *base;
- uint32_t i, addr;
-
- if (!dpu_kms->mmio)
- return 0;
-
- base = dpu_kms->mmio + regset->offset;
-
- /* insert padding spaces, if needed */
- if (regset->offset & 0xF) {
- seq_printf(s, "[%x]", regset->offset & ~0xF);
- for (i = 0; i < (regset->offset & 0xF); i += 4)
- seq_puts(s, " ");
- }
-
- pm_runtime_get_sync(&dpu_kms->pdev->dev);
-
- /* main register output */
- for (i = 0; i < regset->blk_len; i += 4) {
- addr = regset->offset + i;
- if ((addr & 0xF) == 0x0)
- seq_printf(s, i ? "\n[%x]" : "[%x]", addr);
- seq_printf(s, " %08x", readl_relaxed(base + i));
- }
- seq_puts(s, "\n");
- pm_runtime_put_sync(&dpu_kms->pdev->dev);
-
- return 0;
-}
-DEFINE_SHOW_ATTRIBUTE(dpu_regset32);
-
-void dpu_debugfs_create_regset32(const char *name, umode_t mode,
- void *parent,
- uint32_t offset, uint32_t length, struct dpu_kms *dpu_kms)
-{
- struct dpu_debugfs_regset32 *regset;
-
- if (WARN_ON(!name || !dpu_kms || !length))
- return;
-
- regset = devm_kzalloc(&dpu_kms->pdev->dev, sizeof(*regset), GFP_KERNEL);
- if (!regset)
- return;
-
- /* make sure offset is a multiple of 4 */
- regset->offset = round_down(offset, 4);
- regset->blk_len = length;
- regset->dpu_kms = dpu_kms;
-
- debugfs_create_file(name, mode, parent, regset, &dpu_regset32_fops);
-}
-
static void dpu_debugfs_sspp_init(struct dpu_kms *dpu_kms, struct dentry *debugfs_root)
{
struct dentry *entry = debugfs_create_dir("sspp", debugfs_root);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
index 797b4ff3e806..66209e2448d2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
@@ -141,54 +141,6 @@ struct dpu_global_state
struct dpu_global_state
*__must_check dpu_kms_get_global_state(struct drm_atomic_state *s);
-/**
- * Debugfs functions - extra helper functions for debugfs support
- *
- * Main debugfs documentation is located at,
- *
- * Documentation/filesystems/debugfs.rst
- *
- * @dpu_debugfs_create_regset32: Create 32-bit register dump file
- */
-
-/**
- * dpu_debugfs_create_regset32 - Create register read back file for debugfs
- *
- * This function is almost identical to the standard debugfs_create_regset32()
- * function, with the main difference being that a list of register
- * names/offsets do not need to be provided. The 'read' function simply outputs
- * sequential register values over a specified range.
- *
- * @name: File name within debugfs
- * @mode: File mode within debugfs
- * @parent: Parent directory entry within debugfs, can be NULL
- * @offset: sub-block offset
- * @length: sub-block length, in bytes
- * @dpu_kms: pointer to dpu kms structure
- */
-void dpu_debugfs_create_regset32(const char *name, umode_t mode,
- void *parent,
- uint32_t offset, uint32_t length, struct dpu_kms *dpu_kms);
-
-/**
- * dpu_debugfs_get_root - Return root directory entry for KMS's debugfs
- *
- * The return value should be passed as the 'parent' argument to subsequent
- * debugfs create calls.
- *
- * @dpu_kms: Pointer to DPU's KMS structure
- *
- * Return: dentry pointer for DPU's debugfs location
- */
-void *dpu_debugfs_get_root(struct dpu_kms *dpu_kms);
-
-/**
- * DPU info management functions
- * These functions/definitions allow for building up a 'dpu_info' structure
- * containing one or more "key=value\n" entries.
- */
-#define DPU_KMS_INFO_MAX_SIZE 4096
-
/**
* Vblank enable/disable functions
*/
--
2.39.2
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 2/2] drm/msm/dpu: drop debugfs regset32 support
@ 2023-05-21 17:21 ` Dmitry Baryshkov
0 siblings, 0 replies; 32+ messages in thread
From: Dmitry Baryshkov @ 2023-05-21 17:21 UTC (permalink / raw)
To: Rob Clark, Sean Paul, Abhinav Kumar
Cc: Marijn Suijten, Stephen Boyd, David Airlie, Daniel Vetter,
Bjorn Andersson, linux-arm-msm, dri-devel, freedreno
Drop the custom DPU's dpu_debugfs_create_regset32() function. With the
SSPP user being gone, there is no need in this function. While we are at
it also drop unused debugfs declarations from dpu_kms.h.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 65 -------------------------
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 48 ------------------
2 files changed, 113 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index a4293dc0b61b..26597fcb2a09 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -186,71 +186,6 @@ static void dpu_debugfs_danger_init(struct dpu_kms *dpu_kms,
}
-/*
- * Companion structure for dpu_debugfs_create_regset32.
- */
-struct dpu_debugfs_regset32 {
- uint32_t offset;
- uint32_t blk_len;
- struct dpu_kms *dpu_kms;
-};
-
-static int dpu_regset32_show(struct seq_file *s, void *data)
-{
- struct dpu_debugfs_regset32 *regset = s->private;
- struct dpu_kms *dpu_kms = regset->dpu_kms;
- void __iomem *base;
- uint32_t i, addr;
-
- if (!dpu_kms->mmio)
- return 0;
-
- base = dpu_kms->mmio + regset->offset;
-
- /* insert padding spaces, if needed */
- if (regset->offset & 0xF) {
- seq_printf(s, "[%x]", regset->offset & ~0xF);
- for (i = 0; i < (regset->offset & 0xF); i += 4)
- seq_puts(s, " ");
- }
-
- pm_runtime_get_sync(&dpu_kms->pdev->dev);
-
- /* main register output */
- for (i = 0; i < regset->blk_len; i += 4) {
- addr = regset->offset + i;
- if ((addr & 0xF) == 0x0)
- seq_printf(s, i ? "\n[%x]" : "[%x]", addr);
- seq_printf(s, " %08x", readl_relaxed(base + i));
- }
- seq_puts(s, "\n");
- pm_runtime_put_sync(&dpu_kms->pdev->dev);
-
- return 0;
-}
-DEFINE_SHOW_ATTRIBUTE(dpu_regset32);
-
-void dpu_debugfs_create_regset32(const char *name, umode_t mode,
- void *parent,
- uint32_t offset, uint32_t length, struct dpu_kms *dpu_kms)
-{
- struct dpu_debugfs_regset32 *regset;
-
- if (WARN_ON(!name || !dpu_kms || !length))
- return;
-
- regset = devm_kzalloc(&dpu_kms->pdev->dev, sizeof(*regset), GFP_KERNEL);
- if (!regset)
- return;
-
- /* make sure offset is a multiple of 4 */
- regset->offset = round_down(offset, 4);
- regset->blk_len = length;
- regset->dpu_kms = dpu_kms;
-
- debugfs_create_file(name, mode, parent, regset, &dpu_regset32_fops);
-}
-
static void dpu_debugfs_sspp_init(struct dpu_kms *dpu_kms, struct dentry *debugfs_root)
{
struct dentry *entry = debugfs_create_dir("sspp", debugfs_root);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
index 797b4ff3e806..66209e2448d2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
@@ -141,54 +141,6 @@ struct dpu_global_state
struct dpu_global_state
*__must_check dpu_kms_get_global_state(struct drm_atomic_state *s);
-/**
- * Debugfs functions - extra helper functions for debugfs support
- *
- * Main debugfs documentation is located at,
- *
- * Documentation/filesystems/debugfs.rst
- *
- * @dpu_debugfs_create_regset32: Create 32-bit register dump file
- */
-
-/**
- * dpu_debugfs_create_regset32 - Create register read back file for debugfs
- *
- * This function is almost identical to the standard debugfs_create_regset32()
- * function, with the main difference being that a list of register
- * names/offsets do not need to be provided. The 'read' function simply outputs
- * sequential register values over a specified range.
- *
- * @name: File name within debugfs
- * @mode: File mode within debugfs
- * @parent: Parent directory entry within debugfs, can be NULL
- * @offset: sub-block offset
- * @length: sub-block length, in bytes
- * @dpu_kms: pointer to dpu kms structure
- */
-void dpu_debugfs_create_regset32(const char *name, umode_t mode,
- void *parent,
- uint32_t offset, uint32_t length, struct dpu_kms *dpu_kms);
-
-/**
- * dpu_debugfs_get_root - Return root directory entry for KMS's debugfs
- *
- * The return value should be passed as the 'parent' argument to subsequent
- * debugfs create calls.
- *
- * @dpu_kms: Pointer to DPU's KMS structure
- *
- * Return: dentry pointer for DPU's debugfs location
- */
-void *dpu_debugfs_get_root(struct dpu_kms *dpu_kms);
-
-/**
- * DPU info management functions
- * These functions/definitions allow for building up a 'dpu_info' structure
- * containing one or more "key=value\n" entries.
- */
-#define DPU_KMS_INFO_MAX_SIZE 4096
-
/**
* Vblank enable/disable functions
*/
--
2.39.2
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers
2023-05-21 17:21 ` Dmitry Baryshkov
@ 2023-05-21 18:11 ` Marijn Suijten
-1 siblings, 0 replies; 32+ messages in thread
From: Marijn Suijten @ 2023-05-21 18:11 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: freedreno, Sean Paul, Bjorn Andersson, Abhinav Kumar, dri-devel,
Stephen Boyd, linux-arm-msm
On 2023-05-21 20:21:46, Dmitry Baryshkov wrote:
> Drop SSPP-specifig debugfs register dumps in favour of using
> debugfs/dri/0/kms or devcoredump.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 25 ---------------------
> 1 file changed, 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> index bfd82c2921af..6c5ebee2f7cd 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> @@ -727,31 +727,6 @@ int _dpu_hw_sspp_init_debugfs(struct dpu_hw_sspp *hw_pipe, struct dpu_kms *kms,
> debugfs_create_xul("features", 0600,
> debugfs_root, (unsigned long *)&hw_pipe->cap->features);
>
> - /* add register dump support */
> - dpu_debugfs_create_regset32("src_blk", 0400,
> - debugfs_root,
> - sblk->src_blk.base + cfg->base,
> - sblk->src_blk.len,
> - kms);
> -
> - if (cfg->features & BIT(DPU_SSPP_SCALER_QSEED3) ||
> - cfg->features & BIT(DPU_SSPP_SCALER_QSEED3LITE) ||
> - cfg->features & BIT(DPU_SSPP_SCALER_QSEED2) ||
> - cfg->features & BIT(DPU_SSPP_SCALER_QSEED4))
> - dpu_debugfs_create_regset32("scaler_blk", 0400,
> - debugfs_root,
> - sblk->scaler_blk.base + cfg->base,
> - sblk->scaler_blk.len,
> - kms);
> -
> - if (cfg->features & BIT(DPU_SSPP_CSC) ||
> - cfg->features & BIT(DPU_SSPP_CSC_10BIT))
> - dpu_debugfs_create_regset32("csc_blk", 0400,
> - debugfs_root,
> - sblk->csc_blk.base + cfg->base,
> - sblk->csc_blk.len,
> - kms);
> -
> debugfs_create_u32("xin_id",
> 0400,
> debugfs_root,
> --
> 2.39.2
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers
@ 2023-05-21 18:11 ` Marijn Suijten
0 siblings, 0 replies; 32+ messages in thread
From: Marijn Suijten @ 2023-05-21 18:11 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Rob Clark, Sean Paul, Abhinav Kumar, Stephen Boyd, David Airlie,
Daniel Vetter, Bjorn Andersson, linux-arm-msm, dri-devel,
freedreno
On 2023-05-21 20:21:46, Dmitry Baryshkov wrote:
> Drop SSPP-specifig debugfs register dumps in favour of using
> debugfs/dri/0/kms or devcoredump.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 25 ---------------------
> 1 file changed, 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> index bfd82c2921af..6c5ebee2f7cd 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> @@ -727,31 +727,6 @@ int _dpu_hw_sspp_init_debugfs(struct dpu_hw_sspp *hw_pipe, struct dpu_kms *kms,
> debugfs_create_xul("features", 0600,
> debugfs_root, (unsigned long *)&hw_pipe->cap->features);
>
> - /* add register dump support */
> - dpu_debugfs_create_regset32("src_blk", 0400,
> - debugfs_root,
> - sblk->src_blk.base + cfg->base,
> - sblk->src_blk.len,
> - kms);
> -
> - if (cfg->features & BIT(DPU_SSPP_SCALER_QSEED3) ||
> - cfg->features & BIT(DPU_SSPP_SCALER_QSEED3LITE) ||
> - cfg->features & BIT(DPU_SSPP_SCALER_QSEED2) ||
> - cfg->features & BIT(DPU_SSPP_SCALER_QSEED4))
> - dpu_debugfs_create_regset32("scaler_blk", 0400,
> - debugfs_root,
> - sblk->scaler_blk.base + cfg->base,
> - sblk->scaler_blk.len,
> - kms);
> -
> - if (cfg->features & BIT(DPU_SSPP_CSC) ||
> - cfg->features & BIT(DPU_SSPP_CSC_10BIT))
> - dpu_debugfs_create_regset32("csc_blk", 0400,
> - debugfs_root,
> - sblk->csc_blk.base + cfg->base,
> - sblk->csc_blk.len,
> - kms);
> -
> debugfs_create_u32("xin_id",
> 0400,
> debugfs_root,
> --
> 2.39.2
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 2/2] drm/msm/dpu: drop debugfs regset32 support
2023-05-21 17:21 ` Dmitry Baryshkov
@ 2023-05-21 18:14 ` Marijn Suijten
-1 siblings, 0 replies; 32+ messages in thread
From: Marijn Suijten @ 2023-05-21 18:14 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: freedreno, Sean Paul, Bjorn Andersson, Abhinav Kumar, dri-devel,
Stephen Boyd, linux-arm-msm
On 2023-05-21 20:21:47, Dmitry Baryshkov wrote:
> Drop the custom DPU's dpu_debugfs_create_regset32() function. With the
> SSPP user being gone, there is no need in this function. While we are at
> it also drop unused debugfs declarations from dpu_kms.h.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 65 -------------------------
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 48 ------------------
> 2 files changed, 113 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index a4293dc0b61b..26597fcb2a09 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -186,71 +186,6 @@ static void dpu_debugfs_danger_init(struct dpu_kms *dpu_kms,
>
> }
>
> -/*
> - * Companion structure for dpu_debugfs_create_regset32.
> - */
> -struct dpu_debugfs_regset32 {
> - uint32_t offset;
> - uint32_t blk_len;
> - struct dpu_kms *dpu_kms;
> -};
> -
> -static int dpu_regset32_show(struct seq_file *s, void *data)
> -{
> - struct dpu_debugfs_regset32 *regset = s->private;
> - struct dpu_kms *dpu_kms = regset->dpu_kms;
> - void __iomem *base;
> - uint32_t i, addr;
> -
> - if (!dpu_kms->mmio)
> - return 0;
> -
> - base = dpu_kms->mmio + regset->offset;
> -
> - /* insert padding spaces, if needed */
> - if (regset->offset & 0xF) {
> - seq_printf(s, "[%x]", regset->offset & ~0xF);
> - for (i = 0; i < (regset->offset & 0xF); i += 4)
> - seq_puts(s, " ");
> - }
> -
> - pm_runtime_get_sync(&dpu_kms->pdev->dev);
> -
> - /* main register output */
> - for (i = 0; i < regset->blk_len; i += 4) {
> - addr = regset->offset + i;
> - if ((addr & 0xF) == 0x0)
> - seq_printf(s, i ? "\n[%x]" : "[%x]", addr);
> - seq_printf(s, " %08x", readl_relaxed(base + i));
> - }
> - seq_puts(s, "\n");
> - pm_runtime_put_sync(&dpu_kms->pdev->dev);
> -
> - return 0;
> -}
> -DEFINE_SHOW_ATTRIBUTE(dpu_regset32);
> -
> -void dpu_debugfs_create_regset32(const char *name, umode_t mode,
> - void *parent,
> - uint32_t offset, uint32_t length, struct dpu_kms *dpu_kms)
> -{
> - struct dpu_debugfs_regset32 *regset;
> -
> - if (WARN_ON(!name || !dpu_kms || !length))
> - return;
> -
> - regset = devm_kzalloc(&dpu_kms->pdev->dev, sizeof(*regset), GFP_KERNEL);
> - if (!regset)
> - return;
> -
> - /* make sure offset is a multiple of 4 */
> - regset->offset = round_down(offset, 4);
> - regset->blk_len = length;
> - regset->dpu_kms = dpu_kms;
> -
> - debugfs_create_file(name, mode, parent, regset, &dpu_regset32_fops);
> -}
> -
> static void dpu_debugfs_sspp_init(struct dpu_kms *dpu_kms, struct dentry *debugfs_root)
> {
> struct dentry *entry = debugfs_create_dir("sspp", debugfs_root);
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> index 797b4ff3e806..66209e2448d2 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> @@ -141,54 +141,6 @@ struct dpu_global_state
> struct dpu_global_state
> *__must_check dpu_kms_get_global_state(struct drm_atomic_state *s);
>
> -/**
> - * Debugfs functions - extra helper functions for debugfs support
> - *
> - * Main debugfs documentation is located at,
> - *
> - * Documentation/filesystems/debugfs.rst
> - *
> - * @dpu_debugfs_create_regset32: Create 32-bit register dump file
> - */
> -
> -/**
> - * dpu_debugfs_create_regset32 - Create register read back file for debugfs
> - *
> - * This function is almost identical to the standard debugfs_create_regset32()
> - * function, with the main difference being that a list of register
> - * names/offsets do not need to be provided. The 'read' function simply outputs
> - * sequential register values over a specified range.
> - *
> - * @name: File name within debugfs
> - * @mode: File mode within debugfs
> - * @parent: Parent directory entry within debugfs, can be NULL
> - * @offset: sub-block offset
> - * @length: sub-block length, in bytes
> - * @dpu_kms: pointer to dpu kms structure
> - */
> -void dpu_debugfs_create_regset32(const char *name, umode_t mode,
> - void *parent,
> - uint32_t offset, uint32_t length, struct dpu_kms *dpu_kms);
> -
> -/**
> - * dpu_debugfs_get_root - Return root directory entry for KMS's debugfs
> - *
> - * The return value should be passed as the 'parent' argument to subsequent
> - * debugfs create calls.
> - *
> - * @dpu_kms: Pointer to DPU's KMS structure
> - *
> - * Return: dentry pointer for DPU's debugfs location
> - */
> -void *dpu_debugfs_get_root(struct dpu_kms *dpu_kms);
> -
> -/**
> - * DPU info management functions
> - * These functions/definitions allow for building up a 'dpu_info' structure
> - * containing one or more "key=value\n" entries.
> - */
> -#define DPU_KMS_INFO_MAX_SIZE 4096
> -
> /**
> * Vblank enable/disable functions
> */
> --
> 2.39.2
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 2/2] drm/msm/dpu: drop debugfs regset32 support
@ 2023-05-21 18:14 ` Marijn Suijten
0 siblings, 0 replies; 32+ messages in thread
From: Marijn Suijten @ 2023-05-21 18:14 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Rob Clark, Sean Paul, Abhinav Kumar, Stephen Boyd, David Airlie,
Daniel Vetter, Bjorn Andersson, linux-arm-msm, dri-devel,
freedreno
On 2023-05-21 20:21:47, Dmitry Baryshkov wrote:
> Drop the custom DPU's dpu_debugfs_create_regset32() function. With the
> SSPP user being gone, there is no need in this function. While we are at
> it also drop unused debugfs declarations from dpu_kms.h.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 65 -------------------------
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 48 ------------------
> 2 files changed, 113 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index a4293dc0b61b..26597fcb2a09 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -186,71 +186,6 @@ static void dpu_debugfs_danger_init(struct dpu_kms *dpu_kms,
>
> }
>
> -/*
> - * Companion structure for dpu_debugfs_create_regset32.
> - */
> -struct dpu_debugfs_regset32 {
> - uint32_t offset;
> - uint32_t blk_len;
> - struct dpu_kms *dpu_kms;
> -};
> -
> -static int dpu_regset32_show(struct seq_file *s, void *data)
> -{
> - struct dpu_debugfs_regset32 *regset = s->private;
> - struct dpu_kms *dpu_kms = regset->dpu_kms;
> - void __iomem *base;
> - uint32_t i, addr;
> -
> - if (!dpu_kms->mmio)
> - return 0;
> -
> - base = dpu_kms->mmio + regset->offset;
> -
> - /* insert padding spaces, if needed */
> - if (regset->offset & 0xF) {
> - seq_printf(s, "[%x]", regset->offset & ~0xF);
> - for (i = 0; i < (regset->offset & 0xF); i += 4)
> - seq_puts(s, " ");
> - }
> -
> - pm_runtime_get_sync(&dpu_kms->pdev->dev);
> -
> - /* main register output */
> - for (i = 0; i < regset->blk_len; i += 4) {
> - addr = regset->offset + i;
> - if ((addr & 0xF) == 0x0)
> - seq_printf(s, i ? "\n[%x]" : "[%x]", addr);
> - seq_printf(s, " %08x", readl_relaxed(base + i));
> - }
> - seq_puts(s, "\n");
> - pm_runtime_put_sync(&dpu_kms->pdev->dev);
> -
> - return 0;
> -}
> -DEFINE_SHOW_ATTRIBUTE(dpu_regset32);
> -
> -void dpu_debugfs_create_regset32(const char *name, umode_t mode,
> - void *parent,
> - uint32_t offset, uint32_t length, struct dpu_kms *dpu_kms)
> -{
> - struct dpu_debugfs_regset32 *regset;
> -
> - if (WARN_ON(!name || !dpu_kms || !length))
> - return;
> -
> - regset = devm_kzalloc(&dpu_kms->pdev->dev, sizeof(*regset), GFP_KERNEL);
> - if (!regset)
> - return;
> -
> - /* make sure offset is a multiple of 4 */
> - regset->offset = round_down(offset, 4);
> - regset->blk_len = length;
> - regset->dpu_kms = dpu_kms;
> -
> - debugfs_create_file(name, mode, parent, regset, &dpu_regset32_fops);
> -}
> -
> static void dpu_debugfs_sspp_init(struct dpu_kms *dpu_kms, struct dentry *debugfs_root)
> {
> struct dentry *entry = debugfs_create_dir("sspp", debugfs_root);
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> index 797b4ff3e806..66209e2448d2 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> @@ -141,54 +141,6 @@ struct dpu_global_state
> struct dpu_global_state
> *__must_check dpu_kms_get_global_state(struct drm_atomic_state *s);
>
> -/**
> - * Debugfs functions - extra helper functions for debugfs support
> - *
> - * Main debugfs documentation is located at,
> - *
> - * Documentation/filesystems/debugfs.rst
> - *
> - * @dpu_debugfs_create_regset32: Create 32-bit register dump file
> - */
> -
> -/**
> - * dpu_debugfs_create_regset32 - Create register read back file for debugfs
> - *
> - * This function is almost identical to the standard debugfs_create_regset32()
> - * function, with the main difference being that a list of register
> - * names/offsets do not need to be provided. The 'read' function simply outputs
> - * sequential register values over a specified range.
> - *
> - * @name: File name within debugfs
> - * @mode: File mode within debugfs
> - * @parent: Parent directory entry within debugfs, can be NULL
> - * @offset: sub-block offset
> - * @length: sub-block length, in bytes
> - * @dpu_kms: pointer to dpu kms structure
> - */
> -void dpu_debugfs_create_regset32(const char *name, umode_t mode,
> - void *parent,
> - uint32_t offset, uint32_t length, struct dpu_kms *dpu_kms);
> -
> -/**
> - * dpu_debugfs_get_root - Return root directory entry for KMS's debugfs
> - *
> - * The return value should be passed as the 'parent' argument to subsequent
> - * debugfs create calls.
> - *
> - * @dpu_kms: Pointer to DPU's KMS structure
> - *
> - * Return: dentry pointer for DPU's debugfs location
> - */
> -void *dpu_debugfs_get_root(struct dpu_kms *dpu_kms);
> -
> -/**
> - * DPU info management functions
> - * These functions/definitions allow for building up a 'dpu_info' structure
> - * containing one or more "key=value\n" entries.
> - */
> -#define DPU_KMS_INFO_MAX_SIZE 4096
> -
> /**
> * Vblank enable/disable functions
> */
> --
> 2.39.2
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers
2023-05-21 18:11 ` Marijn Suijten
@ 2023-05-21 19:16 ` Marijn Suijten
-1 siblings, 0 replies; 32+ messages in thread
From: Marijn Suijten @ 2023-05-21 19:16 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Rob Clark, Sean Paul, Abhinav Kumar, Stephen Boyd, David Airlie,
Daniel Vetter, Bjorn Andersson, linux-arm-msm, dri-devel,
freedreno
On 2023-05-21 20:12:00, Marijn Suijten wrote:
> On 2023-05-21 20:21:46, Dmitry Baryshkov wrote:
> > Drop SSPP-specifig debugfs register dumps in favour of using
> > debugfs/dri/0/kms or devcoredump.
> >
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>
> Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
>
> > ---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 25 ---------------------
> > 1 file changed, 25 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> > index bfd82c2921af..6c5ebee2f7cd 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> > @@ -727,31 +727,6 @@ int _dpu_hw_sspp_init_debugfs(struct dpu_hw_sspp *hw_pipe, struct dpu_kms *kms,
> > debugfs_create_xul("features", 0600,
> > debugfs_root, (unsigned long *)&hw_pipe->cap->features);
> >
> > - /* add register dump support */
> > - dpu_debugfs_create_regset32("src_blk", 0400,
> > - debugfs_root,
> > - sblk->src_blk.base + cfg->base,
> > - sblk->src_blk.len,
Note that this fails to apply on top of
https://lore.kernel.org/linux-arm-msm/20230429012353.2569481-2-dmitry.baryshkov@linaro.org/
- Marijn
> > - kms);
> > -
> > - if (cfg->features & BIT(DPU_SSPP_SCALER_QSEED3) ||
> > - cfg->features & BIT(DPU_SSPP_SCALER_QSEED3LITE) ||
> > - cfg->features & BIT(DPU_SSPP_SCALER_QSEED2) ||
> > - cfg->features & BIT(DPU_SSPP_SCALER_QSEED4))
> > - dpu_debugfs_create_regset32("scaler_blk", 0400,
> > - debugfs_root,
> > - sblk->scaler_blk.base + cfg->base,
> > - sblk->scaler_blk.len,
> > - kms);
> > -
> > - if (cfg->features & BIT(DPU_SSPP_CSC) ||
> > - cfg->features & BIT(DPU_SSPP_CSC_10BIT))
> > - dpu_debugfs_create_regset32("csc_blk", 0400,
> > - debugfs_root,
> > - sblk->csc_blk.base + cfg->base,
> > - sblk->csc_blk.len,
> > - kms);
> > -
> > debugfs_create_u32("xin_id",
> > 0400,
> > debugfs_root,
> > --
> > 2.39.2
> >
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers
@ 2023-05-21 19:16 ` Marijn Suijten
0 siblings, 0 replies; 32+ messages in thread
From: Marijn Suijten @ 2023-05-21 19:16 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: freedreno, Sean Paul, Bjorn Andersson, Abhinav Kumar, dri-devel,
Stephen Boyd, linux-arm-msm
On 2023-05-21 20:12:00, Marijn Suijten wrote:
> On 2023-05-21 20:21:46, Dmitry Baryshkov wrote:
> > Drop SSPP-specifig debugfs register dumps in favour of using
> > debugfs/dri/0/kms or devcoredump.
> >
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>
> Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
>
> > ---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 25 ---------------------
> > 1 file changed, 25 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> > index bfd82c2921af..6c5ebee2f7cd 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> > @@ -727,31 +727,6 @@ int _dpu_hw_sspp_init_debugfs(struct dpu_hw_sspp *hw_pipe, struct dpu_kms *kms,
> > debugfs_create_xul("features", 0600,
> > debugfs_root, (unsigned long *)&hw_pipe->cap->features);
> >
> > - /* add register dump support */
> > - dpu_debugfs_create_regset32("src_blk", 0400,
> > - debugfs_root,
> > - sblk->src_blk.base + cfg->base,
> > - sblk->src_blk.len,
Note that this fails to apply on top of
https://lore.kernel.org/linux-arm-msm/20230429012353.2569481-2-dmitry.baryshkov@linaro.org/
- Marijn
> > - kms);
> > -
> > - if (cfg->features & BIT(DPU_SSPP_SCALER_QSEED3) ||
> > - cfg->features & BIT(DPU_SSPP_SCALER_QSEED3LITE) ||
> > - cfg->features & BIT(DPU_SSPP_SCALER_QSEED2) ||
> > - cfg->features & BIT(DPU_SSPP_SCALER_QSEED4))
> > - dpu_debugfs_create_regset32("scaler_blk", 0400,
> > - debugfs_root,
> > - sblk->scaler_blk.base + cfg->base,
> > - sblk->scaler_blk.len,
> > - kms);
> > -
> > - if (cfg->features & BIT(DPU_SSPP_CSC) ||
> > - cfg->features & BIT(DPU_SSPP_CSC_10BIT))
> > - dpu_debugfs_create_regset32("csc_blk", 0400,
> > - debugfs_root,
> > - sblk->csc_blk.base + cfg->base,
> > - sblk->csc_blk.len,
> > - kms);
> > -
> > debugfs_create_u32("xin_id",
> > 0400,
> > debugfs_root,
> > --
> > 2.39.2
> >
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers
2023-05-21 17:21 ` Dmitry Baryshkov
@ 2023-05-23 20:01 ` Abhinav Kumar
-1 siblings, 0 replies; 32+ messages in thread
From: Abhinav Kumar @ 2023-05-23 20:01 UTC (permalink / raw)
To: Dmitry Baryshkov, Rob Clark, Sean Paul
Cc: freedreno, linux-arm-msm, Bjorn Andersson, dri-devel,
Stephen Boyd, Marijn Suijten
On 5/21/2023 10:21 AM, Dmitry Baryshkov wrote:
> Drop SSPP-specifig debugfs register dumps in favour of using
> debugfs/dri/0/kms or devcoredump.
>
I did see another series which removes src_blk from the catalog (I am
yet to review that one) . Lets assume that one is fine and this change
will be going on top of that one right?
The concern I have with this change is that although I do agree that we
should be in favor of using debugfs/dri/0/kms ( i have used it a few
times and it works pretty well ), devcoredump does not have the support
to dump sub-blocks . Something which we should add with priority because
even with DSC blocks with the separation of enc/ctl blocks we need that
like I wrote in one of the responses.
So the "len" of the blocks having sub-blocks will be ignored in favor of
the len of the sub-blocks.
If we remove this without adding that support first, its a loss of debug
functionality.
Can we retain these blocks and remove dpu_debugfs_create_regset32 in a
different way?
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 25 ---------------------
> 1 file changed, 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> index bfd82c2921af..6c5ebee2f7cd 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> @@ -727,31 +727,6 @@ int _dpu_hw_sspp_init_debugfs(struct dpu_hw_sspp *hw_pipe, struct dpu_kms *kms,
> debugfs_create_xul("features", 0600,
> debugfs_root, (unsigned long *)&hw_pipe->cap->features);
>
> - /* add register dump support */
> - dpu_debugfs_create_regset32("src_blk", 0400,
> - debugfs_root,
> - sblk->src_blk.base + cfg->base,
> - sblk->src_blk.len,
> - kms);
> -
> - if (cfg->features & BIT(DPU_SSPP_SCALER_QSEED3) ||
> - cfg->features & BIT(DPU_SSPP_SCALER_QSEED3LITE) ||
> - cfg->features & BIT(DPU_SSPP_SCALER_QSEED2) ||
> - cfg->features & BIT(DPU_SSPP_SCALER_QSEED4))
> - dpu_debugfs_create_regset32("scaler_blk", 0400,
> - debugfs_root,
> - sblk->scaler_blk.base + cfg->base,
> - sblk->scaler_blk.len,
> - kms);
> -
> - if (cfg->features & BIT(DPU_SSPP_CSC) ||
> - cfg->features & BIT(DPU_SSPP_CSC_10BIT))
> - dpu_debugfs_create_regset32("csc_blk", 0400,
> - debugfs_root,
> - sblk->csc_blk.base + cfg->base,
> - sblk->csc_blk.len,
> - kms);
> -
> debugfs_create_u32("xin_id",
> 0400,
> debugfs_root,
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers
@ 2023-05-23 20:01 ` Abhinav Kumar
0 siblings, 0 replies; 32+ messages in thread
From: Abhinav Kumar @ 2023-05-23 20:01 UTC (permalink / raw)
To: Dmitry Baryshkov, Rob Clark, Sean Paul
Cc: linux-arm-msm, Bjorn Andersson, dri-devel, Stephen Boyd,
Marijn Suijten, freedreno
On 5/21/2023 10:21 AM, Dmitry Baryshkov wrote:
> Drop SSPP-specifig debugfs register dumps in favour of using
> debugfs/dri/0/kms or devcoredump.
>
I did see another series which removes src_blk from the catalog (I am
yet to review that one) . Lets assume that one is fine and this change
will be going on top of that one right?
The concern I have with this change is that although I do agree that we
should be in favor of using debugfs/dri/0/kms ( i have used it a few
times and it works pretty well ), devcoredump does not have the support
to dump sub-blocks . Something which we should add with priority because
even with DSC blocks with the separation of enc/ctl blocks we need that
like I wrote in one of the responses.
So the "len" of the blocks having sub-blocks will be ignored in favor of
the len of the sub-blocks.
If we remove this without adding that support first, its a loss of debug
functionality.
Can we retain these blocks and remove dpu_debugfs_create_regset32 in a
different way?
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 25 ---------------------
> 1 file changed, 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> index bfd82c2921af..6c5ebee2f7cd 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> @@ -727,31 +727,6 @@ int _dpu_hw_sspp_init_debugfs(struct dpu_hw_sspp *hw_pipe, struct dpu_kms *kms,
> debugfs_create_xul("features", 0600,
> debugfs_root, (unsigned long *)&hw_pipe->cap->features);
>
> - /* add register dump support */
> - dpu_debugfs_create_regset32("src_blk", 0400,
> - debugfs_root,
> - sblk->src_blk.base + cfg->base,
> - sblk->src_blk.len,
> - kms);
> -
> - if (cfg->features & BIT(DPU_SSPP_SCALER_QSEED3) ||
> - cfg->features & BIT(DPU_SSPP_SCALER_QSEED3LITE) ||
> - cfg->features & BIT(DPU_SSPP_SCALER_QSEED2) ||
> - cfg->features & BIT(DPU_SSPP_SCALER_QSEED4))
> - dpu_debugfs_create_regset32("scaler_blk", 0400,
> - debugfs_root,
> - sblk->scaler_blk.base + cfg->base,
> - sblk->scaler_blk.len,
> - kms);
> -
> - if (cfg->features & BIT(DPU_SSPP_CSC) ||
> - cfg->features & BIT(DPU_SSPP_CSC_10BIT))
> - dpu_debugfs_create_regset32("csc_blk", 0400,
> - debugfs_root,
> - sblk->csc_blk.base + cfg->base,
> - sblk->csc_blk.len,
> - kms);
> -
> debugfs_create_u32("xin_id",
> 0400,
> debugfs_root,
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers
2023-05-23 20:01 ` Abhinav Kumar
@ 2023-05-23 20:27 ` Dmitry Baryshkov
-1 siblings, 0 replies; 32+ messages in thread
From: Dmitry Baryshkov @ 2023-05-23 20:27 UTC (permalink / raw)
To: Abhinav Kumar
Cc: Rob Clark, Sean Paul, freedreno, linux-arm-msm, Bjorn Andersson,
dri-devel, Stephen Boyd, Marijn Suijten
On Tue, 23 May 2023 at 23:01, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote:
>
>
>
> On 5/21/2023 10:21 AM, Dmitry Baryshkov wrote:
> > Drop SSPP-specifig debugfs register dumps in favour of using
> > debugfs/dri/0/kms or devcoredump.
> >
>
> I did see another series which removes src_blk from the catalog (I am
> yet to review that one) . Lets assume that one is fine and this change
> will be going on top of that one right?
>
> The concern I have with this change is that although I do agree that we
> should be in favor of using debugfs/dri/0/kms ( i have used it a few
> times and it works pretty well ), devcoredump does not have the support
> to dump sub-blocks . Something which we should add with priority because
> even with DSC blocks with the separation of enc/ctl blocks we need that
> like I wrote in one of the responses.
>
> So the "len" of the blocks having sub-blocks will be ignored in favor of
> the len of the sub-blocks.
>
> If we remove this without adding that support first, its a loss of debug
> functionality.
>
> Can we retain these blocks and remove dpu_debugfs_create_regset32 in a
> different way?
Let's add subblocks dumping. This sounds like a good idea. I'll take a
look closer to the weekend.
>
>
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 25 ---------------------
> > 1 file changed, 25 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> > index bfd82c2921af..6c5ebee2f7cd 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> > @@ -727,31 +727,6 @@ int _dpu_hw_sspp_init_debugfs(struct dpu_hw_sspp *hw_pipe, struct dpu_kms *kms,
> > debugfs_create_xul("features", 0600,
> > debugfs_root, (unsigned long *)&hw_pipe->cap->features);
> >
> > - /* add register dump support */
> > - dpu_debugfs_create_regset32("src_blk", 0400,
> > - debugfs_root,
> > - sblk->src_blk.base + cfg->base,
> > - sblk->src_blk.len,
> > - kms);
> > -
> > - if (cfg->features & BIT(DPU_SSPP_SCALER_QSEED3) ||
> > - cfg->features & BIT(DPU_SSPP_SCALER_QSEED3LITE) ||
> > - cfg->features & BIT(DPU_SSPP_SCALER_QSEED2) ||
> > - cfg->features & BIT(DPU_SSPP_SCALER_QSEED4))
> > - dpu_debugfs_create_regset32("scaler_blk", 0400,
> > - debugfs_root,
> > - sblk->scaler_blk.base + cfg->base,
> > - sblk->scaler_blk.len,
> > - kms);
> > -
> > - if (cfg->features & BIT(DPU_SSPP_CSC) ||
> > - cfg->features & BIT(DPU_SSPP_CSC_10BIT))
> > - dpu_debugfs_create_regset32("csc_blk", 0400,
> > - debugfs_root,
> > - sblk->csc_blk.base + cfg->base,
> > - sblk->csc_blk.len,
> > - kms);
> > -
> > debugfs_create_u32("xin_id",
> > 0400,
> > debugfs_root,
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers
@ 2023-05-23 20:27 ` Dmitry Baryshkov
0 siblings, 0 replies; 32+ messages in thread
From: Dmitry Baryshkov @ 2023-05-23 20:27 UTC (permalink / raw)
To: Abhinav Kumar
Cc: Sean Paul, Bjorn Andersson, dri-devel, Stephen Boyd,
linux-arm-msm, Marijn Suijten, freedreno
On Tue, 23 May 2023 at 23:01, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote:
>
>
>
> On 5/21/2023 10:21 AM, Dmitry Baryshkov wrote:
> > Drop SSPP-specifig debugfs register dumps in favour of using
> > debugfs/dri/0/kms or devcoredump.
> >
>
> I did see another series which removes src_blk from the catalog (I am
> yet to review that one) . Lets assume that one is fine and this change
> will be going on top of that one right?
>
> The concern I have with this change is that although I do agree that we
> should be in favor of using debugfs/dri/0/kms ( i have used it a few
> times and it works pretty well ), devcoredump does not have the support
> to dump sub-blocks . Something which we should add with priority because
> even with DSC blocks with the separation of enc/ctl blocks we need that
> like I wrote in one of the responses.
>
> So the "len" of the blocks having sub-blocks will be ignored in favor of
> the len of the sub-blocks.
>
> If we remove this without adding that support first, its a loss of debug
> functionality.
>
> Can we retain these blocks and remove dpu_debugfs_create_regset32 in a
> different way?
Let's add subblocks dumping. This sounds like a good idea. I'll take a
look closer to the weekend.
>
>
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 25 ---------------------
> > 1 file changed, 25 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> > index bfd82c2921af..6c5ebee2f7cd 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> > @@ -727,31 +727,6 @@ int _dpu_hw_sspp_init_debugfs(struct dpu_hw_sspp *hw_pipe, struct dpu_kms *kms,
> > debugfs_create_xul("features", 0600,
> > debugfs_root, (unsigned long *)&hw_pipe->cap->features);
> >
> > - /* add register dump support */
> > - dpu_debugfs_create_regset32("src_blk", 0400,
> > - debugfs_root,
> > - sblk->src_blk.base + cfg->base,
> > - sblk->src_blk.len,
> > - kms);
> > -
> > - if (cfg->features & BIT(DPU_SSPP_SCALER_QSEED3) ||
> > - cfg->features & BIT(DPU_SSPP_SCALER_QSEED3LITE) ||
> > - cfg->features & BIT(DPU_SSPP_SCALER_QSEED2) ||
> > - cfg->features & BIT(DPU_SSPP_SCALER_QSEED4))
> > - dpu_debugfs_create_regset32("scaler_blk", 0400,
> > - debugfs_root,
> > - sblk->scaler_blk.base + cfg->base,
> > - sblk->scaler_blk.len,
> > - kms);
> > -
> > - if (cfg->features & BIT(DPU_SSPP_CSC) ||
> > - cfg->features & BIT(DPU_SSPP_CSC_10BIT))
> > - dpu_debugfs_create_regset32("csc_blk", 0400,
> > - debugfs_root,
> > - sblk->csc_blk.base + cfg->base,
> > - sblk->csc_blk.len,
> > - kms);
> > -
> > debugfs_create_u32("xin_id",
> > 0400,
> > debugfs_root,
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers
2023-05-23 20:01 ` Abhinav Kumar
@ 2023-05-24 9:48 ` Marijn Suijten
-1 siblings, 0 replies; 32+ messages in thread
From: Marijn Suijten @ 2023-05-24 9:48 UTC (permalink / raw)
To: Abhinav Kumar
Cc: Dmitry Baryshkov, Rob Clark, Sean Paul, freedreno, linux-arm-msm,
Bjorn Andersson, dri-devel, Stephen Boyd
On 2023-05-23 13:01:13, Abhinav Kumar wrote:
>
>
> On 5/21/2023 10:21 AM, Dmitry Baryshkov wrote:
> > Drop SSPP-specifig debugfs register dumps in favour of using
> > debugfs/dri/0/kms or devcoredump.
> >
>
> I did see another series which removes src_blk from the catalog (I am
> yet to review that one) . Lets assume that one is fine and this change
> will be going on top of that one right?
It replaces src_blk with directly accessing the blk (non-sub-block)
directly, because they were overlapping anyway.
> The concern I have with this change is that although I do agree that we
> should be in favor of using debugfs/dri/0/kms ( i have used it a few
> times and it works pretty well ), devcoredump does not have the support
> to dump sub-blocks . Something which we should add with priority because
> even with DSC blocks with the separation of enc/ctl blocks we need that
> like I wrote in one of the responses.
>
> So the "len" of the blocks having sub-blocks will be ignored in favor of
> the len of the sub-blocks.
The sub-blocks are not always contiguous with their parent block, are
they? It's probably better to print the sub-blocks separately with
clear headers anyway rather than dumping the range parent_blk_base to
max(parent_blk_base+len, parent_blk_base+sblk_base+sblk_len...).
- Marijn
> If we remove this without adding that support first, its a loss of debug
> functionality.
>
> Can we retain these blocks and remove dpu_debugfs_create_regset32 in a
> different way?
<snip>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers
@ 2023-05-24 9:48 ` Marijn Suijten
0 siblings, 0 replies; 32+ messages in thread
From: Marijn Suijten @ 2023-05-24 9:48 UTC (permalink / raw)
To: Abhinav Kumar
Cc: Sean Paul, Bjorn Andersson, dri-devel, Stephen Boyd,
linux-arm-msm, Dmitry Baryshkov, freedreno
On 2023-05-23 13:01:13, Abhinav Kumar wrote:
>
>
> On 5/21/2023 10:21 AM, Dmitry Baryshkov wrote:
> > Drop SSPP-specifig debugfs register dumps in favour of using
> > debugfs/dri/0/kms or devcoredump.
> >
>
> I did see another series which removes src_blk from the catalog (I am
> yet to review that one) . Lets assume that one is fine and this change
> will be going on top of that one right?
It replaces src_blk with directly accessing the blk (non-sub-block)
directly, because they were overlapping anyway.
> The concern I have with this change is that although I do agree that we
> should be in favor of using debugfs/dri/0/kms ( i have used it a few
> times and it works pretty well ), devcoredump does not have the support
> to dump sub-blocks . Something which we should add with priority because
> even with DSC blocks with the separation of enc/ctl blocks we need that
> like I wrote in one of the responses.
>
> So the "len" of the blocks having sub-blocks will be ignored in favor of
> the len of the sub-blocks.
The sub-blocks are not always contiguous with their parent block, are
they? It's probably better to print the sub-blocks separately with
clear headers anyway rather than dumping the range parent_blk_base to
max(parent_blk_base+len, parent_blk_base+sblk_base+sblk_len...).
- Marijn
> If we remove this without adding that support first, its a loss of debug
> functionality.
>
> Can we retain these blocks and remove dpu_debugfs_create_regset32 in a
> different way?
<snip>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers
2023-05-24 9:48 ` Marijn Suijten
@ 2023-05-24 10:24 ` Dmitry Baryshkov
-1 siblings, 0 replies; 32+ messages in thread
From: Dmitry Baryshkov @ 2023-05-24 10:24 UTC (permalink / raw)
To: Marijn Suijten
Cc: Abhinav Kumar, Rob Clark, Sean Paul, freedreno, linux-arm-msm,
Bjorn Andersson, dri-devel, Stephen Boyd
On Wed, 24 May 2023 at 12:48, Marijn Suijten
<marijn.suijten@somainline.org> wrote:
>
> On 2023-05-23 13:01:13, Abhinav Kumar wrote:
> >
> >
> > On 5/21/2023 10:21 AM, Dmitry Baryshkov wrote:
> > > Drop SSPP-specifig debugfs register dumps in favour of using
> > > debugfs/dri/0/kms or devcoredump.
> > >
> >
> > I did see another series which removes src_blk from the catalog (I am
> > yet to review that one) . Lets assume that one is fine and this change
> > will be going on top of that one right?
>
> It replaces src_blk with directly accessing the blk (non-sub-block)
> directly, because they were overlapping anyway.
>
> > The concern I have with this change is that although I do agree that we
> > should be in favor of using debugfs/dri/0/kms ( i have used it a few
> > times and it works pretty well ), devcoredump does not have the support
> > to dump sub-blocks . Something which we should add with priority because
> > even with DSC blocks with the separation of enc/ctl blocks we need that
> > like I wrote in one of the responses.
> >
> > So the "len" of the blocks having sub-blocks will be ignored in favor of
> > the len of the sub-blocks.
>
> The sub-blocks are not always contiguous with their parent block, are
> they? It's probably better to print the sub-blocks separately with
> clear headers anyway
I hope this is what Abhinav meant.
> rather than dumping the range parent_blk_base to
> max(parent_blk_base+len, parent_blk_base+sblk_base+sblk_len...).
>
> - Marijn
>
> > If we remove this without adding that support first, its a loss of debug
> > functionality.
> >
> > Can we retain these blocks and remove dpu_debugfs_create_regset32 in a
> > different way?
>
> <snip>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers
@ 2023-05-24 10:24 ` Dmitry Baryshkov
0 siblings, 0 replies; 32+ messages in thread
From: Dmitry Baryshkov @ 2023-05-24 10:24 UTC (permalink / raw)
To: Marijn Suijten
Cc: Sean Paul, Bjorn Andersson, Abhinav Kumar, dri-devel,
Stephen Boyd, linux-arm-msm, freedreno
On Wed, 24 May 2023 at 12:48, Marijn Suijten
<marijn.suijten@somainline.org> wrote:
>
> On 2023-05-23 13:01:13, Abhinav Kumar wrote:
> >
> >
> > On 5/21/2023 10:21 AM, Dmitry Baryshkov wrote:
> > > Drop SSPP-specifig debugfs register dumps in favour of using
> > > debugfs/dri/0/kms or devcoredump.
> > >
> >
> > I did see another series which removes src_blk from the catalog (I am
> > yet to review that one) . Lets assume that one is fine and this change
> > will be going on top of that one right?
>
> It replaces src_blk with directly accessing the blk (non-sub-block)
> directly, because they were overlapping anyway.
>
> > The concern I have with this change is that although I do agree that we
> > should be in favor of using debugfs/dri/0/kms ( i have used it a few
> > times and it works pretty well ), devcoredump does not have the support
> > to dump sub-blocks . Something which we should add with priority because
> > even with DSC blocks with the separation of enc/ctl blocks we need that
> > like I wrote in one of the responses.
> >
> > So the "len" of the blocks having sub-blocks will be ignored in favor of
> > the len of the sub-blocks.
>
> The sub-blocks are not always contiguous with their parent block, are
> they? It's probably better to print the sub-blocks separately with
> clear headers anyway
I hope this is what Abhinav meant.
> rather than dumping the range parent_blk_base to
> max(parent_blk_base+len, parent_blk_base+sblk_base+sblk_len...).
>
> - Marijn
>
> > If we remove this without adding that support first, its a loss of debug
> > functionality.
> >
> > Can we retain these blocks and remove dpu_debugfs_create_regset32 in a
> > different way?
>
> <snip>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers
2023-05-24 9:48 ` Marijn Suijten
@ 2023-05-24 19:18 ` Abhinav Kumar
-1 siblings, 0 replies; 32+ messages in thread
From: Abhinav Kumar @ 2023-05-24 19:18 UTC (permalink / raw)
To: Marijn Suijten
Cc: Dmitry Baryshkov, Rob Clark, Sean Paul, freedreno, linux-arm-msm,
Bjorn Andersson, dri-devel, Stephen Boyd
On 5/24/2023 2:48 AM, Marijn Suijten wrote:
> On 2023-05-23 13:01:13, Abhinav Kumar wrote:
>>
>>
>> On 5/21/2023 10:21 AM, Dmitry Baryshkov wrote:
>>> Drop SSPP-specifig debugfs register dumps in favour of using
>>> debugfs/dri/0/kms or devcoredump.
>>>
>>
>> I did see another series which removes src_blk from the catalog (I am
>> yet to review that one) . Lets assume that one is fine and this change
>> will be going on top of that one right?
>
> It replaces src_blk with directly accessing the blk (non-sub-block)
> directly, because they were overlapping anyway.
>
>> The concern I have with this change is that although I do agree that we
>> should be in favor of using debugfs/dri/0/kms ( i have used it a few
>> times and it works pretty well ), devcoredump does not have the support
>> to dump sub-blocks . Something which we should add with priority because
>> even with DSC blocks with the separation of enc/ctl blocks we need that
>> like I wrote in one of the responses.
>>
>> So the "len" of the blocks having sub-blocks will be ignored in favor of
>> the len of the sub-blocks.
>
> The sub-blocks are not always contiguous with their parent block, are
> they? It's probably better to print the sub-blocks separately with
Yes, not contiguous otherwise we could have just had them in one big range.
> clear headers anyway rather than dumping the range parent_blk_base to
> max(parent_blk_base+len, parent_blk_base+sblk_base+sblk_len...).
>
> - Marijn
When I meant sub-block support to devcoredump, this is how I visualize
them to be printed
=========================SSPP xxx =======================
=========================SSPP_CSC =======================(for SSPP_xxx)
=========================SSPP_QSEED =====================(for SSPP_xxx)
etc
OR for DSC
========================DSC_xxx ==========================
========================DSC_CTL ========================== (for DSC_xxx)
========================DSC_ENC ===========================(for DSC_xxx)
This is clear enough headers.
>
>> If we remove this without adding that support first, its a loss of debug
>> functionality.
>>
>> Can we retain these blocks and remove dpu_debugfs_create_regset32 in a
>> different way?
>
> <snip>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers
@ 2023-05-24 19:18 ` Abhinav Kumar
0 siblings, 0 replies; 32+ messages in thread
From: Abhinav Kumar @ 2023-05-24 19:18 UTC (permalink / raw)
To: Marijn Suijten
Cc: Sean Paul, Bjorn Andersson, dri-devel, Stephen Boyd,
linux-arm-msm, Dmitry Baryshkov, freedreno
On 5/24/2023 2:48 AM, Marijn Suijten wrote:
> On 2023-05-23 13:01:13, Abhinav Kumar wrote:
>>
>>
>> On 5/21/2023 10:21 AM, Dmitry Baryshkov wrote:
>>> Drop SSPP-specifig debugfs register dumps in favour of using
>>> debugfs/dri/0/kms or devcoredump.
>>>
>>
>> I did see another series which removes src_blk from the catalog (I am
>> yet to review that one) . Lets assume that one is fine and this change
>> will be going on top of that one right?
>
> It replaces src_blk with directly accessing the blk (non-sub-block)
> directly, because they were overlapping anyway.
>
>> The concern I have with this change is that although I do agree that we
>> should be in favor of using debugfs/dri/0/kms ( i have used it a few
>> times and it works pretty well ), devcoredump does not have the support
>> to dump sub-blocks . Something which we should add with priority because
>> even with DSC blocks with the separation of enc/ctl blocks we need that
>> like I wrote in one of the responses.
>>
>> So the "len" of the blocks having sub-blocks will be ignored in favor of
>> the len of the sub-blocks.
>
> The sub-blocks are not always contiguous with their parent block, are
> they? It's probably better to print the sub-blocks separately with
Yes, not contiguous otherwise we could have just had them in one big range.
> clear headers anyway rather than dumping the range parent_blk_base to
> max(parent_blk_base+len, parent_blk_base+sblk_base+sblk_len...).
>
> - Marijn
When I meant sub-block support to devcoredump, this is how I visualize
them to be printed
=========================SSPP xxx =======================
=========================SSPP_CSC =======================(for SSPP_xxx)
=========================SSPP_QSEED =====================(for SSPP_xxx)
etc
OR for DSC
========================DSC_xxx ==========================
========================DSC_CTL ========================== (for DSC_xxx)
========================DSC_ENC ===========================(for DSC_xxx)
This is clear enough headers.
>
>> If we remove this without adding that support first, its a loss of debug
>> functionality.
>>
>> Can we retain these blocks and remove dpu_debugfs_create_regset32 in a
>> different way?
>
> <snip>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers
2023-05-24 19:18 ` Abhinav Kumar
@ 2023-05-29 21:36 ` Marijn Suijten
-1 siblings, 0 replies; 32+ messages in thread
From: Marijn Suijten @ 2023-05-29 21:36 UTC (permalink / raw)
To: Abhinav Kumar
Cc: Dmitry Baryshkov, Rob Clark, Sean Paul, freedreno, linux-arm-msm,
Bjorn Andersson, dri-devel, Stephen Boyd
On 2023-05-24 12:18:09, Abhinav Kumar wrote:
>
>
> On 5/24/2023 2:48 AM, Marijn Suijten wrote:
> > On 2023-05-23 13:01:13, Abhinav Kumar wrote:
> >>
> >>
> >> On 5/21/2023 10:21 AM, Dmitry Baryshkov wrote:
> >>> Drop SSPP-specifig debugfs register dumps in favour of using
> >>> debugfs/dri/0/kms or devcoredump.
> >>>
> >>
> >> I did see another series which removes src_blk from the catalog (I am
> >> yet to review that one) . Lets assume that one is fine and this change
> >> will be going on top of that one right?
> >
> > It replaces src_blk with directly accessing the blk (non-sub-block)
> > directly, because they were overlapping anyway.
> >
> >> The concern I have with this change is that although I do agree that we
> >> should be in favor of using debugfs/dri/0/kms ( i have used it a few
> >> times and it works pretty well ), devcoredump does not have the support
> >> to dump sub-blocks . Something which we should add with priority because
> >> even with DSC blocks with the separation of enc/ctl blocks we need that
> >> like I wrote in one of the responses.
> >>
> >> So the "len" of the blocks having sub-blocks will be ignored in favor of
> >> the len of the sub-blocks.
> >
> > The sub-blocks are not always contiguous with their parent block, are
> > they? It's probably better to print the sub-blocks separately with
>
> Yes, not contiguous otherwise we could have just had them in one big range.
>
> > clear headers anyway rather than dumping the range parent_blk_base to
> > max(parent_blk_base+len, parent_blk_base+sblk_base+sblk_len...).
> >
> > - Marijn
>
> When I meant sub-block support to devcoredump, this is how I visualize
> them to be printed
>
> =========================SSPP xxx =======================
> =========================SSPP_CSC =======================(for SSPP_xxx)
> =========================SSPP_QSEED =====================(for SSPP_xxx)
Yeah something along those lines, though I don't really like the `(for
SSPP_xxx)` suffix which we should either drop entirely and make the
"heading" less of a "heading"
========================= SSPP xxx =======================
...
------------------------- SSPP_CSC -----------------------
...
------------------------- SSPP_QSEED ---------------------
...
And/or inline the numbers:
========================= SSPP xxx =======================
...
----------------------- SSPP_xxx_CSC ---------------------
...
---------------------- SSPP_xxx_QSEED --------------------
...
Either works, or any other pattern in the title (e.g `SSPP xxx: CSC`)
that clearly tells the blocks and sub-blocks apart.
- Marijn
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers
@ 2023-05-29 21:36 ` Marijn Suijten
0 siblings, 0 replies; 32+ messages in thread
From: Marijn Suijten @ 2023-05-29 21:36 UTC (permalink / raw)
To: Abhinav Kumar
Cc: Sean Paul, Bjorn Andersson, dri-devel, Stephen Boyd,
linux-arm-msm, Dmitry Baryshkov, freedreno
On 2023-05-24 12:18:09, Abhinav Kumar wrote:
>
>
> On 5/24/2023 2:48 AM, Marijn Suijten wrote:
> > On 2023-05-23 13:01:13, Abhinav Kumar wrote:
> >>
> >>
> >> On 5/21/2023 10:21 AM, Dmitry Baryshkov wrote:
> >>> Drop SSPP-specifig debugfs register dumps in favour of using
> >>> debugfs/dri/0/kms or devcoredump.
> >>>
> >>
> >> I did see another series which removes src_blk from the catalog (I am
> >> yet to review that one) . Lets assume that one is fine and this change
> >> will be going on top of that one right?
> >
> > It replaces src_blk with directly accessing the blk (non-sub-block)
> > directly, because they were overlapping anyway.
> >
> >> The concern I have with this change is that although I do agree that we
> >> should be in favor of using debugfs/dri/0/kms ( i have used it a few
> >> times and it works pretty well ), devcoredump does not have the support
> >> to dump sub-blocks . Something which we should add with priority because
> >> even with DSC blocks with the separation of enc/ctl blocks we need that
> >> like I wrote in one of the responses.
> >>
> >> So the "len" of the blocks having sub-blocks will be ignored in favor of
> >> the len of the sub-blocks.
> >
> > The sub-blocks are not always contiguous with their parent block, are
> > they? It's probably better to print the sub-blocks separately with
>
> Yes, not contiguous otherwise we could have just had them in one big range.
>
> > clear headers anyway rather than dumping the range parent_blk_base to
> > max(parent_blk_base+len, parent_blk_base+sblk_base+sblk_len...).
> >
> > - Marijn
>
> When I meant sub-block support to devcoredump, this is how I visualize
> them to be printed
>
> =========================SSPP xxx =======================
> =========================SSPP_CSC =======================(for SSPP_xxx)
> =========================SSPP_QSEED =====================(for SSPP_xxx)
Yeah something along those lines, though I don't really like the `(for
SSPP_xxx)` suffix which we should either drop entirely and make the
"heading" less of a "heading"
========================= SSPP xxx =======================
...
------------------------- SSPP_CSC -----------------------
...
------------------------- SSPP_QSEED ---------------------
...
And/or inline the numbers:
========================= SSPP xxx =======================
...
----------------------- SSPP_xxx_CSC ---------------------
...
---------------------- SSPP_xxx_QSEED --------------------
...
Either works, or any other pattern in the title (e.g `SSPP xxx: CSC`)
that clearly tells the blocks and sub-blocks apart.
- Marijn
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers
2023-05-29 21:36 ` Marijn Suijten
@ 2023-05-30 17:37 ` Abhinav Kumar
-1 siblings, 0 replies; 32+ messages in thread
From: Abhinav Kumar @ 2023-05-30 17:37 UTC (permalink / raw)
To: Marijn Suijten
Cc: Sean Paul, Bjorn Andersson, dri-devel, Stephen Boyd,
linux-arm-msm, Dmitry Baryshkov, freedreno
On 5/29/2023 2:36 PM, Marijn Suijten wrote:
> On 2023-05-24 12:18:09, Abhinav Kumar wrote:
>>
>>
>> On 5/24/2023 2:48 AM, Marijn Suijten wrote:
>>> On 2023-05-23 13:01:13, Abhinav Kumar wrote:
>>>>
>>>>
>>>> On 5/21/2023 10:21 AM, Dmitry Baryshkov wrote:
>>>>> Drop SSPP-specifig debugfs register dumps in favour of using
>>>>> debugfs/dri/0/kms or devcoredump.
>>>>>
>>>>
>>>> I did see another series which removes src_blk from the catalog (I am
>>>> yet to review that one) . Lets assume that one is fine and this change
>>>> will be going on top of that one right?
>>>
>>> It replaces src_blk with directly accessing the blk (non-sub-block)
>>> directly, because they were overlapping anyway.
>>>
>>>> The concern I have with this change is that although I do agree that we
>>>> should be in favor of using debugfs/dri/0/kms ( i have used it a few
>>>> times and it works pretty well ), devcoredump does not have the support
>>>> to dump sub-blocks . Something which we should add with priority because
>>>> even with DSC blocks with the separation of enc/ctl blocks we need that
>>>> like I wrote in one of the responses.
>>>>
>>>> So the "len" of the blocks having sub-blocks will be ignored in favor of
>>>> the len of the sub-blocks.
>>>
>>> The sub-blocks are not always contiguous with their parent block, are
>>> they? It's probably better to print the sub-blocks separately with
>>
>> Yes, not contiguous otherwise we could have just had them in one big range.
>>
>>> clear headers anyway rather than dumping the range parent_blk_base to
>>> max(parent_blk_base+len, parent_blk_base+sblk_base+sblk_len...).
>>>
>>> - Marijn
>>
>> When I meant sub-block support to devcoredump, this is how I visualize
>> them to be printed
>>
>> =========================SSPP xxx =======================
>> =========================SSPP_CSC =======================(for SSPP_xxx)
>> =========================SSPP_QSEED =====================(for SSPP_xxx)
>
> Yeah something along those lines, though I don't really like the `(for
> SSPP_xxx)` suffix which we should either drop entirely and make the
> "heading" less of a "heading"
>
I wrote that "for SSPP_xxx" to explain the idea, ofcourse it wont be
part of the print itself.
Without that, it matches what you have shared below.
> ========================= SSPP xxx =======================
> ...
> ------------------------- SSPP_CSC -----------------------
> ...
> ------------------------- SSPP_QSEED ---------------------
> ...
>
> And/or inline the numbers:
>
> ========================= SSPP xxx =======================
> ...
> ----------------------- SSPP_xxx_CSC ---------------------
> ...
> ---------------------- SSPP_xxx_QSEED --------------------
> ...
>
sure this is also fine with me.
> Either works, or any other pattern in the title (e.g `SSPP xxx: CSC`)
> that clearly tells the blocks and sub-blocks apart.
>
> - Marijn
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers
@ 2023-05-30 17:37 ` Abhinav Kumar
0 siblings, 0 replies; 32+ messages in thread
From: Abhinav Kumar @ 2023-05-30 17:37 UTC (permalink / raw)
To: Marijn Suijten
Cc: freedreno, linux-arm-msm, Bjorn Andersson, dri-devel,
Stephen Boyd, Dmitry Baryshkov, Sean Paul
On 5/29/2023 2:36 PM, Marijn Suijten wrote:
> On 2023-05-24 12:18:09, Abhinav Kumar wrote:
>>
>>
>> On 5/24/2023 2:48 AM, Marijn Suijten wrote:
>>> On 2023-05-23 13:01:13, Abhinav Kumar wrote:
>>>>
>>>>
>>>> On 5/21/2023 10:21 AM, Dmitry Baryshkov wrote:
>>>>> Drop SSPP-specifig debugfs register dumps in favour of using
>>>>> debugfs/dri/0/kms or devcoredump.
>>>>>
>>>>
>>>> I did see another series which removes src_blk from the catalog (I am
>>>> yet to review that one) . Lets assume that one is fine and this change
>>>> will be going on top of that one right?
>>>
>>> It replaces src_blk with directly accessing the blk (non-sub-block)
>>> directly, because they were overlapping anyway.
>>>
>>>> The concern I have with this change is that although I do agree that we
>>>> should be in favor of using debugfs/dri/0/kms ( i have used it a few
>>>> times and it works pretty well ), devcoredump does not have the support
>>>> to dump sub-blocks . Something which we should add with priority because
>>>> even with DSC blocks with the separation of enc/ctl blocks we need that
>>>> like I wrote in one of the responses.
>>>>
>>>> So the "len" of the blocks having sub-blocks will be ignored in favor of
>>>> the len of the sub-blocks.
>>>
>>> The sub-blocks are not always contiguous with their parent block, are
>>> they? It's probably better to print the sub-blocks separately with
>>
>> Yes, not contiguous otherwise we could have just had them in one big range.
>>
>>> clear headers anyway rather than dumping the range parent_blk_base to
>>> max(parent_blk_base+len, parent_blk_base+sblk_base+sblk_len...).
>>>
>>> - Marijn
>>
>> When I meant sub-block support to devcoredump, this is how I visualize
>> them to be printed
>>
>> =========================SSPP xxx =======================
>> =========================SSPP_CSC =======================(for SSPP_xxx)
>> =========================SSPP_QSEED =====================(for SSPP_xxx)
>
> Yeah something along those lines, though I don't really like the `(for
> SSPP_xxx)` suffix which we should either drop entirely and make the
> "heading" less of a "heading"
>
I wrote that "for SSPP_xxx" to explain the idea, ofcourse it wont be
part of the print itself.
Without that, it matches what you have shared below.
> ========================= SSPP xxx =======================
> ...
> ------------------------- SSPP_CSC -----------------------
> ...
> ------------------------- SSPP_QSEED ---------------------
> ...
>
> And/or inline the numbers:
>
> ========================= SSPP xxx =======================
> ...
> ----------------------- SSPP_xxx_CSC ---------------------
> ...
> ---------------------- SSPP_xxx_QSEED --------------------
> ...
>
sure this is also fine with me.
> Either works, or any other pattern in the title (e.g `SSPP xxx: CSC`)
> that clearly tells the blocks and sub-blocks apart.
>
> - Marijn
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers
2023-05-30 17:37 ` Abhinav Kumar
@ 2023-05-30 20:14 ` Dmitry Baryshkov
-1 siblings, 0 replies; 32+ messages in thread
From: Dmitry Baryshkov @ 2023-05-30 20:14 UTC (permalink / raw)
To: Abhinav Kumar
Cc: Marijn Suijten, Sean Paul, Bjorn Andersson, dri-devel,
Stephen Boyd, linux-arm-msm, freedreno
On Tue, 30 May 2023 at 20:37, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote:
>
>
>
> On 5/29/2023 2:36 PM, Marijn Suijten wrote:
> > On 2023-05-24 12:18:09, Abhinav Kumar wrote:
> >>
> >>
> >> On 5/24/2023 2:48 AM, Marijn Suijten wrote:
> >>> On 2023-05-23 13:01:13, Abhinav Kumar wrote:
> >>>>
> >>>>
> >>>> On 5/21/2023 10:21 AM, Dmitry Baryshkov wrote:
> >>>>> Drop SSPP-specifig debugfs register dumps in favour of using
> >>>>> debugfs/dri/0/kms or devcoredump.
> >>>>>
> >>>>
> >>>> I did see another series which removes src_blk from the catalog (I am
> >>>> yet to review that one) . Lets assume that one is fine and this change
> >>>> will be going on top of that one right?
> >>>
> >>> It replaces src_blk with directly accessing the blk (non-sub-block)
> >>> directly, because they were overlapping anyway.
> >>>
> >>>> The concern I have with this change is that although I do agree that we
> >>>> should be in favor of using debugfs/dri/0/kms ( i have used it a few
> >>>> times and it works pretty well ), devcoredump does not have the support
> >>>> to dump sub-blocks . Something which we should add with priority because
> >>>> even with DSC blocks with the separation of enc/ctl blocks we need that
> >>>> like I wrote in one of the responses.
> >>>>
> >>>> So the "len" of the blocks having sub-blocks will be ignored in favor of
> >>>> the len of the sub-blocks.
> >>>
> >>> The sub-blocks are not always contiguous with their parent block, are
> >>> they? It's probably better to print the sub-blocks separately with
> >>
> >> Yes, not contiguous otherwise we could have just had them in one big range.
> >>
> >>> clear headers anyway rather than dumping the range parent_blk_base to
> >>> max(parent_blk_base+len, parent_blk_base+sblk_base+sblk_len...).
> >>>
> >>> - Marijn
> >>
> >> When I meant sub-block support to devcoredump, this is how I visualize
> >> them to be printed
> >>
> >> =========================SSPP xxx =======================
> >> =========================SSPP_CSC =======================(for SSPP_xxx)
> >> =========================SSPP_QSEED =====================(for SSPP_xxx)
> >
> > Yeah something along those lines, though I don't really like the `(for
> > SSPP_xxx)` suffix which we should either drop entirely and make the
> > "heading" less of a "heading"
> >
>
> I wrote that "for SSPP_xxx" to explain the idea, ofcourse it wont be
> part of the print itself.
>
> Without that, it matches what you have shared below.
>
>
> > ========================= SSPP xxx =======================
> > ...
> > ------------------------- SSPP_CSC -----------------------
> > ...
> > ------------------------- SSPP_QSEED ---------------------
> > ...
> >
> > And/or inline the numbers:
> >
> > ========================= SSPP xxx =======================
> > ...
> > ----------------------- SSPP_xxx_CSC ---------------------
> > ...
> > ---------------------- SSPP_xxx_QSEED --------------------
> > ...
I'd prefer this format. It eases grepping.
> >
>
> sure this is also fine with me.
>
> > Either works, or any other pattern in the title (e.g `SSPP xxx: CSC`)
> > that clearly tells the blocks and sub-blocks apart.
> >
> > - Marijn
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers
@ 2023-05-30 20:14 ` Dmitry Baryshkov
0 siblings, 0 replies; 32+ messages in thread
From: Dmitry Baryshkov @ 2023-05-30 20:14 UTC (permalink / raw)
To: Abhinav Kumar
Cc: freedreno, linux-arm-msm, Bjorn Andersson, dri-devel,
Stephen Boyd, Marijn Suijten, Sean Paul
On Tue, 30 May 2023 at 20:37, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote:
>
>
>
> On 5/29/2023 2:36 PM, Marijn Suijten wrote:
> > On 2023-05-24 12:18:09, Abhinav Kumar wrote:
> >>
> >>
> >> On 5/24/2023 2:48 AM, Marijn Suijten wrote:
> >>> On 2023-05-23 13:01:13, Abhinav Kumar wrote:
> >>>>
> >>>>
> >>>> On 5/21/2023 10:21 AM, Dmitry Baryshkov wrote:
> >>>>> Drop SSPP-specifig debugfs register dumps in favour of using
> >>>>> debugfs/dri/0/kms or devcoredump.
> >>>>>
> >>>>
> >>>> I did see another series which removes src_blk from the catalog (I am
> >>>> yet to review that one) . Lets assume that one is fine and this change
> >>>> will be going on top of that one right?
> >>>
> >>> It replaces src_blk with directly accessing the blk (non-sub-block)
> >>> directly, because they were overlapping anyway.
> >>>
> >>>> The concern I have with this change is that although I do agree that we
> >>>> should be in favor of using debugfs/dri/0/kms ( i have used it a few
> >>>> times and it works pretty well ), devcoredump does not have the support
> >>>> to dump sub-blocks . Something which we should add with priority because
> >>>> even with DSC blocks with the separation of enc/ctl blocks we need that
> >>>> like I wrote in one of the responses.
> >>>>
> >>>> So the "len" of the blocks having sub-blocks will be ignored in favor of
> >>>> the len of the sub-blocks.
> >>>
> >>> The sub-blocks are not always contiguous with their parent block, are
> >>> they? It's probably better to print the sub-blocks separately with
> >>
> >> Yes, not contiguous otherwise we could have just had them in one big range.
> >>
> >>> clear headers anyway rather than dumping the range parent_blk_base to
> >>> max(parent_blk_base+len, parent_blk_base+sblk_base+sblk_len...).
> >>>
> >>> - Marijn
> >>
> >> When I meant sub-block support to devcoredump, this is how I visualize
> >> them to be printed
> >>
> >> =========================SSPP xxx =======================
> >> =========================SSPP_CSC =======================(for SSPP_xxx)
> >> =========================SSPP_QSEED =====================(for SSPP_xxx)
> >
> > Yeah something along those lines, though I don't really like the `(for
> > SSPP_xxx)` suffix which we should either drop entirely and make the
> > "heading" less of a "heading"
> >
>
> I wrote that "for SSPP_xxx" to explain the idea, ofcourse it wont be
> part of the print itself.
>
> Without that, it matches what you have shared below.
>
>
> > ========================= SSPP xxx =======================
> > ...
> > ------------------------- SSPP_CSC -----------------------
> > ...
> > ------------------------- SSPP_QSEED ---------------------
> > ...
> >
> > And/or inline the numbers:
> >
> > ========================= SSPP xxx =======================
> > ...
> > ----------------------- SSPP_xxx_CSC ---------------------
> > ...
> > ---------------------- SSPP_xxx_QSEED --------------------
> > ...
I'd prefer this format. It eases grepping.
> >
>
> sure this is also fine with me.
>
> > Either works, or any other pattern in the title (e.g `SSPP xxx: CSC`)
> > that clearly tells the blocks and sub-blocks apart.
> >
> > - Marijn
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers
2023-05-30 20:14 ` Dmitry Baryshkov
@ 2023-06-04 22:01 ` Marijn Suijten
-1 siblings, 0 replies; 32+ messages in thread
From: Marijn Suijten @ 2023-06-04 22:01 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Abhinav Kumar, Sean Paul, Bjorn Andersson, dri-devel,
Stephen Boyd, linux-arm-msm, freedreno
On 2023-05-30 23:14:19, Dmitry Baryshkov wrote:
> On Tue, 30 May 2023 at 20:37, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote:
> >
> >
> >
> > On 5/29/2023 2:36 PM, Marijn Suijten wrote:
> > > On 2023-05-24 12:18:09, Abhinav Kumar wrote:
> > >>
> > >>
> > >> On 5/24/2023 2:48 AM, Marijn Suijten wrote:
> > >>> On 2023-05-23 13:01:13, Abhinav Kumar wrote:
> > >>>>
> > >>>>
> > >>>> On 5/21/2023 10:21 AM, Dmitry Baryshkov wrote:
> > >>>>> Drop SSPP-specifig debugfs register dumps in favour of using
> > >>>>> debugfs/dri/0/kms or devcoredump.
> > >>>>>
> > >>>>
> > >>>> I did see another series which removes src_blk from the catalog (I am
> > >>>> yet to review that one) . Lets assume that one is fine and this change
> > >>>> will be going on top of that one right?
> > >>>
> > >>> It replaces src_blk with directly accessing the blk (non-sub-block)
> > >>> directly, because they were overlapping anyway.
> > >>>
> > >>>> The concern I have with this change is that although I do agree that we
> > >>>> should be in favor of using debugfs/dri/0/kms ( i have used it a few
> > >>>> times and it works pretty well ), devcoredump does not have the support
> > >>>> to dump sub-blocks . Something which we should add with priority because
> > >>>> even with DSC blocks with the separation of enc/ctl blocks we need that
> > >>>> like I wrote in one of the responses.
> > >>>>
> > >>>> So the "len" of the blocks having sub-blocks will be ignored in favor of
> > >>>> the len of the sub-blocks.
> > >>>
> > >>> The sub-blocks are not always contiguous with their parent block, are
> > >>> they? It's probably better to print the sub-blocks separately with
> > >>
> > >> Yes, not contiguous otherwise we could have just had them in one big range.
> > >>
> > >>> clear headers anyway rather than dumping the range parent_blk_base to
> > >>> max(parent_blk_base+len, parent_blk_base+sblk_base+sblk_len...).
> > >>>
> > >>> - Marijn
> > >>
> > >> When I meant sub-block support to devcoredump, this is how I visualize
> > >> them to be printed
> > >>
> > >> =========================SSPP xxx =======================
> > >> =========================SSPP_CSC =======================(for SSPP_xxx)
> > >> =========================SSPP_QSEED =====================(for SSPP_xxx)
> > >
> > > Yeah something along those lines, though I don't really like the `(for
> > > SSPP_xxx)` suffix which we should either drop entirely and make the
> > > "heading" less of a "heading"
> > >
> >
> > I wrote that "for SSPP_xxx" to explain the idea, ofcourse it wont be
> > part of the print itself.
> >
> > Without that, it matches what you have shared below.
> >
> >
> > > ========================= SSPP xxx =======================
> > > ...
> > > ------------------------- SSPP_CSC -----------------------
> > > ...
> > > ------------------------- SSPP_QSEED ---------------------
> > > ...
> > >
> > > And/or inline the numbers:
> > >
> > > ========================= SSPP xxx =======================
> > > ...
> > > ----------------------- SSPP_xxx_CSC ---------------------
> > > ...
> > > ---------------------- SSPP_xxx_QSEED --------------------
> > > ...
>
> I'd prefer this format. It eases grepping.
Cool. And let's also have spaces around the names :)
- Marijn
>
> > >
> >
> > sure this is also fine with me.
> >
> > > Either works, or any other pattern in the title (e.g `SSPP xxx: CSC`)
> > > that clearly tells the blocks and sub-blocks apart.
> > >
> > > - Marijn
>
>
>
> --
> With best wishes
> Dmitry
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers
@ 2023-06-04 22:01 ` Marijn Suijten
0 siblings, 0 replies; 32+ messages in thread
From: Marijn Suijten @ 2023-06-04 22:01 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: freedreno, linux-arm-msm, Bjorn Andersson, Abhinav Kumar,
dri-devel, Stephen Boyd, Sean Paul
On 2023-05-30 23:14:19, Dmitry Baryshkov wrote:
> On Tue, 30 May 2023 at 20:37, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote:
> >
> >
> >
> > On 5/29/2023 2:36 PM, Marijn Suijten wrote:
> > > On 2023-05-24 12:18:09, Abhinav Kumar wrote:
> > >>
> > >>
> > >> On 5/24/2023 2:48 AM, Marijn Suijten wrote:
> > >>> On 2023-05-23 13:01:13, Abhinav Kumar wrote:
> > >>>>
> > >>>>
> > >>>> On 5/21/2023 10:21 AM, Dmitry Baryshkov wrote:
> > >>>>> Drop SSPP-specifig debugfs register dumps in favour of using
> > >>>>> debugfs/dri/0/kms or devcoredump.
> > >>>>>
> > >>>>
> > >>>> I did see another series which removes src_blk from the catalog (I am
> > >>>> yet to review that one) . Lets assume that one is fine and this change
> > >>>> will be going on top of that one right?
> > >>>
> > >>> It replaces src_blk with directly accessing the blk (non-sub-block)
> > >>> directly, because they were overlapping anyway.
> > >>>
> > >>>> The concern I have with this change is that although I do agree that we
> > >>>> should be in favor of using debugfs/dri/0/kms ( i have used it a few
> > >>>> times and it works pretty well ), devcoredump does not have the support
> > >>>> to dump sub-blocks . Something which we should add with priority because
> > >>>> even with DSC blocks with the separation of enc/ctl blocks we need that
> > >>>> like I wrote in one of the responses.
> > >>>>
> > >>>> So the "len" of the blocks having sub-blocks will be ignored in favor of
> > >>>> the len of the sub-blocks.
> > >>>
> > >>> The sub-blocks are not always contiguous with their parent block, are
> > >>> they? It's probably better to print the sub-blocks separately with
> > >>
> > >> Yes, not contiguous otherwise we could have just had them in one big range.
> > >>
> > >>> clear headers anyway rather than dumping the range parent_blk_base to
> > >>> max(parent_blk_base+len, parent_blk_base+sblk_base+sblk_len...).
> > >>>
> > >>> - Marijn
> > >>
> > >> When I meant sub-block support to devcoredump, this is how I visualize
> > >> them to be printed
> > >>
> > >> =========================SSPP xxx =======================
> > >> =========================SSPP_CSC =======================(for SSPP_xxx)
> > >> =========================SSPP_QSEED =====================(for SSPP_xxx)
> > >
> > > Yeah something along those lines, though I don't really like the `(for
> > > SSPP_xxx)` suffix which we should either drop entirely and make the
> > > "heading" less of a "heading"
> > >
> >
> > I wrote that "for SSPP_xxx" to explain the idea, ofcourse it wont be
> > part of the print itself.
> >
> > Without that, it matches what you have shared below.
> >
> >
> > > ========================= SSPP xxx =======================
> > > ...
> > > ------------------------- SSPP_CSC -----------------------
> > > ...
> > > ------------------------- SSPP_QSEED ---------------------
> > > ...
> > >
> > > And/or inline the numbers:
> > >
> > > ========================= SSPP xxx =======================
> > > ...
> > > ----------------------- SSPP_xxx_CSC ---------------------
> > > ...
> > > ---------------------- SSPP_xxx_QSEED --------------------
> > > ...
>
> I'd prefer this format. It eases grepping.
Cool. And let's also have spaces around the names :)
- Marijn
>
> > >
> >
> > sure this is also fine with me.
> >
> > > Either works, or any other pattern in the title (e.g `SSPP xxx: CSC`)
> > > that clearly tells the blocks and sub-blocks apart.
> > >
> > > - Marijn
>
>
>
> --
> With best wishes
> Dmitry
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers
2023-05-21 19:16 ` Marijn Suijten
@ 2023-06-14 10:39 ` Marijn Suijten
-1 siblings, 0 replies; 32+ messages in thread
From: Marijn Suijten @ 2023-06-14 10:39 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Rob Clark, Sean Paul, Abhinav Kumar, Stephen Boyd, David Airlie,
Daniel Vetter, Bjorn Andersson, linux-arm-msm, dri-devel,
freedreno
On 2023-05-21 21:16:39, Marijn Suijten wrote:
> On 2023-05-21 20:12:00, Marijn Suijten wrote:
> > On 2023-05-21 20:21:46, Dmitry Baryshkov wrote:
> > > Drop SSPP-specifig debugfs register dumps in favour of using
> > > debugfs/dri/0/kms or devcoredump.
> > >
> > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> >
> > Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
> >
> > > ---
> > > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 25 ---------------------
> > > 1 file changed, 25 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> > > index bfd82c2921af..6c5ebee2f7cd 100644
> > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> > > @@ -727,31 +727,6 @@ int _dpu_hw_sspp_init_debugfs(struct dpu_hw_sspp *hw_pipe, struct dpu_kms *kms,
> > > debugfs_create_xul("features", 0600,
> > > debugfs_root, (unsigned long *)&hw_pipe->cap->features);
> > >
> > > - /* add register dump support */
> > > - dpu_debugfs_create_regset32("src_blk", 0400,
> > > - debugfs_root,
> > > - sblk->src_blk.base + cfg->base,
> > > - sblk->src_blk.len,
>
> Note that this fails to apply on top of
> https://lore.kernel.org/linux-arm-msm/20230429012353.2569481-2-dmitry.baryshkov@linaro.org/
Also noticing just now that this whole patch makes sblk unused:
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c: In function '_dpu_hw_sspp_init_debugfs':
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c:620:41: warning: unused variable 'sblk' [-Wunused-variable]
620 | const struct dpu_sspp_sub_blks *sblk = cfg->sblk;
| ^~~~
- Marijn
>
> - Marijn
>
> > > - kms);
> > > -
> > > - if (cfg->features & BIT(DPU_SSPP_SCALER_QSEED3) ||
> > > - cfg->features & BIT(DPU_SSPP_SCALER_QSEED3LITE) ||
> > > - cfg->features & BIT(DPU_SSPP_SCALER_QSEED2) ||
> > > - cfg->features & BIT(DPU_SSPP_SCALER_QSEED4))
> > > - dpu_debugfs_create_regset32("scaler_blk", 0400,
> > > - debugfs_root,
> > > - sblk->scaler_blk.base + cfg->base,
> > > - sblk->scaler_blk.len,
> > > - kms);
> > > -
> > > - if (cfg->features & BIT(DPU_SSPP_CSC) ||
> > > - cfg->features & BIT(DPU_SSPP_CSC_10BIT))
> > > - dpu_debugfs_create_regset32("csc_blk", 0400,
> > > - debugfs_root,
> > > - sblk->csc_blk.base + cfg->base,
> > > - sblk->csc_blk.len,
> > > - kms);
> > > -
> > > debugfs_create_u32("xin_id",
> > > 0400,
> > > debugfs_root,
> > > --
> > > 2.39.2
> > >
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers
@ 2023-06-14 10:39 ` Marijn Suijten
0 siblings, 0 replies; 32+ messages in thread
From: Marijn Suijten @ 2023-06-14 10:39 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: freedreno, Sean Paul, Bjorn Andersson, Abhinav Kumar, dri-devel,
Stephen Boyd, linux-arm-msm
On 2023-05-21 21:16:39, Marijn Suijten wrote:
> On 2023-05-21 20:12:00, Marijn Suijten wrote:
> > On 2023-05-21 20:21:46, Dmitry Baryshkov wrote:
> > > Drop SSPP-specifig debugfs register dumps in favour of using
> > > debugfs/dri/0/kms or devcoredump.
> > >
> > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> >
> > Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
> >
> > > ---
> > > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 25 ---------------------
> > > 1 file changed, 25 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> > > index bfd82c2921af..6c5ebee2f7cd 100644
> > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> > > @@ -727,31 +727,6 @@ int _dpu_hw_sspp_init_debugfs(struct dpu_hw_sspp *hw_pipe, struct dpu_kms *kms,
> > > debugfs_create_xul("features", 0600,
> > > debugfs_root, (unsigned long *)&hw_pipe->cap->features);
> > >
> > > - /* add register dump support */
> > > - dpu_debugfs_create_regset32("src_blk", 0400,
> > > - debugfs_root,
> > > - sblk->src_blk.base + cfg->base,
> > > - sblk->src_blk.len,
>
> Note that this fails to apply on top of
> https://lore.kernel.org/linux-arm-msm/20230429012353.2569481-2-dmitry.baryshkov@linaro.org/
Also noticing just now that this whole patch makes sblk unused:
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c: In function '_dpu_hw_sspp_init_debugfs':
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c:620:41: warning: unused variable 'sblk' [-Wunused-variable]
620 | const struct dpu_sspp_sub_blks *sblk = cfg->sblk;
| ^~~~
- Marijn
>
> - Marijn
>
> > > - kms);
> > > -
> > > - if (cfg->features & BIT(DPU_SSPP_SCALER_QSEED3) ||
> > > - cfg->features & BIT(DPU_SSPP_SCALER_QSEED3LITE) ||
> > > - cfg->features & BIT(DPU_SSPP_SCALER_QSEED2) ||
> > > - cfg->features & BIT(DPU_SSPP_SCALER_QSEED4))
> > > - dpu_debugfs_create_regset32("scaler_blk", 0400,
> > > - debugfs_root,
> > > - sblk->scaler_blk.base + cfg->base,
> > > - sblk->scaler_blk.len,
> > > - kms);
> > > -
> > > - if (cfg->features & BIT(DPU_SSPP_CSC) ||
> > > - cfg->features & BIT(DPU_SSPP_CSC_10BIT))
> > > - dpu_debugfs_create_regset32("csc_blk", 0400,
> > > - debugfs_root,
> > > - sblk->csc_blk.base + cfg->base,
> > > - sblk->csc_blk.len,
> > > - kms);
> > > -
> > > debugfs_create_u32("xin_id",
> > > 0400,
> > > debugfs_root,
> > > --
> > > 2.39.2
> > >
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers
2023-06-14 10:39 ` Marijn Suijten
@ 2023-06-14 10:41 ` Marijn Suijten
-1 siblings, 0 replies; 32+ messages in thread
From: Marijn Suijten @ 2023-06-14 10:41 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Rob Clark, Sean Paul, Abhinav Kumar, Stephen Boyd, David Airlie,
Daniel Vetter, Bjorn Andersson, linux-arm-msm, dri-devel,
freedreno
On 2023-06-14 12:39:13, Marijn Suijten wrote:
<snip>
> > > > - /* add register dump support */
> > > > - dpu_debugfs_create_regset32("src_blk", 0400,
> > > > - debugfs_root,
> > > > - sblk->src_blk.base + cfg->base,
> > > > - sblk->src_blk.len,
> >
> > Note that this fails to apply on top of
> > https://lore.kernel.org/linux-arm-msm/20230429012353.2569481-2-dmitry.baryshkov@linaro.org/
>
> Also noticing just now that this whole patch makes sblk unused:
... thanks to rebasing on top of [1], which is now applied.
[1]: https://lore.kernel.org/all/20230518222238.3815293-6-dmitry.baryshkov@linaro.org/
- Marijn
>
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c: In function '_dpu_hw_sspp_init_debugfs':
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c:620:41: warning: unused variable 'sblk' [-Wunused-variable]
> 620 | const struct dpu_sspp_sub_blks *sblk = cfg->sblk;
> | ^~~~
>
> - Marijn
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers
@ 2023-06-14 10:41 ` Marijn Suijten
0 siblings, 0 replies; 32+ messages in thread
From: Marijn Suijten @ 2023-06-14 10:41 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: freedreno, Sean Paul, Bjorn Andersson, Abhinav Kumar, dri-devel,
Stephen Boyd, linux-arm-msm
On 2023-06-14 12:39:13, Marijn Suijten wrote:
<snip>
> > > > - /* add register dump support */
> > > > - dpu_debugfs_create_regset32("src_blk", 0400,
> > > > - debugfs_root,
> > > > - sblk->src_blk.base + cfg->base,
> > > > - sblk->src_blk.len,
> >
> > Note that this fails to apply on top of
> > https://lore.kernel.org/linux-arm-msm/20230429012353.2569481-2-dmitry.baryshkov@linaro.org/
>
> Also noticing just now that this whole patch makes sblk unused:
... thanks to rebasing on top of [1], which is now applied.
[1]: https://lore.kernel.org/all/20230518222238.3815293-6-dmitry.baryshkov@linaro.org/
- Marijn
>
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c: In function '_dpu_hw_sspp_init_debugfs':
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c:620:41: warning: unused variable 'sblk' [-Wunused-variable]
> 620 | const struct dpu_sspp_sub_blks *sblk = cfg->sblk;
> | ^~~~
>
> - Marijn
^ permalink raw reply [flat|nested] 32+ messages in thread
end of thread, other threads:[~2023-06-14 10:41 UTC | newest]
Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-21 17:21 [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers Dmitry Baryshkov
2023-05-21 17:21 ` Dmitry Baryshkov
2023-05-21 17:21 ` [PATCH 2/2] drm/msm/dpu: drop debugfs regset32 support Dmitry Baryshkov
2023-05-21 17:21 ` Dmitry Baryshkov
2023-05-21 18:14 ` Marijn Suijten
2023-05-21 18:14 ` Marijn Suijten
2023-05-21 18:11 ` [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers Marijn Suijten
2023-05-21 18:11 ` Marijn Suijten
2023-05-21 19:16 ` Marijn Suijten
2023-05-21 19:16 ` Marijn Suijten
2023-06-14 10:39 ` Marijn Suijten
2023-06-14 10:39 ` Marijn Suijten
2023-06-14 10:41 ` Marijn Suijten
2023-06-14 10:41 ` Marijn Suijten
2023-05-23 20:01 ` Abhinav Kumar
2023-05-23 20:01 ` Abhinav Kumar
2023-05-23 20:27 ` Dmitry Baryshkov
2023-05-23 20:27 ` Dmitry Baryshkov
2023-05-24 9:48 ` Marijn Suijten
2023-05-24 9:48 ` Marijn Suijten
2023-05-24 10:24 ` Dmitry Baryshkov
2023-05-24 10:24 ` Dmitry Baryshkov
2023-05-24 19:18 ` Abhinav Kumar
2023-05-24 19:18 ` Abhinav Kumar
2023-05-29 21:36 ` Marijn Suijten
2023-05-29 21:36 ` Marijn Suijten
2023-05-30 17:37 ` Abhinav Kumar
2023-05-30 17:37 ` Abhinav Kumar
2023-05-30 20:14 ` Dmitry Baryshkov
2023-05-30 20:14 ` Dmitry Baryshkov
2023-06-04 22:01 ` Marijn Suijten
2023-06-04 22:01 ` Marijn Suijten
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.