2016-11-07 17:31:41

by Stanimir Varbanov

[permalink] [raw]
Subject: [PATCH v2 0/3] Venus remoteproc driver

Hi,

here is v2 of the Venus remoteproc driver, the changes are:
- removed page table size and page table init SCM calls.

regards,
Stan

Stanimir Varbanov (3):
firmware: qcom: scm: add a video command for state setting
dt-binding: remoteproc: venus rproc dt binding document
remoteproc: qcom: add Venus video core firmware loader driver

.../devicetree/bindings/remoteproc/qcom,venus.txt | 33 ++++
drivers/firmware/qcom_scm-32.c | 18 ++
drivers/firmware/qcom_scm-64.c | 16 ++
drivers/firmware/qcom_scm.c | 16 ++
drivers/firmware/qcom_scm.h | 2 +
drivers/remoteproc/Kconfig | 12 ++
drivers/remoteproc/Makefile | 1 +
drivers/remoteproc/qcom_venus_pil.c | 213 +++++++++++++++++++++
include/linux/qcom_scm.h | 2 +
9 files changed, 313 insertions(+)
create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,venus.txt
create mode 100644 drivers/remoteproc/qcom_venus_pil.c

--
2.7.4


2016-11-07 17:31:55

by Stanimir Varbanov

[permalink] [raw]
Subject: [PATCH v2 1/3] firmware: qcom: scm: add a video command for state setting

This scm call is used to change the video core state, more
specifically it is used to suspend and resume the core.

Signed-off-by: Stanimir Varbanov <[email protected]>
---
drivers/firmware/qcom_scm-32.c | 18 ++++++++++++++++++
drivers/firmware/qcom_scm-64.c | 16 ++++++++++++++++
drivers/firmware/qcom_scm.c | 16 ++++++++++++++++
drivers/firmware/qcom_scm.h | 2 ++
include/linux/qcom_scm.h | 2 ++
5 files changed, 54 insertions(+)

diff --git a/drivers/firmware/qcom_scm-32.c b/drivers/firmware/qcom_scm-32.c
index c6aeedbdcbb0..82c1d8d0d36b 100644
--- a/drivers/firmware/qcom_scm-32.c
+++ b/drivers/firmware/qcom_scm-32.c
@@ -560,3 +560,21 @@ int __qcom_scm_pas_mss_reset(struct device *dev, bool reset)

return ret ? : le32_to_cpu(out);
}
+
+int __qcom_scm_video_set_state(struct device *dev, u32 state, u32 spare)
+{
+ struct {
+ __le32 state;
+ __le32 spare;
+ } req;
+ __le32 scm_ret = 0;
+ int ret;
+
+ req.state = cpu_to_le32(state);
+ req.spare = cpu_to_le32(spare);
+
+ ret = qcom_scm_call(dev, QCOM_SCM_SVC_BOOT, QCOM_SCM_VIDEO_SET_STATE,
+ &req, sizeof(req), &scm_ret, sizeof(scm_ret));
+
+ return ret ? : le32_to_cpu(scm_ret);
+}
diff --git a/drivers/firmware/qcom_scm-64.c b/drivers/firmware/qcom_scm-64.c
index 4a0f5ead4fb5..68484ea2aa51 100644
--- a/drivers/firmware/qcom_scm-64.c
+++ b/drivers/firmware/qcom_scm-64.c
@@ -358,3 +358,19 @@ int __qcom_scm_pas_mss_reset(struct device *dev, bool reset)

return ret ? : res.a1;
}
+
+int __qcom_scm_video_set_state(struct device *dev, u32 state, u32 spare)
+{
+ struct qcom_scm_desc desc = {0};
+ struct arm_smccc_res res;
+ int ret;
+
+ desc.args[0] = state;
+ desc.args[1] = spare;
+ desc.arginfo = QCOM_SCM_ARGS(2);
+
+ ret = qcom_scm_call(dev, QCOM_SCM_SVC_BOOT, QCOM_SCM_VIDEO_SET_STATE,
+ &desc, &res);
+
+ return ret ? : res.a1;
+}
diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index d95c70227c05..7e364691a87c 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -320,6 +320,22 @@ bool qcom_scm_is_available(void)
}
EXPORT_SYMBOL(qcom_scm_is_available);

+int qcom_scm_video_set_state(u32 state, u32 spare)
+{
+ int ret;
+
+ ret = qcom_scm_clk_enable();
+ if (ret)
+ return ret;
+
+ ret = __qcom_scm_video_set_state(__scm->dev, state, spare);
+
+ qcom_scm_clk_disable();
+
+ return ret;
+}
+EXPORT_SYMBOL(qcom_scm_video_set_state);
+
static int qcom_scm_probe(struct platform_device *pdev)
{
struct qcom_scm *scm;
diff --git a/drivers/firmware/qcom_scm.h b/drivers/firmware/qcom_scm.h
index 3584b00fe7e6..4830559b2639 100644
--- a/drivers/firmware/qcom_scm.h
+++ b/drivers/firmware/qcom_scm.h
@@ -15,6 +15,8 @@
#define QCOM_SCM_SVC_BOOT 0x1
#define QCOM_SCM_BOOT_ADDR 0x1
#define QCOM_SCM_BOOT_ADDR_MC 0x11
+#define QCOM_SCM_VIDEO_SET_STATE 0xa
+extern int __qcom_scm_video_set_state(struct device *dev, u32 state, u32 spare);

#define QCOM_SCM_FLAG_HLOS 0x01
#define QCOM_SCM_FLAG_COLDBOOT_MC 0x02
diff --git a/include/linux/qcom_scm.h b/include/linux/qcom_scm.h
index cc32ab852fbc..2ece81a6b078 100644
--- a/include/linux/qcom_scm.h
+++ b/include/linux/qcom_scm.h
@@ -46,4 +46,6 @@ extern void qcom_scm_cpu_power_down(u32 flags);

extern u32 qcom_scm_get_version(void);

+extern int qcom_scm_video_set_state(u32 state, u32 spare);
+
#endif
--
2.7.4

2016-11-07 17:38:23

by Stanimir Varbanov

[permalink] [raw]
Subject: [PATCH v2 3/3] remoteproc: qcom: add Venus video core firmware loader driver

This driver will load and authenticate the Venus firmware and
bringing it core out of reset. Those two functionalities are
via secure monitor calls to trusted environment.

Signed-off-by: Stanimir Varbanov <[email protected]>
---
drivers/remoteproc/Kconfig | 12 ++
drivers/remoteproc/Makefile | 1 +
drivers/remoteproc/qcom_venus_pil.c | 213 ++++++++++++++++++++++++++++++++++++
3 files changed, 226 insertions(+)
create mode 100644 drivers/remoteproc/qcom_venus_pil.c

diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index f396bfef5d42..0e90a2491873 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -107,6 +107,18 @@ config QCOM_WCNSS_PIL
Say y here to support the Peripheral Image Loader for the Qualcomm
Wireless Connectivity Subsystem.

+config QCOM_VENUS_PIL
+ tristate "Qualcomm Venus Peripheral Image Loader"
+ depends on OF && ARCH_QCOM
+ depends on QCOM_SCM
+ select QCOM_MDT_LOADER
+ select REMOTEPROC
+ help
+ Say y here to support Qualcomm Peripherial Image Loader for the
+ Venus remote processor. The Venus remote processor is a
+ micro-controller plus dedicated hardware for video acceleration
+ of video decoding and encoding operations.
+
config ST_REMOTEPROC
tristate "ST remoteproc support"
depends on ARCH_STI
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index 6dfb62ed643f..852711b89844 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -15,4 +15,5 @@ obj-$(CONFIG_QCOM_MDT_LOADER) += qcom_mdt_loader.o
obj-$(CONFIG_QCOM_Q6V5_PIL) += qcom_q6v5_pil.o
obj-$(CONFIG_QCOM_WCNSS_IRIS) += qcom_wcnss_iris.o
obj-$(CONFIG_QCOM_WCNSS_PIL) += qcom_wcnss.o
+obj-$(CONFIG_QCOM_VENUS_PIL) += qcom_venus_pil.o
obj-$(CONFIG_ST_REMOTEPROC) += st_remoteproc.o
diff --git a/drivers/remoteproc/qcom_venus_pil.c b/drivers/remoteproc/qcom_venus_pil.c
new file mode 100644
index 000000000000..6d4e55bffef5
--- /dev/null
+++ b/drivers/remoteproc/qcom_venus_pil.c
@@ -0,0 +1,213 @@
+/*
+ * Qualcomm Venus Peripheral Image Loader
+ *
+ * Copyright (C) 2016 Linaro Ltd
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/dma-mapping.h>
+#include <linux/firmware.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_reserved_mem.h>
+#include <linux/platform_device.h>
+#include <linux/qcom_scm.h>
+#include <linux/remoteproc.h>
+
+#include "qcom_mdt_loader.h"
+#include "remoteproc_internal.h"
+
+#define VENUS_CRASH_REASON_SMEM 425
+#define VENUS_FIRMWARE_NAME "venus.mdt"
+#define VENUS_PAS_ID 9
+#define VENUS_FW_MEM_SIZE SZ_8M
+
+struct qcom_venus {
+ struct device *dev;
+ struct rproc *rproc;
+ phys_addr_t fw_addr;
+ phys_addr_t mem_phys;
+ void *mem_va;
+ size_t mem_size;
+};
+
+static int venus_load(struct rproc *rproc, const struct firmware *fw)
+{
+ struct qcom_venus *venus = rproc->priv;
+ phys_addr_t pa;
+ size_t fw_size;
+ bool relocate;
+ int ret;
+
+ ret = qcom_scm_pas_init_image(VENUS_PAS_ID, fw->data, fw->size);
+ if (ret) {
+ dev_err(&rproc->dev, "invalid firmware metadata (%d)\n", ret);
+ return -EINVAL;
+ }
+
+ ret = qcom_mdt_parse(fw, &venus->fw_addr, &fw_size, &relocate);
+ if (ret) {
+ dev_err(&rproc->dev, "failed to parse mdt header (%d)\n", ret);
+ return ret;
+ }
+
+ if (fw_size > venus->mem_size)
+ return -ENOMEM;
+
+ pa = relocate ? venus->mem_phys : venus->fw_addr;
+
+ ret = qcom_scm_pas_mem_setup(VENUS_PAS_ID, pa, fw_size);
+ if (ret) {
+ dev_err(&rproc->dev, "unable to setup memory (%d)\n", ret);
+ return -EINVAL;
+ }
+
+ return qcom_mdt_load(rproc, fw, VENUS_FIRMWARE_NAME);
+}
+
+static const struct rproc_fw_ops venus_fw_ops = {
+ .find_rsc_table = qcom_mdt_find_rsc_table,
+ .load = venus_load,
+};
+
+static int venus_start(struct rproc *rproc)
+{
+ struct qcom_venus *venus = rproc->priv;
+ int ret;
+
+ ret = qcom_scm_pas_auth_and_reset(VENUS_PAS_ID);
+ if (ret)
+ dev_err(venus->dev,
+ "authentication image and release reset failed (%d)\n",
+ ret);
+
+ return ret;
+}
+
+static int venus_stop(struct rproc *rproc)
+{
+ struct qcom_venus *venus = rproc->priv;
+ int ret;
+
+ ret = qcom_scm_pas_shutdown(VENUS_PAS_ID);
+ if (ret)
+ dev_err(venus->dev, "failed to shutdown: %d\n", ret);
+
+ return ret;
+}
+
+static void *venus_da_to_va(struct rproc *rproc, u64 da, int len)
+{
+ struct qcom_venus *venus = rproc->priv;
+ s64 offset;
+
+ offset = da - venus->fw_addr;
+
+ if (offset < 0 || offset + len > venus->mem_size)
+ return NULL;
+
+ return venus->mem_va + offset;
+}
+
+static const struct rproc_ops venus_ops = {
+ .start = venus_start,
+ .stop = venus_stop,
+ .da_to_va = venus_da_to_va,
+};
+
+static int venus_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct qcom_venus *venus;
+ struct rproc *rproc;
+ int ret;
+
+ if (!qcom_scm_is_available())
+ return -EPROBE_DEFER;
+
+ if (!qcom_scm_pas_supported(VENUS_PAS_ID)) {
+ dev_err(dev, "PAS is not available for venus\n");
+ return -ENXIO;
+ }
+
+ ret = of_reserved_mem_device_init(dev);
+ if (ret)
+ return ret;
+
+ rproc = rproc_alloc(dev, pdev->name, &venus_ops, VENUS_FIRMWARE_NAME,
+ sizeof(*venus));
+ if (!rproc) {
+ dev_err(dev, "unable to allocate remoteproc\n");
+ ret = -ENOMEM;
+ goto release_mem;
+ }
+
+ rproc->fw_ops = &venus_fw_ops;
+ venus = rproc->priv;
+ venus->dev = dev;
+ venus->rproc = rproc;
+ venus->mem_size = VENUS_FW_MEM_SIZE;
+
+ platform_set_drvdata(pdev, venus);
+
+ venus->mem_va = dma_alloc_coherent(dev, venus->mem_size,
+ &venus->mem_phys, GFP_KERNEL);
+ if (!venus->mem_va) {
+ ret = -ENOMEM;
+ goto free_rproc;
+ }
+
+ ret = rproc_add(rproc);
+ if (ret)
+ goto free_mem;
+
+ return 0;
+
+free_mem:
+ dma_free_coherent(dev, venus->mem_size, venus->mem_va, venus->mem_phys);
+free_rproc:
+ rproc_put(rproc);
+release_mem:
+ of_reserved_mem_device_release(dev);
+
+ return ret;
+}
+
+static int venus_remove(struct platform_device *pdev)
+{
+ struct qcom_venus *venus = platform_get_drvdata(pdev);
+ struct device *dev = venus->dev;
+
+ rproc_del(venus->rproc);
+ rproc_put(venus->rproc);
+ dma_free_coherent(dev, venus->mem_size, venus->mem_va, venus->mem_phys);
+ of_reserved_mem_device_release(dev);
+
+ return 0;
+}
+
+static const struct of_device_id venus_of_match[] = {
+ { .compatible = "qcom,venus-pil" },
+ { },
+};
+
+static struct platform_driver venus_driver = {
+ .probe = venus_probe,
+ .remove = venus_remove,
+ .driver = {
+ .name = "qcom-venus-pil",
+ .of_match_table = venus_of_match,
+ },
+};
+
+module_platform_driver(venus_driver);
+MODULE_DESCRIPTION("Peripheral Image Loader for Venus");
+MODULE_LICENSE("GPL v2");
--
2.7.4

2016-11-07 17:39:29

by Stanimir Varbanov

[permalink] [raw]
Subject: [PATCH v2 2/3] dt-binding: remoteproc: venus rproc dt binding document

Add devicetree binding document for Venus remote processor.

Signed-off-by: Stanimir Varbanov <[email protected]>
---
.../devicetree/bindings/remoteproc/qcom,venus.txt | 33 ++++++++++++++++++++++
1 file changed, 33 insertions(+)
create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,venus.txt

diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,venus.txt b/Documentation/devicetree/bindings/remoteproc/qcom,venus.txt
new file mode 100644
index 000000000000..06a2db60fa38
--- /dev/null
+++ b/Documentation/devicetree/bindings/remoteproc/qcom,venus.txt
@@ -0,0 +1,33 @@
+Qualcomm Venus Peripheral Image Loader
+
+This document defines the binding for a component that loads and boots firmware
+on the Qualcomm Venus remote processor core.
+
+- compatible:
+ Usage: required
+ Value type: <string>
+ Definition: must contain "qcom,venus-pil"
+
+- memory-region:
+ Usage: required
+ Value type: <phandle>
+ Definition: a phandle to a node describing reserved memory
+
+* An example
+ reserved-memory {
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges;
+
+ venus_mem: venus@89900000 {
+ compatible = "shared-dma-pool";
+ reg = <0x0 0x89900000 0x0 0x800000>;
+ alignment = <0x1000>;
+ no-map;
+ };
+ };
+
+ rproc_venus@0 {
+ compatible = "qcom,venus-pil";
+ memory-region = <&venus_mem>;
+ };
--
2.7.4

2016-11-10 01:52:56

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] firmware: qcom: scm: add a video command for state setting

On 11/07, Stanimir Varbanov wrote:
> diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
> index d95c70227c05..7e364691a87c 100644
> --- a/drivers/firmware/qcom_scm.c
> +++ b/drivers/firmware/qcom_scm.c
> @@ -320,6 +320,22 @@ bool qcom_scm_is_available(void)
> }
> EXPORT_SYMBOL(qcom_scm_is_available);
>
> +int qcom_scm_video_set_state(u32 state, u32 spare)
> +{
> + int ret;
> +
> + ret = qcom_scm_clk_enable();

Do we need clk control for this? Usually it's only required for
crypto engine things, and turning on video doesn't sound like it
uses crypto. I don't think downstream android kernel does this.

> + if (ret)
> + return ret;
> +
> + ret = __qcom_scm_video_set_state(__scm->dev, state, spare);
> +
> + qcom_scm_clk_disable();
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(qcom_scm_video_set_state);
> +
> static int qcom_scm_probe(struct platform_device *pdev)
> {
> struct qcom_scm *scm;

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

2016-11-10 01:54:07

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] dt-binding: remoteproc: venus rproc dt binding document

On 11/07, Stanimir Varbanov wrote:
> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,venus.txt b/Documentation/devicetree/bindings/remoteproc/qcom,venus.txt
> new file mode 100644
> index 000000000000..06a2db60fa38
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,venus.txt
> @@ -0,0 +1,33 @@
> +Qualcomm Venus Peripheral Image Loader
> +
> +This document defines the binding for a component that loads and boots firmware
> +on the Qualcomm Venus remote processor core.
> +
> +- compatible:
> + Usage: required
> + Value type: <string>
> + Definition: must contain "qcom,venus-pil"
> +
> +- memory-region:
> + Usage: required
> + Value type: <phandle>
> + Definition: a phandle to a node describing reserved memory
> +
> +* An example
> + reserved-memory {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> +
> + venus_mem: venus@89900000 {
> + compatible = "shared-dma-pool";
> + reg = <0x0 0x89900000 0x0 0x800000>;
> + alignment = <0x1000>;
> + no-map;
> + };
> + };
> +
> + rproc_venus@0 {

Not sure we need @0 here in the example if it doesn't have a reg
property.

> + compatible = "qcom,venus-pil";
> + memory-region = <&venus_mem>;
> + };
> --
> 2.7.4
>

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

2016-11-10 08:05:43

by Stanimir Varbanov

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] firmware: qcom: scm: add a video command for state setting

Hi,

On 11/10/2016 03:52 AM, Stephen Boyd wrote:
> On 11/07, Stanimir Varbanov wrote:
>> diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
>> index d95c70227c05..7e364691a87c 100644
>> --- a/drivers/firmware/qcom_scm.c
>> +++ b/drivers/firmware/qcom_scm.c
>> @@ -320,6 +320,22 @@ bool qcom_scm_is_available(void)
>> }
>> EXPORT_SYMBOL(qcom_scm_is_available);
>>
>> +int qcom_scm_video_set_state(u32 state, u32 spare)
>> +{
>> + int ret;
>> +
>> + ret = qcom_scm_clk_enable();
>
> Do we need clk control for this? Usually it's only required for
> crypto engine things, and turning on video doesn't sound like it
> uses crypto. I don't think downstream android kernel does this.

Correct, the crypto clk is not needed.

--
regards,
Stan

2016-11-14 16:58:42

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] dt-binding: remoteproc: venus rproc dt binding document

On Mon, Nov 07, 2016 at 07:30:52PM +0200, Stanimir Varbanov wrote:
> Add devicetree binding document for Venus remote processor.
>
> Signed-off-by: Stanimir Varbanov <[email protected]>
> ---
> .../devicetree/bindings/remoteproc/qcom,venus.txt | 33 ++++++++++++++++++++++
> 1 file changed, 33 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,venus.txt
>
> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,venus.txt b/Documentation/devicetree/bindings/remoteproc/qcom,venus.txt
> new file mode 100644
> index 000000000000..06a2db60fa38
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,venus.txt
> @@ -0,0 +1,33 @@
> +Qualcomm Venus Peripheral Image Loader
> +
> +This document defines the binding for a component that loads and boots firmware
> +on the Qualcomm Venus remote processor core.
> +
> +- compatible:
> + Usage: required
> + Value type: <string>
> + Definition: must contain "qcom,venus-pil"
> +
> +- memory-region:
> + Usage: required
> + Value type: <phandle>
> + Definition: a phandle to a node describing reserved memory
> +
> +* An example
> + reserved-memory {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> +
> + venus_mem: venus@89900000 {
> + compatible = "shared-dma-pool";
> + reg = <0x0 0x89900000 0x0 0x800000>;
> + alignment = <0x1000>;
> + no-map;
> + };
> + };
> +
> + rproc_venus@0 {

s/_/-/

With that,

Acked-by: Rob Herring <[email protected]>

2016-11-14 19:16:47

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] remoteproc: qcom: add Venus video core firmware loader driver

On 11/07, Stanimir Varbanov wrote:
> +#include <linux/module.h>
> +#include <linux/of_reserved_mem.h>
> +#include <linux/platform_device.h>
> +#include <linux/qcom_scm.h>
> +#include <linux/remoteproc.h>
> +
> +#include "qcom_mdt_loader.h"
> +#include "remoteproc_internal.h"
> +
> +#define VENUS_CRASH_REASON_SMEM 425

This is unused. Is there going to be some common smem API to get
the crash reason?
> +
> +static const struct of_device_id venus_of_match[] = {
> + { .compatible = "qcom,venus-pil" },
> + { },
> +};

Add a MODULE_DEVICE_TABLE?

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

2016-11-17 09:09:07

by Stanimir Varbanov

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] remoteproc: qcom: add Venus video core firmware loader driver

Hi,

On 11/14/2016 09:16 PM, Stephen Boyd wrote:
> On 11/07, Stanimir Varbanov wrote:
>> +#include <linux/module.h>
>> +#include <linux/of_reserved_mem.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/qcom_scm.h>
>> +#include <linux/remoteproc.h>
>> +
>> +#include "qcom_mdt_loader.h"
>> +#include "remoteproc_internal.h"
>> +
>> +#define VENUS_CRASH_REASON_SMEM 425
>
> This is unused. Is there going to be some common smem API to get
> the crash reason?

This is leftover and never used, so I will delete it. About smem maybe
Bjorn have some idea?

>> +
>> +static const struct of_device_id venus_of_match[] = {
>> + { .compatible = "qcom,venus-pil" },
>> + { },
>> +};
>
> Add a MODULE_DEVICE_TABLE?
>

OK.

--
regards,
Stan

2016-11-18 17:23:31

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] remoteproc: qcom: add Venus video core firmware loader driver

On Mon 14 Nov 11:16 PST 2016, Stephen Boyd wrote:

> On 11/07, Stanimir Varbanov wrote:
> > +#include <linux/module.h>
> > +#include <linux/of_reserved_mem.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/qcom_scm.h>
> > +#include <linux/remoteproc.h>
> > +
> > +#include "qcom_mdt_loader.h"
> > +#include "remoteproc_internal.h"
> > +
> > +#define VENUS_CRASH_REASON_SMEM 425
>
> This is unused. Is there going to be some common smem API to get
> the crash reason?

There are a couple of these operations that are good candidates to be
shared between all Qualcomm remoteproc drivers. I just haven't spent the
time to figure out how to properly splice it.

For the crash reason specifically, there are three operations done in
the other drivers; read-and-null-terminate, then this is outputed to the
log, also indicating what type of event we had. And finally the
remoteproc core is informed about the crash, with a event-type passed
and we get another line in the log based on that, i.e. there's room for
a few improvements in this area.

Regards,
Bjorn