Linux Virtualization - Nov 2017 - [PATCH 00/13] remove_conflicting_framebuffers() cleanup

This series cleans up duplicated code for replacing firmware FB
driver with proper DRI driver and adds handover support to
Tegra driver.

The last patch is here because it uses new semantics of
remove_conflicting_framebuffers() from this series. This
can be considered independently, though.

---

Micha? Miros?aw (13):
  fbdev: show fbdev number for debugging
  fbdev: add remove_conflicting_pci_framebuffers()
  drm/amdgpu: use simpler remove_conflicting_pci_framebuffers()
  drm/bochs: use simpler remove_conflicting_pci_framebuffers()
  drm/cirrus: use simpler remove_conflicting_pci_framebuffers()
  drm/mgag200: use simpler remove_conflicting_pci_framebuffers()
  drm/radeon: use simpler remove_conflicting_pci_framebuffers()
  drm/virtio: use simpler remove_conflicting_pci_framebuffers()
  staging: sm750fb: use simpler remove_conflicting_pci_framebuffers()
  fbdev: allow apertures == NULL in remove_conflicting_framebuffers()
  drm/vc4: use simpler remove_conflicting_framebuffers(NULL)
  drm/sun4i: use simpler remove_conflicting_framebuffers(NULL)
  drm/tegra: kick out simplefb

 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c  | 23 +-----------------
 drivers/gpu/drm/bochs/bochs_drv.c        | 18 +-------------
 drivers/gpu/drm/cirrus/cirrus_drv.c      | 23 +-----------------
 drivers/gpu/drm/mgag200/mgag200_drv.c    | 21 +----------------
 drivers/gpu/drm/mgag200/mgag200_main.c   |  9 -------
 drivers/gpu/drm/radeon/radeon_drv.c      | 23 +-----------------
 drivers/gpu/drm/sun4i/sun4i_drv.c        | 18 +-------------
 drivers/gpu/drm/tegra/drm.c              |  4 ++++
 drivers/gpu/drm/vc4/vc4_drv.c            | 20 +---------------
 drivers/gpu/drm/virtio/virtgpu_drm_bus.c | 24 +++----------------
 drivers/staging/sm750fb/sm750.c          | 22 +-----------------
 drivers/video/fbdev/core/fbmem.c         | 40 ++++++++++++++++++++++++++++++--
 include/drm/drm_fb_helper.h              | 12 ++++++++++
 include/linux/fb.h                       |  2 ++
 14 files changed, 67 insertions(+), 192 deletions(-)

-- 
2.11.0

Almost all drivers using remove_conflicting_framebuffers() wrap it with
the same code. Extract common part from PCI drivers into separate
remove_conflicting_pci_framebuffers().

Signed-off-by: Micha? Miros?aw <mirq-linux at rere.qmqm.pl>
---
 drivers/video/fbdev/core/fbmem.c | 22 ++++++++++++++++++++++
 include/drm/drm_fb_helper.h      | 12 ++++++++++++
 include/linux/fb.h               |  2 ++
 3 files changed, 36 insertions(+)

diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 30a18d4c9de4..5ea980e5d3b7 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -34,6 +34,7 @@
 #include <linux/fb.h>
 #include <linux/fbcon.h>
 #include <linux/mem_encrypt.h>
+#include <linux/pci.h>
 
 #include <asm/fb.h>
 
@@ -1788,6 +1789,27 @@ int remove_conflicting_framebuffers(struct
apertures_struct *a,
 }
 EXPORT_SYMBOL(remove_conflicting_framebuffers);
 
