[PATCH v2 0/5] drm/vc4: Allow for more boot-time configuration

[PATCH v2 0/5] drm/vc4: Allow for more boot-time configuration

[Maxime Ripard]

Hi,

The proprietary stack for the RaspberryPi allows for a number of video
parameters widely used by their users, but yet don't have any equivalents
in the mainline kernel.

Those options are detailed here:
https://www.raspberrypi.org/documentation/configuration/config-txt/video.md

While not all of them are desirable to have in the mainline kernel, some of
them still have value, such as properties to initialise the overscan or
rotation parameters.

This series is an attempt to support those, and is based on a rewrite of
the command line parser I did a couple of years ago that never reached
upstream (due to a lack of time on my side). While this parser was
initially made to deal with named modes (in order to support TV modes), it
also allowed to extend it more easily, which is why it's resurrected.

Let me know what you think,
Maxime

Changes from v1:
  - Dropped the patches to deal with EDID
  - Added the unit test as selftest
  - Rebased on next

Maxime Ripard (5):
  drm/modes: Rewrite the command line parser
  drm/modes: Support modes names on the command line
  drm/modes: Allow to specify rotation and reflection on the commandline
  drm/modes: Parse overscan properties
  drm/selftests: Add command line parser selftests

 drivers/gpu/drm/drm_connector.c                     |   3 +-
 drivers/gpu/drm/drm_fb_helper.c                     |  55 +-
 drivers/gpu/drm/drm_modes.c                         | 441 +++++--
 drivers/gpu/drm/selftests/Makefile                  |   2 +-
 drivers/gpu/drm/selftests/drm_cmdline_selftests.h   |  49 +-
 drivers/gpu/drm/selftests/test-drm_cmdline_parser.c | 843 +++++++++++++-
 include/drm/drm_connector.h                         |   3 +-
 7 files changed, 1279 insertions(+), 117 deletions(-)
 create mode 100644 drivers/gpu/drm/selftests/drm_cmdline_selftests.h
 create mode 100644 drivers/gpu/drm/selftests/test-drm_cmdline_parser.c

base-commit: 2f4c4d833153517c7791318f8608cf5c212cef68
-- 
git-series 0.9.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 1/5] drm/modes: Rewrite the command line parser

[Maxime Ripard]

From: Maxime Ripard <maxime.ripard@free-electrons.com>

Rewrite the command line parser in order to get away from the state machine
parsing the video mode lines.

Hopefully, this will allow to extend it more easily to support named modes
and / or properties set directly on the command line.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/gpu/drm/drm_modes.c | 308 +++++++++++++++++++++++--------------
 1 file changed, 193 insertions(+), 115 deletions(-)

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 56f92a0bba62..bbacb7743f3b 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -30,6 +30,7 @@
  * authorization from the copyright holder(s) and author(s).
  */
 
+#include <linux/ctype.h>
 #include <linux/list.h>
 #include <linux/list_sort.h>
 #include <linux/export.h>
@@ -1405,6 +1406,131 @@ void drm_connector_list_update(struct drm_connector *connector)
 }
 EXPORT_SYMBOL(drm_connector_list_update);
 
+static int drm_mode_parse_cmdline_bpp(const char *str, char **end_ptr,
+				      struct drm_cmdline_mode *mode)
+{
+	if (str[0] != '-')
+		return -EINVAL;
+
+	mode->bpp = simple_strtol(str + 1, end_ptr, 10);
+	mode->bpp_specified = true;
+
+	return 0;
+}
+
+static int drm_mode_parse_cmdline_refresh(const char *str, char **end_ptr,
+					  struct drm_cmdline_mode *mode)
+{
+	if (str[0] != '@')
+		return -EINVAL;
+
+	mode->refresh = simple_strtol(str + 1, end_ptr, 10);
+	mode->refresh_specified = true;
+
+	return 0;
+}
+
+static int drm_mode_parse_cmdline_extra(const char *str, int length,
+					struct drm_connector *connector,
+					struct drm_cmdline_mode *mode)
+{
+	int i;
+
+	for (i = 0; i < length; i++) {
+		switch (str[i]) {
+		case 'i':
+			mode->interlace = true;
+			break;
+		case 'm':
+			mode->margins = true;
+			break;
+		case 'D':
+			if (mode->force != DRM_FORCE_UNSPECIFIED)
+				return -EINVAL;
+
+			if ((connector->connector_type != DRM_MODE_CONNECTOR_DVII) &&
+			    (connector->connector_type != DRM_MODE_CONNECTOR_HDMIB))
+				mode->force = DRM_FORCE_ON;
+			else
+				mode->force = DRM_FORCE_ON_DIGITAL;
+			break;
+		case 'd':
+			if (mode->force != DRM_FORCE_UNSPECIFIED)
+				return -EINVAL;
+
+			mode->force = DRM_FORCE_OFF;
+			break;
+		case 'e':
+			if (mode->force != DRM_FORCE_UNSPECIFIED)
+				return -EINVAL;
+
+			mode->force = DRM_FORCE_ON;
+			break;
+		default:
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
+static int drm_mode_parse_cmdline_res_mode(const char *str, unsigned int length,
+					   bool extras,
+					   struct drm_connector *connector,
+					   struct drm_cmdline_mode *mode)
+{
+	bool rb = false, cvt = false;
+	int xres = 0, yres = 0;
+	int remaining, i;
+	char *end_ptr;
+
+	xres = simple_strtol(str, &end_ptr, 10);
+
+	if (end_ptr[0] != 'x')
+		return -EINVAL;
+	end_ptr++;
+
+	yres = simple_strtol(end_ptr, &end_ptr, 10);
+
+	remaining = length - (end_ptr - str);
+	if (remaining < 0)
+		return -EINVAL;
+
+	for (i = 0; i < remaining; i++) {
+		switch (end_ptr[i]) {
+		case 'M':
+			cvt = true;
+			break;
+		case 'R':
+			rb = true;
+			break;
+		default:
+			/*
+			 * Try to pass that to our extras parsing
+			 * function to handle the case where the
+			 * extras are directly after the resolution
+			 */
+			if (extras) {
+				int ret = drm_mode_parse_cmdline_extra(end_ptr + i,
+								       1,
+								       connector,
+								       mode);
+				if (ret)
+					return ret;
+			} else {
+				return -EINVAL;
+			}
+		}
+	}
+
+	mode->xres = xres;
+	mode->yres = yres;
+	mode->cvt = cvt;
+	mode->rb = rb;
+
+	return 0;
+}
+
 /**
  * drm_mode_parse_command_line_for_connector - parse command line modeline for connector
  * @mode_option: optional per connector mode option
@@ -1431,13 +1557,12 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option,
 					       struct drm_cmdline_mode *mode)
 {
 	const char *name;
-	unsigned int namelen;
-	bool res_specified = false, bpp_specified = false, refresh_specified = false;
-	unsigned int xres = 0, yres = 0, bpp = 32, refresh = 0;
-	bool yres_specified = false, cvt = false, rb = false;
-	bool interlace = false, margins = false, was_digit = false;
-	int i;
-	enum drm_connector_force force = DRM_FORCE_UNSPECIFIED;
+	bool parse_extras = false;
+	unsigned int bpp_off = 0, refresh_off = 0;
+	unsigned int mode_end = 0;
+	char *bpp_ptr = NULL, *refresh_ptr = NULL, *extra_ptr = NULL;
+	char *bpp_end_ptr = NULL, *refresh_end_ptr = NULL;
+	int ret;
 
 #ifdef CONFIG_FB
 	if (!mode_option)
@@ -1450,127 +1575,80 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option,
 	}
 
 	name = mode_option;
-	namelen = strlen(name);
-	for (i = namelen-1; i >= 0; i--) {
-		switch (name[i]) {
-		case '@':
-			if (!refresh_specified && !bpp_specified &&
-			    !yres_specified && !cvt && !rb && was_digit) {
-				refresh = simple_strtol(&name[i+1], NULL, 10);
-				refresh_specified = true;
-				was_digit = false;
-			} else
-				goto done;
-			break;
-		case '-':
-			if (!bpp_specified && !yres_specified && !cvt &&
-			    !rb && was_digit) {
-				bpp = simple_strtol(&name[i+1], NULL, 10);
-				bpp_specified = true;
-				was_digit = false;
-			} else
-				goto done;
-			break;
-		case 'x':
-			if (!yres_specified && was_digit) {
-				yres = simple_strtol(&name[i+1], NULL, 10);
-				yres_specified = true;
-				was_digit = false;
-			} else
-				goto done;
-			break;
-		case '0' ... '9':
-			was_digit = true;
-			break;
-		case 'M':
-			if (yres_specified || cvt || was_digit)
-				goto done;
-			cvt = true;
-			break;
-		case 'R':
-			if (yres_specified || cvt || rb || was_digit)
-				goto done;
-			rb = true;
-			break;
-		case 'm':
-			if (cvt || yres_specified || was_digit)
-				goto done;
-			margins = true;
-			break;
-		case 'i':
-			if (cvt || yres_specified || was_digit)
-				goto done;
-			interlace = true;
-			break;
-		case 'e':
-			if (yres_specified || bpp_specified || refresh_specified ||
-			    was_digit || (force != DRM_FORCE_UNSPECIFIED))
-				goto done;
 
-			force = DRM_FORCE_ON;
-			break;
-		case 'D':
-			if (yres_specified || bpp_specified || refresh_specified ||
-			    was_digit || (force != DRM_FORCE_UNSPECIFIED))
-				goto done;
+	if (!isdigit(name[0]))
+		return false;
 
-			if ((connector->connector_type != DRM_MODE_CONNECTOR_DVII) &&
-			    (connector->connector_type != DRM_MODE_CONNECTOR_HDMIB))
-				force = DRM_FORCE_ON;
-			else
-				force = DRM_FORCE_ON_DIGITAL;
-			break;
-		case 'd':
-			if (yres_specified || bpp_specified || refresh_specified ||
-			    was_digit || (force != DRM_FORCE_UNSPECIFIED))
-				goto done;
+	/* Try to locate the bpp and refresh specifiers, if any */
+	bpp_ptr = strchr(name, '-');
+	if (bpp_ptr) {
+		bpp_off = bpp_ptr - name;
+		mode->bpp_specified = true;
+	}
 
-			force = DRM_FORCE_OFF;
-			break;
-		default:
-			goto done;
-		}
+	refresh_ptr = strchr(name, '@');
+	if (refresh_ptr) {
+		if (named_mode)
+			return false;
+
+		refresh_off = refresh_ptr - name;
+		mode->refresh_specified = true;
 	}
 
-	if (i < 0 && yres_specified) {
-		char *ch;
-		xres = simple_strtol(name, &ch, 10);
-		if ((ch != NULL) && (*ch == 'x'))
-			res_specified = true;
-		else
-			i = ch - name;
-	} else if (!yres_specified && was_digit) {
-		/* catch mode that begins with digits but has no 'x' */
-		i = 0;
+	/* Locate the end of the name / resolution, and parse it */
+	if (bpp_ptr && refresh_ptr) {
+		mode_end = min(bpp_off, refresh_off);
+	} else if (bpp_ptr) {
+		mode_end = bpp_off;
+	} else if (refresh_ptr) {
+		mode_end = refresh_off;
+	} else {
+		mode_end = strlen(name);
+		parse_extras = true;
 	}
-done:
-	if (i >= 0) {
-		pr_warn("[drm] parse error at position %i in video mode '%s'\n",
-			i, name);
-		mode->specified = false;
+
+	ret = drm_mode_parse_cmdline_res_mode(name, mode_end,
+					      parse_extras,
+					      connector,
+					      mode);
+	if (ret)
 		return false;
-	}
+	mode->specified = true;
 
-	if (res_specified) {
-		mode->specified = true;
-		mode->xres = xres;
-		mode->yres = yres;
+	if (bpp_ptr) {
+		ret = drm_mode_parse_cmdline_bpp(bpp_ptr, &bpp_end_ptr, mode);
+		if (ret)
+			return false;
 	}
 
-	if (refresh_specified) {
-		mode->refresh_specified = true;
-		mode->refresh = refresh;
+	if (refresh_ptr) {
+		ret = drm_mode_parse_cmdline_refresh(refresh_ptr,
+						     &refresh_end_ptr, mode);
+		if (ret)
+			return false;
 	}
 
-	if (bpp_specified) {
-		mode->bpp_specified = true;
-		mode->bpp = bpp;
+	/*
+	 * Locate the end of the bpp / refresh, and parse the extras
+	 * if relevant
+	 */
+	if (bpp_ptr && refresh_ptr)
+		extra_ptr = max(bpp_end_ptr, refresh_end_ptr);
+	else if (bpp_ptr)
+		extra_ptr = bpp_end_ptr;
+	else if (refresh_ptr)
+		extra_ptr = refresh_end_ptr;
+
+	if (extra_ptr) {
+		int remaining = strlen(name) - (extra_ptr - name);
+
+		/*
+		 * We still have characters to process, while
+		 * we shouldn't have any
+		 */
+		if (remaining > 0)
+			return false;
 	}
-	mode->rb = rb;
-	mode->cvt = cvt;
-	mode->interlace = interlace;
-	mode->margins = margins;
-	mode->force = force;
 
 	return true;
 }
-- 
git-series 0.9.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/5] drm/modes: Rewrite the command line parser

[kbuild test robot]

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

Hi Maxime,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.1-rc4 next-20190412]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Maxime-Ripard/drm-modes-Rewrite-the-command-line-parser/20190412-122837
config: x86_64-randconfig-s1-04121728 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

Note: the linux-review/Maxime-Ripard/drm-modes-Rewrite-the-command-line-parser/20190412-122837 HEAD 6993bfc971bbb32a70c38ec6e89a92c658f21f74 builds fine.
      It only hurts bisectibility.

All error/warnings (new ones prefixed by >>):

   In file included from include/linux/kernel.h:11:0,
                    from include/linux/list.h:9,
                    from drivers/gpu//drm/drm_modes.c:34:
   drivers/gpu//drm/drm_modes.c: In function 'drm_mode_parse_command_line_for_connector':
>> drivers/gpu//drm/drm_modes.c:1591:7: error: 'named_mode' undeclared (first use in this function)
      if (named_mode)
          ^
   include/linux/compiler.h:58:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^~~~
>> drivers/gpu//drm/drm_modes.c:1591:3: note: in expansion of macro 'if'
      if (named_mode)
      ^~
   drivers/gpu//drm/drm_modes.c:1591:7: note: each undeclared identifier is reported only once for each function it appears in
      if (named_mode)
          ^
   include/linux/compiler.h:58:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^~~~
>> drivers/gpu//drm/drm_modes.c:1591:3: note: in expansion of macro 'if'
      if (named_mode)
      ^~

vim +/named_mode +1591 drivers/gpu//drm/drm_modes.c

  1571	
  1572		if (!mode_option) {
  1573			mode->specified = false;
  1574			return false;
  1575		}
  1576	
  1577		name = mode_option;
  1578	
  1579		if (!isdigit(name[0]))
  1580			return false;
  1581	
  1582		/* Try to locate the bpp and refresh specifiers, if any */
  1583		bpp_ptr = strchr(name, '-');
  1584		if (bpp_ptr) {
  1585			bpp_off = bpp_ptr - name;
  1586			mode->bpp_specified = true;
  1587		}
  1588	
  1589		refresh_ptr = strchr(name, '@');
  1590		if (refresh_ptr) {
> 1591			if (named_mode)
  1592				return false;
  1593	
  1594			refresh_off = refresh_ptr - name;
  1595			mode->refresh_specified = true;
  1596		}
  1597	
  1598		/* Locate the end of the name / resolution, and parse it */
  1599		if (bpp_ptr && refresh_ptr) {
  1600			mode_end = min(bpp_off, refresh_off);
  1601		} else if (bpp_ptr) {
  1602			mode_end = bpp_off;
  1603		} else if (refresh_ptr) {
  1604			mode_end = refresh_off;
  1605		} else {
  1606			mode_end = strlen(name);
  1607			parse_extras = true;
  1608		}
  1609	
  1610		ret = drm_mode_parse_cmdline_res_mode(name, mode_end,
  1611						      parse_extras,
  1612						      connector,
  1613						      mode);
  1614		if (ret)
  1615			return false;
  1616		mode->specified = true;
  1617	
  1618		if (bpp_ptr) {
  1619			ret = drm_mode_parse_cmdline_bpp(bpp_ptr, &bpp_end_ptr, mode);
  1620			if (ret)
  1621				return false;
  1622		}
  1623	
  1624		if (refresh_ptr) {
  1625			ret = drm_mode_parse_cmdline_refresh(refresh_ptr,
  1626							     &refresh_end_ptr, mode);
  1627			if (ret)
  1628				return false;
  1629		}
  1630	
  1631		/*
  1632		 * Locate the end of the bpp / refresh, and parse the extras
  1633		 * if relevant
  1634		 */
  1635		if (bpp_ptr && refresh_ptr)
  1636			extra_ptr = max(bpp_end_ptr, refresh_end_ptr);
  1637		else if (bpp_ptr)
  1638			extra_ptr = bpp_end_ptr;
  1639		else if (refresh_ptr)
  1640			extra_ptr = refresh_end_ptr;
  1641	
  1642		if (extra_ptr) {
  1643			int remaining = strlen(name) - (extra_ptr - name);
  1644	
  1645			/*
  1646			 * We still have characters to process, while
  1647			 * we shouldn't have any
  1648			 */
  1649			if (remaining > 0)
  1650				return false;
  1651		}
  1652	
  1653		return true;
  1654	}
  1655	EXPORT_SYMBOL(drm_mode_parse_command_line_for_connector);
  1656	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 35790 bytes --]

[-- Attachment #3: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 2/5] drm/modes: Support modes names on the command line

[Maxime Ripard]

From: Maxime Ripard <maxime.ripard@free-electrons.com>

The drm subsystem also uses the video= kernel parameter, and in the
documentation refers to the fbdev documentation for that parameter.

However, that documentation also says that instead of giving the mode using
its resolution we can also give a name. However, DRM doesn't handle that
case at the moment. Even though in most case it shouldn't make any
difference, it might be useful for analog modes, where different standards
might have the same resolution, but still have a few different parameters
that are not encoded in the modes (NTSC vs NTSC-J vs PAL-M for example).

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/gpu/drm/drm_connector.c |  3 +-
 drivers/gpu/drm/drm_fb_helper.c |  4 +++-
 drivers/gpu/drm/drm_modes.c     | 49 +++++++++++++++++++++++-----------
 include/drm/drm_connector.h     |  1 +-
 4 files changed, 41 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 2355124849db..e33814f5940e 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -139,8 +139,9 @@ static void drm_connector_get_cmdline_mode(struct drm_connector *connector)
 		connector->force = mode->force;
 	}
 
-	DRM_DEBUG_KMS("cmdline mode for connector %s %dx%d@%dHz%s%s%s\n",
+	DRM_DEBUG_KMS("cmdline mode for connector %s %s %dx%d@%dHz%s%s%s\n",
 		      connector->name,
+		      mode->name ? mode->name : "",
 		      mode->xres, mode->yres,
 		      mode->refresh_specified ? mode->refresh : 60,
 		      mode->rb ? " reduced blanking" : "",
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index d1ce7bd04cad..b3a5d79436ae 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -2156,6 +2156,10 @@ struct drm_display_mode *drm_pick_cmdline_mode(struct drm_fb_helper_connector *f
 	prefer_non_interlace = !cmdline_mode->interlace;
 again:
 	list_for_each_entry(mode, &fb_helper_conn->connector->modes, head) {
+		/* Check (optional) mode name first */
+		if (!strcmp(mode->name, cmdline_mode->name))
+			return mode;
+
 		/* check width/height */
 		if (mode->hdisplay != cmdline_mode->xres ||
 		    mode->vdisplay != cmdline_mode->yres)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index bbacb7743f3b..9613c1a28487 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1557,7 +1557,7 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option,
 					       struct drm_cmdline_mode *mode)
 {
 	const char *name;
-	bool parse_extras = false;
+	bool named_mode = false, parse_extras = false;
 	unsigned int bpp_off = 0, refresh_off = 0;
 	unsigned int mode_end = 0;
 	char *bpp_ptr = NULL, *refresh_ptr = NULL, *extra_ptr = NULL;
@@ -1576,8 +1576,14 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option,
 
 	name = mode_option;
 
+	/*
+	 * If the first character is not a digit, then it means that
+	 * we have a named mode.
+	 */
 	if (!isdigit(name[0]))
-		return false;
+		named_mode = true;
+	else
+		named_mode = false;
 
 	/* Try to locate the bpp and refresh specifiers, if any */
 	bpp_ptr = strchr(name, '-');
@@ -1607,12 +1613,16 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option,
 		parse_extras = true;
 	}
 
-	ret = drm_mode_parse_cmdline_res_mode(name, mode_end,
-					      parse_extras,
-					      connector,
-					      mode);
-	if (ret)
-		return false;
+	if (named_mode) {
+		strncpy(mode->name, name, mode_end);
+	} else {
+		ret = drm_mode_parse_cmdline_res_mode(name, mode_end,
+						      parse_extras,
+						      connector,
+						      mode);
+		if (ret)
+			return false;
+	}
 	mode->specified = true;
 
 	if (bpp_ptr) {
@@ -1640,14 +1650,23 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option,
 		extra_ptr = refresh_end_ptr;
 
 	if (extra_ptr) {
-		int remaining = strlen(name) - (extra_ptr - name);
+		if (!named_mode) {
+			int len = strlen(name) - (extra_ptr - name);
 
-		/*
-		 * We still have characters to process, while
-		 * we shouldn't have any
-		 */
-		if (remaining > 0)
-			return false;
+			ret = drm_mode_parse_cmdline_extra(extra_ptr, len,
+							   connector, mode);
+			if (ret)
+				return false;
+		} else {
+			int remaining = strlen(name) - (extra_ptr - name);
+
+			/*
+			 * We still have characters to process, while
+			 * we shouldn't have any
+			 */
+			if (remaining > 0)
+				return false;
+		}
 	}
 
 	return true;
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 02a131202add..06aa3b9cb920 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -896,6 +896,7 @@ struct drm_connector_funcs {
 
 /* mode specified on the command line */
 struct drm_cmdline_mode {
+	char name[DRM_DISPLAY_MODE_LEN];
 	bool specified;
 	bool refresh_specified;
 	bool bpp_specified;
-- 
git-series 0.9.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 3/5] drm/modes: Allow to specify rotation and reflection on the commandline

[Maxime Ripard]

Rotations and reflections setup are needed in some scenarios to initialise
properly the initial framebuffer. Some drivers already had a bunch of
quirks to deal with this, such as either a private kernel command line
parameter (omapdss) or on the device tree (various panels).

In order to accomodate this, let's create a video mode parameter to deal
with the rotation and reflexion.

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 drivers/gpu/drm/drm_fb_helper.c |   4 +-
 drivers/gpu/drm/drm_modes.c     | 110 +++++++++++++++++++++++++++------
 include/drm/drm_connector.h     |   1 +-
 3 files changed, 95 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index b3a5d79436ae..8781897559b2 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -2521,6 +2521,7 @@ static void drm_setup_crtc_rotation(struct drm_fb_helper *fb_helper,
 				    struct drm_connector *connector)
 {
 	struct drm_plane *plane = fb_crtc->mode_set.crtc->primary;
+	struct drm_cmdline_mode *mode = &connector->cmdline_mode;
 	uint64_t valid_mask = 0;
 	int i, rotation;
 
@@ -2540,6 +2541,9 @@ static void drm_setup_crtc_rotation(struct drm_fb_helper *fb_helper,
 		rotation = DRM_MODE_ROTATE_0;
 	}
 
+	if (mode->rotation != DRM_MODE_ROTATE_0)
+		fb_crtc->rotation = mode->rotation;
+
 	/*
 	 * TODO: support 90 / 270 degree hardware rotation,
 	 * depending on the hardware this may require the framebuffer
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 9613c1a28487..ac8d70b92b62 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1531,6 +1531,71 @@ static int drm_mode_parse_cmdline_res_mode(const char *str, unsigned int length,
 	return 0;
 }
 
+static int drm_mode_parse_cmdline_options(char *str, size_t len,
+					  struct drm_connector *connector,
+					  struct drm_cmdline_mode *mode)
+{
+	unsigned int rotation = 0;
+	char *sep = str;
+
+	while ((sep = strchr(sep, ','))) {
+		char *delim, *option;
+
+		option = sep + 1;
+		delim = strchr(option, '=');
+		if (!delim) {
+			delim = strchr(option, ',');
+
+			if (!delim)
+				delim = str + len;
+		}
+
+		if (!strncmp(option, "rotate", delim - option)) {
+			const char *value = delim + 1;
+			unsigned int deg;
+
+			deg = simple_strtol(value, &sep, 10);
+
+			/* Make sure we have parsed something */
+			if (sep == value)
+				return -EINVAL;
+
+			switch (deg) {
+			case 0:
+				rotation |= DRM_MODE_ROTATE_0;
+				break;
+
+			case 90:
+				rotation |= DRM_MODE_ROTATE_90;
+				break;
+
+			case 180:
+				rotation |= DRM_MODE_ROTATE_180;
+				break;
+
+			case 270:
+				rotation |= DRM_MODE_ROTATE_270;
+				break;
+
+			default:
+				return -EINVAL;
+			}
+		} else if (!strncmp(option, "reflect_x", delim - option)) {
+			rotation |= DRM_MODE_REFLECT_X;
+			sep = delim;
+		} else if (!strncmp(option, "reflect_y", delim - option)) {
+			rotation |= DRM_MODE_REFLECT_Y;
+			sep = delim;
+		} else {
+			return -EINVAL;
+		}
+	}
+
+	mode->rotation = rotation;
+
+	return 0;
+}
+
 /**
  * drm_mode_parse_command_line_for_connector - parse command line modeline for connector
  * @mode_option: optional per connector mode option
@@ -1558,9 +1623,10 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option,
 {
 	const char *name;
 	bool named_mode = false, parse_extras = false;
-	unsigned int bpp_off = 0, refresh_off = 0;
+	unsigned int bpp_off = 0, refresh_off = 0, options_off = 0;
 	unsigned int mode_end = 0;
 	char *bpp_ptr = NULL, *refresh_ptr = NULL, *extra_ptr = NULL;
+	char *options_ptr = NULL;
 	char *bpp_end_ptr = NULL, *refresh_end_ptr = NULL;
 	int ret;
 
@@ -1601,13 +1667,18 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option,
 		mode->refresh_specified = true;
 	}
 
+	/* Locate the start of named options */
+	options_ptr = strchr(name, ',');
+	if (options_ptr)
+		options_off = options_ptr - name;
+
 	/* Locate the end of the name / resolution, and parse it */
-	if (bpp_ptr && refresh_ptr) {
-		mode_end = min(bpp_off, refresh_off);
-	} else if (bpp_ptr) {
+	if (bpp_ptr) {
 		mode_end = bpp_off;
 	} else if (refresh_ptr) {
 		mode_end = refresh_off;
+	} else if (options_ptr) {
+		mode_end = options_off;
 	} else {
 		mode_end = strlen(name);
 		parse_extras = true;
@@ -1649,24 +1720,23 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option,
 	else if (refresh_ptr)
 		extra_ptr = refresh_end_ptr;
 
-	if (extra_ptr) {
-		if (!named_mode) {
-			int len = strlen(name) - (extra_ptr - name);
+	if (extra_ptr &&
+	    extra_ptr != options_ptr) {
+		int len = strlen(name) - (extra_ptr - name);
 
-			ret = drm_mode_parse_cmdline_extra(extra_ptr, len,
-							   connector, mode);
-			if (ret)
-				return false;
-		} else {
-			int remaining = strlen(name) - (extra_ptr - name);
+		ret = drm_mode_parse_cmdline_extra(extra_ptr, len,
+						   connector, mode);
+		if (ret)
+			return false;
+	}
 
-			/*
-			 * We still have characters to process, while
-			 * we shouldn't have any
-			 */
-			if (remaining > 0)
-				return false;
-		}
+	if (options_ptr) {
+		int len = strlen(name) - (options_ptr - name);
+
+		ret = drm_mode_parse_cmdline_options(options_ptr, len,
+						     connector, mode);
+		if (ret)
+			return false;
 	}
 
 	return true;
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 06aa3b9cb920..dfe7f2304b35 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -907,6 +907,7 @@ struct drm_cmdline_mode {
 	bool interlace;
 	bool cvt;
 	bool margins;
+	unsigned int rotation;
 	enum drm_connector_force force;
 };
 
-- 
git-series 0.9.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 3/5] drm/modes: Allow to specify rotation and reflection on the commandline

[Daniel Vetter]

On Thu, Apr 11, 2019 at 03:22:41PM +0200, Maxime Ripard wrote:
> Rotations and reflections setup are needed in some scenarios to initialise
> properly the initial framebuffer. Some drivers already had a bunch of
> quirks to deal with this, such as either a private kernel command line
> parameter (omapdss) or on the device tree (various panels).
> 
> In order to accomodate this, let's create a video mode parameter to deal
> with the rotation and reflexion.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
>  drivers/gpu/drm/drm_fb_helper.c |   4 +-
>  drivers/gpu/drm/drm_modes.c     | 110 +++++++++++++++++++++++++++------
>  include/drm/drm_connector.h     |   1 +-
>  3 files changed, 95 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index b3a5d79436ae..8781897559b2 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -2521,6 +2521,7 @@ static void drm_setup_crtc_rotation(struct drm_fb_helper *fb_helper,
>  				    struct drm_connector *connector)
>  {
>  	struct drm_plane *plane = fb_crtc->mode_set.crtc->primary;
> +	struct drm_cmdline_mode *mode = &connector->cmdline_mode;
>  	uint64_t valid_mask = 0;
>  	int i, rotation;
>  
> @@ -2540,6 +2541,9 @@ static void drm_setup_crtc_rotation(struct drm_fb_helper *fb_helper,
>  		rotation = DRM_MODE_ROTATE_0;
>  	}
>  
> +	if (mode->rotation != DRM_MODE_ROTATE_0)
> +		fb_crtc->rotation = mode->rotation;

Hm, kinda awkward that we have to tie all this to the fbdev helpers. But
that's already the case with the old stuff, so mea culpa I guess.

> +
>  	/*
>  	 * TODO: support 90 / 270 degree hardware rotation,
>  	 * depending on the hardware this may require the framebuffer
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index 9613c1a28487..ac8d70b92b62 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -1531,6 +1531,71 @@ static int drm_mode_parse_cmdline_res_mode(const char *str, unsigned int length,
>  	return 0;
>  }
>  
> +static int drm_mode_parse_cmdline_options(char *str, size_t len,
> +					  struct drm_connector *connector,
> +					  struct drm_cmdline_mode *mode)
> +{
> +	unsigned int rotation = 0;
> +	char *sep = str;
> +
> +	while ((sep = strchr(sep, ','))) {
> +		char *delim, *option;
> +
> +		option = sep + 1;
> +		delim = strchr(option, '=');
> +		if (!delim) {
> +			delim = strchr(option, ',');
> +
> +			if (!delim)
> +				delim = str + len;
> +		}
> +
> +		if (!strncmp(option, "rotate", delim - option)) {
> +			const char *value = delim + 1;
> +			unsigned int deg;
> +
> +			deg = simple_strtol(value, &sep, 10);
> +
> +			/* Make sure we have parsed something */
> +			if (sep == value)
> +				return -EINVAL;
> +
> +			switch (deg) {
> +			case 0:
> +				rotation |= DRM_MODE_ROTATE_0;
> +				break;
> +
> +			case 90:
> +				rotation |= DRM_MODE_ROTATE_90;
> +				break;
> +
> +			case 180:
> +				rotation |= DRM_MODE_ROTATE_180;
> +				break;
> +
> +			case 270:
> +				rotation |= DRM_MODE_ROTATE_270;
> +				break;
> +
> +			default:
> +				return -EINVAL;
> +			}
> +		} else if (!strncmp(option, "reflect_x", delim - option)) {
> +			rotation |= DRM_MODE_REFLECT_X;
> +			sep = delim;
> +		} else if (!strncmp(option, "reflect_y", delim - option)) {
> +			rotation |= DRM_MODE_REFLECT_Y;
> +			sep = delim;
> +		} else {
> +			return -EINVAL;
> +		}
> +	}
> +
> +	mode->rotation = rotation;
> +
> +	return 0;
> +}
> +
>  /**
>   * drm_mode_parse_command_line_for_connector - parse command line modeline for connector
>   * @mode_option: optional per connector mode option
> @@ -1558,9 +1623,10 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option,
>  {
>  	const char *name;
>  	bool named_mode = false, parse_extras = false;
> -	unsigned int bpp_off = 0, refresh_off = 0;
> +	unsigned int bpp_off = 0, refresh_off = 0, options_off = 0;
>  	unsigned int mode_end = 0;
>  	char *bpp_ptr = NULL, *refresh_ptr = NULL, *extra_ptr = NULL;
> +	char *options_ptr = NULL;
>  	char *bpp_end_ptr = NULL, *refresh_end_ptr = NULL;
>  	int ret;
>  
> @@ -1601,13 +1667,18 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option,
>  		mode->refresh_specified = true;
>  	}
>  
> +	/* Locate the start of named options */
> +	options_ptr = strchr(name, ',');
> +	if (options_ptr)
> +		options_off = options_ptr - name;
> +
>  	/* Locate the end of the name / resolution, and parse it */
> -	if (bpp_ptr && refresh_ptr) {
> -		mode_end = min(bpp_off, refresh_off);
> -	} else if (bpp_ptr) {
> +	if (bpp_ptr) {
>  		mode_end = bpp_off;
>  	} else if (refresh_ptr) {
>  		mode_end = refresh_off;
> +	} else if (options_ptr) {
> +		mode_end = options_off;
>  	} else {
>  		mode_end = strlen(name);
>  		parse_extras = true;
> @@ -1649,24 +1720,23 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option,
>  	else if (refresh_ptr)
>  		extra_ptr = refresh_end_ptr;
>  
> -	if (extra_ptr) {
> -		if (!named_mode) {
> -			int len = strlen(name) - (extra_ptr - name);
> +	if (extra_ptr &&
> +	    extra_ptr != options_ptr) {
> +		int len = strlen(name) - (extra_ptr - name);
>  
> -			ret = drm_mode_parse_cmdline_extra(extra_ptr, len,
> -							   connector, mode);
> -			if (ret)
> -				return false;
> -		} else {
> -			int remaining = strlen(name) - (extra_ptr - name);
> +		ret = drm_mode_parse_cmdline_extra(extra_ptr, len,
> +						   connector, mode);
> +		if (ret)
> +			return false;
> +	}
>  
> -			/*
> -			 * We still have characters to process, while
> -			 * we shouldn't have any
> -			 */
> -			if (remaining > 0)
> -				return false;
> -		}
> +	if (options_ptr) {
> +		int len = strlen(name) - (options_ptr - name);
> +
> +		ret = drm_mode_parse_cmdline_options(options_ptr, len,
> +						     connector, mode);
> +		if (ret)
> +			return false;
>  	}
>  
>  	return true;
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 06aa3b9cb920..dfe7f2304b35 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -907,6 +907,7 @@ struct drm_cmdline_mode {
>  	bool interlace;
>  	bool cvt;
>  	bool margins;
> +	unsigned int rotation;
>  	enum drm_connector_force force;

Kerneldoc for this would be neat.
-Daniel

>  };
>  
> -- 
> git-series 0.9.1
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 3/5] drm/modes: Allow to specify rotation and reflection on the commandline

[Noralf Trønnes]

Den 11.04.2019 15.22, skrev Maxime Ripard:
> Rotations and reflections setup are needed in some scenarios to initialise
> properly the initial framebuffer. Some drivers already had a bunch of
> quirks to deal with this, such as either a private kernel command line
> parameter (omapdss) or on the device tree (various panels).
> 
> In order to accomodate this, let's create a video mode parameter to deal
> with the rotation and reflexion.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
>  drivers/gpu/drm/drm_fb_helper.c |   4 +-
>  drivers/gpu/drm/drm_modes.c     | 110 +++++++++++++++++++++++++++------
>  include/drm/drm_connector.h     |   1 +-
>  3 files changed, 95 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index b3a5d79436ae..8781897559b2 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -2521,6 +2521,7 @@ static void drm_setup_crtc_rotation(struct drm_fb_helper *fb_helper,
>  				    struct drm_connector *connector)
>  {
>  	struct drm_plane *plane = fb_crtc->mode_set.crtc->primary;
> +	struct drm_cmdline_mode *mode = &connector->cmdline_mode;
>  	uint64_t valid_mask = 0;
>  	int i, rotation;
>  
> @@ -2540,6 +2541,9 @@ static void drm_setup_crtc_rotation(struct drm_fb_helper *fb_helper,
>  		rotation = DRM_MODE_ROTATE_0;
>  	}
>  
> +	if (mode->rotation != DRM_MODE_ROTATE_0)
> +		fb_crtc->rotation = mode->rotation;
> +

We already have a property to describe initial display/panel rotation.
If we can set connector->display_info.panel_orientation from the video=
parameter, then there's no need to modify drm_fb_helper, it will just work.

In that case, maybe 'orientation' is a better argument name with values
mapped to the enum.

And further, should we add the property
(drm_connector_init_panel_orientation_property()) so DRM userspace can
know about it to?

Some pointers:
- 8d70f395e6cb ("drm: Add support for a panel-orientation connector
property")
- 8f0cb418393b ("drm/fb-helper: Apply panel orientation connector prop
to the primary plane")
- drivers/gpu/drm/drm_panel_orientation_quirks.c

Noralf.

>  	/*
>  	 * TODO: support 90 / 270 degree hardware rotation,
>  	 * depending on the hardware this may require the framebuffer
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index 9613c1a28487..ac8d70b92b62 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -1531,6 +1531,71 @@ static int drm_mode_parse_cmdline_res_mode(const char *str, unsigned int length,
>  	return 0;
>  }
>  
> +static int drm_mode_parse_cmdline_options(char *str, size_t len,
> +					  struct drm_connector *connector,
> +					  struct drm_cmdline_mode *mode)
> +{
> +	unsigned int rotation = 0;
> +	char *sep = str;
> +
> +	while ((sep = strchr(sep, ','))) {
> +		char *delim, *option;
> +
> +		option = sep + 1;
> +		delim = strchr(option, '=');
> +		if (!delim) {
> +			delim = strchr(option, ',');
> +
> +			if (!delim)
> +				delim = str + len;
> +		}
> +
> +		if (!strncmp(option, "rotate", delim - option)) {
> +			const char *value = delim + 1;
> +			unsigned int deg;
> +
> +			deg = simple_strtol(value, &sep, 10);
> +
> +			/* Make sure we have parsed something */
> +			if (sep == value)
> +				return -EINVAL;
> +
> +			switch (deg) {
> +			case 0:
> +				rotation |= DRM_MODE_ROTATE_0;
> +				break;
> +
> +			case 90:
> +				rotation |= DRM_MODE_ROTATE_90;
> +				break;
> +
> +			case 180:
> +				rotation |= DRM_MODE_ROTATE_180;
> +				break;
> +
> +			case 270:
> +				rotation |= DRM_MODE_ROTATE_270;
> +				break;
> +
> +			default:
> +				return -EINVAL;
> +			}
> +		} else if (!strncmp(option, "reflect_x", delim - option)) {
> +			rotation |= DRM_MODE_REFLECT_X;
> +			sep = delim;
> +		} else if (!strncmp(option, "reflect_y", delim - option)) {
> +			rotation |= DRM_MODE_REFLECT_Y;
> +			sep = delim;
> +		} else {
> +			return -EINVAL;
> +		}
> +	}
> +
> +	mode->rotation = rotation;
> +
> +	return 0;
> +}
> +
>  /**
>   * drm_mode_parse_command_line_for_connector - parse command line modeline for connector
>   * @mode_option: optional per connector mode option
> @@ -1558,9 +1623,10 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option,
>  {
>  	const char *name;
>  	bool named_mode = false, parse_extras = false;
> -	unsigned int bpp_off = 0, refresh_off = 0;
> +	unsigned int bpp_off = 0, refresh_off = 0, options_off = 0;
>  	unsigned int mode_end = 0;
>  	char *bpp_ptr = NULL, *refresh_ptr = NULL, *extra_ptr = NULL;
> +	char *options_ptr = NULL;
>  	char *bpp_end_ptr = NULL, *refresh_end_ptr = NULL;
>  	int ret;
>  
> @@ -1601,13 +1667,18 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option,
>  		mode->refresh_specified = true;
>  	}
>  
> +	/* Locate the start of named options */
> +	options_ptr = strchr(name, ',');
> +	if (options_ptr)
> +		options_off = options_ptr - name;
> +
>  	/* Locate the end of the name / resolution, and parse it */
> -	if (bpp_ptr && refresh_ptr) {
> -		mode_end = min(bpp_off, refresh_off);
> -	} else if (bpp_ptr) {
> +	if (bpp_ptr) {
>  		mode_end = bpp_off;
>  	} else if (refresh_ptr) {
>  		mode_end = refresh_off;
> +	} else if (options_ptr) {
> +		mode_end = options_off;
>  	} else {
>  		mode_end = strlen(name);
>  		parse_extras = true;
> @@ -1649,24 +1720,23 @@ bool drm_mode_parse_command_line_for_connector(const char *mode_option,
>  	else if (refresh_ptr)
>  		extra_ptr = refresh_end_ptr;
>  
> -	if (extra_ptr) {
> -		if (!named_mode) {
> -			int len = strlen(name) - (extra_ptr - name);
> +	if (extra_ptr &&
> +	    extra_ptr != options_ptr) {
> +		int len = strlen(name) - (extra_ptr - name);
>  
> -			ret = drm_mode_parse_cmdline_extra(extra_ptr, len,
> -							   connector, mode);
> -			if (ret)
> -				return false;
> -		} else {
> -			int remaining = strlen(name) - (extra_ptr - name);
> +		ret = drm_mode_parse_cmdline_extra(extra_ptr, len,
> +						   connector, mode);
> +		if (ret)
> +			return false;
> +	}
>  
> -			/*
> -			 * We still have characters to process, while
> -			 * we shouldn't have any
> -			 */
> -			if (remaining > 0)
> -				return false;
> -		}
> +	if (options_ptr) {
> +		int len = strlen(name) - (options_ptr - name);
> +
> +		ret = drm_mode_parse_cmdline_options(options_ptr, len,
> +						     connector, mode);
> +		if (ret)
> +			return false;
>  	}
>  
>  	return true;
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 06aa3b9cb920..dfe7f2304b35 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -907,6 +907,7 @@ struct drm_cmdline_mode {
>  	bool interlace;
>  	bool cvt;
>  	bool margins;
> +	unsigned int rotation;
>  	enum drm_connector_force force;
>  };
>  
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 3/5] drm/modes: Allow to specify rotation and reflection on the commandline

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 2480 bytes --]

Hi Noralf,

On Tue, Apr 16, 2019 at 04:50:00PM +0200, Noralf Trønnes wrote:
> Den 11.04.2019 15.22, skrev Maxime Ripard:
> > Rotations and reflections setup are needed in some scenarios to initialise
> > properly the initial framebuffer. Some drivers already had a bunch of
> > quirks to deal with this, such as either a private kernel command line
> > parameter (omapdss) or on the device tree (various panels).
> >
> > In order to accomodate this, let's create a video mode parameter to deal
> > with the rotation and reflexion.
> >
> > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > ---
> >  drivers/gpu/drm/drm_fb_helper.c |   4 +-
> >  drivers/gpu/drm/drm_modes.c     | 110 +++++++++++++++++++++++++++------
> >  include/drm/drm_connector.h     |   1 +-
> >  3 files changed, 95 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> > index b3a5d79436ae..8781897559b2 100644
> > --- a/drivers/gpu/drm/drm_fb_helper.c
> > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > @@ -2521,6 +2521,7 @@ static void drm_setup_crtc_rotation(struct drm_fb_helper *fb_helper,
> >  				    struct drm_connector *connector)
> >  {
> >  	struct drm_plane *plane = fb_crtc->mode_set.crtc->primary;
> > +	struct drm_cmdline_mode *mode = &connector->cmdline_mode;
> >  	uint64_t valid_mask = 0;
> >  	int i, rotation;
> >
> > @@ -2540,6 +2541,9 @@ static void drm_setup_crtc_rotation(struct drm_fb_helper *fb_helper,
> >  		rotation = DRM_MODE_ROTATE_0;
> >  	}
> >
> > +	if (mode->rotation != DRM_MODE_ROTATE_0)
> > +		fb_crtc->rotation = mode->rotation;
> > +
>
> We already have a property to describe initial display/panel rotation.
> If we can set connector->display_info.panel_orientation from the video=
> parameter, then there's no need to modify drm_fb_helper, it will just work.
>
> In that case, maybe 'orientation' is a better argument name with values
> mapped to the enum.

I wouldn't put it at the same level though. As far as I understand it,
the orientation is a hardware constraint: the hardware has been
designed that way, and should honor that orientation to make it look
with the top, well, on top.

However, the rotation is more of a user choice, and you could
definitely envision having a combination of a rotation and an
orientation constraint.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 3/5] drm/modes: Allow to specify rotation and reflection on the commandline

[Noralf Trønnes]

Den 17.04.2019 16.30, skrev Maxime Ripard:
> Hi Noralf,
> 
> On Tue, Apr 16, 2019 at 04:50:00PM +0200, Noralf Trønnes wrote:
>> Den 11.04.2019 15.22, skrev Maxime Ripard:
>>> Rotations and reflections setup are needed in some scenarios to initialise
>>> properly the initial framebuffer. Some drivers already had a bunch of
>>> quirks to deal with this, such as either a private kernel command line
>>> parameter (omapdss) or on the device tree (various panels).
>>>
>>> In order to accomodate this, let's create a video mode parameter to deal
>>> with the rotation and reflexion.
>>>
>>> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
>>> ---
>>>  drivers/gpu/drm/drm_fb_helper.c |   4 +-
>>>  drivers/gpu/drm/drm_modes.c     | 110 +++++++++++++++++++++++++++------
>>>  include/drm/drm_connector.h     |   1 +-
>>>  3 files changed, 95 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
>>> index b3a5d79436ae..8781897559b2 100644
>>> --- a/drivers/gpu/drm/drm_fb_helper.c
>>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>>> @@ -2521,6 +2521,7 @@ static void drm_setup_crtc_rotation(struct drm_fb_helper *fb_helper,
>>>  				    struct drm_connector *connector)
>>>  {
>>>  	struct drm_plane *plane = fb_crtc->mode_set.crtc->primary;
>>> +	struct drm_cmdline_mode *mode = &connector->cmdline_mode;
>>>  	uint64_t valid_mask = 0;
>>>  	int i, rotation;
>>>
>>> @@ -2540,6 +2541,9 @@ static void drm_setup_crtc_rotation(struct drm_fb_helper *fb_helper,
>>>  		rotation = DRM_MODE_ROTATE_0;
>>>  	}
>>>
>>> +	if (mode->rotation != DRM_MODE_ROTATE_0)
>>> +		fb_crtc->rotation = mode->rotation;
>>> +
>>
>> We already have a property to describe initial display/panel rotation.
>> If we can set connector->display_info.panel_orientation from the video=
>> parameter, then there's no need to modify drm_fb_helper, it will just work.
>>
>> In that case, maybe 'orientation' is a better argument name with values
>> mapped to the enum.
> 
> I wouldn't put it at the same level though. As far as I understand it,
> the orientation is a hardware constraint: the hardware has been
> designed that way, and should honor that orientation to make it look
> with the top, well, on top.
> 
> However, the rotation is more of a user choice, and you could
> definitely envision having a combination of a rotation and an
> orientation constraint.
> 

Does this rotation only apply to the fbdev emulation and should not
propogate to DRM userspace?

I actually don't understand how the DRM plane rotation works for 90/270
rotation. Should the framebuffer width/height be swapped when attaching
to a rotated plane?

Btw drm_setup_crtc_rotation() doesn't support 90/270 plane rotation even
if the hw supports it. It will instead sw rotate fbcon, but fbdev
userspace remains unrotated.

Noralf.

> Maxime
> 
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 3/5] drm/modes: Allow to specify rotation and reflection on the commandline

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 3497 bytes --]

On Wed, Apr 17, 2019 at 04:58:07PM +0200, Noralf Trønnes wrote:
> Den 17.04.2019 16.30, skrev Maxime Ripard:
> > On Tue, Apr 16, 2019 at 04:50:00PM +0200, Noralf Trønnes wrote:
> >> Den 11.04.2019 15.22, skrev Maxime Ripard:
> >>> Rotations and reflections setup are needed in some scenarios to initialise
> >>> properly the initial framebuffer. Some drivers already had a bunch of
> >>> quirks to deal with this, such as either a private kernel command line
> >>> parameter (omapdss) or on the device tree (various panels).
> >>>
> >>> In order to accomodate this, let's create a video mode parameter to deal
> >>> with the rotation and reflexion.
> >>>
> >>> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> >>> ---
> >>>  drivers/gpu/drm/drm_fb_helper.c |   4 +-
> >>>  drivers/gpu/drm/drm_modes.c     | 110 +++++++++++++++++++++++++++------
> >>>  include/drm/drm_connector.h     |   1 +-
> >>>  3 files changed, 95 insertions(+), 20 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> >>> index b3a5d79436ae..8781897559b2 100644
> >>> --- a/drivers/gpu/drm/drm_fb_helper.c
> >>> +++ b/drivers/gpu/drm/drm_fb_helper.c
> >>> @@ -2521,6 +2521,7 @@ static void drm_setup_crtc_rotation(struct drm_fb_helper *fb_helper,
> >>>  				    struct drm_connector *connector)
> >>>  {
> >>>  	struct drm_plane *plane = fb_crtc->mode_set.crtc->primary;
> >>> +	struct drm_cmdline_mode *mode = &connector->cmdline_mode;
> >>>  	uint64_t valid_mask = 0;
> >>>  	int i, rotation;
> >>>
> >>> @@ -2540,6 +2541,9 @@ static void drm_setup_crtc_rotation(struct drm_fb_helper *fb_helper,
> >>>  		rotation = DRM_MODE_ROTATE_0;
> >>>  	}
> >>>
> >>> +	if (mode->rotation != DRM_MODE_ROTATE_0)
> >>> +		fb_crtc->rotation = mode->rotation;
> >>> +
> >>
> >> We already have a property to describe initial display/panel rotation.
> >> If we can set connector->display_info.panel_orientation from the video=
> >> parameter, then there's no need to modify drm_fb_helper, it will just work.
> >>
> >> In that case, maybe 'orientation' is a better argument name with values
> >> mapped to the enum.
> >
> > I wouldn't put it at the same level though. As far as I understand it,
> > the orientation is a hardware constraint: the hardware has been
> > designed that way, and should honor that orientation to make it look
> > with the top, well, on top.
> >
> > However, the rotation is more of a user choice, and you could
> > definitely envision having a combination of a rotation and an
> > orientation constraint.
> >
>
> Does this rotation only apply to the fbdev emulation and should not
> propogate to DRM userspace?

I guess it definitely should be propagated to the DRM userspace.

> I actually don't understand how the DRM plane rotation works for 90/270
> rotation. Should the framebuffer width/height be swapped when attaching
> to a rotated plane?

I'm not sure either, especially since the stride might be a bit
different too. However, since (as you pointed out) it would be much
harder to support, I left it out of that patch series for the moment.

> Btw drm_setup_crtc_rotation() doesn't support 90/270 plane rotation even
> if the hw supports it. It will instead sw rotate fbcon, but fbdev
> userspace remains unrotated.

That would need to be addressed eventually yep.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 4/5] drm/modes: Parse overscan properties

[Maxime Ripard]

Properly configuring the overscan properties might be needed for the
initial setup of the framebuffer for display that still have overscan.
Let's allow for more properties on the kernel command line to setup each
margin.

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 drivers/gpu/drm/drm_fb_helper.c | 47 ++++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/drm_modes.c     | 44 ++++++++++++++++++++++++++++++++-
 include/drm/drm_connector.h     |  1 +-
 3 files changed, 92 insertions(+)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 8781897559b2..4e403fe1f451 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -2567,6 +2567,51 @@ static void drm_setup_crtc_rotation(struct drm_fb_helper *fb_helper,
 	fb_helper->sw_rotations |= DRM_MODE_ROTATE_0;
 }
 
+static void drm_setup_connector_margins(struct drm_connector *connector)
+{
+	struct drm_cmdline_mode *cmdline = &connector->cmdline_mode;
+	struct drm_modeset_acquire_ctx ctx;
+	struct drm_atomic_state *state;
+	struct drm_device *dev = connector->dev;
+	int ret;
+
+	if (!drm_drv_uses_atomic_modeset(dev))
+		return;
+
+	drm_modeset_acquire_init(&ctx, 0);
+
+	state = drm_atomic_state_alloc(dev);
+	state->acquire_ctx = &ctx;
+
+retry:
+	drm_atomic_set_property(state, &connector->base,
+				dev->mode_config.tv_left_margin_property,
+				cmdline->overscan_left);
+
+	drm_atomic_set_property(state, &connector->base,
+				dev->mode_config.tv_right_margin_property,
+				cmdline->overscan_right);
+
+	drm_atomic_set_property(state, &connector->base,
+				dev->mode_config.tv_top_margin_property,
+				cmdline->overscan_top);
+
+	drm_atomic_set_property(state, &connector->base,
+				dev->mode_config.tv_bottom_margin_property,
+				cmdline->overscan_bottom);
+
+	ret = drm_atomic_commit(state);
+	if (ret == -EDEADLK) {
+		drm_atomic_state_clear(state);
+		drm_modeset_backoff(&ctx);
+		goto retry;
+	}
+
+	drm_atomic_state_put(state);
+	drm_modeset_drop_locks(&ctx);
+	drm_modeset_acquire_fini(&ctx);
+}
+
 static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
 			    u32 width, u32 height)
 {
@@ -2680,6 +2725,8 @@ static void drm_setup_crtcs_fb(struct drm_fb_helper *fb_helper)
 		struct drm_connector *connector =
 					fb_helper->connector_info[i]->connector;
 
+		drm_setup_connector_margins(connector);
+
 		/* use first connected connector for the physical dimensions */
 		if (connector->status == connector_status_connected) {
 			info->var.width = connector->display_info.width_mm;
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index ac8d70b92b62..493ba3ccde70 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1586,6 +1586,50 @@ static int drm_mode_parse_cmdline_options(char *str, size_t len,
 		} else if (!strncmp(option, "reflect_y", delim - option)) {
 			rotation |= DRM_MODE_REFLECT_Y;
 			sep = delim;
+		} else if (!strncmp(option, "overscan_right", delim - option)) {
+			const char *value = delim + 1;
+			unsigned int margin;
+
+			margin = simple_strtol(value, &sep, 10);
+
+			/* Make sure we have parsed something */
+			if (sep == value)
+				return -EINVAL;
+
+			mode->overscan_right = margin;
+		} else if (!strncmp(option, "overscan_left", delim - option)) {
+			const char *value = delim + 1;
+			unsigned int margin;
+
+			margin = simple_strtol(value, &sep, 10);
+
+			/* Make sure we have parsed something */
+			if (sep == value)
+				return -EINVAL;
+
+			mode->overscan_left = margin;
+		} else if (!strncmp(option, "overscan_top", delim - option)) {
+			const char *value = delim + 1;
+			unsigned int margin;
+
+			margin = simple_strtol(value, &sep, 10);
+
+			/* Make sure we have parsed something */
+			if (sep == value)
+				return -EINVAL;
+
+			mode->overscan_top = margin;
+		} else if (!strncmp(option, "overscan_bottom", delim - option)) {
+			const char *value = delim + 1;
+			unsigned int margin;
+
+			margin = simple_strtol(value, &sep, 10);
+
+			/* Make sure we have parsed something */
+			if (sep == value)
+				return -EINVAL;
+
+			mode->overscan_bottom = margin;
 		} else {
 			return -EINVAL;
 		}
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index dfe7f2304b35..44d9885dd401 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -903,6 +903,7 @@ struct drm_cmdline_mode {
 	int xres, yres;
 	int bpp;
 	int refresh;
+	int overscan_right, overscan_top, overscan_left, overscan_bottom;
 	bool rb;
 	bool interlace;
 	bool cvt;
-- 
git-series 0.9.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 4/5] drm/modes: Parse overscan properties

[Daniel Vetter]

On Thu, Apr 11, 2019 at 03:22:42PM +0200, Maxime Ripard wrote:
> Properly configuring the overscan properties might be needed for the
> initial setup of the framebuffer for display that still have overscan.
> Let's allow for more properties on the kernel command line to setup each
> margin.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
>  drivers/gpu/drm/drm_fb_helper.c | 47 ++++++++++++++++++++++++++++++++++-
>  drivers/gpu/drm/drm_modes.c     | 44 ++++++++++++++++++++++++++++++++-
>  include/drm/drm_connector.h     |  1 +-
>  3 files changed, 92 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 8781897559b2..4e403fe1f451 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -2567,6 +2567,51 @@ static void drm_setup_crtc_rotation(struct drm_fb_helper *fb_helper,
>  	fb_helper->sw_rotations |= DRM_MODE_ROTATE_0;
>  }
>  
> +static void drm_setup_connector_margins(struct drm_connector *connector)
> +{
> +	struct drm_cmdline_mode *cmdline = &connector->cmdline_mode;
> +	struct drm_modeset_acquire_ctx ctx;
> +	struct drm_atomic_state *state;
> +	struct drm_device *dev = connector->dev;
> +	int ret;
> +
> +	if (!drm_drv_uses_atomic_modeset(dev))
> +		return;
> +
> +	drm_modeset_acquire_init(&ctx, 0);

You can't do more than one atomic commit, and you need to restore
everything (since userspace might have changed stuff) every time. That's
why all the atomic code goes through restore_fbdev_mode_atomic eventually.
See also how rotation is handled already.

I guess you can omit handling this for non-atomic, since no one cares.

> +
> +	state = drm_atomic_state_alloc(dev);
> +	state->acquire_ctx = &ctx;
> +
> +retry:
> +	drm_atomic_set_property(state, &connector->base,
> +				dev->mode_config.tv_left_margin_property,
> +				cmdline->overscan_left);
> +
> +	drm_atomic_set_property(state, &connector->base,
> +				dev->mode_config.tv_right_margin_property,
> +				cmdline->overscan_right);
> +
> +	drm_atomic_set_property(state, &connector->base,
> +				dev->mode_config.tv_top_margin_property,
> +				cmdline->overscan_top);
> +
> +	drm_atomic_set_property(state, &connector->base,
> +				dev->mode_config.tv_bottom_margin_property,
> +				cmdline->overscan_bottom);
> +
> +	ret = drm_atomic_commit(state);
> +	if (ret == -EDEADLK) {
> +		drm_atomic_state_clear(state);
> +		drm_modeset_backoff(&ctx);
> +		goto retry;
> +	}
> +
> +	drm_atomic_state_put(state);
> +	drm_modeset_drop_locks(&ctx);
> +	drm_modeset_acquire_fini(&ctx);
> +}
> +
>  static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
>  			    u32 width, u32 height)
>  {
> @@ -2680,6 +2725,8 @@ static void drm_setup_crtcs_fb(struct drm_fb_helper *fb_helper)
>  		struct drm_connector *connector =
>  					fb_helper->connector_info[i]->connector;
>  
> +		drm_setup_connector_margins(connector);
> +
>  		/* use first connected connector for the physical dimensions */
>  		if (connector->status == connector_status_connected) {
>  			info->var.width = connector->display_info.width_mm;
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index ac8d70b92b62..493ba3ccde70 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -1586,6 +1586,50 @@ static int drm_mode_parse_cmdline_options(char *str, size_t len,
>  		} else if (!strncmp(option, "reflect_y", delim - option)) {
>  			rotation |= DRM_MODE_REFLECT_Y;
>  			sep = delim;
> +		} else if (!strncmp(option, "overscan_right", delim - option)) {
> +			const char *value = delim + 1;
> +			unsigned int margin;
> +
> +			margin = simple_strtol(value, &sep, 10);
> +
> +			/* Make sure we have parsed something */
> +			if (sep == value)
> +				return -EINVAL;
> +
> +			mode->overscan_right = margin;
> +		} else if (!strncmp(option, "overscan_left", delim - option)) {
> +			const char *value = delim + 1;
> +			unsigned int margin;
> +
> +			margin = simple_strtol(value, &sep, 10);
> +
> +			/* Make sure we have parsed something */
> +			if (sep == value)
> +				return -EINVAL;
> +
> +			mode->overscan_left = margin;
> +		} else if (!strncmp(option, "overscan_top", delim - option)) {
> +			const char *value = delim + 1;
> +			unsigned int margin;
> +
> +			margin = simple_strtol(value, &sep, 10);
> +
> +			/* Make sure we have parsed something */
> +			if (sep == value)
> +				return -EINVAL;
> +
> +			mode->overscan_top = margin;
> +		} else if (!strncmp(option, "overscan_bottom", delim - option)) {
> +			const char *value = delim + 1;
> +			unsigned int margin;
> +
> +			margin = simple_strtol(value, &sep, 10);
> +
> +			/* Make sure we have parsed something */
> +			if (sep == value)
> +				return -EINVAL;
> +
> +			mode->overscan_bottom = margin;
>  		} else {
>  			return -EINVAL;
>  		}

This will conflict quite a bit with Noralf's series to extract the modeset
code from fb helpers. Cross review might be a good idea.
-Daniel

> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index dfe7f2304b35..44d9885dd401 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -903,6 +903,7 @@ struct drm_cmdline_mode {
>  	int xres, yres;
>  	int bpp;
>  	int refresh;
> +	int overscan_right, overscan_top, overscan_left, overscan_bottom;
>  	bool rb;
>  	bool interlace;
>  	bool cvt;
> -- 
> git-series 0.9.1
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 4/5] drm/modes: Parse overscan properties

[Noralf Trønnes]

Den 11.04.2019 15.22, skrev Maxime Ripard:
> Properly configuring the overscan properties might be needed for the
> initial setup of the framebuffer for display that still have overscan.
> Let's allow for more properties on the kernel command line to setup each
> margin.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
>  drivers/gpu/drm/drm_fb_helper.c | 47 ++++++++++++++++++++++++++++++++++-
>  drivers/gpu/drm/drm_modes.c     | 44 ++++++++++++++++++++++++++++++++-
>  include/drm/drm_connector.h     |  1 +-
>  3 files changed, 92 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 8781897559b2..4e403fe1f451 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -2567,6 +2567,51 @@ static void drm_setup_crtc_rotation(struct drm_fb_helper *fb_helper,
>  	fb_helper->sw_rotations |= DRM_MODE_ROTATE_0;
>  }
>  
> +static void drm_setup_connector_margins(struct drm_connector *connector)
> +{
> +	struct drm_cmdline_mode *cmdline = &connector->cmdline_mode;
> +	struct drm_modeset_acquire_ctx ctx;
> +	struct drm_atomic_state *state;
> +	struct drm_device *dev = connector->dev;
> +	int ret;
> +
> +	if (!drm_drv_uses_atomic_modeset(dev))
> +		return;
> +
> +	drm_modeset_acquire_init(&ctx, 0);
> +
> +	state = drm_atomic_state_alloc(dev);
> +	state->acquire_ctx = &ctx;
> +
> +retry:
> +	drm_atomic_set_property(state, &connector->base,
> +				dev->mode_config.tv_left_margin_property,
> +				cmdline->overscan_left);
> +
> +	drm_atomic_set_property(state, &connector->base,
> +				dev->mode_config.tv_right_margin_property,
> +				cmdline->overscan_right);
> +
> +	drm_atomic_set_property(state, &connector->base,
> +				dev->mode_config.tv_top_margin_property,
> +				cmdline->overscan_top);
> +
> +	drm_atomic_set_property(state, &connector->base,
> +				dev->mode_config.tv_bottom_margin_property,
> +				cmdline->overscan_bottom);
> +
> +	ret = drm_atomic_commit(state);
> +	if (ret == -EDEADLK) {
> +		drm_atomic_state_clear(state);
> +		drm_modeset_backoff(&ctx);
> +		goto retry;
> +	}
> +
> +	drm_atomic_state_put(state);
> +	drm_modeset_drop_locks(&ctx);
> +	drm_modeset_acquire_fini(&ctx);
> +}
> +

Should we set these property values in drm_connector_init()?
I assume that DRM userspace would want to use these values as well.

Noralf.

>  static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
>  			    u32 width, u32 height)
>  {
> @@ -2680,6 +2725,8 @@ static void drm_setup_crtcs_fb(struct drm_fb_helper *fb_helper)
>  		struct drm_connector *connector =
>  					fb_helper->connector_info[i]->connector;
>  
> +		drm_setup_connector_margins(connector);
> +
>  		/* use first connected connector for the physical dimensions */
>  		if (connector->status == connector_status_connected) {
>  			info->var.width = connector->display_info.width_mm;
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index ac8d70b92b62..493ba3ccde70 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -1586,6 +1586,50 @@ static int drm_mode_parse_cmdline_options(char *str, size_t len,
>  		} else if (!strncmp(option, "reflect_y", delim - option)) {
>  			rotation |= DRM_MODE_REFLECT_Y;
>  			sep = delim;
> +		} else if (!strncmp(option, "overscan_right", delim - option)) {
> +			const char *value = delim + 1;
> +			unsigned int margin;
> +
> +			margin = simple_strtol(value, &sep, 10);
> +
> +			/* Make sure we have parsed something */
> +			if (sep == value)
> +				return -EINVAL;
> +
> +			mode->overscan_right = margin;
> +		} else if (!strncmp(option, "overscan_left", delim - option)) {
> +			const char *value = delim + 1;
> +			unsigned int margin;
> +
> +			margin = simple_strtol(value, &sep, 10);
> +
> +			/* Make sure we have parsed something */
> +			if (sep == value)
> +				return -EINVAL;
> +
> +			mode->overscan_left = margin;
> +		} else if (!strncmp(option, "overscan_top", delim - option)) {
> +			const char *value = delim + 1;
> +			unsigned int margin;
> +
> +			margin = simple_strtol(value, &sep, 10);
> +
> +			/* Make sure we have parsed something */
> +			if (sep == value)
> +				return -EINVAL;
> +
> +			mode->overscan_top = margin;
> +		} else if (!strncmp(option, "overscan_bottom", delim - option)) {
> +			const char *value = delim + 1;
> +			unsigned int margin;
> +
> +			margin = simple_strtol(value, &sep, 10);
> +
> +			/* Make sure we have parsed something */
> +			if (sep == value)
> +				return -EINVAL;
> +
> +			mode->overscan_bottom = margin;
>  		} else {
>  			return -EINVAL;
>  		}
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index dfe7f2304b35..44d9885dd401 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -903,6 +903,7 @@ struct drm_cmdline_mode {
>  	int xres, yres;
>  	int bpp;
>  	int refresh;
> +	int overscan_right, overscan_top, overscan_left, overscan_bottom;
>  	bool rb;
>  	bool interlace;
>  	bool cvt;
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 4/5] drm/modes: Parse overscan properties

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 3333 bytes --]

Hi Noralf,

On Tue, Apr 16, 2019 at 04:52:20PM +0200, Noralf Trønnes wrote:
> Den 11.04.2019 15.22, skrev Maxime Ripard:
> > Properly configuring the overscan properties might be needed for the
> > initial setup of the framebuffer for display that still have overscan.
> > Let's allow for more properties on the kernel command line to setup each
> > margin.
> >
> > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > ---
> >  drivers/gpu/drm/drm_fb_helper.c | 47 ++++++++++++++++++++++++++++++++++-
> >  drivers/gpu/drm/drm_modes.c     | 44 ++++++++++++++++++++++++++++++++-
> >  include/drm/drm_connector.h     |  1 +-
> >  3 files changed, 92 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> > index 8781897559b2..4e403fe1f451 100644
> > --- a/drivers/gpu/drm/drm_fb_helper.c
> > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > @@ -2567,6 +2567,51 @@ static void drm_setup_crtc_rotation(struct drm_fb_helper *fb_helper,
> >  	fb_helper->sw_rotations |= DRM_MODE_ROTATE_0;
> >  }
> >
> > +static void drm_setup_connector_margins(struct drm_connector *connector)
> > +{
> > +	struct drm_cmdline_mode *cmdline = &connector->cmdline_mode;
> > +	struct drm_modeset_acquire_ctx ctx;
> > +	struct drm_atomic_state *state;
> > +	struct drm_device *dev = connector->dev;
> > +	int ret;
> > +
> > +	if (!drm_drv_uses_atomic_modeset(dev))
> > +		return;
> > +
> > +	drm_modeset_acquire_init(&ctx, 0);
> > +
> > +	state = drm_atomic_state_alloc(dev);
> > +	state->acquire_ctx = &ctx;
> > +
> > +retry:
> > +	drm_atomic_set_property(state, &connector->base,
> > +				dev->mode_config.tv_left_margin_property,
> > +				cmdline->overscan_left);
> > +
> > +	drm_atomic_set_property(state, &connector->base,
> > +				dev->mode_config.tv_right_margin_property,
> > +				cmdline->overscan_right);
> > +
> > +	drm_atomic_set_property(state, &connector->base,
> > +				dev->mode_config.tv_top_margin_property,
> > +				cmdline->overscan_top);
> > +
> > +	drm_atomic_set_property(state, &connector->base,
> > +				dev->mode_config.tv_bottom_margin_property,
> > +				cmdline->overscan_bottom);
> > +
> > +	ret = drm_atomic_commit(state);
> > +	if (ret == -EDEADLK) {
> > +		drm_atomic_state_clear(state);
> > +		drm_modeset_backoff(&ctx);
> > +		goto retry;
> > +	}
> > +
> > +	drm_atomic_state_put(state);
> > +	drm_modeset_drop_locks(&ctx);
> > +	drm_modeset_acquire_fini(&ctx);
> > +}
> > +
>
> Should we set these property values in drm_connector_init()?
> I assume that DRM userspace would want to use these values as well.

I'm not sure, I've looked into it, and most drivers will create those
properties and attached *after* running drm_connector_init.

If you're using drm_mode_create_tv_margin_properties, then the first
thing it does is to check whether that property has been created
already, so we're covered.

For drm_connector_attach_tv_margins_properties though, this will force
overwrite the value.

One thing I'm not really fond of either is that you would do this even
if the driver cannot support the margins at all.

Maybe we can put this into drm_connector_attach_tv_margins_properties?

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 4/5] drm/modes: Parse overscan properties

[Noralf Trønnes]

Den 17.04.2019 16.07, skrev Maxime Ripard:
> Hi Noralf,
> 
> On Tue, Apr 16, 2019 at 04:52:20PM +0200, Noralf Trønnes wrote:
>> Den 11.04.2019 15.22, skrev Maxime Ripard:
>>> Properly configuring the overscan properties might be needed for the
>>> initial setup of the framebuffer for display that still have overscan.
>>> Let's allow for more properties on the kernel command line to setup each
>>> margin.
>>>
>>> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
>>> ---
>>>  drivers/gpu/drm/drm_fb_helper.c | 47 ++++++++++++++++++++++++++++++++++-
>>>  drivers/gpu/drm/drm_modes.c     | 44 ++++++++++++++++++++++++++++++++-
>>>  include/drm/drm_connector.h     |  1 +-
>>>  3 files changed, 92 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
>>> index 8781897559b2..4e403fe1f451 100644
>>> --- a/drivers/gpu/drm/drm_fb_helper.c
>>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>>> @@ -2567,6 +2567,51 @@ static void drm_setup_crtc_rotation(struct drm_fb_helper *fb_helper,
>>>  	fb_helper->sw_rotations |= DRM_MODE_ROTATE_0;
>>>  }
>>>
>>> +static void drm_setup_connector_margins(struct drm_connector *connector)
>>> +{
>>> +	struct drm_cmdline_mode *cmdline = &connector->cmdline_mode;
>>> +	struct drm_modeset_acquire_ctx ctx;
>>> +	struct drm_atomic_state *state;
>>> +	struct drm_device *dev = connector->dev;
>>> +	int ret;
>>> +
>>> +	if (!drm_drv_uses_atomic_modeset(dev))
>>> +		return;
>>> +
>>> +	drm_modeset_acquire_init(&ctx, 0);
>>> +
>>> +	state = drm_atomic_state_alloc(dev);
>>> +	state->acquire_ctx = &ctx;
>>> +
>>> +retry:
>>> +	drm_atomic_set_property(state, &connector->base,
>>> +				dev->mode_config.tv_left_margin_property,
>>> +				cmdline->overscan_left);
>>> +
>>> +	drm_atomic_set_property(state, &connector->base,
>>> +				dev->mode_config.tv_right_margin_property,
>>> +				cmdline->overscan_right);
>>> +
>>> +	drm_atomic_set_property(state, &connector->base,
>>> +				dev->mode_config.tv_top_margin_property,
>>> +				cmdline->overscan_top);
>>> +
>>> +	drm_atomic_set_property(state, &connector->base,
>>> +				dev->mode_config.tv_bottom_margin_property,
>>> +				cmdline->overscan_bottom);
>>> +
>>> +	ret = drm_atomic_commit(state);
>>> +	if (ret == -EDEADLK) {
>>> +		drm_atomic_state_clear(state);
>>> +		drm_modeset_backoff(&ctx);
>>> +		goto retry;
>>> +	}
>>> +
>>> +	drm_atomic_state_put(state);
>>> +	drm_modeset_drop_locks(&ctx);
>>> +	drm_modeset_acquire_fini(&ctx);
>>> +}
>>> +
>>
>> Should we set these property values in drm_connector_init()?
>> I assume that DRM userspace would want to use these values as well.
> 
> I'm not sure, I've looked into it, and most drivers will create those
> properties and attached *after* running drm_connector_init.
> 
> If you're using drm_mode_create_tv_margin_properties, then the first
> thing it does is to check whether that property has been created
> already, so we're covered.
> 
> For drm_connector_attach_tv_margins_properties though, this will force
> overwrite the value.
> 
> One thing I'm not really fond of either is that you would do this even
> if the driver cannot support the margins at all.
> 
> Maybe we can put this into drm_connector_attach_tv_margins_properties?
> 

That sounds like a good idea.
It would leave out these which doesn't use that _attach function though:
- i2c/ch7006_drv.c
- i915/intel_tv.c

Not sure how to handle those. i915 uses the connector state to set the
initial margins. Can we set the margins on the connector state in
drm_connector_init() and use those as default values in
drm_connector_attach_tv_margin_properties()?

Noralf.

> Maxime
> 
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 4/5] drm/modes: Parse overscan properties

[Noralf Trønnes]

Den 17.04.2019 17.30, skrev Noralf Trønnes:
> 
> 
> Den 17.04.2019 16.07, skrev Maxime Ripard:
>> Hi Noralf,
>>
>> On Tue, Apr 16, 2019 at 04:52:20PM +0200, Noralf Trønnes wrote:
>>> Den 11.04.2019 15.22, skrev Maxime Ripard:
>>>> Properly configuring the overscan properties might be needed for the
>>>> initial setup of the framebuffer for display that still have overscan.
>>>> Let's allow for more properties on the kernel command line to setup each
>>>> margin.
>>>>
>>>> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
>>>> ---
>>>>  drivers/gpu/drm/drm_fb_helper.c | 47 ++++++++++++++++++++++++++++++++++-
>>>>  drivers/gpu/drm/drm_modes.c     | 44 ++++++++++++++++++++++++++++++++-
>>>>  include/drm/drm_connector.h     |  1 +-
>>>>  3 files changed, 92 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
>>>> index 8781897559b2..4e403fe1f451 100644
>>>> --- a/drivers/gpu/drm/drm_fb_helper.c
>>>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>>>> @@ -2567,6 +2567,51 @@ static void drm_setup_crtc_rotation(struct drm_fb_helper *fb_helper,
>>>>  	fb_helper->sw_rotations |= DRM_MODE_ROTATE_0;
>>>>  }
>>>>
>>>> +static void drm_setup_connector_margins(struct drm_connector *connector)
>>>> +{
>>>> +	struct drm_cmdline_mode *cmdline = &connector->cmdline_mode;
>>>> +	struct drm_modeset_acquire_ctx ctx;
>>>> +	struct drm_atomic_state *state;
>>>> +	struct drm_device *dev = connector->dev;
>>>> +	int ret;
>>>> +
>>>> +	if (!drm_drv_uses_atomic_modeset(dev))
>>>> +		return;
>>>> +
>>>> +	drm_modeset_acquire_init(&ctx, 0);
>>>> +
>>>> +	state = drm_atomic_state_alloc(dev);
>>>> +	state->acquire_ctx = &ctx;
>>>> +
>>>> +retry:
>>>> +	drm_atomic_set_property(state, &connector->base,
>>>> +				dev->mode_config.tv_left_margin_property,
>>>> +				cmdline->overscan_left);
>>>> +
>>>> +	drm_atomic_set_property(state, &connector->base,
>>>> +				dev->mode_config.tv_right_margin_property,
>>>> +				cmdline->overscan_right);
>>>> +
>>>> +	drm_atomic_set_property(state, &connector->base,
>>>> +				dev->mode_config.tv_top_margin_property,
>>>> +				cmdline->overscan_top);
>>>> +
>>>> +	drm_atomic_set_property(state, &connector->base,
>>>> +				dev->mode_config.tv_bottom_margin_property,
>>>> +				cmdline->overscan_bottom);
>>>> +
>>>> +	ret = drm_atomic_commit(state);
>>>> +	if (ret == -EDEADLK) {
>>>> +		drm_atomic_state_clear(state);
>>>> +		drm_modeset_backoff(&ctx);
>>>> +		goto retry;
>>>> +	}
>>>> +
>>>> +	drm_atomic_state_put(state);
>>>> +	drm_modeset_drop_locks(&ctx);
>>>> +	drm_modeset_acquire_fini(&ctx);
>>>> +}
>>>> +
>>>
>>> Should we set these property values in drm_connector_init()?
>>> I assume that DRM userspace would want to use these values as well.
>>
>> I'm not sure, I've looked into it, and most drivers will create those
>> properties and attached *after* running drm_connector_init.
>>
>> If you're using drm_mode_create_tv_margin_properties, then the first
>> thing it does is to check whether that property has been created
>> already, so we're covered.
>>
>> For drm_connector_attach_tv_margins_properties though, this will force
>> overwrite the value.
>>
>> One thing I'm not really fond of either is that you would do this even
>> if the driver cannot support the margins at all.
>>
>> Maybe we can put this into drm_connector_attach_tv_margins_properties?
>>
> 
> That sounds like a good idea.
> It would leave out these which doesn't use that _attach function though:
> - i2c/ch7006_drv.c
> - i915/intel_tv.c
> 
> Not sure how to handle those. i915 uses the connector state to set the
> initial margins. Can we set the margins on the connector state in
> drm_connector_init() and use those as default values in
> drm_connector_attach_tv_margin_properties()?

Nevermind, that won't work as there's no state yet at that point if I'm
not mistaken.

> 
> Noralf.
> 
>> Maxime
>>
>> --
>> Maxime Ripard, Bootlin
>> Embedded Linux and Kernel engineering
>> https://bootlin.com
>>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 5/5] drm/selftests: Add command line parser selftests

[Maxime Ripard]

The command line parser is pretty tough to get right and very error prone,
so let's add a selftest to try to catch any regression.

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 drivers/gpu/drm/selftests/Makefile                  |   2 +-
 drivers/gpu/drm/selftests/drm_cmdline_selftests.h   |  49 +-
 drivers/gpu/drm/selftests/test-drm_cmdline_parser.c | 843 +++++++++++++-
 3 files changed, 893 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/selftests/drm_cmdline_selftests.h
 create mode 100644 drivers/gpu/drm/selftests/test-drm_cmdline_parser.c

diff --git a/drivers/gpu/drm/selftests/Makefile b/drivers/gpu/drm/selftests/Makefile
index 1bb73dc4c88c..971cfe17be68 100644
--- a/drivers/gpu/drm/selftests/Makefile
+++ b/drivers/gpu/drm/selftests/Makefile
@@ -1,5 +1,5 @@
 test-drm_modeset-y := test-drm_modeset_common.o test-drm_plane_helper.o \
                       test-drm_format.o test-drm_framebuffer.o \
-		      test-drm_damage_helper.o
+		      test-drm_damage_helper.o test-drm_cmdline_parser.o
 
 obj-$(CONFIG_DRM_DEBUG_SELFTEST) += test-drm_mm.o test-drm_modeset.o
diff --git a/drivers/gpu/drm/selftests/drm_cmdline_selftests.h b/drivers/gpu/drm/selftests/drm_cmdline_selftests.h
new file mode 100644
index 000000000000..86afe546fd0e
--- /dev/null
+++ b/drivers/gpu/drm/selftests/drm_cmdline_selftests.h
@@ -0,0 +1,49 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* List each unit test as selftest(function)
+ *
+ * The name is used as both an enum and expanded as igt__name to create
+ * a module parameter. It must be unique and legal for a C identifier.
+ *
+ * Tests are executed in order by igt/drm_mm
+ */
+
+#define cmdline_test(test)	selftest(test, test)
+
+cmdline_test(drm_cmdline_test_res)
+cmdline_test(drm_cmdline_test_res_vesa)
+cmdline_test(drm_cmdline_test_res_vesa_rblank)
+cmdline_test(drm_cmdline_test_res_rblank)
+cmdline_test(drm_cmdline_test_res_bpp)
+cmdline_test(drm_cmdline_test_res_refresh)
+cmdline_test(drm_cmdline_test_res_bpp_refresh)
+cmdline_test(drm_cmdline_test_res_bpp_refresh_interlaced)
+cmdline_test(drm_cmdline_test_res_bpp_refresh_margins)
+cmdline_test(drm_cmdline_test_res_bpp_refresh_force_off)
+cmdline_test(drm_cmdline_test_res_bpp_refresh_force_on_off)
+cmdline_test(drm_cmdline_test_res_bpp_refresh_force_on)
+cmdline_test(drm_cmdline_test_res_bpp_refresh_force_on_analog)
+cmdline_test(drm_cmdline_test_res_bpp_refresh_force_on_digital)
+cmdline_test(drm_cmdline_test_res_bpp_refresh_interlaced_margins_force_on)
+cmdline_test(drm_cmdline_test_res_margins_force_on)
+cmdline_test(drm_cmdline_test_res_vesa_margins)
+cmdline_test(drm_cmdline_test_res_invalid_mode)
+cmdline_test(drm_cmdline_test_res_bpp_wrong_place_mode)
+cmdline_test(drm_cmdline_test_name)
+cmdline_test(drm_cmdline_test_name_bpp)
+cmdline_test(drm_cmdline_test_name_refresh)
+cmdline_test(drm_cmdline_test_name_bpp_refresh)
+cmdline_test(drm_cmdline_test_name_refresh_wrong_mode)
+cmdline_test(drm_cmdline_test_name_refresh_invalid_mode)
+cmdline_test(drm_cmdline_test_name_option)
+cmdline_test(drm_cmdline_test_name_bpp_option)
+cmdline_test(drm_cmdline_test_rotate_0)
+cmdline_test(drm_cmdline_test_rotate_90)
+cmdline_test(drm_cmdline_test_rotate_180)
+cmdline_test(drm_cmdline_test_rotate_270)
+cmdline_test(drm_cmdline_test_rotate_invalid_val)
+cmdline_test(drm_cmdline_test_rotate_truncated)
+cmdline_test(drm_cmdline_test_hmirror)
+cmdline_test(drm_cmdline_test_vmirror)
+cmdline_test(drm_cmdline_test_overscan_options)
+cmdline_test(drm_cmdline_test_multiple_options)
+cmdline_test(drm_cmdline_test_invalid_option)
diff --git a/drivers/gpu/drm/selftests/test-drm_cmdline_parser.c b/drivers/gpu/drm/selftests/test-drm_cmdline_parser.c
new file mode 100644
index 000000000000..ae94b076acd4
--- /dev/null
+++ b/drivers/gpu/drm/selftests/test-drm_cmdline_parser.c
@@ -0,0 +1,843 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2019 Bootlin
+ */
+
+#define pr_fmt(fmt) "drm_cmdline: " fmt
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+
+#include <drm/drm_connector.h>
+#include <drm/drm_modes.h>
+
+#define TESTS "drm_cmdline_selftests.h"
+#include "drm_selftest.h"
+#include "test-drm_modeset_common.h"
+
+static int drm_cmdline_test_res(void *ignored)
+{
+	struct drm_connector connector = { 0 };
+	struct drm_cmdline_mode mode = { 0 };
+
+	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480",
+							      &connector,
+							      &mode));
+	FAIL_ON(!mode.specified);
+	FAIL_ON(mode.xres != 720);
+	FAIL_ON(mode.yres != 480);
+
+	FAIL_ON(mode.refresh_specified);
+
+	FAIL_ON(mode.bpp_specified);
+
+	FAIL_ON(mode.rb);
+	FAIL_ON(mode.cvt);
+	FAIL_ON(mode.interlace);
+	FAIL_ON(mode.margins);
+	FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);
+
+	return 0;
+}
+
+static int drm_cmdline_test_res_vesa(void *ignored)
+{
+	struct drm_connector connector = { 0 };
+	struct drm_cmdline_mode mode = { 0 };
+
+	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480M",
+							      &connector,
+							      &mode));
+	FAIL_ON(!mode.specified);
+	FAIL_ON(mode.xres != 720);
+	FAIL_ON(mode.yres != 480);
+
+	FAIL_ON(mode.refresh_specified);
+
+	FAIL_ON(mode.bpp_specified);
+
+	FAIL_ON(mode.rb);
+	FAIL_ON(!mode.cvt);
+	FAIL_ON(mode.interlace);
+	FAIL_ON(mode.margins);
+	FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);
+
+	return 0;
+}
+
+static int drm_cmdline_test_res_vesa_rblank(void *ignored)
+{
+	struct drm_connector connector = { 0 };
+	struct drm_cmdline_mode mode = { 0 };
+
+	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480MR",
+							      &connector,
+							      &mode));
+	FAIL_ON(!mode.specified);
+	FAIL_ON(mode.xres != 720);
+	FAIL_ON(mode.yres != 480);
+
+	FAIL_ON(mode.refresh_specified);
+
+	FAIL_ON(mode.bpp_specified);
+
+	FAIL_ON(!mode.rb);
+	FAIL_ON(!mode.cvt);
+	FAIL_ON(mode.interlace);
+	FAIL_ON(mode.margins);
+	FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);
+
+	return 0;
+}
+
+static int drm_cmdline_test_res_rblank(void *ignored)
+{
+	struct drm_connector connector = { 0 };
+	struct drm_cmdline_mode mode = { 0 };
+
+	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480R",
+							      &connector,
+							      &mode));
+	FAIL_ON(!mode.specified);
+	FAIL_ON(mode.xres != 720);
+	FAIL_ON(mode.yres != 480);
+
+	FAIL_ON(mode.refresh_specified);
+
+	FAIL_ON(mode.bpp_specified);
+
+	FAIL_ON(!mode.rb);
+	FAIL_ON(mode.cvt);
+	FAIL_ON(mode.interlace);
+	FAIL_ON(mode.margins);
+	FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);
+
+	return 0;
+}
+
+static int drm_cmdline_test_res_bpp(void *ignored)
+{
+	struct drm_connector connector = { 0 };
+	struct drm_cmdline_mode mode = { 0 };
+
+	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480-24",
+							      &connector,
+							      &mode));
+	FAIL_ON(!mode.specified);
+	FAIL_ON(mode.xres != 720);
+	FAIL_ON(mode.yres != 480);
+
+	FAIL_ON(mode.refresh_specified);
+
+	FAIL_ON(!mode.bpp_specified);
+	FAIL_ON(mode.bpp != 24);
+
+	FAIL_ON(mode.rb);
+	FAIL_ON(mode.cvt);
+	FAIL_ON(mode.interlace);
+	FAIL_ON(mode.margins);
+	FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);
+
+	return 0;
+}
+
+static int drm_cmdline_test_res_refresh(void *ignored)
+{
+	struct drm_connector connector = { 0 };
+	struct drm_cmdline_mode mode = { 0 };
+
+	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480@60",
+							      &connector,
+							      &mode));
+	FAIL_ON(!mode.specified);
+	FAIL_ON(mode.xres != 720);
+	FAIL_ON(mode.yres != 480);
+
+	FAIL_ON(!mode.refresh_specified);
+	FAIL_ON(mode.refresh != 60);
+
+	FAIL_ON(mode.bpp_specified);
+
+	FAIL_ON(mode.rb);
+	FAIL_ON(mode.cvt);
+	FAIL_ON(mode.interlace);
+	FAIL_ON(mode.margins);
+	FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);
+
+	return 0;
+}
+
+static int drm_cmdline_test_res_bpp_refresh(void *ignored)
+{
+	struct drm_connector connector = { 0 };
+	struct drm_cmdline_mode mode = { 0 };
+
+	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480-24@60",
+							      &connector,
+							      &mode));
+	FAIL_ON(!mode.specified);
+	FAIL_ON(mode.xres != 720);
+	FAIL_ON(mode.yres != 480);
+
+	FAIL_ON(!mode.refresh_specified);
+	FAIL_ON(mode.refresh != 60);
+
+	FAIL_ON(!mode.bpp_specified);
+	FAIL_ON(mode.bpp != 24);
+
+	FAIL_ON(mode.rb);
+	FAIL_ON(mode.cvt);
+	FAIL_ON(mode.interlace);
+	FAIL_ON(mode.margins);
+	FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);
+
+	return 0;
+}
+
+static int drm_cmdline_test_res_bpp_refresh_interlaced(void *ignored)
+{
+	struct drm_connector connector = { 0 };
+	struct drm_cmdline_mode mode = { 0 };
+
+	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480-24@60i",
+							      &connector,
+							      &mode));
+	FAIL_ON(!mode.specified);
+	FAIL_ON(mode.xres != 720);
+	FAIL_ON(mode.yres != 480);
+
+	FAIL_ON(!mode.refresh_specified);
+	FAIL_ON(mode.refresh != 60);
+
+	FAIL_ON(!mode.bpp_specified);
+	FAIL_ON(mode.bpp != 24);
+
+	FAIL_ON(mode.rb);
+	FAIL_ON(mode.cvt);
+	FAIL_ON(!mode.interlace);
+	FAIL_ON(mode.margins);
+	FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);
+
+	return 0;
+}
+
+static int drm_cmdline_test_res_bpp_refresh_margins(void *ignored)
+{
+	struct drm_connector connector = { 0 };
+	struct drm_cmdline_mode mode = { 0 };
+
+	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480-24@60m",
+							      &connector,
+							      &mode));
+	FAIL_ON(!mode.specified);
+	FAIL_ON(mode.xres != 720);
+	FAIL_ON(mode.yres != 480);
+
+	FAIL_ON(!mode.refresh_specified);
+	FAIL_ON(mode.refresh != 60);
+
+	FAIL_ON(!mode.bpp_specified);
+	FAIL_ON(mode.bpp != 24);
+
+	FAIL_ON(mode.rb);
+	FAIL_ON(mode.cvt);
+	FAIL_ON(mode.interlace);
+	FAIL_ON(!mode.margins);
+	FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);
+
+	return 0;
+}
+
+static int drm_cmdline_test_res_bpp_refresh_force_off(void *ignored)
+{
+	struct drm_connector connector = { 0 };
+	struct drm_cmdline_mode mode = { 0 };
+
+	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480-24@60d",
+							      &connector,
+							      &mode));
+	FAIL_ON(!mode.specified);
+	FAIL_ON(mode.xres != 720);
+	FAIL_ON(mode.yres != 480);
+
+	FAIL_ON(!mode.refresh_specified);
+	FAIL_ON(mode.refresh != 60);
+
+	FAIL_ON(!mode.bpp_specified);
+	FAIL_ON(mode.bpp != 24);
+
+	FAIL_ON(mode.rb);
+	FAIL_ON(mode.cvt);
+	FAIL_ON(mode.interlace);
+	FAIL_ON(mode.margins);
+	FAIL_ON(mode.force != DRM_FORCE_OFF);
+
+	return 0;
+}
+
+static int drm_cmdline_test_res_bpp_refresh_force_on_off(void *ignored)
+{
+	struct drm_connector connector = { 0 };
+	struct drm_cmdline_mode mode = { 0 };
+
+	FAIL_ON(drm_mode_parse_command_line_for_connector("720x480-24@60de",
+							       &connector,
+							       &mode));
+
+	return 0;
+}
+
+static int drm_cmdline_test_res_bpp_refresh_force_on(void *ignored)
+{
+	struct drm_connector connector = { 0 };
+	struct drm_cmdline_mode mode = { 0 };
+
+	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480-24@60e",
+							      &connector,
+							      &mode));
+	FAIL_ON(!mode.specified);
+	FAIL_ON(mode.xres != 720);
+	FAIL_ON(mode.yres != 480);
+
+	FAIL_ON(!mode.refresh_specified);
+	FAIL_ON(mode.refresh != 60);
+
+	FAIL_ON(!mode.bpp_specified);
+	FAIL_ON(mode.bpp != 24);
+
+	FAIL_ON(mode.rb);
+	FAIL_ON(mode.cvt);
+	FAIL_ON(mode.interlace);
+	FAIL_ON(mode.margins);
+	FAIL_ON(mode.force != DRM_FORCE_ON);
+
+	return 0;
+}
+
+static int drm_cmdline_test_res_bpp_refresh_force_on_analog(void *ignored)
+{
+	struct drm_connector connector = { 0 };
+	struct drm_cmdline_mode mode = { 0 };
+
+	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480-24@60D",
+							      &connector,
+							      &mode));
+	FAIL_ON(!mode.specified);
+	FAIL_ON(mode.xres != 720);
+	FAIL_ON(mode.yres != 480);
+
+	FAIL_ON(!mode.refresh_specified);
+	FAIL_ON(mode.refresh != 60);
+
+	FAIL_ON(!mode.bpp_specified);
+	FAIL_ON(mode.bpp != 24);
+
+	FAIL_ON(mode.rb);
+	FAIL_ON(mode.cvt);
+	FAIL_ON(mode.interlace);
+	FAIL_ON(mode.margins);
+	FAIL_ON(mode.force != DRM_FORCE_ON);
+
+	return 0;
+}
+
+static int drm_cmdline_test_res_bpp_refresh_force_on_digital(void *ignored)
+{
+	struct drm_connector connector = { 0 };
+	struct drm_cmdline_mode mode = { 0 };
+
+	connector.connector_type = DRM_MODE_CONNECTOR_DVII;
+	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480-24@60D",
+							      &connector,
+							      &mode));
+	FAIL_ON(!mode.specified);
+	FAIL_ON(mode.xres != 720);
+	FAIL_ON(mode.yres != 480);
+
+	FAIL_ON(!mode.refresh_specified);
+	FAIL_ON(mode.refresh != 60);
+
+	FAIL_ON(!mode.bpp_specified);
+	FAIL_ON(mode.bpp != 24);
+
+	FAIL_ON(mode.rb);
+	FAIL_ON(mode.cvt);
+	FAIL_ON(mode.interlace);
+	FAIL_ON(mode.margins);
+	FAIL_ON(mode.force != DRM_FORCE_ON_DIGITAL);
+
+	return 0;
+}
+
+static int drm_cmdline_test_res_bpp_refresh_interlaced_margins_force_on(void *ignored)
+{
+	struct drm_connector connector = { 0 };
+	struct drm_cmdline_mode mode = { 0 };
+
+	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480-24@60ime",
+							      &connector,
+							      &mode));
+	FAIL_ON(!mode.specified);
+	FAIL_ON(mode.xres != 720);
+	FAIL_ON(mode.yres != 480);
+
+	FAIL_ON(!mode.refresh_specified);
+	FAIL_ON(mode.refresh != 60);
+
+	FAIL_ON(!mode.bpp_specified);
+	FAIL_ON(mode.bpp != 24);
+
+	FAIL_ON(mode.rb);
+	FAIL_ON(mode.cvt);
+	FAIL_ON(!mode.interlace);
+	FAIL_ON(!mode.margins);
+	FAIL_ON(mode.force != DRM_FORCE_ON);
+
+	return 0;
+}
+
+static int drm_cmdline_test_res_margins_force_on(void *ignored)
+{
+	struct drm_connector connector = { 0 };
+	struct drm_cmdline_mode mode = { 0 };
+
+	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480me",
+							      &connector,
+							      &mode));
+	FAIL_ON(!mode.specified);
+	FAIL_ON(mode.xres != 720);
+	FAIL_ON(mode.yres != 480);
+
+	FAIL_ON(mode.refresh_specified);
+
+	FAIL_ON(mode.bpp_specified);
+
+	FAIL_ON(mode.rb);
+	FAIL_ON(mode.cvt);
+	FAIL_ON(mode.interlace);
+	FAIL_ON(!mode.margins);
+	FAIL_ON(mode.force != DRM_FORCE_ON);
+
+	return 0;
+}
+
+static int drm_cmdline_test_res_vesa_margins(void *ignored)
+{
+	struct drm_connector connector = { 0 };
+	struct drm_cmdline_mode mode = { 0 };
+
+	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480Mm",
+							      &connector,
+							      &mode));
+	FAIL_ON(!mode.specified);
+	FAIL_ON(mode.xres != 720);
+	FAIL_ON(mode.yres != 480);
+
+	FAIL_ON(mode.refresh_specified);
+
+	FAIL_ON(mode.bpp_specified);
+
+	FAIL_ON(mode.rb);
+	FAIL_ON(!mode.cvt);
+	FAIL_ON(mode.interlace);
+	FAIL_ON(!mode.margins);
+	FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);
+
+	return 0;
+}
+
+static int drm_cmdline_test_res_invalid_mode(void *ignored)
+{
+	struct drm_connector connector = { 0 };
+	struct drm_cmdline_mode mode = { 0 };
+
+	FAIL_ON(drm_mode_parse_command_line_for_connector("720x480f",
+							      &connector,
+							      &mode));
+
+	return 0;
+}
+
+static int drm_cmdline_test_res_bpp_wrong_place_mode(void *ignored)
+{
+	struct drm_connector connector = { 0 };
+	struct drm_cmdline_mode mode = { 0 };
+
+	FAIL_ON(drm_mode_parse_command_line_for_connector("720x480e-24",
+							       &connector,
+							       &mode));
+
+	return 0;
+}
+
+static int drm_cmdline_test_name(void *ignored)
+{
+	struct drm_connector connector = { 0 };
+	struct drm_cmdline_mode mode = { 0 };
+
+	FAIL_ON(!drm_mode_parse_command_line_for_connector("NTSC",
+							      &connector,
+							      &mode));
+	FAIL_ON(strcmp(mode.name, "NTSC"));
+	FAIL_ON(mode.refresh_specified);
+	FAIL_ON(mode.bpp_specified);
+
+	return 0;
+}
+
+static int drm_cmdline_test_name_bpp(void *ignored)
+{
+	struct drm_connector connector = { 0 };
+	struct drm_cmdline_mode mode = { 0 };
+
+	FAIL_ON(!drm_mode_parse_command_line_for_connector("NTSC-24",
+							      &connector,
+							      &mode));
+	FAIL_ON(strcmp(mode.name, "NTSC"));
+
+	FAIL_ON(mode.refresh_specified);
+
+	FAIL_ON(!mode.bpp_specified);
+	FAIL_ON(mode.bpp != 24);
+
+	return 0;
+}
+
+static int drm_cmdline_test_name_bpp_refresh(void *ignored)
+{
+	struct drm_connector connector = { 0 };
+	struct drm_cmdline_mode mode = { 0 };
+
+	FAIL_ON(drm_mode_parse_command_line_for_connector("NTSC-24@60",
+							       &connector,
+							       &mode));
+
+	return 0;
+}
+
+static int drm_cmdline_test_name_refresh(void *ignored)
+{
+	struct drm_connector connector = { 0 };
+	struct drm_cmdline_mode mode = { 0 };
+
+	FAIL_ON(drm_mode_parse_command_line_for_connector("NTSC@60",
+							       &connector,
+							       &mode));
+
+	return 0;
+}
+
+static int drm_cmdline_test_name_refresh_wrong_mode(void *ignored)
+{
+	struct drm_connector connector = { 0 };
+	struct drm_cmdline_mode mode = { 0 };
+
+	FAIL_ON(drm_mode_parse_command_line_for_connector("NTSC@60m",
+							       &connector,
+							       &mode));
+
+	return 0;
+}
+
+static int drm_cmdline_test_name_refresh_invalid_mode(void *ignored)
+{
+	struct drm_connector connector = { 0 };
+	struct drm_cmdline_mode mode = { 0 };
+
+	FAIL_ON(drm_mode_parse_command_line_for_connector("NTSC@60f",
+							       &connector,
+							       &mode));
+
+	return 0;
+}
+
+static int drm_cmdline_test_name_option(void *ignored)
+{
+	struct drm_connector connector = { 0 };
+	struct drm_cmdline_mode mode = { 0 };
+
+	FAIL_ON(!drm_mode_parse_command_line_for_connector("NTSC,rotate=180",
+							      &connector,
+							      &mode));
+	FAIL_ON(!mode.specified);
+	FAIL_ON(strcmp(mode.name, "NTSC"));
+	FAIL_ON(mode.rotation != DRM_MODE_ROTATE_180);
+
+	return 0;
+}
+
+static int drm_cmdline_test_name_bpp_option(void *ignored)
+{
+	struct drm_connector connector = { 0 };
+	struct drm_cmdline_mode mode = { 0 };
+
+	FAIL_ON(!drm_mode_parse_command_line_for_connector("NTSC-24,rotate=180",
+							      &connector,
+							      &mode));
+	FAIL_ON(!mode.specified);
+	FAIL_ON(strcmp(mode.name, "NTSC"));
+	FAIL_ON(mode.rotation != DRM_MODE_ROTATE_180);
+	FAIL_ON(!mode.bpp_specified);
+	FAIL_ON(mode.bpp != 24);
+
+	return 0;
+}
+
+static int drm_cmdline_test_rotate_0(void *ignored)
+{
+	struct drm_connector connector = { 0 };
+	struct drm_cmdline_mode mode = { 0 };
+
+	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480,rotate=0",
+							      &connector,
+							      &mode));
+	FAIL_ON(!mode.specified);
+	FAIL_ON(mode.xres != 720);
+	FAIL_ON(mode.yres != 480);
+	FAIL_ON(mode.rotation != DRM_MODE_ROTATE_0);
+
+	FAIL_ON(mode.refresh_specified);
+
+	FAIL_ON(mode.bpp_specified);
+
+	FAIL_ON(mode.rb);
+	FAIL_ON(mode.cvt);
+	FAIL_ON(mode.interlace);
+	FAIL_ON(mode.margins);
+	FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);
+
+	return 0;
+}
+
+static int drm_cmdline_test_rotate_90(void *ignored)
+{
+	struct drm_connector connector = { 0 };
+	struct drm_cmdline_mode mode = { 0 };
+
+	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480,rotate=90",
+							      &connector,
+							      &mode));
+	FAIL_ON(!mode.specified);
+	FAIL_ON(mode.xres != 720);
+	FAIL_ON(mode.yres != 480);
+	FAIL_ON(mode.rotation != DRM_MODE_ROTATE_90);
+
+	FAIL_ON(mode.refresh_specified);
+
+	FAIL_ON(mode.bpp_specified);
+
+	FAIL_ON(mode.rb);
+	FAIL_ON(mode.cvt);
+	FAIL_ON(mode.interlace);
+	FAIL_ON(mode.margins);
+	FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);
+
+	return 0;
+}
+
+static int drm_cmdline_test_rotate_180(void *ignored)
+{
+	struct drm_connector connector = { 0 };
+	struct drm_cmdline_mode mode = { 0 };
+
+	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480,rotate=180",
+							      &connector,
+							      &mode));
+	FAIL_ON(!mode.specified);
+	FAIL_ON(mode.xres != 720);
+	FAIL_ON(mode.yres != 480);
+	FAIL_ON(mode.rotation != DRM_MODE_ROTATE_180);
+
+	FAIL_ON(mode.refresh_specified);
+
+	FAIL_ON(mode.bpp_specified);
+
+	FAIL_ON(mode.rb);
+	FAIL_ON(mode.cvt);
+	FAIL_ON(mode.interlace);
+	FAIL_ON(mode.margins);
+	FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);
+
+	return 0;
+}
+
+static int drm_cmdline_test_rotate_270(void *ignored)
+{
+	struct drm_connector connector = { 0 };
+	struct drm_cmdline_mode mode = { 0 };
+
+	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480,rotate=270",
+							      &connector,
+							      &mode));
+	FAIL_ON(!mode.specified);
+	FAIL_ON(mode.xres != 720);
+	FAIL_ON(mode.yres != 480);
+	FAIL_ON(mode.rotation != DRM_MODE_ROTATE_270);
+
+	FAIL_ON(mode.refresh_specified);
+
+	FAIL_ON(mode.bpp_specified);
+
+	FAIL_ON(mode.rb);
+	FAIL_ON(mode.cvt);
+	FAIL_ON(mode.interlace);
+	FAIL_ON(mode.margins);
+	FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);
+
+	return 0;
+}
+
+static int drm_cmdline_test_rotate_invalid_val(void *ignored)
+{
+	struct drm_connector connector = { 0 };
+	struct drm_cmdline_mode mode = { 0 };
+
+	FAIL_ON(drm_mode_parse_command_line_for_connector("720x480,rotate=42",
+							       &connector,
+							       &mode));
+
+	return 0;
+}
+
+static int drm_cmdline_test_rotate_truncated(void *ignored)
+{
+	struct drm_connector connector = { 0 };
+	struct drm_cmdline_mode mode = { 0 };
+
+	FAIL_ON(drm_mode_parse_command_line_for_connector("720x480,rotate=",
+							       &connector,
+							       &mode));
+
+	return 0;
+}
+
+static int drm_cmdline_test_hmirror(void *ignored)
+{
+	struct drm_connector connector = { 0 };
+	struct drm_cmdline_mode mode = { 0 };
+
+	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480,reflect_x",
+							      &connector,
+							      &mode));
+	FAIL_ON(!mode.specified);
+	FAIL_ON(mode.xres != 720);
+	FAIL_ON(mode.yres != 480);
+	FAIL_ON(mode.rotation != DRM_MODE_REFLECT_X);
+
+	FAIL_ON(mode.refresh_specified);
+
+	FAIL_ON(mode.bpp_specified);
+
+	FAIL_ON(mode.rb);
+	FAIL_ON(mode.cvt);
+	FAIL_ON(mode.interlace);
+	FAIL_ON(mode.margins);
+	FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);
+
+	return 0;
+}
+
+static int drm_cmdline_test_vmirror(void *ignored)
+{
+	struct drm_connector connector = { 0 };
+	struct drm_cmdline_mode mode = { 0 };
+
+	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480,reflect_y",
+							      &connector,
+							      &mode));
+	FAIL_ON(!mode.specified);
+	FAIL_ON(mode.xres != 720);
+	FAIL_ON(mode.yres != 480);
+	FAIL_ON(mode.rotation != DRM_MODE_REFLECT_Y);
+
+	FAIL_ON(mode.refresh_specified);
+
+	FAIL_ON(mode.bpp_specified);
+
+	FAIL_ON(mode.rb);
+	FAIL_ON(mode.cvt);
+	FAIL_ON(mode.interlace);
+	FAIL_ON(mode.margins);
+	FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);
+
+	return 0;
+}
+
+static int drm_cmdline_test_overscan_options(void *ignored)
+{
+	struct drm_connector connector = { 0 };
+	struct drm_cmdline_mode mode = { 0 };
+
+	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480,overscan_right=14,overscan_left=24,overscan_bottom=36,overscan_top=42",
+							      &connector,
+							      &mode));
+	FAIL_ON(!mode.specified);
+	FAIL_ON(mode.xres != 720);
+	FAIL_ON(mode.yres != 480);
+	FAIL_ON(mode.overscan_right != 14);
+	FAIL_ON(mode.overscan_left != 24);
+	FAIL_ON(mode.overscan_bottom != 36);
+	FAIL_ON(mode.overscan_top != 42);
+
+	FAIL_ON(mode.refresh_specified);
+
+	FAIL_ON(mode.bpp_specified);
+
+	FAIL_ON(mode.rb);
+	FAIL_ON(mode.cvt);
+	FAIL_ON(mode.interlace);
+	FAIL_ON(mode.margins);
+	FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);
+
+	return 0;
+}
+
+static int drm_cmdline_test_multiple_options(void *ignored)
+{
+	struct drm_connector connector = { 0 };
+	struct drm_cmdline_mode mode = { 0 };
+
+	FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480,rotate=270,reflect_x",
+							      &connector,
+							      &mode));
+	FAIL_ON(!mode.specified);
+	FAIL_ON(mode.xres != 720);
+	FAIL_ON(mode.yres != 480);
+	FAIL_ON(mode.rotation != (DRM_MODE_ROTATE_270 | DRM_MODE_REFLECT_X));
+
+	FAIL_ON(mode.refresh_specified);
+
+	FAIL_ON(mode.bpp_specified);
+
+	FAIL_ON(mode.rb);
+	FAIL_ON(mode.cvt);
+	FAIL_ON(mode.interlace);
+	FAIL_ON(mode.margins);
+	FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);
+
+	return 0;
+}
+
+static int drm_cmdline_test_invalid_option(void *ignored)
+{
+	struct drm_connector connector = { 0 };
+	struct drm_cmdline_mode mode = { 0 };
+
+	FAIL_ON(drm_mode_parse_command_line_for_connector("720x480,test=42",
+							       &connector,
+							       &mode));
+
+	return 0;
+}
+
+#include "drm_selftest.c"
+
+static int __init test_drm_cmdline_init(void)
+{
+	int err;
+
+	err = run_selftests(selftests, ARRAY_SIZE(selftests), NULL);
+
+	return err > 0 ? 0 : err;
+}
+module_init(test_drm_cmdline_init);
-- 
git-series 0.9.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 5/5] drm/selftests: Add command line parser selftests

[kbuild test robot]

Hi Maxime,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v5.1-rc4 next-20190411]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Maxime-Ripard/drm-modes-Rewrite-the-command-line-parser/20190412-122837
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'


sparse warnings: (new ones prefixed by >>)

>> drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:20:44: sparse: Using plain integer as NULL pointer
>> drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:21:42: sparse: missing braces around initializer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:45:44: sparse: Using plain integer as NULL pointer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:46:42: sparse: missing braces around initializer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:70:44: sparse: Using plain integer as NULL pointer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:71:42: sparse: missing braces around initializer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:95:44: sparse: Using plain integer as NULL pointer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:96:42: sparse: missing braces around initializer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:120:44: sparse: Using plain integer as NULL pointer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:121:42: sparse: missing braces around initializer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:146:44: sparse: Using plain integer as NULL pointer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:147:42: sparse: missing braces around initializer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:172:44: sparse: Using plain integer as NULL pointer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:173:42: sparse: missing braces around initializer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:199:44: sparse: Using plain integer as NULL pointer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:200:42: sparse: missing braces around initializer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:226:44: sparse: Using plain integer as NULL pointer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:227:42: sparse: missing braces around initializer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:253:44: sparse: Using plain integer as NULL pointer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:254:42: sparse: missing braces around initializer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:280:44: sparse: Using plain integer as NULL pointer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:281:42: sparse: missing braces around initializer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:292:44: sparse: Using plain integer as NULL pointer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:293:42: sparse: missing braces around initializer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:319:44: sparse: Using plain integer as NULL pointer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:320:42: sparse: missing braces around initializer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:346:44: sparse: Using plain integer as NULL pointer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:347:42: sparse: missing braces around initializer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:374:44: sparse: Using plain integer as NULL pointer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:375:42: sparse: missing braces around initializer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:401:44: sparse: Using plain integer as NULL pointer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:402:42: sparse: missing braces around initializer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:426:44: sparse: Using plain integer as NULL pointer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:427:42: sparse: missing braces around initializer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:451:44: sparse: Using plain integer as NULL pointer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:452:42: sparse: missing braces around initializer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:463:44: sparse: Using plain integer as NULL pointer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:464:42: sparse: missing braces around initializer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:475:44: sparse: Using plain integer as NULL pointer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:476:42: sparse: missing braces around initializer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:490:44: sparse: Using plain integer as NULL pointer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:491:42: sparse: missing braces around initializer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:508:44: sparse: Using plain integer as NULL pointer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:509:42: sparse: missing braces around initializer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:520:44: sparse: Using plain integer as NULL pointer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:521:42: sparse: missing braces around initializer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:532:44: sparse: Using plain integer as NULL pointer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:533:42: sparse: missing braces around initializer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:544:44: sparse: Using plain integer as NULL pointer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:545:42: sparse: missing braces around initializer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:556:44: sparse: Using plain integer as NULL pointer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:557:42: sparse: missing braces around initializer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:571:44: sparse: Using plain integer as NULL pointer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:572:42: sparse: missing braces around initializer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:588:44: sparse: Using plain integer as NULL pointer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:589:42: sparse: missing braces around initializer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:614:44: sparse: Using plain integer as NULL pointer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:615:42: sparse: missing braces around initializer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:640:44: sparse: Using plain integer as NULL pointer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:641:42: sparse: missing braces around initializer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:666:44: sparse: Using plain integer as NULL pointer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:667:42: sparse: missing braces around initializer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:692:44: sparse: Using plain integer as NULL pointer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:693:42: sparse: missing braces around initializer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:704:44: sparse: Using plain integer as NULL pointer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:705:42: sparse: missing braces around initializer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:716:44: sparse: Using plain integer as NULL pointer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:717:42: sparse: missing braces around initializer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:742:44: sparse: Using plain integer as NULL pointer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:743:42: sparse: missing braces around initializer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:768:44: sparse: Using plain integer as NULL pointer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:769:42: sparse: missing braces around initializer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:797:44: sparse: Using plain integer as NULL pointer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:798:42: sparse: missing braces around initializer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:823:44: sparse: Using plain integer as NULL pointer
   drivers/gpu/drm/selftests/test-drm_cmdline_parser.c:824:42: sparse: missing braces around initializer

vim +20 drivers/gpu/drm/selftests/test-drm_cmdline_parser.c

    17	
    18	static int drm_cmdline_test_res(void *ignored)
    19	{
  > 20		struct drm_connector connector = { 0 };
  > 21		struct drm_cmdline_mode mode = { 0 };
    22	
    23		FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480",
    24								      &connector,
    25								      &mode));
    26		FAIL_ON(!mode.specified);
    27		FAIL_ON(mode.xres != 720);
    28		FAIL_ON(mode.yres != 480);
    29	
    30		FAIL_ON(mode.refresh_specified);
    31	
    32		FAIL_ON(mode.bpp_specified);
    33	
    34		FAIL_ON(mode.rb);
    35		FAIL_ON(mode.cvt);
    36		FAIL_ON(mode.interlace);
    37		FAIL_ON(mode.margins);
    38		FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);
    39	
    40		return 0;
    41	}
    42	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 5/5] drm/selftests: Add command line parser selftests

[kbuild test robot]

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

Hi Maxime,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.1-rc4 next-20190411]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Maxime-Ripard/drm-modes-Rewrite-the-command-line-parser/20190412-122837
config: x86_64-rhel (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/gpu/drm/selftests/test-drm_cmdline_parser.o: In function `test_drm_cmdline_init':
>> test-drm_cmdline_parser.c:(.init.text+0x0): multiple definition of `init_module'
   drivers/gpu/drm/selftests/test-drm_modeset_common.o:test-drm_modeset_common.c:(.init.text+0x0): first defined here

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 41740 bytes --]

[-- Attachment #3: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel