All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] media: destage Hantro VPU driver
@ 2022-07-18 21:41 Ezequiel Garcia
  2022-07-18 21:41 ` [PATCH] hantro: Remove dedicated control documentation Ezequiel Garcia
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Ezequiel Garcia @ 2022-07-18 21:41 UTC (permalink / raw)
  To: linux-media, linux-kernel
  Cc: Hans Verkuil, Philipp Zabel, Nicolas Dufresne, Benjamin Gaignard,
	Jernej Skrabec, Tomasz Figa, Chris Healy, Ezequiel Garcia

The Hantro mainline driver has been used in production
since several years and was only kept as a staging driver
due the stateless CODEC controls.

Now that all the stateless CODEC controls have been moved
out of staging, graduate the driver as well.

Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
---
 MAINTAINERS                                                 | 2 +-
 drivers/media/platform/Kconfig                              | 1 +
 drivers/media/platform/Makefile                             | 1 +
 .../media/hantro => media/platform/verisilicon}/Kconfig     | 6 +++++-
 .../media/hantro => media/platform/verisilicon}/Makefile    | 0
 .../media/hantro => media/platform/verisilicon}/hantro.h    | 0
 .../hantro => media/platform/verisilicon}/hantro_drv.c      | 0
 .../media/hantro => media/platform/verisilicon}/hantro_g1.c | 0
 .../platform/verisilicon}/hantro_g1_h264_dec.c              | 0
 .../platform/verisilicon}/hantro_g1_mpeg2_dec.c             | 0
 .../hantro => media/platform/verisilicon}/hantro_g1_regs.h  | 0
 .../platform/verisilicon}/hantro_g1_vp8_dec.c               | 0
 .../media/hantro => media/platform/verisilicon}/hantro_g2.c | 0
 .../platform/verisilicon}/hantro_g2_hevc_dec.c              | 0
 .../hantro => media/platform/verisilicon}/hantro_g2_regs.h  | 0
 .../platform/verisilicon}/hantro_g2_vp9_dec.c               | 0
 .../platform/verisilicon}/hantro_h1_jpeg_enc.c              | 0
 .../hantro => media/platform/verisilicon}/hantro_h1_regs.h  | 0
 .../hantro => media/platform/verisilicon}/hantro_h264.c     | 0
 .../hantro => media/platform/verisilicon}/hantro_hevc.c     | 0
 .../media/hantro => media/platform/verisilicon}/hantro_hw.h | 0
 .../hantro => media/platform/verisilicon}/hantro_jpeg.c     | 0
 .../hantro => media/platform/verisilicon}/hantro_jpeg.h     | 0
 .../hantro => media/platform/verisilicon}/hantro_mpeg2.c    | 0
 .../hantro => media/platform/verisilicon}/hantro_postproc.c | 0
 .../hantro => media/platform/verisilicon}/hantro_v4l2.c     | 0
 .../hantro => media/platform/verisilicon}/hantro_v4l2.h     | 0
 .../hantro => media/platform/verisilicon}/hantro_vp8.c      | 0
 .../hantro => media/platform/verisilicon}/hantro_vp9.c      | 0
 .../hantro => media/platform/verisilicon}/hantro_vp9.h      | 0
 .../hantro => media/platform/verisilicon}/imx8m_vpu_hw.c    | 0
 .../platform/verisilicon}/rockchip_vpu2_hw_h264_dec.c       | 0
 .../platform/verisilicon}/rockchip_vpu2_hw_jpeg_enc.c       | 0
 .../platform/verisilicon}/rockchip_vpu2_hw_mpeg2_dec.c      | 0
 .../platform/verisilicon}/rockchip_vpu2_hw_vp8_dec.c        | 0
 .../platform/verisilicon}/rockchip_vpu2_regs.h              | 0
 .../hantro => media/platform/verisilicon}/rockchip_vpu_hw.c | 0
 .../hantro => media/platform/verisilicon}/sama5d4_vdec_hw.c | 0
 .../hantro => media/platform/verisilicon}/sunxi_vpu_hw.c    | 0
 drivers/staging/media/Kconfig                               | 2 --
 drivers/staging/media/Makefile                              | 1 -
 drivers/staging/media/hantro/TODO                           | 2 --
 42 files changed, 8 insertions(+), 7 deletions(-)
 rename drivers/{staging/media/hantro => media/platform/verisilicon}/Kconfig (91%)
 rename drivers/{staging/media/hantro => media/platform/verisilicon}/Makefile (100%)
 rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro.h (100%)
 rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_drv.c (100%)
 rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_g1.c (100%)
 rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_g1_h264_dec.c (100%)
 rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_g1_mpeg2_dec.c (100%)
 rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_g1_regs.h (100%)
 rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_g1_vp8_dec.c (100%)
 rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_g2.c (100%)
 rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_g2_hevc_dec.c (100%)
 rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_g2_regs.h (100%)
 rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_g2_vp9_dec.c (100%)
 rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_h1_jpeg_enc.c (100%)
 rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_h1_regs.h (100%)
 rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_h264.c (100%)
 rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_hevc.c (100%)
 rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_hw.h (100%)
 rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_jpeg.c (100%)
 rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_jpeg.h (100%)
 rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_mpeg2.c (100%)
 rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_postproc.c (100%)
 rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_v4l2.c (100%)
 rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_v4l2.h (100%)
 rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_vp8.c (100%)
 rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_vp9.c (100%)
 rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_vp9.h (100%)
 rename drivers/{staging/media/hantro => media/platform/verisilicon}/imx8m_vpu_hw.c (100%)
 rename drivers/{staging/media/hantro => media/platform/verisilicon}/rockchip_vpu2_hw_h264_dec.c (100%)
 rename drivers/{staging/media/hantro => media/platform/verisilicon}/rockchip_vpu2_hw_jpeg_enc.c (100%)
 rename drivers/{staging/media/hantro => media/platform/verisilicon}/rockchip_vpu2_hw_mpeg2_dec.c (100%)
 rename drivers/{staging/media/hantro => media/platform/verisilicon}/rockchip_vpu2_hw_vp8_dec.c (100%)
 rename drivers/{staging/media/hantro => media/platform/verisilicon}/rockchip_vpu2_regs.h (100%)
 rename drivers/{staging/media/hantro => media/platform/verisilicon}/rockchip_vpu_hw.c (100%)
 rename drivers/{staging/media/hantro => media/platform/verisilicon}/sama5d4_vdec_hw.c (100%)
 rename drivers/{staging/media/hantro => media/platform/verisilicon}/sunxi_vpu_hw.c (100%)
 delete mode 100644 drivers/staging/media/hantro/TODO

diff --git a/MAINTAINERS b/MAINTAINERS
index fee2ac9a98f4..07ed4aaf545d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8686,7 +8686,7 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
 F:	Documentation/devicetree/bindings/media/rockchip,rk3568-vepu.yaml
 F:	Documentation/devicetree/bindings/media/rockchip-vpu.yaml
-F:	drivers/staging/media/hantro/
+F:	drivers/media/platform/verisilicon/
 
 HARD DRIVE ACTIVE PROTECTION SYSTEM (HDAPS) DRIVER
 M:	Frank Seidel <frank@f-seidel.de>
diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index f1056ceaf5a8..a9334263fa9b 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -81,6 +81,7 @@ source "drivers/media/platform/samsung/Kconfig"
 source "drivers/media/platform/st/Kconfig"
 source "drivers/media/platform/sunxi/Kconfig"
 source "drivers/media/platform/ti/Kconfig"
+source "drivers/media/platform/verisilicon/Kconfig"
 source "drivers/media/platform/via/Kconfig"
 source "drivers/media/platform/xilinx/Kconfig"
 
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index a881e97bae95..a91f42024273 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -24,6 +24,7 @@ obj-y += samsung/
 obj-y += st/
 obj-y += sunxi/
 obj-y += ti/
+obj-y += verisilicon/
 obj-y += via/
 obj-y += xilinx/
 
diff --git a/drivers/staging/media/hantro/Kconfig b/drivers/media/platform/verisilicon/Kconfig
similarity index 91%
rename from drivers/staging/media/hantro/Kconfig
rename to drivers/media/platform/verisilicon/Kconfig
index 0172a6822ec2..e65b836b9d78 100644
--- a/drivers/staging/media/hantro/Kconfig
+++ b/drivers/media/platform/verisilicon/Kconfig
@@ -1,7 +1,11 @@
-# SPDX-License-Identifier: GPL-2.0
+# SPDX-License-Identifier: GPL-2.0-only
+
+comment "Verisilicon media platform drivers"
+
 config VIDEO_HANTRO
 	tristate "Hantro VPU driver"
 	depends on ARCH_MXC || ARCH_ROCKCHIP || ARCH_AT91 || ARCH_SUNXI || COMPILE_TEST
+	depends on V4L_MEM2MEM_DRIVERS
 	depends on VIDEO_DEV
 	select MEDIA_CONTROLLER
 	select MEDIA_CONTROLLER_REQUEST_API
diff --git a/drivers/staging/media/hantro/Makefile b/drivers/media/platform/verisilicon/Makefile
similarity index 100%
rename from drivers/staging/media/hantro/Makefile
rename to drivers/media/platform/verisilicon/Makefile
diff --git a/drivers/staging/media/hantro/hantro.h b/drivers/media/platform/verisilicon/hantro.h
similarity index 100%
rename from drivers/staging/media/hantro/hantro.h
rename to drivers/media/platform/verisilicon/hantro.h
diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/media/platform/verisilicon/hantro_drv.c
similarity index 100%
rename from drivers/staging/media/hantro/hantro_drv.c
rename to drivers/media/platform/verisilicon/hantro_drv.c
diff --git a/drivers/staging/media/hantro/hantro_g1.c b/drivers/media/platform/verisilicon/hantro_g1.c
similarity index 100%
rename from drivers/staging/media/hantro/hantro_g1.c
rename to drivers/media/platform/verisilicon/hantro_g1.c
diff --git a/drivers/staging/media/hantro/hantro_g1_h264_dec.c b/drivers/media/platform/verisilicon/hantro_g1_h264_dec.c
similarity index 100%
rename from drivers/staging/media/hantro/hantro_g1_h264_dec.c
rename to drivers/media/platform/verisilicon/hantro_g1_h264_dec.c
diff --git a/drivers/staging/media/hantro/hantro_g1_mpeg2_dec.c b/drivers/media/platform/verisilicon/hantro_g1_mpeg2_dec.c
similarity index 100%
rename from drivers/staging/media/hantro/hantro_g1_mpeg2_dec.c
rename to drivers/media/platform/verisilicon/hantro_g1_mpeg2_dec.c
diff --git a/drivers/staging/media/hantro/hantro_g1_regs.h b/drivers/media/platform/verisilicon/hantro_g1_regs.h
similarity index 100%
rename from drivers/staging/media/hantro/hantro_g1_regs.h
rename to drivers/media/platform/verisilicon/hantro_g1_regs.h
diff --git a/drivers/staging/media/hantro/hantro_g1_vp8_dec.c b/drivers/media/platform/verisilicon/hantro_g1_vp8_dec.c
similarity index 100%
rename from drivers/staging/media/hantro/hantro_g1_vp8_dec.c
rename to drivers/media/platform/verisilicon/hantro_g1_vp8_dec.c
diff --git a/drivers/staging/media/hantro/hantro_g2.c b/drivers/media/platform/verisilicon/hantro_g2.c
similarity index 100%
rename from drivers/staging/media/hantro/hantro_g2.c
rename to drivers/media/platform/verisilicon/hantro_g2.c
diff --git a/drivers/staging/media/hantro/hantro_g2_hevc_dec.c b/drivers/media/platform/verisilicon/hantro_g2_hevc_dec.c
similarity index 100%
rename from drivers/staging/media/hantro/hantro_g2_hevc_dec.c
rename to drivers/media/platform/verisilicon/hantro_g2_hevc_dec.c
diff --git a/drivers/staging/media/hantro/hantro_g2_regs.h b/drivers/media/platform/verisilicon/hantro_g2_regs.h
similarity index 100%
rename from drivers/staging/media/hantro/hantro_g2_regs.h
rename to drivers/media/platform/verisilicon/hantro_g2_regs.h
diff --git a/drivers/staging/media/hantro/hantro_g2_vp9_dec.c b/drivers/media/platform/verisilicon/hantro_g2_vp9_dec.c
similarity index 100%
rename from drivers/staging/media/hantro/hantro_g2_vp9_dec.c
rename to drivers/media/platform/verisilicon/hantro_g2_vp9_dec.c
diff --git a/drivers/staging/media/hantro/hantro_h1_jpeg_enc.c b/drivers/media/platform/verisilicon/hantro_h1_jpeg_enc.c
similarity index 100%
rename from drivers/staging/media/hantro/hantro_h1_jpeg_enc.c
rename to drivers/media/platform/verisilicon/hantro_h1_jpeg_enc.c
diff --git a/drivers/staging/media/hantro/hantro_h1_regs.h b/drivers/media/platform/verisilicon/hantro_h1_regs.h
similarity index 100%
rename from drivers/staging/media/hantro/hantro_h1_regs.h
rename to drivers/media/platform/verisilicon/hantro_h1_regs.h
diff --git a/drivers/staging/media/hantro/hantro_h264.c b/drivers/media/platform/verisilicon/hantro_h264.c
similarity index 100%
rename from drivers/staging/media/hantro/hantro_h264.c
rename to drivers/media/platform/verisilicon/hantro_h264.c
diff --git a/drivers/staging/media/hantro/hantro_hevc.c b/drivers/media/platform/verisilicon/hantro_hevc.c
similarity index 100%
rename from drivers/staging/media/hantro/hantro_hevc.c
rename to drivers/media/platform/verisilicon/hantro_hevc.c
diff --git a/drivers/staging/media/hantro/hantro_hw.h b/drivers/media/platform/verisilicon/hantro_hw.h
similarity index 100%
rename from drivers/staging/media/hantro/hantro_hw.h
rename to drivers/media/platform/verisilicon/hantro_hw.h
diff --git a/drivers/staging/media/hantro/hantro_jpeg.c b/drivers/media/platform/verisilicon/hantro_jpeg.c
similarity index 100%
rename from drivers/staging/media/hantro/hantro_jpeg.c
rename to drivers/media/platform/verisilicon/hantro_jpeg.c
diff --git a/drivers/staging/media/hantro/hantro_jpeg.h b/drivers/media/platform/verisilicon/hantro_jpeg.h
similarity index 100%
rename from drivers/staging/media/hantro/hantro_jpeg.h
rename to drivers/media/platform/verisilicon/hantro_jpeg.h
diff --git a/drivers/staging/media/hantro/hantro_mpeg2.c b/drivers/media/platform/verisilicon/hantro_mpeg2.c
similarity index 100%
rename from drivers/staging/media/hantro/hantro_mpeg2.c
rename to drivers/media/platform/verisilicon/hantro_mpeg2.c
diff --git a/drivers/staging/media/hantro/hantro_postproc.c b/drivers/media/platform/verisilicon/hantro_postproc.c
similarity index 100%
rename from drivers/staging/media/hantro/hantro_postproc.c
rename to drivers/media/platform/verisilicon/hantro_postproc.c
diff --git a/drivers/staging/media/hantro/hantro_v4l2.c b/drivers/media/platform/verisilicon/hantro_v4l2.c
similarity index 100%
rename from drivers/staging/media/hantro/hantro_v4l2.c
rename to drivers/media/platform/verisilicon/hantro_v4l2.c
diff --git a/drivers/staging/media/hantro/hantro_v4l2.h b/drivers/media/platform/verisilicon/hantro_v4l2.h
similarity index 100%
rename from drivers/staging/media/hantro/hantro_v4l2.h
rename to drivers/media/platform/verisilicon/hantro_v4l2.h
diff --git a/drivers/staging/media/hantro/hantro_vp8.c b/drivers/media/platform/verisilicon/hantro_vp8.c
similarity index 100%
rename from drivers/staging/media/hantro/hantro_vp8.c
rename to drivers/media/platform/verisilicon/hantro_vp8.c
diff --git a/drivers/staging/media/hantro/hantro_vp9.c b/drivers/media/platform/verisilicon/hantro_vp9.c
similarity index 100%
rename from drivers/staging/media/hantro/hantro_vp9.c
rename to drivers/media/platform/verisilicon/hantro_vp9.c
diff --git a/drivers/staging/media/hantro/hantro_vp9.h b/drivers/media/platform/verisilicon/hantro_vp9.h
similarity index 100%
rename from drivers/staging/media/hantro/hantro_vp9.h
rename to drivers/media/platform/verisilicon/hantro_vp9.h
diff --git a/drivers/staging/media/hantro/imx8m_vpu_hw.c b/drivers/media/platform/verisilicon/imx8m_vpu_hw.c
similarity index 100%
rename from drivers/staging/media/hantro/imx8m_vpu_hw.c
rename to drivers/media/platform/verisilicon/imx8m_vpu_hw.c
diff --git a/drivers/staging/media/hantro/rockchip_vpu2_hw_h264_dec.c b/drivers/media/platform/verisilicon/rockchip_vpu2_hw_h264_dec.c
similarity index 100%
rename from drivers/staging/media/hantro/rockchip_vpu2_hw_h264_dec.c
rename to drivers/media/platform/verisilicon/rockchip_vpu2_hw_h264_dec.c
diff --git a/drivers/staging/media/hantro/rockchip_vpu2_hw_jpeg_enc.c b/drivers/media/platform/verisilicon/rockchip_vpu2_hw_jpeg_enc.c
similarity index 100%
rename from drivers/staging/media/hantro/rockchip_vpu2_hw_jpeg_enc.c
rename to drivers/media/platform/verisilicon/rockchip_vpu2_hw_jpeg_enc.c
diff --git a/drivers/staging/media/hantro/rockchip_vpu2_hw_mpeg2_dec.c b/drivers/media/platform/verisilicon/rockchip_vpu2_hw_mpeg2_dec.c
similarity index 100%
rename from drivers/staging/media/hantro/rockchip_vpu2_hw_mpeg2_dec.c
rename to drivers/media/platform/verisilicon/rockchip_vpu2_hw_mpeg2_dec.c
diff --git a/drivers/staging/media/hantro/rockchip_vpu2_hw_vp8_dec.c b/drivers/media/platform/verisilicon/rockchip_vpu2_hw_vp8_dec.c
similarity index 100%
rename from drivers/staging/media/hantro/rockchip_vpu2_hw_vp8_dec.c
rename to drivers/media/platform/verisilicon/rockchip_vpu2_hw_vp8_dec.c
diff --git a/drivers/staging/media/hantro/rockchip_vpu2_regs.h b/drivers/media/platform/verisilicon/rockchip_vpu2_regs.h
similarity index 100%
rename from drivers/staging/media/hantro/rockchip_vpu2_regs.h
rename to drivers/media/platform/verisilicon/rockchip_vpu2_regs.h
diff --git a/drivers/staging/media/hantro/rockchip_vpu_hw.c b/drivers/media/platform/verisilicon/rockchip_vpu_hw.c
similarity index 100%
rename from drivers/staging/media/hantro/rockchip_vpu_hw.c
rename to drivers/media/platform/verisilicon/rockchip_vpu_hw.c
diff --git a/drivers/staging/media/hantro/sama5d4_vdec_hw.c b/drivers/media/platform/verisilicon/sama5d4_vdec_hw.c
similarity index 100%
rename from drivers/staging/media/hantro/sama5d4_vdec_hw.c
rename to drivers/media/platform/verisilicon/sama5d4_vdec_hw.c
diff --git a/drivers/staging/media/hantro/sunxi_vpu_hw.c b/drivers/media/platform/verisilicon/sunxi_vpu_hw.c
similarity index 100%
rename from drivers/staging/media/hantro/sunxi_vpu_hw.c
rename to drivers/media/platform/verisilicon/sunxi_vpu_hw.c
diff --git a/drivers/staging/media/Kconfig b/drivers/staging/media/Kconfig
index 421ce9dbf44c..2b8ad4cc8ac5 100644
--- a/drivers/staging/media/Kconfig
+++ b/drivers/staging/media/Kconfig
@@ -24,8 +24,6 @@ source "drivers/staging/media/atomisp/Kconfig"
 
 source "drivers/staging/media/av7110/Kconfig"
 
-source "drivers/staging/media/hantro/Kconfig"
-
 source "drivers/staging/media/imx/Kconfig"
 
 source "drivers/staging/media/ipu3/Kconfig"
diff --git a/drivers/staging/media/Makefile b/drivers/staging/media/Makefile
index 950e96f10aad..a12d0bb9dca3 100644
--- a/drivers/staging/media/Makefile
+++ b/drivers/staging/media/Makefile
@@ -8,7 +8,6 @@ obj-$(CONFIG_VIDEO_ROCKCHIP_VDEC)	+= rkvdec/
 obj-$(CONFIG_VIDEO_STKWEBCAM)	+= stkwebcam/
 obj-$(CONFIG_VIDEO_SUNXI)	+= sunxi/
 obj-$(CONFIG_VIDEO_TEGRA)	+= tegra-video/
-obj-$(CONFIG_VIDEO_HANTRO)	+= hantro/
 obj-$(CONFIG_VIDEO_IPU3_IMGU)	+= ipu3/
 obj-$(CONFIG_VIDEO_ZORAN)	+= zoran/
 obj-$(CONFIG_DVB_AV7110)	+= av7110/
diff --git a/drivers/staging/media/hantro/TODO b/drivers/staging/media/hantro/TODO
deleted file mode 100644
index 8483ff482146..000000000000
--- a/drivers/staging/media/hantro/TODO
+++ /dev/null
@@ -1,2 +0,0 @@
-The V4L controls for the HEVC CODEC are not yet part of the stable uABI,
-we are keeping this driver in staging until the HEVC uABI has been merged.
-- 
2.34.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH] hantro: Remove dedicated control documentation
  2022-07-18 21:41 [PATCH] media: destage Hantro VPU driver Ezequiel Garcia
@ 2022-07-18 21:41 ` Ezequiel Garcia
  2022-07-19 19:34   ` Nicolas Dufresne
  2022-07-18 21:41 ` [PATCH] hantro: Remove incorrect HEVC SPS validation Ezequiel Garcia
  2022-07-19 19:33 ` [PATCH] media: destage Hantro VPU driver Nicolas Dufresne
  2 siblings, 1 reply; 10+ messages in thread
From: Ezequiel Garcia @ 2022-07-18 21:41 UTC (permalink / raw)
  To: linux-media, linux-kernel
  Cc: Hans Verkuil, Philipp Zabel, Nicolas Dufresne, Benjamin Gaignard,
	Jernej Skrabec, Tomasz Figa, Chris Healy, Ezequiel Garcia

The dedicated control required by the HEVC support
was removed, and the driver now calculates the value
internally. Remove the ad-hoc documentation as well.

Fixes: 3360755ef89ab ("media: hantro: Stop using Hantro dedicated control")
Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
---
 .../userspace-api/media/drivers/hantro.rst    | 19 -------------------
 1 file changed, 19 deletions(-)
 delete mode 100644 Documentation/userspace-api/media/drivers/hantro.rst

diff --git a/Documentation/userspace-api/media/drivers/hantro.rst b/Documentation/userspace-api/media/drivers/hantro.rst
deleted file mode 100644
index cd9754b4e005..000000000000
--- a/Documentation/userspace-api/media/drivers/hantro.rst
+++ /dev/null
@@ -1,19 +0,0 @@
-.. SPDX-License-Identifier: GPL-2.0
-
-Hantro video decoder driver
-===========================
-
-The Hantro video decoder driver implements the following driver-specific controls:
-
-``V4L2_CID_HANTRO_HEVC_SLICE_HEADER_SKIP (integer)``
-    Specifies to Hantro HEVC video decoder driver the number of data (in bits) to
-    skip in the slice segment header.
-    If non-IDR, the bits to be skipped go from syntax element "pic_output_flag"
-    to before syntax element "slice_temporal_mvp_enabled_flag".
-    If IDR, the skipped bits are just "pic_output_flag"
-    (separate_colour_plane_flag is not supported).
-
-.. note::
-
-        This control is not yet part of the public kernel API and
-        it is expected to change.
-- 
2.34.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH] hantro: Remove incorrect HEVC SPS validation
  2022-07-18 21:41 [PATCH] media: destage Hantro VPU driver Ezequiel Garcia
  2022-07-18 21:41 ` [PATCH] hantro: Remove dedicated control documentation Ezequiel Garcia
