[PATCH] fb: add support for handoff from firmware to hw framebuffers

[PATCH] fb: add support for handoff from firmware to hw framebuffers

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

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 <airlied@redhat.com>
---
 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;
+
 	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)
+{
+	/* 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;
+			}
+		}
+	}
+
 	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
-- 
1.5.4.1


------------------------------------------------------------------------------
OpenSolaris 2009.06 is a cutting edge operating system for enterprises 
looking to deploy the next generation of Solaris that includes the latest 
innovations from Sun and the OpenSource community. Download a copy and 
enjoy capabilities such as Networking, Storage and Virtualization. 
Go to: http://p.sf.net/sfu/opensolaris-get

Re: [PATCH] fb: add support for handoff from firmware to hw framebuffers

[Dave Airlie]

On Thu, Jun 4, 2009 at 2:46 PM, Dave Airlie <airlied@gmail.com> wrote:
> From: Dave Airlie <airlied@redhat.com>
>
> 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.

btw so far this is just an RFC.

Dave.

------------------------------------------------------------------------------
OpenSolaris 2009.06 is a cutting edge operating system for enterprises 
looking to deploy the next generation of Solaris that includes the latest 
innovations from Sun and the OpenSource community. Download a copy and 
enjoy capabilities such as Networking, Storage and Virtualization. 
Go to: http://p.sf.net/sfu/opensolaris-get

Re: [PATCH] fb: add support for handoff from firmware to hw framebuffers

[Geert Uytterhoeven]

On Thu, Jun 4, 2009 at 06:50, Dave Airlie <airlied@gmail.com> wrote:
> On Thu, Jun 4, 2009 at 2:46 PM, Dave Airlie <airlied@gmail.com> wrote:
>> From: Dave Airlie <airlied@redhat.com>
>>
>> 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.
>
> btw so far this is just an RFC.

But if it's accepted, you also want to add this support to offb.c, and
probably even macfb.c (dupes with valkyriefb.c).

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

------------------------------------------------------------------------------
OpenSolaris 2009.06 is a cutting edge operating system for enterprises 
looking to deploy the next generation of Solaris that includes the latest 
innovations from Sun and the OpenSource community. Download a copy and 
enjoy capabilities such as Networking, Storage and Virtualization. 
Go to: http://p.sf.net/sfu/opensolaris-get

Re: [PATCH] fb: add support for handoff from firmware to hw framebuffers

[Peter Jones]

On 06/04/2009 12:46 AM, Dave Airlie wrote:
> From: Dave Airlie <airlied@redhat.com>
> 
> 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 <airlied@redhat.com>
> ---
>  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

------------------------------------------------------------------------------
OpenSolaris 2009.06 is a cutting edge operating system for enterprises 
looking to deploy the next generation of Solaris that includes the latest 
innovations from Sun and the OpenSource community. Download a copy and 
enjoy capabilities such as Networking, Storage and Virtualization. 
Go to: http://p.sf.net/sfu/opensolaris-get

Re: [PATCH] fb: add support for handoff from firmware to hw framebuffers

[Dave Airlie]

On Fri, Jun 5, 2009 at 12:43 AM, Peter Jones<pjones@redhat.com> wrote:
> On 06/04/2009 12:46 AM, Dave Airlie wrote:
>> From: Dave Airlie <airlied@redhat.com>
>>
>> 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 <airlied@redhat.com>
>> ---
>>  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() .

Probably no need, they'll overlap at some point, and we can't always assume
you'll have a PCI device behind it (well you probably can for efifb.)

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

Changed in v2.

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

Shouldn't matter, the kms drivers won't create an fbdev if nomodeset
is specified so we never get to this point

I've sent v2 which is probably ready for inclusion, not sure what the
path upstream is,

After its merged you can add an efifb fb_destroy function, and the
powerpc people can add offb support.

Dave.

------------------------------------------------------------------------------
OpenSolaris 2009.06 is a cutting edge operating system for enterprises 
looking to deploy the next generation of Solaris that includes the latest 
innovations from Sun and the OpenSource community. Download a copy and 
enjoy capabilities such as Networking, Storage and Virtualization. 
Go to: http://p.sf.net/sfu/opensolaris-get