Hi Jacopo - thanks for the patch. I think this probably should come before 5/8 in the series, and
just hardcode return 0 in rkisp1_params_pre/post_configure() temporarily.
On 05/06/2024 17:54, Jacopo Mondi wrote:
The support for the extensible parameters format introduces the
possibility of failures in handling the parameters buffer.
Errors in parsing the configuration parameters are not propagated
to the rkisp1_config_isp() and the rkisp1_isp_start() functions.
Propagate any possible errors to the callers to report it to userspace.
Signed-off-by: Jacopo Mondi <jacopo.mondi@xxxxxxxxxxxxxxxx>
---
.../media/platform/rockchip/rkisp1/rkisp1-common.h | 10 +++++-----
.../media/platform/rockchip/rkisp1/rkisp1-isp.c | 14 +++++++++-----
.../media/platform/rockchip/rkisp1/rkisp1-params.c | 14 +++++++++-----
3 files changed, 23 insertions(+), 15 deletions(-)
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
index 0bddae8dbdb1..f9df5ed96c98 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
@@ -591,10 +591,10 @@ const struct rkisp1_mbus_info *rkisp1_mbus_info_get_by_code(u32 mbus_code);
* 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_pre_configure(struct rkisp1_params *params,
- enum rkisp1_fmt_raw_pat_type bayer_pat,
- enum v4l2_quantization quantization,
- enum v4l2_ycbcr_encoding ycbcr_encoding);
+int 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
@@ -604,7 +604,7 @@ void rkisp1_params_pre_configure(struct rkisp1_params *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);
+int 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 91301d17d356..05227c6a16fe 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
@@ -310,12 +310,16 @@ static int rkisp1_config_isp(struct rkisp1_isp *isp,
rkisp1_params_disable(&rkisp1->params);
} else {
const struct v4l2_mbus_framefmt *src_frm;
+ int ret;
src_frm = v4l2_subdev_state_get_format(sd_state,
RKISP1_ISP_PAD_SOURCE_VIDEO);
- rkisp1_params_pre_configure(&rkisp1->params, sink_fmt->bayer_pat,
- src_frm->quantization,
- src_frm->ycbcr_enc);
+ ret = rkisp1_params_pre_configure(&rkisp1->params,
+ sink_fmt->bayer_pat,
+ src_frm->quantization,
+ src_frm->ycbcr_enc);
+ if (ret)
+ return ret;
}
isp->sink_fmt = sink_fmt;
@@ -458,9 +462,9 @@ static int rkisp1_isp_start(struct rkisp1_isp *isp,
src_info = rkisp1_mbus_info_get_by_code(src_fmt->code);
if (src_info->pixel_enc != V4L2_PIXEL_ENC_BAYER)
- rkisp1_params_post_configure(&rkisp1->params);
+ ret = rkisp1_params_post_configure(&rkisp1->params);
- return 0;
+ return ret;
I think ret could be returned uninitialised in some circumstances in this function now - if it's not
the IMX8MP version and the pixel encoding is bayer...or am I missing something?
}
/* ----------------------------------------------------------------------------
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
index 3d78e643d0b8..c081fd490b2b 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
@@ -2123,10 +2123,10 @@ static const struct rkisp1_cif_isp_afc_config rkisp1_afc_params_default_config =
14
};
-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)
+int 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_buffer *buf;
@@ -2187,9 +2187,11 @@ void rkisp1_params_pre_configure(struct rkisp1_params *params,
unlock:
spin_unlock_irq(¶ms->config_lock);
+
+ return ret;
}
-void rkisp1_params_post_configure(struct rkisp1_params *params)
+int rkisp1_params_post_configure(struct rkisp1_params *params)
{
struct rkisp1_buffer *buf;
int ret = 0;
@@ -2227,6 +2229,8 @@ void rkisp1_params_post_configure(struct rkisp1_params *params)
unlock:
spin_unlock_irq(¶ms->config_lock);
+
+ return ret;
}
/*
