devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/2] dt-bindings: display: bridge: tc358867: Document default DP preemphasis
@ 2024-06-21 15:53 Marek Vasut
  2024-06-21 15:53 ` [PATCH v2 2/2] drm/bridge: tc358767: Add configurable default preemphasis Marek Vasut
  2024-06-22 11:56 ` [PATCH v2 1/2] dt-bindings: display: bridge: tc358867: Document default DP preemphasis Conor Dooley
  0 siblings, 2 replies; 6+ messages in thread
From: Marek Vasut @ 2024-06-21 15:53 UTC (permalink / raw)
  To: dri-devel
  Cc: Marek Vasut, Andrzej Hajda, Conor Dooley, Daniel Vetter,
	David Airlie, Jernej Skrabec, Jonas Karlman, Krzysztof Kozlowski,
	Laurent Pinchart, Lucas Stach, Maarten Lankhorst, Maxime Ripard,
	Neil Armstrong, Rob Herring, Robert Foss, Thomas Zimmermann,
	devicetree, kernel

Document default DP port preemphasis configurable via new DT property
"toshiba,pre-emphasis". This is useful in case the DP link properties
are known and starting link training from preemphasis setting of 0 dB
is not useful. The preemphasis can be set separately for both DP lanes
in range 0=0dB, 1=3.5dB, 2=6dB .

Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Andrzej Hajda <andrzej.hajda@intel.com>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: David Airlie <airlied@gmail.com>
Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
Cc: Jonas Karlman <jonas@kwiboo.se>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Neil Armstrong <neil.armstrong@linaro.org>
Cc: Rob Herring <robh@kernel.org>
Cc: Robert Foss <rfoss@kernel.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: devicetree@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: kernel@dh-electronics.com
---
V2: - Fix the type to u8 array
    - Fix the enum items to match what they represent
---
 .../display/bridge/toshiba,tc358767.yaml       | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.yaml b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.yaml
index 2ad0cd6dd49e0..6287eb2b40908 100644
--- a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.yaml
@@ -98,6 +98,24 @@ properties:
             reference to a valid eDP panel input endpoint node. This port is
             optional, treated as DP panel if not defined
 
+        properties:
+          endpoint:
+            $ref: /schemas/media/video-interfaces.yaml#
+            unevaluatedProperties: false
+
+            properties:
+              toshiba,pre-emphasis:
+                description:
+                  Display port output Pre-Emphasis settings for both ports.
+                $ref: /schemas/types.yaml#/definitions/uint8-array
+                minItems: 2
+                maxItems: 2
+                items:
+                  enum:
+                    - 0 # No pre-emphasis
+                    - 1 # 3.5dB pre-emphasis
+                    - 2 # 6dB pre-emphasis
+
     oneOf:
       - required:
           - port@0
-- 
2.43.0


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

* [PATCH v2 2/2] drm/bridge: tc358767: Add configurable default preemphasis
  2024-06-21 15:53 [PATCH v2 1/2] dt-bindings: display: bridge: tc358867: Document default DP preemphasis Marek Vasut
@ 2024-06-21 15:53 ` Marek Vasut
  2024-06-22 11:56 ` [PATCH v2 1/2] dt-bindings: display: bridge: tc358867: Document default DP preemphasis Conor Dooley
  1 sibling, 0 replies; 6+ messages in thread
From: Marek Vasut @ 2024-06-21 15:53 UTC (permalink / raw)
  To: dri-devel
  Cc: Marek Vasut, Andrzej Hajda, Conor Dooley, Daniel Vetter,
	David Airlie, Jernej Skrabec, Jonas Karlman, Krzysztof Kozlowski,
	Laurent Pinchart, Lucas Stach, Maarten Lankhorst, Maxime Ripard,
	Neil Armstrong, Rob Herring, Robert Foss, Thomas Zimmermann,
	devicetree, kernel

Make the default DP port preemphasis configurable via new DT property
"toshiba,pre-emphasis". This is useful in case the DP link properties
are known and starting link training from preemphasis setting of 0 dB
is not useful. The preemphasis can be set separately for both DP lanes
in range 0=0dB, 1=3.5dB, 2=6dB .

Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Andrzej Hajda <andrzej.hajda@intel.com>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: David Airlie <airlied@gmail.com>
Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
Cc: Jonas Karlman <jonas@kwiboo.se>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Neil Armstrong <neil.armstrong@linaro.org>
Cc: Rob Herring <robh@kernel.org>
Cc: Robert Foss <rfoss@kernel.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: devicetree@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: kernel@dh-electronics.com
---
V2: - Parse toshiba,pre-emphasis property out of an endpoint of port 2 (the DP port)
---
 drivers/gpu/drm/bridge/tc358767.c | 45 ++++++++++++++++++++++++++-----
 1 file changed, 38 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
