| Message ID | 20240703222533.1662-2-laurent.pinchart@ideasonboard.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | media: rkisp1: Add support for the companding block | expand |
Hi Laurent On Thu, Jul 04, 2024 at 01:25:29AM GMT, Laurent Pinchart wrote: > The BLS parameters passed by userspace are specified for named colour > channels (R, Gr, Gb and B), while the hardware registers reference > positions in the 2x2 CFA pattern (A, B, C and D). > > The BLS values are swapped based on the CFA pattern when writing to or > reading from registers, using hand-roled switch statements. The logic is > duplicated already, and new code will require similar processing. Move > the swap logic to a shared function, using static data to control the > channels order. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > .../platform/rockchip/rkisp1/rkisp1-common.c | 15 +++++ > .../platform/rockchip/rkisp1/rkisp1-common.h | 3 + > .../platform/rockchip/rkisp1/rkisp1-params.c | 58 ++++--------------- > .../platform/rockchip/rkisp1/rkisp1-stats.c | 51 +++++----------- > 4 files changed, 44 insertions(+), 83 deletions(-) > > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.c > index f956b90a407a..90513d15e7a7 100644 > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.c > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.c > @@ -178,3 +178,18 @@ void rkisp1_sd_adjust_crop(struct v4l2_rect *crop, > > rkisp1_sd_adjust_crop_rect(crop, &crop_bounds); > } > + > +void rkisp1_bls_swap_regs(enum rkisp1_fmt_raw_pat_type pattern, > + const u32 input[4], u32 output[4]) > +{ > + static const unsigned int swap[][4] = { Should you declare this as a [4][4] array ? > + [RKISP1_RAW_RGGB] = { 0, 1, 2, 3 }, > + [RKISP1_RAW_GRBG] = { 1, 0, 3, 2 }, > + [RKISP1_RAW_GBRG] = { 2, 3, 0, 1 }, > + [RKISP1_RAW_BGGR] = { 3, 2, 1, 0 }, > + }; > + unsigned int i; 'i' can be declared inside the for() statement > + > + for (i = 0; i < 4; ++i) > + output[i] = input[swap[pattern][i]]; > +} > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h > index c1689c0fa05a..cdf2d30e2bb1 100644 > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h > @@ -645,6 +645,9 @@ void rkisp1_params_post_configure(struct rkisp1_params *params); > */ > void rkisp1_params_disable(struct rkisp1_params *params); > > +void rkisp1_bls_swap_regs(enum rkisp1_fmt_raw_pat_type pattern, > + const u32 input[4], u32 output[4]); > + Should this be declared after rkisp1_sd_adjust_crop() to maintain the same definition ordering as in the C file ? > /* irq handlers */ > irqreturn_t rkisp1_isp_isr(int irq, void *ctx); > irqreturn_t rkisp1_csi_isr(int irq, void *ctx); > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c > index 39a32e98807f..b10cc2701244 100644 > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c > @@ -165,54 +165,20 @@ static void rkisp1_bls_config(struct rkisp1_params *params, > new_control &= RKISP1_CIF_ISP_BLS_ENA; > /* fixed subtraction values */ > if (!arg->enable_auto) { > - const struct rkisp1_cif_isp_bls_fixed_val *pval = > - &arg->fixed_val; > + static const u32 regs[] = { > + RKISP1_CIF_ISP_BLS_A_FIXED, > + RKISP1_CIF_ISP_BLS_B_FIXED, > + RKISP1_CIF_ISP_BLS_C_FIXED, > + RKISP1_CIF_ISP_BLS_D_FIXED, > + }; > + u32 swapped[4]; > > - switch (params->raw_type) { > - case RKISP1_RAW_BGGR: > - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_D_FIXED, > - pval->r); > - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_C_FIXED, > - pval->gr); > - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_B_FIXED, > - pval->gb); > - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_A_FIXED, > - pval->b); > - break; > - case RKISP1_RAW_GBRG: > - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_C_FIXED, > - pval->r); > - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_D_FIXED, > - pval->gr); > - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_A_FIXED, > - pval->gb); > - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_B_FIXED, > - pval->b); > - break; > - case RKISP1_RAW_GRBG: > - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_B_FIXED, > - pval->r); > - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_A_FIXED, > - pval->gr); > - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_D_FIXED, > - pval->gb); > - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_C_FIXED, > - pval->b); > - break; > - case RKISP1_RAW_RGGB: > - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_A_FIXED, > - pval->r); > - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_B_FIXED, > - pval->gr); > - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_C_FIXED, > - pval->gb); > - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_D_FIXED, > - pval->b); > - break; > - default: > - break; > - } > + rkisp1_bls_swap_regs(params->raw_type, regs, swapped); Have you considered making rkisp1_bls_swap_regs() shuffle 'regs' in-place ? Not strictly necessary, just wondering > > + rkisp1_write(params->rkisp1, swapped[0], arg->fixed_val.r); > + rkisp1_write(params->rkisp1, swapped[1], arg->fixed_val.gr); > + rkisp1_write(params->rkisp1, swapped[2], arg->fixed_val.gb); > + rkisp1_write(params->rkisp1, swapped[3], arg->fixed_val.b); > } else { > if (arg->en_windows & BIT(1)) { > rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_H2_START, > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-stats.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-stats.c > index 2795eef91bdd..a502719e916a 100644 > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-stats.c > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-stats.c > @@ -304,48 +304,25 @@ static void rkisp1_stats_get_hst_meas_v12(struct rkisp1_stats *stats, > static void rkisp1_stats_get_bls_meas(struct rkisp1_stats *stats, > struct rkisp1_stat_buffer *pbuf) > { > + static const u32 regs[] = { > + RKISP1_CIF_ISP_BLS_A_MEASURED, > + RKISP1_CIF_ISP_BLS_B_MEASURED, > + RKISP1_CIF_ISP_BLS_C_MEASURED, > + RKISP1_CIF_ISP_BLS_D_MEASURED, > + }; > struct rkisp1_device *rkisp1 = stats->rkisp1; > const struct rkisp1_mbus_info *in_fmt = rkisp1->isp.sink_fmt; > struct rkisp1_cif_isp_bls_meas_val *bls_val; > + u32 swapped[4]; > + > + rkisp1_bls_swap_regs(in_fmt->bayer_pat, regs, swapped); > > bls_val = &pbuf->params.ae.bls_val; > - if (in_fmt->bayer_pat == RKISP1_RAW_BGGR) { > - bls_val->meas_b = > - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_A_MEASURED); > - bls_val->meas_gb = > - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_B_MEASURED); > - bls_val->meas_gr = > - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_C_MEASURED); > - bls_val->meas_r = > - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_D_MEASURED); > - } else if (in_fmt->bayer_pat == RKISP1_RAW_GBRG) { > - bls_val->meas_gb = > - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_A_MEASURED); > - bls_val->meas_b = > - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_B_MEASURED); > - bls_val->meas_r = > - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_C_MEASURED); > - bls_val->meas_gr = > - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_D_MEASURED); > - } else if (in_fmt->bayer_pat == RKISP1_RAW_GRBG) { > - bls_val->meas_gr = > - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_A_MEASURED); > - bls_val->meas_r = > - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_B_MEASURED); > - bls_val->meas_b = > - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_C_MEASURED); > - bls_val->meas_gb = > - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_D_MEASURED); > - } else if (in_fmt->bayer_pat == RKISP1_RAW_RGGB) { > - bls_val->meas_r = > - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_A_MEASURED); > - bls_val->meas_gr = > - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_B_MEASURED); > - bls_val->meas_gb = > - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_C_MEASURED); > - bls_val->meas_b = > - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_D_MEASURED); > - } > + > + bls_val->meas_r = rkisp1_read(rkisp1, swapped[0]); > + bls_val->meas_gr = rkisp1_read(rkisp1, swapped[1]); > + bls_val->meas_gb = rkisp1_read(rkisp1, swapped[2]); > + bls_val->meas_b = rkisp1_read(rkisp1, swapped[3]); > } > > static const struct rkisp1_stats_ops rkisp1_v10_stats_ops = { > -- > Regards, > > Laurent Pinchart >
On Thu, Jul 04, 2024 at 09:53:02AM +0200, Jacopo Mondi wrote: > On Thu, Jul 04, 2024 at 01:25:29AM GMT, Laurent Pinchart wrote: > > The BLS parameters passed by userspace are specified for named colour > > channels (R, Gr, Gb and B), while the hardware registers reference > > positions in the 2x2 CFA pattern (A, B, C and D). > > > > The BLS values are swapped based on the CFA pattern when writing to or > > reading from registers, using hand-roled switch statements. The logic is > > duplicated already, and new code will require similar processing. Move > > the swap logic to a shared function, using static data to control the > > channels order. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > .../platform/rockchip/rkisp1/rkisp1-common.c | 15 +++++ > > .../platform/rockchip/rkisp1/rkisp1-common.h | 3 + > > .../platform/rockchip/rkisp1/rkisp1-params.c | 58 ++++--------------- > > .../platform/rockchip/rkisp1/rkisp1-stats.c | 51 +++++----------- > > 4 files changed, 44 insertions(+), 83 deletions(-) > > > > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.c > > index f956b90a407a..90513d15e7a7 100644 > > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.c > > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.c > > @@ -178,3 +178,18 @@ void rkisp1_sd_adjust_crop(struct v4l2_rect *crop, > > > > rkisp1_sd_adjust_crop_rect(crop, &crop_bounds); > > } > > + > > +void rkisp1_bls_swap_regs(enum rkisp1_fmt_raw_pat_type pattern, > > + const u32 input[4], u32 output[4]) > > +{ > > + static const unsigned int swap[][4] = { > > Should you declare this as a [4][4] array ? Ack. > > + [RKISP1_RAW_RGGB] = { 0, 1, 2, 3 }, > > + [RKISP1_RAW_GRBG] = { 1, 0, 3, 2 }, > > + [RKISP1_RAW_GBRG] = { 2, 3, 0, 1 }, > > + [RKISP1_RAW_BGGR] = { 3, 2, 1, 0 }, > > + }; > > + unsigned int i; > > 'i' can be declared inside the for() statement Backporting could get a bit more annoying, but it's mainline first :-) I'll switch. > > + > > + for (i = 0; i < 4; ++i) > > + output[i] = input[swap[pattern][i]]; > > +} > > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h > > index c1689c0fa05a..cdf2d30e2bb1 100644 > > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h > > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h > > @@ -645,6 +645,9 @@ void rkisp1_params_post_configure(struct rkisp1_params *params); > > */ > > void rkisp1_params_disable(struct rkisp1_params *params); > > > > +void rkisp1_bls_swap_regs(enum rkisp1_fmt_raw_pat_type pattern, > > + const u32 input[4], u32 output[4]); > > + > > Should this be declared after rkisp1_sd_adjust_crop() to maintain the > same definition ordering as in the C file ? OK. > > /* irq handlers */ > > irqreturn_t rkisp1_isp_isr(int irq, void *ctx); > > irqreturn_t rkisp1_csi_isr(int irq, void *ctx); > > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c > > index 39a32e98807f..b10cc2701244 100644 > > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c > > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c > > @@ -165,54 +165,20 @@ static void rkisp1_bls_config(struct rkisp1_params *params, > > new_control &= RKISP1_CIF_ISP_BLS_ENA; > > /* fixed subtraction values */ > > if (!arg->enable_auto) { > > - const struct rkisp1_cif_isp_bls_fixed_val *pval = > > - &arg->fixed_val; > > + static const u32 regs[] = { > > + RKISP1_CIF_ISP_BLS_A_FIXED, > > + RKISP1_CIF_ISP_BLS_B_FIXED, > > + RKISP1_CIF_ISP_BLS_C_FIXED, > > + RKISP1_CIF_ISP_BLS_D_FIXED, > > + }; > > + u32 swapped[4]; > > > > - switch (params->raw_type) { > > - case RKISP1_RAW_BGGR: > > - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_D_FIXED, > > - pval->r); > > - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_C_FIXED, > > - pval->gr); > > - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_B_FIXED, > > - pval->gb); > > - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_A_FIXED, > > - pval->b); > > - break; > > - case RKISP1_RAW_GBRG: > > - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_C_FIXED, > > - pval->r); > > - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_D_FIXED, > > - pval->gr); > > - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_A_FIXED, > > - pval->gb); > > - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_B_FIXED, > > - pval->b); > > - break; > > - case RKISP1_RAW_GRBG: > > - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_B_FIXED, > > - pval->r); > > - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_A_FIXED, > > - pval->gr); > > - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_D_FIXED, > > - pval->gb); > > - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_C_FIXED, > > - pval->b); > > - break; > > - case RKISP1_RAW_RGGB: > > - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_A_FIXED, > > - pval->r); > > - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_B_FIXED, > > - pval->gr); > > - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_C_FIXED, > > - pval->gb); > > - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_D_FIXED, > > - pval->b); > > - break; > > - default: > > - break; > > - } > > + rkisp1_bls_swap_regs(params->raw_type, regs, swapped); > > Have you considered making rkisp1_bls_swap_regs() shuffle 'regs' > in-place ? Not strictly necessary, just wondering Not really. It could work too, but I'd need to go through a temporary array in rkisp1_bls_swap_regs(), and the caller would need to initialize the regs array. In the end I think it would just be more costly. > > + rkisp1_write(params->rkisp1, swapped[0], arg->fixed_val.r); > > + rkisp1_write(params->rkisp1, swapped[1], arg->fixed_val.gr); > > + rkisp1_write(params->rkisp1, swapped[2], arg->fixed_val.gb); > > + rkisp1_write(params->rkisp1, swapped[3], arg->fixed_val.b); > > } else { > > if (arg->en_windows & BIT(1)) { > > rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_H2_START, > > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-stats.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-stats.c > > index 2795eef91bdd..a502719e916a 100644 > > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-stats.c > > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-stats.c > > @@ -304,48 +304,25 @@ static void rkisp1_stats_get_hst_meas_v12(struct rkisp1_stats *stats, > > static void rkisp1_stats_get_bls_meas(struct rkisp1_stats *stats, > > struct rkisp1_stat_buffer *pbuf) > > { > > + static const u32 regs[] = { > > + RKISP1_CIF_ISP_BLS_A_MEASURED, > > + RKISP1_CIF_ISP_BLS_B_MEASURED, > > + RKISP1_CIF_ISP_BLS_C_MEASURED, > > + RKISP1_CIF_ISP_BLS_D_MEASURED, > > + }; > > struct rkisp1_device *rkisp1 = stats->rkisp1; > > const struct rkisp1_mbus_info *in_fmt = rkisp1->isp.sink_fmt; > > struct rkisp1_cif_isp_bls_meas_val *bls_val; > > + u32 swapped[4]; > > + > > + rkisp1_bls_swap_regs(in_fmt->bayer_pat, regs, swapped); > > > > bls_val = &pbuf->params.ae.bls_val; > > - if (in_fmt->bayer_pat == RKISP1_RAW_BGGR) { > > - bls_val->meas_b = > > - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_A_MEASURED); > > - bls_val->meas_gb = > > - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_B_MEASURED); > > - bls_val->meas_gr = > > - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_C_MEASURED); > > - bls_val->meas_r = > > - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_D_MEASURED); > > - } else if (in_fmt->bayer_pat == RKISP1_RAW_GBRG) { > > - bls_val->meas_gb = > > - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_A_MEASURED); > > - bls_val->meas_b = > > - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_B_MEASURED); > > - bls_val->meas_r = > > - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_C_MEASURED); > > - bls_val->meas_gr = > > - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_D_MEASURED); > > - } else if (in_fmt->bayer_pat == RKISP1_RAW_GRBG) { > > - bls_val->meas_gr = > > - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_A_MEASURED); > > - bls_val->meas_r = > > - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_B_MEASURED); > > - bls_val->meas_b = > > - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_C_MEASURED); > > - bls_val->meas_gb = > > - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_D_MEASURED); > > - } else if (in_fmt->bayer_pat == RKISP1_RAW_RGGB) { > > - bls_val->meas_r = > > - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_A_MEASURED); > > - bls_val->meas_gr = > > - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_B_MEASURED); > > - bls_val->meas_gb = > > - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_C_MEASURED); > > - bls_val->meas_b = > > - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_D_MEASURED); > > - } > > + > > + bls_val->meas_r = rkisp1_read(rkisp1, swapped[0]); > > + bls_val->meas_gr = rkisp1_read(rkisp1, swapped[1]); > > + bls_val->meas_gb = rkisp1_read(rkisp1, swapped[2]); > > + bls_val->meas_b = rkisp1_read(rkisp1, swapped[3]); > > } > > > > static const struct rkisp1_stats_ops rkisp1_v10_stats_ops = {
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.c index f956b90a407a..90513d15e7a7 100644 --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.c +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.c @@ -178,3 +178,18 @@ void rkisp1_sd_adjust_crop(struct v4l2_rect *crop, rkisp1_sd_adjust_crop_rect(crop, &crop_bounds); } + +void rkisp1_bls_swap_regs(enum rkisp1_fmt_raw_pat_type pattern, + const u32 input[4], u32 output[4]) +{ + static const unsigned int swap[][4] = { + [RKISP1_RAW_RGGB] = { 0, 1, 2, 3 }, + [RKISP1_RAW_GRBG] = { 1, 0, 3, 2 }, + [RKISP1_RAW_GBRG] = { 2, 3, 0, 1 }, + [RKISP1_RAW_BGGR] = { 3, 2, 1, 0 }, + }; + unsigned int i; + + for (i = 0; i < 4; ++i) + output[i] = input[swap[pattern][i]]; +} diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h index c1689c0fa05a..cdf2d30e2bb1 100644 --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h @@ -645,6 +645,9 @@ void rkisp1_params_post_configure(struct rkisp1_params *params); */ void rkisp1_params_disable(struct rkisp1_params *params); +void rkisp1_bls_swap_regs(enum rkisp1_fmt_raw_pat_type pattern, + const u32 input[4], u32 output[4]); + /* irq handlers */ irqreturn_t rkisp1_isp_isr(int irq, void *ctx); irqreturn_t rkisp1_csi_isr(int irq, void *ctx); diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c index 39a32e98807f..b10cc2701244 100644 --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c @@ -165,54 +165,20 @@ static void rkisp1_bls_config(struct rkisp1_params *params, new_control &= RKISP1_CIF_ISP_BLS_ENA; /* fixed subtraction values */ if (!arg->enable_auto) { - const struct rkisp1_cif_isp_bls_fixed_val *pval = - &arg->fixed_val; + static const u32 regs[] = { + RKISP1_CIF_ISP_BLS_A_FIXED, + RKISP1_CIF_ISP_BLS_B_FIXED, + RKISP1_CIF_ISP_BLS_C_FIXED, + RKISP1_CIF_ISP_BLS_D_FIXED, + }; + u32 swapped[4]; - switch (params->raw_type) { - case RKISP1_RAW_BGGR: - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_D_FIXED, - pval->r); - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_C_FIXED, - pval->gr); - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_B_FIXED, - pval->gb); - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_A_FIXED, - pval->b); - break; - case RKISP1_RAW_GBRG: - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_C_FIXED, - pval->r); - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_D_FIXED, - pval->gr); - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_A_FIXED, - pval->gb); - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_B_FIXED, - pval->b); - break; - case RKISP1_RAW_GRBG: - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_B_FIXED, - pval->r); - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_A_FIXED, - pval->gr); - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_D_FIXED, - pval->gb); - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_C_FIXED, - pval->b); - break; - case RKISP1_RAW_RGGB: - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_A_FIXED, - pval->r); - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_B_FIXED, - pval->gr); - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_C_FIXED, - pval->gb); - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_D_FIXED, - pval->b); - break; - default: - break; - } + rkisp1_bls_swap_regs(params->raw_type, regs, swapped); + rkisp1_write(params->rkisp1, swapped[0], arg->fixed_val.r); + rkisp1_write(params->rkisp1, swapped[1], arg->fixed_val.gr); + rkisp1_write(params->rkisp1, swapped[2], arg->fixed_val.gb); + rkisp1_write(params->rkisp1, swapped[3], arg->fixed_val.b); } else { if (arg->en_windows & BIT(1)) { rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_BLS_H2_START, diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-stats.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-stats.c index 2795eef91bdd..a502719e916a 100644 --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-stats.c +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-stats.c @@ -304,48 +304,25 @@ static void rkisp1_stats_get_hst_meas_v12(struct rkisp1_stats *stats, static void rkisp1_stats_get_bls_meas(struct rkisp1_stats *stats, struct rkisp1_stat_buffer *pbuf) { + static const u32 regs[] = { + RKISP1_CIF_ISP_BLS_A_MEASURED, + RKISP1_CIF_ISP_BLS_B_MEASURED, + RKISP1_CIF_ISP_BLS_C_MEASURED, + RKISP1_CIF_ISP_BLS_D_MEASURED, + }; struct rkisp1_device *rkisp1 = stats->rkisp1; const struct rkisp1_mbus_info *in_fmt = rkisp1->isp.sink_fmt; struct rkisp1_cif_isp_bls_meas_val *bls_val; + u32 swapped[4]; + + rkisp1_bls_swap_regs(in_fmt->bayer_pat, regs, swapped); bls_val = &pbuf->params.ae.bls_val; - if (in_fmt->bayer_pat == RKISP1_RAW_BGGR) { - bls_val->meas_b = - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_A_MEASURED); - bls_val->meas_gb = - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_B_MEASURED); - bls_val->meas_gr = - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_C_MEASURED); - bls_val->meas_r = - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_D_MEASURED); - } else if (in_fmt->bayer_pat == RKISP1_RAW_GBRG) { - bls_val->meas_gb = - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_A_MEASURED); - bls_val->meas_b = - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_B_MEASURED); - bls_val->meas_r = - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_C_MEASURED); - bls_val->meas_gr = - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_D_MEASURED); - } else if (in_fmt->bayer_pat == RKISP1_RAW_GRBG) { - bls_val->meas_gr = - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_A_MEASURED); - bls_val->meas_r = - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_B_MEASURED); - bls_val->meas_b = - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_C_MEASURED); - bls_val->meas_gb = - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_D_MEASURED); - } else if (in_fmt->bayer_pat == RKISP1_RAW_RGGB) { - bls_val->meas_r = - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_A_MEASURED); - bls_val->meas_gr = - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_B_MEASURED); - bls_val->meas_gb = - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_C_MEASURED); - bls_val->meas_b = - rkisp1_read(rkisp1, RKISP1_CIF_ISP_BLS_D_MEASURED); - } + + bls_val->meas_r = rkisp1_read(rkisp1, swapped[0]); + bls_val->meas_gr = rkisp1_read(rkisp1, swapped[1]); + bls_val->meas_gb = rkisp1_read(rkisp1, swapped[2]); + bls_val->meas_b = rkisp1_read(rkisp1, swapped[3]); } static const struct rkisp1_stats_ops rkisp1_v10_stats_ops = {
The BLS parameters passed by userspace are specified for named colour channels (R, Gr, Gb and B), while the hardware registers reference positions in the 2x2 CFA pattern (A, B, C and D). The BLS values are swapped based on the CFA pattern when writing to or reading from registers, using hand-roled switch statements. The logic is duplicated already, and new code will require similar processing. Move the swap logic to a shared function, using static data to control the channels order. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- .../platform/rockchip/rkisp1/rkisp1-common.c | 15 +++++ .../platform/rockchip/rkisp1/rkisp1-common.h | 3 + .../platform/rockchip/rkisp1/rkisp1-params.c | 58 ++++--------------- .../platform/rockchip/rkisp1/rkisp1-stats.c | 51 +++++----------- 4 files changed, 44 insertions(+), 83 deletions(-)