All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/5] staging: media: atomisp: sh_css: Remove #ifdef ISP2401
@ 2023-05-08  6:26 Kate Hsuan
  2023-05-08  6:26 ` [PATCH v2 2/5] staging: media: atomisp: runtime: frame: remove " Kate Hsuan
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Kate Hsuan @ 2023-05-08  6:26 UTC (permalink / raw)
  To: Hans de Goede, Mauro Carvalho Chehab, Sakari Ailus,
	Greg Kroah-Hartman, linux-media, linux-staging
  Cc: Kate Hsuan

The actions of ISP2401 and 2400 are determined at the runtime.

Signed-off-by: Kate Hsuan <hpa@redhat.com>
---
 drivers/staging/media/atomisp/pci/sh_css.c | 524 ++++++++++-----------
 1 file changed, 239 insertions(+), 285 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c
index 93789500416f..4b3fa6d93fe0 100644
--- a/drivers/staging/media/atomisp/pci/sh_css.c
+++ b/drivers/staging/media/atomisp/pci/sh_css.c
@@ -1529,15 +1529,14 @@ ia_css_init(struct device *dev, const struct ia_css_env *env,
 
 	mipi_init();
 
-#ifndef ISP2401
 	/*
 	 * In case this has been programmed already, update internal
 	 * data structure ...
 	 * DEPRECATED
 	 */
-	my_css.page_table_base_index = mmu_get_page_table_base_index(MMU0_ID);
+	if (!IS_ISP2401)
+		my_css.page_table_base_index = mmu_get_page_table_base_index(MMU0_ID);
 
-#endif
 	my_css.irq_type = irq_type;
 
 	my_css_save.irq_type = irq_type;
@@ -1596,10 +1595,8 @@ ia_css_init(struct device *dev, const struct ia_css_env *env,
 	 * sh_css_init_buffer_queues();
 	 */
 
-#if defined(ISP2401)
-	gp_device_reg_store(GP_DEVICE0_ID, _REG_GP_SWITCH_ISYS2401_ADDR, 1);
-#endif
-
+	if (IS_ISP2401)
+		gp_device_reg_store(GP_DEVICE0_ID, _REG_GP_SWITCH_ISYS2401_ADDR, 1);
 
 	if (!IS_ISP2401)
 		dma_set_max_burst_size(DMA0_ID, HIVE_DMA_BUS_DDR_CONN,
@@ -2128,13 +2125,8 @@ ia_css_pipe_destroy(struct ia_css_pipe *pipe)
 						    err);
 			}
 		}
-#ifndef ISP2401
 		ia_css_frame_free_multiple(NUM_VIDEO_TNR_FRAMES,
 					   pipe->pipe_settings.video.tnr_frames);
-#else
-		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,
 					   pipe->pipe_settings.video.delay_frames);
 		break;
@@ -2238,11 +2230,10 @@ int ia_css_irq_translate(
 		case virq_isys_csi:
 			infos |= IA_CSS_IRQ_INFO_INPUT_SYSTEM_ERROR;
 			break;
-#if !defined(ISP2401)
 		case virq_ifmt0_id:
-			infos |= IA_CSS_IRQ_INFO_IF_ERROR;
+			if (!IS_ISP2401)
+				infos |= IA_CSS_IRQ_INFO_IF_ERROR;
 			break;
-#endif
 		case virq_dma:
 			infos |= IA_CSS_IRQ_INFO_DMA_ERROR;
 			break;
@@ -2277,27 +2268,34 @@ int ia_css_irq_enable(
 	IA_CSS_ENTER("info=%d, enable=%d", info, enable);
 
 	switch (info) {
-#if !defined(ISP2401)
 	case IA_CSS_IRQ_INFO_CSS_RECEIVER_SOF:
+		if (IS_ISP2401)
+			/* Just ignore those unused IRQs without printing errors */
+			return 0;
+
 		irq = virq_isys_sof;
 		break;
 	case IA_CSS_IRQ_INFO_CSS_RECEIVER_EOF:
+		if (IS_ISP2401)
+			/* Just ignore those unused IRQs without printing errors */
+			return 0;
+
 		irq = virq_isys_eof;
 		break;
 	case IA_CSS_IRQ_INFO_INPUT_SYSTEM_ERROR:
+		if (IS_ISP2401)
+			/* Just ignore those unused IRQs without printing errors */
+			return 0;
+
 		irq = virq_isys_csi;
 		break;
 	case IA_CSS_IRQ_INFO_IF_ERROR:
+		if (IS_ISP2401)
+			/* Just ignore those unused IRQs without printing errors */
+			return 0;
+
 		irq = virq_ifmt0_id;
 		break;
-#else
-	case IA_CSS_IRQ_INFO_CSS_RECEIVER_SOF:
-	case IA_CSS_IRQ_INFO_CSS_RECEIVER_EOF:
-	case IA_CSS_IRQ_INFO_INPUT_SYSTEM_ERROR:
-	case IA_CSS_IRQ_INFO_IF_ERROR:
-		/* Just ignore those unused IRQs without printing errors */
-		return 0;
-#endif
 	case IA_CSS_IRQ_INFO_DMA_ERROR:
 		irq = virq_dma;
 		break;
@@ -2413,14 +2411,14 @@ alloc_continuous_frames(struct ia_css_pipe *pipe, bool init_time)
 		return -EINVAL;
 	}
 
-#if defined(ISP2401)
-	/* For CSI2+, the continuous frame will hold the full input frame */
-	ref_info.res.width = pipe->stream->config.input_config.input_res.width;
-	ref_info.res.height = pipe->stream->config.input_config.input_res.height;
+	if (IS_ISP2401) {
+		/* For CSI2+, the continuous frame will hold the full input frame */
+		ref_info.res.width = pipe->stream->config.input_config.input_res.width;
+		ref_info.res.height = pipe->stream->config.input_config.input_res.height;
 
-	/* Ensure padded width is aligned for 2401 */
-	ref_info.padded_width = CEIL_MUL(ref_info.res.width, 2 * ISP_VEC_NELEMS);
-#endif
+		/* Ensure padded width is aligned for 2401 */
+		ref_info.padded_width = CEIL_MUL(ref_info.res.width, 2 * ISP_VEC_NELEMS);
+	}
 
 	if (pipe->stream->config.pack_raw_pixels) {
 		ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE_PRIVATE,
@@ -2499,11 +2497,9 @@ load_preview_binaries(struct ia_css_pipe *pipe)
 	int err = 0;
 	bool need_vf_pp = false;
 	bool need_isp_copy_binary = false;
-#ifdef ISP2401
 	bool sensor = false;
-#else
 	bool continuous;
-#endif
+
 	/* preview only have 1 output pin now */
 	struct ia_css_frame_info *pipe_out_info = &pipe->output_info[0];
 	struct ia_css_preview_settings *mycs  = &pipe->pipe_settings.preview;
@@ -2514,11 +2510,9 @@ load_preview_binaries(struct ia_css_pipe *pipe)
 	assert(pipe->mode == IA_CSS_PIPE_ID_PREVIEW);
 
 	online = pipe->stream->config.online;
-#ifdef ISP2401
+
 	sensor = pipe->stream->config.mode == IA_CSS_INPUT_MODE_SENSOR;
-#else
 	continuous = pipe->stream->config.continuous;
-#endif
 
 	if (mycs->preview_binary.info)
 		return 0;
@@ -2627,24 +2621,22 @@ load_preview_binaries(struct ia_css_pipe *pipe)
 			return err;
 	}
 
-#ifdef ISP2401
-	/*
-	 * When the input system is 2401, only the Direct Sensor Mode
-	 * Offline Preview uses the ISP copy binary.
-	 */
-	need_isp_copy_binary = !online && sensor;
-#else
-	/*
-	 * About pipe->stream->config.mode == IA_CSS_INPUT_MODE_MEMORY:
-	 * This is typical the case with SkyCam (which has no input system) but it also applies to all cases
-	 * where the driver chooses for memory based input frames. In these cases, a copy binary (which typical
-	 * copies sensor data to DDR) does not have much use.
-	 */
-	if (!IS_ISP2401)
+	if (IS_ISP2401) {
+		/*
+		 * When the input system is 2401, only the Direct Sensor Mode
+		 * Offline Preview uses the ISP copy binary.
+		 */
+		need_isp_copy_binary = !online && sensor;
+	} else {
+		/*
+		 * About pipe->stream->config.mode == IA_CSS_INPUT_MODE_MEMORY:
+		 * This is typical the case with SkyCam (which has no input system) but it also
+		 * applies to all cases where the driver chooses for memory based input frames.
+		 * In these cases, a copy binary (which typical copies sensor data to DDR) does
+		 * not have much use.
+		 */
 		need_isp_copy_binary = !online && !continuous;
-	else
-		need_isp_copy_binary = !online && !continuous && !(pipe->stream->config.mode == IA_CSS_INPUT_MODE_MEMORY);
-#endif
+	}
 
 	/* Copy */
 	if (need_isp_copy_binary) {
@@ -3125,11 +3117,10 @@ init_in_frameinfo_memory_defaults(struct ia_css_pipe *pipe,
 
 	in_frame->frame_info.format = format;
 
-#ifdef ISP2401
-	if (format == IA_CSS_FRAME_FORMAT_RAW)
+	if (IS_ISP2401 && format == IA_CSS_FRAME_FORMAT_RAW) {
 		in_frame->frame_info.format = (pipe->stream->config.pack_raw_pixels) ?
 		IA_CSS_FRAME_FORMAT_RAW_PACKED : IA_CSS_FRAME_FORMAT_RAW;
-#endif
+	}
 
 	in_frame->frame_info.res.width = pipe->stream->config.input_config.input_res.width;
 	in_frame->frame_info.res.height = pipe->stream->config.input_config.input_res.height;
@@ -3211,18 +3202,18 @@ static int create_host_video_pipeline(struct ia_css_pipe *pipe)
 
 	me->dvs_frame_delay = pipe->dvs_frame_delay;
 
-#ifdef ISP2401
-	/*
-	 * When the input system is 2401, always enable 'in_frameinfo_memory'
-	 * except for the following: online or continuous
-	 */
-	need_in_frameinfo_memory = !(pipe->stream->config.online ||
-				     pipe->stream->config.continuous);
-#else
-	/* Construct in_frame info (only in case we have dynamic input */
-	need_in_frameinfo_memory = pipe->stream->config.mode ==
-				   IA_CSS_INPUT_MODE_MEMORY;
-#endif
+	if (IS_ISP2401) {
+		/*
+		 * When the input system is 2401, always enable 'in_frameinfo_memory'
+		 * except for the following: online or continuous
+		 */
+		need_in_frameinfo_memory = !(pipe->stream->config.online ||
+					     pipe->stream->config.continuous);
+	} else {
+		/* Construct in_frame info (only in case we have dynamic input */
+		need_in_frameinfo_memory = pipe->stream->config.mode ==
+					   IA_CSS_INPUT_MODE_MEMORY;
+	}
 
 	/* Construct in_frame info (only in case we have dynamic input */
 	if (need_in_frameinfo_memory) {
@@ -3268,15 +3259,14 @@ static int create_host_video_pipeline(struct ia_css_pipe *pipe)
 			goto ERR;
 		in_frame = me->stages->args.out_frame[0];
 	} else if (pipe->stream->config.continuous) {
-#ifdef ISP2401
-		/*
-		 * When continuous is enabled, configure in_frame with the
-		 * last pipe, which is the copy pipe.
-		 */
-		in_frame = pipe->stream->last_pipe->continuous_frames[0];
-#else
-		in_frame = pipe->continuous_frames[0];
-#endif
+		if (IS_ISP2401)
+			/*
+			 * When continuous is enabled, configure in_frame with the
+			 * last pipe, which is the copy pipe.
+			 */
+			in_frame = pipe->stream->last_pipe->continuous_frames[0];
+		else
+			in_frame = pipe->continuous_frames[0];
 	}
 
 	ia_css_pipe_util_set_output_frames(out_frames, 0,
@@ -3373,12 +3363,10 @@ create_host_preview_pipeline(struct ia_css_pipe *pipe)
 	struct ia_css_frame *out_frame;
 	struct ia_css_frame *out_frames[IA_CSS_BINARY_MAX_OUTPUT_PORTS];
 	bool need_in_frameinfo_memory = false;
-#ifdef ISP2401
 	bool sensor = false;
 	bool buffered_sensor = false;
 	bool online = false;
 	bool continuous = false;
-#endif
 
 	IA_CSS_ENTER_PRIVATE("pipe = %p", pipe);
 	if ((!pipe) || (!pipe->stream) || (pipe->mode != IA_CSS_PIPE_ID_PREVIEW)) {
@@ -3391,25 +3379,26 @@ create_host_preview_pipeline(struct ia_css_pipe *pipe)
 	me = &pipe->pipeline;
 	ia_css_pipeline_clean(me);
 
-#ifdef ISP2401
-	/*
-	 * When the input system is 2401, always enable 'in_frameinfo_memory'
-	 * except for the following:
-	 * - Direct Sensor Mode Online Preview
-	 * - Buffered Sensor Mode Online Preview
-	 * - Direct Sensor Mode Continuous Preview
-	 * - Buffered Sensor Mode Continuous Preview
-	 */
-	sensor = (pipe->stream->config.mode == IA_CSS_INPUT_MODE_SENSOR);
-	buffered_sensor = (pipe->stream->config.mode == IA_CSS_INPUT_MODE_BUFFERED_SENSOR);
-	online = pipe->stream->config.online;
-	continuous = pipe->stream->config.continuous;
-	need_in_frameinfo_memory =
-	!((sensor && (online || continuous)) || (buffered_sensor && (online || continuous)));
-#else
-	/* Construct in_frame info (only in case we have dynamic input */
-	need_in_frameinfo_memory = pipe->stream->config.mode == IA_CSS_INPUT_MODE_MEMORY;
-#endif
+	if (IS_ISP2401) {
+		/*
+		 * When the input system is 2401, always enable 'in_frameinfo_memory'
+		 * except for the following:
+		 * - Direct Sensor Mode Online Preview
+		 * - Buffered Sensor Mode Online Preview
+		 * - Direct Sensor Mode Continuous Preview
+		 * - Buffered Sensor Mode Continuous Preview
+		 */
+		sensor = (pipe->stream->config.mode == IA_CSS_INPUT_MODE_SENSOR);
+		buffered_sensor = (pipe->stream->config.mode == IA_CSS_INPUT_MODE_BUFFERED_SENSOR);
+		online = pipe->stream->config.online;
+		continuous = pipe->stream->config.continuous;
+		need_in_frameinfo_memory =
+		!((sensor && (online || continuous)) || (buffered_sensor &&
+							(online || continuous)));
+	} else {
+		/* Construct in_frame info (only in case we have dynamic input */
+		need_in_frameinfo_memory = pipe->stream->config.mode == IA_CSS_INPUT_MODE_MEMORY;
+	}
 	if (need_in_frameinfo_memory) {
 		err = init_in_frameinfo_memory_defaults(pipe, &me->in_frame,
 							IA_CSS_FRAME_FORMAT_RAW);
@@ -3420,7 +3409,6 @@ create_host_preview_pipeline(struct ia_css_pipe *pipe)
 	} else {
 		in_frame = NULL;
 	}
-
 	err = init_out_frameinfo_defaults(pipe, &me->out_frame[0], 0);
 	if (err)
 		goto ERR;
@@ -3441,17 +3429,16 @@ create_host_preview_pipeline(struct ia_css_pipe *pipe)
 			goto ERR;
 		in_frame = me->stages->args.out_frame[0];
 	} else if (pipe->stream->config.continuous) {
-#ifdef ISP2401
-		/*
-		 * When continuous is enabled, configure in_frame with the
-		 * last pipe, which is the copy pipe.
-		 */
-		if (continuous || !online)
-			in_frame = pipe->stream->last_pipe->continuous_frames[0];
-
-#else
-		in_frame = pipe->continuous_frames[0];
-#endif
+		if (IS_ISP2401) {
+			/*
+			 * When continuous is enabled, configure in_frame with the
+			 * last pipe, which is the copy pipe.
+			 */
+			if (continuous || !online)
+				in_frame = pipe->stream->last_pipe->continuous_frames[0];
+		} else {
+			in_frame = pipe->continuous_frames[0];
+		}
 	}
 
 	if (vf_pp_binary) {
@@ -3925,19 +3912,19 @@ ia_css_pipe_dequeue_buffer(struct ia_css_pipe *pipe,
 			case IA_CSS_BUFFER_TYPE_OUTPUT_FRAME:
 			case IA_CSS_BUFFER_TYPE_SEC_OUTPUT_FRAME:
 				if (pipe && pipe->stop_requested) {
-#if !defined(ISP2401)
-					/*
-					 * free mipi frames only for old input
-					 * system for 2401 it is done in
-					 * ia_css_stream_destroy call
-					 */
-					return_err = free_mipi_frames(pipe);
-					if (return_err) {
-						IA_CSS_LOG("free_mipi_frames() failed");
-						IA_CSS_LEAVE_ERR(return_err);
-						return return_err;
+					if (!IS_ISP2401) {
+						/*
+						 * free mipi frames only for old input
+						 * system for 2401 it is done in
+						 * ia_css_stream_destroy call
+						 */
+						return_err = free_mipi_frames(pipe);
+						if (return_err) {
+							IA_CSS_LOG("free_mipi_frames() failed");
+							IA_CSS_LEAVE_ERR(return_err);
+							return return_err;
+						}
 					}
-#endif
 					pipe->stop_requested = false;
 				}
 				fallthrough;
@@ -3959,12 +3946,11 @@ ia_css_pipe_dequeue_buffer(struct ia_css_pipe *pipe,
 					pipe->num_invalid_frames--;
 
 				if (frame->frame_info.format == IA_CSS_FRAME_FORMAT_BINARY_8) {
-#ifdef ISP2401
-					frame->planes.binary.size = frame->data_bytes;
-#else
-					frame->planes.binary.size =
-					    sh_css_sp_get_binary_copy_size();
-#endif
+					if (IS_ISP2401)
+						frame->planes.binary.size = frame->data_bytes;
+					else
+						frame->planes.binary.size =
+						    sh_css_sp_get_binary_copy_size();
 				}
 				if (buf_type == IA_CSS_BUFFER_TYPE_OUTPUT_FRAME) {
 					IA_CSS_LOG("pfp: dequeued OF %d with config id %d thread %d",
@@ -4880,22 +4866,20 @@ static int load_video_binaries(struct ia_css_pipe *pipe)
 			    pipe->num_invalid_frames, pipe->dvs_frame_delay);
 
 	/* pqiao TODO: temp hack for PO, should be removed after offline YUVPP is enabled */
-#if !defined(ISP2401)
-	/* Copy */
-	if (!online && !continuous) {
-		/*
-		 * TODO: what exactly needs doing, prepend the copy binary to
-		 *	 video base this only on !online?
-		 */
-		err = load_copy_binary(pipe,
-				       &mycs->copy_binary,
-				       &mycs->video_binary);
-		if (err)
-			return err;
+	if (!IS_ISP2401) {
+		/* Copy */
+		if (!online && !continuous) {
+			/*
+			 * TODO: what exactly needs doing, prepend the copy binary to
+			 *	 video base this only on !online?
+			 */
+			err = load_copy_binary(pipe,
+					       &mycs->copy_binary,
+					       &mycs->video_binary);
+			if (err)
+				return err;
+		}
 	}
-#else
-	(void)continuous;
-#endif
 
 	if (pipe->enable_viewfinder[IA_CSS_PIPE_OUTPUT_STAGE_0] && need_vf_pp) {
 		struct ia_css_binary_descr vf_pp_descr;
@@ -5227,11 +5211,8 @@ static int load_primary_binaries(
 	bool need_pp = false;
 	bool need_isp_copy_binary = false;
 	bool need_ldc = false;
-#ifdef ISP2401
 	bool sensor = false;
-#else
 	bool memory, continuous;
-#endif
 	struct ia_css_frame_info prim_in_info,
 		       prim_out_info,
 		       capt_pp_out_info, vf_info,
@@ -5251,12 +5232,9 @@ static int load_primary_binaries(
 	       pipe->mode == IA_CSS_PIPE_ID_COPY);
 
 	online = pipe->stream->config.online;
-#ifdef ISP2401
 	sensor = (pipe->stream->config.mode == IA_CSS_INPUT_MODE_SENSOR);
-#else
 	memory = pipe->stream->config.mode == IA_CSS_INPUT_MODE_MEMORY;
 	continuous = pipe->stream->config.continuous;
-#endif
 
 	mycs = &pipe->pipe_settings.capture;
 	pipe_out_info = &pipe->output_info[0];
@@ -5462,15 +5440,14 @@ static int load_primary_binaries(
 	if (err)
 		return err;
 
-#ifdef ISP2401
-	/*
-	 * When the input system is 2401, only the Direct Sensor Mode
-	 * Offline Capture uses the ISP copy binary.
-	 */
-	need_isp_copy_binary = !online && sensor;
-#else
-	need_isp_copy_binary = !online && !continuous && !memory;
-#endif
+	if (IS_ISP2401)
+		/*
+		 * When the input system is 2401, only the Direct Sensor Mode
+		 * Offline Capture uses the ISP copy binary.
+		 */
+		need_isp_copy_binary = !online && sensor;
+	else
+		need_isp_copy_binary = !online && !continuous && !memory;
 
 	/* ISP Copy */
 	if (need_isp_copy_binary) {
@@ -5681,10 +5658,10 @@ static int load_advanced_binaries(struct ia_css_pipe *pipe)
 	}
 
 	/* Copy */
-#ifdef ISP2401
-	/* For CSI2+, only the direct sensor mode/online requires ISP copy */
-	need_isp_copy = pipe->stream->config.mode == IA_CSS_INPUT_MODE_SENSOR;
-#endif
+	if (IS_ISP2401)
+		/* For CSI2+, only the direct sensor mode/online requires ISP copy */
+		need_isp_copy = pipe->stream->config.mode == IA_CSS_INPUT_MODE_SENSOR;
+
 	if (need_isp_copy)
 		load_copy_binary(pipe,
 				 &pipe->pipe_settings.capture.copy_binary,
@@ -5829,10 +5806,10 @@ static int load_low_light_binaries(struct ia_css_pipe *pipe)
 	}
 
 	/* Copy */
-#ifdef ISP2401
-	/* For CSI2+, only the direct sensor mode/online requires ISP copy */
-	need_isp_copy = pipe->stream->config.mode == IA_CSS_INPUT_MODE_SENSOR;
-#endif
+	if (IS_ISP2401)
+		/* For CSI2+, only the direct sensor mode/online requires ISP copy */
+		need_isp_copy = pipe->stream->config.mode == IA_CSS_INPUT_MODE_SENSOR;
+
 	if (need_isp_copy)
 		err = load_copy_binary(pipe,
 				       &pipe->pipe_settings.capture.copy_binary,
@@ -5902,10 +5879,9 @@ static int load_capture_binaries(struct ia_css_pipe *pipe)
 	switch (pipe->config.default_capture_config.mode) {
 	case IA_CSS_CAPTURE_MODE_RAW:
 		err = load_copy_binaries(pipe);
-#if defined(ISP2401)
-		if (!err)
+		if (!err && IS_ISP2401)
 			pipe->pipe_settings.capture.copy_binary.online = pipe->stream->config.online;
-#endif
+
 		break;
 	case IA_CSS_CAPTURE_MODE_BAYER:
 		err = load_bayer_isp_binaries(pipe);
@@ -6409,7 +6385,6 @@ load_yuvpp_binaries(struct ia_css_pipe *pipe)
 	else
 		next_binary = NULL;
 
-#if defined(ISP2401)
 	/*
 	 * NOTES
 	 * - Why does the "yuvpp" pipe needs "isp_copy_binary" (i.e. ISP Copy) when
@@ -6427,11 +6402,11 @@ load_yuvpp_binaries(struct ia_css_pipe *pipe)
 	 *   pp_defs.h" for the list of input-frame formats that are supported by the
 	 *   "yuv_scale_binary".
 	 */
-	need_isp_copy_binary =
-	    (pipe->stream->config.input_config.format == ATOMISP_INPUT_FORMAT_YUV422_8);
-#else  /* !ISP2401 */
-	need_isp_copy_binary = true;
-#endif /*  ISP2401 */
+	if (IS_ISP2401)
+		need_isp_copy_binary =
+		    (pipe->stream->config.input_config.format == ATOMISP_INPUT_FORMAT_YUV422_8);
+	else
+		need_isp_copy_binary = true;
 
 	if (need_isp_copy_binary) {
 		err = load_copy_binary(pipe,
@@ -6678,12 +6653,10 @@ create_host_yuvpp_pipeline(struct ia_css_pipe *pipe)
 	struct ia_css_frame *vf_frame[IA_CSS_PIPE_MAX_OUTPUT_STAGE];
 	struct ia_css_pipeline_stage_desc stage_desc;
 	bool need_in_frameinfo_memory = false;
-#ifdef ISP2401
 	bool sensor = false;
 	bool buffered_sensor = false;
 	bool online = false;
 	bool continuous = false;
-#endif
 
 	IA_CSS_ENTER_PRIVATE("pipe = %p", pipe);
 	if ((!pipe) || (!pipe->stream) || (pipe->mode != IA_CSS_PIPE_ID_YUVPP)) {
@@ -6700,24 +6673,24 @@ create_host_yuvpp_pipeline(struct ia_css_pipe *pipe)
 	num_stage  = pipe->pipe_settings.yuvpp.num_yuv_scaler;
 	num_output_stage   = pipe->pipe_settings.yuvpp.num_output;
 
-#ifdef ISP2401
-	/*
-	 * When the input system is 2401, always enable 'in_frameinfo_memory'
-	 * except for the following:
-	 * - Direct Sensor Mode Online Capture
-	 * - Direct Sensor Mode Continuous Capture
-	 * - Buffered Sensor Mode Continuous Capture
-	 */
-	sensor = pipe->stream->config.mode == IA_CSS_INPUT_MODE_SENSOR;
-	buffered_sensor = pipe->stream->config.mode == IA_CSS_INPUT_MODE_BUFFERED_SENSOR;
-	online = pipe->stream->config.online;
-	continuous = pipe->stream->config.continuous;
-	need_in_frameinfo_memory =
-	!((sensor && (online || continuous)) || (buffered_sensor && continuous));
-#else
-	/* Construct in_frame info (only in case we have dynamic input */
-	need_in_frameinfo_memory = pipe->stream->config.mode == IA_CSS_INPUT_MODE_MEMORY;
-#endif
+	if (IS_ISP2401) {
+		/*
+		 * When the input system is 2401, always enable 'in_frameinfo_memory'
+		 * except for the following:
+		 * - Direct Sensor Mode Online Capture
+		 * - Direct Sensor Mode Continuous Capture
+		 * - Buffered Sensor Mode Continuous Capture
+		 */
+		sensor = pipe->stream->config.mode == IA_CSS_INPUT_MODE_SENSOR;
+		buffered_sensor = pipe->stream->config.mode == IA_CSS_INPUT_MODE_BUFFERED_SENSOR;
+		online = pipe->stream->config.online;
+		continuous = pipe->stream->config.continuous;
+		need_in_frameinfo_memory =
+		!((sensor && (online || continuous)) || (buffered_sensor && continuous));
+	} else {
+		/* Construct in_frame info (only in case we have dynamic input */
+		need_in_frameinfo_memory = pipe->stream->config.mode == IA_CSS_INPUT_MODE_MEMORY;
+	}
 	/*
 	 * the input frame can come from:
 	 *
@@ -6808,11 +6781,10 @@ create_host_yuvpp_pipeline(struct ia_css_pipe *pipe)
 	if (pipe->pipe_settings.yuvpp.copy_binary.info) {
 		struct ia_css_frame *in_frame_local = NULL;
 
-#ifdef ISP2401
-		/* After isp copy is enabled in_frame needs to be passed. */
-		if (!online)
+		if (IS_ISP2401 && !online) {
+			/* After isp copy is enabled in_frame needs to be passed. */
 			in_frame_local = in_frame;
-#endif
+		}
 
 		if (need_scaler) {
 			ia_css_pipe_util_set_output_frames(bin_out_frame,
@@ -7031,12 +7003,10 @@ create_host_regular_capture_pipeline(struct ia_css_pipe *pipe)
 	struct ia_css_frame *vf_frame;
 	struct ia_css_pipeline_stage_desc stage_desc;
 	bool need_in_frameinfo_memory = false;
-#ifdef ISP2401
 	bool sensor = false;
 	bool buffered_sensor = false;
 	bool online = false;
 	bool continuous = false;
-#endif
 	unsigned int i, num_yuv_scaler, num_primary_stage;
 	bool need_yuv_pp = false;
 	bool *is_output_stage = NULL;
@@ -7054,25 +7024,27 @@ create_host_regular_capture_pipeline(struct ia_css_pipe *pipe)
 	ia_css_pipeline_clean(me);
 	ia_css_pipe_util_create_output_frames(out_frames);
 
-#ifdef ISP2401
-	/*
-	 * When the input system is 2401, always enable 'in_frameinfo_memory'
-	 * except for the following:
-	 * - Direct Sensor Mode Online Capture
-	 * - Direct Sensor Mode Online Capture
-	 * - Direct Sensor Mode Continuous Capture
-	 * - Buffered Sensor Mode Continuous Capture
-	 */
-	sensor = (pipe->stream->config.mode == IA_CSS_INPUT_MODE_SENSOR);
-	buffered_sensor = (pipe->stream->config.mode == IA_CSS_INPUT_MODE_BUFFERED_SENSOR);
-	online = pipe->stream->config.online;
-	continuous = pipe->stream->config.continuous;
-	need_in_frameinfo_memory =
-	!((sensor && (online || continuous)) || (buffered_sensor && (online || continuous)));
-#else
-	/* Construct in_frame info (only in case we have dynamic input */
-	need_in_frameinfo_memory = pipe->stream->config.mode == IA_CSS_INPUT_MODE_MEMORY;
-#endif
+	if (IS_ISP2401) {
+		/*
+		 * When the input system is 2401, always enable 'in_frameinfo_memory'
+		 * except for the following:
+		 * - Direct Sensor Mode Online Capture
+		 * - Direct Sensor Mode Online Capture
+		 * - Direct Sensor Mode Continuous Capture
+		 * - Buffered Sensor Mode Continuous Capture
+		 */
+		sensor = (pipe->stream->config.mode == IA_CSS_INPUT_MODE_SENSOR);
+		buffered_sensor = (pipe->stream->config.mode == IA_CSS_INPUT_MODE_BUFFERED_SENSOR);
+		online = pipe->stream->config.online;
+		continuous = pipe->stream->config.continuous;
+		need_in_frameinfo_memory =
+		!((sensor && (online || continuous)) || (buffered_sensor &&
+							(online || continuous)));
+	} else {
+		/* Construct in_frame info (only in case we have dynamic input */
+		need_in_frameinfo_memory = pipe->stream->config.mode == IA_CSS_INPUT_MODE_MEMORY;
+	}
+
 	if (need_in_frameinfo_memory) {
 		err = init_in_frameinfo_memory_defaults(pipe, &me->in_frame,
 							IA_CSS_FRAME_FORMAT_RAW);
@@ -7135,27 +7107,27 @@ create_host_regular_capture_pipeline(struct ia_css_pipe *pipe)
 	if (pipe->pipe_settings.capture.copy_binary.info) {
 		if (raw) {
 			ia_css_pipe_util_set_output_frames(out_frames, 0, out_frame);
-#if defined(ISP2401)
-			if (!continuous) {
-				ia_css_pipe_get_generic_stage_desc(&stage_desc,
-								   copy_binary,
-								   out_frames,
-								   in_frame,
-								   NULL);
+			if (IS_ISP2401) {
+				if (!continuous) {
+					ia_css_pipe_get_generic_stage_desc(&stage_desc,
+									   copy_binary,
+									   out_frames,
+									   in_frame,
+									   NULL);
+				} else {
+					in_frame = pipe->stream->last_pipe->continuous_frames[0];
+					ia_css_pipe_get_generic_stage_desc(&stage_desc,
+									   copy_binary,
+									   out_frames,
+									   in_frame,
+									   NULL);
+				}
 			} else {
-				in_frame = pipe->stream->last_pipe->continuous_frames[0];
 				ia_css_pipe_get_generic_stage_desc(&stage_desc,
 								   copy_binary,
 								   out_frames,
-								   in_frame,
-								   NULL);
+								   NULL, NULL);
 			}
-#else
-			ia_css_pipe_get_generic_stage_desc(&stage_desc,
-							   copy_binary,
-							   out_frames,
-							   NULL, NULL);
-#endif
 		} else {
 			ia_css_pipe_util_set_output_frames(out_frames, 0,
 							   in_frame);
@@ -7185,11 +7157,7 @@ create_host_regular_capture_pipeline(struct ia_css_pipe *pipe)
 				local_in_frame = in_frame;
 			else
 				local_in_frame = NULL;
-#ifndef ISP2401
-			if (!need_pp && (i == num_primary_stage - 1))
-#else
-			if (!need_pp && (i == num_primary_stage - 1) && !need_ldc)
-#endif
+			if (!need_pp && (i == num_primary_stage - 1) && (!IS_ISP2401 || !need_ldc))
 				local_out_frame = out_frame;
 			else
 				local_out_frame = NULL;
@@ -7400,23 +7368,14 @@ static int capture_start(struct ia_css_pipe *pipe)
 			return err;
 		}
 	}
-
-#if !defined(ISP2401)
 	/* old isys: need to send_mipi_frames() in all pipe modes */
-	err = send_mipi_frames(pipe);
-	if (err) {
-		IA_CSS_LEAVE_ERR_PRIVATE(err);
-		return err;
-	}
-#else
-	if (pipe->config.mode != IA_CSS_PIPE_MODE_COPY) {
+	if (!IS_ISP2401 || (IS_ISP2401 && pipe->config.mode != IA_CSS_PIPE_MODE_COPY)) {
 		err = send_mipi_frames(pipe);
 		if (err) {
 			IA_CSS_LEAVE_ERR_PRIVATE(err);
 			return err;
 		}
 	}
-#endif
 
 	ia_css_pipeline_get_sp_thread_id(ia_css_pipe_get_pipe_num(pipe), &thread_id);
 	copy_ovrd = 1 << thread_id;
@@ -8123,24 +8082,22 @@ ia_css_stream_create(const struct ia_css_stream_config *stream_config,
 		return err;
 	}
 
-#if !defined(ISP2401)
-	/* We don't support metadata for JPEG stream, since they both use str2mem */
-	if (stream_config->input_config.format == ATOMISP_INPUT_FORMAT_BINARY_8 &&
-	    stream_config->metadata_config.resolution.height > 0) {
-		err = -EINVAL;
-		IA_CSS_LEAVE_ERR(err);
-		return err;
-	}
-#endif
-
-#ifdef ISP2401
-	if (stream_config->online && stream_config->pack_raw_pixels) {
-		IA_CSS_LOG("online and pack raw is invalid on input system 2401");
-		err = -EINVAL;
-		IA_CSS_LEAVE_ERR(err);
-		return err;
+	if (!IS_ISP2401) {
+		/* We don't support metadata for JPEG stream, since they both use str2mem */
+		if (stream_config->input_config.format == ATOMISP_INPUT_FORMAT_BINARY_8 &&
+		    stream_config->metadata_config.resolution.height > 0) {
+			err = -EINVAL;
+			IA_CSS_LEAVE_ERR(err);
+			return err;
+		}
+	} else {
+		if (stream_config->online && stream_config->pack_raw_pixels) {
+			IA_CSS_LOG("online and pack raw is invalid on input system 2401");
+			err = -EINVAL;
+			IA_CSS_LEAVE_ERR(err);
+			return err;
+		}
 	}
-#endif
 
 	ia_css_debug_pipe_graph_dump_stream_config(stream_config);
 
@@ -8223,19 +8180,17 @@ ia_css_stream_create(const struct ia_css_stream_config *stream_config,
 	/* take over stream config */
 	curr_stream->config = *stream_config;
 
-#if defined(ISP2401)
-	if (stream_config->mode == IA_CSS_INPUT_MODE_BUFFERED_SENSOR &&
-	    stream_config->online)
-		curr_stream->config.online = false;
-#endif
+	if (IS_ISP2401) {
+		if (stream_config->mode == IA_CSS_INPUT_MODE_BUFFERED_SENSOR &&
+		    stream_config->online)
+			curr_stream->config.online = false;
 
-#ifdef ISP2401
-	if (curr_stream->config.online) {
-		curr_stream->config.source.port.num_lanes =
-		    stream_config->source.port.num_lanes;
-		curr_stream->config.mode =  IA_CSS_INPUT_MODE_BUFFERED_SENSOR;
+		if (curr_stream->config.online) {
+			curr_stream->config.source.port.num_lanes =
+			    stream_config->source.port.num_lanes;
+			curr_stream->config.mode =  IA_CSS_INPUT_MODE_BUFFERED_SENSOR;
+		}
 	}
-#endif
 	/* in case driver doesn't configure init number of raw buffers, configure it here */
 	if (curr_stream->config.target_num_cont_raw_buf == 0)
 		curr_stream->config.target_num_cont_raw_buf = NUM_CONTINUOUS_FRAMES;
@@ -9162,11 +9117,10 @@ void ia_css_pipe_map_queue(struct ia_css_pipe *pipe, bool map)
 
 	ia_css_pipeline_get_sp_thread_id(pipe_num, &thread_id);
 
-#if defined(ISP2401)
-	need_input_queue = true;
-#else
-	need_input_queue = pipe->stream->config.mode == IA_CSS_INPUT_MODE_MEMORY;
-#endif
+	if (IS_ISP2401)
+		need_input_queue = true;
+	else
+		need_input_queue = pipe->stream->config.mode == IA_CSS_INPUT_MODE_MEMORY;
 
 	/* map required buffer queues to resources */
 	/* TODO: to be improved */
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 2/5] staging: media: atomisp: runtime: frame: remove #ifdef ISP2401
  2023-05-08  6:26 [PATCH v2 1/5] staging: media: atomisp: sh_css: Remove #ifdef ISP2401 Kate Hsuan
@ 2023-05-08  6:26 ` Kate Hsuan
  2023-05-08  6:26 ` [PATCH v2 3/5] staging: media: atomisp: sh_css_sp: Remove " Kate Hsuan
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Kate Hsuan @ 2023-05-08  6:26 UTC (permalink / raw)
  To: Hans de Goede, Mauro Carvalho Chehab, Sakari Ailus,
	Greg Kroah-Hartman, linux-media, linux-staging
  Cc: Kate Hsuan

The actions of ISP2401 and 2400 are determined at the runtime.

Signed-off-by: Kate Hsuan <hpa@redhat.com>
---
 .../media/atomisp/pci/runtime/frame/src/frame.c     | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/runtime/frame/src/frame.c b/drivers/staging/media/atomisp/pci/runtime/frame/src/frame.c
index 83bb42e05421..1e374ae848e3 100644
--- a/drivers/staging/media/atomisp/pci/runtime/frame/src/frame.c
+++ b/drivers/staging/media/atomisp/pci/runtime/frame/src/frame.c
@@ -601,9 +601,6 @@ static void frame_init_qplane6_planes(struct ia_css_frame *frame)
 
 static int frame_allocate_buffer_data(struct ia_css_frame *frame)
 {
-#ifdef ISP2401
-	IA_CSS_ENTER_LEAVE_PRIVATE("frame->data_bytes=%d\n", frame->data_bytes);
-#endif
 	frame->data = hmm_alloc(frame->data_bytes);
 	if (frame->data == mmgr_NULL)
 		return -ENOMEM;
@@ -635,15 +632,11 @@ static int frame_allocate_with_data(struct ia_css_frame **frame,
 
 	if (err) {
 		kvfree(me);
-#ifndef ISP2401
-		return err;
-#else
-		me = NULL;
-#endif
+		*frame = NULL;
+	} else {
+		*frame = me;
 	}
 
-	*frame = me;
-
 	return err;
 }
 
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 3/5] staging: media: atomisp: sh_css_sp: Remove #ifdef ISP2401
  2023-05-08  6:26 [PATCH v2 1/5] staging: media: atomisp: sh_css: Remove #ifdef ISP2401 Kate Hsuan
  2023-05-08  6:26 ` [PATCH v2 2/5] staging: media: atomisp: runtime: frame: remove " Kate Hsuan
@ 2023-05-08  6:26 ` Kate Hsuan
  2023-05-08  6:26 ` [PATCH v2 4/5] staging: media: atomisp: sh_css_firmware: determine firmware version at runtime Kate Hsuan
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Kate Hsuan @ 2023-05-08  6:26 UTC (permalink / raw)
  To: Hans de Goede, Mauro Carvalho Chehab, Sakari Ailus,
	Greg Kroah-Hartman, linux-media, linux-staging
  Cc: Kate Hsuan

The actions of ISP2401 and 2400 will be determined at the runtime.

Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Kate Hsuan <hpa@redhat.com>
---
 drivers/staging/media/atomisp/pci/sh_css_sp.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/sh_css_sp.c b/drivers/staging/media/atomisp/pci/sh_css_sp.c
index 0dd58a7fe2cc..297e1b981720 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_sp.c
+++ b/drivers/staging/media/atomisp/pci/sh_css_sp.c
@@ -952,12 +952,10 @@ sh_css_sp_init_stage(struct ia_css_binary *binary,
 		return 0;
 	}
 
-#if defined(ISP2401)
-	(void)continuous;
-	sh_css_sp_stage.deinterleaved = 0;
-#else
-	sh_css_sp_stage.deinterleaved = ((stage == 0) && continuous);
-#endif
+	if (IS_ISP2401)
+		sh_css_sp_stage.deinterleaved = 0;
+	else
+		sh_css_sp_stage.deinterleaved = ((stage == 0) && continuous);
 
 	initialize_stage_frames(&sh_css_sp_stage.frames);
 	/*
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 4/5] staging: media: atomisp: sh_css_firmware: determine firmware version at runtime
  2023-05-08  6:26 [PATCH v2 1/5] staging: media: atomisp: sh_css: Remove #ifdef ISP2401 Kate Hsuan
  2023-05-08  6:26 ` [PATCH v2 2/5] staging: media: atomisp: runtime: frame: remove " Kate Hsuan
  2023-05-08  6:26 ` [PATCH v2 3/5] staging: media: atomisp: sh_css_sp: Remove " Kate Hsuan
@ 2023-05-08  6:26 ` Kate Hsuan
  2023-05-08  6:26 ` [PATCH v2 5/5] staging: media: atomisp: sh_css_mipi: Remove #ifdef 2041 Kate Hsuan
  2023-05-10 18:40 ` [PATCH v2 1/5] staging: media: atomisp: sh_css: Remove #ifdef ISP2401 Hans de Goede
  4 siblings, 0 replies; 8+ messages in thread
From: Kate Hsuan @ 2023-05-08  6:26 UTC (permalink / raw)
  To: Hans de Goede, Mauro Carvalho Chehab, Sakari Ailus,
	Greg Kroah-Hartman, linux-media, linux-staging
  Cc: Kate Hsuan

The firmware version of ISP2401 and 2400 is determined at runtime.

Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Kate Hsuan <hpa@redhat.com>
---
 .../media/atomisp/pci/sh_css_firmware.c        | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/sh_css_firmware.c b/drivers/staging/media/atomisp/pci/sh_css_firmware.c
index e7ef578db8ab..49ee88fe151d 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_firmware.c
+++ b/drivers/staging/media/atomisp/pci/sh_css_firmware.c
@@ -56,11 +56,8 @@ static struct firmware_header *firmware_header;
  * which will be replaced with the actual RELEASE_VERSION
  * during package generation. Please do not modify
  */
