Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752843AbaGKJfZ (ORCPT ); Fri, 11 Jul 2014 05:35:25 -0400 Received: from hqemgate15.nvidia.com ([216.228.121.64]:6323 "EHLO hqemgate15.nvidia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752588AbaGKJfV (ORCPT ); Fri, 11 Jul 2014 05:35:21 -0400 X-PGP-Universal: processed; by hqnvupgp08.nvidia.com on Fri, 11 Jul 2014 02:28:14 -0700 Message-ID: <53BFAFD4.5040401@nvidia.com> Date: Fri, 11 Jul 2014 18:35:16 +0900 From: Alexandre Courbot Organization: NVIDIA User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.6.0 MIME-Version: 1.0 To: Daniel Vetter CC: Ben Skeggs , David Airlie , David Herrmann , Lucas Stach , Thierry Reding , Maarten Lankhorst , , , , , Alexandre Courbot Subject: Re: [Nouveau] [PATCH v4 4/6] drm/nouveau: synchronize BOs when required References: <1404807961-30530-1-git-send-email-acourbot@nvidia.com> <1404807961-30530-5-git-send-email-acourbot@nvidia.com> <20140710130449.GG17271@phenom.ffwll.local> <53BF4E9B.7090606@nvidia.com> <20140711074138.GW17271@phenom.ffwll.local> In-Reply-To: <20140711074138.GW17271@phenom.ffwll.local> X-NVConfidentiality: public Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 07/11/2014 04:41 PM, Daniel Vetter wrote: > On Fri, Jul 11, 2014 at 11:40:27AM +0900, Alexandre Courbot wrote: >> On 07/10/2014 10:04 PM, Daniel Vetter wrote: >>> On Tue, Jul 08, 2014 at 05:25:59PM +0900, Alexandre Courbot wrote: >>>> On architectures for which access to GPU memory is non-coherent, >>>> caches need to be flushed and invalidated explicitly when BO control >>>> changes between CPU and GPU. >>>> >>>> This patch adds buffer synchronization functions which invokes the >>>> correct API (PCI or DMA) to ensure synchronization is effective. >>>> >>>> Based on the TTM DMA cache helper patches by Lucas Stach. >>>> >>>> Signed-off-by: Lucas Stach >>>> Signed-off-by: Alexandre Courbot >>>> --- >>>> drivers/gpu/drm/nouveau/nouveau_bo.c | 56 +++++++++++++++++++++++++++++++++++ >>>> drivers/gpu/drm/nouveau/nouveau_bo.h | 2 ++ >>>> drivers/gpu/drm/nouveau/nouveau_gem.c | 12 ++++++++ >>>> 3 files changed, 70 insertions(+) >>>> >>>> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c >>>> index 67e9e8e2e2ec..47e4e8886769 100644 >>>> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c >>>> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c >>>> @@ -402,6 +402,60 @@ nouveau_bo_unmap(struct nouveau_bo *nvbo) >>>> ttm_bo_kunmap(&nvbo->kmap); >>>> } >>>> >>>> +void >>>> +nouveau_bo_sync_for_device(struct nouveau_bo *nvbo) >>>> +{ >>>> + struct nouveau_drm *drm = nouveau_bdev(nvbo->bo.bdev); >>>> + struct nouveau_device *device = nouveau_dev(drm->dev); >>>> + struct ttm_dma_tt *ttm_dma = (struct ttm_dma_tt *)nvbo->bo.ttm; >>>> + int i; >>>> + >>>> + if (!ttm_dma) >>>> + return; >>>> + >>>> + if (nv_device_is_cpu_coherent(device) || nvbo->force_coherent) >>>> + return; >>> >>> Is the is_cpu_coherent check really required? On coherent platforms the >>> sync_for_foo should be a noop. It's the dma api's job to encapsulate this >>> knowledge so that drivers can be blissfully ignorant. The explicit >>> is_coherent check makes this a bit leaky. And same comment that underlying >>> the bus-specifics dma-mapping functions are identical. >> >> I think you are right, the is_cpu_coherent check should not be needed here. >> I still think we should have separate paths for the PCI/DMA cases though, >> unless you can point me to a source that clearly states that the PCI API is >> deprecated and that DMA should be used instead. > > Ah, on 2nd look I've found it again. Quoting > Documentation/DMA-API-HOWTO.txt: > > "Note that the DMA API works with any bus independent of the underlying > microprocessor architecture. You should use the DMA API rather than the > bus-specific DMA API, i.e., use the dma_map_*() interfaces rather than the > pci_map_*() interfaces." > > The advice is fairly strong here I think ;-) And imo the idea makes sense, > since it allows drivers like nouveau here to care much less about the > actual bus used to get data to/from the ip block. And if you look at intel > gfx it makes even more sense since the pci layer we have is really just a > thin fake shim whacked on top of the hw (on SoCs at least). Indeed, I stand corrected. :) That's good news actually, as it will simplify the code. Thanks for pointing that out! I will send a new revision that makes use of the DMA API exclusively and will remove the nv_device_map/unmap() functions which are pretty useless now. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/