| Message ID | 20250128080429.3911091-2-quic_dikshita@quicinc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add helper module to select video driver | expand |
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@quicinc.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 >
On 1/28/2025 9:44 PM, Dmitry Baryshkov wrote: > 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@quicinc.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? iris and venus are always built as module, and if we are adding the dependency of this module on IRIS && VENUS then this can't be Y I think. > 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 >> >
On 28/01/2025 09:04, 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@quicinc.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); > > #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; Wouldn't it mark the probe as failed and cause dmesg regressions? > + > 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); You just defined it in the header. Why is this here? > + > 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; Same problems - d,esg regression. > + > 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 @@ Missing SPDX > +# Makefile for Video driver helper > + > +obj-m := video_drv_helper.o > + > 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 Use Linux generic coding style comment, not netdev. > + * 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 > +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); The choice of driver is by module blacklisting, not by failing probes. I don't understand why this patchset is needed and neither commit msg nor above longer code comment explain me that. Just blacklist the module. Best regards, Krzysztof
On Wed, Jan 29, 2025 at 03:23:22PM +0530, Dikshita Agarwal wrote: > > > On 1/28/2025 9:44 PM, Dmitry Baryshkov wrote: > > 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@quicinc.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? > iris and venus are always built as module, This is not correct. > and if we are adding the > dependency of this module on IRIS && VENUS then this can't be Y I think. It surely can. Moreover, if one doesn't enable both Iris and Venus, this module is completely unnecessary. > > 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/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); #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); + 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 + 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 +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) +{ + 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); + +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
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@quicinc.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