All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: Lyude Paul <lyude@redhat.com>
Cc: David Airlie <airlied@linux.ie>,
	nouveau@lists.freedesktop.org,
	open list <linux-kernel@vger.kernel.org>,
	"open list:DRM DRIVER FOR NVIDIA GEFORCE/QUADRO GPUS"
	<dri-devel@lists.freedesktop.org>,
	Ben Skeggs <bskeggs@redhat.com>, Daniel Vetter <daniel@ffwll.ch>,
	Christoph Hellwig <hch@lst.de>
Subject: Re: [Nouveau] [RFC] drm/nouveau/ttm: Stop calling into swiotlb
Date: Fri, 29 Jul 2022 15:12:48 +0200	[thread overview]
Message-ID: <20220729131248.GA27137@lst.de> (raw)
In-Reply-To: <20220728220545.163763-1-lyude@redhat.com>

Hi Lyude, and thanks for taking a look.

> -#if IS_ENABLED(CONFIG_SWIOTLB) && IS_ENABLED(CONFIG_X86)
> -	need_swiotlb = is_swiotlb_active(dev->dev);
> -#endif
> -
>  	ret = ttm_device_init(&drm->ttm.bdev, &nouveau_bo_driver, drm->dev->dev,
> -				  dev->anon_inode->i_mapping,
> -				  dev->vma_offset_manager, need_swiotlb,
> -				  drm->client.mmu.dmabits <= 32);
> +			      dev->anon_inode->i_mapping,
> +			      dev->vma_offset_manager,
> +			      nouveau_drm_use_coherent_gpu_mapping(drm),
> +			      drm->client.mmu.dmabits <= 32);

This will break setups for two reasons:

 - swiotlb is not only used to do device addressing limitations, so
   this will not catch the case of interconnect addressing limitations
   or forced bounce buffering which used used e.g. in secure VMs.
 - we might need bouncing for any DMA address below the physical
   address limit of the CPU

But more fundamentally the use_dma32 argument to ttm_device_init
is rather broken, as the onlyway to get a memory allocation that
fits the DMA addressing needs of a device is to use the proper
DMA mapping helpers. i.e. ttm_pool_alloc_page really needs to use
dma_alloc_pages instead of alloc_pages as a first step.  That way
all users of the TTM pool will always get dma addressable pages
and there is no need to guess the addressing limitations.

The use_dma_alloc is then only needed for users that require coherent
memory and are willing to deal with the limitations that this entails
(e.g. no way to get at the page struct).

>  	if (ret) {
>  		NV_ERROR(drm, "error initialising bo driver, %d\n", ret);
>  		return ret;
> -- 
> 2.35.3
---end quoted text---

WARNING: multiple messages have this Message-ID (diff)
From: Christoph Hellwig <hch@lst.de>
To: Lyude Paul <lyude@redhat.com>
Cc: nouveau@lists.freedesktop.org, Christoph Hellwig <hch@lst.de>,
	Ben Skeggs <bskeggs@redhat.com>,
	Karol Herbst <kherbst@redhat.com>,
	David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>,
	"open list:DRM DRIVER FOR NVIDIA GEFORCE/QUADRO GPUS" 
	<dri-devel@lists.freedesktop.org>,
	open list <linux-kernel@vger.kernel.org>
Subject: Re: [RFC] drm/nouveau/ttm: Stop calling into swiotlb
Date: Fri, 29 Jul 2022 15:12:48 +0200	[thread overview]
Message-ID: <20220729131248.GA27137@lst.de> (raw)
In-Reply-To: <20220728220545.163763-1-lyude@redhat.com>

Hi Lyude, and thanks for taking a look.

> -#if IS_ENABLED(CONFIG_SWIOTLB) && IS_ENABLED(CONFIG_X86)
> -	need_swiotlb = is_swiotlb_active(dev->dev);
> -#endif
> -
>  	ret = ttm_device_init(&drm->ttm.bdev, &nouveau_bo_driver, drm->dev->dev,
> -				  dev->anon_inode->i_mapping,
> -				  dev->vma_offset_manager, need_swiotlb,
> -				  drm->client.mmu.dmabits <= 32);
> +			      dev->anon_inode->i_mapping,
> +			      dev->vma_offset_manager,
> +			      nouveau_drm_use_coherent_gpu_mapping(drm),
> +			      drm->client.mmu.dmabits <= 32);

This will break setups for two reasons:

 - swiotlb is not only used to do device addressing limitations, so
   this will not catch the case of interconnect addressing limitations
   or forced bounce buffering which used used e.g. in secure VMs.
 - we might need bouncing for any DMA address below the physical
   address limit of the CPU

But more fundamentally the use_dma32 argument to ttm_device_init
is rather broken, as the onlyway to get a memory allocation that
fits the DMA addressing needs of a device is to use the proper
DMA mapping helpers. i.e. ttm_pool_alloc_page really needs to use
dma_alloc_pages instead of alloc_pages as a first step.  That way
all users of the TTM pool will always get dma addressable pages
and there is no need to guess the addressing limitations.

The use_dma_alloc is then only needed for users that require coherent
memory and are willing to deal with the limitations that this entails
(e.g. no way to get at the page struct).

>  	if (ret) {
>  		NV_ERROR(drm, "error initialising bo driver, %d\n", ret);
>  		return ret;
> -- 
> 2.35.3
---end quoted text---

  reply	other threads:[~2022-07-29 13:12 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-28 22:05 [RFC] drm/nouveau/ttm: Stop calling into swiotlb Lyude Paul
2022-07-28 22:05 ` Lyude Paul
2022-07-28 22:05 ` [Nouveau] " Lyude Paul
2022-07-29 13:12 ` Christoph Hellwig [this message]
2022-07-29 13:12   ` Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220729131248.GA27137@lst.de \
    --to=hch@lst.de \
    --cc=airlied@linux.ie \
    --cc=bskeggs@redhat.com \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lyude@redhat.com \
    --cc=nouveau@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.