| Message ID | 20251130130220.138695-1-rui.wang@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi 2025. 11. 30. 14:02 keltezéssel, Rui Wang írta: > The rkisp1_flt_config() function performs an initial direct write to > RKISP1_CIF_ISP_FILT_MODE without including the RKISP1_CIF_ISP_FLT_ENA > bit, which clears the filter enable bit in the hardware. > > The subsequent read/modify/write sequence then reads back the register > with the enable bit already cleared and cannot restore it, resulting in > the filter being inadvertently disabled. > > Remove the redundant direct write. The read/modify/write sequence alone > correctly preserves the existing enable bit state while updating the > DNR mode and filter configuration bits. > > Signed-off-by: Rui Wang <rui.wang@ideasonboard.com> > --- I suppose this will have to go to the linux kernel mailing list at some point, but in any case, it looks good to me. The write indeed seems redundant. Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > drivers/media/platform/rockchip/rkisp1/rkisp1-params.c | 6 ------ > 1 file changed, 6 deletions(-) > > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c > index c9f88635224c..6442436a5e42 100644 > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c > @@ -411,12 +411,6 @@ static void rkisp1_flt_config(struct rkisp1_params *params, > rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_FILT_LUM_WEIGHT, > arg->lum_weight); > > - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_FILT_MODE, > - (arg->mode ? RKISP1_CIF_ISP_FLT_MODE_DNR : 0) | > - RKISP1_CIF_ISP_FLT_CHROMA_V_MODE(arg->chr_v_mode) | > - RKISP1_CIF_ISP_FLT_CHROMA_H_MODE(arg->chr_h_mode) | > - RKISP1_CIF_ISP_FLT_GREEN_STAGE1(arg->grn_stage1)); > - > /* avoid to override the old enable value */ > filt_mode = rkisp1_read(params->rkisp1, RKISP1_CIF_ISP_FILT_MODE); > filt_mode &= RKISP1_CIF_ISP_FLT_ENA;
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c index c9f88635224c..6442436a5e42 100644 --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c @@ -411,12 +411,6 @@ static void rkisp1_flt_config(struct rkisp1_params *params, rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_FILT_LUM_WEIGHT, arg->lum_weight); - rkisp1_write(params->rkisp1, RKISP1_CIF_ISP_FILT_MODE, - (arg->mode ? RKISP1_CIF_ISP_FLT_MODE_DNR : 0) | - RKISP1_CIF_ISP_FLT_CHROMA_V_MODE(arg->chr_v_mode) | - RKISP1_CIF_ISP_FLT_CHROMA_H_MODE(arg->chr_h_mode) | - RKISP1_CIF_ISP_FLT_GREEN_STAGE1(arg->grn_stage1)); - /* avoid to override the old enable value */ filt_mode = rkisp1_read(params->rkisp1, RKISP1_CIF_ISP_FILT_MODE); filt_mode &= RKISP1_CIF_ISP_FLT_ENA;
The rkisp1_flt_config() function performs an initial direct write to RKISP1_CIF_ISP_FILT_MODE without including the RKISP1_CIF_ISP_FLT_ENA bit, which clears the filter enable bit in the hardware. The subsequent read/modify/write sequence then reads back the register with the enable bit already cleared and cannot restore it, resulting in the filter being inadvertently disabled. Remove the redundant direct write. The read/modify/write sequence alone correctly preserves the existing enable bit state while updating the DNR mode and filter configuration bits. Signed-off-by: Rui Wang <rui.wang@ideasonboard.com> --- drivers/media/platform/rockchip/rkisp1/rkisp1-params.c | 6 ------ 1 file changed, 6 deletions(-)