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

--

[ message continues ]

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?



--

[ message continues ]

No, it failed with an artifical test while I was working on vga16fb handoff
--

[ message continues ]

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

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.


--

[ message continues ]

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

--

[ message continues ]

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

[ message continues ]

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 @@ ...
[ message continues ]

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
+++ ...
[ message continues ]

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

--

[ message continues ]

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,
+				     ...
[ message continues ]