@ 2022-07-18 21:41 ` Ezequiel Garcia
  2022-07-18 21:48   ` Ezequiel Garcia
  2022-07-19 19:33 ` [PATCH] media: destage Hantro VPU driver Nicolas Dufresne
  2 siblings, 1 reply; 10+ messages in thread
From: Ezequiel Garcia @ 2022-07-18 21:41 UTC (permalink / raw)
  To: linux-media, linux-kernel
  Cc: Hans Verkuil, Philipp Zabel, Nicolas Dufresne, Benjamin Gaignard,
	Jernej Skrabec, Tomasz Figa, Chris Healy, Ezequiel Garcia

Currently, the driver tries to validat the HEVC SPS
against the CAPTURE queue format (i.e. the decoded format).
This is not correct, because typically the SPS control is set
before the CAPTURE queue is negotiated.

Fixes: 135ad96cb4d6b ("media: hantro: Be more accurate on pixel formats step_width constraints")
Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
---
 drivers/media/platform/verisilicon/hantro_drv.c  | 12 ++++++------
 drivers/media/platform/verisilicon/hantro_hevc.c |  9 +--------
 drivers/media/platform/verisilicon/hantro_hw.h   |  1 -
 3 files changed, 7 insertions(+), 15 deletions(-)

diff --git a/drivers/media/platform/verisilicon/hantro_drv.c b/drivers/media/platform/verisilicon/hantro_drv.c
index e5fc0a99b728..2036f72eeb4a 100644
--- a/drivers/media/platform/verisilicon/hantro_drv.c
+++ b/drivers/media/platform/verisilicon/hantro_drv.c
@@ -251,11 +251,6 @@ queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq)
 
 static int hantro_try_ctrl(struct v4l2_ctrl *ctrl)
 {
-	struct hantro_ctx *ctx;
-
-	ctx = container_of(ctrl->handler,
-			   struct hantro_ctx, ctrl_handler);
-
 	if (ctrl->id == V4L2_CID_STATELESS_H264_SPS) {
 		const struct v4l2_ctrl_h264_sps *sps = ctrl->p_new.p_h264_sps;
 
@@ -271,7 +266,12 @@ static int hantro_try_ctrl(struct v4l2_ctrl *ctrl)
 	} else if (ctrl->id == V4L2_CID_STATELESS_HEVC_SPS) {
 		const struct v4l2_ctrl_hevc_sps *sps = ctrl->p_new.p_hevc_sps;
 
-		return hantro_hevc_validate_sps(ctx, sps);
+		if (sps->bit_depth_luma_minus8 != sps->bit_depth_chroma_minus8)
+			/* Luma and chroma bit depth mismatch */
+			return -EINVAL;
+		if (sps->bit_depth_luma_minus8 != 0)
+			/* Only 8-bit is supported */
+			return -EINVAL;
 	} else if (ctrl->id == V4L2_CID_STATELESS_VP9_FRAME) {
 		const struct v4l2_ctrl_vp9_frame *dec_params = ctrl->p_new.p_vp9_frame;
 
diff --git a/drivers/media/platform/verisilicon/hantro_hevc.c b/drivers/media/platform/verisilicon/hantro_hevc.c
index 5984c5fa6f83..b990bc98164c 100644
--- a/drivers/media/platform/verisilicon/hantro_hevc.c
+++ b/drivers/media/platform/verisilicon/hantro_hevc.c
@@ -154,15 +154,8 @@ static int tile_buffer_reallocate(struct hantro_ctx *ctx)
 	return -ENOMEM;
 }
 
-int hantro_hevc_validate_sps(struct hantro_ctx *ctx, const struct v4l2_ctrl_hevc_sps *sps)
+static int hantro_hevc_validate_sps(struct hantro_ctx *ctx, const struct v4l2_ctrl_hevc_sps *sps)
 {
-	if (sps->bit_depth_luma_minus8 != sps->bit_depth_chroma_minus8)
-		/* Luma and chroma bit depth mismatch */
-		return -EINVAL;
-	if (sps->bit_depth_luma_minus8 != 0)
-		/* Only 8-bit is supported */
-		return -EINVAL;
-
 	/*
 	 * for tile pixel format check if the width and height match
 	 * hardware constraints
diff --git a/drivers/media/platform/verisilicon/hantro_hw.h b/drivers/media/platform/verisilicon/hantro_hw.h
index 762631d15fdc..e83f0c523a30 100644
--- a/drivers/media/platform/verisilicon/hantro_hw.h
+++ b/drivers/media/platform/verisilicon/hantro_hw.h
@@ -360,7 +360,6 @@ int hantro_hevc_dec_prepare_run(struct hantro_ctx *ctx);
 void hantro_hevc_ref_init(struct hantro_ctx *ctx);
 dma_addr_t hantro_hevc_get_ref_buf(struct hantro_ctx *ctx, s32 poc);
 int hantro_hevc_add_ref_buf(struct hantro_ctx *ctx, int poc, dma_addr_t addr);
-int hantro_hevc_validate_sps(struct hantro_ctx *ctx, const struct v4l2_ctrl_hevc_sps *sps);
 
 
 static inline unsigned short hantro_vp9_num_sbs(unsigned short dimension)
-- 
2.34.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] hantro: Remove incorrect HEVC SPS validation
  2022-07-18 21:41 ` [PATCH] hantro: Remove incorrect HEVC SPS validation Ezequiel Garcia
