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 <tomi.valkeinen@ideasonboard.com>

diff --git a/drivers/media/platform/raspberrypi/rp1_cfe/cfe.c b/drivers/media/platform/raspberrypi/rp1_cfe/cfe.c
index fab0275..0c74356 100644 (file)
--- 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_node *node)
        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_node *node)
                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_node *node)
        }
 
        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_node *node)
                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;