Re: [PATCH] drm/vmwgfx: respect 'nomodeset'

[Thomas Hellstrom <thellstrom@vmware.com>]

On 10/15/2014 10:38 PM, Rob Clark wrote:
> On Wed, Oct 15, 2014 at 4:35 PM, Rob Clark <robdclark@gmail.com> wrote:
>> On Wed, Oct 15, 2014 at 4:17 PM, Thomas Hellstrom <thellstrom@vmware.com> wrote:
>>> On 10/15/2014 09:46 PM, Rob Clark wrote:
>>>> On Wed, Oct 15, 2014 at 3:24 PM, Thomas Hellstrom <thellstrom@vmware.com> wrote:
>>>>> On 10/15/2014 09:00 PM, Rob Clark wrote:
>>>>>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>>>>>> ---
>>>>>>  drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 7 +++++++
>>>>>>  1 file changed, 7 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
>>>>>> index 18b54ac..f0267b8 100644
>>>>>> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
>>>>>> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
>>>>>> @@ -25,6 +25,7 @@
>>>>>>   *
>>>>>>   **************************************************************************/
>>>>>>  #include <linux/module.h>
>>>>>> +#include <linux/console.h>
>>>>>>
>>>>>>  #include <drm/drmP.h>
>>>>>>  #include "vmwgfx_drv.h"
>>>>>> @@ -1453,6 +1454,12 @@ static int vmw_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>>>>>>  static int __init vmwgfx_init(void)
>>>>>>  {
>>>>>>       int ret;
>>>>>> +
>>>>>> +#ifdef CONFIG_VGA_CONSOLE
>>>>>> +     if (vgacon_text_force())
>>>>>> +             return -EINVAL;
>>>>>> +#endif
>>>>>> +
>>>>> Hmm,
>>>>>
>>>>> From the function name vgacon_text_force() it sounds like this should
>>>>> just stop the driver from initializing fbcon? Not refuse to load?
>>>> yeah, the function is badly named.. it perhaps should be
>>>> vgacon_is_text_forced() or something like that.  But basically it
>>>> returns whether we are forced to text mode.
>>>>
>>>> BR,
>>>> -R
>>> So then I guess it would be more correct to use the output of that
>>> function when determining the value of dev_priv->enable_fb (vmwgfx can
>>> enable the user-space modesetting API without turning on vmwgfx fbcon).
>> well, other drivers, 'nomodeset' forces the driver not to load (to
>> work around buggyness, etc)..  I suppose for vmwgfx perhaps oddball
>> "hardware" is less of a concern.  Although it seems like it would be
>> nice if vmwgfx behaved consistently with the other drivers.
>>
>> Most/all for the drivers have an additional module param that lets you
>> override this and load the driver for UMS in case of 'nomodeset'..
>> which would give you the behaviour you describe.  But I think in the
>> absence of an additional module param specified, the default
>> 'nomodeset' behaviour should be that the driver does not load.
>>
>> But I can add such a module param if you think it is useful..
> oh, wait.. you already have an 'enable_fbdev'..  although I guess that
> is actually meaning "enable_kms"?

It should actually have been "enable_fbcon_and_fbdev" instead of
"enable_fbdev". KMS is always enabled with the vmwgfx driver, we have no
UMS driver using DRM functionality.

I think what confuses me is how "nomodeset" translates to
"use_text_console", (sounds orthogonal to me) but if that's the way that
parameter is presented to the drivers, I guess the correct way after all
is to stop the driver from loading....


/Thomas