* [PATCHv3 5/6] arm64: Use __pa_symbol for kernel symbols
From: Laura Abbott @ 2016-11-21 17:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161118143543.GC1197@leverpostej>
On 11/18/2016 06:35 AM, Mark Rutland wrote:
> Hi Laura,
>
> On Thu, Nov 17, 2016 at 05:16:55PM -0800, Laura Abbott wrote:
>>
>> __pa_symbol is technically the marco that should be used for kernel
>> symbols. Switch to this as a pre-requisite for DEBUG_VIRTUAL which
>> will do bounds checking.
>>
>> Signed-off-by: Laura Abbott <labbott@redhat.com>
>> ---
>> v3: Conversion of more sites besides just _end. Addition of __lm_sym_addr
>> macro to take care of the _va(__pa_symbol(..)) idiom.
>>
>> Note that a copy of __pa_symbol was added to avoid a mess of headers
>> since the #ifndef __pa_symbol case is defined in linux/mm.h
>
> I think we also need to fix up virt_to_phys(__cpu_soft_restart) in
> arch/arm64/kernel/cpu-reset.h. Otherwise, this looks complete for uses
> falling under arch/arm64/.
>
> I also think it's worth mentioning in the commit message that this patch
> adds and __lm_sym_addr() and uses it in some places so that low-level
> helpers can use virt_to_phys() or __pa() consistently.
>
> The PSCI change doesn't conflict with patches [1] that'll go via
> arm-soc, so I'm happy for that PSCI change to go via the arm64 tree,
> though it may be worth splitting into its own patch just in case
> something unexpected crops up.
>
> With those fixed up:
>
> Reviewed-by: Mark Rutland <mark.rutland@arm.com>
>
> [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-November/466522.html
>
> Otherwise, I just have a few nits below.
>
>> @@ -271,7 +271,7 @@ static inline void __kvm_flush_dcache_pud(pud_t pud)
>> kvm_flush_dcache_to_poc(page_address(page), PUD_SIZE);
>> }
>>
>> -#define kvm_virt_to_phys(x) __virt_to_phys((unsigned long)(x))
>> +#define kvm_virt_to_phys(x) __pa_symbol((unsigned long)(x))
>
> Nit: we can drop the unsigned long cast given __pa_symbol() contains
> one.
>
>> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
>> index ffbb9a5..c2041a3 100644
>> --- a/arch/arm64/include/asm/pgtable.h
>> +++ b/arch/arm64/include/asm/pgtable.h
>> @@ -52,7 +52,7 @@ extern void __pgd_error(const char *file, int line, unsigned long val);
>> * for zero-mapped memory areas etc..
>> */
>> extern unsigned long empty_zero_page[PAGE_SIZE / sizeof(unsigned long)];
>> -#define ZERO_PAGE(vaddr) pfn_to_page(PHYS_PFN(__pa(empty_zero_page)))
>> +#define ZERO_PAGE(vaddr) pfn_to_page(PHYS_PFN(__pa_symbol(empty_zero_page)))
>
> Nit: I think we can also simplify this to:
>
> phys_to_page(__pa_symbol(empty_zero_page))
>
> ... since phys_to_page(p) is (pfn_to_page(__phys_to_pfn(p)))
> ... and __phys_to_pfn(p) is PHYS_PFN(p)
>
>> diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c
>> index 6f2ac4f..af8967a 100644
>> --- a/arch/arm64/kernel/insn.c
>> +++ b/arch/arm64/kernel/insn.c
>> @@ -97,7 +97,7 @@ static void __kprobes *patch_map(void *addr, int fixmap)
>> if (module && IS_ENABLED(CONFIG_DEBUG_SET_MODULE_RONX))
>> page = vmalloc_to_page(addr);
>> else if (!module)
>> - page = pfn_to_page(PHYS_PFN(__pa(addr)));
>> + page = pfn_to_page(PHYS_PFN(__pa_symbol(addr)));
>
> Nit: likewise, we can use phys_to_page() here.
>
>> diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
>> index a2c2478..791e87a 100644
>> --- a/arch/arm64/kernel/vdso.c
>> +++ b/arch/arm64/kernel/vdso.c
>> @@ -140,11 +140,11 @@ static int __init vdso_init(void)
>> return -ENOMEM;
>>
>> /* Grab the vDSO data page. */
>> - vdso_pagelist[0] = pfn_to_page(PHYS_PFN(__pa(vdso_data)));
>> + vdso_pagelist[0] = pfn_to_page(PHYS_PFN(__pa_symbol(vdso_data)));
>
> Nit: phys_to_page() again.
>
>>
>> /* Grab the vDSO code pages. */
>> for (i = 0; i < vdso_pages; i++)
>> - vdso_pagelist[i + 1] = pfn_to_page(PHYS_PFN(__pa(&vdso_start)) + i);
>> + vdso_pagelist[i + 1] = pfn_to_page(PHYS_PFN(__pa_symbol(&vdso_start)) + i);
>
> Nit: phys_to_page() again.
I think it makes sense to keep this one as is. It's offsetting
by pfn number and trying force phys_to_page would make it more
difficult to read.
>
> Thanks,
> Mark.
>
Thanks,
Laura
^ permalink raw reply
* [PATCH 0/2] ARM: davinvi: da850 add ohci DT nodes
From: David Lechner @ 2016-11-21 17:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKXjFTPxwxtE1xs1b6+sfwvRNKVLGKM-9Fjs3tVOSaBA+5Wnxg@mail.gmail.com>
On 11/21/2016 11:29 AM, Axel Haslam wrote:
> On Mon, Nov 21, 2016 at 6:04 PM, David Lechner <david@lechnology.com> wrote:
>> On 11/21/2016 10:59 AM, Axel Haslam wrote:
>>>
>>> This adds the DT node for the ohci controller and
>>> enables it for the omapl138-lckd platform.
>>>
>>> DEPENDENCIES:
>>>
>>> 1. [PATCH v6 0/5] USB: ohci-da8xx: Add device tree support
>>> https://lkml.org/lkml/2016/11/21/558
>>>
>>> 2. [PATCH v3 0/2] regulator: handling of error conditions for usb drivers
>>> https://lkml.org/lkml/2016/11/4/465
>>>
>>> Axel Haslam (2):
>>> ARM: dts: da850: Add usb device node
>>> ARM: dts: da850-lcdk: Enable ohci for omapl138 lcdk
>>>
>>> arch/arm/boot/dts/da850-lcdk.dts | 8 ++++++++
>>> arch/arm/boot/dts/da850.dtsi | 8 ++++++++
>>> 2 files changed, 16 insertions(+)
>>>
>>
>> It does not look like you rebased these patches. Sekhar pushed the musb
>> counterpart to v4.10/dt yesterday, which will cause conflicts with this
>> series.
>>
>> https://git.kernel.org/cgit/linux/kernel/git/nsekhar/linux-davinci.git/log/?h=v4.10/dt
>
> Hi David,
>
> i verified that they apply to the current linux-davinci/master.
> Anyways, i can rebase
> and resend once the dependencies are met, and we are ready to merge it.
>
> Regards
> Axel.
>
OK. I think the first patch is fine, but you will have two &usb_phy {
status = "okay"; }; in da850-lcdk.dts now even if applies cleanly. ;-)
^ permalink raw reply
* [PATCH v2 1/5] ARM: memory: da8xx-ddrctl: new driver
From: Robin Murphy @ 2016-11-21 17:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAMpxmJUXJi6PDq0qc-0+r2mLPASLpJUt_njWtXr4Mx4k0Fa82g@mail.gmail.com>
Hi Bartosz, Sekhar,
On 21/11/16 16:48, Bartosz Golaszewski wrote:
> 2016-11-21 17:33 GMT+01:00 Sekhar Nori <nsekhar@ti.com>:
>> On Monday 31 October 2016 08:15 PM, Bartosz Golaszewski wrote:
>>> +static int da8xx_ddrctl_probe(struct platform_device *pdev)
>>> +{
>>> + const struct da8xx_ddrctl_config_knob *knob;
>>> + const struct da8xx_ddrctl_setting *setting;
>>> + struct device_node *node;
>>> + struct resource *res;
>>> + void __iomem *ddrctl;
>>> + struct device *dev;
>>> + u32 reg;
>>> +
>>> + dev = &pdev->dev;
>>> + node = dev->of_node;
>>> +
>>> + setting = da8xx_ddrctl_get_board_settings();
>>> + if (!setting) {
>>> + dev_err(dev, "no settings for board '%s'\n",
>>> + of_flat_dt_get_machine_name());
>>> + return -EINVAL;
>>> + }
>>
>> This causes a section mismatch because of_flat_dt_get_machine_name()
>> has an __init annotation. I did not notice that before, sorry.
>>
>> It can be fixed with a patch like below:
>>
>> ---8<---
>> diff --git a/drivers/memory/da8xx-ddrctl.c b/drivers/memory/da8xx-ddrctl.c
>> index a20e7bbbcbe0..9ca5aab3ac54 100644
>> --- a/drivers/memory/da8xx-ddrctl.c
>> +++ b/drivers/memory/da8xx-ddrctl.c
>> @@ -102,6 +102,18 @@ static const struct da8xx_ddrctl_setting *da8xx_ddrctl_get_board_settings(void)
>> return NULL;
>> }
>>
>> +static const char* da8xx_ddrctl_get_machine_name(void)
>> +{
>> + const char *str;
>> + int ret;
>> +
>> + ret = of_property_read_string(of_root, "model", &str);
>> + if (ret)
>> + ret = of_property_read_string(of_root, "compatible", &str);
>> +
>> + return str;
>> +}
>> +
>> static int da8xx_ddrctl_probe(struct platform_device *pdev)
>> {
>> const struct da8xx_ddrctl_config_knob *knob;
>> @@ -118,7 +130,7 @@ static int da8xx_ddrctl_probe(struct platform_device *pdev)
>> setting = da8xx_ddrctl_get_board_settings();
>> if (!setting) {
>> dev_err(dev, "no settings for board '%s'\n",
>> - of_flat_dt_get_machine_name());
>> + da8xx_ddrctl_get_machine_name());
>> return -EINVAL;
>> }
>> ---8<---
>>
>> A similar fix is required for the other driver in this series (patch
>> 2/5). I need some advise on whether I should introduce a common
>> function to get the machine name post kernel boot-up (I cannot see an
>> existing one). If yes, any advise on which file it should go into?
>>
>
> Hi Sekhar,
>
> thanks for spotting that.
>
> I think we should introduce this function right away, rather than
> having two static functions doing the same thing. If you don't mind,
> I'll try to find a good spot for it and send a follow-up series fixing
> the issue.
As it happens, that was already proposed last week, for much the same
reason:
http://www.mail-archive.com/linuxppc-dev at lists.ozlabs.org/msg111395.html
Robin.
>
> Best regards,
> Bartosz Golaszewski
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply
* [BUG] hdlcd gets confused about base address
From: Russell King - ARM Linux @ 2016-11-21 17:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161121173231.GM1005@e106497-lin.cambridge.arm.com>
On Mon, Nov 21, 2016 at 05:32:32PM +0000, Liviu Dudau wrote:
> On Mon, Nov 21, 2016 at 02:03:49PM +0000, Russell King - ARM Linux wrote:
> > On Mon, Nov 21, 2016 at 01:50:31PM +0000, Liviu Dudau wrote:
> > > On Mon, Nov 21, 2016 at 01:24:19PM +0000, Russell King - ARM Linux wrote:
> > > > On Mon, Nov 21, 2016 at 12:56:53PM +0000, Liviu Dudau wrote:
> > > > > That is mostly due to the check in hdlcd_crtc_disable() which I should
> > > > > remove, I've added it because I was getting a ->disable() hook call
> > > > > before any ->enable() was called at startup time. I need to revisit
> > > > > this as I remember Daniel was commenting that this was not needed.
> > > >
> > > > Removing that test results in:
> > > >
> > > > [drm:drm_atomic_helper_commit_cleanup_done] *ERROR* [CRTC:24:crtc-0] flip_done timed out
> > > >
> > > > and the kernel hanging, seemingly in an IRQs-off region.
> > >
> > > Right, I need to sort this one out. Are you doing these tests out of
> > > some tagged branch that I can get in sync with?
>
> Hi Russell,
>
> >
> > No, not yet, and some of the changes I have are rather hacky.
> >
> > I do always build my full tree of patches (which is currently running at
> > around 320 patches at the moment) but I never share that entire patch
> > set. However, none of those touch i2c (apart from the ones I've recently
> > posted) and the only patches touching hdlcd are those I've posted so far.
> >
> > Most of the problems I'm finding are through trying basic stuff - I'm not
> > doing anything special or unusual to find them, at the moment quite
> > literally just starting Xorg up and shutting it down. For example, the
> > above was caused by logging in on serial, running:
> >
> > Xorg -terminate -verbose
> >
> > and then hitting ^C. (I have lxdm disabled so systemd boots to VT login
> > prompts on both the "framebuffer" and serial - I don't want Xorg coming
> > up when the machine is booting for its nightly KVM boot tests.)
> >
> > I'm afraid that when I try someone elses code, I have a tendency to find
> > loads of seemingly trivial bugs when I try putting it through some basic
> > tests.
>
> I'm not being able to reproduce your bug conditions. I'm running the following
> setup when testing:
>
> - mainline v4.9-rc6
> - edited the juno-base.dtsi file to disable the hdlcd at 7f600000 and
> hdmi-transmitter at 70 nodes to remove the second HDMI output from the test.
> - patched tda998x_drv.c to set interlace_allowed = 0, see below why
> - modified the hdlcd_crtc.c file with the following patch:
>
> -8<-----------------------------------------------------------------------
> diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
> index 48019ae..656dc43 100644
> --- a/drivers/gpu/drm/arm/hdlcd_crtc.c
> +++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
> @@ -156,9 +156,7 @@ static void hdlcd_crtc_disable(struct drm_crtc *crtc)
> {
> struct hdlcd_drm_private *hdlcd = crtc_to_hdlcd_priv(crtc);
>
> - if (!crtc->state->active)
> - return;
> -
> + drm_crtc_vblank_off(crtc);
Don't you need a drm_crtc_vblank_on() call in the enable function?
> hdlcd_write(hdlcd, HDLCD_REG_COMMAND, 0);
> clk_disable_unprepare(hdlcd->clk);
> }
> ->8-----------------------------------------------------------------------
>
> That takes care of the pxlclk refcounting issue you were seeing. I've started
> Xorg several times (and yes, I do see EDID checksum error every now and then,
> specially when running xrandr). When closing down Xorg I get back the framebuffer
> console with the login prompt and no image shifting.
For me, the image shift was 100% reproducable. With the above patch
and a call to drm_crtc_vblank_on() in the enable path, it seems to
behave correctly - I can alternately switch between 1920x1080 and
1280x1024 and it behaves correctly. Indeed, my debug prints show that
the right thing is happening wrt disabling the controller:
[ 76.869136] hdlcd_crtc_disable: active 0
[ 76.869159] hdlcd_plane_atomic_update: pitch 7680 lines 1080
[ 76.888983] hdlcd_plane_atomic_update: pitch 5120 lines 1024
[ 76.888995] hdlcd_crtc_enable: active 1 cmd 00000000
[ 85.262451] hdlcd_crtc_disable: active 0
[ 85.262474] hdlcd_plane_atomic_update: pitch 5120 lines 1024
[ 85.286667] hdlcd_plane_atomic_update: pitch 7680 lines 1080
[ 85.286679] hdlcd_crtc_enable: active 1 cmd 00000000
[ 92.658038] hdlcd_crtc_disable: active 0
[ 92.658057] hdlcd_plane_atomic_update: pitch 7680 lines 1080
[ 92.680659] hdlcd_plane_atomic_update: pitch 5120 lines 1024
[ 92.680668] hdlcd_crtc_enable: active 1 cmd 00000000
[ 97.805205] hdlcd_crtc_disable: active 0
[ 97.805220] hdlcd_plane_atomic_update: pitch 5120 lines 1024
[ 97.834415] hdlcd_plane_atomic_update: pitch 7680 lines 1080
[ 97.834423] hdlcd_crtc_enable: active 1 cmd 00000000
> My monitor is a TV that
> reports that preferred mode is 1080i, however HDLCD and TDA19988 don't talk
> propertly with each other to be able to set the interlaced mode correctly, so
> I've had to disable support for interlacing mode in tda998x_drv.c and now the
> preferred mode that gets picked up is 1920x1200 at 60Hz.
That's more of a generic DRM issue - the CRTC layer doesn't get a
look-in when a connector parses the modes supplied from the display,
so there's no real way for the CRTC layer to apply any kind of
limitations to the available modes, except when a mode is attempted
to be set.
I don't want to see an "interlace" DT property introduced for the
TDA998x, because that's the wrong approach - it would be adding a
property for the needs of the implementation, and not a description
of the hardware.
> Please advise on what other steps I can take to try to reproduce this.
I guess if you've tried and failed to reproduce it, there is something
very specific to my setup which I can't describe.
> P.S: What revision of Juno do you have? Any chance you can capture the start
> of the boot process where the firmware component prints the version numbers?
All I know is that it's a HBI0282B, which thanks to Sudeep's efforts
(he spent _all_ of last Tuesday logged in to one of my systems trying
to upgrade the firmware) is now running the Linaro 16.10 firmware.
Sudeep says that my hardware is a very early revision which went out
the door without being properly calibrated.
Whether that has any bearing on the reproducability of this or not, I've
no idea.
If you want to see the boot messages, head to my autobuilder status page
and look at the juno-kvm entries - they contain all the boot messages
from initial power up of the juno. I'll send you a link in private
mail so googlebot doesn't end up hammering on it after it expires.
--
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
* [PATCH v2 1/5] ARM: memory: da8xx-ddrctl: new driver
From: Sudeep Holla @ 2016-11-21 17:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <d0f3bc66-6dfd-dcdc-a15d-a8f9fdda6048@arm.com>
Hi Robin,
On 21/11/16 17:47, Robin Murphy wrote:
> Hi Bartosz, Sekhar,
>
> On 21/11/16 16:48, Bartosz Golaszewski wrote:
[...]
>> Hi Sekhar,
>>
>> thanks for spotting that.
>>
>> I think we should introduce this function right away, rather than
>> having two static functions doing the same thing. If you don't mind,
>> I'll try to find a good spot for it and send a follow-up series fixing
>> the issue.
>
> As it happens, that was already proposed last week, for much the same
> reason:
>
> http://www.mail-archive.com/linuxppc-dev at lists.ozlabs.org/msg111395.html
>
Thanks for pointing this out, yet another platform to move to the new
API after v4.10.
Hi Shekar, Bartosz,
For v4.10, please continue with the open coding as proposed in this
thread in order to avoid cross tree dependencies. I will rework on the
above patch once v4.10 merge window closes to include all these
occurrence and replace them.
--
Regards,
Sudeep
^ permalink raw reply
* [linux-sunxi] [PATCH v6 0/5] drm: sun8i: Add DE2 HDMI video support
From: Jean-Francois Moine @ 2016-11-21 18:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1c9a02ff-ce6c-7ad7-36fa-8a2ea0b7675e@megous.com>
On Mon, 21 Nov 2016 01:54:53 +0100
Ond?ej Jirman <megous@megous.com> wrote:
> Dne 20.11.2016 v 12:32 Jean-Francois Moine napsal(a):
> > This patchset series adds HDMI video support to the Allwinner
> > sun8i SoCs which include the display engine 2 (DE2).
> > The driver contains the code for the A83T and H3, but it could be
> > used/extended for other SoCs as the A64, H2 and H5.
>
> Hi,
>
> I'm trying to test your patches on Orange Pi PC, and I've run into a few
> issues: (I'm using sunxi-ng with the same patches as last time, to make
> it work with your driver)
>
> 1] I just get pink output on the monitor - there's some signal, but it's
> pink (or more like magenta).
>
> dmesg ouput indicates no error:
>
> [ 1.887823] [drm] Initialized
> [ 1.888503] sun8i-de2 1000000.de-controller: bound
> 1c0c000.lcd-controller (ops 0xc0a63894)
> [ 2.057298] sun8i-de2 1000000.de-controller: bound 1ee0000.hdmi (ops
> 0xc0a63b54)
> [ 2.057304] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
> [ 2.057307] [drm] No driver support for vblank timestamp query.
> [ 2.690862] Console: switching to colour frame buffer device 240x67
> [ 2.723059] sun8i-de2 1000000.de-controller: fb0: frame buffer device
[snip]
My H3 boards work correctly, except the Orange PI 2 when it cannot read
the EDID (but it is OK after reboot).
Did you check if the EDID was correctly read?
Which resolution do you expect?
--
Ken ar c'henta? | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
^ permalink raw reply
* [BUG] hdlcd gets confused about base address
From: Russell King - ARM Linux @ 2016-11-21 18:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161121175602.GM1041@n2100.armlinux.org.uk>
On Mon, Nov 21, 2016 at 05:56:02PM +0000, Russell King - ARM Linux wrote:
> For me, the image shift was 100% reproducable. With the above patch
> and a call to drm_crtc_vblank_on() in the enable path, it seems to
> behave correctly - I can alternately switch between 1920x1080 and
> 1280x1024 and it behaves correctly. Indeed, my debug prints show that
> the right thing is happening wrt disabling the controller:
Here's my version of your patch:
8<=============
From: Russell King <rmk+kernel@armlinux.org.uk>
Subject: [PATCH] drm/arm: hdlcd: fix plane base address update
While testing HDMI with Xorg on the Juno board, I find that when Xorg
starts up or shuts down, the display is shifted significantly to the
right and wrapped in the active region. (No sync bars are visible.)
The timings are correct, it behaves as if the start address has been
shifted many pixels _into_ the framebuffer.
This occurs whenever the display mode size is changed - using xrandr
in Xorg shows that changing the resolution triggers the problem
almost every time, but changing the refresh rate does not.
Using devmem2 to disable and re-enable the HDLCD resolves the issue,
and repeated disable/enable cycles do not make the issue re-appear.
Further debugging shows that we try to update the controller
configuration while enabled.
Alwys ensure that the HDLCD is disabled prior to updating the
controller timings, and use drm_crtc_vblank_off()/drm_crtc_vblank_on()
so that DRM knows whether it can expect vblank interrupts.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/gpu/drm/arm/hdlcd_crtc.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
index c239616f5334..9d683be2e5d3 100644
--- a/drivers/gpu/drm/arm/hdlcd_crtc.c
+++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
@@ -151,15 +151,14 @@ static void hdlcd_crtc_enable(struct drm_crtc *crtc)
clk_prepare_enable(hdlcd->clk);
hdlcd_crtc_mode_set_nofb(crtc);
hdlcd_write(hdlcd, HDLCD_REG_COMMAND, 1);
+ drm_crtc_vblank_on(crtc);
}
static void hdlcd_crtc_disable(struct drm_crtc *crtc)
{
struct hdlcd_drm_private *hdlcd = crtc_to_hdlcd_priv(crtc);
- if (!crtc->state->active)
- return;
-
+ drm_crtc_vblank_off(crtc);
hdlcd_write(hdlcd, HDLCD_REG_COMMAND, 0);
clk_disable_unprepare(hdlcd->clk);
}
--
2.7.4
--
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 related
* [BUG] hdlcd gets confused about base address
From: Liviu Dudau @ 2016-11-21 18:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161121175602.GM1041@n2100.armlinux.org.uk>
On Mon, Nov 21, 2016 at 05:56:02PM +0000, Russell King - ARM Linux wrote:
> On Mon, Nov 21, 2016 at 05:32:32PM +0000, Liviu Dudau wrote:
> > On Mon, Nov 21, 2016 at 02:03:49PM +0000, Russell King - ARM Linux wrote:
> > > On Mon, Nov 21, 2016 at 01:50:31PM +0000, Liviu Dudau wrote:
> > > > On Mon, Nov 21, 2016 at 01:24:19PM +0000, Russell King - ARM Linux wrote:
> > > > > On Mon, Nov 21, 2016 at 12:56:53PM +0000, Liviu Dudau wrote:
> > > > > > That is mostly due to the check in hdlcd_crtc_disable() which I should
> > > > > > remove, I've added it because I was getting a ->disable() hook call
> > > > > > before any ->enable() was called at startup time. I need to revisit
> > > > > > this as I remember Daniel was commenting that this was not needed.
> > > > >
> > > > > Removing that test results in:
> > > > >
> > > > > [drm:drm_atomic_helper_commit_cleanup_done] *ERROR* [CRTC:24:crtc-0] flip_done timed out
> > > > >
> > > > > and the kernel hanging, seemingly in an IRQs-off region.
> > > >
> > > > Right, I need to sort this one out. Are you doing these tests out of
> > > > some tagged branch that I can get in sync with?
> >
> > Hi Russell,
> >
> > >
> > > No, not yet, and some of the changes I have are rather hacky.
> > >
> > > I do always build my full tree of patches (which is currently running at
> > > around 320 patches at the moment) but I never share that entire patch
> > > set. However, none of those touch i2c (apart from the ones I've recently
> > > posted) and the only patches touching hdlcd are those I've posted so far.
> > >
> > > Most of the problems I'm finding are through trying basic stuff - I'm not
> > > doing anything special or unusual to find them, at the moment quite
> > > literally just starting Xorg up and shutting it down. For example, the
> > > above was caused by logging in on serial, running:
> > >
> > > Xorg -terminate -verbose
> > >
> > > and then hitting ^C. (I have lxdm disabled so systemd boots to VT login
> > > prompts on both the "framebuffer" and serial - I don't want Xorg coming
> > > up when the machine is booting for its nightly KVM boot tests.)
> > >
> > > I'm afraid that when I try someone elses code, I have a tendency to find
> > > loads of seemingly trivial bugs when I try putting it through some basic
> > > tests.
> >
> > I'm not being able to reproduce your bug conditions. I'm running the following
> > setup when testing:
> >
> > - mainline v4.9-rc6
> > - edited the juno-base.dtsi file to disable the hdlcd at 7f600000 and
> > hdmi-transmitter at 70 nodes to remove the second HDMI output from the test.
> > - patched tda998x_drv.c to set interlace_allowed = 0, see below why
> > - modified the hdlcd_crtc.c file with the following patch:
> >
> > -8<-----------------------------------------------------------------------
> > diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
> > index 48019ae..656dc43 100644
> > --- a/drivers/gpu/drm/arm/hdlcd_crtc.c
> > +++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
> > @@ -156,9 +156,7 @@ static void hdlcd_crtc_disable(struct drm_crtc *crtc)
> > {
> > struct hdlcd_drm_private *hdlcd = crtc_to_hdlcd_priv(crtc);
> >
> > - if (!crtc->state->active)
> > - return;
> > -
> > + drm_crtc_vblank_off(crtc);
>
> Don't you need a drm_crtc_vblank_on() call in the enable function?
I do, thanks for calling me on that!
>
> > hdlcd_write(hdlcd, HDLCD_REG_COMMAND, 0);
> > clk_disable_unprepare(hdlcd->clk);
> > }
> > ->8-----------------------------------------------------------------------
> >
> > That takes care of the pxlclk refcounting issue you were seeing. I've started
> > Xorg several times (and yes, I do see EDID checksum error every now and then,
> > specially when running xrandr). When closing down Xorg I get back the framebuffer
> > console with the login prompt and no image shifting.
>
> For me, the image shift was 100% reproducable. With the above patch
> and a call to drm_crtc_vblank_on() in the enable path, it seems to
> behave correctly - I can alternately switch between 1920x1080 and
> 1280x1024 and it behaves correctly. Indeed, my debug prints show that
> the right thing is happening wrt disabling the controller:
OK, so I'll take it that you did not also use your patch to fix the base plane
calculations, or was that included as well in your stack?
>
> [ 76.869136] hdlcd_crtc_disable: active 0
> [ 76.869159] hdlcd_plane_atomic_update: pitch 7680 lines 1080
> [ 76.888983] hdlcd_plane_atomic_update: pitch 5120 lines 1024
> [ 76.888995] hdlcd_crtc_enable: active 1 cmd 00000000
> [ 85.262451] hdlcd_crtc_disable: active 0
> [ 85.262474] hdlcd_plane_atomic_update: pitch 5120 lines 1024
> [ 85.286667] hdlcd_plane_atomic_update: pitch 7680 lines 1080
> [ 85.286679] hdlcd_crtc_enable: active 1 cmd 00000000
> [ 92.658038] hdlcd_crtc_disable: active 0
> [ 92.658057] hdlcd_plane_atomic_update: pitch 7680 lines 1080
> [ 92.680659] hdlcd_plane_atomic_update: pitch 5120 lines 1024
> [ 92.680668] hdlcd_crtc_enable: active 1 cmd 00000000
> [ 97.805205] hdlcd_crtc_disable: active 0
> [ 97.805220] hdlcd_plane_atomic_update: pitch 5120 lines 1024
> [ 97.834415] hdlcd_plane_atomic_update: pitch 7680 lines 1080
> [ 97.834423] hdlcd_crtc_enable: active 1 cmd 00000000
>
> > My monitor is a TV that
> > reports that preferred mode is 1080i, however HDLCD and TDA19988 don't talk
> > propertly with each other to be able to set the interlaced mode correctly, so
> > I've had to disable support for interlacing mode in tda998x_drv.c and now the
> > preferred mode that gets picked up is 1920x1200 at 60Hz.
>
> That's more of a generic DRM issue - the CRTC layer doesn't get a
> look-in when a connector parses the modes supplied from the display,
> so there's no real way for the CRTC layer to apply any kind of
> limitations to the available modes, except when a mode is attempted
> to be set.
>
> I don't want to see an "interlace" DT property introduced for the
> TDA998x, because that's the wrong approach - it would be adding a
> property for the needs of the implementation, and not a description
> of the hardware.
AFAICT the issue is the fact that while HDLCD could scan out the alternate
lines with a bit of a convoluted hack, there is no way to tell TDA19988
to generate the interlaced timings. And no, I'm not advocating introducing
a DT property as this is a runtime mode, depending on the resolution
selected by userspace.
>
> > Please advise on what other steps I can take to try to reproduce this.
>
> I guess if you've tried and failed to reproduce it, there is something
> very specific to my setup which I can't describe.
>
> > P.S: What revision of Juno do you have? Any chance you can capture the start
> > of the boot process where the firmware component prints the version numbers?
>
> All I know is that it's a HBI0282B, which thanks to Sudeep's efforts
> (he spent _all_ of last Tuesday logged in to one of my systems trying
> to upgrade the firmware) is now running the Linaro 16.10 firmware.
> Sudeep says that my hardware is a very early revision which went out
> the door without being properly calibrated.
:) That is what you get for having special access to early samples of hardware :)
Might be worth asking your ARM contacts if a swap with an R2 is possible.
But, yes, my tests were also run on a Juno R0 (HBI0282B is just the R0 code)
and mine I'm pretty sure is earlier build than yours (board #13 no less).
>
> Whether that has any bearing on the reproducability of this or not, I've
> no idea.
The one factor that could affect it is the capability of the SCP firmware
to generate the exact pixel clock for your 1080p mode. If it doesn't, then
restoring the old mode might lead to an incorrect synchronisation with the
TDA chip. Current (less than 1.5 years old I guess) SCP firmware has that
sorted via an hdlcd.dat file that pre-calculates a lot of common pixel clock
frequencies).
Best regards,
Liviu
>
> If you want to see the boot messages, head to my autobuilder status page
> and look at the juno-kvm entries - they contain all the boot messages
> from initial power up of the juno. I'll send you a link in private
> mail so googlebot doesn't end up hammering on it after it expires.
>
> --
> 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.
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
?\_(?)_/?
^ permalink raw reply
* [BUG] hdlcd gets confused about base address
From: Liviu Dudau @ 2016-11-21 18:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161121181616.GN1041@n2100.armlinux.org.uk>
On Mon, Nov 21, 2016 at 06:16:16PM +0000, Russell King - ARM Linux wrote:
> On Mon, Nov 21, 2016 at 05:56:02PM +0000, Russell King - ARM Linux wrote:
> > For me, the image shift was 100% reproducable. With the above patch
> > and a call to drm_crtc_vblank_on() in the enable path, it seems to
> > behave correctly - I can alternately switch between 1920x1080 and
> > 1280x1024 and it behaves correctly. Indeed, my debug prints show that
> > the right thing is happening wrt disabling the controller:
>
> Here's my version of your patch:
Thanks! I'll add it to my tree and see if David Airlie is happy to push it
this late into the release cycle. Otherwise it is going to end up in linux-next
quickly and then in drm-next before v4.10.
>
> 8<=============
> From: Russell King <rmk+kernel@armlinux.org.uk>
> Subject: [PATCH] drm/arm: hdlcd: fix plane base address update
>
> While testing HDMI with Xorg on the Juno board, I find that when Xorg
> starts up or shuts down, the display is shifted significantly to the
> right and wrapped in the active region. (No sync bars are visible.)
> The timings are correct, it behaves as if the start address has been
> shifted many pixels _into_ the framebuffer.
>
> This occurs whenever the display mode size is changed - using xrandr
> in Xorg shows that changing the resolution triggers the problem
> almost every time, but changing the refresh rate does not.
>
> Using devmem2 to disable and re-enable the HDLCD resolves the issue,
> and repeated disable/enable cycles do not make the issue re-appear.
> Further debugging shows that we try to update the controller
> configuration while enabled.
>
> Alwys ensure that the HDLCD is disabled prior to updating the
> controller timings, and use drm_crtc_vblank_off()/drm_crtc_vblank_on()
> so that DRM knows whether it can expect vblank interrupts.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Acked-by: Liviu Dudau <Liviu.Dudau@arm.com>
> ---
> drivers/gpu/drm/arm/hdlcd_crtc.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
> index c239616f5334..9d683be2e5d3 100644
> --- a/drivers/gpu/drm/arm/hdlcd_crtc.c
> +++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
> @@ -151,15 +151,14 @@ static void hdlcd_crtc_enable(struct drm_crtc *crtc)
> clk_prepare_enable(hdlcd->clk);
> hdlcd_crtc_mode_set_nofb(crtc);
> hdlcd_write(hdlcd, HDLCD_REG_COMMAND, 1);
> + drm_crtc_vblank_on(crtc);
> }
>
> static void hdlcd_crtc_disable(struct drm_crtc *crtc)
> {
> struct hdlcd_drm_private *hdlcd = crtc_to_hdlcd_priv(crtc);
>
> - if (!crtc->state->active)
> - return;
> -
> + drm_crtc_vblank_off(crtc);
> hdlcd_write(hdlcd, HDLCD_REG_COMMAND, 0);
> clk_disable_unprepare(hdlcd->clk);
> }
> --
> 2.7.4
>
> --
> 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.
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
?\_(?)_/?
^ permalink raw reply
* [linux-sunxi] [PATCH v6 0/5] drm: sun8i: Add DE2 HDMI video support
From: Ondřej Jirman @ 2016-11-21 18:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161121191416.706b80c25d476fa002a001b1@free.fr>
Dne 21.11.2016 v 19:14 Jean-Francois Moine napsal(a):
> On Mon, 21 Nov 2016 01:54:53 +0100
> Ond?ej Jirman <megous@megous.com> wrote:
>
>> Dne 20.11.2016 v 12:32 Jean-Francois Moine napsal(a):
>>> This patchset series adds HDMI video support to the Allwinner
>>> sun8i SoCs which include the display engine 2 (DE2).
>>> The driver contains the code for the A83T and H3, but it could be
>>> used/extended for other SoCs as the A64, H2 and H5.
>>
>> Hi,
>>
>> I'm trying to test your patches on Orange Pi PC, and I've run into a few
>> issues: (I'm using sunxi-ng with the same patches as last time, to make
>> it work with your driver)
>>
>> 1] I just get pink output on the monitor - there's some signal, but it's
>> pink (or more like magenta).
>>
>> dmesg ouput indicates no error:
>>
>> [ 1.887823] [drm] Initialized
>> [ 1.888503] sun8i-de2 1000000.de-controller: bound
>> 1c0c000.lcd-controller (ops 0xc0a63894)
>> [ 2.057298] sun8i-de2 1000000.de-controller: bound 1ee0000.hdmi (ops
>> 0xc0a63b54)
>> [ 2.057304] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
>> [ 2.057307] [drm] No driver support for vblank timestamp query.
>> [ 2.690862] Console: switching to colour frame buffer device 240x67
>> [ 2.723059] sun8i-de2 1000000.de-controller: fb0: frame buffer device
> [snip]
>
> My H3 boards work correctly, except the Orange PI 2 when it cannot read
> the EDID (but it is OK after reboot).
>
> Did you check if the EDID was correctly read?
EDID is correctly read (I verified that it is the same as with the v5
version of the driver), but there's one difference I noted. v5 says dpms
is Off, while v6 says dpms is On.
> Which resolution do you expect?
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161121/1453ca53/attachment.sig>
^ permalink raw reply
* [BUG] hdlcd gets confused about base address
From: Russell King - ARM Linux @ 2016-11-21 18:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161121182324.GP1005@e106497-lin.cambridge.arm.com>
On Mon, Nov 21, 2016 at 06:23:24PM +0000, Liviu Dudau wrote:
> On Mon, Nov 21, 2016 at 05:56:02PM +0000, Russell King - ARM Linux wrote:
> > For me, the image shift was 100% reproducable. With the above patch
> > and a call to drm_crtc_vblank_on() in the enable path, it seems to
> > behave correctly - I can alternately switch between 1920x1080 and
> > 1280x1024 and it behaves correctly. Indeed, my debug prints show that
> > the right thing is happening wrt disabling the controller:
>
> OK, so I'll take it that you did not also use your patch to fix the base
> plane calculations, or was that included as well in your stack?
It was before that patch - so it was using crtc_x and crtc_y. However,
I can guarantee that those were both zero (as I've previously
described.)
> > That's more of a generic DRM issue - the CRTC layer doesn't get a
> > look-in when a connector parses the modes supplied from the display,
> > so there's no real way for the CRTC layer to apply any kind of
> > limitations to the available modes, except when a mode is attempted
> > to be set.
> >
> > I don't want to see an "interlace" DT property introduced for the
> > TDA998x, because that's the wrong approach - it would be adding a
> > property for the needs of the implementation, and not a description
> > of the hardware.
>
> AFAICT the issue is the fact that while HDLCD could scan out the alternate
> lines with a bit of a convoluted hack, there is no way to tell TDA19988
> to generate the interlaced timings. And no, I'm not advocating introducing
> a DT property as this is a runtime mode, depending on the resolution
> selected by userspace.
The TDA998x doesn't "generate" the timings. They come from the input
to it, the TDA998x merely tracks where it is within the frame, so it
knows where it can place things like the infoframes and other data.
So, the responsibility for generating the interlaced timings is with
the CRTC.
That means the CRTC needs to not only scan out alternate lines (which
is the easy bit - setting the pitch to twice the value) but it also
needs to be able to adjust the timing of the vertical sync by half
a line. The HDLCD from what I can see does not support that, the
overall system does not support for interlaced modes.
> > Whether that has any bearing on the reproducability of this or not, I've
> > no idea.
>
> The one factor that could affect it is the capability of the SCP firmware
> to generate the exact pixel clock for your 1080p mode. If it doesn't, then
> restoring the old mode might lead to an incorrect synchronisation with the
> TDA chip. Current (less than 1.5 years old I guess) SCP firmware has that
> sorted via an hdlcd.dat file that pre-calculates a lot of common pixel clock
> frequencies).
The TDA998x takes the sync signals itself to synchronise with the CRTC,
and the pixel clock had better be synchronous with the data being closed
out of the CRTC otherwise its going to be in violation of the RGB data
setup and hold timings - which will cause random colour errors.
That isn't what's going on here - the image is rock stable, it's just
shifted.
I tried inverting the sync signals from the CRTC to the TDA998x, and
that shifts the display (as I expect, because the TDA998x synchronises
on the transition of the sync signals not on their absolute values) and
at that point I get the black sync bars appearing - again as expected.
Same kind of effect if I swap the horizontal front and back porches.
Of course, adjusting such things necessitates the TDA998x to re-lock
to the CRTC each time something like that changes, and the image
shift remains.
As I described originally, the _only_ two things that solved the image
shift was (a) shifting the framebuffer start address earlier than it
should be, or (b) disabling the CRTC and re-enabling the CRTC. Both
of those were tried using devmem2 in userspace with no patches to the
HDLCD code over v4.9-rc5.
The only patches that would be in effect are my TDA998x patch stack
(which you've already tested), the i2c-designware patches to sort that
crappy thing out, and a dirty patch to the TDA998x code to read the
EDID in 16 byte chunks [*], so that the i2c-designware crappage never
causes a problem.
* - I'm not submitting this patch, because while it may solve the
EDID reading issue on Juno, it's putting intimate knowledge of
i2c-designware into the TDA998x driver - it's a hack around the
problem, it's not a real fix. It's possible that there are other
i2c-designware crappages out there which have even smaller FIFOs
which would need us to read in even smaller chunks for reliability.
--
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
* [RFC PATCH v2 4/8] arm64: compat: Add a 32-bit vDSO
From: Catalin Marinas @ 2016-11-21 18:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <f5ff1b88-aba3-2c3a-f65f-66a2e00f4764@arm.com>
On Mon, Nov 21, 2016 at 03:45:55PM +0000, Kevin Brodsky wrote:
> On 04/11/16 20:03, Catalin Marinas wrote:
> >On Fri, Oct 28, 2016 at 11:20:07AM +0100, Kevin Brodsky wrote:
> >>On 28/10/16 04:09, Jisheng Zhang wrote:
> >>>On Thu, 27 Oct 2016 17:30:54 +0100 Kevin Brodsky wrote:
> >>>>+# Force -O2 to avoid libgcc dependencies
> >>>>+VDSO_CFLAGS := -march=armv8-a -O2
> >>>
> >>>For completeness, bringing 32bit compiler need to check whether the 32bit
> >>>toolchain support some options. IIRC, armv8-a support isn't enabled until
> >>>gcc 4.8, so old toolchains such gcc-4.7 will complain:
> >>> error: unrecognized argument in option ?-march=armv8-a?
> >>
> >>That's a fair point. I guess -march=armv8-a is not strictly necessary and
> >>the produced vDSO should be fine if arch/arm/vdso also compiles fine.
> >>However we would still need to pass -march=armv7-a. I'm not sure what to do
> >>between:
> >>* Checking that the compiler supports -march=armv8-a when inspecting
> >>CROSS_COMPILE_ARM32, and if it doesn't vdso32 will not be built.
> >>* Checking whether -march=armv8-a is available here, and if it is not fall
> >>back to -march=armv7-a.
> >
> >Does v8 vs v7 make any difference in the generated code? If not, we
> >could just stick to armv7-a permanently.
>
> I've just tried compiling with -march=armv7-a, and in fact it doesn't
> compile at all. It turns out vgettimeofday.c uses smp_rmb(), which expands
> to dmb ishld on arm64, and ishld doesn't exist in ARMv7. We could possibly
> work around that, but I think requiring GCC 4.8 is reasonable.
Since vgettimeofday.c is meant to be compiled for AArch32, it wouldn't
look too bad to define its own barriers rather than relying on the
AArch64 ones. So you could define v7_smp_rmb/v7_smp_wmb and use them in
this file. Alternatively, replace smp_rmb() with smp_mb() in this file
but with a big comment about ARMv7 compilation requirement and "ishld"
not being available.
--
Catalin
^ permalink raw reply
* [PATCH 0/2] ARM: davinvi: da850 add ohci DT nodes
From: Axel Haslam @ 2016-11-21 18:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4095b7bc-a6dd-144b-4a65-40d86c710087@lechnology.com>
On Mon, Nov 21, 2016 at 6:45 PM, David Lechner <david@lechnology.com> wrote:
> On 11/21/2016 11:29 AM, Axel Haslam wrote:
>>
>> On Mon, Nov 21, 2016 at 6:04 PM, David Lechner <david@lechnology.com>
>> wrote:
>>>
>>> On 11/21/2016 10:59 AM, Axel Haslam wrote:
>>>>
>>>>
>>>> This adds the DT node for the ohci controller and
>>>> enables it for the omapl138-lckd platform.
>>>>
>>>> DEPENDENCIES:
>>>>
>>>> 1. [PATCH v6 0/5] USB: ohci-da8xx: Add device tree support
>>>> https://lkml.org/lkml/2016/11/21/558
>>>>
>>>> 2. [PATCH v3 0/2] regulator: handling of error conditions for usb
>>>> drivers
>>>> https://lkml.org/lkml/2016/11/4/465
>>>>
>>>> Axel Haslam (2):
>>>> ARM: dts: da850: Add usb device node
>>>> ARM: dts: da850-lcdk: Enable ohci for omapl138 lcdk
>>>>
>>>> arch/arm/boot/dts/da850-lcdk.dts | 8 ++++++++
>>>> arch/arm/boot/dts/da850.dtsi | 8 ++++++++
>>>> 2 files changed, 16 insertions(+)
>>>>
>>>
>>> It does not look like you rebased these patches. Sekhar pushed the musb
>>> counterpart to v4.10/dt yesterday, which will cause conflicts with this
>>> series.
>>>
>>>
>>> https://git.kernel.org/cgit/linux/kernel/git/nsekhar/linux-davinci.git/log/?h=v4.10/dt
>>
>>
>> Hi David,
>>
>> i verified that they apply to the current linux-davinci/master.
>> Anyways, i can rebase
>> and resend once the dependencies are met, and we are ready to merge it.
>>
>> Regards
>> Axel.
>>
>
> OK. I think the first patch is fine, but you will have two &usb_phy {
> status = "okay"; }; in da850-lcdk.dts now even if applies cleanly. ;-)
mmm, right! ill remove it when i resend.
^ permalink raw reply
* [PATCH v2 1/4] spi: spi-fsl-dspi: Fix SPI transfer issue when using multiple SPI_IOC_MESSAGE
From: maitysanchayan at gmail.com @ 2016-11-21 19:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161121191847.vg32cwople4qmini@sirena.org.uk>
Hello Mark,
On 16-11-21 19:18:47, Mark Brown wrote:
> On Mon, Nov 21, 2016 at 11:24:01AM +0530, Sanchayan Maity wrote:
> > Current DMA implementation had a bug where the DMA transfer would
> > exit the loop in dspi_transfer_one_message after the completion of
> > a single transfer. This results in a multi message transfer submitted
> > with SPI_IOC_MESSAGE to terminate incorrectly without an error.
>
> Please don't resend already applied patches. If there are any changes
> needed please send incremental changes on top of what's already applied.
This is not a resend of an applied patch. The whole series applies on
top of your topic/fsl-dspi branch and has fixes for the SPI DMA as
incremental changes
- Sanchaya.
^ permalink raw reply
* [PATCH v2 1/4] spi: spi-fsl-dspi: Fix SPI transfer issue when using multiple SPI_IOC_MESSAGE
From: Mark Brown @ 2016-11-21 19:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <bbdbc8df434dd2af74eb351b799a2812a1c1967e.1479706671.git.maitysanchayan@gmail.com>
On Mon, Nov 21, 2016 at 11:24:01AM +0530, Sanchayan Maity wrote:
> Current DMA implementation had a bug where the DMA transfer would
> exit the loop in dspi_transfer_one_message after the completion of
> a single transfer. This results in a multi message transfer submitted
> with SPI_IOC_MESSAGE to terminate incorrectly without an error.
Please don't resend already applied patches. If there are any changes
needed please send incremental changes on top of what's already applied.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 455 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161121/59a1e93d/attachment.sig>
^ permalink raw reply
* [PATCH v2 1/4] spi: spi-fsl-dspi: Fix SPI transfer issue when using multiple SPI_IOC_MESSAGE
From: maitysanchayan at gmail.com @ 2016-11-21 19:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161121191430.GA24521@Sanchayan-Arch.localdomain>
Hello Mark,
On 16-11-22 00:44:30, maitysanchayan at gmail.com wrote:
> Hello Mark,
>
> On 16-11-21 19:18:47, Mark Brown wrote:
> > On Mon, Nov 21, 2016 at 11:24:01AM +0530, Sanchayan Maity wrote:
> > > Current DMA implementation had a bug where the DMA transfer would
> > > exit the loop in dspi_transfer_one_message after the completion of
> > > a single transfer. This results in a multi message transfer submitted
> > > with SPI_IOC_MESSAGE to terminate incorrectly without an error.
> >
> > Please don't resend already applied patches. If there are any changes
> > needed please send incremental changes on top of what's already applied.
>
> This is not a resend of an applied patch. The whole series applies on
> top of your topic/fsl-dspi branch and has fixes for the SPI DMA as
> incremental changes
>
Sorry. I take that back. I now see you applied the patch and I got the applied
mail after I replied.
- Sanchayan.
^ permalink raw reply
* Applied "spi: spi-fsl-dspi: Fix SPI transfer issue when using multiple SPI_IOC_MESSAGE" to the spi tree
From: Mark Brown @ 2016-11-21 19:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <bbdbc8df434dd2af74eb351b799a2812a1c1967e.1479384571.git.maitysanchayan@gmail.com>
The patch
spi: spi-fsl-dspi: Fix SPI transfer issue when using multiple SPI_IOC_MESSAGE
has been applied to the spi tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
>From 9811430465fccae17862213d07eba017c149eb9c Mon Sep 17 00:00:00 2001
From: Sanchayan Maity <maitysanchayan@gmail.com>
Date: Thu, 17 Nov 2016 17:46:48 +0530
Subject: [PATCH] spi: spi-fsl-dspi: Fix SPI transfer issue when using multiple
SPI_IOC_MESSAGE
Current DMA implementation had a bug where the DMA transfer would
exit the loop in dspi_transfer_one_message after the completion of
a single transfer. This results in a multi message transfer submitted
with SPI_IOC_MESSAGE to terminate incorrectly without an error.
Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
Reviewed-by: Stefan Agner <stefan@agner.ch>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
drivers/spi/spi-fsl-dspi.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index bc64700b514d..b1ee1f521ba0 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -714,7 +714,7 @@ static int dspi_transfer_one_message(struct spi_master *master,
SPI_RSER_TFFFE | SPI_RSER_TFFFD |
SPI_RSER_RFDFE | SPI_RSER_RFDFD);
status = dspi_dma_xfer(dspi);
- goto out;
+ break;
default:
dev_err(&dspi->pdev->dev, "unsupported trans_mode %u\n",
trans_mode);
@@ -722,9 +722,13 @@ static int dspi_transfer_one_message(struct spi_master *master,
goto out;
}
- if (wait_event_interruptible(dspi->waitq, dspi->waitflags))
- dev_err(&dspi->pdev->dev, "wait transfer complete fail!\n");
- dspi->waitflags = 0;
+ if (trans_mode != DSPI_DMA_MODE) {
+ if (wait_event_interruptible(dspi->waitq,
+ dspi->waitflags))
+ dev_err(&dspi->pdev->dev,
+ "wait transfer complete fail!\n");
+ dspi->waitflags = 0;
+ }
if (transfer->delay_usecs)
udelay(transfer->delay_usecs);
--
2.10.2
^ permalink raw reply related
* [PATCH 0/2] arm64: dts: NS2: Add sdio1, PCI phys
From: Florian Fainelli @ 2016-11-21 19:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479425103-2119-1-git-send-email-jon.mason@broadcom.com>
On 11/17/2016 03:25 PM, Jon Mason wrote:
> The second SDIO seems to have been forgotten to be enabled in the
> Northstar2 SVK dts. Also, the PCI PHY entries are missing from the PCI
> bus device tree entries.
Series applied, thanks Jon
--
Florian
^ permalink raw reply
* [PATCH] PCI: Add information about describing PCI in ACPI
From: Bjorn Helgaas @ 2016-11-21 20:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <EE11001F9E5DDD47B7634E2F8A612F2E1F929566@lhreml507-mbx>
On Mon, Nov 21, 2016 at 05:23:11PM +0000, Gabriele Paoloni wrote:
> Hi Bjorn
>
> > -----Original Message-----
> > From: linux-pci-owner at vger.kernel.org [mailto:linux-pci-
> > owner at vger.kernel.org] On Behalf Of Bjorn Helgaas
> > Sent: 21 November 2016 16:47
> > To: Gabriele Paoloni
> > Cc: Bjorn Helgaas; linux-pci at vger.kernel.org; linux-
> > acpi at vger.kernel.org; linux-kernel at vger.kernel.org; linux-arm-
> > kernel at lists.infradead.org; linaro-acpi at lists.linaro.org
> > Subject: Re: [PATCH] PCI: Add information about describing PCI in ACPI
> >
> > On Mon, Nov 21, 2016 at 08:52:52AM +0000, Gabriele Paoloni wrote:
> > > Hi Bjorn
> > >
> > > > -----Original Message-----
> > > > From: Bjorn Helgaas [mailto:helgaas at kernel.org]
> > > > Sent: 18 November 2016 17:54
> > > > To: Gabriele Paoloni
> > > > Cc: Bjorn Helgaas; linux-pci at vger.kernel.org; linux-
> > > > acpi at vger.kernel.org; linux-kernel at vger.kernel.org; linux-arm-
> > > > kernel at lists.infradead.org; linaro-acpi at lists.linaro.org
> > > > Subject: Re: [PATCH] PCI: Add information about describing PCI in
> > ACPI
> > > >
> > > > On Fri, Nov 18, 2016 at 05:17:34PM +0000, Gabriele Paoloni wrote:
> > > > > > -----Original Message-----
> > > > > > From: linux-kernel-owner at vger.kernel.org [mailto:linux-kernel-
> > > > > > owner at vger.kernel.org] On Behalf Of Bjorn Helgaas
> > > > > > Sent: 17 November 2016 18:00
> > > >
> > > > > > +Static tables like MCFG, HPET, ECDT, etc., are *not*
> > mechanisms
> > > > for
> > > > > > +reserving address space! The static tables are for things the
> > OS
> > > > > > +needs to know early in boot, before it can parse the ACPI
> > > > namespace.
> > > > > > +If a new table is defined, an old OS needs to operate
> > correctly
> > > > even
> > > > > > +though it ignores the table. _CRS allows that because it is
> > > > generic
> > > > > > +and understood by the old OS; a static table does not.
> > > > >
> > > > > Right so if my understanding is correct you are saying that
> > resources
> > > > > described in the MCFG table should also be declared in PNP0C02
> > > > devices
> > > > > so that the PNP driver can reserve these resources.
> > > >
> > > > Yes.
> > > >
> > > > > On the other side the PCI Root bridge driver should not reserve
> > such
> > > > > resources.
> > > > >
> > > > > Well if my understanding is correct I think we have a problem
> > here:
> > > > > http://lxr.free-electrons.com/source/drivers/pci/ecam.c#L74
> > > > >
> > > > > As you can see pci_ecam_create() will conflict with the pnp
> > driver
> > > > > as it will try to reserve the resources from the MCFG table...
> > > > >
> > > > > Maybe we need to rework pci_ecam_create() ?
> > > >
> > > > I think it's OK as it is.
> > > >
> > > > The pnp/system.c driver does try to reserve PNP0C02 resources, and
> > it
> > > > marks them as "not busy". That way they appear in /proc/iomem and
> > > > won't be allocated for anything else, but they can still be
> > requested
> > > > by drivers, e.g., pci/ecam.c, which will mark them "busy".
> > > >
> > > > This is analogous to what the PCI core does in
> > pci_claim_resource().
> > > > This is really a function of the ACPI/PNP *core*, which should
> > reserve
> > > > all _CRS resources for all devices (not just PNP0C02 devices). But
> > > > it's done by pnp/system.c, and only for PNP0C02, because there's a
> > > > bunch of historical baggage there.
> > > >
> > > > You'll also notice that in this case, things are out of order:
> > > > logically the pnp/system.c reservation should happen first, but in
> > > > fact the pci/ecam.c request happens *before* the pnp/system.c one.
> > > > That means the pnp/system.c one might fail and complain "[mem ...]
> > > > could not be reserved".
> > >
> > > Correct me if I am wrong...
> > >
> > > So currently we are relying on the fact that pci_ecam_create() is
> > called
> > > before the pnp driver.
> > > If the pnp driver came first we would end up in pci_ecam_create()
> > failing
> > > here:
> > > http://lxr.free-electrons.com/source/drivers/pci/ecam.c#L76
> > >
> > > I am not sure but it seems to me like a bit weak condition to rely
> > on...
> > > what about removing the error condition in pci_ecam_create() and
> > logging
> > > just a dev_info()?
> >
> > Huh. I'm confused. I *thought* it would be safe to reverse the
> > order, which would effectively be this:
> >
> > system_pnp_probe
> > reserve_resources_of_dev
> > reserve_range
> > request_mem_region([mem 0xb0000000-0xb1ffffff])
> > ...
> > pci_ecam_create
> > request_resource_conflict([mem 0xb0000000-0xb1ffffff])
> >
> >
> > but I experimented with the patch below on qemu, and it failed as you
> > predicted:
> >
> > ** res test **
> > requested [mem 0xa0000000-0xafffffff]
> > can't claim ECAM area [mem 0xa0000000-0xafffffff]: conflict with ECAM
> > PNP [mem 0xa0000000-0xafffffff]
> >
> > I expected the request_resource_conflict() to succeed since it's
> > completely contained in the "ECAM PNP" region. But I guess I don't
> > understand kernel/resource.c well enough.
>
> I think it fails because effectively the PNP driver is populating the
> iomem_resource resource tree and therefore pci_ecam_create() finds that
> it cannot add the cfg resource to the same hierarchy as it is already
> there...
Right. I'm just surprised because the PNP reservation is marked
"not busy", and a driver (e.g., ECAM) should still be able to request
the resource.
> > I'm not sure we need to fix anything yet, since we currently do the
> > ecam.c request before the system.c one, and any change there would be
> > a long ways off. If/when that *does* change, I think the correct fix
> > would be to change ecam.c so its request succeeds (by changing the way
> > it does the request, fixing kernel/resource.c, or whatever) rather
> > than to reduce the log level and ignore the failure.
>
> Well in my mind I didn't want just to make the error disappear...
> If all the resources should be reserved by the PNP driver then ideally
> we could take away request_resource_conflict() from pci_ecam_create(),
> but this would make buggy some systems with an already shipped BIOS
> that relied on pci_ecam_create() reservation rather than PNP reservation.
I don't want remove the request from ecam.c. Ideally, there should be
TWO lines in /proc/iomem: one from system.c for "pnp 00:01" or
whatever it is, and a second from ecam.c. The first is the generic
one saying "this region is consumed by a piece of hardware, so don't
put anything else here." The second is the driver-specific one saying
"PCI ECAM owns this region, nobody else can use it."
This is the same way we handle PCI BAR resources. Here are two
examples from my laptop. The first (00:08.0) only has one line:
it has a BAR that consumes address space, but I don't have a driver
for it loaded. The second (00:16.0) does have a driver loaded, so it
has a second line showing that the driver owns the space:
f124a000-f124afff : 0000:00:08.0 # from PCI core
f124d000-f124dfff : 0000:00:16.0 # from PCI core
f124d000-f124dfff : mei_me # from mei_me driver
> Just removing the error condition and converting dev_err() into
> dev_info() seems to me like accommodating already shipped BIOS images
> and flagging a reservation that is already done by somebody else
> without compromising the functionality of the PCI Root bridge driver
> (so far the only reason why I can see the error condition there is
> to catch a buggy MCFG with overlapping addresses; so if this is the
> case maybe we need to have a different diagnostic check to make sure
> that the MCFG table is alright)
Ideally I think we should end up with this:
a0000000-afffffff : pnp 00:01
a0000000-afffffff : PCI ECAM
Realistically right now we'll probably end up with only the "PCI ECAM"
line in /proc/iomem and a warning from system.c about not being able
to reserve the space.
If we ever change things to do the generic PNP reservation first, then
we should fix things so ecam.c can claim the space without an error.
^ permalink raw reply
* [kvm-unit-tests PATCH v10 0/3] ARM PMU tests
From: Wei Huang @ 2016-11-21 20:24 UTC (permalink / raw)
To: linux-arm-kernel
Changes from v9:
* Move PMCCNTR related configuration from pmu_init() to sub-tests
* Change the name of loop test function to precise_cycles_loop()
* Print out error detail for each test case in check_cpi()
* Fix cpi convertion from argv
* Change the loop calculation in measure_instrs() after cpi is fixed
Note:
1) Current KVM code has bugs in handling PMCCFILTR write. A fix (see
below) is required for this unit testing code to work correctly under
KVM mode.
https://lists.cs.columbia.edu/pipermail/kvmarm/2016-November/022134.html.
Thanks,
-Wei
Christopher Covington (3):
arm: Add PMU test
arm: pmu: Check cycle count increases
arm: pmu: Add CPI checking
arm/Makefile.common | 3 +-
arm/pmu.c | 347 ++++++++++++++++++++++++++++++++++++++++++++++++++++
arm/unittests.cfg | 19 +++
3 files changed, 368 insertions(+), 1 deletion(-)
create mode 100644 arm/pmu.c
--
1.8.3.1
^ permalink raw reply
* [kvm-unit-tests PATCH v10 1/3] arm: Add PMU test
From: Wei Huang @ 2016-11-21 20:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479759895-10042-1-git-send-email-wei@redhat.com>
From: Christopher Covington <cov@codeaurora.org>
Beginning with a simple sanity check of the control register, add
a unit test for the ARM Performance Monitors Unit (PMU).
Signed-off-by: Christopher Covington <cov@codeaurora.org>
Signed-off-by: Wei Huang <wei@redhat.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
---
arm/Makefile.common | 3 ++-
arm/pmu.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++
arm/unittests.cfg | 5 ++++
3 files changed, 81 insertions(+), 1 deletion(-)
create mode 100644 arm/pmu.c
diff --git a/arm/Makefile.common b/arm/Makefile.common
index f37b5c2..5da2fdd 100644
--- a/arm/Makefile.common
+++ b/arm/Makefile.common
@@ -12,7 +12,8 @@ endif
tests-common = \
$(TEST_DIR)/selftest.flat \
$(TEST_DIR)/spinlock-test.flat \
- $(TEST_DIR)/pci-test.flat
+ $(TEST_DIR)/pci-test.flat \
+ $(TEST_DIR)/pmu.flat
all: test_cases
diff --git a/arm/pmu.c b/arm/pmu.c
new file mode 100644
index 0000000..9d9c53b
--- /dev/null
+++ b/arm/pmu.c
@@ -0,0 +1,74 @@
+/*
+ * Test the ARM Performance Monitors Unit (PMU).
+ *
+ * Copyright (c) 2015-2016, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License version 2.1 and
+ * only version 2.1 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License
+ * for more details.
+ */
+#include "libcflat.h"
+#include "asm/barrier.h"
+
+#define PMU_PMCR_N_SHIFT 11
+#define PMU_PMCR_N_MASK 0x1f
+#define PMU_PMCR_ID_SHIFT 16
+#define PMU_PMCR_ID_MASK 0xff
+#define PMU_PMCR_IMP_SHIFT 24
+#define PMU_PMCR_IMP_MASK 0xff
+
+#if defined(__arm__)
+static inline uint32_t pmcr_read(void)
+{
+ uint32_t ret;
+
+ asm volatile("mrc p15, 0, %0, c9, c12, 0" : "=r" (ret));
+ return ret;
+}
+#elif defined(__aarch64__)
+static inline uint32_t pmcr_read(void)
+{
+ uint32_t ret;
+
+ asm volatile("mrs %0, pmcr_el0" : "=r" (ret));
+ return ret;
+}
+#endif
+
+/*
+ * As a simple sanity check on the PMCR_EL0, ensure the implementer field isn't
+ * null. Also print out a couple other interesting fields for diagnostic
+ * purposes. For example, as of fall 2016, QEMU TCG mode doesn't implement
+ * event counters and therefore reports zero event counters, but hopefully
+ * support for at least the instructions event will be added in the future and
+ * the reported number of event counters will become nonzero.
+ */
+static bool check_pmcr(void)
+{
+ uint32_t pmcr;
+
+ pmcr = pmcr_read();
+
+ printf("PMU implementer: %c\n",
+ (pmcr >> PMU_PMCR_IMP_SHIFT) & PMU_PMCR_IMP_MASK);
+ printf("Identification code: 0x%x\n",
+ (pmcr >> PMU_PMCR_ID_SHIFT) & PMU_PMCR_ID_MASK);
+ printf("Event counters: %d\n",
+ (pmcr >> PMU_PMCR_N_SHIFT) & PMU_PMCR_N_MASK);
+
+ return ((pmcr >> PMU_PMCR_IMP_SHIFT) & PMU_PMCR_IMP_MASK) != 0;
+}
+
+int main(void)
+{
+ report_prefix_push("pmu");
+
+ report("Control register", check_pmcr());
+
+ return report_summary();
+}
diff --git a/arm/unittests.cfg b/arm/unittests.cfg
index ae32a42..816f494 100644
--- a/arm/unittests.cfg
+++ b/arm/unittests.cfg
@@ -58,3 +58,8 @@ groups = selftest
[pci-test]
file = pci-test.flat
groups = pci
+
+# Test PMU support
+[pmu]
+file = pmu.flat
+groups = pmu
--
1.8.3.1
^ permalink raw reply related
* [kvm-unit-tests PATCH v10 2/3] arm: pmu: Check cycle count increases
From: Wei Huang @ 2016-11-21 20:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479759895-10042-1-git-send-email-wei@redhat.com>
From: Christopher Covington <cov@codeaurora.org>
Ensure that reads of the PMCCNTR_EL0 are monotonically increasing,
even for the smallest delta of two subsequent reads.
Signed-off-by: Christopher Covington <cov@codeaurora.org>
Signed-off-by: Wei Huang <wei@redhat.com>
---
arm/pmu.c | 156 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 156 insertions(+)
diff --git a/arm/pmu.c b/arm/pmu.c
index 9d9c53b..176b070 100644
--- a/arm/pmu.c
+++ b/arm/pmu.c
@@ -15,6 +15,9 @@
#include "libcflat.h"
#include "asm/barrier.h"
+#define PMU_PMCR_E (1 << 0)
+#define PMU_PMCR_C (1 << 2)
+#define PMU_PMCR_LC (1 << 6)
#define PMU_PMCR_N_SHIFT 11
#define PMU_PMCR_N_MASK 0x1f
#define PMU_PMCR_ID_SHIFT 16
@@ -22,6 +25,14 @@
#define PMU_PMCR_IMP_SHIFT 24
#define PMU_PMCR_IMP_MASK 0xff
+#define ID_DFR0_PERFMON_SHIFT 24
+#define ID_DFR0_PERFMON_MASK 0xf
+
+#define PMU_CYCLE_IDX 31
+
+#define NR_SAMPLES 10
+
+static unsigned int pmu_version;
#if defined(__arm__)
static inline uint32_t pmcr_read(void)
{
@@ -30,6 +41,69 @@ static inline uint32_t pmcr_read(void)
asm volatile("mrc p15, 0, %0, c9, c12, 0" : "=r" (ret));
return ret;
}
+
+static inline void pmcr_write(uint32_t value)
+{
+ asm volatile("mcr p15, 0, %0, c9, c12, 0" : : "r" (value));
+ isb();
+}
+
+static inline void pmselr_write(uint32_t value)
+{
+ asm volatile("mcr p15, 0, %0, c9, c12, 5" : : "r" (value));
+ isb();
+}
+
+static inline void pmxevtyper_write(uint32_t value)
+{
+ asm volatile("mcr p15, 0, %0, c9, c13, 1" : : "r" (value));
+}
+
+static inline uint64_t pmccntr_read(void)
+{
+ uint32_t lo, hi = 0;
+
+ if (pmu_version == 0x3)
+ asm volatile("mrrc p15, 0, %0, %1, c9" : "=r" (lo), "=r" (hi));
+ else
+ asm volatile("mrc p15, 0, %0, c9, c13, 0" : "=r" (lo));
+
+ return ((uint64_t)hi << 32) | lo;
+}
+
+static inline void pmccntr_write(uint64_t value)
+{
+ uint32_t lo, hi;
+
+ lo = value & 0xffffffff;
+ hi = (value >> 32) & 0xffffffff;
+
+ if (pmu_version == 0x3)
+ asm volatile("mcrr p15, 0, %0, %1, c9" : : "r" (lo), "r" (hi));
+ else
+ asm volatile("mcr p15, 0, %0, c9, c13, 0" : : "r" (lo));
+}
+
+static inline void pmcntenset_write(uint32_t value)
+{
+ asm volatile("mcr p15, 0, %0, c9, c12, 1" : : "r" (value));
+}
+
+/* PMCCFILTR is an obsolete name for PMXEVTYPER31 in ARMv7 */
+static inline void pmccfiltr_write(uint32_t value)
+{
+ pmselr_write(PMU_CYCLE_IDX);
+ pmxevtyper_write(value);
+ isb();
+}
+
+static inline uint32_t id_dfr0_read(void)
+{
+ uint32_t val;
+
+ asm volatile("mrc p15, 0, %0, c0, c1, 2" : "=r" (val));
+ return val;
+}
#elif defined(__aarch64__)
static inline uint32_t pmcr_read(void)
{
@@ -38,6 +112,44 @@ static inline uint32_t pmcr_read(void)
asm volatile("mrs %0, pmcr_el0" : "=r" (ret));
return ret;
}
+
+static inline void pmcr_write(uint32_t value)
+{
+ asm volatile("msr pmcr_el0, %0" : : "r" (value));
+ isb();
+}
+
+static inline uint64_t pmccntr_read(void)
+{
+ uint64_t cycles;
+
+ asm volatile("mrs %0, pmccntr_el0" : "=r" (cycles));
+ return cycles;
+}
+
+static inline void pmccntr_write(uint64_t value)
+{
+ asm volatile("msr pmccntr_el0, %0" : : "r" (value));
+}
+
+static inline void pmcntenset_write(uint32_t value)
+{
+ asm volatile("msr pmcntenset_el0, %0" : : "r" (value));
+}
+
+static inline void pmccfiltr_write(uint32_t value)
+{
+ asm volatile("msr pmccfiltr_el0, %0" : : "r" (value));
+ isb();
+}
+
+static inline uint32_t id_dfr0_read(void)
+{
+ uint32_t id;
+
+ asm volatile("mrs %0, id_dfr0_el1" : "=r" (id));
+ return id;
+}
#endif
/*
@@ -64,11 +176,55 @@ static bool check_pmcr(void)
return ((pmcr >> PMU_PMCR_IMP_SHIFT) & PMU_PMCR_IMP_MASK) != 0;
}
+/*
+ * Ensure that the cycle counter progresses between back-to-back reads.
+ */
+static bool check_cycles_increase(void)
+{
+ bool success = true;
+
+ /* init before event access, this test only cares about cycle count */
+ pmcntenset_write(1 << PMU_CYCLE_IDX);
+ pmccfiltr_write(0); /* count cycles in EL0, EL1, but not EL2 */
+ pmccntr_write(0);
+
+ pmcr_write(pmcr_read() | PMU_PMCR_LC | PMU_PMCR_C | PMU_PMCR_E);
+
+ for (int i = 0; i < NR_SAMPLES; i++) {
+ uint64_t a, b;
+
+ a = pmccntr_read();
+ b = pmccntr_read();
+
+ if (a >= b) {
+ printf("Read %"PRId64" then %"PRId64".\n", a, b);
+ success = false;
+ break;
+ }
+ }
+
+ pmcr_write(pmcr_read() & ~PMU_PMCR_E);
+
+ return success;
+}
+
+void pmu_init(void)
+{
+ uint32_t dfr0;
+
+ /* probe pmu version */
+ dfr0 = id_dfr0_read();
+ pmu_version = (dfr0 >> ID_DFR0_PERFMON_SHIFT) & ID_DFR0_PERFMON_MASK;
+ printf("PMU version: %d\n", pmu_version);
+}
+
int main(void)
{
report_prefix_push("pmu");
+ pmu_init();
report("Control register", check_pmcr());
+ report("Monotonically increasing cycle count", check_cycles_increase());
return report_summary();
}
--
1.8.3.1
^ permalink raw reply related
* [kvm-unit-tests PATCH v10 3/3] arm: pmu: Add CPI checking
From: Wei Huang @ 2016-11-21 20:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479759895-10042-1-git-send-email-wei@redhat.com>
From: Christopher Covington <cov@codeaurora.org>
Calculate the numbers of cycles per instruction (CPI) implied by ARM
PMU cycle counter values. The code includes a strict checking facility
intended for the -icount option in TCG mode in the configuration file.
Signed-off-by: Christopher Covington <cov@codeaurora.org>
Signed-off-by: Wei Huang <wei@redhat.com>
---
arm/pmu.c | 119 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
arm/unittests.cfg | 14 +++++++
2 files changed, 132 insertions(+), 1 deletion(-)
diff --git a/arm/pmu.c b/arm/pmu.c
index 176b070..129ef1e 100644
--- a/arm/pmu.c
+++ b/arm/pmu.c
@@ -104,6 +104,25 @@ static inline uint32_t id_dfr0_read(void)
asm volatile("mrc p15, 0, %0, c0, c1, 2" : "=r" (val));
return val;
}
+
+/*
+ * Extra instructions inserted by the compiler would be difficult to compensate
+ * for, so hand assemble everything between, and including, the PMCR accesses
+ * to start and stop counting. Total cycles = isb + mcr + 2*loop = 2 + 2*loop.
+ */
+static inline void precise_cycles_loop(int loop, uint32_t pmcr)
+{
+ asm volatile(
+ " mcr p15, 0, %[pmcr], c9, c12, 0\n"
+ " isb\n"
+ "1: subs %[loop], %[loop], #1\n"
+ " bgt 1b\n"
+ " mcr p15, 0, %[z], c9, c12, 0\n"
+ " isb\n"
+ : [loop] "+r" (loop)
+ : [pmcr] "r" (pmcr), [z] "r" (0)
+ : "cc");
+}
#elif defined(__aarch64__)
static inline uint32_t pmcr_read(void)
{
@@ -150,6 +169,25 @@ static inline uint32_t id_dfr0_read(void)
asm volatile("mrs %0, id_dfr0_el1" : "=r" (id));
return id;
}
+
+/*
+ * Extra instructions inserted by the compiler would be difficult to compensate
+ * for, so hand assemble everything between, and including, the PMCR accesses
+ * to start and stop counting. Total cycles = isb + msr + 2*loop = 2 + 2*loop.
+ */
+static inline void precise_cycles_loop(int loop, uint32_t pmcr)
+{
+ asm volatile(
+ " msr pmcr_el0, %[pmcr]\n"
+ " isb\n"
+ "1: subs %[loop], %[loop], #1\n"
+ " b.gt 1b\n"
+ " msr pmcr_el0, xzr\n"
+ " isb\n"
+ : [loop] "+r" (loop)
+ : [pmcr] "r" (pmcr)
+ : "cc");
+}
#endif
/*
@@ -208,6 +246,79 @@ static bool check_cycles_increase(void)
return success;
}
+/*
+ * Execute a known number of guest instructions. Only odd instruction counts
+ * greater than or equal to 3 are supported by the in-line assembly code. The
+ * control register (PMCR_EL0) is initialized with the provided value (allowing
+ * for example for the cycle counter or event counters to be reset). At the end
+ * of the exact instruction loop, zero is written to PMCR_EL0 to disable
+ * counting, allowing the cycle counter or event counters to be read at the
+ * leisure of the calling code.
+ */
+static void measure_instrs(int num, uint32_t pmcr)
+{
+ int loop = (num - 2) / 2;
+
+ assert(num >= 4 && ((num - 2) % 2 == 0));
+ precise_cycles_loop(loop, pmcr);
+}
+
+/*
+ * Measure cycle counts for various known instruction counts. Ensure that the
+ * cycle counter progresses (similar to check_cycles_increase() but with more
+ * instructions and using reset and stop controls). If supplied a positive,
+ * nonzero CPI parameter, also strictly check that every measurement matches
+ * it. Strict CPI checking is used to test -icount mode.
+ */
+static bool check_cpi(int cpi)
+{
+ uint32_t pmcr = pmcr_read() | PMU_PMCR_LC | PMU_PMCR_C | PMU_PMCR_E;
+
+ /* init before event access, this test only cares about cycle count */
+ pmcntenset_write(1 << PMU_CYCLE_IDX);
+ pmccfiltr_write(0); /* count cycles in EL0, EL1, but not EL2 */
+
+ if (cpi > 0)
+ printf("Checking for CPI=%d.\n", cpi);
+ printf("instrs : cycles0 cycles1 ...\n");
+
+ for (unsigned int i = 4; i < 300; i += 32) {
+ uint64_t avg, sum = 0;
+
+ printf("%d :", i);
+ for (int j = 0; j < NR_SAMPLES; j++) {
+ uint64_t cycles;
+
+ pmccntr_write(0);
+ measure_instrs(i, pmcr);
+ cycles = pmccntr_read();
+ printf(" %"PRId64"", cycles);
+
+ if (!cycles) {
+ printf("\ncycles not incrementing!\n");
+ return false;
+ } else if (cpi > 0 && cycles != i * cpi) {
+ printf("\nunexpected cycle count received!\n");
+ return false;
+ } else if ((cycles >> 32) != 0) {
+ /* The cycles taken by the loop above should
+ * fit in 32 bits easily. We check the upper
+ * 32 bits of the cycle counter to make sure
+ * there is no supprise. */
+ printf("\ncycle count bigger than 32bit!\n");
+ return false;
+ }
+
+ sum += cycles;
+ }
+ avg = sum / NR_SAMPLES;
+ printf(" sum=%"PRId64" avg=%"PRId64" avg_ipc=%"PRId64" "
+ "avg_cpi=%"PRId64"\n", sum, avg, i / avg, avg / i);
+ }
+
+ return true;
+}
+
void pmu_init(void)
{
uint32_t dfr0;
@@ -218,13 +329,19 @@ void pmu_init(void)
printf("PMU version: %d\n", pmu_version);
}
-int main(void)
+int main(int argc, char *argv[])
{
+ int cpi = 0;
+
+ if (argc > 1)
+ cpi = atol(argv[1]);
+
report_prefix_push("pmu");
pmu_init();
report("Control register", check_pmcr());
report("Monotonically increasing cycle count", check_cycles_increase());
+ report("Cycle/instruction ratio", check_cpi(cpi));
return report_summary();
}
diff --git a/arm/unittests.cfg b/arm/unittests.cfg
index 816f494..044d97c 100644
--- a/arm/unittests.cfg
+++ b/arm/unittests.cfg
@@ -63,3 +63,17 @@ groups = pci
[pmu]
file = pmu.flat
groups = pmu
+
+# Test PMU support (TCG) with -icount IPC=1
+[pmu-tcg-icount-1]
+file = pmu.flat
+extra_params = -icount 0 -append '1'
+groups = pmu
+accel = tcg
+
+# Test PMU support (TCG) with -icount IPC=256
+[pmu-tcg-icount-256]
+file = pmu.flat
+extra_params = -icount 8 -append '256'
+groups = pmu
+accel = tcg
--
1.8.3.1
^ permalink raw reply related
* [PATCH] ARM: lpc32xx: drop duplicate header device.h
From: Sylvain Lemieux @ 2016-11-21 20:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <31c0875cedae8fe20211b209c85b6e012fa9f6a5.1479457940.git.geliangtang@gmail.com>
On Fri, 2016-11-18 at 22:21 +0800, Geliang Tang wrote:
> Drop duplicate header device.h from phy3250.c.
>
> Signed-off-by: Geliang Tang <geliangtang@gmail.com>
> ---
> arch/arm/mach-lpc32xx/phy3250.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/arch/arm/mach-lpc32xx/phy3250.c b/arch/arm/mach-lpc32xx/phy3250.c
> index 0e4cbbe..6c52bd3 100644
> --- a/arch/arm/mach-lpc32xx/phy3250.c
> +++ b/arch/arm/mach-lpc32xx/phy3250.c
> @@ -23,7 +23,6 @@
> #include <linux/interrupt.h>
> #include <linux/irq.h>
> #include <linux/dma-mapping.h>
> -#include <linux/device.h>
> #include <linux/gpio.h>
> #include <linux/amba/bus.h>
> #include <linux/amba/clcd.h>
Reviewed-by: Sylvain Lemieux <slemieux.tyco@gmail.com>
^ permalink raw reply
* [kvm-unit-tests PATCH v10 3/3] arm: pmu: Add CPI checking
From: Christopher Covington @ 2016-11-21 21:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479759895-10042-4-git-send-email-wei@redhat.com>
Hi Wei,
On 11/21/2016 03:24 PM, Wei Huang wrote:
> From: Christopher Covington <cov@codeaurora.org>
I really appreciate your work on these patches. If for any or all of these
you have more lines added/modified than me (or using any other better
metric), please make sure to change the author to be you with
`git commit --amend --reset-author` or equivalent.
> Calculate the numbers of cycles per instruction (CPI) implied by ARM
> PMU cycle counter values. The code includes a strict checking facility
> intended for the -icount option in TCG mode in the configuration file.
>
> Signed-off-by: Christopher Covington <cov@codeaurora.org>
> Signed-off-by: Wei Huang <wei@redhat.com>
> ---
> arm/pmu.c | 119 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
> arm/unittests.cfg | 14 +++++++
> 2 files changed, 132 insertions(+), 1 deletion(-)
>
> diff --git a/arm/pmu.c b/arm/pmu.c
> index 176b070..129ef1e 100644
> --- a/arm/pmu.c
> +++ b/arm/pmu.c
> @@ -104,6 +104,25 @@ static inline uint32_t id_dfr0_read(void)
> asm volatile("mrc p15, 0, %0, c0, c1, 2" : "=r" (val));
> return val;
> }
> +
> +/*
> + * Extra instructions inserted by the compiler would be difficult to compensate
> + * for, so hand assemble everything between, and including, the PMCR accesses
> + * to start and stop counting. Total cycles = isb + mcr + 2*loop = 2 + 2*loop.
> + */
> +static inline void precise_cycles_loop(int loop, uint32_t pmcr)
Nit: I would call this precise_instrs_loop. How many cycles it takes is
IMPLEMENTATION DEFINED.
> +{
> + asm volatile(
> + " mcr p15, 0, %[pmcr], c9, c12, 0\n"
> + " isb\n"
> + "1: subs %[loop], %[loop], #1\n"
> + " bgt 1b\n"
Is there any chance we might need an isb here, to prevent the stop from happening
before or during the loop? Where ISBs are required, the Linux best practice is to
diligently comment why they are needed. Perhaps it would be a good habit to
carry over into kvm-unit-tests.
> + " mcr p15, 0, %[z], c9, c12, 0\n"
> + " isb\n"
> + : [loop] "+r" (loop)
> + : [pmcr] "r" (pmcr), [z] "r" (0)
> + : "cc");
> +}
> #elif defined(__aarch64__)
> static inline uint32_t pmcr_read(void)
> {
> @@ -150,6 +169,25 @@ static inline uint32_t id_dfr0_read(void)
> asm volatile("mrs %0, id_dfr0_el1" : "=r" (id));
> return id;
> }
> +
> +/*
> + * Extra instructions inserted by the compiler would be difficult to compensate
> + * for, so hand assemble everything between, and including, the PMCR accesses
> + * to start and stop counting. Total cycles = isb + msr + 2*loop = 2 + 2*loop.
> + */
> +static inline void precise_cycles_loop(int loop, uint32_t pmcr)
> +{
> + asm volatile(
> + " msr pmcr_el0, %[pmcr]\n"
> + " isb\n"
> + "1: subs %[loop], %[loop], #1\n"
> + " b.gt 1b\n"
> + " msr pmcr_el0, xzr\n"
> + " isb\n"
> + : [loop] "+r" (loop)
> + : [pmcr] "r" (pmcr)
> + : "cc");
> +}
> #endif
>
> /*
> @@ -208,6 +246,79 @@ static bool check_cycles_increase(void)
> return success;
> }
>
> +/*
> + * Execute a known number of guest instructions. Only odd instruction counts
> + * greater than or equal to 3 are supported by the in-line assembly code. The
Nit: needs updating as well (or removal if you prefer)
> + * control register (PMCR_EL0) is initialized with the provided value (allowing
> + * for example for the cycle counter or event counters to be reset). At the end
> + * of the exact instruction loop, zero is written to PMCR_EL0 to disable
> + * counting, allowing the cycle counter or event counters to be read at the
> + * leisure of the calling code.
> + */
> +static void measure_instrs(int num, uint32_t pmcr)
> +{
> + int loop = (num - 2) / 2;
> +
> + assert(num >= 4 && ((num - 2) % 2 == 0));
> + precise_cycles_loop(loop, pmcr);
> +}
> +
> +/*
> + * Measure cycle counts for various known instruction counts. Ensure that the
> + * cycle counter progresses (similar to check_cycles_increase() but with more
> + * instructions and using reset and stop controls). If supplied a positive,
> + * nonzero CPI parameter, also strictly check that every measurement matches
> + * it. Strict CPI checking is used to test -icount mode.
> + */
> +static bool check_cpi(int cpi)
> +{
> + uint32_t pmcr = pmcr_read() | PMU_PMCR_LC | PMU_PMCR_C | PMU_PMCR_E;
> +
> + /* init before event access, this test only cares about cycle count */
> + pmcntenset_write(1 << PMU_CYCLE_IDX);
> + pmccfiltr_write(0); /* count cycles in EL0, EL1, but not EL2 */
> +
> + if (cpi > 0)
> + printf("Checking for CPI=%d.\n", cpi);
> + printf("instrs : cycles0 cycles1 ...\n");
> +
> + for (unsigned int i = 4; i < 300; i += 32) {
> + uint64_t avg, sum = 0;
> +
> + printf("%d :", i);
> + for (int j = 0; j < NR_SAMPLES; j++) {
> + uint64_t cycles;
> +
> + pmccntr_write(0);
> + measure_instrs(i, pmcr);
> + cycles = pmccntr_read();
> + printf(" %"PRId64"", cycles);
> +
> + if (!cycles) {
> + printf("\ncycles not incrementing!\n");
> + return false;
> + } else if (cpi > 0 && cycles != i * cpi) {
> + printf("\nunexpected cycle count received!\n");
> + return false;
> + } else if ((cycles >> 32) != 0) {
> + /* The cycles taken by the loop above should
> + * fit in 32 bits easily. We check the upper
> + * 32 bits of the cycle counter to make sure
> + * there is no supprise. */
> + printf("\ncycle count bigger than 32bit!\n");
> + return false;
> + }
> +
> + sum += cycles;
> + }
> + avg = sum / NR_SAMPLES;
> + printf(" sum=%"PRId64" avg=%"PRId64" avg_ipc=%"PRId64" "
> + "avg_cpi=%"PRId64"\n", sum, avg, i / avg, avg / i);
> + }
> +
> + return true;
> +}
> +
> void pmu_init(void)
> {
> uint32_t dfr0;
> @@ -218,13 +329,19 @@ void pmu_init(void)
> printf("PMU version: %d\n", pmu_version);
> }
This is clearly the right feature register to check. Sorry to have
gotten excited about an inferior method.
Cov
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code
Aurora Forum, a Linux Foundation Collaborative Project.
^ 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