* [PATCH v6 0/4] support reserving crashkernel above 4G on arm64 kdump
From: Chen Zhou @ 2019-08-30 7:11 UTC (permalink / raw)
To: tglx, mingo, catalin.marinas, will, james.morse, dyoung, bhsharma
Cc: Chen Zhou, kexec, linux-kernel, horms, guohanjun,
linux-arm-kernel
I am busy with other things, so it was a long time before this version was
released.
This patch series enable reserving crashkernel above 4G in arm64.
There are following issues in arm64 kdump:
1. We use crashkernel=X to reserve crashkernel below 4G, which will fail
when there is no enough low memory.
2. Currently, crashkernel=Y@X can be used to reserve crashkernel above 4G,
in this case, if swiotlb or DMA buffers are requierd, crash dump kernel
will boot failure because there is no low memory available for allocation.
To solve these issues, introduce crashkernel=X,low to reserve specified
size low memory.
Crashkernel=X tries to reserve memory for the crash dump kernel under
4G. If crashkernel=Y,low is specified simultaneously, reserve spcified
size low memory for crash kdump kernel devices firstly and then reserve
memory above 4G.
When crashkernel is reserved above 4G in memory, that is, crashkernel=X,low
is specified simultaneously, kernel should reserve specified size low memory
for crash dump kernel devices. So there may be two crash kernel regions, one
is below 4G, the other is above 4G.
In order to distinct from the high region and make no effect to the use of
kexec-tools, rename the low region as "Crash kernel (low)", and add DT property
"linux,low-memory-range" to crash dump kernel's dtb to pass the low region.
Besides, we need to modify kexec-tools:
arm64: kdump: add another DT property to crash dump kernel's dtb(see [1])
The previous changes and discussions can be retrieved from:
Changes since [v5]
- Move reserve_crashkernel_low() into kernel/crash_core.c.
- Delete crashkernel=X,high.
- Modify crashkernel=X,low.
If crashkernel=X,low is specified simultaneously, reserve spcified size low
memory for crash kdump kernel devices firstly and then reserve memory above 4G.
In addition, rename crashk_low_res as "Crash kernel (low)" for arm64, and then
pass to crash dump kernel by DT property "linux,low-memory-range".
- Update Documentation/admin-guide/kdump/kdump.rst.
Changes since [v4]
- Reimplement memblock_cap_memory_ranges for multiple ranges by Mike.
Changes since [v3]
- Add memblock_cap_memory_ranges back for multiple ranges.
- Fix some compiling warnings.
Changes since [v2]
- Split patch "arm64: kdump: support reserving crashkernel above 4G" as
two. Put "move reserve_crashkernel_low() into kexec_core.c" in a separate
patch.
Changes since [v1]:
- Move common reserve_crashkernel_low() code into kernel/kexec_core.c.
- Remove memblock_cap_memory_ranges() i added in v1 and implement that
in fdt_enforce_memory_region().
There are at most two crash kernel regions, for two crash kernel regions
case, we cap the memory range [min(regs[*].start), max(regs[*].end)]
and then remove the memory range in the middle.
[1]: http://lists.infradead.org/pipermail/kexec/2019-August/023569.html
[v1]: https://lkml.org/lkml/2019/4/2/1174
[v2]: https://lkml.org/lkml/2019/4/9/86
[v3]: https://lkml.org/lkml/2019/4/9/306
[v4]: https://lkml.org/lkml/2019/4/15/273
[v5]: https://lkml.org/lkml/2019/5/6/1360
Chen Zhou (4):
x86: kdump: move reserve_crashkernel_low() into crash_core.c
arm64: kdump: reserve crashkenel above 4G for crash dump kernel
arm64: kdump: add memory for devices by DT property, low-memory-range
kdump: update Documentation about crashkernel on arm64
Documentation/admin-guide/kdump/kdump.rst | 13 ++++-
Documentation/admin-guide/kernel-parameters.txt | 12 ++++-
arch/arm64/include/asm/kexec.h | 3 ++
arch/arm64/kernel/setup.c | 8 ++-
arch/arm64/mm/init.c | 61 +++++++++++++++++++++--
arch/x86/include/asm/kexec.h | 3 ++
arch/x86/kernel/setup.c | 65 +++----------------------
include/linux/crash_core.h | 4 ++
include/linux/kexec.h | 1 -
kernel/crash_core.c | 65 +++++++++++++++++++++++++
10 files changed, 168 insertions(+), 67 deletions(-)
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v6 2/4] arm64: kdump: reserve crashkenel above 4G for crash dump kernel
From: Chen Zhou @ 2019-08-30 7:11 UTC (permalink / raw)
To: tglx, mingo, catalin.marinas, will, james.morse, dyoung, bhsharma
Cc: Chen Zhou, kexec, linux-kernel, horms, guohanjun,
linux-arm-kernel
In-Reply-To: <20190830071200.56169-1-chenzhou10@huawei.com>
Crashkernel=X tries to reserve memory for the crash dump kernel under
4G. If crashkernel=X,low is specified simultaneously, reserve spcified
size low memory for crash kdump kernel devices firstly and then reserve
memory above 4G.
Signed-off-by: Chen Zhou <chenzhou10@huawei.com>
---
arch/arm64/include/asm/kexec.h | 3 +++
arch/arm64/kernel/setup.c | 8 +++++++-
arch/arm64/mm/init.c | 31 +++++++++++++++++++++++++++++--
3 files changed, 39 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/include/asm/kexec.h b/arch/arm64/include/asm/kexec.h
index 12a561a..88279a9 100644
--- a/arch/arm64/include/asm/kexec.h
+++ b/arch/arm64/include/asm/kexec.h
@@ -25,6 +25,9 @@
#define KEXEC_ARCH KEXEC_ARCH_AARCH64
+/* 2M alignment for crash kernel regions */
+#define CRASH_ALIGN SZ_2M
+
#ifndef __ASSEMBLY__
/**
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index 9c4bad7..2ead608 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -231,7 +231,13 @@ static void __init request_standard_resources(void)
kernel_data.end <= res->end)
request_resource(res, &kernel_data);
#ifdef CONFIG_KEXEC_CORE
- /* Userspace will find "Crash kernel" region in /proc/iomem. */
+ /*
+ * Userspace will find "Crash kernel" region in /proc/iomem.
+ * Note: the low region is renamed as Crash kernel (low).
+ */
+ if (crashk_low_res.end && crashk_low_res.start >= res->start &&
+ crashk_low_res.end <= res->end)
+ request_resource(res, &crashk_low_res);
if (crashk_res.end && crashk_res.start >= res->start &&
crashk_res.end <= res->end)
request_resource(res, &crashk_res);
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index f3c7952..c99f845 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -64,6 +64,7 @@ static void __init reserve_crashkernel(void)
{
unsigned long long crash_base, crash_size;
int ret;
+ phys_addr_t crash_max = ARCH_LOW_ADDRESS_LIMIT;
ret = parse_crashkernel(boot_command_line, memblock_phys_mem_size(),
&crash_size, &crash_base);
@@ -71,12 +72,38 @@ static void __init reserve_crashkernel(void)
if (ret || !crash_size)
return;
+ ret = reserve_crashkernel_low();
+ if (!ret && crashk_low_res.end) {
+ /*
+ * If crashkernel=X,low specified, there may be two regions,
+ * we need to make some changes as follows:
+ *
+ * 1. rename the low region as "Crash kernel (low)"
+ * In order to distinct from the high region and make no effect
+ * to the use of existing kexec-tools, rename the low region as
+ * "Crash kernel (low)".
+ *
+ * 2. change the upper bound for crash memory
+ * Set MEMBLOCK_ALLOC_ACCESSIBLE upper bound for crash memory.
+ *
+ * 3. mark the low region as "nomap"
+ * The low region is intended to be used for crash dump kernel
+ * devices, just mark the low region as "nomap" simply.
+ */
+ const char *rename = "Crash kernel (low)";
+
+ crashk_low_res.name = rename;
+ crash_max = MEMBLOCK_ALLOC_ACCESSIBLE;
+ memblock_mark_nomap(crashk_low_res.start,
+ resource_size(&crashk_low_res));
+ }
+
crash_size = PAGE_ALIGN(crash_size);
if (crash_base == 0) {
/* Current arm64 boot protocol requires 2MB alignment */
- crash_base = memblock_find_in_range(0, ARCH_LOW_ADDRESS_LIMIT,
- crash_size, SZ_2M);
+ crash_base = memblock_find_in_range(0, crash_max, crash_size,
+ SZ_2M);
if (crash_base == 0) {
pr_warn("cannot allocate crashkernel (size:0x%llx)\n",
crash_size);
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v6 3/4] arm64: kdump: add memory for devices by DT property, low-memory-range
From: Chen Zhou @ 2019-08-30 7:11 UTC (permalink / raw)
To: tglx, mingo, catalin.marinas, will, james.morse, dyoung, bhsharma
Cc: Chen Zhou, kexec, linux-kernel, horms, guohanjun,
linux-arm-kernel
In-Reply-To: <20190830071200.56169-1-chenzhou10@huawei.com>
If we want to reserve crashkernel above 4G, we could use parameters
"crashkernel=X crashkernel=Y,low", in this case, specified size low
memory is reserved for crash dump kernel devices and never mapped by
the first kernel. This memory range is advertised to crash dump kernel
via DT property under /chosen,
linux,low-memory-range=<BASE SIZE>
Crash dump kernel reads this property at boot time and call
memblock_add() after memblock_cap_memory_range() has been called.
Signed-off-by: Chen Zhou <chenzhou10@huawei.com>
---
arch/arm64/mm/init.c | 30 +++++++++++++++++++++++++++++-
1 file changed, 29 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index c99f845..a376b18 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -294,6 +294,26 @@ static int __init early_mem(char *p)
}
early_param("mem", early_mem);
+static int __init early_init_dt_scan_lowmem(unsigned long node,
+ const char *uname, int depth, void *data)
+{
+ struct memblock_region *lowmem = data;
+ const __be32 *reg;
+ int len;
+
+ if (depth != 1 || strcmp(uname, "chosen") != 0)
+ return 0;
+
+ reg = of_get_flat_dt_prop(node, "linux,low-memory-range", &len);
+ if (!reg || (len < (dt_root_addr_cells + dt_root_size_cells)))
+ return 1;
+
+ lowmem->base = dt_mem_next_cell(dt_root_addr_cells, ®);
+ lowmem->size = dt_mem_next_cell(dt_root_size_cells, ®);
+
+ return 1;
+}
+
static int __init early_init_dt_scan_usablemem(unsigned long node,
const char *uname, int depth, void *data)
{
@@ -324,13 +344,21 @@ static void __init fdt_enforce_memory_region(void)
if (reg.size)
memblock_cap_memory_range(reg.base, reg.size);
+
+ of_scan_flat_dt(early_init_dt_scan_lowmem, ®);
+
+ if (reg.size)
+ memblock_add(reg.base, reg.size);
}
void __init arm64_memblock_init(void)
{
const s64 linear_region_size = -(s64)PAGE_OFFSET;
- /* Handle linux,usable-memory-range property */
+ /*
+ * Handle linux,usable-memory-range and linux,low-memory-range
+ * properties.
+ */
fdt_enforce_memory_region();
/* Remove memory above our supported physical address size */
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v6 4/4] kdump: update Documentation about crashkernel on arm64
From: Chen Zhou @ 2019-08-30 7:12 UTC (permalink / raw)
To: tglx, mingo, catalin.marinas, will, james.morse, dyoung, bhsharma
Cc: Chen Zhou, kexec, linux-kernel, horms, guohanjun,
linux-arm-kernel
In-Reply-To: <20190830071200.56169-1-chenzhou10@huawei.com>
Now we support crashkernel=X,[low] on arm64, update the Documentation.
Signed-off-by: Chen Zhou <chenzhou10@huawei.com>
---
Documentation/admin-guide/kdump/kdump.rst | 13 +++++++++++--
Documentation/admin-guide/kernel-parameters.txt | 12 +++++++++++-
2 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/Documentation/admin-guide/kdump/kdump.rst b/Documentation/admin-guide/kdump/kdump.rst
index ac7e131..e55173e 100644
--- a/Documentation/admin-guide/kdump/kdump.rst
+++ b/Documentation/admin-guide/kdump/kdump.rst
@@ -299,7 +299,13 @@ Boot into System Kernel
"crashkernel=64M@16M" tells the system kernel to reserve 64 MB of memory
starting at physical address 0x01000000 (16MB) for the dump-capture kernel.
- On x86 and x86_64, use "crashkernel=64M@16M".
+ On x86 use "crashkernel=64M@16M".
+
+ On x86_64, use "crashkernel=Y[@X]" to select a region under 4G first, and
+ fall back to reserve region above 4G when '@offset' hasn't been specified.
+ We can also use "crashkernel=X,high" to select a region above 4G, which
+ also tries to allocate at least 256M below 4G automatically and
+ "crashkernel=Y,low" can be used to allocate specified size low memory.
On ppc64, use "crashkernel=128M@32M".
@@ -316,8 +322,11 @@ Boot into System Kernel
kernel will automatically locate the crash kernel image within the
first 512MB of RAM if X is not given.
- On arm64, use "crashkernel=Y[@X]". Note that the start address of
+ On arm64, use "crashkernel=Y[@X]". Note that the start address of
the kernel, X if explicitly specified, must be aligned to 2MiB (0x200000).
+ If crashkernel=Z,low is specified simultaneously, reserve spcified size
+ low memory for crash kdump kernel devices firstly and then reserve memory
+ above 4G.
Load the Dump-capture Kernel
============================
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 4c19719..069a122 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -708,6 +708,9 @@
[KNL, x86_64] select a region under 4G first, and
fall back to reserve region above 4G when '@offset'
hasn't been specified.
+ [KNL, arm64] If crashkernel=X,low is specified, reserve
+ spcified size low memory for crash kdump kernel devices
+ firstly, and then reserve memory above 4G.
See Documentation/admin-guide/kdump/kdump.rst for further details.
crashkernel=range1:size1[,range2:size2,...][@offset]
@@ -732,12 +735,19 @@
requires at least 64M+32K low memory, also enough extra
low memory is needed to make sure DMA buffers for 32-bit
devices won't run out. Kernel would try to allocate at
- at least 256M below 4G automatically.
+ least 256M below 4G automatically.
This one let user to specify own low range under 4G
for second kernel instead.
0: to disable low allocation.
It will be ignored when crashkernel=X,high is not used
or memory reserved is below 4G.
+ [KNL, arm64] range under 4G.
+ This one let user to specify own low range under 4G
+ for crash dump kernel instead.
+ Different with x86_64, kernel allocates specified size
+ physical memory region only when this parameter is specified
+ instead of trying to allocate at least 256M below 4G
+ automatically.
cryptomgr.notests
[KNL] Disable crypto self-tests
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* AW: [PATCH] ARM: dts: imx7: fix USB controller 'size' parameter
From: Thomas Schaefer @ 2019-08-30 7:11 UTC (permalink / raw)
To: Peter Chen
Cc: marex@denx.de, shawnguo@kernel.org, s.hauer@pengutronix.de,
dl-linux-imx, kernel@pengutronix.de, festevam@gmail.com,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190830022539.GA1146@b29397-desktop>
-----Ursprüngliche Nachricht-----
Von: Peter Chen <peter.chen@nxp.com>
Gesendet: Freitag, 30. August 2019 04:26
> On 19-08-29 17:49:13, Thomas Schaefer wrote:
> > Currently the size parameter in the reg property of usbotg and usbh
> > nodes in imx7s and imx7d dts includes is set to 0x200 which is wrong
> > for the i.MX7 CPU. According to reference manual, spacing of USB
> > controller registers is 0x10000 instead.
> >
> > This patch will fix this and set the 'size' to 0x10000.
> >
> > Signed-off-by: Thomas Schaefer <thomas.schaefer@kontron.com>
> > ---
> > arch/arm/boot/dts/imx7d.dtsi | 2 +-
> > arch/arm/boot/dts/imx7s.dtsi | 4 ++--
> > 2 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/imx7d.dtsi
> > b/arch/arm/boot/dts/imx7d.dtsi index 42528d2812a2..f1b098d28b6e 100644
> > --- a/arch/arm/boot/dts/imx7d.dtsi
> > +++ b/arch/arm/boot/dts/imx7d.dtsi
> > @@ -117,7 +117,7 @@
> > &aips3 {
> > usbotg2: usb@30b20000 {
> > compatible = "fsl,imx7d-usb", "fsl,imx27-usb";
> > - reg = <0x30b20000 0x200>;
> > + reg = <0x30b20000 0x10000>;
> > interrupts = <GIC_SPI 42 IRQ_TYPE_LEVEL_HIGH>;
> > clocks = <&clks IMX7D_USB_CTRL_CLK>;
> > fsl,usbphy = <&usbphynop2>;
> > diff --git a/arch/arm/boot/dts/imx7s.dtsi
> > b/arch/arm/boot/dts/imx7s.dtsi index c1a4fff5ceda..9e25fccf33f0 100644
> > --- a/arch/arm/boot/dts/imx7s.dtsi
> > +++ b/arch/arm/boot/dts/imx7s.dtsi
> > @@ -1088,7 +1088,7 @@
> >
> > usbotg1: usb@30b10000 {
> > compatible = "fsl,imx7d-usb", "fsl,imx27-usb";
> > - reg = <0x30b10000 0x200>;
> > + reg = <0x30b10000 0x10000>;
> > interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
> > clocks = <&clks IMX7D_USB_CTRL_CLK>;
> > fsl,usbphy = <&usbphynop1>;
> > @@ -1099,7 +1099,7 @@
> >
> > usbh: usb@30b30000 {
> > compatible = "fsl,imx7d-usb", "fsl,imx27-usb";
> > - reg = <0x30b30000 0x200>;
> > + reg = <0x30b30000 0x10000>;
> > interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>;
> > clocks = <&clks IMX7D_USB_CTRL_CLK>;
> > fsl,usbphy = <&usbphynop3>;
>
> Hi Thomos,
>
> The core controller range is 0x200, from offset 0x200, it is non-core register, which is used by usbmisc. Fabio said you met problem at u-boot for this size, what's the problem about it?
>
> Thanks,
> Peter
Hi Peter,
When porting one of our i.MX7 based modules to u-boot v2019.07, I found that scanning USB with 'usb start' crashes when trying to scan the _second_ controller enabled in the device tree (the first controller was detected properly). After some investigation I found that the problem was introduced with Marek Vasuts 'usb: ehci-mx6: Fix bus enumeration for DM case' (u-boot commit 501547cec1f7f0438cae388a104ff60f18576c01). This patch uses the 'reg' property in the usbotg and usbh nodes to calculate the device index number for the driver.
Actually, controller range on i.MX6 is 0x200, thus the calculation is correct for i.MX6, but on i.MX7 the base addresses of the controller registers differ by 0x10000 and calculation will fail if more than one USB controller is enabled in the device tree. This is why I suggested to fix this problem in the imx7s and imx7d device tree include files.
Added Marek to this thread.
Best regards,
Thomas
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v2 08/11] phy: phy-mtk-tphy: make the ref clock optional
From: Chunfeng Yun @ 2019-08-30 7:14 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Rob Herring
Cc: Mark Rutland, devicetree, linux-kernel, Chunfeng Yun,
linux-mediatek, Matthias Brugger, linux-arm-kernel
In-Reply-To: <1567149298-29366-1-git-send-email-chunfeng.yun@mediatek.com>
Sometimes the reference clock of USB3 PHY comes from oscillator
directly, and no need refer to a fixed-clock in DTS anymore
if make it optional.
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
v2: no changes
---
drivers/phy/mediatek/phy-mtk-tphy.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/phy/mediatek/phy-mtk-tphy.c b/drivers/phy/mediatek/phy-mtk-tphy.c
index 4a2dc92f10f5..96c62e3a3300 100644
--- a/drivers/phy/mediatek/phy-mtk-tphy.c
+++ b/drivers/phy/mediatek/phy-mtk-tphy.c
@@ -1182,7 +1182,7 @@ static int mtk_tphy_probe(struct platform_device *pdev)
if (tphy->u3phya_ref)
continue;
- instance->ref_clk = devm_clk_get(&phy->dev, "ref");
+ instance->ref_clk = devm_clk_get_optional(&phy->dev, "ref");
if (IS_ERR(instance->ref_clk)) {
dev_err(dev, "failed to get ref_clk(id-%d)\n", port);
retval = PTR_ERR(instance->ref_clk);
--
2.23.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v2 02/11] dt-bindings: phy-mtk-tphy: make the ref clock optional
From: Chunfeng Yun @ 2019-08-30 7:14 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Rob Herring
Cc: Mark Rutland, devicetree, linux-kernel, Chunfeng Yun,
linux-mediatek, Matthias Brugger, linux-arm-kernel
In-Reply-To: <1567149298-29366-1-git-send-email-chunfeng.yun@mediatek.com>
Make the ref clock optional, then we no need refer to a fixed-clock
in DTS anymore when the clock of USB3 PHY comes from oscillator
directly
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
v2: no changes
---
.../devicetree/bindings/phy/phy-mtk-tphy.txt | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/Documentation/devicetree/bindings/phy/phy-mtk-tphy.txt b/Documentation/devicetree/bindings/phy/phy-mtk-tphy.txt
index ce6abfbdfbe1..1f4a36dd80e0 100644
--- a/Documentation/devicetree/bindings/phy/phy-mtk-tphy.txt
+++ b/Documentation/devicetree/bindings/phy/phy-mtk-tphy.txt
@@ -34,12 +34,6 @@ Optional properties (controller (parent) node):
Required properties (port (child) node):
- reg : address and length of the register set for the port.
-- clocks : a list of phandle + clock-specifier pairs, one for each
- entry in clock-names
-- clock-names : must contain
- "ref": 48M reference clock for HighSpeed analog phy; and 26M
- reference clock for SuperSpeed analog phy, sometimes is
- 24M, 25M or 27M, depended on platform.
- #phy-cells : should be 1 (See second example)
cell after port phandle is phy type from:
- PHY_TYPE_USB2
@@ -48,6 +42,13 @@ Required properties (port (child) node):
- PHY_TYPE_SATA
Optional properties (PHY_TYPE_USB2 port (child) node):
+- clocks : a list of phandle + clock-specifier pairs, one for each
+ entry in clock-names
+- clock-names : may contain
+ "ref": 48M reference clock for HighSpeed anolog phy; and 26M
+ reference clock for SuperSpeed anolog phy, sometimes is
+ 24M, 25M or 27M, depended on platform.
+
- mediatek,eye-src : u32, the value of slew rate calibrate
- mediatek,eye-vrt : u32, the selection of VRT reference voltage
- mediatek,eye-term : u32, the selection of HS_TX TERM reference voltage
--
2.23.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v2 03/11] dt-bindings: phy-mtk-tphy: remove unused u3phya_ref clock
From: Chunfeng Yun @ 2019-08-30 7:14 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Rob Herring
Cc: Mark Rutland, devicetree, linux-kernel, Chunfeng Yun,
linux-mediatek, Matthias Brugger, linux-arm-kernel
In-Reply-To: <1567149298-29366-1-git-send-email-chunfeng.yun@mediatek.com>
The u3phya_ref clock is already moved into sub-node, and
renamed as ref clock, no used anymore now, so remove it
to avoid confusion
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
v2: add Reviewed-by Rob
---
Documentation/devicetree/bindings/phy/phy-mtk-tphy.txt | 4 ----
1 file changed, 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/phy/phy-mtk-tphy.txt b/Documentation/devicetree/bindings/phy/phy-mtk-tphy.txt
index 1f4a36dd80e0..48bc1a2e9299 100644
--- a/Documentation/devicetree/bindings/phy/phy-mtk-tphy.txt
+++ b/Documentation/devicetree/bindings/phy/phy-mtk-tphy.txt
@@ -13,10 +13,6 @@ Required properties (controller (parent) node):
"mediatek,mt8173-u3phy";
make use of "mediatek,generic-tphy-v1" on mt2701 instead and
"mediatek,generic-tphy-v2" on mt2712 instead.
- - clocks : (deprecated, use port's clocks instead) a list of phandle +
- clock-specifier pairs, one for each entry in clock-names
- - clock-names : (deprecated, use port's one instead) must contain
- "u3phya_ref": for reference clock of usb3.0 analog phy.
Required nodes : a sub-node is required for each port the controller
provides. Address range information including the usual
--
2.23.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v2 05/11] dt-bindings: phy-mtk-tphy: add the properties about address mapping
From: Chunfeng Yun @ 2019-08-30 7:14 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Rob Herring
Cc: Mark Rutland, devicetree, linux-kernel, Chunfeng Yun,
linux-mediatek, Matthias Brugger, linux-arm-kernel
In-Reply-To: <1567149298-29366-1-git-send-email-chunfeng.yun@mediatek.com>
Add three required properties about the address mapping, including
'#address-cells', '#size-cells' and 'ranges'
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
v2: add Reviewed-by Rob
---
Documentation/devicetree/bindings/phy/phy-mtk-tphy.txt | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/Documentation/devicetree/bindings/phy/phy-mtk-tphy.txt b/Documentation/devicetree/bindings/phy/phy-mtk-tphy.txt
index a859b0db4051..dd75b676b71d 100644
--- a/Documentation/devicetree/bindings/phy/phy-mtk-tphy.txt
+++ b/Documentation/devicetree/bindings/phy/phy-mtk-tphy.txt
@@ -14,6 +14,16 @@ Required properties (controller (parent) node):
make use of "mediatek,generic-tphy-v1" on mt2701 instead and
"mediatek,generic-tphy-v2" on mt2712 instead.
+- #address-cells: the number of cells used to represent physical
+ base addresses.
+- #size-cells: the number of cells used to represent the size of an address.
+- ranges: the address mapping relationship to the parent, defined with
+ - empty value: if optional 'reg' is used.
+ - non-empty value: if optional 'reg' is not used. should set
+ the child's base address to 0, the physical address
+ within parent's address space, and the length of
+ the address map.
+
Required nodes : a sub-node is required for each port the controller
provides. Address range information including the usual
'reg' property is used inside these nodes to describe
--
2.23.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v2 06/11] phy: phy-mtk-tphy: add a property for disconnect threshold
From: Chunfeng Yun @ 2019-08-30 7:14 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Rob Herring
Cc: Mark Rutland, devicetree, linux-kernel, Chunfeng Yun,
linux-mediatek, Matthias Brugger, linux-arm-kernel
In-Reply-To: <1567149298-29366-1-git-send-email-chunfeng.yun@mediatek.com>
This is used to tune the threshold of disconnect
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
v2: no changes
---
drivers/phy/mediatek/phy-mtk-tphy.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/phy/mediatek/phy-mtk-tphy.c b/drivers/phy/mediatek/phy-mtk-tphy.c
index cb2ed3b25068..5afe33621dbc 100644
--- a/drivers/phy/mediatek/phy-mtk-tphy.c
+++ b/drivers/phy/mediatek/phy-mtk-tphy.c
@@ -60,6 +60,8 @@
#define U3P_USBPHYACR6 0x018
#define PA6_RG_U2_BC11_SW_EN BIT(23)
#define PA6_RG_U2_OTG_VBUSCMP_EN BIT(20)
+#define PA6_RG_U2_DISCTH GENMASK(7, 4)
+#define PA6_RG_U2_DISCTH_VAL(x) ((0xf & (x)) << 4)
#define PA6_RG_U2_SQTH GENMASK(3, 0)
#define PA6_RG_U2_SQTH_VAL(x) (0xf & (x))
@@ -300,6 +302,7 @@ struct mtk_phy_instance {
int eye_src;
int eye_vrt;
int eye_term;
+ int discth;
bool bc12_en;
};
@@ -850,9 +853,12 @@ static void phy_parse_property(struct mtk_tphy *tphy,
&instance->eye_vrt);
device_property_read_u32(dev, "mediatek,eye-term",
&instance->eye_term);
- dev_dbg(dev, "bc12:%d, src:%d, vrt:%d, term:%d\n",
+ device_property_read_u32(dev, "mediatek,discth",
+ &instance->discth);
+ dev_dbg(dev, "bc12:%d, src:%d, vrt:%d, term:%d, disc:%d\n",
instance->bc12_en, instance->eye_src,
- instance->eye_vrt, instance->eye_term);
+ instance->eye_vrt, instance->eye_term,
+ instance->discth);
}
static void u2_phy_props_set(struct mtk_tphy *tphy,
@@ -888,6 +894,13 @@ static void u2_phy_props_set(struct mtk_tphy *tphy,
tmp |= PA1_RG_TERM_SEL_VAL(instance->eye_term);
writel(tmp, com + U3P_USBPHYACR1);
}
+
+ if (instance->discth) {
+ tmp = readl(com + U3P_USBPHYACR6);
+ tmp &= ~PA6_RG_U2_DISCTH;
+ tmp |= PA6_RG_U2_DISCTH_VAL(instance->discth);
+ writel(tmp, com + U3P_USBPHYACR6);
+ }
}
static int mtk_phy_init(struct phy *phy)
--
2.23.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v2 04/11] dt-bindings: phy-mtk-tphy: add a new reference clock
From: Chunfeng Yun @ 2019-08-30 7:14 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Rob Herring
Cc: Mark Rutland, devicetree, linux-kernel, Chunfeng Yun,
linux-mediatek, Matthias Brugger, linux-arm-kernel
In-Reply-To: <1567149298-29366-1-git-send-email-chunfeng.yun@mediatek.com>
Usually the digital and analog phys use the same reference clock,
but on some platforms, they are separated, so add another optional
clock to support it.
In order to keep the clock names consistent with PHY IP's, use
the da_ref for analog phy and ref clock for digital phy.
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
v2: fix typo of analog and needed
---
Documentation/devicetree/bindings/phy/phy-mtk-tphy.txt | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/phy/phy-mtk-tphy.txt b/Documentation/devicetree/bindings/phy/phy-mtk-tphy.txt
index 48bc1a2e9299..a859b0db4051 100644
--- a/Documentation/devicetree/bindings/phy/phy-mtk-tphy.txt
+++ b/Documentation/devicetree/bindings/phy/phy-mtk-tphy.txt
@@ -41,9 +41,12 @@ Optional properties (PHY_TYPE_USB2 port (child) node):
- clocks : a list of phandle + clock-specifier pairs, one for each
entry in clock-names
- clock-names : may contain
- "ref": 48M reference clock for HighSpeed anolog phy; and 26M
- reference clock for SuperSpeed anolog phy, sometimes is
+ "ref": 48M reference clock for HighSpeed (digital) phy; and 26M
+ reference clock for SuperSpeed (digital) phy, sometimes is
24M, 25M or 27M, depended on platform.
+ "da_ref": the reference clock of analog phy, used if the clocks
+ of analog and digital phys are separated, otherwise uses
+ "ref" clock only if needed.
- mediatek,eye-src : u32, the value of slew rate calibrate
- mediatek,eye-vrt : u32, the selection of VRT reference voltage
--
2.23.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v2 07/11] phy: phy-mtk-tphy: add a property for internal resistance
From: Chunfeng Yun @ 2019-08-30 7:14 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Rob Herring
Cc: Mark Rutland, devicetree, linux-kernel, Chunfeng Yun,
linux-mediatek, Matthias Brugger, linux-arm-kernel
In-Reply-To: <1567149298-29366-1-git-send-email-chunfeng.yun@mediatek.com>
This is used to tune internal resistance for J-K test
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
v2: no changes
---
drivers/phy/mediatek/phy-mtk-tphy.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/phy/mediatek/phy-mtk-tphy.c b/drivers/phy/mediatek/phy-mtk-tphy.c
index 5afe33621dbc..4a2dc92f10f5 100644
--- a/drivers/phy/mediatek/phy-mtk-tphy.c
+++ b/drivers/phy/mediatek/phy-mtk-tphy.c
@@ -43,6 +43,8 @@
#define PA0_RG_USB20_INTR_EN BIT(5)
#define U3P_USBPHYACR1 0x004
+#define PA1_RG_INTR_CAL GENMASK(23, 19)
+#define PA1_RG_INTR_CAL_VAL(x) ((0x1f & (x)) << 19)
#define PA1_RG_VRT_SEL GENMASK(14, 12)
#define PA1_RG_VRT_SEL_VAL(x) ((0x7 & (x)) << 12)
#define PA1_RG_TERM_SEL GENMASK(10, 8)
@@ -302,6 +304,7 @@ struct mtk_phy_instance {
int eye_src;
int eye_vrt;
int eye_term;
+ int intr;
int discth;
bool bc12_en;
};
@@ -853,12 +856,14 @@ static void phy_parse_property(struct mtk_tphy *tphy,
&instance->eye_vrt);
device_property_read_u32(dev, "mediatek,eye-term",
&instance->eye_term);
+ device_property_read_u32(dev, "mediatek,intr",
+ &instance->intr);
device_property_read_u32(dev, "mediatek,discth",
&instance->discth);
- dev_dbg(dev, "bc12:%d, src:%d, vrt:%d, term:%d, disc:%d\n",
+ dev_dbg(dev, "bc12:%d, src:%d, vrt:%d, term:%d, intr:%d, disc:%d\n",
instance->bc12_en, instance->eye_src,
instance->eye_vrt, instance->eye_term,
- instance->discth);
+ instance->intr, instance->discth);
}
static void u2_phy_props_set(struct mtk_tphy *tphy,
@@ -895,6 +900,13 @@ static void u2_phy_props_set(struct mtk_tphy *tphy,
writel(tmp, com + U3P_USBPHYACR1);
}
+ if (instance->intr) {
+ tmp = readl(com + U3P_USBPHYACR1);
+ tmp &= ~PA1_RG_INTR_CAL;
+ tmp |= PA1_RG_INTR_CAL_VAL(instance->intr);
+ writel(tmp, com + U3P_USBPHYACR1);
+ }
+
if (instance->discth) {
tmp = readl(com + U3P_USBPHYACR6);
tmp &= ~PA6_RG_U2_DISCTH;
--
2.23.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v2 10/11] phy: phy-mtk-tphy: add a new reference clock
From: Chunfeng Yun @ 2019-08-30 7:14 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Rob Herring
Cc: Mark Rutland, devicetree, linux-kernel, Chunfeng Yun,
linux-mediatek, Matthias Brugger, linux-arm-kernel
In-Reply-To: <1567149298-29366-1-git-send-email-chunfeng.yun@mediatek.com>
Usually the digital and analog phys use the same reference clock,
but some platforms have two separate reference clocks for each of
them, so add another optional clock to support them.
In order to keep the clock names consistent with PHY IP's, change
the da_ref for analog phy and ref clock for digital phy.
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
v2: fix typo of analog
---
drivers/phy/mediatek/phy-mtk-tphy.c | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/phy/mediatek/phy-mtk-tphy.c b/drivers/phy/mediatek/phy-mtk-tphy.c
index c6424fd2a06d..cdbcc49f7115 100644
--- a/drivers/phy/mediatek/phy-mtk-tphy.c
+++ b/drivers/phy/mediatek/phy-mtk-tphy.c
@@ -298,7 +298,8 @@ struct mtk_phy_instance {
struct u2phy_banks u2_banks;
struct u3phy_banks u3_banks;
};
- struct clk *ref_clk; /* reference clock of anolog phy */
+ struct clk *ref_clk; /* reference clock of (digital) phy */
+ struct clk *da_ref_clk; /* reference clock of analog phy */
u32 index;
u8 type;
int eye_src;
@@ -925,6 +926,13 @@ static int mtk_phy_init(struct phy *phy)
return ret;
}
+ ret = clk_prepare_enable(instance->da_ref_clk);
+ if (ret) {
+ dev_err(tphy->dev, "failed to enable da_ref\n");
+ clk_disable_unprepare(instance->ref_clk);
+ return ret;
+ }
+
switch (instance->type) {
case PHY_TYPE_USB2:
u2_phy_instance_init(tphy, instance);
@@ -984,6 +992,7 @@ static int mtk_phy_exit(struct phy *phy)
u2_phy_instance_exit(tphy, instance);
clk_disable_unprepare(instance->ref_clk);
+ clk_disable_unprepare(instance->da_ref_clk);
return 0;
}
@@ -1170,6 +1179,14 @@ static int mtk_tphy_probe(struct platform_device *pdev)
retval = PTR_ERR(instance->ref_clk);
goto put_child;
}
+
+ instance->da_ref_clk =
+ devm_clk_get_optional(&phy->dev, "da_ref");
+ if (IS_ERR(instance->da_ref_clk)) {
+ dev_err(dev, "failed to get da_ref_clk(id-%d)\n", port);
+ retval = PTR_ERR(instance->da_ref_clk);
+ goto put_child;
+ }
}
provider = devm_of_phy_provider_register(dev, mtk_phy_xlate);
--
2.23.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v2 01/11] dt-bindings: phy-mtk-tphy: add two optional properties for u2phy
From: Chunfeng Yun @ 2019-08-30 7:14 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Rob Herring
Cc: Mark Rutland, devicetree, linux-kernel, Chunfeng Yun,
linux-mediatek, Matthias Brugger, linux-arm-kernel
Add two optional properties, one for tuning J-K voltage, another for
disconnect threshold, both of them are related with connect detection
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
v2: change description
---
Documentation/devicetree/bindings/phy/phy-mtk-tphy.txt | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/phy/phy-mtk-tphy.txt b/Documentation/devicetree/bindings/phy/phy-mtk-tphy.txt
index a5f7a4f0dbc1..ce6abfbdfbe1 100644
--- a/Documentation/devicetree/bindings/phy/phy-mtk-tphy.txt
+++ b/Documentation/devicetree/bindings/phy/phy-mtk-tphy.txt
@@ -52,6 +52,8 @@ Optional properties (PHY_TYPE_USB2 port (child) node):
- mediatek,eye-vrt : u32, the selection of VRT reference voltage
- mediatek,eye-term : u32, the selection of HS_TX TERM reference voltage
- mediatek,bc12 : bool, enable BC12 of u2phy if support it
+- mediatek,discth : u32, the selection of disconnect threshold
+- mediatek,intr : u32, the selection of internal R (resistance)
Example:
--
2.23.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v2 09/11] phy: phy-mtk-tphy: remove unused u3phya_ref clock
From: Chunfeng Yun @ 2019-08-30 7:14 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Rob Herring
Cc: Mark Rutland, devicetree, linux-kernel, Chunfeng Yun,
linux-mediatek, Matthias Brugger, linux-arm-kernel
In-Reply-To: <1567149298-29366-1-git-send-email-chunfeng.yun@mediatek.com>
The u3phya_ref clock is already moved into sub-node, and
renamed as ref clock, no used anymore now, so remove it,
this can avoid confusion when support new platforms
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
v2: no changes
---
drivers/phy/mediatek/phy-mtk-tphy.c | 18 ------------------
1 file changed, 18 deletions(-)
diff --git a/drivers/phy/mediatek/phy-mtk-tphy.c b/drivers/phy/mediatek/phy-mtk-tphy.c
index 96c62e3a3300..c6424fd2a06d 100644
--- a/drivers/phy/mediatek/phy-mtk-tphy.c
+++ b/drivers/phy/mediatek/phy-mtk-tphy.c
@@ -312,8 +312,6 @@ struct mtk_phy_instance {
struct mtk_tphy {
struct device *dev;
void __iomem *sif_base; /* only shared sif */
- /* deprecated, use @ref_clk instead in phy instance */
- struct clk *u3phya_ref; /* reference clock of usb3 anolog phy */
const struct mtk_phy_pdata *pdata;
struct mtk_phy_instance **phys;
int nphys;
@@ -921,12 +919,6 @@ static int mtk_phy_init(struct phy *phy)
struct mtk_tphy *tphy = dev_get_drvdata(phy->dev.parent);
int ret;
- ret = clk_prepare_enable(tphy->u3phya_ref);
- if (ret) {
- dev_err(tphy->dev, "failed to enable u3phya_ref\n");
- return ret;
- }
-
ret = clk_prepare_enable(instance->ref_clk);
if (ret) {
dev_err(tphy->dev, "failed to enable ref_clk\n");
@@ -992,7 +984,6 @@ static int mtk_phy_exit(struct phy *phy)
u2_phy_instance_exit(tphy, instance);
clk_disable_unprepare(instance->ref_clk);
- clk_disable_unprepare(tphy->u3phya_ref);
return 0;
}
@@ -1127,11 +1118,6 @@ static int mtk_tphy_probe(struct platform_device *pdev)
}
}
- /* it's deprecated, make it optional for backward compatibility */
- tphy->u3phya_ref = devm_clk_get_optional(dev, "u3phya_ref");
- if (IS_ERR(tphy->u3phya_ref))
- return PTR_ERR(tphy->u3phya_ref);
-
tphy->src_ref_clk = U3P_REF_CLK;
tphy->src_coef = U3P_SLEW_RATE_COEF;
/* update parameters of slew rate calibrate if exist */
@@ -1178,10 +1164,6 @@ static int mtk_tphy_probe(struct platform_device *pdev)
phy_set_drvdata(phy, instance);
port++;
- /* if deprecated clock is provided, ignore instance's one */
- if (tphy->u3phya_ref)
- continue;
-
instance->ref_clk = devm_clk_get_optional(&phy->dev, "ref");
if (IS_ERR(instance->ref_clk)) {
dev_err(dev, "failed to get ref_clk(id-%d)\n", port);
--
2.23.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v2 11/11] arm64: dts: mt2712: use non-empty ranges for usb-phy
From: Chunfeng Yun @ 2019-08-30 7:14 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Rob Herring
Cc: Mark Rutland, devicetree, linux-kernel, Chunfeng Yun,
linux-mediatek, Matthias Brugger, linux-arm-kernel
In-Reply-To: <1567149298-29366-1-git-send-email-chunfeng.yun@mediatek.com>
Use non-empty ranges for usb-phy to make the layout of
its registers clearer;
Replace deprecated compatible by generic
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
v2: use generic compatible
---
arch/arm64/boot/dts/mediatek/mt2712e.dtsi | 42 ++++++++++++-----------
1 file changed, 22 insertions(+), 20 deletions(-)
diff --git a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
index 43307bad3f0d..e24f2f2f6004 100644
--- a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
@@ -697,30 +697,31 @@
};
u3phy0: usb-phy@11290000 {
- compatible = "mediatek,mt2712-u3phy";
- #address-cells = <2>;
- #size-cells = <2>;
- ranges;
+ compatible = "mediatek,mt2712-tphy",
+ "mediatek,generic-tphy-v2";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0 0 0x11290000 0x9000>;
status = "okay";
- u2port0: usb-phy@11290000 {
- reg = <0 0x11290000 0 0x700>;
+ u2port0: usb-phy@0 {
+ reg = <0x0 0x700>;
clocks = <&clk26m>;
clock-names = "ref";
#phy-cells = <1>;
status = "okay";
};
- u2port1: usb-phy@11298000 {
- reg = <0 0x11298000 0 0x700>;
+ u2port1: usb-phy@8000 {
+ reg = <0x8000 0x700>;
clocks = <&clk26m>;
clock-names = "ref";
#phy-cells = <1>;
status = "okay";
};
- u3port0: usb-phy@11298700 {
- reg = <0 0x11298700 0 0x900>;
+ u3port0: usb-phy@8700 {
+ reg = <0x8700 0x900>;
clocks = <&clk26m>;
clock-names = "ref";
#phy-cells = <1>;
@@ -760,30 +761,31 @@
};
u3phy1: usb-phy@112e0000 {
- compatible = "mediatek,mt2712-u3phy";
- #address-cells = <2>;
- #size-cells = <2>;
- ranges;
+ compatible = "mediatek,mt2712-tphy",
+ "mediatek,generic-tphy-v2";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0 0 0x112e0000 0x9000>;
status = "okay";
- u2port2: usb-phy@112e0000 {
- reg = <0 0x112e0000 0 0x700>;
+ u2port2: usb-phy@0 {
+ reg = <0x0 0x700>;
clocks = <&clk26m>;
clock-names = "ref";
#phy-cells = <1>;
status = "okay";
};
- u2port3: usb-phy@112e8000 {
- reg = <0 0x112e8000 0 0x700>;
+ u2port3: usb-phy@8000 {
+ reg = <0x8000 0x700>;
clocks = <&clk26m>;
clock-names = "ref";
#phy-cells = <1>;
status = "okay";
};
- u3port1: usb-phy@112e8700 {
- reg = <0 0x112e8700 0 0x900>;
+ u3port1: usb-phy@8700 {
+ reg = <0x8700 0x900>;
clocks = <&clk26m>;
clock-names = "ref";
#phy-cells = <1>;
--
2.23.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v5 1/2] dt-bindings: mailbox: add binding doc for the ARM SMC/HVC mailbox
From: Jassi Brar @ 2019-08-30 7:21 UTC (permalink / raw)
To: Peng Fan
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org,
f.fainelli@gmail.com, andre.przywara@arm.com,
linux-kernel@vger.kernel.org, robh+dt@kernel.org, dl-linux-imx,
sudeep.holla@arm.com, linux-arm-kernel@lists.infradead.org
In-Reply-To: <AM0PR04MB4481785CABB44A8C71CFB8D788BD0@AM0PR04MB4481.eurprd04.prod.outlook.com>
On Fri, Aug 30, 2019 at 1:28 AM Peng Fan <peng.fan@nxp.com> wrote:
> > > +examples:
> > > + - |
> > > + sram@910000 {
> > > + compatible = "mmio-sram";
> > > + reg = <0x0 0x93f000 0x0 0x1000>;
> > > + #address-cells = <1>;
> > > + #size-cells = <1>;
> > > + ranges = <0 0x0 0x93f000 0x1000>;
> > > +
> > > + cpu_scp_lpri: scp-shmem@0 {
> > > + compatible = "arm,scmi-shmem";
> > > + reg = <0x0 0x200>;
> > > + };
> > > +
> > > + cpu_scp_hpri: scp-shmem@200 {
> > > + compatible = "arm,scmi-shmem";
> > > + reg = <0x200 0x200>;
> > > + };
> > > + };
> > > +
> > > + firmware {
> > > + smc_mbox: mailbox {
> > > + #mbox-cells = <1>;
> > > + compatible = "arm,smc-mbox";
> > > + method = "smc";
> > > + arm,num-chans = <0x2>;
> > > + transports = "mem";
> > > + /* Optional */
> > > + arm,func-ids = <0xc20000fe>, <0xc20000ff>;
> > >
> > SMC/HVC is synchronously(block) running in "secure mode", i.e, there can
> > only be one instance running platform wide. Right?
>
> I think there could be channel for TEE, and channel for Linux.
> For virtualization case, there could be dedicated channel for each VM.
>
I am talking from Linux pov. Functions 0xfe and 0xff above, can't both
be active at the same time, right?
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC PATCH V2 4/6] platform: mtk-isp: Add Mediatek DIP driver
From: Tomasz Figa @ 2019-08-30 7:14 UTC (permalink / raw)
To: Frederic Chen
Cc: Kurt.Yang, Shih-Fang.Chuang,
laurent.pinchart+renesas@ideasonboard.com,
Rynn Wu (吳育恩),
Allan Yang (楊智鈞), suleiman@chromium.org,
Jerry-ch Chen (陳敬憲),
Jungo Lin (林明俊), hans.verkuil@cisco.com,
Hung-Wen Hsieh (謝鴻文),
linux-media@vger.kernel.org, devicetree@vger.kernel.org,
Holmes Chiou (邱挺), shik@chromium.org,
yuzhao@chromium.org, linux-mediatek@lists.infradead.org,
matthias.bgg@gmail.com, mchehab@kernel.org,
linux-arm-kernel@lists.infradead.org,
Sean Cheng (鄭昇弘), srv_heupstream,
Sj Huang (黃信璋),
Christie Yu (游雅惠), zwisler@chromium.org,
Alan-YL.Hu
In-Reply-To: <1566875772.10064.99.camel@mtksdccf07>
Hi Frederic,
On Tue, Aug 27, 2019 at 12:16 PM Frederic Chen
<frederic.chen@mediatek.com> wrote:
>
> Dear Tomasz,
>
> I appreciate your comment. I will collaborate more closely with Jungo
> to solve the common issues in DIP and Pass 1(CAM) drivers.
>
Thank you!
Also thanks for replying to all the comments, it's very helpful.
Please check my replies inline. I've snipped out the parts that I
don't have further comments on.
>
> On Wed, 2019-07-31 at 15:10 +0800, Tomasz Figa wrote:
> > Hi Frederic,
> >
> > On Mon, Jul 08, 2019 at 07:04:58PM +0800, frederic.chen@mediatek.com wrote:
[snip]
> >
> > > + dev_buf->vbb.vb2_buf.timestamp =
> > > + in_buf->vbb.vb2_buf.timestamp;
> > > +
> > > + vb2_buffer_done(&dev_buf->vbb.vb2_buf, vbf_state);
> > > +
> > > + node = mtk_dip_vbq_to_node(dev_buf->vbb.vb2_buf.vb2_queue);
> > > + spin_lock(&node->buf_list_lock);
> > > + list_del(&dev_buf->list);
> > > + spin_unlock(&node->buf_list_lock);
> > > +
> > > + dev_dbg(&pipe->dip_dev->pdev->dev,
> > > + "%s:%s: return buf, idx(%d), state(%d)\n",
> > > + pipe->desc->name, node->desc->name,
> > > + dev_buf->vbb.vb2_buf.index, vbf_state);
> > > + }
> >
> > This looks almost the same as what's being done inside
> > mtk_dip_hw_streamoff(). Could we just call this function from the loop
> > there?
>
> I would like to call the function from mtk_dip_hw_streamoff(). The only
> difference is mtk_dip_pipe_job_finish() also remove the buffer from the
> node's internal list.
>
Would anything wrong happen if we also remove the buffer from the
node's internal list in mtk_dip_hw_streamoff()?
Actually, do we need that internal node list? If we have a list of
requests and each request stores its buffer, wouldn't that be enough?
[snip]
> >
> > > +
> > > + width = ALIGN(width, 4);
> > > + stride = DIV_ROUND_UP(width * 3, 2);
> > > + stride = DIV_ROUND_UP(stride * pixel_byte, 8);
> > > +
> > > + if (pix_fmt == V4L2_PIX_FMT_MTISP_F10)
> > > + stride = ALIGN(stride, 4);
> > > + else
> > > + stride = ALIGN(stride, 8);
> >
> > Could you explain all the calculations done above?
>
> If the buffer comes from mtk-cam-p1, the stride setting must be the same
> as p1. I would like to re-implement the codes following p1's design in
> v4 patch as following:
>
> static u32
> mtk_dip_pass1_cal_pack_stride(u32 width, u32 pix_fmt) {
> unsigned int bpl;
> unsigned int pixel_bits =
> get_pixel_byte_by_fmt(mp->pixelformat);
>
> /* Bayer encoding format, P1 HW needs 2 bytes alignment */
> bpl = ALIGN(DIV_ROUND_UP(width * pixel_bits, 8), 2);
>
> /* The setting also needs 4 bytes alignment for DIP HW */
> return ALIGN(bpl, 4);;
If we need 4 bytes alignment, wouldn't the bytes per line value end up
different from P1 anyway? Perhaps we can just remove the ALIGN(..., 2)
above and keep this one? It should be the userspace responsibility
anyway to choose a format compatible with both consumer and producer.
By the way, double semicolon in the line above. :)
> }
>
>
> static __u32
> mtk_dip_pass1_cal_main_stride(u32 width, u32 pix_fmt)
> {
> unsigned int bpl, ppl;
> unsigned int pixel_bits =
> get_pixel_byte_by_fmt(mp->pixelformat);
>
> /*
> * The FULL-G encoding format
> * 1 G component per pixel
> * 1 R component per 4 pixel
> * 1 B component per 4 pixel
> * Total 4G/1R/1B in 4 pixel (pixel per line:ppl)
> */
> ppl = DIV_ROUND_UP(width * 6, 4);
> bpl = DIV_ROUND_UP(ppl * pixel_bits, 8);
>
> /* 4 bytes alignment for 10 bit & others are 8 bytes */
> if (pixel_bits == 10)
> bpl = ALIGN(bpl, 4);
> else
> bpl = ALIGN(bpl, 8);
> }
Spurious }.
>
> return bpl;
> }
>
Looks good to me, thanks!
>
> >
> > > +
> > > + return stride;
> > > +}
> > > +
> > > +static int is_stride_need_to_align(u32 format, u32 *need_aligned_fmts,
> > > + int length)
> > > +{
> > > + int i;
> > > +
> > > + for (i = 0; i < length; i++) {
> > > + if (format == need_aligned_fmts[i])
> > > + return true;
> > > + }
> > > +
> > > + return false;
> > > +}
> > > +
> > > +/* Stride that is accepted by MDP HW */
> > > +static u32 dip_mdp_fmt_get_stride(struct v4l2_pix_format_mplane *mfmt,
> > > + const struct mtk_dip_dev_format *fmt,
> > > + unsigned int plane)
> > > +{
> > > + enum mdp_color c = fmt->mdp_color;
> > > + u32 bytesperline = (mfmt->width * fmt->row_depth[plane]) / 8;
> > > + u32 stride = (bytesperline * MDP_COLOR_BITS_PER_PIXEL(c))
> > > + / fmt->row_depth[0];
> > > +
> > > + if (plane == 0)
> > > + return stride;
> > > +
> > > + if (plane < MDP_COLOR_GET_PLANE_COUNT(c)) {
> > > + if (MDP_COLOR_IS_BLOCK_MODE(c))
> > > + stride = stride / 2;
> > > + return stride;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +/* Stride that is accepted by MDP HW of format with contiguous planes */
> > > +static u32
> > > +dip_mdp_fmt_get_stride_contig(const struct mtk_dip_dev_format *fmt,
> > > + u32 pix_stride,
> > > + unsigned int plane)
> > > +{
> > > + enum mdp_color c = fmt->mdp_color;
> > > + u32 stride = pix_stride;
> > > +
> > > + if (plane == 0)
> > > + return stride;
> > > +
> > > + if (plane < MDP_COLOR_GET_PLANE_COUNT(c)) {
> > > + stride = stride >> MDP_COLOR_GET_H_SUBSAMPLE(c);
> > > + if (MDP_COLOR_IS_UV_COPLANE(c) && !MDP_COLOR_IS_BLOCK_MODE(c))
> > > + stride = stride * 2;
> > > + return stride;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +/* Plane size that is accepted by MDP HW */
> > > +static u32
> > > +dip_mdp_fmt_get_plane_size(const struct mtk_dip_dev_format *fmt,
> > > + u32 stride, u32 height,
> > > + unsigned int plane)
> > > +{
> > > + enum mdp_color c = fmt->mdp_color;
> > > + u32 bytesperline;
> > > +
> > > + bytesperline = (stride * fmt->row_depth[0])
> > > + / MDP_COLOR_BITS_PER_PIXEL(c);
> >
> > Hmm, stride and bytesperline should be exactly the same thing. Could you
> > explain what's happening here?
>
> The stride here is specific for MDP hardware (which uses the same MDP
> stride setting for NV12 and NV12M):
>
> bytesperline = width * row_depth / 8
> MDP stride = width * MDP_COLOR_BITS_PER_PIXEL /8
>
> Therfore,
>
> bytesperline = MDP stride * row_depth / MDP_COLOR_BITS_PER_PIXEL
> MDP stride = bytesperline * MDP_COLOR_BITS_PER_PIXEL/ row_depth
>
I'm sorry I'm still confused. Is there an intermediate buffer between
DIP and MDP that has stride of |MDP stride| and then MDP writes to the
final buffer that has the stride of |bytesperline|?
[snip]
> >
> > > + u32 sizeimage;
> > > +
> > > + if (bpl < min_bpl)
> > > + bpl = min_bpl;
> > > +
> > > + sizeimage = (bpl * mfmt_to_fill->height * dev_fmt->depth[i])
> > > + / dev_fmt->row_depth[i];
> >
> > Shouldn't this be bpl * fmt->height?
>
> Row_depth is the bits of the pixel.
> Depth means the bytes per pixel of the image format.
>
> For example,
> Image: 640 * 480
> YV12: row_depth = 8, depth = 12
YV12 has 3 planes of 8 bits per pixel. Not sure where does this 12 come from.
> Bytes per line = width * row_depth / 8 = 640 * 8/ 8 = 640
> Image size = Bytes per line * height * (depth/ row_depth)
> = 640 * 480 * 1.5
>
I think we might be having some terminology issue here. "row" is
normally the same as "line", which consists of |width| pixels +
padding, which is |bytesperline| bytes in total.
Perhaps you want to store a bits_per_pixel[] and vertical and
horizontal subsampling arrays for all planes of the formats in the
format descriptor.
By the way, have you considered using the V4L2 format helpers [1]?
[1] https://elixir.bootlin.com/linux/v5.3-rc6/source/drivers/media/v4l2-core/v4l2-common.c#L561
> >
> > > + dev_dbg(&pipe->dip_dev->pdev->dev,
> > > + "Non-contiguous-mp-buf(%s),total-plane-size(%d),dma_port(%d)\n",
> > > + buf_name,
> > > + total_plane_size,
> > > + b->usage);
> > > + return 0;
> > > + }
> > > +
> > > + for (; i < MDP_COLOR_GET_PLANE_COUNT(b->format.colorformat); ++i) {
> >
> > I don't see these MDP_ macros defined anywhere. Please don't use macros
> > defined from other drivers. We can embed this information in the
> > mtk_dip_dev_format struct. One would normally call it num_cplanes (color
> > planes).
> >
> > However, I would just make this driver support M formats only and forget
> > about formats with only memory planes count != color planes count.
>
> Since the non-M formats are still used by 8183's user lib now, may I add
> num_cplanes and support the both formats?
>
Okay, let's keep them for now.
[snip]
> > > +
> > > + /* Tuning buffer */
> > > + dev_buf_tuning =
> > > + pipe_job_info->buf_map[MTK_DIP_VIDEO_NODE_ID_TUNING_OUT];
> > > + if (dev_buf_tuning) {
> > > + dev_dbg(&pdev->dev,
> > > + "Tuning buf queued: scp_daddr(%pad),isp_daddr(%pad)\n",
> > > + &dev_buf_tuning->scp_daddr[0],
> > > + &dev_buf_tuning->isp_daddr[0]);
> > > + dip_param->tuning_data.pa =
> > > + (uint32_t)dev_buf_tuning->scp_daddr[0];
> > > + dip_param->tuning_data.present = true;
> > > + dip_param->tuning_data.iova =
> > > + (uint32_t)dev_buf_tuning->isp_daddr[0];
> >
> > Why are these casts needed?
>
> This structure is shared between CPU and scp and the pa and iova is
> defined as 32bits fields.
>
> struct tuning_addr {
> u32 present;
> u32 pa; /* Used by CM4 access */
> u32 iova; /* Used by IOMMU HW access */
> } __attribute__ ((__packed__));
>
I see, thanks.
[snip]
> >
> > > +#define DIP_COMPOSING_MAX_NUM 3
> > > +#define DIP_MAX_ERR_COUNT 188U
> > > +#define DIP_FLUSHING_WQ_TIMEOUT (16U * DIP_MAX_ERR_COUNT)
> >
> > Any rationale behind this particular numbers? Please add comments explaining
> > them.
>
> I would like to adjust the time out value to 1000 / 30 *
> (DIP_COMPOSING_MAX_NUM) * 3.
>
> To wait 3 times of expected frame time (@30fps) in the worst case (max
> number of jobs in SCP).
>
With high system load it could be still possible to hit this. Since it
should normally be impossible to hit this timeout on a system working
correctly, how about just making this something really long like 1
second?
[snip]
> >
> > > +
> > > + dev_dbg(&dip_dev->pdev->dev,
> > > + "%s: wakeup frame_no(%d),num(%d)\n",
> > > + __func__, req->img_fparam.frameparam.frame_no,
> > > + atomic_read(&dip_hw->num_composing));
> > > +
> > > + buf = req->working_buf;
> > > + mtk_dip_wbuf_to_ipi_img_addr(&req->img_fparam.frameparam.subfrm_data,
> > > + &buf->buffer);
> > > + memset(buf->buffer.vaddr, 0, DIP_SUB_FRM_SZ);
> > > + mtk_dip_wbuf_to_ipi_img_sw_addr(&req->img_fparam.frameparam.config_data,
> > > + &buf->config_data);
> > > + memset(buf->config_data.vaddr, 0, DIP_COMP_SZ);
> > > +
> > > + if (!req->img_fparam.frameparam.tuning_data.present) {
> > > + /*
> > > + * When user enqueued without tuning buffer,
> > > + * it would use driver internal buffer.
> > > + */
> > > + dev_dbg(&dip_dev->pdev->dev,
> > > + "%s: frame_no(%d) has no tuning_data\n",
> > > + __func__, req->img_fparam.frameparam.frame_no);
> > > +
> > > + mtk_dip_wbuf_to_ipi_tuning_addr
> > > + (&req->img_fparam.frameparam.tuning_data,
> > > + &buf->tuning_buf);
> > > + memset(buf->tuning_buf.vaddr, 0, DIP_TUNING_SZ);
> > > + }
> > > +
> > > + req->img_fparam.frameparam.state = FRAME_STATE_COMPOSING;
> > > + mtk_dip_wbuf_to_ipi_img_sw_addr(&req->img_fparam.frameparam.self_data,
> > > + &buf->frameparam);
> > > + memcpy(buf->frameparam.vaddr, &req->img_fparam.frameparam,
> > > + sizeof(req->img_fparam.frameparam));
> >
> > Is img_fparam really used at this stage? I can only see ipi_param passed to
> > the IPI.
>
> The content of img_fparam is passed to SCP.
>
> The dip frame information is saved in req->img_fparam.frameparam (in
> mtk_dip_pipe_ipi_params_config()).
>
> The content of req->img_fparam.frameparam is copied to
> buf->frameparam.vaddr.
>
> Since we set ipi_param.frm_param.pa to the buf->frameparam.scp_daddr in
> mtk_dip_wbuf_to_ipi_img_sw_addr(), the content of img_fparam is pass to
> SCP through ipi_param.
Okay, I see. Thanks,
[snip]
> >
> > > + } else {
> > > + for (i = 0; i < *num_planes; i++) {
> > > + if (sizes[i] < fmt->fmt.pix_mp.plane_fmt[i].sizeimage) {
> > > + dev_dbg(&pipe->dip_dev->pdev->dev,
> > > + "%s:%s:%s: invalid buf: %u < %u\n",
> > > + __func__, pipe->desc->name,
> > > + node->desc->name, sizes[0], size);
> > > + return -EINVAL;
> > > + }
> > > + sizes[i] = fmt->fmt.pix_mp.plane_fmt[i].sizeimage;
> >
> > For VIDIOC_CREATEBUFS we also need to handle the case when *num_planes > 0
> > and then we need to honor the values already present in sizes[]. (Note that
> > the code above overrides *num_planes to 1, so we lose the information. The
> > code needs to be restructured to handle that.)
>
> We overrides *num_planes when *num_planes is 0. Is the modification I
> need to do to keep the original value of size[]?
Yes.
>
> if (!*num_planes) {
> *num_planes = 1;
> sizes[0] = fmt->fmt.pix_mp.plane_fmt[0].sizeimage;
> }
>
[snip]
> >
> > > + dev_dbg(&pipe->dip_dev->pdev->dev,
> > > + "%s:%s:%s cnt_nodes_not_streaming(%d), is_pipe_streamon(%d)\n",
> > > + __func__, pipe->desc->name, node->desc->name, count,
> > > + is_pipe_streamon);
> > > +
> > > + if (count && is_pipe_streamon) {
> >
> > For v4l2_subdev_call() shouldn't this be !count && is_pipe_streamon?
>
> Do you mean that pipe->subdev's stop stream should be called after all
> video device are stream off, not the first video device's stream off?
>
Partially. See the comment below. We should stop the subdev when the
first video node stops streaming, but the media pipeline only when all
the nodes stopped.
> >
> > > + ret = v4l2_subdev_call(&pipe->subdev, video, s_stream, 0);
> > > + if (ret)
> > > + dev_err(&pipe->dip_dev->pdev->dev,
> > > + "%s:%s: sub dev s_stream(0) failed(%d)\n",
> > > + pipe->desc->name, node->desc->name, ret);
> > > + media_pipeline_stop(&node->vdev.entity);
> >
> > We should do this one when the last node stops streaming to solve the
> > enable state locking issue as described in my comment to _start_streaming().
>
> I will do this when the last node stops streaming.
>
Ack.
> >
> > > + }
> > > +
> > > + mtk_dip_return_all_buffers(pipe, node, VB2_BUF_STATE_ERROR);
> > > +}
> > > +
> > > +static int mtk_dip_videoc_querycap(struct file *file, void *fh,
> > > + struct v4l2_capability *cap)
> > > +{
> > > + struct mtk_dip_pipe *pipe = video_drvdata(file);
> > > +
> > > + strlcpy(cap->driver, pipe->desc->name,
> > > + sizeof(cap->driver));
> > > + strlcpy(cap->card, pipe->desc->name,
> > > + sizeof(cap->card));
> > > + snprintf(cap->bus_info, sizeof(cap->bus_info),
> > > + "platform:%s", dev_name(pipe->dip_dev->mdev.dev));
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int mtk_dip_videoc_try_fmt(struct file *file, void *fh,
> > > + struct v4l2_format *f)
> >
> > I don't see this function returning any error codes. Please make it void.
>
> mtk_dip_videoc_try_fmt() is used as vidioc_try_fmt_vid_out_mplane op.
> Using void seems to make it incompatible with
> vidioc_try_fmt_vid_out_mplane.
>
> .vidioc_try_fmt_vid_out_mplane = mtk_dip_videoc_try_fmt,
>
> int (*vidioc_try_fmt_vid_out_mplane)(struct file *file, void *fh,
> struct v4l2_format *f);
>
Oops, sorry, I'm not sure why I suggested that.
[snip]
> > > + /* Initialize subdev */
> > > + v4l2_subdev_init(&pipe->subdev, &mtk_dip_subdev_ops);
> > > +
> > > + pipe->subdev.entity.function =
> > > + MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER;
> > > + pipe->subdev.entity.ops = &mtk_dip_media_ops;
> > > + pipe->subdev.flags =
> > > + V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS;
> > > + pipe->subdev.ctrl_handler = NULL;
> > > + pipe->subdev.internal_ops = &mtk_dip_subdev_internal_ops;
> > > +
> > > + for (i = 0; i < pipe->num_nodes; i++)
> > > + pipe->subdev_pads[i].flags =
> > > + V4L2_TYPE_IS_OUTPUT(pipe->nodes[i].desc->buf_type) ?
> > > + MEDIA_PAD_FL_SINK : MEDIA_PAD_FL_SOURCE;
> >
> > Isn't this the other way around?
>
> I checked the document of MEDIA_PAD_FL_SINK and MEDIA_PAD_FL_SOURCE. It
> seems that the codes match the description.
>
> RAW Ouput video device: MEDIA_PAD_FL_SOURCE --> DIP sub dev:
> MEDIA_PAD_FL_SINK
> DIP sub dev: MEDIA_PAD_FL_SOURCE --> MDP Capture video device:
> MEDIA_PAD_FL_SINK
>
> MEDIA_PAD_FL_SINK: Input pad, relative to the entity. Input pads sink
> data and are targets of links.
> MEDIA_PAD_FL_SOURCE: Output pad, relative to the entity. Output pads
> source data and are origins of links.
>
Ah, that's right, sorry for the noise.
[snip]
> > > + {
> > > + .format = V4L2_PIX_FMT_NV12M,
> > > + .mdp_color = MDP_COLOR_NV12,
> > > + .colorspace = V4L2_COLORSPACE_BT2020,
> > > + .depth = { 8, 4 },
> > > + .row_depth = { 8, 8 },
> >
> > What is depth and what is row_depth? They both seem to not match NV12, which
> > should have 16 bits per sample in the CbCr plane.
>
> Fow_depth is the bits of the pixel.
Bits of a YCbCr plane pixel is 16 for NV12.
> Depth means the bytes per pixel of the image foramt.
>
> For example,
> Image: 640 * 480
> YV12: row_depth = 8, depth = 12
> Bytes per line = width * row_depth / 8 = 640 * 8/ 8 = 640
> Image size = Bytes per line * height * (depth/ row_depth)
> = 640 * 480 * 1.5
>
> Image: 640 * 480
> Bytes per line(Y) = width * row_depth/ 8 = 640
> Bytes per line(CbCr) = width * row_depth/ 8 = 640
>
> Image size(Y) = Bytes per line * height * (depth/ row_depth)
> = 640 * 480 * 8/8 = 640 * 480 * 1
>
> Image size(CbCr) = Bytes per line * height * (depth/ row_depth)
> = 640 * 480 * 4/8 = 640 * 480 * 0.5
>
I'd suggest either using the V4L2 format helpers, as suggested in
another comment above with a link OR adopting the typical convention
of having the .depth mean the pixel value size, i.e. 16-bit for CbCr
plane of NV12 and then use subsampling factors to calculate the plane
bytesperline and sizeimage.
[snip]
> > > +static const struct mtk_dip_video_device_desc
> > > +reprocess_output_queues_setting[MTK_DIP_VIDEO_NODE_ID_OUT_TOTAL_NUM] = {
> > > + {
> > > + .id = MTK_DIP_VIDEO_NODE_ID_RAW_OUT,
> > > + .name = "Raw Input",
> > > + .cap = V4L2_CAP_VIDEO_OUTPUT_MPLANE | V4L2_CAP_STREAMING,
> > > + .buf_type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
> > > + .smem_alloc = 0,
> > > + .flags = MEDIA_LNK_FL_ENABLED,
> > > + .fmts = in_fmts,
> > > + .num_fmts = ARRAY_SIZE(in_fmts),
> > > + .default_fmt_idx = 5,
> > > + .default_width = MTK_DIP_OUTPUT_MAX_WIDTH,
> > > + .default_height = MTK_DIP_OUTPUT_MAX_HEIGHT,
> > > + .dma_port = 0,
> > > + .frmsizeenum = &in_frmsizeenum,
> > > + .ops = &mtk_dip_v4l2_video_out_ioctl_ops,
> > > + .description = "Source image to be process",
> > > +
> > > + },
> > > + {
> > > + .id = MTK_DIP_VIDEO_NODE_ID_TUNING_OUT,
> > > + .name = "Tuning",
> > > + .cap = V4L2_CAP_META_OUTPUT | V4L2_CAP_STREAMING,
> > > + .buf_type = V4L2_BUF_TYPE_META_OUTPUT,
> > > + .smem_alloc = 1,
> > > + .flags = 0,
> > > + .fmts = fw_param_fmts,
> > > + .num_fmts = 1,
> > > + .default_fmt_idx = 0,
> > > + .dma_port = 0,
> > > + .frmsizeenum = &in_frmsizeenum,
> > > + .ops = &mtk_dip_v4l2_meta_out_ioctl_ops,
> > > + .description = "Tuning data for image enhancement",
> > > + },
> > > +};
> >
> > The entries here seem to be almost the same to output_queues_setting[], the
> > only difference seems to be default_fmt_idx and description.
> >
> > What's the difference between the capture and reprocess pipes?
>
> The reprocess pipe is completely the same as capture one except the
> default format of the input.
>
Does the default format really matter here? The userspace should set
its own desired format anyway. Then we could just unify the settings
of both pipes.
[snip]
> >
> > > +
> > > + return num;
> > > +}
> > > +
> > > +static int __maybe_unused mtk_dip_suspend(struct device *dev)
> > > +{
> > > + struct mtk_dip_dev *dip_dev = dev_get_drvdata(dev);
> > > + int ret, num;
> > > +
> > > + if (pm_runtime_suspended(dev)) {
> > > + dev_dbg(dev, "%s: pm_runtime_suspended is true, no action\n",
> > > + __func__);
> > > + return 0;
> > > + }
> > > +
> > > + ret = wait_event_timeout(dip_dev->dip_hw.flushing_hw_wq,
> > > + !(num = mtk_dip_get_scp_job_num(dip_dev)),
> > > + msecs_to_jiffies(200));
> >
> > Is 200 milliseconds a reasonable timeout here, i.e. for any potential use
> > case it would always take less than 200 ms to wait for all the tasks running
> > in the ISP?
>
> I would like to adjust the time out value to 1000 / 30 *
> (DIP_COMPOSING_MAX_NUM) * 3.
>
> To wait 3 times of expected frame time (@30fps) in worst case (max
> number of jobs in SCP).
>
As I suggested in another comment above, the worst case for the
hardware might be still better than the scheduling latency we could
get for a heavy loaded system. While that wouldn't really apply here,
because nothing else happening in parallel when the system is
suspending, we could just stick to some really long time out anyway,
e.g. 1 second. We shouldn't hit it anyway - it's just a safety guard
to prevent the system hanging if something goes really bad.
Best regards,
Tomasz
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 5/8] media: cedrus: Detect first slice of a frame
From: Hans Verkuil @ 2019-08-30 7:28 UTC (permalink / raw)
To: Jernej Skrabec, mchehab, paul.kocialkowski, mripard
Cc: devel, pawel, acourbot, jonas, gregkh, wens, tfiga, kyungmin.park,
linux-arm-kernel, linux-media, ezequiel, linux-kernel,
m.szyprowski
In-Reply-To: <20190822194500.2071-6-jernej.skrabec@siol.net>
On 8/22/19 9:44 PM, Jernej Skrabec wrote:
> When codec supports multiple slices in one frame, VPU has to know when
> first slice of each frame is being processed, presumably to correctly
> clear/set data in auxiliary buffers.
>
> Add first_slice field to cedrus_run structure and set it according to
> timestamps of capture and output buffers. If timestamps are different,
> it's first slice and viceversa.
>
> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> ---
> drivers/staging/media/sunxi/cedrus/cedrus.h | 1 +
> drivers/staging/media/sunxi/cedrus/cedrus_dec.c | 2 ++
> 2 files changed, 3 insertions(+)
>
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h b/drivers/staging/media/sunxi/cedrus/cedrus.h
> index 2f017a651848..32cb38e541c6 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus.h
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus.h
> @@ -70,6 +70,7 @@ struct cedrus_mpeg2_run {
> struct cedrus_run {
> struct vb2_v4l2_buffer *src;
> struct vb2_v4l2_buffer *dst;
> + bool first_slice;
>
> union {
> struct cedrus_h264_run h264;
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> index 56ca4c9ad01c..d7b54accfe83 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> @@ -31,6 +31,8 @@ void cedrus_device_run(void *priv)
>
> run.src = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
> run.dst = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
> + run.first_slice =
> + run.src->vb2_buf.timestamp != run.dst->vb2_buf.timestamp;
This is almost correct. To handle the corner case where no timestamp
was ever copied to run.dst->vb2_buf you need this check:
run.first_slice = !run.dst->vb2_buf.copied_timestamp ||
run.src->vb2_buf.timestamp != run.dst->vb2_buf.timestamp;
Regards,
Hans
>
> /* Apply request(s) controls if needed. */
> src_req = run.src->vb2_buf.req_obj.req;
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCHv2 02/11] soc: ti: add initial PRM driver with reset control support
From: Tero Kristo @ 2019-08-30 7:28 UTC (permalink / raw)
To: Philipp Zabel, ssantosh, linux-arm-kernel, linux-omap, robh+dt
Cc: tony, devicetree
In-Reply-To: <1567084339.5345.7.camel@pengutronix.de>
On 29/08/2019 16:12, Philipp Zabel wrote:
> On Wed, 2019-08-28 at 10:19 +0300, Tero Kristo wrote:
>> Add initial PRM (Power and Reset Management) driver for TI OMAP class
>> SoCs. Initially this driver only supports reset control, but can be
>> extended to support rest of the functionality, like powerdomain
>> control, PRCM irq support etc.
>>
>> Signed-off-by: Tero Kristo <t-kristo@ti.com>
>> ---
>> arch/arm/mach-omap2/Kconfig | 1 +
>> drivers/soc/ti/Makefile | 1 +
>> drivers/soc/ti/omap_prm.c | 235 ++++++++++++++++++++++++++++++++++++
>> 3 files changed, 237 insertions(+)
>> create mode 100644 drivers/soc/ti/omap_prm.c
>>
>> diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
>> index fdb6743760a2..ad08d470a2ca 100644
>> --- a/arch/arm/mach-omap2/Kconfig
>> +++ b/arch/arm/mach-omap2/Kconfig
>> @@ -109,6 +109,7 @@ config ARCH_OMAP2PLUS
>> select TI_SYSC
>> select OMAP_IRQCHIP
>> select CLKSRC_TI_32K
>> + select ARCH_HAS_RESET_CONTROLLER
>> help
>> Systems based on OMAP2, OMAP3, OMAP4 or OMAP5
>>
>> diff --git a/drivers/soc/ti/Makefile b/drivers/soc/ti/Makefile
>> index b3868d392d4f..788b5cd1e180 100644
>> --- a/drivers/soc/ti/Makefile
>> +++ b/drivers/soc/ti/Makefile
>> @@ -6,6 +6,7 @@ obj-$(CONFIG_KEYSTONE_NAVIGATOR_QMSS) += knav_qmss.o
>> knav_qmss-y := knav_qmss_queue.o knav_qmss_acc.o
>> obj-$(CONFIG_KEYSTONE_NAVIGATOR_DMA) += knav_dma.o
>> obj-$(CONFIG_AMX3_PM) += pm33xx.o
>> +obj-$(CONFIG_ARCH_OMAP2PLUS) += omap_prm.o
>> obj-$(CONFIG_WKUP_M3_IPC) += wkup_m3_ipc.o
>> obj-$(CONFIG_TI_SCI_PM_DOMAINS) += ti_sci_pm_domains.o
>> obj-$(CONFIG_TI_SCI_INTA_MSI_DOMAIN) += ti_sci_inta_msi.o
>> diff --git a/drivers/soc/ti/omap_prm.c b/drivers/soc/ti/omap_prm.c
>> new file mode 100644
>> index 000000000000..fd5c431f8736
>> --- /dev/null
>> +++ b/drivers/soc/ti/omap_prm.c
>> @@ -0,0 +1,235 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * OMAP2+ PRM driver
>> + *
>> + * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/
>> + * Tero Kristo <t-kristo@ti.com>
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>
> Why <linux/module.h>? This is a builtin driver.
Yeah, not anymore. Let me ditch this.
>
>> +#include <linux/device.h>
>> +#include <linux/io.h>
>> +#include <linux/of.h>
>> +#include <linux/of_device.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/reset-controller.h>
>> +#include <linux/delay.h>
>> +
>> +struct omap_rst_map {
>> + s8 rst;
>> + s8 st;
>> +};
>> +
>> +struct omap_prm_data {
>> + u32 base;
>> + const char *name;
>> + u16 rstctrl;
>> + u16 rstst;
>> + const struct omap_rst_map *rstmap;
>> + u8 flags;
>> +};
>
> I wonder if splitting rstctrl/rstst/rstmap out into a separate structure
> would make sense. That could be linked from omap_reset_data directly.
> That only makes sense if there'd be enough cases where it can be reused
> for multiple PRMs instances.
Hmm, splitting these out would make it possible to share the bits for
ipu:s across devices, same for dsp:s and eve:s.
However, adding too many levels of abstraction makes it kind of
difficult to follow what is happening with the code, and it would only
save maybe ~100 bytes of memory at the moment.
>
>> +
>> +struct omap_prm {
>> + const struct omap_prm_data *data;
>> + void __iomem *base;
>> +};
>> +
>> +struct omap_reset_data {
>> + struct reset_controller_dev rcdev;
>> + struct omap_prm *prm;
>> +};
>> +
>> +#define to_omap_reset_data(p) container_of((p), struct omap_reset_data, rcdev)
>> +
>> +#define OMAP_MAX_RESETS 8
>> +#define OMAP_RESET_MAX_WAIT 10000
>> +
>> +#define OMAP_PRM_HAS_RSTCTRL BIT(0)
>> +#define OMAP_PRM_HAS_RSTST BIT(1)
>> +
>> +#define OMAP_PRM_HAS_RESETS (OMAP_PRM_HAS_RSTCTRL | OMAP_PRM_HAS_RSTST)
>> +
>> +static const struct of_device_id omap_prm_id_table[] = {
>> + { },
>> +};
>> +
>> +static bool _is_valid_reset(struct omap_reset_data *reset, unsigned long id)
>> +{
>> + const struct omap_rst_map *map = reset->prm->data->rstmap;
>> +
>> + while (map && map->rst >= 0) {
>
> If rstmap is never NULL,
>
> while (map->rst >= 0) {
>
> should be enough.
I'll actually re-write this to use the reset mask.
>
>> + if (map->rst == id)
>> + return true;
>> + map++;
>> + }
>> +
>> + return false;
>> +}
>> +
>> +static int omap_reset_status(struct reset_controller_dev *rcdev,
>> + unsigned long id)
>> +{
>> + struct omap_reset_data *reset = to_omap_reset_data(rcdev);
>> + u32 v;
>> +
>> + if (!_is_valid_reset(reset, id))
>> + return -EINVAL;
>
> Don't check this in the status/(de)assert/reset callbacks. Instead,
> implement rcdev.of_xlate and return -EINVAL there, so that invalid ids
> can never be requested.
Yeah, let me do that.
>
>> + v = readl_relaxed(reset->prm->base + reset->prm->data->rstst);
>> + v &= 1 << id;
>> + v >>= id;
>
> omap_get_st_bit below makes it look like the status bit can be in a
> different place than the reset control bit, should that be used here as
> well?
True, this is a bug.
>
>> +
>> + return v;
>> +}
>> +
>> +static int omap_reset_assert(struct reset_controller_dev *rcdev,
>> + unsigned long id)
>> +{
>> + struct omap_reset_data *reset = to_omap_reset_data(rcdev);
>> + u32 v;
>> +
>> + if (!_is_valid_reset(reset, id))
>> + return -EINVAL;
>
> Same as above.
Will drop this one.
>
>> + /* assert the reset control line */
>> + v = readl_relaxed(reset->prm->base + reset->prm->data->rstctrl);
>> + v |= 1 << id;
>> + writel_relaxed(v, reset->prm->base + reset->prm->data->rstctrl);
>
> This read-modify-write should be protected with a lock.
Ok, will add a spinlock. I did think about this before but all the cases
where we are sharing a reset register are to be controlled from the same
process / driver, and the events are effectively serialized. Doesn't
hurt adding it for possible future need though.
>
>> +
>> + return 0;
>> +}
>> +
>> +static int omap_reset_get_st_bit(struct omap_reset_data *reset,
>> + unsigned long id)
>> +{
>> + const struct omap_rst_map *map = reset->prm->data->rstmap;
>> +
>> + while (map && map->rst >= 0) {
>
> Same as above.
Yeah, usage of rstmap is now enforced, so no need to check it here.
>
>> + if (map->rst == id)
>> + return map->st;
>> +
>> + map++;
>> + }
>> +
>> + return id;
>> +}
>> +
>> +/*
>> + * Note that status will not change until clocks are on, and clocks cannot be
>> + * enabled until reset is deasserted. Consumer drivers must check status
>> + * separately after enabling clocks.
>> + */
>> +static int omap_reset_deassert(struct reset_controller_dev *rcdev,
>> + unsigned long id)
>> +{
>> + struct omap_reset_data *reset = to_omap_reset_data(rcdev);
>> + u32 v;
>> + int st_bit;
>> + bool has_rstst;
>> +
>> + if (!_is_valid_reset(reset, id))
>> + return -EINVAL;
>
> Same as above.
Will drop.
>
>> + /* check the current status to avoid de-asserting the line twice */
>> + v = readl_relaxed(reset->prm->base + reset->prm->data->rstctrl);
>> + if (!(v & BIT(id)))
>> + return -EEXIST;
>
> What is the purpose of this? For shared consumers the core already does
> refcounting, and I expect exclusive consumers won't deassert twice.
> Since the reset signal is deasserted after this call, this should not
> return an error.
This is actually a leftover from legacy code; this driver is mostly a
move of the reset handling from platform codebase to be an actual driver
of its own. But yes, I believe this can be dropped.
>
>> +
>> + has_rstst = reset->prm->data->rstst ||
>> + (reset->prm->data->flags & OMAP_PRM_HAS_RSTST);
>> +
>> + if (has_rstst) {
>> + st_bit = omap_reset_get_st_bit(reset, id);
>> +
>> + /* Clear the reset status by writing 1 to the status bit */
>> + v = readl_relaxed(reset->prm->base + reset->prm->data->rstst);
>> + v |= 1 << st_bit;
>> + writel_relaxed(v, reset->prm->base + reset->prm->data->rstst);
>
> What does the value read from the rstst register indicate? Is it the
> current state of the reset line? Is it 0 until deassertion is completed,
> and then it turns to 1?
Value of 1 indicates that the corresponding IP has been reset
successfully. Writing back 1 to the same bit clears it out, so the
status can be polled later on.
>
>> + }
>> +
>> + /* de-assert the reset control line */
>> + v = readl_relaxed(reset->prm->base + reset->prm->data->rstctrl);
>
> Reading the register again seems unnecessary.
I dropped the earlier read for now, so this is again needed.
>
>> + v &= ~(1 << id);
>> + writel_relaxed(v, reset->prm->base + reset->prm->data->rstctrl);
>
> As above, the read-modify-write should be locked.
Yep, will protect this.
>
>> +
>> + return 0;
>> +}
>> +
>> +static const struct reset_control_ops omap_reset_ops = {
>> + .assert = omap_reset_assert,
>> + .deassert = omap_reset_deassert,
>> + .status = omap_reset_status,
>> +};
>> +
>> +static int omap_prm_reset_init(struct platform_device *pdev,
>> + struct omap_prm *prm)
>> +{
>> + struct omap_reset_data *reset;
>> +
>> + /*
>> + * Check if we have controllable resets. If either rstctrl is non-zero
>> + * or OMAP_PRM_HAS_RSTCTRL flag is set, we have reset control register
>> + * for the domain.
>> + */
>> + if (!prm->data->rstctrl && !(prm->data->flags & OMAP_PRM_HAS_RSTCTRL))
>> + return 0;
>> +
>> + reset = devm_kzalloc(&pdev->dev, sizeof(*reset), GFP_KERNEL);
>> + if (!reset)
>> + return -ENOMEM;
>> +
>> + reset->rcdev.owner = THIS_MODULE;
>> + reset->rcdev.ops = &omap_reset_ops;
>> + reset->rcdev.of_node = pdev->dev.of_node;
>> + reset->rcdev.nr_resets = OMAP_MAX_RESETS;
>> +
>> + reset->prm = prm;
>> +
>> + return devm_reset_controller_register(&pdev->dev, &reset->rcdev);
>> +}
>> +
>> +static int omap_prm_probe(struct platform_device *pdev)
>> +{
>> + struct resource *res;
>> + const struct omap_prm_data *data;
>> + struct omap_prm *prm;
>> + const struct of_device_id *match;
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + if (!res)
>> + return -ENODEV;
>
> This can be merged with devm_ioremap_resource below.
Well, I actually use the "res" later on to map the DT node to the
corresponding prm_data based on address.
>
>> + match = of_match_device(omap_prm_id_table, &pdev->dev);
>> + if (!match)
>> + return -ENOTSUPP;
>> +
>> + prm = devm_kzalloc(&pdev->dev, sizeof(*prm), GFP_KERNEL);
>> + if (!prm)
>> + return -ENOMEM;
>> +
>> + data = match->data;
>> +
>> + while (data->base != res->start) {
>> + if (!data->base)
>> + return -EINVAL;
>> + data++;
>> + }
>
> Is this not something that you want to have encoded in the compatible
> string? They all have a different register layout.
With the addition of all the prm instances later on, this changes. Most
of the prm instances will have same register layout then. See v1 data
that was posted earlier [1], but which I dropped for now to keep this
series isolated for reset handling only. In this patch, you see that for
DRA7, all the power domain handling related PRM instances have identical
register layout, they just differ based on base address.
[1] https://www.spinics.net/lists/linux-omap/msg149872.html
It would be possible to encode all of this based on different
compatibles, but then the amount of different compatible strings would
explode... DRA7 is just one SoC.
>
>> +
>> + prm->data = data;
>> +
>> + prm->base = devm_ioremap_resource(&pdev->dev, res);
>
> prm->base = devm_platform_ioremap_resource(pdev, 0);
I still need the "res" pointer as indicated above.
-Tero
>
>> + if (!prm->base)
>> + return -ENOMEM;
>> +
>> + return omap_prm_reset_init(pdev, prm);
>> +}
>> +
>> +static struct platform_driver omap_prm_driver = {
>> + .probe = omap_prm_probe,
>> + .driver = {
>> + .name = KBUILD_MODNAME,
>> + .of_match_table = omap_prm_id_table,
>> + },
>> +};
>> +builtin_platform_driver(omap_prm_driver);
>
> regards
> Philipp
>
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCHv1 0/3] Odroid c2 missing regulator linking
From: Neil Armstrong @ 2019-08-30 7:31 UTC (permalink / raw)
To: Anand Moon
Cc: devicetree, Martin Blumenstingl, Kevin Hilman, Linux Kernel,
Rob Herring, linux-amlogic, linux-arm-kernel, Jerome Brunet
In-Reply-To: <CANAwSgShr-K-44UzdxFC7pvpTye_pbEMdS6ug1eWwYhnsVNGdQ@mail.gmail.com>
On 29/08/2019 20:35, Anand Moon wrote:
> Hi Neil,
>
> On Thu, 29 Aug 2019 at 13:58, Neil Armstrong <narmstrong@baylibre.com> wrote:
>>
>> On 28/08/2019 22:27, Anand Moon wrote:
>>> Below small changes help re-configure or fix missing inter linking
>>> of regulator node.
>>>
>>> Changes based top on my prevoius series.
>>
>> For the serie:
>> Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
>>
>
> Thanks for your review.
>
>>>
>>> [0] https://patchwork.kernel.org/cover/11113091/
>>>
>>> TOOD: Add support for DVFS GXBB odroid board in next series.
>>
>> I'm curious how you will do this !
>
> I was just studying you previous series on how you have implemented
> this feature for C1, N2 and VIM3 boards.
>
> [0] https://patchwork.kernel.org/cover/11114125/
>
> I started gathering key inputs needed for this ie *clk / pwm*
> like VDDCPU and VDDE clk changes.
>
> But it looks like of the complex clk framework needed, so I leave this to the
> expert like your team of developers to do this much quick and efficiently.
On GXBB, GXL, GXM and AXG SoCs, CPU Frequency setting and PWM Regulator setup is
done by the SCPI Co-processor via the SCPI protocol.
Thus, we should not handle it from Linux, and even if we could, we don't have the
registers documentation of the CPU clusters clock tree.
SCPI works fine on all tested devices, except Odroid-C2, because Hardkernel left
the > 1.5GHz freq in the initial SCPI tables loaded by the BL2, i.e. packed with U-Boot.
Nowadays they have removed the bad frequencies, but still some devices uses the old
bootloader.
But in the SCPI case we trust the table returned by the firmware and use it as-in,
and there is no (simple ?) way to override the table and set a max frequency.
This is why we disabled SCPI.
See https://patchwork.kernel.org/patch/9500175/
Neil
>
> Best Regards,
> -Anand
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* RE: [PATCH v5 1/2] dt-bindings: mailbox: add binding doc for the ARM SMC/HVC mailbox
From: Peng Fan @ 2019-08-30 7:37 UTC (permalink / raw)
To: Jassi Brar
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org,
f.fainelli@gmail.com, andre.przywara@arm.com,
linux-kernel@vger.kernel.org, robh+dt@kernel.org, dl-linux-imx,
sudeep.holla@arm.com, linux-arm-kernel@lists.infradead.org
In-Reply-To: <CABb+yY2TREpO7+TFcGgsgQrkmMWwFAgtuJ4GnLPPQ+GEBuh07w@mail.gmail.com>
Hi Jassi,
> Subject: Re: [PATCH v5 1/2] dt-bindings: mailbox: add binding doc for the ARM
> SMC/HVC mailbox
>
> On Fri, Aug 30, 2019 at 1:28 AM Peng Fan <peng.fan@nxp.com> wrote:
>
> > > > +examples:
> > > > + - |
> > > > + sram@910000 {
> > > > + compatible = "mmio-sram";
> > > > + reg = <0x0 0x93f000 0x0 0x1000>;
> > > > + #address-cells = <1>;
> > > > + #size-cells = <1>;
> > > > + ranges = <0 0x0 0x93f000 0x1000>;
> > > > +
> > > > + cpu_scp_lpri: scp-shmem@0 {
> > > > + compatible = "arm,scmi-shmem";
> > > > + reg = <0x0 0x200>;
> > > > + };
> > > > +
> > > > + cpu_scp_hpri: scp-shmem@200 {
> > > > + compatible = "arm,scmi-shmem";
> > > > + reg = <0x200 0x200>;
> > > > + };
> > > > + };
> > > > +
> > > > + firmware {
> > > > + smc_mbox: mailbox {
> > > > + #mbox-cells = <1>;
> > > > + compatible = "arm,smc-mbox";
> > > > + method = "smc";
> > > > + arm,num-chans = <0x2>;
> > > > + transports = "mem";
> > > > + /* Optional */
> > > > + arm,func-ids = <0xc20000fe>, <0xc20000ff>;
> > > >
> > > SMC/HVC is synchronously(block) running in "secure mode", i.e, there
> > > can only be one instance running platform wide. Right?
> >
> > I think there could be channel for TEE, and channel for Linux.
> > For virtualization case, there could be dedicated channel for each VM.
> >
> I am talking from Linux pov. Functions 0xfe and 0xff above, can't both be
> active at the same time, right?
If I get your point correctly,
On UP, both could not be active. On SMP, tx/rx could be both active, anyway
this depends on secure firmware and Linux firmware design.
Do you have any suggestions about arm,func-ids here?
Thanks,
Peng.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH 1/2] drm/mediatek: Only block updates to CRTCs that have a pending update
From: Bibby Hsieh @ 2019-08-30 7:38 UTC (permalink / raw)
To: David Airlie, Matthias Brugger, Daniel Vetter, dri-devel,
linux-mediatek
Cc: drinkcat, Bibby Hsieh, linux-kernel, Daniel Kurtz, tfiga, CK Hu,
Thierry Reding, Philipp Zabel, YT Shen, linux-arm-kernel
In-Reply-To: <20190830073819.16566-1-bibby.hsieh@mediatek.com>
Currently we use a single mutex to allow only a single atomic
update at a time. In truth, though, we really only want to
ensure that only a single atomic update is allowed per CRTC.
In other words, for each atomic update, we only block if there
is a pending update for the CRTCs involved, and don't block if
there are only pending updates for other CRTCs.
Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC MT8173.")
Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
Signed-off-by: Bibby Hsieh <bibby.hsieh@mediatek.com>
---
drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 14 +-
drivers/gpu/drm/mediatek/mtk_drm_drv.c | 182 +++++++++++++++++++++---
drivers/gpu/drm/mediatek/mtk_drm_drv.h | 12 +-
3 files changed, 184 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index b55970a2869d..7697b40baac0 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
@@ -5,6 +5,7 @@
#include <asm/barrier.h>
#include <drm/drmP.h>
+#include <drm/drm_atomic.h>
#include <drm/drm_atomic_helper.h>
#include <drm/drm_plane_helper.h>
#include <drm/drm_probe_helper.h>
@@ -45,6 +46,8 @@ struct mtk_drm_crtc {
struct mtk_disp_mutex *mutex;
unsigned int ddp_comp_nr;
struct mtk_ddp_comp **ddp_comp;
+
+ struct drm_crtc_state *old_crtc_state;
};
struct mtk_crtc_state {
@@ -343,6 +346,7 @@ static void mtk_crtc_ddp_hw_fini(struct mtk_drm_crtc *mtk_crtc)
static void mtk_crtc_ddp_config(struct drm_crtc *crtc)
{
struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
+ struct drm_atomic_state *atomic_state = mtk_crtc->old_crtc_state->state;
struct mtk_crtc_state *state = to_mtk_crtc_state(mtk_crtc->base.state);
struct mtk_ddp_comp *comp = mtk_crtc->ddp_comp[0];
unsigned int i;
@@ -382,6 +386,7 @@ static void mtk_crtc_ddp_config(struct drm_crtc *crtc)
}
}
mtk_crtc->pending_planes = false;
+ mtk_atomic_state_put_queue(atomic_state);
}
}
@@ -451,6 +456,7 @@ static void mtk_drm_crtc_atomic_begin(struct drm_crtc *crtc,
static void mtk_drm_crtc_atomic_flush(struct drm_crtc *crtc,
struct drm_crtc_state *old_crtc_state)
{
+ struct drm_atomic_state *old_atomic_state = old_crtc_state->state;
struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
struct mtk_drm_private *priv = crtc->dev->dev_private;
unsigned int pending_planes = 0;
@@ -469,8 +475,13 @@ static void mtk_drm_crtc_atomic_flush(struct drm_crtc *crtc,
pending_planes |= BIT(i);
}
}
- if (pending_planes)
+
+ if (pending_planes) {
mtk_crtc->pending_planes = true;
+ drm_atomic_state_get(old_atomic_state);
+ mtk_crtc->old_crtc_state = old_crtc_state;
+ }
+
if (crtc->state->color_mgmt_changed)
for (i = 0; i < mtk_crtc->ddp_comp_nr; i++)
mtk_ddp_gamma_set(mtk_crtc->ddp_comp[i], crtc->state);
@@ -526,6 +537,7 @@ static int mtk_drm_crtc_init(struct drm_device *drm,
void mtk_crtc_ddp_irq(struct drm_crtc *crtc, struct mtk_ddp_comp *comp)
{
+
struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
struct mtk_drm_private *priv = crtc->dev->dev_private;
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index c0928b69dc43..b0308a3a7483 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -31,11 +31,120 @@
#define DRIVER_MAJOR 1
#define DRIVER_MINOR 0
-static void mtk_atomic_schedule(struct mtk_drm_private *private,
+struct mtk_atomic_state {
+ struct drm_atomic_state base;
+ struct list_head list;
+ struct work_struct work;
+};
+
+static inline struct mtk_atomic_state *to_mtk_state(struct drm_atomic_state *s)
+{
+ return container_of(s, struct mtk_atomic_state, base);
+}
+
+void mtk_atomic_state_put_queue(struct drm_atomic_state *state)
+{
+ struct drm_device *drm = state->dev;
+ struct mtk_drm_private *mtk_drm = drm->dev_private;
+ struct mtk_atomic_state *mtk_state = to_mtk_state(state);
+ unsigned long flags;
+
+ spin_lock_irqsave(&mtk_drm->unreference.lock, flags);
+ list_add_tail(&mtk_state->list, &mtk_drm->unreference.list);
+ spin_unlock_irqrestore(&mtk_drm->unreference.lock, flags);
+
+ schedule_work(&mtk_drm->unreference.work);
+}
+
+static uint32_t mtk_atomic_crtc_mask(struct drm_device *drm,
+ struct drm_atomic_state *state)
+{
+ uint32_t crtc_mask;
+ int i;
+
+ for (i = 0, crtc_mask = 0; i < drm->mode_config.num_crtc; i++) {
+ struct drm_crtc *crtc = state->crtcs[i].ptr;
+
+ if (crtc)
+ crtc_mask |= (1 << drm_crtc_index(crtc));
+ }
+
+ return crtc_mask;
+}
+
+/*
+ * Block until specified crtcs are no longer pending update, and atomically
+ * mark them as pending update.
+ */
+static int mtk_atomic_get_crtcs(struct drm_device *drm,
+ struct drm_atomic_state *state)
+{
+ struct mtk_drm_private *private = drm->dev_private;
+ uint32_t crtc_mask;
+ int ret;
+
+ crtc_mask = mtk_atomic_crtc_mask(drm, state);
+
+ /*
+ * Wait for all pending updates to complete for the set of crtcs being
+ * changed in this atomic commit
+ */
+ spin_lock(&private->commit.crtcs_event.lock);
+ ret = wait_event_interruptible_locked(private->commit.crtcs_event,
+ !(private->commit.crtcs & crtc_mask));
+ if (ret == 0)
+ private->commit.crtcs |= crtc_mask;
+ spin_unlock(&private->commit.crtcs_event.lock);
+
+ return ret;
+}
+
+/*
+ * Mark specified crtcs as no longer pending update.
+ */
+static void mtk_atomic_put_crtcs(struct drm_device *drm,
+ struct drm_atomic_state *state)
+{
+ struct mtk_drm_private *private = drm->dev_private;
+ uint32_t crtc_mask;
+
+ crtc_mask = mtk_atomic_crtc_mask(drm, state);
+
+ spin_lock(&private->commit.crtcs_event.lock);
+ private->commit.crtcs &= ~crtc_mask;
+ wake_up_all_locked(&private->commit.crtcs_event);
+ spin_unlock(&private->commit.crtcs_event.lock);
+}
+
+static void mtk_unreference_work(struct work_struct *work)
+{
+ struct mtk_drm_private *mtk_drm = container_of(work,
+ struct mtk_drm_private, unreference.work);
+ unsigned long flags;
+ struct mtk_atomic_state *state, *tmp;
+
+ /*
+ * framebuffers cannot be unreferenced in atomic context.
+ * Therefore, only hold the spinlock when iterating unreference_list,
+ * and drop it when doing the unreference.
+ */
+ spin_lock_irqsave(&mtk_drm->unreference.lock, flags);
+ list_for_each_entry_safe(state, tmp, &mtk_drm->unreference.list, list) {
+ list_del(&state->list);
+ spin_unlock_irqrestore(&mtk_drm->unreference.lock, flags);
+ drm_atomic_state_put(&state->base);
+ spin_lock_irqsave(&mtk_drm->unreference.lock, flags);
+ }
+ spin_unlock_irqrestore(&mtk_drm->unreference.lock, flags);
+}
+
+
+static void mtk_atomic_schedule(struct drm_device *drm,
struct drm_atomic_state *state)
{
- private->commit.state = state;
- schedule_work(&private->commit.work);
+ struct mtk_atomic_state *mtk_state = to_mtk_state(state);
+
+ schedule_work(&mtk_state->work);
}
static void mtk_atomic_wait_for_fences(struct drm_atomic_state *state)
@@ -48,13 +157,10 @@ static void mtk_atomic_wait_for_fences(struct drm_atomic_state *state)
mtk_fb_wait(new_plane_state->fb);
}
-static void mtk_atomic_complete(struct mtk_drm_private *private,
+static void mtk_atomic_complete(struct drm_device *drm,
struct drm_atomic_state *state)
{
- struct drm_device *drm = private->drm;
-
mtk_atomic_wait_for_fences(state);
-
/*
* Mediatek drm supports runtime PM, so plane registers cannot be
* written when their crtc is disabled.
@@ -77,53 +183,86 @@ static void mtk_atomic_complete(struct mtk_drm_private *private,
drm_atomic_helper_wait_for_vblanks(drm, state);
drm_atomic_helper_cleanup_planes(drm, state);
+ mtk_atomic_put_crtcs(drm, state);
+
drm_atomic_state_put(state);
}
static void mtk_atomic_work(struct work_struct *work)
{
- struct mtk_drm_private *private = container_of(work,
- struct mtk_drm_private, commit.work);
+ struct mtk_atomic_state *mtk_state = container_of(work,
+ struct mtk_atomic_state, work);
+ struct drm_atomic_state *state = &mtk_state->base;
+ struct drm_device *drm = state->dev;
- mtk_atomic_complete(private, private->commit.state);
+ mtk_atomic_complete(drm, state);
}
static int mtk_atomic_commit(struct drm_device *drm,
struct drm_atomic_state *state,
bool async)
{
- struct mtk_drm_private *private = drm->dev_private;
int ret;
ret = drm_atomic_helper_prepare_planes(drm, state);
if (ret)
return ret;
- mutex_lock(&private->commit.lock);
- flush_work(&private->commit.work);
+ ret = mtk_atomic_get_crtcs(drm, state);
+ if (ret) {
+ drm_atomic_helper_cleanup_planes(drm, state);
+ return ret;
+ }
ret = drm_atomic_helper_swap_state(state, true);
if (ret) {
- mutex_unlock(&private->commit.lock);
drm_atomic_helper_cleanup_planes(drm, state);
return ret;
}
drm_atomic_state_get(state);
if (async)
- mtk_atomic_schedule(private, state);
+ mtk_atomic_schedule(drm, state);
else
- mtk_atomic_complete(private, state);
-
- mutex_unlock(&private->commit.lock);
+ mtk_atomic_complete(drm, state);
return 0;
}
+static struct drm_atomic_state *mtk_drm_atomic_state_alloc(
+ struct drm_device *dev)
+{
+ struct mtk_atomic_state *mtk_state;
+
+ mtk_state = kzalloc(sizeof(*mtk_state), GFP_KERNEL);
+ if (!mtk_state)
+ return NULL;
+
+ if (drm_atomic_state_init(dev, &mtk_state->base) < 0) {
+ kfree(mtk_state);
+ return NULL;
+ }
+
+ INIT_LIST_HEAD(&mtk_state->list);
+ INIT_WORK(&mtk_state->work, mtk_atomic_work);
+
+ return &mtk_state->base;
+}
+
+static void mtk_drm_atomic_state_free(struct drm_atomic_state *state)
+{
+ struct mtk_atomic_state *mtk_state = to_mtk_state(state);
+
+ drm_atomic_state_default_release(state);
+ kfree(mtk_state);
+}
+
static const struct drm_mode_config_funcs mtk_drm_mode_config_funcs = {
.fb_create = mtk_drm_mode_fb_create,
.atomic_check = drm_atomic_helper_check,
.atomic_commit = mtk_atomic_commit,
+ .atomic_state_alloc = mtk_drm_atomic_state_alloc,
+ .atomic_state_free = mtk_drm_atomic_state_free
};
static const enum mtk_ddp_comp_id mt2701_mtk_ddp_main[] = {
@@ -319,6 +458,11 @@ static int mtk_drm_kms_init(struct drm_device *drm)
drm_kms_helper_poll_init(drm);
drm_mode_config_reset(drm);
+ INIT_WORK(&private->unreference.work, mtk_unreference_work);
+ INIT_LIST_HEAD(&private->unreference.list);
+ spin_lock_init(&private->unreference.lock);
+ init_waitqueue_head(&private->commit.crtcs_event);
+
return 0;
err_component_unbind:
@@ -504,8 +648,6 @@ static int mtk_drm_probe(struct platform_device *pdev)
if (!private)
return -ENOMEM;
- mutex_init(&private->commit.lock);
- INIT_WORK(&private->commit.work, mtk_atomic_work);
private->data = of_device_get_match_data(dev);
mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
index 823ba4081c18..0934f83b860d 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
@@ -48,12 +48,16 @@ struct mtk_drm_private {
const struct mtk_mmsys_driver_data *data;
struct {
- struct drm_atomic_state *state;
- struct work_struct work;
- struct mutex lock;
+ uint32_t crtcs;
+ wait_queue_head_t crtcs_event;
} commit;
struct drm_atomic_state *suspend_state;
+ struct {
+ struct work_struct work;
+ struct list_head list;
+ spinlock_t lock;
+ } unreference;
};
extern struct platform_driver mtk_ddp_driver;
@@ -64,4 +68,6 @@ extern struct platform_driver mtk_dpi_driver;
extern struct platform_driver mtk_dsi_driver;
extern struct platform_driver mtk_mipi_tx_driver;
+void mtk_atomic_state_put_queue(struct drm_atomic_state *state);
+
#endif /* MTK_DRM_DRV_H */
--
2.18.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 2/2] drm/mediatek: Bypass atomic helpers for cursor updates
From: Bibby Hsieh @ 2019-08-30 7:38 UTC (permalink / raw)
To: David Airlie, Matthias Brugger, Daniel Vetter, dri-devel,
linux-mediatek
Cc: drinkcat, Bibby Hsieh, linux-kernel, Daniel Kurtz, tfiga, CK Hu,
Thierry Reding, Philipp Zabel, YT Shen, linux-arm-kernel
In-Reply-To: <20190830073819.16566-1-bibby.hsieh@mediatek.com>
Moving the driver to atomic helpers regressed cursor responsiveness,
because cursor updates need their own atomic commits, which have to be
serialized with other commits, that might include fence waits. To avoid
this, in certain conditions, we can bypass the atomic helpers for legacy
cursor update IOCTLs. Currently the conditions are:
- no asynchronous mode setting commit pending,
- no asynchronous commit that updates the cursor plane is pending.
With the above two conditions met, we know that the manual cursor state
update will not conflict with any scheduled update.
Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC MT8173.")
Signed-off-by: Bibby Hsieh <bibby.hsieh@mediatek.com>
Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
---
drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 41 ++++++++++++-
drivers/gpu/drm/mediatek/mtk_drm_crtc.h | 2 +
drivers/gpu/drm/mediatek/mtk_drm_drv.c | 34 ++++++++++-
drivers/gpu/drm/mediatek/mtk_drm_drv.h | 3 +
drivers/gpu/drm/mediatek/mtk_drm_plane.c | 73 +++++++++++++++++++++++-
5 files changed, 148 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index 7697b40baac0..092e502ed27b 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
@@ -40,7 +40,7 @@ struct mtk_drm_crtc {
struct drm_plane *planes;
unsigned int layer_nr;
bool pending_planes;
-
+ bool cursor_update;
void __iomem *config_regs;
const struct mtk_mmsys_reg_data *mmsys_reg_data;
struct mtk_disp_mutex *mutex;
@@ -386,8 +386,45 @@ static void mtk_crtc_ddp_config(struct drm_crtc *crtc)
}
}
mtk_crtc->pending_planes = false;
- mtk_atomic_state_put_queue(atomic_state);
+ if (!mtk_crtc->cursor_update)
+ mtk_atomic_state_put_queue(atomic_state);
+ mtk_crtc->cursor_update = false;
+ }
+}
+
+void mtk_drm_crtc_cursor_update(struct drm_crtc *crtc, struct drm_plane *plane,
+ struct drm_plane_state *plane_state)
+{
+ struct mtk_drm_private *priv = crtc->dev->dev_private;
+ struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
+ const struct drm_plane_helper_funcs *plane_helper_funcs =
+ plane->helper_private;
+ int i;
+
+ if (!mtk_crtc->enabled)
+ return;
+
+ mutex_lock(&priv->hw_lock);
+ plane_helper_funcs->atomic_update(plane, plane_state);
+ for (i = 0; i < mtk_crtc->layer_nr; i++) {
+ struct drm_plane *plane = &mtk_crtc->planes[i];
+ struct mtk_plane_state *plane_state;
+
+ plane_state = to_mtk_plane_state(plane->state);
+ if (plane_state->pending.dirty) {
+ plane_state->pending.config = true;
+ plane_state->pending.dirty = false;
+ }
+ }
+ mtk_crtc->pending_planes = true;
+ mtk_crtc->cursor_update = true;
+
+ if (priv->data->shadow_register) {
+ mtk_disp_mutex_acquire(mtk_crtc->mutex);
+ mtk_crtc_ddp_config(crtc);
+ mtk_disp_mutex_release(mtk_crtc->mutex);
}
+ mutex_unlock(&priv->hw_lock);
}
static void mtk_drm_crtc_atomic_enable(struct drm_crtc *crtc,
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.h b/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
index fcc134eb00c9..46e903be68ec 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
@@ -19,5 +19,7 @@ void mtk_crtc_ddp_irq(struct drm_crtc *crtc, struct mtk_ddp_comp *comp);
int mtk_drm_crtc_create(struct drm_device *drm_dev,
const enum mtk_ddp_comp_id *path,
unsigned int path_len);
+void mtk_drm_crtc_cursor_update(struct drm_crtc *crtc, struct drm_plane *plane,
+ struct drm_plane_state *plane_state);
#endif /* MTK_DRM_CRTC_H */
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index b0308a3a7483..ca754b954c7c 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -80,11 +80,36 @@ static int mtk_atomic_get_crtcs(struct drm_device *drm,
struct drm_atomic_state *state)
{
struct mtk_drm_private *private = drm->dev_private;
- uint32_t crtc_mask;
+ uint32_t crtc_mask, needs_modeset, has_cursor_plane;
+ struct drm_plane *plane;
+ struct drm_plane_state *plane_state;
+ struct drm_crtc *crtc;
+ struct drm_crtc_state *crtc_state;
+ int i;
int ret;
crtc_mask = mtk_atomic_crtc_mask(drm, state);
+ /*
+ * Allow cursor updates unless there is a pending modeset or cursor
+ * plane update.
+ */
+ needs_modeset = 0;
+ for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
+ if (crtc && drm_atomic_crtc_needs_modeset(crtc_state)) {
+ needs_modeset |= (1 << drm_crtc_index(crtc));
+ break;
+ }
+ }
+
+ has_cursor_plane = 0;
+ for_each_new_plane_in_state(state, plane, plane_state, i) {
+ if (crtc && plane->crtc && plane == plane->crtc->cursor) {
+ has_cursor_plane |= (1 << drm_crtc_index(crtc));
+ break;
+ }
+ }
+
/*
* Wait for all pending updates to complete for the set of crtcs being
* changed in this atomic commit
@@ -94,6 +119,8 @@ static int mtk_atomic_get_crtcs(struct drm_device *drm,
!(private->commit.crtcs & crtc_mask));
if (ret == 0)
private->commit.crtcs |= crtc_mask;
+
+ private->commit.flush_for_cursor |= needs_modeset | has_cursor_plane;
spin_unlock(&private->commit.crtcs_event.lock);
return ret;
@@ -112,6 +139,7 @@ static void mtk_atomic_put_crtcs(struct drm_device *drm,
spin_lock(&private->commit.crtcs_event.lock);
private->commit.crtcs &= ~crtc_mask;
+ private->commit.flush_for_cursor &= ~crtc_mask;
wake_up_all_locked(&private->commit.crtcs_event);
spin_unlock(&private->commit.crtcs_event.lock);
}
@@ -160,6 +188,7 @@ static void mtk_atomic_wait_for_fences(struct drm_atomic_state *state)
static void mtk_atomic_complete(struct drm_device *drm,
struct drm_atomic_state *state)
{
+ struct mtk_drm_private *private = drm->dev_private;
mtk_atomic_wait_for_fences(state);
/*
* Mediatek drm supports runtime PM, so plane registers cannot be
@@ -175,10 +204,12 @@ static void mtk_atomic_complete(struct drm_device *drm,
*
* See the kerneldoc entries for these three functions for more details.
*/
+ mutex_lock(&private->hw_lock);
drm_atomic_helper_commit_modeset_disables(drm, state);
drm_atomic_helper_commit_modeset_enables(drm, state);
drm_atomic_helper_commit_planes(drm, state,
DRM_PLANE_COMMIT_ACTIVE_ONLY);
+ mutex_unlock(&private->hw_lock);
drm_atomic_helper_wait_for_vblanks(drm, state);
@@ -462,6 +493,7 @@ static int mtk_drm_kms_init(struct drm_device *drm)
INIT_LIST_HEAD(&private->unreference.list);
spin_lock_init(&private->unreference.lock);
init_waitqueue_head(&private->commit.crtcs_event);
+ mutex_init(&private->hw_lock);
return 0;
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
index 0934f83b860d..4a4c989803d9 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
@@ -50,6 +50,7 @@ struct mtk_drm_private {
struct {
uint32_t crtcs;
wait_queue_head_t crtcs_event;
+ uint32_t flush_for_cursor;
} commit;
struct drm_atomic_state *suspend_state;
@@ -58,6 +59,8 @@ struct mtk_drm_private {
struct list_head list;
spinlock_t lock;
} unreference;
+
+ struct mutex hw_lock;
};
extern struct platform_driver mtk_ddp_driver;
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
index f2ef83aed6f9..59dbdaf07425 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
@@ -7,6 +7,7 @@
#include <drm/drmP.h>
#include <drm/drm_atomic.h>
#include <drm/drm_atomic_helper.h>
+#include <drm/drm_atomic_uapi.h>
#include <drm/drm_plane_helper.h>
#include "mtk_drm_crtc.h"
@@ -69,8 +70,71 @@ static void mtk_drm_plane_destroy_state(struct drm_plane *plane,
kfree(to_mtk_plane_state(state));
}
+static int mtk_plane_cursor_update(struct drm_plane *plane,
+ struct drm_crtc *crtc,
+ struct drm_framebuffer *fb,
+ int crtc_x, int crtc_y,
+ unsigned int crtc_w, unsigned int crtc_h,
+ uint32_t src_x, uint32_t src_y,
+ uint32_t src_w, uint32_t src_h)
+{
+ struct drm_plane_state *plane_state;
+ const struct drm_plane_helper_funcs *plane_helper_funcs =
+ plane->helper_private;
+ int ret;
+
+ plane_state = plane->funcs->atomic_duplicate_state(plane);
+
+ plane_state->crtc_x = crtc_x;
+ plane_state->crtc_y = crtc_y;
+ plane_state->crtc_h = crtc_h;
+ plane_state->crtc_w = crtc_w;
+ plane_state->src_x = src_x;
+ plane_state->src_y = src_y;
+ plane_state->src_h = src_h;
+ plane_state->src_w = src_w;
+
+ drm_atomic_set_fb_for_plane(plane_state, fb);
+
+ ret = plane_helper_funcs->atomic_check(plane, plane_state);
+ if (ret)
+ goto err_destroy;
+
+ swap(plane_state, plane->state);
+
+ mtk_drm_crtc_cursor_update(crtc, plane, plane_state);
+
+err_destroy:
+ plane->funcs->atomic_destroy_state(plane, plane_state);
+ return ret;
+}
+
+static int mtk_plane_update(struct drm_plane *plane,
+ struct drm_crtc *crtc,
+ struct drm_framebuffer *fb,
+ int crtc_x, int crtc_y,
+ unsigned int crtc_w, unsigned int crtc_h,
+ uint32_t src_x, uint32_t src_y,
+ uint32_t src_w, uint32_t src_h,
+ struct drm_modeset_acquire_ctx *ctx)
+{
+ struct mtk_drm_private *private = plane->dev->dev_private;
+ uint32_t crtc_mask = (1 << drm_crtc_index(crtc));
+
+ if (crtc && plane == crtc->cursor &&
+ plane->state->crtc == crtc &&
+ !(private->commit.flush_for_cursor & crtc_mask))
+ return mtk_plane_cursor_update(plane, crtc, fb,
+ crtc_x, crtc_y, crtc_w, crtc_h,
+ src_x, src_y, src_w, src_h);
+
+ return drm_atomic_helper_update_plane(plane, crtc, fb,
+ crtc_x, crtc_y, crtc_w, crtc_h,
+ src_x, src_y, src_w, src_h, ctx);
+}
+
static const struct drm_plane_funcs mtk_plane_funcs = {
- .update_plane = drm_atomic_helper_update_plane,
+ .update_plane = mtk_plane_update,
.disable_plane = drm_atomic_helper_disable_plane,
.destroy = drm_plane_cleanup,
.reset = mtk_plane_reset,
@@ -90,7 +154,12 @@ static int mtk_plane_atomic_check(struct drm_plane *plane,
if (!state->crtc)
return 0;
- crtc_state = drm_atomic_get_crtc_state(state->state, state->crtc);
+ if (state->state)
+ crtc_state = drm_atomic_get_existing_crtc_state(state->state,
+ state->crtc);
+ else /* Special case for asynchronous cursor updates. */
+ crtc_state = state->crtc->state;
+
if (IS_ERR(crtc_state))
return PTR_ERR(crtc_state);
--
2.18.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 0/2] drm/mediatek: fixup cursor moving unsmooth issue
From: Bibby Hsieh @ 2019-08-30 7:38 UTC (permalink / raw)
To: David Airlie, Matthias Brugger, Daniel Vetter, dri-devel,
linux-mediatek
Cc: drinkcat, Bibby Hsieh, linux-kernel, tfiga, CK Hu, Thierry Reding,
Philipp Zabel, YT Shen, linux-arm-kernel
These patches can fixup cursor moving is not smooth when heavy load in
webgl.
Bibby Hsieh (2):
drm/mediatek: Only block updates to CRTCs that have a pending update
drm/mediatek: Bypass atomic helpers for cursor updates
drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 53 +++++-
drivers/gpu/drm/mediatek/mtk_drm_crtc.h | 2 +
drivers/gpu/drm/mediatek/mtk_drm_drv.c | 214 ++++++++++++++++++++---
drivers/gpu/drm/mediatek/mtk_drm_drv.h | 15 +-
drivers/gpu/drm/mediatek/mtk_drm_plane.c | 73 +++++++-
5 files changed, 330 insertions(+), 27 deletions(-)
--
2.18.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox