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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to