On 11.03.2019 16:36, Hans Verkuil wrote: > Hi Eugen, > > On 2/15/19 9:37 AM, Eugen.Hristev@xxxxxxxxxxxxx wrote: >> 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. > > This is quite a big change, so I would like to see the v4l2-compliance > output before I accept this. > > Make sure you compile the very latest v4l2-compliance direct from the > v4l-utils git repo! Hello Hans, Thank you for your review. I will attach the compliance to the v2 patch, after I fix your comment below and resend. You are right, the try format has side effects, and I will change accordingly Eugen > > See also a comment below: > >> >> Signed-off-by: Eugen Hristev <eugen.hristev@xxxxxxxxxxxxx> >> --- >> Hello Ken and possibly others using ISC driver, >> >> I would appreciate if you could test this patch with your sensor, >> because I do not wish to break anything in your setups. >> Feedback is appreciated if any errors appear, so I can fix them. >> I tested with ov5640, ov7670, ov7740(only in 4.19 because on latest it's broken >> for me...) >> Rebased this patch on top of mediatree.git/master >> Thanks! >> >> Eugen >> >> drivers/media/platform/atmel/atmel-isc.c | 882 ++++++++++++++++--------------- >> 1 file changed, 465 insertions(+), 417 deletions(-) >> >> diff --git a/drivers/media/platform/atmel/atmel-isc.c b/drivers/media/platform/atmel/atmel-isc.c >> index 5017896..7f0a6cc 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,7 @@ 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 isc_ctrls ctrls; >> struct work_struct awb_work; >> @@ -210,319 +214,122 @@ struct isc_device { >> struct list_head subdev_entities; >> }; >> >> -static struct isc_format formats_list[] = { >> +/* This is a list of the formats that the ISC can *output* */ >> +static struct isc_format controller_formats[] = { >> { >> - .fourcc = V4L2_PIX_FMT_SBGGR8, >> - .mbus_code = MEDIA_BUS_FMT_SBGGR8_1X8, >> - .flags = FMT_FLAG_RAW_FROM_SENSOR, >> - .bpp = 8, >> - }, >> - { >> - .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 +378,13 @@ static const u32 isc_gamma_table[GAMMA_MAX + 1][GAMMA_ENTRIES] = { >> 0x3E20007, 0x3E90007, 0x3F00008, 0x3F80007 }, >> }; >> >> +#define ISC_IS_SENSOR_RAW_MODE(isc) \ >> + (((isc)->config.sd_format->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 +710,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 +733,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 +743,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 +756,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 +798,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 +823,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 +831,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 +859,16 @@ 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. Hence we can use it if CFA is disabled and CSC is disabled >> + * and the sensor outputs RAW mode. >> + */ >> + if (isc->ctrls.awb && ((pipeline & (CFA_ENABLE | CSC_ENABLE)) == 0) >> + && ISC_IS_SENSOR_RAW_MODE(isc)) >> + isc_set_histogram(isc, true); >> + else >> + isc_set_histogram(isc, false); >> >> /* Update profile */ >> return isc_update_profile(isc); >> @@ -1125,7 +885,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 +984,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 +1026,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 (((formats_list[i].mbus_code & 0xf000) != 0x3000) || >> + !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 +1063,230 @@ 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_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->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_SENSOR_RAW_MODE(isc)) >> + 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_configure_rlp_dma(struct isc_device *isc, bool direct_dump) >> +{ >> + if (direct_dump) { >> + isc->config.rlp_cfg_mode = ISC_RLP_CFG_MODE_DAT8; >> + isc->config.dcfg_imode = ISC_DCFG_IMODE_PACKED8; >> + isc->config.dctrl_dview = ISC_DCTRL_DVIEW_PACKED; >> + isc->config.bpp = 16; >> + return 0; >> + } >> + >> + switch (isc->config.fourcc) { >> + case V4L2_PIX_FMT_SBGGR8: >> + case V4L2_PIX_FMT_SGBRG8: >> + case V4L2_PIX_FMT_SGRBG8: >> + case V4L2_PIX_FMT_SRGGB8: >> + isc->config.rlp_cfg_mode = ISC_RLP_CFG_MODE_DAT8; >> + isc->config.dcfg_imode = ISC_DCFG_IMODE_PACKED8; >> + isc->config.dctrl_dview = ISC_DCTRL_DVIEW_PACKED; >> + isc->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->config.rlp_cfg_mode = ISC_RLP_CFG_MODE_DAT10; >> + isc->config.dcfg_imode = ISC_DCFG_IMODE_PACKED16; >> + isc->config.dctrl_dview = ISC_DCTRL_DVIEW_PACKED; >> + isc->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->config.rlp_cfg_mode = ISC_RLP_CFG_MODE_DAT12; >> + isc->config.dcfg_imode = ISC_DCFG_IMODE_PACKED16; >> + isc->config.dctrl_dview = ISC_DCTRL_DVIEW_PACKED; >> + isc->config.bpp = 16; >> + break; >> + case V4L2_PIX_FMT_RGB565: >> + isc->config.rlp_cfg_mode = ISC_RLP_CFG_MODE_RGB565; >> + isc->config.dcfg_imode = ISC_DCFG_IMODE_PACKED16; >> + isc->config.dctrl_dview = ISC_DCTRL_DVIEW_PACKED; >> + isc->config.bpp = 16; >> + break; >> + case V4L2_PIX_FMT_ARGB444: >> + isc->config.rlp_cfg_mode = ISC_RLP_CFG_MODE_ARGB444; >> + isc->config.dcfg_imode = ISC_DCFG_IMODE_PACKED16; >> + isc->config.dctrl_dview = ISC_DCTRL_DVIEW_PACKED; >> + isc->config.bpp = 16; >> + break; >> + case V4L2_PIX_FMT_ARGB555: >> + isc->config.rlp_cfg_mode = ISC_RLP_CFG_MODE_ARGB555; >> + isc->config.dcfg_imode = ISC_DCFG_IMODE_PACKED16; >> + isc->config.dctrl_dview = ISC_DCTRL_DVIEW_PACKED; >> + isc->config.bpp = 16; >> + break; >> + case V4L2_PIX_FMT_ARGB32: >> + isc->config.rlp_cfg_mode = ISC_RLP_CFG_MODE_ARGB32; >> + isc->config.dcfg_imode = ISC_DCFG_IMODE_PACKED32; >> + isc->config.dctrl_dview = ISC_DCTRL_DVIEW_PACKED; >> + isc->config.bpp = 32; >> + break; >> + case V4L2_PIX_FMT_YUV420: >> + isc->config.rlp_cfg_mode = ISC_RLP_CFG_MODE_YYCC; >> + isc->config.dcfg_imode = ISC_DCFG_IMODE_YC420P; >> + isc->config.dctrl_dview = ISC_DCTRL_DVIEW_PLANAR; >> + isc->config.bpp = 12; >> + break; >> + case V4L2_PIX_FMT_YUV422P: >> + isc->config.rlp_cfg_mode = ISC_RLP_CFG_MODE_YYCC; >> + isc->config.dcfg_imode = ISC_DCFG_IMODE_YC422P; >> + isc->config.dctrl_dview = ISC_DCTRL_DVIEW_PLANAR; >> + isc->config.bpp = 16; >> + break; >> + case V4L2_PIX_FMT_YUYV: >> + isc->config.rlp_cfg_mode = ISC_RLP_CFG_MODE_YYCC; >> + isc->config.dcfg_imode = ISC_DCFG_IMODE_PACKED32; >> + isc->config.dctrl_dview = ISC_DCTRL_DVIEW_PACKED; >> + isc->config.bpp = 16; >> + break; >> + case V4L2_PIX_FMT_GREY: >> + isc->config.rlp_cfg_mode = ISC_RLP_CFG_MODE_DATY8; >> + isc->config.dcfg_imode = ISC_DCFG_IMODE_PACKED8; >> + isc->config.dctrl_dview = ISC_DCTRL_DVIEW_PACKED; >> + isc->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_configure_pipeline(struct isc_device *isc) >> +{ >> + switch (isc->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_SENSOR_RAW_MODE(isc)) { >> + isc->config.bits_pipeline = CFA_ENABLE | >> + WB_ENABLE | GAM_ENABLES; >> + } else { >> + isc->config.bits_pipeline = 0x0; >> + } >> + break; >> + case V4L2_PIX_FMT_YUV420: >> + /* if sensor format is RAW, we convert inside ISC */ >> + if (ISC_IS_SENSOR_RAW_MODE(isc)) { >> + isc->config.bits_pipeline = CFA_ENABLE | >> + CSC_ENABLE | WB_ENABLE | GAM_ENABLES | >> + SUB420_ENABLE | SUB422_ENABLE | CBC_ENABLE; >> + } else { >> + isc->config.bits_pipeline = 0x0; >> + } >> + break; >> + case V4L2_PIX_FMT_YUV422P: >> + /* if sensor format is RAW, we convert inside ISC */ >> + if (ISC_IS_SENSOR_RAW_MODE(isc)) { >> + isc->config.bits_pipeline = CFA_ENABLE | >> + CSC_ENABLE | WB_ENABLE | GAM_ENABLES | >> + SUB422_ENABLE | CBC_ENABLE; >> + } else { >> + isc->config.bits_pipeline = 0x0; >> + } >> + break; >> + case V4L2_PIX_FMT_YUYV: >> + /* if sensor format is RAW, we convert inside ISC */ >> + if (ISC_IS_SENSOR_RAW_MODE(isc)) { >> + isc->config.bits_pipeline = CFA_ENABLE | >> + CSC_ENABLE | WB_ENABLE | GAM_ENABLES | >> + SUB422_ENABLE | CBC_ENABLE; >> + } else { >> + isc->config.bits_pipeline = 0x0; >> + } >> + break; >> + case V4L2_PIX_FMT_GREY: >> + /* if sensor format is RAW, we convert inside ISC */ >> + if (ISC_IS_SENSOR_RAW_MODE(isc)) { >> + isc->config.bits_pipeline = CFA_ENABLE | >> + CSC_ENABLE | WB_ENABLE | GAM_ENABLES | >> + CBC_ENABLE; >> + } else { >> + isc->config.bits_pipeline = 0x0; >> + } >> + break; >> + default: >> + isc->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 +1294,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->user_formats[i]->mbus_code & 0xf000) == 0x3000) { >> + 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->config.sd_format = sd_fmt; > > This is try_fmt(), i.e. it only validates and does not actually make > any changes. But this line suggests that something is actually changed. > > Are you sure this is correct? TRY_FMT must not have any side-effects, and > it can also be called while streaming. > > If nothing else it would need some comments explaining why this is safe. > >> + >> /* 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->config.fourcc = pixfmt->pixelformat; >> + >> + if (isc_validate_formats(isc)) { >> + pixfmt->pixelformat = isc->config.fourcc = sd_fmt->fourcc; >> + /* This should be redundant, format should be supported */ >> + ret = isc_validate_formats(isc); >> + if (ret) >> + goto isc_try_fmt_err; >> + } >> + >> + ret = isc_configure_rlp_dma(isc, rlp_dma_direct_dump); >> + if (ret) >> + goto isc_try_fmt_err; >> + >> + ret = isc_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->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->config, 0, sizeof(isc->config)); >> + >> + return ret; >> } >> >> static int isc_set_fmt(struct isc_device *isc, struct v4l2_format *f) >> @@ -1346,11 +1409,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, ¤t_fmt, &mbus_code); >> + ret = isc_try_fmt(isc, f, &mbus_code); >> if (ret) >> return ret; >> >> @@ -1361,7 +1423,6 @@ static int isc_set_fmt(struct isc_device *isc, struct v4l2_format *f) >> return ret; >> >> isc->fmt = *f; >> - isc->current_fmt = current_fmt; >> >> return 0; >> } >> @@ -1382,7 +1443,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 +1492,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 +1528,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 +1736,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 +1753,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 +1821,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 +1878,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 +1906,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 +1927,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; >> } >> >> > > Regards, > > Hans >