staging: media: atomisp: Remove #ifdef 2401
Subsystems (4)
cover staging: media: atomisp: Remove #ifdef 2401
Since #ifdef ISP2401 is used to determine the action of both models in compiler time, we have to provide two binaries for both models. It is very unfriendly for the users and for the package management aspect.
The proposed patch removed the #ifdef ISP2041 from the codes and made the path for both models can be determined at the runtime. Some of the #ifdef is highly integrated with functions and data structures. If we try to remove them, it will cause many issues, such as duplicated variable/function name and data length. Therefore, these patches focus on removing the #ifdef without affecting the change of structure.
Kate Hsuan (5):
staging: media: atomisp: sh_css: Remove #ifdef ISP2401
staging: media: atomisp: runtime: frame: remove #ifdef ISP2401
staging: media: atomisp: sh_css_sp: Remove #ifdef ISP2401
staging: media: atomisp: sh_css_firmware: determine firmware version
at runtime
staging: media: atomisp: sh_css_mipi: Remove #ifdef 2041
.../atomisp/pci/runtime/frame/src/frame.c | 15 +- drivers/staging/media/atomisp/pci/sh_css.c | 584 +++++++++--------- .../media/atomisp/pci/sh_css_firmware.c | 18 +- .../staging/media/atomisp/pci/sh_css_mipi.c | 101 ++- drivers/staging/media/atomisp/pci/sh_css_sp.c | 10 +- 5 files changed, 359 insertions(+), 369 deletions(-)
Signature · 1 line
2.39.2
PATCH 1/5 staging: media: atomisp: sh_css: Remove #ifdef ISP2401
The actions of ISP2401 and 2400 are determined at the runtime.
--- drivers/staging/media/atomisp/pci/sh_css.c | 584 ++++++++++----------- 1 file changed, 286 insertions(+), 298 deletions(-)
drivers/staging/media/atomisp/pci/sh_css.c +286 -298
@@ -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,13 @@ 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 +if (IS_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); +ia_css_frame_free_multiple (MAX_NUM_VIDEO_DELAY_FRAMES , pipe->pipe_settings.video.delay_frames);break ;
@@ -2238,11 +2235,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 +2273,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 +2416,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 +2502,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 +2515,11 @@ 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 (IS_ISP2401 ) + sensor = pipe->stream->config.mode ==IA_CSS_INPUT_MODE_SENSOR ; +else + continuous = pipe->stream->config.continuous;if (mycs->preview_binary.info)return 0 ;
@@ -2627,24 +2628,26 @@ 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 ) - need_isp_copy_binary = !online && !continuous; +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 - need_isp_copy_binary = !online && !continuous && !(pipe->stream->config.mode ==IA_CSS_INPUT_MODE_MEMORY ); -#endif +/* +* 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 ) + need_isp_copy_binary = !online && !continuous; +else + need_isp_copy_binary = !online && !continuous && + !(pipe->stream->config.mode == +IA_CSS_INPUT_MODE_MEMORY );/* Copy */ if (need_isp_copy_binary) {
@@ -3125,11 +3128,11 @@ 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 ) - in_frame->frame_info.format = (pipe->stream->config.pack_raw_pixels) ? -IA_CSS_FRAME_FORMAT_RAW_PACKED :IA_CSS_FRAME_FORMAT_RAW ; -#endif +if (IS_ISP2401 ) { +if (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 ; + } 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;
@@ -3141,9 +3144,12 @@ init_in_frameinfo_memory_defaults(struct ia_css_pipe *pipe,ia_css_query_internal_queue_id (IA_CSS_BUFFER_TYPE_INPUT_FRAME , thread_id, &queue_id); in_frame->dynamic_queue_id = queue_id; in_frame->buf_type =IA_CSS_BUFFER_TYPE_INPUT_FRAME ; +#ifdef ISP2401 +ia_css_get_crop_offsets (pipe, &in_frame->frame_info);#endif + err =ia_css_frame_init_planes (in_frame); ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE_PRIVATE, "%s() bayer_order = %d\n",
@@ -3211,18 +3217,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 +3274,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 +3378,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 +3394,25 @@ 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 );
@@ -3417,9 +3420,9 @@ create_host_preview_pipeline(struct ia_css_pipe *pipe) goto ERR; in_frame = &me->in_frame; - }else { + }else in_frame =NULL ; - } + err =init_out_frameinfo_defaults (pipe, &me->out_frame[0 ],0 );if (err)
@@ -3441,17 +3444,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 +3927,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 +3961,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 +4881,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 +5226,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 +5247,12 @@ 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 +if (IS_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; + } mycs = &pipe->pipe_settings.capture; pipe_out_info = &pipe->output_info[0 ];
@@ -5462,15 +5458,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 +5676,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 +5824,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 +5897,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 +6403,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 +6420,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 +6671,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 +6691,23 @@ 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 +6798,11 @@ 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) - in_frame_local = in_frame; -#endif +if (IS_ISP2401 ) { +/* After isp copy is enabled in_frame needs to be passed. */ +if (!online) + in_frame_local = in_frame; + }if (need_scaler) { ia_css_pipe_util_set_output_frames(bin_out_frame,
@@ -7031,12 +7021,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 +7042,26 @@ 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 +7124,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,14 +7174,17 @@ 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 - local_out_frame = out_frame; - else + +if (!need_pp && (i == num_primary_stage -1 )) { +if (IS_ISP2401 ) { +if (!need_ldc) + local_out_frame = out_frame; + }else { + local_out_frame = out_frame; + } + }else { local_out_frame =NULL ; + }ia_css_pipe_util_set_output_frames (out_frames,0 , local_out_frame);/* * WARNING: The #if def flag has been added below as a
@@ -7401,22 +7393,22 @@ static int capture_start(struct ia_css_pipe *pipe) } } -#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 ) { +if (pipe->config.mode !=IA_CSS_PIPE_MODE_COPY ) { + err =send_mipi_frames (pipe); +if (err) { +IA_CSS_LEAVE_ERR_PRIVATE (err); +return err; + } + } + }else { +/* 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; } } -#endif ia_css_pipeline_get_sp_thread_id (ia_css_pipe_get_pipe_num (pipe), &thread_id); copy_ovrd =1 << thread_id;
@@ -8123,24 +8115,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 +8213,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 ;
@@ -8665,6 +8653,7 @@ ia_css_stream_start(struct ia_css_stream *stream)for (idx =0 ; idx <IA_CSS_MIPI_SIZE_CHECK_MAX_NOF_ENTRIES_PER_PORT ; idx++) { sh_css_sp_group.config.mipi_sizes_for_check[port][idx] =sh_css_get_mipi_sizes_for_check (port, idx); + } }#endif
@@ -9162,11 +9151,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 */
Signature · 1 line
2.39.2
Re: [PATCH 1/5] staging: media: atomisp: sh_css: Remove #ifdef ISP2401
Hi Kate,
Thank you for your work on this!
A few small remarks below / inline:
Quoted 50 lines · Kate Hsuan
The actions of ISP2401 and 2400 are determined at the runtime.
Signed-off-by Kate Hsuan <[email protected]>--- drivers/staging/media/atomisp/pci/sh_css.c | 584 ++++++++++----------- 1 file changed, 286 insertions(+), 298 deletions(-)drivers/staging/media/atomisp/pci/sh_css.c +4
-11@@ -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,13 @@ 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
drivers/staging/media/atomisp/pci/sh_css.c ia_css_pipe_destroy(struct ia_css_pipe *pipe) · line 2134
Quoted 10 lines
drivers/staging/media/atomisp/pci/sh_css.c +7
-3-ia_css_frame_free_multiple (NUM_VIDEO_TNR_FRAMES , - pipe->pipe_settings.video.tnr_frames); -#endif +if (IS_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); +
Unless I am missing something here the if and else branches contain the same statement here (and so did the original code) so I think this can if ... else ... can be simplified to just:
ia_css_frame_free_multiple(NUM_VIDEO_TNR_FRAMES, pipe->pipe_settings.video.tnr_frames);
?
<snip (+ more snips below)>
Quoted 4 lines
drivers/staging/media/atomisp/pci/sh_css.c +0
-0@@ -2514,11 +2515,11 @@ load_preview_binaries(struct ia_css_pipe *pipe)assert (pipe->mode ==IA_CSS_PIPE_ID_PREVIEW ); online = pipe->stream->config.online;
drivers/staging/media/atomisp/pci/sh_css.c load_preview_binaries(struct ia_css_pipe *pipe) · line 2522
Quoted 10 lines
drivers/staging/media/atomisp/pci/sh_css.c +5
-5-#ifdef ISP2401 - sensor = pipe->stream->config.mode ==IA_CSS_INPUT_MODE_SENSOR ; -#else - continuous = pipe->stream->config.continuous; -#endif + +if (IS_ISP2401 ) + sensor = pipe->stream->config.mode ==IA_CSS_INPUT_MODE_SENSOR ; +else + continuous = pipe->stream->config.continuous;
You can drop the if ... else here and just always initialize both helper variables:
sensor = pipe->stream->config.mode == IA_CSS_INPUT_MODE_SENSOR; continuous = pipe->stream->config.continuous;
Quoted 37 lines
drivers/staging/media/atomisp/pci/sh_css.c +12
-17if (mycs->preview_binary.info)return 0 ;@@ -2627,24 +2628,26 @@ 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 ) - need_isp_copy_binary = !online && !continuous; +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 - need_isp_copy_binary = !online && !continuous && !(pipe->stream->config.mode ==IA_CSS_INPUT_MODE_MEMORY ); -#endif +/* +* 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.
drivers/staging/media/atomisp/pci/sh_css.c load_preview_binaries(struct ia_css_pipe *pipe) · line 2653
Quoted 10 lines
drivers/staging/media/atomisp/pci/sh_css.c +7
-0+*/ +if (!IS_ISP2401 ) + need_isp_copy_binary = !online && !continuous; +else + need_isp_copy_binary = !online && !continuous && + !(pipe->stream->config.mode == +IA_CSS_INPUT_MODE_MEMORY );/* Copy */ if (need_isp_copy_binary) {
The if (!IS_ISP2401) is always true here since we are in an else branch of the "if (IS_ISP2401)" check above (this was already the case in the old code with the #ifdef, but no-one noticed sofar).
So the else branch of the nested if never gets run and can be removed. And since the comment is only about the else branch it can be removed too, so this entire block / chunk can be similified to:
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 {
need_isp_copy_binary = !online && !continuous;
}Quoted 7 lines
drivers/staging/media/atomisp/pci/sh_css.c +0
-3@@ -3125,11 +3128,11 @@ 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 ) - in_frame->frame_info.format = (pipe->stream->config.pack_raw_pixels) ?
drivers/staging/media/atomisp/pci/sh_css.c init_in_frameinfo_memory_defaults(struct ia_css_pipe *pipe, · line 3138
Quoted 10 lines
drivers/staging/media/atomisp/pci/sh_css.c +5
-2- IA_CSS_FRAME_FORMAT_RAW_PACKED :IA_CSS_FRAME_FORMAT_RAW ; -#endif +if (IS_ISP2401 ) { +if (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 ; + } 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;
Please change this to:
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;
}to avoid unnecessary if nesting.
drivers/staging/media/atomisp/pci/sh_css.c init_in_frameinfo_memory_defaults(struct ia_css_pipe *pipe, · line 3155
Quoted 13 lines
drivers/staging/media/atomisp/pci/sh_css.c +3
-0@@ -3141,9 +3144,12 @@ init_in_frameinfo_memory_defaults(struct ia_css_pipe *pipe,ia_css_query_internal_queue_id (IA_CSS_BUFFER_TYPE_INPUT_FRAME , thread_id, &queue_id); in_frame->dynamic_queue_id = queue_id; in_frame->buf_type =IA_CSS_BUFFER_TYPE_INPUT_FRAME ; +#ifdef ISP2401 +ia_css_get_crop_offsets (pipe, &in_frame->frame_info);#endif + err =ia_css_frame_init_planes (in_frame); ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE_PRIVATE, "%s() bayer_order = %d\n",
You are only adding whitespace here? I guess the intent was to actually replace the #ifdef with a if (IS_ISP2401) ?
drivers/staging/media/atomisp/pci/sh_css.c create_host_preview_pipeline(struct ia_css_pipe *pipe) · line 3428
Quoted 12 lines
drivers/staging/media/atomisp/pci/sh_css.c +2
-2@@ -3417,9 +3420,9 @@ create_host_preview_pipeline(struct ia_css_pipe *pipe) goto ERR; in_frame = &me->in_frame; - }else { + }else in_frame =NULL ; - } + err =init_out_frameinfo_defaults (pipe, &me->out_frame[0 ],0 );if (err)
Unrelated coding style change, please drop.
Quoted 6 lines
drivers/staging/media/atomisp/pci/sh_css.c +0
-2@@ -5251,12 +5247,12 @@ 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 );
drivers/staging/media/atomisp/pci/sh_css.c static int load_primary_binaries( · line 5255
Quoted 10 lines
drivers/staging/media/atomisp/pci/sh_css.c +6
-4-#else - memory = pipe->stream->config.mode ==IA_CSS_INPUT_MODE_MEMORY ; - continuous = pipe->stream->config.continuous; -#endif +if (IS_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; + }
As above, just always initialize all helper variables like these:
sensor = (pipe->stream->config.mode == IA_CSS_INPUT_MODE_SENSOR); memory = pipe->stream->config.mode == IA_CSS_INPUT_MODE_MEMORY; continuous = pipe->stream->config.continuous;
Quoted 4 lines
drivers/staging/media/atomisp/pci/sh_css.c +0
-0@@ -6808,11 +6798,11 @@ 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 ;
drivers/staging/media/atomisp/pci/sh_css.c create_host_yuvpp_pipeline(struct ia_css_pipe *pipe) · line 6805
Quoted 10 lines
drivers/staging/media/atomisp/pci/sh_css.c +5
-5-#ifdef ISP2401 -/* After isp copy is enabled in_frame needs to be passed. */ -if (!online) - in_frame_local = in_frame; -#endif +if (IS_ISP2401 ) { +/* After isp copy is enabled in_frame needs to be passed. */ +if (!online) + in_frame_local = in_frame; + }
Please && the 2 conditions together to avoid unnecessary if nesting:
if (IS_ISP2401 && !online) {
/* After isp copy is enabled in_frame needs to be passed. */
in_frame_local = in_frame;
}Quoted 33 lines
drivers/staging/media/atomisp/pci/sh_css.c +10
-19@@ -7054,25 +7042,26 @@ 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 );
drivers/staging/media/atomisp/pci/sh_css.c create_host_regular_capture_pipeline(struct ia_css_pipe *pipe) · line 7064
Quoted 10 lines
drivers/staging/media/atomisp/pci/sh_css.c +10
-0+ 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 ; +
When one branch of an if...else... block needs {} please add {} to both branches (so also add it to the else here).
Quoted 6 lines
drivers/staging/media/atomisp/pci/sh_css.c +0
-2@@ -7185,14 +7174,17 @@ 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 ))
drivers/staging/media/atomisp/pci/sh_css.c create_host_regular_capture_pipeline(struct ia_css_pipe *pipe) · line 7181
Quoted 10 lines
drivers/staging/media/atomisp/pci/sh_css.c +5
-5-#else - if (!need_pp && (i == num_primary_stage - 1) && !need_ldc) -#endif - local_out_frame = out_frame; - else + +if (!need_pp && (i == num_primary_stage -1 )) { +if (IS_ISP2401 ) { +if (!need_ldc) + local_out_frame = out_frame;
this is missing a:
else local_out_frame = NULL;
drivers/staging/media/atomisp/pci/sh_css.c create_host_regular_capture_pipeline(struct ia_css_pipe *pipe) · line 7187
Quoted 6 lines
drivers/staging/media/atomisp/pci/sh_css.c +5
-0+ }else { + local_out_frame = out_frame; + } + }else { local_out_frame =NULL ; + }
Rather then adding the missing else, I think it would be better to just fold this all into:
if (!need_pp && (i == num_primary_stage - 1) && (!IS_ISP2401 || !need_ldc)) local_out_frame = out_frame; else local_out_frame = NULL;
?
Quoted 26 lines
drivers/staging/media/atomisp/pci/sh_css.c +10
-9ia_css_pipe_util_set_output_frames (out_frames,0 , local_out_frame);/* * WARNING: The #if def flag has been added below as a @@ -7401,22 +7393,22 @@ static int capture_start(struct ia_css_pipe *pipe) } } -#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 ) { +if (pipe->config.mode !=IA_CSS_PIPE_MODE_COPY ) { + err =send_mipi_frames (pipe); +if (err) { +IA_CSS_LEAVE_ERR_PRIVATE (err); +return err; + } + } + }else { +/* old isys: need to send_mipi_frames() in all pipe modes */
drivers/staging/media/atomisp/pci/sh_css.c static int capture_start(struct ia_css_pipe *pipe) · line 7414
Quoted 10 lines
drivers/staging/media/atomisp/pci/sh_css.c +0
-1err =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;
Maybe simplify this entire block to:
/* old isys: need to send_mipi_frames() in all pipe modes */
if (!IS_ISP2401 || !pipe->config.mode != IA_CSS_PIPE_MODE_COPY) {
if (pipe->config.mode != IA_CSS_PIPE_MODE_COPY) {
err = send_mipi_frames(pipe);
if (err) {
IA_CSS_LEAVE_ERR_PRIVATE(err);
return err;
}
}
}?
drivers/staging/media/atomisp/pci/sh_css.c ia_css_stream_start(struct ia_css_stream *stream) · line 8659
Quoted 8 lines
drivers/staging/media/atomisp/pci/sh_css.c +1
-0@@ -8665,6 +8653,7 @@ ia_css_stream_start(struct ia_css_stream *stream)for (idx =0 ; idx <IA_CSS_MIPI_SIZE_CHECK_MAX_NOF_ENTRIES_PER_PORT ; idx++) { sh_css_sp_group.config.mipi_sizes_for_check[port][idx] =sh_css_get_mipi_sizes_for_check (port, idx); + } }#endif
Unrelated whitespace chage, please drop.
Regards,
Hans
PATCH 2/5 staging: media: atomisp: runtime: frame: remove #ifdef ISP2401
In reply to Kate HsuanThe actions of ISP2401 and 2400 are determined at the runtime.
--- .../media/atomisp/pci/runtime/frame/src/frame.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-)
drivers/staging/media/atomisp/pci/runtime/frame/src/frame.c +7 -8
@@ -601,9 +601,9 @@ 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 +if (IS_ISP2401 ) +IA_CSS_ENTER_LEAVE_PRIVATE ("frame->data_bytes=%d\n" , frame->data_bytes); + frame->data =hmm_alloc (frame->data_bytes);if (frame->data == mmgr_NULL)return -ENOMEM ;
@@ -635,11 +635,10 @@ static int frame_allocate_with_data(struct ia_css_frame **frame,if (err) {kvfree (me); -#ifndef ISP2401 -return err; -#else - me =NULL ; -#endif +if (IS_ISP2401 ) + me =NULL ; +else +return err; } *frame = me;
Signature · 1 line
2.39.2
Re: [PATCH 2/5] staging: media: atomisp: runtime: frame: remove #ifdef ISP2401
Hi Kate,
Quoted 14 lines · Kate Hsuan
The actions of ISP2401 and 2400 are determined at the runtime.
Signed-off-by Kate Hsuan <[email protected]>--- .../media/atomisp/pci/runtime/frame/src/frame.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-)drivers/staging/media/atomisp/pci/runtime/frame/src/frame.c +0
-0@@ -601,9 +601,9 @@ static void frame_init_qplane6_planes(struct ia_css_frame *frame)static int frame_allocate_buffer_data (struct ia_css_frame *frame)
drivers/staging/media/atomisp/pci/runtime/frame/src/frame.c static void frame_init_qplane6_planes(struct ia_css_frame *frame) · line 609
Quoted 10 lines
drivers/staging/media/atomisp/pci/runtime/frame/src/frame.c +3
-3{ -#ifdef ISP2401 -IA_CSS_ENTER_LEAVE_PRIVATE ("frame->data_bytes=%d\n" , frame->data_bytes); -#endif +if (IS_ISP2401 ) +IA_CSS_ENTER_LEAVE_PRIVATE ("frame->data_bytes=%d\n" , frame->data_bytes); + frame->data =hmm_alloc (frame->data_bytes);if (frame->data == mmgr_NULL)return -ENOMEM ;
This is just a debug log message, IMHO it is best to just completely remove the message for both ISP models.
Quoted 6 lines
drivers/staging/media/atomisp/pci/runtime/frame/src/frame.c +0
-2@@ -635,11 +635,10 @@ static int frame_allocate_with_data(struct ia_css_frame **frame,if (err) {kvfree (me); -#ifndef ISP2401 -return err;
drivers/staging/media/atomisp/pci/runtime/frame/src/frame.c static int frame_allocate_with_data(struct ia_css_frame **frame, · line 644
Quoted 10 lines
drivers/staging/media/atomisp/pci/runtime/frame/src/frame.c +4
-3-#else - me =NULL ; -#endif +if (IS_ISP2401 ) + me =NULL ; +else +return err; } *frame = me;
This one is also weird. I have checked the only caller of this and it does not matter what *frame is set to since as long as this returns non 0 the *frame is ignored and the functions always returns err (just outside of the context of the patch).
So this can be simplified to just:
if (err) {
kvfree(me);
me = NULL;
}And then fall through to the original code of:
*frame = me;
return err; }
The me = NULL is not strictly necessary but setting *frame = NULL on errors is a bit cleaner and may help catch future bugs.
Regards,
Hans
PATCH 3/5 staging: media: atomisp: sh_css_sp: Remove #ifdef ISP2401
In reply to Kate HsuanThe actions of ISP2401 and 2400 will be determined at the runtime.
--- drivers/staging/media/atomisp/pci/sh_css_sp.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)
drivers/staging/media/atomisp/pci/sh_css_sp.c +4 -6
@@ -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);/*
Signature · 1 line
2.39.2
Re: [PATCH 3/5] staging: media: atomisp: sh_css_sp: Remove #ifdef ISP2401
Hi,
Quoted 3 lines · Kate Hsuan
The actions of ISP2401 and 2400 will be determined at the runtime.
Signed-off-by Kate Hsuan <[email protected]>
Thanks, patch looks good to me:
Regards,
Hans
Quoted 25 lines
--- drivers/staging/media/atomisp/pci/sh_css_sp.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)drivers/staging/media/atomisp/pci/sh_css_sp.c +4
-6@@ -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);/*
PATCH 4/5 staging: media: atomisp: sh_css_firmware: determine firmware version at runtime
In reply to Kate HsuanThe firmware version of ISP2401 and 2400 is determined at runtime.
--- .../media/atomisp/pci/sh_css_firmware.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-)
drivers/staging/media/atomisp/pci/sh_css_firmware.c +13 -5
@@ -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 300static 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!",
Signature · 1 line
2.39.2
Re: [PATCH 4/5] staging: media: atomisp: sh_css_firmware: determine firmware version at runtime
Hi,
Quoted 3 lines · Kate Hsuan
The firmware version of ISP2401 and 2400 is determined at runtime.
Signed-off-by Kate Hsuan <[email protected]>
Thanks, patch looks good to me:
Regards,
Hans
Quoted 56 lines
--- .../media/atomisp/pci/sh_css_firmware.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-)drivers/staging/media/atomisp/pci/sh_css_firmware.c +13
-5@@ -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 300static 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!",
PATCH 5/5 staging: media: atomisp: sh_css_mipi: Remove #ifdef 2041
In reply to Kate HsuanThe actions of ISP2401 and 2400 are determined at the runtime.
--- .../staging/media/atomisp/pci/sh_css_mipi.c | 101 +++++++++--------- 1 file changed, 49 insertions(+), 52 deletions(-)
drivers/staging/media/atomisp/pci/sh_css_mipi.c +49 -52
@@ -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,15 @@ allocate_mipi_frames(struct ia_css_pipe *pipe,return -EINVAL ; } -#ifdef ISP2401 -if (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 ; +if (IS_ISP2401 ) { +if (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 +383,30 @@ 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 ; +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. +* 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 ; + } } -#endif ref_count_mipi_allocation[port]++;
@@ -503,14 +500,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 +531,18 @@ 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); +if (IS_ISP2401 ) { +/* 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. */
Signature · 1 line
2.39.2
Re: [PATCH 5/5] staging: media: atomisp: sh_css_mipi: Remove #ifdef 2041
Hi Kate,
Quoted 11 lines · Kate Hsuan
The actions of ISP2401 and 2400 are determined at the runtime.
Signed-off-by Kate Hsuan <[email protected]>--- .../staging/media/atomisp/pci/sh_css_mipi.c | 101 +++++++++--------- 1 file changed, 49 insertions(+), 52 deletions(-)drivers/staging/media/atomisp/pci/sh_css_mipi.c +0
-0
<snip>
Quoted 13 lines
drivers/staging/media/atomisp/pci/sh_css_mipi.c +3
-6@@ -363,15 +360,15 @@ allocate_mipi_frames(struct ia_css_pipe *pipe,return -EINVAL ; } -#ifdef ISP2401 -if (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 ; +if (IS_ISP2401 ) { +if (pipe->stream->config.online) { + ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE_PRIVATE,
drivers/staging/media/atomisp/pci/sh_css_mipi.c allocate_mipi_frames(struct ia_css_pipe *pipe, · line 374
Quoted 10 lines
drivers/staging/media/atomisp/pci/sh_css_mipi.c +4
-1+"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",
Please combine the 2 conditions with && instead of using nested if-s:
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;
}Quoted 41 lines
drivers/staging/media/atomisp/pci/sh_css_mipi.c +15
-22@@ -386,30 +383,30 @@ 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 ; +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. +* 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 ;
drivers/staging/media/atomisp/pci/sh_css_mipi.c allocate_mipi_frames(struct ia_css_pipe *pipe, · line 412
Quoted 10 lines
drivers/staging/media/atomisp/pci/sh_css_mipi.c +8
-1+ } + }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
The "if (ref_count_mipi_allocation[port] != 0) { log-message; return 0; }" block is shared between the ISP2400 and ISP2401. Except for one debug log message using "leave" and the other using "exit" these 2 blocks are 100% the same, so please pick one of the 2 log messages and move this block out of the if (IS_ISP2401) {} block (and drop the else branch).
Quoted 19 lines
drivers/staging/media/atomisp/pci/sh_css_mipi.c +5
-9@@ -534,18 +531,18 @@ 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); +if (IS_ISP2401 ) { +/* 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.
drivers/staging/media/atomisp/pci/sh_css_mipi.c free_mipi_frames(struct ia_css_pipe *pipe) · line 548
Quoted 10 lines
drivers/staging/media/atomisp/pci/sh_css_mipi.c +5
-1+*/ +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. */
Hmm maybe just drop the entire ia_css_debug_dtrace() call here instead of replace the #ifdef with if (IS_ISP2401) { } ?
I don't see much value in the ia_css_debug_dtrace() call, so to me just dropping it seems best.
Regards,
Hans