From 84c9958dd71b8a4dcf16cbf6fdb867c668652634 Mon Sep 17 00:00:00 2001 From: Tomi Valkeinen Date: Wed, 27 Sep 2023 16:00:39 +0300 Subject: [PATCH] media: rp1: cfe: Fix width & height in cfe_start_channel() The logic for handling width & height in cfe_start_channel() is somewhat odd and, afaics, broken. The code reads: bool start_fe = is_fe_enabled(cfe) && test_all_nodes(cfe, NODE_ENABLED, NODE_STREAMING); if (start_fe || is_image_output_node(node)) { width = node->fmt.fmt.pix.width; height = node->fmt.fmt.pix.height; } cfe_start_channel() is called for all video nodes that will be used. So this means that if, say, fe_stats is enabled as the last node, start_fe will be true, and width and height will be taken from fe_stats' node. The width and height will thus contain garbage, which then gets programmed to the csi2 registers. It seems that this often still works fine, though, probably if the width & height are large enough. Drop the above code, and instead get the width & height from the csi2 subdev's sink pad for the csi2 channel that is used. For metadata the width & height will be 0 as before. Signed-off-by: Tomi Valkeinen --- drivers/media/platform/raspberrypi/rp1_cfe/cfe.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) --- a/drivers/media/platform/raspberrypi/rp1_cfe/cfe.c +++ b/drivers/media/platform/raspberrypi/rp1_cfe/cfe.c @@ -763,20 +763,16 @@ static void cfe_start_channel(struct cfe struct v4l2_mbus_framefmt *source_fmt; const struct cfe_fmt *fmt; unsigned long flags; - unsigned int width = 0, height = 0; bool start_fe = is_fe_enabled(cfe) && test_all_nodes(cfe, NODE_ENABLED, NODE_STREAMING); cfe_dbg("%s: [%s]\n", __func__, node_desc[node->id].name); - if (start_fe || is_image_output_node(node)) { - width = node->fmt.fmt.pix.width; - height = node->fmt.fmt.pix.height; - } - state = v4l2_subdev_lock_and_get_active_state(&cfe->csi2.sd); if (start_fe) { + unsigned int width, height; + WARN_ON(!is_fe_enabled(cfe)); cfe_dbg("%s: %s using csi2 channel %d\n", __func__, node_desc[FE_OUT0].name, @@ -785,6 +781,9 @@ static void cfe_start_channel(struct cfe source_fmt = v4l2_subdev_get_pad_format(&cfe->csi2.sd, state, cfe->fe_csi2_channel); fmt = find_format_by_code(source_fmt->code); + width = source_fmt->width; + height = source_fmt->height; + /* * Start the associated CSI2 Channel as well. * @@ -800,6 +799,8 @@ static void cfe_start_channel(struct cfe } if (is_csi2_node(node)) { + unsigned int width = 0, height = 0; + u32 mode = CSI2_MODE_NORMAL; source_fmt = v4l2_subdev_get_pad_format(&cfe->csi2.sd, state, @@ -807,6 +808,9 @@ static void cfe_start_channel(struct cfe fmt = find_format_by_code(source_fmt->code); if (is_image_output_node(node)) { + width = source_fmt->width; + height = source_fmt->height; + if (node->fmt.fmt.pix.pixelformat == fmt->remap[CFE_REMAP_16BIT]) mode = CSI2_MODE_REMAP;