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

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

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