drm_pci_alloc/drm_pci_free are very thin wrappers around the core dma facilities, and we have no special reason within the drm layer to behave differently. In particular, since
commit de09d31dd38a50fdce106c15abd68432eebbd014 Author: Kirill A. Shutemov kirill.shutemov@linux.intel.com Date: Fri Jan 15 16:51:42 2016 -0800
page-flags: define PG_reserved behavior on compound pages
As far as I can see there's no users of PG_reserved on compound pages. Let's use PF_NO_COMPOUND here.
it has been illegal to combine GFP_COMP with SetPageReserved, so lets stop doing both and leave the dma layer to its own devices.
Reported-by: Taketo Kabe Closes: https://gitlab.freedesktop.org/drm/intel/issues/1027 Fixes: de09d31dd38a ("page-flags: define PG_reserved behavior on compound pages") Signed-off-by: Chris Wilson chris@chris-wilson.co.uk Cc: stable@vger.kernel.org # v4.5+ --- drivers/gpu/drm/drm_pci.c | 23 ++--------------------- 1 file changed, 2 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index f2e43d341980..d16dac4325f9 100644 --- a/drivers/gpu/drm/drm_pci.c +++ b/drivers/gpu/drm/drm_pci.c @@ -51,8 +51,6 @@ drm_dma_handle_t *drm_pci_alloc(struct drm_device * dev, size_t size, size_t align) { drm_dma_handle_t *dmah; - unsigned long addr; - size_t sz;
/* pci_alloc_consistent only guarantees alignment to the smallest * PAGE_SIZE order which is greater than or equal to the requested size. @@ -68,20 +66,13 @@ drm_dma_handle_t *drm_pci_alloc(struct drm_device * dev, size_t size, size_t ali dmah->size = size; dmah->vaddr = dma_alloc_coherent(&dev->pdev->dev, size, &dmah->busaddr, - GFP_KERNEL | __GFP_COMP); + GFP_KERNEL);
if (dmah->vaddr == NULL) { kfree(dmah); return NULL; }
- /* XXX - Is virt_to_page() legal for consistent mem? */ - /* Reserve */ - for (addr = (unsigned long)dmah->vaddr, sz = size; - sz > 0; addr += PAGE_SIZE, sz -= PAGE_SIZE) { - SetPageReserved(virt_to_page((void *)addr)); - } - return dmah; }
@@ -94,19 +85,9 @@ EXPORT_SYMBOL(drm_pci_alloc); */ void __drm_legacy_pci_free(struct drm_device * dev, drm_dma_handle_t * dmah) { - unsigned long addr; - size_t sz; - - if (dmah->vaddr) { - /* XXX - Is virt_to_page() legal for consistent mem? */ - /* Unreserve */ - for (addr = (unsigned long)dmah->vaddr, sz = dmah->size; - sz > 0; addr += PAGE_SIZE, sz -= PAGE_SIZE) { - ClearPageReserved(virt_to_page((void *)addr)); - } + if (dmah->vaddr) dma_free_coherent(&dev->pdev->dev, dmah->size, dmah->vaddr, dmah->busaddr); - } }
/**
Internally for "consistent" maps, we create a temporary struct drm_dma_handle in order to user our own dma_alloc_coherent wrapper then destroy the temporary wrap. Simplify our logic by removing the temporary wrapper!
Signed-off-by: Chris Wilson chris@chris-wilson.co.uk --- drivers/gpu/drm/drm_bufs.c | 20 +++++++++----------- drivers/gpu/drm/drm_pci.c | 15 ++------------- drivers/gpu/drm/drm_vm.c | 10 ++++------ include/drm/drm_legacy.h | 6 ------ 4 files changed, 15 insertions(+), 36 deletions(-)
diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c index 8ce9d73fab4f..19297e58b232 100644 --- a/drivers/gpu/drm/drm_bufs.c +++ b/drivers/gpu/drm/drm_bufs.c @@ -149,7 +149,6 @@ static int drm_addmap_core(struct drm_device *dev, resource_size_t offset, { struct drm_local_map *map; struct drm_map_list *list; - drm_dma_handle_t *dmah; unsigned long user_token; int ret;
@@ -324,14 +323,14 @@ static int drm_addmap_core(struct drm_device *dev, resource_size_t offset, * As we're limiting the address to 2^32-1 (or less), * casting it down to 32 bits is no problem, but we * need to point to a 64bit variable first. */ - dmah = drm_pci_alloc(dev, map->size, map->size); - if (!dmah) { + map->handle = dma_alloc_coherent(&dev->pdev->dev, + map->size, + &map->offset, + GFP_KERNEL); + if (!map->handle) { kfree(map); return -ENOMEM; } - map->handle = dmah->vaddr; - map->offset = (unsigned long)dmah->busaddr; - kfree(dmah); break; default: kfree(map); @@ -513,7 +512,6 @@ int drm_legacy_getmap_ioctl(struct drm_device *dev, void *data, int drm_legacy_rmmap_locked(struct drm_device *dev, struct drm_local_map *map) { struct drm_map_list *r_list = NULL, *list_t; - drm_dma_handle_t dmah; int found = 0; struct drm_master *master;
@@ -554,10 +552,10 @@ int drm_legacy_rmmap_locked(struct drm_device *dev, struct drm_local_map *map) case _DRM_SCATTER_GATHER: break; case _DRM_CONSISTENT: - dmah.vaddr = map->handle; - dmah.busaddr = map->offset; - dmah.size = map->size; - __drm_legacy_pci_free(dev, &dmah); + dma_free_coherent(&dev->pdev->dev, + map->size, + map->handle, + map->offset); break; } kfree(map); diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index d16dac4325f9..c6bb98729a26 100644 --- a/drivers/gpu/drm/drm_pci.c +++ b/drivers/gpu/drm/drm_pci.c @@ -78,18 +78,6 @@ drm_dma_handle_t *drm_pci_alloc(struct drm_device * dev, size_t size, size_t ali
EXPORT_SYMBOL(drm_pci_alloc);
-/* - * Free a PCI consistent memory block without freeing its descriptor. - * - * This function is for internal use in the Linux-specific DRM core code. - */ -void __drm_legacy_pci_free(struct drm_device * dev, drm_dma_handle_t * dmah) -{ - if (dmah->vaddr) - dma_free_coherent(&dev->pdev->dev, dmah->size, dmah->vaddr, - dmah->busaddr); -} - /** * drm_pci_free - Free a PCI consistent memory block * @dev: DRM device @@ -100,7 +88,8 @@ void __drm_legacy_pci_free(struct drm_device * dev, drm_dma_handle_t * dmah) */ void drm_pci_free(struct drm_device * dev, drm_dma_handle_t * dmah) { - __drm_legacy_pci_free(dev, dmah); + dma_free_coherent(&dev->pdev->dev, dmah->size, dmah->vaddr, + dmah->busaddr); kfree(dmah); }
diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c index 52e87e4869a5..64619fe90046 100644 --- a/drivers/gpu/drm/drm_vm.c +++ b/drivers/gpu/drm/drm_vm.c @@ -269,8 +269,6 @@ static void drm_vm_shm_close(struct vm_area_struct *vma) }
if (!found_maps) { - drm_dma_handle_t dmah; - switch (map->type) { case _DRM_REGISTERS: case _DRM_FRAME_BUFFER: @@ -284,10 +282,10 @@ static void drm_vm_shm_close(struct vm_area_struct *vma) case _DRM_SCATTER_GATHER: break; case _DRM_CONSISTENT: - dmah.vaddr = map->handle; - dmah.busaddr = map->offset; - dmah.size = map->size; - __drm_legacy_pci_free(dev, &dmah); + dma_free_coherent(&dev->pdev->dev, + map->size, + map->handle, + map->offset); break; } kfree(map); diff --git a/include/drm/drm_legacy.h b/include/drm/drm_legacy.h index 5745710453c8..dcef3598f49e 100644 --- a/include/drm/drm_legacy.h +++ b/include/drm/drm_legacy.h @@ -194,17 +194,11 @@ void drm_legacy_idlelock_release(struct drm_lock_data *lock);
#ifdef CONFIG_PCI
-void __drm_legacy_pci_free(struct drm_device *dev, drm_dma_handle_t * dmah); int drm_legacy_pci_init(struct drm_driver *driver, struct pci_driver *pdriver); void drm_legacy_pci_exit(struct drm_driver *driver, struct pci_driver *pdriver);
#else
-static inline void __drm_legacy_pci_free(struct drm_device *dev, - drm_dma_handle_t *dmah) -{ -} - static inline int drm_legacy_pci_init(struct drm_driver *driver, struct pci_driver *pdriver) {
On Sun, Feb 2, 2020 at 12:16 PM Chris Wilson chris@chris-wilson.co.uk wrote:
Internally for "consistent" maps, we create a temporary struct drm_dma_handle in order to user our own dma_alloc_coherent wrapper then destroy the temporary wrap. Simplify our logic by removing the temporary wrapper!
Signed-off-by: Chris Wilson chris@chris-wilson.co.uk
Reviewed-by: Alex Deucher alexander.deucher@amd.com
drivers/gpu/drm/drm_bufs.c | 20 +++++++++----------- drivers/gpu/drm/drm_pci.c | 15 ++------------- drivers/gpu/drm/drm_vm.c | 10 ++++------ include/drm/drm_legacy.h | 6 ------ 4 files changed, 15 insertions(+), 36 deletions(-)
diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c index 8ce9d73fab4f..19297e58b232 100644 --- a/drivers/gpu/drm/drm_bufs.c +++ b/drivers/gpu/drm/drm_bufs.c @@ -149,7 +149,6 @@ static int drm_addmap_core(struct drm_device *dev, resource_size_t offset, { struct drm_local_map *map; struct drm_map_list *list;
drm_dma_handle_t *dmah; unsigned long user_token; int ret;@@ -324,14 +323,14 @@ static int drm_addmap_core(struct drm_device *dev, resource_size_t offset, * As we're limiting the address to 2^32-1 (or less), * casting it down to 32 bits is no problem, but we * need to point to a 64bit variable first. */
dmah = drm_pci_alloc(dev, map->size, map->size);if (!dmah) {
map->handle = dma_alloc_coherent(&dev->pdev->dev,map->size,&map->offset,GFP_KERNEL);if (!map->handle) { kfree(map); return -ENOMEM; }
map->handle = dmah->vaddr;map->offset = (unsigned long)dmah->busaddr;kfree(dmah); break; default: kfree(map);@@ -513,7 +512,6 @@ int drm_legacy_getmap_ioctl(struct drm_device *dev, void *data, int drm_legacy_rmmap_locked(struct drm_device *dev, struct drm_local_map *map) { struct drm_map_list *r_list = NULL, *list_t;
drm_dma_handle_t dmah; int found = 0; struct drm_master *master;@@ -554,10 +552,10 @@ int drm_legacy_rmmap_locked(struct drm_device *dev, struct drm_local_map *map) case _DRM_SCATTER_GATHER: break; case _DRM_CONSISTENT:
dmah.vaddr = map->handle;dmah.busaddr = map->offset;dmah.size = map->size;__drm_legacy_pci_free(dev, &dmah);
dma_free_coherent(&dev->pdev->dev,map->size,map->handle,map->offset); break; } kfree(map);diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index d16dac4325f9..c6bb98729a26 100644 --- a/drivers/gpu/drm/drm_pci.c +++ b/drivers/gpu/drm/drm_pci.c @@ -78,18 +78,6 @@ drm_dma_handle_t *drm_pci_alloc(struct drm_device * dev, size_t size, size_t ali
EXPORT_SYMBOL(drm_pci_alloc);
-/*
- Free a PCI consistent memory block without freeing its descriptor.
- This function is for internal use in the Linux-specific DRM core code.
- */
-void __drm_legacy_pci_free(struct drm_device * dev, drm_dma_handle_t * dmah) -{
if (dmah->vaddr)dma_free_coherent(&dev->pdev->dev, dmah->size, dmah->vaddr,dmah->busaddr);-}
/**
- drm_pci_free - Free a PCI consistent memory block
- @dev: DRM device
@@ -100,7 +88,8 @@ void __drm_legacy_pci_free(struct drm_device * dev, drm_dma_handle_t * dmah) */ void drm_pci_free(struct drm_device * dev, drm_dma_handle_t * dmah) {
__drm_legacy_pci_free(dev, dmah);
dma_free_coherent(&dev->pdev->dev, dmah->size, dmah->vaddr,dmah->busaddr); kfree(dmah);}
diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c index 52e87e4869a5..64619fe90046 100644 --- a/drivers/gpu/drm/drm_vm.c +++ b/drivers/gpu/drm/drm_vm.c @@ -269,8 +269,6 @@ static void drm_vm_shm_close(struct vm_area_struct *vma) }
if (!found_maps) {
drm_dma_handle_t dmah;switch (map->type) { case _DRM_REGISTERS: case _DRM_FRAME_BUFFER:@@ -284,10 +282,10 @@ static void drm_vm_shm_close(struct vm_area_struct *vma) case _DRM_SCATTER_GATHER: break; case _DRM_CONSISTENT:
dmah.vaddr = map->handle;dmah.busaddr = map->offset;dmah.size = map->size;__drm_legacy_pci_free(dev, &dmah);
dma_free_coherent(&dev->pdev->dev,map->size,map->handle,map->offset); break; } kfree(map);diff --git a/include/drm/drm_legacy.h b/include/drm/drm_legacy.h index 5745710453c8..dcef3598f49e 100644 --- a/include/drm/drm_legacy.h +++ b/include/drm/drm_legacy.h @@ -194,17 +194,11 @@ void drm_legacy_idlelock_release(struct drm_lock_data *lock);
#ifdef CONFIG_PCI
-void __drm_legacy_pci_free(struct drm_device *dev, drm_dma_handle_t * dmah); int drm_legacy_pci_init(struct drm_driver *driver, struct pci_driver *pdriver); void drm_legacy_pci_exit(struct drm_driver *driver, struct pci_driver *pdriver);
#else
-static inline void __drm_legacy_pci_free(struct drm_device *dev,
drm_dma_handle_t *dmah)-{ -}
static inline int drm_legacy_pci_init(struct drm_driver *driver, struct pci_driver *pdriver) { -- 2.25.0
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
drm_pci_alloc is a thin wrapper over dma_coherent_alloc. Ditch the wrapper and just use the dma routines directly.
Signed-off-by: Chris Wilson chris@chris-wilson.co.uk --- drivers/gpu/drm/r128/ati_pcigart.c | 32 +++++++++++++++--------------- drivers/gpu/drm/r128/ati_pcigart.h | 2 +- 2 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/r128/ati_pcigart.c b/drivers/gpu/drm/r128/ati_pcigart.c index 9b4072f97215..3d67afbbf0fc 100644 --- a/drivers/gpu/drm/r128/ati_pcigart.c +++ b/drivers/gpu/drm/r128/ati_pcigart.c @@ -44,9 +44,12 @@ static int drm_ati_alloc_pcigart_table(struct drm_device *dev, struct drm_ati_pcigart_info *gart_info) { - gart_info->table_handle = drm_pci_alloc(dev, gart_info->table_size, - PAGE_SIZE); - if (gart_info->table_handle == NULL) + gart_info->addr = + dma_alloc_coherent(&dev->pdev->dev, + gart_info->table_size, + ^gart_info->bus_addr, + GFP_KERNEL); + if (!gart_info->addr) return -ENOMEM;
return 0; @@ -55,8 +58,10 @@ static int drm_ati_alloc_pcigart_table(struct drm_device *dev, static void drm_ati_free_pcigart_table(struct drm_device *dev, struct drm_ati_pcigart_info *gart_info) { - drm_pci_free(dev, gart_info->table_handle); - gart_info->table_handle = NULL; + dma_free_coherent(&dev->pdev->dev, + gart_info->table_size, + gart_info->addr, + gart_info->bus_addr); }
int drm_ati_pcigart_cleanup(struct drm_device *dev, struct drm_ati_pcigart_info *gart_info) @@ -89,8 +94,7 @@ int drm_ati_pcigart_cleanup(struct drm_device *dev, struct drm_ati_pcigart_info gart_info->bus_addr = 0; }
- if (gart_info->gart_table_location == DRM_ATI_GART_MAIN && - gart_info->table_handle) { + if (gart_info->gart_table_location == DRM_ATI_GART_MAIN) drm_ati_free_pcigart_table(dev, gart_info); }
@@ -103,7 +107,7 @@ int drm_ati_pcigart_init(struct drm_device *dev, struct drm_ati_pcigart_info *ga struct drm_sg_mem *entry = dev->sg; void *address = NULL; unsigned long pages; - u32 *pci_gart = NULL, page_base, gart_idx; + u32 *page_base, gart_idx; dma_addr_t bus_address = 0; int i, j, ret = -ENOMEM; int max_ati_pages, max_real_pages; @@ -128,18 +132,14 @@ int drm_ati_pcigart_init(struct drm_device *dev, struct drm_ati_pcigart_info *ga DRM_ERROR("cannot allocate PCI GART page!\n"); goto done; } - - pci_gart = gart_info->table_handle->vaddr; - address = gart_info->table_handle->vaddr; - bus_address = gart_info->table_handle->busaddr; } else { - address = gart_info->addr; - bus_address = gart_info->bus_addr; DRM_DEBUG("PCI: Gart Table: VRAM %08LX mapped at %08lX\n", (unsigned long long)bus_address, (unsigned long)address); }
+ address = gart_info->addr; + bus_address = gart_info->bus_addr;
max_ati_pages = (gart_info->table_size / sizeof(u32)); max_real_pages = max_ati_pages / (PAGE_SIZE / ATI_PCIGART_PAGE_SIZE); @@ -147,7 +147,7 @@ int drm_ati_pcigart_init(struct drm_device *dev, struct drm_ati_pcigart_info *ga ? entry->pages : max_real_pages;
if (gart_info->gart_table_location == DRM_ATI_GART_MAIN) { - memset(pci_gart, 0, max_ati_pages * sizeof(u32)); + memset(address, 0, max_ati_pages * sizeof(u32)); } else { memset_io((void __iomem *)map->handle, 0, max_ati_pages * sizeof(u32)); } @@ -185,7 +185,7 @@ int drm_ati_pcigart_init(struct drm_device *dev, struct drm_ati_pcigart_info *ga } if (gart_info->gart_table_location == DRM_ATI_GART_MAIN) { - pci_gart[gart_idx] = cpu_to_le32(val); + address[gart_idx] = cpu_to_le32(val); } else { offset = gart_idx * sizeof(u32); writel(val, (void __iomem *)map->handle + offset); diff --git a/drivers/gpu/drm/r128/ati_pcigart.h b/drivers/gpu/drm/r128/ati_pcigart.h index a728a1364e66..6219aced7e84 100644 --- a/drivers/gpu/drm/r128/ati_pcigart.h +++ b/drivers/gpu/drm/r128/ati_pcigart.h @@ -18,7 +18,7 @@ struct drm_ati_pcigart_info { void *addr; dma_addr_t bus_addr; dma_addr_t table_mask; - struct drm_dma_handle *table_handle; + dma_addr_t dma_addr; struct drm_local_map mapping; int table_size; };
On Sun, Feb 2, 2020 at 12:16 PM Chris Wilson chris@chris-wilson.co.uk wrote:
drm_pci_alloc is a thin wrapper over dma_coherent_alloc. Ditch the wrapper and just use the dma routines directly.
Signed-off-by: Chris Wilson chris@chris-wilson.co.uk
drivers/gpu/drm/r128/ati_pcigart.c | 32 +++++++++++++++--------------- drivers/gpu/drm/r128/ati_pcigart.h | 2 +- 2 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/r128/ati_pcigart.c b/drivers/gpu/drm/r128/ati_pcigart.c index 9b4072f97215..3d67afbbf0fc 100644 --- a/drivers/gpu/drm/r128/ati_pcigart.c +++ b/drivers/gpu/drm/r128/ati_pcigart.c @@ -44,9 +44,12 @@ static int drm_ati_alloc_pcigart_table(struct drm_device *dev, struct drm_ati_pcigart_info *gart_info) {
gart_info->table_handle = drm_pci_alloc(dev, gart_info->table_size,PAGE_SIZE);if (gart_info->table_handle == NULL)
gart_info->addr =dma_alloc_coherent(&dev->pdev->dev,gart_info->table_size,^gart_info->bus_addr,
Stray ^ here. With that fixed: Reviewed-by: Alex Deucher alexander.deucher@amd.com
GFP_KERNEL);if (!gart_info->addr) return -ENOMEM; return 0;@@ -55,8 +58,10 @@ static int drm_ati_alloc_pcigart_table(struct drm_device *dev, static void drm_ati_free_pcigart_table(struct drm_device *dev, struct drm_ati_pcigart_info *gart_info) {
drm_pci_free(dev, gart_info->table_handle);gart_info->table_handle = NULL;
dma_free_coherent(&dev->pdev->dev,gart_info->table_size,gart_info->addr,gart_info->bus_addr);}
int drm_ati_pcigart_cleanup(struct drm_device *dev, struct drm_ati_pcigart_info *gart_info) @@ -89,8 +94,7 @@ int drm_ati_pcigart_cleanup(struct drm_device *dev, struct drm_ati_pcigart_info gart_info->bus_addr = 0; }
if (gart_info->gart_table_location == DRM_ATI_GART_MAIN &&gart_info->table_handle) {
if (gart_info->gart_table_location == DRM_ATI_GART_MAIN) drm_ati_free_pcigart_table(dev, gart_info); }@@ -103,7 +107,7 @@ int drm_ati_pcigart_init(struct drm_device *dev, struct drm_ati_pcigart_info *ga struct drm_sg_mem *entry = dev->sg; void *address = NULL; unsigned long pages;
u32 *pci_gart = NULL, page_base, gart_idx;
u32 *page_base, gart_idx; dma_addr_t bus_address = 0; int i, j, ret = -ENOMEM; int max_ati_pages, max_real_pages;@@ -128,18 +132,14 @@ int drm_ati_pcigart_init(struct drm_device *dev, struct drm_ati_pcigart_info *ga DRM_ERROR("cannot allocate PCI GART page!\n"); goto done; }
pci_gart = gart_info->table_handle->vaddr;address = gart_info->table_handle->vaddr;bus_address = gart_info->table_handle->busaddr; } else {address = gart_info->addr;bus_address = gart_info->bus_addr; DRM_DEBUG("PCI: Gart Table: VRAM %08LX mapped at %08lX\n", (unsigned long long)bus_address, (unsigned long)address); }
address = gart_info->addr;bus_address = gart_info->bus_addr; max_ati_pages = (gart_info->table_size / sizeof(u32)); max_real_pages = max_ati_pages / (PAGE_SIZE / ATI_PCIGART_PAGE_SIZE);@@ -147,7 +147,7 @@ int drm_ati_pcigart_init(struct drm_device *dev, struct drm_ati_pcigart_info *ga ? entry->pages : max_real_pages;
if (gart_info->gart_table_location == DRM_ATI_GART_MAIN) {
memset(pci_gart, 0, max_ati_pages * sizeof(u32));
memset(address, 0, max_ati_pages * sizeof(u32)); } else { memset_io((void __iomem *)map->handle, 0, max_ati_pages * sizeof(u32)); }@@ -185,7 +185,7 @@ int drm_ati_pcigart_init(struct drm_device *dev, struct drm_ati_pcigart_info *ga } if (gart_info->gart_table_location == DRM_ATI_GART_MAIN) {
pci_gart[gart_idx] = cpu_to_le32(val);
address[gart_idx] = cpu_to_le32(val); } else { offset = gart_idx * sizeof(u32); writel(val, (void __iomem *)map->handle + offset);diff --git a/drivers/gpu/drm/r128/ati_pcigart.h b/drivers/gpu/drm/r128/ati_pcigart.h index a728a1364e66..6219aced7e84 100644 --- a/drivers/gpu/drm/r128/ati_pcigart.h +++ b/drivers/gpu/drm/r128/ati_pcigart.h @@ -18,7 +18,7 @@ struct drm_ati_pcigart_info { void *addr; dma_addr_t bus_addr; dma_addr_t table_mask;
struct drm_dma_handle *table_handle;
dma_addr_t dma_addr; struct drm_local_map mapping; int table_size;};
2.25.0
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi Chris,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on drm-intel/for-linux-next] [also build test ERROR on drm-tip/drm-tip drm-exynos/exynos-drm-next linus/master next-20200203] [cannot apply to tegra-drm/drm/tegra/for-next drm/drm-next v5.5] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Chris-Wilson/drm-Remove-PageReserve... base: git://anongit.freedesktop.org/drm-intel for-linux-next config: x86_64-randconfig-s2-20200204 (attached as .config) compiler: gcc-4.9 (Debian 4.9.2-10+deb8u1) 4.9.2 reproduce: # save the attached .config to linux build tree make ARCH=x86_64
If you fix the issue, kindly add following tag Reported-by: kbuild test robot lkp@intel.com
All error/warnings (new ones prefixed by >>):
drivers/gpu/drm/r128/ati_pcigart.c: In function 'drm_ati_alloc_pcigart_table':
drivers/gpu/drm/r128/ati_pcigart.c:50:7: error: expected expression before '^' token
^gart_info->bus_addr, ^
drivers/gpu/drm/r128/ati_pcigart.c:48:3: error: too few arguments to function 'dma_alloc_coherent'
dma_alloc_coherent(&dev->pdev->dev, ^ In file included from include/linux/pci-dma-compat.h:8:0, from include/linux/pci.h:2371, from include/drm/drm_pci.h:35, from drivers/gpu/drm/r128/ati_pcigart.c:37: include/linux/dma-mapping.h:641:21: note: declared here static inline void *dma_alloc_coherent(struct device *dev, size_t size, ^ drivers/gpu/drm/r128/ati_pcigart.c: At top level:
drivers/gpu/drm/r128/ati_pcigart.c:101:2: error: expected identifier or '(' before 'return'
return 1; ^
drivers/gpu/drm/r128/ati_pcigart.c:102:1: error: expected identifier or '(' before '}' token
} ^ drivers/gpu/drm/r128/ati_pcigart.c: In function 'drm_ati_pcigart_init':
drivers/gpu/drm/r128/ati_pcigart.c:168:13: warning: assignment makes pointer from integer without a cast
page_base = (u32) entry->busaddr[i]; ^
drivers/gpu/drm/r128/ati_pcigart.c:176:21: error: invalid operands to binary | (have 'u32 *' and 'int')
val = page_base | 0xc; ^ drivers/gpu/drm/r128/ati_pcigart.c:179:22: error: invalid operands to binary >> (have 'u32 *' and 'int') val = (page_base >> 8) | 0xc; ^
drivers/gpu/drm/r128/ati_pcigart.c:183:9: warning: assignment makes integer from pointer without a cast
val = page_base; ^
drivers/gpu/drm/r128/ati_pcigart.c:188:12: warning: dereferencing 'void *' pointer
address[gart_idx] = cpu_to_le32(val); ^
drivers/gpu/drm/r128/ati_pcigart.c:188:5: error: invalid use of void expression
address[gart_idx] = cpu_to_le32(val); ^ drivers/gpu/drm/r128/ati_pcigart.c: In function 'drm_ati_pcigart_cleanup':
drivers/gpu/drm/r128/ati_pcigart.c:99:2: warning: control reaches end of non-void function [-Wreturn-type]
} ^
vim +50 drivers/gpu/drm/r128/ati_pcigart.c
43 44 static int drm_ati_alloc_pcigart_table(struct drm_device *dev, 45 struct drm_ati_pcigart_info *gart_info) 46 { 47 gart_info->addr =
48 dma_alloc_coherent(&dev->pdev->dev,
49 gart_info->table_size,
50 ^gart_info->bus_addr,
51 GFP_KERNEL); 52 if (!gart_info->addr) 53 return -ENOMEM; 54 55 return 0; 56 } 57 58 static void drm_ati_free_pcigart_table(struct drm_device *dev, 59 struct drm_ati_pcigart_info *gart_info) 60 { 61 dma_free_coherent(&dev->pdev->dev, 62 gart_info->table_size, 63 gart_info->addr, 64 gart_info->bus_addr); 65 } 66 67 int drm_ati_pcigart_cleanup(struct drm_device *dev, struct drm_ati_pcigart_info *gart_info) 68 { 69 struct drm_sg_mem *entry = dev->sg; 70 unsigned long pages; 71 int i; 72 int max_pages; 73 74 /* we need to support large memory configurations */ 75 if (!entry) { 76 DRM_ERROR("no scatter/gather memory!\n"); 77 return 0; 78 } 79 80 if (gart_info->bus_addr) { 81 82 max_pages = (gart_info->table_size / sizeof(u32)); 83 pages = (entry->pages <= max_pages) 84 ? entry->pages : max_pages; 85 86 for (i = 0; i < pages; i++) { 87 if (!entry->busaddr[i]) 88 break; 89 pci_unmap_page(dev->pdev, entry->busaddr[i], 90 PAGE_SIZE, PCI_DMA_BIDIRECTIONAL); 91 } 92 93 if (gart_info->gart_table_location == DRM_ATI_GART_MAIN) 94 gart_info->bus_addr = 0; 95 } 96 97 if (gart_info->gart_table_location == DRM_ATI_GART_MAIN) 98 drm_ati_free_pcigart_table(dev, gart_info);
99 }
100
101 return 1; 102 }
103 104 int drm_ati_pcigart_init(struct drm_device *dev, struct drm_ati_pcigart_info *gart_info) 105 { 106 struct drm_local_map *map = &gart_info->mapping; 107 struct drm_sg_mem *entry = dev->sg; 108 void *address = NULL; 109 unsigned long pages; 110 u32 *page_base, gart_idx; 111 dma_addr_t bus_address = 0; 112 int i, j, ret = -ENOMEM; 113 int max_ati_pages, max_real_pages; 114 115 if (!entry) { 116 DRM_ERROR("no scatter/gather memory!\n"); 117 goto done; 118 } 119 120 if (gart_info->gart_table_location == DRM_ATI_GART_MAIN) { 121 DRM_DEBUG("PCI: no table in VRAM: using normal RAM\n"); 122 123 if (pci_set_dma_mask(dev->pdev, gart_info->table_mask)) { 124 DRM_ERROR("fail to set dma mask to 0x%Lx\n", 125 (unsigned long long)gart_info->table_mask); 126 ret = -EFAULT; 127 goto done; 128 } 129 130 ret = drm_ati_alloc_pcigart_table(dev, gart_info); 131 if (ret) { 132 DRM_ERROR("cannot allocate PCI GART page!\n"); 133 goto done; 134 } 135 } else { 136 DRM_DEBUG("PCI: Gart Table: VRAM %08LX mapped at %08lX\n", 137 (unsigned long long)bus_address, 138 (unsigned long)address); 139 } 140 141 address = gart_info->addr; 142 bus_address = gart_info->bus_addr; 143 144 max_ati_pages = (gart_info->table_size / sizeof(u32)); 145 max_real_pages = max_ati_pages / (PAGE_SIZE / ATI_PCIGART_PAGE_SIZE); 146 pages = (entry->pages <= max_real_pages) 147 ? entry->pages : max_real_pages; 148 149 if (gart_info->gart_table_location == DRM_ATI_GART_MAIN) { 150 memset(address, 0, max_ati_pages * sizeof(u32)); 151 } else { 152 memset_io((void __iomem *)map->handle, 0, max_ati_pages * sizeof(u32)); 153 } 154 155 gart_idx = 0; 156 for (i = 0; i < pages; i++) { 157 /* we need to support large memory configurations */ 158 entry->busaddr[i] = pci_map_page(dev->pdev, entry->pagelist[i], 159 0, PAGE_SIZE, PCI_DMA_BIDIRECTIONAL); 160 if (pci_dma_mapping_error(dev->pdev, entry->busaddr[i])) { 161 DRM_ERROR("unable to map PCIGART pages!\n"); 162 drm_ati_pcigart_cleanup(dev, gart_info); 163 address = NULL; 164 bus_address = 0; 165 ret = -ENOMEM; 166 goto done; 167 }
168 page_base = (u32) entry->busaddr[i];
169 170 for (j = 0; j < (PAGE_SIZE / ATI_PCIGART_PAGE_SIZE); j++) { 171 u32 offset; 172 u32 val; 173 174 switch(gart_info->gart_reg_if) { 175 case DRM_ATI_GART_IGP:
176 val = page_base | 0xc;
177 break; 178 case DRM_ATI_GART_PCIE: 179 val = (page_base >> 8) | 0xc; 180 break; 181 default: 182 case DRM_ATI_GART_PCI:
183 val = page_base;
184 break; 185 } 186 if (gart_info->gart_table_location == 187 DRM_ATI_GART_MAIN) {
188 address[gart_idx] = cpu_to_le32(val);
189 } else { 190 offset = gart_idx * sizeof(u32); 191 writel(val, (void __iomem *)map->handle + offset); 192 } 193 gart_idx++; 194 page_base += ATI_PCIGART_PAGE_SIZE; 195 } 196 } 197 ret = 0; 198
--- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation
drm_pci_alloc and drm_pci_free are just very thin wrappers around dma_alloc_coherent, with a note that we should be removing them. Furthermore since
commit de09d31dd38a50fdce106c15abd68432eebbd014 Author: Kirill A. Shutemov kirill.shutemov@linux.intel.com Date: Fri Jan 15 16:51:42 2016 -0800
page-flags: define PG_reserved behavior on compound pages
As far as I can see there's no users of PG_reserved on compound pages. Let's use PF_NO_COMPOUND here.
drm_pci_alloc has been declared broken since it mixes GFP_COMP and SetPageReserved. Avoid this conflict by weaning ourselves off using the abstraction and using the dma functions directly.
Reported-by: Taketo Kabe Closes: https://gitlab.freedesktop.org/drm/intel/issues/1027 Fixes: de09d31dd38a ("page-flags: define PG_reserved behavior on compound pages") Signed-off-by: Chris Wilson chris@chris-wilson.co.uk Cc: stable@vger.kernel.org # v4.5+ --- drivers/gpu/drm/i915/display/intel_display.c | 2 +- .../gpu/drm/i915/gem/i915_gem_object_types.h | 3 - drivers/gpu/drm/i915/gem/i915_gem_phys.c | 98 ++++++++++--------- drivers/gpu/drm/i915/i915_gem.c | 8 +- 4 files changed, 55 insertions(+), 56 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index b0af37fb6d4a..1f584263aa97 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -11234,7 +11234,7 @@ static u32 intel_cursor_base(const struct intel_plane_state *plane_state) u32 base;
if (INTEL_INFO(dev_priv)->display.cursor_needs_physical) - base = obj->phys_handle->busaddr; + base = sg_dma_address(obj->mm.pages->sgl); else base = intel_plane_ggtt_offset(plane_state);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h index f64ad77e6b1e..c2174da35bb0 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h @@ -285,9 +285,6 @@ struct drm_i915_gem_object {
void *gvt_info; }; - - /** for phys allocated objects */ - struct drm_dma_handle *phys_handle; };
static inline struct drm_i915_gem_object * diff --git a/drivers/gpu/drm/i915/gem/i915_gem_phys.c b/drivers/gpu/drm/i915/gem/i915_gem_phys.c index b1b7c1b3038a..b07bb40edd5a 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_phys.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_phys.c @@ -22,88 +22,87 @@ static int i915_gem_object_get_pages_phys(struct drm_i915_gem_object *obj) { struct address_space *mapping = obj->base.filp->f_mapping; - struct drm_dma_handle *phys; - struct sg_table *st; struct scatterlist *sg; - char *vaddr; + struct sg_table *st; + dma_addr_t dma; + void *vaddr; + void *dst; int i; - int err;
if (WARN_ON(i915_gem_object_needs_bit17_swizzle(obj))) return -EINVAL;
- /* Always aligning to the object size, allows a single allocation + /* + * Always aligning to the object size, allows a single allocation * to handle all possible callers, and given typical object sizes, * the alignment of the buddy allocation will naturally match. */ - phys = drm_pci_alloc(obj->base.dev, - roundup_pow_of_two(obj->base.size), - roundup_pow_of_two(obj->base.size)); - if (!phys) + vaddr = dma_alloc_coherent(&obj->base.dev->pdev->dev, + roundup_pow_of_two(obj->base.size), + &dma, GFP_KERNEL); + if (!vaddr) return -ENOMEM;
- vaddr = phys->vaddr; + st = kmalloc(sizeof(*st), GFP_KERNEL); + if (!st) + goto err_pci; + + if (sg_alloc_table(st, 1, GFP_KERNEL)) + goto err_st; + + sg = st->sgl; + sg->offset = 0; + sg->length = obj->base.size; + + sg_assign_page(sg, (struct page *)vaddr); + sg_dma_address(sg) = dma; + sg_dma_len(sg) = obj->base.size; + + dst = vaddr; for (i = 0; i < obj->base.size / PAGE_SIZE; i++) { struct page *page; - char *src; + void *src;
page = shmem_read_mapping_page(mapping, i); - if (IS_ERR(page)) { - err = PTR_ERR(page); - goto err_phys; - } + if (IS_ERR(page)) + goto err_st;
src = kmap_atomic(page); - memcpy(vaddr, src, PAGE_SIZE); - drm_clflush_virt_range(vaddr, PAGE_SIZE); + memcpy(dst, src, PAGE_SIZE); + drm_clflush_virt_range(dst, PAGE_SIZE); kunmap_atomic(src);
put_page(page); - vaddr += PAGE_SIZE; + dst += PAGE_SIZE; }
intel_gt_chipset_flush(&to_i915(obj->base.dev)->gt);
- st = kmalloc(sizeof(*st), GFP_KERNEL); - if (!st) { - err = -ENOMEM; - goto err_phys; - } - - if (sg_alloc_table(st, 1, GFP_KERNEL)) { - kfree(st); - err = -ENOMEM; - goto err_phys; - } - - sg = st->sgl; - sg->offset = 0; - sg->length = obj->base.size; - - sg_dma_address(sg) = phys->busaddr; - sg_dma_len(sg) = obj->base.size; - - obj->phys_handle = phys; - __i915_gem_object_set_pages(obj, st, sg->length);
return 0;
-err_phys: - drm_pci_free(obj->base.dev, phys); - - return err; +err_st: + kfree(st); +err_pci: + dma_free_coherent(&obj->base.dev->pdev->dev, + roundup_pow_of_two(obj->base.size), + vaddr, dma); + return -ENOMEM; }
static void i915_gem_object_put_pages_phys(struct drm_i915_gem_object *obj, struct sg_table *pages) { + dma_addr_t dma = sg_dma_address(pages->sgl); + void *vaddr = sg_page(pages->sgl); + __i915_gem_object_release_shmem(obj, pages, false);
if (obj->mm.dirty) { struct address_space *mapping = obj->base.filp->f_mapping; - char *vaddr = obj->phys_handle->vaddr; + void *src = vaddr; int i;
for (i = 0; i < obj->base.size / PAGE_SIZE; i++) { @@ -115,15 +114,16 @@ i915_gem_object_put_pages_phys(struct drm_i915_gem_object *obj, continue;
dst = kmap_atomic(page); - drm_clflush_virt_range(vaddr, PAGE_SIZE); - memcpy(dst, vaddr, PAGE_SIZE); + drm_clflush_virt_range(src, PAGE_SIZE); + memcpy(dst, src, PAGE_SIZE); kunmap_atomic(dst);
set_page_dirty(page); if (obj->mm.madv == I915_MADV_WILLNEED) mark_page_accessed(page); put_page(page); - vaddr += PAGE_SIZE; + + src += PAGE_SIZE; } obj->mm.dirty = false; } @@ -131,7 +131,9 @@ i915_gem_object_put_pages_phys(struct drm_i915_gem_object *obj, sg_free_table(pages); kfree(pages);
- drm_pci_free(obj->base.dev, obj->phys_handle); + dma_free_coherent(&obj->base.dev->pdev->dev, + roundup_pow_of_two(obj->base.size), + vaddr, dma); }
static void phys_release(struct drm_i915_gem_object *obj) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 7245e056ce77..a712e60b016a 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -180,7 +180,7 @@ i915_gem_phys_pwrite(struct drm_i915_gem_object *obj, struct drm_i915_gem_pwrite *args, struct drm_file *file) { - void *vaddr = obj->phys_handle->vaddr + args->offset; + void *vaddr = sg_page(obj->mm.pages->sgl) + args->offset; char __user *user_data = u64_to_user_ptr(args->data_ptr);
/* @@ -844,10 +844,10 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data, ret = i915_gem_gtt_pwrite_fast(obj, args);
if (ret == -EFAULT || ret == -ENOSPC) { - if (obj->phys_handle) - ret = i915_gem_phys_pwrite(obj, args, file); - else + if (i915_gem_object_has_struct_page(obj)) ret = i915_gem_shmem_pwrite(obj, args); + else + ret = i915_gem_phys_pwrite(obj, args, file); }
i915_gem_object_unpin_pages(obj);
Hi,
[This is an automated email]
This commit has been processed because it contains a "Fixes:" tag, fixing commit: de09d31dd38a ("page-flags: define PG_reserved behavior on compound pages").
The bot has tested the following trees: v5.5.1, v5.4.17, v4.19.101, v4.14.169, v4.9.212.
v5.5.1: Build OK! v5.4.17: Build OK! v4.19.101: Failed to apply! Possible dependencies: 4a3d3f6785be ("drm/i915: Match code to comment and enforce ppgtt for execlists") 4bdafb9ddfa4 ("drm/i915: Remove i915.enable_ppgtt override") 5771caf885ae ("drm/i915/skl+: Decode memory bandwidth and parameters") 6323113b7af6 ("drm/i915: Move SKL IPC WA to HAS_IPC()") 79556df293b2 ("drm/i915/gtt: Enable full-ppgtt by default everywhere") 86b592876cb6 ("drm/i915: Implement 16GB dimm wa for latency level-0") 8a6c5447635c ("drm/i915/kbl+: Enable IPC only for symmetric memory configurations") 900ccf30f9e1 ("drm/i915: Only force GGTT coherency w/a on required chipsets") cbfa59d4b331 ("drm/i915/bxt: Decode memory bandwidth and parameters") d53db442db36 ("drm/i915: Move display device info capabilities to its own struct") f361912aa9bf ("drm/i915/skl+: don't trust IPC value set by BIOS") fd847b8e60e0 ("drm/i915: Do not modifiy reserved bit in gens that do not have IPC")
v4.14.169: Failed to apply! Possible dependencies: 0d6fc92a73e0 ("drm/i915: Separate RPS and RC6 handling for VLV") 37d933fc1728 ("drm/i915: Introduce separate status variable for RC6 and LLC ring frequency setup") 3e8ddd9e5071 ("drm/i915: Nuke some bogus tabs from the pcode defines") 562d9bae08a1 ("drm/i915: Name structure in dev_priv that contains RPS/RC6 state as "gt_pm"") 61843f0e6212 ("drm/i915: Name the IPS_PCODE_CONTROL bit") 771decb0b4d7 ("drm/i915: Rename intel_enable_rc6 to intel_rc6_enabled") 960e54652cee ("drm/i915: Separate RPS and RC6 handling for gen6+") 9f817501bd7f ("drm/i915: Move rps.hw_lock to dev_priv and s/hw_lock/pcu_lock") c56b89f16dd0 ("drm/i915: Use INTEL_GEN everywhere") d46b00dc38c8 ("drm/i915: Separate RPS and RC6 handling for CHV") d53db442db36 ("drm/i915: Move display device info capabilities to its own struct") fb6db0f5bf1d ("drm/i915: Remove unsafe i915.enable_rc6")
v4.9.212: Failed to apply! Possible dependencies: 0031fb96859c ("drm/i915: Assorted dev_priv cleanups") 03cdc1d4f795 ("drm/i915: Store port enum in intel_encoder") 4f8036a28112 ("drm/i915: Make HAS_DDI and HAS_PCH_LPT_LP only take dev_priv") 50a0bc905416 ("drm/i915: Make INTEL_DEVID only take dev_priv") 6e266956a57f ("drm/i915: Make INTEL_PCH_TYPE & co only take dev_priv") 8652744b647e ("drm/i915: Make IS_BROADWELL only take dev_priv") d53db442db36 ("drm/i915: Move display device info capabilities to its own struct")
NOTE: The patch will not be queued to stable trees until it is upstream.
How should we proceed with this patch?
The drm_pci_alloc routines have been a thin wrapper around the core dma coherent routines. Remove the crutch of a wrapper and the exported symbols, marking it for only internal legacy use.
Signed-off-by: Chris Wilson chris@chris-wilson.co.uk --- drivers/gpu/drm/drm_bufs.c | 5 +++-- drivers/gpu/drm/drm_legacy.h | 23 +++++++++++++++++++++++ drivers/gpu/drm/drm_pci.c | 31 ++++++------------------------- include/drm/drm_pci.h | 18 ------------------ 4 files changed, 32 insertions(+), 45 deletions(-)
diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c index 19297e58b232..a33df3744f76 100644 --- a/drivers/gpu/drm/drm_bufs.c +++ b/drivers/gpu/drm/drm_bufs.c @@ -675,7 +675,7 @@ static void drm_cleanup_buf_error(struct drm_device *dev, if (entry->seg_count) { for (i = 0; i < entry->seg_count; i++) { if (entry->seglist[i]) { - drm_pci_free(dev, entry->seglist[i]); + drm_legacy_pci_free(dev, entry->seglist[i]); } } kfree(entry->seglist); @@ -975,7 +975,8 @@ int drm_legacy_addbufs_pci(struct drm_device *dev,
while (entry->buf_count < count) {
- dmah = drm_pci_alloc(dev, PAGE_SIZE << page_order, 0x1000); + dmah = drm_legacy_pci_alloc(dev, + PAGE_SIZE << page_order, 0x1000);
if (!dmah) { /* Set count correctly so we free the proper amount. */ diff --git a/drivers/gpu/drm/drm_legacy.h b/drivers/gpu/drm/drm_legacy.h index 1be3ea320474..3853b45341c7 100644 --- a/drivers/gpu/drm/drm_legacy.h +++ b/drivers/gpu/drm/drm_legacy.h @@ -36,6 +36,7 @@
struct agp_memory; struct drm_device; +struct drm_dma_handle; struct drm_file; struct drm_buf_desc;
@@ -211,4 +212,26 @@ void drm_master_legacy_init(struct drm_master *master); static inline void drm_master_legacy_init(struct drm_master *master) {} #endif
+ +#if IS_ENABLED(CONFIG_DRM_LEGACY) && IS_ENABLED(CONFIG_PCI) + +struct drm_dma_handle * +drm_legacy_pci_alloc(struct drm_device *dev, size_t size, size_t align); +void drm_legacy_pci_free(struct drm_device *dev, struct drm_dma_handle * dmah); + +#else + +static inline struct drm_dma_handle * +drm_legacy_pci_alloc(struct drm_device *dev, size_t size, size_t align) +{ + return NULL; +} + +static inline void drm_legacy_pci_free(struct drm_device *dev, + struct drm_dma_handle *dmah) +{ +} + +#endif + #endif /* __DRM_LEGACY_H__ */ diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index c6bb98729a26..12239498538c 100644 --- a/drivers/gpu/drm/drm_pci.c +++ b/drivers/gpu/drm/drm_pci.c @@ -36,19 +36,10 @@ #include "drm_internal.h" #include "drm_legacy.h"
-/** - * drm_pci_alloc - Allocate a PCI consistent memory block, for DMA. - * @dev: DRM device - * @size: size of block to allocate - * @align: alignment of block - * - * FIXME: This is a needless abstraction of the Linux dma-api and should be - * removed. - * - * Return: A handle to the allocated memory block on success or NULL on - * failure. - */ -drm_dma_handle_t *drm_pci_alloc(struct drm_device * dev, size_t size, size_t align) +#if IS_ENABLED(CONFIG_DRM_LEGACY) && IS_ENABLED(CONFIG_PCI) + +drm_dma_handle_t * +drm_legacy_pci_alloc(struct drm_device * dev, size_t size, size_t align) { drm_dma_handle_t *dmah;
@@ -76,24 +67,14 @@ drm_dma_handle_t *drm_pci_alloc(struct drm_device * dev, size_t size, size_t ali return dmah; }
-EXPORT_SYMBOL(drm_pci_alloc); - -/** - * drm_pci_free - Free a PCI consistent memory block - * @dev: DRM device - * @dmah: handle to memory block - * - * FIXME: This is a needless abstraction of the Linux dma-api and should be - * removed. - */ -void drm_pci_free(struct drm_device * dev, drm_dma_handle_t * dmah) +void drm_legacy_pci_free(struct drm_device * dev, drm_dma_handle_t * dmah) { dma_free_coherent(&dev->pdev->dev, dmah->size, dmah->vaddr, dmah->busaddr); kfree(dmah); }
-EXPORT_SYMBOL(drm_pci_free); +#endif
static int drm_get_pci_domain(struct drm_device *dev) { diff --git a/include/drm/drm_pci.h b/include/drm/drm_pci.h index 9031e217b506..cade5b60b643 100644 --- a/include/drm/drm_pci.h +++ b/include/drm/drm_pci.h @@ -34,34 +34,16 @@
#include <linux/pci.h>
-struct drm_dma_handle; -struct drm_device; struct drm_driver; -struct drm_master;
#ifdef CONFIG_PCI
-struct drm_dma_handle *drm_pci_alloc(struct drm_device *dev, size_t size, - size_t align); -void drm_pci_free(struct drm_device *dev, struct drm_dma_handle * dmah); - int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent, struct drm_driver *driver);
#else
-static inline struct drm_dma_handle *drm_pci_alloc(struct drm_device *dev, - size_t size, size_t align) -{ - return NULL; -} - -static inline void drm_pci_free(struct drm_device *dev, - struct drm_dma_handle *dmah) -{ -} - static inline int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent, struct drm_driver *driver)
On Sun, Feb 2, 2020 at 12:16 PM Chris Wilson chris@chris-wilson.co.uk wrote:
The drm_pci_alloc routines have been a thin wrapper around the core dma coherent routines. Remove the crutch of a wrapper and the exported symbols, marking it for only internal legacy use.
Signed-off-by: Chris Wilson chris@chris-wilson.co.uk
Reviewed-by: Alex Deucher alexander.deucher@amd.com
drivers/gpu/drm/drm_bufs.c | 5 +++-- drivers/gpu/drm/drm_legacy.h | 23 +++++++++++++++++++++++ drivers/gpu/drm/drm_pci.c | 31 ++++++------------------------- include/drm/drm_pci.h | 18 ------------------ 4 files changed, 32 insertions(+), 45 deletions(-)
diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c index 19297e58b232..a33df3744f76 100644 --- a/drivers/gpu/drm/drm_bufs.c +++ b/drivers/gpu/drm/drm_bufs.c @@ -675,7 +675,7 @@ static void drm_cleanup_buf_error(struct drm_device *dev, if (entry->seg_count) { for (i = 0; i < entry->seg_count; i++) { if (entry->seglist[i]) {
drm_pci_free(dev, entry->seglist[i]);
drm_legacy_pci_free(dev, entry->seglist[i]); } } kfree(entry->seglist);@@ -975,7 +975,8 @@ int drm_legacy_addbufs_pci(struct drm_device *dev,
while (entry->buf_count < count) {
dmah = drm_pci_alloc(dev, PAGE_SIZE << page_order, 0x1000);
dmah = drm_legacy_pci_alloc(dev,PAGE_SIZE << page_order, 0x1000); if (!dmah) { /* Set count correctly so we free the proper amount. */diff --git a/drivers/gpu/drm/drm_legacy.h b/drivers/gpu/drm/drm_legacy.h index 1be3ea320474..3853b45341c7 100644 --- a/drivers/gpu/drm/drm_legacy.h +++ b/drivers/gpu/drm/drm_legacy.h @@ -36,6 +36,7 @@
struct agp_memory; struct drm_device; +struct drm_dma_handle; struct drm_file; struct drm_buf_desc;
@@ -211,4 +212,26 @@ void drm_master_legacy_init(struct drm_master *master); static inline void drm_master_legacy_init(struct drm_master *master) {} #endif
+#if IS_ENABLED(CONFIG_DRM_LEGACY) && IS_ENABLED(CONFIG_PCI)
+struct drm_dma_handle * +drm_legacy_pci_alloc(struct drm_device *dev, size_t size, size_t align); +void drm_legacy_pci_free(struct drm_device *dev, struct drm_dma_handle * dmah);
+#else
+static inline struct drm_dma_handle * +drm_legacy_pci_alloc(struct drm_device *dev, size_t size, size_t align) +{
return NULL;+}
+static inline void drm_legacy_pci_free(struct drm_device *dev,
struct drm_dma_handle *dmah)+{ +}
+#endif
#endif /* __DRM_LEGACY_H__ */ diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index c6bb98729a26..12239498538c 100644 --- a/drivers/gpu/drm/drm_pci.c +++ b/drivers/gpu/drm/drm_pci.c @@ -36,19 +36,10 @@ #include "drm_internal.h" #include "drm_legacy.h"
-/**
- drm_pci_alloc - Allocate a PCI consistent memory block, for DMA.
- @dev: DRM device
- @size: size of block to allocate
- @align: alignment of block
- FIXME: This is a needless abstraction of the Linux dma-api and should be
- removed.
- Return: A handle to the allocated memory block on success or NULL on
- failure.
- */
-drm_dma_handle_t *drm_pci_alloc(struct drm_device * dev, size_t size, size_t align) +#if IS_ENABLED(CONFIG_DRM_LEGACY) && IS_ENABLED(CONFIG_PCI)
+drm_dma_handle_t * +drm_legacy_pci_alloc(struct drm_device * dev, size_t size, size_t align) { drm_dma_handle_t *dmah;
@@ -76,24 +67,14 @@ drm_dma_handle_t *drm_pci_alloc(struct drm_device * dev, size_t size, size_t ali return dmah; }
-EXPORT_SYMBOL(drm_pci_alloc);
-/**
- drm_pci_free - Free a PCI consistent memory block
- @dev: DRM device
- @dmah: handle to memory block
- FIXME: This is a needless abstraction of the Linux dma-api and should be
- removed.
- */
-void drm_pci_free(struct drm_device * dev, drm_dma_handle_t * dmah) +void drm_legacy_pci_free(struct drm_device * dev, drm_dma_handle_t * dmah) { dma_free_coherent(&dev->pdev->dev, dmah->size, dmah->vaddr, dmah->busaddr); kfree(dmah); }
-EXPORT_SYMBOL(drm_pci_free); +#endif
static int drm_get_pci_domain(struct drm_device *dev) { diff --git a/include/drm/drm_pci.h b/include/drm/drm_pci.h index 9031e217b506..cade5b60b643 100644 --- a/include/drm/drm_pci.h +++ b/include/drm/drm_pci.h @@ -34,34 +34,16 @@
#include <linux/pci.h>
-struct drm_dma_handle; -struct drm_device; struct drm_driver; -struct drm_master;
#ifdef CONFIG_PCI
-struct drm_dma_handle *drm_pci_alloc(struct drm_device *dev, size_t size,
size_t align);-void drm_pci_free(struct drm_device *dev, struct drm_dma_handle * dmah);
int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent, struct drm_driver *driver);
#else
-static inline struct drm_dma_handle *drm_pci_alloc(struct drm_device *dev,
size_t size, size_t align)-{
return NULL;-}
-static inline void drm_pci_free(struct drm_device *dev,
struct drm_dma_handle *dmah)-{ -}
static inline int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent, struct drm_driver *driver) -- 2.25.0
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi Chris,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on drm-intel/for-linux-next] [also build test ERROR on drm-tip/drm-tip drm-exynos/exynos-drm-next linus/master next-20200203] [cannot apply to tegra-drm/drm/tegra/for-next drm/drm-next v5.5] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Chris-Wilson/drm-Remove-PageReserve... base: git://anongit.freedesktop.org/drm-intel for-linux-next config: arm-multi_v7_defconfig (attached as .config) compiler: arm-linux-gnueabi-gcc (GCC) 7.5.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree GCC_VERSION=7.5.0 make.cross ARCH=arm
If you fix the issue, kindly add following tag Reported-by: kbuild test robot lkp@intel.com
All errors (new ones prefixed by >>):
drivers/gpu/drm/drm_dma.c: In function 'drm_legacy_dma_takedown':
drivers/gpu/drm/drm_dma.c:103:6: error: implicit declaration of function 'drm_pci_free'; did you mean 'arm_dma_free'? [-Werror=implicit-function-declaration]
drm_pci_free(dev, dma->bufs[i].seglist[j]); ^~~~~~~~~~~~ arm_dma_free cc1: some warnings being treated as errors
vim +103 drivers/gpu/drm/drm_dma.c
^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 72 ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 73 /** ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 74 * Cleanup the DMA resources. ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 75 * ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 76 * \param dev DRM device. ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 77 * ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 78 * Free all pages associated with DMA buffers, the buffers and pages lists, and 59c51591a0ac75 drivers/char/drm/drm_dma.c Michael Opdenacker 2007-05-09 79 * finally the drm_device::dma structure itself. ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 80 */ e2e99a8206bcce drivers/gpu/drm/drm_dma.c Daniel Vetter 2013-08-08 81 void drm_legacy_dma_takedown(struct drm_device *dev) ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 82 { cdd55a294c13f8 drivers/char/drm/drm_dma.c Dave Airlie 2007-07-11 83 struct drm_device_dma *dma = dev->dma; ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 84 int i, j; ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 85 e2e99a8206bcce drivers/gpu/drm/drm_dma.c Daniel Vetter 2013-08-08 86 if (!drm_core_check_feature(dev, DRIVER_HAVE_DMA) || fa5386459f06dc drivers/gpu/drm/drm_dma.c Daniel Vetter 2016-08-03 87 !drm_core_check_feature(dev, DRIVER_LEGACY)) e2e99a8206bcce drivers/gpu/drm/drm_dma.c Daniel Vetter 2013-08-08 88 return; e2e99a8206bcce drivers/gpu/drm/drm_dma.c Daniel Vetter 2013-08-08 89 b5e89ed53ed8d2 drivers/char/drm/drm_dma.c Dave Airlie 2005-09-25 90 if (!dma) b5e89ed53ed8d2 drivers/char/drm/drm_dma.c Dave Airlie 2005-09-25 91 return; ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 92 ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 93 /* Clear dma buffers */ ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 94 for (i = 0; i <= DRM_MAX_ORDER; i++) { ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 95 if (dma->bufs[i].seg_count) { ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 96 DRM_DEBUG("order %d: buf_count = %d," ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 97 " seg_count = %d\n", ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 98 i, ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 99 dma->bufs[i].buf_count, ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 100 dma->bufs[i].seg_count); ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 101 for (j = 0; j < dma->bufs[i].seg_count; j++) { ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 102 if (dma->bufs[i].seglist[j]) { ddf19b973be5a9 drivers/char/drm/drm_dma.c Dave Airlie 2006-03-19 @103 drm_pci_free(dev, dma->bufs[i].seglist[j]); ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 104 } ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 105 } 9a298b2acd771d drivers/gpu/drm/drm_dma.c Eric Anholt 2009-03-24 106 kfree(dma->bufs[i].seglist); ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 107 } ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 108 if (dma->bufs[i].buf_count) { ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 109 for (j = 0; j < dma->bufs[i].buf_count; j++) { 9a298b2acd771d drivers/gpu/drm/drm_dma.c Eric Anholt 2009-03-24 110 kfree(dma->bufs[i].buflist[j].dev_private); ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 111 } 9a298b2acd771d drivers/gpu/drm/drm_dma.c Eric Anholt 2009-03-24 112 kfree(dma->bufs[i].buflist); ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 113 } ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 114 } ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 115 9a298b2acd771d drivers/gpu/drm/drm_dma.c Eric Anholt 2009-03-24 116 kfree(dma->buflist); 9a298b2acd771d drivers/gpu/drm/drm_dma.c Eric Anholt 2009-03-24 117 kfree(dma->pagelist); 9a298b2acd771d drivers/gpu/drm/drm_dma.c Eric Anholt 2009-03-24 118 kfree(dev->dma); ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 119 dev->dma = NULL; ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 120 } ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 121
:::::: The code at line 103 was first introduced by commit :::::: ddf19b973be5a96d77c8467f657fe5bd7d126e0f drm: fixup PCI DMA support
:::::: TO: Dave Airlie airlied@linux.ie :::::: CC: Dave Airlie airlied@linux.ie
--- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation
Hi Chris,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on drm-intel/for-linux-next] [also build test ERROR on drm-tip/drm-tip drm-exynos/exynos-drm-next linus/master next-20200203] [cannot apply to tegra-drm/drm/tegra/for-next drm/drm-next v5.5] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Chris-Wilson/drm-Remove-PageReserve... base: git://anongit.freedesktop.org/drm-intel for-linux-next config: x86_64-randconfig-s2-20200204 (attached as .config) compiler: gcc-4.9 (Debian 4.9.2-10+deb8u1) 4.9.2 reproduce: # save the attached .config to linux build tree make ARCH=x86_64
If you fix the issue, kindly add following tag Reported-by: kbuild test robot lkp@intel.com
All errors (new ones prefixed by >>):
drivers/gpu/drm/drm_dma.c: In function 'drm_legacy_dma_takedown':
drivers/gpu/drm/drm_dma.c:103:6: error: implicit declaration of function 'drm_pci_free' [-Werror=implicit-function-declaration]
drm_pci_free(dev, dma->bufs[i].seglist[j]); ^ cc1: some warnings being treated as errors
vim +/drm_pci_free +103 drivers/gpu/drm/drm_dma.c
^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 72 ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 73 /** ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 74 * Cleanup the DMA resources. ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 75 * ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 76 * \param dev DRM device. ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 77 * ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 78 * Free all pages associated with DMA buffers, the buffers and pages lists, and 59c51591a0ac75 drivers/char/drm/drm_dma.c Michael Opdenacker 2007-05-09 79 * finally the drm_device::dma structure itself. ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 80 */ e2e99a8206bcce drivers/gpu/drm/drm_dma.c Daniel Vetter 2013-08-08 81 void drm_legacy_dma_takedown(struct drm_device *dev) ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 82 { cdd55a294c13f8 drivers/char/drm/drm_dma.c Dave Airlie 2007-07-11 83 struct drm_device_dma *dma = dev->dma; ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 84 int i, j; ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 85 e2e99a8206bcce drivers/gpu/drm/drm_dma.c Daniel Vetter 2013-08-08 86 if (!drm_core_check_feature(dev, DRIVER_HAVE_DMA) || fa5386459f06dc drivers/gpu/drm/drm_dma.c Daniel Vetter 2016-08-03 87 !drm_core_check_feature(dev, DRIVER_LEGACY)) e2e99a8206bcce drivers/gpu/drm/drm_dma.c Daniel Vetter 2013-08-08 88 return; e2e99a8206bcce drivers/gpu/drm/drm_dma.c Daniel Vetter 2013-08-08 89 b5e89ed53ed8d2 drivers/char/drm/drm_dma.c Dave Airlie 2005-09-25 90 if (!dma) b5e89ed53ed8d2 drivers/char/drm/drm_dma.c Dave Airlie 2005-09-25 91 return; ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 92 ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 93 /* Clear dma buffers */ ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 94 for (i = 0; i <= DRM_MAX_ORDER; i++) { ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 95 if (dma->bufs[i].seg_count) { ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 96 DRM_DEBUG("order %d: buf_count = %d," ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 97 " seg_count = %d\n", ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 98 i, ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 99 dma->bufs[i].buf_count, ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 100 dma->bufs[i].seg_count); ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 101 for (j = 0; j < dma->bufs[i].seg_count; j++) { ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 102 if (dma->bufs[i].seglist[j]) { ddf19b973be5a9 drivers/char/drm/drm_dma.c Dave Airlie 2006-03-19 @103 drm_pci_free(dev, dma->bufs[i].seglist[j]); ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 104 } ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 105 } 9a298b2acd771d drivers/gpu/drm/drm_dma.c Eric Anholt 2009-03-24 106 kfree(dma->bufs[i].seglist); ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 107 } ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 108 if (dma->bufs[i].buf_count) { ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 109 for (j = 0; j < dma->bufs[i].buf_count; j++) { 9a298b2acd771d drivers/gpu/drm/drm_dma.c Eric Anholt 2009-03-24 110 kfree(dma->bufs[i].buflist[j].dev_private); ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 111 } 9a298b2acd771d drivers/gpu/drm/drm_dma.c Eric Anholt 2009-03-24 112 kfree(dma->bufs[i].buflist); ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 113 } ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 114 } ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 115 9a298b2acd771d drivers/gpu/drm/drm_dma.c Eric Anholt 2009-03-24 116 kfree(dma->buflist); 9a298b2acd771d drivers/gpu/drm/drm_dma.c Eric Anholt 2009-03-24 117 kfree(dma->pagelist); 9a298b2acd771d drivers/gpu/drm/drm_dma.c Eric Anholt 2009-03-24 118 kfree(dev->dma); ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 119 dev->dma = NULL; ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 120 } ^1da177e4c3f41 drivers/char/drm/drm_dma.c Linus Torvalds 2005-04-16 121
:::::: The code at line 103 was first introduced by commit :::::: ddf19b973be5a96d77c8467f657fe5bd7d126e0f drm: fixup PCI DMA support
:::::: TO: Dave Airlie airlied@linux.ie :::::: CC: Dave Airlie airlied@linux.ie
--- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation
On Sun, Feb 02, 2020 at 05:16:35PM +0000, Chris Wilson wrote:
The drm_pci_alloc routines have been a thin wrapper around the core dma coherent routines. Remove the crutch of a wrapper and the exported symbols, marking it for only internal legacy use.
Signed-off-by: Chris Wilson chris@chris-wilson.co.uk
Since Alex bothered to review the drm_bufs&r128 patches ...
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
I think all the other patches I've r-b stamped somewhere else already, but if they changed pls poke. -Daniel
drivers/gpu/drm/drm_bufs.c | 5 +++-- drivers/gpu/drm/drm_legacy.h | 23 +++++++++++++++++++++++ drivers/gpu/drm/drm_pci.c | 31 ++++++------------------------- include/drm/drm_pci.h | 18 ------------------ 4 files changed, 32 insertions(+), 45 deletions(-)
diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c index 19297e58b232..a33df3744f76 100644 --- a/drivers/gpu/drm/drm_bufs.c +++ b/drivers/gpu/drm/drm_bufs.c @@ -675,7 +675,7 @@ static void drm_cleanup_buf_error(struct drm_device *dev, if (entry->seg_count) { for (i = 0; i < entry->seg_count; i++) { if (entry->seglist[i]) {
drm_pci_free(dev, entry->seglist[i]);
drm_legacy_pci_free(dev, entry->seglist[i]); }@@ -975,7 +975,8 @@ int drm_legacy_addbufs_pci(struct drm_device *dev,
while (entry->buf_count < count) {
dmah = drm_pci_alloc(dev, PAGE_SIZE << page_order, 0x1000);
dmah = drm_legacy_pci_alloc(dev,PAGE_SIZE << page_order, 0x1000);if (!dmah) { /* Set count correctly so we free the proper amount. */
diff --git a/drivers/gpu/drm/drm_legacy.h b/drivers/gpu/drm/drm_legacy.h index 1be3ea320474..3853b45341c7 100644 --- a/drivers/gpu/drm/drm_legacy.h +++ b/drivers/gpu/drm/drm_legacy.h @@ -36,6 +36,7 @@
struct agp_memory; struct drm_device; +struct drm_dma_handle; struct drm_file; struct drm_buf_desc;
@@ -211,4 +212,26 @@ void drm_master_legacy_init(struct drm_master *master); static inline void drm_master_legacy_init(struct drm_master *master) {} #endif
+#if IS_ENABLED(CONFIG_DRM_LEGACY) && IS_ENABLED(CONFIG_PCI)
+struct drm_dma_handle * +drm_legacy_pci_alloc(struct drm_device *dev, size_t size, size_t align); +void drm_legacy_pci_free(struct drm_device *dev, struct drm_dma_handle * dmah);
+#else
+static inline struct drm_dma_handle * +drm_legacy_pci_alloc(struct drm_device *dev, size_t size, size_t align) +{
- return NULL;
+}
+static inline void drm_legacy_pci_free(struct drm_device *dev,
struct drm_dma_handle *dmah)+{ +}
+#endif
#endif /* __DRM_LEGACY_H__ */ diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index c6bb98729a26..12239498538c 100644 --- a/drivers/gpu/drm/drm_pci.c +++ b/drivers/gpu/drm/drm_pci.c @@ -36,19 +36,10 @@ #include "drm_internal.h" #include "drm_legacy.h"
-/**
- drm_pci_alloc - Allocate a PCI consistent memory block, for DMA.
- @dev: DRM device
- @size: size of block to allocate
- @align: alignment of block
- FIXME: This is a needless abstraction of the Linux dma-api and should be
- removed.
- Return: A handle to the allocated memory block on success or NULL on
- failure.
- */
-drm_dma_handle_t *drm_pci_alloc(struct drm_device * dev, size_t size, size_t align) +#if IS_ENABLED(CONFIG_DRM_LEGACY) && IS_ENABLED(CONFIG_PCI)
+drm_dma_handle_t * +drm_legacy_pci_alloc(struct drm_device * dev, size_t size, size_t align) { drm_dma_handle_t *dmah;
@@ -76,24 +67,14 @@ drm_dma_handle_t *drm_pci_alloc(struct drm_device * dev, size_t size, size_t ali return dmah; }
-EXPORT_SYMBOL(drm_pci_alloc);
-/**
- drm_pci_free - Free a PCI consistent memory block
- @dev: DRM device
- @dmah: handle to memory block
- FIXME: This is a needless abstraction of the Linux dma-api and should be
- removed.
- */
-void drm_pci_free(struct drm_device * dev, drm_dma_handle_t * dmah) +void drm_legacy_pci_free(struct drm_device * dev, drm_dma_handle_t * dmah) { dma_free_coherent(&dev->pdev->dev, dmah->size, dmah->vaddr, dmah->busaddr); kfree(dmah); }
-EXPORT_SYMBOL(drm_pci_free); +#endif
static int drm_get_pci_domain(struct drm_device *dev) { diff --git a/include/drm/drm_pci.h b/include/drm/drm_pci.h index 9031e217b506..cade5b60b643 100644 --- a/include/drm/drm_pci.h +++ b/include/drm/drm_pci.h @@ -34,34 +34,16 @@
#include <linux/pci.h>
-struct drm_dma_handle; -struct drm_device; struct drm_driver; -struct drm_master;
#ifdef CONFIG_PCI
-struct drm_dma_handle *drm_pci_alloc(struct drm_device *dev, size_t size,
size_t align);-void drm_pci_free(struct drm_device *dev, struct drm_dma_handle * dmah);
int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent, struct drm_driver *driver);
#else
-static inline struct drm_dma_handle *drm_pci_alloc(struct drm_device *dev,
size_t size, size_t align)-{
- return NULL;
-}
-static inline void drm_pci_free(struct drm_device *dev,
struct drm_dma_handle *dmah)-{ -}
static inline int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent, struct drm_driver *driver) -- 2.25.0
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Hi Chris.
On Sun, Feb 02, 2020 at 05:16:31PM +0000, Chris Wilson wrote:
drm_pci_alloc/drm_pci_free are very thin wrappers around the core dma facilities, and we have no special reason within the drm layer to behave differently. In particular, since
commit de09d31dd38a50fdce106c15abd68432eebbd014 Author: Kirill A. Shutemov kirill.shutemov@linux.intel.com Date: Fri Jan 15 16:51:42 2016 -0800
page-flags: define PG_reserved behavior on compound pages As far as I can see there's no users of PG_reserved on compound pages. Let's use PF_NO_COMPOUND here.it has been illegal to combine GFP_COMP with SetPageReserved, so lets stop doing both and leave the dma layer to its own devices.
Reported-by: Taketo Kabe Closes: https://gitlab.freedesktop.org/drm/intel/issues/1027 Fixes: de09d31dd38a ("page-flags: define PG_reserved behavior on compound pages") Signed-off-by: Chris Wilson chris@chris-wilson.co.uk Cc: stable@vger.kernel.org # v4.5+
Out of curiosity I added the full series and tried to build it. I did not really take the time to review the patches.
It failed with alpha allmodconfig like this: (On top of drm-misc-next as of yesterday)
/home/sam/drm/linux.git/drivers/gpu/drm/r128/ati_pcigart.c: In function ‘drm_ati_alloc_pcigart_table’: /home/sam/drm/linux.git/drivers/gpu/drm/r128/ati_pcigart.c:50:7: error: expected expression before ‘^’ token 50 | ^gart_info->bus_addr, | ^ /home/sam/drm/linux.git/drivers/gpu/drm/r128/ati_pcigart.c:48:3: error: too few arguments to function ‘dma_alloc_coherent’ 48 | dma_alloc_coherent(&dev->pdev->dev, | ^~~~~~~~~~~~~~~~~~ In file included from /home/sam/drm/linux.git/arch/alpha/include/asm/pci.h:8, from /home/sam/drm/linux.git/include/linux/pci.h:1777, from /home/sam/drm/linux.git/include/drm/drm_pci.h:35, from /home/sam/drm/linux.git/drivers/gpu/drm/r128/ati_pcigart.c:37: /home/sam/drm/linux.git/include/linux/dma-mapping.h:641:21: note: declared here 641 | static inline void *dma_alloc_coherent(struct device *dev, size_t size, | ^~~~~~~~~~~~~~~~~~ /home/sam/drm/linux.git/drivers/gpu/drm/r128/ati_pcigart.c: At top level: /home/sam/drm/linux.git/drivers/gpu/drm/r128/ati_pcigart.c:101:2: error: expected identifier or ‘(’ before ‘return’ 101 | return 1; | ^~~~~~ /home/sam/drm/linux.git/drivers/gpu/drm/r128/ati_pcigart.c:102:1: error: expected identifier or ‘(’ before ‘}’ token 102 | } | ^ /home/sam/drm/linux.git/drivers/gpu/drm/r128/ati_pcigart.c: In function ‘drm_ati_pcigart_init’: /home/sam/drm/linux.git/drivers/gpu/drm/r128/ati_pcigart.c:168:13: warning: assignment to ‘u32 *’ {aka ‘unsigned int *’} from ‘unsigned int’ makes pointer from integer without a cast [-Wint-conversion] 168 | page_base = (u32) entry->busaddr[i]; | ^ /home/sam/drm/linux.git/drivers/gpu/drm/r128/ati_pcigart.c:176:21: error: invalid operands to binary | (have ‘u32 *’ {aka ‘unsigned int *’} and ‘int’) 176 | val = page_base | 0xc; | ^ /home/sam/drm/linux.git/drivers/gpu/drm/r128/ati_pcigart.c:179:22: error: invalid operands to binary >> (have ‘u32 *’ {aka ‘unsigned int *’} and ‘int’) 179 | val = (page_base >> 8) | 0xc; | ^~ /home/sam/drm/linux.git/drivers/gpu/drm/r128/ati_pcigart.c:183:9: warning: assignment to ‘u32’ {aka ‘unsigned int’} from ‘u32 *’ {aka ‘unsigned int *’} makes integer from pointer without a cast [-Wint-conversion] 183 | val = page_base; | ^ /home/sam/drm/linux.git/drivers/gpu/drm/r128/ati_pcigart.c:188:12: warning: dereferencing ‘void *’ pointer 188 | address[gart_idx] = cpu_to_le32(val);
I did not try other architectures and did not try to fix it.
When I applied the patches checkpatch was no too happy:
Applying: drm: Remove PageReserved manipulation from drm_pci_alloc [sam-pci fa4a1146af59] drm: Remove PageReserved manipulation from drm_pci_alloc Author: Chris Wilson chris@chris-wilson.co.uk Date: Sun Feb 2 17:16:31 2020 +0000 1 file changed, 2 insertions(+), 21 deletions(-) fa4a1146af59 (HEAD -> sam-pci) drm: Remove PageReserved manipulation from drm_pci_alloc -:10: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit de09d31dd38a ("page-flags: define PG_reserved behavior on compound pages")' #10: commit de09d31dd38a50fdce106c15abd68432eebbd014
-:22: ERROR:BAD_SIGN_OFF: Unrecognized email address: 'Taketo Kabe' #22: Reported-by: Taketo Kabe
[sam-pci b553cf48b548] drm/r128: Wean off drm_pci_alloc Author: Chris Wilson chris@chris-wilson.co.uk Date: Sun Feb 2 17:16:33 2020 +0000 2 files changed, 17 insertions(+), 17 deletions(-) b553cf48b548 (HEAD -> sam-pci) drm/r128: Wean off drm_pci_alloc -:13: WARNING:OBSOLETE: drivers/gpu/drm/r128/ati_pcigart.c is marked as 'obsolete' in the MAINTAINERS hierarchy. No unnecessary modifications please.
-:16: WARNING:OBSOLETE: drivers/gpu/drm/r128/ati_pcigart.c is marked as 'obsolete' in the MAINTAINERS hierarchy. No unnecessary modifications please.
-:26: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis #26: FILE: drivers/gpu/drm/r128/ati_pcigart.c:49: + dma_alloc_coherent(&dev->pdev->dev, + gart_info->table_size,
-:27: CHECK:SPACING: spaces preferred around that '^' (ctx:ExV) #27: FILE: drivers/gpu/drm/r128/ati_pcigart.c:50: + ^gart_info->bus_addr, ^
[sam-pci 7e77c3ee282d] drm: Remove exports for drm_pci_alloc/drm_pci_free Author: Chris Wilson chris@chris-wilson.co.uk Date: Sun Feb 2 17:16:35 2020 +0000 4 files changed, 32 insertions(+), 45 deletions(-) 7e77c3ee282d (HEAD -> sam-pci) drm: Remove exports for drm_pci_alloc/drm_pci_free -:53: CHECK:LINE_SPACING: Please don't use multiple blank lines #53: FILE: drivers/gpu/drm/drm_legacy.h:215:
+
-:58: ERROR:POINTER_LOCATION: "foo * bar" should be "foo *bar" #58: FILE: drivers/gpu/drm/drm_legacy.h:220: +void drm_legacy_pci_free(struct drm_device *dev, struct drm_dma_handle * dmah);
-:100: ERROR:POINTER_LOCATION: "foo * bar" should be "foo *bar" #100: FILE: drivers/gpu/drm/drm_pci.c:42: +drm_legacy_pci_alloc(struct drm_device * dev, size_t size, size_t align)
-:119: ERROR:POINTER_LOCATION: "foo * bar" should be "foo *bar" #119: FILE: drivers/gpu/drm/drm_pci.c:70: +void drm_legacy_pci_free(struct drm_device * dev, drm_dma_handle_t * dmah)
-:119: ERROR:POINTER_LOCATION: "foo * bar" should be "foo *bar" #119: FILE: drivers/gpu/drm/drm_pci.c:70: +void drm_legacy_pci_free(struct drm_device * dev, drm_dma_handle_t * dmah)
Sam
On Sun, Feb 2, 2020 at 12:16 PM Chris Wilson chris@chris-wilson.co.uk wrote:
drm_pci_alloc/drm_pci_free are very thin wrappers around the core dma facilities, and we have no special reason within the drm layer to behave differently. In particular, since
commit de09d31dd38a50fdce106c15abd68432eebbd014 Author: Kirill A. Shutemov kirill.shutemov@linux.intel.com Date: Fri Jan 15 16:51:42 2016 -0800
page-flags: define PG_reserved behavior on compound pages As far as I can see there's no users of PG_reserved on compound pages. Let's use PF_NO_COMPOUND here.it has been illegal to combine GFP_COMP with SetPageReserved, so lets stop doing both and leave the dma layer to its own devices.
Reported-by: Taketo Kabe
Needs an email address.
Closes: https://gitlab.freedesktop.org/drm/intel/issues/1027
Should be Bug: rather than Closes:
Fixes: de09d31dd38a ("page-flags: define PG_reserved behavior on compound pages") Signed-off-by: Chris Wilson chris@chris-wilson.co.uk Cc: stable@vger.kernel.org # v4.5+
With those fixed: Reviewed-by: Alex Deucher alexander.deucher@amd.com
drivers/gpu/drm/drm_pci.c | 23 ++--------------------- 1 file changed, 2 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index f2e43d341980..d16dac4325f9 100644 --- a/drivers/gpu/drm/drm_pci.c +++ b/drivers/gpu/drm/drm_pci.c @@ -51,8 +51,6 @@ drm_dma_handle_t *drm_pci_alloc(struct drm_device * dev, size_t size, size_t align) { drm_dma_handle_t *dmah;
unsigned long addr;size_t sz; /* pci_alloc_consistent only guarantees alignment to the smallest * PAGE_SIZE order which is greater than or equal to the requested size.@@ -68,20 +66,13 @@ drm_dma_handle_t *drm_pci_alloc(struct drm_device * dev, size_t size, size_t ali dmah->size = size; dmah->vaddr = dma_alloc_coherent(&dev->pdev->dev, size, &dmah->busaddr,
GFP_KERNEL | __GFP_COMP);
GFP_KERNEL); if (dmah->vaddr == NULL) { kfree(dmah); return NULL; }
/* XXX - Is virt_to_page() legal for consistent mem? *//* Reserve */for (addr = (unsigned long)dmah->vaddr, sz = size;sz > 0; addr += PAGE_SIZE, sz -= PAGE_SIZE) {SetPageReserved(virt_to_page((void *)addr));}return dmah;}
@@ -94,19 +85,9 @@ EXPORT_SYMBOL(drm_pci_alloc); */ void __drm_legacy_pci_free(struct drm_device * dev, drm_dma_handle_t * dmah) {
unsigned long addr;size_t sz;if (dmah->vaddr) {/* XXX - Is virt_to_page() legal for consistent mem? *//* Unreserve */for (addr = (unsigned long)dmah->vaddr, sz = dmah->size;sz > 0; addr += PAGE_SIZE, sz -= PAGE_SIZE) {ClearPageReserved(virt_to_page((void *)addr));}
if (dmah->vaddr) dma_free_coherent(&dev->pdev->dev, dmah->size, dmah->vaddr, dmah->busaddr);
}}
/**
2.25.0
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Quoting Alex Deucher (2020-02-03 21:49:48)
On Sun, Feb 2, 2020 at 12:16 PM Chris Wilson chris@chris-wilson.co.uk wrote:
drm_pci_alloc/drm_pci_free are very thin wrappers around the core dma facilities, and we have no special reason within the drm layer to behave differently. In particular, since
commit de09d31dd38a50fdce106c15abd68432eebbd014 Author: Kirill A. Shutemov kirill.shutemov@linux.intel.com Date: Fri Jan 15 16:51:42 2016 -0800
page-flags: define PG_reserved behavior on compound pages As far as I can see there's no users of PG_reserved on compound pages. Let's use PF_NO_COMPOUND here.it has been illegal to combine GFP_COMP with SetPageReserved, so lets stop doing both and leave the dma layer to its own devices.
Reported-by: Taketo Kabe
Needs an email address.
None provided, I don't insist that they opt in to potential spam harvesting.
Closes: https://gitlab.freedesktop.org/drm/intel/issues/1027
Should be Bug: rather than Closes:
We're using Closes for gitlab, since we hope to integrate with gitlab someday. (Or at least some integrated bug/source management, of which gitlab is the current forerunner.) -Chris
dri-devel@lists.freedesktop.org
-
Alex Deucher -
Chris Wilson -
Daniel Vetter -
kbuild test robot -
Sam Ravnborg -
Sasha Levin