dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] drm/sun4i: Support clock and data polarities on LVDS output
@ 2020-02-14 12:24 Maxime Ripard
  2020-02-14 12:24 ` [PATCH 1/4] drm/connector: Add data polarity flags Maxime Ripard
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Maxime Ripard @ 2020-02-14 12:24 UTC (permalink / raw)
  To: Chen-Yu Tsai, Maxime Ripard, dri-devel
  Cc: Mark Rutland, devicetree, David Airlie, Sam Ravnborg,
	Rob Herring, Sean Paul, Maxime Ripard, Thierry Reding,
	Daniel Vetter, Frank Rowand, linux-arm-kernel

Hi,

The Allwinner LVDS encoder allows to change the polarity of clocks and
data lanes, so let's add the needed bus flags to DRM, panel-lvds and
our encoder driver.

Let me know what you think,
Maxime

Maxime Ripard (4):
  drm/connector: Add data polarity flags
  dt-bindings: panel: lvds: Add properties for clock and data polarities
  drm/panel: lvds: Support data and clock polarity flags
  drm/sun4i: lvds: Support data and clock polarity flags

 Documentation/devicetree/bindings/display/panel/lvds.yaml | 10 +++-
 drivers/gpu/drm/panel/panel-lvds.c                        | 25 +++++++-
 drivers/gpu/drm/sun4i/sun4i_tcon.c                        | 16 ++++-
 include/drm/drm_connector.h                               |  4 +-
 4 files changed, 49 insertions(+), 6 deletions(-)

base-commit: bb6d3fb354c5ee8d6bde2d576eb7220ea09862b9
-- 
git-series 0.9.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [PATCH 1/4] drm/connector: Add data polarity flags
  2020-02-14 12:24 [PATCH 0/4] drm/sun4i: Support clock and data polarities on LVDS output Maxime Ripard
@ 2020-02-14 12:24 ` Maxime Ripard
  2020-02-14 16:13   ` Sam Ravnborg
  2020-02-14 12:24 ` [PATCH 2/4] dt-bindings: panel: lvds: Add properties for clock and data polarities Maxime Ripard
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Maxime Ripard @ 2020-02-14 12:24 UTC (permalink / raw)
  To: Chen-Yu Tsai, Maxime Ripard, dri-devel
  Cc: Mark Rutland, devicetree, David Airlie, Sam Ravnborg,
	Rob Herring, Sean Paul, Maxime Ripard, Thierry Reding,
	Daniel Vetter, Frank Rowand, linux-arm-kernel

Some LVDS encoders can change the polarity of the data signals being
sent. Add a DRM bus flag to reflect this.

Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 include/drm/drm_connector.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 221910948b37..9a08fe6ab7c2 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -330,6 +330,8 @@ enum drm_panel_orientation {
  *					edge of the pixel clock
  * @DRM_BUS_FLAG_SHARP_SIGNALS:		Set if the Sharp-specific signals
  *					(SPL, CLS, PS, REV) must be used
+ * @DRM_BUS_FLAG_DATA_LOW:		The Data signals are active low
+ * @DRM_BUS_FLAG_DATA_HIGH:		The Data signals are active high
  */
 enum drm_bus_flags {
 	DRM_BUS_FLAG_DE_LOW = BIT(0),
@@ -349,6 +351,8 @@ enum drm_bus_flags {
 	DRM_BUS_FLAG_SYNC_SAMPLE_POSEDGE = DRM_BUS_FLAG_SYNC_NEGEDGE,
 	DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE = DRM_BUS_FLAG_SYNC_POSEDGE,
 	DRM_BUS_FLAG_SHARP_SIGNALS = BIT(8),
+	DRM_BUS_FLAG_DATA_LOW = BIT(9),
+	DRM_BUS_FLAG_DATA_HIGH = BIT(10),
 };
 
 /**
-- 
git-series 0.9.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [PATCH 2/4] dt-bindings: panel: lvds: Add properties for clock and data polarities
  2020-02-14 12:24 [PATCH 0/4] drm/sun4i: Support clock and data polarities on LVDS output Maxime Ripard
  2020-02-14 12:24 ` [PATCH 1/4] drm/connector: Add data polarity flags Maxime Ripard
@ 2020-02-14 12:24 ` Maxime Ripard
  2020-02-14 16:11   ` Sam Ravnborg
  2020-02-19 23:12   ` Rob Herring
  2020-02-14 12:24 ` [PATCH 3/4] drm/panel: lvds: Support data and clock polarity flags Maxime Ripard
  2020-02-14 12:24 ` [PATCH 4/4] drm/sun4i: " Maxime Ripard
  3 siblings, 2 replies; 10+ messages in thread
From: Maxime Ripard @ 2020-02-14 12:24 UTC (permalink / raw)
  To: Chen-Yu Tsai, Maxime Ripard, dri-devel
  Cc: Mark Rutland, devicetree, David Airlie, Sam Ravnborg,
	Rob Herring, Sean Paul, Maxime Ripard, Thierry Reding,
	Daniel Vetter, Frank Rowand, linux-arm-kernel

Some LVDS encoders can support multiple polarities on the data and
clock lanes, and similarly some panels require a given polarity on
their inputs. Add a property on the panel to configure the encoder
properly.

Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 Documentation/devicetree/bindings/display/panel/lvds.yaml | 10 ++++++++-
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/panel/lvds.yaml b/Documentation/devicetree/bindings/display/panel/lvds.yaml
index d0083301acbe..4a1111a1ab38 100644
--- a/Documentation/devicetree/bindings/display/panel/lvds.yaml
+++ b/Documentation/devicetree/bindings/display/panel/lvds.yaml
@@ -90,6 +90,16 @@ properties:
       CTL2: Data Enable
       CTL3: 0
 
+  clock-active-low:
+    type: boolean
+    description: >
+      If set, reverse the clock polarity on the clock lane.
+
+  data-active-low:
+    type: boolean
+    description: >
+      If set, reverse the bit polarity on all data lanes.
+
   data-mirror:
     type: boolean
     description:
-- 
git-series 0.9.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [PATCH 3/4] drm/panel: lvds: Support data and clock polarity flags
  2020-02-14 12:24 [PATCH 0/4] drm/sun4i: Support clock and data polarities on LVDS output Maxime Ripard
  2020-02-14 12:24 ` [PATCH 1/4] drm/connector: Add data polarity flags Maxime Ripard
  2020-02-14 12:24 ` [PATCH 2/4] dt-bindings: panel: lvds: Add properties for clock and data polarities Maxime Ripard
@ 2020-02-14 12:24 ` Maxime Ripard
  2020-02-14 12:24 ` [PATCH 4/4] drm/sun4i: " Maxime Ripard
  3 siblings, 0 replies; 10+ messages in thread
From: Maxime Ripard @ 2020-02-14 12:24 UTC (permalink / raw)
  To: Chen-Yu Tsai, Maxime Ripard, dri-devel
  Cc: Mark Rutland, devicetree, David Airlie, Sam Ravnborg,
	Rob Herring, Sean Paul, Maxime Ripard, Thierry Reding,
	Daniel Vetter, Frank Rowand, linux-arm-kernel

Add device tree properties to the panel-lvds driver to set the bus
flags properly.

Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/panel/panel-lvds.c | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-lvds.c b/drivers/gpu/drm/panel/panel-lvds.c
index 5ce3f4a2b7a1..c0d6dcd9e9fc 100644
--- a/drivers/gpu/drm/panel/panel-lvds.c
+++ b/drivers/gpu/drm/panel/panel-lvds.c
@@ -31,6 +31,8 @@ struct panel_lvds {
 	unsigned int height;
 	struct videomode video_mode;
 	unsigned int bus_format;
+	bool clk_active_low;
+	bool data_active_low;
 	bool data_mirror;
 
 	struct regulator *supply;
@@ -83,6 +85,7 @@ static int panel_lvds_get_modes(struct drm_panel *panel,
 {
 	struct panel_lvds *lvds = to_panel_lvds(panel);
 	struct drm_display_mode *mode;
+	unsigned int flags = 0;
 
 	mode = drm_mode_create(connector->dev);
 	if (!mode)
@@ -96,9 +99,23 @@ static int panel_lvds_get_modes(struct drm_panel *panel,
 	connector->display_info.height_mm = lvds->height;
 	drm_display_info_set_bus_formats(&connector->display_info,
 					 &lvds->bus_format, 1);
-	connector->display_info.bus_flags = lvds->data_mirror
-					  ? DRM_BUS_FLAG_DATA_LSB_TO_MSB
-					  : DRM_BUS_FLAG_DATA_MSB_TO_LSB;
+
+	if (lvds->data_mirror)
+		flags |= DRM_BUS_FLAG_DATA_LSB_TO_MSB;
+	else
+		flags |= DRM_BUS_FLAG_DATA_MSB_TO_LSB;
+
+	if (lvds->clk_active_low)
+		flags |= DRM_BUS_FLAG_PIXDATA_NEGEDGE;
+	else
+		flags |= DRM_BUS_FLAG_PIXDATA_POSEDGE;
+
+	if (lvds->data_active_low)
+		flags |= DRM_BUS_FLAG_DATA_LOW;
+	else
+		flags |= DRM_BUS_FLAG_DATA_HIGH;
+
+	connector->display_info.bus_flags = flags;
 
 	return 1;
 }
@@ -159,6 +176,8 @@ static int panel_lvds_parse_dt(struct panel_lvds *lvds)
 		return -EINVAL;
 	}
 
+	lvds->clk_active_low = of_property_read_bool(np, "clock-active-low");
+	lvds->data_active_low = of_property_read_bool(np, "data-active-low");
 	lvds->data_mirror = of_property_read_bool(np, "data-mirror");
 
 	return 0;
-- 
git-series 0.9.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [PATCH 4/4] drm/sun4i: lvds: Support data and clock polarity flags
  2020-02-14 12:24 [PATCH 0/4] drm/sun4i: Support clock and data polarities on LVDS output Maxime Ripard
                   ` (2 preceding siblings ...)
  2020-02-14 12:24 ` [PATCH 3/4] drm/panel: lvds: Support data and clock polarity flags Maxime Ripard
@ 2020-02-14 12:24 ` Maxime Ripard
  3 siblings, 0 replies; 10+ messages in thread
From: Maxime Ripard @ 2020-02-14 12:24 UTC (permalink / raw)
  To: Chen-Yu Tsai, Maxime Ripard, dri-devel
  Cc: Mark Rutland, devicetree, David Airlie, Sam Ravnborg,
	Rob Herring, Sean Paul, Maxime Ripard, Thierry Reding,
	Daniel Vetter, Frank Rowand, linux-arm-kernel

Our LVDS encoder can change the polarity of data and clock signals on
the LVDS link. Make sure we don't ignore the matching bus flags.

Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/sun4i/sun4i_tcon.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index c81cdce6ed55..fdf143042f83 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -404,6 +404,8 @@ static void sun4i_tcon0_mode_set_lvds(struct sun4i_tcon *tcon,
 				      const struct drm_encoder *encoder,
 				      const struct drm_display_mode *mode)
 {
+	struct drm_connector *connector = sun4i_tcon_get_connector(encoder);
+	const struct drm_display_info *info = &connector->display_info;
 	unsigned int bp;
 	u8 clk_delay;
 	u32 reg, val = 0;
@@ -449,9 +451,17 @@ static void sun4i_tcon0_mode_set_lvds(struct sun4i_tcon *tcon,
 		     SUN4I_TCON0_BASIC2_V_TOTAL(mode->crtc_vtotal * 2) |
 		     SUN4I_TCON0_BASIC2_V_BACKPORCH(bp));
 
-	reg = SUN4I_TCON0_LVDS_IF_CLK_SEL_TCON0 |
-		SUN4I_TCON0_LVDS_IF_DATA_POL_NORMAL |
-		SUN4I_TCON0_LVDS_IF_CLK_POL_NORMAL;
+	reg = SUN4I_TCON0_LVDS_IF_CLK_SEL_TCON0;
+	if (info->bus_flags & DRM_BUS_FLAG_PIXDATA_NEGEDGE)
+		reg |= SUN4I_TCON0_LVDS_IF_CLK_POL_INV;
+	else
+		reg |= SUN4I_TCON0_LVDS_IF_CLK_POL_NORMAL;
+
+	if (info->bus_flags & DRM_BUS_FLAG_DATA_LOW)
+		reg |= SUN4I_TCON0_LVDS_IF_DATA_POL_INV;
+	else
+		reg |= SUN4I_TCON0_LVDS_IF_DATA_POL_NORMAL;
+
 	if (sun4i_tcon_get_pixel_depth(encoder) == 24)
 		reg |= SUN4I_TCON0_LVDS_IF_BITWIDTH_24BITS;
 	else
-- 
git-series 0.9.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH 2/4] dt-bindings: panel: lvds: Add properties for clock and data polarities
  2020-02-14 12:24 ` [PATCH 2/4] dt-bindings: panel: lvds: Add properties for clock and data polarities Maxime Ripard
@ 2020-02-14 16:11   ` Sam Ravnborg
  2020-02-20 17:57     ` Maxime Ripard
  2020-02-19 23:12   ` Rob Herring
  1 sibling, 1 reply; 10+ messages in thread
From: Sam Ravnborg @ 2020-02-14 16:11 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Mark Rutland, devicetree, David Airlie, dri-devel, Chen-Yu Tsai,
	Rob Herring, Sean Paul, Thierry Reding, Daniel Vetter,
	Frank Rowand, linux-arm-kernel

Hi Maxime.

On Fri, Feb 14, 2020 at 01:24:39PM +0100, Maxime Ripard wrote:
> Some LVDS encoders can support multiple polarities on the data and
> clock lanes, and similarly some panels require a given polarity on
> their inputs. Add a property on the panel to configure the encoder
> properly.
> 
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>

Not a fan of this binding...
In display-timing.txt we have a specification/description of
the panel-timing node.

The panel-timing node already include information such as:
- hsync-active:
- vsync-active:
- de-active:
- pixelclk-active:
- syncclk-active:

But clock-active-low and data-active-low refer to the bus
more than an individual timing.
So maybe OK not to have it in a panel-timing node.
But then it would IMO be better to include
this in the display-timing node - so we make
this available and standard for all users of the
display-timing node.

I will dig up my patchset to make proper bindings for panel-timing and
display-timing this weeked and resend them.
Then we can discuss if this goes on top or this is specific for the
lvds binding.


> ---
>  Documentation/devicetree/bindings/display/panel/lvds.yaml | 10 ++++++++-
>  1 file changed, 10 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/panel/lvds.yaml b/Documentation/devicetree/bindings/display/panel/lvds.yaml
> index d0083301acbe..4a1111a1ab38 100644
> --- a/Documentation/devicetree/bindings/display/panel/lvds.yaml
> +++ b/Documentation/devicetree/bindings/display/panel/lvds.yaml
> @@ -90,6 +90,16 @@ properties:
>        CTL2: Data Enable
>        CTL3: 0
>  
> +  clock-active-low:
> +    type: boolean
> +    description: >

Should this be "|" and not ">"?
Did this pass dt_binding_check?

> +      If set, reverse the clock polarity on the clock lane.
This text could be a bit more specific. If this is set then
what?
And it seems strange that a clock is active low.
For a clock we often talk about raising or falling edge.

> +
> +  data-active-low:
> +    type: boolean
> +    description: >
Same comment with ">"

> +      If set, reverse the bit polarity on all data lanes.
Same comment about a more explicit description.


	Sam


>    data-mirror:
>      type: boolean
>      description:
> -- 
> git-series 0.9.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH 1/4] drm/connector: Add data polarity flags
  2020-02-14 12:24 ` [PATCH 1/4] drm/connector: Add data polarity flags Maxime Ripard
@ 2020-02-14 16:13   ` Sam Ravnborg
  2020-02-20 18:00     ` Maxime Ripard
  0 siblings, 1 reply; 10+ messages in thread
From: Sam Ravnborg @ 2020-02-14 16:13 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Mark Rutland, devicetree, David Airlie, dri-devel, Chen-Yu Tsai,
	Rob Herring, Sean Paul, Thierry Reding, Daniel Vetter,
	Frank Rowand, linux-arm-kernel

Hi Maxime.

On Fri, Feb 14, 2020 at 01:24:38PM +0100, Maxime Ripard wrote:
> Some LVDS encoders can change the polarity of the data signals being
> sent. Add a DRM bus flag to reflect this.
> 
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
>  include/drm/drm_connector.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 221910948b37..9a08fe6ab7c2 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -330,6 +330,8 @@ enum drm_panel_orientation {
>   *					edge of the pixel clock
>   * @DRM_BUS_FLAG_SHARP_SIGNALS:		Set if the Sharp-specific signals
>   *					(SPL, CLS, PS, REV) must be used
> + * @DRM_BUS_FLAG_DATA_LOW:		The Data signals are active low
> + * @DRM_BUS_FLAG_DATA_HIGH:		The Data signals are active high
Reading the description of these falgs always confuses me.
In this case if neither bit 9 nor bit 10 is set then the data signals
are netiher active low nor active high.
So what can I then expect?

We have the same logic loophole for DRM_BUS_FLAG_SYNC_SAMPLE_POSEDGE
and DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE.
So it is not new, but can we do better here?

	Sam


>   */
>  enum drm_bus_flags {
>  	DRM_BUS_FLAG_DE_LOW = BIT(0),
> @@ -349,6 +351,8 @@ enum drm_bus_flags {
>  	DRM_BUS_FLAG_SYNC_SAMPLE_POSEDGE = DRM_BUS_FLAG_SYNC_NEGEDGE,
>  	DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE = DRM_BUS_FLAG_SYNC_POSEDGE,
>  	DRM_BUS_FLAG_SHARP_SIGNALS = BIT(8),
> +	DRM_BUS_FLAG_DATA_LOW = BIT(9),
> +	DRM_BUS_FLAG_DATA_HIGH = BIT(10),
>  };
>  
>  /**
> -- 
> git-series 0.9.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH 2/4] dt-bindings: panel: lvds: Add properties for clock and data polarities
  2020-02-14 12:24 ` [PATCH 2/4] dt-bindings: panel: lvds: Add properties for clock and data polarities Maxime Ripard
  2020-02-14 16:11   ` Sam Ravnborg
@ 2020-02-19 23:12   ` Rob Herring
  1 sibling, 0 replies; 10+ messages in thread
From: Rob Herring @ 2020-02-19 23:12 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Mark Rutland, devicetree, David Airlie, Sam Ravnborg, dri-devel,
	Chen-Yu Tsai, Thierry Reding, Sean Paul, Daniel Vetter,
	Frank Rowand, linux-arm-kernel

On Fri, Feb 14, 2020 at 01:24:39PM +0100, Maxime Ripard wrote:
> Some LVDS encoders can support multiple polarities on the data and
> clock lanes, and similarly some panels require a given polarity on
> their inputs. Add a property on the panel to configure the encoder
> properly.

If the panel requires a specific setting, then that can be implied by 
the panel's compatible. Does Boris' format constraint solving series 
help here?

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

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

* Re: [PATCH 2/4] dt-bindings: panel: lvds: Add properties for clock and data polarities
  2020-02-14 16:11   ` Sam Ravnborg
@ 2020-02-20 17:57     ` Maxime Ripard
  0 siblings, 0 replies; 10+ messages in thread
From: Maxime Ripard @ 2020-02-20 17:57 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: Mark Rutland, devicetree, David Airlie, dri-devel, Chen-Yu Tsai,
	Rob Herring, Sean Paul, Thierry Reding, Daniel Vetter,
	Frank Rowand, linux-arm-kernel


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

On Fri, Feb 14, 2020 at 05:11:56PM +0100, Sam Ravnborg wrote:
> Hi Maxime.
>
> On Fri, Feb 14, 2020 at 01:24:39PM +0100, Maxime Ripard wrote:
> > Some LVDS encoders can support multiple polarities on the data and
> > clock lanes, and similarly some panels require a given polarity on
> > their inputs. Add a property on the panel to configure the encoder
> > properly.
> >
> > Signed-off-by: Maxime Ripard <maxime@cerno.tech>
>
> Not a fan of this binding...
> In display-timing.txt we have a specification/description of
> the panel-timing node.
>
> The panel-timing node already include information such as:
> - hsync-active:
> - vsync-active:
> - de-active:
> - pixelclk-active:
> - syncclk-active:
>
> But clock-active-low and data-active-low refer to the bus
> more than an individual timing.
> So maybe OK not to have it in a panel-timing node.
> But then it would IMO be better to include
> this in the display-timing node - so we make
> this available and standard for all users of the
> display-timing node.
>
> I will dig up my patchset to make proper bindings for panel-timing and
> display-timing this weeked and resend them.
> Then we can discuss if this goes on top or this is specific for the
> lvds binding.

That makes sense, I'll wait for them to be merged then :)

>
> > ---
> >  Documentation/devicetree/bindings/display/panel/lvds.yaml | 10 ++++++++-
> >  1 file changed, 10 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/display/panel/lvds.yaml b/Documentation/devicetree/bindings/display/panel/lvds.yaml
> > index d0083301acbe..4a1111a1ab38 100644
> > --- a/Documentation/devicetree/bindings/display/panel/lvds.yaml
> > +++ b/Documentation/devicetree/bindings/display/panel/lvds.yaml
> > @@ -90,6 +90,16 @@ properties:
> >        CTL2: Data Enable
> >        CTL3: 0
> >
> > +  clock-active-low:
> > +    type: boolean
> > +    description: >
>
> Should this be "|" and not ">"?
> Did this pass dt_binding_check?

Yes. > means that this is a multi-line string that will drop the \n
between each line, while | will keep it

For a string like this, I believe it makes more sense to let whatever
is using to handle the wrapping, but I don't really have a strong
opinion :)

>
> > +      If set, reverse the clock polarity on the clock lane.
> This text could be a bit more specific. If this is set then
> what?
> And it seems strange that a clock is active low.
> For a clock we often talk about raising or falling edge.
>
> > +
> > +  data-active-low:
> > +    type: boolean
> > +    description: >
> Same comment with ">"
>
> > +      If set, reverse the bit polarity on all data lanes.
> Same comment about a more explicit description.

I'll try to come up with something better. Thanks!
Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

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

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

* Re: [PATCH 1/4] drm/connector: Add data polarity flags
  2020-02-14 16:13   ` Sam Ravnborg
@ 2020-02-20 18:00     ` Maxime Ripard
  0 siblings, 0 replies; 10+ messages in thread
From: Maxime Ripard @ 2020-02-20 18:00 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: Mark Rutland, devicetree, David Airlie, dri-devel, Chen-Yu Tsai,
	Rob Herring, Sean Paul, Thierry Reding, Daniel Vetter,
	Frank Rowand, linux-arm-kernel


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

On Fri, Feb 14, 2020 at 05:13:59PM +0100, Sam Ravnborg wrote:
> Hi Maxime.
>
> On Fri, Feb 14, 2020 at 01:24:38PM +0100, Maxime Ripard wrote:
> > Some LVDS encoders can change the polarity of the data signals being
> > sent. Add a DRM bus flag to reflect this.
> >
> > Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> > ---
> >  include/drm/drm_connector.h | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> > index 221910948b37..9a08fe6ab7c2 100644
> > --- a/include/drm/drm_connector.h
> > +++ b/include/drm/drm_connector.h
> > @@ -330,6 +330,8 @@ enum drm_panel_orientation {
> >   *					edge of the pixel clock
> >   * @DRM_BUS_FLAG_SHARP_SIGNALS:		Set if the Sharp-specific signals
> >   *					(SPL, CLS, PS, REV) must be used
> > + * @DRM_BUS_FLAG_DATA_LOW:		The Data signals are active low
> > + * @DRM_BUS_FLAG_DATA_HIGH:		The Data signals are active high
> Reading the description of these falgs always confuses me.
> In this case if neither bit 9 nor bit 10 is set then the data signals
> are netiher active low nor active high.
> So what can I then expect?
>
> We have the same logic loophole for DRM_BUS_FLAG_SYNC_SAMPLE_POSEDGE
> and DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE.
> So it is not new, but can we do better here?

Honestly, I don't really get it either. I *think* this is to handle
the sampling / output inversion properly which wouldn't be possible if
this was only a bit set or not.

Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

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

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

end of thread, other threads:[~2020-02-21 11:15 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-14 12:24 [PATCH 0/4] drm/sun4i: Support clock and data polarities on LVDS output Maxime Ripard
2020-02-14 12:24 ` [PATCH 1/4] drm/connector: Add data polarity flags Maxime Ripard
2020-02-14 16:13   ` Sam Ravnborg
2020-02-20 18:00     ` Maxime Ripard
2020-02-14 12:24 ` [PATCH 2/4] dt-bindings: panel: lvds: Add properties for clock and data polarities Maxime Ripard
2020-02-14 16:11   ` Sam Ravnborg
2020-02-20 17:57     ` Maxime Ripard
2020-02-19 23:12   ` Rob Herring
2020-02-14 12:24 ` [PATCH 3/4] drm/panel: lvds: Support data and clock polarity flags Maxime Ripard
2020-02-14 12:24 ` [PATCH 4/4] drm/sun4i: " Maxime Ripard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).