All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm: Perform cmdline mode parsing during connector initialisation
@ 2014-08-06  8:08 Daniel Vetter
  2014-08-06  9:43   ` Daniel Vetter
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Daniel Vetter @ 2014-08-06  8:08 UTC (permalink / raw)
  To: Intel Graphics Development, DRI Development; +Cc: Daniel Vetter, Dave Airlie

From: Chris Wilson <chris@chris-wilson.co.uk>

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.

v2: Rebase after connector->name upheaval.

v3: Adapt mga200 to look for the cmdline mode in the new place. Nicely
simplifies things while at that.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=73154
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> (v2)
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org> (v2)
Cc: dri-devel@lists.freedesktop.org
Cc: Julia Lemire <jlemire@matrox.com>
Cc: Dave Airlie <airlied@redhat.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_crtc.c             | 55 +++++++++++++++++++++++++++++
 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/mgag200/mgag200_mode.c | 20 +++--------
 include/drm/drm_crtc.h                 |  1 +
 include/drm/drm_fb_helper.h            |  1 -
 7 files changed, 82 insertions(+), 77 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 3c4a62169f28..d3f1e0033475 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -852,6 +852,59 @@ 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(connector->name, &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", connector->name, s);
+		connector->force = mode->force;
+	}
+
+	DRM_DEBUG_KMS("cmdline mode for connector %s %dx%d@%dHz%s%s%s\n",
+		      connector->name,
+		      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
@@ -903,6 +956,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 d139eddb3d61..9a91ee69af2b 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -171,60 +171,6 @@ int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
 }
 EXPORT_SYMBOL(drm_fb_helper_remove_one_connector);
 
-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(connector->name, &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",
-					 connector->name, s);
-				connector->force = mode->force;
-			}
-
-			DRM_DEBUG_KMS("cmdline mode for connector %s %dx%d@%dHz%s%s%s\n",
-				      connector->name,
-				      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;
@@ -1020,7 +966,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) {
@@ -1267,9 +1213,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,
@@ -1279,7 +1223,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;
 
@@ -1664,8 +1608,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 bedf1894e17e..d1b7d2006529 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 db7d250f7ac7..6857e9ad6339 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)
 {
@@ -141,6 +157,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/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index 45f04dea0ac2..e5ae60d7f0c5 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -1483,11 +1483,7 @@ static int mga_vga_mode_valid(struct drm_connector *connector,
 {
 	struct drm_device *dev = connector->dev;
 	struct mga_device *mdev = (struct mga_device*)dev->dev_private;
-	struct mga_fbdev *mfbdev = mdev->mfbdev;
-	struct drm_fb_helper *fb_helper = &mfbdev->helper;
-	struct drm_fb_helper_connector *fb_helper_conn = NULL;
 	int bpp = 32;
-	int i = 0;
 
 	if (IS_G200_SE(mdev)) {
 		if (mdev->unique_rev_id == 0x01) {
@@ -1537,21 +1533,15 @@ static int mga_vga_mode_valid(struct drm_connector *connector,
 	}
 
 	/* Validate the mode input by the user */
-	for (i = 0; i < fb_helper->connector_count; i++) {
-		if (fb_helper->connector_info[i]->connector == connector) {
-			/* Found the helper for this connector */
-			fb_helper_conn = fb_helper->connector_info[i];
-			if (fb_helper_conn->cmdline_mode.specified) {
-				if (fb_helper_conn->cmdline_mode.bpp_specified) {
-					bpp = fb_helper_conn->cmdline_mode.bpp;
-				}
-			}
+	if (connector->cmdline_mode.specified) {
+		if (connector->cmdline_mode.bpp_specified) {
+			bpp = connector->cmdline_mode.bpp;
 		}
 	}
 
 	if ((mode->hdisplay * mode->vdisplay * (bpp/8)) > mdev->mc.vram_size) {
-		if (fb_helper_conn)
-			fb_helper_conn->cmdline_mode.specified = false;
+		if (connector->cmdline_mode.specified)
+			connector->cmdline_mode.specified = false;
 		return MODE_BAD;
 	}
 
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 62f73bdbcc47..265b20f6bd8b 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -548,6 +548,7 @@ struct drm_connector {
 	void *helper_private;
 
 	/* forced on connector */
+	struct drm_cmdline_mode cmdline_mode;
 	enum drm_connector_force force;
 	bool override_edid;
 	uint32_t encoder_ids[DRM_CONNECTOR_MAX_ENCODER];
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index bfd329d613c4..f4ad254e3488 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 {
-- 
2.0.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH] video/fbdev: Always built-in video= cmdline parsing
  2014-08-06  8:08 [PATCH] drm: Perform cmdline mode parsing during connector initialisation Daniel Vetter
@ 2014-08-06  9:43   ` Daniel Vetter
  2014-08-06 12:05   ` Daniel Vetter
  2014-08-06 12:52   ` Daniel Vetter
  2 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2014-08-06  9:43 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, Intel Graphics Development, Tomi Valkeinen,
	Plagniol-Villard, linux-fbdev

In drm/i915 we want to get at the video= cmdline modes even when we
don't have fbdev support enabled, so that users can always override
the kernel's initial mode selection.

But that gives us a direct depency upon the parsing code in the fbdev
subsystem. Since it's so little code just extract these 2 functions
and always build them in.

Whiel at it fix the checkpatch fail in this code.

Cc: Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

--

I prefer if we can merge this through drm-next since we'll use it
there in follow-up patches.
-Daniel
---
 drivers/video/fbdev/core/Makefile     |   2 +-
 drivers/video/fbdev/core/fb_cmdline.c | 103 ++++++++++++++++++++++++++++++++++
 drivers/video/fbdev/core/fbmem.c      |  92 ------------------------------
 3 files changed, 104 insertions(+), 93 deletions(-)
 create mode 100644 drivers/video/fbdev/core/fb_cmdline.c

diff --git a/drivers/video/fbdev/core/Makefile b/drivers/video/fbdev/core/Makefile
index fa306538dac2..891c1f890e03 100644
--- a/drivers/video/fbdev/core/Makefile
+++ b/drivers/video/fbdev/core/Makefile
@@ -1,4 +1,4 @@
-obj-y                             += fb_notify.o
+obj-y                             += fb_notify.o fb_cmdline.o
 obj-$(CONFIG_FB)                  += fb.o
 fb-y                              := fbmem.o fbmon.o fbcmap.o fbsysfs.o \
                                      modedb.o fbcvt.o
diff --git a/drivers/video/fbdev/core/fb_cmdline.c b/drivers/video/fbdev/core/fb_cmdline.c
new file mode 100644
index 000000000000..91503a43213e
--- /dev/null
+++ b/drivers/video/fbdev/core/fb_cmdline.c
@@ -0,0 +1,103 @@
+/*
+ *  linux/drivers/video/fb_cmdline.c
+ *
+ *  Copyright (C) 2014 Intel Corp
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file COPYING in the main directory of this archive
+ * for more details.
+ *
+ * Authors:
+ *    Vetter <danie.vetter@ffwll.ch>
+ */
+#include <linux/init.h>
+#include <linux/fb.h>
+
+static char *video_options[FB_MAX] __read_mostly;
+static int ofonly __read_mostly;
+
+/**
+ * fb_get_options - get kernel boot parameters
+ * @name:   framebuffer name as it would appear in
+ *          the boot parameter line
+ *          (video=<name>:<options>)
+ * @option: the option will be stored here
+ *
+ * NOTE: Needed to maintain backwards compatibility
+ */
+int fb_get_options(const char *name, char **option)
+{
+	char *opt, *options = NULL;
+	int retval = 0;
+	int name_len = strlen(name), i;
+
+	if (name_len && ofonly && strncmp(name, "offb", 4))
+		retval = 1;
+
+	if (name_len && !retval) {
+		for (i = 0; i < FB_MAX; i++) {
+			if (video_options[i] = NULL)
+				continue;
+			if (!video_options[i][0])
+				continue;
+			opt = video_options[i];
+			if (!strncmp(name, opt, name_len) &&
+			    opt[name_len] = ':')
+				options = opt + name_len + 1;
+		}
+	}
+	/* No match, pass global option */
+	if (!options && option && fb_mode_option)
+		options = kstrdup(fb_mode_option, GFP_KERNEL);
+	if (options && !strncmp(options, "off", 3))
+		retval = 1;
+
+	if (option)
+		*option = options;
+
+	return retval;
+}
+EXPORT_SYMBOL(fb_get_options);
+
+/**
+ *	video_setup - process command line options
+ *	@options: string of options
+ *
+ *	Process command line options for frame buffer subsystem.
+ *
+ *	NOTE: This function is a __setup and __init function.
+ *            It only stores the options.  Drivers have to call
+ *            fb_get_options() as necessary.
+ *
+ *	Returns zero.
+ *
+ */
+static int __init video_setup(char *options)
+{
+	int i, global = 0;
+
+	if (!options || !*options)
+		global = 1;
+
+	if (!global && !strncmp(options, "ofonly", 6)) {
+		ofonly = 1;
+		global = 1;
+	}
+
+	if (!global && !strchr(options, ':')) {
+		fb_mode_option = options;
+		global = 1;
+	}
+
+	if (!global) {
+		for (i = 0; i < FB_MAX; i++) {
+			if (video_options[i] = NULL) {
+				video_options[i] = options;
+				break;
+			}
+		}
+	}
+
+	return 1;
+}
+__setup("video=", video_setup);
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index b5e85f6c1c26..0705d8883ede 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -1908,96 +1908,4 @@ int fb_new_modelist(struct fb_info *info)
 	return err;
 }
 