-#ifdef ISP2401
-static const char *release_version = STR(irci_stable_candrpv_0415_20150521_0458);
-#else
-static const char *release_version = STR(irci_stable_candrpv_0415_20150423_1753);
-#endif
+static const char *release_version_2401 = STR(irci_stable_candrpv_0415_20150521_0458);
+static const char *release_version_2400 = STR(irci_stable_candrpv_0415_20150423_1753);
 
 #define MAX_FW_REL_VER_NAME	300
 static char FW_rel_ver_name[MAX_FW_REL_VER_NAME] = "---";
@@ -191,8 +188,14 @@ sh_css_load_blob_info(const char *fw, const struct ia_css_fw_info *bi,
 bool
 sh_css_check_firmware_version(struct device *dev, const char *fw_data)
 {
+	const char *release_version;
 	struct sh_css_fw_bi_file_h *file_header;
 
+	if (IS_ISP2401)
+		release_version = release_version_2401;
+	else
+		release_version = release_version_2400;
+
 	firmware_header = (struct firmware_header *)fw_data;
 	file_header = &firmware_header->file_header;
 
@@ -225,6 +228,7 @@ sh_css_load_firmware(struct device *dev, const char *fw_data,
 		     unsigned int fw_size)
 {
 	unsigned int i;
+	const char *release_version;
 	struct ia_css_fw_info *binaries;
 	struct sh_css_fw_bi_file_h *file_header;
 	int ret;
@@ -234,6 +238,10 @@ sh_css_load_firmware(struct device *dev, const char *fw_data,
 	binaries = &firmware_header->binary_header;
 	strscpy(FW_rel_ver_name, file_header->version,
 		min(sizeof(FW_rel_ver_name), sizeof(file_header->version)));
+	if (IS_ISP2401)
+		release_version = release_version_2401;
+	else
+		release_version = release_version_2400;
 	ret = sh_css_check_firmware_version(dev, fw_data);
 	if (ret) {
 		IA_CSS_ERROR("CSS code version (%s) and firmware version (%s) mismatch!",
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 5/5] staging: media: atomisp: sh_css_mipi: Remove #ifdef 2041
  2023-05-08  6:26 [PATCH v2 1/5] staging: media: atomisp: sh_css: Remove #ifdef ISP2401 Kate Hsuan
                   ` (2 preceding siblings ...)
  2023-05-08  6:26 ` [PATCH v2 4/5] staging: media: atomisp: sh_css_firmware: determine firmware version at runtime Kate Hsuan
@ 2023-05-08  6:26 ` Kate Hsuan
  2023-05-10 18:34   ` Hans de Goede
  2023-05-10 18:40 ` [PATCH v2 1/5] staging: media: atomisp: sh_css: Remove #ifdef ISP2401 Hans de Goede
  4 siblings, 1 reply; 8+ messages in thread
From: Kate Hsuan @ 2023-05-08  6:26 UTC (permalink / raw)
  To: Hans de Goede, Mauro Carvalho Chehab, Sakari Ailus,
	Greg Kroah-Hartman, linux-media, linux-staging
  Cc: Kate Hsuan

The actions of ISP2401 and 2400 are determined at the runtime.

Signed-off-by: Kate Hsuan <hpa@redhat.com>
---
 .../staging/media/atomisp/pci/sh_css_mipi.c   | 65 ++++++-------------
 1 file changed, 20 insertions(+), 45 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/sh_css_mipi.c b/drivers/staging/media/atomisp/pci/sh_css_mipi.c
index bc6e8598a776..52a1ed63e9a5 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_mipi.c
+++ b/drivers/staging/media/atomisp/pci/sh_css_mipi.c
@@ -67,13 +67,12 @@ ia_css_mipi_frame_calculate_size(const unsigned int width,
 	unsigned int mem_words = 0;
 	unsigned int width_padded = width;
 
-#if defined(ISP2401)
 	/* The changes will be reverted as soon as RAW
 	 * Buffers are deployed by the 2401 Input System
 	 * in the non-continuous use scenario.
 	 */
-	width_padded += (2 * ISP_VEC_NELEMS);
-#endif
+	if (IS_ISP2401)
+		width_padded += (2 * ISP_VEC_NELEMS);
 
 	IA_CSS_ENTER("padded_width=%d, height=%d, format=%d, hasSOLandEOL=%d, embedded_data_size_words=%d\n",
 		     width_padded, height, format, hasSOLandEOL, embedded_data_size_words);
@@ -235,7 +234,6 @@ bool mipi_is_free(void)
 	return true;
 }
 
-#if defined(ISP2401)
 /*
  * @brief Calculate the required MIPI buffer sizes.
  * Based on the stream configuration, calculate the
@@ -342,7 +340,6 @@ static int calculate_mipi_buff_size(struct ia_css_stream_config *stream_cfg,
 	IA_CSS_LEAVE_ERR(err);
 	return err;
 }
-#endif
 
 int
 allocate_mipi_frames(struct ia_css_pipe *pipe,
@@ -363,15 +360,13 @@ allocate_mipi_frames(struct ia_css_pipe *pipe,
 		return -EINVAL;
 	}
 
-#ifdef ISP2401
-	if (pipe->stream->config.online) {
+	if (IS_ISP2401 && pipe->stream->config.online) {
 		ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE_PRIVATE,
 				    "allocate_mipi_frames(%p) exit: no buffers needed for 2401 pipe mode.\n",
 				    pipe);
 		return 0;
 	}
 
-#endif
 	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",
@@ -386,30 +381,22 @@ allocate_mipi_frames(struct ia_css_pipe *pipe,
 		return -EINVAL;
 	}
 
-#ifdef ISP2401
-	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.
-	 */
-	if (ref_count_mipi_allocation[port] != 0) {
-		ref_count_mipi_allocation[port]++;
-		ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE_PRIVATE,
-				    "allocate_mipi_frames(%p) leave: nothing to do, already allocated for this port (port=%d).\n",
-				    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;
+	} else {
+		/*
+		 * 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.
+		 */
+		if (IS_ISP2401)
+			err = calculate_mipi_buff_size(&pipe->stream->config,
+						       &my_css.mipi_frame_size[port]);
 	}
-#endif
 
 	ref_count_mipi_allocation[port]++;
 
@@ -503,14 +490,14 @@ free_mipi_frames(struct ia_css_pipe *pipe)
 		}
 
 		if (ref_count_mipi_allocation[port] > 0) {
-#if !defined(ISP2401)
-			assert(ref_count_mipi_allocation[port] == 1);
-			if (ref_count_mipi_allocation[port] != 1) {
-				IA_CSS_ERROR("free_mipi_frames(%p) exit: wrong ref_count (ref_count=%d).",
-					     pipe, ref_count_mipi_allocation[port]);
-				return err;
+			if (!IS_ISP2401) {
+				assert(ref_count_mipi_allocation[port] == 1);
+				if (ref_count_mipi_allocation[port] != 1) {
+					IA_CSS_ERROR("free_mipi_frames(%p) exit: wrong ref_count (ref_count=%d).",
+						     pipe, ref_count_mipi_allocation[port]);
+					return err;
+				}
 			}
-#endif
 
 			ref_count_mipi_allocation[port]--;
 
@@ -534,18 +521,6 @@ free_mipi_frames(struct ia_css_pipe *pipe)
 				ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE_PRIVATE,
 						    "free_mipi_frames(%p) exit (deallocated).\n", pipe);
 			}
-#if defined(ISP2401)
-			else {
-				/* 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.
-				 */
-				ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE_PRIVATE,
-						    "free_mipi_frames(%p) leave: nothing to do, other streams still use this port (port=%d).\n",
-						    pipe, port);
-			}
-#endif
 		}
 	} else { /* pipe ==NULL */
 		/* AM TEMP: free-ing all mipi buffers just like a legacy code. */
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 5/5] staging: media: atomisp: sh_css_mipi: Remove #ifdef 2041
  2023-05-08  6:26 ` [PATCH v2 5/5] staging: media: atomisp: sh_css_mipi: Remove #ifdef 2041 Kate Hsuan
@ 2023-05-10 18:34   ` Hans de Goede
  2023-05-11 11:18     ` Kate Hsuan
  0 siblings, 1 reply; 8+ messages in thread
From: Hans de Goede @ 2023-05-10 18:34 UTC (permalink / raw)
  To: Kate Hsuan, Mauro Carvalho Chehab, Sakari Ailus,
	Greg Kroah-Hartman, linux-media, linux-staging

Hi Kate,

On 5/8/23 08:26, Kate Hsuan wrote:
> The actions of ISP2401 and 2400 are determined at the runtime.
> 
> Signed-off-by: Kate Hsuan <hpa@redhat.com>
> ---
>  .../staging/media/atomisp/pci/sh_css_mipi.c   | 65 ++++++-------------
>  1 file changed, 20 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/staging/media/atomisp/pci/sh_css_mipi.c b/drivers/staging/media/atomisp/pci/sh_css_mipi.c
> index bc6e8598a776..52a1ed63e9a5 100644
> --- a/drivers/staging/media/atomisp/pci/sh_css_mipi.c
> +++ b/drivers/staging/media/atomisp/pci/sh_css_mipi.c
> @@ -386,30 +381,22 @@ allocate_mipi_frames(struct ia_css_pipe *pipe,
>  		return -EINVAL;
>  	}
>  
> -#ifdef ISP2401
> -	err = calculate_mipi_buff_size(&pipe->stream->config,
> -				       &my_css.mipi_frame_size[port]);

Before you changes this code always run ISP2401, now it only runs
when (ref_count_mipi_allocation[port] != 0) is not true.

So this statement should stay here in the code, just prefixed
with a if (IS_ISP2401) condition.

> -	/*
> -	 * 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.
> -	 */
> -	if (ref_count_mipi_allocation[port] != 0) {
> -		ref_count_mipi_allocation[port]++;
> -		ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE_PRIVATE,
> -				    "allocate_mipi_frames(%p) leave: nothing to do, already allocated for this port (port=%d).\n",
> -				    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;
> +	} else {
> +		/*
> +		 * 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.
> +		 */

This comment block actually belongs to the if (ref_count_mipi_allocation[port] != 0)
check, the code executed if that check passes was actually different between
the ISP2400 and ISP2401 (my bad, sorry). The ISP2401 case did an extra:

		ref_count_mipi_allocation[port]++;

when (ref_count_mipi_allocation[port] != 0), so we need to add:

		if (IS_ISP2401)
			ref_count_mipi_allocation[port]++;

above the return 0 above.

> +		if (IS_ISP2401)
> +			err = calculate_mipi_buff_size(&pipe->stream->config,
> +						       &my_css.mipi_frame_size[port]);

I have fixed this all up while merging your series and the new
diff for this code-block now looks like this:

@@ -386,9 +381,10 @@ allocate_mipi_frames(struct ia_css_pipe *pipe,
 		return -EINVAL;
 	}
 
-#ifdef ISP2401
-	err = calculate_mipi_buff_size(&pipe->stream->config,
-				       &my_css.mipi_frame_size[port]);
+	if (IS_ISP2401)
+		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.
@@ -396,20 +392,14 @@ allocate_mipi_frames(struct ia_css_pipe *pipe,
 	 * In that case only 2400 related code should remain.
 	 */
 	if (ref_count_mipi_allocation[port] != 0) {
-		ref_count_mipi_allocation[port]++;
+		if (IS_ISP2401)
+			ref_count_mipi_allocation[port]++;
+
 		ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE_PRIVATE,
 				    "allocate_mipi_frames(%p) leave: nothing to do, already allocated for this port (port=%d).\n",
 				    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]++;
 


Regards,

Hans


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/5] staging: media: atomisp: sh_css: Remove #ifdef ISP2401
  2023-05-08  6:26 [PATCH v2 1/5] staging: media: atomisp: sh_css: Remove #ifdef ISP2401 Kate Hsuan
                   ` (3 preceding siblings ...)
  2023-05-08  6:26 ` [PATCH v2 5/5] staging: media: atomisp: sh_css_mipi: Remove #ifdef 2041 Kate Hsuan
@ 2023-05-10 18:40 ` Hans de Goede
  4 siblings, 0 replies; 8+ messages in thread
From: Hans de Goede @ 2023-05-10 18:40 UTC (permalink / raw)
  To: Kate Hsuan, Mauro Carvalho Chehab, Sakari Ailus,
	Greg Kroah-Hartman, linux-media, linux-staging

Hi Kate,

On 5/8/23 08:26, Kate Hsuan wrote:
> The actions of ISP2401 and 2400 are determined at the runtime.
> 
> Signed-off-by: Kate Hsuan <hpa@redhat.com>

Thank you for your work on this!

I've merged this series into my hansg/media-atomisp branch:

https://git.kernel.org/pub/scm/linux/kernel/git/hansg/linux.git/log/?h=media-atomisp

(with as noted in my other email a small fix to patch 5/5)

I will send a pull-req to Mauro for merging all the patches
I am collecting there around the time of the 6.4-rc5 release.

Note I'll add a bunch of patches I've been working on myself to
my media-atomisp branch later today. Please base any further atomisp
patches on top of my media-atomisp branch.

Regards,

Hans





> ---
>  drivers/staging/media/atomisp/pci/sh_css.c | 524 ++++++++++-----------
>  1 file changed, 239 insertions(+), 285 deletions(-)
> 
> diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c
> index 93789500416f..4b3fa6d93fe0 100644
> --- a/drivers/staging/media/atomisp/pci/sh_css.c
> +++ b/drivers/staging/media/atomisp/pci/sh_css.c
> @@ -1529,15 +1529,14 @@ ia_css_init(struct device *dev, const struct ia_css_env *env,
>  
>  	mipi_init();
>  
> -#ifndef ISP2401
>  	/*
>  	 * In case this has been programmed already, update internal
>  	 * data structure ...
>  	 * DEPRECATED
>  	 */
> -	my_css.page_table_base_index = mmu_get_page_table_base_index(MMU0_ID);
> +	if (!IS_ISP2401)
> +		my_css.page_table_base_index = mmu_get_page_table_base_index(MMU0_ID);
>  
> -#endif
>  	my_css.irq_type = irq_type;
>  
>  	my_css_save.irq_type = irq_type;
> @@ -1596,10 +1595,8 @@ ia_css_init(struct device *dev, const struct ia_css_env *env,
>  	 * sh_css_init_buffer_queues();
>  	 */
>  
> -#if defined(ISP2401)
> -	gp_device_reg_store(GP_DEVICE0_ID, _REG_GP_SWITCH_ISYS2401_ADDR, 1);
> -#endif
> -
> +	if (IS_ISP2401)
> +		gp_device_reg_store(GP_DEVICE0_ID, _REG_GP_SWITCH_ISYS2401_ADDR, 1);
>  
>  	if (!IS_ISP2401)
>  		dma_set_max_burst_size(DMA0_ID, HIVE_DMA_BUS_DDR_CONN,
> @@ -2128,13 +2125,8 @@ ia_css_pipe_destroy(struct ia_css_pipe *pipe)
>  						    err);
>  			}
>  		}
> -#ifndef ISP2401
>  		ia_css_frame_free_multiple(NUM_VIDEO_TNR_FRAMES,
>  					   pipe->pipe_settings.video.tnr_frames);
> -#else
> -		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,
>  					   pipe->pipe_settings.video.delay_frames);
>  		break;
> @@ -2238,11 +2230,10 @@ int ia_css_irq_translate(
>  		case virq_isys_csi:
>  			infos |= IA_CSS_IRQ_INFO_INPUT_SYSTEM_ERROR;
>  			break;
> -#if !defined(ISP2401)
>  		case virq_ifmt0_id:
> -			infos |= IA_CSS_IRQ_INFO_IF_ERROR;
> +			if (!IS_ISP2401)
> +				infos |= IA_CSS_IRQ_INFO_IF_ERROR;
>  			break;
> -#endif
>  		case virq_dma:
>  			infos |= IA_CSS_IRQ_INFO_DMA_ERROR;
>  			break;
> @@ -2277,27 +2268,34 @@ int ia_css_irq_enable(
>  	IA_CSS_ENTER("info=%d, enable=%d", info, enable);
>  
>  	switch (info) {
> -#if !defined(ISP2401)
>  	case IA_CSS_IRQ_INFO_CSS_RECEIVER_SOF:
> +		if (IS_ISP2401)
> +			/* Just ignore those unused IRQs without printing errors */
> +			return 0;
> +
>  		irq = virq_isys_sof;
>  		break;
>  	case IA_CSS_IRQ_INFO_CSS_RECEIVER_EOF:
> +		if (IS_ISP2401)
> +			/* Just ignore those unused IRQs without printing errors */
> +			return 0;
> +
>  		irq = virq_isys_eof;
>  		break;
>  	case IA_CSS_IRQ_INFO_INPUT_SYSTEM_ERROR:
> +		if (IS_ISP2401)
> +			/* Just ignore those unused IRQs without printing errors */
> +			return 0;
> +
>  		irq = virq_isys_csi;
>  		break;
>  	case IA_CSS_IRQ_INFO_IF_ERROR:
> +		if (IS_ISP2401)
> +			/* Just ignore those unused IRQs without printing errors */
> +			return 0;
> +
>  		irq = virq_ifmt0_id;
>  		break;
> -#else
> -	case IA_CSS_IRQ_INFO_CSS_RECEIVER_SOF:
> -	case IA_CSS_IRQ_INFO_CSS_RECEIVER_EOF:
> -	case IA_CSS_IRQ_INFO_INPUT_SYSTEM_ERROR:
> -	case IA_CSS_IRQ_INFO_IF_ERROR:
> -		/* Just ignore those unused IRQs without printing errors */
> -		return 0;
> -#endif
>  	case IA_CSS_IRQ_INFO_DMA_ERROR:
>  		irq = virq_dma;
>  		break;
> @@ -2413,14 +2411,14 @@ alloc_continuous_frames(struct ia_css_pipe *pipe, bool init_time)
>  		return -EINVAL;
>  	}
>  
> -#if defined(ISP2401)
> -	/* For CSI2+, the continuous frame will hold the full input frame */
> -	ref_info.res.width = pipe->stream->config.input_config.input_res.width;
> -	ref_info.res.height = pipe->stream->config.input_config.input_res.height;
> +	if (IS_ISP2401) {
> +		/* For CSI2+, the continuous frame will hold the full input frame */
> +		ref_info.res.width = pipe->stream->config.input_config.input_res.width;
> +		ref_info.res.height = pipe->stream->config.input_config.input_res.height;
>  
> -	/* Ensure padded width is aligned for 2401 */
> -	ref_info.padded_width = CEIL_MUL(ref_info.res.width, 2 * ISP_VEC_NELEMS);
> -#endif
> +		/* Ensure padded width is aligned for 2401 */
> +		ref_info.padded_width = CEIL_MUL(ref_info.res.width, 2 * ISP_VEC_NELEMS);
> +	}
>  
>  	if (pipe->stream->config.pack_raw_pixels) {
>  		ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE_PRIVATE,
> @@ -2499,11 +2497,9 @@ load_preview_binaries(struct ia_css_pipe *pipe)
>  	int err = 0;
>  	bool need_vf_pp = false;
>  	bool need_isp_copy_binary = false;
> -#ifdef ISP2401
>  	bool sensor = false;
> -#else
>  	bool continuous;
> -#endif
> +
>  	/* preview only have 1 output pin now */
>  	struct ia_css_frame_info *pipe_out_info = &pipe->output_info[0];
>  	struct ia_css_preview_settings *mycs  = &pipe->pipe_settings.preview;
> @@ -2514,11 +2510,9 @@ load_preview_binaries(struct ia_css_pipe *pipe)
>  	assert(pipe->mode == IA_CSS_PIPE_ID_PREVIEW);
>  
>  	online = pipe->stream->config.online;
> -#ifdef ISP2401
> +
>  	sensor = pipe->stream->config.mode == IA_CSS_INPUT_MODE_SENSOR;
> -#else
>  	continuous = pipe->stream->config.continuous;
> -#endif
>  
>  	if (mycs->preview_binary.info)
>  		return 0;
> @@ -2627,24 +2621,22 @@ load_preview_binaries(struct ia_css_pipe *pipe)
>  			return err;
>  	}
>  
> -#ifdef ISP2401
> -	/*
> -	 * When the input system is 2401, only the Direct Sensor Mode
> -	 * Offline Preview uses the ISP copy binary.
> -	 */
> -	need_isp_copy_binary = !online && sensor;
> -#else
> -	/*
> -	 * About pipe->stream->config.mode == IA_CSS_INPUT_MODE_MEMORY:
> -	 * This is typical the case with SkyCam (which has no input system) but it also applies to all cases
> -	 * where the driver chooses for memory based input frames. In these cases, a copy binary (which typical
> -	 * copies sensor data to DDR) does not have much use.
> -	 */
> -	if (!IS_ISP2401)
> +	if (IS_ISP2401) {
> +		/*
> +		 * When the input system is 2401, only the Direct Sensor Mode
> +		 * Offline Preview uses the ISP copy binary.
> +		 */
> +		need_isp_copy_binary = !online && sensor;
> +	} else {
> +		/*
> +		 * About pipe->stream->config.mode == IA_CSS_INPUT_MODE_MEMORY:
> +		 * This is typical the case with SkyCam (which has no input system) but it also
> +		 * applies to all cases where the driver chooses for memory based input frames.
> +		 * In these cases, a copy binary (which typical copies sensor data to DDR) does
> +		 * not have much use.
> +		 */
>  		need_isp_copy_binary = !online && !continuous;
> -	else
> -		need_isp_copy_binary = !online && !continuous && !(pipe->stream->config.mode == IA_CSS_INPUT_MODE_MEMORY);
> -#endif
> +	}
>  
>  	/* Copy */
>  	if (need_isp_copy_binary) {
> @@ -3125,11 +3117,10 @@ init_in_frameinfo_memory_defaults(struct ia_css_pipe *pipe,
>  
>  	in_frame->frame_info.format = format;
>  
> -#ifdef ISP2401
> -	if (format == IA_CSS_FRAME_FORMAT_RAW)
> +	if (IS_ISP2401 && format == IA_CSS_FRAME_FORMAT_RAW) {
>  		in_frame->frame_info.format = (pipe->stream->config.pack_raw_pixels) ?
>  		IA_CSS_FRAME_FORMAT_RAW_PACKED : IA_CSS_FRAME_FORMAT_RAW;
> -#endif
> +	}
>  
>  	in_frame->frame_info.res.width = pipe->stream->config.input_config.input_res.width;
>  	in_frame->frame_info.res.height = pipe->stream->config.input_config.input_res.height;
> @@ -3211,18 +3202,18 @@ static int create_host_video_pipeline(struct ia_css_pipe *pipe)
>  
>  	me->dvs_frame_delay = pipe->dvs_frame_delay;
>  
> -#ifdef ISP2401
> -	/*
> -	 * When the input system is 2401, always enable 'in_frameinfo_memory'
> -	 * except for the following: online or continuous
> -	 */
> -	need_in_frameinfo_memory = !(pipe->stream->config.online ||
> -				     pipe->stream->config.continuous);
> -#else
> -	/* Construct in_frame info (only in case we have dynamic input */
> -	need_in_frameinfo_memory = pipe->stream->config.mode ==
> -				   IA_CSS_INPUT_MODE_MEMORY;
> -#endif
> +	if (IS_ISP2401) {
> +		/*
> +		 * When the input system is 2401, always enable 'in_frameinfo_memory'
> +		 * except for the following: online or continuous
> +		 */
> +		need_in_frameinfo_memory = !(pipe->stream->config.online ||
> +					     pipe->stream->config.continuous);
> +	} else {
> +		/* Construct in_frame info (only in case we have dynamic input */
> +		need_in_frameinfo_memory = pipe->stream->config.mode ==
> +					   IA_CSS_INPUT_MODE_MEMORY;
> +	}
>  
>  	/* Construct in_frame info (only in case we have dynamic input */
>  	if (need_in_frameinfo_memory) {
> @@ -3268,15 +3259,14 @@ static int create_host_video_pipeline(struct ia_css_pipe *pipe)
>  			goto ERR;
>  		in_frame = me->stages->args.out_frame[0];
>  	} else if (pipe->stream->config.continuous) {
> -#ifdef ISP2401
> -		/*
> -		 * When continuous is enabled, configure in_frame with the
> -		 * last pipe, which is the copy pipe.
> -		 */
> -		in_frame = pipe->stream->last_pipe->continuous_frames[0];
> -#else
> -		in_frame = pipe->continuous_frames[0];
> -#endif
> +		if (IS_ISP2401)
> +			/*
> +			 * When continuous is enabled, configure in_frame with the
> +			 * last pipe, which is the copy pipe.
> +			 */
> +			in_frame = pipe->stream->last_pipe->continuous_frames[0];
> +		else
> +			in_frame = pipe->continuous_frames[0];
>  	}
>  
>  	ia_css_pipe_util_set_output_frames(out_frames, 0,
> @@ -3373,12 +3363,10 @@ create_host_preview_pipeline(struct ia_css_pipe *pipe)
>  	struct ia_css_frame *out_frame;
>  	struct ia_css_frame *out_frames[IA_CSS_BINARY_MAX_OUTPUT_PORTS];
>  	bool need_in_frameinfo_memory = false;
> -#ifdef ISP2401
>  	bool sensor = false;
>  	bool buffered_sensor = false;
>  	bool online = false;
>  	bool continuous = false;
> -#endif
>  
>  	IA_CSS_ENTER_PRIVATE("pipe = %p", pipe);
>  	if ((!pipe) || (!pipe->stream) || (pipe->mode != IA_CSS_PIPE_ID_PREVIEW)) {
> @@ -3391,25 +3379,26 @@ create_host_preview_pipeline(struct ia_css_pipe *pipe)
>  	me = &pipe->pipeline;
>  	ia_css_pipeline_clean(me);
>  
> -#ifdef ISP2401
> -	/*
> -	 * When the input system is 2401, always enable 'in_frameinfo_memory'
> -	 * except for the following:
> -	 * - Direct Sensor Mode Online Preview
> -	 * - Buffered Sensor Mode Online Preview
> -	 * - Direct Sensor Mode Continuous Preview
> -	 * - Buffered Sensor Mode Continuous Preview
> -	 */
> -	sensor = (pipe->stream->config.mode == IA_CSS_INPUT_MODE_SENSOR);
> -	buffered_sensor = (pipe->stream->config.mode == IA_CSS_INPUT_MODE_BUFFERED_SENSOR);
> -	online = pipe->stream->config.online;
> -	continuous = pipe->stream->config.continuous;
> -	need_in_frameinfo_memory =
> -	!((sensor && (online || continuous)) || (buffered_sensor && (online || continuous)));
> -#else
> -	/* Construct in_frame info (only in case we have dynamic input */
> -	need_in_frameinfo_memory = pipe->stream->config.mode == IA_CSS_INPUT_MODE_MEMORY;
> -#endif
> +	if (IS_ISP2401) {
> +		/*
> +		 * When the input system is 2401, always enable 'in_frameinfo_memory'
> +		 * except for the following:
> +		 * - Direct Sensor Mode Online Preview
> +		 * - Buffered Sensor Mode Online Preview
> +		 * - Direct Sensor Mode Continuous Preview
> +		 * - Buffered Sensor Mode Continuous Preview
> +		 */
> +		sensor = (pipe->stream->config.mode == IA_CSS_INPUT_MODE_SENSOR);
> +		buffered_sensor = (pipe->stream->config.mode == IA_CSS_INPUT_MODE_BUFFERED_SENSOR);
> +		online = pipe->stream->config.online;
> +		continuous = pipe->stream->config.continuous;
> +		need_in_frameinfo_memory =
> +		!((sensor && (online || continuous)) || (buffered_sensor &&
> +							(online || continuous)));
> +	} else {
> +		/* Construct in_frame info (only in case we have dynamic input */
> +		need_in_frameinfo_memory = pipe->stream->config.mode == IA_CSS_INPUT_MODE_MEMORY;
> +	}
>  	if (need_in_frameinfo_memory) {
>  		err = init_in_frameinfo_memory_defaults(pipe, &me->in_frame,
>  							IA_CSS_FRAME_FORMAT_RAW);
> @@ -3420,7 +3409,6 @@ create_host_preview_pipeline(struct ia_css_pipe *pipe)
>  	} else {
>  		in_frame = NULL;
>  	}
> -
>  	err = init_out_frameinfo_defaults(pipe, &me->out_frame[0], 0);
>  	if (err)
>  		goto ERR;
> @@ -3441,17 +3429,16 @@ create_host_preview_pipeline(struct ia_css_pipe *pipe)
>  			goto ERR;
>  		in_frame = me->stages->args.out_frame[0];
>  	} else if (pipe->stream->config.continuous) {
> -#ifdef ISP2401
> -		/*
> -		 * When continuous is enabled, configure in_frame with the
> -		 * last pipe, which is the copy pipe.
> -		 */
> -		if (continuous || !online)
> -			in_frame = pipe->stream->last_pipe->continuous_frames[0];
> -
> -#else
> -		in_frame = pipe->continuous_frames[0];
> -#endif
> +		if (IS_ISP2401) {
> +			/*
> +			 * When continuous is enabled, configure in_frame with the
> +			 * last pipe, which is the copy pipe.
> +			 */
> +			if (continuous || !online)
> +				in_frame = pipe->stream->last_pipe->continuous_frames[0];
> +		} else {
> +			in_frame = pipe->continuous_frames[0];
> +		}
>  	}
>  
>  	if (vf_pp_binary) {
> @@ -3925,19 +3912,19 @@ ia_css_pipe_dequeue_buffer(struct ia_css_pipe *pipe,
>  			case IA_CSS_BUFFER_TYPE_OUTPUT_FRAME:
>  			case IA_CSS_BUFFER_TYPE_SEC_OUTPUT_FRAME:
>  				if (pipe && pipe->stop_requested) {
> -#if !defined(ISP2401)
> -					/*
> -					 * free mipi frames only for old input
> -					 * system for 2401 it is done in
> -					 * ia_css_stream_destroy call
> -					 */
> -					return_err = free_mipi_frames(pipe);
> -					if (return_err) {
> -						IA_CSS_LOG("free_mipi_frames() failed");
> -						IA_CSS_LEAVE_ERR(return_err);
> -						return return_err;
> +					if (!IS_ISP2401) {
> +						/*
> +						 * free mipi frames only for old input
> +						 * system for 2401 it is done in
> +						 * ia_css_stream_destroy call
> +						 */
> +						return_err = free_mipi_frames(pipe);
> +						if (return_err) {
> +							IA_CSS_LOG("free_mipi_frames() failed");
> +							IA_CSS_LEAVE_ERR(return_err);
> +							return return_err;
> +						}
>  					}
> -#endif
>  					pipe->stop_requested = false;
>  				}
>  				fallthrough;
> @@ -3959,12 +3946,11 @@ ia_css_pipe_dequeue_buffer(struct ia_css_pipe *pipe,
>  					pipe->num_invalid_frames--;
>  
>  				if (frame->frame_info.format == IA_CSS_FRAME_FORMAT_BINARY_8) {
> -#ifdef ISP2401
> -					frame->planes.binary.size = frame->data_bytes;
> -#else
> -					frame->planes.binary.size =
> -					    sh_css_sp_get_binary_copy_size();
> -#endif
> +					if (IS_ISP2401)
> +						frame->planes.binary.size = frame->data_bytes;
> +					else
> +						frame->planes.binary.size =
> +						    sh_css_sp_get_binary_copy_size();
>  				}
>  				if (buf_type == IA_CSS_BUFFER_TYPE_OUTPUT_FRAME) {
>  					IA_CSS_LOG("pfp: dequeued OF %d with config id %d thread %d",
> @@ -4880,22 +4866,20 @@ static int load_video_binaries(struct ia_css_pipe *pipe)
>  			    pipe->num_invalid_frames, pipe->dvs_frame_delay);
>  
>  	/* pqiao TODO: temp hack for PO, should be removed after offline YUVPP is enabled */
> -#if !defined(ISP2401)
> -	/* Copy */
> -	if (!online && !continuous) {
> -		/*
> -		 * TODO: what exactly needs doing, prepend the copy binary to
> -		 *	 video base this only on !online?
> -		 */
> -		err = load_copy_binary(pipe,
> -				       &mycs->copy_binary,
> -				       &mycs->video_binary);
> -		if (err)
> -			return err;
> +	if (!IS_ISP2401) {
> +		/* Copy */
> +		if (!online && !continuous) {
> +			/*
> +			 * TODO: what exactly needs doing, prepend the copy binary to
> +			 *	 video base this only on !online?
> +			 */
> +			err = load_copy_binary(pipe,
> +					       &mycs->copy_binary,
> +					       &mycs->video_binary);
> +			if (err)
> +				return err;
> +		}
>  	}
> -#else
> -	(void)continuous;
> -#endif
>  
>  	if (pipe->enable_viewfinder[IA_CSS_PIPE_OUTPUT_STAGE_0] && need_vf_pp) {
>  		struct ia_css_binary_descr vf_pp_descr;
> @@ -5227,11 +5211,8 @@ static int load_primary_binaries(
>  	bool need_pp = false;
>  	bool need_isp_copy_binary = false;
>  	bool need_ldc = false;
> -#ifdef ISP2401
>  	bool sensor = false;
> -#else
>  	bool memory, continuous;
> -#endif
>  	struct ia_css_frame_info prim_in_info,
>  		       prim_out_info,
>  		       capt_pp_out_info, vf_info,
> @@ -5251,12 +5232,9 @@ static int load_primary_binaries(
>  	       pipe->mode == IA_CSS_PIPE_ID_COPY);
>  
>  	online = pipe->stream->config.online;
> -#ifdef ISP2401
>  	sensor = (pipe->stream->config.mode == IA_CSS_INPUT_MODE_SENSOR);
> -#else
>  	memory = pipe->stream->config.mode == IA_CSS_INPUT_MODE_MEMORY;
>  	continuous = pipe->stream->config.continuous;
> -#endif
>  
>  	mycs = &pipe->pipe_settings.capture;
>  	pipe_out_info = &pipe->output_info[0];
> @@ -5462,15 +5440,14 @@ static int load_primary_binaries(
>  	if (err)
>  		return err;
>  
> -#ifdef ISP2401
> -	/*
> -	 * When the input system is 2401, only the Direct Sensor Mode
> -	 * Offline Capture uses the ISP copy binary.
> -	 */
> -	need_isp_copy_binary = !online && sensor;
> -#else
> -	need_isp_copy_binary = !online && !continuous && !memory;
> -#endif
> +	if (IS_ISP2401)
> +		/*
> +		 * When the input system is 2401, only the Direct Sensor Mode
> +		 * Offline Capture uses the ISP copy binary.
> +		 */
> +		need_isp_copy_binary = !online && sensor;
> +	else
> +		need_isp_copy_binary = !online && !continuous && !memory;
>  
>  	/* ISP Copy */
>  	if (need_isp_copy_binary) {
> @@ -5681,10 +5658,10 @@ static int load_advanced_binaries(struct ia_css_pipe *pipe)
>  	}
>  
>  	/* Copy */
> -#ifdef ISP2401
> -	/* For CSI2+, only the direct sensor mode/online requires ISP copy */
> -	need_isp_copy = pipe->stream->config.mode == IA_CSS_INPUT_MODE_SENSOR;
> -#endif
> +	if (IS_ISP2401)
> +		/* For CSI2+, only the direct sensor mode/online requires ISP copy */
> +		need_isp_copy = pipe->stream->config.mode == IA_CSS_INPUT_MODE_SENSOR;
> +
>  	if (need_isp_copy)
>  		load_copy_binary(pipe,
>  				 &pipe->pipe_settings.capture.copy_binary,
> @@ -5829,10 +5806,10 @@ static int load_low_light_binaries(struct ia_css_pipe *pipe)
>  	}
>  
>  	/* Copy */
> -#ifdef ISP2401
> -	/* For CSI2+, only the direct sensor mode/online requires ISP copy */
> -	need_isp_copy = pipe->stream->config.mode == IA_CSS_INPUT_MODE_SENSOR;
> -#endif
> +	if (IS_ISP2401)
> +		/* For CSI2+, only the direct sensor mode/online requires ISP copy */
> +		need_isp_copy = pipe->stream->config.mode == IA_CSS_INPUT_MODE_SENSOR;
> +
>  	if (need_isp_copy)
>  		err = load_copy_binary(pipe,
>  				       &pipe->pipe_settings.capture.copy_binary,
> @@ -5902,10 +5879,9 @@ static int load_capture_binaries(struct ia_css_pipe *pipe)
>  	switch (pipe->config.default_capture_config.mode) {
>  	case IA_CSS_CAPTURE_MODE_RAW:
>  		err = load_copy_binaries(pipe);
> -#if defined(ISP2401)
> -		if (!err)
> +		if (!err && IS_ISP2401)
>  			pipe->pipe_settings.capture.copy_binary.online = pipe->stream->config.online;
> -#endif
> +
>  		break;
>  	case IA_CSS_CAPTURE_MODE_BAYER:
>  		err = load_bayer_isp_binaries(pipe);
> @@ -6409,7 +6385,6 @@ load_yuvpp_binaries(struct ia_css_pipe *pipe)
>  	else
>  		next_binary = NULL;
>  
> -#if defined(ISP2401)
>  	/*
>  	 * NOTES
>  	 * - Why does the "yuvpp" pipe needs "isp_copy_binary" (i.e. ISP Copy) when
> @@ -6427,11 +6402,11 @@ load_yuvpp_binaries(struct ia_css_pipe *pipe)
>  	 *   pp_defs.h" for the list of input-frame formats that are supported by the
>  	 *   "yuv_scale_binary".
>  	 */
> -	need_isp_copy_binary =
> -	    (pipe->stream->config.input_config.format == ATOMISP_INPUT_FORMAT_YUV422_8);
> -#else  /* !ISP2401 */
> -	need_isp_copy_binary = true;
> -#endif /*  ISP2401 */
> +	if (IS_ISP2401)
> +		need_isp_copy_binary =
> +		    (pipe->stream->config.input_config.format == ATOMISP_INPUT_FORMAT_YUV422_8);
> +	else
> +		need_isp_copy_binary = true;
>  
>  	if (need_isp_copy_binary) {
>  		err = load_copy_binary(pipe,
> @@ -6678,12 +6653,10 @@ create_host_yuvpp_pipeline(struct ia_css_pipe *pipe)
>  	struct ia_css_frame *vf_frame[IA_CSS_PIPE_MAX_OUTPUT_STAGE];
>  	struct ia_css_pipeline_stage_desc stage_desc;
>  	bool need_in_frameinfo_memory = false;
> -#ifdef ISP2401
>  	bool sensor = false;
>  	bool buffered_sensor = false;
>  	bool online = false;
>  	bool continuous = false;
> -#endif
>  
>  	IA_CSS_ENTER_PRIVATE("pipe = %p", pipe);
>  	if ((!pipe) || (!pipe->stream) || (pipe->mode != IA_CSS_PIPE_ID_YUVPP)) {
> @@ -6700,24 +6673,24 @@ create_host_yuvpp_pipeline(struct ia_css_pipe *pipe)
>  	num_stage  = pipe->pipe_settings.yuvpp.num_yuv_scaler;
>  	num_output_stage   = pipe->pipe_settings.yuvpp.num_output;
>  
> -#ifdef ISP2401
> -	/*
> -	 * When the input system is 2401, always enable 'in_frameinfo_memory'
> -	 * except for the following:
> -	 * - Direct Sensor Mode Online Capture
> -	 * - Direct Sensor Mode Continuous Capture
> -	 * - Buffered Sensor Mode Continuous Capture
> -	 */
> -	sensor = pipe->stream->config.mode == IA_CSS_INPUT_MODE_SENSOR;
> -	buffered_sensor = pipe->stream->config.mode == IA_CSS_INPUT_MODE_BUFFERED_SENSOR;
> -	online = pipe->stream->config.online;
> -	continuous = pipe->stream->config.continuous;
> -	need_in_frameinfo_memory =
> -	!((sensor && (online || continuous)) || (buffered_sensor && continuous));
> -#else
> -	/* Construct in_frame info (only in case we have dynamic input */
> -	need_in_frameinfo_memory = pipe->stream->config.mode == IA_CSS_INPUT_MODE_MEMORY;
> -#endif
> +	if (IS_ISP2401) {
> +		/*
> +		 * When the input system is 2401, always enable 'in_frameinfo_memory'
> +		 * except for the following:
> +		 * - Direct Sensor Mode Online Capture
> +		 * - Direct Sensor Mode Continuous Capture
> +		 * - Buffered Sensor Mode Continuous Capture
> +		 */
> +		sensor = pipe->stream->config.mode == IA_CSS_INPUT_MODE_SENSOR;
> +		buffered_sensor = pipe->stream->config.mode == IA_CSS_INPUT_MODE_BUFFERED_SENSOR;
> +		online = pipe->stream->config.online;
> +		continuous = pipe->stream->config.continuous;
> +		need_in_frameinfo_memory =
> +		!((sensor && (online || continuous)) || (buffered_sensor && continuous));
> +	} else {
> +		/* Construct in_frame info (only in case we have dynamic input */
> +		need_in_frameinfo_memory = pipe->stream->config.mode == IA_CSS_INPUT_MODE_MEMORY;
> +	}
>  	/*
>  	 * the input frame can come from:
>  	 *
> @@ -6808,11 +6781,10 @@ create_host_yuvpp_pipeline(struct ia_css_pipe *pipe)
>  	if (pipe->pipe_settings.yuvpp.copy_binary.info) {
>  		struct ia_css_frame *in_frame_local = NULL;
>  
> -#ifdef ISP2401
> -		/* After isp copy is enabled in_frame needs to be passed. */
> -		if (!online)
> +		if (IS_ISP2401 && !online) {
> +			/* After isp copy is enabled in_frame needs to be passed. */
>  			in_frame_local = in_frame;
> -#endif
> +		}
>  
>  		if (need_scaler) {
>  			ia_css_pipe_util_set_output_frames(bin_out_frame,
> @@ -7031,12 +7003,10 @@ create_host_regular_capture_pipeline(struct ia_css_pipe *pipe)
>  	struct ia_css_frame *vf_frame;
>  	struct ia_css_pipeline_stage_desc stage_desc;
>  	bool need_in_frameinfo_memory = false;
> -#ifdef ISP2401
>  	bool sensor = false;
>  	bool buffered_sensor = false;
>  	bool online = false;
>  	bool continuous = false;
> -#endif
>  	unsigned int i, num_yuv_scaler, num_primary_stage;
>  	bool need_yuv_pp = false;
>  	bool *is_output_stage = NULL;
> @@ -7054,25 +7024,27 @@ create_host_regular_capture_pipeline(struct ia_css_pipe *pipe)
>  	ia_css_pipeline_clean(me);
>  	ia_css_pipe_util_create_output_frames(out_frames);
>  
> -#ifdef ISP2401
> -	/*
> -	 * When the input system is 2401, always enable 'in_frameinfo_memory'
> -	 * except for the following:
> -	 * - Direct Sensor Mode Online Capture
> -	 * - Direct Sensor Mode Online Capture
> -	 * - Direct Sensor Mode Continuous Capture
> -	 * - Buffered Sensor Mode Continuous Capture
> -	 */
> -	sensor = (pipe->stream->config.mode == IA_CSS_INPUT_MODE_SENSOR);
> -	buffered_sensor = (pipe->stream->config.mode == IA_CSS_INPUT_MODE_BUFFERED_SENSOR);
> -	online = pipe->stream->config.online;
> -	continuous = pipe->stream->config.continuous;
> -	need_in_frameinfo_memory =
> -	!((sensor && (online || continuous)) || (buffered_sensor && (online || continuous)));
> -#else
> -	/* Construct in_frame info (only in case we have dynamic input */
> -	need_in_frameinfo_memory = pipe->stream->config.mode == IA_CSS_INPUT_MODE_MEMORY;
> -#endif
> +	if (IS_ISP2401) {
> +		/*
> +		 * When the input system is 2401, always enable 'in_frameinfo_memory'
> +		 * except for the following:
> +		 * - Direct Sensor Mode Online Capture
> +		 * - Direct Sensor Mode Online Capture
> +		 * - Direct Sensor Mode Continuous Capture
> +		 * - Buffered Sensor Mode Continuous Capture
> +		 */
> +		sensor = (pipe->stream->config.mode == IA_CSS_INPUT_MODE_SENSOR);
> +		buffered_sensor = (pipe->stream->config.mode == IA_CSS_INPUT_MODE_BUFFERED_SENSOR);
> +		online = pipe->stream->config.online;
> +		continuous = pipe->stream->config.continuous;
> +		need_in_frameinfo_memory =
> +		!((sensor && (online || continuous)) || (buffered_sensor &&
> +							(online || continuous)));
> +	} else {
> +		/* Construct in_frame info (only in case we have dynamic input */
> +		need_in_frameinfo_memory = pipe->stream->config.mode == IA_CSS_INPUT_MODE_MEMORY;
> +	}
> +
>  	if (need_in_frameinfo_memory) {
>  		err = init_in_frameinfo_memory_defaults(pipe, &me->in_frame,
>  							IA_CSS_FRAME_FORMAT_RAW);
> @@ -7135,27 +7107,27 @@ create_host_regular_capture_pipeline(struct ia_css_pipe *pipe)
>  	if (pipe->pipe_settings.capture.copy_binary.info) {
>  		if (raw) {
>  			ia_css_pipe_util_set_output_frames(out_frames, 0, out_frame);
> -#if defined(ISP2401)
> -			if (!continuous) {
> -				ia_css_pipe_get_generic_stage_desc(&stage_desc,
> -								   copy_binary,
> -								   out_frames,
> -								   in_frame,
> -								   NULL);
> +			if (IS_ISP2401) {
> +				if (!continuous) {
> +					ia_css_pipe_get_generic_stage_desc(&stage_desc,
> +									   copy_binary,
> +									   out_frames,
> +									   in_frame,
> +									   NULL);
> +				} else {
> +					in_frame = pipe->stream->last_pipe->continuous_frames[0];
> +					ia_css_pipe_get_generic_stage_desc(&stage_desc,
> +									   copy_binary,
> +									   out_frames,
> +									   in_frame,
> +									   NULL);
> +				}
>  			} else {
> -				in_frame = pipe->stream->last_pipe->continuous_frames[0];
>  				ia_css_pipe_get_generic_stage_desc(&stage_desc,
>  								   copy_binary,
>  								   out_frames,
> -								   in_frame,
> -								   NULL);
> +								   NULL, NULL);
>  			}
> -#else
> -			ia_css_pipe_get_generic_stage_desc(&stage_desc,
> -							   copy_binary,
> -							   out_frames,
> -							   NULL, NULL);
> -#endif
>  		} else {
>  			ia_css_pipe_util_set_output_frames(out_frames, 0,
>  							   in_frame);
> @@ -7185,11 +7157,7 @@ create_host_regular_capture_pipeline(struct ia_css_pipe *pipe)
>  				local_in_frame = in_frame;
>  			else
>  				local_in_frame = NULL;
> -#ifndef ISP2401
> -			if (!need_pp && (i == num_primary_stage - 1))
> -#else
> -			if (!need_pp && (i == num_primary_stage - 1) && !need_ldc)
> -#endif
> +			if (!need_pp && (i == num_primary_stage - 1) && (!IS_ISP2401 || !need_ldc))
>  				local_out_frame = out_frame;
>  			else
>  				local_out_frame = NULL;
> @@ -7400,23 +7368,14 @@ static int capture_start(struct ia_css_pipe *pipe)
>  			return err;
>  		}
>  	}
> -
> -#if !defined(ISP2401)
>  	/* old isys: need to send_mipi_frames() in all pipe modes */
> -	err = send_mipi_frames(pipe);
> -	if (err) {
> -		IA_CSS_LEAVE_ERR_PRIVATE(err);
> -		return err;
> -	}
> -#else
> -	if (pipe->config.mode != IA_CSS_PIPE_MODE_COPY) {
> +	if (!IS_ISP2401 || (IS_ISP2401 && pipe->config.mode != IA_CSS_PIPE_MODE_COPY)) {
>  		err = send_mipi_frames(pipe);
>  		if (err) {
>  			IA_CSS_LEAVE_ERR_PRIVATE(err);
>  			return err;
>  		}
>  	}
> -#endif
>  
>  	ia_css_pipeline_get_sp_thread_id(ia_css_pipe_get_pipe_num(pipe), &thread_id);
>  	copy_ovrd = 1 << thread_id;
> @@ -8123,24 +8082,22 @@ ia_css_stream_create(const struct ia_css_stream_config *stream_config,
>  		return err;
>  	}
>  
> -#if !defined(ISP2401)
> -	/* We don't support metadata for JPEG stream, since they both use str2mem */
> -	if (stream_config->input_config.format == ATOMISP_INPUT_FORMAT_BINARY_8 &&
> -	    stream_config->metadata_config.resolution.height > 0) {
> -		err = -EINVAL;
> -		IA_CSS_LEAVE_ERR(err);
> -		return err;
> -	}
> -#endif
> -
> -#ifdef ISP2401
> -	if (stream_config->online && stream_config->pack_raw_pixels) {
> -		IA_CSS_LOG("online and pack raw is invalid on input system 2401");
> -		err = -EINVAL;
> -		IA_CSS_LEAVE_ERR(err);
> -		return err;
> +	if (!IS_ISP2401) {
> +		/* We don't support metadata for JPEG stream, since they both use str2mem */
> +		if (stream_config->input_config.format == ATOMISP_INPUT_FORMAT_BINARY_8 &&
> +		    stream_config->metadata_config.resolution.height > 0) {
> +			err = -EINVAL;
> +			IA_CSS_LEAVE_ERR(err);
> +			return err;
> +		}
> +	} else {
> +		if (stream_config->online && stream_config->pack_raw_pixels) {
> +			IA_CSS_LOG("online and pack raw is invalid on input system 2401");
> +			err = -EINVAL;
> +			IA_CSS_LEAVE_ERR(err);
> +			return err;
> +		}
>  	}
> -#endif
>  
>  	ia_css_debug_pipe_graph_dump_stream_config(stream_config);
>  
> @@ -8223,19 +8180,17 @@ ia_css_stream_create(const struct ia_css_stream_config *stream_config,
>  	/* take over stream config */
>  	curr_stream->config = *stream_config;
>  
> -#if defined(ISP2401)
> -	if (stream_config->mode == IA_CSS_INPUT_MODE_BUFFERED_SENSOR &&
> -	    stream_config->online)
> -		curr_stream->config.online = false;
> -#endif
> +	if (IS_ISP2401) {
> +		if (stream_config->mode == IA_CSS_INPUT_MODE_BUFFERED_SENSOR &&
> +		    stream_config->online)
> +			curr_stream->config.online = false;
>  
> -#ifdef ISP2401
> -	if (curr_stream->config.online) {
> -		curr_stream->config.source.port.num_lanes =
> -		    stream_config->source.port.num_lanes;
> -		curr_stream->config.mode =  IA_CSS_INPUT_MODE_BUFFERED_SENSOR;
> +		if (curr_stream->config.online) {
> +			curr_stream->config.source.port.num_lanes =
> +			    stream_config->source.port.num_lanes;
> +			curr_stream->config.mode =  IA_CSS_INPUT_MODE_BUFFERED_SENSOR;
> +		}
>  	}
> -#endif
>  	/* in case driver doesn't configure init number of raw buffers, configure it here */
>  	if (curr_stream->config.target_num_cont_raw_buf == 0)
>  		curr_stream->config.target_num_cont_raw_buf = NUM_CONTINUOUS_FRAMES;
> @@ -9162,11 +9117,10 @@ void ia_css_pipe_map_queue(struct ia_css_pipe *pipe, bool map)
>  
>  	ia_css_pipeline_get_sp_thread_id(pipe_num, &thread_id);
>  
> -#if defined(ISP2401)
> -	need_input_queue = true;
> -#else
> -	need_input_queue = pipe->stream->config.mode == IA_CSS_INPUT_MODE_MEMORY;
> -#endif
> +	if (IS_ISP2401)
> +		need_input_queue = true;
> +	else
> +		need_input_queue = pipe->stream->config.mode == IA_CSS_INPUT_MODE_MEMORY;
>  
>  	/* map required buffer queues to resources */
>  	/* TODO: to be improved */


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 5/5] staging: media: atomisp: sh_css_mipi: Remove #ifdef 2041
  2023-05-10 18:34   ` Hans de Goede
@ 2023-05-11 11:18     ` Kate Hsuan
  0 siblings, 0 replies; 8+ messages in thread
From: Kate Hsuan @ 2023-05-11 11:18 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Mauro Carvalho Chehab, Sakari Ailus, Greg Kroah-Hartman,
	linux-media, linux-staging

Hi Hans,

On Thu, May 11, 2023 at 2:34 AM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi Kate,
>
> On 5/8/23 08:26, Kate Hsuan wrote:
> > The actions of ISP2401 and 2400 are determined at the runtime.
> >
> > Signed-off-by: Kate Hsuan <hpa@redhat.com>
> > ---
> >  .../staging/media/atomisp/pci/sh_css_mipi.c   | 65 ++++++-------------
> >  1 file changed, 20 insertions(+), 45 deletions(-)
> >
> > diff --git a/drivers/staging/media/atomisp/pci/sh_css_mipi.c b/drivers/staging/media/atomisp/pci/sh_css_mipi.c
> > index bc6e8598a776..52a1ed63e9a5 100644
> > --- a/drivers/staging/media/atomisp/pci/sh_css_mipi.c
> > +++ b/drivers/staging/media/atomisp/pci/sh_css_mipi.c
> > @@ -386,30 +381,22 @@ allocate_mipi_frames(struct ia_css_pipe *pipe,
> >               return -EINVAL;
> >       }
> >
> > -#ifdef ISP2401
> > -     err = calculate_mipi_buff_size(&pipe->stream->config,
> > -                                    &my_css.mipi_frame_size[port]);
>
> Before you changes this code always run ISP2401, now it only runs
> when (ref_count_mipi_allocation[port] != 0) is not true.
>
> So this statement should stay here in the code, just prefixed
> with a if (IS_ISP2401) condition.
>
> > -     /*
> > -      * 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.
> > -      */
> > -     if (ref_count_mipi_allocation[port] != 0) {
> > -             ref_count_mipi_allocation[port]++;
> > -             ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE_PRIVATE,
> > -                                 "allocate_mipi_frames(%p) leave: nothing to do, already allocated for this port (port=%d).\n",
> > -                                 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;
> > +     } else {
> > +             /*
> > +              * 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.
> > +              */
>
> This comment block actually belongs to the if (ref_count_mipi_allocation[port] != 0)
> check, the code executed if that check passes was actually different between
> the ISP2400 and ISP2401 (my bad, sorry). The ISP2401 case did an extra:
>
>                 ref_count_mipi_allocation[port]++;
>
> when (ref_count_mipi_allocation[port] != 0), so we need to add:
>
>                 if (IS_ISP2401)
>                         ref_count_mipi_allocation[port]++;
>
> above the return 0 above.
>
> > +             if (IS_ISP2401)
> > +                     err = calculate_mipi_buff_size(&pipe->stream->config,
> > +                                                    &my_css.mipi_frame_size[port]);
>
> I have fixed this all up while merging your series and the new
> diff for this code-block now looks like this:
>
> @@ -386,9 +381,10 @@ allocate_mipi_frames(struct ia_css_pipe *pipe,
>                 return -EINVAL;
>         }
>
> -#ifdef ISP2401
> -       err = calculate_mipi_buff_size(&pipe->stream->config,
> -                                      &my_css.mipi_frame_size[port]);
> +       if (IS_ISP2401)
> +               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.
> @@ -396,20 +392,14 @@ allocate_mipi_frames(struct ia_css_pipe *pipe,
>          * In that case only 2400 related code should remain.
>          */
>         if (ref_count_mipi_allocation[port] != 0) {
> -               ref_count_mipi_allocation[port]++;
> +               if (IS_ISP2401)
> +                       ref_count_mipi_allocation[port]++;
> +
>                 ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE_PRIVATE,
>                                     "allocate_mipi_frames(%p) leave: nothing to do, already allocated for this port (port=%d).\n",
>                                     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]++;
>
>
>
> Regards,
>
> Hans
>

It's my bad when trying to simplify the if expressions.
Thank you for fixing this. :)

-- 
BR,
Kate


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2023-05-11 11:18 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-08  6:26 [PATCH v2 1/5] staging: media: atomisp: sh_css: Remove #ifdef ISP2401 Kate Hsuan
2023-05-08  6:26 ` [PATCH v2 2/5] staging: media: atomisp: runtime: frame: remove " Kate Hsuan
2023-05-08  6:26 ` [PATCH v2 3/5] staging: media: atomisp: sh_css_sp: Remove " Kate Hsuan
2023-05-08  6:26 ` [PATCH v2 4/5] staging: media: atomisp: sh_css_firmware: determine firmware version at runtime Kate Hsuan
2023-05-08  6:26 ` [PATCH v2 5/5] staging: media: atomisp: sh_css_mipi: Remove #ifdef 2041 Kate Hsuan
2023-05-10 18:34   ` Hans de Goede
2023-05-11 11:18     ` Kate Hsuan
2023-05-10 18:40 ` [PATCH v2 1/5] staging: media: atomisp: sh_css: Remove #ifdef ISP2401 Hans de Goede

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.