diff mbox series

[v3,03/29] media: iris: add platform driver for iris video device

Message ID 20240827-iris_v3-v3-3-c5fdbbe65e70@quicinc.com (mailing list archive)
State New
Headers show
Series Qualcomm iris video decoder driver | expand

Commit Message

Dikshita Agarwal via B4 Relay Aug. 27, 2024, 10:05 a.m. UTC
From: Dikshita Agarwal <quic_dikshita@quicinc.com>

In preparation of adding H264 decode functionality,
add probe and remove functions to iris video platform driver.

Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com>
---
 drivers/media/platform/qcom/Kconfig           |   1 +
 drivers/media/platform/qcom/Makefile          |   1 +
 drivers/media/platform/qcom/iris/Kconfig      |   9 +++
 drivers/media/platform/qcom/iris/Makefile     |   3 +
 drivers/media/platform/qcom/iris/iris_core.h  |  29 +++++++
 drivers/media/platform/qcom/iris/iris_probe.c | 108 ++++++++++++++++++++++++++
 6 files changed, 151 insertions(+)

Comments

Bryan O'Donoghue Aug. 27, 2024, 2:08 p.m. UTC | #1
On 27/08/2024 11:05, Dikshita Agarwal via B4 Relay wrote:
> +static const struct of_device_id iris_dt_match[] = {
> +	{ .compatible = "qcom,sm8550-iris", },
> +	{ .compatible = "qcom,sm8250-venus", },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, iris_dt_match);

The enabling patch for the compat strings should come last - if its 
first then the time between the compat add and the last patch is a dead 
zone where things are bound to break on a booting board.

---
bod
Dmitry Baryshkov Aug. 29, 2024, 9:13 a.m. UTC | #2
On Tue, Aug 27, 2024 at 03:08:03PM GMT, Bryan O'Donoghue wrote:
> On 27/08/2024 11:05, Dikshita Agarwal via B4 Relay wrote:
> > +static const struct of_device_id iris_dt_match[] = {
> > +	{ .compatible = "qcom,sm8550-iris", },
> > +	{ .compatible = "qcom,sm8250-venus", },
> > +	{ },
> > +};
> > +MODULE_DEVICE_TABLE(of, iris_dt_match);
> 
> The enabling patch for the compat strings should come last - if its first
> then the time between the compat add and the last patch is a dead zone where
> things are bound to break on a booting board.

But then it's impossible to test the driver in the interim state.
Moreover enabling it at the end only makes it hard to follow platform
data changes. What about adding sm8550 at this point and adding sm8250
at the end? Or enabling qcom,sm8550-iris and the fake qcom,sm8250-iris
now (and clearly documenting it as fake) and as the last patch change it
to qcom,sm8250-venus.
Bryan O'Donoghue Aug. 29, 2024, 9:36 a.m. UTC | #3
On 29/08/2024 10:13, Dmitry Baryshkov wrote:
>   What about adding sm8550 at this point and adding sm8250
> at the end?

That works too.
Dikshita Agarwal Sept. 5, 2024, 6:12 a.m. UTC | #4
On 8/29/2024 2:43 PM, Dmitry Baryshkov wrote:
> On Tue, Aug 27, 2024 at 03:08:03PM GMT, Bryan O'Donoghue wrote:
>> On 27/08/2024 11:05, Dikshita Agarwal via B4 Relay wrote:
>>> +static const struct of_device_id iris_dt_match[] = {
>>> +	{ .compatible = "qcom,sm8550-iris", },
>>> +	{ .compatible = "qcom,sm8250-venus", },
>>> +	{ },
>>> +};
>>> +MODULE_DEVICE_TABLE(of, iris_dt_match);
>>
>> The enabling patch for the compat strings should come last - if its first
>> then the time between the compat add and the last patch is a dead zone where
>> things are bound to break on a booting board.
> 
> But then it's impossible to test the driver in the interim state.
> Moreover enabling it at the end only makes it hard to follow platform
> data changes. What about adding sm8550 at this point and adding sm8250
> at the end? Or enabling qcom,sm8550-iris and the fake qcom,sm8250-iris
> now (and clearly documenting it as fake) and as the last patch change it
> to qcom,sm8250-venus.

Sure, we will add qcom,sm8250-iris at this point so that it enables the
testing of the driver, and will add one patch at the last to add
qcom,sm8250-venus.

Thanks,
Dikshita
>
Dikshita Agarwal Sept. 5, 2024, 6:15 a.m. UTC | #5
On 9/5/2024 11:42 AM, Dikshita Agarwal wrote:
> 
> 
> On 8/29/2024 2:43 PM, Dmitry Baryshkov wrote:
>> On Tue, Aug 27, 2024 at 03:08:03PM GMT, Bryan O'Donoghue wrote:
>>> On 27/08/2024 11:05, Dikshita Agarwal via B4 Relay wrote:
>>>> +static const struct of_device_id iris_dt_match[] = {
>>>> +	{ .compatible = "qcom,sm8550-iris", },
>>>> +	{ .compatible = "qcom,sm8250-venus", },
>>>> +	{ },
>>>> +};
>>>> +MODULE_DEVICE_TABLE(of, iris_dt_match);
>>>
>>> The enabling patch for the compat strings should come last - if its first
>>> then the time between the compat add and the last patch is a dead zone where
>>> things are bound to break on a booting board.
>>
>> But then it's impossible to test the driver in the interim state.
>> Moreover enabling it at the end only makes it hard to follow platform
>> data changes. What about adding sm8550 at this point and adding sm8250
>> at the end? Or enabling qcom,sm8550-iris and the fake qcom,sm8250-iris
>> now (and clearly documenting it as fake) and as the last patch change it
>> to qcom,sm8250-venus.
> 
> Sure, we will add qcom,sm8250-iris at this point so that it enables the
> testing of the driver, and will add one patch at the last to add
> qcom,sm8250-venus.
Sorry fixing the typos. what I meant was,
we will add qcom,sm8550-iris at this point so that it enables the
testing of the driver, and will add one patch at the last to add
qcom,sm8250-venus.
> 
> Thanks,
> Dikshita
>>
>
Dmitry Baryshkov Sept. 5, 2024, 10:11 a.m. UTC | #6
On Thu, Sep 05, 2024 at 11:45:25AM GMT, Dikshita Agarwal wrote:
> 
> 
> On 9/5/2024 11:42 AM, Dikshita Agarwal wrote:
> > 
> > 
> > On 8/29/2024 2:43 PM, Dmitry Baryshkov wrote:
> >> On Tue, Aug 27, 2024 at 03:08:03PM GMT, Bryan O'Donoghue wrote:
> >>> On 27/08/2024 11:05, Dikshita Agarwal via B4 Relay wrote:
> >>>> +static const struct of_device_id iris_dt_match[] = {
> >>>> +	{ .compatible = "qcom,sm8550-iris", },
> >>>> +	{ .compatible = "qcom,sm8250-venus", },
> >>>> +	{ },
> >>>> +};
> >>>> +MODULE_DEVICE_TABLE(of, iris_dt_match);
> >>>
> >>> The enabling patch for the compat strings should come last - if its first
> >>> then the time between the compat add and the last patch is a dead zone where
> >>> things are bound to break on a booting board.
> >>
> >> But then it's impossible to test the driver in the interim state.
> >> Moreover enabling it at the end only makes it hard to follow platform
> >> data changes. What about adding sm8550 at this point and adding sm8250
> >> at the end? Or enabling qcom,sm8550-iris and the fake qcom,sm8250-iris
> >> now (and clearly documenting it as fake) and as the last patch change it
> >> to qcom,sm8250-venus.
> > 
> > Sure, we will add qcom,sm8250-iris at this point so that it enables the
> > testing of the driver, and will add one patch at the last to add
> > qcom,sm8250-venus.
> Sorry fixing the typos. what I meant was,
> we will add qcom,sm8550-iris at this point so that it enables the
> testing of the driver, and will add one patch at the last to add
> qcom,sm8250-venus.

I hope you meant 'to change qcom,sm8250-iris to qcom,sm8250-venus'. Also
please clearly document that qcom,sm8250-iris is a temporary thing just
to facilitate documentation and testing of the driver to be removed as a
last patch.

> > 
> > Thanks,
> > Dikshita
> >>
> >
Dikshita Agarwal Sept. 5, 2024, 10:59 a.m. UTC | #7
On 9/5/2024 3:41 PM, Dmitry Baryshkov wrote:
> On Thu, Sep 05, 2024 at 11:45:25AM GMT, Dikshita Agarwal wrote:
>>
>>
>> On 9/5/2024 11:42 AM, Dikshita Agarwal wrote:
>>>
>>>
>>> On 8/29/2024 2:43 PM, Dmitry Baryshkov wrote:
>>>> On Tue, Aug 27, 2024 at 03:08:03PM GMT, Bryan O'Donoghue wrote:
>>>>> On 27/08/2024 11:05, Dikshita Agarwal via B4 Relay wrote:
>>>>>> +static const struct of_device_id iris_dt_match[] = {
>>>>>> +	{ .compatible = "qcom,sm8550-iris", },
>>>>>> +	{ .compatible = "qcom,sm8250-venus", },
>>>>>> +	{ },
>>>>>> +};
>>>>>> +MODULE_DEVICE_TABLE(of, iris_dt_match);
>>>>>
>>>>> The enabling patch for the compat strings should come last - if its first
>>>>> then the time between the compat add and the last patch is a dead zone where
>>>>> things are bound to break on a booting board.
>>>>
>>>> But then it's impossible to test the driver in the interim state.
>>>> Moreover enabling it at the end only makes it hard to follow platform
>>>> data changes. What about adding sm8550 at this point and adding sm8250
>>>> at the end? Or enabling qcom,sm8550-iris and the fake qcom,sm8250-iris
>>>> now (and clearly documenting it as fake) and as the last patch change it
>>>> to qcom,sm8250-venus.
>>>
>>> Sure, we will add qcom,sm8250-iris at this point so that it enables the
>>> testing of the driver, and will add one patch at the last to add
>>> qcom,sm8250-venus.
>> Sorry fixing the typos. what I meant was,
>> we will add qcom,sm8550-iris at this point so that it enables the
>> testing of the driver, and will add one patch at the last to add
>> qcom,sm8250-venus.
> 
> I hope you meant 'to change qcom,sm8250-iris to qcom,sm8250-venus'. Also
> please clearly document that qcom,sm8250-iris is a temporary thing just
> to facilitate documentation and testing of the driver to be removed as a
> last patch.
> 
I was agreeing to follow this suggestion of yours
"What about adding sm8550 at this point and adding sm8250
at the end?"
Where we will add sm8550(qcom,sm8550-iris) first so driver can be tested on
sm8550 and add sm8250(qcom,sm8250-venus) in the last patch.
I think Bryan also agreed to the same.
>>>
>>> Thanks,
>>> Dikshita
>>>>
>>>
>
Dmitry Baryshkov Sept. 5, 2024, 11:07 a.m. UTC | #8
On Thu, 5 Sept 2024 at 13:59, Dikshita Agarwal
<quic_dikshita@quicinc.com> wrote:
>
>
>
> On 9/5/2024 3:41 PM, Dmitry Baryshkov wrote:
> > On Thu, Sep 05, 2024 at 11:45:25AM GMT, Dikshita Agarwal wrote:
> >>
> >>
> >> On 9/5/2024 11:42 AM, Dikshita Agarwal wrote:
> >>>
> >>>
> >>> On 8/29/2024 2:43 PM, Dmitry Baryshkov wrote:
> >>>> On Tue, Aug 27, 2024 at 03:08:03PM GMT, Bryan O'Donoghue wrote:
> >>>>> On 27/08/2024 11:05, Dikshita Agarwal via B4 Relay wrote:
> >>>>>> +static const struct of_device_id iris_dt_match[] = {
> >>>>>> +        { .compatible = "qcom,sm8550-iris", },
> >>>>>> +        { .compatible = "qcom,sm8250-venus", },
> >>>>>> +        { },
> >>>>>> +};
> >>>>>> +MODULE_DEVICE_TABLE(of, iris_dt_match);
> >>>>>
> >>>>> The enabling patch for the compat strings should come last - if its first
> >>>>> then the time between the compat add and the last patch is a dead zone where
> >>>>> things are bound to break on a booting board.
> >>>>
> >>>> But then it's impossible to test the driver in the interim state.
> >>>> Moreover enabling it at the end only makes it hard to follow platform
> >>>> data changes. What about adding sm8550 at this point and adding sm8250
> >>>> at the end? Or enabling qcom,sm8550-iris and the fake qcom,sm8250-iris
> >>>> now (and clearly documenting it as fake) and as the last patch change it
> >>>> to qcom,sm8250-venus.
> >>>
> >>> Sure, we will add qcom,sm8250-iris at this point so that it enables the
> >>> testing of the driver, and will add one patch at the last to add
> >>> qcom,sm8250-venus.
> >> Sorry fixing the typos. what I meant was,
> >> we will add qcom,sm8550-iris at this point so that it enables the
> >> testing of the driver, and will add one patch at the last to add
> >> qcom,sm8250-venus.
> >
> > I hope you meant 'to change qcom,sm8250-iris to qcom,sm8250-venus'. Also
> > please clearly document that qcom,sm8250-iris is a temporary thing just
> > to facilitate documentation and testing of the driver to be removed as a
> > last patch.
> >
> I was agreeing to follow this suggestion of yours
> "What about adding sm8550 at this point and adding sm8250
> at the end?"
> Where we will add sm8550(qcom,sm8550-iris) first so driver can be tested on
> sm8550 and add sm8250(qcom,sm8250-venus) in the last patch.
> I think Bryan also agreed to the same.

