diff mbox

[5/5] drm/i915: Kick out vga console

Message ID 20140630065955.GD7687@nuc-i3427.alporthouse.com (mailing list archive)
State New, archived
Headers show

Commit Message

Chris Wilson June 30, 2014, 6:59 a.m. UTC
On Sat, Jun 28, 2014 at 11:55:19PM -0400, Ed Tomlinson wrote:
> On Saturday 28 June 2014 15:28:22 Ed Tomlinson wrote:
> 
> Resend without html krud which causes list to bounce the message.
> 
> > Hi
> > 
> > This commit ( a4de05268e674e8ed31df6348269e22d6c6a1803 ) hangs my boot with 3.16-git.  Reverting it lets the boot proceed. 
> > 
> > I have an i7 with a built-in i915 and an pcie r7 260x.  The R7 is the primary console.  The i915 is initialized
> > but does not have a physical display attached.
> > 
> > With the patch applied the boot stops at the messages:
> > 
> > [drm] Memory usable by graphics device = 2048M
> > [drm] Replacing VGA console driver

The issue looks like that we are ripping out the radeon fb_con whilst it
is active and that upsets everyone. In which case, I think the
compromise is:


-Chris

Comments

David Herrmann June 30, 2014, 8:19 a.m. UTC | #1
Hi

On Mon, Jun 30, 2014 at 8:59 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Sat, Jun 28, 2014 at 11:55:19PM -0400, Ed Tomlinson wrote:
>> On Saturday 28 June 2014 15:28:22 Ed Tomlinson wrote:
>>
>> Resend without html krud which causes list to bounce the message.
>>
>> > Hi
>> >
>> > This commit ( a4de05268e674e8ed31df6348269e22d6c6a1803 ) hangs my boot with 3.16-git.  Reverting it lets the boot proceed.
>> >
>> > I have an i7 with a built-in i915 and an pcie r7 260x.  The R7 is the primary console.  The i915 is initialized
>> > but does not have a physical display attached.
>> >
>> > With the patch applied the boot stops at the messages:
>> >
>> > [drm] Memory usable by graphics device = 2048M
>> > [drm] Replacing VGA console driver
>
> The issue looks like that we are ripping out the radeon fb_con whilst it
> is active and that upsets everyone. In which case, I think the
> compromise is:

do_take_over_console() should only touch console-drivers like fbcon.
How does this affect the underlying fbdev device? Ripping out active
or inactive fbcon should be just fine, shouldn't it?

Given that this hard-locks at kick_out_vgacon(), this looks more like
a dead-lock to me.

Thanks
David
--
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
Ed Tomlinson July 1, 2014, 1:51 p.m. UTC | #2
Hi Chris,

I had to rediff to get a patch that applies...  I am not hanging with this applied - it
does look like the i915 is starting is initialization later boot the new kernel.

[    2.389796] [drm] Radeon Display Connectors
[    2.389798] [drm] Connector 0:
[    2.389799] [drm]   DP-1
[    2.389799] [drm]   HPD2
[    2.389801] [drm]   DDC: 0x6530 0x6530 0x6534 0x6534 0x6538 0x6538 0x653c 0x653c
[    2.389802] [drm]   Encoders:
[    2.389803] [drm]     DFP1: INTERNAL_UNIPHY2
[    2.389804] [drm] Connector 1:
[    2.389805] [drm]   HDMI-A-1
[    2.389805] [drm]   HPD3
[    2.389806] [drm]   DDC: 0x6550 0x6550 0x6554 0x6554 0x6558 0x6558 0x655c 0x655c
[    2.389807] [drm]   Encoders:
[    2.389808] [drm]     DFP2: INTERNAL_UNIPHY2
[    2.389809] [drm] Connector 2:
[    2.389810] [drm]   DVI-D-1
[    2.389811] [drm]   HPD1
[    2.389812] [drm]   DDC: 0x6560 0x6560 0x6564 0x6564 0x6568 0x6568 0x656c 0x656c
[    2.389813] [drm]   Encoders:
[    2.389814] [drm]     DFP3: INTERNAL_UNIPHY1
[    2.389815] [drm] Connector 3:
[    2.389815] [drm]   DVI-I-1
[    2.389816] [drm]   HPD6
[    2.389817] [drm]   DDC: 0x6580 0x6580 0x6584 0x6584 0x6588 0x6588 0x658c 0x658c
[    2.389818] [drm]   Encoders:
[    2.389819] [drm]     DFP4: INTERNAL_UNIPHY
[    2.389820] [drm]     CRT1: INTERNAL_KLDSCP_DAC1
[    2.435689] raid6: avx2x2   24564 MB/s
[    2.435691] Switched to clocksource tsc
[    2.489087] usb 1-8: new low-speed USB device number 7 using xhci_hcd
[    2.492403] raid6: avx2x4   34887 MB/s
[    2.492404] raid6: using algorithm avx2x4 (34887 MB/s)
[    2.492405] raid6: using avx2x2 recovery algorithm
[    2.492789] xor: automatically using best checksumming function:
[    2.502557] Adding 5779452k swap on /dev/sdb2.  Priority:-2 extents:1 across:5779452k FS
[    2.511532] [drm] fb mappable at 0xE098E000
[    2.511536] [drm] vram apper at 0xE0000000
[    2.511538] [drm] size 9216000
[    2.511539] [drm] fb depth is 24
[    2.511541] [drm]    pitch is 7680
[    2.511590] fbcon: radeondrmfb (fb0) is primary device
[    2.516691] nct6775: Found NCT6776D/F or compatible chip at 0x2e:0x290
[    2.525778]    avx       : 41474.400 MB/sec
[    2.532408] Console: switching to colour frame buffer device 240x75
[    2.535567] radeon 0000:01:00.0: fb0: radeondrmfb frame buffer device
[    2.535567] radeon 0000:01:00.0: registered panic notifier
[    2.544968] Btrfs loaded
[    2.545276] BTRFS: device fsid 9d4254aa-6715-4fa8-986a-1af0d51768ad devid 1 transid 308068 /dev/sdc1
[    2.545739] BTRFS: device fsid 9d4254aa-6715-4fa8-986a-1af0d51768ad devid 2 transid 308068 /dev/sdb1
[    2.552946] [drm] Initialized radeon 2.39.0 20080528 for 0000:01:00.0 on minor 0
[    2.553248] [drm] Memory usable by graphics device = 2048M
[    2.553273] [drm] Replacing VGA console driver
[    2.572539] i915 0000:00:02.0: irq 46 for MSI/MSI-X
[    2.572546] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
[    2.572565] [drm] Driver supports precise vblank timestamp query.

If you are happy with this you can give this patch my tested by.

Thanks
Ed Tomlinson

On Monday 30 June 2014 07:59:55 Chris Wilson wrote:
> On Sat, Jun 28, 2014 at 11:55:19PM -0400, Ed Tomlinson wrote:
> > On Saturday 28 June 2014 15:28:22 Ed Tomlinson wrote:
> > 
> > Resend without html krud which causes list to bounce the message.
> > 
> > > Hi
> > > 
> > > This commit ( a4de05268e674e8ed31df6348269e22d6c6a1803 ) hangs my boot with 3.16-git.  Reverting it lets the boot proceed. 
> > > 
> > > I have an i7 with a built-in i915 and an pcie r7 260x.  The R7 is the primary console.  The i915 is initialized
> > > but does not have a physical display attached.
> > > 
> > > With the patch applied the boot stops at the messages:
> > > 
> > > [drm] Memory usable by graphics device = 2048M
> > > [drm] Replacing VGA console driver
> 
> The issue looks like that we are ripping out the radeon fb_con whilst it
> is active and that upsets everyone. In which case, I think the
> compromise is:
> 
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 5f44581..4915f1d 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1439,18 +1439,20 @@ static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
>  #else
>  static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
>  {
> -       int ret;
> +       int ret = 0;
>  
>         DRM_INFO("Replacing VGA console driver\n");
>  
>         console_lock();
> -       ret = do_take_over_console(&dummy_con, 0, MAX_NR_CONSOLES - 1, 1);
> -       if (ret == 0) {
> -               ret = do_unregister_con_driver(&vga_con);
> -
> -               /* Ignore "already unregistered". */
> -               if (ret == -ENODEV)
> -                       ret = 0;
> +       if (con_is_bound(&vga_con)) {
> +               ret = do_take_over_console(&dummy_con, 0, MAX_NR_CONSOLES - 1, 1);
> +               if (ret == 0) {
> +                       ret = do_unregister_con_driver(&vga_con);
> +
> +                       /* Ignore "already unregistered". */
> +                       if (ret == -ENODEV)
> +                               ret = 0;
> +               }
>         }
>         console_unlock();
> 
> -Chris
> 
> 

--
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
Daniel Vetter July 7, 2014, 8:48 a.m. UTC | #3
On Mon, Jun 30, 2014 at 07:59:55AM +0100, Chris Wilson wrote:
> On Sat, Jun 28, 2014 at 11:55:19PM -0400, Ed Tomlinson wrote:
> > On Saturday 28 June 2014 15:28:22 Ed Tomlinson wrote:
> > 
> > Resend without html krud which causes list to bounce the message.
> > 
> > > Hi
> > > 
> > > This commit ( a4de05268e674e8ed31df6348269e22d6c6a1803 ) hangs my boot with 3.16-git.  Reverting it lets the boot proceed. 
> > > 
> > > I have an i7 with a built-in i915 and an pcie r7 260x.  The R7 is the primary console.  The i915 is initialized
> > > but does not have a physical display attached.
> > > 
> > > With the patch applied the boot stops at the messages:
> > > 
> > > [drm] Memory usable by graphics device = 2048M
> > > [drm] Replacing VGA console driver
> 
> The issue looks like that we are ripping out the radeon fb_con whilst it
> is active and that upsets everyone. In which case, I think the
> compromise is:
> 
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 5f44581..4915f1d 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1439,18 +1439,20 @@ static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
>  #else
>  static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
>  {
> -       int ret;
> +       int ret = 0;
>  
>         DRM_INFO("Replacing VGA console driver\n");
>  
>         console_lock();
> -       ret = do_take_over_console(&dummy_con, 0, MAX_NR_CONSOLES - 1, 1);
> -       if (ret == 0) {
> -               ret = do_unregister_con_driver(&vga_con);
> -
> -               /* Ignore "already unregistered". */
> -               if (ret == -ENODEV)
> -                       ret = 0;
> +       if (con_is_bound(&vga_con)) {
> +               ret = do_take_over_console(&dummy_con, 0, MAX_NR_CONSOLES - 1, 1);
> +               if (ret == 0) {
> +                       ret = do_unregister_con_driver(&vga_con);

Hm, we should only conditionalize the take_over_console - unregistering
vga_con is kinda the point to make sure it's gone for real. Ed, can you
please retest with the if (con_is_bound) check just for the
do_take_over_console call?

Still puzzled wtf is going on here since as David says this should be a
no-op.

Thanks, Daniel
> +
> +                       /* Ignore "already unregistered". */
> +                       if (ret == -ENODEV)
> +                               ret = 0;
> +               }
>         }
>         console_unlock();
> 
> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Ed Tomlinson July 7, 2014, 10:45 a.m. UTC | #4
Daniel,

I am not quite sure I understand what you want me to test?
Do you want me to try it without:

> > +               if (ret == 0) {
> > +                       ret = do_unregister_con_driver(&vga_con);

Thanks
Ed


On Monday 07 July 2014 10:48:26 Daniel Vetter wrote:
> On Mon, Jun 30, 2014 at 07:59:55AM +0100, Chris Wilson wrote:
> > On Sat, Jun 28, 2014 at 11:55:19PM -0400, Ed Tomlinson wrote:
> > > On Saturday 28 June 2014 15:28:22 Ed Tomlinson wrote:
> > > 
> > > Resend without html krud which causes list to bounce the message.
> > > 
> > > > Hi
> > > > 
> > > > This commit ( a4de05268e674e8ed31df6348269e22d6c6a1803 ) hangs my boot with 3.16-git.  Reverting it lets the boot proceed. 
> > > > 
> > > > I have an i7 with a built-in i915 and an pcie r7 260x.  The R7 is the primary console.  The i915 is initialized
> > > > but does not have a physical display attached.
> > > > 
> > > > With the patch applied the boot stops at the messages:
> > > > 
> > > > [drm] Memory usable by graphics device = 2048M
> > > > [drm] Replacing VGA console driver
> > 
> > The issue looks like that we are ripping out the radeon fb_con whilst it
> > is active and that upsets everyone. In which case, I think the
> > compromise is:
> > 
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> > index 5f44581..4915f1d 100644
> > --- a/drivers/gpu/drm/i915/i915_dma.c
> > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > @@ -1439,18 +1439,20 @@ static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
> >  #else
> >  static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
> >  {
> > -       int ret;
> > +       int ret = 0;
> >  
> >         DRM_INFO("Replacing VGA console driver\n");
> >  
> >         console_lock();
> > -       ret = do_take_over_console(&dummy_con, 0, MAX_NR_CONSOLES - 1, 1);
> > -       if (ret == 0) {
> > -               ret = do_unregister_con_driver(&vga_con);
> > -
> > -               /* Ignore "already unregistered". */
> > -               if (ret == -ENODEV)
> > -                       ret = 0;
> > +       if (con_is_bound(&vga_con)) {
> > +               ret = do_take_over_console(&dummy_con, 0, MAX_NR_CONSOLES - 1, 1);
> > +               if (ret == 0) {
> > +                       ret = do_unregister_con_driver(&vga_con);
> 
> Hm, we should only conditionalize the take_over_console - unregistering
> vga_con is kinda the point to make sure it's gone for real. Ed, can you
> please retest with the if (con_is_bound) check just for the
> do_take_over_console call?
> 
> Still puzzled wtf is going on here since as David says this should be a
> no-op.
> 
> Thanks, Daniel
> > +
> > +                       /* Ignore "already unregistered". */
> > +                       if (ret == -ENODEV)
> > +                               ret = 0;
> > +               }
> >         }
> >         console_unlock();
> > 
> > -Chris
> > 
> 
> 

--
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
Ed Tomlinson July 7, 2014, 10:59 a.m. UTC | #5
Daniel,

Just to be sure.  The intel card here should not be claiming the real console.  It does
not have an output device and the bios set set so the radeon is the primary device.

Ed


On Monday 07 July 2014 10:48:26 Daniel Vetter wrote:
> On Mon, Jun 30, 2014 at 07:59:55AM +0100, Chris Wilson wrote:
> > On Sat, Jun 28, 2014 at 11:55:19PM -0400, Ed Tomlinson wrote:
> > > On Saturday 28 June 2014 15:28:22 Ed Tomlinson wrote:
> > > 
> > > Resend without html krud which causes list to bounce the message.
> > > 
> > > > Hi
> > > > 
> > > > This commit ( a4de05268e674e8ed31df6348269e22d6c6a1803 ) hangs my boot with 3.16-git.  Reverting it lets the boot proceed. 
> > > > 
> > > > I have an i7 with a built-in i915 and an pcie r7 260x.  The R7 is the primary console.  The i915 is initialized
> > > > but does not have a physical display attached.
> > > > 
> > > > With the patch applied the boot stops at the messages:
> > > > 
> > > > [drm] Memory usable by graphics device = 2048M
> > > > [drm] Replacing VGA console driver
> > 
> > The issue looks like that we are ripping out the radeon fb_con whilst it
> > is active and that upsets everyone. In which case, I think the
> > compromise is:
> > 
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> > index 5f44581..4915f1d 100644
> > --- a/drivers/gpu/drm/i915/i915_dma.c
> > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > @@ -1439,18 +1439,20 @@ static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
> >  #else
> >  static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
> >  {
> > -       int ret;
> > +       int ret = 0;
> >  
> >         DRM_INFO("Replacing VGA console driver\n");
> >  
> >         console_lock();
> > -       ret = do_take_over_console(&dummy_con, 0, MAX_NR_CONSOLES - 1, 1);
> > -       if (ret == 0) {
> > -               ret = do_unregister_con_driver(&vga_con);
> > -
> > -               /* Ignore "already unregistered". */
> > -               if (ret == -ENODEV)
> > -                       ret = 0;
> > +       if (con_is_bound(&vga_con)) {
> > +               ret = do_take_over_console(&dummy_con, 0, MAX_NR_CONSOLES - 1, 1);
> > +               if (ret == 0) {
> > +                       ret = do_unregister_con_driver(&vga_con);
> 
> Hm, we should only conditionalize the take_over_console - unregistering
> vga_con is kinda the point to make sure it's gone for real. Ed, can you
> please retest with the if (con_is_bound) check just for the
> do_take_over_console call?
> 
> Still puzzled wtf is going on here since as David says this should be a
> no-op.
> 
> Thanks, Daniel
> > +
> > +                       /* Ignore "already unregistered". */
> > +                       if (ret == -ENODEV)
> > +                               ret = 0;
> > +               }
> >         }
> >         console_unlock();
> > 
> > -Chris
> > 
> 
> 

--
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
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 5f44581..4915f1d 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1439,18 +1439,20 @@  static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
 #else
 static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
 {
-       int ret;
+       int ret = 0;
 
        DRM_INFO("Replacing VGA console driver\n");
 
        console_lock();
-       ret = do_take_over_console(&dummy_con, 0, MAX_NR_CONSOLES - 1, 1);
-       if (ret == 0) {
-               ret = do_unregister_con_driver(&vga_con);
-
-               /* Ignore "already unregistered". */
-               if (ret == -ENODEV)
-                       ret = 0;
+       if (con_is_bound(&vga_con)) {
+               ret = do_take_over_console(&dummy_con, 0, MAX_NR_CONSOLES - 1, 1);
+               if (ret == 0) {
+                       ret = do_unregister_con_driver(&vga_con);
+
+                       /* Ignore "already unregistered". */
+                       if (ret == -ENODEV)
+                               ret = 0;
+               }
        }
        console_unlock();