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

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

[Vladimir 'phcoder' Serbinenko]

[-- Attachment #1: Type: text/plain, Size: 106 bytes --]

-- 
Regards
Vladimir 'phcoder' Serbinenko

Personal git repository: http://repo.or.cz/w/grub2/phcoder.git

[-- Attachment #2: videoauto.diff --]
[-- Type: text/plain, Size: 8659 bytes --]

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.  */

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

[Robert Millan]

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."

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

[Vladimir 'phcoder' Serbinenko]

On Mon, Aug 17, 2009 at 3:49 PM, Robert Millan<rmh@aybabtu.com> wrote:
>
>
> Please could you include a description on what this does?
>
when gfxmode=auto the driver is allowed to pick best mode in its
opinion among the modes appropriate for the task. This is needed
because
1) drivers may be limited to only few modes and it may happen that
none of supported modes is in usual list of modes to be checked.
2) driver may know more about connected peripherics and e.g. prefer
1280x800 if it's native resolution of flat panel.



-- 
Regards
Vladimir 'phcoder' Serbinenko

Personal git repository: http://repo.or.cz/w/grub2/phcoder.git

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

[Robert Millan]

On Fri, Aug 21, 2009 at 02:57:45PM +0200, Vladimir 'phcoder' Serbinenko wrote:
> On Mon, Aug 17, 2009 at 3:49 PM, Robert Millan<rmh@aybabtu.com> wrote:
> >
> >
> > Please could you include a description on what this does?
> >
> when gfxmode=auto the driver is allowed to pick best mode in its
> opinion among the modes appropriate for the task. This is needed
> because
> 1) drivers may be limited to only few modes and it may happen that
> none of supported modes is in usual list of modes to be checked.
> 2) driver may know more about connected peripherics and e.g. prefer
> 1280x800 if it's native resolution of flat panel.

Ah, I see.  Yeah, we really should contemplate into detecting native
resolutions.

But is there any reason we don't want this behaviour when gfxmode was
not set?  If we can avoid an additional keyword, what we do fits better
with what the user would expect.

-- 
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."

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

[Vladimir 'phcoder' Serbinenko]

>
> Ah, I see.  Yeah, we really should contemplate into detecting native
> resolutions.
>
> But is there any reason we don't want this behaviour when gfxmode was
> not set?  If we can avoid an additional keyword, what we do fits better
> with what the user would expect.
When gfxmode wasn't set it's treated like if it was set to "auto".I
feel like having keyword "auto" can be handy in situations like
gfxpayload=keep,auto
Which with payloads needing working graphics would mean "use current
graphics mode if we're already in one, otherwise autodetect"
>
> --
> 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."
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>



-- 
Regards
Vladimir 'phcoder' Serbinenko

Personal git repository: http://repo.or.cz/w/grub2/phcoder.git

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

[Robert Millan]

On Sun, Aug 23, 2009 at 01:01:16PM +0200, Vladimir 'phcoder' Serbinenko wrote:
> >
> > Ah, I see.  Yeah, we really should contemplate into detecting native
> > resolutions.
> >
> > But is there any reason we don't want this behaviour when gfxmode was
> > not set?  If we can avoid an additional keyword, what we do fits better
> > with what the user would expect.
> When gfxmode wasn't set it's treated like if it was set to "auto".I
> feel like having keyword "auto" can be handy in situations like
> gfxpayload=keep,auto
> Which with payloads needing working graphics would mean "use current
> graphics mode if we're already in one, otherwise autodetect"

I would interpret "keep" as "keep what we have now".  If we're in text mode,
then "keep text mode".

I think native resolution detection is an opportunity for us to simplify the
user interface, rather than add more options to it.

-- 
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."

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

[Vladimir 'phcoder' Serbinenko]

> I would interpret "keep" as "keep what we have now".  If we're in text mode,
> then "keep text mode".
Yes. But loader may put constraints like "I don't accept text mode".
In this case if we're in text mode then "keep" is rejected as
unacceptable.
>
> I think native resolution detection is an opportunity for us to simplify the
> user interface, rather than add more options to it.
>
These keywords are handy especially internally. User should never need
to set anything to "auto" or "keep,auto" as it's default values in
some contexts
> --
> 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."
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>



-- 
Regards
Vladimir 'phcoder' Serbinenko

Personal git repository: http://repo.or.cz/w/grub2/phcoder.git