* Re: [PATCH v1 7/9] ASoC: samsung: arndale: Add support for WM1811 CODEC
From: Sylwester Nawrocki @ 2019-09-19 12:00 UTC (permalink / raw)
To: Charles Keepax
Cc: devicetree, alsa-devel, linux-samsung-soc, b.zolnierkie, sbkim73,
patches, lgirdwood, krzk, robh+dt, broonie, linux-arm-kernel,
m.szyprowski
In-Reply-To: <20190918144553.GJ10204@ediswmail.ad.cirrus.com>
On 9/18/19 16:45, Charles Keepax wrote:
> If your removing the of_match_ptr below I think the
> __maybe_unused should be removed as well.
Good point, I will remove the now unneeded __maybe_unused as well.
--
Thanks,
Sylwester
_______________________________________________
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 v2] serial: imx: adapt rx buffer and dma periods
From: Uwe Kleine-König @ 2019-09-19 12:00 UTC (permalink / raw)
To: Philipp Puschmann
Cc: fugang.duan, kernel, gregkh, s.hauer, linux-kernel, linux-imx,
linux-serial, jslaby, shawnguo, yibin.gong, festevam,
linux-arm-kernel, l.stach
In-Reply-To: <9923d405-a62d-0c6a-6039-f4a7bc3bf2e8@emlix.com>
On Thu, Sep 19, 2019 at 01:40:03PM +0200, Philipp Puschmann wrote:
> Hi Uwe
>
> Am 19.09.19 um 13:22 schrieb Uwe Kleine-König:
> > On Thu, Sep 19, 2019 at 12:26:28PM +0200, Philipp Puschmann wrote:
> >> Using only 4 DMA periods for UART RX is very few if we have a high
> >> frequency of small transfers - like in our case using Bluetooth with
> >> many small packets via UART - causing many dma transfers but in each
> >> only filling a fraction of a single buffer. Such a case may lead to
> >> the situation that DMA RX transfer is triggered but no free buffer is
> >> available. While we have addressed the dma handling already with
> >> "dmaengine: imx-sdma: fix dma freezes" we still want to avoid
> >
> > Is this statement still true now that you split this patch out of your
> > bigger series?
> Yes. The dma patches care about stopping DMA channel. This patch tries to
> avoid that the channel runs out of usable buffers (aka dma periods).
>
> >
> >> UART RX FIFO overrun. So we decrease the size of the buffers and
> >> increase their number and the total buffer size.
> >
> > What happens when such an RX FIFO overrun happens? Are characters lost?
> > Or only time?
> Good question. In explanation i have missed an important point:
> When using HW flowcontrol via RTS/CTS and the buffer is full CTS is used to
> tell the remote device - here the Bluetooth chip - to stop sending data.
> For a while this prevents losing of characters. But then the remote device
> comes into trouble as its internal TX buffers runs over. Depends on the
> device how it handles this case and if it recovers if data flow is enabled
> again.
>
> In case without HW flow control characters would be lost. Depends on the upper
> layer what happens then.
>
> > Does your change have an influence if I do fewer but
> > bigger transfers?
> Don't think so. The dma periods are raw data buffers. If one is full the next one
> is being used. For the performance i don't see a significant difference between
> using 1 kB buffers or 4 kB buffers.
Would be great to add these infos to the commit log.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH 0/3] arm64: meson: add support for A1 Power Domains
From: Jianxin Pan @ 2019-09-19 12:11 UTC (permalink / raw)
To: Kevin Hilman, linux-amlogic
Cc: devicetree, Hanjie Lin, Victor Wan, Jianxin Pan, linux-pm,
Martin Blumenstingl, Neil Armstrong, linux-kernel, Rob Herring,
Jian Hu, Xingyu Chen, linux-arm-kernel, Jerome Brunet
This patchset introduces a "Secure Power Doamin Controller". In A1/C1, power
controller registers such as PWRCTRL_FOCRSTN, PWRCTRL_PWR_OFF, PWRCTRL_MEM_PD
and PWRCTRL_ISO_EN, are in the secure domain, and should be accessed from ATF
by smc.
This patchset is based on A1 DTB series at [0].
[0] https://lore.kernel.org/linux-amlogic/1568276370-54181-1-git-send-email-jianxin.pan@amlogic.com
Jianxin Pan (3):
dt-bindings: power: add Amlogic secure power domains bindings
soc: amlogic: Add support for Secure power domains controller
arm64: dts: meson: a1: add secure power domain controller
.../bindings/power/amlogic,meson-sec-pwrc.yaml | 32 ++++
arch/arm64/boot/dts/amlogic/meson-a1.dtsi | 6 +
drivers/soc/amlogic/Kconfig | 13 ++
drivers/soc/amlogic/Makefile | 1 +
drivers/soc/amlogic/meson-secure-pwrc.c | 182 +++++++++++++++++++++
include/dt-bindings/power/meson-a1-power.h | 32 ++++
6 files changed, 266 insertions(+)
create mode 100644 Documentation/devicetree/bindings/power/amlogic,meson-sec-pwrc.yaml
create mode 100644 drivers/soc/amlogic/meson-secure-pwrc.c
create mode 100644 include/dt-bindings/power/meson-a1-power.h
--
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 1/3] dt-bindings: power: add Amlogic secure power domains bindings
From: Jianxin Pan @ 2019-09-19 12:11 UTC (permalink / raw)
To: Kevin Hilman, linux-amlogic
Cc: devicetree, Hanjie Lin, Victor Wan, Jianxin Pan, Neil Armstrong,
Martin Blumenstingl, linux-pm, linux-kernel, Zhiqiang Liang,
Rob Herring, Jian Hu, Xingyu Chen, linux-arm-kernel,
Jerome Brunet
In-Reply-To: <1568895064-4116-1-git-send-email-jianxin.pan@amlogic.com>
Add the bindings for the Amlogic Secure power domains, controlling the
secure power domains.
The bindings targets the Amlogic A1 and C1 compatible SoCs, in which the
power domain registers are in secure world.
Signed-off-by: Jianxin Pan <jianxin.pan@amlogic.com>
Signed-off-by: Zhiqiang Liang <zhiqiang.liang@amlogic.com>
---
.../bindings/power/amlogic,meson-sec-pwrc.yaml | 32 ++++++++++++++++++++++
include/dt-bindings/power/meson-a1-power.h | 32 ++++++++++++++++++++++
2 files changed, 64 insertions(+)
create mode 100644 Documentation/devicetree/bindings/power/amlogic,meson-sec-pwrc.yaml
create mode 100644 include/dt-bindings/power/meson-a1-power.h
diff --git a/Documentation/devicetree/bindings/power/amlogic,meson-sec-pwrc.yaml b/Documentation/devicetree/bindings/power/amlogic,meson-sec-pwrc.yaml
new file mode 100644
index 00000000..327e0d9
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/amlogic,meson-sec-pwrc.yaml
@@ -0,0 +1,32 @@
+# SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+# Copyright (c) 2019 Amlogic, Inc
+# Author: Jianxin Pan <jianxin.pan@amlogic.com>
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/power/amlogic,meson-sec-pwrc.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Amlogic Meson Secure Power Domains
+
+maintainers:
+ - Jianxin Pan <jianxin.pan@amlogic.com>
+
+description: |+
+ A1/C1 series The Secure Power Domains node should be the child of a syscon
+ node with the required property.
+
+properties:
+ compatible:
+ enum:
+ - amlogic,meson-a1-pwrc
+
+required:
+ - compatible
+
+examples:
+ - |
+ pwrc: power-controller {
+ compatible = "amlogic,meson-a1-pwrc";
+ };
+
+
diff --git a/include/dt-bindings/power/meson-a1-power.h b/include/dt-bindings/power/meson-a1-power.h
new file mode 100644
index 00000000..6cf50bf
--- /dev/null
+++ b/include/dt-bindings/power/meson-a1-power.h
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: (GPL-2.0+ or MIT) */
+/*
+ * Copyright (c) 2019 Amlogic, Inc.
+ * Author: Jianxin Pan <jianxin.pan@amlogic.com>
+ */
+
+#ifndef _DT_BINDINGS_MESON_A1_POWER_H
+#define _DT_BINDINGS_MESON_A1_POWER_H
+
+#define PWRC_DSPA_ID 8
+#define PWRC_DSPB_ID 9
+#define PWRC_UART_ID 10
+#define PWRC_DMC_ID 11
+#define PWRC_I2C_ID 12
+#define PWRC_PSRAM_ID 13
+#define PWRC_ACODEC_ID 14
+#define PWRC_AUDIO_ID 15
+#define PWRC_OTP_ID 16
+#define PWRC_DMA_ID 17
+#define PWRC_SD_EMMC_ID 18
+#define PWRC_RAMA_ID 19
+#define PWRC_RAMB_ID 20
+#define PWRC_IR_ID 21
+#define PWRC_SPICC_ID 22
+#define PWRC_SPIFC_ID 23
+#define PWRC_USB_ID 24
+#define PWRC_NIC_ID 25
+#define PWRC_PDMIN_ID 26
+#define PWRC_RSA_ID 27
+#define PWRC_MAX_ID 28
+
+#endif
--
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 2/3] soc: amlogic: Add support for Secure power domains controller
From: Jianxin Pan @ 2019-09-19 12:11 UTC (permalink / raw)
To: Kevin Hilman, linux-amlogic
Cc: devicetree, Hanjie Lin, Victor Wan, Jianxin Pan, Neil Armstrong,
Martin Blumenstingl, linux-pm, linux-kernel, Zhiqiang Liang,
Rob Herring, Jian Hu, Xingyu Chen, linux-arm-kernel,
Jerome Brunet
In-Reply-To: <1568895064-4116-1-git-send-email-jianxin.pan@amlogic.com>
Add support for the Amlogic Secure Power controller. In A1/C1 series, power
control registers are in secure domain, and should be accessed by smc.
Signed-off-by: Jianxin Pan <jianxin.pan@amlogic.com>
Signed-off-by: Zhiqiang Liang <zhiqiang.liang@amlogic.com>
---
drivers/soc/amlogic/Kconfig | 13 +++
drivers/soc/amlogic/Makefile | 1 +
drivers/soc/amlogic/meson-secure-pwrc.c | 182 ++++++++++++++++++++++++++++++++
3 files changed, 196 insertions(+)
create mode 100644 drivers/soc/amlogic/meson-secure-pwrc.c
diff --git a/drivers/soc/amlogic/Kconfig b/drivers/soc/amlogic/Kconfig
index bc2c912..6cb06e7 100644
--- a/drivers/soc/amlogic/Kconfig
+++ b/drivers/soc/amlogic/Kconfig
@@ -48,6 +48,19 @@ config MESON_EE_PM_DOMAINS
Say yes to expose Amlogic Meson Everything-Else Power Domains as
Generic Power Domains.
+config MESON_SECURE_PM_DOMAINS
+ bool "Amlogic Meson Secure Power Domains driver"
+ depends on ARCH_MESON || COMPILE_TEST
+ depends on PM && OF
+ depends on HAVE_ARM_SMCCC
+ default ARCH_MESON
+ select PM_GENERIC_DOMAINS
+ select PM_GENERIC_DOMAINS_OF
+ help
+ Support for the power controller on Amlogic A1/C1 series.
+ Say yes to expose Amlogic Meson Secure Power Domains as Generic
+ Power Domains.
+
config MESON_MX_SOCINFO
bool "Amlogic Meson MX SoC Information driver"
depends on ARCH_MESON || COMPILE_TEST
diff --git a/drivers/soc/amlogic/Makefile b/drivers/soc/amlogic/Makefile
index de79d044..7b8c5d3 100644
--- a/drivers/soc/amlogic/Makefile
+++ b/drivers/soc/amlogic/Makefile
@@ -5,3 +5,4 @@ obj-$(CONFIG_MESON_GX_SOCINFO) += meson-gx-socinfo.o
obj-$(CONFIG_MESON_GX_PM_DOMAINS) += meson-gx-pwrc-vpu.o
obj-$(CONFIG_MESON_MX_SOCINFO) += meson-mx-socinfo.o
obj-$(CONFIG_MESON_EE_PM_DOMAINS) += meson-ee-pwrc.o
+obj-$(CONFIG_MESON_SECURE_PM_DOMAINS) += meson-secure-pwrc.o
diff --git a/drivers/soc/amlogic/meson-secure-pwrc.c b/drivers/soc/amlogic/meson-secure-pwrc.c
new file mode 100644
index 00000000..00c7232
--- /dev/null
+++ b/drivers/soc/amlogic/meson-secure-pwrc.c
@@ -0,0 +1,182 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (c) 2019 Amlogic, Inc.
+ * Author: Jianxin Pan <jianxin.pan@amlogic.com>
+ */
+#include <linux/io.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+#include <dt-bindings/power/meson-a1-power.h>
+#include <linux/arm-smccc.h>
+
+#define PWRC_ON 1
+#define PWRC_OFF 0
+#define SMC_PWRC_SET 0x82000093
+#define SMC_PWRC_GET 0x82000095
+
+struct meson_secure_pwrc_domain {
+ struct generic_pm_domain base;
+ unsigned int index;
+};
+
+struct meson_secure_pwrc {
+ struct meson_secure_pwrc_domain *domains;
+ struct genpd_onecell_data xlate;
+};
+
+struct meson_secure_pwrc_domain_desc {
+ unsigned int index;
+ unsigned int flags;
+ char *name;
+ bool (*get_power)(struct meson_secure_pwrc_domain *pwrc_domain);
+};
+
+struct meson_secure_pwrc_domain_data {
+ unsigned int count;
+ struct meson_secure_pwrc_domain_desc *domains;
+};
+
+static bool pwrc_secure_get_power(struct meson_secure_pwrc_domain *pwrc_domain)
+{
+ struct arm_smccc_res res;
+
+ arm_smccc_smc(SMC_PWRC_GET, pwrc_domain->index, 0,
+ 0, 0, 0, 0, 0, &res);
+
+ return res.a0 & 0x1;
+}
+
+static int meson_secure_pwrc_off(struct generic_pm_domain *domain)
+{
+ struct arm_smccc_res res;
+ struct meson_secure_pwrc_domain *pwrc_domain =
+ container_of(domain, struct meson_secure_pwrc_domain, base);
+
+ arm_smccc_smc(SMC_PWRC_SET, pwrc_domain->index, PWRC_OFF,
+ 0, 0, 0, 0, 0, &res);
+
+ return 0;
+}
+
+static int meson_secure_pwrc_on(struct generic_pm_domain *domain)
+{
+ struct arm_smccc_res res;
+ struct meson_secure_pwrc_domain *pwrc_domain =
+ container_of(domain, struct meson_secure_pwrc_domain, base);
+
+ arm_smccc_smc(SMC_PWRC_SET, pwrc_domain->index, PWRC_ON,
+ 0, 0, 0, 0, 0, &res);
+
+ return 0;
+}
+
+#define SEC_PD(__name, __flag) \
+{ \
+ .name = #__name, \
+ .index = PWRC_##__name##_ID, \
+ .get_power = pwrc_secure_get_power, \
+ .flags = __flag, \
+}
+
+static struct meson_secure_pwrc_domain_desc a1_pwrc_domains[] = {
+ SEC_PD(DSPA, 0),
+ SEC_PD(DSPB, 0),
+ SEC_PD(UART, GENPD_FLAG_ALWAYS_ON),
+ SEC_PD(DMC, GENPD_FLAG_ALWAYS_ON),
+ SEC_PD(I2C, 0),
+ SEC_PD(PSRAM, 0),
+ SEC_PD(ACODEC, 0),
+ SEC_PD(AUDIO, 0),
+ SEC_PD(OTP, 0),
+ SEC_PD(DMA, 0),
+ SEC_PD(SD_EMMC, 0),
+ SEC_PD(RAMA, 0),
+ SEC_PD(RAMB, GENPD_FLAG_ALWAYS_ON),
+ SEC_PD(IR, 0),
+ SEC_PD(SPICC, 0),
+ SEC_PD(SPIFC, 0),
+ SEC_PD(USB, 0),
+ SEC_PD(NIC, GENPD_FLAG_ALWAYS_ON),
+ SEC_PD(PDMIN, 0),
+ SEC_PD(RSA, 0),
+};
+
+static int meson_secure_pwrc_probe(struct platform_device *pdev)
+{
+ const struct meson_secure_pwrc_domain_data *match;
+ struct meson_secure_pwrc *pwrc;
+ int i;
+
+ match = of_device_get_match_data(&pdev->dev);
+ if (!match) {
+ dev_err(&pdev->dev, "failed to get match data\n");
+ return -ENODEV;
+ }
+
+ pwrc = devm_kzalloc(&pdev->dev, sizeof(*pwrc), GFP_KERNEL);
+ if (!pwrc)
+ return -ENOMEM;
+
+ pwrc->xlate.domains = devm_kcalloc(&pdev->dev, match->count,
+ sizeof(*pwrc->xlate.domains),
+ GFP_KERNEL);
+ if (!pwrc->xlate.domains)
+ return -ENOMEM;
+
+ pwrc->domains = devm_kcalloc(&pdev->dev, match->count,
+ sizeof(*pwrc->domains), GFP_KERNEL);
+ if (!pwrc->domains)
+ return -ENOMEM;
+
+ pwrc->xlate.num_domains = match->count;
+ platform_set_drvdata(pdev, pwrc);
+
+ for (i = 0 ; i < match->count ; ++i) {
+ struct meson_secure_pwrc_domain *dom = &pwrc->domains[i];
+
+ if (!match->domains[i].index)
+ continue;
+
+ dom->index = match->domains[i].index;
+ dom->base.name = match->domains[i].name;
+ dom->base.flags = match->domains[i].flags;
+ dom->base.power_on = meson_secure_pwrc_on;
+ dom->base.power_off = meson_secure_pwrc_off;
+
+ pm_genpd_init(&dom->base, NULL,
+ (match->domains[i].get_power ?
+ match->domains[i].get_power(dom) : true));
+
+ pwrc->xlate.domains[i] = &dom->base;
+ }
+
+ return of_genpd_add_provider_onecell(pdev->dev.of_node, &pwrc->xlate);
+}
+
+static struct meson_secure_pwrc_domain_data meson_secure_a1_pwrc_data = {
+ .domains = a1_pwrc_domains,
+ .count = ARRAY_SIZE(a1_pwrc_domains),
+};
+
+static const struct of_device_id meson_secure_pwrc_match_table[] = {
+ {
+ .compatible = "amlogic,meson-a1-pwrc",
+ .data = &meson_secure_a1_pwrc_data,
+ },
+ { }
+};
+
+static struct platform_driver meson_secure_pwrc_driver = {
+ .probe = meson_secure_pwrc_probe,
+ .driver = {
+ .name = "meson_secure_pwrc",
+ .of_match_table = meson_secure_pwrc_match_table,
+ },
+};
+
+static int meson_secure_pwrc_init(void)
+{
+ return platform_driver_register(&meson_secure_pwrc_driver);
+}
+arch_initcall_sync(meson_secure_pwrc_init);
--
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 3/3] arm64: dts: meson: a1: add secure power domain controller
From: Jianxin Pan @ 2019-09-19 12:11 UTC (permalink / raw)
To: Kevin Hilman, linux-amlogic
Cc: devicetree, Hanjie Lin, Victor Wan, Jianxin Pan, Neil Armstrong,
Martin Blumenstingl, linux-pm, linux-kernel, Zhiqiang Liang,
Rob Herring, Jian Hu, Xingyu Chen, linux-arm-kernel,
Jerome Brunet
In-Reply-To: <1568895064-4116-1-git-send-email-jianxin.pan@amlogic.com>
Enable power domain controller for Meson A1 SoC.
Signed-off-by: Jianxin Pan <jianxin.pan@amlogic.com>
Signed-off-by: Zhiqiang Liang <zhiqiang.liang@amlogic.com>
---
arch/arm64/boot/dts/amlogic/meson-a1.dtsi | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
index 7210ad0..d689e5c 100644
--- a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
@@ -93,6 +93,12 @@
clock-names = "xtal", "pclk", "baud";
status = "disabled";
};
+
+ pwrc: power-controller {
+ compatible = "amlogic,meson-a1-pwrc";
+ #power-domain-cells = <1>;
+ status = "okay";
+ };
};
gic: interrupt-controller@ff901000 {
--
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
* Re: [RFC PATCH v3 4/6] psci: Add hvc call service for ptp_kvm.
From: Paolo Bonzini @ 2019-09-19 12:13 UTC (permalink / raw)
To: Marc Zyngier, Jianyong Wu (Arm Technology China),
netdev@vger.kernel.org, yangbo.lu@nxp.com, john.stultz@linaro.org,
tglx@linutronix.de, sean.j.christopherson@intel.com,
richardcochran@gmail.com, Mark Rutland, Will Deacon,
Suzuki Poulose
Cc: Justin He (Arm Technology China), kvm@vger.kernel.org,
Steve Capper, linux-kernel@vger.kernel.org,
Kaly Xin (Arm Technology China), nd,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <a1b554b8-4417-5305-3419-fe71a8c50842@kernel.org>
On 19/09/19 13:39, Marc Zyngier wrote:
>> I don't think it's ugly but more important, using tk->tkr_mono.clock is
>> incorrect. See how the x86 code hardcodes &kvm_clock, it's the same for
>> ARM.
> Not really. The guest kernel is free to use any clocksource it wishes.
Understood, in fact it's the same on x86.
However, for PTP to work, the cycles value returned by the clocksource
must match the one returned by the hypercall. So for ARM
get_device_system_crosststamp must receive the arch timer clocksource,
so that it will return -ENODEV if the active clocksource is anything else.
Paolo
> In some cases, it is actually desirable (like these broken systems that
> cannot use an in-kernel irqchip...). Maybe it is that on x86 the guest
> only uses the kvm_clock, but that's a much harder sell on ARM. The fact
> that ptp_kvm assumes that the clocksource is fixed doesn't seem correct
> in that case.
_______________________________________________
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] drm: sun8i-ui/vi: Fix layer zpos change/atomic modesetting
From: Ondřej Jirman @ 2019-09-19 12:20 UTC (permalink / raw)
To: Maxime Ripard
Cc: David Airlie, linux-kernel, dri-devel, Chen-Yu Tsai,
Daniel Vetter, linux-arm-kernel
In-Reply-To: <20190918201617.5gwzmshoxbcxbmrx@gilmour>
On Wed, Sep 18, 2019 at 10:16:17PM +0200, Maxime Ripard wrote:
> On Wed, Sep 18, 2019 at 05:23:09PM +0200, Ondřej Jirman wrote:
> > Hi,
> >
> > On Wed, Sep 18, 2019 at 04:17:34PM +0200, Maxime Ripard wrote:
> > > Hi,
> > >
> > > On Sun, Sep 15, 2019 at 12:03:37AM +0200, megous@megous.com wrote:
> > > > From: Ondrej Jirman <megous@megous.com>
> > > >
> > > > There are various issues that this re-work of sun8i_[uv]i_layer_enable
> > > > function fixes:
> > > >
> > > > - Make sure that we re-initialize zpos on reset
> > > > - Minimize register updates by doing them only when state changes
> > > > - Fix issue where DE pipe might get disabled even if it is no longer
> > > > used by the layer that's currently calling sun8i_ui_layer_enable
> > > > - .atomic_disable callback is not really needed because .atomic_update
> > > > can do the disable too, so drop the duplicate code
> > > >
> > > > Signed-off-by: Ondrej Jirman <megous@megous.com>
> > >
> > > It looks like these fixes should be in separate patches. Is there any
> > > reason it's not the case?
> >
> > Bullet points just describe the resulting effect/benefits of the change to fix
> > the pipe control register update issue (see the referenced e-mail).
>
> It's definitely ok to have multiple patches needed to address a single
> perceived issue.
Yes, but I can't simply split the patch. In order for each change to work on its
own, they'd have to be done differently than the final result.
I wouldn't mind at all if it was just a simple splitting, but you're asking
for too much work, this time, for no benefit that I can see.
> A commit is not about what you're fixing but what you're changing. And
> the fact that you have tha bullet list in the first place proves that
> you have multiple logical changes in your patch.
>
> And even then, your commit log mentions that you're fixing multiple
> issues (without explaining them).
I can reword the commit message if that helps, and skip the bullet list if it
is confusing. There's a single core issue and that is that the driver doesn't
update the pipe/channel configuration correctly leading to disabling of
arbitrary layers (not even those being updated - update of UI layer may disable
VI layer as a side effect for example) at wrong times. And only changes
necessary to debug/fix this are included.
I may try generating a nicer patch with a different diff options, if it makes it
more readable for review.
> > I can maybe split off the first bullet point into a separate patch. But
> > I can't guarantee it will not make the original issue worse, because it might
> > have been hiding the other issue with register updates.
> >
> > The rest is just a result of the single logical change. It doesn't work
> > individually, it all has the goal of fixing the issue as a whole.
> >
> > If I were to split it I would have to actually re-implement .atomic_disable
> > callback only to remove it in the next patch. I don't see the benefit.
>
> Your commit log says that you remove atomic_disable. Why would you
> remove it, to add it back, to remove it again?
Because if I remove it I need to re-implement the functionality in the update
callback. The core will change what is called based on presence of callbacks.
It's not a simple removal.
If I first implement the new sun8i_[uv]i_layer_enable and update callback,
keeping a disable callback would not work, because the new update callback
will only work if disable callback is not defined (because it it is, then
the drm core will not call the update callback in all cases that I need).
regards,
o.
> Maxime
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
_______________________________________________
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 1/1] sched/rt: avoid contend with CFS task
From: Vincent Guittot @ 2019-09-19 12:27 UTC (permalink / raw)
To: Jing-Ting Wu
Cc: wsd_upstream, Peter Zijlstra, linux-kernel, Valentin Schneider,
linux-mediatek, Matthias Brugger, Qais Yousef, LAK
In-Reply-To: <1568892135.4892.10.camel@mtkswgap22>
On Thu, 19 Sep 2019 at 13:22, Jing-Ting Wu <jing-ting.wu@mediatek.com> wrote:
>
> On Thu, 2019-09-05 at 16:01 +0200, Vincent Guittot wrote:
> > Hi Jing-Ting,
> >
> > On Thu, 5 Sep 2019 at 15:26, Jing-Ting Wu <jing-ting.wu@mediatek.com> wrote:
> > >
> > > On Fri, 2019-08-30 at 15:55 +0100, Qais Yousef wrote:
> > > > On 08/29/19 11:38, Valentin Schneider wrote:
> > > > > On 29/08/2019 04:15, Jing-Ting Wu wrote:
> > > > > > At original linux design, RT & CFS scheduler are independent.
> > > > > > Current RT task placement policy will select the first cpu in
> > > > > > lowest_mask, even if the first CPU is running a CFS task.
> > > > > > This may put RT task to a running cpu and let CFS task runnable.
> > > > > >
> > > > > > So we select idle cpu in lowest_mask first to avoid preempting
> > > > > > CFS task.
> > > > > >
> > > > >
> > > > > Regarding the RT & CFS thing, that's working as intended. RT is a whole
> > > > > class above CFS, it shouldn't have to worry about CFS.
> > > > >
> > > > > On the other side of things, CFS does worry about RT. We have the concept
> > > > > of RT-pressure in the CFS scheduler, where RT tasks will reduce a CPU's
> > > > > capacity (see fair.c::scale_rt_capacity()).
> > > > >
> > > > > CPU capacity is looked at on CFS wakeup (see wake_cap() and
> > > > > find_idlest_cpu()), and the periodic load balancer tries to spread load
> > > > > over capacity, so it'll tend to put less things on CPUs that are also
> > > > > running RT tasks.
> > > > >
> > > > > If RT were to start avoiding rqs with CFS tasks, we'd end up with a nasty
> > > > > situation were both are avoiding each other. It's even more striking when
> > > > > you see that RT pressure is done with a rq-wide RT util_avg, which
> > > > > *doesn't* get migrated when a RT task migrates. So if you decide to move
> > > > > a RT task to an idle CPU "B" because CPU "A" had runnable CFS tasks, the
> > > > > CFS scheduler will keep seeing CPU "B" as not significantly RT-pressured
> > > > > while that util_avg signal ramps up, whereas it would correctly see CPU
> > > > > "A" as RT-pressured if the RT task previously ran there.
> > > > >
> > > > > So overall I think this is the wrong approach.
> > > >
> > > > I like the idea, but yeah tend to agree the current approach might not be
> > > > enough.
> > > >
> > > > I think the major problem here is that on generic systems where CFS is a first
> > > > class citizen, RT tasks can be hostile to them - not always necessarily for a
> > > > good reason.
> > > >
> > > > To further complicate the matter, even among CFS tasks we can't tell which are
> > > > more important than the others - though hopefully latency-nice proposal will
> > > > make the situation better.
> > > >
> > > > So I agree we have a problem here, but I think this patch is just a temporary
> > > > band aid and we need to do better. Though I have no concrete suggestion yet on
> > > > how to do that.
> > > >
> > > > Another thing I couldn't quantify yet how common and how severe this problem is
> > > > yet. Jing-Ting, if you can share the details of your use case that'd be great.
> > > >
> > > > Cheers
> > > >
> > > > --
> > > > Qais Yousef
> > >
> > >
> > > I agree that the nasty situation will happen.The current approach and this patch might not be enough.
> >
> > RT task should not harm its cache hotness and responsiveness for the
> > benefit of a CFS task
> >
>
> Yes, it’s a good point to both consider cache hotness. We have revised
> the implementation to select a better idle CPU in the same sched_domain
> of prev_cpu (with the same cache hotness) when the RT task wakeup.
>
> I modify the code of find_lowest_rq as following:
> @@ -1648,6 +1629,9 @@ static int find_lowest_rq(struct task_struct
> *task)
> struct cpumask *lowest_mask =
> this_cpu_cpumask_var_ptr(local_cpu_mask);
> int this_cpu = smp_processor_id();
> int cpu = task_cpu(task);
> + int i;
> + struct rq *prev_rq = cpu_rq(cpu);
> + struct sched_domain *prev_sd;
>
> /* Make sure the mask is initialized first */
> if (unlikely(!lowest_mask))
> @@ -1659,6 +1643,24 @@ static int find_lowest_rq(struct task_struct
> *task)
> if (!cpupri_find(&task_rq(task)->rd->cpupri, task, lowest_mask))
> return -1; /* No targets found */
>
> + /* Choose previous cpu if it is idle and it fits lowest_mask */
> + if (cpumask_test_cpu(cpu, lowest_mask) && idle_cpu(cpu))
> + return cpu;
> +
> + rcu_read_lock();
> + prev_sd = rcu_dereference(prev_rq->sd);
> +
> + if (prev_sd) {
> + /* Choose idle_cpu among lowest_mask and it is closest
> to our hot cache data */
> + for_each_cpu(i, lowest_mask) {
> + if (idle_cpu(i) && cpumask_test_cpu(i,
> sched_domain_span(prev_sd))) {
> + rcu_read_unlock();
> + return i;
> + }
> + }
> + }
> + rcu_read_unlock();
> +
> /*
> * At this point we have built a mask of CPUs representing the
> * lowest priority tasks in the system. Now we want to elect
>
>
>
> > > But for requirement of performance, I think it is better to differentiate between idle CPU and CPU has CFS task.
> > >
> > > For example, we use rt-app to evaluate runnable time on non-patched environment.
> > > There are (NR_CPUS-1) heavy CFS tasks and 1 RT Task. When a CFS task is running, the RT task wakes up and choose the same CPU.
> > > The CFS task will be preempted and keep runnable until it is migrated to another cpu by load balance.
> > > But load balance is not triggered immediately, it will be triggered until timer tick hits with some condition satisfied(ex. rq->next_balance).
> >
> > Yes you will have to wait for the next tick that will trigger an idle
> > load balance because you have an idle cpu and 2 runnable tack (1 RT +
> > 1CFS) on the same CPU. But you should not wait for more than 1 tick
> >
> > The current load_balance doesn't handle correctly the situation of 1
> > CFS and 1 RT task on same CPU while 1 CPU is idle. There is a rework
> > of the load_balance that is under review on the mailing list that
> > fixes this problem and your CFS task should migrate to the idle CPU
> > faster than now
> >
>
> Period load balance should be triggered when current jiffies is behind
> rq->next_balance, but rq->next_balance is not often exactly the same
> with next tick.
> If cpu_busy, interval = sd->balance_interval * sd->busy_factor, and
But if there is an idle CPU on the system, the next idle load balance
should apply shortly because the busy_factor is not used for this CPU
which is not busy.
In this case, the next_balance interval is sd_weight which is probably
4ms at cluster level and 8ms at system level in your case. This means
between 1 and 2 ticks
longer interval means that :
-all cpu are busy,
-the cpu has trigger an all_pinned case
-the idle load balance fails to migrate the task. And this is probably
the your case. https://lore.kernel.org/patchwork/patch/1129117/ should
reduce time before migrating the CFS task to 1-2 ticks
> interval is clamped by 1 to max_load_balance_interval.
> By experiment, in a system with HZ=250, available_cpus = 8, the
> max_load_balance_interval = HZ * available_cpus / 10 = 250 * 8 / 10 =
> 200 jiffies,
> It would let rq->next_balance = sd->last_balance + interval, the maximum
> interval = 200 jiffies, result in more than 1 sched-tick to migrate a
> CFS task.
>
>
>
> > > CFS tasks may be runnable for a long time. In this test case, it increase 332.091 ms runnable time for CFS task.
> > >
> > > The detailed log is shown as following, CFS task(thread1-6580) is preempted by RT task(thread0-6674) about 332ms:
> >
> > 332ms is quite long and is probably not an idle load blanace but a
> > busy load balance
> >
> > > thread1-6580 [003] dnh2 94.452898: sched_wakeup: comm=thread0 pid=6674 prio=89 target_cpu=003
> > > thread1-6580 [003] d..2 94.452916: sched_switch: prev_comm=thread1 prev_pid=6580 prev_prio=120 prev_state=R ==> next_comm=thread0 next_pid=6674 next_prio=89
> > > .... 332.091ms
> > > krtatm-1930 [001] d..2 94.785007: sched_migrate_task: comm=thread1 pid=6580 prio=120 orig_cpu=3 dest_cpu=1
> > > krtatm-1930 [001] d..2 94.785020: sched_switch: prev_comm=krtatm prev_pid=1930 prev_prio=100 prev_state=S ==> next_comm=thread1 next_pid=6580 next_prio=120
> >
> > your CFS task has not moved on the idle CPU but has replaced another task
> >
>
> I think it is minor and reasonable, because CPU1 has triggered idle
> balance (when krtatm task is the last task leaving CPU1) to pull the
> thread1-6580.
so it was a newly-idle load_balance not an idle load balance
>
>
>
> Best regards,
> Jing-Ting Wu
>
> > Regards,
> > Vincent
> > >
> > > So I think choose idle CPU at RT wake up flow could reduce the CFS runnable time.
> > >
> > >
> > > Best regards,
> > > Jing-Ting Wu
> > >
> > >
>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCHv6 01/10] dt-bindings: omap: add new binding for PRM instances
From: Tero Kristo @ 2019-09-19 12:30 UTC (permalink / raw)
To: ssantosh, p.zabel, robh+dt, tony; +Cc: devicetree, linux-omap, linux-arm-kernel
In-Reply-To: <20190917174817.GA27938@bogus>
Add new binding for OMAP PRM (Power and Reset Manager) instances. Each
of these will act as a power domain controller and potentially as a reset
provider.
Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
v6: added common compatible as per request from Tony Lindgren. This is
to simplify the support code in patch #10 of the series slightly
.../devicetree/bindings/arm/omap/prm-inst.txt | 30 +++++++++++++++++++
1 file changed, 30 insertions(+)
create mode 100644 Documentation/devicetree/bindings/arm/omap/prm-inst.txt
diff --git a/Documentation/devicetree/bindings/arm/omap/prm-inst.txt b/Documentation/devicetree/bindings/arm/omap/prm-inst.txt
new file mode 100644
index 000000000000..31f8d8b44a51
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/omap/prm-inst.txt
@@ -0,0 +1,30 @@
+OMAP PRM instance bindings
+
+Power and Reset Manager is an IP block on OMAP family of devices which
+handle the power domains and their current state, and provide reset
+handling for the domains and/or separate IP blocks under the power domain
+hierarchy.
+
+Required properties:
+- compatible: Must contain one of the following:
+ "ti,am3-prm-inst"
+ "ti,am4-prm-inst"
+ "ti,omap4-prm-inst"
+ "ti,omap5-prm-inst"
+ "ti,dra7-prm-inst"
+ and additionally must contain:
+ "ti,omap-prm-inst"
+- reg: Contains PRM instance register address range
+ (base address and length)
+
+Optional properties:
+- #reset-cells: Should be 1 if the PRM instance in question supports resets.
+
+Example:
+
+prm_dsp2: prm@1b00 {
+ compatible = "ti,omap-prm-inst", "ti,dra7-prm-inst";
+ reg = <0x1b00 0x40>;
+ #reset-cells = <1>;
+ clocks = <&dsp2_clkctrl DRA7_DSP2_MMU0_DSP2_CLKCTRL 0>;
+};
--
2.17.1
--
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 related
* Re: [PATCH] arm64: dts: mt8183: Add node for the Mali GPU
From: Alyssa Rosenzweig @ 2019-09-19 12:32 UTC (permalink / raw)
To: Nicolas Boichat
Cc: Mark Rutland, devicetree, Dominik Behr,
linux-kernel@vger.kernel.org, Douglas Anderson, Rob Herring,
moderated list:ARM/Mediatek SoC support, Boris Brezillon,
Matthias Brugger, Steven Price, Nick Fan,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <CANMq1KD++==d0Mb6T2gKU1T7c_MaedswOYdxqEqEKKUL1bxgiw@mail.gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 1371 bytes --]
> > By the time MT8183 shows up in more concrete devices, it will, certainly
> > in kernel-space and likely in userspace as well. At present, the DDK can
> > be modified to run on top of the in-tree Mali drivers, i.e. "Bifrost on
> > mainline linux-next (+ page table/compatible patches), with blob
> > userspace".
> >
> > While the open userspace isn't ready here quite yet, I would definitely
> > encourage upstream kernel for ChromeOS, since then there's no need to
> > maintain the out-of-tree GPU driver.
>
> That's an interesting idea, I had no idea, thanks for the info!
>
> Would that work with midgard as well? We have released hardware with
> RK3288/3399, so it might be nice to experiment with these first.
Yes, the above would work with Midgard as well with no changes needed.
Ping Steven about thtat (CC'd).
> > More immediately, per Rob's review, it's important that the bindings
> > accepted upstream work with the in-tree Bifrost driver. Conceptually,
> > once Mesa supports Bifrost, if I install Debian on a MT8183 board,
> > everything should just work. I shouldn't need MT-specific changes / need
> > to change names for the DT. Regardless of which kernel driver you end up
> > using, minimally sharing the DT is good for everyone :-)
>
> Yes. I'll try to dig further with MTK, but this may take some time.
Thank you!
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCHv6 10/10] ARM: OMAP2+: pdata-quirks: add PRM data for reset support
From: Tero Kristo @ 2019-09-19 12:32 UTC (permalink / raw)
To: ssantosh, p.zabel, tony; +Cc: devicetree, linux-omap, robh+dt, linux-arm-kernel
In-Reply-To: <20190912170953.GT52127@atomide.com>
The parent clockdomain for reset must be in force wakeup mode, otherwise
the reset may never complete. Add pdata quirks for this purpose for PRM
driver.
Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
v6: replaced platform specific compatibles with a single common
compatible check
arch/arm/mach-omap2/pdata-quirks.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/arm/mach-omap2/pdata-quirks.c b/arch/arm/mach-omap2/pdata-quirks.c
index 6c6f8fce854e..45ffa1204cd2 100644
--- a/arch/arm/mach-omap2/pdata-quirks.c
+++ b/arch/arm/mach-omap2/pdata-quirks.c
@@ -25,6 +25,7 @@
#include <linux/platform_data/ti-sysc.h>
#include <linux/platform_data/wkup_m3.h>
#include <linux/platform_data/asoc-ti-mcbsp.h>
+#include <linux/platform_data/ti-prm.h>
#include "clockdomain.h"
#include "common.h"
@@ -565,6 +566,12 @@ void omap_pcs_legacy_init(int irq, void (*rearm)(void))
pcs_pdata.rearm = rearm;
}
+static struct ti_prm_platform_data ti_prm_pdata = {
+ .clkdm_deny_idle = clkdm_deny_idle,
+ .clkdm_allow_idle = clkdm_allow_idle,
+ .clkdm_lookup = clkdm_lookup,
+};
+
/*
* GPIOs for TWL are initialized by the I2C bus and need custom
* handing until DSS has device tree bindings.
@@ -664,6 +671,7 @@ static struct of_dev_auxdata omap_auxdata_lookup[] = {
/* Common auxdata */
OF_DEV_AUXDATA("ti,sysc", 0, NULL, &ti_sysc_pdata),
OF_DEV_AUXDATA("pinctrl-single", 0, NULL, &pcs_pdata),
+ OF_DEV_AUXDATA("ti,omap-prm-inst", 0, NULL, &ti_prm_pdata),
{ /* sentinel */ },
};
--
2.17.1
--
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 related
* [PATCH] [v2] net: stmmac: selftest: avoid large stack usage
From: Arnd Bergmann @ 2019-09-19 12:33 UTC (permalink / raw)
To: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, David S. Miller,
Maxime Coquelin
Cc: linux-arm-kernel, netdev, linux-stm32, Arnd Bergmann,
linux-kernel
Putting a struct stmmac_rss object on the stack is a bad idea,
as it exceeds the warning limit for a stack frame on 32-bit architectures:
drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c:1221:12: error: stack frame size of 1208 bytes in function '__stmmac_test_l3filt' [-Werror,-Wframe-larger-than=]
drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c:1338:12: error: stack frame size of 1208 bytes in function '__stmmac_test_l4filt' [-Werror,-Wframe-larger-than=]
As the object is the trivial empty case, change the called function
to accept a NULL pointer to mean the same thing and remove the
large variable in the two callers.
Fixes: 4647e021193d ("net: stmmac: selftests: Add selftest for L3/L4 Filters")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
v2: simply configure function, based on feedback from Jose
---
.../net/ethernet/stmicro/stmmac/dwxgmac2_core.c | 5 ++---
.../net/ethernet/stmicro/stmmac/stmmac_selftests.c | 14 ++++----------
2 files changed, 6 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
index d5173dd02a71..2b277b2c586b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
@@ -523,19 +523,18 @@ static int dwxgmac2_rss_configure(struct mac_device_info *hw,
struct stmmac_rss *cfg, u32 num_rxq)
{
void __iomem *ioaddr = hw->pcsr;
- u32 *key = (u32 *)cfg->key;
int i, ret;
u32 value;
value = readl(ioaddr + XGMAC_RSS_CTRL);
- if (!cfg->enable) {
+ if (!cfg || !cfg->enable) {
value &= ~XGMAC_RSSE;
writel(value, ioaddr + XGMAC_RSS_CTRL);
return 0;
}
for (i = 0; i < (sizeof(cfg->key) / sizeof(u32)); i++) {
- ret = dwxgmac2_rss_write_reg(ioaddr, true, i, *key++);
+ ret = dwxgmac2_rss_write_reg(ioaddr, true, i, cfg->key[i]);
if (ret)
return ret;
}
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
index c56e89e1ae56..9c8d210b2d6a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
@@ -1233,12 +1233,9 @@ static int __stmmac_test_l3filt(struct stmmac_priv *priv, u32 dst, u32 src,
return -EOPNOTSUPP;
if (!priv->dma_cap.l3l4fnum)
return -EOPNOTSUPP;
- if (priv->rss.enable) {
- struct stmmac_rss rss = { .enable = false, };
-
- stmmac_rss_configure(priv, priv->hw, &rss,
+ if (priv->rss.enable)
+ stmmac_rss_configure(priv, priv->hw, NULL,
priv->plat->rx_queues_to_use);
- }
dissector = kzalloc(sizeof(*dissector), GFP_KERNEL);
if (!dissector) {
@@ -1357,12 +1354,9 @@ static int __stmmac_test_l4filt(struct stmmac_priv *priv, u32 dst, u32 src,
return -EOPNOTSUPP;
if (!priv->dma_cap.l3l4fnum)
return -EOPNOTSUPP;
- if (priv->rss.enable) {
- struct stmmac_rss rss = { .enable = false, };
-
- stmmac_rss_configure(priv, priv->hw, &rss,
+ if (priv->rss.enable)
+ stmmac_rss_configure(priv, priv->hw, NULL,
priv->plat->rx_queues_to_use);
- }
dissector = kzalloc(sizeof(*dissector), GFP_KERNEL);
if (!dissector) {
--
2.20.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] stmmac: selftest: avoid large stack usage
From: Arnd Bergmann @ 2019-09-19 12:34 UTC (permalink / raw)
To: Jose Abreu
Cc: Alexandre Torgue, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, David S. Miller, Maxime Coquelin,
Giuseppe Cavallaro, linux-stm32@st-md-mailman.stormreply.com,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <BN8PR12MB3266E044DDF00F227B9B191CD3890@BN8PR12MB3266.namprd12.prod.outlook.com>
On Thu, Sep 19, 2019 at 9:58 AM Jose Abreu <Jose.Abreu@synopsys.com> wrote:
>
> From: Arnd Bergmann <arnd@arndb.de>
> Date: Sep/18/2019, 20:54:34 (UTC+00:00)
>
> > + if (!cfg || !cfg->enable) {
> > value &= ~XGMAC_RSSE;
> > writel(value, ioaddr + XGMAC_RSS_CTRL);
> > return 0;
> > }
> >
> > for (i = 0; i < (sizeof(cfg->key) / sizeof(u32)); i++) {
> > - ret = dwxgmac2_rss_write_reg(ioaddr, true, i, *key++);
> > + if (cfg)
> > + ret = dwxgmac2_rss_write_reg(ioaddr, true, i, cfg->key[i]);
> > + else
> > + ret = dwxgmac2_rss_write_reg(ioaddr, true, i, 0);
> > +
> > if (ret)
> > return ret;
> > }
> >
> > for (i = 0; i < ARRAY_SIZE(cfg->table); i++) {
> > - ret = dwxgmac2_rss_write_reg(ioaddr, false, i, cfg->table[i]);
> > + if (cfg)
> > + ret = dwxgmac2_rss_write_reg(ioaddr, false, i, cfg->table[i]);
> > + else
> > + ret = dwxgmac2_rss_write_reg(ioaddr, false, i, 0);
> > +
>
> I don't get these "if (cfg)" checks. You already check earlier if cfg is
> NULL and return if so. I don't think you need these extra checks.
Ah, you are right, I missed the 'return 0', that makes it much easier.
> Also, your subject line should be something like: "net: stmmac:
> selftests: ..."
I think both styles is common for network drivers, though I think most
just leave out the 'net:'. I changed it in v2 now.
Arnd
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [RFC PATCH V1] riscv-privileged: Add broadcast mode to sfence.vma
From: guoren @ 2019-09-19 12:35 UTC (permalink / raw)
To: benh
Cc: julien.thierry, catalin.marinas, palmer, will.deacon, Atish.Patra,
julien.grall, gary, linux-riscv, kvmarm, jean-philippe,
linux-csky, rppt, Guo Ren, jacob.jun.pan, tech-privileged,
marc.zyngier, linux-arm-kernel, feiteng_li, Anup.Patel,
linux-kernel, iommu, dwmw2
From: Guo Ren <ren_guo@c-sky.com>
The patch is for https://github.com/riscv/riscv-isa-manual
The proposal has been talked in LPC-2019 RISC-V MC ref [1]. Here is the
formal patch.
Introduction
============
Using the Hardware TLB broadcast invalidation instruction to maintain the
system TLB is a good choice and it'll simplify the system software design.
The proposal hopes to add a broadcast mode to the sfence.vma in the
riscv-privilege specification. To support the sfence.vma broadcast mode,
there are two modification introduced below:
1) Add PGD.PPN (root page table's PPN) as the unique identifier of the
address space in addition to asid/vmid. Compared to the dynamically
changed asid/vmid, PGD.PPN is fixed throughout the address space life
cycle. This feature enables uniform address space identification
between different TLB systems (actually, it's difficult to unify the
asid/vmid between the CPU system and the IOMMU system, because their
mechanisms are different)
2) Modify the definition of the sfence.vma instruction from synchronous
mode to asynchronous mode, which means that the completion of the TLB
operation is not guaranteed when the sfence.vma instruction retires.
It needs to be completed by checking the flag bit on the hart. The
sfence.vma request finish can notify the software by generating an
interrupt. This function alleviates the large delay of TLB invalidation
in the PCI ATS system.
Add S1/S2.PGD.PPN for ASID/VMID
===============================
PGD is global directory (defined in linux) and PPN is page physical number
(defined in riscv-spec). PGD.PNN corresponds to the root page table pointer
of the address space, i.e. mm->pgd (linux concept).
In CPU/IOMMU TLB, we use asid/vmid to distinguish the address space of
process or virtual machine. Due to the limitation of id encoding, it can
only represent a part(window) of the address space. S1/S2.PGD.PPN are the
root page table's PPNs of the address spaces and S1/S2.PGD.PPN are the
unique identifier of the address spaces.
For the CPU SMP system, you can use context switch to perform the necessary
software mechanism to ensure that the asid/vmid on all harts is consistent
(please refer to the arm64 asid mechanism). In this way, the TLB broadcast
invalidation instruction can determine the address space processed on all
harts by asid/vmid.
Different from the CPU SMP system, there is no context switch for the
DMA-IOMMU system, so the unification with the CPU asid/vmid cannot be
guaranteed. So we need a unique identifier for the address space to
establish a communication bridge between the TLBs of different systems.
That is PGD.PPN (for virtualization scenarios: S1/S2.PGD.PPN)
current:
sfence.vma rs1 = vaddr, rs2 = asid
hfence.vvma rs1 = vaddr, rs2 = asid
hfence.gvma rs1 = gaddr, rs2 = vmid
proposed:
sfence.vma rs1 = vaddr, rs2 = mode:ppn:asid
hfence.vvma rs1 = vaddr, rs2 = mode:ppn:asid
hfence.gvma rs1 = gaddr, rs2 = mode:ppn:vmid
mode - broadcast | local
ppn - the PPN of the address space of the root page table
vmid/asid - the window identifier of the address space
At the Linux Plumber Conference 2019 RISCV-MC, ref:[1], we've showed two
IOMMU examples to explain how it work with hardware.
1) In a lightweight IOMMU system (up to 64 address spaces), the hardware
could directly convert PGD.PPN into DID (IOMMU ASID)
2) For the PCI ATS scenario, its IO ASID/VMID encoding space can support
a very large number of address spaces. We use two reverse mapping
tables to let the hardware translate S1/S2.PGD.PPN into IO ASID/VMID.
ASYNC BROADCAST SFENCE.VMA
===========================
To support the high latency broadcast sfence.vma operation in the PCI ATS
usage scenario, we modify the sfence.vma from synchronous mode to
asynchronous mode. (For simpler implementation, if hardware only implement
synchronous mode and software still work in asynchronous mode)
To implement the asynchronous mode, 3 features are added:
1) sstatus:TLBI
A "status bit - TLBI" is added to the sstatus register. The TLBI status
bit indicates if there are still outstanding sfence.vma requests on the
current hart.
Value:
1: sfence.vma requests are not completed.
0: all sfece.vma requests completed, request queue is empty.
2) sstatus:TLBIC
A "control bits - TLBIC" is added to sstatus register. The TLBIC control
bits are controlled by software.
"Write 1" will trigger the current hart check to see if there are still
outstanding sfence.vma requests. If there are unfinished requests, an
interrupt will be generated when the request is completed, notifying the
software that all of the current sfence.vma requests have been completed.
"Write 0" will cause nothing.
3) supervisor interrupt register (sip & sie):TLBI finish interrupt
A per-hart interrupt is added to supervisor interrupt registers.
When all sfence.vma requests are completed and sstatus:TLBIC has been
triggered, hart will receive a TLBI finish interrupt. Just like timer,
software and external interrupt's definition in sip & sie.
Fake code:
flush_tlb_page(vma, addr) {
asid = cpu_asid(vma->vm_mm);
ppn = PFN_DOWN(vma->vm_mm->pgd);
sfence.vma (addr, 1|PPN_OFFSET(ppn)|asid); //1. start request
while(sstatus:TLBI) if (time_out() > 1ms) break; //2. loop check
while (sstatus:TLBI) {
...
set sstatus:TLBIC;
wait_TLBI_finish_interrupt(); //3. wait irq, io_schedule
}
}
Here we give 2 level check:
1) loop check sstatus:TLBI, CPU could response Interrupt.
2) set sstatus:TLBIC and wait for irq, CPU schedule out for other task.
ACE-DVM Example
===============
Honestly, "broadcasting addr, asid, vmid, S1/S2.PGD.PPN to interconnects"
and "ASYNC SFENCE.VMA" could be implemented by ACE-DVM protocol ref [2].
There are 3 types of transactions in DVM:
- DVM operation
Send all information to the interconnect, including addr, asid,
S1.PGD.PPN, vmid, S2.PGD.PPN.
- DVM synchronization
Check that all DVM operations have been completed. If not, it will use
state machine to wait DVM complete requests.
- DVM complete
Return transaction from components, eg: IOMMU. If hart has received all
DVM completes which are triggered by sfence.vma instructions and
"sstatus:TLBIC" has been set, a TLBI finish interrupt is triggered.
(Actually, we do not need to implement the above functions strictly
according to the ACE specification :P )
1: https://www.linuxplumbersconf.org/event/4/contributions/307/
2: AMBA AXI and ACE Protocol Specification - Distributed Virtual Memory
Transactions"
Signed-off-by: Guo Ren <ren_guo@c-sky.com>
Reviewed-by: Li Feiteng <feiteng_li@c-sky.com>
---
src/hypervisor.tex | 43 ++++++++-------
src/supervisor.tex | 155 +++++++++++++++++++++++++++++++++++++++++------------
2 files changed, 143 insertions(+), 55 deletions(-)
diff --git a/src/hypervisor.tex b/src/hypervisor.tex
index 47b90b2..3718819 100644
--- a/src/hypervisor.tex
+++ b/src/hypervisor.tex
@@ -1094,15 +1094,15 @@ The hypervisor extension adds two new privileged fence instructions.
\multicolumn{1}{c|}{opcode} \\
\hline
7 & 5 & 5 & 3 & 5 & 7 \\
-HFENCE.GVMA & vmid & gaddr & PRIV & 0 & SYSTEM \\
-HFENCE.VVMA & asid & vaddr & PRIV & 0 & SYSTEM \\
+HFENCE.GVMA & mode:ppn:vmid & gaddr & PRIV & 0 & SYSTEM \\
+HFENCE.VVMA & mode:ppn:asid & vaddr & PRIV & 0 & SYSTEM \\
\end{tabular}
\end{center}
The hypervisor memory-management fence instructions, HFENCE.GVMA and
HFENCE.VVMA, are valid only in HS-mode when {\tt mstatus}.TVM=0, or in M-mode
(irrespective of {\tt mstatus}.TVM).
-These instructions perform a function similar to SFENCE.VMA
+These instructions perform a function similar to SFENCE.VMA (broadcast/local)
(Section~\ref{sec:sfence.vma}), except applying to the guest-physical
memory-management data structures controlled by CSR {\tt hgatp} (HFENCE.GVMA)
or the VS-level memory-management data structures controlled by CSR {\tt vsatp}
@@ -1136,11 +1136,10 @@ An HFENCE.VVMA instruction applies only to a single virtual machine, identified
by the setting of {\tt hgatp}.VMID when HFENCE.VVMA executes.
\end{commentary}
-When {\em rs2}$\neq${\tt x0}, bits XLEN-1:ASIDMAX of the value held in {\em
-rs2} are reserved for future use and should be zeroed by software and ignored
-by current implementations.
-Furthermore, if ASIDLEN~$<$~ASIDMAX, the implementation shall ignore bits
-ASIDMAX-1:ASIDLEN of the value held in {\em rs2}.
+When {\em rs2}$\neq${\tt x0}, bits contain 3 informations: mode, ppn, asid.
+1) mode control HFENCE.VVMA broadcast or not.
+2) ppn is the root page talbe's PPN of the asid address space.
+3) asid is the identifier of process in virtual machine.
\begin{commentary}
Simpler implementations of HFENCE.VVMA can ignore the guest virtual address in
@@ -1168,11 +1167,10 @@ physical addresses in PMP address registers (Section~\ref{sec:pmp}) and in page
table entries (Sections \ref{sec:sv32}, \ref{sec:sv39}, and~\ref{sec:sv48}).
\end{commentary}
-When {\em rs2}$\neq${\tt x0}, bits XLEN-1:VMIDMAX of the value held in {\em
-rs2} are reserved for future use and should be zeroed by software and ignored
-by current implementations.
-Furthermore, if VMIDLEN~$<$~VMIDMAX, the implementation shall ignore bits
-VMIDMAX-1:VMIDLEN of the value held in {\em rs2}.
+When {\em rs2}$\neq${\tt x0}, bits contain 3 informations: mode, vmid, ppn.
+1) mode control HFENCE.GVMA broadcast or not.
+2) ppn is the root page talbe's PPN of the vmid address space.
+3) vmid is the identifier of virtual machine.
\begin{commentary}
Simpler implementations of HFENCE.GVMA can ignore the guest physical address in
@@ -1567,21 +1565,22 @@ register.
\subsection{Memory-Management Fences}
The behavior of the SFENCE.VMA instruction is affected by the current
-virtualization mode V. When V=0, the virtual-address argument is an HS-level
-virtual address, and the ASID argument is an HS-level ASID.
+virtualization mode V. When V=0, the rs1 argument is an HS-level
+virtual address, and the rs2 argument is an HS-level ASID and root page table's PPN.
The instruction orders stores only to HS-level address-translation structures
with subsequent HS-level address translations.
-When V=1, the virtual-address argument to SFENCE.VMA is a guest virtual
-address within the current virtual machine, and the ASID argument is a VS-level
-ASID within the current virtual machine.
+When V=1, the rs1 argument to SFENCE.VMA is a guest virtual
+address within the current virtual machine, and the rs2 argument is a VS-level
+ASID and root page table's PPN within the current virtual machine.
The current virtual machine is identified by the VMID field of CSR {\tt hgatp},
-and the effective ASID can be considered to be the combination of this VMID
-with the VS-level ASID.
+and the effective ASID and root page table's PPN can be considered to be the
+combination of this VMID and root page table's PPN with the VS-level ASID and
+root page table's PPN.
The SFENCE.VMA instruction orders stores only to the VS-level
address-translation structures with subsequent VS-level address translations
-for the same virtual machine, i.e., only when {\tt hgatp}.VMID is the same as
-when the SFENCE.VMA executed.
+for the same virtual machine, i.e., only when {\tt hgatp}.VMID and {\\tt hgatp}.PPN is
+the same as when the SFENCE.VMA executed.
Hypervisor instructions HFENCE.GVMA and HFENCE.VVMA provide additional
memory-management fences to complement SFENCE.VMA.
diff --git a/src/supervisor.tex b/src/supervisor.tex
index ba3ced5..2877b7a 100644
--- a/src/supervisor.tex
+++ b/src/supervisor.tex
@@ -47,10 +47,12 @@ register keeps track of the processor's current operating state.
\begin{center}
\setlength{\tabcolsep}{4pt}
\scalebox{0.95}{
-\begin{tabular}{cWcccccWccccWcc}
+\begin{tabular}{cccWcccccWccccWcc}
\\
\instbit{31} &
-\instbitrange{30}{20} &
+\instbit{30} &
+\instbit{29} &
+\instbitrange{28}{20} &
\instbit{19} &
\instbit{18} &
\instbit{17} &
@@ -66,6 +68,8 @@ register keeps track of the processor's current operating state.
\instbit{0} \\
\hline
\multicolumn{1}{|c|}{SD} &
+\multicolumn{1}{|c|}{TLBI} &
+\multicolumn{1}{|c|}{TLBIC} &
\multicolumn{1}{c|}{\wpri} &
\multicolumn{1}{c|}{MXR} &
\multicolumn{1}{c|}{SUM} &
@@ -82,7 +86,7 @@ register keeps track of the processor's current operating state.
\multicolumn{1}{c|}{\wpri}
\\
\hline
-1 & 11 & 1 & 1 & 1 & 2 & 2 & 4 & 1 & 1 & 1 & 1 & 3 & 1 & 1 \\
+1 & 1 & 1 & 10 & 1 & 1 & 1 & 2 & 2 & 4 & 1 & 1 & 1 & 1 & 3 & 1 & 1 \\
\end{tabular}}
\end{center}
}
@@ -95,10 +99,12 @@ register keeps track of the processor's current operating state.
{\footnotesize
\begin{center}
\setlength{\tabcolsep}{4pt}
-\begin{tabular}{cMFScccc}
+\begin{tabular}{cccMFScccc}
\\
\instbit{SXLEN-1} &
-\instbitrange{SXLEN-2}{34} &
+\instbit{SXLEN-2} &
+\instbit{SXLEN-3} &
+\instbitrange{SXLEN-4}{34} &
\instbitrange{33}{32} &
\instbitrange{31}{20} &
\instbit{19} &
@@ -107,6 +113,8 @@ register keeps track of the processor's current operating state.
\\
\hline
\multicolumn{1}{|c|}{SD} &
+\multicolumn{1}{|c|}{TLBI} &
+\multicolumn{1}{|c|}{TLBIC} &
\multicolumn{1}{c|}{\wpri} &
\multicolumn{1}{c|}{UXL[1:0]} &
\multicolumn{1}{c|}{\wpri} &
@@ -115,7 +123,7 @@ register keeps track of the processor's current operating state.
\multicolumn{1}{c|}{\wpri} &
\\
\hline
-1 & SXLEN-35 & 2 & 12 & 1 & 1 & 1 & \\
+1 & 1 & 1 & SXLEN-37 & 2 & 12 & 1 & 1 & 1 & \\
\end{tabular}
\begin{tabular}{cWWFccccWcc}
\\
@@ -152,6 +160,17 @@ register keeps track of the processor's current operating state.
\label{sstatusreg}
\end{figure*}
+The TLBI (read-only) bit indicates that any async sfence.vma operations are
+still pended on the hart. The value:0 means that there is no sfence.vma
+operations pending and value:1 means that there are still sfence.vma operations
+pending on the hart.
+
+When the sstatus:TLBIC bit is written 1, it triggers the hardware to check if
+there are any TLB invalidate operations being pended. When all operations are
+finished, a TLB Invalidate finish interrupt will be triggered
+(see Section~\ref{sipreg}). When the sstatus:TLBIC bit is written 0, it will
+cause nothing. Reading sstatus:TLBIC bit will alaways return 0.
+
The SPP bit indicates the privilege level at which a hart was executing before
entering supervisor mode. When a trap is taken, SPP is set to 0 if the trap
originated from user mode, or 1 otherwise. When an SRET instruction
@@ -329,8 +348,10 @@ SXLEN-bit read/write register containing interrupt enable bits.
{\footnotesize
\begin{center}
\setlength{\tabcolsep}{4pt}
-\begin{tabular}{KcFcFcc}
-\instbitrange{SXLEN-1}{10} &
+\begin{tabular}{KcFcFcFcc}
+\instbitrange{SXLEN-1}{14} &
+\instbit{13} &
+\instbitrange{12}{10} &
\instbit{9} &
\instbitrange{8}{6} &
\instbit{5} &
@@ -339,6 +360,8 @@ SXLEN-bit read/write register containing interrupt enable bits.
\instbit{0} \\
\hline
\multicolumn{1}{|c|}{\wpri} &
+\multicolumn{1}{c|}{STLBIP} &
+\multicolumn{1}{|c|}{\wpri} &
\multicolumn{1}{c|}{SEIP} &
\multicolumn{1}{c|}{\wpri} &
\multicolumn{1}{c|}{STIP} &
@@ -346,7 +369,7 @@ SXLEN-bit read/write register containing interrupt enable bits.
\multicolumn{1}{c|}{SSIP} &
\multicolumn{1}{c|}{\wpri} \\
\hline
-SXLEN-10 & 1 & 3 & 1 & 3 & 1 & 1 \\
+SXLEN-14 & 1 & 3 & 1 & 3 & 1 & 3 & 1 & 1 \\
\end{tabular}
\end{center}
}
@@ -359,8 +382,10 @@ SXLEN-10 & 1 & 3 & 1 & 3 & 1 & 1 \\
{\footnotesize
\begin{center}
\setlength{\tabcolsep}{4pt}
-\begin{tabular}{KcFcFcc}
-\instbitrange{SXLEN-1}{10} &
+\begin{tabular}{KcFcFcFcc}
+\instbitrange{SXLEN-1}{14} &
+\instbit{13} &
+\instbitrange{12}{10} &
\instbit{9} &
\instbitrange{8}{6} &
\instbit{5} &
@@ -369,6 +394,8 @@ SXLEN-10 & 1 & 3 & 1 & 3 & 1 & 1 \\
\instbit{0} \\
\hline
\multicolumn{1}{|c|}{\wpri} &
+\multicolumn{1}{c|}{STLBIE} &
+\multicolumn{1}{|c|}{\wpri} &
\multicolumn{1}{c|}{SEIE} &
\multicolumn{1}{c|}{\wpri} &
\multicolumn{1}{c|}{STIE} &
@@ -376,7 +403,7 @@ SXLEN-10 & 1 & 3 & 1 & 3 & 1 & 1 \\
\multicolumn{1}{c|}{SSIE} &
\multicolumn{1}{c|}{\wpri} \\
\hline
-SXLEN-10 & 1 & 3 & 1 & 3 & 1 & 1 \\
+SXLEN-14 & 1 & 3 & 1 & 3 & 1 & 3 & 1 & 1 \\
\end{tabular}
\end{center}
}
@@ -410,6 +437,12 @@ when the SEIE bit in the {\tt sie} register is clear. The implementation
should provide facilities to mask, unmask, and query the cause of external
interrupts.
+A supervisor-level TLB Invalidate finish interrupt is pending if the STLBIP bit
+in the {\tt sip} register is set. Supervisor-level TLB Invalidate finish
+interrupts are disabled when the STLBIE bit in the {\tt sie} register is clear.
+When hart tlb invalidate operations are finished, hardware will change sstatus:TLBI
+bit from 1 to 0 and trigger TLB Invalidate finish interrupt.
+
\begin{commentary}
The {\tt sip} and {\tt sie} registers are subsets of the {\tt mip} and {\tt
mie} registers. Reading any field, or writing any writable field, of {\tt
@@ -598,7 +631,9 @@ so is only guaranteed to hold supported exception codes.
1 & 5 & Supervisor timer interrupt \\
1 & 6--8 & {\em Reserved} \\
1 & 9 & Supervisor external interrupt \\
- 1 & 10--15 & {\em Reserved} \\
+ 1 & 10--11 & {\em Reserved} \\
+ 1 & 12 & Supervisor TLBI finish interrupt \\
+ 1 & 13--15 & {\em Reserved} \\
1 & $\ge$16 & {\em Available for platform use} \\ \hline
0 & 0 & Instruction address misaligned \\
0 & 1 & Instruction access fault \\
@@ -884,7 +919,7 @@ provided.
\multicolumn{1}{c|}{opcode} \\
\hline
7 & 5 & 5 & 3 & 5 & 7 \\
-SFENCE.VMA & asid & vaddr & PRIV & 0 & SYSTEM \\
+SFENCE.VMA & mode:ppn:asid & vaddr & LOCAL & 0 & SYSTEM \\
\end{tabular}
\end{center}
@@ -899,21 +934,70 @@ from that hart to the memory-management data structures.
Further details on the behavior of this instruction are
described in Section~\ref{virt-control} and Section~\ref{pmp-vmem}.
+SFENCE.VMA is defined as an asynchronous completion instruction, which means
+that the TLB operation is not guaranteed to complete when the instruction retires.
+Software need check sstatus:TLBI to determine all TLB operations complete.
+The sstatus:TLBI described in Section~\ref{sstatus}. When hardware change
+sstatus:TLBI bit from 1 to 0, the TLB Invalidate finish interrupt will be
+triggered.
+
\begin{commentary}
-The SFENCE.VMA is used to flush any local hardware caches related to
+The SFENCE.VMA is used to flush any local/remote hardware caches related to
address translation. It is specified as a fence rather than a TLB
flush to provide cleaner semantics with respect to which instructions
are affected by the flush operation and to support a wider variety of
dynamic caching structures and memory-management schemes. SFENCE.VMA
is also used by higher privilege levels to synchronize page table
-writes and the address translation hardware.
+writes and the address translation hardware. There is a mode bit to determine
+sfence.vma would broadcast on interconnect or not.
\end{commentary}
-SFENCE.VMA orders only the local hart's implicit references to the
-memory-management data structures.
+\begin{figure}[h!]
+{\footnotesize
+\begin{center}
+\begin{tabular}{c@{}E@{}K}
+\instbit{31} &
+\instbitrange{30}{9} &
+\instbitrange{8}{0} \\
+\hline
+\multicolumn{1}{|c|}{{\tt MODE}} &
+\multicolumn{1}{|c|}{{\tt PPN (root page table)}} &
+\multicolumn{1}{|c|}{{\tt ASID}} \\
+\hline
+1 & 22 & 9 \\
+\end{tabular}
+\end{center}
+}
+\vspace{-0.1in}
+\caption{RV32 sfence.vma rs2 format.}
+\label{rv32satp}
+\end{figure}
+
+\begin{figure}[h!]
+{\footnotesize
+\begin{center}
+\begin{tabular}{@{}S@{}T@{}U}
+\instbitrange{63}{60} &
+\instbitrange{59}{16} &
+\instbitrange{15}{0} \\
+\hline
+\multicolumn{1}{|c|}{{\tt MODE}} &
+\multicolumn{1}{|c|}{{\tt PPN (root page table)}} &
+\multicolumn{1}{|c|}{{\tt ASID}} \\
+\hline
+4 & 44 & 16 \\
+\end{tabular}
+\end{center}
+}
+\vspace{-0.1in}
+\caption{RV64 sfence.vma rs2 format, for MODE values, only highest bit:63 is
+valid and others are reserved.}
+\label{rv64satp}
+\end{figure}
\begin{commentary}
-Consequently, other harts must be notified separately when the
+The mode's highest bit could control sfence.vma behavior with 1:broadcast or 0:local.
+If only have mode:local, other harts must be notified separately when the
memory-management data structures have been modified.
One approach is to use 1)
a local data fence to ensure local writes are visible globally, then
@@ -928,8 +1012,17 @@ modified for a single address mapping (i.e., one page or superpage), {\em rs1}
can specify a virtual address within that mapping to effect a translation
fence for that mapping only. Furthermore, for the common case that the
translation data structures have only been modified for a single address-space
-identifier, {\em rs2} can specify the address space. The behavior of
-SFENCE.VMA depends on {\em rs1} and {\em rs2} as follows:
+identifier, {\em rs2} can specify the address space with {\tt satp} format
+which include asid and root page table's PPN information.
+
+\begin{commentary}
+We use ASID and root page table's PPN to determine address space and the format
+stored in rs2 is similar with {\tt satp} described in Section~\ref{sec:satp}.
+ASID are used by local harts and root page table's PPN of the asid are used by
+other different TLB systems, eg: IOMMU.
+\end{commentary}
+
+The behavior of SFENCE.VMA depends on {\em rs1} and {\em rs2} as follows:
\begin{itemize}
\item If {\em rs1}={\tt x0} and {\em rs2}={\tt x0}, the fence orders all
@@ -939,23 +1032,18 @@ SFENCE.VMA depends on {\em rs1} and {\em rs2} as follows:
all reads and writes made to any level of the page tables, but only
for the address space identified by integer register {\em rs2}.
Accesses to {\em global} mappings (see Section~\ref{sec:translation})
- are not ordered.
+ are not ordered. The mode field in rs2 is determine broadcast or local.
\item If {\em rs1}$\neq${\tt x0} and {\em rs2}={\tt x0}, the fence orders
only reads and writes made to the leaf page table entry corresponding
to the virtual address in {\em rs1}, for all address spaces.
\item If {\em rs1}$\neq${\tt x0} and {\em rs2}$\neq${\tt x0}, the fence
orders only reads and writes made to the leaf page table entry
corresponding to the virtual address in {\em rs1}, for the address
- space identified by integer register {\em rs2}.
+ space identified by integer register {\em rs2}. The mode field in rs2
+ is determine broadcast or local.
Accesses to global mappings are not ordered.
\end{itemize}
-When {\em rs2}$\neq${\tt x0}, bits SXLEN-1:ASIDMAX of the value held in {\em
-rs2} are reserved for future use and should be zeroed by software and ignored
-by current implementations. Furthermore, if ASIDLEN~$<$~ASIDMAX, the
-implementation shall ignore bits ASIDMAX-1:ASIDLEN of the value held in {\em
-rs2}.
-
\begin{commentary}
Simpler implementations can ignore the virtual address in {\em rs1} and
the ASID value in {\em rs2} and always perform a global fence.
@@ -994,7 +1082,7 @@ can execute the same SFENCE.VMA instruction while a different ASID is loaded
into {\tt satp}, provided the next time {\tt satp} is loaded with the recycled
ASID, it is simultaneously loaded with the new page table.
-\item If the implementation does not provide ASIDs, or software chooses to
+\item If the implementation does not provide ASIDs and PPNs, or software chooses to
always use ASID 0, then after every {\tt satp} write, software should execute
SFENCE.VMA with {\em rs1}={\tt x0}. In the common case that no global
translations have been modified, {\em rs2} should be set to a register other than
@@ -1003,13 +1091,14 @@ not flushed.
\item If software modifies a non-leaf PTE, it should execute SFENCE.VMA with
{\em rs1}={\tt x0}. If any PTE along the traversal path had its G bit set,
-{\em rs2} must be {\tt x0}; otherwise, {\em rs2} should be set to the ASID for
-which the translation is being modified.
+{\em rs2} must be {\tt x0}; otherwise, {\em rs2} should be set to the ASID and
+root page table's PPN for which the translation is being modified.
\item If software modifies a leaf PTE, it should execute SFENCE.VMA with {\em
rs1} set to a virtual address within the page. If any PTE along the traversal
path had its G bit set, {\em rs2} must be {\tt x0}; otherwise, {\em rs2}
-should be set to the ASID for which the translation is being modified.
+should be set to the ASID and root page table's PPN for which the translation
+is being modified.
\item For the special cases of increasing the permissions on a leaf PTE and
changing an invalid PTE to a valid leaf, software may choose to execute
--
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
* Re: [PATCH v1 2/9] mfd: wm8994: Add support for MCLKn clock control
From: Sylwester Nawrocki @ 2019-09-19 12:48 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: devicetree, alsa-devel, linux-samsung-soc, ckeepax, b.zolnierkie,
sbkim73, patches, broonie, lgirdwood, robh+dt, linux-arm-kernel,
m.szyprowski
In-Reply-To: <20190919075924.GB13195@pi3>
On 9/19/19 09:59, Krzysztof Kozlowski wrote:
> On Wed, Sep 18, 2019 at 12:46:27PM +0200, Sylwester Nawrocki wrote:
>> The WM1811/WM8994/WM8958 audio CODEC DT bindings specify two optional
>> clocks: "MCLK1", "MCLK2". Add code for getting those clocks in the MFD
>> part of the wm8994 driver so they can be further handled in the audio
>> CODEC part.
>
> I think these are needed only for the codec so how about getting them in
> codec's probe?
The clocks are specified in the (I2C slave device) DT node which corresponds
to the device as a whole and to which binds the MFD driver. AFAICT those
clocks are only needed by the CODEC sub-driver but in general such clocks
could be used to provide device's system clock used by other functionalities
than audio. We are doing something similar for other MFD drivers already
and I would rather stick to one pattern. IMHO it's clearer to see what
device the clocks belong to that way.
--
Regards,
Sylwester
_______________________________________________
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 v1 8/9] ASoC: samsung: arndale: Add missing OF node dereferencing
From: Sylwester Nawrocki @ 2019-09-19 12:49 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: devicetree, alsa-devel, linux-samsung-soc, ckeepax, b.zolnierkie,
sbkim73, patches, lgirdwood, robh+dt, broonie, linux-arm-kernel,
m.szyprowski
In-Reply-To: <20190919082211.GF13195@pi3>
On 9/19/19 10:22, Krzysztof Kozlowski wrote:
> Wasn't this issue introduced in 5/9? It looks weird to fix it here...
It has not been introduced by 5/9, the issue was there already before
my patch, there was even no remove() callback where OF node references
could be dropped.
--
Thanks,
Sylwester
_______________________________________________
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 v1 2/9] mfd: wm8994: Add support for MCLKn clock control
From: Mark Brown @ 2019-09-19 12:50 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: devicetree, alsa-devel, linux-samsung-soc, ckeepax, b.zolnierkie,
sbkim73, patches, lgirdwood, robh+dt, Sylwester Nawrocki,
linux-arm-kernel, m.szyprowski
In-Reply-To: <20190919075924.GB13195@pi3>
[-- Attachment #1.1: Type: text/plain, Size: 613 bytes --]
On Thu, Sep 19, 2019 at 09:59:24AM +0200, Krzysztof Kozlowski wrote:
> On Wed, Sep 18, 2019 at 12:46:27PM +0200, Sylwester Nawrocki wrote:
> > The WM1811/WM8994/WM8958 audio CODEC DT bindings specify two optional
> > clocks: "MCLK1", "MCLK2". Add code for getting those clocks in the MFD
> > part of the wm8994 driver so they can be further handled in the audio
> > CODEC part.
> I think these are needed only for the codec so how about getting them in
> codec's probe?
Yeah. IIRC when these were added a machine driver was grabbing them. I
don't think that machine driver ever made it's way upstream though.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
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 v1 8/9] ASoC: samsung: arndale: Add missing OF node dereferencing
From: Krzysztof Kozlowski @ 2019-09-19 12:58 UTC (permalink / raw)
To: Sylwester Nawrocki
Cc: devicetree, alsa-devel, linux-samsung-soc@vger.kernel.org,
ckeepax, Bartłomiej Żołnierkiewicz, sbkim73,
patches, lgirdwood, robh+dt, broonie, linux-arm-kernel,
Marek Szyprowski
In-Reply-To: <a0072745-f7c1-86ad-2344-d73dd210e1ad@samsung.com>
On Thu, 19 Sep 2019 at 14:49, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote:
>
> On 9/19/19 10:22, Krzysztof Kozlowski wrote:
> > Wasn't this issue introduced in 5/9? It looks weird to fix it here...
>
> It has not been introduced by 5/9, the issue was there already before
> my patch, there was even no remove() callback where OF node references
> could be dropped.
I see. Then please put it as first patch. You should not mix bugfixes
with features. If mixing, be sure they can be applied before the
features.
Best regards,
Krzysztof
_______________________________________________
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 3/3] clk: meson: clk-pll: always enable a critical PLL when setting the rate
From: Jerome Brunet @ 2019-09-19 13:01 UTC (permalink / raw)
To: Neil Armstrong, Stephen Boyd
Cc: linux-amlogic, linux-kernel, linux-clk, linux-arm-kernel,
Neil Armstrong
In-Reply-To: <20190919093809.21364-1-narmstrong@baylibre.com>
On Thu 19 Sep 2019 at 11:38, Neil Armstrong <narmstrong@baylibre.com> wrote:
> Make sure we always enable a PLL on a set_rate() when the PLL is
> flagged as critical.
>
> This fixes the case when the Amlogic G12A SYS_PLL gets disabled by the
> PSCI firmware when resuming from suspend-to-memory, in the case
> where the CPU was not clocked by the SYS_PLL, but by the fixed PLL
> fixed divisors.
> In this particular case, when changing the PLL rate, CCF doesn't handle
> the fact the PLL could have been disabled in the meantime and set_rate()
> only changes the rate and never enables it again.
>
> Fixes: d6e81845b7d9 ("clk: meson: clk-pll: check if the clock is already enabled')
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
> drivers/clk/meson/clk-pll.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
> index ddb1e5634739..8c5adccb7959 100644
> --- a/drivers/clk/meson/clk-pll.c
> +++ b/drivers/clk/meson/clk-pll.c
> @@ -379,7 +379,7 @@ static int meson_clk_pll_set_rate(struct clk_hw *hw, unsigned long rate,
> }
>
> /* If the pll is stopped, bail out now */
> - if (!enabled)
> + if (!(hw->init->flags & CLK_IS_CRITICAL) && !enabled)
This is surely a work around to the issue at hand but:
* Enabling the clock, critical or not, should not be done but the
set_rate() callback. This is not the purpose of this callback.
* Enabling the clock in such way does not walk the tree. So, if there is
ever another PSCI Fw which disable we would get into the same issue
again. IOW, This is not specific to the PLL driver so it should not have
to deal with this.
Since this clock can change out of CCF maybe it should be marked with
CLK_GET_RATE_NOCACHE ?
When CCF hits a clock with CLK_GET_RATE_NOCACHE while walking the tree,
in addition to to calling get_rate(), CCF could also call is_enabled()
if the clock has CLK_IS_CRITICAL and possibly .enable() ?
Stephen, what do you think ?
> return 0;
>
> if (meson_clk_pll_enable(hw)) {
> --
> 2.22.0
_______________________________________________
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] [v2] net: stmmac: selftest: avoid large stack usage
From: Jose Abreu @ 2019-09-19 13:01 UTC (permalink / raw)
To: Arnd Bergmann, Giuseppe Cavallaro, Alexandre Torgue,
David S. Miller, Maxime Coquelin
Cc: netdev@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20190919123416.3070938-1-arnd@arndb.de>
From: Arnd Bergmann <arnd@arndb.de>
Date: Sep/19/2019, 13:33:43 (UTC+00:00)
> Putting a struct stmmac_rss object on the stack is a bad idea,
> as it exceeds the warning limit for a stack frame on 32-bit architectures:
>
> drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c:1221:12: error: stack frame size of 1208 bytes in function '__stmmac_test_l3filt' [-Werror,-Wframe-larger-than=]
> drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c:1338:12: error: stack frame size of 1208 bytes in function '__stmmac_test_l4filt' [-Werror,-Wframe-larger-than=]
>
> As the object is the trivial empty case, change the called function
> to accept a NULL pointer to mean the same thing and remove the
> large variable in the two callers.
>
> Fixes: 4647e021193d ("net: stmmac: selftests: Add selftest for L3/L4 Filters")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> v2: simply configure function, based on feedback from Jose
Looks good to me. Thanks for the fix :)
Acked-by: Jose Abreu <joabreu@synopsys.com>
---
Thanks,
Jose Miguel Abreu
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* "arm64: use asm-generic/dma-mapping.h" introduced a kbuild warning
From: Qian Cai @ 2019-09-19 13:06 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Michal Marek, linux-kbuild, linux-kernel, Masahiro Yamada,
Stefano Stabellini, Will Deacon, linux-arm-kernel
The commit 5489c8e0cf03 ("arm64: use asm-generic/dma-mapping.h") introduced a
kbuild warning,
scripts/Makefile.asm-generic:25: redundant generic-y found in
arch/arm64/include/asm/Kbuild: dma-mapping.h
_______________________________________________
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 RFC 11/14] arm64: Move the ASID allocator code in a separate file
From: Guo Ren @ 2019-09-19 13:07 UTC (permalink / raw)
To: Jean-Philippe Brucker
Cc: aou, Linux Kernel Mailing List, Arnd Bergmann, suzuki.poulose,
Marc Zyngier, Catalin Marinas, Palmer Dabbelt, christoffer.dall,
iommu, Mike Rapoport, Anup Patel, Atish Patra, Julien Grall,
james.morse, gary, Paul Walmsley, linux-riscv, Will Deacon,
kvmarm, linux-arm-kernel
In-Reply-To: <057a0af3-93f7-271c-170e-4b31e6894c3c@linaro.org>
Hi,
On Mon, Sep 16, 2019 at 8:57 PM Jean-Philippe Brucker
<jean-philippe@linaro.org> wrote:
> On 13/09/2019 09:13, Guo Ren wrote:
> > Another idea is seperate remote TLB invalidate into two instructions:
> >
> > - sfence.vma.b.asyc
> > - sfence.vma.b.barrier // wait all async TLB invalidate operations
> > finished for all harts.
>
> It's not clear to me how this helps, but I probably don't have the whole
> picture. If you have a place where it is safe to wait for the barrier to
> complete, why not do the whole invalidate there?
>
> > (I remember who mentioned me separate them into two instructions after
> > session. Anup? Is the idea right ?)
Forget it, I still use irq signal in my formal proposal [1]. I also
couldn't image the whole picture :P
> > To solve the problem, we could define a async mode in sfence.vma.b to
> > slove the problem and finished with per_cpu_irq/exception.
>
> The solution I had to this problem is pinning the ASID [1] used by the
> IOMMU, to prevent the CPU from recycling the ASID on rollover. This way
> the CPU doesn't have to wait for IOMMU invalidations to complete, when
> scheduling a task that might not even have anything to do with the IOMMU.
>
> In the Arm SMMU, ASID and IOASID (PASID) are separate identifiers. IOASID
> indexes an entry in the context descriptor table, which contains the ASID.
> So with unpinned shared ASID you don't need to invalidate the ATC on
> rollover, since the IOASID doesn't change, but you do need to modify the
> context descriptor and invalidate cached versions of it.
The terminology confused me a lot. I perfer use PASID for IOMMU and
ASID is for CPU.
Arm's entry of the context descriptor table contains a "IOASID"
IOASID != ASID for CPU_TLB and IOMMU_TLB.
When you say "since the IOASID doesn't change",Is it PASID or my IOASID ? -_*!
PASID in PCI-sig was used to determine transfer address space.
For intel, the entry which is indexed by PASID also contain S1/S2.PGD
and DID(VMID).
For arm, the entry which is indexed by PASID only contain S1.PGD and
IOASID. Compare to Intel Vt-d Scalable mode, arm's design can't
support PCI Virtual Function.
>
> Once you have pinned ASIDs, you could also declare that IOASID = ASID. I
> don't remember finding an argument to strictly forbid it, even though ASID
> and IOASID have different sizes on Arm (respectively 8/16 and 20 bits).
ASID and IOASID are hard to keep the same between CPU system and IOMMU
system. So I introduce S1/S2.PGD.PPN as a bridge between CPUs and
IOMMUs.
See my proposal [1]
1: https://lore.kernel.org/linux-csky/1568896556-28769-1-git-send-email-guoren@kernel.org/T/#u
--
Best Regards
Guo Ren
ML: https://lore.kernel.org/linux-csky/
_______________________________________________
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 v1 2/9] mfd: wm8994: Add support for MCLKn clock control
From: Krzysztof Kozlowski @ 2019-09-19 13:07 UTC (permalink / raw)
To: Sylwester Nawrocki
Cc: devicetree, alsa-devel, linux-samsung-soc@vger.kernel.org,
ckeepax, Bartłomiej Żołnierkiewicz, sbkim73,
patches, broonie, lgirdwood, robh+dt, linux-arm-kernel,
Marek Szyprowski
In-Reply-To: <420d3c8a-a31e-1edf-3112-b9800beace1e@samsung.com>
On Thu, 19 Sep 2019 at 14:49, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote:
>
> On 9/19/19 09:59, Krzysztof Kozlowski wrote:
> > On Wed, Sep 18, 2019 at 12:46:27PM +0200, Sylwester Nawrocki wrote:
> >> The WM1811/WM8994/WM8958 audio CODEC DT bindings specify two optional
> >> clocks: "MCLK1", "MCLK2". Add code for getting those clocks in the MFD
> >> part of the wm8994 driver so they can be further handled in the audio
> >> CODEC part.
> >
> > I think these are needed only for the codec so how about getting them in
> > codec's probe?
>
> The clocks are specified in the (I2C slave device) DT node which corresponds
> to the device as a whole and to which binds the MFD driver. AFAICT those
> clocks are only needed by the CODEC sub-driver but in general such clocks
> could be used to provide device's system clock used by other functionalities
> than audio. We are doing something similar for other MFD drivers already
> and I would rather stick to one pattern. IMHO it's clearer to see what
> device the clocks belong to that way.
The existing pattern is not an excuse for doing some things in a
uglier way... If we agree that these are codec-only resources, then
they should be acquired by codec driver. This is one minor step to
solve difficult inter-device dependencies which stops from reusing
parts of the code (some child of MFD heavily depends on parent).
Existing MFD drivers sometimes follow this pattern but that's wrong.
They follow the ugly or even crappy pattern.
Your codec driver still refers to parent and it has its resources,
e.g. parent's device node pointer.
Best regards,
Krzysztof
_______________________________________________
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] drm: sun8i-ui/vi: Fix layer zpos change/atomic modesetting
From: Ondřej Jirman @ 2019-09-19 13:12 UTC (permalink / raw)
To: Maxime Ripard, David Airlie, Daniel Vetter, Chen-Yu Tsai,
dri-devel, linux-arm-kernel, linux-kernel
In-Reply-To: <20190919122058.fhpuafogdq7oir2d@core.my.home>
On Thu, Sep 19, 2019 at 02:20:58PM +0200, megous hlavni wrote:
> On Wed, Sep 18, 2019 at 10:16:17PM +0200, Maxime Ripard wrote:
> > On Wed, Sep 18, 2019 at 05:23:09PM +0200, Ondřej Jirman wrote:
> > > Hi,
> > >
> > > On Wed, Sep 18, 2019 at 04:17:34PM +0200, Maxime Ripard wrote:
> > > > Hi,
> > > >
> > > > On Sun, Sep 15, 2019 at 12:03:37AM +0200, megous@megous.com wrote:
> > > > > From: Ondrej Jirman <megous@megous.com>
> > > > >
> > > > > There are various issues that this re-work of sun8i_[uv]i_layer_enable
> > > > > function fixes:
> > > > >
> > > > > - Make sure that we re-initialize zpos on reset
> > > > > - Minimize register updates by doing them only when state changes
> > > > > - Fix issue where DE pipe might get disabled even if it is no longer
> > > > > used by the layer that's currently calling sun8i_ui_layer_enable
> > > > > - .atomic_disable callback is not really needed because .atomic_update
> > > > > can do the disable too, so drop the duplicate code
> > > > >
> > > > > Signed-off-by: Ondrej Jirman <megous@megous.com>
> > > >
> > > > It looks like these fixes should be in separate patches. Is there any
> > > > reason it's not the case?
> > >
> > > Bullet points just describe the resulting effect/benefits of the change to fix
> > > the pipe control register update issue (see the referenced e-mail).
> >
> > It's definitely ok to have multiple patches needed to address a single
> > perceived issue.
>
> Yes, but I can't simply split the patch. In order for each change to work on its
> own, they'd have to be done differently than the final result.
>
> I wouldn't mind at all if it was just a simple splitting, but you're asking
> for too much work, this time, for no benefit that I can see.
>
> > A commit is not about what you're fixing but what you're changing. And
> > the fact that you have tha bullet list in the first place proves that
> > you have multiple logical changes in your patch.
> >
> > And even then, your commit log mentions that you're fixing multiple
> > issues (without explaining them).
>
> I can reword the commit message if that helps, and skip the bullet list if it
> is confusing. There's a single core issue and that is that the driver doesn't
> update the pipe/channel configuration correctly leading to disabling of
> arbitrary layers (not even those being updated - update of UI layer may disable
> VI layer as a side effect for example) at wrong times. And only changes
> necessary to debug/fix this are included.
How about this:
A problem was found where identical configuration of planes leads
to different register settings at the HW layer when using a X server
with modesetting driver and one plane marked as a cursor.
On first run of the X server, only the black screen and the layer
containing the cursor is visible. Switching to console and back
corrects the situation.
I have dumped registers, and found out this:
(In both cases there are two enabled planes, plane 1 with zpos 0 and
plane 3 with zpos 1).
1) First Xorg run:
0x01101000 : 00000201
0x01101080 : 00000030
BLD_FILL_COLOR_CTL: (aka SUN8I_MIXER_BLEND_PIPE_CTL)
P1_EN
BLD_CH_RTCTL: (aka SUN8I_MIXER_BLEND_ROUTE)
P0_RTCTL channel0
P1_RTCTL channel3
2) After switch to console and back to Xorg:
0x01101000 : 00000301
0x01101080 : 00000031
BLD_FILL_COLOR_CTL:
P1_EN and P0_EN
BLD_CH_RTCTL:
P0_RTCTL channel1
P1_RTCTL channel3
What happens is that sun8i_ui_layer_enable() function may disable
blending pipes even if it is no longer assigned to its layer, because
of incorrect state/zpos tracking in the driver.
In particular, layer 1 is configured to zpos 0 and thus uses pipe 0.
When layer 3 is enabled by X server, sun8i_ui_layer_enable() will get
called with old_zpos=0 zpos=1, which will lead to disabling of pipe 0.
In general this issue can happen to any layer during enable or zpos
changes on multiple layers at once.
To correct this we now pass previous enabled/disabled state of the
layer, and pass real previous zpos of the layer to
sun8i_ui_layer_enable() and rework the sun8i_ui_layer_enable() function
to react to the state changes correctly. In order to not complicate
the atomic_disable callback with all of the above changes, we simply
remove it and implement all the chanes as part of atomic_update, which
also reduces the code duplication.
To make this all work, initial zpos positions of all layers need to be
restored to initial values on reset.
> I may try generating a nicer patch with a different diff options, if it makes it
> more readable for review.
>
> > > I can maybe split off the first bullet point into a separate patch. But
> > > I can't guarantee it will not make the original issue worse, because it might
> > > have been hiding the other issue with register updates.
> > >
> > > The rest is just a result of the single logical change. It doesn't work
> > > individually, it all has the goal of fixing the issue as a whole.
> > >
> > > If I were to split it I would have to actually re-implement .atomic_disable
> > > callback only to remove it in the next patch. I don't see the benefit.
> >
> > Your commit log says that you remove atomic_disable. Why would you
> > remove it, to add it back, to remove it again?
>
> Because if I remove it I need to re-implement the functionality in the update
> callback. The core will change what is called based on presence of callbacks.
> It's not a simple removal.
>
> If I first implement the new sun8i_[uv]i_layer_enable and update callback,
> keeping a disable callback would not work, because the new update callback
> will only work if disable callback is not defined (because it it is, then
> the drm core will not call the update callback in all cases that I need).
>
> regards,
> o.
>
> > Maxime
>
>
>
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
_______________________________________________
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