-static char *video_options[FB_MAX] __read_mostly;
-static int ofonly __read_mostly;
-
-/**
- * fb_get_options - get kernel boot parameters
- * @name:   framebuffer name as it would appear in
- *          the boot parameter line
- *          (video=<name>:<options>)
- * @option: the option will be stored here
- *
- * NOTE: Needed to maintain backwards compatibility
- */
-int fb_get_options(const char *name, char **option)
-{
-	char *opt, *options = NULL;
-	int retval = 0;
-	int name_len = strlen(name), i;
-
-	if (name_len && ofonly && strncmp(name, "offb", 4))
-		retval = 1;
-
-	if (name_len && !retval) {
-		for (i = 0; i < FB_MAX; i++) {
-			if (video_options[i] = NULL)
-				continue;
-			if (!video_options[i][0])
-				continue;
-			opt = video_options[i];
-			if (!strncmp(name, opt, name_len) &&
-			    opt[name_len] = ':')
-				options = opt + name_len + 1;
-		}
-	}
-	/* No match, pass global option */
-	if (!options && option && fb_mode_option)
-		options = kstrdup(fb_mode_option, GFP_KERNEL);
-	if (options && !strncmp(options, "off", 3))
-		retval = 1;
-
-	if (option)
-		*option = options;
-
-	return retval;
-}
-EXPORT_SYMBOL(fb_get_options);
-
-#ifndef MODULE
-/**
- *	video_setup - process command line options
- *	@options: string of options
- *
- *	Process command line options for frame buffer subsystem.
- *
- *	NOTE: This function is a __setup and __init function.
- *            It only stores the options.  Drivers have to call
- *            fb_get_options() as necessary.
- *
- *	Returns zero.
- *
- */
-static int __init video_setup(char *options)
-{
-	int i, global = 0;
-
-	if (!options || !*options)
- 		global = 1;
-
- 	if (!global && !strncmp(options, "ofonly", 6)) {
- 		ofonly = 1;
- 		global = 1;
- 	}
-
- 	if (!global && !strchr(options, ':')) {
- 		fb_mode_option = options;
- 		global = 1;
- 	}
-
- 	if (!global) {
- 		for (i = 0; i < FB_MAX; i++) {
- 			if (video_options[i] = NULL) {
- 				video_options[i] = options;
- 				break;
- 			}
-
-		}
-	}
-
-	return 1;
-}
-__setup("video=", video_setup);
-#endif
-
 MODULE_LICENSE("GPL");
-- 
2.0.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH] video/fbdev: Always built-in video= cmdline parsing
@ 2014-08-06  9:43   ` Daniel Vetter
  0 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2014-08-06  9:43 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, Intel Graphics Development, Tomi Valkeinen,
	Plagniol-Villard, linux-fbdev

In drm/i915 we want to get at the video= cmdline modes even when we
don't have fbdev support enabled, so that users can always override
the kernel's initial mode selection.

But that gives us a direct depency upon the parsing code in the fbdev
subsystem. Since it's so little code just extract these 2 functions
and always build them in.

Whiel at it fix the checkpatch fail in this code.

Cc: Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

--

I prefer if we can merge this through drm-next since we'll use it
there in follow-up patches.
-Daniel
---
 drivers/video/fbdev/core/Makefile     |   2 +-
 drivers/video/fbdev/core/fb_cmdline.c | 103 ++++++++++++++++++++++++++++++++++
 drivers/video/fbdev/core/fbmem.c      |  92 ------------------------------
 3 files changed, 104 insertions(+), 93 deletions(-)
 create mode 100644 drivers/video/fbdev/core/fb_cmdline.c

diff --git a/drivers/video/fbdev/core/Makefile b/drivers/video/fbdev/core/Makefile
index fa306538dac2..891c1f890e03 100644
--- a/drivers/video/fbdev/core/Makefile
+++ b/drivers/video/fbdev/core/Makefile
@@ -1,4 +1,4 @@
-obj-y                             += fb_notify.o
+obj-y                             += fb_notify.o fb_cmdline.o
 obj-$(CONFIG_FB)                  += fb.o
 fb-y                              := fbmem.o fbmon.o fbcmap.o fbsysfs.o \
                                      modedb.o fbcvt.o
diff --git a/drivers/video/fbdev/core/fb_cmdline.c b/drivers/video/fbdev/core/fb_cmdline.c
new file mode 100644
index 000000000000..91503a43213e
--- /dev/null
+++ b/drivers/video/fbdev/core/fb_cmdline.c
@@ -0,0 +1,103 @@
+/*
+ *  linux/drivers/video/fb_cmdline.c
+ *
+ *  Copyright (C) 2014 Intel Corp
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file COPYING in the main directory of this archive
+ * for more details.
+ *
+ * Authors:
+ *    Vetter <danie.vetter@ffwll.ch>
+ */
+#include <linux/init.h>
+#include <linux/fb.h>
+
+static char *video_options[FB_MAX] __read_mostly;
+static int ofonly __read_mostly;
+
+/**
+ * fb_get_options - get kernel boot parameters
+ * @name:   framebuffer name as it would appear in
+ *          the boot parameter line
+ *          (video=<name>:<options>)
+ * @option: the option will be stored here
+ *
+ * NOTE: Needed to maintain backwards compatibility
+ */
+int fb_get_options(const char *name, char **option)
+{
+	char *opt, *options = NULL;
+	int retval = 0;
+	int name_len = strlen(name), i;
+
+	if (name_len && ofonly && strncmp(name, "offb", 4))
+		retval = 1;
+
+	if (name_len && !retval) {
+		for (i = 0; i < FB_MAX; i++) {
+			if (video_options[i] == NULL)
+				continue;
+			if (!video_options[i][0])
+				continue;
+			opt = video_options[i];
+			if (!strncmp(name, opt, name_len) &&
+			    opt[name_len] == ':')
+				options = opt + name_len + 1;
+		}
+	}
+	/* No match, pass global option */
+	if (!options && option && fb_mode_option)
+		options = kstrdup(fb_mode_option, GFP_KERNEL);
+	if (options && !strncmp(options, "off", 3))
+		retval = 1;
+
+	if (option)
+		*option = options;
+
+	return retval;
+}
+EXPORT_SYMBOL(fb_get_options);
+
+/**
+ *	video_setup - process command line options
+ *	@options: string of options
+ *
+ *	Process command line options for frame buffer subsystem.
+ *
+ *	NOTE: This function is a __setup and __init function.
+ *            It only stores the options.  Drivers have to call
+ *            fb_get_options() as necessary.
+ *
+ *	Returns zero.
+ *
+ */
+static int __init video_setup(char *options)
+{
+	int i, global = 0;
+
+	if (!options || !*options)
+		global = 1;
+
+	if (!global && !strncmp(options, "ofonly", 6)) {
+		ofonly = 1;
+		global = 1;
+	}
+
+	if (!global && !strchr(options, ':')) {
+		fb_mode_option = options;
+		global = 1;
+	}
+
+	if (!global) {
+		for (i = 0; i < FB_MAX; i++) {
+			if (video_options[i] == NULL) {
+				video_options[i] = options;
+				break;
+			}
+		}
+	}
+
+	return 1;
+}
+__setup("video=", video_setup);
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index b5e85f6c1c26..0705d8883ede 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -1908,96 +1908,4 @@ int fb_new_modelist(struct fb_info *info)
 	return err;
 }
 
-static char *video_options[FB_MAX] __read_mostly;
-static int ofonly __read_mostly;
-
-/**
- * fb_get_options - get kernel boot parameters
- * @name:   framebuffer name as it would appear in
- *          the boot parameter line
- *          (video=<name>:<options>)
- * @option: the option will be stored here
- *
- * NOTE: Needed to maintain backwards compatibility
- */
-int fb_get_options(const char *name, char **option)
-{
-	char *opt, *options = NULL;
-	int retval = 0;
-	int name_len = strlen(name), i;
-
-	if (name_len && ofonly && strncmp(name, "offb", 4))
-		retval = 1;
-
-	if (name_len && !retval) {
-		for (i = 0; i < FB_MAX; i++) {
-			if (video_options[i] == NULL)
-				continue;
-			if (!video_options[i][0])
-				continue;
-			opt = video_options[i];
-			if (!strncmp(name, opt, name_len) &&
-			    opt[name_len] == ':')
-				options = opt + name_len + 1;
-		}
-	}
-	/* No match, pass global option */
-	if (!options && option && fb_mode_option)
-		options = kstrdup(fb_mode_option, GFP_KERNEL);
-	if (options && !strncmp(options, "off", 3))
-		retval = 1;
-
-	if (option)
-		*option = options;
-
-	return retval;
-}
-EXPORT_SYMBOL(fb_get_options);
-
-#ifndef MODULE
-/**
- *	video_setup - process command line options
- *	@options: string of options
- *
- *	Process command line options for frame buffer subsystem.
- *
- *	NOTE: This function is a __setup and __init function.
- *            It only stores the options.  Drivers have to call
- *            fb_get_options() as necessary.
- *
- *	Returns zero.
- *
- */
-static int __init video_setup(char *options)
-{
-	int i, global = 0;
-
-	if (!options || !*options)
- 		global = 1;
-
- 	if (!global && !strncmp(options, "ofonly", 6)) {
- 		ofonly = 1;
- 		global = 1;
- 	}
-
- 	if (!global && !strchr(options, ':')) {
- 		fb_mode_option = options;
- 		global = 1;
- 	}
-
- 	if (!global) {
- 		for (i = 0; i < FB_MAX; i++) {
- 			if (video_options[i] == NULL) {
- 				video_options[i] = options;
- 				break;
- 			}
-
-		}
-	}
-
-	return 1;
-}
-__setup("video=", video_setup);
-#endif
-
 MODULE_LICENSE("GPL");
-- 
2.0.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] video/fbdev: Always built-in video= cmdline parsing
  2014-08-06  9:43   ` Daniel Vetter
@ 2014-08-06 10:27     ` Geert Uytterhoeven
  -1 siblings, 0 replies; 16+ messages in thread
From: Geert Uytterhoeven @ 2014-08-06 10:27 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Intel Graphics Development, Tomi Valkeinen, Plagniol-Villard,
	Linux Fbdev development list, DRI Development

On Wed, Aug 6, 2014 at 11:43 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> In drm/i915 we want to get at the video= cmdline modes even when we
> don't have fbdev support enabled, so that users can always override
> the kernel's initial mode selection.
>
> But that gives us a direct depency upon the parsing code in the fbdev
> subsystem. Since it's so little code just extract these 2 functions
> and always build them in.

How much is "so little"? Think memory-constrained systems.

You can still build it depending on CONFIG_FB or CONFIG_DRM_I915.

> diff --git a/drivers/video/fbdev/core/Makefile b/drivers/video/fbdev/core/Makefile
> index fa306538dac2..891c1f890e03 100644
> --- a/drivers/video/fbdev/core/Makefile
> +++ b/drivers/video/fbdev/core/Makefile
> @@ -1,4 +1,4 @@
> -obj-y                             += fb_notify.o

Oh, this is already unconditional. Who are its users?

> +obj-y                             += fb_notify.o fb_cmdline.o
>  obj-$(CONFIG_FB)                  += fb.o
>  fb-y                              := fbmem.o fbmon.o fbcmap.o fbsysfs.o \
>                                       modedb.o fbcvt.o
> diff --git a/drivers/video/fbdev/core/fb_cmdline.c b/drivers/video/fbdev/core/fb_cmdline.c
> new file mode 100644
> index 000000000000..91503a43213e
> --- /dev/null
> +++ b/drivers/video/fbdev/core/fb_cmdline.c
> @@ -0,0 +1,103 @@
> +/*
> + *  linux/drivers/video/fb_cmdline.c
> + *
> + *  Copyright (C) 2014 Intel Corp
> + *
> + * This file is subject to the terms and conditions of the GNU General Public
> + * License.  See the file COPYING in the main directory of this archive
> + * for more details.
> + *
> + * Authors:
> + *    Vetter <danie.vetter@ffwll.ch>
> + */

The above chunk doesn't sound appropriate for extracting existing code...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] video/fbdev: Always built-in video= cmdline parsing
@ 2014-08-06 10:27     ` Geert Uytterhoeven
  0 siblings, 0 replies; 16+ messages in thread
From: Geert Uytterhoeven @ 2014-08-06 10:27 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Intel Graphics Development, Tomi Valkeinen, Plagniol-Villard,
	Linux Fbdev development list, DRI Development

On Wed, Aug 6, 2014 at 11:43 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> In drm/i915 we want to get at the video= cmdline modes even when we
> don't have fbdev support enabled, so that users can always override
> the kernel's initial mode selection.
>
> But that gives us a direct depency upon the parsing code in the fbdev
> subsystem. Since it's so little code just extract these 2 functions
> and always build them in.

How much is "so little"? Think memory-constrained systems.

You can still build it depending on CONFIG_FB or CONFIG_DRM_I915.

> diff --git a/drivers/video/fbdev/core/Makefile b/drivers/video/fbdev/core/Makefile
> index fa306538dac2..891c1f890e03 100644
> --- a/drivers/video/fbdev/core/Makefile
> +++ b/drivers/video/fbdev/core/Makefile
> @@ -1,4 +1,4 @@
> -obj-y                             += fb_notify.o

Oh, this is already unconditional. Who are its users?

> +obj-y                             += fb_notify.o fb_cmdline.o
>  obj-$(CONFIG_FB)                  += fb.o
>  fb-y                              := fbmem.o fbmon.o fbcmap.o fbsysfs.o \
>                                       modedb.o fbcvt.o
> diff --git a/drivers/video/fbdev/core/fb_cmdline.c b/drivers/video/fbdev/core/fb_cmdline.c
> new file mode 100644
> index 000000000000..91503a43213e
> --- /dev/null
> +++ b/drivers/video/fbdev/core/fb_cmdline.c
> @@ -0,0 +1,103 @@
> +/*
> + *  linux/drivers/video/fb_cmdline.c
> + *
> + *  Copyright (C) 2014 Intel Corp
> + *
> + * This file is subject to the terms and conditions of the GNU General Public
> + * License.  See the file COPYING in the main directory of this archive
> + * for more details.
> + *
> + * Authors:
> + *    Vetter <danie.vetter@ffwll.ch>
> + */

The above chunk doesn't sound appropriate for extracting existing code...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH] video/fbdev: Always built-in video= cmdline parsing
  2014-08-06  8:08 [PATCH] drm: Perform cmdline mode parsing during connector initialisation Daniel Vetter
@ 2014-08-06 12:05   ` Daniel Vetter
  2014-08-06 12:05   ` Daniel Vetter
  2014-08-06 12:52   ` Daniel Vetter
  2 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2014-08-06 12:05 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, Intel Graphics Development, Tomi Valkeinen,
	Plagniol-Villard, linux-fbdev

In drm/i915 we want to get at the video= cmdline modes even when we
don't have fbdev support enabled, so that users can always override
the kernel's initial mode selection.

But that gives us a direct depency upon the parsing code in the fbdev
subsystem. Since it's so little code just extract these 2 functions
and always build them in.

Whiel at it fix the checkpatch fail in this code.

v2: Also move fb_mode_option. Spotted by the kbuild.

Cc: Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

--

I prefer if we can merge this through drm-next since we'll use it
there in follow-up patches.
-Daniel
---
 drivers/video/fbdev/core/Makefile     |   2 +-
 drivers/video/fbdev/core/fb_cmdline.c | 106 ++++++++++++++++++++++++++++++++++
 drivers/video/fbdev/core/fbmem.c      |  92 -----------------------------
 drivers/video/fbdev/core/modedb.c     |   3 -
 4 files changed, 107 insertions(+), 96 deletions(-)
 create mode 100644 drivers/video/fbdev/core/fb_cmdline.c

diff --git a/drivers/video/fbdev/core/Makefile b/drivers/video/fbdev/core/Makefile
index fa306538dac2..891c1f890e03 100644
--- a/drivers/video/fbdev/core/Makefile
+++ b/drivers/video/fbdev/core/Makefile
@@ -1,4 +1,4 @@
-obj-y                             += fb_notify.o
+obj-y                             += fb_notify.o fb_cmdline.o
 obj-$(CONFIG_FB)                  += fb.o
 fb-y                              := fbmem.o fbmon.o fbcmap.o fbsysfs.o \
                                      modedb.o fbcvt.o
diff --git a/drivers/video/fbdev/core/fb_cmdline.c b/drivers/video/fbdev/core/fb_cmdline.c
new file mode 100644
index 000000000000..e8ce614bdd03
--- /dev/null
+++ b/drivers/video/fbdev/core/fb_cmdline.c
@@ -0,0 +1,106 @@
+/*
+ *  linux/drivers/video/fb_cmdline.c
+ *
+ *  Copyright (C) 2014 Intel Corp
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file COPYING in the main directory of this archive
+ * for more details.
+ *
+ * Authors:
+ *    Vetter <danie.vetter@ffwll.ch>
+ */
+#include <linux/init.h>
+#include <linux/fb.h>
+
+static char *video_options[FB_MAX] __read_mostly;
+static int ofonly __read_mostly;
+
+const char *fb_mode_option;
+EXPORT_SYMBOL_GPL(fb_mode_option);
+
+/**
+ * fb_get_options - get kernel boot parameters
+ * @name:   framebuffer name as it would appear in
+ *          the boot parameter line
+ *          (video=<name>:<options>)
+ * @option: the option will be stored here
+ *
+ * NOTE: Needed to maintain backwards compatibility
+ */
+int fb_get_options(const char *name, char **option)
+{
+	char *opt, *options = NULL;
+	int retval = 0;
+	int name_len = strlen(name), i;
+
+	if (name_len && ofonly && strncmp(name, "offb", 4))
+		retval = 1;
+
+	if (name_len && !retval) {
+		for (i = 0; i < FB_MAX; i++) {
+			if (video_options[i] = NULL)
+				continue;
+			if (!video_options[i][0])
+				continue;
+			opt = video_options[i];
+			if (!strncmp(name, opt, name_len) &&
+			    opt[name_len] = ':')
+				options = opt + name_len + 1;
+		}
+	}
+	/* No match, pass global option */
+	if (!options && option && fb_mode_option)
+		options = kstrdup(fb_mode_option, GFP_KERNEL);
+	if (options && !strncmp(options, "off", 3))
+		retval = 1;
+
+	if (option)
+		*option = options;
+
+	return retval;
+}
+EXPORT_SYMBOL(fb_get_options);
+
+/**
+ *	video_setup - process command line options
+ *	@options: string of options
+ *
+ *	Process command line options for frame buffer subsystem.
+ *
+ *	NOTE: This function is a __setup and __init function.
+ *            It only stores the options.  Drivers have to call
+ *            fb_get_options() as necessary.
+ *
+ *	Returns zero.
+ *
+ */
+static int __init video_setup(char *options)
+{
+	int i, global = 0;
+
+	if (!options || !*options)
+		global = 1;
+
+	if (!global && !strncmp(options, "ofonly", 6)) {
+		ofonly = 1;
+		global = 1;
+	}
+
+	if (!global && !strchr(options, ':')) {
+		fb_mode_option = options;
+		global = 1;
+	}
+
+	if (!global) {
+		for (i = 0; i < FB_MAX; i++) {
+			if (video_options[i] = NULL) {
+				video_options[i] = options;
+				break;
+			}
+		}
+	}
+
+	return 1;
+}
+__setup("video=", video_setup);
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index b5e85f6c1c26..0705d8883ede 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -1908,96 +1908,4 @@ int fb_new_modelist(struct fb_info *info)
 	return err;
 }
 
-static char *video_options[FB_MAX] __read_mostly;
-static int ofonly __read_mostly;
-
-/**
- * fb_get_options - get kernel boot parameters
- * @name:   framebuffer name as it would appear in
- *          the boot parameter line
- *          (video=<name>:<options>)
- * @option: the option will be stored here
- *
- * NOTE: Needed to maintain backwards compatibility
- */
-int fb_get_options(const char *name, char **option)
-{
-	char *opt, *options = NULL;
-	int retval = 0;
-	int name_len = strlen(name), i;
-
-	if (name_len && ofonly && strncmp(name, "offb", 4))
-		retval = 1;
-
-	if (name_len && !retval) {
-		for (i = 0; i < FB_MAX; i++) {
-			if (video_options[i] = NULL)
-				continue;
-			if (!video_options[i][0])
-				continue;
-			opt = video_options[i];
-			if (!strncmp(name, opt, name_len) &&
-			    opt[name_len] = ':')
-				options = opt + name_len + 1;
-		}
-	}
-	/* No match, pass global option */
-	if (!options && option && fb_mode_option)
-		options = kstrdup(fb_mode_option, GFP_KERNEL);
-	if (options && !strncmp(options, "off", 3))
-		retval = 1;
-
-	if (option)
-		*option = options;
-
-	return retval;
-}
-EXPORT_SYMBOL(fb_get_options);
-
-#ifndef MODULE
-/**
- *	video_setup - process command line options
- *	@options: string of options
- *
- *	Process command line options for frame buffer subsystem.
- *
- *	NOTE: This function is a __setup and __init function.
- *            It only stores the options.  Drivers have to call
- *            fb_get_options() as necessary.
- *
- *	Returns zero.
- *
- */
-static int __init video_setup(char *options)
-{
-	int i, global = 0;
-
-	if (!options || !*options)
- 		global = 1;
-
- 	if (!global && !strncmp(options, "ofonly", 6)) {
- 		ofonly = 1;
- 		global = 1;
- 	}
-
- 	if (!global && !strchr(options, ':')) {
- 		fb_mode_option = options;
- 		global = 1;
- 	}
-
- 	if (!global) {
- 		for (i = 0; i < FB_MAX; i++) {
- 			if (video_options[i] = NULL) {
- 				video_options[i] = options;
- 				break;
- 			}
-
-		}
-	}
-
-	return 1;
-}
-__setup("video=", video_setup);
-#endif
-
 MODULE_LICENSE("GPL");
diff --git a/drivers/video/fbdev/core/modedb.c b/drivers/video/fbdev/core/modedb.c
index a9a907c440d7..388f7971494b 100644
--- a/drivers/video/fbdev/core/modedb.c
+++ b/drivers/video/fbdev/core/modedb.c
@@ -29,9 +29,6 @@
 #define DPRINTK(fmt, args...)
 #endif
 
-const char *fb_mode_option;
-EXPORT_SYMBOL_GPL(fb_mode_option);
-
 /*
  *  Standard video mode definitions (taken from XFree86)
  */
-- 
2.0.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH] video/fbdev: Always built-in video= cmdline parsing
@ 2014-08-06 12:05   ` Daniel Vetter
  0 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2014-08-06 12:05 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, Intel Graphics Development, Tomi Valkeinen,
	Plagniol-Villard, linux-fbdev

In drm/i915 we want to get at the video= cmdline modes even when we
don't have fbdev support enabled, so that users can always override
the kernel's initial mode selection.

But that gives us a direct depency upon the parsing code in the fbdev
subsystem. Since it's so little code just extract these 2 functions
and always build them in.

Whiel at it fix the checkpatch fail in this code.

v2: Also move fb_mode_option. Spotted by the kbuild.

Cc: Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

--

I prefer if we can merge this through drm-next since we'll use it
there in follow-up patches.
-Daniel
---
 drivers/video/fbdev/core/Makefile     |   2 +-
 drivers/video/fbdev/core/fb_cmdline.c | 106 ++++++++++++++++++++++++++++++++++
 drivers/video/fbdev/core/fbmem.c      |  92 -----------------------------
 drivers/video/fbdev/core/modedb.c     |   3 -
 4 files changed, 107 insertions(+), 96 deletions(-)
 create mode 100644 drivers/video/fbdev/core/fb_cmdline.c

diff --git a/drivers/video/fbdev/core/Makefile b/drivers/video/fbdev/core/Makefile
index fa306538dac2..891c1f890e03 100644
--- a/drivers/video/fbdev/core/Makefile
+++ b/drivers/video/fbdev/core/Makefile
@@ -1,4 +1,4 @@
-obj-y                             += fb_notify.o
+obj-y                             += fb_notify.o fb_cmdline.o
 obj-$(CONFIG_FB)                  += fb.o
 fb-y                              := fbmem.o fbmon.o fbcmap.o fbsysfs.o \
                                      modedb.o fbcvt.o
diff --git a/drivers/video/fbdev/core/fb_cmdline.c b/drivers/video/fbdev/core/fb_cmdline.c
new file mode 100644
index 000000000000..e8ce614bdd03
--- /dev/null
+++ b/drivers/video/fbdev/core/fb_cmdline.c
@@ -0,0 +1,106 @@
+/*
+ *  linux/drivers/video/fb_cmdline.c
+ *
+ *  Copyright (C) 2014 Intel Corp
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file COPYING in the main directory of this archive
+ * for more details.
+ *
+ * Authors:
+ *    Vetter <danie.vetter@ffwll.ch>
+ */
+#include <linux/init.h>
+#include <linux/fb.h>
+
+static char *video_options[FB_MAX] __read_mostly;
+static int ofonly __read_mostly;
+
+const char *fb_mode_option;
+EXPORT_SYMBOL_GPL(fb_mode_option);
+
+/**
+ * fb_get_options - get kernel boot parameters
+ * @name:   framebuffer name as it would appear in
+ *          the boot parameter line
+ *          (video=<name>:<options>)
+ * @option: the option will be stored here
+ *
+ * NOTE: Needed to maintain backwards compatibility
+ */
+int fb_get_options(const char *name, char **option)
+{
+	char *opt, *options = NULL;
+	int retval = 0;
+	int name_len = strlen(name), i;
+
+	if (name_len && ofonly && strncmp(name, "offb", 4))
+		retval = 1;
+
+	if (name_len && !retval) {
+		for (i = 0; i < FB_MAX; i++) {
+			if (video_options[i] == NULL)
+				continue;
+			if (!video_options[i][0])
+				continue;
+			opt = video_options[i];
+			if (!strncmp(name, opt, name_len) &&
+			    opt[name_len] == ':')
+				options = opt + name_len + 1;
+		}
+	}
+	/* No match, pass global option */
+	if (!options && option && fb_mode_option)
+		options = kstrdup(fb_mode_option, GFP_KERNEL);
+	if (options && !strncmp(options, "off", 3))
+		retval = 1;
+
+	if (option)
+		*option = options;
+
+	return retval;
+}
+EXPORT_SYMBOL(fb_get_options);
+
+/**
+ *	video_setup - process command line options
+ *	@options: string of options
+ *
+ *	Process command line options for frame buffer subsystem.
+ *
+ *	NOTE: This function is a __setup and __init function.
+ *            It only stores the options.  Drivers have to call
+ *            fb_get_options() as necessary.
+ *
+ *	Returns zero.
+ *
+ */
+static int __init video_setup(char *options)
+{
+	int i, global = 0;
+
+	if (!options || !*options)
+		global = 1;
+
+	if (!global && !strncmp(options, "ofonly", 6)) {
+		ofonly = 1;
+		global = 1;
+	}
+
+	if (!global && !strchr(options, ':')) {
+		fb_mode_option = options;
+		global = 1;
+	}
+
+	if (!global) {
+		for (i = 0; i < FB_MAX; i++) {
+			if (video_options[i] == NULL) {
+				video_options[i] = options;
+				break;
+			}
+		}
+	}
+
+	return 1;
+}
+__setup("video=", video_setup);
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index b5e85f6c1c26..0705d8883ede 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -1908,96 +1908,4 @@ int fb_new_modelist(struct fb_info *info)
 	return err;
 }
 
-static char *video_options[FB_MAX] __read_mostly;
-static int ofonly __read_mostly;
-
-/**
- * fb_get_options - get kernel boot parameters
- * @name:   framebuffer name as it would appear in
- *          the boot parameter line
- *          (video=<name>:<options>)
- * @option: the option will be stored here
- *
- * NOTE: Needed to maintain backwards compatibility
- */
-int fb_get_options(const char *name, char **option)
-{
-	char *opt, *options = NULL;
-	int retval = 0;
-	int name_len = strlen(name), i;
-
-	if (name_len && ofonly && strncmp(name, "offb", 4))
-		retval = 1;
-
-	if (name_len && !retval) {
-		for (i = 0; i < FB_MAX; i++) {
-			if (video_options[i] == NULL)
-				continue;
-			if (!video_options[i][0])
-				continue;
-			opt = video_options[i];
-			if (!strncmp(name, opt, name_len) &&
-			    opt[name_len] == ':')
-				options = opt + name_len + 1;
-		}
-	}
-	/* No match, pass global option */
-	if (!options && option && fb_mode_option)
-		options = kstrdup(fb_mode_option, GFP_KERNEL);
-	if (options && !strncmp(options, "off", 3))
-		retval = 1;
-
-	if (option)
-		*option = options;
-
-	return retval;
-}
-EXPORT_SYMBOL(fb_get_options);
-
-#ifndef MODULE
-/**
- *	video_setup - process command line options
- *	@options: string of options
- *
- *	Process command line options for frame buffer subsystem.
- *
- *	NOTE: This function is a __setup and __init function.
- *            It only stores the options.  Drivers have to call
- *            fb_get_options() as necessary.
- *
- *	Returns zero.
- *
- */
-static int __init video_setup(char *options)
-{
-	int i, global = 0;
-
-	if (!options || !*options)
- 		global = 1;
-
- 	if (!global && !strncmp(options, "ofonly", 6)) {
- 		ofonly = 1;
- 		global = 1;
- 	}
-
- 	if (!global && !strchr(options, ':')) {
- 		fb_mode_option = options;
- 		global = 1;
- 	}
-
- 	if (!global) {
- 		for (i = 0; i < FB_MAX; i++) {
- 			if (video_options[i] == NULL) {
- 				video_options[i] = options;
- 				break;
- 			}
-
-		}
-	}
-
-	return 1;
-}
-__setup("video=", video_setup);
-#endif
-
 MODULE_LICENSE("GPL");
