Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 689588 - >x11-drivers/nvidia-drivers-430 should install nvidia-sleep.sh and systemd service files
Summary: >x11-drivers/nvidia-drivers-430 should install nvidia-sleep.sh and systemd se...
Status: RESOLVED FIXED
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: Current packages (show other bugs)
Hardware: All Linux
: Normal normal (vote)
Assignee: Jeroen Roovers (RETIRED)
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-07-10 08:57 UTC by Maik
Modified: 2020-06-27 04:22 UTC (History)
3 users (show)

See Also:
Package list:
Runtime testing required: ---


Attachments
working (for me) nvidia-sleep.sh (nvidia-sleep.sh,868 bytes, text/plain)
2019-07-13 12:21 UTC, Maik
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Maik 2019-07-10 08:57:49 UTC
Starting with the v430 driver, the new kernel driver callback /proc/driver/nvidia/suspend was added to save/restore video memory on suspend/hibernate/resume to mitigate FBO corruption on resume:
https://download.nvidia.com/XFree86/Linux-x86_64/430.09/README/powermanagement.html
The driver package contains systemd units and a script.
Comment 1 Maik 2019-07-13 12:21:15 UTC
Created attachment 582756 [details]
working (for me) nvidia-sleep.sh

The nvidia-sleep.sh script included in the driver package is unusable, attaching a much simpler, working one which using chvt and fgconsole for vt switching.
Comment 2 Jeroen Roovers (RETIRED) gentoo-dev 2019-07-13 12:35:03 UTC
Comment on attachment 582756 [details]
working (for me) nvidia-sleep.sh

Comments below the lines they relate to.


>#!/bin/bash

Nothing in particular should require bash specific features.

>
>RUN_DIR="/var/run/nvidia-sleep"

1) Shouldn't that be /run/... ?
2) It doesn't seem to make sense to me to create a directory in which a single temporary file will be placed.

>XORG_VT_FILE="${RUN_DIR}"/Xorg.vt_number
>
>case "$1" in
>    suspend|hibernate)
>        /bin/mkdir -p "${RUN_DIR}"
>        /usr/bin/fgconsole > "${XORG_VT_FILE}"
>        /usr/bin/chvt 63
>        if [[ $? -ne 0 ]]; then

This should be replaced with a non-bash test.

>                exit $?
>        fi
>        /bin/echo "$1" > /proc/driver/nvidia/suspend
>        exit $?
>        ;;
>    resume)
>        /bin/echo "$1" > /proc/driver/nvidia/suspend 

1) Looks like the line ends with a space.
2) What if this fails?

>        #
>        # Check if Xorg was determined to be running at the time
>        # of suspend, and whether its VT was recorded.  If so,
>        # attempt to switch back to this VT.
>        #
>        if [[ -f "${XORG_VT_FILE}" ]]; then
>            XORG_PID=$(cat "${XORG_VT_FILE}")
>            /bin/rm "${XORG_VT_FILE}"
>            /usr/bin/chvt "${XORG_PID}"
>        fi
>        exit 0
>        ;;
>    *)
>        exit 1
>esac
Comment 3 Maik 2019-07-13 12:50:17 UTC
(In reply to Jeroen Roovers from comment #2)
> Comment on attachment 582756 [details]
> >    resume)
> >        /bin/echo "$1" > /proc/driver/nvidia/suspend 
> 2) What if this fails?
If what fails? echoing? Please clarify.
echoing "resume" to that handle is instructing the driver to restore the video memory from tmp file. If that fails, the result would be the same corruption as it is now without that function. But there's no call back to get the result, AFAIK. Would be informational only, anyway, since you can literally see that it didn't work.
This is just the original script, I basically only replaced a lot of crap with that fgconsole line to get it to work at all, call it WIP.
Comment 4 Maik 2019-07-13 12:55:56 UTC
On second thought: those handles of course only exist if the nvidia driver is loaded so there has to be a check at the start of the script so it bails out if that isn't the case.
Comment 5 Maik 2019-07-13 14:34:09 UTC
A little ToDo-list for nvidia-sleep.sh besides the changes from post #2:
- make sure the handle /proc/driver/nvidia/suspend exists, i.e. the (correct) driver is loaded
- check /proc/driver/nvidia/params that the correct module parameter NVreg_PreserveVideoMemoryAllocations=1 is set
- make sure no Xorg is started on vt63, otherwise use other vt# and make note of it.
- writing to that handle only ever errors out if anything else than suspend,resume or hibernate is written to it, shouldn't happen due to switch case and previous check.
Comment 6 Sergey Ilinykh 2020-04-18 14:37:18 UTC
any news?

I added a package to my overlay which installs missed stuff right from driver's samples.

https://github.com/rion-overlay/rion-overlay/tree/master/sys-apps/nvidia-systemd-pm (layman -a rion)

But I believe this stuff has to be installed by nvidia-drivers package.
Comment 7 Larry the Git Cow gentoo-dev 2020-04-20 10:14:48 UTC
The bug has been closed via the following commit(s):

https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=19c0b0218d3848798c162725167be753c080621e

commit 19c0b0218d3848798c162725167be753c080621e
Author:     Jeroen Roovers <jer@gentoo.org>
AuthorDate: 2020-04-20 08:29:46 +0000
Commit:     Jeroen Roovers <jer@gentoo.org>
CommitDate: 2020-04-20 10:14:42 +0000

    x11-drivers/nvidia-drivers: Install nvidia-sleep.sh
    
    Package-Manager: Portage-2.3.99, Repoman-2.3.22
    Closes: https://bugs.gentoo.org/689588
    Signed-off-by: Jeroen Roovers <jer@gentoo.org>

 .../nvidia-drivers/nvidia-drivers-430.64-r2.ebuild | 556 ++++++++++++++++++++
 .../nvidia-drivers/nvidia-drivers-435.21-r2.ebuild | 574 ++++++++++++++++++++
 .../nvidia-drivers/nvidia-drivers-440.82-r1.ebuild | 580 +++++++++++++++++++++
 3 files changed, 1710 insertions(+)
Comment 8 Sergey Ilinykh 2020-04-20 10:22:31 UTC
I updated my ebuild accordingly.
But why not install everything (including systemd service files and one additional helper script) from nvidia-drivers?
Comment 9 Jeroen Roovers (RETIRED) gentoo-dev 2020-04-20 10:25:46 UTC
(In reply to Sergey Ilinykh from comment #8)
> I updated my ebuild accordingly.

I have no idea what that means.

> But why not install everything (including systemd service files

I assumed it installed those already. Reopening accordingly.

> and one additional helper script) from nvidia-drivers?

I have no idea what that means.
Comment 10 Sergey Ilinykh 2020-04-20 10:29:33 UTC
I already referenced my ebuild above (https://github.com/rion-overlay/rion-overlay/tree/master/sys-apps/nvidia-systemd-pm ). Its src_install has exactly what I mean.
Thanks in advance.
Comment 11 Jeroen Roovers (RETIRED) gentoo-dev 2020-04-20 10:39:58 UTC
Comment on attachment 582756 [details]
working (for me) nvidia-sleep.sh

This is basically the same as the upstream nvidia-sleep.sh.
Comment 12 Sergey Ilinykh 2020-04-20 10:41:31 UTC
And well according to the docs it tries to keep nvidia driver state in /tmp which iirc is tmpfs for Gentoo.
Particularly my laptop has enough RAM for this. But I can't say for everyone else. So maybe it has to be somehow configurable. Or at least some bold warning has to be shown.
Comment 13 Jeroen Roovers (RETIRED) gentoo-dev 2020-04-20 10:42:50 UTC
Hi systemd@.

nvidia-drivers packages three systemd files which need to be installed.

~/portage/x11-drivers/nvidia-drivers-440.82/work $
find -name '*service'
./nvidia-hibernate.service
./nvidia-resume.service
./nvidia-suspend.service

systemd.eclass dislikes those file names. How do I fix that?
Comment 14 Jeroen Roovers (RETIRED) gentoo-dev 2020-04-20 10:43:29 UTC
(In reply to Sergey Ilinykh from comment #10)
> I already referenced my ebuild above
> (https://github.com/rion-overlay/rion-overlay/tree/master/sys-apps/nvidia-
> systemd-pm ). Its src_install has exactly what I mean.
> Thanks in advance.

Please attach anything you want to discuss here.
Comment 15 Sergey Ilinykh 2020-04-20 10:44:51 UTC
> Please attach anything you want to discuss here.

src_install() {
	insinto /lib/systemd/system
	doins *.service
	exeinto /lib/systemd/system-sleep
	doexe nvidia
	echo "options nvidia NVreg_PreserveVideoMemoryAllocations=1" > nvidia-pm.conf
	insinto /etc/modprobe.d/
	doins nvidia-pm.conf

	ewarn "To enable nvidia sleep services execute next commands:"
	ewarn "    systemctl enable nvidia-suspend.service"
	ewarn "    systemctl enable nvidia-hibernate.service"
	ewarn "    systemctl enable nvidia-resume.service"
	ewarn "Note nvidia kernel module reload (or reboot) is required"
	ewarn "to accept new driver options."
	ewarn "More details at /usr/share/doc/nvidia-drivers-${PV}/html/powermanagement.html"
}
Comment 16 Maik 2020-04-20 11:36:06 UTC
(In reply to Sergey Ilinykh from comment #12)
> And well according to the docs it tries to keep nvidia driver state in /tmp
> which iirc is tmpfs for Gentoo.
> Particularly my laptop has enough RAM for this. But I can't say for everyone
> else. So maybe it has to be somehow configurable. Or at least some bold
> warning has to be shown.

From Nvidia docs:
By default, these files are created in /tmp, but this location can be changed with the TemporaryFilePath kernel module parameter, e.g. TemporaryFilePath=/run

Don't know if all those messages about enabling the systemd files and kernel module parameters should be in the emerge messages or rather a link to the doc (see OP) since there are more FS requirements to be met when a different location is set.
Comment 17 Mike Gilbert gentoo-dev 2020-04-20 15:46:07 UTC
(In reply to Jeroen Roovers from comment #13)
> systemd.eclass dislikes those file names. How do I fix that?

I'm not sure what this means. How are you calling systemd.eclass, and what unexpected behavior do you experience?
Comment 18 Jeroen Roovers (RETIRED) gentoo-dev 2020-04-21 06:38:29 UTC
(In reply to Sergey Ilinykh from comment #15)
> > Please attach anything you want to discuss here.
> 
> src_install() {

Thanks!

> 	insinto /lib/systemd/system

^^^There is probably some voodoo in systemd.eclass that should help decide what "/lib/systemd/system" should be.

> 	doins *.service
> 	exeinto /lib/systemd/system-sleep
> 	doexe nvidia
> 	echo "options nvidia NVreg_PreserveVideoMemoryAllocations=1" >
> nvidia-pm.conf
> 	insinto /etc/modprobe.d/
> 	doins nvidia-pm.conf

^^^We already install /etc/modprobe.d/nvidia.conf to sets kernel module options, so I wonder if we should be able to simply add this line to nvidia.conf instead.

> 	ewarn "To enable nvidia sleep services execute next commands:"
> 	ewarn "    systemctl enable nvidia-suspend.service"
> 	ewarn "    systemctl enable nvidia-hibernate.service"
> 	ewarn "    systemctl enable nvidia-resume.service"

^^^I guess there no way to make this easier?
Comment 19 Jeroen Roovers (RETIRED) gentoo-dev 2020-04-21 06:41:00 UTC
(In reply to Mike Gilbert from comment #17)
> (In reply to Jeroen Roovers from comment #13)
> > systemd.eclass dislikes those file names. How do I fix that?
> 
> I'm not sure what this means. How are you calling systemd.eclass, and what
> unexpected behavior do you experience?

>>> Install x11-drivers/nvidia-drivers-430.64-r2 into /home/jer/portage/x11-drivers/nvidia-drivers-430.64-r2/image
 * ERROR: x11-drivers/nvidia-drivers-430.64-r2::gentoo failed (install phase):
 *   Source file needs .conf suffix
 *
 * Call stack:
 *     ebuild.sh, line  125:  Called src_install
 *   environment, line 4182:  Called systemd_install_serviced 'nvidia-hibernate.service'
 *   environment, line 4464:  Called die
 * The specific snippet of code:
 *           [[ ${src} == *.conf ]] || die "Source file needs .conf suffix";

Apparently this is invalid. :-)
Comment 20 Sergey Ilinykh 2020-04-21 13:06:25 UTC
> We already install /etc/modprobe.d/nvidia.conf to sets kernel module options, so I wonder if we should be able to simply add this line to nvidia.conf instead.

sounds good to me.
Comment 21 Mike Gilbert gentoo-dev 2020-04-21 14:31:39 UTC
(In reply to Jeroen Roovers from comment #19)

Use "systemd_dounit" instead.

To use OpenRC as an analogy:

systemd_dounit is like "doinitd". It installs a systemd unit to /lib/systemd/system. Said unit may be a service definition (*.service), similar in function to an init script. Other unit types are also possible.

systemd_install_serviced is like "doconfd". It installs a unit override config file as /etc/systemd/system/*.service.d/*.conf. This is optional, and only rarely used when there are settings that a sysadmin would typically customize and this is not possible through a domain-specific config file.

As an aside: At some point, I might rename "systemd_install_serviced" to "systemd_do_unit.d", to match the portage helper naming convention and to make it obvious that the function can be used for units other than services.
Comment 22 Jeroen Roovers (RETIRED) gentoo-dev 2020-04-29 05:58:16 UTC
(In reply to Sergey Ilinykh from comment #20)
> > We already install /etc/modprobe.d/nvidia.conf to sets kernel module options, so I wonder if we should be able to simply add this line to nvidia.conf instead.
> 
> sounds good to me.

OK, does it work to add that separately or does that variable need to be included on the same line as the other variables?
Comment 23 Jeroen Roovers (RETIRED) gentoo-dev 2020-04-29 05:59:55 UTC
(In reply to Mike Gilbert from comment #21)
> (In reply to Jeroen Roovers from comment #19)
> 
> Use "systemd_dounit" instead.

Right, that appears to work.
Comment 24 Jeroen Roovers (RETIRED) gentoo-dev 2020-04-29 07:09:33 UTC
(In reply to Sergey Ilinykh from comment #15)
> 	ewarn "More details at
> /usr/share/doc/nvidia-drivers-${PV}/html/powermanagement.html"
> }

That document also mentions TemporaryFilePath, and yet (using 440.82):

[   13.351291] nvidia: unknown parameter 'TemporaryFilePath' ignored
Comment 25 Maik 2020-04-29 07:36:45 UTC
modinfo nvidia tells the correct name is
NVreg_TemporaryFilePath
Comment 26 Jeroen Roovers (RETIRED) gentoo-dev 2020-04-29 07:55:57 UTC
(In reply to Maik from comment #25)
> modinfo nvidia tells the correct name is
> NVreg_TemporaryFilePath

Yes, I'll try to integrate that.
Comment 27 Larry the Git Cow gentoo-dev 2020-04-29 08:08:22 UTC
The bug has been referenced in the following commit(s):

https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=613c3dc0e336ee65e262d082084d9a05da84a849

commit 613c3dc0e336ee65e262d082084d9a05da84a849
Author:     Jeroen Roovers <jer@gentoo.org>
AuthorDate: 2020-04-29 08:07:49 +0000
Commit:     Jeroen Roovers <jer@gentoo.org>
CommitDate: 2020-04-29 08:08:19 +0000

    x11-drivers/nvidia-drivers: Install systemd service files
    
    Package-Manager: Portage-2.3.99, Repoman-2.3.22
    Bug: https://bugs.gentoo.org/689588
    Signed-off-by: Jeroen Roovers <jer@gentoo.org>

 x11-drivers/nvidia-drivers/files/nvidia-430.conf   |  20 +
 .../nvidia-drivers/nvidia-drivers-430.64-r3.ebuild | 566 ++++++++++++++++++++
 .../nvidia-drivers/nvidia-drivers-435.21-r3.ebuild | 584 ++++++++++++++++++++
 .../nvidia-drivers/nvidia-drivers-440.82-r2.ebuild | 590 +++++++++++++++++++++
 4 files changed, 1760 insertions(+)
Comment 28 Jeroen Roovers (RETIRED) gentoo-dev 2020-04-30 17:30:11 UTC
(In reply to Larry the Git Cow from comment #27)
> The bug has been referenced in the following commit(s):
> 
> https://gitweb.gentoo.org/repo/gentoo.git/commit/
> ?id=613c3dc0e336ee65e262d082084d9a05da84a849
> 
> commit 613c3dc0e336ee65e262d082084d9a05da84a849
> Author:     Jeroen Roovers <jer@gentoo.org>
> AuthorDate: 2020-04-29 08:07:49 +0000
> Commit:     Jeroen Roovers <jer@gentoo.org>
> CommitDate: 2020-04-29 08:08:19 +0000
> 
>     x11-drivers/nvidia-drivers: Install systemd service files
>     
>     Package-Manager: Portage-2.3.99, Repoman-2.3.22
>     Bug: https://bugs.gentoo.org/689588
>     Signed-off-by: Jeroen Roovers <jer@gentoo.org>
> 
>  x11-drivers/nvidia-drivers/files/nvidia-430.conf   |  20 +
>  .../nvidia-drivers/nvidia-drivers-430.64-r3.ebuild | 566
> ++++++++++++++++++++
>  .../nvidia-drivers/nvidia-drivers-435.21-r3.ebuild | 584
> ++++++++++++++++++++
>  .../nvidia-drivers/nvidia-drivers-440.82-r2.ebuild | 590
> +++++++++++++++++++++
>  4 files changed, 1760 insertions(+)

So does that work? Anyone?
Comment 29 Maik 2020-04-30 19:08:28 UTC
The file /lib/systemd/system-sleep/nvidia seems to be missing

Comment #18 referenced it
> 	exeinto /lib/systemd/system-sleep
> 	doexe nvidia

but now I can't find it in the ebuild x11-drivers/nvidia-drivers-440.82-r2
Comment 30 Maik 2020-04-30 19:18:42 UTC
Some general oddity: since I had this running since july 2019, I already had /usr/bin/nvidia-sleep.sh. This file now got overwritten with the one from ebuild without any collision warning. Bug in portage?
Comment 31 Mike Gilbert gentoo-dev 2020-04-30 19:32:24 UTC
(In reply to Maik from comment #30)

Portage probably warned you it was overwriting the file because the installed file was not owned by any package.
Comment 32 Jeroen Roovers (RETIRED) gentoo-dev 2020-04-30 19:38:12 UTC
(In reply to Maik from comment #29)
> The file /lib/systemd/system-sleep/nvidia seems to be missing
> 
> Comment #18 referenced it
> > 	exeinto /lib/systemd/system-sleep
> > 	doexe nvidia
> 
> but now I can't find it in the ebuild x11-drivers/nvidia-drivers-440.82-r2

Oh I missed that. Sorry.
Comment 33 Maik 2020-04-30 19:46:16 UTC
(In reply to Mike Gilbert from comment #31)
> (In reply to Maik from comment #30)
> 
> Portage probably warned you it was overwriting the file because the
> installed file was not owned by any package.

Nope, no warning at all, nothing. Or does portage now check if contents/hash of unowned collision match?
Comment 34 Jeroen Roovers (RETIRED) gentoo-dev 2020-04-30 19:54:49 UTC
So that would be $(systemd_get_utildir)/system-sleep then?
Comment 35 Mike Gilbert gentoo-dev 2020-04-30 20:00:45 UTC
(In reply to Jeroen Roovers from comment #34)
> So that would be $(systemd_get_utildir)/system-sleep then?

Technically, we should add an eclass function to expose the systemdsleepdir pkgconfig variable, but $(systemd_get_utildir)/system-sleep will work for now.
Comment 36 Larry the Git Cow gentoo-dev 2020-05-01 04:08:26 UTC
The bug has been referenced in the following commit(s):

https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=b622e2f86a2be4087fd4c6b6444380715ca4375b

commit b622e2f86a2be4087fd4c6b6444380715ca4375b
Author:     Jeroen Roovers <jer@gentoo.org>
AuthorDate: 2020-04-30 20:03:48 +0000
Commit:     Jeroen Roovers <jer@gentoo.org>
CommitDate: 2020-05-01 04:08:22 +0000

    x11-drivers/nvidia-drivers: Install /lib/systemd/system-sleep/nvidia
    
    Package-Manager: Portage-2.3.99, Repoman-2.3.22
    Bug: https://bugs.gentoo.org/689588
    Signed-off-by: Jeroen Roovers <jer@gentoo.org>

 ...{nvidia-drivers-430.64-r3.ebuild => nvidia-drivers-430.64-r4.ebuild} | 2 ++
 ...{nvidia-drivers-435.21-r3.ebuild => nvidia-drivers-435.21-r4.ebuild} | 2 ++
 ...{nvidia-drivers-440.82-r2.ebuild => nvidia-drivers-440.82-r3.ebuild} | 2 ++
 3 files changed, 6 insertions(+)
Comment 37 Maik 2020-05-01 09:51:58 UTC
Thank you, works now.
Closing this.
Comment 38 Jeroen Roovers (RETIRED) gentoo-dev 2020-05-01 11:11:19 UTC
Thanks.
Comment 39 Jeroen Roovers (RETIRED) gentoo-dev 2020-05-24 06:09:14 UTC
Apparently calling systemd_get_utildir() this way is incorrect because it prefixes PREFIX. Or at least that is what [0] implies.


[0] https://qa-reports.gentoo.org/output/gentoo-ci/88e4371c/output.html#x11-drivers/nvidia-drivers
Comment 40 Larry the Git Cow gentoo-dev 2020-06-27 04:22:50 UTC
The bug has been closed via the following commit(s):

https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=7e7c112c7d334a19e0048054a9ce888964b50ae3

commit 7e7c112c7d334a19e0048054a9ce888964b50ae3
Author:     Jeroen Roovers <jer@gentoo.org>
AuthorDate: 2020-06-27 04:14:36 +0000
Commit:     Jeroen Roovers <jer@gentoo.org>
CommitDate: 2020-06-27 04:22:42 +0000

    x11-drivers/nvidia-drivers: Do not use systemd_get_utildir()
    
    Package-Manager: Portage-2.3.103, Repoman-2.3.23
    Closes: https://bugs.gentoo.org/689588
    Signed-off-by: Jeroen Roovers <jer@gentoo.org>

 .../nvidia-drivers/nvidia-drivers-430.64-r5.ebuild | 564 ++++++++++++++++++++
 .../nvidia-drivers/nvidia-drivers-435.21-r5.ebuild | 582 ++++++++++++++++++++
 .../nvidia-drivers-440.100-r1.ebuild               | 585 ++++++++++++++++++++
 .../nvidia-drivers/nvidia-drivers-440.82-r3.ebuild |   2 +-
 .../nvidia-drivers/nvidia-drivers-440.82-r4.ebuild | 588 +++++++++++++++++++++
 .../nvidia-drivers/nvidia-drivers-450.51-r1.ebuild | 586 ++++++++++++++++++++
 6 files changed, 2906 insertions(+), 1 deletion(-)