[PATCH v2 0/4] drm/rockchip: add RK3568 LVDS support

[PATCH v2 0/4] drm/rockchip: add RK3568 LVDS support

[Rok Markovic]

RK3568 has a native LVDS transmitter that mainline does not support:
rockchip_lvds.c only binds rk3288 and px30. This series adds it.

The transmitter has no register block of its own. It is programmed
entirely through the GRF and re-uses the MIPI DSI0 D-PHY in
PHY_MODE_LVDS, which phy-rockchip-inno-dsidphy already supports. So the
DT node takes the same shape as px30: reg-less, no clock of its own,
just a phy reference, a GRF handle and a power domain. Nothing in the
LVDS path is clocked from the DSI controller's APB clock - the vendor
BSP scans out with PCLK_DSITX_0's gate off - so the driver claims no
clock either.

Patch 1 is an independent fix: rockchip_lvds_encoder_atomic_check()
never copied the connector's bus_flags into the CRTC state, so a panel's
pixelclk-active could not reach the hardware at all. It is a no-op for
the SoCs supported today - rk3288 and px30 pair with the VOP1 driver,
which never reads bus_flags; only VOP2 consumes it - and becomes live
with patch 3.

Patches 2-4 add the binding, the driver and the SoC DT node. No existing
code paths are modified: patch 3 only adds rk3568 functions, and every
rk3288/px30 function is left byte-identical.

The driver work is based on Alibek Omarov's 2023 posting [1], which was
never merged. The changes on top of it are described in patch 3's
changelog. The important one: phy_power_on() must be called from the
encoder enable path, not from probe. Called at probe it runs the phy's
LVDS bring-up - PLL power-on, settle, mode select, serializer reset
pulse, lane enables - while the GRF has not yet switched the block to
LVDS mode and the VOP is not driving dclk. The serializer is clocked
from dclk and latches dead coming out of that reset. The failure mode is
nasty: every register in the phy, the GRF and the VOP reads back
byte-identical to a working system while the lanes sit at common mode
and never toggle.

[1] https://lore.kernel.org/all/20230119184807.171132-1-a1ba.omarov@gmail.com/

Not covered: LVDS1 / dual-channel. I have no hardware for it, so only
LVDS0 is wired up.

One integration note for anyone using this: VOP2 spreads its windows
only over video ports that have a connected endpoint, and writes
cumulative layer boundaries into OVL_PORT_SEL. If LVDS is put on vp2
while vp1 has no endpoint, the boundaries come out non-monotonic
(PORT0=2, PORT1=8, PORT2=5) and no window ever reaches vp2. Putting LVDS
on vp1 avoids it. That looks like a separate VOP2 bug and is not
addressed here.

Changes in v2:
  - Drop the clock. The block has no register space of its own, so the
    DSI controller's APB clock never reaches it; the vendor BSP scans
    out LVDS with PCLK_DSITX_0 gated off. The binding, the driver and
    the DT node no longer carry a clock. rk3568 now takes the same
    shape as px30. (patches 2, 3, 4)
  - Merge the GRF programming into rk3568_lvds_poweron(): one masked
    write to VO_CON2 instead of two, and check the output type before
    resuming runtime PM. (patch 3)
  - Balance the enable/disable teardown so a .enable() that fails
    partway does not leave the phy power count or the runtime-PM count
    to underflow when the atomic helpers then call .disable(). (patch 3)
  - Drop the in-code comments that restated phy_set_mode() internals or
    the write-masked GRF convention; the phy-ordering rationale stays in
    patch 3's changelog. (patch 3)
  - Cover letter: drop an incorrect reference to a "VO_CON3" register;
    that name is not in the RK3568 TRM.

Testing
-------

Boot-tested on 7.1.3, with this series as posted, on two RK3568 boards:

  - MYIR MYD-LR3568X eval board, 1024x600 LVDS panel at 59.4 MHz
  - Kanardia Nesis V (custom board, MYIR MY3568 SOM), 1280x800 at 72 MHz

Both light up and scan out correctly; VOP2 reports the programmed mode
with real_clk matching the requested pixel clock exactly.

What has *not* been tested:

  - Patch 1 has not been tested on rk3288 or px30 hardware. The argument
    that it cannot regress them is by inspection - VOP1 has no reader of
    bus_flags - not by measurement. Review of that claim is welcome.
  - Dual-channel/LVDS1 is untested (not implemented).
  - Suspend/resume and DPMS cycling are untested. The P2S_EN re-assert
    in patch 3 is what should make a second enable work; it is reasoned
    from the poweroff path clearing it, not from a measured DPMS cycle.

dt_binding_check passes on patch 2. dtbs_check on the board DTB reports
no issues against the new lvds schema. checkpatch --strict is clean on
patches 1, 2 and 4; on patch 3 it emits only two false positives - the
lore URL in the changelog exceeds 75 chars, and a "should normally be
const" on the rk3568 encoder helper funcs, which are const (the wrap
splits "static const" onto its own line, matching the rk3288/px30
entries above them).

Tooling disclosure
------------------

Per Documentation/process/coding-assistants.rst and
generated-content.rst: this series was produced with the help of Claude
(claude-opus-4-8), used interactively across a long debugging session
rather than from a single prompt. The assistant read the vendor BSP
driver and the phy/VOP2 sources, identified the phy-ordering bug that
patch 3 fixes, and wrote the patch text and changelogs. All hardware
testing, every oscilloscope measurement and register dump, and the two
decisive debugging ideas - diffing register state against a working
vendor board, and using MIPI DSI on the shared D-PHY to prove the lanes
were good - were mine. I have reviewed every line and can defend it.
The Signed-off-by is mine alone.

No coccinelle/sparse/smatch runs were involved.

Rok Markovic (4):
  drm/rockchip: lvds: propagate bus_flags to the CRTC state
  dt-bindings: display: rockchip,lvds: add RK3568
  drm/rockchip: lvds: add RK3568 support
  arm64: dts: rockchip: rk356x: add LVDS node

 .../display/rockchip/rockchip,lvds.yaml       |  22 +++
 arch/arm64/boot/dts/rockchip/rk356x-base.dtsi |  23 +++
 drivers/gpu/drm/rockchip/rockchip_lvds.c      | 134 +++++++++++++++++-
 drivers/gpu/drm/rockchip/rockchip_lvds.h      |  10 ++
 4 files changed, 188 insertions(+), 1 deletion(-)

-- 
2.43.0

[PATCH v2 0/4] drm/rockchip: add RK3568 LVDS support

[Rok Markovic]

RK3568 has a native LVDS transmitter that mainline does not support:
rockchip_lvds.c only binds rk3288 and px30. This series adds it.

The transmitter has no register block of its own. It is programmed
entirely through the GRF and re-uses the MIPI DSI0 D-PHY in
PHY_MODE_LVDS, which phy-rockchip-inno-dsidphy already supports. So the
DT node takes the same shape as px30: reg-less, no clock of its own,
just a phy reference, a GRF handle and a power domain. Nothing in the
LVDS path is clocked from the DSI controller's APB clock - the vendor
BSP scans out with PCLK_DSITX_0's gate off - so the driver claims no
clock either.

Patch 1 is an independent fix: rockchip_lvds_encoder_atomic_check()
never copied the connector's bus_flags into the CRTC state, so a panel's
pixelclk-active could not reach the hardware at all. It is a no-op for
the SoCs supported today - rk3288 and px30 pair with the VOP1 driver,
which never reads bus_flags; only VOP2 consumes it - and becomes live
with patch 3.

Patches 2-4 add the binding, the driver and the SoC DT node. No existing
code paths are modified: patch 3 only adds rk3568 functions, and every
rk3288/px30 function is left byte-identical.

The driver work is based on Alibek Omarov's 2023 posting [1], which was
never merged. The changes on top of it are described in patch 3's
changelog. The important one: phy_power_on() must be called from the
encoder enable path, not from probe. Called at probe it runs the phy's
LVDS bring-up - PLL power-on, settle, mode select, serializer reset
pulse, lane enables - while the GRF has not yet switched the block to
LVDS mode and the VOP is not driving dclk. The serializer is clocked
from dclk and latches dead coming out of that reset. The failure mode is
nasty: every register in the phy, the GRF and the VOP reads back
byte-identical to a working system while the lanes sit at common mode
and never toggle.

[1] https://lore.kernel.org/all/20230119184807.171132-1-a1ba.omarov@gmail.com/

Not covered: LVDS1 / dual-channel. I have no hardware for it, so only
LVDS0 is wired up.

One integration note for anyone using this: VOP2 spreads its windows
only over video ports that have a connected endpoint, and writes
cumulative layer boundaries into OVL_PORT_SEL. If LVDS is put on vp2
while vp1 has no endpoint, the boundaries come out non-monotonic
(PORT0=2, PORT1=8, PORT2=5) and no window ever reaches vp2. Putting LVDS
on vp1 avoids it. That looks like a separate VOP2 bug and is not
addressed here.

Changes in v2:
  - Drop the clock. The block has no register space of its own, so the
    DSI controller's APB clock never reaches it; the vendor BSP scans
    out LVDS with PCLK_DSITX_0 gated off. The binding, the driver and
    the DT node no longer carry a clock. rk3568 now takes the same
    shape as px30. (patches 2, 3, 4)
  - Merge the GRF programming into rk3568_lvds_poweron(): one masked
    write to VO_CON2 instead of two, and check the output type before
    resuming runtime PM. (patch 3)
  - Balance the enable/disable teardown so a .enable() that fails
    partway does not leave the phy power count or the runtime-PM count
    to underflow when the atomic helpers then call .disable(). (patch 3)
  - Drop the in-code comments that restated phy_set_mode() internals or
    the write-masked GRF convention; the phy-ordering rationale stays in
    patch 3's changelog. (patch 3)
  - Cover letter: drop an incorrect reference to a "VO_CON3" register;
    that name is not in the RK3568 TRM.

Testing
-------

Boot-tested on 7.1.3, with this series as posted, on two RK3568 boards:

  - MYIR MYD-LR3568X eval board, 1024x600 LVDS panel at 59.4 MHz
  - Kanardia Nesis V (custom board, MYIR MY3568 SOM), 1280x800 at 72 MHz

Both light up and scan out correctly; VOP2 reports the programmed mode
with real_clk matching the requested pixel clock exactly.

What has *not* been tested:

  - Patch 1 has not been tested on rk3288 or px30 hardware. The argument
    that it cannot regress them is by inspection - VOP1 has no reader of
    bus_flags - not by measurement. Review of that claim is welcome.
  - Dual-channel/LVDS1 is untested (not implemented).
  - Suspend/resume and DPMS cycling are untested. The P2S_EN re-assert
    in patch 3 is what should make a second enable work; it is reasoned
    from the poweroff path clearing it, not from a measured DPMS cycle.

dt_binding_check passes on patch 2. dtbs_check on the board DTB reports
no issues against the new lvds schema. checkpatch --strict is clean on
patches 1, 2 and 4; on patch 3 it emits only two false positives - the
lore URL in the changelog exceeds 75 chars, and a "should normally be
const" on the rk3568 encoder helper funcs, which are const (the wrap
splits "static const" onto its own line, matching the rk3288/px30
entries above them).

Tooling disclosure
------------------

Per Documentation/process/coding-assistants.rst and
generated-content.rst: this series was produced with the help of Claude
(claude-opus-4-8), used interactively across a long debugging session
rather than from a single prompt. The assistant read the vendor BSP
driver and the phy/VOP2 sources, identified the phy-ordering bug that
patch 3 fixes, and wrote the patch text and changelogs. All hardware
testing, every oscilloscope measurement and register dump, and the two
decisive debugging ideas - diffing register state against a working
vendor board, and using MIPI DSI on the shared D-PHY to prove the lanes
were good - were mine. I have reviewed every line and can defend it.
The Signed-off-by is mine alone.

No coccinelle/sparse/smatch runs were involved.

Rok Markovic (4):
  drm/rockchip: lvds: propagate bus_flags to the CRTC state
  dt-bindings: display: rockchip,lvds: add RK3568
  drm/rockchip: lvds: add RK3568 support
  arm64: dts: rockchip: rk356x: add LVDS node

 .../display/rockchip/rockchip,lvds.yaml       |  22 +++
 arch/arm64/boot/dts/rockchip/rk356x-base.dtsi |  23 +++
 drivers/gpu/drm/rockchip/rockchip_lvds.c      | 134 +++++++++++++++++-
 drivers/gpu/drm/rockchip/rockchip_lvds.h      |  10 ++
 4 files changed, 188 insertions(+), 1 deletion(-)

-- 
2.43.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH v2 1/4] drm/rockchip: lvds: propagate bus_flags to the CRTC state

[Rok Markovic]

rockchip_lvds_encoder_atomic_check() sets output_mode and output_type
but never copies the connector's bus_flags into the Rockchip CRTC
state, unlike dw_dp-rockchip.c and dw-mipi-dsi2-rockchip.c which both
do.

panel-lvds parses pixelclk-active from the DT display timing and
publishes it on the connector as DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE.
VOP2 reads that flag back out of the CRTC state to decide
POLFLAG_DCLK_INV. With the flag dropped here, vcstate->bus_flags is
always zero, so a panel that asks to be clocked on the falling edge is
driven on the rising one, and there is no way to express the panel's
requirement from DT at all.

No functional change for the SoCs currently supported by this driver:
rk3288 and px30 pair with the VOP1 driver (rockchip_drm_vop.c), which
never reads bus_flags - only VOP2 consumes it. The flag becomes live
with the RK3568 support added later in this series.

Signed-off-by: Rok Markovic <rok@kanardia.eu>
Assisted-by: Claude:claude-opus-4-8
---
No changes since v1.

 drivers/gpu/drm/rockchip/rockchip_lvds.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.c b/drivers/gpu/drm/rockchip/rockchip_lvds.c
index 75f898a..95fa0a9 100644
--- a/drivers/gpu/drm/rockchip/rockchip_lvds.c
+++ b/drivers/gpu/drm/rockchip/rockchip_lvds.c
@@ -129,7 +129,15 @@ rockchip_lvds_encoder_atomic_check(struct drm_encoder *encoder,
 				   struct drm_connector_state *conn_state)
 {
 	struct rockchip_crtc_state *s = to_rockchip_crtc_state(crtc_state);
-
+	struct drm_connector *connector = conn_state->connector;
+
+	/*
+	 * The VOP derives the pixel clock polarity from this. Without it a
+	 * panel that declares pixelclk-active = <0> is clocked on the wrong
+	 * edge. panel-lvds fills the connector's bus_flags in from the DT
+	 * display timing.
+	 */
+	s->bus_flags = connector->display_info.bus_flags;
 	s->output_mode = ROCKCHIP_OUT_MODE_P888;
 	s->output_type = DRM_MODE_CONNECTOR_LVDS;
 
-- 
2.43.0

[PATCH v2 1/4] drm/rockchip: lvds: propagate bus_flags to the CRTC state

[Rok Markovic]

rockchip_lvds_encoder_atomic_check() sets output_mode and output_type
but never copies the connector's bus_flags into the Rockchip CRTC
state, unlike dw_dp-rockchip.c and dw-mipi-dsi2-rockchip.c which both
do.

panel-lvds parses pixelclk-active from the DT display timing and
publishes it on the connector as DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE.
VOP2 reads that flag back out of the CRTC state to decide
POLFLAG_DCLK_INV. With the flag dropped here, vcstate->bus_flags is
always zero, so a panel that asks to be clocked on the falling edge is
driven on the rising one, and there is no way to express the panel's
requirement from DT at all.

No functional change for the SoCs currently supported by this driver:
rk3288 and px30 pair with the VOP1 driver (rockchip_drm_vop.c), which
never reads bus_flags - only VOP2 consumes it. The flag becomes live
with the RK3568 support added later in this series.

Signed-off-by: Rok Markovic <rok@kanardia.eu>
Assisted-by: Claude:claude-opus-4-8
---
No changes since v1.

 drivers/gpu/drm/rockchip/rockchip_lvds.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.c b/drivers/gpu/drm/rockchip/rockchip_lvds.c
index 75f898a..95fa0a9 100644
--- a/drivers/gpu/drm/rockchip/rockchip_lvds.c
+++ b/drivers/gpu/drm/rockchip/rockchip_lvds.c
@@ -129,7 +129,15 @@ rockchip_lvds_encoder_atomic_check(struct drm_encoder *encoder,
 				   struct drm_connector_state *conn_state)
 {
 	struct rockchip_crtc_state *s = to_rockchip_crtc_state(crtc_state);
-
+	struct drm_connector *connector = conn_state->connector;
+
+	/*
+	 * The VOP derives the pixel clock polarity from this. Without it a
+	 * panel that declares pixelclk-active = <0> is clocked on the wrong
+	 * edge. panel-lvds fills the connector's bus_flags in from the DT
+	 * display timing.
+	 */
+	s->bus_flags = connector->display_info.bus_flags;
 	s->output_mode = ROCKCHIP_OUT_MODE_P888;
 	s->output_type = DRM_MODE_CONNECTOR_LVDS;
 
-- 
2.43.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH v2 2/4] dt-bindings: display: rockchip,lvds: add RK3568

[Rok Markovic]

The RK3568 LVDS transmitter has no register block of its own: it is
programmed entirely through the GRF and re-uses the MIPI DSI0 D-PHY
(rockchip,rk3568-dsi-dphy) in LVDS mode. It therefore takes the same
shape as px30: phys/phy-names, no reg, and no clock of its own.

Since the block exposes no MMIO, nothing in the LVDS path is clocked
from the DSI controller's APB clock; that gate is held off while LVDS
scans out. It has no analog supplies of its own and no pinctrl either:
the lanes are dedicated D-PHY pins, not muxable GPIOs.

Signed-off-by: Rok Markovic <rok@kanardia.eu>
Assisted-by: Claude:claude-opus-4-8
---
Changes in v2:
  - RK3568 now takes no clock (clocks/clock-names disabled, as for
    px30): the block has no register space, so nothing consumes the
    DSI controller's APB clock.

 .../display/rockchip/rockchip,lvds.yaml       | 22 +++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip,lvds.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip,lvds.yaml
index 03b002a..b77d4fa 100644
--- a/Documentation/devicetree/bindings/display/rockchip/rockchip,lvds.yaml
+++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,lvds.yaml
@@ -15,6 +15,7 @@ properties:
     enum:
       - rockchip,px30-lvds
       - rockchip,rk3288-lvds
+      - rockchip,rk3568-lvds
 
   reg:
     maxItems: 1
@@ -121,6 +122,27 @@ allOf:
         - avdd1v8-supply
         - avdd3v3-supply
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: rockchip,rk3568-lvds
+
+    then:
+      properties:
+        reg: false
+        clocks: false
+        clock-names: false
+        avdd1v0-supply: false
+        avdd1v8-supply: false
+        avdd3v3-supply: false
+        pinctrl-names: false
+        pinctrl-0: false
+
+      required:
+        - phys
+        - phy-names
+
 additionalProperties: false
 
 examples:
-- 
2.43.0

[PATCH v2 2/4] dt-bindings: display: rockchip,lvds: add RK3568

[Rok Markovic]

The RK3568 LVDS transmitter has no register block of its own: it is
programmed entirely through the GRF and re-uses the MIPI DSI0 D-PHY
(rockchip,rk3568-dsi-dphy) in LVDS mode. It therefore takes the same
shape as px30: phys/phy-names, no reg, and no clock of its own.

Since the block exposes no MMIO, nothing in the LVDS path is clocked
from the DSI controller's APB clock; that gate is held off while LVDS
scans out. It has no analog supplies of its own and no pinctrl either:
the lanes are dedicated D-PHY pins, not muxable GPIOs.

Signed-off-by: Rok Markovic <rok@kanardia.eu>
Assisted-by: Claude:claude-opus-4-8
---
Changes in v2:
  - RK3568 now takes no clock (clocks/clock-names disabled, as for
    px30): the block has no register space, so nothing consumes the
    DSI controller's APB clock.

 .../display/rockchip/rockchip,lvds.yaml       | 22 +++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip,lvds.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip,lvds.yaml
index 03b002a..b77d4fa 100644
--- a/Documentation/devicetree/bindings/display/rockchip/rockchip,lvds.yaml
+++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,lvds.yaml
@@ -15,6 +15,7 @@ properties:
     enum:
       - rockchip,px30-lvds
       - rockchip,rk3288-lvds
+      - rockchip,rk3568-lvds
 
   reg:
     maxItems: 1
@@ -121,6 +122,27 @@ allOf:
         - avdd1v8-supply
         - avdd3v3-supply
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: rockchip,rk3568-lvds
+
+    then:
+      properties:
+        reg: false
+        clocks: false
+        clock-names: false
+        avdd1v0-supply: false
+        avdd1v8-supply: false
+        avdd3v3-supply: false
+        pinctrl-names: false
+        pinctrl-0: false
+
+      required:
+        - phys
+        - phy-names
+
 additionalProperties: false
 
 examples:
-- 
2.43.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH v2 3/4] drm/rockchip: lvds: add RK3568 support

[Rok Markovic]

The RK3568 LVDS transmitter has no register block of its own. It is
driven entirely through the GRF and re-uses the MIPI DSI0 D-PHY in
PHY_MODE_LVDS, which phy-rockchip-inno-dsidphy already supports.

Based on Alibek Omarov's earlier posting [1], with the changes below.

Power the D-PHY from the encoder enable path rather than from probe.
phy_power_on() runs the phy driver's whole LVDS bring-up: PLL and
bandgap power-on, a settle, PLL mode select, then a reset pulse of the
serializer and the lane enables. None of that is safe at probe time -
the GRF has not yet switched the block to LVDS mode (LVDS0_MODE_EN is
set from rk3568_lvds_poweron(), i.e. the enable path) and the VOP is
not driving dclk. The serializer is clocked from dclk and latches its
state coming out of that reset, so it comes up dead and stays dead.
The failure is silent: every register in the phy, the GRF and the VOP
reads back exactly as on a working system while the lanes sit at
common mode and never toggle.

Program RK3568_LVDS0_DCLK_INV_SEL from the CRTC state's bus_flags so
the panel's declared pixelclk-active is honoured on the LVDS block as
well as on the VOP pin polarity. Both have to agree with the edge the
panel samples on.

Re-assert RK3568_LVDS0_P2S_EN in rk3568_lvds_poweron().
rk3568_lvds_poweroff() clears MODE_EN and P2S_EN together, so setting
P2S_EN once at probe would leave the parallel-to-serial converter off
after the first disable/enable cycle.

Use regmap_write() rather than regmap_update_bits() for the GRF. These
registers are write-masked - the upper 16 bits select which of the
lower 16 a write may change - so there is nothing to preserve and no
reason to read first. Passing a FIELD_PREP_WM16() value as both the
mask and the value of an update_bits() applies the masking twice and
only works by accident.

Take no clock. The block exposes no MMIO of its own, so there is
nothing for the DSI controller's APB clock to reach; the vendor BSP
scans out LVDS with PCLK_DSITX_0's gate off, and so does this driver.

Between the above, nothing needs programming at probe at all: the GRF
is written entirely from the enable path, so px30_lvds_probe() is left
alone rather than being refactored into a shared phy helper.

Track the enabled state so that a .enable() that bails out unwinds
itself and the .disable() that the atomic helpers still call afterwards
becomes a no-op, rather than underflowing the phy power count and the
runtime-PM count of an encoder that never came up.

[1] https://lore.kernel.org/all/20230119184807.171132-1-a1ba.omarov@gmail.com/

Co-developed-by: Alibek Omarov <a1ba.omarov@gmail.com>
Signed-off-by: Alibek Omarov <a1ba.omarov@gmail.com>
Signed-off-by: Rok Markovic <rok@kanardia.eu>
Assisted-by: Claude:claude-opus-4-8
---
Changes in v2:
  - Drop the clock. rk3568_lvds_poweron() no longer enables a pclk;
    the block has no MMIO for the DSI APB clock to reach, and the
    vendor BSP scans out with PCLK_DSITX_0 gated off.
  - Merge rk3568_lvds_grf_config() into rk3568_lvds_poweron(): a
    single masked write to VO_CON2 instead of two, and check the
    output type before resuming runtime PM.
  - Balance the enable/disable teardown so a .enable() that fails
    partway does not underflow the phy power and runtime-PM counts
    when the atomic helpers then call .disable().
  - Use pm_runtime_resume_and_get(), matching the rk3288/px30 paths.
  - Drop the in-code comments restating phy_set_mode() internals and
    the write-masked GRF convention; the ordering rationale is in the
    commit message.

 drivers/gpu/drm/rockchip/rockchip_lvds.c | 124 +++++++++++++++++++++++
 drivers/gpu/drm/rockchip/rockchip_lvds.h |  10 ++
 2 files changed, 134 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.c b/drivers/gpu/drm/rockchip/rockchip_lvds.c
index 95fa0a9..9af21fc 100644
--- a/drivers/gpu/drm/rockchip/rockchip_lvds.c
+++ b/drivers/gpu/drm/rockchip/rockchip_lvds.c
@@ -51,6 +51,7 @@ struct rockchip_lvds {
 	struct regmap *grf;
 	struct clk *pclk;
 	struct phy *dphy;
+	bool enabled;
 	const struct rockchip_lvds_soc_data *soc_data;
 	int output; /* rgb lvds or dual lvds output */
 	int format; /* vesa or jeida format */
@@ -435,6 +436,100 @@ static void px30_lvds_encoder_disable(struct drm_encoder *encoder)
 	drm_panel_unprepare(lvds->panel);
 }
 
+static int rk3568_lvds_poweron(struct drm_encoder *encoder)
+{
+	struct rockchip_lvds *lvds = encoder_to_lvds(encoder);
+	struct rockchip_crtc_state *s =
+		to_rockchip_crtc_state(encoder->crtc->state);
+	bool negedge = !!(s->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE);
+	int ret;
+
+	if (lvds->output != DISPLAY_OUTPUT_LVDS) {
+		DRM_DEV_ERROR(lvds->dev, "Unsupported display output %d\n",
+			      lvds->output);
+		return -EINVAL;
+	}
+
+	ret = pm_runtime_resume_and_get(lvds->dev);
+	if (ret < 0) {
+		DRM_DEV_ERROR(lvds->dev, "failed to get pm runtime: %d\n", ret);
+		return ret;
+	}
+
+	regmap_write(lvds->grf, RK3568_GRF_VO_CON2,
+		     RK3568_LVDS0_MODE_EN(1) | RK3568_LVDS0_P2S_EN(1) |
+		     RK3568_LVDS0_DCLK_INV_SEL(negedge));
+	regmap_write(lvds->grf, RK3568_GRF_VO_CON0,
+		     RK3568_LVDS0_SELECT(lvds->format) |
+		     RK3568_LVDS0_MSBSEL(1));
+
+	return 0;
+}
+
+static void rk3568_lvds_poweroff(struct rockchip_lvds *lvds)
+{
+	regmap_write(lvds->grf, RK3568_GRF_VO_CON2,
+		     RK3568_LVDS0_MODE_EN(0) | RK3568_LVDS0_P2S_EN(0));
+
+	pm_runtime_put(lvds->dev);
+}
+
+static void rk3568_lvds_encoder_enable(struct drm_encoder *encoder)
+{
+	struct rockchip_lvds *lvds = encoder_to_lvds(encoder);
+	int ret;
+
+	drm_panel_prepare(lvds->panel);
+
+	ret = rk3568_lvds_poweron(encoder);
+	if (ret) {
+		DRM_DEV_ERROR(lvds->dev, "failed to power on LVDS: %d\n", ret);
+		goto err_unprepare;
+	}
+
+	/* The GRF must be in LVDS mode with dclk running before the phy. */
+	ret = phy_set_mode(lvds->dphy, PHY_MODE_LVDS);
+	if (ret) {
+		DRM_DEV_ERROR(lvds->dev, "failed to set phy mode: %d\n", ret);
+		goto err_poweroff;
+	}
+
+	ret = phy_power_on(lvds->dphy);
+	if (ret) {
+		DRM_DEV_ERROR(lvds->dev, "failed to power on phy: %d\n", ret);
+		goto err_poweroff;
+	}
+
+	lvds->enabled = true;
+	drm_panel_enable(lvds->panel);
+	return;
+
+err_poweroff:
+	rk3568_lvds_poweroff(lvds);
+err_unprepare:
+	drm_panel_unprepare(lvds->panel);
+}
+
+static void rk3568_lvds_encoder_disable(struct drm_encoder *encoder)
+{
+	struct rockchip_lvds *lvds = encoder_to_lvds(encoder);
+
+	/*
+	 * .enable() cannot report failure, so the atomic helpers call this even
+	 * when it bailed out and already unwound itself. Tear down only what an
+	 * enable that ran to completion left behind, or the phy power count and
+	 * the runtime-PM count of a half-enabled encoder underflow.
+	 */
+	if (!lvds->enabled)
+		return;
+
+	drm_panel_disable(lvds->panel);
+	phy_power_off(lvds->dphy);
+	rk3568_lvds_poweroff(lvds);
+	drm_panel_unprepare(lvds->panel);
+	lvds->enabled = false;
+}
+
 static const
 struct drm_encoder_helper_funcs rk3288_lvds_encoder_helper_funcs = {
 	.enable = rk3288_lvds_encoder_enable,
@@ -449,6 +544,13 @@ struct drm_encoder_helper_funcs px30_lvds_encoder_helper_funcs = {
 	.atomic_check = rockchip_lvds_encoder_atomic_check,
 };
 
+static const
+struct drm_encoder_helper_funcs rk3568_lvds_encoder_helper_funcs = {
+	.enable = rk3568_lvds_encoder_enable,
+	.disable = rk3568_lvds_encoder_disable,
+	.atomic_check = rockchip_lvds_encoder_atomic_check,
+};
+
 static int rk3288_lvds_probe(struct platform_device *pdev,
 			     struct rockchip_lvds *lvds)
 {
@@ -512,6 +614,17 @@ static int px30_lvds_probe(struct platform_device *pdev,
 	return phy_power_on(lvds->dphy);
 }
 
+static int rk3568_lvds_probe(struct platform_device *pdev,
+			     struct rockchip_lvds *lvds)
+{
+	/* The phy is powered on from the encoder enable path, not here. */
+	lvds->dphy = devm_phy_get(&pdev->dev, "dphy");
+	if (IS_ERR(lvds->dphy))
+		return PTR_ERR(lvds->dphy);
+
+	return phy_init(lvds->dphy);
+}
+
 static const struct rockchip_lvds_soc_data rk3288_lvds_data = {
 	.probe = rk3288_lvds_probe,
 	.helper_funcs = &rk3288_lvds_encoder_helper_funcs,
@@ -522,6 +635,11 @@ static const struct rockchip_lvds_soc_data px30_lvds_data = {
 	.helper_funcs = &px30_lvds_encoder_helper_funcs,
 };
 
+static const struct rockchip_lvds_soc_data rk3568_lvds_data = {
+	.probe = rk3568_lvds_probe,
+	.helper_funcs = &rk3568_lvds_encoder_helper_funcs,
+};
+
 static const struct of_device_id rockchip_lvds_dt_ids[] = {
 	{
 		.compatible = "rockchip,rk3288-lvds",
@@ -531,6 +649,10 @@ static const struct of_device_id rockchip_lvds_dt_ids[] = {
 		.compatible = "rockchip,px30-lvds",
 		.data = &px30_lvds_data
 	},
+	{
+		.compatible = "rockchip,rk3568-lvds",
+		.data = &rk3568_lvds_data
+	},
 	{}
 };
 MODULE_DEVICE_TABLE(of, rockchip_lvds_dt_ids);
@@ -601,6 +723,8 @@ static int rockchip_lvds_bind(struct device *dev, struct device *master,
 	encoder = &lvds->encoder.encoder;
 	encoder->possible_crtcs = drm_of_find_possible_crtcs(drm_dev,
 							     dev->of_node);
+	rockchip_drm_encoder_set_crtc_endpoint_id(&lvds->encoder,
+						  dev->of_node, 0, 0);
 
 	ret = drm_simple_encoder_init(drm_dev, encoder, DRM_MODE_ENCODER_LVDS);
 	if (ret < 0) {
diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.h b/drivers/gpu/drm/rockchip/rockchip_lvds.h
index 2d92447..93d3415 100644
--- a/drivers/gpu/drm/rockchip/rockchip_lvds.h
+++ b/drivers/gpu/drm/rockchip/rockchip_lvds.h
@@ -121,4 +121,14 @@
 #define   PX30_LVDS_P2S_EN(val)			FIELD_PREP_WM16(BIT(6), (val))
 #define   PX30_LVDS_VOP_SEL(val)		FIELD_PREP_WM16(BIT(1), (val))
 
+#define RK3568_GRF_VO_CON0			0x0360
+#define   RK3568_LVDS0_SELECT(val)		FIELD_PREP_WM16(GENMASK(5, 4), (val))
+#define   RK3568_LVDS0_MSBSEL(val)		FIELD_PREP_WM16(BIT(3), (val))
+
+#define RK3568_GRF_VO_CON2			0x0368
+#define   RK3568_LVDS0_DCLK_INV_SEL(val)	FIELD_PREP_WM16(BIT(9), (val))
+#define   RK3568_LVDS0_DCLK_DIV2_SEL(val)	FIELD_PREP_WM16(BIT(8), (val))
+#define   RK3568_LVDS0_MODE_EN(val)		FIELD_PREP_WM16(BIT(1), (val))
+#define   RK3568_LVDS0_P2S_EN(val)		FIELD_PREP_WM16(BIT(0), (val))
+
 #endif /* _ROCKCHIP_LVDS_ */
-- 
2.43.0

[PATCH v2 3/4] drm/rockchip: lvds: add RK3568 support

[Rok Markovic]

The RK3568 LVDS transmitter has no register block of its own. It is
driven entirely through the GRF and re-uses the MIPI DSI0 D-PHY in
PHY_MODE_LVDS, which phy-rockchip-inno-dsidphy already supports.

Based on Alibek Omarov's earlier posting [1], with the changes below.

Power the D-PHY from the encoder enable path rather than from probe.
phy_power_on() runs the phy driver's whole LVDS bring-up: PLL and
bandgap power-on, a settle, PLL mode select, then a reset pulse of the
serializer and the lane enables. None of that is safe at probe time -
the GRF has not yet switched the block to LVDS mode (LVDS0_MODE_EN is
set from rk3568_lvds_poweron(), i.e. the enable path) and the VOP is
not driving dclk. The serializer is clocked from dclk and latches its
state coming out of that reset, so it comes up dead and stays dead.
The failure is silent: every register in the phy, the GRF and the VOP
reads back exactly as on a working system while the lanes sit at
common mode and never toggle.

Program RK3568_LVDS0_DCLK_INV_SEL from the CRTC state's bus_flags so
the panel's declared pixelclk-active is honoured on the LVDS block as
well as on the VOP pin polarity. Both have to agree with the edge the
panel samples on.

Re-assert RK3568_LVDS0_P2S_EN in rk3568_lvds_poweron().
rk3568_lvds_poweroff() clears MODE_EN and P2S_EN together, so setting
P2S_EN once at probe would leave the parallel-to-serial converter off
after the first disable/enable cycle.

Use regmap_write() rather than regmap_update_bits() for the GRF. These
registers are write-masked - the upper 16 bits select which of the
lower 16 a write may change - so there is nothing to preserve and no
reason to read first. Passing a FIELD_PREP_WM16() value as both the
mask and the value of an update_bits() applies the masking twice and
only works by accident.

Take no clock. The block exposes no MMIO of its own, so there is
nothing for the DSI controller's APB clock to reach; the vendor BSP
scans out LVDS with PCLK_DSITX_0's gate off, and so does this driver.

Between the above, nothing needs programming at probe at all: the GRF
is written entirely from the enable path, so px30_lvds_probe() is left
alone rather than being refactored into a shared phy helper.

Track the enabled state so that a .enable() that bails out unwinds
itself and the .disable() that the atomic helpers still call afterwards
becomes a no-op, rather than underflowing the phy power count and the
runtime-PM count of an encoder that never came up.

[1] https://lore.kernel.org/all/20230119184807.171132-1-a1ba.omarov@gmail.com/

Co-developed-by: Alibek Omarov <a1ba.omarov@gmail.com>
Signed-off-by: Alibek Omarov <a1ba.omarov@gmail.com>
Signed-off-by: Rok Markovic <rok@kanardia.eu>
Assisted-by: Claude:claude-opus-4-8
---
Changes in v2:
  - Drop the clock. rk3568_lvds_poweron() no longer enables a pclk;
    the block has no MMIO for the DSI APB clock to reach, and the
    vendor BSP scans out with PCLK_DSITX_0 gated off.
  - Merge rk3568_lvds_grf_config() into rk3568_lvds_poweron(): a
    single masked write to VO_CON2 instead of two, and check the
    output type before resuming runtime PM.
  - Balance the enable/disable teardown so a .enable() that fails
    partway does not underflow the phy power and runtime-PM counts
    when the atomic helpers then call .disable().
  - Use pm_runtime_resume_and_get(), matching the rk3288/px30 paths.
  - Drop the in-code comments restating phy_set_mode() internals and
    the write-masked GRF convention; the ordering rationale is in the
    commit message.

 drivers/gpu/drm/rockchip/rockchip_lvds.c | 124 +++++++++++++++++++++++
 drivers/gpu/drm/rockchip/rockchip_lvds.h |  10 ++
 2 files changed, 134 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.c b/drivers/gpu/drm/rockchip/rockchip_lvds.c
index 95fa0a9..9af21fc 100644
--- a/drivers/gpu/drm/rockchip/rockchip_lvds.c
+++ b/drivers/gpu/drm/rockchip/rockchip_lvds.c
@@ -51,6 +51,7 @@ struct rockchip_lvds {
 	struct regmap *grf;
 	struct clk *pclk;
 	struct phy *dphy;
+	bool enabled;
 	const struct rockchip_lvds_soc_data *soc_data;
 	int output; /* rgb lvds or dual lvds output */
 	int format; /* vesa or jeida format */
@@ -435,6 +436,100 @@ static void px30_lvds_encoder_disable(struct drm_encoder *encoder)
 	drm_panel_unprepare(lvds->panel);
 }
 
+static int rk3568_lvds_poweron(struct drm_encoder *encoder)
+{
+	struct rockchip_lvds *lvds = encoder_to_lvds(encoder);
+	struct rockchip_crtc_state *s =
+		to_rockchip_crtc_state(encoder->crtc->state);
+	bool negedge = !!(s->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE);
+	int ret;
+
+	if (lvds->output != DISPLAY_OUTPUT_LVDS) {
+		DRM_DEV_ERROR(lvds->dev, "Unsupported display output %d\n",
+			      lvds->output);
+		return -EINVAL;
+	}
+
+	ret = pm_runtime_resume_and_get(lvds->dev);
+	if (ret < 0) {
+		DRM_DEV_ERROR(lvds->dev, "failed to get pm runtime: %d\n", ret);
+		return ret;
+	}
+
+	regmap_write(lvds->grf, RK3568_GRF_VO_CON2,
+		     RK3568_LVDS0_MODE_EN(1) | RK3568_LVDS0_P2S_EN(1) |
+		     RK3568_LVDS0_DCLK_INV_SEL(negedge));
+	regmap_write(lvds->grf, RK3568_GRF_VO_CON0,
+		     RK3568_LVDS0_SELECT(lvds->format) |
+		     RK3568_LVDS0_MSBSEL(1));
+
+	return 0;
+}
+
+static void rk3568_lvds_poweroff(struct rockchip_lvds *lvds)
+{
+	regmap_write(lvds->grf, RK3568_GRF_VO_CON2,
+		     RK3568_LVDS0_MODE_EN(0) | RK3568_LVDS0_P2S_EN(0));
+
+	pm_runtime_put(lvds->dev);
+}
+
+static void rk3568_lvds_encoder_enable(struct drm_encoder *encoder)
+{
+	struct rockchip_lvds *lvds = encoder_to_lvds(encoder);
+	int ret;
+
+	drm_panel_prepare(lvds->panel);
+
+	ret = rk3568_lvds_poweron(encoder);
+	if (ret) {
+		DRM_DEV_ERROR(lvds->dev, "failed to power on LVDS: %d\n", ret);
+		goto err_unprepare;
+	}
+
+	/* The GRF must be in LVDS mode with dclk running before the phy. */
+	ret = phy_set_mode(lvds->dphy, PHY_MODE_LVDS);
+	if (ret) {
+		DRM_DEV_ERROR(lvds->dev, "failed to set phy mode: %d\n", ret);
+		goto err_poweroff;
+	}
+
+	ret = phy_power_on(lvds->dphy);
+	if (ret) {
+		DRM_DEV_ERROR(lvds->dev, "failed to power on phy: %d\n", ret);
+		goto err_poweroff;
+	}
+
+	lvds->enabled = true;
+	drm_panel_enable(lvds->panel);
+	return;
+
+err_poweroff:
+	rk3568_lvds_poweroff(lvds);
+err_unprepare:
+	drm_panel_unprepare(lvds->panel);
+}
+
+static void rk3568_lvds_encoder_disable(struct drm_encoder *encoder)
+{
+	struct rockchip_lvds *lvds = encoder_to_lvds(encoder);
+
+	/*
+	 * .enable() cannot report failure, so the atomic helpers call this even
+	 * when it bailed out and already unwound itself. Tear down only what an
+	 * enable that ran to completion left behind, or the phy power count and
+	 * the runtime-PM count of a half-enabled encoder underflow.
+	 */
+	if (!lvds->enabled)
+		return;
+
+	drm_panel_disable(lvds->panel);
+	phy_power_off(lvds->dphy);
+	rk3568_lvds_poweroff(lvds);
+	drm_panel_unprepare(lvds->panel);
+	lvds->enabled = false;
+}
+
 static const
 struct drm_encoder_helper_funcs rk3288_lvds_encoder_helper_funcs = {
 	.enable = rk3288_lvds_encoder_enable,
@@ -449,6 +544,13 @@ struct drm_encoder_helper_funcs px30_lvds_encoder_helper_funcs = {
 	.atomic_check = rockchip_lvds_encoder_atomic_check,
 };
 
+static const
+struct drm_encoder_helper_funcs rk3568_lvds_encoder_helper_funcs = {
+	.enable = rk3568_lvds_encoder_enable,
+	.disable = rk3568_lvds_encoder_disable,
+	.atomic_check = rockchip_lvds_encoder_atomic_check,
+};
+
 static int rk3288_lvds_probe(struct platform_device *pdev,
 			     struct rockchip_lvds *lvds)
 {
@@ -512,6 +614,17 @@ static int px30_lvds_probe(struct platform_device *pdev,
 	return phy_power_on(lvds->dphy);
 }
 
+static int rk3568_lvds_probe(struct platform_device *pdev,
+			     struct rockchip_lvds *lvds)
+{
+	/* The phy is powered on from the encoder enable path, not here. */
+	lvds->dphy = devm_phy_get(&pdev->dev, "dphy");
+	if (IS_ERR(lvds->dphy))
+		return PTR_ERR(lvds->dphy);
+
+	return phy_init(lvds->dphy);
+}
+
 static const struct rockchip_lvds_soc_data rk3288_lvds_data = {
 	.probe = rk3288_lvds_probe,
 	.helper_funcs = &rk3288_lvds_encoder_helper_funcs,
@@ -522,6 +635,11 @@ static const struct rockchip_lvds_soc_data px30_lvds_data = {
 	.helper_funcs = &px30_lvds_encoder_helper_funcs,
 };
 
+static const struct rockchip_lvds_soc_data rk3568_lvds_data = {
+	.probe = rk3568_lvds_probe,
+	.helper_funcs = &rk3568_lvds_encoder_helper_funcs,
+};
+
 static const struct of_device_id rockchip_lvds_dt_ids[] = {
 	{
 		.compatible = "rockchip,rk3288-lvds",
@@ -531,6 +649,10 @@ static const struct of_device_id rockchip_lvds_dt_ids[] = {
 		.compatible = "rockchip,px30-lvds",
 		.data = &px30_lvds_data
 	},
+	{
+		.compatible = "rockchip,rk3568-lvds",
+		.data = &rk3568_lvds_data
+	},
 	{}
 };
 MODULE_DEVICE_TABLE(of, rockchip_lvds_dt_ids);
@@ -601,6 +723,8 @@ static int rockchip_lvds_bind(struct device *dev, struct device *master,
 	encoder = &lvds->encoder.encoder;
 	encoder->possible_crtcs = drm_of_find_possible_crtcs(drm_dev,
 							     dev->of_node);
+	rockchip_drm_encoder_set_crtc_endpoint_id(&lvds->encoder,
+						  dev->of_node, 0, 0);
 
 	ret = drm_simple_encoder_init(drm_dev, encoder, DRM_MODE_ENCODER_LVDS);
 	if (ret < 0) {
diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.h b/drivers/gpu/drm/rockchip/rockchip_lvds.h
index 2d92447..93d3415 100644
--- a/drivers/gpu/drm/rockchip/rockchip_lvds.h
+++ b/drivers/gpu/drm/rockchip/rockchip_lvds.h
@@ -121,4 +121,14 @@
 #define   PX30_LVDS_P2S_EN(val)			FIELD_PREP_WM16(BIT(6), (val))
 #define   PX30_LVDS_VOP_SEL(val)		FIELD_PREP_WM16(BIT(1), (val))
 
+#define RK3568_GRF_VO_CON0			0x0360
+#define   RK3568_LVDS0_SELECT(val)		FIELD_PREP_WM16(GENMASK(5, 4), (val))
+#define   RK3568_LVDS0_MSBSEL(val)		FIELD_PREP_WM16(BIT(3), (val))
+
+#define RK3568_GRF_VO_CON2			0x0368
+#define   RK3568_LVDS0_DCLK_INV_SEL(val)	FIELD_PREP_WM16(BIT(9), (val))
+#define   RK3568_LVDS0_DCLK_DIV2_SEL(val)	FIELD_PREP_WM16(BIT(8), (val))
+#define   RK3568_LVDS0_MODE_EN(val)		FIELD_PREP_WM16(BIT(1), (val))
+#define   RK3568_LVDS0_P2S_EN(val)		FIELD_PREP_WM16(BIT(0), (val))
+
 #endif /* _ROCKCHIP_LVDS_ */
-- 
2.43.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH v2 4/4] arm64: dts: rockchip: rk356x: add LVDS node

[Rok Markovic]

This commit adds the LVDS transmitter node for the RK356x SoC, assigning
its D-PHY, GRF handle and power domain. The block has no register block
and no clock of its own. It is left disabled by default, to be enabled
by specific board files.

Signed-off-by: Rok Markovic <rok@kanardia.eu>
Assisted-by: Claude:claude-opus-4-8
---
Changes in v2:
  - Drop the clock from the LVDS node; the block takes none.

 arch/arm64/boot/dts/rockchip/rk356x-base.dtsi | 23 +++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk356x-base.dtsi b/arch/arm64/boot/dts/rockchip/rk356x-base.dtsi
index 64bdd8b..61a932a 100644
--- a/arch/arm64/boot/dts/rockchip/rk356x-base.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk356x-base.dtsi
@@ -878,6 +878,29 @@
 		};
 	};
 
+	lvds: lvds {
+		compatible = "rockchip,rk3568-lvds";
+		phys = <&dsi_dphy0>;
+		phy-names = "dphy";
+		power-domains = <&power RK3568_PD_VO>;
+		rockchip,grf = <&grf>;
+		rockchip,output = "lvds";
+		status = "disabled";
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			lvds_in: port@0 {
+				reg = <0>;
+			};
+
+			lvds_out: port@1 {
+				reg = <1>;
+			};
+		};
+	};
+
 	qos_gpu: qos@fe128000 {
 		compatible = "rockchip,rk3568-qos", "syscon";
 		reg = <0x0 0xfe128000 0x0 0x20>;
-- 
2.43.0

[PATCH v2 4/4] arm64: dts: rockchip: rk356x: add LVDS node

[Rok Markovic]

This commit adds the LVDS transmitter node for the RK356x SoC, assigning
its D-PHY, GRF handle and power domain. The block has no register block
and no clock of its own. It is left disabled by default, to be enabled
by specific board files.

Signed-off-by: Rok Markovic <rok@kanardia.eu>
Assisted-by: Claude:claude-opus-4-8
---
Changes in v2:
  - Drop the clock from the LVDS node; the block takes none.

 arch/arm64/boot/dts/rockchip/rk356x-base.dtsi | 23 +++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk356x-base.dtsi b/arch/arm64/boot/dts/rockchip/rk356x-base.dtsi
index 64bdd8b..61a932a 100644
--- a/arch/arm64/boot/dts/rockchip/rk356x-base.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk356x-base.dtsi
@@ -878,6 +878,29 @@
 		};
 	};
 
+	lvds: lvds {
+		compatible = "rockchip,rk3568-lvds";
+		phys = <&dsi_dphy0>;
+		phy-names = "dphy";
+		power-domains = <&power RK3568_PD_VO>;
+		rockchip,grf = <&grf>;
+		rockchip,output = "lvds";
+		status = "disabled";
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			lvds_in: port@0 {
+				reg = <0>;
+			};
+
+			lvds_out: port@1 {
+				reg = <1>;
+			};
+		};
+	};
+
 	qos_gpu: qos@fe128000 {
 		compatible = "rockchip,rk3568-qos", "syscon";
 		reg = <0x0 0xfe128000 0x0 0x20>;
-- 
2.43.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH v2 1/4] drm/rockchip: lvds: propagate bus_flags to the CRTC state

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] rockchip_lvds_bind overwrites external bridge function pointers, performs invalid frees on error paths, and leaks objects on unbind.
--

--- Patch [1]: [PATCH v2 1/4] drm/rockchip: lvds: propagate bus_flags to the CRTC state ---
commit 1b0e3d7f00cf4d61b195bc93e8d5dc954705b92b
Author: Rok Markovic <rok@kanardia.eu>

drm/rockchip: lvds: propagate bus_flags to the CRTC state

rockchip_lvds_encoder_atomic_check() sets output_mode and output_type
but never copies the connector's bus_flags into the Rockchip CRTC state.
This prevents the VOP from deriving the pixel clock polarity, causing
panels to be clocked on the wrong edge.

> diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.c b/drivers/gpu/drm/rockchip/rockchip_lvds.c
> index 7a0c4fa29f2f0..150299c12298a 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_lvds.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_lvds.c

[ ... ]

While reviewing this patch, I noticed some pre-existing issues in the same
file.

Looking at rockchip_lvds_bind():

	if (lvds->bridge) {
		lvds->bridge->driver_private = lvds;
		lvds->bridge->ops = DRM_BRIDGE_OP_MODES;
		lvds->bridge->funcs = &rockchip_lvds_bridge_funcs;

[Severity: High]
This is a pre-existing issue, but does overwriting the bridge's function
pointers and private data here destroy the foreign driver state if this is
an external bridge? 

It seems this would break drm_bridge_is_panel() and prevent the bridge from
being enabled or providing modes.


Further down in rockchip_lvds_bind() on the error path:

err_free_bridge:
	drm_panel_bridge_remove(lvds->bridge);

[Severity: High]
This is a pre-existing issue, but could this error path leak the panel bridge?

Since the bridge's funcs were overwritten earlier in this function,
drm_bridge_is_panel() will return false. This would cause
drm_panel_bridge_remove() to early-return without freeing the bridge.


Looking at rockchip_lvds_unbind():

static void rockchip_lvds_unbind(struct device *dev, struct device *master,
				void *data)
{
	struct rockchip_lvds *lvds = dev_get_drvdata(dev);
	const struct drm_encoder_helper_funcs *encoder_funcs;

	encoder_funcs = lvds->soc_data->helper_funcs;
	encoder_funcs->disable(&lvds->encoder.encoder);
	pm_runtime_disable(dev);
}

[Severity: High]
This isn't a bug introduced by this patch, but is the unbind function
missing cleanup for the bridge, encoder, and connector? 

It appears these objects will leak and corrupt the DRM state list on
every unbind.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724080824.115260-1-rok@kanardia.eu?part=1

Re: [PATCH v2 3/4] drm/rockchip: lvds: add RK3568 support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [Medium] Missing `phy_exit()` cleanup for the PHY initialized in `rk3568_lvds_probe()`.

Pre-existing issues:
- [High] The driver illegally overwrites the `funcs`, `ops`, and `driver_private` fields of a `drm_bridge` object obtained via `drm_of_find_panel_or_bridge()`.
- [High] Unbalanced `of_node_put()` causes a use-after-free/double-free of a device node reference.
--

--- Patch [3]: [PATCH v2 3/4] drm/rockchip: lvds: add RK3568 support ---
commit ff72089a77c6360b2f3e3636251c56b8d17b88e7
Author: Rok Markovic <rok@kanardia.eu>

drm/rockchip: lvds: add RK3568 support

The RK3568 LVDS transmitter has no register block of its own. It is
driven entirely through the GRF and re-uses the MIPI DSI0 D-PHY in
PHY_MODE_LVDS.

Link: https://lore.kernel.org/all/20230119184807.171132-1-a1ba.omarov@gmail.com/

> diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.c b/drivers/gpu/drm/rockchip/rockchip_lvds.c
> index 150299c12298a..fcf599ae6c6dd 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_lvds.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_lvds.c

[ ... ]

> @@ -512,6 +614,17 @@ static int px30_lvds_probe(struct platform_device *pdev,
>  	return phy_power_on(lvds->dphy);
>  }
>  
> +static int rk3568_lvds_probe(struct platform_device *pdev,
> +			     struct rockchip_lvds *lvds)
> +{
> +	/* The phy is powered on from the encoder enable path, not here. */
> +	lvds->dphy = devm_phy_get(&pdev->dev, "dphy");
> +	if (IS_ERR(lvds->dphy))
> +		return PTR_ERR(lvds->dphy);
> +
> +	return phy_init(lvds->dphy);
> +}
> +

[Severity: Medium]
Since devm_phy_get() does not automatically call phy_exit(), does this code
leak the PHY's initialization state and resources when the driver is unbound
or fails to probe after initialization?

rk3568_lvds_probe() calls phy_init(lvds->dphy) directly. If phy_init()
succeeds but the probe later fails, or when the device is removed, it
appears phy_exit() is never called to clean up.

[ ... ]

> @@ -601,6 +723,8 @@ static int rockchip_lvds_bind(struct device *dev, struct device *master,
>  	encoder = &lvds->encoder.encoder;
>  	encoder->possible_crtcs = drm_of_find_possible_crtcs(drm_dev,
>  							     dev->of_node);
> +	rockchip_drm_encoder_set_crtc_endpoint_id(&lvds->encoder,
> +						  dev->of_node, 0, 0);
>  
>  	ret = drm_simple_encoder_init(drm_dev, encoder, DRM_MODE_ENCODER_LVDS);
>  	if (ret < 0) {

[Severity: High]
This is a pre-existing issue, but I noticed a potential reference counting
issue in rockchip_lvds_bind():

drivers/gpu/drm/rockchip/rockchip_lvds.c:rockchip_lvds_bind() {
    ...
	if (lvds->panel)
		remote = lvds->panel->dev->of_node;
	else
		remote = lvds->bridge->of_node;
    ...
	pm_runtime_enable(dev);
	of_node_put(remote);
	of_node_put(port);
}

The remote node pointer is borrowed directly without taking a new reference
(e.g. via of_node_get()), but is then unconditionally passed to
of_node_put(). Can this cause a double-free or use-after-free of the
device node when the reference count is decremented incorrectly?

[Severity: High]
This isn't a bug introduced by this patch, but I also noticed that
rockchip_lvds_bind() overwrites the bridge fields:

drivers/gpu/drm/rockchip/rockchip_lvds.c:rockchip_lvds_bind() {
    ...
	if (lvds->bridge) {
		lvds->bridge->driver_private = lvds;
		lvds->bridge->ops = DRM_BRIDGE_OP_MODES;
		lvds->bridge->funcs = &rockchip_lvds_bridge_funcs;
    ...
}

If drm_of_find_panel_or_bridge() returns an external bridge (when
lvds->panel is NULL), does this overwrite the external bridge's own
function pointers and private data?

This could erase the original operations like atomic_enable and
atomic_disable, preventing the external bridge from functioning, and could
cause memory corruption or crashes when the DRM core invokes the
overwritten function pointers.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724080824.115260-1-rok@kanardia.eu?part=3