Re: [RFC v2 03/10] exynos5-fimc-is: Adds common driver header files

2013-07-09 Thread Arun Kumar K
Hi Sylwester,

Thank you for the review.

On Fri, Jun 21, 2013 at 4:16 AM, Sylwester Nawrocki
sylvester.nawro...@gmail.com wrote:
 On 05/31/2013 03:03 PM, Arun Kumar K wrote:

 This patch adds all the common header files used by the fimc-is
 driver. It includes the commands for interfacing with the firmware
 and error codes from IS firmware, metadata and command parameter
 definitions.

 Signed-off-by: Arun Kumar Karun...@samsung.com
 Signed-off-by: Kilyeon Imkilyeon...@samsung.com
 ---
   drivers/media/platform/exynos5-is/fimc-is-cmd.h|  201 
   drivers/media/platform/exynos5-is/fimc-is-err.h|  261 
   .../media/platform/exynos5-is/fimc-is-metadata.h   |  771 
   drivers/media/platform/exynos5-is/fimc-is-param.h  | 1259
 
   4 files changed, 2492 insertions(+)
   create mode 100644 drivers/media/platform/exynos5-is/fimc-is-cmd.h
   create mode 100644 drivers/media/platform/exynos5-is/fimc-is-err.h
   create mode 100644 drivers/media/platform/exynos5-is/fimc-is-metadata.h
   create mode 100644 drivers/media/platform/exynos5-is/fimc-is-param.h

 diff --git a/drivers/media/platform/exynos5-is/fimc-is-cmd.h
 b/drivers/media/platform/exynos5-is/fimc-is-cmd.h
 new file mode 100644
 index 000..4adf832
 --- /dev/null
 +++ b/drivers/media/platform/exynos5-is/fimc-is-cmd.h
 @@ -0,0 +1,201 @@
 +/*
 + * Samsung Exynos5 SoC series FIMC-IS driver
 + *
 + * Copyright (c) 2013 Samsung Electronics Co., Ltd
 + * Kil-yeon Limkilyeon...@samsung.com
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License version 2 as
 + * published by the Free Software Foundation.
 + */
 +
 +#ifndef FIMC_IS_CMD_H
 +#define FIMC_IS_CMD_H
 +
 +#define IS_COMMAND_VER 122 /* IS COMMAND VERSION 1.22 */
 +
 +enum is_cmd {
 +   /* HOST -  IS */
 +   HIC_PREVIEW_STILL = 0x1,
 +   HIC_PREVIEW_VIDEO,
 +   HIC_CAPTURE_STILL,
 +   HIC_CAPTURE_VIDEO,
 +   HIC_PROCESS_START,
 +   HIC_PROCESS_STOP,
 +   HIC_STREAM_ON,
 +   HIC_STREAM_OFF,
 +   HIC_SHOT,
 +   HIC_GET_STATIC_METADATA,
 +   HIC_SET_CAM_CONTROL,
 +   HIC_GET_CAM_CONTROL,
 +   HIC_SET_PARAMETER,
 +   HIC_GET_PARAMETER,
 +   HIC_SET_A5_MEM_ACCESS,
 +   RESERVED2,
 +   HIC_GET_STATUS,
 +   /* SENSOR PART*/
 +   HIC_OPEN_SENSOR,
 +   HIC_CLOSE_SENSOR,
 +   HIC_SIMMIAN_INIT,
 +   HIC_SIMMIAN_WRITE,
 +   HIC_SIMMIAN_READ,
 +   HIC_POWER_DOWN,
 +   HIC_GET_SET_FILE_ADDR,
 +   HIC_LOAD_SET_FILE,
 +   HIC_MSG_CONFIG,
 +   HIC_MSG_TEST,
 +   /* IS -  HOST */
 +   IHC_GET_SENSOR_NUMBER = 0x1000,
 +   /* Parameter1 : Address of space to copy a setfile */
 +   /* Parameter2 : Space szie */
 +   IHC_SET_SHOT_MARK,
 +   /* PARAM1 : a frame number */
 +   /* PARAM2 : confidence level(smile 0~100) */
 +   /* PARMA3 : confidence level(blink 0~100) */
 +   IHC_SET_FACE_MARK,
 +   /* PARAM1 : coordinate count */
 +   /* PARAM2 : coordinate buffer address */
 +   IHC_FRAME_DONE,
 +   /* PARAM1 : frame start number */
 +   /* PARAM2 : frame count */
 +   IHC_AA_DONE,
 +   IHC_NOT_READY,
 +   IHC_FLASH_READY
 +};
 +
 +enum is_reply {
 +   ISR_DONE= 0x2000,
 +   ISR_NDONE
 +};
 +
 +enum is_scenario_id {
 +   ISS_PREVIEW_STILL,
 +   ISS_PREVIEW_VIDEO,
 +   ISS_CAPTURE_STILL,
 +   ISS_CAPTURE_VIDEO,
 +   ISS_END
 +};
 +
 +enum is_subscenario_id {
 +   ISS_SUB_SCENARIO_STILL,
 +   ISS_SUB_SCENARIO_VIDEO,
 +   ISS_SUB_SCENARIO_SCENE1,
 +   ISS_SUB_SCENARIO_SCENE2,
 +   ISS_SUB_SCENARIO_SCENE3,
 +   ISS_SUB_END
 +};
 +
 +struct is_setfile_header_element {
 +   u32 binary_addr;
 +   u32 binary_size;
 +};
 +
 +struct is_setfile_header {
 +   struct is_setfile_header_element isp[ISS_END];
 +   struct is_setfile_header_element drc[ISS_END];
 +   struct is_setfile_header_element fd[ISS_END];
 +};
 +
 +struct is_common_reg {
 +   u32 hicmd;
 +   u32 hic_sensorid;
 +   u32 hic_param1;
 +   u32 hic_param2;
 +   u32 hic_param3;
 +   u32 hic_param4;


 How about replacing the above 4 fields with

 u32 hic_param[4];

 ?

Ok.


 +   u32 reserved1[3];
 +
 +   u32 ihcmd_iflag;
 +   u32 ihcmd;
 +   u32 ihc_sensorid;
 +   u32 ihc_param1;
 +   u32 ihc_param2;
 +   u32 ihc_param3;
 +   u32 ihc_param4;


 Similarly,

 u32 ihc_param[4];

 and for other groups of fields below ?


Will make this change.

 +
 +   u32 reserved2[3];
 +
 +   u32 isp_bayer_iflag;
 +   u32 isp_bayer_sensor_id;
 diff --git a/drivers/media/platform/exynos5-is/fimc-is-err.h
 b/drivers/media/platform/exynos5-is/fimc-is-err.h
 new file mode 100644
 index 000..49d7cf5
 --- /dev/null
 +++ b/drivers/media/platform/exynos5-is/fimc-is-err.h
 @@ -0,0 +1,261 @@
 +/*
 + * Samsung Exynos5 SoC series 

Re: [RFC v2 03/10] exynos5-fimc-is: Adds common driver header files

2013-06-21 Thread Arun Kumar K
Hi Sylwester,

On Fri, Jun 21, 2013 at 4:16 AM, Sylwester Nawrocki
sylvester.nawro...@gmail.com wrote:
 Guys, I was wondering how difficult would be to make a common driver
 for the Exynos4 and Exynos5 FIMC-IS ? My feeling is that it would allow
 to save significant amount of code, since the hardware has many
 similarities. I imagine it would be a lot of work, and testing would have
 been a bit difficult. But would it really to troublesome to make a common
 driver ? Could you list some arguments against it ? For the MFC we have
 same driver, handling different firmware versions. Similarly for the other
 media IPs. Only the FIMC-IS subsystems would have separate drivers.
 My intentions is really only to reduce the amount of code we would have
 to merge with this new driver, nothing else. But I'm not going to push
 for the common driver if this is too much trouble.

We have thought about it while starting the development and major
arguments against common driver are :

- FIMC-IS IP has significantly changed from Exynos4.
In Exynos4, it has sub-components ISP, DRC and FD where as in exynos5,
it has ISP, DRC, SCC, ODC, DIS. 3DNR, SCP and FD.

- The FW design has changed considerably to make use of camera2 api
interface. Most of the code in the new driver is for this FW interface
which are done in fimc_is_pipeline.* and fimc_is_interface.*. This is the
major reason against a common driver as the new FW expects each
input frame to be passed along with the controls in a SHOT command.
It is a request-response mode handled per-frame by the FW which is
a major design philosophy change from exynos4.

- Two scalers introduced in the pipeline capable of DMA out which
again changes the pipeline design considerably compared to exynos4.

- The only common part of code between exynos 4 and 5 now is in
the fimc-isp.c and fimc-is-sensor.c and some control structures
in header files. If re-used, only some user controls part can be
re-used and most of the code will still be different.
From the exynos5 driver, still the fimc-is-scaler.*, fimc-is-pipeline.*,
fimc-is-interface.* has to be retained which constitutes majority of the LOC.

Regards
Arun
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html