Re: [PATCH 2/2] "auto" keyword in gfxmode and gfxpayload specification

[Robert Millan <rmh@aybabtu.com>]

Please could you include a description on what this does?

Also, ChangeLog entry is missing (maybe this will suffice to explain).


On Sun, Aug 16, 2009 at 08:51:56PM +0200, Vladimir 'phcoder' Serbinenko wrote:
> -- 
> Regards
> Vladimir 'phcoder' Serbinenko
> 
> Personal git repository: http://repo.or.cz/w/grub2/phcoder.git

> diff --git a/commands/videotest.c b/commands/videotest.c
> index 07f61bd..07735cd 100644
> --- a/commands/videotest.c
> +++ b/commands/videotest.c
> @@ -30,10 +30,7 @@ grub_cmd_videotest (grub_command_t cmd __attribute__ ((unused)),
>                      int argc __attribute__ ((unused)),
>                      char **args __attribute__ ((unused)))
>  {
> -  if (grub_video_set_mode ("1024x768;800x600;640x480",
> -			   GRUB_VIDEO_MODE_TYPE_PURE_TEXT, 0) != GRUB_ERR_NONE)
> -    return grub_errno;
> -
> +  grub_err_t err;
>    grub_video_color_t color;
>    unsigned int x;
>    unsigned int y;
> @@ -50,6 +47,10 @@ grub_cmd_videotest (grub_command_t cmd __attribute__ ((unused)),
>    const char *str;
>    int texty;
>  
> +  err = grub_video_set_mode ("auto", GRUB_VIDEO_MODE_TYPE_PURE_TEXT, 0);
> +  if (err)
> +    return err;
> +
>    grub_video_get_viewport (&x, &y, &width, &height);
>  
>    grub_video_create_render_target (&text_layer, width, height,
> diff --git a/loader/i386/linux.c b/loader/i386/linux.c
> index c5802d8..1dea93d 100644
> --- a/loader/i386/linux.c
> +++ b/loader/i386/linux.c
> @@ -42,7 +42,7 @@
>     GRUB and Linux (saving boot time and visual glitches).  Official GRUB, OTOH,
>     needs to be conservative.  */
>  #ifdef GRUB_ASSUME_LINUX_HAS_FB_SUPPORT
> -#define DEFAULT_VIDEO_MODE "keep,1024x768,800x600,640x480"
> +#define DEFAULT_VIDEO_MODE "keep,auto"
>  #else
>  #define DEFAULT_VIDEO_MODE "text"
>  #endif
> diff --git a/loader/i386/pc/xnu.c b/loader/i386/pc/xnu.c
> index adca8c6..cc63744 100644
> --- a/loader/i386/pc/xnu.c
> +++ b/loader/i386/pc/xnu.c
> @@ -26,7 +26,7 @@
>  #define min(a,b) (((a) < (b)) ? (a) : (b))
>  #define max(a,b) (((a) > (b)) ? (a) : (b))
>  
> -#define DEFAULT_VIDEO_MODE "1024x768x32,800x600x32,640x480x32"
> +#define DEFAULT_VIDEO_MODE "auto"
>  
>  /* Setup video for xnu. */
>  grub_err_t
> diff --git a/term/gfxterm.c b/term/gfxterm.c
> index e6c6746..57c51cf 100644
> --- a/term/gfxterm.c
> +++ b/term/gfxterm.c
> @@ -27,7 +27,7 @@
>  #include <grub/bitmap.h>
>  #include <grub/command.h>
>  
> -#define DEFAULT_VIDEO_MODE "1024x768,800x600,640x480"
> +#define DEFAULT_VIDEO_MODE "auto"
>  #define DEFAULT_BORDER_WIDTH	10
>  
>  #define DEFAULT_STANDARD_COLOR  0x07
> diff --git a/video/i386/pc/vbe.c b/video/i386/pc/vbe.c
> index c4878f6..b160911 100644
> --- a/video/i386/pc/vbe.c
> +++ b/video/i386/pc/vbe.c
> @@ -429,8 +429,8 @@ grub_video_vbe_setup (unsigned int width, unsigned int height,
>          /* Not compatible memory model.  */
>          continue;
>  
> -      if ((mode_info.x_resolution != width)
> -          || (mode_info.y_resolution != height))
> +      if (((mode_info.x_resolution != width)
> +	   || (mode_info.y_resolution != height)) && width != 0 && height != 0)
>          /* Non matching resolution.  */
>          continue;
>  
> @@ -457,9 +457,12 @@ grub_video_vbe_setup (unsigned int width, unsigned int height,
>        if ((depth != 0) && (mode_info.bits_per_pixel != depth))
>          continue;
>  
> -      /* Select mode with most number of bits per pixel.  */
> +      /* Select mode with most of "volume" (size of framebuffer in bits).  */
>        if (best_mode != 0)
> -        if (mode_info.bits_per_pixel < best_mode_info.bits_per_pixel)
> +        if ((grub_uint64_t) mode_info.bits_per_pixel * mode_info.x_resolution
> +	    * mode_info.y_resolution
> +	    < (grub_uint64_t) best_mode_info.bits_per_pixel
> +	    * best_mode_info.x_resolution * best_mode_info.y_resolution)
>            continue;
>  
>        /* Save so far best mode information for later use.  */
> diff --git a/video/video.c b/video/video.c
> index 1c5a35d..ab6627d 100644
> --- a/video/video.c
> +++ b/video/video.c
> @@ -397,6 +397,70 @@ grub_video_get_active_render_target (struct grub_video_render_target **target)
>    return grub_video_adapter_active->get_active_render_target (target);
>  }
>  
> +/* Parse <width>x<height>[x<depth>]*/
> +static grub_err_t
> +parse_modespec (const char *current_mode, int *width, int *height, int *depth)
> +{
> +  const char *value;
> +  const char *param = current_mode;
> +
> +  *width = *height = *depth = -1;
> +
> +  if (grub_strcmp (param, "auto") == 0)
> +    {
> +      *width = *height = 0;
> +      return GRUB_ERR_NONE;
> +    }
> +
> +  /* Find width value.  */
> +  value = param;
> +  param = grub_strchr(param, 'x');
> +  if (param == NULL)
> +    return grub_error (GRUB_ERR_BAD_ARGUMENT,
> +		       "Invalid mode: %s\n",
> +		       current_mode);
> +
> +  param++;
> +  
> +  *width = grub_strtoul (value, 0, 0);
> +  if (grub_errno != GRUB_ERR_NONE)
> +      return grub_error (GRUB_ERR_BAD_ARGUMENT,
> +			 "Invalid mode: %s\n",
> +			 current_mode);
> +  
> +  /* Find height value.  */
> +  value = param;
> +  param = grub_strchr(param, 'x');
> +  if (param == NULL)
> +    {
> +      *height = grub_strtoul (value, 0, 0);
> +      if (grub_errno != GRUB_ERR_NONE)
> +	return grub_error (GRUB_ERR_BAD_ARGUMENT,
> +			   "Invalid mode: %s\n",
> +			   current_mode);
> +    }
> +  else
> +    {
> +      /* We have optional color depth value.  */
> +      param++;
> +      
> +      *height = grub_strtoul (value, 0, 0);
> +      if (grub_errno != GRUB_ERR_NONE)
> +	return grub_error (GRUB_ERR_BAD_ARGUMENT,
> +			   "Invalid mode: %s\n",
> +			   current_mode);
> +      
> +      /* Convert color depth value.  */
> +      value = param;
> +      *depth = grub_strtoul (value, 0, 0);
> +      if (grub_errno != GRUB_ERR_NONE)
> +	return grub_error (GRUB_ERR_BAD_ARGUMENT,
> +			   "Invalid mode: %s\n",
> +			   current_mode);
> +    }
> +  return GRUB_ERR_NONE;
> +}
> +
>  grub_err_t
>  grub_video_set_mode (const char *modestring,
>  		     unsigned int modemask,
> @@ -405,8 +469,6 @@ grub_video_set_mode (const char *modestring,
>    char *tmp;
>    char *next_mode;
>    char *current_mode;
> -  char *param;
> -  char *value;
>    char *modevar;
>  
>    modevalue &= modemask;
> @@ -481,6 +543,7 @@ grub_video_set_mode (const char *modestring,
>        int width = -1;
>        int height = -1;
>        int depth = -1;
> +      grub_err_t err;
>        unsigned int flags = modevalue;
>        unsigned int flagmask = modemask;
>  
> @@ -505,12 +568,10 @@ grub_video_set_mode (const char *modestring,
>  
>        /* Initialize token holders.  */
>        current_mode = tmp;
> -      param = tmp;
> -      value = NULL;
>  
>        /* XXX: we assume that we're in pure text mode if
>  	 no video mode is initialized. Is it always true? */
> -      if (grub_strcmp (param, "text") == 0)
> +      if (grub_strcmp (current_mode, "text") == 0)
>  	{
>  	  struct grub_video_mode_info mode_info;
>  
> @@ -529,105 +590,13 @@ grub_video_set_mode (const char *modestring,
>  	    }
>  	}
>  
> -      /* Parse <width>x<height>[x<depth>]*/
> -
> -      /* Find width value.  */
> -      value = param;
> -      param = grub_strchr(param, 'x');
> -      if (param == NULL)
> +      err = parse_modespec (current_mode, &width, &height, &depth);
> +      if (err)
>  	{
> -	  grub_err_t rc;
> -
> -	  /* First setup error message.  */
> -	  rc = grub_error (GRUB_ERR_BAD_ARGUMENT,
> -			   "Invalid mode: %s\n",
> -			   current_mode);
> -
>  	  /* Free memory before returning.  */
>  	  grub_free (modevar);
>  
> -	  return rc;
> -	}
> -
> -      *param = 0;
> -      param++;
> -
> -      width = grub_strtoul (value, 0, 0);
> -      if (grub_errno != GRUB_ERR_NONE)
> -	{
> -	  grub_err_t rc;
> -
> -	  /* First setup error message.  */
> -	  rc = grub_error (GRUB_ERR_BAD_ARGUMENT,
> -			   "Invalid mode: %s\n",
> -			   current_mode);
> -
> -	  /* Free memory before returning.  */
> -	  grub_free (modevar);
> -
> -	  return rc;
> -	}
> -
> -      /* Find height value.  */
> -      value = param;
> -      param = grub_strchr(param, 'x');
> -      if (param == NULL)
> -	{
> -	  height = grub_strtoul (value, 0, 0);
> -	  if (grub_errno != GRUB_ERR_NONE)
> -	    {
> -	      grub_err_t rc;
> -
> -	      /* First setup error message.  */
> -	      rc = grub_error (GRUB_ERR_BAD_ARGUMENT,
> -			       "Invalid mode: %s\n",
> -			       current_mode);
> -
> -	      /* Free memory before returning.  */
> -	      grub_free (modevar);
> -
> -	      return rc;
> -	    }
> -	}
> -      else
> -	{
> -	  /* We have optional color depth value.  */
> -	  *param = 0;
> -	  param++;
> -
> -	  height = grub_strtoul (value, 0, 0);
> -	  if (grub_errno != GRUB_ERR_NONE)
> -	    {
> -	      grub_err_t rc;
> -
> -	      /* First setup error message.  */
> -	      rc = grub_error (GRUB_ERR_BAD_ARGUMENT,
> -			       "Invalid mode: %s\n",
> -			       current_mode);
> -
> -	      /* Free memory before returning.  */
> -	      grub_free (modevar);
> -
> -	      return rc;
> -	    }
> -
> -	  /* Convert color depth value.  */
> -	  value = param;
> -	  depth = grub_strtoul (value, 0, 0);
> -	  if (grub_errno != GRUB_ERR_NONE)
> -	    {
> -	      grub_err_t rc;
> -
> -	      /* First setup error message.  */
> -	      rc = grub_error (GRUB_ERR_BAD_ARGUMENT,
> -			       "Invalid mode: %s\n",
> -			       current_mode);
> -
> -	      /* Free memory before returning.  */
> -	      grub_free (modevar);
> -
> -	      return rc;
> -	    }
> +	  return err;
>  	}
>  
>        /* Try out video mode.  */

> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel


-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."