[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v2 1/3] efi: try to use the currently set GOP mode
On 31.03.2023 11:59, Roger Pau Monne wrote: > Modify efi_find_gop_mode() so that passing cols or rows as 0 is > interpreted as a request to attempt to keep the currently set mode, > and do so if the mode query for information is successful and the depth > is supported. > > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx> > --- > Changes since v1: > - Only update cols or rows if the value is 0. > - Leave depth alone. > --- > xen/common/efi/boot.c | 21 +++++++++++++++++++++ > 1 file changed, 21 insertions(+) While I realize that docs/misc/efi.pandoc currently doesn't describe what rows/cols being zero means, I think that file needs updating in the course of what you're doing. Irrespective of this I'm uncertain about the change in behavior: Presently both values being 0 means "find biggest resolution mode", in an attempt to have as much info on the screen at a time as possible (in particular in case of problems). I certainly appreciate the desire to have a way to say "keep the current mode", but I don't think this should alter behavior of what's presently considered valid config settings. > --- a/xen/common/efi/boot.c > +++ b/xen/common/efi/boot.c > @@ -930,6 +930,27 @@ static UINTN __init > efi_find_gop_mode(EFI_GRAPHICS_OUTPUT_PROTOCOL *gop, > UINTN gop_mode = ~0, info_size, size; > unsigned int i; > > + if ( (!cols || !rows) && gop->Mode->Mode < gop->Mode->MaxMode ) > + { > + /* If no (valid) resolution suggested, try to use the current mode. > */ > + status = gop->QueryMode(gop, gop->Mode->Mode, &info_size, > &mode_info); > + if ( EFI_ERROR(status) ) > + PrintErr(L"Invalid current graphics mode\r\n"); > + else if ( mode_info->PixelFormat < PixelBltOnly ) > + return gop->Mode->Mode; What if one of cols/rows was non-zero? You then wouldn't fulfill the request. "depth", if non-zero, is also entirely ignored. (We don't fulfill such a request right now either, but not doing so becomes more odd now imo. In fact right now in this case we leave the screen alone, if I'm not mistaken, so there already looks to be a way to achieve what you're after.) > + else > + { > + /* > + * Try to find a mode with the same resolution and a valid pixel > + * format. > + */ Is "valid" the right word here? I think you mean "usable for us" or some such? > + if ( !cols ) > + cols = mode_info->HorizontalResolution; > + if ( !rows ) > + rows = mode_info->VerticalResolution; > + } > + } Overall with the resulting behavior I'm not sure the title really describes what's being done. You "try" only in certain cases. Jan
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |