On Thu, 15 May 2014 10:54:36 +0100 Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote: > i915.ko has a custom fbdev initialisation routine that aims to preserve > the current mode set by the BIOS, unless overruled by the user. The > user's wishes are determined by what, if any, mode is specified on the > command line (via the video= parameter). However, that command line mode > is first parsed by drm_fb_helper_initial_config() which is called after > i915.ko's custom initial_config() as a fallback method. So in order for > us to honour it, we need to move the cmdline parser earlier. If we > perform the connector cmdline parsing as soon as we initialise the > connector, that cmdline mode and forced status is then available even if > the fbdev helper is not compiled in or never called. > > We also then expose the cmdline user mode in the connector mode lists. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=73154 > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > Cc: Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx> > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > Cc: Daniel Vetter <daniel.vetter@xxxxxxxx> > --- > drivers/gpu/drm/drm_crtc.c | 56 +++++++++++++++++++++++++++++++ > drivers/gpu/drm/drm_fb_helper.c | 64 ++---------------------------------- > drivers/gpu/drm/drm_modes.c | 1 + > drivers/gpu/drm/drm_probe_helper.c | 17 ++++++++++ > drivers/gpu/drm/i915/intel_drv.h | 5 --- > include/drm/drm_crtc.h | 1 + > include/drm/drm_fb_helper.h | 1 - > 7 files changed, 78 insertions(+), 67 deletions(-) > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index 461d19b..93c9198 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -799,6 +799,60 @@ static void drm_mode_remove(struct drm_connector *connector, > } > > /** > + * drm_connector_get_cmdline_mode - reads the user's cmdline mode > + * @connector: connector to quwery > + * @mode: returned mode > + * > + * The kernel supports per-connector configration of its consoles through > + * use of the video= parameter. This function parses that option and > + * extracts the user's specified mode (or enable/disable status) for a > + * particular connector. This is typically only used during the early fbdev > + * setup. > + */ > +static void drm_connector_get_cmdline_mode(struct drm_connector *connector) > +{ > + struct drm_cmdline_mode *mode = &connector->cmdline_mode; > + char *option = NULL; > + > + if (fb_get_options(drm_get_connector_name(connector), &option)) > + return; > + > + if (!drm_mode_parse_command_line_for_connector(option, > + connector, > + mode)) > + return; > + > + 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" : ""); > +} > + > +/** > * drm_connector_init - Init a preallocated connector > * @dev: DRM device > * @connector: the connector to init > @@ -842,6 +896,8 @@ int drm_connector_init(struct drm_device *dev, > connector->edid_blob_ptr = NULL; > connector->status = connector_status_unknown; > > + drm_connector_get_cmdline_mode(connector); > + > list_add_tail(&connector->head, &dev->mode_config.connector_list); > dev->mode_config.num_connector++; > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > index e95ed58..c3b53f9 100644 > --- a/drivers/gpu/drm/drm_fb_helper.c > +++ b/drivers/gpu/drm/drm_fb_helper.c > @@ -105,60 +105,6 @@ fail: > } > EXPORT_SYMBOL(drm_fb_helper_single_add_all_connectors); > > -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(connector), &option)) > - continue; > - > - 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; > -} > - > static void drm_fb_helper_save_lut_atomic(struct drm_crtc *crtc, struct drm_fb_helper *helper) > { > uint16_t *r_base, *g_base, *b_base; > @@ -910,7 +856,7 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper, > struct drm_fb_helper_connector *fb_helper_conn = fb_helper->connector_info[i]; > struct drm_cmdline_mode *cmdline_mode; > > - cmdline_mode = &fb_helper_conn->cmdline_mode; > + cmdline_mode = &fb_helper_conn->connector->cmdline_mode; > > if (cmdline_mode->bpp_specified) { > switch (cmdline_mode->bpp) { > @@ -1158,9 +1104,7 @@ EXPORT_SYMBOL(drm_has_preferred_mode); > > static bool drm_has_cmdline_mode(struct drm_fb_helper_connector *fb_connector) > { > - struct drm_cmdline_mode *cmdline_mode; > - cmdline_mode = &fb_connector->cmdline_mode; > - return cmdline_mode->specified; > + return fb_connector->connector->cmdline_mode.specified; > } > > struct drm_display_mode *drm_pick_cmdline_mode(struct drm_fb_helper_connector *fb_helper_conn, > @@ -1170,7 +1114,7 @@ struct drm_display_mode *drm_pick_cmdline_mode(struct drm_fb_helper_connector *f > struct drm_display_mode *mode = NULL; > bool prefer_non_interlace; > > - cmdline_mode = &fb_helper_conn->cmdline_mode; > + cmdline_mode = &fb_helper_conn->connector->cmdline_mode; > if (cmdline_mode->specified == false) > return mode; > > @@ -1555,8 +1499,6 @@ bool drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper, int bpp_sel) > struct drm_device *dev = fb_helper->dev; > int count = 0; > > - drm_fb_helper_parse_command_line(fb_helper); > - > mutex_lock(&dev->mode_config.mutex); > count = drm_fb_helper_probe_connector_modes(fb_helper, > dev->mode_config.max_width, > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c > index bedf189..d1b7d20 100644 > --- a/drivers/gpu/drm/drm_modes.c > +++ b/drivers/gpu/drm/drm_modes.c > @@ -1259,6 +1259,7 @@ drm_mode_create_from_cmdline_mode(struct drm_device *dev, > if (!mode) > return NULL; > > + mode->type |= DRM_MODE_TYPE_USERDEF; > drm_mode_set_crtcinfo(mode, CRTC_INTERLACE_HALVE_V); > return mode; > } > diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c > index 79f07f2..006e3f9 100644 > --- a/drivers/gpu/drm/drm_probe_helper.c > +++ b/drivers/gpu/drm/drm_probe_helper.c > @@ -82,6 +82,22 @@ static void drm_mode_validate_flag(struct drm_connector *connector, > return; > } > > +static int drm_helper_probe_add_cmdline_mode(struct drm_connector *connector) > +{ > + struct drm_display_mode *mode; > + > + if (!connector->cmdline_mode.specified) > + return 0; > + > + mode = drm_mode_create_from_cmdline_mode(connector->dev, > + &connector->cmdline_mode); > + if (mode == NULL) > + return 0; > + > + drm_mode_probed_add(connector, mode); > + return 1; > +} > + > static int drm_helper_probe_single_connector_modes_merge_bits(struct drm_connector *connector, > uint32_t maxX, uint32_t maxY, bool merge_type_bits) > { > @@ -134,6 +150,7 @@ static int drm_helper_probe_single_connector_modes_merge_bits(struct drm_connect > > if (count == 0 && connector->status == connector_status_connected) > count = drm_add_modes_noedid(connector, 1024, 768); > + count += drm_helper_probe_add_cmdline_mode(connector); > if (count == 0) > goto prune; > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 32a74e1..f38f941 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -828,7 +828,6 @@ void intel_dvo_init(struct drm_device *dev); > /* legacy fbdev emulation in intel_fbdev.c */ > #ifdef CONFIG_DRM_I915_FBDEV > extern int intel_fbdev_init(struct drm_device *dev); > -extern void intel_fbdev_initial_config(struct drm_device *dev); > extern void intel_fbdev_fini(struct drm_device *dev); > extern void intel_fbdev_set_suspend(struct drm_device *dev, int state); > extern void intel_fbdev_output_poll_changed(struct drm_device *dev); > @@ -839,10 +838,6 @@ static inline int intel_fbdev_init(struct drm_device *dev) > return 0; > } > > -static inline void intel_fbdev_initial_config(struct drm_device *dev) > -{ > -} > - > static inline void intel_fbdev_fini(struct drm_device *dev) > { > } > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > index c061bb3..4dde1fb 100644 > --- a/include/drm/drm_crtc.h > +++ b/include/drm/drm_crtc.h > @@ -508,6 +508,7 @@ struct drm_connector { > void *helper_private; > > /* forced on connector */ > + struct drm_cmdline_mode cmdline_mode; > enum drm_connector_force force; > uint32_t encoder_ids[DRM_CONNECTOR_MAX_ENCODER]; > struct drm_encoder *encoder; /* currently active encoder */ > diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h > index 6e622f7..a8ae1b9 100644 > --- a/include/drm/drm_fb_helper.h > +++ b/include/drm/drm_fb_helper.h > @@ -77,7 +77,6 @@ struct drm_fb_helper_funcs { > > struct drm_fb_helper_connector { > struct drm_connector *connector; > - struct drm_cmdline_mode cmdline_mode; > }; > > struct drm_fb_helper { Looks ok... it mixes together things that could be in separate patches (like the userdef fix), but overall I think it's an improvement. Reviewed-by: Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx> -- Jesse Barnes, Intel Open Source Technology Center _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel