Re: [PATCH] drm, drm/i915/lvds: Honour video= parameter to override LVDS fixed mode

On Wed, 2011-02-09 at 15:01 +0000, Chris Wilson wrote:

The LVDS code ignores any connector for which it cannot find a fixed mode (through an EDID, vBIOS tables or the current active mode). Some platforms may include an LVDS header on the board and this may then be partnered with a panel without an EDID. This results in us ignoring the connector and not lighting up the panel.

Yeah not like this.

you want to make the command line the *last* option we use, the final fallback. LVDS panels have EDID and VBT hardcoded modes for a good reason, they don't work with other modes that well. You always want to use a scaler on the LVDS panel to do modes not the native mode. So if I have a VBT or EDID and you set video= I should get a scaled mode, not garbage.

So what I suspect you really want is to leave video= alone or enhance it somehow, or maybe add i915.lvds_native_mode= parameter.

Dave.

Under UMS, it was possible to override this by specifying the mode through the Xorg.conf. For KMS, one specifies the modeline through the video= parameter. So we need to include this user modeline when checking for panel fixed modes.

The machinery to parse the video= modes and generate the appropriate drm_mode is already built into drm_fb_herlper and so we can just extract, move it to the core and also use it from intel_lvds.c

Reported-by: Oliver Seitz info@vtnd.de Signed-off-by: Chris Wilson chris@chris-wilson.co.uk Cc: Dave Airlie airlied@redhat.com

---

drivers/gpu/drm/drm_fb_helper.c | 207 +++++++------------------------------ drivers/gpu/drm/drm_modes.c | 154 +++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_lvds.c | 17 +++ include/drm/drmP.h | 25 +++++ include/drm/drm_fb_helper.h | 16 +--- 5 files changed, 233 insertions(+), 186 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 6977a1c..5a80412 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -70,174 +70,50 @@ fail: } EXPORT_SYMBOL(drm_fb_helper_single_add_all_connectors);

-/**

• • drm_fb_helper_connector_parse_command_line - parse command line for connector
• • @connector - connector to parse line for
• • @mode_option - per connector mode option
• • This parses the connector specific then generic command lines for
• • modes and options to configure the connector.
• • This uses the same parameters as the fb modedb.c, except for extra
• • <xres>x<yres>[M][R][-<bpp>][@<refresh>][i][m][eDd]
• • enable/enable Digital/disable bit at the end
• */

-static bool drm_fb_helper_connector_parse_command_line(struct drm_fb_helper_connector *fb_helper_conn,

• 				       const char *mode_option)
  

-{

• const char *name;
• unsigned int namelen;
• int res_specified = 0, bpp_specified = 0, refresh_specified = 0;
• unsigned int xres = 0, yres = 0, bpp = 32, refresh = 0;
• int yres_specified = 0, cvt = 0, rb = 0, interlace = 0, margins = 0;
• int i;
• enum drm_connector_force force = DRM_FORCE_UNSPECIFIED;
• struct drm_fb_helper_cmdline_mode *cmdline_mode;
• struct drm_connector *connector;
• if (!fb_helper_conn)

• return false;
  

• connector = fb_helper_conn->connector;
• cmdline_mode = &fb_helper_conn->cmdline_mode;
• if (!mode_option)

• mode_option = fb_mode_option;
  

• if (!mode_option) {

• cmdline_mode->specified = false;
  

• return false;
  

• }
• name = mode_option;
• namelen = strlen(name);
• for (i = namelen-1; i >= 0; i--) {

• switch (name[i]) {
  

• case '@':
  

• 	namelen = i;
  

• 	if (!refresh_specified && !bpp_specified &&
  

• 	    !yres_specified) {
  

• 		refresh = simple_strtol(&name[i+1], NULL, 10);
  

• 		refresh_specified = 1;
  

• 		if (cvt || rb)
  

• 			cvt = 0;
  

• 	} else
  

• 		goto done;
  

• 	break;
  

• case '-':
  

• 	namelen = i;
  

• 	if (!bpp_specified && !yres_specified) {
  

• 		bpp = simple_strtol(&name[i+1], NULL, 10);
  

• 		bpp_specified = 1;
  

• 		if (cvt || rb)
  

• 			cvt = 0;
  

• 	} else
  

• 		goto done;
  

• 	break;
  

• case 'x':
  

• 	if (!yres_specified) {
  

• 		yres = simple_strtol(&name[i+1], NULL, 10);
  

• 		yres_specified = 1;
  

• 	} else
  

• 		goto done;
  

• case '0' ... '9':
  

• 	break;
  

• case 'M':
  

• 	if (!yres_specified)
  

• 		cvt = 1;
  

• 	break;
  

• case 'R':
  

• 	if (cvt)
  

• 		rb = 1;
  

• 	break;
  

• case 'm':
  

• 	if (!cvt)
  

• 		margins = 1;
  

• 	break;
  

• case 'i':
  

• 	if (!cvt)
  

• 		interlace = 1;
  

• 	break;
  

• case 'e':
  

• 	force = DRM_FORCE_ON;
  

• 	break;
  

• case 'D':
  

• 	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':
  

• 	force = DRM_FORCE_OFF;
  

• 	break;
  

• default:
  

• 	goto done;
  

• }
  

• }
• if (i < 0 && yres_specified) {

• xres = simple_strtol(name, NULL, 10);
  

• res_specified = 1;
  

• }

-done:

• DRM_DEBUG_KMS("cmdline mode for connector %s %dx%d@%dHz%s%s%s\n",

• drm_get_connector_name(connector), xres, yres,
  

• (refresh) ? refresh : 60, (rb) ? " reduced blanking" :
  

• "", (margins) ? " with margins" : "", (interlace) ?
  

• " interlaced" : "");
  

• if (force) {

• const char *s;
  

• switch (force) {
  

• case DRM_FORCE_OFF: s = "OFF"; break;
  

• case DRM_FORCE_ON_DIGITAL: s = "ON - dig"; break;
  

• default:
  

• case DRM_FORCE_ON: s = "ON"; break;
  

• }
  

• DRM_INFO("forcing %s connector %s\n",
  

• 	 drm_get_connector_name(connector), s);
  

• connector->force = force;
  

• }
• if (res_specified) {

• cmdline_mode->specified = true;
  

• cmdline_mode->xres = xres;
  

• cmdline_mode->yres = yres;
  

• }
• if (refresh_specified) {

• cmdline_mode->refresh_specified = true;
  

• cmdline_mode->refresh = refresh;
  

• }
• if (bpp_specified) {

• cmdline_mode->bpp_specified = true;
  

• cmdline_mode->bpp = bpp;
  

• }
• cmdline_mode->rb = rb ? true : false;
• cmdline_mode->cvt = cvt ? true : false;
• cmdline_mode->interlace = interlace ? true : false;
• return true;

-}