To point out, qcom,sm8250-iris can not stay.

> >>>
> >>> Thanks,
> >>> Dikshita
> >>>>
> >>>
> >
Dikshita Agarwal Sept. 5, 2024, 11:13 a.m. UTC | #9
On 9/5/2024 4:37 PM, Dmitry Baryshkov wrote:
> On Thu, 5 Sept 2024 at 13:59, Dikshita Agarwal
> <quic_dikshita@quicinc.com> wrote:
>>
>>
>>
>> On 9/5/2024 3:41 PM, Dmitry Baryshkov wrote:
>>> On Thu, Sep 05, 2024 at 11:45:25AM GMT, Dikshita Agarwal wrote:
>>>>
>>>>
>>>> On 9/5/2024 11:42 AM, Dikshita Agarwal wrote:
>>>>>
>>>>>
>>>>> On 8/29/2024 2:43 PM, Dmitry Baryshkov wrote:
>>>>>> On Tue, Aug 27, 2024 at 03:08:03PM GMT, Bryan O'Donoghue wrote:
>>>>>>> On 27/08/2024 11:05, Dikshita Agarwal via B4 Relay wrote:
>>>>>>>> +static const struct of_device_id iris_dt_match[] = {
>>>>>>>> +        { .compatible = "qcom,sm8550-iris", },
>>>>>>>> +        { .compatible = "qcom,sm8250-venus", },
>>>>>>>> +        { },
>>>>>>>> +};
>>>>>>>> +MODULE_DEVICE_TABLE(of, iris_dt_match);
>>>>>>>
>>>>>>> The enabling patch for the compat strings should come last - if its first
>>>>>>> then the time between the compat add and the last patch is a dead zone where
>>>>>>> things are bound to break on a booting board.
>>>>>>
>>>>>> But then it's impossible to test the driver in the interim state.
>>>>>> Moreover enabling it at the end only makes it hard to follow platform
>>>>>> data changes. What about adding sm8550 at this point and adding sm8250
>>>>>> at the end? Or enabling qcom,sm8550-iris and the fake qcom,sm8250-iris
>>>>>> now (and clearly documenting it as fake) and as the last patch change it
>>>>>> to qcom,sm8250-venus.
>>>>>
>>>>> Sure, we will add qcom,sm8250-iris at this point so that it enables the
>>>>> testing of the driver, and will add one patch at the last to add
>>>>> qcom,sm8250-venus.
>>>> Sorry fixing the typos. what I meant was,
>>>> we will add qcom,sm8550-iris at this point so that it enables the
>>>> testing of the driver, and will add one patch at the last to add
>>>> qcom,sm8250-venus.
>>>
>>> I hope you meant 'to change qcom,sm8250-iris to qcom,sm8250-venus'. Also
>>> please clearly document that qcom,sm8250-iris is a temporary thing just
>>> to facilitate documentation and testing of the driver to be removed as a
>>> last patch.
>>>
>> I was agreeing to follow this suggestion of yours
>> "What about adding sm8550 at this point and adding sm8250
>> at the end?"
>> Where we will add sm8550(qcom,sm8550-iris) first so driver can be tested on
>> sm8550 and add sm8250(qcom,sm8250-venus) in the last patch.
>> I think Bryan also agreed to the same.
> 
> To point out, qcom,sm8250-iris can not stay.
yes, we are not using it currently as well.
> 
>>>>>
>>>>> Thanks,
>>>>> Dikshita
>>>>>>
>>>>>
>>>
> 
> 
>
diff mbox series

Patch

diff --git a/drivers/media/platform/qcom/Kconfig b/drivers/media/platform/qcom/Kconfig
index cc5799b9ea00..4f4d3a68e6e5 100644
--- a/drivers/media/platform/qcom/Kconfig
+++ b/drivers/media/platform/qcom/Kconfig
@@ -3,4 +3,5 @@ 
 comment "Qualcomm media platform drivers"
 
 source "drivers/media/platform/qcom/camss/Kconfig"
+source "drivers/media/platform/qcom/iris/Kconfig"
 source "drivers/media/platform/qcom/venus/Kconfig"
diff --git a/drivers/media/platform/qcom/Makefile b/drivers/media/platform/qcom/Makefile
index 4f055c396e04..ea2221a202c0 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 += iris/
 obj-y += venus/
diff --git a/drivers/media/platform/qcom/iris/Kconfig b/drivers/media/platform/qcom/iris/Kconfig
new file mode 100644
index 000000000000..a9986aaa448d
--- /dev/null
+++ b/drivers/media/platform/qcom/iris/Kconfig
@@ -0,0 +1,9 @@ 
+config VIDEO_QCOM_IRIS
+        tristate "Qualcomm Iris V4L2 decoder driver"
+        depends on VIDEO_DEV
+        depends on ARCH_QCOM || COMPILE_TEST
+        help
+          This is a V4L2 driver for Qualcomm Iris video accelerator
+          hardware. It accelerates 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 000000000000..913da225486b
--- /dev/null
+++ b/drivers/media/platform/qcom/iris/Makefile
@@ -0,0 +1,3 @@ 
+iris-objs += iris_probe.o \
+
+obj-$(CONFIG_VIDEO_QCOM_IRIS) += iris.o
diff --git a/drivers/media/platform/qcom/iris/iris_core.h b/drivers/media/platform/qcom/iris/iris_core.h
new file mode 100644
index 000000000000..402f0aaef995
--- /dev/null
+++ b/drivers/media/platform/qcom/iris/iris_core.h
@@ -0,0 +1,29 @@ 
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2022-2024 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#ifndef _IRIS_CORE_H_
+#define _IRIS_CORE_H_
+
+#include <media/v4l2-device.h>
+
+/**
+ * struct iris_core - holds core parameters valid for all instances
+ *
+ * @dev: reference to device structure
+ * @reg_base: IO memory base address
+ * @irq: iris irq
+ * @v4l2_dev: a holder for v4l2 device structure
+ * @vdev_dec: iris video device structure for decoder
+ */
+
+struct iris_core {
+	struct device				*dev;
+	void __iomem				*reg_base;
+	int					irq;
+	struct v4l2_device			v4l2_dev;
+	struct video_device			*vdev_dec;
+};
+
+#endif
diff --git a/drivers/media/platform/qcom/iris/iris_probe.c b/drivers/media/platform/qcom/iris/iris_probe.c
new file mode 100644
index 000000000000..0a54fdaa1ab5
--- /dev/null
+++ b/drivers/media/platform/qcom/iris/iris_probe.c
@@ -0,0 +1,108 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2022-2024 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#include <linux/module.h>
+
+#include "iris_core.h"
+
+static int iris_register_video_device(struct iris_core *core)
+{
+	struct video_device *vdev;
+	int ret;
+
+	vdev = video_device_alloc();
+	if (!vdev)
+		return -ENOMEM;
+
+	strscpy(vdev->name, "qcom-iris-decoder", sizeof(vdev->name));
+	vdev->release = video_device_release;
+	vdev->vfl_dir = VFL_DIR_M2M;
+	vdev->v4l2_dev = &core->v4l2_dev;
+	vdev->device_caps = V4L2_CAP_VIDEO_M2M_MPLANE | V4L2_CAP_STREAMING;
+
+	ret = video_register_device(vdev, VFL_TYPE_VIDEO, -1);
+	if (ret)
+		goto err_vdev_release;
+
+	core->vdev_dec = vdev;
+	video_set_drvdata(vdev, core);
+
+	return 0;
+
+err_vdev_release:
+	video_device_release(vdev);
+
+	return ret;
+}
+
+static void iris_remove(struct platform_device *pdev)
+{
+	struct iris_core *core;
+
+	core = platform_get_drvdata(pdev);
+	if (!core)
+		return;
+
+	video_unregister_device(core->vdev_dec);
+
+	v4l2_device_unregister(&core->v4l2_dev);
+}
+
+static int iris_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct iris_core *core;
+	int ret;
+
+	core = devm_kzalloc(&pdev->dev, sizeof(*core), GFP_KERNEL);
+	if (!core)
+		return -ENOMEM;
+	core->dev = dev;
+
+	core->reg_base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(core->reg_base))
+		return PTR_ERR(core->reg_base);
+
+	core->irq = platform_get_irq(pdev, 0);
+	if (core->irq < 0)
+		return core->irq;
+
+	ret = v4l2_device_register(dev, &core->v4l2_dev);
+	if (ret)
+		return ret;
+
+	ret = iris_register_video_device(core);
+	if (ret)
+		goto err_v4l2_unreg;
+
+	platform_set_drvdata(pdev, core);
+
+	return 0;
+
+err_v4l2_unreg:
+	v4l2_device_unregister(&core->v4l2_dev);
+
+	return ret;
+}
+
+static const struct of_device_id iris_dt_match[] = {
+	{ .compatible = "qcom,sm8550-iris", },
+	{ .compatible = "qcom,sm8250-venus", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, iris_dt_match);
+
+static struct platform_driver qcom_iris_driver = {
+	.probe = iris_probe,
+	.remove_new = iris_remove,
+	.driver = {
+		.name = "qcom-iris",
+		.of_match_table = iris_dt_match,
+	},
+};
+
+module_platform_driver(qcom_iris_driver);
+MODULE_DESCRIPTION("Qualcomm Iris video driver");
+MODULE_LICENSE("GPL");