[{"id":28691,"web_url":"https://patchwork.libcamera.org/comment/28691/","msgid":"<20240216182327.GF7120@pendragon.ideasonboard.com>","date":"2024-02-16T18:23:27","subject":"Re: [PATCH v2] ipa: rkisp1: Add RKISP1_V_IMX8MP version","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the patch.\n\nOn Fri, Feb 16, 2024 at 08:09:52PM +0900, Paul Elder wrote:\n> Add the version number for RKISP1_V_IMX8MP, and initialize values for\n> it.\n> \n> Signed-off-by: Paul Elder \n> ---\n> This patch goes with the kernel series [1] which adds support for\n> i.MX8MP.\n> \n> [1] https://lore.kernel.org/linux-media/20240216095458.2919694-1-paul.elder@ideasonboard.com/\n> ---\n> include/linux/rkisp1-config.h | 8 +++++---\n> src/ipa/rkisp1/rkisp1.cpp | 1 +\n> 2 files changed, 6 insertions(+), 3 deletions(-)\n> \n> diff --git a/include/linux/rkisp1-config.h b/include/linux/rkisp1-config.h\n> index ec7cde8c..2d1c448a 100644\n> --- a/include/linux/rkisp1-config.h\n> +++ b/include/linux/rkisp1-config.h\n> @@ -4,8 +4,8 @@\n> * Copyright (C) 2017 Rockchip Electronics Co., Ltd.\n> */\n> \n> -#ifndef _RKISP1_CONFIG_H\n> -#define _RKISP1_CONFIG_H\n> +#ifndef _UAPI_RKISP1_CONFIG_H\n> +#define _UAPI_RKISP1_CONFIG_H\n> \n> #include \n> \n> @@ -179,12 +179,14 @@\n> * @RKISP1_V11: declared in the original vendor code, but not used\n> * @RKISP1_V12: used at least in rk3326 and px30\n> * @RKISP1_V13: used at least in rk1808\n> + * @RKISP1_V_IMX8MP: used in at least imx8mp\n> */\n> enum rkisp1_cif_isp_version {\n> \tRKISP1_V10 = 10,\n> \tRKISP1_V11,\n> \tRKISP1_V12,\n> \tRKISP1_V13,\n> +\tRKISP1_V_IMX8MP,\n> };\n> \n> enum rkisp1_cif_isp_histogram_mode {\n> @@ -992,4 +994,4 @@ struct rkisp1_stat_buffer {\n> \tstruct rkisp1_cif_isp_stat params;\n> };\n> \n> -#endif /* _RKISP1_CONFIG_H */\n> +#endif /* _UAPI_RKISP1_CONFIG_H */\n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index 6544c925..11e31bf4 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -128,6 +128,7 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,\n> \t/* \\todo Add support for other revisions */\n> \tswitch (hwRevision) {\n> \tcase RKISP1_V10:\n> +\tcase RKISP1_V_IMX8MP:\n\nThere's a similar version check in agc.cpp that needs to be updated, as\nthe i.MX8MP has 16 histogram bins. And now that I'm looking at this, the\nthree variables below are never used. I'll post a v2 of this patch as a\nseries that includes cleanups.\n\n> \t\thwHistBinNMax_ = RKISP1_CIF_ISP_HIST_BIN_N_MAX_V10;\n> \t\thwGammaOutMaxSamples_ = RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10;\n> \t\thwHistogramWeightGridsSize_ = RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V10;","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id F3FB3C3257\n\tfor ;\n\tFri, 16 Feb 2024 18:23:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 580D662801;\n\tFri, 16 Feb 2024 19:23:25 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2FED361CAE\n\tfor ;\n\tFri, 16 Feb 2024 19:23:24 +0100 (CET)","from pendragon.ideasonboard.com (89-27-53-110.bb.dnainternet.fi\n\t[89.27.53.110])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4E57E105D;\n\tFri, 16 Feb 2024 19:23:19 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"miMahNXf\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1708107799;\n\tbh=DBMttSoe+U1dy4Wmy+seWo2XbK19IqVQbadinH9SbTE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=miMahNXfeS35zFsKJctkSvB7N7SqPf5fqg9xoN01PtMQLbgTjWJKqAqvAIqqJttDm\n\tmVLMooRxx9ZhnmLtgO2XLBo8CajycZdt0M6aPuDOPxH9dcuZ5clzHRwPGHEb2mZlnV\n\tl3WqQN7fjPgHnUi4LeBG/SHWNmnyvmo6/N1wYOrs=","Date":"Fri, 16 Feb 2024 20:23:27 +0200","From":"Laurent Pinchart ","To":"Paul Elder ","Subject":"Re: [PATCH v2] ipa: rkisp1: Add RKISP1_V_IMX8MP version","Message-ID":"<20240216182327.GF7120@pendragon.ideasonboard.com>","References":"<20240216110952.3169023-1-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240216110952.3169023-1-paul.elder@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28696,"web_url":"https://patchwork.libcamera.org/comment/28696/","msgid":"<20310c87-8c51-4559-809a-f786513644cb@ideasonboard.com>","date":"2024-02-20T08:42:59","subject":"Re: [PATCH v2] ipa: rkisp1: Add RKISP1_V_IMX8MP version","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Paul,\n\nAm 16.02.24 um 12:09 schrieb Paul Elder:\n> Add the version number for RKISP1_V_IMX8MP, and initialize values for\n> it.\n> \n> Signed-off-by: Paul Elder \n> ---\n> This patch goes with the kernel series [1] which adds support for\n> i.MX8MP.\n> \n> [1] https://lore.kernel.org/linux-media/20240216095458.2919694-1-paul.elder@ideasonboard.com/\n> ---\n> include/linux/rkisp1-config.h | 8 +++++---\n> src/ipa/rkisp1/rkisp1.cpp | 1 +\n> 2 files changed, 6 insertions(+), 3 deletions(-)\n> \n> diff --git a/include/linux/rkisp1-config.h b/include/linux/rkisp1-config.h\n> index ec7cde8c..2d1c448a 100644\n> --- a/include/linux/rkisp1-config.h\n> +++ b/include/linux/rkisp1-config.h\n> @@ -4,8 +4,8 @@\n> * Copyright (C) 2017 Rockchip Electronics Co., Ltd.\n> */\n> \n> -#ifndef _RKISP1_CONFIG_H\n> -#define _RKISP1_CONFIG_H\n> +#ifndef _UAPI_RKISP1_CONFIG_H\n> +#define _UAPI_RKISP1_CONFIG_H\n> \n> #include \n> \n> @@ -179,12 +179,14 @@\n> * @RKISP1_V11: declared in the original vendor code, but not used\n> * @RKISP1_V12: used at least in rk3326 and px30\n> * @RKISP1_V13: used at least in rk1808\n> + * @RKISP1_V_IMX8MP: used in at least imx8mp\n> */\n> enum rkisp1_cif_isp_version {\n> \tRKISP1_V10 = 10,\n> \tRKISP1_V11,\n> \tRKISP1_V12,\n> \tRKISP1_V13,\n> +\tRKISP1_V_IMX8MP,\nmaybe it's a bit late in the process. But would it make sense to somehow \nreference the upstream names used by VSI? (This would then spill over to \nthe kernel patches).\nThese files give a pretty good overview:\nhttps://github.com/nxp-imx/isp-vvcam/blob/lf-5.15.y_2.0.0/vvcam/isp/isp_version.h\nhttps://github.com/nxp-imx/isp-vvcam/blob/lf-5.15.y_2.0.0/vvcam/isp/mrv_all_regs.h\n\nAnd according to \nhttps://github.com/nxp-imx/meta-imx/blob/mickledore-6.1.22-2.0.0/meta-bsp/recipes-bsp/isp-imx/isp-imx_4.2.2.22.0.bb\n\nthe isp of the imx8mp is officially named ISP8000NANO_V1802\n\n> };\n> \n> enum rkisp1_cif_isp_histogram_mode {\n> @@ -992,4 +994,4 @@ struct rkisp1_stat_buffer {\n> \tstruct rkisp1_cif_isp_stat params;\n> };\n> \n> -#endif /* _RKISP1_CONFIG_H */\n> +#endif /* _UAPI_RKISP1_CONFIG_H */\n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index 6544c925..11e31bf4 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -128,6 +128,7 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,\n> \t/* \\todo Add support for other revisions */\n> \tswitch (hwRevision) {\n> \tcase RKISP1_V10:\n> +\tcase RKISP1_V_IMX8MP:\n> \t\thwHistBinNMax_ = RKISP1_CIF_ISP_HIST_BIN_N_MAX_V10;\n> \t\thwGammaOutMaxSamples_ = RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10;\n> \t\thwHistogramWeightGridsSize_ = RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V10;","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 31EA2C3257\n\tfor ;\n\tTue, 20 Feb 2024 08:43:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1044B62807;\n\tTue, 20 Feb 2024 09:43:04 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id ED39261CA3\n\tfor ;\n\tTue, 20 Feb 2024 09:43:01 +0100 (CET)","from [IPV6:2a00:6020:448c:6c00:8d25:d85f:6c20:5cde] (unknown\n\t[IPv6:2a00:6020:448c:6c00:8d25:d85f:6c20:5cde])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E1B2A899\n\tfor ;\n\tTue, 20 Feb 2024 09:42:54 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"JpcwmSst\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1708418574;\n\tbh=6OND1x+wxXehCwHLsDqvNFU3b2Qk7/4jzjQcM+/RD/k=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=JpcwmSstX890q/H72O5kx9egGTAuXmkYzhS5Vrd7qwA1C0yXDRMyCOQYc5+L+OIxn\n\tsx8kpYE1BlFNR22zy/SG57KzSIj239pw5j1k0DvDD/AVMEnsCCce0o4+wv458kveo4\n\toDed49/fqYY2FurrBOwUSSzvj2f4+T5EQyjtCns4=","Message-ID":"<20310c87-8c51-4559-809a-f786513644cb@ideasonboard.com>","Date":"Tue, 20 Feb 2024 09:42:59 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2] ipa: rkisp1: Add RKISP1_V_IMX8MP version","To":"libcamera-devel@lists.libcamera.org","References":"<20240216110952.3169023-1-paul.elder@ideasonboard.com>","Content-Language":"en-US","From":"Stefan Klug ","In-Reply-To":"<20240216110952.3169023-1-paul.elder@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28716,"web_url":"https://patchwork.libcamera.org/comment/28716/","msgid":"<20240223122100.GD18146@pendragon.ideasonboard.com>","date":"2024-02-23T12:21:00","subject":"Re: [PATCH v2] ipa: rkisp1: Add RKISP1_V_IMX8MP version","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Stefan,\n\nOn Tue, Feb 20, 2024 at 09:42:59AM +0100, Stefan Klug wrote:\n> Am 16.02.24 um 12:09 schrieb Paul Elder:\n> > Add the version number for RKISP1_V_IMX8MP, and initialize values for\n> > it.\n> > \n> > Signed-off-by: Paul Elder \n> > ---\n> > This patch goes with the kernel series [1] which adds support for\n> > i.MX8MP.\n> > \n> > [1] https://lore.kernel.org/linux-media/20240216095458.2919694-1-paul.elder@ideasonboard.com/\n> > ---\n> > include/linux/rkisp1-config.h | 8 +++++---\n> > src/ipa/rkisp1/rkisp1.cpp | 1 +\n> > 2 files changed, 6 insertions(+), 3 deletions(-)\n> > \n> > diff --git a/include/linux/rkisp1-config.h b/include/linux/rkisp1-config.h\n> > index ec7cde8c..2d1c448a 100644\n> > --- a/include/linux/rkisp1-config.h\n> > +++ b/include/linux/rkisp1-config.h\n> > @@ -4,8 +4,8 @@\n> > * Copyright (C) 2017 Rockchip Electronics Co., Ltd.\n> > */\n> > \n> > -#ifndef _RKISP1_CONFIG_H\n> > -#define _RKISP1_CONFIG_H\n> > +#ifndef _UAPI_RKISP1_CONFIG_H\n> > +#define _UAPI_RKISP1_CONFIG_H\n> > \n> > #include \n> > \n> > @@ -179,12 +179,14 @@\n> > * @RKISP1_V11: declared in the original vendor code, but not used\n> > * @RKISP1_V12: used at least in rk3326 and px30\n> > * @RKISP1_V13: used at least in rk1808\n> > + * @RKISP1_V_IMX8MP: used in at least imx8mp\n> > */\n> > enum rkisp1_cif_isp_version {\n> > \tRKISP1_V10 = 10,\n> > \tRKISP1_V11,\n> > \tRKISP1_V12,\n> > \tRKISP1_V13,\n> > +\tRKISP1_V_IMX8MP,\n>\n> maybe it's a bit late in the process. But would it make sense to somehow \n> reference the upstream names used by VSI? (This would then spill over to \n> the kernel patches).\n> These files give a pretty good overview:\n> https://github.com/nxp-imx/isp-vvcam/blob/lf-5.15.y_2.0.0/vvcam/isp/isp_version.h\n> https://github.com/nxp-imx/isp-vvcam/blob/lf-5.15.y_2.0.0/vvcam/isp/mrv_all_regs.h\n> \n> And according to \n> https://github.com/nxp-imx/meta-imx/blob/mickledore-6.1.22-2.0.0/meta-bsp/recipes-bsp/isp-imx/isp-imx_4.2.2.22.0.bb\n> \n> the isp of the imx8mp is officially named ISP8000NANO_V1802\n\nI'd like to, but I'm lacking information :-( We don't know if the\nversion in the i.MX8MP is unmodified compared to the V1802. In\nparticular, I suspect the 34-bit DMA addresses may be an extension\nspecific to the i.MX8MP, not necessarily present in other V1802. More\ngenerally, I don't know if there are any other V1802, or where the other\nversions of the ISP may have been integrated. As we're really blind\nhere, we decided to add a version specific to the i.MX8MP.\n\nIdeally I would have liked to use the value reported by the hardware\nversion register instead of a custom enum, but we also lack information\nthere. In particular, we don't know if the version register may take the\nsame value for different ISP versions that need to be handled\ndifferently by the driver, and we also don't know if there's a risk of a\nversion number clash between Rockchip and VSI.\n\n> > };\n> > \n> > enum rkisp1_cif_isp_histogram_mode {\n> > @@ -992,4 +994,4 @@ struct rkisp1_stat_buffer {\n> > \tstruct rkisp1_cif_isp_stat params;\n> > };\n> > \n> > -#endif /* _RKISP1_CONFIG_H */\n> > +#endif /* _UAPI_RKISP1_CONFIG_H */\n> > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> > index 6544c925..11e31bf4 100644\n> > --- a/src/ipa/rkisp1/rkisp1.cpp\n> > +++ b/src/ipa/rkisp1/rkisp1.cpp\n> > @@ -128,6 +128,7 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,\n> > \t/* \\todo Add support for other revisions */\n> > \tswitch (hwRevision) {\n> > \tcase RKISP1_V10:\n> > +\tcase RKISP1_V_IMX8MP:\n> > \t\thwHistBinNMax_ = RKISP1_CIF_ISP_HIST_BIN_N_MAX_V10;\n> > \t\thwGammaOutMaxSamples_ = RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10;\n> > \t\thwHistogramWeightGridsSize_ = RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V10;","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id CC63EBD80A\n\tfor ;\n\tFri, 23 Feb 2024 12:20:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1F44D627FD;\n\tFri, 23 Feb 2024 13:20:59 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 93F7761CA1\n\tfor ;\n\tFri, 23 Feb 2024 13:20:57 +0100 (CET)","from pendragon.ideasonboard.com (89-27-53-110.bb.dnainternet.fi\n\t[89.27.53.110])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0E8479B6;\n\tFri, 23 Feb 2024 13:20:47 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"gGeYeB/n\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1708690848;\n\tbh=r5feTR/h6P0I2f/yIwFosEw9gGKFGVwrZaIxJDV+lrg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=gGeYeB/ng5r8PGm8xGAGJnYBGt+lNV4nmAGhw7rfkVTvUWDVIQuVJj3+SKLiMlpiJ\n\tj1r70Yom24EwWtg+YBVuMTUTwcQZVjd07MPBfDAd3awusqceOEahqGL1rI9KGDXMSq\n\tLZIOAduErrz9QqTcPZZyYtpASqK+19lu0jTefpxs=","Date":"Fri, 23 Feb 2024 14:21:00 +0200","From":"Laurent Pinchart ","To":"Stefan Klug ","Subject":"Re: [PATCH v2] ipa: rkisp1: Add RKISP1_V_IMX8MP version","Message-ID":"<20240223122100.GD18146@pendragon.ideasonboard.com>","References":"<20240216110952.3169023-1-paul.elder@ideasonboard.com>\n\t<20310c87-8c51-4559-809a-f786513644cb@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20310c87-8c51-4559-809a-f786513644cb@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]