-
Notifications
You must be signed in to change notification settings - Fork 202
hw/xfree86/modes: Update Default Mode Selection in xf86Crtc to use highest refresh rate #815
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
base: master
Are you sure you want to change the base?
Conversation
|
@HaplessIdiot you should be able to update your old PR. Just push/force push your new changes to your old branch. |
| refresh = ((float)mode->Clock * 1000.0f) / | ||
| ((float)mode->HTotal * (float)mode->VTotal); | ||
| if ((int)(refresh + 0.5f) >= 75) | ||
| return mode->name; |
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.
This should be inside the previous if body.
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.
thanks that can clean it up even further! great catch. this part was needed to help with the weird 59.94hz and similar NTSC refresh rates that are NOT cleanly 60hz or 120hz needed rounded to make the function behave as normal.
|
I can’t say if this change does what it’s supposed to do really, need other reviewers opinion. |
when im in XFCE and KDE Plasma it makes it select the EDID option with the highest refresh rate on my TV that makes it go to 1080p60hz so it has some wierdness with 4k30hz tvs but you can set it back to the higher resolution mode been using this handy patch since july to save time in display configs. would really like to see if it performs correctly outside of those two window managers. |
If this code is unable to find any mode >75hz it goes to fallback and just selects the first available mode in the list. What’s the desired outcome here? Can we also take into account resolution if that information is available? Or select the highest available refresh rate even if it’s less than 75hz? |
callmetango
left a comment
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.
I am sceptical that the mode selection works so simple and I would like to see more reasoning in an attached issue about the corner cases. My guts feeling tells me that the highest refresh rate might no always be the best or even a working one. It also tells me that this might not always be the native one. Let alone resolution. I haven't researched this any further so please prove me wrong.
I think you are right adding further features than just checking for hz rate would be nicer. i can add a highest refresh rate check too so it does the highest hz with res also incorporated into the decision. been using this on my own stuff for a while since july, its a cheap edit. What would a fast refresh rate be? 60hz and higher? |
it behaves correctly for me under KDE/XFCE but i have NOT done wide testing outside of garuda os and steamdeck os. it works with my random mix of 2 displays and tv somehow. i had to add some patches for SOME NVIDIA DRIVERS so the 1080 can act right with it. VRefresh works strange on this card i have no idea how the newer nvidia cards will like their display read so that needs further testing |
|
@ALL what's the status of this ? |
…sue X11Libre#122 This change makes the Default Mode Selection set itself to Highest Refresh Rate at and after 75hz or fallback to the default setting in xorg.conf as requested by issue X11Libre#122 Feel free to simplify or edit the function for further compatibility. Reposted from X11Libre#409 into one single commit
This change makes the Default Mode Selection set itself to Highest Refresh Rate at and after 75hz or fallback to the default setting in xorg.conf as requested by issue #122
Feel free to simplify or edit the function for further compatibility.
Reposted from #409 into one single commit