diff --git a/drivers/video/fbdev/core/modedb.c b/drivers/video/fbdev/core/modedb.c
index a9a907c440d7..388f7971494b 100644
--- a/drivers/video/fbdev/core/modedb.c
+++ b/drivers/video/fbdev/core/modedb.c
@@ -29,9 +29,6 @@
 #define DPRINTK(fmt, args...)
 #endif
 
-const char *fb_mode_option;
-EXPORT_SYMBOL_GPL(fb_mode_option);
-
 /*
  *  Standard video mode definitions (taken from XFree86)
  */
-- 
2.0.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] video/fbdev: Always built-in video= cmdline parsing
  2014-08-06 10:27     ` Geert Uytterhoeven
@ 2014-08-06 12:48       ` Daniel Vetter
  -1 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2014-08-06 12:48 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Linux Fbdev development list, Daniel Vetter,
	Intel Graphics Development, DRI Development, Tomi Valkeinen,
	Plagniol-Villard

On Wed, Aug 06, 2014 at 12:27:32PM +0200, Geert Uytterhoeven wrote:
> On Wed, Aug 6, 2014 at 11:43 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > In drm/i915 we want to get at the video= cmdline modes even when we
> > don't have fbdev support enabled, so that users can always override
> > the kernel's initial mode selection.
> >
> > But that gives us a direct depency upon the parsing code in the fbdev
> > subsystem. Since it's so little code just extract these 2 functions
> > and always build them in.
> 
> How much is "so little"? Think memory-constrained systems.
> 
> You can still build it depending on CONFIG_FB or CONFIG_DRM_I915.

I'll do it as an option selected by FB and DRM then.
> 
> > diff --git a/drivers/video/fbdev/core/Makefile b/drivers/video/fbdev/core/Makefile
> > index fa306538dac2..891c1f890e03 100644
> > --- a/drivers/video/fbdev/core/Makefile
> > +++ b/drivers/video/fbdev/core/Makefile
> > @@ -1,4 +1,4 @@
> > -obj-y                             += fb_notify.o
> 
> Oh, this is already unconditional. Who are its users?

Welcome in fbdev-land. Not going to fix this, since my dragon-slaying
sword is already broken.

> 
> > +obj-y                             += fb_notify.o fb_cmdline.o
> >  obj-$(CONFIG_FB)                  += fb.o
> >  fb-y                              := fbmem.o fbmon.o fbcmap.o fbsysfs.o \
> >                                       modedb.o fbcvt.o
> > diff --git a/drivers/video/fbdev/core/fb_cmdline.c b/drivers/video/fbdev/core/fb_cmdline.c
> > new file mode 100644
> > index 000000000000..91503a43213e
> > --- /dev/null
> > +++ b/drivers/video/fbdev/core/fb_cmdline.c
> > @@ -0,0 +1,103 @@
> > +/*
> > + *  linux/drivers/video/fb_cmdline.c
> > + *
> > + *  Copyright (C) 2014 Intel Corp
> > + *
> > + * This file is subject to the terms and conditions of the GNU General Public
> > + * License.  See the file COPYING in the main directory of this archive
> > + * for more details.
> > + *
> > + * Authors:
> > + *    Vetter <danie.vetter@ffwll.ch>
> > + */
> 
> The above chunk doesn't sound appropriate for extracting existing code...

Well it all horribly predates git history, so no idea who actually wrote
this. I'll copy over the old header on top.
-Daniel
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] video/fbdev: Always built-in video= cmdline parsing
@ 2014-08-06 12:48       ` Daniel Vetter
  0 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2014-08-06 12:48 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Linux Fbdev development list, Daniel Vetter,
	Intel Graphics Development, DRI Development, Tomi Valkeinen,
	Plagniol-Villard

On Wed, Aug 06, 2014 at 12:27:32PM +0200, Geert Uytterhoeven wrote:
> On Wed, Aug 6, 2014 at 11:43 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > In drm/i915 we want to get at the video= cmdline modes even when we
> > don't have fbdev support enabled, so that users can always override
> > the kernel's initial mode selection.
> >
> > But that gives us a direct depency upon the parsing code in the fbdev
> > subsystem. Since it's so little code just extract these 2 functions
> > and always build them in.
> 
> How much is "so little"? Think memory-constrained systems.
> 
> You can still build it depending on CONFIG_FB or CONFIG_DRM_I915.

I'll do it as an option selected by FB and DRM then.
> 
> > diff --git a/drivers/video/fbdev/core/Makefile b/drivers/video/fbdev/core/Makefile
> > index fa306538dac2..891c1f890e03 100644
> > --- a/drivers/video/fbdev/core/Makefile
> > +++ b/drivers/video/fbdev/core/Makefile
> > @@ -1,4 +1,4 @@
> > -obj-y                             += fb_notify.o
> 
> Oh, this is already unconditional. Who are its users?

Welcome in fbdev-land. Not going to fix this, since my dragon-slaying
sword is already broken.

> 
> > +obj-y                             += fb_notify.o fb_cmdline.o
> >  obj-$(CONFIG_FB)                  += fb.o
> >  fb-y                              := fbmem.o fbmon.o fbcmap.o fbsysfs.o \
> >                                       modedb.o fbcvt.o
> > diff --git a/drivers/video/fbdev/core/fb_cmdline.c b/drivers/video/fbdev/core/fb_cmdline.c
> > new file mode 100644
> > index 000000000000..91503a43213e
> > --- /dev/null
> > +++ b/drivers/video/fbdev/core/fb_cmdline.c
> > @@ -0,0 +1,103 @@
> > +/*
> > + *  linux/drivers/video/fb_cmdline.c
> > + *
> > + *  Copyright (C) 2014 Intel Corp
> > + *
> > + * This file is subject to the terms and conditions of the GNU General Public
> > + * License.  See the file COPYING in the main directory of this archive
> > + * for more details.
> > + *
> > + * Authors:
> > + *    Vetter <danie.vetter@ffwll.ch>
> > + */
> 
> The above chunk doesn't sound appropriate for extracting existing code...

Well it all horribly predates git history, so no idea who actually wrote
this. I'll copy over the old header on top.
-Daniel
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/2] video/fbdev: Always built-in video= cmdline parsing
  2014-08-06  8:08 [PATCH] drm: Perform cmdline mode parsing during connector initialisation Daniel Vetter
@ 2014-08-06 12:52   ` Daniel Vetter
  2014-08-06 12:05   ` Daniel Vetter
  2014-08-06 12:52   ` Daniel Vetter
  2 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2014-08-06 12:52 UTC (permalink / raw)
  To: DRI Development
  Cc: linux-fbdev, Daniel Vetter, Intel Graphics Development,
	Tomi Valkeinen, Geert Uytterhoeven, Plagniol-Villard

In drm/i915 we want to get at the video= cmdline modes even when we
don't have fbdev support enabled, so that users can always override
the kernel's initial mode selection.

But that gives us a direct depency upon the parsing code in the fbdev
subsystem. Since it's so little code just extract these 2 functions
and always build them in.

Whiel at it fix the checkpatch fail in this code.

v2: Also move fb_mode_option. Spotted by the kbuild.

v3: Review from Geert:
- Keep the old copyright notice from fb_mem.c, although I have no
idea what exactly applies.
- Only compile this when needed.

Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

--

I prefer if we can merge this through drm-next since we'll use it
there in follow-up patches.
-Daniel
---
 drivers/video/fbdev/Kconfig           |   4 ++
 drivers/video/fbdev/core/Makefile     |   1 +
 drivers/video/fbdev/core/fb_cmdline.c | 110 ++++++++++++++++++++++++++++++++++
 drivers/video/fbdev/core/fbmem.c      |  92 ----------------------------
 drivers/video/fbdev/core/modedb.c     |   3 -
 5 files changed, 115 insertions(+), 95 deletions(-)
 create mode 100644 drivers/video/fbdev/core/fb_cmdline.c

diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index 59c98bfd5a8a..f1458c95a688 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -4,6 +4,7 @@
 
 menuconfig FB
 	tristate "Support for frame buffer devices"
+	select FB_CMDLINE
 	---help---
 	  The frame buffer device provides an abstraction for the graphics
 	  hardware. It represents the frame buffer of some video hardware and
@@ -52,6 +53,9 @@ config FIRMWARE_EDID
 	 combination with certain motherboards and monitors are known to
 	 suffer from this problem.
 
+config FB_CMDLINE
+	bool
+
 config FB_DDC
        tristate
        depends on FB
diff --git a/drivers/video/fbdev/core/Makefile b/drivers/video/fbdev/core/Makefile
index fa306538dac2..67f28e20a892 100644
--- a/drivers/video/fbdev/core/Makefile
+++ b/drivers/video/fbdev/core/Makefile
@@ -1,4 +1,5 @@
 obj-y                             += fb_notify.o
+obj-$(CONFIG_FB_CMDLINE)          += fb_cmdline.o
 obj-$(CONFIG_FB)                  += fb.o
 fb-y                              := fbmem.o fbmon.o fbcmap.o fbsysfs.o \
                                      modedb.o fbcvt.o
