Page MenuHomeFreeBSD
Differential D26253

linuxkpi: Add backlight support
ClosedPublic
Actions

Authored by manu on Aug 31 2020, 2:56 PM.
Tags
None
Referenced Files
F113077953: D26253.id76592.diff
Wed, Mar 26, 5:27 AM
Unknown Object (File)
Mon, Mar 17, 12:07 PM
Unknown Object (File)
Thu, Mar 6, 3:16 AM
Unknown Object (File)
Wed, Mar 5, 9:28 AM
Unknown Object (File)
Feb 23 2025, 10:39 AM
Unknown Object (File)
Feb 23 2025, 2:53 AM
Unknown Object (File)
Feb 22 2025, 9:00 AM
Unknown Object (File)
Feb 22 2025, 6:18 AM
View All Files
Subscribers
imp
zeising
linuxkpi

Details

Reviewers
• hselasky
emaste
Summary

Add backlight function to linuxkpi.
Graphics drivers expose the backlight of the panel directly so allow them to use the backlight subsystem so
user can use backlight(8) to configure them.

Sponsored-by: The FreeBSD Foundation.

Test Plan

Tested using https://github.com/freebsd/drm-kmod/tree/backlight on intel and amd laptop.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

manu created this revision.Aug 31 2020, 2:56 PM
Herald added a subscriber: imp. · View Herald TranscriptAug 31 2020, 2:56 PM
manu requested review of this revision.Aug 31 2020, 2:56 PM
manu added a parent revision: D26250: Add backlight subsystem.
manu edited the test plan for this revision. (Show Details)
manu added a subscriber: zeising.
manu updated this revision to Diff 76405.Aug 31 2020, 2:59 PM

Remove changes that should have been part of D26250

manu mentioned this in D26250: Add backlight subsystem.Aug 31 2020, 3:00 PM
zeising added a comment.Aug 31 2020, 3:01 PM

Nice work!
How is the backlight adjustments exposed to users?
Do we need to change drm-current-kmod?

sys/compat/linuxkpi/common/include/linux/backlight.h
32–33

Should this be _LINUX_BACKLIGHT_H_ ?

93

I think you should add something like
/* !_LINUX_BACKLIGHT_H */ here

manu added a comment.Aug 31 2020, 3:07 PM
In D26253#583404, @zeising wrote:

Nice work!
How is the backlight adjustments exposed to users?

See https://reviews.freebsd.org/D26251

Do we need to change drm-current-kmod?

Yes I think so, I'll not commit before making a PR (if required).

sys/compat/linuxkpi/common/include/linux/backlight.h
32–33

Yeah, I'll update

93

It's not needed but indeed better.

manu updated this revision to Diff 76410.Aug 31 2020, 3:10 PM
• hselasky accepted this revision.Aug 31 2020, 4:09 PM
This revision is now accepted and ready to land.Aug 31 2020, 4:09 PM
• hselasky added inline comments.Aug 31 2020, 4:11 PM
sys/compat/linuxkpi/common/include/linux/backlight.h
78

Please use function macros here !

manu updated this revision to Diff 76592.Sep 3 2020, 12:35 PM

Address @hselasky's comments.

This revision now requires review to proceed.Sep 3 2020, 12:35 PM
• hselasky added inline comments.Sep 3 2020, 12:42 PM
sys/modules/linuxkpi/Makefile
26

I think backlight_if.h should be added to LINUXKPI_GENSRCS definition in

sys/conf/kmod.mk:LINUXKPI_GENSRCS

• hselasky accepted this revision.Sep 3 2020, 12:44 PM
• hselasky added inline comments.
sys/modules/linuxkpi/Makefile
26

Forget that. It is only used when building the LinuxKPI!

This revision is now accepted and ready to land.Sep 3 2020, 12:44 PM
manu updated this revision to Diff 76597.Sep 3 2020, 12:45 PM

Add backlight_if.h to LINUXKPI_GENSRC variable.

This revision now requires review to proceed.Sep 3 2020, 12:45 PM
manu added a comment.Sep 3 2020, 12:47 PM

No this is the right place for _if.h files.

manu marked 3 inline comments as done.Sep 3 2020, 12:48 PM
• hselasky accepted this revision.Sep 3 2020, 12:51 PM
This revision is now accepted and ready to land.Sep 3 2020, 12:51 PM
manu updated this revision to Diff 77106.Sep 16 2020, 4:33 PM

Add support for GETINFO and free backlight data.

This revision now requires review to proceed.Sep 16 2020, 4:33 PM
• hselasky accepted this revision.Sep 16 2020, 5:17 PM

Don't forget to bump the FreeBSD KERNEL version after this.

This revision is now accepted and ready to land.Sep 16 2020, 5:17 PM
manu closed this revision.Oct 5 2020, 2:14 PM

Commited in r366372

Herald added a subscriber: linuxkpi. · View Herald TranscriptOct 5 2020, 2:14 PM

Revision Contents
Changeset List

PathSize
sys/
compat/
linuxkpi/
common/
include/
linux/
92 lines
4 lines
src/
1 line
65 lines
modules/
linuxkpi/
3 lines
sys/
5 lines

Diff 76405

View Options

sys/compat/linuxkpi/common/include/linux/backlight.h

Context not available.
JAVA="%%LOCALBASE%%/bin/java" JAVA="%%LOCALBASE%%/bin/java"
JARPATH="%%JAVALIBDIR%%" JARPATH="%%JAVALIBDIR%%"
export JAVA_VERSION="%%JAVA_VERSION%%"
glewisUnsubmitted
Not Done
Inline Actions

Note that this may cause a subtle change in behaviour if either the ports tree is not present on a system or if javavmwrapper is otherwise instructed to use it's own internal logic (e.g. if JAVAVM_FALLBACK_ONLY is set).

> env JAVAVM_FALLBACK_ONLY=yes JAVA_VERSION=1.6+ /usr/local/bin/java -version                                                                       openjdk version "12.0.2" 2019-07-16

> env JAVAVM_FALLBACK_ONLY=yes /usr/local/bin/java -version 
openjdk version "13.0.1" 2019-10-15

This will depend on what versions the user has installed. This is actually more of a bug in javavmwrapper which I'll look into (it doesn't understand JDK 13 yet). It also doesn't have a default version (e.g. 8) and uses the "newest" one if no version is supplied. If a version is supplied then it uses the newest version that it understands.

An alternative to all these script changes would be to have bsd.java.mk always add JAVA_VERSION to SUB_LIST, using the version it ends up deciding on if none is set.

glewis: Note that this may cause a subtle change in behaviour if either the ports tree is not present…
jkimAuthorUnsubmitted
Not Done
Inline Actions

AFAIK, the inconsistency with JAVAVM_FALLBACK_ONLY is intentional. Actually, the manual page says:

However, this option, when used with scripts installed by a port,
may result in Java VM selection inconsistent with that intended
by the script author.
jkim: AFAIK, the inconsistency with JAVAVM_FALLBACK_ONLY is intentional. Actually, the manual page…
glewisUnsubmitted
Not Done
Inline Actions

As the author of javavmwrapper, it is certainly the case that when using JAVAVM_FALLBACK_ONLY that you may get different behaviour from what you do when the ports tree is present.

What is a bug is that the presence of "JAVA_VERSION=1.6+" in the two command lines above shouldn't make a difference. Whether it is present or not, the fallback logic should choose the same version. That is what I was pointing out. So by removing that line from the invocation, this change may affect what version is chosen in that situation, even though it shouldn't. The likelihood of this impacting people is quite low though, so I'm not overly concerned. I also have a fix I can commit after this change (I don't want to commit while this is in flight since it will conflict with changes here).

glewis: As the author of javavmwrapper, it is certainly the case that when using JAVAVM_FALLBACK_ONLY…
jkimAuthorUnsubmitted
Not Done
Inline Actions

Understood. Yes, it looks like there's a bug in javavmwrapper. It should not show any difference with or without the ports tree as the PR167799 said.

jkim: Understood. Yes, it looks like there's a bug in javavmwrapper. It should not show any…
exec "${JAVA}" -jar "${JARPATH}/%%JARNAME%%" "$@" exec "${JAVA}" -jar "${JARPATH}/%%JARNAME%%" "$@"
Context not available.
View Options

sys/compat/linuxkpi/common/include/linux/device.h

Context not available.
JAVA="%%LOCALBASE%%/bin/java" JAVA="%%LOCALBASE%%/bin/java"
JARPATH="%%JAVALIBDIR%%" JARPATH="%%JAVALIBDIR%%"
export JAVA_VERSION="%%JAVA_VERSION%%"
glewisUnsubmitted
Not Done
Inline Actions

Note that this may cause a subtle change in behaviour if either the ports tree is not present on a system or if javavmwrapper is otherwise instructed to use it's own internal logic (e.g. if JAVAVM_FALLBACK_ONLY is set).

> env JAVAVM_FALLBACK_ONLY=yes JAVA_VERSION=1.6+ /usr/local/bin/java -version                                                                       openjdk version "12.0.2" 2019-07-16

> env JAVAVM_FALLBACK_ONLY=yes /usr/local/bin/java -version 
openjdk version "13.0.1" 2019-10-15

This will depend on what versions the user has installed. This is actually more of a bug in javavmwrapper which I'll look into (it doesn't understand JDK 13 yet). It also doesn't have a default version (e.g. 8) and uses the "newest" one if no version is supplied. If a version is supplied then it uses the newest version that it understands.

An alternative to all these script changes would be to have bsd.java.mk always add JAVA_VERSION to SUB_LIST, using the version it ends up deciding on if none is set.

glewis: Note that this may cause a subtle change in behaviour if either the ports tree is not present…
jkimAuthorUnsubmitted
Not Done
Inline Actions

AFAIK, the inconsistency with JAVAVM_FALLBACK_ONLY is intentional. Actually, the manual page says:

However, this option, when used with scripts installed by a port,
may result in Java VM selection inconsistent with that intended
by the script author.
jkim: AFAIK, the inconsistency with JAVAVM_FALLBACK_ONLY is intentional. Actually, the manual page…
glewisUnsubmitted
Not Done
Inline Actions

As the author of javavmwrapper, it is certainly the case that when using JAVAVM_FALLBACK_ONLY that you may get different behaviour from what you do when the ports tree is present.

What is a bug is that the presence of "JAVA_VERSION=1.6+" in the two command lines above shouldn't make a difference. Whether it is present or not, the fallback logic should choose the same version. That is what I was pointing out. So by removing that line from the invocation, this change may affect what version is chosen in that situation, even though it shouldn't. The likelihood of this impacting people is quite low though, so I'm not overly concerned. I also have a fix I can commit after this change (I don't want to commit while this is in flight since it will conflict with changes here).

glewis: As the author of javavmwrapper, it is certainly the case that when using JAVAVM_FALLBACK_ONLY…
jkimAuthorUnsubmitted
Not Done
Inline Actions

Understood. Yes, it looks like there's a bug in javavmwrapper. It should not show any difference with or without the ports tree as the PR167799 said.

jkim: Understood. Yes, it looks like there's a bug in javavmwrapper. It should not show any…
exec "${JAVA}" -jar "${JARPATH}/%%JARNAME%%" "$@" exec "${JAVA}" -jar "${JARPATH}/%%JARNAME%%" "$@"
Context not available.
View Options

sys/compat/linuxkpi/common/src/linux_kmod.c

Context not available.
JAVA="%%LOCALBASE%%/bin/java" JAVA="%%LOCALBASE%%/bin/java"
JARPATH="%%JAVALIBDIR%%" JARPATH="%%JAVALIBDIR%%"
export JAVA_VERSION="%%JAVA_VERSION%%"
glewisUnsubmitted
Not Done
Inline Actions

Note that this may cause a subtle change in behaviour if either the ports tree is not present on a system or if javavmwrapper is otherwise instructed to use it's own internal logic (e.g. if JAVAVM_FALLBACK_ONLY is set).

> env JAVAVM_FALLBACK_ONLY=yes JAVA_VERSION=1.6+ /usr/local/bin/java -version                                                                       openjdk version "12.0.2" 2019-07-16

> env JAVAVM_FALLBACK_ONLY=yes /usr/local/bin/java -version 
openjdk version "13.0.1" 2019-10-15

This will depend on what versions the user has installed. This is actually more of a bug in javavmwrapper which I'll look into (it doesn't understand JDK 13 yet). It also doesn't have a default version (e.g. 8) and uses the "newest" one if no version is supplied. If a version is supplied then it uses the newest version that it understands.

An alternative to all these script changes would be to have bsd.java.mk always add JAVA_VERSION to SUB_LIST, using the version it ends up deciding on if none is set.

glewis: Note that this may cause a subtle change in behaviour if either the ports tree is not present…
jkimAuthorUnsubmitted
Not Done
Inline Actions

AFAIK, the inconsistency with JAVAVM_FALLBACK_ONLY is intentional. Actually, the manual page says:

However, this option, when used with scripts installed by a port,
may result in Java VM selection inconsistent with that intended
by the script author.
jkim: AFAIK, the inconsistency with JAVAVM_FALLBACK_ONLY is intentional. Actually, the manual page…
glewisUnsubmitted
Not Done
Inline Actions

