[PATCH] Accept 3d controllers and not only VGA controllers.

[PATCH] Accept 3d controllers and not only VGA controllers.

[Josef Larsson]

[-- Attachment #1.1.1: Type: text/plain, Size: 2448 bytes --]

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);
-- 
2.13.6


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 862 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH] Accept 3d controllers and not only VGA controllers.

[Tobias Klausmann]

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

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH] Accept 3d controllers and not only VGA controllers.

[Josef Larsson]

[-- Attachment #1.1.1: Type: text/plain, Size: 3292 bytes --]

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.

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
>



[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 862 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH] Accept 3d controllers and not only VGA controllers.

[Tobias Klausmann]

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

Re: [PATCH] Accept 3d controllers and not only VGA controllers.

[Ben Skeggs]

On 14 December 2017 at 23:45, Tobias Klausmann
<tobias.johannes.klausmann@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.

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

Re: [PATCH] Accept 3d controllers and not only VGA controllers.

[Karol Herbst]

On 14 December 2017 11:55:35 p.m. GMT+01:00, Ben Skeggs <skeggsb@gmail.com> wrote:
>On 14 December 2017 at 23:45, Tobias Klausmann
><tobias.johannes.klausmann@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

Re: [PATCH] Accept 3d controllers and not only VGA controllers.

[Josef Larsson]

[-- Attachment #1.1.1: Type: text/plain, Size: 6580 bytes --]

Without a NULL pointer safe-guard patch, I get a kernel oops when I plug
in external displays in my docking station, (exactly the same issue as
https://bugs.freedesktop.org/show_bug.cgi?id=101778) and without
removing or modifying the check (accepting PCI_CLASS_DISPLAY_3D in the
if-condition), I cannot use external displays through my docking
station. This is on an optimus system where I use reverse PRIME to be
able to use the connectors at all. Having some kind of safe-guard makes
sense to me, and that is available in the i915-driver of the
corresponding functions.

If there are better patches to fix my two problems, I am willing to try
them out, because I really think this should be handled somehow.

One note though: These patches do not make the docking station
experience perfect. They only make it not quite as bad. For example,
undocking when using external displays forces Xorg to restart, and when
docking without a patch for xf86-video-nouveau DDX by Ilia Mirkin
(https://people.freedesktop.org/~imirkin/patches/0001-drmmode-update-logic-for-dynamic-connectors-paths-an.patch),
the external docking station displays are not detected.


On 2017-12-14 23:59, Karol Herbst wrote:
>
> On 14 December 2017 11:55:35 p.m. GMT+01:00, Ben Skeggs <skeggsb@gmail.com> wrote:
>> On 14 December 2017 at 23:45, Tobias Klausmann
>> <tobias.johannes.klausmann-AqjdNwhu20eELgA04lAiVw@public.gmane.org> 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-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
>>> https://lists.freedesktop.org/mailman/listinfo/nouveau
>> _______________________________________________
>> Nouveau mailing list
>> Nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
>> https://lists.freedesktop.org/mailman/listinfo/nouveau



[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 862 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH] Accept 3d controllers and not only VGA controllers.

[Karol Herbst]

I've discussed it with Ben and we actually found a better solution.
There are just some calls inside those functions which should get NULL
checks, nv50_mstm_register_connector and nv50_mstm_destroy_connector.
Or at least something similiar so that the code doesn't depent on the
fbcon object being there.

On Mon, Dec 18, 2017 at 6:30 PM, Josef Larsson <josef.lar@gmail.com> wrote:
> Without a NULL pointer safe-guard patch, I get a kernel oops when I plug
> in external displays in my docking station, (exactly the same issue as
> https://bugs.freedesktop.org/show_bug.cgi?id=101778) and without
> removing or modifying the check (accepting PCI_CLASS_DISPLAY_3D in the
> if-condition), I cannot use external displays through my docking
> station. This is on an optimus system where I use reverse PRIME to be
> able to use the connectors at all. Having some kind of safe-guard makes
> sense to me, and that is available in the i915-driver of the
> corresponding functions.
>
> If there are better patches to fix my two problems, I am willing to try
> them out, because I really think this should be handled somehow.
>
> One note though: These patches do not make the docking station
> experience perfect. They only make it not quite as bad. For example,
> undocking when using external displays forces Xorg to restart, and when
> docking without a patch for xf86-video-nouveau DDX by Ilia Mirkin
> (https://people.freedesktop.org/~imirkin/patches/0001-drmmode-update-logic-for-dynamic-connectors-paths-an.patch),
> the external docking station displays are not detected.
>
>
> On 2017-12-14 23:59, Karol Herbst wrote:
>>
>> On 14 December 2017 11:55:35 p.m. GMT+01:00, Ben Skeggs <skeggsb@gmail.com> wrote:
>>> On 14 December 2017 at 23:45, Tobias Klausmann
>>> <tobias.johannes.klausmann@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
>
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH] Accept 3d controllers and not only VGA controllers.

[Josef Larsson]

[-- Attachment #1.1.1: Type: text/plain, Size: 7704 bytes --]

Sounds good to me, if we remove or modify the PCI_CLASS check in
nouveau_fbcon.c as well :).

On 2017-12-18 18:36, Karol Herbst wrote:
> I've discussed it with Ben and we actually found a better solution.
> There are just some calls inside those functions which should get NULL
> checks, nv50_mstm_register_connector and nv50_mstm_destroy_connector.
> Or at least something similiar so that the code doesn't depent on the
> fbcon object being there.
>
> On Mon, Dec 18, 2017 at 6:30 PM, Josef Larsson <josef.lar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> Without a NULL pointer safe-guard patch, I get a kernel oops when I plug
>> in external displays in my docking station, (exactly the same issue as
>> https://bugs.freedesktop.org/show_bug.cgi?id=101778) and without
>> removing or modifying the check (accepting PCI_CLASS_DISPLAY_3D in the
>> if-condition), I cannot use external displays through my docking
>> station. This is on an optimus system where I use reverse PRIME to be
>> able to use the connectors at all. Having some kind of safe-guard makes
>> sense to me, and that is available in the i915-driver of the
>> corresponding functions.
>>
>> If there are better patches to fix my two problems, I am willing to try
>> them out, because I really think this should be handled somehow.
>>
>> One note though: These patches do not make the docking station
>> experience perfect. They only make it not quite as bad. For example,
>> undocking when using external displays forces Xorg to restart, and when
>> docking without a patch for xf86-video-nouveau DDX by Ilia Mirkin
>> (https://people.freedesktop.org/~imirkin/patches/0001-drmmode-update-logic-for-dynamic-connectors-paths-an.patch),
>> the external docking station displays are not detected.
>>
>>
>> On 2017-12-14 23:59, Karol Herbst wrote:
>>> On 14 December 2017 11:55:35 p.m. GMT+01:00, Ben Skeggs <skeggsb@gmail.com> wrote:
>>>> On 14 December 2017 at 23:45, Tobias Klausmann
>>>> <tobias.johannes.klausmann-AqjdNwhu20eELgA04lAiVw@public.gmane.org> 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-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
>>>>> https://lists.freedesktop.org/mailman/listinfo/nouveau
>>>> _______________________________________________
>>>> Nouveau mailing list
>>>> Nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
>>>> https://lists.freedesktop.org/mailman/listinfo/nouveau
>>
>>
>> _______________________________________________
>> Nouveau mailing list
>> Nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
>> https://lists.freedesktop.org/mailman/listinfo/nouveau
>>



[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 862 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau