devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/3] drm/tidss: Add OLDI bridge support
@ 2024-11-24 14:36 Aradhya Bhatia
  2024-11-24 14:36 ` [PATCH v4 1/3] dt-bindings: display: ti,am65x-dss: Re-indent the example Aradhya Bhatia
                   ` (4 more replies)
  0 siblings, 5 replies; 16+ messages in thread
From: Aradhya Bhatia @ 2024-11-24 14:36 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Tomi Valkeinen,
	Jyri Sarha, Maarten Lankhorst, Thomas Zimmermann, Maxime Ripard,
	David Airlie, Laurent Pinchart, Simona Vetter
  Cc: Dmitry Baryshkov, Nishanth Menon, Vignesh Raghavendra,
	Devarsh Thakkar, Praneeth Bajjuri, Udit Kumar, Jayesh Choudhary,
	Francesco Dolcini, Alexander Sverdlin, Max Krummenacher,
	DRI Development List, Devicetree List, Linux Kernel List,
	Aradhya Bhatia

Hello all,

This patch series add support for the dual OLDI TXes supported in Texas
Instruments' AM62x and AM62Px family of SoCs. The OLDI TXes support single-lvds
lvds-clone, and dual-lvds modes. These 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.

Also, since the DRM framework only really supports a linear encoder-bridge
chain, the OLDI driver creates a DRM bridge ONLY for the primary OLDI TX in
cases of dual-link or cloned single-link OLDI modes. That bridge then attaches
to the tidss's display core - which consists of a CRTC, an Encoder (dummy) and a
bridge (dummy). On the other end, it attaches to OLDI sinks (panels or other
bridges).

Since the OLDI TX have a hardware dependency with the VP, 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 a patch allowing DRM
bridges to get pre-enabled before the CRTC of that bridge is enabled[0]. Without
that patch, some warnings or glitches can be seen.

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

Due to lack of hardware, I haven't been able to test single-link / cloned
single-link OLDI modes. I have only used a sample cloned single-link DTBO and
booted the board with it. I didn't see any probe_deferred errors (as seen
previously), and the `kmsprint` utility enumerated the display details fine.

Regardless, I'd appreciate it if somebody can test it, and report back if they
observe any issues.

Thanks,
Aradhya


Additional Notes:

* Important note about a false positive in dtbs_check *
Both the ports, port0 and port1, are required for the OLDI functionality to
work. The schema suggests this condition too. Additionally, the OLDI devicetree
node is expected to be defined in the soc.dtsi file, and kept as disabled.
Over the current platforms (Beagleplay and SK-AM625 EVM), the OLDI panel is not
always attached, and hence we use a DT overlay to add panel details - which is
where we enable the OLDI nodes. The structure of files is like this -

- soc.dtsi                  (OLDI disabled)
- soc-baseboard.dts         (OLDI disabled)
- soc-baseboard-panel.dtso  (OLDI enabled)

During dtbs_check runs, it was observed that the check was not able to rule out
OLDI issues even when its DT was disabled in the soc-baseboard.dts. It is
impractical and impossible to add OLDI ports prior to the panel overlay file.
While the dtbs_check usually ignores checking disabled devicetree nodes, it was
unable to do so in the OLDI's case.


* 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:
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:
V3: https://lore.kernel.org/all/20240716084248.1393666-1-a-bhatia1@ti.com/
V2: https://lore.kernel.org/all/20240715200953.1213284-1-a-bhatia1@ti.com/
V1: https://lore.kernel.org/all/20240511193055.1686149-1-a-bhatia1@ti.com/


[0]: Dependency Patch: 
("drm/atomic-helper: Re-order bridge chain pre-enable and post-disable")
https://lore.kernel.org/all/20240622110929.3115714-11-a-bhatia1@ti.com/

[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_v4_tests

[5]: ("ti,am65x-dss.yaml: oldi-txes: Missing additionalProperties/
      unevaluatedProperties constraint")
https://lore.kernel.org/all/172107979988.1595945.9666141982402158422.robh@kernel.org/

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    | 119 ++++
 .../bindings/display/ti/ti,am65x-dss.yaml     | 195 ++++++-
 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            | 537 ++++++++++++++++++
 drivers/gpu/drm/tidss/tidss_oldi.h            |  51 ++
 11 files changed, 935 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: cfba9f07a1d6aeca38f47f1f472cfb0ba133d341
-- 
2.34.1


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

* [PATCH v4 1/3] dt-bindings: display: ti,am65x-dss: Re-indent the example
  2024-11-24 14:36 [PATCH v4 0/3] drm/tidss: Add OLDI bridge support Aradhya Bhatia
@ 2024-11-24 14:36 ` Aradhya Bhatia
  2024-11-24 14:36 ` [PATCH v4 2/3] dt-bindings: display: ti: Add schema for AM625 OLDI Transmitter Aradhya Bhatia
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 16+ messages in thread
From: Aradhya Bhatia @ 2024-11-24 14:36 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Tomi Valkeinen,
	Jyri Sarha, Maarten Lankhorst, Thomas Zimmermann, Maxime Ripard,
	David Airlie, Laurent Pinchart, Simona Vetter
  Cc: Dmitry Baryshkov, Nishanth Menon, Vignesh Raghavendra,
	Devarsh Thakkar, Praneeth Bajjuri, Udit Kumar, Jayesh Choudhary,
	Francesco Dolcini, Alexander Sverdlin, Max Krummenacher,
	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] 16+ messages in thread

* [PATCH v4 2/3] dt-bindings: display: ti: Add schema for AM625 OLDI Transmitter
  2024-11-24 14:36 [PATCH v4 0/3] drm/tidss: Add OLDI bridge support Aradhya Bhatia
  2024-11-24 14:36 ` [PATCH v4 1/3] dt-bindings: display: ti,am65x-dss: Re-indent the example Aradhya Bhatia
@ 2024-11-24 14:36 ` Aradhya Bhatia
  2024-12-04 14:09   ` Rob Herring
  2024-11-24 14:36 ` [PATCH v4 3/3] drm/tidss: Add OLDI bridge support Aradhya Bhatia
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 16+ messages in thread
From: Aradhya Bhatia @ 2024-11-24 14:36 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Tomi Valkeinen,
	Jyri Sarha, Maarten Lankhorst, Thomas Zimmermann, Maxime Ripard,
	David Airlie, Laurent Pinchart, Simona Vetter
  Cc: Dmitry Baryshkov, Nishanth Menon, Vignesh Raghavendra,
	Devarsh Thakkar, Praneeth Bajjuri, Udit Kumar, Jayesh Choudhary,
	Francesco Dolcini, Alexander Sverdlin, Max Krummenacher,
	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    | 119 ++++++++++++++
 .../bindings/display/ti/ti,am65x-dss.yaml     | 153 ++++++++++++++++++
 MAINTAINERS                                   |   1 +
 3 files changed, 273 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..51b24db04e89
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/ti/ti,am625-oldi.yaml
@@ -0,0 +1,119 @@
+# 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
+
+examples:
+  - |
+    #include <dt-bindings/soc/ti,sci_pm_domain.h>
+
+    oldi-transmitters {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        oldi: oldi@0 {
+            reg = <0>;
+            clocks = <&k3_clks 186 0>;
+            clock-names = "serial";
+            ti,oldi-io-ctrl = <&dss_oldi_io_ctrl>;
+            ports {
+                #address-cells = <1>;
+                #size-cells = <0>;
+                port@0 {
+                    reg = <0>;
+                    oldi_in: endpoint {
+                        remote-endpoint = <&dpi_out>;
+                    };
+                };
+                port@1 {
+                    reg = <1>;
+                    oldi_out: endpoint {
+                        remote-endpoint = <&panel_in>;
+                    };
+                };
+            };
+        };
+    };
+
+...
diff --git a/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml b/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
index 399d68986326..eb6a65f9970d 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,26 @@ 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]$':
+        type: object
+        $ref: ti,am625-oldi.yaml#
+        unevaluatedProperties: false
+        description: OLDI transmitters connected to the DSS VPs
+
 allOf:
   - if:
       properties:
