From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id B7BBFC25B4E for ; Tue, 24 Jan 2023 08:19:29 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id B0C8F8539D; Tue, 24 Jan 2023 09:19:26 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=linaro.org header.i=@linaro.org header.b="r4nrCRyy"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 9B565855A8; Tue, 24 Jan 2023 09:19:24 +0100 (CET) Received: from mail-ej1-x634.google.com (mail-ej1-x634.google.com [IPv6:2a00:1450:4864:20::634]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 4DB4F82F69 for ; Tue, 24 Jan 2023 09:19:21 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=ilias.apalodimas@linaro.org Received: by mail-ej1-x634.google.com with SMTP id ss4so36783183ejb.11 for ; Tue, 24 Jan 2023 00:19:21 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=+XZnwCTJPDTplcCGqBntfVmdsfGgGPoJVALGKUj28pQ=; b=r4nrCRyy1ugbozvigVyDlTaZaKS6XULZKfCPyfNwICt76TWnqPdUdJ6ICZMnLVmGec Tdd3HE9L4tqrDrckSiIi9yQAKy9m5kfR4gqS7pF/pgjuEECPcviu7xFydF2oqkPmwTKU hjZOFtUYXeihdNR2DSOa5EGJoCmjV4kOzyRKw/kmU7YoflPWFFkaViXqZpVjPRKTeVFF qk0aQFPCFgn+CHENFe3ya2Nvu/Wdu4/9h1sXI1JgHOr3g/hNNzRto88SeG4Lrc/0vB07 IFk6t993C3o9y3xop8ukaqJPk5pLisMY76YegiFqU36akf+Axc5Yrexxo9p4pf6x5Bb1 EoQg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=+XZnwCTJPDTplcCGqBntfVmdsfGgGPoJVALGKUj28pQ=; b=kzl3w4zjSEJ0r/igYakmzcROUJEycN+rx2SwO3oa0Aj76aIQhJZazZhnvjgkRIfnqe yxF8wABWJ5SG+VpPEvyZr+Uhr1aY8MuxCYDNfsFRT3VWHbgQuoXG4oVwyVbpYd4XvKFu ScSFfyYkfFEntNJX3lzZW5INuZxqvK6nukqBK5qb2ZxuC5i+BOvAWkrwlJLvaQw2Z994 6s2HZRuF3RHMj09erSTkgSx3ijWHl04d6i/6M3HtRHXnuBnIzRwVKerVed2Y8rwoCenM /Gycbpvjy/ynNb+qU4q62h/TGHBpBlC5COpi2FRO0XcB+WQfrVwAF1xViresXedUkKde jHXw== X-Gm-Message-State: AFqh2krin30cjVVLPS7dmdYYK62AmusIt1cgVSa4AbR/VOUU0pc+e76C Z7mkBLX9dqwSw73gCXkzBowgkQ== X-Google-Smtp-Source: AMrXdXtJNm1qB91xloLwvyCCco7i/RYBRxuuUf5SQvWGXnR7J1Kq9clXA0XhG5yij8JGeZ/D964/1Q== X-Received: by 2002:a17:907:6c16:b0:84d:4493:c83f with SMTP id rl22-20020a1709076c1600b0084d4493c83fmr32228779ejc.6.1674548360784; Tue, 24 Jan 2023 00:19:20 -0800 (PST) Received: from hera (ppp079167090036.access.hol.gr. [79.167.90.36]) by smtp.gmail.com with ESMTPSA id fx18-20020a170906b75200b0084d35ffbc20sm562606ejb.68.2023.01.24.00.19.19 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 24 Jan 2023 00:19:20 -0800 (PST) Date: Tue, 24 Jan 2023 10:19:18 +0200 From: Ilias Apalodimas To: Masahisa Kojima Cc: u-boot@lists.denx.de, Heinrich Schuchardt Subject: Re: [PATCH v5 3/4] eficonfig: add vertical scroll support Message-ID: References: <20230124065616.25559-1-masahisa.kojima@linaro.org> <20230124065616.25559-4-masahisa.kojima@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230124065616.25559-4-masahisa.kojima@linaro.org> X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.6 at phobos.denx.de X-Virus-Status: Clean On Tue, Jan 24, 2023 at 03:56:15PM +0900, Masahisa Kojima wrote: > The current eficonfig menu does not support vertical scroll, > so it can not display the menu entries greater than > the console row size. > > This commit add the vertial scroll support. > The console size is retrieved by > SIMPLE_TEXT_OUTPUT_PROTOCOL.QueryMode() service, then > calculates the row size for menu entry by subtracting > menu header and description row size from the console row size. > "start" and "end" are added in the efimenu structure. > "start" keeps the menu entry index at the top, "end" keeps > the bottom menu entry index. item_data_print() menu function > only draws the menu entry between "start" and "end". > > This commit also fixes the issue that "Save" and "Quit" > entries can be moved by BKEY_PLUS in change boot order menu. > > Signed-off-by: Masahisa Kojima > --- > Changes in v5: > - create common function to update efi_menu active, start and end > - fix the issue that "Save" and "Quit" can be moved by BKEY_PLUS press > > Changes in v3: > - modify "reverse" local variable type to bool > > Changes in v2: > - add comment when the user key press is not valid > - add const qualifier to eficonfig_change_boot_order_desc > > cmd/eficonfig.c | 92 ++++++++++++++++++++++++++++++++++++-------- > include/efi_config.h | 4 ++ > include/efi_loader.h | 1 + > 3 files changed, 80 insertions(+), 17 deletions(-) > > diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c > index 01bd1b05bc..47c04cf536 100644 > --- a/cmd/eficonfig.c > +++ b/cmd/eficonfig.c > @@ -30,8 +30,13 @@ static const char *eficonfig_change_boot_order_desc = > " Press SPACE to activate or deactivate the entry\n" > " Select [Save] to complete, ESC/CTRL+C to quit"; > > +static struct efi_simple_text_output_protocol *cout; > +static int avail_row; > + > #define EFICONFIG_DESCRIPTION_MAX 32 > #define EFICONFIG_OPTIONAL_DATA_MAX 64 > +#define EFICONFIG_MENU_HEADER_ROW_NUM 3 > +#define EFICONFIG_MENU_DESC_ROW_NUM 5 > > /** > * struct eficonfig_filepath_info - structure to be used to store file path > @@ -122,6 +127,30 @@ struct eficonfig_save_boot_order_data { > bool selected; > }; > > +/** > + * struct eficonfig_menu_adjust - update start and end entry index > + * > + * @efi_menu: pointer to efimenu structure > + * @add: flag to add or substract the index > + */ > +static void eficonfig_menu_adjust(struct efimenu *efi_menu, bool add) > +{ > + if (add) > + ++efi_menu->active; > + else > + --efi_menu->active; > + > + if (add && efi_menu->end < efi_menu->active) { > + efi_menu->start++; > + efi_menu->end++; > + } else if (!add && efi_menu->start > efi_menu->active) { > + efi_menu->start--; > + efi_menu->end--; > + } > +} > +#define eficonfig_menu_up(_a) eficonfig_menu_adjust(_a, false) > +#define eficonfig_menu_down(_a) eficonfig_menu_adjust(_a, true) > + > /** > * eficonfig_print_msg() - print message > * > @@ -157,18 +186,16 @@ void eficonfig_print_entry(void *data) > struct eficonfig_entry *entry = data; > bool reverse = (entry->efi_menu->active == entry->num); > > - /* TODO: support scroll or page for many entries */ > + if (entry->efi_menu->start > entry->num || entry->efi_menu->end < entry->num) > + return; > > - /* > - * Move cursor to line where the entry will be drawn (entry->num) > - * First 3 lines(menu header) + 1 empty line > - */ > - printf(ANSI_CURSOR_POSITION, entry->num + 4, 7); > + printf(ANSI_CURSOR_POSITION, (entry->num - entry->efi_menu->start) + > + EFICONFIG_MENU_HEADER_ROW_NUM + 1, 7); > > if (reverse) > puts(ANSI_COLOR_REVERSE); > > - printf("%s", entry->title); > + printf(ANSI_CLEAR_LINE "%s", entry->title); > > if (reverse) > puts(ANSI_COLOR_RESET); > @@ -191,8 +218,8 @@ void eficonfig_display_statusline(struct menu *m) > ANSI_CURSOR_POSITION ANSI_CLEAR_LINE ANSI_CURSOR_POSITION > "%s" > ANSI_CLEAR_LINE_TO_END, > - 1, 1, entry->efi_menu->menu_header, entry->efi_menu->count + 5, 1, > - entry->efi_menu->count + 6, 1, entry->efi_menu->menu_desc); > + 1, 1, entry->efi_menu->menu_header, avail_row + 4, 1, > + avail_row + 5, 1, entry->efi_menu->menu_desc); > } > > /** > @@ -217,12 +244,14 @@ char *eficonfig_choice_entry(void *data) > switch (key) { > case BKEY_UP: > if (efi_menu->active > 0) > - --efi_menu->active; > + eficonfig_menu_up(efi_menu); > + > /* no menu key selected, regenerate menu */ > return NULL; > case BKEY_DOWN: > if (efi_menu->active < efi_menu->count - 1) > - ++efi_menu->active; > + eficonfig_menu_down(efi_menu); > + > /* no menu key selected, regenerate menu */ > return NULL; > case BKEY_SELECT: > @@ -402,6 +431,8 @@ efi_status_t eficonfig_process_common(struct efimenu *efi_menu, > > efi_menu->delay = -1; > efi_menu->active = 0; > + efi_menu->start = 0; > + efi_menu->end = avail_row - 1; > > if (menu_header) { > efi_menu->menu_header = strdup(menu_header); > @@ -1868,7 +1899,11 @@ static void eficonfig_print_change_boot_order_entry(void *data) > struct eficonfig_entry *entry = data; > bool reverse = (entry->efi_menu->active == entry->num); > > - printf(ANSI_CURSOR_POSITION, entry->num + 4, 7); > + if (entry->efi_menu->start > entry->num || entry->efi_menu->end < entry->num) > + return; > + > + printf(ANSI_CURSOR_POSITION ANSI_CLEAR_LINE, > + (entry->num - entry->efi_menu->start) + EFICONFIG_MENU_HEADER_ROW_NUM + 1, 7); > > if (reverse) > puts(ANSI_COLOR_REVERSE); > @@ -1906,7 +1941,8 @@ char *eficonfig_choice_change_boot_order(void *data) > > switch (key) { > case BKEY_PLUS: > - if (efi_menu->active > 0) { > + if (efi_menu->active > 0 && > + efi_menu->active < efi_menu->count - 2) { > list_for_each_safe(pos, n, &efi_menu->list) { > entry = list_entry(pos, struct eficonfig_entry, list); > if (entry->num == efi_menu->active) > @@ -1917,11 +1953,14 @@ char *eficonfig_choice_change_boot_order(void *data) > tmp->num++; > list_del(&tmp->list); > list_add(&tmp->list, &entry->list); > + > + eficonfig_menu_up(efi_menu); > } > - fallthrough; > + return NULL; > case BKEY_UP: > if (efi_menu->active > 0) > - --efi_menu->active; > + eficonfig_menu_up(efi_menu); > + > return NULL; > case BKEY_MINUS: > if (efi_menu->active < efi_menu->count - 3) { > @@ -1936,12 +1975,13 @@ char *eficonfig_choice_change_boot_order(void *data) > list_del(&entry->list); > list_add(&entry->list, &tmp->list); > > - ++efi_menu->active; > + eficonfig_menu_down(efi_menu); > } > return NULL; > case BKEY_DOWN: > if (efi_menu->active < efi_menu->count - 1) > - ++efi_menu->active; > + eficonfig_menu_down(efi_menu); > + > return NULL; > case BKEY_SELECT: > /* "Save" */ > @@ -2590,6 +2630,7 @@ static efi_status_t eficonfig_init(void) > efi_status_t ret = EFI_SUCCESS; > static bool init; > struct efi_handler *handler; > + unsigned long columns, rows; > > if (!init) { > ret = efi_search_protocol(efi_root, &efi_guid_text_input_protocol, &handler); > @@ -2600,6 +2641,23 @@ static efi_status_t eficonfig_init(void) > EFI_OPEN_PROTOCOL_GET_PROTOCOL); > if (ret != EFI_SUCCESS) > return ret; > + ret = efi_search_protocol(efi_root, &efi_guid_text_output_protocol, &handler); > + if (ret != EFI_SUCCESS) > + return ret; > + > + ret = efi_protocol_open(handler, (void **)&cout, efi_root, NULL, > + EFI_OPEN_PROTOCOL_GET_PROTOCOL); > + if (ret != EFI_SUCCESS) > + return ret; > + > + cout->query_mode(cout, cout->mode->mode, &columns, &rows); > + avail_row = rows - (EFICONFIG_MENU_HEADER_ROW_NUM + > + EFICONFIG_MENU_DESC_ROW_NUM); > + if (avail_row <= 0) { > + eficonfig_print_msg("Console size is too small!"); > + return EFI_INVALID_PARAMETER; > + } > + /* TODO: Should we check the minimum column size? */ > } > > init = true; > diff --git a/include/efi_config.h b/include/efi_config.h > index cec5715f84..6a104e4b1d 100644 > --- a/include/efi_config.h > +++ b/include/efi_config.h > @@ -49,6 +49,8 @@ struct eficonfig_entry { > * @menu_header: menu header string > * @menu_desc: menu description string > * @list: menu entry list structure > + * @start: top menu index to draw > + * @end: bottom menu index to draw > */ > struct efimenu { > int delay; > @@ -57,6 +59,8 @@ struct efimenu { > char *menu_header; > const char *menu_desc; > struct list_head list; > + int start; > + int end; > }; > > /** > diff --git a/include/efi_loader.h b/include/efi_loader.h > index f9e427f090..4560b0d04c 100644 > --- a/include/efi_loader.h > +++ b/include/efi_loader.h > @@ -328,6 +328,7 @@ extern const efi_guid_t efi_esrt_guid; > extern const efi_guid_t smbios_guid; > /*GUID of console */ > extern const efi_guid_t efi_guid_text_input_protocol; > +extern const efi_guid_t efi_guid_text_output_protocol; > > extern char __efi_runtime_start[], __efi_runtime_stop[]; > extern char __efi_runtime_rel_start[], __efi_runtime_rel_stop[]; > -- > 2.17.1 > Reviewed-by: Ilias Apalodimas