index 2b035a136a6e5..8f81fc960d0fa 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -241,6 +241,10 @@
 
 /* Link Training */
 #define DP0_SRCCTRL		0x06a0
+#define DP0_SRCCTRL_PRE1		GENMASK(29, 28)
+#define DP0_SRCCTRL_SWG1		GENMASK(25, 24)
+#define DP0_SRCCTRL_PRE0		GENMASK(21, 20)
+#define DP0_SRCCTRL_SWG0		GENMASK(17, 16)
 #define DP0_SRCCTRL_SCRMBLDIS		BIT(13)
 #define DP0_SRCCTRL_EN810B		BIT(12)
 #define DP0_SRCCTRL_NOTP		(0 << 8)
@@ -278,6 +282,8 @@
 #define AUDIFDATA6		0x0720	/* DP0 Audio Info Frame Bytes 27 to 24 */
 
 #define DP1_SRCCTRL		0x07a0	/* DP1 Control Register */
+#define DP1_SRCCTRL_PRE			GENMASK(21, 20)
+#define DP1_SRCCTRL_SWG			GENMASK(17, 16)
 
 /* PHY */
 #define DP_PHY_CTRL		0x0800
@@ -369,6 +375,7 @@ struct tc_data {
 
 	u32			rev;
 	u8			assr;
+	u8			pre_emphasis[2];
 
 	struct gpio_desc	*sd_gpio;
 	struct gpio_desc	*reset_gpio;
@@ -1090,13 +1097,17 @@ static int tc_main_link_enable(struct tc_data *tc)
 			return ret;
 	}
 
-	ret = regmap_write(tc->regmap, DP0_SRCCTRL, tc_srcctrl(tc));
+	ret = regmap_write(tc->regmap, DP0_SRCCTRL,
+			   tc_srcctrl(tc) |
+			   FIELD_PREP(DP0_SRCCTRL_PRE0, tc->pre_emphasis[0]) |
+			   FIELD_PREP(DP0_SRCCTRL_PRE1, tc->pre_emphasis[1]));
 	if (ret)
 		return ret;
 	/* SSCG and BW27 on DP1 must be set to the same as on DP0 */
 	ret = regmap_write(tc->regmap, DP1_SRCCTRL,
 		 (tc->link.spread ? DP0_SRCCTRL_SSCG : 0) |
-		 ((tc->link.rate != 162000) ? DP0_SRCCTRL_BW27 : 0));
+		 ((tc->link.rate != 162000) ? DP0_SRCCTRL_BW27 : 0) |
+		 FIELD_PREP(DP1_SRCCTRL_PRE, tc->pre_emphasis[1]));
 	if (ret)
 		return ret;
 
@@ -1188,8 +1199,10 @@ static int tc_main_link_enable(struct tc_data *tc)
 		goto err_dpcd_write;
 
 	/* Reset voltage-swing & pre-emphasis */
-	tmp[0] = tmp[1] = DP_TRAIN_VOLTAGE_SWING_LEVEL_0 |
-			  DP_TRAIN_PRE_EMPH_LEVEL_0;
+	tmp[0] = DP_TRAIN_VOLTAGE_SWING_LEVEL_0 |
+		 FIELD_PREP(DP_TRAIN_PRE_EMPHASIS_MASK, tc->pre_emphasis[0]);
+	tmp[1] = DP_TRAIN_VOLTAGE_SWING_LEVEL_0 |
+		 FIELD_PREP(DP_TRAIN_PRE_EMPHASIS_MASK, tc->pre_emphasis[1]);
 	ret = drm_dp_dpcd_write(aux, DP_TRAINING_LANE0_SET, tmp, 2);
 	if (ret < 0)
 		goto err_dpcd_write;
@@ -1213,7 +1226,9 @@ static int tc_main_link_enable(struct tc_data *tc)
 	ret = regmap_write(tc->regmap, DP0_SRCCTRL,
 			   tc_srcctrl(tc) | DP0_SRCCTRL_SCRMBLDIS |
 			   DP0_SRCCTRL_AUTOCORRECT |
-			   DP0_SRCCTRL_TP1);
+			   DP0_SRCCTRL_TP1 |
+			   FIELD_PREP(DP0_SRCCTRL_PRE0, tc->pre_emphasis[0]) |
+			   FIELD_PREP(DP0_SRCCTRL_PRE1, tc->pre_emphasis[1]));
 	if (ret)
 		return ret;
 
@@ -1248,7 +1263,9 @@ static int tc_main_link_enable(struct tc_data *tc)
 	ret = regmap_write(tc->regmap, DP0_SRCCTRL,
 			   tc_srcctrl(tc) | DP0_SRCCTRL_SCRMBLDIS |
 			   DP0_SRCCTRL_AUTOCORRECT |
-			   DP0_SRCCTRL_TP2);
+			   DP0_SRCCTRL_TP2 |
+			   FIELD_PREP(DP0_SRCCTRL_PRE0, tc->pre_emphasis[0]) |
+			   FIELD_PREP(DP0_SRCCTRL_PRE1, tc->pre_emphasis[1]));
 	if (ret)
 		return ret;
 
@@ -1274,7 +1291,9 @@ static int tc_main_link_enable(struct tc_data *tc)
 
 	/* Clear Training Pattern, set AutoCorrect Mode = 1 */
 	ret = regmap_write(tc->regmap, DP0_SRCCTRL, tc_srcctrl(tc) |
-			   DP0_SRCCTRL_AUTOCORRECT);
+			   DP0_SRCCTRL_AUTOCORRECT |
+			   FIELD_PREP(DP0_SRCCTRL_PRE0, tc->pre_emphasis[0]) |
+			   FIELD_PREP(DP0_SRCCTRL_PRE1, tc->pre_emphasis[1]));
 	if (ret)
 		return ret;
 
@@ -2408,6 +2427,18 @@ static int tc_probe_bridge_endpoint(struct tc_data *tc)
 			return -EINVAL;
 		}
 		mode |= BIT(endpoint.port);
+		if (endpoint.port != 2)
+			continue;
+
+		of_property_read_u8_array(node, "toshiba,pre-emphasis",
+					  tc->pre_emphasis,
+					  ARRAY_SIZE(tc->pre_emphasis));
+
+		if (tc->pre_emphasis[0] < 0 || tc->pre_emphasis[0] > 2 ||
+		    tc->pre_emphasis[1] < 0 || tc->pre_emphasis[1] > 2) {
+			dev_err(dev, "Incorrect Pre-Emphasis setting, use either 0=0dB 1=3.5dB 2=6dB\n");
+			return -EINVAL;
+		}
 	}
 
 	if (mode == mode_dpi_to_edp || mode == mode_dpi_to_dp) {
-- 
2.43.0


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

* Re: [PATCH v2 1/2] dt-bindings: display: bridge: tc358867: Document default DP preemphasis
  2024-06-21 15:53 [PATCH v2 1/2] dt-bindings: display: bridge: tc358867: Document default DP preemphasis Marek Vasut
  2024-06-21 15:53 ` [PATCH v2 2/2] drm/bridge: tc358767: Add configurable default preemphasis Marek Vasut
@ 2024-06-22 11:56 ` Conor Dooley
  2024-06-23 14:48   ` Marek Vasut
  1 sibling, 1 reply; 6+ messages in thread
From: Conor Dooley @ 2024-06-22 11:56 UTC (permalink / raw)
  To: Marek Vasut
  Cc: dri-devel, Andrzej Hajda, Conor Dooley, Daniel Vetter,
	David Airlie, Jernej Skrabec, Jonas Karlman, Krzysztof Kozlowski,
	Laurent Pinchart, Lucas Stach, Maarten Lankhorst, Maxime Ripard,
	Neil Armstrong, Rob Herring, Robert Foss, Thomas Zimmermann,
	devicetree, kernel

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

On Fri, Jun 21, 2024 at 05:53:53PM +0200, Marek Vasut wrote:
> Document default DP port preemphasis configurable via new DT property
> "toshiba,pre-emphasis". This is useful in case the DP link properties
> are known and starting link training from preemphasis setting of 0 dB
> is not useful. The preemphasis can be set separately for both DP lanes
> in range 0=0dB, 1=3.5dB, 2=6dB .
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Andrzej Hajda <andrzej.hajda@intel.com>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
> Cc: Jonas Karlman <jonas@kwiboo.se>
> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Neil Armstrong <neil.armstrong@linaro.org>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Robert Foss <rfoss@kernel.org>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: devicetree@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: kernel@dh-electronics.com
> ---
> V2: - Fix the type to u8 array
>     - Fix the enum items to match what they represent
> ---
>  .../display/bridge/toshiba,tc358767.yaml       | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.yaml b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.yaml
> index 2ad0cd6dd49e0..6287eb2b40908 100644
> --- a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.yaml
> @@ -98,6 +98,24 @@ properties:
>              reference to a valid eDP panel input endpoint node. This port is
>              optional, treated as DP panel if not defined
>  
> +        properties:
> +          endpoint:
> +            $ref: /schemas/media/video-interfaces.yaml#
> +            unevaluatedProperties: false
> +
> +            properties:
> +              toshiba,pre-emphasis:
> +                description:
> +                  Display port output Pre-Emphasis settings for both ports.

Why here and not in the port nodes?

> +                $ref: /schemas/types.yaml#/definitions/uint8-array
> +                minItems: 2
> +                maxItems: 2
> +                items:
> +                  enum:
> +                    - 0 # No pre-emphasis
> +                    - 1 # 3.5dB pre-emphasis
> +                    - 2 # 6dB pre-emphasis

I'd love to say please make this -bB and put this in units, but that'd
require it being a string..

> +
>      oneOf:
>        - required:
>            - port@0
> -- 
> 2.43.0
> 

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

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

* Re: [PATCH v2 1/2] dt-bindings: display: bridge: tc358867: Document default DP preemphasis
  2024-06-22 11:56 ` [PATCH v2 1/2] dt-bindings: display: bridge: tc358867: Document default DP preemphasis Conor Dooley
@ 2024-06-23 14:48   ` Marek Vasut
  2024-06-23 17:20     ` Conor Dooley
  0 siblings, 1 reply; 6+ messages in thread
From: Marek Vasut @ 2024-06-23 14:48 UTC (permalink / raw)
  To: Conor Dooley
  Cc: dri-devel, Andrzej Hajda, Conor Dooley, Daniel Vetter,
	David Airlie, Jernej Skrabec, Jonas Karlman, Krzysztof Kozlowski,
	Laurent Pinchart, Lucas Stach, Maarten Lankhorst, Maxime Ripard,
	Neil Armstrong, Rob Herring, Robert Foss, Thomas Zimmermann,
	devicetree, kernel

On 6/22/24 1:56 PM, Conor Dooley wrote:
> On Fri, Jun 21, 2024 at 05:53:53PM +0200, Marek Vasut wrote:
>> Document default DP port preemphasis configurable via new DT property
>> "toshiba,pre-emphasis". This is useful in case the DP link properties
>> are known and starting link training from preemphasis setting of 0 dB
>> is not useful. The preemphasis can be set separately for both DP lanes
>> in range 0=0dB, 1=3.5dB, 2=6dB .
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> ---
>> V2: - Fix the type to u8 array
>>      - Fix the enum items to match what they represent
>> ---
>>   .../display/bridge/toshiba,tc358767.yaml       | 18 ++++++++++++++++++
>>   1 file changed, 18 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.yaml b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.yaml
>> index 2ad0cd6dd49e0..6287eb2b40908 100644
>> --- a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.yaml
>> +++ b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.yaml
>> @@ -98,6 +98,24 @@ properties:
>>               reference to a valid eDP panel input endpoint node. This port is
>>               optional, treated as DP panel if not defined
>>   
>> +        properties:
>> +          endpoint:
>> +            $ref: /schemas/media/video-interfaces.yaml#
>> +            unevaluatedProperties: false
>> +
>> +            properties:
>> +              toshiba,pre-emphasis:
>> +                description:
>> +                  Display port output Pre-Emphasis settings for both ports.
> 
> Why here and not in the port nodes?

There was a short discussion about that in V1:

https://lore.kernel.org/all/00e9ef90-3bbe-4556-8da9-462f65928781@denx.de/

"
Let's keep it in the endpoint node.

There is some mention in the TC9595 datasheet that the DP might operate
in some split mode, where each DP lane is used to feed one display (?),
so I assume in that case there might be two endpoints (?), but that is
not supported right now.

If that is ever needed, I guess this array would have minItems 1 and
maxItems 2 and another endpoint would be added to the schema for this
port 2.
"

>> +                $ref: /schemas/types.yaml#/definitions/uint8-array
>> +                minItems: 2
>> +                maxItems: 2
>> +                items:
>> +                  enum:
>> +                    - 0 # No pre-emphasis
>> +                    - 1 # 3.5dB pre-emphasis
>> +                    - 2 # 6dB pre-emphasis
> 
> I'd love to say please make this -bB and put this in units, but that'd
> require it being a string..

I can do that. Do you think that's worth it ?

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

* Re: [PATCH v2 1/2] dt-bindings: display: bridge: tc358867: Document default DP preemphasis
  2024-06-23 14:48   ` Marek Vasut
@ 2024-06-23 17:20     ` Conor Dooley
  2024-06-24  0:05       ` Marek Vasut
  0 siblings, 1 reply; 6+ messages in thread
From: Conor Dooley @ 2024-06-23 17:20 UTC (permalink / raw)
  To: Marek Vasut
  Cc: dri-devel, Andrzej Hajda, Conor Dooley, Daniel Vetter,
	David Airlie, Jernej Skrabec, Jonas Karlman, Krzysztof Kozlowski,
	Laurent Pinchart, Lucas Stach, Maarten Lankhorst, Maxime Ripard,
	Neil Armstrong, Rob Herring, Robert Foss, Thomas Zimmermann,
	devicetree, kernel

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

On Sun, Jun 23, 2024 at 04:48:47PM +0200, Marek Vasut wrote:
> On 6/22/24 1:56 PM, Conor Dooley wrote:
> > On Fri, Jun 21, 2024 at 05:53:53PM +0200, Marek Vasut wrote:
> > > Document default DP port preemphasis configurable via new DT property
> > > "toshiba,pre-emphasis". This is useful in case the DP link properties
> > > are known and starting link training from preemphasis setting of 0 dB
> > > is not useful. The preemphasis can be set separately for both DP lanes
> > > in range 0=0dB, 1=3.5dB, 2=6dB .
> > > 
> > > Signed-off-by: Marek Vasut <marex@denx.de>
> > > ---
> > > V2: - Fix the type to u8 array
> > >      - Fix the enum items to match what they represent
> > > ---
> > >   .../display/bridge/toshiba,tc358767.yaml       | 18 ++++++++++++++++++
> > >   1 file changed, 18 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.yaml b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.yaml
> > > index 2ad0cd6dd49e0..6287eb2b40908 100644
> > > --- a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.yaml
> > > +++ b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.yaml
> > > @@ -98,6 +98,24 @@ properties:
> > >               reference to a valid eDP panel input endpoint node. This port is
> > >               optional, treated as DP panel if not defined
> > > +        properties:
> > > +          endpoint:
> > > +            $ref: /schemas/media/video-interfaces.yaml#
> > > +            unevaluatedProperties: false
> > > +
> > > +            properties:
> > > +              toshiba,pre-emphasis:
> > > +                description:
> > > +                  Display port output Pre-Emphasis settings for both ports.
> > 
> > Why here and not in the port nodes?
> 
> There was a short discussion about that in V1:
> 
> https://lore.kernel.org/all/00e9ef90-3bbe-4556-8da9-462f65928781@denx.de/
> 
> "
> Let's keep it in the endpoint node.
> 
> There is some mention in the TC9595 datasheet that the DP might operate
> in some split mode, where each DP lane is used to feed one display (?),
> so I assume in that case there might be two endpoints (?), but that is
> not supported right now.
> 
> If that is ever needed, I guess this array would have minItems 1 and
> maxItems 2 and another endpoint would be added to the schema for this
> port 2.
> "

Can this be put in the commit message please?

> > > +                $ref: /schemas/types.yaml#/definitions/uint8-array
> > > +                minItems: 2
> > > +                maxItems: 2
> > > +                items:
> > > +                  enum:
> > > +                    - 0 # No pre-emphasis
> > > +                    - 1 # 3.5dB pre-emphasis
> > > +                    - 2 # 6dB pre-emphasis
> > 
> > I'd love to say please make this -bB and put this in units, but that'd
> > require it being a string..
> 
> I can do that. Do you think that's worth it ?

I dunno, I'd advocate for it for any other unit cos I would ask for the
unit to be changed into something that didn't require fractions. But for
decibels, that just going to be confusing given how it works. I think
for dB it's just not worth it.

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

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

* Re: [PATCH v2 1/2] dt-bindings: display: bridge: tc358867: Document default DP preemphasis
  2024-06-23 17:20     ` Conor Dooley
@ 2024-06-24  0:05       ` Marek Vasut
  0 siblings, 0 replies; 6+ messages in thread
From: Marek Vasut @ 2024-06-24  0:05 UTC (permalink / raw)
  To: Conor Dooley
  Cc: dri-devel, Andrzej Hajda, Conor Dooley, Daniel Vetter,
	David Airlie, Jernej Skrabec, Jonas Karlman, Krzysztof Kozlowski,
	Laurent Pinchart, Lucas Stach, Maarten Lankhorst, Maxime Ripard,
	Neil Armstrong, Rob Herring, Robert Foss, Thomas Zimmermann,
	devicetree, kernel

On 6/23/24 7:20 PM, Conor Dooley wrote:
> On Sun, Jun 23, 2024 at 04:48:47PM +0200, Marek Vasut wrote:
>> On 6/22/24 1:56 PM, Conor Dooley wrote:
>>> On Fri, Jun 21, 2024 at 05:53:53PM +0200, Marek Vasut wrote:
>>>> Document default DP port preemphasis configurable via new DT property
>>>> "toshiba,pre-emphasis". This is useful in case the DP link properties
>>>> are known and starting link training from preemphasis setting of 0 dB
>>>> is not useful. The preemphasis can be set separately for both DP lanes
>>>> in range 0=0dB, 1=3.5dB, 2=6dB .
>>>>
>>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>>> ---
>>>> V2: - Fix the type to u8 array
>>>>       - Fix the enum items to match what they represent
>>>> ---
>>>>    .../display/bridge/toshiba,tc358767.yaml       | 18 ++++++++++++++++++
>>>>    1 file changed, 18 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.yaml b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.yaml
>>>> index 2ad0cd6dd49e0..6287eb2b40908 100644
>>>> --- a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.yaml
>>>> +++ b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.yaml
>>>> @@ -98,6 +98,24 @@ properties:
>>>>                reference to a valid eDP panel input endpoint node. This port is
>>>>                optional, treated as DP panel if not defined
>>>> +        properties:
>>>> +          endpoint:
>>>> +            $ref: /schemas/media/video-interfaces.yaml#
>>>> +            unevaluatedProperties: false
>>>> +
>>>> +            properties:
>>>> +              toshiba,pre-emphasis:
>>>> +                description:
>>>> +                  Display port output Pre-Emphasis settings for both ports.
>>>
>>> Why here and not in the port nodes?
>>
>> There was a short discussion about that in V1:
>>
>> https://lore.kernel.org/all/00e9ef90-3bbe-4556-8da9-462f65928781@denx.de/
>>
>> "
>> Let's keep it in the endpoint node.
>>
>> There is some mention in the TC9595 datasheet that the DP might operate
>> in some split mode, where each DP lane is used to feed one display (?),
>> so I assume in that case there might be two endpoints (?), but that is
>> not supported right now.
>>
>> If that is ever needed, I guess this array would have minItems 1 and
>> maxItems 2 and another endpoint would be added to the schema for this
>> port 2.
>> "
> 
> Can this be put in the commit message please?

Will do.

>>>> +                $ref: /schemas/types.yaml#/definitions/uint8-array
>>>> +                minItems: 2
>>>> +                maxItems: 2
>>>> +                items:
>>>> +                  enum:
>>>> +                    - 0 # No pre-emphasis
>>>> +                    - 1 # 3.5dB pre-emphasis
>>>> +                    - 2 # 6dB pre-emphasis
>>>
>>> I'd love to say please make this -bB and put this in units, but that'd
>>> require it being a string..
>>
>> I can do that. Do you think that's worth it ?
> 
> I dunno, I'd advocate for it for any other unit cos I would ask for the
> unit to be changed into something that didn't require fractions. But for
> decibels, that just going to be confusing given how it works. I think
> for dB it's just not worth it.

All right then.

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

end of thread, other threads:[~2024-06-24  0:36 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-21 15:53 [PATCH v2 1/2] dt-bindings: display: bridge: tc358867: Document default DP preemphasis Marek Vasut
2024-06-21 15:53 ` [PATCH v2 2/2] drm/bridge: tc358767: Add configurable default preemphasis Marek Vasut
2024-06-22 11:56 ` [PATCH v2 1/2] dt-bindings: display: bridge: tc358867: Document default DP preemphasis Conor Dooley
2024-06-23 14:48   ` Marek Vasut
2024-06-23 17:20     ` Conor Dooley
2024-06-24  0:05       ` Marek Vasut

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).