static int drm_fb_helper_parse_command_line(struct drm_fb_helper *fb_helper) { struct drm_fb_helper_connector *fb_helper_conn; int i;

for (i = 0; i < fb_helper->connector_count; i++) {

• struct drm_cmdline_mode *mode;
  

• struct drm_connector *connector;
  

  char *option = NULL;

  fb_helper_conn = fb_helper->connector_info[i];

• connector = fb_helper_conn->connector;
  

• mode = &fb_helper_conn->cmdline_mode;
  
  

  /* do something on return - turn off connector maybe */

• if (fb_get_options(drm_get_connector_name(fb_helper_conn->connector), &option))
  

• if (fb_get_options(drm_get_connector_name(connector), &option))
  continue;
  
  

• drm_fb_helper_connector_parse_command_line(fb_helper_conn, option);
  

• if (drm_mode_parse_command_line_for_connector(option,
  

• 					      connector,
  

• 					      mode)) {
  

• 	if (mode->force) {
  

• 		const char *s;
  

• 		switch (mode->force) {
  

• 		case DRM_FORCE_OFF: s = "OFF"; break;
  

• 		case DRM_FORCE_ON_DIGITAL: s = "ON - dig"; break;
  

• 		default:
  

• 		case DRM_FORCE_ON: s = "ON"; break;
  

• 		}
  

• 		DRM_INFO("forcing %s connector %s\n",
  

• 			 drm_get_connector_name(connector), s);
  

• 		connector->force = mode->force;
  

• 	}
  

• 	DRM_DEBUG_KMS("cmdline mode for connector %s %dx%d@%dHz%s%s%s\n",
  

• 		      drm_get_connector_name(connector),
  

• 		      mode->xres, mode->yres,
  

• 		      mode->refresh_specified ? mode->refresh : 60,
  

• 		      mode->rb ? " reduced blanking" : "",
  

• 		      mode->margins ? " with margins" : "",
  

• 		      mode->interlace ?  " interlaced" : "");
  

• }
  

• } return 0;

} @@ -883,7 +759,7 @@ int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper, /* first up get a count of crtcs now in use and new min/maxes width/heights */ for (i = 0; i < fb_helper->connector_count; i++) { struct drm_fb_helper_connector *fb_helper_conn = fb_helper->connector_info[i];

• struct drm_fb_helper_cmdline_mode *cmdline_mode;
  

• struct drm_cmdline_mode *cmdline_mode;
  
  

  cmdline_mode = &fb_helper_conn->cmdline_mode;

@@ -1105,7 +981,7 @@ static struct drm_display_mode *drm_has_preferred_mode(struct drm_fb_helper_conn

static bool drm_has_cmdline_mode(struct drm_fb_helper_connector *fb_connector) {

• struct drm_fb_helper_cmdline_mode *cmdline_mode;

• struct drm_cmdline_mode *cmdline_mode; cmdline_mode = &fb_connector->cmdline_mode; return cmdline_mode->specified;

} @@ -1113,7 +989,7 @@ static bool drm_has_cmdline_mode(struct drm_fb_helper_connector *fb_connector) static struct drm_display_mode *drm_pick_cmdline_mode(struct drm_fb_helper_connector *fb_helper_conn, int width, int height) {

• struct drm_fb_helper_cmdline_mode *cmdline_mode;

• struct drm_cmdline_mode *cmdline_mode; struct drm_display_mode *mode = NULL;

  cmdline_mode = &fb_helper_conn->cmdline_mode;

@@ -1145,19 +1021,8 @@ static struct drm_display_mode *drm_pick_cmdline_mode(struct drm_fb_helper_conne }

create_mode:

• if (cmdline_mode->cvt)

• mode = drm_cvt_mode(fb_helper_conn->connector->dev,
  

• 		    cmdline_mode->xres, cmdline_mode->yres,
  

• 		    cmdline_mode->refresh_specified ? cmdline_mode->refresh : 60,
  

• 		    cmdline_mode->rb, cmdline_mode->interlace,
  

• 		    cmdline_mode->margins);
  

• else

• mode = drm_gtf_mode(fb_helper_conn->connector->dev,
  

• 		    cmdline_mode->xres, cmdline_mode->yres,
  

• 		    cmdline_mode->refresh_specified ? cmdline_mode->refresh : 60,
  

• 		    cmdline_mode->interlace,
  

• 		    cmdline_mode->margins);
  

• drm_mode_set_crtcinfo(mode, CRTC_INTERLACE_HALVE_V);

• mode = drm_mode_create_from_cmdline_mode(fb_helper_conn->connector->dev,

• 				 cmdline_mode);
  

  list_add(&mode->head, &fb_helper_conn->connector->modes); return mode;

} diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 58e65f9..f9da47e 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -974,3 +974,157 @@ void drm_mode_connector_list_update(struct drm_connector *connector) } } EXPORT_SYMBOL(drm_mode_connector_list_update);

+/**

• • drm_mode_parse_command_line_for_connector - parse command line for connector
• • @mode_option - per connector mode option
• • @connector - connector to parse line for
• • This parses the connector specific then generic command lines for
• • modes and options to configure the connector.
• • This uses the same parameters as the fb modedb.c, except for extra
• • <xres>x<yres>[M][R][-<bpp>][@<refresh>][i][m][eDd]
• • enable/enable Digital/disable bit at the end
• */

+bool drm_mode_parse_command_line_for_connector(const char *mode_option,

• 			       struct drm_connector *connector,
  

• 			       struct drm_cmdline_mode *mode)
  

+{

• const char *name;
• unsigned int namelen;
• int res_specified = 0, bpp_specified = 0, refresh_specified = 0;
• unsigned int xres = 0, yres = 0, bpp = 32, refresh = 0;
• int yres_specified = 0, cvt = 0, rb = 0, interlace = 0, margins = 0;
• int i;
• enum drm_connector_force force = DRM_FORCE_UNSPECIFIED;
• if (!mode_option)

• mode_option = fb_mode_option;
  

• if (!mode_option) {

• mode->specified = false;
  

• return false;
  

• }
• name = mode_option;
• namelen = strlen(name);
• for (i = namelen-1; i >= 0; i--) {

• switch (name[i]) {
  

• case '@':
  

• 	namelen = i;
  

• 	if (!refresh_specified && !bpp_specified &&
  

• 	    !yres_specified) {
  

• 		refresh = simple_strtol(&name[i+1], NULL, 10);
  

• 		refresh_specified = 1;
  

• 		if (cvt || rb)
  

• 			cvt = 0;
  

• 	} else
  

• 		goto done;
  

• 	break;
  

• case '-':
  

• 	namelen = i;
  

• 	if (!bpp_specified && !yres_specified) {
  

• 		bpp = simple_strtol(&name[i+1], NULL, 10);
  

• 		bpp_specified = 1;
  

• 		if (cvt || rb)
  

• 			cvt = 0;
  

• 	} else
  

• 		goto done;
  

• 	break;
  

• case 'x':
  

• 	if (!yres_specified) {
  

• 		yres = simple_strtol(&name[i+1], NULL, 10);
  

• 		yres_specified = 1;
  

• 	} else
  

• 		goto done;
  

• case '0' ... '9':
  

• 	break;
  

• case 'M':
  

• 	if (!yres_specified)
  

• 		cvt = 1;
  

• 	break;
  

• case 'R':
  

• 	if (cvt)
  

• 		rb = 1;
  

• 	break;
  

• case 'm':
  

• 	if (!cvt)
  

• 		margins = 1;
  

• 	break;
  

• case 'i':
  

• 	if (!cvt)
  

• 		interlace = 1;
  

• 	break;
  

• case 'e':
  

• 	force = DRM_FORCE_ON;
  

• 	break;
  

• case 'D':
  

• 	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':
  

• 	force = DRM_FORCE_OFF;
  

• 	break;
  

• default:
  

• 	goto done;
  

• }
  

• }
• if (i < 0 && yres_specified) {

• xres = simple_strtol(name, NULL, 10);
  

• res_specified = 1;
  

• }

+done:

• if (res_specified) {

• mode->specified = true;
  

• mode->xres = xres;
  

• mode->yres = yres;
  

• }
• if (refresh_specified) {

• mode->refresh_specified = true;
  

• mode->refresh = refresh;
  

• }
• if (bpp_specified) {

• mode->bpp_specified = true;
  

• mode->bpp = bpp;
  

• }
• mode->rb = rb ? true : false;
• mode->cvt = cvt ? true : false;
• mode->interlace = interlace ? true : false;
• mode->force = force;
• return true;

+} +EXPORT_SYMBOL(drm_mode_parse_command_line_for_connector);

+struct drm_display_mode * +drm_mode_create_from_cmdline_mode(struct drm_device *dev,

• 		  struct drm_cmdline_mode *cmd)
  

+{

• struct drm_display_mode *mode;
• if (cmd->cvt)

• mode = drm_cvt_mode(dev,
  

• 		    cmd->xres, cmd->yres,
  

• 		    cmd->refresh_specified ? cmd->refresh : 60,
  

• 		    cmd->rb, cmd->interlace,
  

• 		    cmd->margins);
  

• else

• mode = drm_gtf_mode(dev,
  

• 		    cmd->xres, cmd->yres,
  

• 		    cmd->refresh_specified ? cmd->refresh : 60,
  

• 		    cmd->interlace,
  

• 		    cmd->margins);
  

• if (!mode)

• return NULL;
  

• drm_mode_set_crtcinfo(mode, CRTC_INTERLACE_HALVE_V);
• return mode;

+} +EXPORT_SYMBOL(drm_mode_create_from_cmdline_mode); diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c index cd08960..af4ef17 100644 --- a/drivers/gpu/drm/i915/intel_lvds.c +++ b/drivers/gpu/drm/i915/intel_lvds.c @@ -874,6 +874,8 @@ bool intel_lvds_init(struct drm_device *dev) struct drm_encoder *encoder; struct drm_display_mode *scan; /* *modes, *bios_mode; */ struct drm_crtc *crtc;

• char *cmdline_option = NULL;
• struct drm_cmdline_mode cmdline_mode; u32 lvds; int pipe; u8 pin;

@@ -951,6 +953,7 @@ bool intel_lvds_init(struct drm_device *dev) intel_lvds->fitting_mode = DRM_MODE_SCALE_ASPECT; /* * LVDS discovery:

• * 0) user override
  

  • 1. check for EDID on DDC
  • 2. check for VBT data
  • 3. check to see if LVDS is already on

@@ -959,6 +962,20 @@ bool intel_lvds_init(struct drm_device *dev) * if closed, act like it's not there for now */

• if (fb_get_options(drm_get_connector_name(connector),

• 	   &cmdline_option) == 0 &&
  

•    drm_mode_parse_command_line_for_connector(cmdline_option,
  

• 				      connector,
  

• 				      &cmdline_mode)) {
  

• intel_lvds->fixed_mode =
  

• 	drm_mode_create_from_cmdline_mode(dev, &cmdline_mode);
  

• if (intel_lvds->fixed_mode) {
  

• 	intel_lvds->fixed_mode->type |=
  

• 		DRM_MODE_TYPE_PREFERRED;
  

• 	goto out;
  

• }
  

• }
• /*
  • Attempt to get the fixed panel mode from DDC. Assume that the
  • preferred mode is the right one.

diff --git a/include/drm/drmP.h b/include/drm/drmP.h index fe29aad..bf01108 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -968,6 +968,22 @@ struct drm_minor { struct drm_mode_group mode_group; };

+/* mode specified on the command line */ +struct drm_cmdline_mode {

• bool specified;
• bool refresh_specified;
• bool bpp_specified;
• int xres, yres;
• int bpp;
• int refresh;
• bool rb;
• bool interlace;
• bool cvt;
• bool margins;
• enum drm_connector_force force;

+};

struct drm_pending_vblank_event { struct drm_pending_event base; int pipe; @@ -1381,6 +1397,15 @@ extern int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, struct drm_crtc *refcrtc); extern void drm_calc_timestamping_constants(struct drm_crtc *crtc);

+extern bool +drm_mode_parse_command_line_for_connector(const char *mode_option,

• 			  struct drm_connector *connector,
  

• 			  struct drm_cmdline_mode *mode);
  

+extern struct drm_display_mode * +drm_mode_create_from_cmdline_mode(struct drm_device *dev,

• 		  struct drm_cmdline_mode *cmd);
  

/* Modesetting support */ extern void drm_vblank_pre_modeset(struct drm_device *dev, int crtc); extern void drm_vblank_post_modeset(struct drm_device *dev, int crtc); diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h index f22e7fe..4e66488 100644 --- a/include/drm/drm_fb_helper.h +++ b/include/drm/drm_fb_helper.h @@ -40,20 +40,6 @@ struct drm_fb_helper_crtc { struct drm_display_mode *desired_mode; };

-/* mode specified on the command line */ -struct drm_fb_helper_cmdline_mode {

• bool specified;
• bool refresh_specified;
• bool bpp_specified;
• int xres, yres;
• int bpp;
• int refresh;
• bool rb;
• bool interlace;
• bool cvt;
• bool margins;

-};

struct drm_fb_helper_surface_size { u32 fb_width; u32 fb_height; @@ -74,8 +60,8 @@ struct drm_fb_helper_funcs { };

struct drm_fb_helper_connector {

• struct drm_fb_helper_cmdline_mode cmdline_mode; struct drm_connector *connector;

• struct drm_cmdline_mode cmdline_mode;

};

struct drm_fb_helper {