diff --git a/drivers/video/fbdev/core/fb_cmdline.c b/drivers/video/fbdev/core/fb_cmdline.c
new file mode 100644
index 000000000000..39509ccd92f1
--- /dev/null
+++ b/drivers/video/fbdev/core/fb_cmdline.c
@@ -0,0 +1,110 @@
+/*
+ *  linux/drivers/video/fb_cmdline.c
+ *
+ *  Copyright (C) 2014 Intel Corp
+ *  Copyright (C) 1994 Martin Schaller
+ *
+ *	2001 - Documented with DocBook
+ *	- Brad Douglas <brad@neruo.com>
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file COPYING in the main directory of this archive
+ * for more details.
+ *
+ * Authors:
+ *    Vetter <danie.vetter@ffwll.ch>
+ */
+#include <linux/init.h>
+#include <linux/fb.h>
+
+static char *video_options[FB_MAX] __read_mostly;
+static int ofonly __read_mostly;
+
+const char *fb_mode_option;
+EXPORT_SYMBOL_GPL(fb_mode_option);
+
+/**
+ * fb_get_options - get kernel boot parameters
+ * @name:   framebuffer name as it would appear in
+ *          the boot parameter line
+ *          (video=<name>:<options>)
+ * @option: the option will be stored here
+ *
+ * NOTE: Needed to maintain backwards compatibility
+ */
+int fb_get_options(const char *name, char **option)
+{
+	char *opt, *options = NULL;
+	int retval = 0;
+	int name_len = strlen(name), i;
+
+	if (name_len && ofonly && strncmp(name, "offb", 4))
+		retval = 1;
+
+	if (name_len && !retval) {
+		for (i = 0; i < FB_MAX; i++) {
+			if (video_options[i] = NULL)
+				continue;
+			if (!video_options[i][0])
+				continue;
+			opt = video_options[i];
+			if (!strncmp(name, opt, name_len) &&
+			    opt[name_len] = ':')
+				options = opt + name_len + 1;
+		}
+	}
+	/* No match, pass global option */
+	if (!options && option && fb_mode_option)
+		options = kstrdup(fb_mode_option, GFP_KERNEL);
+	if (options && !strncmp(options, "off", 3))
+		retval = 1;
+
+	if (option)
+		*option = options;
+
+	return retval;
+}
+EXPORT_SYMBOL(fb_get_options);
+
+/**
+ *	video_setup - process command line options
+ *	@options: string of options
+ *
+ *	Process command line options for frame buffer subsystem.
+ *
+ *	NOTE: This function is a __setup and __init function.
+ *            It only stores the options.  Drivers have to call
+ *            fb_get_options() as necessary.
+ *
+ *	Returns zero.
+ *
+ */
+static int __init video_setup(char *options)
+{
+	int i, global = 0;
+
+	if (!options || !*options)
+		global = 1;
+
+	if (!global && !strncmp(options, "ofonly", 6)) {
+		ofonly = 1;
+		global = 1;
+	}
+
+	if (!global && !strchr(options, ':')) {
+		fb_mode_option = options;
+		global = 1;
+	}
+
+	if (!global) {
+		for (i = 0; i < FB_MAX; i++) {
+			if (video_options[i] = NULL) {
+				video_options[i] = options;
+				break;
+			}
+		}
+	}
+
+	return 1;
+}
+__setup("video=", video_setup);
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index b5e85f6c1c26..0705d8883ede 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -1908,96 +1908,4 @@ int fb_new_modelist(struct fb_info *info)
 	return err;
 }
 
-static char *video_options[FB_MAX] __read_mostly;
-static int ofonly __read_mostly;
-
-/**
- * fb_get_options - get kernel boot parameters
- * @name:   framebuffer name as it would appear in
- *          the boot parameter line
- *          (video=<name>:<options>)
- * @option: the option will be stored here
- *
- * NOTE: Needed to maintain backwards compatibility
- */
-int fb_get_options(const char *name, char **option)
-{
-	char *opt, *options = NULL;
-	int retval = 0;
-	int name_len = strlen(name), i;
-
-	if (name_len && ofonly && strncmp(name, "offb", 4))
-		retval = 1;
-
-	if (name_len && !retval) {
-		for (i = 0; i < FB_MAX; i++) {
-			if (video_options[i] = NULL)
-				continue;
-			if (!video_options[i][0])
-				continue;
-			opt = video_options[i];
-			if (!strncmp(name, opt, name_len) &&
-			    opt[name_len] = ':')
-				options = opt + name_len + 1;
-		}
-	}
-	/* No match, pass global option */
-	if (!options && option && fb_mode_option)
-		options = kstrdup(fb_mode_option, GFP_KERNEL);
-	if (options && !strncmp(options, "off", 3))
-		retval = 1;
-
-	if (option)
-		*option = options;
-
-	return retval;
-}
-EXPORT_SYMBOL(fb_get_options);
-
-#ifndef MODULE
-/**
- *	video_setup - process command line options
- *	@options: string of options
- *
- *	Process command line options for frame buffer subsystem.
- *
- *	NOTE: This function is a __setup and __init function.
- *            It only stores the options.  Drivers have to call
- *            fb_get_options() as necessary.
- *
- *	Returns zero.
- *
- */
-static int __init video_setup(char *options)
-{
-	int i, global = 0;
-
-	if (!options || !*options)
- 		global = 1;
-
- 	if (!global && !strncmp(options, "ofonly", 6)) {
- 		ofonly = 1;
- 		global = 1;
- 	}
-
- 	if (!global && !strchr(options, ':')) {
- 		fb_mode_option = options;
- 		global = 1;
- 	}
-
- 	if (!global) {
- 		for (i = 0; i < FB_MAX; i++) {
- 			if (video_options[i] = NULL) {
- 				video_options[i] = options;
- 				break;
- 			}
-
-		}
-	}
-
-	return 1;
-}
-__setup("video=", video_setup);
-#endif
-
 MODULE_LICENSE("GPL");
diff --git a/drivers/video/fbdev/core/modedb.c b/drivers/video/fbdev/core/modedb.c
index a9a907c440d7..388f7971494b 100644
--- a/drivers/video/fbdev/core/modedb.c
+++ b/drivers/video/fbdev/core/modedb.c
@@ -29,9 +29,6 @@
 #define DPRINTK(fmt, args...)
 #endif
 
-const char *fb_mode_option;
-EXPORT_SYMBOL_GPL(fb_mode_option);
-
 /*
  *  Standard video mode definitions (taken from XFree86)
  */
-- 
2.0.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 1/2] video/fbdev: Always built-in video= cmdline parsing
@ 2014-08-06 12:52   ` Daniel Vetter
  0 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2014-08-06 12:52 UTC (permalink / raw)
  To: DRI Development
  Cc: linux-fbdev, Daniel Vetter, Intel Graphics Development,
	Tomi Valkeinen, Geert Uytterhoeven, Plagniol-Villard

In drm/i915 we want to get at the video= cmdline modes even when we
don't have fbdev support enabled, so that users can always override
the kernel's initial mode selection.

But that gives us a direct depency upon the parsing code in the fbdev
subsystem. Since it's so little code just extract these 2 functions
and always build them in.

Whiel at it fix the checkpatch fail in this code.

v2: Also move fb_mode_option. Spotted by the kbuild.

v3: Review from Geert:
- Keep the old copyright notice from fb_mem.c, although I have no
idea what exactly applies.
- Only compile this when needed.

Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

--

I prefer if we can merge this through drm-next since we'll use it
there in follow-up patches.
-Daniel
---
 drivers/video/fbdev/Kconfig           |   4 ++
 drivers/video/fbdev/core/Makefile     |   1 +
 drivers/video/fbdev/core/fb_cmdline.c | 110 ++++++++++++++++++++++++++++++++++
 drivers/video/fbdev/core/fbmem.c      |  92 ----------------------------
 drivers/video/fbdev/core/modedb.c     |   3 -
 5 files changed, 115 insertions(+), 95 deletions(-)
 create mode 100644 drivers/video/fbdev/core/fb_cmdline.c

diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index 59c98bfd5a8a..f1458c95a688 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -4,6 +4,7 @@
 
 menuconfig FB
 	tristate "Support for frame buffer devices"
+	select FB_CMDLINE
 	---help---
 	  The frame buffer device provides an abstraction for the graphics
 	  hardware. It represents the frame buffer of some video hardware and
@@ -52,6 +53,9 @@ config FIRMWARE_EDID
 	 combination with certain motherboards and monitors are known to
 	 suffer from this problem.
 
+config FB_CMDLINE
+	bool
+
 config FB_DDC
        tristate
        depends on FB
diff --git a/drivers/video/fbdev/core/Makefile b/drivers/video/fbdev/core/Makefile
index fa306538dac2..67f28e20a892 100644
--- a/drivers/video/fbdev/core/Makefile
+++ b/drivers/video/fbdev/core/Makefile
@@ -1,4 +1,5 @@
 obj-y                             += fb_notify.o
+obj-$(CONFIG_FB_CMDLINE)          += fb_cmdline.o
 obj-$(CONFIG_FB)                  += fb.o
 fb-y                              := fbmem.o fbmon.o fbcmap.o fbsysfs.o \
                                      modedb.o fbcvt.o
diff --git a/drivers/video/fbdev/core/fb_cmdline.c b/drivers/video/fbdev/core/fb_cmdline.c
new file mode 100644
index 000000000000..39509ccd92f1
--- /dev/null
+++ b/drivers/video/fbdev/core/fb_cmdline.c
@@ -0,0 +1,110 @@
+/*
+ *  linux/drivers/video/fb_cmdline.c
+ *
+ *  Copyright (C) 2014 Intel Corp
+ *  Copyright (C) 1994 Martin Schaller
+ *
+ *	2001 - Documented with DocBook
+ *	- Brad Douglas <brad@neruo.com>
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file COPYING in the main directory of this archive
+ * for more details.
+ *
+ * Authors:
+ *    Vetter <danie.vetter@ffwll.ch>
+ */
+#include <linux/init.h>
+#include <linux/fb.h>
+
+static char *video_options[FB_MAX] __read_mostly;
+static int ofonly __read_mostly;
+
+const char *fb_mode_option;
+EXPORT_SYMBOL_GPL(fb_mode_option);
+
+/**
+ * fb_get_options - get kernel boot parameters
+ * @name:   framebuffer name as it would appear in
+ *          the boot parameter line
+ *          (video=<name>:<options>)
+ * @option: the option will be stored here
+ *
+ * NOTE: Needed to maintain backwards compatibility
+ */
+int fb_get_options(const char *name, char **option)
+{
+	char *opt, *options = NULL;
+	int retval = 0;
+	int name_len = strlen(name), i;
+
+	if (name_len && ofonly && strncmp(name, "offb", 4))
+		retval = 1;
+
+	if (name_len && !retval) {
+		for (i = 0; i < FB_MAX; i++) {
+			if (video_options[i] == NULL)
+				continue;
+			if (!video_options[i][0])
+				continue;
+			opt = video_options[i];
+			if (!strncmp(name, opt, name_len) &&
+			    opt[name_len] == ':')
+				options = opt + name_len + 1;
+		}
+	}
+	/* No match, pass global option */
+	if (!options && option && fb_mode_option)
+		options = kstrdup(fb_mode_option, GFP_KERNEL);
+	if (options && !strncmp(options, "off", 3))
+		retval = 1;
+
+	if (option)
+		*option = options;
+
+	return retval;
+}
+EXPORT_SYMBOL(fb_get_options);
+
+/**
+ *	video_setup - process command line options
+ *	@options: string of options
+ *
+ *	Process command line options for frame buffer subsystem.
+ *
+ *	NOTE: This function is a __setup and __init function.
+ *            It only stores the options.  Drivers have to call
+ *            fb_get_options() as necessary.
+ *
+ *	Returns zero.
+ *
+ */
+static int __init video_setup(char *options)
+{
+	int i, global = 0;
+
+	if (!options || !*options)
+		global = 1;
+
+	if (!global && !strncmp(options, "ofonly", 6)) {
+		ofonly = 1;
+		global = 1;
+	}
+
+	if (!global && !strchr(options, ':')) {
+		fb_mode_option = options;
+		global = 1;
+	}
+
+	if (!global) {
+		for (i = 0; i < FB_MAX; i++) {
+			if (video_options[i] == NULL) {
+				video_options[i] = options;
+				break;
+			}
+		}
+	}
+
+	return 1;
+}
+__setup("video=", video_setup);
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index b5e85f6c1c26..0705d8883ede 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -1908,96 +1908,4 @@ int fb_new_modelist(struct fb_info *info)
 	return err;
 }
 
-static char *video_options[FB_MAX] __read_mostly;
-static int ofonly __read_mostly;
-
-/**
- * fb_get_options - get kernel boot parameters
- * @name:   framebuffer name as it would appear in
- *          the boot parameter line
- *          (video=<name>:<options>)
- * @option: the option will be stored here
- *
- * NOTE: Needed to maintain backwards compatibility
- */
-int fb_get_options(const char *name, char **option)
-{
-	char *opt, *options = NULL;
-	int retval = 0;
-	int name_len = strlen(name), i;
-
-	if (name_len && ofonly && strncmp(name, "offb", 4))
-		retval = 1;
-
-	if (name_len && !retval) {
-		for (i = 0; i < FB_MAX; i++) {
-			if (video_options[i] == NULL)
-				continue;
-			if (!video_options[i][0])
-				continue;
-			opt = video_options[i];
-			if (!strncmp(name, opt, name_len) &&
-			    opt[name_len] == ':')
-				options = opt + name_len + 1;
-		}
-	}
-	/* No match, pass global option */
-	if (!options && option && fb_mode_option)
-		options = kstrdup(fb_mode_option, GFP_KERNEL);
-	if (options && !strncmp(options, "off", 3))
-		retval = 1;
-
-	if (option)
-		*option = options;
-
-	return retval;
-}
-EXPORT_SYMBOL(fb_get_options);
-
-#ifndef MODULE
-/**
- *	video_setup - process command line options
- *	@options: string of options
- *
- *	Process command line options for frame buffer subsystem.
- *
- *	NOTE: This function is a __setup and __init function.
- *            It only stores the options.  Drivers have to call
- *            fb_get_options() as necessary.
- *
- *	Returns zero.
- *
- */
-static int __init video_setup(char *options)
-{
-	int i, global = 0;
-
-	if (!options || !*options)
- 		global = 1;
-
- 	if (!global && !strncmp(options, "ofonly", 6)) {
- 		ofonly = 1;
- 		global = 1;
- 	}
-
- 	if (!global && !strchr(options, ':')) {
- 		fb_mode_option = options;
- 		global = 1;
- 	}
-
- 	if (!global) {
- 		for (i = 0; i < FB_MAX; i++) {
- 			if (video_options[i] == NULL) {
- 				video_options[i] = options;
- 				break;
- 			}
-
-		}
-	}
-
-	return 1;
-}
-__setup("video=", video_setup);
-#endif
-
 MODULE_LICENSE("GPL");
diff --git a/drivers/video/fbdev/core/modedb.c b/drivers/video/fbdev/core/modedb.c
index a9a907c440d7..388f7971494b 100644
--- a/drivers/video/fbdev/core/modedb.c
+++ b/drivers/video/fbdev/core/modedb.c
@@ -29,9 +29,6 @@
 #define DPRINTK(fmt, args...)
 #endif
 
-const char *fb_mode_option;
-EXPORT_SYMBOL_GPL(fb_mode_option);
-
 /*
  *  Standard video mode definitions (taken from XFree86)
  */
-- 
2.0.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/2] drm: Perform cmdline mode parsing during connector initialisation
  2014-08-06 12:52   ` Daniel Vetter
  (?)
@ 2014-08-06 12:52   ` Daniel Vetter
  -1 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2014-08-06 12:52 UTC (permalink / raw)
  To: DRI Development; +Cc: Daniel Vetter, Intel Graphics Development, Dave Airlie

From: Chris Wilson <chris@chris-wilson.co.uk>

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.

v2: Rebase after connector->name upheaval.

v3: Adapt mga200 to look for the cmdline mode in the new place. Nicely
simplifies things while at that.

v4: Fix checkpatch.

v5: Select FB_CMDLINE to adapt to the changed fbdev patch.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=73154
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> (v2)
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org> (v2)
Cc: dri-devel@lists.freedesktop.org
Cc: Julia Lemire <jlemire@matrox.com>
Cc: Dave Airlie <airlied@redhat.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/Kconfig                |  1 +
 drivers/gpu/drm/drm_crtc.c             | 55 +++++++++++++++++++++++++++++
 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/mgag200/mgag200_mode.c | 21 +++--------
 include/drm/drm_crtc.h                 |  1 +
 include/drm/drm_fb_helper.h            |  1 -
 8 files changed, 83 insertions(+), 78 deletions(-)

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 31894c8c1773..367f5dd23291 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -8,6 +8,7 @@ menuconfig DRM
 	tristate "Direct Rendering Manager (XFree86 4.1.0 and higher DRI support)"
 	depends on (AGP || AGP=n) && !EMULATED_CMPXCHG && MMU && HAS_DMA
 	select HDMI
+	select FB_CMDLINE
 	select I2C
 	select I2C_ALGOBIT
 	select DMA_SHARED_BUFFER
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 33ff631c8d23..66d3bfb8d264 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -863,6 +863,59 @@ 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(connector->name, &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", connector->name, s);
+		connector->force = mode->force;
+	}
+
+	DRM_DEBUG_KMS("cmdline mode for connector %s %dx%d@%dHz%s%s%s\n",
+		      connector->name,
+		      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
@@ -914,6 +967,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 3144db9dc0f1..3a6b6635e3f5 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -171,60 +171,6 @@ int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
 }
 EXPORT_SYMBOL(drm_fb_helper_remove_one_connector);
 
-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(connector->name, &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",
-					 connector->name, s);
-				connector->force = mode->force;
-			}
-
-			DRM_DEBUG_KMS("cmdline mode for connector %s %dx%d@%dHz%s%s%s\n",
-				      connector->name,
-				      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;
@@ -1013,7 +959,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) {
@@ -1260,9 +1206,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,
@@ -1272,7 +1216,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;
 
@@ -1657,8 +1601,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 bedf1894e17e..d1b7d2006529 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 db7d250f7ac7..6857e9ad6339 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)
 {
@@ -141,6 +157,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/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index 45f04dea0ac2..83485ab81ce8 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -1483,11 +1483,7 @@ static int mga_vga_mode_valid(struct drm_connector *connector,
 {
 	struct drm_device *dev = connector->dev;
 	struct mga_device *mdev = (struct mga_device*)dev->dev_private;
-	struct mga_fbdev *mfbdev = mdev->mfbdev;
-	struct drm_fb_helper *fb_helper = &mfbdev->helper;
-	struct drm_fb_helper_connector *fb_helper_conn = NULL;
 	int bpp = 32;
-	int i = 0;
 
 	if (IS_G200_SE(mdev)) {
 		if (mdev->unique_rev_id == 0x01) {
@@ -1537,21 +1533,14 @@ static int mga_vga_mode_valid(struct drm_connector *connector,
 	}
 
 	/* Validate the mode input by the user */
-	for (i = 0; i < fb_helper->connector_count; i++) {
-		if (fb_helper->connector_info[i]->connector == connector) {
-			/* Found the helper for this connector */
-			fb_helper_conn = fb_helper->connector_info[i];
-			if (fb_helper_conn->cmdline_mode.specified) {
-				if (fb_helper_conn->cmdline_mode.bpp_specified) {
-					bpp = fb_helper_conn->cmdline_mode.bpp;
-				}
-			}
-		}
+	if (connector->cmdline_mode.specified) {
+		if (connector->cmdline_mode.bpp_specified)
+			bpp = connector->cmdline_mode.bpp;
 	}
 
 	if ((mode->hdisplay * mode->vdisplay * (bpp/8)) > mdev->mc.vram_size) {
-		if (fb_helper_conn)
-			fb_helper_conn->cmdline_mode.specified = false;
+		if (connector->cmdline_mode.specified)
+			connector->cmdline_mode.specified = false;
 		return MODE_BAD;
 	}
 
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index f1105d0da059..c530b4920a09 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -548,6 +548,7 @@ struct drm_connector {
 	void *helper_private;
 
 	/* forced on connector */
+	struct drm_cmdline_mode cmdline_mode;
 	enum drm_connector_force force;
 	bool override_edid;
 	uint32_t encoder_ids[DRM_CONNECTOR_MAX_ENCODER];
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index bfd329d613c4..f4ad254e3488 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 {
-- 
2.0.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] video/fbdev: Always built-in video= cmdline parsing
  2014-08-06 12:52   ` Daniel Vetter
@ 2014-09-30 11:40     ` Tomi Valkeinen
  -1 siblings, 0 replies; 16+ messages in thread
From: Tomi Valkeinen @ 2014-09-30 11:40 UTC (permalink / raw)
  To: Daniel Vetter, DRI Development
  Cc: Plagniol-Villard, Intel Graphics Development, Geert Uytterhoeven,
	linux-fbdev

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

Hi,

On 06/08/14 15:52, Daniel Vetter wrote:
> In drm/i915 we want to get at the video= cmdline modes even when we
> don't have fbdev support enabled, so that users can always override
> the kernel's initial mode selection.
> 
> But that gives us a direct depency upon the parsing code in the fbdev
> subsystem. Since it's so little code just extract these 2 functions
> and always build them in.
> 
> Whiel at it fix the checkpatch fail in this code.
> 
> v2: Also move fb_mode_option. Spotted by the kbuild.
> 
> v3: Review from Geert:
> - Keep the old copyright notice from fb_mem.c, although I have no
> idea what exactly applies.
> - Only compile this when needed.
> 
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: linux-fbdev@vger.kernel.org
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> --
> 
> I prefer if we can merge this through drm-next since we'll use it
> there in follow-up patches.
> -Daniel
> ---
>  drivers/video/fbdev/Kconfig           |   4 ++
>  drivers/video/fbdev/core/Makefile     |   1 +
>  drivers/video/fbdev/core/fb_cmdline.c | 110 ++++++++++++++++++++++++++++++++++
>  drivers/video/fbdev/core/fbmem.c      |  92 ----------------------------
>  drivers/video/fbdev/core/modedb.c     |   3 -
>  5 files changed, 115 insertions(+), 95 deletions(-)
>  create mode 100644 drivers/video/fbdev/core/fb_cmdline.c

Sorry for late response.

Looks fine for me, and I'm fine merging it via drm-next.

Acked-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] video/fbdev: Always built-in video= cmdline parsing
@ 2014-09-30 11:40     ` Tomi Valkeinen
  0 siblings, 0 replies; 16+ messages in thread
From: Tomi Valkeinen @ 2014-09-30 11:40 UTC (permalink / raw)
  To: Daniel Vetter, DRI Development
  Cc: Plagniol-Villard, Intel Graphics Development, Geert Uytterhoeven,
	linux-fbdev


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

Hi,

On 06/08/14 15:52, Daniel Vetter wrote:
> In drm/i915 we want to get at the video= cmdline modes even when we
> don't have fbdev support enabled, so that users can always override
> the kernel's initial mode selection.
> 
> But that gives us a direct depency upon the parsing code in the fbdev
> subsystem. Since it's so little code just extract these 2 functions
> and always build them in.
> 
> Whiel at it fix the checkpatch fail in this code.
> 
> v2: Also move fb_mode_option. Spotted by the kbuild.
> 
> v3: Review from Geert:
> - Keep the old copyright notice from fb_mem.c, although I have no
> idea what exactly applies.
> - Only compile this when needed.
> 
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: linux-fbdev@vger.kernel.org
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> --
> 
> I prefer if we can merge this through drm-next since we'll use it
> there in follow-up patches.
> -Daniel
> ---
>  drivers/video/fbdev/Kconfig           |   4 ++
>  drivers/video/fbdev/core/Makefile     |   1 +
>  drivers/video/fbdev/core/fb_cmdline.c | 110 ++++++++++++++++++++++++++++++++++
>  drivers/video/fbdev/core/fbmem.c      |  92 ----------------------------
>  drivers/video/fbdev/core/modedb.c     |   3 -
>  5 files changed, 115 insertions(+), 95 deletions(-)
>  create mode 100644 drivers/video/fbdev/core/fb_cmdline.c

Sorry for late response.

Looks fine for me, and I'm fine merging it via drm-next.

Acked-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

 Tomi



[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] video/fbdev: Always built-in video= cmdline parsing
  2014-09-30 11:40     ` Tomi Valkeinen
@ 2014-09-30 11:53       ` Daniel Vetter
  -1 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2014-09-30 11:53 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Plagniol-Villard, Intel Graphics Development, Geert Uytterhoeven,
	Linux Fbdev development list, DRI Development

On Tue, Sep 30, 2014 at 1:40 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On 06/08/14 15:52, Daniel Vetter wrote:
>> In drm/i915 we want to get at the video= cmdline modes even when we
>> don't have fbdev support enabled, so that users can always override
>> the kernel's initial mode selection.
>>
>> But that gives us a direct depency upon the parsing code in the fbdev
>> subsystem. Since it's so little code just extract these 2 functions
>> and always build them in.
>>
>> Whiel at it fix the checkpatch fail in this code.
>>
>> v2: Also move fb_mode_option. Spotted by the kbuild.
>>
>> v3: Review from Geert:
>> - Keep the old copyright notice from fb_mem.c, although I have no
>> idea what exactly applies.
>> - Only compile this when needed.
>>
>> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
>> Cc: Plagniol-Villard <plagnioj@jcrosoft.com>
>> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> Cc: linux-fbdev@vger.kernel.org
>> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>
>> --
>>
>> I prefer if we can merge this through drm-next since we'll use it
>> there in follow-up patches.
>> -Daniel
>> ---
>>  drivers/video/fbdev/Kconfig           |   4 ++
>>  drivers/video/fbdev/core/Makefile     |   1 +
>>  drivers/video/fbdev/core/fb_cmdline.c | 110 ++++++++++++++++++++++++++++++++++
>>  drivers/video/fbdev/core/fbmem.c      |  92 ----------------------------
>>  drivers/video/fbdev/core/modedb.c     |   3 -
>>  5 files changed, 115 insertions(+), 95 deletions(-)
>>  create mode 100644 drivers/video/fbdev/core/fb_cmdline.c
>
> Sorry for late response.
>
> Looks fine for me, and I'm fine merging it via drm-next.
>
> Acked-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

Well totally forgotten about this one here - I even forgot to drop the
"pls ack this" text from the commit message before sending the pull
request to Dave :(

Anyway, thanks for having a look.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] video/fbdev: Always built-in video= cmdline parsing
@ 2014-09-30 11:53       ` Daniel Vetter
  0 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2014-09-30 11:53 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Plagniol-Villard, Intel Graphics Development, Geert Uytterhoeven,
	Linux Fbdev development list, DRI Development

On Tue, Sep 30, 2014 at 1:40 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On 06/08/14 15:52, Daniel Vetter wrote:
>> In drm/i915 we want to get at the video= cmdline modes even when we
>> don't have fbdev support enabled, so that users can always override
>> the kernel's initial mode selection.
>>
>> But that gives us a direct depency upon the parsing code in the fbdev
>> subsystem. Since it's so little code just extract these 2 functions
>> and always build them in.
>>
>> Whiel at it fix the checkpatch fail in this code.
>>
>> v2: Also move fb_mode_option. Spotted by the kbuild.
>>
>> v3: Review from Geert:
>> - Keep the old copyright notice from fb_mem.c, although I have no
>> idea what exactly applies.
>> - Only compile this when needed.
>>
>> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
>> Cc: Plagniol-Villard <plagnioj@jcrosoft.com>
>> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> Cc: linux-fbdev@vger.kernel.org
>> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>
>> --
>>
>> I prefer if we can merge this through drm-next since we'll use it
>> there in follow-up patches.
>> -Daniel
>> ---
>>  drivers/video/fbdev/Kconfig           |   4 ++
>>  drivers/video/fbdev/core/Makefile     |   1 +
>>  drivers/video/fbdev/core/fb_cmdline.c | 110 ++++++++++++++++++++++++++++++++++
>>  drivers/video/fbdev/core/fbmem.c      |  92 ----------------------------
>>  drivers/video/fbdev/core/modedb.c     |   3 -
>>  5 files changed, 115 insertions(+), 95 deletions(-)
>>  create mode 100644 drivers/video/fbdev/core/fb_cmdline.c
>
> Sorry for late response.
>
> Looks fine for me, and I'm fine merging it via drm-next.
>
> Acked-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

Well totally forgotten about this one here - I even forgot to drop the
"pls ack this" text from the commit message before sending the pull
request to Dave :(

Anyway, thanks for having a look.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2014-09-30 11:53 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-06  8:08 [PATCH] drm: Perform cmdline mode parsing during connector initialisation Daniel Vetter
2014-08-06  9:43 ` [PATCH] video/fbdev: Always built-in video= cmdline parsing Daniel Vetter
2014-08-06  9:43   ` Daniel Vetter
2014-08-06 10:27   ` Geert Uytterhoeven
2014-08-06 10:27     ` Geert Uytterhoeven
2014-08-06 12:48     ` Daniel Vetter
2014-08-06 12:48       ` Daniel Vetter
2014-08-06 12:05 ` Daniel Vetter
2014-08-06 12:05   ` Daniel Vetter
2014-08-06 12:52 ` [PATCH 1/2] " Daniel Vetter
2014-08-06 12:52   ` Daniel Vetter
2014-08-06 12:52   ` [PATCH 2/2] drm: Perform cmdline mode parsing during connector initialisation Daniel Vetter
2014-09-30 11:40   ` [PATCH 1/2] video/fbdev: Always built-in video= cmdline parsing Tomi Valkeinen
2014-09-30 11:40     ` Tomi Valkeinen
2014-09-30 11:53     ` Daniel Vetter
2014-09-30 11:53       ` Daniel Vetter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.