[v3,03/29] media: iris: add platform driver for iris video device
Commit Message
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
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
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.
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.
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
>
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
>>
>
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
> >>
> >
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
>>>>
>>>
>
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
> >>>>
> >>>
> >
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
>>>>>>
>>>>>
>>>
>
>
>
@@ -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"
@@ -1,3 +1,4 @@
# SPDX-License-Identifier: GPL-2.0-only
obj-y += camss/
+obj-y += iris/
obj-y += venus/
new file mode 100644
@@ -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.
new file mode 100644
@@ -0,0 +1,3 @@
+iris-objs += iris_probe.o \
+
+obj-$(CONFIG_VIDEO_QCOM_IRIS) += iris.o
new file mode 100644
@@ -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
new file mode 100644
@@ -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");