Discussion:
designware: prefetch and 64-bit addr support?
L H
2014-09-03 20:57:11 UTC
Permalink
Greetings my fellow PCI designware developers. I'm working on an
implementation that relies on the PCIe DesignWare IP and am writing a
platform-specific layer to link with the designware code, but have run
into a couple challenges.

Reading the pcie-designware.c (looking at the master branch of a repo
cloned from git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git --
is that the best branch/repo to view?), it appears to me that the
dw_pcie_host_init() code doesn't comprehend prefetch memory ranges,
but instead just assumes that a memory range is non-prefetch despite
its encoding in the device-tree. Am I reading that correctly? The
loop for walking the ranges begins with:

for_each_of_pci_range(&parser, &range) {
unsigned long restype = range.flags & IORESOURCE_TYPE_BITS;

The bitmask for IORESOURCE_TYPE_BITS would result in a restype
ignorant of the IORESOURCE_PREFETCH bit, right? The ensuing code
flow then treats the range as non-prefetch unless I missed something.

Curiously, I also noted that the for_each_of_pcie_range() which calls
of_pci_range_parser_one() that in turn includes calls to
of_bus_pci_get_flags() does not set the IORESOURCE_MEM_64 bit parsed
from the range either.

If I followed that all correctly, is the proper assumption that all of
the current implementations relying on the designware code only use
32-bit non-prefetch memory ranges?

The platform which I'm working on will use a large 64-bit PCI memory
address space, and thus I need 64-bit memory apertures which implies,
I think, only prefetch memory.

I hasten to add this isn't a complaint or critique of the valuable
work developed up to this point. I'm very grateful to have it as a
rich starting point. I'm reaching out to the list here as a newbie to
verify my comprehension of the designware code, and if valid, to
inquire if there's any effort by anyone currently to support 64-bit
prefetch ranges. I've accepted the real possibility that I might be
blazing this trail myself, but just wanted to check if anyone else has
considered it or working on this support to avoid a redundant effort.

thanks,
LH
Bjorn Helgaas
2014-09-04 03:34:40 UTC
Permalink
[+cc Mohit, Jingoo (from MAINTAINERS)]
Post by L H
Greetings my fellow PCI designware developers. I'm working on an
implementation that relies on the PCIe DesignWare IP and am writing a
platform-specific layer to link with the designware code, but have run
into a couple challenges.
Reading the pcie-designware.c (looking at the master branch of a repo
cloned from git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git --
is that the best branch/repo to view?),
That's a good place to look. If I have pending changes for that code,
they will be on the pci/host-designware branch in that same repo.
Changes in that branch will generally be merged immediately after the
next major Linux release. For example, right now we're in the middle
of the v3.17 cycle. v3.16 was released August 3, the v3.17 release
will probably be in early October, and the contents of my
pci/host-designware branch will be merged in mid-October and be
released as part of v3.18, probably in mid-December.
Post by L H
it appears to me that the
dw_pcie_host_init() code doesn't comprehend prefetch memory ranges,
but instead just assumes that a memory range is non-prefetch despite
its encoding in the device-tree. Am I reading that correctly? The
for_each_of_pci_range(&parser, &range) {
unsigned long restype = range.flags & IORESOURCE_TYPE_BITS;
The bitmask for IORESOURCE_TYPE_BITS would result in a restype
ignorant of the IORESOURCE_PREFETCH bit, right? The ensuing code
flow then treats the range as non-prefetch unless I missed something.
Curiously, I also noted that the for_each_of_pcie_range() which calls
of_pci_range_parser_one() that in turn includes calls to
of_bus_pci_get_flags() does not set the IORESOURCE_MEM_64 bit parsed
from the range either.
If I followed that all correctly, is the proper assumption that all of
the current implementations relying on the designware code only use
32-bit non-prefetch memory ranges?
The platform which I'm working on will use a large 64-bit PCI memory
address space, and thus I need 64-bit memory apertures which implies,
I think, only prefetch memory.
I hasten to add this isn't a complaint or critique of the valuable
work developed up to this point. I'm very grateful to have it as a
rich starting point. I'm reaching out to the list here as a newbie to
verify my comprehension of the designware code, and if valid, to
inquire if there's any effort by anyone currently to support 64-bit
prefetch ranges. I've accepted the real possibility that I might be
blazing this trail myself, but just wanted to check if anyone else has
considered it or working on this support to avoid a redundant effort.
thanks,
LH
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
More majordomo info at http://vger.kernel.org/majordomo-info.html
Liviu Dudau
2014-09-10 14:15:14 UTC
Permalink
Post by L H
Greetings my fellow PCI designware developers. I'm working on an
implementation that relies on the PCIe DesignWare IP and am writing a
platform-specific layer to link with the designware code, but have ru=
n
Post by L H
into a couple challenges.
=20
Reading the pcie-designware.c (looking at the master branch of a repo
cloned from git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git -=
-
Post by L H
is that the best branch/repo to view?), it appears to me that the
dw_pcie_host_init() code doesn't comprehend prefetch memory ranges,
but instead just assumes that a memory range is non-prefetch despite
its encoding in the device-tree. Am I reading that correctly? The
=20
for_each_of_pci_range(&parser, &range) {
unsigned long restype =3D range.flags & IORESOURCE_TYPE_BITS;
=20
The bitmask for IORESOURCE_TYPE_BITS would result in a restype
ignorant of the IORESOURCE_PREFETCH bit, right? The ensuing code
flow then treats the range as non-prefetch unless I missed something.
Hi LH,

You are not reading all of the code. The designware code only cares
about the type of the range (IO, MEM, cfg?). But the conversion from
range to resource that happens in of_pci_range_to_resource() *does*
preserve the prefetchable flags and that is being used by the generic
PCI framework.
Post by L H
=20
Curiously, I also noted that the for_each_of_pcie_range() which calls
of_pci_range_parser_one() that in turn includes calls to
of_bus_pci_get_flags() does not set the IORESOURCE_MEM_64 bit parsed
from the range either.
Where exactly do you expect those flags to be read into?
Post by L H
=20
If I followed that all correctly, is the proper assumption that all o=
f
Post by L H
the current implementations relying on the designware code only use
32-bit non-prefetch memory ranges?
No, I don't think that is correct. What I would say is that the current
designware PCIe code does not care whether the memory is prefetcheable
or not for the DW specific part of the setup.

Best regards,
Liviu
Post by L H
=20
The platform which I'm working on will use a large 64-bit PCI memory
address space, and thus I need 64-bit memory apertures which implies,
I think, only prefetch memory.
=20
I hasten to add this isn't a complaint or critique of the valuable
work developed up to this point. I'm very grateful to have it as a
rich starting point. I'm reaching out to the list here as a newbie t=
o
Post by L H
verify my comprehension of the designware code, and if valid, to
inquire if there's any effort by anyone currently to support 64-bit
prefetch ranges. I've accepted the real possibility that I might be
blazing this trail myself, but just wanted to check if anyone else ha=
s
Post by L H
considered it or working on this support to avoid a redundant effort.
=20
thanks,
LH
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" =
in
Post by L H
More majordomo info at http://vger.kernel.org/majordomo-info.html
=20
--=20
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
=C2=AF\_(=E3=83=84)_/=C2=AF
L H
2014-10-10 13:54:25 UTC
Permalink
Thank you Liviu for the detailed response. I've been buried with work
and out on personal matters so I'm just now able to resume
participation on this thread.

I did successfully get prefetch memory support working on my platform.
As you indicated, because the prefetchable flags are preserved in the
resource, the PCI framework handled it just fine, with, however, a
small change to dw_pcie_setup_rc(). That's because that function
originally just assumed that the memory region was non-prefetch. I
added a check for the prefetch flag in dw_pcie_setup_rc() and
initialized the root-complex's host bridge registers appropriately:


- /* setup memory base, memory limit */
- membase =3D ((u32)pp->mem_base & 0xfff00000) >> 16;
- memlimit =3D (config->mem_size + (u32)pp->mem_base) & 0xfff0000=
0;
- val =3D memlimit | membase;
- dw_pcie_writel_rc(pp, val, PCI_MEMORY_BASE);
+ if (!(pp->mem.flags & IORESOURCE_PREFETCH)) {
+ /* setup memory base, memory limit */
+ membase =3D ((u32)pp->mem_base & 0xfff00000) >> 16;
+ memlimit =3D (config->mem_size + (u32)pp->mem_base) & 0=
xfff00000;
+ val =3D memlimit | membase;
+ dw_pcie_writel_rc(pp, val, PCI_MEMORY_BASE);
+ }
+ else {
+ /* setup prefetch memory base, limit */
+ membase =3D (lower_32_bits(pp->mem_base) & 0xfff00000) =
Post by Liviu Dudau
Post by L H
16;
+ memlimit =3D (lower_32_bits(config->mem_size) +
+ lower_32_bits(pp->mem_base)) & 0xfff000=
00;
+ val =3D memlimit | membase;
+ dw_pcie_writel_rc(pp, val, PCI_PREF_MEMORY_BASE);
+ dw_pcie_writel_rc(pp, upper_32_bits(pp->mem_base),
PCI_PREF_BASE_UPPER32);
+ dw_pcie_writel_rc(pp, upper_32_bits(pp->mem_base +
config->mem_size), PCI_PREF_LIMIT_UPPER32);
+ }

Note that this, of course, follows the current driver implementation
that supports only one PCI memory region: either prefetch or
non-prefetch. For the purposes of supporting more SoC's which employ
DesignWare, the driver can easily be extended to support both but I
understand the mindset of leaving that to whomever needs multiple
memory regions.

thanks again,
LH
Post by Liviu Dudau
Post by L H
Greetings my fellow PCI designware developers. I'm working on an
implementation that relies on the PCIe DesignWare IP and am writing =
a
Post by Liviu Dudau
Post by L H
platform-specific layer to link with the designware code, but have r=
un
Post by Liviu Dudau
Post by L H
into a couple challenges.
Reading the pcie-designware.c (looking at the master branch of a rep=
o
Post by Liviu Dudau
Post by L H
cloned from git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git =
--
Post by Liviu Dudau
Post by L H
is that the best branch/repo to view?), it appears to me that the
dw_pcie_host_init() code doesn't comprehend prefetch memory ranges,
but instead just assumes that a memory range is non-prefetch despite
its encoding in the device-tree. Am I reading that correctly? The
for_each_of_pci_range(&parser, &range) {
unsigned long restype =3D range.flags & IORESOURCE_TYPE_BITS=
;
Post by Liviu Dudau
Post by L H
The bitmask for IORESOURCE_TYPE_BITS would result in a restype
ignorant of the IORESOURCE_PREFETCH bit, right? The ensuing code
flow then treats the range as non-prefetch unless I missed something=
=2E
Post by Liviu Dudau
Hi LH,
You are not reading all of the code. The designware code only cares
about the type of the range (IO, MEM, cfg?). But the conversion from
range to resource that happens in of_pci_range_to_resource() *does*
preserve the prefetchable flags and that is being used by the generic
PCI framework.
Post by L H
Curiously, I also noted that the for_each_of_pcie_range() which call=
s
Post by Liviu Dudau
Post by L H
of_pci_range_parser_one() that in turn includes calls to
of_bus_pci_get_flags() does not set the IORESOURCE_MEM_64 bit parsed
from the range either.
Where exactly do you expect those flags to be read into?
Post by L H
If I followed that all correctly, is the proper assumption that all =
of
Post by Liviu Dudau
Post by L H
the current implementations relying on the designware code only use
32-bit non-prefetch memory ranges?
No, I don't think that is correct. What I would say is that the curre=
nt
Post by Liviu Dudau
designware PCIe code does not care whether the memory is prefetcheabl=
e
Post by Liviu Dudau
or not for the DW specific part of the setup.
Best regards,
Liviu
Post by L H
The platform which I'm working on will use a large 64-bit PCI memory
address space, and thus I need 64-bit memory apertures which implies=
,
Post by Liviu Dudau
Post by L H
I think, only prefetch memory.
I hasten to add this isn't a complaint or critique of the valuable
work developed up to this point. I'm very grateful to have it as a
rich starting point. I'm reaching out to the list here as a newbie =
to
Post by Liviu Dudau
Post by L H
verify my comprehension of the designware code, and if valid, to
inquire if there's any effort by anyone currently to support 64-bit
prefetch ranges. I've accepted the real possibility that I might be
blazing this trail myself, but just wanted to check if anyone else h=
as
Post by Liviu Dudau
Post by L H
considered it or working on this support to avoid a redundant effort=
=2E
Post by Liviu Dudau
Post by L H
thanks,
LH
--
To unsubscribe from this list: send the line "unsubscribe linux-pci"=
in
Post by Liviu Dudau
Post by L H
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
=C2=AF\_(=E3=83=84)_/=C2=AF
Loading...