As the author of javavmwrapper, it is certainly the case that when using JAVAVM_FALLBACK_ONLY that you may get different behaviour from what you do when the ports tree is present.

What is a bug is that the presence of "JAVA_VERSION=1.6+" in the two command lines above shouldn't make a difference. Whether it is present or not, the fallback logic should choose the same version. That is what I was pointing out. So by removing that line from the invocation, this change may affect what version is chosen in that situation, even though it shouldn't. The likelihood of this impacting people is quite low though, so I'm not overly concerned. I also have a fix I can commit after this change (I don't want to commit while this is in flight since it will conflict with changes here).

glewis: As the author of javavmwrapper, it is certainly the case that when using JAVAVM_FALLBACK_ONLY…
jkimAuthorUnsubmitted
Not Done
Inline Actions

Understood. Yes, it looks like there's a bug in javavmwrapper. It should not show any difference with or without the ports tree as the PR167799 said.

jkim: Understood. Yes, it looks like there's a bug in javavmwrapper. It should not show any…
exec "${JAVA}" -jar "${JARPATH}/%%JARNAME%%" "$@" exec "${JAVA}" -jar "${JARPATH}/%%JARNAME%%" "$@"
Context not available.
View Options

sys/compat/linuxkpi/common/src/linux_pci.c

Context not available.
JAVA="%%LOCALBASE%%/bin/java" JAVA="%%LOCALBASE%%/bin/java"
JARPATH="%%JAVALIBDIR%%" JARPATH="%%JAVALIBDIR%%"
export JAVA_VERSION="%%JAVA_VERSION%%"
glewisUnsubmitted
Not Done
Inline Actions

Note that this may cause a subtle change in behaviour if either the ports tree is not present on a system or if javavmwrapper is otherwise instructed to use it's own internal logic (e.g. if JAVAVM_FALLBACK_ONLY is set).

> env JAVAVM_FALLBACK_ONLY=yes JAVA_VERSION=1.6+ /usr/local/bin/java -version                                                                       openjdk version "12.0.2" 2019-07-16

> env JAVAVM_FALLBACK_ONLY=yes /usr/local/bin/java -version 
openjdk version "13.0.1" 2019-10-15

This will depend on what versions the user has installed. This is actually more of a bug in javavmwrapper which I'll look into (it doesn't understand JDK 13 yet). It also doesn't have a default version (e.g. 8) and uses the "newest" one if no version is supplied. If a version is supplied then it uses the newest version that it understands.

An alternative to all these script changes would be to have bsd.java.mk always add JAVA_VERSION to SUB_LIST, using the version it ends up deciding on if none is set.

glewis: Note that this may cause a subtle change in behaviour if either the ports tree is not present…
jkimAuthorUnsubmitted
Not Done
Inline Actions

AFAIK, the inconsistency with JAVAVM_FALLBACK_ONLY is intentional. Actually, the manual page says:

However, this option, when used with scripts installed by a port,
may result in Java VM selection inconsistent with that intended
by the script author.
jkim: AFAIK, the inconsistency with JAVAVM_FALLBACK_ONLY is intentional. Actually, the manual page…
glewisUnsubmitted
Not Done
Inline Actions

As the author of javavmwrapper, it is certainly the case that when using JAVAVM_FALLBACK_ONLY that you may get different behaviour from what you do when the ports tree is present.

What is a bug is that the presence of "JAVA_VERSION=1.6+" in the two command lines above shouldn't make a difference. Whether it is present or not, the fallback logic should choose the same version. That is what I was pointing out. So by removing that line from the invocation, this change may affect what version is chosen in that situation, even though it shouldn't. The likelihood of this impacting people is quite low though, so I'm not overly concerned. I also have a fix I can commit after this change (I don't want to commit while this is in flight since it will conflict with changes here).

glewis: As the author of javavmwrapper, it is certainly the case that when using JAVAVM_FALLBACK_ONLY…
jkimAuthorUnsubmitted
Not Done
Inline Actions

Understood. Yes, it looks like there's a bug in javavmwrapper. It should not show any difference with or without the ports tree as the PR167799 said.

jkim: Understood. Yes, it looks like there's a bug in javavmwrapper. It should not show any…
exec "${JAVA}" -jar "${JARPATH}/%%JARNAME%%" "$@" exec "${JAVA}" -jar "${JARPATH}/%%JARNAME%%" "$@"
Context not available.
View Options

sys/modules/linuxkpi/Makefile

Loading...
View Options

sys/sys/backlight.h

Loading...