Created attachment 74945 [details] [review]
Fix

To be honest I am a little confused why your system does not load nouveau at an earlier stage, rather than relying on X. Either case this is a valid point which I did not consider while writing the original patch

On 18/02/13 00:47, Dave Airlie wrote:> On Sun, Feb 17, 2013 at 6:48 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote:
>> "Regression" caused by
>> commit e34cfbd5bd23f7f15372af52d8a39a5715ce7310
>> Author: Emil Velikov <emil.l.velikov@gmail.com>
>> Date:   Fri Nov 2 03:57:41 2012 +0000
>>
>>     nouveau: Factor out common code to NVHasKMS()
>>
>>     As the name suggests checks if it has kernel mode setting,
>>     prints out the interface version and checkes if the chipset
>>     is supported
>>
>>     Function is used in NVPciProbe and NVPlatformProbe
>>
>> Without this change X will fail with '[drm] KMS not enabled' if
>> the kernel module is not loaded
> 
> Thats correct, the X server is not responsible for loading the kernel module.
> 
> If the kernel module isn't loaded then there is a bug elsewhere.
> 
> Dave.
>

Probably due to the fact that lacking boot splash image, there is nothing which would need the nouveau before :-).

(I have standard VGA text screen before loading X and I don't see any reason to change that - it may prove usefull in case of some problems and without problems I'm starting X immediately anyway.)

Thanks for fix. I'll test it after reboot.

This case reminds me XKCD's "Workflow" [1]

Please note that KMS is not only about "shiny boot splash". Here is a snippet from Arch's Wiki [2]

"...
Kernel Mode Setting (KMS) is a method for setting display resolution and depth in the kernel space rather than user space.
...
even kernel space power-saving.
..."

Not to mention that there were(are) a few cases where the handover (vga->nouveau) was causing issues. With that said I believe that udev(or similar in your case/distro) should kick in and load nouveau before X

Personally I do not feel to strong either way, thus it may boil down to convincing the maintainer that "nouveau kernel module should not be loaded before X" :) . In which case this patch will be committed

[1] http://xkcd.com/1172/
[2] https://wiki.archlinux.org/index.php/Kernel_Mode_Setting

Blacklisting nouveau module can lead to exactly this situation - nouveau is not automatically loaded on boot, but can be loaded on request by xserver (if configured to use nouveau).

Should I post this bug on http://www.explainxkcd.com/wiki/index.php?title=1172 :-) ?

I repeat: there is NO mode switch in my workflow before starting X, therefore no need for KMS. Power saving is also not needed, as I'm starting the X in five minutes, usually faster.

Also, remembered another advantage of this setup. I could decide to start the proprietary nvidia driver instead of nouveau. (If it's still possible to compile it against current kernel, I must admit I didn't checked lately.)

I don't have nouveau blacklisted, that's easy enough to check :-). In fact, I don't have it mentioned in module configuration at all, meaning kernel shows great amount of intelligence by finding it. I think following lines from /var/log/hotplug relates:

Mon Feb 18 09:59:22 CET 2013: module add /module/cfbfillrect
Mon Feb 18 09:59:22 CET 2013: module add /module/cfbimgblt
Mon Feb 18 09:59:22 CET 2013: module add /module/video
Mon Feb 18 09:59:22 CET 2013: drivers add /bus/acpi/drivers/video
Mon Feb 18 09:59:22 CET 2013: module add /module/wmi
Mon Feb 18 09:59:22 CET 2013: drivers add /bus/acpi/drivers/wmi
Mon Feb 18 09:59:22 CET 2013: class add /class/wmi
Mon Feb 18 09:59:22 CET 2013: module add /module/mxm_wmi
Mon Feb 18 09:59:22 CET 2013: module add /module/cfbcopyarea
Mon Feb 18 09:59:22 CET 2013: module add /module/drm_kms_helper
Mon Feb 18 09:59:22 CET 2013: module add /module/ttm
Mon Feb 18 09:59:22 CET 2013: drm add /devices/virtual/drm/ttm
Mon Feb 18 09:59:23 CET 2013: module add /module/nouveau

Also, apparently I'm using http://linux-hotplug.sourceforge.net ... not sure about udev. I have it installed, but no daemon is running. Still, before you changed the xf86-video-nouveau, I had no problems.

Thanks for the information guys. All of those are valid cases which I have not thought of (and to be honest this is the first time I've heard about linux-hotplug)

The patch has been posted on the mailing list, and I hope it will be pushed in due course. With a slightly more sensible commit message

having the X server load the module isn't supported, I don't care if it ever worked before. Its racy and horrible. The way Linux works if you have a driver the kernel is responsible for loading it, the X server isn't.

Add nouveau to /etc/modules.conf

I can confirm the patch works (and unlike how I switched the calls myself probably cleans up after itself correctly :-)).

Dave Airlie: Isn't modules.conf obsolete?
Also, it's still kernel loading the driver, it's not like X calling modprobe, isn't it? And if there is race, dealing with it by hoping that there will be few seconds between the modprobe and starting X just by chance isn't exactly correct either.

The usual mechanism, nowadays, for modules to load is that the kernel creates some stuff in /sys, udev is notified about it, looks at the module_alias which describe what it is that the modules claim they support, and loads the right ones. In this case the nouveau module should (and does) claim to support some set of pci devices. (I might have that a little wrong, but that's the overall gist.) If you're not using udev/have disabled that somehow, I don't think it's too much to ask you to load the module yourself.

In any case, I don't think having this in the bugtracker will lead to the resolution that you're seeking. If you really want to change the behaviour, send a patch, ping people, etc. It sounds like the various people are in favor of the current behaviour (I know I am -- I hate it when things do stuff behind my back). For anyone looking at this later, see discussion that followed http://lists.freedesktop.org/archives/nouveau/2013-February/012222.html .

If you want to boot s.t. you can load the nvidia driver, you can either boot with nouveau.modeset=0, or take a look at http://nouveau.freedesktop.org/wiki/KernelModeSetting/ for how to unload a "mode-setted" module.

Mirkin: You might've overlooked it, but there IS patch in the message you posted reference to. I don't see what other patch might be needed.

Also, the behaviour I'm seeking is behaviour previous versions (like xf86-video-nouveau-0.0.16) had. Versions which already supported KMS.

I fail to see why you think it's necessary to change behaviour of xf86-video-nouveau to support less workflows just to force the workflow you prefer. All replies against this patch I saw was on ideological ground, none presented any real reason why it's better this way.

But ok. If I'm only one needing this workflow, I can accept the need to patch xf86-video-nouveau myself. Still better than reworking whole setup to workaround your decision.

(Note: Problem with modprobing nouveau manually is that I would need to login as root or have other setuid binary for that ... hmmm, ok, maybe sudo would suffice.)

PS: Oh, and isn't "I hate it when things do stuff behind my back" argument AGAINST udev?

To put this in another light

(In reply to comment #11)
> Mirkin: You might've overlooked it, but there IS patch in the message you
> posted reference to. I don't see what other patch might be needed.
> 
No-one is saying there is a need for (other) patch(es)

> Also, the behaviour I'm seeking is behaviour previous versions (like
> xf86-video-nouveau-0.0.16) had. Versions which already supported KMS.
> 
Yes we made a mistake and forgot to do this when we dropped UMS. Sorry for that, we are not perfect :\

> I fail to see why you think it's necessary to change behaviour of
> xf86-video-nouveau to support less workflows just to force the workflow you
> prefer. All replies against this patch I saw was on ideological ground, none
> presented any real reason why it's better this way.
> 
Afaics thing will work fine if you use udev/eudev/mdev over linux-hotplug. In the latter of which I cannot see any progress (or changes) over the last few years.

Whereas for the ideological ground - I think that the maintainer of the kernel drm subsystem(Dave) would know better. Yes a bit more information would not hurt, but considering the other drivers are doing this I do not see it as a wrong thing/bikeshedding.

Btw, I believe he already mentioned that it's racy :)

> But ok. If I'm only one needing this workflow, I can accept the need to
> patch xf86-video-nouveau myself. Still better than reworking whole setup to
> workaround your decision.
> 
Whichever works for you :)

(In reply to comment #13)
> (In reply to comment #11)
> > Mirkin: You might've overlooked it, but there IS patch in the message you
> > posted reference to. I don't see what other patch might be needed.
> > 
> No-one is saying there is a need for (other) patch(es)
> 

"If you really want to change the behaviour, send a patch, ping people, etc."

> > I fail to see why you think it's necessary to change behaviour of
> > xf86-video-nouveau to support less workflows just to force the workflow you
> > prefer. All replies against this patch I saw was on ideological ground, none
> > presented any real reason why it's better this way.
> > 
> Afaics thing will work fine if you use udev/eudev/mdev over linux-hotplug.
> In the latter of which I cannot see any progress (or changes) over the last
> few years.
> 

I successfully ignored devfs. Fads like that come and go. No need to change what's working. Current attempts of integrating pulseaudio and init doesn't convince me udev is stable. Sure, sysvinit didn't have much change in last few years ... because it's working. Systemd, on the other hand ...

> Whereas for the ideological ground - I think that the maintainer of the
> kernel drm subsystem(Dave) would know better. Yes a bit more information
> would not hurt, but considering the other drivers are doing this I do not
> see it as a wrong thing/bikeshedding.
> 
> Btw, I believe he already mentioned that it's racy :)
> 

He mentioned that there is race when you start the module and X server immediately starts using it. If you start udev and immediately after it start X, the race will still be there, wouldn't it? In fact, it will be even worse, as udev doesn't exactly have predictable timing.

With distributions trying to make startup fast and parallel and often starting display managers-based logins, I'm actually surprised it works.

If the code would be supposed to solve the race, it would contain cycle sleeping until some ioctl returns the module finished initialization. Probably with timeout around 30 seconds.