Hi Laurent
On Thu, Jul 04, 2024 at 01:25:32AM GMT, Laurent Pinchart wrote:
> From: Paul Elder <paul.elder@xxxxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxxx>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> ---
> 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
>
