| Message ID | 20201030112225.2095909-1-helen.koike@collabora.com |
|---|---|
| Headers | show |
| Series | destage Rockchip ISP1 driver | expand |
On 30/10/2020 12:22, Helen Koike wrote: > From: Shunqian Zheng <zhengsq@rock-chips.com> > > Add the Rockchip ISP1 specific processing parameter format > V4L2_META_FMT_RK_ISP1_PARAMS and metadata format > V4L2_META_FMT_RK_ISP1_STAT_3A for 3A. > > Signed-off-by: Shunqian Zheng <zhengsq@rock-chips.com> > Signed-off-by: Jacob Chen <jacob2.chen@rock-chips.com> > Signed-off-by: Helen Koike <helen.koike@collabora.com> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > Hello, > > This patch is a continuation of: > > https://patchwork.kernel.org/project/linux-arm-kernel/patch/20191106120132.6876-2-helen.koike@collabora.com/ > > These formats are already documented under > Documentation/userspace-api/media/v4l/pixfmt-meta-rkisp1.rst > > We had agreed to keep under > drivers/staging/media/rkisp1/uapi/rkisp1-config.h while the driver was > in staging, since we are moving it out of staging, I guess this is the > time :) > > Regards, > Helen > --- > drivers/media/v4l2-core/v4l2-ioctl.c | 2 ++ > drivers/staging/media/rkisp1/uapi/rkisp1-config.h | 4 ---- > include/uapi/linux/videodev2.h | 4 ++++ > 3 files changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c > index eeff398fbdcc1..202597d031c6b 100644 > --- a/drivers/media/v4l2-core/v4l2-ioctl.c > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c > @@ -1402,6 +1402,8 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt) > case V4L2_META_FMT_UVC: descr = "UVC Payload Header Metadata"; break; > case V4L2_META_FMT_D4XX: descr = "Intel D4xx UVC Metadata"; break; > case V4L2_META_FMT_VIVID: descr = "Vivid Metadata"; break; > + case V4L2_META_FMT_RK_ISP1_PARAMS: descr = "Rockchip ISP1 3A params"; break; > + case V4L2_META_FMT_RK_ISP1_STAT_3A: descr = "Rockchip ISP1 3A statistics"; break; Use 'Params' and 'Statistics' to conform to the other descriptions. Regards, Hans > > default: > /* Compressed formats */ > diff --git a/drivers/staging/media/rkisp1/uapi/rkisp1-config.h b/drivers/staging/media/rkisp1/uapi/rkisp1-config.h > index 8d906cc7da8fc..6e449e7842605 100644 > --- a/drivers/staging/media/rkisp1/uapi/rkisp1-config.h > +++ b/drivers/staging/media/rkisp1/uapi/rkisp1-config.h > @@ -9,10 +9,6 @@ > > #include <linux/types.h> > > -/* Vendor specific - used for RK_ISP1 camera sub-system */ > -#define V4L2_META_FMT_RK_ISP1_PARAMS v4l2_fourcc('R', 'K', '1', 'P') /* Rockchip ISP1 params */ > -#define V4L2_META_FMT_RK_ISP1_STAT_3A v4l2_fourcc('R', 'K', '1', 'S') /* Rockchip ISP1 3A statistics */ > - > /* Defect Pixel Cluster Detection */ > #define RKISP1_CIF_ISP_MODULE_DPCC (1U << 0) > /* Black Level Subtraction */ > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > index 534eaa4d39bc8..c2e13ba81196b 100644 > --- a/include/uapi/linux/videodev2.h > +++ b/include/uapi/linux/videodev2.h > @@ -770,6 +770,10 @@ struct v4l2_pix_format { > #define V4L2_META_FMT_D4XX v4l2_fourcc('D', '4', 'X', 'X') /* D4XX Payload Header metadata */ > #define V4L2_META_FMT_VIVID v4l2_fourcc('V', 'I', 'V', 'D') /* Vivid Metadata */ > > +/* Vendor specific - used for RK_ISP1 camera sub-system */ > +#define V4L2_META_FMT_RK_ISP1_PARAMS v4l2_fourcc('R', 'K', '1', 'P') /* Rockchip ISP1 params */ > +#define V4L2_META_FMT_RK_ISP1_STAT_3A v4l2_fourcc('R', 'K', '1', 'S') /* Rockchip ISP1 3A statistics */ > + > /* priv field value to indicates that subsequent fields are valid. */ > #define V4L2_PIX_FMT_PRIV_MAGIC 0xfeedcafe > >
On 30/10/2020 12:22, Helen Koike wrote: > All the items in the TODO list were addressed, uapi was reviewed, > documentation written, checkpatch errors fixed, several bugs fixed. > > There is no big reason to keep this driver in staging, so move it out. > > Signed-off-by: Helen Koike <helen.koike@collabora.com> Mauro held off on the patch that moved the bindings out of staging. I think that patch should be folded into this patch. Ditto for the binding MAINTAINERS patch, that can be folded in the next patch. > > --- > .../media/v4l/pixfmt-meta-rkisp1.rst | 2 +- > drivers/media/platform/Kconfig | 18 ++++++++++++++++++ > drivers/media/platform/Makefile | 1 + > .../platform/rockchip}/rkisp1/Makefile | 0 > .../rockchip}/rkisp1/rkisp1-capture.c | 0 > .../platform/rockchip}/rkisp1/rkisp1-common.c | 0 > .../platform/rockchip}/rkisp1/rkisp1-common.h | 2 +- > .../platform/rockchip}/rkisp1/rkisp1-dev.c | 0 > .../platform/rockchip}/rkisp1/rkisp1-isp.c | 0 > .../platform/rockchip}/rkisp1/rkisp1-params.c | 0 > .../platform/rockchip}/rkisp1/rkisp1-regs.h | 0 > .../rockchip}/rkisp1/rkisp1-resizer.c | 0 > .../platform/rockchip}/rkisp1/rkisp1-stats.c | 0 > drivers/staging/media/Kconfig | 2 -- > drivers/staging/media/Makefile | 1 - > drivers/staging/media/rkisp1/Kconfig | 19 ------------------- > drivers/staging/media/rkisp1/TODO | 6 ------ > .../uapi/linux}/rkisp1-config.h | 0 > 18 files changed, 21 insertions(+), 30 deletions(-) > rename drivers/{staging/media => media/platform/rockchip}/rkisp1/Makefile (100%) > rename drivers/{staging/media => media/platform/rockchip}/rkisp1/rkisp1-capture.c (100%) > rename drivers/{staging/media => media/platform/rockchip}/rkisp1/rkisp1-common.c (100%) > rename drivers/{staging/media => media/platform/rockchip}/rkisp1/rkisp1-common.h (99%) > rename drivers/{staging/media => media/platform/rockchip}/rkisp1/rkisp1-dev.c (100%) > rename drivers/{staging/media => media/platform/rockchip}/rkisp1/rkisp1-isp.c (100%) > rename drivers/{staging/media => media/platform/rockchip}/rkisp1/rkisp1-params.c (100%) > rename drivers/{staging/media => media/platform/rockchip}/rkisp1/rkisp1-regs.h (100%) > rename drivers/{staging/media => media/platform/rockchip}/rkisp1/rkisp1-resizer.c (100%) > rename drivers/{staging/media => media/platform/rockchip}/rkisp1/rkisp1-stats.c (100%) > delete mode 100644 drivers/staging/media/rkisp1/Kconfig > delete mode 100644 drivers/staging/media/rkisp1/TODO > rename {drivers/staging/media/rkisp1/uapi => include/uapi/linux}/rkisp1-config.h (100%) > > diff --git a/Documentation/userspace-api/media/v4l/pixfmt-meta-rkisp1.rst b/Documentation/userspace-api/media/v4l/pixfmt-meta-rkisp1.rst > index 7e43837ed260a..f3671472d4105 100644 > --- a/Documentation/userspace-api/media/v4l/pixfmt-meta-rkisp1.rst > +++ b/Documentation/userspace-api/media/v4l/pixfmt-meta-rkisp1.rst > @@ -46,4 +46,4 @@ important tuning tools using software control loop. > rkisp1 uAPI data types > ====================== > > -.. kernel-doc:: drivers/staging/media/rkisp1/uapi/rkisp1-config.h > +.. kernel-doc:: include/uapi/linux/rkisp1-config.h > diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig > index a3cb104956d56..202d447759fd8 100644 > --- a/drivers/media/platform/Kconfig > +++ b/drivers/media/platform/Kconfig > @@ -448,6 +448,24 @@ config VIDEO_RENESAS_VSP1 > To compile this driver as a module, choose M here: the module > will be called vsp1. > > +config VIDEO_ROCKCHIP_ISP1 > + tristate "Rockchip Image Signal Processing v1 Unit driver" > + depends on VIDEO_V4L2 && OF > + depends on ARCH_ROCKCHIP || COMPILE_TEST > + select MEDIA_CONTROLLER > + select VIDEO_V4L2_SUBDEV_API > + select VIDEOBUF2_DMA_CONTIG > + select VIDEOBUF2_VMALLOC > + select V4L2_FWNODE > + select GENERIC_PHY_MIPI_DPHY > + default n > + help > + Enable this to support the Image Signal Processing (ISP) module > + present in RK3399 SoCs. > + > + To compile this driver as a module, choose M here: the module > + will be called rockchip-isp1. > + Hmm, this ends up in the 'Memory-to-memory multimedia devices' section of the Kconfig, but this should be in 'V4L platform devices'. Can you rebase this series and the two binding related patches to the latest media_tree master? Everything else is in place, so it is time to move this driver out of staging. Regards, Hans > config VIDEO_ROCKCHIP_RGA > tristate "Rockchip Raster 2d Graphic Acceleration Unit" > depends on VIDEO_DEV && VIDEO_V4L2 > diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile > index 62b6cdc8c7300..b342714228db4 100644 > --- a/drivers/media/platform/Makefile > +++ b/drivers/media/platform/Makefile > @@ -52,6 +52,7 @@ obj-$(CONFIG_VIDEO_RENESAS_FDP1) += rcar_fdp1.o > obj-$(CONFIG_VIDEO_RENESAS_JPU) += rcar_jpu.o > obj-$(CONFIG_VIDEO_RENESAS_VSP1) += vsp1/ > > +obj-$(CONFIG_VIDEO_ROCKCHIP_ISP1) += rockchip/rkisp1/ > obj-$(CONFIG_VIDEO_ROCKCHIP_RGA) += rockchip/rga/ > > obj-y += omap/ > diff --git a/drivers/staging/media/rkisp1/Makefile b/drivers/media/platform/rockchip/rkisp1/Makefile > similarity index 100% > rename from drivers/staging/media/rkisp1/Makefile > rename to drivers/media/platform/rockchip/rkisp1/Makefile > diff --git a/drivers/staging/media/rkisp1/rkisp1-capture.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c > similarity index 100% > rename from drivers/staging/media/rkisp1/rkisp1-capture.c > rename to drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c > diff --git a/drivers/staging/media/rkisp1/rkisp1-common.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.c > similarity index 100% > rename from drivers/staging/media/rkisp1/rkisp1-common.c > rename to drivers/media/platform/rockchip/rkisp1/rkisp1-common.c > diff --git a/drivers/staging/media/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h > similarity index 99% > rename from drivers/staging/media/rkisp1/rkisp1-common.h > rename to drivers/media/platform/rockchip/rkisp1/rkisp1-common.h > index 692333c66f9d1..3a134e97161cb 100644 > --- a/drivers/staging/media/rkisp1/rkisp1-common.h > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h > @@ -13,6 +13,7 @@ > > #include <linux/clk.h> > #include <linux/mutex.h> > +#include <linux/rkisp1-config.h> > #include <media/media-device.h> > #include <media/media-entity.h> > #include <media/v4l2-ctrls.h> > @@ -20,7 +21,6 @@ > #include <media/videobuf2-v4l2.h> > > #include "rkisp1-regs.h" > -#include "uapi/rkisp1-config.h" > > /* > * flags on the 'direction' field in struct 'rkisp1_isp_mbus_info' that indicate > diff --git a/drivers/staging/media/rkisp1/rkisp1-dev.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c > similarity index 100% > rename from drivers/staging/media/rkisp1/rkisp1-dev.c > rename to drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c > diff --git a/drivers/staging/media/rkisp1/rkisp1-isp.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c > similarity index 100% > rename from drivers/staging/media/rkisp1/rkisp1-isp.c > rename to drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c > diff --git a/drivers/staging/media/rkisp1/rkisp1-params.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c > similarity index 100% > rename from drivers/staging/media/rkisp1/rkisp1-params.c > rename to drivers/media/platform/rockchip/rkisp1/rkisp1-params.c > diff --git a/drivers/staging/media/rkisp1/rkisp1-regs.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h > similarity index 100% > rename from drivers/staging/media/rkisp1/rkisp1-regs.h > rename to drivers/media/platform/rockchip/rkisp1/rkisp1-regs.h > diff --git a/drivers/staging/media/rkisp1/rkisp1-resizer.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c > similarity index 100% > rename from drivers/staging/media/rkisp1/rkisp1-resizer.c > rename to drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c > diff --git a/drivers/staging/media/rkisp1/rkisp1-stats.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-stats.c > similarity index 100% > rename from drivers/staging/media/rkisp1/rkisp1-stats.c > rename to drivers/media/platform/rockchip/rkisp1/rkisp1-stats.c > diff --git a/drivers/staging/media/Kconfig b/drivers/staging/media/Kconfig > index 747c6cf1d795e..e8996b1c3b351 100644 > --- a/drivers/staging/media/Kconfig > +++ b/drivers/staging/media/Kconfig > @@ -44,6 +44,4 @@ source "drivers/staging/media/tegra-video/Kconfig" > > source "drivers/staging/media/ipu3/Kconfig" > > -source "drivers/staging/media/rkisp1/Kconfig" > - > endif > diff --git a/drivers/staging/media/Makefile b/drivers/staging/media/Makefile > index b59571826ba69..24b5873ff7608 100644 > --- a/drivers/staging/media/Makefile > +++ b/drivers/staging/media/Makefile > @@ -10,5 +10,4 @@ obj-$(CONFIG_VIDEO_TEGRA) += tegra-video/ > obj-$(CONFIG_TEGRA_VDE) += tegra-vde/ > obj-$(CONFIG_VIDEO_HANTRO) += hantro/ > obj-$(CONFIG_VIDEO_IPU3_IMGU) += ipu3/ > -obj-$(CONFIG_VIDEO_ROCKCHIP_ISP1) += rkisp1/ > obj-$(CONFIG_VIDEO_ZORAN) += zoran/ > diff --git a/drivers/staging/media/rkisp1/Kconfig b/drivers/staging/media/rkisp1/Kconfig > deleted file mode 100644 > index 41f5def9ea442..0000000000000 > --- a/drivers/staging/media/rkisp1/Kconfig > +++ /dev/null > @@ -1,19 +0,0 @@ > -# SPDX-License-Identifier: GPL-2.0-only > - > -config VIDEO_ROCKCHIP_ISP1 > - tristate "Rockchip Image Signal Processing v1 Unit driver" > - depends on VIDEO_V4L2 && OF > - depends on ARCH_ROCKCHIP || COMPILE_TEST > - select MEDIA_CONTROLLER > - select VIDEO_V4L2_SUBDEV_API > - select VIDEOBUF2_DMA_CONTIG > - select VIDEOBUF2_VMALLOC > - select V4L2_FWNODE > - select GENERIC_PHY_MIPI_DPHY > - default n > - help > - Enable this to support the Image Signal Processing (ISP) module > - present in RK3399 SoCs. > - > - To compile this driver as a module, choose M here: the module > - will be called rockchip-isp1. > diff --git a/drivers/staging/media/rkisp1/TODO b/drivers/staging/media/rkisp1/TODO > deleted file mode 100644 > index bb5548cd6bd1b..0000000000000 > --- a/drivers/staging/media/rkisp1/TODO > +++ /dev/null > @@ -1,6 +0,0 @@ > -NOTES: > -* All v4l2-compliance test must pass. > -* Stats and params can be tested with libcamera and ChromiumOS stack. > - > -Please CC patches to Linux Media <linux-media@vger.kernel.org> and > -Helen Koike <helen.koike@collabora.com>. > diff --git a/drivers/staging/media/rkisp1/uapi/rkisp1-config.h b/include/uapi/linux/rkisp1-config.h > similarity index 100% > rename from drivers/staging/media/rkisp1/uapi/rkisp1-config.h > rename to include/uapi/linux/rkisp1-config.h >
On 2020-11-05 17:43, Hans Verkuil wrote: > On 30/10/2020 12:22, Helen Koike wrote: >> From: Shunqian Zheng <zhengsq@rock-chips.com> >> >> Add the Rockchip ISP1 specific processing parameter format >> V4L2_META_FMT_RK_ISP1_PARAMS and metadata format >> V4L2_META_FMT_RK_ISP1_STAT_3A for 3A. >> >> Signed-off-by: Shunqian Zheng <zhengsq@rock-chips.com> >> Signed-off-by: Jacob Chen <jacob2.chen@rock-chips.com> >> Signed-off-by: Helen Koike <helen.koike@collabora.com> >> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >> >> --- >> Hello, >> >> This patch is a continuation of: >> >> https://patchwork.kernel.org/project/linux-arm-kernel/patch/20191106120132.6876-2-helen.koike@collabora.com/ >> >> These formats are already documented under >> Documentation/userspace-api/media/v4l/pixfmt-meta-rkisp1.rst >> >> We had agreed to keep under >> drivers/staging/media/rkisp1/uapi/rkisp1-config.h while the driver was >> in staging, since we are moving it out of staging, I guess this is the >> time :) >> >> Regards, >> Helen >> --- >> drivers/media/v4l2-core/v4l2-ioctl.c | 2 ++ >> drivers/staging/media/rkisp1/uapi/rkisp1-config.h | 4 ---- >> include/uapi/linux/videodev2.h | 4 ++++ >> 3 files changed, 6 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c >> index eeff398fbdcc1..202597d031c6b 100644 >> --- a/drivers/media/v4l2-core/v4l2-ioctl.c >> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c >> @@ -1402,6 +1402,8 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt) >> case V4L2_META_FMT_UVC: descr = "UVC Payload Header Metadata"; break; >> case V4L2_META_FMT_D4XX: descr = "Intel D4xx UVC Metadata"; break; >> case V4L2_META_FMT_VIVID: descr = "Vivid Metadata"; break; >> + case V4L2_META_FMT_RK_ISP1_PARAMS: descr = "Rockchip ISP1 3A params"; break; >> + case V4L2_META_FMT_RK_ISP1_STAT_3A: descr = "Rockchip ISP1 3A statistics"; break; > > Use 'Params' and 'Statistics' to conform to the other descriptions. Drive-by observation: "parameters" is the exact same length as "statistics". I could understand abbreviating both, or neither, but the mix just looks decidedly odd. Robin. > > Regards, > > Hans > >> >> default: >> /* Compressed formats */ >> diff --git a/drivers/staging/media/rkisp1/uapi/rkisp1-config.h b/drivers/staging/media/rkisp1/uapi/rkisp1-config.h >> index 8d906cc7da8fc..6e449e7842605 100644 >> --- a/drivers/staging/media/rkisp1/uapi/rkisp1-config.h >> +++ b/drivers/staging/media/rkisp1/uapi/rkisp1-config.h >> @@ -9,10 +9,6 @@ >> >> #include <linux/types.h> >> >> -/* Vendor specific - used for RK_ISP1 camera sub-system */ >> -#define V4L2_META_FMT_RK_ISP1_PARAMS v4l2_fourcc('R', 'K', '1', 'P') /* Rockchip ISP1 params */ >> -#define V4L2_META_FMT_RK_ISP1_STAT_3A v4l2_fourcc('R', 'K', '1', 'S') /* Rockchip ISP1 3A statistics */ >> - >> /* Defect Pixel Cluster Detection */ >> #define RKISP1_CIF_ISP_MODULE_DPCC (1U << 0) >> /* Black Level Subtraction */ >> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h >> index 534eaa4d39bc8..c2e13ba81196b 100644 >> --- a/include/uapi/linux/videodev2.h >> +++ b/include/uapi/linux/videodev2.h >> @@ -770,6 +770,10 @@ struct v4l2_pix_format { >> #define V4L2_META_FMT_D4XX v4l2_fourcc('D', '4', 'X', 'X') /* D4XX Payload Header metadata */ >> #define V4L2_META_FMT_VIVID v4l2_fourcc('V', 'I', 'V', 'D') /* Vivid Metadata */ >> >> +/* Vendor specific - used for RK_ISP1 camera sub-system */ >> +#define V4L2_META_FMT_RK_ISP1_PARAMS v4l2_fourcc('R', 'K', '1', 'P') /* Rockchip ISP1 params */ >> +#define V4L2_META_FMT_RK_ISP1_STAT_3A v4l2_fourcc('R', 'K', '1', 'S') /* Rockchip ISP1 3A statistics */ >> + >> /* priv field value to indicates that subsequent fields are valid. */ >> #define V4L2_PIX_FMT_PRIV_MAGIC 0xfeedcafe >> >> > > > _______________________________________________ > Linux-rockchip mailing list > Linux-rockchip@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-rockchip >