2021-09-17 05:52:18

by Thomas Zimmermann

[permalink] [raw]
Subject: [PATCH 0/5] lib: devres: Add managed helpers for write-combine setup

Add devm_arch_phys_wc_add() and devm_arch_io_reserve_memtype_wc() for
automatic cleanup of writecombine setup.

Several DRM drivers use the non-managed functions for setting their
framebuffer memory to write-combine access. Convert ast, mgag200 and
vboxvideo. Remove rsp clean-up code form drivers.

Tested on mgag200 hardware.

Thomas Zimmermann (5):
lib: devres: Add managed arch_phys_wc_add()
lib: devres: Add managed arch_io_reserve_memtype_wc()
drm/ast: Use managed interfaces for framebuffer write combining
drm/mgag200: Use managed interfaces for framebuffer write combining
drm/vboxvideo: Use managed interfaces for framebuffer write combining

drivers/gpu/drm/ast/ast_drv.h | 2 -
drivers/gpu/drm/ast/ast_mm.c | 27 ++++-----
drivers/gpu/drm/mgag200/mgag200_drv.h | 2 -
drivers/gpu/drm/mgag200/mgag200_mm.c | 35 +++---------
drivers/gpu/drm/vboxvideo/vbox_drv.c | 5 +-
drivers/gpu/drm/vboxvideo/vbox_drv.h | 1 -
drivers/gpu/drm/vboxvideo/vbox_ttm.c | 17 +++---
include/linux/io.h | 5 ++
lib/devres.c | 82 +++++++++++++++++++++++++++
9 files changed, 113 insertions(+), 63 deletions(-)

--
2.33.0


2021-09-17 05:53:42

by Thomas Zimmermann

[permalink] [raw]
Subject: [PATCH 1/5] lib: devres: Add managed arch_phys_wc_add()

Add devm_arch_phys_wc_add() as managed wrapper around arch_phys_wc_add().
Useful for several grahpics drivers that set framebuffer memory to write
combining.

Signed-off-by: Thomas Zimmermann <[email protected]>
---
include/linux/io.h | 2 ++
lib/devres.c | 36 ++++++++++++++++++++++++++++++++++++
2 files changed, 38 insertions(+)

diff --git a/include/linux/io.h b/include/linux/io.h
index 9595151d800d..fcd8ea79c5df 100644
--- a/include/linux/io.h
+++ b/include/linux/io.h
@@ -132,6 +132,8 @@ static inline int arch_phys_wc_index(int handle)
#endif
#endif

+int devm_arch_phys_wc_add(struct device *dev, unsigned long base, unsigned long size);
+
enum {
/* See memremap() kernel-doc for usage description... */
MEMREMAP_WB = 1 << 0,
diff --git a/lib/devres.c b/lib/devres.c
index b0e1c6702c71..24d4d849ff67 100644
--- a/lib/devres.c
+++ b/lib/devres.c
@@ -528,3 +528,39 @@ void pcim_iounmap_regions(struct pci_dev *pdev, int mask)
}
EXPORT_SYMBOL(pcim_iounmap_regions);
#endif /* CONFIG_PCI */
+
+static void devm_arch_phys_ac_add_release(struct device *dev, void *res)
+{
+ arch_phys_wc_del(*((int *)res));
+}
+
+/**
+ * devm_arch_phys_wc_add - Managed arch_phys_wc_add()
+ * @dev: Managed device
+ * @base: Memory base address
+ * @size: Size of memory range
+ *
+ * Adds a WC MTRR using arch_phys_wc_add() and sets up a release callback.
+ * See arch_phys_wc_add() for more information.
+ */
+int devm_arch_phys_wc_add(struct device *dev, unsigned long base, unsigned long size)
+{
+ int *mtrr;
+ int ret;
+
+ mtrr = devres_alloc(devm_arch_phys_ac_add_release, sizeof(*mtrr), GFP_KERNEL);
+ if (!mtrr)
+ return -ENOMEM;
+
+ ret = arch_phys_wc_add(base, size);
+ if (ret < 0) {
+ devres_free(mtrr);
+ return ret;
+ }
+
+ *mtrr = ret;
+ devres_add(dev, mtrr);
+
+ return ret;
+}
+EXPORT_SYMBOL(devm_arch_phys_wc_add);
--
2.33.0

2021-09-17 05:55:55

by Thomas Zimmermann

[permalink] [raw]
Subject: [PATCH 2/5] lib: devres: Add managed arch_io_reserve_memtype_wc()

Add devm_arch_io_reserve_memtype_wc() as managed wrapper around
arch_io_reserve_memtype_wc(). Useful for several grahpics drivers
that set framebuffer memory to write combining.

Signed-off-by: Thomas Zimmermann <[email protected]>
---
include/linux/io.h | 3 +++
lib/devres.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 49 insertions(+)

diff --git a/include/linux/io.h b/include/linux/io.h
index fcd8ea79c5df..5fc800390fe4 100644
--- a/include/linux/io.h
+++ b/include/linux/io.h
@@ -168,4 +168,7 @@ static inline void arch_io_free_memtype_wc(resource_size_t base,
}
#endif

+int devm_arch_io_reserve_memtype_wc(struct device *dev, resource_size_t start,
+ resource_size_t size);
+
#endif /* _LINUX_IO_H */
diff --git a/lib/devres.c b/lib/devres.c
index 24d4d849ff67..14664bbb4875 100644
--- a/lib/devres.c
+++ b/lib/devres.c
@@ -564,3 +564,49 @@ int devm_arch_phys_wc_add(struct device *dev, unsigned long base, unsigned long
return ret;
}
EXPORT_SYMBOL(devm_arch_phys_wc_add);
+
+struct arch_io_reserve_memtype_wc_devres {
+ resource_size_t start;
+ resource_size_t size;
+};
+
+static void devm_arch_io_free_memtype_wc_release(struct device *dev, void *res)
+{
+ const struct arch_io_reserve_memtype_wc_devres *this = res;
+
+ arch_io_free_memtype_wc(this->start, this->size);
+}
+
+/**
+ * devm_arch_io_reserve_memtype_wc - Managed arch_io_reserve_memtype_wc()
+ * @dev: Managed device
+ * @start: Memory base address
+ * @size: Size of memory range
+ *
+ * Reserves a memory range with WC caching using arch_io_reserve_memtype_wc()
+ * and sets up a release callback See arch_io_reserve_memtype_wc() for more
+ * information.
+ */
+int devm_arch_io_reserve_memtype_wc(struct device *dev, resource_size_t start,
+ resource_size_t size)
+{
+ struct arch_io_reserve_memtype_wc_devres *dr;
+ int ret;
+
+ dr = devres_alloc(devm_arch_io_free_memtype_wc_release, sizeof(*dr), GFP_KERNEL);
+ if (!dr)
+ return -ENOMEM;
+
+ ret = arch_io_reserve_memtype_wc(start, size);
+ if (ret < 0) {
+ devres_free(dr);
+ return ret;
+ }
+
+ dr->start = start;
+ dr->size = size;
+ devres_add(dev, dr);
+
+ return ret;
+}
+EXPORT_SYMBOL(devm_arch_io_reserve_memtype_wc);
--
2.33.0

2021-09-17 05:55:59

by Thomas Zimmermann

[permalink] [raw]
Subject: [PATCH 3/5] drm/ast: Use managed interfaces for framebuffer write combining

Replace arch_phys_wc_add() and arch_io_reserve_memtype_wc() with
the rsp managed functions. Allows for removing the cleanup code
for memory management

Signed-off-by: Thomas Zimmermann <[email protected]>
---
drivers/gpu/drm/ast/ast_drv.h | 2 --
drivers/gpu/drm/ast/ast_mm.c | 27 ++++++++++-----------------
2 files changed, 10 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 39ca338eb80b..2cfce7dc95af 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -158,8 +158,6 @@ struct ast_private {
uint32_t dram_type;
uint32_t mclk;

- int fb_mtrr;
-
struct drm_plane primary_plane;
struct ast_cursor_plane cursor_plane;
struct drm_crtc crtc;
diff --git a/drivers/gpu/drm/ast/ast_mm.c b/drivers/gpu/drm/ast/ast_mm.c
index 7592f1b9e1f1..6e999408dda9 100644
--- a/drivers/gpu/drm/ast/ast_mm.c
+++ b/drivers/gpu/drm/ast/ast_mm.c
@@ -74,35 +74,28 @@ static u32 ast_get_vram_size(struct ast_private *ast)
return vram_size;
}

-static void ast_mm_release(struct drm_device *dev, void *ptr)
-{
- struct ast_private *ast = to_ast_private(dev);
- struct pci_dev *pdev = to_pci_dev(dev->dev);
-
- arch_phys_wc_del(ast->fb_mtrr);
- arch_io_free_memtype_wc(pci_resource_start(pdev, 0),
- pci_resource_len(pdev, 0));
-}
-
int ast_mm_init(struct ast_private *ast)
{
struct drm_device *dev = &ast->base;
struct pci_dev *pdev = to_pci_dev(dev->dev);
+ resource_size_t base, size;
u32 vram_size;
int ret;

+ base = pci_resource_start(pdev, 0);
+ size = pci_resource_len(pdev, 0);
+
+ /* Don't fail on errors, but performance might be reduced. */
+ devm_arch_io_reserve_memtype_wc(dev->dev, base, size);
+ devm_arch_phys_wc_add(dev->dev, base, size);
+
vram_size = ast_get_vram_size(ast);

- ret = drmm_vram_helper_init(dev, pci_resource_start(pdev, 0), vram_size);
+ ret = drmm_vram_helper_init(dev, base, vram_size);
if (ret) {
drm_err(dev, "Error initializing VRAM MM; %d\n", ret);
return ret;
}

- arch_io_reserve_memtype_wc(pci_resource_start(pdev, 0),
- pci_resource_len(pdev, 0));
- ast->fb_mtrr = arch_phys_wc_add(pci_resource_start(pdev, 0),
- pci_resource_len(pdev, 0));
-
- return drmm_add_action_or_reset(dev, ast_mm_release, NULL);
+ return 0;
}
--
2.33.0

2021-09-17 05:57:00

by Thomas Zimmermann

[permalink] [raw]
Subject: [PATCH 5/5] drm/vboxvideo: Use managed interfaces for framebuffer write combining

Replace arch_phys_wc_add() with the rsp managed function. Allows for
removing the cleanup code for memory management

Signed-off-by: Thomas Zimmermann <[email protected]>
---
drivers/gpu/drm/vboxvideo/vbox_drv.c | 5 +----
drivers/gpu/drm/vboxvideo/vbox_drv.h | 1 -
drivers/gpu/drm/vboxvideo/vbox_ttm.c | 17 ++++++++---------
3 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.c b/drivers/gpu/drm/vboxvideo/vbox_drv.c
index 2b81cb259d23..a6c81af37345 100644
--- a/drivers/gpu/drm/vboxvideo/vbox_drv.c
+++ b/drivers/gpu/drm/vboxvideo/vbox_drv.c
@@ -69,7 +69,7 @@ static int vbox_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)

ret = vbox_mode_init(vbox);
if (ret)
- goto err_mm_fini;
+ goto err_hw_fini;

ret = vbox_irq_init(vbox);
if (ret)
@@ -87,8 +87,6 @@ static int vbox_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
vbox_irq_fini(vbox);
err_mode_fini:
vbox_mode_fini(vbox);
-err_mm_fini:
- vbox_mm_fini(vbox);
err_hw_fini:
vbox_hw_fini(vbox);
return ret;
@@ -101,7 +99,6 @@ static void vbox_pci_remove(struct pci_dev *pdev)
drm_dev_unregister(&vbox->ddev);
vbox_irq_fini(vbox);
vbox_mode_fini(vbox);
- vbox_mm_fini(vbox);
vbox_hw_fini(vbox);
}

diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.h b/drivers/gpu/drm/vboxvideo/vbox_drv.h
index 4903b91d7fe4..e77bd6512eb1 100644
--- a/drivers/gpu/drm/vboxvideo/vbox_drv.h
+++ b/drivers/gpu/drm/vboxvideo/vbox_drv.h
@@ -139,7 +139,6 @@ void vbox_mode_fini(struct vbox_private *vbox);
void vbox_report_caps(struct vbox_private *vbox);

int vbox_mm_init(struct vbox_private *vbox);
-void vbox_mm_fini(struct vbox_private *vbox);

/* vbox_irq.c */
int vbox_irq_init(struct vbox_private *vbox);
diff --git a/drivers/gpu/drm/vboxvideo/vbox_ttm.c b/drivers/gpu/drm/vboxvideo/vbox_ttm.c
index fd8a53a4d8d6..dc24c2172fd4 100644
--- a/drivers/gpu/drm/vboxvideo/vbox_ttm.c
+++ b/drivers/gpu/drm/vboxvideo/vbox_ttm.c
@@ -13,22 +13,21 @@
int vbox_mm_init(struct vbox_private *vbox)
{
int ret;
+ resource_size_t base, size;
struct drm_device *dev = &vbox->ddev;
struct pci_dev *pdev = to_pci_dev(dev->dev);

- ret = drmm_vram_helper_init(dev, pci_resource_start(pdev, 0),
- vbox->available_vram_size);
+ base = pci_resource_start(pdev, 0);
+ size = pci_resource_len(pdev, 0);
+
+ /* Don't fail on errors, but performance might be reduced. */
+ devm_arch_phys_wc_add(&pdev->dev, base, size);
+
+ ret = drmm_vram_helper_init(dev, base, vbox->available_vram_size);
if (ret) {
DRM_ERROR("Error initializing VRAM MM; %d\n", ret);
return ret;
}

- vbox->fb_mtrr = arch_phys_wc_add(pci_resource_start(pdev, 0),
- pci_resource_len(pdev, 0));
return 0;
}
-
-void vbox_mm_fini(struct vbox_private *vbox)
-{
- arch_phys_wc_del(vbox->fb_mtrr);
-}
--
2.33.0

2021-09-17 06:18:34

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH 5/5] drm/vboxvideo: Use managed interfaces for framebuffer write combining

Hi,

On 9/16/21 8:16 PM, Thomas Zimmermann wrote:
> Replace arch_phys_wc_add() with the rsp managed function. Allows for
> removing the cleanup code for memory management
>
> Signed-off-by: Thomas Zimmermann <[email protected]>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <[email protected]>

Regards,

Hans


> ---
> drivers/gpu/drm/vboxvideo/vbox_drv.c | 5 +----
> drivers/gpu/drm/vboxvideo/vbox_drv.h | 1 -
> drivers/gpu/drm/vboxvideo/vbox_ttm.c | 17 ++++++++---------
> 3 files changed, 9 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.c b/drivers/gpu/drm/vboxvideo/vbox_drv.c
> index 2b81cb259d23..a6c81af37345 100644
> --- a/drivers/gpu/drm/vboxvideo/vbox_drv.c
> +++ b/drivers/gpu/drm/vboxvideo/vbox_drv.c
> @@ -69,7 +69,7 @@ static int vbox_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>
> ret = vbox_mode_init(vbox);
> if (ret)
> - goto err_mm_fini;
> + goto err_hw_fini;
>
> ret = vbox_irq_init(vbox);
> if (ret)
> @@ -87,8 +87,6 @@ static int vbox_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> vbox_irq_fini(vbox);
> err_mode_fini:
> vbox_mode_fini(vbox);
> -err_mm_fini:
> - vbox_mm_fini(vbox);
> err_hw_fini:
> vbox_hw_fini(vbox);
> return ret;
> @@ -101,7 +99,6 @@ static void vbox_pci_remove(struct pci_dev *pdev)
> drm_dev_unregister(&vbox->ddev);
> vbox_irq_fini(vbox);
> vbox_mode_fini(vbox);
> - vbox_mm_fini(vbox);
> vbox_hw_fini(vbox);
> }
>
> diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.h b/drivers/gpu/drm/vboxvideo/vbox_drv.h
> index 4903b91d7fe4..e77bd6512eb1 100644
> --- a/drivers/gpu/drm/vboxvideo/vbox_drv.h
> +++ b/drivers/gpu/drm/vboxvideo/vbox_drv.h
> @@ -139,7 +139,6 @@ void vbox_mode_fini(struct vbox_private *vbox);
> void vbox_report_caps(struct vbox_private *vbox);
>
> int vbox_mm_init(struct vbox_private *vbox);
> -void vbox_mm_fini(struct vbox_private *vbox);
>
> /* vbox_irq.c */
> int vbox_irq_init(struct vbox_private *vbox);
> diff --git a/drivers/gpu/drm/vboxvideo/vbox_ttm.c b/drivers/gpu/drm/vboxvideo/vbox_ttm.c
> index fd8a53a4d8d6..dc24c2172fd4 100644
> --- a/drivers/gpu/drm/vboxvideo/vbox_ttm.c
> +++ b/drivers/gpu/drm/vboxvideo/vbox_ttm.c
> @@ -13,22 +13,21 @@
> int vbox_mm_init(struct vbox_private *vbox)
> {
> int ret;
> + resource_size_t base, size;
> struct drm_device *dev = &vbox->ddev;
> struct pci_dev *pdev = to_pci_dev(dev->dev);
>
> - ret = drmm_vram_helper_init(dev, pci_resource_start(pdev, 0),
> - vbox->available_vram_size);
> + base = pci_resource_start(pdev, 0);
> + size = pci_resource_len(pdev, 0);
> +
> + /* Don't fail on errors, but performance might be reduced. */
> + devm_arch_phys_wc_add(&pdev->dev, base, size);
> +
> + ret = drmm_vram_helper_init(dev, base, vbox->available_vram_size);
> if (ret) {
> DRM_ERROR("Error initializing VRAM MM; %d\n", ret);
> return ret;
> }
>
> - vbox->fb_mtrr = arch_phys_wc_add(pci_resource_start(pdev, 0),
> - pci_resource_len(pdev, 0));
> return 0;
> }
> -
> -void vbox_mm_fini(struct vbox_private *vbox)
> -{
> - arch_phys_wc_del(vbox->fb_mtrr);
> -}
>

2021-09-17 08:08:45

by Thomas Zimmermann

[permalink] [raw]
Subject: [PATCH 4/5] drm/mgag200: Use managed interfaces for framebuffer write combining

Replace arch_phys_wc_add() and arch_io_reserve_memtype_wc() with
the rsp managed functions. Allows for removing the cleanup code
for memory management

Signed-off-by: Thomas Zimmermann <[email protected]>
---
drivers/gpu/drm/mgag200/mgag200_drv.h | 2 --
drivers/gpu/drm/mgag200/mgag200_mm.c | 35 ++++++---------------------
2 files changed, 7 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h
index 196f74a0834e..4368112023f7 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.h
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
@@ -224,8 +224,6 @@ struct mga_device {

enum mga_type type;

- int fb_mtrr;
-
union {
struct {
long ref_clk;
diff --git a/drivers/gpu/drm/mgag200/mgag200_mm.c b/drivers/gpu/drm/mgag200/mgag200_mm.c
index b667371b69a4..fa996d46feed 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mm.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mm.c
@@ -75,26 +75,12 @@ 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);
- struct pci_dev *pdev = to_pci_dev(dev->dev);
-
- mdev->vram_fb_available = 0;
- iounmap(mdev->vram);
- arch_io_free_memtype_wc(pci_resource_start(pdev, 0),
- pci_resource_len(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->base;
struct pci_dev *pdev = to_pci_dev(dev->dev);
u8 misc;
resource_size_t start, len;
- int ret;

WREG_ECRT(0x04, 0x00);

@@ -112,15 +98,13 @@ int mgag200_mm_init(struct mga_device *mdev)
return -ENXIO;
}

- arch_io_reserve_memtype_wc(start, len);
-
- mdev->fb_mtrr = arch_phys_wc_add(start, len);
+ /* Don't fail on errors, but performance might be reduced. */
+ devm_arch_io_reserve_memtype_wc(dev->dev, start, len);
+ devm_arch_phys_wc_add(dev->dev, start, len);

- mdev->vram = ioremap(start, len);
- if (!mdev->vram) {
- ret = -ENOMEM;
- goto err_arch_phys_wc_del;
- }
+ mdev->vram = devm_ioremap(dev->dev, start, len);
+ if (!mdev->vram)
+ return -ENOMEM;

mdev->mc.vram_size = mgag200_probe_vram(mdev, mdev->vram, len);
mdev->mc.vram_base = start;
@@ -128,10 +112,5 @@ int mgag200_mm_init(struct mga_device *mdev)

mdev->vram_fb_available = mdev->mc.vram_size;

- 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;
+ return 0;
}
--
2.33.0

2021-09-17 14:57:12

by Daniel Vetter

[permalink] [raw]
Subject: Re: [PATCH 5/5] drm/vboxvideo: Use managed interfaces for framebuffer write combining

On Thu, Sep 16, 2021 at 09:28:53PM +0200, Hans de Goede wrote:
> Hi,
>
> On 9/16/21 8:16 PM, Thomas Zimmermann wrote:
> > Replace arch_phys_wc_add() with the rsp managed function. Allows for
> > removing the cleanup code for memory management
> >
> > Signed-off-by: Thomas Zimmermann <[email protected]>
>
> Thanks, patch looks good to me:
>
> Reviewed-by: Hans de Goede <[email protected]>

Maybe review entire series and then ask Thomas to unblock some of your
stuff?
-Daniel

>
> Regards,
>
> Hans
>
>
> > ---
> > drivers/gpu/drm/vboxvideo/vbox_drv.c | 5 +----
> > drivers/gpu/drm/vboxvideo/vbox_drv.h | 1 -
> > drivers/gpu/drm/vboxvideo/vbox_ttm.c | 17 ++++++++---------
> > 3 files changed, 9 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.c b/drivers/gpu/drm/vboxvideo/vbox_drv.c
> > index 2b81cb259d23..a6c81af37345 100644
> > --- a/drivers/gpu/drm/vboxvideo/vbox_drv.c
> > +++ b/drivers/gpu/drm/vboxvideo/vbox_drv.c
> > @@ -69,7 +69,7 @@ static int vbox_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> >
> > ret = vbox_mode_init(vbox);
> > if (ret)
> > - goto err_mm_fini;
> > + goto err_hw_fini;
> >
> > ret = vbox_irq_init(vbox);
> > if (ret)
> > @@ -87,8 +87,6 @@ static int vbox_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> > vbox_irq_fini(vbox);
> > err_mode_fini:
> > vbox_mode_fini(vbox);
> > -err_mm_fini:
> > - vbox_mm_fini(vbox);
> > err_hw_fini:
> > vbox_hw_fini(vbox);
> > return ret;
> > @@ -101,7 +99,6 @@ static void vbox_pci_remove(struct pci_dev *pdev)
> > drm_dev_unregister(&vbox->ddev);
> > vbox_irq_fini(vbox);
> > vbox_mode_fini(vbox);
> > - vbox_mm_fini(vbox);
> > vbox_hw_fini(vbox);
> > }
> >
> > diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.h b/drivers/gpu/drm/vboxvideo/vbox_drv.h
> > index 4903b91d7fe4..e77bd6512eb1 100644
> > --- a/drivers/gpu/drm/vboxvideo/vbox_drv.h
> > +++ b/drivers/gpu/drm/vboxvideo/vbox_drv.h
> > @@ -139,7 +139,6 @@ void vbox_mode_fini(struct vbox_private *vbox);
> > void vbox_report_caps(struct vbox_private *vbox);
> >
> > int vbox_mm_init(struct vbox_private *vbox);
> > -void vbox_mm_fini(struct vbox_private *vbox);
> >
> > /* vbox_irq.c */
> > int vbox_irq_init(struct vbox_private *vbox);
> > diff --git a/drivers/gpu/drm/vboxvideo/vbox_ttm.c b/drivers/gpu/drm/vboxvideo/vbox_ttm.c
> > index fd8a53a4d8d6..dc24c2172fd4 100644
> > --- a/drivers/gpu/drm/vboxvideo/vbox_ttm.c
> > +++ b/drivers/gpu/drm/vboxvideo/vbox_ttm.c
> > @@ -13,22 +13,21 @@
> > int vbox_mm_init(struct vbox_private *vbox)
> > {
> > int ret;
> > + resource_size_t base, size;
> > struct drm_device *dev = &vbox->ddev;
> > struct pci_dev *pdev = to_pci_dev(dev->dev);
> >
> > - ret = drmm_vram_helper_init(dev, pci_resource_start(pdev, 0),
> > - vbox->available_vram_size);
> > + base = pci_resource_start(pdev, 0);
> > + size = pci_resource_len(pdev, 0);
> > +
> > + /* Don't fail on errors, but performance might be reduced. */
> > + devm_arch_phys_wc_add(&pdev->dev, base, size);
> > +
> > + ret = drmm_vram_helper_init(dev, base, vbox->available_vram_size);
> > if (ret) {
> > DRM_ERROR("Error initializing VRAM MM; %d\n", ret);
> > return ret;
> > }
> >
> > - vbox->fb_mtrr = arch_phys_wc_add(pci_resource_start(pdev, 0),
> > - pci_resource_len(pdev, 0));
> > return 0;
> > }
> > -
> > -void vbox_mm_fini(struct vbox_private *vbox)
> > -{
> > - arch_phys_wc_del(vbox->fb_mtrr);
> > -}
> >
>

--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

2021-09-17 15:23:49

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH 0/5] lib: devres: Add managed helpers for write-combine setup

Hi,

On 9/16/21 8:15 PM, Thomas Zimmermann wrote:
> Add devm_arch_phys_wc_add() and devm_arch_io_reserve_memtype_wc() for
> automatic cleanup of writecombine setup.
>
> Several DRM drivers use the non-managed functions for setting their
> framebuffer memory to write-combine access. Convert ast, mgag200 and
> vboxvideo. Remove rsp clean-up code form drivers.
>
> Tested on mgag200 hardware.
>
> Thomas Zimmermann (5):
> lib: devres: Add managed arch_phys_wc_add()
> lib: devres: Add managed arch_io_reserve_memtype_wc()
> drm/ast: Use managed interfaces for framebuffer write combining
> drm/mgag200: Use managed interfaces for framebuffer write combining
> drm/vboxvideo: Use managed interfaces for framebuffer write combining

Thanks, the entire series looks good to me:

Reviewed-by: Hans de Goede <[email protected]>

For the series.

Regards,

Hans

2021-09-20 09:53:45

by Thomas Zimmermann

[permalink] [raw]
Subject: Re: [PATCH 0/5] lib: devres: Add managed helpers for write-combine setup

Hi

Am 17.09.21 um 16:47 schrieb Hans de Goede:
> Hi,
>
> On 9/16/21 8:15 PM, Thomas Zimmermann wrote:
>> Add devm_arch_phys_wc_add() and devm_arch_io_reserve_memtype_wc() for
>> automatic cleanup of writecombine setup.
>>
>> Several DRM drivers use the non-managed functions for setting their
>> framebuffer memory to write-combine access. Convert ast, mgag200 and
>> vboxvideo. Remove rsp clean-up code form drivers.
>>
>> Tested on mgag200 hardware.
>>
>> Thomas Zimmermann (5):
>> lib: devres: Add managed arch_phys_wc_add()
>> lib: devres: Add managed arch_io_reserve_memtype_wc()
>> drm/ast: Use managed interfaces for framebuffer write combining
>> drm/mgag200: Use managed interfaces for framebuffer write combining
>> drm/vboxvideo: Use managed interfaces for framebuffer write combining
>
> Thanks, the entire series looks good to me:
>
> Reviewed-by: Hans de Goede <[email protected]>
>
> For the series.

Thanks. Let me know if I can review some DRM code for you, as Daniel
suggested.

I'll wait a bit before merging the patches. Maybe devres devs want to
comment.

Best regards
Thomas

>
> Regards,
>
> Hans
>

--
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


Attachments:
OpenPGP_signature (855.00 B)
OpenPGP digital signature

2021-09-20 09:54:38

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH 0/5] lib: devres: Add managed helpers for write-combine setup

Hi,

On 9/20/21 10:01 AM, Thomas Zimmermann wrote:
> Hi
>
> Am 17.09.21 um 16:47 schrieb Hans de Goede:
>> Hi,
>>
>> On 9/16/21 8:15 PM, Thomas Zimmermann wrote:
>>> Add devm_arch_phys_wc_add() and devm_arch_io_reserve_memtype_wc() for
>>> automatic cleanup of writecombine setup.
>>>
>>> Several DRM drivers use the non-managed functions for setting their
>>> framebuffer memory to write-combine access. Convert ast, mgag200 and
>>> vboxvideo. Remove rsp clean-up code form drivers.
>>>
>>> Tested on mgag200 hardware.
>>>
>>> Thomas Zimmermann (5):
>>>    lib: devres: Add managed arch_phys_wc_add()
>>>    lib: devres: Add managed arch_io_reserve_memtype_wc()
>>>    drm/ast: Use managed interfaces for framebuffer write combining
>>>    drm/mgag200: Use managed interfaces for framebuffer write combining
>>>    drm/vboxvideo: Use managed interfaces for framebuffer write combining
>>
>> Thanks, the entire series looks good to me:
>>
>> Reviewed-by: Hans de Goede <[email protected]>
>>
>> For the series.
>
> Thanks. Let me know if I can review some DRM code for you, as Daniel suggested.

Thank you for the offer, ATM I don't have anything queued up in need of review,
but I will probably make use of your offer in the future :)
Regards,

Hans