[PATCH 0/3] Remove #ifdef ISP2401 and unifying sh_css_sp_group structure

[PATCH 0/3] Remove #ifdef ISP2401 and unifying sh_css_sp_group structure

[Kate Hsuan]

sh_css_sp_group structure was managed by #ifdef ISP2401. The configuration
structure was also determined in compile time which means we have to
compile the kernel code twice for each ISP2400 and ISP2401.

In order to determine the ISP types in runtime, the configuration
structure needs to be unified. It allows can be used to configure both
types of ISP. Since the length of the sh_css_sp_group for ISP2400 and
ISP2401 is different, before submitting the configuration to ISP, the data
has to be carefully put the necessary part to a buffer and then submitted
to the ISP.

Kate Hsuan (3):
  media: atomisp: sh_css_internal: Unifying sh_css_sp_group to remove
    #ifdef ISP2401
  atomisp: sh_css_params: write the sp_group config according to the ISP
    model
  media: atomisp: ia_css_debug: remove unused codes

 .../runtime/debug/interface/ia_css_debug.h    |  6 --
 .../pci/runtime/debug/src/ia_css_debug.c      | 76 +------------------
 .../media/atomisp/pci/sh_css_internal.h       | 35 +++------
 .../staging/media/atomisp/pci/sh_css_params.c | 37 ++++++++-
 4 files changed, 48 insertions(+), 106 deletions(-)

-- 
2.40.1

[PATCH 1/3] media: atomisp: sh_css_internal: Unifying sh_css_sp_group to remove #ifdef ISP2401

[Kate Hsuan]

Since some parts of the data scructure elements are determined in compile
time, the configuration data structure should be compiled for both two
ISP models. In order to set the configuration for both ISP model in
runtime, Thesh_css_sp_group is unified to one data structure for the
configuration to ensure the data structure can be used for both ISP2400
and 2401.

Signed-off-by: Kate Hsuan <hpa@redhat.com>
---
 .../media/atomisp/pci/sh_css_internal.h       | 35 ++++++-------------
 1 file changed, 11 insertions(+), 24 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/sh_css_internal.h b/drivers/staging/media/atomisp/pci/sh_css_internal.h
index d98f1323441e..2fa0b3e45fe0 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_internal.h
+++ b/drivers/staging/media/atomisp/pci/sh_css_internal.h
@@ -22,9 +22,7 @@
 #include <platform_support.h>
 #include <linux/stdarg.h>
 
-#if !defined(ISP2401)
 #include "input_formatter.h"
-#endif
 #include "input_system.h"
 
 #include "ia_css_types.h"
@@ -95,19 +93,20 @@
  *	 these threads can't be used as image pipe
  */
 
-#if !defined(ISP2401)
-#define SH_CSS_SP_INTERNAL_METADATA_THREAD	1
-#else
-#define SH_CSS_SP_INTERNAL_METADATA_THREAD	0
-#endif
+#define SH_CSS_SP_INTERNAL_METADATA_THREAD_2400	1
+#define SH_CSS_SP_INTERNAL_METADATA_THREAD_2401	0
 
 #define SH_CSS_SP_INTERNAL_SERVICE_THREAD		1
 
 #define SH_CSS_MAX_SP_THREADS		5
 
-#define SH_CSS_MAX_SP_INTERNAL_THREADS	(\
-	 SH_CSS_SP_INTERNAL_SERVICE_THREAD +\
-	 SH_CSS_SP_INTERNAL_METADATA_THREAD)
+#define SH_CSS_MAX_SP_INTERNAL_THREADS_2400	(\
+	SH_CSS_SP_INTERNAL_SERVICE_THREAD +\
+	 SH_CSS_SP_INTERNAL_METADATA_THREAD_2400)
+
+#define SH_CSS_MAX_SP_INTERNAL_THREADS_2401	(\
+	SH_CSS_SP_INTERNAL_SERVICE_THREAD +\
+	 SH_CSS_SP_INTERNAL_METADATA_THREAD_2401)
 
 #define SH_CSS_MAX_PIPELINES	SH_CSS_MAX_SP_THREADS
 
@@ -357,14 +356,12 @@ struct sh_css_sp_debug_command {
 	u32 dma_sw_reg;
 };
 
-#if !defined(ISP2401)
 /* SP input formatter configuration.*/
 struct sh_css_sp_input_formatter_set {
 	u32				stream_format;
 	input_formatter_cfg_t	config_a;
 	input_formatter_cfg_t	config_b;
 };
-#endif
 
 #define IA_CSS_MIPI_SIZE_CHECK_MAX_NOF_ENTRIES_PER_PORT (3)
 
@@ -377,7 +374,7 @@ struct sh_css_sp_config {
 	     frames are locked when their EOF event is successfully sent to the
 	     host (true) or when they are passed to the preview/video pipe
 	     (false). */
-#if !defined(ISP2401)
+
 	struct {
 		u8					a_changed;
 		u8					b_changed;
@@ -385,15 +382,13 @@ struct sh_css_sp_config {
 		struct sh_css_sp_input_formatter_set
 			set[SH_CSS_MAX_IF_CONFIGS]; /* CSI-2 port is used as index. */
 	} input_formatter;
-#endif
-#if !defined(ISP2401)
+
 	sync_generator_cfg_t	sync_gen;
 	tpg_cfg_t		tpg;
 	prbs_cfg_t		prbs;
 	input_system_cfg_t	input_circuit;
 	u8			input_circuit_cfg_changed;
 	u32		mipi_sizes_for_check[N_CSI_PORTS][IA_CSS_MIPI_SIZE_CHECK_MAX_NOF_ENTRIES_PER_PORT];
-#endif
 	u8                 enable_isys_event_queue;
 	u8			disable_cont_vf;
 };
@@ -409,7 +404,6 @@ enum sh_css_stage_type {
 #define SH_CSS_PIPE_CONFIG_SAMPLE_PARAMS_MASK \
 	((SH_CSS_PIPE_CONFIG_SAMPLE_PARAMS << SH_CSS_MAX_SP_THREADS) - 1)
 
-#if defined(ISP2401)
 struct sh_css_sp_pipeline_terminal {
 	union {
 		/* Input System 2401 */
@@ -442,7 +436,6 @@ struct sh_css_sp_pipeline_io_status {
 	u32	running[N_INPUT_SYSTEM_CSI_PORT];	/** configured streams */
 };
 
-#endif
 enum sh_css_port_dir {
 	SH_CSS_PORT_INPUT  = 0,
 	SH_CSS_PORT_OUTPUT  = 1
@@ -641,10 +634,8 @@ struct sh_css_sp_stage {
 struct sh_css_sp_group {
 	struct sh_css_sp_config		config;
 	struct sh_css_sp_pipeline	pipe[SH_CSS_MAX_SP_THREADS];
-#if defined(ISP2401)
 	struct sh_css_sp_pipeline_io	pipe_io[SH_CSS_MAX_SP_THREADS];
 	struct sh_css_sp_pipeline_io_status	pipe_io_status;
-#endif
 	struct sh_css_sp_debug_command	debug;
 };
 
@@ -922,13 +913,11 @@ sh_css_frame_info_set_width(struct ia_css_frame_info *info,
 			    unsigned int width,
 			    unsigned int aligned);
 
-#if !defined(ISP2401)
 
 unsigned int
 sh_css_get_mipi_sizes_for_check(const unsigned int port,
 				const unsigned int idx);
 
-#endif
 
 ia_css_ptr
 sh_css_store_sp_group_to_ddr(void);
@@ -971,11 +960,9 @@ sh_css_continuous_is_enabled(uint8_t pipe_num);
 struct ia_css_pipe *
 find_pipe_by_num(uint32_t pipe_num);
 
-#ifdef ISP2401
 void
 ia_css_get_crop_offsets(
     struct ia_css_pipe *pipe,
     struct ia_css_frame_info *in_frame);
-#endif
 
 #endif /* _SH_CSS_INTERNAL_H_ */
-- 
2.40.1

Re: [PATCH 1/3] media: atomisp: sh_css_internal: Unifying sh_css_sp_group to remove #ifdef ISP2401

[Hans de Goede]

Hi,

On 6/5/23 12:29, Kate Hsuan wrote:
> Since some parts of the data scructure elements are determined in compile
> time, the configuration data structure should be compiled for both two
> ISP models. In order to set the configuration for both ISP model in
> runtime, Thesh_css_sp_group is unified to one data structure for the
> configuration to ensure the data structure can be used for both ISP2400
> and 2401.
> 
> Signed-off-by: Kate Hsuan <hpa@redhat.com>

Please squash this patch into patch 2/3 even if the intermediate results with just this patch compiles it will not run properly, so just fold the 2 patches into 1 please.

Regards,

Hans



> ---
>  .../media/atomisp/pci/sh_css_internal.h       | 35 ++++++-------------
>  1 file changed, 11 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/staging/media/atomisp/pci/sh_css_internal.h b/drivers/staging/media/atomisp/pci/sh_css_internal.h
> index d98f1323441e..2fa0b3e45fe0 100644
> --- a/drivers/staging/media/atomisp/pci/sh_css_internal.h
> +++ b/drivers/staging/media/atomisp/pci/sh_css_internal.h
> @@ -22,9 +22,7 @@
>  #include <platform_support.h>
>  #include <linux/stdarg.h>
>  
> -#if !defined(ISP2401)
>  #include "input_formatter.h"
> -#endif
>  #include "input_system.h"
>  
>  #include "ia_css_types.h"
> @@ -95,19 +93,20 @@
>   *	 these threads can't be used as image pipe
>   */
>  
> -#if !defined(ISP2401)
> -#define SH_CSS_SP_INTERNAL_METADATA_THREAD	1
> -#else
> -#define SH_CSS_SP_INTERNAL_METADATA_THREAD	0
> -#endif
> +#define SH_CSS_SP_INTERNAL_METADATA_THREAD_2400	1
> +#define SH_CSS_SP_INTERNAL_METADATA_THREAD_2401	0
>  
>  #define SH_CSS_SP_INTERNAL_SERVICE_THREAD		1
>  
>  #define SH_CSS_MAX_SP_THREADS		5
>  
> -#define SH_CSS_MAX_SP_INTERNAL_THREADS	(\
> -	 SH_CSS_SP_INTERNAL_SERVICE_THREAD +\
> -	 SH_CSS_SP_INTERNAL_METADATA_THREAD)
> +#define SH_CSS_MAX_SP_INTERNAL_THREADS_2400	(\
> +	SH_CSS_SP_INTERNAL_SERVICE_THREAD +\
> +	 SH_CSS_SP_INTERNAL_METADATA_THREAD_2400)
> +
> +#define SH_CSS_MAX_SP_INTERNAL_THREADS_2401	(\
> +	SH_CSS_SP_INTERNAL_SERVICE_THREAD +\
> +	 SH_CSS_SP_INTERNAL_METADATA_THREAD_2401)
>  
>  #define SH_CSS_MAX_PIPELINES	SH_CSS_MAX_SP_THREADS
>  
> @@ -357,14 +356,12 @@ struct sh_css_sp_debug_command {
>  	u32 dma_sw_reg;
>  };
>  
> -#if !defined(ISP2401)
>  /* SP input formatter configuration.*/
>  struct sh_css_sp_input_formatter_set {
>  	u32				stream_format;
>  	input_formatter_cfg_t	config_a;
>  	input_formatter_cfg_t	config_b;
>  };
> -#endif
>  
>  #define IA_CSS_MIPI_SIZE_CHECK_MAX_NOF_ENTRIES_PER_PORT (3)
>  
> @@ -377,7 +374,7 @@ struct sh_css_sp_config {
>  	     frames are locked when their EOF event is successfully sent to the
>  	     host (true) or when they are passed to the preview/video pipe
>  	     (false). */
> -#if !defined(ISP2401)
> +
>  	struct {
>  		u8					a_changed;
>  		u8					b_changed;
> @@ -385,15 +382,13 @@ struct sh_css_sp_config {
>  		struct sh_css_sp_input_formatter_set
>  			set[SH_CSS_MAX_IF_CONFIGS]; /* CSI-2 port is used as index. */
>  	} input_formatter;
> -#endif
> -#if !defined(ISP2401)
> +
>  	sync_generator_cfg_t	sync_gen;
>  	tpg_cfg_t		tpg;
>  	prbs_cfg_t		prbs;
>  	input_system_cfg_t	input_circuit;
>  	u8			input_circuit_cfg_changed;
>  	u32		mipi_sizes_for_check[N_CSI_PORTS][IA_CSS_MIPI_SIZE_CHECK_MAX_NOF_ENTRIES_PER_PORT];
> -#endif
>  	u8                 enable_isys_event_queue;
>  	u8			disable_cont_vf;
>  };
> @@ -409,7 +404,6 @@ enum sh_css_stage_type {
>  #define SH_CSS_PIPE_CONFIG_SAMPLE_PARAMS_MASK \
>  	((SH_CSS_PIPE_CONFIG_SAMPLE_PARAMS << SH_CSS_MAX_SP_THREADS) - 1)
>  
> -#if defined(ISP2401)
>  struct sh_css_sp_pipeline_terminal {
>  	union {
>  		/* Input System 2401 */
> @@ -442,7 +436,6 @@ struct sh_css_sp_pipeline_io_status {
>  	u32	running[N_INPUT_SYSTEM_CSI_PORT];	/** configured streams */
>  };
>  
> -#endif
>  enum sh_css_port_dir {
>  	SH_CSS_PORT_INPUT  = 0,
>  	SH_CSS_PORT_OUTPUT  = 1
> @@ -641,10 +634,8 @@ struct sh_css_sp_stage {
>  struct sh_css_sp_group {
>  	struct sh_css_sp_config		config;
>  	struct sh_css_sp_pipeline	pipe[SH_CSS_MAX_SP_THREADS];
> -#if defined(ISP2401)
>  	struct sh_css_sp_pipeline_io	pipe_io[SH_CSS_MAX_SP_THREADS];
>  	struct sh_css_sp_pipeline_io_status	pipe_io_status;
> -#endif
>  	struct sh_css_sp_debug_command	debug;
>  };
>  
> @@ -922,13 +913,11 @@ sh_css_frame_info_set_width(struct ia_css_frame_info *info,
>  			    unsigned int width,
>  			    unsigned int aligned);
>  
> -#if !defined(ISP2401)
>  
>  unsigned int
>  sh_css_get_mipi_sizes_for_check(const unsigned int port,
>  				const unsigned int idx);
>  
> -#endif
>  
>  ia_css_ptr
>  sh_css_store_sp_group_to_ddr(void);
> @@ -971,11 +960,9 @@ sh_css_continuous_is_enabled(uint8_t pipe_num);
>  struct ia_css_pipe *
>  find_pipe_by_num(uint32_t pipe_num);
>  
> -#ifdef ISP2401
>  void
>  ia_css_get_crop_offsets(
>      struct ia_css_pipe *pipe,
>      struct ia_css_frame_info *in_frame);
> -#endif
>  
>  #endif /* _SH_CSS_INTERNAL_H_ */

[PATCH 2/3] atomisp: sh_css_params: write the sp_group config according to the ISP model

[Kate Hsuan]

Pick up the necessary part of sp_group configuration for both model and
then copy those parts into a tempetory buffer. This buffer is finally
written to the ISP with correct length.

Signed-off-by: Kate Hsuan <hpa@redhat.com>
---
 .../staging/media/atomisp/pci/sh_css_params.c | 37 ++++++++++++++++++-
 1 file changed, 35 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/sh_css_params.c b/drivers/staging/media/atomisp/pci/sh_css_params.c
index 588f2adab058..2913d9d6d226 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_params.c
+++ b/drivers/staging/media/atomisp/pci/sh_css_params.c
@@ -3720,10 +3720,43 @@ struct ia_css_shading_table *ia_css_get_shading_table(struct ia_css_stream
 
 ia_css_ptr sh_css_store_sp_group_to_ddr(void)
 {
+	u8 *write_buf;
+	u8 *buf_ptr;
+
 	IA_CSS_ENTER_LEAVE_PRIVATE("void");
+
+	write_buf = kzalloc(sizeof(u8) * 8192, GFP_KERNEL);
+
+	buf_ptr = write_buf;
+	if (IS_ISP2401) {
+		memcpy(buf_ptr, &sh_css_sp_group.config, sizeof(u8) * 5);
+		buf_ptr += (sizeof(u8) * 5);
+		memset(buf_ptr, 0, 3);
+		buf_ptr += 3; /* Padding 3 bytes for struct sh_css_sp_config*/
+	} else {
+		memcpy(buf_ptr, &sh_css_sp_group.config, sizeof(sh_css_sp_group.config));
+		buf_ptr += sizeof(sh_css_sp_group.config);
+	}
+
+	memcpy(buf_ptr, &sh_css_sp_group.pipe, sizeof(sh_css_sp_group.pipe));
+	buf_ptr += sizeof(sh_css_sp_group.pipe);
+
+	if (IS_ISP2401) {
+		memcpy(buf_ptr, &sh_css_sp_group.pipe_io, sizeof(sh_css_sp_group.pipe_io));
+		buf_ptr += sizeof(sh_css_sp_group.pipe_io);
+		memcpy(buf_ptr, &sh_css_sp_group.pipe_io_status,
+		       sizeof(sh_css_sp_group.pipe_io_status));
+		buf_ptr += sizeof(sh_css_sp_group.pipe_io_status);
+	}
+
+	memcpy(buf_ptr, &sh_css_sp_group.debug, sizeof(sh_css_sp_group.debug));
+	buf_ptr += sizeof(sh_css_sp_group.debug);
+
 	hmm_store(xmem_sp_group_ptrs,
-		   &sh_css_sp_group,
-		   sizeof(struct sh_css_sp_group));
+		  write_buf,
+		  buf_ptr - write_buf);
+
+	kfree(write_buf);
 	return xmem_sp_group_ptrs;
 }
 
-- 
2.40.1

Re: [PATCH 2/3] atomisp: sh_css_params: write the sp_group config according to the ISP model

[Dan Carpenter]

On Mon, Jun 05, 2023 at 06:29:02PM +0800, Kate Hsuan wrote:
> Pick up the necessary part of sp_group configuration for both model and
> then copy those parts into a tempetory buffer. This buffer is finally
> written to the ISP with correct length.
> 
> Signed-off-by: Kate Hsuan <hpa@redhat.com>
> ---
>  .../staging/media/atomisp/pci/sh_css_params.c | 37 ++++++++++++++++++-
>  1 file changed, 35 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/media/atomisp/pci/sh_css_params.c b/drivers/staging/media/atomisp/pci/sh_css_params.c
> index 588f2adab058..2913d9d6d226 100644
> --- a/drivers/staging/media/atomisp/pci/sh_css_params.c
> +++ b/drivers/staging/media/atomisp/pci/sh_css_params.c
> @@ -3720,10 +3720,43 @@ struct ia_css_shading_table *ia_css_get_shading_table(struct ia_css_stream
>  
>  ia_css_ptr sh_css_store_sp_group_to_ddr(void)
>  {
> +	u8 *write_buf;
> +	u8 *buf_ptr;
> +
>  	IA_CSS_ENTER_LEAVE_PRIVATE("void");
> +
> +	write_buf = kzalloc(sizeof(u8) * 8192, GFP_KERNEL);

Please add a check for allocation failure.

regards,
dan carpenter


> +
> +	buf_ptr = write_buf;
> +	if (IS_ISP2401) {
> +		memcpy(buf_ptr, &sh_css_sp_group.config, sizeof(u8) * 5);
> +		buf_ptr += (sizeof(u8) * 5);
> +		memset(buf_ptr, 0, 3);
> +		buf_ptr += 3; /* Padding 3 bytes for struct sh_css_sp_config*/
> +	} else {
> +		memcpy(buf_ptr, &sh_css_sp_group.config, sizeof(sh_css_sp_group.config));
> +		buf_ptr += sizeof(sh_css_sp_group.config);
> +	}

Re: [PATCH 2/3] atomisp: sh_css_params: write the sp_group config according to the ISP model

[Kate Hsuan]

Hi,

On Mon, Jun 5, 2023 at 7:11 PM Dan Carpenter <dan.carpenter@linaro.org> wrote:
>
> On Mon, Jun 05, 2023 at 06:29:02PM +0800, Kate Hsuan wrote:
> > Pick up the necessary part of sp_group configuration for both model and
> > then copy those parts into a tempetory buffer. This buffer is finally
> > written to the ISP with correct length.
> >
> > Signed-off-by: Kate Hsuan <hpa@redhat.com>
> > ---
> >  .../staging/media/atomisp/pci/sh_css_params.c | 37 ++++++++++++++++++-
> >  1 file changed, 35 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/staging/media/atomisp/pci/sh_css_params.c b/drivers/staging/media/atomisp/pci/sh_css_params.c
> > index 588f2adab058..2913d9d6d226 100644
> > --- a/drivers/staging/media/atomisp/pci/sh_css_params.c
> > +++ b/drivers/staging/media/atomisp/pci/sh_css_params.c
> > @@ -3720,10 +3720,43 @@ struct ia_css_shading_table *ia_css_get_shading_table(struct ia_css_stream
> >
> >  ia_css_ptr sh_css_store_sp_group_to_ddr(void)
> >  {
> > +     u8 *write_buf;
> > +     u8 *buf_ptr;
> > +
> >       IA_CSS_ENTER_LEAVE_PRIVATE("void");
> > +
> > +     write_buf = kzalloc(sizeof(u8) * 8192, GFP_KERNEL);
>
> Please add a check for allocation failure.
>
> regards,
> dan carpenter

Thank you for reviewing this.
I will fix it in the v2 patch.

>
>
> > +
> > +     buf_ptr = write_buf;
> > +     if (IS_ISP2401) {
> > +             memcpy(buf_ptr, &sh_css_sp_group.config, sizeof(u8) * 5);
> > +             buf_ptr += (sizeof(u8) * 5);
> > +             memset(buf_ptr, 0, 3);
> > +             buf_ptr += 3; /* Padding 3 bytes for struct sh_css_sp_config*/
> > +     } else {
> > +             memcpy(buf_ptr, &sh_css_sp_group.config, sizeof(sh_css_sp_group.config));
> > +             buf_ptr += sizeof(sh_css_sp_group.config);
> > +     }
>


-- 
BR,
Kate

Re: [PATCH 2/3] atomisp: sh_css_params: write the sp_group config according to the ISP model

[Hans de Goede]

Hi Kate,

On 6/5/23 12:29, Kate Hsuan wrote:
> Pick up the necessary part of sp_group configuration for both model and
> then copy those parts into a tempetory buffer. This buffer is finally
> written to the ISP with correct length.
> 
> Signed-off-by: Kate Hsuan <hpa@redhat.com>
> ---
>  .../staging/media/atomisp/pci/sh_css_params.c | 37 ++++++++++++++++++-
>  1 file changed, 35 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/media/atomisp/pci/sh_css_params.c b/drivers/staging/media/atomisp/pci/sh_css_params.c
> index 588f2adab058..2913d9d6d226 100644
> --- a/drivers/staging/media/atomisp/pci/sh_css_params.c
> +++ b/drivers/staging/media/atomisp/pci/sh_css_params.c
> @@ -3720,10 +3720,43 @@ struct ia_css_shading_table *ia_css_get_shading_table(struct ia_css_stream
>  
>  ia_css_ptr sh_css_store_sp_group_to_ddr(void)
>  {
> +	u8 *write_buf;
> +	u8 *buf_ptr;
> +
>  	IA_CSS_ENTER_LEAVE_PRIVATE("void");
> +
> +	write_buf = kzalloc(sizeof(u8) * 8192, GFP_KERNEL);

Please use sizeof(struct sh_css_sp_group) here, since you have dropped all the #ifdef-s in the header now that is the largest size which you need now.

> +
> +	buf_ptr = write_buf;
> +	if (IS_ISP2401) {
> +		memcpy(buf_ptr, &sh_css_sp_group.config, sizeof(u8) * 5);

This is wrong, there is a big hole between:

        u8                      no_isp_sync; /* Signal host immediately after start */
        u8                      enable_raw_pool_locking; /** Enable Raw Buffer Locking for HAL
        u8                      lock_all;

and:

        u8                 enable_isys_event_queue;
        u8                      disable_cont_vf;

filled with ISP2400 specific data now, so you are copying what likely is some empty alignment bytes before the ISP2400 specific input_formatter struct now instead of copying enable_isys_event_queue
and disable_cont_vf.

So you need to split the memcpy into 2 memcpy calls. You can calculate the source offset of enable_isys_event_queue in sh_css_sp_group.config with offsetof(struct sh_css_sp_config, enable_isys_event_queue), or better yet, just take the address of it:

	if (IS_ISP2401) {
		memcpy(buf_ptr, &sh_css_sp_group.config, 3);
		buf_ptr += 3;
		memcpy(buf_ptr, &sh_css_sp_group.config.enable_isys_event_queue, 2);
		buf_ptr += 2;
		memset(buf_ptr, 0, 3);
		buf_ptr += 3; /* Padding 3 bytes for struct sh_css_sp_config*/
	} else {

Also note I have dropped the "* sizeof(u8)" here, you already dropped that yourself for the memset / padding bits and dropping it makes the code easier to read IMHO.
		


> +		buf_ptr += (sizeof(u8) * 5);
> +		memset(buf_ptr, 0, 3);
> +		buf_ptr += 3; /* Padding 3 bytes for struct sh_css_sp_config*/
> +	} else {
> +		memcpy(buf_ptr, &sh_css_sp_group.config, sizeof(sh_css_sp_group.config));
> +		buf_ptr += sizeof(sh_css_sp_group.config);
> +	}
> +
> +	memcpy(buf_ptr, &sh_css_sp_group.pipe, sizeof(sh_css_sp_group.pipe));
> +	buf_ptr += sizeof(sh_css_sp_group.pipe);
> +
> +	if (IS_ISP2401) {
> +		memcpy(buf_ptr, &sh_css_sp_group.pipe_io, sizeof(sh_css_sp_group.pipe_io));
> +		buf_ptr += sizeof(sh_css_sp_group.pipe_io);
> +		memcpy(buf_ptr, &sh_css_sp_group.pipe_io_status,
> +		       sizeof(sh_css_sp_group.pipe_io_status));
> +		buf_ptr += sizeof(sh_css_sp_group.pipe_io_status);
> +	}
> +
> +	memcpy(buf_ptr, &sh_css_sp_group.debug, sizeof(sh_css_sp_group.debug));
> +	buf_ptr += sizeof(sh_css_sp_group.debug);
> +
>  	hmm_store(xmem_sp_group_ptrs,
> -		   &sh_css_sp_group,
> -		   sizeof(struct sh_css_sp_group));
> +		  write_buf,
> +		  buf_ptr - write_buf);
> +
> +	kfree(write_buf);
>  	return xmem_sp_group_ptrs;
>  }
>  

The rest looks good at a quick glance, but I need to take a closer look later.

Regards,

Hans

Re: [PATCH 2/3] atomisp: sh_css_params: write the sp_group config according to the ISP model

[Hans de Goede]

Hi Kate,

On 6/6/23 13:02, Hans de Goede wrote:
> Hi Kate,
> 
> On 6/5/23 12:29, Kate Hsuan wrote:
>> Pick up the necessary part of sp_group configuration for both model and
>> then copy those parts into a tempetory buffer. This buffer is finally
>> written to the ISP with correct length.
>>
>> Signed-off-by: Kate Hsuan <hpa@redhat.com>
>> ---
>>  .../staging/media/atomisp/pci/sh_css_params.c | 37 ++++++++++++++++++-
>>  1 file changed, 35 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/staging/media/atomisp/pci/sh_css_params.c b/drivers/staging/media/atomisp/pci/sh_css_params.c
>> index 588f2adab058..2913d9d6d226 100644
>> --- a/drivers/staging/media/atomisp/pci/sh_css_params.c
>> +++ b/drivers/staging/media/atomisp/pci/sh_css_params.c
>> @@ -3720,10 +3720,43 @@ struct ia_css_shading_table *ia_css_get_shading_table(struct ia_css_stream
>>  
>>  ia_css_ptr sh_css_store_sp_group_to_ddr(void)
>>  {
>> +	u8 *write_buf;
>> +	u8 *buf_ptr;
>> +
>>  	IA_CSS_ENTER_LEAVE_PRIVATE("void");
>> +
>> +	write_buf = kzalloc(sizeof(u8) * 8192, GFP_KERNEL);
> 
> Please use sizeof(struct sh_css_sp_group) here, since you have dropped all the #ifdef-s in the header now that is the largest size which you need now.
> 
>> +
>> +	buf_ptr = write_buf;
>> +	if (IS_ISP2401) {
>> +		memcpy(buf_ptr, &sh_css_sp_group.config, sizeof(u8) * 5);
> 
> This is wrong, there is a big hole between:
> 
>         u8                      no_isp_sync; /* Signal host immediately after start */
>         u8                      enable_raw_pool_locking; /** Enable Raw Buffer Locking for HAL
>         u8                      lock_all;
> 
> and:
> 
>         u8                 enable_isys_event_queue;
>         u8                      disable_cont_vf;
> 
> filled with ISP2400 specific data now, so you are copying what likely is some empty alignment bytes before the ISP2400 specific input_formatter struct now instead of copying enable_isys_event_queue
> and disable_cont_vf.
> 
> So you need to split the memcpy into 2 memcpy calls. You can calculate the source offset of enable_isys_event_queue in sh_css_sp_group.config with offsetof(struct sh_css_sp_config, enable_isys_event_queue), or better yet, just take the address of it:
> 
> 	if (IS_ISP2401) {
> 		memcpy(buf_ptr, &sh_css_sp_group.config, 3);
> 		buf_ptr += 3;
> 		memcpy(buf_ptr, &sh_css_sp_group.config.enable_isys_event_queue, 2);
> 		buf_ptr += 2;
> 		memset(buf_ptr, 0, 3);
> 		buf_ptr += 3; /* Padding 3 bytes for struct sh_css_sp_config*/
> 	} else {
> 
> Also note I have dropped the "* sizeof(u8)" here, you already dropped that yourself for the memset / padding bits and dropping it makes the code easier to read IMHO.
> 		
> 
> 
>> +		buf_ptr += (sizeof(u8) * 5);
>> +		memset(buf_ptr, 0, 3);
>> +		buf_ptr += 3; /* Padding 3 bytes for struct sh_css_sp_config*/
>> +	} else {
>> +		memcpy(buf_ptr, &sh_css_sp_group.config, sizeof(sh_css_sp_group.config));
>> +		buf_ptr += sizeof(sh_css_sp_group.config);
>> +	}
>> +
>> +	memcpy(buf_ptr, &sh_css_sp_group.pipe, sizeof(sh_css_sp_group.pipe));
>> +	buf_ptr += sizeof(sh_css_sp_group.pipe);
>> +
>> +	if (IS_ISP2401) {
>> +		memcpy(buf_ptr, &sh_css_sp_group.pipe_io, sizeof(sh_css_sp_group.pipe_io));
>> +		buf_ptr += sizeof(sh_css_sp_group.pipe_io);
>> +		memcpy(buf_ptr, &sh_css_sp_group.pipe_io_status,
>> +		       sizeof(sh_css_sp_group.pipe_io_status));
>> +		buf_ptr += sizeof(sh_css_sp_group.pipe_io_status);
>> +	}
>> +
>> +	memcpy(buf_ptr, &sh_css_sp_group.debug, sizeof(sh_css_sp_group.debug));
>> +	buf_ptr += sizeof(sh_css_sp_group.debug);
>> +
>>  	hmm_store(xmem_sp_group_ptrs,
>> -		   &sh_css_sp_group,
>> -		   sizeof(struct sh_css_sp_group));
>> +		  write_buf,
>> +		  buf_ptr - write_buf);
>> +
>> +	kfree(write_buf);
>>  	return xmem_sp_group_ptrs;
>>  }
>>  
> 
> The rest looks good at a quick glance, but I need to take a closer look later.

I have taken a closer look at the rest of the patch now and except for my one previous remark this looks good to me.

Thank you for working on this.

Regards,

Hans

Re: [PATCH 2/3] atomisp: sh_css_params: write the sp_group config according to the ISP model

[Kate Hsuan]

Hi,

On Tue, Jun 6, 2023 at 7:02 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi Kate,
>
> On 6/5/23 12:29, Kate Hsuan wrote:
> > Pick up the necessary part of sp_group configuration for both model and
> > then copy those parts into a tempetory buffer. This buffer is finally
> > written to the ISP with correct length.
> >
> > Signed-off-by: Kate Hsuan <hpa@redhat.com>
> > ---
> >  .../staging/media/atomisp/pci/sh_css_params.c | 37 ++++++++++++++++++-
> >  1 file changed, 35 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/staging/media/atomisp/pci/sh_css_params.c b/drivers/staging/media/atomisp/pci/sh_css_params.c
> > index 588f2adab058..2913d9d6d226 100644
> > --- a/drivers/staging/media/atomisp/pci/sh_css_params.c
> > +++ b/drivers/staging/media/atomisp/pci/sh_css_params.c
> > @@ -3720,10 +3720,43 @@ struct ia_css_shading_table *ia_css_get_shading_table(struct ia_css_stream
> >
> >  ia_css_ptr sh_css_store_sp_group_to_ddr(void)
> >  {
> > +     u8 *write_buf;
> > +     u8 *buf_ptr;
> > +
> >       IA_CSS_ENTER_LEAVE_PRIVATE("void");
> > +
> > +     write_buf = kzalloc(sizeof(u8) * 8192, GFP_KERNEL);
>
> Please use sizeof(struct sh_css_sp_group) here, since you have dropped all the #ifdef-s in the header now that is the largest size which you need now.

Okay.

>
> > +
> > +     buf_ptr = write_buf;
> > +     if (IS_ISP2401) {
> > +             memcpy(buf_ptr, &sh_css_sp_group.config, sizeof(u8) * 5);
>
> This is wrong, there is a big hole between:
>
>         u8                      no_isp_sync; /* Signal host immediately after start */
>         u8                      enable_raw_pool_locking; /** Enable Raw Buffer Locking for HAL
>         u8                      lock_all;
>
> and:
>
>         u8                 enable_isys_event_queue;
>         u8                      disable_cont_vf;
>
> filled with ISP2400 specific data now, so you are copying what likely is some empty alignment bytes before the ISP2400 specific input_formatter struct now instead of copying enable_isys_event_queue
> and disable_cont_vf.
>
> So you need to split the memcpy into 2 memcpy calls. You can calculate the source offset of enable_isys_event_queue in sh_css_sp_group.config with offsetof(struct sh_css_sp_config, enable_isys_event_queue), or better yet, just take the address of it:
>
>         if (IS_ISP2401) {
>                 memcpy(buf_ptr, &sh_css_sp_group.config, 3);
>                 buf_ptr += 3;
>                 memcpy(buf_ptr, &sh_css_sp_group.config.enable_isys_event_queue, 2);
>                 buf_ptr += 2;
>                 memset(buf_ptr, 0, 3);
>                 buf_ptr += 3; /* Padding 3 bytes for struct sh_css_sp_config*/
>         } else {
>
> Also note I have dropped the "* sizeof(u8)" here, you already dropped that yourself for the memset / padding bits and dropping it makes the code easier to read IMHO.
>
>

Ops, My bad. I tried to simplify the memcpy() part and made the things
wrong. I should skip the structure between them. I'll fix it in the v2
patch.

Thank you for pointing out this error.

>
> > +             buf_ptr += (sizeof(u8) * 5);
> > +             memset(buf_ptr, 0, 3);
> > +             buf_ptr += 3; /* Padding 3 bytes for struct sh_css_sp_config*/
> > +     } else {
> > +             memcpy(buf_ptr, &sh_css_sp_group.config, sizeof(sh_css_sp_group.config));
> > +             buf_ptr += sizeof(sh_css_sp_group.config);
> > +     }
> > +
> > +     memcpy(buf_ptr, &sh_css_sp_group.pipe, sizeof(sh_css_sp_group.pipe));
> > +     buf_ptr += sizeof(sh_css_sp_group.pipe);
> > +
> > +     if (IS_ISP2401) {
> > +             memcpy(buf_ptr, &sh_css_sp_group.pipe_io, sizeof(sh_css_sp_group.pipe_io));
> > +             buf_ptr += sizeof(sh_css_sp_group.pipe_io);
> > +             memcpy(buf_ptr, &sh_css_sp_group.pipe_io_status,
> > +                    sizeof(sh_css_sp_group.pipe_io_status));
> > +             buf_ptr += sizeof(sh_css_sp_group.pipe_io_status);
> > +     }
> > +
> > +     memcpy(buf_ptr, &sh_css_sp_group.debug, sizeof(sh_css_sp_group.debug));
> > +     buf_ptr += sizeof(sh_css_sp_group.debug);
> > +
> >       hmm_store(xmem_sp_group_ptrs,
> > -                &sh_css_sp_group,
> > -                sizeof(struct sh_css_sp_group));
> > +               write_buf,
> > +               buf_ptr - write_buf);
> > +
> > +     kfree(write_buf);
> >       return xmem_sp_group_ptrs;
> >  }
> >
>
> The rest looks good at a quick glance, but I need to take a closer look later.
>
> Regards,
>
> Hans
>


-- 
BR,
Kate

[PATCH 3/3] media: atomisp: ia_css_debug: remove unused codes

[Kate Hsuan]

This portion of the codes was not referenced so it can be completely
removed.

Signed-off-by: Kate Hsuan <hpa@redhat.com>
---
 .../runtime/debug/interface/ia_css_debug.h    |  6 --
 .../pci/runtime/debug/src/ia_css_debug.c      | 76 +------------------
 2 files changed, 2 insertions(+), 80 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/runtime/debug/interface/ia_css_debug.h b/drivers/staging/media/atomisp/pci/runtime/debug/interface/ia_css_debug.h
index fff89e9b4b01..3a3d72c6eaaa 100644
--- a/drivers/staging/media/atomisp/pci/runtime/debug/interface/ia_css_debug.h
+++ b/drivers/staging/media/atomisp/pci/runtime/debug/interface/ia_css_debug.h
@@ -141,12 +141,6 @@ static inline void __printf(2, 0) ia_css_debug_vdtrace(unsigned int level,
 __printf(2, 3) void ia_css_debug_dtrace(unsigned int level,
 					const char *fmt, ...);
 
-/*! @brief Dump sp thread's stack contents
- * SP thread's stack contents are set to 0xcafecafe. This function dumps the
- * stack to inspect if the stack's boundaries are compromised.
- * @return	None
- */
-void ia_css_debug_dump_sp_stack_info(void);
 
 /*! @brief Function to set the global dtrace verbosity level.
  * @param[in]	trace_level	Maximum level of the messages to be traced.
diff --git a/drivers/staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c b/drivers/staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c
index bb6204cb42c5..bb30146c5fe7 100644
--- a/drivers/staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c
+++ b/drivers/staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c
@@ -105,7 +105,8 @@
  * TODO:SH_CSS_MAX_SP_THREADS is not the max number of sp threads
  * future rework should fix this and remove the define MAX_THREAD_NUM
  */
-#define MAX_THREAD_NUM (SH_CSS_MAX_SP_THREADS + SH_CSS_MAX_SP_INTERNAL_THREADS)
+#define MAX_THREAD_NUM_2400 (SH_CSS_MAX_SP_THREADS + SH_CSS_MAX_SP_INTERNAL_THREADS_2400)
+#define MAX_THREAD_NUM_2401 (SH_CSS_MAX_SP_THREADS + SH_CSS_MAX_SP_INTERNAL_THREADS_2401)
 
 static struct pipe_graph_class {
 	bool do_init;
@@ -147,79 +148,6 @@ void ia_css_debug_dtrace(unsigned int level, const char *fmt, ...)
 	va_end(ap);
 }
 
-static void debug_dump_long_array_formatted(
-    const sp_ID_t sp_id,
-    hrt_address stack_sp_addr,
-    unsigned int stack_size)
-{
-	unsigned int i;
-	u32 val;
-	u32 addr = (uint32_t)stack_sp_addr;
-	u32 stack_size_words = CEIL_DIV(stack_size, sizeof(uint32_t));
-
-	/* When size is not multiple of four, last word is only relevant for
-	 * remaining bytes */
-	for (i = 0; i < stack_size_words; i++) {
-		val = sp_dmem_load_uint32(sp_id, (hrt_address)addr);
-		if ((i % 8) == 0)
-			ia_css_debug_dtrace(IA_CSS_DEBUG_VERBOSE, "\n");
-
-		ia_css_debug_dtrace(IA_CSS_DEBUG_VERBOSE, "0x%08x ", val);
-		addr += sizeof(uint32_t);
-	}
-
-	ia_css_debug_dtrace(IA_CSS_DEBUG_VERBOSE, "\n");
-}
-
-static void debug_dump_sp_stack_info(
-    const sp_ID_t sp_id)
-{
-	const struct ia_css_fw_info *fw;
-	unsigned int HIVE_ADDR_sp_threads_stack;
-	unsigned int HIVE_ADDR_sp_threads_stack_size;
-	u32 stack_sizes[MAX_THREAD_NUM];
-	u32 stack_sp_addr[MAX_THREAD_NUM];
-	unsigned int i;
-
-	fw = &sh_css_sp_fw;
-
-	ia_css_debug_dtrace(IA_CSS_DEBUG_VERBOSE, "sp_id(%u) stack info\n", sp_id);
-	ia_css_debug_dtrace(IA_CSS_DEBUG_VERBOSE,
-			    "from objects stack_addr_offset:0x%x stack_size_offset:0x%x\n",
-			    fw->info.sp.threads_stack,
-			    fw->info.sp.threads_stack_size);
-
-	HIVE_ADDR_sp_threads_stack = fw->info.sp.threads_stack;
-	HIVE_ADDR_sp_threads_stack_size = fw->info.sp.threads_stack_size;
-
-	if (fw->info.sp.threads_stack == 0 ||
-	    fw->info.sp.threads_stack_size == 0)
-		return;
-
-	(void)HIVE_ADDR_sp_threads_stack;
-	(void)HIVE_ADDR_sp_threads_stack_size;
-
-	sp_dmem_load(sp_id,
-		     (unsigned int)sp_address_of(sp_threads_stack),
-		     &stack_sp_addr, sizeof(stack_sp_addr));
-	sp_dmem_load(sp_id,
-		     (unsigned int)sp_address_of(sp_threads_stack_size),
-		     &stack_sizes, sizeof(stack_sizes));
-
-	for (i = 0 ; i < MAX_THREAD_NUM; i++) {
-		ia_css_debug_dtrace(IA_CSS_DEBUG_VERBOSE,
-				    "thread: %u stack_addr: 0x%08x stack_size: %u\n",
-				    i, stack_sp_addr[i], stack_sizes[i]);
-		debug_dump_long_array_formatted(sp_id, (hrt_address)stack_sp_addr[i],
-						stack_sizes[i]);
-	}
-}
-
-void ia_css_debug_dump_sp_stack_info(void)
-{
-	debug_dump_sp_stack_info(SP0_ID);
-}
-
 void ia_css_debug_set_dtrace_level(const unsigned int trace_level)
 {
 	dbg_level = trace_level;
-- 
2.40.1

Re: [PATCH 3/3] media: atomisp: ia_css_debug: remove unused codes

[Hans de Goede]

Hi Kate,

On 6/5/23 12:29, Kate Hsuan wrote:
> This portion of the codes was not referenced so it can be completely
> removed.
> 
> Signed-off-by: Kate Hsuan <hpa@redhat.com>

So this patch fixes compile errors introduced by the first patch
and MAX_THREAD_NUM is still used after this by code in
"#ifdef HAS_WATCHDOG_SP_THREAD_DEBUG" blocks.

Note HAS_WATCHDOG_SP_THREAD_DEBUG is never set.

So IMHO the right thing to do here is:

1. Introduce a new patch which should be the first patch
   of the set which removes all code blocks protected by
   "#ifdef HAS_WATCHDOG_SP_THREAD_DEBUG" as well as
   the "uncomment this here" HAS_WATCHDOG_SP_THREAD_DEBUG bits
   in drivers/staging/media/atomisp/pci/hive_isp_css_common/debug_global.h

2.Make this patch the second (first patch after the
  HAS_WATCHDOG_SP_THREAD_DEBUG removal) patch of the series.

  And instead of renaming MAX_THREAD_NUM to
  MAX_THREAD_NUM_2400 / MAX_THREAD_NUM_2401 just entirely drop it
  and also drop the following now no longer used defines:
  SH_CSS_SP_INTERNAL_METADATA_THREAD
  SH_CSS_SP_INTERNAL_SERVICE_THREAD
  SH_CSS_MAX_SP_INTERNAL_THREADS

Regards,

Hans


SH_CSS_SP_INTERNAL_METADATA_THREAD and
  
the entire now no longer 

> ---
>  .../runtime/debug/interface/ia_css_debug.h    |  6 --
>  .../pci/runtime/debug/src/ia_css_debug.c      | 76 +------------------
>  2 files changed, 2 insertions(+), 80 deletions(-)
> 
> diff --git a/drivers/staging/media/atomisp/pci/runtime/debug/interface/ia_css_debug.h b/drivers/staging/media/atomisp/pci/runtime/debug/interface/ia_css_debug.h
> index fff89e9b4b01..3a3d72c6eaaa 100644
> --- a/drivers/staging/media/atomisp/pci/runtime/debug/interface/ia_css_debug.h
> +++ b/drivers/staging/media/atomisp/pci/runtime/debug/interface/ia_css_debug.h
> @@ -141,12 +141,6 @@ static inline void __printf(2, 0) ia_css_debug_vdtrace(unsigned int level,
>  __printf(2, 3) void ia_css_debug_dtrace(unsigned int level,
>  					const char *fmt, ...);
>  
> -/*! @brief Dump sp thread's stack contents
> - * SP thread's stack contents are set to 0xcafecafe. This function dumps the
> - * stack to inspect if the stack's boundaries are compromised.
> - * @return	None
> - */
> -void ia_css_debug_dump_sp_stack_info(void);
>  
>  /*! @brief Function to set the global dtrace verbosity level.
>   * @param[in]	trace_level	Maximum level of the messages to be traced.
> diff --git a/drivers/staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c b/drivers/staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c
> index bb6204cb42c5..bb30146c5fe7 100644
> --- a/drivers/staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c
> +++ b/drivers/staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c
> @@ -105,7 +105,8 @@
>   * TODO:SH_CSS_MAX_SP_THREADS is not the max number of sp threads
>   * future rework should fix this and remove the define MAX_THREAD_NUM
>   */
> -#define MAX_THREAD_NUM (SH_CSS_MAX_SP_THREADS + SH_CSS_MAX_SP_INTERNAL_THREADS)
> +#define MAX_THREAD_NUM_2400 (SH_CSS_MAX_SP_THREADS + SH_CSS_MAX_SP_INTERNAL_THREADS_2400)
> +#define MAX_THREAD_NUM_2401 (SH_CSS_MAX_SP_THREADS + SH_CSS_MAX_SP_INTERNAL_THREADS_2401)
>  
>  static struct pipe_graph_class {
>  	bool do_init;
> @@ -147,79 +148,6 @@ void ia_css_debug_dtrace(unsigned int level, const char *fmt, ...)
>  	va_end(ap);
>  }
>  
> -static void debug_dump_long_array_formatted(
> -    const sp_ID_t sp_id,
> -    hrt_address stack_sp_addr,
> -    unsigned int stack_size)
> -{
> -	unsigned int i;
> -	u32 val;
> -	u32 addr = (uint32_t)stack_sp_addr;
> -	u32 stack_size_words = CEIL_DIV(stack_size, sizeof(uint32_t));
> -
> -	/* When size is not multiple of four, last word is only relevant for
> -	 * remaining bytes */
> -	for (i = 0; i < stack_size_words; i++) {
> -		val = sp_dmem_load_uint32(sp_id, (hrt_address)addr);
> -		if ((i % 8) == 0)
> -			ia_css_debug_dtrace(IA_CSS_DEBUG_VERBOSE, "\n");
> -
> -		ia_css_debug_dtrace(IA_CSS_DEBUG_VERBOSE, "0x%08x ", val);
> -		addr += sizeof(uint32_t);
> -	}
> -
> -	ia_css_debug_dtrace(IA_CSS_DEBUG_VERBOSE, "\n");
> -}
> -
> -static void debug_dump_sp_stack_info(
> -    const sp_ID_t sp_id)
> -{
> -	const struct ia_css_fw_info *fw;
> -	unsigned int HIVE_ADDR_sp_threads_stack;
> -	unsigned int HIVE_ADDR_sp_threads_stack_size;
> -	u32 stack_sizes[MAX_THREAD_NUM];
> -	u32 stack_sp_addr[MAX_THREAD_NUM];
> -	unsigned int i;
> -
> -	fw = &sh_css_sp_fw;
> -
> -	ia_css_debug_dtrace(IA_CSS_DEBUG_VERBOSE, "sp_id(%u) stack info\n", sp_id);
> -	ia_css_debug_dtrace(IA_CSS_DEBUG_VERBOSE,
> -			    "from objects stack_addr_offset:0x%x stack_size_offset:0x%x\n",
> -			    fw->info.sp.threads_stack,
> -			    fw->info.sp.threads_stack_size);
> -
> -	HIVE_ADDR_sp_threads_stack = fw->info.sp.threads_stack;
> -	HIVE_ADDR_sp_threads_stack_size = fw->info.sp.threads_stack_size;
> -
> -	if (fw->info.sp.threads_stack == 0 ||
> -	    fw->info.sp.threads_stack_size == 0)
> -		return;
> -
> -	(void)HIVE_ADDR_sp_threads_stack;
> -	(void)HIVE_ADDR_sp_threads_stack_size;
> -
> -	sp_dmem_load(sp_id,
> -		     (unsigned int)sp_address_of(sp_threads_stack),
> -		     &stack_sp_addr, sizeof(stack_sp_addr));
> -	sp_dmem_load(sp_id,
> -		     (unsigned int)sp_address_of(sp_threads_stack_size),
> -		     &stack_sizes, sizeof(stack_sizes));
> -
> -	for (i = 0 ; i < MAX_THREAD_NUM; i++) {
> -		ia_css_debug_dtrace(IA_CSS_DEBUG_VERBOSE,
> -				    "thread: %u stack_addr: 0x%08x stack_size: %u\n",
> -				    i, stack_sp_addr[i], stack_sizes[i]);
> -		debug_dump_long_array_formatted(sp_id, (hrt_address)stack_sp_addr[i],
> -						stack_sizes[i]);
> -	}
> -}
> -
> -void ia_css_debug_dump_sp_stack_info(void)
> -{
> -	debug_dump_sp_stack_info(SP0_ID);
> -}
> -
>  void ia_css_debug_set_dtrace_level(const unsigned int trace_level)
>  {
>  	dbg_level = trace_level;