Skip to content
This repository was archived by the owner on Apr 1, 2026. It is now read-only.

Support for running Redshift outside X#42

Merged
jonls merged 1 commit intosharpbracket:masterfrom
maandree:gamma-drm
Mar 22, 2014
Merged

Support for running Redshift outside X#42
jonls merged 1 commit intosharpbracket:masterfrom
maandree:gamma-drm

Conversation

@maandree
Copy link
Copy Markdown
Contributor

Using Direct Rendering Manager, Redshift can now change gamma ramps for the monitors while running in Linux's TTY. This requires that the users is a member of the group video (as is always required when working with graphics without a display server.)

If Direct Rendering Manager is used to change the gamma ramps while an graphical session is active in the foreground (and X display is running on the active VT) Direct Rendering Manager will ignore the request and report that you do not have sufficient permissions. This rejection is ignored so nothing funny happens if the users opens a VT with a graphical session.

Changes to gamma ramps using Direct Rendering Manager are in effect on all VT:s with the exception on graphical ones because the X servers keeps tracks of gamma ramps for each display and applies the gamma ramps associated with a display when it is reactivated.

Possible improvements:

  • Figure out whether Redshift is associated with an X display, and user DRM otherwise.

Comment thread src/gamma-drm.c
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

open should be enough to determine if permissions are ok by checking errno (or maybe I misunderstand this TODO?).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note well expressed TODO: The idea is that on failure the permission bits, owner and groups as well as you group membership should be read to determine why your are not able to open it and give a good explaination why you cannot use it. For example, the sysadmin might have given you membership to video but have change the permissions so that only root can get access to the graphics card as it can change monitor resolution which for example some schools (for some reason I do not understand) remove that permission from you.

@jonls
Copy link
Copy Markdown
Collaborator

jonls commented Mar 16, 2014

Very cool, I did not know that DRM was capable of adjusting gamma ramps.

@maandree
Copy link
Copy Markdown
Contributor Author

Grep:ing /usr/include is king!

Comment thread src/gamma-drm.c Outdated
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some weird style here, should be } else {...

@maandree
Copy link
Copy Markdown
Contributor Author

I have fixed the check at atoi and consistency with braces. But I do not know how to squash the commits without having to rewrite them and all commits in the other pull requests.

@jonls
Copy link
Copy Markdown
Collaborator

jonls commented Mar 20, 2014

Ok then how about you squash these and I merge them, and you can then rebase your other pulls on the new master?

@maandree
Copy link
Copy Markdown
Contributor Author

I have squash the commits in this branch.

Comment thread src/gamma-drm.c Outdated
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems that we could end here if drmModeCrtcGetGamma fails, in which case perror("malloc") is misleading.

@jonls
Copy link
Copy Markdown
Collaborator

jonls commented Mar 21, 2014

Great, I did some more testing and I have a few more questions. I noticed that there is no error message when there is a failure to adjust gamma, instead it will just pretend that everything is ok. Did you add more error reporting in the multimonitor branch? If you already did we can just postpone it for that pull.

I see that you put drm at the top of the gamma_methods array in redshift.c which means that it will take priority above randr and vidmode. I don't think that is right since most people will still expect randr to run by default. I think drm should have autostart set to 0 so that it will only start when explicitly selected.

@maandree
Copy link
Copy Markdown
Contributor Author

I noticed that there is no error message when there is a failure to adjust gamma, instead it will just pretend that everything is ok.

Everything is ok, I tought I added a comment but perhaps I forget. But I did write about it in the pull request:

If Direct Rendering Manager is used to change the gamma ramps while an graphical
session is active in the foreground (and X display is running on the active VT) Direct
Rendering Manager will ignore the request and report that you do not have sufficient
permissions. This rejection is ignored so nothing funny happens if the users opens a
VT with a graphical session.

I see that you put drm at the top of the gamma_methods array in redshift.c which means that it will take priority above randr and vidmode.

Did not think about that, I just went alphabetical. I will fix that.

@jonls
Copy link
Copy Markdown
Collaborator

jonls commented Mar 21, 2014

Ok, what I mean is, e.g if DRM is supported but the size of the gamma ramp is returned as 0 then we skip that gamma ramp because we cannot do any adjustment but actually we should probably report an error so the user can determine that something is not working.

@maandree
Copy link
Copy Markdown
Contributor Author

Oh, I missed to check that, I will add a check with an error report.

@jonls
Copy link
Copy Markdown
Collaborator

jonls commented Mar 22, 2014

Works fine for me now. I noticed that the gamma adjustment is not reinstated when switching to an X screen and back again, which is a pity. Do you see the same issue? If you squash the commits again I will merge.

@maandree
Copy link
Copy Markdown
Contributor Author

Yes, the gamma settings from the X display is used. If you have on instance for X and another instance for TTY, it shouldn't be noticable provided that they have the same configurations.

… by using Direct Rendering Manager

Signed-off-by: Mattias Andrée <maandree@operamail.com>
@maandree
Copy link
Copy Markdown
Contributor Author

I have squashed the commits.

jonls pushed a commit that referenced this pull request Mar 22, 2014
Support for running Redshift outside X using libdrm
@jonls jonls merged commit 09efadd into sharpbracket:master Mar 22, 2014
@maandree maandree deleted the gamma-drm branch March 22, 2014 19:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants