diff mbox series

[v4,10/12] drm/modes: Parse overscan properties

Message ID c16d281d1dc69ee60e95169807e7e2ab2992f133.1560514379.git-series.maxime.ripard@bootlin.com (mailing list archive)
State New, archived
Headers show
Series drm/vc4: Allow for more boot-time configuration | expand

Commit Message

Maxime Ripard June 14, 2019, 12:13 p.m. UTC
Properly configuring the overscan properties might be needed for the
initial setup of the framebuffer for display that still have overscan.
Let's allow for more properties on the kernel command line to setup each
margin.

Reviewed-by: Noralf Trønnes <noralf@tronnes.org>
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 drivers/gpu/drm/drm_modes.c | 44 ++++++++++++++++++++++++++++++++++++++-
 include/drm/drm_connector.h | 12 +++++-----
 2 files changed, 50 insertions(+), 6 deletions(-)

Comments

Noralf Trønnes June 15, 2019, 3:40 p.m. UTC | #1
Den 14.06.2019 14.13, skrev Maxime Ripard:
> Properly configuring the overscan properties might be needed for the
> initial setup of the framebuffer for display that still have overscan.
> Let's allow for more properties on the kernel command line to setup each
> margin.

This also needs to be documented in Documentation/fb/modedb.txt

> 
> Reviewed-by: Noralf Trønnes <noralf@tronnes.org>
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
>  drivers/gpu/drm/drm_modes.c | 44 ++++++++++++++++++++++++++++++++++++++-
>  include/drm/drm_connector.h | 12 +++++-----
>  2 files changed, 50 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index b92b7df6784a..25d2ba595750 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -1609,6 +1609,50 @@ static int drm_mode_parse_cmdline_options(char *str, size_t len,
>  		} else if (!strncmp(option, "reflect_y", delim - option)) {
>  			rotation |= DRM_MODE_REFLECT_Y;
>  			sep = delim;
> +		} else if (!strncmp(option, "margin_right", delim - option)) {

I wonder if this should be called tv_margin_right to distinguish it from
'm' (the mode specifier) which also is a margin? Or can these margins be
reused on other display types later on? A little stupid to have both
'tv_margin_right' and 'margin_right'.

> +			const char *value = delim + 1;
> +			unsigned int margin;
> +
> +			margin = simple_strtol(value, &sep, 10);
> +
> +			/* Make sure we have parsed something */
> +			if (sep == value)
> +				return -EINVAL;
> +
> +			mode->tv_margins.right = margin;
> +		} else if (!strncmp(option, "margin_left", delim - option)) {
> +			const char *value = delim + 1;
> +			unsigned int margin;
> +
> +			margin = simple_strtol(value, &sep, 10);
> +
> +			/* Make sure we have parsed something */
> +			if (sep == value)
> +				return -EINVAL;
> +
> +			mode->tv_margins.left = margin;
> +		} else if (!strncmp(option, "margin_top", delim - option)) {
> +			const char *value = delim + 1;
> +			unsigned int margin;
> +
> +			margin = simple_strtol(value, &sep, 10);
> +
> +			/* Make sure we have parsed something */
> +			if (sep == value)
> +				return -EINVAL;
> +
> +			mode->tv_margins.top = margin;
> +		} else if (!strncmp(option, "margin_bottom", delim - option)) {
> +			const char *value = delim + 1;
> +			unsigned int margin;
> +
> +			margin = simple_strtol(value, &sep, 10);
> +
> +			/* Make sure we have parsed something */
> +			if (sep == value)
> +				return -EINVAL;
> +
> +			mode->tv_margins.bottom = margin;
>  		} else {
>  			return -EINVAL;
>  		}
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index c58a35b34c1a..6841c46e6781 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -505,12 +505,7 @@ struct drm_connector_tv_margins {
>   */
>  struct drm_tv_connector_state {
>  	enum drm_mode_subconnector subconnector;
> -	struct {
> -		unsigned int left;
> -		unsigned int right;
> -		unsigned int top;
> -		unsigned int bottom;
> -	} margins;
> +	struct drm_connector_tv_margins margins;
>  	unsigned int mode;
>  	unsigned int brightness;
>  	unsigned int contrast;

As mentioned this needs moving to patch 8.

Noralf.

> @@ -1039,6 +1034,11 @@ struct drm_cmdline_mode {
>  	 * DRM_MODE_ROTATE_180 are supported at the moment.
>  	 */
>  	unsigned int rotation;
> +
> +	/**
> +	 * @tv_margins: TV margins to apply to the mode.
> +	 */
> +	struct drm_connector_tv_margins tv_margins;
>  };
>  
>  /**
>
Noralf Trønnes June 16, 2019, 9:54 a.m. UTC | #2
Den 15.06.2019 17.40, skrev Noralf Trønnes:
> 
> 
> Den 14.06.2019 14.13, skrev Maxime Ripard:
>> Properly configuring the overscan properties might be needed for the
>> initial setup of the framebuffer for display that still have overscan.
>> Let's allow for more properties on the kernel command line to setup each
>> margin.
> 
> This also needs to be documented in Documentation/fb/modedb.txt
> 
>>
>> Reviewed-by: Noralf Trønnes <noralf@tronnes.org>
>> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
>> ---
>>  drivers/gpu/drm/drm_modes.c | 44 ++++++++++++++++++++++++++++++++++++++-
>>  include/drm/drm_connector.h | 12 +++++-----
>>  2 files changed, 50 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
>> index b92b7df6784a..25d2ba595750 100644
>> --- a/drivers/gpu/drm/drm_modes.c
>> +++ b/drivers/gpu/drm/drm_modes.c
>> @@ -1609,6 +1609,50 @@ static int drm_mode_parse_cmdline_options(char *str, size_t len,
>>  		} else if (!strncmp(option, "reflect_y", delim - option)) {
>>  			rotation |= DRM_MODE_REFLECT_Y;
>>  			sep = delim;
>> +		} else if (!strncmp(option, "margin_right", delim - option)) {
> 
> I wonder if this should be called tv_margin_right to distinguish it from
> 'm' (the mode specifier) which also is a margin? Or can these margins be
> reused on other display types later on? A little stupid to have both
> 'tv_margin_right' and 'margin_right'.

And after a nights sleep I think 'margin_right' is just fine as long as
it's documented in modedb.txt as TV margins.

So with an modedb.txt entry and the struct change moved to patch 8:

Reviewed-by: Noralf Trønnes <noralf@tronnes.org>

> 
>> +			const char *value = delim + 1;
>> +			unsigned int margin;
>> +
>> +			margin = simple_strtol(value, &sep, 10);
>> +
>> +			/* Make sure we have parsed something */
>> +			if (sep == value)
>> +				return -EINVAL;
>> +
>> +			mode->tv_margins.right = margin;
>> +		} else if (!strncmp(option, "margin_left", delim - option)) {
>> +			const char *value = delim + 1;
>> +			unsigned int margin;
>> +
>> +			margin = simple_strtol(value, &sep, 10);
>> +
>> +			/* Make sure we have parsed something */
>> +			if (sep == value)
>> +				return -EINVAL;
>> +
>> +			mode->tv_margins.left = margin;
>> +		} else if (!strncmp(option, "margin_top", delim - option)) {
>> +			const char *value = delim + 1;
>> +			unsigned int margin;
>> +
>> +			margin = simple_strtol(value, &sep, 10);
>> +
>> +			/* Make sure we have parsed something */
>> +			if (sep == value)
>> +				return -EINVAL;
>> +
>> +			mode->tv_margins.top = margin;
>> +		} else if (!strncmp(option, "margin_bottom", delim - option)) {
>> +			const char *value = delim + 1;
>> +			unsigned int margin;
>> +
>> +			margin = simple_strtol(value, &sep, 10);
>> +
>> +			/* Make sure we have parsed something */
>> +			if (sep == value)
>> +				return -EINVAL;
>> +
>> +			mode->tv_margins.bottom = margin;
>>  		} else {
>>  			return -EINVAL;
>>  		}
>> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
>> index c58a35b34c1a..6841c46e6781 100644
>> --- a/include/drm/drm_connector.h
>> +++ b/include/drm/drm_connector.h
>> @@ -505,12 +505,7 @@ struct drm_connector_tv_margins {
>>   */
>>  struct drm_tv_connector_state {
>>  	enum drm_mode_subconnector subconnector;
>> -	struct {
>> -		unsigned int left;
>> -		unsigned int right;
>> -		unsigned int top;
>> -		unsigned int bottom;
>> -	} margins;
>> +	struct drm_connector_tv_margins margins;
>>  	unsigned int mode;
>>  	unsigned int brightness;
>>  	unsigned int contrast;
> 
> As mentioned this needs moving to patch 8.
> 
> Noralf.
> 
>> @@ -1039,6 +1034,11 @@ struct drm_cmdline_mode {
>>  	 * DRM_MODE_ROTATE_180 are supported at the moment.
>>  	 */
>>  	unsigned int rotation;
>> +
>> +	/**
>> +	 * @tv_margins: TV margins to apply to the mode.
>> +	 */
>> +	struct drm_connector_tv_margins tv_margins;
>>  };
>>  
>>  /**
>>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index b92b7df6784a..25d2ba595750 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1609,6 +1609,50 @@  static int drm_mode_parse_cmdline_options(char *str, size_t len,
 		} else if (!strncmp(option, "reflect_y", delim - option)) {
 			rotation |= DRM_MODE_REFLECT_Y;
 			sep = delim;
+		} else if (!strncmp(option, "margin_right", delim - option)) {
+			const char *value = delim + 1;
+			unsigned int margin;
+
+			margin = simple_strtol(value, &sep, 10);
+
+			/* Make sure we have parsed something */
+			if (sep == value)
+				return -EINVAL;
+
+			mode->tv_margins.right = margin;
+		} else if (!strncmp(option, "margin_left", delim - option)) {
+			const char *value = delim + 1;
+			unsigned int margin;
+
+			margin = simple_strtol(value, &sep, 10);
+
+			/* Make sure we have parsed something */
+			if (sep == value)
+				return -EINVAL;
+
+			mode->tv_margins.left = margin;
+		} else if (!strncmp(option, "margin_top", delim - option)) {
+			const char *value = delim + 1;
+			unsigned int margin;
+
+			margin = simple_strtol(value, &sep, 10);
+
+			/* Make sure we have parsed something */
+			if (sep == value)
+				return -EINVAL;
+
+			mode->tv_margins.top = margin;
+		} else if (!strncmp(option, "margin_bottom", delim - option)) {
+			const char *value = delim + 1;
+			unsigned int margin;
+
+			margin = simple_strtol(value, &sep, 10);
+
+			/* Make sure we have parsed something */
+			if (sep == value)
+				return -EINVAL;
+
+			mode->tv_margins.bottom = margin;
 		} else {
 			return -EINVAL;
 		}
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index c58a35b34c1a..6841c46e6781 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -505,12 +505,7 @@  struct drm_connector_tv_margins {
  */
 struct drm_tv_connector_state {
 	enum drm_mode_subconnector subconnector;
-	struct {
-		unsigned int left;
-		unsigned int right;
-		unsigned int top;
-		unsigned int bottom;
-	} margins;
+	struct drm_connector_tv_margins margins;
 	unsigned int mode;
 	unsigned int brightness;
 	unsigned int contrast;
@@ -1039,6 +1034,11 @@  struct drm_cmdline_mode {
 	 * DRM_MODE_ROTATE_180 are supported at the moment.
 	 */
 	unsigned int rotation;
+
+	/**
+	 * @tv_margins: TV margins to apply to the mode.
+	 */
+	struct drm_connector_tv_margins tv_margins;
 };
 
 /**