* [PATCH net-next v3 4/4] ARM64: dts: meson: odroidc2: disable advertisement EEE for GbE.
From: Jerome Brunet @ 2016-11-28 9:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480326409-25419-1-git-send-email-jbrunet@baylibre.com>
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
index e6e3491d48a5..5624714d2b16 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
@@ -46,6 +46,7 @@
#include "meson-gxbb.dtsi"
#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/net/mdio.h>
/ {
compatible = "hardkernel,odroid-c2", "amlogic,meson-gxbb";
@@ -98,3 +99,18 @@
pinctrl-0 = <&i2c_a_pins>;
pinctrl-names = "default";
};
+
+ðmac {
+ phy-handle = <ð_phy0>;
+
+ mdio {
+ compatible = "snps,dwmac-mdio";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ eth_phy0: ethernet-phy at 0 {
+ reg = <0>;
+ eee-broken-modes = <MDIO_EEE_1000T>;
+ };
+ };
+};
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v3 3/4] dt: bindings: add ethernet phy eee-broken-modes option documentation
From: Jerome Brunet @ 2016-11-28 9:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480326409-25419-1-git-send-email-jbrunet@baylibre.com>
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
Documentation/devicetree/bindings/net/phy.txt | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/phy.txt b/Documentation/devicetree/bindings/net/phy.txt
index 4627da3d52c4..54749b60a466 100644
--- a/Documentation/devicetree/bindings/net/phy.txt
+++ b/Documentation/devicetree/bindings/net/phy.txt
@@ -38,6 +38,8 @@ Optional Properties:
- enet-phy-lane-swap: If set, indicates the PHY will swap the TX/RX lanes to
compensate for the board being designed with the lanes swapped.
+- eee-broken-modes: Bits to clear in the MDIO_AN_EEE_ADV register to
+ disable EEE broken modes.
Example:
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v3 2/4] dt-bindings: net: add EEE capability constants
From: Jerome Brunet @ 2016-11-28 9:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480326409-25419-1-git-send-email-jbrunet@baylibre.com>
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
include/dt-bindings/net/mdio.h | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
create mode 100644 include/dt-bindings/net/mdio.h
diff --git a/include/dt-bindings/net/mdio.h b/include/dt-bindings/net/mdio.h
new file mode 100644
index 000000000000..99c6d903d439
--- /dev/null
+++ b/include/dt-bindings/net/mdio.h
@@ -0,0 +1,19 @@
+/*
+ * This header provides generic constants for ethernet MDIO bindings
+ */
+
+#ifndef _DT_BINDINGS_NET_MDIO_H
+#define _DT_BINDINGS_NET_MDIO_H
+
+/*
+ * EEE capability Advertisement
+ */
+
+#define MDIO_EEE_100TX 0x0002 /* 100TX EEE cap */
+#define MDIO_EEE_1000T 0x0004 /* 1000T EEE cap */
+#define MDIO_EEE_10GT 0x0008 /* 10GT EEE cap */
+#define MDIO_EEE_1000KX 0x0010 /* 1000KX EEE cap */
+#define MDIO_EEE_10GKX4 0x0020 /* 10G KX4 EEE cap */
+#define MDIO_EEE_10GKR 0x0040 /* 10G KR EEE cap */
+
+#endif
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v3 1/4] net: phy: add an option to disable EEE advertisement
From: Jerome Brunet @ 2016-11-28 9:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480326409-25419-1-git-send-email-jbrunet@baylibre.com>
This patch adds an option to disable EEE advertisement in the generic PHY
by providing a mask of prohibited modes corresponding to the value found in
the MDIO_AN_EEE_ADV register.
On some platforms, PHY Low power idle seems to be causing issues, even
breaking the link some cases. The patch provides a convenient way for these
platforms to disable EEE advertisement and work around the issue.
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
drivers/net/phy/phy.c | 3 ++
drivers/net/phy/phy_device.c | 80 +++++++++++++++++++++++++++++++++++++++-----
include/linux/phy.h | 3 ++
3 files changed, 77 insertions(+), 9 deletions(-)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 73adbaa9ac86..a3981cc6448a 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -1396,6 +1396,9 @@ int phy_ethtool_set_eee(struct phy_device *phydev, struct ethtool_eee *data)
{
int val = ethtool_adv_to_mmd_eee_adv_t(data->advertised);
+ /* Mask prohibited EEE modes */
+ val &= ~phydev->eee_broken_modes;
+
phy_write_mmd_indirect(phydev, MDIO_AN_EEE_ADV, MDIO_MMD_AN, val);
return 0;
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index ba86c191a13e..83e52f1b80f2 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1121,6 +1121,43 @@ static int genphy_config_advert(struct phy_device *phydev)
}
/**
+ * genphy_config_eee_advert - disable unwanted eee mode advertisement
+ * @phydev: target phy_device struct
+ *
+ * Description: Writes MDIO_AN_EEE_ADV after disabling unsupported energy
+ * efficent ethernet modes. Returns 0 if the PHY's advertisement hasn't
+ * changed, and 1 if it has changed.
+ */
+static int genphy_config_eee_advert(struct phy_device *phydev)
+{
+ u32 broken = phydev->eee_broken_modes;
+ u32 old_adv, adv;
+
+ /* Nothing to disable */
+ if (!broken)
+ return 0;
+
+ /* If the following call fails, we assume that EEE is not
+ * supported by the phy. If we read 0, EEE is not advertised
+ * In both case, we don't need to continue
+ */
+ adv = phy_read_mmd_indirect(phydev, MDIO_AN_EEE_ADV, MDIO_MMD_AN);
+ if (adv <= 0)
+ return 0;
+
+ old_adv = adv;
+ adv &= ~broken;
+
+ /* Advertising remains unchanged with the broken mask */
+ if (old_adv == adv)
+ return 0;
+
+ phy_write_mmd_indirect(phydev, MDIO_AN_EEE_ADV, MDIO_MMD_AN, adv);
+
+ return 1;
+}
+
+/**
* genphy_setup_forced - configures/forces speed/duplex from @phydev
* @phydev: target phy_device struct
*
@@ -1178,15 +1215,20 @@ EXPORT_SYMBOL(genphy_restart_aneg);
*/
int genphy_config_aneg(struct phy_device *phydev)
{
- int result;
+ int err, changed;
+
+ changed = genphy_config_eee_advert(phydev);
if (AUTONEG_ENABLE != phydev->autoneg)
return genphy_setup_forced(phydev);
- result = genphy_config_advert(phydev);
- if (result < 0) /* error */
- return result;
- if (result == 0) {
+ err = genphy_config_advert(phydev);
+ if (err < 0) /* error */
+ return err;
+
+ changed |= err;
+
+ if (changed == 0) {
/* Advertisement hasn't changed, but maybe aneg was never on to
* begin with? Or maybe phy was isolated?
*/
@@ -1196,16 +1238,16 @@ int genphy_config_aneg(struct phy_device *phydev)
return ctl;
if (!(ctl & BMCR_ANENABLE) || (ctl & BMCR_ISOLATE))
- result = 1; /* do restart aneg */
+ changed = 1; /* do restart aneg */
}
/* Only restart aneg if we are advertising something different
* than we were before.
*/
- if (result > 0)
- result = genphy_restart_aneg(phydev);
+ if (changed > 0)
+ return genphy_restart_aneg(phydev);
- return result;
+ return 0;
}
EXPORT_SYMBOL(genphy_config_aneg);
@@ -1563,6 +1605,21 @@ static void of_set_phy_supported(struct phy_device *phydev)
__set_phy_supported(phydev, max_speed);
}
+static void of_set_phy_eee_broken(struct phy_device *phydev)
+{
+ struct device_node *node = phydev->mdio.dev.of_node;
+ u32 broken;
+
+ if (!IS_ENABLED(CONFIG_OF_MDIO))
+ return;
+
+ if (!node)
+ return;
+
+ if (!of_property_read_u32(node, "eee-broken-modes", &broken))
+ phydev->eee_broken_modes = broken;
+}
+
/**
* phy_probe - probe and init a PHY device
* @dev: device to probe and init
@@ -1600,6 +1657,11 @@ static int phy_probe(struct device *dev)
of_set_phy_supported(phydev);
phydev->advertising = phydev->supported;
+ /* Get the EEE modes we want to prohibit. We will ask
+ * the PHY stop advertising these mode later on
+ */
+ of_set_phy_eee_broken(phydev);
+
/* Set the state to READY by default */
phydev->state = PHY_READY;
diff --git a/include/linux/phy.h b/include/linux/phy.h
index edde28ce163a..b53177fd38af 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -417,6 +417,9 @@ struct phy_device {
u32 advertising;
u32 lp_advertising;
+ /* Energy efficient ethernet modes which should be prohibited */
+ u32 eee_broken_modes;
+
int autoneg;
int link_timeout;
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v3 0/4] Fix OdroidC2 Gigabit Tx link issue
From: Jerome Brunet @ 2016-11-28 9:46 UTC (permalink / raw)
To: linux-arm-kernel
This patchset fixes an issue with the OdroidC2 board (DWMAC + RTL8211F).
The platform seems to enter LPI on the Rx path too often while performing
relatively high TX transfer. This eventually break the link (both Tx and
Rx), and require to bring the interface down and up again to get the Rx
path working again.
The root cause of this issue is not fully understood yet but disabling EEE
advertisement on the PHY prevent this feature to be negotiated.
With this change, the link is stable and reliable, with the expected
throughput performance.
The patchset adds options in the generic phy driver to disable EEE
advertisement, through device tree. The way it is done is very similar
to the handling of the max-speed property.
Changes since V2: [2]
- Rename "eee-advert-disable" to "eee-broken-modes" to make the intended
purpose of this option clear (flag broken configuration, not a
configuration option)
- Add DT bindings constants so the DT configuration is more user friendly
- Submit to net-next instead of net.
Changes since V1: [1]
- Disable the advertisement of EEE in the generic code instead of the
realtek driver.
[1] : http://lkml.kernel.org/r/1479220154-25851-1-git-send-email-jbrunet at baylibre.com
[2] : http://lkml.kernel.org/r/1479742524-30222-1-git-send-email-jbrunet at baylibre.com
Jerome Brunet (4):
net: phy: add an option to disable EEE advertisement
dt-bindings: net: add EEE capability constants
dt: bindings: add ethernet phy eee-broken-modes option documentation
ARM64: dts: meson: odroidc2: disable advertisement EEE for GbE.
Documentation/devicetree/bindings/net/phy.txt | 2 +
.../arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts | 16 +++++
drivers/net/phy/phy.c | 3 +
drivers/net/phy/phy_device.c | 80 +++++++++++++++++++---
include/dt-bindings/net/mdio.h | 19 +++++
include/linux/phy.h | 3 +
6 files changed, 114 insertions(+), 9 deletions(-)
create mode 100644 include/dt-bindings/net/mdio.h
--
2.7.4
^ permalink raw reply
* [kvm-unit-tests PATCH v7 07/11] arm/tlbflush-code: Add TLB flush during code execution test
From: Andrew Jones @ 2016-11-28 9:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161124161033.11456-8-alex.bennee@linaro.org>
On Thu, Nov 24, 2016 at 04:10:29PM +0000, Alex Benn?e wrote:
> This adds a fairly brain dead torture test for TLB flushes intended for
> stressing the MTTCG QEMU build. It takes the usual -smp option for
> multiple CPUs.
>
> By default it CPU0 will do a TLBIALL flush after each cycle. You can
> pass options via -append to control additional aspects of the test:
>
> - "page" flush each page in turn (one per function)
> - "self" do the flush after each computation cycle
> - "verbose" report progress on each computation cycle
>
> Signed-off-by: Alex Benn?e <alex.bennee@linaro.org>
> CC: Mark Rutland <mark.rutland@arm.com>
>
> ---
> v2
> - rename to tlbflush-test
> - made makefile changes cleaner
> - added self/other flush mode
> - create specific prefix
> - whitespace fixes
> v3
> - using new SMP framework for test runing
> v4
> - merge in the unitests.cfg
> v5
> - max out at -smp 4
> - printf fmtfix
> v7
> - rename to tlbflush-code
> - int -> bool flags
> ---
> arm/Makefile.common | 2 +
> arm/tlbflush-code.c | 212 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> arm/unittests.cfg | 24 ++++++
> 3 files changed, 238 insertions(+)
> create mode 100644 arm/tlbflush-code.c
>
> diff --git a/arm/Makefile.common b/arm/Makefile.common
> index cca0d9c..de99a6e 100644
> --- a/arm/Makefile.common
> +++ b/arm/Makefile.common
> @@ -13,6 +13,7 @@ tests-common = $(TEST_DIR)/selftest.flat
> tests-common += $(TEST_DIR)/spinlock-test.flat
> tests-common += $(TEST_DIR)/pci-test.flat
> tests-common += $(TEST_DIR)/gic.flat
> +tests-common += $(TEST_DIR)/tlbflush-code.flat
>
> all: test_cases
>
> @@ -81,3 +82,4 @@ generated_files = $(asm-offsets)
> test_cases: $(generated_files) $(tests-common) $(tests)
>
> $(TEST_DIR)/selftest.o $(cstart.o): $(asm-offsets)
> +$(TEST_DIR)/tlbflush-code.elf: $(cstart.o) $(TEST_DIR)/tlbflush-code.o
This should no longer be necessary.
> diff --git a/arm/tlbflush-code.c b/arm/tlbflush-code.c
> new file mode 100644
> index 0000000..cb5cdc2
> --- /dev/null
> +++ b/arm/tlbflush-code.c
> @@ -0,0 +1,212 @@
> +/*
> + * TLB Flush Race Tests
> + *
> + * These tests are designed to test for incorrect TLB flush semantics
> + * under emulation. The initial CPU will set all the others working a
> + * compuation task and will then trigger TLB flushes across the
> + * system. It doesn't actually need to re-map anything but the flushes
> + * themselves will trigger QEMU's TCG self-modifying code detection
> + * which will invalidate any generated code causing re-translation.
> + * Eventually the code buffer will fill and a general tb_lush() will
> + * be triggered.
> + *
> + * Copyright (C) 2016, Linaro, Alex Benn?e <alex.bennee@linaro.org>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.
> + */
> +
> +#include <libcflat.h>
> +#include <asm/smp.h>
> +#include <asm/cpumask.h>
> +#include <asm/barrier.h>
> +#include <asm/mmu.h>
> +
> +#define SEQ_LENGTH 10
> +#define SEQ_HASH 0x7cd707fe
> +
> +static cpumask_t smp_test_complete;
> +static int flush_count = 1000000;
> +static bool flush_self;
> +static bool flush_page;
> +static bool flush_verbose;
> +
> +/*
> + * Work functions
> + *
> + * These work functions need to be:
> + *
> + * - page aligned, so we can flush one function at a time
> + * - have branches, so QEMU TCG generates multiple basic blocks
> + * - call across pages, so we exercise the TCG basic block slow path
> + */
> +
> +/* Adler32 */
> +__attribute__((aligned(PAGE_SIZE))) uint32_t hash_array(const void *buf,
> + size_t buflen)
I think I'd prefer
__attribute__((aligned(PAGE_SIZE)))
uint32_t hash_array(const void *buf, size_t buflen)
to handle the long line
> +{
> + const uint8_t *data = (uint8_t *) buf;
> + uint32_t s1 = 1;
> + uint32_t s2 = 0;
> +
> + for (size_t n = 0; n < buflen; n++) {
> + s1 = (s1 + data[n]) % 65521;
> + s2 = (s2 + s1) % 65521;
> + }
> + return (s2 << 16) | s1;
> +}
> +
> +__attribute__((aligned(PAGE_SIZE))) void create_fib_sequence(int length,
> + unsigned int *array)
> +{
> + int i;
> +
> + /* first two values */
> + array[0] = 0;
> + array[1] = 1;
> + for (i=2; i<length; i++) {
> + array[i] = array[i-2] + array[i-1];
> + }
please don't use {} for one-liners. Try running the kernel's check_patch
on your patches. Applies many places below
> +}
> +
> +__attribute__((aligned(PAGE_SIZE))) unsigned long long factorial(unsigned int n)
long line
> +{
> + unsigned int i;
> + unsigned long long fac = 1;
> + for (i=1; i<=n; i++)
> + {
> + fac = fac * i;
> + }
> + return fac;
> +}
> +
> +__attribute__((aligned(PAGE_SIZE))) void factorial_array
> +(unsigned int n, unsigned int *input, unsigned long long *output)
> +{
> + unsigned int i;
> + for (i=0; i<n; i++) {
> + output[i] = factorial(input[i]);
> + }
> +}
> +
> +__attribute__((aligned(PAGE_SIZE))) unsigned int do_computation(void)
> +{
> + unsigned int fib_array[SEQ_LENGTH];
> + unsigned long long facfib_array[SEQ_LENGTH];
> + uint32_t fib_hash, facfib_hash;
> +
> + create_fib_sequence(SEQ_LENGTH, &fib_array[0]);
> + fib_hash = hash_array(&fib_array[0], sizeof(fib_array));
> + factorial_array(SEQ_LENGTH, &fib_array[0], &facfib_array[0]);
> + facfib_hash = hash_array(&facfib_array[0], sizeof(facfib_array));
> +
> + return (fib_hash ^ facfib_hash);
> +}
> +
> +/* This provides a table of the work functions so we can flush each
> + * page individually
> + */
> +static void * pages[] = {&hash_array, &create_fib_sequence, &factorial,
> + &factorial_array, &do_computation};
please put the '*' by pages
> +
> +static void do_flush(int i)
> +{
> + if (flush_page) {
> + flush_tlb_page((unsigned long)pages[i % ARRAY_SIZE(pages)]);
> + } else {
> + flush_tlb_all();
> + }
> +}
> +
> +
> +static void just_compute(void)
> +{
> + int i, errors = 0;
> + int cpu = smp_processor_id();
> +
> + uint32_t result;
> +
> + printf("CPU%d online\n", cpu);
> +
> + for (i=0; i < flush_count; i++) {
> + result = do_computation();
> +
> + if (result != SEQ_HASH) {
> + errors++;
> + printf("CPU%d: seq%d 0x%"PRIx32"!=0x%x\n",
> + cpu, i, result, SEQ_HASH);
> + }
> +
> + if (flush_verbose && (i % 1000) == 0) {
> + printf("CPU%d: seq%d\n", cpu, i);
> + }
> +
> + if (flush_self) {
> + do_flush(i);
> + }
> + }
> +
> + report("CPU%d: Done - Errors: %d\n", errors == 0, cpu, errors);
> +
> + cpumask_set_cpu(cpu, &smp_test_complete);
> + if (cpu != 0)
> + halt();
> +}
> +
> +static void just_flush(void)
> +{
> + int cpu = smp_processor_id();
> + int i = 0;
> +
> + /* set our CPU as done, keep flushing until everyone else
> + finished */
Not our comment style
> + cpumask_set_cpu(cpu, &smp_test_complete);
> +
> + while (!cpumask_full(&smp_test_complete)) {
> + do_flush(i++);
> + }
> +
> + report("CPU%d: Done - Triggered %d flushes\n", true, cpu, i);
> +}
> +
> +int main(int argc, char **argv)
> +{
> + int cpu, i;
> + char prefix[100];
> +
> + for (i=0; i<argc; i++) {
> + char *arg = argv[i];
> +
> + if (strcmp(arg, "page") == 0) {
> + flush_page = true;
> + }
> +
> + if (strcmp(arg, "self") == 0) {
> + flush_self = true;
> + }
> +
> + if (strcmp(arg, "verbose") == 0) {
> + flush_verbose = true;
> + }
> + }
> +
> + snprintf(prefix, sizeof(prefix), "tlbflush_%s_%s",
> + flush_page?"page":"all",
> + flush_self?"self":"other");
> + report_prefix_push(prefix);
> +
> + for_each_present_cpu(cpu) {
> + if (cpu == 0)
> + continue;
> + smp_boot_secondary(cpu, just_compute);
> + }
> +
> + if (flush_self)
> + just_compute();
> + else
> + just_flush();
> +
> + while (!cpumask_full(&smp_test_complete))
> + cpu_relax();
> +
> + return report_summary();
> +}
> diff --git a/arm/unittests.cfg b/arm/unittests.cfg
> index c7392c7..beaae84 100644
> --- a/arm/unittests.cfg
> +++ b/arm/unittests.cfg
> @@ -72,3 +72,27 @@ file = gic.flat
> smp = $MAX_SMP
> extra_params = -machine gic-version=3 -append 'ipi'
> groups = gic
> +
> +# TLB Torture Tests
> +[tlbflush-code::all_other]
We don't use the '::' style anymore, as it doesn't work
well with mkstandalone.
> +file = tlbflush-code.flat
> +smp = $(($MAX_SMP>4?4:$MAX_SMP))
> +groups = tlbflush
> +
> +[tlbflush-code::page_other]
> +file = tlbflush-code.flat
> +smp = $(($MAX_SMP>4?4:$MAX_SMP))
> +extra_params = -append 'page'
> +groups = tlbflush
> +
> +[tlbflush-code::all_self]
> +file = tlbflush-code.flat
> +smp = $(($MAX_SMP>4?4:$MAX_SMP))
> +extra_params = -append 'self'
> +groups = tlbflush
> +
> +[tlbflush-code::page_self]
> +file = tlbflush-code.flat
> +smp = $(($MAX_SMP>4?4:$MAX_SMP))
> +extra_params = -append 'page self'
> +groups = tlbflush
> --
> 2.10.1
>
I only did a superficial review, but it looks familiar. I guess I've
reviewed some of it before.
drew
^ permalink raw reply
* [RFC PATCH 3/3] dt-bindings: display: add Amlogic Meson DRM Bindings
From: Laurent Pinchart @ 2016-11-28 9:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <9f32e3bd-531b-0be7-8579-3af52469c421@baylibre.com>
Hi Neil,
On Monday 28 Nov 2016 10:23:43 Neil Armstrong wrote:
> On 11/28/2016 09:33 AM, Laurent Pinchart wrote:
> > On Friday 25 Nov 2016 17:03:11 Neil Armstrong wrote:
> >> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> >> ---
> >>
> >> .../bindings/display/meson/meson-drm.txt | 134 +++++++++++++++
> >> 1 file changed, 134 insertions(+)
> >> create mode 100644
> >>
> >> Documentation/devicetree/bindings/display/meson/meson-drm.txt
> >>
> >> diff --git
> >> a/Documentation/devicetree/bindings/display/meson/meson-drm.txt
> >> b/Documentation/devicetree/bindings/display/meson/meson-drm.txt new file
> >> mode 100644
> >> index 0000000..89c1b5f
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/display/meson/meson-drm.txt
> >> @@ -0,0 +1,134 @@
> >> +Amlogic Meson Display Controller
> >> +================================
> >> +
> >> +The Amlogic Meson Display controller is composed of several components
> >> +that are going to be documented below:
> >> +
> >> +DMC|---------------VPU (Video Processing Unit)------------|------
> >> HHI------|
> >> + | vd1 _______ _____________ _____________ |
> >> |
> >> +D |-------| |----| | | | | HDMI PLL
> >> |
> >> +D | vd2 | VIU | | Video Post | | Video Encs |<---|-----VCLK
> >> |
> >> +R |-------| |----| Processing | | | |
> >> |
> >> + | osd2 | | | |---| Enci ------|----|-----
> >> VDAC------|
> >> +R |-------| CSC |----| Scalers | | Encp ------|----|----HDMI-
> >> TX----|
> >> +A | osd1 | | | Blenders | |
> >> Encl-------|----|---------------|
> >> +M |-------|______|----|____________| |____________| |
> >> |
> >> +___|______________________________________________________|____________
> >> ___|
> >> +
> >> +
> >> +VIU: Video Input Unit
> >> +---------------------
> >> +
> >> +The Video Input Unit is in charge of the pixel scanout from the DDR
> >> memory.
> >> +It fetches the frames addresses, stride and parameters from the "Canvas"
> >> memory.
> >> +This part is also in charge of the CSC (Colorspace Conversion).
> >> +It can handle 2 OSD Planes and 2 Video Planes.
> >> +
> >> +VPP: Video Processing Unit
> >
> > Do you mean "Video Post Processing" ? In your diagram above Video
> > Processing Unit is abbreviated VPU and covers the VIU, VPP and encoders.
>
> Exact, I meant VPP here.
>
> >> +--------------------------
> >> +
> >> +The Video Processing Unit is in charge if the scaling and blending of
> >> the
> >> +various planes into a single pixel stream.
> >> +There is a special "pre-blending" used by the video planes with a
> >> dedicated
> >> +scaler and a "post-blending" to merge with the OSD Planes.
> >> +The OSD planes also have a dedicated scaler for one of the OSD.
> >> +
> >> +VENC: Video Encoders
> >> +--------------------
> >> +
> >> +The VENC is composed of the multiple pixel encoders :
> >> + - ENCI : Interlace Video encoder for CVBS and Interlace HDMI
> >> + - ENCP : Progressive Video Encoder for HDMI
> >> + - ENCL : LCD LVDS Encoder
> >> +The VENC Unit gets a Pixel Clocks (VCLK) from a dedicated HDMI PLL and
> >> clock
> >> +tree and provides the scanout clock to the VPP and VIU.
> >> +The ENCI is connected to a single VDAC for Composite Output.
> >> +The ENCI and ENCP are connected to an on-chip HDMI Transceiver.
> >> +
> >> +Device Tree Bindings:
> >> +---------------------
> >> +
> >> +VPU: Video Processing Unit
> >> +--------------------------
> >> +
> >> +Required properties:
> >> + - compatible: value should be different for each SoC family as :
> >> + - GXBB (S905) : "amlogic,meson-gxbb-vpu"
> >> + - GXL (S905X, S905D) : "amlogic,meson-gxl-vpu"
> >> + - GXM (S912) : "amlogic,meson-gxm-vpu"
> >> + followed by the common "amlogic,meson-gx-vpu"
> >> + - reg: base address and size of he following memory-mapped regions :
> >> + - vpu
> >> + - hhi
> >> + - dmc
> >> + - reg-names: should contain the names of the previous memory regions
> >> + - interrupts: should contain the VENC Vsync interrupt number
> >> +
> >> +- ports: A ports node with endpoint definitions as defined in
> >> + Documentation/devicetree/bindings/media/video-interfaces.txt. The
> >> + second port should be the output endpoints for VENC connectors.
> >> +
> >> +VENC CBVS Output
> >> +----------------------
> >> +
> >> +The VENC can output Composite/CVBS output via a decicated VDAC.
> >> +
> >> +Required properties:
> >> + - compatible: value must be one of:
> >> + - compatible: value should be different for each SoC family as :
> > One of those two lines is redundant.
>
> Will fix.
>
> >> + - GXBB (S905) : "amlogic,meson-gxbb-venc-cvbs"
> >> + - GXL (S905X, S905D) : "amlogic,meson-gxl-venc-cvbs"
> >> + - GXM (S912) : "amlogic,meson-gxm-venc-cvbs"
> >> + followed by the common "amlogic,meson-gx-venc-cvbs"
> >> +
> >
> > No registers ? Are the encoders registers part of the VPU register space,
> > intertwined in a way that they can't be specified separately here ?
>
> Exact, all the video registers on the Amlogic SoC are part of a long history
> of fixup/enhance from very old SoCs, it's quite hard to distinguish a Venc
> registers array since they are mixed with the multiple encoders
> registers...
In that case is there really a reason to model the encoders as separate nodes
in DT ?
> The only separate registers are the VDAC and HDMI PHY, I may move them to
> these separate nodes since they are part of the HHI register space.
>
> It is a problem if I move them in the next release ? Next release will
> certainly have HDMI support, and will have these refactorings.
Given that DT bindings are considered as a stable ABI, I'm afraid it's an
issue.
> >> +- ports: A ports node with endpoint definitions as defined in
> >> + Documentation/devicetree/bindings/media/video-interfaces.txt. The
> >> + first port should be the input endpoints, connected ot the VPU node.
> >> +
> >> +Example:
> >> +
> >> +venc_cvbs: venc-cvbs {
> >> + compatible = "amlogic,meson-gxbb-venc-cvbs";
> >> + status = "okay";
> >> +
> >> + ports {
> >> + #address-cells = <1>;
> >> + #size-cells = <0>;
> >> +
> >> + enc_cvbs_in: port at 0 {
> >> + #address-cells = <1>;
> >> + #size-cells = <0>;
> >> + reg = <0>;
> >> +
> >> + venc_cvbs_in_vpu: endpoint at 0 {
> >> + reg = <0>;
> >> + remote-endpoint = <&vpu_out_venc_cvbs>;
> >> + };
> >> + };
> >> + };
> >> +};
> >> +
> >> +vpu: vpu at d0100000 {
> >> + compatible = "amlogic,meson-gxbb-vpu";
> >> + reg = <0x0 0xd0100000 0x0 0x100000>,
> >> + <0x0 0xc883c000 0x0 0x1000>,
> >> + <0x0 0xc8838000 0x0 0x1000>;
> >> + reg-names = "base", "hhi", "dmc";
> >> + interrupts = <GIC_SPI 3 IRQ_TYPE_EDGE_RISING>;
> >> +
> >> + ports {
> >> + #address-cells = <1>;
> >> + #size-cells = <0>;
> >> +
> >> + vpu_out: port at 1 {
> >> + #address-cells = <1>;
> >> + #size-cells = <0>;
> >> + reg = <1>;
> >> +
> >> + vpu_out_venc_cvbs: endpoint at 0 {
> >> + reg = <0>;
> >> + remote-endpoint = <&venc_cvbs_in_vpu>;
> >> + };
> >> + };
> >> + };
> >> +};
>
> Thanks for the review !
--
Regards,
Laurent Pinchart
^ permalink raw reply
* [RFC PATCH 1/3] drm: Add support for Amlogic Meson Graphic Controller
From: Neil Armstrong @ 2016-11-28 9:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161128081601.wapare3pyzmzghvp@phenom.ffwll.local>
Hi Daniel,
On 11/28/2016 09:16 AM, Daniel Vetter wrote:
> On Fri, Nov 25, 2016 at 05:03:09PM +0100, Neil Armstrong wrote:
>> The Amlogic Meson Display controller is composed of several components :
>>
>> DMC|---------------VPU (Video Processing Unit)----------------|------HHI------|
>> | vd1 _______ _____________ _________________ | |
>> D |-------| |----| | | | | HDMI PLL |
>> D | vd2 | VIU | | Video Post | | Video Encoders |<---|-----VCLK |
>> R |-------| |----| Processing | | | | |
>> | osd2 | | | |---| Enci ----------|----|-----VDAC------|
>> R |-------| CSC |----| Scalers | | Encp ----------|----|----HDMI-TX----|
>> A | osd1 | | | Blenders | | Encl ----------|----|---------------|
>> M |-------|______|----|____________| |________________| | |
>> ___|__________________________________________________________|_______________|
>>
>>
>> VIU: Video Input Unit
>> ---------------------
>>
>> The Video Input Unit is in charge of the pixel scanout from the DDR memory.
>> It fetches the frames addresses, stride and parameters from the "Canvas" memory.
>> This part is also in charge of the CSC (Colorspace Conversion).
>> It can handle 2 OSD Planes and 2 Video Planes.
>>
>> VPP: Video Processing Unit
>> --------------------------
>>
>> The Video Processing Unit is in charge if the scaling and blending of the
>> various planes into a single pixel stream.
>> There is a special "pre-blending" used by the video planes with a dedicated
>> scaler and a "post-blending" to merge with the OSD Planes.
>> The OSD planes also have a dedicated scaler for one of the OSD.
>>
>> VENC: Video Encoders
>> --------------------
>>
>> The VENC is composed of the multiple pixel encoders :
>> - ENCI : Interlace Video encoder for CVBS and Interlace HDMI
>> - ENCP : Progressive Video Encoder for HDMI
>> - ENCL : LCD LVDS Encoder
>> The VENC Unit gets a Pixel Clocks (VCLK) from a dedicated HDMI PLL and clock
>> tree and provides the scanout clock to the VPP and VIU.
>> The ENCI is connected to a single VDAC for Composite Output.
>> The ENCI and ENCP are connected to an on-chip HDMI Transceiver.
>>
>> This driver is a DRM/KMS driver using the following DRM components :
>> - GEM-CMA
>> - PRIME-CMA
>> - Atomic Modesetting
>> - FBDev-CMA
>>
>> For the following SoCs :
>> - GXBB Family (S905)
>> - GXL Family (S905X, S905D)
>> - GXM Family (S912)
>>
>> The current driver only supports the CVBS PAL/NTSC output modes, but the
>> CRTC/Planes management should support bigger modes.
>> But Advanced Colorspace Conversion, Scaling and HDMI Modes will be added in
>> a second time.
>>
>> The Device Tree bindings makes use of the endpoints video interface definitions
>> to connect to the optional CVBS and in the future the HDMI Connector nodes.
>>
>> HDMI Support is planned for a next release.
>>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>
> Few small comments below, but looks reasonable overall. Once you have acks
> for the DT part pls submit the entire series as a pull request to Dave
> Airlie (with an additional patch to add a MAINTAINERS entry).
Thanks for the review.
Ok, will add the MAINTAINERS entry.
>
> Cheers, Daniel
>
>> ---
>> drivers/gpu/drm/Kconfig | 2 +
>> drivers/gpu/drm/Makefile | 1 +
>> drivers/gpu/drm/meson/Kconfig | 8 +
>> drivers/gpu/drm/meson/Makefile | 5 +
>> drivers/gpu/drm/meson/meson_canvas.c | 96 +++
>> drivers/gpu/drm/meson/meson_canvas.h | 31 +
>> drivers/gpu/drm/meson/meson_crtc.c | 176 ++++
>> drivers/gpu/drm/meson/meson_crtc.h | 34 +
>> drivers/gpu/drm/meson/meson_cvbs.c | 293 +++++++
>> drivers/gpu/drm/meson/meson_cvbs.h | 32 +
>> drivers/gpu/drm/meson/meson_drv.c | 383 +++++++++
>> drivers/gpu/drm/meson/meson_drv.h | 68 ++
>> drivers/gpu/drm/meson/meson_plane.c | 150 ++++
>> drivers/gpu/drm/meson/meson_plane.h | 32 +
>> drivers/gpu/drm/meson/meson_registers.h | 1395 +++++++++++++++++++++++++++++++
>> drivers/gpu/drm/meson/meson_vclk.c | 169 ++++
>> drivers/gpu/drm/meson/meson_vclk.h | 36 +
>> drivers/gpu/drm/meson/meson_venc.c | 286 +++++++
>> drivers/gpu/drm/meson/meson_venc.h | 77 ++
>> drivers/gpu/drm/meson/meson_viu.c | 497 +++++++++++
>> drivers/gpu/drm/meson/meson_viu.h | 37 +
>> drivers/gpu/drm/meson/meson_vpp.c | 189 +++++
>> drivers/gpu/drm/meson/meson_vpp.h | 43 +
>> 23 files changed, 4040 insertions(+)
>> create mode 100644 drivers/gpu/drm/meson/Kconfig
>> create mode 100644 drivers/gpu/drm/meson/Makefile
>> create mode 100644 drivers/gpu/drm/meson/meson_canvas.c
>> create mode 100644 drivers/gpu/drm/meson/meson_canvas.h
>> create mode 100644 drivers/gpu/drm/meson/meson_crtc.c
>> create mode 100644 drivers/gpu/drm/meson/meson_crtc.h
>> create mode 100644 drivers/gpu/drm/meson/meson_cvbs.c
>> create mode 100644 drivers/gpu/drm/meson/meson_cvbs.h
>> create mode 100644 drivers/gpu/drm/meson/meson_drv.c
>> create mode 100644 drivers/gpu/drm/meson/meson_drv.h
>> create mode 100644 drivers/gpu/drm/meson/meson_plane.c
>> create mode 100644 drivers/gpu/drm/meson/meson_plane.h
>> create mode 100644 drivers/gpu/drm/meson/meson_registers.h
>> create mode 100644 drivers/gpu/drm/meson/meson_vclk.c
>> create mode 100644 drivers/gpu/drm/meson/meson_vclk.h
>> create mode 100644 drivers/gpu/drm/meson/meson_venc.c
>> create mode 100644 drivers/gpu/drm/meson/meson_venc.h
>> create mode 100644 drivers/gpu/drm/meson/meson_viu.c
>> create mode 100644 drivers/gpu/drm/meson/meson_viu.h
>> create mode 100644 drivers/gpu/drm/meson/meson_vpp.c
>> create mode 100644 drivers/gpu/drm/meson/meson_vpp.h
>>
[...]
>> +
>> +static void meson_crtc_atomic_begin(struct drm_crtc *crtc,
>> + struct drm_crtc_state *state)
>> +{
>> + struct meson_crtc *meson_crtc = to_meson_crtc(crtc);
>> + unsigned long flags;
>> +
>> + if (crtc->state->event) {
>> + WARN_ON(drm_crtc_vblank_get(crtc) != 0);
>> +
>> + spin_lock_irqsave(&crtc->dev->event_lock, flags);
>> + meson_crtc->event = crtc->state->event;
>> + spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
>> + crtc->state->event = NULL;
>
> If you set this to NULL here
>> + }
>> +}
>> +
>> +static void meson_crtc_atomic_flush(struct drm_crtc *crtc,
>> + struct drm_crtc_state *old_crtc_state)
>> +{
>> + struct meson_crtc *meson_crtc = to_meson_crtc(crtc);
>> + struct drm_pending_vblank_event *event = crtc->state->event;
>> +
>> + if (meson_crtc->priv->viu.osd1_enabled)
>> + meson_crtc->priv->viu.osd1_commit = true;
>> +
>> + if (event) {
>> + crtc->state->event = NULL;
>> +
>> + spin_lock_irq(&crtc->dev->event_lock);
>> + if (drm_crtc_vblank_get(crtc) == 0)
>> + drm_crtc_arm_vblank_event(crtc, event);
>> + else
>> + drm_crtc_send_vblank_event(crtc, event);
>> + spin_unlock_irq(&crtc->dev->event_lock);
>> + }
>
> This here becomes dead code. And indeed it is, since you have your own
> special crtc/vblank irq handling code right below. Please remove to avoid
> confusion.
Ok, will clarify.
>
>> +}
>> +
>> +static const struct drm_crtc_helper_funcs meson_crtc_helper_funcs = {
>> + .enable = meson_crtc_enable,
>> + .disable = meson_crtc_disable,
>> + .atomic_begin = meson_crtc_atomic_begin,
>> + .atomic_flush = meson_crtc_atomic_flush,
>> +};
>> +
>> +void meson_crtc_irq(struct meson_drm *priv)
>> +{
>> + struct meson_crtc *meson_crtc = to_meson_crtc(priv->crtc);
>> + unsigned long flags;
>> +
>> + meson_viu_sync_osd1(priv);
>> +
>> + drm_crtc_handle_vblank(priv->crtc);
>> +
>> + spin_lock_irqsave(&priv->drm->event_lock, flags);
>> + if (meson_crtc->event) {
>> + drm_crtc_send_vblank_event(priv->crtc, meson_crtc->event);
>> + drm_crtc_vblank_put(priv->crtc);
>> + meson_crtc->event = NULL;
>> + }
>> + spin_unlock_irqrestore(&priv->drm->event_lock, flags);
>> +}
>> +
[...]
>> +
>> +static int meson_cvbs_encoder_atomic_check(struct drm_encoder *encoder,
>> + struct drm_crtc_state *crtc_state,
>> + struct drm_connector_state *conn_state)
>> +{
>> + return 0;
>> +}
>
> Dummy atomic_check isn't needed, pls remove.
OK, with your following comments, will replace with a proper mode check here.
>> +
>> +static void meson_cvbs_encoder_disable(struct drm_encoder *encoder)
>> +{
>> + struct meson_cvbs *meson_cvbs = encoder_to_meson_cvbs(encoder);
>> +
>> + meson_venci_cvbs_disable(meson_cvbs->priv);
>> +}
>> +
>> +static void meson_cvbs_encoder_enable(struct drm_encoder *encoder)
>> +{
>> + struct meson_cvbs *meson_cvbs = encoder_to_meson_cvbs(encoder);
>> +
>> + meson_venci_cvbs_enable(meson_cvbs->priv);
>> +}
>
> Personally I'd remove the indirection above, more direct code is easier to
> read.
I understand, I'll maybe change the meson_venci_cvbs_XXable to be directly added to the ops.
I want to keep the registers setup in separate files and keep a clean DRM/HW separation.
>> +
>> +static void meson_cvbs_encoder_mode_set(struct drm_encoder *encoder,
>> + struct drm_display_mode *mode,
>> + struct drm_display_mode *adjusted_mode)
>> +{
>> + struct meson_cvbs *meson_cvbs = encoder_to_meson_cvbs(encoder);
>> + int i;
>> +
>> + drm_mode_debug_printmodeline(mode);
>> +
>> + for (i = 0; i < ARRAY_SIZE(meson_cvbs_modes); ++i) {
>> + struct meson_cvbs_mode *meson_mode = &meson_cvbs_modes[i];
>> +
>> + if (drm_mode_equal(mode, &meson_mode->mode)) {
>> + meson_cvbs->mode = meson_mode;
>> +
>> + meson_venci_cvbs_mode_set(meson_cvbs->priv,
>> + meson_mode->enci);
>> + break;
>> + }
>> + }
>> +}
>
> What happens if userspace sets a mode you don't have? I guess you do need
> a real atomic_check, so you can return -EINVAL if that's the case ;-)
Will add a proper atomic_check !
>
>> +
>> +static const struct drm_encoder_helper_funcs meson_cvbs_encoder_helper_funcs = {
>> + .atomic_check = meson_cvbs_encoder_atomic_check,
>> + .disable = meson_cvbs_encoder_disable,
>> + .enable = meson_cvbs_encoder_enable,
>> + .mode_set = meson_cvbs_encoder_mode_set,
>> +};
>> +
>> +/* Connector */
>> +
>> +static void meson_cvbs_connector_destroy(struct drm_connector *connector)
>> +{
>> + drm_connector_cleanup(connector);
>> +}
>> +
>> +static enum drm_connector_status
>> +meson_cvbs_connector_detect(struct drm_connector *connector, bool force)
>> +{
>
> FIXME: Implement load-detect?
Actually it's not implemented anywhere on Amlogic SoCs, will add a FIXME comment here !
>
>> + return connector_status_connected;
>> +}
>> +
>> +static int meson_cvbs_connector_get_modes(struct drm_connector *connector)
>> +{
>> + struct drm_device *dev = connector->dev;
>> + struct drm_display_mode *mode;
>> + int i;
>> +
>> + for (i = 0; i < ARRAY_SIZE(meson_cvbs_modes); ++i) {
>> + struct meson_cvbs_mode *meson_mode = &meson_cvbs_modes[i];
>> +
>> + mode = drm_mode_duplicate(dev, &meson_mode->mode);
>> + if (!mode) {
>> + DRM_ERROR("Failed to create a new display mode\n");
>> + return 0;
>> + }
>> +
>> + drm_mode_probed_add(connector, mode);
>> + }
>> +
>> + return i;
>> +}
>> +
>> +static int meson_cvbs_connector_mode_valid(struct drm_connector *connector,
>> + struct drm_display_mode *mode)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < ARRAY_SIZE(meson_cvbs_modes); ++i) {
>> + struct meson_cvbs_mode *meson_mode = &meson_cvbs_modes[i];
>> +
>> + if (drm_mode_equal(mode, &meson_mode->mode))
>> + return MODE_OK;
>> + }
>> +
>> + return MODE_BAD;
>> +}
>
> Ok, this is confusion, but I thought the docs explain this. mode_valid is
> only to validate the modes added in get_modes. This is useful for outputs
> which add modes from an EDID, but not in this case. Having a mode_valid
> unfortunately doesn't ensure that these modes will be rejected in a
> modeset, for that you need a separate mode_fixup or atomic_check on the
> encoder.
>
> It's a bit a long-standing issue, but not entirely non-trivial to fix up:
> In the general case the atomic_check is for a specific configuration,
> whereaas mode_valid must only filter modes that won't work in any
> configuration. For display blocks with lots of shared resources there's a
> big difference between the two.
>
> Please double-check the kerneldoc for all these hooks, and if this is not
> clearly enough explained for you then pls raise this (or even better,
> submit at patch).
OK, for now it seems quite clear, but I clearly missed the atomic_check case.
>
>> +
>> +static const struct drm_connector_funcs meson_cvbs_connector_funcs = {
>> + .dpms = drm_atomic_helper_connector_dpms,
>> + .detect = meson_cvbs_connector_detect,
>> + .fill_modes = drm_helper_probe_single_connector_modes,
>> + .destroy = meson_cvbs_connector_destroy,
>> + .reset = drm_atomic_helper_connector_reset,
>> + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
>> + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>> +};
>> +
[...]
>> +
>> +static int meson_drv_bind(struct device *dev)
>> +{
>> + struct platform_device *pdev = to_platform_device(dev);
>> + struct meson_drm *priv;
>> + struct drm_device *drm;
>> + struct resource *res;
>> + void __iomem *regs;
>> + int ret;
>> +
>> + drm = drm_dev_alloc(&meson_driver, dev);
>> + if (IS_ERR(drm))
>> + return PTR_ERR(drm);
>> +
>> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>> + if (!priv) {
>> + ret = -ENOMEM;
>> + goto free_drm;
>> + }
>> + drm->dev_private = priv;
>> + priv->drm = drm;
>> + priv->dev = dev;
>> +
>> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "base");
>> + regs = devm_ioremap_resource(dev, res);
>> + if (IS_ERR(regs))
>> + return PTR_ERR(regs);
>> +
>> + priv->io_base = regs;
>> +
>> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "hhi");
>> + /* Simply ioremap since it may be a shared register zone */
>> + regs = devm_ioremap(dev, res->start, resource_size(res));
>> + if (!regs)
>> + return -EADDRNOTAVAIL;
>> +
>> + priv->hhi = devm_regmap_init_mmio(dev, regs,
>> + &meson_regmap_config);
>> + if (IS_ERR(priv->hhi)) {
>> + dev_err(&pdev->dev, "Couldn't create the HHI regmap\n");
>> + return PTR_ERR(priv->hhi);
>> + }
>> +
>> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dmc");
>> + /* Simply ioremap since it may be a shared register zone */
>> + regs = devm_ioremap(dev, res->start, resource_size(res));
>> + if (!regs)
>> + return -EADDRNOTAVAIL;
>> +
>> + priv->dmc = devm_regmap_init_mmio(dev, regs,
>> + &meson_regmap_config);
>> + if (IS_ERR(priv->dmc)) {
>> + dev_err(&pdev->dev, "Couldn't create the DMC regmap\n");
>> + return PTR_ERR(priv->dmc);
>> + }
>> +
>> + priv->vsync_irq = platform_get_irq(pdev, 0);
>> +
>> + /* Hardware Initialization */
>> +
>> + meson_vpp_init(priv);
>> + meson_viu_init(priv);
>> + meson_venc_init(priv);
>> +
>> + drm_vblank_init(drm, 1);
>> + drm_mode_config_init(drm);
>> +
>> + /* Components Initialization */
>> +
>> + ret = component_bind_all(drm->dev, drm);
>> + if (ret) {
>> + dev_err(drm->dev, "Couldn't bind all components\n");
>> + goto free_drm;
>> + }
>> +
>> + ret = meson_plane_create(priv);
>> + if (ret)
>> + goto free_drm;
>> +
>> + ret = meson_crtc_create(priv);
>> + if (ret)
>> + goto free_drm;
>> +
>> + ret = drm_irq_install(drm, priv->vsync_irq);
>> + if (ret)
>> + goto free_drm;
>> +
>> + drm_mode_config_reset(drm);
>> + drm->mode_config.max_width = 8192;
>> + drm->mode_config.max_height = 8192;
>> + drm->mode_config.funcs = &meson_mode_config_funcs;
>> +
>> + priv->fbdev = drm_fbdev_cma_init(drm, 32,
>> + drm->mode_config.num_crtc,
>> + drm->mode_config.num_connector);
>> + if (IS_ERR(priv->fbdev)) {
>> + ret = PTR_ERR(priv->fbdev);
>> + goto free_drm;
>> + }
>> +
>> + drm_kms_helper_poll_init(drm);
>> +
>> + ret = drm_dev_register(drm, 0);
>> + if (ret)
>> + goto free_drm;
>> +
>> + platform_set_drvdata(pdev, priv);
>
> You need this before the drm_dev_register call I think.
Would be cleaner indeed.
>> +
>> + return 0;
>> +
>> +free_drm:
>> + drm_dev_unref(drm);
>> +
>> + return ret;
>> +}
>> +
[...]
>> +
>> +static int meson_plane_atomic_check(struct drm_plane *plane,
>> + struct drm_plane_state *state)
>> +{
>> + struct drm_rect src = {
>> + .x1 = state->src_x,
>> + .y1 = state->src_y,
>> + .x2 = state->src_x + state->src_w,
>> + .y2 = state->src_y + state->src_h,
>> + };
>> + struct drm_rect dest = {
>> + .x1 = state->crtc_x,
>> + .y1 = state->crtc_y,
>> + .x2 = state->crtc_x + state->crtc_w,
>> + .y2 = state->crtc_y + state->crtc_h,
>> + };
>> +
>> + if (state->fb) {
>> + int ret;
>> +
>> + ret = drm_rect_calc_hscale(&src, &dest,
>> + DRM_PLANE_HELPER_NO_SCALING,
>> + DRM_PLANE_HELPER_NO_SCALING);
>> + if (ret < 0)
>> + return ret;
>> +
>> + ret = drm_rect_calc_vscale(&src, &dest,
>> + DRM_PLANE_HELPER_NO_SCALING,
>> + DRM_PLANE_HELPER_NO_SCALING);
>> + if (ret < 0)
>> + return ret;
>> + }
>
> drm_plane_helper_check_state gives you the above in less code.
Ok
>
>> +
>> + return 0;
>> +}
>> +
>> +static void meson_plane_atomic_update(struct drm_plane *plane,
>> + struct drm_plane_state *old_state)
>> +{
>> + struct meson_plane *meson_plane = to_meson_plane(plane);
>> +
>> + /*
>> + * Update Coordinates
>> + * Update Formats
>> + * Update Buffer
>> + * Enable Plane
>> + */
>> + meson_viu_update_osd1(meson_plane->priv, plane);
>> + meson_canvas_update_osd1_buffer(meson_plane->priv, plane);
>> +}
>> +
[...]
>> +
>> +void meson_viu_update_osd1(struct meson_drm *priv, struct drm_plane *plane)
>> +{
>> + struct drm_plane_state *state = plane->state;
>> + struct drm_framebuffer *fb = state->fb;
>> + struct drm_rect src = {
>> + .x1 = (state->src_x),
>> + .y1 = (state->src_y),
>> + .x2 = (state->src_x + state->src_w),
>> + .y2 = (state->src_y + state->src_h),
>> + };
>> + struct drm_rect dest = {
>> + .x1 = state->crtc_x,
>> + .y1 = state->crtc_y,
>> + .x2 = state->crtc_x + state->crtc_w,
>> + .y2 = state->crtc_y + state->crtc_h,
>> + };
>> + unsigned long flags;
>> +
>> + spin_lock_irqsave(&priv->drm->event_lock, flags);
>> +
>> + /* Enable OSD and BLK0, set max global alpha */
>> + priv->viu.osd1_ctrl_stat = OSD_ENABLE |
>> + (0xFF << OSD_GLOBAL_ALPHA_SHIFT) |
>> + OSD_BLK0_ENABLE;
>> +
>> + /* Set up BLK0 to point to the right canvas */
>> + priv->viu.osd1_blk0_cfg[0] = ((MESON_CANVAS_ID_OSD1 << OSD_CANVAS_SEL) |
>> + OSD_ENDIANNESS_LE);
>> +
>> + /* On GXBB, Use the old non-HDR RGB2YUV converter */
>> + if (meson_vpu_is_compatible(priv, "amlogic,meson-gxbb-vpu"))
>> + priv->viu.osd1_blk0_cfg[0] |= OSD_OUTPUT_COLOR_RGB;
>> +
>> + switch (fb->pixel_format) {
>> + case DRM_FORMAT_XRGB8888:
>> + /* For XRGB, replace the pixel's alpha by 0xFF */
>> + writel_bits_relaxed(OSD_REPLACE_EN, OSD_REPLACE_EN,
>> + priv->io_base + _REG(VIU_OSD1_CTRL_STAT2));
>> + priv->viu.osd1_blk0_cfg[0] |= OSD_BLK_MODE_32 |
>> + OSD_COLOR_MATRIX_32_ARGB;
>> + break;
>> + case DRM_FORMAT_ARGB8888:
>> + /* For ARGB, use the pixel's alpha */
>> + writel_bits_relaxed(OSD_REPLACE_EN, 0,
>> + priv->io_base + _REG(VIU_OSD1_CTRL_STAT2));
>> + priv->viu.osd1_blk0_cfg[0] |= OSD_BLK_MODE_32 |
>> + OSD_COLOR_MATRIX_32_ARGB;
>> + break;
>> + case DRM_FORMAT_RGB888:
>> + priv->viu.osd1_blk0_cfg[0] |= OSD_BLK_MODE_24 |
>> + OSD_COLOR_MATRIX_24_RGB;
>> + break;
>> + case DRM_FORMAT_RGB565:
>> + priv->viu.osd1_blk0_cfg[0] |= OSD_BLK_MODE_16 |
>> + OSD_COLOR_MATRIX_16_RGB565;
>> + break;
>> + };
>> +
>> + if (state->crtc->mode.flags & DRM_MODE_FLAG_INTERLACE) {
>> + priv->viu.osd1_interlace = true;
>> +
>> + dest.y1 /= 2;
>> + dest.y2 /= 2;
>> + } else {
>> + priv->viu.osd1_interlace = true;
>> + meson_vpp_disable_interlace_vscaler_osd1(priv);
>> + }
>> +
>> + /*
>> + * The format of these registers is (x2 << 16 | x1),
>> + * where x2 is exclusive.
>> + * e.g. +30x1920 would be (1919 << 16) | 30
>> + */
>> + priv->viu.osd1_blk0_cfg[1] = ((fixed16_to_int(src.x2) - 1) << 16) |
>> + fixed16_to_int(src.x1);
>> + priv->viu.osd1_blk0_cfg[2] = ((fixed16_to_int(src.y2) - 1) << 16) |
>> + fixed16_to_int(src.y1);
>> + priv->viu.osd1_blk0_cfg[3] = ((dest.x2 - 1) << 16) | dest.x1;
>> + priv->viu.osd1_blk0_cfg[4] = ((dest.y2 - 1) << 16) | dest.y1;
>> +
>> + spin_unlock_irqrestore(&priv->drm->event_lock, flags);
>> +}
>> +
>> +void meson_viu_sync_osd1(struct meson_drm *priv)
>> +{
>> + /* Update the OSD registers */
>> + if (priv->viu.osd1_enabled && priv->viu.osd1_commit) {
>> + writel_relaxed(priv->viu.osd1_ctrl_stat,
>> + priv->io_base + _REG(VIU_OSD1_CTRL_STAT));
>> + writel_relaxed(priv->viu.osd1_blk0_cfg[0],
>> + priv->io_base + _REG(VIU_OSD1_BLK0_CFG_W0));
>> + writel_relaxed(priv->viu.osd1_blk0_cfg[1],
>> + priv->io_base + _REG(VIU_OSD1_BLK0_CFG_W1));
>> + writel_relaxed(priv->viu.osd1_blk0_cfg[2],
>> + priv->io_base + _REG(VIU_OSD1_BLK0_CFG_W2));
>> + writel_relaxed(priv->viu.osd1_blk0_cfg[3],
>> + priv->io_base + _REG(VIU_OSD1_BLK0_CFG_W3));
>> + writel_relaxed(priv->viu.osd1_blk0_cfg[4],
>> + priv->io_base + _REG(VIU_OSD1_BLK0_CFG_W4));
>> +
>> + if (priv->viu.osd1_interlace) {
>> + struct drm_plane *plane = priv->primary_plane;
>> + struct drm_plane_state *state = plane->state;
>> + struct drm_rect dest = {
>> + .x1 = state->crtc_x,
>> + .y1 = state->crtc_y,
>> + .x2 = state->crtc_x + state->crtc_w,
>> + .y2 = state->crtc_y + state->crtc_h,
>> + };
>> +
>> + meson_vpp_setup_interlace_vscaler_osd1(priv, &dest);
>> + }
>> +
>> + meson_vpp_enable_osd1(priv);
>> +
>> + priv->viu.osd1_commit = false;
>> + }
>> +}
>
> Again I'd remove the indirection and for these put them into your plane
> implementation directly.
OK, I'll make them callable from the DRM ops directly.
>
>> +
>> +
>> +/* OSD csc defines */
>> +
>> +enum viu_matrix_sel_e {
>> + VIU_MATRIX_OSD_EOTF = 0,
>> + VIU_MATRIX_OSD,
>> +};
>> +
>> +enum viu_lut_sel_e {
>> + VIU_LUT_OSD_EOTF = 0,
>> + VIU_LUT_OSD_OETF,
>> +};
>> +
>> +#define COEFF_NORM(a) ((int)((((a) * 2048.0) + 1) / 2))
>> +#define MATRIX_5X3_COEF_SIZE 24
>> +
>> +#define EOTF_COEFF_NORM(a) ((int)((((a) * 4096.0) + 1) / 2))
>> +#define EOTF_COEFF_SIZE 10
>> +#define EOTF_COEFF_RIGHTSHIFT 1
>> +
>> +static int RGB709_to_YUV709l_coeff[MATRIX_5X3_COEF_SIZE] = {
>> + 0, 0, 0, /* pre offset */
>> + COEFF_NORM(0.181873), COEFF_NORM(0.611831), COEFF_NORM(0.061765),
>> + COEFF_NORM(-0.100251), COEFF_NORM(-0.337249), COEFF_NORM(0.437500),
>> + COEFF_NORM(0.437500), COEFF_NORM(-0.397384), COEFF_NORM(-0.040116),
>> + 0, 0, 0, /* 10'/11'/12' */
>> + 0, 0, 0, /* 20'/21'/22' */
>> + 64, 512, 512, /* offset */
>> + 0, 0, 0 /* mode, right_shift, clip_en */
>> +};
>> +
>> +/* eotf matrix: bypass */
>> +static int eotf_bypass_coeff[EOTF_COEFF_SIZE] = {
>> + EOTF_COEFF_NORM(1.0), EOTF_COEFF_NORM(0.0), EOTF_COEFF_NORM(0.0),
>> + EOTF_COEFF_NORM(0.0), EOTF_COEFF_NORM(1.0), EOTF_COEFF_NORM(0.0),
>> + EOTF_COEFF_NORM(0.0), EOTF_COEFF_NORM(0.0), EOTF_COEFF_NORM(1.0),
>> + EOTF_COEFF_RIGHTSHIFT /* right shift */
>> +};
>> +
>> +void meson_viu_set_osd_matrix(struct meson_drm *priv,
>> + enum viu_matrix_sel_e m_select,
>> + int *m, bool csc_on)
>> +{
>> + if (m_select == VIU_MATRIX_OSD) {
>> + /* osd matrix, VIU_MATRIX_0 */
>> + writel(((m[0] & 0xfff) << 16) | (m[1] & 0xfff),
>> + priv->io_base + _REG(VIU_OSD1_MATRIX_PRE_OFFSET0_1));
>> + writel(m[2] & 0xfff,
>> + priv->io_base + _REG(VIU_OSD1_MATRIX_PRE_OFFSET2));
>> + writel(((m[3] & 0x1fff) << 16) | (m[4] & 0x1fff),
>> + priv->io_base + _REG(VIU_OSD1_MATRIX_COEF00_01));
>> + writel(((m[5] & 0x1fff) << 16) | (m[6] & 0x1fff),
>> + priv->io_base + _REG(VIU_OSD1_MATRIX_COEF02_10));
>> + writel(((m[7] & 0x1fff) << 16) | (m[8] & 0x1fff),
>> + priv->io_base + _REG(VIU_OSD1_MATRIX_COEF11_12));
>> + writel(((m[9] & 0x1fff) << 16) | (m[10] & 0x1fff),
>> + priv->io_base + _REG(VIU_OSD1_MATRIX_COEF20_21));
>> +
>> + if (m[21]) {
>> + writel(((m[11] & 0x1fff) << 16) | (m[12] & 0x1fff),
>> + priv->io_base +
>> + _REG(VIU_OSD1_MATRIX_COEF22_30));
>> + writel(((m[13] & 0x1fff) << 16) | (m[14] & 0x1fff),
>> + priv->io_base +
>> + _REG(VIU_OSD1_MATRIX_COEF31_32));
>> + writel(((m[15] & 0x1fff) << 16) | (m[16] & 0x1fff),
>> + priv->io_base +
>> + _REG(VIU_OSD1_MATRIX_COEF40_41));
>> + writel(m[17] & 0x1fff, priv->io_base +
>> + _REG(VIU_OSD1_MATRIX_COLMOD_COEF42));
>> + } else
>> + writel((m[11] & 0x1fff) << 16, priv->io_base +
>> + _REG(VIU_OSD1_MATRIX_COEF22_30));
>> +
>> + writel(((m[18] & 0xfff) << 16) | (m[19] & 0xfff),
>> + priv->io_base + _REG(VIU_OSD1_MATRIX_OFFSET0_1));
>> + writel(m[20] & 0xfff,
>> + priv->io_base + _REG(VIU_OSD1_MATRIX_OFFSET2));
>> +
>> + writel_bits_relaxed(3 << 30, m[21] << 30,
>> + priv->io_base + _REG(VIU_OSD1_MATRIX_COLMOD_COEF42));
>> + writel_bits_relaxed(7 << 16, m[22] << 16,
>> + priv->io_base + _REG(VIU_OSD1_MATRIX_COLMOD_COEF42));
>> +
>> + /* 23 reserved for clipping control */
>> + writel_bits_relaxed(BIT(0), csc_on ? BIT(0) : 0,
>> + priv->io_base + _REG(VIU_OSD1_MATRIX_CTRL));
>> + writel_bits_relaxed(BIT(1), 0,
>> + priv->io_base + _REG(VIU_OSD1_MATRIX_CTRL));
>> + } else if (m_select == VIU_MATRIX_OSD_EOTF) {
>> + int i;
>> +
>> + /* osd eotf matrix, VIU_MATRIX_OSD_EOTF */
>> + for (i = 0; i < 5; i++)
>> + writel(((m[i * 2] & 0x1fff) << 16) |
>> + (m[i * 2 + 1] & 0x1fff), priv->io_base +
>> + _REG(VIU_OSD1_EOTF_CTL + i + 1));
>> +
>> + writel_bits_relaxed(BIT(30), csc_on ? BIT(30) : 0,
>> + priv->io_base + _REG(VIU_OSD1_EOTF_CTL));
>> + writel_bits_relaxed(BIT(31), csc_on ? BIT(31) : 0,
>> + priv->io_base + _REG(VIU_OSD1_EOTF_CTL));
>> + }
>> +}
>> +
>> +#define OSD_EOTF_LUT_SIZE 33
>> +#define OSD_OETF_LUT_SIZE 41
>> +
>> +void meson_viu_set_osd_lut(struct meson_drm *priv, enum viu_lut_sel_e lut_sel,
>> + unsigned int *r_map, unsigned int *g_map,
>> + unsigned int *b_map,
>> + bool csc_on)
>> +{
>> + unsigned int addr_port;
>> + unsigned int data_port;
>> + unsigned int ctrl_port;
>> + int i;
>> +
>> + if (lut_sel == VIU_LUT_OSD_EOTF) {
>> + addr_port = VIU_OSD1_EOTF_LUT_ADDR_PORT;
>> + data_port = VIU_OSD1_EOTF_LUT_DATA_PORT;
>> + ctrl_port = VIU_OSD1_EOTF_CTL;
>> + } else if (lut_sel == VIU_LUT_OSD_OETF) {
>> + addr_port = VIU_OSD1_OETF_LUT_ADDR_PORT;
>> + data_port = VIU_OSD1_OETF_LUT_DATA_PORT;
>> + ctrl_port = VIU_OSD1_OETF_CTL;
>> + } else
>> + return;
>> +
>> + if (lut_sel == VIU_LUT_OSD_OETF) {
>> + writel(0, priv->io_base + _REG(addr_port));
>> +
>> + for (i = 0; i < 20; i++)
>> + writel(r_map[i * 2] | (r_map[i * 2 + 1] << 16),
>> + priv->io_base + _REG(data_port));
>> +
>> + writel(r_map[OSD_OETF_LUT_SIZE - 1] | (g_map[0] << 16),
>> + priv->io_base + _REG(data_port));
>> +
>> + for (i = 0; i < 20; i++)
>> + writel(g_map[i * 2 + 1] | (g_map[i * 2 + 2] << 16),
>> + priv->io_base + _REG(data_port));
>> +
>> + for (i = 0; i < 20; i++)
>> + writel(b_map[i * 2] | (b_map[i * 2 + 1] << 16),
>> + priv->io_base + _REG(data_port));
>> +
>> + writel(b_map[OSD_OETF_LUT_SIZE - 1],
>> + priv->io_base + _REG(data_port));
>> +
>> + if (csc_on)
>> + writel_bits_relaxed(0x7 << 29, 7 << 29,
>> + priv->io_base + _REG(ctrl_port));
>> + else
>> + writel_bits_relaxed(0x7 << 29, 0,
>> + priv->io_base + _REG(ctrl_port));
>> + } else if (lut_sel == VIU_LUT_OSD_EOTF) {
>> + writel(0, priv->io_base + _REG(addr_port));
>> +
>> + for (i = 0; i < 20; i++)
>> + writel(r_map[i * 2] | (r_map[i * 2 + 1] << 16),
>> + priv->io_base + _REG(data_port));
>> +
>> + writel(r_map[OSD_EOTF_LUT_SIZE - 1] | (g_map[0] << 16),
>> + priv->io_base + _REG(data_port));
>> +
>> + for (i = 0; i < 20; i++)
>> + writel(g_map[i * 2 + 1] | (g_map[i * 2 + 2] << 16),
>> + priv->io_base + _REG(data_port));
>> +
>> + for (i = 0; i < 20; i++)
>> + writel(b_map[i * 2] | (b_map[i * 2 + 1] << 16),
>> + priv->io_base + _REG(data_port));
>> +
>> + writel(b_map[OSD_EOTF_LUT_SIZE - 1],
>> + priv->io_base + _REG(data_port));
>> +
>> + if (csc_on)
>> + writel_bits_relaxed(7 << 27, 7 << 27,
>> + priv->io_base + _REG(ctrl_port));
>> + else
>> + writel_bits_relaxed(7 << 27, 0,
>> + priv->io_base + _REG(ctrl_port));
>> +
>> + writel_bits_relaxed(BIT(31), BIT(31),
>> + priv->io_base + _REG(ctrl_port));
>> + }
>> +}
>> +
>> +/* eotf lut: linear */
>> +static unsigned int eotf_33_linear_mapping[OSD_EOTF_LUT_SIZE] = {
>> + 0x0000, 0x0200, 0x0400, 0x0600,
>> + 0x0800, 0x0a00, 0x0c00, 0x0e00,
>> + 0x1000, 0x1200, 0x1400, 0x1600,
>> + 0x1800, 0x1a00, 0x1c00, 0x1e00,
>> + 0x2000, 0x2200, 0x2400, 0x2600,
>> + 0x2800, 0x2a00, 0x2c00, 0x2e00,
>> + 0x3000, 0x3200, 0x3400, 0x3600,
>> + 0x3800, 0x3a00, 0x3c00, 0x3e00,
>> + 0x4000
>> +};
>> +
>> +/* osd oetf lut: linear */
>> +static unsigned int oetf_41_linear_mapping[OSD_OETF_LUT_SIZE] = {
>> + 0, 0, 0, 0,
>> + 0, 32, 64, 96,
>> + 128, 160, 196, 224,
>> + 256, 288, 320, 352,
>> + 384, 416, 448, 480,
>> + 512, 544, 576, 608,
>> + 640, 672, 704, 736,
>> + 768, 800, 832, 864,
>> + 896, 928, 960, 992,
>> + 1023, 1023, 1023, 1023,
>> + 1023
>> +};
>
> Might be fun to expose this using the color manager stuff, see
> drm_crtc_enable_color_mgmt().
Yes, I'll use them when the HDMI stuff lands ! This will certainly need such helpers !
>> +
>> +static void meson_viu_load_matrix(struct meson_drm *priv)
>> +{
>> + /* eotf lut bypass */
>> + meson_viu_set_osd_lut(priv, VIU_LUT_OSD_EOTF,
>> + eotf_33_linear_mapping, /* R */
>> + eotf_33_linear_mapping, /* G */
>> + eotf_33_linear_mapping, /* B */
>> + false);
>> +
>> + /* eotf matrix bypass */
>> + meson_viu_set_osd_matrix(priv, VIU_MATRIX_OSD_EOTF,
>> + eotf_bypass_coeff,
>> + false);
>> +
>> + /* oetf lut bypass */
>> + meson_viu_set_osd_lut(priv, VIU_LUT_OSD_OETF,
>> + oetf_41_linear_mapping, /* R */
>> + oetf_41_linear_mapping, /* G */
>> + oetf_41_linear_mapping, /* B */
>> + false);
>> +
>> + /* osd matrix RGB709 to YUV709 limit */
>> + meson_viu_set_osd_matrix(priv, VIU_MATRIX_OSD,
>> + RGB709_to_YUV709l_coeff,
>> + true);
>> +}
>> +
Neil
^ permalink raw reply
* [Qemu-devel] [kvm-unit-tests PATCH v7 06/11] arm/Makefile.common: force -fno-pic
From: Andrew Jones @ 2016-11-28 9:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161124161033.11456-7-alex.bennee@linaro.org>
On Thu, Nov 24, 2016 at 04:10:28PM +0000, Alex Benn?e wrote:
> As distro compilers move towards defaults for build hardening for things
> like ASLR we need to force -fno-pic. Failure to do can lead to weird
> relocation problems when we build our "lat" binaries.
>
> Signed-off-by: Alex Benn?e <alex.bennee@linaro.org>
> ---
> arm/Makefile.common | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arm/Makefile.common b/arm/Makefile.common
> index 52f7440..cca0d9c 100644
> --- a/arm/Makefile.common
> +++ b/arm/Makefile.common
> @@ -21,6 +21,7 @@ phys_base = $(LOADADDR)
>
> CFLAGS += -std=gnu99
> CFLAGS += -ffreestanding
> +CFLAGS += -fno-pic
> CFLAGS += -Wextra
> CFLAGS += -O2
> CFLAGS += -I lib -I lib/libfdt
> --
> 2.10.1
>
>
Applied to arm/next
https://github.com/rhdrjones/kvm-unit-tests/commits/arm/next
Thanks,
drew
^ permalink raw reply
* [PATCH v3] clkdev: add devm_of_clk_get()
From: Kuninori Morimoto @ 2016-11-28 9:32 UTC (permalink / raw)
To: linux-arm-kernel
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Current Linux has of_clk_get(), but doesn't have devm_of_clk_get().
This patch adds it.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
v2 -> v3
- implement in clk-devres.c, and reused existing devm_clk_release()
drivers/clk/clk-devres.c | 21 +++++++++++++++++++++
include/linux/clk.h | 7 +++++++
2 files changed, 28 insertions(+)
diff --git a/drivers/clk/clk-devres.c b/drivers/clk/clk-devres.c
index 8f57154..2449b25 100644
--- a/drivers/clk/clk-devres.c
+++ b/drivers/clk/clk-devres.c
@@ -53,3 +53,24 @@ void devm_clk_put(struct device *dev, struct clk *clk)
WARN_ON(ret);
}
EXPORT_SYMBOL(devm_clk_put);
+
+struct clk *devm_of_clk_get(struct device *dev,
+ struct device_node *np, int index)
+{
+ struct clk **ptr, *clk;
+
+ ptr = devres_alloc(devm_clk_release, sizeof(*ptr), GFP_KERNEL);
+ if (!ptr)
+ return ERR_PTR(-ENOMEM);
+
+ clk = of_clk_get(np, index);
+ if (!IS_ERR(clk)) {
+ *ptr = clk;
+ devres_add(dev, ptr);
+ } else {
+ devres_free(ptr);
+ }
+
+ return clk;
+}
+EXPORT_SYMBOL(devm_of_clk_get);
diff --git a/include/linux/clk.h b/include/linux/clk.h
index 123c027..1b713db 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -506,6 +506,8 @@ static inline void clk_disable_unprepare(struct clk *clk)
#if defined(CONFIG_OF) && defined(CONFIG_COMMON_CLK)
struct clk *of_clk_get(struct device_node *np, int index);
+struct clk *devm_of_clk_get(struct device *dev,
+ struct device_node *np, int index);
struct clk *of_clk_get_by_name(struct device_node *np, const char *name);
struct clk *of_clk_get_from_provider(struct of_phandle_args *clkspec);
#else
@@ -513,6 +515,11 @@ static inline struct clk *of_clk_get(struct device_node *np, int index)
{
return ERR_PTR(-ENOENT);
}
+static inline struct clk *devm_of_clk_get(struct device *dev,
+ struct device_node *np, int index)
+{
+ return ERR_PTR(-ENOENT);
+}
static inline struct clk *of_clk_get_by_name(struct device_node *np,
const char *name)
{
--
1.9.1
^ permalink raw reply related
* [RFC PATCH 3/3] dt-bindings: display: add Amlogic Meson DRM Bindings
From: Neil Armstrong @ 2016-11-28 9:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <3721857.XEGXu9B51N@avalon>
Hi Laurent,
On 11/28/2016 09:33 AM, Laurent Pinchart wrote:
> Hi Neil,
>
> Thank you for the patch.
>
> On Friday 25 Nov 2016 17:03:11 Neil Armstrong wrote:
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>> .../bindings/display/meson/meson-drm.txt | 134 +++++++++++++++++
>> 1 file changed, 134 insertions(+)
>> create mode 100644
>> Documentation/devicetree/bindings/display/meson/meson-drm.txt
>>
>> diff --git a/Documentation/devicetree/bindings/display/meson/meson-drm.txt
>> b/Documentation/devicetree/bindings/display/meson/meson-drm.txt new file
>> mode 100644
>> index 0000000..89c1b5f
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/meson/meson-drm.txt
>> @@ -0,0 +1,134 @@
>> +Amlogic Meson Display Controller
>> +================================
>> +
>> +The Amlogic Meson Display controller is composed of several components
>> +that are going to be documented below:
>> +
>> +DMC|---------------VPU (Video Processing Unit)------------|------HHI------|
>> + | vd1 _______ _____________ _____________ | |
>> +D |-------| |----| | | | | HDMI PLL |
>> +D | vd2 | VIU | | Video Post | | Video Encs |<---|-----VCLK |
>> +R |-------| |----| Processing | | | | |
>> + | osd2 | | | |---| Enci ------|----|-----VDAC------|
>> +R |-------| CSC |----| Scalers | | Encp ------|----|----HDMI-TX----|
>> +A | osd1 | | | Blenders | | Encl-------|----|---------------|
>> +M |-------|______|----|____________| |____________| | |
>> +___|______________________________________________________|_______________|
>> +
>> +
>> +VIU: Video Input Unit
>> +---------------------
>> +
>> +The Video Input Unit is in charge of the pixel scanout from the DDR memory.
>> +It fetches the frames addresses, stride and parameters from the "Canvas"
>> memory.
>> +This part is also in charge of the CSC (Colorspace Conversion).
>> +It can handle 2 OSD Planes and 2 Video Planes.
>> +
>> +VPP: Video Processing Unit
>
> Do you mean "Video Post Processing" ? In your diagram above Video Processing
> Unit is abbreviated VPU and covers the VIU, VPP and encoders.
Exact, I meant VPP here.
>
>> +--------------------------
>> +
>> +The Video Processing Unit is in charge if the scaling and blending of the
>> +various planes into a single pixel stream.
>> +There is a special "pre-blending" used by the video planes with a dedicated
>> +scaler and a "post-blending" to merge with the OSD Planes.
>> +The OSD planes also have a dedicated scaler for one of the OSD.
>> +
>> +VENC: Video Encoders
>> +--------------------
>> +
>> +The VENC is composed of the multiple pixel encoders :
>> + - ENCI : Interlace Video encoder for CVBS and Interlace HDMI
>> + - ENCP : Progressive Video Encoder for HDMI
>> + - ENCL : LCD LVDS Encoder
>> +The VENC Unit gets a Pixel Clocks (VCLK) from a dedicated HDMI PLL and
>> clock
>> +tree and provides the scanout clock to the VPP and VIU.
>> +The ENCI is connected to a single VDAC for Composite Output.
>> +The ENCI and ENCP are connected to an on-chip HDMI Transceiver.
>> +
>> +Device Tree Bindings:
>> +---------------------
>> +
>> +VPU: Video Processing Unit
>> +--------------------------
>> +
>> +Required properties:
>> + - compatible: value should be different for each SoC family as :
>> + - GXBB (S905) : "amlogic,meson-gxbb-vpu"
>> + - GXL (S905X, S905D) : "amlogic,meson-gxl-vpu"
>> + - GXM (S912) : "amlogic,meson-gxm-vpu"
>> + followed by the common "amlogic,meson-gx-vpu"
>> + - reg: base address and size of he following memory-mapped regions :
>> + - vpu
>> + - hhi
>> + - dmc
>> + - reg-names: should contain the names of the previous memory regions
>> + - interrupts: should contain the VENC Vsync interrupt number
>> +
>> +- ports: A ports node with endpoint definitions as defined in
>> + Documentation/devicetree/bindings/media/video-interfaces.txt. The
>> + second port should be the output endpoints for VENC connectors.
>> +
>> +VENC CBVS Output
>> +----------------------
>> +
>> +The VENC can output Composite/CVBS output via a decicated VDAC.
>> +
>> +Required properties:
>> + - compatible: value must be one of:
>> + - compatible: value should be different for each SoC family as :
>
> One of those two lines is redundant.
Will fix.
>
>> + - GXBB (S905) : "amlogic,meson-gxbb-venc-cvbs"
>> + - GXL (S905X, S905D) : "amlogic,meson-gxl-venc-cvbs"
>> + - GXM (S912) : "amlogic,meson-gxm-venc-cvbs"
>> + followed by the common "amlogic,meson-gx-venc-cvbs"
>> +
>
> No registers ? Are the encoders registers part of the VPU register space,
> intertwined in a way that they can't be specified separately here ?
Exact, all the video registers on the Amlogic SoC are part of a long history of fixup/enhance from very old SoCs, it's
quite hard to distinguish a Venc registers array since they are mixed with the multiple encoders registers...
The only separate registers are the VDAC and HDMI PHY, I may move them to these separate nodes since they are part of the HHI register space.
It is a problem if I move them in the next release ? Next release will certainly have HDMI support, and will have these refactorings.
>
>> +- ports: A ports node with endpoint definitions as defined in
>> + Documentation/devicetree/bindings/media/video-interfaces.txt. The
>> + first port should be the input endpoints, connected ot the VPU node.
>> +
>> +Example:
>> +
>> +venc_cvbs: venc-cvbs {
>> + compatible = "amlogic,meson-gxbb-venc-cvbs";
>> + status = "okay";
>> +
>> + ports {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + enc_cvbs_in: port at 0 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + reg = <0>;
>> +
>> + venc_cvbs_in_vpu: endpoint at 0 {
>> + reg = <0>;
>> + remote-endpoint = <&vpu_out_venc_cvbs>;
>> + };
>> + };
>> + };
>> +};
>> +
>> +vpu: vpu at d0100000 {
>> + compatible = "amlogic,meson-gxbb-vpu";
>> + reg = <0x0 0xd0100000 0x0 0x100000>,
>> + <0x0 0xc883c000 0x0 0x1000>,
>> + <0x0 0xc8838000 0x0 0x1000>;
>> + reg-names = "base", "hhi", "dmc";
>> + interrupts = <GIC_SPI 3 IRQ_TYPE_EDGE_RISING>;
>> +
>> + ports {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + vpu_out: port at 1 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + reg = <1>;
>> +
>> + vpu_out_venc_cvbs: endpoint at 0 {
>> + reg = <0>;
>> + remote-endpoint = <&venc_cvbs_in_vpu>;
>> + };
>> + };
>> + };
>> +};
>
Thanks for the review !
Neil
^ permalink raw reply
* [PATCH v2] clkdev: add devm_of_clk_get()
From: Kuninori Morimoto @ 2016-11-28 9:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161128091456.GR14217@n2100.armlinux.org.uk>
Hi Russell
> > Current Linux has of_clk_get(), but doesn't have devm_of_clk_get().
> > This patch adds it. This is based on devm_clk_get()
> >
> > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
>
> Please put this in drivers/clk/clk-devres.c, where you'll find that
> we have devm_clk_release() which is identical to your
> devm_of_clk_release(). It'll also not need the dummy definition of
> devm_of_clk_get().
OK, will do
^ permalink raw reply
* [kvm-unit-tests PATCH v7 04/11] libcflat: add PRI(dux)32 format types
From: Andrew Jones @ 2016-11-28 9:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161124161033.11456-5-alex.bennee@linaro.org>
On Thu, Nov 24, 2016 at 04:10:26PM +0000, Alex Benn?e wrote:
> So we can have portable formatting of uint32_t types.
>
> Signed-off-by: Alex Benn?e <alex.bennee@linaro.org>
> ---
> lib/libcflat.h | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/lib/libcflat.h b/lib/libcflat.h
> index bdcc561..6dab5be 100644
> --- a/lib/libcflat.h
> +++ b/lib/libcflat.h
> @@ -55,12 +55,17 @@ typedef _Bool bool;
> #define true 1
>
> #if __SIZEOF_LONG__ == 8
> +# define __PRI32_PREFIX
> # define __PRI64_PREFIX "l"
> # define __PRIPTR_PREFIX "l"
> #else
> +# define __PRI32_PREFIX "l"
But a 32-bit value is an 'int' and an 'int' shouldn't ever
require an 'l'. Why was this necessary?
> # define __PRI64_PREFIX "ll"
> # define __PRIPTR_PREFIX
> #endif
> +#define PRId32 __PRI32_PREFIX "d"
> +#define PRIu32 __PRI32_PREFIX "u"
> +#define PRIx32 __PRI32_PREFIX "x"
> #define PRId64 __PRI64_PREFIX "d"
> #define PRIu64 __PRI64_PREFIX "u"
> #define PRIx64 __PRI64_PREFIX "x"
> --
> 2.10.1
>
> _______________________________________________
> kvmarm mailing list
> kvmarm at lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply
* [PATCH v2] clkdev: add devm_of_clk_get()
From: Russell King - ARM Linux @ 2016-11-28 9:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <8737icayac.wl%kuninori.morimoto.gx@renesas.com>
On Mon, Nov 28, 2016 at 06:56:52AM +0000, Kuninori Morimoto wrote:
> Current Linux has of_clk_get(), but doesn't have devm_of_clk_get().
> This patch adds it. This is based on devm_clk_get()
>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Please put this in drivers/clk/clk-devres.c, where you'll find that
we have devm_clk_release() which is identical to your
devm_of_clk_release(). It'll also not need the dummy definition of
devm_of_clk_get().
Thanks.
> ---
> v1 -> v2
>
> - update git log
>
> drivers/clk/clkdev.c | 26 ++++++++++++++++++++++++++
> include/linux/clk.h | 7 +++++++
> 2 files changed, 33 insertions(+)
>
> diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
> index 89cc700..93a613b 100644
> --- a/drivers/clk/clkdev.c
> +++ b/drivers/clk/clkdev.c
> @@ -55,6 +55,32 @@ struct clk *of_clk_get(struct device_node *np, int index)
> }
> EXPORT_SYMBOL(of_clk_get);
>
> +static void devm_of_clk_release(struct device *dev, void *res)
> +{
> + clk_put(*(struct clk **)res);
> +}
> +
> +struct clk *devm_of_clk_get(struct device *dev,
> + struct device_node *np, int index)
> +{
> + struct clk **ptr, *clk;
> +
> + ptr = devres_alloc(devm_of_clk_release, sizeof(*ptr), GFP_KERNEL);
> + if (!ptr)
> + return ERR_PTR(-ENOMEM);
> +
> + clk = of_clk_get(np, index);
> + if (!IS_ERR(clk)) {
> + *ptr = clk;
> + devres_add(dev, ptr);
> + } else {
> + devres_free(ptr);
> + }
> +
> + return clk;
> +}
> +EXPORT_SYMBOL(devm_of_clk_get);
> +
> static struct clk *__of_clk_get_by_name(struct device_node *np,
> const char *dev_id,
> const char *name)
> diff --git a/include/linux/clk.h b/include/linux/clk.h
> index a89ba4e..33cd540 100644
> --- a/include/linux/clk.h
> +++ b/include/linux/clk.h
> @@ -502,6 +502,8 @@ struct of_phandle_args;
>
> #if defined(CONFIG_OF) && defined(CONFIG_COMMON_CLK)
> struct clk *of_clk_get(struct device_node *np, int index);
> +struct clk *devm_of_clk_get(struct device *dev,
> + struct device_node *np, int index);
> struct clk *of_clk_get_by_name(struct device_node *np, const char *name);
> struct clk *of_clk_get_from_provider(struct of_phandle_args *clkspec);
> #else
> @@ -509,6 +511,11 @@ static inline struct clk *of_clk_get(struct device_node *np, int index)
> {
> return ERR_PTR(-ENOENT);
> }
> +static inline struct clk *devm_of_clk_get(struct device *dev,
> + struct device_node *np, int index)
> +{
> + return ERR_PTR(-ENOENT);
> +}
> static inline struct clk *of_clk_get_by_name(struct device_node *np,
> const char *name)
> {
> --
> 1.9.1
>
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* [Qemu-devel] [kvm-unit-tests PATCH v7 03/11] run_tests: allow passing of options to QEMU
From: Andrew Jones @ 2016-11-28 9:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161124161033.11456-4-alex.bennee@linaro.org>
On Thu, Nov 24, 2016 at 04:10:25PM +0000, Alex Benn?e wrote:
> This introduces a the option -o for passing of options directly to QEMU
> which is useful. In my case I'm using it to toggle MTTCG on an off:
>
> ./run_tests.sh -t -o "-tcg mttcg=on"
>
> Signed-off-by: Alex Benn?e <alex.bennee@linaro.org>
> ---
> run_tests.sh | 10 +++++++---
> scripts/functions.bash | 13 +++++++------
> 2 files changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/run_tests.sh b/run_tests.sh
> index 4f2e5cb..05cc7fb 100755
> --- a/run_tests.sh
> +++ b/run_tests.sh
> @@ -13,10 +13,11 @@ function usage()
> {
> cat <<EOF
>
> -Usage: $0 [-g group] [-a accel] [-t] [-h] [-v]
> +Usage: $0 [-g group] [-a accel] [-o qemu_opts] [-t] [-h] [-v]
>
> -g: Only execute tests in the given group
> -a: Force acceleration mode (tcg/kvm)
> + -o: additional options for QEMU command line
> -t: disable timeouts
> -h: Output this help text
> -v: Enables verbose mode
> @@ -30,7 +31,7 @@ EOF
> RUNTIME_arch_run="./$TEST_DIR/run"
> source scripts/runtime.bash
>
> -while getopts "g:a:thv" opt; do
> +while getopts "g:a:o:thv" opt; do
> case $opt in
> g)
> only_group=$OPTARG
> @@ -38,6 +39,9 @@ while getopts "g:a:thv" opt; do
> a)
> force_accel=$OPTARG
> ;;
> + o)
> + extra_opts=$OPTARG
> + ;;
> t)
> no_timeout="yes"
> ;;
> @@ -67,4 +71,4 @@ RUNTIME_log_stdout () {
> config=$TEST_DIR/unittests.cfg
> rm -f test.log
> printf "BUILD_HEAD=$(cat build-head)\n\n" > test.log
> -for_each_unittest $config run
> +for_each_unittest $config run "$extra_opts"
> diff --git a/scripts/functions.bash b/scripts/functions.bash
> index ee9143c..d38a69e 100644
> --- a/scripts/functions.bash
> +++ b/scripts/functions.bash
> @@ -2,11 +2,12 @@
> function for_each_unittest()
> {
> local unittests="$1"
> - local cmd="$2"
> - local testname
> + local cmd="$2"
> + local extra_opts=$3
> + local testname
We use tabs in this file. Not sure why cmd and testname got
changed too...
> local smp
> local kernel
> - local opts
> + local opts=$extra_opts
> local groups
> local arch
> local check
> @@ -21,7 +22,7 @@ function for_each_unittest()
> testname=${BASH_REMATCH[1]}
> smp=1
> kernel=""
> - opts=""
> + opts=$extra_opts
> groups=""
> arch=""
> check=""
> @@ -32,7 +33,7 @@ function for_each_unittest()
> elif [[ $line =~ ^smp\ *=\ *(.*)$ ]]; then
> smp=${BASH_REMATCH[1]}
> elif [[ $line =~ ^extra_params\ *=\ *(.*)$ ]]; then
> - opts=${BASH_REMATCH[1]}
> + opts="$opts ${BASH_REMATCH[1]}"
> elif [[ $line =~ ^groups\ *=\ *(.*)$ ]]; then
> groups=${BASH_REMATCH[1]}
> elif [[ $line =~ ^arch\ *=\ *(.*)$ ]]; then
> @@ -45,6 +46,6 @@ function for_each_unittest()
> timeout=${BASH_REMATCH[1]}
> fi
> done
> - "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
> + "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
> exec {fd}<&-
> }
> --
> 2.10.1
>
>
This is a pretty good idea, but I think I might like the extra options
to be given like this instead
./run_tests.sh [run_tests.sh options] -- [qemu options]
Thanks,
drew
^ permalink raw reply
* [kvm-unit-tests PATCH v7 02/11] run_tests: allow disabling of timeouts
From: Andrew Jones @ 2016-11-28 9:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161124161033.11456-3-alex.bennee@linaro.org>
On Thu, Nov 24, 2016 at 04:10:24PM +0000, Alex Benn?e wrote:
> Certainly during development of the tests and MTTCG there are times when
> the timeout just gets in the way.
>
> Signed-off-by: Alex Benn?e <alex.bennee@linaro.org>
> ---
> run_tests.sh | 8 ++++++--
> scripts/runtime.bash | 4 ++++
> 2 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/run_tests.sh b/run_tests.sh
> index b88c36f..4f2e5cb 100755
> --- a/run_tests.sh
> +++ b/run_tests.sh
> @@ -13,10 +13,11 @@ function usage()
> {
> cat <<EOF
>
> -Usage: $0 [-g group] [-a accel] [-h] [-v]
> +Usage: $0 [-g group] [-a accel] [-t] [-h] [-v]
>
> -g: Only execute tests in the given group
> -a: Force acceleration mode (tcg/kvm)
> + -t: disable timeouts
> -h: Output this help text
> -v: Enables verbose mode
>
> @@ -29,7 +30,7 @@ EOF
> RUNTIME_arch_run="./$TEST_DIR/run"
> source scripts/runtime.bash
>
> -while getopts "g:a:hv" opt; do
> +while getopts "g:a:thv" opt; do
> case $opt in
> g)
> only_group=$OPTARG
> @@ -37,6 +38,9 @@ while getopts "g:a:hv" opt; do
> a)
> force_accel=$OPTARG
> ;;
> + t)
> + no_timeout="yes"
> + ;;
> h)
> usage
> exit
> diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> index 578cf32..968ff6d 100644
> --- a/scripts/runtime.bash
> +++ b/scripts/runtime.bash
> @@ -79,6 +79,10 @@ function run()
> accel=$force_accel
> fi
>
> + if [ "$no_timeout" = "yes" ]; then
> + timeout=""
> + fi
> +
> if [ -n "$arch" ] && [ "$arch" != "$ARCH" ]; then
> echo "`SKIP` $1 ($arch only)"
> return 2
> --
> 2.10.1
>
A timeout value of zero disables the timeout. So you just need to run
TIMEOUT=0 ./run_tests.sh, or add it to config.mak.
drew
^ permalink raw reply
* [kvm-unit-tests PATCH v7 01/11] run_tests: allow forcing of acceleration mode
From: Andrew Jones @ 2016-11-28 8:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161124161033.11456-2-alex.bennee@linaro.org>
On Thu, Nov 24, 2016 at 04:10:23PM +0000, Alex Benn?e wrote:
> While tests can be pegged to tcg it is useful to override this from time
> to time, especially when testing correctness on real systems.
> ---
> run_tests.sh | 8 ++++++--
> scripts/runtime.bash | 4 ++++
> 2 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/run_tests.sh b/run_tests.sh
> index 254129d..b88c36f 100755
> --- a/run_tests.sh
> +++ b/run_tests.sh
> @@ -13,9 +13,10 @@ function usage()
> {
> cat <<EOF
>
> -Usage: $0 [-g group] [-h] [-v]
> +Usage: $0 [-g group] [-a accel] [-h] [-v]
>
> -g: Only execute tests in the given group
> + -a: Force acceleration mode (tcg/kvm)
> -h: Output this help text
> -v: Enables verbose mode
>
> @@ -28,11 +29,14 @@ EOF
> RUNTIME_arch_run="./$TEST_DIR/run"
> source scripts/runtime.bash
>
> -while getopts "g:hv" opt; do
> +while getopts "g:a:hv" opt; do
> case $opt in
> g)
> only_group=$OPTARG
> ;;
> + a)
> + force_accel=$OPTARG
> + ;;
> h)
> usage
> exit
> diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> index 11a40a9..578cf32 100644
> --- a/scripts/runtime.bash
> +++ b/scripts/runtime.bash
> @@ -75,6 +75,10 @@ function run()
> return;
> fi
>
> + if [ -n "$force_accel" ]; then
> + accel=$force_accel
> + fi
> +
> if [ -n "$arch" ] && [ "$arch" != "$ARCH" ]; then
> echo "`SKIP` $1 ($arch only)"
> return 2
> --
> 2.10.1
We can already do 'ACCEL=tcg ./run_tests.sh' to force, for example, tcg.
Additionally, you can add any env you want to the config.mak after running
configure,
echo ACCEL=tcg >> config.mak
If you still prefer a cmdline parameter, then I'd suggest a boolean
instead, with the default being KVM. So the param would be '-tcg', or
something.
Thanks,
drew
^ permalink raw reply
* [PATCH net-next 0/5] Support Armada 37xx SoC (ARMv8 64-bits) in mvneta driver
From: Jisheng Zhang @ 2016-11-28 8:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.2b146800967005632cd02d4da77397e6e2fdf51f.1480087510.git-series.gregory.clement@free-electrons.com>
Hi Gregory,
On Fri, 25 Nov 2016 16:30:13 +0100 Gregory CLEMENT wrote:
> Hi,
>
> The Armada 37xx is a new ARMv8 SoC from Marvell using same network
> controller as the older Armada 370/38x/XP SoCs. This series adapts the
> driver in order to be able to use it on this new SoC. The main changes
> are:
>
> - 64-bits support: the first patches allow using the driver on a 64-bit
> architecture.
>
> - MBUS support: the mbus configuration is different on Armada 37xx
> from the older SoCs.
>
> - per cpu interrupt: Armada 37xx do not support per cpu interrupt for
> the NETA IP, the non-per-CPU behavior was added back.
>
> The first item is solved by patches 1 to 3.
> The 2 last items are solved by patch 4.
> In patch 5 the dt support is added.
>
> Beside Armada 37xx, the series have been tested on Armada XP and
> Armada 38x (with Hardware Buffer Management and with Software Buffer
> Managment).
AFAICT, this is a V2? seems no Change log ;)
Thanks,
Jisheng
^ permalink raw reply
* [PATCH net-next 1/5] net: mvneta: Use cacheable memory to store the rx buffer virtual address
From: Jisheng Zhang @ 2016-11-28 8:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <7e6004f918d3fcde9ae71e7893d26b19086236a3.1480087510.git-series.gregory.clement@free-electrons.com>
Hi Gregory,
On Fri, 25 Nov 2016 16:30:14 +0100 Gregory CLEMENT wrote:
> Until now the virtual address of the received buffer were stored in the
> cookie field of the rx descriptor. However, this field is 32-bits only
> which prevents to use the driver on a 64-bits architecture.
>
> With this patch the virtual address is stored in an array not shared with
> the hardware (no more need to use the DMA API). Thanks to this, it is
> possible to use cache contrary to the access of the rx descriptor member.
>
> The change is done in the swbm path only because the hwbm uses the cookie
> field, this also means that currently the hwbm is not usable in 64-bits.
>
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> ---
> drivers/net/ethernet/marvell/mvneta.c | 96 ++++++++++++++++++++++++----
> 1 file changed, 84 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
> index 87274d4ab102..b6849f88cab7 100644
> --- a/drivers/net/ethernet/marvell/mvneta.c
> +++ b/drivers/net/ethernet/marvell/mvneta.c
> @@ -561,6 +561,9 @@ struct mvneta_rx_queue {
> u32 pkts_coal;
> u32 time_coal;
>
> + /* Virtual address of the RX buffer */
> + void **buf_virt_addr;
can we store buf_phys_addr in cacheable memory as well?
> +
> /* Virtual address of the RX DMA descriptors array */
> struct mvneta_rx_desc *descs;
>
> @@ -1573,10 +1576,14 @@ static void mvneta_tx_done_pkts_coal_set(struct mvneta_port *pp,
>
> /* Handle rx descriptor fill by setting buf_cookie and buf_phys_addr */
> static void mvneta_rx_desc_fill(struct mvneta_rx_desc *rx_desc,
> - u32 phys_addr, u32 cookie)
> + u32 phys_addr, void *virt_addr,
> + struct mvneta_rx_queue *rxq)
> {
> - rx_desc->buf_cookie = cookie;
> + int i;
> +
> rx_desc->buf_phys_addr = phys_addr;
> + i = rx_desc - rxq->descs;
> + rxq->buf_virt_addr[i] = virt_addr;
> }
>
> /* Decrement sent descriptors counter */
> @@ -1781,7 +1788,8 @@ EXPORT_SYMBOL_GPL(mvneta_frag_free);
>
> /* Refill processing for SW buffer management */
> static int mvneta_rx_refill(struct mvneta_port *pp,
> - struct mvneta_rx_desc *rx_desc)
> + struct mvneta_rx_desc *rx_desc,
> + struct mvneta_rx_queue *rxq)
>
> {
> dma_addr_t phys_addr;
> @@ -1799,7 +1807,7 @@ static int mvneta_rx_refill(struct mvneta_port *pp,
> return -ENOMEM;
> }
>
> - mvneta_rx_desc_fill(rx_desc, phys_addr, (u32)data);
> + mvneta_rx_desc_fill(rx_desc, phys_addr, data, rxq);
> return 0;
> }
>
> @@ -1861,7 +1869,12 @@ static void mvneta_rxq_drop_pkts(struct mvneta_port *pp,
>
> for (i = 0; i < rxq->size; i++) {
> struct mvneta_rx_desc *rx_desc = rxq->descs + i;
> - void *data = (void *)rx_desc->buf_cookie;
> + void *data;
> +
> + if (!pp->bm_priv)
> + data = rxq->buf_virt_addr[i];
> + else
> + data = (void *)(uintptr_t)rx_desc->buf_cookie;
>
> dma_unmap_single(pp->dev->dev.parent, rx_desc->buf_phys_addr,
> MVNETA_RX_BUF_SIZE(pp->pkt_size), DMA_FROM_DEVICE);
> @@ -1894,12 +1907,13 @@ static int mvneta_rx_swbm(struct mvneta_port *pp, int rx_todo,
> unsigned char *data;
> dma_addr_t phys_addr;
> u32 rx_status, frag_size;
> - int rx_bytes, err;
> + int rx_bytes, err, index;
>
> rx_done++;
> rx_status = rx_desc->status;
> rx_bytes = rx_desc->data_size - (ETH_FCS_LEN + MVNETA_MH_SIZE);
> - data = (unsigned char *)rx_desc->buf_cookie;
> + index = rx_desc - rxq->descs;
> + data = (unsigned char *)rxq->buf_virt_addr[index];
> phys_addr = rx_desc->buf_phys_addr;
>
> if (!mvneta_rxq_desc_is_first_last(rx_status) ||
> @@ -1938,7 +1952,7 @@ static int mvneta_rx_swbm(struct mvneta_port *pp, int rx_todo,
> }
>
> /* Refill processing */
> - err = mvneta_rx_refill(pp, rx_desc);
> + err = mvneta_rx_refill(pp, rx_desc, rxq);
> if (err) {
> netdev_err(dev, "Linux processing - Can't refill\n");
> rxq->missed++;
> @@ -2020,7 +2034,7 @@ static int mvneta_rx_hwbm(struct mvneta_port *pp, int rx_todo,
> rx_done++;
> rx_status = rx_desc->status;
> rx_bytes = rx_desc->data_size - (ETH_FCS_LEN + MVNETA_MH_SIZE);
> - data = (unsigned char *)rx_desc->buf_cookie;
> + data = (u8 *)(uintptr_t)rx_desc->buf_cookie;
> phys_addr = rx_desc->buf_phys_addr;
> pool_id = MVNETA_RX_GET_BM_POOL_ID(rx_desc);
> bm_pool = &pp->bm_priv->bm_pools[pool_id];
> @@ -2708,6 +2722,57 @@ static int mvneta_poll(struct napi_struct *napi, int budget)
> return rx_done;
> }
>
> +/* Refill processing for HW buffer management */
> +static int mvneta_rx_hwbm_refill(struct mvneta_port *pp,
> + struct mvneta_rx_desc *rx_desc)
> +
> +{
> + dma_addr_t phys_addr;
> + void *data;
> +
> + data = mvneta_frag_alloc(pp->frag_size);
> + if (!data)
> + return -ENOMEM;
> +
> + phys_addr = dma_map_single(pp->dev->dev.parent, data,
> + MVNETA_RX_BUF_SIZE(pp->pkt_size),
> + DMA_FROM_DEVICE);
> + if (unlikely(dma_mapping_error(pp->dev->dev.parent, phys_addr))) {
> + mvneta_frag_free(pp->frag_size, data);
> + return -ENOMEM;
> + }
> +
> + phys_addr += pp->rx_offset_correction;
> + rx_desc->buf_phys_addr = phys_addr;
> + rx_desc->buf_cookie = (uintptr_t)data;
> +
> + return 0;
> +}
> +
> +/* Handle rxq fill: allocates rxq skbs; called when initializing a port */
> +static int mvneta_rxq_bm_fill(struct mvneta_port *pp,
> + struct mvneta_rx_queue *rxq,
> + int num)
> +{
> + int i;
> +
> + for (i = 0; i < num; i++) {
> + memset(rxq->descs + i, 0, sizeof(struct mvneta_rx_desc));
> + if (mvneta_rx_hwbm_refill(pp, rxq->descs + i) != 0) {
> + netdev_err(pp->dev, "%s:rxq %d, %d of %d buffs filled\n",
> + __func__, rxq->id, i, num);
> + break;
> + }
> + }
> +
> + /* Add this number of RX descriptors as non occupied (ready to
> + * get packets)
> + */
> + mvneta_rxq_non_occup_desc_add(pp, rxq, i);
> +
> + return i;
> +}
> +
> /* Handle rxq fill: allocates rxq skbs; called when initializing a port */
> static int mvneta_rxq_fill(struct mvneta_port *pp, struct mvneta_rx_queue *rxq,
> int num)
> @@ -2716,7 +2781,7 @@ static int mvneta_rxq_fill(struct mvneta_port *pp, struct mvneta_rx_queue *rxq,
>
> for (i = 0; i < num; i++) {
> memset(rxq->descs + i, 0, sizeof(struct mvneta_rx_desc));
> - if (mvneta_rx_refill(pp, rxq->descs + i) != 0) {
> + if (mvneta_rx_refill(pp, rxq->descs + i, rxq) != 0) {
> netdev_err(pp->dev, "%s:rxq %d, %d of %d buffs filled\n",
> __func__, rxq->id, i, num);
> break;
> @@ -2784,14 +2849,21 @@ static int mvneta_rxq_init(struct mvneta_port *pp,
> mvneta_rxq_buf_size_set(pp, rxq,
> MVNETA_RX_BUF_SIZE(pp->pkt_size));
> mvneta_rxq_bm_disable(pp, rxq);
> +
> + rxq->buf_virt_addr = devm_kmalloc(pp->dev->dev.parent,
> + rxq->size * sizeof(void *),
> + GFP_KERNEL);
I would suggest allocate this buffer during probe. Otherwise, there's
memory leak if we either change the mtu or close then open the eth in
a loop, e.g
while true
do
ifconfig eth0 up
ifconfig eth0 down
done
Thanks,
Jisheng
> + if (!rxq->buf_virt_addr)
> + return -ENOMEM;
> +
> + mvneta_rxq_fill(pp, rxq, rxq->size);
> } else {
> mvneta_rxq_bm_enable(pp, rxq);
> mvneta_rxq_long_pool_set(pp, rxq);
> mvneta_rxq_short_pool_set(pp, rxq);
> + mvneta_rxq_bm_fill(pp, rxq, rxq->size);
> }
>
> - mvneta_rxq_fill(pp, rxq, rxq->size);
> -
> return 0;
> }
>
^ permalink raw reply
* [RFC PATCH 3/3] dt-bindings: display: add Amlogic Meson DRM Bindings
From: Laurent Pinchart @ 2016-11-28 8:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480089791-12517-4-git-send-email-narmstrong@baylibre.com>
Hi Neil,
Thank you for the patch.
On Friday 25 Nov 2016 17:03:11 Neil Armstrong wrote:
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
> .../bindings/display/meson/meson-drm.txt | 134 +++++++++++++++++
> 1 file changed, 134 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/display/meson/meson-drm.txt
>
> diff --git a/Documentation/devicetree/bindings/display/meson/meson-drm.txt
> b/Documentation/devicetree/bindings/display/meson/meson-drm.txt new file
> mode 100644
> index 0000000..89c1b5f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/meson/meson-drm.txt
> @@ -0,0 +1,134 @@
> +Amlogic Meson Display Controller
> +================================
> +
> +The Amlogic Meson Display controller is composed of several components
> +that are going to be documented below:
> +
> +DMC|---------------VPU (Video Processing Unit)------------|------HHI------|
> + | vd1 _______ _____________ _____________ | |
> +D |-------| |----| | | | | HDMI PLL |
> +D | vd2 | VIU | | Video Post | | Video Encs |<---|-----VCLK |
> +R |-------| |----| Processing | | | | |
> + | osd2 | | | |---| Enci ------|----|-----VDAC------|
> +R |-------| CSC |----| Scalers | | Encp ------|----|----HDMI-TX----|
> +A | osd1 | | | Blenders | | Encl-------|----|---------------|
> +M |-------|______|----|____________| |____________| | |
> +___|______________________________________________________|_______________|
> +
> +
> +VIU: Video Input Unit
> +---------------------
> +
> +The Video Input Unit is in charge of the pixel scanout from the DDR memory.
> +It fetches the frames addresses, stride and parameters from the "Canvas"
> memory.
> +This part is also in charge of the CSC (Colorspace Conversion).
> +It can handle 2 OSD Planes and 2 Video Planes.
> +
> +VPP: Video Processing Unit
Do you mean "Video Post Processing" ? In your diagram above Video Processing
Unit is abbreviated VPU and covers the VIU, VPP and encoders.
> +--------------------------
> +
> +The Video Processing Unit is in charge if the scaling and blending of the
> +various planes into a single pixel stream.
> +There is a special "pre-blending" used by the video planes with a dedicated
> +scaler and a "post-blending" to merge with the OSD Planes.
> +The OSD planes also have a dedicated scaler for one of the OSD.
> +
> +VENC: Video Encoders
> +--------------------
> +
> +The VENC is composed of the multiple pixel encoders :
> + - ENCI : Interlace Video encoder for CVBS and Interlace HDMI
> + - ENCP : Progressive Video Encoder for HDMI
> + - ENCL : LCD LVDS Encoder
> +The VENC Unit gets a Pixel Clocks (VCLK) from a dedicated HDMI PLL and
> clock
> +tree and provides the scanout clock to the VPP and VIU.
> +The ENCI is connected to a single VDAC for Composite Output.
> +The ENCI and ENCP are connected to an on-chip HDMI Transceiver.
> +
> +Device Tree Bindings:
> +---------------------
> +
> +VPU: Video Processing Unit
> +--------------------------
> +
> +Required properties:
> + - compatible: value should be different for each SoC family as :
> + - GXBB (S905) : "amlogic,meson-gxbb-vpu"
> + - GXL (S905X, S905D) : "amlogic,meson-gxl-vpu"
> + - GXM (S912) : "amlogic,meson-gxm-vpu"
> + followed by the common "amlogic,meson-gx-vpu"
> + - reg: base address and size of he following memory-mapped regions :
> + - vpu
> + - hhi
> + - dmc
> + - reg-names: should contain the names of the previous memory regions
> + - interrupts: should contain the VENC Vsync interrupt number
> +
> +- ports: A ports node with endpoint definitions as defined in
> + Documentation/devicetree/bindings/media/video-interfaces.txt. The
> + second port should be the output endpoints for VENC connectors.
> +
> +VENC CBVS Output
> +----------------------
> +
> +The VENC can output Composite/CVBS output via a decicated VDAC.
> +
> +Required properties:
> + - compatible: value must be one of:
> + - compatible: value should be different for each SoC family as :
One of those two lines is redundant.
> + - GXBB (S905) : "amlogic,meson-gxbb-venc-cvbs"
> + - GXL (S905X, S905D) : "amlogic,meson-gxl-venc-cvbs"
> + - GXM (S912) : "amlogic,meson-gxm-venc-cvbs"
> + followed by the common "amlogic,meson-gx-venc-cvbs"
> +
No registers ? Are the encoders registers part of the VPU register space,
intertwined in a way that they can't be specified separately here ?
> +- ports: A ports node with endpoint definitions as defined in
> + Documentation/devicetree/bindings/media/video-interfaces.txt. The
> + first port should be the input endpoints, connected ot the VPU node.
> +
> +Example:
> +
> +venc_cvbs: venc-cvbs {
> + compatible = "amlogic,meson-gxbb-venc-cvbs";
> + status = "okay";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + enc_cvbs_in: port at 0 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0>;
> +
> + venc_cvbs_in_vpu: endpoint at 0 {
> + reg = <0>;
> + remote-endpoint = <&vpu_out_venc_cvbs>;
> + };
> + };
> + };
> +};
> +
> +vpu: vpu at d0100000 {
> + compatible = "amlogic,meson-gxbb-vpu";
> + reg = <0x0 0xd0100000 0x0 0x100000>,
> + <0x0 0xc883c000 0x0 0x1000>,
> + <0x0 0xc8838000 0x0 0x1000>;
> + reg-names = "base", "hhi", "dmc";
> + interrupts = <GIC_SPI 3 IRQ_TYPE_EDGE_RISING>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + vpu_out: port at 1 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <1>;
> +
> + vpu_out_venc_cvbs: endpoint at 0 {
> + reg = <0>;
> + remote-endpoint = <&venc_cvbs_in_vpu>;
> + };
> + };
> + };
> +};
--
Regards,
Laurent Pinchart
^ permalink raw reply
* [PATCH v2 2/3] ARM: davinci: da850-evm: use gpio descriptor for mmc pins
From: Sekhar Nori @ 2016-11-28 8:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161124150454.23899-3-ahaslam@baylibre.com>
On Thursday 24 November 2016 08:34 PM, Axel Haslam wrote:
> Currently the mmc driver is polling the gpio to know if the
> card was removed.
>
> By using a gpio descriptor instead of the platform callbacks,
> the driver will be able to register the gpio using the mmc core
> API's designed for this purpose.
>
> This has the advantage that an irq will be registered, and
> polling is no longer needed. Also, a dependency on platform
> callbacks is removed for this board.
>
> Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
Applied.
Thanks,
Sekhar
^ permalink raw reply
* [PATCH v2 1/3] ARM: davinci: hawk: use gpio descriptor for mmc pins
From: Sekhar Nori @ 2016-11-28 8:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161124150454.23899-2-ahaslam@baylibre.com>
On Thursday 24 November 2016 08:34 PM, Axel Haslam wrote:
> Currently the mmc driver is polling the gpio to know if the
> card was removed.
>
> By using a gpio descriptor instead of the platform callbacks,
> the driver will be able to register the gpio using the mmc core
> API's designed for this purpose.
s/API's/APIs
>
> This has the advantage that an irq will be registered, and
> polling is no longer needed. Also, a dependency on platform
> callbacks is removed for this board.
>
> Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
> ---
> arch/arm/mach-davinci/board-omapl138-hawk.c | 42 ++++++++---------------------
> 1 file changed, 11 insertions(+), 31 deletions(-)
>
> diff --git a/arch/arm/mach-davinci/board-omapl138-hawk.c b/arch/arm/mach-davinci/board-omapl138-hawk.c
> index a4e8726..a2966d3 100644
> --- a/arch/arm/mach-davinci/board-omapl138-hawk.c
> +++ b/arch/arm/mach-davinci/board-omapl138-hawk.c
> @@ -15,6 +15,7 @@
> #include <linux/gpio.h>
> #include <linux/platform_data/gpio-davinci.h>
> #include <linux/regulator/machine.h>
> +#include <linux/gpio/machine.h>
Moved this include to below linux/gpio.h
Applied to v4.10/soc
Thanks,
Sekhar
^ permalink raw reply
* [PATCH 2/2] net: dsa: mv88e6xxx: Add 88E6176 device tree support
From: Uwe Kleine-König @ 2016-11-28 8:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161127231009.GA17704@lunn.ch>
Hello Andrew,
On Mon, Nov 28, 2016 at 12:10:09AM +0100, Andrew Lunn wrote:
> > Try to see it from my perspective: I see that some vf610 device I don't
> > have (found via `git grep marvell,mv88e6` or so) uses
> > "marvell,mv88e6085". I then assume it has that device on board. How
> > would I know it doesn't? Same for the other boards you mention.
> >
> > Unfortunately some of your replies are slightly cryptic. Had you simply
> > replied 'please just use "marvell,mv88e6085" instead', it would've been
> > much more clear what you want. (Same for extending the subject instead
> > of just pointing to some FAQ.)
>
> By reading the FAQ you have learnt more than me saying put the correct
> tree in the subject line. By asking you to explain why you need a
> compatible string, i'm trying to make you think, look at the code and
> understand it. In the future, you might think and understand the code
> before posting a patch, and then we all save time.
I agree to Andreas though, that it makes an school teacher impression.
Something like:
Please fix the subject. Check the FAQ for the details, which btw
is worth a read completely.
is IMHO better in this regard and once you found the problem there you
don't need to ask back if it's that what was meant.
> > So are you okay with patch 1/2 documenting the compatible? Then we could
> > drop 2/2 and use "marvell,mv88e6176", "marvell,mv88e6085" instead of
> > just the latter. Or would you rather drop both and keep the actual chip
> > a comment?
>
> A comment only please.
I still wonder (and didn't get an answer back when I asked about this)
why a comment is preferred here. For other devices I know it's usual and
requested by the maintainers to use:
compatible = "exact name", "earlyer device to match driver";
. This is more robust, documents the situation more formally and makes
it better greppable. The price to pay is only a few bytes in the dtb
which IMO is ok.
Best regards
Uwe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161128/c9e0e382/attachment.sig>
^ permalink raw reply
* [PATCH] ARM: dts: da850: specify the maximum bandwidth for tilcdc
From: Sekhar Nori @ 2016-11-28 7:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <953743fb-54f9-fa6f-bfdd-43d92271864f@ti.com>
On Monday 28 November 2016 01:12 PM, Tomi Valkeinen wrote:
> On 28/11/16 07:24, Sekhar Nori wrote:
>> On Friday 25 November 2016 09:07 PM, Bartosz Golaszewski wrote:
>>> It has been determined that the maximum resolution supported correctly
>>> by tilcdc rev1 on da850 SoCs is 800x600 at 60. Due to memory throughput
>>> constraints we must filter out higher modes.
>>>
>>> Specify the max-bandwidth property for the display node for
>>> da850-based boards.
>>>
>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>> ---
>>> arch/arm/boot/dts/da850.dtsi | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>>> index 8e30d9b..9b7c444 100644
>>> --- a/arch/arm/boot/dts/da850.dtsi
>>> +++ b/arch/arm/boot/dts/da850.dtsi
>>> @@ -452,6 +452,7 @@
>>> compatible = "ti,da850-tilcdc";
>>> reg = <0x213000 0x1000>;
>>> interrupts = <52>;
>>> + max-bandwidth = <28800000>;
>>
>> If this is effectively the max pixel clock that the device supports,
>> then why not use the datasheet specified value of 37.5 MHz (Tc = 26.66 ns).
>
> There's a separate property for max-pixelclock. This one is maximum
> pixels per second (which does sound almost the same), but the doc says
> it's about the particular memory interface + LCDC combination.
DA850 supports both mDDR and DDR2, at slightly different speeds. So
memory bandwidth limitation is also board specific. This should probably
move to board file.
But I would like to know why using max-pixelclock is not good enough.
Have experiments shown that LCDC on DA850 LCDK underflows even if pixel
clock is below the datasheet recommendation?
Thanks,
Sekhar
^ permalink raw reply
* [PATCH] ARM: dts: da850: specify the maximum bandwidth for tilcdc
From: Tomi Valkeinen @ 2016-11-28 7:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <8829c208-0674-43c0-8449-ef764071583f@ti.com>
On 28/11/16 07:24, Sekhar Nori wrote:
> On Friday 25 November 2016 09:07 PM, Bartosz Golaszewski wrote:
>> It has been determined that the maximum resolution supported correctly
>> by tilcdc rev1 on da850 SoCs is 800x600 at 60. Due to memory throughput
>> constraints we must filter out higher modes.
>>
>> Specify the max-bandwidth property for the display node for
>> da850-based boards.
>>
>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>> ---
>> arch/arm/boot/dts/da850.dtsi | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>> index 8e30d9b..9b7c444 100644
>> --- a/arch/arm/boot/dts/da850.dtsi
>> +++ b/arch/arm/boot/dts/da850.dtsi
>> @@ -452,6 +452,7 @@
>> compatible = "ti,da850-tilcdc";
>> reg = <0x213000 0x1000>;
>> interrupts = <52>;
>> + max-bandwidth = <28800000>;
>
> If this is effectively the max pixel clock that the device supports,
> then why not use the datasheet specified value of 37.5 MHz (Tc = 26.66 ns).
There's a separate property for max-pixelclock. This one is maximum
pixels per second (which does sound almost the same), but the doc says
it's about the particular memory interface + LCDC combination.
But this 'max-bandwidth' does sound quite odd, as the it really should
be bytes, not pixels... Bad bindings again, which we just have to use.
Tomi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161128/12541df9/attachment.sig>
^ 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