[PATCH 1/3] fbmem: fix aperture overlapping check

[PATCH 1/3] fbmem: fix aperture overlapping check

[marcin.slusarz]

fb_do_apertures_overlap is returning wrong value when one aperture
is completely whithin the other. Add generic ranges_overlap macro
(probably kernel.h candidate) and use it here.

Signed-off-by: Marcin Slusarz <marcin.slusarz@gmail.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Peter Jones <pjones@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 drivers/video/fbmem.c |   24 +++++++++++++++++-------
 1 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index a15b44e..41f2e5e 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -1468,15 +1468,25 @@ static int fb_check_foreignness(struct fb_info *fi)
 	return 0;
 }
 
+/**
+ * ranges_overlap - check whether two ranges overlap (their intersection is not empty)
+ * @start1: start of the first range
+ * @size1:  length of the first range
+ * @start2: start of the second range
+ * @size2:  length of the second range
+ */
+#define ranges_overlap(start1, size1, start2, size2) ({	\
+	typeof(start1) __start1 = (start1);		\
+	typeof(size1)  __size1  = (size1);		\
+	typeof(start2) __start2 = (start2);		\
+	typeof(size2)  __size2  = (size2);		\
+	__start1 < __start2 + __size2 && __start1 + __size1 > __start2; \
+})
+
 static bool fb_do_apertures_overlap(struct fb_info *gen, struct fb_info *hw)
 {
-	/* is the generic aperture base the same as the HW one */
-	if (gen->aperture_base = hw->aperture_base)
-		return true;
-	/* is the generic aperture base inside the hw base->hw base+size */
-	if (gen->aperture_base > hw->aperture_base && gen->aperture_base <= hw->aperture_base + hw->aperture_size)
-		return true;
-	return false;
+	return ranges_overlap(gen->aperture_base, gen->aperture_size,
+				hw->aperture_base, hw->aperture_size);
 }
 /**
  *	register_framebuffer - registers a frame buffer device
-- 
1.7.0.4

[PATCH 1/3] fbmem: fix aperture overlapping check

[marcin.slusarz]

fb_do_apertures_overlap is returning wrong value when one aperture
is completely whithin the other. Add generic ranges_overlap macro
(probably kernel.h candidate) and use it here.

Signed-off-by: Marcin Slusarz <marcin.slusarz@gmail.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Peter Jones <pjones@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 drivers/video/fbmem.c |   24 +++++++++++++++++-------
 1 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index a15b44e..41f2e5e 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -1468,15 +1468,25 @@ static int fb_check_foreignness(struct fb_info *fi)
 	return 0;
 }
 
+/**
+ * ranges_overlap - check whether two ranges overlap (their intersection is not empty)
+ * @start1: start of the first range
+ * @size1:  length of the first range
+ * @start2: start of the second range
+ * @size2:  length of the second range
+ */
+#define ranges_overlap(start1, size1, start2, size2) ({	\
+	typeof(start1) __start1 = (start1);		\
+	typeof(size1)  __size1  = (size1);		\
+	typeof(start2) __start2 = (start2);		\
+	typeof(size2)  __size2  = (size2);		\
+	__start1 < __start2 + __size2 && __start1 + __size1 > __start2; \
+})
+
 static bool fb_do_apertures_overlap(struct fb_info *gen, struct fb_info *hw)
 {
-	/* is the generic aperture base the same as the HW one */
-	if (gen->aperture_base == hw->aperture_base)
-		return true;
-	/* is the generic aperture base inside the hw base->hw base+size */
-	if (gen->aperture_base > hw->aperture_base && gen->aperture_base <= hw->aperture_base + hw->aperture_size)
-		return true;
-	return false;
+	return ranges_overlap(gen->aperture_base, gen->aperture_size,
+				hw->aperture_base, hw->aperture_size);
 }
 /**
  *	register_framebuffer - registers a frame buffer device
-- 
1.7.0.4

[PATCH 1/3] fbmem: fix aperture overlapping check

[marcin.slusarz]

fb_do_apertures_overlap is returning wrong value when one aperture
is completely whithin the other. Add generic ranges_overlap macro
(probably kernel.h candidate) and use it here.

Signed-off-by: Marcin Slusarz <marcin.slusarz@gmail.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Peter Jones <pjones@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 drivers/video/fbmem.c |   24 +++++++++++++++++-------
 1 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index a15b44e..41f2e5e 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -1468,15 +1468,25 @@ static int fb_check_foreignness(struct fb_info *fi)
 	return 0;
 }
 
+/**
+ * ranges_overlap - check whether two ranges overlap (their intersection is not empty)
+ * @start1: start of the first range
+ * @size1:  length of the first range
+ * @start2: start of the second range
+ * @size2:  length of the second range
+ */
+#define ranges_overlap(start1, size1, start2, size2) ({	\
+	typeof(start1) __start1 = (start1);		\
+	typeof(size1)  __size1  = (size1);		\
+	typeof(start2) __start2 = (start2);		\
+	typeof(size2)  __size2  = (size2);		\
+	__start1 < __start2 + __size2 && __start1 + __size1 > __start2; \
+})
+
 static bool fb_do_apertures_overlap(struct fb_info *gen, struct fb_info *hw)
 {
-	/* is the generic aperture base the same as the HW one */
-	if (gen->aperture_base == hw->aperture_base)
-		return true;
-	/* is the generic aperture base inside the hw base->hw base+size */
-	if (gen->aperture_base > hw->aperture_base && gen->aperture_base <= hw->aperture_base + hw->aperture_size)
-		return true;
-	return false;
+	return ranges_overlap(gen->aperture_base, gen->aperture_size,
+				hw->aperture_base, hw->aperture_size);
 }
 /**
  *	register_framebuffer - registers a frame buffer device
-- 
1.7.0.4

[PATCH 2/3] fbdev: allow passing more than one aperture for handoff

[marcin.slusarz]

It simplifies nouveau code by removal of detection which
region to pass to kick vesafb/efifb.

Signed-off-by: Marcin Slusarz <marcin.slusarz@gmail.com>
Cc: Eric Anholt <eric@anholt.net>
Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: Thomas Hellstrom <thellstrom@vmware.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Peter Jones <pjones@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 drivers/gpu/drm/i915/intel_fb.c         |   11 +++-
 drivers/gpu/drm/nouveau/nouveau_fbcon.c |   84 +++++++++---------------------
 drivers/gpu/drm/radeon/radeon_fb.c      |    9 +++-
 drivers/gpu/drm/vmwgfx/vmwgfx_fb.c      |   10 +++-
 drivers/video/efifb.c                   |   11 +++-
 drivers/video/fbmem.c                   |   20 +++++++-
 drivers/video/fbsysfs.c                 |    1 +
 drivers/video/offb.c                    |   28 ++++++----
 drivers/video/vesafb.c                  |   11 +++-
 include/linux/fb.h                      |   16 +++++-
 10 files changed, 114 insertions(+), 87 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
index 8cd791d..a9192ab 100644
--- a/drivers/gpu/drm/i915/intel_fb.c
+++ b/drivers/gpu/drm/i915/intel_fb.c
@@ -192,11 +192,16 @@ static int intelfb_create(struct drm_device *dev, uint32_t fb_width,
 
 
 	/* setup aperture base/size for vesafb takeover */
-	info->aperture_base = dev->mode_config.fb_base;
+	info->apertures = alloc_apertures(1);
+	if (!info->apertures) {
+		ret = -ENOMEM;
+		goto out_unpin;
+	}
+	info->apertures->ranges[0].base = dev->mode_config.fb_base;
 	if (IS_I9XX(dev))
-		info->aperture_size = pci_resource_len(dev->pdev, 2);
+		info->apertures->ranges[0].size = pci_resource_len(dev->pdev, 2);
 	else
-		info->aperture_size = pci_resource_len(dev->pdev, 0);
+		info->apertures->ranges[0].size = pci_resource_len(dev->pdev, 0);
 
 	info->fix.smem_start = dev->mode_config.fb_base + obj_priv->gtt_offset;
 	info->fix.smem_len = size;
diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
index 68cedd9..206368b 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
@@ -161,44 +161,6 @@ static struct drm_fb_helper_funcs nouveau_fbcon_helper_funcs = {
 	.gamma_get = nouveau_fbcon_gamma_get
 };
 