@ 2022-07-18 21:48   ` Ezequiel Garcia
  0 siblings, 0 replies; 10+ messages in thread
From: Ezequiel Garcia @ 2022-07-18 21:48 UTC (permalink / raw)
  To: linux-media, linux-kernel
  Cc: Hans Verkuil, Philipp Zabel, Nicolas Dufresne, Benjamin Gaignard,
	Jernej Skrabec, Tomasz Figa, Chris Healy

On Mon, Jul 18, 2022 at 06:41:23PM -0300, Ezequiel Garcia wrote:
> Currently, the driver tries to validat the HEVC SPS
> against the CAPTURE queue format (i.e. the decoded format).
> This is not correct, because typically the SPS control is set
> before the CAPTURE queue is negotiated.
> 
> Fixes: 135ad96cb4d6b ("media: hantro: Be more accurate on pixel formats step_width constraints")
> Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>

This is actually a v2. I've chosen to move the
control checks back to try_ctrl, but leave the tiled
format alignment check.

This ALIGN() check can be optimized, moving the alignment
to the TRY_FMT step (instead of failing at device_run).
However, this is a minor optimization, so I'd like to
avoid the scope creep, and pospone the changes regarding
the tiled format resolution alignment.

Thanks,
Ezequiel

> ---
>  drivers/media/platform/verisilicon/hantro_drv.c  | 12 ++++++------
>  drivers/media/platform/verisilicon/hantro_hevc.c |  9 +--------
>  drivers/media/platform/verisilicon/hantro_hw.h   |  1 -
>  3 files changed, 7 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/media/platform/verisilicon/hantro_drv.c b/drivers/media/platform/verisilicon/hantro_drv.c
> index e5fc0a99b728..2036f72eeb4a 100644
> --- a/drivers/media/platform/verisilicon/hantro_drv.c
> +++ b/drivers/media/platform/verisilicon/hantro_drv.c
> @@ -251,11 +251,6 @@ queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq)
>  
>  static int hantro_try_ctrl(struct v4l2_ctrl *ctrl)
>  {
> -	struct hantro_ctx *ctx;
> -
> -	ctx = container_of(ctrl->handler,
> -			   struct hantro_ctx, ctrl_handler);
> -
>  	if (ctrl->id == V4L2_CID_STATELESS_H264_SPS) {
>  		const struct v4l2_ctrl_h264_sps *sps = ctrl->p_new.p_h264_sps;
>  
> @@ -271,7 +266,12 @@ static int hantro_try_ctrl(struct v4l2_ctrl *ctrl)
>  	} else if (ctrl->id == V4L2_CID_STATELESS_HEVC_SPS) {
>  		const struct v4l2_ctrl_hevc_sps *sps = ctrl->p_new.p_hevc_sps;
>  
> -		return hantro_hevc_validate_sps(ctx, sps);
> +		if (sps->bit_depth_luma_minus8 != sps->bit_depth_chroma_minus8)
> +			/* Luma and chroma bit depth mismatch */
> +			return -EINVAL;
> +		if (sps->bit_depth_luma_minus8 != 0)
> +			/* Only 8-bit is supported */
> +			return -EINVAL;
>  	} else if (ctrl->id == V4L2_CID_STATELESS_VP9_FRAME) {
>  		const struct v4l2_ctrl_vp9_frame *dec_params = ctrl->p_new.p_vp9_frame;
>  
> diff --git a/drivers/media/platform/verisilicon/hantro_hevc.c b/drivers/media/platform/verisilicon/hantro_hevc.c
> index 5984c5fa6f83..b990bc98164c 100644
> --- a/drivers/media/platform/verisilicon/hantro_hevc.c
> +++ b/drivers/media/platform/verisilicon/hantro_hevc.c
> @@ -154,15 +154,8 @@ static int tile_buffer_reallocate(struct hantro_ctx *ctx)
>  	return -ENOMEM;
>  }
>  
> -int hantro_hevc_validate_sps(struct hantro_ctx *ctx, const struct v4l2_ctrl_hevc_sps *sps)
> +static int hantro_hevc_validate_sps(struct hantro_ctx *ctx, const struct v4l2_ctrl_hevc_sps *sps)
>  {
> -	if (sps->bit_depth_luma_minus8 != sps->bit_depth_chroma_minus8)
> -		/* Luma and chroma bit depth mismatch */
> -		return -EINVAL;
> -	if (sps->bit_depth_luma_minus8 != 0)
> -		/* Only 8-bit is supported */
> -		return -EINVAL;
> -
>  	/*
>  	 * for tile pixel format check if the width and height match
>  	 * hardware constraints
> diff --git a/drivers/media/platform/verisilicon/hantro_hw.h b/drivers/media/platform/verisilicon/hantro_hw.h
> index 762631d15fdc..e83f0c523a30 100644
> --- a/drivers/media/platform/verisilicon/hantro_hw.h
> +++ b/drivers/media/platform/verisilicon/hantro_hw.h
> @@ -360,7 +360,6 @@ int hantro_hevc_dec_prepare_run(struct hantro_ctx *ctx);
>  void hantro_hevc_ref_init(struct hantro_ctx *ctx);
>  dma_addr_t hantro_hevc_get_ref_buf(struct hantro_ctx *ctx, s32 poc);
>  int hantro_hevc_add_ref_buf(struct hantro_ctx *ctx, int poc, dma_addr_t addr);
> -int hantro_hevc_validate_sps(struct hantro_ctx *ctx, const struct v4l2_ctrl_hevc_sps *sps);
>  
>  
>  static inline unsigned short hantro_vp9_num_sbs(unsigned short dimension)
> -- 
> 2.34.3
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] media: destage Hantro VPU driver
  2022-07-18 21:41 [PATCH] media: destage Hantro VPU driver Ezequiel Garcia
  2022-07-18 21:41 ` [PATCH] hantro: Remove dedicated control documentation Ezequiel Garcia
  2022-07-18 21:41 ` [PATCH] hantro: Remove incorrect HEVC SPS validation Ezequiel Garcia
@ 2022-07-19 19:33 ` Nicolas Dufresne
  2 siblings, 0 replies; 10+ messages in thread
