public inbox for linux-staging@lists.linux.dev
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox