linux-efi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Arvind Sankar <nivedita@alum.mit.edu>
To: Ard Biesheuvel <ardb@kernel.org>
Cc: Arvind Sankar <nivedita@alum.mit.edu>,
	Hans de Goede <hdegoede@redhat.com>,
	linux-efi <linux-efi@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 00/14] efi/gop: Refactoring + mode-setting feature
Date: Wed, 25 Mar 2020 18:10:08 -0400	[thread overview]
Message-ID: <20200325221007.GA290267@rani.riverdale.lan> (raw)
In-Reply-To: <CAKv+Gu8-iK-FQrgCY6YGXyg155chMPJQZeQr-i_xQbqoQ57F0g@mail.gmail.com>

On Wed, Mar 25, 2020 at 05:41:43PM +0100, Ard Biesheuvel wrote:
> On Fri, 20 Mar 2020 at 03:00, Arvind Sankar <nivedita@alum.mit.edu> wrote:
> >
> > This series is against tip:efi/core.
> >
> > Patches 1-9 are small cleanups and refactoring of the code in
> > libstub/gop.c.
> >
> > The rest of the patches add the ability to use a command-line option to
> > switch the gop's display mode.
> >
> > The options supported are:
> > video=efifb:mode=n
> >         Choose a specific mode number
> > video=efifb:<xres>x<yres>[-(rgb|bgr|<bpp>)]
> >         Specify mode by resolution and optionally color depth
> > video=efifb:auto
> >         Let the EFI stub choose the highest resolution mode available.
> >
> > The mode-setting additions increase code size of gop.o by about 3k on
> > x86-64 with EFI_MIXED enabled.
> >
> > Changes in v2 (HT lkp@intel.com):
> > - Fix __efistub_global attribute to be after the variable.
> >   (NB: bunch of other places should ideally be fixed, those I guess
> >   don't matter as they are scalars?)
> > - Silence -Wmaybe-uninitialized warning in set_mode function.
> >
> 
> These look good to me. The only question I have is whether it would be
> possible to use the existing next_arg() and parse_option_str()
> functions to replace some of the open code parsing that goes on in
> patches 11 - 14.
> 

I don't think so -- next_arg is for parsing space-separated param=value
pairs, so efi_parse_options can use it, but it doesn't work for the
comma-separated options we'll have within the value.

parse_option_str would only work for the "auto" option, but it scans the
entire option string and just returns whether it was there or not, so it
wouldn't be too useful either, since we have to check for the other
possibilities anyway.

It would be nice to have a more generic library for cmdline parsing,
there are a lot of places that have to open-code option parsing like
this.

There's one thing I noticed while working at this, btw. The Makefile
specifies -ffreestanding, but at least x86 builds without having to
specify that. With -ffreestanding, the compiler doesn't optimize string
functions -- strlen(string literal) into a compile-time constant, for
eg. A couple hundred bytes or so can be saved by removing that option,
if it also works for ARM.

  reply	other threads:[~2020-03-25 22:10 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-19 19:28 [PATCH 00/14] efi/gop: Refactoring + mode-setting feature Arvind Sankar
2020-03-19 19:28 ` [PATCH 01/14] efi/gop: Remove redundant current_fb_base Arvind Sankar
2020-03-19 19:28 ` [PATCH 02/14] efi/gop: Move check for framebuffer before con_out Arvind Sankar
2020-03-19 19:28 ` [PATCH 03/14] efi/gop: Get mode information outside the loop Arvind Sankar
2020-03-19 19:28 ` [PATCH 04/14] efi/gop: Factor out locating the gop into a function Arvind Sankar
2020-03-19 19:28 ` [PATCH 05/14] efi/gop: Slightly re-arrange logic of find_gop Arvind Sankar
2020-03-19 19:28 ` [PATCH 06/14] efi/gop: Move variable declarations into loop block Arvind Sankar
2020-03-19 19:28 ` [PATCH 07/14] efi/gop: Use helper macros for populating lfb_base Arvind Sankar
2020-03-19 19:28 ` [PATCH 08/14] efi/gop: Use helper macros for find_bits Arvind Sankar
2020-03-19 19:28 ` [PATCH 09/14] efi/gop: Remove unreachable code from setup_pixel_info Arvind Sankar
2020-03-19 19:28 ` [PATCH 10/14] efi/gop: Add prototypes for query_mode and set_mode Arvind Sankar
2020-03-19 19:28 ` [PATCH 11/14] efi/gop: Allow specifying mode number on command line Arvind Sankar
2020-03-19 22:53   ` kbuild test robot
2020-03-20  1:09   ` kbuild test robot
2020-03-20 14:36   ` Dan Carpenter
2020-03-20 17:51     ` Arvind Sankar
2020-03-19 19:28 ` [PATCH 12/14] efi/gop: Allow specifying mode by <xres>x<yres> Arvind Sankar
2020-03-19 19:28 ` [PATCH 13/14] efi/gop: Allow specifying depth as well as resolution Arvind Sankar
2020-03-19 19:28 ` [PATCH 14/14] efi/gop: Allow automatically choosing the best mode Arvind Sankar
2020-03-19 20:02 ` [PATCH 00/14] efi/gop: Refactoring + mode-setting feature Ard Biesheuvel
2020-03-20  2:00 ` [PATCH v2 " Arvind Sankar
2020-03-25 16:41   ` Ard Biesheuvel
2020-03-25 22:10     ` Arvind Sankar [this message]
2020-03-25 23:36       ` Ard Biesheuvel
2020-03-26 10:41         ` Ard Biesheuvel
2020-03-28 16:06           ` efi/gop: skip cur_mode in choose_mode_auto Arvind Sankar
2020-03-28 16:06             ` [PATCH v3] efi/gop: Allow automatically choosing the best mode Arvind Sankar
2020-03-29  7:47             ` efi/gop: skip cur_mode in choose_mode_auto Ard Biesheuvel
2020-03-26 23:56         ` [PATCH v2 00/14] efi/gop: Refactoring + mode-setting feature Arvind Sankar
2020-03-28 15:43           ` Ard Biesheuvel
2020-03-25 16:50   ` Hans de Goede
2020-03-25 22:13     ` Arvind Sankar
2020-03-20  2:00 ` [PATCH v2 01/14] efi/gop: Remove redundant current_fb_base Arvind Sankar
2020-03-20  2:00 ` [PATCH v2 02/14] efi/gop: Move check for framebuffer before con_out Arvind Sankar
2020-03-20  2:00 ` [PATCH v2 03/14] efi/gop: Get mode information outside the loop Arvind Sankar
2020-03-20  2:00 ` [PATCH v2 04/14] efi/gop: Factor out locating the gop into a function Arvind Sankar
2020-03-20  2:00 ` [PATCH v2 05/14] efi/gop: Slightly re-arrange logic of find_gop Arvind Sankar
2020-03-20  2:00 ` [PATCH v2 06/14] efi/gop: Move variable declarations into loop block Arvind Sankar
2020-03-20  2:00 ` [PATCH v2 07/14] efi/gop: Use helper macros for populating lfb_base Arvind Sankar
2020-03-20  2:00 ` [PATCH v2 08/14] efi/gop: Use helper macros for find_bits Arvind Sankar
2020-03-20  2:00 ` [PATCH v2 09/14] efi/gop: Remove unreachable code from setup_pixel_info Arvind Sankar
2020-03-20  2:00 ` [PATCH v2 10/14] efi/gop: Add prototypes for query_mode and set_mode Arvind Sankar
2020-03-20  2:00 ` [PATCH v2 11/14] efi/gop: Allow specifying mode number on command line Arvind Sankar
2020-03-20  2:00 ` [PATCH v2 12/14] efi/gop: Allow specifying mode by <xres>x<yres> Arvind Sankar
2020-03-20  2:00 ` [PATCH v2 13/14] efi/gop: Allow specifying depth as well as resolution Arvind Sankar
2020-03-20  2:00 ` [PATCH v2 14/14] efi/gop: Allow automatically choosing the best mode Arvind Sankar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200325221007.GA290267@rani.riverdale.lan \
    --to=nivedita@alum.mit.edu \
    --cc=ardb@kernel.org \
    --cc=hdegoede@redhat.com \
    --cc=linux-efi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).