Hi Philipp,

On 10/05/2018 02:44 AM, Philipp Zabel wrote:
Hi Steve,

On Thu, 2018-10-04 at 11:53 -0700, Steve Longerbeam wrote:


+
+               /* framelines for NTSC / PAL */
+               height = (std & V4L2_STD_525_60) ? 525 : 625;
I think this is a bit convoluted. Instead of initializing std, then
possibly changing it, and then comparing to the inital value, and then
checking it again to determine the new height, why not just:

                if (width == 720 && height == 480) {
                        std = V4L2_STD_NTSC;
                        height = 525;
                } else if (width == 720 && height == 576) {
                        std = V4L2_STD_PAL;
                        height = 625;
                } else {
                        dev_err(csi->ipu->dev,
                                "Unsupported interlaced video mode\n");
                        ret = -EINVAL;
                        goto out_unlock;
                }

?

Yes that was a bit convoluted, fixed.


/*
         * if cycles is set, we need to handle this over multiple cycles as
         * generic/bayer data
         */
-       if (is_parallel_bus(&priv->upstream_ep) && incc->cycles) {
-               if_fmt.width *= incc->cycles;
If the input format width passed to ipu_csi_init_interface is not
multiplied by the number of cycles per pixel anymore, width in the
CSI_SENS_FRM_SIZE register will be set to the unmultiplied value from
infmt.
This breaks 779680e2e793 ("media: imx: add support for RGB565_2X8 on
parallel bus").

Oops, that was a mistake, thanks for catching, fixed.

Steve

_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to