-
Notifications
You must be signed in to change notification settings - Fork 77
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
lightdm-fb: add hotplug support for display port #466
base: master
Are you sure you want to change the base?
Conversation
Add hotplug support for display port. Add lightdm-fb package to lxde.yml by default Fixes: siemens#463 Signed-off-by: Arulpandiyan Vadivel <[email protected]>
@jan-kiszka @BaochengSu Kindly help to review the changes Display port hotplug is working logs were attached for reference below.
|
# | ||
|
||
# Add support for hotplug of Display Port | ||
sed -i 's/LABEL="systemd_end"/SUBSYSTEM=="graphics", KERNEL=="fb0", SYMLINK="fb0", TAG+="systemd"\nLABEL="systemd_end"/g' /lib/udev/rules.d/99-systemd.rules |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please ship and install a file to /etc. Make this package a customization package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in addition, the whole purpose of rules.d is to let people add their own rules (in separate files)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The correct debian way would be to use dh_installudev
. By that, the file also gets correctly numbered. Just put it into its own package and add a file <package>.udev
in the debian folder. For details, see man dh_installudev
.
# Add support for hotplug of Display Port | ||
sed -i 's/LABEL="systemd_end"/SUBSYSTEM=="graphics", KERNEL=="fb0", SYMLINK="fb0", TAG+="systemd"\nLABEL="systemd_end"/g' /lib/udev/rules.d/99-systemd.rules | ||
|
||
sed -i '/plymouth/d' /lib/systemd/system/lightdm.service |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, thats's what /etc/systemd
is for.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is also unclear what we are doing here (I mean I understand that we are essentially killing every single line where the word plymouth can be found but why?)
And the commit messages of the patch should explain what is going on, not just list what it does. |
Will this receive an update? |
Yes we will get an update,
Can you help me to understand what do you mean by making customization package? Rest of the comments I can understand |
Create something like customizations-lxde or add your custom lightdm.service to https://github.com/siemens/meta-iot2050/tree/master/recipes-core/customizations-example if the lxde image is selected. The former might be cleaner, see also https://github.com/siemens/meta-iot2050/tree/master/recipes-core/customizations-swupdate |
Thanks for the information, I will update the PR shortly |
sed -i 's/LABEL="systemd_end"/SUBSYSTEM=="graphics", KERNEL=="fb0", SYMLINK="fb0", TAG+="systemd"\nLABEL="systemd_end"/g' /lib/udev/rules.d/99-systemd.rules | ||
|
||
sed -i '/plymouth/d' /lib/systemd/system/lightdm.service | ||
sed -i '/Service/i [email protected] [email protected]\[email protected] [email protected]\nBindsTo=dev-fb0.device\nAfter=dev-fb0.device\n' /lib/systemd/system/lightdm.service |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please use a systemd dropin (we should avoid patching systemd units from other packages)
@@ -0,0 +1,12 @@ | |||
# |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we ship a postinst script, we should ideally have a postrm script to support uninstallation
Waiting for an update... |
Still no update - while gates are closing... |
Add hotplug support for display port.
Add lightdm-fb package to lxde.yml by default
Fixes: #463