[PATCH] drm/prime: remove cargo-cult locking from map_sg helper

2013-07-11 Thread Laurent Pinchart
Hi Daniel, Thanks for the patch. On Wednesday 10 July 2013 16:48:45 Daniel Vetter wrote: > I've checked both implementations (radeon/nouveau) and they both grab > the page array from ttm simply by dereferencing it and then wrapping > it up with drm_prime_pages_to_sg in the callback and map it

Re: [PATCH] drm/prime: remove cargo-cult locking from map_sg helper

2013-07-11 Thread Laurent Pinchart
Hi Daniel, Thanks for the patch. On Wednesday 10 July 2013 16:48:45 Daniel Vetter wrote: I've checked both implementations (radeon/nouveau) and they both grab the page array from ttm simply by dereferencing it and then wrapping it up with drm_prime_pages_to_sg in the callback and map it with

[PATCH] drm/prime: remove cargo-cult locking from map_sg helper

2013-07-10 Thread Maarten Lankhorst
Op 10-07-13 16:48, Daniel Vetter schreef: > I've checked both implementations (radeon/nouveau) and they both grab > the page array from ttm simply by dereferencing it and then wrapping > it up with drm_prime_pages_to_sg in the callback and map it with > dma_map_sg (in the helper). > > Only the

[PATCH] drm/prime: remove cargo-cult locking from map_sg helper

2013-07-10 Thread Daniel Vetter
On Wed, Jul 10, 2013 at 5:18 PM, Konrad Rzeszutek Wilk wrote: >> So after a bit of irc chatting with Maarten this seems to be more >> involved. The above check is to cache the dma mapping, but the >> implementation is bogus in tons of ways: >> - If direction changes we don't bother with unmaping

[PATCH] drm/prime: remove cargo-cult locking from map_sg helper

2013-07-10 Thread Daniel Vetter
I've checked both implementations (radeon/nouveau) and they both grab the page array from ttm simply by dereferencing it and then wrapping it up with drm_prime_pages_to_sg in the callback and map it with dma_map_sg (in the helper). Only the grabbing of the underlying page array is anything we

[PATCH] drm/prime: remove cargo-cult locking from map_sg helper

2013-07-10 Thread Daniel Vetter
On Wed, Jul 10, 2013 at 3:46 PM, Laurent Pinchart wrote: > On Wednesday 10 July 2013 13:54:33 Daniel Vetter wrote: >> I've checked both implementations (radeon/nouveau) and they both grab >> the page array from ttm simply by dereferencing it and then wrapping >> it up with drm_prime_pages_to_sg

[PATCH] drm/prime: remove cargo-cult locking from map_sg helper

2013-07-10 Thread Laurent Pinchart
Hi Daniel, On Wednesday 10 July 2013 13:54:33 Daniel Vetter wrote: > I've checked both implementations (radeon/nouveau) and they both grab > the page array from ttm simply by dereferencing it and then wrapping > it up with drm_prime_pages_to_sg in the callback and map it with > dma_map_sg (in the

[PATCH] drm/prime: remove cargo-cult locking from map_sg helper

2013-07-10 Thread Daniel Vetter
On Wed, Jul 10, 2013 at 2:03 PM, Maarten Lankhorst wrote: > Op 10-07-13 13:54, Daniel Vetter schreef: >> I've checked both implementations (radeon/nouveau) and they both grab >> the page array from ttm simply by dereferencing it and then wrapping >> it up with drm_prime_pages_to_sg in the

[PATCH] drm/prime: remove cargo-cult locking from map_sg helper

2013-07-10 Thread Maarten Lankhorst
Op 10-07-13 13:54, Daniel Vetter schreef: > I've checked both implementations (radeon/nouveau) and they both grab > the page array from ttm simply by dereferencing it and then wrapping > it up with drm_prime_pages_to_sg in the callback and map it with > dma_map_sg (in the helper). > > Only the

[PATCH] drm/prime: remove cargo-cult locking from map_sg helper

2013-07-10 Thread Daniel Vetter
I've checked both implementations (radeon/nouveau) and they both grab the page array from ttm simply by dereferencing it and then wrapping it up with drm_prime_pages_to_sg in the callback and map it with dma_map_sg (in the helper). Only the grabbing of the underlying page array is anything we

[PATCH] drm/prime: remove cargo-cult locking from map_sg helper

2013-07-10 Thread Konrad Rzeszutek Wilk
> So after a bit of irc chatting with Maarten this seems to be more > involved. The above check is to cache the dma mapping, but the > implementation is bogus in tons of ways: > - If direction changes we don't bother with unmaping and freeing the > mapping, but simply leak it. > - This will break

[PATCH] drm/prime: remove cargo-cult locking from map_sg helper

2013-07-10 Thread Daniel Vetter
I've checked both implementations (radeon/nouveau) and they both grab the page array from ttm simply by dereferencing it and then wrapping it up with drm_prime_pages_to_sg in the callback and map it with dma_map_sg (in the helper). Only the grabbing of the underlying page array is anything we

Re: [PATCH] drm/prime: remove cargo-cult locking from map_sg helper

2013-07-10 Thread Maarten Lankhorst
Op 10-07-13 13:54, Daniel Vetter schreef: I've checked both implementations (radeon/nouveau) and they both grab the page array from ttm simply by dereferencing it and then wrapping it up with drm_prime_pages_to_sg in the callback and map it with dma_map_sg (in the helper). Only the grabbing

Re: [PATCH] drm/prime: remove cargo-cult locking from map_sg helper

2013-07-10 Thread Daniel Vetter
On Wed, Jul 10, 2013 at 2:03 PM, Maarten Lankhorst maarten.lankho...@canonical.com wrote: Op 10-07-13 13:54, Daniel Vetter schreef: I've checked both implementations (radeon/nouveau) and they both grab the page array from ttm simply by dereferencing it and then wrapping it up with

Re: [PATCH] drm/prime: remove cargo-cult locking from map_sg helper

2013-07-10 Thread Laurent Pinchart
Hi Daniel, On Wednesday 10 July 2013 13:54:33 Daniel Vetter wrote: I've checked both implementations (radeon/nouveau) and they both grab the page array from ttm simply by dereferencing it and then wrapping it up with drm_prime_pages_to_sg in the callback and map it with dma_map_sg (in the

Re: [PATCH] drm/prime: remove cargo-cult locking from map_sg helper

2013-07-10 Thread Daniel Vetter
On Wed, Jul 10, 2013 at 3:46 PM, Laurent Pinchart laurent.pinch...@ideasonboard.com wrote: On Wednesday 10 July 2013 13:54:33 Daniel Vetter wrote: I've checked both implementations (radeon/nouveau) and they both grab the page array from ttm simply by dereferencing it and then wrapping it up

[PATCH] drm/prime: remove cargo-cult locking from map_sg helper

2013-07-10 Thread Daniel Vetter
I've checked both implementations (radeon/nouveau) and they both grab the page array from ttm simply by dereferencing it and then wrapping it up with drm_prime_pages_to_sg in the callback and map it with dma_map_sg (in the helper). Only the grabbing of the underlying page array is anything we

Re: [PATCH] drm/prime: remove cargo-cult locking from map_sg helper

2013-07-10 Thread Konrad Rzeszutek Wilk
So after a bit of irc chatting with Maarten this seems to be more involved. The above check is to cache the dma mapping, but the implementation is bogus in tons of ways: - If direction changes we don't bother with unmaping and freeing the mapping, but simply leak it. - This will break if the

Re: [PATCH] drm/prime: remove cargo-cult locking from map_sg helper

2013-07-10 Thread Daniel Vetter
On Wed, Jul 10, 2013 at 5:18 PM, Konrad Rzeszutek Wilk konrad.w...@oracle.com wrote: So after a bit of irc chatting with Maarten this seems to be more involved. The above check is to cache the dma mapping, but the implementation is bogus in tons of ways: - If direction changes we don't bother

Re: [PATCH] drm/prime: remove cargo-cult locking from map_sg helper

2013-07-10 Thread Maarten Lankhorst
Op 10-07-13 16:48, Daniel Vetter schreef: I've checked both implementations (radeon/nouveau) and they both grab the page array from ttm simply by dereferencing it and then wrapping it up with drm_prime_pages_to_sg in the callback and map it with dma_map_sg (in the helper). Only the grabbing