From: Nicolas Dufresne @ 2022-07-19 19:33 UTC (permalink / raw)
  To: Ezequiel Garcia, linux-media, linux-kernel
  Cc: Hans Verkuil, Philipp Zabel, Benjamin Gaignard, Jernej Skrabec,
	Tomasz Figa, Chris Healy

Le lundi 18 juillet 2022 à 18:41 -0300, Ezequiel Garcia a écrit :
> The Hantro mainline driver has been used in production
> since several years and was only kept as a staging driver
> due the stateless CODEC controls.
> 
> Now that all the stateless CODEC controls have been moved
> out of staging, graduate the driver as well.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>

Acked-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

> ---
>  MAINTAINERS                                                 | 2 +-
>  drivers/media/platform/Kconfig                              | 1 +
>  drivers/media/platform/Makefile                             | 1 +
>  .../media/hantro => media/platform/verisilicon}/Kconfig     | 6 +++++-
>  .../media/hantro => media/platform/verisilicon}/Makefile    | 0
>  .../media/hantro => media/platform/verisilicon}/hantro.h    | 0
>  .../hantro => media/platform/verisilicon}/hantro_drv.c      | 0
>  .../media/hantro => media/platform/verisilicon}/hantro_g1.c | 0
>  .../platform/verisilicon}/hantro_g1_h264_dec.c              | 0
>  .../platform/verisilicon}/hantro_g1_mpeg2_dec.c             | 0
>  .../hantro => media/platform/verisilicon}/hantro_g1_regs.h  | 0
>  .../platform/verisilicon}/hantro_g1_vp8_dec.c               | 0
>  .../media/hantro => media/platform/verisilicon}/hantro_g2.c | 0
>  .../platform/verisilicon}/hantro_g2_hevc_dec.c              | 0
>  .../hantro => media/platform/verisilicon}/hantro_g2_regs.h  | 0
>  .../platform/verisilicon}/hantro_g2_vp9_dec.c               | 0
>  .../platform/verisilicon}/hantro_h1_jpeg_enc.c              | 0
>  .../hantro => media/platform/verisilicon}/hantro_h1_regs.h  | 0
>  .../hantro => media/platform/verisilicon}/hantro_h264.c     | 0
>  .../hantro => media/platform/verisilicon}/hantro_hevc.c     | 0
>  .../media/hantro => media/platform/verisilicon}/hantro_hw.h | 0
>  .../hantro => media/platform/verisilicon}/hantro_jpeg.c     | 0
>  .../hantro => media/platform/verisilicon}/hantro_jpeg.h     | 0
>  .../hantro => media/platform/verisilicon}/hantro_mpeg2.c    | 0
>  .../hantro => media/platform/verisilicon}/hantro_postproc.c | 0
>  .../hantro => media/platform/verisilicon}/hantro_v4l2.c     | 0
>  .../hantro => media/platform/verisilicon}/hantro_v4l2.h     | 0
>  .../hantro => media/platform/verisilicon}/hantro_vp8.c      | 0
>  .../hantro => media/platform/verisilicon}/hantro_vp9.c      | 0
>  .../hantro => media/platform/verisilicon}/hantro_vp9.h      | 0
>  .../hantro => media/platform/verisilicon}/imx8m_vpu_hw.c    | 0
>  .../platform/verisilicon}/rockchip_vpu2_hw_h264_dec.c       | 0
>  .../platform/verisilicon}/rockchip_vpu2_hw_jpeg_enc.c       | 0
>  .../platform/verisilicon}/rockchip_vpu2_hw_mpeg2_dec.c      | 0
>  .../platform/verisilicon}/rockchip_vpu2_hw_vp8_dec.c        | 0
>  .../platform/verisilicon}/rockchip_vpu2_regs.h              | 0
>  .../hantro => media/platform/verisilicon}/rockchip_vpu_hw.c | 0
>  .../hantro => media/platform/verisilicon}/sama5d4_vdec_hw.c | 0
>  .../hantro => media/platform/verisilicon}/sunxi_vpu_hw.c    | 0
>  drivers/staging/media/Kconfig                               | 2 --
>  drivers/staging/media/Makefile                              | 1 -
>  drivers/staging/media/hantro/TODO                           | 2 --
>  42 files changed, 8 insertions(+), 7 deletions(-)
>  rename drivers/{staging/media/hantro => media/platform/verisilicon}/Kconfig (91%)
>  rename drivers/{staging/media/hantro => media/platform/verisilicon}/Makefile (100%)
>  rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro.h (100%)
>  rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_drv.c (100%)
>  rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_g1.c (100%)
>  rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_g1_h264_dec.c (100%)
>  rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_g1_mpeg2_dec.c (100%)
>  rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_g1_regs.h (100%)
>  rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_g1_vp8_dec.c (100%)
>  rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_g2.c (100%)
>  rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_g2_hevc_dec.c (100%)
>  rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_g2_regs.h (100%)
>  rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_g2_vp9_dec.c (100%)
>  rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_h1_jpeg_enc.c (100%)
>  rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_h1_regs.h (100%)
>  rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_h264.c (100%)
>  rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_hevc.c (100%)
>  rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_hw.h (100%)
>  rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_jpeg.c (100%)
>  rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_jpeg.h (100%)
>  rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_mpeg2.c (100%)
>  rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_postproc.c (100%)
>  rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_v4l2.c (100%)
>  rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_v4l2.h (100%)
>  rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_vp8.c (100%)
>  rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_vp9.c (100%)
>  rename drivers/{staging/media/hantro => media/platform/verisilicon}/hantro_vp9.h (100%)
>  rename drivers/{staging/media/hantro => media/platform/verisilicon}/imx8m_vpu_hw.c (100%)
>  rename drivers/{staging/media/hantro => media/platform/verisilicon}/rockchip_vpu2_hw_h264_dec.c (100%)
>  rename drivers/{staging/media/hantro => media/platform/verisilicon}/rockchip_vpu2_hw_jpeg_enc.c (100%)
>  rename drivers/{staging/media/hantro => media/platform/verisilicon}/rockchip_vpu2_hw_mpeg2_dec.c (100%)
>  rename drivers/{staging/media/hantro => media/platform/verisilicon}/rockchip_vpu2_hw_vp8_dec.c (100%)
>  rename drivers/{staging/media/hantro => media/platform/verisilicon}/rockchip_vpu2_regs.h (100%)
>  rename drivers/{staging/media/hantro => media/platform/verisilicon}/rockchip_vpu_hw.c (100%)
>  rename drivers/{staging/media/hantro => media/platform/verisilicon}/sama5d4_vdec_hw.c (100%)
>  rename drivers/{staging/media/hantro => media/platform/verisilicon}/sunxi_vpu_hw.c (100%)
>  delete mode 100644 drivers/staging/media/hantro/TODO
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index fee2ac9a98f4..07ed4aaf545d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8686,7 +8686,7 @@ S:	Maintained
>  F:	Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
>  F:	Documentation/devicetree/bindings/media/rockchip,rk3568-vepu.yaml
>  F:	Documentation/devicetree/bindings/media/rockchip-vpu.yaml
> -F:	drivers/staging/media/hantro/
> +F:	drivers/media/platform/verisilicon/
>  
>  HARD DRIVE ACTIVE PROTECTION SYSTEM (HDAPS) DRIVER
>  M:	Frank Seidel <frank@f-seidel.de>
> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> index f1056ceaf5a8..a9334263fa9b 100644
> --- a/drivers/media/platform/Kconfig
> +++ b/drivers/media/platform/Kconfig
> @@ -81,6 +81,7 @@ source "drivers/media/platform/samsung/Kconfig"
>  source "drivers/media/platform/st/Kconfig"
>  source "drivers/media/platform/sunxi/Kconfig"
>  source "drivers/media/platform/ti/Kconfig"
> +source "drivers/media/platform/verisilicon/Kconfig"
>  source "drivers/media/platform/via/Kconfig"
>  source "drivers/media/platform/xilinx/Kconfig"
>  
> diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
> index a881e97bae95..a91f42024273 100644
> --- a/drivers/media/platform/Makefile
> +++ b/drivers/media/platform/Makefile
> @@ -24,6 +24,7 @@ obj-y += samsung/
>  obj-y += st/
>  obj-y += sunxi/
>  obj-y += ti/
> +obj-y += verisilicon/
>  obj-y += via/
>  obj-y += xilinx/
>  
> diff --git a/drivers/staging/media/hantro/Kconfig b/drivers/media/platform/verisilicon/Kconfig
> similarity index 91%
> rename from drivers/staging/media/hantro/Kconfig
> rename to drivers/media/platform/verisilicon/Kconfig
> index 0172a6822ec2..e65b836b9d78 100644
> --- a/drivers/staging/media/hantro/Kconfig
> +++ b/drivers/media/platform/verisilicon/Kconfig
> @@ -1,7 +1,11 @@
> -# SPDX-License-Identifier: GPL-2.0
> +# SPDX-License-Identifier: GPL-2.0-only
> +
> +comment "Verisilicon media platform drivers"
> +
>  config VIDEO_HANTRO
>  	tristate "Hantro VPU driver"
>  	depends on ARCH_MXC || ARCH_ROCKCHIP || ARCH_AT91 || ARCH_SUNXI || COMPILE_TEST
> +	depends on V4L_MEM2MEM_DRIVERS
>  	depends on VIDEO_DEV
>  	select MEDIA_CONTROLLER
>  	select MEDIA_CONTROLLER_REQUEST_API
> diff --git a/drivers/staging/media/hantro/Makefile b/drivers/media/platform/verisilicon/Makefile
> similarity index 100%
> rename from drivers/staging/media/hantro/Makefile
> rename to drivers/media/platform/verisilicon/Makefile
> diff --git a/drivers/staging/media/hantro/hantro.h b/drivers/media/platform/verisilicon/hantro.h
> similarity index 100%
> rename from drivers/staging/media/hantro/hantro.h
> rename to drivers/media/platform/verisilicon/hantro.h
> diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/media/platform/verisilicon/hantro_drv.c
> similarity index 100%
> rename from drivers/staging/media/hantro/hantro_drv.c
> rename to drivers/media/platform/verisilicon/hantro_drv.c
> diff --git a/drivers/staging/media/hantro/hantro_g1.c b/drivers/media/platform/verisilicon/hantro_g1.c
> similarity index 100%
> rename from drivers/staging/media/hantro/hantro_g1.c
> rename to drivers/media/platform/verisilicon/hantro_g1.c
> diff --git a/drivers/staging/media/hantro/hantro_g1_h264_dec.c b/drivers/media/platform/verisilicon/hantro_g1_h264_dec.c
> similarity index 100%
> rename from drivers/staging/media/hantro/hantro_g1_h264_dec.c
> rename to drivers/media/platform/verisilicon/hantro_g1_h264_dec.c
> diff --git a/drivers/staging/media/hantro/hantro_g1_mpeg2_dec.c b/drivers/media/platform/verisilicon/hantro_g1_mpeg2_dec.c
> similarity index 100%
> rename from drivers/staging/media/hantro/hantro_g1_mpeg2_dec.c
> rename to drivers/media/platform/verisilicon/hantro_g1_mpeg2_dec.c
> diff --git a/drivers/staging/media/hantro/hantro_g1_regs.h b/drivers/media/platform/verisilicon/hantro_g1_regs.h
> similarity index 100%
> rename from drivers/staging/media/hantro/hantro_g1_regs.h
> rename to drivers/media/platform/verisilicon/hantro_g1_regs.h
> diff --git a/drivers/staging/media/hantro/hantro_g1_vp8_dec.c b/drivers/media/platform/verisilicon/hantro_g1_vp8_dec.c
> similarity index 100%
> rename from drivers/staging/media/hantro/hantro_g1_vp8_dec.c
> rename to drivers/media/platform/verisilicon/hantro_g1_vp8_dec.c
> diff --git a/drivers/staging/media/hantro/hantro_g2.c b/drivers/media/platform/verisilicon/hantro_g2.c
> similarity index 100%
> rename from drivers/staging/media/hantro/hantro_g2.c
> rename to drivers/media/platform/verisilicon/hantro_g2.c
> diff --git a/drivers/staging/media/hantro/hantro_g2_hevc_dec.c b/drivers/media/platform/verisilicon/hantro_g2_hevc_dec.c
> similarity index 100%
> rename from drivers/staging/media/hantro/hantro_g2_hevc_dec.c
> rename to drivers/media/platform/verisilicon/hantro_g2_hevc_dec.c
> diff --git a/drivers/staging/media/hantro/hantro_g2_regs.h b/drivers/media/platform/verisilicon/hantro_g2_regs.h
> similarity index 100%
> rename from drivers/staging/media/hantro/hantro_g2_regs.h
> rename to drivers/media/platform/verisilicon/hantro_g2_regs.h
> diff --git a/drivers/staging/media/hantro/hantro_g2_vp9_dec.c b/drivers/media/platform/verisilicon/hantro_g2_vp9_dec.c
> similarity index 100%
> rename from drivers/staging/media/hantro/hantro_g2_vp9_dec.c
> rename to drivers/media/platform/verisilicon/hantro_g2_vp9_dec.c
> diff --git a/drivers/staging/media/hantro/hantro_h1_jpeg_enc.c b/drivers/media/platform/verisilicon/hantro_h1_jpeg_enc.c
> similarity index 100%
> rename from drivers/staging/media/hantro/hantro_h1_jpeg_enc.c
> rename to drivers/media/platform/verisilicon/hantro_h1_jpeg_enc.c
> diff --git a/drivers/staging/media/hantro/hantro_h1_regs.h b/drivers/media/platform/verisilicon/hantro_h1_regs.h
> similarity index 100%
> rename from drivers/staging/media/hantro/hantro_h1_regs.h
> rename to drivers/media/platform/verisilicon/hantro_h1_regs.h
> diff --git a/drivers/staging/media/hantro/hantro_h264.c b/drivers/media/platform/verisilicon/hantro_h264.c
> similarity index 100%
> rename from drivers/staging/media/hantro/hantro_h264.c
> rename to drivers/media/platform/verisilicon/hantro_h264.c
> diff --git a/drivers/staging/media/hantro/hantro_hevc.c b/drivers/media/platform/verisilicon/hantro_hevc.c
> similarity index 100%
> rename from drivers/staging/media/hantro/hantro_hevc.c
> rename to drivers/media/platform/verisilicon/hantro_hevc.c
> diff --git a/drivers/staging/media/hantro/hantro_hw.h b/drivers/media/platform/verisilicon/hantro_hw.h
> similarity index 100%
> rename from drivers/staging/media/hantro/hantro_hw.h
> rename to drivers/media/platform/verisilicon/hantro_hw.h
> diff --git a/drivers/staging/media/hantro/hantro_jpeg.c b/drivers/media/platform/verisilicon/hantro_jpeg.c
> similarity index 100%
> rename from drivers/staging/media/hantro/hantro_jpeg.c
> rename to drivers/media/platform/verisilicon/hantro_jpeg.c
> diff --git a/drivers/staging/media/hantro/hantro_jpeg.h b/drivers/media/platform/verisilicon/hantro_jpeg.h
> similarity index 100%
> rename from drivers/staging/media/hantro/hantro_jpeg.h
> rename to drivers/media/platform/verisilicon/hantro_jpeg.h
> diff --git a/drivers/staging/media/hantro/hantro_mpeg2.c b/drivers/media/platform/verisilicon/hantro_mpeg2.c
> similarity index 100%
> rename from drivers/staging/media/hantro/hantro_mpeg2.c
> rename to drivers/media/platform/verisilicon/hantro_mpeg2.c
> diff --git a/drivers/staging/media/hantro/hantro_postproc.c b/drivers/media/platform/verisilicon/hantro_postproc.c
> similarity index 100%
> rename from drivers/staging/media/hantro/hantro_postproc.c
> rename to drivers/media/platform/verisilicon/hantro_postproc.c
> diff --git a/drivers/staging/media/hantro/hantro_v4l2.c b/drivers/media/platform/verisilicon/hantro_v4l2.c
> similarity index 100%
> rename from drivers/staging/media/hantro/hantro_v4l2.c
> rename to drivers/media/platform/verisilicon/hantro_v4l2.c
> diff --git a/drivers/staging/media/hantro/hantro_v4l2.h b/drivers/media/platform/verisilicon/hantro_v4l2.h
> similarity index 100%
> rename from drivers/staging/media/hantro/hantro_v4l2.h
> rename to drivers/media/platform/verisilicon/hantro_v4l2.h
> diff --git a/drivers/staging/media/hantro/hantro_vp8.c b/drivers/media/platform/verisilicon/hantro_vp8.c
> similarity index 100%
> rename from drivers/staging/media/hantro/hantro_vp8.c
> rename to drivers/media/platform/verisilicon/hantro_vp8.c
> diff --git a/drivers/staging/media/hantro/hantro_vp9.c b/drivers/media/platform/verisilicon/hantro_vp9.c
> similarity index 100%
> rename from drivers/staging/media/hantro/hantro_vp9.c
> rename to drivers/media/platform/verisilicon/hantro_vp9.c
> diff --git a/drivers/staging/media/hantro/hantro_vp9.h b/drivers/media/platform/verisilicon/hantro_vp9.h
> similarity index 100%
> rename from drivers/staging/media/hantro/hantro_vp9.h
> rename to drivers/media/platform/verisilicon/hantro_vp9.h
> diff --git a/drivers/staging/media/hantro/imx8m_vpu_hw.c b/drivers/media/platform/verisilicon/imx8m_vpu_hw.c
> similarity index 100%
> rename from drivers/staging/media/hantro/imx8m_vpu_hw.c
> rename to drivers/media/platform/verisilicon/imx8m_vpu_hw.c
> diff --git a/drivers/staging/media/hantro/rockchip_vpu2_hw_h264_dec.c b/drivers/media/platform/verisilicon/rockchip_vpu2_hw_h264_dec.c
> similarity index 100%
> rename from drivers/staging/media/hantro/rockchip_vpu2_hw_h264_dec.c
> rename to drivers/media/platform/verisilicon/rockchip_vpu2_hw_h264_dec.c
> diff --git a/drivers/staging/media/hantro/rockchip_vpu2_hw_jpeg_enc.c b/drivers/media/platform/verisilicon/rockchip_vpu2_hw_jpeg_enc.c
> similarity index 100%
> rename from drivers/staging/media/hantro/rockchip_vpu2_hw_jpeg_enc.c
> rename to drivers/media/platform/verisilicon/rockchip_vpu2_hw_jpeg_enc.c
> diff --git a/drivers/staging/media/hantro/rockchip_vpu2_hw_mpeg2_dec.c b/drivers/media/platform/verisilicon/rockchip_vpu2_hw_mpeg2_dec.c
> similarity index 100%
> rename from drivers/staging/media/hantro/rockchip_vpu2_hw_mpeg2_dec.c
> rename to drivers/media/platform/verisilicon/rockchip_vpu2_hw_mpeg2_dec.c
> diff --git a/drivers/staging/media/hantro/rockchip_vpu2_hw_vp8_dec.c b/drivers/media/platform/verisilicon/rockchip_vpu2_hw_vp8_dec.c
> similarity index 100%
> rename from drivers/staging/media/hantro/rockchip_vpu2_hw_vp8_dec.c
> rename to drivers/media/platform/verisilicon/rockchip_vpu2_hw_vp8_dec.c
> diff --git a/drivers/staging/media/hantro/rockchip_vpu2_regs.h b/drivers/media/platform/verisilicon/rockchip_vpu2_regs.h
> similarity index 100%
> rename from drivers/staging/media/hantro/rockchip_vpu2_regs.h
> rename to drivers/media/platform/verisilicon/rockchip_vpu2_regs.h
> diff --git a/drivers/staging/media/hantro/rockchip_vpu_hw.c b/drivers/media/platform/verisilicon/rockchip_vpu_hw.c
> similarity index 100%
> rename from drivers/staging/media/hantro/rockchip_vpu_hw.c
> rename to drivers/media/platform/verisilicon/rockchip_vpu_hw.c
> diff --git a/drivers/staging/media/hantro/sama5d4_vdec_hw.c b/drivers/media/platform/verisilicon/sama5d4_vdec_hw.c
> similarity index 100%
> rename from drivers/staging/media/hantro/sama5d4_vdec_hw.c
> rename to drivers/media/platform/verisilicon/sama5d4_vdec_hw.c
> diff --git a/drivers/staging/media/hantro/sunxi_vpu_hw.c b/drivers/media/platform/verisilicon/sunxi_vpu_hw.c
> similarity index 100%
> rename from drivers/staging/media/hantro/sunxi_vpu_hw.c
> rename to drivers/media/platform/verisilicon/sunxi_vpu_hw.c
> diff --git a/drivers/staging/media/Kconfig b/drivers/staging/media/Kconfig
> index 421ce9dbf44c..2b8ad4cc8ac5 100644
> --- a/drivers/staging/media/Kconfig
> +++ b/drivers/staging/media/Kconfig
> @@ -24,8 +24,6 @@ source "drivers/staging/media/atomisp/Kconfig"
>  
>  source "drivers/staging/media/av7110/Kconfig"
>  
> -source "drivers/staging/media/hantro/Kconfig"
> -
>  source "drivers/staging/media/imx/Kconfig"
>  
>  source "drivers/staging/media/ipu3/Kconfig"
> diff --git a/drivers/staging/media/Makefile b/drivers/staging/media/Makefile
> index 950e96f10aad..a12d0bb9dca3 100644
> --- a/drivers/staging/media/Makefile
> +++ b/drivers/staging/media/Makefile
> @@ -8,7 +8,6 @@ obj-$(CONFIG_VIDEO_ROCKCHIP_VDEC)	+= rkvdec/
>  obj-$(CONFIG_VIDEO_STKWEBCAM)	+= stkwebcam/
>  obj-$(CONFIG_VIDEO_SUNXI)	+= sunxi/
>  obj-$(CONFIG_VIDEO_TEGRA)	+= tegra-video/
> -obj-$(CONFIG_VIDEO_HANTRO)	+= hantro/
>  obj-$(CONFIG_VIDEO_IPU3_IMGU)	+= ipu3/
>  obj-$(CONFIG_VIDEO_ZORAN)	+= zoran/
>  obj-$(CONFIG_DVB_AV7110)	+= av7110/
> diff --git a/drivers/staging/media/hantro/TODO b/drivers/staging/media/hantro/TODO
> deleted file mode 100644
> index 8483ff482146..000000000000
> --- a/drivers/staging/media/hantro/TODO
> +++ /dev/null
> @@ -1,2 +0,0 @@
> -The V4L controls for the HEVC CODEC are not yet part of the stable uABI,
> -we are keeping this driver in staging until the HEVC uABI has been merged.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] hantro: Remove dedicated control documentation
  2022-07-18 21:41 ` [PATCH] hantro: Remove dedicated control documentation Ezequiel Garcia
@ 2022-07-19 19:34   ` Nicolas Dufresne
  0 siblings, 0 replies; 10+ messages in thread
From: Nicolas Dufresne @ 2022-07-19 19:34 UTC (permalink / raw)
  To: Ezequiel Garcia, linux-media, linux-kernel
  Cc: Hans Verkuil, Philipp Zabel, Benjamin Gaignard, Jernej Skrabec,
	Tomasz Figa, Chris Healy

Le lundi 18 juillet 2022 à 18:41 -0300, Ezequiel Garcia a écrit :
> The dedicated control required by the HEVC support
> was removed, and the driver now calculates the value
> internally. Remove the ad-hoc documentation as well.
> 
> Fixes: 3360755ef89ab ("media: hantro: Stop using Hantro dedicated control")
> Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>

Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

> ---
>  .../userspace-api/media/drivers/hantro.rst    | 19 -------------------
>  1 file changed, 19 deletions(-)
>  delete mode 100644 Documentation/userspace-api/media/drivers/hantro.rst
> 
> diff --git a/Documentation/userspace-api/media/drivers/hantro.rst b/Documentation/userspace-api/media/drivers/hantro.rst
> deleted file mode 100644
> index cd9754b4e005..000000000000
> --- a/Documentation/userspace-api/media/drivers/hantro.rst
> +++ /dev/null
> @@ -1,19 +0,0 @@
> -.. SPDX-License-Identifier: GPL-2.0
> -
> -Hantro video decoder driver
> -===========================
> -
> -The Hantro video decoder driver implements the following driver-specific controls:
> -
> -``V4L2_CID_HANTRO_HEVC_SLICE_HEADER_SKIP (integer)``
> -    Specifies to Hantro HEVC video decoder driver the number of data (in bits) to
> -    skip in the slice segment header.
> -    If non-IDR, the bits to be skipped go from syntax element "pic_output_flag"
> -    to before syntax element "slice_temporal_mvp_enabled_flag".
> -    If IDR, the skipped bits are just "pic_output_flag"
> -    (separate_colour_plane_flag is not supported).
> -
> -.. note::
> -
> -        This control is not yet part of the public kernel API and
> -        it is expected to change.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] hantro: Remove incorrect HEVC SPS validation
  2022-07-08  8:42     ` Hans Verkuil
@ 2022-07-18 12:08       ` Ezequiel Garcia
  0 siblings, 0 replies; 10+ messages in thread
