Hi Sidraya, On 18/08/2021 16:10, sidraya.bj@xxxxxxxxxxxxxxxxxxx wrote: > From: Sidraya <sidraya.bj@xxxxxxxxxxxxxxxxxxx> > > This series of patches implements v4l2 based Decoder driver for H264, > H265 and MJPEG decoding standards.This Driver is for D5520 H/W decoder on > DRA8x SOC of J721e platform. > This driver has been tested on v5.14-rc6 kernel for following > decoding standards on v4l2 based Gstreamer 1.16 plug-in. > 1. H264 > 2. H265 > 3. MJPEG > > Note: > Currently Driver uses list, map and queue custom data structure APIs > implementation and IOMMU custom framework.We are working on replacing > customised APIs with Linux kernel generic framework APIs. > Meanwhile would like to address review comments from > reviewers before merging to main media/platform subsystem. OK, so I consider this an RFC series rather than an actual driver submission and I'll mark it as such in patchwork. First of all, patch 13/30 never made it to the linux-media mailinglist, so the series as a whole will not apply. Can you repost 13/30? One possible reason why 13/30 might have been dropped is if it it a really large patch. You may have to split it up in that case. Did you run v4l2-compliance for this driver? Always compile v4l2-compliance (part of https://git.linuxtv.org/v4l-utils.git/) from the git repo sources to make sure you have most recent tests. I need to see the output of 'v4l2-compliance' as part of the cover letter. There shouldn't be any failures. I see a lot of pointless #ifdef DEBUG_DECODER_DRIVER lines. Either just drop the debug code or use dev_dbg/pr_debug. Ditto for VDEC_ASSERT(). This really pollutes the code. Can you provide a high-level description of the hardware? It seems like this driver is a lot more complex than other decoder drivers, but it is not immediately clear why that is. One reason might be that the hardware/firmware is a stateless decoder, while the driver exposes a stateful decoder API. See the m2m interface documentation for the differences between the two: https://hverkuil.home.xs4all.nl/spec/userspace-api/v4l/dev-mem2mem.html If that's the case, then the driver should really be a stateless driver, that will likely make things much easier. In any case, this driver clearly needs a lot more work. Regards, Hans > > Sidraya (30): > dt-bindings: Add binding for img,d5500-vxd for DRA8x > v4l: vxd-dec: Create mmu programming helper library > v4l: vxd-dec: Create vxd_dec Mem Manager helper library > v4l: vxd-dec: Add vxd helper library > v4l: vxd-dec: Add IMG VXD Video Decoder mem to mem drive > v4l: vxd-dec: Add hardware control modules > v4l: vxd-dec: Add vxd core module > v4l: vxd-dec: Add translation control modules > v4l: vxd-dec: Add idgen api modules > v4l: vxd-dec: Add utility modules > v4l: vxd-dec: Add TALMMU module > v4l: vxd-dec: Add VDEC MMU wrapper > v4l: vxd-dec: Add Bistream Preparser (BSPP) module > v4l: vxd-dec: Add common headers > v4l: vxd-dec: Add firmware interface headers > v4l: vxd-dec: Add pool api modules > v4l: vxd-dec: This patch implements resource manage component > v4l: vxd-dec: This patch implements pixel processing library > v4l:vxd-dec:vdecdd utility library > v4l:vxd-dec:Decoder resource component > v4l:vxd-dec:Decoder Core Component > v4l:vxd-dec:vdecdd headers added > v4l:vxd-dec:Decoder Component > v4l:vxd-dec: Add resource manager > v4l: videodev2: Add 10bit definitions for NV12 and NV16 color formats > media: Kconfig: Add Video decoder kconfig and Makefile entries > media: platform: vxd: Kconfig: Add Video decoder Kconfig and Makefile > IMG DEC V4L2 Interface function implementations > arm64: dts: dra82: Add v4l2 vxd_dec device node > ARM64: ti_sdk_arm64_release_defconfig: Enable d5520 video decoder > driver > > .../bindings/media/img,d5520-vxd.yaml | 52 + > MAINTAINERS | 114 + > arch/arm64/boot/dts/ti/k3-j721e-main.dtsi | 9 + > .../configs/ti_sdk_arm64_release_defconfig | 7407 +++++++++++++++++ > drivers/media/v4l2-core/v4l2-ioctl.c | 2 + > drivers/staging/media/Kconfig | 2 + > drivers/staging/media/Makefile | 1 + > drivers/staging/media/vxd/common/addr_alloc.c | 499 ++ > drivers/staging/media/vxd/common/addr_alloc.h | 238 + > drivers/staging/media/vxd/common/dq.c | 248 + > drivers/staging/media/vxd/common/dq.h | 36 + > drivers/staging/media/vxd/common/hash.c | 481 ++ > drivers/staging/media/vxd/common/hash.h | 86 + > drivers/staging/media/vxd/common/idgen_api.c | 449 + > drivers/staging/media/vxd/common/idgen_api.h | 59 + > drivers/staging/media/vxd/common/img_errors.h | 104 + > drivers/staging/media/vxd/common/img_mem.h | 43 + > .../staging/media/vxd/common/img_mem_man.c | 1124 +++ > .../staging/media/vxd/common/img_mem_man.h | 231 + > .../media/vxd/common/img_mem_unified.c | 276 + > drivers/staging/media/vxd/common/imgmmu.c | 782 ++ > drivers/staging/media/vxd/common/imgmmu.h | 180 + > drivers/staging/media/vxd/common/lst.c | 119 + > drivers/staging/media/vxd/common/lst.h | 37 + > drivers/staging/media/vxd/common/pool.c | 228 + > drivers/staging/media/vxd/common/pool.h | 66 + > drivers/staging/media/vxd/common/pool_api.c | 709 ++ > drivers/staging/media/vxd/common/pool_api.h | 113 + > drivers/staging/media/vxd/common/ra.c | 972 +++ > drivers/staging/media/vxd/common/ra.h | 200 + > drivers/staging/media/vxd/common/resource.c | 576 ++ > drivers/staging/media/vxd/common/resource.h | 66 + > drivers/staging/media/vxd/common/rman_api.c | 620 ++ > drivers/staging/media/vxd/common/rman_api.h | 66 + > drivers/staging/media/vxd/common/talmmu_api.c | 753 ++ > drivers/staging/media/vxd/common/talmmu_api.h | 246 + > drivers/staging/media/vxd/common/vid_buf.h | 42 + > drivers/staging/media/vxd/common/work_queue.c | 188 + > drivers/staging/media/vxd/common/work_queue.h | 66 + > drivers/staging/media/vxd/decoder/Kconfig | 13 + > drivers/staging/media/vxd/decoder/Makefile | 129 + > drivers/staging/media/vxd/decoder/bspp.c | 2479 ++++++ > drivers/staging/media/vxd/decoder/bspp.h | 363 + > drivers/staging/media/vxd/decoder/bspp_int.h | 514 ++ > drivers/staging/media/vxd/decoder/core.c | 3656 ++++++++ > drivers/staging/media/vxd/decoder/core.h | 72 + > .../staging/media/vxd/decoder/dec_resources.c | 554 ++ > .../staging/media/vxd/decoder/dec_resources.h | 46 + > drivers/staging/media/vxd/decoder/decoder.c | 4622 ++++++++++ > drivers/staging/media/vxd/decoder/decoder.h | 375 + > .../staging/media/vxd/decoder/fw_interface.h | 818 ++ > drivers/staging/media/vxd/decoder/h264_idx.h | 60 + > .../media/vxd/decoder/h264_secure_parser.c | 3051 +++++++ > .../media/vxd/decoder/h264_secure_parser.h | 278 + > drivers/staging/media/vxd/decoder/h264_vlc.h | 604 ++ > .../staging/media/vxd/decoder/h264fw_data.h | 652 ++ > .../media/vxd/decoder/h264fw_data_shared.h | 759 ++ > .../media/vxd/decoder/hevc_secure_parser.c | 2895 +++++++ > .../media/vxd/decoder/hevc_secure_parser.h | 455 + > .../staging/media/vxd/decoder/hevcfw_data.h | 472 ++ > .../media/vxd/decoder/hevcfw_data_shared.h | 767 ++ > .../staging/media/vxd/decoder/hw_control.c | 1211 +++ > .../staging/media/vxd/decoder/hw_control.h | 144 + > .../media/vxd/decoder/img_dec_common.h | 278 + > .../media/vxd/decoder/img_msvdx_cmds.h | 279 + > .../media/vxd/decoder/img_msvdx_core_regs.h | 22 + > .../media/vxd/decoder/img_msvdx_vdmc_regs.h | 26 + > .../media/vxd/decoder/img_msvdx_vec_regs.h | 60 + > .../staging/media/vxd/decoder/img_pixfmts.h | 195 + > .../media/vxd/decoder/img_profiles_levels.h | 33 + > .../media/vxd/decoder/img_pvdec_core_regs.h | 60 + > .../media/vxd/decoder/img_pvdec_pixel_regs.h | 35 + > .../media/vxd/decoder/img_pvdec_test_regs.h | 39 + > .../media/vxd/decoder/img_vdec_fw_msg.h | 192 + > .../vxd/decoder/img_video_bus4_mmu_regs.h | 120 + > .../media/vxd/decoder/jpeg_secure_parser.c | 645 ++ > .../media/vxd/decoder/jpeg_secure_parser.h | 37 + > .../staging/media/vxd/decoder/jpegfw_data.h | 83 + > .../media/vxd/decoder/jpegfw_data_shared.h | 84 + > drivers/staging/media/vxd/decoder/mem_io.h | 42 + > drivers/staging/media/vxd/decoder/mmu_defs.h | 42 + > drivers/staging/media/vxd/decoder/pixel_api.c | 895 ++ > drivers/staging/media/vxd/decoder/pixel_api.h | 152 + > .../media/vxd/decoder/pvdec_entropy_regs.h | 33 + > drivers/staging/media/vxd/decoder/pvdec_int.h | 82 + > .../media/vxd/decoder/pvdec_vec_be_regs.h | 35 + > drivers/staging/media/vxd/decoder/reg_io2.h | 74 + > .../staging/media/vxd/decoder/scaler_setup.h | 59 + > drivers/staging/media/vxd/decoder/swsr.c | 1657 ++++ > drivers/staging/media/vxd/decoder/swsr.h | 278 + > .../media/vxd/decoder/translation_api.c | 1725 ++++ > .../media/vxd/decoder/translation_api.h | 42 + > drivers/staging/media/vxd/decoder/vdec_defs.h | 548 ++ > .../media/vxd/decoder/vdec_mmu_wrapper.c | 829 ++ > .../media/vxd/decoder/vdec_mmu_wrapper.h | 174 + > .../staging/media/vxd/decoder/vdecdd_defs.h | 446 + > .../staging/media/vxd/decoder/vdecdd_utils.c | 95 + > .../staging/media/vxd/decoder/vdecdd_utils.h | 93 + > .../media/vxd/decoder/vdecdd_utils_buf.c | 897 ++ > .../staging/media/vxd/decoder/vdecfw_share.h | 36 + > .../staging/media/vxd/decoder/vdecfw_shared.h | 893 ++ > drivers/staging/media/vxd/decoder/vxd_core.c | 1683 ++++ > drivers/staging/media/vxd/decoder/vxd_dec.c | 185 + > drivers/staging/media/vxd/decoder/vxd_dec.h | 477 ++ > drivers/staging/media/vxd/decoder/vxd_ext.h | 74 + > drivers/staging/media/vxd/decoder/vxd_int.c | 1137 +++ > drivers/staging/media/vxd/decoder/vxd_int.h | 128 + > .../staging/media/vxd/decoder/vxd_mmu_defs.h | 30 + > drivers/staging/media/vxd/decoder/vxd_props.h | 80 + > drivers/staging/media/vxd/decoder/vxd_pvdec.c | 1745 ++++ > .../media/vxd/decoder/vxd_pvdec_priv.h | 126 + > .../media/vxd/decoder/vxd_pvdec_regs.h | 779 ++ > drivers/staging/media/vxd/decoder/vxd_v4l2.c | 2129 +++++ > include/uapi/linux/videodev2.h | 2 + > 114 files changed, 62369 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/img,d5520-vxd.yaml > create mode 100644 arch/arm64/configs/ti_sdk_arm64_release_defconfig > create mode 100644 drivers/staging/media/vxd/common/addr_alloc.c > create mode 100644 drivers/staging/media/vxd/common/addr_alloc.h > create mode 100644 drivers/staging/media/vxd/common/dq.c > create mode 100644 drivers/staging/media/vxd/common/dq.h > create mode 100644 drivers/staging/media/vxd/common/hash.c > create mode 100644 drivers/staging/media/vxd/common/hash.h > create mode 100644 drivers/staging/media/vxd/common/idgen_api.c > create mode 100644 drivers/staging/media/vxd/common/idgen_api.h > create mode 100644 drivers/staging/media/vxd/common/img_errors.h > create mode 100644 drivers/staging/media/vxd/common/img_mem.h > create mode 100644 drivers/staging/media/vxd/common/img_mem_man.c > create mode 100644 drivers/staging/media/vxd/common/img_mem_man.h > create mode 100644 drivers/staging/media/vxd/common/img_mem_unified.c > create mode 100644 drivers/staging/media/vxd/common/imgmmu.c > create mode 100644 drivers/staging/media/vxd/common/imgmmu.h > create mode 100644 drivers/staging/media/vxd/common/lst.c > create mode 100644 drivers/staging/media/vxd/common/lst.h > create mode 100644 drivers/staging/media/vxd/common/pool.c > create mode 100644 drivers/staging/media/vxd/common/pool.h > create mode 100644 drivers/staging/media/vxd/common/pool_api.c > create mode 100644 drivers/staging/media/vxd/common/pool_api.h > create mode 100644 drivers/staging/media/vxd/common/ra.c > create mode 100644 drivers/staging/media/vxd/common/ra.h > create mode 100644 drivers/staging/media/vxd/common/resource.c > create mode 100644 drivers/staging/media/vxd/common/resource.h > create mode 100644 drivers/staging/media/vxd/common/rman_api.c > create mode 100644 drivers/staging/media/vxd/common/rman_api.h > create mode 100644 drivers/staging/media/vxd/common/talmmu_api.c > create mode 100644 drivers/staging/media/vxd/common/talmmu_api.h > create mode 100644 drivers/staging/media/vxd/common/vid_buf.h > create mode 100644 drivers/staging/media/vxd/common/work_queue.c > create mode 100644 drivers/staging/media/vxd/common/work_queue.h > create mode 100644 drivers/staging/media/vxd/decoder/Kconfig > create mode 100644 drivers/staging/media/vxd/decoder/Makefile > create mode 100644 drivers/staging/media/vxd/decoder/bspp.c > create mode 100644 drivers/staging/media/vxd/decoder/bspp.h > create mode 100644 drivers/staging/media/vxd/decoder/bspp_int.h > create mode 100644 drivers/staging/media/vxd/decoder/core.c > create mode 100644 drivers/staging/media/vxd/decoder/core.h > create mode 100644 drivers/staging/media/vxd/decoder/dec_resources.c > create mode 100644 drivers/staging/media/vxd/decoder/dec_resources.h > create mode 100644 drivers/staging/media/vxd/decoder/decoder.c > create mode 100644 drivers/staging/media/vxd/decoder/decoder.h > create mode 100644 drivers/staging/media/vxd/decoder/fw_interface.h > create mode 100644 drivers/staging/media/vxd/decoder/h264_idx.h > create mode 100644 drivers/staging/media/vxd/decoder/h264_secure_parser.c > create mode 100644 drivers/staging/media/vxd/decoder/h264_secure_parser.h > create mode 100644 drivers/staging/media/vxd/decoder/h264_vlc.h > create mode 100644 drivers/staging/media/vxd/decoder/h264fw_data.h > create mode 100644 drivers/staging/media/vxd/decoder/h264fw_data_shared.h > create mode 100644 drivers/staging/media/vxd/decoder/hevc_secure_parser.c > create mode 100644 drivers/staging/media/vxd/decoder/hevc_secure_parser.h > create mode 100644 drivers/staging/media/vxd/decoder/hevcfw_data.h > create mode 100644 drivers/staging/media/vxd/decoder/hevcfw_data_shared.h > create mode 100644 drivers/staging/media/vxd/decoder/hw_control.c > create mode 100644 drivers/staging/media/vxd/decoder/hw_control.h > create mode 100644 drivers/staging/media/vxd/decoder/img_dec_common.h > create mode 100644 drivers/staging/media/vxd/decoder/img_msvdx_cmds.h > create mode 100644 drivers/staging/media/vxd/decoder/img_msvdx_core_regs.h > create mode 100644 drivers/staging/media/vxd/decoder/img_msvdx_vdmc_regs.h > create mode 100644 drivers/staging/media/vxd/decoder/img_msvdx_vec_regs.h > create mode 100644 drivers/staging/media/vxd/decoder/img_pixfmts.h > create mode 100644 drivers/staging/media/vxd/decoder/img_profiles_levels.h > create mode 100644 drivers/staging/media/vxd/decoder/img_pvdec_core_regs.h > create mode 100644 drivers/staging/media/vxd/decoder/img_pvdec_pixel_regs.h > create mode 100644 drivers/staging/media/vxd/decoder/img_pvdec_test_regs.h > create mode 100644 drivers/staging/media/vxd/decoder/img_vdec_fw_msg.h > create mode 100644 drivers/staging/media/vxd/decoder/img_video_bus4_mmu_regs.h > create mode 100644 drivers/staging/media/vxd/decoder/jpeg_secure_parser.c > create mode 100644 drivers/staging/media/vxd/decoder/jpeg_secure_parser.h > create mode 100644 drivers/staging/media/vxd/decoder/jpegfw_data.h > create mode 100644 drivers/staging/media/vxd/decoder/jpegfw_data_shared.h > create mode 100644 drivers/staging/media/vxd/decoder/mem_io.h > create mode 100644 drivers/staging/media/vxd/decoder/mmu_defs.h > create mode 100644 drivers/staging/media/vxd/decoder/pixel_api.c > create mode 100644 drivers/staging/media/vxd/decoder/pixel_api.h > create mode 100644 drivers/staging/media/vxd/decoder/pvdec_entropy_regs.h > create mode 100644 drivers/staging/media/vxd/decoder/pvdec_int.h > create mode 100644 drivers/staging/media/vxd/decoder/pvdec_vec_be_regs.h > create mode 100644 drivers/staging/media/vxd/decoder/reg_io2.h > create mode 100644 drivers/staging/media/vxd/decoder/scaler_setup.h > create mode 100644 drivers/staging/media/vxd/decoder/swsr.c > create mode 100644 drivers/staging/media/vxd/decoder/swsr.h > create mode 100644 drivers/staging/media/vxd/decoder/translation_api.c > create mode 100644 drivers/staging/media/vxd/decoder/translation_api.h > create mode 100644 drivers/staging/media/vxd/decoder/vdec_defs.h > create mode 100644 drivers/staging/media/vxd/decoder/vdec_mmu_wrapper.c > create mode 100644 drivers/staging/media/vxd/decoder/vdec_mmu_wrapper.h > create mode 100644 drivers/staging/media/vxd/decoder/vdecdd_defs.h > create mode 100644 drivers/staging/media/vxd/decoder/vdecdd_utils.c > create mode 100644 drivers/staging/media/vxd/decoder/vdecdd_utils.h > create mode 100644 drivers/staging/media/vxd/decoder/vdecdd_utils_buf.c > create mode 100644 drivers/staging/media/vxd/decoder/vdecfw_share.h > create mode 100644 drivers/staging/media/vxd/decoder/vdecfw_shared.h > create mode 100644 drivers/staging/media/vxd/decoder/vxd_core.c > create mode 100644 drivers/staging/media/vxd/decoder/vxd_dec.c > create mode 100644 drivers/staging/media/vxd/decoder/vxd_dec.h > create mode 100644 drivers/staging/media/vxd/decoder/vxd_ext.h > create mode 100644 drivers/staging/media/vxd/decoder/vxd_int.c > create mode 100644 drivers/staging/media/vxd/decoder/vxd_int.h > create mode 100644 drivers/staging/media/vxd/decoder/vxd_mmu_defs.h > create mode 100644 drivers/staging/media/vxd/decoder/vxd_props.h > create mode 100644 drivers/staging/media/vxd/decoder/vxd_pvdec.c > create mode 100644 drivers/staging/media/vxd/decoder/vxd_pvdec_priv.h > create mode 100644 drivers/staging/media/vxd/decoder/vxd_pvdec_regs.h > create mode 100644 drivers/staging/media/vxd/decoder/vxd_v4l2.c >