Skip to content
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

CRTSwitchRes Updates: Including Raspberry PI #8132

Merged
merged 21 commits into from
Jan 31, 2019
Merged

CRTSwitchRes Updates: Including Raspberry PI #8132

merged 21 commits into from
Jan 31, 2019

Conversation

alphanu1
Copy link
Contributor

New Linux switching method partially implemented. Not all system calls have been replaced yet

Linux restore desktop resolution fixed.

Monitor index switching and auto enumerate for output detection in Linux (still working on the windows method)

Added Raspberry PI Switching method. This Uses Raspiberry PI userland. Currently this is if defed, this is a temporary measure and will need to be placed in a new display server eventually. Not all of userland is currently used. However, it may in the future.

@hizzlekizzle hizzlekizzle requested review from inactive123 and a user January 31, 2019 02:19
@inactive123
Copy link
Contributor

Just a general note - don't use sprintf anymore, I will clean this up for you in this case. Use snprintf instead of sprintf always.

@inactive123 inactive123 merged commit 3d092c8 into libretro:master Jan 31, 2019
@orbea
Copy link
Contributor

orbea commented Feb 1, 2019

@alphanu1 and @twinaphex New warnings.

gfx/display_servers/dispserv_x11.c: In function ‘x11_display_server_set_resolution’:
gfx/display_servers/dispserv_x11.c:138:12: warning: variable ‘scrn’ set but not used [-Wunused-but-set-variable]
    Screen *scrn             = NULL;
            ^~~~
gfx/display_servers/dispserv_x11.c: In function ‘x11_display_server_destroy’:
gfx/display_servers/dispserv_x11.c:86:34: warning: ‘%s’ directive output may be truncated writing up to 249 bytes into a region of size between 233 and 482 [-Wformat-truncation=]
             "xrandr --delmode %s %s",orig_output, old_mode);
                                  ^~               ~~~~~~~~
gfx/display_servers/dispserv_x11.c:85:7: note: ‘snprintf’ output between 19 and 517 bytes into a destination of size 500
       snprintf(output, sizeof(output),
       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
             "xrandr --delmode %s %s",orig_output, old_mode);
             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

@alphanu1
Copy link
Contributor Author

alphanu1 commented Feb 1, 2019

I will have a look into this. I have not noticed them while I was compiling.

@orbea what is your compile environment?

@orbea
Copy link
Contributor

orbea commented Feb 1, 2019

@alphanu1 Slackware Linux with gcc-8.2.0. These warnings were new with gcc8 and they used to be really spammy with RetroArch, but most have been entirely silenced/fixed until recently again.

@orbea
Copy link
Contributor

orbea commented Feb 1, 2019

@alphanu1 Would this work? I'm not sure how to test this change, but it at least silences the warning here.

diff --git a/gfx/display_servers/dispserv_x11.c b/gfx/display_servers/dispserv_x11.c
index 2c6d168cb4..07c5250be0 100644
--- a/gfx/display_servers/dispserv_x11.c
+++ b/gfx/display_servers/dispserv_x11.c
@@ -44,7 +44,7 @@ static char new_mode[250]       = {0};
 static char xrandr[250]         = {0};
 static char fbset[150]          = {0};
 static char output[500]         = {0};
-static char output4[500]         = {0};
+static char output4[500]        = {0};
 static bool crt_en              = false;
 static unsigned crtid           = 20;
 static XRRModeInfo crt_rrmode;
@@ -83,11 +83,9 @@ static void x11_display_server_destroy(void *data)
       system(output);
 
       snprintf(output, sizeof(output),
-            "xrandr --delmode %s %s",orig_output, old_mode);
+            "xrandr --delmode %s %.s",orig_output, old_mode);
       system(output);
 
-
-
       snprintf(output, sizeof(output), "xrandr --rmmode %s", old_mode);
       system(output);
    }

That would leave this warning.

gfx/display_servers/dispserv_x11.c: In function ‘x11_display_server_set_resolution’:
gfx/display_servers/dispserv_x11.c:136:12: warning: variable ‘scrn’ set but not used [-Wunused-but-set-variable]
    Screen *scrn             = NULL;
            ^~~~

Do you plan to do something with this variable or should it just be removed?

@alphanu1
Copy link
Contributor Author

alphanu1 commented Feb 1, 2019

The Screen variable will be used soon. It can remove for now though if you wish.

I'm going to create a pull request in a min as there is a resolution restore issue with auto detect.

@orbea
Copy link
Contributor

orbea commented Feb 3, 2019

@alphanu1 This appears to have broken compilation on the rpi? See issue #8159

Also would you mind squashing your PRs in the future and not changing all the line endings? It makes it much harder to review and test for regressions...

@alphanu1
Copy link
Contributor Author

alphanu1 commented Feb 3, 2019

Hey @orbea

I've just read through all the comments. Looks like it's been solved.

I'm not quite sure what happened with the line ending, it may have been my editor. Chers for letting me know I'll look out for it in the future.

@dankcushions
Copy link
Contributor

why was the entire RPI userland brought in? surely if you're using an RPI this is already available?

zoltanvb added a commit to zoltanvb/RetroArch that referenced this pull request Dec 1, 2023
Userland was added at: libretro#8132
However, its only use was with CRTSwitchRes together with VideoCore.
Over the times, the legacy VideoCore drivers have lost usage as
nowadays Mesa supports the VideoCore IV chip in RPi 0..3, and
RPi 4 and/or 64-bit was never supported.

Option is kept to use the legacy drivers, but it is now utilized
from the OS.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants