[PATCH 00/13] media: atomisp: reduce the gap for ISP2401

[PATCH 00/13] media: atomisp: reduce the gap for ISP2401

[Mauro Carvalho Chehab]

As the driver and the firmware are closely bound each other and we're
using the Intel Aero firmware, with is widely available, make the sh_css
code closer to the OOT device driver for such board.

As a plus, it reduces the number of #ifdefs inside the code.

Patch 1: drop several boilerplate files;
Patch 2 and above: reduce sh_css gap.

Mauro Carvalho Chehab (13):
  media: atomisp: drop empty files
  media: atomisp: simplif sh_css_defs.h
  media: atomisp: sh_css_metrics: drop some unused code
  media: atomisp: sh_css_mipi: cleanup the code
  media: atomisp: sh_css_params: remove tests for ISP2401
  media: atomisp: sh_css_params: cleanup the code
  media: atomisp: remove #ifdef HAS_NO_HMEM
  media: atomisp: get rid of USE_WINDOWS_BINNING_FACTOR tests
  media: atomisp: get rid of #ifdef HAS_BL
  media: atomisp: get rid of sctbl_legacy_*
  media: atomisp: sh_css_param_shading: fix comments coding style
  media: atomisp: fix a bug when applying the binning factor
  media: atomisp: sh_css_sp: better support the current firmware

 drivers/staging/media/atomisp/Makefile        |   4 -
 .../staging/media/atomisp/pci/atomisp_cmd.c   |  28 +--
 .../staging/media/atomisp/pci/ia_css_pipe.h   |   4 +-
 .../media/atomisp/pci/ia_css_stream_public.h  |  10 +-
 .../pci/isp/kernels/bh/bh_2/ia_css_bh.host.c  |   2 -
 .../raw_aa_binning_1.0/ia_css_raa.host.c      |   2 -
 .../isp/kernels/s3a/s3a_1.0/ia_css_s3a.host.c |   5 -
 .../isp/kernels/tnr/tnr_1.0/ia_css_tnr.host.c |   6 +-
 .../kernels/tnr/tnr_1.0/ia_css_tnr_param.h    |   4 +-
 .../pci/isp/modes/interface/isp_const.h       |   4 +-
 .../runtime/binary/interface/ia_css_binary.h  |   2 -
 .../atomisp/pci/runtime/binary/src/binary.c   |  18 +-
 .../pci/runtime/debug/src/ia_css_debug.c      |   2 +-
 drivers/staging/media/atomisp/pci/sh_css.c    |  16 +-
 .../staging/media/atomisp/pci/sh_css_defs.h   |  48 +---
 .../media/atomisp/pci/sh_css_firmware.h       |   3 -
 .../media/atomisp/pci/sh_css_internal.h       |   2 +-
 .../media/atomisp/pci/sh_css_metadata.c       |  17 --
 .../media/atomisp/pci/sh_css_metrics.c        |  25 --
 .../staging/media/atomisp/pci/sh_css_mipi.c   | 159 +++----------
 .../staging/media/atomisp/pci/sh_css_mipi.h   |  15 --
 .../staging/media/atomisp/pci/sh_css_morph.c  |  17 --
 .../media/atomisp/pci/sh_css_param_shading.c  |  84 +++----
 .../staging/media/atomisp/pci/sh_css_params.c | 219 ++----------------
 .../staging/media/atomisp/pci/sh_css_params.h |   8 -
 .../media/atomisp/pci/sh_css_shading.c        |  17 --
 drivers/staging/media/atomisp/pci/sh_css_sp.c |  17 +-
 .../staging/media/atomisp/pci/sh_css_stream.c |  17 --
 28 files changed, 151 insertions(+), 604 deletions(-)
 delete mode 100644 drivers/staging/media/atomisp/pci/sh_css_metadata.c
 delete mode 100644 drivers/staging/media/atomisp/pci/sh_css_morph.c
 delete mode 100644 drivers/staging/media/atomisp/pci/sh_css_shading.c
 delete mode 100644 drivers/staging/media/atomisp/pci/sh_css_stream.c

-- 
2.33.1

[PATCH 01/13] media: atomisp: drop empty files

[Mauro Carvalho Chehab]

There's nothing inside such files. Just drop them.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---

To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH 00/13] at: https://lore.kernel.org/all/cover.1637140900.git.mchehab+huawei@kernel.org/

 drivers/staging/media/atomisp/Makefile          |  4 ----
 .../staging/media/atomisp/pci/sh_css_metadata.c | 17 -----------------
 .../staging/media/atomisp/pci/sh_css_morph.c    | 17 -----------------
 .../staging/media/atomisp/pci/sh_css_shading.c  | 17 -----------------
 .../staging/media/atomisp/pci/sh_css_stream.c   | 17 -----------------
 5 files changed, 72 deletions(-)
 delete mode 100644 drivers/staging/media/atomisp/pci/sh_css_metadata.c
 delete mode 100644 drivers/staging/media/atomisp/pci/sh_css_morph.c
 delete mode 100644 drivers/staging/media/atomisp/pci/sh_css_shading.c
 delete mode 100644 drivers/staging/media/atomisp/pci/sh_css_stream.c

diff --git a/drivers/staging/media/atomisp/Makefile b/drivers/staging/media/atomisp/Makefile
index a7ad8cfab602..2485d7b3fee2 100644
--- a/drivers/staging/media/atomisp/Makefile
+++ b/drivers/staging/media/atomisp/Makefile
@@ -27,20 +27,16 @@ atomisp-objs += \
 	pci/sh_css_firmware.o \
 	pci/sh_css_host_data.o \
 	pci/sh_css_hrt.o \
-	pci/sh_css_metadata.o \
 	pci/sh_css_metrics.o \
 	pci/sh_css_mipi.o \
 	pci/sh_css_mmu.o \
-	pci/sh_css_morph.o \
 	pci/sh_css.o \
 	pci/sh_css_param_dvs.o \
 	pci/sh_css_param_shading.o \
 	pci/sh_css_params.o \
 	pci/sh_css_properties.o \
-	pci/sh_css_shading.o \
 	pci/sh_css_sp.o \
 	pci/sh_css_stream_format.o \
-	pci/sh_css_stream.o \
 	pci/sh_css_version.o \
 	pci/base/circbuf/src/circbuf.o \
 	pci/base/refcount/src/refcount.o \
diff --git a/drivers/staging/media/atomisp/pci/sh_css_metadata.c b/drivers/staging/media/atomisp/pci/sh_css_metadata.c
deleted file mode 100644
index 04a4b7da85e2..000000000000
--- a/drivers/staging/media/atomisp/pci/sh_css_metadata.c
+++ /dev/null
@@ -1,17 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * Support for Intel Camera Imaging ISP subsystem.
- * Copyright (c) 2015, Intel Corporation.
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms and conditions of the GNU General Public License,
- * version 2, as published by the Free Software Foundation.
- *
- * This program is distributed in the hope it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
- * more details.
- */
-
-/* This file will contain the code to implement the functions declared in ia_css_metadata.h
-   and associated helper functions */
diff --git a/drivers/staging/media/atomisp/pci/sh_css_morph.c b/drivers/staging/media/atomisp/pci/sh_css_morph.c
deleted file mode 100644
index edd1da941ccb..000000000000
--- a/drivers/staging/media/atomisp/pci/sh_css_morph.c
+++ /dev/null
@@ -1,17 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * Support for Intel Camera Imaging ISP subsystem.
- * Copyright (c) 2015, Intel Corporation.
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms and conditions of the GNU General Public License,
- * version 2, as published by the Free Software Foundation.
- *
- * This program is distributed in the hope it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
- * more details.
- */
-
-/* This file will contain the code to implement the functions declared in ia_css_morph.h
-   and associated helper functions */
diff --git a/drivers/staging/media/atomisp/pci/sh_css_shading.c b/drivers/staging/media/atomisp/pci/sh_css_shading.c
deleted file mode 100644
index 462caf9cb571..000000000000
--- a/drivers/staging/media/atomisp/pci/sh_css_shading.c
+++ /dev/null
@@ -1,17 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * Support for Intel Camera Imaging ISP subsystem.
- * Copyright (c) 2015, Intel Corporation.
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms and conditions of the GNU General Public License,
- * version 2, as published by the Free Software Foundation.
- *
- * This program is distributed in the hope it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
- * more details.
- */
-
-/* This file will contain the code to implement the functions declared in ia_css_shading.h
-   and associated helper functions */
diff --git a/drivers/staging/media/atomisp/pci/sh_css_stream.c b/drivers/staging/media/atomisp/pci/sh_css_stream.c
deleted file mode 100644
index a768ce90f51c..000000000000
--- a/drivers/staging/media/atomisp/pci/sh_css_stream.c
+++ /dev/null
@@ -1,17 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * Support for Intel Camera Imaging ISP subsystem.
- * Copyright (c) 2015, Intel Corporation.
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms and conditions of the GNU General Public License,
- * version 2, as published by the Free Software Foundation.
- *
- * This program is distributed in the hope it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
- * more details.
- */
-
-/* This file will contain the code to implement the functions declared in ia_css_stream.h
-   and associated helper functions */
-- 
2.33.1

[PATCH 02/13] media: atomisp: simplif sh_css_defs.h

[Mauro Carvalho Chehab]

There are several unused macros. Simplify the logic there, making
it closer to the Intel Aero driver and the corresponding firmware,
as this is what we have widely available for this device.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---

To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH 00/13] at: https://lore.kernel.org/all/cover.1637140900.git.mchehab+huawei@kernel.org/

 .../staging/media/atomisp/pci/atomisp_cmd.c   | 28 +++--------
 .../staging/media/atomisp/pci/ia_css_pipe.h   |  4 +-
 .../isp/kernels/tnr/tnr_1.0/ia_css_tnr.host.c |  6 +--
 .../kernels/tnr/tnr_1.0/ia_css_tnr_param.h    |  4 +-
 .../pci/isp/modes/interface/isp_const.h       |  4 +-
 .../atomisp/pci/runtime/binary/src/binary.c   |  8 ++--
 .../pci/runtime/debug/src/ia_css_debug.c      |  2 +-
 drivers/staging/media/atomisp/pci/sh_css.c    | 10 ++--
 .../staging/media/atomisp/pci/sh_css_defs.h   | 48 ++++---------------
 .../media/atomisp/pci/sh_css_internal.h       |  2 +-
 10 files changed, 36 insertions(+), 80 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.c b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
index 4783590969b6..7181b901cde4 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
@@ -3617,18 +3617,10 @@ int atomisp_cp_lsc_table(struct atomisp_sub_device *asd,
 	}
 
 	/* Shading table size per color */
-	if (!IS_ISP2401) {
-		if (st->width > ISP2400_SH_CSS_MAX_SCTBL_WIDTH_PER_COLOR ||
-		    st->height > ISP2400_SH_CSS_MAX_SCTBL_HEIGHT_PER_COLOR) {
-			dev_err(asd->isp->dev, "shading table w/h validate failed!");
-			return -EINVAL;
-		}
-	} else {
-		if (st->width > ISP2401_SH_CSS_MAX_SCTBL_WIDTH_PER_COLOR ||
-		    st->height > ISP2401_SH_CSS_MAX_SCTBL_HEIGHT_PER_COLOR) {
-			dev_err(asd->isp->dev, "shading table w/h validate failed!");
-			return -EINVAL;
-		}
+	if (st->width > SH_CSS_MAX_SCTBL_WIDTH_PER_COLOR ||
+	    st->height > SH_CSS_MAX_SCTBL_HEIGHT_PER_COLOR) {
+		dev_err(asd->isp->dev, "shading table w/h validate failed!");
+		return -EINVAL;
 	}
 
 	shading_table = atomisp_css_shading_table_alloc(st->width, st->height);
@@ -6181,15 +6173,9 @@ int atomisp_set_shading_table(struct atomisp_sub_device *asd,
 	}
 
 	/* Shading table size per color */
-	if (!IS_ISP2401) {
-		if (user_shading_table->width > ISP2400_SH_CSS_MAX_SCTBL_WIDTH_PER_COLOR ||
-		    user_shading_table->height > ISP2400_SH_CSS_MAX_SCTBL_HEIGHT_PER_COLOR)
-			return -EINVAL;
-	} else {
-		if (user_shading_table->width > ISP2401_SH_CSS_MAX_SCTBL_WIDTH_PER_COLOR ||
-		    user_shading_table->height > ISP2401_SH_CSS_MAX_SCTBL_HEIGHT_PER_COLOR)
-			return -EINVAL;
-	}
+	if (user_shading_table->width > SH_CSS_MAX_SCTBL_WIDTH_PER_COLOR ||
+	    user_shading_table->height > SH_CSS_MAX_SCTBL_HEIGHT_PER_COLOR)
+		return -EINVAL;
 
 	shading_table = atomisp_css_shading_table_alloc(
 			    user_shading_table->width, user_shading_table->height);
diff --git a/drivers/staging/media/atomisp/pci/ia_css_pipe.h b/drivers/staging/media/atomisp/pci/ia_css_pipe.h
index bb0abf9bffb1..fb58535bff40 100644
--- a/drivers/staging/media/atomisp/pci/ia_css_pipe.h
+++ b/drivers/staging/media/atomisp/pci/ia_css_pipe.h
@@ -33,7 +33,7 @@ struct ia_css_preview_settings {
 
 	/* 2401 only for these two - do we in fact use them for anything real */
 	struct ia_css_frame *delay_frames[MAX_NUM_VIDEO_DELAY_FRAMES];
-	struct ia_css_frame *tnr_frames[NUM_TNR_FRAMES];
+	struct ia_css_frame *tnr_frames[NUM_VIDEO_TNR_FRAMES];
 
 	struct ia_css_pipe *copy_pipe;
 	struct ia_css_pipe *capture_pipe;
@@ -81,7 +81,7 @@ struct ia_css_video_settings {
 	struct ia_css_binary vf_pp_binary;
 	struct ia_css_binary *yuv_scaler_binary;
 	struct ia_css_frame *delay_frames[MAX_NUM_VIDEO_DELAY_FRAMES];
-	struct ia_css_frame *tnr_frames[NUM_TNR_FRAMES];
+	struct ia_css_frame *tnr_frames[NUM_VIDEO_TNR_FRAMES];
 	struct ia_css_frame *vf_pp_in_frame;
 	struct ia_css_pipe *copy_pipe;
 	struct ia_css_pipe *capture_pipe;
diff --git a/drivers/staging/media/atomisp/pci/isp/kernels/tnr/tnr_1.0/ia_css_tnr.host.c b/drivers/staging/media/atomisp/pci/isp/kernels/tnr/tnr_1.0/ia_css_tnr.host.c
index 7177cf292fb0..53050c0c49fc 100644
--- a/drivers/staging/media/atomisp/pci/isp/kernels/tnr/tnr_1.0/ia_css_tnr.host.c
+++ b/drivers/staging/media/atomisp/pci/isp/kernels/tnr/tnr_1.0/ia_css_tnr.host.c
@@ -84,7 +84,7 @@ int ia_css_tnr_config(struct sh_css_isp_tnr_isp_config *to,
 		return ret;
 	to->width_a_over_b = elems_a / to->port_b.elems;
 	to->frame_height = from->tnr_frames[0]->info.res.height;
-	for (i = 0; i < NUM_TNR_FRAMES; i++) {
+	for (i = 0; i < NUM_VIDEO_TNR_FRAMES; i++) {
 		to->tnr_frame_addr[i] = from->tnr_frames[i]->data +
 					from->tnr_frames[i]->planes.yuyv.offset;
 	}
@@ -102,7 +102,7 @@ int ia_css_tnr_configure(const struct ia_css_binary     *binary,
 	struct ia_css_tnr_configuration config;
 	unsigned int i;
 
-	for (i = 0; i < NUM_TNR_FRAMES; i++)
+	for (i = 0; i < NUM_VIDEO_TNR_FRAMES; i++)
 		config.tnr_frames[i] = frames[i];
 
 	return ia_css_configure_tnr(binary, &config);
@@ -115,7 +115,7 @@ ia_css_init_tnr_state(
 {
 	(void)size;
 
-	assert(NUM_TNR_FRAMES >= 2);
+	assert(NUM_VIDEO_TNR_FRAMES >= 2);
 	assert(sizeof(*state) == size);
 	state->tnr_in_buf_idx = 0;
 	state->tnr_out_buf_idx = 1;
diff --git a/drivers/staging/media/atomisp/pci/isp/kernels/tnr/tnr_1.0/ia_css_tnr_param.h b/drivers/staging/media/atomisp/pci/isp/kernels/tnr/tnr_1.0/ia_css_tnr_param.h
index 60a2542cf685..551dd5cfa9f1 100644
--- a/drivers/staging/media/atomisp/pci/isp/kernels/tnr/tnr_1.0/ia_css_tnr_param.h
+++ b/drivers/staging/media/atomisp/pci/isp/kernels/tnr/tnr_1.0/ia_css_tnr_param.h
@@ -28,14 +28,14 @@ struct sh_css_isp_tnr_params {
 };
 
 struct ia_css_tnr_configuration {
-	const struct ia_css_frame *tnr_frames[NUM_TNR_FRAMES];
+	const struct ia_css_frame *tnr_frames[NUM_VIDEO_TNR_FRAMES];
 };
 
 struct sh_css_isp_tnr_isp_config {
 	u32 width_a_over_b;
 	u32 frame_height;
 	struct dma_port_config port_b;
-	ia_css_ptr tnr_frame_addr[NUM_TNR_FRAMES];
+	ia_css_ptr tnr_frame_addr[NUM_VIDEO_TNR_FRAMES];
 };
 
 #endif /* __IA_CSS_TNR_PARAM_H */
diff --git a/drivers/staging/media/atomisp/pci/isp/modes/interface/isp_const.h b/drivers/staging/media/atomisp/pci/isp/modes/interface/isp_const.h
index e5c595e5020b..bfe4f5976771 100644
--- a/drivers/staging/media/atomisp/pci/isp/modes/interface/isp_const.h
+++ b/drivers/staging/media/atomisp/pci/isp/modes/interface/isp_const.h
@@ -154,9 +154,9 @@ more details.
 /* [isp vmem] table size[vectors] per line per color (GR,R,B,GB),
    multiples of NWAY */
 #define ISP2400_SCTBL_VECTORS_PER_LINE_PER_COLOR \
-	CEIL_DIV(ISP2400_SH_CSS_MAX_SCTBL_WIDTH_PER_COLOR, ISP_VEC_NELEMS)
+	CEIL_DIV(SH_CSS_MAX_SCTBL_WIDTH_PER_COLOR, ISP_VEC_NELEMS)
 #define ISP2401_SCTBL_VECTORS_PER_LINE_PER_COLOR \
-	CEIL_DIV(ISP2401_SH_CSS_MAX_SCTBL_WIDTH_PER_COLOR, ISP_VEC_NELEMS)
+	CEIL_DIV(SH_CSS_MAX_SCTBL_WIDTH_PER_COLOR, ISP_VEC_NELEMS)
 /* [isp vmem] table size[vectors] per line for 4colors (GR,R,B,GB),
    multiples of NWAY */
 #define SCTBL_VECTORS_PER_LINE \
diff --git a/drivers/staging/media/atomisp/pci/runtime/binary/src/binary.c b/drivers/staging/media/atomisp/pci/runtime/binary/src/binary.c
index 8c0e02e4e1af..851e1d34a521 100644
--- a/drivers/staging/media/atomisp/pci/runtime/binary/src/binary.c
+++ b/drivers/staging/media/atomisp/pci/runtime/binary/src/binary.c
@@ -1336,12 +1336,12 @@ ia_css_binary_fill_info(const struct ia_css_binary_xinfo *xinfo,
 	if (info->enable.sc)
 	{
 		if (!IS_ISP2401) {
-			binary->sctbl_width_per_color = _ISP2400_SCTBL_WIDTH_PER_COLOR(sc_3a_dis_padded_width, s3a_log_deci);
-			binary->sctbl_aligned_width_per_color = ISP2400_SH_CSS_MAX_SCTBL_ALIGNED_WIDTH_PER_COLOR;
-			binary->sctbl_height = _ISP2400_SCTBL_HEIGHT(sc_3a_dis_height, s3a_log_deci);
+			binary->sctbl_width_per_color = _ISP_SCTBL_WIDTH_PER_COLOR(sc_3a_dis_padded_width, s3a_log_deci);
+			binary->sctbl_aligned_width_per_color = SH_CSS_MAX_SCTBL_ALIGNED_WIDTH_PER_COLOR;
+			binary->sctbl_height = _ISP_SCTBL_HEIGHT(sc_3a_dis_height, s3a_log_deci);
 		} else {
 			binary->sctbl_width_per_color = _ISP2401_SCTBL_WIDTH_PER_COLOR(isp_internal_width, s3a_log_deci);
-			binary->sctbl_aligned_width_per_color = ISP2401_SH_CSS_MAX_SCTBL_ALIGNED_WIDTH_PER_COLOR;
+			binary->sctbl_aligned_width_per_color = SH_CSS_MAX_SCTBL_ALIGNED_WIDTH_PER_COLOR;
 			binary->sctbl_height = _ISP2401_SCTBL_HEIGHT(isp_internal_height, s3a_log_deci);
 			binary->sctbl_legacy_width_per_color  = _ISP_SCTBL_LEGACY_WIDTH_PER_COLOR(sc_3a_dis_padded_width, s3a_log_deci);
 			binary->sctbl_legacy_height = _ISP_SCTBL_LEGACY_HEIGHT(sc_3a_dis_height, s3a_log_deci);
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 1227a3e4d197..7bdbc1107404 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
@@ -2534,7 +2534,7 @@ ia_css_debug_pipe_graph_dump_stage(
 		    "in", true);
 	}
 
-	for (i = 0; i < NUM_TNR_FRAMES; i++) {
+	for (i = 0; i < NUM_VIDEO_TNR_FRAMES; i++) {
 		if (stage->args.tnr_frames[i]) {
 			ia_css_debug_pipe_graph_dump_frame(
 			    stage->args.tnr_frames[i], id,
diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c
index e5e22f966286..acaa4c8b1884 100644
--- a/drivers/staging/media/atomisp/pci/sh_css.c
+++ b/drivers/staging/media/atomisp/pci/sh_css.c
@@ -1261,7 +1261,7 @@ void sh_css_binary_args_reset(struct sh_css_binary_args *args)
 {
 	unsigned int i;
 
-	for (i = 0; i < NUM_TNR_FRAMES; i++)
+	for (i = 0; i < NUM_VIDEO_TNR_FRAMES; i++)
 		args->tnr_frames[i] = NULL;
 	for (i = 0; i < MAX_NUM_VIDEO_DELAY_FRAMES; i++)
 		args->delay_frames[i] = NULL;
@@ -2228,10 +2228,10 @@ ia_css_pipe_destroy(struct ia_css_pipe *pipe)
 			}
 		}
 #ifndef ISP2401
-		ia_css_frame_free_multiple(NUM_TNR_FRAMES,
+		ia_css_frame_free_multiple(NUM_VIDEO_TNR_FRAMES,
 					   pipe->pipe_settings.video.tnr_frames);
 #else
-		ia_css_frame_free_multiple(NUM_TNR_FRAMES,
+		ia_css_frame_free_multiple(NUM_VIDEO_TNR_FRAMES,
 					   pipe->pipe_settings.video.tnr_frames);
 #endif
 		ia_css_frame_free_multiple(MAX_NUM_VIDEO_DELAY_FRAMES,
@@ -3424,7 +3424,7 @@ static int create_host_video_pipeline(struct ia_css_pipe *pipe)
 	if (video_stage) {
 		int frm;
 
-		for (frm = 0; frm < NUM_TNR_FRAMES; frm++) {
+		for (frm = 0; frm < NUM_VIDEO_TNR_FRAMES; frm++) {
 			video_stage->args.tnr_frames[frm] =
 			    pipe->pipe_settings.video.tnr_frames[frm];
 		}
@@ -5163,7 +5163,7 @@ static int load_video_binaries(struct ia_css_pipe *pipe)
 	tnr_info.format = IA_CSS_FRAME_FORMAT_YUV_LINE;
 	tnr_info.raw_bit_depth = SH_CSS_TNR_BIT_DEPTH;
 
-	for (i = 0; i < NUM_TNR_FRAMES; i++) {
+	for (i = 0; i < NUM_VIDEO_TNR_FRAMES; i++) {
 		if (mycs->tnr_frames[i]) {
 			ia_css_frame_free(mycs->tnr_frames[i]);
 			mycs->tnr_frames[i] = NULL;
diff --git a/drivers/staging/media/atomisp/pci/sh_css_defs.h b/drivers/staging/media/atomisp/pci/sh_css_defs.h
index 4dd2aa06659a..353e9ea890dd 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_defs.h
+++ b/drivers/staging/media/atomisp/pci/sh_css_defs.h
@@ -163,50 +163,20 @@ RGB[0,8191],coef[-8192,8191] -> RGB[0,8191]
 #define SH_CSS_MORPH_TABLE_ELEMS_PER_DDR_WORD \
 	(HIVE_ISP_DDR_WORD_BYTES / SH_CSS_MORPH_TABLE_ELEM_BYTES)
 
-#define ISP2400_SH_CSS_MAX_SCTBL_WIDTH_PER_COLOR   (SH_CSS_MAX_BQ_GRID_WIDTH + 1)
-#define ISP2400_SH_CSS_MAX_SCTBL_HEIGHT_PER_COLOR   (SH_CSS_MAX_BQ_GRID_HEIGHT + 1)
+#define SH_CSS_MAX_SCTBL_WIDTH_PER_COLOR   (SH_CSS_MAX_BQ_GRID_WIDTH + 1)
+#define SH_CSS_MAX_SCTBL_HEIGHT_PER_COLOR   (SH_CSS_MAX_BQ_GRID_HEIGHT + 1)
 
-#define ISP2400_SH_CSS_MAX_SCTBL_ALIGNED_WIDTH_PER_COLOR \
-	CEIL_MUL(ISP2400_SH_CSS_MAX_SCTBL_WIDTH_PER_COLOR, ISP_VEC_NELEMS)
-
-/* TODO: I will move macros of "*_SCTBL_*" to SC kernel.
-   "+ 2" should be "+ SH_CSS_SCTBL_CENTERING_MARGIN + SH_CSS_SCTBL_LAST_GRID_COUNT". (michie, Sep/23/2014) */
-#define ISP2401_SH_CSS_MAX_SCTBL_WIDTH_PER_COLOR   (SH_CSS_MAX_BQ_GRID_WIDTH + 2)
-#define ISP2401_SH_CSS_MAX_SCTBL_HEIGHT_PER_COLOR   (SH_CSS_MAX_BQ_GRID_HEIGHT + 2)
-
-#define ISP2401_SH_CSS_MAX_SCTBL_ALIGNED_WIDTH_PER_COLOR \
-	CEIL_MUL(ISP2400_SH_CSS_MAX_SCTBL_WIDTH_PER_COLOR, ISP_VEC_NELEMS)
+#define SH_CSS_MAX_SCTBL_ALIGNED_WIDTH_PER_COLOR \
+	CEIL_MUL(SH_CSS_MAX_SCTBL_WIDTH_PER_COLOR, ISP_VEC_NELEMS)
 
 /* Each line of this table is aligned to the maximum line width. */
 #define SH_CSS_MAX_S3ATBL_WIDTH              SH_CSS_MAX_BQ_GRID_WIDTH
 
 /* Video mode specific DVS define */
 /* The video binary supports a delay of 1 or 2 frames */
-#define VIDEO_FRAME_DELAY		2
+#define MAX_DVS_FRAME_DELAY		2
 /* +1 because DVS reads the previous and writes the current frame concurrently */
-#define MAX_NUM_VIDEO_DELAY_FRAMES	(VIDEO_FRAME_DELAY + 1)
-
-/* Preview mode specific DVS define. */
-/* In preview we only need GDC functionality (and not the DVS functionality) */
-/* The minimum number of DVS frames you need is 2, one were GDC reads from and another where GDC writes into */
-#define NUM_PREVIEW_DVS_FRAMES		(2)
-
-/* TNR is no longer exclusive to video, SkyCam preview has TNR too (same kernel as video).
- * All uses the generic define NUM_TNR_FRAMES. The define NUM_VIDEO_TNR_FRAMES has been deprecated.
- *
- * Notes
- * 1) The value depends on the used TNR kernel and is not something that depends on the mode
- *    and it is not something you just could choice.
- * 2) For the luma only pipeline a version that supports two different sets of TNR reference frames
- * is being used.
- *.
- */
-#define NUM_VALID_TNR_REF_FRAMES		(1) /* At least one valid TNR reference frame is required */
-#define NUM_TNR_FRAMES_PER_REF_BUF_SET		(2)
-/* In luma-only mode alternate illuminated frames are supported, that requires two double buffers */
-#define NUM_TNR_REF_BUF_SETS	(1)
-
-#define NUM_TNR_FRAMES		(NUM_TNR_FRAMES_PER_REF_BUF_SET * NUM_TNR_REF_BUF_SETS)
+#define MAX_NUM_VIDEO_DELAY_FRAMES	(MAX_DVS_FRAME_DELAY + 1)
 
 #define NUM_VIDEO_TNR_FRAMES		2
 
@@ -235,11 +205,11 @@ RGB[0,8191],coef[-8192,8191] -> RGB[0,8191]
 	CEIL_MUL(_ISP_MORPH_TABLE_WIDTH(width), \
 		 SH_CSS_MORPH_TABLE_ELEMS_PER_DDR_WORD)
 
-#define _ISP2400_SCTBL_WIDTH_PER_COLOR(input_width, deci_factor_log2) \
+#define _ISP_SCTBL_WIDTH_PER_COLOR(input_width, deci_factor_log2) \
 	(ISP_BQ_GRID_WIDTH(input_width, deci_factor_log2) + 1)
-#define _ISP2400_SCTBL_HEIGHT(input_height, deci_factor_log2) \
+#define _ISP_SCTBL_HEIGHT(input_height, deci_factor_log2) \
 	(ISP_BQ_GRID_HEIGHT(input_height, deci_factor_log2) + 1)
-#define _ISP2400_SCTBL_ALIGNED_WIDTH_PER_COLOR(input_width, deci_factor_log2) \
+#define _ISP_SCTBL_ALIGNED_WIDTH_PER_COLOR(input_width, deci_factor_log2) \
 	CEIL_MUL(_ISP_SCTBL_WIDTH_PER_COLOR(input_width, deci_factor_log2), \
 		 ISP_VEC_NELEMS)
 
diff --git a/drivers/staging/media/atomisp/pci/sh_css_internal.h b/drivers/staging/media/atomisp/pci/sh_css_internal.h
index a3693902611f..71961a4220a6 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_internal.h
+++ b/drivers/staging/media/atomisp/pci/sh_css_internal.h
@@ -270,7 +270,7 @@ struct sh_css_binary_args {
 	struct ia_css_frame *in_frame;	     /* input frame */
 	const struct ia_css_frame
 		*delay_frames[MAX_NUM_VIDEO_DELAY_FRAMES];   /* reference input frame */
-	const struct ia_css_frame *tnr_frames[NUM_TNR_FRAMES];   /* tnr frames */
+	const struct ia_css_frame *tnr_frames[NUM_VIDEO_TNR_FRAMES];   /* tnr frames */
 	struct ia_css_frame
 		*out_frame[IA_CSS_BINARY_MAX_OUTPUT_PORTS];      /* output frame */
 	struct ia_css_frame *out_vf_frame;   /* viewfinder output frame */
-- 
2.33.1

[PATCH 03/13] media: atomisp: sh_css_metrics: drop some unused code

[Mauro Carvalho Chehab]

There are two #ifdefs there which aren't defined anywhere.

So, just drop the dead code.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---

To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH 00/13] at: https://lore.kernel.org/all/cover.1637140900.git.mchehab+huawei@kernel.org/

 .../media/atomisp/pci/sh_css_metrics.c        | 25 -------------------
 1 file changed, 25 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/sh_css_metrics.c b/drivers/staging/media/atomisp/pci/sh_css_metrics.c
index 9744bbebe1bc..8ded6cdd1575 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_metrics.c
+++ b/drivers/staging/media/atomisp/pci/sh_css_metrics.c
@@ -123,40 +123,15 @@ sh_css_metrics_sample_pcs(void)
 	unsigned int pc;
 	unsigned int msink;
 
-#if SUSPEND
-	unsigned int sc = 0;
-	unsigned int stopped_sc = 0;
-	unsigned int resume_sc = 0;
-#endif
 
-#if MULTIPLE_PCS
-	int i;
-	unsigned int pc_tab[NOF_PCS];
-
-	for (i = 0; i < NOF_PCS; i++)
-		pc_tab[i] = 0;
-#endif
 
 	if (!pc_histogram_enabled)
 		return;
 
 	if (isp_histogram) {
-#if SUSPEND
-		/* STOP the ISP */
-		isp_ctrl_store(ISP0_ID, ISP_SC_REG, STOP_MASK);
-#endif
 		msink = isp_ctrl_load(ISP0_ID, ISP_CTRL_SINK_REG);
-#if MULTIPLE_PCS
-		for (i = 0; i < NOF_PCS; i++)
-			pc_tab[i] = isp_ctrl_load(ISP0_ID, ISP_PC_REG);
-#else
 		pc = isp_ctrl_load(ISP0_ID, ISP_PC_REG);
-#endif
 
-#if SUSPEND
-		/* RESUME the ISP */
-		isp_ctrl_store(ISP0_ID, ISP_SC_REG, RESUME_MASK);
-#endif
 		isp_histogram->msink[pc] &= msink;
 		stall = (msink != 0x7FF);
 
-- 
2.33.1

Re: [PATCH 03/13] media: atomisp: sh_css_metrics: drop some unused code

[Dan Carpenter]

On Wed, Nov 17, 2021 at 09:25:28AM +0000, Mauro Carvalho Chehab wrote:
>  .../media/atomisp/pci/sh_css_metrics.c        | 25 -------------------
>  1 file changed, 25 deletions(-)
> 
> diff --git a/drivers/staging/media/atomisp/pci/sh_css_metrics.c b/drivers/staging/media/atomisp/pci/sh_css_metrics.c
> index 9744bbebe1bc..8ded6cdd1575 100644
> --- a/drivers/staging/media/atomisp/pci/sh_css_metrics.c
> +++ b/drivers/staging/media/atomisp/pci/sh_css_metrics.c
> @@ -123,40 +123,15 @@ sh_css_metrics_sample_pcs(void)
>  	unsigned int pc;
>  	unsigned int msink;
>  
> -#if SUSPEND
> -	unsigned int sc = 0;
> -	unsigned int stopped_sc = 0;
> -	unsigned int resume_sc = 0;
> -#endif
>  
> -#if MULTIPLE_PCS
> -	int i;
> -	unsigned int pc_tab[NOF_PCS];
> -
> -	for (i = 0; i < NOF_PCS; i++)
> -		pc_tab[i] = 0;
> -#endif
>  
>  	if (!pc_histogram_enabled)
>  		return;

There are three blank lines in a row now.  Checkpatch is not smart
enough to spot this until you re-run it with the -f file mode after
applying the patch.

This is an easy mistake to make and you did it a couple times in
yesterday's patchset as well.  No big deal.  Could you re-run
checkpatch.pl at the end and clean it up afterwards?

regards,
dan carpenter

[PATCH 04/13] media: atomisp: sh_css_mipi: cleanup the code

[Mauro Carvalho Chehab]

With the ISP2401 firmware we're using, the code differences
are not that much from ISP2400. Cleanup the code in order
to make it closer to Intel Aero driver.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---

To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH 00/13] at: https://lore.kernel.org/all/cover.1637140900.git.mchehab+huawei@kernel.org/

 .../staging/media/atomisp/pci/sh_css_mipi.c   | 159 +++++-------------
 .../staging/media/atomisp/pci/sh_css_mipi.h   |  15 --
 2 files changed, 38 insertions(+), 136 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/sh_css_mipi.c b/drivers/staging/media/atomisp/pci/sh_css_mipi.c
index cfaa4719177c..0acf75497ae7 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_mipi.c
+++ b/drivers/staging/media/atomisp/pci/sh_css_mipi.c
@@ -33,47 +33,6 @@
 static u32
 ref_count_mipi_allocation[N_CSI_PORTS]; /* Initialized in mipi_init */
 
-/*
- * Check if a source port or TPG/PRBS ID is valid
- */
-static bool ia_css_mipi_is_source_port_valid(struct ia_css_pipe *pipe,
-	unsigned int *pport)
-{
-	bool ret = true;
-	unsigned int port = 0;
-	unsigned int max_ports = 0;
-
-	switch (pipe->stream->config.mode) {
-	case IA_CSS_INPUT_MODE_BUFFERED_SENSOR:
-		port = (unsigned int)pipe->stream->config.source.port.port;
-		max_ports = N_CSI_PORTS;
-		break;
-	case IA_CSS_INPUT_MODE_TPG:
-		port = (unsigned int)pipe->stream->config.source.tpg.id;
-		max_ports = N_CSS_TPG_IDS;
-		break;
-	case IA_CSS_INPUT_MODE_PRBS:
-		port = (unsigned int)pipe->stream->config.source.prbs.id;
-		max_ports = N_CSS_PRBS_IDS;
-		break;
-	default:
-		assert(false);
-		ret = false;
-		break;
-	}
-
-	if (ret) {
-		assert(port < max_ports);
-
-		if (port >= max_ports)
-			ret = false;
-	}
-
-	*pport = port;
-
-	return ret;
-}
-
 /* Assumptions:
  *	- A line is multiple of 4 bytes = 1 word.
  *	- Each frame has SOF and EOF (each 1 word).
@@ -227,6 +186,10 @@ ia_css_mipi_frame_calculate_size(const unsigned int width,
 	return err;
 }
 
+/*
+ * Check if a source port or TPG/PRBS ID is valid
+ */
+
 #if !defined(ISP2401)
 int
 ia_css_mipi_frame_enable_check_on_size(const enum mipi_port_id port,
@@ -272,16 +235,20 @@ bool mipi_is_free(void)
 	return true;
 }
 
-int
-calculate_mipi_buff_size(
-    struct ia_css_stream_config *stream_cfg,
-    unsigned int *size_mem_words)
+#if defined(ISP2401)
+/*
+ * @brief Calculate the required MIPI buffer sizes.
+ * Based on the stream configuration, calculate the
+ * required MIPI buffer sizes (in DDR words).
+ *
+ * @param[in]   stream_cfg              Point to the target stream configuration
+ * @param[out]  size_mem_words  MIPI buffer size in DDR words.
+ *
+ * @return
+ */
+static int calculate_mipi_buff_size(struct ia_css_stream_config *stream_cfg,
+				    unsigned int *size_mem_words)
 {
-#if !defined(ISP2401)
-	int err = -EINVAL;
-	(void)stream_cfg;
-	(void)size_mem_words;
-#else
 	unsigned int width;
 	unsigned int height;
 	enum atomisp_input_format format;
@@ -373,26 +340,9 @@ calculate_mipi_buff_size(
 	*size_mem_words = mem_words_per_buff;
 
 	IA_CSS_LEAVE_ERR(err);
-#endif
 	return err;
 }
-
-static bool buffers_needed(struct ia_css_pipe *pipe)
-{
-	if (!IS_ISP2401) {
-		if (pipe->stream->config.mode == IA_CSS_INPUT_MODE_BUFFERED_SENSOR)
-			return true;
-		else
-			return false;
-	}
-
-	if (pipe->stream->config.mode == IA_CSS_INPUT_MODE_BUFFERED_SENSOR ||
-	    pipe->stream->config.mode == IA_CSS_INPUT_MODE_TPG ||
-	    pipe->stream->config.mode == IA_CSS_INPUT_MODE_PRBS)
-		return true;
-
-	return false;
-}
+#endif
 
 int
 allocate_mipi_frames(struct ia_css_pipe *pipe,
@@ -422,45 +372,25 @@ allocate_mipi_frames(struct ia_css_pipe *pipe,
 	}
 
 #endif
-
-	if (!buffers_needed(pipe)) {
+	if (pipe->stream->config.mode != IA_CSS_INPUT_MODE_BUFFERED_SENSOR) {
 		ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE_PRIVATE,
 				    "allocate_mipi_frames(%p) exit: no buffers needed for pipe mode.\n",
 				    pipe);
 		return 0; /* AM TODO: Check  */
 	}
 
-	if (!IS_ISP2401) {
-		port = (unsigned int)pipe->stream->config.source.port.port;
-	} else {
-		/* Returns true if port is valid. So, invert it */
-		err = !ia_css_mipi_is_source_port_valid(pipe, &port);
-	}
-
-	assert(port < N_CSI_PORTS);
-
-	if ((!IS_ISP2401 && port >= N_CSI_PORTS) ||
-	    (IS_ISP2401 && err)) {
+	port = (unsigned int)pipe->stream->config.source.port.port;
+	if (port >= N_CSI_PORTS) {
 		IA_CSS_ERROR("allocate_mipi_frames(%p) exit: port is not correct (port=%d).",
 			     pipe, port);
 		return -EINVAL;
 	}
 
 #ifdef ISP2401
-	err = calculate_mipi_buff_size(
-	    &pipe->stream->config,
-	    &my_css.mipi_frame_size[port]);
-#endif
-
-#if !defined(ISP2401)
-	if (ref_count_mipi_allocation[port] != 0) {
-		ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE_PRIVATE,
-				    "allocate_mipi_frames(%p) exit: already allocated for this port (port=%d).\n",
-				    pipe, port);
-		return 0;
-	}
-#else
-	/* 2401 system allows multiple streams to use same physical port. This is not
+	err = calculate_mipi_buff_size(&pipe->stream->config,
+				       &my_css.mipi_frame_size[port]);
+	/*
+	 * 2401 system allows multiple streams to use same physical port. This is not
 	 * true for 2400 system. Currently 2401 uses MIPI buffers as a temporary solution.
 	 * TODO AM: Once that is changed (removed) this code should be removed as well.
 	 * In that case only 2400 related code should remain.
@@ -472,6 +402,13 @@ allocate_mipi_frames(struct ia_css_pipe *pipe,
 				    pipe, port);
 		return 0;
 	}
+#else
+	if (ref_count_mipi_allocation[port] != 0) {
+		ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE_PRIVATE,
+				    "allocate_mipi_frames(%p) exit: already allocated for this port (port=%d).\n",
+				    pipe, port);
+		return 0;
+	}
 #endif
 
 	ref_count_mipi_allocation[port]++;
@@ -552,23 +489,15 @@ free_mipi_frames(struct ia_css_pipe *pipe)
 			return -EINVAL;
 		}
 
-		if (!buffers_needed(pipe)) {
+		if (pipe->stream->config.mode != IA_CSS_INPUT_MODE_BUFFERED_SENSOR) {
 			IA_CSS_ERROR("free_mipi_frames(%p) exit: wrong mode.",
 				     pipe);
 			return err;
 		}
 
-		if (!IS_ISP2401) {
-			port = (unsigned int)pipe->stream->config.source.port.port;
-		} else {
-			/* Returns true if port is valid. So, invert it */
-			err = !ia_css_mipi_is_source_port_valid(pipe, &port);
-		}
+		port = (unsigned int)pipe->stream->config.source.port.port;
 
-		assert(port < N_CSI_PORTS);
-
-		if ((!IS_ISP2401 && port >= N_CSI_PORTS) ||
-		    (IS_ISP2401 && err)) {
+		if (port >= N_CSI_PORTS) {
 			IA_CSS_ERROR("free_mipi_frames(%p, %d) exit: pipe port is not correct.",
 				     pipe, port);
 			return err;
@@ -647,11 +576,7 @@ send_mipi_frames(struct ia_css_pipe *pipe)
 {
 	int err = -EINVAL;
 	unsigned int i;
-#ifndef ISP2401
 	unsigned int port;
-#else
-	unsigned int port = 0;
-#endif
 
 	IA_CSS_ENTER_PRIVATE("pipe=%p", pipe);
 
@@ -664,23 +589,15 @@ send_mipi_frames(struct ia_css_pipe *pipe)
 
 	/* multi stream video needs mipi buffers */
 	/* nothing to be done in other cases. */
-	if (!buffers_needed(pipe)) {
+	if (pipe->stream->config.mode != IA_CSS_INPUT_MODE_BUFFERED_SENSOR) {
 		IA_CSS_LOG("nothing to be done for this mode");
 		return 0;
 		/* TODO: AM: maybe this should be returning an error. */
 	}
 
-	if (!IS_ISP2401) {
-		port = (unsigned int)pipe->stream->config.source.port.port;
-	} else {
-		/* Returns true if port is valid. So, invert it */
-		err = !ia_css_mipi_is_source_port_valid(pipe, &port);
-	}
+	port = (unsigned int)pipe->stream->config.source.port.port;
 
-	assert(port < N_CSI_PORTS);
-
-	if ((!IS_ISP2401 && port >= N_CSI_PORTS) ||
-	    (IS_ISP2401 && err)) {
+	if (port >= N_CSI_PORTS) {
 		IA_CSS_ERROR("send_mipi_frames(%p) exit: invalid port specified (port=%d).",
 			     pipe, port);
 		return err;
diff --git a/drivers/staging/media/atomisp/pci/sh_css_mipi.h b/drivers/staging/media/atomisp/pci/sh_css_mipi.h
index dffec2205620..e6c86d0ac483 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_mipi.h
+++ b/drivers/staging/media/atomisp/pci/sh_css_mipi.h
@@ -34,19 +34,4 @@ free_mipi_frames(struct ia_css_pipe *pipe);
 int
 send_mipi_frames(struct ia_css_pipe *pipe);
 
-/**
- * @brief Calculate the required MIPI buffer sizes.
- * Based on the stream configuration, calculate the
- * required MIPI buffer sizes (in DDR words).
- *
- * @param[in]	stream_cfg		Point to the target stream configuration
- * @param[out]	size_mem_words	MIPI buffer size in DDR words.
- *
- * @return
- */
-int
-calculate_mipi_buff_size(
-    struct ia_css_stream_config *stream_cfg,
-    unsigned int *size_mem_words);
-
 #endif /* __SH_CSS_MIPI_H */
-- 
2.33.1

[PATCH 05/13] media: atomisp: sh_css_params: remove tests for ISP2401

[Mauro Carvalho Chehab]

Those tests are related to the input system, which is the same
for the chosen firmware, so both ISP2400 and ISP2401 will be
identical with that regards.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---

To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH 00/13] at: https://lore.kernel.org/all/cover.1637140900.git.mchehab+huawei@kernel.org/

 .../staging/media/atomisp/pci/sh_css_params.c | 117 ++----------------
 1 file changed, 8 insertions(+), 109 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/sh_css_params.c b/drivers/staging/media/atomisp/pci/sh_css_params.c
index 448b07162382..68d8f54473ea 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_params.c
+++ b/drivers/staging/media/atomisp/pci/sh_css_params.c
@@ -104,15 +104,10 @@
 	(sizeof(char) * (binary)->in_frame_info.res.height * \
 	 (binary)->in_frame_info.padded_width)
 
-#define ISP2400_SCTBL_BYTES(binary) \
+#define SCTBL_BYTES(binary) \
 	(sizeof(unsigned short) * (binary)->sctbl_height * \
 	 (binary)->sctbl_aligned_width_per_color * IA_CSS_SC_NUM_COLORS)
 
-#define ISP2401_SCTBL_BYTES(binary) \
-	(sizeof(unsigned short) * max((binary)->sctbl_height, (binary)->sctbl_legacy_height) * \
-			/* height should be the larger height between new api and legacy api */ \
-	 (binary)->sctbl_aligned_width_per_color * IA_CSS_SC_NUM_COLORS)
-
 #define MORPH_PLANE_BYTES(binary) \
 	(SH_CSS_MORPH_TABLE_ELEM_BYTES * (binary)->morph_tbl_aligned_width * \
 	 (binary)->morph_tbl_height)
@@ -1611,18 +1606,6 @@ ia_css_set_param_exceptions(const struct ia_css_pipe *pipe,
 	params->dp_config.r  = params->wb_config.r;
 	params->dp_config.b  = params->wb_config.b;
 	params->dp_config.gb = params->wb_config.gb;
-
-	if (IS_ISP2401) {
-		assert(pipe);
-		assert(pipe->mode < IA_CSS_PIPE_ID_NUM);
-
-		if (pipe->mode < IA_CSS_PIPE_ID_NUM) {
-			params->pipe_dp_config[pipe->mode].gr = params->wb_config.gr;
-			params->pipe_dp_config[pipe->mode].r  = params->wb_config.r;
-			params->pipe_dp_config[pipe->mode].b  = params->wb_config.b;
-			params->pipe_dp_config[pipe->mode].gb = params->wb_config.gb;
-		}
-	}
 }
 
 /* ISP2401 */
@@ -2072,16 +2055,6 @@ sh_css_init_isp_params_from_config(struct ia_css_pipe *pipe,
 	params->output_frame = config->output_frame;
 	params->isp_parameters_id = config->isp_config_id;
 
-	/* Currently we do not offer CSS interface to set different
-	 * configurations for DPC, i.e. depending on DPC being enabled
-	 * before (NORM+OBC) or after. The folllowing code to set the
-	 * DPC configuration should be updated when this interface is made
-	 * available */
-	if (IS_ISP2401) {
-		sh_css_set_dp_config(pipe, params, config->dp_config);
-		ia_css_set_param_exceptions(pipe, params);
-	}
-
 	if (0 ==
 	    sh_css_select_dp_10bpp_config(pipe, &is_dp_10bpp)) {
 		/* return an error when both DPC and BDS is enabled by the
@@ -2096,8 +2069,7 @@ sh_css_init_isp_params_from_config(struct ia_css_pipe *pipe,
 		goto exit;
 	}
 
-	if (!IS_ISP2401)
-		ia_css_set_param_exceptions(pipe, params);
+	ia_css_set_param_exceptions(pipe, params);
 
 exit:
 	IA_CSS_LEAVE_ERR_PRIVATE(err);
@@ -2514,29 +2486,7 @@ sh_css_init_isp_params_from_global(struct ia_css_stream *stream,
 		ia_css_set_ob_config(params, &default_ob_config);
 		ia_css_set_dp_config(params, &default_dp_config);
 
-		if (!IS_ISP2401) {
-			ia_css_set_param_exceptions(pipe_in, params);
-		} else {
-			for (i = 0; i < stream->num_pipes; i++) {
-				if (sh_css_select_dp_10bpp_config(stream->pipes[i],
-								&is_dp_10bpp) == 0) {
-					/* set the return value as false if both DPC and
-					* BDS is enabled by the user. But we do not return
-					* the value immediately to enable internal firmware
-					* feature testing. */
-					if (is_dp_10bpp) {
-						sh_css_set_dp_config(stream->pipes[i], params, &default_dp_10bpp_config);
-					} else {
-						sh_css_set_dp_config(stream->pipes[i], params, &default_dp_config);
-					}
-				} else {
-					retval = false;
-					goto exit;
-				}
-
-				ia_css_set_param_exceptions(stream->pipes[i], params);
-			}
-		}
+		ia_css_set_param_exceptions(pipe_in, params);
 
 		ia_css_set_de_config(params, &default_de_config);
 		ia_css_set_gc_config(params, &default_gc_config);
@@ -2641,20 +2591,9 @@ sh_css_init_isp_params_from_global(struct ia_css_stream *stream,
 				retval = false;
 				goto exit;
 			}
-			if (IS_ISP2401) {
-				if (stream->pipes[i]->mode < IA_CSS_PIPE_ID_NUM) {
-					sh_css_set_dp_config(stream->pipes[i], params,
-							    &stream_params->pipe_dp_config[stream->pipes[i]->mode]);
-					ia_css_set_param_exceptions(stream->pipes[i], params);
-				} else {
-					retval = false;
-					goto exit;
-				}
-			}
 		}
 
-		if (!IS_ISP2401)
-			ia_css_set_param_exceptions(pipe_in, params);
+		ia_css_set_param_exceptions(pipe_in, params);
 
 		params->fpn_config.data = stream_params->fpn_config.data;
 		params->config_changed[IA_CSS_FPN_ID] =
@@ -2806,10 +2745,7 @@ int ia_css_pipe_set_bci_scaler_lut(struct ia_css_pipe *pipe,
 	pipe->scaler_pp_lut = mmgr_NULL;
 
 	if (!stream_started) {
-		if (!IS_ISP2401)
-			pipe->scaler_pp_lut = hmm_alloc(sizeof(zoom_table), HMM_BO_PRIVATE, 0, NULL, 0);
-		else
-			pipe->scaler_pp_lut = sh_css_params_alloc_gdc_lut();
+		pipe->scaler_pp_lut = hmm_alloc(sizeof(zoom_table), HMM_BO_PRIVATE, 0, NULL, 0);
 
 		if (pipe->scaler_pp_lut == mmgr_NULL) {
 			ia_css_debug_dtrace(IA_CSS_DEBUG_ERROR,
@@ -2851,10 +2787,7 @@ int sh_css_params_map_and_store_default_gdc_lut(void)
 
 	host_lut_store((void *)zoom_table);
 
-	if (!IS_ISP2401)
-		default_gdc_lut = hmm_alloc(sizeof(zoom_table), HMM_BO_PRIVATE, 0, NULL, 0);
-	else
-		default_gdc_lut = sh_css_params_alloc_gdc_lut();
+	default_gdc_lut = hmm_alloc(sizeof(zoom_table), HMM_BO_PRIVATE, 0, NULL, 0);
 
 	if (default_gdc_lut == mmgr_NULL)
 		return -ENOMEM;
@@ -3450,18 +3383,12 @@ sh_css_params_write_to_ddr_internal(
 
 	if (binary->info->sp.enable.sc) {
 		u32 enable_conv;
-		size_t bytes;
-
-		if (!IS_ISP2401)
-			bytes = ISP2400_SCTBL_BYTES(binary);
-		else
-			bytes = ISP2401_SCTBL_BYTES(binary);
 
 		enable_conv = params->shading_settings.enable_shading_table_conversion;
 
 		buff_realloced = reallocate_buffer(&ddr_map->sc_tbl,
 						   &ddr_map_size->sc_tbl,
-						   bytes,
+						   SCTBL_BYTES(binary),
 						   params->sc_table_changed,
 						   &err);
 		if (err) {
@@ -3546,28 +3473,6 @@ sh_css_params_write_to_ddr_internal(
 		}
 	}
 
-	/* DPC configuration is made pipe specific to allow flexibility in positioning of the
-	 * DPC kernel. The code below sets the pipe specific configuration to
-	 * individual binaries. */
-	if (IS_ISP2401 &&
-	    params->pipe_dpc_config_changed[pipe_id] && binary->info->sp.enable.dpc) {
-		unsigned int size   =
-		    stage->binary->info->mem_offsets.offsets.param->dmem.dp.size;
-
-		unsigned int offset =
-		    stage->binary->info->mem_offsets.offsets.param->dmem.dp.offset;
-
-		if (size) {
-			ia_css_dp_encode((struct sh_css_isp_dp_params *)
-					 &binary->mem_params.params[IA_CSS_PARAM_CLASS_PARAM][IA_CSS_ISP_DMEM].address[offset],
-					 &params->pipe_dp_config[pipe_id], size);
-
-			params->isp_params_changed = true;
-			params->isp_mem_params_changed[pipe_id][stage->stage_num][IA_CSS_ISP_DMEM] =
-			    true;
-		}
-	}
-
 	if (params->config_changed[IA_CSS_MACC_ID] && binary->info->sp.enable.macc) {
 		unsigned int i, j, idx;
 		unsigned int idx_map[] = {
@@ -3649,13 +3554,7 @@ sh_css_params_write_to_ddr_internal(
 			if (!params->pipe_dvs_6axis_config[pipe_id]) {
 				struct ia_css_resolution dvs_offset = {0};
 
-				if (!IS_ISP2401) {
-					dvs_offset.width = (PIX_SHIFT_FILTER_RUN_IN_X + binary->dvs_envelope.width) / 2;
-				} else {
-					if (binary->dvs_envelope.width || binary->dvs_envelope.height) {
-						dvs_offset.width  = (PIX_SHIFT_FILTER_RUN_IN_X + binary->dvs_envelope.width) / 2;
-					}
-				}
+				dvs_offset.width = (PIX_SHIFT_FILTER_RUN_IN_X + binary->dvs_envelope.width) / 2;
 				dvs_offset.height = (PIX_SHIFT_FILTER_RUN_IN_Y + binary->dvs_envelope.height) / 2;
 
 				params->pipe_dvs_6axis_config[pipe_id] =
-- 
2.33.1

[PATCH 06/13] media: atomisp: sh_css_params: cleanup the code

[Mauro Carvalho Chehab]

Now that the tests for the new ISP2401 input system were
dropped, simplify the code, making it closer to the Intel
Aero device driver.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---

To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH 00/13] at: https://lore.kernel.org/all/cover.1637140900.git.mchehab+huawei@kernel.org/

 drivers/staging/media/atomisp/pci/sh_css.c    |  6 +-
 .../staging/media/atomisp/pci/sh_css_params.c | 92 +++----------------
 .../staging/media/atomisp/pci/sh_css_params.h |  8 --
 3 files changed, 15 insertions(+), 91 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c
index acaa4c8b1884..1d605e533e29 100644
--- a/drivers/staging/media/atomisp/pci/sh_css.c
+++ b/drivers/staging/media/atomisp/pci/sh_css.c
@@ -2250,8 +2250,10 @@ ia_css_pipe_destroy(struct ia_css_pipe *pipe)
 		break;
 	}
 
-	sh_css_params_free_gdc_lut(pipe->scaler_pp_lut);
-	pipe->scaler_pp_lut = mmgr_NULL;
+	if (pipe->scaler_pp_lut != mmgr_NULL) {
+		hmm_free(pipe->scaler_pp_lut);
+		pipe->scaler_pp_lut = mmgr_NULL;
+	}
 
 	my_css.active_pipes[ia_css_pipe_get_pipe_num(pipe)] = NULL;
 	sh_css_pipe_free_shading_table(pipe);
diff --git a/drivers/staging/media/atomisp/pci/sh_css_params.c b/drivers/staging/media/atomisp/pci/sh_css_params.c
index 68d8f54473ea..f3f8d99c84a4 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_params.c
+++ b/drivers/staging/media/atomisp/pci/sh_css_params.c
@@ -1020,16 +1020,6 @@ sh_css_params_set_binning_factor(struct ia_css_stream *stream,
 	return params->sc_table_changed;
 }
 
-static void
-sh_css_update_shading_table_status(struct ia_css_pipe *pipe,
-				   struct ia_css_isp_parameters *params)
-{
-	if (params && pipe && (pipe->pipe_num != params->sc_table_last_pipe_num)) {
-		params->sc_table_dirty = true;
-		params->sc_table_last_pipe_num = pipe->pipe_num;
-	}
-}
-
 static void
 sh_css_set_shading_table(struct ia_css_stream *stream,
 			 struct ia_css_isp_parameters *params,
@@ -1043,10 +1033,9 @@ sh_css_set_shading_table(struct ia_css_stream *stream,
 	if (!table->enable)
 		table = NULL;
 
-	if ((table != params->sc_table) || params->sc_table_dirty) {
+	if (table != params->sc_table) {
 		params->sc_table = table;
 		params->sc_table_changed = true;
-		params->sc_table_dirty = false;
 		/* Not very clean, this goes to sh_css.c to invalidate the
 		 * shading table for all pipes. Should replaced by a loop
 		 * and a pipe-specific call.
@@ -1608,45 +1597,6 @@ ia_css_set_param_exceptions(const struct ia_css_pipe *pipe,
 	params->dp_config.gb = params->wb_config.gb;
 }
 
-/* ISP2401 */
-static void
-sh_css_set_dp_config(const struct ia_css_pipe *pipe,
-		     struct ia_css_isp_parameters *params,
-		     const struct ia_css_dp_config *config)
-{
-	if (!config)
-		return;
-
-	assert(params);
-	assert(pipe);
-	assert(pipe->mode < IA_CSS_PIPE_ID_NUM);
-
-	IA_CSS_ENTER_PRIVATE("config=%p", config);
-	ia_css_dp_debug_dtrace(config, IA_CSS_DEBUG_TRACE_PRIVATE);
-	if (pipe->mode < IA_CSS_PIPE_ID_NUM) {
-		params->pipe_dp_config[pipe->mode] = *config;
-		params->pipe_dpc_config_changed[pipe->mode] = true;
-	}
-	IA_CSS_LEAVE_PRIVATE("void");
-}
-
-static void
-sh_css_get_dp_config(const struct ia_css_pipe *pipe,
-		     const struct ia_css_isp_parameters *params,
-		     struct ia_css_dp_config *config)
-{
-	if (!config)
-		return;
-
-	assert(params);
-	assert(pipe);
-	IA_CSS_ENTER_PRIVATE("config=%p", config);
-
-	*config = params->pipe_dp_config[pipe->mode];
-
-	IA_CSS_LEAVE_PRIVATE("void");
-}
-
 static void
 sh_css_set_nr_config(struct ia_css_isp_parameters *params,
 		     const struct ia_css_nr_config *config)
@@ -1718,9 +1668,7 @@ sh_css_set_pipe_dvs_6axis_config(const struct ia_css_pipe *pipe,
 
 	copy_dvs_6axis_table(params->pipe_dvs_6axis_config[pipe->mode], dvs_config);
 
-#if !defined(HAS_NO_DVS_6AXIS_CONFIG_UPDATE)
 	params->pipe_dvs_6axis_config_changed[pipe->mode] = true;
-#endif
 
 	IA_CSS_LEAVE_PRIVATE("void");
 }
@@ -2039,7 +1987,6 @@ sh_css_init_isp_params_from_config(struct ia_css_pipe *pipe,
 		sh_css_set_pipe_dvs_6axis_config(pipe, params, config->dvs_6axis_config);
 	sh_css_set_dz_config(params, config->dz_config);
 	sh_css_set_motion_vector(params, config->motion_vector);
-	sh_css_update_shading_table_status(pipe_in, params);
 	sh_css_set_shading_table(pipe->stream, params, config->shading_table);
 	sh_css_set_morph_table(params, config->morph_table);
 	sh_css_set_macc_table(params, config->macc_table);
@@ -2104,7 +2051,6 @@ ia_css_pipe_get_isp_config(struct ia_css_pipe *pipe,
 	sh_css_get_ee_config(params, config->ee_config);
 	sh_css_get_baa_config(params, config->baa_config);
 	sh_css_get_pipe_dvs_6axis_config(pipe, params, config->dvs_6axis_config);
-	sh_css_get_dp_config(pipe, params, config->dp_config);
 	sh_css_get_macc_table(params, config->macc_table);
 	sh_css_get_gamma_table(params, config->gamma_table);
 	sh_css_get_ctc_table(params, config->ctc_table);
@@ -2523,8 +2469,6 @@ sh_css_init_isp_params_from_global(struct ia_css_stream *stream,
 
 		params->sc_table = NULL;
 		params->sc_table_changed = true;
-		params->sc_table_dirty = false;
-		params->sc_table_last_pipe_num = 0;
 
 		ia_css_sdis2_clear_coefficients(&params->dvs2_coefs);
 		params->dvs2_coef_table_changed = true;
@@ -2582,11 +2526,8 @@ sh_css_init_isp_params_from_global(struct ia_css_stream *stream,
 				 * BDS is enabled by the user. But we do not return
 				 * the value immediately to enable internal firmware
 				 * feature testing. */
-
-				if (is_dp_10bpp) {
-					retval = false;
-					/* FIXME: should it ignore this error? */
-				}
+				retval = !is_dp_10bpp;
+				/* FIXME: should it ignore this error? */
 			} else {
 				retval = false;
 				goto exit;
@@ -2604,13 +2545,10 @@ sh_css_init_isp_params_from_global(struct ia_css_stream *stream,
 		sh_css_set_morph_table(params, stream_params->morph_table);
 
 		if (stream_params->sc_table) {
-			sh_css_update_shading_table_status(pipe_in, params);
 			sh_css_set_shading_table(stream, params, stream_params->sc_table);
 		} else {
 			params->sc_table = NULL;
 			params->sc_table_changed = true;
-			params->sc_table_dirty = false;
-			params->sc_table_last_pipe_num = 0;
 		}
 
 		/* Only IA_CSS_PIPE_ID_VIDEO & IA_CSS_PIPE_ID_CAPTURE will support dvs_6axis_config*/
@@ -2703,18 +2641,6 @@ static void host_lut_store(const void *lut)
 		gdc_lut_store((gdc_ID_t)i, (const int (*)[HRT_GDC_N]) lut);
 }
 
-/* Note that allocation is in ipu address space. */
-inline ia_css_ptr sh_css_params_alloc_gdc_lut(void)
-{
-	return hmm_alloc(sizeof(zoom_table), HMM_BO_PRIVATE, 0, NULL, 0);
-}
-
-inline void sh_css_params_free_gdc_lut(ia_css_ptr addr)
-{
-	if (addr != mmgr_NULL)
-		hmm_free(addr);
-}
-
 int ia_css_pipe_set_bci_scaler_lut(struct ia_css_pipe *pipe,
 	const void *lut)
 {
@@ -2741,8 +2667,10 @@ int ia_css_pipe_set_bci_scaler_lut(struct ia_css_pipe *pipe,
 	}
 
 	/* Free any existing tables. */
-	sh_css_params_free_gdc_lut(pipe->scaler_pp_lut);
-	pipe->scaler_pp_lut = mmgr_NULL;
+	if (pipe->scaler_pp_lut != mmgr_NULL) {
+		hmm_free(pipe->scaler_pp_lut);
+		pipe->scaler_pp_lut = mmgr_NULL;
+	}
 
 	if (!stream_started) {
 		pipe->scaler_pp_lut = hmm_alloc(sizeof(zoom_table), HMM_BO_PRIVATE, 0, NULL, 0);
@@ -2805,8 +2733,10 @@ void sh_css_params_free_default_gdc_lut(void)
 {
 	IA_CSS_ENTER_PRIVATE("void");
 
-	sh_css_params_free_gdc_lut(default_gdc_lut);
-	default_gdc_lut = mmgr_NULL;
+	if (default_gdc_lut != mmgr_NULL) {
+		hmm_free(default_gdc_lut);
+		default_gdc_lut = mmgr_NULL;
+	}
 
 	IA_CSS_LEAVE_PRIVATE("void");
 }
diff --git a/drivers/staging/media/atomisp/pci/sh_css_params.h b/drivers/staging/media/atomisp/pci/sh_css_params.h
index 62a7b6ada237..bbca19d0e8fc 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_params.h
+++ b/drivers/staging/media/atomisp/pci/sh_css_params.h
@@ -121,8 +121,6 @@ struct ia_css_isp_parameters {
 	bool dvs2_coef_table_changed;
 	bool morph_table_changed;
 	bool sc_table_changed;
-	bool sc_table_dirty;
-	unsigned int sc_table_last_pipe_num;
 	bool anr_thres_changed;
 	/* ---- deprecated: replaced with pipe_dvs_6axis_config_changed ---- */
 	bool dvs_6axis_config_changed;
@@ -168,12 +166,6 @@ ia_css_params_alloc_convert_sctbl(
 struct ia_css_isp_config *
 sh_css_pipe_isp_config_get(struct ia_css_pipe *pipe);
 
-/* ipu address allocation/free for gdc lut */
-ia_css_ptr
-sh_css_params_alloc_gdc_lut(void);
-void
-sh_css_params_free_gdc_lut(ia_css_ptr addr);
-
 int
 sh_css_params_map_and_store_default_gdc_lut(void);
 
-- 
2.33.1

[PATCH 07/13] media: atomisp: remove #ifdef HAS_NO_HMEM

[Mauro Carvalho Chehab]

This is not defined anywhere, so, solve the ifdefs, getting
rid of them.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---

To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH 00/13] at: https://lore.kernel.org/all/cover.1637140900.git.mchehab+huawei@kernel.org/

 .../atomisp/pci/isp/kernels/bh/bh_2/ia_css_bh.host.c   |  2 --
 .../raw_aa_binning_1.0/ia_css_raa.host.c               |  2 --
 .../pci/isp/kernels/s3a/s3a_1.0/ia_css_s3a.host.c      |  5 -----
 .../media/atomisp/pci/runtime/binary/src/binary.c      |  4 ----
 drivers/staging/media/atomisp/pci/sh_css_params.c      | 10 ----------
 5 files changed, 23 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/isp/kernels/bh/bh_2/ia_css_bh.host.c b/drivers/staging/media/atomisp/pci/isp/kernels/bh/bh_2/ia_css_bh.host.c
index 82aa69b74677..2091f001502d 100644
--- a/drivers/staging/media/atomisp/pci/isp/kernels/bh/bh_2/ia_css_bh.host.c
+++ b/drivers/staging/media/atomisp/pci/isp/kernels/bh/bh_2/ia_css_bh.host.c
@@ -13,7 +13,6 @@
  * more details.
  */
 
-#if !defined(HAS_NO_HMEM)
 
 #include "ia_css_types.h"
 #include "sh_css_internal.h"
@@ -63,4 +62,3 @@ ia_css_bh_encode(
 	    uDIGIT_FITTING(from->ae_y_coef_b, 16, SH_CSS_AE_YCOEF_SHIFT);
 }
 
-#endif
diff --git a/drivers/staging/media/atomisp/pci/isp/kernels/raw_aa_binning/raw_aa_binning_1.0/ia_css_raa.host.c b/drivers/staging/media/atomisp/pci/isp/kernels/raw_aa_binning/raw_aa_binning_1.0/ia_css_raa.host.c
index 29c707ecf9f3..9b756daddee0 100644
--- a/drivers/staging/media/atomisp/pci/isp/kernels/raw_aa_binning/raw_aa_binning_1.0/ia_css_raa.host.c
+++ b/drivers/staging/media/atomisp/pci/isp/kernels/raw_aa_binning/raw_aa_binning_1.0/ia_css_raa.host.c
@@ -13,7 +13,6 @@
  * more details.
  */
 
-#if !defined(HAS_NO_HMEM)
 
 #include "ia_css_types.h"
 #include "sh_css_internal.h"
@@ -32,4 +31,3 @@ ia_css_raa_encode(
 	(void)from;
 }
 
-#endif
diff --git a/drivers/staging/media/atomisp/pci/isp/kernels/s3a/s3a_1.0/ia_css_s3a.host.c b/drivers/staging/media/atomisp/pci/isp/kernels/s3a/s3a_1.0/ia_css_s3a.host.c
index ba52c80df4a5..bd7b89d9475b 100644
--- a/drivers/staging/media/atomisp/pci/isp/kernels/s3a/s3a_1.0/ia_css_s3a.host.c
+++ b/drivers/staging/media/atomisp/pci/isp/kernels/s3a/s3a_1.0/ia_css_s3a.host.c
@@ -227,10 +227,6 @@ ia_css_s3a_hmem_decode(
     struct ia_css_3a_statistics *host_stats,
     const struct ia_css_bh_table *hmem_buf)
 {
-#if defined(HAS_NO_HMEM)
-	(void)host_stats;
-	(void)hmem_buf;
-#else
 	struct ia_css_3a_rgby_output	*out_ptr;
 	int			i;
 
@@ -291,7 +287,6 @@ ia_css_s3a_hmem_decode(
 	out_ptr[0].g -= diff;
 	out_ptr[0].b -= diff;
 	out_ptr[0].y -= diff;
-#endif
 }
 
 void
diff --git a/drivers/staging/media/atomisp/pci/runtime/binary/src/binary.c b/drivers/staging/media/atomisp/pci/runtime/binary/src/binary.c
index 851e1d34a521..bbb93cb8c070 100644
--- a/drivers/staging/media/atomisp/pci/runtime/binary/src/binary.c
+++ b/drivers/staging/media/atomisp/pci/runtime/binary/src/binary.c
@@ -805,11 +805,7 @@ ia_css_binary_3a_grid_info(const struct ia_css_binary *binary,
 	s3a_info->deci_factor_log2  = binary->deci_factor_log2;
 	s3a_info->elem_bit_depth    = SH_CSS_BAYER_BITS;
 	s3a_info->use_dmem          = binary->info->sp.s3a.s3atbl_use_dmem;
-#if defined(HAS_NO_HMEM)
-	s3a_info->has_histogram     = 1;
-#else
 	s3a_info->has_histogram     = 0;
-#endif
 	IA_CSS_LEAVE_ERR_PRIVATE(err);
 	return err;
 }
diff --git a/drivers/staging/media/atomisp/pci/sh_css_params.c b/drivers/staging/media/atomisp/pci/sh_css_params.c
index f3f8d99c84a4..41ed8e4600ff 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_params.c
+++ b/drivers/staging/media/atomisp/pci/sh_css_params.c
@@ -16,12 +16,10 @@
 #include "gdc_device.h"		/* gdc_lut_store(), ... */
 #include "isp.h"			/* ISP_VEC_ELEMBITS */
 #include "vamem.h"
-#if !defined(HAS_NO_HMEM)
 #ifndef __INLINE_HMEM__
 #define __INLINE_HMEM__
 #endif
 #include "hmem.h"
-#endif /* !defined(HAS_NO_HMEM) */
 #define IA_CSS_INCLUDE_PARAMETERS
 #define IA_CSS_INCLUDE_ACC_PARAMETERS
 
@@ -1489,10 +1487,8 @@ ia_css_translate_3a_statistics(
 		ia_css_s3a_vmem_decode(host_stats, isp_stats->vmem_stats_hi,
 				       isp_stats->vmem_stats_lo);
 	}
-#if !defined(HAS_NO_HMEM)
 	IA_CSS_LOG("3A: HMEM");
 	ia_css_s3a_hmem_decode(host_stats, isp_stats->hmem_stats);
-#endif
 
 	IA_CSS_LEAVE("void");
 }
@@ -2157,9 +2153,7 @@ ia_css_isp_3a_statistics_allocate(const struct ia_css_3a_grid_info *grid)
 		me->vmem_size = ISP_S3ATBL_HI_LO_STRIDE_BYTES *
 				grid->aligned_height;
 	}
-#if !defined(HAS_NO_HMEM)
 	me->hmem_size = sizeof_hmem(HMEM0_ID);
-#endif
 
 	/* All subsections need to be aligned to the system bus width */
 	me->dmem_size = CEIL_MUL(me->dmem_size, HIVE_ISP_DDR_WORD_BYTES);
@@ -4148,12 +4142,8 @@ ia_css_3a_statistics_allocate(const struct ia_css_3a_grid_info *grid)
 	me->data = kvmalloc(grid_size * sizeof(*me->data), GFP_KERNEL);
 	if (!me->data)
 		goto err;
-#if !defined(HAS_NO_HMEM)
 	/* No weighted histogram, no structure, treat the histogram data as a byte dump in a byte array */
 	me->rgby_data = kvmalloc(sizeof_hmem(HMEM0_ID), GFP_KERNEL);
-#else
-	me->rgby_data = NULL;
-#endif
 
 	IA_CSS_LEAVE("return=%p", me);
 	return me;
-- 
2.33.1

[PATCH 08/13] media: atomisp: get rid of USE_WINDOWS_BINNING_FACTOR tests

[Mauro Carvalho Chehab]

This is meant to select a platform-dependent factor between
Linux and Windows. It makes no sense to keep it on Kernel.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---

To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH 00/13] at: https://lore.kernel.org/all/cover.1637140900.git.mchehab+huawei@kernel.org/

 .../media/atomisp/pci/ia_css_stream_public.h  | 10 ++++-----
 .../media/atomisp/pci/sh_css_param_shading.c  | 22 -------------------
 2 files changed, 4 insertions(+), 28 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/ia_css_stream_public.h b/drivers/staging/media/atomisp/pci/ia_css_stream_public.h
index 649f22b03de8..47846ece8d64 100644
--- a/drivers/staging/media/atomisp/pci/ia_css_stream_public.h
+++ b/drivers/staging/media/atomisp/pci/ia_css_stream_public.h
@@ -102,12 +102,10 @@ struct ia_css_stream_config {
 		isys_config[IA_CSS_STREAM_MAX_ISYS_STREAM_PER_CH];
 	struct ia_css_stream_input_config input_config;
 
-	/* Currently, Android and Windows platforms interpret the binning_factor parameter
-	 * differently. In Android, the binning factor is expressed in the form
-	 * 2^N * 2^N, whereas in Windows platform, the binning factor is N*N
-	 * To use the Windows method of specification, the caller has to define
-	 * macro USE_WINDOWS_BINNING_FACTOR. This is for backward compatibility only
-	 * and will be deprecated. In the future,all platforms will use the N*N method
+	/*
+	 * Currently, Linux and Windows platforms interpret the binning_factor
+	 * parameter differently. In Linux, the binning factor is expressed
+	 * in the form 2^N * 2^N
 	 */
 	/* ISP2401 */
 	unsigned int sensor_binning_factor; /** Binning factor used by sensor
diff --git a/drivers/staging/media/atomisp/pci/sh_css_param_shading.c b/drivers/staging/media/atomisp/pci/sh_css_param_shading.c
index 69cc4e423d8b..51d83ce0d9cb 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_param_shading.c
+++ b/drivers/staging/media/atomisp/pci/sh_css_param_shading.c
@@ -271,33 +271,11 @@ prepare_shading_table(const struct ia_css_shading_table *in_table,
 		      bds_denominator -
 		      binary->info->sp.pipeline.top_cropping;
 
-#if !defined(USE_WINDOWS_BINNING_FACTOR)
-	/* @deprecated{This part of the code will be replaced by the code
-	 * in the #else section below to make the calculation same across
-	 * all platforms.
-	 * Android and Windows platforms interpret the binning_factor parameter
-	 * differently. In Android, the binning factor is expressed in the form
-	 * 2^N * 2^N, whereas in Windows platform, the binning factor is N*N}
-	 */
-
-	/* We take into account the binning done by the sensor. We do this
-	   by cropping the non-binned part of the shading table and then
-	   increasing the size of a grid cell with this same binning factor. */
-	input_width  <<= sensor_binning;
-	input_height <<= sensor_binning;
-	/* We also scale the padding by the same binning factor. This will
-	   make it much easier later on to calculate the padding of the
-	   shading table. */
-	left_padding  <<= sensor_binning;
-	right_padding <<= sensor_binning;
-	top_padding   <<= sensor_binning;
-#else
 	input_width   *= sensor_binning;
 	input_height  *= sensor_binning;
 	left_padding  *= sensor_binning;
 	right_padding *= sensor_binning;
 	top_padding   *= sensor_binning;
-#endif /*USE_WINDOWS_BINNING_FACTOR*/
 
 	/* during simulation, the used resolution can exceed the sensor
 	   resolution, so we clip it. */
-- 
2.33.1

[PATCH 09/13] media: atomisp: get rid of #ifdef HAS_BL

[Mauro Carvalho Chehab]

This does nothing but declare extern to a non-existing var.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---

To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH 00/13] at: https://lore.kernel.org/all/cover.1637140900.git.mchehab+huawei@kernel.org/

 drivers/staging/media/atomisp/pci/sh_css_firmware.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/sh_css_firmware.h b/drivers/staging/media/atomisp/pci/sh_css_firmware.h
index 66cd38f08f71..a73ce703adfb 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_firmware.h
+++ b/drivers/staging/media/atomisp/pci/sh_css_firmware.h
@@ -29,9 +29,6 @@ struct  sh_css_fw_bi_file_h {
 };
 
 extern struct ia_css_fw_info     sh_css_sp_fw;
-#if defined(HAS_BL)
-extern struct ia_css_fw_info     sh_css_bl_fw;
-#endif /* HAS_BL */
 extern struct ia_css_blob_descr *sh_css_blob_info;
 extern unsigned int sh_css_num_binaries;
 
-- 
2.33.1

[PATCH 10/13] media: atomisp: get rid of sctbl_legacy_*

[Mauro Carvalho Chehab]

Those seem to be used only on certain ISP2401 firmwares that
aren't supported by the driver. So, get rid of them.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---

To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH 00/13] at: https://lore.kernel.org/all/cover.1637140900.git.mchehab+huawei@kernel.org/

 .../atomisp/pci/runtime/binary/interface/ia_css_binary.h  | 2 --
 .../staging/media/atomisp/pci/runtime/binary/src/binary.c | 6 ------
 drivers/staging/media/atomisp/pci/sh_css_param_shading.c  | 8 ++++----
 3 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/runtime/binary/interface/ia_css_binary.h b/drivers/staging/media/atomisp/pci/runtime/binary/interface/ia_css_binary.h
index 6f110d241836..9935ac860bc2 100644
--- a/drivers/staging/media/atomisp/pci/runtime/binary/interface/ia_css_binary.h
+++ b/drivers/staging/media/atomisp/pci/runtime/binary/interface/ia_css_binary.h
@@ -130,8 +130,6 @@ struct ia_css_binary {
 	int                      sctbl_width_per_color;
 	int                      sctbl_aligned_width_per_color;
 	int                      sctbl_height;
-	int                      sctbl_legacy_width_per_color;
-	int                      sctbl_legacy_height;
 	struct ia_css_sdis_info	 dis;
 	struct ia_css_resolution dvs_envelope;
 	bool                     online;
diff --git a/drivers/staging/media/atomisp/pci/runtime/binary/src/binary.c b/drivers/staging/media/atomisp/pci/runtime/binary/src/binary.c
index bbb93cb8c070..9af23fdb127b 100644
--- a/drivers/staging/media/atomisp/pci/runtime/binary/src/binary.c
+++ b/drivers/staging/media/atomisp/pci/runtime/binary/src/binary.c
@@ -1339,18 +1339,12 @@ ia_css_binary_fill_info(const struct ia_css_binary_xinfo *xinfo,
 			binary->sctbl_width_per_color = _ISP2401_SCTBL_WIDTH_PER_COLOR(isp_internal_width, s3a_log_deci);
 			binary->sctbl_aligned_width_per_color = SH_CSS_MAX_SCTBL_ALIGNED_WIDTH_PER_COLOR;
 			binary->sctbl_height = _ISP2401_SCTBL_HEIGHT(isp_internal_height, s3a_log_deci);
-			binary->sctbl_legacy_width_per_color  = _ISP_SCTBL_LEGACY_WIDTH_PER_COLOR(sc_3a_dis_padded_width, s3a_log_deci);
-			binary->sctbl_legacy_height = _ISP_SCTBL_LEGACY_HEIGHT(sc_3a_dis_height, s3a_log_deci);
 		}
 	} else
 	{
 		binary->sctbl_width_per_color         = 0;
 		binary->sctbl_aligned_width_per_color = 0;
 		binary->sctbl_height                  = 0;
-		if (IS_ISP2401) {
-			binary->sctbl_legacy_width_per_color  = 0;
-			binary->sctbl_legacy_height	      = 0;
-		}
 	}
 	ia_css_sdis_init_info(&binary->dis,
 			      sc_3a_dis_width,
diff --git a/drivers/staging/media/atomisp/pci/sh_css_param_shading.c b/drivers/staging/media/atomisp/pci/sh_css_param_shading.c
index 51d83ce0d9cb..02faf5069bb8 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_param_shading.c
+++ b/drivers/staging/media/atomisp/pci/sh_css_param_shading.c
@@ -242,8 +242,8 @@ prepare_shading_table(const struct ia_css_shading_table *in_table,
 
 	if (!in_table) {
 		sh_css_params_shading_id_table_generate(target_table,
-							binary->sctbl_legacy_width_per_color,
-							binary->sctbl_legacy_height);
+							binary->sctbl_width_per_color,
+							binary->sctbl_height);
 		return;
 	}
 
@@ -284,8 +284,8 @@ prepare_shading_table(const struct ia_css_shading_table *in_table,
 
 	/* This prepare_shading_table() function is called only in legacy API (not in new API).
 	   Then, the legacy shading table width and height should be used. */
-	table_width  = binary->sctbl_legacy_width_per_color;
-	table_height = binary->sctbl_legacy_height;
+	table_width  = binary->sctbl_width_per_color;
+	table_height = binary->sctbl_height;
 
 	result = ia_css_shading_table_alloc(table_width, table_height);
 	if (!result) {
-- 
2.33.1

[PATCH 11/13] media: atomisp: sh_css_param_shading: fix comments coding style

[Mauro Carvalho Chehab]

The comments are not following Kernel coding style.

Also, there are two missing comments that are found at the Intel Aero
device driver code. Add them.

No functional changes.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---

To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH 00/13] at: https://lore.kernel.org/all/cover.1637140900.git.mchehab+huawei@kernel.org/

 .../media/atomisp/pci/sh_css_param_shading.c  | 52 +++++++++++++------
 1 file changed, 37 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/sh_css_param_shading.c b/drivers/staging/media/atomisp/pci/sh_css_param_shading.c
index 02faf5069bb8..90b5f9014c5e 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_param_shading.c
+++ b/drivers/staging/media/atomisp/pci/sh_css_param_shading.c
@@ -118,8 +118,10 @@ crop_and_interpolate(unsigned int cropped_width,
 		int ty, src_y0, src_y1;
 		unsigned int sy0, sy1, dy0, dy1, divy;
 
-		/* calculate target point and make sure it falls within
-		   the table */
+		/*
+		 * calculate target point and make sure it falls within
+		 * the table
+		 */
 		ty = out_start_row + i * out_cell_size;
 
 		/* calculate closest source points in shading table and
@@ -166,19 +168,24 @@ crop_and_interpolate(unsigned int cropped_width,
 			src_x1 = clamp(src_x1, 0, (int)table_width - 1);
 			tx = min(clamp(tx, 0, (int)sensor_width - 1),
 				 (int)table_cell_w);
-			/* calculate closest source points for distance
-			   computation */
+			/*
+			 * calculate closest source points for distance
+			 * computation
+			 */
 			sx0 = min(src_x0 * in_cell_size, sensor_width - 1);
 			sx1 = min(src_x1 * in_cell_size, sensor_width - 1);
-			/* calculate distances between source and target
-			   pixels */
+			/*
+			 * calculate distances between source and target
+			 * pixels
+			 */
 			dx0 = tx - sx0;
 			dx1 = sx1 - tx;
 			divx = sx1 - sx0;
 			/* if we're at the edge, we just use the closest
-			   point still in the grid. We make up for the divider
-			   in this case by setting the distance to
-			   out_cell_size, since it's actually 0. */
+			 * point still in the grid. We make up for the divider
+			 * in this case by setting the distance to
+			 * out_cell_size, since it's actually 0.
+			 */
 			if (divx == 0) {
 				dx0 = 1;
 				divx = 1;
@@ -271,14 +278,26 @@ prepare_shading_table(const struct ia_css_shading_table *in_table,
 		      bds_denominator -
 		      binary->info->sp.pipeline.top_cropping;
 
+	/*
+	 * We take into account the binning done by the sensor. We do this
+	 * by cropping the non-binned part of the shading table and then
+	 * increasing the size of a grid cell with this same binning factor.
+	 */
 	input_width   *= sensor_binning;
 	input_height  *= sensor_binning;
+	/*
+	 * We also scale the padding by the same binning factor. This will
+	 * make it much easier later on to calculate the padding of the
+	 * shading table.
+	 */
 	left_padding  *= sensor_binning;
 	right_padding *= sensor_binning;
 	top_padding   *= sensor_binning;
 
-	/* during simulation, the used resolution can exceed the sensor
-	   resolution, so we clip it. */
+	/*
+	 * during simulation, the used resolution can exceed the sensor
+	 * resolution, so we clip it.
+	 */
 	input_width  = min(input_width,  in_table->sensor_width);
 	input_height = min(input_height, in_table->sensor_height);
 
@@ -296,8 +315,10 @@ prepare_shading_table(const struct ia_css_shading_table *in_table,
 	result->sensor_height = in_table->sensor_height;
 	result->fraction_bits = in_table->fraction_bits;
 
-	/* now we crop the original shading table and then interpolate to the
-	   requested resolution and decimation factor. */
+	/*
+	 * now we crop the original shading table and then interpolate to the
+	 * requested resolution and decimation factor.
+	 */
 	for (i = 0; i < IA_CSS_SC_NUM_COLORS; i++) {
 		crop_and_interpolate(input_width, input_height,
 				     left_padding, right_padding, top_padding,
@@ -354,9 +375,10 @@ ia_css_shading_table_free(struct ia_css_shading_table *table)
 	if (!table)
 		return;
 
-	/* We only output logging when the table is not NULL, otherwise
+	/*
+	 * We only output logging when the table is not NULL, otherwise
 	 * logs will give the impression that a table was freed.
-	 * */
+	 */
 	IA_CSS_ENTER("");
 
 	for (i = 0; i < IA_CSS_SC_NUM_COLORS; i++) {
-- 
2.33.1

[PATCH 12/13] media: atomisp: fix a bug when applying the binning factor

[Mauro Carvalho Chehab]

The binning factor is actually a shift value, where 0 means
to not touch the value. This is propagated from the sensors'
properties:

	.bin_factor_x
	.bin_factor_y

At their resolution setting tables.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---

To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH 00/13] at: https://lore.kernel.org/all/cover.1637140900.git.mchehab+huawei@kernel.org/

 .../staging/media/atomisp/pci/sh_css_param_shading.c   | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/sh_css_param_shading.c b/drivers/staging/media/atomisp/pci/sh_css_param_shading.c
index 90b5f9014c5e..005b0ab41588 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_param_shading.c
+++ b/drivers/staging/media/atomisp/pci/sh_css_param_shading.c
@@ -283,16 +283,16 @@ prepare_shading_table(const struct ia_css_shading_table *in_table,
 	 * by cropping the non-binned part of the shading table and then
 	 * increasing the size of a grid cell with this same binning factor.
 	 */
-	input_width   *= sensor_binning;
-	input_height  *= sensor_binning;
+	input_width   <<= sensor_binning;
+	input_height  <<= sensor_binning;
 	/*
 	 * We also scale the padding by the same binning factor. This will
 	 * make it much easier later on to calculate the padding of the
 	 * shading table.
 	 */
-	left_padding  *= sensor_binning;
-	right_padding *= sensor_binning;
-	top_padding   *= sensor_binning;
+	left_padding  <<= sensor_binning;
+	right_padding <<= sensor_binning;
+	top_padding   <<= sensor_binning;
 
 	/*
 	 * during simulation, the used resolution can exceed the sensor
-- 
2.33.1

Re: [PATCH 12/13] media: atomisp: fix a bug when applying the binning factor

[Mauro Carvalho Chehab]

Em Wed, 17 Nov 2021 09:25:37 +0000
Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:

> The binning factor is actually a shift value, where 0 means
> to not touch the value. This is propagated from the sensors'
> properties:
> 
> 	.bin_factor_x
> 	.bin_factor_y
> 
> At their resolution setting tables.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

It turns that this actually not a bug. It is just that I reverted
the logic on patch 08/13.

So, I'll merge this one there and re-send this series.


> ---
> 
> To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
> See [PATCH 00/13] at: https://lore.kernel.org/all/cover.1637140900.git.mchehab+huawei@kernel.org/
> 
>  .../staging/media/atomisp/pci/sh_css_param_shading.c   | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/media/atomisp/pci/sh_css_param_shading.c b/drivers/staging/media/atomisp/pci/sh_css_param_shading.c
> index 90b5f9014c5e..005b0ab41588 100644
> --- a/drivers/staging/media/atomisp/pci/sh_css_param_shading.c
> +++ b/drivers/staging/media/atomisp/pci/sh_css_param_shading.c
> @@ -283,16 +283,16 @@ prepare_shading_table(const struct ia_css_shading_table *in_table,
>  	 * by cropping the non-binned part of the shading table and then
>  	 * increasing the size of a grid cell with this same binning factor.
>  	 */
> -	input_width   *= sensor_binning;
> -	input_height  *= sensor_binning;
> +	input_width   <<= sensor_binning;
> +	input_height  <<= sensor_binning;
>  	/*
>  	 * We also scale the padding by the same binning factor. This will
>  	 * make it much easier later on to calculate the padding of the
>  	 * shading table.
>  	 */
> -	left_padding  *= sensor_binning;
> -	right_padding *= sensor_binning;
> -	top_padding   *= sensor_binning;
> +	left_padding  <<= sensor_binning;
> +	right_padding <<= sensor_binning;
> +	top_padding   <<= sensor_binning;
>  
>  	/*
>  	 * during simulation, the used resolution can exceed the sensor

[PATCH 13/13] media: atomisp: sh_css_sp: better support the current firmware

[Mauro Carvalho Chehab]

As we're using Intel Aero firmware, make the code closer to the
driver for such device.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---

To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH 00/13] at: https://lore.kernel.org/all/cover.1637140900.git.mchehab+huawei@kernel.org/

 drivers/staging/media/atomisp/pci/sh_css_sp.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/sh_css_sp.c b/drivers/staging/media/atomisp/pci/sh_css_sp.c
index f6db1f4a0e1d..a11078acb072 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_sp.c
+++ b/drivers/staging/media/atomisp/pci/sh_css_sp.c
@@ -982,6 +982,7 @@ sh_css_sp_init_stage(struct ia_css_binary *binary,
 	/* Make sure binary name is smaller than allowed string size */
 	assert(strlen(binary_name) < SH_CSS_MAX_BINARY_NAME - 1);
 	strscpy(sh_css_isp_stage.binary_name, binary_name, SH_CSS_MAX_BINARY_NAME);
+	sh_css_isp_stage.binary_name[SH_CSS_MAX_BINARY_NAME - 1] = 0;
 	sh_css_isp_stage.mem_initializers = *isp_mem_if;
 
 	/*
@@ -1013,16 +1014,14 @@ sh_css_sp_init_stage(struct ia_css_binary *binary,
 		return err;
 
 #ifdef ISP2401
-	if (stage == 0) {
-		pipe = find_pipe_by_num(sh_css_sp_group.pipe[thread_id].pipe_num);
-		if (!pipe)
-			return -EINVAL;
+	pipe = find_pipe_by_num(sh_css_sp_group.pipe[thread_id].pipe_num);
+	if (!pipe)
+		return -EINVAL;
 
-		if (args->in_frame)
-			ia_css_get_crop_offsets(pipe, &args->in_frame->info);
-		else
-			ia_css_get_crop_offsets(pipe, &binary->in_frame_info);
-	}
+	if (args->in_frame)
+		ia_css_get_crop_offsets(pipe, &args->in_frame->info);
+	else if (&binary->in_frame_info)
+		ia_css_get_crop_offsets(pipe, &binary->in_frame_info);
 #else
 	(void)pipe; /*avoid build warning*/
 #endif
-- 
2.33.1

Re: [PATCH 13/13] media: atomisp: sh_css_sp: better support the current firmware

[Dan Carpenter]

On Wed, Nov 17, 2021 at 09:25:38AM +0000, Mauro Carvalho Chehab wrote:
> As we're using Intel Aero firmware, make the code closer to the
> driver for such device.

I don't really understand this commit message.  Is there a out of tree
Intel driver?

> diff --git a/drivers/staging/media/atomisp/pci/sh_css_sp.c b/drivers/staging/media/atomisp/pci/sh_css_sp.c
> index f6db1f4a0e1d..a11078acb072 100644
> --- a/drivers/staging/media/atomisp/pci/sh_css_sp.c
> +++ b/drivers/staging/media/atomisp/pci/sh_css_sp.c
> @@ -982,6 +982,7 @@ sh_css_sp_init_stage(struct ia_css_binary *binary,
>  	/* Make sure binary name is smaller than allowed string size */
>  	assert(strlen(binary_name) < SH_CSS_MAX_BINARY_NAME - 1);
>  	strscpy(sh_css_isp_stage.binary_name, binary_name, SH_CSS_MAX_BINARY_NAME);
> +	sh_css_isp_stage.binary_name[SH_CSS_MAX_BINARY_NAME - 1] = 0;

No, need for this.  strscpy() already guarantees that the result is NULL
terminated.  That's one of the main reasons Linus invented it instead of
using strncpy().

>  #ifdef ISP2401
> -	if (stage == 0) {
> -		pipe = find_pipe_by_num(sh_css_sp_group.pipe[thread_id].pipe_num);
> -		if (!pipe)
> -			return -EINVAL;
> +	pipe = find_pipe_by_num(sh_css_sp_group.pipe[thread_id].pipe_num);
> +	if (!pipe)
> +		return -EINVAL;
>  
> -		if (args->in_frame)
> -			ia_css_get_crop_offsets(pipe, &args->in_frame->info);
> -		else
> -			ia_css_get_crop_offsets(pipe, &binary->in_frame_info);
> -	}
> +	if (args->in_frame)
> +		ia_css_get_crop_offsets(pipe, &args->in_frame->info);
> +	else if (&binary->in_frame_info)
                  ^^^^^^^^^^^^^^^^^^^^^

This condition is wrong.  This is the address of something in the middle
of "binary" so it can't be NULL.

> +		ia_css_get_crop_offsets(pipe, &binary->in_frame_info);
>  #else
>  	(void)pipe; /*avoid build warning*/
>  #endif

regards,
dan carpenter

Re: [PATCH 13/13] media: atomisp: sh_css_sp: better support the current firmware

[Mauro Carvalho Chehab]

Em Wed, 17 Nov 2021 13:02:38 +0300
Dan Carpenter <dan.carpenter@oracle.com> escreveu:

> On Wed, Nov 17, 2021 at 09:25:38AM +0000, Mauro Carvalho Chehab wrote:
> > As we're using Intel Aero firmware, make the code closer to the
> > driver for such device.  
> 
> I don't really understand this commit message.  Is there a out of tree
> Intel driver?

Yes. It is at:

	https://github.com/intel-aero/meta-intel-aero-base/tree/master/recipes-kernel/linux/linux-yocto

I guess I was too lazy to place the above on almost all patches in this 
series. There are some past patches for atomisp that already contains
such info.

The atomisp's TODO mentions it:


	- for CHT: /lib/firmware/shisp_2401a0_v21.bin

	  https://github.com/intel-aero/meta-intel-aero-base/blob/master/recipes-kernel/linux/linux-yocto/shisp_2401a0_v21.bin

-

Long answer:

Basically, as far as I'm aware, the original atomisp driver is produced by
some code generator, which not only dynamically add code to the driver's
source code output, but also sets several #ifdefs and dynamically change the
firmware API depending on the Atom CPU, features and on other options.

If you take a look at the original commit which added this driver:

	a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2")

It basically had some efforts from Alan to support both ISP2400 (BYT) and
ISP2401 (CHT) versions at the same code.

If you look on its TODO, it says:

	"The ISP code depends on the exact FW version. The version defined in
	   BYT: 
	   drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c
	   static const char *release_version = STR(irci_stable_candrpv_0415_20150521_0458);
	   CHT:
	   drivers/staging/media/atomisp/pci/atomisp2/css/sh_css_firmware.c
	   static const char *release_version = STR(irci_ecr-master_20150911_0724);"

So, the versions for BYT and CHT at the original patch were different, and
there are lots of #ifdefs to test between such differences.

There's another problem, through. The irci_ecr-master_20150911_0724 firmware
was never placed at linux-firmware, and got lost in time. I don't know
anyone whose has such exact version. It seems it got lost in time, no idea
why.

The only non-android Linux driver we know is for the Intel Aero hardware,
which is Cherrytail (CHT). For such hardware, the firmware is also provided
at the Intel Aero Yocto repositories, at:

	https://github.com/intel-aero/meta-intel-aero-base/blob/master/recipes-kernel/linux/linux-yocto/shisp_2401a0_v21.bin

The version of this firmware is:

	$ strings /lib/firmware/shisp_2401a0_v21.bin |grep irci_stable_
	irci_stable_candrpv_0415_20150521_0458

As we have both drivers and firmware for such version, we're actually
adjusting the upstream code to match this version for ISP2401 (CHT).

There's a side effect with this change: the code for ISP2400 and ISP2401
will be both based on the same version, which helps to reduce the
differences at the driver level - that's assuming that we can:

	- find a publicly-available ISP2400 
	  irci_stable_candrpv_0415_20150521_0458 binary;
	- have some BYT hardware for testing with upstream drivers[1];
	- have an Android driver for the same fix, in order to help
	  double-checking if the code is compatible with the firmware.

[1] On some BYT BIOS, the normal ACPI method of detecting the hardware
doesn't work, as the ISP is visible though the GPU. That's the case
of a commonly found hardware (Asus T100 with the production BIOS).

> 
> > diff --git a/drivers/staging/media/atomisp/pci/sh_css_sp.c b/drivers/staging/media/atomisp/pci/sh_css_sp.c
> > index f6db1f4a0e1d..a11078acb072 100644
> > --- a/drivers/staging/media/atomisp/pci/sh_css_sp.c
> > +++ b/drivers/staging/media/atomisp/pci/sh_css_sp.c
> > @@ -982,6 +982,7 @@ sh_css_sp_init_stage(struct ia_css_binary *binary,
> >  	/* Make sure binary name is smaller than allowed string size */
> >  	assert(strlen(binary_name) < SH_CSS_MAX_BINARY_NAME - 1);
> >  	strscpy(sh_css_isp_stage.binary_name, binary_name, SH_CSS_MAX_BINARY_NAME);
> > +	sh_css_isp_stage.binary_name[SH_CSS_MAX_BINARY_NAME - 1] = 0;  
> 
> No, need for this.  strscpy() already guarantees that the result is NULL
> terminated.  That's one of the main reasons Linus invented it instead of
> using strncpy().
> 
> >  #ifdef ISP2401
> > -	if (stage == 0) {
> > -		pipe = find_pipe_by_num(sh_css_sp_group.pipe[thread_id].pipe_num);
> > -		if (!pipe)
> > -			return -EINVAL;
> > +	pipe = find_pipe_by_num(sh_css_sp_group.pipe[thread_id].pipe_num);
> > +	if (!pipe)
> > +		return -EINVAL;
> >  
> > -		if (args->in_frame)
> > -			ia_css_get_crop_offsets(pipe, &args->in_frame->info);
> > -		else
> > -			ia_css_get_crop_offsets(pipe, &binary->in_frame_info);
> > -	}
> > +	if (args->in_frame)
> > +		ia_css_get_crop_offsets(pipe, &args->in_frame->info);
> > +	else if (&binary->in_frame_info)  
>                   ^^^^^^^^^^^^^^^^^^^^^
> 
> This condition is wrong.  This is the address of something in the middle
> of "binary" so it can't be NULL.
> 
> > +		ia_css_get_crop_offsets(pipe, &binary->in_frame_info);
> >  #else
> >  	(void)pipe; /*avoid build warning*/
> >  #endif  
> 
> regards,
> dan carpenter

Re: [PATCH 13/13] media: atomisp: sh_css_sp: better support the current firmware

[Dan Carpenter]

On Wed, Nov 17, 2021 at 11:19:56AM +0000, Mauro Carvalho Chehab wrote:
> Em Wed, 17 Nov 2021 13:02:38 +0300
> Dan Carpenter <dan.carpenter@oracle.com> escreveu:
> 
> > On Wed, Nov 17, 2021 at 09:25:38AM +0000, Mauro Carvalho Chehab wrote:
> > > As we're using Intel Aero firmware, make the code closer to the
> > > driver for such device.  
> > 
> > I don't really understand this commit message.  Is there a out of tree
> > Intel driver?
> 
> Yes. It is at:
> 
> 	https://github.com/intel-aero/meta-intel-aero-base/tree/master/recipes-kernel/linux/linux-yocto
> 

Fine, but only removing the if (stage == 0) { is correct.  The other two
changes are harmless but wrong.

regards,
dan carpenter