From: Sylwester Nawrocki <sylvester.nawrocki@gmail.com>
To: Arun Kumar K <arun.kk@samsung.com>
Cc: linux-media@vger.kernel.org, linux-samsung-soc@vger.kernel.org,
devicetree-discuss@lists.ozlabs.org, s.nawrocki@samsung.com,
kgene.kim@samsung.com, kilyeon.im@samsung.com,
arunkk.samsung@gmail.com
Subject: Re: [RFC 04/12] exynos-fimc-is: Adds common driver header files
Date: Sat, 23 Mar 2013 15:05:43 +0100 [thread overview]
Message-ID: <514DB6B7.1050304@gmail.com> (raw)
In-Reply-To: <1362754765-2651-5-git-send-email-arun.kk@samsung.com>
On 03/08/2013 03:59 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 K<arun.kk@samsung.com>
> Signed-off-by: Kilyeon Im<kilyeon.im@samsung.com>
> ---
> drivers/media/platform/exynos5-is/fimc-is-cmd.h | 211 ++
> drivers/media/platform/exynos5-is/fimc-is-err.h | 258 +++
> .../media/platform/exynos5-is/fimc-is-metadata.h | 771 +++++++
> drivers/media/platform/exynos5-is/fimc-is-param.h | 2163 ++++++++++++++++++++
> 4 files changed, 3403 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 0000000..f117f41
> --- /dev/null
> +++ b/drivers/media/platform/exynos5-is/fimc-is-cmd.h
[...]
> +#define HOST_SET_INT_BIT 0x00000001
> +#define HOST_CLR_INT_BIT 0x00000001
> +#define IS_SET_INT_BIT 0x00000001
> +#define IS_CLR_INT_BIT 0x00000001
> +
> +#define HOST_SET_INTERRUPT(base) (base->uiINTGR0 |= HOST_SET_INT_BIT)
> +#define HOST_CLR_INTERRUPT(base) (base->uiINTCR0 |= HOST_CLR_INT_BIT)
> +#define IS_SET_INTERRUPT(base) (base->uiINTGR1 |= IS_SET_INT_BIT)
> +#define IS_CLR_INTERRUPT(base) (base->uiINTCR1 |= IS_CLR_INT_BIT)
These 8 appear to be all unused unused definitions. Probably can be removed.
> 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 0000000..76472a9
> --- /dev/null
> +++ b/drivers/media/platform/exynos5-is/fimc-is-err.h
> @@ -0,0 +1,258 @@
> +/*
> + * Samsung Exynos5 SoC series FIMC-IS driver
> + *
> + * Copyright (c) 2013 Samsung Electronics Co., Ltd
> + * Kil-yeon Lim<kilyeon.im@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_ERR_H
> +#define FIMC_IS_ERR_H
> +
> +#define IS_ERROR_VER 012 /* IS ERROR VERSION 0.07 */
> +
> +#define IS_ERROR_SUCCESS 0
> +/* General 1 ~ 100 */
> +#define IS_ERROR_INVALID_COMMAND (IS_ERROR_SUCCESS+1)
> +#define IS_ERROR_REQUEST_FAIL (IS_ERROR_INVALID_COMMAND+1)
> +#define IS_ERROR_INVALID_SCENARIO (IS_ERROR_REQUEST_FAIL+1)
> +#define IS_ERROR_INVALID_SENSORID (IS_ERROR_INVALID_SCENARIO+1)
> +#define IS_ERROR_INVALID_MODE_CHANGE (IS_ERROR_INVALID_SENSORID+1)
> +#define IS_ERROR_INVALID_MAGIC_NUMBER (IS_ERROR_INVALID_MODE_CHANGE+1)
> +#define IS_ERROR_INVALID_SETFILE_HDR (IS_ERROR_INVALID_MAGIC_NUMBER+1)
> +#define IS_ERROR_ISP_SETFILE_VERSION_MISMATCH (IS_ERROR_INVALID_SETFILE_HDR+1)
> +#define IS_ERROR_ISP_SETFILE_REVISION_MISMATCH\
> + (IS_ERROR_ISP_SETFILE_VERSION_MISMATCH+1)
> +#define IS_ERROR_BUSY (IS_ERROR_ISP_SETFILE_REVISION_MISMATCH+1)
> +#define IS_ERROR_SET_PARAMETER (IS_ERROR_BUSY+1)
> +#define IS_ERROR_INVALID_PATH (IS_ERROR_SET_PARAMETER+1)
> +#define IS_ERROR_OPEN_SENSOR_FAIL (IS_ERROR_INVALID_PATH+1)
> +#define IS_ERROR_ENTRY_MSG_THREAD_DOWN (IS_ERROR_OPEN_SENSOR_FAIL+1)
> +#define IS_ERROR_ISP_FRAME_END_NOT_DONE (IS_ERROR_ENTRY_MSG_THREAD_DOWN+1)
> +#define IS_ERROR_DRC_FRAME_END_NOT_DONE (IS_ERROR_ISP_FRAME_END_NOT_DONE+1)
> +#define IS_ERROR_SCALERC_FRAME_END_NOT_DONE (IS_ERROR_DRC_FRAME_END_NOT_DONE+1)
> +#define IS_ERROR_ODC_FRAME_END_NOT_DONE (IS_ERROR_SCALERC_FRAME_END_NOT_DONE+1)
> +#define IS_ERROR_DIS_FRAME_END_NOT_DONE (IS_ERROR_ODC_FRAME_END_NOT_DONE+1)
> +#define IS_ERROR_TDNR_FRAME_END_NOT_DONE (IS_ERROR_DIS_FRAME_END_NOT_DONE+1)
> +#define IS_ERROR_SCALERP_FRAME_END_NOT_DONE (IS_ERROR_TDNR_FRAME_END_NOT_DONE+1)
> +#define IS_ERROR_WAIT_STREAM_OFF_NOT_DONE\
> + (IS_ERROR_SCALERP_FRAME_END_NOT_DONE+1)
> +#define IS_ERROR_NO_MSG_IS_RECEIVED (IS_ERROR_WAIT_STREAM_OFF_NOT_DONE+1)
> +#define IS_ERROR_SENSOR_MSG_FAIL (IS_ERROR_NO_MSG_IS_RECEIVED+1)
> +#define IS_ERROR_ISP_MSG_FAIL (IS_ERROR_SENSOR_MSG_FAIL+1)
> +#define IS_ERROR_DRC_MSG_FAIL (IS_ERROR_ISP_MSG_FAIL+1)
> +#define IS_ERROR_SCALERC_MSG_FAIL (IS_ERROR_DRC_MSG_FAIL+1)
> +#define IS_ERROR_ODC_MSG_FAIL (IS_ERROR_SCALERC_MSG_FAIL+1)
> +#define IS_ERROR_DIS_MSG_FAIL (IS_ERROR_ODC_MSG_FAIL+1)
> +#define IS_ERROR_TDNR_MSG_FAIL (IS_ERROR_DIS_MSG_FAIL+1)
> +#define IS_ERROR_SCALERP_MSG_FAIL (IS_ERROR_TDNR_MSG_FAIL+1)
> +#define IS_ERROR_LHFD_MSG_FAIL (IS_ERROR_SCALERP_MSG_FAIL+1)
> +#define IS_ERROR_INTERNAL_STOP (IS_ERROR_LHFD_MSG_FAIL+1)
> +#define IS_ERROR_UNKNOWN 1000
> +#define IS_ERROR_TIME_OUT_FLAG 0x80000000
> +
> +/* Sensor 100 ~ 200 */
> +#define IS_ERROR_SENSOR_PWRDN_FAIL 100
> +#define IS_ERROR_SENSOR_STREAM_ON_FAIL (IS_ERROR_SENSOR_PWRDN_FAIL+1)
> +#define IS_ERROR_SENSOR_STREAM_OFF_FAIL (IS_ERROR_SENSOR_STREAM_ON_FAIL+1)
> +
> +/* ISP 200 ~ 300 */
> +#define IS_ERROR_ISP_PWRDN_FAIL 200
> +#define IS_ERROR_ISP_MULTIPLE_INPUT (IS_ERROR_ISP_PWRDN_FAIL+1)
> +#define IS_ERROR_ISP_ABSENT_INPUT (IS_ERROR_ISP_MULTIPLE_INPUT+1)
> +#define IS_ERROR_ISP_ABSENT_OUTPUT (IS_ERROR_ISP_ABSENT_INPUT+1)
> +#define IS_ERROR_ISP_NONADJACENT_OUTPUT (IS_ERROR_ISP_ABSENT_OUTPUT+1)
> +#define IS_ERROR_ISP_FORMAT_MISMATCH (IS_ERROR_ISP_NONADJACENT_OUTPUT+1)
> +#define IS_ERROR_ISP_WIDTH_MISMATCH (IS_ERROR_ISP_FORMAT_MISMATCH+1)
> +#define IS_ERROR_ISP_HEIGHT_MISMATCH (IS_ERROR_ISP_WIDTH_MISMATCH+1)
> +#define IS_ERROR_ISP_BITWIDTH_MISMATCH (IS_ERROR_ISP_HEIGHT_MISMATCH+1)
> +#define IS_ERROR_ISP_FRAME_END_TIME_OUT (IS_ERROR_ISP_BITWIDTH_MISMATCH+1)
> +
> +/* DRC 300 ~ 400 */
> +#define IS_ERROR_DRC_PWRDN_FAIL 300
> +#define IS_ERROR_DRC_MULTIPLE_INPUT (IS_ERROR_DRC_PWRDN_FAIL+1)
> +#define IS_ERROR_DRC_ABSENT_INPUT (IS_ERROR_DRC_MULTIPLE_INPUT+1)
> +#define IS_ERROR_DRC_NONADJACENT_INTPUT (IS_ERROR_DRC_ABSENT_INPUT+1)
> +#define IS_ERROR_DRC_ABSENT_OUTPUT (IS_ERROR_DRC_NONADJACENT_INTPUT+1)
> +#define IS_ERROR_DRC_NONADJACENT_OUTPUT (IS_ERROR_DRC_ABSENT_OUTPUT+1)
> +#define IS_ERROR_DRC_FORMAT_MISMATCH (IS_ERROR_DRC_NONADJACENT_OUTPUT+1)
> +#define IS_ERROR_DRC_WIDTH_MISMATCH (IS_ERROR_DRC_FORMAT_MISMATCH+1)
> +#define IS_ERROR_DRC_HEIGHT_MISMATCH (IS_ERROR_DRC_WIDTH_MISMATCH+1)
> +#define IS_ERROR_DRC_BITWIDTH_MISMATCH (IS_ERROR_DRC_HEIGHT_MISMATCH+1)
> +#define IS_ERROR_DRC_FRAME_END_TIME_OUT (IS_ERROR_DRC_BITWIDTH_MISMATCH+1)
> +
> +/*SCALERC(400~500)*/
> +#define IS_ERROR_SCALERC_PWRDN_FAIL 400
> +
> +/*ODC(500~600)*/
> +#define IS_ERROR_ODC_PWRDN_FAIL 500
> +
> +/*DIS(600~700)*/
> +#define IS_ERROR_DIS_PWRDN_FAIL 600
> +
> +/*TDNR(700~800)*/
> +#define IS_ERROR_TDNR_PWRDN_FAIL 700
> +
> +/*SCALERP(800~900)*/
> +#define IS_ERROR_SCALERP_PWRDN_FAIL 800
> +
> +/*FD(900~1000)*/
> +#define IS_ERROR_FD_PWRDN_FAIL 900
> +#define IS_ERROR_FD_MULTIPLE_INPUT (IS_ERROR_FD_PWRDN_FAIL+1)
> +#define IS_ERROR_FD_ABSENT_INPUT (IS_ERROR_FD_MULTIPLE_INPUT+1)
> +#define IS_ERROR_FD_NONADJACENT_INPUT (IS_ERROR_FD_ABSENT_INPUT+1)
> +#define IS_ERROR_LHFD_FRAME_END_TIME_OUT \
> + (IS_ERROR_FD_NONADJACENT_INPUT+1)
nit: It feels an enum would be more appropriate for these.
> diff --git a/drivers/media/platform/exynos5-is/fimc-is-param.h b/drivers/media/platform/exynos5-is/fimc-is-param.h
> new file mode 100644
> index 0000000..63eb8d9
> --- /dev/null
> +++ b/drivers/media/platform/exynos5-is/fimc-is-param.h
> @@ -0,0 +1,2163 @@
> +/*
> + * Samsung Exynos5 SoC series FIMC-IS driver
> + *
> + * Copyright (c) 2013 Samsung Electronics Co., Ltd
> + * Kil-yeon Lim<kilyeon.im@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_PARAM_H
> +#define FIMC_IS_PARAM_H
> +
> +#define IS_REGION_VER 145 /* IS REGION VERSION 1.45 */
> +
> +/* MACROs */
> +#define IS_SET_PARAM_BIT(dev, num) \
> + (num>= 32 ? set_bit((num-32),&dev->p_region_index2) \
> + : set_bit(num,&dev->p_region_index1))
> +#define IS_INC_PARAM_NUM(dev) atomic_inc(&dev->p_region_num)
> +
> +#define IS_PARAM_GLOBAL(dev) (dev->is_p_region->parameter.global)
> +#define IS_PARAM_ISP(dev) (dev->is_p_region->parameter.isp)
> +#define IS_PARAM_DRC(dev) (dev->is_p_region->parameter.drc)
> +#define IS_PARAM_FD(dev) (dev->is_p_region->parameter.fd)
> +#define IS_HEADER(dev) (dev->is_p_region->header)
> +#define IS_FACE(dev) (dev->is_p_region->face)
> +#define IS_SHARED(dev) (dev->is_shared_region)
> +#define IS_PARAM_SIZE (FIMC_IS_REGION_SIZE + 1)
> +
> +/* Global control */
> +#define IS_SET_PARAM_GLOBAL_SHOTMODE_CMD(dev, x) \
> + (dev->is_p_region->parameter.global.shotmode.cmd = x)
> +#define IS_SET_PARAM_GLOBAL_SHOTMODE_SKIPFRAMES(dev, x) \
> + (dev->is_p_region->parameter.global.shotmode.skip_frames = x)
> +
> +/* Sensor control */
> +#define IS_SENSOR_SET_FRAME_RATE(dev, x) \
> + (dev->is_p_region->parameter.sensor.frame_rate.frame_rate = x)
> +
> +/* ISP Macros */
> +#define IS_ISP_SET_PARAM_CONTROL_CMD(dev, x) \
> + (dev->is_p_region->parameter.isp.control.cmd = x)
Hmm, these macros look pretty ugly. They obfuscate what's really being done.
I would try to rewrite the code so we use pointers to target data structures
instead.
> +#ifndef BIT0
> +#define BIT0 0x00000001
> +#define BIT1 0x00000002
> +#define BIT2 0x00000004
> +#define BIT3 0x00000008
> +#define BIT4 0x00000010
> +#define BIT5 0x00000020
> +#define BIT6 0x00000040
> +#define BIT7 0x00000080
> +#define BIT8 0x00000100
> +#define BIT9 0x00000200
> +#define BIT10 0x00000400
> +#define BIT11 0x00000800
> +#define BIT12 0x00001000
> +#define BIT13 0x00002000
> +#define BIT14 0x00004000
> +#define BIT15 0x00008000
> +#define BIT16 0x00010000
> +#define BIT17 0x00020000
> +#define BIT18 0x00040000
> +#define BIT19 0x00080000
> +#define BIT20 0x00100000
> +#define BIT21 0x00200000
> +#define BIT22 0x00400000
> +#define BIT23 0x00800000
> +#define BIT24 0x01000000
> +#define BIT25 0x02000000
> +#define BIT26 0x04000000
> +#define BIT27 0x08000000
> +#define BIT28 0x10000000
> +#define BIT29 0x20000000
> +#define BIT30 0x40000000
> +#define BIT31 0x80000000
> +#define BIT32 0x0000000100000000ULL
> +#define BIT33 0x0000000200000000ULL
> +#define BIT34 0x0000000400000000ULL
> +#define BIT35 0x0000000800000000ULL
> +#define BIT36 0x0000001000000000ULL
> +#define BIT37 0x0000002000000000ULL
> +#define BIT38 0x0000004000000000ULL
> +#define BIT39 0x0000008000000000ULL
> +#define BIT40 0x0000010000000000ULL
> +#define BIT41 0x0000020000000000ULL
> +#define BIT42 0x0000040000000000ULL
> +#define BIT43 0x0000080000000000ULL
> +#define BIT44 0x0000100000000000ULL
> +#define BIT45 0x0000200000000000ULL
> +#define BIT46 0x0000400000000000ULL
> +#define BIT47 0x0000800000000000ULL
> +#define BIT48 0x0001000000000000ULL
> +#define BIT49 0x0002000000000000ULL
> +#define BIT50 0x0004000000000000ULL
> +#define BIT51 0x0008000000000000ULL
> +#define BIT52 0x0010000000000000ULL
> +#define BIT53 0x0020000000000000ULL
> +#define BIT54 0x0040000000000000ULL
> +#define BIT55 0x0080000000000000ULL
> +#define BIT56 0x0100000000000000ULL
> +#define BIT57 0x0200000000000000ULL
> +#define BIT58 0x0400000000000000ULL
> +#define BIT59 0x0800000000000000ULL
> +#define BIT60 0x1000000000000000ULL
> +#define BIT61 0x2000000000000000ULL
> +#define BIT62 0x4000000000000000ULL
> +#define BIT63 0x8000000000000000ULL
Are you sure this is a good idea ? What do you think BIT() macro available
in bitops.h does ? And are all these BIT definitions really used anywhere ?
> +#define INC_BIT(bit) (bit<<1)
> +#define INC_NUM(bit) (bit + 1)
> +#endif
> +
> +#define MAGIC_NUMBER 0x01020304
> +
> +#define PARAMETER_MAX_SIZE 128 /* in byte */
> +#define PARAMETER_MAX_MEMBER (PARAMETER_MAX_SIZE/4)
> +
> +enum is_entry {
> + ENTRY_GLOBAL,
> + ENTRY_BUFFER,
> + ENTRY_SENSOR,
> + ENTRY_ISP,
> + ENTRY_DRC,
> + ENTRY_SCALERC,
> + ENTRY_ODC,
> + ENTRY_DIS,
> + ENTRY_TDNR,
> + ENTRY_SCALERP,
> + ENTRY_LHFD, /* 10 */
> + ENTRY_END
Are these used somewhere ?
> +};
> +
> +enum is_param_set_bit {
> + PARAM_GLOBAL_SHOTMODE = 0,
> + PARAM_SENSOR_CONTROL,
> + PARAM_SENSOR_OTF_INPUT,
> + PARAM_SENSOR_OTF_OUTPUT,
> + PARAM_SENSOR_FRAME_RATE,
> + PARAM_SENSOR_DMA_OUTPUT,
> + PARAM_BUFFER_CONTROL,
> + PARAM_BUFFER_OTF_INPUT,
> + PARAM_BUFFER_OTF_OUTPUT,
> + PARAM_ISP_CONTROL,
> + PARAM_ISP_OTF_INPUT = 10,
> + PARAM_ISP_DMA1_INPUT,
> + PARAM_ISP_DMA2_INPUT,
> + PARAM_ISP_AA,
> + PARAM_ISP_FLASH,
> + PARAM_ISP_AWB,
> + PARAM_ISP_IMAGE_EFFECT,
> + PARAM_ISP_ISO,
> + PARAM_ISP_ADJUST,
> + PARAM_ISP_METERING,
> + PARAM_ISP_AFC = 20,
> + PARAM_ISP_OTF_OUTPUT,
> + PARAM_ISP_DMA1_OUTPUT,
> + PARAM_ISP_DMA2_OUTPUT,
> + PARAM_DRC_CONTROL,
> + PARAM_DRC_OTF_INPUT,
> + PARAM_DRC_DMA_INPUT,
> + PARAM_DRC_OTF_OUTPUT,
> + PARAM_SCALERC_CONTROL,
> + PARAM_SCALERC_OTF_INPUT,
> + PARAM_SCALERC_IMAGE_EFFECT = 30,
> + PARAM_SCALERC_INPUT_CROP,
> + PARAM_SCALERC_OUTPUT_CROP,
> + PARAM_SCALERC_OTF_OUTPUT,
> + PARAM_SCALERC_DMA_OUTPUT = 34,
> + PARAM_ODC_CONTROL,
> + PARAM_ODC_OTF_INPUT,
> + PARAM_ODC_OTF_OUTPUT,
> + PARAM_DIS_CONTROL,
> + PARAM_DIS_OTF_INPUT,
> + PARAM_DIS_OTF_OUTPUT = 40,
> + PARAM_TDNR_CONTROL,
> + PARAM_TDNR_OTF_INPUT,
> + PARAM_TDNR_1ST_FRAME,
> + PARAM_TDNR_OTF_OUTPUT,
> + PARAM_TDNR_DMA_OUTPUT,
> + PARAM_SCALERP_CONTROL,
> + PARAM_SCALERP_OTF_INPUT,
> + PARAM_SCALERP_IMAGE_EFFECT,
> + PARAM_SCALERP_INPUT_CROP,
> + PARAM_SCALERP_OUTPUT_CROP = 50,
> + PARAM_SCALERP_ROTATION,
> + PARAM_SCALERP_FLIP,
> + PARAM_SCALERP_OTF_OUTPUT,
> + PARAM_SCALERP_DMA_OUTPUT,
> + PARAM_FD_CONTROL,
> + PARAM_FD_OTF_INPUT,
> + PARAM_FD_DMA_INPUT,
> + PARAM_FD_CONFIG = 58,
> + PARAM_END,
> +};
> +
> +#define ADDRESS_TO_OFFSET(start, end) ((uint32)end - (uint32)start)
> +#define OFFSET_TO_NUM(offset) ((offset)>>6)
> +#define IS_OFFSET_LOWBIT(offset) (OFFSET_TO_NUM(offset)>= \
> + 32 ? false : true)
> +#define OFFSET_TO_BIT(offset) \
> + {(IS_OFFSET_LOWBIT(offset) ? (1<<OFFSET_TO_NUM(offset)) \
> + : (1<<(OFFSET_TO_NUM(offset)-32))}
> +#define LOWBIT_OF_NUM(num) (num>= 32 ? 0 : BIT0<<num)
> +#define HIGHBIT_OF_NUM(num) (num>= 32 ? BIT0<<(num-32) : 0)
> +
> +/* 0~31 */
> +#define PARAM_GLOBAL_SHOTMODE 0
> +#define PARAM_SENSOR_CONTROL INC_NUM(PARAM_GLOBAL_SHOTMODE)
> +#define PARAM_SENSOR_OTF_INPUT INC_NUM(PARAM_SENSOR_CONTROL)
> +#define PARAM_SENSOR_OTF_OUTPUT INC_NUM(PARAM_SENSOR_OTF_INPUT)
> +#define PARAM_SENSOR_FRAME_RATE INC_NUM(PARAM_SENSOR_OTF_OUTPUT)
> +#define PARAM_SENSOR_DMA_OUTPUT INC_NUM(PARAM_SENSOR_FRAME_RATE)
> +#define PARAM_BUFFER_CONTROL INC_NUM(PARAM_SENSOR_DMA_OUTPUT)
> +#define PARAM_BUFFER_OTF_INPUT INC_NUM(PARAM_BUFFER_CONTROL)
> +#define PARAM_BUFFER_OTF_OUTPUT INC_NUM(PARAM_BUFFER_OTF_INPUT)
> +#define PARAM_ISP_CONTROL INC_NUM(PARAM_BUFFER_OTF_OUTPUT)
> +#define PARAM_ISP_OTF_INPUT INC_NUM(PARAM_ISP_CONTROL)
> +#define PARAM_ISP_DMA1_INPUT INC_NUM(PARAM_ISP_OTF_INPUT)
> +#define PARAM_ISP_DMA2_INPUT INC_NUM(PARAM_ISP_DMA1_INPUT)
> +#define PARAM_ISP_AA INC_NUM(PARAM_ISP_DMA2_INPUT)
> +#define PARAM_ISP_FLASH INC_NUM(PARAM_ISP_AA)
> +#define PARAM_ISP_AWB INC_NUM(PARAM_ISP_FLASH)
> +#define PARAM_ISP_IMAGE_EFFECT INC_NUM(PARAM_ISP_AWB)
> +#define PARAM_ISP_ISO INC_NUM(PARAM_ISP_IMAGE_EFFECT)
> +#define PARAM_ISP_ADJUST INC_NUM(PARAM_ISP_ISO)
> +#define PARAM_ISP_METERING INC_NUM(PARAM_ISP_ADJUST)
> +#define PARAM_ISP_AFC INC_NUM(PARAM_ISP_METERING)
> +#define PARAM_ISP_OTF_OUTPUT INC_NUM(PARAM_ISP_AFC)
> +#define PARAM_ISP_DMA1_OUTPUT INC_NUM(PARAM_ISP_OTF_OUTPUT)
> +#define PARAM_ISP_DMA2_OUTPUT INC_NUM(PARAM_ISP_DMA1_OUTPUT)
> +#define PARAM_DRC_CONTROL INC_NUM(PARAM_ISP_DMA2_OUTPUT)
> +#define PARAM_DRC_OTF_INPUT INC_NUM(PARAM_DRC_CONTROL)
> +#define PARAM_DRC_DMA_INPUT INC_NUM(PARAM_DRC_OTF_INPUT)
> +#define PARAM_DRC_OTF_OUTPUT INC_NUM(PARAM_DRC_DMA_INPUT)
> +#define PARAM_SCALERC_CONTROL INC_NUM(PARAM_DRC_OTF_OUTPUT)
> +#define PARAM_SCALERC_OTF_INPUT INC_NUM(PARAM_SCALERC_CONTROL)
> +#define PARAM_SCALERC_IMAGE_EFFECT INC_NUM(PARAM_SCALERC_OTF_INPUT)
> +#define PARAM_SCALERC_INPUT_CROP INC_NUM(PARAM_SCALERC_IMAGE_EFFECT)
> +#define PARAM_SCALERC_OUTPUT_CROP INC_NUM(PARAM_SCALERC_INPUT_CROP)
> +#define PARAM_SCALERC_OTF_OUTPUT INC_NUM(PARAM_SCALERC_OUTPUT_CROP)
>
> +/* 32~63 */
> +#define PARAM_SCALERC_DMA_OUTPUT INC_NUM(PARAM_SCALERC_OTF_OUTPUT)
> +#define PARAM_ODC_CONTROL INC_NUM(PARAM_SCALERC_DMA_OUTPUT)
> +#define PARAM_ODC_OTF_INPUT INC_NUM(PARAM_ODC_CONTROL)
> +#define PARAM_ODC_OTF_OUTPUT INC_NUM(PARAM_ODC_OTF_INPUT)
> +#define PARAM_DIS_CONTROL INC_NUM(PARAM_ODC_OTF_OUTPUT)
> +#define PARAM_DIS_OTF_INPUT INC_NUM(PARAM_DIS_CONTROL)
> +#define PARAM_DIS_OTF_OUTPUT INC_NUM(PARAM_DIS_OTF_INPUT)
> +#define PARAM_TDNR_CONTROL INC_NUM(PARAM_DIS_OTF_OUTPUT)
> +#define PARAM_TDNR_OTF_INPUT INC_NUM(PARAM_TDNR_CONTROL)
> +#define PARAM_TDNR_1ST_FRAME INC_NUM(PARAM_TDNR_OTF_INPUT)
> +#define PARAM_TDNR_OTF_OUTPUT INC_NUM(PARAM_TDNR_1ST_FRAME)
> +#define PARAM_TDNR_DMA_OUTPUT INC_NUM(PARAM_TDNR_OTF_OUTPUT)
> +#define PARAM_SCALERP_CONTROL INC_NUM(PARAM_TDNR_DMA_OUTPUT)
> +#define PARAM_SCALERP_OTF_INPUT INC_NUM(PARAM_SCALERP_CONTROL)
> +#define PARAM_SCALERP_IMAGE_EFFECT INC_NUM(PARAM_SCALERP_OTF_INPUT)
> +#define PARAM_SCALERP_INPUT_CROP INC_NUM(PARAM_SCALERP_IMAGE_EFFECT)
> +#define PARAM_SCALERP_OUTPUT_CROP INC_NUM(PARAM_SCALERP_INPUT_CROP)
> +#define PARAM_SCALERP_ROTATION INC_NUM(PARAM_SCALERP_OUTPUT_CROP)
> +#define PARAM_SCALERP_FLIP INC_NUM(PARAM_SCALERP_ROTATION)
> +#define PARAM_SCALERP_OTF_OUTPUT INC_NUM(PARAM_SCALERP_FLIP)
> +#define PARAM_SCALERP_DMA_OUTPUT INC_NUM(PARAM_SCALERP_OTF_OUTPUT)
> +#define PARAM_FD_CONTROL INC_NUM(PARAM_SCALERP_DMA_OUTPUT)
> +#define PARAM_FD_OTF_INPUT INC_NUM(PARAM_FD_CONTROL)
> +#define PARAM_FD_DMA_INPUT INC_NUM(PARAM_FD_OTF_INPUT)
> +#define PARAM_FD_CONFIG INC_NUM(PARAM_FD_DMA_INPUT)
> +#define PARAM_END INC_NUM(PARAM_FD_CONFIG)
Isn't it much easier to define these as enum ?
> +#define PARAM_STRNUM_GLOBAL (PARAM_GLOBAL_SHOTMODE)
> +#define PARAM_RANGE_GLOBAL 1
> +#define PARAM_STRNUM_SENSOR (PARAM_SENSOR_BYPASS)
> +#define PARAM_RANGE_SENSOR 5
> +#define PARAM_STRNUM_BUFFER (PARAM_BUFFER_BYPASS)
> +#define PARAM_RANGE_BUFFER 3
> +#define PARAM_STRNUM_ISP (PARAM_ISP_BYPASS)
> +#define PARAM_RANGE_ISP 15
> +#define PARAM_STRNUM_DRC (PARAM_DRC_BYPASS)
> +#define PARAM_RANGE_DRC 4
> +#define PARAM_STRNUM_SCALERC (PARAM_SCALERC_BYPASS)
> +#define PARAM_RANGE_SCALERC 7
> +#define PARAM_STRNUM_ODC (PARAM_ODC_BYPASS)
> +#define PARAM_RANGE_ODC 3
> +#define PARAM_STRNUM_DIS (PARAM_DIS_BYPASS)
> +#define PARAM_RANGE_DIS 3
> +#define PARAM_STRNUM_TDNR (PARAM_TDNR_BYPASS)
> +#define PARAM_RANGE_TDNR 5
> +#define PARAM_STRNUM_SCALERP (PARAM_SCALERP_BYPASS)
> +#define PARAM_RANGE_SCALERP 9
> +#define PARAM_STRNUM_LHFD (PARAM_FD_BYPASS)
> +#define PARAM_RANGE_LHFD 4
Are these definitions used anywhere ?
> +#define PARAM_LOW_MASK (0xFFFFFFFF)
> +#define PARAM_HIGH_MASK (0x07FFFFFF)
> +
[...]
> +struct param_dma_input {
> + u32 cmd;
> + u32 width;
> + u32 height;
> + u32 format;
> + u32 bitwidth;
> + u32 plane;
> + u32 order;
> + u32 buffer_number;
> + u32 buffer_address;
> + u32 uibayercropoffsetx;
> + u32 uibayercropoffsety;
> + u32 uibayercropwidth;
> + u32 uibayercropheight;
> + u32 uidmacropoffsetx;
> + u32 uidmacropoffsety;
> + u32 uidmacropwidth;
> + u32 uidmacropheight;
> + u32 uiuserminframetime;
> + u32 uiusermaxframetime;
That is really hard to read. Perhaps we can have some underscores
added so it looks like ui_user_max_frametime ?
> + u32 uiwideframegap;
> + u32 uiframegap;
> + u32 uilinegap;
> + u32 uireserved[PARAMETER_MAX_MEMBER-23];
> + u32 err;
> +};
next prev parent reply other threads:[~2013-03-23 14:05 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-03-08 14:59 [RFC 00/12] Exynos5 FIMC-IS driver Arun Kumar K
2013-03-08 14:59 ` [RFC 01/12] exynos-fimc-is: Adding device tree nodes Arun Kumar K
2013-03-23 13:14 ` Sylwester Nawrocki
2013-03-26 12:17 ` Arun Kumar K
2013-03-26 22:51 ` Sylwester Nawrocki
2013-03-27 4:31 ` Arun Kumar K
2013-03-27 13:47 ` Sylwester Nawrocki
2013-03-28 5:10 ` Arun Kumar K
2013-03-29 0:30 ` Sylwester Nawrocki
2013-04-10 4:32 ` Arun Kumar K
2013-03-08 14:59 ` [RFC 02/12] exynos-fimc-is: Adding ARCH support for fimc-is Arun Kumar K
2013-03-08 14:59 ` [RFC 03/12] exynos-fimc-is: Adds fimc-is driver core files Arun Kumar K
2013-03-23 13:41 ` Sylwester Nawrocki
2013-03-08 14:59 ` [RFC 04/12] exynos-fimc-is: Adds common driver header files Arun Kumar K
2013-03-23 14:05 ` Sylwester Nawrocki [this message]
2013-03-08 14:59 ` [RFC 05/12] exynos-fimc-is: Adds the register definition and context header Arun Kumar K
2013-03-23 14:20 ` Sylwester Nawrocki
2013-03-08 14:59 ` [RFC 06/12] exynos-fimc-is: Adds the sensor subdev Arun Kumar K
2013-03-23 14:48 ` Sylwester Nawrocki
2013-03-08 14:59 ` [RFC 07/12] exynos-fimc-is: Adds isp subdev Arun Kumar K
2013-03-23 18:38 ` Sylwester Nawrocki
2013-03-08 14:59 ` [RFC 08/12] exynos-fimc-is: Adds scaler subdev Arun Kumar K
2013-03-08 14:59 ` [RFC 09/12] exynos-fimc-is: Adds the hardware pipeline control Arun Kumar K
2013-03-08 14:59 ` [RFC 10/12] exynos-fimc-is: Adds the hardware interface module Arun Kumar K
2013-03-23 19:01 ` Sylwester Nawrocki
2013-03-08 14:59 ` [RFC 11/12] exynos-fimc-is: Adds the Kconfig and Makefile Arun Kumar K
2013-03-23 19:02 ` Sylwester Nawrocki
2013-03-08 14:59 ` [RFC 12/12] mipi-csis: Enable all interrupts for fimc-is usage Arun Kumar K
2013-03-12 16:01 ` Sylwester Nawrocki
2013-03-13 4:09 ` Arun Kumar K
2013-04-03 12:31 ` Sylwester Nawrocki
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=514DB6B7.1050304@gmail.com \
--to=sylvester.nawrocki@gmail.com \
--cc=arun.kk@samsung.com \
--cc=arunkk.samsung@gmail.com \
--cc=devicetree-discuss@lists.ozlabs.org \
--cc=kgene.kim@samsung.com \
--cc=kilyeon.im@samsung.com \
--cc=linux-media@vger.kernel.org \
--cc=linux-samsung-soc@vger.kernel.org \
--cc=s.nawrocki@samsung.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).