[PATCH v1 4/5] media: rkisp1: Add feature flags for BLS and compand

[Jacopo Mondi]

Hi Laurent

On Thu, Jul 04, 2024 at 01:25:32AM GMT, Laurent Pinchart wrote:
> From: Paul Elder <paul.elder at ideasonboard.com>
>
> Add feature flags for the dedicated black level subtraction hardware
> block and for the compand hardware block. The companding feature flag is
> added on its own (as opposed to "the absence of BLS") because we will
> need it later for when we add support for the companding block.
>
> Skip BLS configuration when the BLS feature flag is unset, as devices
> without the dedicated BLS block cannot configure a hardware block that
> doesn't exist.
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  drivers/media/platform/rockchip/rkisp1/rkisp1-common.h | 4 ++++
>  drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c    | 9 ++++++---
>  drivers/media/platform/rockchip/rkisp1/rkisp1-params.c | 7 +++++++
>  3 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> index cdf2d30e2bb1..607e1a024d02 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> @@ -116,6 +116,8 @@ enum rkisp1_isp_pad {
>   * @RKISP1_FEATURE_SELF_PATH: The ISP has a self path
>   * @RKISP1_FEATURE_DUAL_CROP: The ISP has the dual crop block at the resizer input
>   * @RKISP1_FEATURE_DMA_34BIT: The ISP uses 34-bit DMA addresses
> + * @RKISP1_FEATURE_BLS: The ISP has a dedicated BLS block
> + * @RKISP1_FEATURE_COMPAND: The ISP has a companding block
>   *
>   * The ISP features are stored in a bitmask in &rkisp1_info.features and allow
>   * the driver to implement support for features present in some ISP versions
> @@ -127,6 +129,8 @@ enum rkisp1_feature {
>  	RKISP1_FEATURE_SELF_PATH = BIT(2),
>  	RKISP1_FEATURE_DUAL_CROP = BIT(3),
>  	RKISP1_FEATURE_DMA_34BIT = BIT(4),
> +	RKISP1_FEATURE_BLS = BIT(5),
> +	RKISP1_FEATURE_COMPAND = BIT(6),
>  };
>
>  #define rkisp1_has_feature(rkisp1, feature) \
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
> index 0535ce57e862..dd114ab77800 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
> @@ -509,7 +509,8 @@ static const struct rkisp1_info px30_isp_info = {
>  	.isp_ver = RKISP1_V12,
>  	.features = RKISP1_FEATURE_MIPI_CSI2
>  		  | RKISP1_FEATURE_SELF_PATH
> -		  | RKISP1_FEATURE_DUAL_CROP,
> +		  | RKISP1_FEATURE_DUAL_CROP
> +		  | RKISP1_FEATURE_BLS,

Doesn't apply for me on top of [media-stage/master + my ext params
series]

In particular, in media-stage I don't see
>  	.max_width = 3264,
>  	.max_height = 2448,

these

>  };
> @@ -532,7 +533,8 @@ static const struct rkisp1_info rk3399_isp_info = {
>  	.isp_ver = RKISP1_V10,
>  	.features = RKISP1_FEATURE_MIPI_CSI2
>  		  | RKISP1_FEATURE_SELF_PATH
> -		  | RKISP1_FEATURE_DUAL_CROP,
> +		  | RKISP1_FEATURE_DUAL_CROP
> +		  | RKISP1_FEATURE_BLS,
>  	.max_width = 4416,
>  	.max_height = 3312,
>  };
> @@ -554,7 +556,8 @@ static const struct rkisp1_info imx8mp_isp_info = {
>  	.isr_size = ARRAY_SIZE(imx8mp_isp_isrs),
>  	.isp_ver = RKISP1_V_IMX8MP,
>  	.features = RKISP1_FEATURE_MAIN_STRIDE
> -		  | RKISP1_FEATURE_DMA_34BIT,
> +		  | RKISP1_FEATURE_DMA_34BIT
> +		  | RKISP1_FEATURE_COMPAND,
>  	.max_width = 4096,
>  	.max_height = 3072,
>  };
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
> index 92312b4dabf6..bac9d4972493 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
> @@ -1268,6 +1268,12 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params,
>  	module_cfg_update = new_params->module_cfg_update;
>  	module_ens = new_params->module_ens;
>
> +	if (!rkisp1_has_feature(params->rkisp1, BLS)) {
> +		module_en_update &= ~RKISP1_CIF_ISP_MODULE_BLS;
> +		module_cfg_update &= ~RKISP1_CIF_ISP_MODULE_BLS;
> +		module_ens &= ~RKISP1_CIF_ISP_MODULE_BLS;
> +	}
> +

or is it easier to read if you

	if (rkisp1_has_feature(params->rkisp1, BLS)) {
                /* update bls config */
                if (module_cfg_update & RKISP1_CIF_ISP_MODULE_BLS)
                        rkisp1_bls_config(params,
                                          &new_params->others.bls_config);

                if (module_en_update & RKISP1_CIF_ISP_MODULE_BLS) {
                        if (module_ens & RKISP1_CIF_ISP_MODULE_BLS)
                                rkisp1_param_set_bits(params,
                                                      RKISP1_CIF_ISP_BLS_CTRL,
                                                      RKISP1_CIF_ISP_BLS_ENA);
                        else
                                rkisp1_param_clear_bits(params,
                                                        RKISP1_CIF_ISP_BLS_CTRL,
                                                        RKISP1_CIF_ISP_BLS_ENA);
                }
        }

below ?


>  	/* update dpc config */
>  	if (module_cfg_update & RKISP1_CIF_ISP_MODULE_DPCC)
>  		rkisp1_dpcc_config(params,
> @@ -1851,6 +1857,7 @@ static const struct rkisp1_ext_params_handler {
>  		.size		= sizeof(struct rkisp1_ext_params_bls_config),
>  		.handler	= rkisp1_ext_params_bls,
>  		.group		= RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS,
> +		.features	= RKISP1_FEATURE_BLS,
>  	},
>  	[RKISP1_EXT_PARAMS_BLOCK_TYPE_DPCC] = {
>  		.size		= sizeof(struct rkisp1_ext_params_dpcc_config),
> --
> Regards,
>
> Laurent Pinchart
>