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@xxxxxxx>
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Dave Airlie <airlied@xxxxxxxxxx>
> ---
> 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 {
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
