Re: [PATCH 07/14] drm/mgag200: Switch to managed MM
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
>> ---
>> 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
>>
>> +#include
>> +
>> #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
>>
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
signature.asc
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 07/14] drm/mgag200: Switch to managed MM
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
> ---
> 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
>
> +#include
> +
> #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
>
--
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
[PATCH 07/14] drm/mgag200: Switch to managed MM
The memory-management code now cleans up automatically as part of
device destruction.
Signed-off-by: Thomas Zimmermann
---
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
+#include
+
#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;
-}
--
2.26.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
