[PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers

[PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers

[Dmitry Baryshkov]

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

[PATCH 2/2] drm/msm/dpu: drop debugfs regset32 support

[Dmitry Baryshkov]

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

Re: [PATCH 2/2] drm/msm/dpu: drop debugfs regset32 support

[Marijn Suijten]

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
> 

Re: [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers

[Marijn Suijten]

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
> 

Re: [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers

[Marijn Suijten]

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
> > 

Re: [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers

[Marijn Suijten]

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
> > > 

Re: [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers

[Marijn Suijten]

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

Re: [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers

[Abhinav Kumar]

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,

Re: [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers

[Dmitry Baryshkov]

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

Re: [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers

[Marijn Suijten]

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>

Re: [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers

[Dmitry Baryshkov]

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

Re: [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers

[Abhinav Kumar]

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>

Re: [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers

[Marijn Suijten]

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

Re: [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers

[Abhinav Kumar]

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

Re: [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers

[Dmitry Baryshkov]

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

Re: [PATCH 1/2] drm/msm/dpu: drop SSPP register dumpers

[Marijn Suijten]

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