+int remove_conflicting_pci_framebuffers(struct pci_dev *pdev, int res_id, const
char *name)
+{
+	struct apertures_struct *ap;
+	bool primary = false;
+
+	ap = alloc_apertures(1);
+	if (!ap)
+		return -ENOMEM;
+
+	ap->ranges[0].base = pci_resource_start(pdev, res_id);
+	ap->ranges[0].size = pci_resource_len(pdev, res_id);
+#ifdef CONFIG_X86
+	primary = pdev->resource[PCI_ROM_RESOURCE].flags &
+					IORESOURCE_ROM_SHADOW;
+#endif
+	remove_conflicting_framebuffers(ap, name, primary);
+	kfree(ap);
+	return 0;
+}
+EXPORT_SYMBOL(remove_conflicting_pci_framebuffers);
+
 /**
  *	register_framebuffer - registers a frame buffer device
  *	@fb_info: frame buffer info structure
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index 33fe95927742..ac3412290289 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -520,4 +520,16 @@ drm_fb_helper_remove_conflicting_framebuffers(struct
apertures_struct *a,
 #endif
 }
 
+static inline int
+drm_fb_helper_remove_conflicting_pci_framebuffers(struct pci_dev *pdev,
+						  int resource_id,
+						  const char *name)
+{
+#if IS_REACHABLE(CONFIG_FB)
+	return remove_conflicting_pci_framebuffers(pdev, resource_id, name);
+#else
+	return 0;
+#endif
+}
+
 #endif
diff --git a/include/linux/fb.h b/include/linux/fb.h
index f4386b0ccf40..4196cb09e58e 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -624,6 +624,8 @@ extern ssize_t fb_sys_write(struct fb_info *info, const char
__user *buf,
 extern int register_framebuffer(struct fb_info *fb_info);
 extern int unregister_framebuffer(struct fb_info *fb_info);
 extern int unlink_framebuffer(struct fb_info *fb_info);
+extern int remove_conflicting_pci_framebuffers(struct pci_dev *pdev, int
res_id,
+					       const char *name);
 extern int remove_conflicting_framebuffers(struct apertures_struct *a,
 					   const char *name, bool primary);
 extern int fb_prepare_logo(struct fb_info *fb_info, int rotate);
-- 
2.11.0

Signed-off-by: Micha? Miros?aw <mirq-linux at rere.qmqm.pl>
---
 drivers/gpu/drm/cirrus/cirrus_drv.c | 23 +----------------------
 1 file changed, 1 insertion(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/cirrus/cirrus_drv.c
b/drivers/gpu/drm/cirrus/cirrus_drv.c
index 69c4e352dd78..85ed8657c862 100644
--- a/drivers/gpu/drm/cirrus/cirrus_drv.c
+++ b/drivers/gpu/drm/cirrus/cirrus_drv.c
@@ -42,33 +42,12 @@ static const struct pci_device_id pciidlist[] = {
 };
 
 
-static int cirrus_kick_out_firmware_fb(struct pci_dev *pdev)
-{
-	struct apertures_struct *ap;
-	bool primary = false;
-
-	ap = alloc_apertures(1);
-	if (!ap)
-		return -ENOMEM;
-
-	ap->ranges[0].base = pci_resource_start(pdev, 0);
-	ap->ranges[0].size = pci_resource_len(pdev, 0);
-
-#ifdef CONFIG_X86
-	primary = pdev->resource[PCI_ROM_RESOURCE].flags &
IORESOURCE_ROM_SHADOW;
-#endif
-	drm_fb_helper_remove_conflicting_framebuffers(ap, "cirrusdrmfb",
primary);
-	kfree(ap);
-
-	return 0;
-}
-
 static int cirrus_pci_probe(struct pci_dev *pdev,
 			    const struct pci_device_id *ent)
 {
 	int ret;
 
-	ret = cirrus_kick_out_firmware_fb(pdev);
+	ret = drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, 0,
"cirrusdrmfb");
 	if (ret)
 		return ret;
 
-- 
2.11.0

Signed-off-by: Micha? Miros?aw <mirq-linux at rere.qmqm.pl>
---
 drivers/gpu/drm/bochs/bochs_drv.c | 18 +-----------------
 1 file changed, 1 insertion(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/bochs/bochs_drv.c
b/drivers/gpu/drm/bochs/bochs_drv.c
index 7b20318483e4..c61b40c72b62 100644
--- a/drivers/gpu/drm/bochs/bochs_drv.c
+++ b/drivers/gpu/drm/bochs/bochs_drv.c
@@ -143,22 +143,6 @@ static const struct dev_pm_ops bochs_pm_ops = {
 /* ---------------------------------------------------------------------- */
 /* pci interface                                                          */
 
-static int bochs_kick_out_firmware_fb(struct pci_dev *pdev)
-{
-	struct apertures_struct *ap;
-
-	ap = alloc_apertures(1);
-	if (!ap)
-		return -ENOMEM;
-
-	ap->ranges[0].base = pci_resource_start(pdev, 0);
-	ap->ranges[0].size = pci_resource_len(pdev, 0);
-	drm_fb_helper_remove_conflicting_framebuffers(ap, "bochsdrmfb",
false);
-	kfree(ap);
-
-	return 0;
-}
-
 static int bochs_pci_probe(struct pci_dev *pdev,
 			   const struct pci_device_id *ent)
 {
@@ -171,7 +155,7 @@ static int bochs_pci_probe(struct pci_dev *pdev,
 		return -ENOMEM;
 	}
 
-	ret = bochs_kick_out_firmware_fb(pdev);
+	ret = drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, 0,
"bochsdrmfb");
 	if (ret)
 		return ret;
 
-- 
2.11.0

Signed-off-by: Micha? Miros?aw <mirq-linux at rere.qmqm.pl>
---
 drivers/gpu/drm/virtio/virtgpu_drm_bus.c | 24 +++---------------------
 1 file changed, 3 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_drm_bus.c
b/drivers/gpu/drm/virtio/virtgpu_drm_bus.c
index 7df8d0c9026a..115ed546ca4e 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drm_bus.c
+++ b/drivers/gpu/drm/virtio/virtgpu_drm_bus.c
@@ -28,26 +28,6 @@
 
 #include "virtgpu_drv.h"
 
-static void virtio_pci_kick_out_firmware_fb(struct pci_dev *pci_dev)
-{
-	struct apertures_struct *ap;
-	bool primary;
-
-	ap = alloc_apertures(1);
-	if (!ap)
-		return;
-
-	ap->ranges[0].base = pci_resource_start(pci_dev, 0);
-	ap->ranges[0].size = pci_resource_len(pci_dev, 0);
-
-	primary = pci_dev->resource[PCI_ROM_RESOURCE].flags
-		& IORESOURCE_ROM_SHADOW;
-
-	drm_fb_helper_remove_conflicting_framebuffers(ap, "virtiodrmfb",
primary);
-
-	kfree(ap);
-}
-
 int drm_virtio_init(struct drm_driver *driver, struct virtio_device *vdev)
 {
 	struct drm_device *dev;
@@ -69,7 +49,9 @@ int drm_virtio_init(struct drm_driver *driver, struct
virtio_device *vdev)
 			 pname);
 		dev->pdev = pdev;
 		if (vga)
-			virtio_pci_kick_out_firmware_fb(pdev);
+			drm_fb_helper_remove_conflicting_pci_framebuffers(pdev,
+									  0,
+									  "virtiodrmfb");
 
 		snprintf(unique, sizeof(unique), "pci:%s", pname);
 		ret = drm_dev_set_unique(dev, unique);
-- 
2.11.0

