| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <sb3beoyhnlcdfjbm37ogpdoph7m4fecpbuu3myglnpzblpnqhw@wdyskeps3uuh>
Date: Tue, 28 Jan 2025 18:14:14 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Dikshita Agarwal <quic_dikshita@...cinc.com>
Cc: quic_vgarodia@...cinc.com, quic_abhinavk@...cinc.com,
mchehab@...nel.org, robh@...nel.org, krzk+dt@...nel.org, p.zabel@...gutronix.de,
hverkuil@...all.nl, sebastian.fricke@...labora.com, bryan.odonoghue@...aro.org,
neil.armstrong@...aro.org, nicolas@...fresne.ca, u.kleine-koenig@...libre.com,
stefan.schmidt@...aro.org, lujianhua000@...il.com, linux-arm-msm@...r.kernel.org,
linux-media@...r.kernel.org, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
krzysztof.kozlowski@...aro.org, johan@...nel.org
Subject: Re: [RFC PATCH v10 1/2] media: iris: introduce helper module to
select video driver
On Tue, Jan 28, 2025 at 01:34:28PM +0530, Dikshita Agarwal wrote:
> Introduce a helper module with a kernel param to select between
> venus and iris drivers for platforms supported by both drivers.
>
> Signed-off-by: Dikshita Agarwal <quic_dikshita@...cinc.com>
> ---
> drivers/media/platform/qcom/Makefile | 1 +
> drivers/media/platform/qcom/iris/iris_core.h | 1 +
> drivers/media/platform/qcom/iris/iris_probe.c | 3 +
> drivers/media/platform/qcom/venus/core.c | 5 ++
> .../platform/qcom/video_drv_helper/Makefile | 4 ++
> .../qcom/video_drv_helper/video_drv_helper.c | 70 +++++++++++++++++++
> .../qcom/video_drv_helper/video_drv_helper.h | 11 +++
> 7 files changed, 95 insertions(+)
> create mode 100644 drivers/media/platform/qcom/video_drv_helper/Makefile
> create mode 100644 drivers/media/platform/qcom/video_drv_helper/video_drv_helper.c
> create mode 100644 drivers/media/platform/qcom/video_drv_helper/video_drv_helper.h
>
> diff --git a/drivers/media/platform/qcom/Makefile b/drivers/media/platform/qcom/Makefile
> index ea2221a202c0..15accde3bd67 100644
> --- a/drivers/media/platform/qcom/Makefile
> +++ b/drivers/media/platform/qcom/Makefile
> @@ -2,3 +2,4 @@
> obj-y += camss/
> obj-y += iris/
> obj-y += venus/
> +obj-y += video_drv_helper/
> diff --git a/drivers/media/platform/qcom/iris/iris_core.h b/drivers/media/platform/qcom/iris/iris_core.h
> index 37fb4919fecc..7108e751ff88 100644
> --- a/drivers/media/platform/qcom/iris/iris_core.h
> +++ b/drivers/media/platform/qcom/iris/iris_core.h
> @@ -107,5 +107,6 @@ struct iris_core {
>
> int iris_core_init(struct iris_core *core);
> void iris_core_deinit(struct iris_core *core);
> +extern bool video_drv_should_bind(struct device *dev, bool is_iris_driver);
s/extern //g
>
> #endif
> diff --git a/drivers/media/platform/qcom/iris/iris_probe.c b/drivers/media/platform/qcom/iris/iris_probe.c
> index 954cc7c0cc97..276461ade811 100644
> --- a/drivers/media/platform/qcom/iris/iris_probe.c
> +++ b/drivers/media/platform/qcom/iris/iris_probe.c
> @@ -196,6 +196,9 @@ static int iris_probe(struct platform_device *pdev)
> u64 dma_mask;
> int ret;
>
> + if (!video_drv_should_bind(&pdev->dev, true))
> + return -ENODEV;
> +
> core = devm_kzalloc(&pdev->dev, sizeof(*core), GFP_KERNEL);
> if (!core)
> return -ENOMEM;
> diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c
> index 77d48578ecd2..b38be7812efe 100644
> --- a/drivers/media/platform/qcom/venus/core.c
> +++ b/drivers/media/platform/qcom/venus/core.c
> @@ -369,12 +369,17 @@ static int venus_add_dynamic_nodes(struct venus_core *core)
> static void venus_remove_dynamic_nodes(struct venus_core *core) {}
> #endif
>
> +extern bool video_drv_should_bind(struct device *dev, bool is_iris_driver);
Use #include instead.
> +
> static int venus_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> struct venus_core *core;
> int ret;
>
> + if (!video_drv_should_bind(&pdev->dev, false))
> + return -ENODEV;
> +
> core = devm_kzalloc(dev, sizeof(*core), GFP_KERNEL);
> if (!core)
> return -ENOMEM;
> diff --git a/drivers/media/platform/qcom/video_drv_helper/Makefile b/drivers/media/platform/qcom/video_drv_helper/Makefile
> new file mode 100644
> index 000000000000..82567e0392fb
> --- /dev/null
> +++ b/drivers/media/platform/qcom/video_drv_helper/Makefile
> @@ -0,0 +1,4 @@
> +# Makefile for Video driver helper
> +
> +obj-m := video_drv_helper.o
Always built as a module? And what if iris or venus are built into the
kernel?
Add a normal Kconfig symbol, tristate, no Kconfig string. Use depends on
IRIS && VENUS (and maybe default y) to let it be built only if both
drivers are enabled.
> +
> diff --git a/drivers/media/platform/qcom/video_drv_helper/video_drv_helper.c b/drivers/media/platform/qcom/video_drv_helper/video_drv_helper.c
> new file mode 100644
> index 000000000000..9009c2906e54
> --- /dev/null
> +++ b/drivers/media/platform/qcom/video_drv_helper/video_drv_helper.c
> @@ -0,0 +1,70 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2025 Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +
> +#include "video_drv_helper.h"
> +
> +/* The venus driver supports only hfi gen1 to communicate with the firmware while
> + * the iris driver supports both hfi gen1 and hfi gen2.
> + * The support of hfi gen1 is added to the iris driver with the intention that
> + * it can support old gen1 interface based firmware, while enabling gen2 based future SOCs.
> + * With this, the plan is to migrate older SOCs from venus to iris.
> + * As of now, since the iris driver supports only entry level features and doesn't have
> + * feature parity with the venus driver, a runtime-selection is provided to user via
> + * module parameter 'prefer_venus' to select the driver.
> + * This selection is available only for the SoCs which are supported by both venus
> + * and iris eg: SM8250.
> + * When the feature parity is achieved, the plan is to switch the default to point to
> + * the iris driver, then gradually start removing platforms from venus.
> + * Hardware supported by only venus - 8916, 8996, SDM660, SDM845, SC7180, SC7280
> + * Hardware supported by only iris - SM8550
> + * Hardware supported by both venus and iris - SM8250
> + */
> +
> +#if !IS_REACHABLE(CONFIG_VIDEO_QCOM_VENUS) || !IS_REACHABLE(CONFIG_VIDEO_QCOM_IRIS)
> +bool video_drv_should_bind(struct device *dev, bool is_iris_driver)
> +{
> + /* If just a single driver is enabled, use it no matter what */
> + return true;
> +}
> +
> +#else
Move the stub funtion to header.
> +static bool prefer_venus = true;
> +MODULE_PARM_DESC(prefer_venus, "Select whether venus or iris driver should be preferred");
> +module_param(prefer_venus, bool, 0444);
> +
> +/* list all platforms supported by both venus and iris drivers */
> +static const char *const venus_to_iris_migration[] = {
> + "qcom,sm8250-venus",
> + NULL,
> +};
> +
> +bool video_drv_should_bind(struct device *dev, bool is_iris_driver)
The prefix is too broad, but maybe its fine.
> +{
> + if (of_device_compatible_match(dev->of_node, venus_to_iris_migration))
> + return prefer_venus ? !is_iris_driver : is_iris_driver;
> +
> + return true;
> +}
> +EXPORT_SYMBOL_GPL(video_drv_should_bind);
> +#endif
> +
> +static int __init video_drv_helper_init(void)
> +{
> + return 0;
> +}
> +
> +static void __exit video_drv_helper_exit(void)
> +{
> +}
> +
> +module_init(video_drv_helper_init);
> +module_exit(video_drv_helper_exit);
No need for this, please drop.
> +
> +MODULE_DESCRIPTION("A video driver helper module");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/media/platform/qcom/video_drv_helper/video_drv_helper.h b/drivers/media/platform/qcom/video_drv_helper/video_drv_helper.h
> new file mode 100644
> index 000000000000..6d835227fec2
> --- /dev/null
> +++ b/drivers/media/platform/qcom/video_drv_helper/video_drv_helper.h
> @@ -0,0 +1,11 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (c) 2025 Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +
> +#ifndef __VIDEO_DRV_HELPER_H__
> +#define __VIDEO_DRV_HELPER_H__
> +
> +bool video_drv_should_bind(struct device *dev, bool is_iris_driver);
> +
> +#endif
> --
> 2.34.1
>
--
With best wishes
Dmitry
Powered by blists - more mailing lists