vt/fbcon binding and handover fixes

vt/fbcon binding and handover fixes

[Dave Airlie]

I've been working on some issues with the fb handoff between vesafb and KMS
on my machine with a dual-gpu card. These 3 patches are the primary result
of this, to fix a number of issues where the VT layer and fbcon layers
got themselves into a place that they couldn't get out off, having the second
card complicates things nicely where we have an fbdev registered but fbcon
gets unbound to the dummy console and can't get back.

Also a fix to the overlap tests which had an off-by-one.

Dave.

[PATCH 1/3] fb: fix overlapping test off-by-one.

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

On my system with a radeon x2, the first GPU was not overlapping vesa
but the test decided it was.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/video/fbmem.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index 0e6aa3d..4ac1201 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -1458,7 +1458,7 @@ static bool apertures_overlap(struct aperture *gen, struct aperture *hw)
 	if (gen->base == hw->base)
 		return true;
 	/* is the generic aperture base inside the hw base->hw base+size */
-	if (gen->base > hw->base && gen->base <= hw->base + hw->size)
+	if (gen->base > hw->base && gen->base < hw->base + hw->size)
 		return true;
 	return false;
 }
-- 
1.7.1

[PATCH 2/3] vt: fix issue when fbcon wants to takeover a second time.

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

With framebuffer handover and multiple GPUs, we get into a
position where the fbcon unbinds the vesafb framebuffer for GPU 1,
but we still have a radeon framebuffer bound from GPU 0, so
we don't unregister the console driver. Then when we tried to bind
the new radeon framebuffer for GPU1 we never get to the bind
call as we fail due to the console being registered already.

This changes the return value to -EBUSY when the driver is
already registered and continues to bind for -EBUSY.

Signed-off-by: Dave Airlie <airlied@redhat.com>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: Greg KH <greg@suse.de>
---
 drivers/tty/vt/vt.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index a8ec48e..d781496 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -3524,7 +3524,7 @@ int register_con_driver(const struct consw *csw, int first, int last)
 
 		/* already registered */
 		if (con_driver->con == csw)
-			retval = -EINVAL;
+			retval = -EBUSY;
 	}
 
 	if (retval)
@@ -3635,7 +3635,12 @@ int take_over_console(const struct consw *csw, int first, int last, int deflt)
 	int err;
 
 	err = register_con_driver(csw, first, last);
-
+	/* if we get an busy error we still want to bind the console driver
+	 * and return success, as we may have unbound the console driver
+	 * but not unregistered it.
+	 */
+	if (err == -EBUSY)
+		err = 0;
 	if (!err)
 		bind_con_driver(csw, first, last, deflt);
 
-- 
1.7.1

[PATCH 3/3] fbcon: fix situation where fbcon gets deinitialised and can't reinit.

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

Situation as follow:
2 GPUs + vesafb + kms.

GPU 1 is primary, vesafb binds to it as fb0
radeon loads
GPU 0 loads as fb1
GPU 1 loads, vesafb gets kicked off which causes fb0 to unbind
console, which causes the dummy console to rebind.

this means fbcon_deinit gets called, which calls fbcon_exit
since the console isn't bound anymore and we set fbcon_has_exited.

GPU 1 creates a new fb0 which is primary and we want to be console.
fbcon_fb_registered gets called sets the primary up and calls set_con2fb_map,
however as fbcon_has_exited is set nothing further ever happens.

This patch bypasses the fbcon_has_exited and checks if the console is unbound,
if its unbound it calls the fbcon_takeover which calls the vt layer to
call the fbcon_startup method and everthing works.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/video/console/fbcon.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
index 7ccc967..6662687 100644
--- a/drivers/video/console/fbcon.c
+++ b/drivers/video/console/fbcon.c
@@ -823,10 +823,10 @@ static int set_con2fb_map(int unit, int newidx, int user)
 	if (oldidx == newidx)
 		return 0;
 
-	if (!info || fbcon_has_exited)
+	if (!info)
 		return -EINVAL;
 
- 	if (!err && !search_for_mapped_con()) {
+	if (!search_for_mapped_con() || !con_is_bound(&fb_con)) {
 		info_idx = newidx;
 		return fbcon_takeover(0);
 	}
-- 
1.7.1

Re: [PATCH 1/3] fb: fix overlapping test off-by-one.

[Michel Dänzer]

On Die, 2010-12-21 at 11:41 +1000, Dave Airlie wrote: 
> From: Dave Airlie <airlied@redhat.com>
> 
> On my system with a radeon x2, the first GPU was not overlapping vesa
> but the test decided it was.
> 
> Signed-off-by: Dave Airlie <airlied@redhat.com>
> ---
>  drivers/video/fbmem.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
> index 0e6aa3d..4ac1201 100644
> --- a/drivers/video/fbmem.c
> +++ b/drivers/video/fbmem.c
> @@ -1458,7 +1458,7 @@ static bool apertures_overlap(struct aperture *gen, struct aperture *hw)
>  	if (gen->base == hw->base)
>  		return true;
>  	/* is the generic aperture base inside the hw base->hw base+size */
> -	if (gen->base > hw->base && gen->base <= hw->base + hw->size)
> +	if (gen->base > hw->base && gen->base < hw->base + hw->size)

Good catch.

Reviewed-by: Michel Dänzer <michel@daenzer.net>


-- 
Earthling Michel Dänzer           |                http://www.vmware.com
Libre software enthusiast         |          Debian, X and DRI developer

Re: [PATCH 1/3] fb: fix overlapping test off-by-one.

[Paul Mundt]

On Tue, Dec 21, 2010 at 08:29:03AM +0100, Michel D?nzer wrote:
> On Die, 2010-12-21 at 11:41 +1000, Dave Airlie wrote: 
> > From: Dave Airlie <airlied@redhat.com>
> > 
> > On my system with a radeon x2, the first GPU was not overlapping vesa
> > but the test decided it was.
> > 
> > Signed-off-by: Dave Airlie <airlied@redhat.com>
> > ---
> >  drivers/video/fbmem.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
> > index 0e6aa3d..4ac1201 100644
> > --- a/drivers/video/fbmem.c
> > +++ b/drivers/video/fbmem.c
> > @@ -1458,7 +1458,7 @@ static bool apertures_overlap(struct aperture *gen, struct aperture *hw)
> >  	if (gen->base == hw->base)
> >  		return true;
> >  	/* is the generic aperture base inside the hw base->hw base+size */
> > -	if (gen->base > hw->base && gen->base <= hw->base + hw->size)
> > +	if (gen->base > hw->base && gen->base < hw->base + hw->size)
> 
> Good catch.
> 
> Reviewed-by: Michel D??nzer <michel@daenzer.net>
> 
I'll queue this for .37, thanks.

Re: [PATCH 3/3] fbcon: fix situation where fbcon gets deinitialised and can't reinit.

[Paul Mundt]

On Tue, Dec 21, 2010 at 11:41:17AM +1000, Dave Airlie wrote:
> From: Dave Airlie <airlied@redhat.com>
> 
> Situation as follow:
> 2 GPUs + vesafb + kms.
> 
> GPU 1 is primary, vesafb binds to it as fb0
> radeon loads
> GPU 0 loads as fb1
> GPU 1 loads, vesafb gets kicked off which causes fb0 to unbind
> console, which causes the dummy console to rebind.
> 
> this means fbcon_deinit gets called, which calls fbcon_exit
> since the console isn't bound anymore and we set fbcon_has_exited.
> 
> GPU 1 creates a new fb0 which is primary and we want to be console.
> fbcon_fb_registered gets called sets the primary up and calls set_con2fb_map,
> however as fbcon_has_exited is set nothing further ever happens.
> 
> This patch bypasses the fbcon_has_exited and checks if the console is unbound,
> if its unbound it calls the fbcon_takeover which calls the vt layer to
> call the fbcon_startup method and everthing works.
> 
> Signed-off-by: Dave Airlie <airlied@redhat.com>

This one looks fine, too. This does presumably depend on the change in
behaviour introduced by your 2/3 patch though. I'll hold off on applying
this until that one gets acked, then we can do this either for the next
-rc or for .38.

Re: [PATCH 2/3] vt: fix issue when fbcon wants to takeover a second time.

[Dave Airlie]

Hi Greg,

here is the patch I think you missed.

Dave.

On Tue, Dec 21, 2010 at 11:41 AM, Dave Airlie <airlied@gmail.com> wrote:
> From: Dave Airlie <airlied@redhat.com>
>
> With framebuffer handover and multiple GPUs, we get into a
> position where the fbcon unbinds the vesafb framebuffer for GPU 1,
> but we still have a radeon framebuffer bound from GPU 0, so
> we don't unregister the console driver. Then when we tried to bind
> the new radeon framebuffer for GPU1 we never get to the bind
> call as we fail due to the console being registered already.
>
> This changes the return value to -EBUSY when the driver is
> already registered and continues to bind for -EBUSY.
>
> Signed-off-by: Dave Airlie <airlied@redhat.com>
> Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
> Cc: Greg KH <greg@suse.de>
> ---
>  drivers/tty/vt/vt.c |    9 +++++++--
>  1 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index a8ec48e..d781496 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -3524,7 +3524,7 @@ int register_con_driver(const struct consw *csw, int first, int last)
>
>                /* already registered */
>                if (con_driver->con == csw)
> -                       retval = -EINVAL;
> +                       retval = -EBUSY;
>        }
>
>        if (retval)
> @@ -3635,7 +3635,12 @@ int take_over_console(const struct consw *csw, int first, int last, int deflt)
>        int err;
>
>        err = register_con_driver(csw, first, last);
> -
> +       /* if we get an busy error we still want to bind the console driver
> +        * and return success, as we may have unbound the console driver
> +        * but not unregistered it.
> +        */
> +       if (err == -EBUSY)
> +               err = 0;
>        if (!err)
>                bind_con_driver(csw, first, last, deflt);
>
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH 2/3] vt: fix issue when fbcon wants to takeover a second time.

[Greg KH]

On Fri, Jan 07, 2011 at 09:57:41AM +1000, Dave Airlie wrote:
> Hi Greg,
> 
> here is the patch I think you missed.

Ah, thanks, I did.  I'll queue this up for after .38-rc1 is out as it's
a bug fix.  It should go to -stable as well for .37 right?

thanks,

greg k-h

Re: [PATCH 2/3] vt: fix issue when fbcon wants to takeover a second time.

[Dave Airlie]

On Thu, 2011-01-06 at 16:50 -0800, Greg KH wrote:
> On Fri, Jan 07, 2011 at 09:57:41AM +1000, Dave Airlie wrote:
> > Hi Greg,
> > 
> > here is the patch I think you missed.
> 
> Ah, thanks, I did.  I'll queue this up for after .38-rc1 is out as it's
> a bug fix.  It should go to -stable as well for .37 right?

Yup, sounds like it should.

Dave.