| Message ID | 20220823172156.9374-1-laurent.pinchart@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
On 23.08.2022 20:21, Laurent Pinchart wrote: >The ISP8000Nano v18.02 (found in the i.MX8MP) requires the ISP to be >enabled (as indicated by the ISP_CTRL.ISP_ENABLE bit) to configure the >lens shading table in internal RAM. The driver currently configures all >ISP initial parameters before enabling the ISP, which causes the LSC RAM >to not be initialized properly. > >To fix this, split the rkisp1_params_configure() function into a >rkisp1_params_pre_configure() and a rkisp1_params_post_configure(). The >former configures all ISP parameters but LSC, while the latter >configures LSC. To implement this, the rkisp1_params_apply_params_cfg() >function is deconstructed, with two small helpers created to deal with >the parameters buffers, which are then used in rkisp1_params_isr(), >rkisp1_params_pre_configure() and rkisp1_params_post_configure(). > >While this initialization ordering is only needed for the ISP8000Nano >v18.02, it doesn't affect other ISP versions negatively, and can thus be >followed unconditionally. > >Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >--- >Changes since v1: > >- Fix typo >--- > .../platform/rockchip/rkisp1/rkisp1-common.h | 29 ++- > .../platform/rockchip/rkisp1/rkisp1-isp.c | 9 +- > .../platform/rockchip/rkisp1/rkisp1-params.c | 169 ++++++++++++------ > 3 files changed, 143 insertions(+), 64 deletions(-) > >diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h >index 1383c13e22b8..8b317060ab97 100644 >--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h >+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h >@@ -589,19 +589,32 @@ void rkisp1_sd_adjust_crop(struct v4l2_rect *crop, > */ > const struct rkisp1_mbus_info *rkisp1_mbus_info_get_by_code(u32 mbus_code); > >-/* rkisp1_params_configure - configure the params when stream starts. >- * This function is called by the isp entity upon stream starts. >- * The function applies the initial configuration of the parameters. >+/* >+ * rkisp1_params_pre_configure - Configure the params before stream start > * >- * @params: pointer to rkisp1_params. >+ * @params: pointer to rkisp1_params > * @bayer_pat: the bayer pattern on the isp video sink pad > * @quantization: the quantization configured on the isp's src pad > * @ycbcr_encoding: the ycbcr_encoding configured on the isp's src pad >+ * >+ * This function is called by the ISP entity just before the ISP gets started. >+ * It applies the initial ISP parameters from the first params buffer, but >+ * skips LSC as it needs to be configured after the ISP is started. > */ >-void rkisp1_params_configure(struct rkisp1_params *params, >- enum rkisp1_fmt_raw_pat_type bayer_pat, >- enum v4l2_quantization quantization, >- enum v4l2_ycbcr_encoding ycbcr_encoding); >+void rkisp1_params_pre_configure(struct rkisp1_params *params, >+ enum rkisp1_fmt_raw_pat_type bayer_pat, >+ enum v4l2_quantization quantization, >+ enum v4l2_ycbcr_encoding ycbcr_encoding); >+ >+/* >+ * rkisp1_params_post_configure - Configure the params after stream start >+ * >+ * @params: pointer to rkisp1_params >+ * >+ * This function is called by the ISP entity just after the ISP gets started. >+ * It applies the initial ISP LSC parameters from the first params buffer. >+ */ >+void rkisp1_params_post_configure(struct rkisp1_params *params); > > /* rkisp1_params_disable - disable all parameters. > * This function is called by the isp entity upon stream start >diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c >index 20c01e0e2e17..81a8fe66f5a7 100644 >--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c >+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c >@@ -343,9 +343,9 @@ static int rkisp1_config_isp(struct rkisp1_isp *isp, > src_frm = rkisp1_isp_get_pad_fmt(isp, NULL, > RKISP1_ISP_PAD_SOURCE_VIDEO, > V4L2_SUBDEV_FORMAT_ACTIVE); >- rkisp1_params_configure(&rkisp1->params, sink_fmt->bayer_pat, >- src_frm->quantization, >- src_frm->ycbcr_enc); >+ rkisp1_params_pre_configure(&rkisp1->params, sink_fmt->bayer_pat, >+ src_frm->quantization, >+ src_frm->ycbcr_enc); > } > > return 0; >@@ -462,6 +462,9 @@ static int rkisp1_isp_start(struct rkisp1_isp *isp, struct media_pad *source) > RKISP1_CIF_ISP_CTRL_ISP_INFORM_ENABLE; > rkisp1_write(rkisp1, RKISP1_CIF_ISP_CTRL, val); > >+ if (isp->src_fmt->pixel_enc == V4L2_PIXEL_ENC_BAYER) >+ rkisp1_params_post_configure(&rkisp1->params); hi, I see that the pre-configure is actually called when format is not bayer. Thanks, Dafna >+ > return 0; > } > >diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c >index 123c26fc1679..d8731ebbf479 100644 >--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c >+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c >@@ -1297,22 +1297,6 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params, > RKISP1_CIF_ISP_CTRL_ISP_GAMMA_IN_ENA); > } > >- /* update lsc config */ >- if (module_cfg_update & RKISP1_CIF_ISP_MODULE_LSC) >- rkisp1_lsc_config(params, >- &new_params->others.lsc_config); >- >- if (module_en_update & RKISP1_CIF_ISP_MODULE_LSC) { >- if (module_ens & RKISP1_CIF_ISP_MODULE_LSC) >- rkisp1_param_set_bits(params, >- RKISP1_CIF_ISP_LSC_CTRL, >- RKISP1_CIF_ISP_LSC_CTRL_ENA); >- else >- rkisp1_param_clear_bits(params, >- RKISP1_CIF_ISP_LSC_CTRL, >- RKISP1_CIF_ISP_LSC_CTRL_ENA); >- } >- > /* update awb gains */ > if (module_cfg_update & RKISP1_CIF_ISP_MODULE_AWB_GAIN) > params->ops->awb_gain_config(params, &new_params->others.awb_gain_config); >@@ -1429,6 +1413,33 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params, > } > } > >+static void >+rkisp1_isp_isr_lsc_config(struct rkisp1_params *params, >+ const struct rkisp1_params_cfg *new_params) >+{ >+ unsigned int module_en_update, module_cfg_update, module_ens; >+ >+ module_en_update = new_params->module_en_update; >+ module_cfg_update = new_params->module_cfg_update; >+ module_ens = new_params->module_ens; >+ >+ /* update lsc config */ >+ if (module_cfg_update & RKISP1_CIF_ISP_MODULE_LSC) >+ rkisp1_lsc_config(params, >+ &new_params->others.lsc_config); >+ >+ if (module_en_update & RKISP1_CIF_ISP_MODULE_LSC) { >+ if (module_ens & RKISP1_CIF_ISP_MODULE_LSC) >+ rkisp1_param_set_bits(params, >+ RKISP1_CIF_ISP_LSC_CTRL, >+ RKISP1_CIF_ISP_LSC_CTRL_ENA); >+ else >+ rkisp1_param_clear_bits(params, >+ RKISP1_CIF_ISP_LSC_CTRL, >+ RKISP1_CIF_ISP_LSC_CTRL_ENA); >+ } >+} >+ > static void rkisp1_isp_isr_meas_config(struct rkisp1_params *params, > struct rkisp1_params_cfg *new_params) > { >@@ -1490,47 +1501,60 @@ static void rkisp1_isp_isr_meas_config(struct rkisp1_params *params, > } > } > >-static void rkisp1_params_apply_params_cfg(struct rkisp1_params *params, >- unsigned int frame_sequence) >+static bool rkisp1_params_get_buffer(struct rkisp1_params *params, >+ struct rkisp1_buffer **buf, >+ struct rkisp1_params_cfg **cfg) > { >- struct rkisp1_params_cfg *new_params; >- struct rkisp1_buffer *cur_buf = NULL; >- > if (list_empty(¶ms->params)) >- return; >+ return false; > >- cur_buf = list_first_entry(¶ms->params, >- struct rkisp1_buffer, queue); >+ *buf = list_first_entry(¶ms->params, struct rkisp1_buffer, queue); >+ *cfg = vb2_plane_vaddr(&(*buf)->vb.vb2_buf, 0); > >- new_params = (struct rkisp1_params_cfg *)vb2_plane_vaddr(&cur_buf->vb.vb2_buf, 0); >+ return true; >+} > >- rkisp1_isp_isr_other_config(params, new_params); >- rkisp1_isp_isr_meas_config(params, new_params); >+static void rkisp1_params_complete_buffer(struct rkisp1_params *params, >+ struct rkisp1_buffer *buf, >+ unsigned int frame_sequence) >+{ >+ list_del(&buf->queue); > >- /* update shadow register immediately */ >- rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CTRL, RKISP1_CIF_ISP_CTRL_ISP_CFG_UPD); >- >- list_del(&cur_buf->queue); >- >- cur_buf->vb.sequence = frame_sequence; >- vb2_buffer_done(&cur_buf->vb.vb2_buf, VB2_BUF_STATE_DONE); >+ buf->vb.sequence = frame_sequence; >+ vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_DONE); > } > > void rkisp1_params_isr(struct rkisp1_device *rkisp1) > { >- /* >- * This isr is called when the ISR finishes processing a frame (RKISP1_CIF_ISP_FRAME). >- * Configurations performed here will be applied on the next frame. >- * Since frame_sequence is updated on the vertical sync signal, we should use >- * frame_sequence + 1 here to indicate to userspace on which frame these parameters >- * are being applied. >- */ >- unsigned int frame_sequence = rkisp1->isp.frame_sequence + 1; > struct rkisp1_params *params = &rkisp1->params; >+ struct rkisp1_params_cfg *new_params; >+ struct rkisp1_buffer *cur_buf; > > spin_lock(¶ms->config_lock); >- rkisp1_params_apply_params_cfg(params, frame_sequence); > >+ if (!rkisp1_params_get_buffer(params, &cur_buf, &new_params)) >+ goto unlock; >+ >+ rkisp1_isp_isr_other_config(params, new_params); >+ rkisp1_isp_isr_lsc_config(params, new_params); >+ rkisp1_isp_isr_meas_config(params, new_params); >+ >+ /* update shadow register immediately */ >+ rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CTRL, >+ RKISP1_CIF_ISP_CTRL_ISP_CFG_UPD); >+ >+ /* >+ * This isr is called when the ISR finishes processing a frame >+ * (RKISP1_CIF_ISP_FRAME). Configurations performed here will be >+ * applied on the next frame. Since frame_sequence is updated on the >+ * vertical sync signal, we should use frame_sequence + 1 here to >+ * indicate to userspace on which frame these parameters are being >+ * applied. >+ */ >+ rkisp1_params_complete_buffer(params, cur_buf, >+ rkisp1->isp.frame_sequence + 1); >+ >+unlock: > spin_unlock(¶ms->config_lock); > } > >@@ -1573,9 +1597,18 @@ static const struct rkisp1_cif_isp_afc_config rkisp1_afc_params_default_config = > 14 > }; > >-static void rkisp1_params_config_parameter(struct rkisp1_params *params) >+void rkisp1_params_pre_configure(struct rkisp1_params *params, >+ enum rkisp1_fmt_raw_pat_type bayer_pat, >+ enum v4l2_quantization quantization, >+ enum v4l2_ycbcr_encoding ycbcr_encoding) > { > struct rkisp1_cif_isp_hst_config hst = rkisp1_hst_params_default_config; >+ struct rkisp1_params_cfg *new_params; >+ struct rkisp1_buffer *cur_buf; >+ >+ params->quantization = quantization; >+ params->ycbcr_encoding = ycbcr_encoding; >+ params->raw_type = bayer_pat; > > params->ops->awb_meas_config(params, &rkisp1_awb_params_default_config); > params->ops->awb_meas_enable(params, &rkisp1_awb_params_default_config, >@@ -1599,20 +1632,50 @@ static void rkisp1_params_config_parameter(struct rkisp1_params *params) > spin_lock_irq(¶ms->config_lock); > > /* apply the first buffer if there is one already */ >- rkisp1_params_apply_params_cfg(params, 0); > >+ if (!rkisp1_params_get_buffer(params, &cur_buf, &new_params)) >+ goto unlock; >+ >+ rkisp1_isp_isr_other_config(params, new_params); >+ rkisp1_isp_isr_meas_config(params, new_params); >+ >+ /* update shadow register immediately */ >+ rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CTRL, >+ RKISP1_CIF_ISP_CTRL_ISP_CFG_UPD); >+ >+unlock: > spin_unlock_irq(¶ms->config_lock); > } > >-void rkisp1_params_configure(struct rkisp1_params *params, >- enum rkisp1_fmt_raw_pat_type bayer_pat, >- enum v4l2_quantization quantization, >- enum v4l2_ycbcr_encoding ycbcr_encoding) >+void rkisp1_params_post_configure(struct rkisp1_params *params) > { >- params->quantization = quantization; >- params->ycbcr_encoding = ycbcr_encoding; >- params->raw_type = bayer_pat; >- rkisp1_params_config_parameter(params); >+ struct rkisp1_params_cfg *new_params; >+ struct rkisp1_buffer *cur_buf; >+ >+ spin_lock_irq(¶ms->config_lock); >+ >+ /* >+ * Apply LSC parameters from the first buffer (if any is already >+ * available. This must be done after the ISP gets started in the >+ * ISP8000Nano v18.02 (found in the i.MX8MP) as access to the LSC RAM >+ * is gated by the ISP_CTRL.ISP_ENABLE bit. As this initialization >+ * ordering doesn't affect other ISP versions negatively, do so >+ * unconditionally. >+ */ >+ >+ if (!rkisp1_params_get_buffer(params, &cur_buf, &new_params)) >+ goto unlock; >+ >+ rkisp1_isp_isr_lsc_config(params, new_params); >+ >+ /* update shadow register immediately */ >+ rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CTRL, >+ RKISP1_CIF_ISP_CTRL_ISP_CFG_UPD); >+ >+ rkisp1_params_complete_buffer(params, cur_buf, 0); >+ >+unlock: >+ spin_unlock_irq(¶ms->config_lock); > } > > /* >-- >Regards, > >Laurent Pinchart > > >_______________________________________________ >Linux-rockchip mailing list >Linux-rockchip@lists.infradead.org >http://lists.infradead.org/mailman/listinfo/linux-rockchip
Hi Dafna, On Sat, Sep 03, 2022 at 06:09:42AM +0300, Dafna Hirschfeld wrote: > On 23.08.2022 20:21, Laurent Pinchart wrote: > > The ISP8000Nano v18.02 (found in the i.MX8MP) requires the ISP to be > > enabled (as indicated by the ISP_CTRL.ISP_ENABLE bit) to configure the > > lens shading table in internal RAM. The driver currently configures all > > ISP initial parameters before enabling the ISP, which causes the LSC RAM > > to not be initialized properly. > > > > To fix this, split the rkisp1_params_configure() function into a > > rkisp1_params_pre_configure() and a rkisp1_params_post_configure(). The > > former configures all ISP parameters but LSC, while the latter > > configures LSC. To implement this, the rkisp1_params_apply_params_cfg() > > function is deconstructed, with two small helpers created to deal with > > the parameters buffers, which are then used in rkisp1_params_isr(), > > rkisp1_params_pre_configure() and rkisp1_params_post_configure(). > > > > While this initialization ordering is only needed for the ISP8000Nano > > v18.02, it doesn't affect other ISP versions negatively, and can thus be > > followed unconditionally. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > Changes since v1: > > > > - Fix typo > > --- > > .../platform/rockchip/rkisp1/rkisp1-common.h | 29 ++- > > .../platform/rockchip/rkisp1/rkisp1-isp.c | 9 +- > > .../platform/rockchip/rkisp1/rkisp1-params.c | 169 ++++++++++++------ > > 3 files changed, 143 insertions(+), 64 deletions(-) > > > > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h > > index 1383c13e22b8..8b317060ab97 100644 > > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h > > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h > > @@ -589,19 +589,32 @@ void rkisp1_sd_adjust_crop(struct v4l2_rect *crop, > > */ > > const struct rkisp1_mbus_info *rkisp1_mbus_info_get_by_code(u32 mbus_code); > > > > -/* rkisp1_params_configure - configure the params when stream starts. > > - * This function is called by the isp entity upon stream starts. > > - * The function applies the initial configuration of the parameters. > > +/* > > + * rkisp1_params_pre_configure - Configure the params before stream start > > * > > - * @params: pointer to rkisp1_params. > > + * @params: pointer to rkisp1_params > > * @bayer_pat: the bayer pattern on the isp video sink pad > > * @quantization: the quantization configured on the isp's src pad > > * @ycbcr_encoding: the ycbcr_encoding configured on the isp's src pad > > + * > > + * This function is called by the ISP entity just before the ISP gets started. > > + * It applies the initial ISP parameters from the first params buffer, but > > + * skips LSC as it needs to be configured after the ISP is started. > > */ > > -void rkisp1_params_configure(struct rkisp1_params *params, > > - enum rkisp1_fmt_raw_pat_type bayer_pat, > > - enum v4l2_quantization quantization, > > - enum v4l2_ycbcr_encoding ycbcr_encoding); > > +void rkisp1_params_pre_configure(struct rkisp1_params *params, > > + enum rkisp1_fmt_raw_pat_type bayer_pat, > > + enum v4l2_quantization quantization, > > + enum v4l2_ycbcr_encoding ycbcr_encoding); > > + > > +/* > > + * rkisp1_params_post_configure - Configure the params after stream start > > + * > > + * @params: pointer to rkisp1_params > > + * > > + * This function is called by the ISP entity just after the ISP gets started. > > + * It applies the initial ISP LSC parameters from the first params buffer. > > + */ > > +void rkisp1_params_post_configure(struct rkisp1_params *params); > > > > /* rkisp1_params_disable - disable all parameters. > > * This function is called by the isp entity upon stream start > > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c > > index 20c01e0e2e17..81a8fe66f5a7 100644 > > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c > > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c > > @@ -343,9 +343,9 @@ static int rkisp1_config_isp(struct rkisp1_isp *isp, > > src_frm = rkisp1_isp_get_pad_fmt(isp, NULL, > > RKISP1_ISP_PAD_SOURCE_VIDEO, > > V4L2_SUBDEV_FORMAT_ACTIVE); > > - rkisp1_params_configure(&rkisp1->params, sink_fmt->bayer_pat, > > - src_frm->quantization, > > - src_frm->ycbcr_enc); > > + rkisp1_params_pre_configure(&rkisp1->params, sink_fmt->bayer_pat, > > + src_frm->quantization, > > + src_frm->ycbcr_enc); > > } > > > > return 0; > > @@ -462,6 +462,9 @@ static int rkisp1_isp_start(struct rkisp1_isp *isp, struct media_pad *source) > > RKISP1_CIF_ISP_CTRL_ISP_INFORM_ENABLE; > > rkisp1_write(rkisp1, RKISP1_CIF_ISP_CTRL, val); > > > > + if (isp->src_fmt->pixel_enc == V4L2_PIXEL_ENC_BAYER) > > + rkisp1_params_post_configure(&rkisp1->params); > > hi, > I see that the pre-configure is actually called when format is not bayer. You're absolutely right. It ended up working by chance. I'll replace == with != in v1.2. > > + > > return 0; > > } > > > > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c > > index 123c26fc1679..d8731ebbf479 100644 > > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c > > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c > > @@ -1297,22 +1297,6 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params, > > RKISP1_CIF_ISP_CTRL_ISP_GAMMA_IN_ENA); > > } > > > > - /* update lsc config */ > > - if (module_cfg_update & RKISP1_CIF_ISP_MODULE_LSC) > > - rkisp1_lsc_config(params, > > - &new_params->others.lsc_config); > > - > > - if (module_en_update & RKISP1_CIF_ISP_MODULE_LSC) { > > - if (module_ens & RKISP1_CIF_ISP_MODULE_LSC) > > - rkisp1_param_set_bits(params, > > - RKISP1_CIF_ISP_LSC_CTRL, > > - RKISP1_CIF_ISP_LSC_CTRL_ENA); > > - else > > - rkisp1_param_clear_bits(params, > > - RKISP1_CIF_ISP_LSC_CTRL, > > - RKISP1_CIF_ISP_LSC_CTRL_ENA); > > - } > > - > > /* update awb gains */ > > if (module_cfg_update & RKISP1_CIF_ISP_MODULE_AWB_GAIN) > > params->ops->awb_gain_config(params, &new_params->others.awb_gain_config); > > @@ -1429,6 +1413,33 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params, > > } > > } > > > > +static void > > +rkisp1_isp_isr_lsc_config(struct rkisp1_params *params, > > + const struct rkisp1_params_cfg *new_params) > > +{ > > + unsigned int module_en_update, module_cfg_update, module_ens; > > + > > + module_en_update = new_params->module_en_update; > > + module_cfg_update = new_params->module_cfg_update; > > + module_ens = new_params->module_ens; > > + > > + /* update lsc config */ > > + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_LSC) > > + rkisp1_lsc_config(params, > > + &new_params->others.lsc_config); > > + > > + if (module_en_update & RKISP1_CIF_ISP_MODULE_LSC) { > > + if (module_ens & RKISP1_CIF_ISP_MODULE_LSC) > > + rkisp1_param_set_bits(params, > > + RKISP1_CIF_ISP_LSC_CTRL, > > + RKISP1_CIF_ISP_LSC_CTRL_ENA); > > + else > > + rkisp1_param_clear_bits(params, > > + RKISP1_CIF_ISP_LSC_CTRL, > > + RKISP1_CIF_ISP_LSC_CTRL_ENA); > > + } > > +} > > + > > static void rkisp1_isp_isr_meas_config(struct rkisp1_params *params, > > struct rkisp1_params_cfg *new_params) > > { > > @@ -1490,47 +1501,60 @@ static void rkisp1_isp_isr_meas_config(struct rkisp1_params *params, > > } > > } > > > > -static void rkisp1_params_apply_params_cfg(struct rkisp1_params *params, > > - unsigned int frame_sequence) > > +static bool rkisp1_params_get_buffer(struct rkisp1_params *params, > > + struct rkisp1_buffer **buf, > > + struct rkisp1_params_cfg **cfg) > > { > > - struct rkisp1_params_cfg *new_params; > > - struct rkisp1_buffer *cur_buf = NULL; > > - > > if (list_empty(¶ms->params)) > > - return; > > + return false; > > > > - cur_buf = list_first_entry(¶ms->params, > > - struct rkisp1_buffer, queue); > > + *buf = list_first_entry(¶ms->params, struct rkisp1_buffer, queue); > > + *cfg = vb2_plane_vaddr(&(*buf)->vb.vb2_buf, 0); > > > > - new_params = (struct rkisp1_params_cfg *)vb2_plane_vaddr(&cur_buf->vb.vb2_buf, 0); > > + return true; > > +} > > > > - rkisp1_isp_isr_other_config(params, new_params); > > - rkisp1_isp_isr_meas_config(params, new_params); > > +static void rkisp1_params_complete_buffer(struct rkisp1_params *params, > > + struct rkisp1_buffer *buf, > > + unsigned int frame_sequence) > > +{ > > + list_del(&buf->queue); > > > > - /* update shadow register immediately */ > > - rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CTRL, RKISP1_CIF_ISP_CTRL_ISP_CFG_UPD); > > - > > - list_del(&cur_buf->queue); > > - > > - cur_buf->vb.sequence = frame_sequence; > > - vb2_buffer_done(&cur_buf->vb.vb2_buf, VB2_BUF_STATE_DONE); > > + buf->vb.sequence = frame_sequence; > > + vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_DONE); > > } > > > > void rkisp1_params_isr(struct rkisp1_device *rkisp1) > > { > > - /* > > - * This isr is called when the ISR finishes processing a frame (RKISP1_CIF_ISP_FRAME). > > - * Configurations performed here will be applied on the next frame. > > - * Since frame_sequence is updated on the vertical sync signal, we should use > > - * frame_sequence + 1 here to indicate to userspace on which frame these parameters > > - * are being applied. > > - */ > > - unsigned int frame_sequence = rkisp1->isp.frame_sequence + 1; > > struct rkisp1_params *params = &rkisp1->params; > > + struct rkisp1_params_cfg *new_params; > > + struct rkisp1_buffer *cur_buf; > > > > spin_lock(¶ms->config_lock); > > - rkisp1_params_apply_params_cfg(params, frame_sequence); > > > > + if (!rkisp1_params_get_buffer(params, &cur_buf, &new_params)) > > + goto unlock; > > + > > + rkisp1_isp_isr_other_config(params, new_params); > > + rkisp1_isp_isr_lsc_config(params, new_params); > > + rkisp1_isp_isr_meas_config(params, new_params); > > + > > + /* update shadow register immediately */ > > + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CTRL, > > + RKISP1_CIF_ISP_CTRL_ISP_CFG_UPD); > > + > > + /* > > + * This isr is called when the ISR finishes processing a frame > > + * (RKISP1_CIF_ISP_FRAME). Configurations performed here will be > > + * applied on the next frame. Since frame_sequence is updated on the > > + * vertical sync signal, we should use frame_sequence + 1 here to > > + * indicate to userspace on which frame these parameters are being > > + * applied. > > + */ > > + rkisp1_params_complete_buffer(params, cur_buf, > > + rkisp1->isp.frame_sequence + 1); > > + > > +unlock: > > spin_unlock(¶ms->config_lock); > > } > > > > @@ -1573,9 +1597,18 @@ static const struct rkisp1_cif_isp_afc_config rkisp1_afc_params_default_config = > > 14 > > }; > > > > -static void rkisp1_params_config_parameter(struct rkisp1_params *params) > > +void rkisp1_params_pre_configure(struct rkisp1_params *params, > > + enum rkisp1_fmt_raw_pat_type bayer_pat, > > + enum v4l2_quantization quantization, > > + enum v4l2_ycbcr_encoding ycbcr_encoding) > > { > > struct rkisp1_cif_isp_hst_config hst = rkisp1_hst_params_default_config; > > + struct rkisp1_params_cfg *new_params; > > + struct rkisp1_buffer *cur_buf; > > + > > + params->quantization = quantization; > > + params->ycbcr_encoding = ycbcr_encoding; > > + params->raw_type = bayer_pat; > > > > params->ops->awb_meas_config(params, &rkisp1_awb_params_default_config); > > params->ops->awb_meas_enable(params, &rkisp1_awb_params_default_config, > > @@ -1599,20 +1632,50 @@ static void rkisp1_params_config_parameter(struct rkisp1_params *params) > > spin_lock_irq(¶ms->config_lock); > > > > /* apply the first buffer if there is one already */ > > - rkisp1_params_apply_params_cfg(params, 0); > > > > + if (!rkisp1_params_get_buffer(params, &cur_buf, &new_params)) > > + goto unlock; > > + > > + rkisp1_isp_isr_other_config(params, new_params); > > + rkisp1_isp_isr_meas_config(params, new_params); > > + > > + /* update shadow register immediately */ > > + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CTRL, > > + RKISP1_CIF_ISP_CTRL_ISP_CFG_UPD); > > + > > +unlock: > > spin_unlock_irq(¶ms->config_lock); > > } > > > > -void rkisp1_params_configure(struct rkisp1_params *params, > > - enum rkisp1_fmt_raw_pat_type bayer_pat, > > - enum v4l2_quantization quantization, > > - enum v4l2_ycbcr_encoding ycbcr_encoding) > > +void rkisp1_params_post_configure(struct rkisp1_params *params) > > { > > - params->quantization = quantization; > > - params->ycbcr_encoding = ycbcr_encoding; > > - params->raw_type = bayer_pat; > > - rkisp1_params_config_parameter(params); > > + struct rkisp1_params_cfg *new_params; > > + struct rkisp1_buffer *cur_buf; > > + > > + spin_lock_irq(¶ms->config_lock); > > + > > + /* > > + * Apply LSC parameters from the first buffer (if any is already > > + * available. This must be done after the ISP gets started in the > > + * ISP8000Nano v18.02 (found in the i.MX8MP) as access to the LSC RAM > > + * is gated by the ISP_CTRL.ISP_ENABLE bit. As this initialization > > + * ordering doesn't affect other ISP versions negatively, do so > > + * unconditionally. > > + */ > > + > > + if (!rkisp1_params_get_buffer(params, &cur_buf, &new_params)) > > + goto unlock; > > + > > + rkisp1_isp_isr_lsc_config(params, new_params); > > + > > + /* update shadow register immediately */ > > + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CTRL, > > + RKISP1_CIF_ISP_CTRL_ISP_CFG_UPD); > > + > > + rkisp1_params_complete_buffer(params, cur_buf, 0); > > + > > +unlock: > > + spin_unlock_irq(¶ms->config_lock); > > } > > > > /*
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h index 1383c13e22b8..8b317060ab97 100644 --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h @@ -589,19 +589,32 @@ void rkisp1_sd_adjust_crop(struct v4l2_rect *crop, */ const struct rkisp1_mbus_info *rkisp1_mbus_info_get_by_code(u32 mbus_code); -/* rkisp1_params_configure - configure the params when stream starts. - * This function is called by the isp entity upon stream starts. - * The function applies the initial configuration of the parameters. +/* + * rkisp1_params_pre_configure - Configure the params before stream start * - * @params: pointer to rkisp1_params. + * @params: pointer to rkisp1_params * @bayer_pat: the bayer pattern on the isp video sink pad * @quantization: the quantization configured on the isp's src pad * @ycbcr_encoding: the ycbcr_encoding configured on the isp's src pad + * + * This function is called by the ISP entity just before the ISP gets started. + * It applies the initial ISP parameters from the first params buffer, but + * skips LSC as it needs to be configured after the ISP is started. */ -void rkisp1_params_configure(struct rkisp1_params *params, - enum rkisp1_fmt_raw_pat_type bayer_pat, - enum v4l2_quantization quantization, - enum v4l2_ycbcr_encoding ycbcr_encoding); +void rkisp1_params_pre_configure(struct rkisp1_params *params, + enum rkisp1_fmt_raw_pat_type bayer_pat, + enum v4l2_quantization quantization, + enum v4l2_ycbcr_encoding ycbcr_encoding); + +/* + * rkisp1_params_post_configure - Configure the params after stream start + * + * @params: pointer to rkisp1_params + * + * This function is called by the ISP entity just after the ISP gets started. + * It applies the initial ISP LSC parameters from the first params buffer. + */ +void rkisp1_params_post_configure(struct rkisp1_params *params); /* rkisp1_params_disable - disable all parameters. * This function is called by the isp entity upon stream start diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c index 20c01e0e2e17..81a8fe66f5a7 100644 --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c @@ -343,9 +343,9 @@ static int rkisp1_config_isp(struct rkisp1_isp *isp, src_frm = rkisp1_isp_get_pad_fmt(isp, NULL, RKISP1_ISP_PAD_SOURCE_VIDEO, V4L2_SUBDEV_FORMAT_ACTIVE); - rkisp1_params_configure(&rkisp1->params, sink_fmt->bayer_pat, - src_frm->quantization, - src_frm->ycbcr_enc); + rkisp1_params_pre_configure(&rkisp1->params, sink_fmt->bayer_pat, + src_frm->quantization, + src_frm->ycbcr_enc); } return 0; @@ -462,6 +462,9 @@ static int rkisp1_isp_start(struct rkisp1_isp *isp, struct media_pad *source) RKISP1_CIF_ISP_CTRL_ISP_INFORM_ENABLE; rkisp1_write(rkisp1, RKISP1_CIF_ISP_CTRL, val); + if (isp->src_fmt->pixel_enc == V4L2_PIXEL_ENC_BAYER) + rkisp1_params_post_configure(&rkisp1->params); + return 0; } diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c index 123c26fc1679..d8731ebbf479 100644 --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c @@ -1297,22 +1297,6 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params, RKISP1_CIF_ISP_CTRL_ISP_GAMMA_IN_ENA); } - /* update lsc config */ - if (module_cfg_update & RKISP1_CIF_ISP_MODULE_LSC) - rkisp1_lsc_config(params, - &new_params->others.lsc_config); - - if (module_en_update & RKISP1_CIF_ISP_MODULE_LSC) { - if (module_ens & RKISP1_CIF_ISP_MODULE_LSC) - rkisp1_param_set_bits(params, - RKISP1_CIF_ISP_LSC_CTRL, - RKISP1_CIF_ISP_LSC_CTRL_ENA); - else - rkisp1_param_clear_bits(params, - RKISP1_CIF_ISP_LSC_CTRL, - RKISP1_CIF_ISP_LSC_CTRL_ENA); - } - /* update awb gains */ if (module_cfg_update & RKISP1_CIF_ISP_MODULE_AWB_GAIN) params->ops->awb_gain_config(params, &new_params->others.awb_gain_config); @@ -1429,6 +1413,33 @@ rkisp1_isp_isr_other_config(struct rkisp1_params *params, } } +static void +rkisp1_isp_isr_lsc_config(struct rkisp1_params *params, + const struct rkisp1_params_cfg *new_params) +{ + unsigned int module_en_update, module_cfg_update, module_ens; + + module_en_update = new_params->module_en_update; + module_cfg_update = new_params->module_cfg_update; + module_ens = new_params->module_ens; + + /* update lsc config */ + if (module_cfg_update & RKISP1_CIF_ISP_MODULE_LSC) + rkisp1_lsc_config(params, + &new_params->others.lsc_config); + + if (module_en_update & RKISP1_CIF_ISP_MODULE_LSC) { + if (module_ens & RKISP1_CIF_ISP_MODULE_LSC) + rkisp1_param_set_bits(params, + RKISP1_CIF_ISP_LSC_CTRL, + RKISP1_CIF_ISP_LSC_CTRL_ENA); + else + rkisp1_param_clear_bits(params, + RKISP1_CIF_ISP_LSC_CTRL, + RKISP1_CIF_ISP_LSC_CTRL_ENA); + } +} + static void rkisp1_isp_isr_meas_config(struct rkisp1_params *params, struct rkisp1_params_cfg *new_params) { @@ -1490,47 +1501,60 @@ static void rkisp1_isp_isr_meas_config(struct rkisp1_params *params, } } -static void rkisp1_params_apply_params_cfg(struct rkisp1_params *params, - unsigned int frame_sequence) +static bool rkisp1_params_get_buffer(struct rkisp1_params *params, + struct rkisp1_buffer **buf, + struct rkisp1_params_cfg **cfg) { - struct rkisp1_params_cfg *new_params; - struct rkisp1_buffer *cur_buf = NULL; - if (list_empty(¶ms->params)) - return; + return false; - cur_buf = list_first_entry(¶ms->params, - struct rkisp1_buffer, queue); + *buf = list_first_entry(¶ms->params, struct rkisp1_buffer, queue); + *cfg = vb2_plane_vaddr(&(*buf)->vb.vb2_buf, 0); - new_params = (struct rkisp1_params_cfg *)vb2_plane_vaddr(&cur_buf->vb.vb2_buf, 0); + return true; +} - rkisp1_isp_isr_other_config(params, new_params); - rkisp1_isp_isr_meas_config(params, new_params); +static void rkisp1_params_complete_buffer(struct rkisp1_params *params, + struct rkisp1_buffer *buf, + unsigned int frame_sequence) +{ + list_del(&buf->queue); - /* update shadow register immediately */ - rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CTRL, RKISP1_CIF_ISP_CTRL_ISP_CFG_UPD); - - list_del(&cur_buf->queue); - - cur_buf->vb.sequence = frame_sequence; - vb2_buffer_done(&cur_buf->vb.vb2_buf, VB2_BUF_STATE_DONE); + buf->vb.sequence = frame_sequence; + vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_DONE); } void rkisp1_params_isr(struct rkisp1_device *rkisp1) { - /* - * This isr is called when the ISR finishes processing a frame (RKISP1_CIF_ISP_FRAME). - * Configurations performed here will be applied on the next frame. - * Since frame_sequence is updated on the vertical sync signal, we should use - * frame_sequence + 1 here to indicate to userspace on which frame these parameters - * are being applied. - */ - unsigned int frame_sequence = rkisp1->isp.frame_sequence + 1; struct rkisp1_params *params = &rkisp1->params; + struct rkisp1_params_cfg *new_params; + struct rkisp1_buffer *cur_buf; spin_lock(¶ms->config_lock); - rkisp1_params_apply_params_cfg(params, frame_sequence); + if (!rkisp1_params_get_buffer(params, &cur_buf, &new_params)) + goto unlock; + + rkisp1_isp_isr_other_config(params, new_params); + rkisp1_isp_isr_lsc_config(params, new_params); + rkisp1_isp_isr_meas_config(params, new_params); + + /* update shadow register immediately */ + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CTRL, + RKISP1_CIF_ISP_CTRL_ISP_CFG_UPD); + + /* + * This isr is called when the ISR finishes processing a frame + * (RKISP1_CIF_ISP_FRAME). Configurations performed here will be + * applied on the next frame. Since frame_sequence is updated on the + * vertical sync signal, we should use frame_sequence + 1 here to + * indicate to userspace on which frame these parameters are being + * applied. + */ + rkisp1_params_complete_buffer(params, cur_buf, + rkisp1->isp.frame_sequence + 1); + +unlock: spin_unlock(¶ms->config_lock); } @@ -1573,9 +1597,18 @@ static const struct rkisp1_cif_isp_afc_config rkisp1_afc_params_default_config = 14 }; -static void rkisp1_params_config_parameter(struct rkisp1_params *params) +void rkisp1_params_pre_configure(struct rkisp1_params *params, + enum rkisp1_fmt_raw_pat_type bayer_pat, + enum v4l2_quantization quantization, + enum v4l2_ycbcr_encoding ycbcr_encoding) { struct rkisp1_cif_isp_hst_config hst = rkisp1_hst_params_default_config; + struct rkisp1_params_cfg *new_params; + struct rkisp1_buffer *cur_buf; + + params->quantization = quantization; + params->ycbcr_encoding = ycbcr_encoding; + params->raw_type = bayer_pat; params->ops->awb_meas_config(params, &rkisp1_awb_params_default_config); params->ops->awb_meas_enable(params, &rkisp1_awb_params_default_config, @@ -1599,20 +1632,50 @@ static void rkisp1_params_config_parameter(struct rkisp1_params *params) spin_lock_irq(¶ms->config_lock); /* apply the first buffer if there is one already */ - rkisp1_params_apply_params_cfg(params, 0); + if (!rkisp1_params_get_buffer(params, &cur_buf, &new_params)) + goto unlock; + + rkisp1_isp_isr_other_config(params, new_params); + rkisp1_isp_isr_meas_config(params, new_params); + + /* update shadow register immediately */ + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CTRL, + RKISP1_CIF_ISP_CTRL_ISP_CFG_UPD); + +unlock: spin_unlock_irq(¶ms->config_lock); } -void rkisp1_params_configure(struct rkisp1_params *params, - enum rkisp1_fmt_raw_pat_type bayer_pat, - enum v4l2_quantization quantization, - enum v4l2_ycbcr_encoding ycbcr_encoding) +void rkisp1_params_post_configure(struct rkisp1_params *params) { - params->quantization = quantization; - params->ycbcr_encoding = ycbcr_encoding; - params->raw_type = bayer_pat; - rkisp1_params_config_parameter(params); + struct rkisp1_params_cfg *new_params; + struct rkisp1_buffer *cur_buf; + + spin_lock_irq(¶ms->config_lock); + + /* + * Apply LSC parameters from the first buffer (if any is already + * available. This must be done after the ISP gets started in the + * ISP8000Nano v18.02 (found in the i.MX8MP) as access to the LSC RAM + * is gated by the ISP_CTRL.ISP_ENABLE bit. As this initialization + * ordering doesn't affect other ISP versions negatively, do so + * unconditionally. + */ + + if (!rkisp1_params_get_buffer(params, &cur_buf, &new_params)) + goto unlock; + + rkisp1_isp_isr_lsc_config(params, new_params); + + /* update shadow register immediately */ + rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CTRL, + RKISP1_CIF_ISP_CTRL_ISP_CFG_UPD); + + rkisp1_params_complete_buffer(params, cur_buf, 0); + +unlock: + spin_unlock_irq(¶ms->config_lock); } /*
The ISP8000Nano v18.02 (found in the i.MX8MP) requires the ISP to be enabled (as indicated by the ISP_CTRL.ISP_ENABLE bit) to configure the lens shading table in internal RAM. The driver currently configures all ISP initial parameters before enabling the ISP, which causes the LSC RAM to not be initialized properly. To fix this, split the rkisp1_params_configure() function into a rkisp1_params_pre_configure() and a rkisp1_params_post_configure(). The former configures all ISP parameters but LSC, while the latter configures LSC. To implement this, the rkisp1_params_apply_params_cfg() function is deconstructed, with two small helpers created to deal with the parameters buffers, which are then used in rkisp1_params_isr(), rkisp1_params_pre_configure() and rkisp1_params_post_configure(). While this initialization ordering is only needed for the ISP8000Nano v18.02, it doesn't affect other ISP versions negatively, and can thus be followed unconditionally. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- Changes since v1: - Fix typo --- .../platform/rockchip/rkisp1/rkisp1-common.h | 29 ++- .../platform/rockchip/rkisp1/rkisp1-isp.c | 9 +- .../platform/rockchip/rkisp1/rkisp1-params.c | 169 ++++++++++++------ 3 files changed, 143 insertions(+), 64 deletions(-)