RE: [PATCH v2 1/2] media: atmel: atmel-isc: reworked driver and formats

From: Ken Sloat
Date: Wed Mar 13 2019 - 13:34:00 EST

> -----Original Message-----
> From: Eugen.Hristev@xxxxxxxxxxxxx <Eugen.Hristev@xxxxxxxxxxxxx>
> Sent: Wednesday, March 13, 2019 3:25 AM
> To: hverkuil@xxxxxxxxx; Nicolas.Ferre@xxxxxxxxxxxxx; linux-
> media@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; mchehab@xxxxxxxxxx
> Cc: Ken Sloat <KSloat@xxxxxxxxxxxxxx>; Eugen.Hristev@xxxxxxxxxxxxx
> Subject: [PATCH v2 1/2] media: atmel: atmel-isc: reworked driver and formats
>
> From: Eugen Hristev <eugen.hristev@xxxxxxxxxxxxx>
>
> This change is a redesign in the formats and the way the ISC is configured w.r.t.
> sensor format and the output format from the ISC.
> I have changed the splitting between sensor output (which is also ISC input) and
> ISC output.
> The sensor format represents the way the sensor is configured, and what ISC is
> receiving.
> The format configuration represents the way ISC is interpreting the data and
> formatting the output to the subsystem.
> Now it's much easier to figure out what is the ISC configuration for input, and
> what is the configuration for output.
> The non-raw format can be obtained directly from sensor or it can be done
> inside the ISC. The controller format list will include a configuration for each
> format.
> The old supported formats are still in place, if we want to dump the sensor
> format directly to the output, the try format routine will detect and configure
> the pipeline accordingly.
> This also fixes the previous issues when the raw format was NULL which
> resulted in many crashes for sensors which did not have the expected/tested
> formats.
>
> Signed-off-by: Eugen Hristev <eugen.hristev@xxxxxxxxxxxxx>
> ---
>
> Hello,
>
> I have changed the try vs set settings, with having another configuration for try,
> which will be copied to the actual configuration only after set will be called.
>
> This patch keeps the original formats. I added a second patch that fixes ARGB
> format w.r.t. byte endianess inside the format.
>
> Changes in v2:
> - now have try_config and also config, all configuration setting will be done
> initially on try_config, and then if everything is OK, in set_fmt, it will be copied
> to the actual config.
> - changed macro IS_RAW to apply on mbus code and not isc structure, it's now
> called ISC_IS_FORMAT_RAW and it's called everywhere it's needed.
> - renamed all functions for configure modules (dma, rlp, pipeline, formats)
> with 'try' prefix
>
> tested with sensors ov7670, ov7740, ov5640
>
> v4l2-compliance:
>
> v4l2-compliance SHA: 32cf495ff5da24df54936fae3bf0eb91fba77f3a, 32 bits
>
> Compliance test for atmel_isc device /atmel_deo0:isc f0008000.isc:
> ================= START STATUS ================= atmel_isc
> f0008000.isc: Brightness: 0 atmel_isc f0008000.isc: Contrast: 256 atmel_isc
> f0008000.isc: Gamma: 2
>
> atmeiver Info:
> Driver name : atmel_isc
> Card type : Atmel Image Sensor Controller
> Bus info : platform:atmel_isc f0008000.isc
> Driver version : 5.0.0
> Capabilities : 0x84200001
> Video Capture
> Streaming
> Extended Pix Format
> Device Capabilities
> Device Caps : 0x04200001
> Video Capture
> Streaming
> Extended Pix Format
>
> Required ioctls:
> test VIDIOC_QUERYCAP: OK
>
> Allow for multiple opens:
> test second /dev/video0 open: OK
> test VIDIOC_QUERYCAP: OK
> test VIDIOC_G/S_PRIORITY: OK
> test for unlimited opens: OK
>
> Debug ioctls:
> test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported) l_isc f0008000.isc:
> White Balance, Automatic: true atmel_isc f0008000.isc: ==================
> END STATUS ==================
> test VIDIOC_LOG_STATUS: OK
>
> Input ioctls:
> test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
> test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
> test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
> test VIDIOC_ENUMAUDIO: OK (Not Supported)
> test VIDIOC_G/S/ENUMINPUT: OK
> test VIDIOC_G/S_AUDIO: OK (Not Supported)
> Inputs: 1 Audio Inputs: 0 Tuners: 0
>
> Output ioctls:
> test VIDIOC_G/S_MODULATOR: OK (Not Supported)
> test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
> test VIDIOC_ENUMAUDOUT: OK (Not Supported)
> test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
> test VIDIOC_G/S_AUDOUT: OK (Not Supported)
> Outputs: 0 Audio Outputs: 0 Modulators: 0
>
> Input/Output configuration ioctls:
> test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
> test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
> test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
> test VIDIOC_G/S_EDID: OK (Not Supported)
>
> Control ioctls (Input 0):
> test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
> test VIDIOC_QUERYCTRL: OK
> test VIDIOC_G/S_CTRL: OK
> test VIDIOC_G/S/TRY_EXT_CTRLS: OK
> test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK
> test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
> Standard Controls: 5 Private Controls: 0
>
> Format ioctls (Input 0):
> test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
> test VIDIOC_G/S_PARM: OK
> test VIDIOC_G_FBUF: OK (Not Supported)
> test VIDIOC_G_FMT: OK
> test VIDIOC_TRY_FMT: OK
> test VIDIOC_S_FMT: OK
> test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
> test Cropping: OK (Not Supported)
> test Composing: OK (Not Supported)
> test Scaling: OK (Not Supported)
>
> Codec ioctls (Input 0):
> test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
> test VIDIOC_G_ENC_INDEX: OK (Not Supported)
> test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)
>
> Buffer ioctls (Input 0):
> test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
> test VIDIOC_EXPBUF: OK
> test Requests: OK (Not Supported)
>
> Total for atmel_isc device /dev/video0: 44, Succeeded: 44, Failed: 0, Warnings:
> 0
>
>
> drivers/media/platform/atmel/atmel-isc.c | 888 ++++++++++++++++-------------
> --
> 1 file changed, 471 insertions(+), 417 deletions(-)
>
> diff --git a/drivers/media/platform/atmel/atmel-isc.c
> b/drivers/media/platform/atmel/atmel-isc.c
> index 5017896..f4ecb24 100644
> --- a/drivers/media/platform/atmel/atmel-isc.c
> +++ b/drivers/media/platform/atmel/atmel-isc.c
> @@ -89,35 +89,25 @@ struct isc_subdev_entity {
> struct list_head list;
> };
>
> -/* Indicate the format is generated by the sensor */
> -#define FMT_FLAG_FROM_SENSOR BIT(0)
> -/* Indicate the format is produced by ISC itself */
> -#define FMT_FLAG_FROM_CONTROLLER BIT(1)
> -/* Indicate a Raw Bayer format */
> -#define FMT_FLAG_RAW_FORMAT BIT(2)
> -
> -#define FMT_FLAG_RAW_FROM_SENSOR (FMT_FLAG_FROM_SENSOR |
> \
> - FMT_FLAG_RAW_FORMAT)
> -
> /*
> * struct isc_format - ISC media bus format information
> + This structure represents the interface between the ISC
> + and the sensor. It's the input format received by
> + the ISC.
> * @fourcc: Fourcc code for this format
> * @mbus_code: V4L2 media bus format code.
> - * flags: Indicate format from sensor or converted by controller
> - * @bpp: Bits per pixel (when stored in memory)
> - * (when transferred over a bus)
> - * @sd_support: Subdev supports this format
> - * @isc_support: ISC can convert raw format to this format
> + * @cfa_baycfg: If this format is RAW BAYER, indicate the type
> of bayer.
> + this is either BGBG, RGRG, etc.
> + * @pfe_cfg0_bps: Number of hardware data lines connected to the ISC
> */
>
> struct isc_format {
> u32 fourcc;
> u32 mbus_code;
> - u32 flags;
> - u8 bpp;
> + u32 cfa_baycfg;
>
> bool sd_support;
> - bool isc_support;
> + u32 pfe_cfg0_bps;
> };
>
> /* Pipeline bitmap */
> @@ -135,16 +125,31 @@ struct isc_format {
>
> #define GAM_ENABLES (GAM_RENABLE | GAM_GENABLE | GAM_BENABLE |
> GAM_ENABLE)
>
> +/*
> + * struct fmt_config - ISC format configuration and internal pipeline
> + This structure represents the internal configuration
> + of the ISC.
> + It also holds the format that ISC will present to v4l2.
> + * @sd_format: Pointer to an isc_format struct that holds the
> sensor
> + configuration.
> + * @fourcc: Fourcc code for this format.
> + * @bpp: Bytes per pixel in the current format.
> + * @rlp_cfg_mode: Configuration of the RLP (rounding, limiting packaging)
> + * @dcfg_imode: Configuration of the input of the DMA module
> + * @dctrl_dview: Configuration of the output of the DMA module
> + * @bits_pipeline: Configuration of the pipeline, which modules are
> enabled
> + */
> struct fmt_config {
> - u32 fourcc;
> + struct isc_format *sd_format;
>
> - u32 pfe_cfg0_bps;
> - u32 cfa_baycfg;
> - u32 rlp_cfg_mode;
> - u32 dcfg_imode;
> - u32 dctrl_dview;
> + u32 fourcc;
> + u8 bpp;
>
> - u32 bits_pipeline;
> + u32 rlp_cfg_mode;
> + u32 dcfg_imode;
> + u32 dctrl_dview;
> +
> + u32 bits_pipeline;
> };
>
> #define HIST_ENTRIES 512
> @@ -196,8 +201,9 @@ struct isc_device {
> struct v4l2_format fmt;
> struct isc_format **user_formats;
> unsigned int num_user_formats;
> - const struct isc_format *current_fmt;
> - const struct isc_format *raw_fmt;
> +
> + struct fmt_config config;
> + struct fmt_config try_config;
>
> struct isc_ctrls ctrls;
> struct work_struct awb_work;
> @@ -210,319 +216,122 @@ struct isc_device {
> struct list_head subdev_entities;
> };
>
> -static struct isc_format formats_list[] = {
> - {
> - .fourcc = V4L2_PIX_FMT_SBGGR8,
> - .mbus_code = MEDIA_BUS_FMT_SBGGR8_1X8,
> - .flags = FMT_FLAG_RAW_FROM_SENSOR,
> - .bpp = 8,
> - },
> +/* This is a list of the formats that the ISC can *output* */ static
> +struct isc_format controller_formats[] = {
> {
> - .fourcc = V4L2_PIX_FMT_SGBRG8,
> - .mbus_code = MEDIA_BUS_FMT_SGBRG8_1X8,
> - .flags = FMT_FLAG_RAW_FROM_SENSOR,
> - .bpp = 8,
> - },
> - {
> - .fourcc = V4L2_PIX_FMT_SGRBG8,
> - .mbus_code = MEDIA_BUS_FMT_SGRBG8_1X8,
> - .flags = FMT_FLAG_RAW_FROM_SENSOR,
> - .bpp = 8,
> - },
> - {
> - .fourcc = V4L2_PIX_FMT_SRGGB8,
> - .mbus_code = MEDIA_BUS_FMT_SRGGB8_1X8,
> - .flags = FMT_FLAG_RAW_FROM_SENSOR,
> - .bpp = 8,
> - },
> - {
> - .fourcc = V4L2_PIX_FMT_SBGGR10,
> - .mbus_code = MEDIA_BUS_FMT_SBGGR10_1X10,
> - .flags = FMT_FLAG_RAW_FROM_SENSOR,
> - .bpp = 16,
> - },
> - {
> - .fourcc = V4L2_PIX_FMT_SGBRG10,
> - .mbus_code = MEDIA_BUS_FMT_SGBRG10_1X10,
> - .flags = FMT_FLAG_RAW_FROM_SENSOR,
> - .bpp = 16,
> - },
> - {
> - .fourcc = V4L2_PIX_FMT_SGRBG10,
> - .mbus_code = MEDIA_BUS_FMT_SGRBG10_1X10,
> - .flags = FMT_FLAG_RAW_FROM_SENSOR,
> - .bpp = 16,
> - },
> - {
> - .fourcc = V4L2_PIX_FMT_SRGGB10,
> - .mbus_code = MEDIA_BUS_FMT_SRGGB10_1X10,
> - .flags = FMT_FLAG_RAW_FROM_SENSOR,
> - .bpp = 16,
> + .fourcc = V4L2_PIX_FMT_ARGB444,
> },
> {
> - .fourcc = V4L2_PIX_FMT_SBGGR12,
> - .mbus_code = MEDIA_BUS_FMT_SBGGR12_1X12,
> - .flags = FMT_FLAG_RAW_FROM_SENSOR,
> - .bpp = 16,
> + .fourcc = V4L2_PIX_FMT_ARGB555,
> },
> {
> - .fourcc = V4L2_PIX_FMT_SGBRG12,
> - .mbus_code = MEDIA_BUS_FMT_SGBRG12_1X12,
> - .flags = FMT_FLAG_RAW_FROM_SENSOR,
> - .bpp = 16,
> + .fourcc = V4L2_PIX_FMT_RGB565,
> },
> {
> - .fourcc = V4L2_PIX_FMT_SGRBG12,
> - .mbus_code = MEDIA_BUS_FMT_SGRBG12_1X12,
> - .flags = FMT_FLAG_RAW_FROM_SENSOR,
> - .bpp = 16,
> + .fourcc = V4L2_PIX_FMT_ARGB32,
> },
> {
> - .fourcc = V4L2_PIX_FMT_SRGGB12,
> - .mbus_code = MEDIA_BUS_FMT_SRGGB12_1X12,
> - .flags = FMT_FLAG_RAW_FROM_SENSOR,
> - .bpp = 16,
> + .fourcc = V4L2_PIX_FMT_YUV420,
> },
> {
> - .fourcc = V4L2_PIX_FMT_YUV420,
> - .mbus_code = 0x0,
> - .flags = FMT_FLAG_FROM_CONTROLLER,
> - .bpp = 12,
> + .fourcc = V4L2_PIX_FMT_YUYV,
> },
> {
> .fourcc = V4L2_PIX_FMT_YUV422P,
> - .mbus_code = 0x0,
> - .flags = FMT_FLAG_FROM_CONTROLLER,
> - .bpp = 16,
> },
> {
> .fourcc = V4L2_PIX_FMT_GREY,
> - .mbus_code = MEDIA_BUS_FMT_Y8_1X8,
> - .flags = FMT_FLAG_FROM_CONTROLLER |
> - FMT_FLAG_FROM_SENSOR,
> - .bpp = 8,
> - },
> - {
> - .fourcc = V4L2_PIX_FMT_ARGB444,
> - .mbus_code = MEDIA_BUS_FMT_RGB444_2X8_PADHI_LE,
> - .flags = FMT_FLAG_FROM_CONTROLLER,
> - .bpp = 16,
> - },
> - {
> - .fourcc = V4L2_PIX_FMT_ARGB555,
> - .mbus_code = MEDIA_BUS_FMT_RGB555_2X8_PADHI_LE,
> - .flags = FMT_FLAG_FROM_CONTROLLER,
> - .bpp = 16,
> - },
> - {
> - .fourcc = V4L2_PIX_FMT_RGB565,
> - .mbus_code = MEDIA_BUS_FMT_RGB565_2X8_LE,
> - .flags = FMT_FLAG_FROM_CONTROLLER,
> - .bpp = 16,
> - },
> - {
> - .fourcc = V4L2_PIX_FMT_ARGB32,
> - .mbus_code = MEDIA_BUS_FMT_ARGB8888_1X32,
> - .flags = FMT_FLAG_FROM_CONTROLLER,
> - .bpp = 32,
> - },
> - {
> - .fourcc = V4L2_PIX_FMT_YUYV,
> - .mbus_code = MEDIA_BUS_FMT_YUYV8_2X8,
> - .flags = FMT_FLAG_FROM_CONTROLLER |
> - FMT_FLAG_FROM_SENSOR,
> - .bpp = 16,
> },
> };
>
> -static struct fmt_config fmt_configs_list[] = {
> +/* This is a list of formats that the ISC can receive as *input* */
> +static struct isc_format formats_list[] = {
> {
> .fourcc = V4L2_PIX_FMT_SBGGR8,
> + .mbus_code = MEDIA_BUS_FMT_SBGGR8_1X8,
> .pfe_cfg0_bps = ISC_PFE_CFG0_BPS_EIGHT,
> .cfa_baycfg = ISC_BAY_CFG_BGBG,
> - .rlp_cfg_mode = ISC_RLP_CFG_MODE_DAT8,
> - .dcfg_imode = ISC_DCFG_IMODE_PACKED8,
> - .dctrl_dview = ISC_DCTRL_DVIEW_PACKED,
> - .bits_pipeline = 0x0,
> },
> {
> .fourcc = V4L2_PIX_FMT_SGBRG8,
> + .mbus_code = MEDIA_BUS_FMT_SGBRG8_1X8,
> .pfe_cfg0_bps = ISC_PFE_CFG0_BPS_EIGHT,
> .cfa_baycfg = ISC_BAY_CFG_GBGB,
> - .rlp_cfg_mode = ISC_RLP_CFG_MODE_DAT8,
> - .dcfg_imode = ISC_DCFG_IMODE_PACKED8,
> - .dctrl_dview = ISC_DCTRL_DVIEW_PACKED,
> - .bits_pipeline = 0x0,
> },
> {
> .fourcc = V4L2_PIX_FMT_SGRBG8,
> + .mbus_code = MEDIA_BUS_FMT_SGRBG8_1X8,
> .pfe_cfg0_bps = ISC_PFE_CFG0_BPS_EIGHT,
> .cfa_baycfg = ISC_BAY_CFG_GRGR,
> - .rlp_cfg_mode = ISC_RLP_CFG_MODE_DAT8,
> - .dcfg_imode = ISC_DCFG_IMODE_PACKED8,
> - .dctrl_dview = ISC_DCTRL_DVIEW_PACKED,
> - .bits_pipeline = 0x0,
> },
> {
> .fourcc = V4L2_PIX_FMT_SRGGB8,
> + .mbus_code = MEDIA_BUS_FMT_SRGGB8_1X8,
> .pfe_cfg0_bps = ISC_PFE_CFG0_BPS_EIGHT,
> .cfa_baycfg = ISC_BAY_CFG_RGRG,
> - .rlp_cfg_mode = ISC_RLP_CFG_MODE_DAT8,
> - .dcfg_imode = ISC_DCFG_IMODE_PACKED8,
> - .dctrl_dview = ISC_DCTRL_DVIEW_PACKED,
> - .bits_pipeline = 0x0,
> },
> {
> .fourcc = V4L2_PIX_FMT_SBGGR10,
> + .mbus_code = MEDIA_BUS_FMT_SBGGR10_1X10,
> .pfe_cfg0_bps = ISC_PFG_CFG0_BPS_TEN,
> - .cfa_baycfg = ISC_BAY_CFG_BGBG,
> - .rlp_cfg_mode = ISC_RLP_CFG_MODE_DAT10,
> - .dcfg_imode = ISC_DCFG_IMODE_PACKED16,
> - .dctrl_dview = ISC_DCTRL_DVIEW_PACKED,
> - .bits_pipeline = 0x0,
> + .cfa_baycfg = ISC_BAY_CFG_RGRG,
> },
> {
> .fourcc = V4L2_PIX_FMT_SGBRG10,
> + .mbus_code = MEDIA_BUS_FMT_SGBRG10_1X10,
> .pfe_cfg0_bps = ISC_PFG_CFG0_BPS_TEN,
> .cfa_baycfg = ISC_BAY_CFG_GBGB,
> - .rlp_cfg_mode = ISC_RLP_CFG_MODE_DAT10,
> - .dcfg_imode = ISC_DCFG_IMODE_PACKED16,
> - .dctrl_dview = ISC_DCTRL_DVIEW_PACKED,
> - .bits_pipeline = 0x0,
> },
> {
> .fourcc = V4L2_PIX_FMT_SGRBG10,
> + .mbus_code = MEDIA_BUS_FMT_SGRBG10_1X10,
> .pfe_cfg0_bps = ISC_PFG_CFG0_BPS_TEN,
> .cfa_baycfg = ISC_BAY_CFG_GRGR,
> - .rlp_cfg_mode = ISC_RLP_CFG_MODE_DAT10,
> - .dcfg_imode = ISC_DCFG_IMODE_PACKED16,
> - .dctrl_dview = ISC_DCTRL_DVIEW_PACKED,
> - .bits_pipeline = 0x0,
> },
> {
> .fourcc = V4L2_PIX_FMT_SRGGB10,
> + .mbus_code = MEDIA_BUS_FMT_SRGGB10_1X10,
> .pfe_cfg0_bps = ISC_PFG_CFG0_BPS_TEN,
> .cfa_baycfg = ISC_BAY_CFG_RGRG,
> - .rlp_cfg_mode = ISC_RLP_CFG_MODE_DAT10,
> - .dcfg_imode = ISC_DCFG_IMODE_PACKED16,
> - .dctrl_dview = ISC_DCTRL_DVIEW_PACKED,
> - .bits_pipeline = 0x0,
> },
> {
> .fourcc = V4L2_PIX_FMT_SBGGR12,
> + .mbus_code = MEDIA_BUS_FMT_SBGGR12_1X12,
> .pfe_cfg0_bps = ISC_PFG_CFG0_BPS_TWELVE,
> .cfa_baycfg = ISC_BAY_CFG_BGBG,
> - .rlp_cfg_mode = ISC_RLP_CFG_MODE_DAT12,
> - .dcfg_imode = ISC_DCFG_IMODE_PACKED16,
> - .dctrl_dview = ISC_DCTRL_DVIEW_PACKED,
> - .bits_pipeline = 0x0,
> },
> {
> .fourcc = V4L2_PIX_FMT_SGBRG12,
> + .mbus_code = MEDIA_BUS_FMT_SGBRG12_1X12,
> .pfe_cfg0_bps = ISC_PFG_CFG0_BPS_TWELVE,
> .cfa_baycfg = ISC_BAY_CFG_GBGB,
> - .rlp_cfg_mode = ISC_RLP_CFG_MODE_DAT12,
> - .dcfg_imode = ISC_DCFG_IMODE_PACKED16,
> - .dctrl_dview = ISC_DCTRL_DVIEW_PACKED,
> - .bits_pipeline = 0x0
> },
> {
> .fourcc = V4L2_PIX_FMT_SGRBG12,
> + .mbus_code = MEDIA_BUS_FMT_SGRBG12_1X12,
> .pfe_cfg0_bps = ISC_PFG_CFG0_BPS_TWELVE,
> .cfa_baycfg = ISC_BAY_CFG_GRGR,
> - .rlp_cfg_mode = ISC_RLP_CFG_MODE_DAT12,
> - .dcfg_imode = ISC_DCFG_IMODE_PACKED16,
> - .dctrl_dview = ISC_DCTRL_DVIEW_PACKED,
> - .bits_pipeline = 0x0,
> },
> {
> .fourcc = V4L2_PIX_FMT_SRGGB12,
> + .mbus_code = MEDIA_BUS_FMT_SRGGB12_1X12,
> .pfe_cfg0_bps = ISC_PFG_CFG0_BPS_TWELVE,
> .cfa_baycfg = ISC_BAY_CFG_RGRG,
> - .rlp_cfg_mode = ISC_RLP_CFG_MODE_DAT12,
> - .dcfg_imode = ISC_DCFG_IMODE_PACKED16,
> - .dctrl_dview = ISC_DCTRL_DVIEW_PACKED,
> - .bits_pipeline = 0x0,
> - },
> - {
> - .fourcc = V4L2_PIX_FMT_YUV420,
> - .pfe_cfg0_bps = ISC_PFE_CFG0_BPS_EIGHT,
> - .cfa_baycfg = ISC_BAY_CFG_BGBG,
> - .rlp_cfg_mode = ISC_RLP_CFG_MODE_YYCC,
> - .dcfg_imode = ISC_DCFG_IMODE_YC420P,
> - .dctrl_dview = ISC_DCTRL_DVIEW_PLANAR,
> - .bits_pipeline = SUB420_ENABLE | SUB422_ENABLE |
> - CBC_ENABLE | CSC_ENABLE |
> - GAM_ENABLES |
> - CFA_ENABLE | WB_ENABLE,
> - },
> - {
> - .fourcc = V4L2_PIX_FMT_YUV422P,
> - .pfe_cfg0_bps = ISC_PFE_CFG0_BPS_EIGHT,
> - .cfa_baycfg = ISC_BAY_CFG_BGBG,
> - .rlp_cfg_mode = ISC_RLP_CFG_MODE_YYCC,
> - .dcfg_imode = ISC_DCFG_IMODE_YC422P,
> - .dctrl_dview = ISC_DCTRL_DVIEW_PLANAR,
> - .bits_pipeline = SUB422_ENABLE |
> - CBC_ENABLE | CSC_ENABLE |
> - GAM_ENABLES |
> - CFA_ENABLE | WB_ENABLE,
> },
> {
> .fourcc = V4L2_PIX_FMT_GREY,
> + .mbus_code = MEDIA_BUS_FMT_Y8_1X8,
> .pfe_cfg0_bps = ISC_PFE_CFG0_BPS_EIGHT,
> - .cfa_baycfg = ISC_BAY_CFG_BGBG,
> - .rlp_cfg_mode = ISC_RLP_CFG_MODE_DATY8,
> - .dcfg_imode = ISC_DCFG_IMODE_PACKED8,
> - .dctrl_dview = ISC_DCTRL_DVIEW_PACKED,
> - .bits_pipeline = CBC_ENABLE | CSC_ENABLE |
> - GAM_ENABLES |
> - CFA_ENABLE | WB_ENABLE,
> - },
> - {
> - .fourcc = V4L2_PIX_FMT_ARGB444,
> - .pfe_cfg0_bps = ISC_PFE_CFG0_BPS_EIGHT,
> - .cfa_baycfg = ISC_BAY_CFG_BGBG,
> - .rlp_cfg_mode = ISC_RLP_CFG_MODE_ARGB444,
> - .dcfg_imode = ISC_DCFG_IMODE_PACKED16,
> - .dctrl_dview = ISC_DCTRL_DVIEW_PACKED,
> - .bits_pipeline = GAM_ENABLES | CFA_ENABLE |
> WB_ENABLE,
> },
> {
> - .fourcc = V4L2_PIX_FMT_ARGB555,
> + .fourcc = V4L2_PIX_FMT_YUYV,
> + .mbus_code = MEDIA_BUS_FMT_YUYV8_2X8,
> .pfe_cfg0_bps = ISC_PFE_CFG0_BPS_EIGHT,
> - .cfa_baycfg = ISC_BAY_CFG_BGBG,
> - .rlp_cfg_mode = ISC_RLP_CFG_MODE_ARGB555,
> - .dcfg_imode = ISC_DCFG_IMODE_PACKED16,
> - .dctrl_dview = ISC_DCTRL_DVIEW_PACKED,
> - .bits_pipeline = GAM_ENABLES | CFA_ENABLE |
> WB_ENABLE,
> },
> {
> .fourcc = V4L2_PIX_FMT_RGB565,
> + .mbus_code = MEDIA_BUS_FMT_RGB565_2X8_LE,
> .pfe_cfg0_bps = ISC_PFE_CFG0_BPS_EIGHT,
> - .cfa_baycfg = ISC_BAY_CFG_BGBG,
> - .rlp_cfg_mode = ISC_RLP_CFG_MODE_RGB565,
> - .dcfg_imode = ISC_DCFG_IMODE_PACKED16,
> - .dctrl_dview = ISC_DCTRL_DVIEW_PACKED,
> - .bits_pipeline = GAM_ENABLES | CFA_ENABLE |
> WB_ENABLE,
> - },
> - {
> - .fourcc = V4L2_PIX_FMT_ARGB32,
> - .pfe_cfg0_bps = ISC_PFE_CFG0_BPS_EIGHT,
> - .cfa_baycfg = ISC_BAY_CFG_BGBG,
> - .rlp_cfg_mode = ISC_RLP_CFG_MODE_ARGB32,
> - .dcfg_imode = ISC_DCFG_IMODE_PACKED32,
> - .dctrl_dview = ISC_DCTRL_DVIEW_PACKED,
> - .bits_pipeline = GAM_ENABLES | CFA_ENABLE |
> WB_ENABLE,
> - },
> - {
> - .fourcc = V4L2_PIX_FMT_YUYV,
> - .pfe_cfg0_bps = ISC_PFE_CFG0_BPS_EIGHT,
> - .cfa_baycfg = ISC_BAY_CFG_BGBG,
> - .rlp_cfg_mode = ISC_RLP_CFG_MODE_DAT8,
> - .dcfg_imode = ISC_DCFG_IMODE_PACKED8,
> - .dctrl_dview = ISC_DCTRL_DVIEW_PACKED,
> - .bits_pipeline = 0x0
> },
> };
>
> @@ -571,6 +380,13 @@ static const u32 isc_gamma_table[GAMMA_MAX +
> 1][GAMMA_ENTRIES] = {
> 0x3E20007, 0x3E90007, 0x3F00008, 0x3F80007 }, };
>
> +#define ISC_IS_FORMAT_RAW(mbus_code) \
> + (((mbus_code) & 0xf000) == 0x3000)
> +
> +static unsigned int debug;
> +module_param(debug, int, 0644);
> +MODULE_PARM_DESC(debug, "debug level (0-2)");
> +
> static unsigned int sensor_preferred = 1; module_param(sensor_preferred,
> uint, 0644); MODULE_PARM_DESC(sensor_preferred,
> @@ -896,40 +712,17 @@ static int isc_buffer_prepare(struct vb2_buffer *vb)
> return 0;
> }
>
> -static inline bool sensor_is_preferred(const struct isc_format *isc_fmt) -{
> - return (sensor_preferred && isc_fmt->sd_support) ||
> - !isc_fmt->isc_support;
> -}
> -
> -static struct fmt_config *get_fmt_config(u32 fourcc) -{
> - struct fmt_config *config;
> - int i;
> -
> - config = &fmt_configs_list[0];
> - for (i = 0; i < ARRAY_SIZE(fmt_configs_list); i++) {
> - if (config->fourcc == fourcc)
> - return config;
> -
> - config++;
> - }
> - return NULL;
> -}
> -
> static void isc_start_dma(struct isc_device *isc) {
> struct regmap *regmap = isc->regmap;
> - struct v4l2_pix_format *pixfmt = &isc->fmt.fmt.pix;
> - u32 sizeimage = pixfmt->sizeimage;
> - struct fmt_config *config = get_fmt_config(isc->current_fmt->fourcc);
> + u32 sizeimage = isc->fmt.fmt.pix.sizeimage;
> u32 dctrl_dview;
> dma_addr_t addr0;
>
> addr0 = vb2_dma_contig_plane_dma_addr(&isc->cur_frm-
> >vb.vb2_buf, 0);
> regmap_write(regmap, ISC_DAD0, addr0);
>
> - switch (pixfmt->pixelformat) {
> + switch (isc->config.fourcc) {
> case V4L2_PIX_FMT_YUV420:
> regmap_write(regmap, ISC_DAD1, addr0 + (sizeimage * 2) /
> 3);
> regmap_write(regmap, ISC_DAD2, addr0 + (sizeimage * 5) /
> 6); @@ -942,10 +735,7 @@ static void isc_start_dma(struct isc_device *isc)
> break;
> }
>
> - if (sensor_is_preferred(isc->current_fmt))
> - dctrl_dview = ISC_DCTRL_DVIEW_PACKED;
> - else
> - dctrl_dview = config->dctrl_dview;
> + dctrl_dview = isc->config.dctrl_dview;
>
> regmap_write(regmap, ISC_DCTRL, dctrl_dview | ISC_DCTRL_IE_IS);
> regmap_write(regmap, ISC_CTRLEN, ISC_CTRL_CAPTURE); @@ -955,7
> +745,6 @@ static void isc_set_pipeline(struct isc_device *isc, u32 pipeline) {
> struct regmap *regmap = isc->regmap;
> struct isc_ctrls *ctrls = &isc->ctrls;
> - struct fmt_config *config = get_fmt_config(isc->raw_fmt->fourcc);
> u32 val, bay_cfg;
> const u32 *gamma;
> unsigned int i;
> @@ -969,7 +758,7 @@ static void isc_set_pipeline(struct isc_device *isc, u32
> pipeline)
> if (!pipeline)
> return;
>
> - bay_cfg = config->cfa_baycfg;
> + bay_cfg = isc->config.sd_format->cfa_baycfg;
>
> regmap_write(regmap, ISC_WB_CFG, bay_cfg);
> regmap_write(regmap, ISC_WB_O_RGR, 0x0); @@ -1011,24 +800,24
> @@ static int isc_update_profile(struct isc_device *isc)
> }
>
> if (counter < 0) {
> - v4l2_warn(&isc->v4l2_dev, "Time out to update profie\n");
> + v4l2_warn(&isc->v4l2_dev, "Time out to update profile\n");
> return -ETIMEDOUT;
> }
>
> return 0;
> }
>
> -static void isc_set_histogram(struct isc_device *isc)
> +static void isc_set_histogram(struct isc_device *isc, bool enable)
> {
> struct regmap *regmap = isc->regmap;
> struct isc_ctrls *ctrls = &isc->ctrls;
> - struct fmt_config *config = get_fmt_config(isc->raw_fmt->fourcc);
>
> - if (ctrls->awb && (ctrls->hist_stat != HIST_ENABLED)) {
> + if (enable) {
> regmap_write(regmap, ISC_HIS_CFG,
> ISC_HIS_CFG_MODE_R |
> - (config->cfa_baycfg <<
> ISC_HIS_CFG_BAYSEL_SHIFT) |
> - ISC_HIS_CFG_RAR);
> + (isc->config.sd_format->cfa_baycfg
> + << ISC_HIS_CFG_BAYSEL_SHIFT) |
> + ISC_HIS_CFG_RAR);
> regmap_write(regmap, ISC_HIS_CTRL, ISC_HIS_CTRL_EN);
> regmap_write(regmap, ISC_INTEN, ISC_INT_HISDONE);
> ctrls->hist_id = ISC_HIS_CFG_MODE_R;
> @@ -1036,7 +825,7 @@ static void isc_set_histogram(struct isc_device *isc)
> regmap_write(regmap, ISC_CTRLEN, ISC_CTRL_HISREQ);
>
> ctrls->hist_stat = HIST_ENABLED;
> - } else if (!ctrls->awb && (ctrls->hist_stat != HIST_DISABLED)) {
> + } else {
> regmap_write(regmap, ISC_INTDIS, ISC_INT_HISDONE);
> regmap_write(regmap, ISC_HIS_CTRL, ISC_HIS_CTRL_DIS);
>
> @@ -1044,53 +833,18 @@ static void isc_set_histogram(struct isc_device *isc)
> }
> }
>
> -static inline void isc_get_param(const struct isc_format *fmt,
> - u32 *rlp_mode, u32 *dcfg)
> -{
> - struct fmt_config *config = get_fmt_config(fmt->fourcc);
> -
> - *dcfg = ISC_DCFG_YMBSIZE_BEATS8;
> -
> - switch (fmt->fourcc) {
> - case V4L2_PIX_FMT_SBGGR10:
> - case V4L2_PIX_FMT_SGBRG10:
> - case V4L2_PIX_FMT_SGRBG10:
> - case V4L2_PIX_FMT_SRGGB10:
> - case V4L2_PIX_FMT_SBGGR12:
> - case V4L2_PIX_FMT_SGBRG12:
> - case V4L2_PIX_FMT_SGRBG12:
> - case V4L2_PIX_FMT_SRGGB12:
> - *rlp_mode = config->rlp_cfg_mode;
> - *dcfg |= config->dcfg_imode;
> - break;
> - default:
> - *rlp_mode = ISC_RLP_CFG_MODE_DAT8;
> - *dcfg |= ISC_DCFG_IMODE_PACKED8;
> - break;
> - }
> -}
> -
> static int isc_configure(struct isc_device *isc) {
> struct regmap *regmap = isc->regmap;
> - const struct isc_format *current_fmt = isc->current_fmt;
> - struct fmt_config *curfmt_config = get_fmt_config(current_fmt-
> >fourcc);
> - struct fmt_config *rawfmt_config = get_fmt_config(isc->raw_fmt-
> >fourcc);
> - struct isc_subdev_entity *subdev = isc->current_subdev;
> u32 pfe_cfg0, rlp_mode, dcfg, mask, pipeline;
> + struct isc_subdev_entity *subdev = isc->current_subdev;
>
> - if (sensor_is_preferred(current_fmt)) {
> - pfe_cfg0 = curfmt_config->pfe_cfg0_bps;
> - pipeline = 0x0;
> - isc_get_param(current_fmt, &rlp_mode, &dcfg);
> - isc->ctrls.hist_stat = HIST_INIT;
> - } else {
> - pfe_cfg0 = rawfmt_config->pfe_cfg0_bps;
> - pipeline = curfmt_config->bits_pipeline;
> - rlp_mode = curfmt_config->rlp_cfg_mode;
> - dcfg = curfmt_config->dcfg_imode |
> + pfe_cfg0 = isc->config.sd_format->pfe_cfg0_bps;
> + rlp_mode = isc->config.rlp_cfg_mode;
> + pipeline = isc->config.bits_pipeline;
> +
> + dcfg = isc->config.dcfg_imode |
> ISC_DCFG_YMBSIZE_BEATS8 |
> ISC_DCFG_CMBSIZE_BEATS8;
> - }
>
> pfe_cfg0 |= subdev->pfe_cfg0 | ISC_PFE_CFG0_MODE_PROGRESSIVE;
> mask = ISC_PFE_CFG0_BPS_MASK | ISC_PFE_CFG0_HPOL_LOW | @@ -
> 1107,8 +861,15 @@ static int isc_configure(struct isc_device *isc)
> /* Set the pipeline */
> isc_set_pipeline(isc, pipeline);
>
> - if (pipeline)
> - isc_set_histogram(isc);
> + /*
> + * The current implemented histogram is available for RAW R, B, GB
> + * channels. We need to check if sensor is outputting RAW BAYER
> + */
> + if (isc->ctrls.awb &&
> + ISC_IS_FORMAT_RAW(isc->config.sd_format->mbus_code))
> + isc_set_histogram(isc, true);
> + else
> + isc_set_histogram(isc, false);
>
> /* Update profile */
> return isc_update_profile(isc);
> @@ -1125,7 +886,8 @@ static int isc_start_streaming(struct vb2_queue *vq,
> unsigned int count)
> /* Enable stream on the sub device */
> ret = v4l2_subdev_call(isc->current_subdev->sd, video, s_stream, 1);
> if (ret && ret != -ENOIOCTLCMD) {
> - v4l2_err(&isc->v4l2_dev, "stream on failed in subdev\n");
> + v4l2_err(&isc->v4l2_dev, "stream on failed in subdev %d\n",
> + ret);
> goto err_start_stream;
> }
>
> @@ -1223,6 +985,22 @@ static void isc_buffer_queue(struct vb2_buffer *vb)
> spin_unlock_irqrestore(&isc->dma_queue_lock, flags); }
>
> +static struct isc_format *find_format_by_fourcc(struct isc_device *isc,
> + unsigned int fourcc)
> +{
> + unsigned int num_formats = isc->num_user_formats;
> + struct isc_format *fmt;
> + unsigned int i;
> +
> + for (i = 0; i < num_formats; i++) {
> + fmt = isc->user_formats[i];
> + if (fmt->fourcc == fourcc)
> + return fmt;
> + }
> +
> + return NULL;
> +}
> +
> static const struct vb2_ops isc_vb2_ops = {
> .queue_setup = isc_queue_setup,
> .wait_prepare = vb2_ops_wait_prepare,
> @@ -1249,15 +1027,31 @@ static int isc_querycap(struct file *file, void *priv,
> static int isc_enum_fmt_vid_cap(struct file *file, void *priv,
> struct v4l2_fmtdesc *f)
> {
> - struct isc_device *isc = video_drvdata(file);
> u32 index = f->index;
> + u32 i, supported_index;
>
> - if (index >= isc->num_user_formats)
> - return -EINVAL;
> + if (index < ARRAY_SIZE(controller_formats)) {
> + f->pixelformat = controller_formats[index].fourcc;
> + return 0;
> + }
>
> - f->pixelformat = isc->user_formats[index]->fourcc;
> + index -= ARRAY_SIZE(controller_formats);
>
> - return 0;
> + i = 0;
> + supported_index = 0;
> +
> + for (i = 0; i < ARRAY_SIZE(formats_list); i++) {
> + if (!ISC_IS_FORMAT_RAW(formats_list[i].mbus_code) ||
> + !formats_list[i].sd_support)
> + continue;
> + if (supported_index == index) {
> + f->pixelformat = formats_list[i].fourcc;
> + return 0;
> + }
> + supported_index++;
> + }
> +
> + return -EINVAL;
> }
>
> static int isc_g_fmt_vid_cap(struct file *file, void *priv, @@ -1270,26
> +1064,231 @@ static int isc_g_fmt_vid_cap(struct file *file, void *priv,
> return 0;
> }
>
> -static struct isc_format *find_format_by_fourcc(struct isc_device *isc,
> - unsigned int fourcc)
> +/*
> + * Checks the current configured format, if ISC can output it,
> + * considering which type of format the ISC receives from the sensor
> +*/ static int isc_try_validate_formats(struct isc_device *isc)
> {
> - unsigned int num_formats = isc->num_user_formats;
> - struct isc_format *fmt;
> - unsigned int i;
> + int ret;
> + bool bayer = false, yuv = false, rgb = false, grey = false;
> +
> + /* all formats supported by the RLP module are OK */
> + switch (isc->try_config.fourcc) {
> + case V4L2_PIX_FMT_SBGGR8:
> + case V4L2_PIX_FMT_SGBRG8:
> + case V4L2_PIX_FMT_SGRBG8:
> + case V4L2_PIX_FMT_SRGGB8:
> + case V4L2_PIX_FMT_SBGGR10:
> + case V4L2_PIX_FMT_SGBRG10:
> + case V4L2_PIX_FMT_SGRBG10:
> + case V4L2_PIX_FMT_SRGGB10:
> + case V4L2_PIX_FMT_SBGGR12:
> + case V4L2_PIX_FMT_SGBRG12:
> + case V4L2_PIX_FMT_SGRBG12:
> + case V4L2_PIX_FMT_SRGGB12:
> + ret = 0;
> + bayer = true;
> + break;
>
> - for (i = 0; i < num_formats; i++) {
> - fmt = isc->user_formats[i];
> - if (fmt->fourcc == fourcc)
> - return fmt;
> + case V4L2_PIX_FMT_YUV420:
> + case V4L2_PIX_FMT_YUV422P:
> + case V4L2_PIX_FMT_YUYV:
> + ret = 0;
> + yuv = true;
> + break;
> +
> + case V4L2_PIX_FMT_RGB565:
> + case V4L2_PIX_FMT_ARGB32:
> + case V4L2_PIX_FMT_ARGB444:
> + case V4L2_PIX_FMT_ARGB555:
> + ret = 0;
> + rgb = true;
> + break;
> + case V4L2_PIX_FMT_GREY:
> + ret = 0;
> + grey = true;
> + break;
> + default:
> + /* any other different formats are not supported */
> + ret = -EINVAL;
> }
>
> - return NULL;
> + /* we cannot output RAW/Grey if we do not receive RAW */
> + if ((bayer || grey) &&
> + !ISC_IS_FORMAT_RAW(isc->try_config.sd_format->mbus_code))
> + return -EINVAL;
> +
> + return ret;
> +}
> +
> +/*
> + * Configures the RLP and DMA modules, depending on the output format
> + * configured for the ISC.
> + * If direct_dump == true, just dump raw data 8 bits.
> + */
> +static int isc_try_configure_rlp_dma(struct isc_device *isc, bool
> +direct_dump) {
> + if (direct_dump) {
> + isc->try_config.rlp_cfg_mode = ISC_RLP_CFG_MODE_DAT8;
> + isc->try_config.dcfg_imode = ISC_DCFG_IMODE_PACKED8;
> + isc->try_config.dctrl_dview = ISC_DCTRL_DVIEW_PACKED;
> + isc->try_config.bpp = 16;
> + return 0;
> + }
> +
> + switch (isc->try_config.fourcc) {
> + case V4L2_PIX_FMT_SBGGR8:
> + case V4L2_PIX_FMT_SGBRG8:
> + case V4L2_PIX_FMT_SGRBG8:
> + case V4L2_PIX_FMT_SRGGB8:
> + isc->try_config.rlp_cfg_mode = ISC_RLP_CFG_MODE_DAT8;
> + isc->try_config.dcfg_imode = ISC_DCFG_IMODE_PACKED8;
> + isc->try_config.dctrl_dview = ISC_DCTRL_DVIEW_PACKED;
> + isc->try_config.bpp = 8;
> + break;
> + case V4L2_PIX_FMT_SBGGR10:
> + case V4L2_PIX_FMT_SGBRG10:
> + case V4L2_PIX_FMT_SGRBG10:
> + case V4L2_PIX_FMT_SRGGB10:
> + isc->try_config.rlp_cfg_mode = ISC_RLP_CFG_MODE_DAT10;
> + isc->try_config.dcfg_imode = ISC_DCFG_IMODE_PACKED16;
> + isc->try_config.dctrl_dview = ISC_DCTRL_DVIEW_PACKED;
> + isc->try_config.bpp = 16;
> + break;
> + case V4L2_PIX_FMT_SBGGR12:
> + case V4L2_PIX_FMT_SGBRG12:
> + case V4L2_PIX_FMT_SGRBG12:
> + case V4L2_PIX_FMT_SRGGB12:
> + isc->try_config.rlp_cfg_mode = ISC_RLP_CFG_MODE_DAT12;
> + isc->try_config.dcfg_imode = ISC_DCFG_IMODE_PACKED16;
> + isc->try_config.dctrl_dview = ISC_DCTRL_DVIEW_PACKED;
> + isc->try_config.bpp = 16;
> + break;
> + case V4L2_PIX_FMT_RGB565:
> + isc->try_config.rlp_cfg_mode = ISC_RLP_CFG_MODE_RGB565;
> + isc->try_config.dcfg_imode = ISC_DCFG_IMODE_PACKED16;
> + isc->try_config.dctrl_dview = ISC_DCTRL_DVIEW_PACKED;
> + isc->try_config.bpp = 16;
> + break;
> + case V4L2_PIX_FMT_ARGB444:
> + isc->try_config.rlp_cfg_mode =
> ISC_RLP_CFG_MODE_ARGB444;
> + isc->try_config.dcfg_imode = ISC_DCFG_IMODE_PACKED16;
> + isc->try_config.dctrl_dview = ISC_DCTRL_DVIEW_PACKED;
> + isc->try_config.bpp = 16;
> + break;
> + case V4L2_PIX_FMT_ARGB555:
> + isc->try_config.rlp_cfg_mode =
> ISC_RLP_CFG_MODE_ARGB555;
> + isc->try_config.dcfg_imode = ISC_DCFG_IMODE_PACKED16;
> + isc->try_config.dctrl_dview = ISC_DCTRL_DVIEW_PACKED;
> + isc->try_config.bpp = 16;
> + break;
> + case V4L2_PIX_FMT_ARGB32:
> + isc->try_config.rlp_cfg_mode = ISC_RLP_CFG_MODE_ARGB32;
> + isc->try_config.dcfg_imode = ISC_DCFG_IMODE_PACKED32;
> + isc->try_config.dctrl_dview = ISC_DCTRL_DVIEW_PACKED;
> + isc->try_config.bpp = 32;
> + break;
> + case V4L2_PIX_FMT_YUV420:
> + isc->try_config.rlp_cfg_mode = ISC_RLP_CFG_MODE_YYCC;
> + isc->try_config.dcfg_imode = ISC_DCFG_IMODE_YC420P;
> + isc->try_config.dctrl_dview = ISC_DCTRL_DVIEW_PLANAR;
> + isc->try_config.bpp = 12;
> + break;
> + case V4L2_PIX_FMT_YUV422P:
> + isc->try_config.rlp_cfg_mode = ISC_RLP_CFG_MODE_YYCC;
> + isc->try_config.dcfg_imode = ISC_DCFG_IMODE_YC422P;
> + isc->try_config.dctrl_dview = ISC_DCTRL_DVIEW_PLANAR;
> + isc->try_config.bpp = 16;
> + break;
> + case V4L2_PIX_FMT_YUYV:
> + isc->try_config.rlp_cfg_mode = ISC_RLP_CFG_MODE_YYCC;
> + isc->try_config.dcfg_imode = ISC_DCFG_IMODE_PACKED32;
> + isc->try_config.dctrl_dview = ISC_DCTRL_DVIEW_PACKED;
> + isc->try_config.bpp = 16;
> + break;
> + case V4L2_PIX_FMT_GREY:
> + isc->try_config.rlp_cfg_mode = ISC_RLP_CFG_MODE_DATY8;
> + isc->try_config.dcfg_imode = ISC_DCFG_IMODE_PACKED8;
> + isc->try_config.dctrl_dview = ISC_DCTRL_DVIEW_PACKED;
> + isc->try_config.bpp = 8;
> + break;
> + default:
> + return -EINVAL;
> + }
> + return 0;
> +}
> +
> +/*
> + * Configuring pipeline modules, depending on which format the ISC
> +outputs
> + * and considering which format it has as input from the sensor.
> + */
> +static int isc_try_configure_pipeline(struct isc_device *isc) {
> + switch (isc->try_config.fourcc) {
> + case V4L2_PIX_FMT_RGB565:
> + case V4L2_PIX_FMT_ARGB555:
> + case V4L2_PIX_FMT_ARGB444:
> + case V4L2_PIX_FMT_ARGB32:
> + /* if sensor format is RAW, we convert inside ISC */
> + if (ISC_IS_FORMAT_RAW(isc->try_config.sd_format-
> >mbus_code)) {
> + isc->try_config.bits_pipeline = CFA_ENABLE |
> + WB_ENABLE | GAM_ENABLES;
> + } else {
> + isc->try_config.bits_pipeline = 0x0;
> + }
> + break;
> + case V4L2_PIX_FMT_YUV420:
> + /* if sensor format is RAW, we convert inside ISC */
> + if (ISC_IS_FORMAT_RAW(isc->try_config.sd_format-
> >mbus_code)) {
> + isc->try_config.bits_pipeline = CFA_ENABLE |
> + CSC_ENABLE | WB_ENABLE | GAM_ENABLES |
> + SUB420_ENABLE | SUB422_ENABLE |
> CBC_ENABLE;
> + } else {
> + isc->try_config.bits_pipeline = 0x0;
> + }
> + break;
> + case V4L2_PIX_FMT_YUV422P:
> + /* if sensor format is RAW, we convert inside ISC */
> + if (ISC_IS_FORMAT_RAW(isc->try_config.sd_format-
> >mbus_code)) {
> + isc->try_config.bits_pipeline = CFA_ENABLE |
> + CSC_ENABLE | WB_ENABLE | GAM_ENABLES |
> + SUB422_ENABLE | CBC_ENABLE;
> + } else {
> + isc->try_config.bits_pipeline = 0x0;
> + }
> + break;
> + case V4L2_PIX_FMT_YUYV:
> + /* if sensor format is RAW, we convert inside ISC */
> + if (ISC_IS_FORMAT_RAW(isc->try_config.sd_format-
> >mbus_code)) {
> + isc->try_config.bits_pipeline = CFA_ENABLE |
> + CSC_ENABLE | WB_ENABLE | GAM_ENABLES |
> + SUB422_ENABLE | CBC_ENABLE;
> + } else {
> + isc->try_config.bits_pipeline = 0x0;
> + }
> + break;
> + case V4L2_PIX_FMT_GREY:
> + if (ISC_IS_FORMAT_RAW(isc->try_config.sd_format-
> >mbus_code)) {
> + /* if sensor format is RAW, we convert inside ISC */
> + isc->try_config.bits_pipeline = CFA_ENABLE |
> + CSC_ENABLE | WB_ENABLE | GAM_ENABLES |
> + CBC_ENABLE;
> + } else {
> + isc->try_config.bits_pipeline = 0x0;
> + }
> + break;
> + default:
> + isc->try_config.bits_pipeline = 0x0;
> + }
> + return 0;
> }
>
> static int isc_try_fmt(struct isc_device *isc, struct v4l2_format *f,
> - struct isc_format **current_fmt, u32 *code)
> + u32 *code)
> {
> - struct isc_format *isc_fmt;
> + int i;
> + struct isc_format *sd_fmt = NULL, *direct_fmt = NULL;
> struct v4l2_pix_format *pixfmt = &f->fmt.pix;
> struct v4l2_subdev_pad_config pad_cfg;
> struct v4l2_subdev_format format = {
> @@ -1297,48 +1296,114 @@ static int isc_try_fmt(struct isc_device *isc, struct
> v4l2_format *f,
> };
> u32 mbus_code;
> int ret;
> + bool rlp_dma_direct_dump = false;
>
> if (f->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> return -EINVAL;
>
> - isc_fmt = find_format_by_fourcc(isc, pixfmt->pixelformat);
> - if (!isc_fmt) {
> - v4l2_warn(&isc->v4l2_dev, "Format 0x%x not found\n",
> - pixfmt->pixelformat);
> - isc_fmt = isc->user_formats[isc->num_user_formats - 1];
> - pixfmt->pixelformat = isc_fmt->fourcc;
> + /* Step 1: find a RAW format that is supported */
> + for (i = 0; i < isc->num_user_formats; i++) {
> + if (ISC_IS_FORMAT_RAW(isc->user_formats[i]->mbus_code)) {
> + sd_fmt = isc->user_formats[i];
> + break;
> + }
> + }
> + /* Step 2: We can continue with this RAW format, or we can look
> + * for better: maybe sensor supports directly what we need.
> + */
> + direct_fmt = find_format_by_fourcc(isc, pixfmt->pixelformat);
> +
> + /* Step 3: We have both. We decide given the module parameter which
> + * one to use.
> + */
> + if (direct_fmt && sd_fmt && sensor_preferred)
> + sd_fmt = direct_fmt;
> +
> + /* Step 4: we do not have RAW but we have a direct format. Use it. */
> + if (direct_fmt && !sd_fmt)
> + sd_fmt = direct_fmt;
> +
> + /* Step 5: if we are using a direct format, we need to package
> + * everything as 8 bit data and just dump it
> + */
> + if (sd_fmt == direct_fmt)
> + rlp_dma_direct_dump = true;
> +
> + /* Step 6: We have no format. This can happen if the userspace
> + * requests some weird/invalid format.
> + * In this case, default to whatever we have
> + */
> + if (!sd_fmt && !direct_fmt) {
> + sd_fmt = isc->user_formats[isc->num_user_formats - 1];
> + v4l2_dbg(1, debug, &isc->v4l2_dev,
> + "Sensor not supporting %.4s, using %.4s\n",
> + (char *)&pixfmt->pixelformat, (char *)&sd_fmt-
> >fourcc);
> }
>
> + /* Step 7: Print out what we decided for debugging */
> + v4l2_dbg(1, debug, &isc->v4l2_dev,
> + "Preferring to have sensor using format %.4s\n",
> + (char *)&sd_fmt->fourcc);
> +
> + /* Step 8: at this moment we decided which format the subdev will use
> */
> + isc->try_config.sd_format = sd_fmt;
> +
> /* Limit to Atmel ISC hardware capabilities */
> if (pixfmt->width > ISC_MAX_SUPPORT_WIDTH)
> pixfmt->width = ISC_MAX_SUPPORT_WIDTH;
> if (pixfmt->height > ISC_MAX_SUPPORT_HEIGHT)
> pixfmt->height = ISC_MAX_SUPPORT_HEIGHT;
>
> - if (sensor_is_preferred(isc_fmt))
> - mbus_code = isc_fmt->mbus_code;
> - else
> - mbus_code = isc->raw_fmt->mbus_code;
> + /*
> + * The mbus format is the one the subdev outputs.
> + * The pixels will be transferred in this format Sensor -> ISC
> + */
> + mbus_code = sd_fmt->mbus_code;
> +
> + /*
> + * Validate formats. If the required format is not OK, default to raw.
> + */
> +
> + isc->try_config.fourcc = pixfmt->pixelformat;
> +
> + if (isc_try_validate_formats(isc)) {
> + pixfmt->pixelformat = isc->try_config.fourcc = sd_fmt->fourcc;
> + /* This should be redundant, format should be supported */
> + ret = isc_try_validate_formats(isc);
> + if (ret)
> + goto isc_try_fmt_err;
> + }
> +
> + ret = isc_try_configure_rlp_dma(isc, rlp_dma_direct_dump);
> + if (ret)
> + goto isc_try_fmt_err;
> +
> + ret = isc_try_configure_pipeline(isc);
> + if (ret)
> + goto isc_try_fmt_err;
>
> v4l2_fill_mbus_format(&format.format, pixfmt, mbus_code);
> ret = v4l2_subdev_call(isc->current_subdev->sd, pad, set_fmt,
> &pad_cfg, &format);
> if (ret < 0)
> - return ret;
> + goto isc_try_fmt_err;
>
> v4l2_fill_pix_format(pixfmt, &format.format);
>
> pixfmt->field = V4L2_FIELD_NONE;
> - pixfmt->bytesperline = (pixfmt->width * isc_fmt->bpp) >> 3;
> + pixfmt->bytesperline = (pixfmt->width * isc->try_config.bpp) >> 3;
> pixfmt->sizeimage = pixfmt->bytesperline * pixfmt->height;
>
> - if (current_fmt)
> - *current_fmt = isc_fmt;
> -
> if (code)
> *code = mbus_code;
>
> return 0;
> +
> +isc_try_fmt_err:
> + v4l2_err(&isc->v4l2_dev, "Could not find any possible format for a
> working pipeline\n");
> + memset(&isc->try_config, 0, sizeof(isc->try_config));
> +
> + return ret;
> }
>
> static int isc_set_fmt(struct isc_device *isc, struct v4l2_format *f) @@ -
> 1346,11 +1411,10 @@ static int isc_set_fmt(struct isc_device *isc, struct
> v4l2_format *f)
> struct v4l2_subdev_format format = {
> .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> };
> - struct isc_format *current_fmt;
> - u32 mbus_code;
> + u32 mbus_code = 0;
> int ret;
>
> - ret = isc_try_fmt(isc, f, &current_fmt, &mbus_code);
> + ret = isc_try_fmt(isc, f, &mbus_code);
> if (ret)
> return ret;
>
> @@ -1361,7 +1425,10 @@ static int isc_set_fmt(struct isc_device *isc, struct
> v4l2_format *f)
> return ret;
>
> isc->fmt = *f;
> - isc->current_fmt = current_fmt;
> + /* make the try configuration active */
> + memcpy(&isc->config, &isc->try_config, sizeof(isc->config));
> +
> + v4l2_dbg(1, debug, &isc->v4l2_dev, "New ISC configuration in
> +place\n");
>
> return 0;
> }
> @@ -1382,7 +1449,7 @@ static int isc_try_fmt_vid_cap(struct file *file, void
> *priv, {
> struct isc_device *isc = video_drvdata(file);
>
> - return isc_try_fmt(isc, f, NULL, NULL);
> + return isc_try_fmt(isc, f, NULL);
> }
>
> static int isc_enum_input(struct file *file, void *priv, @@ -1431,27 +1498,31
> @@ static int isc_enum_framesizes(struct file *file, void *fh,
> struct v4l2_frmsizeenum *fsize) {
> struct isc_device *isc = video_drvdata(file);
> - const struct isc_format *isc_fmt;
> struct v4l2_subdev_frame_size_enum fse = {
> .index = fsize->index,
> .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> };
> - int ret;
> + int ret = -EINVAL;
> + int i;
>
> - isc_fmt = find_format_by_fourcc(isc, fsize->pixel_format);
> - if (!isc_fmt)
> - return -EINVAL;
> + for (i = 0; i < isc->num_user_formats; i++)
> + if (isc->user_formats[i]->fourcc == fsize->pixel_format)
> + ret = 0;
>
> - if (sensor_is_preferred(isc_fmt))
> - fse.code = isc_fmt->mbus_code;
> - else
> - fse.code = isc->raw_fmt->mbus_code;
> + for (i = 0; i < ARRAY_SIZE(controller_formats); i++)
> + if (controller_formats[i].fourcc == fsize->pixel_format)
> + ret = 0;
> +
> + if (ret)
> + return ret;
>
> ret = v4l2_subdev_call(isc->current_subdev->sd, pad,
> enum_frame_size,
> NULL, &fse);
> if (ret)
> return ret;
>
> + fse.code = isc->config.sd_format->mbus_code;
> +
> fsize->type = V4L2_FRMSIZE_TYPE_DISCRETE;
> fsize->discrete.width = fse.max_width;
> fsize->discrete.height = fse.max_height; @@ -1463,29 +1534,32 @@
> static int isc_enum_frameintervals(struct file *file, void *fh,
> struct v4l2_frmivalenum *fival) {
> struct isc_device *isc = video_drvdata(file);
> - const struct isc_format *isc_fmt;
> struct v4l2_subdev_frame_interval_enum fie = {
> .index = fival->index,
> .width = fival->width,
> .height = fival->height,
> .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> };
> - int ret;
> + int ret = -EINVAL;
> + int i;
>
> - isc_fmt = find_format_by_fourcc(isc, fival->pixel_format);
> - if (!isc_fmt)
> - return -EINVAL;
> + for (i = 0; i < isc->num_user_formats; i++)
> + if (isc->user_formats[i]->fourcc == fival->pixel_format)
> + ret = 0;
>
> - if (sensor_is_preferred(isc_fmt))
> - fie.code = isc_fmt->mbus_code;
> - else
> - fie.code = isc->raw_fmt->mbus_code;
> + for (i = 0; i < ARRAY_SIZE(controller_formats); i++)
> + if (controller_formats[i].fourcc == fival->pixel_format)
> + ret = 0;
> +
> + if (ret)
> + return ret;
>
> ret = v4l2_subdev_call(isc->current_subdev->sd, pad,
> enum_frame_interval, NULL, &fie);
> if (ret)
> return ret;
>
> + fie.code = isc->config.sd_format->mbus_code;
> fival->type = V4L2_FRMIVAL_TYPE_DISCRETE;
> fival->discrete = fie.interval;
>
> @@ -1668,7 +1742,6 @@ static void isc_awb_work(struct work_struct *w)
> struct isc_device *isc =
> container_of(w, struct isc_device, awb_work);
> struct regmap *regmap = isc->regmap;
> - struct fmt_config *config = get_fmt_config(isc->raw_fmt->fourcc);
> struct isc_ctrls *ctrls = &isc->ctrls;
> u32 hist_id = ctrls->hist_id;
> u32 baysel;
> @@ -1686,7 +1759,7 @@ static void isc_awb_work(struct work_struct *w)
> }
>
> ctrls->hist_id = hist_id;
> - baysel = config->cfa_baycfg << ISC_HIS_CFG_BAYSEL_SHIFT;
> + baysel = isc->config.sd_format->cfa_baycfg <<
> +ISC_HIS_CFG_BAYSEL_SHIFT;
>
> pm_runtime_get_sync(isc->dev);
>
> @@ -1754,7 +1827,6 @@ static int isc_ctrl_init(struct isc_device *isc)
> return 0;
> }
>
> -
> static int isc_async_bound(struct v4l2_async_notifier *notifier,
> struct v4l2_subdev *subdev,
> struct v4l2_async_subdev *asd)
> @@ -1812,35 +1884,20 @@ static int isc_formats_init(struct isc_device *isc)
> .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> };
>
> + num_fmts = 0;
> while (!v4l2_subdev_call(subdev, pad, enum_mbus_code,
> NULL, &mbus_code)) {
> mbus_code.index++;
>
> fmt = find_format_by_code(mbus_code.code, &i);
> - if ((!fmt) || (!(fmt->flags & FMT_FLAG_FROM_SENSOR)))
> + if (!fmt) {
> + v4l2_warn(&isc->v4l2_dev, "Mbus code %x not
> supported\n",
> + mbus_code.code);
> continue;
> + }
>
> fmt->sd_support = true;
> -
> - if (fmt->flags & FMT_FLAG_RAW_FORMAT)
> - isc->raw_fmt = fmt;
> - }
> -
> - fmt = &formats_list[0];
> - for (i = 0; i < list_size; i++) {
> - if (fmt->flags & FMT_FLAG_FROM_CONTROLLER)
> - fmt->isc_support = true;
> -
> - fmt++;
> - }
> -
> - fmt = &formats_list[0];
> - num_fmts = 0;
> - for (i = 0; i < list_size; i++) {
> - if (fmt->isc_support || fmt->sd_support)
> - num_fmts++;
> -
> - fmt++;
> + num_fmts++;
> }
>
> if (!num_fmts)
> @@ -1855,9 +1912,8 @@ static int isc_formats_init(struct isc_device *isc)
>
> fmt = &formats_list[0];
> for (i = 0, j = 0; i < list_size; i++) {
> - if (fmt->isc_support || fmt->sd_support)
> + if (fmt->sd_support)
> isc->user_formats[j++] = fmt;
> -
> fmt++;
> }
>
> @@ -1877,13 +1933,11 @@ static int isc_set_default_fmt(struct isc_device
> *isc)
> };
> int ret;
>
> - ret = isc_try_fmt(isc, &f, NULL, NULL);
> + ret = isc_try_fmt(isc, &f, NULL);
> if (ret)
> return ret;
>
> - isc->current_fmt = isc->user_formats[0];
> isc->fmt = f;
> -
> return 0;
> }
>
> --
> 2.7.4

Hi Eugen/Hans,

Sorry for the late reply on this, I had some changes I needed to do to my setup to get it working with the latest changes in the media tree. I just tested your v2 iteration here Eugen, and confirmed, yes it does indeed work with my setup and obviously no longer gets a kernel oops as was happening for me in the previous version of the driver.

One thing I would like to comment about my setup, is that while it is a YUV type format, rather than the format in the driver (V4L2_PIX_FMT_YUYV), my setup actually uses V4L2_PIX_FMT_UYVY. In the previous version of the driver, I actually made modifications in my tree to support this format.

For quick testing purposes on your patch since the format specification method has changed, I simply replaced V4L2_PIX_FMT_YUYV statements with V4L2_PIX_FMT_UYVY and MEDIA_BUS_FMT_YUYV8_2X8 with MEDIA_BUS_FMT_UYVY8_2X8.

Perhaps this alternative YUV format could be added to the current list as well? Per my test, all of the other settings you have applied in the ISC for YUV work just fine for my setup, I am simply using a different variant of the YUV format. If you don't want to add support for this now, I can certainly submit a future patch as well. Let me know if there is anything else you would like me to test for you also.

Thanks,
Ken Sloat