[PATCH 1/2] i386/pc386/VESA framebuffer driver: modified and extended initialization options
Jan Dolezal
dolezj21 at fel.cvut.cz
Tue May 19 17:11:13 UTC 2015
Hello Gedare,
thank you for the swift response..
On 19.5.2015 17:38, Gedare Bloom wrote:
> On Tue, May 19, 2015 at 10:34 AM, Jan Dolezal <dolezj21 at fel.cvut.cz> wrote:
>> driver is not initialized by default
>> initialization is possible through multiboot command line option or
>> through string variable set in user's module allowing the driver
>> to evaluate this variable after the two modules are linked together
>> ---
>> c/src/lib/libbsp/i386/pc386/console/fb_vesa_rm.c | 152 +++++++++++++++++++----
>> 1 file changed, 127 insertions(+), 25 deletions(-)
>>
>> diff --git a/c/src/lib/libbsp/i386/pc386/console/fb_vesa_rm.c b/c/src/lib/libbsp/i386/pc386/console/fb_vesa_rm.c
>> index d6fd174..5c4d8bf 100644
>> --- a/c/src/lib/libbsp/i386/pc386/console/fb_vesa_rm.c
>> +++ b/c/src/lib/libbsp/i386/pc386/console/fb_vesa_rm.c
>> @@ -19,8 +19,14 @@
>> *
>> * Driver reads parameter from multiboot command line to setup video:
>> * "--video=<resX>x<resY>[-<bpp>]"
>> - * If cmdline parameter is not specified an attempt for obtaining
>> - * resolution from display attached is made.
>> + * "--video=auto" - try EDID to find mode that fits the display attached best
>> + * "--video=none" / "--video=off" - do not initialize the driver
>> + * If cmdline parameter is not specified the rtems_console_vbe_default
>> + * variable content is tested (see doc below).
>> + * Command line option has higher priority. rtems_console_vbe_default is probed
>> + * only if cmdline "--video=" is not specified at all.
>> + *
>> + * If neither of the above options is specified the driver is not initialized.
>> */
>>
>> /*
>> @@ -31,7 +37,7 @@
>> * found in the file LICENSE in this distribution or at
>> * http://www.rtems.org/license/LICENSE.
>> *
>> - * The code for rtems_buffer_* functions were greatly
>> + * The code for rtems_buffer_* functions was greatly
>> * inspired or coppied from:
>> * - RTEMS fb_cirrus.c - Alexandru-Sever Horin (alex.sever.h at gmail.com)
>> */
>> @@ -53,6 +59,22 @@
>> #define FB_VESA_NAME "FB_VESA_RM"
>>
>> /**
>> + * @brief Allows to enable initialization of this driver from an application
>> + * by setting the value of this variable to non null value in
>> + * user's module. The value of this variable will be then updated
>> + * when linked with application's object.
>> + *
>> + * Further if the value points to string in format:
>> + * "<resX>x<resY>[-<bpp>]"
>> + * "auto" - try EDID to find mode that fits the display attached best
>> + * "none" / "off" - do not initialize the driver
>> + * the given parameters are used if applicable.
>> + *
>> + * Command line option string has priority over this string.
>> + */
>> +const char * const rtems_console_vbe_default;
>> +
> If this variable is not exported elsewhere, it should be made 'static'.>
The point is to make this variable available for exporting somewhere
else, specifically the application that wishes to use the driver, sets
this variable to the string containing the resolution requested to be
set. When the application is linked together with this module,
rtems_console_vbe_default points to that string and the driver can
initialize itself using the string from the application.
>
>> +/**
>> * @brief Initializes VBE framebuffer during bootup.
>> *
>> * utilizes switches to real mode interrupts and therefore must be
>> @@ -212,6 +234,14 @@ typedef struct {
>> uint8_t bpp;
>> } Mode_params;
>>
>> +typedef enum {
>> + NO_SUITABLE_MODE = -1,
>> + BAD_FORMAT = -2,
>> + AUTO_SELECT = -3,
>> + DONT_INIT = -4,
>> + NO_MODE_REQ = -5,
>> +} mode_err_ret_val;
>> +
>
> Can the valid and invalid modes be in the same enum, and that can be
> used as the type too?>
Valid modes are obtained from graphics card and may be manufacturer or
card dependant. Some of the modes specified by the VESA standard are
defined as macros here:
git.rtems.org/rtems/tree/c/src/lib/libbsp/i386/pc386/include/vbe3.h
I added the enum (especially its values) to make the code better readable.
>
>> /**
>> * @brief Find mode by resolution in the given list of modes
>> *
>> @@ -249,37 +279,48 @@ static uint16_t find_mode_by_resolution(Mode_params *mode_list,
>> }
>> i++;
>> }
>> - return -1;
>> + return NO_SUITABLE_MODE;
>> }
>>
>> /**
>> - * @brief Find mode given within command line.
>> + * @brief Find mode given in string format.
>> *
>> - * Parse command line option "--video=" if available.
>> * expected format
>> - * --video=<resX>x<resY>[-<bpp>]
>> + * <resX>x<resY>[-<bpp>]
>> * numbers <resX>, <resY> and <bpp> are decadic
>> *
>> * @param[in] mode_list list of modes to be searched
>> * @param[in] list_length number of modes in the list
>> + * @param[in] video_string string to be parsed
>> * @retval video mode number to be set
>> - * @retval -1 on parsing error or when no suitable mode found
>> + * @retval -1 no suitable mode found
>> + * @retval -2 bad format of the video_string
>> + * @retval -3 automatic mode selection requested
>> + * @retval -4 request to not initialize graphics
>> + * @retval -5 no mode requested/empty video string
>> */
>> -static uint16_t find_mode_using_cmdline(Mode_params *mode_list,
>> - uint8_t list_length)
>> +static uint16_t find_mode_from_string(Mode_params *mode_list,
>> + uint8_t list_length,
>> + const char *video_string)
> Is it ok to cast a negative enum value into a uint16_t?>
I don't quite understand, what do you mean by ok.
I use negative numbers to get values from the end of uint16_t range
which should not overlap with possible valid mode numbers. Return value
is used to inform the user why the driver was not initialized. The deciding
switch cases (below) then have explicit casts into a uint16_t, so it
is compared correctly.
>
>> {
>> const char* opt;
>> Mode_params cmdline_mode;
>> char* endptr;
>> - cmdline_mode.bpp = 0;
>> - opt = bsp_cmdline_arg("--video=");
>> + cmdline_mode.bpp = 16; /* default bpp */
>> + opt = video_string;
>> if (opt)
>> {
>> - opt += sizeof("--video=")-1;
>> + if (strncmp(opt, "auto", 4) == 0)
>> + return AUTO_SELECT;
>> + if (strncmp(opt, "none", 4) == 0 ||
>> + strncmp(opt, "off", 3) == 0)
>> + {
>> + return DONT_INIT;
>> + }
>> cmdline_mode.resX = strtol(opt, &endptr, 10);
>> if (*endptr != 'x')
>> {
>> - return -1;
>> + return BAD_FORMAT;
>> }
>> opt = endptr+1;
>> cmdline_mode.resY = strtol(opt, &endptr, 10);
>> @@ -294,21 +335,50 @@ static uint16_t find_mode_using_cmdline(Mode_params *mode_list,
>> cmdline_mode.bpp = strtol(opt, &endptr, 10);
>> if (*endptr != ' ')
>> {
>> - return -1;
>> + return BAD_FORMAT;
>> }
>> }
>> case ' ':
>> case 0:
>> break;
>> default:
>> - return -1;
>> + return BAD_FORMAT;
>> }
>>
>> - if (find_mode_by_resolution(mode_list, list_length, &cmdline_mode) !=
>> - (uint16_t)-1)
>> - return cmdline_mode.mode_number;
>> + return find_mode_by_resolution(mode_list, list_length, &cmdline_mode);
>> }
>> - return -1;
>> + return NO_MODE_REQ;
>> +}
>> +
>> +
>> +/**
>> + * @brief Find mode given within command line.
>> + *
>> + * Parse command line option "--video=" if available.
>> + * expected format
>> + * --video=<resX>x<resY>[-<bpp>]
>> + * numbers <resX>, <resY> and <bpp> are decadic
>> + *
>> + * @param[in] mode_list list of modes to be searched
>> + * @param[in] list_length number of modes in the list
>> + * @retval video mode number to be set
>> + * @retval -1 no suitable mode found
>> + * @retval -2 bad format of the video_string
>> + * @retval -3 automatic mode selection requested
>> + * @retval -4 request to not initialize graphics
>> + * @retval -5 no mode requested/empty video string
>> + */
>> +static uint16_t find_mode_using_cmdline(Mode_params *mode_list,
>> + uint8_t list_length)
>> +{
>> + const char* opt;
>> + opt = bsp_cmdline_arg("--video=");
>> + if (opt)
>> + {
>> + opt += sizeof("--video=")-1;
>> + return find_mode_from_string(mode_list, list_length, opt);
>> + }
>> + return NO_MODE_REQ;
>> }
>>
>> /**
>> @@ -569,6 +639,7 @@ void vesa_realmode_bootup_init(void)
>> uint16_t *modeNOPtr = (uint16_t*)
>> i386_Real_to_physical(*(vmpSegOff+1), *vmpSegOff);
>> uint16_t iterator = 0;
>> +
>> if (*(uint16_t*)vib->VideoModePtr == VBE_STUB_VideoModeList)
>> {
>> printk(FB_VESA_NAME " VBE Core not implemented!\n");
>> @@ -618,6 +689,40 @@ void vesa_realmode_bootup_init(void)
>> sorted_mode_params[nextFilteredMode].mode_number = 0;
>>
>> uint8_t number_of_modes = nextFilteredMode;
>> +
>> + /* first search for video argument in multiboot options */
>> + vbe_used_mode = find_mode_using_cmdline(sorted_mode_params,
>> + number_of_modes);
>> +
>> + if (vbe_used_mode == (uint16_t) NO_MODE_REQ) {
>> + vbe_used_mode = find_mode_from_string(sorted_mode_params,
>> + number_of_modes, rtems_console_vbe_default);
>> + if (vbe_used_mode != (uint16_t) NO_MODE_REQ) {
>> + printk(FB_VESA_NAME " using application option to select"
>> + " video mode\n");
>> + }
>> + }
>> + else
>> + {
>> + printk(FB_VESA_NAME " using command line option '--video='"
>> + "to select video mode\n");
>> + }
>> +
>> + switch (vbe_used_mode) {
>> + case (uint16_t) NO_SUITABLE_MODE:
>> + printk(FB_VESA_NAME " requested mode not found\n");
>> + return;
>> + case (uint16_t) BAD_FORMAT:
>> + printk(FB_VESA_NAME " bad format of video requested\n");
>> + return;
>> + case (uint16_t) DONT_INIT:
>> + printk(FB_VESA_NAME " selected not to initialize graphics\n");
>> + return;
>> + case (uint16_t) NO_MODE_REQ:
>> + printk(FB_VESA_NAME " not initialized, no video selected\n");
>> + return;
>> + }
>> +
>> /* sort filtered modes */
>> Mode_params modeXchgPlace;
>> iterator = 0;
>> @@ -657,12 +762,9 @@ void vesa_realmode_bootup_init(void)
>> iterator++;
>> }
>>
>> - /* first search for video argument in multiboot options */
>> - vbe_used_mode = find_mode_using_cmdline(sorted_mode_params,
>> - number_of_modes);
>> - if (vbe_used_mode == (uint16_t)-1)
>> + if (vbe_used_mode == (uint16_t)AUTO_SELECT)
>> {
>> - printk(FB_VESA_NAME " video on command line not provided"
>> + printk(FB_VESA_NAME " auto video mode selected"
>> "\n\ttrying EDID ...\n");
>> /* second search monitor for good resolution */
>> vbe_used_mode = find_mode_using_EDID(sorted_mode_params,
>> --
>> 1.9.1
>>
>> _______________________________________________
>> devel mailing list
>> devel at rtems.org
>> http://lists.rtems.org/mailman/listinfo/devel
More information about the devel
mailing list