thr3ads.net - Nouveau - [Nouveau] [PATCH] Accept 3d controllers and not only VGA controllers. [Nov 2017]

If this information is useful, please help other people find it:
Share via:

Josef Larsson

2017-Nov-10 18:49 UTC

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

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 862 bytes
Desc: OpenPGP digital signature
URL:
<https://lists.freedesktop.org/archives/nouveau/attachments/20171110/dfd0ec9e/attachment.sig>

Tobias Klausmann

2017-Nov-11 00:05 UTC

head link

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

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

Josef Larsson

2017-Dec-03 19:56 UTC

head link

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

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
>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 862 bytes
Desc: OpenPGP digital signature
URL:
<https://lists.freedesktop.org/archives/nouveau/attachments/20171203/781df0d5/attachment.sig>

Possibly Parallel Threads

Search for more possibly parallel threads

Nouveau - Nov 2017 - [PATCH] Accept 3d controllers and not only VGA controllers.

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

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

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

Possibly Parallel Threads