* [PATCH 1/2] drm: remove usage of drm_pci_alloc/free
@ 2021-04-23 2:02 Joseph Kogut
2021-04-23 2:02 ` [PATCH 2/2] drm: remove legacy drm_pci_alloc/free abstraction Joseph Kogut
2021-04-27 7:17 ` [PATCH 1/2] drm: remove usage of drm_pci_alloc/free Daniel Vetter
0 siblings, 2 replies; 3+ messages in thread
From: Joseph Kogut @ 2021-04-23 2:02 UTC (permalink / raw)
To: dri-devel
Cc: Joseph Kogut, Thomas Zimmermann, David Airlie, Lee Jones,
linux-kernel, Sam Ravnborg
Remove usage of legacy dma-api abstraction in preparation for removal
Signed-off-by: Joseph Kogut <joseph.kogut@gmail.com>
---
Checkpatch warns here that r128 is marked obsolete, and asks for no
unnecessary modifications.
This series aims to address the FIXME in drivers/gpu/drm/drm_pci.c
explaining that drm_pci_alloc/free is a needless abstraction of the
dma-api, and it should be removed. Unfortunately, doing this requires
removing the usage from an obsolete driver as well.
If this patch is rejected for modifying an obsolete driver, would it be
appropriate to follow up removing the FIXME from drm_pci?
drivers/gpu/drm/drm_bufs.c | 19 ++++++++++++++++---
drivers/gpu/drm/drm_dma.c | 8 +++++++-
drivers/gpu/drm/r128/ati_pcigart.c | 22 ++++++++++++++++++----
3 files changed, 41 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c
index e3d77dfefb0a..94bc1f6049c9 100644
--- a/drivers/gpu/drm/drm_bufs.c
+++ b/drivers/gpu/drm/drm_bufs.c
@@ -674,12 +674,17 @@ int drm_legacy_rmmap_ioctl(struct drm_device *dev, void *data,
static void drm_cleanup_buf_error(struct drm_device *dev,
struct drm_buf_entry *entry)
{
+ drm_dma_handle_t *dmah;
int i;
if (entry->seg_count) {
for (i = 0; i < entry->seg_count; i++) {
if (entry->seglist[i]) {
- drm_pci_free(dev, entry->seglist[i]);
+ dmah = entry->seglist[i];
+ dma_free_coherent(dev->dev,
+ dmah->size,
+ dmah->vaddr,
+ dmah->busaddr);
}
}
kfree(entry->seglist);
@@ -978,10 +983,18 @@ int drm_legacy_addbufs_pci(struct drm_device *dev,
page_count = 0;
while (entry->buf_count < count) {
+ dmah = kmalloc(sizeof(drm_dma_handle_t), GFP_KERNEL);
+ if (!dmah)
+ return -ENOMEM;
- dmah = drm_pci_alloc(dev, PAGE_SIZE << page_order, 0x1000);
+ dmah->size = total;
+ dmah->vaddr = dma_alloc_coherent(dev->dev,
+ dmah->size,
+ &dmah->busaddr,
+ GFP_KERNEL);
+ if (!dmah->vaddr) {
+ kfree(dmah);
- if (!dmah) {
/* Set count correctly so we free the proper amount. */
entry->buf_count = count;
entry->seg_count = count;
diff --git a/drivers/gpu/drm/drm_dma.c b/drivers/gpu/drm/drm_dma.c
index d07ba54ec945..eb6b741a6f99 100644
--- a/drivers/gpu/drm/drm_dma.c
+++ b/drivers/gpu/drm/drm_dma.c
@@ -81,6 +81,7 @@ int drm_legacy_dma_setup(struct drm_device *dev)
void drm_legacy_dma_takedown(struct drm_device *dev)
{
struct drm_device_dma *dma = dev->dma;
+ drm_dma_handle_t *dmah;
int i, j;
if (!drm_core_check_feature(dev, DRIVER_HAVE_DMA) ||
@@ -100,7 +101,12 @@ void drm_legacy_dma_takedown(struct drm_device *dev)
dma->bufs[i].seg_count);
for (j = 0; j < dma->bufs[i].seg_count; j++) {
if (dma->bufs[i].seglist[j]) {
- drm_pci_free(dev, dma->bufs[i].seglist[j]);
+ dmah = dma->bufs[i].seglist[j];
+ dma_free_coherent(dev->dev,
+ dmah->size,
+ dmah->vaddr,
+ dmah->busaddr);
+ kfree(dmah);
}
}
kfree(dma->bufs[i].seglist);
diff --git a/drivers/gpu/drm/r128/ati_pcigart.c b/drivers/gpu/drm/r128/ati_pcigart.c
index 1234ec60c0af..fbb0cfd79758 100644
--- a/drivers/gpu/drm/r128/ati_pcigart.c
+++ b/drivers/gpu/drm/r128/ati_pcigart.c
@@ -45,18 +45,32 @@
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)
+ drm_dma_handle_t *dmah = kmalloc(sizeof(drm_dma_handle_t), GFP_KERNEL);
+
+ if (!dmah)
+ return -ENOMEM;
+
+ dmah->size = gart_info->table_size;
+ dmah->vaddr = dma_alloc_coherent(dev->dev,
+ dmah->size,
+ &dmah->busaddr,
+ GFP_KERNEL);
+
+ if (!dmah->vaddr) {
+ kfree(dmah);
return -ENOMEM;
+ }
+ gart_info->table_handle = dmah;
return 0;
}
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);
+ drm_dma_handle_t *dmah = gart_info->table_handle;
+
+ dma_free_coherent(dev->dev, dmah->size, dmah->vaddr, dmah->busaddr);
gart_info->table_handle = NULL;
}
--
2.31.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 3+ messages in thread* [PATCH 2/2] drm: remove legacy drm_pci_alloc/free abstraction
2021-04-23 2:02 [PATCH 1/2] drm: remove usage of drm_pci_alloc/free Joseph Kogut
@ 2021-04-23 2:02 ` Joseph Kogut
2021-04-27 7:17 ` [PATCH 1/2] drm: remove usage of drm_pci_alloc/free Daniel Vetter
1 sibling, 0 replies; 3+ messages in thread
From: Joseph Kogut @ 2021-04-23 2:02 UTC (permalink / raw)
To: dri-devel; +Cc: Joseph Kogut, Thomas Zimmermann, David Airlie, linux-kernel
The drm_pci_alloc/free abstraction of the dma-api is no longer required,
remove it.
Signed-off-by: Joseph Kogut <joseph.kogut@gmail.com>
---
drivers/gpu/drm/drm_pci.c | 58 ---------------------------------------
include/drm/drm_legacy.h | 4 ---
2 files changed, 62 deletions(-)
diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
index 2294a1580d35..1a1a2d4046e9 100644
--- a/drivers/gpu/drm/drm_pci.c
+++ b/drivers/gpu/drm/drm_pci.c
@@ -41,64 +41,6 @@
/* List of devices hanging off drivers with stealth attach. */
static LIST_HEAD(legacy_dev_list);
static DEFINE_MUTEX(legacy_dev_list_lock);
-
-/**
- * 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)
-{
- drm_dma_handle_t *dmah;
-
- /* pci_alloc_consistent only guarantees alignment to the smallest
- * PAGE_SIZE order which is greater than or equal to the requested size.
- * Return NULL here for now to make sure nobody tries for larger alignment
- */
- if (align > size)
- return NULL;
-
- dmah = kmalloc(sizeof(drm_dma_handle_t), GFP_KERNEL);
- if (!dmah)
- return NULL;
-
- dmah->size = size;
- dmah->vaddr = dma_alloc_coherent(dev->dev, size,
- &dmah->busaddr,
- GFP_KERNEL);
-
- if (dmah->vaddr == NULL) {
- kfree(dmah);
- return NULL;
- }
-
- 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)
-{
- dma_free_coherent(dev->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_legacy.h b/include/drm/drm_legacy.h
index 8ed04e9be997..faf64319be76 100644
--- a/include/drm/drm_legacy.h
+++ b/include/drm/drm_legacy.h
@@ -194,10 +194,6 @@ void drm_legacy_idlelock_release(struct drm_lock_data *lock);
#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_legacy_pci_init(const struct drm_driver *driver,
struct pci_driver *pdriver);
void drm_legacy_pci_exit(const struct drm_driver *driver,
--
2.31.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH 1/2] drm: remove usage of drm_pci_alloc/free
2021-04-23 2:02 [PATCH 1/2] drm: remove usage of drm_pci_alloc/free Joseph Kogut
2021-04-23 2:02 ` [PATCH 2/2] drm: remove legacy drm_pci_alloc/free abstraction Joseph Kogut
@ 2021-04-27 7:17 ` Daniel Vetter
1 sibling, 0 replies; 3+ messages in thread
From: Daniel Vetter @ 2021-04-27 7:17 UTC (permalink / raw)
To: Joseph Kogut
Cc: David Airlie, Lee Jones, linux-kernel, dri-devel,
Thomas Zimmermann, Sam Ravnborg
On Thu, Apr 22, 2021 at 07:02:43PM -0700, Joseph Kogut wrote:
> Remove usage of legacy dma-api abstraction in preparation for removal
>
> Signed-off-by: Joseph Kogut <joseph.kogut@gmail.com>
> ---
> Checkpatch warns here that r128 is marked obsolete, and asks for no
> unnecessary modifications.
>
> This series aims to address the FIXME in drivers/gpu/drm/drm_pci.c
> explaining that drm_pci_alloc/free is a needless abstraction of the
> dma-api, and it should be removed. Unfortunately, doing this requires
> removing the usage from an obsolete driver as well.
>
> If this patch is rejected for modifying an obsolete driver, would it be
> appropriate to follow up removing the FIXME from drm_pci?
Feels like a good enough reason, both patches queued up in drm-misc-next
for 5.14. Thanks a lot for doing them.
-Daniel
>
> drivers/gpu/drm/drm_bufs.c | 19 ++++++++++++++++---
> drivers/gpu/drm/drm_dma.c | 8 +++++++-
> drivers/gpu/drm/r128/ati_pcigart.c | 22 ++++++++++++++++++----
> 3 files changed, 41 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c
> index e3d77dfefb0a..94bc1f6049c9 100644
> --- a/drivers/gpu/drm/drm_bufs.c
> +++ b/drivers/gpu/drm/drm_bufs.c
> @@ -674,12 +674,17 @@ int drm_legacy_rmmap_ioctl(struct drm_device *dev, void *data,
> static void drm_cleanup_buf_error(struct drm_device *dev,
> struct drm_buf_entry *entry)
> {
> + drm_dma_handle_t *dmah;
> int i;
>
> if (entry->seg_count) {
> for (i = 0; i < entry->seg_count; i++) {
> if (entry->seglist[i]) {
> - drm_pci_free(dev, entry->seglist[i]);
> + dmah = entry->seglist[i];
> + dma_free_coherent(dev->dev,
> + dmah->size,
> + dmah->vaddr,
> + dmah->busaddr);
> }
> }
> kfree(entry->seglist);
> @@ -978,10 +983,18 @@ int drm_legacy_addbufs_pci(struct drm_device *dev,
> page_count = 0;
>
> while (entry->buf_count < count) {
> + dmah = kmalloc(sizeof(drm_dma_handle_t), GFP_KERNEL);
> + if (!dmah)
> + return -ENOMEM;
>
> - dmah = drm_pci_alloc(dev, PAGE_SIZE << page_order, 0x1000);
> + dmah->size = total;
> + dmah->vaddr = dma_alloc_coherent(dev->dev,
> + dmah->size,
> + &dmah->busaddr,
> + GFP_KERNEL);
> + if (!dmah->vaddr) {
> + kfree(dmah);
>
> - if (!dmah) {
> /* Set count correctly so we free the proper amount. */
> entry->buf_count = count;
> entry->seg_count = count;
> diff --git a/drivers/gpu/drm/drm_dma.c b/drivers/gpu/drm/drm_dma.c
> index d07ba54ec945..eb6b741a6f99 100644
> --- a/drivers/gpu/drm/drm_dma.c
> +++ b/drivers/gpu/drm/drm_dma.c
> @@ -81,6 +81,7 @@ int drm_legacy_dma_setup(struct drm_device *dev)
> void drm_legacy_dma_takedown(struct drm_device *dev)
> {
> struct drm_device_dma *dma = dev->dma;
> + drm_dma_handle_t *dmah;
> int i, j;
>
> if (!drm_core_check_feature(dev, DRIVER_HAVE_DMA) ||
> @@ -100,7 +101,12 @@ void drm_legacy_dma_takedown(struct drm_device *dev)
> dma->bufs[i].seg_count);
> for (j = 0; j < dma->bufs[i].seg_count; j++) {
> if (dma->bufs[i].seglist[j]) {
> - drm_pci_free(dev, dma->bufs[i].seglist[j]);
> + dmah = dma->bufs[i].seglist[j];
> + dma_free_coherent(dev->dev,
> + dmah->size,
> + dmah->vaddr,
> + dmah->busaddr);
> + kfree(dmah);
> }
> }
> kfree(dma->bufs[i].seglist);
> diff --git a/drivers/gpu/drm/r128/ati_pcigart.c b/drivers/gpu/drm/r128/ati_pcigart.c
> index 1234ec60c0af..fbb0cfd79758 100644
> --- a/drivers/gpu/drm/r128/ati_pcigart.c
> +++ b/drivers/gpu/drm/r128/ati_pcigart.c
> @@ -45,18 +45,32 @@
> 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)
> + drm_dma_handle_t *dmah = kmalloc(sizeof(drm_dma_handle_t), GFP_KERNEL);
> +
> + if (!dmah)
> + return -ENOMEM;
> +
> + dmah->size = gart_info->table_size;
> + dmah->vaddr = dma_alloc_coherent(dev->dev,
> + dmah->size,
> + &dmah->busaddr,
> + GFP_KERNEL);
> +
> + if (!dmah->vaddr) {
> + kfree(dmah);
> return -ENOMEM;
> + }
>
> + gart_info->table_handle = dmah;
> return 0;
> }
>
> 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);
> + drm_dma_handle_t *dmah = gart_info->table_handle;
> +
> + dma_free_coherent(dev->dev, dmah->size, dmah->vaddr, dmah->busaddr);
> gart_info->table_handle = NULL;
> }
>
> --
> 2.31.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-04-27 7:18 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-23 2:02 [PATCH 1/2] drm: remove usage of drm_pci_alloc/free Joseph Kogut
2021-04-23 2:02 ` [PATCH 2/2] drm: remove legacy drm_pci_alloc/free abstraction Joseph Kogut
2021-04-27 7:17 ` [PATCH 1/2] drm: remove usage of drm_pci_alloc/free Daniel Vetter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).