Re: [RFC PATCH V4 4/4] platform: mtk-isp: Add Mediatek FD driver

From: Tomasz Figa <tfiga@chromium.org>
To: Jerry-ch Chen <Jerry-ch.Chen@mediatek.com>
Cc: linux-devicetree <devicetree@vger.kernel.org>,
	"Sean Cheng (鄭昇弘)" <Sean.Cheng@mediatek.com>,
	"Laurent Pinchart" <laurent.pinchart+renesas@ideasonboard.com>,
	zwisler@chromium.org,
	srv_heupstream <srv_heupstream@mediatek.com>,
	"Christie Yu (游雅惠)" <christie.yu@mediatek.com>,
	"Hans Verkuil" <hverkuil@xs4all.nl>,
	"Jungo Lin (林明俊)" <jungo.lin@mediatek.com>,
	"Sj Huang" <sj.huang@mediatek.com>,
	yuzhao@chromium.org, "Hans Verkuil" <hans.verkuil@cisco.com>,
	"Pi-Hsun Shih" <pihsun@chromium.org>,
	"Frederic Chen (陳俊元)" <frederic.chen@mediatek.com>,
	"Matthias Brugger" <matthias.bgg@gmail.com>,
	"moderated list:ARM/Mediatek SoC support"
	<linux-mediatek@lists.infradead.org>,
	"Mauro Carvalho Chehab" <mchehab@kernel.org>,
	"list@263.net:IOMMU DRIVERS <iommu@lists.linux-foundation.org>,
	Joerg Roedel <joro@8bytes.org>,
	" <linux-arm-kernel@lists.infradead.org>,
	"Linux Media Mailing List" <linux-media@vger.kernel.org>
Subject: Re: [RFC PATCH V4 4/4] platform: mtk-isp: Add Mediatek FD driver
Date: Fri, 29 May 2020 14:59:55 +0200	[thread overview]
Message-ID: <CAAFQd5BBfapVv_3cwGte=p=6G8QXZQP=-ciZ8NBZZeSBGrHmCA@mail.gmail.com> (raw)
In-Reply-To: <1590755163.23156.24.camel@mtksdccf07>

On Fri, May 29, 2020 at 2:26 PM Jerry-ch Chen
<Jerry-ch.Chen@mediatek.com> wrote:
>
> Hi Tomasz,
>
> I Appreciate your review comments, here's the reply.
>
> On Mon, 2020-05-25 at 14:24 +0200, Tomasz Figa wrote:
> > r
> >
> > On Fri, May 22, 2020 at 4:11 PM Jerry-ch Chen
> > <Jerry-ch.Chen@mediatek.com> wrote:
> > >
> > > Hi Tomasz,
> > >
> > > On Thu, 2020-05-21 at 18:28 +0000, Tomasz Figa wrote:
> > > > Hi Jerry,
> > > >
> > > > On Wed, Dec 04, 2019 at 08:47:32PM +0800, Jerry-ch Chen wrote:
[snip]
> > Isn't still a need to clamp() width and height to min/max, though?
> Yes, I'll add them back.
>
> This function will be refined as :
>
> static void mtk_fd_fill_pixfmt_mp(struct v4l2_pix_format_mplane *dfmt,
>                                   u32 pixfmt)
> {
>         v4l2_fill_pixfmt_mp(dfmt, pixfmt, dfmt->width, dfmt->height);
>
>         dfmt->field = V4L2_FIELD_NONE;
>         dfmt->colorspace = V4L2_COLORSPACE_BT2020;
>         dfmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
>         dfmt->quantization = V4L2_QUANTIZATION_DEFAULT;
>         dfmt->xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(dfmt->colorspace);
>
>         /* Keep user setting as possible */
>         dfmt->width = clamp(dfmt->width,
>                             MTK_FD_OUTPUT_MIN_WIDTH,
>                             MTK_FD_OUTPUT_MAX_WIDTH);
>         dfmt->height = clamp(dfmt->height,
>                              MTK_FD_OUTPUT_MIN_HEIGHT,
>                              MTK_FD_OUTPUT_MAX_HEIGHT);

Note that this would cause the other fields of dfmt to be inconsistent
with width and height. The correct way to do this would be to first
clamp and then call v4l2_fill_pixfmt_mp().

Best regards,
Tomasz

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel