Re: [Intel-gfx] [PATCH i-g-t 7/7] lib/igt_kms: Use kernel command line mode if specified

[Brian Starkey]

On Tue, Apr 25, 2017 at 07:58:18PM +0300, Ville Syrjälä wrote:

On Tue, Apr 25, 2017 at 05:45:13PM +0100, Brian Starkey wrote:

If "video=" is specified on the kernel command-line, use it to override
the default mode in kmstest_get_connector_default_mode.

If a mode override was provided on the command-line, it was probably for
good reason so we should honor it.


Isn't the kernel marking the cmdline mode as preferred already? And if
not, maybe it should.



It doesn't as far as I've seen. It uses the cmdline mode for setting
up the fbdev emulation but I don't think it marks the mode list (or we
aren't calling the helper that does it).

I'll have a look at what it would take to do that.

-Brian



Signed-off-by: Brian Starkey 
---
 lib/igt_kms.c |  135 +
 1 file changed, 135 insertions(+)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 474aa005b9fa..97f80a46354d 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -766,6 +766,131 @@ void kmstest_force_edid(int drm_fd, drmModeConnector 
*connector,
igt_assert(ret != -1);
 }

+/*
+ * Extract xres, yres, refresh and interlaced from a string of the form:
+ *   x[M][R][-][@][i][m][eDd]
+ * xres and yres must be specified, refresh is optional and will be set to
+ * -1 if not present. interlaced defaults to false.
+ */
+static int parse_cmdline_mode(char *video_str, unsigned int *xres,
+ unsigned int *yres,
+ int *refresh, bool *interlaced)
+{
+   int match, len = strlen(video_str);
+   char *token = strtok(video_str, "@");
+
+   if (!token)
+   return -1;
+
+   *interlaced = false;
+   *refresh = -1;
+
+   match = sscanf(token, "%dx%d", xres, yres);
+   if (match != 2)
+   return -1;
+
+   if (strlen(token) < len - 1) {
+   token += strlen(token) + 1;
+
+   match = sscanf(token, "%d", refresh);
+   if (match != 1 || (*refresh < 0))
+   return -1;
+
+   if (strchr(token, 'i'))
+   *interlaced = true;
+   }
+
+   return 0;
+}
+
+static const drmModeModeInfo *
+connector_match_cmdline_mode(const drmModeConnector *connector,
+unsigned int xres, unsigned int yres, int refresh,
+bool interlaced)
+{
+   const drmModeModeInfo *mode = NULL;
+   int i;
+
+   for (i = 0; i < connector->count_modes; i++) {
+   mode = >modes[i];
+   if (mode->hdisplay == xres &&
+   mode->vdisplay == yres &&
+   (refresh < 0 || refresh == mode->vrefresh) &&
+   interlaced == !!(mode->flags & DRM_MODE_FLAG_INTERLACE))
+   return mode;
+   }
+
+   return NULL;
+}
+
+static const drmModeModeInfo *
+kmstest_get_cmdline_mode(int drm_fd, drmModeConnector *connector)
+{
+   char c, *str = NULL, *cursor, *conn_name = NULL;
+   const drmModeModeInfo *mode = NULL;
+   unsigned int size = 0;
+   FILE *fp = NULL;
+
+   fp = fopen("/proc/cmdline", "r");
+   if (!fp)
+   return NULL;
+
+   /* lseek/fseek+ftell/stat don't work on /proc/cmdline */
+   while (fread(, 1, 1, fp))
+   size++;
+   rewind(fp);
+
+   str = calloc(1, size + 1);
+   if (!str)
+   goto done;
+
+   if (fread(str, 1, size, fp) != size)
+   goto done;
+
+   if (!asprintf(_name, "%s-%d:",
+ kmstest_connector_type_str(connector->connector_type),
+ connector->connector_type_id))
+   goto done;
+
+   cursor = str;
+   while ((cursor = strstr(cursor, "video="))) {
+   unsigned int xres, yres;
+   bool interlaced;
+   int refresh;
+
+   cursor += strlen("video=");
+   cursor = strtok(cursor, " \n");
+   if (!cursor)
+   break;
+
+   /* Strip the name if it matches ours */
+   if (!strncmp(cursor, conn_name, strlen(conn_name)))
+   cursor += strlen(conn_name);
+
+   /*
+* Consider this "video=" specification only if it has no
+* name. If the name matched, we would have already stripped it
+* above
+*/
+   if (!strstr(cursor, ":") &&
+   !parse_cmdline_mode(cursor, , , , 
)) {
+   mode = connector_match_cmdline_mode(connector, xres,
+   yres, refresh,
+   interlaced);
+   if (mode)
+   break;
+   }
+
+   cursor += strlen(cursor) + 1;
+   }
+
+done:
+   free(conn_name);
+   free(str);
+ 

Re: [Intel-gfx] [PATCH i-g-t 7/7] lib/igt_kms: Use kernel command line mode if specified

[Ville Syrjälä]

On Tue, Apr 25, 2017 at 05:45:13PM +0100, Brian Starkey wrote:
> If "video=" is specified on the kernel command-line, use it to override
> the default mode in kmstest_get_connector_default_mode.
> 
> If a mode override was provided on the command-line, it was probably for
> good reason so we should honor it.

Isn't the kernel marking the cmdline mode as preferred already? And if
not, maybe it should.

> 
> Signed-off-by: Brian Starkey 
> ---
>  lib/igt_kms.c |  135 
> +
>  1 file changed, 135 insertions(+)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 474aa005b9fa..97f80a46354d 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -766,6 +766,131 @@ void kmstest_force_edid(int drm_fd, drmModeConnector 
> *connector,
>   igt_assert(ret != -1);
>  }
>  
> +/*
> + * Extract xres, yres, refresh and interlaced from a string of the form:
> + *   x[M][R][-][@][i][m][eDd]
> + * xres and yres must be specified, refresh is optional and will be set to
> + * -1 if not present. interlaced defaults to false.
> + */
> +static int parse_cmdline_mode(char *video_str, unsigned int *xres,
> +   unsigned int *yres,
> +   int *refresh, bool *interlaced)
> +{
> + int match, len = strlen(video_str);
> + char *token = strtok(video_str, "@");
> +
> + if (!token)
> + return -1;
> +
> + *interlaced = false;
> + *refresh = -1;
> +
> + match = sscanf(token, "%dx%d", xres, yres);
> + if (match != 2)
> + return -1;
> +
> + if (strlen(token) < len - 1) {
> + token += strlen(token) + 1;
> +
> + match = sscanf(token, "%d", refresh);
> + if (match != 1 || (*refresh < 0))
> + return -1;
> +
> + if (strchr(token, 'i'))
> + *interlaced = true;
> + }
> +
> + return 0;
> +}
> +
> +static const drmModeModeInfo *
> +connector_match_cmdline_mode(const drmModeConnector *connector,
> +  unsigned int xres, unsigned int yres, int refresh,
> +  bool interlaced)
> +{
> + const drmModeModeInfo *mode = NULL;
> + int i;
> +
> + for (i = 0; i < connector->count_modes; i++) {
> + mode = >modes[i];
> + if (mode->hdisplay == xres &&
> + mode->vdisplay == yres &&
> + (refresh < 0 || refresh == mode->vrefresh) &&
> + interlaced == !!(mode->flags & DRM_MODE_FLAG_INTERLACE))
> + return mode;
> + }
> +
> + return NULL;
> +}
> +
> +static const drmModeModeInfo *
> +kmstest_get_cmdline_mode(int drm_fd, drmModeConnector *connector)
> +{
> + char c, *str = NULL, *cursor, *conn_name = NULL;
> + const drmModeModeInfo *mode = NULL;
> + unsigned int size = 0;
> + FILE *fp = NULL;
> +
> + fp = fopen("/proc/cmdline", "r");
> + if (!fp)
> + return NULL;
> +
> + /* lseek/fseek+ftell/stat don't work on /proc/cmdline */
> + while (fread(, 1, 1, fp))
> + size++;
> + rewind(fp);
> +
> + str = calloc(1, size + 1);
> + if (!str)
> + goto done;
> +
> + if (fread(str, 1, size, fp) != size)
> + goto done;
> +
> + if (!asprintf(_name, "%s-%d:",
> +   kmstest_connector_type_str(connector->connector_type),
> +   connector->connector_type_id))
> + goto done;
> +
> + cursor = str;
> + while ((cursor = strstr(cursor, "video="))) {
> + unsigned int xres, yres;
> + bool interlaced;
> + int refresh;
> +
> + cursor += strlen("video=");
> + cursor = strtok(cursor, " \n");
> + if (!cursor)
> + break;
> +
> + /* Strip the name if it matches ours */
> + if (!strncmp(cursor, conn_name, strlen(conn_name)))
> + cursor += strlen(conn_name);
> +
> + /*
> +  * Consider this "video=" specification only if it has no
> +  * name. If the name matched, we would have already stripped it
> +  * above
> +  */
> + if (!strstr(cursor, ":") &&
> + !parse_cmdline_mode(cursor, , , , 
> )) {
> + mode = connector_match_cmdline_mode(connector, xres,
> + yres, refresh,
> + interlaced);
> + if (mode)
> + break;
> + }
> +
> + cursor += strlen(cursor) + 1;
> + }
> +
> +done:
> + free(conn_name);
> + free(str);
> + fclose(fp);
> + return mode;
> +}
> +
>  /**
>   * kmstest_get_connector_default_mode:
>   * @drm_fd: DRM fd
> @@ -773,6 +898,8 @@ void kmstest_force_edid(int drm_fd, 

[Intel-gfx] [PATCH i-g-t 7/7] lib/igt_kms: Use kernel command line mode if specified

[Brian Starkey]

If "video=" is specified on the kernel command-line, use it to override
the default mode in kmstest_get_connector_default_mode.

If a mode override was provided on the command-line, it was probably for
good reason so we should honor it.

Signed-off-by: Brian Starkey 
---
 lib/igt_kms.c |  135 +
 1 file changed, 135 insertions(+)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 474aa005b9fa..97f80a46354d 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -766,6 +766,131 @@ void kmstest_force_edid(int drm_fd, drmModeConnector 
*connector,
igt_assert(ret != -1);
 }
 
+/*
+ * Extract xres, yres, refresh and interlaced from a string of the form:
+ *   x[M][R][-][@][i][m][eDd]
+ * xres and yres must be specified, refresh is optional and will be set to
+ * -1 if not present. interlaced defaults to false.
+ */
+static int parse_cmdline_mode(char *video_str, unsigned int *xres,
+ unsigned int *yres,
+ int *refresh, bool *interlaced)
+{
+   int match, len = strlen(video_str);
+   char *token = strtok(video_str, "@");
+
+   if (!token)
+   return -1;
+
+   *interlaced = false;
+   *refresh = -1;
+
+   match = sscanf(token, "%dx%d", xres, yres);
+   if (match != 2)
+   return -1;
+
+   if (strlen(token) < len - 1) {
+   token += strlen(token) + 1;
+
+   match = sscanf(token, "%d", refresh);
+   if (match != 1 || (*refresh < 0))
+   return -1;
+
+   if (strchr(token, 'i'))
+   *interlaced = true;
+   }
+
+   return 0;
+}
+
+static const drmModeModeInfo *
+connector_match_cmdline_mode(const drmModeConnector *connector,
+unsigned int xres, unsigned int yres, int refresh,
+bool interlaced)
+{
+   const drmModeModeInfo *mode = NULL;
+   int i;
+
+   for (i = 0; i < connector->count_modes; i++) {
+   mode = >modes[i];
+   if (mode->hdisplay == xres &&
+   mode->vdisplay == yres &&
+   (refresh < 0 || refresh == mode->vrefresh) &&
+   interlaced == !!(mode->flags & DRM_MODE_FLAG_INTERLACE))
+   return mode;
+   }
+
+   return NULL;
+}
+
+static const drmModeModeInfo *
+kmstest_get_cmdline_mode(int drm_fd, drmModeConnector *connector)
+{
+   char c, *str = NULL, *cursor, *conn_name = NULL;
+   const drmModeModeInfo *mode = NULL;
+   unsigned int size = 0;
+   FILE *fp = NULL;
+
+   fp = fopen("/proc/cmdline", "r");
+   if (!fp)
+   return NULL;
+
+   /* lseek/fseek+ftell/stat don't work on /proc/cmdline */
+   while (fread(, 1, 1, fp))
+   size++;
+   rewind(fp);
+
+   str = calloc(1, size + 1);
+   if (!str)
+   goto done;
+
+   if (fread(str, 1, size, fp) != size)
+   goto done;
+
+   if (!asprintf(_name, "%s-%d:",
+ kmstest_connector_type_str(connector->connector_type),
+ connector->connector_type_id))
+   goto done;
+
+   cursor = str;
+   while ((cursor = strstr(cursor, "video="))) {
+   unsigned int xres, yres;
+   bool interlaced;
+   int refresh;
+
+   cursor += strlen("video=");
+   cursor = strtok(cursor, " \n");
+   if (!cursor)
+   break;
+
+   /* Strip the name if it matches ours */
+   if (!strncmp(cursor, conn_name, strlen(conn_name)))
+   cursor += strlen(conn_name);
+
+   /*
+* Consider this "video=" specification only if it has no
+* name. If the name matched, we would have already stripped it
+* above
+*/
+   if (!strstr(cursor, ":") &&
+   !parse_cmdline_mode(cursor, , , , 
)) {
+   mode = connector_match_cmdline_mode(connector, xres,
+   yres, refresh,
+   interlaced);
+   if (mode)
+   break;
+   }
+
+   cursor += strlen(cursor) + 1;
+   }
+
+done:
+   free(conn_name);
+   free(str);
+   fclose(fp);
+   return mode;
+}
+
 /**
  * kmstest_get_connector_default_mode:
  * @drm_fd: DRM fd
@@ -773,6 +898,8 @@ void kmstest_force_edid(int drm_fd, drmModeConnector 
*connector,
  * @mode: libdrm mode
  *
  * Retrieves the default mode for @connector and stores it in @mode.
+ * If video= is specified (optionally for this specific connector) on the
+ * kernel command line, then it is used as the default.
  *
  * Returns: true on success, false on