On 14 December 2017 11:55:35 p.m. GMT+01:00, Ben Skeggs <skeg...@gmail.com> wrote: >On 14 December 2017 at 23:45, Tobias Klausmann ><tobias.johannes.klausm...@mni.thm.de> wrote: >> >> On 12/3/17 8:56 PM, Josef Larsson wrote: >>> >>> Sure, I can easily split it into two commits, but I would like to >have >>> an OK on the actual code changes before splitting the patch. >I'm not actually sure this is a good idea still. As I recall, part of >the purpose of that check is to prevent nouveau taking over as the >primary fbcon when it's the secondary display in the system. ie. >Optimus system with Intel driving the display, NVIDIA GPU has no >displays attached. If nouveau loads first, at some point in history, >you'd have ended up with a blank console. > >I'm not sure if this is still the case, but it warrants checking >before making this change. > >I'm more interested in what this actually solves, and why not having >fbcon prevents external displays from being used. > >Ben. >
I think the issue is we can't really put any trust in this. my kepler GPU on my laptop is a VGA device, not a 3D accelerator, even though everything is wird to intel, it is a MXM card though, so in theory it is able to drive displays. We need something else to detect if the GPU is the main one or not. >>> >>> Best regards, >>> >>> Josef Larsson >>> >>> >>> On 2017-11-11 01:05, Tobias Klausmann wrote: >>>> >>>> On 11/10/17 7:49 PM, Josef Larsson wrote: >>>>> >>>>> Accept 3d controllers and not only VGA controllers. According to >Ilia >>>>> Mirkin, >>>>> the VGA controller check should be removed. This makes it possible >>>>> to use external connectors on a docking station (40A5) for a >Thinkpad >>>>> P51. >>>>> (See Bug 101778). >>>>> >>>>> lspci example: >>>>> >>>>> 01:00.0 3D controller: NVIDIA Corporation GM107GLM [Quadro M1200 >Mobile] >>>>> (rev a2) >>>>> >>>>> Also include safe-guards to avoid NULL dereferencing of fbcon, >which is >>>>> how this bug was found. >>>>> --- >>>>> drivers/gpu/drm/nouveau/nouveau_fbcon.c | 3 +-- >>>>> drivers/gpu/drm/nouveau/nv50_display.c | 13 +++++++++++++ >>>>> 2 files changed, 14 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c >>>>> b/drivers/gpu/drm/nouveau/nouveau_fbcon.c >>>>> index 2b12d82aac15..6b4d374a9d82 100644 >>>>> --- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c >>>>> +++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c >>>>> @@ -498,8 +498,7 @@ nouveau_fbcon_init(struct drm_device *dev) >>>>> int preferred_bpp; >>>>> int ret; >>>>> - if (!dev->mode_config.num_crtc || >>>>> - (dev->pdev->class >> 8) != PCI_CLASS_DISPLAY_VGA) >>>>> + if (!dev->mode_config.num_crtc) >>>>> return 0; >>>>> fbcon = kzalloc(sizeof(struct nouveau_fbdev), >GFP_KERNEL); >>>>> diff --git a/drivers/gpu/drm/nouveau/nv50_display.c >>>>> b/drivers/gpu/drm/nouveau/nv50_display.c >>>>> index fb47d46050ec..061daf036407 100644 >>>>> --- a/drivers/gpu/drm/nouveau/nv50_display.c >>>>> +++ b/drivers/gpu/drm/nouveau/nv50_display.c >>>>> @@ -3214,6 +3214,13 @@ nv50_mstm_destroy_connector(struct >>>>> drm_dp_mst_topology_mgr *mgr, >>>>> struct nouveau_drm *drm = nouveau_drm(connector->dev); >>>>> struct nv50_mstc *mstc = nv50_mstc(connector); >>>>> + if (!drm->fbcon) >>>>> + { >>>>> + NV_WARN(drm, "drm->fbcon of %s point to NULL. Will not >destroy >>>>> connector\n", >>>>> + connector->name); >>>>> + return; >>>>> + } >>>>> + >>>>> drm_connector_unregister(&mstc->connector); >>>>> drm_modeset_lock_all(drm->dev); >>>>> @@ -3229,6 +3236,12 @@ nv50_mstm_register_connector(struct >drm_connector >>>>> *connector) >>>>> { >>>>> struct nouveau_drm *drm = nouveau_drm(connector->dev); >>>>> + if (!drm->fbcon) >>>>> + { >>>>> + NV_WARN(drm, "drm->fbcon of %s point to NULL. Will not >register >>>>> connector\n", >>>>> + connector->name); >>>>> + return; >>>>> + } >>>>> drm_modeset_lock_all(drm->dev); >>>>> drm_fb_helper_add_one_connector(&drm->fbcon->helper, >connector); >>>>> drm_modeset_unlock_all(drm->dev); >>>>> >>>>> >>>> Hi, >>>> >>>> the patch looks OK to me, yet as noted in IRC, i'd like to have >this >>>> patch split into two and have the ->fbcon check as a precondition >to >>>> the 3D Controller part. But lets see what the other and more clever >>>> people think about it! :) >>>> >>>> Greetings, >>>> >>>> Tobias >>>> >>> >> >> Ping, >> >> adding Ben Skeggs and Dave Airlied to CC, maybe this will get this >little >> one commited! >> >> >> Greetings, >> >> Tobias >> >> >> _______________________________________________ >> Nouveau mailing list >> Nouveau@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/nouveau >_______________________________________________ >Nouveau mailing list >Nouveau@lists.freedesktop.org >https://lists.freedesktop.org/mailman/listinfo/nouveau _______________________________________________ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau