| Message ID | 1403603667-11302-3-git-send-email-acourbot@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jun 24, 2014 at 06:54:26PM +0900, Alexandre Courbot wrote: > From: Lucas Stach <dev@lynxeye.de> > > On architectures for which access to GPU memory is non-coherent, > caches need to be flushed and invalidated explicitly at the > appropriate places. Introduce two small helpers to make things > easy for TTM-based drivers. Have you run this with DMA API debugging enabled? I suspect you haven't, and I recommend that you do.
On 06/24/2014 07:02 PM, Russell King - ARM Linux wrote: > On Tue, Jun 24, 2014 at 06:54:26PM +0900, Alexandre Courbot wrote: >> From: Lucas Stach <dev@lynxeye.de> >> >> On architectures for which access to GPU memory is non-coherent, >> caches need to be flushed and invalidated explicitly at the >> appropriate places. Introduce two small helpers to make things >> easy for TTM-based drivers. > > Have you run this with DMA API debugging enabled? I suspect you haven't, > and I recommend that you do. # cat /sys/kernel/debug/dma-api/error_count 162621 (?°?°??? ???)
On 06/24/2014 07:33 PM, Alexandre Courbot wrote: > On 06/24/2014 07:02 PM, Russell King - ARM Linux wrote: >> On Tue, Jun 24, 2014 at 06:54:26PM +0900, Alexandre Courbot wrote: >>> From: Lucas Stach <dev@lynxeye.de> >>> >>> On architectures for which access to GPU memory is non-coherent, >>> caches need to be flushed and invalidated explicitly at the >>> appropriate places. Introduce two small helpers to make things >>> easy for TTM-based drivers. >> >> Have you run this with DMA API debugging enabled? I suspect you haven't, >> and I recommend that you do. > > # cat /sys/kernel/debug/dma-api/error_count > 162621 > > (?°?°??? ???) *puts table back on its feet* So, yeah - TTM memory is not allocated using the DMA API, hence we cannot use the DMA API to sync it. Thanks Russell for pointing it out. The only alternative I see here is to flush the CPU caches when syncing for the device, and invalidate them for the other direction. Of course if the device has caches on its side as well the opposite operation must also be done for it. Guess the only way is to handle it all by ourselves here. :/
On Tue, Jun 24, 2014 at 7:55 PM, Alexandre Courbot <acourbot@nvidia.com> wrote: > On 06/24/2014 07:33 PM, Alexandre Courbot wrote: >> >> On 06/24/2014 07:02 PM, Russell King - ARM Linux wrote: >>> >>> On Tue, Jun 24, 2014 at 06:54:26PM +0900, Alexandre Courbot wrote: >>>> >>>> From: Lucas Stach <dev@lynxeye.de> >>>> >>>> On architectures for which access to GPU memory is non-coherent, >>>> caches need to be flushed and invalidated explicitly at the >>>> appropriate places. Introduce two small helpers to make things >>>> easy for TTM-based drivers. >>> >>> >>> Have you run this with DMA API debugging enabled? I suspect you haven't, >>> and I recommend that you do. >> >> >> # cat /sys/kernel/debug/dma-api/error_count >> 162621 >> >> (?°?°??? ???) > > > *puts table back on its feet* > > So, yeah - TTM memory is not allocated using the DMA API, hence we cannot > use the DMA API to sync it. Thanks Russell for pointing it out. > > The only alternative I see here is to flush the CPU caches when syncing for > the device, and invalidate them for the other direction. Of course if the > device has caches on its side as well the opposite operation must also be > done for it. Guess the only way is to handle it all by ourselves here. :/ ... and it really sucks. Basically if we cannot use the DMA API here we will lose the convenience of having a portable API that does just the right thing for the underlying platform. Without it we would have to duplicate arm_iommu_sync_single_for_cpu/device() and we would only have support for ARM. The usage of the DMA API that we are doing might be illegal, but in essence it does exactly what we need - at least for ARM. What are the alternatives?
op 24-06-14 14:23, Alexandre Courbot schreef: > On Tue, Jun 24, 2014 at 7:55 PM, Alexandre Courbot <acourbot@nvidia.com> wrote: >> On 06/24/2014 07:33 PM, Alexandre Courbot wrote: >>> On 06/24/2014 07:02 PM, Russell King - ARM Linux wrote: >>>> On Tue, Jun 24, 2014 at 06:54:26PM +0900, Alexandre Courbot wrote: >>>>> From: Lucas Stach <dev@lynxeye.de> >>>>> >>>>> On architectures for which access to GPU memory is non-coherent, >>>>> caches need to be flushed and invalidated explicitly at the >>>>> appropriate places. Introduce two small helpers to make things >>>>> easy for TTM-based drivers. >>>> >>>> Have you run this with DMA API debugging enabled? I suspect you haven't, >>>> and I recommend that you do. >>> >>> # cat /sys/kernel/debug/dma-api/error_count >>> 162621 >>> >>> (?°?°??? ???) >> >> *puts table back on its feet* >> >> So, yeah - TTM memory is not allocated using the DMA API, hence we cannot >> use the DMA API to sync it. Thanks Russell for pointing it out. >> >> The only alternative I see here is to flush the CPU caches when syncing for >> the device, and invalidate them for the other direction. Of course if the >> device has caches on its side as well the opposite operation must also be >> done for it. Guess the only way is to handle it all by ourselves here. :/ > ... and it really sucks. Basically if we cannot use the DMA API here > we will lose the convenience of having a portable API that does just > the right thing for the underlying platform. Without it we would have > to duplicate arm_iommu_sync_single_for_cpu/device() and we would only > have support for ARM. > > The usage of the DMA API that we are doing might be illegal, but in > essence it does exactly what we need - at least for ARM. What are the > alternatives? Convert TTM to use the dma api? :-) ~Maarten
On Tue, Jun 24, 2014 at 09:23:05PM +0900, Alexandre Courbot wrote: > On Tue, Jun 24, 2014 at 7:55 PM, Alexandre Courbot <acourbot@nvidia.com> wrote: > > The only alternative I see here is to flush the CPU caches when syncing for > > the device, and invalidate them for the other direction. Of course if the > > device has caches on its side as well the opposite operation must also be > > done for it. Guess the only way is to handle it all by ourselves here. :/ > > ... and it really sucks. Basically if we cannot use the DMA API here > we will lose the convenience of having a portable API that does just > the right thing for the underlying platform. Without it we would have > to duplicate arm_iommu_sync_single_for_cpu/device() and we would only > have support for ARM. > > The usage of the DMA API that we are doing might be illegal, but in > essence it does exactly what we need - at least for ARM. What are the > alternatives? It may seem /to you/ as a driver developer to be the easiest thing in the world to abuse an API in a way that it's not supposed to be used, and it is easy to do that. However, what you're actually saying is that you don't respect your fellow kernel developers who have to maintain the other side of that interface. When they need to change the implementation of that interface, what if those changes then screw your abuse of that interface. The reason we define the behaviours and properties of APIs is to give both the user and the implementer of the API some degree of latitude in how that interface works, so that it can be maintained into the future. If abuses (such as these) are allowed, then we've lost, because the interface can no longer be sanely maintained - especially if driver authors eventually end up not caring about their pile of abuse they've created after they've moved on to new wonderful hardware. With an API such as the DMA API, where we have hundreds, if not a thousand users of it, this /really/ matters. We've been here before with the DMA API on older ARM platforms, where we've had people abusing the API or going beneath the API because "it does what they need it to", which then makes stuff much harder to change at architecture level. Last time it happened, it was when ARMv6 came along and ARM moved away from VIVT caches. The options were either to break the crap drivers and support ARMv6+ CPUs, or keep the crap drivers working and not support DMA in any shape or form on ARMv6+. Obviously, this was too important to for one or two abusive drivers to block, so I changed the architecture level /anyway/ and just said screw the drivers which end up being broken by their short-sightedness, they can either rot or someone else can fix them. I have no beef for intentionally breaking stuff when people abuse well defined interfaces and/or refuse to discuss their requirements when interfaces don't quite do what they want - or worse, refuse to listen to objections. As I say, it's disrespectful to your fellow kernel developers to abuse well defined interfaces.
On Tue, Jun 24, 2014 at 10:09 PM, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Tue, Jun 24, 2014 at 09:23:05PM +0900, Alexandre Courbot wrote: >> On Tue, Jun 24, 2014 at 7:55 PM, Alexandre Courbot <acourbot@nvidia.com> wrote: >> > The only alternative I see here is to flush the CPU caches when syncing for >> > the device, and invalidate them for the other direction. Of course if the >> > device has caches on its side as well the opposite operation must also be >> > done for it. Guess the only way is to handle it all by ourselves here. :/ >> >> ... and it really sucks. Basically if we cannot use the DMA API here >> we will lose the convenience of having a portable API that does just >> the right thing for the underlying platform. Without it we would have >> to duplicate arm_iommu_sync_single_for_cpu/device() and we would only >> have support for ARM. >> >> The usage of the DMA API that we are doing might be illegal, but in >> essence it does exactly what we need - at least for ARM. What are the >> alternatives? > > It may seem /to you/ as a driver developer to be the easiest thing in > the world to abuse an API in a way that it's not supposed to be used, > and it is easy to do that. > > However, what you're actually saying is that you don't respect your > fellow kernel developers who have to maintain the other side of that > interface. When they need to change the implementation of that > interface, what if those changes then screw your abuse of that > interface. > > The reason we define the behaviours and properties of APIs is to give > both the user and the implementer of the API some degree of latitude > in how that interface works, so that it can be maintained into the > future. If abuses (such as these) are allowed, then we've lost, > because the interface can no longer be sanely maintained - especially > if driver authors eventually end up not caring about their pile of > abuse they've created after they've moved on to new wonderful hardware. > > With an API such as the DMA API, where we have hundreds, if not a > thousand users of it, this /really/ matters. > > We've been here before with the DMA API on older ARM platforms, where > we've had people abusing the API or going beneath the API because "it > does what they need it to", which then makes stuff much harder to change > at architecture level. > > Last time it happened, it was when ARMv6 came along and ARM moved away > from VIVT caches. The options were either to break the crap drivers > and support ARMv6+ CPUs, or keep the crap drivers working and not > support DMA in any shape or form on ARMv6+. > > Obviously, this was too important to for one or two abusive drivers to > block, so I changed the architecture level /anyway/ and just said screw > the drivers which end up being broken by their short-sightedness, they > can either rot or someone else can fix them. > > I have no beef for intentionally breaking stuff when people abuse well > defined interfaces and/or refuse to discuss their requirements when > interfaces don't quite do what they want - or worse, refuse to listen > to objections. > > As I say, it's disrespectful to your fellow kernel developers to abuse > well defined interfaces. Apologies if I sounded that way - I wasn't suggesting that we carry on with this clearly illegal usage of the DMA API, but was merely noting that if we were to implement the intended behavior on our side it would look just like what the ARM implementation currently does. My question about alternatives wasn't rethorical, I am not so familiar with this part of the system (as should be obvious by now) and would like to know whether there aren't other solutions that would spare us the need to re-implement something that already exists.
Am Dienstag, den 24.06.2014, 14:27 +0200 schrieb Maarten Lankhorst: > op 24-06-14 14:23, Alexandre Courbot schreef: > > On Tue, Jun 24, 2014 at 7:55 PM, Alexandre Courbot <acourbot@nvidia.com> wrote: > >> On 06/24/2014 07:33 PM, Alexandre Courbot wrote: > >>> On 06/24/2014 07:02 PM, Russell King - ARM Linux wrote: > >>>> On Tue, Jun 24, 2014 at 06:54:26PM +0900, Alexandre Courbot wrote: > >>>>> From: Lucas Stach <dev@lynxeye.de> > >>>>> > >>>>> On architectures for which access to GPU memory is non-coherent, > >>>>> caches need to be flushed and invalidated explicitly at the > >>>>> appropriate places. Introduce two small helpers to make things > >>>>> easy for TTM-based drivers. > >>>> > >>>> Have you run this with DMA API debugging enabled? I suspect you haven't, > >>>> and I recommend that you do. > >>> > >>> # cat /sys/kernel/debug/dma-api/error_count > >>> 162621 > >>> > >>> (?°?°??? ???) > >> > >> *puts table back on its feet* > >> > >> So, yeah - TTM memory is not allocated using the DMA API, hence we cannot > >> use the DMA API to sync it. Thanks Russell for pointing it out. > >> > >> The only alternative I see here is to flush the CPU caches when syncing for > >> the device, and invalidate them for the other direction. Of course if the > >> device has caches on its side as well the opposite operation must also be > >> done for it. Guess the only way is to handle it all by ourselves here. :/ > > ... and it really sucks. Basically if we cannot use the DMA API here > > we will lose the convenience of having a portable API that does just > > the right thing for the underlying platform. Without it we would have > > to duplicate arm_iommu_sync_single_for_cpu/device() and we would only > > have support for ARM. > > > > The usage of the DMA API that we are doing might be illegal, but in > > essence it does exactly what we need - at least for ARM. What are the > > alternatives? > Convert TTM to use the dma api? :-) Actually TTM already has a page alloc backend using the DMA API. It's just not used for the standard case right now. I would argue that we should just use this page allocator (which has the side effect of getting pages from CMA if available -> you are actually free to change the caching) and do away with the other allocator in the ARM case. Regards, Lucas
On Tue, Jun 24, 2014 at 10:25 PM, Lucas Stach <l.stach@pengutronix.de> wrote: > Am Dienstag, den 24.06.2014, 14:27 +0200 schrieb Maarten Lankhorst: >> op 24-06-14 14:23, Alexandre Courbot schreef: >> > On Tue, Jun 24, 2014 at 7:55 PM, Alexandre Courbot <acourbot@nvidia.com> wrote: >> >> On 06/24/2014 07:33 PM, Alexandre Courbot wrote: >> >>> On 06/24/2014 07:02 PM, Russell King - ARM Linux wrote: >> >>>> On Tue, Jun 24, 2014 at 06:54:26PM +0900, Alexandre Courbot wrote: >> >>>>> From: Lucas Stach <dev@lynxeye.de> >> >>>>> >> >>>>> On architectures for which access to GPU memory is non-coherent, >> >>>>> caches need to be flushed and invalidated explicitly at the >> >>>>> appropriate places. Introduce two small helpers to make things >> >>>>> easy for TTM-based drivers. >> >>>> >> >>>> Have you run this with DMA API debugging enabled? I suspect you haven't, >> >>>> and I recommend that you do. >> >>> >> >>> # cat /sys/kernel/debug/dma-api/error_count >> >>> 162621 >> >>> >> >>> (?°?°??? ???) >> >> >> >> *puts table back on its feet* >> >> >> >> So, yeah - TTM memory is not allocated using the DMA API, hence we cannot >> >> use the DMA API to sync it. Thanks Russell for pointing it out. >> >> >> >> The only alternative I see here is to flush the CPU caches when syncing for >> >> the device, and invalidate them for the other direction. Of course if the >> >> device has caches on its side as well the opposite operation must also be >> >> done for it. Guess the only way is to handle it all by ourselves here. :/ >> > ... and it really sucks. Basically if we cannot use the DMA API here >> > we will lose the convenience of having a portable API that does just >> > the right thing for the underlying platform. Without it we would have >> > to duplicate arm_iommu_sync_single_for_cpu/device() and we would only >> > have support for ARM. >> > >> > The usage of the DMA API that we are doing might be illegal, but in >> > essence it does exactly what we need - at least for ARM. What are the >> > alternatives? >> Convert TTM to use the dma api? :-) > > Actually TTM already has a page alloc backend using the DMA API. It's > just not used for the standard case right now. Indeed, and Nouveau even already makes use of it if CONFIG_SWIOTLB is set apparently. > I would argue that we should just use this page allocator (which has the > side effect of getting pages from CMA if available -> you are actually > free to change the caching) and do away with the other allocator in the > ARM case. Mm? Does it mean that CMA memory is not mapped into lowmem? That would certainly help in the present case, but I wonder how useful it will be once the iommu support is in place. Will also need to consider performance of such coherent memory for e.g. user-space mappings. Anyway, I will experiment a bit with this tomorrow, thanks!
Am Dienstag, den 24.06.2014, 22:52 +0900 schrieb Alexandre Courbot: > On Tue, Jun 24, 2014 at 10:25 PM, Lucas Stach <l.stach@pengutronix.de> wrote: > > Am Dienstag, den 24.06.2014, 14:27 +0200 schrieb Maarten Lankhorst: > >> op 24-06-14 14:23, Alexandre Courbot schreef: > >> > On Tue, Jun 24, 2014 at 7:55 PM, Alexandre Courbot <acourbot@nvidia.com> wrote: > >> >> On 06/24/2014 07:33 PM, Alexandre Courbot wrote: > >> >>> On 06/24/2014 07:02 PM, Russell King - ARM Linux wrote: > >> >>>> On Tue, Jun 24, 2014 at 06:54:26PM +0900, Alexandre Courbot wrote: > >> >>>>> From: Lucas Stach <dev@lynxeye.de> > >> >>>>> > >> >>>>> On architectures for which access to GPU memory is non-coherent, > >> >>>>> caches need to be flushed and invalidated explicitly at the > >> >>>>> appropriate places. Introduce two small helpers to make things > >> >>>>> easy for TTM-based drivers. > >> >>>> > >> >>>> Have you run this with DMA API debugging enabled? I suspect you haven't, > >> >>>> and I recommend that you do. > >> >>> > >> >>> # cat /sys/kernel/debug/dma-api/error_count > >> >>> 162621 > >> >>> > >> >>> (?°?°??? ???) > >> >> > >> >> *puts table back on its feet* > >> >> > >> >> So, yeah - TTM memory is not allocated using the DMA API, hence we cannot > >> >> use the DMA API to sync it. Thanks Russell for pointing it out. > >> >> > >> >> The only alternative I see here is to flush the CPU caches when syncing for > >> >> the device, and invalidate them for the other direction. Of course if the > >> >> device has caches on its side as well the opposite operation must also be > >> >> done for it. Guess the only way is to handle it all by ourselves here. :/ > >> > ... and it really sucks. Basically if we cannot use the DMA API here > >> > we will lose the convenience of having a portable API that does just > >> > the right thing for the underlying platform. Without it we would have > >> > to duplicate arm_iommu_sync_single_for_cpu/device() and we would only > >> > have support for ARM. > >> > > >> > The usage of the DMA API that we are doing might be illegal, but in > >> > essence it does exactly what we need - at least for ARM. What are the > >> > alternatives? > >> Convert TTM to use the dma api? :-) > > > > Actually TTM already has a page alloc backend using the DMA API. It's > > just not used for the standard case right now. > > Indeed, and Nouveau even already makes use of it if CONFIG_SWIOTLB is > set apparently. > > > I would argue that we should just use this page allocator (which has the > > side effect of getting pages from CMA if available -> you are actually > > free to change the caching) and do away with the other allocator in the > > ARM case. > > Mm? Does it mean that CMA memory is not mapped into lowmem? That would > certainly help in the present case, but I wonder how useful it will be > once the iommu support is in place. Will also need to consider > performance of such coherent memory for e.g. user-space mappings. > > Anyway, I will experiment a bit with this tomorrow, thanks! CMA memory is reserved before the lowmem section mapping is set up. It is then mapped with individual 4k pages before giving it back to the buddy allocator. This means CMA pages in use by the kernel are mapped into lowmem, but they are actually unmapped from lowmem once you allocate them as DMA memory. Regards, Lucas
On Tue, Jun 24, 2014 at 10:58 PM, Lucas Stach <l.stach@pengutronix.de> wrote: > Am Dienstag, den 24.06.2014, 22:52 +0900 schrieb Alexandre Courbot: >> On Tue, Jun 24, 2014 at 10:25 PM, Lucas Stach <l.stach@pengutronix.de> wrote: >> > Am Dienstag, den 24.06.2014, 14:27 +0200 schrieb Maarten Lankhorst: >> >> op 24-06-14 14:23, Alexandre Courbot schreef: >> >> > On Tue, Jun 24, 2014 at 7:55 PM, Alexandre Courbot <acourbot@nvidia.com> wrote: >> >> >> On 06/24/2014 07:33 PM, Alexandre Courbot wrote: >> >> >>> On 06/24/2014 07:02 PM, Russell King - ARM Linux wrote: >> >> >>>> On Tue, Jun 24, 2014 at 06:54:26PM +0900, Alexandre Courbot wrote: >> >> >>>>> From: Lucas Stach <dev@lynxeye.de> >> >> >>>>> >> >> >>>>> On architectures for which access to GPU memory is non-coherent, >> >> >>>>> caches need to be flushed and invalidated explicitly at the >> >> >>>>> appropriate places. Introduce two small helpers to make things >> >> >>>>> easy for TTM-based drivers. >> >> >>>> >> >> >>>> Have you run this with DMA API debugging enabled? I suspect you haven't, >> >> >>>> and I recommend that you do. >> >> >>> >> >> >>> # cat /sys/kernel/debug/dma-api/error_count >> >> >>> 162621 >> >> >>> >> >> >>> (?°?°??? ???) >> >> >> >> >> >> *puts table back on its feet* >> >> >> >> >> >> So, yeah - TTM memory is not allocated using the DMA API, hence we cannot >> >> >> use the DMA API to sync it. Thanks Russell for pointing it out. >> >> >> >> >> >> The only alternative I see here is to flush the CPU caches when syncing for >> >> >> the device, and invalidate them for the other direction. Of course if the >> >> >> device has caches on its side as well the opposite operation must also be >> >> >> done for it. Guess the only way is to handle it all by ourselves here. :/ >> >> > ... and it really sucks. Basically if we cannot use the DMA API here >> >> > we will lose the convenience of having a portable API that does just >> >> > the right thing for the underlying platform. Without it we would have >> >> > to duplicate arm_iommu_sync_single_for_cpu/device() and we would only >> >> > have support for ARM. >> >> > >> >> > The usage of the DMA API that we are doing might be illegal, but in >> >> > essence it does exactly what we need - at least for ARM. What are the >> >> > alternatives? >> >> Convert TTM to use the dma api? :-) >> > >> > Actually TTM already has a page alloc backend using the DMA API. It's >> > just not used for the standard case right now. >> >> Indeed, and Nouveau even already makes use of it if CONFIG_SWIOTLB is >> set apparently. >> >> > I would argue that we should just use this page allocator (which has the >> > side effect of getting pages from CMA if available -> you are actually >> > free to change the caching) and do away with the other allocator in the >> > ARM case. >> >> Mm? Does it mean that CMA memory is not mapped into lowmem? That would >> certainly help in the present case, but I wonder how useful it will be >> once the iommu support is in place. Will also need to consider >> performance of such coherent memory for e.g. user-space mappings. >> >> Anyway, I will experiment a bit with this tomorrow, thanks! > > CMA memory is reserved before the lowmem section mapping is set up. It > is then mapped with individual 4k pages before giving it back to the > buddy allocator. > This means CMA pages in use by the kernel are mapped into lowmem, but > they are actually unmapped from lowmem once you allocate them as DMA > memory. Thanks for the explanation. I really need to spend more time studying the DMA allocator. I wonder if all this is already explained somewhere in Documentation/ ?
On Tue, Jun 24, 2014 at 6:25 AM, Lucas Stach <l.stach@pengutronix.de> wrote: > Am Dienstag, den 24.06.2014, 14:27 +0200 schrieb Maarten Lankhorst: >> op 24-06-14 14:23, Alexandre Courbot schreef: >> > On Tue, Jun 24, 2014 at 7:55 PM, Alexandre Courbot <acourbot@nvidia.com> wrote: >> >> On 06/24/2014 07:33 PM, Alexandre Courbot wrote: >> >>> On 06/24/2014 07:02 PM, Russell King - ARM Linux wrote: >> >>>> On Tue, Jun 24, 2014 at 06:54:26PM +0900, Alexandre Courbot wrote: >> >>>>> From: Lucas Stach <dev@lynxeye.de> >> >>>>> >> >>>>> On architectures for which access to GPU memory is non-coherent, >> >>>>> caches need to be flushed and invalidated explicitly at the >> >>>>> appropriate places. Introduce two small helpers to make things >> >>>>> easy for TTM-based drivers. >> >>>> >> >>>> Have you run this with DMA API debugging enabled? I suspect you haven't, >> >>>> and I recommend that you do. >> >>> >> >>> # cat /sys/kernel/debug/dma-api/error_count >> >>> 162621 >> >>> >> >>> (?°?°??? ???) >> >> >> >> *puts table back on its feet* >> >> >> >> So, yeah - TTM memory is not allocated using the DMA API, hence we cannot >> >> use the DMA API to sync it. Thanks Russell for pointing it out. >> >> >> >> The only alternative I see here is to flush the CPU caches when syncing for >> >> the device, and invalidate them for the other direction. Of course if the >> >> device has caches on its side as well the opposite operation must also be >> >> done for it. Guess the only way is to handle it all by ourselves here. :/ >> > ... and it really sucks. Basically if we cannot use the DMA API here >> > we will lose the convenience of having a portable API that does just >> > the right thing for the underlying platform. Without it we would have >> > to duplicate arm_iommu_sync_single_for_cpu/device() and we would only >> > have support for ARM. >> > >> > The usage of the DMA API that we are doing might be illegal, but in >> > essence it does exactly what we need - at least for ARM. What are the >> > alternatives? >> Convert TTM to use the dma api? :-) > > Actually TTM already has a page alloc backend using the DMA API. It's > just not used for the standard case right now. > > I would argue that we should just use this page allocator (which has the > side effect of getting pages from CMA if available -> you are actually > free to change the caching) and do away with the other allocator in the > ARM case. CMA comes with its own set of (severe) limitations though, in particular it's not possible to map arbitrary CPU pages into the GPU without incurring a copy, you add arbitrary memory limits etc. Overall that's not really a good pick for the long term... Stéphane
On Tue, Jun 24, 2014 at 10:58 PM, Lucas Stach <l.stach@pengutronix.de> wrote: > Am Dienstag, den 24.06.2014, 22:52 +0900 schrieb Alexandre Courbot: >> On Tue, Jun 24, 2014 at 10:25 PM, Lucas Stach <l.stach@pengutronix.de> wrote: >> > Am Dienstag, den 24.06.2014, 14:27 +0200 schrieb Maarten Lankhorst: >> >> op 24-06-14 14:23, Alexandre Courbot schreef: >> >> > On Tue, Jun 24, 2014 at 7:55 PM, Alexandre Courbot <acourbot@nvidia.com> wrote: >> >> >> On 06/24/2014 07:33 PM, Alexandre Courbot wrote: >> >> >>> On 06/24/2014 07:02 PM, Russell King - ARM Linux wrote: >> >> >>>> On Tue, Jun 24, 2014 at 06:54:26PM +0900, Alexandre Courbot wrote: >> >> >>>>> From: Lucas Stach <dev@lynxeye.de> >> >> >>>>> >> >> >>>>> On architectures for which access to GPU memory is non-coherent, >> >> >>>>> caches need to be flushed and invalidated explicitly at the >> >> >>>>> appropriate places. Introduce two small helpers to make things >> >> >>>>> easy for TTM-based drivers. >> >> >>>> >> >> >>>> Have you run this with DMA API debugging enabled? I suspect you haven't, >> >> >>>> and I recommend that you do. >> >> >>> >> >> >>> # cat /sys/kernel/debug/dma-api/error_count >> >> >>> 162621 >> >> >>> >> >> >>> (?°?°??? ???) >> >> >> >> >> >> *puts table back on its feet* >> >> >> >> >> >> So, yeah - TTM memory is not allocated using the DMA API, hence we cannot >> >> >> use the DMA API to sync it. Thanks Russell for pointing it out. >> >> >> >> >> >> The only alternative I see here is to flush the CPU caches when syncing for >> >> >> the device, and invalidate them for the other direction. Of course if the >> >> >> device has caches on its side as well the opposite operation must also be >> >> >> done for it. Guess the only way is to handle it all by ourselves here. :/ >> >> > ... and it really sucks. Basically if we cannot use the DMA API here >> >> > we will lose the convenience of having a portable API that does just >> >> > the right thing for the underlying platform. Without it we would have >> >> > to duplicate arm_iommu_sync_single_for_cpu/device() and we would only >> >> > have support for ARM. >> >> > >> >> > The usage of the DMA API that we are doing might be illegal, but in >> >> > essence it does exactly what we need - at least for ARM. What are the >> >> > alternatives? >> >> Convert TTM to use the dma api? :-) >> > >> > Actually TTM already has a page alloc backend using the DMA API. It's >> > just not used for the standard case right now. >> >> Indeed, and Nouveau even already makes use of it if CONFIG_SWIOTLB is >> set apparently. >> >> > I would argue that we should just use this page allocator (which has the >> > side effect of getting pages from CMA if available -> you are actually >> > free to change the caching) and do away with the other allocator in the >> > ARM case. >> >> Mm? Does it mean that CMA memory is not mapped into lowmem? That would >> certainly help in the present case, but I wonder how useful it will be >> once the iommu support is in place. Will also need to consider >> performance of such coherent memory for e.g. user-space mappings. >> >> Anyway, I will experiment a bit with this tomorrow, thanks! > > CMA memory is reserved before the lowmem section mapping is set up. It > is then mapped with individual 4k pages before giving it back to the > buddy allocator. > This means CMA pages in use by the kernel are mapped into lowmem, but > they are actually unmapped from lowmem once you allocate them as DMA > memory. Tried enabling the DMA page allocation for GK20A. The great news is that with it caching works as expected and that DMA API debugging does not complain anymore when calling the sync functions. Actually, since the DMA page allocator returns coherent memory, there is no need for these sync functions anymore, which makes things easier. This seems to be the simplest way towards enabling GK20A - albeit performance suffers a little, but we can revisit that later once we have IOMMU support. I would not claim that we are fully compliant with what the DMA API expects though. For instance, pages allocated for TTM via dma_alloc_coherent() are later re-mapped in ttm_bo_kmap_ttm() using vmap(), potentially with a different pgprot. The waste of address space produced by these two simultaneous mappings aside, is this even allowed? And when it comes to mapping these pages to user-space, TTM does it without calling dma_mmap_coherent(), and again with whatever flags we give it. IIUC memory allocated through the DMA API should only be touched through the vaddr returned by dma_alloc_coherent(), or through mappings provided by other DMA API functions. So is TTM misusing the DMA API here? In general, should we still be afraid of non-identical mappings on modern CPUs like the A15 found in Tegra K1? I have heard contradictory information so far and would really like to be able to understand this once and for all, as it would give us more choices as for which memory provider we can use. Thanks, Alex.
On Wed, Jun 25, 2014 at 1:00 PM, Stéphane Marchesin <stephane.marchesin@gmail.com> wrote: > On Tue, Jun 24, 2014 at 6:25 AM, Lucas Stach <l.stach@pengutronix.de> wrote: >> Am Dienstag, den 24.06.2014, 14:27 +0200 schrieb Maarten Lankhorst: >>> op 24-06-14 14:23, Alexandre Courbot schreef: >>> > On Tue, Jun 24, 2014 at 7:55 PM, Alexandre Courbot <acourbot@nvidia.com> wrote: >>> >> On 06/24/2014 07:33 PM, Alexandre Courbot wrote: >>> >>> On 06/24/2014 07:02 PM, Russell King - ARM Linux wrote: >>> >>>> On Tue, Jun 24, 2014 at 06:54:26PM +0900, Alexandre Courbot wrote: >>> >>>>> From: Lucas Stach <dev@lynxeye.de> >>> >>>>> >>> >>>>> On architectures for which access to GPU memory is non-coherent, >>> >>>>> caches need to be flushed and invalidated explicitly at the >>> >>>>> appropriate places. Introduce two small helpers to make things >>> >>>>> easy for TTM-based drivers. >>> >>>> >>> >>>> Have you run this with DMA API debugging enabled? I suspect you haven't, >>> >>>> and I recommend that you do. >>> >>> >>> >>> # cat /sys/kernel/debug/dma-api/error_count >>> >>> 162621 >>> >>> >>> >>> (?°?°??? ???) >>> >> >>> >> *puts table back on its feet* >>> >> >>> >> So, yeah - TTM memory is not allocated using the DMA API, hence we cannot >>> >> use the DMA API to sync it. Thanks Russell for pointing it out. >>> >> >>> >> The only alternative I see here is to flush the CPU caches when syncing for >>> >> the device, and invalidate them for the other direction. Of course if the >>> >> device has caches on its side as well the opposite operation must also be >>> >> done for it. Guess the only way is to handle it all by ourselves here. :/ >>> > ... and it really sucks. Basically if we cannot use the DMA API here >>> > we will lose the convenience of having a portable API that does just >>> > the right thing for the underlying platform. Without it we would have >>> > to duplicate arm_iommu_sync_single_for_cpu/device() and we would only >>> > have support for ARM. >>> > >>> > The usage of the DMA API that we are doing might be illegal, but in >>> > essence it does exactly what we need - at least for ARM. What are the >>> > alternatives? >>> Convert TTM to use the dma api? :-) >> >> Actually TTM already has a page alloc backend using the DMA API. It's >> just not used for the standard case right now. >> >> I would argue that we should just use this page allocator (which has the >> side effect of getting pages from CMA if available -> you are actually >> free to change the caching) and do away with the other allocator in the >> ARM case. > > CMA comes with its own set of (severe) limitations though, in > particular it's not possible to map arbitrary CPU pages into the GPU > without incurring a copy, you add arbitrary memory limits etc. Overall > that's not really a good pick for the long term... We don't plan to rely on CMA for too long. IOMMU support is on the way and should make our life easier, although no matter the source of memory, we will still have the issue of the lowmem mappings. So far it sounds like CMA is the only way to "undo" them, so in the end it may come down to whether or not the multi-mapping contraint applies to TK1. I will tap into our internal sources of knowledge to try and figure this one out.
On Thu, Jun 26, 2014 at 11:53:20PM +0900, Alexandre Courbot wrote: > We don't plan to rely on CMA for too long. IOMMU support is on the way > and should make our life easier, although no matter the source of > memory, we will still have the issue of the lowmem mappings. When it comes to DMA memory, talking about lowmem vs highmem is utterly meaningless. The lowmem/highmem split is entirely a software concept and is completely adjustable. An extreme example is that you can boot any platform with more than 32MB of memory with 32MB of lowmem and the remainder as highmem.
On Fri, Jun 27, 2014 at 1:10 AM, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Thu, Jun 26, 2014 at 11:53:20PM +0900, Alexandre Courbot wrote: >> We don't plan to rely on CMA for too long. IOMMU support is on the way >> and should make our life easier, although no matter the source of >> memory, we will still have the issue of the lowmem mappings. > > When it comes to DMA memory, talking about lowmem vs highmem is utterly > meaningless. > > The lowmem/highmem split is entirely a software concept and is completely > adjustable. An extreme example is that you can boot any platform with > more than 32MB of memory with 32MB of lowmem and the remainder as > highmem. True, but isn't it also the case that all lowmem is already mapped in the kernel address space, and that re-mapping this memory with different cache settings (e.g. by creating a WC mapping for user-space to write into) is undefined on ARM and must be avoided? That is the issue I was referring to.
On Thu, Jun 26, 2014 at 7:17 PM, Alexandre Courbot <gnurou@gmail.com> wrote: > On Fri, Jun 27, 2014 at 1:10 AM, Russell King - ARM Linux > <linux@arm.linux.org.uk> wrote: >> On Thu, Jun 26, 2014 at 11:53:20PM +0900, Alexandre Courbot wrote: >>> We don't plan to rely on CMA for too long. IOMMU support is on the way >>> and should make our life easier, although no matter the source of >>> memory, we will still have the issue of the lowmem mappings. >> >> When it comes to DMA memory, talking about lowmem vs highmem is utterly >> meaningless. >> >> The lowmem/highmem split is entirely a software concept and is completely >> adjustable. An extreme example is that you can boot any platform with >> more than 32MB of memory with 32MB of lowmem and the remainder as >> highmem. > > True, but isn't it also the case that all lowmem is already mapped in > the kernel address space, and that re-mapping this memory with > different cache settings (e.g. by creating a WC mapping for user-space > to write into) is undefined on ARM and must be avoided? That is the > issue I was referring to. > dma memory should be removed from the kernel linear map (if needed).. assuming it is allocated w/ dma api's. btw, something I've been wondering for a little while, but haven't had time to investigate. Not sure if this applies to you as well. But seems like I have IOMMU's which can be outer-coherent (snoop L2), but I *think* they are not inner-coherent (L1). No idea if current dma memory code can grok this and only do inner-cache op's.. BR, -R > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c index 75f319090043..66c16ad35f70 100644 --- a/drivers/gpu/drm/ttm/ttm_tt.c +++ b/drivers/gpu/drm/ttm/ttm_tt.c @@ -38,6 +38,7 @@ #include <linux/swap.h> #include <linux/slab.h> #include <linux/export.h> +#include <linux/dma-mapping.h> #include <drm/drm_cache.h> #include <drm/drm_mem_util.h> #include <drm/ttm/ttm_module.h> @@ -248,6 +249,30 @@ void ttm_dma_tt_fini(struct ttm_dma_tt *ttm_dma) } EXPORT_SYMBOL(ttm_dma_tt_fini); +void ttm_dma_tt_cache_sync_for_device(struct ttm_dma_tt *ttm_dma, + struct device *dev) +{ + unsigned long i; + + for (i = 0; i < ttm_dma->ttm.num_pages; i++) { + dma_sync_single_for_device(dev, ttm_dma->dma_address[i], + PAGE_SIZE, DMA_TO_DEVICE); + } +} +EXPORT_SYMBOL(ttm_dma_tt_cache_sync_for_device); + +void ttm_dma_tt_cache_sync_for_cpu(struct ttm_dma_tt *ttm_dma, + struct device *dev) +{ + unsigned long i; + + for (i = 0; i < ttm_dma->ttm.num_pages; i++) { + dma_sync_single_for_cpu(dev, ttm_dma->dma_address[i], + PAGE_SIZE, DMA_FROM_DEVICE); + } +} +EXPORT_SYMBOL(ttm_dma_tt_cache_sync_for_cpu); + void ttm_tt_unbind(struct ttm_tt *ttm) { int ret; diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h index a5183da3ef92..52fb709568fc 100644 --- a/include/drm/ttm/ttm_bo_driver.h +++ b/include/drm/ttm/ttm_bo_driver.h @@ -41,6 +41,7 @@ #include <linux/fs.h> #include <linux/spinlock.h> #include <linux/reservation.h> +#include <linux/device.h> struct ttm_backend_func { /** @@ -690,6 +691,33 @@ extern int ttm_tt_swapout(struct ttm_tt *ttm, */ extern void ttm_tt_unpopulate(struct ttm_tt *ttm); +/** + * ttm_dma_tt_cache_sync_for_device: + * + * @ttm A struct ttm_tt of the type returned by ttm_dma_tt_init. + * @dev A struct device representing the device to which to sync. + * + * This function will flush the CPU caches on arches where snooping in the + * TT is not available. On fully coherent arches this will turn into an (almost) + * noop. This makes sure that data written by the CPU is visible to the device. + */ +extern void ttm_dma_tt_cache_sync_for_device(struct ttm_dma_tt *ttm_dma, + struct device *dev); + +/** + * ttm_dma_tt_cache_sync_for_cpu: + * + * @ttm A struct ttm_tt of the type returned by ttm_dma_tt_init. + * @dev A struct device representing the device from which to sync. + * + * This function will invalidate the CPU caches on arches where snooping in the + * TT is not available. On fully coherent arches this will turn into an (almost) + * noop. This makes sure that the CPU does not read any stale cached or + * prefetched data. + */ +extern void ttm_dma_tt_cache_sync_for_cpu(struct ttm_dma_tt *ttm_dma, + struct device *dev); + /* * ttm_bo.c */