-#if defined(__i386__) || defined(__x86_64__)
-static bool
-nouveau_fbcon_has_vesafb_or_efifb(struct drm_device *dev)
-{
-	struct pci_dev *pdev = dev->pdev;
-	int ramin;
-
-	if (screen_info.orig_video_isVGA != VIDEO_TYPE_VLFB &&
-	    screen_info.orig_video_isVGA != VIDEO_TYPE_EFI)
-		return false;
-
-	if (screen_info.lfb_base < pci_resource_start(pdev, 1))
-		goto not_fb;
-
-	if (screen_info.lfb_base + screen_info.lfb_size >=
-	    pci_resource_start(pdev, 1) + pci_resource_len(pdev, 1))
-		goto not_fb;
-
-	return true;
-not_fb:
-	ramin = 2;
-	if (pci_resource_len(pdev, ramin) == 0) {
-		ramin = 3;
-		if (pci_resource_len(pdev, ramin) == 0)
-			return false;
-	}
-
-	if (screen_info.lfb_base < pci_resource_start(pdev, ramin))
-		return false;
-
-	if (screen_info.lfb_base + screen_info.lfb_size >=
-	    pci_resource_start(pdev, ramin) + pci_resource_len(pdev, ramin))
-		return false;
-
-	return true;
-}
-#endif
-
 void
 nouveau_fbcon_zfill(struct drm_device *dev)
 {
@@ -231,7 +193,9 @@ nouveau_fbcon_create(struct drm_device *dev, uint32_t fb_width,
 	struct nouveau_framebuffer *nouveau_fb;
 	struct nouveau_bo *nvbo;
 	struct drm_mode_fb_cmd mode_cmd;
-	struct device *device = &dev->pdev->dev;
+	struct pci_dev *pdev = dev->pdev;
+	struct device *device = &pdev->dev;
+	struct apertures_struct *aper;
 	int size, ret;
 
 	mode_cmd.width = surface_width;
@@ -314,28 +278,30 @@ nouveau_fbcon_create(struct drm_device *dev, uint32_t fb_width,
 	drm_fb_helper_fill_var(info, fb, fb_width, fb_height);
 
 	/* FIXME: we really shouldn't expose mmio space at all */
-	info->fix.mmio_start = pci_resource_start(dev->pdev, 1);
-	info->fix.mmio_len = pci_resource_len(dev->pdev, 1);
+	info->fix.mmio_start = pci_resource_start(pdev, 1);
+	info->fix.mmio_len = pci_resource_len(pdev, 1);
 
 	/* Set aperture base/size for vesafb takeover */
-#if defined(__i386__) || defined(__x86_64__)
-	if (nouveau_fbcon_has_vesafb_or_efifb(dev)) {
-		/* Some NVIDIA VBIOS' are stupid and decide to put the
-		 * framebuffer in the middle of the PRAMIN BAR for
-		 * whatever reason.  We need to know the exact lfb_base
-		 * to get vesafb kicked off, and the only reliable way
-		 * we have left is to find out lfb_base the same way
-		 * vesafb did.
-		 */
-		info->aperture_base = screen_info.lfb_base;
-		info->aperture_size = screen_info.lfb_size;
-		if (screen_info.orig_video_isVGA == VIDEO_TYPE_VLFB)
-			info->aperture_size *= 65536;
-	} else
-#endif
-	{
-		info->aperture_base = info->fix.mmio_start;
-		info->aperture_size = info->fix.mmio_len;
+	aper = info->apertures = alloc_apertures(3);
+	if (!info->apertures) {
+		ret = -ENOMEM;
+		goto out_unref;
+	}
+
+	aper->ranges[0].base = pci_resource_start(pdev, 1);
+	aper->ranges[0].size = pci_resource_len(pdev, 1);
+	aper->count = 1;
+
+	if (pci_resource_len(pdev, 2)) {
+		aper->ranges[aper->count].base = pci_resource_start(pdev, 2);
+		aper->ranges[aper->count].size = pci_resource_len(pdev, 2);
+		aper->count++;
+	}
+
+	if (pci_resource_len(pdev, 3)) {
+		aper->ranges[aper->count].base = pci_resource_start(pdev, 3);
+		aper->ranges[aper->count].size = pci_resource_len(pdev, 3);
+		aper->count++;
 	}
 
 	info->pixmap.size = 64*1024;
diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c
index 8fccbf2..2b57cb4 100644
--- a/drivers/gpu/drm/radeon/radeon_fb.c
+++ b/drivers/gpu/drm/radeon/radeon_fb.c
@@ -263,8 +263,13 @@ int radeonfb_create(struct drm_device *dev,
 	drm_fb_helper_fill_var(info, fb, fb_width, fb_height);
 
 	/* setup aperture base/size for vesafb takeover */
-	info->aperture_base = rdev->ddev->mode_config.fb_base;
-	info->aperture_size = rdev->mc.real_vram_size;
+	info->apertures = alloc_apertures(1);
+	if (!info->apertures) {
+		ret = -ENOMEM;
+		goto out_unref;
+	}
+	info->apertures->ranges[0].base = rdev->ddev->mode_config.fb_base;
+	info->apertures->ranges[0].size = rdev->mc.real_vram_size;
 
 	info->fix.mmio_start = 0;
 	info->fix.mmio_len = 0;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
index a933670..0248c6b 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
@@ -559,8 +559,13 @@ int vmw_fb_init(struct vmw_private *vmw_priv)
 	info->pixmap.scan_align = 1;
 #endif
 
-	info->aperture_base = vmw_priv->vram_start;
-	info->aperture_size = vmw_priv->vram_size;
+	info->apertures = alloc_apertures(1);
+	if (!info->apertures) {
+		ret = -ENOMEM;
+		goto err_aper;
+	}
+	info->apertures->ranges[0].base = vmw_priv->vram_start;
+	info->apertures->ranges[0].size = vmw_priv->vram_size;
 
 	/*
 	 * Dirty & Deferred IO
@@ -580,6 +585,7 @@ int vmw_fb_init(struct vmw_private *vmw_priv)
 
 err_defio:
 	fb_deferred_io_cleanup(info);
+err_aper:
 	ttm_bo_kunmap(&par->map);
 err_unref:
 	ttm_bo_unref((struct ttm_buffer_object **)&par->vmw_bo);
diff --git a/drivers/video/efifb.c b/drivers/video/efifb.c
index 581d2db..3b98656 100644
--- a/drivers/video/efifb.c
+++ b/drivers/video/efifb.c
@@ -165,7 +165,7 @@ static void efifb_destroy(struct fb_info *info)
 {
 	if (info->screen_base)
 		iounmap(info->screen_base);
-	release_mem_region(info->aperture_base, info->aperture_size);
+	release_mem_region(info->apertures->ranges[0].base, info->apertures->ranges[0].size);
 	framebuffer_release(info);
 }
 
@@ -289,8 +289,13 @@ static int __devinit efifb_probe(struct platform_device *dev)
 	info->pseudo_palette = info->par;
 	info->par = NULL;
 
-	info->aperture_base = efifb_fix.smem_start;
-	info->aperture_size = size_remap;
+	info->apertures = alloc_apertures(1);
+	if (!info->apertures) {
+		err = -ENOMEM;
+		goto err_release_fb;
+	}
+	info->apertures->ranges[0].base = efifb_fix.smem_start;
+	info->apertures->ranges[0].size = size_remap;
 
 	info->screen_base = ioremap(efifb_fix.smem_start, efifb_fix.smem_len);
 	if (!info->screen_base) {
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index 41f2e5e..8824d2c 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -1485,8 +1485,24 @@ static int fb_check_foreignness(struct fb_info *fi)
 
 static bool fb_do_apertures_overlap(struct fb_info *gen, struct fb_info *hw)
 {
-	return ranges_overlap(gen->aperture_base, gen->aperture_size,
-				hw->aperture_base, hw->aperture_size);
+	int i, j;
+	struct apertures_struct *hwa = hw->apertures;
+	struct apertures_struct *gena = gen->apertures;
+	if (!hwa || !gena)
+		return false;
+
+	for (i = 0; i < hwa->count; ++i) {
+		struct aperture *h = &hwa->ranges[i];
+		for (j = 0; j < gena->count; ++j) {
+			struct aperture *g = &gena->ranges[j];
+			printk(KERN_DEBUG "checking generic (%llx %llx) vs hw (%llx %llx)\n",
+				g->base, g->size, h->base, h->size);
+			if (ranges_overlap(h->base, h->size, g->base, g->size))
+				return true;
+		}
+	}
+
+	return false;
 }
 /**
  *	register_framebuffer - registers a frame buffer device
diff --git a/drivers/video/fbsysfs.c b/drivers/video/fbsysfs.c
index d4a2c11..fd5519b 100644
--- a/drivers/video/fbsysfs.c
+++ b/drivers/video/fbsysfs.c
@@ -79,6 +79,7 @@ EXPORT_SYMBOL(framebuffer_alloc);
  */
 void framebuffer_release(struct fb_info *info)
 {
+	kfree(info->apertures);
 	kfree(info);
 }
 EXPORT_SYMBOL(framebuffer_release);
diff --git a/drivers/video/offb.c b/drivers/video/offb.c
index b043ac8..3d202b2 100644
--- a/drivers/video/offb.c
+++ b/drivers/video/offb.c
@@ -286,7 +286,7 @@ static void offb_destroy(struct fb_info *info)
 {
 	if (info->screen_base)
 		iounmap(info->screen_base);
-	release_mem_region(info->aperture_base, info->aperture_size);
+	release_mem_region(info->apertures->ranges[0].base, info->apertures->ranges[0].size);
 	framebuffer_release(info);
 }
 
@@ -492,8 +492,11 @@ static void __init offb_init_fb(const char *name, const char *full_name,
 	var->vmode = FB_VMODE_NONINTERLACED;
 
 	/* set offb aperture size for generic probing */
-	info->aperture_base = address;
-	info->aperture_size = fix->smem_len;
+	info->apertures = alloc_apertures(1);
+	if (!info->apertures)
+		goto out_aper;
+	info->apertures->ranges[0].base = address;
+	info->apertures->ranges[0].size = fix->smem_len;
 
 	info->fbops = &offb_ops;
 	info->screen_base = ioremap(address, fix->smem_len);
@@ -502,17 +505,20 @@ static void __init offb_init_fb(const char *name, const char *full_name,
 
 	fb_alloc_cmap(&info->cmap, 256, 0);
 
-	if (register_framebuffer(info) < 0) {
-		iounmap(par->cmap_adr);
-		par->cmap_adr = NULL;
-		iounmap(info->screen_base);
-		framebuffer_release(info);
-		release_mem_region(res_start, res_size);
-		return;
-	}
+	if (register_framebuffer(info) < 0)
+		goto out_err;
 
 	printk(KERN_INFO "fb%d: Open Firmware frame buffer device on %s\n",
 	       info->node, full_name);
+	return;
+
+out_err:
+	iounmap(info->screen_base);
+out_aper:
+	iounmap(par->cmap_adr);
+	par->cmap_adr = NULL;
+	framebuffer_release(info);
+	release_mem_region(res_start, res_size);
 }
 
 
diff --git a/drivers/video/vesafb.c b/drivers/video/vesafb.c
index ef4128c..5de9ee1 100644
--- a/drivers/video/vesafb.c
+++ b/drivers/video/vesafb.c
@@ -178,7 +178,7 @@ static void vesafb_destroy(struct fb_info *info)
 {
 	if (info->screen_base)
 		iounmap(info->screen_base);
-	release_mem_region(info->aperture_base, info->aperture_size);
+	release_mem_region(info->apertures->ranges[0].base, info->apertures->ranges[0].size);
 	framebuffer_release(info);
 }
 
@@ -296,8 +296,13 @@ static int __devinit vesafb_probe(struct platform_device *dev)
 	info->par = NULL;
 
 	/* set vesafb aperture size for generic probing */
-	info->aperture_base = screen_info.lfb_base;
-	info->aperture_size = size_total;
+	info->apertures = alloc_apertures(1);
+	if (!info->apertures) {
+		err = -ENOMEM;
+		goto err;
+	}
+	info->apertures->ranges[0].base = screen_info.lfb_base;
+	info->apertures->ranges[0].size = size_total;
 
 	info->screen_base = ioremap(vesafb_fix.smem_start, vesafb_fix.smem_len);
 	if (!info->screen_base) {
diff --git a/include/linux/fb.h b/include/linux/fb.h
index c10163b..9605b81 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -862,10 +862,22 @@ struct fb_info {
 	/* we need the PCI or similiar aperture base/size not
 	   smem_start/size as smem_start may just be an object
 	   allocated inside the aperture so may not actually overlap */
-	resource_size_t aperture_base;
-	resource_size_t aperture_size;
+	struct apertures_struct {
+		unsigned int count;
+		struct aperture {
+			resource_size_t base;
+			resource_size_t size;
+		} ranges[0];
+	} *apertures;
 };
 
+static inline struct apertures_struct *alloc_apertures(unsigned int max_num) {
+	struct apertures_struct *a = kzalloc(sizeof(struct apertures_struct)
+			+ max_num * sizeof(struct aperture), GFP_KERNEL);
+	a->count = max_num;
+	return a;
+}
+
 #ifdef MODULE
 #define FBINFO_DEFAULT	FBINFO_MODULE
 #else
-- 
1.7.0.4

[PATCH 2/3] fbdev: allow passing more than one aperture for handoff

[marcin.slusarz]

It simplifies nouveau code by removal of detection which
region to pass to kick vesafb/efifb.

Signed-off-by: Marcin Slusarz <marcin.slusarz@gmail.com>
Cc: Eric Anholt <eric@anholt.net>
Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: Thomas Hellstrom <thellstrom@vmware.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Peter Jones <pjones@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 drivers/gpu/drm/i915/intel_fb.c         |   11 +++-
 drivers/gpu/drm/nouveau/nouveau_fbcon.c |   84 +++++++++---------------------
 drivers/gpu/drm/radeon/radeon_fb.c      |    9 +++-
 drivers/gpu/drm/vmwgfx/vmwgfx_fb.c      |   10 +++-
 drivers/video/efifb.c                   |   11 +++-
 drivers/video/fbmem.c                   |   20 +++++++-
 drivers/video/fbsysfs.c                 |    1 +
 drivers/video/offb.c                    |   28 ++++++----
 drivers/video/vesafb.c                  |   11 +++-
 include/linux/fb.h                      |   16 +++++-
 10 files changed, 114 insertions(+), 87 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
index 8cd791d..a9192ab 100644
--- a/drivers/gpu/drm/i915/intel_fb.c
+++ b/drivers/gpu/drm/i915/intel_fb.c
@@ -192,11 +192,16 @@ static int intelfb_create(struct drm_device *dev, uint32_t fb_width,
 
 
 	/* setup aperture base/size for vesafb takeover */
-	info->aperture_base = dev->mode_config.fb_base;
+	info->apertures = alloc_apertures(1);
+	if (!info->apertures) {
+		ret = -ENOMEM;
+		goto out_unpin;
+	}
+	info->apertures->ranges[0].base = dev->mode_config.fb_base;
 	if (IS_I9XX(dev))
-		info->aperture_size = pci_resource_len(dev->pdev, 2);
+		info->apertures->ranges[0].size = pci_resource_len(dev->pdev, 2);
 	else
-		info->aperture_size = pci_resource_len(dev->pdev, 0);
+		info->apertures->ranges[0].size = pci_resource_len(dev->pdev, 0);
 
 	info->fix.smem_start = dev->mode_config.fb_base + obj_priv->gtt_offset;
 	info->fix.smem_len = size;
diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
index 68cedd9..206368b 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
@@ -161,44 +161,6 @@ static struct drm_fb_helper_funcs nouveau_fbcon_helper_funcs = {
 	.gamma_get = nouveau_fbcon_gamma_get
 };
 
-#if defined(__i386__) || defined(__x86_64__)
-static bool
-nouveau_fbcon_has_vesafb_or_efifb(struct drm_device *dev)
-{
-	struct pci_dev *pdev = dev->pdev;
-	int ramin;
-
-	if (screen_info.orig_video_isVGA != VIDEO_TYPE_VLFB &&
-	    screen_info.orig_video_isVGA != VIDEO_TYPE_EFI)
-		return false;
-
-	if (screen_info.lfb_base < pci_resource_start(pdev, 1))
-		goto not_fb;
-
-	if (screen_info.lfb_base + screen_info.lfb_size >-	    pci_resource_start(pdev, 1) + pci_resource_len(pdev, 1))
-		goto not_fb;
-
-	return true;
-not_fb:
-	ramin = 2;
-	if (pci_resource_len(pdev, ramin) = 0) {
-		ramin = 3;
-		if (pci_resource_len(pdev, ramin) = 0)
-			return false;
-	}
-
-	if (screen_info.lfb_base < pci_resource_start(pdev, ramin))
-		return false;
-
-	if (screen_info.lfb_base + screen_info.lfb_size >-	    pci_resource_start(pdev, ramin) + pci_resource_len(pdev, ramin))
-		return false;
-
-	return true;
-}
-#endif
-
 void
 nouveau_fbcon_zfill(struct drm_device *dev)
 {
@@ -231,7 +193,9 @@ nouveau_fbcon_create(struct drm_device *dev, uint32_t fb_width,
 	struct nouveau_framebuffer *nouveau_fb;
 	struct nouveau_bo *nvbo;
 	struct drm_mode_fb_cmd mode_cmd;
-	struct device *device = &dev->pdev->dev;
+	struct pci_dev *pdev = dev->pdev;
+	struct device *device = &pdev->dev;
+	struct apertures_struct *aper;
 	int size, ret;
 
 	mode_cmd.width = surface_width;
@@ -314,28 +278,30 @@ nouveau_fbcon_create(struct drm_device *dev, uint32_t fb_width,
 	drm_fb_helper_fill_var(info, fb, fb_width, fb_height);
 
 	/* FIXME: we really shouldn't expose mmio space at all */
-	info->fix.mmio_start = pci_resource_start(dev->pdev, 1);
-	info->fix.mmio_len = pci_resource_len(dev->pdev, 1);
+	info->fix.mmio_start = pci_resource_start(pdev, 1);
+	info->fix.mmio_len = pci_resource_len(pdev, 1);
 
 	/* Set aperture base/size for vesafb takeover */
-#if defined(__i386__) || defined(__x86_64__)
-	if (nouveau_fbcon_has_vesafb_or_efifb(dev)) {
-		/* Some NVIDIA VBIOS' are stupid and decide to put the
-		 * framebuffer in the middle of the PRAMIN BAR for
-		 * whatever reason.  We need to know the exact lfb_base
-		 * to get vesafb kicked off, and the only reliable way
-		 * we have left is to find out lfb_base the same way
-		 * vesafb did.
-		 */
-		info->aperture_base = screen_info.lfb_base;
-		info->aperture_size = screen_info.lfb_size;
-		if (screen_info.orig_video_isVGA = VIDEO_TYPE_VLFB)
-			info->aperture_size *= 65536;
-	} else
-#endif
-	{
-		info->aperture_base = info->fix.mmio_start;
-		info->aperture_size = info->fix.mmio_len;
+	aper = info->apertures = alloc_apertures(3);
+	if (!info->apertures) {
+		ret = -ENOMEM;
+		goto out_unref;
+	}
+
+	aper->ranges[0].base = pci_resource_start(pdev, 1);
+	aper->ranges[0].size = pci_resource_len(pdev, 1);
+	aper->count = 1;
+
+	if (pci_resource_len(pdev, 2)) {
+		aper->ranges[aper->count].base = pci_resource_start(pdev, 2);
+		aper->ranges[aper->count].size = pci_resource_len(pdev, 2);
+		aper->count++;
+	}
+
+	if (pci_resource_len(pdev, 3)) {
+		aper->ranges[aper->count].base = pci_resource_start(pdev, 3);
+		aper->ranges[aper->count].size = pci_resource_len(pdev, 3);
+		aper->count++;
 	}
 
 	info->pixmap.size = 64*1024;
diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c
index 8fccbf2..2b57cb4 100644
--- a/drivers/gpu/drm/radeon/radeon_fb.c
+++ b/drivers/gpu/drm/radeon/radeon_fb.c
@@ -263,8 +263,13 @@ int radeonfb_create(struct drm_device *dev,
 	drm_fb_helper_fill_var(info, fb, fb_width, fb_height);
 
 	/* setup aperture base/size for vesafb takeover */
-	info->aperture_base = rdev->ddev->mode_config.fb_base;
-	info->aperture_size = rdev->mc.real_vram_size;
+	info->apertures = alloc_apertures(1);
+	if (!info->apertures) {
+		ret = -ENOMEM;
+		goto out_unref;
+	}
+	info->apertures->ranges[0].base = rdev->ddev->mode_config.fb_base;
+	info->apertures->ranges[0].size = rdev->mc.real_vram_size;
 
 	info->fix.mmio_start = 0;
 	info->fix.mmio_len = 0;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
index a933670..0248c6b 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
@@ -559,8 +559,13 @@ int vmw_fb_init(struct vmw_private *vmw_priv)
 	info->pixmap.scan_align = 1;
 #endif
 
-	info->aperture_base = vmw_priv->vram_start;
-	info->aperture_size = vmw_priv->vram_size;
+	info->apertures = alloc_apertures(1);
+	if (!info->apertures) {
+		ret = -ENOMEM;
+		goto err_aper;
+	}
+	info->apertures->ranges[0].base = vmw_priv->vram_start;
+	info->apertures->ranges[0].size = vmw_priv->vram_size;
 
 	/*
 	 * Dirty & Deferred IO
@@ -580,6 +585,7 @@ int vmw_fb_init(struct vmw_private *vmw_priv)
 
 err_defio:
 	fb_deferred_io_cleanup(info);
+err_aper:
 	ttm_bo_kunmap(&par->map);
 err_unref:
 	ttm_bo_unref((struct ttm_buffer_object **)&par->vmw_bo);
diff --git a/drivers/video/efifb.c b/drivers/video/efifb.c
index 581d2db..3b98656 100644
--- a/drivers/video/efifb.c
+++ b/drivers/video/efifb.c
@@ -165,7 +165,7 @@ static void efifb_destroy(struct fb_info *info)
 {
 	if (info->screen_base)
 		iounmap(info->screen_base);
-	release_mem_region(info->aperture_base, info->aperture_size);
+	release_mem_region(info->apertures->ranges[0].base, info->apertures->ranges[0].size);
 	framebuffer_release(info);
 }
 
@@ -289,8 +289,13 @@ static int __devinit efifb_probe(struct platform_device *dev)
 	info->pseudo_palette = info->par;
 	info->par = NULL;
 
-	info->aperture_base = efifb_fix.smem_start;
-	info->aperture_size = size_remap;
+	info->apertures = alloc_apertures(1);
+	if (!info->apertures) {
+		err = -ENOMEM;
+		goto err_release_fb;
+	}
+	info->apertures->ranges[0].base = efifb_fix.smem_start;
+	info->apertures->ranges[0].size = size_remap;
 
 	info->screen_base = ioremap(efifb_fix.smem_start, efifb_fix.smem_len);
 	if (!info->screen_base) {
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index 41f2e5e..8824d2c 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -1485,8 +1485,24 @@ static int fb_check_foreignness(struct fb_info *fi)
 
 static bool fb_do_apertures_overlap(struct fb_info *gen, struct fb_info *hw)
 {
-	return ranges_overlap(gen->aperture_base, gen->aperture_size,
-				hw->aperture_base, hw->aperture_size);
+	int i, j;
+	struct apertures_struct *hwa = hw->apertures;
+	struct apertures_struct *gena = gen->apertures;
+	if (!hwa || !gena)
+		return false;
+
+	for (i = 0; i < hwa->count; ++i) {
+		struct aperture *h = &hwa->ranges[i];
+		for (j = 0; j < gena->count; ++j) {
+			struct aperture *g = &gena->ranges[j];
+			printk(KERN_DEBUG "checking generic (%llx %llx) vs hw (%llx %llx)\n",
+				g->base, g->size, h->base, h->size);
+			if (ranges_overlap(h->base, h->size, g->base, g->size))
+				return true;
+		}
+	}
+
+	return false;
 }
 /**
  *	register_framebuffer - registers a frame buffer device
diff --git a/drivers/video/fbsysfs.c b/drivers/video/fbsysfs.c
index d4a2c11..fd5519b 100644
--- a/drivers/video/fbsysfs.c
+++ b/drivers/video/fbsysfs.c
@@ -79,6 +79,7 @@ EXPORT_SYMBOL(framebuffer_alloc);
  */
 void framebuffer_release(struct fb_info *info)
 {
+	kfree(info->apertures);
 	kfree(info);
 }
 EXPORT_SYMBOL(framebuffer_release);
diff --git a/drivers/video/offb.c b/drivers/video/offb.c
index b043ac8..3d202b2 100644
--- a/drivers/video/offb.c
+++ b/drivers/video/offb.c
@@ -286,7 +286,7 @@ static void offb_destroy(struct fb_info *info)
 {
 	if (info->screen_base)
 		iounmap(info->screen_base);
-	release_mem_region(info->aperture_base, info->aperture_size);
+	release_mem_region(info->apertures->ranges[0].base, info->apertures->ranges[0].size);
 	framebuffer_release(info);
 }
 
@@ -492,8 +492,11 @@ static void __init offb_init_fb(const char *name, const char *full_name,
 	var->vmode = FB_VMODE_NONINTERLACED;
 
 	/* set offb aperture size for generic probing */
-	info->aperture_base = address;
-	info->aperture_size = fix->smem_len;
+	info->apertures = alloc_apertures(1);
+	if (!info->apertures)
+		goto out_aper;
+	info->apertures->ranges[0].base = address;
+	info->apertures->ranges[0].size = fix->smem_len;
 
 	info->fbops = &offb_ops;
 	info->screen_base = ioremap(address, fix->smem_len);
@@ -502,17 +505,20 @@ static void __init offb_init_fb(const char *name, const char *full_name,
 
 	fb_alloc_cmap(&info->cmap, 256, 0);
 
-	if (register_framebuffer(info) < 0) {
-		iounmap(par->cmap_adr);
-		par->cmap_adr = NULL;
-		iounmap(info->screen_base);
-		framebuffer_release(info);
-		release_mem_region(res_start, res_size);
-		return;
-	}
+	if (register_framebuffer(info) < 0)
+		goto out_err;
 
 	printk(KERN_INFO "fb%d: Open Firmware frame buffer device on %s\n",
 	       info->node, full_name);
+	return;
+
+out_err:
+	iounmap(info->screen_base);
+out_aper:
+	iounmap(par->cmap_adr);
+	par->cmap_adr = NULL;
+	framebuffer_release(info);
+	release_mem_region(res_start, res_size);
 }
 
 
diff --git a/drivers/video/vesafb.c b/drivers/video/vesafb.c
index ef4128c..5de9ee1 100644
--- a/drivers/video/vesafb.c
+++ b/drivers/video/vesafb.c
@@ -178,7 +178,7 @@ static void vesafb_destroy(struct fb_info *info)
 {
 	if (info->screen_base)
 		iounmap(info->screen_base);
-	release_mem_region(info->aperture_base, info->aperture_size);
+	release_mem_region(info->apertures->ranges[0].base, info->apertures->ranges[0].size);
 	framebuffer_release(info);
 }
 
@@ -296,8 +296,13 @@ static int __devinit vesafb_probe(struct platform_device *dev)
 	info->par = NULL;
 
 	/* set vesafb aperture size for generic probing */
-	info->aperture_base = screen_info.lfb_base;
-	info->aperture_size = size_total;
+	info->apertures = alloc_apertures(1);
+	if (!info->apertures) {
+		err = -ENOMEM;
+		goto err;
+	}
+	info->apertures->ranges[0].base = screen_info.lfb_base;
+	info->apertures->ranges[0].size = size_total;
 
 	info->screen_base = ioremap(vesafb_fix.smem_start, vesafb_fix.smem_len);
 	if (!info->screen_base) {
diff --git a/include/linux/fb.h b/include/linux/fb.h
index c10163b..9605b81 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -862,10 +862,22 @@ struct fb_info {
 	/* we need the PCI or similiar aperture base/size not
 	   smem_start/size as smem_start may just be an object
 	   allocated inside the aperture so may not actually overlap */
-	resource_size_t aperture_base;
-	resource_size_t aperture_size;
+	struct apertures_struct {
+		unsigned int count;
+		struct aperture {
+			resource_size_t base;
+			resource_size_t size;
+		} ranges[0];
+	} *apertures;
 };
 
+static inline struct apertures_struct *alloc_apertures(unsigned int max_num) {
+	struct apertures_struct *a = kzalloc(sizeof(struct apertures_struct)
+			+ max_num * sizeof(struct aperture), GFP_KERNEL);
+	a->count = max_num;
+	return a;
+}
+
 #ifdef MODULE
 #define FBINFO_DEFAULT	FBINFO_MODULE
 #else
-- 
1.7.0.4

[PATCH 2/3] fbdev: allow passing more than one aperture for handoff

[marcin.slusarz]

It simplifies nouveau code by removal of detection which
region to pass to kick vesafb/efifb.

Signed-off-by: Marcin Slusarz <marcin.slusarz@gmail.com>
Cc: Eric Anholt <eric@anholt.net>
Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: Thomas Hellstrom <thellstrom@vmware.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Peter Jones <pjones@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 drivers/gpu/drm/i915/intel_fb.c         |   11 +++-
 drivers/gpu/drm/nouveau/nouveau_fbcon.c |   84 +++++++++---------------------
 drivers/gpu/drm/radeon/radeon_fb.c      |    9 +++-
 drivers/gpu/drm/vmwgfx/vmwgfx_fb.c      |   10 +++-
 drivers/video/efifb.c                   |   11 +++-
 drivers/video/fbmem.c                   |   20 +++++++-
 drivers/video/fbsysfs.c                 |    1 +
 drivers/video/offb.c                    |   28 ++++++----
 drivers/video/vesafb.c                  |   11 +++-
 include/linux/fb.h                      |   16 +++++-
 10 files changed, 114 insertions(+), 87 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
index 8cd791d..a9192ab 100644
--- a/drivers/gpu/drm/i915/intel_fb.c
+++ b/drivers/gpu/drm/i915/intel_fb.c
@@ -192,11 +192,16 @@ static int intelfb_create(struct drm_device *dev, uint32_t fb_width,
 
 
 	/* setup aperture base/size for vesafb takeover */
-	info->aperture_base = dev->mode_config.fb_base;
+	info->apertures = alloc_apertures(1);
+	if (!info->apertures) {
+		ret = -ENOMEM;
+		goto out_unpin;
+	}
+	info->apertures->ranges[0].base = dev->mode_config.fb_base;
 	if (IS_I9XX(dev))
-		info->aperture_size = pci_resource_len(dev->pdev, 2);
+		info->apertures->ranges[0].size = pci_resource_len(dev->pdev, 2);
 	else
-		info->aperture_size = pci_resource_len(dev->pdev, 0);
+		info->apertures->ranges[0].size = pci_resource_len(dev->pdev, 0);
 
 	info->fix.smem_start = dev->mode_config.fb_base + obj_priv->gtt_offset;
 	info->fix.smem_len = size;
diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
index 68cedd9..206368b 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
@@ -161,44 +161,6 @@ static struct drm_fb_helper_funcs nouveau_fbcon_helper_funcs = {
 	.gamma_get = nouveau_fbcon_gamma_get
 };
 
-#if defined(__i386__) || defined(__x86_64__)
-static bool
-nouveau_fbcon_has_vesafb_or_efifb(struct drm_device *dev)
-{
-	struct pci_dev *pdev = dev->pdev;
-	int ramin;
-
-	if (screen_info.orig_video_isVGA != VIDEO_TYPE_VLFB &&
-	    screen_info.orig_video_isVGA != VIDEO_TYPE_EFI)
-		return false;
-
-	if (screen_info.lfb_base < pci_resource_start(pdev, 1))
-		goto not_fb;
-
-	if (screen_info.lfb_base + screen_info.lfb_size >=
-	    pci_resource_start(pdev, 1) + pci_resource_len(pdev, 1))
-		goto not_fb;
-
-	return true;
-not_fb:
-	ramin = 2;
-	if (pci_resource_len(pdev, ramin) == 0) {
-		ramin = 3;
-		if (pci_resource_len(pdev, ramin) == 0)
-			return false;
-	}
-
-	if (screen_info.lfb_base < pci_resource_start(pdev, ramin))
-		return false;
-
-	if (screen_info.lfb_base + screen_info.lfb_size >=
-	    pci_resource_start(pdev, ramin) + pci_resource_len(pdev, ramin))
-		return false;
-
-	return true;
-}
-#endif
-
 void
 nouveau_fbcon_zfill(struct drm_device *dev)
 {
@@ -231,7 +193,9 @@ nouveau_fbcon_create(struct drm_device *dev, uint32_t fb_width,
 	struct nouveau_framebuffer *nouveau_fb;
 	struct nouveau_bo *nvbo;
 	struct drm_mode_fb_cmd mode_cmd;
-	struct device *device = &dev->pdev->dev;
+	struct pci_dev *pdev = dev->pdev;
+	struct device *device = &pdev->dev;
+	struct apertures_struct *aper;
 	int size, ret;
 
 	mode_cmd.width = surface_width;
@@ -314,28 +278,30 @@ nouveau_fbcon_create(struct drm_device *dev, uint32_t fb_width,
 	drm_fb_helper_fill_var(info, fb, fb_width, fb_height);
 
 	/* FIXME: we really shouldn't expose mmio space at all */
-	info->fix.mmio_start = pci_resource_start(dev->pdev, 1);
-	info->fix.mmio_len = pci_resource_len(dev->pdev, 1);
+	info->fix.mmio_start = pci_resource_start(pdev, 1);
+	info->fix.mmio_len = pci_resource_len(pdev, 1);
 
 	/* Set aperture base/size for vesafb takeover */
-#if defined(__i386__) || defined(__x86_64__)
-	if (nouveau_fbcon_has_vesafb_or_efifb(dev)) {
-		/* Some NVIDIA VBIOS' are stupid and decide to put the
-		 * framebuffer in the middle of the PRAMIN BAR for
-		 * whatever reason.  We need to know the exact lfb_base
-		 * to get vesafb kicked off, and the only reliable way
-		 * we have left is to find out lfb_base the same way
-		 * vesafb did.
-		 */
-		info->aperture_base = screen_info.lfb_base;
-		info->aperture_size = screen_info.lfb_size;
-		if (screen_info.orig_video_isVGA == VIDEO_TYPE_VLFB)
-			info->aperture_size *= 65536;
-	} else
-#endif
-	{
-		info->aperture_base = info->fix.mmio_start;
-		info->aperture_size = info->fix.mmio_len;
+	aper = info->apertures = alloc_apertures(3);
+	if (!info->apertures) {
+		ret = -ENOMEM;
+		goto out_unref;
+	}
+
+	aper->ranges[0].base = pci_resource_start(pdev, 1);
+	aper->ranges[0].size = pci_resource_len(pdev, 1);
+	aper->count = 1;
+
+	if (pci_resource_len(pdev, 2)) {
+		aper->ranges[aper->count].base = pci_resource_start(pdev, 2);
+		aper->ranges[aper->count].size = pci_resource_len(pdev, 2);
+		aper->count++;
+	}
+
+	if (pci_resource_len(pdev, 3)) {
+		aper->ranges[aper->count].base = pci_resource_start(pdev, 3);
+		aper->ranges[aper->count].size = pci_resource_len(pdev, 3);
+		aper->count++;
 	}
 
 	info->pixmap.size = 64*1024;
diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c
index 8fccbf2..2b57cb4 100644
--- a/drivers/gpu/drm/radeon/radeon_fb.c
+++ b/drivers/gpu/drm/radeon/radeon_fb.c
@@ -263,8 +263,13 @@ int radeonfb_create(struct drm_device *dev,
 	drm_fb_helper_fill_var(info, fb, fb_width, fb_height);
 
 	/* setup aperture base/size for vesafb takeover */
-	info->aperture_base = rdev->ddev->mode_config.fb_base;
-	info->aperture_size = rdev->mc.real_vram_size;
+	info->apertures = alloc_apertures(1);
+	if (!info->apertures) {
+		ret = -ENOMEM;
+		goto out_unref;
+	}
+	info->apertures->ranges[0].base = rdev->ddev->mode_config.fb_base;
+	info->apertures->ranges[0].size = rdev->mc.real_vram_size;
 
 	info->fix.mmio_start = 0;
 	info->fix.mmio_len = 0;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
index a933670..0248c6b 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
@@ -559,8 +559,13 @@ int vmw_fb_init(struct vmw_private *vmw_priv)
 	info->pixmap.scan_align = 1;
 #endif
 
-	info->aperture_base = vmw_priv->vram_start;
-	info->aperture_size = vmw_priv->vram_size;
+	info->apertures = alloc_apertures(1);
+	if (!info->apertures) {
+		ret = -ENOMEM;
+		goto err_aper;
+	}
+	info->apertures->ranges[0].base = vmw_priv->vram_start;
+	info->apertures->ranges[0].size = vmw_priv->vram_size;
 
 	/*
 	 * Dirty & Deferred IO
@@ -580,6 +585,7 @@ int vmw_fb_init(struct vmw_private *vmw_priv)
 
 err_defio:
 	fb_deferred_io_cleanup(info);
+err_aper:
 	ttm_bo_kunmap(&par->map);
 err_unref:
 	ttm_bo_unref((struct ttm_buffer_object **)&par->vmw_bo);
diff --git a/drivers/video/efifb.c b/drivers/video/efifb.c
index 581d2db..3b98656 100644
--- a/drivers/video/efifb.c
+++ b/drivers/video/efifb.c
@@ -165,7 +165,7 @@ static void efifb_destroy(struct fb_info *info)
 {
 	if (info->screen_base)
 		iounmap(info->screen_base);
-	release_mem_region(info->aperture_base, info->aperture_size);
+	release_mem_region(info->apertures->ranges[0].base, info->apertures->ranges[0].size);
 	framebuffer_release(info);
 }
 
@@ -289,8 +289,13 @@ static int __devinit efifb_probe(struct platform_device *dev)
 	info->pseudo_palette = info->par;
 	info->par = NULL;
 
-	info->aperture_base = efifb_fix.smem_start;
-	info->aperture_size = size_remap;
+	info->apertures = alloc_apertures(1);
+	if (!info->apertures) {
+		err = -ENOMEM;
+		goto err_release_fb;
+	}
+	info->apertures->ranges[0].base = efifb_fix.smem_start;
+	info->apertures->ranges[0].size = size_remap;
 
 	info->screen_base = ioremap(efifb_fix.smem_start, efifb_fix.smem_len);
 	if (!info->screen_base) {
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index 41f2e5e..8824d2c 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -1485,8 +1485,24 @@ static int fb_check_foreignness(struct fb_info *fi)
 
 static bool fb_do_apertures_overlap(struct fb_info *gen, struct fb_info *hw)
 {
-	return ranges_overlap(gen->aperture_base, gen->aperture_size,
-				hw->aperture_base, hw->aperture_size);
+	int i, j;
+	struct apertures_struct *hwa = hw->apertures;
+	struct apertures_struct *gena = gen->apertures;
+	if (!hwa || !gena)
+		return false;
+
+	for (i = 0; i < hwa->count; ++i) {
+		struct aperture *h = &hwa->ranges[i];
+		for (j = 0; j < gena->count; ++j) {
+			struct aperture *g = &gena->ranges[j];
+			printk(KERN_DEBUG "checking generic (%llx %llx) vs hw (%llx %llx)\n",
+				g->base, g->size, h->base, h->size);
+			if (ranges_overlap(h->base, h->size, g->base, g->size))
+				return true;
+		}
+	}
+
+	return false;
 }
 /**
  *	register_framebuffer - registers a frame buffer device
diff --git a/drivers/video/fbsysfs.c b/drivers/video/fbsysfs.c
index d4a2c11..fd5519b 100644
--- a/drivers/video/fbsysfs.c
+++ b/drivers/video/fbsysfs.c
@@ -79,6 +79,7 @@ EXPORT_SYMBOL(framebuffer_alloc);
  */
 void framebuffer_release(struct fb_info *info)
 {
+	kfree(info->apertures);
 	kfree(info);
 }
 EXPORT_SYMBOL(framebuffer_release);
diff --git a/drivers/video/offb.c b/drivers/video/offb.c
index b043ac8..3d202b2 100644
--- a/drivers/video/offb.c
+++ b/drivers/video/offb.c
@@ -286,7 +286,7 @@ static void offb_destroy(struct fb_info *info)
 {
 	if (info->screen_base)
 		iounmap(info->screen_base);
-	release_mem_region(info->aperture_base, info->aperture_size);
+	release_mem_region(info->apertures->ranges[0].base, info->apertures->ranges[0].size);
 	framebuffer_release(info);
 }
 
@@ -492,8 +492,11 @@ static void __init offb_init_fb(const char *name, const char *full_name,
 	var->vmode = FB_VMODE_NONINTERLACED;
 
 	/* set offb aperture size for generic probing */
-	info->aperture_base = address;
-	info->aperture_size = fix->smem_len;
+	info->apertures = alloc_apertures(1);
+	if (!info->apertures)
+		goto out_aper;
+	info->apertures->ranges[0].base = address;
+	info->apertures->ranges[0].size = fix->smem_len;
 
 	info->fbops = &offb_ops;
 	info->screen_base = ioremap(address, fix->smem_len);
@@ -502,17 +505,20 @@ static void __init offb_init_fb(const char *name, const char *full_name,
 
 	fb_alloc_cmap(&info->cmap, 256, 0);
 
-	if (register_framebuffer(info) < 0) {
-		iounmap(par->cmap_adr);
-		par->cmap_adr = NULL;
-		iounmap(info->screen_base);
-		framebuffer_release(info);
-		release_mem_region(res_start, res_size);
-		return;
-	}
+	if (register_framebuffer(info) < 0)
+		goto out_err;
 
 	printk(KERN_INFO "fb%d: Open Firmware frame buffer device on %s\n",
 	       info->node, full_name);
+	return;
+
+out_err:
+	iounmap(info->screen_base);
+out_aper:
+	iounmap(par->cmap_adr);
+	par->cmap_adr = NULL;
+	framebuffer_release(info);
+	release_mem_region(res_start, res_size);
 }
 
 
diff --git a/drivers/video/vesafb.c b/drivers/video/vesafb.c
index ef4128c..5de9ee1 100644
--- a/drivers/video/vesafb.c
+++ b/drivers/video/vesafb.c
@@ -178,7 +178,7 @@ static void vesafb_destroy(struct fb_info *info)
 {
 	if (info->screen_base)
 		iounmap(info->screen_base);
-	release_mem_region(info->aperture_base, info->aperture_size);
+	release_mem_region(info->apertures->ranges[0].base, info->apertures->ranges[0].size);
 	framebuffer_release(info);
 }
 
@@ -296,8 +296,13 @@ static int __devinit vesafb_probe(struct platform_device *dev)
 	info->par = NULL;
 
 	/* set vesafb aperture size for generic probing */
-	info->aperture_base = screen_info.lfb_base;
-	info->aperture_size = size_total;
+	info->apertures = alloc_apertures(1);
+	if (!info->apertures) {
+		err = -ENOMEM;
+		goto err;
+	}
+	info->apertures->ranges[0].base = screen_info.lfb_base;
+	info->apertures->ranges[0].size = size_total;
 
 	info->screen_base = ioremap(vesafb_fix.smem_start, vesafb_fix.smem_len);
 	if (!info->screen_base) {
diff --git a/include/linux/fb.h b/include/linux/fb.h
index c10163b..9605b81 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -862,10 +862,22 @@ struct fb_info {
 	/* we need the PCI or similiar aperture base/size not
 	   smem_start/size as smem_start may just be an object
 	   allocated inside the aperture so may not actually overlap */
-	resource_size_t aperture_base;
-	resource_size_t aperture_size;
+	struct apertures_struct {
+		unsigned int count;
+		struct aperture {
+			resource_size_t base;
+			resource_size_t size;
+		} ranges[0];
+	} *apertures;
 };
 
+static inline struct apertures_struct *alloc_apertures(unsigned int max_num) {
+	struct apertures_struct *a = kzalloc(sizeof(struct apertures_struct)
+			+ max_num * sizeof(struct aperture), GFP_KERNEL);
+	a->count = max_num;
+	return a;
+}
+
 #ifdef MODULE
 #define FBINFO_DEFAULT	FBINFO_MODULE
 #else
-- 
1.7.0.4

[PATCH 3/3] fbmem, drm/nouveau: kick firmware framebuffers as soon as possible

[marcin.slusarz]

Currently vesafb/efifb/... is kicked when hardware driver is registering
framebuffer. To do it hardware must be fully functional, so there's a short
window between start of initialisation and framebuffer registration when
two drivers touch the hardware. Unfortunately sometimes it breaks nouveau
initialisation.

Fix it by kicking firmware driver(s) before we start touching the hardware.

Reported-by: Didier Spaier <didier.spaier@epsm.fr>
Signed-off-by: Marcin Slusarz <marcin.slusarz@gmail.com>
Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Peter Jones <pjones@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 drivers/gpu/drm/nouveau/nouveau_drv.h   |    2 +
 drivers/gpu/drm/nouveau/nouveau_fbcon.c |   19 +-------------
 drivers/gpu/drm/nouveau/nouveau_state.c |   43 +++++++++++++++++++++++++++++++
 drivers/video/fbmem.c                   |   43 ++++++++++++++++++-------------
 include/linux/fb.h                      |    1 +
 5 files changed, 72 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_drv.h b/drivers/gpu/drm/nouveau/nouveau_drv.h
index cb16a1b..a1f61f7 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drv.h
+++ b/drivers/gpu/drm/nouveau/nouveau_drv.h
@@ -621,6 +621,8 @@ struct drm_nouveau_private {
 	struct {
 		struct dentry *channel_root;
 	} debugfs;
+
+	struct apertures_struct *apertures;
 };
 
 static inline struct drm_nouveau_private *
diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
index 206368b..5e4367b 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
@@ -195,7 +195,6 @@ nouveau_fbcon_create(struct drm_device *dev, uint32_t fb_width,
 	struct drm_mode_fb_cmd mode_cmd;
 	struct pci_dev *pdev = dev->pdev;
 	struct device *device = &pdev->dev;
-	struct apertures_struct *aper;
 	int size, ret;
 
 	mode_cmd.width = surface_width;
@@ -282,28 +281,12 @@ nouveau_fbcon_create(struct drm_device *dev, uint32_t fb_width,
 	info->fix.mmio_len = pci_resource_len(pdev, 1);
 
 	/* Set aperture base/size for vesafb takeover */
-	aper = info->apertures = alloc_apertures(3);
+	info->apertures = dev_priv->apertures;
 	if (!info->apertures) {
 		ret = -ENOMEM;
 		goto out_unref;
 	}
 
-	aper->ranges[0].base = pci_resource_start(pdev, 1);
-	aper->ranges[0].size = pci_resource_len(pdev, 1);
-	aper->count = 1;
-
-	if (pci_resource_len(pdev, 2)) {
-		aper->ranges[aper->count].base = pci_resource_start(pdev, 2);
-		aper->ranges[aper->count].size = pci_resource_len(pdev, 2);
-		aper->count++;
-	}
-
-	if (pci_resource_len(pdev, 3)) {
-		aper->ranges[aper->count].base = pci_resource_start(pdev, 3);
-		aper->ranges[aper->count].size = pci_resource_len(pdev, 3);
-		aper->count++;
-	}
-
 	info->pixmap.size = 64*1024;
 	info->pixmap.buf_align = 8;
 	info->pixmap.access_align = 32;
diff --git a/drivers/gpu/drm/nouveau/nouveau_state.c b/drivers/gpu/drm/nouveau/nouveau_state.c
index a9e9cf3..cfa3239 100644
--- a/drivers/gpu/drm/nouveau/nouveau_state.c
+++ b/drivers/gpu/drm/nouveau/nouveau_state.c
@@ -636,6 +636,43 @@ static void nouveau_OF_copy_vbios_to_ramin(struct drm_device *dev)
 #endif
 }
 
+static struct apertures_struct *nouveau_get_apertures(struct drm_device *dev)
+{
+	struct pci_dev *pdev = dev->pdev;
+	struct apertures_struct *aper = alloc_apertures(3);
+	if (!aper)
+		return NULL;
+
+	aper->ranges[0].base = pci_resource_start(pdev, 1);
+	aper->ranges[0].size = pci_resource_len(pdev, 1);
+	aper->count = 1;
+
+	if (pci_resource_len(pdev, 2)) {
+		aper->ranges[aper->count].base = pci_resource_start(pdev, 2);
+		aper->ranges[aper->count].size = pci_resource_len(pdev, 2);
+		aper->count++;
+	}
+
+	if (pci_resource_len(pdev, 3)) {
+		aper->ranges[aper->count].base = pci_resource_start(pdev, 3);
+		aper->ranges[aper->count].size = pci_resource_len(pdev, 3);
+		aper->count++;
+	}
+
+	return aper;
+}
+
+static int nouveau_remove_conflicting_drivers(struct drm_device *dev)
+{
+	struct drm_nouveau_private *dev_priv = dev->dev_private;
+	dev_priv->apertures = nouveau_get_apertures(dev);
+	if (!dev_priv->apertures)
+		return -ENOMEM;
+	
+	remove_conflicting_framebuffers(dev_priv->apertures, "nouveaufb");
+	return 0;
+}
+
 int nouveau_load(struct drm_device *dev, unsigned long flags)
 {
 	struct drm_nouveau_private *dev_priv;
@@ -723,6 +760,12 @@ int nouveau_load(struct drm_device *dev, unsigned long flags)
 	NV_INFO(dev, "Detected an NV%2x generation card (0x%08x)\n",
 		dev_priv->card_type, reg0);
 
+	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
+		int ret = nouveau_remove_conflicting_drivers(dev);
+		if (ret)
+			return ret;
+	}
+
 	/* map larger RAMIN aperture on NV40 cards */
 	dev_priv->ramin  = NULL;
 	if (dev_priv->card_type >= NV_40) {
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index 8824d2c..f5297a0 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -1483,11 +1483,10 @@ static int fb_check_foreignness(struct fb_info *fi)
 	__start1 < __start2 + __size2 && __start1 + __size1 > __start2; \
 })
 
-static bool fb_do_apertures_overlap(struct fb_info *gen, struct fb_info *hw)
+static bool fb_do_apertures_overlap(struct apertures_struct *gena,
+				    struct apertures_struct *hwa)
 {
 	int i, j;
-	struct apertures_struct *hwa = hw->apertures;
-	struct apertures_struct *gena = gen->apertures;
 	if (!hwa || !gena)
 		return false;
 
@@ -1504,6 +1503,28 @@ static bool fb_do_apertures_overlap(struct fb_info *gen, struct fb_info *hw)
 
 	return false;
 }
+
+void remove_conflicting_framebuffers(struct apertures_struct *a, const char *name)
+{
+	int i;
+
+	/* check all firmware fbs and kick off if the base addr overlaps */
+	for (i = 0 ; i < FB_MAX; i++) {
+		if (!registered_fb[i])
+			continue;
+
+		if (!(registered_fb[i]->flags & FBINFO_MISC_FIRMWARE))
+			continue;
+
+		if (fb_do_apertures_overlap(registered_fb[i]->apertures, a)) {
+			printk(KERN_ERR "fb: conflicting fb hw usage "
+			       "%s vs %s - removing generic driver\n",
+			       name, registered_fb[i]->fix.id);
+			unregister_framebuffer(registered_fb[i]);
+		}
+	}
+}
+
 /**
  *	register_framebuffer - registers a frame buffer device
  *	@fb_info: frame buffer info structure
@@ -1527,21 +1548,7 @@ register_framebuffer(struct fb_info *fb_info)
 	if (fb_check_foreignness(fb_info))
 		return -ENOSYS;
 
-	/* check all firmware fbs and kick off if the base addr overlaps */
-	for (i = 0 ; i < FB_MAX; i++) {
-		if (!registered_fb[i])
-			continue;
-
-		if (registered_fb[i]->flags & FBINFO_MISC_FIRMWARE) {
-			if (fb_do_apertures_overlap(registered_fb[i], fb_info)) {
-				printk(KERN_ERR "fb: conflicting fb hw usage "
-				       "%s vs %s - removing generic driver\n",
-				       fb_info->fix.id,
-				       registered_fb[i]->fix.id);
-				unregister_framebuffer(registered_fb[i]);
-			}
-		}
-	}
+	remove_conflicting_framebuffers(fb_info->apertures, fb_info->fix.id);
 
 	num_registered_fb++;
 	for (i = 0 ; i < FB_MAX; i++)
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 9605b81..9e854a7 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -970,6 +970,7 @@ extern ssize_t fb_sys_write(struct fb_info *info, const char __user *buf,
 /* drivers/video/fbmem.c */
 extern int register_framebuffer(struct fb_info *fb_info);
 extern int unregister_framebuffer(struct fb_info *fb_info);
+extern void remove_conflicting_framebuffers(struct apertures_struct *a, const char *name);
 extern int fb_prepare_logo(struct fb_info *fb_info, int rotate);
 extern int fb_show_logo(struct fb_info *fb_info, int rotate);
 extern char* fb_get_buffer_offset(struct fb_info *info, struct fb_pixmap *buf, u32 size);
-- 
1.7.0.4

[PATCH 3/3] fbmem, drm/nouveau: kick firmware framebuffers as soon as possible

[marcin.slusarz]

Currently vesafb/efifb/... is kicked when hardware driver is registering
framebuffer. To do it hardware must be fully functional, so there's a short
window between start of initialisation and framebuffer registration when
two drivers touch the hardware. Unfortunately sometimes it breaks nouveau
initialisation.

Fix it by kicking firmware driver(s) before we start touching the hardware.

Reported-by: Didier Spaier <didier.spaier@epsm.fr>
Signed-off-by: Marcin Slusarz <marcin.slusarz@gmail.com>
Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Peter Jones <pjones@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 drivers/gpu/drm/nouveau/nouveau_drv.h   |    2 +
 drivers/gpu/drm/nouveau/nouveau_fbcon.c |   19 +-------------
 drivers/gpu/drm/nouveau/nouveau_state.c |   43 +++++++++++++++++++++++++++++++
 drivers/video/fbmem.c                   |   43 ++++++++++++++++++-------------
 include/linux/fb.h                      |    1 +
 5 files changed, 72 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_drv.h b/drivers/gpu/drm/nouveau/nouveau_drv.h
index cb16a1b..a1f61f7 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drv.h
+++ b/drivers/gpu/drm/nouveau/nouveau_drv.h
@@ -621,6 +621,8 @@ struct drm_nouveau_private {
 	struct {
 		struct dentry *channel_root;
 	} debugfs;
+
+	struct apertures_struct *apertures;
 };
 
 static inline struct drm_nouveau_private *
diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
index 206368b..5e4367b 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
@@ -195,7 +195,6 @@ nouveau_fbcon_create(struct drm_device *dev, uint32_t fb_width,
 	struct drm_mode_fb_cmd mode_cmd;
 	struct pci_dev *pdev = dev->pdev;
 	struct device *device = &pdev->dev;
-	struct apertures_struct *aper;
 	int size, ret;
 
 	mode_cmd.width = surface_width;
@@ -282,28 +281,12 @@ nouveau_fbcon_create(struct drm_device *dev, uint32_t fb_width,
 	info->fix.mmio_len = pci_resource_len(pdev, 1);
 
 	/* Set aperture base/size for vesafb takeover */
-	aper = info->apertures = alloc_apertures(3);
+	info->apertures = dev_priv->apertures;
 	if (!info->apertures) {
 		ret = -ENOMEM;
 		goto out_unref;
 	}
 
-	aper->ranges[0].base = pci_resource_start(pdev, 1);
-	aper->ranges[0].size = pci_resource_len(pdev, 1);
-	aper->count = 1;
-
-	if (pci_resource_len(pdev, 2)) {
-		aper->ranges[aper->count].base = pci_resource_start(pdev, 2);
-		aper->ranges[aper->count].size = pci_resource_len(pdev, 2);
-		aper->count++;
-	}
-
-	if (pci_resource_len(pdev, 3)) {
-		aper->ranges[aper->count].base = pci_resource_start(pdev, 3);
-		aper->ranges[aper->count].size = pci_resource_len(pdev, 3);
-		aper->count++;
-	}
-
 	info->pixmap.size = 64*1024;
 	info->pixmap.buf_align = 8;
 	info->pixmap.access_align = 32;
diff --git a/drivers/gpu/drm/nouveau/nouveau_state.c b/drivers/gpu/drm/nouveau/nouveau_state.c
index a9e9cf3..cfa3239 100644
--- a/drivers/gpu/drm/nouveau/nouveau_state.c
+++ b/drivers/gpu/drm/nouveau/nouveau_state.c
@@ -636,6 +636,43 @@ static void nouveau_OF_copy_vbios_to_ramin(struct drm_device *dev)
 #endif
 }
 
+static struct apertures_struct *nouveau_get_apertures(struct drm_device *dev)
+{
+	struct pci_dev *pdev = dev->pdev;
+	struct apertures_struct *aper = alloc_apertures(3);
+	if (!aper)
+		return NULL;
+
+	aper->ranges[0].base = pci_resource_start(pdev, 1);
+	aper->ranges[0].size = pci_resource_len(pdev, 1);
+	aper->count = 1;
+
+	if (pci_resource_len(pdev, 2)) {
+		aper->ranges[aper->count].base = pci_resource_start(pdev, 2);
+		aper->ranges[aper->count].size = pci_resource_len(pdev, 2);
+		aper->count++;
+	}
+
+	if (pci_resource_len(pdev, 3)) {
+		aper->ranges[aper->count].base = pci_resource_start(pdev, 3);
+		aper->ranges[aper->count].size = pci_resource_len(pdev, 3);
+		aper->count++;
+	}
+
+	return aper;
+}
+
+static int nouveau_remove_conflicting_drivers(struct drm_device *dev)
+{
+	struct drm_nouveau_private *dev_priv = dev->dev_private;
+	dev_priv->apertures = nouveau_get_apertures(dev);
+	if (!dev_priv->apertures)
+		return -ENOMEM;
+	
+	remove_conflicting_framebuffers(dev_priv->apertures, "nouveaufb");
+	return 0;
+}
+
 int nouveau_load(struct drm_device *dev, unsigned long flags)
 {
 	struct drm_nouveau_private *dev_priv;
@@ -723,6 +760,12 @@ int nouveau_load(struct drm_device *dev, unsigned long flags)
 	NV_INFO(dev, "Detected an NV%2x generation card (0x%08x)\n",
 		dev_priv->card_type, reg0);
 
+	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
+		int ret = nouveau_remove_conflicting_drivers(dev);
+		if (ret)
+			return ret;
+	}
+
 	/* map larger RAMIN aperture on NV40 cards */
 	dev_priv->ramin  = NULL;
 	if (dev_priv->card_type >= NV_40) {
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index 8824d2c..f5297a0 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -1483,11 +1483,10 @@ static int fb_check_foreignness(struct fb_info *fi)
 	__start1 < __start2 + __size2 && __start1 + __size1 > __start2; \
 })
 
-static bool fb_do_apertures_overlap(struct fb_info *gen, struct fb_info *hw)
+static bool fb_do_apertures_overlap(struct apertures_struct *gena,
+				    struct apertures_struct *hwa)
 {
 	int i, j;
-	struct apertures_struct *hwa = hw->apertures;
-	struct apertures_struct *gena = gen->apertures;
 	if (!hwa || !gena)
 		return false;
 
@@ -1504,6 +1503,28 @@ static bool fb_do_apertures_overlap(struct fb_info *gen, struct fb_info *hw)
 
 	return false;
 }
+
+void remove_conflicting_framebuffers(struct apertures_struct *a, const char *name)
+{
+	int i;
+
+	/* check all firmware fbs and kick off if the base addr overlaps */
+	for (i = 0 ; i < FB_MAX; i++) {
+		if (!registered_fb[i])
+			continue;
+
+		if (!(registered_fb[i]->flags & FBINFO_MISC_FIRMWARE))
+			continue;
+
+		if (fb_do_apertures_overlap(registered_fb[i]->apertures, a)) {
+			printk(KERN_ERR "fb: conflicting fb hw usage "
+			       "%s vs %s - removing generic driver\n",
+			       name, registered_fb[i]->fix.id);
+			unregister_framebuffer(registered_fb[i]);
+		}
+	}
+}
+
 /**
  *	register_framebuffer - registers a frame buffer device
  *	@fb_info: frame buffer info structure
@@ -1527,21 +1548,7 @@ register_framebuffer(struct fb_info *fb_info)
 	if (fb_check_foreignness(fb_info))
 		return -ENOSYS;
 
-	/* check all firmware fbs and kick off if the base addr overlaps */
-	for (i = 0 ; i < FB_MAX; i++) {
-		if (!registered_fb[i])
-			continue;
-
-		if (registered_fb[i]->flags & FBINFO_MISC_FIRMWARE) {
-			if (fb_do_apertures_overlap(registered_fb[i], fb_info)) {
-				printk(KERN_ERR "fb: conflicting fb hw usage "
-				       "%s vs %s - removing generic driver\n",
-				       fb_info->fix.id,
-				       registered_fb[i]->fix.id);
-				unregister_framebuffer(registered_fb[i]);
-			}
-		}
-	}
+	remove_conflicting_framebuffers(fb_info->apertures, fb_info->fix.id);
 
 	num_registered_fb++;
 	for (i = 0 ; i < FB_MAX; i++)
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 9605b81..9e854a7 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -970,6 +970,7 @@ extern ssize_t fb_sys_write(struct fb_info *info, const char __user *buf,
 /* drivers/video/fbmem.c */
 extern int register_framebuffer(struct fb_info *fb_info);
 extern int unregister_framebuffer(struct fb_info *fb_info);
+extern void remove_conflicting_framebuffers(struct apertures_struct *a, const char *name);
 extern int fb_prepare_logo(struct fb_info *fb_info, int rotate);
 extern int fb_show_logo(struct fb_info *fb_info, int rotate);
 extern char* fb_get_buffer_offset(struct fb_info *info, struct fb_pixmap *buf, u32 size);
-- 
1.7.0.4

[PATCH 3/3] fbmem, drm/nouveau: kick firmware framebuffers as soon as possible

[marcin.slusarz]

Currently vesafb/efifb/... is kicked when hardware driver is registering
framebuffer. To do it hardware must be fully functional, so there's a short
window between start of initialisation and framebuffer registration when
two drivers touch the hardware. Unfortunately sometimes it breaks nouveau
initialisation.

Fix it by kicking firmware driver(s) before we start touching the hardware.

Reported-by: Didier Spaier <didier.spaier@epsm.fr>
Signed-off-by: Marcin Slusarz <marcin.slusarz@gmail.com>
Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Peter Jones <pjones@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 drivers/gpu/drm/nouveau/nouveau_drv.h   |    2 +
 drivers/gpu/drm/nouveau/nouveau_fbcon.c |   19 +-------------
 drivers/gpu/drm/nouveau/nouveau_state.c |   43 +++++++++++++++++++++++++++++++
 drivers/video/fbmem.c                   |   43 ++++++++++++++++++-------------
 include/linux/fb.h                      |    1 +
 5 files changed, 72 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_drv.h b/drivers/gpu/drm/nouveau/nouveau_drv.h
index cb16a1b..a1f61f7 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drv.h
+++ b/drivers/gpu/drm/nouveau/nouveau_drv.h
@@ -621,6 +621,8 @@ struct drm_nouveau_private {
 	struct {
 		struct dentry *channel_root;
 	} debugfs;
+
+	struct apertures_struct *apertures;
 };
 
 static inline struct drm_nouveau_private *
diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
index 206368b..5e4367b 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
@@ -195,7 +195,6 @@ nouveau_fbcon_create(struct drm_device *dev, uint32_t fb_width,
 	struct drm_mode_fb_cmd mode_cmd;
 	struct pci_dev *pdev = dev->pdev;
 	struct device *device = &pdev->dev;
-	struct apertures_struct *aper;
 	int size, ret;
 
 	mode_cmd.width = surface_width;
@@ -282,28 +281,12 @@ nouveau_fbcon_create(struct drm_device *dev, uint32_t fb_width,
 	info->fix.mmio_len = pci_resource_len(pdev, 1);
 
 	/* Set aperture base/size for vesafb takeover */
-	aper = info->apertures = alloc_apertures(3);
+	info->apertures = dev_priv->apertures;
 	if (!info->apertures) {
 		ret = -ENOMEM;
 		goto out_unref;
 	}
 
-	aper->ranges[0].base = pci_resource_start(pdev, 1);
-	aper->ranges[0].size = pci_resource_len(pdev, 1);
-	aper->count = 1;
-
-	if (pci_resource_len(pdev, 2)) {
-		aper->ranges[aper->count].base = pci_resource_start(pdev, 2);
-		aper->ranges[aper->count].size = pci_resource_len(pdev, 2);
-		aper->count++;
-	}
-
-	if (pci_resource_len(pdev, 3)) {
-		aper->ranges[aper->count].base = pci_resource_start(pdev, 3);
-		aper->ranges[aper->count].size = pci_resource_len(pdev, 3);
-		aper->count++;
-	}
-
 	info->pixmap.size = 64*1024;
 	info->pixmap.buf_align = 8;
 	info->pixmap.access_align = 32;
diff --git a/drivers/gpu/drm/nouveau/nouveau_state.c b/drivers/gpu/drm/nouveau/nouveau_state.c
index a9e9cf3..cfa3239 100644
--- a/drivers/gpu/drm/nouveau/nouveau_state.c
+++ b/drivers/gpu/drm/nouveau/nouveau_state.c
@@ -636,6 +636,43 @@ static void nouveau_OF_copy_vbios_to_ramin(struct drm_device *dev)
 #endif
 }
 
+static struct apertures_struct *nouveau_get_apertures(struct drm_device *dev)
+{
+	struct pci_dev *pdev = dev->pdev;
+	struct apertures_struct *aper = alloc_apertures(3);
+	if (!aper)
+		return NULL;
+
+	aper->ranges[0].base = pci_resource_start(pdev, 1);
+	aper->ranges[0].size = pci_resource_len(pdev, 1);
+	aper->count = 1;
+
+	if (pci_resource_len(pdev, 2)) {
+		aper->ranges[aper->count].base = pci_resource_start(pdev, 2);
+		aper->ranges[aper->count].size = pci_resource_len(pdev, 2);
+		aper->count++;
+	}
+
+	if (pci_resource_len(pdev, 3)) {
+		aper->ranges[aper->count].base = pci_resource_start(pdev, 3);
+		aper->ranges[aper->count].size = pci_resource_len(pdev, 3);
+		aper->count++;
+	}
+
+	return aper;
+}
+
+static int nouveau_remove_conflicting_drivers(struct drm_device *dev)
+{
+	struct drm_nouveau_private *dev_priv = dev->dev_private;
+	dev_priv->apertures = nouveau_get_apertures(dev);
+	if (!dev_priv->apertures)
+		return -ENOMEM;
+	
+	remove_conflicting_framebuffers(dev_priv->apertures, "nouveaufb");
+	return 0;
+}
+
 int nouveau_load(struct drm_device *dev, unsigned long flags)
 {
 	struct drm_nouveau_private *dev_priv;
@@ -723,6 +760,12 @@ int nouveau_load(struct drm_device *dev, unsigned long flags)
 	NV_INFO(dev, "Detected an NV%2x generation card (0x%08x)\n",
 		dev_priv->card_type, reg0);
 
+	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
+		int ret = nouveau_remove_conflicting_drivers(dev);
+		if (ret)
+			return ret;
+	}
+
 	/* map larger RAMIN aperture on NV40 cards */
 	dev_priv->ramin  = NULL;
 	if (dev_priv->card_type >= NV_40) {
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index 8824d2c..f5297a0 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -1483,11 +1483,10 @@ static int fb_check_foreignness(struct fb_info *fi)
 	__start1 < __start2 + __size2 && __start1 + __size1 > __start2; \
 })
 
-static bool fb_do_apertures_overlap(struct fb_info *gen, struct fb_info *hw)
+static bool fb_do_apertures_overlap(struct apertures_struct *gena,
+				    struct apertures_struct *hwa)
 {
 	int i, j;
-	struct apertures_struct *hwa = hw->apertures;
-	struct apertures_struct *gena = gen->apertures;
 	if (!hwa || !gena)
 		return false;
 
@@ -1504,6 +1503,28 @@ static bool fb_do_apertures_overlap(struct fb_info *gen, struct fb_info *hw)
 
 	return false;
 }
+
+void remove_conflicting_framebuffers(struct apertures_struct *a, const char *name)
+{
+	int i;
+
+	/* check all firmware fbs and kick off if the base addr overlaps */
+	for (i = 0 ; i < FB_MAX; i++) {
+		if (!registered_fb[i])
+			continue;
+
+		if (!(registered_fb[i]->flags & FBINFO_MISC_FIRMWARE))
+			continue;
+
+		if (fb_do_apertures_overlap(registered_fb[i]->apertures, a)) {
+			printk(KERN_ERR "fb: conflicting fb hw usage "
+			       "%s vs %s - removing generic driver\n",
+			       name, registered_fb[i]->fix.id);
+			unregister_framebuffer(registered_fb[i]);
+		}
+	}
+}
+
 /**
  *	register_framebuffer - registers a frame buffer device
  *	@fb_info: frame buffer info structure
@@ -1527,21 +1548,7 @@ register_framebuffer(struct fb_info *fb_info)
 	if (fb_check_foreignness(fb_info))
 		return -ENOSYS;
 
-	/* check all firmware fbs and kick off if the base addr overlaps */
-	for (i = 0 ; i < FB_MAX; i++) {
-		if (!registered_fb[i])
-			continue;
-
-		if (registered_fb[i]->flags & FBINFO_MISC_FIRMWARE) {
-			if (fb_do_apertures_overlap(registered_fb[i], fb_info)) {
-				printk(KERN_ERR "fb: conflicting fb hw usage "
-				       "%s vs %s - removing generic driver\n",
-				       fb_info->fix.id,
-				       registered_fb[i]->fix.id);
-				unregister_framebuffer(registered_fb[i]);
-			}
-		}
-	}
+	remove_conflicting_framebuffers(fb_info->apertures, fb_info->fix.id);
 
 	num_registered_fb++;
 	for (i = 0 ; i < FB_MAX; i++)
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 9605b81..9e854a7 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -970,6 +970,7 @@ extern ssize_t fb_sys_write(struct fb_info *info, const char __user *buf,
 /* drivers/video/fbmem.c */
 extern int register_framebuffer(struct fb_info *fb_info);
 extern int unregister_framebuffer(struct fb_info *fb_info);
+extern void remove_conflicting_framebuffers(struct apertures_struct *a, const char *name);
 extern int fb_prepare_logo(struct fb_info *fb_info, int rotate);
 extern int fb_show_logo(struct fb_info *fb_info, int rotate);
 extern char* fb_get_buffer_offset(struct fb_info *info, struct fb_pixmap *buf, u32 size);
-- 
1.7.0.4

[PATCH 3/3] fbmem, drm/nouveau: kick firmware framebuffers as soon as possible

[marcin.slusarz-Re5JQEeQqe8AvxtiuMwx3w]

Currently vesafb/efifb/... is kicked when hardware driver is registering
framebuffer. To do it hardware must be fully functional, so there's a short
window between start of initialisation and framebuffer registration when
two drivers touch the hardware. Unfortunately sometimes it breaks nouveau
initialisation.

Fix it by kicking firmware driver(s) before we start touching the hardware.

Reported-by: Didier Spaier <didier.spaier-GqzDj6/B2j8@public.gmane.org>
Signed-off-by: Marcin Slusarz <marcin.slusarz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Ben Skeggs <bskeggs-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: Dave Airlie <airlied-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: Peter Jones <pjones-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
---
 drivers/gpu/drm/nouveau/nouveau_drv.h   |    2 +
 drivers/gpu/drm/nouveau/nouveau_fbcon.c |   19 +-------------
 drivers/gpu/drm/nouveau/nouveau_state.c |   43 +++++++++++++++++++++++++++++++
 drivers/video/fbmem.c                   |   43 ++++++++++++++++++-------------
 include/linux/fb.h                      |    1 +
 5 files changed, 72 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_drv.h b/drivers/gpu/drm/nouveau/nouveau_drv.h
index cb16a1b..a1f61f7 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drv.h
+++ b/drivers/gpu/drm/nouveau/nouveau_drv.h
@@ -621,6 +621,8 @@ struct drm_nouveau_private {
 	struct {
 		struct dentry *channel_root;
 	} debugfs;
+
+	struct apertures_struct *apertures;
 };
 
 static inline struct drm_nouveau_private *
diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
index 206368b..5e4367b 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
@@ -195,7 +195,6 @@ nouveau_fbcon_create(struct drm_device *dev, uint32_t fb_width,
 	struct drm_mode_fb_cmd mode_cmd;
 	struct pci_dev *pdev = dev->pdev;
 	struct device *device = &pdev->dev;
-	struct apertures_struct *aper;
 	int size, ret;
 
 	mode_cmd.width = surface_width;
@@ -282,28 +281,12 @@ nouveau_fbcon_create(struct drm_device *dev, uint32_t fb_width,
 	info->fix.mmio_len = pci_resource_len(pdev, 1);
 
 	/* Set aperture base/size for vesafb takeover */
-	aper = info->apertures = alloc_apertures(3);
+	info->apertures = dev_priv->apertures;
 	if (!info->apertures) {
 		ret = -ENOMEM;
 		goto out_unref;
 	}
 
-	aper->ranges[0].base = pci_resource_start(pdev, 1);
-	aper->ranges[0].size = pci_resource_len(pdev, 1);
-	aper->count = 1;
-
-	if (pci_resource_len(pdev, 2)) {
-		aper->ranges[aper->count].base = pci_resource_start(pdev, 2);
-		aper->ranges[aper->count].size = pci_resource_len(pdev, 2);
-		aper->count++;
-	}
-
-	if (pci_resource_len(pdev, 3)) {
-		aper->ranges[aper->count].base = pci_resource_start(pdev, 3);
-		aper->ranges[aper->count].size = pci_resource_len(pdev, 3);
-		aper->count++;
-	}
-
 	info->pixmap.size = 64*1024;
 	info->pixmap.buf_align = 8;
 	info->pixmap.access_align = 32;
diff --git a/drivers/gpu/drm/nouveau/nouveau_state.c b/drivers/gpu/drm/nouveau/nouveau_state.c
index a9e9cf3..cfa3239 100644
--- a/drivers/gpu/drm/nouveau/nouveau_state.c
+++ b/drivers/gpu/drm/nouveau/nouveau_state.c
@@ -636,6 +636,43 @@ static void nouveau_OF_copy_vbios_to_ramin(struct drm_device *dev)
 #endif
 }
 
+static struct apertures_struct *nouveau_get_apertures(struct drm_device *dev)
+{
+	struct pci_dev *pdev = dev->pdev;
+	struct apertures_struct *aper = alloc_apertures(3);
+	if (!aper)
+		return NULL;
+
+	aper->ranges[0].base = pci_resource_start(pdev, 1);
+	aper->ranges[0].size = pci_resource_len(pdev, 1);
+	aper->count = 1;
+
+	if (pci_resource_len(pdev, 2)) {
+		aper->ranges[aper->count].base = pci_resource_start(pdev, 2);
+		aper->ranges[aper->count].size = pci_resource_len(pdev, 2);
+		aper->count++;
+	}
+
+	if (pci_resource_len(pdev, 3)) {
+		aper->ranges[aper->count].base = pci_resource_start(pdev, 3);
+		aper->ranges[aper->count].size = pci_resource_len(pdev, 3);
+		aper->count++;
+	}
+
+	return aper;
+}
+
+static int nouveau_remove_conflicting_drivers(struct drm_device *dev)
+{
+	struct drm_nouveau_private *dev_priv = dev->dev_private;
+	dev_priv->apertures = nouveau_get_apertures(dev);
+	if (!dev_priv->apertures)
+		return -ENOMEM;
+	
+	remove_conflicting_framebuffers(dev_priv->apertures, "nouveaufb");
+	return 0;
+}
+
 int nouveau_load(struct drm_device *dev, unsigned long flags)
 {
 	struct drm_nouveau_private *dev_priv;
@@ -723,6 +760,12 @@ int nouveau_load(struct drm_device *dev, unsigned long flags)
 	NV_INFO(dev, "Detected an NV%2x generation card (0x%08x)\n",
 		dev_priv->card_type, reg0);
 
+	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
+		int ret = nouveau_remove_conflicting_drivers(dev);
+		if (ret)
+			return ret;
+	}
+
 	/* map larger RAMIN aperture on NV40 cards */
 	dev_priv->ramin  = NULL;
 	if (dev_priv->card_type >= NV_40) {
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index 8824d2c..f5297a0 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -1483,11 +1483,10 @@ static int fb_check_foreignness(struct fb_info *fi)
 	__start1 < __start2 + __size2 && __start1 + __size1 > __start2; \
 })
 
-static bool fb_do_apertures_overlap(struct fb_info *gen, struct fb_info *hw)
+static bool fb_do_apertures_overlap(struct apertures_struct *gena,
+				    struct apertures_struct *hwa)
 {
 	int i, j;
-	struct apertures_struct *hwa = hw->apertures;
-	struct apertures_struct *gena = gen->apertures;
 	if (!hwa || !gena)
 		return false;
 
@@ -1504,6 +1503,28 @@ static bool fb_do_apertures_overlap(struct fb_info *gen, struct fb_info *hw)
 
 	return false;
 }
+
+void remove_conflicting_framebuffers(struct apertures_struct *a, const char *name)
+{
+	int i;
+
+	/* check all firmware fbs and kick off if the base addr overlaps */
+	for (i = 0 ; i < FB_MAX; i++) {
+		if (!registered_fb[i])
+			continue;
+
+		if (!(registered_fb[i]->flags & FBINFO_MISC_FIRMWARE))
+			continue;
+
+		if (fb_do_apertures_overlap(registered_fb[i]->apertures, a)) {
+			printk(KERN_ERR "fb: conflicting fb hw usage "
+			       "%s vs %s - removing generic driver\n",
+			       name, registered_fb[i]->fix.id);
+			unregister_framebuffer(registered_fb[i]);
+		}
+	}
+}
+
 /**
  *	register_framebuffer - registers a frame buffer device
  *	@fb_info: frame buffer info structure
@@ -1527,21 +1548,7 @@ register_framebuffer(struct fb_info *fb_info)
 	if (fb_check_foreignness(fb_info))
 		return -ENOSYS;
 
-	/* check all firmware fbs and kick off if the base addr overlaps */
-	for (i = 0 ; i < FB_MAX; i++) {
-		if (!registered_fb[i])
-			continue;
-
-		if (registered_fb[i]->flags & FBINFO_MISC_FIRMWARE) {
-			if (fb_do_apertures_overlap(registered_fb[i], fb_info)) {
-				printk(KERN_ERR "fb: conflicting fb hw usage "
-				       "%s vs %s - removing generic driver\n",
-				       fb_info->fix.id,
-				       registered_fb[i]->fix.id);
-				unregister_framebuffer(registered_fb[i]);
-			}
-		}
-	}
+	remove_conflicting_framebuffers(fb_info->apertures, fb_info->fix.id);
 
 	num_registered_fb++;
 	for (i = 0 ; i < FB_MAX; i++)
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 9605b81..9e854a7 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -970,6 +970,7 @@ extern ssize_t fb_sys_write(struct fb_info *info, const char __user *buf,
 /* drivers/video/fbmem.c */
 extern int register_framebuffer(struct fb_info *fb_info);
 extern int unregister_framebuffer(struct fb_info *fb_info);
+extern void remove_conflicting_framebuffers(struct apertures_struct *a, const char *name);
 extern int fb_prepare_logo(struct fb_info *fb_info, int rotate);
 extern int fb_show_logo(struct fb_info *fb_info, int rotate);
 extern char* fb_get_buffer_offset(struct fb_info *info, struct fb_pixmap *buf, u32 size);
-- 
1.7.0.4

Re: [PATCH 3/3] fbmem, drm/nouveau: kick firmware framebuffers as

[Marcin Slusarz]

On Sat, Apr 10, 2010 at 09:55:34PM +0200, marcin.slusarz@gmail.com wrote:
> Currently vesafb/efifb/... is kicked when hardware driver is registering
> framebuffer. To do it hardware must be fully functional, so there's a short
> window between start of initialisation and framebuffer registration when
> two drivers touch the hardware. Unfortunately sometimes it breaks nouveau
> initialisation.
> 
> Fix it by kicking firmware driver(s) before we start touching the hardware.
> 
> Reported-by: Didier Spaier <didier.spaier@epsm.fr>
> Signed-off-by: Marcin Slusarz <marcin.slusarz@gmail.com>
> Cc: Ben Skeggs <bskeggs@redhat.com>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Peter Jones <pjones@redhat.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> ---

With this little patch below:
Tested-by: Didier Spaier <didier.spaier@epsm.fr>

---
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index f5297a0..42ae782 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -1524,6 +1524,7 @@ void remove_conflicting_framebuffers(struct apertures_struct *a, const char *nam
 		}
 	}
 }
+EXPORT_SYMBOL(remove_conflicting_framebuffers);
 
 /**
  *	register_framebuffer - registers a frame buffer device

Re: [PATCH 3/3] fbmem, drm/nouveau: kick firmware framebuffers as soon as possible

[Marcin Slusarz]

On Sat, Apr 10, 2010 at 09:55:34PM +0200, marcin.slusarz@gmail.com wrote:
> Currently vesafb/efifb/... is kicked when hardware driver is registering
> framebuffer. To do it hardware must be fully functional, so there's a short
> window between start of initialisation and framebuffer registration when
> two drivers touch the hardware. Unfortunately sometimes it breaks nouveau
> initialisation.
> 
> Fix it by kicking firmware driver(s) before we start touching the hardware.
> 
> Reported-by: Didier Spaier <didier.spaier@epsm.fr>
> Signed-off-by: Marcin Slusarz <marcin.slusarz@gmail.com>
> Cc: Ben Skeggs <bskeggs@redhat.com>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Peter Jones <pjones@redhat.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> ---

With this little patch below:
Tested-by: Didier Spaier <didier.spaier@epsm.fr>

---
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index f5297a0..42ae782 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -1524,6 +1524,7 @@ void remove_conflicting_framebuffers(struct apertures_struct *a, const char *nam
 		}
 	}
 }
+EXPORT_SYMBOL(remove_conflicting_framebuffers);
 
 /**
  *	register_framebuffer - registers a frame buffer device

Re: [PATCH 3/3] fbmem, drm/nouveau: kick firmware framebuffers as soon as possible

[Marcin Slusarz]

On Sat, Apr 10, 2010 at 09:55:34PM +0200, marcin.slusarz@gmail.com wrote:
> Currently vesafb/efifb/... is kicked when hardware driver is registering
> framebuffer. To do it hardware must be fully functional, so there's a short
> window between start of initialisation and framebuffer registration when
> two drivers touch the hardware. Unfortunately sometimes it breaks nouveau
> initialisation.
> 
> Fix it by kicking firmware driver(s) before we start touching the hardware.
> 
> Reported-by: Didier Spaier <didier.spaier@epsm.fr>
> Signed-off-by: Marcin Slusarz <marcin.slusarz@gmail.com>
> Cc: Ben Skeggs <bskeggs@redhat.com>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Peter Jones <pjones@redhat.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> ---

With this little patch below:
Tested-by: Didier Spaier <didier.spaier@epsm.fr>

---
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index f5297a0..42ae782 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -1524,6 +1524,7 @@ void remove_conflicting_framebuffers(struct apertures_struct *a, const char *nam
 		}
 	}
 }
+EXPORT_SYMBOL(remove_conflicting_framebuffers);
 
 /**
  *	register_framebuffer - registers a frame buffer device

Re: [PATCH 1/3] fbmem: fix aperture overlapping check

[Dave Airlie]

On Sat, 2010-04-10 at 21:55 +0200, marcin.slusarz@gmail.com wrote:
> fb_do_apertures_overlap is returning wrong value when one aperture
> is completely whithin the other. Add generic ranges_overlap macro
> (probably kernel.h candidate) and use it here.
> 

That doesn't seem right.

The rules are:

the generic aperture has to be equal or smaller than the hw aperture,
otherwise the generic driver will be trashing random hw pieces on the
machine.

So with that in mind, the check makes sure the generic aperture starts
somewhere inside the hw aperture, which the test clearly gets right.

Have you got a pointer to a machine where it fails?

Dave.


> Signed-off-by: Marcin Slusarz <marcin.slusarz@gmail.com>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Peter Jones <pjones@redhat.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> ---
>  drivers/video/fbmem.c |   24 +++++++++++++++++-------
>  1 files changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
> index a15b44e..41f2e5e 100644
> --- a/drivers/video/fbmem.c
> +++ b/drivers/video/fbmem.c
> @@ -1468,15 +1468,25 @@ static int fb_check_foreignness(struct fb_info *fi)
>  	return 0;
>  }
>  
> +/**
> + * ranges_overlap - check whether two ranges overlap (their intersection is not empty)
> + * @start1: start of the first range
> + * @size1:  length of the first range
> + * @start2: start of the second range
> + * @size2:  length of the second range
> + */
> +#define ranges_overlap(start1, size1, start2, size2) ({	\
> +	typeof(start1) __start1 = (start1);		\
> +	typeof(size1)  __size1  = (size1);		\
> +	typeof(start2) __start2 = (start2);		\
> +	typeof(size2)  __size2  = (size2);		\
> +	__start1 < __start2 + __size2 && __start1 + __size1 > __start2; \
> +})
> +
>  static bool fb_do_apertures_overlap(struct fb_info *gen, struct fb_info *hw)
>  {
> -	/* is the generic aperture base the same as the HW one */
> -	if (gen->aperture_base = hw->aperture_base)
> -		return true;
> -	/* is the generic aperture base inside the hw base->hw base+size */
> -	if (gen->aperture_base > hw->aperture_base && gen->aperture_base <= hw->aperture_base + hw->aperture_size)
> -		return true;
> -	return false;
> +	return ranges_overlap(gen->aperture_base, gen->aperture_size,
> +				hw->aperture_base, hw->aperture_size);
>  }
>  /**
>   *	register_framebuffer - registers a frame buffer device

Re: [PATCH 1/3] fbmem: fix aperture overlapping check

[Dave Airlie]

On Sat, 2010-04-10 at 21:55 +0200, marcin.slusarz@gmail.com wrote:
> fb_do_apertures_overlap is returning wrong value when one aperture
> is completely whithin the other. Add generic ranges_overlap macro
> (probably kernel.h candidate) and use it here.
> 

That doesn't seem right.

The rules are:

the generic aperture has to be equal or smaller than the hw aperture,
otherwise the generic driver will be trashing random hw pieces on the
machine.

So with that in mind, the check makes sure the generic aperture starts
somewhere inside the hw aperture, which the test clearly gets right.

Have you got a pointer to a machine where it fails?

Dave.


> Signed-off-by: Marcin Slusarz <marcin.slusarz@gmail.com>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Peter Jones <pjones@redhat.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> ---
>  drivers/video/fbmem.c |   24 +++++++++++++++++-------
>  1 files changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
> index a15b44e..41f2e5e 100644
> --- a/drivers/video/fbmem.c
> +++ b/drivers/video/fbmem.c
> @@ -1468,15 +1468,25 @@ static int fb_check_foreignness(struct fb_info *fi)
>  	return 0;
>  }
>  
> +/**
> + * ranges_overlap - check whether two ranges overlap (their intersection is not empty)
> + * @start1: start of the first range
> + * @size1:  length of the first range
> + * @start2: start of the second range
> + * @size2:  length of the second range
> + */
> +#define ranges_overlap(start1, size1, start2, size2) ({	\
> +	typeof(start1) __start1 = (start1);		\
> +	typeof(size1)  __size1  = (size1);		\
> +	typeof(start2) __start2 = (start2);		\
> +	typeof(size2)  __size2  = (size2);		\
> +	__start1 < __start2 + __size2 && __start1 + __size1 > __start2; \
> +})
> +
>  static bool fb_do_apertures_overlap(struct fb_info *gen, struct fb_info *hw)
>  {
> -	/* is the generic aperture base the same as the HW one */
> -	if (gen->aperture_base == hw->aperture_base)
> -		return true;
> -	/* is the generic aperture base inside the hw base->hw base+size */
> -	if (gen->aperture_base > hw->aperture_base && gen->aperture_base <= hw->aperture_base + hw->aperture_size)
> -		return true;
> -	return false;
> +	return ranges_overlap(gen->aperture_base, gen->aperture_size,
> +				hw->aperture_base, hw->aperture_size);
>  }
>  /**
>   *	register_framebuffer - registers a frame buffer device

Re: [PATCH 1/3] fbmem: fix aperture overlapping check

[Marcin Slusarz]

On Mon, Apr 12, 2010 at 09:54:28AM +1000, Dave Airlie wrote:
> On Sat, 2010-04-10 at 21:55 +0200, marcin.slusarz@gmail.com wrote:
> > fb_do_apertures_overlap is returning wrong value when one aperture
> > is completely whithin the other. Add generic ranges_overlap macro
> > (probably kernel.h candidate) and use it here.
> > 
> 
> That doesn't seem right.
> 
> The rules are:
> 
> the generic aperture has to be equal or smaller than the hw aperture,
> otherwise the generic driver will be trashing random hw pieces on the
> machine.

Why "it has to"? Why generic aperture can't start one byte before hw?

> So with that in mind, the check makes sure the generic aperture starts
> somewhere inside the hw aperture, which the test clearly gets right.

So your only objection is that it's impossible with the current code?

> Have you got a pointer to a machine where it fails?

No, it failed with an artifical test while I was working on vga16fb handoff
(unfinished).

> Dave.
> 
> 
> > Signed-off-by: Marcin Slusarz <marcin.slusarz@gmail.com>
> > Cc: Dave Airlie <airlied@redhat.com>
> > Cc: Peter Jones <pjones@redhat.com>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > ---
> >  drivers/video/fbmem.c |   24 +++++++++++++++++-------
> >  1 files changed, 17 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
> > index a15b44e..41f2e5e 100644
> > --- a/drivers/video/fbmem.c
> > +++ b/drivers/video/fbmem.c
> > @@ -1468,15 +1468,25 @@ static int fb_check_foreignness(struct fb_info *fi)
> >  	return 0;
> >  }
> >  
> > +/**
> > + * ranges_overlap - check whether two ranges overlap (their intersection is not empty)
> > + * @start1: start of the first range
> > + * @size1:  length of the first range
> > + * @start2: start of the second range
> > + * @size2:  length of the second range
> > + */
> > +#define ranges_overlap(start1, size1, start2, size2) ({	\
> > +	typeof(start1) __start1 = (start1);		\
> > +	typeof(size1)  __size1  = (size1);		\
> > +	typeof(start2) __start2 = (start2);		\
> > +	typeof(size2)  __size2  = (size2);		\
> > +	__start1 < __start2 + __size2 && __start1 + __size1 > __start2; \
> > +})
> > +
> >  static bool fb_do_apertures_overlap(struct fb_info *gen, struct fb_info *hw)
> >  {
> > -	/* is the generic aperture base the same as the HW one */
> > -	if (gen->aperture_base == hw->aperture_base)
> > -		return true;
> > -	/* is the generic aperture base inside the hw base->hw base+size */
> > -	if (gen->aperture_base > hw->aperture_base && gen->aperture_base <= hw->aperture_base + hw->aperture_size)
> > -		return true;
> > -	return false;
> > +	return ranges_overlap(gen->aperture_base, gen->aperture_size,
> > +				hw->aperture_base, hw->aperture_size);
> >  }
> >  /**
> >   *	register_framebuffer - registers a frame buffer device
> 
> 

Re: [PATCH 1/3] fbmem: fix aperture overlapping check

[Marcin Slusarz]

On Mon, Apr 12, 2010 at 09:54:28AM +1000, Dave Airlie wrote:
> On Sat, 2010-04-10 at 21:55 +0200, marcin.slusarz@gmail.com wrote:
> > fb_do_apertures_overlap is returning wrong value when one aperture
> > is completely whithin the other. Add generic ranges_overlap macro
> > (probably kernel.h candidate) and use it here.
> > 
> 
> That doesn't seem right.
> 
> The rules are:
> 
> the generic aperture has to be equal or smaller than the hw aperture,
> otherwise the generic driver will be trashing random hw pieces on the
> machine.

Why "it has to"? Why generic aperture can't start one byte before hw?

> So with that in mind, the check makes sure the generic aperture starts
> somewhere inside the hw aperture, which the test clearly gets right.

So your only objection is that it's impossible with the current code?

> Have you got a pointer to a machine where it fails?

No, it failed with an artifical test while I was working on vga16fb handoff
(unfinished).

> Dave.
> 
> 
> > Signed-off-by: Marcin Slusarz <marcin.slusarz@gmail.com>
> > Cc: Dave Airlie <airlied@redhat.com>
> > Cc: Peter Jones <pjones@redhat.com>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > ---
> >  drivers/video/fbmem.c |   24 +++++++++++++++++-------
> >  1 files changed, 17 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
> > index a15b44e..41f2e5e 100644
> > --- a/drivers/video/fbmem.c
> > +++ b/drivers/video/fbmem.c
> > @@ -1468,15 +1468,25 @@ static int fb_check_foreignness(struct fb_info *fi)
> >  	return 0;
> >  }
> >  
> > +/**
> > + * ranges_overlap - check whether two ranges overlap (their intersection is not empty)
> > + * @start1: start of the first range
> > + * @size1:  length of the first range
> > + * @start2: start of the second range
> > + * @size2:  length of the second range
> > + */
> > +#define ranges_overlap(start1, size1, start2, size2) ({	\
> > +	typeof(start1) __start1 = (start1);		\
> > +	typeof(size1)  __size1  = (size1);		\
> > +	typeof(start2) __start2 = (start2);		\
> > +	typeof(size2)  __size2  = (size2);		\
> > +	__start1 < __start2 + __size2 && __start1 + __size1 > __start2; \
> > +})
> > +
> >  static bool fb_do_apertures_overlap(struct fb_info *gen, struct fb_info *hw)
> >  {
> > -	/* is the generic aperture base the same as the HW one */
> > -	if (gen->aperture_base == hw->aperture_base)
> > -		return true;
> > -	/* is the generic aperture base inside the hw base->hw base+size */
> > -	if (gen->aperture_base > hw->aperture_base && gen->aperture_base <= hw->aperture_base + hw->aperture_size)
> > -		return true;
> > -	return false;
> > +	return ranges_overlap(gen->aperture_base, gen->aperture_size,
> > +				hw->aperture_base, hw->aperture_size);
> >  }
> >  /**
> >   *	register_framebuffer - registers a frame buffer device
> 
> 

Re: [PATCH 1/3] fbmem: fix aperture overlapping check

[Marcin Slusarz]

On Mon, Apr 12, 2010 at 09:54:28AM +1000, Dave Airlie wrote:
> On Sat, 2010-04-10 at 21:55 +0200, marcin.slusarz@gmail.com wrote:
> > fb_do_apertures_overlap is returning wrong value when one aperture
> > is completely whithin the other. Add generic ranges_overlap macro
> > (probably kernel.h candidate) and use it here.
> > 
> 
> That doesn't seem right.
> 
> The rules are:
> 
> the generic aperture has to be equal or smaller than the hw aperture,
> otherwise the generic driver will be trashing random hw pieces on the
> machine.

Why "it has to"? Why generic aperture can't start one byte before hw?

> So with that in mind, the check makes sure the generic aperture starts
> somewhere inside the hw aperture, which the test clearly gets right.

So your only objection is that it's impossible with the current code?

> Have you got a pointer to a machine where it fails?

No, it failed with an artifical test while I was working on vga16fb handoff
(unfinished).

> Dave.
> 
> 
> > Signed-off-by: Marcin Slusarz <marcin.slusarz@gmail.com>
> > Cc: Dave Airlie <airlied@redhat.com>
> > Cc: Peter Jones <pjones@redhat.com>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > ---
> >  drivers/video/fbmem.c |   24 +++++++++++++++++-------
> >  1 files changed, 17 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
> > index a15b44e..41f2e5e 100644
> > --- a/drivers/video/fbmem.c
> > +++ b/drivers/video/fbmem.c
> > @@ -1468,15 +1468,25 @@ static int fb_check_foreignness(struct fb_info *fi)
> >  	return 0;
> >  }
> >  
> > +/**
> > + * ranges_overlap - check whether two ranges overlap (their intersection is not empty)
> > + * @start1: start of the first range
> > + * @size1:  length of the first range
> > + * @start2: start of the second range
> > + * @size2:  length of the second range
> > + */
> > +#define ranges_overlap(start1, size1, start2, size2) ({	\
> > +	typeof(start1) __start1 = (start1);		\
> > +	typeof(size1)  __size1  = (size1);		\
> > +	typeof(start2) __start2 = (start2);		\
> > +	typeof(size2)  __size2  = (size2);		\
> > +	__start1 < __start2 + __size2 && __start1 + __size1 > __start2; \
> > +})
> > +
> >  static bool fb_do_apertures_overlap(struct fb_info *gen, struct fb_info *hw)
> >  {
> > -	/* is the generic aperture base the same as the HW one */
> > -	if (gen->aperture_base = hw->aperture_base)
> > -		return true;
> > -	/* is the generic aperture base inside the hw base->hw base+size */
> > -	if (gen->aperture_base > hw->aperture_base && gen->aperture_base <= hw->aperture_base + hw->aperture_size)
> > -		return true;
> > -	return false;
> > +	return ranges_overlap(gen->aperture_base, gen->aperture_size,
> > +				hw->aperture_base, hw->aperture_size);
> >  }
> >  /**
> >   *	register_framebuffer - registers a frame buffer device
> 
> 

Re: [PATCH 1/3] fbmem: fix aperture overlapping check

[Marcin Slusarz]

On Mon, Apr 12, 2010 at 09:54:28AM +1000, Dave Airlie wrote:
> On Sat, 2010-04-10 at 21:55 +0200, marcin.slusarz@gmail.com wrote:
> > fb_do_apertures_overlap is returning wrong value when one aperture
> > is completely whithin the other. Add generic ranges_overlap macro
> > (probably kernel.h candidate) and use it here.
> > 
> 
> That doesn't seem right.
> 
> The rules are:
> 
> the generic aperture has to be equal or smaller than the hw aperture,
> otherwise the generic driver will be trashing random hw pieces on the
> machine.

Why "it has to"? Why generic aperture can't start one byte before hw?

> So with that in mind, the check makes sure the generic aperture starts
> somewhere inside the hw aperture, which the test clearly gets right.

So your only objection is that it's impossible with the current code?

> Have you got a pointer to a machine where it fails?

No, it failed with an artifical test while I was working on vga16fb handoff
(unfinished).

> Dave.
> 
> 
> > Signed-off-by: Marcin Slusarz <marcin.slusarz@gmail.com>
> > Cc: Dave Airlie <airlied@redhat.com>
> > Cc: Peter Jones <pjones@redhat.com>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > ---
> >  drivers/video/fbmem.c |   24 +++++++++++++++++-------
> >  1 files changed, 17 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
> > index a15b44e..41f2e5e 100644
> > --- a/drivers/video/fbmem.c
> > +++ b/drivers/video/fbmem.c
> > @@ -1468,15 +1468,25 @@ static int fb_check_foreignness(struct fb_info *fi)
> >  	return 0;
> >  }
> >  
> > +/**
> > + * ranges_overlap - check whether two ranges overlap (their intersection is not empty)
> > + * @start1: start of the first range
> > + * @size1:  length of the first range
> > + * @start2: start of the second range
> > + * @size2:  length of the second range
> > + */
> > +#define ranges_overlap(start1, size1, start2, size2) ({	\
> > +	typeof(start1) __start1 = (start1);		\
> > +	typeof(size1)  __size1  = (size1);		\
> > +	typeof(start2) __start2 = (start2);		\
> > +	typeof(size2)  __size2  = (size2);		\
> > +	__start1 < __start2 + __size2 && __start1 + __size1 > __start2; \
> > +})
> > +
> >  static bool fb_do_apertures_overlap(struct fb_info *gen, struct fb_info *hw)
> >  {
> > -	/* is the generic aperture base the same as the HW one */
> > -	if (gen->aperture_base == hw->aperture_base)
> > -		return true;
> > -	/* is the generic aperture base inside the hw base->hw base+size */
> > -	if (gen->aperture_base > hw->aperture_base && gen->aperture_base <= hw->aperture_base + hw->aperture_size)
> > -		return true;
> > -	return false;
> > +	return ranges_overlap(gen->aperture_base, gen->aperture_size,
> > +				hw->aperture_base, hw->aperture_size);
> >  }
> >  /**
> >   *	register_framebuffer - registers a frame buffer device
> 
> 

Re: [PATCH 1/3] fbmem: fix aperture overlapping check

[Marcin Slusarz]

On Mon, Apr 12, 2010 at 09:54:28AM +1000, Dave Airlie wrote:
> On Sat, 2010-04-10 at 21:55 +0200, marcin.slusarz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
> > fb_do_apertures_overlap is returning wrong value when one aperture
> > is completely whithin the other. Add generic ranges_overlap macro
> > (probably kernel.h candidate) and use it here.
> > 
> 
> That doesn't seem right.
> 
> The rules are:
> 
> the generic aperture has to be equal or smaller than the hw aperture,
> otherwise the generic driver will be trashing random hw pieces on the
> machine.

Why "it has to"? Why generic aperture can't start one byte before hw?

> So with that in mind, the check makes sure the generic aperture starts
> somewhere inside the hw aperture, which the test clearly gets right.

So your only objection is that it's impossible with the current code?

> Have you got a pointer to a machine where it fails?

No, it failed with an artifical test while I was working on vga16fb handoff
(unfinished).

> Dave.
> 
> 
> > Signed-off-by: Marcin Slusarz <marcin.slusarz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > Cc: Dave Airlie <airlied-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > Cc: Peter Jones <pjones-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > Cc: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
> > ---
> >  drivers/video/fbmem.c |   24 +++++++++++++++++-------
> >  1 files changed, 17 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
> > index a15b44e..41f2e5e 100644
> > --- a/drivers/video/fbmem.c
> > +++ b/drivers/video/fbmem.c
> > @@ -1468,15 +1468,25 @@ static int fb_check_foreignness(struct fb_info *fi)
> >  	return 0;
> >  }
> >  
> > +/**
> > + * ranges_overlap - check whether two ranges overlap (their intersection is not empty)
> > + * @start1: start of the first range
> > + * @size1:  length of the first range
> > + * @start2: start of the second range
> > + * @size2:  length of the second range
> > + */
> > +#define ranges_overlap(start1, size1, start2, size2) ({	\
> > +	typeof(start1) __start1 = (start1);		\
> > +	typeof(size1)  __size1  = (size1);		\
> > +	typeof(start2) __start2 = (start2);		\
> > +	typeof(size2)  __size2  = (size2);		\
> > +	__start1 < __start2 + __size2 && __start1 + __size1 > __start2; \
> > +})
> > +
> >  static bool fb_do_apertures_overlap(struct fb_info *gen, struct fb_info *hw)
> >  {
> > -	/* is the generic aperture base the same as the HW one */
> > -	if (gen->aperture_base == hw->aperture_base)
> > -		return true;
> > -	/* is the generic aperture base inside the hw base->hw base+size */
> > -	if (gen->aperture_base > hw->aperture_base && gen->aperture_base <= hw->aperture_base + hw->aperture_size)
> > -		return true;
> > -	return false;
> > +	return ranges_overlap(gen->aperture_base, gen->aperture_size,
> > +				hw->aperture_base, hw->aperture_size);
> >  }
> >  /**
> >   *	register_framebuffer - registers a frame buffer device
> 
> 

Re: [PATCH 1/3] fbmem: fix aperture overlapping check

[Dave Airlie]

On Mon, 2010-04-12 at 13:34 +0200, Marcin Slusarz wrote:
> On Mon, Apr 12, 2010 at 09:54:28AM +1000, Dave Airlie wrote:
> > On Sat, 2010-04-10 at 21:55 +0200, marcin.slusarz@gmail.com wrote:
> > > fb_do_apertures_overlap is returning wrong value when one aperture
> > > is completely whithin the other. Add generic ranges_overlap macro
> > > (probably kernel.h candidate) and use it here.
> > > 
> > 
> > That doesn't seem right.
> > 
> > The rules are:
> > 
> > the generic aperture has to be equal or smaller than the hw aperture,
> > otherwise the generic driver will be trashing random hw pieces on the
> > machine.

> Why "it has to"? Why generic aperture can't start one byte before hw?

Think about how it works a bit more, the answer is obvious.

The hw aperture is defined by the actual hardware, not by the OS or my
code. The PCI apertures are well defined. Now for a generic driver like
vesa or offb to access the hardware it needs to access the hw apertures.

It can't go accessing one byte before the hw aperture, as that will
crash the machine, or write to some other devices hw.

> 
> > So with that in mind, the check makes sure the generic aperture starts
> > somewhere inside the hw aperture, which the test clearly gets right.
> 
> So your only objection is that it's impossible with the current code?
> 
> > Have you got a pointer to a machine where it fails?
> 
> No, it failed with an artifical test while I was working on vga16fb handoff
> (unfinished).

You won't be able to make this work for vga16fb from what I can see
since it access 0xa000 directly, not via any of the defined apertures
that vesafb/offb use. vga16fb will need a different approach I suspect.

Dave.

Re: [PATCH 1/3] fbmem: fix aperture overlapping check

[Dave Airlie]

On Mon, 2010-04-12 at 13:34 +0200, Marcin Slusarz wrote:
> On Mon, Apr 12, 2010 at 09:54:28AM +1000, Dave Airlie wrote:
> > On Sat, 2010-04-10 at 21:55 +0200, marcin.slusarz@gmail.com wrote:
> > > fb_do_apertures_overlap is returning wrong value when one aperture
> > > is completely whithin the other. Add generic ranges_overlap macro
> > > (probably kernel.h candidate) and use it here.
> > > 
> > 
> > That doesn't seem right.
> > 
> > The rules are:
> > 
> > the generic aperture has to be equal or smaller than the hw aperture,
> > otherwise the generic driver will be trashing random hw pieces on the
> > machine.

> Why "it has to"? Why generic aperture can't start one byte before hw?

Think about how it works a bit more, the answer is obvious.

The hw aperture is defined by the actual hardware, not by the OS or my
code. The PCI apertures are well defined. Now for a generic driver like
vesa or offb to access the hardware it needs to access the hw apertures.

It can't go accessing one byte before the hw aperture, as that will
crash the machine, or write to some other devices hw.

> 
> > So with that in mind, the check makes sure the generic aperture starts
> > somewhere inside the hw aperture, which the test clearly gets right.
> 
> So your only objection is that it's impossible with the current code?
> 
> > Have you got a pointer to a machine where it fails?
> 
> No, it failed with an artifical test while I was working on vga16fb handoff
> (unfinished).

You won't be able to make this work for vga16fb from what I can see
since it access 0xa000 directly, not via any of the defined apertures
that vesafb/offb use. vga16fb will need a different approach I suspect.

Dave.

Re: [PATCH 1/3] fbmem: fix aperture overlapping check

[Marcin Slusarz]

On Tue, Apr 13, 2010 at 06:28:21AM +1000, Dave Airlie wrote:
> On Mon, 2010-04-12 at 13:34 +0200, Marcin Slusarz wrote:
> > On Mon, Apr 12, 2010 at 09:54:28AM +1000, Dave Airlie wrote:
> > > On Sat, 2010-04-10 at 21:55 +0200, marcin.slusarz@gmail.com wrote:
> > > > fb_do_apertures_overlap is returning wrong value when one aperture
> > > > is completely whithin the other. Add generic ranges_overlap macro
> > > > (probably kernel.h candidate) and use it here.
> > > > 
> > > 
> > > That doesn't seem right.
> > > 
> > > The rules are:
> > > 
> > > the generic aperture has to be equal or smaller than the hw aperture,
> > > otherwise the generic driver will be trashing random hw pieces on the
> > > machine.
> 
> > Why "it has to"? Why generic aperture can't start one byte before hw?
> 
> Think about how it works a bit more, the answer is obvious.
> 
> The hw aperture is defined by the actual hardware, not by the OS or my
> code. The PCI apertures are well defined. Now for a generic driver like
> vesa or offb to access the hardware it needs to access the hw apertures.
> 
> It can't go accessing one byte before the hw aperture, as that will
> crash the machine, or write to some other devices hw.

Ok, thanks for explanation. I'll drop this patch and rebase the others.

> > > So with that in mind, the check makes sure the generic aperture starts
> > > somewhere inside the hw aperture, which the test clearly gets right.
> > 
> > So your only objection is that it's impossible with the current code?
> > 
> > > Have you got a pointer to a machine where it fails?
> > 
> > No, it failed with an artifical test while I was working on vga16fb handoff
> > (unfinished).
> 
> You won't be able to make this work for vga16fb from what I can see
> since it access 0xa000 directly, not via any of the defined apertures
> that vesafb/offb use. vga16fb will need a different approach I suspect.

Yeah, there's an idea to claim 0xa0000 as an aperture of vga16fb and then
do the same in KMS driver but only if it's the primary card. 
(You hinted to use SHADOW resource bit to check whether card is primary)
I think I'll try this approach soon.

Marcin

Re: [PATCH 1/3] fbmem: fix aperture overlapping check

[Marcin Slusarz]

On Tue, Apr 13, 2010 at 06:28:21AM +1000, Dave Airlie wrote:
> On Mon, 2010-04-12 at 13:34 +0200, Marcin Slusarz wrote:
> > On Mon, Apr 12, 2010 at 09:54:28AM +1000, Dave Airlie wrote:
> > > On Sat, 2010-04-10 at 21:55 +0200, marcin.slusarz@gmail.com wrote:
> > > > fb_do_apertures_overlap is returning wrong value when one aperture
> > > > is completely whithin the other. Add generic ranges_overlap macro
> > > > (probably kernel.h candidate) and use it here.
> > > > 
> > > 
> > > That doesn't seem right.
> > > 
> > > The rules are:
> > > 
> > > the generic aperture has to be equal or smaller than the hw aperture,
> > > otherwise the generic driver will be trashing random hw pieces on the
> > > machine.
> 
> > Why "it has to"? Why generic aperture can't start one byte before hw?
> 
> Think about how it works a bit more, the answer is obvious.
> 
> The hw aperture is defined by the actual hardware, not by the OS or my
> code. The PCI apertures are well defined. Now for a generic driver like
> vesa or offb to access the hardware it needs to access the hw apertures.
> 
> It can't go accessing one byte before the hw aperture, as that will
> crash the machine, or write to some other devices hw.

Ok, thanks for explanation. I'll drop this patch and rebase the others.

> > > So with that in mind, the check makes sure the generic aperture starts
> > > somewhere inside the hw aperture, which the test clearly gets right.
> > 
> > So your only objection is that it's impossible with the current code?
> > 
> > > Have you got a pointer to a machine where it fails?
> > 
> > No, it failed with an artifical test while I was working on vga16fb handoff
> > (unfinished).
> 
> You won't be able to make this work for vga16fb from what I can see
> since it access 0xa000 directly, not via any of the defined apertures
> that vesafb/offb use. vga16fb will need a different approach I suspect.

Yeah, there's an idea to claim 0xa0000 as an aperture of vga16fb and then
do the same in KMS driver but only if it's the primary card. 
(You hinted to use SHADOW resource bit to check whether card is primary)
I think I'll try this approach soon.

Marcin

Re: [PATCH 1/3] fbmem: fix aperture overlapping check

[Dave Airlie]

>
> Ok, thanks for explanation. I'll drop this patch and rebase the others.

Cool,

>> You won't be able to make this work for vga16fb from what I can see
>> since it access 0xa000 directly, not via any of the defined apertures
>> that vesafb/offb use. vga16fb will need a different approach I suspect.
>
> Yeah, there's an idea to claim 0xa0000 as an aperture of vga16fb and then
> do the same in KMS driver but only if it's the primary card.
> (You hinted to use SHADOW resource bit to check whether card is primary)
> I think I'll try this approach soon.

Yeah as long as we kick it off once, certain things like vga switcheroo change
who the primary gpu is at runtime.

but it should be okay if both drivers have the kickoff paths.

Dave.

Re: [PATCH 1/3] fbmem: fix aperture overlapping check

[Dave Airlie]

>
> Ok, thanks for explanation. I'll drop this patch and rebase the others.

Cool,

>> You won't be able to make this work for vga16fb from what I can see
>> since it access 0xa000 directly, not via any of the defined apertures
>> that vesafb/offb use. vga16fb will need a different approach I suspect.
>
> Yeah, there's an idea to claim 0xa0000 as an aperture of vga16fb and then
> do the same in KMS driver but only if it's the primary card.
> (You hinted to use SHADOW resource bit to check whether card is primary)
> I think I'll try this approach soon.

Yeah as long as we kick it off once, certain things like vga switcheroo change
who the primary gpu is at runtime.

but it should be okay if both drivers have the kickoff paths.

Dave.

Re: [PATCH 1/3] fbmem: fix aperture overlapping check

[Dave Airlie]

>
> Ok, thanks for explanation. I'll drop this patch and rebase the others.

Cool,

>> You won't be able to make this work for vga16fb from what I can see
>> since it access 0xa000 directly, not via any of the defined apertures
>> that vesafb/offb use. vga16fb will need a different approach I suspect.
>
> Yeah, there's an idea to claim 0xa0000 as an aperture of vga16fb and then
> do the same in KMS driver but only if it's the primary card.
> (You hinted to use SHADOW resource bit to check whether card is primary)
> I think I'll try this approach soon.

Yeah as long as we kick it off once, certain things like vga switcheroo change
who the primary gpu is at runtime.

but it should be okay if both drivers have the kickoff paths.

Dave.

Re: [PATCH 1/3] fbmem: fix aperture overlapping check

[Dave Airlie]

>
> Ok, thanks for explanation. I'll drop this patch and rebase the others.

Cool,

>> You won't be able to make this work for vga16fb from what I can see
>> since it access 0xa000 directly, not via any of the defined apertures
>> that vesafb/offb use. vga16fb will need a different approach I suspect.
>
> Yeah, there's an idea to claim 0xa0000 as an aperture of vga16fb and then
> do the same in KMS driver but only if it's the primary card.
> (You hinted to use SHADOW resource bit to check whether card is primary)
> I think I'll try this approach soon.

Yeah as long as we kick it off once, certain things like vga switcheroo change
who the primary gpu is at runtime.

but it should be okay if both drivers have the kickoff paths.

Dave.

[PATCH] vga16fb, drm/nouveau: vga16fb->nouveau handoff

[Marcin Slusarz]

On Mon, Apr 12, 2010 at 11:33:27PM +0200, Marcin Slusarz wrote:
> > > > Have you got a pointer to a machine where it fails?
> > > 
> > > No, it failed with an artifical test while I was working on vga16fb handoff
> > > (unfinished).
> > 
> > You won't be able to make this work for vga16fb from what I can see
> > since it access 0xa000 directly, not via any of the defined apertures
> > that vesafb/offb use. vga16fb will need a different approach I suspect.
> 
> Yeah, there's an idea to claim 0xa0000 as an aperture of vga16fb and then
> do the same in KMS driver but only if it's the primary card. 
> (You hinted to use SHADOW resource bit to check whether card is primary)
> I think I'll try this approach soon.

Patch below works for me, but I couldn't test the case when nvidia card is secondary.

---
From: Marcin Slusarz <marcin.slusarz@gmail.com>
Date: Tue, 13 Apr 2010 09:20:53 +0200
Subject: [PATCH] vga16fb, drm/nouveau: vga16fb->nouveau handoff

let vga16fb claim 0xA0000+0x10000 region as its aperture;
nouveau does not use it, so we need to lie to kick vga16fb,
but only if it's driving the primary card (by checking
IORESOURCE_ROM_SHADOW resource flag)

Signed-off-by: Marcin Slusarz <marcin.slusarz@gmail.com>
---
 drivers/gpu/drm/nouveau/nouveau_state.c |   11 ++++++++++-
 drivers/video/vga16fb.c                 |   26 +++++++++++++++++++-------
 2 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_state.c b/drivers/gpu/drm/nouveau/nouveau_state.c
index cfa3239..a101861 100644
--- a/drivers/gpu/drm/nouveau/nouveau_state.c
+++ b/drivers/gpu/drm/nouveau/nouveau_state.c
@@ -636,10 +636,12 @@ static void nouveau_OF_copy_vbios_to_ramin(struct drm_device *dev)
 #endif
 }
 
+#define VGA_FB_PHYS 0xA0000
+#define VGA_FB_PHYS_LEN 65536
 static struct apertures_struct *nouveau_get_apertures(struct drm_device *dev)
 {
 	struct pci_dev *pdev = dev->pdev;
-	struct apertures_struct *aper = alloc_apertures(3);
+	struct apertures_struct *aper = alloc_apertures(4);
 	if (!aper)
 		return NULL;
 
@@ -658,6 +660,13 @@ static struct apertures_struct *nouveau_get_apertures(struct drm_device *dev)
 		aper->ranges[aper->count].size = pci_resource_len(pdev, 3);
 		aper->count++;
 	}
+	
+	/* if it's the primary card, claim 0xa0000 as ours to kick vga16fb */
+	if (pdev->resource[PCI_ROM_RESOURCE].flags & IORESOURCE_ROM_SHADOW) {
+		aper->ranges[aper->count].base = VGA_FB_PHYS;
+		aper->ranges[aper->count].size = VGA_FB_PHYS_LEN;
+		aper->count++;
+	}
 
 	return aper;
 }
diff --git a/drivers/video/vga16fb.c b/drivers/video/vga16fb.c
index 76d8dae..45d7a3d 100644
--- a/drivers/video/vga16fb.c
+++ b/drivers/video/vga16fb.c
@@ -1264,10 +1264,19 @@ static void vga16fb_imageblit(struct fb_info *info, const struct fb_image *image
 		vga_imageblit_color(info, image);
 }
 
+static void vga16fb_destroy(struct fb_info *info)
+{
+	iounmap(info->screen_base);
+	fb_dealloc_cmap(&info->cmap);
+	/* XXX unshare VGA regions */
+	framebuffer_release(info);
+}
+
 static struct fb_ops vga16fb_ops = {
 	.owner		= THIS_MODULE,
 	.fb_open        = vga16fb_open,
 	.fb_release     = vga16fb_release,
+	.fb_destroy	= vga16fb_destroy,
 	.fb_check_var	= vga16fb_check_var,
 	.fb_set_par	= vga16fb_set_par,
 	.fb_setcolreg 	= vga16fb_setcolreg,
@@ -1307,6 +1316,11 @@ static int __devinit vga16fb_probe(struct platform_device *dev)
 		ret = -ENOMEM;
 		goto err_fb_alloc;
 	}
+	info->apertures = alloc_apertures(1);
+	if (!info->apertures) {
+		ret = -ENOMEM;
+		goto err_ioremap;
+	}
 
 	/* XXX share VGA_FB_PHYS and I/O region with vgacon and others */
 	info->screen_base = (void __iomem *)VGA_MAP_MEM(VGA_FB_PHYS, 0);
@@ -1336,7 +1350,7 @@ static int __devinit vga16fb_probe(struct platform_device *dev)
 	info->fix = vga16fb_fix;
 	/* supports rectangles with widths of multiples of 8 */
 	info->pixmap.blit_x = 1 << 7 | 1 << 15 | 1 << 23 | 1 << 31;
-	info->flags = FBINFO_FLAG_DEFAULT |
+	info->flags = FBINFO_FLAG_DEFAULT | FBINFO_MISC_FIRMWARE |
 		FBINFO_HWACCEL_YPAN;
 
 	i = (info->var.bits_per_pixel = 8) ? 256 : 16;
@@ -1355,6 +1369,9 @@ static int __devinit vga16fb_probe(struct platform_device *dev)
 
 	vga16fb_update_fix(info);
 
+	info->apertures->ranges[0].base = VGA_FB_PHYS;
+	info->apertures->ranges[0].size = VGA_FB_PHYS_LEN;
+
 	if (register_framebuffer(info) < 0) {
 		printk(KERN_ERR "vga16fb: unable to register framebuffer\n");
 		ret = -EINVAL;
@@ -1381,13 +1398,8 @@ static int vga16fb_remove(struct platform_device *dev)
 {
 	struct fb_info *info = platform_get_drvdata(dev);
 
-	if (info) {
+	if (info)
 		unregister_framebuffer(info);
-		iounmap(info->screen_base);
-		fb_dealloc_cmap(&info->cmap);
-	/* XXX unshare VGA regions */
-		framebuffer_release(info);
-	}
 
 	return 0;
 }
-- 
1.7.0.4

[PATCH] vga16fb, drm/nouveau: vga16fb->nouveau handoff

[Marcin Slusarz]

On Mon, Apr 12, 2010 at 11:33:27PM +0200, Marcin Slusarz wrote:
> > > > Have you got a pointer to a machine where it fails?
> > > 
> > > No, it failed with an artifical test while I was working on vga16fb handoff
> > > (unfinished).
> > 
> > You won't be able to make this work for vga16fb from what I can see
> > since it access 0xa000 directly, not via any of the defined apertures
> > that vesafb/offb use. vga16fb will need a different approach I suspect.
> 
> Yeah, there's an idea to claim 0xa0000 as an aperture of vga16fb and then
> do the same in KMS driver but only if it's the primary card. 
> (You hinted to use SHADOW resource bit to check whether card is primary)
> I think I'll try this approach soon.

Patch below works for me, but I couldn't test the case when nvidia card is secondary.

---
From: Marcin Slusarz <marcin.slusarz@gmail.com>
Date: Tue, 13 Apr 2010 09:20:53 +0200
Subject: [PATCH] vga16fb, drm/nouveau: vga16fb->nouveau handoff

let vga16fb claim 0xA0000+0x10000 region as its aperture;
nouveau does not use it, so we need to lie to kick vga16fb,
but only if it's driving the primary card (by checking
IORESOURCE_ROM_SHADOW resource flag)

Signed-off-by: Marcin Slusarz <marcin.slusarz@gmail.com>
---
 drivers/gpu/drm/nouveau/nouveau_state.c |   11 ++++++++++-
 drivers/video/vga16fb.c                 |   26 +++++++++++++++++++-------
 2 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_state.c b/drivers/gpu/drm/nouveau/nouveau_state.c
index cfa3239..a101861 100644
--- a/drivers/gpu/drm/nouveau/nouveau_state.c
+++ b/drivers/gpu/drm/nouveau/nouveau_state.c
@@ -636,10 +636,12 @@ static void nouveau_OF_copy_vbios_to_ramin(struct drm_device *dev)
 #endif
 }
 
+#define VGA_FB_PHYS 0xA0000
+#define VGA_FB_PHYS_LEN 65536
 static struct apertures_struct *nouveau_get_apertures(struct drm_device *dev)
 {
 	struct pci_dev *pdev = dev->pdev;
-	struct apertures_struct *aper = alloc_apertures(3);
+	struct apertures_struct *aper = alloc_apertures(4);
 	if (!aper)
 		return NULL;
 
@@ -658,6 +660,13 @@ static struct apertures_struct *nouveau_get_apertures(struct drm_device *dev)
 		aper->ranges[aper->count].size = pci_resource_len(pdev, 3);
 		aper->count++;
 	}
+	
+	/* if it's the primary card, claim 0xa0000 as ours to kick vga16fb */
+	if (pdev->resource[PCI_ROM_RESOURCE].flags & IORESOURCE_ROM_SHADOW) {
+		aper->ranges[aper->count].base = VGA_FB_PHYS;
+		aper->ranges[aper->count].size = VGA_FB_PHYS_LEN;
+		aper->count++;
+	}
 
 	return aper;
 }
diff --git a/drivers/video/vga16fb.c b/drivers/video/vga16fb.c
index 76d8dae..45d7a3d 100644
--- a/drivers/video/vga16fb.c
+++ b/drivers/video/vga16fb.c
@@ -1264,10 +1264,19 @@ static void vga16fb_imageblit(struct fb_info *info, const struct fb_image *image
 		vga_imageblit_color(info, image);
 }
 
+static void vga16fb_destroy(struct fb_info *info)
+{
+	iounmap(info->screen_base);
+	fb_dealloc_cmap(&info->cmap);
+	/* XXX unshare VGA regions */
+	framebuffer_release(info);
+}
+
 static struct fb_ops vga16fb_ops = {
 	.owner		= THIS_MODULE,
 	.fb_open        = vga16fb_open,
 	.fb_release     = vga16fb_release,
+	.fb_destroy	= vga16fb_destroy,
 	.fb_check_var	= vga16fb_check_var,
 	.fb_set_par	= vga16fb_set_par,
 	.fb_setcolreg 	= vga16fb_setcolreg,
@@ -1307,6 +1316,11 @@ static int __devinit vga16fb_probe(struct platform_device *dev)
 		ret = -ENOMEM;
 		goto err_fb_alloc;
 	}
+	info->apertures = alloc_apertures(1);
+	if (!info->apertures) {
+		ret = -ENOMEM;
+		goto err_ioremap;
+	}
 
 	/* XXX share VGA_FB_PHYS and I/O region with vgacon and others */
 	info->screen_base = (void __iomem *)VGA_MAP_MEM(VGA_FB_PHYS, 0);
@@ -1336,7 +1350,7 @@ static int __devinit vga16fb_probe(struct platform_device *dev)
 	info->fix = vga16fb_fix;
 	/* supports rectangles with widths of multiples of 8 */
 	info->pixmap.blit_x = 1 << 7 | 1 << 15 | 1 << 23 | 1 << 31;
-	info->flags = FBINFO_FLAG_DEFAULT |
+	info->flags = FBINFO_FLAG_DEFAULT | FBINFO_MISC_FIRMWARE |
 		FBINFO_HWACCEL_YPAN;
 
 	i = (info->var.bits_per_pixel == 8) ? 256 : 16;
@@ -1355,6 +1369,9 @@ static int __devinit vga16fb_probe(struct platform_device *dev)
 
 	vga16fb_update_fix(info);
 
+	info->apertures->ranges[0].base = VGA_FB_PHYS;
+	info->apertures->ranges[0].size = VGA_FB_PHYS_LEN;
+
 	if (register_framebuffer(info) < 0) {
 		printk(KERN_ERR "vga16fb: unable to register framebuffer\n");
 		ret = -EINVAL;
@@ -1381,13 +1398,8 @@ static int vga16fb_remove(struct platform_device *dev)
 {
 	struct fb_info *info = platform_get_drvdata(dev);
 
-	if (info) {
+	if (info)
 		unregister_framebuffer(info);
-		iounmap(info->screen_base);
-		fb_dealloc_cmap(&info->cmap);
-	/* XXX unshare VGA regions */
-		framebuffer_release(info);
-	}
 
 	return 0;
 }
-- 
1.7.0.4

[PATCH] vga16fb, drm: vga16fb->drm handoff

[Marcin Slusarz]

On Tue, Apr 13, 2010 at 09:50:04PM +0200, Marcin Slusarz wrote:
> On Mon, Apr 12, 2010 at 11:33:27PM +0200, Marcin Slusarz wrote:
> > > > > Have you got a pointer to a machine where it fails?
> > > > 
> > > > No, it failed with an artifical test while I was working on vga16fb handoff
> > > > (unfinished).
> > > 
> > > You won't be able to make this work for vga16fb from what I can see
> > > since it access 0xa000 directly, not via any of the defined apertures
> > > that vesafb/offb use. vga16fb will need a different approach I suspect.
> > 
> > Yeah, there's an idea to claim 0xa0000 as an aperture of vga16fb and then
> > do the same in KMS driver but only if it's the primary card. 
> > (You hinted to use SHADOW resource bit to check whether card is primary)
> > I think I'll try this approach soon.
> 
> Patch below works for me, but I couldn't test the case when nvidia card is secondary.
> 
> ---
> From: Marcin Slusarz <marcin.slusarz@gmail.com>
> Date: Tue, 13 Apr 2010 09:20:53 +0200
> Subject: [PATCH] vga16fb, drm/nouveau: vga16fb->nouveau handoff
> 
> let vga16fb claim 0xA0000+0x10000 region as its aperture;
> nouveau does not use it, so we need to lie to kick vga16fb,
> but only if it's driving the primary card (by checking
> IORESOURCE_ROM_SHADOW resource flag)
> 
> Signed-off-by: Marcin Slusarz <marcin.slusarz@gmail.com>
> ---
>  drivers/gpu/drm/nouveau/nouveau_state.c |   11 ++++++++++-
>  drivers/video/vga16fb.c                 |   26 +++++++++++++++++++-------
>  2 files changed, 29 insertions(+), 8 deletions(-)
> 

More generic approach below - it should work for all drm drivers.
Unfortunately vga16fb handoff has 2 other issues:
- It can be compiled as module, so it can be loaded after KMS driver (and
  nothing prevents it right now)
- vga16fb registration error path is iounmapping memory which was not
  ioremapped.
I'm going to fix it soon.

---
From: Marcin Slusarz <marcin.slusarz@gmail.com>
Subject: [PATCH] vga16fb, drm: vga16fb->drm handoff

let vga16fb claim 0xA0000+0x10000 region as its aperture;
drm drivers don't use it, so we have to detect it and kick
vga16fb manually - but only if drm is driving the primary card
(by checking IORESOURCE_ROM_SHADOW resource flag)

Signed-off-by: Marcin Slusarz <marcin.slusarz@gmail.com>
---
 drivers/gpu/drm/i915/intel_fb.c         |    1 +
 drivers/gpu/drm/nouveau/nouveau_fbcon.c |    1 +
 drivers/gpu/drm/nouveau/nouveau_state.c |    3 ++-
 drivers/gpu/drm/radeon/radeon_fb.c      |    1 +
 drivers/gpu/drm/vmwgfx/vmwgfx_fb.c      |    1 +
 drivers/video/fbmem.c                   |   19 ++++++++++++++++---
 drivers/video/vga16fb.c                 |   26 +++++++++++++++++++-------
 include/linux/fb.h                      |    4 +++-
 8 files changed, 44 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
index 6dbc277..8e403be 100644
--- a/drivers/gpu/drm/i915/intel_fb.c
+++ b/drivers/gpu/drm/i915/intel_fb.c
@@ -201,6 +201,7 @@ static int intelfb_create(struct drm_device *dev, uint32_t fb_width,
 		info->apertures->ranges[0].size = pci_resource_len(dev->pdev, 2);
 	else
 		info->apertures->ranges[0].size = pci_resource_len(dev->pdev, 0);
+	info->pcidev = dev->pdev;
 
 	info->fix.smem_start = dev->mode_config.fb_base + obj_priv->gtt_offset;
 	info->fix.smem_len = size;
diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
index 660746b..2e0d840 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
@@ -285,6 +285,7 @@ nouveau_fbcon_create(struct drm_device *dev, uint32_t fb_width,
 		ret = -ENOMEM;
 		goto out_unref;
 	}
+	info->pcidev = pdev;
 
 	info->pixmap.size = 64*1024;
 	info->pixmap.buf_align = 8;
diff --git a/drivers/gpu/drm/nouveau/nouveau_state.c b/drivers/gpu/drm/nouveau/nouveau_state.c
index 3efc339..4916cf2 100644
--- a/drivers/gpu/drm/nouveau/nouveau_state.c
+++ b/drivers/gpu/drm/nouveau/nouveau_state.c
@@ -670,7 +670,8 @@ static int nouveau_remove_conflicting_drivers(struct drm_device *dev)
 	if (!dev_priv->apertures)
 		return -ENOMEM;
 	
-	remove_conflicting_framebuffers(dev_priv->apertures, "nouveaufb");
+	remove_conflicting_framebuffers(dev_priv->apertures, "nouveaufb",
+					dev->pdev);
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c
index df06cdf..17de92a 100644
--- a/drivers/gpu/drm/radeon/radeon_fb.c
+++ b/drivers/gpu/drm/radeon/radeon_fb.c
@@ -271,6 +271,7 @@ int radeonfb_create(struct drm_device *dev,
 	}
 	info->apertures->ranges[0].base = rdev->ddev->mode_config.fb_base;
 	info->apertures->ranges[0].size = rdev->mc.real_vram_size;
+	info->pcidev = dev->pdev;
 
 	info->fix.mmio_start = 0;
 	info->fix.mmio_len = 0;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
index 0248c6b..eda1336 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
@@ -566,6 +566,7 @@ int vmw_fb_init(struct vmw_private *vmw_priv)
 	}
 	info->apertures->ranges[0].base = vmw_priv->vram_start;
 	info->apertures->ranges[0].size = vmw_priv->vram_size;
+	info->pcidev = vmw_priv->dev->pdev;
 
 	/*
 	 * Dirty & Deferred IO
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index 7cfcd71..4628a59 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -32,6 +32,7 @@
 #include <linux/device.h>
 #include <linux/efi.h>
 #include <linux/fb.h>
+#include <linux/pci.h>
 
 #include <asm/fb.h>
 
@@ -1500,19 +1501,30 @@ static bool fb_do_apertures_overlap(struct apertures_struct *gena,
 	return false;
 }
 
-void remove_conflicting_framebuffers(struct apertures_struct *a, const char *name)
+#define VGA_FB_PHYS 0xA0000
+void remove_conflicting_framebuffers(struct apertures_struct *a,
+				     const char *name, struct pci_dev *pcidev)
 {
 	int i;
+	bool primary = false;
+
+	if (pcidev)
+		primary = pcidev->resource[PCI_ROM_RESOURCE].flags & IORESOURCE_ROM_SHADOW;
 
 	/* check all firmware fbs and kick off if the base addr overlaps */
 	for (i = 0 ; i < FB_MAX; i++) {
+		struct apertures_struct *gen_aper;
 		if (!registered_fb[i])
 			continue;
 
 		if (!(registered_fb[i]->flags & FBINFO_MISC_FIRMWARE))
 			continue;
+		
+		gen_aper = registered_fb[i]->apertures;
+		if (fb_do_apertures_overlap(gen_aper, a) ||
+			(primary && gen_aper && gen_aper->count &&
+			 gen_aper->ranges[0].base = VGA_FB_PHYS)) {
 
-		if (fb_do_apertures_overlap(registered_fb[i]->apertures, a)) {
 			printk(KERN_ERR "fb: conflicting fb hw usage "
 			       "%s vs %s - removing generic driver\n",
 			       name, registered_fb[i]->fix.id);
@@ -1545,7 +1557,8 @@ register_framebuffer(struct fb_info *fb_info)
 	if (fb_check_foreignness(fb_info))
 		return -ENOSYS;
 
-	remove_conflicting_framebuffers(fb_info->apertures, fb_info->fix.id);
+	remove_conflicting_framebuffers(fb_info->apertures, fb_info->fix.id,
+					fb_info->pcidev);
 
 	num_registered_fb++;
 	for (i = 0 ; i < FB_MAX; i++)
diff --git a/drivers/video/vga16fb.c b/drivers/video/vga16fb.c
index bf638a4..149c47a 100644
--- a/drivers/video/vga16fb.c
+++ b/drivers/video/vga16fb.c
@@ -1263,10 +1263,19 @@ static void vga16fb_imageblit(struct fb_info *info, const struct fb_image *image
 		vga_imageblit_color(info, image);
 }
 
+static void vga16fb_destroy(struct fb_info *info)
+{
+	iounmap(info->screen_base);
+	fb_dealloc_cmap(&info->cmap);
+	/* XXX unshare VGA regions */
+	framebuffer_release(info);
+}
+
 static struct fb_ops vga16fb_ops = {
 	.owner		= THIS_MODULE,
 	.fb_open        = vga16fb_open,
 	.fb_release     = vga16fb_release,
+	.fb_destroy	= vga16fb_destroy,
 	.fb_check_var	= vga16fb_check_var,
 	.fb_set_par	= vga16fb_set_par,
 	.fb_setcolreg 	= vga16fb_setcolreg,
@@ -1306,6 +1315,11 @@ static int __devinit vga16fb_probe(struct platform_device *dev)
 		ret = -ENOMEM;
 		goto err_fb_alloc;
 	}
+	info->apertures = alloc_apertures(1);
+	if (!info->apertures) {
+		ret = -ENOMEM;
+		goto err_ioremap;
+	}
 
 	/* XXX share VGA_FB_PHYS and I/O region with vgacon and others */
 	info->screen_base = (void __iomem *)VGA_MAP_MEM(VGA_FB_PHYS, 0);
@@ -1335,7 +1349,7 @@ static int __devinit vga16fb_probe(struct platform_device *dev)
 	info->fix = vga16fb_fix;
 	/* supports rectangles with widths of multiples of 8 */
 	info->pixmap.blit_x = 1 << 7 | 1 << 15 | 1 << 23 | 1 << 31;
-	info->flags = FBINFO_FLAG_DEFAULT |
+	info->flags = FBINFO_FLAG_DEFAULT | FBINFO_MISC_FIRMWARE |
 		FBINFO_HWACCEL_YPAN;
 
 	i = (info->var.bits_per_pixel = 8) ? 256 : 16;
@@ -1354,6 +1368,9 @@ static int __devinit vga16fb_probe(struct platform_device *dev)
 
 	vga16fb_update_fix(info);
 
+	info->apertures->ranges[0].base = VGA_FB_PHYS;
+	info->apertures->ranges[0].size = VGA_FB_PHYS_LEN;
+
 	if (register_framebuffer(info) < 0) {
 		printk(KERN_ERR "vga16fb: unable to register framebuffer\n");
 		ret = -EINVAL;
@@ -1380,13 +1397,8 @@ static int vga16fb_remove(struct platform_device *dev)
 {
 	struct fb_info *info = platform_get_drvdata(dev);
 
-	if (info) {
+	if (info)
 		unregister_framebuffer(info);
-		iounmap(info->screen_base);
-		fb_dealloc_cmap(&info->cmap);
-	/* XXX unshare VGA regions */
-		framebuffer_release(info);
-	}
 
 	return 0;
 }
diff --git a/include/linux/fb.h b/include/linux/fb.h
index f88e254..4b48e2f 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -870,6 +870,7 @@ struct fb_info {
 			resource_size_t size;
 		} ranges[0];
 	} *apertures;
+	struct pci_dev *pcidev;
 };
 
 static inline struct apertures_struct *alloc_apertures(unsigned int max_num) {
@@ -971,7 +972,8 @@ extern ssize_t fb_sys_write(struct fb_info *info, const char __user *buf,
 /* drivers/video/fbmem.c */
 extern int register_framebuffer(struct fb_info *fb_info);
 extern int unregister_framebuffer(struct fb_info *fb_info);
-extern void remove_conflicting_framebuffers(struct apertures_struct *a, const char *name);
+extern void remove_conflicting_framebuffers(struct apertures_struct *a,
+				const char *name, struct pci_dev *pcidev);
 extern int fb_prepare_logo(struct fb_info *fb_info, int rotate);
 extern int fb_show_logo(struct fb_info *fb_info, int rotate);
 extern char* fb_get_buffer_offset(struct fb_info *info, struct fb_pixmap *buf, u32 size);
-- 
1.7.0.4

[PATCH] vga16fb, drm: vga16fb->drm handoff

[Marcin Slusarz]

On Tue, Apr 13, 2010 at 09:50:04PM +0200, Marcin Slusarz wrote:
> On Mon, Apr 12, 2010 at 11:33:27PM +0200, Marcin Slusarz wrote:
> > > > > Have you got a pointer to a machine where it fails?
> > > > 
> > > > No, it failed with an artifical test while I was working on vga16fb handoff
> > > > (unfinished).
> > > 
> > > You won't be able to make this work for vga16fb from what I can see
> > > since it access 0xa000 directly, not via any of the defined apertures
> > > that vesafb/offb use. vga16fb will need a different approach I suspect.
> > 
> > Yeah, there's an idea to claim 0xa0000 as an aperture of vga16fb and then
> > do the same in KMS driver but only if it's the primary card. 
> > (You hinted to use SHADOW resource bit to check whether card is primary)
> > I think I'll try this approach soon.
> 
> Patch below works for me, but I couldn't test the case when nvidia card is secondary.
> 
> ---
> From: Marcin Slusarz <marcin.slusarz@gmail.com>
> Date: Tue, 13 Apr 2010 09:20:53 +0200
> Subject: [PATCH] vga16fb, drm/nouveau: vga16fb->nouveau handoff
> 
> let vga16fb claim 0xA0000+0x10000 region as its aperture;
> nouveau does not use it, so we need to lie to kick vga16fb,
> but only if it's driving the primary card (by checking
> IORESOURCE_ROM_SHADOW resource flag)
> 
> Signed-off-by: Marcin Slusarz <marcin.slusarz@gmail.com>
> ---
>  drivers/gpu/drm/nouveau/nouveau_state.c |   11 ++++++++++-
>  drivers/video/vga16fb.c                 |   26 +++++++++++++++++++-------
>  2 files changed, 29 insertions(+), 8 deletions(-)
> 

More generic approach below - it should work for all drm drivers.
Unfortunately vga16fb handoff has 2 other issues:
- It can be compiled as module, so it can be loaded after KMS driver (and
  nothing prevents it right now)
- vga16fb registration error path is iounmapping memory which was not
  ioremapped.
I'm going to fix it soon.

---
From: Marcin Slusarz <marcin.slusarz@gmail.com>
Subject: [PATCH] vga16fb, drm: vga16fb->drm handoff

let vga16fb claim 0xA0000+0x10000 region as its aperture;
drm drivers don't use it, so we have to detect it and kick
vga16fb manually - but only if drm is driving the primary card
(by checking IORESOURCE_ROM_SHADOW resource flag)

Signed-off-by: Marcin Slusarz <marcin.slusarz@gmail.com>
---
 drivers/gpu/drm/i915/intel_fb.c         |    1 +
 drivers/gpu/drm/nouveau/nouveau_fbcon.c |    1 +
 drivers/gpu/drm/nouveau/nouveau_state.c |    3 ++-
 drivers/gpu/drm/radeon/radeon_fb.c      |    1 +
 drivers/gpu/drm/vmwgfx/vmwgfx_fb.c      |    1 +
 drivers/video/fbmem.c                   |   19 ++++++++++++++++---
 drivers/video/vga16fb.c                 |   26 +++++++++++++++++++-------
 include/linux/fb.h                      |    4 +++-
 8 files changed, 44 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
index 6dbc277..8e403be 100644
--- a/drivers/gpu/drm/i915/intel_fb.c
+++ b/drivers/gpu/drm/i915/intel_fb.c
@@ -201,6 +201,7 @@ static int intelfb_create(struct drm_device *dev, uint32_t fb_width,
 		info->apertures->ranges[0].size = pci_resource_len(dev->pdev, 2);
 	else
 		info->apertures->ranges[0].size = pci_resource_len(dev->pdev, 0);
+	info->pcidev = dev->pdev;
 
 	info->fix.smem_start = dev->mode_config.fb_base + obj_priv->gtt_offset;
 	info->fix.smem_len = size;
diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
index 660746b..2e0d840 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
@@ -285,6 +285,7 @@ nouveau_fbcon_create(struct drm_device *dev, uint32_t fb_width,
 		ret = -ENOMEM;
 		goto out_unref;
 	}
+	info->pcidev = pdev;
 
 	info->pixmap.size = 64*1024;
 	info->pixmap.buf_align = 8;
diff --git a/drivers/gpu/drm/nouveau/nouveau_state.c b/drivers/gpu/drm/nouveau/nouveau_state.c
index 3efc339..4916cf2 100644
--- a/drivers/gpu/drm/nouveau/nouveau_state.c
+++ b/drivers/gpu/drm/nouveau/nouveau_state.c
@@ -670,7 +670,8 @@ static int nouveau_remove_conflicting_drivers(struct drm_device *dev)
 	if (!dev_priv->apertures)
 		return -ENOMEM;
 	
-	remove_conflicting_framebuffers(dev_priv->apertures, "nouveaufb");
+	remove_conflicting_framebuffers(dev_priv->apertures, "nouveaufb",
+					dev->pdev);
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c
index df06cdf..17de92a 100644
--- a/drivers/gpu/drm/radeon/radeon_fb.c
+++ b/drivers/gpu/drm/radeon/radeon_fb.c
@@ -271,6 +271,7 @@ int radeonfb_create(struct drm_device *dev,
 	}
 	info->apertures->ranges[0].base = rdev->ddev->mode_config.fb_base;
 	info->apertures->ranges[0].size = rdev->mc.real_vram_size;
+	info->pcidev = dev->pdev;
 
 	info->fix.mmio_start = 0;
 	info->fix.mmio_len = 0;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
index 0248c6b..eda1336 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
@@ -566,6 +566,7 @@ int vmw_fb_init(struct vmw_private *vmw_priv)
 	}
 	info->apertures->ranges[0].base = vmw_priv->vram_start;
 	info->apertures->ranges[0].size = vmw_priv->vram_size;
+	info->pcidev = vmw_priv->dev->pdev;
 
 	/*
 	 * Dirty & Deferred IO
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index 7cfcd71..4628a59 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -32,6 +32,7 @@
 #include <linux/device.h>
 #include <linux/efi.h>
 #include <linux/fb.h>
+#include <linux/pci.h>
 
 #include <asm/fb.h>
 
@@ -1500,19 +1501,30 @@ static bool fb_do_apertures_overlap(struct apertures_struct *gena,
 	return false;
 }
 
-void remove_conflicting_framebuffers(struct apertures_struct *a, const char *name)
+#define VGA_FB_PHYS 0xA0000
+void remove_conflicting_framebuffers(struct apertures_struct *a,
+				     const char *name, struct pci_dev *pcidev)
 {
 	int i;
+	bool primary = false;
+
+	if (pcidev)
+		primary = pcidev->resource[PCI_ROM_RESOURCE].flags & IORESOURCE_ROM_SHADOW;
 
 	/* check all firmware fbs and kick off if the base addr overlaps */
 	for (i = 0 ; i < FB_MAX; i++) {
+		struct apertures_struct *gen_aper;
 		if (!registered_fb[i])
 			continue;
 
 		if (!(registered_fb[i]->flags & FBINFO_MISC_FIRMWARE))
 			continue;
+		
+		gen_aper = registered_fb[i]->apertures;
+		if (fb_do_apertures_overlap(gen_aper, a) ||
+			(primary && gen_aper && gen_aper->count &&
+			 gen_aper->ranges[0].base == VGA_FB_PHYS)) {
 
-		if (fb_do_apertures_overlap(registered_fb[i]->apertures, a)) {
 			printk(KERN_ERR "fb: conflicting fb hw usage "
 			       "%s vs %s - removing generic driver\n",
 			       name, registered_fb[i]->fix.id);
@@ -1545,7 +1557,8 @@ register_framebuffer(struct fb_info *fb_info)
 	if (fb_check_foreignness(fb_info))
 		return -ENOSYS;
 
-	remove_conflicting_framebuffers(fb_info->apertures, fb_info->fix.id);
+	remove_conflicting_framebuffers(fb_info->apertures, fb_info->fix.id,
+					fb_info->pcidev);
 
 	num_registered_fb++;
 	for (i = 0 ; i < FB_MAX; i++)
diff --git a/drivers/video/vga16fb.c b/drivers/video/vga16fb.c
index bf638a4..149c47a 100644
--- a/drivers/video/vga16fb.c
+++ b/drivers/video/vga16fb.c
@@ -1263,10 +1263,19 @@ static void vga16fb_imageblit(struct fb_info *info, const struct fb_image *image
 		vga_imageblit_color(info, image);
 }
 
+static void vga16fb_destroy(struct fb_info *info)
+{
+	iounmap(info->screen_base);
+	fb_dealloc_cmap(&info->cmap);
+	/* XXX unshare VGA regions */
+	framebuffer_release(info);
+}
+
 static struct fb_ops vga16fb_ops = {
 	.owner		= THIS_MODULE,
 	.fb_open        = vga16fb_open,
 	.fb_release     = vga16fb_release,
+	.fb_destroy	= vga16fb_destroy,
 	.fb_check_var	= vga16fb_check_var,
 	.fb_set_par	= vga16fb_set_par,
 	.fb_setcolreg 	= vga16fb_setcolreg,
@@ -1306,6 +1315,11 @@ static int __devinit vga16fb_probe(struct platform_device *dev)
 		ret = -ENOMEM;
 		goto err_fb_alloc;
 	}
+	info->apertures = alloc_apertures(1);
+	if (!info->apertures) {
+		ret = -ENOMEM;
+		goto err_ioremap;
+	}
 
 	/* XXX share VGA_FB_PHYS and I/O region with vgacon and others */
 	info->screen_base = (void __iomem *)VGA_MAP_MEM(VGA_FB_PHYS, 0);
@@ -1335,7 +1349,7 @@ static int __devinit vga16fb_probe(struct platform_device *dev)
 	info->fix = vga16fb_fix;
 	/* supports rectangles with widths of multiples of 8 */
 	info->pixmap.blit_x = 1 << 7 | 1 << 15 | 1 << 23 | 1 << 31;
-	info->flags = FBINFO_FLAG_DEFAULT |
+	info->flags = FBINFO_FLAG_DEFAULT | FBINFO_MISC_FIRMWARE |
 		FBINFO_HWACCEL_YPAN;
 
 	i = (info->var.bits_per_pixel == 8) ? 256 : 16;
@@ -1354,6 +1368,9 @@ static int __devinit vga16fb_probe(struct platform_device *dev)
 
 	vga16fb_update_fix(info);
 
+	info->apertures->ranges[0].base = VGA_FB_PHYS;
+	info->apertures->ranges[0].size = VGA_FB_PHYS_LEN;
+
 	if (register_framebuffer(info) < 0) {
 		printk(KERN_ERR "vga16fb: unable to register framebuffer\n");
 		ret = -EINVAL;
@@ -1380,13 +1397,8 @@ static int vga16fb_remove(struct platform_device *dev)
 {
 	struct fb_info *info = platform_get_drvdata(dev);
 
-	if (info) {
+	if (info)
 		unregister_framebuffer(info);
-		iounmap(info->screen_base);
-		fb_dealloc_cmap(&info->cmap);
-	/* XXX unshare VGA regions */
-		framebuffer_release(info);
-	}
 
 	return 0;
 }
diff --git a/include/linux/fb.h b/include/linux/fb.h
index f88e254..4b48e2f 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -870,6 +870,7 @@ struct fb_info {
 			resource_size_t size;
 		} ranges[0];
 	} *apertures;
+	struct pci_dev *pcidev;
 };
 
 static inline struct apertures_struct *alloc_apertures(unsigned int max_num) {
@@ -971,7 +972,8 @@ extern ssize_t fb_sys_write(struct fb_info *info, const char __user *buf,
 /* drivers/video/fbmem.c */
 extern int register_framebuffer(struct fb_info *fb_info);
 extern int unregister_framebuffer(struct fb_info *fb_info);
-extern void remove_conflicting_framebuffers(struct apertures_struct *a, const char *name);
+extern void remove_conflicting_framebuffers(struct apertures_struct *a,
+				const char *name, struct pci_dev *pcidev);
 extern int fb_prepare_logo(struct fb_info *fb_info, int rotate);
 extern int fb_show_logo(struct fb_info *fb_info, int rotate);
 extern char* fb_get_buffer_offset(struct fb_info *info, struct fb_pixmap *buf, u32 size);
-- 
1.7.0.4

Re: [PATCH] vga16fb, drm: vga16fb->drm handoff

[James Simmons]

> More generic approach below - it should work for all drm drivers.
> Unfortunately vga16fb handoff has 2 other issues:
> - It can be compiled as module, so it can be loaded after KMS driver (and
>   nothing prevents it right now)
> - vga16fb registration error path is iounmapping memory which was not
>   ioremapped.
> I'm going to fix it soon.

Nak. See comments below.
 
> ---
> From: Marcin Slusarz <marcin.slusarz@gmail.com>
> Subject: [PATCH] vga16fb, drm: vga16fb->drm handoff
> 
> let vga16fb claim 0xA0000+0x10000 region as its aperture;
> drm drivers don't use it, so we have to detect it and kick
> vga16fb manually - but only if drm is driving the primary card
> (by checking IORESOURCE_ROM_SHADOW resource flag)
> 
> Signed-off-by: Marcin Slusarz <marcin.slusarz@gmail.com>
> ---
>  drivers/gpu/drm/i915/intel_fb.c         |    1 +
>  drivers/gpu/drm/nouveau/nouveau_fbcon.c |    1 +
>  drivers/gpu/drm/nouveau/nouveau_state.c |    3 ++-
>  drivers/gpu/drm/radeon/radeon_fb.c      |    1 +
>  drivers/gpu/drm/vmwgfx/vmwgfx_fb.c      |    1 +
>  drivers/video/fbmem.c                   |   19 ++++++++++++++++---
>  drivers/video/vga16fb.c                 |   26 +++++++++++++++++++-------
>  include/linux/fb.h                      |    4 +++-
>  8 files changed, 44 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
> index 6dbc277..8e403be 100644
> --- a/drivers/gpu/drm/i915/intel_fb.c
> +++ b/drivers/gpu/drm/i915/intel_fb.c
> @@ -201,6 +201,7 @@ static int intelfb_create(struct drm_device *dev, uint32_t fb_width,
>  		info->apertures->ranges[0].size = pci_resource_len(dev->pdev, 2);
>  	else
>  		info->apertures->ranges[0].size = pci_resource_len(dev->pdev, 0);
> +	info->pcidev = dev->pdev;

No pci junk in struct fb_info. It needs to be bus independent.

> diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
> index 7cfcd71..4628a59 100644
> --- a/drivers/video/fbmem.c
> +++ b/drivers/video/fbmem.c
> @@ -32,6 +32,7 @@
>  #include <linux/device.h>
>  #include <linux/efi.h>
>  #include <linux/fb.h>
> +#include <linux/pci.h>

Please remove pci header.
  
>  #include <asm/fb.h>
>  
> @@ -1500,19 +1501,30 @@ static bool fb_do_apertures_overlap(struct apertures_struct *gena,
>  	return false;
>  }
>  
> -void remove_conflicting_framebuffers(struct apertures_struct *a, const char *name)
> +#define VGA_FB_PHYS 0xA0000
> +void remove_conflicting_framebuffers(struct apertures_struct *a,
> +				     const char *name, struct pci_dev *pcidev)
>  {
>  	int i;
> +	bool primary = false;
> +
> +	if (pcidev)
> +		primary = pcidev->resource[PCI_ROM_RESOURCE].flags & IORESOURCE_ROM_SHADOW;

Not portable. Please use fb_is_primary_device(info) instead. It will also 
make your code cleaner.

Re: [PATCH] vga16fb, drm: vga16fb->drm handoff

[James Simmons]

> More generic approach below - it should work for all drm drivers.
> Unfortunately vga16fb handoff has 2 other issues:
> - It can be compiled as module, so it can be loaded after KMS driver (and
>   nothing prevents it right now)
> - vga16fb registration error path is iounmapping memory which was not
>   ioremapped.
> I'm going to fix it soon.

Nak. See comments below.
 
> ---
> From: Marcin Slusarz <marcin.slusarz@gmail.com>
> Subject: [PATCH] vga16fb, drm: vga16fb->drm handoff
> 
> let vga16fb claim 0xA0000+0x10000 region as its aperture;
> drm drivers don't use it, so we have to detect it and kick
> vga16fb manually - but only if drm is driving the primary card
> (by checking IORESOURCE_ROM_SHADOW resource flag)
> 
> Signed-off-by: Marcin Slusarz <marcin.slusarz@gmail.com>
> ---
>  drivers/gpu/drm/i915/intel_fb.c         |    1 +
>  drivers/gpu/drm/nouveau/nouveau_fbcon.c |    1 +
>  drivers/gpu/drm/nouveau/nouveau_state.c |    3 ++-
>  drivers/gpu/drm/radeon/radeon_fb.c      |    1 +
>  drivers/gpu/drm/vmwgfx/vmwgfx_fb.c      |    1 +
>  drivers/video/fbmem.c                   |   19 ++++++++++++++++---
>  drivers/video/vga16fb.c                 |   26 +++++++++++++++++++-------
>  include/linux/fb.h                      |    4 +++-
>  8 files changed, 44 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
> index 6dbc277..8e403be 100644
> --- a/drivers/gpu/drm/i915/intel_fb.c
> +++ b/drivers/gpu/drm/i915/intel_fb.c
> @@ -201,6 +201,7 @@ static int intelfb_create(struct drm_device *dev, uint32_t fb_width,
>  		info->apertures->ranges[0].size = pci_resource_len(dev->pdev, 2);
>  	else
>  		info->apertures->ranges[0].size = pci_resource_len(dev->pdev, 0);
> +	info->pcidev = dev->pdev;

No pci junk in struct fb_info. It needs to be bus independent.

> diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
> index 7cfcd71..4628a59 100644
> --- a/drivers/video/fbmem.c
> +++ b/drivers/video/fbmem.c
> @@ -32,6 +32,7 @@
>  #include <linux/device.h>
>  #include <linux/efi.h>
>  #include <linux/fb.h>
> +#include <linux/pci.h>

Please remove pci header.
  
>  #include <asm/fb.h>
>  
> @@ -1500,19 +1501,30 @@ static bool fb_do_apertures_overlap(struct apertures_struct *gena,
>  	return false;
>  }
>  
> -void remove_conflicting_framebuffers(struct apertures_struct *a, const char *name)
> +#define VGA_FB_PHYS 0xA0000
> +void remove_conflicting_framebuffers(struct apertures_struct *a,
> +				     const char *name, struct pci_dev *pcidev)
>  {
>  	int i;
> +	bool primary = false;
> +
> +	if (pcidev)
> +		primary = pcidev->resource[PCI_ROM_RESOURCE].flags & IORESOURCE_ROM_SHADOW;

Not portable. Please use fb_is_primary_device(info) instead. It will also 
make your code cleaner.

Re: [PATCH] vga16fb, drm: vga16fb->drm handoff

[James Simmons]

> More generic approach below - it should work for all drm drivers.
> Unfortunately vga16fb handoff has 2 other issues:
> - It can be compiled as module, so it can be loaded after KMS driver (and
>   nothing prevents it right now)
> - vga16fb registration error path is iounmapping memory which was not
>   ioremapped.
> I'm going to fix it soon.

Nak. See comments below.
 
> ---
> From: Marcin Slusarz <marcin.slusarz@gmail.com>
> Subject: [PATCH] vga16fb, drm: vga16fb->drm handoff
> 
> let vga16fb claim 0xA0000+0x10000 region as its aperture;
> drm drivers don't use it, so we have to detect it and kick
> vga16fb manually - but only if drm is driving the primary card
> (by checking IORESOURCE_ROM_SHADOW resource flag)
> 
> Signed-off-by: Marcin Slusarz <marcin.slusarz@gmail.com>
> ---
>  drivers/gpu/drm/i915/intel_fb.c         |    1 +
>  drivers/gpu/drm/nouveau/nouveau_fbcon.c |    1 +
>  drivers/gpu/drm/nouveau/nouveau_state.c |    3 ++-
>  drivers/gpu/drm/radeon/radeon_fb.c      |    1 +
>  drivers/gpu/drm/vmwgfx/vmwgfx_fb.c      |    1 +
>  drivers/video/fbmem.c                   |   19 ++++++++++++++++---
>  drivers/video/vga16fb.c                 |   26 +++++++++++++++++++-------
>  include/linux/fb.h                      |    4 +++-
>  8 files changed, 44 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
> index 6dbc277..8e403be 100644
> --- a/drivers/gpu/drm/i915/intel_fb.c
> +++ b/drivers/gpu/drm/i915/intel_fb.c
> @@ -201,6 +201,7 @@ static int intelfb_create(struct drm_device *dev, uint32_t fb_width,
>  		info->apertures->ranges[0].size = pci_resource_len(dev->pdev, 2);
>  	else
>  		info->apertures->ranges[0].size = pci_resource_len(dev->pdev, 0);
> +	info->pcidev = dev->pdev;

No pci junk in struct fb_info. It needs to be bus independent.

> diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
> index 7cfcd71..4628a59 100644
> --- a/drivers/video/fbmem.c
> +++ b/drivers/video/fbmem.c
> @@ -32,6 +32,7 @@
>  #include <linux/device.h>
>  #include <linux/efi.h>
>  #include <linux/fb.h>
> +#include <linux/pci.h>

Please remove pci header.
  
>  #include <asm/fb.h>
>  
> @@ -1500,19 +1501,30 @@ static bool fb_do_apertures_overlap(struct apertures_struct *gena,
>  	return false;
>  }
>  
> -void remove_conflicting_framebuffers(struct apertures_struct *a, const char *name)
> +#define VGA_FB_PHYS 0xA0000
> +void remove_conflicting_framebuffers(struct apertures_struct *a,
> +				     const char *name, struct pci_dev *pcidev)
>  {
>  	int i;
> +	bool primary = false;
> +
> +	if (pcidev)
> +		primary = pcidev->resource[PCI_ROM_RESOURCE].flags & IORESOURCE_ROM_SHADOW;

Not portable. Please use fb_is_primary_device(info) instead. It will also 
make your code cleaner.

Re: [PATCH] vga16fb, drm: vga16fb->drm handoff

[Marcin Slusarz]

On Mon, Apr 19, 2010 at 05:20:10PM +0100, James Simmons wrote:
> 
> > More generic approach below - it should work for all drm drivers.
> > Unfortunately vga16fb handoff has 2 other issues:
> > - It can be compiled as module, so it can be loaded after KMS driver (and
> >   nothing prevents it right now)
> > - vga16fb registration error path is iounmapping memory which was not
> >   ioremapped.
> > I'm going to fix it soon.
> 
> Nak. See comments below.

Thanks for a review.
  
> > ---
> > From: Marcin Slusarz <marcin.slusarz@gmail.com>
> > Subject: [PATCH] vga16fb, drm: vga16fb->drm handoff
> > 
> > let vga16fb claim 0xA0000+0x10000 region as its aperture;
> > drm drivers don't use it, so we have to detect it and kick
> > vga16fb manually - but only if drm is driving the primary card
> > (by checking IORESOURCE_ROM_SHADOW resource flag)
> > 
> > Signed-off-by: Marcin Slusarz <marcin.slusarz@gmail.com>
> > ---
> >  drivers/gpu/drm/i915/intel_fb.c         |    1 +
> >  drivers/gpu/drm/nouveau/nouveau_fbcon.c |    1 +
> >  drivers/gpu/drm/nouveau/nouveau_state.c |    3 ++-
> >  drivers/gpu/drm/radeon/radeon_fb.c      |    1 +
> >  drivers/gpu/drm/vmwgfx/vmwgfx_fb.c      |    1 +
> >  drivers/video/fbmem.c                   |   19 ++++++++++++++++---
> >  drivers/video/vga16fb.c                 |   26 +++++++++++++++++++-------
> >  include/linux/fb.h                      |    4 +++-
> >  8 files changed, 44 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
> > index 6dbc277..8e403be 100644
> > --- a/drivers/gpu/drm/i915/intel_fb.c
> > +++ b/drivers/gpu/drm/i915/intel_fb.c
> > @@ -201,6 +201,7 @@ static int intelfb_create(struct drm_device *dev, uint32_t fb_width,
> >  		info->apertures->ranges[0].size = pci_resource_len(dev->pdev, 2);
> >  	else
> >  		info->apertures->ranges[0].size = pci_resource_len(dev->pdev, 0);
> > +	info->pcidev = dev->pdev;
> 
> No pci junk in struct fb_info. It needs to be bus independent.
> 
> > diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
> > index 7cfcd71..4628a59 100644
> > --- a/drivers/video/fbmem.c
> > +++ b/drivers/video/fbmem.c
> > @@ -32,6 +32,7 @@
> >  #include <linux/device.h>
> >  #include <linux/efi.h>
> >  #include <linux/fb.h>
> > +#include <linux/pci.h>
> 
> Please remove pci header.
>   
> >  #include <asm/fb.h>
> >  
> > @@ -1500,19 +1501,30 @@ static bool fb_do_apertures_overlap(struct apertures_struct *gena,
> >  	return false;
> >  }
> >  
> > -void remove_conflicting_framebuffers(struct apertures_struct *a, const char *name)
> > +#define VGA_FB_PHYS 0xA0000
> > +void remove_conflicting_framebuffers(struct apertures_struct *a,
> > +				     const char *name, struct pci_dev *pcidev)
> >  {
> >  	int i;
> > +	bool primary = false;
> > +
> > +	if (pcidev)
> > +		primary = pcidev->resource[PCI_ROM_RESOURCE].flags & IORESOURCE_ROM_SHADOW;
> 
> Not portable. Please use fb_is_primary_device(info) instead. It will also 
> make your code cleaner.
> 

It simplified the code, but I still need to figure out whether the card is primary when going
the early kick path (when I don't have struct fb_info yet) - the check is now in nouveau.

---
From: Marcin Slusarz <marcin.slusarz@gmail.com>
Subject: [PATCH v2] vga16fb, drm: vga16fb->drm handoff

let vga16fb claim 0xA0000+0x10000 region as its aperture;
drm drivers don't use it, so we have to detect it and kick
vga16fb manually - but only if drm is driving the primary card

Signed-off-by: Marcin Slusarz <marcin.slusarz@gmail.com>
---
 drivers/gpu/drm/nouveau/nouveau_state.c |    7 ++++++-
 drivers/video/fbmem.c                   |   14 +++++++++++---
 drivers/video/vga16fb.c                 |   26 +++++++++++++++++++-------
 include/linux/fb.h                      |    3 ++-
 4 files changed, 38 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_state.c b/drivers/gpu/drm/nouveau/nouveau_state.c
index 3efc339..4c193ed 100644
--- a/drivers/gpu/drm/nouveau/nouveau_state.c
+++ b/drivers/gpu/drm/nouveau/nouveau_state.c
@@ -669,8 +669,13 @@ static int nouveau_remove_conflicting_drivers(struct drm_device *dev)
 	dev_priv->apertures = nouveau_get_apertures(dev);
 	if (!dev_priv->apertures)
 		return -ENOMEM;
+	bool primary = false;
+
+#ifdef CONFIG_X86
+	primary = dev->pdev->resource[PCI_ROM_RESOURCE].flags & IORESOURCE_ROM_SHADOW;
+#endif
 	
-	remove_conflicting_framebuffers(dev_priv->apertures, "nouveaufb");
+	remove_conflicting_framebuffers(dev_priv->apertures, "nouveaufb", primary);
 	return 0;
 }
 
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index 7cfcd71..6deb57c 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -1500,19 +1500,26 @@ static bool fb_do_apertures_overlap(struct apertures_struct *gena,
 	return false;
 }
 
-void remove_conflicting_framebuffers(struct apertures_struct *a, const char *name)
+#define VGA_FB_PHYS 0xA0000
+void remove_conflicting_framebuffers(struct apertures_struct *a,
+				     const char *name, bool primary)
 {
 	int i;
 
 	/* check all firmware fbs and kick off if the base addr overlaps */
 	for (i = 0 ; i < FB_MAX; i++) {
+		struct apertures_struct *gen_aper;
 		if (!registered_fb[i])
 			continue;
 
 		if (!(registered_fb[i]->flags & FBINFO_MISC_FIRMWARE))
 			continue;
+		
+		gen_aper = registered_fb[i]->apertures;
+		if (fb_do_apertures_overlap(gen_aper, a) ||
+			(primary && gen_aper && gen_aper->count &&
+			 gen_aper->ranges[0].base = VGA_FB_PHYS)) {
 
-		if (fb_do_apertures_overlap(registered_fb[i]->apertures, a)) {
 			printk(KERN_ERR "fb: conflicting fb hw usage "
 			       "%s vs %s - removing generic driver\n",
 			       name, registered_fb[i]->fix.id);
@@ -1545,7 +1552,8 @@ register_framebuffer(struct fb_info *fb_info)
 	if (fb_check_foreignness(fb_info))
 		return -ENOSYS;
 
-	remove_conflicting_framebuffers(fb_info->apertures, fb_info->fix.id);
+	remove_conflicting_framebuffers(fb_info->apertures, fb_info->fix.id,
+					 fb_is_primary_device(fb_info));
 
 	num_registered_fb++;
 	for (i = 0 ; i < FB_MAX; i++)
diff --git a/drivers/video/vga16fb.c b/drivers/video/vga16fb.c
index bf638a4..149c47a 100644
--- a/drivers/video/vga16fb.c
+++ b/drivers/video/vga16fb.c
@@ -1263,10 +1263,19 @@ static void vga16fb_imageblit(struct fb_info *info, const struct fb_image *image
 		vga_imageblit_color(info, image);
 }
 
+static void vga16fb_destroy(struct fb_info *info)
+{
+	iounmap(info->screen_base);
+	fb_dealloc_cmap(&info->cmap);
+	/* XXX unshare VGA regions */
+	framebuffer_release(info);
+}
+
 static struct fb_ops vga16fb_ops = {
 	.owner		= THIS_MODULE,
 	.fb_open        = vga16fb_open,
 	.fb_release     = vga16fb_release,
+	.fb_destroy	= vga16fb_destroy,
 	.fb_check_var	= vga16fb_check_var,
 	.fb_set_par	= vga16fb_set_par,
 	.fb_setcolreg 	= vga16fb_setcolreg,
@@ -1306,6 +1315,11 @@ static int __devinit vga16fb_probe(struct platform_device *dev)
 		ret = -ENOMEM;
 		goto err_fb_alloc;
 	}
+	info->apertures = alloc_apertures(1);
+	if (!info->apertures) {
+		ret = -ENOMEM;
+		goto err_ioremap;
+	}
 
 	/* XXX share VGA_FB_PHYS and I/O region with vgacon and others */
 	info->screen_base = (void __iomem *)VGA_MAP_MEM(VGA_FB_PHYS, 0);
@@ -1335,7 +1349,7 @@ static int __devinit vga16fb_probe(struct platform_device *dev)
 	info->fix = vga16fb_fix;
 	/* supports rectangles with widths of multiples of 8 */
 	info->pixmap.blit_x = 1 << 7 | 1 << 15 | 1 << 23 | 1 << 31;
-	info->flags = FBINFO_FLAG_DEFAULT |
+	info->flags = FBINFO_FLAG_DEFAULT | FBINFO_MISC_FIRMWARE |
 		FBINFO_HWACCEL_YPAN;
 
 	i = (info->var.bits_per_pixel = 8) ? 256 : 16;
@@ -1354,6 +1368,9 @@ static int __devinit vga16fb_probe(struct platform_device *dev)
 
 	vga16fb_update_fix(info);
 
+	info->apertures->ranges[0].base = VGA_FB_PHYS;
+	info->apertures->ranges[0].size = VGA_FB_PHYS_LEN;
+
 	if (register_framebuffer(info) < 0) {
 		printk(KERN_ERR "vga16fb: unable to register framebuffer\n");
 		ret = -EINVAL;
@@ -1380,13 +1397,8 @@ static int vga16fb_remove(struct platform_device *dev)
 {
 	struct fb_info *info = platform_get_drvdata(dev);
 
-	if (info) {
+	if (info)
 		unregister_framebuffer(info);
-		iounmap(info->screen_base);
-		fb_dealloc_cmap(&info->cmap);
-	/* XXX unshare VGA regions */
-		framebuffer_release(info);
-	}
 
 	return 0;
 }
diff --git a/include/linux/fb.h b/include/linux/fb.h
index f88e254..1296af4 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -971,7 +971,8 @@ extern ssize_t fb_sys_write(struct fb_info *info, const char __user *buf,
 /* drivers/video/fbmem.c */
 extern int register_framebuffer(struct fb_info *fb_info);
 extern int unregister_framebuffer(struct fb_info *fb_info);
-extern void remove_conflicting_framebuffers(struct apertures_struct *a, const char *name);
+extern void remove_conflicting_framebuffers(struct apertures_struct *a,
+				const char *name, bool primary);
 extern int fb_prepare_logo(struct fb_info *fb_info, int rotate);
 extern int fb_show_logo(struct fb_info *fb_info, int rotate);
 extern char* fb_get_buffer_offset(struct fb_info *info, struct fb_pixmap *buf, u32 size);
-- 
1.7.0.4

Re: [PATCH] vga16fb, drm: vga16fb->drm handoff

[Marcin Slusarz]

On Mon, Apr 19, 2010 at 05:20:10PM +0100, James Simmons wrote:
> 
> > More generic approach below - it should work for all drm drivers.
> > Unfortunately vga16fb handoff has 2 other issues:
> > - It can be compiled as module, so it can be loaded after KMS driver (and
> >   nothing prevents it right now)
> > - vga16fb registration error path is iounmapping memory which was not
> >   ioremapped.
> > I'm going to fix it soon.
> 
> Nak. See comments below.

Thanks for a review.
  
> > ---
> > From: Marcin Slusarz <marcin.slusarz@gmail.com>
> > Subject: [PATCH] vga16fb, drm: vga16fb->drm handoff
> > 
> > let vga16fb claim 0xA0000+0x10000 region as its aperture;
> > drm drivers don't use it, so we have to detect it and kick
> > vga16fb manually - but only if drm is driving the primary card
> > (by checking IORESOURCE_ROM_SHADOW resource flag)
> > 
> > Signed-off-by: Marcin Slusarz <marcin.slusarz@gmail.com>
> > ---
> >  drivers/gpu/drm/i915/intel_fb.c         |    1 +
> >  drivers/gpu/drm/nouveau/nouveau_fbcon.c |    1 +
> >  drivers/gpu/drm/nouveau/nouveau_state.c |    3 ++-
> >  drivers/gpu/drm/radeon/radeon_fb.c      |    1 +
> >  drivers/gpu/drm/vmwgfx/vmwgfx_fb.c      |    1 +
> >  drivers/video/fbmem.c                   |   19 ++++++++++++++++---
> >  drivers/video/vga16fb.c                 |   26 +++++++++++++++++++-------
> >  include/linux/fb.h                      |    4 +++-
> >  8 files changed, 44 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
> > index 6dbc277..8e403be 100644
> > --- a/drivers/gpu/drm/i915/intel_fb.c
> > +++ b/drivers/gpu/drm/i915/intel_fb.c
> > @@ -201,6 +201,7 @@ static int intelfb_create(struct drm_device *dev, uint32_t fb_width,
> >  		info->apertures->ranges[0].size = pci_resource_len(dev->pdev, 2);
> >  	else
> >  		info->apertures->ranges[0].size = pci_resource_len(dev->pdev, 0);
> > +	info->pcidev = dev->pdev;
> 
> No pci junk in struct fb_info. It needs to be bus independent.
> 
> > diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
> > index 7cfcd71..4628a59 100644
> > --- a/drivers/video/fbmem.c
> > +++ b/drivers/video/fbmem.c
> > @@ -32,6 +32,7 @@
> >  #include <linux/device.h>
> >  #include <linux/efi.h>
> >  #include <linux/fb.h>
> > +#include <linux/pci.h>
> 
> Please remove pci header.
>   
> >  #include <asm/fb.h>
> >  
> > @@ -1500,19 +1501,30 @@ static bool fb_do_apertures_overlap(struct apertures_struct *gena,
> >  	return false;
> >  }
> >  
> > -void remove_conflicting_framebuffers(struct apertures_struct *a, const char *name)
> > +#define VGA_FB_PHYS 0xA0000
> > +void remove_conflicting_framebuffers(struct apertures_struct *a,
> > +				     const char *name, struct pci_dev *pcidev)
> >  {
> >  	int i;
> > +	bool primary = false;
> > +
> > +	if (pcidev)
> > +		primary = pcidev->resource[PCI_ROM_RESOURCE].flags & IORESOURCE_ROM_SHADOW;
> 
> Not portable. Please use fb_is_primary_device(info) instead. It will also 
> make your code cleaner.
> 

It simplified the code, but I still need to figure out whether the card is primary when going
the early kick path (when I don't have struct fb_info yet) - the check is now in nouveau.

---
From: Marcin Slusarz <marcin.slusarz@gmail.com>
Subject: [PATCH v2] vga16fb, drm: vga16fb->drm handoff

let vga16fb claim 0xA0000+0x10000 region as its aperture;
drm drivers don't use it, so we have to detect it and kick
vga16fb manually - but only if drm is driving the primary card

Signed-off-by: Marcin Slusarz <marcin.slusarz@gmail.com>
---
 drivers/gpu/drm/nouveau/nouveau_state.c |    7 ++++++-
 drivers/video/fbmem.c                   |   14 +++++++++++---
 drivers/video/vga16fb.c                 |   26 +++++++++++++++++++-------
 include/linux/fb.h                      |    3 ++-
 4 files changed, 38 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_state.c b/drivers/gpu/drm/nouveau/nouveau_state.c
index 3efc339..4c193ed 100644
--- a/drivers/gpu/drm/nouveau/nouveau_state.c
+++ b/drivers/gpu/drm/nouveau/nouveau_state.c
@@ -669,8 +669,13 @@ static int nouveau_remove_conflicting_drivers(struct drm_device *dev)
 	dev_priv->apertures = nouveau_get_apertures(dev);
 	if (!dev_priv->apertures)
 		return -ENOMEM;
+	bool primary = false;
+
+#ifdef CONFIG_X86
+	primary = dev->pdev->resource[PCI_ROM_RESOURCE].flags & IORESOURCE_ROM_SHADOW;
+#endif
 	
-	remove_conflicting_framebuffers(dev_priv->apertures, "nouveaufb");
+	remove_conflicting_framebuffers(dev_priv->apertures, "nouveaufb", primary);
 	return 0;
 }
 
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index 7cfcd71..6deb57c 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -1500,19 +1500,26 @@ static bool fb_do_apertures_overlap(struct apertures_struct *gena,
 	return false;
 }
 
-void remove_conflicting_framebuffers(struct apertures_struct *a, const char *name)
+#define VGA_FB_PHYS 0xA0000
+void remove_conflicting_framebuffers(struct apertures_struct *a,
+				     const char *name, bool primary)
 {
 	int i;
 
 	/* check all firmware fbs and kick off if the base addr overlaps */
 	for (i = 0 ; i < FB_MAX; i++) {
+		struct apertures_struct *gen_aper;
 		if (!registered_fb[i])
 			continue;
 
 		if (!(registered_fb[i]->flags & FBINFO_MISC_FIRMWARE))
 			continue;
+		
+		gen_aper = registered_fb[i]->apertures;
+		if (fb_do_apertures_overlap(gen_aper, a) ||
+			(primary && gen_aper && gen_aper->count &&
+			 gen_aper->ranges[0].base == VGA_FB_PHYS)) {
 
-		if (fb_do_apertures_overlap(registered_fb[i]->apertures, a)) {
 			printk(KERN_ERR "fb: conflicting fb hw usage "
 			       "%s vs %s - removing generic driver\n",
 			       name, registered_fb[i]->fix.id);
@@ -1545,7 +1552,8 @@ register_framebuffer(struct fb_info *fb_info)
 	if (fb_check_foreignness(fb_info))
 		return -ENOSYS;
 
-	remove_conflicting_framebuffers(fb_info->apertures, fb_info->fix.id);
+	remove_conflicting_framebuffers(fb_info->apertures, fb_info->fix.id,
+					 fb_is_primary_device(fb_info));
 
 	num_registered_fb++;
 	for (i = 0 ; i < FB_MAX; i++)
diff --git a/drivers/video/vga16fb.c b/drivers/video/vga16fb.c
index bf638a4..149c47a 100644
--- a/drivers/video/vga16fb.c
+++ b/drivers/video/vga16fb.c
@@ -1263,10 +1263,19 @@ static void vga16fb_imageblit(struct fb_info *info, const struct fb_image *image
 		vga_imageblit_color(info, image);
 }
 
+static void vga16fb_destroy(struct fb_info *info)
+{
+	iounmap(info->screen_base);
+	fb_dealloc_cmap(&info->cmap);
+	/* XXX unshare VGA regions */
+	framebuffer_release(info);
+}
+
 static struct fb_ops vga16fb_ops = {
 	.owner		= THIS_MODULE,
 	.fb_open        = vga16fb_open,
 	.fb_release     = vga16fb_release,
+	.fb_destroy	= vga16fb_destroy,
 	.fb_check_var	= vga16fb_check_var,
 	.fb_set_par	= vga16fb_set_par,
 	.fb_setcolreg 	= vga16fb_setcolreg,
@@ -1306,6 +1315,11 @@ static int __devinit vga16fb_probe(struct platform_device *dev)
 		ret = -ENOMEM;
 		goto err_fb_alloc;
 	}
+	info->apertures = alloc_apertures(1);
+	if (!info->apertures) {
+		ret = -ENOMEM;
+		goto err_ioremap;
+	}
 
 	/* XXX share VGA_FB_PHYS and I/O region with vgacon and others */
 	info->screen_base = (void __iomem *)VGA_MAP_MEM(VGA_FB_PHYS, 0);
@@ -1335,7 +1349,7 @@ static int __devinit vga16fb_probe(struct platform_device *dev)
 	info->fix = vga16fb_fix;
 	/* supports rectangles with widths of multiples of 8 */
 	info->pixmap.blit_x = 1 << 7 | 1 << 15 | 1 << 23 | 1 << 31;
-	info->flags = FBINFO_FLAG_DEFAULT |
+	info->flags = FBINFO_FLAG_DEFAULT | FBINFO_MISC_FIRMWARE |
 		FBINFO_HWACCEL_YPAN;
 
 	i = (info->var.bits_per_pixel == 8) ? 256 : 16;
@@ -1354,6 +1368,9 @@ static int __devinit vga16fb_probe(struct platform_device *dev)
 
 	vga16fb_update_fix(info);
 
+	info->apertures->ranges[0].base = VGA_FB_PHYS;
+	info->apertures->ranges[0].size = VGA_FB_PHYS_LEN;
+
 	if (register_framebuffer(info) < 0) {
 		printk(KERN_ERR "vga16fb: unable to register framebuffer\n");
 		ret = -EINVAL;
@@ -1380,13 +1397,8 @@ static int vga16fb_remove(struct platform_device *dev)
 {
 	struct fb_info *info = platform_get_drvdata(dev);
 
-	if (info) {
+	if (info)
 		unregister_framebuffer(info);
-		iounmap(info->screen_base);
-		fb_dealloc_cmap(&info->cmap);
-	/* XXX unshare VGA regions */
-		framebuffer_release(info);
-	}
 
 	return 0;
 }
diff --git a/include/linux/fb.h b/include/linux/fb.h
index f88e254..1296af4 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -971,7 +971,8 @@ extern ssize_t fb_sys_write(struct fb_info *info, const char __user *buf,
 /* drivers/video/fbmem.c */
 extern int register_framebuffer(struct fb_info *fb_info);
 extern int unregister_framebuffer(struct fb_info *fb_info);
-extern void remove_conflicting_framebuffers(struct apertures_struct *a, const char *name);
+extern void remove_conflicting_framebuffers(struct apertures_struct *a,
+				const char *name, bool primary);
 extern int fb_prepare_logo(struct fb_info *fb_info, int rotate);
 extern int fb_show_logo(struct fb_info *fb_info, int rotate);
 extern char* fb_get_buffer_offset(struct fb_info *info, struct fb_pixmap *buf, u32 size);
-- 
1.7.0.4

Re: [PATCH] vga16fb, drm: vga16fb->drm handoff

[Marcin Slusarz]

On Mon, Apr 19, 2010 at 05:20:10PM +0100, James Simmons wrote:
> 
> > More generic approach below - it should work for all drm drivers.
> > Unfortunately vga16fb handoff has 2 other issues:
> > - It can be compiled as module, so it can be loaded after KMS driver (and
> >   nothing prevents it right now)
> > - vga16fb registration error path is iounmapping memory which was not
> >   ioremapped.
> > I'm going to fix it soon.
> 
> Nak. See comments below.

Thanks for a review.
  
> > ---
> > From: Marcin Slusarz <marcin.slusarz@gmail.com>
> > Subject: [PATCH] vga16fb, drm: vga16fb->drm handoff
> > 
> > let vga16fb claim 0xA0000+0x10000 region as its aperture;
> > drm drivers don't use it, so we have to detect it and kick
> > vga16fb manually - but only if drm is driving the primary card
> > (by checking IORESOURCE_ROM_SHADOW resource flag)
> > 
> > Signed-off-by: Marcin Slusarz <marcin.slusarz@gmail.com>
> > ---
> >  drivers/gpu/drm/i915/intel_fb.c         |    1 +
> >  drivers/gpu/drm/nouveau/nouveau_fbcon.c |    1 +
> >  drivers/gpu/drm/nouveau/nouveau_state.c |    3 ++-
> >  drivers/gpu/drm/radeon/radeon_fb.c      |    1 +
> >  drivers/gpu/drm/vmwgfx/vmwgfx_fb.c      |    1 +
> >  drivers/video/fbmem.c                   |   19 ++++++++++++++++---
> >  drivers/video/vga16fb.c                 |   26 +++++++++++++++++++-------
> >  include/linux/fb.h                      |    4 +++-
> >  8 files changed, 44 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
> > index 6dbc277..8e403be 100644
> > --- a/drivers/gpu/drm/i915/intel_fb.c
> > +++ b/drivers/gpu/drm/i915/intel_fb.c
> > @@ -201,6 +201,7 @@ static int intelfb_create(struct drm_device *dev, uint32_t fb_width,
> >  		info->apertures->ranges[0].size = pci_resource_len(dev->pdev, 2);
> >  	else
> >  		info->apertures->ranges[0].size = pci_resource_len(dev->pdev, 0);
> > +	info->pcidev = dev->pdev;
> 
> No pci junk in struct fb_info. It needs to be bus independent.
> 
> > diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
> > index 7cfcd71..4628a59 100644
> > --- a/drivers/video/fbmem.c
> > +++ b/drivers/video/fbmem.c
> > @@ -32,6 +32,7 @@
> >  #include <linux/device.h>
> >  #include <linux/efi.h>
> >  #include <linux/fb.h>
> > +#include <linux/pci.h>
> 
> Please remove pci header.
>   
> >  #include <asm/fb.h>
> >  
> > @@ -1500,19 +1501,30 @@ static bool fb_do_apertures_overlap(struct apertures_struct *gena,
> >  	return false;
> >  }
> >  
> > -void remove_conflicting_framebuffers(struct apertures_struct *a, const char *name)
> > +#define VGA_FB_PHYS 0xA0000
> > +void remove_conflicting_framebuffers(struct apertures_struct *a,
> > +				     const char *name, struct pci_dev *pcidev)
> >  {
> >  	int i;
> > +	bool primary = false;
> > +
> > +	if (pcidev)
> > +		primary = pcidev->resource[PCI_ROM_RESOURCE].flags & IORESOURCE_ROM_SHADOW;
> 
> Not portable. Please use fb_is_primary_device(info) instead. It will also 
> make your code cleaner.
> 

It simplified the code, but I still need to figure out whether the card is primary when going
the early kick path (when I don't have struct fb_info yet) - the check is now in nouveau.

---
From: Marcin Slusarz <marcin.slusarz@gmail.com>
Subject: [PATCH v2] vga16fb, drm: vga16fb->drm handoff

let vga16fb claim 0xA0000+0x10000 region as its aperture;
drm drivers don't use it, so we have to detect it and kick
vga16fb manually - but only if drm is driving the primary card

Signed-off-by: Marcin Slusarz <marcin.slusarz@gmail.com>
---
 drivers/gpu/drm/nouveau/nouveau_state.c |    7 ++++++-
 drivers/video/fbmem.c                   |   14 +++++++++++---
 drivers/video/vga16fb.c                 |   26 +++++++++++++++++++-------
 include/linux/fb.h                      |    3 ++-
 4 files changed, 38 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_state.c b/drivers/gpu/drm/nouveau/nouveau_state.c
index 3efc339..4c193ed 100644
--- a/drivers/gpu/drm/nouveau/nouveau_state.c
+++ b/drivers/gpu/drm/nouveau/nouveau_state.c
@@ -669,8 +669,13 @@ static int nouveau_remove_conflicting_drivers(struct drm_device *dev)
 	dev_priv->apertures = nouveau_get_apertures(dev);
 	if (!dev_priv->apertures)
 		return -ENOMEM;
+	bool primary = false;
+
+#ifdef CONFIG_X86
+	primary = dev->pdev->resource[PCI_ROM_RESOURCE].flags & IORESOURCE_ROM_SHADOW;
+#endif
 	
-	remove_conflicting_framebuffers(dev_priv->apertures, "nouveaufb");
+	remove_conflicting_framebuffers(dev_priv->apertures, "nouveaufb", primary);
 	return 0;
 }
 
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index 7cfcd71..6deb57c 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -1500,19 +1500,26 @@ static bool fb_do_apertures_overlap(struct apertures_struct *gena,
 	return false;
 }
 
-void remove_conflicting_framebuffers(struct apertures_struct *a, const char *name)
+#define VGA_FB_PHYS 0xA0000
+void remove_conflicting_framebuffers(struct apertures_struct *a,
+				     const char *name, bool primary)
 {
 	int i;
 
 	/* check all firmware fbs and kick off if the base addr overlaps */
 	for (i = 0 ; i < FB_MAX; i++) {
+		struct apertures_struct *gen_aper;
 		if (!registered_fb[i])
 			continue;
 
 		if (!(registered_fb[i]->flags & FBINFO_MISC_FIRMWARE))
 			continue;
+		
+		gen_aper = registered_fb[i]->apertures;
+		if (fb_do_apertures_overlap(gen_aper, a) ||
+			(primary && gen_aper && gen_aper->count &&
+			 gen_aper->ranges[0].base == VGA_FB_PHYS)) {
 
-		if (fb_do_apertures_overlap(registered_fb[i]->apertures, a)) {
 			printk(KERN_ERR "fb: conflicting fb hw usage "
 			       "%s vs %s - removing generic driver\n",
 			       name, registered_fb[i]->fix.id);
@@ -1545,7 +1552,8 @@ register_framebuffer(struct fb_info *fb_info)
 	if (fb_check_foreignness(fb_info))
 		return -ENOSYS;
 
-	remove_conflicting_framebuffers(fb_info->apertures, fb_info->fix.id);
+	remove_conflicting_framebuffers(fb_info->apertures, fb_info->fix.id,
+					 fb_is_primary_device(fb_info));
 
 	num_registered_fb++;
 	for (i = 0 ; i < FB_MAX; i++)
diff --git a/drivers/video/vga16fb.c b/drivers/video/vga16fb.c
index bf638a4..149c47a 100644
--- a/drivers/video/vga16fb.c
+++ b/drivers/video/vga16fb.c
@@ -1263,10 +1263,19 @@ static void vga16fb_imageblit(struct fb_info *info, const struct fb_image *image
 		vga_imageblit_color(info, image);
 }
 
+static void vga16fb_destroy(struct fb_info *info)
+{
+	iounmap(info->screen_base);
+	fb_dealloc_cmap(&info->cmap);
+	/* XXX unshare VGA regions */
+	framebuffer_release(info);
+}
+
 static struct fb_ops vga16fb_ops = {
 	.owner		= THIS_MODULE,
 	.fb_open        = vga16fb_open,
 	.fb_release     = vga16fb_release,
+	.fb_destroy	= vga16fb_destroy,
 	.fb_check_var	= vga16fb_check_var,
 	.fb_set_par	= vga16fb_set_par,
 	.fb_setcolreg 	= vga16fb_setcolreg,
@@ -1306,6 +1315,11 @@ static int __devinit vga16fb_probe(struct platform_device *dev)
 		ret = -ENOMEM;
 		goto err_fb_alloc;
 	}
+	info->apertures = alloc_apertures(1);
+	if (!info->apertures) {
+		ret = -ENOMEM;
+		goto err_ioremap;
+	}
 
 	/* XXX share VGA_FB_PHYS and I/O region with vgacon and others */
 	info->screen_base = (void __iomem *)VGA_MAP_MEM(VGA_FB_PHYS, 0);
@@ -1335,7 +1349,7 @@ static int __devinit vga16fb_probe(struct platform_device *dev)
 	info->fix = vga16fb_fix;
 	/* supports rectangles with widths of multiples of 8 */
 	info->pixmap.blit_x = 1 << 7 | 1 << 15 | 1 << 23 | 1 << 31;
-	info->flags = FBINFO_FLAG_DEFAULT |
+	info->flags = FBINFO_FLAG_DEFAULT | FBINFO_MISC_FIRMWARE |
 		FBINFO_HWACCEL_YPAN;
 
 	i = (info->var.bits_per_pixel == 8) ? 256 : 16;
@@ -1354,6 +1368,9 @@ static int __devinit vga16fb_probe(struct platform_device *dev)
 
 	vga16fb_update_fix(info);
 
+	info->apertures->ranges[0].base = VGA_FB_PHYS;
+	info->apertures->ranges[0].size = VGA_FB_PHYS_LEN;
+
 	if (register_framebuffer(info) < 0) {
 		printk(KERN_ERR "vga16fb: unable to register framebuffer\n");
 		ret = -EINVAL;
@@ -1380,13 +1397,8 @@ static int vga16fb_remove(struct platform_device *dev)
 {
 	struct fb_info *info = platform_get_drvdata(dev);
 
-	if (info) {
+	if (info)
 		unregister_framebuffer(info);
-		iounmap(info->screen_base);
-		fb_dealloc_cmap(&info->cmap);
-	/* XXX unshare VGA regions */
-		framebuffer_release(info);
-	}
 
 	return 0;
 }
diff --git a/include/linux/fb.h b/include/linux/fb.h
index f88e254..1296af4 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -971,7 +971,8 @@ extern ssize_t fb_sys_write(struct fb_info *info, const char __user *buf,
 /* drivers/video/fbmem.c */
 extern int register_framebuffer(struct fb_info *fb_info);
 extern int unregister_framebuffer(struct fb_info *fb_info);
-extern void remove_conflicting_framebuffers(struct apertures_struct *a, const char *name);
+extern void remove_conflicting_framebuffers(struct apertures_struct *a,
+				const char *name, bool primary);
 extern int fb_prepare_logo(struct fb_info *fb_info, int rotate);
 extern int fb_show_logo(struct fb_info *fb_info, int rotate);
 extern char* fb_get_buffer_offset(struct fb_info *info, struct fb_pixmap *buf, u32 size);
-- 
1.7.0.4

Re: [PATCH 1/3] fbmem: fix aperture overlapping check

[Dave Airlie]

On Sat, 2010-04-10 at 21:55 +0200, marcin.slusarz@gmail.com wrote:
> fb_do_apertures_overlap is returning wrong value when one aperture
> is completely whithin the other. Add generic ranges_overlap macro
> (probably kernel.h candidate) and use it here.
> 

That doesn't seem right.

The rules are:

the generic aperture has to be equal or smaller than the hw aperture,
otherwise the generic driver will be trashing random hw pieces on the
machine.

So with that in mind, the check makes sure the generic aperture starts
somewhere inside the hw aperture, which the test clearly gets right.

Have you got a pointer to a machine where it fails?

Dave.


> Signed-off-by: Marcin Slusarz <marcin.slusarz@gmail.com>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Peter Jones <pjones@redhat.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> ---
>  drivers/video/fbmem.c |   24 +++++++++++++++++-------
>  1 files changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
> index a15b44e..41f2e5e 100644
> --- a/drivers/video/fbmem.c
> +++ b/drivers/video/fbmem.c
> @@ -1468,15 +1468,25 @@ static int fb_check_foreignness(struct fb_info *fi)
>  	return 0;
>  }
>  
> +/**
> + * ranges_overlap - check whether two ranges overlap (their intersection is not empty)
> + * @start1: start of the first range
> + * @size1:  length of the first range
> + * @start2: start of the second range
> + * @size2:  length of the second range
> + */
> +#define ranges_overlap(start1, size1, start2, size2) ({	\
> +	typeof(start1) __start1 = (start1);		\
> +	typeof(size1)  __size1  = (size1);		\
> +	typeof(start2) __start2 = (start2);		\
> +	typeof(size2)  __size2  = (size2);		\
> +	__start1 < __start2 + __size2 && __start1 + __size1 > __start2; \
> +})
> +
>  static bool fb_do_apertures_overlap(struct fb_info *gen, struct fb_info *hw)
>  {
> -	/* is the generic aperture base the same as the HW one */
> -	if (gen->aperture_base == hw->aperture_base)
> -		return true;
> -	/* is the generic aperture base inside the hw base->hw base+size */
> -	if (gen->aperture_base > hw->aperture_base && gen->aperture_base <= hw->aperture_base + hw->aperture_size)
> -		return true;
> -	return false;
> +	return ranges_overlap(gen->aperture_base, gen->aperture_size,
> +				hw->aperture_base, hw->aperture_size);
>  }
>  /**
>   *	register_framebuffer - registers a frame buffer device