diff mbox series

[07/14] drm/mgag200: Switch to managed MM

Message ID 20200605135803.19811-8-tzimmermann@suse.de (mailing list archive)
State New, archived
Headers show
Series drm/mgag200: Use managed interfaces for auto-cleanup | expand

Commit Message

Thomas Zimmermann June 5, 2020, 1:57 p.m. UTC 
The memory-management code now cleans up automatically as part of
device destruction.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/mgag200/mgag200_drv.h  |  1 -
 drivers/gpu/drm/mgag200/mgag200_main.c |  5 +----
 drivers/gpu/drm/mgag200/mgag200_mm.c   | 28 ++++++++++++++------------
 3 files changed, 16 insertions(+), 18 deletions(-)

Comments

Daniel Vetter June 5, 2020, 4:22 p.m. UTC | #1 
On Fri, Jun 05, 2020 at 03:57:56PM +0200, Thomas Zimmermann wrote:
> The memory-management code now cleans up automatically as part of
> device destruction.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/gpu/drm/mgag200/mgag200_drv.h  |  1 -
>  drivers/gpu/drm/mgag200/mgag200_main.c |  5 +----
>  drivers/gpu/drm/mgag200/mgag200_mm.c   | 28 ++++++++++++++------------
>  3 files changed, 16 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h
> index cd786ffe319b8..7b6e6827a9a21 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_drv.h
> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
> @@ -198,6 +198,5 @@ void mgag200_i2c_destroy(struct mga_i2c_chan *i2c);
>  
>  				/* mgag200_mm.c */
>  int mgag200_mm_init(struct mga_device *mdev);
> -void mgag200_mm_fini(struct mga_device *mdev);
>  
>  #endif				/* __MGAG200_DRV_H__ */
> diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c b/drivers/gpu/drm/mgag200/mgag200_main.c
> index e9ad783c2b44d..49bcdfcb40a4e 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_main.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_main.c
> @@ -57,13 +57,11 @@ int mgag200_driver_load(struct drm_device *dev, unsigned long flags)
>  	ret = mgag200_modeset_init(mdev);
>  	if (ret) {
>  		drm_err(dev, "Fatal error during modeset init: %d\n", ret);
> -		goto err_mgag200_mm_fini;
> +		goto err_mm;
>  	}
>  
>  	return 0;
>  
> -err_mgag200_mm_fini:
> -	mgag200_mm_fini(mdev);
>  err_mm:
>  	dev->dev_private = NULL;
>  	return ret;
> @@ -75,6 +73,5 @@ void mgag200_driver_unload(struct drm_device *dev)
>  
>  	if (mdev == NULL)
>  		return;
> -	mgag200_mm_fini(mdev);
>  	dev->dev_private = NULL;
>  }
> diff --git a/drivers/gpu/drm/mgag200/mgag200_mm.c b/drivers/gpu/drm/mgag200/mgag200_mm.c
> index f56b0456994f4..1b1e5ec5d1ceb 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_mm.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_mm.c
> @@ -28,6 +28,8 @@
>  
>  #include <linux/pci.h>
>  
> +#include <drm/drm_managed.h>
> +
>  #include "mgag200_drv.h"
>  
>  static size_t mgag200_probe_vram(struct mga_device *mdev, void __iomem *mem,
> @@ -73,6 +75,18 @@ static size_t mgag200_probe_vram(struct mga_device *mdev, void __iomem *mem,
>  	return offset - 65536;
>  }
>  
> +static void mgag200_mm_release(struct drm_device *dev, void *ptr)
> +{
> +	struct mga_device *mdev = to_mga_device(dev);
> +
> +	mdev->vram_fb_available = 0;
> +	iounmap(mdev->vram);
> +	arch_io_free_memtype_wc(pci_resource_start(dev->pdev, 0),
> +				pci_resource_len(dev->pdev, 0));
> +	arch_phys_wc_del(mdev->fb_mtrr);
> +	mdev->fb_mtrr = 0;
> +}
> +
>  int mgag200_mm_init(struct mga_device *mdev)
>  {
>  	struct drm_device *dev = mdev->dev;
> @@ -104,22 +118,10 @@ int mgag200_mm_init(struct mga_device *mdev)
>  
>  	mdev->vram_fb_available = mdev->mc.vram_size;
>  
> -	return 0;
> +	return drmm_add_action_or_reset(dev, mgag200_mm_release, NULL);
>  
>  err_arch_phys_wc_del:
>  	arch_phys_wc_del(mdev->fb_mtrr);
>  	arch_io_free_memtype_wc(start, len);

Btw I think devm versions of these two would benefit a bunch of drivers in
their cleanup code. devm_ not drmm_ since it's hw resource cleanup. In
case you ever run out of ideas :-)

Cheeres, Daniel

>  	return ret;
>  }
> -
> -void mgag200_mm_fini(struct mga_device *mdev)
> -{
> -	struct drm_device *dev = mdev->dev;
> -
> -	mdev->vram_fb_available = 0;
> -	iounmap(mdev->vram);
> -	arch_io_free_memtype_wc(pci_resource_start(dev->pdev, 0),
> -				pci_resource_len(dev->pdev, 0));
> -	arch_phys_wc_del(mdev->fb_mtrr);
> -	mdev->fb_mtrr = 0;
> -}
> -- 
> 2.26.2
>
Thomas Zimmermann June 8, 2020, 12:57 p.m. UTC | #2 
Hi

Am 05.06.20 um 18:22 schrieb Daniel Vetter:
> On Fri, Jun 05, 2020 at 03:57:56PM +0200, Thomas Zimmermann wrote:
>> The memory-management code now cleans up automatically as part of
>> device destruction.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>  drivers/gpu/drm/mgag200/mgag200_drv.h  |  1 -
>>  drivers/gpu/drm/mgag200/mgag200_main.c |  5 +----
>>  drivers/gpu/drm/mgag200/mgag200_mm.c   | 28 ++++++++++++++------------
>>  3 files changed, 16 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h
>> index cd786ffe319b8..7b6e6827a9a21 100644
>> --- a/drivers/gpu/drm/mgag200/mgag200_drv.h
>> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
>> @@ -198,6 +198,5 @@ void mgag200_i2c_destroy(struct mga_i2c_chan *i2c);
>>  
>>  				/* mgag200_mm.c */
>>  int mgag200_mm_init(struct mga_device *mdev);
>> -void mgag200_mm_fini(struct mga_device *mdev);
>>  
>>  #endif				/* __MGAG200_DRV_H__ */
>> diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c b/drivers/gpu/drm/mgag200/mgag200_main.c
>> index e9ad783c2b44d..49bcdfcb40a4e 100644
>> --- a/drivers/gpu/drm/mgag200/mgag200_main.c
>> +++ b/drivers/gpu/drm/mgag200/mgag200_main.c
>> @@ -57,13 +57,11 @@ int mgag200_driver_load(struct drm_device *dev, unsigned long flags)
>>  	ret = mgag200_modeset_init(mdev);
>>  	if (ret) {
>>  		drm_err(dev, "Fatal error during modeset init: %d\n", ret);
>> -		goto err_mgag200_mm_fini;
>> +		goto err_mm;
>>  	}
>>  
>>  	return 0;
>>  
>> -err_mgag200_mm_fini:
>> -	mgag200_mm_fini(mdev);
>>  err_mm:
>>  	dev->dev_private = NULL;
>>  	return ret;
>> @@ -75,6 +73,5 @@ void mgag200_driver_unload(struct drm_device *dev)
>>  
>>  	if (mdev == NULL)
>>  		return;
>> -	mgag200_mm_fini(mdev);
>>  	dev->dev_private = NULL;
>>  }
>> diff --git a/drivers/gpu/drm/mgag200/mgag200_mm.c b/drivers/gpu/drm/mgag200/mgag200_mm.c
>> index f56b0456994f4..1b1e5ec5d1ceb 100644
>> --- a/drivers/gpu/drm/mgag200/mgag200_mm.c
>> +++ b/drivers/gpu/drm/mgag200/mgag200_mm.c
>> @@ -28,6 +28,8 @@
>>  
>>  #include <linux/pci.h>
>>  
>> +#include <drm/drm_managed.h>
>> +
>>  #include "mgag200_drv.h"
>>  
>>  static size_t mgag200_probe_vram(struct mga_device *mdev, void __iomem *mem,
>> @@ -73,6 +75,18 @@ static size_t mgag200_probe_vram(struct mga_device *mdev, void __iomem *mem,
>>  	return offset - 65536;
>>  }
>>  
>> +static void mgag200_mm_release(struct drm_device *dev, void *ptr)
>> +{
>> +	struct mga_device *mdev = to_mga_device(dev);
>> +
>> +	mdev->vram_fb_available = 0;
>> +	iounmap(mdev->vram);
>> +	arch_io_free_memtype_wc(pci_resource_start(dev->pdev, 0),
>> +				pci_resource_len(dev->pdev, 0));
>> +	arch_phys_wc_del(mdev->fb_mtrr);
>> +	mdev->fb_mtrr = 0;
>> +}
>> +
>>  int mgag200_mm_init(struct mga_device *mdev)
>>  {
>>  	struct drm_device *dev = mdev->dev;
>> @@ -104,22 +118,10 @@ int mgag200_mm_init(struct mga_device *mdev)
>>  
>>  	mdev->vram_fb_available = mdev->mc.vram_size;
>>  
>> -	return 0;
>> +	return drmm_add_action_or_reset(dev, mgag200_mm_release, NULL);
>>  
>>  err_arch_phys_wc_del:
>>  	arch_phys_wc_del(mdev->fb_mtrr);
>>  	arch_io_free_memtype_wc(start, len);
> 
> Btw I think devm versions of these two would benefit a bunch of drivers in
> their cleanup code. devm_ not drmm_ since it's hw resource cleanup. In

Yup, I was looking and couldn't find them. Their non-existence is the
reason for the release function. Having them would make sense.

> case you ever run out of ideas :-)

I already have patches for further mgag200 mode-setting cleanups and ast
managed initialization. So no shortage of idea here. :)

Best regards
Thomas

> 
> Cheeres, Daniel
> 
>>  	return ret;
>>  }
>> -
>> -void mgag200_mm_fini(struct mga_device *mdev)
>> -{
>> -	struct drm_device *dev = mdev->dev;
>> -
>> -	mdev->vram_fb_available = 0;
>> -	iounmap(mdev->vram);
>> -	arch_io_free_memtype_wc(pci_resource_start(dev->pdev, 0),
>> -				pci_resource_len(dev->pdev, 0));
>> -	arch_phys_wc_del(mdev->fb_mtrr);
>> -	mdev->fb_mtrr = 0;
>> -}
>> -- 
>> 2.26.2
>>
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h
index cd786ffe319b8..7b6e6827a9a21 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.h
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
@@ -198,6 +198,5 @@  void mgag200_i2c_destroy(struct mga_i2c_chan *i2c);
 
 				/* mgag200_mm.c */
 int mgag200_mm_init(struct mga_device *mdev);
-void mgag200_mm_fini(struct mga_device *mdev);
 
 #endif				/* __MGAG200_DRV_H__ */
diff --git a/drivers/gpu/drm/mgag200/mgag200_main.c b/drivers/gpu/drm/mgag200/mgag200_main.c
index e9ad783c2b44d..49bcdfcb40a4e 100644
--- a/drivers/gpu/drm/mgag200/mgag200_main.c
+++ b/drivers/gpu/drm/mgag200/mgag200_main.c
@@ -57,13 +57,11 @@  int mgag200_driver_load(struct drm_device *dev, unsigned long flags)
 	ret = mgag200_modeset_init(mdev);
 	if (ret) {
 		drm_err(dev, "Fatal error during modeset init: %d\n", ret);
-		goto err_mgag200_mm_fini;
+		goto err_mm;
 	}
 
 	return 0;
 
-err_mgag200_mm_fini:
-	mgag200_mm_fini(mdev);
 err_mm:
 	dev->dev_private = NULL;
 	return ret;
@@ -75,6 +73,5 @@  void mgag200_driver_unload(struct drm_device *dev)
 
 	if (mdev == NULL)
 		return;
-	mgag200_mm_fini(mdev);
 	dev->dev_private = NULL;
 }
diff --git a/drivers/gpu/drm/mgag200/mgag200_mm.c b/drivers/gpu/drm/mgag200/mgag200_mm.c
index f56b0456994f4..1b1e5ec5d1ceb 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mm.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mm.c
@@ -28,6 +28,8 @@ 
 
 #include <linux/pci.h>
 
+#include <drm/drm_managed.h>
+
 #include "mgag200_drv.h"
 
 static size_t mgag200_probe_vram(struct mga_device *mdev, void __iomem *mem,
@@ -73,6 +75,18 @@  static size_t mgag200_probe_vram(struct mga_device *mdev, void __iomem *mem,
 	return offset - 65536;
 }
 
+static void mgag200_mm_release(struct drm_device *dev, void *ptr)
+{
+	struct mga_device *mdev = to_mga_device(dev);
+
+	mdev->vram_fb_available = 0;
+	iounmap(mdev->vram);
+	arch_io_free_memtype_wc(pci_resource_start(dev->pdev, 0),
+				pci_resource_len(dev->pdev, 0));
+	arch_phys_wc_del(mdev->fb_mtrr);
+	mdev->fb_mtrr = 0;
+}
+
 int mgag200_mm_init(struct mga_device *mdev)
 {
 	struct drm_device *dev = mdev->dev;
@@ -104,22 +118,10 @@  int mgag200_mm_init(struct mga_device *mdev)
 
 	mdev->vram_fb_available = mdev->mc.vram_size;
 
-	return 0;
+	return drmm_add_action_or_reset(dev, mgag200_mm_release, NULL);
 
 err_arch_phys_wc_del:
 	arch_phys_wc_del(mdev->fb_mtrr);
 	arch_io_free_memtype_wc(start, len);
 	return ret;
 }
-
-void mgag200_mm_fini(struct mga_device *mdev)
-{
-	struct drm_device *dev = mdev->dev;
-
-	mdev->vram_fb_available = 0;
-	iounmap(mdev->vram);
-	arch_io_free_memtype_wc(pci_resource_start(dev->pdev, 0),
-				pci_resource_len(dev->pdev, 0));
-	arch_phys_wc_del(mdev->fb_mtrr);
-	mdev->fb_mtrr = 0;
-}