From: Ezequiel Garcia @ 2022-07-18 12:08 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Sebastian Fricke, linux-media, linux-kernel, Philipp Zabel,
	Nicolas Dufresne, Benjamin Gaignard

Hi Hans,

On Fri, Jul 08, 2022 at 10:42:43AM +0200, Hans Verkuil wrote:
> 
> 
> On 6/30/22 07:02, Sebastian Fricke wrote:
> > Hey Ezequiel,
> > 
> > On 29.06.2022 16:56, Ezequiel Garcia wrote:
> >> Currently, the driver tries to validat the HEVC SPS
> > 
> > s/validat/validate/
> > 
> >> against the CAPTURE queue format (i.e. the decoded format).
> >> This is not correct, because typically the SPS control is set
> >> before the CAPTURE queue is negotiated.
> >>
> >> In addition to this, a format validation in hantro_hevc_dec_prepare_run()
> >> is also suboptimal, because hantro_hevc_dec_prepare_run() runs in the context
> >> of v4l2_m2m_ops.device_run, as part of a decoding job.
> >>
> >> Format and control validations should happen before decoding starts,
> >> in the context of ioctls such as S_CTRL, S_FMT, or STREAMON.
> >>
> >> Remove the validation for now.
> > 
> > Couldn't we add a small wrapper around STREAMON to perform that
> > validation? I feel like "remove the validation for now", seems like a
> > vague statement.
> 
> I agree. Basically two things happen in this patch: two sanity checks
> for the SPS control are moved to try_ctrl, and that part looks good.
> 
> So that can be a separate patch.
> 
> The second part is the removal of the format+control validation, but it
> is not clear why removing it altogether is wrong. Shouldn't it still be
> done somewhere? And if not, why not?
> 

Sorry for the delayed reply. After giving this format+control validation
more thought, I believe the V4L2 API provides a natural way of handling.

When the format is negotiated in TRY_FMT, the control needs to be used
to offer a valid resolution.

The driver is capable of negotiating the resolution of the stream
using the control information, instead of failing.

I'll send a v2 with a new proposal.

Thanks,
Ezequiel

