* Re: bit fields && data tearing
From: Benjamin Herrenschmidt @ 2014-09-03 23:11 UTC (permalink / raw)
To: Peter Hurley
Cc: Jakub Jelinek, linux-arch, Tony Luck, linux-ia64, Oleg Nesterov,
linux-kernel, Paul Mackerras, Paul E. McKenney, linuxppc-dev,
Miroslav Franc, Richard Henderson
In-Reply-To: <54079B70.4050200@hurleysoftware.com>
On Wed, 2014-09-03 at 18:51 -0400, Peter Hurley wrote:
> Apologies for hijacking this thread but I need to extend this discussion
> somewhat regarding what a compiler might do with adjacent fields in a structure.
>
> The tty subsystem defines a large aggregate structure, struct tty_struct.
> Importantly, several different locks apply to different fields within that
> structure; ie., a specific spinlock will be claimed before updating or accessing
> certain fields while a different spinlock will be claimed before updating or
> accessing certain _adjacent_ fields.
>
> What is necessary and sufficient to prevent accidental false-sharing?
> The patch below was flagged as insufficient on ia64, and possibly ARM.
We expect native aligned scalar types to be accessed atomically (the
read/modify/write of a larger quantity that gcc does on some bitfield
cases has been flagged as a gcc bug, but shouldn't happen on normal
scalar types).
I am not 100% certain of "bool" here, I assume it's treated as a normal
scalar and thus atomic but if unsure, you can always use int.
Another option is to use the atomic bitops and make these bits in a
bitmask but that is probably unnecessary if you have locks already.
Cheers,
Ben.
> Regards,
> Peter Hurley
>
> --- >% ---
> Subject: [PATCH 21/26] tty: Convert tty_struct bitfield to bools
>
> The stopped, hw_stopped, flow_stopped and packet bits are smp-unsafe
> and interrupt-unsafe. For example,
>
> CPU 0 | CPU 1
> |
> tty->flow_stopped = 1 | tty->hw_stopped = 0
>
> One of these updates will be corrupted, as the bitwise operation
> on the bitfield is non-atomic.
>
> Ensure each flag has a separate memory location, so concurrent
> updates do not corrupt orthogonal states.
>
> Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
> ---
> include/linux/tty.h | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/tty.h b/include/linux/tty.h
> index 1c3316a..7cf61cb 100644
> --- a/include/linux/tty.h
> +++ b/include/linux/tty.h
> @@ -261,7 +261,10 @@ struct tty_struct {
> unsigned long flags;
> int count;
> struct winsize winsize; /* winsize_mutex */
> - unsigned char stopped:1, hw_stopped:1, flow_stopped:1, packet:1;
> + bool stopped;
> + bool hw_stopped;
> + bool flow_stopped;
> + bool packet;
> unsigned char ctrl_status; /* ctrl_lock */
> unsigned int receive_room; /* Bytes free for queue */
> int flow_change;
^ permalink raw reply
* TTM placement & caching issue/questions
From: Benjamin Herrenschmidt @ 2014-09-04 0:12 UTC (permalink / raw)
To: dri-devel; +Cc: Dave Airlie, linuxppc-dev, Alex Deucher
Hi folks !
I've been tracking down some problems with the recent DRI on powerpc and
stumbled upon something that doesn't look right, and not necessarily
only for us.
Now it's possible that I haven't fully understood the code here and I
also don't know to what extent some of that behaviour is necessary for
some platforms such as Intel GTT bits.
What I've observed with a simple/dumb (no DMA) driver like AST (but this
probably happens more generally) is that when evicting a BO from VRAM
into System memory, the TTM tries to preserve the existing caching
attributes of the VRAM object.
>From what I can tell, we end up with going from VRAM to System memory
type, and we eventually call ttm_bo_select_caching() to select the
caching option for the target.
This will, from what I can tell, try to use the same caching mode as the
original object:
if ((cur_placement & caching) != 0)
result |= (cur_placement & caching);
And cur_placement comes from bo->mem.placement which as far as I can
tell is based on the placement array which the drivers set up.
Now they tend to uniformly setup the placement for System memory as
TTM_PL_MASK_CACHING which enables all caching modes.
So I end up with, for example, my System memory BOs having
TTM_PL_FLAG_CACHED not set (though they also don't have
TTM_PL_FLAG_UNCACHED) and TTM_PL_FLAG_WC.
We don't seem to use the man->default_caching (which will have
TTM_PL_FLAG_CACHED) unless there is no matching bit at all between the
proposed placement and the existing caching mode.
Now this is a problem for several reason that I can think of:
- On a number of powerpc platforms, such as all our server 64-bit one
for example, it's actually illegal to map system memory non-cached. The
system is fully cache coherent for all possible DMA originators (that we
care about at least) and mapping memory non-cachable while it's mapped
cachable in the linear mapping can cause nasty cache paradox which, when
detected by HW, can checkstop the system.
- A similar issue exists, afaik, on ARM >= v7, so anything mapped
non-cachable must be removed from the linear mapping explicitly since
otherwise it can be speculatively prefetched into the cache.
- I don't know about x86, but even then, it looks quite sub-optimal to
map the memory backing of the BOs and access it using a WC rather than a
cachable mapping attribute.
Now, some folks on IRC mentioned that there might be reasons for the
current behaviour as to not change the caching attributes when going
in/out of the GTT on Intel, I don't know how that relates and how that
works, but maybe that should be enforced by having a different placement
mask specifically on those chipsets.
Dave, should we change the various PCI drivers for generally coherent
devices such that the System memory type doesn't allow placements
without CACHED attribute ? Or at least on coherent platforms ? How do
detect that ? Should we have a TTM helper to establish the default
memory placement attributes that "normal PCI" drivers call to set that
up so we can have all the necessary arch ifdefs in one single place, at
least for "classic PCI/PCIe" stuff (AGP might need additional tweaks) ?
Non-PCI and "special" drivers like Intel can use a different set of
placement attributes to represent the requirements of those specific
platforms (mostly thinking of embedded ARM here which under some
circumstances might actually require non-cached mappings).
Or am I missing another part of the puzzle ?
As it-is, things are broken for me even for dumb drivers, and I suspect
to a large extent with radeon and nouveau too, though in some case we
might get away with it most of the time ... until the machine locks up
for some unexplainable reason... This might cause problems on existing
distros such as RHEL7 with our radeon adapters even.
Any suggestion of what's the best approach to fix it ? I'm happy to
produce the patches but I'm not that familiar with the TTM so I would
like to make sure I'm the right track first :-)
Cheers,
Ben.
^ permalink raw reply
* Re: bit fields && data tearing
From: Peter Hurley @ 2014-09-03 22:51 UTC (permalink / raw)
To: Jakub Jelinek, Paul E. McKenney, Richard Henderson
Cc: linux-arch, Tony Luck, linux-ia64, Oleg Nesterov, linux-kernel,
Paul Mackerras, linuxppc-dev, Miroslav Franc
In-Reply-To: <20140712181328.GA8738@redhat.com>
[ +cc linux-arch, Tony Luck,
On 07/12/2014 02:13 PM, Oleg Nesterov wrote:
> Hello,
>
> I am not sure I should ask here, but since Documentation/memory-barriers.txt
> mentions load/store tearing perhaps my question is not completely off-topic...
>
> I am fighting with mysterious RHEL bug, it can be reproduced on ppc and s390
> but not on x86. Finally I seem to understand the problem, and I even wrote the
> stupid kernel module to ensure, see it below at the end.
>
> It triggers the problem immediately, kt_2() sees the wrong value in freeze_stop.
> (If I turn ->freeze_stop int "long", the problem goes away).
>
> So the question is: is this gcc bug or the code below is buggy?
>
> If it is buggy, then probably memory-barriers.txt could mention that you should
> be carefull with bit fields, even ACCESS_ONCE() obviously can't help.
>
> Or this just discloses my ignorance and you need at least aligned(long) after a
> bit field to be thread-safe ? I thought that compiler should take care and add
> the necessary alignment if (say) CPU can't update a single byte/uint.
Apologies for hijacking this thread but I need to extend this discussion
somewhat regarding what a compiler might do with adjacent fields in a structure.
The tty subsystem defines a large aggregate structure, struct tty_struct.
Importantly, several different locks apply to different fields within that
structure; ie., a specific spinlock will be claimed before updating or accessing
certain fields while a different spinlock will be claimed before updating or
accessing certain _adjacent_ fields.
What is necessary and sufficient to prevent accidental false-sharing?
The patch below was flagged as insufficient on ia64, and possibly ARM.
Regards,
Peter Hurley
--- >% ---
Subject: [PATCH 21/26] tty: Convert tty_struct bitfield to bools
The stopped, hw_stopped, flow_stopped and packet bits are smp-unsafe
and interrupt-unsafe. For example,
CPU 0 | CPU 1
|
tty->flow_stopped = 1 | tty->hw_stopped = 0
One of these updates will be corrupted, as the bitwise operation
on the bitfield is non-atomic.
Ensure each flag has a separate memory location, so concurrent
updates do not corrupt orthogonal states.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
include/linux/tty.h | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 1c3316a..7cf61cb 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -261,7 +261,10 @@ struct tty_struct {
unsigned long flags;
int count;
struct winsize winsize; /* winsize_mutex */
- unsigned char stopped:1, hw_stopped:1, flow_stopped:1, packet:1;
+ bool stopped;
+ bool hw_stopped;
+ bool flow_stopped;
+ bool packet;
unsigned char ctrl_status; /* ctrl_lock */
unsigned int receive_room; /* Bytes free for queue */
int flow_change;
--
2.1.0
^ permalink raw reply related
* Re: TTM placement & caching issue/questions
From: Jerome Glisse @ 2014-09-04 1:55 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Alex Deucher, linuxppc-dev, Michel Dänzer, Christian Koenig,
dri-devel
In-Reply-To: <1409789547.30640.136.camel@pasglop>
On Thu, Sep 04, 2014 at 10:12:27AM +1000, Benjamin Herrenschmidt wrote:
> Hi folks !
>
> I've been tracking down some problems with the recent DRI on powerpc and
> stumbled upon something that doesn't look right, and not necessarily
> only for us.
>
> Now it's possible that I haven't fully understood the code here and I
> also don't know to what extent some of that behaviour is necessary for
> some platforms such as Intel GTT bits.
>
> What I've observed with a simple/dumb (no DMA) driver like AST (but this
> probably happens more generally) is that when evicting a BO from VRAM
> into System memory, the TTM tries to preserve the existing caching
> attributes of the VRAM object.
>
> From what I can tell, we end up with going from VRAM to System memory
> type, and we eventually call ttm_bo_select_caching() to select the
> caching option for the target.
>
> This will, from what I can tell, try to use the same caching mode as the
> original object:
>
> if ((cur_placement & caching) != 0)
> result |= (cur_placement & caching);
>
> And cur_placement comes from bo->mem.placement which as far as I can
> tell is based on the placement array which the drivers set up.
>
> Now they tend to uniformly setup the placement for System memory as
> TTM_PL_MASK_CACHING which enables all caching modes.
>
> So I end up with, for example, my System memory BOs having
> TTM_PL_FLAG_CACHED not set (though they also don't have
> TTM_PL_FLAG_UNCACHED) and TTM_PL_FLAG_WC.
>
> We don't seem to use the man->default_caching (which will have
> TTM_PL_FLAG_CACHED) unless there is no matching bit at all between the
> proposed placement and the existing caching mode.
>
> Now this is a problem for several reason that I can think of:
>
> - On a number of powerpc platforms, such as all our server 64-bit one
> for example, it's actually illegal to map system memory non-cached. The
> system is fully cache coherent for all possible DMA originators (that we
> care about at least) and mapping memory non-cachable while it's mapped
> cachable in the linear mapping can cause nasty cache paradox which, when
> detected by HW, can checkstop the system.
>
> - A similar issue exists, afaik, on ARM >= v7, so anything mapped
> non-cachable must be removed from the linear mapping explicitly since
> otherwise it can be speculatively prefetched into the cache.
>
> - I don't know about x86, but even then, it looks quite sub-optimal to
> map the memory backing of the BOs and access it using a WC rather than a
> cachable mapping attribute.
>
> Now, some folks on IRC mentioned that there might be reasons for the
> current behaviour as to not change the caching attributes when going
> in/out of the GTT on Intel, I don't know how that relates and how that
> works, but maybe that should be enforced by having a different placement
> mask specifically on those chipsets.
>
> Dave, should we change the various PCI drivers for generally coherent
> devices such that the System memory type doesn't allow placements
> without CACHED attribute ? Or at least on coherent platforms ? How do
> detect that ? Should we have a TTM helper to establish the default
> memory placement attributes that "normal PCI" drivers call to set that
> up so we can have all the necessary arch ifdefs in one single place, at
> least for "classic PCI/PCIe" stuff (AGP might need additional tweaks) ?
>
> Non-PCI and "special" drivers like Intel can use a different set of
> placement attributes to represent the requirements of those specific
> platforms (mostly thinking of embedded ARM here which under some
> circumstances might actually require non-cached mappings).
> Or am I missing another part of the puzzle ?
>
> As it-is, things are broken for me even for dumb drivers, and I suspect
> to a large extent with radeon and nouveau too, though in some case we
> might get away with it most of the time ... until the machine locks up
> for some unexplainable reason... This might cause problems on existing
> distros such as RHEL7 with our radeon adapters even.
>
> Any suggestion of what's the best approach to fix it ? I'm happy to
> produce the patches but I'm not that familiar with the TTM so I would
> like to make sure I'm the right track first :-)
While i agree about the issue of incoherent double map of same page, i
think we have more issue. For instance lattely AMD have been pushing a
lot of patches to move things to use uncached memory for radeon and as
usual thoses patches comes with no comment to the motivations of those
changes. I am ccing the usual suspect ;)
What i understand is that uncached mapping for some frequently use buffer
give a significant performance boost (i am assuming this has to do with
all the snoop pci transaction overhead).
>From my understanding this is something that is allow on other OS and
any driver wishing to compete from performance point of view will want
that.
So i think we need to get a platform flags and or set_pages_array_wc|uc
needs to fail and this would fallback to cached mapping if the fallback
code still works. So if your arch properly return and error for those
cache changing function then you should be fine.
This also means that we need to fix ttm_tt_set_placement_caching so that
when it returns an error it switches to cached mapping. Which will always
work.
Cheers,
Jérôme
>
> Cheers,
> Ben.
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: TTM placement & caching issue/questions
From: Jerome Glisse @ 2014-09-04 2:07 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Alex Deucher, linuxppc-dev, Michel Dänzer, Christian Koenig,
dri-devel
In-Reply-To: <20140904015548.GB4835@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 6133 bytes --]
On Wed, Sep 03, 2014 at 09:55:53PM -0400, Jerome Glisse wrote:
> On Thu, Sep 04, 2014 at 10:12:27AM +1000, Benjamin Herrenschmidt wrote:
> > Hi folks !
> >
> > I've been tracking down some problems with the recent DRI on powerpc and
> > stumbled upon something that doesn't look right, and not necessarily
> > only for us.
> >
> > Now it's possible that I haven't fully understood the code here and I
> > also don't know to what extent some of that behaviour is necessary for
> > some platforms such as Intel GTT bits.
> >
> > What I've observed with a simple/dumb (no DMA) driver like AST (but this
> > probably happens more generally) is that when evicting a BO from VRAM
> > into System memory, the TTM tries to preserve the existing caching
> > attributes of the VRAM object.
> >
> > From what I can tell, we end up with going from VRAM to System memory
> > type, and we eventually call ttm_bo_select_caching() to select the
> > caching option for the target.
> >
> > This will, from what I can tell, try to use the same caching mode as the
> > original object:
> >
> > if ((cur_placement & caching) != 0)
> > result |= (cur_placement & caching);
> >
> > And cur_placement comes from bo->mem.placement which as far as I can
> > tell is based on the placement array which the drivers set up.
> >
> > Now they tend to uniformly setup the placement for System memory as
> > TTM_PL_MASK_CACHING which enables all caching modes.
> >
> > So I end up with, for example, my System memory BOs having
> > TTM_PL_FLAG_CACHED not set (though they also don't have
> > TTM_PL_FLAG_UNCACHED) and TTM_PL_FLAG_WC.
> >
> > We don't seem to use the man->default_caching (which will have
> > TTM_PL_FLAG_CACHED) unless there is no matching bit at all between the
> > proposed placement and the existing caching mode.
> >
> > Now this is a problem for several reason that I can think of:
> >
> > - On a number of powerpc platforms, such as all our server 64-bit one
> > for example, it's actually illegal to map system memory non-cached. The
> > system is fully cache coherent for all possible DMA originators (that we
> > care about at least) and mapping memory non-cachable while it's mapped
> > cachable in the linear mapping can cause nasty cache paradox which, when
> > detected by HW, can checkstop the system.
> >
> > - A similar issue exists, afaik, on ARM >= v7, so anything mapped
> > non-cachable must be removed from the linear mapping explicitly since
> > otherwise it can be speculatively prefetched into the cache.
> >
> > - I don't know about x86, but even then, it looks quite sub-optimal to
> > map the memory backing of the BOs and access it using a WC rather than a
> > cachable mapping attribute.
> >
> > Now, some folks on IRC mentioned that there might be reasons for the
> > current behaviour as to not change the caching attributes when going
> > in/out of the GTT on Intel, I don't know how that relates and how that
> > works, but maybe that should be enforced by having a different placement
> > mask specifically on those chipsets.
> >
> > Dave, should we change the various PCI drivers for generally coherent
> > devices such that the System memory type doesn't allow placements
> > without CACHED attribute ? Or at least on coherent platforms ? How do
> > detect that ? Should we have a TTM helper to establish the default
> > memory placement attributes that "normal PCI" drivers call to set that
> > up so we can have all the necessary arch ifdefs in one single place, at
> > least for "classic PCI/PCIe" stuff (AGP might need additional tweaks) ?
> >
> > Non-PCI and "special" drivers like Intel can use a different set of
> > placement attributes to represent the requirements of those specific
> > platforms (mostly thinking of embedded ARM here which under some
> > circumstances might actually require non-cached mappings).
> > Or am I missing another part of the puzzle ?
> >
> > As it-is, things are broken for me even for dumb drivers, and I suspect
> > to a large extent with radeon and nouveau too, though in some case we
> > might get away with it most of the time ... until the machine locks up
> > for some unexplainable reason... This might cause problems on existing
> > distros such as RHEL7 with our radeon adapters even.
> >
> > Any suggestion of what's the best approach to fix it ? I'm happy to
> > produce the patches but I'm not that familiar with the TTM so I would
> > like to make sure I'm the right track first :-)
>
> While i agree about the issue of incoherent double map of same page, i
> think we have more issue. For instance lattely AMD have been pushing a
> lot of patches to move things to use uncached memory for radeon and as
> usual thoses patches comes with no comment to the motivations of those
> changes. I am ccing the usual suspect ;)
>
> What i understand is that uncached mapping for some frequently use buffer
> give a significant performance boost (i am assuming this has to do with
> all the snoop pci transaction overhead).
>
> From my understanding this is something that is allow on other OS and
> any driver wishing to compete from performance point of view will want
> that.
>
>
> So i think we need to get a platform flags and or set_pages_array_wc|uc
> needs to fail and this would fallback to cached mapping if the fallback
> code still works. So if your arch properly return and error for those
> cache changing function then you should be fine.
>
> This also means that we need to fix ttm_tt_set_placement_caching so that
> when it returns an error it switches to cached mapping. Which will always
> work.
So in the meantime the attached patch should work, it just silently ignore
the caching attribute request on non x86 instead of pretending that things
are setup as expected and then latter the radeon ou nouveau hw unsetting
the snoop bit.
It's not tested but i think it should work.
>
> Cheers,
> Jérôme
>
> >
> > Cheers,
> > Ben.
> >
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
[-- Attachment #2: 0001-drm-ttm-force-cached-mapping-on-non-x86-platform.patch --]
[-- Type: text/plain, Size: 1687 bytes --]
>From df0b5d1488daed05d7b4508759401a3e89cd4a38 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Glisse?= <jglisse@redhat.com>
Date: Wed, 3 Sep 2014 22:04:34 -0400
Subject: [PATCH] drm/ttm: force cached mapping on non x86 platform.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
People interested in providing uncached or write combined mapping
on there architecture need to do the ground work inside there arch
specific code to allow to break the linear kernel mapping so that
page mapping attributes can be updated, in the meantime force cached
mapping for non x86 architecture.
Signed-off-by: Jérôme Glisse <jglisse@redhat.com>
---
drivers/gpu/drm/ttm/ttm_tt.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index bf080ab..de14b83 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -89,14 +89,6 @@ static inline int ttm_tt_set_page_caching(struct page *p,
return ret;
}
-#else /* CONFIG_X86 */
-static inline int ttm_tt_set_page_caching(struct page *p,
- enum ttm_caching_state c_old,
- enum ttm_caching_state c_new)
-{
- return 0;
-}
-#endif /* CONFIG_X86 */
/*
* Change caching policy for the linear kernel map
@@ -149,6 +141,15 @@ out_err:
return ret;
}
+#else /* CONFIG_X86 */
+static int ttm_tt_set_caching(struct ttm_tt *ttm,
+ enum ttm_caching_state c_state)
+{
+ ttm->caching_state = tt_cached;
+ return 0;
+}
+#endif /* CONFIG_X86 */
+
int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t placement)
{
enum ttm_caching_state state;
--
1.9.3
^ permalink raw reply related
* Re: TTM placement & caching issue/questions
From: Benjamin Herrenschmidt @ 2014-09-04 2:15 UTC (permalink / raw)
To: Jerome Glisse
Cc: Alex Deucher, linuxppc-dev, Michel Dänzer, Christian Koenig,
dri-devel
In-Reply-To: <20140904015548.GB4835@gmail.com>
On Wed, 2014-09-03 at 21:55 -0400, Jerome Glisse wrote:
> So i think we need to get a platform flags and or set_pages_array_wc|uc
> needs to fail and this would fallback to cached mapping if the fallback
> code still works. So if your arch properly return and error for those
> cache changing function then you should be fine.
>
> This also means that we need to fix ttm_tt_set_placement_caching so that
> when it returns an error it switches to cached mapping. Which will always
> work.
Can't I just filter the mem_type definitions in the mem_type_manager
with something along that totally untested patch ?
Or do I *also* need to make those set_page_array_* things fail ?
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1308,6 +1308,24 @@ int ttm_bo_evict_mm(struct ttm_bo_device *bdev, unsigned
}
EXPORT_SYMBOL(ttm_bo_evict_mm);
+static void ttm_bo_filter_mem_type(struct ttm_bo_device *bdev, unsigned type,
+ struct ttm_mem_type_manager *man)
+{
+ /*
+ * On some architectures/patforms, we cannot allow non-cachable
+ * mappings of system memory. This can be a problem with AGP on
+ * old G5 systems vs. TTM_PL_TT but we don't really have a choice
+ * at this point on ppc64 at least and the AGP on these never
+ * worked reliably anyway.
+ */
+#if defined(CONFIG_PPC) && !defined(CONFIG_NOT_COHERENT_CACHE)
+ if (type == TTM_PL_SYSTEM || type == TTM_PL_TT) {
+ man->available_caching &= TTM_PL_FLAG_CACHED;
+ man->default_caching &= man->available_caching;
+ }
+#endif
+}
+
int ttm_bo_init_mm(struct ttm_bo_device *bdev, unsigned type,
unsigned long p_size)
{
@@ -1327,6 +1345,8 @@ int ttm_bo_init_mm(struct ttm_bo_device *bdev, unsigned ty
return ret;
man->bdev = bdev;
+ ttm_bo_filter_mem_type(bdev, type, man);
+
ret = 0;
if (type != TTM_PL_SYSTEM) {
ret = (*man->func->init)(man, p_size);
^ permalink raw reply
* Re: TTM placement & caching issue/questions
From: Benjamin Herrenschmidt @ 2014-09-04 2:25 UTC (permalink / raw)
To: Jerome Glisse
Cc: Alex Deucher, linuxppc-dev, Michel Dänzer, Christian Koenig,
dri-devel
In-Reply-To: <20140904020742.GC4835@gmail.com>
On Wed, 2014-09-03 at 22:07 -0400, Jerome Glisse wrote:
> So in the meantime the attached patch should work, it just silently ignore
> the caching attribute request on non x86 instead of pretending that things
> are setup as expected and then latter the radeon ou nouveau hw unsetting
> the snoop bit.
>
> It's not tested but i think it should work.
I'm still getting placements with !CACHED going from bo_memcpy in
ttm_io_prot() though ... I'm looking at filtering the placement
attributes instead.
Ben.
> >
> > Cheers,
> > Jérôme
> >
> > >
> > > Cheers,
> > > Ben.
> > >
> > >
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: TTM placement & caching issue/questions
From: Jerome Glisse @ 2014-09-04 2:31 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Alex Deucher, linuxppc-dev, Michel Dänzer, Christian Koenig,
dri-devel
In-Reply-To: <1409797523.25089.8.camel@pasglop>
[-- Attachment #1: Type: text/plain, Size: 945 bytes --]
On Thu, Sep 04, 2014 at 12:25:23PM +1000, Benjamin Herrenschmidt wrote:
> On Wed, 2014-09-03 at 22:07 -0400, Jerome Glisse wrote:
>
> > So in the meantime the attached patch should work, it just silently ignore
> > the caching attribute request on non x86 instead of pretending that things
> > are setup as expected and then latter the radeon ou nouveau hw unsetting
> > the snoop bit.
> >
> > It's not tested but i think it should work.
>
> I'm still getting placements with !CACHED going from bo_memcpy in
> ttm_io_prot() though ... I'm looking at filtering the placement
> attributes instead.
>
> Ben.
Ok so this one should do the trick.
>
> > >
> > > Cheers,
> > > Jérôme
> > >
> > > >
> > > > Cheers,
> > > > Ben.
> > > >
> > > >
> > > > _______________________________________________
> > > > dri-devel mailing list
> > > > dri-devel@lists.freedesktop.org
> > > > http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
[-- Attachment #2: 0001-drm-ttm-force-cached-mapping-on-non-x86-platform.patch --]
[-- Type: text/plain, Size: 4525 bytes --]
>From def7a056d042220f91016d0a7c245ba8e96f90ba Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Glisse?= <jglisse@redhat.com>
Date: Wed, 3 Sep 2014 22:04:34 -0400
Subject: [PATCH] drm/ttm: force cached mapping on non x86 platform.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
People interested in providing uncached or write combined mapping
on there architecture need to do the ground work inside there arch
specific code to allow to break the linear kernel mapping so that
page mapping attributes can be updated, in the meantime force cached
mapping for non x86 architecture.
Signed-off-by: Jérôme Glisse <jglisse@redhat.com>
---
drivers/gpu/drm/radeon/radeon_ttm.c | 2 +-
drivers/gpu/drm/ttm/ttm_bo.c | 2 +-
drivers/gpu/drm/ttm/ttm_tt.c | 47 ++++++++++++++++++++++++++++---------
include/drm/ttm/ttm_bo_driver.h | 2 +-
4 files changed, 39 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index 72afe82..4dd5060 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -304,7 +304,7 @@ static int radeon_move_vram_ram(struct ttm_buffer_object *bo,
return r;
}
- r = ttm_tt_set_placement_caching(bo->ttm, tmp_mem.placement);
+ r = ttm_tt_set_placement_caching(bo->ttm, &tmp_mem.placement);
if (unlikely(r)) {
goto out_cleanup;
}
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 3da89d5..4dc21c3 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -305,7 +305,7 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
goto out_err;
}
- ret = ttm_tt_set_placement_caching(bo->ttm, mem->placement);
+ ret = ttm_tt_set_placement_caching(bo->ttm, &mem->placement);
if (ret)
goto out_err;
diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index bf080ab..7cbdb48 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -89,14 +89,6 @@ static inline int ttm_tt_set_page_caching(struct page *p,
return ret;
}
-#else /* CONFIG_X86 */
-static inline int ttm_tt_set_page_caching(struct page *p,
- enum ttm_caching_state c_old,
- enum ttm_caching_state c_new)
-{
- return 0;
-}
-#endif /* CONFIG_X86 */
/*
* Change caching policy for the linear kernel map
@@ -149,19 +141,52 @@ out_err:
return ret;
}
-int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t placement)
+int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t *placement)
{
enum ttm_caching_state state;
- if (placement & TTM_PL_FLAG_WC)
+ if (*placement & TTM_PL_FLAG_WC)
state = tt_wc;
- else if (placement & TTM_PL_FLAG_UNCACHED)
+ else if (*placement & TTM_PL_FLAG_UNCACHED)
state = tt_uncached;
else
state = tt_cached;
return ttm_tt_set_caching(ttm, state);
}
+#else /* CONFIG_X86 */
+int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t *placement)
+{
+ if (placement & (TTM_PL_TT | TTM_PL_FLAG_SYSTEM)) {
+ ttm->caching_state = tt_cached;
+ } else {
+ if (placement & TTM_PL_FLAG_WC)
+ ttm->caching_state = tt_wc;
+ else if (placement & TTM_PL_FLAG_UNCACHED)
+ ttm->caching_state = tt_uncached;
+ else
+ ttm->caching_state = tt_cached;
+ }
+ /*
+ * Some architecture force cached so we need to reflect the
+ * new ttm->caching_state into the mem->placement flags.
+ */
+ *placement &= ~TTM_PL_MASK_CACHING;
+ switch (bo->ttm->caching_state) {
+ case tt_wc:
+ *placement |= TTM_PL_FLAG_WC;
+ break;
+ case tt_uncached:
+ *placement |= TTM_PL_FLAG_UNCACHED;
+ break;
+ case tt_cached:
+ default:
+ *placement |= TTM_PL_FLAG_CACHED;
+ break;
+ }
+ return 0;
+}
+#endif /* CONFIG_X86 */
EXPORT_SYMBOL(ttm_tt_set_placement_caching);
void ttm_tt_destroy(struct ttm_tt *ttm)
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 1d9f0f1..cbc5ad2 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -669,7 +669,7 @@ extern int ttm_tt_swapin(struct ttm_tt *ttm);
* hit RAM. This function may be very costly as it involves global TLB
* and cache flushes and potential page splitting / combining.
*/
-extern int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t placement);
+extern int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t *placement);
extern int ttm_tt_swapout(struct ttm_tt *ttm,
struct file *persistent_swap_storage);
--
1.9.3
^ permalink raw reply related
* Re: TTM placement & caching issue/questions
From: Jerome Glisse @ 2014-09-04 2:32 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Alex Deucher, linuxppc-dev, Michel Dänzer, Christian Koenig,
dri-devel
In-Reply-To: <20140904023117.GD4835@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 5913 bytes --]
On Wed, Sep 03, 2014 at 10:31:18PM -0400, Jerome Glisse wrote:
> On Thu, Sep 04, 2014 at 12:25:23PM +1000, Benjamin Herrenschmidt wrote:
> > On Wed, 2014-09-03 at 22:07 -0400, Jerome Glisse wrote:
> >
> > > So in the meantime the attached patch should work, it just silently ignore
> > > the caching attribute request on non x86 instead of pretending that things
> > > are setup as expected and then latter the radeon ou nouveau hw unsetting
> > > the snoop bit.
> > >
> > > It's not tested but i think it should work.
> >
> > I'm still getting placements with !CACHED going from bo_memcpy in
> > ttm_io_prot() though ... I'm looking at filtering the placement
> > attributes instead.
> >
> > Ben.
>
> Ok so this one should do the trick.
And this one should build :)
>
>
> >
> > > >
> > > > Cheers,
> > > > Jérôme
> > > >
> > > > >
> > > > > Cheers,
> > > > > Ben.
> > > > >
> > > > >
> > > > > _______________________________________________
> > > > > dri-devel mailing list
> > > > > dri-devel@lists.freedesktop.org
> > > > > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> >
> From def7a056d042220f91016d0a7c245ba8e96f90ba Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Glisse?= <jglisse@redhat.com>
> Date: Wed, 3 Sep 2014 22:04:34 -0400
> Subject: [PATCH] drm/ttm: force cached mapping on non x86 platform.
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> People interested in providing uncached or write combined mapping
> on there architecture need to do the ground work inside there arch
> specific code to allow to break the linear kernel mapping so that
> page mapping attributes can be updated, in the meantime force cached
> mapping for non x86 architecture.
>
> Signed-off-by: Jérôme Glisse <jglisse@redhat.com>
> ---
> drivers/gpu/drm/radeon/radeon_ttm.c | 2 +-
> drivers/gpu/drm/ttm/ttm_bo.c | 2 +-
> drivers/gpu/drm/ttm/ttm_tt.c | 47 ++++++++++++++++++++++++++++---------
> include/drm/ttm/ttm_bo_driver.h | 2 +-
> 4 files changed, 39 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
> index 72afe82..4dd5060 100644
> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> @@ -304,7 +304,7 @@ static int radeon_move_vram_ram(struct ttm_buffer_object *bo,
> return r;
> }
>
> - r = ttm_tt_set_placement_caching(bo->ttm, tmp_mem.placement);
> + r = ttm_tt_set_placement_caching(bo->ttm, &tmp_mem.placement);
> if (unlikely(r)) {
> goto out_cleanup;
> }
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 3da89d5..4dc21c3 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -305,7 +305,7 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
> goto out_err;
> }
>
> - ret = ttm_tt_set_placement_caching(bo->ttm, mem->placement);
> + ret = ttm_tt_set_placement_caching(bo->ttm, &mem->placement);
> if (ret)
> goto out_err;
>
> diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
> index bf080ab..7cbdb48 100644
> --- a/drivers/gpu/drm/ttm/ttm_tt.c
> +++ b/drivers/gpu/drm/ttm/ttm_tt.c
> @@ -89,14 +89,6 @@ static inline int ttm_tt_set_page_caching(struct page *p,
>
> return ret;
> }
> -#else /* CONFIG_X86 */
> -static inline int ttm_tt_set_page_caching(struct page *p,
> - enum ttm_caching_state c_old,
> - enum ttm_caching_state c_new)
> -{
> - return 0;
> -}
> -#endif /* CONFIG_X86 */
>
> /*
> * Change caching policy for the linear kernel map
> @@ -149,19 +141,52 @@ out_err:
> return ret;
> }
>
> -int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t placement)
> +int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t *placement)
> {
> enum ttm_caching_state state;
>
> - if (placement & TTM_PL_FLAG_WC)
> + if (*placement & TTM_PL_FLAG_WC)
> state = tt_wc;
> - else if (placement & TTM_PL_FLAG_UNCACHED)
> + else if (*placement & TTM_PL_FLAG_UNCACHED)
> state = tt_uncached;
> else
> state = tt_cached;
>
> return ttm_tt_set_caching(ttm, state);
> }
> +#else /* CONFIG_X86 */
> +int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t *placement)
> +{
> + if (placement & (TTM_PL_TT | TTM_PL_FLAG_SYSTEM)) {
> + ttm->caching_state = tt_cached;
> + } else {
> + if (placement & TTM_PL_FLAG_WC)
> + ttm->caching_state = tt_wc;
> + else if (placement & TTM_PL_FLAG_UNCACHED)
> + ttm->caching_state = tt_uncached;
> + else
> + ttm->caching_state = tt_cached;
> + }
> + /*
> + * Some architecture force cached so we need to reflect the
> + * new ttm->caching_state into the mem->placement flags.
> + */
> + *placement &= ~TTM_PL_MASK_CACHING;
> + switch (bo->ttm->caching_state) {
> + case tt_wc:
> + *placement |= TTM_PL_FLAG_WC;
> + break;
> + case tt_uncached:
> + *placement |= TTM_PL_FLAG_UNCACHED;
> + break;
> + case tt_cached:
> + default:
> + *placement |= TTM_PL_FLAG_CACHED;
> + break;
> + }
> + return 0;
> +}
> +#endif /* CONFIG_X86 */
> EXPORT_SYMBOL(ttm_tt_set_placement_caching);
>
> void ttm_tt_destroy(struct ttm_tt *ttm)
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index 1d9f0f1..cbc5ad2 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -669,7 +669,7 @@ extern int ttm_tt_swapin(struct ttm_tt *ttm);
> * hit RAM. This function may be very costly as it involves global TLB
> * and cache flushes and potential page splitting / combining.
> */
> -extern int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t placement);
> +extern int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t *placement);
> extern int ttm_tt_swapout(struct ttm_tt *ttm,
> struct file *persistent_swap_storage);
>
> --
> 1.9.3
>
[-- Attachment #2: 0001-drm-ttm-force-cached-mapping-on-non-x86-platform.patch --]
[-- Type: text/plain, Size: 4527 bytes --]
>From 6ed69ceee96873ab351881c6b9f931cec39f396a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Glisse?= <jglisse@redhat.com>
Date: Wed, 3 Sep 2014 22:04:34 -0400
Subject: [PATCH] drm/ttm: force cached mapping on non x86 platform.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
People interested in providing uncached or write combined mapping
on there architecture need to do the ground work inside there arch
specific code to allow to break the linear kernel mapping so that
page mapping attributes can be updated, in the meantime force cached
mapping for non x86 architecture.
Signed-off-by: Jérôme Glisse <jglisse@redhat.com>
---
drivers/gpu/drm/radeon/radeon_ttm.c | 2 +-
drivers/gpu/drm/ttm/ttm_bo.c | 2 +-
drivers/gpu/drm/ttm/ttm_tt.c | 47 ++++++++++++++++++++++++++++---------
include/drm/ttm/ttm_bo_driver.h | 2 +-
4 files changed, 39 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index 72afe82..4dd5060 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -304,7 +304,7 @@ static int radeon_move_vram_ram(struct ttm_buffer_object *bo,
return r;
}
- r = ttm_tt_set_placement_caching(bo->ttm, tmp_mem.placement);
+ r = ttm_tt_set_placement_caching(bo->ttm, &tmp_mem.placement);
if (unlikely(r)) {
goto out_cleanup;
}
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 3da89d5..4dc21c3 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -305,7 +305,7 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
goto out_err;
}
- ret = ttm_tt_set_placement_caching(bo->ttm, mem->placement);
+ ret = ttm_tt_set_placement_caching(bo->ttm, &mem->placement);
if (ret)
goto out_err;
diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index bf080ab..efd49b9 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -89,14 +89,6 @@ static inline int ttm_tt_set_page_caching(struct page *p,
return ret;
}
-#else /* CONFIG_X86 */
-static inline int ttm_tt_set_page_caching(struct page *p,
- enum ttm_caching_state c_old,
- enum ttm_caching_state c_new)
-{
- return 0;
-}
-#endif /* CONFIG_X86 */
/*
* Change caching policy for the linear kernel map
@@ -149,19 +141,52 @@ out_err:
return ret;
}
-int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t placement)
+int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t *placement)
{
enum ttm_caching_state state;
- if (placement & TTM_PL_FLAG_WC)
+ if (*placement & TTM_PL_FLAG_WC)
state = tt_wc;
- else if (placement & TTM_PL_FLAG_UNCACHED)
+ else if (*placement & TTM_PL_FLAG_UNCACHED)
state = tt_uncached;
else
state = tt_cached;
return ttm_tt_set_caching(ttm, state);
}
+#else /* CONFIG_X86 */
+int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t *placement)
+{
+ if (*placement & (TTM_PL_TT | TTM_PL_FLAG_SYSTEM)) {
+ ttm->caching_state = tt_cached;
+ } else {
+ if (*placement & TTM_PL_FLAG_WC)
+ ttm->caching_state = tt_wc;
+ else if (placement & TTM_PL_FLAG_UNCACHED)
+ ttm->caching_state = tt_uncached;
+ else
+ ttm->caching_state = tt_cached;
+ }
+ /*
+ * Some architecture force cached so we need to reflect the
+ * new ttm->caching_state into the mem->placement flags.
+ */
+ *placement &= ~TTM_PL_MASK_CACHING;
+ switch (bo->ttm->caching_state) {
+ case tt_wc:
+ *placement |= TTM_PL_FLAG_WC;
+ break;
+ case tt_uncached:
+ *placement |= TTM_PL_FLAG_UNCACHED;
+ break;
+ case tt_cached:
+ default:
+ *placement |= TTM_PL_FLAG_CACHED;
+ break;
+ }
+ return 0;
+}
+#endif /* CONFIG_X86 */
EXPORT_SYMBOL(ttm_tt_set_placement_caching);
void ttm_tt_destroy(struct ttm_tt *ttm)
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 1d9f0f1..cbc5ad2 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -669,7 +669,7 @@ extern int ttm_tt_swapin(struct ttm_tt *ttm);
* hit RAM. This function may be very costly as it involves global TLB
* and cache flushes and potential page splitting / combining.
*/
-extern int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t placement);
+extern int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t *placement);
extern int ttm_tt_swapout(struct ttm_tt *ttm,
struct file *persistent_swap_storage);
--
1.9.3
^ permalink raw reply related
* Re: TTM placement & caching issue/questions
From: Jerome Glisse @ 2014-09-04 2:36 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Alex Deucher, linuxppc-dev, Michel Dänzer, Christian Koenig,
dri-devel
In-Reply-To: <20140904023117.GD4835@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 5924 bytes --]
On Wed, Sep 03, 2014 at 10:31:18PM -0400, Jerome Glisse wrote:
> On Thu, Sep 04, 2014 at 12:25:23PM +1000, Benjamin Herrenschmidt wrote:
> > On Wed, 2014-09-03 at 22:07 -0400, Jerome Glisse wrote:
> >
> > > So in the meantime the attached patch should work, it just silently ignore
> > > the caching attribute request on non x86 instead of pretending that things
> > > are setup as expected and then latter the radeon ou nouveau hw unsetting
> > > the snoop bit.
> > >
> > > It's not tested but i think it should work.
> >
> > I'm still getting placements with !CACHED going from bo_memcpy in
> > ttm_io_prot() though ... I'm looking at filtering the placement
> > attributes instead.
> >
> > Ben.
>
> Ok so this one should do the trick.
Ok final version ... famous last word.
>
>
> >
> > > >
> > > > Cheers,
> > > > Jérôme
> > > >
> > > > >
> > > > > Cheers,
> > > > > Ben.
> > > > >
> > > > >
> > > > > _______________________________________________
> > > > > dri-devel mailing list
> > > > > dri-devel@lists.freedesktop.org
> > > > > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> >
> From def7a056d042220f91016d0a7c245ba8e96f90ba Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Glisse?= <jglisse@redhat.com>
> Date: Wed, 3 Sep 2014 22:04:34 -0400
> Subject: [PATCH] drm/ttm: force cached mapping on non x86 platform.
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> People interested in providing uncached or write combined mapping
> on there architecture need to do the ground work inside there arch
> specific code to allow to break the linear kernel mapping so that
> page mapping attributes can be updated, in the meantime force cached
> mapping for non x86 architecture.
>
> Signed-off-by: Jérôme Glisse <jglisse@redhat.com>
> ---
> drivers/gpu/drm/radeon/radeon_ttm.c | 2 +-
> drivers/gpu/drm/ttm/ttm_bo.c | 2 +-
> drivers/gpu/drm/ttm/ttm_tt.c | 47 ++++++++++++++++++++++++++++---------
> include/drm/ttm/ttm_bo_driver.h | 2 +-
> 4 files changed, 39 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
> index 72afe82..4dd5060 100644
> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> @@ -304,7 +304,7 @@ static int radeon_move_vram_ram(struct ttm_buffer_object *bo,
> return r;
> }
>
> - r = ttm_tt_set_placement_caching(bo->ttm, tmp_mem.placement);
> + r = ttm_tt_set_placement_caching(bo->ttm, &tmp_mem.placement);
> if (unlikely(r)) {
> goto out_cleanup;
> }
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 3da89d5..4dc21c3 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -305,7 +305,7 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
> goto out_err;
> }
>
> - ret = ttm_tt_set_placement_caching(bo->ttm, mem->placement);
> + ret = ttm_tt_set_placement_caching(bo->ttm, &mem->placement);
> if (ret)
> goto out_err;
>
> diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
> index bf080ab..7cbdb48 100644
> --- a/drivers/gpu/drm/ttm/ttm_tt.c
> +++ b/drivers/gpu/drm/ttm/ttm_tt.c
> @@ -89,14 +89,6 @@ static inline int ttm_tt_set_page_caching(struct page *p,
>
> return ret;
> }
> -#else /* CONFIG_X86 */
> -static inline int ttm_tt_set_page_caching(struct page *p,
> - enum ttm_caching_state c_old,
> - enum ttm_caching_state c_new)
> -{
> - return 0;
> -}
> -#endif /* CONFIG_X86 */
>
> /*
> * Change caching policy for the linear kernel map
> @@ -149,19 +141,52 @@ out_err:
> return ret;
> }
>
> -int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t placement)
> +int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t *placement)
> {
> enum ttm_caching_state state;
>
> - if (placement & TTM_PL_FLAG_WC)
> + if (*placement & TTM_PL_FLAG_WC)
> state = tt_wc;
> - else if (placement & TTM_PL_FLAG_UNCACHED)
> + else if (*placement & TTM_PL_FLAG_UNCACHED)
> state = tt_uncached;
> else
> state = tt_cached;
>
> return ttm_tt_set_caching(ttm, state);
> }
> +#else /* CONFIG_X86 */
> +int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t *placement)
> +{
> + if (placement & (TTM_PL_TT | TTM_PL_FLAG_SYSTEM)) {
> + ttm->caching_state = tt_cached;
> + } else {
> + if (placement & TTM_PL_FLAG_WC)
> + ttm->caching_state = tt_wc;
> + else if (placement & TTM_PL_FLAG_UNCACHED)
> + ttm->caching_state = tt_uncached;
> + else
> + ttm->caching_state = tt_cached;
> + }
> + /*
> + * Some architecture force cached so we need to reflect the
> + * new ttm->caching_state into the mem->placement flags.
> + */
> + *placement &= ~TTM_PL_MASK_CACHING;
> + switch (bo->ttm->caching_state) {
> + case tt_wc:
> + *placement |= TTM_PL_FLAG_WC;
> + break;
> + case tt_uncached:
> + *placement |= TTM_PL_FLAG_UNCACHED;
> + break;
> + case tt_cached:
> + default:
> + *placement |= TTM_PL_FLAG_CACHED;
> + break;
> + }
> + return 0;
> +}
> +#endif /* CONFIG_X86 */
> EXPORT_SYMBOL(ttm_tt_set_placement_caching);
>
> void ttm_tt_destroy(struct ttm_tt *ttm)
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index 1d9f0f1..cbc5ad2 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -669,7 +669,7 @@ extern int ttm_tt_swapin(struct ttm_tt *ttm);
> * hit RAM. This function may be very costly as it involves global TLB
> * and cache flushes and potential page splitting / combining.
> */
> -extern int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t placement);
> +extern int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t *placement);
> extern int ttm_tt_swapout(struct ttm_tt *ttm,
> struct file *persistent_swap_storage);
>
> --
> 1.9.3
>
[-- Attachment #2: 0001-drm-ttm-force-cached-mapping-on-non-x86-platform.patch --]
[-- Type: text/plain, Size: 4186 bytes --]
>From 236038e18dc303bb9aa877922e01963d3fb0b7af Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Glisse?= <jglisse@redhat.com>
Date: Wed, 3 Sep 2014 22:04:34 -0400
Subject: [PATCH] drm/ttm: force cached mapping on non x86 platform.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
People interested in providing uncached or write combined mapping
on there architecture need to do the ground work inside there arch
specific code to allow to break the linear kernel mapping so that
page mapping attributes can be updated, in the meantime force cached
mapping for non x86 architecture.
Signed-off-by: Jérôme Glisse <jglisse@redhat.com>
---
drivers/gpu/drm/radeon/radeon_ttm.c | 2 +-
drivers/gpu/drm/ttm/ttm_bo.c | 2 +-
drivers/gpu/drm/ttm/ttm_tt.c | 32 +++++++++++++++++++++-----------
include/drm/ttm/ttm_bo_driver.h | 2 +-
4 files changed, 24 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index 72afe82..4dd5060 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -304,7 +304,7 @@ static int radeon_move_vram_ram(struct ttm_buffer_object *bo,
return r;
}
- r = ttm_tt_set_placement_caching(bo->ttm, tmp_mem.placement);
+ r = ttm_tt_set_placement_caching(bo->ttm, &tmp_mem.placement);
if (unlikely(r)) {
goto out_cleanup;
}
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 3da89d5..4dc21c3 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -305,7 +305,7 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
goto out_err;
}
- ret = ttm_tt_set_placement_caching(bo->ttm, mem->placement);
+ ret = ttm_tt_set_placement_caching(bo->ttm, &mem->placement);
if (ret)
goto out_err;
diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index bf080ab..a0df803 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -89,14 +89,6 @@ static inline int ttm_tt_set_page_caching(struct page *p,
return ret;
}
-#else /* CONFIG_X86 */
-static inline int ttm_tt_set_page_caching(struct page *p,
- enum ttm_caching_state c_old,
- enum ttm_caching_state c_new)
-{
- return 0;
-}
-#endif /* CONFIG_X86 */
/*
* Change caching policy for the linear kernel map
@@ -149,19 +141,37 @@ out_err:
return ret;
}
-int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t placement)
+int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t *placement)
{
enum ttm_caching_state state;
- if (placement & TTM_PL_FLAG_WC)
+ if (*placement & TTM_PL_FLAG_WC)
state = tt_wc;
- else if (placement & TTM_PL_FLAG_UNCACHED)
+ else if (*placement & TTM_PL_FLAG_UNCACHED)
state = tt_uncached;
else
state = tt_cached;
return ttm_tt_set_caching(ttm, state);
}
+#else /* CONFIG_X86 */
+int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t *placement)
+{
+ if (*placement & (TTM_PL_TT | TTM_PL_FLAG_SYSTEM)) {
+ ttm->caching_state = tt_cached;
+ *placement &= ~TTM_PL_MASK_CACHING;
+ *placement |= TTM_PL_FLAG_CACHED;
+ } else {
+ if (*placement & TTM_PL_FLAG_WC)
+ ttm->caching_state = tt_wc;
+ else if (placement & TTM_PL_FLAG_UNCACHED)
+ ttm->caching_state = tt_uncached;
+ else
+ ttm->caching_state = tt_cached;
+ }
+ return 0;
+}
+#endif /* CONFIG_X86 */
EXPORT_SYMBOL(ttm_tt_set_placement_caching);
void ttm_tt_destroy(struct ttm_tt *ttm)
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 1d9f0f1..cbc5ad2 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -669,7 +669,7 @@ extern int ttm_tt_swapin(struct ttm_tt *ttm);
* hit RAM. This function may be very costly as it involves global TLB
* and cache flushes and potential page splitting / combining.
*/
-extern int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t placement);
+extern int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t *placement);
extern int ttm_tt_swapout(struct ttm_tt *ttm,
struct file *persistent_swap_storage);
--
1.9.3
^ permalink raw reply related
* Re: [RFC PATCH v3 2/3] powerpc: Only set numa node information for present cpus at boottime
From: Cyril Bur @ 2014-09-04 3:53 UTC (permalink / raw)
To: Nishanth Aravamudan, Li Zhong; +Cc: Nathan Fontenot, linuxppc-dev, paulus
In-Reply-To: <20140903030247.GB31420@linux.vnet.ibm.com>
On 03/09/14 13:02, Nishanth Aravamudan wrote:
> On 27.08.2014 [17:34:00 +0800], Li Zhong wrote:
>> As Nish suggested, it makes more sense to init the numa node informatiion
>> for present cpus at boottime, which could also avoid WARN_ON(1) in
>> numa_setup_cpu().
Hit this on a Power8 LPAR. With the patchset applied the warnings no
longer present.
>>
>> With this change, we also need to change the smp_prepare_cpus() to set up
>> numa information only on present cpus.
>>
>> For those possible, but not present cpus, their numa information
>> will be set up after they are started, as the original code did before commit
>> 2fabf084b6ad.
>>
>> Cc: Nishanth Aravamudan <nacc@linux.vnet.ibm.com>
>> Cc: Nathan Fontenot <nfont@linux.vnet.ibm.com>
>> Signed-off-by: Li Zhong <zhong@linux.vnet.ibm.com>
>
> Acked-by: Nishanth Aravamudan <nacc@linux.vnet.ibm.com>
Tested-by: Cyril Bur <cyril.bur@au1.ibm.com>
>> ---
>> arch/powerpc/kernel/smp.c | 10 ++++++++--
>> arch/powerpc/mm/numa.c | 2 +-
>> 2 files changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
>> index a0738af..dc0e774 100644
>> --- a/arch/powerpc/kernel/smp.c
>> +++ b/arch/powerpc/kernel/smp.c
>> @@ -379,8 +379,11 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
>> /*
>> * numa_node_id() works after this.
>> */
>> - set_cpu_numa_node(cpu, numa_cpu_lookup_table[cpu]);
>> - set_cpu_numa_mem(cpu, local_memory_node(numa_cpu_lookup_table[cpu]));
>> + if (cpu_present(cpu)) {
>> + set_cpu_numa_node(cpu, numa_cpu_lookup_table[cpu]);
>> + set_cpu_numa_mem(cpu,
>> + local_memory_node(numa_cpu_lookup_table[cpu]));
>> + }
>> }
>>
>> cpumask_set_cpu(boot_cpuid, cpu_sibling_mask(boot_cpuid));
>> @@ -728,6 +731,9 @@ void start_secondary(void *unused)
>> }
>> traverse_core_siblings(cpu, true);
>>
>> + set_numa_node(numa_cpu_lookup_table[cpu]);
>> + set_numa_mem(local_memory_node(numa_cpu_lookup_table[cpu]));
>> +
>> smp_wmb();
>> notify_cpu_starting(cpu);
>> set_cpu_online(cpu, true);
>> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
>> index 9918c02..3a9061e 100644
>> --- a/arch/powerpc/mm/numa.c
>> +++ b/arch/powerpc/mm/numa.c
>> @@ -1127,7 +1127,7 @@ void __init do_init_bootmem(void)
>> * even before we online them, so that we can use cpu_to_{node,mem}
>> * early in boot, cf. smp_prepare_cpus().
>> */
>> - for_each_possible_cpu(cpu) {
>> + for_each_present_cpu(cpu) {
>> numa_setup_cpu((unsigned long)cpu);
>> }
>> }
>> --
>> 1.9.1
>>
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
^ permalink raw reply
* [PATCH v2] QE: move qe code from arch/powerpc to drivers/soc
From: Zhao Qiang @ 2014-09-04 5:06 UTC (permalink / raw)
To: leoli, linuxppc-dev, B07421; +Cc: Zhao Qiang, R63061, linux-kernel
LS1 is arm cpu and it has qe ip block.
move qe code from platform directory to public directory.
QE is an IP block integrates several comunications peripheral
controllers. It can implement a variety of applications, such
as uart, usb and tdm and so on.
Signed-off-by: Zhao Qiang <B45475@freescale.com>
---
Changes for v2:
- mv code to drivers/soc
arch/powerpc/Kconfig | 2 -
arch/powerpc/platforms/83xx/km83xx.c | 4 +-
arch/powerpc/platforms/83xx/misc.c | 2 +-
arch/powerpc/platforms/83xx/mpc832x_mds.c | 4 +-
arch/powerpc/platforms/83xx/mpc832x_rdb.c | 4 +-
arch/powerpc/platforms/83xx/mpc836x_mds.c | 4 +-
arch/powerpc/platforms/83xx/mpc836x_rdk.c | 4 +-
arch/powerpc/platforms/85xx/common.c | 2 +-
arch/powerpc/platforms/85xx/corenet_generic.c | 2 +-
arch/powerpc/platforms/85xx/mpc85xx_mds.c | 4 +-
arch/powerpc/platforms/85xx/mpc85xx_rdb.c | 4 +-
arch/powerpc/platforms/85xx/twr_p102x.c | 4 +-
arch/powerpc/platforms/Kconfig | 19 ---------
arch/powerpc/sysdev/Makefile | 1 -
arch/powerpc/sysdev/qe_lib/Kconfig | 27 -------------
drivers/net/ethernet/freescale/fsl_pq_mdio.c | 2 +-
drivers/net/ethernet/freescale/ucc_geth.c | 8 ++--
drivers/net/ethernet/freescale/ucc_geth.h | 8 ++--
drivers/soc/Kconfig | 2 +
drivers/soc/Makefile | 2 +
drivers/soc/qe/Kconfig | 45 ++++++++++++++++++++++
.../sysdev/qe_lib => drivers/soc/qe}/Makefile | 0
.../sysdev/qe_lib => drivers/soc/qe}/gpio.c | 2 +-
.../powerpc/sysdev/qe_lib => drivers/soc/qe}/qe.c | 4 +-
.../sysdev/qe_lib => drivers/soc/qe}/qe_ic.c | 2 +-
.../sysdev/qe_lib => drivers/soc/qe}/qe_ic.h | 2 +-
.../sysdev/qe_lib => drivers/soc/qe}/qe_io.c | 2 +-
.../powerpc/sysdev/qe_lib => drivers/soc/qe}/ucc.c | 6 +--
.../sysdev/qe_lib => drivers/soc/qe}/ucc_fast.c | 8 ++--
.../sysdev/qe_lib => drivers/soc/qe}/ucc_slow.c | 8 ++--
.../powerpc/sysdev/qe_lib => drivers/soc/qe}/usb.c | 4 +-
drivers/spi/spi-fsl-cpm.c | 2 +-
drivers/tty/serial/ucc_uart.c | 2 +-
drivers/usb/gadget/fsl_qe_udc.c | 2 +-
drivers/usb/host/fhci-hcd.c | 2 +-
drivers/usb/host/fhci-hub.c | 2 +-
drivers/usb/host/fhci-sched.c | 2 +-
drivers/usb/host/fhci.h | 4 +-
.../include/asm => include/linux/fsl}/immap_qe.h | 0
.../powerpc/include/asm => include/linux/fsl}/qe.h | 2 +-
.../include/asm => include/linux/fsl}/qe_ic.h | 0
.../include/asm => include/linux/fsl}/ucc.h | 4 +-
.../include/asm => include/linux/fsl}/ucc_fast.h | 6 +--
.../include/asm => include/linux/fsl}/ucc_slow.h | 6 +--
44 files changed, 113 insertions(+), 113 deletions(-)
delete mode 100644 arch/powerpc/sysdev/qe_lib/Kconfig
create mode 100644 drivers/soc/qe/Kconfig
rename {arch/powerpc/sysdev/qe_lib => drivers/soc/qe}/Makefile (100%)
rename {arch/powerpc/sysdev/qe_lib => drivers/soc/qe}/gpio.c (99%)
rename {arch/powerpc/sysdev/qe_lib => drivers/soc/qe}/qe.c (99%)
rename {arch/powerpc/sysdev/qe_lib => drivers/soc/qe}/qe_ic.c (99%)
rename {arch/powerpc/sysdev/qe_lib => drivers/soc/qe}/qe_ic.h (98%)
rename {arch/powerpc/sysdev/qe_lib => drivers/soc/qe}/qe_io.c (99%)
rename {arch/powerpc/sysdev/qe_lib => drivers/soc/qe}/ucc.c (98%)
rename {arch/powerpc/sysdev/qe_lib => drivers/soc/qe}/ucc_fast.c (98%)
rename {arch/powerpc/sysdev/qe_lib => drivers/soc/qe}/ucc_slow.c (98%)
rename {arch/powerpc/sysdev/qe_lib => drivers/soc/qe}/usb.c (96%)
rename {arch/powerpc/include/asm => include/linux/fsl}/immap_qe.h (100%)
rename {arch/powerpc/include/asm => include/linux/fsl}/qe.h (99%)
rename {arch/powerpc/include/asm => include/linux/fsl}/qe_ic.h (100%)
rename {arch/powerpc/include/asm => include/linux/fsl}/ucc.h (96%)
rename {arch/powerpc/include/asm => include/linux/fsl}/ucc_fast.h (98%)
rename {arch/powerpc/include/asm => include/linux/fsl}/ucc_slow.h (99%)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index bd6dd6e..65ca032 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -1050,8 +1050,6 @@ source "drivers/Kconfig"
source "fs/Kconfig"
-source "arch/powerpc/sysdev/qe_lib/Kconfig"
-
source "lib/Kconfig"
source "arch/powerpc/Kconfig.debug"
diff --git a/arch/powerpc/platforms/83xx/km83xx.c b/arch/powerpc/platforms/83xx/km83xx.c
index bf4c447..584d8cc 100644
--- a/arch/powerpc/platforms/83xx/km83xx.c
+++ b/arch/powerpc/platforms/83xx/km83xx.c
@@ -37,8 +37,8 @@
#include <asm/udbg.h>
#include <sysdev/fsl_soc.h>
#include <sysdev/fsl_pci.h>
-#include <asm/qe.h>
-#include <asm/qe_ic.h>
+#include <linux/fsl/qe.h>
+#include <linux/fsl/qe_ic.h>
#include "mpc83xx.h"
diff --git a/arch/powerpc/platforms/83xx/misc.c b/arch/powerpc/platforms/83xx/misc.c
index 125336f..3e2e6d2 100644
--- a/arch/powerpc/platforms/83xx/misc.c
+++ b/arch/powerpc/platforms/83xx/misc.c
@@ -17,7 +17,7 @@
#include <asm/io.h>
#include <asm/hw_irq.h>
#include <asm/ipic.h>
-#include <asm/qe_ic.h>
+#include <linux/fsl/qe_ic.h>
#include <sysdev/fsl_soc.h>
#include <sysdev/fsl_pci.h>
diff --git a/arch/powerpc/platforms/83xx/mpc832x_mds.c b/arch/powerpc/platforms/83xx/mpc832x_mds.c
index 8d76220..e1186be 100644
--- a/arch/powerpc/platforms/83xx/mpc832x_mds.c
+++ b/arch/powerpc/platforms/83xx/mpc832x_mds.c
@@ -36,8 +36,8 @@
#include <asm/udbg.h>
#include <sysdev/fsl_soc.h>
#include <sysdev/fsl_pci.h>
-#include <asm/qe.h>
-#include <asm/qe_ic.h>
+#include <linux/fsl/qe.h>
+#include <linux/fsl/qe_ic.h>
#include "mpc83xx.h"
diff --git a/arch/powerpc/platforms/83xx/mpc832x_rdb.c b/arch/powerpc/platforms/83xx/mpc832x_rdb.c
index eff5baa..9f75944 100644
--- a/arch/powerpc/platforms/83xx/mpc832x_rdb.c
+++ b/arch/powerpc/platforms/83xx/mpc832x_rdb.c
@@ -25,8 +25,8 @@
#include <asm/time.h>
#include <asm/ipic.h>
#include <asm/udbg.h>
-#include <asm/qe.h>
-#include <asm/qe_ic.h>
+#include <linux/fsl/qe.h>
+#include <linux/fsl/qe_ic.h>
#include <sysdev/fsl_soc.h>
#include <sysdev/fsl_pci.h>
diff --git a/arch/powerpc/platforms/83xx/mpc836x_mds.c b/arch/powerpc/platforms/83xx/mpc836x_mds.c
index 1a26d2f..7c1a22f 100644
--- a/arch/powerpc/platforms/83xx/mpc836x_mds.c
+++ b/arch/powerpc/platforms/83xx/mpc836x_mds.c
@@ -44,8 +44,8 @@
#include <sysdev/fsl_soc.h>
#include <sysdev/fsl_pci.h>
#include <sysdev/simple_gpio.h>
-#include <asm/qe.h>
-#include <asm/qe_ic.h>
+#include <linux/fsl/qe.h>
+#include <linux/fsl/qe_ic.h>
#include "mpc83xx.h"
diff --git a/arch/powerpc/platforms/83xx/mpc836x_rdk.c b/arch/powerpc/platforms/83xx/mpc836x_rdk.c
index b63b42d..5e17d71 100644
--- a/arch/powerpc/platforms/83xx/mpc836x_rdk.c
+++ b/arch/powerpc/platforms/83xx/mpc836x_rdk.c
@@ -20,8 +20,8 @@
#include <asm/time.h>
#include <asm/ipic.h>
#include <asm/udbg.h>
-#include <asm/qe.h>
-#include <asm/qe_ic.h>
+#include <linux/fsl/qe.h>
+#include <linux/fsl/qe_ic.h>
#include <sysdev/fsl_soc.h>
#include <sysdev/fsl_pci.h>
diff --git a/arch/powerpc/platforms/85xx/common.c b/arch/powerpc/platforms/85xx/common.c
index b564b5e..dfb21da 100644
--- a/arch/powerpc/platforms/85xx/common.c
+++ b/arch/powerpc/platforms/85xx/common.c
@@ -9,7 +9,7 @@
#include <linux/of_irq.h>
#include <linux/of_platform.h>
-#include <asm/qe.h>
+#include <linux/fsl/qe.h>
#include <sysdev/cpm2_pic.h>
#include "mpc85xx.h"
diff --git a/arch/powerpc/platforms/85xx/corenet_generic.c b/arch/powerpc/platforms/85xx/corenet_generic.c
index d22dd85..265c756 100644
--- a/arch/powerpc/platforms/85xx/corenet_generic.c
+++ b/arch/powerpc/platforms/85xx/corenet_generic.c
@@ -26,7 +26,7 @@
#include <asm/udbg.h>
#include <asm/mpic.h>
#include <asm/ehv_pic.h>
-#include <asm/qe_ic.h>
+#include <linux/fsl/qe_ic.h>
#include <linux/of_platform.h>
#include <sysdev/fsl_soc.h>
diff --git a/arch/powerpc/platforms/85xx/mpc85xx_mds.c b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
index a392e94..7cf8eb5 100644
--- a/arch/powerpc/platforms/85xx/mpc85xx_mds.c
+++ b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
@@ -47,8 +47,8 @@
#include <sysdev/fsl_soc.h>
#include <sysdev/fsl_pci.h>
#include <sysdev/simple_gpio.h>
-#include <asm/qe.h>
-#include <asm/qe_ic.h>
+#include <linux/fsl/qe.h>
+#include <linux/fsl/qe_ic.h>
#include <asm/mpic.h>
#include <asm/swiotlb.h>
#include <asm/fsl_guts.h>
diff --git a/arch/powerpc/platforms/85xx/mpc85xx_rdb.c b/arch/powerpc/platforms/85xx/mpc85xx_rdb.c
index e358bed..d1b6d1b 100644
--- a/arch/powerpc/platforms/85xx/mpc85xx_rdb.c
+++ b/arch/powerpc/platforms/85xx/mpc85xx_rdb.c
@@ -25,8 +25,8 @@
#include <asm/prom.h>
#include <asm/udbg.h>
#include <asm/mpic.h>
-#include <asm/qe.h>
-#include <asm/qe_ic.h>
+#include <linux/fsl/qe.h>
+#include <linux/fsl/qe_ic.h>
#include <asm/fsl_guts.h>
#include <sysdev/fsl_soc.h>
diff --git a/arch/powerpc/platforms/85xx/twr_p102x.c b/arch/powerpc/platforms/85xx/twr_p102x.c
index 1eadb6d..7df08d9 100644
--- a/arch/powerpc/platforms/85xx/twr_p102x.c
+++ b/arch/powerpc/platforms/85xx/twr_p102x.c
@@ -21,8 +21,8 @@
#include <asm/pci-bridge.h>
#include <asm/udbg.h>
#include <asm/mpic.h>
-#include <asm/qe.h>
-#include <asm/qe_ic.h>
+#include <linux/fsl/qe.h>
+#include <linux/fsl/qe_ic.h>
#include <asm/fsl_guts.h>
#include <sysdev/fsl_soc.h>
diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kconfig
index 391b3f6..ae8879c 100644
--- a/arch/powerpc/platforms/Kconfig
+++ b/arch/powerpc/platforms/Kconfig
@@ -277,25 +277,6 @@ config TAU_AVERAGE
If in doubt, say N here.
-config QUICC_ENGINE
- bool "Freescale QUICC Engine (QE) Support"
- depends on FSL_SOC && PPC32
- select PPC_LIB_RHEAP
- select CRC32
- help
- The QUICC Engine (QE) is a new generation of communications
- coprocessors on Freescale embedded CPUs (akin to CPM in older chips).
- Selecting this option means that you wish to build a kernel
- for a machine with a QE coprocessor.
-
-config QE_GPIO
- bool "QE GPIO support"
- depends on QUICC_ENGINE
- select ARCH_REQUIRE_GPIOLIB
- help
- Say Y here if you're going to use hardware that connects to the
- QE GPIOs.
-
config CPM2
bool "Enable support for the CPM2 (Communications Processor Module)"
depends on (FSL_SOC_BOOKE && PPC32) || 8260
diff --git a/arch/powerpc/sysdev/Makefile b/arch/powerpc/sysdev/Makefile
index f7cb2a1..087d301 100644
--- a/arch/powerpc/sysdev/Makefile
+++ b/arch/powerpc/sysdev/Makefile
@@ -26,7 +26,6 @@ obj-$(CONFIG_FSL_85XX_CACHE_SRAM) += fsl_85xx_l2ctlr.o fsl_85xx_cache_sram.o
obj-$(CONFIG_SIMPLE_GPIO) += simple_gpio.o
obj-$(CONFIG_FSL_RIO) += fsl_rio.o fsl_rmu.o
obj-$(CONFIG_TSI108_BRIDGE) += tsi108_pci.o tsi108_dev.o
-obj-$(CONFIG_QUICC_ENGINE) += qe_lib/
mv64x60-$(CONFIG_PCI) += mv64x60_pci.o
obj-$(CONFIG_MV64X60) += $(mv64x60-y) mv64x60_pic.o mv64x60_dev.o \
mv64x60_udbg.o
diff --git a/arch/powerpc/sysdev/qe_lib/Kconfig b/arch/powerpc/sysdev/qe_lib/Kconfig
deleted file mode 100644
index 3c25199..0000000
--- a/arch/powerpc/sysdev/qe_lib/Kconfig
+++ /dev/null
@@ -1,27 +0,0 @@
-#
-# QE Communication options
-#
-
-config UCC_SLOW
- bool
- default y if SERIAL_QE
- help
- This option provides qe_lib support to UCC slow
- protocols: UART, BISYNC, QMC
-
-config UCC_FAST
- bool
- default y if UCC_GETH
- help
- This option provides qe_lib support to UCC fast
- protocols: HDLC, Ethernet, ATM, transparent
-
-config UCC
- bool
- default y if UCC_FAST || UCC_SLOW
-
-config QE_USB
- bool
- default y if USB_FSL_QE
- help
- QE USB Controller support
diff --git a/drivers/net/ethernet/freescale/fsl_pq_mdio.c b/drivers/net/ethernet/freescale/fsl_pq_mdio.c
index 583e71a..958353c 100644
--- a/drivers/net/ethernet/freescale/fsl_pq_mdio.c
+++ b/drivers/net/ethernet/freescale/fsl_pq_mdio.c
@@ -28,7 +28,7 @@
#include <linux/of_device.h>
#include <asm/io.h>
-#include <asm/ucc.h> /* for ucc_set_qe_mux_mii_mng() */
+#include <linux/fsl/ucc.h> /* for ucc_set_qe_mux_mii_mng() */
#include "gianfar.h"
diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c
index fab39e2..ef7740c 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -40,10 +40,10 @@
#include <asm/uaccess.h>
#include <asm/irq.h>
#include <asm/io.h>
-#include <asm/immap_qe.h>
-#include <asm/qe.h>
-#include <asm/ucc.h>
-#include <asm/ucc_fast.h>
+#include <linux/fsl/immap_qe.h>
+#include <linux/fsl/qe.h>
+#include <linux/fsl/ucc.h>
+#include <linux/fsl/ucc_fast.h>
#include <asm/machdep.h>
#include "ucc_geth.h"
diff --git a/drivers/net/ethernet/freescale/ucc_geth.h b/drivers/net/ethernet/freescale/ucc_geth.h
index 75f3371..a803635 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.h
+++ b/drivers/net/ethernet/freescale/ucc_geth.h
@@ -22,11 +22,11 @@
#include <linux/list.h>
#include <linux/if_ether.h>
-#include <asm/immap_qe.h>
-#include <asm/qe.h>
+#include <linux/fsl/immap_qe.h>
+#include <linux/fsl/qe.h>
-#include <asm/ucc.h>
-#include <asm/ucc_fast.h>
+#include <linux/fsl/ucc.h>
+#include <linux/fsl/ucc_fast.h>
#define DRV_DESC "QE UCC Gigabit Ethernet Controller"
#define DRV_NAME "ucc_geth"
diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig
index c854385..5432178 100644
--- a/drivers/soc/Kconfig
+++ b/drivers/soc/Kconfig
@@ -2,4 +2,6 @@ menu "SOC (System On Chip) specific Drivers"
source "drivers/soc/qcom/Kconfig"
+source "drivers/soc/qe/Kconfig"
+
endmenu
diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
index 0f7c447..5da1a482 100644
--- a/drivers/soc/Makefile
+++ b/drivers/soc/Makefile
@@ -3,3 +3,5 @@
#
obj-$(CONFIG_ARCH_QCOM) += qcom/
+
+obj-$(CONFIG_QUICC_ENGINE) += qe/
diff --git a/drivers/soc/qe/Kconfig b/drivers/soc/qe/Kconfig
new file mode 100644
index 0000000..8b03ca2
--- /dev/null
+++ b/drivers/soc/qe/Kconfig
@@ -0,0 +1,45 @@
+#
+# QE Communication options
+#
+config QUICC_ENGINE
+ bool "Freescale QUICC Engine (QE) Support"
+ depends on FSL_SOC && PPC32
+ select PPC_LIB_RHEAP
+ select CRC32
+ help
+ The QUICC Engine (QE) is a new generation of communications
+ coprocessors on Freescale embedded CPUs (akin to CPM in older chips).
+ Selecting this option means that you wish to build a kernel
+ for a machine with a QE coprocessor.
+
+config QE_GPIO
+ bool "QE GPIO support"
+ depends on QUICC_ENGINE
+ select ARCH_REQUIRE_GPIOLIB
+ help
+ Say Y here if you're going to use hardware that connects to the
+ QE GPIOs.
+
+config UCC_SLOW
+ bool
+ default y if SERIAL_QE
+ help
+ This option provides qe_lib support to UCC slow
+ protocols: UART, BISYNC, QMC
+
+config UCC_FAST
+ bool
+ default y if UCC_GETH
+ help
+ This option provides qe_lib support to UCC fast
+ protocols: HDLC, Ethernet, ATM, transparent
+
+config UCC
+ bool
+ default y if UCC_FAST || UCC_SLOW
+
+config QE_USB
+ bool
+ default y if USB_FSL_QE
+ help
+ QE USB Controller support
diff --git a/arch/powerpc/sysdev/qe_lib/Makefile b/drivers/soc/qe/Makefile
similarity index 100%
rename from arch/powerpc/sysdev/qe_lib/Makefile
rename to drivers/soc/qe/Makefile
diff --git a/arch/powerpc/sysdev/qe_lib/gpio.c b/drivers/soc/qe/gpio.c
similarity index 99%
rename from arch/powerpc/sysdev/qe_lib/gpio.c
rename to drivers/soc/qe/gpio.c
index 521e67a..1e38588 100644
--- a/arch/powerpc/sysdev/qe_lib/gpio.c
+++ b/drivers/soc/qe/gpio.c
@@ -21,7 +21,7 @@
#include <linux/gpio.h>
#include <linux/slab.h>
#include <linux/export.h>
-#include <asm/qe.h>
+#include <linux/fsl/qe.h>
struct qe_gpio_chip {
struct of_mm_gpio_chip mm_gc;
diff --git a/arch/powerpc/sysdev/qe_lib/qe.c b/drivers/soc/qe/qe.c
similarity index 99%
rename from arch/powerpc/sysdev/qe_lib/qe.c
rename to drivers/soc/qe/qe.c
index 238a07b..1c5beef 100644
--- a/arch/powerpc/sysdev/qe_lib/qe.c
+++ b/drivers/soc/qe/qe.c
@@ -32,8 +32,8 @@
#include <asm/irq.h>
#include <asm/page.h>
#include <asm/pgtable.h>
-#include <asm/immap_qe.h>
-#include <asm/qe.h>
+#include <linux/fsl/immap_qe.h>
+#include <linux/fsl/qe.h>
#include <asm/prom.h>
#include <asm/rheap.h>
diff --git a/arch/powerpc/sysdev/qe_lib/qe_ic.c b/drivers/soc/qe/qe_ic.c
similarity index 99%
rename from arch/powerpc/sysdev/qe_lib/qe_ic.c
rename to drivers/soc/qe/qe_ic.c
index b2b87c3..eb4d160 100644
--- a/arch/powerpc/sysdev/qe_lib/qe_ic.c
+++ b/drivers/soc/qe/qe_ic.c
@@ -28,7 +28,7 @@
#include <asm/irq.h>
#include <asm/io.h>
#include <asm/prom.h>
-#include <asm/qe_ic.h>
+#include <linux/fsl/qe_ic.h>
#include "qe_ic.h"
diff --git a/arch/powerpc/sysdev/qe_lib/qe_ic.h b/drivers/soc/qe/qe_ic.h
similarity index 98%
rename from arch/powerpc/sysdev/qe_lib/qe_ic.h
rename to drivers/soc/qe/qe_ic.h
index efef7ab..5c4480e 100644
--- a/arch/powerpc/sysdev/qe_lib/qe_ic.h
+++ b/drivers/soc/qe/qe_ic.h
@@ -16,7 +16,7 @@
#ifndef _POWERPC_SYSDEV_QE_IC_H
#define _POWERPC_SYSDEV_QE_IC_H
-#include <asm/qe_ic.h>
+#include <linux/fsl/qe_ic.h>
#define NR_QE_IC_INTS 64
diff --git a/arch/powerpc/sysdev/qe_lib/qe_io.c b/drivers/soc/qe/qe_io.c
similarity index 99%
rename from arch/powerpc/sysdev/qe_lib/qe_io.c
rename to drivers/soc/qe/qe_io.c
index d099941..3bdc2c7 100644
--- a/arch/powerpc/sysdev/qe_lib/qe_io.c
+++ b/drivers/soc/qe/qe_io.c
@@ -21,7 +21,7 @@
#include <linux/ioport.h>
#include <asm/io.h>
-#include <asm/qe.h>
+#include <linux/fsl/qe.h>
#include <asm/prom.h>
#include <sysdev/fsl_soc.h>
diff --git a/arch/powerpc/sysdev/qe_lib/ucc.c b/drivers/soc/qe/ucc.c
similarity index 98%
rename from arch/powerpc/sysdev/qe_lib/ucc.c
rename to drivers/soc/qe/ucc.c
index 621575b..36a206f 100644
--- a/arch/powerpc/sysdev/qe_lib/ucc.c
+++ b/drivers/soc/qe/ucc.c
@@ -21,9 +21,9 @@
#include <asm/irq.h>
#include <asm/io.h>
-#include <asm/immap_qe.h>
-#include <asm/qe.h>
-#include <asm/ucc.h>
+#include <linux/fsl/immap_qe.h>
+#include <linux/fsl/qe.h>
+#include <linux/fsl/ucc.h>
int ucc_set_qe_mux_mii_mng(unsigned int ucc_num)
{
diff --git a/arch/powerpc/sysdev/qe_lib/ucc_fast.c b/drivers/soc/qe/ucc_fast.c
similarity index 98%
rename from arch/powerpc/sysdev/qe_lib/ucc_fast.c
rename to drivers/soc/qe/ucc_fast.c
index 65aaf15..223bb36 100644
--- a/arch/powerpc/sysdev/qe_lib/ucc_fast.c
+++ b/drivers/soc/qe/ucc_fast.c
@@ -21,11 +21,11 @@
#include <linux/export.h>
#include <asm/io.h>
-#include <asm/immap_qe.h>
-#include <asm/qe.h>
+#include <linux/fsl/immap_qe.h>
+#include <linux/fsl/qe.h>
-#include <asm/ucc.h>
-#include <asm/ucc_fast.h>
+#include <linux/fsl/ucc.h>
+#include <linux/fsl/ucc_fast.h>
void ucc_fast_dump_regs(struct ucc_fast_private * uccf)
{
diff --git a/arch/powerpc/sysdev/qe_lib/ucc_slow.c b/drivers/soc/qe/ucc_slow.c
similarity index 98%
rename from arch/powerpc/sysdev/qe_lib/ucc_slow.c
rename to drivers/soc/qe/ucc_slow.c
index befaf11..7dfff79 100644
--- a/arch/powerpc/sysdev/qe_lib/ucc_slow.c
+++ b/drivers/soc/qe/ucc_slow.c
@@ -21,11 +21,11 @@
#include <linux/export.h>
#include <asm/io.h>
-#include <asm/immap_qe.h>
-#include <asm/qe.h>
+#include <linux/fsl/immap_qe.h>
+#include <linux/fsl/qe.h>
-#include <asm/ucc.h>
-#include <asm/ucc_slow.h>
+#include <linux/fsl/ucc.h>
+#include <linux/fsl/ucc_slow.h>
u32 ucc_slow_get_qe_cr_subblock(int uccs_num)
{
diff --git a/arch/powerpc/sysdev/qe_lib/usb.c b/drivers/soc/qe/usb.c
similarity index 96%
rename from arch/powerpc/sysdev/qe_lib/usb.c
rename to drivers/soc/qe/usb.c
index 27f23bd..0bcdf7d 100644
--- a/arch/powerpc/sysdev/qe_lib/usb.c
+++ b/drivers/soc/qe/usb.c
@@ -17,8 +17,8 @@
#include <linux/errno.h>
#include <linux/export.h>
#include <linux/io.h>
-#include <asm/immap_qe.h>
-#include <asm/qe.h>
+#include <linux/fsl/immap_qe.h>
+#include <linux/fsl/qe.h>
int qe_usb_clock_set(enum qe_clock clk, int rate)
{
diff --git a/drivers/spi/spi-fsl-cpm.c b/drivers/spi/spi-fsl-cpm.c
index 54b0637..b23676f 100644
--- a/drivers/spi/spi-fsl-cpm.c
+++ b/drivers/spi/spi-fsl-cpm.c
@@ -22,7 +22,7 @@
#include <linux/dma-mapping.h>
#include <linux/of_address.h>
#include <asm/cpm.h>
-#include <asm/qe.h>
+#include <linux/fsl/qe.h>
#include "spi-fsl-lib.h"
#include "spi-fsl-cpm.h"
diff --git a/drivers/tty/serial/ucc_uart.c b/drivers/tty/serial/ucc_uart.c
index d569ca5..493f74e 100644
--- a/drivers/tty/serial/ucc_uart.c
+++ b/drivers/tty/serial/ucc_uart.c
@@ -31,7 +31,7 @@
#include <linux/dma-mapping.h>
#include <linux/fs_uart_pd.h>
-#include <asm/ucc_slow.h>
+#include <linux/fsl/ucc_slow.h>
#include <linux/firmware.h>
#include <asm/reg.h>
diff --git a/drivers/usb/gadget/fsl_qe_udc.c b/drivers/usb/gadget/fsl_qe_udc.c
index ad54833..a1d4e00 100644
--- a/drivers/usb/gadget/fsl_qe_udc.c
+++ b/drivers/usb/gadget/fsl_qe_udc.c
@@ -38,7 +38,7 @@
#include <linux/usb/ch9.h>
#include <linux/usb/gadget.h>
#include <linux/usb/otg.h>
-#include <asm/qe.h>
+#include <linux/fsl/qe.h>
#include <asm/cpm.h>
#include <asm/dma.h>
#include <asm/reg.h>
diff --git a/drivers/usb/host/fhci-hcd.c b/drivers/usb/host/fhci-hcd.c
index 1cf68ea..7df4e74 100644
--- a/drivers/usb/host/fhci-hcd.c
+++ b/drivers/usb/host/fhci-hcd.c
@@ -31,7 +31,7 @@
#include <linux/of_platform.h>
#include <linux/of_gpio.h>
#include <linux/slab.h>
-#include <asm/qe.h>
+#include <linux/fsl/qe.h>
#include <asm/fsl_gtm.h>
#include "fhci.h"
diff --git a/drivers/usb/host/fhci-hub.c b/drivers/usb/host/fhci-hub.c
index 6af2512..31b4402 100644
--- a/drivers/usb/host/fhci-hub.c
+++ b/drivers/usb/host/fhci-hub.c
@@ -24,7 +24,7 @@
#include <linux/usb.h>
#include <linux/usb/hcd.h>
#include <linux/gpio.h>
-#include <asm/qe.h>
+#include <linux/fsl/qe.h>
#include "fhci.h"
/* virtual root hub specific descriptor */
diff --git a/drivers/usb/host/fhci-sched.c b/drivers/usb/host/fhci-sched.c
index 95ca598..6f1d4ad 100644
--- a/drivers/usb/host/fhci-sched.c
+++ b/drivers/usb/host/fhci-sched.c
@@ -25,7 +25,7 @@
#include <linux/io.h>
#include <linux/usb.h>
#include <linux/usb/hcd.h>
-#include <asm/qe.h>
+#include <linux/fsl/qe.h>
#include <asm/fsl_gtm.h>
#include "fhci.h"
diff --git a/drivers/usb/host/fhci.h b/drivers/usb/host/fhci.h
index 154e6a0..d7c49531 100644
--- a/drivers/usb/host/fhci.h
+++ b/drivers/usb/host/fhci.h
@@ -27,8 +27,8 @@
#include <linux/io.h>
#include <linux/usb.h>
#include <linux/usb/hcd.h>
-#include <asm/qe.h>
-#include <asm/immap_qe.h>
+#include <linux/fsl/qe.h>
+#include <linux/fsl/immap_qe.h>
#define USB_CLOCK 48000000
diff --git a/arch/powerpc/include/asm/immap_qe.h b/include/linux/fsl/immap_qe.h
similarity index 100%
rename from arch/powerpc/include/asm/immap_qe.h
rename to include/linux/fsl/immap_qe.h
diff --git a/arch/powerpc/include/asm/qe.h b/include/linux/fsl/qe.h
similarity index 99%
rename from arch/powerpc/include/asm/qe.h
rename to include/linux/fsl/qe.h
index 32b9bfa..1c9d626 100644
--- a/arch/powerpc/include/asm/qe.h
+++ b/include/linux/fsl/qe.h
@@ -20,7 +20,7 @@
#include <linux/errno.h>
#include <linux/err.h>
#include <asm/cpm.h>
-#include <asm/immap_qe.h>
+#include <linux/fsl/immap_qe.h>
#define QE_NUM_OF_SNUM 256 /* There are 256 serial number in QE */
#define QE_NUM_OF_BRGS 16
diff --git a/arch/powerpc/include/asm/qe_ic.h b/include/linux/fsl/qe_ic.h
similarity index 100%
rename from arch/powerpc/include/asm/qe_ic.h
rename to include/linux/fsl/qe_ic.h
diff --git a/arch/powerpc/include/asm/ucc.h b/include/linux/fsl/ucc.h
similarity index 96%
rename from arch/powerpc/include/asm/ucc.h
rename to include/linux/fsl/ucc.h
index 6927ac2..d448813 100644
--- a/arch/powerpc/include/asm/ucc.h
+++ b/include/linux/fsl/ucc.h
@@ -15,8 +15,8 @@
#ifndef __UCC_H__
#define __UCC_H__
-#include <asm/immap_qe.h>
-#include <asm/qe.h>
+#include <linux/fsl/immap_qe.h>
+#include <linux/fsl/qe.h>
#define STATISTICS
diff --git a/arch/powerpc/include/asm/ucc_fast.h b/include/linux/fsl/ucc_fast.h
similarity index 98%
rename from arch/powerpc/include/asm/ucc_fast.h
rename to include/linux/fsl/ucc_fast.h
index 72ea9ba..101fae7 100644
--- a/arch/powerpc/include/asm/ucc_fast.h
+++ b/include/linux/fsl/ucc_fast.h
@@ -16,10 +16,10 @@
#include <linux/kernel.h>
-#include <asm/immap_qe.h>
-#include <asm/qe.h>
+#include <linux/fsl/immap_qe.h>
+#include <linux/fsl/qe.h>
-#include <asm/ucc.h>
+#include <linux/fsl/ucc.h>
/* Receive BD's status */
#define R_E 0x80000000 /* buffer empty */
diff --git a/arch/powerpc/include/asm/ucc_slow.h b/include/linux/fsl/ucc_slow.h
similarity index 99%
rename from arch/powerpc/include/asm/ucc_slow.h
rename to include/linux/fsl/ucc_slow.h
index c44131e..61f4c83 100644
--- a/arch/powerpc/include/asm/ucc_slow.h
+++ b/include/linux/fsl/ucc_slow.h
@@ -17,10 +17,10 @@
#include <linux/kernel.h>
-#include <asm/immap_qe.h>
-#include <asm/qe.h>
+#include <linux/fsl/immap_qe.h>
+#include <linux/fsl/qe.h>
-#include <asm/ucc.h>
+#include <linux/fsl/ucc.h>
/* transmit BD's status */
#define T_R 0x80000000 /* ready bit */
--
2.1.0.27.g96db324
^ permalink raw reply related
* Re: TTM placement & caching issue/questions
From: Benjamin Herrenschmidt @ 2014-09-04 5:23 UTC (permalink / raw)
To: Jerome Glisse
Cc: Alex Deucher, linuxppc-dev, Michel Dänzer, Christian Koenig,
dri-devel
In-Reply-To: <20140904023656.GF4835@gmail.com>
On Wed, 2014-09-03 at 22:36 -0400, Jerome Glisse wrote:
> On Wed, Sep 03, 2014 at 10:31:18PM -0400, Jerome Glisse wrote:
> > On Thu, Sep 04, 2014 at 12:25:23PM +1000, Benjamin Herrenschmidt wrote:
> > > On Wed, 2014-09-03 at 22:07 -0400, Jerome Glisse wrote:
> > >
> > > > So in the meantime the attached patch should work, it just silently ignore
> > > > the caching attribute request on non x86 instead of pretending that things
> > > > are setup as expected and then latter the radeon ou nouveau hw unsetting
> > > > the snoop bit.
> > > >
> > > > It's not tested but i think it should work.
> > >
> > > I'm still getting placements with !CACHED going from bo_memcpy in
> > > ttm_io_prot() though ... I'm looking at filtering the placement
> > > attributes instead.
> > >
> > > Ben.
> >
> > Ok so this one should do the trick.
>
> Ok final version ... famous last word.
Minus a couple of obvious typos that prevent if from building, it seems
to do the trick for me with the AST driver, no more bad mappings.
I'll still send a patch that catches the incorrect mapping attempts
inside ttm_io_prot() and warns to help future debugging and avoid
"random" behaviour. (I need to fix other things in the powerpc code
in there anyway).
Cheers,
Ben.
^ permalink raw reply
* [PATCH] iommu/fsl: Fix warning resulting from adding PCI device twice
From: Varun Sethi @ 2014-09-04 6:03 UTC (permalink / raw)
To: joro, iommu, alex.williamson, Emilian.Medve, linuxppc-dev,
linux-kernel
Cc: Varun Sethi
iommu_group_get_for_dev determines the iommu group for the PCI device and adds
the device to the group.
In the PAMU driver we were again adding the device to the same group without checking
if the device already had an iommu group. This resulted in the following warning.
sysfs: cannot create duplicate filename '/devices/ffe200000.pcie/pci0000:00/0000:00:00.0/iommu_group'
------------[ cut here ]------------
WARNING: at fs/sysfs/dir.c:31
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.17.0-rc3-00002-g7505cea-dirty #126
task: c0000001fe0a0000 ti: c0000001fe044000 task.ti: c0000001fe044000
NIP: c00000000018879c LR: c000000000188798 CTR: c00000000001ea50
REGS: c0000001fe047040 TRAP: 0700 Not tainted (3.17.0-rc3-00002-g7505cea-dirty)
MSR: 0000000080029000 <CE,EE,ME> CR: 24ad8e22 XER: 20000000
SOFTE: 1
GPR00: c000000000188798 c0000001fe0472c0 c0000000009a52e0 0000000000000065
GPR04: 0000000000000001 0000000000000000 3a30303a00000000 0000000027000000
GPR08: 2f696f6d00000000 c0000000008d3830 c0000000009b3938 c0000000009bb3d0
GPR12: 0000000028ad8e24 c00000000fff4000 c00000000000205c 0000000000000000
GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR20: 0000000000000000 0000000000000000 0000000000000000 c0000000008a4c70
GPR24: c0000000007e9010 c0000001fe0140a8 ffffffffffffffef 0000000000000001
GPR28: c0000001fe22ebb8 c0000000007e9010 c00000000090bf10 c0000001fe220000
NIP [c00000000018879c] .sysfs_warn_dup+0x74/0xa4
LR [c000000000188798] .sysfs_warn_dup+0x70/0xa4
Call Trace:
[c0000001fe0472c0] [c000000000188798] .sysfs_warn_dup+0x70/0xa4 (unreliable)
[c0000001fe047350] [c000000000188d34] .sysfs_do_create_link_sd.clone.2+0x168/0x174
[c0000001fe047400] [c0000000004b3cf8] .iommu_group_add_device+0x78/0x244
[c0000001fe0474b0] [c0000000004b6964] .fsl_pamu_add_device+0x88/0x1a8
[c0000001fe047570] [c0000000004b3960] .iommu_bus_notifier+0xdc/0x15c
[c0000001fe047600] [c000000000059848] .notifier_call_chain+0x8c/0xe8
[c0000001fe0476a0] [c000000000059d04] .__blocking_notifier_call_chain+0x58/0x84
[c0000001fe047750] [c00000000036619c] .device_add+0x464/0x5c8
[c0000001fe047820] [c000000000300ebc] .pci_device_add+0x14c/0x17c
[c0000001fe0478c0] [c000000000300fbc] .pci_scan_single_device+0xd0/0xf4
[c0000001fe047970] [c00000000030104c] .pci_scan_slot+0x6c/0x18c
[c0000001fe047a10] [c00000000030226c] .pci_scan_child_bus+0x40/0x114
[c0000001fe047ac0] [c000000000021974] .pcibios_scan_phb+0x240/0x2c8
[c0000001fe047b70] [c00000000085a970] .pcibios_init+0x64/0xc8
[c0000001fe047c00] [c000000000001884] .do_one_initcall+0xbc/0x224
[c0000001fe047d00] [c000000000852d50] .kernel_init_freeable+0x14c/0x21c
[c0000001fe047db0] [c000000000002078] .kernel_init+0x1c/0xfa4
[c0000001fe047e30] [c000000000000884] .ret_from_kernel_thread+0x58/0xd4
Instruction dump:
7c7f1b79 4182001c 7fe4fb78 7f83e378 38a01000 4bffc905 60000000 7c641b78
e87e8008 7fa5eb78 48482ff5 60000000 <0fe00000> 7fe3fb78 4bf7bd39 60000000
Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com>
---
drivers/iommu/fsl_pamu_domain.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
index 61d1daf..af5a548 100644
--- a/drivers/iommu/fsl_pamu_domain.c
+++ b/drivers/iommu/fsl_pamu_domain.c
@@ -984,7 +984,7 @@ static int fsl_pamu_add_device(struct device *dev)
struct iommu_group *group = ERR_PTR(-ENODEV);
struct pci_dev *pdev;
const u32 *prop;
- int ret, len;
+ int ret = 0, len;
/*
* For platform devices we allocate a separate group for
@@ -1007,7 +1007,13 @@ static int fsl_pamu_add_device(struct device *dev)
if (IS_ERR(group))
return PTR_ERR(group);
- ret = iommu_group_add_device(group, dev);
+ /*
+ * Check if device has already been added to an iommu group.
+ * Group could have already been created for a PCI device in
+ * the iommu_group_get_for_dev path.
+ */
+ if (!iommu_group_get(dev))
+ ret = iommu_group_add_device(group, dev);
iommu_group_put(group);
return ret;
--
1.7.9.5
^ permalink raw reply related
* Re: TTM placement & caching issue/questions
From: Gabriel Paubert @ 2014-09-04 6:45 UTC (permalink / raw)
To: Jerome Glisse
Cc: Michel Dänzer, dri-devel, linuxppc-dev, Alex Deucher,
Christian Koenig
In-Reply-To: <20140904023656.GF4835@gmail.com>
On Wed, Sep 03, 2014 at 10:36:57PM -0400, Jerome Glisse wrote:
> On Wed, Sep 03, 2014 at 10:31:18PM -0400, Jerome Glisse wrote:
> > On Thu, Sep 04, 2014 at 12:25:23PM +1000, Benjamin Herrenschmidt wrote:
> > > On Wed, 2014-09-03 at 22:07 -0400, Jerome Glisse wrote:
> > >
> > > > So in the meantime the attached patch should work, it just silently ignore
> > > > the caching attribute request on non x86 instead of pretending that things
> > > > are setup as expected and then latter the radeon ou nouveau hw unsetting
> > > > the snoop bit.
> > > >
> > > > It's not tested but i think it should work.
> > >
> > > I'm still getting placements with !CACHED going from bo_memcpy in
> > > ttm_io_prot() though ... I'm looking at filtering the placement
> > > attributes instead.
> > >
> > > Ben.
> >
> > Ok so this one should do the trick.
>
> Ok final version ... famous last word.
[snipped older version]
> >From 236038e18dc303bb9aa877922e01963d3fb0b7af Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Glisse?= <jglisse@redhat.com>
> Date: Wed, 3 Sep 2014 22:04:34 -0400
> Subject: [PATCH] drm/ttm: force cached mapping on non x86 platform.
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> People interested in providing uncached or write combined mapping
> on there architecture need to do the ground work inside there arch
s/there/their/g
> specific code to allow to break the linear kernel mapping so that
> page mapping attributes can be updated, in the meantime force cached
> mapping for non x86 architecture.
>
> Signed-off-by: Jérôme Glisse <jglisse@redhat.com>
> ---
> drivers/gpu/drm/radeon/radeon_ttm.c | 2 +-
> drivers/gpu/drm/ttm/ttm_bo.c | 2 +-
> drivers/gpu/drm/ttm/ttm_tt.c | 32 +++++++++++++++++++++-----------
> include/drm/ttm/ttm_bo_driver.h | 2 +-
> 4 files changed, 24 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
> index 72afe82..4dd5060 100644
> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> @@ -304,7 +304,7 @@ static int radeon_move_vram_ram(struct ttm_buffer_object *bo,
> return r;
> }
>
> - r = ttm_tt_set_placement_caching(bo->ttm, tmp_mem.placement);
> + r = ttm_tt_set_placement_caching(bo->ttm, &tmp_mem.placement);
> if (unlikely(r)) {
> goto out_cleanup;
> }
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 3da89d5..4dc21c3 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -305,7 +305,7 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
> goto out_err;
> }
>
> - ret = ttm_tt_set_placement_caching(bo->ttm, mem->placement);
> + ret = ttm_tt_set_placement_caching(bo->ttm, &mem->placement);
> if (ret)
> goto out_err;
>
> diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
> index bf080ab..a0df803 100644
> --- a/drivers/gpu/drm/ttm/ttm_tt.c
> +++ b/drivers/gpu/drm/ttm/ttm_tt.c
> @@ -89,14 +89,6 @@ static inline int ttm_tt_set_page_caching(struct page *p,
>
> return ret;
> }
> -#else /* CONFIG_X86 */
> -static inline int ttm_tt_set_page_caching(struct page *p,
> - enum ttm_caching_state c_old,
> - enum ttm_caching_state c_new)
> -{
> - return 0;
> -}
> -#endif /* CONFIG_X86 */
>
> /*
> * Change caching policy for the linear kernel map
> @@ -149,19 +141,37 @@ out_err:
> return ret;
> }
>
> -int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t placement)
> +int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t *placement)
> {
> enum ttm_caching_state state;
>
> - if (placement & TTM_PL_FLAG_WC)
> + if (*placement & TTM_PL_FLAG_WC)
> state = tt_wc;
> - else if (placement & TTM_PL_FLAG_UNCACHED)
> + else if (*placement & TTM_PL_FLAG_UNCACHED)
> state = tt_uncached;
> else
> state = tt_cached;
>
> return ttm_tt_set_caching(ttm, state);
> }
> +#else /* CONFIG_X86 */
> +int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t *placement)
> +{
> + if (*placement & (TTM_PL_TT | TTM_PL_FLAG_SYSTEM)) {
> + ttm->caching_state = tt_cached;
> + *placement &= ~TTM_PL_MASK_CACHING;
> + *placement |= TTM_PL_FLAG_CACHED;
> + } else {
> + if (*placement & TTM_PL_FLAG_WC)
> + ttm->caching_state = tt_wc;
> + else if (placement & TTM_PL_FLAG_UNCACHED)
> + ttm->caching_state = tt_uncached;
> + else
> + ttm->caching_state = tt_cached;
> + }
> + return 0;
> +}
> +#endif /* CONFIG_X86 */
> EXPORT_SYMBOL(ttm_tt_set_placement_caching);
>
> void ttm_tt_destroy(struct ttm_tt *ttm)
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index 1d9f0f1..cbc5ad2 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -669,7 +669,7 @@ extern int ttm_tt_swapin(struct ttm_tt *ttm);
> * hit RAM. This function may be very costly as it involves global TLB
> * and cache flushes and potential page splitting / combining.
> */
> -extern int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t placement);
> +extern int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t *placement);
> extern int ttm_tt_swapout(struct ttm_tt *ttm,
> struct file *persistent_swap_storage);
>
> --
> 1.9.3
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply
* Re: TTM placement & caching issue/questions
From: Michel Dänzer @ 2014-09-04 7:19 UTC (permalink / raw)
To: Jerome Glisse, Benjamin Herrenschmidt
Cc: Alex Deucher, linuxppc-dev, Michel Dänzer, Christian Koenig,
dri-devel
In-Reply-To: <20140904023656.GF4835@gmail.com>
On 04.09.2014 11:36, Jerome Glisse wrote:
> On Wed, Sep 03, 2014 at 10:31:18PM -0400, Jerome Glisse wrote:
>> On Thu, Sep 04, 2014 at 12:25:23PM +1000, Benjamin Herrenschmidt wrote:
>>> On Wed, 2014-09-03 at 22:07 -0400, Jerome Glisse wrote:
>>>
>>>> So in the meantime the attached patch should work, it just silently ignore
>>>> the caching attribute request on non x86 instead of pretending that things
>>>> are setup as expected and then latter the radeon ou nouveau hw unsetting
>>>> the snoop bit.
>>>>
>>>> It's not tested but i think it should work.
>>>
>>> I'm still getting placements with !CACHED going from bo_memcpy in
>>> ttm_io_prot() though ... I'm looking at filtering the placement
>>> attributes instead.
>>>
>>> Ben.
>>
>> Ok so this one should do the trick.
>
> Ok final version ... famous last word.
[...]
> +#else /* CONFIG_X86 */
> +int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t *placement)
> +{
> + if (*placement & (TTM_PL_TT | TTM_PL_FLAG_SYSTEM)) {
> + ttm->caching_state = tt_cached;
> + *placement &= ~TTM_PL_MASK_CACHING;
> + *placement |= TTM_PL_FLAG_CACHED;
NAK, this will break AGP on PowerMacs.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
^ permalink raw reply
* Re: TTM placement & caching issue/questions
From: Michel Dänzer @ 2014-09-04 7:12 UTC (permalink / raw)
To: Jerome Glisse, Benjamin Herrenschmidt
Cc: Alex Deucher, linuxppc-dev, Christian Koenig, dri-devel
In-Reply-To: <20140904015548.GB4835@gmail.com>
On 04.09.2014 10:55, Jerome Glisse wrote:
>
> While i agree about the issue of incoherent double map of same page, i
> think we have more issue. For instance lattely AMD have been pushing a
> lot of patches to move things to use uncached memory for radeon and as
> usual thoses patches comes with no comment to the motivations of those
> changes.
That would have been a fair review comment...
> What i understand is that uncached mapping for some frequently use buffer
> give a significant performance boost (i am assuming this has to do with
> all the snoop pci transaction overhead).
Exactly, although it's a win even if the data is written by the CPU only
once and read by the GPU only once.
> This also means that we need to fix ttm_tt_set_placement_caching so that
> when it returns an error it switches to cached mapping. Which will always
> work.
GTT with AGP being one exception.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
^ permalink raw reply
* Re: TTM placement & caching issue/questions
From: Benjamin Herrenschmidt @ 2014-09-04 7:54 UTC (permalink / raw)
To: Michel Dänzer
Cc: linuxppc-dev, Michel Dänzer, dri-devel, Alex Deucher,
Jerome Glisse, Christian Koenig
In-Reply-To: <54081282.3040005@daenzer.net>
On Thu, 2014-09-04 at 16:19 +0900, Michel Dänzer wrote:
> > +#else /* CONFIG_X86 */
> > +int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t
> *placement)
> > +{
> > + if (*placement & (TTM_PL_TT | TTM_PL_FLAG_SYSTEM)) {
> > + ttm->caching_state = tt_cached;
> > + *placement &= ~TTM_PL_MASK_CACHING;
> > + *placement |= TTM_PL_FLAG_CACHED;
>
> NAK, this will break AGP on PowerMacs.
... which doesn't work reliably anyway with DRI2 :-)
The problem is ... with DRI1 I think we had tricks to take out the
AGP from the linear mapping but that want away, didn't we ?
In any case, we are playing with fire on these by allowing the
cache paradox. It just happens that those old CPUs aren't *that*
aggressive at speculative prefetch and we probably rarely hit the
lockups that they would cause...
Michel, what do you recommend we do then ? The patch I sent to
double check in ttm_io_prot() has a specific hack to avoid warning
on PowerMac for the above reason, but we need to fix Jerome if we
want to keep that broken-by-design Mac AGP functionality going :-)
Maybe we could add a similar ifdef in the above ?
Cheers,
Ben.
^ permalink raw reply
* Re: TTM placement & caching issue/questions
From: Thomas Hellstrom @ 2014-09-04 7:44 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Jerome Glisse, linuxppc-dev, Michel Danzer, dri-devel
In-Reply-To: <1409789547.30640.136.camel@pasglop>
Hi!
Let me try to bring some clarity and suggestions into this.
On 09/04/2014 02:12 AM, Benjamin Herrenschmidt wrote:
> Hi folks !
>
> I've been tracking down some problems with the recent DRI on powerpc and
> stumbled upon something that doesn't look right, and not necessarily
> only for us.
>
> Now it's possible that I haven't fully understood the code here and I
> also don't know to what extent some of that behaviour is necessary for
> some platforms such as Intel GTT bits.
>
> What I've observed with a simple/dumb (no DMA) driver like AST (but this
> probably happens more generally) is that when evicting a BO from VRAM
> into System memory, the TTM tries to preserve the existing caching
> attributes of the VRAM object.
>
> >From what I can tell, we end up with going from VRAM to System memory
> type, and we eventually call ttm_bo_select_caching() to select the
> caching option for the target.
>
> This will, from what I can tell, try to use the same caching mode as the
> original object:
>
> if ((cur_placement & caching) != 0)
> result |= (cur_placement & caching);
>
> And cur_placement comes from bo->mem.placement which as far as I can
> tell is based on the placement array which the drivers set up.
This originates from the fact that when evicting GTT memory, on x86 it's
unnecessary and undesirable to switch caching mode when going to system.
>
> Now they tend to uniformly setup the placement for System memory as
> TTM_PL_MASK_CACHING which enables all caching modes.
>
> So I end up with, for example, my System memory BOs having
> TTM_PL_FLAG_CACHED not set (though they also don't have
> TTM_PL_FLAG_UNCACHED) and TTM_PL_FLAG_WC.
>
> We don't seem to use the man->default_caching (which will have
> TTM_PL_FLAG_CACHED) unless there is no matching bit at all between the
> proposed placement and the existing caching mode.
>
> Now this is a problem for several reason that I can think of:
>
> - On a number of powerpc platforms, such as all our server 64-bit one
> for example, it's actually illegal to map system memory non-cached. The
> system is fully cache coherent for all possible DMA originators (that we
> care about at least) and mapping memory non-cachable while it's mapped
> cachable in the linear mapping can cause nasty cache paradox which, when
> detected by HW, can checkstop the system.
>
> - A similar issue exists, afaik, on ARM >= v7, so anything mapped
> non-cachable must be removed from the linear mapping explicitly since
> otherwise it can be speculatively prefetched into the cache.
>
> - I don't know about x86, but even then, it looks quite sub-optimal to
> map the memory backing of the BOs and access it using a WC rather than a
> cachable mapping attribute.
Last time I tested, (and it seems like Michel is on the same track),
writing with the CPU to write-combined memory was substantially faster
than writing to cached memory, with the additional side-effect that CPU
caches are left unpolluted.
Moreover (although only tested on Intel's embedded chipsets), texturing
from cpu-cache-coherent PCI memory was a real GPU performance hog
compared to texturing from non-snooped memory. Hence, whenever a buffer
could be classified as GPU-read-only (or almost at least), it should be
placed in write-combined memory.
>
> Now, some folks on IRC mentioned that there might be reasons for the
> current behaviour as to not change the caching attributes when going
> in/out of the GTT on Intel, I don't know how that relates and how that
> works, but maybe that should be enforced by having a different placement
> mask specifically on those chipsets.
>
> Dave, should we change the various PCI drivers for generally coherent
> devices such that the System memory type doesn't allow placements
> without CACHED attribute ? Or at least on coherent platforms ? How do
> detect that ? Should we have a TTM helper to establish the default
> memory placement attributes that "normal PCI" drivers call to set that
> up so we can have all the necessary arch ifdefs in one single place, at
> least for "classic PCI/PCIe" stuff (AGP might need additional tweaks) ?
>
> Non-PCI and "special" drivers like Intel can use a different set of
> placement attributes to represent the requirements of those specific
> platforms (mostly thinking of embedded ARM here which under some
> circumstances might actually require non-cached mappings).
> Or am I missing another part of the puzzle ?
>
> As it-is, things are broken for me even for dumb drivers, and I suspect
> to a large extent with radeon and nouveau too, though in some case we
> might get away with it most of the time ... until the machine locks up
> for some unexplainable reason... This might cause problems on existing
> distros such as RHEL7 with our radeon adapters even.
>
> Any suggestion of what's the best approach to fix it ? I'm happy to
> produce the patches but I'm not that familiar with the TTM so I would
> like to make sure I'm the right track first :-)
I dislike the approach of rewriting placements. In some cases I think it
won't even work, because placements are declared 'static const'
What I'd suggest is instead to intercept the driver response from
init_mem_type() and filter out undesired caching modes from
available_caching and default_caching, perhaps also looking at whether
the memory type is mappable or not. This should have the additional
benefit of working everywhere, and if a caching mode is selected that's
not available on the platform, you'll simply get an error. (I guess?)
/Thomas
>
> Cheers,
> Ben.
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: TTM placement & caching issue/questions
From: Michel Dänzer @ 2014-09-04 7:59 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Alex Deucher, linuxppc-dev, Michel Dänzer, Christian Koenig,
dri-devel
In-Reply-To: <1409817275.4246.40.camel@pasglop>
On 04.09.2014 16:54, Benjamin Herrenschmidt wrote:
> On Thu, 2014-09-04 at 16:19 +0900, Michel Dänzer wrote:
>>> +#else /* CONFIG_X86 */
>>> +int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t
>> *placement)
>>> +{
>>> + if (*placement & (TTM_PL_TT | TTM_PL_FLAG_SYSTEM)) {
>>> + ttm->caching_state = tt_cached;
>>> + *placement &= ~TTM_PL_MASK_CACHING;
>>> + *placement |= TTM_PL_FLAG_CACHED;
>>
>> NAK, this will break AGP on PowerMacs.
>
> ... which doesn't work reliably anyway with DRI2 :-)
Define 'not reliably'. I have uptimes of weeks, and I'm pretty sure I'm
not alone, at least with AGP 1x it seems to work quite well for most
people. So I don't see the justification for intentionally breaking it
completely for all of us.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
^ permalink raw reply
* Re: TTM placement & caching issue/questions
From: Michel Dänzer @ 2014-09-04 7:59 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Alex Deucher, linuxppc-dev, Michel Dänzer, Christian Koenig,
dri-devel
In-Reply-To: <54081BC5.8000703@daenzer.net>
On 04.09.2014 16:59, Michel Dänzer wrote:
> On 04.09.2014 16:54, Benjamin Herrenschmidt wrote:
>> On Thu, 2014-09-04 at 16:19 +0900, Michel Dänzer wrote:
>>>> +#else /* CONFIG_X86 */
>>>> +int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t
>>> *placement)
>>>> +{
>>>> + if (*placement & (TTM_PL_TT | TTM_PL_FLAG_SYSTEM)) {
>>>> + ttm->caching_state = tt_cached;
>>>> + *placement &= ~TTM_PL_MASK_CACHING;
>>>> + *placement |= TTM_PL_FLAG_CACHED;
>>>
>>> NAK, this will break AGP on PowerMacs.
>>
>> ... which doesn't work reliably anyway with DRI2 :-)
>
> Define 'not reliably'. I have uptimes of weeks, and I'm pretty sure I'm
> not alone, at least with AGP 1x it seems to work quite well for most
> people. So I don't see the justification for intentionally breaking it
> completely for all of us.
Even more so because PCI GART is unusably slow in general.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
^ permalink raw reply
* Re: TTM placement & caching issue/questions
From: Benjamin Herrenschmidt @ 2014-09-04 8:06 UTC (permalink / raw)
To: Thomas Hellstrom; +Cc: Jerome Glisse, linuxppc-dev, Michel Danzer, dri-devel
In-Reply-To: <54081844.7000604@vmware.com>
On Thu, 2014-09-04 at 09:44 +0200, Thomas Hellstrom wrote:
> > This will, from what I can tell, try to use the same caching mode as the
> > original object:
> >
> > if ((cur_placement & caching) != 0)
> > result |= (cur_placement & caching);
> >
> > And cur_placement comes from bo->mem.placement which as far as I can
> > tell is based on the placement array which the drivers set up.
>
> This originates from the fact that when evicting GTT memory, on x86 it's
> unnecessary and undesirable to switch caching mode when going to system.
But that's what I don't quite understand. We have two different mappings
here. The VRAM and the memory object. We wouldn't be "switching"... we
are creating a temporary mapping for the memory object in order to do
the memcpy, but we seem to be doing it by using the caching attributes
of the VRAM object.... or am I missing something ? I don't see how that
makes sense so I suppose I'm missing something here :-)
> Last time I tested, (and it seems like Michel is on the same track),
> writing with the CPU to write-combined memory was substantially faster
> than writing to cached memory, with the additional side-effect that CPU
> caches are left unpolluted.
That's very strange indeed. It's certainly an x86 specific artifact,
even if we were allowed by our hypervisor to map memory non-cachable
(the HW somewhat can), we tend to have a higher throughput by going
cachable, but that could be due to the way the PowerBus works (it's
basically very biased toward cachable transactions).
> I dislike the approach of rewriting placements. In some cases I think it
> won't even work, because placements are declared 'static const'
>
> What I'd suggest is instead to intercept the driver response from
> init_mem_type() and filter out undesired caching modes from
> available_caching and default_caching,
This was my original intent but Jerome seems to have different ideas
(see his proposed patches). I'm happy to revive mine as well and post it
as an alternative after I've tested it a bit more (tomorrow).
> perhaps also looking at whether
> the memory type is mappable or not. This should have the additional
> benefit of working everywhere, and if a caching mode is selected that's
> not available on the platform, you'll simply get an error. (I guess?)
You mean that if not mappable we don't bother filtering ?
The rule is really for me pretty simple:
- If it's system memory (PL_SYSTEM/PL_TT), it MUST be cachable
- If it's PCIe memory space (VRAM, registers, ...) it MUST be
non-cachable.
Cheers,
Ben.
> /Thomas
>
>
> >
> > Cheers,
> > Ben.
> >
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: TTM placement & caching issue/questions
From: Benjamin Herrenschmidt @ 2014-09-04 8:07 UTC (permalink / raw)
To: Michel Dänzer
Cc: Alex Deucher, linuxppc-dev, Michel Dänzer, Christian Koenig,
dri-devel
In-Reply-To: <54081BC5.8000703@daenzer.net>
On Thu, 2014-09-04 at 16:59 +0900, Michel Dänzer wrote:
>
> Define 'not reliably'. I have uptimes of weeks, and I'm pretty sure I'm
> not alone, at least with AGP 1x it seems to work quite well for most
> people. So I don't see the justification for intentionally breaking it
> completely for all of us.
Oh I wasn't arguing for breaking it, just jesting. We need to keep it
working. It's amazing how well broken stuff actually work though :-)
I mean, it's architecturally broken and if we get a collision between
the cache and the NCU, the chip will crash. We just get lucky I suppose.
Anyway, I'll try a different approach tomorrow see how it goes.
Cheers,
Ben.
^ permalink raw reply
* RE: bit fields && data tearing
From: David Laight @ 2014-09-04 8:43 UTC (permalink / raw)
To: 'Benjamin Herrenschmidt', Peter Hurley
Cc: Jakub Jelinek, linux-arch@vger.kernel.org, Tony Luck,
linux-ia64@vger.kernel.org, Oleg Nesterov,
linux-kernel@vger.kernel.org, Paul Mackerras, Paul E. McKenney,
linuxppc-dev@lists.ozlabs.org, Miroslav Franc, Richard Henderson
In-Reply-To: <1409785893.30640.118.camel@pasglop>
RnJvbTogIEJlbmphbWluIEhlcnJlbnNjaG1pZHQNCj4gT24gV2VkLCAyMDE0LTA5LTAzIGF0IDE4
OjUxIC0wNDAwLCBQZXRlciBIdXJsZXkgd3JvdGU6DQo+IA0KPiA+IEFwb2xvZ2llcyBmb3IgaGlq
YWNraW5nIHRoaXMgdGhyZWFkIGJ1dCBJIG5lZWQgdG8gZXh0ZW5kIHRoaXMgZGlzY3Vzc2lvbg0K
PiA+IHNvbWV3aGF0IHJlZ2FyZGluZyB3aGF0IGEgY29tcGlsZXIgbWlnaHQgZG8gd2l0aCBhZGph
Y2VudCBmaWVsZHMgaW4gYSBzdHJ1Y3R1cmUuDQo+ID4NCj4gPiBUaGUgdHR5IHN1YnN5c3RlbSBk
ZWZpbmVzIGEgbGFyZ2UgYWdncmVnYXRlIHN0cnVjdHVyZSwgc3RydWN0IHR0eV9zdHJ1Y3QuDQo+
ID4gSW1wb3J0YW50bHksIHNldmVyYWwgZGlmZmVyZW50IGxvY2tzIGFwcGx5IHRvIGRpZmZlcmVu
dCBmaWVsZHMgd2l0aGluIHRoYXQNCj4gPiBzdHJ1Y3R1cmU7IGllLiwgYSBzcGVjaWZpYyBzcGlu
bG9jayB3aWxsIGJlIGNsYWltZWQgYmVmb3JlIHVwZGF0aW5nIG9yIGFjY2Vzc2luZw0KPiA+IGNl
cnRhaW4gZmllbGRzIHdoaWxlIGEgZGlmZmVyZW50IHNwaW5sb2NrIHdpbGwgYmUgY2xhaW1lZCBi
ZWZvcmUgdXBkYXRpbmcgb3INCj4gPiBhY2Nlc3NpbmcgY2VydGFpbiBfYWRqYWNlbnRfIGZpZWxk
cy4NCj4gPg0KPiA+IFdoYXQgaXMgbmVjZXNzYXJ5IGFuZCBzdWZmaWNpZW50IHRvIHByZXZlbnQg
YWNjaWRlbnRhbCBmYWxzZS1zaGFyaW5nPw0KPiA+IFRoZSBwYXRjaCBiZWxvdyB3YXMgZmxhZ2dl
ZCBhcyBpbnN1ZmZpY2llbnQgb24gaWE2NCwgYW5kIHBvc3NpYmx5IEFSTS4NCj4gDQo+IFdlIGV4
cGVjdCBuYXRpdmUgYWxpZ25lZCBzY2FsYXIgdHlwZXMgdG8gYmUgYWNjZXNzZWQgYXRvbWljYWxs
eSAodGhlDQo+IHJlYWQvbW9kaWZ5L3dyaXRlIG9mIGEgbGFyZ2VyIHF1YW50aXR5IHRoYXQgZ2Nj
IGRvZXMgb24gc29tZSBiaXRmaWVsZA0KPiBjYXNlcyBoYXMgYmVlbiBmbGFnZ2VkIGFzIGEgZ2Nj
IGJ1ZywgYnV0IHNob3VsZG4ndCBoYXBwZW4gb24gbm9ybWFsDQo+IHNjYWxhciB0eXBlcykuDQoN
ClRoYXQgaXNuJ3QgdHJ1ZSBvbiBhbGwgYXJjaGl0ZWN0dXJlcyBmb3IgaXRlbXMgc21hbGxlciB0
aGFuIGEgbWFjaGluZSB3b3JkLg0KQXQgbGVhc3Qgb25lIGhhcyB0byBkbyBybXcgZm9yIGJ5dGUg
YWNjZXNzZXMuDQoNCglEYXZpZA0KDQo+IEkgYW0gbm90IDEwMCUgY2VydGFpbiBvZiAiYm9vbCIg
aGVyZSwgSSBhc3N1bWUgaXQncyB0cmVhdGVkIGFzIGEgbm9ybWFsDQo+IHNjYWxhciBhbmQgdGh1
cyBhdG9taWMgYnV0IGlmIHVuc3VyZSwgeW91IGNhbiBhbHdheXMgdXNlIGludC4NCj4gDQo+IEFu
b3RoZXIgb3B0aW9uIGlzIHRvIHVzZSB0aGUgYXRvbWljIGJpdG9wcyBhbmQgbWFrZSB0aGVzZSBi
aXRzIGluIGENCj4gYml0bWFzayBidXQgdGhhdCBpcyBwcm9iYWJseSB1bm5lY2Vzc2FyeSBpZiB5
b3UgaGF2ZSBsb2NrcyBhbHJlYWR5Lg0KPiANCj4gQ2hlZXJzLA0KPiBCZW4uDQo=
^ permalink raw reply
* Re: TTM placement & caching issue/questions
From: Thomas Hellstrom @ 2014-09-04 8:46 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Jerome Glisse, linuxppc-dev, Michel Danzer, dri-devel
In-Reply-To: <1409817962.4246.51.camel@pasglop>
On 09/04/2014 10:06 AM, Benjamin Herrenschmidt wrote:
> On Thu, 2014-09-04 at 09:44 +0200, Thomas Hellstrom wrote:
>
>>> This will, from what I can tell, try to use the same caching mode as the
>>> original object:
>>>
>>> if ((cur_placement & caching) != 0)
>>> result |= (cur_placement & caching);
>>>
>>> And cur_placement comes from bo->mem.placement which as far as I can
>>> tell is based on the placement array which the drivers set up.
>> This originates from the fact that when evicting GTT memory, on x86 it's
>> unnecessary and undesirable to switch caching mode when going to system.
> But that's what I don't quite understand. We have two different mappings
> here. The VRAM and the memory object. We wouldn't be "switching"... we
> are creating a temporary mapping for the memory object in order to do
> the memcpy, but we seem to be doing it by using the caching attributes
> of the VRAM object.... or am I missing something ? I don't see how that
> makes sense so I suppose I'm missing something here :-)
Well, the intention when TTM was written was that the driver writer
should be smart enough that when he wanted a move from unached VRAM to
system, he'd request cached system in the placement flags in the first
place. If TTM somehow overrides such a request, that's a bug in TTM.
If the move, for example, is a result of an eviction, then the driver
evict_flags() function should ideally look at the current placement and
decide about a suitable placement based on that: vram-to-system moves
should generally request cacheable memory if the next access is expected
by the CPU. Probably write-combined otherwise.
If the move is the result of a TTM swapout, TTM will automatically
select cachable system, and for most other moves, I think the driver
writer is in full control.
>
>> Last time I tested, (and it seems like Michel is on the same track),
>> writing with the CPU to write-combined memory was substantially faster
>> than writing to cached memory, with the additional side-effect that CPU
>> caches are left unpolluted.
> That's very strange indeed. It's certainly an x86 specific artifact,
> even if we were allowed by our hypervisor to map memory non-cachable
> (the HW somewhat can), we tend to have a higher throughput by going
> cachable, but that could be due to the way the PowerBus works (it's
> basically very biased toward cachable transactions).
>
>> I dislike the approach of rewriting placements. In some cases I think it
>> won't even work, because placements are declared 'static const'
>>
>> What I'd suggest is instead to intercept the driver response from
>> init_mem_type() and filter out undesired caching modes from
>> available_caching and default_caching,
> This was my original intent but Jerome seems to have different ideas
> (see his proposed patches). I'm happy to revive mine as well and post it
> as an alternative after I've tested it a bit more (tomorrow).
>
>> perhaps also looking at whether
>> the memory type is mappable or not. This should have the additional
>> benefit of working everywhere, and if a caching mode is selected that's
>> not available on the platform, you'll simply get an error. (I guess?)
> You mean that if not mappable we don't bother filtering ?
>
> The rule is really for me pretty simple:
>
> - If it's system memory (PL_SYSTEM/PL_TT), it MUST be cachable
>
> - If it's PCIe memory space (VRAM, registers, ...) it MUST be
> non-cachable.
Yes, something along these lines. I guess checking for VRAM or
TTM_MEMTYPE_FLAG_FIXED would perhaps do the trick
/Thomas
>
> Cheers,
> Ben.
>
>> /Thomas
>>
>>
>>> Cheers,
>>> Ben.
>>>
>>>
>>> _______________________________________________
>>> dri-devel mailing list
>>> dri-devel@lists.freedesktop.org
>>> https://urldefense.proofpoint.com/v1/url?u=http://lists.freedesktop.org/mailman/listinfo/dri-devel&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=l5Ago9ekmVFZ3c4M6eauqrJWGwjf6fTb%2BP3CxbBFkVM%3D%0A&m=C9AHL1VngKBOxe2UrNP2eCZo6FLqdlr6Y90rpfE5rUs%3D%0A&s=73da0633bafc5d54bf116bc861d48d13c39cf8f41832adfb739709e98ec05553
>
^ 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