[PATCH 1/2] drm/connector: Document the optional margin props in the HDMI section

[PATCH 1/2] drm/connector: Document the optional margin props in the HDMI section

[Boris Brezillon]

HDMI connectors can optionally be attached margin props. Let's
document them.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 drivers/gpu/drm/drm_connector.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 2355124849db..95dfb322b14e 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1116,6 +1116,23 @@ EXPORT_SYMBOL(drm_mode_create_dvi_i_properties);
  *	Drivers can set up this property by calling
  *	drm_connector_attach_content_type_property(). Decoding to
  *	infoframe values is done through drm_hdmi_avi_infoframe_content_type().
+ *
+ * TV margins (optional):
+ *	Defines invisible area of a screen so that the CRTC driver can possibly
+ *	scale the output image and move it to make it entirely visible. All
+ *	margins are defined in pixels and the valid range is 0 - 100.
+ *
+ *	left margin:
+ *		Left margin
+ *
+ *	right margin:
+ *		Left margin
+ *
+ *	top margin:
+ *		Top margin
+ *
+ *	bottom margin:
+ *		Bottom margin
  */
 
 /**
-- 
2.20.1

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

[PATCH 2/2] drm/connector: Document the TV props

[Boris Brezillon]

Document TV connector props and get rid of the according entries in
the csv file.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 Documentation/gpu/drm-kms.rst        |  6 +++
 Documentation/gpu/kms-properties.csv | 13 -------
 drivers/gpu/drm/drm_connector.c      | 56 ++++++++++++++++++++++++++++
 3 files changed, 62 insertions(+), 13 deletions(-)

diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index 23a3c986ef6d..376f88e56d14 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -449,6 +449,12 @@ HDMI Specific Connector Properties
 .. kernel-doc:: drivers/gpu/drm/drm_connector.c
    :doc: HDMI connector properties
 
+TV Connector Properties
+-----------------------
+
+.. kernel-doc:: drivers/gpu/drm/drm_connector.c
+   :doc: TV connector properties
+
 Plane Composition Properties
 ----------------------------
 
diff --git a/Documentation/gpu/kms-properties.csv b/Documentation/gpu/kms-properties.csv
index 07ed22ea3bd6..1a8277f3028d 100644
--- a/Documentation/gpu/kms-properties.csv
+++ b/Documentation/gpu/kms-properties.csv
@@ -1,19 +1,6 @@
 Owner Module/Drivers,Group,Property Name,Type,Property Values,Object attached,Description/Restrictions
 ,DVI-I,“subconnector”,ENUM,"{ “Unknown”, “DVI-D”, “DVI-A” }",Connector,TBD
 ,,“select subconnector”,ENUM,"{ “Automatic”, “DVI-D”, “DVI-A” }",Connector,TBD
-,TV,“subconnector”,ENUM,"{ ""Unknown"", ""Composite"", ""SVIDEO"", ""Component"", ""SCART"" }",Connector,TBD
-,,“select subconnector”,ENUM,"{ ""Automatic"", ""Composite"", ""SVIDEO"", ""Component"", ""SCART"" }",Connector,TBD
-,,“mode”,ENUM,"{ ""NTSC_M"", ""NTSC_J"", ""NTSC_443"", ""PAL_B"" } etc.",Connector,TBD
-,,“left margin”,RANGE,"Min=0, Max=100",Connector,TBD
-,,“right margin”,RANGE,"Min=0, Max=100",Connector,TBD
-,,“top margin”,RANGE,"Min=0, Max=100",Connector,TBD
-,,“bottom margin”,RANGE,"Min=0, Max=100",Connector,TBD
-,,“brightness”,RANGE,"Min=0, Max=100",Connector,TBD
-,,“contrast”,RANGE,"Min=0, Max=100",Connector,TBD
-,,“flicker reduction”,RANGE,"Min=0, Max=100",Connector,TBD
-,,“overscan”,RANGE,"Min=0, Max=100",Connector,TBD
-,,“saturation”,RANGE,"Min=0, Max=100",Connector,TBD
-,,“hue”,RANGE,"Min=0, Max=100",Connector,TBD
 ,Virtual GPU,“suggested X”,RANGE,"Min=0, Max=0xffffffff",Connector,property to suggest an X offset for a connector
 ,,“suggested Y”,RANGE,"Min=0, Max=0xffffffff",Connector,property to suggest an Y offset for a connector
 ,Optional,"""aspect ratio""",ENUM,"{ ""None"", ""4:3"", ""16:9"" }",Connector,TDB
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 95dfb322b14e..bb581994b48e 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1249,6 +1249,62 @@ int drm_mode_create_tv_margin_properties(struct drm_device *dev)
 }
 EXPORT_SYMBOL(drm_mode_create_tv_margin_properties);
 
+/**
+ * DOC: TV connector properties
+ *
+ * subconnector:
+ *	Subconnector can be Composite, SVIDEO, Component or SCART. For legacy
+ *	reasons we have two properties, one to select the subconnector and one
+ *	showing the currently selected subconnector.
+ *
+ *	select subconnector:
+ *		Select a subconnector type. There's an 'Automatic' value to let
+ *		DRM select the subconnector automatically
+ *
+ * 	subconnector:
+ * 		Shows the currently selected subconnector
+ *
+ * margins:
+ *	Defines invisible area of a screen so that the CRTC driver can possibly
+ *	scale the output image and move it to make it entirely visible. All
+ *	margins are defined in pixels and the valid range is 0 - 100.
+ *
+ *	left margin:
+ *		Left margin
+ *
+ *	right margin:
+ *		Left margin
+ *
+ *	top margin:
+ *		Top margin
+ *
+ *	bottom margin:
+ *		Bottom margin
+ *
+ * mode:
+ *	Exposes all supported TV modes and allows one to select a mode. The
+ *	list of supported modes is driver dependent, but you should basically
+ *	find one or more variants of the PAL and NTSC standards.
+ *
+ * brightness:
+ *	Brigthness settings expressed in percent
+ *
+ * contrast:
+ *	Contrast settings expressed in percent
+ *
+ * flicker reduction:
+ *	Flicker reduction settings expressed in percent
+ *
+ * overscan:
+ *	Overscan settings
+ *
+ * saturation:
+ *	Saturation settings expressed in percent
+ *
+ * hue:
+ *	Hue settings expressed in percent
+ */
+
 /**
  * drm_mode_create_tv_properties - create TV specific connector properties
  * @dev: DRM device
-- 
2.20.1

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

Re: [PATCH 1/2] drm/connector: Document the optional margin props in the HDMI section

[Daniel Vetter]

On Wed, Mar 27, 2019 at 10:59:15AM +0100, Boris Brezillon wrote:
> HDMI connectors can optionally be attached margin props. Let's
> document them.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
>  drivers/gpu/drm/drm_connector.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 2355124849db..95dfb322b14e 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1116,6 +1116,23 @@ EXPORT_SYMBOL(drm_mode_create_dvi_i_properties);
>   *	Drivers can set up this property by calling
>   *	drm_connector_attach_content_type_property(). Decoding to
>   *	infoframe values is done through drm_hdmi_avi_infoframe_content_type().
> + *
> + * TV margins (optional):
> + *	Defines invisible area of a screen so that the CRTC driver can possibly
> + *	scale the output image and move it to make it entirely visible. All
> + *	margins are defined in pixels and the valid range is 0 - 100.

Maybe add the usual hints for driver writers too:

"Drivers can set this up by calling
drm_connector_attach_tv_margin_properties(), the values are stored in
&drm_connector_state.tv."

With that: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

I'm also not entirely sure it's in pixels, but *shrug*
-Daniel


> + *
> + *	left margin:
> + *		Left margin
> + *
> + *	right margin:
> + *		Left margin
> + *
> + *	top margin:
> + *		Top margin
> + *
> + *	bottom margin:
> + *		Bottom margin
>   */
>  
>  /**
> -- 
> 2.20.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm/connector: Document the TV props

[Daniel Vetter]

On Wed, Mar 27, 2019 at 10:59:16AM +0100, Boris Brezillon wrote:
> Document TV connector props and get rid of the according entries in
> the csv file.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
>  Documentation/gpu/drm-kms.rst        |  6 +++
>  Documentation/gpu/kms-properties.csv | 13 -------
>  drivers/gpu/drm/drm_connector.c      | 56 ++++++++++++++++++++++++++++
>  3 files changed, 62 insertions(+), 13 deletions(-)
> 
> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> index 23a3c986ef6d..376f88e56d14 100644
> --- a/Documentation/gpu/drm-kms.rst
> +++ b/Documentation/gpu/drm-kms.rst
> @@ -449,6 +449,12 @@ HDMI Specific Connector Properties
>  .. kernel-doc:: drivers/gpu/drm/drm_connector.c
>     :doc: HDMI connector properties
>  
> +TV Connector Properties
> +-----------------------
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_connector.c
> +   :doc: TV connector properties
> +
>  Plane Composition Properties
>  ----------------------------
>  
> diff --git a/Documentation/gpu/kms-properties.csv b/Documentation/gpu/kms-properties.csv
> index 07ed22ea3bd6..1a8277f3028d 100644
> --- a/Documentation/gpu/kms-properties.csv
> +++ b/Documentation/gpu/kms-properties.csv
> @@ -1,19 +1,6 @@
>  Owner Module/Drivers,Group,Property Name,Type,Property Values,Object attached,Description/Restrictions
>  ,DVI-I,“subconnector”,ENUM,"{ “Unknown”, “DVI-D”, “DVI-A” }",Connector,TBD
>  ,,“select subconnector”,ENUM,"{ “Automatic”, “DVI-D”, “DVI-A” }",Connector,TBD
> -,TV,“subconnector”,ENUM,"{ ""Unknown"", ""Composite"", ""SVIDEO"", ""Component"", ""SCART"" }",Connector,TBD
> -,,“select subconnector”,ENUM,"{ ""Automatic"", ""Composite"", ""SVIDEO"", ""Component"", ""SCART"" }",Connector,TBD
> -,,“mode”,ENUM,"{ ""NTSC_M"", ""NTSC_J"", ""NTSC_443"", ""PAL_B"" } etc.",Connector,TBD
> -,,“left margin”,RANGE,"Min=0, Max=100",Connector,TBD
> -,,“right margin”,RANGE,"Min=0, Max=100",Connector,TBD
> -,,“top margin”,RANGE,"Min=0, Max=100",Connector,TBD
> -,,“bottom margin”,RANGE,"Min=0, Max=100",Connector,TBD
> -,,“brightness”,RANGE,"Min=0, Max=100",Connector,TBD
> -,,“contrast”,RANGE,"Min=0, Max=100",Connector,TBD
> -,,“flicker reduction”,RANGE,"Min=0, Max=100",Connector,TBD
> -,,“overscan”,RANGE,"Min=0, Max=100",Connector,TBD
> -,,“saturation”,RANGE,"Min=0, Max=100",Connector,TBD
> -,,“hue”,RANGE,"Min=0, Max=100",Connector,TBD
>  ,Virtual GPU,“suggested X”,RANGE,"Min=0, Max=0xffffffff",Connector,property to suggest an X offset for a connector
>  ,,“suggested Y”,RANGE,"Min=0, Max=0xffffffff",Connector,property to suggest an Y offset for a connector
>  ,Optional,"""aspect ratio""",ENUM,"{ ""None"", ""4:3"", ""16:9"" }",Connector,TDB
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 95dfb322b14e..bb581994b48e 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1249,6 +1249,62 @@ int drm_mode_create_tv_margin_properties(struct drm_device *dev)
>  }
>  EXPORT_SYMBOL(drm_mode_create_tv_margin_properties);
>  
> +/**
> + * DOC: TV connector properties
> + *
> + * subconnector:
> + *	Subconnector can be Composite, SVIDEO, Component or SCART. For legacy
> + *	reasons we have two properties, one to select the subconnector and one
> + *	showing the currently selected subconnector.
> + *
> + *	select subconnector:
> + *		Select a subconnector type. There's an 'Automatic' value to let
> + *		DRM select the subconnector automatically
> + *
> + * 	subconnector:
> + * 		Shows the currently selected subconnector

There's also subconnector values for DVI-D vs. DVI-I on some drivers. Not
sure we want to include these here though ...

> + *
> + * margins:
> + *	Defines invisible area of a screen so that the CRTC driver can possibly
> + *	scale the output image and move it to make it entirely visible. All
> + *	margins are defined in pixels and the valid range is 0 - 100.
> + *
> + *	left margin:
> + *		Left margin
> + *
> + *	right margin:
> + *		Left margin
> + *
> + *	top margin:
> + *		Top margin
> + *
> + *	bottom margin:
> + *		Bottom margin

This feels redundant with the margins text in patch 1. I think better to
remove.

> + *
> + * mode:
> + *	Exposes all supported TV modes and allows one to select a mode. The
> + *	list of supported modes is driver dependent, but you should basically
> + *	find one or more variants of the PAL and NTSC standards.
> + *
> + * brightness:
> + *	Brigthness settings expressed in percent
> + *
> + * contrast:
> + *	Contrast settings expressed in percent
> + *
> + * flicker reduction:
> + *	Flicker reduction settings expressed in percent
> + *
> + * overscan:
> + *	Overscan settings
> + *
> + * saturation:
> + *	Saturation settings expressed in percent
> + *
> + * hue:
> + *	Hue settings expressed in percent
> + */

