[PATCH] drm,drm/i915: Export cmdline mode parsing

[PATCH] drm,drm/i915: Export cmdline mode parsing

[Chris Wilson]

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 export the routine out of the helper and
call it first.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_crtc.c         | 58 ++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_fb_helper.c    | 47 +++---------------------------
 drivers/gpu/drm/i915/intel_fbdev.c |  2 ++
 include/drm/drm_crtc.h             |  3 +-
 4 files changed, 66 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 656788a9bf4b..853804277921 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -852,6 +852,64 @@ int drm_connector_init(struct drm_device *dev,
 EXPORT_SYMBOL(drm_connector_init);
 
 /**
+ * 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.
+ */
+void drm_connector_get_cmdline_mode(struct drm_connector *connector,
+				    struct drm_cmdline_mode *mode)
+{
+	char *option = NULL;
+
+	if (mode->specified)
+		return;
+
+	if (fb_get_options(drm_get_connector_name(connector), &option))
+		return;
+
+	if (!drm_mode_parse_command_line_for_connector(option,
+						       connector,
+						       mode))
+		return;
+
+	if (mode->force) {
+		const char *s;
+
+		switch (mode->force) {
+		case DRM_FORCE_OFF:
+			s = "OFF";
+			break;
+		case DRM_FORCE_ON_DIGITAL:
+			s = "ON - dig";
+			break;
+		default:
+		case DRM_FORCE_ON:
+			s = "ON";
+			break;
+		}
+
+		DRM_INFO("forcing %s connector %s\n",
+			 drm_get_connector_name(connector), s);
+		connector->force = mode->force;
+	}
+
+	DRM_DEBUG_KMS("cmdline mode for connector %s %dx%d@%dHz%s%s%s\n",
+		      drm_get_connector_name(connector),
+		      mode->xres, mode->yres,
+		      mode->refresh_specified ? mode->refresh : 60,
+		      mode->rb ? " reduced blanking" : "",
+		      mode->margins ? " with margins" : "",
+		      mode->interlace ?  " interlaced" : "");
+}
+EXPORT_SYMBOL(drm_connector_get_cmdline_mode);
+
+/**
  * drm_connector_cleanup - cleans up an initialised connector
  * @connector: connector to cleanup
  *
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index e95ed5805f07..f61a6c02101c 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -107,55 +107,16 @@ EXPORT_SYMBOL(drm_fb_helper_single_add_all_connectors);
 
 static int drm_fb_helper_parse_command_line(struct drm_fb_helper *fb_helper)
 {
-	struct drm_fb_helper_connector *fb_helper_conn;
 	int i;
 
 	for (i = 0; i < fb_helper->connector_count; i++) {
-		struct drm_cmdline_mode *mode;
-		struct drm_connector *connector;
-		char *option = NULL;
+		struct drm_fb_helper_connector *fb_helper_conn;
 
 		fb_helper_conn = fb_helper->connector_info[i];
-		connector = fb_helper_conn->connector;
-		mode = &fb_helper_conn->cmdline_mode;
-
-		/* do something on return - turn off connector maybe */
-		if (fb_get_options(drm_get_connector_name(connector), &option))
-			continue;
-
-		if (drm_mode_parse_command_line_for_connector(option,
-							      connector,
-							      mode)) {
-			if (mode->force) {
-				const char *s;
-				switch (mode->force) {
-				case DRM_FORCE_OFF:
-					s = "OFF";
-					break;
-				case DRM_FORCE_ON_DIGITAL:
-					s = "ON - dig";
-					break;
-				default:
-				case DRM_FORCE_ON:
-					s = "ON";
-					break;
-				}
-
-				DRM_INFO("forcing %s connector %s\n",
-					 drm_get_connector_name(connector), s);
-				connector->force = mode->force;
-			}
-
-			DRM_DEBUG_KMS("cmdline mode for connector %s %dx%d@%dHz%s%s%s\n",
-				      drm_get_connector_name(connector),
-				      mode->xres, mode->yres,
-				      mode->refresh_specified ? mode->refresh : 60,
-				      mode->rb ? " reduced blanking" : "",
-				      mode->margins ? " with margins" : "",
-				      mode->interlace ?  " interlaced" : "");
-		}
-
+		drm_connector_get_cmdline_mode(fb_helper_conn->connector,
+					       &fb_helper_conn->cmdline_mode);
 	}
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index d8f5b0f1357c..d22bfe4dc9f3 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -349,6 +349,8 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
 			continue;
 		}
 
+		drm_connector_get_cmdline_mode(connector, &fb_conn->cmdline_mode);
+
 		if (connector->force == DRM_FORCE_OFF) {
 			DRM_DEBUG_KMS("connector %s is disabled by user, skipping\n",
 				      drm_get_connector_name(connector));
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index d6026788a15d..1409669e6d2c 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -856,7 +856,8 @@ extern int drm_connector_init(struct drm_device *dev,
 			      struct drm_connector *connector,
 			      const struct drm_connector_funcs *funcs,
 			      int connector_type);
-
+extern void drm_connector_get_cmdline_mode(struct drm_connector *connector,
+					   struct drm_cmdline_mode *mode);
 extern void drm_connector_cleanup(struct drm_connector *connector);
 /* helper to unplug all connectors from sysfs for device */
 extern void drm_connector_unplug_all(struct drm_device *dev);
-- 
2.0.0.rc2

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

Re: [PATCH] drm,drm/i915: Export cmdline mode parsing

[Jani Nikula]

On Tue, 13 May 2014, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> i915.ko has a custom fbdev initialisation routine that aims to preserve
> the current mode set by the BIOS, unless overruled by the user. The
> user's wishes are determined by what, if any, mode is specified on the
> command line (via the video= parameter). However, that command line mode
> is first parsed by drm_fb_helper_initial_config() which is called after
> i915.ko's custom initial_config() as a fallback method. So in order for
> us to honour it, we need to export the routine out of the helper and
> call it first.

Is this an answer to https://bugs.freedesktop.org/show_bug.cgi?id=73154?

Jani.


>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/drm_crtc.c         | 58 ++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/drm_fb_helper.c    | 47 +++---------------------------
>  drivers/gpu/drm/i915/intel_fbdev.c |  2 ++
>  include/drm/drm_crtc.h             |  3 +-
>  4 files changed, 66 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 656788a9bf4b..853804277921 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -852,6 +852,64 @@ int drm_connector_init(struct drm_device *dev,
>  EXPORT_SYMBOL(drm_connector_init);
>  
>  /**
> + * 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.
> + */
> +void drm_connector_get_cmdline_mode(struct drm_connector *connector,
> +				    struct drm_cmdline_mode *mode)
> +{
> +	char *option = NULL;
> +
> +	if (mode->specified)
> +		return;
> +
> +	if (fb_get_options(drm_get_connector_name(connector), &option))
> +		return;
> +
> +	if (!drm_mode_parse_command_line_for_connector(option,
> +						       connector,
> +						       mode))
> +		return;
> +
> +	if (mode->force) {
> +		const char *s;
> +
> +		switch (mode->force) {
> +		case DRM_FORCE_OFF:
> +			s = "OFF";
> +			break;
> +		case DRM_FORCE_ON_DIGITAL:
> +			s = "ON - dig";
> +			break;
> +		default:
> +		case DRM_FORCE_ON:
> +			s = "ON";
> +			break;
> +		}
> +
> +		DRM_INFO("forcing %s connector %s\n",
> +			 drm_get_connector_name(connector), s);
> +		connector->force = mode->force;
> +	}
> +
> +	DRM_DEBUG_KMS("cmdline mode for connector %s %dx%d@%dHz%s%s%s\n",
> +		      drm_get_connector_name(connector),
> +		      mode->xres, mode->yres,
> +		      mode->refresh_specified ? mode->refresh : 60,
> +		      mode->rb ? " reduced blanking" : "",
> +		      mode->margins ? " with margins" : "",
> +		      mode->interlace ?  " interlaced" : "");
> +}
> +EXPORT_SYMBOL(drm_connector_get_cmdline_mode);
> +
> +/**
>   * drm_connector_cleanup - cleans up an initialised connector
>   * @connector: connector to cleanup
>   *
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index e95ed5805f07..f61a6c02101c 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -107,55 +107,16 @@ EXPORT_SYMBOL(drm_fb_helper_single_add_all_connectors);
>  
>  static int drm_fb_helper_parse_command_line(struct drm_fb_helper *fb_helper)
>  {
> -	struct drm_fb_helper_connector *fb_helper_conn;
>  	int i;
>  
>  	for (i = 0; i < fb_helper->connector_count; i++) {
> -		struct drm_cmdline_mode *mode;
> -		struct drm_connector *connector;
> -		char *option = NULL;
> +		struct drm_fb_helper_connector *fb_helper_conn;
>  
>  		fb_helper_conn = fb_helper->connector_info[i];
> -		connector = fb_helper_conn->connector;
> -		mode = &fb_helper_conn->cmdline_mode;
> -
> -		/* do something on return - turn off connector maybe */
> -		if (fb_get_options(drm_get_connector_name(connector), &option))
> -			continue;
> -
> -		if (drm_mode_parse_command_line_for_connector(option,
> -							      connector,
> -							      mode)) {
> -			if (mode->force) {
> -				const char *s;
> -				switch (mode->force) {
> -				case DRM_FORCE_OFF:
> -					s = "OFF";
> -					break;
> -				case DRM_FORCE_ON_DIGITAL:
> -					s = "ON - dig";
> -					break;
> -				default:
> -				case DRM_FORCE_ON:
> -					s = "ON";
> -					break;
> -				}
> -
> -				DRM_INFO("forcing %s connector %s\n",
> -					 drm_get_connector_name(connector), s);
> -				connector->force = mode->force;
> -			}
> -
> -			DRM_DEBUG_KMS("cmdline mode for connector %s %dx%d@%dHz%s%s%s\n",
> -				      drm_get_connector_name(connector),
> -				      mode->xres, mode->yres,
> -				      mode->refresh_specified ? mode->refresh : 60,
> -				      mode->rb ? " reduced blanking" : "",
> -				      mode->margins ? " with margins" : "",
> -				      mode->interlace ?  " interlaced" : "");
> -		}
> -
> +		drm_connector_get_cmdline_mode(fb_helper_conn->connector,
> +					       &fb_helper_conn->cmdline_mode);
>  	}
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> index d8f5b0f1357c..d22bfe4dc9f3 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -349,6 +349,8 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
>  			continue;
>  		}
>  
> +		drm_connector_get_cmdline_mode(connector, &fb_conn->cmdline_mode);
> +
>  		if (connector->force == DRM_FORCE_OFF) {
>  			DRM_DEBUG_KMS("connector %s is disabled by user, skipping\n",
>  				      drm_get_connector_name(connector));
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index d6026788a15d..1409669e6d2c 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -856,7 +856,8 @@ extern int drm_connector_init(struct drm_device *dev,
>  			      struct drm_connector *connector,
>  			      const struct drm_connector_funcs *funcs,
>  			      int connector_type);
> -
> +extern void drm_connector_get_cmdline_mode(struct drm_connector *connector,
> +					   struct drm_cmdline_mode *mode);
>  extern void drm_connector_cleanup(struct drm_connector *connector);
>  /* helper to unplug all connectors from sysfs for device */
>  extern void drm_connector_unplug_all(struct drm_device *dev);
> -- 
> 2.0.0.rc2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm,drm/i915: Export cmdline mode parsing

[Chris Wilson]

On Wed, May 14, 2014 at 11:31:47AM +0300, Jani Nikula wrote:
> On Tue, 13 May 2014, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > i915.ko has a custom fbdev initialisation routine that aims to preserve
> > the current mode set by the BIOS, unless overruled by the user. The
> > user's wishes are determined by what, if any, mode is specified on the
> > command line (via the video= parameter). However, that command line mode
> > is first parsed by drm_fb_helper_initial_config() which is called after
> > i915.ko's custom initial_config() as a fallback method. So in order for
> > us to honour it, we need to export the routine out of the helper and
> > call it first.
> 
> Is this an answer to https://bugs.freedesktop.org/show_bug.cgi?id=73154?

Yes. Not in this patch, but we can also use in that case as well.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm,drm/i915: Export cmdline mode parsing

[Rodrigo Vivi]

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

This patch got a conflict on latest -collector. Is it still needed?
If yes, could you please rebase?

I was also going to tell about the "Make the pysical object coherent..."
but I noticed you already rebased.

Thanks,
Rodrigo.



On Wed, May 14, 2014 at 1:46 AM, Chris Wilson <chris@chris-wilson.co.uk>
wrote:

> On Wed, May 14, 2014 at 11:31:47AM +0300, Jani Nikula wrote:
> > On Tue, 13 May 2014, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > > i915.ko has a custom fbdev initialisation routine that aims to preserve
> > > the current mode set by the BIOS, unless overruled by the user. The
> > > user's wishes are determined by what, if any, mode is specified on the
> > > command line (via the video= parameter). However, that command line
> mode
> > > is first parsed by drm_fb_helper_initial_config() which is called after
> > > i915.ko's custom initial_config() as a fallback method. So in order for
> > > us to honour it, we need to export the routine out of the helper and
> > > call it first.
> >
> > Is this an answer to https://bugs.freedesktop.org/show_bug.cgi?id=73154?
>
> Yes. Not in this patch, but we can also use in that case as well.
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br

[-- Attachment #1.2: Type: text/html, Size: 2491 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