staging: atomisp: activate ATOMISP2401 support
Subsystems (4)
cover staging: atomisp: activate ATOMISP2401 support
Currently atomisp module supports Intel's Baytrail SoC and contains
some compilation switches to support Intel's Cherrytrail SoC instead.
The patchset aims to :
- 1/2: activate ATOMISP2400 or ATOMISP2401 from the menu.
- 2/2: fix compilation errors for ATOMISP2401.
I'm not so confident with patch 2/2, as it is only working around the non declared functions by using the 2400 path. As I couln't find any declaration/definition for the ISP2401 missing functions...So any help would be appreciated. Also patch 2/2 doesn't correct any cosmetic changes reported by checkpatch.pl as explained in TODO step 6.
Vincent Hervieux (2):
staging: atomisp: add menu entries to choose between ATOMISP_2400 and
ATOMISP_2401.
staging: atomisp: fix compilation errors in case of ISP2401.
drivers/staging/media/atomisp/pci/Kconfig | 23 +++++++++++++++++++++ .../staging/media/atomisp/pci/atomisp2/Makefile | 10 ++++++++- .../media/atomisp/pci/atomisp2/atomisp_cmd.c | 5 ++--- .../media/atomisp/pci/atomisp2/atomisp_v4l2.c | 6 +++++- .../pci/atomisp2/css2400/ia_css_acc_types.h | 1 + .../css2400/runtime/debug/src/ia_css_debug.c | 3 --- .../media/atomisp/pci/atomisp2/css2400/sh_css.c | 24 ++++++++++------------ .../atomisp/pci/atomisp2/css2400/sh_css_params.c | 8 +------- 8 files changed, 52 insertions(+), 28 deletions(-)
Signature · 1 line
2.11.0
PATCH 1/2 staging: atomisp: add menu entries to choose between ATOMISP_2400 and ATOMISP_2401.
--- drivers/staging/media/atomisp/pci/Kconfig | 23 ++++++++++++++++++++++ .../staging/media/atomisp/pci/atomisp2/Makefile | 10 +++++++++- 2 files changed, 32 insertions(+), 1 deletion(-)
drivers/staging/media/atomisp/pci/Kconfig +23 -0
@@ -11,3 +11,26 @@ config VIDEO_ATOMISP camera imaging subsystem. To compile this driver as a module, choose M here: the module will be called atomisp + +choice + prompt "Intel Atom Image Signal Processor Driver Type" + depends on VIDEO_ATOMISP + default VIDEO_ATOMISP_ISP2400 + help + Intel Atom Image Signal Processor Driver actually doesn't support + dynamically all SoC. + So need to choose at compilation time which SoC it can support. + Please refer to staging TODO for more details. + +config VIDEO_ATOMISP_ISP2400 + bool "ISP2400" + help + Atom ISP for Merrifield, Baytrail SoC. + +config VIDEO_ATOMISP_ISP2401 + bool "ISP2401" + help + Atom ISP for Anniedale (Merrifield+ / Moorefield), Cherrytrail SoC. + +endchoice +
drivers/staging/media/atomisp/pci/atomisp2/Makefile +9 -1
@@ -155,7 +155,7 @@ atomisp-objs += \ hmm/hmm_dynamic_pool.o \ hrt/hive_isp_css_mm_hrt.o \ atomisp_v4l2.o - + # These will be needed when clean merge CHT support nicely into the driver # Keep them here handy for when we get to that point #
@@ -347,8 +347,16 @@ DEFINES := -DHRT_HW -DHRT_ISP_CSS_CUSTOM_HOST -DHRT_USE_VIR_ADDRS -D__HOST__ #DEFINES += -DPUNIT_CAMERA_BUSY #DEFINES += -DUSE_KMEM_CACHE +ifeq ($(CONFIG_VIDEO_ATOMISP_ISP2400),y) +# Merrifield, Baytrail DEFINES += -DATOMISP_POSTFIX=\"css2400b0_v21\" -DISP2400B0 DEFINES += -DSYSTEM_hive_isp_css_2400_system -DISP2400 +endif +ifeq ($(CONFIG_VIDEO_ATOMISP_ISP2401),y) +# Anniedale (Merrifield+ / Moorefield), Cherrytrail +DEFINES += -DATOMISP_POSTFIX=\"css2401a0_v21\" -DISP2401A0 +DEFINES += -DSYSTEM_hive_isp_css_2400_system -DISP2401 -DISP2401_NEW_INPUT_SYSTEM +endif ccflags-y += $(INCLUDES) $(DEFINES) -fno-common
Signature · 1 line
2.11.0
PATCH 2/2 staging: atomisp: fix compilation errors in case of ISP2401.
In reply to Vincent Hervieux--- .../media/atomisp/pci/atomisp2/atomisp_cmd.c | 5 ++--- .../media/atomisp/pci/atomisp2/atomisp_v4l2.c | 6 +++++- .../pci/atomisp2/css2400/ia_css_acc_types.h | 1 + .../css2400/runtime/debug/src/ia_css_debug.c | 3 --- .../media/atomisp/pci/atomisp2/css2400/sh_css.c | 24 ++++++++++------------ .../atomisp/pci/atomisp2/css2400/sh_css_params.c | 8 +------- 6 files changed, 20 insertions(+), 27 deletions(-)
drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c +2 -3
@@ -1045,9 +1045,8 @@ void atomisp_buf_done(struct atomisp_sub_device *asd, int error, asd->re_trigger_capture = false;dev_dbg (isp->dev,"Trigger capture again for new buffer. err=%d\n" , err); - }else { - asd->re_trigger_capture = true; - } + }else { + asd->re_trigger_capture = true;#endif }break ;
drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c +5 -1
@@ -1602,4 +1602,8 @@ module_exit(atomisp_exit);MODULE_AUTHOR ("Wen Wang <[email protected]>" );MODULE_AUTHOR ("Xiaolin Zhang <[email protected]>" );MODULE_LICENSE ("GPL" ); -MODULE_DESCRIPTION ("Intel ATOM Platform ISP Driver" ); +#if defined(ISP2400 ) || defined(ISP2400B0 ) +MODULE_DESCRIPTION ("Intel ATOM Platform ISP Driver 2400" ); +#elif defined(ISP2401 ) +MODULE_DESCRIPTION ("Intel ATOM Platform ISP Driver 2401" ); +#endif
drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_acc_types.h +1 -0
@@ -222,6 +222,7 @@ struct ia_css_binary_info {uint8_t luma_only;uint8_t input_yuv;uint8_t input_raw; +uint8_t lace_stats;#endif uint8_t reduced_pipe;uint8_t vf_veceven;
drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/debug/src/ia_css_debug.c +0 -3
@@ -49,9 +49,6 @@#include "assert_support.h" #include "print_support.h" #include "string_support.h" -#ifdef ISP2401 -#include "ia_css_system_ctrl.h" -#endif #include "fifo_monitor.h"
drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c +11 -13
@@ -1496,7 +1496,7 @@ sh_css_invalidate_shading_tables(struct ia_css_stream *stream) "sh_css_invalidate_shading_tables() leave: return_void\n"); } -#ifndef ISP2401 +#if 1 /* was ndef ISP2401 but this function is used by ISP2401 on line 1758 */ static void enable_interrupts (enum ia_css_irq_type irq_type) {
@@ -1709,7 +1709,7 @@ ia_css_init(const struct ia_css_env *env, enable =gpio_reg_load (GPIO0_ID , _gpio_block_reg_do_e) |GPIO_FLASH_PIN_MASK ;sh_css_mmu_set_page_table_base_index (mmu_l1_base); -#ifndef ISP2401 +#if 1 /* was ndef ISP2401 but ia_css_save_mmu_base_addr is not declared */ my_css_save.mmu_base = mmu_l1_base;#else ia_css_save_mmu_base_addr (mmu_l1_base);
@@ -1726,7 +1726,7 @@ ia_css_init(const struct ia_css_env *env,return err; } -#ifndef ISP2401 +#if 1 /* was ndef ISP2401 but ia_css_save_restore_data_init is not declared */ IA_CSS_LOG ("init: %d" , my_css_save_initialized);#else ia_css_save_restore_data_init ();
@@ -1750,7 +1750,7 @@ ia_css_init(const struct ia_css_env *env,#endif my_css.irq_type = irq_type; -#ifndef ISP2401 +#if 1 /* was ndef ISP2401 but ia_css_save_irq_type is not declared */ my_css_save.irq_type = irq_type;#else ia_css_save_irq_type (irq_type);
@@ -9776,7 +9776,7 @@ ia_css_stream_create(const struct ia_css_stream_config *stream_config, *stream = curr_stream; ERR: -#ifndef ISP2401 +#if 1 /* was ndef ISP2401, but ia_css_save_stream is not declared */ if (err ==IA_CSS_SUCCESS ) {/* working mode: enter into the seed list */
@@ -9819,7 +9819,7 @@ ia_css_stream_destroy(struct ia_css_stream *stream)enum ia_css_err err =IA_CSS_SUCCESS ;#ifdef ISP2401 enum ia_css_err err1 =IA_CSS_SUCCESS ; -enum ia_css_err err2 =IA_CSS_SUCCESS ; +/* unused enum ia_css_err err2 = IA_CSS_SUCCESS; */ #endif IA_CSS_ENTER_PRIVATE ("stream = %p" , stream);
@@ -9915,7 +9915,7 @@ ia_css_stream_destroy(struct ia_css_stream *stream)kfree (stream->pipes); stream->pipes =NULL ; stream->num_pipes =0 ; -#ifndef ISP2401 +#if 1 /*was ndef ISP2401, but ia_css_save_restore_remove_stream is not declared */ /* working mode: take out of the seed list */ if (my_css_save.mode == sh_css_mode_working)for (i=0 ;i<MAX_ACTIVE_STREAMS ;i++)
@@ -10113,7 +10113,6 @@ ia_css_stream_has_stopped(struct ia_css_stream *stream)return stopped; } -#ifndef ISP2401 /* * Destroy the stream and all the pipes related to it. * The stream handle is used to identify the correct entry in the css_save struct
@@ -10141,7 +10140,6 @@ ia_css_stream_unload(struct ia_css_stream *stream)return IA_CSS_SUCCESS ; } -#endif enum ia_css_err ia_css_temp_pipe_to_pipe_id (const struct ia_css_pipe *pipe,enum ia_css_pipe_id *pipe_id) {
@@ -10427,7 +10425,7 @@ ia_css_start_sp(void)sh_css_setup_queues ();ia_css_bufq_dump_queue_info (); -#ifdef ISP2401 +#if 0 /* was def ISP2401, but ia_css_is_system_mode_suspend_or_resume and ia_css_set_system_mode are not declared */ if (ia_css_is_system_mode_suspend_or_resume () == false) {/* skip in suspend/resume flow */ ia_css_set_system_mode (IA_CSS_SYS_MODE_WORKING ); }
@@ -10495,7 +10493,7 @@ ia_css_stop_sp(void)sh_css_hmm_buffer_record_uninit (); -#ifndef ISP2401 +#if 1 /* was ndef ISP2401, but ia_css_is_system_mode_suspend_or_resume is not declared */ /* clear pending param sets from refcount */ sh_css_param_clear_param_sets ();#else
@@ -11003,7 +11001,7 @@ sh_css_hmm_buffer_record_init(void) {int i; -#ifndef ISP2401 +#if 1 /* was ndef ISP2401 but ia_css_is_system_mode_suspend_or_resume is not defined */ for (i =0 ; i <MAX_HMM_BUFFER_NUM ; i++) {sh_css_hmm_buffer_record_reset (&hmm_buffer_record[i]);#else
@@ -11021,7 +11019,7 @@ sh_css_hmm_buffer_record_uninit(void)int i;struct sh_css_hmm_buffer_record *buffer_record =NULL ; -#ifndef ISP2401 +#if 1 /* was ndef ISP2401 but ia_css_is_system_mode_suspend_or_resume is not defined */ buffer_record = &hmm_buffer_record[0 ];for (i =0 ; i <MAX_HMM_BUFFER_NUM ; i++) {if (buffer_record->in_use) {
drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_params.c +1 -7
@@ -46,7 +46,7 @@#include "ia_css_pipeline.h" #include "ia_css_debug.h" #include "memory_access.h" -#if 0 /* FIXME */ +#ifdef ISP2401 #include "memory_realloc.h" #endif #include "ia_css_isp_param.h"
@@ -3128,9 +3128,6 @@ sh_css_init_isp_params_from_global(struct ia_css_stream *stream,ia_css_sdis_clear_coefficients (¶ms->dvs_coefs); params->dis_coef_table_changed = true; -#ifdef ISP2401 -ia_css_tnr3_set_default_config (¶ms->tnr3_config); -#endif }else {
@@ -3929,9 +3926,6 @@ sh_css_param_update_isp_params(struct ia_css_pipe *curr_pipe,*/ g_param_buffer_enqueue_count++;assert (g_param_buffer_enqueue_count < g_param_buffer_dequeue_count+50 ); -#ifdef ISP2401 -ia_css_save_latest_paramset_ptr (pipe, cpy); -#endif /* * Tell the SP which queues are not empty, * by sending the software event.
Signature · 1 line
2.11.0
Re: [PATCH 2/2] staging: atomisp: fix compilation errors in case of ISP2401.
We always need a changelog. And actually this seems a bit involved so there is stuff to explain.
Quoted 71 lines · Vincent Hervieux
Signed-off-by Vincent Hervieux <[email protected]>--- .../media/atomisp/pci/atomisp2/atomisp_cmd.c | 5 ++--- .../media/atomisp/pci/atomisp2/atomisp_v4l2.c | 6 +++++- .../pci/atomisp2/css2400/ia_css_acc_types.h | 1 + .../css2400/runtime/debug/src/ia_css_debug.c | 3 --- .../media/atomisp/pci/atomisp2/css2400/sh_css.c | 24 ++++++++++------------ .../atomisp/pci/atomisp2/css2400/sh_css_params.c | 8 +------- 6 files changed, 20 insertions(+), 27 deletions(-)drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c +2
-3@@ -1045,9 +1045,8 @@ void atomisp_buf_done(struct atomisp_sub_device *asd, int error, asd->re_trigger_capture = false;dev_dbg (isp->dev,"Trigger capture again for new buffer. err=%d\n" , err); - }else { - asd->re_trigger_capture = true; - } + }else { + asd->re_trigger_capture = true;#endif }break ;drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c +5
-1@@ -1602,4 +1602,8 @@ module_exit(atomisp_exit);MODULE_AUTHOR ("Wen Wang <[email protected]>" );MODULE_AUTHOR ("Xiaolin Zhang <[email protected]>" );MODULE_LICENSE ("GPL" ); -MODULE_DESCRIPTION ("Intel ATOM Platform ISP Driver" ); +#if defined(ISP2400 ) || defined(ISP2400B0 ) +MODULE_DESCRIPTION ("Intel ATOM Platform ISP Driver 2400" ); +#elif defined(ISP2401 ) +MODULE_DESCRIPTION ("Intel ATOM Platform ISP Driver 2401" ); +#endif drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_acc_types.h +1
-0@@ -222,6 +222,7 @@ struct ia_css_binary_info {uint8_t luma_only;uint8_t input_yuv;uint8_t input_raw; +uint8_t lace_stats;#endif uint8_t reduced_pipe;uint8_t vf_veceven;drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/debug/src/ia_css_debug.c +0
-3@@ -49,9 +49,6 @@#include "assert_support.h" #include "print_support.h" #include "string_support.h" -#ifdef ISP2401 -#include "ia_css_system_ctrl.h" -#endif #include "fifo_monitor.h" drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c +0
-0
drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c sh_css_invalidate_shading_tables(struct ia_css_stream *stream) · line 1499
Quoted 5 lines
drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c +1
-1@@ -1496,7 +1496,7 @@ sh_css_invalidate_shading_tables(struct ia_css_stream *stream) "sh_css_invalidate_shading_tables() leave: return_void\n"); } -#ifndef ISP2401 +#if 1 /* was ndef ISP2401 but this function is used by ISP2401 on line 1758 */
Just delete the #if. (I haven't looked at the code). These comments should probably be in the changelog. You probably want to break this patch up into several patches and add a little changelog for each explaining what's going on.
Extra curly brace. Bad indenting. Add a missing struct member. Delete references to header file that doesn't exist. Delete defines.
regards,
dan carpenter
Re: [PATCH 1/2] staging: atomisp: add menu entries to choose between ATOMISP_2400 and ATOMISP_2401.
In reply to Vincent HervieuxNo changelog. No Signed-off-by line. Without reading too carefully, or trying to do a build, it looks like we're activating the menu items and then fixing the build. It should be the other way around so that we don't break git bisect. People are always doing randconfig and the autobuilders detect breakage really quick.
drivers/staging/media/atomisp/pci/atomisp2/Makefile DEFINES := -DHRT_HW -DHRT_ISP_CSS_CUSTOM_HOST -DHRT_USE_VIR_ADDRS -D__HOST__ · line 358
Quoted 13 lines · Vincent Hervieux
drivers/staging/media/atomisp/pci/atomisp2/Makefile +7
-0@@ -347,8 +347,16 @@ DEFINES := -DHRT_HW -DHRT_ISP_CSS_CUSTOM_HOST -DHRT_USE_VIR_ADDRS -D__HOST__ #DEFINES += -DPUNIT_CAMERA_BUSY #DEFINES += -DUSE_KMEM_CACHE +ifeq ($(CONFIG_VIDEO_ATOMISP_ISP2400),y) +# Merrifield, Baytrail DEFINES += -DATOMISP_POSTFIX=\"css2400b0_v21\" -DISP2400B0 DEFINES += -DSYSTEM_hive_isp_css_2400_system -DISP2400 +endif +ifeq ($(CONFIG_VIDEO_ATOMISP_ISP2401),y) +# Anniedale (Merrifield+ / Moorefield), Cherrytrail +DEFINES += -DATOMISP_POSTFIX=\"css2401a0_v21\" -DISP2401A0 +DEFINES += -DSYSTEM_hive_isp_css_2400_system -DISP2401 -DISP2401_NEW_INPUT_SYSTEM
These defines are a bit ugly. Eventually we will want to get rid of these.
regards,
dan carpenter
Quoted 7 lines · On Mon, 11 Sep 2017 20:49:27 +0200 Vincent Hervieux…
Currently atomisp module supports Intel's Baytrail SoC and contains
some compilation switches to support Intel's Cherrytrail SoC instead.
The patchset aims to :
- 1/2: activate ATOMISP2400 or ATOMISP2401 from the menu.
- 2/2: fix compilation errors for ATOMISP2401.
I'm not so confident with patch 2/2, as it is only working around the non declared functions by using the 2400 path. As I couln't find any declaration/definition for the ISP2401 missing functions...So any help would be appreciated. Also patch 2/2 doesn't correct any cosmetic changes reported by checkpatch.pl as explained in TODO step 6.
Please don't. Right now we know what work is to be done and tested.
Alan