| Message ID | 20241127103031.1007893-2-andrej.picej@norik.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | sn65dsi83: Add LVDS_VCOM option in device-tree | expand |
Hi Rob, On 27. 11. 24 16:16, Rob Herring wrote: > On Wed, Nov 27, 2024 at 11:30:29AM +0100, Andrej Picej wrote: >> From: Janine Hagemann <j.hagemann@phytec.de> >> >> Add an optional property to change LVDS output voltage. This depends on >> the connected display specifications. With this property we directly set >> the LVDS_VCOM (0x19) register. >> Better register property mapping would be quite tricky. Please check >> bridge's datasheet for details on how register values set the LVDS >> data lines and LVDS clock output voltage. >> >> Signed-off-by: Janine Hagemann <j.hagemann@phytec.de> >> Signed-off-by: Andrej Picej <andrej.picej@norik.com> >> --- >> .../bindings/display/bridge/ti,sn65dsi83.yaml | 14 +++++++++++++- >> 1 file changed, 13 insertions(+), 1 deletion(-) >> >> diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml >> index 48a97bb3e2e0..5b2c0c281824 100644 >> --- a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml >> +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml >> @@ -58,6 +58,12 @@ properties: >> - const: 2 >> - const: 3 >> - const: 4 >> + ti,lvds-vcom: >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + description: LVDS output voltage configuration. This defines >> + LVDS_VCOM (0x19) register value. Check bridge's datasheet for >> + details on how register values set the LVDS data lines and >> + LVDS clock output voltage. > > Constraints? 0 - 2^32 are all valid values? Not really, only first 6 bits, which also means that this can be uint8 then. Will fix with other issues. > >> >> port@1: >> $ref: /schemas/graph.yaml#/$defs/port-base >> @@ -78,6 +84,12 @@ properties: >> - const: 2 >> - const: 3 >> - const: 4 >> + ti,lvds-vcom: >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + description: LVDS output voltage configuration. This defines >> + LVDS_VCOM (0x19) register value. Check bridge's datasheet for >> + details on how register values set the LVDS data lines and >> + LVDS clock output voltage. > > Never good to just have 2 copies of the same thing. Move the whole port > schema to a $defs entry and add the property there. Then just $ref it: > > port@0: > description: Video port for MIPI DSI Channel-A input > $ref: '#/$defs/dsi-port' > > > $defs: > dsi-port: > $ref: /schemas/graph.yaml#/$defs/port-base > unevaluatedProperties: false > description: Video port for MIPI DSI inputs > > properties: > endpoint: > $ref: /schemas/media/video-interfaces.yaml# > unevaluatedProperties: false > > properties: > data-lanes: > description: array of physical DSI data lane indexes. > minItems: 1 > items: > - const: 1 > - const: 2 > - const: 3 > - const: 4 > Ok will do it like this + just noticed that we are adding this under MIPI DSI port, and not LVDS output port for which these property is meant for. Will move it there. Thanks. Best regards, Andrej
On Thu, Nov 28, 2024 at 09:46:33AM +0100, Andrej Picej wrote: > On 27. 11. 24 16:16, Rob Herring wrote: > > On Wed, Nov 27, 2024 at 11:30:29AM +0100, Andrej Picej wrote: > > > From: Janine Hagemann <j.hagemann@phytec.de> > > > > > > Add an optional property to change LVDS output voltage. This depends on > > > the connected display specifications. With this property we directly set > > > the LVDS_VCOM (0x19) register. > > > Better register property mapping would be quite tricky. Please check > > > bridge's datasheet for details on how register values set the LVDS > > > data lines and LVDS clock output voltage. > > > > > > Signed-off-by: Janine Hagemann <j.hagemann@phytec.de> > > > Signed-off-by: Andrej Picej <andrej.picej@norik.com> > > > --- > > > .../bindings/display/bridge/ti,sn65dsi83.yaml | 14 +++++++++++++- > > > 1 file changed, 13 insertions(+), 1 deletion(-) > > > > > > diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml > > > index 48a97bb3e2e0..5b2c0c281824 100644 > > > --- a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml > > > +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml > > > @@ -58,6 +58,12 @@ properties: > > > - const: 2 > > > - const: 3 > > > - const: 4 > > > + ti,lvds-vcom: > > > + $ref: /schemas/types.yaml#/definitions/uint32 > > > + description: LVDS output voltage configuration. This defines > > > + LVDS_VCOM (0x19) register value. Check bridge's datasheet for > > > + details on how register values set the LVDS data lines and > > > + LVDS clock output voltage. > > > > Constraints? 0 - 2^32 are all valid values? > > Not really, only first 6 bits, which also means that this can be uint8 then. > Will fix with other issues. Also, generally speaking directly using register values is really frowned upon, even more so when they match a value expressed in a standard unit. Maxime
Hi Maxime, On 28. 11. 24 11:29, Maxime Ripard wrote: > On Thu, Nov 28, 2024 at 09:46:33AM +0100, Andrej Picej wrote: >> On 27. 11. 24 16:16, Rob Herring wrote: >>> On Wed, Nov 27, 2024 at 11:30:29AM +0100, Andrej Picej wrote: >>>> From: Janine Hagemann <j.hagemann@phytec.de> >>>> >>>> Add an optional property to change LVDS output voltage. This depends on >>>> the connected display specifications. With this property we directly set >>>> the LVDS_VCOM (0x19) register. >>>> Better register property mapping would be quite tricky. Please check >>>> bridge's datasheet for details on how register values set the LVDS >>>> data lines and LVDS clock output voltage. >>>> >>>> Signed-off-by: Janine Hagemann <j.hagemann@phytec.de> >>>> Signed-off-by: Andrej Picej <andrej.picej@norik.com> >>>> --- >>>> .../bindings/display/bridge/ti,sn65dsi83.yaml | 14 +++++++++++++- >>>> 1 file changed, 13 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml >>>> index 48a97bb3e2e0..5b2c0c281824 100644 >>>> --- a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml >>>> +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml >>>> @@ -58,6 +58,12 @@ properties: >>>> - const: 2 >>>> - const: 3 >>>> - const: 4 >>>> + ti,lvds-vcom: >>>> + $ref: /schemas/types.yaml#/definitions/uint32 >>>> + description: LVDS output voltage configuration. This defines >>>> + LVDS_VCOM (0x19) register value. Check bridge's datasheet for >>>> + details on how register values set the LVDS data lines and >>>> + LVDS clock output voltage. >>> >>> Constraints? 0 - 2^32 are all valid values? >> >> Not really, only first 6 bits, which also means that this can be uint8 then. >> Will fix with other issues. > > Also, generally speaking directly using register values is really > frowned upon, even more so when they match a value expressed in a > standard unit. Yes, I am aware that this is not how devide-tree/device drivers should work. But setting this values based on wanted LVDS voltage will be quite tricky. Matching a value expressed in mV would be quite hard, take a look in the bridge datasheet [1], Chapter 6.5 Electrical Characteristics (|VOD|). Basically both: - LVDS data line output and - LVDS clock voltage is determined by the CSR 0x19.3:2. So when checking which Reg setting CSR 0x19 should be set to both conditions should meet specifications of the connected display. Output voltage for the same CSR 0x19 setting differs between LVDS data lines and LVDS clock. Anyway, I'll prepare a v2 which only sets a part of this register, a bitfield (2 bits) that is responsible for LVDS differential output voltage. [1] https://www.ti.com/lit/ds/symlink/sn65dsi83.pdf?ts=1732738773429&ref_url=https%253A%252F%252Fwww.mouser.co.uk%252F Best regards, Andrej > > Maxime
On Thu, Nov 28, 2024 at 11:57:16AM +0100, Andrej Picej wrote: > Hi Maxime, > > On 28. 11. 24 11:29, Maxime Ripard wrote: > > On Thu, Nov 28, 2024 at 09:46:33AM +0100, Andrej Picej wrote: > > > On 27. 11. 24 16:16, Rob Herring wrote: > > > > On Wed, Nov 27, 2024 at 11:30:29AM +0100, Andrej Picej wrote: > > > > > From: Janine Hagemann <j.hagemann@phytec.de> > > > > > > > > > > Add an optional property to change LVDS output voltage. This depends on > > > > > the connected display specifications. With this property we directly set > > > > > the LVDS_VCOM (0x19) register. > > > > > Better register property mapping would be quite tricky. Please check > > > > > bridge's datasheet for details on how register values set the LVDS > > > > > data lines and LVDS clock output voltage. > > > > > > > > > > Signed-off-by: Janine Hagemann <j.hagemann@phytec.de> > > > > > Signed-off-by: Andrej Picej <andrej.picej@norik.com> > > > > > --- > > > > > .../bindings/display/bridge/ti,sn65dsi83.yaml | 14 +++++++++++++- > > > > > 1 file changed, 13 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml > > > > > index 48a97bb3e2e0..5b2c0c281824 100644 > > > > > --- a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml > > > > > +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml > > > > > @@ -58,6 +58,12 @@ properties: > > > > > - const: 2 > > > > > - const: 3 > > > > > - const: 4 > > > > > + ti,lvds-vcom: > > > > > + $ref: /schemas/types.yaml#/definitions/uint32 > > > > > + description: LVDS output voltage configuration. This defines > > > > > + LVDS_VCOM (0x19) register value. Check bridge's datasheet for > > > > > + details on how register values set the LVDS data lines and > > > > > + LVDS clock output voltage. > > > > > > > > Constraints? 0 - 2^32 are all valid values? > > > > > > Not really, only first 6 bits, which also means that this can be uint8 then. > > > Will fix with other issues. > > > > Also, generally speaking directly using register values is really > > frowned upon, even more so when they match a value expressed in a > > standard unit. > > Yes, I am aware that this is not how devide-tree/device drivers should work. > But setting this values based on wanted LVDS voltage will be quite tricky. > Matching a value expressed in mV would be quite hard, take a look in the > bridge datasheet [1], Chapter 6.5 Electrical Characteristics (|VOD|). > Basically both: > - LVDS data line output and > - LVDS clock voltage > is determined by the CSR 0x19.3:2. So when checking which Reg setting CSR > 0x19 should be set to both conditions should meet specifications of the > connected display. Output voltage for the same CSR 0x19 setting differs > between LVDS data lines and LVDS clock. > > Anyway, I'll prepare a v2 which only sets a part of this register, a > bitfield (2 bits) that is responsible for LVDS differential output voltage. Please use mV to specify the value in device tree. Yes, it's not easy. Yes, it is still recommended for multiple reasons. > > [1] https://www.ti.com/lit/ds/symlink/sn65dsi83.pdf?ts=1732738773429&ref_url=https%253A%252F%252Fwww.mouser.co.uk%252F > > Best regards, > Andrej > > > > > Maxime
diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml index 48a97bb3e2e0..5b2c0c281824 100644 --- a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml @@ -58,6 +58,12 @@ properties: - const: 2 - const: 3 - const: 4 + ti,lvds-vcom: + $ref: /schemas/types.yaml#/definitions/uint32 + description: LVDS output voltage configuration. This defines + LVDS_VCOM (0x19) register value. Check bridge's datasheet for + details on how register values set the LVDS data lines and + LVDS clock output voltage. port@1: $ref: /schemas/graph.yaml#/$defs/port-base @@ -78,6 +84,12 @@ properties: - const: 2 - const: 3 - const: 4 + ti,lvds-vcom: + $ref: /schemas/types.yaml#/definitions/uint32 + description: LVDS output voltage configuration. This defines + LVDS_VCOM (0x19) register value. Check bridge's datasheet for + details on how register values set the LVDS data lines and + LVDS clock output voltage. port@2: $ref: /schemas/graph.yaml#/properties/port @@ -120,7 +132,7 @@ allOf: properties: port@1: false -additionalProperties: false +additionalProperties: true examples: - |