[Nouveau] [PATCH] drm/connector: Allow max possible encoders to attach to a connector
From: Dhinakaran Pandiyan
Currently we restrict the number of encoders that can be linked to
a connector to 3, increase it to match the maximum number of encoders
that can be initialized(32).
To more effiently do that lets switch from an array of encoder ids to
bitmask.
Also removing the best_encoder hook from the drivers that only have
one encoder per connector(this ones have one encoder in the whole
driver), pick_single_encoder_for_connector() will do the same job
with no functional change.
Suggested-by: Ville Syrjälä
Cc: Ville Syrjälä
Cc: Alex Deucher
Cc: dri-de...@lists.freedesktop.org
Cc: intel-...@lists.freedesktop.org
Cc: nouveau@lists.freedesktop.org
Cc: amd-...@lists.freedesktop.org
Signed-off-by: Dhinakaran Pandiyan
Signed-off-by: José Roberto de Souza
---
.../gpu/drm/amd/amdgpu/amdgpu_connectors.c| 23 +-
drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 5 ++-
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 6 +++-
drivers/gpu/drm/ast/ast_mode.c| 12 ---
drivers/gpu/drm/drm_atomic_helper.c | 9 --
drivers/gpu/drm/drm_client_modeset.c | 3 +-
drivers/gpu/drm/drm_connector.c | 31 +--
drivers/gpu/drm/drm_probe_helper.c| 3 +-
drivers/gpu/drm/mgag200/mgag200_mode.c| 11 ---
drivers/gpu/drm/nouveau/dispnv04/disp.c | 2 +-
drivers/gpu/drm/nouveau/dispnv50/disp.c | 2 +-
drivers/gpu/drm/nouveau/nouveau_connector.c | 7 ++---
drivers/gpu/drm/radeon/radeon_connectors.c| 27 ++--
drivers/gpu/drm/udl/udl_connector.c | 8 -
include/drm/drm_connector.h | 18 +--
15 files changed, 53 insertions(+), 114 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
index ece55c8fa673..d8729285f731 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
@@ -217,11 +217,10 @@ amdgpu_connector_update_scratch_regs(struct drm_connector
*connector,
struct drm_encoder *encoder;
const struct drm_connector_helper_funcs *connector_funcs =
connector->helper_private;
bool connected;
- int i;
best_encoder = connector_funcs->best_encoder(connector);
- drm_connector_for_each_possible_encoder(connector, encoder, i) {
+ drm_connector_for_each_possible_encoder(connector, encoder) {
if ((encoder == best_encoder) && (status ==
connector_status_connected))
connected = true;
else
@@ -236,9 +235,8 @@ amdgpu_connector_find_encoder(struct drm_connector
*connector,
int encoder_type)
{
struct drm_encoder *encoder;
- int i;
- drm_connector_for_each_possible_encoder(connector, encoder, i) {
+ drm_connector_for_each_possible_encoder(connector, encoder) {
if (encoder->encoder_type == encoder_type)
return encoder;
}
@@ -347,10 +345,9 @@ static struct drm_encoder *
amdgpu_connector_best_single_encoder(struct drm_connector *connector)
{
struct drm_encoder *encoder;
- int i;
/* pick the first one */
- drm_connector_for_each_possible_encoder(connector, encoder, i)
+ drm_connector_for_each_possible_encoder(connector, encoder)
return encoder;
return NULL;
@@ -1065,9 +1062,8 @@ amdgpu_connector_dvi_detect(struct drm_connector
*connector, bool force)
/* find analog encoder */
if (amdgpu_connector->dac_load_detect) {
struct drm_encoder *encoder;
- int i;
- drm_connector_for_each_possible_encoder(connector, encoder, i) {
+ drm_connector_for_each_possible_encoder(connector, encoder) {
if (encoder->encoder_type != DRM_MODE_ENCODER_DAC &&
encoder->encoder_type != DRM_MODE_ENCODER_TVDAC)
continue;
@@ -1117,9 +1113,8 @@ amdgpu_connector_dvi_encoder(struct drm_connector
*connector)
{
struct amdgpu_connector *amdgpu_connector =
to_amdgpu_connector(connector);
struct drm_encoder *encoder;
- int i;
- drm_connector_for_each_possible_encoder(connector, encoder, i) {
+ drm_connector_for_each_possible_encoder(connector, encoder) {
if (amdgpu_connector->use_digital == true) {
if (encoder->encoder_type == DRM_MODE_ENCODER_TMDS)
return encoder;
@@ -1134,7 +1129,7 @@ amdgpu_connector_dvi_encoder(struct drm_connector
*connector)
/* then check use digitial */
/* pick the first one */
- drm_connector_for_each_possible_encoder(connector, encoder, i)
+ drm_connector_for_each_possible_encoder(connector, encoder)
return encoder;
return NULL;
@@ -1271,9
Re: [Nouveau] [Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/connector: Allow max possible encoders to attach to a connector (rev2)
On Fri, 2019-08-16 at 21:29 +, Patchwork wrote: > == Series Details == > > Series: drm/connector: Allow max possible encoders to attach to a > connector (rev2) > URL : https://patchwork.freedesktop.org/series/62743/ > State : warning > > == Summary == > > $ dim sparse origin/drm-tip > Sparse version: v0.6.0 > Commit: drm/connector: Allow max possible encoders to attach to a > connector > + ^ > + } > +drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:4802:1: > warning: control reaches end of non-void function [-Wreturn-type] > +drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c: In > function ‘amdgpu_dm_connector_to_encoder’: Missed a "return NULL;" that will not be reached. Will fix that in the next version after get some comments. > > ___ > Intel-gfx mailing list > intel-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [Intel-gfx] [PATCH v6 08/17] drm/ttm: use gem vma_node
On Mon, Aug 05, 2019 at 04:01:10PM +0200, Gerd Hoffmann wrote:
> Drop vma_node from ttm_buffer_object, use the gem struct
> (base.vma_node) instead.
>
> Signed-off-by: Gerd Hoffmann
> Reviewed-by: Christian König
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 2 +-
> drivers/gpu/drm/qxl/qxl_object.h | 2 +-
> drivers/gpu/drm/radeon/radeon_object.h | 2 +-
> drivers/gpu/drm/virtio/virtgpu_drv.h | 2 +-
> include/drm/ttm/ttm_bo_api.h | 4
> drivers/gpu/drm/drm_gem_vram_helper.c | 2 +-
> drivers/gpu/drm/nouveau/nouveau_display.c | 2 +-
> drivers/gpu/drm/nouveau/nouveau_gem.c | 2 +-
> drivers/gpu/drm/ttm/ttm_bo.c | 8
> drivers/gpu/drm/ttm/ttm_bo_util.c | 2 +-
> drivers/gpu/drm/ttm/ttm_bo_vm.c| 9 +
> drivers/gpu/drm/virtio/virtgpu_prime.c | 3 ---
> drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 4 ++--
> drivers/gpu/drm/vmwgfx/vmwgfx_surface.c| 4 ++--
> 14 files changed, 21 insertions(+), 27 deletions(-)
Hi Gerd,
I've been seeing a regression on Nouveau with recent linux-next releases
and git bisect points at this commit as the first bad one. If I revert
it (there's a tiny conflict with a patch that was merged subsequently),
things are back to normal.
I think the reason for this issue is that Nouveau doesn't use GEM
objects for all buffer objects, and even when it uses GEM objects, the
code will not initialize the GEM object until after the buffer objects
and the backing TTM objects have been created.
I tried to fix that by making sure drm_gem_object_init() gets called by
Nouveau before ttm_bo_init(), but the changes are fairly involved and I
was unable to get the GEM reference counting right. I can look into the
proper fix some more, but it might be worth reverting this patch for
now to get Nouveau working again.
Thierry
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index 645a189d365c..113fb2feb437 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -191,7 +191,7 @@ static inline unsigned
> amdgpu_bo_gpu_page_alignment(struct amdgpu_bo *bo)
> */
> static inline u64 amdgpu_bo_mmap_offset(struct amdgpu_bo *bo)
> {
> - return drm_vma_node_offset_addr(>tbo.vma_node);
> + return drm_vma_node_offset_addr(>tbo.base.vma_node);
> }
>
> /**
> diff --git a/drivers/gpu/drm/qxl/qxl_object.h
> b/drivers/gpu/drm/qxl/qxl_object.h
> index b812d4ae9d0d..8ae54ba7857c 100644
> --- a/drivers/gpu/drm/qxl/qxl_object.h
> +++ b/drivers/gpu/drm/qxl/qxl_object.h
> @@ -60,7 +60,7 @@ static inline unsigned long qxl_bo_size(struct qxl_bo *bo)
>
> static inline u64 qxl_bo_mmap_offset(struct qxl_bo *bo)
> {
> - return drm_vma_node_offset_addr(>tbo.vma_node);
> + return drm_vma_node_offset_addr(>tbo.base.vma_node);
> }
>
> static inline int qxl_bo_wait(struct qxl_bo *bo, u32 *mem_type,
> diff --git a/drivers/gpu/drm/radeon/radeon_object.h
> b/drivers/gpu/drm/radeon/radeon_object.h
> index 9ffd8215d38a..e5554bf9140e 100644
> --- a/drivers/gpu/drm/radeon/radeon_object.h
> +++ b/drivers/gpu/drm/radeon/radeon_object.h
> @@ -116,7 +116,7 @@ static inline unsigned
> radeon_bo_gpu_page_alignment(struct radeon_bo *bo)
> */
> static inline u64 radeon_bo_mmap_offset(struct radeon_bo *bo)
> {
> - return drm_vma_node_offset_addr(>tbo.vma_node);
> + return drm_vma_node_offset_addr(>tbo.base.vma_node);
> }
>
> extern int radeon_bo_wait(struct radeon_bo *bo, u32 *mem_type,
> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h
> b/drivers/gpu/drm/virtio/virtgpu_drv.h
> index f4ecea6054ba..e28829661724 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_drv.h
> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
> @@ -396,7 +396,7 @@ static inline void virtio_gpu_object_unref(struct
> virtio_gpu_object **bo)
>
> static inline u64 virtio_gpu_object_mmap_offset(struct virtio_gpu_object *bo)
> {
> - return drm_vma_node_offset_addr(>tbo.vma_node);
> + return drm_vma_node_offset_addr(>tbo.base.vma_node);
> }
>
> static inline int virtio_gpu_object_reserve(struct virtio_gpu_object *bo,
> diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
> index fa050f0328ab..7ffc50a3303d 100644
> --- a/include/drm/ttm/ttm_bo_api.h
> +++ b/include/drm/ttm/ttm_bo_api.h
> @@ -152,7 +152,6 @@ struct ttm_tt;
> * @ddestroy: List head for the delayed destroy list.
> * @swap: List head for swap LRU list.
> * @moving: Fence set when BO is moving
> - * @vma_node: Address space manager node.
> * @offset: The current GPU offset, which can have different meanings
> * depending on the memory type. For SYSTEM type memory, it should be 0.
> * @cur_placement: Hint of current placement.
> @@ -219,9 +218,6 @@ struct ttm_buffer_object {
>*/
>
> struct dma_fence *moving;
> -
> - struct drm_vma_offset_node vma_node;
> -
> unsigned
Re: [Nouveau] [PATCH] nouveau/hmm: map pages after migration
On Fri, Aug 16, 2019 at 5:11 PM Christoph Hellwig wrote: > > On Mon, Aug 12, 2019 at 12:42:30PM -0700, Ralph Campbell wrote: > > > > On 8/10/19 4:13 AM, Christoph Hellwig wrote: > >> On something vaguely related to this patch: > >> > >> You use the NVIF_VMM_PFNMAP_V0_V* defines from nvif/if000c.h, which are > >> a little odd as we only ever set these bits, but they also don't seem > >> to appear to be in values that are directly fed to the hardware. > >> > >> On the other hand mmu/vmm.h defines a set of NVIF_VMM_PFNMAP_V0_* > > > > Yes, I see NVKM_VMM_PFN_* > > > >> constants with similar names and identical values, and those are used > >> in mmu/vmmgp100.c and what appears to finally do the low-level dma > >> mapping and talking to the hardware. Are these two sets of constants > >> supposed to be the same? Are the actual hardware values or just a > >> driver internal interface? > > > > It looks a bit odd to me too. > > I don't really know the structure/history of nouveau. > > Perhaps Ben Skeggs can shed more light on your question. > > Ben, do you have any insights on these constants? Those sets of constants are (currently) supposed to be the same value. They don't necessarily map to the HW directly at this stage, and something different will likely be done in the future as HW changes. Ben. ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [PATCH 04/15] mm: remove the pgmap field from struct hmm_vma_walk
On 8/16/19 10:28 AM, Jason Gunthorpe wrote: On Fri, Aug 16, 2019 at 10:21:41AM -0700, Dan Williams wrote: We can do a get_dev_pagemap inside the page_walk and touch the pgmap, or we can do the 'device mutex && retry' pattern and touch the pgmap in the driver, under that lock. However in all cases the current get_dev_pagemap()'s in the page walk are not necessary, and we can delete them. Yes, as long as 'struct page' instances resulting from that lookup are not passed outside of that lock. Indeed. Also, I was reflecting over lunch that the hmm_range_fault should only return DEVICE_PRIVATE pages for the caller's device (see other thread with HCH), and in this case, the caller should also be responsible to ensure that the driver is not calling hmm_range_fault at the same time it is deleting it's own DEVICE_PRIVATE mapping - ie by fencing its page fault handler. Yes, that would make it a one step process to access another device's migrated memory pages. Right now, it has to be a two step process where the caller calls hmm_range_fault, check the struct page to see if it is device private and not owned, then call hmm_range_fault again with range->pfns[i] |= range->flags[HMM_PFN_DEVICE_PRIVATE] to cause the other device to migrate the page back to system memory.
Re: [Nouveau] DMA-API: cacheline tracking ENOMEM, dma-debug disabled due to nouveau ?
On Fri, Aug 16, 2019 at 4:31 PM Corentin Labbe wrote: > On Wed, Aug 14, 2019 at 07:49:27PM +0200, Daniel Vetter wrote: > > On Wed, Aug 14, 2019 at 04:50:33PM +0200, Corentin Labbe wrote: > > > Hello > > > > > > Since lot of release (at least since 4.19), I hit the following error > > > message: > > > DMA-API: cacheline tracking ENOMEM, dma-debug disabled > > > > > > After hitting that, I try to check who is creating so many DMA mapping > > > and see: > > > cat /sys/kernel/debug/dma-api/dump | cut -d' ' -f2 | sort | uniq -c > > > 6 ahci > > > 257 e1000e > > > 6 ehci-pci > > >5891 nouveau > > > 24 uhci_hcd > > > > > > Does nouveau having this high number of DMA mapping is normal ? > > > > Yeah seems perfectly fine for a gpu. > > Note that it never go down and when I terminate my X session, it stays the > same. > So without any "real" GPU work, does it is still normal to have so many > active mapping ? Might just be the dma_alloc cache. It should go down under memory pressure I think. Otherwise might also be a leak. > For example, when doing some transfer, the ahci mapping number changes and > then always go down to 6. gpu drivers tend to cache everything, all the time ... -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [PATCH 04/15] mm: remove the pgmap field from struct hmm_vma_walk
On Fri, Aug 16, 2019 at 10:21:41AM -0700, Dan Williams wrote: > > We can do a get_dev_pagemap inside the page_walk and touch the pgmap, > > or we can do the 'device mutex && retry' pattern and touch the pgmap > > in the driver, under that lock. > > > > However in all cases the current get_dev_pagemap()'s in the page walk > > are not necessary, and we can delete them. > > Yes, as long as 'struct page' instances resulting from that lookup are > not passed outside of that lock. Indeed. Also, I was reflecting over lunch that the hmm_range_fault should only return DEVICE_PRIVATE pages for the caller's device (see other thread with HCH), and in this case, the caller should also be responsible to ensure that the driver is not calling hmm_range_fault at the same time it is deleting it's own DEVICE_PRIVATE mapping - ie by fencing its page fault handler. This does not apply to PCI_P2PDMA, but, lets see how that looks when we get there. So the whole thing seems pretty safe. Jason
Re: turn hmm migrate_vma upside down v3
On Fri, Aug 16, 2019 at 08:51:41AM +0200, Christoph Hellwig wrote: > Jason, > > are you going to look into picking this up? Unfortunately there is > a hole pile in this area still pending, including the kvmppc secure > memory driver from Bharata that depends on the work. Done, Lets see if Dan will comment on the pagemap part (looks straightforward to me), and then after we grab that I will declare hmm.git non-rebasing and Bharata can build his series upon it. As a reminder, please do not send hmm.git inside another pull request to Linus without making it very clear in that cover letter and Cc'ing me. Thanks. Regards, Jason
Re: [PATCH 04/15] mm: remove the pgmap field from struct hmm_vma_walk
On Fri, Aug 16, 2019 at 5:24 AM Jason Gunthorpe wrote: > > On Thu, Aug 15, 2019 at 08:54:46PM -0700, Dan Williams wrote: > > > > However, this means we cannot do any processing of ZONE_DEVICE pages > > > outside the driver lock, so eg, doing any DMA map that might rely on > > > MEMORY_DEVICE_PCI_P2PDMA has to be done in the driver lock, which is > > > a bit unfortunate. > > > > Wouldn't P2PDMA use page pins? Not needing to hold a lock over > > ZONE_DEVICE page operations was one of the motivations for plumbing > > get_dev_pagemap() with a percpu-ref. > > hmm_range_fault() doesn't use page pins at all, so if a ZONE_DEVICE > page comes out of it then it needs to use another locking pattern. > > If I follow it all right: > > We can do a get_dev_pagemap inside the page_walk and touch the pgmap, > or we can do the 'device mutex && retry' pattern and touch the pgmap > in the driver, under that lock. > > However in all cases the current get_dev_pagemap()'s in the page walk > are not necessary, and we can delete them. Yes, as long as 'struct page' instances resulting from that lookup are not passed outside of that lock.
Re: [Nouveau] turn hmm migrate_vma upside down v3
On Wed, Aug 14, 2019 at 09:59:18AM +0200, Christoph Hellwig wrote: > Hi Jérôme, Ben and Jason, > > below is a series against the hmm tree which starts revamping the > migrate_vma functionality. The prime idea is to export three slightly > lower level functions and thus avoid the need for migrate_vma_ops > callbacks. > > Diffstat: > > 7 files changed, 282 insertions(+), 614 deletions(-) Yay, another big wack of code gone Applied to hmm.git Thanks, Jason ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [PATCH 01/10] mm: turn migrate_vma upside down
On Wed, Aug 14, 2019 at 09:59:19AM +0200, Christoph Hellwig wrote: > There isn't any good reason to pass callbacks to migrate_vma. Instead > we can just export the three steps done by this function to drivers and > let them sequence the operation without callbacks. This removes a lot > of boilerplate code as-is, and will allow the drivers to drastically > improve code flow and error handling further on. > > Signed-off-by: Christoph Hellwig > Reviewed-by: Ralph Campbell > --- > Documentation/vm/hmm.rst | 55 +- > drivers/gpu/drm/nouveau/nouveau_dmem.c | 122 +++-- > include/linux/migrate.h| 118 ++-- > mm/migrate.c | 244 +++-- > 4 files changed, 195 insertions(+), 344 deletions(-) The mechanical transformation looks OK, I squashed the below nits in, let me know if I got it wrong Reviewed-by: Jason Gunthorpe diff --git a/Documentation/vm/hmm.rst b/Documentation/vm/hmm.rst index 4f81c77059e305..0a5960beccf76d 100644 --- a/Documentation/vm/hmm.rst +++ b/Documentation/vm/hmm.rst @@ -339,9 +339,8 @@ Migration to and from device memory === Because the CPU cannot access device memory, migration must use the device DMA -engine to perform copy from and to device memory. For this we need a new to -use migrate_vma_setup(), migrate_vma_pages(), and migrate_vma_finalize() -helpers. +engine to perform copy from and to device memory. For this we need to use +migrate_vma_setup(), migrate_vma_pages(), and migrate_vma_finalize() helpers. Memory cgroup (memcg) and rss accounting diff --git a/mm/migrate.c b/mm/migrate.c index 993386cb53358d..0e78ebd720c0e4 100644 --- a/mm/migrate.c +++ b/mm/migrate.c @@ -2622,10 +2622,9 @@ static void migrate_vma_unmap(struct migrate_vma *migrate) * successfully migrated, and which were not. Successfully migrated pages will * have the MIGRATE_PFN_MIGRATE flag set for their src array entry. * - * It is safe to update device page table from within the finalize_and_map() - * callback because both destination and source page are still locked, and the - * mmap_sem is held in read mode (hence no one can unmap the range being - * migrated). + * It is safe to update device page table after migrate_vma_pages() because + * both destination and source page are still locked, and the mmap_sem is held + * in read mode (hence no one can unmap the range being migrated). * * Once the caller is done cleaning up things and updating its page table (if it * chose to do so, this is not an obligation) it finally calls @@ -2657,10 +2656,11 @@ int migrate_vma_setup(struct migrate_vma *args) args->npages = 0; migrate_vma_collect(args); - if (args->cpages) - migrate_vma_prepare(args); - if (args->cpages) - migrate_vma_unmap(args); + if (!args->cpages) + return 0; + + migrate_vma_prepare(args); + migrate_vma_unmap(args); /* * At this point pages are locked and unmapped, and thus they have
Re: [Nouveau] [PATCH 09/10] mm: remove the unused MIGRATE_PFN_DEVICE flag
On Wed, Aug 14, 2019 at 09:59:27AM +0200, Christoph Hellwig wrote: > No one ever checks this flag, and we could easily get that information > from the page if needed. > > Signed-off-by: Christoph Hellwig > Reviewed-by: Ralph Campbell > --- > drivers/gpu/drm/nouveau/nouveau_dmem.c | 3 +-- > include/linux/migrate.h| 1 - > mm/migrate.c | 4 ++-- > 3 files changed, 3 insertions(+), 5 deletions(-) Reviewed-by: Jason Gunthorpe Jason ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH 10/10] mm: remove CONFIG_MIGRATE_VMA_HELPER
On Wed, Aug 14, 2019 at 09:59:28AM +0200, Christoph Hellwig wrote: > CONFIG_MIGRATE_VMA_HELPER guards helpers that are required for proper > devic private memory support. Remove the option and just check for > CONFIG_DEVICE_PRIVATE instead. > > Signed-off-by: Christoph Hellwig > --- > drivers/gpu/drm/nouveau/Kconfig | 1 - > mm/Kconfig | 3 --- > mm/migrate.c| 4 ++-- > 3 files changed, 2 insertions(+), 6 deletions(-) Reviewed-by: Jason Gunthorpe Aside from it making sense, I checked no other driver enables DEVICE_PRIVATE, so no change in kernel build. Jason ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: DMA-API: cacheline tracking ENOMEM, dma-debug disabled due to nouveau ?
On Wed, Aug 14, 2019 at 07:49:27PM +0200, Daniel Vetter wrote: > On Wed, Aug 14, 2019 at 04:50:33PM +0200, Corentin Labbe wrote: > > Hello > > > > Since lot of release (at least since 4.19), I hit the following error > > message: > > DMA-API: cacheline tracking ENOMEM, dma-debug disabled > > > > After hitting that, I try to check who is creating so many DMA mapping and > > see: > > cat /sys/kernel/debug/dma-api/dump | cut -d' ' -f2 | sort | uniq -c > > 6 ahci > > 257 e1000e > > 6 ehci-pci > >5891 nouveau > > 24 uhci_hcd > > > > Does nouveau having this high number of DMA mapping is normal ? > > Yeah seems perfectly fine for a gpu. Note that it never go down and when I terminate my X session, it stays the same. So without any "real" GPU work, does it is still normal to have so many active mapping ? For example, when doing some transfer, the ahci mapping number changes and then always go down to 6.
Re: [Nouveau] [PATCH 04/15] mm: remove the pgmap field from struct hmm_vma_walk
On Fri, Aug 16, 2019 at 12:30:41PM +, Jason Gunthorpe wrote: > > For instance, a system may have multiple DEVICE_PRIVATE map's owned by > the same driver - but multiple physical devices using that driver. > > Each physical device's driver should only ever get DEVICE_PRIVATE > pages for it's own on-device memory. Never a DEVICE_PRIVATE for > another device's memory. > > The dev_pagemap_ops would not be unique enough, right? True. > > Probably also clusters of same-driver struct device can share a > DEVICE_PRIVATE, at least high end GPU's now have private memory > coherency busses between their devices. > > Since we want to trigger migration to CPU on incompatible > DEVICE_PRIVATE pages, it seems best to sort this out in the > hmm_range_fault? > > Maybe some sort of unique ID inside the page->pgmap and passed as > input? Yes, we'll probably need an owner field. And it's not just hmm_range_fault, the migrate_vma_* routines as affected in the same way. ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [PATCH 04/15] mm: remove the pgmap field from struct hmm_vma_walk
On Fri, Aug 16, 2019 at 06:44:48AM +0200, Christoph Hellwig wrote: > On Fri, Aug 16, 2019 at 12:43:07AM +, Jason Gunthorpe wrote: > > On Thu, Aug 15, 2019 at 04:51:33PM -0400, Jerome Glisse wrote: > > > > > struct page. In this case any way we can update the > > > nouveau_dmem_page() to check that page page->pgmap == the > > > expected pgmap. > > > > I was also wondering if that is a problem.. just blindly doing a > > container_of on the page->pgmap does seem like it assumes that only > > this driver is using DEVICE_PRIVATE. > > > > It seems like something missing in hmm_range_fault, it should be told > > what DEVICE_PRIVATE is acceptable to trigger HMM_PFN_DEVICE_PRIVATE > > and fault all others? > > The whole device private handling in hmm and migrate_vma seems pretty > broken as far as I can tell, and I have some WIP patches. Basically we > should not touch (or possibly eventually call migrate to ram eventually > in the future) device private pages not owned by the caller, where I > try to defined the caller by the dev_pagemap_ops instance. I think it needs to be more elaborate. For instance, a system may have multiple DEVICE_PRIVATE map's owned by the same driver - but multiple physical devices using that driver. Each physical device's driver should only ever get DEVICE_PRIVATE pages for it's own on-device memory. Never a DEVICE_PRIVATE for another device's memory. The dev_pagemap_ops would not be unique enough, right? Probably also clusters of same-driver struct device can share a DEVICE_PRIVATE, at least high end GPU's now have private memory coherency busses between their devices. Since we want to trigger migration to CPU on incompatible DEVICE_PRIVATE pages, it seems best to sort this out in the hmm_range_fault? Maybe some sort of unique ID inside the page->pgmap and passed as input? Jason
Re: [PATCH 04/15] mm: remove the pgmap field from struct hmm_vma_walk
On Thu, Aug 15, 2019 at 08:54:46PM -0700, Dan Williams wrote: > > However, this means we cannot do any processing of ZONE_DEVICE pages > > outside the driver lock, so eg, doing any DMA map that might rely on > > MEMORY_DEVICE_PCI_P2PDMA has to be done in the driver lock, which is > > a bit unfortunate. > > Wouldn't P2PDMA use page pins? Not needing to hold a lock over > ZONE_DEVICE page operations was one of the motivations for plumbing > get_dev_pagemap() with a percpu-ref. hmm_range_fault() doesn't use page pins at all, so if a ZONE_DEVICE page comes out of it then it needs to use another locking pattern. If I follow it all right: We can do a get_dev_pagemap inside the page_walk and touch the pgmap, or we can do the 'device mutex && retry' pattern and touch the pgmap in the driver, under that lock. However in all cases the current get_dev_pagemap()'s in the page walk are not necessary, and we can delete them. ? Jason
Re: [Nouveau] turn hmm migrate_vma upside down v3
On Fri, Aug 16, 2019 at 08:51:41AM +0200, Christoph Hellwig wrote: > Jason, > > are you going to look into picking this up? Unfortunately there is > a hole pile in this area still pending, including the kvmppc secure > memory driver from Bharata that depends on the work. > > mm folks: migrate.c is mostly a classic MM file except for the hmm > additions. Do you want to also look over this or just let it pass? Yes, after you explained the functions were hmm ones, it seems OK to go to hmm.git. I was waiting for the dust to settle, I see Ralph tested-by, are we good now? Jason ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH] nouveau/hmm: map pages after migration
On Mon, Aug 12, 2019 at 12:42:30PM -0700, Ralph Campbell wrote: > > On 8/10/19 4:13 AM, Christoph Hellwig wrote: >> On something vaguely related to this patch: >> >> You use the NVIF_VMM_PFNMAP_V0_V* defines from nvif/if000c.h, which are >> a little odd as we only ever set these bits, but they also don't seem >> to appear to be in values that are directly fed to the hardware. >> >> On the other hand mmu/vmm.h defines a set of NVIF_VMM_PFNMAP_V0_* > > Yes, I see NVKM_VMM_PFN_* > >> constants with similar names and identical values, and those are used >> in mmu/vmmgp100.c and what appears to finally do the low-level dma >> mapping and talking to the hardware. Are these two sets of constants >> supposed to be the same? Are the actual hardware values or just a >> driver internal interface? > > It looks a bit odd to me too. > I don't really know the structure/history of nouveau. > Perhaps Ben Skeggs can shed more light on your question. Ben, do you have any insights on these constants? ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] turn hmm migrate_vma upside down v3
Jason, are you going to look into picking this up? Unfortunately there is a hole pile in this area still pending, including the kvmppc secure memory driver from Bharata that depends on the work. mm folks: migrate.c is mostly a classic MM file except for the hmm additions. Do you want to also look over this or just let it pass? ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
