Re: [PATCH 33/33] iris: enable building of iris video driver

Dmitry Baryshkov
2023-07-28T14:41:00Z

On Fri, 28 Jul 2023 at 17:28, Vikash Garodia <quic_vgarodia@quicinc.com> wrote:
>
> From: Dikshita Agarwal <quic_dikshita@quicinc.com>
>
> This adds iris driver Makefile and Kconfig, also changes
> v4l2 platform/qcom Makefile/Kconfig in order to
> enable compilation of the driver.
>
> Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com>
> Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
> ---
> drivers/media/platform/qcom/Kconfig | 1 +
> drivers/media/platform/qcom/Makefile | 1 +
> drivers/media/platform/qcom/iris/Kconfig | 15 ++++++++++
> drivers/media/platform/qcom/iris/Makefile | 46 +++++++++++++++++++++++++++++++
> 4 files changed, 63 insertions(+)
> create mode 100644 drivers/media/platform/qcom/iris/Kconfig
> create mode 100644 drivers/media/platform/qcom/iris/Makefile
>
> diff --git a/drivers/media/platform/qcom/Kconfig b/drivers/media/platform/qcom/Kconfig
> index cc5799b..b86bebd 100644
> --- a/drivers/media/platform/qcom/Kconfig
> +++ b/drivers/media/platform/qcom/Kconfig
> @@ -4,3 +4,4 @@ comment "Qualcomm media platform drivers"
>
> source "drivers/media/platform/qcom/camss/Kconfig"
> source "drivers/media/platform/qcom/venus/Kconfig"
> +source "drivers/media/platform/qcom/iris/Kconfig"
> diff --git a/drivers/media/platform/qcom/Makefile b/drivers/media/platform/qcom/Makefile
> index 4f055c3..83eea29 100644
> --- a/drivers/media/platform/qcom/Makefile
> +++ b/drivers/media/platform/qcom/Makefile
> @@ -1,3 +1,4 @@
> # SPDX-License-Identifier: GPL-2.0-only
> obj-y += camss/
> obj-y += venus/
> +obj-y += iris/
> diff --git a/drivers/media/platform/qcom/iris/Kconfig b/drivers/media/platform/qcom/iris/Kconfig
> new file mode 100644
> index 0000000..d434c31
> --- /dev/null
> +++ b/drivers/media/platform/qcom/iris/Kconfig
> @@ -0,0 +1,15 @@
> +config VIDEO_QCOM_IRIS
> + tristate "Qualcomm Iris V4L2 encoder/decoder driver"
> + depends on V4L_MEM2MEM_DRIVERS
> + depends on VIDEO_DEV && QCOM_SMEM
> + depends on (ARCH_QCOM && IOMMU_DMA) || COMPILE_TEST
> + select QCOM_MDT_LOADER if ARCH_QCOM
> + select QCOM_SCM
> + select VIDEOBUF2_DMA_CONTIG
> + select V4L2_MEM2MEM_DEV
> + select DMABUF_HEAPS
> + help
> + This is a V4L2 driver for Qualcomm Iris video accelerator
> + hardware. It accelerates encoding and decoding operations
> + on various Qualcomm SoCs.
> + To compile this driver as a module choose m here.
> diff --git a/drivers/media/platform/qcom/iris/Makefile b/drivers/media/platform/qcom/iris/Makefile
> new file mode 100644
> index 0000000..e681c4f
> --- /dev/null
> +++ b/drivers/media/platform/qcom/iris/Makefile
> @@ -0,0 +1,46 @@
> +KBUILD_OPTIONS+= VIDEO_ROOT=$(KERNEL_SRC)/$(M)
> +
> +VIDEO_COMPILE_TIME = $(shell date)
> +VIDEO_COMPILE_BY = $(shell whoami | sed 's/\\/\\\\/')
> +VIDEO_COMPILE_HOST = $(shell uname -n)
> +VIDEO_GEN_PATH = $(srctree)/$(src)/vidc/inc/video_generated_h
> +
> +$(shell echo '#define VIDEO_COMPILE_TIME "$(VIDEO_COMPILE_TIME)"' > $(VIDEO_GEN_PATH))
> +$(shell echo '#define VIDEO_COMPILE_BY "$(VIDEO_COMPILE_BY)"' >> $(VIDEO_GEN_PATH))
> +$(shell echo '#define VIDEO_COMPILE_HOST "$(VIDEO_COMPILE_HOST)"' >> $(VIDEO_GEN_PATH))

Why do you need this at all?

> +
> +iris-objs += vidc/src/msm_vidc_v4l2.o \
> + vidc/src/msm_vidc_vb2.o \
> + vidc/src/msm_vidc.o \
> + vidc/src/msm_vdec.o \
> + vidc/src/msm_venc.o \
> + vidc/src/msm_vidc_driver.o \
> + vidc/src/msm_vidc_control.o \
> + vidc/src/msm_vidc_buffer.o \
> + vidc/src/msm_vidc_power.o \
> + vidc/src/msm_vidc_probe.o \
> + vidc/src/resources.o \
> + vidc/src/firmware.o \
> + vidc/src/msm_vidc_debug.o \
> + vidc/src/msm_vidc_memory.o \
> + vidc/src/venus_hfi.o \
> + vidc/src/venus_hfi_queue.o \
> + vidc/src/hfi_packet.o \
> + vidc/src/venus_hfi_response.o \
> + vidc/src/msm_vidc_state.o \
> + platform/common/src/msm_vidc_platform.o \
> + platform/sm8550/src/msm_vidc_sm8550.o \
> + variant/common/src/msm_vidc_variant.o \
> + variant/iris3/src/msm_vidc_buffer_iris3.o \
> + variant/iris3/src/msm_vidc_iris3.o \
> + variant/iris3/src/msm_vidc_power_iris3.o \
> + variant/iris3/src/msm_vidc_bus_iris3.o \
> + variant/iris3/src/msm_vidc_clock_iris3.o
> +
> +obj-$(CONFIG_VIDEO_QCOM_IRIS) += iris.o
> +
> +ccflags-y += -I$(srctree)/$(src)/vidc/inc
> +ccflags-y += -I$(srctree)/$(src)/platform/common/inc
> +ccflags-y += -I$(srctree)/$(src)/platform/sm8550/inc
> +ccflags-y += -I$(srctree)/$(src)/variant/common/inc
> +ccflags-y += -I$(srctree)/$(src)/variant/iris3/inc

For me this is a sign of the bad structure of the include files.
Please define proper interfaces between submodules. The parts of the
driver usually should include files from the top-level dir only (and
from the local subdirectory of course).

> --
> 2.7.4
>

--
With best wishes
Dmitry