[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20211104124505.2e0f655d.dorota.czaplejewicz@puri.sm>
Date: Thu, 4 Nov 2021 12:45:05 +0100
From: Dorota Czaplejewicz <dorota.czaplejewicz@...i.sm>
To: Dan Carpenter <dan.carpenter@...cle.com>
Cc: Steve Longerbeam <slongerbeam@...il.com>,
Philipp Zabel <p.zabel@...gutronix.de>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Shawn Guo <shawnguo@...nel.org>,
Sascha Hauer <s.hauer@...gutronix.de>,
Pengutronix Kernel Team <kernel@...gutronix.de>,
Fabio Estevam <festevam@...il.com>,
NXP Linux Team <linux-imx@....com>,
linux-media@...r.kernel.org, linux-staging@...ts.linux.dev,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
kernel@...i.sm, phone-devel@...r.kernel.org
Subject: Re: [PATCHv2 4/4] media: imx: Use dedicated format handler for
i.MX7/8
Hi,
On Wed, 3 Nov 2021 13:41:00 +0300
Dan Carpenter <dan.carpenter@...cle.com> wrote:
> On Sun, Oct 17, 2021 at 01:08:37PM +0200, Dorota Czaplejewicz wrote:
> > +static int imx78_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
> > + const struct v4l2_mbus_framefmt *mbus,
> > + const struct imx_media_pixfmt *cc)
> > +{
> > + u32 width;
> > + u32 stride;
> > + u8 divisor;
> > +
> > + if (!cc) {
> > + cc = imx_media_find_ipu_format(mbus->code,
> > + PIXFMT_SEL_YUV_RGB);
> > + if (!cc)
> > + cc = imx_media_find_mbus_format(mbus->code,
> > + PIXFMT_SEL_ANY);
> > + if (!cc)
> > + return -EINVAL;
> > + }
> > +
> > + /*
> > + * TODO: the IPU currently does not support the AYUV32 format,
> > + * so until it does convert to a supported YUV format.
> > + */
> > + if (cc->ipufmt && cc->cs == IPUV3_COLORSPACE_YUV) {
> > + u32 code;
> > +
> > + imx_media_enum_mbus_formats(&code, 0, PIXFMT_SEL_YUV);
> > + cc = imx_media_find_mbus_format(code, PIXFMT_SEL_YUV);
>
> Do we need a if (!cc) NULL check after this assignment?
In v3 of this series, this statement is only present in the IMX56 code path, which is unmodified compared to the original.
However, there's a missing check in the IMX78. I'm not sure if those can fail in practice, but for the sake of correctness, I rolled out an updated v4 series, where both checks are present. Message-id: 20211104113631.206899-1-dorota.czaplejewicz@...i.sm
Cheers,
Dorota
>
> > + }
> > +
> > + /*
> > + * The hardware can handle line lengths divisible by 4 bytes,
> > + * as long as the number of lines is even.
> > + * Otherwise, use the value of 8 bytes recommended in the datasheet.
> > + */
> > + divisor = 4 << (mbus->height % 2);
> > +
> > + width = round_up(mbus->width, divisor);
> > +
> > + if (cc->planar)
> > + stride = round_up(width, 16);
> > + else
> > + stride = round_up((width * cc->bpp) >> 3, divisor);
> > +
> > + pix->width = width;
> > + pix->height = mbus->height;
> > + pix->pixelformat = cc->fourcc;
> > + pix->colorspace = mbus->colorspace;
> > + pix->xfer_func = mbus->xfer_func;
> > + pix->ycbcr_enc = mbus->ycbcr_enc;
> > + pix->quantization = mbus->quantization;
> > + pix->field = mbus->field;
> > + pix->bytesperline = stride;
> > + pix->sizeimage = cc->planar ? ((stride * pix->height * cc->bpp) >> 3) :
> > + stride * pix->height;
> > +
> > + return 0;
> > +}
>
> regards,
> dan carpenter
Content of type "application/pgp-signature" skipped
Powered by blists - more mailing lists