[6/7] media: rkisp1: Implement extensible params support
Commit Message
Implement support in rkisp1-params for the extensible configuration
parameters format.
Create a list of handlers for each ISP block that wraps the existing
configuration functions and handles the ISP block enablement.
Parse the configuration parameters buffer in rkisp1_ext_params_config
and filter the enable blocks by group, to allow setting the 'other'
groups separately from the 'lsc' group to support the pre/post-configure
operations.
Implement the vb2 buf_out_validate() operation to validate the
extensible format buffer content at qbuf time.
Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
.../platform/rockchip/rkisp1/rkisp1-common.h | 3 +
.../platform/rockchip/rkisp1/rkisp1-params.c | 553 +++++++++++++++++-
2 files changed, 540 insertions(+), 16 deletions(-)
Comments
Hi Jacopo,
Thank you for the patch.
On Fri, Jun 21, 2024 at 04:54:04PM +0200, Jacopo Mondi wrote:
> Implement support in rkisp1-params for the extensible configuration
> parameters format.
>
> Create a list of handlers for each ISP block that wraps the existing
> configuration functions and handles the ISP block enablement.
>
> Parse the configuration parameters buffer in rkisp1_ext_params_config
> and filter the enable blocks by group, to allow setting the 'other'
> groups separately from the 'lsc' group to support the pre/post-configure
> operations.
>
> Implement the vb2 buf_out_validate() operation to validate the
> extensible format buffer content at qbuf time.
Is there a particular reason to do so in .buf_out_validate() instead of
.buf_prepare() ?
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> ---
> .../platform/rockchip/rkisp1/rkisp1-common.h | 3 +
> .../platform/rockchip/rkisp1/rkisp1-params.c | 553 +++++++++++++++++-
> 2 files changed, 540 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> index ccd2065351b4..bffd936f989a 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> @@ -390,6 +390,7 @@ struct rkisp1_params_ops {
> * @quantization: the quantization configured on the isp's src pad
> * @ycbcr_encoding the YCbCr encoding
> * @raw_type: the bayer pattern on the isp video sink pad
> + * @enabled_blocks: bitmask of enabled ISP blocks
> */
> struct rkisp1_params {
> struct rkisp1_vdev_node vnode;
> @@ -404,6 +405,8 @@ struct rkisp1_params {
> enum v4l2_quantization quantization;
> enum v4l2_ycbcr_encoding ycbcr_encoding;
> enum rkisp1_fmt_raw_pat_type raw_type;
> +
> + u32 enabled_blocks;
> };
>
> /*
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
> index e38d2da994f5..f3ea70c7e0c1 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
> @@ -35,6 +35,9 @@
> #define RKISP1_ISP_CC_COEFF(n) \
> (RKISP1_CIF_ISP_CC_COEFF_0 + (n) * 4)
>
> +#define RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS BIT(0)
> +#define RKISP1_EXT_PARAMS_BLOCK_GROUP_LSC BIT(1)
> +
> enum rkisp1_params_formats {
> RKISP1_PARAMS_FIXED,
> RKISP1_PARAMS_EXTENSIBLE,
> @@ -1529,6 +1532,454 @@ rkisp1_params_get_buffer(struct rkisp1_params *params)
> queue);
> }
>
> +/*------------------------------------------------------------------------------
> + * Extensible parameters format handling
> + */
> +
> +static void rkisp1_ext_params_bls(struct rkisp1_params *params,
> + struct rkisp1_ext_params_block_header *hdr)
> +{
> + struct rkisp1_ext_params_bls_config *bls =
> + (struct rkisp1_ext_params_bls_config *)hdr;
> +
> + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_BLS_CTRL,
> + RKISP1_CIF_ISP_BLS_ENA);
> + return;
> + }
> +
> + rkisp1_bls_config(params, &bls->bls_config);
> +
> + if (!(params->enabled_blocks & BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_BLS)))
> + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_BLS_CTRL,
> + RKISP1_CIF_ISP_BLS_ENA);
> +}
> +
> +static void rkisp1_ext_params_dpcc(struct rkisp1_params *params,
> + struct rkisp1_ext_params_block_header *hdr)
> +{
> + struct rkisp1_ext_params_dpcc_config *dpcc =
> + (struct rkisp1_ext_params_dpcc_config *)hdr;
> +
> + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_DPCC_MODE,
> + RKISP1_CIF_ISP_DPCC_MODE_DPCC_ENABLE);
> + return;
> + }
> +
> + rkisp1_dpcc_config(params, &dpcc->dpcc_config);
> +
> + if (!(params->enabled_blocks &
> + BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_DPCC)))
> + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_DPCC_MODE,
> + RKISP1_CIF_ISP_DPCC_MODE_DPCC_ENABLE);
> +}
> +
> +static void rkisp1_ext_params_sdg(struct rkisp1_params *params,
> + struct rkisp1_ext_params_block_header *hdr)
> +{
> + struct rkisp1_ext_params_sdg_config *sdg =
> + (struct rkisp1_ext_params_sdg_config *)hdr;
> +
> + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_CTRL,
> + RKISP1_CIF_ISP_CTRL_ISP_GAMMA_IN_ENA);
> + return;
> + }
> +
> + rkisp1_sdg_config(params, &sdg->sdg_config);
> +
> + if (!(params->enabled_blocks & BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_SDG)))
> + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CTRL,
> + RKISP1_CIF_ISP_CTRL_ISP_GAMMA_IN_ENA);
> +}
> +
> +static void rkisp1_ext_params_lsc(struct rkisp1_params *params,
> + struct rkisp1_ext_params_block_header *hdr)
> +{
> + struct rkisp1_ext_params_lsc_config *lsc =
> + (struct rkisp1_ext_params_lsc_config *)hdr;
> +
> + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_LSC_CTRL,
> + RKISP1_CIF_ISP_LSC_CTRL_ENA);
> + return;
> + }
> +
> + rkisp1_lsc_config(params, &lsc->lsc_config);
> +
> + if (!(params->enabled_blocks & BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_LSC)))
> + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_LSC_CTRL,
> + RKISP1_CIF_ISP_LSC_CTRL_ENA);
> +}
> +
> +static void rkisp1_ext_params_awbg(struct rkisp1_params *params,
> + struct rkisp1_ext_params_block_header *hdr)
> +{
> + struct rkisp1_ext_params_awb_gain_config *awbg =
> + (struct rkisp1_ext_params_awb_gain_config *)hdr;
> +
> + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_CTRL,
> + RKISP1_CIF_ISP_CTRL_ISP_AWB_ENA);
> + return;
> + }
> +
> + params->ops->awb_gain_config(params, &awbg->awb_config);
> +
> + if (!(params->enabled_blocks &
> + BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_AWB_GAINS)))
> + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CTRL,
> + RKISP1_CIF_ISP_CTRL_ISP_AWB_ENA);
> +}
> +
> +static void rkisp1_ext_params_flt(struct rkisp1_params *params,
> + struct rkisp1_ext_params_block_header *hdr)
> +{
> + struct rkisp1_ext_params_flt_config *flt =
> + (struct rkisp1_ext_params_flt_config *)hdr;
> +
> + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_FILT_MODE,
> + RKISP1_CIF_ISP_FLT_ENA);
> + return;
> + }
> +
> + rkisp1_flt_config(params, &flt->flt_config);
> +
> + if (!(params->enabled_blocks & BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_FLT)))
> + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_FILT_MODE,
> + RKISP1_CIF_ISP_FLT_ENA);
> +}
> +
> +static void rkisp1_ext_params_bdm(struct rkisp1_params *params,
> + struct rkisp1_ext_params_block_header *hdr)
> +{
> + struct rkisp1_ext_params_bdm_config *bdm =
> + (struct rkisp1_ext_params_bdm_config *)hdr;
> +
> + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_DEMOSAIC,
> + RKISP1_CIF_ISP_DEMOSAIC_BYPASS);
> + return;
> + }
> +
> + rkisp1_bdm_config(params, &bdm->bdm_config);
> +
> + if (!(params->enabled_blocks & BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_BDM)))
> + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_DEMOSAIC,
> + RKISP1_CIF_ISP_DEMOSAIC_BYPASS);
> +}
> +
> +static void rkisp1_ext_params_ctk(struct rkisp1_params *params,
> + struct rkisp1_ext_params_block_header *hdr)
> +{
> + struct rkisp1_ext_params_ctk_config *ctk =
> + (struct rkisp1_ext_params_ctk_config *)hdr;
> +
> + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> + rkisp1_ctk_enable(params, false);
> + return;
> + }
> +
> + rkisp1_ctk_config(params, &ctk->ctk_config);
> +
> + if (!(params->enabled_blocks & BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_CTK)))
> + rkisp1_ctk_enable(params, true);
> +}
> +
> +static void rkisp1_ext_params_goc(struct rkisp1_params *params,
> + struct rkisp1_ext_params_block_header *hdr)
> +{
> + struct rkisp1_ext_params_goc_config *goc =
> + (struct rkisp1_ext_params_goc_config *)hdr;
> +
> + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_CTRL,
> + RKISP1_CIF_ISP_CTRL_ISP_GAMMA_OUT_ENA);
> + return;
> + }
> +
> + params->ops->goc_config(params, &goc->goc_config);
> +
> + if (!(params->enabled_blocks & BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_GOC)))
> + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CTRL,
> + RKISP1_CIF_ISP_CTRL_ISP_GAMMA_OUT_ENA);
> +}
> +
> +static void rkisp1_ext_params_dpf(struct rkisp1_params *params,
> + struct rkisp1_ext_params_block_header *hdr)
> +{
> + struct rkisp1_ext_params_dpf_config *dpf =
> + (struct rkisp1_ext_params_dpf_config *)hdr;
> +
> + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_DPF_MODE,
> + RKISP1_CIF_ISP_DPF_MODE_EN);
> + return;
> + }
> +
> + rkisp1_dpf_config(params, &dpf->dpf_config);
> +
> + if (!(params->enabled_blocks & BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_DPF)))
> + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_DPF_MODE,
> + RKISP1_CIF_ISP_DPF_MODE_EN);
> +}
> +
> +static void rkisp1_ext_params_dpfs(struct rkisp1_params *params,
> + struct rkisp1_ext_params_block_header *hdr)
> +{
> + struct rkisp1_ext_params_dpf_strength_config *dpfs =
> + (struct rkisp1_ext_params_dpf_strength_config *)hdr;
> +
> + rkisp1_dpf_strength_config(params, &dpfs->dpf_strength_config);
> +}
> +
> +static void rkisp1_ext_params_cproc(struct rkisp1_params *params,
> + struct rkisp1_ext_params_block_header *hdr)
> +{
> + struct rkisp1_ext_params_cproc_config *cproc =
> + (struct rkisp1_ext_params_cproc_config *)hdr;
> +
> + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> + rkisp1_param_clear_bits(params, RKISP1_CIF_C_PROC_CTRL,
> + RKISP1_CIF_C_PROC_CTR_ENABLE);
> + return;
> + }
> +
> + rkisp1_cproc_config(params, &cproc->cproc_config);
> +
> + if (!(params->enabled_blocks &
> + BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_CPROC)))
> + rkisp1_param_set_bits(params, RKISP1_CIF_C_PROC_CTRL,
> + RKISP1_CIF_C_PROC_CTR_ENABLE);
> +}
> +
> +static void rkisp1_ext_params_ie(struct rkisp1_params *params,
> + struct rkisp1_ext_params_block_header *hdr)
> +{
> + struct rkisp1_ext_params_ie_config *ie =
> + (struct rkisp1_ext_params_ie_config *)hdr;
> +
> + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> + rkisp1_ie_enable(params, false);
> + return;
> + }
> +
> + rkisp1_ie_config(params, &ie->ie_config);
> +
> + if (!(params->enabled_blocks & BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_IE)))
> + rkisp1_ie_enable(params, true);
> +}
> +
> +static void rkisp1_ext_params_awbm(struct rkisp1_params *params,
> + struct rkisp1_ext_params_block_header *hdr)
> +{
> + struct rkisp1_ext_params_awb_meas_config *awbm =
> + (struct rkisp1_ext_params_awb_meas_config *)hdr;
> +
> + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> + params->ops->awb_meas_enable(params, &awbm->awb_meas_config,
> + false);
> + return;
> + }
> +
> + params->ops->awb_meas_config(params, &awbm->awb_meas_config);
> +
> + if (!(params->enabled_blocks &
> + BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_AWB_MEAS)))
> + params->ops->awb_meas_enable(params, &awbm->awb_meas_config,
> + true);
> +}
> +
> +static void rkisp1_ext_params_hstm(struct rkisp1_params *params,
> + struct rkisp1_ext_params_block_header *hdr)
> +{
> + struct rkisp1_ext_params_hst_config *hst =
> + (struct rkisp1_ext_params_hst_config *)hdr;
> +
> + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> + params->ops->hst_enable(params, &hst->hst_config, false);
> + return;
> + }
> +
> + params->ops->hst_config(params, &hst->hst_config);
> +
> + if (!(params->enabled_blocks &
> + BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_HST_MEAS)))
> + params->ops->hst_enable(params, &hst->hst_config, true);
> +}
> +
> +static void rkisp1_ext_params_aecm(struct rkisp1_params *params,
> + struct rkisp1_ext_params_block_header *hdr)
> +{
> + struct rkisp1_ext_params_aec_config *aec =
> + (struct rkisp1_ext_params_aec_config *)hdr;
> +
> + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_EXP_CTRL,
> + RKISP1_CIF_ISP_EXP_ENA);
> + return;
> + }
> +
> + params->ops->aec_config(params, &aec->aec_config);
> +
> + if (!(params->enabled_blocks &
> + BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_AEC_MEAS)))
> + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_EXP_CTRL,
> + RKISP1_CIF_ISP_EXP_ENA);
> +}
> +
> +static void rkisp1_ext_params_afcm(struct rkisp1_params *params,
> + struct rkisp1_ext_params_block_header *hdr)
> +{
> + struct rkisp1_ext_params_afc_config *afc =
> + (struct rkisp1_ext_params_afc_config *)hdr;
> +
> + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_AFM_CTRL,
> + RKISP1_CIF_ISP_AFM_ENA);
> + return;
> + }
> +
> + params->ops->afm_config(params, &afc->afc_config);
> +
> + if (!(params->enabled_blocks &
> + BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_AFC_MEAS)))
> + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_AFM_CTRL,
> + RKISP1_CIF_ISP_AFM_ENA);
> +}
I still think we could get rid of most of these functions by adding
.enable(), .configure() and .disable() operations to
rkisp1_ext_params_handler. That can be done later.
> +
> +typedef void (*rkisp1_block_handler)(struct rkisp1_params *params,
> + struct rkisp1_ext_params_block_header *hdr);
If it doesn't cause any issue, I'd make the hdr const.
> +
> +static const struct rkisp1_ext_params_handler {
> + size_t size;
> + rkisp1_block_handler handler;
> + unsigned int group;
> +} rkisp1_ext_params_handlers[] = {
> + [RKISP1_EXT_PARAMS_BLOCK_TYPE_BLS] = {
> + .size = sizeof(struct rkisp1_ext_params_bls_config),
> + .handler = rkisp1_ext_params_bls,
> + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> + },
> + [RKISP1_EXT_PARAMS_BLOCK_TYPE_DPCC] = {
> + .size = sizeof(struct rkisp1_ext_params_dpcc_config),
> + .handler = rkisp1_ext_params_dpcc,
> + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> + },
> + [RKISP1_EXT_PARAMS_BLOCK_TYPE_SDG] = {
> + .size = sizeof(struct rkisp1_ext_params_sdg_config),
> + .handler = rkisp1_ext_params_sdg,
> + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> + },
> + [RKISP1_EXT_PARAMS_BLOCK_TYPE_AWB_GAINS] = {
> + .size =
> + sizeof(struct rkisp1_ext_params_awb_gain_config),
> + .handler = rkisp1_ext_params_awbg,
> + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> + },
> + [RKISP1_EXT_PARAMS_BLOCK_TYPE_FLT] = {
> + .size = sizeof(struct rkisp1_ext_params_flt_config),
> + .handler = rkisp1_ext_params_flt,
> + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> + },
> + [RKISP1_EXT_PARAMS_BLOCK_TYPE_BDM] = {
> + .size = sizeof(struct rkisp1_ext_params_bdm_config),
> + .handler = rkisp1_ext_params_bdm,
> + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> + },
> + [RKISP1_EXT_PARAMS_BLOCK_TYPE_CTK] = {
> + .size = sizeof(struct rkisp1_ext_params_ctk_config),
> + .handler = rkisp1_ext_params_ctk,
> + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> + },
> + [RKISP1_EXT_PARAMS_BLOCK_TYPE_GOC] = {
> + .size = sizeof(struct rkisp1_ext_params_goc_config),
> + .handler = rkisp1_ext_params_goc,
> + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> + },
> + [RKISP1_EXT_PARAMS_BLOCK_TYPE_DPF] = {
> + .size = sizeof(struct rkisp1_ext_params_dpf_config),
> + .handler = rkisp1_ext_params_dpf,
> + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> + },
> + [RKISP1_EXT_PARAMS_BLOCK_TYPE_DPF_STRENGTH] = {
> + .size =
> + sizeof(struct rkisp1_ext_params_dpf_strength_config),
> + .handler = rkisp1_ext_params_dpfs,
> + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> + },
> + [RKISP1_EXT_PARAMS_BLOCK_TYPE_CPROC] = {
> + .size = sizeof(struct rkisp1_ext_params_cproc_config),
> + .handler = rkisp1_ext_params_cproc,
> + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> + },
> + [RKISP1_EXT_PARAMS_BLOCK_TYPE_IE] = {
> + .size = sizeof(struct rkisp1_ext_params_ie_config),
> + .handler = rkisp1_ext_params_ie,
> + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> + },
> + [RKISP1_EXT_PARAMS_BLOCK_TYPE_LSC] = {
> + .size = sizeof(struct rkisp1_ext_params_lsc_config),
> + .handler = rkisp1_ext_params_lsc,
> + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_LSC
> + },
> + [RKISP1_EXT_PARAMS_BLOCK_TYPE_AWB_MEAS] = {
> + .size =
> + sizeof(struct rkisp1_ext_params_awb_meas_config),
> + .handler = rkisp1_ext_params_awbm,
> + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> + },
> + [RKISP1_EXT_PARAMS_BLOCK_TYPE_HST_MEAS] = {
> + .size = sizeof(struct rkisp1_ext_params_hst_config),
> + .handler = rkisp1_ext_params_hstm,
> + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> + },
> + [RKISP1_EXT_PARAMS_BLOCK_TYPE_AEC_MEAS] = {
> + .size = sizeof(struct rkisp1_ext_params_aec_config),
> + .handler = rkisp1_ext_params_aecm,
> + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> + },
> + [RKISP1_EXT_PARAMS_BLOCK_TYPE_AFC_MEAS] = {
> + .size = sizeof(struct rkisp1_ext_params_afc_config),
> + .handler = rkisp1_ext_params_afcm,
> + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> + },
> +};
> +
> +static void rkisp1_ext_params_config(struct rkisp1_params *params,
> + struct rkisp1_ext_params_cfg *cfg,
> + u32 block_group_mask)
> +{
> + size_t block_offset = 0;
> +
> + if (WARN_ON(!cfg))
> + return;
> +
> + /* Walk the list of parameter blocks and process them. */
> + while (block_offset < cfg->total_size) {
> + const struct rkisp1_ext_params_handler *block_handler;
> + struct rkisp1_ext_params_block_header *block;
const
> +
> + block = (struct rkisp1_ext_params_block_header *)
> + &cfg->data[block_offset];
> + block_offset += block->size;
> +
> + /* Make sure the block is in the list of groups to configure. */
> + block_handler = &rkisp1_ext_params_handlers[block->type];
> + if (!(block_handler->group & block_group_mask))
> + continue;
> +
> + block_handler->handler(params, block);
> +
> + if (block->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE)
> + params->enabled_blocks &= ~BIT(block->type);
> + else
> + params->enabled_blocks |= BIT(block->type);
> + }
> +}
> +
> static void rkisp1_params_complete_buffer(struct rkisp1_params *params,
> struct rkisp1_params_buffer *buf,
> unsigned int frame_sequence)
> @@ -1550,9 +2001,15 @@ void rkisp1_params_isr(struct rkisp1_device *rkisp1)
> if (!cur_buf)
> goto unlock;
>
> - rkisp1_isp_isr_other_config(params, cur_buf->cfg);
> - rkisp1_isp_isr_lsc_config(params, cur_buf->cfg);
> - rkisp1_isp_isr_meas_config(params, cur_buf->cfg);
> + if (params->metafmt->dataformat == V4L2_META_FMT_RK_ISP1_PARAMS) {
> + rkisp1_isp_isr_other_config(params, cur_buf->cfg);
> + rkisp1_isp_isr_lsc_config(params, cur_buf->cfg);
> + rkisp1_isp_isr_meas_config(params, cur_buf->cfg);
> + } else {
> + rkisp1_ext_params_config(params, cur_buf->cfg,
> + RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS |
> + RKISP1_EXT_PARAMS_BLOCK_GROUP_LSC);
> + }
>
> /* update shadow register immediately */
> rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CTRL,
> @@ -1651,8 +2108,13 @@ void rkisp1_params_pre_configure(struct rkisp1_params *params,
> if (!cur_buf)
> goto unlock;
>
> - rkisp1_isp_isr_other_config(params, cur_buf->cfg);
> - rkisp1_isp_isr_meas_config(params, cur_buf->cfg);
> + if (params->metafmt->dataformat == V4L2_META_FMT_RK_ISP1_PARAMS) {
> + rkisp1_isp_isr_other_config(params, cur_buf->cfg);
> + rkisp1_isp_isr_meas_config(params, cur_buf->cfg);
> + } else {
> + rkisp1_ext_params_config(params, cur_buf->cfg,
> + RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS);
> + }
>
> /* update shadow register immediately */
> rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CTRL,
> @@ -1680,7 +2142,11 @@ void rkisp1_params_post_configure(struct rkisp1_params *params)
> if (!cur_buf)
> goto unlock;
>
> - rkisp1_isp_isr_lsc_config(params, cur_buf->cfg);
> + if (params->metafmt->dataformat == V4L2_META_FMT_RK_ISP1_PARAMS)
> + rkisp1_isp_isr_lsc_config(params, cur_buf->cfg);
> + else
> + rkisp1_ext_params_config(params, cur_buf->cfg,
> + RKISP1_EXT_PARAMS_BLOCK_GROUP_LSC);
>
> /* update shadow register immediately */
> rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CTRL,
> @@ -1874,10 +2340,6 @@ static void rkisp1_params_vb2_buf_queue(struct vb2_buffer *vb)
>
> static int rkisp1_params_vb2_buf_prepare(struct vb2_buffer *vb)
> {
> - struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> - struct rkisp1_params_buffer *params_buf =
> - container_of(vbuf, struct rkisp1_params_buffer, vb);
> - void *cfg = vb2_plane_vaddr(¶ms_buf->vb.vb2_buf, 0);
> struct rkisp1_params *params = vb->vb2_queue->drv_priv;
>
> if (vb2_plane_size(vb, 0) < params->metafmt->buffersize)
> @@ -1885,12 +2347,6 @@ static int rkisp1_params_vb2_buf_prepare(struct vb2_buffer *vb)
>
> vb2_set_plane_payload(vb, 0, params->metafmt->buffersize);
>
> - /*
> - * Copy the parameters buffer to the internal scratch buffer to avoid
> - * userspace modifying the buffer content while the driver processes it.
> - */
> - memcpy(params_buf->cfg, cfg, params->metafmt->buffersize);
> -
> return 0;
> }
>
> @@ -1911,12 +2367,77 @@ static void rkisp1_params_vb2_stop_streaming(struct vb2_queue *vq)
>
> list_for_each_entry(buf, &tmp_list, queue)
> vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_ERROR);
> +
> + params->enabled_blocks = 0;
> +}
> +
> +static int rkisp1_params_validate_ext_params(struct rkisp1_params *params,
> + struct rkisp1_ext_params_cfg *cfg)
> +{
> + size_t block_offset = 0;
> +
> + if (cfg->total_size > RKISP1_EXT_PARAMS_MAX_SIZE) {
> + dev_dbg(params->rkisp1->dev,
> + "Invalid parameters buffer size %u\n",
> + cfg->total_size);
> + return -EINVAL;
> + }
Following my review comments on patch 5/7, you should also validate that
offsetof(struct rkisp1_ext_params_cfg, data) + cfg->total_size >= plane payload.
> +
> + /* Walk the list of parameter blocks and validate them. */
> + while (block_offset < cfg->total_size) {
while (cfg->total_size - block_offset >=
sizeof(struct rkisp1_ext_params_block_header)) {
Otherwise there could be just one byte left, and dereferencing block
below would read uninitialized memory.
> + const struct rkisp1_ext_params_handler *hdlr;
Maybe handler, it's not much longer, and is more readable.
> + struct rkisp1_ext_params_block_header *block;
const
> +
> + block = (struct rkisp1_ext_params_block_header *)
> + &cfg->data[block_offset];
> +
> + if (block->type >= RKISP1_EXT_PARAMS_BLOCK_TYPE_SENTINEL) {
if (block->type >= ARRAY_SIZE(rkisp1_ext_params_handlers)) {
which allows you to drop RKISP1_EXT_PARAMS_BLOCK_TYPE_SENTINEL from the
UAPI header.
> + dev_dbg(params->rkisp1->dev,
> + "Invalid parameters block type\n");
> + return -EINVAL;
> + }
> +
> + hdlr = &rkisp1_ext_params_handlers[block->type];
> + if (block->size != hdlr->size) {
> + dev_dbg(params->rkisp1->dev,
> + "Invalid parameters block size\n");
> + return -EINVAL;
> + }
You need to test here that block->size >= cfg->total_size -
block_offset. It may be easier to add a new local variable at the top of
the function
size_t remaining_size = cfg->total_size;
The loop would then become
while (remaining_size >= sizeof(struct rkisp1_ext_params_block_header)) {
const struct rkisp1_ext_params_block_header *block;
const struct rkisp1_ext_params_handler *handler;
block = (struct rkisp1_ext_params_block_header *)
&cfg->data[block_offset];
if (block->type >= ARRAY_SIZE(rkisp1_ext_params_handlers)) {
dev_dbg(params->rkisp1->dev,
"Invalid parameters block type\n");
return -EINVAL;
}
if (block->size > remaining_size) {
dev_dbg(params->rkisp1->dev,
"Premature end of parameters data\n");
return -EINVAL;
}
handler = &rkisp1_ext_params_handlers[block->type];
if (block->size != handler->size) {
dev_dbg(params->rkisp1->dev,
"Invalid parameters block size\n");
return -EINVAL;
}
block_offset += block->size;
reamining_size -= block->size;
}
> +
> + block_offset += block->size;
> + }
> +
> + return 0;
> +}
> +
> +static int rkisp1_params_vb2_buf_out_validate(struct vb2_buffer *vb)
> +{
> + struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> + struct rkisp1_params_buffer *params_buf =
> + container_of(vbuf, struct rkisp1_params_buffer, vb);
> + struct vb2_queue *vq = vb->vb2_queue;
> + struct rkisp1_params *params = vq->drv_priv;
> + struct rkisp1_ext_params_cfg *cfg =
> + vb2_plane_vaddr(¶ms_buf->vb.vb2_buf, 0);
> +
> + /*
> + * Copy the parameters buffer to the internal scratch buffer to avoid
> + * userspace modifying the buffer content while the driver processes it.
> + */
> + memcpy(params_buf->cfg, cfg, params->metafmt->buffersize);
> +
> + /* Fixed parameters format doesn't require validation. */
> + if (params->metafmt->dataformat == V4L2_META_FMT_RK_ISP1_PARAMS)
> + return 0;
> +
> + return rkisp1_params_validate_ext_params(params, cfg);
> }
>
> static const struct vb2_ops rkisp1_params_vb2_ops = {
> .queue_setup = rkisp1_params_vb2_queue_setup,
> .buf_init = rkisp1_params_vb2_buf_init,
> .buf_cleanup = rkisp1_params_vb2_buf_cleanup,
> + .buf_out_validate = rkisp1_params_vb2_buf_out_validate,
> .wait_prepare = vb2_ops_wait_prepare,
> .wait_finish = vb2_ops_wait_finish,
> .buf_queue = rkisp1_params_vb2_buf_queue,
Hi Laurent
On Sat, Jun 29, 2024 at 05:36:07PM GMT, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Fri, Jun 21, 2024 at 04:54:04PM +0200, Jacopo Mondi wrote:
> > Implement support in rkisp1-params for the extensible configuration
> > parameters format.
> >
> > Create a list of handlers for each ISP block that wraps the existing
> > configuration functions and handles the ISP block enablement.
> >
> > Parse the configuration parameters buffer in rkisp1_ext_params_config
> > and filter the enable blocks by group, to allow setting the 'other'
> > groups separately from the 'lsc' group to support the pre/post-configure
> > operations.
> >
> > Implement the vb2 buf_out_validate() operation to validate the
> > extensible format buffer content at qbuf time.
>
> Is there a particular reason to do so in .buf_out_validate() instead of
> .buf_prepare() ?
>
It felt like it's the right place where to perform validation
* @buf_out_validate: called when the output buffer is prepared or queued
* to a request; drivers can use this to validate
* userspace-provided information; this is required only
* for OUTPUT queues.
out buffers are validated -before- buf_prepare() is called. I admit it
might have no practical differences though. Would you prefer to
collapse everything into _prepare ? If I drop
vb2_set_plane_payload(vb, 0, params->metafmt->buffersize);
Not much will remain in _prepare() and I could even drop it.
> > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > ---
> > .../platform/rockchip/rkisp1/rkisp1-common.h | 3 +
> > .../platform/rockchip/rkisp1/rkisp1-params.c | 553 +++++++++++++++++-
> > 2 files changed, 540 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> > index ccd2065351b4..bffd936f989a 100644
> > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> > @@ -390,6 +390,7 @@ struct rkisp1_params_ops {
> > * @quantization: the quantization configured on the isp's src pad
> > * @ycbcr_encoding the YCbCr encoding
> > * @raw_type: the bayer pattern on the isp video sink pad
> > + * @enabled_blocks: bitmask of enabled ISP blocks
> > */
> > struct rkisp1_params {
> > struct rkisp1_vdev_node vnode;
> > @@ -404,6 +405,8 @@ struct rkisp1_params {
> > enum v4l2_quantization quantization;
> > enum v4l2_ycbcr_encoding ycbcr_encoding;
> > enum rkisp1_fmt_raw_pat_type raw_type;
> > +
> > + u32 enabled_blocks;
> > };
> >
> > /*
> > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
> > index e38d2da994f5..f3ea70c7e0c1 100644
> > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
> > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
> > @@ -35,6 +35,9 @@
> > #define RKISP1_ISP_CC_COEFF(n) \
> > (RKISP1_CIF_ISP_CC_COEFF_0 + (n) * 4)
> >
> > +#define RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS BIT(0)
> > +#define RKISP1_EXT_PARAMS_BLOCK_GROUP_LSC BIT(1)
> > +
> > enum rkisp1_params_formats {
> > RKISP1_PARAMS_FIXED,
> > RKISP1_PARAMS_EXTENSIBLE,
> > @@ -1529,6 +1532,454 @@ rkisp1_params_get_buffer(struct rkisp1_params *params)
> > queue);
> > }
> >
> > +/*------------------------------------------------------------------------------
> > + * Extensible parameters format handling
> > + */
> > +
> > +static void rkisp1_ext_params_bls(struct rkisp1_params *params,
> > + struct rkisp1_ext_params_block_header *hdr)
> > +{
> > + struct rkisp1_ext_params_bls_config *bls =
> > + (struct rkisp1_ext_params_bls_config *)hdr;
> > +
> > + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> > + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_BLS_CTRL,
> > + RKISP1_CIF_ISP_BLS_ENA);
> > + return;
> > + }
> > +
> > + rkisp1_bls_config(params, &bls->bls_config);
> > +
> > + if (!(params->enabled_blocks & BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_BLS)))
> > + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_BLS_CTRL,
> > + RKISP1_CIF_ISP_BLS_ENA);
> > +}
> > +
> > +static void rkisp1_ext_params_dpcc(struct rkisp1_params *params,
> > + struct rkisp1_ext_params_block_header *hdr)
> > +{
> > + struct rkisp1_ext_params_dpcc_config *dpcc =
> > + (struct rkisp1_ext_params_dpcc_config *)hdr;
> > +
> > + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> > + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_DPCC_MODE,
> > + RKISP1_CIF_ISP_DPCC_MODE_DPCC_ENABLE);
> > + return;
> > + }
> > +
> > + rkisp1_dpcc_config(params, &dpcc->dpcc_config);
> > +
> > + if (!(params->enabled_blocks &
> > + BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_DPCC)))
> > + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_DPCC_MODE,
> > + RKISP1_CIF_ISP_DPCC_MODE_DPCC_ENABLE);
> > +}
> > +
> > +static void rkisp1_ext_params_sdg(struct rkisp1_params *params,
> > + struct rkisp1_ext_params_block_header *hdr)
> > +{
> > + struct rkisp1_ext_params_sdg_config *sdg =
> > + (struct rkisp1_ext_params_sdg_config *)hdr;
> > +
> > + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> > + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_CTRL,
> > + RKISP1_CIF_ISP_CTRL_ISP_GAMMA_IN_ENA);
> > + return;
> > + }
> > +
> > + rkisp1_sdg_config(params, &sdg->sdg_config);
> > +
> > + if (!(params->enabled_blocks & BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_SDG)))
> > + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CTRL,
> > + RKISP1_CIF_ISP_CTRL_ISP_GAMMA_IN_ENA);
> > +}
> > +
> > +static void rkisp1_ext_params_lsc(struct rkisp1_params *params,
> > + struct rkisp1_ext_params_block_header *hdr)
> > +{
> > + struct rkisp1_ext_params_lsc_config *lsc =
> > + (struct rkisp1_ext_params_lsc_config *)hdr;
> > +
> > + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> > + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_LSC_CTRL,
> > + RKISP1_CIF_ISP_LSC_CTRL_ENA);
> > + return;
> > + }
> > +
> > + rkisp1_lsc_config(params, &lsc->lsc_config);
> > +
> > + if (!(params->enabled_blocks & BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_LSC)))
> > + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_LSC_CTRL,
> > + RKISP1_CIF_ISP_LSC_CTRL_ENA);
> > +}
> > +
> > +static void rkisp1_ext_params_awbg(struct rkisp1_params *params,
> > + struct rkisp1_ext_params_block_header *hdr)
> > +{
> > + struct rkisp1_ext_params_awb_gain_config *awbg =
> > + (struct rkisp1_ext_params_awb_gain_config *)hdr;
> > +
> > + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> > + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_CTRL,
> > + RKISP1_CIF_ISP_CTRL_ISP_AWB_ENA);
> > + return;
> > + }
> > +
> > + params->ops->awb_gain_config(params, &awbg->awb_config);
> > +
> > + if (!(params->enabled_blocks &
> > + BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_AWB_GAINS)))
> > + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CTRL,
> > + RKISP1_CIF_ISP_CTRL_ISP_AWB_ENA);
> > +}
> > +
> > +static void rkisp1_ext_params_flt(struct rkisp1_params *params,
> > + struct rkisp1_ext_params_block_header *hdr)
> > +{
> > + struct rkisp1_ext_params_flt_config *flt =
> > + (struct rkisp1_ext_params_flt_config *)hdr;
> > +
> > + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> > + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_FILT_MODE,
> > + RKISP1_CIF_ISP_FLT_ENA);
> > + return;
> > + }
> > +
> > + rkisp1_flt_config(params, &flt->flt_config);
> > +
> > + if (!(params->enabled_blocks & BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_FLT)))
> > + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_FILT_MODE,
> > + RKISP1_CIF_ISP_FLT_ENA);
> > +}
> > +
> > +static void rkisp1_ext_params_bdm(struct rkisp1_params *params,
> > + struct rkisp1_ext_params_block_header *hdr)
> > +{
> > + struct rkisp1_ext_params_bdm_config *bdm =
> > + (struct rkisp1_ext_params_bdm_config *)hdr;
> > +
> > + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> > + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_DEMOSAIC,
> > + RKISP1_CIF_ISP_DEMOSAIC_BYPASS);
> > + return;
> > + }
> > +
> > + rkisp1_bdm_config(params, &bdm->bdm_config);
> > +
> > + if (!(params->enabled_blocks & BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_BDM)))
> > + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_DEMOSAIC,
> > + RKISP1_CIF_ISP_DEMOSAIC_BYPASS);
> > +}
> > +
> > +static void rkisp1_ext_params_ctk(struct rkisp1_params *params,
> > + struct rkisp1_ext_params_block_header *hdr)
> > +{
> > + struct rkisp1_ext_params_ctk_config *ctk =
> > + (struct rkisp1_ext_params_ctk_config *)hdr;
> > +
> > + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> > + rkisp1_ctk_enable(params, false);
> > + return;
> > + }
> > +
> > + rkisp1_ctk_config(params, &ctk->ctk_config);
> > +
> > + if (!(params->enabled_blocks & BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_CTK)))
> > + rkisp1_ctk_enable(params, true);
> > +}
> > +
> > +static void rkisp1_ext_params_goc(struct rkisp1_params *params,
> > + struct rkisp1_ext_params_block_header *hdr)
> > +{
> > + struct rkisp1_ext_params_goc_config *goc =
> > + (struct rkisp1_ext_params_goc_config *)hdr;
> > +
> > + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> > + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_CTRL,
> > + RKISP1_CIF_ISP_CTRL_ISP_GAMMA_OUT_ENA);
> > + return;
> > + }
> > +
> > + params->ops->goc_config(params, &goc->goc_config);
> > +
> > + if (!(params->enabled_blocks & BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_GOC)))
> > + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CTRL,
> > + RKISP1_CIF_ISP_CTRL_ISP_GAMMA_OUT_ENA);
> > +}
> > +
> > +static void rkisp1_ext_params_dpf(struct rkisp1_params *params,
> > + struct rkisp1_ext_params_block_header *hdr)
> > +{
> > + struct rkisp1_ext_params_dpf_config *dpf =
> > + (struct rkisp1_ext_params_dpf_config *)hdr;
> > +
> > + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> > + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_DPF_MODE,
> > + RKISP1_CIF_ISP_DPF_MODE_EN);
> > + return;
> > + }
> > +
> > + rkisp1_dpf_config(params, &dpf->dpf_config);
> > +
> > + if (!(params->enabled_blocks & BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_DPF)))
> > + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_DPF_MODE,
> > + RKISP1_CIF_ISP_DPF_MODE_EN);
> > +}
> > +
> > +static void rkisp1_ext_params_dpfs(struct rkisp1_params *params,
> > + struct rkisp1_ext_params_block_header *hdr)
> > +{
> > + struct rkisp1_ext_params_dpf_strength_config *dpfs =
> > + (struct rkisp1_ext_params_dpf_strength_config *)hdr;
> > +
> > + rkisp1_dpf_strength_config(params, &dpfs->dpf_strength_config);
> > +}
> > +
> > +static void rkisp1_ext_params_cproc(struct rkisp1_params *params,
> > + struct rkisp1_ext_params_block_header *hdr)
> > +{
> > + struct rkisp1_ext_params_cproc_config *cproc =
> > + (struct rkisp1_ext_params_cproc_config *)hdr;
> > +
> > + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> > + rkisp1_param_clear_bits(params, RKISP1_CIF_C_PROC_CTRL,
> > + RKISP1_CIF_C_PROC_CTR_ENABLE);
> > + return;
> > + }
> > +
> > + rkisp1_cproc_config(params, &cproc->cproc_config);
> > +
> > + if (!(params->enabled_blocks &
> > + BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_CPROC)))
> > + rkisp1_param_set_bits(params, RKISP1_CIF_C_PROC_CTRL,
> > + RKISP1_CIF_C_PROC_CTR_ENABLE);
> > +}
> > +
> > +static void rkisp1_ext_params_ie(struct rkisp1_params *params,
> > + struct rkisp1_ext_params_block_header *hdr)
> > +{
> > + struct rkisp1_ext_params_ie_config *ie =
> > + (struct rkisp1_ext_params_ie_config *)hdr;
> > +
> > + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> > + rkisp1_ie_enable(params, false);
> > + return;
> > + }
> > +
> > + rkisp1_ie_config(params, &ie->ie_config);
> > +
> > + if (!(params->enabled_blocks & BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_IE)))
> > + rkisp1_ie_enable(params, true);
> > +}
> > +
> > +static void rkisp1_ext_params_awbm(struct rkisp1_params *params,
> > + struct rkisp1_ext_params_block_header *hdr)
> > +{
> > + struct rkisp1_ext_params_awb_meas_config *awbm =
> > + (struct rkisp1_ext_params_awb_meas_config *)hdr;
> > +
> > + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> > + params->ops->awb_meas_enable(params, &awbm->awb_meas_config,
> > + false);
> > + return;
> > + }
> > +
> > + params->ops->awb_meas_config(params, &awbm->awb_meas_config);
> > +
> > + if (!(params->enabled_blocks &
> > + BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_AWB_MEAS)))
> > + params->ops->awb_meas_enable(params, &awbm->awb_meas_config,
> > + true);
> > +}
> > +
> > +static void rkisp1_ext_params_hstm(struct rkisp1_params *params,
> > + struct rkisp1_ext_params_block_header *hdr)
> > +{
> > + struct rkisp1_ext_params_hst_config *hst =
> > + (struct rkisp1_ext_params_hst_config *)hdr;
> > +
> > + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> > + params->ops->hst_enable(params, &hst->hst_config, false);
> > + return;
> > + }
> > +
> > + params->ops->hst_config(params, &hst->hst_config);
> > +
> > + if (!(params->enabled_blocks &
> > + BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_HST_MEAS)))
> > + params->ops->hst_enable(params, &hst->hst_config, true);
> > +}
> > +
> > +static void rkisp1_ext_params_aecm(struct rkisp1_params *params,
> > + struct rkisp1_ext_params_block_header *hdr)
> > +{
> > + struct rkisp1_ext_params_aec_config *aec =
> > + (struct rkisp1_ext_params_aec_config *)hdr;
> > +
> > + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> > + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_EXP_CTRL,
> > + RKISP1_CIF_ISP_EXP_ENA);
> > + return;
> > + }
> > +
> > + params->ops->aec_config(params, &aec->aec_config);
> > +
> > + if (!(params->enabled_blocks &
> > + BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_AEC_MEAS)))
> > + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_EXP_CTRL,
> > + RKISP1_CIF_ISP_EXP_ENA);
> > +}
> > +
> > +static void rkisp1_ext_params_afcm(struct rkisp1_params *params,
> > + struct rkisp1_ext_params_block_header *hdr)
> > +{
> > + struct rkisp1_ext_params_afc_config *afc =
> > + (struct rkisp1_ext_params_afc_config *)hdr;
> > +
> > + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> > + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_AFM_CTRL,
> > + RKISP1_CIF_ISP_AFM_ENA);
> > + return;
> > + }
> > +
> > + params->ops->afm_config(params, &afc->afc_config);
> > +
> > + if (!(params->enabled_blocks &
> > + BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_AFC_MEAS)))
> > + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_AFM_CTRL,
> > + RKISP1_CIF_ISP_AFM_ENA);
> > +}
>
> I still think we could get rid of most of these functions by adding
> .enable(), .configure() and .disable() operations to
> rkisp1_ext_params_handler. That can be done later.
>
> > +
> > +typedef void (*rkisp1_block_handler)(struct rkisp1_params *params,
> > + struct rkisp1_ext_params_block_header *hdr);
>
> If it doesn't cause any issue, I'd make the hdr const.
>
> > +
> > +static const struct rkisp1_ext_params_handler {
> > + size_t size;
> > + rkisp1_block_handler handler;
> > + unsigned int group;
> > +} rkisp1_ext_params_handlers[] = {
> > + [RKISP1_EXT_PARAMS_BLOCK_TYPE_BLS] = {
> > + .size = sizeof(struct rkisp1_ext_params_bls_config),
> > + .handler = rkisp1_ext_params_bls,
> > + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> > + },
> > + [RKISP1_EXT_PARAMS_BLOCK_TYPE_DPCC] = {
> > + .size = sizeof(struct rkisp1_ext_params_dpcc_config),
> > + .handler = rkisp1_ext_params_dpcc,
> > + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> > + },
> > + [RKISP1_EXT_PARAMS_BLOCK_TYPE_SDG] = {
> > + .size = sizeof(struct rkisp1_ext_params_sdg_config),
> > + .handler = rkisp1_ext_params_sdg,
> > + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> > + },
> > + [RKISP1_EXT_PARAMS_BLOCK_TYPE_AWB_GAINS] = {
> > + .size =
> > + sizeof(struct rkisp1_ext_params_awb_gain_config),
> > + .handler = rkisp1_ext_params_awbg,
> > + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> > + },
> > + [RKISP1_EXT_PARAMS_BLOCK_TYPE_FLT] = {
> > + .size = sizeof(struct rkisp1_ext_params_flt_config),
> > + .handler = rkisp1_ext_params_flt,
> > + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> > + },
> > + [RKISP1_EXT_PARAMS_BLOCK_TYPE_BDM] = {
> > + .size = sizeof(struct rkisp1_ext_params_bdm_config),
> > + .handler = rkisp1_ext_params_bdm,
> > + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> > + },
> > + [RKISP1_EXT_PARAMS_BLOCK_TYPE_CTK] = {
> > + .size = sizeof(struct rkisp1_ext_params_ctk_config),
> > + .handler = rkisp1_ext_params_ctk,
> > + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> > + },
> > + [RKISP1_EXT_PARAMS_BLOCK_TYPE_GOC] = {
> > + .size = sizeof(struct rkisp1_ext_params_goc_config),
> > + .handler = rkisp1_ext_params_goc,
> > + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> > + },
> > + [RKISP1_EXT_PARAMS_BLOCK_TYPE_DPF] = {
> > + .size = sizeof(struct rkisp1_ext_params_dpf_config),
> > + .handler = rkisp1_ext_params_dpf,
> > + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> > + },
> > + [RKISP1_EXT_PARAMS_BLOCK_TYPE_DPF_STRENGTH] = {
> > + .size =
> > + sizeof(struct rkisp1_ext_params_dpf_strength_config),
> > + .handler = rkisp1_ext_params_dpfs,
> > + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> > + },
> > + [RKISP1_EXT_PARAMS_BLOCK_TYPE_CPROC] = {
> > + .size = sizeof(struct rkisp1_ext_params_cproc_config),
> > + .handler = rkisp1_ext_params_cproc,
> > + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> > + },
> > + [RKISP1_EXT_PARAMS_BLOCK_TYPE_IE] = {
> > + .size = sizeof(struct rkisp1_ext_params_ie_config),
> > + .handler = rkisp1_ext_params_ie,
> > + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> > + },
> > + [RKISP1_EXT_PARAMS_BLOCK_TYPE_LSC] = {
> > + .size = sizeof(struct rkisp1_ext_params_lsc_config),
> > + .handler = rkisp1_ext_params_lsc,
> > + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_LSC
> > + },
> > + [RKISP1_EXT_PARAMS_BLOCK_TYPE_AWB_MEAS] = {
> > + .size =
> > + sizeof(struct rkisp1_ext_params_awb_meas_config),
> > + .handler = rkisp1_ext_params_awbm,
> > + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> > + },
> > + [RKISP1_EXT_PARAMS_BLOCK_TYPE_HST_MEAS] = {
> > + .size = sizeof(struct rkisp1_ext_params_hst_config),
> > + .handler = rkisp1_ext_params_hstm,
> > + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> > + },
> > + [RKISP1_EXT_PARAMS_BLOCK_TYPE_AEC_MEAS] = {
> > + .size = sizeof(struct rkisp1_ext_params_aec_config),
> > + .handler = rkisp1_ext_params_aecm,
> > + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> > + },
> > + [RKISP1_EXT_PARAMS_BLOCK_TYPE_AFC_MEAS] = {
> > + .size = sizeof(struct rkisp1_ext_params_afc_config),
> > + .handler = rkisp1_ext_params_afcm,
> > + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> > + },
> > +};
> > +
> > +static void rkisp1_ext_params_config(struct rkisp1_params *params,
> > + struct rkisp1_ext_params_cfg *cfg,
> > + u32 block_group_mask)
> > +{
> > + size_t block_offset = 0;
> > +
> > + if (WARN_ON(!cfg))
> > + return;
> > +
> > + /* Walk the list of parameter blocks and process them. */
> > + while (block_offset < cfg->total_size) {
> > + const struct rkisp1_ext_params_handler *block_handler;
> > + struct rkisp1_ext_params_block_header *block;
>
> const
>
> > +
> > + block = (struct rkisp1_ext_params_block_header *)
> > + &cfg->data[block_offset];
> > + block_offset += block->size;
> > +
> > + /* Make sure the block is in the list of groups to configure. */
> > + block_handler = &rkisp1_ext_params_handlers[block->type];
> > + if (!(block_handler->group & block_group_mask))
> > + continue;
> > +
> > + block_handler->handler(params, block);
> > +
> > + if (block->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE)
> > + params->enabled_blocks &= ~BIT(block->type);
> > + else
> > + params->enabled_blocks |= BIT(block->type);
> > + }
> > +}
> > +
> > static void rkisp1_params_complete_buffer(struct rkisp1_params *params,
> > struct rkisp1_params_buffer *buf,
> > unsigned int frame_sequence)
> > @@ -1550,9 +2001,15 @@ void rkisp1_params_isr(struct rkisp1_device *rkisp1)
> > if (!cur_buf)
> > goto unlock;
> >
> > - rkisp1_isp_isr_other_config(params, cur_buf->cfg);
> > - rkisp1_isp_isr_lsc_config(params, cur_buf->cfg);
> > - rkisp1_isp_isr_meas_config(params, cur_buf->cfg);
> > + if (params->metafmt->dataformat == V4L2_META_FMT_RK_ISP1_PARAMS) {
> > + rkisp1_isp_isr_other_config(params, cur_buf->cfg);
> > + rkisp1_isp_isr_lsc_config(params, cur_buf->cfg);
> > + rkisp1_isp_isr_meas_config(params, cur_buf->cfg);
> > + } else {
> > + rkisp1_ext_params_config(params, cur_buf->cfg,
> > + RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS |
> > + RKISP1_EXT_PARAMS_BLOCK_GROUP_LSC);
> > + }
> >
> > /* update shadow register immediately */
> > rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CTRL,
> > @@ -1651,8 +2108,13 @@ void rkisp1_params_pre_configure(struct rkisp1_params *params,
> > if (!cur_buf)
> > goto unlock;
> >
> > - rkisp1_isp_isr_other_config(params, cur_buf->cfg);
> > - rkisp1_isp_isr_meas_config(params, cur_buf->cfg);
> > + if (params->metafmt->dataformat == V4L2_META_FMT_RK_ISP1_PARAMS) {
> > + rkisp1_isp_isr_other_config(params, cur_buf->cfg);
> > + rkisp1_isp_isr_meas_config(params, cur_buf->cfg);
> > + } else {
> > + rkisp1_ext_params_config(params, cur_buf->cfg,
> > + RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS);
> > + }
> >
> > /* update shadow register immediately */
> > rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CTRL,
> > @@ -1680,7 +2142,11 @@ void rkisp1_params_post_configure(struct rkisp1_params *params)
> > if (!cur_buf)
> > goto unlock;
> >
> > - rkisp1_isp_isr_lsc_config(params, cur_buf->cfg);
> > + if (params->metafmt->dataformat == V4L2_META_FMT_RK_ISP1_PARAMS)
> > + rkisp1_isp_isr_lsc_config(params, cur_buf->cfg);
> > + else
> > + rkisp1_ext_params_config(params, cur_buf->cfg,
> > + RKISP1_EXT_PARAMS_BLOCK_GROUP_LSC);
> >
> > /* update shadow register immediately */
> > rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CTRL,
> > @@ -1874,10 +2340,6 @@ static void rkisp1_params_vb2_buf_queue(struct vb2_buffer *vb)
> >
> > static int rkisp1_params_vb2_buf_prepare(struct vb2_buffer *vb)
> > {
> > - struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> > - struct rkisp1_params_buffer *params_buf =
> > - container_of(vbuf, struct rkisp1_params_buffer, vb);
> > - void *cfg = vb2_plane_vaddr(¶ms_buf->vb.vb2_buf, 0);
> > struct rkisp1_params *params = vb->vb2_queue->drv_priv;
> >
> > if (vb2_plane_size(vb, 0) < params->metafmt->buffersize)
> > @@ -1885,12 +2347,6 @@ static int rkisp1_params_vb2_buf_prepare(struct vb2_buffer *vb)
> >
> > vb2_set_plane_payload(vb, 0, params->metafmt->buffersize);
> >
> > - /*
> > - * Copy the parameters buffer to the internal scratch buffer to avoid
> > - * userspace modifying the buffer content while the driver processes it.
> > - */
> > - memcpy(params_buf->cfg, cfg, params->metafmt->buffersize);
> > -
> > return 0;
> > }
> >
> > @@ -1911,12 +2367,77 @@ static void rkisp1_params_vb2_stop_streaming(struct vb2_queue *vq)
> >
> > list_for_each_entry(buf, &tmp_list, queue)
> > vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_ERROR);
> > +
> > + params->enabled_blocks = 0;
> > +}
> > +
> > +static int rkisp1_params_validate_ext_params(struct rkisp1_params *params,
> > + struct rkisp1_ext_params_cfg *cfg)
> > +{
> > + size_t block_offset = 0;
> > +
> > + if (cfg->total_size > RKISP1_EXT_PARAMS_MAX_SIZE) {
> > + dev_dbg(params->rkisp1->dev,
> > + "Invalid parameters buffer size %u\n",
> > + cfg->total_size);
> > + return -EINVAL;
> > + }
>
> Following my review comments on patch 5/7, you should also validate that
> offsetof(struct rkisp1_ext_params_cfg, data) + cfg->total_size >= plane payload.
>
> > +
> > + /* Walk the list of parameter blocks and validate them. */
> > + while (block_offset < cfg->total_size) {
>
> while (cfg->total_size - block_offset >=
> sizeof(struct rkisp1_ext_params_block_header)) {
>
> Otherwise there could be just one byte left, and dereferencing block
> below would read uninitialized memory.
>
> > + const struct rkisp1_ext_params_handler *hdlr;
>
> Maybe handler, it's not much longer, and is more readable.
>
> > + struct rkisp1_ext_params_block_header *block;
>
> const
>
> > +
> > + block = (struct rkisp1_ext_params_block_header *)
> > + &cfg->data[block_offset];
> > +
> > + if (block->type >= RKISP1_EXT_PARAMS_BLOCK_TYPE_SENTINEL) {
>
> if (block->type >= ARRAY_SIZE(rkisp1_ext_params_handlers)) {
>
> which allows you to drop RKISP1_EXT_PARAMS_BLOCK_TYPE_SENTINEL from the
> UAPI header.
>
> > + dev_dbg(params->rkisp1->dev,
> > + "Invalid parameters block type\n");
> > + return -EINVAL;
> > + }
> > +
> > + hdlr = &rkisp1_ext_params_handlers[block->type];
> > + if (block->size != hdlr->size) {
> > + dev_dbg(params->rkisp1->dev,
> > + "Invalid parameters block size\n");
> > + return -EINVAL;
> > + }
>
> You need to test here that block->size >= cfg->total_size -
> block_offset. It may be easier to add a new local variable at the top of
> the function
>
> size_t remaining_size = cfg->total_size;
>
> The loop would then become
>
> while (remaining_size >= sizeof(struct rkisp1_ext_params_block_header)) {
> const struct rkisp1_ext_params_block_header *block;
> const struct rkisp1_ext_params_handler *handler;
>
> block = (struct rkisp1_ext_params_block_header *)
> &cfg->data[block_offset];
>
> if (block->type >= ARRAY_SIZE(rkisp1_ext_params_handlers)) {
> dev_dbg(params->rkisp1->dev,
> "Invalid parameters block type\n");
> return -EINVAL;
> }
>
> if (block->size > remaining_size) {
> dev_dbg(params->rkisp1->dev,
> "Premature end of parameters data\n");
> return -EINVAL;
> }
>
> handler = &rkisp1_ext_params_handlers[block->type];
> if (block->size != handler->size) {
> dev_dbg(params->rkisp1->dev,
> "Invalid parameters block size\n");
> return -EINVAL;
> }
>
> block_offset += block->size;
> reamining_size -= block->size;
> }
>
> > +
> > + block_offset += block->size;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int rkisp1_params_vb2_buf_out_validate(struct vb2_buffer *vb)
> > +{
> > + struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> > + struct rkisp1_params_buffer *params_buf =
> > + container_of(vbuf, struct rkisp1_params_buffer, vb);
> > + struct vb2_queue *vq = vb->vb2_queue;
> > + struct rkisp1_params *params = vq->drv_priv;
> > + struct rkisp1_ext_params_cfg *cfg =
> > + vb2_plane_vaddr(¶ms_buf->vb.vb2_buf, 0);
> > +
> > + /*
> > + * Copy the parameters buffer to the internal scratch buffer to avoid
> > + * userspace modifying the buffer content while the driver processes it.
> > + */
> > + memcpy(params_buf->cfg, cfg, params->metafmt->buffersize);
> > +
> > + /* Fixed parameters format doesn't require validation. */
> > + if (params->metafmt->dataformat == V4L2_META_FMT_RK_ISP1_PARAMS)
> > + return 0;
> > +
> > + return rkisp1_params_validate_ext_params(params, cfg);
> > }
> >
> > static const struct vb2_ops rkisp1_params_vb2_ops = {
> > .queue_setup = rkisp1_params_vb2_queue_setup,
> > .buf_init = rkisp1_params_vb2_buf_init,
> > .buf_cleanup = rkisp1_params_vb2_buf_cleanup,
> > + .buf_out_validate = rkisp1_params_vb2_buf_out_validate,
> > .wait_prepare = vb2_ops_wait_prepare,
> > .wait_finish = vb2_ops_wait_finish,
> > .buf_queue = rkisp1_params_vb2_buf_queue,
>
> --
> Regards,
>
> Laurent Pinchart
>
Hi Jacopo,
On Mon, Jul 01, 2024 at 12:43:14PM +0200, Jacopo Mondi wrote:
> On Sat, Jun 29, 2024 at 05:36:07PM GMT, Laurent Pinchart wrote:
> > On Fri, Jun 21, 2024 at 04:54:04PM +0200, Jacopo Mondi wrote:
> > > Implement support in rkisp1-params for the extensible configuration
> > > parameters format.
> > >
> > > Create a list of handlers for each ISP block that wraps the existing
> > > configuration functions and handles the ISP block enablement.
> > >
> > > Parse the configuration parameters buffer in rkisp1_ext_params_config
> > > and filter the enable blocks by group, to allow setting the 'other'
> > > groups separately from the 'lsc' group to support the pre/post-configure
> > > operations.
> > >
> > > Implement the vb2 buf_out_validate() operation to validate the
> > > extensible format buffer content at qbuf time.
> >
> > Is there a particular reason to do so in .buf_out_validate() instead of
> > .buf_prepare() ?
> >
>
> It felt like it's the right place where to perform validation
>
> * @buf_out_validate: called when the output buffer is prepared or queued
> * to a request; drivers can use this to validate
> * userspace-provided information; this is required only
> * for OUTPUT queues.
>
> out buffers are validated -before- buf_prepare() is called. I admit it
> might have no practical differences though. Would you prefer to
> collapse everything into _prepare ? If I drop
>
> vb2_set_plane_payload(vb, 0, params->metafmt->buffersize);
>
> Not much will remain in _prepare() and I could even drop it.
I think I'd collapse it, yes. .buf_out_validate() seems to have been
introduced in codecs and I'm not sure how well-defined its semantics
are. The documentation also states "to a request".
> > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > > ---
> > > .../platform/rockchip/rkisp1/rkisp1-common.h | 3 +
> > > .../platform/rockchip/rkisp1/rkisp1-params.c | 553 +++++++++++++++++-
> > > 2 files changed, 540 insertions(+), 16 deletions(-)
> > >
> > > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> > > index ccd2065351b4..bffd936f989a 100644
> > > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> > > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> > > @@ -390,6 +390,7 @@ struct rkisp1_params_ops {
> > > * @quantization: the quantization configured on the isp's src pad
> > > * @ycbcr_encoding the YCbCr encoding
> > > * @raw_type: the bayer pattern on the isp video sink pad
> > > + * @enabled_blocks: bitmask of enabled ISP blocks
> > > */
> > > struct rkisp1_params {
> > > struct rkisp1_vdev_node vnode;
> > > @@ -404,6 +405,8 @@ struct rkisp1_params {
> > > enum v4l2_quantization quantization;
> > > enum v4l2_ycbcr_encoding ycbcr_encoding;
> > > enum rkisp1_fmt_raw_pat_type raw_type;
> > > +
> > > + u32 enabled_blocks;
> > > };
> > >
> > > /*
> > > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
> > > index e38d2da994f5..f3ea70c7e0c1 100644
> > > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
> > > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
> > > @@ -35,6 +35,9 @@
> > > #define RKISP1_ISP_CC_COEFF(n) \
> > > (RKISP1_CIF_ISP_CC_COEFF_0 + (n) * 4)
> > >
> > > +#define RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS BIT(0)
> > > +#define RKISP1_EXT_PARAMS_BLOCK_GROUP_LSC BIT(1)
> > > +
> > > enum rkisp1_params_formats {
> > > RKISP1_PARAMS_FIXED,
> > > RKISP1_PARAMS_EXTENSIBLE,
> > > @@ -1529,6 +1532,454 @@ rkisp1_params_get_buffer(struct rkisp1_params *params)
> > > queue);
> > > }
> > >
> > > +/*------------------------------------------------------------------------------
> > > + * Extensible parameters format handling
> > > + */
> > > +
> > > +static void rkisp1_ext_params_bls(struct rkisp1_params *params,
> > > + struct rkisp1_ext_params_block_header *hdr)
> > > +{
> > > + struct rkisp1_ext_params_bls_config *bls =
> > > + (struct rkisp1_ext_params_bls_config *)hdr;
> > > +
> > > + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> > > + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_BLS_CTRL,
> > > + RKISP1_CIF_ISP_BLS_ENA);
> > > + return;
> > > + }
> > > +
> > > + rkisp1_bls_config(params, &bls->bls_config);
> > > +
> > > + if (!(params->enabled_blocks & BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_BLS)))
> > > + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_BLS_CTRL,
> > > + RKISP1_CIF_ISP_BLS_ENA);
> > > +}
> > > +
> > > +static void rkisp1_ext_params_dpcc(struct rkisp1_params *params,
> > > + struct rkisp1_ext_params_block_header *hdr)
> > > +{
> > > + struct rkisp1_ext_params_dpcc_config *dpcc =
> > > + (struct rkisp1_ext_params_dpcc_config *)hdr;
> > > +
> > > + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> > > + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_DPCC_MODE,
> > > + RKISP1_CIF_ISP_DPCC_MODE_DPCC_ENABLE);
> > > + return;
> > > + }
> > > +
> > > + rkisp1_dpcc_config(params, &dpcc->dpcc_config);
> > > +
> > > + if (!(params->enabled_blocks &
> > > + BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_DPCC)))
> > > + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_DPCC_MODE,
> > > + RKISP1_CIF_ISP_DPCC_MODE_DPCC_ENABLE);
> > > +}
> > > +
> > > +static void rkisp1_ext_params_sdg(struct rkisp1_params *params,
> > > + struct rkisp1_ext_params_block_header *hdr)
> > > +{
> > > + struct rkisp1_ext_params_sdg_config *sdg =
> > > + (struct rkisp1_ext_params_sdg_config *)hdr;
> > > +
> > > + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> > > + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_CTRL,
> > > + RKISP1_CIF_ISP_CTRL_ISP_GAMMA_IN_ENA);
> > > + return;
> > > + }
> > > +
> > > + rkisp1_sdg_config(params, &sdg->sdg_config);
> > > +
> > > + if (!(params->enabled_blocks & BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_SDG)))
> > > + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CTRL,
> > > + RKISP1_CIF_ISP_CTRL_ISP_GAMMA_IN_ENA);
> > > +}
> > > +
> > > +static void rkisp1_ext_params_lsc(struct rkisp1_params *params,
> > > + struct rkisp1_ext_params_block_header *hdr)
> > > +{
> > > + struct rkisp1_ext_params_lsc_config *lsc =
> > > + (struct rkisp1_ext_params_lsc_config *)hdr;
> > > +
> > > + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> > > + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_LSC_CTRL,
> > > + RKISP1_CIF_ISP_LSC_CTRL_ENA);
> > > + return;
> > > + }
> > > +
> > > + rkisp1_lsc_config(params, &lsc->lsc_config);
> > > +
> > > + if (!(params->enabled_blocks & BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_LSC)))
> > > + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_LSC_CTRL,
> > > + RKISP1_CIF_ISP_LSC_CTRL_ENA);
> > > +}
> > > +
> > > +static void rkisp1_ext_params_awbg(struct rkisp1_params *params,
> > > + struct rkisp1_ext_params_block_header *hdr)
> > > +{
> > > + struct rkisp1_ext_params_awb_gain_config *awbg =
> > > + (struct rkisp1_ext_params_awb_gain_config *)hdr;
> > > +
> > > + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> > > + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_CTRL,
> > > + RKISP1_CIF_ISP_CTRL_ISP_AWB_ENA);
> > > + return;
> > > + }
> > > +
> > > + params->ops->awb_gain_config(params, &awbg->awb_config);
> > > +
> > > + if (!(params->enabled_blocks &
> > > + BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_AWB_GAINS)))
> > > + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CTRL,
> > > + RKISP1_CIF_ISP_CTRL_ISP_AWB_ENA);
> > > +}
> > > +
> > > +static void rkisp1_ext_params_flt(struct rkisp1_params *params,
> > > + struct rkisp1_ext_params_block_header *hdr)
> > > +{
> > > + struct rkisp1_ext_params_flt_config *flt =
> > > + (struct rkisp1_ext_params_flt_config *)hdr;
> > > +
> > > + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> > > + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_FILT_MODE,
> > > + RKISP1_CIF_ISP_FLT_ENA);
> > > + return;
> > > + }
> > > +
> > > + rkisp1_flt_config(params, &flt->flt_config);
> > > +
> > > + if (!(params->enabled_blocks & BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_FLT)))
> > > + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_FILT_MODE,
> > > + RKISP1_CIF_ISP_FLT_ENA);
> > > +}
> > > +
> > > +static void rkisp1_ext_params_bdm(struct rkisp1_params *params,
> > > + struct rkisp1_ext_params_block_header *hdr)
> > > +{
> > > + struct rkisp1_ext_params_bdm_config *bdm =
> > > + (struct rkisp1_ext_params_bdm_config *)hdr;
> > > +
> > > + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> > > + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_DEMOSAIC,
> > > + RKISP1_CIF_ISP_DEMOSAIC_BYPASS);
> > > + return;
> > > + }
> > > +
> > > + rkisp1_bdm_config(params, &bdm->bdm_config);
> > > +
> > > + if (!(params->enabled_blocks & BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_BDM)))
> > > + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_DEMOSAIC,
> > > + RKISP1_CIF_ISP_DEMOSAIC_BYPASS);
> > > +}
> > > +
> > > +static void rkisp1_ext_params_ctk(struct rkisp1_params *params,
> > > + struct rkisp1_ext_params_block_header *hdr)
> > > +{
> > > + struct rkisp1_ext_params_ctk_config *ctk =
> > > + (struct rkisp1_ext_params_ctk_config *)hdr;
> > > +
> > > + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> > > + rkisp1_ctk_enable(params, false);
> > > + return;
> > > + }
> > > +
> > > + rkisp1_ctk_config(params, &ctk->ctk_config);
> > > +
> > > + if (!(params->enabled_blocks & BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_CTK)))
> > > + rkisp1_ctk_enable(params, true);
> > > +}
> > > +
> > > +static void rkisp1_ext_params_goc(struct rkisp1_params *params,
> > > + struct rkisp1_ext_params_block_header *hdr)
> > > +{
> > > + struct rkisp1_ext_params_goc_config *goc =
> > > + (struct rkisp1_ext_params_goc_config *)hdr;
> > > +
> > > + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> > > + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_CTRL,
> > > + RKISP1_CIF_ISP_CTRL_ISP_GAMMA_OUT_ENA);
> > > + return;
> > > + }
> > > +
> > > + params->ops->goc_config(params, &goc->goc_config);
> > > +
> > > + if (!(params->enabled_blocks & BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_GOC)))
> > > + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CTRL,
> > > + RKISP1_CIF_ISP_CTRL_ISP_GAMMA_OUT_ENA);
> > > +}
> > > +
> > > +static void rkisp1_ext_params_dpf(struct rkisp1_params *params,
> > > + struct rkisp1_ext_params_block_header *hdr)
> > > +{
> > > + struct rkisp1_ext_params_dpf_config *dpf =
> > > + (struct rkisp1_ext_params_dpf_config *)hdr;
> > > +
> > > + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> > > + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_DPF_MODE,
> > > + RKISP1_CIF_ISP_DPF_MODE_EN);
> > > + return;
> > > + }
> > > +
> > > + rkisp1_dpf_config(params, &dpf->dpf_config);
> > > +
> > > + if (!(params->enabled_blocks & BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_DPF)))
> > > + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_DPF_MODE,
> > > + RKISP1_CIF_ISP_DPF_MODE_EN);
> > > +}
> > > +
> > > +static void rkisp1_ext_params_dpfs(struct rkisp1_params *params,
> > > + struct rkisp1_ext_params_block_header *hdr)
> > > +{
> > > + struct rkisp1_ext_params_dpf_strength_config *dpfs =
> > > + (struct rkisp1_ext_params_dpf_strength_config *)hdr;
> > > +
> > > + rkisp1_dpf_strength_config(params, &dpfs->dpf_strength_config);
> > > +}
> > > +
> > > +static void rkisp1_ext_params_cproc(struct rkisp1_params *params,
> > > + struct rkisp1_ext_params_block_header *hdr)
> > > +{
> > > + struct rkisp1_ext_params_cproc_config *cproc =
> > > + (struct rkisp1_ext_params_cproc_config *)hdr;
> > > +
> > > + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> > > + rkisp1_param_clear_bits(params, RKISP1_CIF_C_PROC_CTRL,
> > > + RKISP1_CIF_C_PROC_CTR_ENABLE);
> > > + return;
> > > + }
> > > +
> > > + rkisp1_cproc_config(params, &cproc->cproc_config);
> > > +
> > > + if (!(params->enabled_blocks &
> > > + BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_CPROC)))
> > > + rkisp1_param_set_bits(params, RKISP1_CIF_C_PROC_CTRL,
> > > + RKISP1_CIF_C_PROC_CTR_ENABLE);
> > > +}
> > > +
> > > +static void rkisp1_ext_params_ie(struct rkisp1_params *params,
> > > + struct rkisp1_ext_params_block_header *hdr)
> > > +{
> > > + struct rkisp1_ext_params_ie_config *ie =
> > > + (struct rkisp1_ext_params_ie_config *)hdr;
> > > +
> > > + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> > > + rkisp1_ie_enable(params, false);
> > > + return;
> > > + }
> > > +
> > > + rkisp1_ie_config(params, &ie->ie_config);
> > > +
> > > + if (!(params->enabled_blocks & BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_IE)))
> > > + rkisp1_ie_enable(params, true);
> > > +}
> > > +
> > > +static void rkisp1_ext_params_awbm(struct rkisp1_params *params,
> > > + struct rkisp1_ext_params_block_header *hdr)
> > > +{
> > > + struct rkisp1_ext_params_awb_meas_config *awbm =
> > > + (struct rkisp1_ext_params_awb_meas_config *)hdr;
> > > +
> > > + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> > > + params->ops->awb_meas_enable(params, &awbm->awb_meas_config,
> > > + false);
> > > + return;
> > > + }
> > > +
> > > + params->ops->awb_meas_config(params, &awbm->awb_meas_config);
> > > +
> > > + if (!(params->enabled_blocks &
> > > + BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_AWB_MEAS)))
> > > + params->ops->awb_meas_enable(params, &awbm->awb_meas_config,
> > > + true);
> > > +}
> > > +
> > > +static void rkisp1_ext_params_hstm(struct rkisp1_params *params,
> > > + struct rkisp1_ext_params_block_header *hdr)
> > > +{
> > > + struct rkisp1_ext_params_hst_config *hst =
> > > + (struct rkisp1_ext_params_hst_config *)hdr;
> > > +
> > > + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> > > + params->ops->hst_enable(params, &hst->hst_config, false);
> > > + return;
> > > + }
> > > +
> > > + params->ops->hst_config(params, &hst->hst_config);
> > > +
> > > + if (!(params->enabled_blocks &
> > > + BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_HST_MEAS)))
> > > + params->ops->hst_enable(params, &hst->hst_config, true);
> > > +}
> > > +
> > > +static void rkisp1_ext_params_aecm(struct rkisp1_params *params,
> > > + struct rkisp1_ext_params_block_header *hdr)
> > > +{
> > > + struct rkisp1_ext_params_aec_config *aec =
> > > + (struct rkisp1_ext_params_aec_config *)hdr;
> > > +
> > > + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> > > + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_EXP_CTRL,
> > > + RKISP1_CIF_ISP_EXP_ENA);
> > > + return;
> > > + }
> > > +
> > > + params->ops->aec_config(params, &aec->aec_config);
> > > +
> > > + if (!(params->enabled_blocks &
> > > + BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_AEC_MEAS)))
> > > + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_EXP_CTRL,
> > > + RKISP1_CIF_ISP_EXP_ENA);
> > > +}
> > > +
> > > +static void rkisp1_ext_params_afcm(struct rkisp1_params *params,
> > > + struct rkisp1_ext_params_block_header *hdr)
> > > +{
> > > + struct rkisp1_ext_params_afc_config *afc =
> > > + (struct rkisp1_ext_params_afc_config *)hdr;
> > > +
> > > + if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
> > > + rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_AFM_CTRL,
> > > + RKISP1_CIF_ISP_AFM_ENA);
> > > + return;
> > > + }
> > > +
> > > + params->ops->afm_config(params, &afc->afc_config);
> > > +
> > > + if (!(params->enabled_blocks &
> > > + BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_AFC_MEAS)))
> > > + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_AFM_CTRL,
> > > + RKISP1_CIF_ISP_AFM_ENA);
> > > +}
> >
> > I still think we could get rid of most of these functions by adding
> > .enable(), .configure() and .disable() operations to
> > rkisp1_ext_params_handler. That can be done later.
> >
> > > +
> > > +typedef void (*rkisp1_block_handler)(struct rkisp1_params *params,
> > > + struct rkisp1_ext_params_block_header *hdr);
> >
> > If it doesn't cause any issue, I'd make the hdr const.
> >
> > > +
> > > +static const struct rkisp1_ext_params_handler {
> > > + size_t size;
> > > + rkisp1_block_handler handler;
> > > + unsigned int group;
> > > +} rkisp1_ext_params_handlers[] = {
> > > + [RKISP1_EXT_PARAMS_BLOCK_TYPE_BLS] = {
> > > + .size = sizeof(struct rkisp1_ext_params_bls_config),
> > > + .handler = rkisp1_ext_params_bls,
> > > + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> > > + },
> > > + [RKISP1_EXT_PARAMS_BLOCK_TYPE_DPCC] = {
> > > + .size = sizeof(struct rkisp1_ext_params_dpcc_config),
> > > + .handler = rkisp1_ext_params_dpcc,
> > > + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> > > + },
> > > + [RKISP1_EXT_PARAMS_BLOCK_TYPE_SDG] = {
> > > + .size = sizeof(struct rkisp1_ext_params_sdg_config),
> > > + .handler = rkisp1_ext_params_sdg,
> > > + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> > > + },
> > > + [RKISP1_EXT_PARAMS_BLOCK_TYPE_AWB_GAINS] = {
> > > + .size =
> > > + sizeof(struct rkisp1_ext_params_awb_gain_config),
> > > + .handler = rkisp1_ext_params_awbg,
> > > + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> > > + },
> > > + [RKISP1_EXT_PARAMS_BLOCK_TYPE_FLT] = {
> > > + .size = sizeof(struct rkisp1_ext_params_flt_config),
> > > + .handler = rkisp1_ext_params_flt,
> > > + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> > > + },
> > > + [RKISP1_EXT_PARAMS_BLOCK_TYPE_BDM] = {
> > > + .size = sizeof(struct rkisp1_ext_params_bdm_config),
> > > + .handler = rkisp1_ext_params_bdm,
> > > + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> > > + },
> > > + [RKISP1_EXT_PARAMS_BLOCK_TYPE_CTK] = {
> > > + .size = sizeof(struct rkisp1_ext_params_ctk_config),
> > > + .handler = rkisp1_ext_params_ctk,
> > > + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> > > + },
> > > + [RKISP1_EXT_PARAMS_BLOCK_TYPE_GOC] = {
> > > + .size = sizeof(struct rkisp1_ext_params_goc_config),
> > > + .handler = rkisp1_ext_params_goc,
> > > + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> > > + },
> > > + [RKISP1_EXT_PARAMS_BLOCK_TYPE_DPF] = {
> > > + .size = sizeof(struct rkisp1_ext_params_dpf_config),
> > > + .handler = rkisp1_ext_params_dpf,
> > > + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> > > + },
> > > + [RKISP1_EXT_PARAMS_BLOCK_TYPE_DPF_STRENGTH] = {
> > > + .size =
> > > + sizeof(struct rkisp1_ext_params_dpf_strength_config),
> > > + .handler = rkisp1_ext_params_dpfs,
> > > + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> > > + },
> > > + [RKISP1_EXT_PARAMS_BLOCK_TYPE_CPROC] = {
> > > + .size = sizeof(struct rkisp1_ext_params_cproc_config),
> > > + .handler = rkisp1_ext_params_cproc,
> > > + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> > > + },
> > > + [RKISP1_EXT_PARAMS_BLOCK_TYPE_IE] = {
> > > + .size = sizeof(struct rkisp1_ext_params_ie_config),
> > > + .handler = rkisp1_ext_params_ie,
> > > + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> > > + },
> > > + [RKISP1_EXT_PARAMS_BLOCK_TYPE_LSC] = {
> > > + .size = sizeof(struct rkisp1_ext_params_lsc_config),
> > > + .handler = rkisp1_ext_params_lsc,
> > > + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_LSC
> > > + },
> > > + [RKISP1_EXT_PARAMS_BLOCK_TYPE_AWB_MEAS] = {
> > > + .size =
> > > + sizeof(struct rkisp1_ext_params_awb_meas_config),
> > > + .handler = rkisp1_ext_params_awbm,
> > > + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> > > + },
> > > + [RKISP1_EXT_PARAMS_BLOCK_TYPE_HST_MEAS] = {
> > > + .size = sizeof(struct rkisp1_ext_params_hst_config),
> > > + .handler = rkisp1_ext_params_hstm,
> > > + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> > > + },
> > > + [RKISP1_EXT_PARAMS_BLOCK_TYPE_AEC_MEAS] = {
> > > + .size = sizeof(struct rkisp1_ext_params_aec_config),
> > > + .handler = rkisp1_ext_params_aecm,
> > > + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> > > + },
> > > + [RKISP1_EXT_PARAMS_BLOCK_TYPE_AFC_MEAS] = {
> > > + .size = sizeof(struct rkisp1_ext_params_afc_config),
> > > + .handler = rkisp1_ext_params_afcm,
> > > + .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
> > > + },
> > > +};
> > > +
> > > +static void rkisp1_ext_params_config(struct rkisp1_params *params,
> > > + struct rkisp1_ext_params_cfg *cfg,
> > > + u32 block_group_mask)
> > > +{
> > > + size_t block_offset = 0;
> > > +
> > > + if (WARN_ON(!cfg))
> > > + return;
> > > +
> > > + /* Walk the list of parameter blocks and process them. */
> > > + while (block_offset < cfg->total_size) {
> > > + const struct rkisp1_ext_params_handler *block_handler;
> > > + struct rkisp1_ext_params_block_header *block;
> >
> > const
> >
> > > +
> > > + block = (struct rkisp1_ext_params_block_header *)
> > > + &cfg->data[block_offset];
> > > + block_offset += block->size;
> > > +
> > > + /* Make sure the block is in the list of groups to configure. */
> > > + block_handler = &rkisp1_ext_params_handlers[block->type];
> > > + if (!(block_handler->group & block_group_mask))
> > > + continue;
> > > +
> > > + block_handler->handler(params, block);
> > > +
> > > + if (block->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE)
> > > + params->enabled_blocks &= ~BIT(block->type);
> > > + else
> > > + params->enabled_blocks |= BIT(block->type);
> > > + }
> > > +}
> > > +
> > > static void rkisp1_params_complete_buffer(struct rkisp1_params *params,
> > > struct rkisp1_params_buffer *buf,
> > > unsigned int frame_sequence)
> > > @@ -1550,9 +2001,15 @@ void rkisp1_params_isr(struct rkisp1_device *rkisp1)
> > > if (!cur_buf)
> > > goto unlock;
> > >
> > > - rkisp1_isp_isr_other_config(params, cur_buf->cfg);
> > > - rkisp1_isp_isr_lsc_config(params, cur_buf->cfg);
> > > - rkisp1_isp_isr_meas_config(params, cur_buf->cfg);
> > > + if (params->metafmt->dataformat == V4L2_META_FMT_RK_ISP1_PARAMS) {
> > > + rkisp1_isp_isr_other_config(params, cur_buf->cfg);
> > > + rkisp1_isp_isr_lsc_config(params, cur_buf->cfg);
> > > + rkisp1_isp_isr_meas_config(params, cur_buf->cfg);
> > > + } else {
> > > + rkisp1_ext_params_config(params, cur_buf->cfg,
> > > + RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS |
> > > + RKISP1_EXT_PARAMS_BLOCK_GROUP_LSC);
> > > + }
> > >
> > > /* update shadow register immediately */
> > > rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CTRL,
> > > @@ -1651,8 +2108,13 @@ void rkisp1_params_pre_configure(struct rkisp1_params *params,
> > > if (!cur_buf)
> > > goto unlock;
> > >
> > > - rkisp1_isp_isr_other_config(params, cur_buf->cfg);
> > > - rkisp1_isp_isr_meas_config(params, cur_buf->cfg);
> > > + if (params->metafmt->dataformat == V4L2_META_FMT_RK_ISP1_PARAMS) {
> > > + rkisp1_isp_isr_other_config(params, cur_buf->cfg);
> > > + rkisp1_isp_isr_meas_config(params, cur_buf->cfg);
> > > + } else {
> > > + rkisp1_ext_params_config(params, cur_buf->cfg,
> > > + RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS);
> > > + }
> > >
> > > /* update shadow register immediately */
> > > rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CTRL,
> > > @@ -1680,7 +2142,11 @@ void rkisp1_params_post_configure(struct rkisp1_params *params)
> > > if (!cur_buf)
> > > goto unlock;
> > >
> > > - rkisp1_isp_isr_lsc_config(params, cur_buf->cfg);
> > > + if (params->metafmt->dataformat == V4L2_META_FMT_RK_ISP1_PARAMS)
> > > + rkisp1_isp_isr_lsc_config(params, cur_buf->cfg);
> > > + else
> > > + rkisp1_ext_params_config(params, cur_buf->cfg,
> > > + RKISP1_EXT_PARAMS_BLOCK_GROUP_LSC);
> > >
> > > /* update shadow register immediately */
> > > rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CTRL,
> > > @@ -1874,10 +2340,6 @@ static void rkisp1_params_vb2_buf_queue(struct vb2_buffer *vb)
> > >
> > > static int rkisp1_params_vb2_buf_prepare(struct vb2_buffer *vb)
> > > {
> > > - struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> > > - struct rkisp1_params_buffer *params_buf =
> > > - container_of(vbuf, struct rkisp1_params_buffer, vb);
> > > - void *cfg = vb2_plane_vaddr(¶ms_buf->vb.vb2_buf, 0);
> > > struct rkisp1_params *params = vb->vb2_queue->drv_priv;
> > >
> > > if (vb2_plane_size(vb, 0) < params->metafmt->buffersize)
> > > @@ -1885,12 +2347,6 @@ static int rkisp1_params_vb2_buf_prepare(struct vb2_buffer *vb)
> > >
> > > vb2_set_plane_payload(vb, 0, params->metafmt->buffersize);
> > >
> > > - /*
> > > - * Copy the parameters buffer to the internal scratch buffer to avoid
> > > - * userspace modifying the buffer content while the driver processes it.
> > > - */
> > > - memcpy(params_buf->cfg, cfg, params->metafmt->buffersize);
> > > -
> > > return 0;
> > > }
> > >
> > > @@ -1911,12 +2367,77 @@ static void rkisp1_params_vb2_stop_streaming(struct vb2_queue *vq)
> > >
> > > list_for_each_entry(buf, &tmp_list, queue)
> > > vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_ERROR);
> > > +
> > > + params->enabled_blocks = 0;
> > > +}
> > > +
> > > +static int rkisp1_params_validate_ext_params(struct rkisp1_params *params,
> > > + struct rkisp1_ext_params_cfg *cfg)
> > > +{
> > > + size_t block_offset = 0;
> > > +
> > > + if (cfg->total_size > RKISP1_EXT_PARAMS_MAX_SIZE) {
> > > + dev_dbg(params->rkisp1->dev,
> > > + "Invalid parameters buffer size %u\n",
> > > + cfg->total_size);
> > > + return -EINVAL;
> > > + }
> >
> > Following my review comments on patch 5/7, you should also validate that
> > offsetof(struct rkisp1_ext_params_cfg, data) + cfg->total_size >= plane payload.
> >
> > > +
> > > + /* Walk the list of parameter blocks and validate them. */
> > > + while (block_offset < cfg->total_size) {
> >
> > while (cfg->total_size - block_offset >=
> > sizeof(struct rkisp1_ext_params_block_header)) {
> >
> > Otherwise there could be just one byte left, and dereferencing block
> > below would read uninitialized memory.
> >
> > > + const struct rkisp1_ext_params_handler *hdlr;
> >
> > Maybe handler, it's not much longer, and is more readable.
> >
> > > + struct rkisp1_ext_params_block_header *block;
> >
> > const
> >
> > > +
> > > + block = (struct rkisp1_ext_params_block_header *)
> > > + &cfg->data[block_offset];
> > > +
> > > + if (block->type >= RKISP1_EXT_PARAMS_BLOCK_TYPE_SENTINEL) {
> >
> > if (block->type >= ARRAY_SIZE(rkisp1_ext_params_handlers)) {
> >
> > which allows you to drop RKISP1_EXT_PARAMS_BLOCK_TYPE_SENTINEL from the
> > UAPI header.
> >
> > > + dev_dbg(params->rkisp1->dev,
> > > + "Invalid parameters block type\n");
> > > + return -EINVAL;
> > > + }
> > > +
> > > + hdlr = &rkisp1_ext_params_handlers[block->type];
> > > + if (block->size != hdlr->size) {
> > > + dev_dbg(params->rkisp1->dev,
> > > + "Invalid parameters block size\n");
> > > + return -EINVAL;
> > > + }
> >
> > You need to test here that block->size >= cfg->total_size -
> > block_offset. It may be easier to add a new local variable at the top of
> > the function
> >
> > size_t remaining_size = cfg->total_size;
> >
> > The loop would then become
> >
> > while (remaining_size >= sizeof(struct rkisp1_ext_params_block_header)) {
> > const struct rkisp1_ext_params_block_header *block;
> > const struct rkisp1_ext_params_handler *handler;
> >
> > block = (struct rkisp1_ext_params_block_header *)
> > &cfg->data[block_offset];
> >
> > if (block->type >= ARRAY_SIZE(rkisp1_ext_params_handlers)) {
> > dev_dbg(params->rkisp1->dev,
> > "Invalid parameters block type\n");
> > return -EINVAL;
> > }
> >
> > if (block->size > remaining_size) {
> > dev_dbg(params->rkisp1->dev,
> > "Premature end of parameters data\n");
> > return -EINVAL;
> > }
> >
> > handler = &rkisp1_ext_params_handlers[block->type];
> > if (block->size != handler->size) {
> > dev_dbg(params->rkisp1->dev,
> > "Invalid parameters block size\n");
> > return -EINVAL;
> > }
> >
> > block_offset += block->size;
> > reamining_size -= block->size;
> > }
> >
> > > +
> > > + block_offset += block->size;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int rkisp1_params_vb2_buf_out_validate(struct vb2_buffer *vb)
> > > +{
> > > + struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> > > + struct rkisp1_params_buffer *params_buf =
> > > + container_of(vbuf, struct rkisp1_params_buffer, vb);
> > > + struct vb2_queue *vq = vb->vb2_queue;
> > > + struct rkisp1_params *params = vq->drv_priv;
> > > + struct rkisp1_ext_params_cfg *cfg =
> > > + vb2_plane_vaddr(¶ms_buf->vb.vb2_buf, 0);
> > > +
> > > + /*
> > > + * Copy the parameters buffer to the internal scratch buffer to avoid
> > > + * userspace modifying the buffer content while the driver processes it.
> > > + */
> > > + memcpy(params_buf->cfg, cfg, params->metafmt->buffersize);
> > > +
> > > + /* Fixed parameters format doesn't require validation. */
> > > + if (params->metafmt->dataformat == V4L2_META_FMT_RK_ISP1_PARAMS)
> > > + return 0;
> > > +
> > > + return rkisp1_params_validate_ext_params(params, cfg);
> > > }
> > >
> > > static const struct vb2_ops rkisp1_params_vb2_ops = {
> > > .queue_setup = rkisp1_params_vb2_queue_setup,
> > > .buf_init = rkisp1_params_vb2_buf_init,
> > > .buf_cleanup = rkisp1_params_vb2_buf_cleanup,
> > > + .buf_out_validate = rkisp1_params_vb2_buf_out_validate,
> > > .wait_prepare = vb2_ops_wait_prepare,
> > > .wait_finish = vb2_ops_wait_finish,
> > > .buf_queue = rkisp1_params_vb2_buf_queue,
@@ -390,6 +390,7 @@ struct rkisp1_params_ops {
* @quantization: the quantization configured on the isp's src pad
* @ycbcr_encoding the YCbCr encoding
* @raw_type: the bayer pattern on the isp video sink pad
+ * @enabled_blocks: bitmask of enabled ISP blocks
*/
struct rkisp1_params {
struct rkisp1_vdev_node vnode;
@@ -404,6 +405,8 @@ struct rkisp1_params {
enum v4l2_quantization quantization;
enum v4l2_ycbcr_encoding ycbcr_encoding;
enum rkisp1_fmt_raw_pat_type raw_type;
+
+ u32 enabled_blocks;
};
/*
@@ -35,6 +35,9 @@
#define RKISP1_ISP_CC_COEFF(n) \
(RKISP1_CIF_ISP_CC_COEFF_0 + (n) * 4)
+#define RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS BIT(0)
+#define RKISP1_EXT_PARAMS_BLOCK_GROUP_LSC BIT(1)
+
enum rkisp1_params_formats {
RKISP1_PARAMS_FIXED,
RKISP1_PARAMS_EXTENSIBLE,
@@ -1529,6 +1532,454 @@ rkisp1_params_get_buffer(struct rkisp1_params *params)
queue);
}
+/*------------------------------------------------------------------------------
+ * Extensible parameters format handling
+ */
+
+static void rkisp1_ext_params_bls(struct rkisp1_params *params,
+ struct rkisp1_ext_params_block_header *hdr)
+{
+ struct rkisp1_ext_params_bls_config *bls =
+ (struct rkisp1_ext_params_bls_config *)hdr;
+
+ if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
+ rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_BLS_CTRL,
+ RKISP1_CIF_ISP_BLS_ENA);
+ return;
+ }
+
+ rkisp1_bls_config(params, &bls->bls_config);
+
+ if (!(params->enabled_blocks & BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_BLS)))
+ rkisp1_param_set_bits(params, RKISP1_CIF_ISP_BLS_CTRL,
+ RKISP1_CIF_ISP_BLS_ENA);
+}
+
+static void rkisp1_ext_params_dpcc(struct rkisp1_params *params,
+ struct rkisp1_ext_params_block_header *hdr)
+{
+ struct rkisp1_ext_params_dpcc_config *dpcc =
+ (struct rkisp1_ext_params_dpcc_config *)hdr;
+
+ if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
+ rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_DPCC_MODE,
+ RKISP1_CIF_ISP_DPCC_MODE_DPCC_ENABLE);
+ return;
+ }
+
+ rkisp1_dpcc_config(params, &dpcc->dpcc_config);
+
+ if (!(params->enabled_blocks &
+ BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_DPCC)))
+ rkisp1_param_set_bits(params, RKISP1_CIF_ISP_DPCC_MODE,
+ RKISP1_CIF_ISP_DPCC_MODE_DPCC_ENABLE);
+}
+
+static void rkisp1_ext_params_sdg(struct rkisp1_params *params,
+ struct rkisp1_ext_params_block_header *hdr)
+{
+ struct rkisp1_ext_params_sdg_config *sdg =
+ (struct rkisp1_ext_params_sdg_config *)hdr;
+
+ if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
+ rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_CTRL,
+ RKISP1_CIF_ISP_CTRL_ISP_GAMMA_IN_ENA);
+ return;
+ }
+
+ rkisp1_sdg_config(params, &sdg->sdg_config);
+
+ if (!(params->enabled_blocks & BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_SDG)))
+ rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CTRL,
+ RKISP1_CIF_ISP_CTRL_ISP_GAMMA_IN_ENA);
+}
+
+static void rkisp1_ext_params_lsc(struct rkisp1_params *params,
+ struct rkisp1_ext_params_block_header *hdr)
+{
+ struct rkisp1_ext_params_lsc_config *lsc =
+ (struct rkisp1_ext_params_lsc_config *)hdr;
+
+ if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
+ rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_LSC_CTRL,
+ RKISP1_CIF_ISP_LSC_CTRL_ENA);
+ return;
+ }
+
+ rkisp1_lsc_config(params, &lsc->lsc_config);
+
+ if (!(params->enabled_blocks & BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_LSC)))
+ rkisp1_param_set_bits(params, RKISP1_CIF_ISP_LSC_CTRL,
+ RKISP1_CIF_ISP_LSC_CTRL_ENA);
+}
+
+static void rkisp1_ext_params_awbg(struct rkisp1_params *params,
+ struct rkisp1_ext_params_block_header *hdr)
+{
+ struct rkisp1_ext_params_awb_gain_config *awbg =
+ (struct rkisp1_ext_params_awb_gain_config *)hdr;
+
+ if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
+ rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_CTRL,
+ RKISP1_CIF_ISP_CTRL_ISP_AWB_ENA);
+ return;
+ }
+
+ params->ops->awb_gain_config(params, &awbg->awb_config);
+
+ if (!(params->enabled_blocks &
+ BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_AWB_GAINS)))
+ rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CTRL,
+ RKISP1_CIF_ISP_CTRL_ISP_AWB_ENA);
+}
+
+static void rkisp1_ext_params_flt(struct rkisp1_params *params,
+ struct rkisp1_ext_params_block_header *hdr)
+{
+ struct rkisp1_ext_params_flt_config *flt =
+ (struct rkisp1_ext_params_flt_config *)hdr;
+
+ if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
+ rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_FILT_MODE,
+ RKISP1_CIF_ISP_FLT_ENA);
+ return;
+ }
+
+ rkisp1_flt_config(params, &flt->flt_config);
+
+ if (!(params->enabled_blocks & BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_FLT)))
+ rkisp1_param_set_bits(params, RKISP1_CIF_ISP_FILT_MODE,
+ RKISP1_CIF_ISP_FLT_ENA);
+}
+
+static void rkisp1_ext_params_bdm(struct rkisp1_params *params,
+ struct rkisp1_ext_params_block_header *hdr)
+{
+ struct rkisp1_ext_params_bdm_config *bdm =
+ (struct rkisp1_ext_params_bdm_config *)hdr;
+
+ if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
+ rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_DEMOSAIC,
+ RKISP1_CIF_ISP_DEMOSAIC_BYPASS);
+ return;
+ }
+
+ rkisp1_bdm_config(params, &bdm->bdm_config);
+
+ if (!(params->enabled_blocks & BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_BDM)))
+ rkisp1_param_set_bits(params, RKISP1_CIF_ISP_DEMOSAIC,
+ RKISP1_CIF_ISP_DEMOSAIC_BYPASS);
+}
+
+static void rkisp1_ext_params_ctk(struct rkisp1_params *params,
+ struct rkisp1_ext_params_block_header *hdr)
+{
+ struct rkisp1_ext_params_ctk_config *ctk =
+ (struct rkisp1_ext_params_ctk_config *)hdr;
+
+ if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
+ rkisp1_ctk_enable(params, false);
+ return;
+ }
+
+ rkisp1_ctk_config(params, &ctk->ctk_config);
+
+ if (!(params->enabled_blocks & BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_CTK)))
+ rkisp1_ctk_enable(params, true);
+}
+
+static void rkisp1_ext_params_goc(struct rkisp1_params *params,
+ struct rkisp1_ext_params_block_header *hdr)
+{
+ struct rkisp1_ext_params_goc_config *goc =
+ (struct rkisp1_ext_params_goc_config *)hdr;
+
+ if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
+ rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_CTRL,
+ RKISP1_CIF_ISP_CTRL_ISP_GAMMA_OUT_ENA);
+ return;
+ }
+
+ params->ops->goc_config(params, &goc->goc_config);
+
+ if (!(params->enabled_blocks & BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_GOC)))
+ rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CTRL,
+ RKISP1_CIF_ISP_CTRL_ISP_GAMMA_OUT_ENA);
+}
+
+static void rkisp1_ext_params_dpf(struct rkisp1_params *params,
+ struct rkisp1_ext_params_block_header *hdr)
+{
+ struct rkisp1_ext_params_dpf_config *dpf =
+ (struct rkisp1_ext_params_dpf_config *)hdr;
+
+ if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
+ rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_DPF_MODE,
+ RKISP1_CIF_ISP_DPF_MODE_EN);
+ return;
+ }
+
+ rkisp1_dpf_config(params, &dpf->dpf_config);
+
+ if (!(params->enabled_blocks & BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_DPF)))
+ rkisp1_param_set_bits(params, RKISP1_CIF_ISP_DPF_MODE,
+ RKISP1_CIF_ISP_DPF_MODE_EN);
+}
+
+static void rkisp1_ext_params_dpfs(struct rkisp1_params *params,
+ struct rkisp1_ext_params_block_header *hdr)
+{
+ struct rkisp1_ext_params_dpf_strength_config *dpfs =
+ (struct rkisp1_ext_params_dpf_strength_config *)hdr;
+
+ rkisp1_dpf_strength_config(params, &dpfs->dpf_strength_config);
+}
+
+static void rkisp1_ext_params_cproc(struct rkisp1_params *params,
+ struct rkisp1_ext_params_block_header *hdr)
+{
+ struct rkisp1_ext_params_cproc_config *cproc =
+ (struct rkisp1_ext_params_cproc_config *)hdr;
+
+ if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
+ rkisp1_param_clear_bits(params, RKISP1_CIF_C_PROC_CTRL,
+ RKISP1_CIF_C_PROC_CTR_ENABLE);
+ return;
+ }
+
+ rkisp1_cproc_config(params, &cproc->cproc_config);
+
+ if (!(params->enabled_blocks &
+ BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_CPROC)))
+ rkisp1_param_set_bits(params, RKISP1_CIF_C_PROC_CTRL,
+ RKISP1_CIF_C_PROC_CTR_ENABLE);
+}
+
+static void rkisp1_ext_params_ie(struct rkisp1_params *params,
+ struct rkisp1_ext_params_block_header *hdr)
+{
+ struct rkisp1_ext_params_ie_config *ie =
+ (struct rkisp1_ext_params_ie_config *)hdr;
+
+ if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
+ rkisp1_ie_enable(params, false);
+ return;
+ }
+
+ rkisp1_ie_config(params, &ie->ie_config);
+
+ if (!(params->enabled_blocks & BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_IE)))
+ rkisp1_ie_enable(params, true);
+}
+
+static void rkisp1_ext_params_awbm(struct rkisp1_params *params,
+ struct rkisp1_ext_params_block_header *hdr)
+{
+ struct rkisp1_ext_params_awb_meas_config *awbm =
+ (struct rkisp1_ext_params_awb_meas_config *)hdr;
+
+ if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
+ params->ops->awb_meas_enable(params, &awbm->awb_meas_config,
+ false);
+ return;
+ }
+
+ params->ops->awb_meas_config(params, &awbm->awb_meas_config);
+
+ if (!(params->enabled_blocks &
+ BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_AWB_MEAS)))
+ params->ops->awb_meas_enable(params, &awbm->awb_meas_config,
+ true);
+}
+
+static void rkisp1_ext_params_hstm(struct rkisp1_params *params,
+ struct rkisp1_ext_params_block_header *hdr)
+{
+ struct rkisp1_ext_params_hst_config *hst =
+ (struct rkisp1_ext_params_hst_config *)hdr;
+
+ if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
+ params->ops->hst_enable(params, &hst->hst_config, false);
+ return;
+ }
+
+ params->ops->hst_config(params, &hst->hst_config);
+
+ if (!(params->enabled_blocks &
+ BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_HST_MEAS)))
+ params->ops->hst_enable(params, &hst->hst_config, true);
+}
+
+static void rkisp1_ext_params_aecm(struct rkisp1_params *params,
+ struct rkisp1_ext_params_block_header *hdr)
+{
+ struct rkisp1_ext_params_aec_config *aec =
+ (struct rkisp1_ext_params_aec_config *)hdr;
+
+ if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
+ rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_EXP_CTRL,
+ RKISP1_CIF_ISP_EXP_ENA);
+ return;
+ }
+
+ params->ops->aec_config(params, &aec->aec_config);
+
+ if (!(params->enabled_blocks &
+ BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_AEC_MEAS)))
+ rkisp1_param_set_bits(params, RKISP1_CIF_ISP_EXP_CTRL,
+ RKISP1_CIF_ISP_EXP_ENA);
+}
+
+static void rkisp1_ext_params_afcm(struct rkisp1_params *params,
+ struct rkisp1_ext_params_block_header *hdr)
+{
+ struct rkisp1_ext_params_afc_config *afc =
+ (struct rkisp1_ext_params_afc_config *)hdr;
+
+ if (hdr->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE) {
+ rkisp1_param_clear_bits(params, RKISP1_CIF_ISP_AFM_CTRL,
+ RKISP1_CIF_ISP_AFM_ENA);
+ return;
+ }
+
+ params->ops->afm_config(params, &afc->afc_config);
+
+ if (!(params->enabled_blocks &
+ BIT(RKISP1_EXT_PARAMS_BLOCK_TYPE_AFC_MEAS)))
+ rkisp1_param_set_bits(params, RKISP1_CIF_ISP_AFM_CTRL,
+ RKISP1_CIF_ISP_AFM_ENA);
+}
+
+typedef void (*rkisp1_block_handler)(struct rkisp1_params *params,
+ struct rkisp1_ext_params_block_header *hdr);
+
+static const struct rkisp1_ext_params_handler {
+ size_t size;
+ rkisp1_block_handler handler;
+ unsigned int group;
+} rkisp1_ext_params_handlers[] = {
+ [RKISP1_EXT_PARAMS_BLOCK_TYPE_BLS] = {
+ .size = sizeof(struct rkisp1_ext_params_bls_config),
+ .handler = rkisp1_ext_params_bls,
+ .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
+ },
+ [RKISP1_EXT_PARAMS_BLOCK_TYPE_DPCC] = {
+ .size = sizeof(struct rkisp1_ext_params_dpcc_config),
+ .handler = rkisp1_ext_params_dpcc,
+ .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
+ },
+ [RKISP1_EXT_PARAMS_BLOCK_TYPE_SDG] = {
+ .size = sizeof(struct rkisp1_ext_params_sdg_config),
+ .handler = rkisp1_ext_params_sdg,
+ .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
+ },
+ [RKISP1_EXT_PARAMS_BLOCK_TYPE_AWB_GAINS] = {
+ .size =
+ sizeof(struct rkisp1_ext_params_awb_gain_config),
+ .handler = rkisp1_ext_params_awbg,
+ .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
+ },
+ [RKISP1_EXT_PARAMS_BLOCK_TYPE_FLT] = {
+ .size = sizeof(struct rkisp1_ext_params_flt_config),
+ .handler = rkisp1_ext_params_flt,
+ .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
+ },
+ [RKISP1_EXT_PARAMS_BLOCK_TYPE_BDM] = {
+ .size = sizeof(struct rkisp1_ext_params_bdm_config),
+ .handler = rkisp1_ext_params_bdm,
+ .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
+ },
+ [RKISP1_EXT_PARAMS_BLOCK_TYPE_CTK] = {
+ .size = sizeof(struct rkisp1_ext_params_ctk_config),
+ .handler = rkisp1_ext_params_ctk,
+ .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
+ },
+ [RKISP1_EXT_PARAMS_BLOCK_TYPE_GOC] = {
+ .size = sizeof(struct rkisp1_ext_params_goc_config),
+ .handler = rkisp1_ext_params_goc,
+ .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
+ },
+ [RKISP1_EXT_PARAMS_BLOCK_TYPE_DPF] = {
+ .size = sizeof(struct rkisp1_ext_params_dpf_config),
+ .handler = rkisp1_ext_params_dpf,
+ .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
+ },
+ [RKISP1_EXT_PARAMS_BLOCK_TYPE_DPF_STRENGTH] = {
+ .size =
+ sizeof(struct rkisp1_ext_params_dpf_strength_config),
+ .handler = rkisp1_ext_params_dpfs,
+ .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
+ },
+ [RKISP1_EXT_PARAMS_BLOCK_TYPE_CPROC] = {
+ .size = sizeof(struct rkisp1_ext_params_cproc_config),
+ .handler = rkisp1_ext_params_cproc,
+ .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
+ },
+ [RKISP1_EXT_PARAMS_BLOCK_TYPE_IE] = {
+ .size = sizeof(struct rkisp1_ext_params_ie_config),
+ .handler = rkisp1_ext_params_ie,
+ .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
+ },
+ [RKISP1_EXT_PARAMS_BLOCK_TYPE_LSC] = {
+ .size = sizeof(struct rkisp1_ext_params_lsc_config),
+ .handler = rkisp1_ext_params_lsc,
+ .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_LSC
+ },
+ [RKISP1_EXT_PARAMS_BLOCK_TYPE_AWB_MEAS] = {
+ .size =
+ sizeof(struct rkisp1_ext_params_awb_meas_config),
+ .handler = rkisp1_ext_params_awbm,
+ .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
+ },
+ [RKISP1_EXT_PARAMS_BLOCK_TYPE_HST_MEAS] = {
+ .size = sizeof(struct rkisp1_ext_params_hst_config),
+ .handler = rkisp1_ext_params_hstm,
+ .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
+ },
+ [RKISP1_EXT_PARAMS_BLOCK_TYPE_AEC_MEAS] = {
+ .size = sizeof(struct rkisp1_ext_params_aec_config),
+ .handler = rkisp1_ext_params_aecm,
+ .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
+ },
+ [RKISP1_EXT_PARAMS_BLOCK_TYPE_AFC_MEAS] = {
+ .size = sizeof(struct rkisp1_ext_params_afc_config),
+ .handler = rkisp1_ext_params_afcm,
+ .group = RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS
+ },
+};
+
+static void rkisp1_ext_params_config(struct rkisp1_params *params,
+ struct rkisp1_ext_params_cfg *cfg,
+ u32 block_group_mask)
+{
+ size_t block_offset = 0;
+
+ if (WARN_ON(!cfg))
+ return;
+
+ /* Walk the list of parameter blocks and process them. */
+ while (block_offset < cfg->total_size) {
+ const struct rkisp1_ext_params_handler *block_handler;
+ struct rkisp1_ext_params_block_header *block;
+
+ block = (struct rkisp1_ext_params_block_header *)
+ &cfg->data[block_offset];
+ block_offset += block->size;
+
+ /* Make sure the block is in the list of groups to configure. */
+ block_handler = &rkisp1_ext_params_handlers[block->type];
+ if (!(block_handler->group & block_group_mask))
+ continue;
+
+ block_handler->handler(params, block);
+
+ if (block->enable == RKISP1_EXT_PARAMS_BLOCK_DISABLE)
+ params->enabled_blocks &= ~BIT(block->type);
+ else
+ params->enabled_blocks |= BIT(block->type);
+ }
+}
+
static void rkisp1_params_complete_buffer(struct rkisp1_params *params,
struct rkisp1_params_buffer *buf,
unsigned int frame_sequence)
@@ -1550,9 +2001,15 @@ void rkisp1_params_isr(struct rkisp1_device *rkisp1)
if (!cur_buf)
goto unlock;
- rkisp1_isp_isr_other_config(params, cur_buf->cfg);
- rkisp1_isp_isr_lsc_config(params, cur_buf->cfg);
- rkisp1_isp_isr_meas_config(params, cur_buf->cfg);
+ if (params->metafmt->dataformat == V4L2_META_FMT_RK_ISP1_PARAMS) {
+ rkisp1_isp_isr_other_config(params, cur_buf->cfg);
+ rkisp1_isp_isr_lsc_config(params, cur_buf->cfg);
+ rkisp1_isp_isr_meas_config(params, cur_buf->cfg);
+ } else {
+ rkisp1_ext_params_config(params, cur_buf->cfg,
+ RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS |
+ RKISP1_EXT_PARAMS_BLOCK_GROUP_LSC);
+ }
/* update shadow register immediately */
rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CTRL,
@@ -1651,8 +2108,13 @@ void rkisp1_params_pre_configure(struct rkisp1_params *params,
if (!cur_buf)
goto unlock;
- rkisp1_isp_isr_other_config(params, cur_buf->cfg);
- rkisp1_isp_isr_meas_config(params, cur_buf->cfg);
+ if (params->metafmt->dataformat == V4L2_META_FMT_RK_ISP1_PARAMS) {
+ rkisp1_isp_isr_other_config(params, cur_buf->cfg);
+ rkisp1_isp_isr_meas_config(params, cur_buf->cfg);
+ } else {
+ rkisp1_ext_params_config(params, cur_buf->cfg,
+ RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS);
+ }
/* update shadow register immediately */
rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CTRL,
@@ -1680,7 +2142,11 @@ void rkisp1_params_post_configure(struct rkisp1_params *params)
if (!cur_buf)
goto unlock;
- rkisp1_isp_isr_lsc_config(params, cur_buf->cfg);
+ if (params->metafmt->dataformat == V4L2_META_FMT_RK_ISP1_PARAMS)
+ rkisp1_isp_isr_lsc_config(params, cur_buf->cfg);
+ else
+ rkisp1_ext_params_config(params, cur_buf->cfg,
+ RKISP1_EXT_PARAMS_BLOCK_GROUP_LSC);
/* update shadow register immediately */
rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CTRL,
@@ -1874,10 +2340,6 @@ static void rkisp1_params_vb2_buf_queue(struct vb2_buffer *vb)
static int rkisp1_params_vb2_buf_prepare(struct vb2_buffer *vb)
{
- struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
- struct rkisp1_params_buffer *params_buf =
- container_of(vbuf, struct rkisp1_params_buffer, vb);
- void *cfg = vb2_plane_vaddr(¶ms_buf->vb.vb2_buf, 0);
struct rkisp1_params *params = vb->vb2_queue->drv_priv;
if (vb2_plane_size(vb, 0) < params->metafmt->buffersize)
@@ -1885,12 +2347,6 @@ static int rkisp1_params_vb2_buf_prepare(struct vb2_buffer *vb)
vb2_set_plane_payload(vb, 0, params->metafmt->buffersize);
- /*
- * Copy the parameters buffer to the internal scratch buffer to avoid
- * userspace modifying the buffer content while the driver processes it.
- */
- memcpy(params_buf->cfg, cfg, params->metafmt->buffersize);
-
return 0;
}
@@ -1911,12 +2367,77 @@ static void rkisp1_params_vb2_stop_streaming(struct vb2_queue *vq)
list_for_each_entry(buf, &tmp_list, queue)
vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_ERROR);
+
+ params->enabled_blocks = 0;
+}
+
+static int rkisp1_params_validate_ext_params(struct rkisp1_params *params,
+ struct rkisp1_ext_params_cfg *cfg)
+{
+ size_t block_offset = 0;
+
+ if (cfg->total_size > RKISP1_EXT_PARAMS_MAX_SIZE) {
+ dev_dbg(params->rkisp1->dev,
+ "Invalid parameters buffer size %u\n",
+ cfg->total_size);
+ return -EINVAL;
+ }
+
+ /* Walk the list of parameter blocks and validate them. */
+ while (block_offset < cfg->total_size) {
+ const struct rkisp1_ext_params_handler *hdlr;
+ struct rkisp1_ext_params_block_header *block;
+
+ block = (struct rkisp1_ext_params_block_header *)
+ &cfg->data[block_offset];
+
+ if (block->type >= RKISP1_EXT_PARAMS_BLOCK_TYPE_SENTINEL) {
+ dev_dbg(params->rkisp1->dev,
+ "Invalid parameters block type\n");
+ return -EINVAL;
+ }
+
+ hdlr = &rkisp1_ext_params_handlers[block->type];
+ if (block->size != hdlr->size) {
+ dev_dbg(params->rkisp1->dev,
+ "Invalid parameters block size\n");
+ return -EINVAL;
+ }
+
+ block_offset += block->size;
+ }
+
+ return 0;
+}
+
+static int rkisp1_params_vb2_buf_out_validate(struct vb2_buffer *vb)
+{
+ struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
+ struct rkisp1_params_buffer *params_buf =
+ container_of(vbuf, struct rkisp1_params_buffer, vb);
+ struct vb2_queue *vq = vb->vb2_queue;
+ struct rkisp1_params *params = vq->drv_priv;
+ struct rkisp1_ext_params_cfg *cfg =
+ vb2_plane_vaddr(¶ms_buf->vb.vb2_buf, 0);
+
+ /*
+ * Copy the parameters buffer to the internal scratch buffer to avoid
+ * userspace modifying the buffer content while the driver processes it.
+ */
+ memcpy(params_buf->cfg, cfg, params->metafmt->buffersize);
+
+ /* Fixed parameters format doesn't require validation. */
+ if (params->metafmt->dataformat == V4L2_META_FMT_RK_ISP1_PARAMS)
+ return 0;
+
+ return rkisp1_params_validate_ext_params(params, cfg);
}
static const struct vb2_ops rkisp1_params_vb2_ops = {
.queue_setup = rkisp1_params_vb2_queue_setup,
.buf_init = rkisp1_params_vb2_buf_init,
.buf_cleanup = rkisp1_params_vb2_buf_cleanup,
+ .buf_out_validate = rkisp1_params_vb2_buf_out_validate,
.wait_prepare = vb2_ops_wait_prepare,
.wait_finish = vb2_ops_wait_finish,
.buf_queue = rkisp1_params_vb2_buf_queue,