* [PATCH v5 0/3] drm/tidss: Add OLDI bridge support
@ 2025-02-09 16:09 Aradhya Bhatia
2025-02-09 16:09 ` [PATCH v5 1/3] dt-bindings: display: ti,am65x-dss: Re-indent the example Aradhya Bhatia
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Aradhya Bhatia @ 2025-02-09 16:09 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Tomi Valkeinen,
Jyri Sarha
Cc: Maarten Lankhorst, Thomas Zimmermann, Maxime Ripard, David Airlie,
Laurent Pinchart, Simona Vetter, Nishanth Menon,
Vignesh Raghavendra, Devarsh Thakkar, Praneeth Bajjuri,
Udit Kumar, Jayesh Choudhary, Francesco Dolcini,
DRI Development List, Devicetree List, Linux Kernel List,
Aradhya Bhatia
Hello all,
This patch series adds support for the dual OLDI TXes supported in Texas
Instruments' AM62x and AM62Px family of SoCs. The OLDI TX hardware supports
single-lvds, lvds-clone, and dual-lvds modes. These TXes have now been
represented through DRM bridges within TI-DSS.
* Some history and hardware description for this patch series *
This patch series is a complete re-vamp from the previously posted series[1] and
hence, the version index has been reset to v1. The OLDI support from that series
was dropped and only the base support for AM62x DSS was kept (and eventually
merged)[2].
The OLDI display that the tidss driver today supports, could not be extended for
the newer SoCs. The OLDI display in tidss is modelled after the DSS and OLDI
hardware in the AM65x SoC. The DSS in AM65x SoC, has two video-ports. Both these
video-ports (VP) output DPI video signals. One of the DPI output (from VP1) from
the DSS connects to a singular OLDI TX present inside the SoC. There is no other
way for the DPI from VP1 to be taken out of the SoC. The other DPI output
however - the one from VP2 - is taken out of the SoC as is. Hence we have an
OLDI bus output and a DPI bus output from the SoC. Since the VP1 and OLDI are
tightly coupled, the tidss driver considers them as a single entity. That is
why, any OLDI sink connects directly to the DSS ports in the OF graphs.
The newer SoCs have varying DSS and OLDI integrations.
The AM62x DSS also has 2 VPs. The 2nd VP, VP2, outputs DPI signals which are
taken out of the SoC - similar to the AM65x above. For the VP1, there are 2 OLDI
TXes. These OLDI TXes can only receive DPI signals from VP1, and don't connect
to VP2 at all.
The AM62Px SoC has 2 OLDI TXes like AM62x SoC. However, the AM62Px SoC also has
2 separate DSSes. The 2 OLDI TXes can now be shared between the 2 VPs of the 2
DSSes.
The addition of the 2nd OLDI TX (and a 2nd DSS in AM62Px) creates a need for
some major changes for a full feature experience.
1. The OF graph needs to be updated to accurately show the data flow.
2. The tidss and OLDI drivers now need to support the dual-link and the cloned
single-link OLDI video signals.
3. The drivers also need to support the case where 2 OLDI TXes are connected to
2 different VPs - thereby creating 2 independent streams of single-link OLDI
outputs.
Note that the OLDI does not have registers of its own. It is still dependent on
the parent VP. The VP that provides the DPI video signals to the OLDI TXes, also
gives the OLDI TXes all the config data. That is to say, the hardware doesn't
sit on the bus directly - but does so through the DSS.
In light of all of these hardware variations, it was decided to have a separate
OLDI driver (unlike AM65x) but not entirely separate so as to be a platform
device. The OLDI TXes are now being represented as DRM bridges under the tidss.
* Regarding the Dependency Patches *
Since the OLDI TXes have a hardware dependency with the parent VP(s), the OLDI
configuration needs to happen before that VP is enabled for streaming. VP stream
enable takes place in tidss_crtc_atomic_enable hook. I have posted patches[0]
allowing DRM bridges to get pre-enabled before the CRTC of that bridge is
enabled. Without those patches, some warnings or glitches may be observed.
* Regarding the Drop of Clone Mode support *
Another key point to note is that the support for clone mode has been dropped
from the tidss OLDI driver, from v5 onwards. If the DT is configured for a clone
mode, the driver will report an error and exit. This has been done because the
driver was not supporting a specific case of clone mode where 2 OLDI sink
bridges connected to the 2 OLDI TXes require active programming (unlike the
simple-panels which do not). The driver does not support creation of two
encoder-bridge pipelines (along with the parent tidss driver) to allow program
any subsequent bridges (OLDI sinks and bridges thereafter).
The code fragments that write the OLDI config to enable clone mode have been
kept as they are, for future, but the driver will not continue to probe if it
detects a clone mode configuration, for the time being.
This drop of clone mode support can be undone by applying this _soft-tested_
patch[6] on top of this series. This patch will revert the driver to previous
revisions and will allow OLDI sinks that don't require active programming (for
example: simple-panels) to work with the driver. Note that this isn't the ideal
way to run clone mode, but it just works for any bridge pipeline after OLDT TX
that does not require additional configuration after the OLDI (for example: a
couple of simple lvds panels connected directly to the OLDI TXes in clone mode).
These patches have been tested on AM625 based Beagleplay[3] platform with a
Lincolntech LCD185 dual-lvds panel. The patches with complete support including
the expected devicetree configuration of the OLDI TXes can be found in the
"next_oldi_v5_5-tests" branch of my github fork[4]. This branch also has support
for Microtips dual-lvds panel (SK-LCD1) which is compatible with the SK-AM625
EVM platform.
I'd appreciate it if somebody can test it, and report back if they observe any
issues.
Thanks,
Aradhya
* Important note about the authorship of patches *
All the patches in the of this series were authored when I owned a "ti.com"
based email id, i.e. <a-bhatia1@ti.com>. This email id is not in use anymore,
and all the work done later has been part of my personal work. Since the
original patches were authored using TI's email id, I have maintained the
original authorships as they are, as well as their sign offs.
I have further added another sign off that uses my current (and personal) email
id, the one that is being used to send this revision, i.e.
<aradhya.bhatia@linux.dev>.
---
Change Log:
V5:
- Implement fixes suggested by Rob Herring in patch-2.
* Drop the example from OLDI schema.
* Fix the DSS schema conditions.
- Drop the OLDI clone mode support from the driver as it was incomplete and
could not account for cases where OLDI TXes were connected to another pair
of bridges that would require additional programming, instead of a pair of
simple-panels which wouldn't.
V4:
- Implement fixes suggested by Krzysztof Kozlowski:
* Squash patches v3:2/4 and v3:3/4 to v4:2/3, and add more hardware details
in commit description.
* Change the serial clock name for OLDI, from "s_clk" to "serial".
* Fix the required condition in the OLDI schema.
* Other minor fixes.
- Change "oldi-txes" OLDI DT node name to "oldi-transmitters".
- Update secondary-OLDI property requirements to be more relaxing for AM62P
DSS configuration.
V3:
- Fix the dt_binding_check warning in patch 3/4[5] by adding
"additionalProperties" constraint.
V2:
- Add all the R-b and A-b tags from Laurent Pinchart, Rob Herring, and
Tomi Valkeinen.
- Reword the subject for patch 1/4.
- Reword the commit descriptions to add proper hardware detail.
- Drop the change in schema reference for port@0 in patch 3/4.
- Lots of improvements for patch 4/4.
* Refactor OLDI selection logic in tidss_oldi_tx_power().
* Add "companion_instance" support to identify the OLDI index in
dual-link or cloned sinle-link modes.
* De-initialize tidss_oldi during tidss removal.
* Use dev_err_probe() instead of dev_err().
* Drop OLDI(n) macro.
* Move OLDI Config register bits to tidss_dispc_regs.h.
* Drop oldi bridge atomic_check().
* s/%d/%u for all print instances of "oldi_instance".
* Move OLDI init after DISPC init in tidss_probe.
* Use devm_drm_of_get_bridge() instead of
drm_of_find_panel_or_bridge() to find the next bridge and drop all
the drm_panel support from tidss_oldi.
Previous revisions:
V1: https://lore.kernel.org/all/20240511193055.1686149-1-a-bhatia1@ti.com/
V2: https://lore.kernel.org/all/20240715200953.1213284-1-a-bhatia1@ti.com/
V3: https://lore.kernel.org/all/20240716084248.1393666-1-a-bhatia1@ti.com/
V4: https://lore.kernel.org/all/20241124143649.686995-1-aradhya.bhatia@linux.dev/
[0]: Dependency Patches:
("drm/atomic-helper: Refactor crtc & encoder-bridge op loops into separate functions")
https://lore.kernel.org/all/20250209121621.34677-3-aradhya.bhatia@linux.dev/
("drm/atomic-helper: Separate out bridge pre_enable/post_disable from enable/disable)
https://lore.kernel.org/all/20250209121621.34677-4-aradhya.bhatia@linux.dev/
("drm/atomic-helper: Re-order bridge chain pre-enable and post-disable")
https://lore.kernel.org/all/20250209121621.34677-5-aradhya.bhatia@linux.dev/
[1]: AM62 OLDI Series - v7
https://lore.kernel.org/all/20230125113529.13952-1-a-bhatia1@ti.com/
[2]: AM62 DSS Series - v9
https://lore.kernel.org/all/20230616150900.6617-1-a-bhatia1@ti.com/
[3]: TI AM625 SoC based Beagleplay platform
https://www.beagleboard.org/boards/beagleplay
[4]: GitHub Fork for OLDI tests
https://github.com/aradhya07/linux-ab/tree/next_oldi_v5_5-tests
[5]: ("ti,am65x-dss.yaml: oldi-txes: Missing additionalProperties/
unevaluatedProperties constraint")
https://lore.kernel.org/all/172107979988.1595945.9666141982402158422.robh@kernel.org/
[6]: Undo drop of OLDI clone mode support
https://gist.github.com/aradhya07/afb55761e64eb440d47d5de3ea2a3cb7
Aradhya Bhatia (3):
dt-bindings: display: ti,am65x-dss: Re-indent the example
dt-bindings: display: ti: Add schema for AM625 OLDI Transmitter
drm/tidss: Add OLDI bridge support
.../bindings/display/ti/ti,am625-oldi.yaml | 88 +++
.../bindings/display/ti/ti,am65x-dss.yaml | 196 +++++-
MAINTAINERS | 1 +
drivers/gpu/drm/tidss/Makefile | 3 +-
drivers/gpu/drm/tidss/tidss_dispc.c | 20 +-
drivers/gpu/drm/tidss/tidss_dispc.h | 4 +
drivers/gpu/drm/tidss/tidss_dispc_regs.h | 14 +
drivers/gpu/drm/tidss/tidss_drv.c | 9 +
drivers/gpu/drm/tidss/tidss_drv.h | 5 +
drivers/gpu/drm/tidss/tidss_oldi.c | 558 ++++++++++++++++++
drivers/gpu/drm/tidss/tidss_oldi.h | 51 ++
11 files changed, 926 insertions(+), 23 deletions(-)
create mode 100644 Documentation/devicetree/bindings/display/ti/ti,am625-oldi.yaml
create mode 100644 drivers/gpu/drm/tidss/tidss_oldi.c
create mode 100644 drivers/gpu/drm/tidss/tidss_oldi.h
base-commit: ffd294d346d185b70e28b1a28abe367bbfe53c04
--
2.34.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v5 1/3] dt-bindings: display: ti,am65x-dss: Re-indent the example
2025-02-09 16:09 [PATCH v5 0/3] drm/tidss: Add OLDI bridge support Aradhya Bhatia
@ 2025-02-09 16:09 ` Aradhya Bhatia
2025-02-09 16:09 ` [PATCH v5 2/3] dt-bindings: display: ti: Add schema for AM625 OLDI Transmitter Aradhya Bhatia
2025-02-09 16:09 ` [PATCH v5 3/3] drm/tidss: Add OLDI bridge support Aradhya Bhatia
2 siblings, 0 replies; 12+ messages in thread
From: Aradhya Bhatia @ 2025-02-09 16:09 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Tomi Valkeinen,
Jyri Sarha
Cc: Maarten Lankhorst, Thomas Zimmermann, Maxime Ripard, David Airlie,
Laurent Pinchart, Simona Vetter, Nishanth Menon,
Vignesh Raghavendra, Devarsh Thakkar, Praneeth Bajjuri,
Udit Kumar, Jayesh Choudhary, Francesco Dolcini,
DRI Development List, Devicetree List, Linux Kernel List,
Aradhya Bhatia
From: Aradhya Bhatia <a-bhatia1@ti.com>
Reduce tab size from 8 spaces to 4 spaces to make the bindings
consistent, and easy to expand.
Acked-by: Rob Herring (Arm) <robh@kernel.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
Signed-off-by: Aradhya Bhatia <aradhya.bhatia@linux.dev>
---
.../bindings/display/ti/ti,am65x-dss.yaml | 54 +++++++++----------
1 file changed, 27 insertions(+), 27 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml b/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
index 55e3e490d0e6..399d68986326 100644
--- a/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
+++ b/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
@@ -142,32 +142,32 @@ examples:
#include <dt-bindings/soc/ti,sci_pm_domain.h>
dss: dss@4a00000 {
- compatible = "ti,am65x-dss";
- reg = <0x04a00000 0x1000>, /* common */
- <0x04a02000 0x1000>, /* vidl1 */
- <0x04a06000 0x1000>, /* vid */
- <0x04a07000 0x1000>, /* ovr1 */
- <0x04a08000 0x1000>, /* ovr2 */
- <0x04a0a000 0x1000>, /* vp1 */
- <0x04a0b000 0x1000>, /* vp2 */
- <0x04a01000 0x1000>; /* common1 */
- reg-names = "common", "vidl1", "vid",
- "ovr1", "ovr2", "vp1", "vp2", "common1";
- ti,am65x-oldi-io-ctrl = <&dss_oldi_io_ctrl>;
- power-domains = <&k3_pds 67 TI_SCI_PD_EXCLUSIVE>;
- clocks = <&k3_clks 67 1>,
- <&k3_clks 216 1>,
- <&k3_clks 67 2>;
- clock-names = "fck", "vp1", "vp2";
- interrupts = <GIC_SPI 166 IRQ_TYPE_EDGE_RISING>;
- ports {
- #address-cells = <1>;
- #size-cells = <0>;
- port@0 {
- reg = <0>;
- oldi_out0: endpoint {
- remote-endpoint = <&lcd_in0>;
- };
- };
+ compatible = "ti,am65x-dss";
+ reg = <0x04a00000 0x1000>, /* common */
+ <0x04a02000 0x1000>, /* vidl1 */
+ <0x04a06000 0x1000>, /* vid */
+ <0x04a07000 0x1000>, /* ovr1 */
+ <0x04a08000 0x1000>, /* ovr2 */
+ <0x04a0a000 0x1000>, /* vp1 */
+ <0x04a0b000 0x1000>, /* vp2 */
+ <0x04a01000 0x1000>; /* common1 */
+ reg-names = "common", "vidl1", "vid",
+ "ovr1", "ovr2", "vp1", "vp2", "common1";
+ ti,am65x-oldi-io-ctrl = <&dss_oldi_io_ctrl>;
+ power-domains = <&k3_pds 67 TI_SCI_PD_EXCLUSIVE>;
+ clocks = <&k3_clks 67 1>,
+ <&k3_clks 216 1>,
+ <&k3_clks 67 2>;
+ clock-names = "fck", "vp1", "vp2";
+ interrupts = <GIC_SPI 166 IRQ_TYPE_EDGE_RISING>;
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ port@0 {
+ reg = <0>;
+ oldi_out0: endpoint {
+ remote-endpoint = <&lcd_in0>;
+ };
};
+ };
};
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v5 2/3] dt-bindings: display: ti: Add schema for AM625 OLDI Transmitter
2025-02-09 16:09 [PATCH v5 0/3] drm/tidss: Add OLDI bridge support Aradhya Bhatia
2025-02-09 16:09 ` [PATCH v5 1/3] dt-bindings: display: ti,am65x-dss: Re-indent the example Aradhya Bhatia
@ 2025-02-09 16:09 ` Aradhya Bhatia
2025-02-11 12:24 ` Tomi Valkeinen
2025-02-19 14:22 ` Rob Herring (Arm)
2025-02-09 16:09 ` [PATCH v5 3/3] drm/tidss: Add OLDI bridge support Aradhya Bhatia
2 siblings, 2 replies; 12+ messages in thread
From: Aradhya Bhatia @ 2025-02-09 16:09 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Tomi Valkeinen,
Jyri Sarha
Cc: Maarten Lankhorst, Thomas Zimmermann, Maxime Ripard, David Airlie,
Laurent Pinchart, Simona Vetter, Nishanth Menon,
Vignesh Raghavendra, Devarsh Thakkar, Praneeth Bajjuri,
Udit Kumar, Jayesh Choudhary, Francesco Dolcini,
DRI Development List, Devicetree List, Linux Kernel List,
Aradhya Bhatia
From: Aradhya Bhatia <a-bhatia1@ti.com>
The OLDI transmitters (TXes) do not have registers of their own, and are
dependent on the source video-ports (VPs) from the DSS to provide
configuration data. This hardware doesn't directly sit on the internal
bus of the SoC, but does so via the DSS. Hence, the OLDI TXes are
supposed to be child nodes under the DSS, and not independent devices.
Two of the OLDI TXes can function in tandem to output dual-link OLDI
output, or cloned single-link outputs. In these cases, one OLDI will be
the primary OLDI, and the other one, a companion. The following diagram
represents such a configuration.
+-----+-----+ +-------+
| | | | |
| | VP1 +----+--->+ OLDI0 | (Primary - may need companion)
| | | | | |
| DSS +-----+ | +-------+
| | | |
| | VP2 | | +-------+
| | | | | |
+-----+-----+ +--->+ OLDI1 | (Companion OLDI)
| |
+-------+
The DSS in AM625 SoC has a configuration like the one above. The AM625
DSS VP1 (port@0) can connect and control 2 OLDI TXes, to use them in
dual-link or cloned single-link OLDI modes. It is only the VP1 that can
connect to either OLDI TXes for the AM625 DSS, and not the VP2.
Alternatively, on some future TI SoCs, along with the above
configuration, the OLDI TX can _also_ connect to separate video sources,
making them work entirely independent of each other. In this case,
neither of the OLDIs are "companion" or "secondary" OLDIs, and nor do
they require one. They both are independent and primary OLDIs. The
following diagram represents such a configuration.
+-----+-----+ +-------+
| | | | |
| | VP1 +--+----------->+ OLDI0 | (Primary - may need companion)
| | | | | |
| +-----+ | +-------+
| | | |
| | VP2 | |
| | | |
| DSS +-----+ | +---+ +-------+
| | | +-->+ M | | |
| | VP3 +----->+ U +--->+ OLDI1 | (Companion or Primary)
| | | | X | | |
| +-----+ +---+ +-------+
| | |
| | VP4 |
| | |
+-----+-----+
Note that depending on the mux configuration, the OLDIs can either be
working together in tandem - sourced by VP1, OR, they could be working
independently sourced by VP1 and VP3 respectively.
The idea is to support all the configurations with this OLDI TX schema.
The OLDI functionality is further supported by a system-control module,
which contains a few registers to control OLDI IO power and other
electrical characteristics of the IO lanes.
Add devicetree binding schema for the OLDI TXes to support various
configurations, and extend their support to the AM625 DSS.
Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
Signed-off-by: Aradhya Bhatia <aradhya.bhatia@linux.dev>
---
.../bindings/display/ti/ti,am625-oldi.yaml | 88 ++++++++++
.../bindings/display/ti/ti,am65x-dss.yaml | 154 ++++++++++++++++++
MAINTAINERS | 1 +
3 files changed, 243 insertions(+)
create mode 100644 Documentation/devicetree/bindings/display/ti/ti,am625-oldi.yaml
diff --git a/Documentation/devicetree/bindings/display/ti/ti,am625-oldi.yaml b/Documentation/devicetree/bindings/display/ti/ti,am625-oldi.yaml
new file mode 100644
index 000000000000..42a80a512660
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/ti/ti,am625-oldi.yaml
@@ -0,0 +1,88 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/ti/ti,am625-oldi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Texas Instruments AM625 OLDI Transmitter
+
+maintainers:
+ - Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
+ - Aradhya Bhatia <aradhya.bhatia@linux.dev>
+
+description:
+ The AM625 TI Keystone OpenLDI transmitter (OLDI TX) supports serialized RGB
+ pixel data transmission between host and flat panel display over LVDS (Low
+ Voltage Differential Sampling) interface. The OLDI TX consists of 7-to-1 data
+ serializers, and 4-data and 1-clock LVDS outputs. It supports the LVDS output
+ formats "jeida-18", "jeida-24" and "vesa-18", and can accept 24-bit RGB or
+ padded and un-padded 18-bit RGB bus formats as input.
+
+properties:
+ reg:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+ description: serial clock input for the OLDI transmitters
+
+ clock-names:
+ const: serial
+
+ ti,companion-oldi:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description:
+ phandle to companion OLDI transmitter. This property is mandatory for the
+ primarty OLDI TX if the OLDI TXes are expected to work either in dual-lvds
+ mode or in clone mode. This property should point to the secondary OLDI
+ TX.
+
+ ti,secondary-oldi:
+ type: boolean
+ description:
+ Boolean property to mark the OLDI transmitter as the secondary one, when the
+ OLDI hardware is expected to run as a companion HW, in cases of dual-lvds
+ mode or clone mode. The primary OLDI hardware is responsible for all the
+ hardware configuration.
+
+ ti,oldi-io-ctrl:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description:
+ phandle to syscon device node mapping OLDI IO_CTRL registers found in the
+ control MMR region. These registers are required to toggle the I/O lane
+ power, and control its electrical characteristics.
+
+ ports:
+ $ref: /schemas/graph.yaml#/properties/ports
+
+ properties:
+ port@0:
+ $ref: /schemas/graph.yaml#/properties/port
+ description: Parallel RGB input port
+
+ port@1:
+ $ref: /schemas/graph.yaml#/properties/port
+ description: LVDS output port
+
+ required:
+ - port@0
+ - port@1
+
+allOf:
+ - if:
+ required:
+ - ti,secondary-oldi
+ then:
+ properties:
+ ti,companion-oldi: false
+
+required:
+ - reg
+ - clocks
+ - clock-names
+ - ti,oldi-io-ctrl
+ - ports
+
+additionalProperties: false
+
+...
diff --git a/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml b/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
index 399d68986326..a82c525631ea 100644
--- a/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
+++ b/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
@@ -91,6 +91,24 @@ properties:
For AM625 DSS, the internal DPI output port node from video
port 1.
For AM62A7 DSS, the port is tied off inside the SoC.
+ properties:
+ endpoint@0:
+ $ref: /schemas/graph.yaml#/properties/endpoint
+ description:
+ For AM625 DSS, VP Connection to OLDI0.
+ For AM65X DSS, OLDI output from the SoC.
+
+ endpoint@1:
+ $ref: /schemas/graph.yaml#/properties/endpoint
+ description:
+ For AM625 DSS, VP Connection to OLDI1.
+
+ anyOf:
+ - required:
+ - endpoint
+ - required:
+ - endpoint@0
+ - endpoint@1
port@1:
$ref: /schemas/graph.yaml#/properties/port
@@ -112,6 +130,25 @@ properties:
Input memory (from main memory to dispc) bandwidth limit in
bytes per second
+ oldi-transmitters:
+ description:
+ Child node under the DSS, to describe all the OLDI transmitters connected
+ to the DSS videoports.
+ type: object
+ additionalProperties: false
+
+ properties:
+ "#address-cells":
+ const: 1
+
+ "#size-cells":
+ const: 0
+
+ patternProperties:
+ '^oldi@[0-1]$':
+ $ref: ti,am625-oldi.yaml#
+ description: OLDI transmitters connected to the DSS VPs
+
allOf:
- if:
properties:
@@ -120,10 +157,25 @@ allOf:
const: ti,am62a7-dss
then:
properties:
+ oldi-transmitters: false
ports:
properties:
port@0: false
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: ti,am65x-dss
+ then:
+ properties:
+ oldi-transmitters: false
+ ports:
+ properties:
+ port@0:
+ properties:
+ endpoint@1: false
+
required:
- compatible
- reg
@@ -171,3 +223,105 @@ examples:
};
};
};
+
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/soc/ti,sci_pm_domain.h>
+
+ bus {
+ #address-cells = <2>;
+ #size-cells = <2>;
+ dss1: dss@30200000 {
+ compatible = "ti,am625-dss";
+ reg = <0x00 0x30200000 0x00 0x1000>, /* common */
+ <0x00 0x30202000 0x00 0x1000>, /* vidl1 */
+ <0x00 0x30206000 0x00 0x1000>, /* vid */
+ <0x00 0x30207000 0x00 0x1000>, /* ovr1 */
+ <0x00 0x30208000 0x00 0x1000>, /* ovr2 */
+ <0x00 0x3020a000 0x00 0x1000>, /* vp1 */
+ <0x00 0x3020b000 0x00 0x1000>, /* vp2 */
+ <0x00 0x30201000 0x00 0x1000>; /* common1 */
+ reg-names = "common", "vidl1", "vid",
+ "ovr1", "ovr2", "vp1", "vp2", "common1";
+ power-domains = <&k3_pds 186 TI_SCI_PD_EXCLUSIVE>;
+ clocks = <&k3_clks 186 6>,
+ <&vp1_clock>,
+ <&k3_clks 186 2>;
+ clock-names = "fck", "vp1", "vp2";
+ interrupts = <GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH>;
+ oldi-transmitters {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ oldi0: oldi@0 {
+ reg = <0>;
+ clocks = <&k3_clks 186 0>;
+ clock-names = "serial";
+ ti,companion-oldi = <&oldi1>;
+ ti,oldi-io-ctrl = <&dss_oldi_io_ctrl>;
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ port@0 {
+ reg = <0>;
+ oldi0_in: endpoint {
+ remote-endpoint = <&dpi0_out0>;
+ };
+ };
+ port@1 {
+ reg = <1>;
+ oldi0_out: endpoint {
+ remote-endpoint = <&panel_in0>;
+ };
+ };
+ };
+ };
+ oldi1: oldi@1 {
+ reg = <1>;
+ clocks = <&k3_clks 186 0>;
+ clock-names = "serial";
+ ti,secondary-oldi;
+ ti,oldi-io-ctrl = <&dss_oldi_io_ctrl>;
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ port@0 {
+ reg = <0>;
+ oldi1_in: endpoint {
+ remote-endpoint = <&dpi0_out1>;
+ };
+ };
+ port@1 {
+ reg = <1>;
+ oldi1_out: endpoint {
+ remote-endpoint = <&panel_in1>;
+ };
+ };
+ };
+ };
+ };
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ port@0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0>;
+ dpi0_out0: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&oldi0_in>;
+ };
+ dpi0_out1: endpoint@1 {
+ reg = <1>;
+ remote-endpoint = <&oldi1_in>;
+ };
+ };
+ port@1 {
+ reg = <1>;
+ dpi1_out: endpoint {
+ remote-endpoint = <&hdmi_bridge>;
+ };
+ };
+ };
+ };
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index 0fa7c5728f1e..88fa2d9435b8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7816,6 +7816,7 @@ M: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
L: dri-devel@lists.freedesktop.org
S: Maintained
T: git https://gitlab.freedesktop.org/drm/misc/kernel.git
+F: Documentation/devicetree/bindings/display/ti/ti,am625-oldi.yaml
F: Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
F: Documentation/devicetree/bindings/display/ti/ti,j721e-dss.yaml
F: Documentation/devicetree/bindings/display/ti/ti,k2g-dss.yaml
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v5 3/3] drm/tidss: Add OLDI bridge support
2025-02-09 16:09 [PATCH v5 0/3] drm/tidss: Add OLDI bridge support Aradhya Bhatia
2025-02-09 16:09 ` [PATCH v5 1/3] dt-bindings: display: ti,am65x-dss: Re-indent the example Aradhya Bhatia
2025-02-09 16:09 ` [PATCH v5 2/3] dt-bindings: display: ti: Add schema for AM625 OLDI Transmitter Aradhya Bhatia
@ 2025-02-09 16:09 ` Aradhya Bhatia
2025-02-11 10:57 ` Tomi Valkeinen
2 siblings, 1 reply; 12+ messages in thread
From: Aradhya Bhatia @ 2025-02-09 16:09 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Tomi Valkeinen,
Jyri Sarha
Cc: Maarten Lankhorst, Thomas Zimmermann, Maxime Ripard, David Airlie,
Laurent Pinchart, Simona Vetter, Nishanth Menon,
Vignesh Raghavendra, Devarsh Thakkar, Praneeth Bajjuri,
Udit Kumar, Jayesh Choudhary, Francesco Dolcini,
DRI Development List, Devicetree List, Linux Kernel List,
Aradhya Bhatia
From: Aradhya Bhatia <a-bhatia1@ti.com>
The AM62x and AM62Px SoCs feature 2 OLDI TXes each, which makes it
possible to connect them in dual-link or cloned single-link OLDI display
modes. The current OLDI support in tidss_dispc.c can only support for
a single OLDI TX, connected to a VP and doesn't really support
configuration of OLDIs in the other modes. The current OLDI support in
tidss_dispc.c also works on the principle that the OLDI output can only
be served by one, and only one, DSS video-port. This isn't the case in
the AM62Px SoC, where there are 2 DSS controllers present that share the
OLDI TXes.
Having their own devicetree and their own bridge entity will help
support the various display modes and sharing possiblilities of the OLDI
hardware.
For all these reasons, add support for the OLDI TXes as DRM bridges.
Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
Signed-off-by: Aradhya Bhatia <aradhya.bhatia@linux.dev>
---
drivers/gpu/drm/tidss/Makefile | 3 +-
drivers/gpu/drm/tidss/tidss_dispc.c | 20 +-
drivers/gpu/drm/tidss/tidss_dispc.h | 4 +
drivers/gpu/drm/tidss/tidss_dispc_regs.h | 14 +
drivers/gpu/drm/tidss/tidss_drv.c | 9 +
drivers/gpu/drm/tidss/tidss_drv.h | 5 +
drivers/gpu/drm/tidss/tidss_oldi.c | 558 +++++++++++++++++++++++
drivers/gpu/drm/tidss/tidss_oldi.h | 51 +++
8 files changed, 662 insertions(+), 2 deletions(-)
create mode 100644 drivers/gpu/drm/tidss/tidss_oldi.c
create mode 100644 drivers/gpu/drm/tidss/tidss_oldi.h
diff --git a/drivers/gpu/drm/tidss/Makefile b/drivers/gpu/drm/tidss/Makefile
index 312645271014..b6d6becf1683 100644
--- a/drivers/gpu/drm/tidss/Makefile
+++ b/drivers/gpu/drm/tidss/Makefile
@@ -7,6 +7,7 @@ tidss-y := tidss_crtc.o \
tidss_irq.o \
tidss_plane.o \
tidss_scale_coefs.o \
- tidss_dispc.o
+ tidss_dispc.o \
+ tidss_oldi.o
obj-$(CONFIG_DRM_TIDSS) += tidss.o
diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c b/drivers/gpu/drm/tidss/tidss_dispc.c
index 1ad711f8d2a8..8631a89e6155 100644
--- a/drivers/gpu/drm/tidss/tidss_dispc.c
+++ b/drivers/gpu/drm/tidss/tidss_dispc.c
@@ -466,6 +466,25 @@ static u32 dispc_vp_read(struct dispc_device *dispc, u32 hw_videoport, u16 reg)
return ioread32(base + reg);
}
+void tidss_configure_oldi(struct tidss_device *tidss, u32 hw_videoport,
+ u32 oldi_cfg)
+{
+ u32 count = 0;
+ u32 oldi_reset_bit = BIT(5 + hw_videoport);
+
+ dispc_vp_write(tidss->dispc, hw_videoport, DISPC_VP_DSS_OLDI_CFG, oldi_cfg);
+
+ if (oldi_cfg != 0) {
+ while (!(oldi_reset_bit & dispc_read(tidss->dispc, DSS_SYSSTATUS)) &&
+ count < 10000)
+ count++;
+
+ if (!(oldi_reset_bit & dispc_read(tidss->dispc, DSS_SYSSTATUS)))
+ dev_warn(tidss->dispc->dev, "%s: timeout waiting OLDI reset done\n",
+ __func__);
+ }
+}
+
/*
* TRM gives bitfields as start:end, where start is the higher bit
* number. For example 7:0
@@ -1310,7 +1329,6 @@ void dispc_vp_disable_clk(struct dispc_device *dispc, u32 hw_videoport)
* Calculate the percentage difference between the requested pixel clock rate
* and the effective rate resulting from calculating the clock divider value.
*/
-static
unsigned int dispc_pclk_diff(unsigned long rate, unsigned long real_rate)
{
int r = rate / 100, rr = real_rate / 100;
diff --git a/drivers/gpu/drm/tidss/tidss_dispc.h b/drivers/gpu/drm/tidss/tidss_dispc.h
index 086327d51a90..fab248f2055a 100644
--- a/drivers/gpu/drm/tidss/tidss_dispc.h
+++ b/drivers/gpu/drm/tidss/tidss_dispc.h
@@ -94,6 +94,10 @@ extern const struct dispc_features dispc_am62a7_feats;
extern const struct dispc_features dispc_am65x_feats;
extern const struct dispc_features dispc_j721e_feats;
+void tidss_configure_oldi(struct tidss_device *tidss, u32 hw_videoport,
+ u32 oldi_cfg);
+unsigned int dispc_pclk_diff(unsigned long rate, unsigned long real_rate);
+
void dispc_set_irqenable(struct dispc_device *dispc, dispc_irq_t mask);
dispc_irq_t dispc_read_and_clear_irqstatus(struct dispc_device *dispc);
diff --git a/drivers/gpu/drm/tidss/tidss_dispc_regs.h b/drivers/gpu/drm/tidss/tidss_dispc_regs.h
index 13feedfe5d6d..03f7098029e6 100644
--- a/drivers/gpu/drm/tidss/tidss_dispc_regs.h
+++ b/drivers/gpu/drm/tidss/tidss_dispc_regs.h
@@ -226,6 +226,20 @@ enum dispc_common_regs {
#define DISPC_VP_DSS_DMA_THREADSIZE 0x170 /* J721E */
#define DISPC_VP_DSS_DMA_THREADSIZE_STATUS 0x174 /* J721E */
+/* OLDI Config Bits (DISPC_VP_DSS_OLDI_CFG) */
+#define OLDI_ENABLE BIT(0)
+#define OLDI_MAP (BIT(1) | BIT(2) | BIT(3))
+#define OLDI_SRC BIT(4)
+#define OLDI_CLONE_MODE BIT(5)
+#define OLDI_MASTERSLAVE BIT(6)
+#define OLDI_DEPOL BIT(7)
+#define OLDI_MSB BIT(8)
+#define OLDI_LBEN BIT(9)
+#define OLDI_LBDATA BIT(10)
+#define OLDI_DUALMODESYNC BIT(11)
+#define OLDI_SOFTRST BIT(12)
+#define OLDI_TPATCFG BIT(13)
+
/*
* OLDI IO_CTRL register offsets. On AM654 the registers are found
* from CTRL_MMR0, there the syscon regmap should map 0x14 bytes from
diff --git a/drivers/gpu/drm/tidss/tidss_drv.c b/drivers/gpu/drm/tidss/tidss_drv.c
index 7c8fd6407d82..27b9f86f1eb2 100644
--- a/drivers/gpu/drm/tidss/tidss_drv.c
+++ b/drivers/gpu/drm/tidss/tidss_drv.c
@@ -24,6 +24,7 @@
#include "tidss_drv.h"
#include "tidss_kms.h"
#include "tidss_irq.h"
+#include "tidss_oldi.h"
/* Power management */
@@ -148,6 +149,10 @@ static int tidss_probe(struct platform_device *pdev)
return ret;
}
+ ret = tidss_oldi_init(tidss);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to init OLDI\n");
+
pm_runtime_enable(dev);
pm_runtime_set_autosuspend_delay(dev, 1000);
@@ -204,6 +209,8 @@ static int tidss_probe(struct platform_device *pdev)
pm_runtime_dont_use_autosuspend(dev);
pm_runtime_disable(dev);
+ tidss_oldi_deinit(tidss);
+
return ret;
}
@@ -228,6 +235,8 @@ static void tidss_remove(struct platform_device *pdev)
pm_runtime_dont_use_autosuspend(dev);
pm_runtime_disable(dev);
+ tidss_oldi_deinit(tidss);
+
/* devm allocated dispc goes away with the dev so mark it NULL */
dispc_remove(tidss);
diff --git a/drivers/gpu/drm/tidss/tidss_drv.h b/drivers/gpu/drm/tidss/tidss_drv.h
index d7f27b0b0315..6c0fe1d989ee 100644
--- a/drivers/gpu/drm/tidss/tidss_drv.h
+++ b/drivers/gpu/drm/tidss/tidss_drv.h
@@ -11,8 +11,10 @@
#define TIDSS_MAX_PORTS 4
#define TIDSS_MAX_PLANES 4
+#define TIDSS_MAX_OLDI_TXES 2
typedef u32 dispc_irq_t;
+struct tidss_oldi;
struct tidss_device {
struct drm_device ddev; /* DRM device for DSS */
@@ -27,6 +29,9 @@ struct tidss_device {
unsigned int num_planes;
struct drm_plane *planes[TIDSS_MAX_PLANES];
+ unsigned int num_oldis;
+ struct tidss_oldi *oldis[TIDSS_MAX_OLDI_TXES];
+
unsigned int irq;
spinlock_t wait_lock; /* protects the irq masks */
diff --git a/drivers/gpu/drm/tidss/tidss_oldi.c b/drivers/gpu/drm/tidss/tidss_oldi.c
new file mode 100644
index 000000000000..4af13a01f546
--- /dev/null
+++ b/drivers/gpu/drm/tidss/tidss_oldi.c
@@ -0,0 +1,558 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2024 - Texas Instruments Incorporated
+ *
+ * Aradhya Bhatia <a-bhatia1@ti.com>
+ */
+
+#include <linux/clk.h>
+#include <linux/of.h>
+#include <linux/of_graph.h>
+#include <linux/mfd/syscon.h>
+#include <linux/media-bus-format.h>
+#include <linux/regmap.h>
+
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_bridge.h>
+#include <drm/drm_of.h>
+
+#include "tidss_dispc.h"
+#include "tidss_dispc_regs.h"
+#include "tidss_oldi.h"
+
+struct tidss_oldi {
+ struct tidss_device *tidss;
+ struct device *dev;
+
+ struct drm_bridge bridge;
+ struct drm_bridge *next_bridge;
+
+ enum tidss_oldi_link_type link_type;
+ const struct oldi_bus_format *bus_format;
+ u32 oldi_instance;
+ u32 companion_instance;
+ u32 parent_vp;
+
+ struct clk *serial;
+ struct regmap *io_ctrl;
+};
+
+static const struct oldi_bus_format oldi_bus_formats[] = {
+ { MEDIA_BUS_FMT_RGB666_1X7X3_SPWG, 18, SPWG_18, MEDIA_BUS_FMT_RGB666_1X18 },
+ { MEDIA_BUS_FMT_RGB888_1X7X4_SPWG, 24, SPWG_24, MEDIA_BUS_FMT_RGB888_1X24 },
+ { MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA, 24, JEIDA_24, MEDIA_BUS_FMT_RGB888_1X24 },
+};
+
+#define OLDI_IDLE_CLK_HZ 25000000 /*25 MHz */
+
+static inline struct tidss_oldi *
+drm_bridge_to_tidss_oldi(struct drm_bridge *bridge)
+{
+ return container_of(bridge, struct tidss_oldi, bridge);
+}
+
+static int tidss_oldi_bridge_attach(struct drm_bridge *bridge,
+ enum drm_bridge_attach_flags flags)
+{
+ struct tidss_oldi *oldi = drm_bridge_to_tidss_oldi(bridge);
+
+ if (!oldi->next_bridge) {
+ dev_err(oldi->dev,
+ "%s: OLDI%u Failure attach next bridge\n",
+ __func__, oldi->oldi_instance);
+ return -ENODEV;
+ }
+
+ if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)) {
+ dev_err(oldi->dev,
+ "%s: OLDI%u DRM_BRIDGE_ATTACH_NO_CONNECTOR is mandatory.\n",
+ __func__, oldi->oldi_instance);
+ return -EINVAL;
+ }
+
+ return drm_bridge_attach(bridge->encoder, oldi->next_bridge,
+ bridge, flags);
+}
+
+static int
+tidss_oldi_set_serial_clk(struct tidss_oldi *oldi, unsigned long rate)
+{
+ unsigned long new_rate;
+ int ret;
+
+ ret = clk_set_rate(oldi->serial, rate);
+ if (ret) {
+ dev_err(oldi->dev,
+ "OLDI%u: failed to set serial clk rate to %lu Hz\n",
+ oldi->oldi_instance, rate);
+ return ret;
+ }
+
+ new_rate = clk_get_rate(oldi->serial);
+
+ if (dispc_pclk_diff(rate, new_rate) > 5)
+ dev_warn(oldi->dev,
+ "OLDI%u Clock rate %lu differs over 5%% from requested %lu\n",
+ oldi->oldi_instance, new_rate, rate);
+
+ dev_dbg(oldi->dev, "OLDI%u: new rate %lu Hz (requested %lu Hz)\n",
+ oldi->oldi_instance, clk_get_rate(oldi->serial), rate);
+
+ return 0;
+}
+
+static void tidss_oldi_tx_power(struct tidss_oldi *oldi, bool enable)
+{
+ u32 mask;
+
+ /*
+ * The power control bits are Active Low, and remain powered off by
+ * default. That is, the bits are set to 1. To power on the OLDI TXes,
+ * the bits must be cleared to 0. Since there are cases where not all
+ * OLDI TXes are being used, the power logic selectively powers them
+ * on.
+ * Setting the variable 'val' to particular bit masks, makes sure that
+ * the unrequired OLDI TXes remain powered off.
+ */
+
+ if (enable) {
+ switch (oldi->link_type) {
+ case OLDI_MODE_SINGLE_LINK:
+ /* Power-on only the required OLDI TX's IO*/
+ mask = OLDI_PWRDOWN_TX(oldi->oldi_instance) | OLDI_PWRDN_BG;
+ break;
+ case OLDI_MODE_CLONE_SINGLE_LINK:
+ case OLDI_MODE_DUAL_LINK:
+ /* Power-on both the OLDI TXes' IOs */
+ mask = OLDI_PWRDOWN_TX(oldi->oldi_instance) |
+ OLDI_PWRDOWN_TX(oldi->companion_instance) |
+ OLDI_PWRDN_BG;
+ break;
+ default:
+ /*
+ * This code execution should never reach here as any
+ * OLDI with an unsupported OLDI mode would never get
+ * registered in the first place.
+ * However, power-off the OLDI in concern just in case.
+ */
+ mask = OLDI_PWRDOWN_TX(oldi->oldi_instance);
+ enable = false;
+ break;
+ }
+ } else {
+ switch (oldi->link_type) {
+ case OLDI_MODE_CLONE_SINGLE_LINK:
+ case OLDI_MODE_DUAL_LINK:
+ mask = OLDI_PWRDOWN_TX(oldi->oldi_instance) |
+ OLDI_PWRDOWN_TX(oldi->companion_instance) |
+ OLDI_PWRDN_BG;
+ break;
+ case OLDI_MODE_SINGLE_LINK:
+ default:
+ mask = OLDI_PWRDOWN_TX(oldi->oldi_instance);
+ break;
+ }
+ }
+
+ regmap_update_bits(oldi->io_ctrl, OLDI_PD_CTRL, mask, enable ? 0 : mask);
+}
+
+static int tidss_oldi_config(struct tidss_oldi *oldi)
+{
+ const struct oldi_bus_format *bus_fmt = NULL;
+ u32 oldi_cfg = 0;
+
+ bus_fmt = oldi->bus_format;
+
+ /*
+ * MASTERSLAVE and SRC bits of OLDI Config are always set to 0.
+ */
+
+ if (bus_fmt->data_width == 24)
+ oldi_cfg |= OLDI_MSB;
+ else if (bus_fmt->data_width != 18)
+ dev_warn(oldi->dev,
+ "OLDI%u: DSS port width %d not supported\n",
+ oldi->oldi_instance, bus_fmt->data_width);
+
+ oldi_cfg |= OLDI_DEPOL;
+
+ oldi_cfg = (oldi_cfg & (~OLDI_MAP)) | (bus_fmt->oldi_mode_reg_val << 1);
+
+ oldi_cfg |= OLDI_SOFTRST;
+
+ oldi_cfg |= OLDI_ENABLE;
+
+ switch (oldi->link_type) {
+ case OLDI_MODE_SINGLE_LINK:
+ /* All configuration is done for this mode. */
+ break;
+
+ case OLDI_MODE_CLONE_SINGLE_LINK:
+ oldi_cfg |= OLDI_CLONE_MODE;
+ break;
+
+ case OLDI_MODE_DUAL_LINK:
+ /* data-mapping field also indicates dual-link mode */
+ oldi_cfg |= BIT(3);
+ oldi_cfg |= OLDI_DUALMODESYNC;
+ break;
+
+ default:
+ dev_err(oldi->dev, "OLDI%u: Unsupported mode.\n",
+ oldi->oldi_instance);
+ return -EINVAL;
+ }
+
+ tidss_configure_oldi(oldi->tidss, oldi->parent_vp, oldi_cfg);
+
+ return 0;
+}
+
+static void tidss_oldi_atomic_pre_enable(struct drm_bridge *bridge,
+ struct drm_bridge_state *old_bridge_state)
+{
+ struct tidss_oldi *oldi = drm_bridge_to_tidss_oldi(bridge);
+ struct drm_atomic_state *state = old_bridge_state->base.state;
+ struct drm_connector *connector;
+ struct drm_connector_state *conn_state;
+ struct drm_crtc_state *crtc_state;
+ struct drm_display_mode *mode;
+
+ connector = drm_atomic_get_new_connector_for_encoder(state,
+ bridge->encoder);
+ if (WARN_ON(!connector))
+ return;
+
+ conn_state = drm_atomic_get_new_connector_state(state, connector);
+ if (WARN_ON(!conn_state))
+ return;
+
+ crtc_state = drm_atomic_get_new_crtc_state(state, conn_state->crtc);
+ if (WARN_ON(!crtc_state))
+ return;
+
+ mode = &crtc_state->adjusted_mode;
+
+ /* Configure the OLDI params*/
+ tidss_oldi_config(oldi);
+
+ /* Set the OLDI serial clock (7 times the pixel clock) */
+ tidss_oldi_set_serial_clk(oldi, mode->clock * 7 * 1000);
+
+ /* Enable OLDI IO power */
+ tidss_oldi_tx_power(oldi, true);
+}
+
+static void tidss_oldi_atomic_post_disable(struct drm_bridge *bridge,
+ struct drm_bridge_state *old_bridge_state)
+{
+ struct tidss_oldi *oldi = drm_bridge_to_tidss_oldi(bridge);
+
+ /* Disable OLDI IO power */
+ tidss_oldi_tx_power(oldi, false);
+
+ /* Set the OLDI serial clock to IDLE Frequency */
+ tidss_oldi_set_serial_clk(oldi, OLDI_IDLE_CLK_HZ);
+
+ /* Clear OLDI Config */
+ tidss_configure_oldi(oldi->tidss, oldi->parent_vp, 0);
+}
+
+#define MAX_INPUT_SEL_FORMATS 1
+
+static u32 *tidss_oldi_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
+ struct drm_bridge_state *bridge_state,
+ struct drm_crtc_state *crtc_state,
+ struct drm_connector_state *conn_state,
+ u32 output_fmt,
+ unsigned int *num_input_fmts)
+{
+ struct tidss_oldi *oldi = drm_bridge_to_tidss_oldi(bridge);
+ u32 *input_fmts;
+ int i;
+
+ *num_input_fmts = 0;
+
+ for (i = 0; i < ARRAY_SIZE(oldi_bus_formats); i++)
+ if (oldi_bus_formats[i].bus_fmt == output_fmt)
+ break;
+
+ if (i == ARRAY_SIZE(oldi_bus_formats))
+ return NULL;
+
+ input_fmts = kcalloc(MAX_INPUT_SEL_FORMATS, sizeof(*input_fmts),
+ GFP_KERNEL);
+ if (!input_fmts)
+ return NULL;
+
+ *num_input_fmts = 1;
+ input_fmts[0] = oldi_bus_formats[i].input_bus_fmt;
+ oldi->bus_format = &oldi_bus_formats[i];
+
+ return input_fmts;
+}
+
+static const struct drm_bridge_funcs tidss_oldi_bridge_funcs = {
+ .attach = tidss_oldi_bridge_attach,
+ .atomic_pre_enable = tidss_oldi_atomic_pre_enable,
+ .atomic_post_disable = tidss_oldi_atomic_post_disable,
+ .atomic_get_input_bus_fmts = tidss_oldi_atomic_get_input_bus_fmts,
+ .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
+ .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+ .atomic_reset = drm_atomic_helper_bridge_reset,
+};
+
+static int get_oldi_mode(struct device_node *oldi_tx, u32 *companion_instance)
+{
+ struct device_node *companion;
+ struct device_node *port0, *port1;
+ int pixel_order;
+
+ /*
+ * Find if the OLDI is paired with another OLDI for combined OLDI
+ * operation (dual-lvds or clone).
+ */
+ companion = of_parse_phandle(oldi_tx, "ti,companion-oldi", 0);
+ if (!companion) {
+ /*
+ * OLDI TXes in Single Link mode do not have companion
+ * OLDI TXes and, Secondary OLDI nodes don't need this
+ * information.
+ */
+ *companion_instance = -1;
+
+ if (of_property_read_bool(oldi_tx, "ti,secondary-oldi"))
+ return OLDI_MODE_SECONDARY;
+
+ /*
+ * The OLDI TX does not have a companion, nor is it a
+ * secondary OLDI. It will operate independently.
+ */
+ return OLDI_MODE_SINGLE_LINK;
+ }
+
+ if (of_property_read_u32(companion, "reg", companion_instance))
+ return OLDI_MODE_UNSUPPORTED;
+
+ /*
+ * We need to work out if the sink is expecting us to function in
+ * dual-link mode. We do this by looking at the DT port nodes we are
+ * connected to, if they are marked as expecting even pixels and
+ * odd pixels than we need to enable vertical stripe output.
+ */
+ port0 = of_graph_get_port_by_id(oldi_tx, 1);
+ port1 = of_graph_get_port_by_id(companion, 1);
+ pixel_order = drm_of_lvds_get_dual_link_pixel_order(port0, port1);
+ of_node_put(port0);
+ of_node_put(port1);
+ of_node_put(companion);
+
+ switch (pixel_order) {
+ case -EINVAL:
+ /*
+ * The dual link properties were not found in at least
+ * one of the sink nodes. Since 2 OLDI ports are present
+ * in the DT, it can be safely assumed that the required
+ * configuration is Clone Mode.
+ */
+ return OLDI_MODE_CLONE_SINGLE_LINK;
+
+ case DRM_LVDS_DUAL_LINK_ODD_EVEN_PIXELS:
+ return OLDI_MODE_DUAL_LINK;
+
+ /* Unsupported OLDI Modes */
+ case DRM_LVDS_DUAL_LINK_EVEN_ODD_PIXELS:
+ default:
+ return OLDI_MODE_UNSUPPORTED;
+ }
+}
+
+static int get_parent_dss_vp(struct device_node *oldi_tx, u32 *parent_vp)
+{
+ struct device_node *ep, *dss_port;
+ int ret;
+
+ ep = of_graph_get_endpoint_by_regs(oldi_tx, OLDI_INPUT_PORT, -1);
+ if (ep) {
+ dss_port = of_graph_get_remote_port(ep);
+ if (!dss_port) {
+ ret = -ENODEV;
+ goto err_return_ep_port;
+ }
+
+ ret = of_property_read_u32(dss_port, "reg", parent_vp);
+
+ of_node_put(dss_port);
+err_return_ep_port:
+ of_node_put(ep);
+ return ret;
+ }
+
+ return -ENODEV;
+}
+
+static const struct drm_bridge_timings default_tidss_oldi_timings = {
+ .input_bus_flags = DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE
+ | DRM_BUS_FLAG_DE_HIGH,
+};
+
+void tidss_oldi_deinit(struct tidss_device *tidss)
+{
+ for (int i = 0; i < tidss->num_oldis; i++) {
+ if (tidss->oldis[i]) {
+ drm_bridge_remove(&tidss->oldis[i]->bridge);
+ tidss->oldis[i] = NULL;
+ }
+ }
+}
+
+int tidss_oldi_init(struct tidss_device *tidss)
+{
+ struct tidss_oldi *oldi;
+ struct device_node *child;
+ struct drm_bridge *bridge;
+ u32 parent_vp, oldi_instance, companion_instance;
+ enum tidss_oldi_link_type link_type = OLDI_MODE_UNSUPPORTED;
+ struct device_node *oldi_parent;
+ int ret = 0;
+
+ tidss->num_oldis = 0;
+
+ oldi_parent = of_get_child_by_name(tidss->dev->of_node, "oldi-transmitters");
+ if (!oldi_parent)
+ /* Return gracefully */
+ return 0;
+
+ for_each_child_of_node(oldi_parent, child) {
+ ret = get_parent_dss_vp(child, &parent_vp);
+ if (ret) {
+ if (ret == -ENODEV) {
+ /*
+ * ENODEV means that this particular OLDI node
+ * is not connected with the DSS, which is not
+ * a harmful case. There could be another OLDI
+ * which may still be connected.
+ * Continue to search for that.
+ */
+ ret = 0;
+ continue;
+ }
+ goto err_put_node;
+ }
+
+ ret = of_property_read_u32(child, "reg", &oldi_instance);
+ if (ret)
+ goto err_put_node;
+
+ /*
+ * Now that its confirmed that OLDI is connected with DSS, let's
+ * continue getting the OLDI sinks ahead and other OLDI
+ * properties.
+ */
+ bridge = devm_drm_of_get_bridge(tidss->dev, child,
+ OLDI_OURPUT_PORT, 0);
+ if (IS_ERR(bridge)) {
+ /*
+ * Either there was no OLDI sink in the devicetree, or
+ * the OLDI sink has not been added yet. In any case,
+ * return.
+ * We don't want to have an OLDI node connected to DSS
+ * but not to any sink.
+ */
+ ret = dev_err_probe(tidss->dev, PTR_ERR(bridge),
+ "no panel/bridge for OLDI%u.\n",
+ oldi_instance);
+ goto err_put_node;
+ }
+
+ link_type = get_oldi_mode(child, &companion_instance);
+ if (link_type == OLDI_MODE_UNSUPPORTED) {
+ ret = dev_err_probe(tidss->dev, -EINVAL,
+ "OLDI%u: Unsupported OLDI connection.\n",
+ oldi_instance);
+ goto err_put_node;
+ } else if (link_type == OLDI_MODE_CLONE_SINGLE_LINK) {
+ /*
+ * The OLDI driver cannot support OLDI clone mode
+ * properly at present.
+ * The clone mode requires 2 working encoder-bridge
+ * pipelines, generating from the same crtc. The DRM
+ * framework does not support this at present. If
+ * there were to be, say, 2 OLDI sink bridges each
+ * connected to an OLDI TXes, they couldn't both be
+ * supported simultaneously.
+ * This driver still has some code pertaining to OLDI
+ * clone mode configuration in DSS hardware for future,
+ * when there is a better infrastructure in the DRM
+ * framework to support 2 encoder-bridge pipelines
+ * simultaneously.
+ * Till that time, this driver shall error out if it
+ * detects a clone mode configuration.
+ */
+ ret = dev_err_probe(tidss->dev, -EOPNOTSUPP,
+ "The OLDI driver does not support Clone Mode at present.\n");
+ goto err_put_node;
+ } else if (link_type == OLDI_MODE_SECONDARY) {
+ /*
+ * This is the secondary OLDI node, which serves as a
+ * companinon to the primary OLDI, when it is configured
+ * for the dual-lvds mode. Since the primary OLDI will
+ * be a part of bridge chain, no need to put this one
+ * too. Continue onto the next OLDI node.
+ */
+ continue;
+ }
+
+ oldi = devm_kzalloc(tidss->dev, sizeof(*oldi), GFP_KERNEL);
+ if (!oldi) {
+ ret = -ENOMEM;
+ goto err_put_node;
+ }
+
+ oldi->parent_vp = parent_vp;
+ oldi->oldi_instance = oldi_instance;
+ oldi->companion_instance = companion_instance;
+ oldi->link_type = link_type;
+ oldi->dev = tidss->dev;
+ oldi->next_bridge = bridge;
+
+ oldi->io_ctrl = syscon_regmap_lookup_by_phandle(child,
+ "ti,oldi-io-ctrl");
+ if (IS_ERR(oldi->io_ctrl)) {
+ ret = dev_err_probe(oldi->dev, PTR_ERR(oldi->io_ctrl),
+ "OLDI%u: syscon_regmap_lookup_by_phandle failed.\n",
+ oldi_instance);
+ goto err_put_node;
+ }
+
+ oldi->serial = of_clk_get_by_name(child, "serial");
+ if (IS_ERR(oldi->serial)) {
+ ret = dev_err_probe(oldi->dev, PTR_ERR(oldi->serial),
+ "OLDI%u: Failed to get serial clock.\n",
+ oldi_instance);
+ goto err_put_node;
+ }
+
+ /* Register the bridge. */
+ oldi->bridge.of_node = child;
+ oldi->bridge.driver_private = oldi;
+ oldi->bridge.funcs = &tidss_oldi_bridge_funcs;
+ oldi->bridge.timings = &default_tidss_oldi_timings;
+
+ tidss->oldis[tidss->num_oldis++] = oldi;
+ oldi->tidss = tidss;
+
+ drm_bridge_add(&oldi->bridge);
+ }
+
+ of_node_put(child);
+ of_node_put(oldi_parent);
+
+ return 0;
+
+err_put_node:
+ of_node_put(child);
+ of_node_put(oldi_parent);
+ return ret;
+}
diff --git a/drivers/gpu/drm/tidss/tidss_oldi.h b/drivers/gpu/drm/tidss/tidss_oldi.h
new file mode 100644
index 000000000000..e0bdd199d128
--- /dev/null
+++ b/drivers/gpu/drm/tidss/tidss_oldi.h
@@ -0,0 +1,51 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (C) 2024 - Texas Instruments Incorporated
+ *
+ * Aradhya Bhatia <a-bhatia1@ti.com>
+ */
+
+#ifndef __TIDSS_OLDI_H__
+#define __TIDSS_OLDI_H__
+
+#include "tidss_drv.h"
+
+struct tidss_oldi;
+
+/* OLDI PORTS */
+#define OLDI_INPUT_PORT 0
+#define OLDI_OURPUT_PORT 1
+
+/* Control MMR Registers */
+
+/* Register offsets */
+#define OLDI_PD_CTRL 0x100
+#define OLDI_LB_CTRL 0x104
+
+/* Power control bits */
+#define OLDI_PWRDOWN_TX(n) BIT(n)
+
+/* LVDS Bandgap reference Enable/Disable */
+#define OLDI_PWRDN_BG BIT(8)
+
+enum tidss_oldi_link_type {
+ OLDI_MODE_UNSUPPORTED,
+ OLDI_MODE_SINGLE_LINK,
+ OLDI_MODE_CLONE_SINGLE_LINK,
+ OLDI_MODE_DUAL_LINK,
+ OLDI_MODE_SECONDARY,
+};
+
+enum oldi_mode_reg_val { SPWG_18 = 0, JEIDA_24 = 1, SPWG_24 = 2 };
+
+struct oldi_bus_format {
+ u32 bus_fmt;
+ u32 data_width;
+ enum oldi_mode_reg_val oldi_mode_reg_val;
+ u32 input_bus_fmt;
+};
+
+int tidss_oldi_init(struct tidss_device *tidss);
+void tidss_oldi_deinit(struct tidss_device *tidss);
+
+#endif /* __TIDSS_OLDI_H__ */
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v5 3/3] drm/tidss: Add OLDI bridge support
2025-02-09 16:09 ` [PATCH v5 3/3] drm/tidss: Add OLDI bridge support Aradhya Bhatia
@ 2025-02-11 10:57 ` Tomi Valkeinen
2025-02-11 12:25 ` Tomi Valkeinen
0 siblings, 1 reply; 12+ messages in thread
From: Tomi Valkeinen @ 2025-02-11 10:57 UTC (permalink / raw)
To: Aradhya Bhatia
Cc: Maarten Lankhorst, Thomas Zimmermann, Maxime Ripard, David Airlie,
Laurent Pinchart, Simona Vetter, Nishanth Menon,
Vignesh Raghavendra, Devarsh Thakkar, Praneeth Bajjuri,
Udit Kumar, Jayesh Choudhary, Francesco Dolcini,
DRI Development List, Devicetree List, Linux Kernel List,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jyri Sarha
Hi,
On 09/02/2025 18:09, Aradhya Bhatia wrote:
> From: Aradhya Bhatia <a-bhatia1@ti.com>
>
> The AM62x and AM62Px SoCs feature 2 OLDI TXes each, which makes it
> possible to connect them in dual-link or cloned single-link OLDI display
> modes. The current OLDI support in tidss_dispc.c can only support for
> a single OLDI TX, connected to a VP and doesn't really support
> configuration of OLDIs in the other modes. The current OLDI support in
> tidss_dispc.c also works on the principle that the OLDI output can only
> be served by one, and only one, DSS video-port. This isn't the case in
> the AM62Px SoC, where there are 2 DSS controllers present that share the
> OLDI TXes.
>
> Having their own devicetree and their own bridge entity will help
> support the various display modes and sharing possiblilities of the OLDI
> hardware.
>
> For all these reasons, add support for the OLDI TXes as DRM bridges.
>
> Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
> Signed-off-by: Aradhya Bhatia <aradhya.bhatia@linux.dev>
> ---
> drivers/gpu/drm/tidss/Makefile | 3 +-
> drivers/gpu/drm/tidss/tidss_dispc.c | 20 +-
> drivers/gpu/drm/tidss/tidss_dispc.h | 4 +
> drivers/gpu/drm/tidss/tidss_dispc_regs.h | 14 +
> drivers/gpu/drm/tidss/tidss_drv.c | 9 +
> drivers/gpu/drm/tidss/tidss_drv.h | 5 +
> drivers/gpu/drm/tidss/tidss_oldi.c | 558 +++++++++++++++++++++++
> drivers/gpu/drm/tidss/tidss_oldi.h | 51 +++
> 8 files changed, 662 insertions(+), 2 deletions(-)
> create mode 100644 drivers/gpu/drm/tidss/tidss_oldi.c
> create mode 100644 drivers/gpu/drm/tidss/tidss_oldi.h
>
> diff --git a/drivers/gpu/drm/tidss/Makefile b/drivers/gpu/drm/tidss/Makefile
> index 312645271014..b6d6becf1683 100644
> --- a/drivers/gpu/drm/tidss/Makefile
> +++ b/drivers/gpu/drm/tidss/Makefile
> @@ -7,6 +7,7 @@ tidss-y := tidss_crtc.o \
> tidss_irq.o \
> tidss_plane.o \
> tidss_scale_coefs.o \
> - tidss_dispc.o
> + tidss_dispc.o \
> + tidss_oldi.o
>
> obj-$(CONFIG_DRM_TIDSS) += tidss.o
> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c b/drivers/gpu/drm/tidss/tidss_dispc.c
> index 1ad711f8d2a8..8631a89e6155 100644
> --- a/drivers/gpu/drm/tidss/tidss_dispc.c
> +++ b/drivers/gpu/drm/tidss/tidss_dispc.c
> @@ -466,6 +466,25 @@ static u32 dispc_vp_read(struct dispc_device *dispc, u32 hw_videoport, u16 reg)
> return ioread32(base + reg);
> }
>
> +void tidss_configure_oldi(struct tidss_device *tidss, u32 hw_videoport,
> + u32 oldi_cfg)
> +{
> + u32 count = 0;
> + u32 oldi_reset_bit = BIT(5 + hw_videoport);
> +
> + dispc_vp_write(tidss->dispc, hw_videoport, DISPC_VP_DSS_OLDI_CFG, oldi_cfg);
> +
> + if (oldi_cfg != 0) {
> + while (!(oldi_reset_bit & dispc_read(tidss->dispc, DSS_SYSSTATUS)) &&
> + count < 10000)
> + count++;
> +
> + if (!(oldi_reset_bit & dispc_read(tidss->dispc, DSS_SYSSTATUS)))
> + dev_warn(tidss->dispc->dev, "%s: timeout waiting OLDI reset done\n",
> + __func__);
> + }
> +}
The timeout sounds like an error. Better to return an error value, and
handle it in tidss_oldi_config()?
You could also
if (!oldi_cfg)
return 0;
But would it actually be nicer to have a separate void function for
disabling?
> +
> /*
> * TRM gives bitfields as start:end, where start is the higher bit
> * number. For example 7:0
> @@ -1310,7 +1329,6 @@ void dispc_vp_disable_clk(struct dispc_device *dispc, u32 hw_videoport)
> * Calculate the percentage difference between the requested pixel clock rate
> * and the effective rate resulting from calculating the clock divider value.
> */
> -static
> unsigned int dispc_pclk_diff(unsigned long rate, unsigned long real_rate)
> {
> int r = rate / 100, rr = real_rate / 100;
> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.h b/drivers/gpu/drm/tidss/tidss_dispc.h
> index 086327d51a90..fab248f2055a 100644
> --- a/drivers/gpu/drm/tidss/tidss_dispc.h
> +++ b/drivers/gpu/drm/tidss/tidss_dispc.h
> @@ -94,6 +94,10 @@ extern const struct dispc_features dispc_am62a7_feats;
> extern const struct dispc_features dispc_am65x_feats;
> extern const struct dispc_features dispc_j721e_feats;
>
> +void tidss_configure_oldi(struct tidss_device *tidss, u32 hw_videoport,
> + u32 oldi_cfg);
> +unsigned int dispc_pclk_diff(unsigned long rate, unsigned long real_rate);
> +
> void dispc_set_irqenable(struct dispc_device *dispc, dispc_irq_t mask);
> dispc_irq_t dispc_read_and_clear_irqstatus(struct dispc_device *dispc);
>
> diff --git a/drivers/gpu/drm/tidss/tidss_dispc_regs.h b/drivers/gpu/drm/tidss/tidss_dispc_regs.h
> index 13feedfe5d6d..03f7098029e6 100644
> --- a/drivers/gpu/drm/tidss/tidss_dispc_regs.h
> +++ b/drivers/gpu/drm/tidss/tidss_dispc_regs.h
> @@ -226,6 +226,20 @@ enum dispc_common_regs {
> #define DISPC_VP_DSS_DMA_THREADSIZE 0x170 /* J721E */
> #define DISPC_VP_DSS_DMA_THREADSIZE_STATUS 0x174 /* J721E */
>
> +/* OLDI Config Bits (DISPC_VP_DSS_OLDI_CFG) */
> +#define OLDI_ENABLE BIT(0)
> +#define OLDI_MAP (BIT(1) | BIT(2) | BIT(3))
> +#define OLDI_SRC BIT(4)
> +#define OLDI_CLONE_MODE BIT(5)
> +#define OLDI_MASTERSLAVE BIT(6)
> +#define OLDI_DEPOL BIT(7)
> +#define OLDI_MSB BIT(8)
> +#define OLDI_LBEN BIT(9)
> +#define OLDI_LBDATA BIT(10)
> +#define OLDI_DUALMODESYNC BIT(11)
> +#define OLDI_SOFTRST BIT(12)
> +#define OLDI_TPATCFG BIT(13)
> +
> /*
> * OLDI IO_CTRL register offsets. On AM654 the registers are found
> * from CTRL_MMR0, there the syscon regmap should map 0x14 bytes from
> diff --git a/drivers/gpu/drm/tidss/tidss_drv.c b/drivers/gpu/drm/tidss/tidss_drv.c
> index 7c8fd6407d82..27b9f86f1eb2 100644
> --- a/drivers/gpu/drm/tidss/tidss_drv.c
> +++ b/drivers/gpu/drm/tidss/tidss_drv.c
> @@ -24,6 +24,7 @@
> #include "tidss_drv.h"
> #include "tidss_kms.h"
> #include "tidss_irq.h"
> +#include "tidss_oldi.h"
>
> /* Power management */
>
> @@ -148,6 +149,10 @@ static int tidss_probe(struct platform_device *pdev)
> return ret;
> }
>
> + ret = tidss_oldi_init(tidss);
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to init OLDI\n");
> +
> pm_runtime_enable(dev);
>
> pm_runtime_set_autosuspend_delay(dev, 1000);
> @@ -204,6 +209,8 @@ static int tidss_probe(struct platform_device *pdev)
> pm_runtime_dont_use_autosuspend(dev);
> pm_runtime_disable(dev);
>
> + tidss_oldi_deinit(tidss);
> +
> return ret;
> }
>
> @@ -228,6 +235,8 @@ static void tidss_remove(struct platform_device *pdev)
> pm_runtime_dont_use_autosuspend(dev);
> pm_runtime_disable(dev);
>
> + tidss_oldi_deinit(tidss);
> +
> /* devm allocated dispc goes away with the dev so mark it NULL */
> dispc_remove(tidss);
>
> diff --git a/drivers/gpu/drm/tidss/tidss_drv.h b/drivers/gpu/drm/tidss/tidss_drv.h
> index d7f27b0b0315..6c0fe1d989ee 100644
> --- a/drivers/gpu/drm/tidss/tidss_drv.h
> +++ b/drivers/gpu/drm/tidss/tidss_drv.h
> @@ -11,8 +11,10 @@
>
> #define TIDSS_MAX_PORTS 4
> #define TIDSS_MAX_PLANES 4
> +#define TIDSS_MAX_OLDI_TXES 2
>
> typedef u32 dispc_irq_t;
> +struct tidss_oldi;
>
> struct tidss_device {
> struct drm_device ddev; /* DRM device for DSS */
> @@ -27,6 +29,9 @@ struct tidss_device {
> unsigned int num_planes;
> struct drm_plane *planes[TIDSS_MAX_PLANES];
>
> + unsigned int num_oldis;
> + struct tidss_oldi *oldis[TIDSS_MAX_OLDI_TXES];
> +
> unsigned int irq;
>
> spinlock_t wait_lock; /* protects the irq masks */
> diff --git a/drivers/gpu/drm/tidss/tidss_oldi.c b/drivers/gpu/drm/tidss/tidss_oldi.c
> new file mode 100644
> index 000000000000..4af13a01f546
> --- /dev/null
> +++ b/drivers/gpu/drm/tidss/tidss_oldi.c
> @@ -0,0 +1,558 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2024 - Texas Instruments Incorporated
> + *
> + * Aradhya Bhatia <a-bhatia1@ti.com>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/of.h>
> +#include <linux/of_graph.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/media-bus-format.h>
> +#include <linux/regmap.h>
> +
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_bridge.h>
> +#include <drm/drm_of.h>
> +
> +#include "tidss_dispc.h"
> +#include "tidss_dispc_regs.h"
> +#include "tidss_oldi.h"
> +
> +struct tidss_oldi {
> + struct tidss_device *tidss;
> + struct device *dev;
> +
> + struct drm_bridge bridge;
> + struct drm_bridge *next_bridge;
> +
> + enum tidss_oldi_link_type link_type;
> + const struct oldi_bus_format *bus_format;
> + u32 oldi_instance;
> + u32 companion_instance;
> + u32 parent_vp;
> +
> + struct clk *serial;
> + struct regmap *io_ctrl;
> +};
> +
> +static const struct oldi_bus_format oldi_bus_formats[] = {
> + { MEDIA_BUS_FMT_RGB666_1X7X3_SPWG, 18, SPWG_18, MEDIA_BUS_FMT_RGB666_1X18 },
> + { MEDIA_BUS_FMT_RGB888_1X7X4_SPWG, 24, SPWG_24, MEDIA_BUS_FMT_RGB888_1X24 },
> + { MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA, 24, JEIDA_24, MEDIA_BUS_FMT_RGB888_1X24 },
> +};
> +
> +#define OLDI_IDLE_CLK_HZ 25000000 /*25 MHz */
> +
> +static inline struct tidss_oldi *
> +drm_bridge_to_tidss_oldi(struct drm_bridge *bridge)
> +{
> + return container_of(bridge, struct tidss_oldi, bridge);
> +}
> +
> +static int tidss_oldi_bridge_attach(struct drm_bridge *bridge,
> + enum drm_bridge_attach_flags flags)
> +{
> + struct tidss_oldi *oldi = drm_bridge_to_tidss_oldi(bridge);
> +
> + if (!oldi->next_bridge) {
> + dev_err(oldi->dev,
> + "%s: OLDI%u Failure attach next bridge\n",
> + __func__, oldi->oldi_instance);
> + return -ENODEV;
> + }
> +
> + if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)) {
> + dev_err(oldi->dev,
> + "%s: OLDI%u DRM_BRIDGE_ATTACH_NO_CONNECTOR is mandatory.\n",
> + __func__, oldi->oldi_instance);
> + return -EINVAL;
> + }
> +
> + return drm_bridge_attach(bridge->encoder, oldi->next_bridge,
> + bridge, flags);
> +}
> +
> +static int
> +tidss_oldi_set_serial_clk(struct tidss_oldi *oldi, unsigned long rate)
> +{
> + unsigned long new_rate;
> + int ret;
> +
> + ret = clk_set_rate(oldi->serial, rate);
> + if (ret) {
> + dev_err(oldi->dev,
> + "OLDI%u: failed to set serial clk rate to %lu Hz\n",
> + oldi->oldi_instance, rate);
> + return ret;
> + }
> +
> + new_rate = clk_get_rate(oldi->serial);
> +
> + if (dispc_pclk_diff(rate, new_rate) > 5)
> + dev_warn(oldi->dev,
> + "OLDI%u Clock rate %lu differs over 5%% from requested %lu\n",
> + oldi->oldi_instance, new_rate, rate);
> +
> + dev_dbg(oldi->dev, "OLDI%u: new rate %lu Hz (requested %lu Hz)\n",
> + oldi->oldi_instance, clk_get_rate(oldi->serial), rate);
> +
> + return 0;
> +}
> +
> +static void tidss_oldi_tx_power(struct tidss_oldi *oldi, bool enable)
> +{
> + u32 mask;
> +
> + /*
> + * The power control bits are Active Low, and remain powered off by
> + * default. That is, the bits are set to 1. To power on the OLDI TXes,
> + * the bits must be cleared to 0. Since there are cases where not all
> + * OLDI TXes are being used, the power logic selectively powers them
> + * on.
> + * Setting the variable 'val' to particular bit masks, makes sure that
> + * the unrequired OLDI TXes remain powered off.
> + */
> +
> + if (enable) {
> + switch (oldi->link_type) {
> + case OLDI_MODE_SINGLE_LINK:
> + /* Power-on only the required OLDI TX's IO*/
> + mask = OLDI_PWRDOWN_TX(oldi->oldi_instance) | OLDI_PWRDN_BG;
> + break;
> + case OLDI_MODE_CLONE_SINGLE_LINK:
> + case OLDI_MODE_DUAL_LINK:
> + /* Power-on both the OLDI TXes' IOs */
> + mask = OLDI_PWRDOWN_TX(oldi->oldi_instance) |
> + OLDI_PWRDOWN_TX(oldi->companion_instance) |
> + OLDI_PWRDN_BG;
> + break;
> + default:
> + /*
> + * This code execution should never reach here as any
> + * OLDI with an unsupported OLDI mode would never get
> + * registered in the first place.
> + * However, power-off the OLDI in concern just in case.
> + */
> + mask = OLDI_PWRDOWN_TX(oldi->oldi_instance);
> + enable = false;
> + break;
> + }
> + } else {
> + switch (oldi->link_type) {
> + case OLDI_MODE_CLONE_SINGLE_LINK:
> + case OLDI_MODE_DUAL_LINK:
> + mask = OLDI_PWRDOWN_TX(oldi->oldi_instance) |
> + OLDI_PWRDOWN_TX(oldi->companion_instance) |
> + OLDI_PWRDN_BG;
> + break;
> + case OLDI_MODE_SINGLE_LINK:
> + default:
> + mask = OLDI_PWRDOWN_TX(oldi->oldi_instance);
> + break;
> + }
> + }
> +
> + regmap_update_bits(oldi->io_ctrl, OLDI_PD_CTRL, mask, enable ? 0 : mask);
> +}
> +
> +static int tidss_oldi_config(struct tidss_oldi *oldi)
> +{
> + const struct oldi_bus_format *bus_fmt = NULL;
> + u32 oldi_cfg = 0;
> +
> + bus_fmt = oldi->bus_format;
> +
> + /*
> + * MASTERSLAVE and SRC bits of OLDI Config are always set to 0.
> + */
> +
> + if (bus_fmt->data_width == 24)
> + oldi_cfg |= OLDI_MSB;
> + else if (bus_fmt->data_width != 18)
> + dev_warn(oldi->dev,
> + "OLDI%u: DSS port width %d not supported\n",
> + oldi->oldi_instance, bus_fmt->data_width);
> +
> + oldi_cfg |= OLDI_DEPOL;
> +
> + oldi_cfg = (oldi_cfg & (~OLDI_MAP)) | (bus_fmt->oldi_mode_reg_val << 1);
> +
> + oldi_cfg |= OLDI_SOFTRST;
> +
> + oldi_cfg |= OLDI_ENABLE;
> +
> + switch (oldi->link_type) {
> + case OLDI_MODE_SINGLE_LINK:
> + /* All configuration is done for this mode. */
> + break;
> +
> + case OLDI_MODE_CLONE_SINGLE_LINK:
> + oldi_cfg |= OLDI_CLONE_MODE;
> + break;
> +
> + case OLDI_MODE_DUAL_LINK:
> + /* data-mapping field also indicates dual-link mode */
> + oldi_cfg |= BIT(3);
> + oldi_cfg |= OLDI_DUALMODESYNC;
> + break;
> +
> + default:
> + dev_err(oldi->dev, "OLDI%u: Unsupported mode.\n",
> + oldi->oldi_instance);
> + return -EINVAL;
> + }
> +
> + tidss_configure_oldi(oldi->tidss, oldi->parent_vp, oldi_cfg);
> +
> + return 0;
> +}
> +
> +static void tidss_oldi_atomic_pre_enable(struct drm_bridge *bridge,
> + struct drm_bridge_state *old_bridge_state)
> +{
> + struct tidss_oldi *oldi = drm_bridge_to_tidss_oldi(bridge);
> + struct drm_atomic_state *state = old_bridge_state->base.state;
> + struct drm_connector *connector;
> + struct drm_connector_state *conn_state;
> + struct drm_crtc_state *crtc_state;
> + struct drm_display_mode *mode;
> +
> + connector = drm_atomic_get_new_connector_for_encoder(state,
> + bridge->encoder);
> + if (WARN_ON(!connector))
> + return;
> +
> + conn_state = drm_atomic_get_new_connector_state(state, connector);
> + if (WARN_ON(!conn_state))
> + return;
> +
> + crtc_state = drm_atomic_get_new_crtc_state(state, conn_state->crtc);
> + if (WARN_ON(!crtc_state))
> + return;
> +
> + mode = &crtc_state->adjusted_mode;
> +
> + /* Configure the OLDI params*/
> + tidss_oldi_config(oldi);
> +
> + /* Set the OLDI serial clock (7 times the pixel clock) */
> + tidss_oldi_set_serial_clk(oldi, mode->clock * 7 * 1000);
> +
> + /* Enable OLDI IO power */
> + tidss_oldi_tx_power(oldi, true);
> +}
> +
> +static void tidss_oldi_atomic_post_disable(struct drm_bridge *bridge,
> + struct drm_bridge_state *old_bridge_state)
> +{
> + struct tidss_oldi *oldi = drm_bridge_to_tidss_oldi(bridge);
> +
> + /* Disable OLDI IO power */
> + tidss_oldi_tx_power(oldi, false);
> +
> + /* Set the OLDI serial clock to IDLE Frequency */
> + tidss_oldi_set_serial_clk(oldi, OLDI_IDLE_CLK_HZ);
> +
> + /* Clear OLDI Config */
> + tidss_configure_oldi(oldi->tidss, oldi->parent_vp, 0);
> +}
> +
> +#define MAX_INPUT_SEL_FORMATS 1
> +
> +static u32 *tidss_oldi_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
> + struct drm_bridge_state *bridge_state,
> + struct drm_crtc_state *crtc_state,
> + struct drm_connector_state *conn_state,
> + u32 output_fmt,
> + unsigned int *num_input_fmts)
> +{
> + struct tidss_oldi *oldi = drm_bridge_to_tidss_oldi(bridge);
> + u32 *input_fmts;
> + int i;
> +
> + *num_input_fmts = 0;
> +
> + for (i = 0; i < ARRAY_SIZE(oldi_bus_formats); i++)
> + if (oldi_bus_formats[i].bus_fmt == output_fmt)
> + break;
> +
> + if (i == ARRAY_SIZE(oldi_bus_formats))
> + return NULL;
> +
> + input_fmts = kcalloc(MAX_INPUT_SEL_FORMATS, sizeof(*input_fmts),
> + GFP_KERNEL);
> + if (!input_fmts)
> + return NULL;
> +
> + *num_input_fmts = 1;
> + input_fmts[0] = oldi_bus_formats[i].input_bus_fmt;
> + oldi->bus_format = &oldi_bus_formats[i];
> +
> + return input_fmts;
> +}
> +
> +static const struct drm_bridge_funcs tidss_oldi_bridge_funcs = {
> + .attach = tidss_oldi_bridge_attach,
Looks like an extra tab there?
Other than those two cosmetic issues, I think this looks fine. I also
tested on AM62-SK.
However, one more thing. We'll have a separate OLDI bridge, but we still
will have the old OLDI code for AM65x in the tidss_dispc.c. And to mix
things up, we will have some new OLDI code there too
(tidss_configure_oldi). Could you check the AM65x specific code and
perhaps rename the functions to am65x or such, to make this clearer.
Perhaps also the DISPC_VP_OLDI should be DISPC_VP_OLDI_AM65X, as that VP
type shouldn't be used for anything else.
Tomi
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v5 2/3] dt-bindings: display: ti: Add schema for AM625 OLDI Transmitter
2025-02-09 16:09 ` [PATCH v5 2/3] dt-bindings: display: ti: Add schema for AM625 OLDI Transmitter Aradhya Bhatia
@ 2025-02-11 12:24 ` Tomi Valkeinen
2025-02-13 12:33 ` Aradhya Bhatia
2025-02-19 14:22 ` Rob Herring (Arm)
1 sibling, 1 reply; 12+ messages in thread
From: Tomi Valkeinen @ 2025-02-11 12:24 UTC (permalink / raw)
To: Aradhya Bhatia
Cc: Maarten Lankhorst, Thomas Zimmermann, Maxime Ripard, David Airlie,
Laurent Pinchart, Simona Vetter, Nishanth Menon,
Vignesh Raghavendra, Devarsh Thakkar, Praneeth Bajjuri,
Udit Kumar, Jayesh Choudhary, Francesco Dolcini,
DRI Development List, Devicetree List, Linux Kernel List,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jyri Sarha
Hi,
On 09/02/2025 18:09, Aradhya Bhatia wrote:
> From: Aradhya Bhatia <a-bhatia1@ti.com>
>
> The OLDI transmitters (TXes) do not have registers of their own, and are
> dependent on the source video-ports (VPs) from the DSS to provide
> configuration data. This hardware doesn't directly sit on the internal
> bus of the SoC, but does so via the DSS. Hence, the OLDI TXes are
> supposed to be child nodes under the DSS, and not independent devices.
>
> Two of the OLDI TXes can function in tandem to output dual-link OLDI
> output, or cloned single-link outputs. In these cases, one OLDI will be
> the primary OLDI, and the other one, a companion. The following diagram
> represents such a configuration.
>
> +-----+-----+ +-------+
> | | | | |
> | | VP1 +----+--->+ OLDI0 | (Primary - may need companion)
> | | | | | |
> | DSS +-----+ | +-------+
> | | | |
> | | VP2 | | +-------+
> | | | | | |
> +-----+-----+ +--->+ OLDI1 | (Companion OLDI)
> | |
> +-------+
>
> The DSS in AM625 SoC has a configuration like the one above. The AM625
> DSS VP1 (port@0) can connect and control 2 OLDI TXes, to use them in
> dual-link or cloned single-link OLDI modes. It is only the VP1 that can
> connect to either OLDI TXes for the AM625 DSS, and not the VP2.
>
> Alternatively, on some future TI SoCs, along with the above
> configuration, the OLDI TX can _also_ connect to separate video sources,
> making them work entirely independent of each other. In this case,
> neither of the OLDIs are "companion" or "secondary" OLDIs, and nor do
> they require one. They both are independent and primary OLDIs. The
> following diagram represents such a configuration.
>
> +-----+-----+ +-------+
> | | | | |
> | | VP1 +--+----------->+ OLDI0 | (Primary - may need companion)
> | | | | | |
> | +-----+ | +-------+
> | | | |
> | | VP2 | |
> | | | |
> | DSS +-----+ | +---+ +-------+
> | | | +-->+ M | | |
> | | VP3 +----->+ U +--->+ OLDI1 | (Companion or Primary)
> | | | | X | | |
> | +-----+ +---+ +-------+
> | | |
> | | VP4 |
> | | |
> +-----+-----+
>
> Note that depending on the mux configuration, the OLDIs can either be
> working together in tandem - sourced by VP1, OR, they could be working
> independently sourced by VP1 and VP3 respectively.
> The idea is to support all the configurations with this OLDI TX schema.
>
> The OLDI functionality is further supported by a system-control module,
> which contains a few registers to control OLDI IO power and other
> electrical characteristics of the IO lanes.
>
> Add devicetree binding schema for the OLDI TXes to support various
> configurations, and extend their support to the AM625 DSS.
>
> Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
> Signed-off-by: Aradhya Bhatia <aradhya.bhatia@linux.dev>
> ---
> .../bindings/display/ti/ti,am625-oldi.yaml | 88 ++++++++++
> .../bindings/display/ti/ti,am65x-dss.yaml | 154 ++++++++++++++++++
> MAINTAINERS | 1 +
> 3 files changed, 243 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/display/ti/ti,am625-oldi.yaml
>
> diff --git a/Documentation/devicetree/bindings/display/ti/ti,am625-oldi.yaml b/Documentation/devicetree/bindings/display/ti/ti,am625-oldi.yaml
> new file mode 100644
> index 000000000000..42a80a512660
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/ti/ti,am625-oldi.yaml
> @@ -0,0 +1,88 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/ti/ti,am625-oldi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Texas Instruments AM625 OLDI Transmitter
> +
> +maintainers:
> + - Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> + - Aradhya Bhatia <aradhya.bhatia@linux.dev>
> +
> +description:
> + The AM625 TI Keystone OpenLDI transmitter (OLDI TX) supports serialized RGB
> + pixel data transmission between host and flat panel display over LVDS (Low
> + Voltage Differential Sampling) interface. The OLDI TX consists of 7-to-1 data
> + serializers, and 4-data and 1-clock LVDS outputs. It supports the LVDS output
> + formats "jeida-18", "jeida-24" and "vesa-18", and can accept 24-bit RGB or
> + padded and un-padded 18-bit RGB bus formats as input.
> +
> +properties:
> + reg:
> + maxItems: 1
> +
> + clocks:
> + maxItems: 1
> + description: serial clock input for the OLDI transmitters
> +
> + clock-names:
> + const: serial
> +
> + ti,companion-oldi:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description:
> + phandle to companion OLDI transmitter. This property is mandatory for the
> + primarty OLDI TX if the OLDI TXes are expected to work either in dual-lvds
> + mode or in clone mode. This property should point to the secondary OLDI
> + TX.
> +
> + ti,secondary-oldi:
> + type: boolean
> + description:
> + Boolean property to mark the OLDI transmitter as the secondary one, when the
> + OLDI hardware is expected to run as a companion HW, in cases of dual-lvds
> + mode or clone mode. The primary OLDI hardware is responsible for all the
> + hardware configuration.
I think these work, but I'm wondering if we would ever need to check
something from the main oldi from the secondary oldi. In that case
"crossed phandles" would be better, i.e. something like:
(in the first oldi:)
ti,slave-oldi = <phandle-to-second-oldi>
(in the second oldi:)
ti,master-oldi = <phandle-to-first-oldi>
Then again, if we ever need that, even with these bindings the driver
could find the first oldi, but needs to go via the dss's node.
So, just a thought.
Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Tomi
> + ti,oldi-io-ctrl:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description:
> + phandle to syscon device node mapping OLDI IO_CTRL registers found in the
> + control MMR region. These registers are required to toggle the I/O lane
> + power, and control its electrical characteristics.
> +
> + ports:
> + $ref: /schemas/graph.yaml#/properties/ports
> +
> + properties:
> + port@0:
> + $ref: /schemas/graph.yaml#/properties/port
> + description: Parallel RGB input port
> +
> + port@1:
> + $ref: /schemas/graph.yaml#/properties/port
> + description: LVDS output port
> +
> + required:
> + - port@0
> + - port@1
> +
> +allOf:
> + - if:
> + required:
> + - ti,secondary-oldi
> + then:
> + properties:
> + ti,companion-oldi: false
> +
> +required:
> + - reg
> + - clocks
> + - clock-names
> + - ti,oldi-io-ctrl
> + - ports
> +
> +additionalProperties: false
> +
> +...
> diff --git a/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml b/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
> index 399d68986326..a82c525631ea 100644
> --- a/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
> +++ b/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
> @@ -91,6 +91,24 @@ properties:
> For AM625 DSS, the internal DPI output port node from video
> port 1.
> For AM62A7 DSS, the port is tied off inside the SoC.
> + properties:
> + endpoint@0:
> + $ref: /schemas/graph.yaml#/properties/endpoint
> + description:
> + For AM625 DSS, VP Connection to OLDI0.
> + For AM65X DSS, OLDI output from the SoC.
> +
> + endpoint@1:
> + $ref: /schemas/graph.yaml#/properties/endpoint
> + description:
> + For AM625 DSS, VP Connection to OLDI1.
> +
> + anyOf:
> + - required:
> + - endpoint
> + - required:
> + - endpoint@0
> + - endpoint@1
>
> port@1:
> $ref: /schemas/graph.yaml#/properties/port
> @@ -112,6 +130,25 @@ properties:
> Input memory (from main memory to dispc) bandwidth limit in
> bytes per second
>
> + oldi-transmitters:
> + description:
> + Child node under the DSS, to describe all the OLDI transmitters connected
> + to the DSS videoports.
> + type: object
> + additionalProperties: false
> +
> + properties:
> + "#address-cells":
> + const: 1
> +
> + "#size-cells":
> + const: 0
> +
> + patternProperties:
> + '^oldi@[0-1]$':
> + $ref: ti,am625-oldi.yaml#
> + description: OLDI transmitters connected to the DSS VPs
> +
> allOf:
> - if:
> properties:
> @@ -120,10 +157,25 @@ allOf:
> const: ti,am62a7-dss
> then:
> properties:
> + oldi-transmitters: false
> ports:
> properties:
> port@0: false
>
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: ti,am65x-dss
> + then:
> + properties:
> + oldi-transmitters: false
> + ports:
> + properties:
> + port@0:
> + properties:
> + endpoint@1: false
> +
> required:
> - compatible
> - reg
> @@ -171,3 +223,105 @@ examples:
> };
> };
> };
> +
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + #include <dt-bindings/interrupt-controller/irq.h>
> + #include <dt-bindings/soc/ti,sci_pm_domain.h>
> +
> + bus {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + dss1: dss@30200000 {
> + compatible = "ti,am625-dss";
> + reg = <0x00 0x30200000 0x00 0x1000>, /* common */
> + <0x00 0x30202000 0x00 0x1000>, /* vidl1 */
> + <0x00 0x30206000 0x00 0x1000>, /* vid */
> + <0x00 0x30207000 0x00 0x1000>, /* ovr1 */
> + <0x00 0x30208000 0x00 0x1000>, /* ovr2 */
> + <0x00 0x3020a000 0x00 0x1000>, /* vp1 */
> + <0x00 0x3020b000 0x00 0x1000>, /* vp2 */
> + <0x00 0x30201000 0x00 0x1000>; /* common1 */
> + reg-names = "common", "vidl1", "vid",
> + "ovr1", "ovr2", "vp1", "vp2", "common1";
> + power-domains = <&k3_pds 186 TI_SCI_PD_EXCLUSIVE>;
> + clocks = <&k3_clks 186 6>,
> + <&vp1_clock>,
> + <&k3_clks 186 2>;
> + clock-names = "fck", "vp1", "vp2";
> + interrupts = <GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH>;
> + oldi-transmitters {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + oldi0: oldi@0 {
> + reg = <0>;
> + clocks = <&k3_clks 186 0>;
> + clock-names = "serial";
> + ti,companion-oldi = <&oldi1>;
> + ti,oldi-io-ctrl = <&dss_oldi_io_ctrl>;
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + port@0 {
> + reg = <0>;
> + oldi0_in: endpoint {
> + remote-endpoint = <&dpi0_out0>;
> + };
> + };
> + port@1 {
> + reg = <1>;
> + oldi0_out: endpoint {
> + remote-endpoint = <&panel_in0>;
> + };
> + };
> + };
> + };
> + oldi1: oldi@1 {
> + reg = <1>;
> + clocks = <&k3_clks 186 0>;
> + clock-names = "serial";
> + ti,secondary-oldi;
> + ti,oldi-io-ctrl = <&dss_oldi_io_ctrl>;
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + port@0 {
> + reg = <0>;
> + oldi1_in: endpoint {
> + remote-endpoint = <&dpi0_out1>;
> + };
> + };
> + port@1 {
> + reg = <1>;
> + oldi1_out: endpoint {
> + remote-endpoint = <&panel_in1>;
> + };
> + };
> + };
> + };
> + };
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + port@0 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0>;
> + dpi0_out0: endpoint@0 {
> + reg = <0>;
> + remote-endpoint = <&oldi0_in>;
> + };
> + dpi0_out1: endpoint@1 {
> + reg = <1>;
> + remote-endpoint = <&oldi1_in>;
> + };
> + };
> + port@1 {
> + reg = <1>;
> + dpi1_out: endpoint {
> + remote-endpoint = <&hdmi_bridge>;
> + };
> + };
> + };
> + };
> + };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 0fa7c5728f1e..88fa2d9435b8 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7816,6 +7816,7 @@ M: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> L: dri-devel@lists.freedesktop.org
> S: Maintained
> T: git https://gitlab.freedesktop.org/drm/misc/kernel.git
> +F: Documentation/devicetree/bindings/display/ti/ti,am625-oldi.yaml
> F: Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
> F: Documentation/devicetree/bindings/display/ti/ti,j721e-dss.yaml
> F: Documentation/devicetree/bindings/display/ti/ti,k2g-dss.yaml
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v5 3/3] drm/tidss: Add OLDI bridge support
2025-02-11 10:57 ` Tomi Valkeinen
@ 2025-02-11 12:25 ` Tomi Valkeinen
0 siblings, 0 replies; 12+ messages in thread
From: Tomi Valkeinen @ 2025-02-11 12:25 UTC (permalink / raw)
To: Aradhya Bhatia
Cc: Maarten Lankhorst, Thomas Zimmermann, Maxime Ripard, David Airlie,
Laurent Pinchart, Simona Vetter, Nishanth Menon,
Vignesh Raghavendra, Devarsh Thakkar, Praneeth Bajjuri,
Udit Kumar, Jayesh Choudhary, Francesco Dolcini,
DRI Development List, Devicetree List, Linux Kernel List,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jyri Sarha
Hi,
On 11/02/2025 12:57, Tomi Valkeinen wrote:
> Hi,
>
> On 09/02/2025 18:09, Aradhya Bhatia wrote:
>> From: Aradhya Bhatia <a-bhatia1@ti.com>
>>
>> The AM62x and AM62Px SoCs feature 2 OLDI TXes each, which makes it
>> possible to connect them in dual-link or cloned single-link OLDI display
>> modes. The current OLDI support in tidss_dispc.c can only support for
>> a single OLDI TX, connected to a VP and doesn't really support
>> configuration of OLDIs in the other modes. The current OLDI support in
>> tidss_dispc.c also works on the principle that the OLDI output can only
>> be served by one, and only one, DSS video-port. This isn't the case in
>> the AM62Px SoC, where there are 2 DSS controllers present that share the
>> OLDI TXes.
>>
>> Having their own devicetree and their own bridge entity will help
>> support the various display modes and sharing possiblilities of the OLDI
>> hardware.
>>
>> For all these reasons, add support for the OLDI TXes as DRM bridges.
>>
>> Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
>> Signed-off-by: Aradhya Bhatia <aradhya.bhatia@linux.dev>
>> ---
>> drivers/gpu/drm/tidss/Makefile | 3 +-
>> drivers/gpu/drm/tidss/tidss_dispc.c | 20 +-
>> drivers/gpu/drm/tidss/tidss_dispc.h | 4 +
>> drivers/gpu/drm/tidss/tidss_dispc_regs.h | 14 +
>> drivers/gpu/drm/tidss/tidss_drv.c | 9 +
>> drivers/gpu/drm/tidss/tidss_drv.h | 5 +
>> drivers/gpu/drm/tidss/tidss_oldi.c | 558 +++++++++++++++++++++++
>> drivers/gpu/drm/tidss/tidss_oldi.h | 51 +++
>> 8 files changed, 662 insertions(+), 2 deletions(-)
>> create mode 100644 drivers/gpu/drm/tidss/tidss_oldi.c
>> create mode 100644 drivers/gpu/drm/tidss/tidss_oldi.h
>>
>> diff --git a/drivers/gpu/drm/tidss/Makefile b/drivers/gpu/drm/tidss/
>> Makefile
>> index 312645271014..b6d6becf1683 100644
>> --- a/drivers/gpu/drm/tidss/Makefile
>> +++ b/drivers/gpu/drm/tidss/Makefile
>> @@ -7,6 +7,7 @@ tidss-y := tidss_crtc.o \
>> tidss_irq.o \
>> tidss_plane.o \
>> tidss_scale_coefs.o \
>> - tidss_dispc.o
>> + tidss_dispc.o \
>> + tidss_oldi.o
>> obj-$(CONFIG_DRM_TIDSS) += tidss.o
>> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c b/drivers/gpu/drm/
>> tidss/tidss_dispc.c
>> index 1ad711f8d2a8..8631a89e6155 100644
>> --- a/drivers/gpu/drm/tidss/tidss_dispc.c
>> +++ b/drivers/gpu/drm/tidss/tidss_dispc.c
>> @@ -466,6 +466,25 @@ static u32 dispc_vp_read(struct dispc_device
>> *dispc, u32 hw_videoport, u16 reg)
>> return ioread32(base + reg);
>> }
>> +void tidss_configure_oldi(struct tidss_device *tidss, u32 hw_videoport,
>> + u32 oldi_cfg)
>> +{
>> + u32 count = 0;
>> + u32 oldi_reset_bit = BIT(5 + hw_videoport);
>> +
>> + dispc_vp_write(tidss->dispc, hw_videoport, DISPC_VP_DSS_OLDI_CFG,
>> oldi_cfg);
>> +
>> + if (oldi_cfg != 0) {
>> + while (!(oldi_reset_bit & dispc_read(tidss->dispc,
>> DSS_SYSSTATUS)) &&
>> + count < 10000)
>> + count++;
>> +
>> + if (!(oldi_reset_bit & dispc_read(tidss->dispc, DSS_SYSSTATUS)))
>> + dev_warn(tidss->dispc->dev, "%s: timeout waiting OLDI
>> reset done\n",
>> + __func__);
>> + }
>> +}
>
> The timeout sounds like an error. Better to return an error value, and
> handle it in tidss_oldi_config()?
>
> You could also
>
> if (!oldi_cfg)
> return 0;
>
> But would it actually be nicer to have a separate void function for
> disabling?
>
>> +
>> /*
>> * TRM gives bitfields as start:end, where start is the higher bit
>> * number. For example 7:0
>> @@ -1310,7 +1329,6 @@ void dispc_vp_disable_clk(struct dispc_device
>> *dispc, u32 hw_videoport)
>> * Calculate the percentage difference between the requested pixel
>> clock rate
>> * and the effective rate resulting from calculating the clock
>> divider value.
>> */
>> -static
>> unsigned int dispc_pclk_diff(unsigned long rate, unsigned long
>> real_rate)
>> {
>> int r = rate / 100, rr = real_rate / 100;
>> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.h b/drivers/gpu/drm/
>> tidss/tidss_dispc.h
>> index 086327d51a90..fab248f2055a 100644
>> --- a/drivers/gpu/drm/tidss/tidss_dispc.h
>> +++ b/drivers/gpu/drm/tidss/tidss_dispc.h
>> @@ -94,6 +94,10 @@ extern const struct dispc_features dispc_am62a7_feats;
>> extern const struct dispc_features dispc_am65x_feats;
>> extern const struct dispc_features dispc_j721e_feats;
>> +void tidss_configure_oldi(struct tidss_device *tidss, u32 hw_videoport,
>> + u32 oldi_cfg);
>> +unsigned int dispc_pclk_diff(unsigned long rate, unsigned long
>> real_rate);
>> +
>> void dispc_set_irqenable(struct dispc_device *dispc, dispc_irq_t mask);
>> dispc_irq_t dispc_read_and_clear_irqstatus(struct dispc_device *dispc);
>> diff --git a/drivers/gpu/drm/tidss/tidss_dispc_regs.h b/drivers/gpu/
>> drm/tidss/tidss_dispc_regs.h
>> index 13feedfe5d6d..03f7098029e6 100644
>> --- a/drivers/gpu/drm/tidss/tidss_dispc_regs.h
>> +++ b/drivers/gpu/drm/tidss/tidss_dispc_regs.h
>> @@ -226,6 +226,20 @@ enum dispc_common_regs {
>> #define DISPC_VP_DSS_DMA_THREADSIZE 0x170 /* J721E */
>> #define DISPC_VP_DSS_DMA_THREADSIZE_STATUS 0x174 /* J721E */
>> +/* OLDI Config Bits (DISPC_VP_DSS_OLDI_CFG) */
>> +#define OLDI_ENABLE BIT(0)
>> +#define OLDI_MAP (BIT(1) | BIT(2) | BIT(3))
>> +#define OLDI_SRC BIT(4)
>> +#define OLDI_CLONE_MODE BIT(5)
>> +#define OLDI_MASTERSLAVE BIT(6)
>> +#define OLDI_DEPOL BIT(7)
>> +#define OLDI_MSB BIT(8)
>> +#define OLDI_LBEN BIT(9)
>> +#define OLDI_LBDATA BIT(10)
>> +#define OLDI_DUALMODESYNC BIT(11)
>> +#define OLDI_SOFTRST BIT(12)
>> +#define OLDI_TPATCFG BIT(13)
>> +
>> /*
>> * OLDI IO_CTRL register offsets. On AM654 the registers are found
>> * from CTRL_MMR0, there the syscon regmap should map 0x14 bytes from
>> diff --git a/drivers/gpu/drm/tidss/tidss_drv.c b/drivers/gpu/drm/
>> tidss/tidss_drv.c
>> index 7c8fd6407d82..27b9f86f1eb2 100644
>> --- a/drivers/gpu/drm/tidss/tidss_drv.c
>> +++ b/drivers/gpu/drm/tidss/tidss_drv.c
>> @@ -24,6 +24,7 @@
>> #include "tidss_drv.h"
>> #include "tidss_kms.h"
>> #include "tidss_irq.h"
>> +#include "tidss_oldi.h"
>> /* Power management */
>> @@ -148,6 +149,10 @@ static int tidss_probe(struct platform_device *pdev)
>> return ret;
>> }
>> + ret = tidss_oldi_init(tidss);
>> + if (ret)
>> + return dev_err_probe(dev, ret, "failed to init OLDI\n");
>> +
>> pm_runtime_enable(dev);
>> pm_runtime_set_autosuspend_delay(dev, 1000);
>> @@ -204,6 +209,8 @@ static int tidss_probe(struct platform_device *pdev)
>> pm_runtime_dont_use_autosuspend(dev);
>> pm_runtime_disable(dev);
>> + tidss_oldi_deinit(tidss);
>> +
>> return ret;
>> }
>> @@ -228,6 +235,8 @@ static void tidss_remove(struct platform_device
>> *pdev)
>> pm_runtime_dont_use_autosuspend(dev);
>> pm_runtime_disable(dev);
>> + tidss_oldi_deinit(tidss);
>> +
>> /* devm allocated dispc goes away with the dev so mark it NULL */
>> dispc_remove(tidss);
>> diff --git a/drivers/gpu/drm/tidss/tidss_drv.h b/drivers/gpu/drm/
>> tidss/tidss_drv.h
>> index d7f27b0b0315..6c0fe1d989ee 100644
>> --- a/drivers/gpu/drm/tidss/tidss_drv.h
>> +++ b/drivers/gpu/drm/tidss/tidss_drv.h
>> @@ -11,8 +11,10 @@
>> #define TIDSS_MAX_PORTS 4
>> #define TIDSS_MAX_PLANES 4
>> +#define TIDSS_MAX_OLDI_TXES 2
>> typedef u32 dispc_irq_t;
>> +struct tidss_oldi;
>> struct tidss_device {
>> struct drm_device ddev; /* DRM device for DSS */
>> @@ -27,6 +29,9 @@ struct tidss_device {
>> unsigned int num_planes;
>> struct drm_plane *planes[TIDSS_MAX_PLANES];
>> + unsigned int num_oldis;
>> + struct tidss_oldi *oldis[TIDSS_MAX_OLDI_TXES];
>> +
>> unsigned int irq;
>> spinlock_t wait_lock; /* protects the irq masks */
>> diff --git a/drivers/gpu/drm/tidss/tidss_oldi.c b/drivers/gpu/drm/
>> tidss/tidss_oldi.c
>> new file mode 100644
>> index 000000000000..4af13a01f546
>> --- /dev/null
>> +++ b/drivers/gpu/drm/tidss/tidss_oldi.c
>> @@ -0,0 +1,558 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Copyright (C) 2024 - Texas Instruments Incorporated
>> + *
>> + * Aradhya Bhatia <a-bhatia1@ti.com>
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/of.h>
>> +#include <linux/of_graph.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/media-bus-format.h>
>> +#include <linux/regmap.h>
>> +
>> +#include <drm/drm_atomic_helper.h>
>> +#include <drm/drm_bridge.h>
>> +#include <drm/drm_of.h>
>> +
>> +#include "tidss_dispc.h"
>> +#include "tidss_dispc_regs.h"
>> +#include "tidss_oldi.h"
>> +
>> +struct tidss_oldi {
>> + struct tidss_device *tidss;
>> + struct device *dev;
>> +
>> + struct drm_bridge bridge;
>> + struct drm_bridge *next_bridge;
>> +
>> + enum tidss_oldi_link_type link_type;
>> + const struct oldi_bus_format *bus_format;
>> + u32 oldi_instance;
>> + u32 companion_instance;
>> + u32 parent_vp;
>> +
>> + struct clk *serial;
>> + struct regmap *io_ctrl;
>> +};
>> +
>> +static const struct oldi_bus_format oldi_bus_formats[] = {
>> + { MEDIA_BUS_FMT_RGB666_1X7X3_SPWG, 18, SPWG_18,
>> MEDIA_BUS_FMT_RGB666_1X18 },
>> + { MEDIA_BUS_FMT_RGB888_1X7X4_SPWG, 24, SPWG_24,
>> MEDIA_BUS_FMT_RGB888_1X24 },
>> + { MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA, 24, JEIDA_24,
>> MEDIA_BUS_FMT_RGB888_1X24 },
>> +};
>> +
>> +#define OLDI_IDLE_CLK_HZ 25000000 /*25 MHz */
>> +
>> +static inline struct tidss_oldi *
>> +drm_bridge_to_tidss_oldi(struct drm_bridge *bridge)
>> +{
>> + return container_of(bridge, struct tidss_oldi, bridge);
>> +}
>> +
>> +static int tidss_oldi_bridge_attach(struct drm_bridge *bridge,
>> + enum drm_bridge_attach_flags flags)
>> +{
>> + struct tidss_oldi *oldi = drm_bridge_to_tidss_oldi(bridge);
>> +
>> + if (!oldi->next_bridge) {
>> + dev_err(oldi->dev,
>> + "%s: OLDI%u Failure attach next bridge\n",
>> + __func__, oldi->oldi_instance);
>> + return -ENODEV;
>> + }
>> +
>> + if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)) {
>> + dev_err(oldi->dev,
>> + "%s: OLDI%u DRM_BRIDGE_ATTACH_NO_CONNECTOR is mandatory.\n",
>> + __func__, oldi->oldi_instance);
>> + return -EINVAL;
>> + }
>> +
>> + return drm_bridge_attach(bridge->encoder, oldi->next_bridge,
>> + bridge, flags);
>> +}
>> +
>> +static int
>> +tidss_oldi_set_serial_clk(struct tidss_oldi *oldi, unsigned long rate)
>> +{
>> + unsigned long new_rate;
>> + int ret;
>> +
>> + ret = clk_set_rate(oldi->serial, rate);
>> + if (ret) {
>> + dev_err(oldi->dev,
>> + "OLDI%u: failed to set serial clk rate to %lu Hz\n",
>> + oldi->oldi_instance, rate);
>> + return ret;
>> + }
>> +
>> + new_rate = clk_get_rate(oldi->serial);
>> +
>> + if (dispc_pclk_diff(rate, new_rate) > 5)
>> + dev_warn(oldi->dev,
>> + "OLDI%u Clock rate %lu differs over 5%% from requested
>> %lu\n",
>> + oldi->oldi_instance, new_rate, rate);
>> +
>> + dev_dbg(oldi->dev, "OLDI%u: new rate %lu Hz (requested %lu Hz)\n",
>> + oldi->oldi_instance, clk_get_rate(oldi->serial), rate);
>> +
>> + return 0;
>> +}
>> +
>> +static void tidss_oldi_tx_power(struct tidss_oldi *oldi, bool enable)
>> +{
>> + u32 mask;
>> +
>> + /*
>> + * The power control bits are Active Low, and remain powered off by
>> + * default. That is, the bits are set to 1. To power on the OLDI
>> TXes,
>> + * the bits must be cleared to 0. Since there are cases where not
>> all
>> + * OLDI TXes are being used, the power logic selectively powers them
>> + * on.
>> + * Setting the variable 'val' to particular bit masks, makes sure
>> that
>> + * the unrequired OLDI TXes remain powered off.
>> + */
>> +
>> + if (enable) {
>> + switch (oldi->link_type) {
>> + case OLDI_MODE_SINGLE_LINK:
>> + /* Power-on only the required OLDI TX's IO*/
>> + mask = OLDI_PWRDOWN_TX(oldi->oldi_instance) | OLDI_PWRDN_BG;
>> + break;
>> + case OLDI_MODE_CLONE_SINGLE_LINK:
>> + case OLDI_MODE_DUAL_LINK:
>> + /* Power-on both the OLDI TXes' IOs */
>> + mask = OLDI_PWRDOWN_TX(oldi->oldi_instance) |
>> + OLDI_PWRDOWN_TX(oldi->companion_instance) |
>> + OLDI_PWRDN_BG;
>> + break;
>> + default:
>> + /*
>> + * This code execution should never reach here as any
>> + * OLDI with an unsupported OLDI mode would never get
>> + * registered in the first place.
>> + * However, power-off the OLDI in concern just in case.
>> + */
>> + mask = OLDI_PWRDOWN_TX(oldi->oldi_instance);
>> + enable = false;
>> + break;
>> + }
>> + } else {
>> + switch (oldi->link_type) {
>> + case OLDI_MODE_CLONE_SINGLE_LINK:
>> + case OLDI_MODE_DUAL_LINK:
>> + mask = OLDI_PWRDOWN_TX(oldi->oldi_instance) |
>> + OLDI_PWRDOWN_TX(oldi->companion_instance) |
>> + OLDI_PWRDN_BG;
>> + break;
>> + case OLDI_MODE_SINGLE_LINK:
>> + default:
>> + mask = OLDI_PWRDOWN_TX(oldi->oldi_instance);
>> + break;
>> + }
>> + }
>> +
>> + regmap_update_bits(oldi->io_ctrl, OLDI_PD_CTRL, mask, enable ?
>> 0 : mask);
>> +}
>> +
>> +static int tidss_oldi_config(struct tidss_oldi *oldi)
>> +{
>> + const struct oldi_bus_format *bus_fmt = NULL;
>> + u32 oldi_cfg = 0;
>> +
>> + bus_fmt = oldi->bus_format;
>> +
>> + /*
>> + * MASTERSLAVE and SRC bits of OLDI Config are always set to 0.
>> + */
>> +
>> + if (bus_fmt->data_width == 24)
>> + oldi_cfg |= OLDI_MSB;
>> + else if (bus_fmt->data_width != 18)
>> + dev_warn(oldi->dev,
>> + "OLDI%u: DSS port width %d not supported\n",
>> + oldi->oldi_instance, bus_fmt->data_width);
>> +
>> + oldi_cfg |= OLDI_DEPOL;
>> +
>> + oldi_cfg = (oldi_cfg & (~OLDI_MAP)) | (bus_fmt->oldi_mode_reg_val
>> << 1);
>> +
>> + oldi_cfg |= OLDI_SOFTRST;
>> +
>> + oldi_cfg |= OLDI_ENABLE;
>> +
>> + switch (oldi->link_type) {
>> + case OLDI_MODE_SINGLE_LINK:
>> + /* All configuration is done for this mode. */
>> + break;
>> +
>> + case OLDI_MODE_CLONE_SINGLE_LINK:
>> + oldi_cfg |= OLDI_CLONE_MODE;
>> + break;
>> +
>> + case OLDI_MODE_DUAL_LINK:
>> + /* data-mapping field also indicates dual-link mode */
>> + oldi_cfg |= BIT(3);
>> + oldi_cfg |= OLDI_DUALMODESYNC;
>> + break;
>> +
>> + default:
>> + dev_err(oldi->dev, "OLDI%u: Unsupported mode.\n",
>> + oldi->oldi_instance);
>> + return -EINVAL;
>> + }
>> +
>> + tidss_configure_oldi(oldi->tidss, oldi->parent_vp, oldi_cfg);
>> +
>> + return 0;
>> +}
>> +
>> +static void tidss_oldi_atomic_pre_enable(struct drm_bridge *bridge,
>> + struct drm_bridge_state *old_bridge_state)
>> +{
>> + struct tidss_oldi *oldi = drm_bridge_to_tidss_oldi(bridge);
>> + struct drm_atomic_state *state = old_bridge_state->base.state;
>> + struct drm_connector *connector;
>> + struct drm_connector_state *conn_state;
>> + struct drm_crtc_state *crtc_state;
>> + struct drm_display_mode *mode;
>> +
>> + connector = drm_atomic_get_new_connector_for_encoder(state,
>> + bridge->encoder);
>> + if (WARN_ON(!connector))
>> + return;
>> +
>> + conn_state = drm_atomic_get_new_connector_state(state, connector);
>> + if (WARN_ON(!conn_state))
>> + return;
>> +
>> + crtc_state = drm_atomic_get_new_crtc_state(state, conn_state->crtc);
>> + if (WARN_ON(!crtc_state))
>> + return;
>> +
>> + mode = &crtc_state->adjusted_mode;
>> +
>> + /* Configure the OLDI params*/
>> + tidss_oldi_config(oldi);
>> +
>> + /* Set the OLDI serial clock (7 times the pixel clock) */
>> + tidss_oldi_set_serial_clk(oldi, mode->clock * 7 * 1000);
>> +
>> + /* Enable OLDI IO power */
>> + tidss_oldi_tx_power(oldi, true);
>> +}
>> +
>> +static void tidss_oldi_atomic_post_disable(struct drm_bridge *bridge,
>> + struct drm_bridge_state *old_bridge_state)
>> +{
>> + struct tidss_oldi *oldi = drm_bridge_to_tidss_oldi(bridge);
>> +
>> + /* Disable OLDI IO power */
>> + tidss_oldi_tx_power(oldi, false);
>> +
>> + /* Set the OLDI serial clock to IDLE Frequency */
>> + tidss_oldi_set_serial_clk(oldi, OLDI_IDLE_CLK_HZ);
>> +
>> + /* Clear OLDI Config */
>> + tidss_configure_oldi(oldi->tidss, oldi->parent_vp, 0);
>> +}
>> +
>> +#define MAX_INPUT_SEL_FORMATS 1
>> +
>> +static u32 *tidss_oldi_atomic_get_input_bus_fmts(struct drm_bridge
>> *bridge,
>> + struct drm_bridge_state *bridge_state,
>> + struct drm_crtc_state *crtc_state,
>> + struct drm_connector_state *conn_state,
>> + u32 output_fmt,
>> + unsigned int *num_input_fmts)
>> +{
>> + struct tidss_oldi *oldi = drm_bridge_to_tidss_oldi(bridge);
>> + u32 *input_fmts;
>> + int i;
>> +
>> + *num_input_fmts = 0;
>> +
>> + for (i = 0; i < ARRAY_SIZE(oldi_bus_formats); i++)
>> + if (oldi_bus_formats[i].bus_fmt == output_fmt)
>> + break;
>> +
>> + if (i == ARRAY_SIZE(oldi_bus_formats))
>> + return NULL;
>> +
>> + input_fmts = kcalloc(MAX_INPUT_SEL_FORMATS, sizeof(*input_fmts),
>> + GFP_KERNEL);
>> + if (!input_fmts)
>> + return NULL;
>> +
>> + *num_input_fmts = 1;
>> + input_fmts[0] = oldi_bus_formats[i].input_bus_fmt;
>> + oldi->bus_format = &oldi_bus_formats[i];
>> +
>> + return input_fmts;
>> +}
>> +
>> +static const struct drm_bridge_funcs tidss_oldi_bridge_funcs = {
>> + .attach = tidss_oldi_bridge_attach,
>
> Looks like an extra tab there?
>
> Other than those two cosmetic issues, I think this looks fine. I also
> tested on AM62-SK.
Also, feel free to add:
Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Tomi
> However, one more thing. We'll have a separate OLDI bridge, but we still
> will have the old OLDI code for AM65x in the tidss_dispc.c. And to mix
> things up, we will have some new OLDI code there too
> (tidss_configure_oldi). Could you check the AM65x specific code and
> perhaps rename the functions to am65x or such, to make this clearer.
> Perhaps also the DISPC_VP_OLDI should be DISPC_VP_OLDI_AM65X, as that VP
> type shouldn't be used for anything else.
>
> Tomi
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v5 2/3] dt-bindings: display: ti: Add schema for AM625 OLDI Transmitter
2025-02-11 12:24 ` Tomi Valkeinen
@ 2025-02-13 12:33 ` Aradhya Bhatia
2025-02-13 13:20 ` Tomi Valkeinen
0 siblings, 1 reply; 12+ messages in thread
From: Aradhya Bhatia @ 2025-02-13 12:33 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Maarten Lankhorst, Thomas Zimmermann, Maxime Ripard, David Airlie,
Laurent Pinchart, Simona Vetter, Nishanth Menon,
Vignesh Raghavendra, Devarsh Thakkar, Praneeth Bajjuri,
Udit Kumar, Jayesh Choudhary, Francesco Dolcini,
DRI Development List, Devicetree List, Linux Kernel List,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jyri Sarha
Hi Tomi,
Thank you for reviewing the patches!
On 11/02/25 17:54, Tomi Valkeinen wrote:
> Hi,
>
> On 09/02/2025 18:09, Aradhya Bhatia wrote:
>> From: Aradhya Bhatia <a-bhatia1@ti.com>
>>
>> The OLDI transmitters (TXes) do not have registers of their own, and are
>> dependent on the source video-ports (VPs) from the DSS to provide
>> configuration data. This hardware doesn't directly sit on the internal
>> bus of the SoC, but does so via the DSS. Hence, the OLDI TXes are
>> supposed to be child nodes under the DSS, and not independent devices.
>>
>> Two of the OLDI TXes can function in tandem to output dual-link OLDI
>> output, or cloned single-link outputs. In these cases, one OLDI will be
>> the primary OLDI, and the other one, a companion. The following diagram
>> represents such a configuration.
>>
>> +-----+-----+ +-------+
>> | | | | |
>> | | VP1 +----+--->+ OLDI0 | (Primary - may need companion)
>> | | | | | |
>> | DSS +-----+ | +-------+
>> | | | |
>> | | VP2 | | +-------+
>> | | | | | |
>> +-----+-----+ +--->+ OLDI1 | (Companion OLDI)
>> | |
>> +-------+
>>
>> The DSS in AM625 SoC has a configuration like the one above. The AM625
>> DSS VP1 (port@0) can connect and control 2 OLDI TXes, to use them in
>> dual-link or cloned single-link OLDI modes. It is only the VP1 that can
>> connect to either OLDI TXes for the AM625 DSS, and not the VP2.
>>
>> Alternatively, on some future TI SoCs, along with the above
>> configuration, the OLDI TX can _also_ connect to separate video sources,
>> making them work entirely independent of each other. In this case,
>> neither of the OLDIs are "companion" or "secondary" OLDIs, and nor do
>> they require one. They both are independent and primary OLDIs. The
>> following diagram represents such a configuration.
>>
>> +-----+-----+ +-------+
>> | | | | |
>> | | VP1 +--+----------->+ OLDI0 | (Primary - may need companion)
>> | | | | | |
>> | +-----+ | +-------+
>> | | | |
>> | | VP2 | |
>> | | | |
>> | DSS +-----+ | +---+ +-------+
>> | | | +-->+ M | | |
>> | | VP3 +----->+ U +--->+ OLDI1 | (Companion or Primary)
>> | | | | X | | |
>> | +-----+ +---+ +-------+
>> | | |
>> | | VP4 |
>> | | |
>> +-----+-----+
>>
>> Note that depending on the mux configuration, the OLDIs can either be
>> working together in tandem - sourced by VP1, OR, they could be working
>> independently sourced by VP1 and VP3 respectively.
>> The idea is to support all the configurations with this OLDI TX schema.
>>
>> The OLDI functionality is further supported by a system-control module,
>> which contains a few registers to control OLDI IO power and other
>> electrical characteristics of the IO lanes.
>>
>> Add devicetree binding schema for the OLDI TXes to support various
>> configurations, and extend their support to the AM625 DSS.
>>
>> Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
>> Signed-off-by: Aradhya Bhatia <aradhya.bhatia@linux.dev>
>> ---
>> .../bindings/display/ti/ti,am625-oldi.yaml | 88 ++++++++++
>> .../bindings/display/ti/ti,am65x-dss.yaml | 154 ++++++++++++++++++
>> MAINTAINERS | 1 +
>> 3 files changed, 243 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/display/ti/
>> ti,am625-oldi.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/display/ti/ti,am625-
>> oldi.yaml b/Documentation/devicetree/bindings/display/ti/ti,am625-
>> oldi.yaml
>> new file mode 100644
>> index 000000000000..42a80a512660
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/ti/ti,am625-oldi.yaml
>> @@ -0,0 +1,88 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/display/ti/ti,am625-oldi.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Texas Instruments AM625 OLDI Transmitter
>> +
>> +maintainers:
>> + - Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>> + - Aradhya Bhatia <aradhya.bhatia@linux.dev>
>> +
>> +description:
>> + The AM625 TI Keystone OpenLDI transmitter (OLDI TX) supports
>> serialized RGB
>> + pixel data transmission between host and flat panel display over
>> LVDS (Low
>> + Voltage Differential Sampling) interface. The OLDI TX consists of
>> 7-to-1 data
>> + serializers, and 4-data and 1-clock LVDS outputs. It supports the
>> LVDS output
>> + formats "jeida-18", "jeida-24" and "vesa-18", and can accept 24-bit
>> RGB or
>> + padded and un-padded 18-bit RGB bus formats as input.
>> +
>> +properties:
>> + reg:
>> + maxItems: 1
>> +
>> + clocks:
>> + maxItems: 1
>> + description: serial clock input for the OLDI transmitters
>> +
>> + clock-names:
>> + const: serial
>> +
>> + ti,companion-oldi:
>> + $ref: /schemas/types.yaml#/definitions/phandle
>> + description:
>> + phandle to companion OLDI transmitter. This property is
>> mandatory for the
>> + primarty OLDI TX if the OLDI TXes are expected to work either
>> in dual-lvds
>> + mode or in clone mode. This property should point to the
>> secondary OLDI
>> + TX.
>> +
>> + ti,secondary-oldi:
>> + type: boolean
>> + description:
>> + Boolean property to mark the OLDI transmitter as the secondary
>> one, when the
>> + OLDI hardware is expected to run as a companion HW, in cases of
>> dual-lvds
>> + mode or clone mode. The primary OLDI hardware is responsible
>> for all the
>> + hardware configuration.
>
> I think these work, but I'm wondering if we would ever need to check
> something from the main oldi from the secondary oldi. In that case
> "crossed phandles" would be better, i.e. something like:
>
> (in the first oldi:)
> ti,slave-oldi = <phandle-to-second-oldi>
>
> (in the second oldi:)
> ti,master-oldi = <phandle-to-first-oldi>
When I had first designed the code and the devicetree for OLDI, it was
done so with the belief that we wouldn't reqiure a bridge instance for
the secondary OLDI, at all.
While that idea holds true for dual-lvds configuration, it doesn't so
for the clone mode configuration. For clone mode, as you pointed out, we
will require a 2nd bridge instance to configure any of the bridges and
panels that come after the 2nd OLDI.
>
> Then again, if we ever need that, even with these bindings the driver
> could find the first oldi, but needs to go via the dss's node.
While it is possible to do it this way, it might not be the cleanest
one. And _if_ there is a ever a DSS in future with more than 2 OLDI
TXes, say 4, then the decipher logic may get too complicated.
While I cannot think of any case where the secondary OLDI bridge DT
might need to access the primary OLDI bridge at the moment, I wonder if
we should play it safer and have this option anyway.
Maybe something like this?
(primary OLDI)
ti,primary-oldi;
ti,companion-oldi = <phandle-to-secondary-oldi>;
(secondary OLDI)
ti,secondary-oldi;
ti,companion-oldi = <phandle-to-primary-oldi>;
I will have to drop the condition below, and add ti,primary-oldi
property.
>
> So, just a thought.
>
> Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>
>
>> + ti,oldi-io-ctrl:
>> + $ref: /schemas/types.yaml#/definitions/phandle
>> + description:
>> + phandle to syscon device node mapping OLDI IO_CTRL registers
>> found in the
>> + control MMR region. These registers are required to toggle the
>> I/O lane
>> + power, and control its electrical characteristics.
>> +
>> + ports:
>> + $ref: /schemas/graph.yaml#/properties/ports
>> +
>> + properties:
>> + port@0:
>> + $ref: /schemas/graph.yaml#/properties/port
>> + description: Parallel RGB input port
>> +
>> + port@1:
>> + $ref: /schemas/graph.yaml#/properties/port
>> + description: LVDS output port
>> +
>> + required:
>> + - port@0
>> + - port@1
>> +
>> +allOf:
>> + - if:
>> + required:
>> + - ti,secondary-oldi
>> + then:
>> + properties:
>> + ti,companion-oldi: false
>> +
(this condition)
>> +required:
>> + - reg
>> + - clocks
>> + - clock-names
>> + - ti,oldi-io-ctrl
>> + - ports
>> +
>> +additionalProperties: false
>> +
>> +...
>> diff --git a/Documentation/devicetree/bindings/display/ti/ti,am65x-
>> dss.yaml b/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
>> index 399d68986326..a82c525631ea 100644
>> --- a/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
>> +++ b/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
>> @@ -91,6 +91,24 @@ properties:
>> For AM625 DSS, the internal DPI output port node from video
>> port 1.
>> For AM62A7 DSS, the port is tied off inside the SoC.
>> + properties:
>> + endpoint@0:
>> + $ref: /schemas/graph.yaml#/properties/endpoint
>> + description:
>> + For AM625 DSS, VP Connection to OLDI0.
>> + For AM65X DSS, OLDI output from the SoC.
>> +
>> + endpoint@1:
>> + $ref: /schemas/graph.yaml#/properties/endpoint
>> + description:
>> + For AM625 DSS, VP Connection to OLDI1.
>> +
>> + anyOf:
>> + - required:
>> + - endpoint
>> + - required:
>> + - endpoint@0
>> + - endpoint@1
>> port@1:
>> $ref: /schemas/graph.yaml#/properties/port
>> @@ -112,6 +130,25 @@ properties:
>> Input memory (from main memory to dispc) bandwidth limit in
>> bytes per second
>> + oldi-transmitters:
>> + description:
>> + Child node under the DSS, to describe all the OLDI transmitters
>> connected
>> + to the DSS videoports.
>> + type: object
>> + additionalProperties: false
>> +
>> + properties:
>> + "#address-cells":
>> + const: 1
>> +
>> + "#size-cells":
>> + const: 0
>> +
>> + patternProperties:
>> + '^oldi@[0-1]$':
>> + $ref: ti,am625-oldi.yaml#
>> + description: OLDI transmitters connected to the DSS VPs
>> +
>> allOf:
>> - if:
>> properties:
>> @@ -120,10 +157,25 @@ allOf:
>> const: ti,am62a7-dss
>> then:
>> properties:
>> + oldi-transmitters: false
>> ports:
>> properties:
>> port@0: false
>> + - if:
>> + properties:
>> + compatible:
>> + contains:
>> + const: ti,am65x-dss
>> + then:
>> + properties:
>> + oldi-transmitters: false
>> + ports:
>> + properties:
>> + port@0:
>> + properties:
>> + endpoint@1: false
>> +
>> required:
>> - compatible
>> - reg
>> @@ -171,3 +223,105 @@ examples:
>> };
>> };
>> };
>> +
>> + - |
>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
>> + #include <dt-bindings/interrupt-controller/irq.h>
>> + #include <dt-bindings/soc/ti,sci_pm_domain.h>
>> +
>> + bus {
>> + #address-cells = <2>;
>> + #size-cells = <2>;
>> + dss1: dss@30200000 {
>> + compatible = "ti,am625-dss";
>> + reg = <0x00 0x30200000 0x00 0x1000>, /* common */
>> + <0x00 0x30202000 0x00 0x1000>, /* vidl1 */
>> + <0x00 0x30206000 0x00 0x1000>, /* vid */
>> + <0x00 0x30207000 0x00 0x1000>, /* ovr1 */
>> + <0x00 0x30208000 0x00 0x1000>, /* ovr2 */
>> + <0x00 0x3020a000 0x00 0x1000>, /* vp1 */
>> + <0x00 0x3020b000 0x00 0x1000>, /* vp2 */
>> + <0x00 0x30201000 0x00 0x1000>; /* common1 */
>> + reg-names = "common", "vidl1", "vid",
>> + "ovr1", "ovr2", "vp1", "vp2", "common1";
>> + power-domains = <&k3_pds 186 TI_SCI_PD_EXCLUSIVE>;
>> + clocks = <&k3_clks 186 6>,
>> + <&vp1_clock>,
>> + <&k3_clks 186 2>;
>> + clock-names = "fck", "vp1", "vp2";
>> + interrupts = <GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH>;
>> + oldi-transmitters {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + oldi0: oldi@0 {
>> + reg = <0>;
>> + clocks = <&k3_clks 186 0>;
>> + clock-names = "serial";
>> + ti,companion-oldi = <&oldi1>;
>> + ti,oldi-io-ctrl = <&dss_oldi_io_ctrl>;
>> + ports {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + port@0 {
>> + reg = <0>;
>> + oldi0_in: endpoint {
>> + remote-endpoint = <&dpi0_out0>;
>> + };
>> + };
>> + port@1 {
>> + reg = <1>;
>> + oldi0_out: endpoint {
>> + remote-endpoint = <&panel_in0>;
>> + };
>> + };
>> + };
>> + };
>> + oldi1: oldi@1 {
>> + reg = <1>;
>> + clocks = <&k3_clks 186 0>;
>> + clock-names = "serial";
>> + ti,secondary-oldi;
>> + ti,oldi-io-ctrl = <&dss_oldi_io_ctrl>;
>> + ports {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + port@0 {
>> + reg = <0>;
>> + oldi1_in: endpoint {
>> + remote-endpoint = <&dpi0_out1>;
>> + };
>> + };
>> + port@1 {
>> + reg = <1>;
>> + oldi1_out: endpoint {
>> + remote-endpoint = <&panel_in1>;
>> + };
>> + };
>> + };
>> + };
>> + };
>> + ports {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + port@0 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + reg = <0>;
>> + dpi0_out0: endpoint@0 {
>> + reg = <0>;
>> + remote-endpoint = <&oldi0_in>;
>> + };
>> + dpi0_out1: endpoint@1 {
>> + reg = <1>;
>> + remote-endpoint = <&oldi1_in>;
>> + };
>> + };
>> + port@1 {
>> + reg = <1>;
>> + dpi1_out: endpoint {
>> + remote-endpoint = <&hdmi_bridge>;
>> + };
>> + };
>> + };
>> + };
>> + };
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 0fa7c5728f1e..88fa2d9435b8 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -7816,6 +7816,7 @@ M: Tomi Valkeinen
>> <tomi.valkeinen@ideasonboard.com>
>> L: dri-devel@lists.freedesktop.org
>> S: Maintained
>> T: git https://gitlab.freedesktop.org/drm/misc/kernel.git
>> +F: Documentation/devicetree/bindings/display/ti/ti,am625-oldi.yaml
>> F: Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
>> F: Documentation/devicetree/bindings/display/ti/ti,j721e-dss.yaml
>> F: Documentation/devicetree/bindings/display/ti/ti,k2g-dss.yaml
>
--
Regards
Aradhya
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v5 2/3] dt-bindings: display: ti: Add schema for AM625 OLDI Transmitter
2025-02-13 12:33 ` Aradhya Bhatia
@ 2025-02-13 13:20 ` Tomi Valkeinen
2025-02-14 12:41 ` Aradhya Bhatia
0 siblings, 1 reply; 12+ messages in thread
From: Tomi Valkeinen @ 2025-02-13 13:20 UTC (permalink / raw)
To: Aradhya Bhatia
Cc: Maarten Lankhorst, Thomas Zimmermann, Maxime Ripard, David Airlie,
Laurent Pinchart, Simona Vetter, Nishanth Menon,
Vignesh Raghavendra, Devarsh Thakkar, Praneeth Bajjuri,
Udit Kumar, Jayesh Choudhary, Francesco Dolcini,
DRI Development List, Devicetree List, Linux Kernel List,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jyri Sarha
Hi,
On 13/02/2025 14:33, Aradhya Bhatia wrote:
>>> + ti,companion-oldi:
>>> + $ref: /schemas/types.yaml#/definitions/phandle
>>> + description:
>>> + phandle to companion OLDI transmitter. This property is
>>> mandatory for the
>>> + primarty OLDI TX if the OLDI TXes are expected to work either
>>> in dual-lvds
>>> + mode or in clone mode. This property should point to the
>>> secondary OLDI
>>> + TX.
>>> +
>>> + ti,secondary-oldi:
>>> + type: boolean
>>> + description:
>>> + Boolean property to mark the OLDI transmitter as the secondary
>>> one, when the
>>> + OLDI hardware is expected to run as a companion HW, in cases of
>>> dual-lvds
>>> + mode or clone mode. The primary OLDI hardware is responsible
>>> for all the
>>> + hardware configuration.
>>
>> I think these work, but I'm wondering if we would ever need to check
>> something from the main oldi from the secondary oldi. In that case
>> "crossed phandles" would be better, i.e. something like:
>>
>> (in the first oldi:)
>> ti,slave-oldi = <phandle-to-second-oldi>
>>
>> (in the second oldi:)
>> ti,master-oldi = <phandle-to-first-oldi>
>
> When I had first designed the code and the devicetree for OLDI, it was
> done so with the belief that we wouldn't reqiure a bridge instance for
> the secondary OLDI, at all.
>
> While that idea holds true for dual-lvds configuration, it doesn't so
> for the clone mode configuration. For clone mode, as you pointed out, we
> will require a 2nd bridge instance to configure any of the bridges and
> panels that come after the 2nd OLDI.
>
>
>>
>> Then again, if we ever need that, even with these bindings the driver
>> could find the first oldi, but needs to go via the dss's node.
>
> While it is possible to do it this way, it might not be the cleanest
> one. And _if_ there is a ever a DSS in future with more than 2 OLDI
> TXes, say 4, then the decipher logic may get too complicated.
>
> While I cannot think of any case where the secondary OLDI bridge DT
> might need to access the primary OLDI bridge at the moment, I wonder if
> we should play it safer and have this option anyway.
>
> Maybe something like this?
>
> (primary OLDI)
> ti,primary-oldi;
> ti,companion-oldi = <phandle-to-secondary-oldi>;
>
> (secondary OLDI)
> ti,secondary-oldi;
> ti,companion-oldi = <phandle-to-primary-oldi>;
How is this different than my proposal, except a bit more verbose?
If you're thinking about a 4-OLDI hardware, how would this work there?
(but I want to say that even if it's good to plan for the future, we
shouldn't plan too much based on imaginary hardware =).
Tomi
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v5 2/3] dt-bindings: display: ti: Add schema for AM625 OLDI Transmitter
2025-02-13 13:20 ` Tomi Valkeinen
@ 2025-02-14 12:41 ` Aradhya Bhatia
2025-02-19 14:20 ` Rob Herring
0 siblings, 1 reply; 12+ messages in thread
From: Aradhya Bhatia @ 2025-02-14 12:41 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Maarten Lankhorst, Thomas Zimmermann, Maxime Ripard, David Airlie,
Laurent Pinchart, Simona Vetter, Nishanth Menon,
Vignesh Raghavendra, Devarsh Thakkar, Praneeth Bajjuri,
Udit Kumar, Jayesh Choudhary, Francesco Dolcini,
DRI Development List, Devicetree List, Linux Kernel List,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jyri Sarha
Hi Tomi,
On 13/02/25 18:50, Tomi Valkeinen wrote:
> Hi,
>
> On 13/02/2025 14:33, Aradhya Bhatia wrote:
>
>>>> + ti,companion-oldi:
>>>> + $ref: /schemas/types.yaml#/definitions/phandle
>>>> + description:
>>>> + phandle to companion OLDI transmitter. This property is
>>>> mandatory for the
>>>> + primarty OLDI TX if the OLDI TXes are expected to work either
>>>> in dual-lvds
>>>> + mode or in clone mode. This property should point to the
>>>> secondary OLDI
>>>> + TX.
>>>> +
>>>> + ti,secondary-oldi:
>>>> + type: boolean
>>>> + description:
>>>> + Boolean property to mark the OLDI transmitter as the secondary
>>>> one, when the
>>>> + OLDI hardware is expected to run as a companion HW, in cases of
>>>> dual-lvds
>>>> + mode or clone mode. The primary OLDI hardware is responsible
>>>> for all the
>>>> + hardware configuration.
>>>
>>> I think these work, but I'm wondering if we would ever need to check
>>> something from the main oldi from the secondary oldi. In that case
>>> "crossed phandles" would be better, i.e. something like:
>>>
>>> (in the first oldi:)
>>> ti,slave-oldi = <phandle-to-second-oldi>
>>>
>>> (in the second oldi:)
>>> ti,master-oldi = <phandle-to-first-oldi>
>>
>> When I had first designed the code and the devicetree for OLDI, it was
>> done so with the belief that we wouldn't reqiure a bridge instance for
>> the secondary OLDI, at all.
>>
>> While that idea holds true for dual-lvds configuration, it doesn't so
>> for the clone mode configuration. For clone mode, as you pointed out, we
>> will require a 2nd bridge instance to configure any of the bridges and
>> panels that come after the 2nd OLDI.
>>
>>
>>>
>>> Then again, if we ever need that, even with these bindings the driver
>>> could find the first oldi, but needs to go via the dss's node.
>>
>> While it is possible to do it this way, it might not be the cleanest
>> one. And _if_ there is a ever a DSS in future with more than 2 OLDI
>> TXes, say 4, then the decipher logic may get too complicated.
>>
>> While I cannot think of any case where the secondary OLDI bridge DT
>> might need to access the primary OLDI bridge at the moment, I wonder if
>> we should play it safer and have this option anyway.
>>
>> Maybe something like this?
>>
>> (primary OLDI)
>> ti,primary-oldi;
>> ti,companion-oldi = <phandle-to-secondary-oldi>;
>>
>> (secondary OLDI)
>> ti,secondary-oldi;
>> ti,companion-oldi = <phandle-to-primary-oldi>;
>
> How is this different than my proposal, except a bit more verbose?
That's all the difference there is. Just an alternative to what you
suggested.
>
> If you're thinking about a 4-OLDI hardware, how would this work there?
I didn't mean that my alternative would be more helpful. I meant that
passing phandles would be a simpler way for 4-OLDI hardware in general.
We'd have to sift through a max of 4 OLDI nodes to find the right
primary OLDI for a given secondary OLDI - if we try to find it via the
dss and oldi-transmitter parent DT nodes. Passing phandles directly
would save on all that logic.
> (but I want to say that even if it's good to plan for the future, we
> shouldn't plan too much based on imaginary hardware =).
>
That's, of course, true too! =)
It's been tricky enough dealing with the hardware combinations as they
are today!
I will add one more reason though, which made me get along with the idea
of passing phandles. And then I will defer to you to make the call,
since I don't have the strongest of feelings either way.
Passing phandles would allow for _that_ condition to get dropped; making
the bindings slightly more flexible to accommodate for any future
surprises (especially around the clone mode lvds configuration).
(That condition being where the bindings either allow a companion-oldi
phandle OR allow the secondary-oldi boolean (but not both)).
I could drop that condition without any other changes too, making the
companion-oldi property optional for secondary-oldi - but this feels
incomplete.
Hence, the addition of the primary-oldi boolean. The companion-oldi
phandle property will be conditionally required when any one of the
boolean properties is present.
--
Regards
Aradhya
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v5 2/3] dt-bindings: display: ti: Add schema for AM625 OLDI Transmitter
2025-02-14 12:41 ` Aradhya Bhatia
@ 2025-02-19 14:20 ` Rob Herring
0 siblings, 0 replies; 12+ messages in thread
From: Rob Herring @ 2025-02-19 14:20 UTC (permalink / raw)
To: Aradhya Bhatia
Cc: Tomi Valkeinen, Maarten Lankhorst, Thomas Zimmermann,
Maxime Ripard, David Airlie, Laurent Pinchart, Simona Vetter,
Nishanth Menon, Vignesh Raghavendra, Devarsh Thakkar,
Praneeth Bajjuri, Udit Kumar, Jayesh Choudhary, Francesco Dolcini,
DRI Development List, Devicetree List, Linux Kernel List,
Krzysztof Kozlowski, Conor Dooley, Jyri Sarha
On Fri, Feb 14, 2025 at 06:11:11PM +0530, Aradhya Bhatia wrote:
> Hi Tomi,
>
>
> On 13/02/25 18:50, Tomi Valkeinen wrote:
> > Hi,
> >
> > On 13/02/2025 14:33, Aradhya Bhatia wrote:
> >
> >>>> + ti,companion-oldi:
> >>>> + $ref: /schemas/types.yaml#/definitions/phandle
> >>>> + description:
> >>>> + phandle to companion OLDI transmitter. This property is
> >>>> mandatory for the
> >>>> + primarty OLDI TX if the OLDI TXes are expected to work either
> >>>> in dual-lvds
> >>>> + mode or in clone mode. This property should point to the
> >>>> secondary OLDI
> >>>> + TX.
> >>>> +
> >>>> + ti,secondary-oldi:
> >>>> + type: boolean
> >>>> + description:
> >>>> + Boolean property to mark the OLDI transmitter as the secondary
> >>>> one, when the
> >>>> + OLDI hardware is expected to run as a companion HW, in cases of
> >>>> dual-lvds
> >>>> + mode or clone mode. The primary OLDI hardware is responsible
> >>>> for all the
> >>>> + hardware configuration.
> >>>
> >>> I think these work, but I'm wondering if we would ever need to check
> >>> something from the main oldi from the secondary oldi. In that case
> >>> "crossed phandles" would be better, i.e. something like:
> >>>
> >>> (in the first oldi:)
> >>> ti,slave-oldi = <phandle-to-second-oldi>
> >>>
> >>> (in the second oldi:)
> >>> ti,master-oldi = <phandle-to-first-oldi>
> >>
> >> When I had first designed the code and the devicetree for OLDI, it was
> >> done so with the belief that we wouldn't reqiure a bridge instance for
> >> the secondary OLDI, at all.
> >>
> >> While that idea holds true for dual-lvds configuration, it doesn't so
> >> for the clone mode configuration. For clone mode, as you pointed out, we
> >> will require a 2nd bridge instance to configure any of the bridges and
> >> panels that come after the 2nd OLDI.
> >>
> >>
> >>>
> >>> Then again, if we ever need that, even with these bindings the driver
> >>> could find the first oldi, but needs to go via the dss's node.
> >>
> >> While it is possible to do it this way, it might not be the cleanest
> >> one. And _if_ there is a ever a DSS in future with more than 2 OLDI
> >> TXes, say 4, then the decipher logic may get too complicated.
> >>
> >> While I cannot think of any case where the secondary OLDI bridge DT
> >> might need to access the primary OLDI bridge at the moment, I wonder if
> >> we should play it safer and have this option anyway.
> >>
> >> Maybe something like this?
> >>
> >> (primary OLDI)
> >> ti,primary-oldi;
> >> ti,companion-oldi = <phandle-to-secondary-oldi>;
> >>
> >> (secondary OLDI)
> >> ti,secondary-oldi;
> >> ti,companion-oldi = <phandle-to-primary-oldi>;
> >
> > How is this different than my proposal, except a bit more verbose?
>
> That's all the difference there is. Just an alternative to what you
> suggested.
>
> >
> > If you're thinking about a 4-OLDI hardware, how would this work there?
>
> I didn't mean that my alternative would be more helpful. I meant that
> passing phandles would be a simpler way for 4-OLDI hardware in general.
>
> We'd have to sift through a max of 4 OLDI nodes to find the right
> primary OLDI for a given secondary OLDI - if we try to find it via the
> dss and oldi-transmitter parent DT nodes. Passing phandles directly
> would save on all that logic.
I prefer the data in the DT be the minimum needed. Parsing the DT
doesn't need to be particularly fast because you should only do it once.
There's even a function already to find occurrences of a property name
all over the tree.
Rob
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v5 2/3] dt-bindings: display: ti: Add schema for AM625 OLDI Transmitter
2025-02-09 16:09 ` [PATCH v5 2/3] dt-bindings: display: ti: Add schema for AM625 OLDI Transmitter Aradhya Bhatia
2025-02-11 12:24 ` Tomi Valkeinen
@ 2025-02-19 14:22 ` Rob Herring (Arm)
1 sibling, 0 replies; 12+ messages in thread
From: Rob Herring (Arm) @ 2025-02-19 14:22 UTC (permalink / raw)
To: Aradhya Bhatia
Cc: Linux Kernel List, Jayesh Choudhary, Vignesh Raghavendra,
Maarten Lankhorst, Krzysztof Kozlowski, Francesco Dolcini,
Nishanth Menon, Thomas Zimmermann, Praneeth Bajjuri,
Maxime Ripard, Tomi Valkeinen, Udit Kumar, Devicetree List,
Devarsh Thakkar, Laurent Pinchart, Conor Dooley, David Airlie,
Jyri Sarha, DRI Development List, Simona Vetter
On Sun, 09 Feb 2025 21:39:24 +0530, Aradhya Bhatia wrote:
> From: Aradhya Bhatia <a-bhatia1@ti.com>
>
> The OLDI transmitters (TXes) do not have registers of their own, and are
> dependent on the source video-ports (VPs) from the DSS to provide
> configuration data. This hardware doesn't directly sit on the internal
> bus of the SoC, but does so via the DSS. Hence, the OLDI TXes are
> supposed to be child nodes under the DSS, and not independent devices.
>
> Two of the OLDI TXes can function in tandem to output dual-link OLDI
> output, or cloned single-link outputs. In these cases, one OLDI will be
> the primary OLDI, and the other one, a companion. The following diagram
> represents such a configuration.
>
> +-----+-----+ +-------+
> | | | | |
> | | VP1 +----+--->+ OLDI0 | (Primary - may need companion)
> | | | | | |
> | DSS +-----+ | +-------+
> | | | |
> | | VP2 | | +-------+
> | | | | | |
> +-----+-----+ +--->+ OLDI1 | (Companion OLDI)
> | |
> +-------+
>
> The DSS in AM625 SoC has a configuration like the one above. The AM625
> DSS VP1 (port@0) can connect and control 2 OLDI TXes, to use them in
> dual-link or cloned single-link OLDI modes. It is only the VP1 that can
> connect to either OLDI TXes for the AM625 DSS, and not the VP2.
>
> Alternatively, on some future TI SoCs, along with the above
> configuration, the OLDI TX can _also_ connect to separate video sources,
> making them work entirely independent of each other. In this case,
> neither of the OLDIs are "companion" or "secondary" OLDIs, and nor do
> they require one. They both are independent and primary OLDIs. The
> following diagram represents such a configuration.
>
> +-----+-----+ +-------+
> | | | | |
> | | VP1 +--+----------->+ OLDI0 | (Primary - may need companion)
> | | | | | |
> | +-----+ | +-------+
> | | | |
> | | VP2 | |
> | | | |
> | DSS +-----+ | +---+ +-------+
> | | | +-->+ M | | |
> | | VP3 +----->+ U +--->+ OLDI1 | (Companion or Primary)
> | | | | X | | |
> | +-----+ +---+ +-------+
> | | |
> | | VP4 |
> | | |
> +-----+-----+
>
> Note that depending on the mux configuration, the OLDIs can either be
> working together in tandem - sourced by VP1, OR, they could be working
> independently sourced by VP1 and VP3 respectively.
> The idea is to support all the configurations with this OLDI TX schema.
>
> The OLDI functionality is further supported by a system-control module,
> which contains a few registers to control OLDI IO power and other
> electrical characteristics of the IO lanes.
>
> Add devicetree binding schema for the OLDI TXes to support various
> configurations, and extend their support to the AM625 DSS.
>
> Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
> Signed-off-by: Aradhya Bhatia <aradhya.bhatia@linux.dev>
> ---
> .../bindings/display/ti/ti,am625-oldi.yaml | 88 ++++++++++
> .../bindings/display/ti/ti,am65x-dss.yaml | 154 ++++++++++++++++++
> MAINTAINERS | 1 +
> 3 files changed, 243 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/display/ti/ti,am625-oldi.yaml
>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2025-02-19 14:22 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-09 16:09 [PATCH v5 0/3] drm/tidss: Add OLDI bridge support Aradhya Bhatia
2025-02-09 16:09 ` [PATCH v5 1/3] dt-bindings: display: ti,am65x-dss: Re-indent the example Aradhya Bhatia
2025-02-09 16:09 ` [PATCH v5 2/3] dt-bindings: display: ti: Add schema for AM625 OLDI Transmitter Aradhya Bhatia
2025-02-11 12:24 ` Tomi Valkeinen
2025-02-13 12:33 ` Aradhya Bhatia
2025-02-13 13:20 ` Tomi Valkeinen
2025-02-14 12:41 ` Aradhya Bhatia
2025-02-19 14:20 ` Rob Herring
2025-02-19 14:22 ` Rob Herring (Arm)
2025-02-09 16:09 ` [PATCH v5 3/3] drm/tidss: Add OLDI bridge support Aradhya Bhatia
2025-02-11 10:57 ` Tomi Valkeinen
2025-02-11 12:25 ` Tomi Valkeinen
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).