linux-kernel - Re: [PATCH v1 10/10] uapi: linux: add mediatek isp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241118095850.GA30268@pendragon.ideasonboard.com>
Date: Mon, 18 Nov 2024 11:58:50 +0200
From: Laurent Pinchart <laurent.pinchart@...asonboard.com>
To: CK Hu (胡俊光) <ck.hu@...iatek.com>
Cc: "sumit.semwal@...aro.org" <sumit.semwal@...aro.org>,
	"christian.koenig@....com" <christian.koenig@....com>,
	"mchehab@...nel.org" <mchehab@...nel.org>,
	"conor+dt@...nel.org" <conor+dt@...nel.org>,
	"robh@...nel.org" <robh@...nel.org>,
	"matthias.bgg@...il.com" <matthias.bgg@...il.com>,
	"krzk+dt@...nel.org" <krzk+dt@...nel.org>,
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
	Shu-hsiang Yang (楊舒翔) <Shu-hsiang.Yang@...iatek.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>,
	"yunkec@...omium.org" <yunkec@...omium.org>,
	"linaro-mm-sig@...ts.linaro.org" <linaro-mm-sig@...ts.linaro.org>,
	"linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	Yaya Chang (張雅清) <Yaya.Chang@...iatek.com>,
	Project_Global_Chrome_Upstream_Group <Project_Global_Chrome_Upstream_Group@...iatek.com>,
	"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
	Teddy Chen (陳乾元) <Teddy.Chen@...iatek.com>,
	"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>,
	"hidenorik@...omium.org" <hidenorik@...omium.org>,
	Shun-Yi Wang (王順億) <Shun-Yi.Wang@...iatek.com>
Subject: Re: [PATCH v1 10/10] uapi: linux: add mediatek isp_7x camsys user api

On Wed, Nov 06, 2024 at 03:04:37PM +0200, Laurent Pinchart wrote:
> On Mon, Oct 14, 2024 at 05:56:40AM +0000, CK Hu (胡俊光) wrote:
> > Hi, Shu-hsiang:
> > 
> > On Wed, 2024-10-09 at 19:15 +0800, Shu-hsiang Yang wrote:
> > > Add UAPI for MediaTek ISP platform, providing user-space
> > > interfaces for the new camsys driver.
> > > 
> > > Signed-off-by: Shu-hsiang Yang <Shu-hsiang.Yang@...iatek.com>
> > > ---
> > 
> > [snip]
> > 
> > > +
> > > +/* MTK ISP camsys controls */
> > > +#define V4L2_CID_MTK_CAM_USED_ENGINE_LIMIT	(V4L2_CID_USER_MTK_CAM_BASE + 1)
> > > +#define V4L2_CID_MTK_CAM_BIN_LIMIT		(V4L2_CID_USER_MTK_CAM_BASE + 2)
> > > +#define V4L2_CID_MTK_CAM_FRZ_LIMIT		(V4L2_CID_USER_MTK_CAM_BASE + 3)
> > > +#define V4L2_CID_MTK_CAM_RESOURCE_PLAN_POLICY	(V4L2_CID_USER_MTK_CAM_BASE + 4)
> > > +#define V4L2_CID_MTK_CAM_USED_ENGINE		(V4L2_CID_USER_MTK_CAM_BASE + 5)
> > > +#define V4L2_CID_MTK_CAM_BIN			(V4L2_CID_USER_MTK_CAM_BASE + 6)
> > > +#define V4L2_CID_MTK_CAM_FRZ			(V4L2_CID_USER_MTK_CAM_BASE + 7)
> > > +#define V4L2_CID_MTK_CAM_USED_ENGINE_TRY	(V4L2_CID_USER_MTK_CAM_BASE + 8)
> > > +#define V4L2_CID_MTK_CAM_BIN_TRY		(V4L2_CID_USER_MTK_CAM_BASE + 9)
> > > +#define V4L2_CID_MTK_CAM_FRZ_TRY		(V4L2_CID_USER_MTK_CAM_BASE + 10)
> > > +#define V4L2_CID_MTK_CAM_PIXEL_RATE		(V4L2_CID_USER_MTK_CAM_BASE + 11)
> > > +#define V4L2_CID_MTK_CAM_FEATURE		(V4L2_CID_USER_MTK_CAM_BASE + 12)
> > > +#define V4L2_CID_MTK_CAM_SYNC_ID		(V4L2_CID_USER_MTK_CAM_BASE + 13)
> > > +#define V4L2_CID_MTK_CAM_RAW_PATH_SELECT	(V4L2_CID_USER_MTK_CAM_BASE + 14)
> > > +#define V4L2_CID_MTK_CAM_HSF_EN			(V4L2_CID_USER_MTK_CAM_BASE + 15)
> > > +#define V4L2_CID_MTK_CAM_PDE_INFO		(V4L2_CID_USER_MTK_CAM_BASE + 16)
> > > +#define V4L2_CID_MTK_CAM_MSTREAM_EXPOSURE	(V4L2_CID_USER_MTK_CAM_BASE + 17)
> > > +#define V4L2_CID_MTK_CAM_RAW_RESOURCE_CALC	(V4L2_CID_USER_MTK_CAM_BASE + 18)
> > > +#define V4L2_CID_MTK_CAM_TG_FLASH_CFG		(V4L2_CID_USER_MTK_CAM_BASE + 19)
> > > +#define V4L2_CID_MTK_CAM_RAW_RESOURCE_UPDATE	(V4L2_CID_USER_MTK_CAM_BASE + 20)
> > > +#define V4L2_CID_MTK_CAM_CAMSYS_HW_MODE		(V4L2_CID_USER_MTK_CAM_BASE + 21)
> > > +
> > 
> > Please give introduction of how to use these user space interface.
> 
> I'm very, very *not* thrilled by all this. It looks like a big pile of
> hacks really. Every single parameter used by those controls needs to be
> clearly documented, including explaining how they are used, in order for
> us to review the API. I suspect that many of the parameters should
> instead be handled through the ISP parameters buffers, or be controlled
> from standard V4L2 APIs.

While on the topic of documentation, we also need a high-level
architecture document. This patch series adds 30000 lines of code, it's
very hard for reviewers to understand the architecture of the driver
just by looking at the code. At the very least, a high-level description
of the hardware (at least from the view point of the interface the
firmware exposes) is needed.

-- 
Regards,

Laurent Pinchart