@@ -123,6 +161,19 @@ allOf:
         ports:
           properties:
             port@0: false
+            oldi-transmitters: false
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: ti,am65x-dss
+    then:
+      properties:
+        oldi-transmitters: false
+        port@0:
+          properties:
+            endpoint@1: false
 
 required:
   - compatible
@@ -171,3 +222,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 93415153247b..d47a56c4d276 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7817,6 +7817,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] 16+ messages in thread

* [PATCH v4 3/3] drm/tidss: Add OLDI bridge support
  2024-11-24 14:36 [PATCH v4 0/3] drm/tidss: Add OLDI bridge support Aradhya Bhatia
  2024-11-24 14:36 ` [PATCH v4 1/3] dt-bindings: display: ti,am65x-dss: Re-indent the example Aradhya Bhatia
  2024-11-24 14:36 ` [PATCH v4 2/3] dt-bindings: display: ti: Add schema for AM625 OLDI Transmitter Aradhya Bhatia
@ 2024-11-24 14:36 ` Aradhya Bhatia
  2025-03-18 13:35   ` Sverdlin, Alexander
  2024-12-03 12:12 ` [PATCH v4 0/3] " Tomi Valkeinen
  2025-03-19 18:00 ` Sverdlin, Alexander
  4 siblings, 1 reply; 16+ messages in thread
From: Aradhya Bhatia @ 2024-11-24 14:36 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Tomi Valkeinen,
	Jyri Sarha, Maarten Lankhorst, Thomas Zimmermann, Maxime Ripard,
	David Airlie, Laurent Pinchart, Simona Vetter
  Cc: Dmitry Baryshkov, Nishanth Menon, Vignesh Raghavendra,
	Devarsh Thakkar, Praneeth Bajjuri, Udit Kumar, Jayesh Choudhary,
	Francesco Dolcini, Alexander Sverdlin, Max Krummenacher,
	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       | 537 +++++++++++++++++++++++
 drivers/gpu/drm/tidss/tidss_oldi.h       |  51 +++
 8 files changed, 641 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 2428b9aaa003..b83026321969 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..19c5a7de1c32
--- /dev/null
+++ b/drivers/gpu/drm/tidss/tidss_oldi.c
@@ -0,0 +1,537 @@
+// 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_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] 16+ messages in thread

* Re: [PATCH v4 0/3] drm/tidss: Add OLDI bridge support
  2024-11-24 14:36 [PATCH v4 0/3] drm/tidss: Add OLDI bridge support Aradhya Bhatia
                   ` (2 preceding siblings ...)
  2024-11-24 14:36 ` [PATCH v4 3/3] drm/tidss: Add OLDI bridge support Aradhya Bhatia
@ 2024-12-03 12:12 ` Tomi Valkeinen
  2024-12-03 18:14   ` Aradhya Bhatia
  2025-03-19 18:00 ` Sverdlin, Alexander
  4 siblings, 1 reply; 16+ messages in thread
From: Tomi Valkeinen @ 2024-12-03 12:12 UTC (permalink / raw)
  To: Aradhya Bhatia
  Cc: Dmitry Baryshkov, Nishanth Menon, Vignesh Raghavendra,
	Devarsh Thakkar, Praneeth Bajjuri, Udit Kumar, Jayesh Choudhary,
	Francesco Dolcini, Alexander Sverdlin, Max Krummenacher,
	DRI Development List, Devicetree List, Linux Kernel List,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jyri Sarha,
	Maarten Lankhorst, Thomas Zimmermann, Maxime Ripard, David Airlie,
	Laurent Pinchart, Simona Vetter

Hi,

On 24/11/2024 16:36, Aradhya Bhatia wrote:
> Hello all,
> 
> This patch series add support for the dual OLDI TXes supported in Texas
> Instruments' AM62x and AM62Px family of SoCs. The OLDI TXes support single-lvds
> lvds-clone, and dual-lvds modes. These 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.
> 
> Also, since the DRM framework only really supports a linear encoder-bridge
> chain, the OLDI driver creates a DRM bridge ONLY for the primary OLDI TX in
> cases of dual-link or cloned single-link OLDI modes. That bridge then attaches

How does the clone case work, then? There are two panels, what does the 
second one connect to?

> to the tidss's display core - which consists of a CRTC, an Encoder (dummy) and a
> bridge (dummy). On the other end, it attaches to OLDI sinks (panels or other
> bridges).
> 
> Since the OLDI TX have a hardware dependency with the VP, 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 a patch allowing DRM
> bridges to get pre-enabled before the CRTC of that bridge is enabled[0]. Without
> that patch, some warnings or glitches can be seen.
> 
> 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_v4_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.
> 
> Due to lack of hardware, I haven't been able to test single-link / cloned
> single-link OLDI modes. I have only used a sample cloned single-link DTBO and
> booted the board with it. I didn't see any probe_deferred errors (as seen
> previously), and the `kmsprint` utility enumerated the display details fine.
> 
> Regardless, I'd appreciate it if somebody can test it, and report back if they
> observe any issues.
> 
> Thanks,
> Aradhya
> 
> 
> Additional Notes:
> 
> * Important note about a false positive in dtbs_check *
> Both the ports, port0 and port1, are required for the OLDI functionality to
> work. The schema suggests this condition too. Additionally, the OLDI devicetree
> node is expected to be defined in the soc.dtsi file, and kept as disabled.
> Over the current platforms (Beagleplay and SK-AM625 EVM), the OLDI panel is not
> always attached, and hence we use a DT overlay to add panel details - which is
> where we enable the OLDI nodes. The structure of files is like this -
> 
> - soc.dtsi                  (OLDI disabled)
> - soc-baseboard.dts         (OLDI disabled)
> - soc-baseboard-panel.dtso  (OLDI enabled)
> 
> During dtbs_check runs, it was observed that the check was not able to rule out
> OLDI issues even when its DT was disabled in the soc-baseboard.dts. It is
> impractical and impossible to add OLDI ports prior to the panel overlay file.
> While the dtbs_check usually ignores checking disabled devicetree nodes, it was
> unable to do so in the OLDI's case.

While there might be something amiss with dtbs_check, what's the problem 
with adding both port nodes to the soc.dtsi? If you have no endpoints 
there, it's not connected to anything.

  Tomi

> 
> 
> * 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:
> 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:
> V3: https://lore.kernel.org/all/20240716084248.1393666-1-a-bhatia1@ti.com/
> V2: https://lore.kernel.org/all/20240715200953.1213284-1-a-bhatia1@ti.com/
> V1: https://lore.kernel.org/all/20240511193055.1686149-1-a-bhatia1@ti.com/
> 
> 
> [0]: Dependency Patch:
> ("drm/atomic-helper: Re-order bridge chain pre-enable and post-disable")
> https://lore.kernel.org/all/20240622110929.3115714-11-a-bhatia1@ti.com/
> 
> [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_v4_tests
> 
> [5]: ("ti,am65x-dss.yaml: oldi-txes: Missing additionalProperties/
>        unevaluatedProperties constraint")
> https://lore.kernel.org/all/172107979988.1595945.9666141982402158422.robh@kernel.org/
> 
> 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    | 119 ++++
>   .../bindings/display/ti/ti,am65x-dss.yaml     | 195 ++++++-
>   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            | 537 ++++++++++++++++++
>   drivers/gpu/drm/tidss/tidss_oldi.h            |  51 ++
>   11 files changed, 935 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: cfba9f07a1d6aeca38f47f1f472cfb0ba133d341


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

* Re: [PATCH v4 0/3] drm/tidss: Add OLDI bridge support
  2024-12-03 12:12 ` [PATCH v4 0/3] " Tomi Valkeinen
@ 2024-12-03 18:14   ` Aradhya Bhatia
  2024-12-03 18:36     ` Tomi Valkeinen
  0 siblings, 1 reply; 16+ messages in thread
From: Aradhya Bhatia @ 2024-12-03 18:14 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Dmitry Baryshkov, Nishanth Menon, Vignesh Raghavendra,
	Devarsh Thakkar, Praneeth Bajjuri, Udit Kumar, Jayesh Choudhary,
	Francesco Dolcini, Alexander Sverdlin, Max Krummenacher,
	DRI Development List, Devicetree List, Linux Kernel List,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jyri Sarha,
	Maarten Lankhorst, Thomas Zimmermann, Maxime Ripard, David Airlie,
	Laurent Pinchart, Simona Vetter

Hi,

On 03/12/24 17:42, Tomi Valkeinen wrote:
> Hi,
> 
> On 24/11/2024 16:36, Aradhya Bhatia wrote:
>> Hello all,
>>
>> This patch series add support for the dual OLDI TXes supported in Texas
>> Instruments' AM62x and AM62Px family of SoCs. The OLDI TXes support
>> single-lvds
>> lvds-clone, and dual-lvds modes. These 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.
>>
>> Also, since the DRM framework only really supports a linear encoder-
>> bridge
>> chain, the OLDI driver creates a DRM bridge ONLY for the primary OLDI
>> TX in
>> cases of dual-link or cloned single-link OLDI modes. That bridge then
>> attaches
> 
> How does the clone case work, then? There are two panels, what does the
> second one connect to?

For the clone case, the devicetree will show the true connections - as
they are in the hardware.

2 endpoints from a single DSS VP devicetree port will be connected to 2
OLDIs, OLDI0 and OLDI1. The outputs of these OLDIs will be connected to
2 distinct single-link panels.

The driver and DRM side of things do not show the same picture, however.
The tidss_oldi code creates and registers a drm_bridge only for the
primary OLDI. The driver is capable of detecting the expected OLDI mode,
and if a companion OLDI is present, then the primary OLDI drm_bridge
keeps a note of that.

The clock and config resources are shared between the primary and
companion OLDI hardware. So configuring the primary OLDI takes care of
the companion too.
The only case where it is not shared is the OLDI IO bit in the Control
MMR (ctrl_mmr) region. But, since the primary OLDI drm_bridge remains
aware about the presence of companion OLDI, it makes sure to enable /
disable the comapnion OLDI IO when required.

> 
>> to the tidss's display core - which consists of a CRTC, an Encoder
>> (dummy) and a
>> bridge (dummy). On the other end, it attaches to OLDI sinks (panels or
>> other
>> bridges).
>>
>> Since the OLDI TX have a hardware dependency with the VP, 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 a patch allowing
>> DRM
>> bridges to get pre-enabled before the CRTC of that bridge is
>> enabled[0]. Without
>> that patch, some warnings or glitches can be seen.
>>
>> 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_v4_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.
>>
>> Due to lack of hardware, I haven't been able to test single-link / cloned
>> single-link OLDI modes. I have only used a sample cloned single-link
>> DTBO and
>> booted the board with it. I didn't see any probe_deferred errors (as seen
>> previously), and the `kmsprint` utility enumerated the display details
>> fine.
>>
>> Regardless, I'd appreciate it if somebody can test it, and report back
>> if they
>> observe any issues.
>>
>> Thanks,
>> Aradhya
>>
>>
>> Additional Notes:
>>
>> * Important note about a false positive in dtbs_check *
>> Both the ports, port0 and port1, are required for the OLDI
>> functionality to
>> work. The schema suggests this condition too. Additionally, the OLDI
>> devicetree
>> node is expected to be defined in the soc.dtsi file, and kept as
>> disabled.
>> Over the current platforms (Beagleplay and SK-AM625 EVM), the OLDI
>> panel is not
>> always attached, and hence we use a DT overlay to add panel details -
>> which is
>> where we enable the OLDI nodes. The structure of files is like this -
>>
>> - soc.dtsi                  (OLDI disabled)
>> - soc-baseboard.dts         (OLDI disabled)
>> - soc-baseboard-panel.dtso  (OLDI enabled)
>>
>> During dtbs_check runs, it was observed that the check was not able to
>> rule out
>> OLDI issues even when its DT was disabled in the soc-baseboard.dts. It is
>> impractical and impossible to add OLDI ports prior to the panel
>> overlay file.
>> While the dtbs_check usually ignores checking disabled devicetree
>> nodes, it was
>> unable to do so in the OLDI's case.
> 
> While there might be something amiss with dtbs_check, what's the problem
> with adding both port nodes to the soc.dtsi? If you have no endpoints
> there, it's not connected to anything.
>

Ran dtbs_check with this. The errors are silenced indeed. I didn't
really like having empty ports in an already long DSS devicetree node,
but this way the checks remain clean. I will use this for DT patches.
Thank you! =)

> 
[...]
> 

-- 
Regards
Aradhya


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

* Re: [PATCH v4 0/3] drm/tidss: Add OLDI bridge support
  2024-12-03 18:14   ` Aradhya Bhatia
@ 2024-12-03 18:36     ` Tomi Valkeinen
  2024-12-09  4:17       ` Aradhya Bhatia
  0 siblings, 1 reply; 16+ messages in thread
From: Tomi Valkeinen @ 2024-12-03 18:36 UTC (permalink / raw)
  To: Aradhya Bhatia
  Cc: Dmitry Baryshkov, Nishanth Menon, Vignesh Raghavendra,
	Devarsh Thakkar, Praneeth Bajjuri, Udit Kumar, Jayesh Choudhary,
	Francesco Dolcini, Alexander Sverdlin, Max Krummenacher,
	DRI Development List, Devicetree List, Linux Kernel List,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jyri Sarha,
	Maarten Lankhorst, Thomas Zimmermann, Maxime Ripard, David Airlie,
	Laurent Pinchart, Simona Vetter

Hi,

On 03/12/2024 20:14, Aradhya Bhatia wrote:
> Hi,
> 
> On 03/12/24 17:42, Tomi Valkeinen wrote:
>> Hi,
>>
>> On 24/11/2024 16:36, Aradhya Bhatia wrote:
>>> Hello all,
>>>
>>> This patch series add support for the dual OLDI TXes supported in Texas
>>> Instruments' AM62x and AM62Px family of SoCs. The OLDI TXes support
>>> single-lvds
>>> lvds-clone, and dual-lvds modes. These 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.
>>>
>>> Also, since the DRM framework only really supports a linear encoder-
>>> bridge
>>> chain, the OLDI driver creates a DRM bridge ONLY for the primary OLDI
>>> TX in
>>> cases of dual-link or cloned single-link OLDI modes. That bridge then
>>> attaches
>>
>> How does the clone case work, then? There are two panels, what does the
>> second one connect to?
> 
> For the clone case, the devicetree will show the true connections - as
> they are in the hardware.
> 
> 2 endpoints from a single DSS VP devicetree port will be connected to 2
> OLDIs, OLDI0 and OLDI1. The outputs of these OLDIs will be connected to
> 2 distinct single-link panels.
> 
> The driver and DRM side of things do not show the same picture, however.
> The tidss_oldi code creates and registers a drm_bridge only for the
> primary OLDI. The driver is capable of detecting the expected OLDI mode,
> and if a companion OLDI is present, then the primary OLDI drm_bridge
> keeps a note of that.
> 
> The clock and config resources are shared between the primary and
> companion OLDI hardware. So configuring the primary OLDI takes care of
> the companion too.
> The only case where it is not shared is the OLDI IO bit in the Control
> MMR (ctrl_mmr) region. But, since the primary OLDI drm_bridge remains
> aware about the presence of companion OLDI, it makes sure to enable /
> disable the comapnion OLDI IO when required.

But if there's just one bridge (for the first oldi), how is the second 
panel connected to the DRM pipeline? Who e.g. calls the 
drm_panel_funcs.enable() in the panel driver for the second panel?

Or, say, if we have two LVDS->HDMI bridges, with the cloning setup, how 
does all the plumbing work if "DRM framework only really supports a 
linear encoder-bridge chain"?

  Tomi


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

* Re: [PATCH v4 2/3] dt-bindings: display: ti: Add schema for AM625 OLDI Transmitter
  2024-11-24 14:36 ` [PATCH v4 2/3] dt-bindings: display: ti: Add schema for AM625 OLDI Transmitter Aradhya Bhatia
@ 2024-12-04 14:09   ` Rob Herring
  0 siblings, 0 replies; 16+ messages in thread
From: Rob Herring @ 2024-12-04 14:09 UTC (permalink / raw)
  To: Aradhya Bhatia
  Cc: Krzysztof Kozlowski, Conor Dooley, Tomi Valkeinen, Jyri Sarha,
	Maarten Lankhorst, Thomas Zimmermann, Maxime Ripard, David Airlie,
	Laurent Pinchart, Simona Vetter, Dmitry Baryshkov, Nishanth Menon,
	Vignesh Raghavendra, Devarsh Thakkar, Praneeth Bajjuri,
	Udit Kumar, Jayesh Choudhary, Francesco Dolcini,
	Alexander Sverdlin, Max Krummenacher, DRI Development List,
	Devicetree List, Linux Kernel List

On Sun, Nov 24, 2024 at 08:06:48PM +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    | 119 ++++++++++++++
>  .../bindings/display/ti/ti,am65x-dss.yaml     | 153 ++++++++++++++++++
>  MAINTAINERS                                   |   1 +
>  3 files changed, 273 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..51b24db04e89
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/ti/ti,am625-oldi.yaml
> @@ -0,0 +1,119 @@
> +# 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
> +
> +examples:
> +  - |
> +    #include <dt-bindings/soc/ti,sci_pm_domain.h>
> +
> +    oldi-transmitters {

Drop the example here. It's incomplete without the parent and it is 
never validated with this schema because there is no way to select it.

> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        oldi: oldi@0 {
> +            reg = <0>;
> +            clocks = <&k3_clks 186 0>;
> +            clock-names = "serial";
> +            ti,oldi-io-ctrl = <&dss_oldi_io_ctrl>;
> +            ports {
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +                port@0 {
> +                    reg = <0>;
> +                    oldi_in: endpoint {
> +                        remote-endpoint = <&dpi_out>;
> +                    };
> +                };
> +                port@1 {
> +                    reg = <1>;
> +                    oldi_out: endpoint {
> +                        remote-endpoint = <&panel_in>;
> +                    };
> +                };
> +            };
> +        };
> +    };
> +
> +...
> diff --git a/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml b/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
> index 399d68986326..eb6a65f9970d 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,26 @@ 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

blank line here.

> +    properties:
> +      "#address-cells":
> +        const: 1
> +
> +      "#size-cells":
> +        const: 0
> +
> +    patternProperties:
> +      '^oldi@[0-1]$':
> +        type: object
> +        $ref: ti,am625-oldi.yaml#
> +        unevaluatedProperties: false
> +        description: OLDI transmitters connected to the DSS VPs

You can drop 'type' and 'unevaluatedProperties' here.

> +
>  allOf:
>    - if:
>        properties:
> @@ -123,6 +161,19 @@ allOf:
>          ports:
>            properties:
>              port@0: false
> +            oldi-transmitters: false
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: ti,am65x-dss
> +    then:
> +      properties:
> +        oldi-transmitters: false
> +        port@0:

You missed a level with 'ports'.

> +          properties:
> +            endpoint@1: false
>  
>  required:
>    - compatible
> @@ -171,3 +222,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 93415153247b..d47a56c4d276 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7817,6 +7817,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	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 0/3] drm/tidss: Add OLDI bridge support
  2024-12-03 18:36     ` Tomi Valkeinen
@ 2024-12-09  4:17       ` Aradhya Bhatia
  0 siblings, 0 replies; 16+ messages in thread
From: Aradhya Bhatia @ 2024-12-09  4:17 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Dmitry Baryshkov, Nishanth Menon, Vignesh Raghavendra,
	Devarsh Thakkar, Praneeth Bajjuri, Udit Kumar, Jayesh Choudhary,
	Francesco Dolcini, Alexander Sverdlin, Max Krummenacher,
	DRI Development List, Devicetree List, Linux Kernel List,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jyri Sarha,
	Maarten Lankhorst, Thomas Zimmermann, Maxime Ripard, David Airlie,
	Laurent Pinchart, Simona Vetter

Hi,

On 04/12/24 00:06, Tomi Valkeinen wrote:
> Hi,
> 
> On 03/12/2024 20:14, Aradhya Bhatia wrote:
>> Hi,
>>
>> On 03/12/24 17:42, Tomi Valkeinen wrote:
>>> Hi,
>>>
>>> On 24/11/2024 16:36, Aradhya Bhatia wrote:
>>>> Hello all,
>>>>
>>>> This patch series add support for the dual OLDI TXes supported in Texas
>>>> Instruments' AM62x and AM62Px family of SoCs. The OLDI TXes support
>>>> single-lvds
>>>> lvds-clone, and dual-lvds modes. These 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.
>>>>
>>>> Also, since the DRM framework only really supports a linear encoder-
>>>> bridge
>>>> chain, the OLDI driver creates a DRM bridge ONLY for the primary OLDI
>>>> TX in
>>>> cases of dual-link or cloned single-link OLDI modes. That bridge then
>>>> attaches
>>>
>>> How does the clone case work, then? There are two panels, what does the
>>> second one connect to?
>>
>> For the clone case, the devicetree will show the true connections - as
>> they are in the hardware.
>>
>> 2 endpoints from a single DSS VP devicetree port will be connected to 2
>> OLDIs, OLDI0 and OLDI1. The outputs of these OLDIs will be connected to
>> 2 distinct single-link panels.
>>
>> The driver and DRM side of things do not show the same picture, however.
>> The tidss_oldi code creates and registers a drm_bridge only for the
>> primary OLDI. The driver is capable of detecting the expected OLDI mode,
>> and if a companion OLDI is present, then the primary OLDI drm_bridge
>> keeps a note of that.
>>
>> The clock and config resources are shared between the primary and
>> companion OLDI hardware. So configuring the primary OLDI takes care of
>> the companion too.
>> The only case where it is not shared is the OLDI IO bit in the Control
>> MMR (ctrl_mmr) region. But, since the primary OLDI drm_bridge remains
>> aware about the presence of companion OLDI, it makes sure to enable /
>> disable the comapnion OLDI IO when required.
> 
> But if there's just one bridge (for the first oldi), how is the second
> panel connected to the DRM pipeline? Who e.g. calls the
> drm_panel_funcs.enable() in the panel driver for the second panel?
> 
> Or, say, if we have two LVDS->HDMI bridges, with the cloning setup, how
> does all the plumbing work if "DRM framework only really supports a
> linear encoder-bridge chain"?
> 

Right... The driver does not account for such a case at present. The
simple panels don't require any additional programming, which is why a
clone mode with them just happens to work out.

Since there is still only 1 VP behind this, there could only be a single
crtc. Perhaps, we can have an additional tidss encoder (connected to the
same crtc) to start this another encoder-bridge chain. I am still murky
with the details here, but I will try to see what needs to be done.

Thank you! =)


-- 
Regards
Aradhya


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

* Re: [PATCH v4 3/3] drm/tidss: Add OLDI bridge support
  2024-11-24 14:36 ` [PATCH v4 3/3] drm/tidss: Add OLDI bridge support Aradhya Bhatia
@ 2025-03-18 13:35   ` Sverdlin, Alexander
  2025-03-20 13:27     ` Aradhya Bhatia
  0 siblings, 1 reply; 16+ messages in thread
From: Sverdlin, Alexander @ 2025-03-18 13:35 UTC (permalink / raw)
  To: laurent.pinchart@ideasonboard.com, tzimmermann@suse.de,
	simona@ffwll.ch, jyri.sarha@iki.fi,
	tomi.valkeinen@ideasonboard.com, robh@kernel.org,
	airlied@gmail.com, krzk+dt@kernel.org, aradhya.bhatia@linux.dev,
	maarten.lankhorst@linux.intel.com, conor+dt@kernel.org,
	mripard@kernel.org
  Cc: j-choudhary@ti.com, dmitry.baryshkov@linaro.org, u-kumar1@ti.com,
	max.oss.09@gmail.com, francesco@dolcini.it, devarsht@ti.com,
	dri-devel@lists.freedesktop.org, nm@ti.com, vigneshr@ti.com,
	devicetree@vger.kernel.org, praneeth@ti.com,
	linux-kernel@vger.kernel.org

Hi Aradhya!

On Sun, 2024-11-24 at 20:06 +0530, 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.

...

> +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) {

Would for_each_available_child_of_node() make sense here so that
k3-am62-main.dtsi would have both ports with status = "disabled" and
the users will enable one or another?

> +		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;
> +		}

-- 
Alexander Sverdlin
Siemens AG
www.siemens.com

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

* Re: [PATCH v4 0/3] drm/tidss: Add OLDI bridge support
  2024-11-24 14:36 [PATCH v4 0/3] drm/tidss: Add OLDI bridge support Aradhya Bhatia
                   ` (3 preceding siblings ...)
  2024-12-03 12:12 ` [PATCH v4 0/3] " Tomi Valkeinen
@ 2025-03-19 18:00 ` Sverdlin, Alexander
  2025-03-20 13:24   ` Aradhya Bhatia
  4 siblings, 1 reply; 16+ messages in thread
From: Sverdlin, Alexander @ 2025-03-19 18:00 UTC (permalink / raw)
  To: aradhya.bhatia@linux.dev
  Cc: j-choudhary@ti.com, u-kumar1@ti.com,
	dri-devel@lists.freedesktop.org, devicetree@vger.kernel.org,
	devarsht@ti.com, linux-kernel@vger.kernel.org, nm@ti.com,
	vigneshr@ti.com, praneeth@ti.com

Thank you for the patches, Aradhya!

On Sun, 2024-11-24 at 20:06 +0530, Aradhya Bhatia wrote:
> Regardless, I'd appreciate it if somebody can test it, and report back if they
> observe any issues.

I've tried to test the patchset with necessary pre-requisites and DT additions
with a single channel LVDS pannel and while I'm not successful yet, I've also noticed
the following warning:

tidss 30200000.dss: vp0: Clock rate 24285714 differs over 5% from requested 37000000

even though later the clock seems to be correctly set up:

$ cat /sys/kernel/debug/clk/clk_summary 

                                 enable  prepare  protect                                duty  hardware                            connection
   clock                          count    count    count        rate   accuracy phase  cycle    enable   consumer                         id
---------------------------------------------------------------------------------------------------------------------------------------------
 clk:186:6                           1       1        0        250000000   0          0     50000      Y   30200000.dss                    fck                      
                                                                                                           deviceless                      no_connection_id         
 clk:186:4                           0       0        0        0           0          0     50000      Y   deviceless                      no_connection_id         
 clk:186:3                           0       0        0        170000000   0          0     50000      Y   deviceless                      no_connection_id         
    clk:186:2                        0       0        0        170000000   0          0     50000      Y      30200000.dss                    vp2                      
                                                                                                              deviceless                      no_connection_id         
 clk:186:0                           1       1        0        259090909   0          0     50000      Y   oldi@0                          serial                   
                                                                                                           deviceless                      no_connection_id         
    clock-divider-oldi               1       1        0        37012987    0          0     50000      Y      30200000.dss                    vp1                      

Looks like "clock-divider-oldi" doesn't propagate clk_set_rate() to the parent,
but the parent is being set later independently?

-- 
Alexander Sverdlin
Siemens AG
www.siemens.com

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

* Re: [PATCH v4 0/3] drm/tidss: Add OLDI bridge support
  2025-03-19 18:00 ` Sverdlin, Alexander
@ 2025-03-20 13:24   ` Aradhya Bhatia
  2025-03-20 13:30     ` Sverdlin, Alexander
  2025-03-25 18:57     ` Sverdlin, Alexander
  0 siblings, 2 replies; 16+ messages in thread
From: Aradhya Bhatia @ 2025-03-20 13:24 UTC (permalink / raw)
  To: Sverdlin, Alexander
  Cc: j-choudhary@ti.com, u-kumar1@ti.com,
	dri-devel@lists.freedesktop.org, devicetree@vger.kernel.org,
	devarsht@ti.com, linux-kernel@vger.kernel.org, nm@ti.com,
	vigneshr@ti.com, praneeth@ti.com

Hi Alexander,

Thank you for testing and reviewing the patches!

On 19/03/25 23:30, Sverdlin, Alexander wrote:
> Thank you for the patches, Aradhya!
> 
> On Sun, 2024-11-24 at 20:06 +0530, Aradhya Bhatia wrote:
>> Regardless, I'd appreciate it if somebody can test it, and report back if they
>> observe any issues.
> 
> I've tried to test the patchset with necessary pre-requisites and DT additions
> with a single channel LVDS pannel and while I'm not successful yet, I've also noticed
> the following warning:
> 
> tidss 30200000.dss: vp0: Clock rate 24285714 differs over 5% from requested 37000000
> 
> even though later the clock seems to be correctly set up:
> 
> $ cat /sys/kernel/debug/clk/clk_summary 
> 
>                                  enable  prepare  protect                                duty  hardware                            connection
>    clock                          count    count    count        rate   accuracy phase  cycle    enable   consumer                         id
> ---------------------------------------------------------------------------------------------------------------------------------------------
>  clk:186:6                           1       1        0        250000000   0          0     50000      Y   30200000.dss                    fck                      
>                                                                                                            deviceless                      no_connection_id         
>  clk:186:4                           0       0        0        0           0          0     50000      Y   deviceless                      no_connection_id         
>  clk:186:3                           0       0        0        170000000   0          0     50000      Y   deviceless                      no_connection_id         
>     clk:186:2                        0       0        0        170000000   0          0     50000      Y      30200000.dss                    vp2                      
>                                                                                                               deviceless                      no_connection_id         
>  clk:186:0                           1       1        0        259090909   0          0     50000      Y   oldi@0                          serial                   
>                                                                                                            deviceless                      no_connection_id         
>     clock-divider-oldi               1       1        0        37012987    0          0     50000      Y      30200000.dss                    vp1                      
> 
> Looks like "clock-divider-oldi" doesn't propagate clk_set_rate() to the parent,
> but the parent is being set later independently?
> 

Yes, you are right. The "clock-divider-oldi" does not propagate the
clk_set_rate() to the parent.

The actual parent clock is now owned by the oldi bridge driver, and that
is what sets the clock rate (7 * pixel freq). The equivalent action from
tidss vp (DRM crtc) is a no op in cases of OLDI display pipeline.

Usually, the DRM crtc is enabled first, before _any_ of the DRM bridges
get pre_enabled or enabled.

Since, OLDI is a DRM bridge, the OLDI enable (and by extension the
actual clk_set_rate()) takes place _after_ the DRM crtc has been
enabled (which is why you see the parent being set later on).

DRM crtc enable is where tidss vp attempts (and fails) to set the clock
rate, causing the warning you see initially.


I have attempted to re-order the bridge pre_enable and crtc enable calls
in these patches[0], separately.

While you have mentioned that you did add the prerequisites, could you
confirm that you applied the (now older) dependency patch mentioned in
the v4 cover-letter[1]?
Ideally, you should not observe these concerns if [1] were successfully
applied.

More importantly, if you are already on latest linux-next, I would
request you to use v6 of this OLDI series[2], along with the latest
dependency patches[0], as the older dependency patch is simply not
applicable on latest kernel anymore! =)

I'd appreciate it if you are able to test the latest revisions on your
single-link setup, and report back any issue you see! Thank you! =)


-- 
Regards
Aradhya



[0]: Pre Requisite patches that re-order crtc/encoder/bridge sequences
(latest revision).

a. ("drm/atomic-helper: Refactor crtc & encoder-bridge op loops into
separate functions")
https://lore.kernel.org/all/20250226155737.565931-3-aradhya.bhatia@linux.dev/

b. ("drm/atomic-helper: Separate out bridge pre_enable/post_disable from
enable/disable")
https://lore.kernel.org/all/20250226155737.565931-4-aradhya.bhatia@linux.dev/

c. ("drm/atomic-helper: Re-order bridge chain pre-enable and post-disable")
https://lore.kernel.org/all/20250226155737.565931-5-aradhya.bhatia@linux.dev/


[1]: Dependency patch mentioned in v4 OLDI series.
("drm/atomic-helper: Re-order bridge chain pre-enable and post-disable")
https://lore.kernel.org/all/20240622110929.3115714-11-a-bhatia1@ti.com/


[2]: Latest OLDI series (v6)
https://lore.kernel.org/all/20250226181300.756610-1-aradhya.bhatia@linux.dev/


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

* Re: [PATCH v4 3/3] drm/tidss: Add OLDI bridge support
  2025-03-18 13:35   ` Sverdlin, Alexander
@ 2025-03-20 13:27     ` Aradhya Bhatia
  0 siblings, 0 replies; 16+ messages in thread
From: Aradhya Bhatia @ 2025-03-20 13:27 UTC (permalink / raw)
  To: Sverdlin, Alexander, laurent.pinchart@ideasonboard.com,
	tzimmermann@suse.de, simona@ffwll.ch, jyri.sarha@iki.fi,
	tomi.valkeinen@ideasonboard.com, robh@kernel.org,
	airlied@gmail.com, krzk+dt@kernel.org,
	maarten.lankhorst@linux.intel.com, conor+dt@kernel.org,
	mripard@kernel.org
  Cc: j-choudhary@ti.com, dmitry.baryshkov@linaro.org, u-kumar1@ti.com,
	max.oss.09@gmail.com, francesco@dolcini.it, devarsht@ti.com,
	dri-devel@lists.freedesktop.org, nm@ti.com, vigneshr@ti.com,
	devicetree@vger.kernel.org, praneeth@ti.com,
	linux-kernel@vger.kernel.org

Hi,

On 18/03/25 19:05, Sverdlin, Alexander wrote:
> Hi Aradhya!
> 
> On Sun, 2024-11-24 at 20:06 +0530, 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.
> 
> ...
> 
>> +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) {
> 
> Would for_each_available_child_of_node() make sense here so that
> k3-am62-main.dtsi would have both ports with status = "disabled" and
> the users will enable one or another?
> 

Thank you for the suggestion!

for_each_available_child_of_node() does seem to be the better option.
I will send another revision.


-- 
Regards
Aradhya


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

* Re: [PATCH v4 0/3] drm/tidss: Add OLDI bridge support
  2025-03-20 13:24   ` Aradhya Bhatia
@ 2025-03-20 13:30     ` Sverdlin, Alexander
  2025-03-25 18:57     ` Sverdlin, Alexander
  1 sibling, 0 replies; 16+ messages in thread
From: Sverdlin, Alexander @ 2025-03-20 13:30 UTC (permalink / raw)
  To: aradhya.bhatia@linux.dev
  Cc: j-choudhary@ti.com, u-kumar1@ti.com,
	dri-devel@lists.freedesktop.org, devicetree@vger.kernel.org,
	devarsht@ti.com, linux-kernel@vger.kernel.org, nm@ti.com,
	vigneshr@ti.com, praneeth@ti.com

Hi Aradhya!

On Thu, 2025-03-20 at 18:54 +0530, Aradhya Bhatia wrote:
> While you have mentioned that you did add the prerequisites, could you
> confirm that you applied the (now older) dependency patch mentioned in
> the v4 cover-letter[1]?
> Ideally, you should not observe these concerns if [1] were successfully
> applied.

Seems that I've indeed missed most of the dependencies and only had
"drm/bridge: Introduce early_enable and late disable" so that it builds ;-)

> More importantly, if you are already on latest linux-next, I would
> request you to use v6 of this OLDI series[2], along with the latest
> dependency patches[0], as the older dependency patch is simply not
> applicable on latest kernel anymore! =)
> 
> I'd appreciate it if you are able to test the latest revisions on your
> single-link setup, and report back any issue you see! Thank you! =)

Thanks for the references!
I'll update, test and get back to you!

> [0]: Pre Requisite patches that re-order crtc/encoder/bridge sequences
> (latest revision).
> 
> a. ("drm/atomic-helper: Refactor crtc & encoder-bridge op loops into
> separate functions")
> https://lore.kernel.org/all/20250226155737.565931-3-aradhya.bhatia@linux.dev/
> 
> b. ("drm/atomic-helper: Separate out bridge pre_enable/post_disable from
> enable/disable")
> https://lore.kernel.org/all/20250226155737.565931-4-aradhya.bhatia@linux.dev/
> 
> c. ("drm/atomic-helper: Re-order bridge chain pre-enable and post-disable")
> https://lore.kernel.org/all/20250226155737.565931-5-aradhya.bhatia@linux.dev/
> 
> 
> [1]: Dependency patch mentioned in v4 OLDI series.
> ("drm/atomic-helper: Re-order bridge chain pre-enable and post-disable")
> https://lore.kernel.org/all/20240622110929.3115714-11-a-bhatia1@ti.com/
> 
> 
> [2]: Latest OLDI series (v6)
> https://lore.kernel.org/all/20250226181300.756610-1-aradhya.bhatia@linux.dev/

-- 
Alexander Sverdlin
Siemens AG
www.siemens.com

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

* Re: [PATCH v4 0/3] drm/tidss: Add OLDI bridge support
  2025-03-20 13:24   ` Aradhya Bhatia
  2025-03-20 13:30     ` Sverdlin, Alexander
@ 2025-03-25 18:57     ` Sverdlin, Alexander
  2025-03-29 13:45       ` Aradhya Bhatia
  1 sibling, 1 reply; 16+ messages in thread
From: Sverdlin, Alexander @ 2025-03-25 18:57 UTC (permalink / raw)
  To: aradhya.bhatia@linux.dev
  Cc: j-choudhary@ti.com, u-kumar1@ti.com,
	dri-devel@lists.freedesktop.org, devicetree@vger.kernel.org,
	devarsht@ti.com, linux-kernel@vger.kernel.org, nm@ti.com,
	vigneshr@ti.com, praneeth@ti.com

Hi Aradhya!

On Thu, 2025-03-20 at 18:54 +0530, Aradhya Bhatia wrote:
> > I've tried to test the patchset with necessary pre-requisites and DT additions
> > with a single channel LVDS pannel and while I'm not successful yet, I've also noticed
> > the following warning:
> > 
> > tidss 30200000.dss: vp0: Clock rate 24285714 differs over 5% from requested 37000000

...

> While you have mentioned that you did add the prerequisites, could you
> confirm that you applied the (now older) dependency patch mentioned in
> the v4 cover-letter[1]?
> Ideally, you should not observe these concerns if [1] were successfully
> applied.
> 
> More importantly, if you are already on latest linux-next, I would
> request you to use v6 of this OLDI series[2], along with the latest
> dependency patches[0], as the older dependency patch is simply not
> applicable on latest kernel anymore! =)

Thanks for all the hints and links! I can confirm that with linux-next and this
time all the necessary dependencies applied, I don't see the above warning.

-- 
Alexander Sverdlin
Siemens AG
www.siemens.com

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

* Re: [PATCH v4 0/3] drm/tidss: Add OLDI bridge support
  2025-03-25 18:57     ` Sverdlin, Alexander
@ 2025-03-29 13:45       ` Aradhya Bhatia
  0 siblings, 0 replies; 16+ messages in thread
From: Aradhya Bhatia @ 2025-03-29 13:45 UTC (permalink / raw)
  To: Sverdlin, Alexander
  Cc: j-choudhary@ti.com, u-kumar1@ti.com,
	dri-devel@lists.freedesktop.org, devicetree@vger.kernel.org,
	devarsht@ti.com, linux-kernel@vger.kernel.org, nm@ti.com,
	vigneshr@ti.com, praneeth@ti.com

Hi Alexander,

On 26/03/25 00:27, Sverdlin, Alexander wrote:
> Hi Aradhya!
> 
> On Thu, 2025-03-20 at 18:54 +0530, Aradhya Bhatia wrote:
>>> I've tried to test the patchset with necessary pre-requisites and DT additions
>>> with a single channel LVDS pannel and while I'm not successful yet, I've also noticed
>>> the following warning:
>>>
>>> tidss 30200000.dss: vp0: Clock rate 24285714 differs over 5% from requested 37000000
> 
> ...
> 
>> While you have mentioned that you did add the prerequisites, could you
>> confirm that you applied the (now older) dependency patch mentioned in
>> the v4 cover-letter[1]?
>> Ideally, you should not observe these concerns if [1] were successfully
>> applied.
>>
>> More importantly, if you are already on latest linux-next, I would
>> request you to use v6 of this OLDI series[2], along with the latest
>> dependency patches[0], as the older dependency patch is simply not
>> applicable on latest kernel anymore! =)
> 
> Thanks for all the hints and links! I can confirm that with linux-next and this
> time all the necessary dependencies applied, I don't see the above warning.
> 

I am glad it worked! Thank you for taking the time out, and testing the
patches!  =)

-- 
Regards
Aradhya


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

end of thread, other threads:[~2025-03-29 13:46 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-24 14:36 [PATCH v4 0/3] drm/tidss: Add OLDI bridge support Aradhya Bhatia
2024-11-24 14:36 ` [PATCH v4 1/3] dt-bindings: display: ti,am65x-dss: Re-indent the example Aradhya Bhatia
2024-11-24 14:36 ` [PATCH v4 2/3] dt-bindings: display: ti: Add schema for AM625 OLDI Transmitter Aradhya Bhatia
2024-12-04 14:09   ` Rob Herring
2024-11-24 14:36 ` [PATCH v4 3/3] drm/tidss: Add OLDI bridge support Aradhya Bhatia
2025-03-18 13:35   ` Sverdlin, Alexander
2025-03-20 13:27     ` Aradhya Bhatia
2024-12-03 12:12 ` [PATCH v4 0/3] " Tomi Valkeinen
2024-12-03 18:14   ` Aradhya Bhatia
2024-12-03 18:36     ` Tomi Valkeinen
2024-12-09  4:17       ` Aradhya Bhatia
2025-03-19 18:00 ` Sverdlin, Alexander
2025-03-20 13:24   ` Aradhya Bhatia
2025-03-20 13:30     ` Sverdlin, Alexander
2025-03-25 18:57     ` Sverdlin, Alexander
2025-03-29 13:45       ` Aradhya Bhatia

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