On Fri, Nov 24, 2017 at 06:53:31PM +0100, Micha? Miros?aw
wrote:
> Almost all drivers using remove_conflicting_framebuffers() wrap it with
> the same code. Extract common part from PCI drivers into separate
> remove_conflicting_pci_framebuffers().
> 
> Signed-off-by: Micha? Miros?aw <mirq-linux at rere.qmqm.pl>
Since the only driver that seems to use this is the staging one, which imo
is a DOA project, not sure it's worth to bother with this here.
-Daniel
> ---
>  drivers/video/fbdev/core/fbmem.c | 22 ++++++++++++++++++++++
>  include/drm/drm_fb_helper.h      | 12 ++++++++++++
>  include/linux/fb.h               |  2 ++
>  3 files changed, 36 insertions(+)
> 
> diff --git a/drivers/video/fbdev/core/fbmem.c
b/drivers/video/fbdev/core/fbmem.c
> index 30a18d4c9de4..5ea980e5d3b7 100644
> --- a/drivers/video/fbdev/core/fbmem.c
> +++ b/drivers/video/fbdev/core/fbmem.c
> @@ -34,6 +34,7 @@
>  #include <linux/fb.h>
>  #include <linux/fbcon.h>
>  #include <linux/mem_encrypt.h>
> +#include <linux/pci.h>
>  
>  #include <asm/fb.h>
>  
> @@ -1788,6 +1789,27 @@ int remove_conflicting_framebuffers(struct
apertures_struct *a,
>  }
>  EXPORT_SYMBOL(remove_conflicting_framebuffers);
>  
> +int remove_conflicting_pci_framebuffers(struct pci_dev *pdev, int res_id,
const char *name)
> +{
> +	struct apertures_struct *ap;
> +	bool primary = false;
> +
> +	ap = alloc_apertures(1);
> +	if (!ap)
> +		return -ENOMEM;
> +
> +	ap->ranges[0].base = pci_resource_start(pdev, res_id);
> +	ap->ranges[0].size = pci_resource_len(pdev, res_id);
> +#ifdef CONFIG_X86
> +	primary = pdev->resource[PCI_ROM_RESOURCE].flags &
> +					IORESOURCE_ROM_SHADOW;
> +#endif
> +	remove_conflicting_framebuffers(ap, name, primary);
> +	kfree(ap);
> +	return 0;
> +}
> +EXPORT_SYMBOL(remove_conflicting_pci_framebuffers);
> +
>  /**
>   *	register_framebuffer - registers a frame buffer device
>   *	@fb_info: frame buffer info structure
> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> index 33fe95927742..ac3412290289 100644
> --- a/include/drm/drm_fb_helper.h
> +++ b/include/drm/drm_fb_helper.h
> @@ -520,4 +520,16 @@ drm_fb_helper_remove_conflicting_framebuffers(struct
apertures_struct *a,
>  #endif
>  }
>  
> +static inline int
> +drm_fb_helper_remove_conflicting_pci_framebuffers(struct pci_dev *pdev,
> +						  int resource_id,
> +						  const char *name)
> +{
> +#if IS_REACHABLE(CONFIG_FB)
> +	return remove_conflicting_pci_framebuffers(pdev, resource_id, name);
> +#else
> +	return 0;
> +#endif
> +}
> +
>  #endif
> diff --git a/include/linux/fb.h b/include/linux/fb.h
> index f4386b0ccf40..4196cb09e58e 100644
> --- a/include/linux/fb.h
> +++ b/include/linux/fb.h
> @@ -624,6 +624,8 @@ extern ssize_t fb_sys_write(struct fb_info *info, const
char __user *buf,
>  extern int register_framebuffer(struct fb_info *fb_info);
>  extern int unregister_framebuffer(struct fb_info *fb_info);
>  extern int unlink_framebuffer(struct fb_info *fb_info);
> +extern int remove_conflicting_pci_framebuffers(struct pci_dev *pdev, int
res_id,
> +					       const char *name);
>  extern int remove_conflicting_framebuffers(struct apertures_struct *a,
>  					   const char *name, bool primary);
>  extern int fb_prepare_logo(struct fb_info *fb_info, int rotate);
> -- 
> 2.11.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

On Fri, Nov 24, 2017 at 06:53:25PM +0100, Micha? Miros?aw
wrote:
> This series cleans up duplicated code for replacing firmware FB
> driver with proper DRI driver and adds handover support to
> Tegra driver.
> 
> The last patch is here because it uses new semantics of
> remove_conflicting_framebuffers() from this series. This
> can be considered independently, though.
Except for that patches I've commented on:

Acked-by: Daniel Vetter <daniel.vetter at ffwll.ch>

Since this is for tegra and Thierry has drm-misc commit rights, it's
probably simplest when Thierry pushes this all to drm-misc once driver
maintainers had a chance to look at it. Also needs and ack from Bart for
the fbdev sides.
-Daniel
> 
> ---
> 
> Micha? Miros?aw (13):
>   fbdev: show fbdev number for debugging
>   fbdev: add remove_conflicting_pci_framebuffers()
>   drm/amdgpu: use simpler remove_conflicting_pci_framebuffers()
>   drm/bochs: use simpler remove_conflicting_pci_framebuffers()
>   drm/cirrus: use simpler remove_conflicting_pci_framebuffers()
>   drm/mgag200: use simpler remove_conflicting_pci_framebuffers()
>   drm/radeon: use simpler remove_conflicting_pci_framebuffers()
>   drm/virtio: use simpler remove_conflicting_pci_framebuffers()
>   staging: sm750fb: use simpler remove_conflicting_pci_framebuffers()
>   fbdev: allow apertures == NULL in remove_conflicting_framebuffers()
>   drm/vc4: use simpler remove_conflicting_framebuffers(NULL)
>   drm/sun4i: use simpler remove_conflicting_framebuffers(NULL)
>   drm/tegra: kick out simplefb
> 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c  | 23 +-----------------
>  drivers/gpu/drm/bochs/bochs_drv.c        | 18 +-------------
>  drivers/gpu/drm/cirrus/cirrus_drv.c      | 23 +-----------------
>  drivers/gpu/drm/mgag200/mgag200_drv.c    | 21 +----------------
>  drivers/gpu/drm/mgag200/mgag200_main.c   |  9 -------
>  drivers/gpu/drm/radeon/radeon_drv.c      | 23 +-----------------
>  drivers/gpu/drm/sun4i/sun4i_drv.c        | 18 +-------------
>  drivers/gpu/drm/tegra/drm.c              |  4 ++++
>  drivers/gpu/drm/vc4/vc4_drv.c            | 20 +---------------
>  drivers/gpu/drm/virtio/virtgpu_drm_bus.c | 24 +++----------------
>  drivers/staging/sm750fb/sm750.c          | 22 +-----------------
>  drivers/video/fbdev/core/fbmem.c         | 40
++++++++++++++++++++++++++++++--
>  include/drm/drm_fb_helper.h              | 12 ++++++++++
>  include/linux/fb.h                       |  2 ++
>  14 files changed, 67 insertions(+), 192 deletions(-)
> 
> -- 
> 2.11.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

On Mon, Nov 27, 2017 at 11:30:44AM +0100, Daniel Vetter
wrote:
> On Fri, Nov 24, 2017 at 06:53:25PM +0100, Micha? Miros?aw wrote:
> > This series cleans up duplicated code for replacing firmware FB
> > driver with proper DRI driver and adds handover support to
> > Tegra driver.
> > 
> > The last patch is here because it uses new semantics of
> > remove_conflicting_framebuffers() from this series. This
> > can be considered independently, though.
> 
> Except for that patches I've commented on:
> 
> Acked-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> 
> Since this is for tegra and Thierry has drm-misc commit rights, it's
> probably simplest when Thierry pushes this all to drm-misc once driver
> maintainers had a chance to look at it.
Will do.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL:
<http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20171127/704d890e/attachment-0001.sig>

On Monday, November 27, 2017 11:30:44 AM Daniel Vetter
wrote:
> On Fri, Nov 24, 2017 at 06:53:25PM +0100, Micha? Miros?aw wrote:
> > This series cleans up duplicated code for replacing firmware FB
> > driver with proper DRI driver and adds handover support to
> > Tegra driver.
Please Cc: me on and linux-fbdev ML on fbdev related patches
(I was Cc:-ed only on the cover letter and patch #10, linux-fbdev
was not Cc:-ed at all).
> > The last patch is here because it uses new semantics of
> > remove_conflicting_framebuffers() from this series. This
> > can be considered independently, though.
> 
> Except for that patches I've commented on:
> 
> Acked-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> 
> Since this is for tegra and Thierry has drm-misc commit rights, it's
> probably simplest when Thierry pushes this all to drm-misc once driver
> maintainers had a chance to look at it. Also needs and ack from Bart for
> the fbdev sides.
For fbdev changes:

Acked-by: Bartlomiej Zolnierkiewicz <b.zolnierkie at samsung.com>

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics