Le 06/10/2021 à 15:54, Mark Cave-Ayland a écrit : > On 06/10/2021 13:24, Laurent Vivier wrote: > >>> This is where it becomes a bit trickier, since technically booting Linux >>> with -kernel you can use >>> any supported values as long as everything fits in the video RAM which is >>> why there isn't currently >>> a hard-coded list :) >>> >> >> We need the list of "supported values". I don't want to read the code or try >> values combination >> until it works. >> >> In a perfect world, I would like to be able to use any value I want with >> "-kernel". >> >> For instance I was using "-g 1200x800x24" and it was working fine. >> >> Now I have: >> >> qemu-system-m68k: unknown display mode: width 1200, height 800, depth 24 >> >> If it's not possible (because the original hardware cannot provide it, and >> we don't know the base >> address or things like that), we don't need the list of the display id, but >> the list of available >> modes: (width,height,depth). >> >> Rougly, something like: >> >> qemu-system-m68k: unknown display mode: width 1200, height 800, depth 24 >> Available modes: >> 1152x870x8 >> 1152x870x4 >> 1152x870x2 >> 1152x870x1 >> 800x600x24 >> 800x600x8 >> 800x600x4 >> 800x600x2 >> 800x600x1 >> 640x480x24 >> 640x480x8 >> 640x480x4 >> 640x480x2 >> 640x480x1 >> >> diff --git a/hw/display/macfb.c b/hw/display/macfb.c >> index 5b8812e9e7d8..4b352eb89c3f 100644 >> --- a/hw/display/macfb.c >> +++ b/hw/display/macfb.c >> @@ -438,6 +438,26 @@ static MacFbMode *macfb_find_mode(MacfbDisplayType >> display_type, >> return NULL; >> } >> >> +static gchar *macfb_mode_list(void) >> +{ >> + gchar *list = NULL; >> + gchar *mode; >> + MacFbMode *macfb_mode; >> + int i; >> + >> + for (i = 0; i < ARRAY_SIZE(macfb_mode_table); i++) { >> + macfb_mode = &macfb_mode_table[i]; >> + >> + mode = g_strdup_printf(" %dx%dx%d\n", macfb_mode->width, >> + macfb_mode->height, macfb_mode->depth); >> + list = g_strconcat(mode, list, NULL); >> + g_free(mode); >> + } >> + >> + return list; >> +} >> + >> + >> static void macfb_update_display(void *opaque) >> { >> MacfbState *s = opaque; >> @@ -620,8 +640,13 @@ static bool macfb_common_realize(DeviceState *dev, >> MacfbState *s, Error **errp) >> >> s->mode = macfb_find_mode(s->type, s->width, s->height, s->depth); >> if (!s->mode) { >> + gchar *list; >> error_setg(errp, "unknown display mode: width %d, height %d, depth >> %d", >> s->width, s->height, s->depth); >> + list = macfb_mode_list(); >> + error_append_hint(errp, "Available modes:\n%s", list); >> + g_free(list); >> + >> return false; >> } > > Hi Laurent, > > Thanks for the example - I can certainly squash this into patch 8.
yes, please. > As for allowing extra resolutions via -kernel, since the check is being done > in > macfb_common_realize() then it would be possible to add a qdev property that > only gets set when > -kernel is used on the command line which bypasses the mode check if you > prefer? > I think it can wait and be done by a patch later. For the moment we can focus on MacOS. > I'm not sure that your existing example of "-g 1200x800x24" (or indeed any > resolution with 24-bit > depth) with -kernel will still work after this patchset given that the 24-bit > encoding scheme has > changed. Presumably this would also need a corresponding change in the > bootinfo/kernel framebuffer/X > configuration somewhere? The kernel framebuffer should be easy to fix, if needed, normally we pass the needed information via the bootinfo structure. My X configuration is broken for a while. With debian/sid I've never been able to start X (even on a real q800, I think), and with debian/etch we need a special kernel as the ADB stack has been broken with old kernel. I was not able to start X for a while now... And GNOME desktop is not available on debian/sid m68k (some packages are missing). Perhaps I should try Xfce. So, don't worry about that... Thanks, Laurent