Not sure we want to sprinkle the usual set of references to create/attach
functions. Probably overkill, up to you.

With the redundant margins section removed:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> +
>  /**
>   * drm_mode_create_tv_properties - create TV specific connector properties
>   * @dev: DRM device
> -- 
> 2.20.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm/connector: Document the TV props

[Boris Brezillon]

On Wed, 27 Mar 2019 12:26:07 +0100
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Wed, Mar 27, 2019 at 10:59:16AM +0100, Boris Brezillon wrote:
> > Document TV connector props and get rid of the according entries in
> > the csv file.
> > 
> > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> > ---
> >  Documentation/gpu/drm-kms.rst        |  6 +++
> >  Documentation/gpu/kms-properties.csv | 13 -------
> >  drivers/gpu/drm/drm_connector.c      | 56 ++++++++++++++++++++++++++++
> >  3 files changed, 62 insertions(+), 13 deletions(-)
> > 
> > diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> > index 23a3c986ef6d..376f88e56d14 100644
> > --- a/Documentation/gpu/drm-kms.rst
> > +++ b/Documentation/gpu/drm-kms.rst
> > @@ -449,6 +449,12 @@ HDMI Specific Connector Properties
> >  .. kernel-doc:: drivers/gpu/drm/drm_connector.c
> >     :doc: HDMI connector properties
> >  
> > +TV Connector Properties
> > +-----------------------
> > +
> > +.. kernel-doc:: drivers/gpu/drm/drm_connector.c
> > +   :doc: TV connector properties
> > +
> >  Plane Composition Properties
> >  ----------------------------
> >  
> > diff --git a/Documentation/gpu/kms-properties.csv b/Documentation/gpu/kms-properties.csv
> > index 07ed22ea3bd6..1a8277f3028d 100644
> > --- a/Documentation/gpu/kms-properties.csv
> > +++ b/Documentation/gpu/kms-properties.csv
> > @@ -1,19 +1,6 @@
> >  Owner Module/Drivers,Group,Property Name,Type,Property Values,Object attached,Description/Restrictions
> >  ,DVI-I,“subconnector”,ENUM,"{ “Unknown”, “DVI-D”, “DVI-A” }",Connector,TBD
> >  ,,“select subconnector”,ENUM,"{ “Automatic”, “DVI-D”, “DVI-A” }",Connector,TBD
> > -,TV,“subconnector”,ENUM,"{ ""Unknown"", ""Composite"", ""SVIDEO"", ""Component"", ""SCART"" }",Connector,TBD
> > -,,“select subconnector”,ENUM,"{ ""Automatic"", ""Composite"", ""SVIDEO"", ""Component"", ""SCART"" }",Connector,TBD
> > -,,“mode”,ENUM,"{ ""NTSC_M"", ""NTSC_J"", ""NTSC_443"", ""PAL_B"" } etc.",Connector,TBD
> > -,,“left margin”,RANGE,"Min=0, Max=100",Connector,TBD
> > -,,“right margin”,RANGE,"Min=0, Max=100",Connector,TBD
> > -,,“top margin”,RANGE,"Min=0, Max=100",Connector,TBD
> > -,,“bottom margin”,RANGE,"Min=0, Max=100",Connector,TBD
> > -,,“brightness”,RANGE,"Min=0, Max=100",Connector,TBD
> > -,,“contrast”,RANGE,"Min=0, Max=100",Connector,TBD
> > -,,“flicker reduction”,RANGE,"Min=0, Max=100",Connector,TBD
> > -,,“overscan”,RANGE,"Min=0, Max=100",Connector,TBD
> > -,,“saturation”,RANGE,"Min=0, Max=100",Connector,TBD
> > -,,“hue”,RANGE,"Min=0, Max=100",Connector,TBD
> >  ,Virtual GPU,“suggested X”,RANGE,"Min=0, Max=0xffffffff",Connector,property to suggest an X offset for a connector
> >  ,,“suggested Y”,RANGE,"Min=0, Max=0xffffffff",Connector,property to suggest an Y offset for a connector
> >  ,Optional,"""aspect ratio""",ENUM,"{ ""None"", ""4:3"", ""16:9"" }",Connector,TDB
> > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > index 95dfb322b14e..bb581994b48e 100644
> > --- a/drivers/gpu/drm/drm_connector.c
> > +++ b/drivers/gpu/drm/drm_connector.c
> > @@ -1249,6 +1249,62 @@ int drm_mode_create_tv_margin_properties(struct drm_device *dev)
> >  }
> >  EXPORT_SYMBOL(drm_mode_create_tv_margin_properties);
> >  
> > +/**
> > + * DOC: TV connector properties
> > + *
> > + * subconnector:
> > + *	Subconnector can be Composite, SVIDEO, Component or SCART. For legacy
> > + *	reasons we have two properties, one to select the subconnector and one
> > + *	showing the currently selected subconnector.
> > + *
> > + *	select subconnector:
> > + *		Select a subconnector type. There's an 'Automatic' value to let
> > + *		DRM select the subconnector automatically
> > + *
> > + * 	subconnector:
> > + * 		Shows the currently selected subconnector  
> 
> There's also subconnector values for DVI-D vs. DVI-I on some drivers. Not
> sure we want to include these here though ...

I'm documenting the TV connector props here. Maybe I should move that
to the "standard connector properties"...

> 
> > + *
> > + * margins:
> > + *	Defines invisible area of a screen so that the CRTC driver can possibly
> > + *	scale the output image and move it to make it entirely visible. All
> > + *	margins are defined in pixels and the valid range is 0 - 100.
> > + *
> > + *	left margin:
> > + *		Left margin
> > + *
> > + *	right margin:
> > + *		Left margin
> > + *
> > + *	top margin:
> > + *		Top margin
> > + *
> > + *	bottom margin:
> > + *		Bottom margin  
> 
> This feels redundant with the margins text in patch 1. I think better to
> remove.

Problem is, the margin props were documented in the HDMI connector
props section. If we want to make it applicable to both HDMI and TV
connectors I should move it to the "standard connector properties"
section and mention that these props only apply to HDMI and TV
connectors.

> 
> > + *
> > + * mode:
> > + *	Exposes all supported TV modes and allows one to select a mode. The
> > + *	list of supported modes is driver dependent, but you should basically
> > + *	find one or more variants of the PAL and NTSC standards.
> > + *
> > + * brightness:
> > + *	Brigthness settings expressed in percent
> > + *
> > + * contrast:
> > + *	Contrast settings expressed in percent
> > + *
> > + * flicker reduction:
> > + *	Flicker reduction settings expressed in percent
> > + *
> > + * overscan:
> > + *	Overscan settings
> > + *
> > + * saturation:
> > + *	Saturation settings expressed in percent
> > + *
> > + * hue:
> > + *	Hue settings expressed in percent
> > + */  
> 
> Not sure we want to sprinkle the usual set of references to create/attach
> functions. Probably overkill, up to you.
> 
> With the redundant margins section removed:
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> > +
> >  /**
> >   * drm_mode_create_tv_properties - create TV specific connector properties
> >   * @dev: DRM device
> > -- 
> > 2.20.1
> >   
> 

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

Re: [PATCH 1/2] drm/connector: Document the optional margin props in the HDMI section

[Boris Brezillon]

On Wed, 27 Mar 2019 12:19:30 +0100
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Wed, Mar 27, 2019 at 10:59:15AM +0100, Boris Brezillon wrote:
> > HDMI connectors can optionally be attached margin props. Let's
> > document them.
> > 
> > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> > ---
> >  drivers/gpu/drm/drm_connector.c | 17 +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > index 2355124849db..95dfb322b14e 100644
> > --- a/drivers/gpu/drm/drm_connector.c
> > +++ b/drivers/gpu/drm/drm_connector.c
> > @@ -1116,6 +1116,23 @@ EXPORT_SYMBOL(drm_mode_create_dvi_i_properties);
> >   *	Drivers can set up this property by calling
> >   *	drm_connector_attach_content_type_property(). Decoding to
> >   *	infoframe values is done through drm_hdmi_avi_infoframe_content_type().
> > + *
> > + * TV margins (optional):
> > + *	Defines invisible area of a screen so that the CRTC driver can possibly
> > + *	scale the output image and move it to make it entirely visible. All
> > + *	margins are defined in pixels and the valid range is 0 - 100.  
> 
> Maybe add the usual hints for driver writers too:
> 
> "Drivers can set this up by calling
> drm_connector_attach_tv_margin_properties(), the values are stored in
> &drm_connector_state.tv."
> 
> With that: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> I'm also not entirely sure it's in pixels, but *shrug*

I'm sure the VC4 driver considers it's expressed in pixels :P. This was
decided after the discussion we had with Ville (when I submitted
margins support for VC4).

> -Daniel
> 
> 
> > + *
> > + *	left margin:
> > + *		Left margin
> > + *
> > + *	right margin:
> > + *		Left margin
> > + *
> > + *	top margin:
> > + *		Top margin
> > + *
> > + *	bottom margin:
> > + *		Bottom margin
> >   */
> >  
> >  /**
> > -- 
> > 2.20.1
> >   
> 

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

Re: [PATCH 2/2] drm/connector: Document the TV props

[Daniel Vetter]

On Wed, Mar 27, 2019 at 01:23:46PM +0100, Boris Brezillon wrote:
> On Wed, 27 Mar 2019 12:26:07 +0100
> Daniel Vetter <daniel@ffwll.ch> wrote:
> 
> > On Wed, Mar 27, 2019 at 10:59:16AM +0100, Boris Brezillon wrote:
> > > Document TV connector props and get rid of the according entries in
> > > the csv file.
> > > 
> > > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> > > ---
> > >  Documentation/gpu/drm-kms.rst        |  6 +++
> > >  Documentation/gpu/kms-properties.csv | 13 -------
> > >  drivers/gpu/drm/drm_connector.c      | 56 ++++++++++++++++++++++++++++
> > >  3 files changed, 62 insertions(+), 13 deletions(-)
> > > 
> > > diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> > > index 23a3c986ef6d..376f88e56d14 100644
> > > --- a/Documentation/gpu/drm-kms.rst
> > > +++ b/Documentation/gpu/drm-kms.rst
> > > @@ -449,6 +449,12 @@ HDMI Specific Connector Properties
> > >  .. kernel-doc:: drivers/gpu/drm/drm_connector.c
> > >     :doc: HDMI connector properties
> > >  
> > > +TV Connector Properties
> > > +-----------------------
> > > +
> > > +.. kernel-doc:: drivers/gpu/drm/drm_connector.c
> > > +   :doc: TV connector properties
> > > +
> > >  Plane Composition Properties
> > >  ----------------------------
> > >  
> > > diff --git a/Documentation/gpu/kms-properties.csv b/Documentation/gpu/kms-properties.csv
> > > index 07ed22ea3bd6..1a8277f3028d 100644
> > > --- a/Documentation/gpu/kms-properties.csv
> > > +++ b/Documentation/gpu/kms-properties.csv
> > > @@ -1,19 +1,6 @@
> > >  Owner Module/Drivers,Group,Property Name,Type,Property Values,Object attached,Description/Restrictions
> > >  ,DVI-I,“subconnector”,ENUM,"{ “Unknown”, “DVI-D”, “DVI-A” }",Connector,TBD
> > >  ,,“select subconnector”,ENUM,"{ “Automatic”, “DVI-D”, “DVI-A” }",Connector,TBD
> > > -,TV,“subconnector”,ENUM,"{ ""Unknown"", ""Composite"", ""SVIDEO"", ""Component"", ""SCART"" }",Connector,TBD
> > > -,,“select subconnector”,ENUM,"{ ""Automatic"", ""Composite"", ""SVIDEO"", ""Component"", ""SCART"" }",Connector,TBD
> > > -,,“mode”,ENUM,"{ ""NTSC_M"", ""NTSC_J"", ""NTSC_443"", ""PAL_B"" } etc.",Connector,TBD
> > > -,,“left margin”,RANGE,"Min=0, Max=100",Connector,TBD
> > > -,,“right margin”,RANGE,"Min=0, Max=100",Connector,TBD
> > > -,,“top margin”,RANGE,"Min=0, Max=100",Connector,TBD
> > > -,,“bottom margin”,RANGE,"Min=0, Max=100",Connector,TBD
> > > -,,“brightness”,RANGE,"Min=0, Max=100",Connector,TBD
> > > -,,“contrast”,RANGE,"Min=0, Max=100",Connector,TBD
> > > -,,“flicker reduction”,RANGE,"Min=0, Max=100",Connector,TBD
> > > -,,“overscan”,RANGE,"Min=0, Max=100",Connector,TBD
> > > -,,“saturation”,RANGE,"Min=0, Max=100",Connector,TBD
> > > -,,“hue”,RANGE,"Min=0, Max=100",Connector,TBD
> > >  ,Virtual GPU,“suggested X”,RANGE,"Min=0, Max=0xffffffff",Connector,property to suggest an X offset for a connector
> > >  ,,“suggested Y”,RANGE,"Min=0, Max=0xffffffff",Connector,property to suggest an Y offset for a connector
> > >  ,Optional,"""aspect ratio""",ENUM,"{ ""None"", ""4:3"", ""16:9"" }",Connector,TDB
> > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > > index 95dfb322b14e..bb581994b48e 100644
> > > --- a/drivers/gpu/drm/drm_connector.c
> > > +++ b/drivers/gpu/drm/drm_connector.c
> > > @@ -1249,6 +1249,62 @@ int drm_mode_create_tv_margin_properties(struct drm_device *dev)
> > >  }
> > >  EXPORT_SYMBOL(drm_mode_create_tv_margin_properties);
> > >  
> > > +/**
> > > + * DOC: TV connector properties
> > > + *
> > > + * subconnector:
> > > + *	Subconnector can be Composite, SVIDEO, Component or SCART. For legacy
> > > + *	reasons we have two properties, one to select the subconnector and one
> > > + *	showing the currently selected subconnector.
> > > + *
> > > + *	select subconnector:
> > > + *		Select a subconnector type. There's an 'Automatic' value to let
> > > + *		DRM select the subconnector automatically
> > > + *
> > > + * 	subconnector:
> > > + * 		Shows the currently selected subconnector  
> > 
> > There's also subconnector values for DVI-D vs. DVI-I on some drivers. Not
> > sure we want to include these here though ...
> 
> I'm documenting the TV connector props here. Maybe I should move that
> to the "standard connector properties"...
> 
> > 
> > > + *
> > > + * margins:
> > > + *	Defines invisible area of a screen so that the CRTC driver can possibly
> > > + *	scale the output image and move it to make it entirely visible. All
> > > + *	margins are defined in pixels and the valid range is 0 - 100.
> > > + *
> > > + *	left margin:
> > > + *		Left margin
> > > + *
> > > + *	right margin:
> > > + *		Left margin
> > > + *
> > > + *	top margin:
> > > + *		Top margin
> > > + *
> > > + *	bottom margin:
> > > + *		Bottom margin  
> > 
> > This feels redundant with the margins text in patch 1. I think better to
> > remove.
> 
> Problem is, the margin props were documented in the HDMI connector
> props section. If we want to make it applicable to both HDMI and TV
> connectors I should move it to the "standard connector properties"
> section and mention that these props only apply to HDMI and TV
> connectors.

Hm, I thought that's where they're documented, right below the content
protection stuff ... I guess I misread.
-Daniel

> 
> > 
> > > + *
> > > + * mode:
> > > + *	Exposes all supported TV modes and allows one to select a mode. The
> > > + *	list of supported modes is driver dependent, but you should basically
> > > + *	find one or more variants of the PAL and NTSC standards.
> > > + *
> > > + * brightness:
> > > + *	Brigthness settings expressed in percent
> > > + *
> > > + * contrast:
> > > + *	Contrast settings expressed in percent
> > > + *
> > > + * flicker reduction:
> > > + *	Flicker reduction settings expressed in percent
> > > + *
> > > + * overscan:
> > > + *	Overscan settings
> > > + *
> > > + * saturation:
> > > + *	Saturation settings expressed in percent
> > > + *
> > > + * hue:
> > > + *	Hue settings expressed in percent
> > > + */  
> > 
> > Not sure we want to sprinkle the usual set of references to create/attach
> > functions. Probably overkill, up to you.
> > 
> > With the redundant margins section removed:
> > 
> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > 
> > > +
> > >  /**
> > >   * drm_mode_create_tv_properties - create TV specific connector properties
> > >   * @dev: DRM device
> > > -- 
> > > 2.20.1
> > >   
> > 
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel