[PATCH 1/2] ARM: DT: msm: Add Qualcomm's PRNG driver binding document
This adds Qualcomm PRNG driver device tree binding documentation
to use as an example in dts trees.
Signed-off-by: Stanimir Varbanov
---
Documentation/devicetree/bindings/rng/qcom,prng.txt | 17 +
1 file changed, 17 insertions(+)
create mode 100644 Documentation/devicetree/bindings/rng/qcom,prng.txt
diff --git a/Documentation/devicetree/bindings/rng/qcom,prng.txt
b/Documentation/devicetree/bindings/rng/qcom,prng.txt
new file mode 100644
index ..92be00085ab1
--- /dev/null
+++ b/Documentation/devicetree/bindings/rng/qcom,prng.txt
@@ -0,0 +1,17 @@
+Qualcomm MSM pseudo random number generator.
+
+Required properties:
+
+- compatible : should be "qcom,prng"
+- reg : specifies base physical address and size of the registers map
+- clocks : phandle to clock-controller plus clock-specifier pair
+- clock-names : "prng" clocks all registers, FIFO and circuits in PRNG IP block
+
+Example:
+
+ rng {
+ compatible = "qcom,prng";
+ reg = <0xf9bff000 0x200>;
+ clocks = <&clock GCC_PRNG_AHB_CLK>;
+ clock-names = "prng";
+ };
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/2] Add support for Qualcomm's PRNG
This patch set adds hardware RNG driver wich is used to control the Qualcomm's PRNG hardware block. The first patch document the DT bindings needed to sucessfuly probe the driver and the second patch adds the driver. Comments are welecome! Stanimir Varbanov (2): ARM: DT: msm: Add Qualcomm's PRNG driver binding document hwrng: msm: Add PRNG support for MSM SoC's .../devicetree/bindings/rng/qcom,prng.txt | 17 ++ drivers/char/hw_random/Kconfig | 12 ++ drivers/char/hw_random/Makefile| 1 + drivers/char/hw_random/msm-rng.c | 211 + 4 files changed, 241 insertions(+) create mode 100644 Documentation/devicetree/bindings/rng/qcom,prng.txt create mode 100644 drivers/char/hw_random/msm-rng.c -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] hwrng: msm: Add PRNG support for MSM SoC's
This adds a driver for hardware random number generator present
on Qualcomm MSM SoC's.
Signed-off-by: Stanimir Varbanov
---
drivers/char/hw_random/Kconfig | 12 +++
drivers/char/hw_random/Makefile | 1 +
drivers/char/hw_random/msm-rng.c | 211 +++
3 files changed, 224 insertions(+)
create mode 100644 drivers/char/hw_random/msm-rng.c
diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
index 0aa9d91daef5..d902330cef43 100644
--- a/drivers/char/hw_random/Kconfig
+++ b/drivers/char/hw_random/Kconfig
@@ -314,3 +314,15 @@ config HW_RANDOM_TPM
module will be called tpm-rng.
If unsure, say Y.
+
+config HW_RANDOM_MSM
+tristate "Qualcomm MSM Random Number Generator support"
+depends on HW_RANDOM && ARCH_MSM && HAVE_CLK
+---help---
+ This driver provides kernel-side support for the Random Number
+ Generator hardware found on Qualcomm MSM SoCs.
+
+ To compile this driver as a module, choose M here. the
+ module will be called msm-rng.
+
+ If unsure, say Y.
diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile
index bed467c9300e..441a0a7705a2 100644
--- a/drivers/char/hw_random/Makefile
+++ b/drivers/char/hw_random/Makefile
@@ -27,3 +27,4 @@ obj-$(CONFIG_HW_RANDOM_PSERIES) += pseries-rng.o
obj-$(CONFIG_HW_RANDOM_EXYNOS) += exynos-rng.o
obj-$(CONFIG_HW_RANDOM_TPM) += tpm-rng.o
obj-$(CONFIG_HW_RANDOM_BCM2835) += bcm2835-rng.o
+obj-$(CONFIG_HW_RANDOM_MSM) += msm-rng.o
diff --git a/drivers/char/hw_random/msm-rng.c b/drivers/char/hw_random/msm-rng.c
new file mode 100644
index ..c4545d2da949
--- /dev/null
+++ b/drivers/char/hw_random/msm-rng.c
@@ -0,0 +1,211 @@
+/*
+ * Copyright (c) 2011-2013, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only 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
+#include
+#include
+#include
+#include
+#include
+
+/* Device specific register offsets */
+#define PRNG_DATA_OUT 0x
+#define PRNG_STATUS0x0004
+#define PRNG_LFSR_CFG 0x0100
+#define PRNG_CONFIG0x0104
+
+/* Device specific register masks and config values */
+#define PRNG_LFSR_CFG_MASK 0x
+#define PRNG_LFSR_CFG_CLOCKS 0x
+#define PRNG_CONFIG_MASK 0x0002
+#define PRNG_CONFIG_HW_ENABLE BIT(1)
+#define PRNG_STATUS_DATA_AVAIL BIT(0)
+
+#define MAX_HW_FIFO_DEPTH 16
+#define MAX_HW_FIFO_SIZE (MAX_HW_FIFO_DEPTH * 4)
+#define WORD_SZ4
+
+struct msm_rng {
+ void __iomem *base;
+ struct clk *clk;
+};
+
+static int msm_rng_enable(struct msm_rng *rng, int enable)
+{
+ u32 val;
+ int ret;
+
+ ret = clk_prepare_enable(rng->clk);
+ if (ret)
+ return ret;
+
+ if (enable) {
+ /* Enable PRNG only if it is not already enabled */
+ val = readl_relaxed(rng->base + PRNG_CONFIG);
+ if (val & PRNG_CONFIG_HW_ENABLE)
+ goto already_enabled;
+
+ /* PRNG is not enabled */
+ val = readl_relaxed(rng->base + PRNG_LFSR_CFG);
+ val &= ~PRNG_LFSR_CFG_MASK;
+ val |= PRNG_LFSR_CFG_CLOCKS;
+ writel(val, rng->base + PRNG_LFSR_CFG);
+
+ val = readl_relaxed(rng->base + PRNG_CONFIG);
+ val &= ~PRNG_CONFIG_MASK;
+ val |= PRNG_CONFIG_HW_ENABLE;
+ writel(val, rng->base + PRNG_CONFIG);
+ } else {
+ val = readl_relaxed(rng->base + PRNG_CONFIG);
+ val &= ~PRNG_CONFIG_MASK;
+ writel(val, rng->base + PRNG_CONFIG);
+ }
+
+already_enabled:
+ clk_disable_unprepare(rng->clk);
+ return 0;
+}
+
+static int msm_rng_read(struct hwrng *hwrng, void *data, size_t max, bool wait)
+{
+ struct msm_rng *rng = (struct msm_rng *)hwrng->priv;
+ size_t currsize = 0;
+ u32 *retdata = data;
+ size_t maxsize;
+ int ret;
+ u32 val;
+
+ /* calculate max size bytes to transfer back to caller */
+ maxsize = min_t(size_t, MAX_HW_FIFO_SIZE, max);
+
+ /* no room for word data */
+ if (maxsize < WORD_SZ)
+ return 0;
+
+ ret = clk_prepare_enable(rng->clk);
+ if (ret)
+ return ret;
+
+ /* read random data from hardware */
+ do {
+ val = readl_r
Re: [PATCH 0/2] Add support for Qualcomm's PRNG
Hi Ted, On 10/03/2013 07:51 PM, Theodore Ts'o wrote: > On Thu, Oct 03, 2013 at 05:52:33PM +0300, Stanimir Varbanov wrote: >> This patch set adds hardware RNG driver wich is used to control the >> Qualcomm's PRNG hardware block. >> The first patch document the DT bindings needed to sucessfuly probe >> the driver and the second patch adds the driver. > > Is this really a PRNG (pseudo-random number generator)? What are the > guarantees which Qualcomm is providing for the PRNG? If it's really a > PRNG such as AES(i++, NSA_KEY), then kudo to Qualcomm for being > honest. > > If it is supposed to be (or at least claimed to be) a secure random > number generator ala RDRAND suitable for use in cryptographic > applications, calling it a PRNG is going to be a bit misleading. I guess that it should follow NIST 800-90 recommendation, but I'm not aware what DRBG mechanism is used. To be honest I really don't know the hardware implementation details. I put PRNG abbreviation in the cover letter just because I saw that defines for register offsets are prefixed with PRNG_*. I could rename all occurrences of PRNG to RNG. Is that will be enough to avoid confusions? regadrs, Stan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] hwrng: msm: Add PRNG support for MSM SoC's
Hi Stephen,
Thanks for the quick review!
On 10/03/2013 10:25 PM, Stephen Boyd wrote:
> On 10/03/13 07:52, Stanimir Varbanov wrote:
>> +#define PRNG_CONFIG_MASK0x0002
>> +#define PRNG_CONFIG_HW_ENABLE BIT(1)
>
> These two are the same so please drop the PRNG_CONFIG_MASK define and
> just use the PRNG_CONFIG_HW_ENABLE define.
>
OK I will drop the mask and rework the code related to it.
>> +#define PRNG_STATUS_DATA_AVAIL BIT(0)
>> +
>> +#define MAX_HW_FIFO_DEPTH 16
>> +#define MAX_HW_FIFO_SIZE(MAX_HW_FIFO_DEPTH * 4)
>> +#define WORD_SZ 4
>> +
>> +struct msm_rng {
>> +void __iomem *base;
>> +struct clk *clk;
>> +};
>> +
>> +static int msm_rng_enable(struct msm_rng *rng, int enable)
>> +{
>> +u32 val;
>> +int ret;
>> +
>> +ret = clk_prepare_enable(rng->clk);
>> +if (ret)
>> +return ret;
>> +
>> +if (enable) {
>> +/* Enable PRNG only if it is not already enabled */
>> +val = readl_relaxed(rng->base + PRNG_CONFIG);
>> +if (val & PRNG_CONFIG_HW_ENABLE)
>> +goto already_enabled;
>> +
>> +/* PRNG is not enabled */
>> +val = readl_relaxed(rng->base + PRNG_LFSR_CFG);
>> +val &= ~PRNG_LFSR_CFG_MASK;
>> +val |= PRNG_LFSR_CFG_CLOCKS;
>> +writel(val, rng->base + PRNG_LFSR_CFG);
>> +
>> +val = readl_relaxed(rng->base + PRNG_CONFIG);
>> +val &= ~PRNG_CONFIG_MASK;
>> +val |= PRNG_CONFIG_HW_ENABLE;
>> +writel(val, rng->base + PRNG_CONFIG);
>
> This could just be
>
> val = readl_relaxed(rng->base + PRNG_CONFIG);
> val |= PRNG_CONFIG_HW_ENABLE;
> writel(val, rng->base + PRNG_CONFIG);
>
>
>> +} else {
>> +val = readl_relaxed(rng->base + PRNG_CONFIG);
>> +val &= ~PRNG_CONFIG_MASK;
>> +writel(val, rng->base + PRNG_CONFIG);
>> +}
>> +
>> +already_enabled:
>> +clk_disable_unprepare(rng->clk);
>> +return 0;
>> +}
>> +
> [...]
>> +static int msm_rng_probe(struct platform_device *pdev)
>> +{
>> +struct msm_rng *rng;
>> +struct device_node *np;
>> +struct resource res;
>> +int ret;
>> +
>> +np = of_node_get(pdev->dev.of_node);
>> +if (!np)
>> +return -ENODEV;
>
> This is unnecessary.
I used this call because CONFIG_OF_DYNAMIC could be enabled at some
time. Isn't that possible? I saw that of_node_get|put is used in .probe
on few places in drivers.
>
>> +
>> +rng = devm_kzalloc(&pdev->dev, sizeof(*rng), GFP_KERNEL);
>> +if (!rng) {
>> +ret = -ENOMEM;
>> +goto err_exit;
>> +}
>> +
>> +ret = of_address_to_resource(np, 0, &res);
>> +if (ret)
>> +goto err_exit;
>
> We should just do
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> rng->base = devm_ioremap_resource(&pdev->dev, res);
> if (IS_ERR(rng->base))
> return PTR_ERR(rng->base);
>
>> +
>> +rng->base = devm_ioremap_resource(&pdev->dev, &res);
>> +if (IS_ERR(rng->base)) {
>> +ret = PTR_ERR(rng->base);
>> +goto err_exit;
>> +}
>> +
>> +rng->clk = devm_clk_get(&pdev->dev, "prng");
>> +if (IS_ERR(rng->clk)) {
>> +ret = PTR_ERR(rng->clk);
>> +goto err_exit;
>> +}
>> +
>> +msm_rng_ops.priv = (unsigned long) rng;
>> +ret = hwrng_register(&msm_rng_ops);
>> +if (ret)
>> +dev_err(&pdev->dev, "failed to register hwrng\n");
>> +
>> +err_exit:
>
> Doing all that should make this goto exit path unnecessary.
>
>> +of_node_put(np);
>> +return ret;
>> +}
>> +
>> +static int msm_rng_remove(struct platform_device *pdev)
>> +{
>> +hwrng_unregister(&msm_rng_ops);
>> +return 0;
>> +}
>> +
>> +static struct of_device_id msm_rng_of_match[] = {
>
> const?
>
>> +{ .compatible = "qcom,prng", },
>> +{}
>> +};
>> +MODULE_DEVICE_TABLE(of, msm_rng_of_match);
>> +
>> +static struct platform_driver msm_rng_driver = {
>> +.probe = msm_rng_probe,
>> +.remove = msm_rng_remove,
>> +.driver = {
>> +.name = KBUILD_MODNAME,
>> +.owner = THIS_MODULE,
>> +.of_match_table = of_match_ptr(msm_rng_of_match),
>> +}
>> +};
>> +module_platform_driver(msm_rng_driver);
>> +
>> +MODULE_AUTHOR("The Linux Foundation");
>> +MODULE_DESCRIPTION("Qualcomm MSM random number generator driver");
>> +MODULE_LICENSE("GPL v2");
>
>
regards,
Stan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 2/3] usb: phy: Add Qualcomm SS-USB and HS-USB drivers for DW PHY's
Hi Ivan,
Few comments below.
On 10/07/2013 10:44 AM, Ivan T. Ivanov wrote:
> From: "Ivan T. Ivanov"
>
> These drivers handles control and configuration of the HS
> and SS USB PHY transceivers. They are part of the driver
> which manage Synopsys DesignWare USB3 controller stack
> inside Qualcomm SoC's.
>
> Signed-off-by: Ivan T. Ivanov
> ---
> drivers/usb/phy/Kconfig | 11 ++
> drivers/usb/phy/Makefile|2 +
> drivers/usb/phy/phy-msm-dw-hs.c | 329 ++
> drivers/usb/phy/phy-msm-dw-ss.c | 375
> +++
> 4 files changed, 717 insertions(+)
> create mode 100644 drivers/usb/phy/phy-msm-dw-hs.c
> create mode 100644 drivers/usb/phy/phy-msm-dw-ss.c
>
> diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
> index d5589f9..bbb2d0e 100644
> --- a/drivers/usb/phy/Kconfig
> +++ b/drivers/usb/phy/Kconfig
> @@ -214,6 +214,17 @@ config USB_RCAR_PHY
> To compile this driver as a module, choose M here: the
> module will be called phy-rcar-usb.
>
> +config USB_MSM_DW_PHYS
> + tristate "Qualcomm USB controller DW PHY's wrappers support"
> + depends on (USB || USB_GADGET) && ARCH_MSM
> + select USB_PHY
> + help
> + Enable this to support the DW USB PHY transceivers on MSM chips
> + with DWC3 USB core. It handles PHY initialization, clock
> + management required after resetting the hardware and power
> + management. This driver is required even for peripheral only or
> + host only mode configurations.
> +
> config USB_ULPI
> bool "Generic ULPI Transceiver Driver"
> depends on ARM
> diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
> index 2135e85..4813eb5 100644
> --- a/drivers/usb/phy/Makefile
> +++ b/drivers/usb/phy/Makefile
> @@ -26,6 +26,8 @@ obj-$(CONFIG_USB_EHCI_TEGRA)+=
> phy-tegra-usb.o
> obj-$(CONFIG_USB_GPIO_VBUS) += phy-gpio-vbus-usb.o
> obj-$(CONFIG_USB_ISP1301)+= phy-isp1301.o
> obj-$(CONFIG_USB_MSM_OTG)+= phy-msm-usb.o
> +obj-$(CONFIG_USB_MSM_DW_PHYS)+= phy-msm-dw-hs.o
> +obj-$(CONFIG_USB_MSM_DW_PHYS)+= phy-msm-dw-ss.o
> obj-$(CONFIG_USB_MV_OTG) += phy-mv-usb.o
> obj-$(CONFIG_USB_MXS_PHY)+= phy-mxs-usb.o
> obj-$(CONFIG_USB_RCAR_PHY) += phy-rcar-usb.o
> diff --git a/drivers/usb/phy/phy-msm-dw-hs.c b/drivers/usb/phy/phy-msm-dw-hs.c
> new file mode 100644
> index 000..d29c1f1
> --- /dev/null
> +++ b/drivers/usb/phy/phy-msm-dw-hs.c
> @@ -0,0 +1,329 @@
> +/* Copyright (c) 2013, Code Aurora Forum. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only 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
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +/**
> + * USB QSCRATCH Hardware registers
> + */
> +#define QSCRATCH_CTRL_REG(0x04)
> +#define QSCRATCH_GENERAL_CFG (0x08)
> +#define PHY_CTRL_REG (0x10)
> +#define PARAMETER_OVERRIDE_X_REG (0x14)
> +#define CHARGING_DET_CTRL_REG(0x18)
> +#define CHARGING_DET_OUTPUT_REG (0x1c)
> +#define ALT_INTERRUPT_EN_REG (0x20)
> +#define PHY_IRQ_STAT_REG (0x24)
> +#define CGCTL_REG(0x28)
> +
Please remove braces above and below.
> +#define PHY_3P3_VOL_MIN 305 /* uV */
> +#define PHY_3P3_VOL_MAX 330 /* uV */
> +#define PHY_3P3_HPM_LOAD 16000 /* uA */
> +
> +#define PHY_1P8_VOL_MIN 180 /* uV */
> +#define PHY_1P8_VOL_MAX 180 /* uV */
> +#define PHY_1P8_HPM_LOAD 19000 /* uA */
> +
> +/* TODO: these are suspicious */
> +#define USB_VDDCX_NO 1 /* index */
> +#define USB_VDDCX_MIN5 /* index */
> +#define USB_VDDCX_MAX7 /* index */
> +
> +struct msm_dw_hs_phy {
> + struct usb_phy phy;
> + void __iomem*base;
> + struct device *dev;
> +
> + struct clk *xo_clk;
> + struct clk *sleep_a_clk;
> +
> + struct regulator*v3p3;
> + struct regulator*v1p8;
> + struct regulator*vddcx;
> + struct regulator*vbus;
> +};
> +
> +#define phy_to_dw_phy(x)container_of((x), struct msm_dw_hs_phy,
> phy)
I think that using tab after #define is uncommon, isn't it?
> +
> +
> +/**
>
Re: [PATCH v6 3/3] usb: dwc3: Add Qualcomm DWC3 glue layer driver
Hi Ivan,
Minor comments below.
On 10/07/2013 10:44 AM, Ivan T. Ivanov wrote:
> From: "Ivan T. Ivanov"
>
> DWC3 glue layer is hardware layer around Synopsys DesignWare
> USB3 core. Its purpose is to supply Synopsys IP with required
> clocks, voltages and interface it with the rest of the SoC.
>
> Signed-off-by: Ivan T. Ivanov
> ---
> drivers/usb/dwc3/Kconfig|8 +++
> drivers/usb/dwc3/Makefile |1 +
> drivers/usb/dwc3/dwc3-msm.c | 168
> +++
> 3 files changed, 177 insertions(+)
> create mode 100644 drivers/usb/dwc3/dwc3-msm.c
>
> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
> index 70fc430..4c7b5a4 100644
> --- a/drivers/usb/dwc3/Kconfig
> +++ b/drivers/usb/dwc3/Kconfig
> @@ -59,6 +59,14 @@ config USB_DWC3_EXYNOS
> Recent Exynos5 SoCs ship with one DesignWare Core USB3 IP inside,
> say 'Y' or 'M' if you have one such device.
>
> +config USB_DWC3_MSM
> + tristate "Qualcomm MSM/APQ Platforms"
> + default USB_DWC3
> + select USB_MSM_DWC3_PHYS
> + help
> + Recent Qualcomm SoCs ship with one DesignWare Core USB3 IP inside,
> + say 'Y' or 'M' if you have one such device.
> +
> config USB_DWC3_PCI
> tristate "PCIe-based Platforms"
> depends on PCI
> diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
> index dd17601..a90de66 100644
> --- a/drivers/usb/dwc3/Makefile
> +++ b/drivers/usb/dwc3/Makefile
> @@ -31,4 +31,5 @@ endif
>
> obj-$(CONFIG_USB_DWC3_OMAP) += dwc3-omap.o
> obj-$(CONFIG_USB_DWC3_EXYNOS)+= dwc3-exynos.o
> +obj-$(CONFIG_USB_DWC3_MSM) += dwc3-msm.o
> obj-$(CONFIG_USB_DWC3_PCI) += dwc3-pci.o
> diff --git a/drivers/usb/dwc3/dwc3-msm.c b/drivers/usb/dwc3/dwc3-msm.c
> new file mode 100644
> index 000..1d73f92
> --- /dev/null
> +++ b/drivers/usb/dwc3/dwc3-msm.c
> @@ -0,0 +1,168 @@
> +/* Copyright (c) 2013, The Linux Foundation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only 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
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +struct dwc3_msm {
> + struct device *dev;
> +
> + struct clk *core_clk;
> + struct clk *iface_clk;
> + struct clk *sleep_clk;
> + struct clk *utmi_clk;
> +
> + struct regulator*gdsc;
> +};
> +
> +static int dwc3_msm_probe(struct platform_device *pdev)
> +{
> + struct device_node *node = pdev->dev.of_node;
> + struct dwc3_msm *mdwc;
> + struct resource *res;
> + void __iomem *tcsr;
> + int ret = 0;
> +
> + mdwc = devm_kzalloc(&pdev->dev, sizeof(*mdwc), GFP_KERNEL);
> + if (!mdwc)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, mdwc);
> +
> + mdwc->dev = &pdev->dev;
> +
> + mdwc->gdsc = devm_regulator_get(mdwc->dev, "gdsc");
> +
> + mdwc->core_clk = devm_clk_get(mdwc->dev, "core");
> + if (IS_ERR(mdwc->core_clk)) {
> + dev_dbg(mdwc->dev, "failed to get core clock\n");
> + return PTR_ERR(mdwc->core_clk);
> + }
> +
> + mdwc->iface_clk = devm_clk_get(mdwc->dev, "iface");
> + if (IS_ERR(mdwc->iface_clk)) {
> + dev_dbg(mdwc->dev, "failed to get iface clock\n");
> + return PTR_ERR(mdwc->iface_clk);
> + }
> +
> + mdwc->sleep_clk = devm_clk_get(mdwc->dev, "sleep");
> + if (IS_ERR(mdwc->sleep_clk)) {
> + dev_dbg(mdwc->dev, "failed to get sleep clock\n");
> + return PTR_ERR(mdwc->sleep_clk);
> + }
> +
> + mdwc->utmi_clk = devm_clk_get(mdwc->dev, "utmi");
> + if (IS_ERR(mdwc->utmi_clk)) {
> + dev_dbg(mdwc->dev, "failed to get utmi clock\n");
> + return PTR_ERR(mdwc->utmi_clk);
> + }
I'm not sure that those dev_dbg() are useful at all.
> +
> + if (!IS_ERR(mdwc->gdsc)) {
> + ret = regulator_enable(mdwc->gdsc);
> + if (ret)
> + dev_err(mdwc->dev, "cannot enable gdsc\n");
> + }
> +
> + /*
> + * DWC3 Core requires its CORE CLK (aka master / bus clk) to
> + * run at 125Mhz in SSUSB mode and >60MHZ for HSUSB mode.
> + */
> + clk_set_rate(mdwc->core_clk, 12500);
> + clk_prepare_enable(mdwc->core_clk);
> + clk_prepare_enable(mdwc->iface_clk);
> + clk_prepare_enable(mdwc->sleep_clk);
> + clk_prepare_enable(mdwc->utmi_clk);
All function calls above could return errors.
Re: [PATCH v5] mmc: sdhci-msm: Add support for MSM chipsets
Hi Georgi,
Thanks for the patch.
I have some commnets below.
On 09/16/2013 05:23 PM, Georgi Djakov wrote:
> This platform driver adds the support of Secure Digital Host Controller
> Interface compliant controller found in Qualcomm MSM chipsets.
>
> CC: Asutosh Das
> CC: Venkat Gopalakrishnan
> CC: Sahitya Tummala
> CC: Subhash Jadavani
> Signed-off-by: Georgi Djakov
> ---
> Changes from v4:
> - Simplified sdhci_msm_vreg_disable() and sdhci_msm_set_vdd_io_vol()
> - Use devm_ioremap_resource() instead of devm_ioremap()
> - Converted IS_ERR_OR_NULL to IS_ERR
> - Disable regulators in sdhci_msm_remove()
> - Check for DT node at the beginning in sdhci_msm_probe()
> - Removed more redundant code
> - Changes in some error messages
> - Minor fixes
>
> Changes from v3:
> - Allocate memory for all required structs at once
> - Added termination entry in sdhci_msm_dt_match[]
> - Fixed a missing sdhci_pltfm_free() in probe()
> - Removed redundant of_match_ptr
> - Removed the unneeded function sdhci_msm_vreg_reset()
>
> Changes from v2:
> - Added DT bindings for clocks
> - Moved voltage regulators data to platform data
> - Removed unneeded includes
> - Removed obsolete and wrapper functions
> - Removed error checking where unnecessary
> - Removed redundant _clk suffix from clock names
> - Just return instead of goto where possible
> - Minor fixes
>
> Changes from v1:
> - GPIO references are replaced by pinctrl
> - DT parsing is done mostly by mmc_of_parse()
> - Use of_match_device() for DT matching
> - A few minor changes
>
> .../devicetree/bindings/mmc/sdhci-msm.txt | 71 +++
> drivers/mmc/host/Kconfig | 13 +
> drivers/mmc/host/Makefile |1 +
> drivers/mmc/host/sdhci-msm.c | 627
>
> 4 files changed, 712 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mmc/sdhci-msm.txt
> create mode 100644 drivers/mmc/host/sdhci-msm.c
>
> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
> b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
> new file mode 100644
> index 000..ee112da
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
> @@ -0,0 +1,71 @@
> +* Qualcomm SDHCI controller (sdhci-msm)
> +
> +This file documents differences between the core properties in mmc.txt
> +and the properties used by the sdhci-msm driver.
> +
> +Required properties:
> +- compatible: should be "qcom,sdhci-msm"
> +- reg: should contain SDHC, SD Core register map
> +- reg-names: indicates various resources passed to driver (via reg proptery)
> by name
> + "reg-names" examples are "hc_mem" and "core_mem"
> +- interrupts: should contain SDHC interrupts
> +- interrupt-names: indicates interrupts passed to driver (via interrupts
> property) by name
> + "interrupt-names" examples are "hc_irq" and "pwr_irq"
> +- -supply: phandle to the regulator device tree node
> + "supply-name" examples are "vdd" and "vdd-io"
> +- pinctrl-names: Should contain only one value - "default".
> +- pinctrl-0: Should specify pin control groups used for this controller.
> +- clocks: phandles to clock instances of the device tree nodes
> +- clock-names:
> + iface: Main peripheral bus clock (PCLK/HCLK - AHB Bus clock) (required)
> + core: SDC MMC clock (MCLK) (required)
> + bus: SDCC bus voter clock (optional)
> +
> +Optional properties:
> +- qcom,bus-speed-mode - specifies supported bus speed modes by host
> + The supported bus speed modes are :
> + "HS200_1p8v" - indicates that host can support HS200 at 1.8v
> + "HS200_1p2v" - indicates that host can support HS200 at 1.2v
> + "DDR_1p8v" - indicates that host can support DDR mode at 1.8v
> + "DDR_1p2v" - indicates that host can support DDR mode at 1.2v
> +
> +In the following, can be vdd (flash core voltage) or vdd-io (I/O
> voltage).
> +- qcom,-always-on - specifies whether supply should be kept "on"
> always.
> +- qcom,-lpm-sup - specifies whether supply can be kept in low power
> mode (lpm).
> +- qcom,-voltage-level - specifies voltage levels for supply. Should
> be
> +specified in pairs (min, max), units uV.
> +- qcom,-current-level - specifies load levels for supply in lpm or
> high power mode
> + (hpm). Should be specified in pairs (lpm, hpm), units uA.
> +
> +Example:
> +
> + aliases {
> + sdhc1 = &sdhc_1;
> + };
> +
> + sdhc_1: qcom,sdhc@f9824900 {
> + compatible = "qcom,sdhci-msm";
> + reg = <0xf9824900 0x11c>, <0xf9824000 0x800>;
> + reg-names = "hc_mem", "core_mem";
> + interrupts = <0 123 0>, <0 138 0>;
> + interrupt-names = "hc_irq", "pwr_irq";
> + bus-width = <4>;
> + non-removable;
> +
> + vdd-supply = <&pm8941_l21>;
> + vdd-io-supply = <&pm8941_l13>;
> + qcom,vdd-voltage-level = <295 295>;
> + qcom,vdd-curre
Re: [PATCH 1/2] hwspinlock/omap: add support for dt nodes
Hi Suman, Thanks for the patch. On 09/03/2013 08:52 PM, Suman Anna wrote: > HwSpinlock IP is present only on OMAP4 and other newer SoCs, > which are all device-tree boot only. This patch adds the > base support for parsing the DT nodes, and removes the code > dealing with the traditional platform device instantiation. > > Signed-off-by: Suman Anna > --- > .../devicetree/bindings/hwlock/omap-hwspinlock.txt | 28 ++ > arch/arm/mach-omap2/Makefile | 3 -- > arch/arm/mach-omap2/hwspinlock.c | 60 > -- > drivers/hwspinlock/omap_hwspinlock.c | 21 ++-- > 4 files changed, 45 insertions(+), 67 deletions(-) > create mode 100644 > Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt > delete mode 100644 arch/arm/mach-omap2/hwspinlock.c > > diff --git a/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt > b/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt > new file mode 100644 > index 000..adfb8ad > --- /dev/null > +++ b/Documentation/devicetree/bindings/hwlock/omap-hwspinlock.txt > @@ -0,0 +1,28 @@ > +OMAP4+ HwSpinlock Driver > + > +Required properties: > +- compatible:Currently supports only "ti,omap4-hwspinlock" > for > + OMAP44xx, OMAP54xx, AM33xx, AM43xx, DRA7xx SoCs > +- reg: Contains the hwspinlock register address range > (base > + address and length) > +- ti,hwmods: Name of the hwmod associated with the hwspinlock device > + > +Optional properties: > +- base_id: Base Id for the locks for a particular hwspinlock Could you rename base_id to base-id. The property name convention is to use dashes in DT world. regards, Stan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] hwrng: msm: Add PRNG support for MSM SoC's
Hi Stephen,
On 10/04/2013 07:37 PM, Stephen Boyd wrote:
> On 10/04/13 09:31, Stanimir Varbanov wrote:
>>
>>>> +static int msm_rng_probe(struct platform_device *pdev)
>>>> +{
>>>> + struct msm_rng *rng;
>>>> + struct device_node *np;
>>>> + struct resource res;
>>>> + int ret;
>>>> +
>>>> + np = of_node_get(pdev->dev.of_node);
>>>> + if (!np)
>>>> + return -ENODEV;
>>> This is unnecessary.
>> I used this call because CONFIG_OF_DYNAMIC could be enabled at some
>> time. Isn't that possible? I saw that of_node_get|put is used in .probe
>> on few places in drivers.
>
> So far we aren't selecting that config on ARM.
>
> If you look at of_device_alloc() you'll see
>
> dev->dev.of_node = of_node_get(np);
>
> so any platform devices created from of_platform_populate won't have
> their of_node go away.
Thanks for the pointers, it makes sense. I'll remove the calls to
of_node_get|put.
regards,
Stan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/2] Add support for Qualcomm's PRNG
Hi Ted, On 10/04/2013 09:10 PM, Theodore Ts'o wrote: > On Fri, Oct 04, 2013 at 07:23:50PM +0300, Stanimir Varbanov wrote: >> I guess that it should follow NIST 800-90 recommendation, but I'm not >> aware what DRBG mechanism is used. >> >> To be honest I really don't know the hardware implementation details. I >> put PRNG abbreviation in the cover letter just because I saw that >> defines for register offsets are prefixed with PRNG_*. I could rename >> all occurrences of PRNG to RNG. Is that will be enough to avoid confusions? > > If that's what the Qualcomm documentation uses, maybe we should stick > with it, and add some explanatory comments. Is there any > documentation for this block that is public that you can either send > me a a pointer to? No, there is no public documentation for the block. Here is the driver documentation which I used as a base [1]. My guess was that - if it is PRNG (got from hardware description link above) than according to wiki [2] it is also known as a deterministic random bit generator (DRBG). The recommendation for RNG using DRBG is NIST 800-90. Of course I could be wrong, so I can add a comment that this is just a guess and we shouldn't over-reliance on this. regards, Stan [1] https://www.codeaurora.org/cgit/quic/la/kernel/msm/tree/Documentation/arm/msm/msm_rng-driver.txt?h=jb_3.2.1 [2] http://en.wikipedia.org/wiki/Pseudorandom_number_generator -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/2] Add support for Qualcomm's PRNG
Hi Ted, Peter, On 10/09/2013 06:07 PM, H. Peter Anvin wrote: > On 10/09/2013 07:46 AM, Stanimir Varbanov wrote: >> >> No, there is no public documentation for the block. Here is the driver >> documentation which I used as a base [1]. >> >> My guess was that - if it is PRNG (got from hardware description link >> above) than according to wiki [2] it is also known as a deterministic >> random bit generator (DRBG). The recommendation for RNG using DRBG is >> NIST 800-90. >> >> Of course I could be wrong, so I can add a comment that this is just a >> guess and we shouldn't over-reliance on this. >> > > There needs to be an architecturally guaranteed lower bound on the > entropic content for this to be at all useful. However, the hwrandom > interface is currently expecting fully entropic output (which is almost > certainly bogus... consider the PowerPC random number generator[1]) and > so using it for a PRNG output is directly wrong. This is part of why > RDRAND support is implemented directly in rngd so that we can do the > required cryptographic data reduction to produce fully entropic output. I ran the rngtest with following command line: # cat /dev/hw_random | rngtest -c 10 Copyright (c) 2004 by Henrique de Moraes Holschuh This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. rngtest: starting FIPS tests... rngtest: bits received from input: 200032 rngtest: FIPS 140-2 successes: 99925 rngtest: FIPS 140-2 failures: 75 rngtest: FIPS 140-2(2001-10-10) Monobit: 10 rngtest: FIPS 140-2(2001-10-10) Poker: 9 rngtest: FIPS 140-2(2001-10-10) Runs: 20 rngtest: FIPS 140-2(2001-10-10) Long run: 38 rngtest: FIPS 140-2(2001-10-10) Continuous run: 0 rngtest: input channel speed: (min=1.267; avg=53.222; max=2384.186)Mibits/s rngtest: FIPS tests speed: (min=3.016; avg=48.847; max=49.931)Mibits/s rngtest: Program run time: 75191914 microseconds Could you guys comment those results? regards, Stan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] gpio: msm: Add device tree and irqdomain support for gpio-msm-v2
Hi, Rohit
Thanks for the patch!
On 05/21/2013 09:32 PM, Rohit Vaswani wrote:
> This cleans up the gpio-msm-v2 driver of all the global define usage.
> The number of gpios are now defined in the device tree. This enables
> adding irqdomain support as well.
>
> Signed-off-by: Rohit Vaswani
> ---
>
> static DEFINE_SPINLOCK(tlmm_lock);
> @@ -168,18 +173,20 @@ static void msm_gpio_free(struct gpio_chip *chip,
> unsigned offset)
>
> static int msm_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
> {
> - return MSM_GPIO_TO_INT(chip->base + offset);
> + struct msm_gpio_dev *g_dev = to_msm_gpio_dev(chip);
> + struct irq_domain *domain = g_dev->domain;
> + return irq_create_mapping(domain, offset);
IMO here you should use irq_find_mapping() and create irq mapping once
in .probe. See below comment.
> }
>
> -static int msm_gpio_probe(struct platform_device *dev)
> +static struct lock_class_key msm_gpio_lock_class;
> +
> +static int msm_gpio_irq_domain_map(struct irq_domain *d, unsigned int irq,
> +irq_hw_number_t hwirq)
> +{
> + irq_set_lockdep_class(irq, &msm_gpio_lock_class);
> + irq_set_chip_and_handler(irq, &msm_gpio_irq_chip,
> + handle_level_irq);
> + set_irq_flags(irq, IRQF_VALID);
> +
> + return 0;
> +}
> +
> +static const struct irq_domain_ops msm_gpio_irq_domain_ops = {
> + .xlate = irq_domain_xlate_twocell,
> + .map = msm_gpio_irq_domain_map,
> +};
> +
> +static int msm_gpio_irqdomain_init(struct device_node *node, int ngpio)
> {
> - int i, irq, ret;
> + msm_gpio.domain = irq_domain_add_linear(node, ngpio,
> + &msm_gpio_irq_domain_ops, &msm_gpio);
> + if (!msm_gpio.domain) {
> + WARN(1, "Cannot allocate irq_domain\n");
Are you sure that we want to WARN if no memory? I'd return an error and
fail the probe if the driver can't works without interrupts.
> + return -ENOMEM;
> + }
> +
> + return 0;
> +}
> +
> +static int msm_gpio_probe(struct platform_device *pdev)
> +{
> + int i, irq, ret, ngpio;
> + struct resource *res;
> +
> + msm_gpio.gpio_chip.label = pdev->name;
> + msm_gpio.gpio_chip.dev = &pdev->dev;
> + of_property_read_u32(pdev->dev.of_node, "ngpio", &ngpio);
> + msm_gpio.gpio_chip.ngpio = ngpio;
> +
> + res = platform_get_resource(&pdev->dev, IORESOURCE_MEM, 0);
> + if (!res) {
> + dev_err(&pdev->dev, "%s: no mem resource\n", __func__);
> + return -EINVAL;
> + }
> +
> + msm_tlmm_base = devm_ioremap_resource(pdev->dev, res);
> + if (!msm_tlmm_base) {
> + dev_err(&pdev->dev, "Couldn't allocate memory for msm tlmm
> base\n");
> + return -ENOMEM;
> + }
> +
> + msm_gpio.enabled_irqs = devm_kzalloc(&pdev->dev,
> + sizeof(unsigned long) * ngpio,
> + GFP_KERNEL);
> + msm_gpio.wake_irqs = devm_kzalloc(&pdev->dev,
> + sizeof(unsigned long) * ngpio,
> + GFP_KERNEL);
> + msm_gpio.dual_edge_irqs = devm_kzalloc(&pdev->dev,
> + sizeof(unsigned long) * ngpio,
> + GFP_KERNEL);
> + bitmap_zero(msm_gpio.enabled_irqs, ngpio);
> + bitmap_zero(msm_gpio.wake_irqs, ngpio);
> + bitmap_zero(msm_gpio.dual_edge_irqs, ngpio);
>
> - bitmap_zero(msm_gpio.enabled_irqs, NR_GPIO_IRQS);
> - bitmap_zero(msm_gpio.wake_irqs, NR_GPIO_IRQS);
> - bitmap_zero(msm_gpio.dual_edge_irqs, NR_GPIO_IRQS);
> - msm_gpio.gpio_chip.label = dev->name;
> ret = gpiochip_add(&msm_gpio.gpio_chip);
> - if (ret < 0)
> + if (ret < 0) {
> + dev_err(&pdev->dev, "gpiochip_add failed with error %d\n", ret);
> return ret;
> + }
> +
> + summary_irq = platform_get_irq(pdev, 0);
> + if (summary_irq < 0) {
> + dev_err(&pdev->dev, "No Summary irq defined for msmgpio\n");
> + return summary_irq;
> + }
> +
> + msm_gpio_irqdomain_init(pdev->dev.of_node, msm_gpio.gpio_chip.ngpio);
Adding irqdomain might fail, could you check the return value. And if
irqdomain init fail do we need to set up chained handler for summary_irq
at all?
>
> for (i = 0; i < msm_gpio.gpio_chip.ngpio; ++i) {
> irq = msm_gpio_to_irq(&msm_gpio.gpio_chip, i);
I'd call irq_create_mapping() instead. This way the mapping will be
created once in .probe and use irq_find_mapping() in gpio_to_irq.
> + irq_set_lockdep_class(irq, &msm_gpio_lock_class);
> irq_set_chip_and_handler(irq, &msm_gpio_irq_chip,
>handle_level_irq);
> set_irq_flags(irq, IRQF_VALID);
These three function calls are not needed anymore because
irq_create_mapping() will call inter
Re: [PATCHv2 3/3] gpio: msm: Add device tree and irqdomain support for gpio-msm-v2
Hi Rohit,
Thanks for the new version!
I have few more comments below.
On 05/23/2013 03:29 AM, Rohit Vaswani wrote:
> This cleans up the gpio-msm-v2 driver of all the global define usage.
> The number of gpios are now defined in the device tree. This enables
> adding irqdomain support as well.
>
> Signed-off-by: Rohit Vaswani
> ---
> .../devicetree/bindings/gpio/gpio-msm.txt | 26 +++
> arch/arm/boot/dts/msm8660-surf.dts | 11 ++
> arch/arm/boot/dts/msm8960-cdp.dts | 11 ++
> drivers/gpio/Kconfig |2 +-
> drivers/gpio/gpio-msm-v2.c | 168
> +---
> 5 files changed, 157 insertions(+), 61 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/gpio/gpio-msm.txt
>
> static int msm_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
> {
> - return MSM_GPIO_TO_INT(chip->base + offset);
> + struct msm_gpio_dev *g_dev = to_msm_gpio_dev(chip);
> + struct irq_domain *domain = g_dev->domain;
Could you add blank line here.
> + return irq_create_mapping(domain, offset);
> }
>
> static inline int msm_irq_to_gpio(struct gpio_chip *chip, unsigned irq)
> {
> - return irq - MSM_GPIO_TO_INT(chip->base);
> + struct irq_data *irq_data = irq_get_irq_data(irq);
Blank line please.
> + return irq_data->hwirq;
> }
>
> static struct msm_gpio_dev msm_gpio = {
> .gpio_chip = {
> .base = 0,
> - .ngpio= NR_GPIO_IRQS,
> .direction_input = msm_gpio_direction_input,
> .direction_output = msm_gpio_direction_output,
> .get = msm_gpio_get,
> @@ -226,9 +234,9 @@ static void msm_gpio_update_dual_edge_pos(unsigned gpio)
> if (intstat || val == val2)
> return;
> } while (loop_limit-- > 0);
> - pr_err("dual-edge irq failed to stabilize, "
> + pr_err("%s: dual-edge irq failed to stabilize, "
> "interrupts dropped. %#08x != %#08x\n",
> -val, val2);
> +__func__, val, val2);
> }
>
> static void msm_gpio_irq_ack(struct irq_data *d)
> @@ -312,10 +320,11 @@ static void msm_summary_irq_handler(unsigned int irq,
> struct irq_desc *desc)
> {
> unsigned long i;
> struct irq_chip *chip = irq_desc_get_chip(desc);
> + int ngpio = msm_gpio.gpio_chip.ngpio;
>
> chained_irq_enter(chip, desc);
>
> - for_each_set_bit(i, msm_gpio.enabled_irqs, NR_GPIO_IRQS) {
> + for_each_set_bit(i, msm_gpio.enabled_irqs, ngpio) {
> if (readl(GPIO_INTR_STATUS(i)) & BIT(INTR_STATUS))
> generic_handle_irq(msm_gpio_to_irq(&msm_gpio.gpio_chip,
> i));
As you decided to call irq_create_mapping() from msm_gpio_to_irq, then
in irq handler you must call irq_find_mapping() as stated in
IRQ-domain.txt and probably check return value.
> +static int msm_gpio_irqdomain_init(struct device_node *node, int ngpio)
> +{
> + msm_gpio.domain = irq_domain_add_linear(node, ngpio,
> + &msm_gpio_irq_domain_ops, &msm_gpio);
> + if (!msm_gpio.domain)
> + return -ENOMEM;
> +
> + return 0;
> +}
> +
This function seems meaningless, could you call irq_domain_add_linear
from .probe directly?
- Stan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/8] doc: DT: vidc: binding document for Qualcomm video driver
Hi Rob,
Thanks for the review!
On 09/16/2016 05:19 PM, Rob Herring wrote:
> On Wed, Sep 07, 2016 at 02:37:02PM +0300, Stanimir Varbanov wrote:
>> Adds binding document for vidc video encoder/decoder driver
>>
>> Cc: Rob Herring
>> Cc: Mark Rutland
>> Cc: devicet...@vger.kernel.org
>> Signed-off-by: Stanimir Varbanov
>> ---
>> .../devicetree/bindings/media/qcom,vidc.txt| 61
>> ++
>> 1 file changed, 61 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/media/qcom,vidc.txt
>>
>> diff --git a/Documentation/devicetree/bindings/media/qcom,vidc.txt
>> b/Documentation/devicetree/bindings/media/qcom,vidc.txt
>> new file mode 100644
>> index ..0d50a7b2e3ed
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/qcom,vidc.txt
>> @@ -0,0 +1,61 @@
>> +* Qualcomm video encoder/decoder accelerator
>> +
>> +- compatible:
>> +Usage: required
>> +Value type:
>> +Definition: Value should contain
>
> ... one of:
>
>> +- "qcom,vidc-msm8916"
>> +- "qcom,vidc-msm8996"
>> +- reg:
>> +Usage: required
>> +Value type:
>> +Definition: Register ranges as listed in the reg-names property
>> +
>> +- interrupts:
>> +Usage: required
>> +Value type:
>> +Definition:
>
> How many interrupts?
It is one, thanks for the catch.
>
>> +
>> +- power-domains:
>> +Usage: required
>> +Value type:
>> +Definition: A phandle and power domain specifier pairs to the
>> +power domain which is responsible for collapsing
>> +and restoring power to the peripheral
>
> How many power domains?
Good question, for vidc-msm8916 it is one power-domain, for vidc-msm8996
the power domains should be 3. Here the problem is that the genpd
doesn't permit more than one power-domain per device.
>
>> +
>> +- clocks:
>> +Usage: required
>> +Value type:
>> +Definition: List of phandle and clock specifier pairs as listed
>> +in clock-names property
>> +- clock-names:
>> +Usage: required
>> +Value type:
>> +Definition: Should contain the following entries
>> +- "core" Core video accelerator clock
>> +- "iface" Video accelerator AHB clock
>> +- "bus" Video accelerator AXI clock
>> +- rproc:
>> +Usage: required
>> +Value type:
>> +Definition: A phandle to remote processor responsible for
>> +firmware loading
>> +
>> +- iommus:
>> +Usage: required
>> +Value type:
>> +Definition: A list of phandle and IOMMU specifier pairs
>> +
>> +* An Example
>> +qcom,vidc@1d0 {
>
> node names should be generic: video-codec@
correct, will update it in next version.
--
regards,
Stan
Re: [PATCH 0/8] Qualcomm video decoder/encoder driver
Hi Hans, On 09/05/2016 05:47 PM, Hans Verkuil wrote: > On 08/22/2016 03:13 PM, Stanimir Varbanov wrote: >> This patchset introduces a basic support for Qualcomm video >> acceleration hardware used for video stream decoding/encoding. >> The video IP can found on various qcom SoCs like apq8084, msm8916 >> and msm8996, hence it is widly distributed but the driver is >> missing in the mainline. >> >> The v4l2 driver is something like a wrapper over Host Firmware >> Interface. The HFI itself is a set of command and message packets >> send/received through shared memory, and its purpose is to >> comunicate with the firmware which is run on remote processor. >> The Venus is the name of the video hardware IP that doing the >> video acceleration. >> >> From the software point of view the HFI interface is implemented >> in the files with prefix hfi_xxx. It acts as a translation layer >> between HFI and v4l2 layer. There is one special file in the >> driver called hfi_venus which doing most of the driver >> orchestration work. Something more it setups Venus core, run it >> and handle commands and messages from low-level point of view with >> the help of provided functions by HFI interface. >> >> I think that the driver is in good shape for mainline kernel, and >> I hope the review comments will help to improve it, so please >> do review and make comments. > > I was hoping that I could finish reviewing this patch series today, > but that didn't work out. > > I have more time next week, but I wonder if it isn't better if you make a > v2 first, taking my comments into account. Then I can review v2 next week. OK, I have more of your comments addressed plus few of Bjorn's too. > > Also test with the latest v4l2-compliance (i.e. as of today) since I improved > a few tests relating to g/s_selection and g/s_parm. Sure, I will retest and post the results in cover letter. -- regards, Stan
[PATCH v2 3/8] media: vidc: decoder: add video decoder files
This consists of video decoder implementation plus decoder
controls.
Signed-off-by: Stanimir Varbanov
---
drivers/media/platform/qcom/vidc/vdec.c | 1091 +
drivers/media/platform/qcom/vidc/vdec.h | 29 +
drivers/media/platform/qcom/vidc/vdec_ctrls.c | 200 +
drivers/media/platform/qcom/vidc/vdec_ctrls.h | 21 +
4 files changed, 1341 insertions(+)
create mode 100644 drivers/media/platform/qcom/vidc/vdec.c
create mode 100644 drivers/media/platform/qcom/vidc/vdec.h
create mode 100644 drivers/media/platform/qcom/vidc/vdec_ctrls.c
create mode 100644 drivers/media/platform/qcom/vidc/vdec_ctrls.h
diff --git a/drivers/media/platform/qcom/vidc/vdec.c
b/drivers/media/platform/qcom/vidc/vdec.c
new file mode 100644
index ..433fe4f697d1
--- /dev/null
+++ b/drivers/media/platform/qcom/vidc/vdec.c
@@ -0,0 +1,1091 @@
+/*
+ * Copyright (c) 2012-2015, The Linux Foundation. All rights reserved.
+ * 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 and
+ * only 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
+#include
+#include
+#include
+#include
+#include
+
+#include "core.h"
+#include "helpers.h"
+#include "vdec_ctrls.h"
+
+#define MACROBLOCKS_PER_PIXEL (16 * 16)
+
+static u32 get_framesize_nv12(int plane, u32 height, u32 width)
+{
+ u32 y_stride, uv_stride, y_plane;
+ u32 y_sclines, uv_sclines, uv_plane;
+ u32 size;
+
+ y_stride = ALIGN(width, 128);
+ uv_stride = ALIGN(width, 128);
+ y_sclines = ALIGN(height, 32);
+ uv_sclines = ALIGN(((height + 1) >> 1), 16);
+
+ y_plane = y_stride * y_sclines;
+ uv_plane = uv_stride * uv_sclines + SZ_4K;
+ size = y_plane + uv_plane + SZ_8K;
+
+ return ALIGN(size, SZ_4K);
+}
+
+static u32 get_framesize_compressed(u32 mbs_per_frame)
+{
+ return ((mbs_per_frame * MACROBLOCKS_PER_PIXEL * 3 / 2) / 2) + 128;
+}
+
+static const struct vidc_format vdec_formats[] = {
+ {
+ .pixfmt = V4L2_PIX_FMT_NV12,
+ .num_planes = 1,
+ .type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
+ }, {
+ .pixfmt = V4L2_PIX_FMT_MPEG4,
+ .num_planes = 1,
+ .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
+ }, {
+ .pixfmt = V4L2_PIX_FMT_MPEG2,
+ .num_planes = 1,
+ .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
+ }, {
+ .pixfmt = V4L2_PIX_FMT_H263,
+ .num_planes = 1,
+ .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
+ }, {
+ .pixfmt = V4L2_PIX_FMT_VC1_ANNEX_G,
+ .num_planes = 1,
+ .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
+ }, {
+ .pixfmt = V4L2_PIX_FMT_VC1_ANNEX_L,
+ .num_planes = 1,
+ .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
+ }, {
+ .pixfmt = V4L2_PIX_FMT_H264,
+ .num_planes = 1,
+ .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
+ }, {
+ .pixfmt = V4L2_PIX_FMT_VP8,
+ .num_planes = 1,
+ .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
+ }, {
+ .pixfmt = V4L2_PIX_FMT_XVID,
+ .num_planes = 1,
+ .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
+ },
+};
+
+static const struct vidc_format *find_format(u32 pixfmt, u32 type)
+{
+ const struct vidc_format *fmt = vdec_formats;
+ unsigned int size = ARRAY_SIZE(vdec_formats);
+ unsigned int i;
+
+ for (i = 0; i < size; i++) {
+ if (fmt[i].pixfmt == pixfmt)
+ break;
+ }
+
+ if (i == size || fmt[i].type != type)
+ return NULL;
+
+ return &fmt[i];
+}
+
+static const struct vidc_format *find_format_by_index(int index, u32 type)
+{
+ const struct vidc_format *fmt = vdec_formats;
+ unsigned int size = ARRAY_SIZE(vdec_formats);
+ int i, k = 0;
+
+ if (index < 0 || index > size)
+ return NULL;
+
+ for (i = 0; i < size; i++) {
+ if (fmt[i].type != type)
+ continue;
+ if (k == index)
+ break;
+ k++;
+ }
+
+ if (i == size)
+ return NULL;
+
+ return &fmt[i];
+}
+
+static int vdec_set_properties(struct vidc_inst *inst)
+{
+ struct vdec_controls *ctr = &inst->controls.dec;
+ struct hfi_core *hfi = &inst->core->hfi;
+ s
[PATCH v2 2/8] media: vidc: adding core part and helper functions
This adds core part of the vidc driver common helper functions
used by encoder and decoder specific files.
* core.c has implemented the platform dirver methods, file
operations and v4l2 registration.
* helpers.c has implemented common helper functions for:
- buffer management
- vb2_ops and functions for format propagation,
- functions for allocating and freeing buffers for
internal usage. The buffer parameters describing internal
buffers depends on current format, resolution and codec.
- functions for calculation of current load of the
hardware. Depending on the count of instances and
resolutions it selects the best clock rate for the video
core.
Signed-off-by: Stanimir Varbanov
---
drivers/media/platform/qcom/vidc/core.c| 559 ++
drivers/media/platform/qcom/vidc/core.h| 207 ++
drivers/media/platform/qcom/vidc/helpers.c | 604 +
drivers/media/platform/qcom/vidc/helpers.h | 43 ++
4 files changed, 1413 insertions(+)
create mode 100644 drivers/media/platform/qcom/vidc/core.c
create mode 100644 drivers/media/platform/qcom/vidc/core.h
create mode 100644 drivers/media/platform/qcom/vidc/helpers.c
create mode 100644 drivers/media/platform/qcom/vidc/helpers.h
diff --git a/drivers/media/platform/qcom/vidc/core.c
b/drivers/media/platform/qcom/vidc/core.c
new file mode 100644
index ..0e495f456721
--- /dev/null
+++ b/drivers/media/platform/qcom/vidc/core.c
@@ -0,0 +1,559 @@
+/*
+ * Copyright (c) 2012-2015, The Linux Foundation. All rights reserved.
+ * 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 and
+ * only 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
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "core.h"
+#include "vdec.h"
+#include "venc.h"
+
+static void vidc_add_inst(struct vidc_core *core, struct vidc_inst *inst)
+{
+ mutex_lock(&core->lock);
+ list_add_tail(&inst->list, &core->instances);
+ mutex_unlock(&core->lock);
+}
+
+static void vidc_del_inst(struct vidc_core *core, struct vidc_inst *inst)
+{
+ struct vidc_inst *pos, *n;
+
+ mutex_lock(&core->lock);
+ list_for_each_entry_safe(pos, n, &core->instances, list) {
+ if (pos == inst)
+ list_del(&inst->list);
+ }
+ mutex_unlock(&core->lock);
+}
+
+struct vidc_sys_error {
+ struct vidc_core *core;
+ struct delayed_work work;
+};
+
+static void vidc_sys_error_handler(struct work_struct *work)
+{
+ struct vidc_sys_error *handler =
+ container_of(work, struct vidc_sys_error, work.work);
+ struct vidc_core *core = handler->core;
+ struct hfi_core *hfi = &core->hfi;
+ struct device *dev = core->dev;
+ int ret;
+
+ mutex_lock(&hfi->lock);
+ if (hfi->state != CORE_INVALID)
+ goto exit;
+
+ mutex_unlock(&hfi->lock);
+
+ ret = vidc_hfi_core_deinit(hfi);
+ if (ret)
+ dev_err(dev, "core: deinit failed (%d)\n", ret);
+
+ mutex_lock(&hfi->lock);
+
+ rproc_report_crash(core->rproc, RPROC_FATAL_ERROR);
+
+ rproc_shutdown(core->rproc);
+
+ ret = rproc_boot(core->rproc);
+ if (ret)
+ goto exit;
+
+ hfi->state = CORE_INIT;
+
+exit:
+ mutex_unlock(&hfi->lock);
+ kfree(handler);
+}
+
+static int vidc_event_notify(struct hfi_core *hfi, u32 event)
+{
+ struct vidc_sys_error *handler;
+ struct hfi_inst *inst;
+
+ switch (event) {
+ case EVT_SYS_WATCHDOG_TIMEOUT:
+ case EVT_SYS_ERROR:
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ mutex_lock(&hfi->lock);
+
+ hfi->state = CORE_INVALID;
+
+ list_for_each_entry(inst, &hfi->instances, list) {
+ mutex_lock(&inst->lock);
+ inst->state = INST_INVALID;
+ mutex_unlock(&inst->lock);
+ }
+
+ mutex_unlock(&hfi->lock);
+
+ handler = kzalloc(sizeof(*handler), GFP_KERNEL);
+ if (!handler)
+ return -ENOMEM;
+
+ handler->core = container_of(hfi, struct vidc_core, hfi);
+ INIT_DELAYED_WORK(&handler->work, vidc_sys_error_handler);
+
+ /*
+* Sleep for 5 sec to ensure venus has compl
[PATCH v2 8/8] media: vidc: enable building of the video codec driver
This adds changes in v4l2 platform directory to include the vidc driver and show it in kernel config. Signed-off-by: Stanimir Varbanov --- drivers/media/platform/Kconfig | 1 + drivers/media/platform/Makefile | 1 + 2 files changed, 2 insertions(+) diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig index f25344bc7912..e52640417b0a 100644 --- a/drivers/media/platform/Kconfig +++ b/drivers/media/platform/Kconfig @@ -111,6 +111,7 @@ source "drivers/media/platform/s5p-tv/Kconfig" source "drivers/media/platform/am437x/Kconfig" source "drivers/media/platform/xilinx/Kconfig" source "drivers/media/platform/rcar-vin/Kconfig" +source "drivers/media/platform/qcom/Kconfig" config VIDEO_TI_CAL tristate "TI CAL (Camera Adaptation Layer) driver" diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile index 21771c1a13fb..d8fc75ddcc73 100644 --- a/drivers/media/platform/Makefile +++ b/drivers/media/platform/Makefile @@ -51,6 +51,7 @@ obj-$(CONFIG_VIDEO_RENESAS_JPU) += rcar_jpu.o obj-$(CONFIG_VIDEO_RENESAS_VSP1) += vsp1/ obj-y += omap/ +obj-y += qcom/ obj-$(CONFIG_VIDEO_AM437X_VPFE)+= am437x/ -- 2.7.4
[PATCH v2 4/8] media: vidc: encoder: add video encoder files
This adds encoder part of the driver plus encoder controls.
Signed-off-by: Stanimir Varbanov
---
drivers/media/platform/qcom/vidc/venc.c | 1252 +
drivers/media/platform/qcom/vidc/venc.h | 29 +
drivers/media/platform/qcom/vidc/venc_ctrls.c | 396
drivers/media/platform/qcom/vidc/venc_ctrls.h | 23 +
4 files changed, 1700 insertions(+)
create mode 100644 drivers/media/platform/qcom/vidc/venc.c
create mode 100644 drivers/media/platform/qcom/vidc/venc.h
create mode 100644 drivers/media/platform/qcom/vidc/venc_ctrls.c
create mode 100644 drivers/media/platform/qcom/vidc/venc_ctrls.h
diff --git a/drivers/media/platform/qcom/vidc/venc.c
b/drivers/media/platform/qcom/vidc/venc.c
new file mode 100644
index ..3b65f851a807
--- /dev/null
+++ b/drivers/media/platform/qcom/vidc/venc.c
@@ -0,0 +1,1252 @@
+/*
+ * Copyright (c) 2012-2015, The Linux Foundation. All rights reserved.
+ * 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 and
+ * only 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
+#include
+#include
+#include
+#include
+#include
+
+#include "core.h"
+#include "helpers.h"
+#include "venc_ctrls.h"
+
+#define NUM_B_FRAMES_MAX 4
+
+static u32 get_framesize_nv12(int plane, u32 height, u32 width)
+{
+ u32 y_stride, uv_stride, y_plane;
+ u32 y_sclines, uv_sclines, uv_plane;
+ u32 size;
+
+ y_stride = ALIGN(width, 128);
+ uv_stride = ALIGN(width, 128);
+ y_sclines = ALIGN(height, 32);
+ uv_sclines = ALIGN(((height + 1) >> 1), 16);
+
+ y_plane = y_stride * y_sclines;
+ uv_plane = uv_stride * uv_sclines + SZ_4K;
+ size = y_plane + uv_plane + SZ_8K;
+ size = ALIGN(size, SZ_4K);
+
+ return size;
+}
+
+static u32 get_framesize_compressed(u32 height, u32 width)
+{
+ u32 sz = ALIGN(height, 32) * ALIGN(width, 32) * 3 / 2;
+
+ return ALIGN(sz, SZ_4K);
+}
+
+static const struct vidc_format venc_formats[] = {
+ {
+ .pixfmt = V4L2_PIX_FMT_NV12,
+ .num_planes = 1,
+ .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
+ }, {
+ .pixfmt = V4L2_PIX_FMT_MPEG4,
+ .num_planes = 1,
+ .type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
+ }, {
+ .pixfmt = V4L2_PIX_FMT_H263,
+ .num_planes = 1,
+ .type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
+ }, {
+ .pixfmt = V4L2_PIX_FMT_H264,
+ .num_planes = 1,
+ .type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
+ }, {
+ .pixfmt = V4L2_PIX_FMT_VP8,
+ .num_planes = 1,
+ .type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
+ },
+};
+
+static const struct vidc_format *find_format(u32 pixfmt, int type)
+{
+ const struct vidc_format *fmt = venc_formats;
+ unsigned int size = ARRAY_SIZE(venc_formats);
+ unsigned int i;
+
+ for (i = 0; i < size; i++) {
+ if (fmt[i].pixfmt == pixfmt)
+ break;
+ }
+
+ if (i == size || fmt[i].type != type)
+ return NULL;
+
+ return &fmt[i];
+}
+
+static const struct vidc_format *find_format_by_index(int index, int type)
+{
+ const struct vidc_format *fmt = venc_formats;
+ unsigned int size = ARRAY_SIZE(venc_formats);
+ int i, k = 0;
+
+ if (index < 0 || index > size)
+ return NULL;
+
+ for (i = 0; i < size; i++) {
+ if (fmt[i].type != type)
+ continue;
+ if (k == index)
+ break;
+ k++;
+ }
+
+ if (i == size)
+ return NULL;
+
+ return &fmt[i];
+}
+
+static int venc_v4l2_to_hfi(int id, int value)
+{
+ switch (id) {
+ case V4L2_CID_MPEG_VIDEO_MPEG4_LEVEL:
+ switch (value) {
+ case V4L2_MPEG_VIDEO_MPEG4_LEVEL_0:
+ default:
+ return HFI_MPEG4_LEVEL_0;
+ case V4L2_MPEG_VIDEO_MPEG4_LEVEL_0B:
+ return HFI_MPEG4_LEVEL_0b;
+ case V4L2_MPEG_VIDEO_MPEG4_LEVEL_1:
+ return HFI_MPEG4_LEVEL_1;
+ case V4L2_MPEG_VIDEO_MPEG4_LEVEL_2:
+ return HFI_MPEG4_LEVEL_2;
+ case V4L2_MPEG_VIDEO_MPEG4_LEVEL_3:
+ return HFI_MPEG4_LEVEL_3;
+ case V4L2_MPEG_VIDEO_MPEG4_LEVEL_4:
+
[PATCH v2 6/8] media: vidc: add Venus HFI files
Here is the implementation of Venus video accelerator low-level
functionality. It contanins code which setup the registers and
startup uthe processor, allocate and manipulates with the shared
memory used for sending commands and receiving messages.
Signed-off-by: Stanimir Varbanov
---
drivers/media/platform/qcom/vidc/hfi_venus.c| 1534 +++
drivers/media/platform/qcom/vidc/hfi_venus.h| 25 +
drivers/media/platform/qcom/vidc/hfi_venus_io.h | 98 ++
3 files changed, 1657 insertions(+)
create mode 100644 drivers/media/platform/qcom/vidc/hfi_venus.c
create mode 100644 drivers/media/platform/qcom/vidc/hfi_venus.h
create mode 100644 drivers/media/platform/qcom/vidc/hfi_venus_io.h
diff --git a/drivers/media/platform/qcom/vidc/hfi_venus.c
b/drivers/media/platform/qcom/vidc/hfi_venus.c
new file mode 100644
index ..0799a2f13f8b
--- /dev/null
+++ b/drivers/media/platform/qcom/vidc/hfi_venus.c
@@ -0,0 +1,1534 @@
+/*
+ * Copyright (c) 2012-2015, The Linux Foundation. All rights reserved.
+ * 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 and
+ * only 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
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "core.h"
+#include "hfi_cmds.h"
+#include "hfi_msgs.h"
+#include "hfi_venus.h"
+#include "hfi_venus_io.h"
+
+#define HFI_MASK_QHDR_TX_TYPE 0xff00
+#define HFI_MASK_QHDR_RX_TYPE 0x00ff
+#define HFI_MASK_QHDR_PRI_TYPE 0xff00
+#define HFI_MASK_QHDR_ID_TYPE 0x00ff
+
+#define HFI_HOST_TO_CTRL_CMD_Q 0
+#define HFI_CTRL_TO_HOST_MSG_Q 1
+#define HFI_CTRL_TO_HOST_DBG_Q 2
+#define HFI_MASK_QHDR_STATUS 0x00ff
+
+#define IFACEQ_NUM 3
+#define IFACEQ_CMD_IDX 0
+#define IFACEQ_MSG_IDX 1
+#define IFACEQ_DBG_IDX 2
+#define IFACEQ_MAX_BUF_COUNT 50
+#define IFACEQ_MAX_PARALLEL_CLNTS 16
+#define IFACEQ_DFLT_QHDR 0x0101
+
+#define POLL_INTERVAL_US 50
+
+#define IFACEQ_MAX_PKT_SIZE1024
+#define IFACEQ_MED_PKT_SIZE768
+#define IFACEQ_MIN_PKT_SIZE8
+#define IFACEQ_VAR_SMALL_PKT_SIZE 100
+#define IFACEQ_VAR_LARGE_PKT_SIZE 512
+#define IFACEQ_VAR_HUGE_PKT_SIZE (1024 * 12)
+
+enum tzbsp_video_state {
+ TZBSP_VIDEO_STATE_SUSPEND = 0,
+ TZBSP_VIDEO_STATE_RESUME
+};
+
+struct hfi_queue_table_header {
+ u32 version;
+ u32 size;
+ u32 qhdr0_offset;
+ u32 qhdr_size;
+ u32 num_q;
+ u32 num_active_q;
+};
+
+struct hfi_queue_header {
+ u32 status;
+ u32 start_addr;
+ u32 type;
+ u32 q_size;
+ u32 pkt_size;
+ u32 pkt_drop_cnt;
+ u32 rx_wm;
+ u32 tx_wm;
+ u32 rx_req;
+ u32 tx_req;
+ u32 rx_irq_status;
+ u32 tx_irq_status;
+ u32 read_idx;
+ u32 write_idx;
+};
+
+#define IFACEQ_TABLE_SIZE \
+ (sizeof(struct hfi_queue_table_header) +\
+sizeof(struct hfi_queue_header) * IFACEQ_NUM)
+
+#define IFACEQ_QUEUE_SIZE (IFACEQ_MAX_PKT_SIZE * \
+ IFACEQ_MAX_BUF_COUNT * IFACEQ_MAX_PARALLEL_CLNTS)
+
+#define IFACEQ_GET_QHDR_START_ADDR(ptr, i) \
+ (void *)(((ptr) + sizeof(struct hfi_queue_table_header)) + \
+ ((i) * sizeof(struct hfi_queue_header)))
+
+#define QDSS_SIZE SZ_4K
+#define SFR_SIZE SZ_4K
+#define QUEUE_SIZE \
+ (IFACEQ_TABLE_SIZE + (IFACEQ_QUEUE_SIZE * IFACEQ_NUM))
+
+#define ALIGNED_QDSS_SIZE ALIGN(QDSS_SIZE, SZ_4K)
+#define ALIGNED_SFR_SIZE ALIGN(SFR_SIZE, SZ_4K)
+#define ALIGNED_QUEUE_SIZE ALIGN(QUEUE_SIZE, SZ_4K)
+#define SHARED_QSIZE ALIGN(ALIGNED_SFR_SIZE + ALIGNED_QUEUE_SIZE + \
+ ALIGNED_QDSS_SIZE, SZ_1M)
+
+struct mem_desc {
+ dma_addr_t da; /* device address */
+ void *kva; /* kernel virtual address */
+ u32 size;
+ unsigned long attrs;
+};
+
+struct iface_queue {
+ struct hfi_queue_header *qhdr;
+ struct mem_desc qmem;
+};
+
+enum venus_state {
+ VENUS_STATE_DEINIT = 1,
+ VENUS_STATE_INIT,
+};
+
+struct venus_hfi_device {
+ struct device *dev;
+ void __iomem *base;
+ u32 irq_status;
+ u32 last_packet_type;
+ bool power_enabled;
+ bool suspended;
+ struct completion pwr_collapse_prep;
+ struct completion release_r
[PATCH v2 5/8] media: vidc: add Host Firmware Interface (HFI)
This is the implementation of HFI. It is loaded with the
responsibility to comunicate with the firmware through an
interface commands and messages.
- hfi.c has interface functions used by the core, decoder
and encoder parts to comunicate with the firmware. For example
there are functions for session and core initialisation.
- hfi_cmds has packetization operations which preparing
packets to be send from host to firmware.
- hfi_msgs takes care of messages sent from firmware to the
host.
Signed-off-by: Stanimir Varbanov
---
drivers/media/platform/qcom/vidc/hfi.c| 617
drivers/media/platform/qcom/vidc/hfi.h| 272 ++
drivers/media/platform/qcom/vidc/hfi_cmds.c | 1261 +
drivers/media/platform/qcom/vidc/hfi_cmds.h | 338 +++
drivers/media/platform/qcom/vidc/hfi_helper.h | 1143 ++
drivers/media/platform/qcom/vidc/hfi_msgs.c | 1072 +
drivers/media/platform/qcom/vidc/hfi_msgs.h | 298 ++
7 files changed, 5001 insertions(+)
create mode 100644 drivers/media/platform/qcom/vidc/hfi.c
create mode 100644 drivers/media/platform/qcom/vidc/hfi.h
create mode 100644 drivers/media/platform/qcom/vidc/hfi_cmds.c
create mode 100644 drivers/media/platform/qcom/vidc/hfi_cmds.h
create mode 100644 drivers/media/platform/qcom/vidc/hfi_helper.h
create mode 100644 drivers/media/platform/qcom/vidc/hfi_msgs.c
create mode 100644 drivers/media/platform/qcom/vidc/hfi_msgs.h
diff --git a/drivers/media/platform/qcom/vidc/hfi.c
b/drivers/media/platform/qcom/vidc/hfi.c
new file mode 100644
index ..f5ae53b0e3f2
--- /dev/null
+++ b/drivers/media/platform/qcom/vidc/hfi.c
@@ -0,0 +1,617 @@
+/*
+ * Copyright (c) 2012-2013, The Linux Foundation. All rights reserved.
+ * 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 and
+ * only 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
+#include
+#include
+#include
+#include
+#include
+
+#include "core.h"
+#include "hfi.h"
+#include "hfi_cmds.h"
+#include "hfi_venus.h"
+
+#define TIMEOUTmsecs_to_jiffies(1000)
+
+static u32 to_codec_type(u32 pixfmt)
+{
+ switch (pixfmt) {
+ case V4L2_PIX_FMT_H264:
+ case V4L2_PIX_FMT_H264_NO_SC:
+ return HFI_VIDEO_CODEC_H264;
+ case V4L2_PIX_FMT_H263:
+ return HFI_VIDEO_CODEC_H263;
+ case V4L2_PIX_FMT_MPEG1:
+ return HFI_VIDEO_CODEC_MPEG1;
+ case V4L2_PIX_FMT_MPEG2:
+ return HFI_VIDEO_CODEC_MPEG2;
+ case V4L2_PIX_FMT_MPEG4:
+ return HFI_VIDEO_CODEC_MPEG4;
+ case V4L2_PIX_FMT_VC1_ANNEX_G:
+ case V4L2_PIX_FMT_VC1_ANNEX_L:
+ return HFI_VIDEO_CODEC_VC1;
+ case V4L2_PIX_FMT_VP8:
+ return HFI_VIDEO_CODEC_VP8;
+ case V4L2_PIX_FMT_XVID:
+ return HFI_VIDEO_CODEC_DIVX;
+ default:
+ return 0;
+ }
+}
+
+int vidc_hfi_core_init(struct hfi_core *hfi)
+{
+ int ret = 0;
+
+ mutex_lock(&hfi->lock);
+
+ if (hfi->state >= CORE_INIT)
+ goto unlock;
+
+ init_completion(&hfi->done);
+
+ ret = call_hfi_op(hfi, core_init, hfi);
+ if (ret)
+ goto unlock;
+
+ ret = wait_for_completion_timeout(&hfi->done, TIMEOUT);
+ if (!ret) {
+ ret = -ETIMEDOUT;
+ goto unlock;
+ }
+
+ ret = 0;
+
+ if (hfi->error != HFI_ERR_NONE) {
+ ret = -EIO;
+ goto unlock;
+ }
+
+ hfi->state = CORE_INIT;
+unlock:
+ mutex_unlock(&hfi->lock);
+ return ret;
+}
+
+int vidc_hfi_core_deinit(struct hfi_core *hfi)
+{
+ struct device *dev = hfi->dev;
+ int ret = 0;
+
+ mutex_lock(&hfi->lock);
+
+ if (hfi->state == CORE_UNINIT)
+ goto unlock;
+
+ if (!list_empty(&hfi->instances)) {
+ ret = -EBUSY;
+ goto unlock;
+ }
+
+ ret = call_hfi_op(hfi, core_deinit, hfi);
+ if (ret)
+ dev_err(dev, "core deinit failed: %d\n", ret);
+
+ hfi->state = CORE_UNINIT;
+
+unlock:
+ mutex_unlock(&hfi->lock);
+ return ret;
+}
+
+int vidc_hfi_core_suspend(struct hfi_core *hfi)
+{
+ return call_hfi_op(hfi, suspend, hfi);
+}
+
+int vidc_hfi_core_resume(struct hfi_core *hfi)
+{
+ return call_hfi_op(hfi, resume, hfi);
+}
+
+int vidc_hfi_core_trigger_ssr(struct hfi
[PATCH v2 1/8] doc: DT: vidc: binding document for Qualcomm video driver
Adds binding document for vidc video encoder/decoder driver
Cc: Rob Herring
Cc: Mark Rutland
Cc: devicet...@vger.kernel.org
Signed-off-by: Stanimir Varbanov
---
.../devicetree/bindings/media/qcom,vidc.txt| 61 ++
1 file changed, 61 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/qcom,vidc.txt
diff --git a/Documentation/devicetree/bindings/media/qcom,vidc.txt
b/Documentation/devicetree/bindings/media/qcom,vidc.txt
new file mode 100644
index ..0d50a7b2e3ed
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/qcom,vidc.txt
@@ -0,0 +1,61 @@
+* Qualcomm video encoder/decoder accelerator
+
+- compatible:
+ Usage: required
+ Value type:
+ Definition: Value should contain
+ - "qcom,vidc-msm8916"
+ - "qcom,vidc-msm8996"
+- reg:
+ Usage: required
+ Value type:
+ Definition: Register ranges as listed in the reg-names property
+
+- interrupts:
+ Usage: required
+ Value type:
+ Definition:
+
+- power-domains:
+ Usage: required
+ Value type:
+ Definition: A phandle and power domain specifier pairs to the
+ power domain which is responsible for collapsing
+ and restoring power to the peripheral
+
+- clocks:
+ Usage: required
+ Value type:
+ Definition: List of phandle and clock specifier pairs as listed
+ in clock-names property
+- clock-names:
+ Usage: required
+ Value type:
+ Definition: Should contain the following entries
+ - "core" Core video accelerator clock
+ - "iface" Video accelerator AHB clock
+ - "bus" Video accelerator AXI clock
+- rproc:
+ Usage: required
+ Value type:
+ Definition: A phandle to remote processor responsible for
+ firmware loading
+
+- iommus:
+ Usage: required
+ Value type:
+ Definition: A list of phandle and IOMMU specifier pairs
+
+* An Example
+ qcom,vidc@1d0 {
+ compatible = "qcom,vidc-msm8916";
+ reg = <0x01d0 0xff000>;
+ clocks = <&gcc GCC_VENUS0_VCODEC0_CLK>,
+<&gcc GCC_VENUS0_AHB_CLK>,
+<&gcc GCC_VENUS0_AXI_CLK>;
+ clock-names = "core", "iface", "bus";
+ interrupts = ;
+ power-domains = <&gcc VENUS_GDSC>;
+ rproc = <&vidc_rproc>;
+ iommus = <&apps_iommu 5>;
+ };
--
2.7.4
[PATCH v2 7/8] media: vidc: add Makefiles and Kconfig files
Makefile and Kconfig files to build the video codec driver. Signed-off-by: Stanimir Varbanov --- drivers/media/platform/qcom/Kconfig | 8 drivers/media/platform/qcom/Makefile | 6 ++ drivers/media/platform/qcom/vidc/Makefile | 15 +++ 3 files changed, 29 insertions(+) create mode 100644 drivers/media/platform/qcom/Kconfig create mode 100644 drivers/media/platform/qcom/Makefile create mode 100644 drivers/media/platform/qcom/vidc/Makefile diff --git a/drivers/media/platform/qcom/Kconfig b/drivers/media/platform/qcom/Kconfig new file mode 100644 index ..4bad5c0f68e4 --- /dev/null +++ b/drivers/media/platform/qcom/Kconfig @@ -0,0 +1,8 @@ +comment "Qualcomm V4L2 drivers" + +menuconfig QCOM_VIDC +tristate "Qualcomm V4L2 encoder/decoder driver" +depends on ARCH_QCOM && VIDEO_V4L2 +depends on IOMMU_DMA +depends on QCOM_VENUS_PIL +select VIDEOBUF2_DMA_SG diff --git a/drivers/media/platform/qcom/Makefile b/drivers/media/platform/qcom/Makefile new file mode 100644 index ..150892f6533b --- /dev/null +++ b/drivers/media/platform/qcom/Makefile @@ -0,0 +1,6 @@ +# +# Makefile for the QCOM spcific video device drivers +# based on V4L2. +# + +obj-$(CONFIG_QCOM_VIDC) += vidc/ diff --git a/drivers/media/platform/qcom/vidc/Makefile b/drivers/media/platform/qcom/vidc/Makefile new file mode 100644 index ..f8b5f9a438ee --- /dev/null +++ b/drivers/media/platform/qcom/vidc/Makefile @@ -0,0 +1,15 @@ +# Makefile for Qualcomm vidc driver + +vidc-objs += \ + core.o \ + helpers.o \ + vdec.o \ + vdec_ctrls.o \ + venc.o \ + venc_ctrls.o \ + hfi_venus.o \ + hfi_msgs.o \ + hfi_cmds.o \ + hfi.o \ + +obj-$(CONFIG_QCOM_VIDC) += vidc.o -- 2.7.4
[PATCH v2 0/8] Qualcomm video decoder/encoder driver
ded Pix Format Compliance test for device /dev/video1 (not using libv4l2): Required ioctls: test VIDIOC_QUERYCAP: OK Allow for multiple opens: test second video open: OK test VIDIOC_QUERYCAP: OK test VIDIOC_G/S_PRIORITY: OK test for unlimited opens: OK Debug ioctls: test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported) test VIDIOC_LOG_STATUS: OK (Not Supported) Input ioctls: test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported) test VIDIOC_ENUMAUDIO: OK (Not Supported) test VIDIOC_G/S/ENUMINPUT: OK (Not Supported) test VIDIOC_G/S_AUDIO: OK (Not Supported) Inputs: 0 Audio Inputs: 0 Tuners: 0 Output ioctls: test VIDIOC_G/S_MODULATOR: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_ENUMAUDOUT: OK (Not Supported) test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported) test VIDIOC_G/S_AUDOUT: OK (Not Supported) Outputs: 0 Audio Outputs: 0 Modulators: 0 Input/Output configuration ioctls: test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported) test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported) test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported) test VIDIOC_G/S_EDID: OK (Not Supported) Control ioctls: test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK test VIDIOC_QUERYCTRL: OK test VIDIOC_G/S_CTRL: OK test VIDIOC_G/S/TRY_EXT_CTRLS: OK test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK test VIDIOC_G/S_JPEGCOMP: OK (Not Supported) Standard Controls: 32 Private Controls: 0 Format ioctls: test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK test VIDIOC_G/S_PARM: OK test VIDIOC_G_FBUF: OK (Not Supported) test VIDIOC_G_FMT: OK test VIDIOC_TRY_FMT: OK test VIDIOC_S_FMT: OK test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported) test Cropping: OK test Composing: OK (Not Supported) test Scaling: OK Codec ioctls: test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported) test VIDIOC_G_ENC_INDEX: OK (Not Supported) test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported) Buffer ioctls: test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK test VIDIOC_EXPBUF: OK Test input 0: Total: 43, Succeeded: 43, Failed: 0, Warnings: 0 Stanimir Varbanov (8): doc: DT: vidc: binding document for Qualcomm video driver media: vidc: adding core part and helper functions media: vidc: decoder: add video decoder files media: vidc: encoder: add video encoder files media: vidc: add Host Firmware Interface (HFI) media: vidc: add Venus HFI files media: vidc: add Makefiles and Kconfig files media: vidc: enable building of the video codec driver .../devicetree/bindings/media/qcom,vidc.txt| 61 + drivers/media/platform/Kconfig |1 + drivers/media/platform/Makefile|1 + drivers/media/platform/qcom/Kconfig|8 + drivers/media/platform/qcom/Makefile |6 + drivers/media/platform/qcom/vidc/Makefile | 15 + drivers/media/platform/qcom/vidc/core.c| 559 +++ drivers/media/platform/qcom/vidc/core.h| 207 +++ drivers/media/platform/qcom/vidc/helpers.c | 604 drivers/media/platform/qcom/vidc/helpers.h | 43 + drivers/media/platform/qcom/vidc/hfi.c | 617 drivers/media/platform/qcom/vidc/hfi.h | 272 drivers/media/platform/qcom/vidc/hfi_cmds.c| 1261 drivers/media/platform/qcom/vidc/hfi_cmds.h| 338 + drivers/media/platform/qcom/vidc/hfi_helper.h | 1143 +++ drivers/media/platform/qcom/vidc/hfi_msgs.c| 1072 ++ drivers/media/platform/qcom/vidc/hfi_msgs.h| 298 drivers/media/platform/qcom/vidc/hfi_venus.c | 1534 drivers/media/platform/qcom/vidc/hfi_venus.h | 25 + drivers/media/platform/qcom/vidc/hfi_venus_io.h| 98 ++ drivers/media/platform/qcom/vidc/vdec.c| 1091 ++ drivers/media/platform/qcom/vidc/vdec.h| 29 + drivers/media/platform/qcom/vidc/vdec_ctrls.c | 200 +++ drivers/media/platform/qcom/vidc/vdec_ctrls.h | 21 + drivers/media/platform/qcom/vidc/venc.c| 1252 drivers/media/platform/qcom/vidc/venc.h| 29 + drivers/media/platform/qcom/vidc/venc_ctrls.c | 396 + drivers/media/platform/qcom/vidc/venc_ctrls.h | 23 + 28 files changed, 11204 insertions(+) create m
Re: [PATCH 3/4] dt-binding: remoteproc: venus rproc dt binding document
Hi Bjorn,
On 09/02/2016 11:12 PM, Bjorn Andersson wrote:
> On Fri 02 Sep 04:52 PDT 2016, Marek Szyprowski wrote:
>
>> Hi,
>>
>>
>> On 2016-09-01 16:58, Stanimir Varbanov wrote:
>>> Hi,
>>>
>>> Cc: Marek
>>>
>>
>> ...
>>
>>>>>> But I presume we have the implementation issue of dma_alloc_coherent()
>>>>>> failing in either case with the 5MB size. I think we need to look into
>>>>> I'd be good to include Marek Szyprowski? At least he will know what
>>>>> design restrictions there are.
>>>>>
>>>> Please do. The more I look at this the more I think we must use the
>>>> existing infrastructure for allocating "dma memory". Getting
>>>> dma_alloc_coherent() supporting non-power-of-2 memory regions would
>>> Just to be precise it should be dma_alloc_from_coherent().
>>>
>>> Marek, what is your opinion on that, can we make dma_alloc_from_coherent
>>> able to allocate memory for sizes with bigger granularity.
>>>
>>> For your convenience here [1] is the mail thread.
>>
>> There should be no technical restrictions to add support for bigger
>> granularity than power-of-2. dma_alloc_from_coherent uses standard
>> bitmap based allocator, so it already support tracking allocations of
>> arbitrary size.
>
> I believe we should be able to change the parameter of
> bitmap_{find_free,release,allocate}_region() to take a size rather than
> an order.
>
> The mask used in __reg_op() is an unsigned long, that is stamped over
> the region to be masked or cleared, so there are some clear restrictions
> in what parameters we can pass there - without having to break this
> operation up in steps.
>
> But if drive the offset by taking the next power-of-two of the size and
> then align the number of bits to min(count, BITS_PER_LONG) we should
> retain the performance characteristics and requirements of __reg_op().
>
>> However for the small allocations (smaller than 64KiB?, 512KiB?) it
>> would make sense to keep nearest-power-of-2 round up to prevent memory
>> fragmentation.
>
> But in our case each bit matches a single page, so by making sure the
> mask always fills the unsigned long in the larger cases we would end up
> with having to align things to 128kb (or 256kb if unsigned long is 64
> bit).
>
>
> Does this sound reasonable?
I haven't looked in bitmap details, but can't we reuse genalloc?
--
regards,
Stan
Re: [PATCH] crypto: qce: Initialize core src clock @100Mhz
Hi Iaroslav, On 09/03/2016 07:45 PM, Iaroslav Gridin wrote: > Without that, QCE performance is about 2x less. On which platform? The clock rates are per SoC. > > Signed-off-by: Iaroslav Gridin > --- > drivers/crypto/qce/core.c | 18 +- > drivers/crypto/qce/core.h | 2 +- > 2 files changed, 18 insertions(+), 2 deletions(-) > > diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c > index 0cde513..657354c 100644 > --- a/drivers/crypto/qce/core.c > +++ b/drivers/crypto/qce/core.c > @@ -193,6 +193,10 @@ static int qce_crypto_probe(struct platform_device *pdev) > if (ret < 0) > return ret; > > + qce->core_src = devm_clk_get(qce->dev, "core_src"); > + if (IS_ERR(qce->core_src)) > + return PTR_ERR(qce->core_src); > + > qce->core = devm_clk_get(qce->dev, "core"); > if (IS_ERR(qce->core)) > return PTR_ERR(qce->core); > @@ -205,10 +209,20 @@ static int qce_crypto_probe(struct platform_device > *pdev) > if (IS_ERR(qce->bus)) > return PTR_ERR(qce->bus); > > - ret = clk_prepare_enable(qce->core); > + ret = clk_prepare_enable(qce->core_src); > if (ret) > return ret; > > + ret = clk_set_rate(qce->core_src, 1); Could you point me from where you got this number? Also I think you shouldn't be requesting "core_src" it should be a parent of "core" clock in the clock tree. Did you tried to set rate on "core" clock? regards, Stan
Re: [PATCH] ARM: dts: msm8974: Add definitions for QCE & cryptobam
Hi Iaroslav,
On 08/30/2016 06:37 PM, Iaroslav Gridin wrote:
> From: Voker57
>
> Add device tree definitions for Qualcomm Cryptography engine and its BAM
> Signed-off-by: Iaroslav Gridin
> ---
> arch/arm/boot/dts/qcom-msm8974.dtsi | 42
> +
> 1 file changed, 42 insertions(+)
>
> diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi
> b/arch/arm/boot/dts/qcom-msm8974.dtsi
> index 561d4d1..c0da739 100644
> --- a/arch/arm/boot/dts/qcom-msm8974.dtsi
> +++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
> @@ -287,6 +287,48 @@
> reg = <0xf9011000 0x1000>;
> };
>
> + cryptobam: dma@fd444000 {
> + compatible = "qcom,bam-v1.4.0";
> + reg = <0xfd444000 0x15000>;
> + interrupts = <0 236 0>;
should be
interrupts = ;
> + clocks = <&gcc GCC_CE2_AHB_CLK>,
> + <&gcc GCC_CE2_AXI_CLK>,
> + <&gcc GCC_CE2_CLK>;
> + clock-names = "bam_clk", "axi_clk", "core_clk";
> + #dma-cells = <1>;
> + qcom,ee = <1>;
> + qcom,controlled-remotely;
> + };
indentation please.
> +
> + qcom,qcrypto@fd44 {
> + compatible = "qcom,crypto-v5.1";
> + reg = <0xfd45a000 0x6000>;
> + reg-names = "crypto-base";
> + interrupts = <0 236 0>;
> + qcom,bam-pipe-pair = <2>;
> + qcom,ce-hw-instance = <1>;
> + qcom,ce-device = <0>;
You are getting those 3 properties from qcom downstream kernel, so they
are not relevant to qce in mainline kernel, please drop them.
> + clocks = <&gcc GCC_CE2_CLK>,
> + <&gcc GCC_CE2_AHB_CLK>,
> + <&gcc GCC_CE2_AXI_CLK>,
> + <&gcc CE2_CLK_SRC>;
> +
> + dmas = <&cryptobam 2>, <&cryptobam 3>;
> + dma-names = "rx", "tx";
> + clock-names = "core", "iface", "bus", "core_src";
from here to ...
> + qcom,clk-mgmt-sus-res;
> + qcom,msm-bus,name = "qcrypto-noc";
> +
> + qcom,msm-bus,num-cases = <2>;
> + qcom,msm-bus,num-paths = <1>;
> + qcom,use-sw-aes-cbc-ecb-ctr-algo;
> + qcom,use-sw-aes-xts-algo;
> + qcom,use-sw-ahash-algo;
> + qcom,msm-bus,vectors-KBps = <56 512 0 0>,
> + <56 512 3936000 393600>;
... here, please drop these properties they are no parsed and useful.
regards,
Stan
Re: [PATCH] dma: qcom: Add initialization of axi and core clocks
Hi Iaroslav,
Could you Cc linux-arm-msm ML next time.
Cc: Stephen and linux-arm-msm
Andy, Stephen probably we don't have conclusion about adding those two
clocks in bam driver but do you have some better ideas?
On 08/30/2016 06:42 PM, Iaroslav Gridin wrote:
> From: Voker57
>
> These initialization are missing and causing bam not to init
> Signed-off-by: Iaroslav Gridin
> ---
> drivers/dma/qcom/bam_dma.c | 21 +
> 1 file changed, 21 insertions(+)
>
> diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
> index 03c4eb3..faae0c8 100644
> --- a/drivers/dma/qcom/bam_dma.c
> +++ b/drivers/dma/qcom/bam_dma.c
> @@ -395,6 +395,8 @@ struct bam_device {
> const struct reg_offset_data *layout;
>
> struct clk *bamclk;
> + struct clk *axi_clk;
> + struct clk *core_clk;
> int irq;
>
> /* dma start transaction tasklet */
> @@ -1189,6 +1191,25 @@ static int bam_dma_probe(struct platform_device *pdev)
> return ret;
> }
>
> + bdev->axi_clk = devm_clk_get(bdev->dev, "axi_clk");
> + if (IS_ERR(bdev->axi_clk))
> + bdev->axi_clk = NULL;
> +
> + ret = clk_prepare_enable(bdev->axi_clk);
> + if (ret) {
> + dev_err(bdev->dev, "failed to prepare/enable axi clock\n");
> + return ret;
> + }
> +
> + bdev->core_clk = devm_clk_get(bdev->dev, "core_clk");
> + if (IS_ERR(bdev->core_clk))
> + bdev->core_clk = NULL;
> +
> + ret = clk_prepare_enable(bdev->core_clk);
> + if (ret) {
> + dev_err(bdev->dev, "failed to prepare/enable core clock\n");
> + return ret;
> + }
> ret = bam_init(bdev);
> if (ret)
> goto err_disable_clk;
>
regards,
Stan
--
regards,
Stan
Re: [PATCH 3/4] dt-binding: remoteproc: venus rproc dt binding document
Hi, Cc: Marek On 08/30/2016 08:17 PM, Bjorn Andersson wrote: > On Mon 29 Aug 04:48 PDT 2016, Stanimir Varbanov wrote: > > [..] >>> Trying to wrap my head around how the iommu part works here. The >>> downstream code seems to indicate that this is a "generic" secure iommu >>> interface - used by venus, camera and kgsl; likely for dealing with DRM >>> protected buffers. >> >> The secure iommu interface is for content protected buffers. But these >> secure iommu contexts aren't used by msm DRM nor Venus in mainline. In >> Venus case I use non-secure iommu context for data buffers. >> > > We must consider the case when DRM, VFE and Venus handles protected > buffers. That would be interesting topic. > >>> >>> As such the iommu tables are not part of the venus rproc; I believe they >>> should either be tied into the msm-iommu driver or perhaps exposed as >>> its own iommu(?). >> >> The page tables are in msm-iommu driver. >> > > So, just to verify your answer, the msm-iommu driver will handle both > protected and unprotected? yes, at least for Venus case, the secured buffers are tied to different iommu contexts (and stream IDs). But this needs to be verified more carefully. > >>> >>> >>> But I presume from your inclusion that you've concluded that the venus >>> firmware we have refuses to execute without these tables at least >>> initialized, is this correct? >> >> Yes, the SMC call for PAS memory-setup will fail if this page table is >> not initialized. >> > > If the msm-iommu driver will handle the protected buffers (or if there > will be a separate iommu driver for protected buffers) it should issue > these calls, to not be dependant on the rproc-venus driver. For msm8916 we don't have iommu driver in mainline, I don't know what is the status with msm8996. > > With that I think we should make the rproc-venus driver depend on this > being setup (even if this means creating a "dummy" driver for the > protected iommu handling for now). This sounds scary :) > >>> >>>>> >>>>>> The address is not really fixed, cause the firmware could support >>>>>> relocation. In this example I just picked up the next free memory region >>>>>> in memory-reserved from msm8916.dtsi. >>>>>> >>>>> >>>>> In 8974 we do have a physical region where it's expected to be loaded. >>>>> >>>>> So in line with upcoming remoteproc work we should support referencing a >>>>> reserved-memory node with either reg or size. >>>>> >>>>> In the case of spotting a "reg" we're currently better off using >>>>> ioremap. We're looking at getting the remoteproc core to deal with this >>>>> mess. >>>> >>>> You mean that remoteproc core will parse memory-region property? >>>> >>> >>> It has to, because it's a quite common scenario for remoteproc drivers >>> to either get its backing memory from a static region or be restricted >>> to part of system ram - properties that reserved-memory and >>> memory-region captures already. >> >> OK, I have no issues with that. My concern is the manual parsing of >> 'memory-region' and 'reg' properties in remoteproc core. >> >> So that idea is to have generic binding for rproc, that would be good. >> > > I do share your concerns here. But it's a recurring issue with > remoteproc drivers. > > [..] >>> But I presume we have the implementation issue of dma_alloc_coherent() >>> failing in either case with the 5MB size. I think we need to look into >> >> I'd be good to include Marek Szyprowski? At least he will know what >> design restrictions there are. >> > > Please do. The more I look at this the more I think we must use the > existing infrastructure for allocating "dma memory". Getting > dma_alloc_coherent() supporting non-power-of-2 memory regions would Just to be precise it should be dma_alloc_from_coherent(). Marek, what is your opinion on that, can we make dma_alloc_from_coherent able to allocate memory for sizes with bigger granularity. For your convenience here [1] is the mail thread. > allow us to use the existing infrastructure, for both fixed and > dynamically placed memory carveouts in remoteproc. [1] https://lkml.org/lkml/2016/8/19/570 -- regards, Stan
Re: [PATCH 3/4] dt-binding: remoteproc: venus rproc dt binding document
Hi Rob, On 08/23/2016 08:32 PM, Rob Herring wrote: > On Fri, Aug 19, 2016 at 06:53:19PM +0300, Stanimir Varbanov wrote: >> Add devicetree binding document for Venus remote processor. >> >> Signed-off-by: Stanimir Varbanov >> --- >> .../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 ..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. > > This does not make sense to me. Venus is the video encoder/decoder h/w, > right? Why is the firmware loader separate from the codec block? Why > rproc is used? Are there multiple clients? Naming it rproc_venus implies > there aren't. And why does the firmware loading need 8MB of memory at a > fixed address? > The firmware for Venus case is 5MB. And here is 8MB because of dma_alloc_from_coherent size restriction. The address is not really fixed, cause the firmware could support relocation. In this example I just picked up the next free memory region in memory-reserved from msm8916.dtsi. regards, Stan
Re: [PATCH 2/4] firmware: qcom: scm: add iommu scm calls for pg table
Hi Puja, On 08/24/2016 09:35 PM, Gupta, Puja wrote: > On 8/19/2016 8:53 AM, Stanimir Varbanov wrote: >> Those two scm calls are used to get the size of secure iommu >> page table and to pass physical memory address for this page >> table. The calls are used by remoteproc venus driver to load >> the firmware. > Do we really need these additional scm calls for venus? why can't we > just reuse existing __qcom_scm_pas_mem_setup() call? We are using __qcom_scm_pas_mem_setup() but it will return error if I did not provide page table memory. At least for apq8016 this step looks like is mandatory. Do you have some idea how those iommu calls can be avoided? -- regards, Stan
Re: [PATCH 3/4] dt-binding: remoteproc: venus rproc dt binding document
Hi Bjorn,
On 08/25/2016 03:05 AM, Bjorn Andersson wrote:
> On Wed 24 Aug 08:36 PDT 2016, Stanimir Varbanov wrote:
>
>> Hi Rob,
>>
>> On 08/23/2016 08:32 PM, Rob Herring wrote:
>>> On Fri, Aug 19, 2016 at 06:53:19PM +0300, Stanimir Varbanov wrote:
>>>> Add devicetree binding document for Venus remote processor.
>>>>
>>>> Signed-off-by: Stanimir Varbanov
>>>> ---
>>>> .../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 ..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.
>>>
>>> This does not make sense to me. Venus is the video encoder/decoder h/w,
>>> right? Why is the firmware loader separate from the codec block? Why
>>> rproc is used? Are there multiple clients? Naming it rproc_venus implies
>>> there aren't. And why does the firmware loading need 8MB of memory at a
>>> fixed address?
>>>
>>
>> The firmware for Venus case is 5MB. And here is 8MB because of
>> dma_alloc_from_coherent size restriction.
>>
>
> Then you should specify it 5MB large and we'll have to deal with this
> implementation issue in the code. I've created a JIRA ticket for
> the dma_alloc_from_coherent() behavior.
Infact it should be 5MB + ~100KB for iommu page table.
>
>> The address is not really fixed, cause the firmware could support
>> relocation. In this example I just picked up the next free memory region
>> in memory-reserved from msm8916.dtsi.
>>
>
> In 8974 we do have a physical region where it's expected to be loaded.
>
> So in line with upcoming remoteproc work we should support referencing a
> reserved-memory node with either reg or size.
>
> In the case of spotting a "reg" we're currently better off using
> ioremap. We're looking at getting the remoteproc core to deal with this
> mess.
You mean that remoteproc core will parse memory-region property?
>
>
> So, on 8916 I think you should use the form:
>
> venus_mem: venus {
> size = <0x50>;
> };
Don't forget that the physical address where the firmware is stored has
some range, the scm call will fail if it is out of the expected range,
probably because of some security reasons. So maybe alloc-ranges should
be specified here.
>
> And I don't think you should use the shared-dma-pool compatible, because
> this is not a region for multiple devices to allocate dma memory out of.
Then I cannot reuse reserved-mem infrastructure.
--
regards,
Stan
Re: [PATCH 2/8] media: vidc: adding core part and helper functions
Hi Bjorn, Thanks for the review and comments! On 08/23/2016 05:50 AM, Bjorn Andersson wrote: > On Mon 22 Aug 06:13 PDT 2016, Stanimir Varbanov wrote: > > Hi Stan, > >> This adds core part of the vidc driver common helper functions >> used by encoder and decoder specific files. > > I believe "vidc" is short for "video core" and this is not the only > "video core" from Qualcomm. This driver is the v4l2 <-> hfi interface and What other "video core"s do you know? > uses either two ram based fifos _or_ apr tal for communication with the > implementation. > > In the case of apr, the other side is not the venus core but rather the > "VIDC" apr service on the Hexagon DSP. In this case the hfi packets are > encapsulated in apr packets. Although this is not used in 8916 it would > be nice to be able to add this later... OK, you are talking about q6_hfi.c which file is found in msm-3.10 and maybe older kernel versions. There is a function vidc_hfi_create() which currently creates venus hfi interface but it aways could be extended to call q6 DSP specific function. > > > But I think we should call this driver "hfi" - or at least venus, as > it's not compatible with e.g the "blackbird" found in 8064, which is > also called "vidc". Do you think that vidc driver for 8064 will ever reach the mainline kernel? I personally don't like hfi nor venus other suggestions? Does "vidcore" or "vcore" makes sense? > >> >> - core.c has implemented the platform dirver methods, file >> operations and v4l2 registration. >> >> - helpers.c has implemented common helper functions for >> buffer management, vb2_ops and functions for format propagation. >> >> - int_bufs.c implements functions for allocating and freeing >> buffers for internal usage. The buffer parameters describing >> internal buffers depends on current format, resolution and >> codec. >> >> - load.c consists functions for calculation of current load >> of the hardware. Depending on the count of instances and >> resolutions it selects the best clock rate for the video >> core. >> >> - mem.c has two functions for memory allocation, currently >> those functions are used for internal buffers and to allocate >> the shared memory for communication with firmware via HFI >> (Host Firmware Interface) interface commands. > > Please drop this; see comments on mem_alloc() OK. > >> >> - resources.c exports a structure describing the details >> specific to platform and SoC. >> >> Signed-off-by: Stanimir Varbanov >> --- > > This doesn't compile, as it depends on later patches. Also there are > plenty of functions that are related to later patches and would with be > better to include there, to keep the size of this patch down. > >> drivers/media/platform/qcom/vidc/core.c | 548 >> +++ >> drivers/media/platform/qcom/vidc/core.h | 196 ++ >> drivers/media/platform/qcom/vidc/helpers.c | 394 +++ >> drivers/media/platform/qcom/vidc/helpers.h | 43 +++ >> drivers/media/platform/qcom/vidc/int_bufs.c | 325 >> drivers/media/platform/qcom/vidc/int_bufs.h | 23 ++ >> drivers/media/platform/qcom/vidc/load.c | 104 + >> drivers/media/platform/qcom/vidc/load.h | 22 ++ >> drivers/media/platform/qcom/vidc/mem.c | 64 >> drivers/media/platform/qcom/vidc/mem.h | 32 ++ >> drivers/media/platform/qcom/vidc/resources.c | 46 +++ >> drivers/media/platform/qcom/vidc/resources.h | 46 +++ >> 12 files changed, 1843 insertions(+) >> create mode 100644 drivers/media/platform/qcom/vidc/core.c >> create mode 100644 drivers/media/platform/qcom/vidc/core.h >> create mode 100644 drivers/media/platform/qcom/vidc/helpers.c >> create mode 100644 drivers/media/platform/qcom/vidc/helpers.h >> create mode 100644 drivers/media/platform/qcom/vidc/int_bufs.c >> create mode 100644 drivers/media/platform/qcom/vidc/int_bufs.h >> create mode 100644 drivers/media/platform/qcom/vidc/load.c >> create mode 100644 drivers/media/platform/qcom/vidc/load.h >> create mode 100644 drivers/media/platform/qcom/vidc/mem.c >> create mode 100644 drivers/media/platform/qcom/vidc/mem.h >> create mode 100644 drivers/media/platform/qcom/vidc/resources.c >> create mode 100644 drivers/media/platform/qcom/vidc/resources.h >> >> diff --git a/drivers/media/platform/qcom/vidc/core.c >> b/drivers/media/platform/qcom/vidc/core.c >> new file mode 1006
Re: [PATCH 5/8] media: vidc: add Host Firmware Interface (HFI)
Hi Bjorn,
Thanks for the comments!
On 08/23/2016 06:25 AM, Bjorn Andersson wrote:
> On Mon 22 Aug 06:13 PDT 2016, Stanimir Varbanov wrote:
>
>> This is the implementation of HFI. It is loaded with the
>> responsibility to comunicate with the firmware through an
>> interface commands and messages.
>>
>> - hfi.c has interface functions used by the core, decoder
>> and encoder parts to comunicate with the firmware. For example
>> there are functions for session and core initialisation.
>>
>
> I can't help feeling that the split between core.c and hfi.c is a
> remnant of a vidc driver supporting both HFI and pre-HFI with the same
> v4l code.
>
> What do you think about merging vidc_core with hfi_core and vidc_inst
> with hfi_inst? Both seems to be in a 1:1 relationship.
OK, I can give it a try.
>
>> - hfi_cmds has packetization operations which preparing
>> packets to be send from host to firmware.
>>
>> - hfi_msgs takes care of messages sent from firmware to the
>> host.
>>
> [..]
>> diff --git a/drivers/media/platform/qcom/vidc/hfi_cmds.c
>> b/drivers/media/platform/qcom/vidc/hfi_cmds.c
> [..]
>> +
>> +static const struct hfi_packetization_ops hfi_default = {
>> +.sys_init = pkt_sys_init,
>> +.sys_pc_prep = pkt_sys_pc_prep,
>> +.sys_idle_indicator = pkt_sys_idle_indicator,
>> +.sys_power_control = pkt_sys_power_control,
>> +.sys_set_resource = pkt_sys_set_resource,
>> +.sys_release_resource = pkt_sys_unset_resource,
>> +.sys_debug_config = pkt_sys_debug_config,
>> +.sys_coverage_config = pkt_sys_coverage_config,
>> +.sys_ping = pkt_sys_ping,
>> +.sys_image_version = pkt_sys_image_version,
>> +.ssr_cmd = pkt_ssr_cmd,
>> +.session_init = pkt_session_init,
>> +.session_cmd = pkt_session_cmd,
>> +.session_set_buffers = pkt_session_set_buffers,
>> +.session_release_buffers = pkt_session_release_buffers,
>> +.session_etb_decoder = pkt_session_etb_decoder,
>> +.session_etb_encoder = pkt_session_etb_encoder,
>> +.session_ftb = pkt_session_ftb,
>> +.session_parse_seq_header = pkt_session_parse_seq_header,
>> +.session_get_seq_hdr = pkt_session_get_seq_hdr,
>> +.session_flush = pkt_session_flush,
>> +.session_get_property = pkt_session_get_property,
>> +.session_set_property = pkt_session_set_property,
>> +};
>> +
>> +static const struct hfi_packetization_ops *get_3xx_ops(void)
>> +{
>> +static struct hfi_packetization_ops hfi_3xx;
>> +
>> +hfi_3xx = hfi_default;
>> +hfi_3xx.session_set_property = pkt_session_set_property_3xx;
>> +
>> +return &hfi_3xx;
>> +}
>> +
>> +const struct hfi_packetization_ops *
>> +hfi_get_pkt_ops(enum hfi_packetization_type type)
>
> The only reasonable argument I can come up with for not just exposing
> these as global functions would be that there are 23 of them... Can we
> skip the jump table?
Of course we can, but what will be the benefit, increased readability ?
Let's keep it for now. Once the driver is merged some smarter guy could
came up with better solution ;)
>
>> +{
>> +switch (type) {
>> +case HFI_PACKETIZATION_LEGACY:
>> +return &hfi_default;
>> +case HFI_PACKETIZATION_3XX:
>> +return get_3xx_ops();
>> +}
>> +
>> +return NULL;
>> +}
>
> Regards,
> Bjorn
>
--
regards,
Stan
Re: [RFC PATCH] media: vb2: add bidirectional flag in vb2_queue
Hi Hans,
On 08/15/2017 01:04 PM, Hans Verkuil wrote:
> On 08/14/17 10:41, Stanimir Varbanov wrote:
>> Hi,
>>
>> This RFC patch is intended to give to the drivers a choice to change
>> the default behavior of the v4l2-core DMA mapping direction from
>> DMA_TO/FROM_DEVICE (depending on the buffer type CAPTURE or OUTPUT)
>> to DMA_BIDIRECTIONAL during queue_init time.
>>
>> Initially the issue with DMA mapping direction has been found in
>> Venus encoder driver where the firmware side of the driver adds few
>> lines padding on bottom of the image buffer, and the consequence was
>> triggering of IOMMU protection faults.
>>
>> Probably other drivers could also has a benefit of this feature (hint)
>> in the future.
>>
>> Signed-off-by: Stanimir Varbanov
>> ---
>> drivers/media/v4l2-core/videobuf2-core.c | 3 +++
>> include/media/videobuf2-core.h | 11 +++
>> 2 files changed, 14 insertions(+)
>>
>> diff --git a/drivers/media/v4l2-core/videobuf2-core.c
>> b/drivers/media/v4l2-core/videobuf2-core.c
>> index 14f83cecfa92..17d07fda4cdc 100644
>> --- a/drivers/media/v4l2-core/videobuf2-core.c
>> +++ b/drivers/media/v4l2-core/videobuf2-core.c
>> @@ -200,6 +200,9 @@ static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
>> int plane;
>> int ret = -ENOMEM;
>>
>> +if (q->bidirectional)
>> +dma_dir = DMA_BIDIRECTIONAL;
>> +
>
> Does this only have to be used in mem_alloc? In the __prepare_*() it is still
> using
> DMA_TO/FROM_DEVICE.
Yes, it looks like the DMA direction should be covered in the
__prepare_* too. Thus the patch should look like below:
diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c
index 14f83cecfa92..0089e7dac7dd 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -188,14 +188,21 @@ module_param(debug, int, 0644);
static void __vb2_queue_cancel(struct vb2_queue *q);
static void __enqueue_in_driver(struct vb2_buffer *vb);
+static enum dma_data_direction __get_dma_dir(struct vb2_queue *q)
+{
+ if (q->bidirectional)
+ return DMA_BIDIRECTIONAL;
+
+ return q->is_output ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
+}
+
/**
* __vb2_buf_mem_alloc() - allocate video memory for the given buffer
*/
static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
{
struct vb2_queue *q = vb->vb2_queue;
- enum dma_data_direction dma_dir =
- q->is_output ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
+ enum dma_data_direction dma_dir = __get_dma_dir(q);
void *mem_priv;
int plane;
int ret = -ENOMEM;
@@ -978,8 +985,7 @@ static int __prepare_userptr(struct vb2_buffer *vb,
const void *pb)
void *mem_priv;
unsigned int plane;
int ret = 0;
- enum dma_data_direction dma_dir =
- q->is_output ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
+ enum dma_data_direction dma_dir = __get_dma_dir(q);
bool reacquired = vb->planes[0].mem_priv == NULL;
memset(planes, 0, sizeof(planes[0]) * vb->num_planes);
@@ -1096,8 +1102,7 @@ static int __prepare_dmabuf(struct vb2_buffer *vb,
const void *pb)
void *mem_priv;
unsigned int plane;
int ret = 0;
- enum dma_data_direction dma_dir =
- q->is_output ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
+ enum dma_data_direction dma_dir = __get_dma_dir(q);
bool reacquired = vb->planes[0].mem_priv == NULL;
memset(planes, 0, sizeof(planes[0]) * vb->num_planes);
--
regards,
Stan
Re: [PATCH v7 3/3] PCI: dwc: qcom: Add support for IPQ8074 PCIe controller
Hi Varada,
Thanks for the patch!
On 08/17/2017 10:43 AM, Varadarajan Narayanan wrote:
> Add support for the IPQ8074 PCIe controller. IPQ8074 supports
> Gen 1/2, one lane, two PCIe root complex with support for MSI and
> legacy interrupts, and it conforms to PCI Express Base 2.1
> specification.
>
> The core init is the similar to the existing SoC, however the
> clocks and reset lines differ.
>
> Signed-off-by: smuthayy
> Signed-off-by: Varadarajan Narayanan
> ---
> drivers/pci/dwc/pcie-qcom.c | 233
>
> 1 file changed, 233 insertions(+)
>
> diff --git a/drivers/pci/dwc/pcie-qcom.c b/drivers/pci/dwc/pcie-qcom.c
> index c4cd039..5bfbbd3 100644
> --- a/drivers/pci/dwc/pcie-qcom.c
> +++ b/drivers/pci/dwc/pcie-qcom.c
> @@ -37,6 +37,20 @@
> #include "pcie-designware.h"
>
> #define PCIE20_PARF_SYS_CTRL 0x00
> +#define MST_WAKEUP_ENBIT(13)
> +#define SLV_WAKEUP_ENBIT(12)
> +#define MSTR_ACLK_CGC_DISBIT(10)
> +#define SLV_ACLK_CGC_DIS BIT(9)
> +#define CORE_CLK_CGC_DIS BIT(6)
> +#define AUX_PWR_DET BIT(4)
> +#define L23_CLK_RMV_DIS BIT(2)
> +#define L1_CLK_RMV_DIS BIT(1)
> +
> +#define PCIE20_COMMAND_STATUS0x04
> +#define CMD_BME_VAL 0x4
> +#define PCIE20_DEVICE_CONTROL2_STATUS2 0x98
> +#define PCIE_CAP_CPL_TIMEOUT_DISABLE 0x10
> +
> #define PCIE20_PARF_PHY_CTRL 0x40
> #define PCIE20_PARF_PHY_REFCLK 0x4C
> #define PCIE20_PARF_DBI_BASE_ADDR0x168
> @@ -58,9 +72,21 @@
> #define CFG_BRIDGE_SB_INIT BIT(0)
>
> #define PCIE20_CAP 0x70
> +#define PCIE20_CAP_LINK_CAPABILITIES (PCIE20_CAP + 0xc)
> +#define PCIE20_CAP_ACTIVE_STATE_LINK_PM_SUPPORT (BIT(10) | BIT(11))
> +#define PCIE20_CAP_LINK_1(PCIE20_CAP + 0x14)
> +#define PCIE_CAP_LINK1_VAL 0x2fd7f
> +
> +#define PCIE20_PARF_Q2A_FLUSH0x1ac
> +
> +#define PCIE20_MISC_CONTROL_1_REG0x8bc
> +#define DBI_RO_WR_EN 1
>
> #define PERST_DELAY_US 1000
>
> +#define PCIE20_v3_PARF_SLV_ADDR_SPACE_SIZE 0x358
> +#define SLV_ADDR_SPACE_SZ 0x1000
> +
> struct qcom_pcie_resources_2_1_0 {
> struct clk *iface_clk;
> struct clk *core_clk;
> @@ -110,11 +136,21 @@ struct qcom_pcie_resources_2_4_0 {
> struct reset_control *phy_ahb_reset;
> };
>
> +struct qcom_pcie_resources_2_3_3 {
> + struct clk *iface;
> + struct clk *axi_m_clk;
> + struct clk *axi_s_clk;
> + struct clk *ahb_clk;
> + struct clk *aux_clk;
> + struct reset_control *rst[7];
> +};
> +
> union qcom_pcie_resources {
> struct qcom_pcie_resources_1_0_0 v1_0_0;
> struct qcom_pcie_resources_2_1_0 v2_1_0;
> struct qcom_pcie_resources_2_3_2 v2_3_2;
> struct qcom_pcie_resources_2_4_0 v2_4_0;
> + struct qcom_pcie_resources_2_3_3 v2_3_3;
Could you add the struct v2_3_3 before v2_4_0 to keep the union sorted.
> };
>
> struct qcom_pcie;
> @@ -884,6 +920,194 @@ static int qcom_pcie_init_2_4_0(struct qcom_pcie *pcie)
> return ret;
> }
>
> +static int qcom_pcie_get_resources_2_3_3(struct qcom_pcie *pcie)
> +{
> + struct qcom_pcie_resources_2_3_3 *res = &pcie->res.v2_3_3;
> + struct dw_pcie *pci = pcie->pci;
> + struct device *dev = pci->dev;
> + int i;
> + const char *rst_names[] = { "axi_m", "axi_s", "pipe",
> + "axi_m_sticky", "sticky",
> + "ahb", "sleep", };
> +
> + res->iface = devm_clk_get(dev, "iface");
> + if (IS_ERR(res->iface))
> + return PTR_ERR(res->iface);
> +
> + res->axi_m_clk = devm_clk_get(dev, "axi_m");
> + if (IS_ERR(res->axi_m_clk))
> + return PTR_ERR(res->axi_m_clk);
> +
> + res->axi_s_clk = devm_clk_get(dev, "axi_s");
> + if (IS_ERR(res->axi_s_clk))
> + return PTR_ERR(res->axi_s_clk);
> +
> + res->ahb_clk = devm_clk_get(dev, "ahb");
> + if (IS_ERR(res->ahb_clk))
> + return PTR_ERR(res->ahb_clk);
> +
> + res->aux_clk = devm_clk_get(dev, "aux");
> + if (IS_ERR(res->aux_clk))
> + return PTR_ERR(res->aux_clk);
> +
> + for (i = 0; i < ARRAY_SIZE(rst_names); i++) {
> + res->rst[i] = devm_reset_control_get(dev, rst_names[i]);
> + if (IS_ERR(res->rst[i]))
> + return PTR_ERR(res->rst[i]);
> + }
> +
> + return 0;
> +}
> +
> +static void qcom_pcie_deinit_2_3_3(struct qcom_pcie *pcie)
> +{
> + struct qcom_pcie_resources_2_3_3 *res = &pcie->res.v2_3_3;
> +
> + clk_disable_unprepare(res->iface
Re: [PATCH v8 1/3] PCI: dwc: qcom: Use block IP version for operations
Hi Varada, On 08/17/2017 02:20 PM, Varadarajan Narayanan wrote: > Presently, when support for a new SoC is added, the driver ops > structures and functions are versioned with plain 1, 2, 3 etc. > Instead use the block IP version number. > > Signed-off-by: Varadarajan Narayanan > --- > drivers/pci/dwc/pcie-qcom.c | 132 > +++- > 1 file changed, 68 insertions(+), 64 deletions(-) The patch cannot be applied on todays linux-next so you have to rebase the patches on top of Bjorn's pci.git tree. Otherwise you have my: Acked-by: Stanimir Varbanov regards, Stan
Re: [PATCH v8 3/3] PCI: dwc: qcom: Add support for IPQ8074 PCIe controller
Varada,
On 08/17/2017 02:20 PM, Varadarajan Narayanan wrote:
> Add support for the IPQ8074 PCIe controller. IPQ8074 supports
> Gen 1/2, one lane, two PCIe root complex with support for MSI and
> legacy interrupts, and it conforms to PCI Express Base 2.1
> specification.
>
> The core init is the similar to the existing SoC, however the
> clocks and reset lines differ.
>
> Signed-off-by: smuthayy
> Signed-off-by: Varadarajan Narayanan
> ---
> drivers/pci/dwc/pcie-qcom.c | 208
>
> 1 file changed, 208 insertions(+)
This patch will need re-basing too.
With both comments below addressed:
Acked-by: Stanimir Varbanov
>
> diff --git a/drivers/pci/dwc/pcie-qcom.c b/drivers/pci/dwc/pcie-qcom.c
> index c4cd039..1cb03dd 100644
> --- a/drivers/pci/dwc/pcie-qcom.c
> +++ b/drivers/pci/dwc/pcie-qcom.c
> @@ -37,6 +37,20 @@
> #include "pcie-designware.h"
>
> #define PCIE20_PARF_SYS_CTRL 0x00
> +#define MST_WAKEUP_ENBIT(13)
> +#define SLV_WAKEUP_ENBIT(12)
> +#define MSTR_ACLK_CGC_DISBIT(10)
> +#define SLV_ACLK_CGC_DIS BIT(9)
> +#define CORE_CLK_CGC_DIS BIT(6)
> +#define AUX_PWR_DET BIT(4)
> +#define L23_CLK_RMV_DIS BIT(2)
> +#define L1_CLK_RMV_DIS BIT(1)
> +
> +#define PCIE20_COMMAND_STATUS0x04
> +#define CMD_BME_VAL 0x4
> +#define PCIE20_DEVICE_CONTROL2_STATUS2 0x98
> +#define PCIE_CAP_CPL_TIMEOUT_DISABLE 0x10
> +
> #define PCIE20_PARF_PHY_CTRL 0x40
> #define PCIE20_PARF_PHY_REFCLK 0x4C
> #define PCIE20_PARF_DBI_BASE_ADDR0x168
> @@ -58,9 +72,21 @@
> #define CFG_BRIDGE_SB_INIT BIT(0)
>
> #define PCIE20_CAP 0x70
> +#define PCIE20_CAP_LINK_CAPABILITIES (PCIE20_CAP + 0xc)
> +#define PCIE20_CAP_ACTIVE_STATE_LINK_PM_SUPPORT (BIT(10) | BIT(11))
> +#define PCIE20_CAP_LINK_1(PCIE20_CAP + 0x14)
> +#define PCIE_CAP_LINK1_VAL 0x2fd7f
> +
> +#define PCIE20_PARF_Q2A_FLUSH0x1ac
> +
> +#define PCIE20_MISC_CONTROL_1_REG0x8bc
> +#define DBI_RO_WR_EN 1
>
> #define PERST_DELAY_US 1000
>
> +#define PCIE20_v3_PARF_SLV_ADDR_SPACE_SIZE 0x358
> +#define SLV_ADDR_SPACE_SZ 0x1000
Please use tabs instead of spaces.
> +
> struct qcom_pcie_resources_2_1_0 {
> struct clk *iface_clk;
> struct clk *core_clk;
> @@ -110,10 +136,20 @@ struct qcom_pcie_resources_2_4_0 {
> struct reset_control *phy_ahb_reset;
> };
>
> +struct qcom_pcie_resources_2_3_3 {
> + struct clk *iface;
> + struct clk *axi_m_clk;
> + struct clk *axi_s_clk;
> + struct clk *ahb_clk;
> + struct clk *aux_clk;
> + struct reset_control *rst[7];
> +};
> +
> union qcom_pcie_resources {
> struct qcom_pcie_resources_1_0_0 v1_0_0;
> struct qcom_pcie_resources_2_1_0 v2_1_0;
> struct qcom_pcie_resources_2_3_2 v2_3_2;
> + struct qcom_pcie_resources_2_3_3 v2_3_3;
> struct qcom_pcie_resources_2_4_0 v2_4_0;
> };
>
> @@ -884,6 +920,169 @@ static int qcom_pcie_init_2_4_0(struct qcom_pcie *pcie)
> return ret;
> }
>
> +static int qcom_pcie_get_resources_2_3_3(struct qcom_pcie *pcie)
> +{
> + struct qcom_pcie_resources_2_3_3 *res = &pcie->res.v2_3_3;
> + struct dw_pcie *pci = pcie->pci;
> + struct device *dev = pci->dev;
> + int i;
> + const char *rst_names[] = { "axi_m", "axi_s", "pipe",
> + "axi_m_sticky", "sticky",
> + "ahb", "sleep", };
> +
> + res->iface = devm_clk_get(dev, "iface");
> + if (IS_ERR(res->iface))
> + return PTR_ERR(res->iface);
> +
> + res->axi_m_clk = devm_clk_get(dev, "axi_m");
> + if (IS_ERR(res->axi_m_clk))
> + return PTR_ERR(res->axi_m_clk);
> +
> + res->axi_s_clk = devm_clk_get(dev, "axi_s");
> + if (IS_ERR(res->axi_s_clk))
> + return PTR_ERR(res->axi_s_clk);
> +
> + res->ahb_clk = devm_clk_get(dev, "ahb");
> + if (IS_ERR(res->ahb_clk))
> + return PTR_ERR(res->ahb_clk);
> +
> + res->
Re: [RFC PATCH] media: vb2: add bidirectional flag in vb2_queue
Hi Laurent,
On 08/16/2017 03:28 PM, Laurent Pinchart wrote:
> Hi Stan,
>
> On Wednesday 16 Aug 2017 14:46:50 Stanimir Varbanov wrote:
>> On 08/15/2017 01:04 PM, Hans Verkuil wrote:
>>> On 08/14/17 10:41, Stanimir Varbanov wrote:
>>>> Hi,
>>>>
>>>> This RFC patch is intended to give to the drivers a choice to change
>>>> the default behavior of the v4l2-core DMA mapping direction from
>>>> DMA_TO/FROM_DEVICE (depending on the buffer type CAPTURE or OUTPUT)
>>>> to DMA_BIDIRECTIONAL during queue_init time.
>>>>
>>>> Initially the issue with DMA mapping direction has been found in
>>>> Venus encoder driver where the firmware side of the driver adds few
>>>> lines padding on bottom of the image buffer, and the consequence was
>>>> triggering of IOMMU protection faults.
>>>>
>>>> Probably other drivers could also has a benefit of this feature (hint)
>>>> in the future.
>>>>
>>>> Signed-off-by: Stanimir Varbanov
>>>> ---
>>>>
>>>> drivers/media/v4l2-core/videobuf2-core.c | 3 +++
>>>> include/media/videobuf2-core.h | 11 +++
>>>> 2 files changed, 14 insertions(+)
>>>>
>>>> diff --git a/drivers/media/v4l2-core/videobuf2-core.c
>>>> b/drivers/media/v4l2-core/videobuf2-core.c index
>>>> 14f83cecfa92..17d07fda4cdc 100644
>>>> --- a/drivers/media/v4l2-core/videobuf2-core.c
>>>> +++ b/drivers/media/v4l2-core/videobuf2-core.c
>>>> @@ -200,6 +200,9 @@ static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
>>>>
>>>>int plane;
>>>>int ret = -ENOMEM;
>>>>
>>>> + if (q->bidirectional)
>>>> + dma_dir = DMA_BIDIRECTIONAL;
>>>> +
>>>
>>> Does this only have to be used in mem_alloc? In the __prepare_*() it is
>>> still using DMA_TO/FROM_DEVICE.
>>
>> Yes, it looks like the DMA direction should be covered in the
>> __prepare_* too. Thus the patch should look like below:
>>
>> diff --git a/drivers/media/v4l2-core/videobuf2-core.c
>> b/drivers/media/v4l2-core/videobuf2-core.c
>> index 14f83cecfa92..0089e7dac7dd 100644
>> --- a/drivers/media/v4l2-core/videobuf2-core.c
>> +++ b/drivers/media/v4l2-core/videobuf2-core.c
>> @@ -188,14 +188,21 @@ module_param(debug, int, 0644);
>> static void __vb2_queue_cancel(struct vb2_queue *q);
>> static void __enqueue_in_driver(struct vb2_buffer *vb);
>>
>> +static enum dma_data_direction __get_dma_dir(struct vb2_queue *q)
>> +{
>> +if (q->bidirectional)
>> +return DMA_BIDIRECTIONAL;
>> +
>> +return q->is_output ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
>
> We could also compute the DMA direction once only and store it in the queue.
> I
> have no big preference at the moment.
Yes, I like the idea. I'll cook a regular patch where the DMA direction
will be computed in vb2_core_queue_init().
--
regards,
Stan
Re: [Patch v5 12/12] Documention: v4l: Documentation for HEVC CIDs
Hi, On 07/17/2017 02:18 PM, Smitha T Murthy wrote: > On Fri, 2017-07-07 at 17:59 +0300, Stanimir Varbanov wrote: >> Hi, >> >> On 06/19/2017 08:10 AM, Smitha T Murthy wrote: >>> Added V4l2 controls for HEVC encoder >>> >>> Signed-off-by: Smitha T Murthy >>> --- >>> Documentation/media/uapi/v4l/extended-controls.rst | 364 >>> + >>> 1 file changed, 364 insertions(+) >>> >> >>> +MFC 10.10 MPEG Controls >>> +--- >>> + >>> +The following MPEG class controls deal with MPEG decoding and encoding >>> +settings that are specific to the Multi Format Codec 10.10 device present >>> +in the S5P and Exynos family of SoCs by Samsung. >>> + >>> + >>> +.. _mfc1010-control-id: >>> + >>> +MFC 10.10 Control IDs >>> +^ >>> + >>> +``V4L2_CID_MPEG_MFC10_VIDEO_HEVC_REF_NUMBER_FOR_PFRAMES (integer)`` >>> +Selects number of P reference pictures required for HEVC encoder. >>> +P-Frame can use 1 or 2 frames for reference. >>> + >>> +``V4L2_CID_MPEG_MFC10_VIDEO_HEVC_PREPEND_SPSPPS_TO_IDR (integer)`` >>> +Indicates whether to generate SPS and PPS at every IDR. Setting it to 0 >>> +disables generating SPS and PPS at every IDR. Setting it to one enables >>> +generating SPS and PPS at every IDR. >>> + >> >> I'm not sure those two should be driver specific, have to check does >> venus driver has similar controls. >> > Yes please check and let me know if you have similar controls, I will > move it out. The venus encoder also has such a control so you can move it out of MFC specific controls. Also I think this control should be valid for every codec which supports IDR, i.e. H264, so I think you could drop _HEVC_from the control name. Do you plan to resend the patchset soon so that it could be applied for 4.14? If you haven't time let me know I can help with the generic HEVC part of the patchset. -- regards, Stan
Re: [PATCH] media: venus: fix duplicated code for different branches
Hi Gustavo,
On 08/18/2017 02:12 AM, Gustavo A. R. Silva wrote:
> Refactor code in order to avoid identical code for different branches.
>
> This issue was detected with the help of Coccinelle.
>
> Addresses-Coverity-ID: 1415317
> Signed-off-by: Gustavo A. R. Silva
> ---
> This code was reported by Coverity and it was tested by compilation only.
> Please, verify if this is an actual bug.
Yes looks like copy/paste error, and yes it is a bug.
>
> drivers/media/platform/qcom/venus/helpers.c | 6 +-
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/venus/helpers.c
> b/drivers/media/platform/qcom/venus/helpers.c
> index 5f4434c..8a5c467 100644
> --- a/drivers/media/platform/qcom/venus/helpers.c
> +++ b/drivers/media/platform/qcom/venus/helpers.c
> @@ -240,11 +240,7 @@ static void return_buf_error(struct venus_inst *inst,
> {
> struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx;
>
> - if (vbuf->vb2_buf.type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
> - v4l2_m2m_src_buf_remove_by_buf(m2m_ctx, vbuf);
> - else
> - v4l2_m2m_src_buf_remove_by_buf(m2m_ctx, vbuf);
the correct fix must replace the second v4l2_m2m_src_* with v4l2_m2m_dst_*.
> -
> + v4l2_m2m_src_buf_remove_by_buf(m2m_ctx, vbuf);
> v4l2_m2m_buf_done(vbuf, VB2_BUF_STATE_ERROR);
> }
>
>
--
regards,
Stan
Re: [PATCH v2] firmware: qcom_scm: Fix to allow COMPILE_TEST-ing
Hi Bjorn, On 06/20/2017 12:00 AM, Bjorn Andersson wrote: > On Mon 19 Jun 05:19 PDT 2017, Stanimir Varbanov wrote: > >> Hi Olof, >> >> On 06/19/2017 02:25 PM, Stanimir Varbanov wrote: >>> Hi Olof, >>> >>> On 06/19/2017 08:35 AM, Olof Johansson wrote: >>>> Hi, >>>> >>>> On Thu, Jun 8, 2017 at 8:23 AM, Stanimir Varbanov > [..] >>>> >>>> It probably makes more sense to stub the driver->scm interface than >>>> the internal scm interface if what you're looking for is driver >>>> compile_test coverage. >>> >>> Actually, this is the state of qcom_scm if we don't apply the this >>> patch, and it didn't help. Thinking more on that it looks like that >>> adding COMPILE_TEST in 'config QCOM_SCM' is controversial. >>> >>> Arnd, Andy any ideas how to proceed. If this patch is not get merged >>> (and I/we cannot find better solution) the video driver for qualcomm >>> platforms will be rejected for 4.13. >>> >> >> Currently the dependences are: >> >> VIDEO_QCOM_VENUS selects QCOM_MDT_LOADER >> QCOM_MDT_LOADER selects QCOM_SCM >> >> And I came to this, >> >> diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig >> index 9fca977ef18d..b8657c561eae 100644 >> --- a/drivers/soc/qcom/Kconfig >> +++ b/drivers/soc/qcom/Kconfig >> @@ -12,7 +12,7 @@ config QCOM_GSBI >> >> config QCOM_MDT_LOADER >> tristate >> - select QCOM_SCM >> + depends on QCOM_SCM || COMPILE_TEST > > The problem with this is that QCOM_SCM is not user selectable and you > can't select MDT_LOADER if you make it depend on QCOM_SCM - as this > might not be enabled by the user (if you make it user selectable). > > > The problem that I see changing this is that there's no point in making > QCOM_SCM and QCOM_MDT_LOADER user selectable - they don't serve a > purpose on their own. > > > PS. Don't you depend on scm functionality directly from the venus driver > as well? Yes, qcom video codec driver depends directly on QCOM_SCM. But I'm relying on the fact that QCOM_MDT_LOADER depends on QCOM_SCM, and probably QCOM_MDT_LOADER will need to depends on QCOM_SCM forever. -- regards, Stan
[PATCH v2] media: venus: enable building with COMPILE_TEST
We want all media drivers to build with COMPILE_TEST, as the Coverity instance we use on Kernel works only for x86. Also, our test workflow relies on it, in order to identify git bisect breakages. Signed-off-by: Mauro Carvalho Chehab Signed-off-by: Stanimir Varbanov --- Changes since v1: - select QCOM_MDT_LOADER and QCOM_SCM conditionally. drivers/media/platform/Kconfig | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig index 6027dbd4e04d..b7381a4722e2 100644 --- a/drivers/media/platform/Kconfig +++ b/drivers/media/platform/Kconfig @@ -467,8 +467,9 @@ config VIDEO_TI_VPE_DEBUG config VIDEO_QCOM_VENUS tristate "Qualcomm Venus V4L2 encoder/decoder driver" depends on VIDEO_DEV && VIDEO_V4L2 && HAS_DMA - depends on ARCH_QCOM && IOMMU_DMA - select QCOM_MDT_LOADER + depends on (ARCH_QCOM && IOMMU_DMA) || COMPILE_TEST + select QCOM_MDT_LOADER if (ARM || ARM64) + select QCOM_SCM if (ARM || ARM64) select VIDEOBUF2_DMA_SG select V4L2_MEM2MEM_DEV ---help--- -- 2.7.4
Re: linux-next: build failure after merge of the v4l-dvb tree
Hi Stephen,
On 06/21/2017 04:25 AM, Stephen Rothwell wrote:
> Hi Mauro,
>
> After merging the v4l-dvb tree, today's linux-next build (x86_64
> allmodconfig) failed like this:
>
> ERROR: "qcom_scm_is_available"
> [drivers/media/platform/qcom/venus/venus-core.ko] undefined!
> ERROR: "qcom_scm_pas_shutdown"
> [drivers/media/platform/qcom/venus/venus-core.ko] undefined!
> ERROR: "qcom_scm_set_remote_state"
> [drivers/media/platform/qcom/venus/venus-core.ko] undefined!
>
> Caused by commits
>
> af2c3834c8ca ("[media] media: venus: adding core part and helper functions")
> d96d3f30c0f2 ("[media] media: venus: hfi: add Venus HFI files")
> 703528005e58 ("[media] media: venus: enable building of Venus video driver")
>
> I have merged the 4vl-dvb tree from next-20170620 for today.
>
I've investigated this and the issue comes from this commit:
d4348b43bbcadb6d5faee36024245944b6600d25 firmware: qcom_scm: Fix to
allow COMPILE_TEST-ing
which is from Andy's qcom git tree, I asked Andy to drop this patch
because it was rejected.
I also manually merged v4l-dvb:master and reverted above commit. After
that allmodconfig builds fine and the subject error from venus driver
disappeared.
Sorry for inconvenience.
--
regards,
Stan
Re: [PATCH] media: vb2: unify calling of set_page_dirty_lock
On 10/17/2017 05:19 PM, Nicolas Dufresne wrote: > Le mardi 17 octobre 2017 à 13:14 +0300, Sakari Ailus a écrit : >> On Sun, Oct 15, 2017 at 07:09:24PM -0400, Nicolas Dufresne wrote: >>> Le dimanche 15 octobre 2017 à 23:40 +0300, Sakari Ailus a écrit : >>>> Hi Nicolas, >>>> >>>> On Tue, Oct 10, 2017 at 11:40:10AM -0400, Nicolas Dufresne wrote: >>>>> Le mardi 29 août 2017 à 14:26 +0300, Stanimir Varbanov a écrit : >>>>>> Currently videobuf2-dma-sg checks for dma direction for >>>>>> every single page and videobuf2-dc lacks any dma direction >>>>>> checks and calls set_page_dirty_lock unconditionally. >>>>>> >>>>>> Thus unify and align the invocations of set_page_dirty_lock >>>>>> for videobuf2-dc, videobuf2-sg memory allocators with >>>>>> videobuf2-vmalloc, i.e. the pattern used in vmalloc has been >>>>>> copied to dc and dma-sg. >>>>> >>>>> Just before we go too far in "doing like vmalloc", I would like to >>>>> share this small video that display coherency issues when rendering >>>>> vmalloc backed DMABuf over various KMS/DRM driver. I can reproduce >>>>> this >>>>> easily with Intel and MSM display drivers using UVC or Vivid as >>>>> source. >>>>> >>>>> The following is an HDMI capture of the following GStreamer >>>>> pipeline >>>>> running on Dragonboard 410c. >>>>> >>>>> gst-launch-1.0 -v v4l2src device=/dev/video2 ! video/x- >>>>> raw,format=NV16,width=1280,height=720 ! kmssink >>>>> https://people.collabora.com/~nicolas/vmalloc-issue.mov >>>>> >>>>> Feedback on this issue would be more then welcome. It's not clear >>>>> to me >>>>> who's bug is this (v4l2, drm or iommu). The software is unlikely to >>>>> be >>>>> blamed as this same pipeline works fine with non-vmalloc based >>>>> sources. >>>> >>>> Could you elaborate this a little bit more? Which Intel CPU do you >>>> have >>>> there? >>> >>> I have tested with Skylake and Ivy Bridge and on Dragonboard 410c >>> (Qualcomm APQ8016 SoC) (same visual artefact) >> >> I presume kmssink draws on the display. Which GPU did you use? > > In order, GPU will be Iris Pro 580, Intel® Ivybridge Mobile and an > Adreno (3x ?). Why does it matter ? I'm pretty sure the GPU is not used > on the DB410c for this use case. Nicolas, for me this looks like a problem in v4l2. In the case of vivid the stats overlay (where the coherency issues are observed, and most probably the issue will be observed on the whole image but fortunately it is a static image pattern) are filled by the CPU but I cannot see where the cache is flushed. Also I'm wondering why .finish method is missing for dma-vmalloc mem_ops. To be sure that the problem is in vmalloc v4l2 allocator, could you change the allocator to dma-contig, there is a module param for that called 'allocators'. -- regards, Stan
Re: [PATCH 1/4] dt-binding: gpio: Add Qualcomm SMSM device tree documentation
On 08/27/2015 08:37 PM, Bjorn Andersson wrote:
> This documents a device tree binding for exposing the Qualcomm Shared
> Memory State Machine as a set of gpio- and interrupt-controllers.
>
> Signed-off-by: Bjorn Andersson
> ---
> .../devicetree/bindings/gpio/qcom,smsm.txt | 114
> +
> += EXAMPLE
> +The following example shows the SMEM setup for controlling properties of the
> +wireless processor, defined from the 8974 apps processor's point-of-view. It
> +encompasses one outbound entry and the outgoing interrupt for the wireless
> +processor.
> +
> +smsm {
> + compatible = "qcom,smsm";
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + qcom,ipc-3 = <&apcs 8 19>;
Can we use something more descriptive here, for example
qcom,ipc-rpm = <&apcs 8 19>;
and replace these magic numbers with defines?
> +
> + apps_smsm: apps@0 {
> + reg = <0>;
> +
> + gpio-controller;
> + #gpio-cells = <2>;
> + };
> +
> + wcnss_smsm: wcnss@7 {
> + reg = <7>;
> + interrupts = <0 144 1>;
> +
> + interrupt-controller;
> + #interrupt-cells = <2>;
> + };
> +};
--
regards,
Stan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 1/4] venus: firmware: add routine to reset ARM9
Hi Vikash,
On 07/04/2018 10:06 PM, Vikash Garodia wrote:
> Add routine to reset the ARM9 and brings it out of reset. Also
> abstract the Venus CPU state handling with a new function. This
> is in preparation to add PIL functionality in venus driver.
>
> Signed-off-by: Vikash Garodia
> ---
> drivers/media/platform/qcom/venus/core.h | 1 +
> drivers/media/platform/qcom/venus/firmware.c | 36
>
> drivers/media/platform/qcom/venus/firmware.h | 1 +
> drivers/media/platform/qcom/venus/hfi_venus.c| 13 +++--
> drivers/media/platform/qcom/venus/hfi_venus_io.h | 5
> 5 files changed, 47 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/venus/core.h
> b/drivers/media/platform/qcom/venus/core.h
> index 2f02365..eb5ee66 100644
> --- a/drivers/media/platform/qcom/venus/core.h
> +++ b/drivers/media/platform/qcom/venus/core.h
> @@ -129,6 +129,7 @@ struct venus_core {
> struct device *dev;
> struct device *dev_dec;
> struct device *dev_enc;
> + bool no_tz;
> struct mutex lock;
> struct list_head instances;
> atomic_t insts_count;
> diff --git a/drivers/media/platform/qcom/venus/firmware.c
> b/drivers/media/platform/qcom/venus/firmware.c
> index 521d4b3..3968553d 100644
> --- a/drivers/media/platform/qcom/venus/firmware.c
> +++ b/drivers/media/platform/qcom/venus/firmware.c
> @@ -22,11 +22,47 @@
> #include
> #include
>
> +#include "core.h"
> #include "firmware.h"
> +#include "hfi_venus_io.h"
>
> #define VENUS_PAS_ID 9
> #define VENUS_FW_MEM_SIZE(6 * SZ_1M)
>
> +static void venus_reset_cpu(struct venus_core *core)
> +{
> + void __iomem *base = core->base;
> +
> + writel(0, base + WRAPPER_FW_START_ADDR);
> + writel(VENUS_FW_MEM_SIZE, base + WRAPPER_FW_END_ADDR);
> + writel(0, base + WRAPPER_CPA_START_ADDR);
> + writel(VENUS_FW_MEM_SIZE, base + WRAPPER_CPA_END_ADDR);
> + writel(0x0, base + WRAPPER_CPU_CGC_DIS);
> + writel(0x0, base + WRAPPER_CPU_CLOCK_CONFIG);
> +
> + /* Make sure all register writes are committed. */
> + mb();
> +
> + /* Bring ARM9 out of reset */
> + writel_relaxed(0, base + WRAPPER_A9SS_SW_RESET);
replace writel_relaxed with writel and drop above mb. The writel has wmb
just before writing so I think using writel here is better choice.
> +}
> +
> +int venus_set_hw_state(struct venus_core *core, bool resume)
s/resume/suspend as it is done in the function prototype in firmware.h.
> +{
> + void __iomem *base = core->base;
> +
> + if (!core->no_tz)
> + return qcom_scm_set_remote_state(resume, 0);
> +
> + if (resume)
> + venus_reset_cpu(core);
> + else
> + writel(1, base + WRAPPER_A9SS_SW_RESET);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(venus_set_hw_state);
> +
> int venus_boot(struct device *dev, const char *fwname)
> {
> const struct firmware *mdt;
> diff --git a/drivers/media/platform/qcom/venus/firmware.h
> b/drivers/media/platform/qcom/venus/firmware.h
> index 428efb5..ff2e70e 100644
> --- a/drivers/media/platform/qcom/venus/firmware.h
> +++ b/drivers/media/platform/qcom/venus/firmware.h
> @@ -18,5 +18,6 @@
>
> int venus_boot(struct device *dev, const char *fwname);
> int venus_shutdown(struct device *dev);
> +int venus_set_hw_state(struct venus_core *core, bool suspend);
could you make two inline functions here, call them
venus_set_hw_state_suspend() and venus_set_hw_state_resume() which just
call venus_set_hw_state with the right state.
>
> #endif
> diff --git a/drivers/media/platform/qcom/venus/hfi_venus.c
> b/drivers/media/platform/qcom/venus/hfi_venus.c
> index 9366dae..5b56925 100644
> --- a/drivers/media/platform/qcom/venus/hfi_venus.c
> +++ b/drivers/media/platform/qcom/venus/hfi_venus.c
> @@ -19,7 +19,6 @@
> #include
> #include
> #include
> -#include
> #include
>
> #include "core.h"
> @@ -27,6 +26,7 @@
> #include "hfi_msgs.h"
> #include "hfi_venus.h"
> #include "hfi_venus_io.h"
> +#include "firmware.h"
>
> #define HFI_MASK_QHDR_TX_TYPE0xff00
> #define HFI_MASK_QHDR_RX_TYPE0x00ff
> @@ -55,11 +55,6 @@
> #define IFACEQ_VAR_LARGE_PKT_SIZE512
> #define IFACEQ_VAR_HUGE_PKT_SIZE (1024 * 12)
>
> -enum tzbsp_video_state {
> - TZBSP_VIDEO_STATE_SUSPEND = 0,
> - TZBSP_VIDEO_STATE_RESUME
> -};
> -
> struct hfi_queue_table_header {
> u32 version;
> u32 size;
> @@ -575,7 +570,7 @@ static int venus_power_off(struct venus_hfi_device *hdev)
> if (!hdev->power_enabled)
> return 0;
>
> - ret = qcom_scm_set_remote_state(TZBSP_VIDEO_STATE_SUSPEND, 0);
> + ret = venus_set_hw_state(hdev->core, false);
... and use venus_set_hw_state_suspend()
> if (ret)
> return ret;
>
> @@ -595,7 +590,7 @@ static int venus_power_on(struct venus_hfi_device *hdev)
> if (hdev->power_enabled)
>
Re: [PATCH v3 3/4] venus: firmware: add no TZ boot and shutdown routine
Hi Vikash,
On 07/04/2018 10:06 PM, Vikash Garodia wrote:
> Video hardware is mainly comprised of vcodec subsystem and video
> control subsystem. Video control has ARM9 which executes the video
> firmware instructions whereas vcodec does the video frame processing.
> This change adds support to load the video firmware and bring ARM9
> out of reset for platforms which does not have trustzone.
>
> Signed-off-by: Vikash Garodia
> ---
> drivers/media/platform/qcom/venus/core.c | 6 +-
> drivers/media/platform/qcom/venus/core.h | 5 ++
> drivers/media/platform/qcom/venus/firmware.c | 93
> ++--
> drivers/media/platform/qcom/venus/firmware.h | 2 +-
> 4 files changed, 98 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/venus/core.c
> b/drivers/media/platform/qcom/venus/core.c
> index 75b9785..393994e 100644
> --- a/drivers/media/platform/qcom/venus/core.c
> +++ b/drivers/media/platform/qcom/venus/core.c
> @@ -76,7 +76,7 @@ static void venus_sys_error_handler(struct work_struct
> *work)
> hfi_core_deinit(core, true);
> hfi_destroy(core);
> mutex_lock(&core->lock);
> - venus_shutdown(core->dev);
> + venus_shutdown(core);
>
> pm_runtime_put_sync(core->dev);
>
> @@ -323,7 +323,7 @@ static int venus_probe(struct platform_device *pdev)
> err_core_deinit:
> hfi_core_deinit(core, false);
> err_venus_shutdown:
> - venus_shutdown(dev);
> + venus_shutdown(core);
> err_runtime_disable:
> pm_runtime_set_suspended(dev);
> pm_runtime_disable(dev);
> @@ -344,7 +344,7 @@ static int venus_remove(struct platform_device *pdev)
> WARN_ON(ret);
>
> hfi_destroy(core);
> - venus_shutdown(dev);
> + venus_shutdown(core);
> of_platform_depopulate(dev);
>
> pm_runtime_put_sync(dev);
> diff --git a/drivers/media/platform/qcom/venus/core.h
> b/drivers/media/platform/qcom/venus/core.h
> index eb5ee66..abe4ddf 100644
> --- a/drivers/media/platform/qcom/venus/core.h
> +++ b/drivers/media/platform/qcom/venus/core.h
> @@ -81,6 +81,10 @@ struct venus_caps {
> bool valid; /* used only for Venus v1xx */
> };
>
> +struct video_firmware {
> + struct device *dev;
> + struct iommu_domain *iommu_domain;
> +};
add an empty line here
> /**
> * struct venus_core - holds core parameters valid for all instances
> *
> @@ -129,6 +133,7 @@ struct venus_core {
> struct device *dev;
> struct device *dev_dec;
> struct device *dev_enc;
> + struct video_firmware fw;
> bool no_tz;
> struct mutex lock;
> struct list_head instances;
> diff --git a/drivers/media/platform/qcom/venus/firmware.c
> b/drivers/media/platform/qcom/venus/firmware.c
> index 6d6cca4..2a98d9e 100644
> --- a/drivers/media/platform/qcom/venus/firmware.c
> +++ b/drivers/media/platform/qcom/venus/firmware.c
> @@ -12,8 +12,10 @@
> *
> */
>
> +#include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -27,7 +29,8 @@
> #include "hfi_venus_io.h"
>
> #define VENUS_PAS_ID 9
> -#define VENUS_FW_MEM_SIZE(6 * SZ_1M)
> +#define VENUS_FW_MEM_SIZE(5 * SZ_1M)
This change should be subject to a separate patch.
> +#define VENUS_FW_START_ADDR 0x0
>
> static void venus_reset_cpu(struct venus_core *core)
> {
> @@ -121,6 +124,76 @@ static int venus_load_fw(struct venus_core *core, const
> char *fwname,
> return ret;
> }
>
> +int venus_boot_noTZ(struct venus_core *core, phys_addr_t mem_phys,
> + size_t mem_size)
please use lowercase for function names and correct the size_t mem_size
indentation.
> +{
> + struct iommu_domain *iommu;
s/iommu/iommu_dom ?
> + struct device *dev;
> + int ret;
> +
> + if (!core->fw.dev)
> + return -EPROBE_DEFER;
> +
> + dev = core->fw.dev;
you could make this initialization in declaration and check for !dev
above in the if statement.
> +
> + iommu = iommu_domain_alloc(&platform_bus_type);
> + if (!iommu) {
> + dev_err(dev, "Failed to allocate iommu domain\n");
> + return -ENOMEM;
> + }
> +
> + ret = iommu_attach_device(iommu, dev);
> + if (ret) {
> + dev_err(dev, "could not attach device\n");
> + goto err_attach;
> + }
> +
> + ret = iommu_map(iommu, VENUS_FW_START_ADDR, mem_phys, mem_size,
> + IOMMU_READ|IOMMU_WRITE|IOMMU_PRIV);
spaces between IOMMU_xxx
> + if (ret) {
> + dev_err(dev, "could not map video firmware region\n");
> + goto err_map;
> + }
add blank line here
> + core->fw.iommu_domain = iommu;
> + venus_reset_cpu(core);
> +
> + return 0;
> +
> +err_map:
> + iommu_detach_device(iommu, dev);
> +err_attach:
> + iommu_domain_free(iommu);
> + return ret;
> +}
> +
> +int venus_shutdown_noTZ(struct venus_core
Re: [PATCH v3 2/4] venus: firmware: move load firmware in a separate function
Hi Vikash,
On 07/04/2018 10:06 PM, Vikash Garodia wrote:
> Separate firmware loading part into a new function.
I cannot apply this patch in order to test the series.
>
> Signed-off-by: Vikash Garodia
> ---
> drivers/media/platform/qcom/venus/core.c | 4 +-
> drivers/media/platform/qcom/venus/firmware.c | 58
> +---
> drivers/media/platform/qcom/venus/firmware.h | 2 +-
> 3 files changed, 39 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/venus/core.c
> b/drivers/media/platform/qcom/venus/core.c
> index bb6add9..75b9785 100644
> --- a/drivers/media/platform/qcom/venus/core.c
> +++ b/drivers/media/platform/qcom/venus/core.c
> @@ -84,7 +84,7 @@ static void venus_sys_error_handler(struct work_struct
> *work)
>
> pm_runtime_get_sync(core->dev);
>
> - ret |= venus_boot(core->dev, core->res->fwname);
> + ret |= venus_boot(core);
>
> ret |= hfi_core_resume(core, true);
>
> @@ -284,7 +284,7 @@ static int venus_probe(struct platform_device *pdev)
> if (ret < 0)
> goto err_runtime_disable;
>
> - ret = venus_boot(dev, core->res->fwname);
> + ret = venus_boot(core);
> if (ret)
> goto err_runtime_disable;
>
> diff --git a/drivers/media/platform/qcom/venus/firmware.c
> b/drivers/media/platform/qcom/venus/firmware.c
> index 3968553d..6d6cca4 100644
> --- a/drivers/media/platform/qcom/venus/firmware.c
> +++ b/drivers/media/platform/qcom/venus/firmware.c
> @@ -63,40 +63,37 @@ int venus_set_hw_state(struct venus_core *core, bool
> resume)
> }
> EXPORT_SYMBOL_GPL(venus_set_hw_state);
>
> -int venus_boot(struct device *dev, const char *fwname)
> +static int venus_load_fw(struct venus_core *core, const char *fwname,
> + phys_addr_t *mem_phys, size_t *mem_size)
the next function argument on new line should be just below open brace
> {
> const struct firmware *mdt;
> struct device_node *node;
> - phys_addr_t mem_phys;
> + struct device *dev;
> struct resource r;
> ssize_t fw_size;
> - size_t mem_size;
> void *mem_va;
> int ret;
>
> - if (!IS_ENABLED(CONFIG_QCOM_MDT_LOADER) || !qcom_scm_is_available())
> - return -EPROBE_DEFER;
> -
> + dev = core->dev;
> node = of_parse_phandle(dev->of_node, "memory-region", 0);
> if (!node) {
> dev_err(dev, "no memory-region specified\n");
> return -EINVAL;
> }
> -
please don't delete above blank line
> ret = of_address_to_resource(node, 0, &r);
> if (ret)
> return ret;
>
> - mem_phys = r.start;
> - mem_size = resource_size(&r);
> + *mem_phys = r.start;
> + *mem_size = resource_size(&r);
>
> - if (mem_size < VENUS_FW_MEM_SIZE)
> + if (*mem_size < VENUS_FW_MEM_SIZE)
> return -EINVAL;
>
> - mem_va = memremap(r.start, mem_size, MEMREMAP_WC);
> + mem_va = memremap(r.start, *mem_size, MEMREMAP_WC);
> if (!mem_va) {
> dev_err(dev, "unable to map memory region: %pa+%zx\n",
> - &r.start, mem_size);
> + &r.start, *mem_size);
> return -ENOMEM;
> }
>
> @@ -110,24 +107,41 @@ int venus_boot(struct device *dev, const char *fwname)
> release_firmware(mdt);
> goto err_unmap;
> }
> -
> - ret = qcom_mdt_load(dev, mdt, fwname, VENUS_PAS_ID, mem_va, mem_phys,
> - mem_size);
> + if (core->no_tz)
> + ret = qcom_mdt_load_no_init(dev, mdt, fwname, VENUS_PAS_ID,
> + mem_va, *mem_phys, *mem_size, NULL);
indentation is wrong see the original code
> + else
> + ret = qcom_mdt_load(dev, mdt, fwname, VENUS_PAS_ID, mem_va,
> + *mem_phys, *mem_size);
indentation again
>
> release_firmware(mdt);
>
> - if (ret)
> - goto err_unmap;
> -
> - ret = qcom_scm_pas_auth_and_reset(VENUS_PAS_ID);
> - if (ret)
> - goto err_unmap;
> -
> err_unmap:
> memunmap(mem_va);
> return ret;
> }
>
> +int venus_boot(struct venus_core *core)
> +{
> + phys_addr_t mem_phys;
> + size_t mem_size;
> + int ret;
> + struct device *dev;
please order declarations from longer to shorter.
> +
> + if (!IS_ENABLED(CONFIG_QCOM_MDT_LOADER))
> + return -EPROBE_DEFER;
the original venus_boot was checking for || !qcom_scm_is_available() why
is that removed? It was done on purpose.
> +
> + dev = core->dev;
this could be done when dev is declared.
> +
> + ret = venus_load_fw(core, core->res->fwname, &mem_phys, &mem_size);
> + if (ret) {
> + dev_err(dev, "fail to load video firmware\n");
> + return -EINVAL;
> + }
> +
> + return qcom_scm_pas_auth_and_reset(VENUS_PAS_ID);
> +}
> +
> int venus_shutdown(struct device *dev)
> {
> return qcom_
Re: [PATCH v3 1/4] venus: firmware: add routine to reset ARM9
Hi,
On 07/04/2018 10:06 PM, Vikash Garodia wrote:
> Add routine to reset the ARM9 and brings it out of reset. Also
> abstract the Venus CPU state handling with a new function. This
> is in preparation to add PIL functionality in venus driver.
>
> Signed-off-by: Vikash Garodia
> ---
> drivers/media/platform/qcom/venus/core.h | 1 +
> drivers/media/platform/qcom/venus/firmware.c | 36
>
> drivers/media/platform/qcom/venus/firmware.h | 1 +
> drivers/media/platform/qcom/venus/hfi_venus.c| 13 +++--
> drivers/media/platform/qcom/venus/hfi_venus_io.h | 5
> 5 files changed, 47 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/venus/core.h
> b/drivers/media/platform/qcom/venus/core.h
> index 2f02365..eb5ee66 100644
> --- a/drivers/media/platform/qcom/venus/core.h
> +++ b/drivers/media/platform/qcom/venus/core.h
> @@ -129,6 +129,7 @@ struct venus_core {
> struct device *dev;
> struct device *dev_dec;
> struct device *dev_enc;
> + bool no_tz;
> struct mutex lock;
> struct list_head instances;
> atomic_t insts_count;
> diff --git a/drivers/media/platform/qcom/venus/firmware.c
> b/drivers/media/platform/qcom/venus/firmware.c
> index 521d4b3..3968553d 100644
> --- a/drivers/media/platform/qcom/venus/firmware.c
> +++ b/drivers/media/platform/qcom/venus/firmware.c
> @@ -22,11 +22,47 @@
> #include
> #include
>
> +#include "core.h"
> #include "firmware.h"
> +#include "hfi_venus_io.h"
>
> #define VENUS_PAS_ID 9
> #define VENUS_FW_MEM_SIZE(6 * SZ_1M)
>
> +static void venus_reset_cpu(struct venus_core *core)
> +{
> + void __iomem *base = core->base;
> +
> + writel(0, base + WRAPPER_FW_START_ADDR);
> + writel(VENUS_FW_MEM_SIZE, base + WRAPPER_FW_END_ADDR);
> + writel(0, base + WRAPPER_CPA_START_ADDR);
> + writel(VENUS_FW_MEM_SIZE, base + WRAPPER_CPA_END_ADDR);
> + writel(0x0, base + WRAPPER_CPU_CGC_DIS);
> + writel(0x0, base + WRAPPER_CPU_CLOCK_CONFIG);
> +
> + /* Make sure all register writes are committed. */
> + mb();
> +
> + /* Bring ARM9 out of reset */
> + writel_relaxed(0, base + WRAPPER_A9SS_SW_RESET);
> +}
> +
> +int venus_set_hw_state(struct venus_core *core, bool resume)
> +{
> + void __iomem *base = core->base;
> +
> + if (!core->no_tz)
> + return qcom_scm_set_remote_state(resume, 0);
> +
> + if (resume)
> + venus_reset_cpu(core);
> + else
> + writel(1, base + WRAPPER_A9SS_SW_RESET);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(venus_set_hw_state);
This export_symbol shouldn't be needed
--
regards,
Stan
Re: [PATCH] PCI: qcom: add missing supplies required for msm8996
Hey Srini, As there are no comments I'd propose to change the endpoint supplies to more generic names. On 12/08/2017 11:20 AM, srinivas.kandaga...@linaro.org wrote: > From: Srinivas Kandagatla > > This patch adds supplies that are required for msm8996. Two of them vdda > and vdda-1p8 are analog supplies that go in to controller, and the rest > of the two vddpe's are supplies to PCIe endpoints. > > Without these supplies PCIe endpoints which require power supplies are > not enumerated at all, as there is no one to power it up. > > Signed-off-by: Srinivas Kandagatla > --- > .../devicetree/bindings/pci/qcom,pcie.txt | 16 + > drivers/pci/dwc/pcie-qcom.c| 28 > -- > 2 files changed, 42 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt > b/Documentation/devicetree/bindings/pci/qcom,pcie.txt > index 3c9d321b3d3b..045102cb3e12 100644 > --- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt > +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt > @@ -179,6 +179,11 @@ > Value type: > Definition: A phandle to the core analog power supply > > +- vdda-1p8-supply: > + Usage: required for msm8996 > + Value type: > + Definition: A phandle to the 1.8v analog power supply > + This should be dropped, because it is part of the phy. > - vdda_phy-supply: > Usage: required for ipq/apq8064 > Value type: > @@ -189,6 +194,15 @@ > Value type: > Definition: A phandle to the analog power supply for IC which generates > reference clock > +- vddpe-supply: > + Usage: optional > + Value type: > + Definition: A phandle to the PCIe endpoint power supply vddpe_3v3-supply > + > +- vddpe1-supply: > + Usage: optional > + Value type: > + Definition: A phandle to the PCIe endpoint power supply 1 vddpe_1v5-supply > > - phys: > Usage: required for apq8084 > @@ -205,6 +219,8 @@ > Value type: > Definition: List of phandle and GPIO specifier pairs. Should contain > - "perst-gpios" PCIe endpoint reset signal line > + - "pe_en-gpios" PCIe endpoint enable signal line > + - "pe_en1-gpios" PCIe endpoint enable1 signal line We don't need those gpios, the regulator driver will manipulate these gpios when we call regulator_enable/disable. -- regards, Stan
Re: [PATCH] PCI: qcom: add missing supplies required for msm8996
Hi, On 01/23/2018 11:46 AM, Srinivas Kandagatla wrote: > > > On 23/01/18 09:23, Stanimir Varbanov wrote: >> Hey Srini, >> >> As there are no comments I'd propose to change the endpoint supplies to >> more generic names. >> > Sure, I will respin this with your suggestions, except the 3v3 and 1v5 > suffix due to the reasons below: >>> +- vdda-1p8-supply: >>> + Usage: required for msm8996 >>> + Value type: >>> + Definition: A phandle to the 1.8v analog power supply >>> + >> >> This should be dropped, because it is part of the phy. > Yep. > >> >>> - vdda_phy-supply: >>> Usage: required for ipq/apq8064 >>> Value type: >>> @@ -189,6 +194,15 @@ >>> Value type: >>> Definition: A phandle to the analog power supply for IC which >>> generates >>> reference clock >>> +- vddpe-supply: >>> + Usage: optional >>> + Value type: >>> + Definition: A phandle to the PCIe endpoint power supply >> >> vddpe_3v3-supply > Why do we need suffix here? AFAIU, It does not add any value, instead it > would confuse the users. vddpe and vddpe1 is already confusing as well. Lets imagine that powering up the endpointX needs some specific sequence between 3v3 and 1v5 and endpointY (which could be connected on the same PCIe lane) has different power sequence, how we would handle that in the qcom pcie host driver? > > These are power supplies for endpoint which could be of any voltage. In I don't think that could be any values see PCIe mini card electromechanical specification. There on the connector are provided 3v3 and 1v5. > this case both endpoint supplies are 3v3, these could be 1.8 or 5v or > 12v in some other cases. If we see hw designs with 5v and 12v we could extend the binding and the driver with support for them. I want to be exact in the names and voltages in the driver and bindings. -- regards, Stan
Re: [PATCH v2] venus: vdec: fix format enumeration
Hi Hans, Could you take this patch it not too late. On 20.03.2018 15:42, Stanimir Varbanov wrote: Hi Alex, Thanks! On 03/19/2018 11:32 AM, Alexandre Courbot wrote: find_format_by_index() stops enumerating formats as soon as the index matches, and returns NULL if venus_helper_check_codec() finds out that the format is not supported. This prevents formats to be properly enumerated if a non-supported format is present, as the enumeration will end with it. Fix this by moving the call to venus_helper_check_codec() into the loop, and keep enumerating when it fails. Fixes: 29f0133ec6 media: venus: use helper function to check supported codecs Signed-off-by: Alexandre Courbot --- drivers/media/platform/qcom/venus/vdec.c | 13 +++-- drivers/media/platform/qcom/venus/venc.c | 13 +++-- 2 files changed, 14 insertions(+), 12 deletions(-) Acked-by: Stanimir Varbanov regards, Stan
Re: [PATCH v3 2/2] drivers: soc: Add LLCC driver
Hi, On 4.04.2018 20:30, risha...@codeaurora.org wrote: Hi Stanimir, We incorporated all your comments except the following: 1. Removing the driver that maintains the SCT (system cache table) per chipset. As responded earlier the data is expected to change from chipset to chipset and would clutter the main driver if we choose using compatible string. We think its good to keep the data separate from core driver. 2. Changing struct llcc_slice_desc to llcc_desc: The descriptor is specific to each slice and not the whole llcc. All the properties such as id and size are specific to each slice and not whole llcc. please let me know if you are ok with the approach. Based on that I can send my next revision of changes. Yes, please go ahead and send the next version. regards, Stan
Re: [PATCH] pcie: qcom: Add support to enable pcie refclk
Hi Srini, On 03/15/2018 04:41 PM, srinivas.kandaga...@linaro.org wrote: > From: Srinivas Kandagatla > > This patch adds support to enable 100MHz pcie refclk, > On some boards like DB600c this clock is not enabled by default. > > Signed-off-by: Srinivas Kandagatla > --- > Documentation/devicetree/bindings/pci/qcom,pcie.txt | 1 + > drivers/pci/dwc/pcie-qcom.c | 21 > - > 2 files changed, 21 insertions(+), 1 deletion(-) Acked-by: Stanimir Varbanov -- regards, Stan
Re: [PATCH v2] venus: vdec: fix format enumeration
Hi Alex, Thanks! On 03/19/2018 11:32 AM, Alexandre Courbot wrote: > find_format_by_index() stops enumerating formats as soon as the index > matches, and returns NULL if venus_helper_check_codec() finds out that > the format is not supported. This prevents formats to be properly > enumerated if a non-supported format is present, as the enumeration will > end with it. > > Fix this by moving the call to venus_helper_check_codec() into the loop, > and keep enumerating when it fails. > > Fixes: 29f0133ec6 media: venus: use helper function to check supported codecs > > Signed-off-by: Alexandre Courbot > --- > drivers/media/platform/qcom/venus/vdec.c | 13 +++-- > drivers/media/platform/qcom/venus/venc.c | 13 +++-- > 2 files changed, 14 insertions(+), 12 deletions(-) Acked-by: Stanimir Varbanov -- regards, Stan
Re: [PATCH v3] media: venus: add support for key frame
Hi Hans,
On 11/29/18 9:40 PM, Tomasz Figa wrote:
> On Thu, Nov 29, 2018 at 3:10 AM wrote:
>>
>>
>> Hi Stan,
>>
>> On 2018-11-29 16:01, Stanimir Varbanov wrote:
>>> Hi Tomasz,
>>>
>>> On 11/3/18 5:01 AM, Tomasz Figa wrote:
>>>> Hi Malathi,
>>>>
>>>> On Fri, Nov 2, 2018 at 9:58 PM Malathi Gottam
>>>> wrote:
>>>>>
>>>>> When client requests for a keyframe, set the property
>>>>> to hardware to generate the sync frame.
>>>>>
>>>>> Signed-off-by: Malathi Gottam
>>>>> ---
>>>>> drivers/media/platform/qcom/venus/venc_ctrls.c | 20
>>>>> +++-
>>>>> 1 file changed, 19 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/media/platform/qcom/venus/venc_ctrls.c
>>>>> b/drivers/media/platform/qcom/venus/venc_ctrls.c
>>>>> index 45910172..59fe7fc 100644
>>>>> --- a/drivers/media/platform/qcom/venus/venc_ctrls.c
>>>>> +++ b/drivers/media/platform/qcom/venus/venc_ctrls.c
>>>>> @@ -79,8 +79,10 @@ static int venc_op_s_ctrl(struct v4l2_ctrl *ctrl)
>>>>> {
>>>>> struct venus_inst *inst = ctrl_to_inst(ctrl);
>>>>> struct venc_controls *ctr = &inst->controls.enc;
>>>>> + struct hfi_enable en = { .enable = 1 };
>>>>> u32 bframes;
>>>>> int ret;
>>>>> + u32 ptype;
>>>>>
>>>>> switch (ctrl->id) {
>>>>> case V4L2_CID_MPEG_VIDEO_BITRATE_MODE:
>>>>> @@ -173,6 +175,19 @@ static int venc_op_s_ctrl(struct v4l2_ctrl
>>>>> *ctrl)
>>>>>
>>>>> ctr->num_b_frames = bframes;
>>>>> break;
>>>>> + case V4L2_CID_MPEG_VIDEO_FORCE_KEY_FRAME:
>>>>> + mutex_lock(&inst->lock);
>>>>> + if (inst->streamon_out && inst->streamon_cap) {
>>>>
>>>> We had a discussion on this in v2. I don't remember seeing any
>>>> conclusion.
>>>>
>>>> Obviously the hardware should generate a keyframe naturally when the
>>>> CAPTURE streaming starts, which is where the encoding starts, but the
>>>> state of the OUTPUT queue should not affect this.
>>>>
>>>> The application is free to stop and start streaming on the OUTPUT
>>>> queue as it goes and it shouldn't imply any side effects in the
>>>> encoded bitstream (e.g. a keyframe inserted). So:
>>>> - a sequence of STREAMOFF(OUTPUT),
>>>> S_CTRL(V4L2_CID_MPEG_VIDEO_FORCE_KEY_FRAME), STREAMON(OUTPUT) should
>>>> explicitly generate a keyframe,
>>>
>>> I agree with you, but presently we don't follow strictly the stateful
>>> encoder spec. In this spirit I think proposed patch is applicable to
>>> the
>>> current state of the encoder driver, and your comment should be
>>> addressed in the follow-up patches where we have to re-factor a bit
>>> start/stop_streaming according to the encoder documentation.
>>>
>>> But until then we have to get that patch.
>>
>> So I can see that this patch is good implementation of forcing sync
>> frame
>> under current encoder state.
>>
>> Can you please ack the same.
>
> Okay, assuming that when you start streaming you naturally get a
> keyframe, I'm okay with this patch, since it actually fixes the
> missing key frame request, so from the general encoder interface point
> of view:
>
> Acked-by: Tomasz Figa
Thanks Tomasz!
Acked-by: Stanimir Varbanov
--
regards,
Stan
Re: [PATCH 1/1] media: venus: core: Set dma maximum segment size
Hi Vivek, Thanks for the patch! On 12/5/18 10:31 AM, Vivek Gautam wrote: > Turning on CONFIG_DMA_API_DEBUG_SG results in the following error: > > [ 460.308650] [ cut here ] > [ 460.313490] qcom-venus aa0.video-codec: DMA-API: mapping sg segment > longer than device claims to support [len=4194304] [max=65536] > [ 460.326017] WARNING: CPU: 3 PID: 3555 at src/kernel/dma/debug.c:1301 > debug_dma_map_sg+0x174/0x254 > [ 460.33] Modules linked in: venus_dec venus_enc videobuf2_dma_sg > videobuf2_memops hci_uart btqca bluetooth venus_core v4l2_mem2mem > videobuf2_v4l2 videobuf2_common ath10k_snoc ath10k_core ath lzo lzo_compress > zramjoydev > [ 460.375811] CPU: 3 PID: 3555 Comm: V4L2DecoderThre Tainted: GW > 4.19.1 #82 > [ 460.384223] Hardware name: Google Cheza (rev1) (DT) > [ 460.389251] pstate: 6049 (nZCv daif +PAN -UAO) > [ 460.394191] pc : debug_dma_map_sg+0x174/0x254 > [ 460.398680] lr : debug_dma_map_sg+0x174/0x254 > [ 460.403162] sp : ff80200c37d0 > [ 460.406583] x29: ff80200c3830 x28: 0001 > [ 460.412056] x27: x26: ffc0f785ea80 > [ 460.417532] x25: x24: ffc0f4ea1290 > [ 460.423001] x23: ffc09e700300 x22: ffc0f4ea1290 > [ 460.428470] x21: ff8009037000 x20: 0001 > [ 460.433936] x19: ff80091b x18: > [ 460.439411] x17: x16: f251 > [ 460.444885] x15: 0006 x14: 0720072007200720 > [ 460.450354] x13: ff800af536e0 x12: > [ 460.455822] x11: x10: > [ 460.461288] x9 : 537944d9c6c48d00 x8 : 537944d9c6c48d00 > [ 460.466758] x7 : x6 : ffc0f8d98f80 > [ 460.472230] x5 : x4 : > [ 460.477703] x3 : 008a x2 : ffc0fdb13948 > [ 460.483170] x1 : ffc0fdb0b0b0 x0 : 007a > [ 460.488640] Call trace: > [ 460.491165] debug_dma_map_sg+0x174/0x254 > [ 460.495307] vb2_dma_sg_alloc+0x260/0x2dc [videobuf2_dma_sg] > [ 460.501150] __vb2_queue_alloc+0x164/0x374 [videobuf2_common] > [ 460.507076] vb2_core_reqbufs+0xfc/0x23c [videobuf2_common] > [ 460.512815] vb2_reqbufs+0x44/0x5c [videobuf2_v4l2] > [ 460.517853] v4l2_m2m_reqbufs+0x44/0x78 [v4l2_mem2mem] > [ 460.523144] v4l2_m2m_ioctl_reqbufs+0x1c/0x28 [v4l2_mem2mem] > [ 460.528976] v4l_reqbufs+0x30/0x40 > [ 460.532480] __video_do_ioctl+0x36c/0x454 > [ 460.536610] video_usercopy+0x25c/0x51c > [ 460.540572] video_ioctl2+0x38/0x48 > [ 460.544176] v4l2_ioctl+0x60/0x74 > [ 460.547602] do_video_ioctl+0x948/0x3520 > [ 460.551648] v4l2_compat_ioctl32+0x60/0x98 > [ 460.555872] __arm64_compat_sys_ioctl+0x134/0x20c > [ 460.560718] el0_svc_common+0x9c/0xe4 > [ 460.564498] el0_svc_compat_handler+0x2c/0x38 > [ 460.568982] el0_svc_compat+0x8/0x18 > [ 460.572672] ---[ end trace ce209b87b2f3af88 ]--- > > From above warning one would deduce that the sg segment will overflow > the device's capacity. In reality, the hardware can accommodate larger > sg segments. > So, initialize the max segment size properly to weed out this warning. > > Based on a similar patch sent by Sean Paul for mdss: > https://patchwork.kernel.org/patch/10671457/ > > Signed-off-by: Vivek Gautam > --- > drivers/media/platform/qcom/venus/core.c | 8 > 1 file changed, 8 insertions(+) Acked-by: Stanimir Varbanov -- regards, Stan
[PATCH v4 10/27] venus: hfi_venus: add suspend functionality for Venus 4xx
This adds suspend (power collapse) functionality by reusing
the suspend function for Venus 3xx and also enables idle indicator
property for Venus 4xx (where it is disabled by default).
Signed-off-by: Stanimir Varbanov
Reviewed-by: Tomasz Figa
---
drivers/media/platform/qcom/venus/hfi_venus.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/qcom/venus/hfi_venus.c
b/drivers/media/platform/qcom/venus/hfi_venus.c
index 7a83e967a8ea..9366dae16b0a 100644
--- a/drivers/media/platform/qcom/venus/hfi_venus.c
+++ b/drivers/media/platform/qcom/venus/hfi_venus.c
@@ -879,6 +879,14 @@ static int venus_sys_set_default_properties(struct
venus_hfi_device *hdev)
if (ret)
dev_warn(dev, "setting fw debug msg ON failed (%d)\n", ret);
+ /*
+* Idle indicator is disabled by default on some 4xx firmware versions,
+* enable it explicitly in order to make suspend functional by checking
+* WFI (wait-for-interrupt) bit.
+*/
+ if (IS_V4(hdev->core))
+ venus_sys_idle_indicator = true;
+
ret = venus_sys_set_idle_message(hdev, venus_sys_idle_indicator);
if (ret)
dev_warn(dev, "setting idle response ON failed (%d)\n", ret);
@@ -1533,7 +1541,8 @@ static int venus_suspend_3xx(struct venus_core *core)
static int venus_suspend(struct venus_core *core)
{
- if (core->res->hfi_version == HFI_VERSION_3XX)
+ if (core->res->hfi_version == HFI_VERSION_3XX ||
+ core->res->hfi_version == HFI_VERSION_4XX)
return venus_suspend_3xx(core);
return venus_suspend_1xx(core);
--
2.14.1
Re: [PATCH] PCI: qcom: Fix error handling in pm_runtime support
Hi Bjorn, On 05/25/2018 10:09 PM, Bjorn Andersson wrote: > The driver does not cope with the fact that probe can fail in a number > of cases after enabling pm_runtime on the device, this results in > warnings about "Unbalanced pm_runtime_enable". Further more if probe > fails after invoking host_init the power-domain will be left referenced. > > As it's not possible for the error handling in qcom_pcie_host_init() to > handle errors happening after returning from that function the > pm_runtime_get_sync() is moved to probe() as well. > > Signed-off-by: Bjorn Andersson Please add: Fixes: 854b69efbdd2903991506ac5d5624d2cfb5b4e2f PCI: qcom: add runtime pm support to pcie_port > --- > > I'm sorry for not spotting this last week when we discussed the previous > patch. > > drivers/pci/dwc/pcie-qcom.c | 65 ++--- the path has been changed to drivers/pci/controller/dwc/pcie-qcom.c > 1 file changed, 46 insertions(+), 19 deletions(-) > > diff --git a/drivers/pci/dwc/pcie-qcom.c b/drivers/pci/dwc/pcie-qcom.c > index 3f35098b71b1..f2453196278f 100644 > --- a/drivers/pci/dwc/pcie-qcom.c > +++ b/drivers/pci/dwc/pcie-qcom.c > @@ -1088,8 +1088,6 @@ static int qcom_pcie_host_init(struct pcie_port *pp) > struct qcom_pcie *pcie = to_qcom_pcie(pci); > int ret; > > - pm_runtime_get_sync(pci->dev); > - > qcom_ep_reset_assert(pcie); > > ret = pcie->ops->init(pcie); > @@ -1126,7 +1124,6 @@ static int qcom_pcie_host_init(struct pcie_port *pp) > phy_power_off(pcie->phy); > err_deinit: > pcie->ops->deinit(pcie); > - pm_runtime_put_sync(pci->dev); > > return ret; > } > @@ -1207,7 +1204,6 @@ static int qcom_pcie_probe(struct platform_device *pdev) > struct qcom_pcie *pcie; > int ret; > > - > pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL); > if (!pcie) > return -ENOMEM; > @@ -1217,6 +1213,7 @@ static int qcom_pcie_probe(struct platform_device *pdev) > return -ENOMEM; > > pm_runtime_enable(dev); > + pm_runtime_get_sync(dev); pm_runtime_get_sync could fail, please check for errors. With those changes addressed: Acked-by: Stanimir Varbanov -- regards, Stan
Re: [PATCH v2 11/29] venus: venc,vdec: adds clocks needed for venus 4xx
Hi Tomasz,
On 05/24/2018 09:11 AM, Tomasz Figa wrote:
> Hi Stanimir,
>
> On Tue, May 15, 2018 at 5:10 PM Stanimir Varbanov <
> stanimir.varba...@linaro.org> wrote:
>
>> This extends the clocks number to support suspend and resume
>> on Venus version 4xx.
>
>> Signed-off-by: Stanimir Varbanov
>> ---
>> drivers/media/platform/qcom/venus/core.h | 4 +--
>> drivers/media/platform/qcom/venus/vdec.c | 42
> ++--
>> drivers/media/platform/qcom/venus/venc.c | 42
> ++--
>> 3 files changed, 72 insertions(+), 16 deletions(-)
>
>> diff --git a/drivers/media/platform/qcom/venus/core.h
> b/drivers/media/platform/qcom/venus/core.h
>> index 8d3e150800c9..b5b9a84e9155 100644
>> --- a/drivers/media/platform/qcom/venus/core.h
>> +++ b/drivers/media/platform/qcom/venus/core.h
>> @@ -92,8 +92,8 @@ struct venus_core {
>> void __iomem *base;
>> int irq;
>> struct clk *clks[VIDC_CLKS_NUM_MAX];
>> - struct clk *core0_clk;
>> - struct clk *core1_clk;
>> + struct clk *core0_clk, *core0_bus_clk;
>> + struct clk *core1_clk, *core1_bus_clk;
>> struct video_device *vdev_dec;
>> struct video_device *vdev_enc;
>> struct v4l2_device v4l2_dev;
>> diff --git a/drivers/media/platform/qcom/venus/vdec.c
> b/drivers/media/platform/qcom/venus/vdec.c
>> index 261a51adeef2..c45452634e7e 100644
>> --- a/drivers/media/platform/qcom/venus/vdec.c
>> +++ b/drivers/media/platform/qcom/venus/vdec.c
>> @@ -1081,12 +1081,18 @@ static int vdec_probe(struct platform_device
> *pdev)
>> if (!core)
>> return -EPROBE_DEFER;
>
>> - if (core->res->hfi_version == HFI_VERSION_3XX) {
>> + if (IS_V3(core) || IS_V4(core)) {
>> core->core0_clk = devm_clk_get(dev, "core");
>> if (IS_ERR(core->core0_clk))
>> return PTR_ERR(core->core0_clk);
>> }
>
>> + if (IS_V4(core)) {
>> + core->core0_bus_clk = devm_clk_get(dev, "bus");
>> + if (IS_ERR(core->core0_bus_clk))
>> + return PTR_ERR(core->core0_bus_clk);
>> + }
>> +
>
> Rather than doing this conditional dance, wouldn't it make more sense to
> just list all the clocks in variant data struct and use clk_bulk_get()?
Do you mean the same as it is done for venus/core.c ?
>
>> platform_set_drvdata(pdev, core);
>
>> vdev = video_device_alloc();
>> @@ -1132,12 +1138,23 @@ static __maybe_unused int
> vdec_runtime_suspend(struct device *dev)
>> {
>> struct venus_core *core = dev_get_drvdata(dev);
>
>> - if (core->res->hfi_version == HFI_VERSION_1XX)
>> + if (IS_V1(core))
>> return 0;
>
>> - writel(0, core->base + WRAPPER_VDEC_VCODEC_POWER_CONTROL);
>> + if (IS_V3(core))
>> + writel(0, core->base + WRAPPER_VDEC_VCODEC_POWER_CONTROL);
>> + else if (IS_V4(core))
>> + writel(0, core->base +
> WRAPPER_VCODEC0_MMCC_POWER_CONTROL);
>> +
>> + if (IS_V4(core))
>> + clk_disable_unprepare(core->core0_bus_clk);
>> +
>> clk_disable_unprepare(core->core0_clk);
>> - writel(1, core->base + WRAPPER_VDEC_VCODEC_POWER_CONTROL);
>> +
>> + if (IS_V3(core))
>> + writel(1, core->base + WRAPPER_VDEC_VCODEC_POWER_CONTROL);
>> + else if (IS_V4(core))
>> + writel(1, core->base +
> WRAPPER_VCODEC0_MMCC_POWER_CONTROL);
>
> Almost every step here differs between version. I'd suggest splitting this
> into separate functions for both versions.
I think it will be better to squash this patch with 13/29.
--
regards,
Stan
Re: [PATCH 01/28] venus: hfi_msgs: correct pointer increment
Hi Tomasz, Thanks for the review! On 05/18/2018 11:33 AM, Tomasz Figa wrote: > Hi Stanimir, > > Thanks for the series. I'll be gradually reviewing subsequent patches. Stay > tuned. :) > Please consider that there is a v2 of this patchset. :) > > Reviewed-by: Tomasz Figa > Thanks! -- regards, Stan
[PATCH v3 27/29] venus: implementing multi-stream support
This is implementing multi-stream decoder support. The multi-stream
will be used to enable/disable the primary/secondary decoder
outputs. Depending on formats on both decoder outputs we could
implement downscale, dithering and supporting UBWC (universal
bandwidth compression) formats. The UBWC compressed raw format is
used to optimize interconnect bandwidth for bigger resolutions
like 4K and hence we will get some power-saving benefits as well.
Both decoder outputs are distinguished by buffer_type field in
the HFI packets. For example HFI_BUFFER_OUTPUT is the buffer type
for primary decoder output and HFI_BUFFER_OUTPUT2 is for secondary
decoder output.
Starting from Venus 4xx the DPB buffers format must be UBWC, so
the multi-stream becomes mandatory for this Venus version. That
means that we need to allocate internally in the driver a set of
DPB buffers (with UBWC NV12 format) and give them to the firmware.
The other decoder output (we called it OPB) format will be NV12
linear format and with the same resolution (or smaller in case
the user wants to downscale).
Signed-off-by: Stanimir Varbanov
---
Hi Hans, I have updated patch description a bit. Is it now clearer?
If you have some particular questions I could try to answer and
extend the description.
Also, as Tomasz Figa started to review the patchset I'll postpone
patchset v3 a bit.
drivers/media/platform/qcom/venus/core.h| 2 +
drivers/media/platform/qcom/venus/helpers.c | 204 +++-
drivers/media/platform/qcom/venus/helpers.h | 6 +
drivers/media/platform/qcom/venus/vdec.c| 93 -
drivers/media/platform/qcom/venus/venc.c| 1 +
5 files changed, 302 insertions(+), 4 deletions(-)
diff --git a/drivers/media/platform/qcom/venus/core.h
b/drivers/media/platform/qcom/venus/core.h
index 1cac8dc79cd1..afa4e49e0e06 100644
--- a/drivers/media/platform/qcom/venus/core.h
+++ b/drivers/media/platform/qcom/venus/core.h
@@ -210,6 +210,7 @@ struct venus_buffer {
* @list: used for attach an instance to the core
* @lock: instance lock
* @core: a reference to the core struct
+ * @dpbbufs: a list of decoded picture buffers
* @internalbufs: a list of internal bufferes
* @registeredbufs:a list of registered capture bufferes
* @delayed_processa list of delayed buffers
@@ -259,6 +260,7 @@ struct venus_inst {
struct list_head list;
struct mutex lock;
struct venus_core *core;
+ struct list_head dpbbufs;
struct list_head internalbufs;
struct list_head registeredbufs;
struct list_head delayed_process;
diff --git a/drivers/media/platform/qcom/venus/helpers.c
b/drivers/media/platform/qcom/venus/helpers.c
index ed569705ecac..87dcf9973e6f 100644
--- a/drivers/media/platform/qcom/venus/helpers.c
+++ b/drivers/media/platform/qcom/venus/helpers.c
@@ -85,6 +85,112 @@ bool venus_helper_check_codec(struct venus_inst *inst, u32
v4l2_pixfmt)
}
EXPORT_SYMBOL_GPL(venus_helper_check_codec);
+static int venus_helper_queue_dpb_bufs(struct venus_inst *inst)
+{
+ struct intbuf *buf;
+ int ret = 0;
+
+ if (list_empty(&inst->dpbbufs))
+ return 0;
+
+ list_for_each_entry(buf, &inst->dpbbufs, list) {
+ struct hfi_frame_data fdata;
+
+ memset(&fdata, 0, sizeof(fdata));
+ fdata.alloc_len = buf->size;
+ fdata.device_addr = buf->da;
+ fdata.buffer_type = buf->type;
+
+ ret = hfi_session_process_buf(inst, &fdata);
+ if (ret)
+ goto fail;
+ }
+
+fail:
+ return ret;
+}
+
+int venus_helper_free_dpb_bufs(struct venus_inst *inst)
+{
+ struct intbuf *buf, *n;
+
+ if (list_empty(&inst->dpbbufs))
+ return 0;
+
+ list_for_each_entry_safe(buf, n, &inst->dpbbufs, list) {
+ list_del_init(&buf->list);
+ dma_free_attrs(inst->core->dev, buf->size, buf->va, buf->da,
+ buf->attrs);
+ kfree(buf);
+ }
+
+ INIT_LIST_HEAD(&inst->dpbbufs);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(venus_helper_free_dpb_bufs);
+
+int venus_helper_alloc_dpb_bufs(struct venus_inst *inst)
+{
+ struct venus_core *core = inst->core;
+ struct device *dev = core->dev;
+ enum hfi_version ver = core->res->hfi_version;
+ struct hfi_buffer_requirements bufreq;
+ u32 buftype = inst->dpb_buftype;
+ unsigned int dpb_size = 0;
+ struct intbuf *buf;
+ unsigned int i;
+ u32 count;
+ int ret;
+
+ /* no need to allocate dpb buffers */
+ if (!inst->dpb_fmt)
+ return 0;
+
+ if (inst->dpb_buftype == HFI_BUFFER_OUTPUT)
+ dpb_size = inst->output_buf_size;
+ else if (inst->dpb_buftype == HFI_BUFFER_OUTPUT2)
+
Re: [GIT PULL] Qualcomm Driver updates for 4.21 - Part 2
Hi Andy, On 12/8/18 7:13 AM, Andy Gross wrote: > The following changes since commit b601f73130a375c912d9f2ec93c5f3cea5d6a3da: > > drm: msm: Check cmd_db_read_aux_data() for failure (2018-11-29 17:41:53 > -0600) > > are available in the git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/agross/linux.git > tags/qcom-drivers-for-4.21-2 > > for you to fetch changes up to 92def04bd7b46f5b186f985d5c9d3804858c3c2f: > > MAINTAINERS: Change Todor Tomov's email address (2018-12-05 16:45:34 -0600) > > > Qualcomm ARM Based Driver Updates for v4.21 Part 2 > > * Fix MAINTAINERS email address for Todor > * Fix SCM compilation error > > > Jonathan Marek (1): > firmware: qcom: scm: fix compilation error when disabled > > Todor Tomov (1): > MAINTAINERS: Change Todor Tomov's email address I saw this patch is applied also in media_tree.git, not sure that is problem though. -- regards, Stan
Re: [GIT PULL] Qualcomm Driver updates for 4.21 - Part 2
On 10.12.18 г. 21:05 ч., Andy Gross wrote: On Mon, Dec 10, 2018 at 02:20:47PM +0200, Stanimir Varbanov wrote: Hi Andy, On 12/8/18 7:13 AM, Andy Gross wrote: The following changes since commit b601f73130a375c912d9f2ec93c5f3cea5d6a3da: drm: msm: Check cmd_db_read_aux_data() for failure (2018-11-29 17:41:53 -0600) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/agross/linux.git tags/qcom-drivers-for-4.21-2 for you to fetch changes up to 92def04bd7b46f5b186f985d5c9d3804858c3c2f: MAINTAINERS: Change Todor Tomov's email address (2018-12-05 16:45:34 -0600) Qualcomm ARM Based Driver Updates for v4.21 Part 2 * Fix MAINTAINERS email address for Todor * Fix SCM compilation error Jonathan Marek (1): firmware: qcom: scm: fix compilation error when disabled Todor Tomov (1): MAINTAINERS: Change Todor Tomov's email address I saw this patch is applied also in media_tree.git, not sure that is problem though. Which one? The maintainer or the scm? the maintainer one. regards, Stan
Re: [PATCH] arm64: dts: qcom: sdm845: Add SCM DT node
Hi Sibi,
On 10/26/2018 03:25 PM, Sibi Sankar wrote:
> Add SCM DT node to enable SCM functionality on SDM845.
>
> Signed-off-by: Sibi Sankar
> ---
> arch/arm64/boot/dts/qcom/sdm845.dtsi | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index b72bdb0a31a5..fad22acfda4d 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -221,6 +221,12 @@
> };
> };
>
> + firmware {
> + scm {
> + compatible = "qcom,scm-sdm845", "qcom,scm";
> + };
> + };
What will happen if the platform is without tz firmware? I'd move this
DT node in sdm845-mtp.dts or at least make it status = disabled.
--
regards,
Stan
Re: [PATCH] media: venus: support VB2_USERPTR IO mode
Hi Alex, On 10/11/2018 09:50 AM, Alexandre Courbot wrote: > Please ignore this patch - I did not notice that a similar one has > been sent before. The difference is that you made it for decoder as well. Do you need userptr for decoder? -- regards, Stan
Re: [PATCH] iommu: arm-smmu: handle client iommu translation fault handlers
Hi Will, On 10/10/2018 08:08 PM, Will Deacon wrote: > On Wed, Oct 10, 2018 at 05:44:07PM +0300, Stanimir Varbanov wrote: >> Call iommu client translation fault handler(s). >> >> Signed-off-by: Stanimir Varbanov >> --- >> drivers/iommu/arm-smmu.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c >> index 066f4c8daf4e..02a8aab0cc59 100644 >> --- a/drivers/iommu/arm-smmu.c >> +++ b/drivers/iommu/arm-smmu.c >> @@ -568,6 +568,9 @@ static irqreturn_t arm_smmu_context_fault(int irq, void >> *dev) >> fsr, iova, fsynr, cfg->cbndx); >> >> writel(fsr, cb_base + ARM_SMMU_CB_FSR); >> + >> +report_iommu_fault(domain, smmu->dev, iova, 0); > > Who's using this? Generally, the callback here isn't so useful because it's The idea was to use it from video driver to dump few multimedia interconnect registers which will give us some more info about the fault. > called in irq context. We also don't enable stalling since 3714ce1d6655. > > We previously had fairly detailed discussions on the list with Rob Clark > about alternative ways to design this, but I don't remember where it ended > up. It will be useful if you point me to the discussion. Thanks! -- regards, Stan
Re: [PATCH v11 1/5] venus: firmware: add routine to reset ARM9
Hi Joe,
On 10/18/2018 04:42 AM, Joe Perches wrote:
> On Wed, 2018-10-17 at 11:49 +0300, Stanimir Varbanov wrote:
>> On 10/08/2018 04:32 PM, Vikash Garodia wrote:
>>> Add routine to reset the ARM9 and brings it out of reset. Also
>>> abstract the Venus CPU state handling with a new function. This
>>> is in preparation to add PIL functionality in venus driver.
> []
>>> diff --git a/drivers/media/platform/qcom/venus/core.h
>>> b/drivers/media/platform/qcom/venus/core.h
> []
>>> @@ -129,6 +130,7 @@ struct venus_core {
>>> struct device *dev;
>>> struct device *dev_dec;
>>> struct device *dev_enc;
>>> + bool use_tz;
>>
>> could you make it unsigned? For more info please run checkpatch --strict.
>>
>> I know that we have structure members of type bool already - that should
>> be fixed with follow-up patches, I guess.
>
> That's probably not necessary.
>
> I personally have no issue with bool struct members that
> are only used on a transitory basis and not used by hardware
> or shared between multiple cpus with different hardware
> alignment requirements.
Thanks for the clarification. I personally have preference to 'unsigned'
for such flag, but let Hans decide which one to take.
>
> Nothing in this struct is saved or shared.
>
> Perhaps the checkpatch message should be expanded to
> enumerate when bool use in a struct is acceptable.
>
It'd be good to explain more, because it sounds imperative to every
structure.
--
regards,
Stan
Re: [PATCH 1/2] v4l2-ctrl: Add decoder conceal color control
On 2/9/21 1:05 PM, Hans Verkuil wrote:
> On 09/02/2021 10:45, Stanimir Varbanov wrote:
>> Add decoder v4l2 control to set conceal color.
>>
>> Signed-off-by: Stanimir Varbanov
>> ---
>> .../media/v4l/ext-ctrls-codec.rst | 20 +++
>> drivers/media/v4l2-core/v4l2-ctrls.c | 9 +
>> include/uapi/linux/v4l2-controls.h| 1 +
>> 3 files changed, 30 insertions(+)
>>
>> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>> b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>> index 00944e97d638..994650052333 100644
>> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>> @@ -674,6 +674,26 @@ enum v4l2_mpeg_video_frame_skip_mode -
>> is currently displayed (decoded). This value is reset to 0 whenever
>> the decoder is started.
>>
>> +``V4L2_CID_MPEG_VIDEO_DEC_CONCEAL_COLOR (integer64)``
>> +This control sets conceal color in YUV color space. It describes the
>> +client preference of error conceal color in case of error where
>> +reference frame is missing. The decoder would paint the reference
>> +buffer with preferred color and use it for future decoding.
>> +Applicable to decoders.
>
> You should mention explicitly that this is using 16-bit color components
> and expects Limited Range.
I don't want to limit the client to Limited range only. I'll mention in
the description that both ranges are valid.
>
>> +
>> +.. flat-table::
>> +:header-rows: 0
>> +:stub-columns: 0
>> +
>> +* - Bit 0:15
>> + - Y luminance
>> +* - Bit 16:31
>> + - Cb chrominance
>> +* - Bit 32:47
>> + - Cr chrominance
>> +* - Bit 48:63
>> + - Must be zero
>> +
>> ``V4L2_CID_MPEG_VIDEO_DECODER_SLICE_INTERFACE (boolean)``
>> If enabled the decoder expects to receive a single slice per buffer,
>> otherwise the decoder expects a single frame in per buffer.
>> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
>> b/drivers/media/v4l2-core/v4l2-ctrls.c
>> index 016cf6204cbb..a3b9d28a00b7 100644
>> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
>> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
>> @@ -945,6 +945,7 @@ const char *v4l2_ctrl_get_name(u32 id)
>> case V4L2_CID_MPEG_VIDEO_VBV_SIZE: return "VBV
>> Buffer Size";
>> case V4L2_CID_MPEG_VIDEO_DEC_PTS: return "Video
>> Decoder PTS";
>> case V4L2_CID_MPEG_VIDEO_DEC_FRAME: return "Video
>> Decoder Frame Count";
>> +case V4L2_CID_MPEG_VIDEO_DEC_CONCEAL_COLOR: return "Video
>> Decoder Conceal Color";
>> case V4L2_CID_MPEG_VIDEO_VBV_DELAY: return "Initial
>> Delay for VBV Control";
>> case V4L2_CID_MPEG_VIDEO_MV_H_SEARCH_RANGE: return
>> "Horizontal MV Search Range";
>> case V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE: return
>> "Vertical MV Search Range";
>> @@ -1430,6 +1431,14 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum
>> v4l2_ctrl_type *type,
>> *max = 0x7fffLL;
>> *step = 1;
>> break;
>> +case V4L2_CID_MPEG_VIDEO_DEC_CONCEAL_COLOR:
>> +*type = V4L2_CTRL_TYPE_INTEGER64;
>> +*min = 0;
>> +/* default for 8bit black, luma is 16, chroma is 128 */
>
> Since this is 16 bit the actual default luma value for black is 4096 and for
> chroma use
> 32768 (i.e. both values are times 256).
If we follow this for pixel format with 10bit per channel we have to
multiply by 64?
>
>> +*def = 0x8000800010LL;
>> +*max = 0xLL;
>> +*step = 1;
>> +break;
>> case V4L2_CID_PIXEL_RATE:
>> *type = V4L2_CTRL_TYPE_INTEGER64;
>> *flags |= V4L2_CTRL_FLAG_READ_ONLY;
>> diff --git a/include/uapi/linux/v4l2-controls.h
>> b/include/uapi/linux/v4l2-controls.h
>> index 039c0d7add1b..5e5a3068be2d 100644
>> --- a/include/uapi/linux/v4l2-controls.h
>> +++ b/include/uapi/linux/v4l2-controls.h
>> @@ -428,6 +428,7 @@ enum v4l2_mpeg_video_multi_slice_mode {
>> #define V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE
>> (V4L2_CID_CODEC_BASE+228)
>> #define V4L2_CID_MPEG_VIDEO_FORCE_KEY_FRAME
>> (V4L2_CID_CODEC_BASE+229)
>> #define V4L2_CID_MPEG_VIDEO_BASELAYER_PRIORITY_ID
>> (V4L2_CID_CODEC_BASE+230)
>> +#define V4L2_CID_MPEG_VIDEO_DEC_CONCEAL_COLOR
>> (V4L2_CID_CODEC_BASE+231)
>>
>> /* CIDs for the MPEG-2 Part 2 (H.262) codec */
>> #define V4L2_CID_MPEG_VIDEO_MPEG2_LEVEL
>> (V4L2_CID_CODEC_BASE+270)
>>
>
> Regards,
>
> Hans
>
--
regards,
Stan
Re: [PATCH 1/2] v4l2-ctrl: Add decoder conceal color control
On 2/15/21 1:57 PM, Hans Verkuil wrote: > On 15/02/2021 12:32, Stanimir Varbanov wrote: >> >> >> On 2/9/21 1:05 PM, Hans Verkuil wrote: >>> On 09/02/2021 10:45, Stanimir Varbanov wrote: >>>> Add decoder v4l2 control to set conceal color. >>>> >>>> Signed-off-by: Stanimir Varbanov >>>> --- >>>> .../media/v4l/ext-ctrls-codec.rst | 20 +++ >>>> drivers/media/v4l2-core/v4l2-ctrls.c | 9 + >>>> include/uapi/linux/v4l2-controls.h| 1 + >>>> 3 files changed, 30 insertions(+) >>>> >>>> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst >>>> b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst >>>> index 00944e97d638..994650052333 100644 >>>> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst >>>> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst >>>> @@ -674,6 +674,26 @@ enum v4l2_mpeg_video_frame_skip_mode - >>>> is currently displayed (decoded). This value is reset to 0 whenever >>>> the decoder is started. >>>> >>>> +``V4L2_CID_MPEG_VIDEO_DEC_CONCEAL_COLOR (integer64)`` >>>> +This control sets conceal color in YUV color space. It describes the >>>> +client preference of error conceal color in case of error where >>>> +reference frame is missing. The decoder would paint the reference >>>> +buffer with preferred color and use it for future decoding. >>>> +Applicable to decoders. >>> >>> You should mention explicitly that this is using 16-bit color components >>> and expects Limited Range. >> >> I don't want to limit the client to Limited range only. I'll mention in >> the description that both ranges are valid. > > OK, but then you need to describe what the color format depends on. See more > below. > >> >>> >>>> + >>>> +.. flat-table:: >>>> +:header-rows: 0 >>>> +:stub-columns: 0 >>>> + >>>> +* - Bit 0:15 >>>> + - Y luminance >>>> +* - Bit 16:31 >>>> + - Cb chrominance >>>> +* - Bit 32:47 >>>> + - Cr chrominance >>>> +* - Bit 48:63 >>>> + - Must be zero >>>> + The table how the bits are spread into int64. >>>> ``V4L2_CID_MPEG_VIDEO_DECODER_SLICE_INTERFACE (boolean)`` >>>> If enabled the decoder expects to receive a single slice per buffer, >>>> otherwise the decoder expects a single frame in per buffer. >>>> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c >>>> b/drivers/media/v4l2-core/v4l2-ctrls.c >>>> index 016cf6204cbb..a3b9d28a00b7 100644 >>>> --- a/drivers/media/v4l2-core/v4l2-ctrls.c >>>> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c >>>> @@ -945,6 +945,7 @@ const char *v4l2_ctrl_get_name(u32 id) >>>>case V4L2_CID_MPEG_VIDEO_VBV_SIZE: return "VBV >>>> Buffer Size"; >>>>case V4L2_CID_MPEG_VIDEO_DEC_PTS: return "Video >>>> Decoder PTS"; >>>>case V4L2_CID_MPEG_VIDEO_DEC_FRAME: return "Video >>>> Decoder Frame Count"; >>>> + case V4L2_CID_MPEG_VIDEO_DEC_CONCEAL_COLOR: return "Video >>>> Decoder Conceal Color"; >>>>case V4L2_CID_MPEG_VIDEO_VBV_DELAY: return "Initial >>>> Delay for VBV Control"; >>>>case V4L2_CID_MPEG_VIDEO_MV_H_SEARCH_RANGE: return >>>> "Horizontal MV Search Range"; >>>>case V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE: return >>>> "Vertical MV Search Range"; >>>> @@ -1430,6 +1431,14 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum >>>> v4l2_ctrl_type *type, >>>>*max = 0x7fffLL; >>>>*step = 1; >>>>break; >>>> + case V4L2_CID_MPEG_VIDEO_DEC_CONCEAL_COLOR: >>>> + *type = V4L2_CTRL_TYPE_INTEGER64; >>>> + *min = 0; >>>> + /* default for 8bit black, luma is 16, chroma is 128 */ >>> >>> Since this is 16 bit the actual default luma value for black is 4096 and >>> for chroma use >>> 32768 (i.e. both values are times 256). >>
[PATCH] clk: fix possible null pointer dereference
The commit 646cafc6 (clk: Change clk_ops->determine_rate to
return a clk_hw as the best parent) opens a possibility for
null pointer dereference, fix this.
Signed-off-by: Stanimir Varbanov
---
drivers/clk/clk.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index f4963b7..d48ac71 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1366,7 +1366,7 @@ static struct clk *clk_calc_new_rates(struct clk *clk,
unsigned long rate)
new_rate = clk->ops->determine_rate(clk->hw, rate,
&best_parent_rate,
&parent_hw);
- parent = parent_hw->clk;
+ parent = parent_hw ? parent_hw->clk : NULL;
} else if (clk->ops->round_rate) {
new_rate = clk->ops->round_rate(clk->hw, rate,
&best_parent_rate);
--
1.7.0.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] dmaengine: qcom_bam_dma: add one more optional clock
Hi Andy, On 09/07/2014 08:55 PM, Stanimir Varbanov wrote: > The BAM is tightly coupled with the peripheral to which it > belongs. In that sprit to access the BAM configuration > registers the driver needs to enable some peripheral > clocks. Currently the DT node enables bamclk which seems > is not enough for some peripherals (for example the crypto > engine wants core and iface clocks). This change attempts > to solve this issue by adding one more optional clock > in bam_dma driver. > What is your opinion on this patch? > Signed-off-by: Stanimir Varbanov > --- > .../devicetree/bindings/dma/qcom_bam_dma.txt | 12 -- > drivers/dma/qcom_bam_dma.c | 44 > +++--- > 2 files changed, 38 insertions(+), 18 deletions(-) > > diff --git a/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt > b/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt > index d75a9d7..2376897 100644 > --- a/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt > +++ b/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt > @@ -6,8 +6,11 @@ Required properties: > - interrupts: Should contain the one interrupt shared by all channels > - #dma-cells: must be <1>, the cell in the dmas property of the client device >represents the channel number > -- clocks: required clock > -- clock-names: must contain "bam_clk" entry > +- clocks: list of required clock plus one optional clock. The optional clock > + is needed for some peripherals and can be omitted. > +- clock-names: must contain "core" clock name representing the required clock > + plus the optional "iface" clock name depending on > + peripheral needs. > - qcom,ee : indicates the active Execution Environment identifier (0-7) used > in >the secure world. > > @@ -17,8 +20,9 @@ Example: > compatible = "qcom,bam-v1.4.0"; > reg = <0xf9984000 0x15000>; > interrupts = <0 94 0>; > - clocks = <&gcc GCC_BAM_DMA_AHB_CLK>; > - clock-names = "bam_clk"; > + clocks = <&gcc GCC_BLSP1_UART2_APPS_CLK>, > + <&gcc GCC_BLSP1_AHB_CLK>; > + clock-names = "core", "iface"; > #dma-cells = <1>; > qcom,ee = <0>; > }; -- regards, Stan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 5/5] ARM: qcom: Add Qualcomm APQ8084 SoC
Hi Arnd, Thanks for the comments! On 12/12/2014 07:33 PM, Arnd Bergmann wrote: > On Friday 12 December 2014 19:14:01 Stanimir Varbanov wrote: >> +config ARCH_APQ8084 >> + bool "Enable support for APQ8084" >> + select HAVE_ARM_ARCH_TIMER >> + select PCI >> + select PCI_DOMAINS >> + select PCI_MSI >> + > > I would prefer not to see 'select PCI' here. Also the driver is written > to work without PCI_MSI, so I'd be much happier with just > > config ARCH_APQ8084 > bool "Enable support for APQ8084" > select HAVE_ARM_ARCH_TIMER > select PCI_DOMAINS if PCI What about this config ARCH_APQ8084 bool "Enable support for APQ8084" select HAVE_ARM_ARCH_TIMER select MIGHT_HAVE_PCI select PCI_DOMAINS if PCI -- regards, Stan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] spmi: pmic_arb: add support for hw version 2
Hi Gilad,
On 01/20/2015 03:10 AM, Gilad Avidov wrote:
> Qualcomm PMIC Arbiter version-2 changes from version-1 are:
>
> - Some diffrent register offsets.
> - New channel register space, one per PMIC peripheral (ppid).
> All tx tarffic uses these channels.
> - New observer register space. All rx trafic uses this space.
> - Diffrent command format for spmi command registers.
>
> Signed-off-by: Gilad Avidov
> Acked-by: Sagar Dharia
> ---
> .../bindings/spmi/qcom,spmi-pmic-arb.txt | 11 +-
> drivers/spmi/spmi-pmic-arb.c | 295
> ++---
> 2 files changed, 263 insertions(+), 43 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt
> b/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt
> index 715d099..827bd21 100644
> --- a/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt
> +++ b/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt
> @@ -1,11 +1,11 @@
> Qualcomm SPMI Controller (PMIC Arbiter)
>
> -The SPMI PMIC Arbiter is found on the Snapdragon 800 Series. It is an SPMI
> +The SPMI PMIC Arbiter is found on Snapdragon chipsets. It is an SPMI
> controller with wrapping arbitration logic to allow for multiple on-chip
> devices to control a single SPMI master.
>
> -The PMIC Arbiter can also act as an interrupt controller, providing
> interrupts
> -to slave devices.
> +The PMIC Arbiter is also an interrupt controller, interrupting the Snapdragon
> +on dtection of a sequence initiated by a request-capable-slave to the master.
>
>
> -/* Non-data command */
> -static int pmic_arb_cmd(struct spmi_controller *ctrl, u8 opc, u8 sid)
> +static int
> +pmic_arb_non_data_cmd_v1(struct spmi_controller *ctrl, u8 opc, u8 sid)
> {
> struct spmi_pmic_arb_dev *pmic_arb = spmi_controller_get_drvdata(ctrl);
> unsigned long flags;
> u32 cmd;
> int rc;
> -
> - /* Check for valid non-data command */
> - if (opc < SPMI_CMD_RESET || opc > SPMI_CMD_WAKEUP)
> - return -EINVAL;
> + u32 offset = pmic_arb->ver->offset(pmic_arb, sid, 0);
>
> cmd = ((opc | 0x40) << 27) | ((sid & 0xf) << 20);
>
> raw_spin_lock_irqsave(&pmic_arb->lock, flags);
> - pmic_arb_base_write(pmic_arb, PMIC_ARB_CMD(pmic_arb->channel), cmd);
> - rc = pmic_arb_wait_for_done(ctrl);
> + pmic_arb_base_write(pmic_arb, offset + PMIC_ARB_CMD, cmd);
> + rc = pmic_arb_wait_for_done(ctrl, pmic_arb->wr_base, sid, 0);
> raw_spin_unlock_irqrestore(&pmic_arb->lock, flags);
>
> return rc;
> }
>
> +/* Unsupported by HW */
> +static int
> +pmic_arb_non_data_cmd_v2(struct spmi_controller *ctrl, u8 opc, u8 sid)
> +{
> + return -EOPNOTSUPP;
> +}
Does pmic arbiter v2 supports SPMI_CMD_WAKEUP and SPMI_CMD_SHUTDOWN
commands? If so how we send those commands to the arbiter when the
.non_data_cmd operation returns EOPNOTSUPP. If we returning EOPNOTSUPP
the spmi bus .probe method will not call spmi driver .probe. See spmi.c
spmi_drv_probe().
--
regards,
Stan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] spmi: pmic_arb: add support for hw version 2
Hi Gilad,
>>
>>> -/* Non-data command */
>>> -static int pmic_arb_cmd(struct spmi_controller *ctrl, u8 opc, u8 sid)
>>> +static int
>>> +pmic_arb_non_data_cmd_v1(struct spmi_controller *ctrl, u8 opc, u8 sid)
>>> {
>>> struct spmi_pmic_arb_dev *pmic_arb =
>>> spmi_controller_get_drvdata(ctrl);
>>> unsigned long flags;
>>> u32 cmd;
>>> int rc;
>>> -
>>> -/* Check for valid non-data command */
>>> -if (opc < SPMI_CMD_RESET || opc > SPMI_CMD_WAKEUP)
>>> -return -EINVAL;
>>> +u32 offset = pmic_arb->ver->offset(pmic_arb, sid, 0);
>>> cmd = ((opc | 0x40) << 27) | ((sid & 0xf) << 20);
>>> raw_spin_lock_irqsave(&pmic_arb->lock, flags);
>>> -pmic_arb_base_write(pmic_arb, PMIC_ARB_CMD(pmic_arb->channel),
>>> cmd);
>>> -rc = pmic_arb_wait_for_done(ctrl);
>>> +pmic_arb_base_write(pmic_arb, offset + PMIC_ARB_CMD, cmd);
>>> +rc = pmic_arb_wait_for_done(ctrl, pmic_arb->wr_base, sid, 0);
>>> raw_spin_unlock_irqrestore(&pmic_arb->lock, flags);
>>> return rc;
>>> }
>>> +/* Unsupported by HW */
>>> +static int
>>> +pmic_arb_non_data_cmd_v2(struct spmi_controller *ctrl, u8 opc, u8 sid)
>>> +{
>>> +return -EOPNOTSUPP;
>>> +}
>
> Hi Stanimir,
>
>> Does pmic arbiter v2 supports SPMI_CMD_WAKEUP and SPMI_CMD_SHUTDOWN
>> commands? If so how we send those commands to the arbiter when the
> pmic-arbiter v2 does not support non-data commands including the two
> that you have mentioned above.
>> .non_data_cmd operation returns EOPNOTSUPP. If we returning EOPNOTSUPP
>> the spmi bus .probe method will not call spmi driver .probe. See spmi.c
>> spmi_drv_probe().
> Very keen observation!
>
> The slaves that I'm working on do not need nor support the wakeup command.
> I'll add a patch to add a new device tree boolean property to the
> framework, maybe "skip-wakeup", for similar slaves.
> Then change spmi_drv_probe() to skip the wakeup if the property is there.
No, instead of adding one more dt property just return zero from
.non_data_cmd operation and keep the comment that this is not supported
on v2. The other way is to fill the .non_data_cmd = NULL on v2 and add a
check for .non_data_cmd != NULL in the callers.
--
regards,
Stan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] spmi: pmic_arb: add support for hw version 2
Hi Gilad, >> /* Interrupt Controller */ >> #define SPMI_PIC_OWNER_ACC_STATUS(M, N) (0x + ((32 * (M)) + (4 * >> (N > > It looks like these macros would change too, but nothing has been done > here. Interrupts haven't been tested? Stephen is right, the irq related operations are not used in irq_chip methods. Do you plan to add it in next version? >> +/** >> + * pmic_arb_ver: version dependent functionality and values. >> + * >> + * @non_data_cmd: on v1 issues an spmi non-data command. >> + * on v2 no HW support, returns -EOPNOTSUPP. >> + * @offset: on v1 offset of per-ee channel. >> + * on v2 offset of per-ee and per-ppid channel. >> + * @fmt_cmd:formats a GENI/SPMI command. >> + * @owner_acc_status: on v1 offset of >> PMIC_ARB_SPMI_PIC_OWNERm_ACC_STATUSn >> + * on v2 offset of SPMI_PIC_OWNERm_ACC_STATUSn. >> + * @acc_enable: on v1 offset of PMIC_ARB_SPMI_PIC_ACC_ENABLEn >> + * on v2 offset of SPMI_PIC_ACC_ENABLEn. >> + * @irq_status: on v1 offset of PMIC_ARB_SPMI_PIC_IRQ_STATUSn >> + * on v2 offset of SPMI_PIC_IRQ_STATUSn. >> + * @irq_clear: on v1 offset of PMIC_ARB_SPMI_PIC_IRQ_CLEARn >> + * on v2 offset of SPMI_PIC_IRQ_CLEARn. >> + * @geni_ctrl: PMIC_ARB_GENI_CTRL offset. >> + * @geni_status:PMIC_ARB_GENI_STATUS offset. >> + * @protocol_irq_status: PMIC_ARB_PROTOCOL_IRQ_STATUS offset. >> + */ -- regards, Stan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] pinctrl: qcom: enable generic support and input-enable pinctrl conf
Enables generic pinconf support and add handling for 'input-enable'
pinconf property.
Signed-off-by: Stanimir Varbanov
---
drivers/pinctrl/qcom/pinctrl-msm.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c
b/drivers/pinctrl/qcom/pinctrl-msm.c
index e730935..4a47eb1 100644
--- a/drivers/pinctrl/qcom/pinctrl-msm.c
+++ b/drivers/pinctrl/qcom/pinctrl-msm.c
@@ -193,11 +193,11 @@ static int msm_config_reg(struct msm_pinctrl *pctrl,
*mask = 7;
break;
case PIN_CONFIG_OUTPUT:
+ case PIN_CONFIG_INPUT_ENABLE:
*bit = g->oe_bit;
*mask = 1;
break;
default:
- dev_err(pctrl->dev, "Invalid config param %04x\n", param);
return -ENOTSUPP;
}
@@ -208,14 +208,12 @@ static int msm_config_get(struct pinctrl_dev *pctldev,
unsigned int pin,
unsigned long *config)
{
- dev_err(pctldev->dev, "pin_config_set op not supported\n");
return -ENOTSUPP;
}
static int msm_config_set(struct pinctrl_dev *pctldev, unsigned int pin,
unsigned long *configs, unsigned num_configs)
{
- dev_err(pctldev->dev, "pin_config_set op not supported\n");
return -ENOTSUPP;
}
@@ -276,9 +274,11 @@ static int msm_config_group_get(struct pinctrl_dev
*pctldev,
val = readl(pctrl->regs + g->io_reg);
arg = !!(val & BIT(g->in_bit));
break;
+ case PIN_CONFIG_INPUT_ENABLE:
+ val = readl(pctrl->regs + g->io_reg);
+ arg = !!(val & BIT(g->in_bit));
+ break;
default:
- dev_err(pctrl->dev, "Unsupported config parameter: %x\n",
- param);
return -EINVAL;
}
@@ -348,6 +348,10 @@ static int msm_config_group_set(struct pinctrl_dev
*pctldev,
/* enable output */
arg = 1;
break;
+ case PIN_CONFIG_INPUT_ENABLE:
+ /* disable output */
+ arg = 0;
+ break;
default:
dev_err(pctrl->dev, "Unsupported config parameter:
%x\n",
param);
@@ -372,6 +376,9 @@ static int msm_config_group_set(struct pinctrl_dev *pctldev,
}
static const struct pinconf_ops msm_pinconf_ops = {
+#ifdef CONFIG_GENERIC_PINCONF
+ .is_generic = true,
+#endif
.pin_config_get = msm_config_get,
.pin_config_set = msm_config_set,
.pin_config_group_get = msm_config_group_get,
--
1.7.0.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/3] pinctrl: Qualcomm msm8916 pinctrl driver
This series adds a pinctrl driver for Snapdragon 410 (msm8916) SoC. The first patch increase the register address variable size, next adds a binding document and the last patch adds the pinctrl driver Comments are welcome! regards, Stan Joonwoo Park (2): pinctrl: qcom: increase variable size for register addresses pinctrl: qcom: Add msm8916 pinctrl driver Stanimir Varbanov (1): DT: pinctrl: Document Qualcomm MSM8916 pinctrl binding .../bindings/pinctrl/qcom,msm8916-pinctrl.txt | 186 +++ drivers/pinctrl/qcom/Kconfig |8 + drivers/pinctrl/qcom/Makefile |1 + drivers/pinctrl/qcom/pinctrl-msm.h | 10 +- drivers/pinctrl/qcom/pinctrl-msm8916.c | 1280 5 files changed, 1480 insertions(+), 5 deletions(-) create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,msm8916-pinctrl.txt create mode 100644 drivers/pinctrl/qcom/pinctrl-msm8916.c -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/3] pinctrl: qcom: increase variable size for register addresses
From: Joonwoo Park
Newer MSM SoCs have TLMM hardware block upper than 16 bit. Increase to
32 bit registers to hold addresses correctly.
Signed-off-by: Joonwoo Park
Signed-off-by: Stanimir Varbanov
---
drivers/pinctrl/qcom/pinctrl-msm.h | 10 +-
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/pinctrl/qcom/pinctrl-msm.h
b/drivers/pinctrl/qcom/pinctrl-msm.h
index b952c4b..54fdd04 100644
--- a/drivers/pinctrl/qcom/pinctrl-msm.h
+++ b/drivers/pinctrl/qcom/pinctrl-msm.h
@@ -70,11 +70,11 @@ struct msm_pingroup {
unsigned *funcs;
unsigned nfuncs;
- s16 ctl_reg;
- s16 io_reg;
- s16 intr_cfg_reg;
- s16 intr_status_reg;
- s16 intr_target_reg;
+ u32 ctl_reg;
+ u32 io_reg;
+ u32 intr_cfg_reg;
+ u32 intr_status_reg;
+ u32 intr_target_reg;
unsigned mux_bit:5;
--
1.7.0.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 3/3] pinctrl: qcom: Add msm8916 pinctrl driver
From: Joonwoo Park
Add initial pinctrl driver to support pin configuration with
pinctrl framework for msm8916.
Signed-off-by: Joonwoo Park
Signed-off-by: Stanimir Varbanov
---
drivers/pinctrl/qcom/Kconfig |8 +
drivers/pinctrl/qcom/Makefile |1 +
drivers/pinctrl/qcom/pinctrl-msm8916.c | 1280
3 files changed, 1289 insertions(+), 0 deletions(-)
create mode 100644 drivers/pinctrl/qcom/pinctrl-msm8916.c
diff --git a/drivers/pinctrl/qcom/Kconfig b/drivers/pinctrl/qcom/Kconfig
index 3cd243c..ea575f6 100644
--- a/drivers/pinctrl/qcom/Kconfig
+++ b/drivers/pinctrl/qcom/Kconfig
@@ -47,6 +47,14 @@ config PINCTRL_MSM8X74
This is the pinctrl, pinmux, pinconf and gpiolib driver for the
Qualcomm TLMM block found in the Qualcomm 8974 platform.
+config PINCTRL_MSM8916
+ tristate "Qualcomm 8916 pin controller driver"
+ depends on GPIOLIB && OF
+ select PINCTRL_MSM
+ help
+ This is the pinctrl, pinmux, pinconf and gpiolib driver for the
+ Qualcomm TLMM block found on the Qualcomm 8916 platform.
+
config PINCTRL_QCOM_SPMI_PMIC
tristate "Qualcomm SPMI PMIC pin controller driver"
depends on GPIOLIB && OF && SPMI
diff --git a/drivers/pinctrl/qcom/Makefile b/drivers/pinctrl/qcom/Makefile
index bfd79af..6895870 100644
--- a/drivers/pinctrl/qcom/Makefile
+++ b/drivers/pinctrl/qcom/Makefile
@@ -5,5 +5,6 @@ obj-$(CONFIG_PINCTRL_APQ8084) += pinctrl-apq8084.o
obj-$(CONFIG_PINCTRL_IPQ8064) += pinctrl-ipq8064.o
obj-$(CONFIG_PINCTRL_MSM8960) += pinctrl-msm8960.o
obj-$(CONFIG_PINCTRL_MSM8X74) += pinctrl-msm8x74.o
+obj-$(CONFIG_PINCTRL_MSM8916) += pinctrl-msm8916.o
obj-$(CONFIG_PINCTRL_QCOM_SPMI_PMIC) += pinctrl-spmi-gpio.o
obj-$(CONFIG_PINCTRL_QCOM_SPMI_PMIC) += pinctrl-spmi-mpp.o
diff --git a/drivers/pinctrl/qcom/pinctrl-msm8916.c
b/drivers/pinctrl/qcom/pinctrl-msm8916.c
new file mode 100644
index 000..3b851de
--- /dev/null
+++ b/drivers/pinctrl/qcom/pinctrl-msm8916.c
@@ -0,0 +1,1280 @@
+/*
+ * Copyright (c) 2015, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only 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
+#include
+#include
+#include
+
+#include "pinctrl-msm.h"
+
+static const struct pinctrl_pin_desc msm8916_pins[] = {
+ PINCTRL_PIN(0, "GPIO_0"),
+ PINCTRL_PIN(1, "GPIO_1"),
+ PINCTRL_PIN(2, "GPIO_2"),
+ PINCTRL_PIN(3, "GPIO_3"),
+ PINCTRL_PIN(4, "GPIO_4"),
+ PINCTRL_PIN(5, "GPIO_5"),
+ PINCTRL_PIN(6, "GPIO_6"),
+ PINCTRL_PIN(7, "GPIO_7"),
+ PINCTRL_PIN(8, "GPIO_8"),
+ PINCTRL_PIN(9, "GPIO_9"),
+ PINCTRL_PIN(10, "GPIO_10"),
+ PINCTRL_PIN(11, "GPIO_11"),
+ PINCTRL_PIN(12, "GPIO_12"),
+ PINCTRL_PIN(13, "GPIO_13"),
+ PINCTRL_PIN(14, "GPIO_14"),
+ PINCTRL_PIN(15, "GPIO_15"),
+ PINCTRL_PIN(16, "GPIO_16"),
+ PINCTRL_PIN(17, "GPIO_17"),
+ PINCTRL_PIN(18, "GPIO_18"),
+ PINCTRL_PIN(19, "GPIO_19"),
+ PINCTRL_PIN(20, "GPIO_20"),
+ PINCTRL_PIN(21, "GPIO_21"),
+ PINCTRL_PIN(22, "GPIO_22"),
+ PINCTRL_PIN(23, "GPIO_23"),
+ PINCTRL_PIN(24, "GPIO_24"),
+ PINCTRL_PIN(25, "GPIO_25"),
+ PINCTRL_PIN(26, "GPIO_26"),
+ PINCTRL_PIN(27, "GPIO_27"),
+ PINCTRL_PIN(28, "GPIO_28"),
+ PINCTRL_PIN(29, "GPIO_29"),
+ PINCTRL_PIN(30, "GPIO_30"),
+ PINCTRL_PIN(31, "GPIO_31"),
+ PINCTRL_PIN(32, "GPIO_32"),
+ PINCTRL_PIN(33, "GPIO_33"),
+ PINCTRL_PIN(34, "GPIO_34"),
+ PINCTRL_PIN(35, "GPIO_35"),
+ PINCTRL_PIN(36, "GPIO_36"),
+ PINCTRL_PIN(37, "GPIO_37"),
+ PINCTRL_PIN(38, "GPIO_38"),
+ PINCTRL_PIN(39, "GPIO_39"),
+ PINCTRL_PIN(40, "GPIO_40"),
+ PINCTRL_PIN(41, "GPIO_41"),
+ PINCTRL_PIN(42, "GPIO_42"),
+ PINCTRL_PIN(43, "GPIO_43"),
+ PINCTRL_PIN(44, "GPIO_44"),
+ PINCTRL_PIN(45, "GPIO_45"),
+ PINCTRL_PIN(46, "GPIO_46"),
+ PINCTRL_PIN(47, "GPIO_47"),
+ PINCTRL_PIN(48, &quo
[PATCH 2/3] DT: pinctrl: Document Qualcomm MSM8916 pinctrl binding
Adds devicetree binding documentation. Signed-off-by: Stanimir Varbanov --- .../bindings/pinctrl/qcom,msm8916-pinctrl.txt | 186 1 files changed, 186 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,msm8916-pinctrl.txt diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,msm8916-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/qcom,msm8916-pinctrl.txt new file mode 100644 index 000..10c2dcf --- /dev/null +++ b/Documentation/devicetree/bindings/pinctrl/qcom,msm8916-pinctrl.txt @@ -0,0 +1,186 @@ +Qualcomm MSM8916 TLMM block + +This binding describes the Top Level Mode Multiplexer block found in the +MSM8916 platform. + +- compatible: + Usage: required + Value type: + Definition: must be "qcom,msm8916-pinctrl" + +- reg: + Usage: required + Value type: + Definition: the base address and size of the TLMM register space. + +- interrupts: + Usage: required + Value type: + Definition: should specify the TLMM summary IRQ. + +- interrupt-controller: + Usage: required + Value type: + Definition: identifies this node as an interrupt controller + +- #interrupt-cells: + Usage: required + Value type: + Definition: must be 2. Specifying the pin number and flags, as defined + in + +- gpio-controller: + Usage: required + Value type: + Definition: identifies this node as a gpio controller + +- #gpio-cells: + Usage: required + Value type: + Definition: must be 2. Specifying the pin number and flags, as defined + in + +Please refer to ../gpio/gpio.txt and ../interrupt-controller/interrupts.txt for +a general description of GPIO and interrupt bindings. + +Please refer to pinctrl-bindings.txt in this directory for details of the +common pinctrl bindings used by client devices, including the meaning of the +phrase "pin configuration node". + +The pin configuration nodes act as a container for an arbitrary number of +subnodes. Each of these subnodes represents some desired configuration for a +pin, a group, or a list of pins or groups. This configuration can include the +mux function to select on those pin(s)/group(s), and various pin configuration +parameters, such as pull-up, drive strength, etc. + + +PIN CONFIGURATION NODES: + +The name of each subnode is not important; all subnodes should be enumerated +and processed purely based on their content. + +Each subnode only affects those parameters that are explicitly listed. In +other words, a subnode that lists a mux function but no pin configuration +parameters implies no information about any pin configuration parameters. +Similarly, a pin subnode that describes a pullup parameter implies no +information about e.g. the mux function. + + +The following generic properties as defined in pinctrl-bindings.txt are valid +to specify in a pin configuration subnode: + +- pins: + Usage: required + Value type: + Definition: List of gpio pins affected by the properties specified in + this subnode. Valid pins are: + gpio0-gpio121, + sdc1_clk, + sdc1_cmd, + sdc1_data + sdc2_clk, + sdc2_cmd, + sdc2_data, + qdsd_cmd, + qdsd_data0, + qdsd_data1, + qdsd_data2, + qdsd_data3 + +- function: + Usage: required + Value type: + Definition: Specify the alternative function to be configured for the + specified pins. Functions are only valid for gpio pins. + Valid values are: + adsp_ext, alsp_int, atest_bbrx0, atest_bbrx1, atest_char, atest_char0, + atest_char1, atest_char2, atest_char3, atest_combodac, atest_gpsadc0, + atest_gpsadc1, atest_tsens, atest_wlan0, atest_wlan1, backlight_en, + bimc_dte0, bimc_dte1, blsp1_spi, blsp2_spi, blsp3_spi, blsp_i2c1, + blsp_i2c2, blsp_i2c3, blsp_i2c4, blsp_i2c5, blsp_i2c6, blsp_spi1, + blsp_spi2, blsp_spi3, blsp_spi4, blsp_spi5, blsp_spi6, blsp_uart1, + blsp_uart2, blsp_uim1, blsp_uim2, cam1_rst, cam1_standby, cam_mclk, + cci_async, cci_i2c, cci_timer0, cci_timer1, cci_timer2, cdc_pdm0, + codec_mad, dbg_out, display_5v, dmic0_clk, dmic0_data, dsi_rst, + ebi0_wrcdc, euro_us, ext_lpass, flash_strobe, gcc_gp1_clk_a, + gcc_gp1_clk_b, gcc_gp2_clk_a, gcc_gp2_clk_b, gcc_gp3_clk_a, + gcc_gp3_clk_b, gpio, gsm0_tx0, gsm0_tx1, gsm1_tx0, gsm1_tx1, gyro_accl, + kpsns0, kpsns1, kpsns2, ldo_en, ldo_update, mag_int, mdp_vsync, + modem_tsync, m_voc, nav_pps, nav_tsync, pa_indicator, pbs0, pbs1, pbs2, + pri_mi2s, pri_mi2s_ws, prng_rosc, pwr_crypto_enabled_a, + pwr_crypto_enabled_b, pwr_modem_enabled_a,
Re: [PATCH] pinctrl: qcom: enable generic support and input-enable pinctrl conf
Hi Stephen,
Thanks for the comments!
On 01/27/2015 03:18 AM, Stephen Boyd wrote:
> On 01/26/15 08:24, Stanimir Varbanov wrote:
>> Enables generic pinconf support and add handling for 'input-enable'
>> pinconf property.
>>
>> Signed-off-by: Stanimir Varbanov
>> ---
>> drivers/pinctrl/qcom/pinctrl-msm.c | 17 -
>> 1 files changed, 12 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c
>> b/drivers/pinctrl/qcom/pinctrl-msm.c
>> index e730935..4a47eb1 100644
>> --- a/drivers/pinctrl/qcom/pinctrl-msm.c
>> +++ b/drivers/pinctrl/qcom/pinctrl-msm.c
>> @@ -193,11 +193,11 @@ static int msm_config_reg(struct msm_pinctrl *pctrl,
>> *mask = 7;
>> break;
>> case PIN_CONFIG_OUTPUT:
>> +case PIN_CONFIG_INPUT_ENABLE:
>> *bit = g->oe_bit;
>> *mask = 1;
>> break;
>> default:
>> -dev_err(pctrl->dev, "Invalid config param %04x\n", param);
>
> Is this removed because the generic framework already takes care of
> printing this information on such errors?
yes, more or less. These error messages just floods the screen when
using debugfs entries. Even more ENOTSUPP is legal error for generic
pinconf.
>
>> return -ENOTSUPP;
>> }
>>
>> @@ -208,14 +208,12 @@ static int msm_config_get(struct pinctrl_dev *pctldev,
>>unsigned int pin,
>>unsigned long *config)
>> {
>> -dev_err(pctldev->dev, "pin_config_set op not supported\n");
>
> Same for here.
>
>> return -ENOTSUPP;
>> }
>>
>> static int msm_config_set(struct pinctrl_dev *pctldev, unsigned int pin,
>> unsigned long *configs, unsigned num_configs)
>> {
>> -dev_err(pctldev->dev, "pin_config_set op not supported\n");
>
> And here.
>
>> return -ENOTSUPP;
>> }
>>
>> @@ -276,9 +274,11 @@ static int msm_config_group_get(struct pinctrl_dev
>> *pctldev,
>> val = readl(pctrl->regs + g->io_reg);
>> arg = !!(val & BIT(g->in_bit));
>> break;
>> +case PIN_CONFIG_INPUT_ENABLE:
>> +val = readl(pctrl->regs + g->io_reg);
>> +arg = !!(val & BIT(g->in_bit));
>> +break;
>
> This bit is used to read the value of the gpio. If the gpio is high,
> this bit will read back as a 1. If the gpio is low, this bit will read
> back as a 0. I don't see how this is related to input-enable? Doesn't
Maybe I had to split on few patches. The change in .pin_config_group_get
is related to debugfs dump. The change which adds input-enable support
is in .pin_config_group_set.
> input-enable mean we configure the pin to accept input? In that sense,
> configuring the pin to accept input would sort of be like configuring
> the pin for function "gpio" so that we can read this bit and see if the
> pin is high or low, but I don't know if we care to support that. I think
> we rely on users configuring the pin for the gpio function though.
Why you do not care, because you think that tlmm doesn't support it or
because it is a driver responsibility to set up gpio function plus
input/output configuration?
I can imagine usecase where the driver implements an 'idle' pinctrl
state which configure its pins to be gpio input enabled to save power
assuming that the previous 'default' pinctrl state it was gpio output.
In that case the driver doesn't need to call gpio_direction_input() for
every gpio, only call pinctrl_select_state('idle').
What's wrong with this?
>
>> default:
>> -dev_err(pctrl->dev, "Unsupported config parameter: %x\n",
>> -param);
>> return -EINVAL;
>> }
>>
>> @@ -348,6 +348,10 @@ static int msm_config_group_set(struct pinctrl_dev
>> *pctldev,
>> /* enable output */
>> arg = 1;
>> break;
>> +case PIN_CONFIG_INPUT_ENABLE:
>> +/* disable output */
>> +arg = 0;
>> +break;
>> default:
>> dev_err(pctrl->dev, "Unsupported config parameter:
>> %x\n",
>> param);
>> @@ -372,6 +376,9 @@ static int msm_config_group_set(struct pinctrl_dev
>> *pctldev,
>> }
>>
&g
Re: [PATCH] pinctrl: qcom: enable generic support and input-enable pinctrl conf
On 01/27/2015 03:23 AM, Stephen Boyd wrote:
> On 01/26/15 17:18, Stephen Boyd wrote:
>> On 01/26/15 08:24, Stanimir Varbanov wrote:
>>
>>> return -ENOTSUPP;
>>> }
>>>
>>> @@ -208,14 +208,12 @@ static int msm_config_get(struct pinctrl_dev *pctldev,
>>> unsigned int pin,
>>> unsigned long *config)
>>> {
>>> - dev_err(pctldev->dev, "pin_config_set op not supported\n");
>> Same for here.
>>
>>> return -ENOTSUPP;
>>> }
>>>
>>> static int msm_config_set(struct pinctrl_dev *pctldev, unsigned int pin,
>>> unsigned long *configs, unsigned num_configs)
>>> {
>>> - dev_err(pctldev->dev, "pin_config_set op not supported\n");
>> And here.
>
> Actually it looks like we can just remove these two functions and the
> core does the right thing and prints errors. That would be a good patch.
>
Agreed.
--
regards,
Stan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/6] soc: qcom: gsbi: Add support for ADM CRCI muxing
Hi Andy, On 01/28/2015 12:10 AM, Andy Gross wrote: > This patch adds automatic configuration for the ADM CRCI muxing required to > support DMA operations for GSBI clients. The GSBI mode and instance determine > the correct TCSR ADM CRCI MUX value that must be programmed so that the DMA > works properly. > > Signed-off-by: Andy Gross > --- > .../devicetree/bindings/soc/qcom/qcom,gsbi.txt | 17 ++- > drivers/soc/qcom/Kconfig |1 + > drivers/soc/qcom/qcom_gsbi.c | 148 > +++- > 3 files changed, 158 insertions(+), 8 deletions(-) > > diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig > index 7bd2c94..32f20be 100644 > --- a/drivers/soc/qcom/Kconfig > +++ b/drivers/soc/qcom/Kconfig > @@ -4,6 +4,7 @@ > config QCOM_GSBI > tristate "QCOM General Serial Bus Interface" > depends on ARCH_QCOM > + select MFD_SYSCON Wrong indentation? In fact the original Kconfig entry has spaces instead of tabs, could you prepare a cleanup patch for this. > help >Say y here to enable GSBI support. The GSBI provides control >functions for connecting the underlying serial UART, SPI, and I2C -- regards, Stan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 0/3] pinctrl: Qualcomm msm8916 pinctrl driver
Changes since v1: - correct the subject and description in 1/3 - not break the fields in msm8916_groups[] array in 3/3 - added Reviewed-by tags on all three patches --- This series adds a pinctrl driver for Snapdragon 410 (msm8916) SoC. The first patch increase the register address variable size, next adds a binding document and the last patch adds the pinctrl driver Comments are welcome! regards, Stan Joonwoo Park (2): pinctrl: qcom: increase variable size for register offsets pinctrl: qcom: Add msm8916 pinctrl driver Stanimir Varbanov (1): DT: pinctrl: Document Qualcomm MSM8916 pinctrl binding .../bindings/pinctrl/qcom,msm8916-pinctrl.txt | 186 +++ drivers/pinctrl/qcom/Kconfig |8 + drivers/pinctrl/qcom/Makefile |1 + drivers/pinctrl/qcom/pinctrl-msm.h | 10 +- drivers/pinctrl/qcom/pinctrl-msm8916.c | 1247 5 files changed, 1447 insertions(+), 5 deletions(-) create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,msm8916-pinctrl.txt create mode 100644 drivers/pinctrl/qcom/pinctrl-msm8916.c -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 1/3] pinctrl: qcom: increase variable size for register offsets
From: Joonwoo Park
On newer TLMM hardware blocks the registers are spread and
we need an offsets upper than 16 bits to address them. Increase
the register offset variables to 32 bits size.
Signed-off-by: Joonwoo Park
Signed-off-by: Stanimir Varbanov
Reviewed-by: Bjorn Andersson
---
drivers/pinctrl/qcom/pinctrl-msm.h | 10 +-
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/pinctrl/qcom/pinctrl-msm.h
b/drivers/pinctrl/qcom/pinctrl-msm.h
index b952c4b..54fdd04 100644
--- a/drivers/pinctrl/qcom/pinctrl-msm.h
+++ b/drivers/pinctrl/qcom/pinctrl-msm.h
@@ -70,11 +70,11 @@ struct msm_pingroup {
unsigned *funcs;
unsigned nfuncs;
- s16 ctl_reg;
- s16 io_reg;
- s16 intr_cfg_reg;
- s16 intr_status_reg;
- s16 intr_target_reg;
+ u32 ctl_reg;
+ u32 io_reg;
+ u32 intr_cfg_reg;
+ u32 intr_status_reg;
+ u32 intr_target_reg;
unsigned mux_bit:5;
--
1.7.0.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 3/3] pinctrl: qcom: Add msm8916 pinctrl driver
From: Joonwoo Park
Add initial pinctrl driver to support pin configuration with
pinctrl framework for msm8916.
Signed-off-by: Joonwoo Park
Signed-off-by: Stanimir Varbanov
Reviewed-by: Bjorn Andersson
---
drivers/pinctrl/qcom/Kconfig |8 +
drivers/pinctrl/qcom/Makefile |1 +
drivers/pinctrl/qcom/pinctrl-msm8916.c | 1247
3 files changed, 1256 insertions(+), 0 deletions(-)
create mode 100644 drivers/pinctrl/qcom/pinctrl-msm8916.c
diff --git a/drivers/pinctrl/qcom/Kconfig b/drivers/pinctrl/qcom/Kconfig
index 3cd243c..ea575f6 100644
--- a/drivers/pinctrl/qcom/Kconfig
+++ b/drivers/pinctrl/qcom/Kconfig
@@ -47,6 +47,14 @@ config PINCTRL_MSM8X74
This is the pinctrl, pinmux, pinconf and gpiolib driver for the
Qualcomm TLMM block found in the Qualcomm 8974 platform.
+config PINCTRL_MSM8916
+ tristate "Qualcomm 8916 pin controller driver"
+ depends on GPIOLIB && OF
+ select PINCTRL_MSM
+ help
+ This is the pinctrl, pinmux, pinconf and gpiolib driver for the
+ Qualcomm TLMM block found on the Qualcomm 8916 platform.
+
config PINCTRL_QCOM_SPMI_PMIC
tristate "Qualcomm SPMI PMIC pin controller driver"
depends on GPIOLIB && OF && SPMI
diff --git a/drivers/pinctrl/qcom/Makefile b/drivers/pinctrl/qcom/Makefile
index bfd79af..6895870 100644
--- a/drivers/pinctrl/qcom/Makefile
+++ b/drivers/pinctrl/qcom/Makefile
@@ -5,5 +5,6 @@ obj-$(CONFIG_PINCTRL_APQ8084) += pinctrl-apq8084.o
obj-$(CONFIG_PINCTRL_IPQ8064) += pinctrl-ipq8064.o
obj-$(CONFIG_PINCTRL_MSM8960) += pinctrl-msm8960.o
obj-$(CONFIG_PINCTRL_MSM8X74) += pinctrl-msm8x74.o
+obj-$(CONFIG_PINCTRL_MSM8916) += pinctrl-msm8916.o
obj-$(CONFIG_PINCTRL_QCOM_SPMI_PMIC) += pinctrl-spmi-gpio.o
obj-$(CONFIG_PINCTRL_QCOM_SPMI_PMIC) += pinctrl-spmi-mpp.o
diff --git a/drivers/pinctrl/qcom/pinctrl-msm8916.c
b/drivers/pinctrl/qcom/pinctrl-msm8916.c
new file mode 100644
index 000..0de3c06
--- /dev/null
+++ b/drivers/pinctrl/qcom/pinctrl-msm8916.c
@@ -0,0 +1,1247 @@
+/*
+ * Copyright (c) 2015, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only 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
+#include
+#include
+#include
+
+#include "pinctrl-msm.h"
+
+static const struct pinctrl_pin_desc msm8916_pins[] = {
+ PINCTRL_PIN(0, "GPIO_0"),
+ PINCTRL_PIN(1, "GPIO_1"),
+ PINCTRL_PIN(2, "GPIO_2"),
+ PINCTRL_PIN(3, "GPIO_3"),
+ PINCTRL_PIN(4, "GPIO_4"),
+ PINCTRL_PIN(5, "GPIO_5"),
+ PINCTRL_PIN(6, "GPIO_6"),
+ PINCTRL_PIN(7, "GPIO_7"),
+ PINCTRL_PIN(8, "GPIO_8"),
+ PINCTRL_PIN(9, "GPIO_9"),
+ PINCTRL_PIN(10, "GPIO_10"),
+ PINCTRL_PIN(11, "GPIO_11"),
+ PINCTRL_PIN(12, "GPIO_12"),
+ PINCTRL_PIN(13, "GPIO_13"),
+ PINCTRL_PIN(14, "GPIO_14"),
+ PINCTRL_PIN(15, "GPIO_15"),
+ PINCTRL_PIN(16, "GPIO_16"),
+ PINCTRL_PIN(17, "GPIO_17"),
+ PINCTRL_PIN(18, "GPIO_18"),
+ PINCTRL_PIN(19, "GPIO_19"),
+ PINCTRL_PIN(20, "GPIO_20"),
+ PINCTRL_PIN(21, "GPIO_21"),
+ PINCTRL_PIN(22, "GPIO_22"),
+ PINCTRL_PIN(23, "GPIO_23"),
+ PINCTRL_PIN(24, "GPIO_24"),
+ PINCTRL_PIN(25, "GPIO_25"),
+ PINCTRL_PIN(26, "GPIO_26"),
+ PINCTRL_PIN(27, "GPIO_27"),
+ PINCTRL_PIN(28, "GPIO_28"),
+ PINCTRL_PIN(29, "GPIO_29"),
+ PINCTRL_PIN(30, "GPIO_30"),
+ PINCTRL_PIN(31, "GPIO_31"),
+ PINCTRL_PIN(32, "GPIO_32"),
+ PINCTRL_PIN(33, "GPIO_33"),
+ PINCTRL_PIN(34, "GPIO_34"),
+ PINCTRL_PIN(35, "GPIO_35"),
+ PINCTRL_PIN(36, "GPIO_36"),
+ PINCTRL_PIN(37, "GPIO_37"),
+ PINCTRL_PIN(38, "GPIO_38"),
+ PINCTRL_PIN(39, "GPIO_39"),
+ PINCTRL_PIN(40, "GPIO_40"),
+ PINCTRL_PIN(41, "GPIO_41"),
+ PINCTRL_PIN(42, "GPIO_42"),
+ PINCTRL_PIN(43, "GPIO_43"),
+ PINCTRL_PIN(44, "GPIO_44"),
+ PINCTRL_PIN(45, "GPIO_45"),
+ PINCTRL_PIN(46, "GPIO_46"),
+ PINCTRL_PIN(47, "GPIO_47&
[PATCH v2 2/3] DT: pinctrl: Document Qualcomm MSM8916 pinctrl binding
Adds devicetree binding documentation. Signed-off-by: Stanimir Varbanov Reviewed-by: Bjorn Andersson --- .../bindings/pinctrl/qcom,msm8916-pinctrl.txt | 186 1 files changed, 186 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,msm8916-pinctrl.txt diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,msm8916-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/qcom,msm8916-pinctrl.txt new file mode 100644 index 000..10c2dcf --- /dev/null +++ b/Documentation/devicetree/bindings/pinctrl/qcom,msm8916-pinctrl.txt @@ -0,0 +1,186 @@ +Qualcomm MSM8916 TLMM block + +This binding describes the Top Level Mode Multiplexer block found in the +MSM8916 platform. + +- compatible: + Usage: required + Value type: + Definition: must be "qcom,msm8916-pinctrl" + +- reg: + Usage: required + Value type: + Definition: the base address and size of the TLMM register space. + +- interrupts: + Usage: required + Value type: + Definition: should specify the TLMM summary IRQ. + +- interrupt-controller: + Usage: required + Value type: + Definition: identifies this node as an interrupt controller + +- #interrupt-cells: + Usage: required + Value type: + Definition: must be 2. Specifying the pin number and flags, as defined + in + +- gpio-controller: + Usage: required + Value type: + Definition: identifies this node as a gpio controller + +- #gpio-cells: + Usage: required + Value type: + Definition: must be 2. Specifying the pin number and flags, as defined + in + +Please refer to ../gpio/gpio.txt and ../interrupt-controller/interrupts.txt for +a general description of GPIO and interrupt bindings. + +Please refer to pinctrl-bindings.txt in this directory for details of the +common pinctrl bindings used by client devices, including the meaning of the +phrase "pin configuration node". + +The pin configuration nodes act as a container for an arbitrary number of +subnodes. Each of these subnodes represents some desired configuration for a +pin, a group, or a list of pins or groups. This configuration can include the +mux function to select on those pin(s)/group(s), and various pin configuration +parameters, such as pull-up, drive strength, etc. + + +PIN CONFIGURATION NODES: + +The name of each subnode is not important; all subnodes should be enumerated +and processed purely based on their content. + +Each subnode only affects those parameters that are explicitly listed. In +other words, a subnode that lists a mux function but no pin configuration +parameters implies no information about any pin configuration parameters. +Similarly, a pin subnode that describes a pullup parameter implies no +information about e.g. the mux function. + + +The following generic properties as defined in pinctrl-bindings.txt are valid +to specify in a pin configuration subnode: + +- pins: + Usage: required + Value type: + Definition: List of gpio pins affected by the properties specified in + this subnode. Valid pins are: + gpio0-gpio121, + sdc1_clk, + sdc1_cmd, + sdc1_data + sdc2_clk, + sdc2_cmd, + sdc2_data, + qdsd_cmd, + qdsd_data0, + qdsd_data1, + qdsd_data2, + qdsd_data3 + +- function: + Usage: required + Value type: + Definition: Specify the alternative function to be configured for the + specified pins. Functions are only valid for gpio pins. + Valid values are: + adsp_ext, alsp_int, atest_bbrx0, atest_bbrx1, atest_char, atest_char0, + atest_char1, atest_char2, atest_char3, atest_combodac, atest_gpsadc0, + atest_gpsadc1, atest_tsens, atest_wlan0, atest_wlan1, backlight_en, + bimc_dte0, bimc_dte1, blsp1_spi, blsp2_spi, blsp3_spi, blsp_i2c1, + blsp_i2c2, blsp_i2c3, blsp_i2c4, blsp_i2c5, blsp_i2c6, blsp_spi1, + blsp_spi2, blsp_spi3, blsp_spi4, blsp_spi5, blsp_spi6, blsp_uart1, + blsp_uart2, blsp_uim1, blsp_uim2, cam1_rst, cam1_standby, cam_mclk, + cci_async, cci_i2c, cci_timer0, cci_timer1, cci_timer2, cdc_pdm0, + codec_mad, dbg_out, display_5v, dmic0_clk, dmic0_data, dsi_rst, + ebi0_wrcdc, euro_us, ext_lpass, flash_strobe, gcc_gp1_clk_a, + gcc_gp1_clk_b, gcc_gp2_clk_a, gcc_gp2_clk_b, gcc_gp3_clk_a, + gcc_gp3_clk_b, gpio, gsm0_tx0, gsm0_tx1, gsm1_tx0, gsm1_tx1, gyro_accl, + kpsns0, kpsns1, kpsns2, ldo_en, ldo_update, mag_int, mdp_vsync, + modem_tsync, m_voc, nav_pps, nav_tsync, pa_indicator, pbs0, pbs1, pbs2, + pri_mi2s, pri_mi2s_ws, prng_rosc, pwr_crypto_enabled_a, +
Re: [PATCH 4/4] media: venus: add PIL support
Hi Vikash,
On 05/17/2018 02:32 PM, Vikash Garodia wrote:
> This adds support to load the video firmware
> and bring ARM9 out of reset. This is useful
> for platforms which does not have trustzone
> to reset the ARM9.
>
> Signed-off-by: Vikash Garodia
> ---
> .../devicetree/bindings/media/qcom,venus.txt | 8 +-
> drivers/media/platform/qcom/venus/core.c | 67 +++--
> drivers/media/platform/qcom/venus/core.h | 6 +
> drivers/media/platform/qcom/venus/firmware.c | 163
> +
> drivers/media/platform/qcom/venus/firmware.h | 10 +-
> 5 files changed, 217 insertions(+), 37 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/media/qcom,venus.txt
> b/Documentation/devicetree/bindings/media/qcom,venus.txt
> index 00d0d1b..0ff0f2d 100644
> --- a/Documentation/devicetree/bindings/media/qcom,venus.txt
> +++ b/Documentation/devicetree/bindings/media/qcom,venus.txt
for this change in DT binding you have to cc devicetree ML. And probably
it could be separate patch.
> @@ -53,7 +53,7 @@
>
> * Subnodes
> The Venus video-codec node must contain two subnodes representing
> -video-decoder and video-encoder.
> +video-decoder and video-encoder, one optional firmware subnode.
>
> Every of video-encoder or video-decoder subnode should have:
>
> @@ -79,6 +79,8 @@ Every of video-encoder or video-decoder subnode should have:
> power domain which is responsible for collapsing
> and restoring power to the subcore.
>
> +The firmware sub node must contain the iommus specifiers for ARM9.
> +
> * An Example
> video-codec@1d0 {
> compatible = "qcom,msm8916-venus";
> @@ -105,4 +107,8 @@ Every of video-encoder or video-decoder subnode should
> have:
> clock-names = "core";
> power-domains = <&mmcc VENUS_CORE1_GDSC>;
> };
> + firmware {
venus-firmware
> + compatible = "qcom,venus-pil-no-tz";
this should be following the other subnodes compatible names:
compatible = "venus-firmware";
> + iommus = <&apps_smmu 0x10b2 0x0>;
> + }
> };
> diff --git a/drivers/media/platform/qcom/venus/core.c
> b/drivers/media/platform/qcom/venus/core.c
> index 1308488..16910558 100644
> --- a/drivers/media/platform/qcom/venus/core.c
> +++ b/drivers/media/platform/qcom/venus/core.c
> @@ -22,6 +22,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -30,6 +31,7 @@
> #include "vdec.h"
> #include "venc.h"
> #include "firmware.h"
> +#include "hfi_venus.h"
>
> static void venus_event_notify(struct venus_core *core, u32 event)
> {
> @@ -76,7 +78,7 @@ static void venus_sys_error_handler(struct work_struct
> *work)
> hfi_core_deinit(core, true);
> hfi_destroy(core);
> mutex_lock(&core->lock);
> - venus_shutdown(core->dev);
> + venus_shutdown(core);
>
> pm_runtime_put_sync(core->dev);
>
> @@ -84,7 +86,7 @@ static void venus_sys_error_handler(struct work_struct
> *work)
>
> pm_runtime_get_sync(core->dev);
>
> - ret |= venus_boot(core->dev, core->res->fwname);
> + ret |= venus_boot(core);
>
> ret |= hfi_core_resume(core, true);
>
> @@ -179,6 +181,20 @@ static u32 to_v4l2_codec_type(u32 codec)
> }
> }
>
> +static int store_firmware_dev(struct device *dev, void *data)
> +{
> + struct venus_core *core;
> +
> + core = (struct venus_core *)data;
> + if (!core)
> + return -EINVAL;
> +
> + if (of_device_is_compatible(dev->of_node, "qcom,venus-pil-no-tz"))
> + core->fw.dev = dev;
> +
> + return 0;
> +}
> +
> static int venus_enumerate_codecs(struct venus_core *core, u32 type)
> {
> const struct hfi_inst_ops dummy_ops = {};
> @@ -229,6 +245,7 @@ static int venus_probe(struct platform_device *pdev)
> struct device *dev = &pdev->dev;
> struct venus_core *core;
> struct resource *r;
> + struct iommu_domain *iommu_domain;
> int ret;
>
> core = devm_kzalloc(dev, sizeof(*core), GFP_KERNEL);
> @@ -279,7 +296,14 @@ static int venus_probe(struct platform_device *pdev)
> if (ret < 0)
> goto err_runtime_disable;
>
> - ret = venus_boot(dev, core->res->fwname);
> + ret = of_platform_populate(dev->of_node, NULL, NULL, dev);
> + if (ret)
> + goto err_runtime_disable;
> +
> + /* Attempt to register child devices */
This comment is wrong, the child devices are created by
of_platform_populate above.
> + ret = device_for_each_child(dev, core, store_firmware_dev);
Why we need these complex gymnastics to get struct device pointer when
that could be done in venus_firmware .probe method?
I think the answer is because you want to avoid having venus-firmware.ko
(because you have to have separate struct device for iommu SID). In that
case it would be be
Re: [PATCH 3/4] venus: add check to make scm calls
Hi Vikash,
On 05/17/2018 02:32 PM, Vikash Garodia wrote:
> In order to invoke scm calls, ensure that the platform
> has the required support to invoke the scm calls in
> secure world. This code is in preparation to add PIL
> functionality in venus driver.
>
> Signed-off-by: Vikash Garodia
> ---
> drivers/media/platform/qcom/venus/hfi_venus.c | 26 +++---
> 1 file changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/venus/hfi_venus.c
> b/drivers/media/platform/qcom/venus/hfi_venus.c
> index f61d34b..9bcce94 100644
> --- a/drivers/media/platform/qcom/venus/hfi_venus.c
> +++ b/drivers/media/platform/qcom/venus/hfi_venus.c
> @@ -27,6 +27,7 @@
> #include "hfi_msgs.h"
> #include "hfi_venus.h"
> #include "hfi_venus_io.h"
> +#include "firmware.h"
>
> #define HFI_MASK_QHDR_TX_TYPE0xff00
> #define HFI_MASK_QHDR_RX_TYPE0x00ff
> @@ -570,13 +571,19 @@ static int venus_halt_axi(struct venus_hfi_device *hdev)
> static int venus_power_off(struct venus_hfi_device *hdev)
> {
> int ret;
> + void __iomem *reg_base;
>
> if (!hdev->power_enabled)
> return 0;
>
> - ret = qcom_scm_set_remote_state(TZBSP_VIDEO_STATE_SUSPEND, 0);
> - if (ret)
> - return ret;
> + if (qcom_scm_is_available()) {
> + ret = qcom_scm_set_remote_state(TZBSP_VIDEO_STATE_SUSPEND, 0);
I think it will be clearer if we abstract qcom_scm_set_remote_state to
something like venus_set_state(SUSPEND|RESUME) in firmware.c and export
the functions to be used here.
--
regards,
Stan
Re: [PATCH 4/4] media: venus: add PIL support
Hi,
On 05/22/2018 04:02 PM, Stanimir Varbanov wrote:
> Hi Vikash,
>
> On 05/17/2018 02:32 PM, Vikash Garodia wrote:
>> This adds support to load the video firmware
>> and bring ARM9 out of reset. This is useful
>> for platforms which does not have trustzone
>> to reset the ARM9.
>>
>> Signed-off-by: Vikash Garodia
>> ---
>> .../devicetree/bindings/media/qcom,venus.txt | 8 +-
>> drivers/media/platform/qcom/venus/core.c | 67 +++--
>> drivers/media/platform/qcom/venus/core.h | 6 +
>> drivers/media/platform/qcom/venus/firmware.c | 163
>> +
>> drivers/media/platform/qcom/venus/firmware.h | 10 +-
>> 5 files changed, 217 insertions(+), 37 deletions(-)
>>
>>
>> -int venus_shutdown(struct device *dev)
>> +int venus_boot_noTZ(struct venus_core *core, phys_addr_t mem_phys,
>> +size_t mem_size)
>> {
>> -return qcom_scm_pas_shutdown(VENUS_PAS_ID);
>> +struct iommu_domain *iommu;
>> +struct device *dev;
>> +int ret;
>> +
>> +if (!core->fw.dev)
>> +return -EPROBE_DEFER;
>> +
>> +dev = core->fw.dev;
>> +
>> +iommu = iommu_domain_alloc(&platform_bus_type);
>> +if (!iommu) {
>> +dev_err(dev, "Failed to allocate iommu domain\n");
>> +return -ENOMEM;
>> +}
>> +
>> +iommu->geometry.aperture_start = 0x0;
>> +iommu->geometry.aperture_end = VENUS_FW_MEM_SIZE;
>
> The same comment for geometry params as for venus_probe is valid here.
Infact aperture_end will be overwritten by arm-smmu driver in the next
call to iommu_attach_device(), and by chance geometry.force_aperture
will become true.
I wonder is that geometry params are supposed to be used by drivers or
by iommu drivers?
>
>> +
>> +ret = iommu_attach_device(iommu, dev);
>> +if (ret) {
>> +dev_err(dev, "could not attach device\n");
>> +goto err_attach;
>> +}
>> +
>> +ret = iommu_map(iommu, core->fw.iova, mem_phys, mem_size,
>> +IOMMU_READ|IOMMU_WRITE|IOMMU_PRIV);
>
> iova is not initialized and is zero, maybe we don't need that variable
> in the venus_firmware structure?
>
>> +if (ret) {
>> +dev_err(dev, "could not map video firmware region\n");
>> +goto err_map;
>> +}
>> +core->fw.iommu_domain = iommu;
>> +venus_reset_hw(core);
>> +
>> +return 0;
>> +
>> +err_map:
>> +iommu_detach_device(iommu, dev);
>> +err_attach:
>> +iommu_domain_free(iommu);
>> +return ret;
>> }
>> +
--
regards,
Stan
Re: [PATCH 3/4] venus: add check to make scm calls
Hi Jordan,
On 22.05.2018 22:50, Jordan Crouse wrote:
On Tue, May 22, 2018 at 04:04:51PM +0300, Stanimir Varbanov wrote:
Hi Vikash,
On 05/17/2018 02:32 PM, Vikash Garodia wrote:
In order to invoke scm calls, ensure that the platform
has the required support to invoke the scm calls in
secure world. This code is in preparation to add PIL
functionality in venus driver.
Signed-off-by: Vikash Garodia
---
drivers/media/platform/qcom/venus/hfi_venus.c | 26 +++---
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/media/platform/qcom/venus/hfi_venus.c
b/drivers/media/platform/qcom/venus/hfi_venus.c
index f61d34b..9bcce94 100644
--- a/drivers/media/platform/qcom/venus/hfi_venus.c
+++ b/drivers/media/platform/qcom/venus/hfi_venus.c
@@ -27,6 +27,7 @@
#include "hfi_msgs.h"
#include "hfi_venus.h"
#include "hfi_venus_io.h"
+#include "firmware.h"
#define HFI_MASK_QHDR_TX_TYPE 0xff00
#define HFI_MASK_QHDR_RX_TYPE 0x00ff
@@ -570,13 +571,19 @@ static int venus_halt_axi(struct venus_hfi_device *hdev)
static int venus_power_off(struct venus_hfi_device *hdev)
{
int ret;
+ void __iomem *reg_base;
if (!hdev->power_enabled)
return 0;
- ret = qcom_scm_set_remote_state(TZBSP_VIDEO_STATE_SUSPEND, 0);
- if (ret)
- return ret;
+ if (qcom_scm_is_available()) {
+ ret = qcom_scm_set_remote_state(TZBSP_VIDEO_STATE_SUSPEND, 0);
I think it will be clearer if we abstract qcom_scm_set_remote_state to
something like venus_set_state(SUSPEND|RESUME) in firmware.c and export
the functions to be used here.
This specific function is a little odd because the SCM function got overloaded
and used as a hardware workaround for the adreno a5xx zap shader.
When we added it for the GPU we knew the day would come that we would need it
for Venus so we kept the name purposely generic. You can wrap if if you want
but just know that there are other non video entities out there using it.
Sorry I wasn't clear, by abstract it I meant to introduce a new
venus_set_state function in venus/firmware.c where we'll select
tz/non-tz functions for suspend / resume depending on the configuration.
regards,
Stan
Re: [PATCH v2] PCI: qcom: add runtime pm support to pcie_port
Hi Srini, On 05/23/2018 01:44 PM, Srinivas Kandagatla wrote: > This patch is required when the pcie controller sits on a bus with > its own power domain and clocks which are controlled via a bus driver > like simple pm bus. As these bus driver have runtime pm enabled, it makes > sense to update the usage counter so that the runtime pm does not suspend > the clks or power domain associated with the bus driver. > > Signed-off-by: Srinivas Kandagatla > Signed-off-by: Bjorn Andersson > --- > drivers/pci/dwc/pcie-qcom.c | 9 - > 1 file changed, 8 insertions(+), 1 deletion(-) Acked-by: Stanimir Varbanov -- regards, Stan
Re: [PATCH v3 2/2] drivers: soc: Add LLCC driver
Hi, On 03/27/2018 09:52 PM, Rishabh Bhatnagar wrote: > LLCC (Last Level Cache Controller) provides additional cache memory > in the system. LLCC is partitioned into muliple slices and each > slice getting its own priority, size, ID and other config parameters. > LLCC driver programs these parameters for each slice. Clients that > are assigned to use LLCC need to get information such size & ID of the > slice they get and activate or deactivate the slice as needed. LLCC driver > provides API interfaces for the clients to perform these operations. > > Signed-off-by: Channagoud Kadabi > Signed-off-by: Rishabh Bhatnagar > --- > drivers/soc/qcom/Kconfig | 16 ++ > drivers/soc/qcom/Makefile | 2 + > drivers/soc/qcom/llcc-sdm845.c | 120 ++ > drivers/soc/qcom/llcc-slice.c | 454 > + I'd name it just llcc.c, slice suffix didn't add any value. > include/linux/soc/qcom/llcc-qcom.h | 178 +++ > 5 files changed, 770 insertions(+) > create mode 100644 drivers/soc/qcom/llcc-sdm845.c > create mode 100644 drivers/soc/qcom/llcc-slice.c > create mode 100644 include/linux/soc/qcom/llcc-qcom.h > > diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig > index e050eb8..28237fc 100644 > --- a/drivers/soc/qcom/Kconfig > +++ b/drivers/soc/qcom/Kconfig > @@ -21,6 +21,22 @@ config QCOM_GSBI >functions for connecting the underlying serial UART, SPI, and I2C >devices to the output pins. > > +config QCOM_LLCC > + tristate "Qualcomm Technologies, Inc. LLCC driver" > + depends on ARCH_QCOM > + help > + Qualcomm Technologies, Inc. platform specific LLCC driver for Last > + Level Cache. This provides interfaces to client's that use the LLCC. > + Say yes here to enable LLCC slice driver. > + > +config QCOM_SDM845_LLCC > + tristate "Qualcomm Technologies, Inc. SDM845 LLCC driver" > + depends on QCOM_LLCC > + help > + Say yes here to enable the LLCC driver for SDM845. This is provides > + data required to configure LLCC so that clients can start using the > + LLCC slices. > + > config QCOM_MDT_LOADER > tristate > select QCOM_SCM > diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile > index dcebf28..e16d6a2 100644 > --- a/drivers/soc/qcom/Makefile > +++ b/drivers/soc/qcom/Makefile > @@ -12,3 +12,5 @@ obj-$(CONFIG_QCOM_SMEM_STATE) += smem_state.o > obj-$(CONFIG_QCOM_SMP2P) += smp2p.o > obj-$(CONFIG_QCOM_SMSM) += smsm.o > obj-$(CONFIG_QCOM_WCNSS_CTRL) += wcnss_ctrl.o > +obj-$(CONFIG_QCOM_LLCC) += llcc-slice.o > +obj-$(CONFIG_QCOM_SDM845_LLCC) += llcc-sdm845.o > diff --git a/drivers/soc/qcom/llcc-sdm845.c b/drivers/soc/qcom/llcc-sdm845.c > new file mode 100644 > index 000..cd431d9 > --- /dev/null > +++ b/drivers/soc/qcom/llcc-sdm845.c > @@ -0,0 +1,120 @@ > +/* Copyright (c) 2017-2018, The Linux Foundation. All rights reserved. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 and > + * only 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 > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > + > +/* > + * SCT entry contains of the following parameters > + * name: Name of the client's use case for which the llcc slice is used > + * uid: Unique id for the client's use case > + * slice_id: llcc slice id for each client > + * max_cap: The maximum capacity of the cache slice provided in KB > + * priority: Priority of the client used to select victim line for > replacement > + * fixed_size: Determine of the slice has a fixed capacity > + * bonus_ways: Bonus ways to be used by any slice, bonus way is used only if > + * it't not a reserved way. > + * res_ways: Reserved ways for the cache slice, the reserved ways cannot be > used > + * by any other client than the one its assigned to. > + * cache_mode: Each slice operates as a cache, this controls the mode of the > + * slice normal or TCM > + * probe_target_ways: Determines what ways to probe for access hit. When > + *configured to 1 only bonus and reseved ways are probed. > + *when configured to 0 all ways in llcc are probed. > + * dis_cap_alloc: Disable capacity based allocation for a client > + * retain_on_pc: If this bit is set and client has maitained active vote > + * then the ways assigned to this client are not flushed on > power > + * collapse. > + * activate_on_init: Activate the slice immidiately after the SCT is
Re: [PATCH v3 1/2] Documentation: Documentation for qcom, llcc
Hi,
On 03/27/2018 09:52 PM, Rishabh Bhatnagar wrote:
> Documentation for last level cache controller device tree bindings,
> client bindings usage examples.
>
> Signed-off-by: Channagoud Kadabi
> Signed-off-by: Rishabh Bhatnagar
> ---
> .../devicetree/bindings/arm/msm/qcom,llcc.txt | 70
> ++
> 1 file changed, 70 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt
>
> diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt
> b/Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt
> new file mode 100644
> index 000..ceb20a4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/msm/qcom,llcc.txt
> @@ -0,0 +1,70 @@
> +== Introduction==
> +
> +LLCC (Last Level Cache Controller) driver is implemented as a platform
> device.
> +The driver Programs the SCT (system configuration table). The SCT programming
> +divides the system cache into slices. Each slice is assigned an ID A.K.A
> +SCID(Sub-cache ID).
> +HW modules that are designated to use the system cache are known as clients.
> +Each client must also be represented as a node in the device tree just like
> +any other hw module.
> +One client can have multiple SCID's assigned meaning each client could get
> +multiple slices in the cache. Client can use the slices for various
> pre-defined
> +usecases. Each client defines a set of names for these usecases in its
> +device tree binding.
> +Client makes a request to LLCC device to get cache-slices properties for each
> +of its usecase. Client gets the information like cache slice ID and size of
> the
> +cache slice.
All this describes the driver, please just describe what is LLCC and for
what it used for.
> +
> +== llcc device ==
drop this.
> +
> +Properties:
> +- compatible:
> + Usage: required
> + Value type:
> + Definition: must be "qcom,sdm855-llcc"
> +
> +- reg:
> + Usage: required
> + Value Type:
> + Definition: must be addresses and sizes of the LLCC registers
> +
> +- #cache-cells:
> + Usage: required
> + Value Type:
> + Definition: Number of cache cells, must be 1
> +
> +- max-slices:
> + usage: required
> + Value Type:
> + Definition: Number of cache slices supported by hardware
> +
> +Example:
> +
> + llcc: qcom,sdm855-llcc@0110 {
this should be qcom,llcc@110
> + compatible = "qcom,sdm845-llcc";
> + reg = <0x0110 0x25>;
> + #cache-cells = <1>;
> + max-slices = <32>;
> + };
> +
> +== Client ==
> +
> +Required properties:
> +- cache-slice-names:
in my opinion this shouldn't be required but optional.
> + Usage: required
> + Value type:
> + Definition: A set of names that identify the usecase names of a client
> that
> + uses cache slice. These strings are used to look up the
> cache slice
> + entries by name.
> +
> +- cache-slices:
> + Usage: required
> + Value type:
> + Definition: The tuple has phandle to llcc device as the first argument
> and
> + the second argument is the usecase id of the client.
> +For Example:
> +
> + venus {
> + cache-slice-names = "vidsc0", "vidsc1";
> + cache-slices = <&llcc 2>, <&llcc 3>;
please make 2,3 and so on #defines.
--
regards,
Stan
Re: [PATCH 26/28] venus: implementing multi-stream support
Hi Vikash,
Please write the comments for the chunk of code for which they are refer to.
On 2.05.2018 10:40, Vikash Garodia wrote:
Hello Stanimir,
On 2018-04-24 18:14, Stanimir Varbanov wrote:
This is implementing a multi-stream decoder support. The multi
stream gives an option to use the secondary decoder output
with different raw format (or the same in case of crop).
Signed-off-by: Stanimir Varbanov
---
drivers/media/platform/qcom/venus/core.h | 1 +
drivers/media/platform/qcom/venus/helpers.c | 204
+++-
drivers/media/platform/qcom/venus/helpers.h | 6 +
drivers/media/platform/qcom/venus/vdec.c | 91 -
drivers/media/platform/qcom/venus/venc.c | 1 +
5 files changed, 299 insertions(+), 4 deletions(-)
diff --git a/drivers/media/platform/qcom/venus/core.h
b/drivers/media/platform/qcom/venus/core.h
index 4d6c05f156c4..85e66e2dd672 100644
--- a/drivers/media/platform/qcom/venus/core.h
+++ b/drivers/media/platform/qcom/venus/core.h
@@ -259,6 +259,7 @@ struct venus_inst {
struct list_head list;
struct mutex lock;
struct venus_core *core;
+ struct list_head dpbbufs;
struct list_head internalbufs;
struct list_head registeredbufs;
struct list_head delayed_process;
The dpb buffers queued to hardware will be returned back to host either
during flush
or when the session is stopped. Host should not send these buffers to
client.
That's correct.
vdec_buf_done should be handling in a way to drop dpb buffers from
sending to client.
That is also correct, vdec_buf_done is trying to find the buffer by
index from a list of queued buffers from v4l2 clients. See
venus_helper_vb2_buf_queue where it is calling v4l2_m2m_buf_queue.
So for the dpb buffers venus_helper_find_buf() will return NULL.
regards,
Stan
[PATCH v2 05/29] venus: hfi: support session continue for 4xx version
This makes possible to handle session_continue for 4xx as well.
Signed-off-by: Stanimir Varbanov
---
drivers/media/platform/qcom/venus/hfi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/platform/qcom/venus/hfi.c
b/drivers/media/platform/qcom/venus/hfi.c
index bca894a00c07..cbc6fad05e47 100644
--- a/drivers/media/platform/qcom/venus/hfi.c
+++ b/drivers/media/platform/qcom/venus/hfi.c
@@ -312,7 +312,7 @@ int hfi_session_continue(struct venus_inst *inst)
{
struct venus_core *core = inst->core;
- if (core->res->hfi_version != HFI_VERSION_3XX)
+ if (core->res->hfi_version == HFI_VERSION_1XX)
return 0;
return core->ops->session_continue(inst);
--
2.14.1
[PATCH v2 01/29] venus: hfi_msgs: correct pointer increment
Data pointer should be incremented by size of the structure not the size of a pointer, correct the mistake. Signed-off-by: Stanimir Varbanov --- drivers/media/platform/qcom/venus/hfi_msgs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/qcom/venus/hfi_msgs.c b/drivers/media/platform/qcom/venus/hfi_msgs.c index 90c93d9603dc..589e1a6b36a9 100644 --- a/drivers/media/platform/qcom/venus/hfi_msgs.c +++ b/drivers/media/platform/qcom/venus/hfi_msgs.c @@ -60,14 +60,14 @@ static void event_seq_changed(struct venus_core *core, struct venus_inst *inst, frame_sz = (struct hfi_framesize *)data_ptr; event.width = frame_sz->width; event.height = frame_sz->height; - data_ptr += sizeof(frame_sz); + data_ptr += sizeof(*frame_sz); break; case HFI_PROPERTY_PARAM_PROFILE_LEVEL_CURRENT: data_ptr += sizeof(u32); profile_level = (struct hfi_profile_level *)data_ptr; event.profile = profile_level->profile; event.level = profile_level->level; - data_ptr += sizeof(profile_level); + data_ptr += sizeof(*profile_level); break; default: break; -- 2.14.1