> Regards,
> 
> 	Hans
> 
> > 
> > Greetings,
> > Sebastian
> > 
> >>
> >> Fixes: 135ad96cb4d6b ("media: hantro: Be more accurate on pixel formats step_width constraints")
> >> Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
> >> ---
> >> drivers/staging/media/hantro/hantro_drv.c  | 12 ++++-----
> >> drivers/staging/media/hantro/hantro_hevc.c | 30 ----------------------
> >> drivers/staging/media/hantro/hantro_hw.h   |  1 -
> >> 3 files changed, 6 insertions(+), 37 deletions(-)
> >>
> >> diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
> >> index afddf7ac0731..2387ca85ab54 100644
> >> --- a/drivers/staging/media/hantro/hantro_drv.c
> >> +++ b/drivers/staging/media/hantro/hantro_drv.c
> >> @@ -253,11 +253,6 @@ queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq)
> >>
> >> static int hantro_try_ctrl(struct v4l2_ctrl *ctrl)
> >> {
> >> -    struct hantro_ctx *ctx;
> >> -
> >> -    ctx = container_of(ctrl->handler,
> >> -               struct hantro_ctx, ctrl_handler);
> >> -
> >>     if (ctrl->id == V4L2_CID_STATELESS_H264_SPS) {
> >>         const struct v4l2_ctrl_h264_sps *sps = ctrl->p_new.p_h264_sps;
> >>
> >> @@ -273,7 +268,12 @@ static int hantro_try_ctrl(struct v4l2_ctrl *ctrl)
> >>     } else if (ctrl->id == V4L2_CID_MPEG_VIDEO_HEVC_SPS) {
> >>         const struct v4l2_ctrl_hevc_sps *sps = ctrl->p_new.p_hevc_sps;
> >>
> >> -        return hantro_hevc_validate_sps(ctx, sps);
> >> +        if (sps->bit_depth_luma_minus8 != sps->bit_depth_chroma_minus8)
> >> +            /* Luma and chroma bit depth mismatch */
> >> +            return -EINVAL;
> >> +        if (sps->bit_depth_luma_minus8 != 0)
> >> +            /* Only 8-bit is supported */
> >> +            return -EINVAL;
> >>     } else if (ctrl->id == V4L2_CID_STATELESS_VP9_FRAME) {
> >>         const struct v4l2_ctrl_vp9_frame *dec_params = ctrl->p_new.p_vp9_frame;
> >>
> >> diff --git a/drivers/staging/media/hantro/hantro_hevc.c b/drivers/staging/media/hantro/hantro_hevc.c
> >> index bd924896e409..f86c98e19177 100644
> >> --- a/drivers/staging/media/hantro/hantro_hevc.c
> >> +++ b/drivers/staging/media/hantro/hantro_hevc.c
> >> @@ -154,32 +154,6 @@ static int tile_buffer_reallocate(struct hantro_ctx *ctx)
> >>     return -ENOMEM;
> >> }
> >>
> >> -int hantro_hevc_validate_sps(struct hantro_ctx *ctx, const struct v4l2_ctrl_hevc_sps *sps)
> >> -{
> >> -    if (sps->bit_depth_luma_minus8 != sps->bit_depth_chroma_minus8)
> >> -        /* Luma and chroma bit depth mismatch */
> >> -        return -EINVAL;
> >> -    if (sps->bit_depth_luma_minus8 != 0)
> >> -        /* Only 8-bit is supported */
> >> -        return -EINVAL;
> >> -
> >> -    /*
> >> -     * for tile pixel format check if the width and height match
> >> -     * hardware constraints
> >> -     */
> >> -    if (ctx->vpu_dst_fmt->fourcc == V4L2_PIX_FMT_NV12_4L4) {
> >> -        if (ctx->dst_fmt.width !=
> >> -            ALIGN(sps->pic_width_in_luma_samples, ctx->vpu_dst_fmt->frmsize.step_width))
> >> -            return -EINVAL;
> >> -
> >> -        if (ctx->dst_fmt.height !=
> >> -            ALIGN(sps->pic_height_in_luma_samples, ctx->vpu_dst_fmt->frmsize.step_height))
> >> -            return -EINVAL;
> >> -    }
> >> -
> >> -    return 0;
> >> -}
> >> -
> >> int hantro_hevc_dec_prepare_run(struct hantro_ctx *ctx)
> >> {
> >>     struct hantro_hevc_dec_hw_ctx *hevc_ctx = &ctx->hevc_dec;
> >> @@ -203,10 +177,6 @@ int hantro_hevc_dec_prepare_run(struct hantro_ctx *ctx)
> >>     if (WARN_ON(!ctrls->sps))
> >>         return -EINVAL;
> >>
> >> -    ret = hantro_hevc_validate_sps(ctx, ctrls->sps);
> >> -    if (ret)
> >> -        return ret;
> >> -
> >>     ctrls->pps =
> >>         hantro_get_ctrl(ctx, V4L2_CID_MPEG_VIDEO_HEVC_PPS);
> >>     if (WARN_ON(!ctrls->pps))
> >> diff --git a/drivers/staging/media/hantro/hantro_hw.h b/drivers/staging/media/hantro/hantro_hw.h
> >> index a2e0f0836281..5edff0f0be20 100644
> >> --- a/drivers/staging/media/hantro/hantro_hw.h
> >> +++ b/drivers/staging/media/hantro/hantro_hw.h
> >> @@ -359,7 +359,6 @@ int hantro_hevc_dec_prepare_run(struct hantro_ctx *ctx);
> >> void hantro_hevc_ref_init(struct hantro_ctx *ctx);
> >> dma_addr_t hantro_hevc_get_ref_buf(struct hantro_ctx *ctx, int poc);
> >> int hantro_hevc_add_ref_buf(struct hantro_ctx *ctx, int poc, dma_addr_t addr);
> >> -int hantro_hevc_validate_sps(struct hantro_ctx *ctx, const struct v4l2_ctrl_hevc_sps *sps);
> >>
> >>
> >> static inline unsigned short hantro_vp9_num_sbs(unsigned short dimension)
> >> -- 
> >> 2.31.1
> >>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] hantro: Remove incorrect HEVC SPS validation
  2022-06-30  5:02   ` Sebastian Fricke
@ 2022-07-08  8:42     ` Hans Verkuil
  2022-07-18 12:08       ` Ezequiel Garcia
  0 siblings, 1 reply; 10+ messages in thread
From: Hans Verkuil @ 2022-07-08  8:42 UTC (permalink / raw)
  To: Sebastian Fricke, Ezequiel Garcia
  Cc: linux-media, linux-kernel, Philipp Zabel, Nicolas Dufresne,
	Benjamin Gaignard



On 6/30/22 07:02, Sebastian Fricke wrote:
> Hey Ezequiel,
> 
> On 29.06.2022 16:56, Ezequiel Garcia wrote:
>> Currently, the driver tries to validat the HEVC SPS
> 
> s/validat/validate/
> 
>> against the CAPTURE queue format (i.e. the decoded format).
>> This is not correct, because typically the SPS control is set
>> before the CAPTURE queue is negotiated.
>>
>> In addition to this, a format validation in hantro_hevc_dec_prepare_run()
>> is also suboptimal, because hantro_hevc_dec_prepare_run() runs in the context
>> of v4l2_m2m_ops.device_run, as part of a decoding job.
>>
>> Format and control validations should happen before decoding starts,
>> in the context of ioctls such as S_CTRL, S_FMT, or STREAMON.
>>
>> Remove the validation for now.
> 
> Couldn't we add a small wrapper around STREAMON to perform that
> validation? I feel like "remove the validation for now", seems like a
> vague statement.

I agree. Basically two things happen in this patch: two sanity checks
for the SPS control are moved to try_ctrl, and that part looks good.

So that can be a separate patch.

The second part is the removal of the format+control validation, but it
is not clear why removing it altogether is wrong. Shouldn't it still be
done somewhere? And if not, why not?

Regards,

	Hans

> 
> Greetings,
> Sebastian
> 
>>
>> Fixes: 135ad96cb4d6b ("media: hantro: Be more accurate on pixel formats step_width constraints")
>> Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
>> ---
>> drivers/staging/media/hantro/hantro_drv.c  | 12 ++++-----
>> drivers/staging/media/hantro/hantro_hevc.c | 30 ----------------------
>> drivers/staging/media/hantro/hantro_hw.h   |  1 -
>> 3 files changed, 6 insertions(+), 37 deletions(-)
>>
>> diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
>> index afddf7ac0731..2387ca85ab54 100644
>> --- a/drivers/staging/media/hantro/hantro_drv.c
>> +++ b/drivers/staging/media/hantro/hantro_drv.c
>> @@ -253,11 +253,6 @@ queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq)
>>
>> static int hantro_try_ctrl(struct v4l2_ctrl *ctrl)
>> {
>> -    struct hantro_ctx *ctx;
>> -
>> -    ctx = container_of(ctrl->handler,
>> -               struct hantro_ctx, ctrl_handler);
>> -
>>     if (ctrl->id == V4L2_CID_STATELESS_H264_SPS) {
>>         const struct v4l2_ctrl_h264_sps *sps = ctrl->p_new.p_h264_sps;
>>
>> @@ -273,7 +268,12 @@ static int hantro_try_ctrl(struct v4l2_ctrl *ctrl)
>>     } else if (ctrl->id == V4L2_CID_MPEG_VIDEO_HEVC_SPS) {
>>         const struct v4l2_ctrl_hevc_sps *sps = ctrl->p_new.p_hevc_sps;
>>
>> -        return hantro_hevc_validate_sps(ctx, sps);
>> +        if (sps->bit_depth_luma_minus8 != sps->bit_depth_chroma_minus8)
>> +            /* Luma and chroma bit depth mismatch */
>> +            return -EINVAL;
>> +        if (sps->bit_depth_luma_minus8 != 0)
>> +            /* Only 8-bit is supported */
>> +            return -EINVAL;
>>     } else if (ctrl->id == V4L2_CID_STATELESS_VP9_FRAME) {
>>         const struct v4l2_ctrl_vp9_frame *dec_params = ctrl->p_new.p_vp9_frame;
>>
>> diff --git a/drivers/staging/media/hantro/hantro_hevc.c b/drivers/staging/media/hantro/hantro_hevc.c
>> index bd924896e409..f86c98e19177 100644
>> --- a/drivers/staging/media/hantro/hantro_hevc.c
>> +++ b/drivers/staging/media/hantro/hantro_hevc.c
>> @@ -154,32 +154,6 @@ static int tile_buffer_reallocate(struct hantro_ctx *ctx)
>>     return -ENOMEM;
>> }
>>
>> -int hantro_hevc_validate_sps(struct hantro_ctx *ctx, const struct v4l2_ctrl_hevc_sps *sps)
>> -{
>> -    if (sps->bit_depth_luma_minus8 != sps->bit_depth_chroma_minus8)
>> -        /* Luma and chroma bit depth mismatch */
>> -        return -EINVAL;
>> -    if (sps->bit_depth_luma_minus8 != 0)
>> -        /* Only 8-bit is supported */
>> -        return -EINVAL;
>> -
>> -    /*
>> -     * for tile pixel format check if the width and height match
>> -     * hardware constraints
>> -     */
>> -    if (ctx->vpu_dst_fmt->fourcc == V4L2_PIX_FMT_NV12_4L4) {
>> -        if (ctx->dst_fmt.width !=
>> -            ALIGN(sps->pic_width_in_luma_samples, ctx->vpu_dst_fmt->frmsize.step_width))
>> -            return -EINVAL;
>> -
>> -        if (ctx->dst_fmt.height !=
>> -            ALIGN(sps->pic_height_in_luma_samples, ctx->vpu_dst_fmt->frmsize.step_height))
>> -            return -EINVAL;
>> -    }
>> -
>> -    return 0;
>> -}
>> -
>> int hantro_hevc_dec_prepare_run(struct hantro_ctx *ctx)
>> {
>>     struct hantro_hevc_dec_hw_ctx *hevc_ctx = &ctx->hevc_dec;
>> @@ -203,10 +177,6 @@ int hantro_hevc_dec_prepare_run(struct hantro_ctx *ctx)
>>     if (WARN_ON(!ctrls->sps))
>>         return -EINVAL;
>>
>> -    ret = hantro_hevc_validate_sps(ctx, ctrls->sps);
>> -    if (ret)
>> -        return ret;
>> -
>>     ctrls->pps =
>>         hantro_get_ctrl(ctx, V4L2_CID_MPEG_VIDEO_HEVC_PPS);
>>     if (WARN_ON(!ctrls->pps))
>> diff --git a/drivers/staging/media/hantro/hantro_hw.h b/drivers/staging/media/hantro/hantro_hw.h
>> index a2e0f0836281..5edff0f0be20 100644
>> --- a/drivers/staging/media/hantro/hantro_hw.h
>> +++ b/drivers/staging/media/hantro/hantro_hw.h
>> @@ -359,7 +359,6 @@ int hantro_hevc_dec_prepare_run(struct hantro_ctx *ctx);
>> void hantro_hevc_ref_init(struct hantro_ctx *ctx);
>> dma_addr_t hantro_hevc_get_ref_buf(struct hantro_ctx *ctx, int poc);
>> int hantro_hevc_add_ref_buf(struct hantro_ctx *ctx, int poc, dma_addr_t addr);
>> -int hantro_hevc_validate_sps(struct hantro_ctx *ctx, const struct v4l2_ctrl_hevc_sps *sps);
>>
>>
>> static inline unsigned short hantro_vp9_num_sbs(unsigned short dimension)
>> -- 
>> 2.31.1
>>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] hantro: Remove incorrect HEVC SPS validation
  2022-06-29 19:56 ` [PATCH] hantro: Remove incorrect HEVC SPS validation Ezequiel Garcia
@ 2022-06-30  5:02   ` Sebastian Fricke
  2022-07-08  8:42     ` Hans Verkuil
  0 siblings, 1 reply; 10+ messages in thread
From: Sebastian Fricke @ 2022-06-30  5:02 UTC (permalink / raw)
  To: Ezequiel Garcia
  Cc: linux-media, linux-kernel, Hans Verkuil, Philipp Zabel,
	Nicolas Dufresne, Benjamin Gaignard

Hey Ezequiel,

On 29.06.2022 16:56, Ezequiel Garcia wrote:
>Currently, the driver tries to validat the HEVC SPS

s/validat/validate/

>against the CAPTURE queue format (i.e. the decoded format).
>This is not correct, because typically the SPS control is set
>before the CAPTURE queue is negotiated.
>
>In addition to this, a format validation in hantro_hevc_dec_prepare_run()
>is also suboptimal, because hantro_hevc_dec_prepare_run() runs in the context
>of v4l2_m2m_ops.device_run, as part of a decoding job.
>
>Format and control validations should happen before decoding starts,
>in the context of ioctls such as S_CTRL, S_FMT, or STREAMON.
>
>Remove the validation for now.

Couldn't we add a small wrapper around STREAMON to perform that
validation? I feel like "remove the validation for now", seems like a
vague statement.

Greetings,
Sebastian

>
>Fixes: 135ad96cb4d6b ("media: hantro: Be more accurate on pixel formats step_width constraints")
>Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
>---
> drivers/staging/media/hantro/hantro_drv.c  | 12 ++++-----
> drivers/staging/media/hantro/hantro_hevc.c | 30 ----------------------
> drivers/staging/media/hantro/hantro_hw.h   |  1 -
> 3 files changed, 6 insertions(+), 37 deletions(-)
>
>diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
>index afddf7ac0731..2387ca85ab54 100644
>--- a/drivers/staging/media/hantro/hantro_drv.c
>+++ b/drivers/staging/media/hantro/hantro_drv.c
>@@ -253,11 +253,6 @@ queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq)
>
> static int hantro_try_ctrl(struct v4l2_ctrl *ctrl)
> {
>-	struct hantro_ctx *ctx;
>-
>-	ctx = container_of(ctrl->handler,
>-			   struct hantro_ctx, ctrl_handler);
>-
> 	if (ctrl->id == V4L2_CID_STATELESS_H264_SPS) {
> 		const struct v4l2_ctrl_h264_sps *sps = ctrl->p_new.p_h264_sps;
>
>@@ -273,7 +268,12 @@ static int hantro_try_ctrl(struct v4l2_ctrl *ctrl)
> 	} else if (ctrl->id == V4L2_CID_MPEG_VIDEO_HEVC_SPS) {
> 		const struct v4l2_ctrl_hevc_sps *sps = ctrl->p_new.p_hevc_sps;
>
>-		return hantro_hevc_validate_sps(ctx, sps);
>+		if (sps->bit_depth_luma_minus8 != sps->bit_depth_chroma_minus8)
>+			/* Luma and chroma bit depth mismatch */
>+			return -EINVAL;
>+		if (sps->bit_depth_luma_minus8 != 0)
>+			/* Only 8-bit is supported */
>+			return -EINVAL;
> 	} else if (ctrl->id == V4L2_CID_STATELESS_VP9_FRAME) {
> 		const struct v4l2_ctrl_vp9_frame *dec_params = ctrl->p_new.p_vp9_frame;
>
>diff --git a/drivers/staging/media/hantro/hantro_hevc.c b/drivers/staging/media/hantro/hantro_hevc.c
>index bd924896e409..f86c98e19177 100644
>--- a/drivers/staging/media/hantro/hantro_hevc.c
>+++ b/drivers/staging/media/hantro/hantro_hevc.c
>@@ -154,32 +154,6 @@ static int tile_buffer_reallocate(struct hantro_ctx *ctx)
> 	return -ENOMEM;
> }
>
>-int hantro_hevc_validate_sps(struct hantro_ctx *ctx, const struct v4l2_ctrl_hevc_sps *sps)
>-{
>-	if (sps->bit_depth_luma_minus8 != sps->bit_depth_chroma_minus8)
>-		/* Luma and chroma bit depth mismatch */
>-		return -EINVAL;
>-	if (sps->bit_depth_luma_minus8 != 0)
>-		/* Only 8-bit is supported */
>-		return -EINVAL;
>-
>-	/*
>-	 * for tile pixel format check if the width and height match
>-	 * hardware constraints
>-	 */
>-	if (ctx->vpu_dst_fmt->fourcc == V4L2_PIX_FMT_NV12_4L4) {
>-		if (ctx->dst_fmt.width !=
>-		    ALIGN(sps->pic_width_in_luma_samples, ctx->vpu_dst_fmt->frmsize.step_width))
>-			return -EINVAL;
>-
>-		if (ctx->dst_fmt.height !=
>-		    ALIGN(sps->pic_height_in_luma_samples, ctx->vpu_dst_fmt->frmsize.step_height))
>-			return -EINVAL;
>-	}
>-
>-	return 0;
>-}
>-
> int hantro_hevc_dec_prepare_run(struct hantro_ctx *ctx)
> {
> 	struct hantro_hevc_dec_hw_ctx *hevc_ctx = &ctx->hevc_dec;
>@@ -203,10 +177,6 @@ int hantro_hevc_dec_prepare_run(struct hantro_ctx *ctx)
> 	if (WARN_ON(!ctrls->sps))
> 		return -EINVAL;
>
>-	ret = hantro_hevc_validate_sps(ctx, ctrls->sps);
>-	if (ret)
>-		return ret;
>-
> 	ctrls->pps =
> 		hantro_get_ctrl(ctx, V4L2_CID_MPEG_VIDEO_HEVC_PPS);
> 	if (WARN_ON(!ctrls->pps))
>diff --git a/drivers/staging/media/hantro/hantro_hw.h b/drivers/staging/media/hantro/hantro_hw.h
>index a2e0f0836281..5edff0f0be20 100644
>--- a/drivers/staging/media/hantro/hantro_hw.h
>+++ b/drivers/staging/media/hantro/hantro_hw.h
>@@ -359,7 +359,6 @@ int hantro_hevc_dec_prepare_run(struct hantro_ctx *ctx);
> void hantro_hevc_ref_init(struct hantro_ctx *ctx);
> dma_addr_t hantro_hevc_get_ref_buf(struct hantro_ctx *ctx, int poc);
> int hantro_hevc_add_ref_buf(struct hantro_ctx *ctx, int poc, dma_addr_t addr);
>-int hantro_hevc_validate_sps(struct hantro_ctx *ctx, const struct v4l2_ctrl_hevc_sps *sps);
>
>
> static inline unsigned short hantro_vp9_num_sbs(unsigned short dimension)
>-- 
>2.31.1
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] hantro: Remove incorrect HEVC SPS validation
  2022-06-29 19:56 [PATCH] hantro: Fix RK3399 H.264 format advertising Ezequiel Garcia
@ 2022-06-29 19:56 ` Ezequiel Garcia
  2022-06-30  5:02   ` Sebastian Fricke
  0 siblings, 1 reply; 10+ messages in thread
From: Ezequiel Garcia @ 2022-06-29 19:56 UTC (permalink / raw)
  To: linux-media, linux-kernel
  Cc: Hans Verkuil, Philipp Zabel, Nicolas Dufresne, Benjamin Gaignard,
	Ezequiel Garcia

Currently, the driver tries to validat the HEVC SPS
against the CAPTURE queue format (i.e. the decoded format).
This is not correct, because typically the SPS control is set
before the CAPTURE queue is negotiated.

In addition to this, a format validation in hantro_hevc_dec_prepare_run()
is also suboptimal, because hantro_hevc_dec_prepare_run() runs in the context
of v4l2_m2m_ops.device_run, as part of a decoding job.

Format and control validations should happen before decoding starts,
in the context of ioctls such as S_CTRL, S_FMT, or STREAMON.

Remove the validation for now.

Fixes: 135ad96cb4d6b ("media: hantro: Be more accurate on pixel formats step_width constraints")
Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
---
 drivers/staging/media/hantro/hantro_drv.c  | 12 ++++-----
 drivers/staging/media/hantro/hantro_hevc.c | 30 ----------------------
 drivers/staging/media/hantro/hantro_hw.h   |  1 -
 3 files changed, 6 insertions(+), 37 deletions(-)

diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
index afddf7ac0731..2387ca85ab54 100644
--- a/drivers/staging/media/hantro/hantro_drv.c
+++ b/drivers/staging/media/hantro/hantro_drv.c
@@ -253,11 +253,6 @@ queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq)
 
 static int hantro_try_ctrl(struct v4l2_ctrl *ctrl)
 {
-	struct hantro_ctx *ctx;
-
-	ctx = container_of(ctrl->handler,
-			   struct hantro_ctx, ctrl_handler);
-
 	if (ctrl->id == V4L2_CID_STATELESS_H264_SPS) {
 		const struct v4l2_ctrl_h264_sps *sps = ctrl->p_new.p_h264_sps;
 
@@ -273,7 +268,12 @@ static int hantro_try_ctrl(struct v4l2_ctrl *ctrl)
 	} else if (ctrl->id == V4L2_CID_MPEG_VIDEO_HEVC_SPS) {
 		const struct v4l2_ctrl_hevc_sps *sps = ctrl->p_new.p_hevc_sps;
 
-		return hantro_hevc_validate_sps(ctx, sps);
+		if (sps->bit_depth_luma_minus8 != sps->bit_depth_chroma_minus8)
+			/* Luma and chroma bit depth mismatch */
+			return -EINVAL;
+		if (sps->bit_depth_luma_minus8 != 0)
+			/* Only 8-bit is supported */
+			return -EINVAL;
 	} else if (ctrl->id == V4L2_CID_STATELESS_VP9_FRAME) {
 		const struct v4l2_ctrl_vp9_frame *dec_params = ctrl->p_new.p_vp9_frame;
 
diff --git a/drivers/staging/media/hantro/hantro_hevc.c b/drivers/staging/media/hantro/hantro_hevc.c
index bd924896e409..f86c98e19177 100644
--- a/drivers/staging/media/hantro/hantro_hevc.c
+++ b/drivers/staging/media/hantro/hantro_hevc.c
@@ -154,32 +154,6 @@ static int tile_buffer_reallocate(struct hantro_ctx *ctx)
 	return -ENOMEM;
 }
 
-int hantro_hevc_validate_sps(struct hantro_ctx *ctx, const struct v4l2_ctrl_hevc_sps *sps)
-{
-	if (sps->bit_depth_luma_minus8 != sps->bit_depth_chroma_minus8)
-		/* Luma and chroma bit depth mismatch */
-		return -EINVAL;
-	if (sps->bit_depth_luma_minus8 != 0)
-		/* Only 8-bit is supported */
-		return -EINVAL;
-
-	/*
-	 * for tile pixel format check if the width and height match
-	 * hardware constraints
-	 */
-	if (ctx->vpu_dst_fmt->fourcc == V4L2_PIX_FMT_NV12_4L4) {
-		if (ctx->dst_fmt.width !=
-		    ALIGN(sps->pic_width_in_luma_samples, ctx->vpu_dst_fmt->frmsize.step_width))
-			return -EINVAL;
-
-		if (ctx->dst_fmt.height !=
-		    ALIGN(sps->pic_height_in_luma_samples, ctx->vpu_dst_fmt->frmsize.step_height))
-			return -EINVAL;
-	}
-
-	return 0;
-}
-
 int hantro_hevc_dec_prepare_run(struct hantro_ctx *ctx)
 {
 	struct hantro_hevc_dec_hw_ctx *hevc_ctx = &ctx->hevc_dec;
@@ -203,10 +177,6 @@ int hantro_hevc_dec_prepare_run(struct hantro_ctx *ctx)
 	if (WARN_ON(!ctrls->sps))
 		return -EINVAL;
 
-	ret = hantro_hevc_validate_sps(ctx, ctrls->sps);
-	if (ret)
-		return ret;
-
 	ctrls->pps =
 		hantro_get_ctrl(ctx, V4L2_CID_MPEG_VIDEO_HEVC_PPS);
 	if (WARN_ON(!ctrls->pps))
diff --git a/drivers/staging/media/hantro/hantro_hw.h b/drivers/staging/media/hantro/hantro_hw.h
index a2e0f0836281..5edff0f0be20 100644
--- a/drivers/staging/media/hantro/hantro_hw.h
+++ b/drivers/staging/media/hantro/hantro_hw.h
@@ -359,7 +359,6 @@ int hantro_hevc_dec_prepare_run(struct hantro_ctx *ctx);
 void hantro_hevc_ref_init(struct hantro_ctx *ctx);
 dma_addr_t hantro_hevc_get_ref_buf(struct hantro_ctx *ctx, int poc);
 int hantro_hevc_add_ref_buf(struct hantro_ctx *ctx, int poc, dma_addr_t addr);
-int hantro_hevc_validate_sps(struct hantro_ctx *ctx, const struct v4l2_ctrl_hevc_sps *sps);
 
 
 static inline unsigned short hantro_vp9_num_sbs(unsigned short dimension)
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-07-19 19:34 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-18 21:41 [PATCH] media: destage Hantro VPU driver Ezequiel Garcia
2022-07-18 21:41 ` [PATCH] hantro: Remove dedicated control documentation Ezequiel Garcia
2022-07-19 19:34   ` Nicolas Dufresne
2022-07-18 21:41 ` [PATCH] hantro: Remove incorrect HEVC SPS validation Ezequiel Garcia
2022-07-18 21:48   ` Ezequiel Garcia
2022-07-19 19:33 ` [PATCH] media: destage Hantro VPU driver Nicolas Dufresne
  -- strict thread matches above, loose matches on Subject: below --
2022-06-29 19:56 [PATCH] hantro: Fix RK3399 H.264 format advertising Ezequiel Garcia
2022-06-29 19:56 ` [PATCH] hantro: Remove incorrect HEVC SPS validation Ezequiel Garcia
2022-06-30  5:02   ` Sebastian Fricke
2022-07-08  8:42     ` Hans Verkuil
2022-07-18 12:08       ` Ezequiel Garcia

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.