Menu
▾
▴
Re: [Linux-fbdev-devel] [PATCH] fb: add support for handoff from firmware to hw framebuffers
|
From: Peter J. <pj...@re...> - 2009-06-04 14:43:23
|
On 06/04/2009 12:46 AM, Dave Airlie wrote:
> From: Dave Airlie <ai...@re...>
>
> With KMS we have ran into an issue where we really want the KMS fb driver
> to be the one running the console, so panics etc can be shown by switching
> out of X etc.
>
> However with vesafb/efifb built-in, we end up with those on fb0 and the
> KMS fb driver on fb1, driving the same piece of hw, so this adds an fb info
> flag to denote a firmware fbdev, and adds a new aperture base/size range
> which can be compared when the hw drivers are installed to see if there
> is a conflict with a firmware driver, and if there is the firmware driver is
> unregistered and the hw driver takes over.
>
> It uses new aperture_base/size members instead of comparing on the fix
> smem_start/length, as smem_start/length might for example only cover the
> first 1MB of the PCI aperture, and we could allocate the kms fb from 8MB
> into the aperture, thus they would never overlap.
>
> Signed-off-by: Dave Airlie <ai...@re...>
> ---
> drivers/gpu/drm/i915/intel_fb.c | 8 ++++++++
> drivers/video/efifb.c | 5 ++++-
> drivers/video/fbmem.c | 24 ++++++++++++++++++++++++
> drivers/video/vesafb.c | 6 +++++-
> include/linux/fb.h | 9 ++++++++-
> 5 files changed, 49 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
> index e4652dc..f1849d3 100644
> --- a/drivers/gpu/drm/i915/intel_fb.c
> +++ b/drivers/gpu/drm/i915/intel_fb.c
> @@ -504,6 +504,14 @@ static int intelfb_create(struct drm_device *dev, uint32_t fb_width,
> info->fbops = &intelfb_ops;
>
> info->fix.line_length = fb->pitch;
> +
> + /* setup aperture base/size for vesafb takeover */
> + info->aperture_base = dev->mode_config.fb_base;
> + if (IS_I9XX(dev))
> + info->aperture_size = pci_resource_len(dev->pdev, 2);
> + else
> + info->aperture_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/video/efifb.c b/drivers/video/efifb.c
> index 8dea2bc..eb12182 100644
> --- a/drivers/video/efifb.c
> +++ b/drivers/video/efifb.c
> @@ -280,6 +280,9 @@ static int __init 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_total;
> +
It seems like we aught to be looking up the PCI BAR here, reserving it, and
using that as aperture_{base,size} -- though that's not strictly required by
fb_compare_aperture_sizes() .
> info->screen_base = ioremap(efifb_fix.smem_start, efifb_fix.smem_len);
> if (!info->screen_base) {
> printk(KERN_ERR "efifb: abort, cannot ioremap video memory "
> @@ -337,7 +340,7 @@ static int __init efifb_probe(struct platform_device *dev)
> info->fbops = &efifb_ops;
> info->var = efifb_defined;
> info->fix = efifb_fix;
> - info->flags = FBINFO_FLAG_DEFAULT;
> + info->flags = FBINFO_FLAG_DEFAULT | FBINFO_MISC_FIRMWARE;
>
> if ((err = fb_alloc_cmap(&info->cmap, 256, 0)) < 0) {
> printk(KERN_ERR "efifb: cannot allocate colormap\n");
> diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
> index d412a1d..3dd033b 100644
> --- a/drivers/video/fbmem.c
> +++ b/drivers/video/fbmem.c
> @@ -1462,6 +1462,16 @@ static int fb_check_foreignness(struct fb_info *fi)
> return 0;
> }
>
> +static bool fb_compare_aperture_sizes(struct fb_info *gen, struct fb_info *hw)
The name here seems weird -- something like fb_do_apertures_overlap() or even just fb_compare_apertures() would be more clear to me.
> +{
> + /* 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;
> +}
> /**
> * register_framebuffer - registers a frame buffer device
> * @fb_info: frame buffer info structure
> @@ -1485,6 +1495,20 @@ 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_compare_aperture_sizes(registered_fb[i], fb_info)) {
> + printk("fb: conflicting fb hw usage - removing generic driver\n", registered_fb[i]->aperture_base, fb_info->aperture_base);
> + unregister_framebuffer(registered_fb[i]);
> + break;
> + }
> + }
> + }
Seems like we should we also have a "nomodeset" check in this codepath.
> +
> num_registered_fb++;
> for (i = 0 ; i < FB_MAX; i++)
> if (!registered_fb[i])
> diff --git a/drivers/video/vesafb.c b/drivers/video/vesafb.c
> index d6856f4..52f03b2 100644
> --- a/drivers/video/vesafb.c
> +++ b/drivers/video/vesafb.c
> @@ -286,6 +286,10 @@ static int __init vesafb_probe(struct platform_device *dev)
> info->pseudo_palette = info->par;
> info->par = NULL;
>
> + /* set vesafb aperture size for generic probing */
> + info->aperture_base = screen_info.lfb_base;
> + info->aperture_size = size_total;
> +
> info->screen_base = ioremap(vesafb_fix.smem_start, vesafb_fix.smem_len);
> if (!info->screen_base) {
> printk(KERN_ERR
> @@ -437,7 +441,7 @@ static int __init vesafb_probe(struct platform_device *dev)
> info->fbops = &vesafb_ops;
> info->var = vesafb_defined;
> info->fix = vesafb_fix;
> - info->flags = FBINFO_FLAG_DEFAULT |
> + info->flags = FBINFO_FLAG_DEFAULT | FBINFO_MISC_FIRMWARE |
> (ypan ? FBINFO_HWACCEL_YPAN : 0);
>
> if (!ypan)
> diff --git a/include/linux/fb.h b/include/linux/fb.h
> index 330c4b1..88cbe56 100644
> --- a/include/linux/fb.h
> +++ b/include/linux/fb.h
> @@ -786,6 +786,8 @@ struct fb_tile_ops {
> #define FBINFO_MISC_USEREVENT 0x10000 /* event request
> from userspace */
> #define FBINFO_MISC_TILEBLITTING 0x20000 /* use tile blitting */
> +#define FBINFO_MISC_FIRMWARE 0x40000 /* a replaceable firmware
> + inited framebuffer */
>
> /* A driver may set this flag to indicate that it does want a set_par to be
> * called every time when fbcon_switch is executed. The advantage is that with
> @@ -854,7 +856,12 @@ struct fb_info {
> u32 state; /* Hardware state i.e suspend */
> void *fbcon_par; /* fbcon use-only private area */
> /* From here on everything is device dependent */
> - void *par;
> + void *par;
> + /* 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;
> };
>
> #ifdef MODULE
--
Peter
|
