From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755130Ab0DTTzP (ORCPT ); Tue, 20 Apr 2010 15:55:15 -0400 Received: from ey-out-2122.google.com ([74.125.78.27]:51095 "EHLO ey-out-2122.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754059Ab0DTTzL (ORCPT ); Tue, 20 Apr 2010 15:55:11 -0400 DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; b=LMBMOH3LqCsyOA5SUgOynZ4LNvkQfLTXad3OJ+r627us7lOKhwmAw8/91lBcfg01+U Tv9mkZX64/ib4iNsOqK4RMxQW+FLT1lw6lQFdbKc0SoJPTIYnwchAFshglUtYIXO2AgF YRlhcsCluMlsIJC3t5NDqq9kRVUIMJqXzHHtY= Date: Tue, 20 Apr 2010 21:54:41 +0200 From: Marcin Slusarz To: James Simmons Cc: Dave Airlie , linux-fbdev@vger.kernel.org, nouveau@lists.freedesktop.org, LKML , dri-devel , Peter Jones , Andrew Morton Subject: Re: [PATCH] vga16fb, drm: vga16fb->drm handoff Message-ID: <20100420195441.GA2915@joi.lan> References: <1270929334-3742-1-git-send-email-marcin.slusarz@gmail.com> <1271030068.3554.1.camel@localhost> <20100412113438.GB2789@joi.lan> <1271104101.5726.3.camel@t60prh> <20100412213327.GA4463@joi.lan> <20100413195004.GA4402@joi.lan> <20100418215751.GA2733@joi.lan> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.20 (2009-06-14) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > > 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 > > --- > > 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 > > #include > > #include > > +#include > > Please remove pci header. > > > #include > > > > @@ -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 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 --- 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 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcin Slusarz Date: Tue, 20 Apr 2010 19:54:41 +0000 Subject: Re: [PATCH] vga16fb, drm: vga16fb->drm handoff Message-Id: <20100420195441.GA2915@joi.lan> List-Id: References: <1270929334-3742-1-git-send-email-marcin.slusarz@gmail.com> <1271030068.3554.1.camel@localhost> <20100412113438.GB2789@joi.lan> <1271104101.5726.3.camel@t60prh> <20100412213327.GA4463@joi.lan> <20100413195004.GA4402@joi.lan> <20100418215751.GA2733@joi.lan> In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: James Simmons Cc: linux-fbdev@vger.kernel.org, nouveau@lists.freedesktop.org, LKML , dri-devel , Peter Jones , Dave Airlie , Andrew Morton 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 > > 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 > > --- > > 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 > > #include > > #include > > +#include > > Please remove pci header. > > > #include > > > > @@ -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 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 --- 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 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcin Slusarz Subject: Re: [PATCH] vga16fb, drm: vga16fb->drm handoff Date: Tue, 20 Apr 2010 21:54:41 +0200 Message-ID: <20100420195441.GA2915@joi.lan> References: <1270929334-3742-1-git-send-email-marcin.slusarz@gmail.com> <1271030068.3554.1.camel@localhost> <20100412113438.GB2789@joi.lan> <1271104101.5726.3.camel@t60prh> <20100412213327.GA4463@joi.lan> <20100413195004.GA4402@joi.lan> <20100418215751.GA2733@joi.lan> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces@lists.freedesktop.org Errors-To: dri-devel-bounces@lists.freedesktop.org To: James Simmons Cc: linux-fbdev@vger.kernel.org, nouveau@lists.freedesktop.org, LKML , dri-devel , Peter Jones , Dave Airlie , Andrew Morton List-Id: nouveau.vger.kernel.org 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 > > 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 > > --- > > 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 > > #include > > #include > > +#include > > Please remove pci header. > > > #include > > > > @@ -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 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 --- 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