[PATCH 0/9] Support 10-bit YUV422 and 8/10-bit YUV420 color format on DW HDMI QP

[PATCH 0/9] Support 10-bit YUV422 and 8/10-bit YUV420 color format on DW HDMI QP

[Cristian Ciocaltea]

This series extends the output color format support of the Rockchip DW
HDMI QP controller to cover 10-bit YUV 4:2:2 and 8/10-bit YUV 4:2:0.

Please note this has a runtime dependency on the Rockchip Samsung HDPTX
PHY driver bug fixes posted separately as [1].  While there is no build
dependency, those fixes are required to address clock rate calculation
and synchronization issues that arise when changing the color depth
(bpc) while keeping the modeline constant.

Patches 1, 2 & 9 improve VOP2 robustness on RK3588, helping recover from
exceptions and preventing random display output glitches observed when
switching modes that also change the color format, e.g. from RGB to YUV
4:2:0 and vice versa.

Patch 3 avoids an incorrect DCLK source switch for 10-bit YUV 4:2:2 by
forcing 8 bpc in the bandwidth check.

Patches 4-6 are independent cleanups/improvements and can be applied on
their own.

Patch 7 adds MEDIA_BUS_FMT_UYVY10_1X20 for 10-bit YUV 4:2:2 output,
configuring the PHY with 8 bpc.  YUV 4:2:2 always transmits two 12-bit
components per pixel regardless of color depth, so from a clock-rate
perspective it is equivalent to three 8-bit RGB components.

Patch 8 advertises YUV 4:2:0 output, now that the bus-format and VOP2
support are in place.

Tested on Radxa ROCK 5B (RK3588) and Radxa ROCK 4D (RK3576), up to
4K@60Hz YUV 4:2:0 and 4K@30Hz RGB.

[1] https://lore.kernel.org/all/20260612-hdptx-clk-fixes-v4-0-ce5e1d456cda@collabora.com/

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
Cristian Ciocaltea (9):
      dt-bindings: display: vop2: Add missing reset properties
      drm/rockchip: vop2: Reset AXI and DCLK to improve robustness
      drm/rockchip: vop2: Avoid DCLK source switch for 10-bit YUV422 output
      drm/rockchip: vop2: Consolidate HDMI PHY PLL clock parent switch
      drm/rockchip: vop2: Switch to enum vop_csc_format
      drm/bridge: dw-hdmi-qp: Log resolution and refresh rate in atomic_enable()
      drm/rockchip: dw_hdmi_qp: Support 10-bit YUV422 output format
      drm/rockchip: dw_hdmi_qp: Enable YUV420 output format
      arm64: dts: rockchip: Add RK3588 VOP2 resets

 .../bindings/display/rockchip/rockchip-vop2.yaml   | 42 ++++++++++
 arch/arm64/boot/dts/rockchip/rk3588-base.dtsi      | 12 +++
 drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c       | 10 ++-
 drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c     | 13 +++-
 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c       | 90 +++++++++++++++-------
 drivers/gpu/drm/rockchip/rockchip_drm_vop2.h       |  4 +
 6 files changed, 141 insertions(+), 30 deletions(-)
---
base-commit: 2bfa2e7f19a106966ca97e0aaaad49e17f614cbb
change-id: 20260617-dw-hdmi-qp-yuv-2b0f7bb5ba81

[PATCH 1/9] dt-bindings: display: vop2: Add missing reset properties

[Cristian Ciocaltea]

Document the VOP2 resets corresponding to the AXI, AHB and DCLK_VP0..2
clocks, which are common to all supported SoCs, plus DCLK_VP3 which is
provided only on RK3588.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 .../bindings/display/rockchip/rockchip-vop2.yaml   | 42 ++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml
index 93da1fb9adc4..d3bc5380f910 100644
--- a/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml
+++ b/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml
@@ -82,6 +82,20 @@ properties:
       - {}
       - {}
 
+  resets:
+    minItems: 5
+    maxItems: 6
+
+  reset-names:
+    minItems: 5
+    items:
+      - const: axi
+      - const: ahb
+      - const: dclk_vp0
+      - const: dclk_vp1
+      - const: dclk_vp2
+      - const: dclk_vp3
+
   rockchip,grf:
     $ref: /schemas/types.yaml#/definitions/phandle
     description:
@@ -148,6 +162,12 @@ allOf:
         clock-names:
           maxItems: 5
 
+        resets:
+          maxItems: 5
+
+        reset-names:
+          maxItems: 5
+
         interrupts:
           maxItems: 1
 
@@ -194,6 +214,12 @@ allOf:
             - {}
             - const: pll_hdmiphy0
 
+        resets:
+          maxItems: 5
+
+        reset-names:
+          maxItems: 5
+
         interrupts:
           minItems: 4
 
@@ -246,6 +272,12 @@ allOf:
             - const: pll_hdmiphy0
             - const: pll_hdmiphy1
 
+        resets:
+          minItems: 6
+
+        reset-names:
+          minItems: 6
+
         interrupts:
           maxItems: 1
 
@@ -289,6 +321,16 @@ examples:
                               "dclk_vp0",
                               "dclk_vp1",
                               "dclk_vp2";
+                resets = <&cru SRST_A_VOP>,
+                         <&cru SRST_H_VOP>,
+                         <&cru SRST_VOP0>,
+                         <&cru SRST_VOP1>,
+                         <&cru SRST_VOP2>;
+                reset-names = "axi",
+                              "ahb",
+                              "dclk_vp0",
+                              "dclk_vp1",
+                              "dclk_vp2";
                 power-domains = <&power RK3568_PD_VO>;
                 rockchip,grf = <&grf>;
                 iommus = <&vop_mmu>;

-- 
2.54.0

Re: [PATCH 1/9] dt-bindings: display: vop2: Add missing reset properties

[Diederik de Haas]

Hi Cristian,

Thanks for this series :-) Just 1 nit (at the end) ...

On Wed Jun 17, 2026 at 8:52 PM CEST, Cristian Ciocaltea wrote:
> Document the VOP2 resets corresponding to the AXI, AHB and DCLK_VP0..2
> clocks, which are common to all supported SoCs, plus DCLK_VP3 which is
> provided only on RK3588.
>
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> ---
>  .../bindings/display/rockchip/rockchip-vop2.yaml   | 42 ++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml
> index 93da1fb9adc4..d3bc5380f910 100644
> --- a/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml
> +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml
> @@ -82,6 +82,20 @@ properties:
>        - {}
>        - {}
>  
> +  resets:
> +    minItems: 5
> +    maxItems: 6
> +
> +  reset-names:
> +    minItems: 5
> +    items:
> +      - const: axi
> +      - const: ahb
> +      - const: dclk_vp0
> +      - const: dclk_vp1
> +      - const: dclk_vp2
> +      - const: dclk_vp3
> +
>    rockchip,grf:
>      $ref: /schemas/types.yaml#/definitions/phandle
>      description:
> @@ -148,6 +162,12 @@ allOf:
>          clock-names:
>            maxItems: 5
>  
> +        resets:
> +          maxItems: 5
> +
> +        reset-names:
> +          maxItems: 5
> +
>          interrupts:
>            maxItems: 1
>  
> @@ -194,6 +214,12 @@ allOf:
>              - {}
>              - const: pll_hdmiphy0
>  
> +        resets:
> +          maxItems: 5
> +
> +        reset-names:
> +          maxItems: 5
> +
>          interrupts:
>            minItems: 4
>  
> @@ -246,6 +272,12 @@ allOf:
>              - const: pll_hdmiphy0
>              - const: pll_hdmiphy1
>  
> +        resets:
> +          minItems: 6
> +
> +        reset-names:
> +          minItems: 6
> +
>          interrupts:
>            maxItems: 1
>  
> @@ -289,6 +321,16 @@ examples:
>                                "dclk_vp0",
>                                "dclk_vp1",
>                                "dclk_vp2";
> +                resets = <&cru SRST_A_VOP>,
> +                         <&cru SRST_H_VOP>,
> +                         <&cru SRST_VOP0>,
> +                         <&cru SRST_VOP1>,
> +                         <&cru SRST_VOP2>;
> +                reset-names = "axi",
> +                              "ahb",
> +                              "dclk_vp0",
> +                              "dclk_vp1",
> +                              "dclk_vp2";
>                  power-domains = <&power RK3568_PD_VO>;

Place reset* props below power-domains (like in patch 9) ?
So everyone who copies your example has the correct sorting order.

Cheers,
  Diederik

>                  rockchip,grf = <&grf>;
>                  iommus = <&vop_mmu>;

Re: [PATCH 1/9] dt-bindings: display: vop2: Add missing reset properties

[Cristian Ciocaltea]

Hi Diederik,

On 6/18/26 10:58 AM, Diederik de Haas wrote:
> Hi Cristian,
> 
> Thanks for this series :-) Just 1 nit (at the end) ...
> 
> On Wed Jun 17, 2026 at 8:52 PM CEST, Cristian Ciocaltea wrote:
>> Document the VOP2 resets corresponding to the AXI, AHB and DCLK_VP0..2
>> clocks, which are common to all supported SoCs, plus DCLK_VP3 which is
>> provided only on RK3588.
>>
>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>> ---
>>  .../bindings/display/rockchip/rockchip-vop2.yaml   | 42 ++++++++++++++++++++++
>>  1 file changed, 42 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml
>> index 93da1fb9adc4..d3bc5380f910 100644
>> --- a/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml
>> +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml
[...]

>> @@ -289,6 +321,16 @@ examples:
>>                                "dclk_vp0",
>>                                "dclk_vp1",
>>                                "dclk_vp2";
>> +                resets = <&cru SRST_A_VOP>,
>> +                         <&cru SRST_H_VOP>,
>> +                         <&cru SRST_VOP0>,
>> +                         <&cru SRST_VOP1>,
>> +                         <&cru SRST_VOP2>;
>> +                reset-names = "axi",
>> +                              "ahb",
>> +                              "dclk_vp0",
>> +                              "dclk_vp1",
>> +                              "dclk_vp2";
>>                  power-domains = <&power RK3568_PD_VO>;
> 
> Place reset* props below power-domains (like in patch 9) ?
> So everyone who copies your example has the correct sorting order.

The example doesn't strictly follow that ordering either — see e.g. the iommus
property — so I placed the resets right after the clocks, which keeps the
related properties grouped together.

That said, I don't have a strong preference. 

Heiko, is there a convention you'd like the Rockchip bindings to follow here?
Happy to reorder if so.

Regards,
Cristian

> 
> Cheers,
>   Diederik
> 
>>                  rockchip,grf = <&grf>;
>>                  iommus = <&vop_mmu>;
> 

Re: [PATCH 1/9] dt-bindings: display: vop2: Add missing reset properties

[Krzysztof Kozlowski]

On Wed, Jun 17, 2026 at 09:51:54PM +0300, Cristian Ciocaltea wrote:
> Document the VOP2 resets corresponding to the AXI, AHB and DCLK_VP0..2
> clocks, which are common to all supported SoCs, plus DCLK_VP3 which is
> provided only on RK3588.
> 
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> ---
>  .../bindings/display/rockchip/rockchip-vop2.yaml   | 42 ++++++++++++++++++++++
>  1 file changed, 42 insertions(+)

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof

[PATCH 2/9] drm/rockchip: vop2: Reset AXI and DCLK to improve robustness

[Cristian Ciocaltea]

Assert the AXI reset in the CRTC disable path, and the VP DCLK reset in
the enable path.

These resets are intended to leave the hardware in a clean state for the
next use, helping recover from exceptions such as IOMMU page faults, as
well as to prevent random display output glitches, such as a blank
image, observed when switching modes that also change the color format,
e.g. from RGB to YUV420 and vice versa.

For now this seems to affect only the RK3588, hence the resets are
optional and will be provided in the device tree for this SoC only.

Co-developed-by: Andy Yan <andy.yan@rock-chips.com>
Signed-off-by: Andy Yan <andy.yan@rock-chips.com>
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 35 ++++++++++++++++++++++++++++
 drivers/gpu/drm/rockchip/rockchip_drm_vop2.h |  4 ++++
 2 files changed, 39 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
index 4cce3e336f5b..2833fb49ad81 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
@@ -17,6 +17,7 @@
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/regmap.h>
+#include <linux/reset.h>
 #include <linux/swab.h>
 
 #include <drm/drm.h>
@@ -860,6 +861,26 @@ static int vop2_core_clks_prepare_enable(struct vop2 *vop2)
 	return ret;
 }
 
+static void vop2_clk_reset(struct vop2 *vop2, struct reset_control *rstc)
+{
+	int ret;
+
+	if (!rstc)
+		return;
+
+	ret = reset_control_assert(rstc);
+	if (ret < 0) {
+		drm_warn(vop2->drm, "failed to assert reset: %d\n", ret);
+		return;
+	}
+
+	udelay(10);
+
+	ret = reset_control_deassert(rstc);
+	if (ret < 0)
+		drm_err(vop2->drm, "failed to deassert reset: %d\n", ret);
+}
+
 static void rk3588_vop2_power_domain_enable_all(struct vop2 *vop2)
 {
 	u32 pd;
@@ -938,6 +959,8 @@ static void vop2_disable(struct vop2 *vop2)
 {
 	rockchip_drm_dma_detach_device(vop2->drm, vop2->dev);
 
+	vop2_clk_reset(vop2, vop2->axi_rst);
+
 	pm_runtime_put_sync(vop2->dev);
 
 	regcache_drop_region(vop2->map, 0, vop2_regmap_config.max_register);
@@ -1948,6 +1971,8 @@ static void vop2_crtc_atomic_enable(struct drm_crtc *crtc,
 
 	vop2_crtc_atomic_try_set_gamma(vop2, vp, crtc, crtc_state);
 
+	vop2_clk_reset(vop2, vp->dclk_rst);
+
 	drm_crtc_vblank_on(crtc);
 
 	vop2_unlock(vop2);
@@ -2531,6 +2556,11 @@ static int vop2_create_crtcs(struct vop2 *vop2)
 			return dev_err_probe(drm->dev, PTR_ERR(vp->dclk),
 					     "failed to get %s\n", dclk_name);
 
+		vp->dclk_rst = devm_reset_control_get_optional(vop2->dev, dclk_name);
+		if (IS_ERR(vp->dclk_rst))
+			return dev_err_probe(drm->dev, PTR_ERR(vp->dclk_rst),
+					     "failed to get %s reset\n", dclk_name);
+
 		np = of_graph_get_remote_node(dev->of_node, i, -1);
 		if (!np) {
 			drm_dbg(vop2->drm, "%s: No remote for vp%d\n", __func__, i);
@@ -2890,6 +2920,11 @@ static int vop2_bind(struct device *dev, struct device *master, void *data)
 		return dev_err_probe(drm->dev, PTR_ERR(vop2->pll_hdmiphy1),
 				     "failed to get pll_hdmiphy1\n");
 
+	vop2->axi_rst = devm_reset_control_get_optional(vop2->dev, "axi");
+	if (IS_ERR(vop2->axi_rst))
+		return dev_err_probe(drm->dev, PTR_ERR(vop2->axi_rst),
+				     "failed to get axi reset\n");
+
 	vop2->irq = platform_get_irq(pdev, 0);
 	if (vop2->irq < 0)
 		return dev_err_probe(drm->dev, vop2->irq, "cannot find irq for vop2\n");
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
index ffcb39c130aa..14b437d2d088 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.h
@@ -165,6 +165,8 @@ enum vop2_win_regs {
 	VOP2_WIN_MAX_REG,
 };
 
+struct reset_control;
+
 struct vop2_regs_dump {
 	const char *name;
 	u32 base;
@@ -238,6 +240,7 @@ struct vop2_video_port {
 	struct vop2 *vop2;
 	struct clk *dclk;
 	struct clk *dclk_src;
+	struct reset_control *dclk_rst;
 	unsigned int id;
 	const struct vop2_video_port_data *data;
 
@@ -329,6 +332,7 @@ struct vop2 {
 	struct clk *pclk;
 	struct clk *pll_hdmiphy0;
 	struct clk *pll_hdmiphy1;
+	struct reset_control *axi_rst;
 
 	/* optional internal rgb encoder */
 	struct rockchip_rgb *rgb;

-- 
2.54.0

Re: [PATCH 2/9] drm/rockchip: vop2: Reset AXI and DCLK to improve robustness

[Philipp Zabel]

On Mi, 2026-06-17 at 21:51 +0300, Cristian Ciocaltea wrote:
> Assert the AXI reset in the CRTC disable path, and the VP DCLK reset in
> the enable path.
> 
> These resets are intended to leave the hardware in a clean state for the
> next use, helping recover from exceptions such as IOMMU page faults, as
> well as to prevent random display output glitches, such as a blank
> image, observed when switching modes that also change the color format,
> e.g. from RGB to YUV420 and vice versa.
> 
> For now this seems to affect only the RK3588, hence the resets are
> optional and will be provided in the device tree for this SoC only.
> 
> Co-developed-by: Andy Yan <andy.yan@rock-chips.com>
> Signed-off-by: Andy Yan <andy.yan@rock-chips.com>
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> ---
>  drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 35 ++++++++++++++++++++++++++++
>  drivers/gpu/drm/rockchip/rockchip_drm_vop2.h |  4 ++++
>  2 files changed, 39 insertions(+)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> index 4cce3e336f5b..2833fb49ad81 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> @@ -17,6 +17,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/regmap.h>
> +#include <linux/reset.h>
>  #include <linux/swab.h>
>  
>  #include <drm/drm.h>
> @@ -860,6 +861,26 @@ static int vop2_core_clks_prepare_enable(struct vop2 *vop2)
>  	return ret;
>  }
>  
> +static void vop2_clk_reset(struct vop2 *vop2, struct reset_control *rstc)

The _clk part of the function name is misleading ...

[...]
> @@ -938,6 +959,8 @@ static void vop2_disable(struct vop2 *vop2)
>  {
>  	rockchip_drm_dma_detach_device(vop2->drm, vop2->dev);
>  
> +	vop2_clk_reset(vop2, vop2->axi_rst);

... because this function is also called with the AXI reset control.

> +
>  	pm_runtime_put_sync(vop2->dev);
>  
>  	regcache_drop_region(vop2->map, 0, vop2_regmap_config.max_register);
> @@ -1948,6 +1971,8 @@ static void vop2_crtc_atomic_enable(struct drm_crtc *crtc,
>  
>  	vop2_crtc_atomic_try_set_gamma(vop2, vp, crtc, crtc_state);
>  
> +	vop2_clk_reset(vop2, vp->dclk_rst);
> +
>  	drm_crtc_vblank_on(crtc);
>  
>  	vop2_unlock(vop2);
> @@ -2531,6 +2556,11 @@ static int vop2_create_crtcs(struct vop2 *vop2)
>  			return dev_err_probe(drm->dev, PTR_ERR(vp->dclk),
>  					     "failed to get %s\n", dclk_name);
>  
> +		vp->dclk_rst = devm_reset_control_get_optional(vop2->dev, dclk_name);

Please use devm_reset_control_get_optional_exclusive() directly.

> +		if (IS_ERR(vp->dclk_rst))
> +			return dev_err_probe(drm->dev, PTR_ERR(vp->dclk_rst),
> +					     "failed to get %s reset\n", dclk_name);
> +
>  		np = of_graph_get_remote_node(dev->of_node, i, -1);
>  		if (!np) {
>  			drm_dbg(vop2->drm, "%s: No remote for vp%d\n", __func__, i);
> @@ -2890,6 +2920,11 @@ static int vop2_bind(struct device *dev, struct device *master, void *data)
>  		return dev_err_probe(drm->dev, PTR_ERR(vop2->pll_hdmiphy1),
>  				     "failed to get pll_hdmiphy1\n");
>  
> +	vop2->axi_rst = devm_reset_control_get_optional(vop2->dev, "axi");

Same as above, devm_reset_control_get_optional_exclusive().

regards
Philipp

Re: [PATCH 2/9] drm/rockchip: vop2: Reset AXI and DCLK to improve robustness

[Cristian Ciocaltea]

On 6/18/26 12:39 PM, Philipp Zabel wrote:
> On Mi, 2026-06-17 at 21:51 +0300, Cristian Ciocaltea wrote:
>> Assert the AXI reset in the CRTC disable path, and the VP DCLK reset in
>> the enable path.
>>
>> These resets are intended to leave the hardware in a clean state for the
>> next use, helping recover from exceptions such as IOMMU page faults, as
>> well as to prevent random display output glitches, such as a blank
>> image, observed when switching modes that also change the color format,
>> e.g. from RGB to YUV420 and vice versa.
>>
>> For now this seems to affect only the RK3588, hence the resets are
>> optional and will be provided in the device tree for this SoC only.
>>
>> Co-developed-by: Andy Yan <andy.yan@rock-chips.com>
>> Signed-off-by: Andy Yan <andy.yan@rock-chips.com>
>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>> ---
>>  drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 35 ++++++++++++++++++++++++++++
>>  drivers/gpu/drm/rockchip/rockchip_drm_vop2.h |  4 ++++
>>  2 files changed, 39 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>> index 4cce3e336f5b..2833fb49ad81 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>> @@ -17,6 +17,7 @@
>>  #include <linux/platform_device.h>
>>  #include <linux/pm_runtime.h>
>>  #include <linux/regmap.h>
>> +#include <linux/reset.h>
>>  #include <linux/swab.h>
>>  
>>  #include <drm/drm.h>
>> @@ -860,6 +861,26 @@ static int vop2_core_clks_prepare_enable(struct vop2 *vop2)
>>  	return ret;
>>  }
>>  
>> +static void vop2_clk_reset(struct vop2 *vop2, struct reset_control *rstc)
> 
> The _clk part of the function name is misleading ...

Ack.  

We need to make this clearly distinct from another similarly named function,
vop2_crtc_reset(), hence I'd propose:

- vop2_reset_assert_deassert()
- vop2_reset_cycle()
- vop2_do_reset()

Any preference / alternative suggestions?

> 
> [...]
>> @@ -938,6 +959,8 @@ static void vop2_disable(struct vop2 *vop2)
>>  {
>>  	rockchip_drm_dma_detach_device(vop2->drm, vop2->dev);
>>  
>> +	vop2_clk_reset(vop2, vop2->axi_rst);
> 
> ... because this function is also called with the AXI reset control.
> 
>> +
>>  	pm_runtime_put_sync(vop2->dev);
>>  
>>  	regcache_drop_region(vop2->map, 0, vop2_regmap_config.max_register);
>> @@ -1948,6 +1971,8 @@ static void vop2_crtc_atomic_enable(struct drm_crtc *crtc,
>>  
>>  	vop2_crtc_atomic_try_set_gamma(vop2, vp, crtc, crtc_state);
>>  
>> +	vop2_clk_reset(vop2, vp->dclk_rst);
>> +
>>  	drm_crtc_vblank_on(crtc);
>>  
>>  	vop2_unlock(vop2);
>> @@ -2531,6 +2556,11 @@ static int vop2_create_crtcs(struct vop2 *vop2)
>>  			return dev_err_probe(drm->dev, PTR_ERR(vp->dclk),
>>  					     "failed to get %s\n", dclk_name);
>>  
>> +		vp->dclk_rst = devm_reset_control_get_optional(vop2->dev, dclk_name);
> 
> Please use devm_reset_control_get_optional_exclusive() directly.

Thanks for pointing this out, I missed the comment mentioning the explicit API
transition.

>> +		if (IS_ERR(vp->dclk_rst))
>> +			return dev_err_probe(drm->dev, PTR_ERR(vp->dclk_rst),
>> +					     "failed to get %s reset\n", dclk_name);
>> +
>>  		np = of_graph_get_remote_node(dev->of_node, i, -1);
>>  		if (!np) {
>>  			drm_dbg(vop2->drm, "%s: No remote for vp%d\n", __func__, i);
>> @@ -2890,6 +2920,11 @@ static int vop2_bind(struct device *dev, struct device *master, void *data)
>>  		return dev_err_probe(drm->dev, PTR_ERR(vop2->pll_hdmiphy1),
>>  				     "failed to get pll_hdmiphy1\n");
>>  
>> +	vop2->axi_rst = devm_reset_control_get_optional(vop2->dev, "axi");
> 
> Same as above, devm_reset_control_get_optional_exclusive().

Ack.

Thanks for reviewing,
Cristian

Re: [PATCH 2/9] drm/rockchip: vop2: Reset AXI and DCLK to improve robustness

[Philipp Zabel]

On Do, 2026-06-18 at 14:46 +0300, Cristian Ciocaltea wrote:
> On 6/18/26 12:39 PM, Philipp Zabel wrote:
> > On Mi, 2026-06-17 at 21:51 +0300, Cristian Ciocaltea wrote:
> > > Assert the AXI reset in the CRTC disable path, and the VP DCLK reset in
> > > the enable path.
> > > 
> > > These resets are intended to leave the hardware in a clean state for the
> > > next use, helping recover from exceptions such as IOMMU page faults, as
> > > well as to prevent random display output glitches, such as a blank
> > > image, observed when switching modes that also change the color format,
> > > e.g. from RGB to YUV420 and vice versa.
> > > 
> > > For now this seems to affect only the RK3588, hence the resets are
> > > optional and will be provided in the device tree for this SoC only.
> > > 
> > > Co-developed-by: Andy Yan <andy.yan@rock-chips.com>
> > > Signed-off-by: Andy Yan <andy.yan@rock-chips.com>
> > > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> > > ---
> > >  drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 35 ++++++++++++++++++++++++++++
> > >  drivers/gpu/drm/rockchip/rockchip_drm_vop2.h |  4 ++++
> > >  2 files changed, 39 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > > index 4cce3e336f5b..2833fb49ad81 100644
> > > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > > @@ -17,6 +17,7 @@
> > >  #include <linux/platform_device.h>
> > >  #include <linux/pm_runtime.h>
> > >  #include <linux/regmap.h>
> > > +#include <linux/reset.h>
> > >  #include <linux/swab.h>
> > >  
> > >  #include <drm/drm.h>
> > > @@ -860,6 +861,26 @@ static int vop2_core_clks_prepare_enable(struct vop2 *vop2)
> > >  	return ret;
> > >  }
> > >  
> > > +static void vop2_clk_reset(struct vop2 *vop2, struct reset_control *rstc)
> > 
> > The _clk part of the function name is misleading ...
> 
> Ack.  
> 
> We need to make this clearly distinct from another similarly named function,
> vop2_crtc_reset(), hence I'd propose:
> 
> - vop2_reset_assert_deassert()
> - vop2_reset_cycle()
> - vop2_do_reset()
> 
> Any preference / alternative suggestions?

Or vop2_reset_control_reset(). The three above are equally fine.

regards
Philipp

Re: [PATCH 2/9] drm/rockchip: vop2: Reset AXI and DCLK to improve robustness

[sashiko-bot]

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

New issues:
- [High] AXI reset is performed after detaching the IOMMU domain, risking memory corruption.

Pre-existing issues:
- [High] Unpowered hardware access and unbalanced PM runtime usage due to ignored error in vop2_enable().
- [High] Clock enable count underflow and resource tracking mismatch on early return in vop2_crtc_atomic_enable().
--

commit 13864800def8231541cc3847f090a3ab6ab365c2
Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
Subject: drm/rockchip: vop2: Reset AXI and DCLK to improve robustness

Assert the AXI reset in the CRTC disable path, and the VP DCLK reset in
the enable path. These resets are intended to leave the hardware in a clean
state for the next use, helping recover from exceptions such as IOMMU page
faults.

> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> index 4cce3e336f5b3..2833fb49ad81a 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c

[ ... ]

> @@ -938,6 +959,8 @@ static void vop2_disable(struct vop2 *vop2)
>  {
>  	rockchip_drm_dma_detach_device(vop2->drm, vop2->dev);
>  
> +	vop2_clk_reset(vop2, vop2->axi_rst);
> +

[Severity: High]
Does asserting the AXI reset here risk memory corruption?

If the AXI master is recovering from an exception and still issuing DMA
requests, detaching the IOMMU domain first leaves a window where the master
could access physical memory without translation. 

Could this lead to memory corruption or spurious page faults? Should the AXI
master be explicitly reset before the IOMMU is detached?

>  	pm_runtime_put_sync(vop2->dev);
>  
>  	regcache_drop_region(vop2->map, 0, vop2_regmap_config.max_register);

[ ... ]

> @@ -1948,6 +1971,8 @@ static void vop2_crtc_atomic_enable(struct drm_crtc *crtc,
>  
>  	vop2_crtc_atomic_try_set_gamma(vop2, vp, crtc, crtc_state);
>  
> +	vop2_clk_reset(vop2, vp->dclk_rst);
> +

[Severity: High]
This isn't a bug introduced by this patch, but is it possible for unpowered
hardware access to cause a crash if vop2_enable() fails?

In vop2_enable(), if pm_runtime_resume_and_get() or clock preparation fails,
it logs an error and returns void. However, vop2_crtc_atomic_enable()
unconditionally increments vop2->enable_count and continues configuring
hardware:

	if (!vop2->enable_count)
		vop2_enable(vop2);

	vop2->enable_count++;

Since the power domain and core clocks would remain off on failure, could the
subsequent writes to VOP2 hardware registers trigger a synchronous external
abort and crash the system?

[Severity: High]
This is a pre-existing issue, but can an early return in
vop2_crtc_atomic_enable() cause severe state mismatches?

If clk_prepare_enable(vp->dclk) fails earlier in this function:

	ret = clk_prepare_enable(vp->dclk);
	if (ret < 0) {
		drm_err(vop2->drm, "failed to enable dclk for video port%d - %d\n",
			vp->id, ret);
		vop2_unlock(vop2);
		return;
	}

The function returns early without updating state properly. During disable,
vop2_crtc_atomic_disable() blindly decrements the tracking state:

	clk_disable_unprepare(vp->dclk);

	vop2->enable_count--;

	if (!vop2->enable_count)
		vop2_disable(vop2);

If another CRTC is active, couldn't this underflow or prematurely drop the
count to 0, unintentionally shutting off core clocks and PM runtime for the
actively running CRTC?

>  	drm_crtc_vblank_on(crtc);
>  
>  	vop2_unlock(vop2);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-dw-hdmi-qp-yuv-v1-0-a665cfd06d7d@collabora.com?part=2

[PATCH 3/9] drm/rockchip: vop2: Avoid DCLK source switch for 10-bit YUV422 output

[Cristian Ciocaltea]

Currently the color depth is always factored into the DCLK source
decision for HDMI output, which can break certain modes when operating
with depths greater than 8 bpc.

When the required transmission rate exceeds the 600 MHz limit of the
HDMI PHY PLL, e.g. for 4K@60Hz 10-bit RGB output, VOP2 will normally
fall back to using the less accurate system CRU as a DCLK source,
assuming HDMI 2.1 FRL is supported by the pipeline, otherwise the mode
will be rejected.  For YUV420 output format this never happens, as it
uses half of the RGB bandwidth, hence the rate remains within the PHY
PLL limits.

On the other hand, YUV422 always transmits two 12-bit components per
clock cycle, regardless of the color depth, which from a clock-rate
perspective is equivalent to three 8-bit RGB components.  For example,
4K@60Hz 10-bit YUV422 requires the same bandwidth as 4K@60Hz 8-bit RGB,
typically 594 MHz.  However, VOP2 wrongly assumes it needs 742.5 MHz
(594 * 10 / 8) and ends up switching the DCLK source.

As a consequence, the modes requiring uncommon pixel clocks, such as
those corresponding to fractional refresh rates, will fail.  An example
is 3840x2160@59.94Hz, which would likely rely on the 593.407 MHz clock
rate unsupported by the system CRU.

Note this only affects YUV422 with color depths greater than 8 bpc; for
8-bit YUV422 the 8/bpc factor is unity and the bandwidth check is
already correct.

Prevent the incorrect switches of DCLK source to system CRU for YUV422
output format by forcing 8 bpc when checking the bandwidth.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
index 2833fb49ad81..17d21e08ad97 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
@@ -1922,8 +1922,17 @@ static void vop2_crtc_atomic_enable(struct drm_crtc *crtc,
 	 * to 4K@60Hz, if available, otherwise keep using the system CRU.
 	 */
 	if (vop2->pll_hdmiphy0 || vop2->pll_hdmiphy1) {
-		unsigned long max_dclk = DIV_ROUND_CLOSEST_ULL(VOP2_MAX_DCLK_RATE * 8,
-							       vcstate->output_bpc);
+		/*
+		 * YUV422 always transmits two 12-bit components per clock
+		 * cycle, regardless of the color depth, which from a rate
+		 * perspective is equivalent to three 8-bit RGB components.
+		 * Force 8 bpc here so the bandwidth check reflects the actual
+		 * TMDS rate and avoids an unnecessary DCLK source switch.
+		 */
+		unsigned int bpc = vcstate->output_mode == ROCKCHIP_OUT_MODE_YUV422 ?
+					8 : vcstate->output_bpc;
+		unsigned long max_dclk = DIV_ROUND_CLOSEST_ULL(VOP2_MAX_DCLK_RATE * 8, bpc);
+
 		if (clock <= max_dclk) {
 			drm_for_each_encoder_mask(encoder, crtc->dev, crtc_state->encoder_mask) {
 				struct rockchip_encoder *rkencoder = to_rockchip_encoder(encoder);

-- 
2.54.0

Re: [PATCH 3/9] drm/rockchip: vop2: Avoid DCLK source switch for 10-bit YUV422 output

[sashiko-bot]

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

Pre-existing issues:
- [High] Division by zero in max_dclk calculation due to uninitialized output_bpc.
- [High] Integer overflow in max_dclk calculation on 32-bit architectures.
- [High] Atomic context violation due to calling sleeping functions.
--

commit cdb8cbbc3fd201db3157d6704b3856b8565e10e2
Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>

drm/rockchip: vop2: Avoid DCLK source switch for 10-bit YUV422 output
    
The commit modifies the DCLK source decision logic for HDMI output in VOP2.
It prevents unnecessary switching of the DCLK source to the system CRU for
10-bit YUV422 output by forcing the bandwidth check to 8 bpc.

> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> index 2833fb49ad81a..17d21e08ad977 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> @@ -1922,8 +1922,17 @@ static void vop2_crtc_atomic_enable(struct drm_crtc *crtc,

[Severity: High]
This is a pre-existing issue, but does this function violate atomic context
rules?

According to the DRM subsystem guidelines, CRTC atomic enable callbacks execute
in atomic context where sleeping is forbidden. However, vop2_crtc_atomic_enable()
acquires a mutex via vop2_lock() and later calls clk_prepare_enable(), both of
which can sleep.

Can this cause scheduling while atomic warnings or deadlocks during atomic
commits?

>  	 * to 4K@60Hz, if available, otherwise keep using the system CRU.
>  	 */
>  	if (vop2->pll_hdmiphy0 || vop2->pll_hdmiphy1) {
> -		unsigned long max_dclk = DIV_ROUND_CLOSEST_ULL(VOP2_MAX_DCLK_RATE * 8,
> -							       vcstate->output_bpc);
> +		/*
> +		 * YUV422 always transmits two 12-bit components per clock
> +		 * cycle, regardless of the color depth, which from a rate
> +		 * perspective is equivalent to three 8-bit RGB components.
> +		 * Force 8 bpc here so the bandwidth check reflects the actual
> +		 * TMDS rate and avoids an unnecessary DCLK source switch.
> +		 */
> +		unsigned int bpc = vcstate->output_mode == ROCKCHIP_OUT_MODE_YUV422 ?
> +					8 : vcstate->output_bpc;
> +		unsigned long max_dclk = DIV_ROUND_CLOSEST_ULL(VOP2_MAX_DCLK_RATE * 8, bpc);

[Severity: High]
This isn't a bug introduced by this patch, but can bpc be 0 here and cause a
division by zero?

If the VOP2 driver is used with a non-HDMI output like DSI or LVDS on a board
that also has an HDMI PHY, vop2->pll_hdmiphy0 or vop2->pll_hdmiphy1 will be
non-NULL. Since DSI/LVDS encoders do not set vcstate->output_bpc, it defaults
to 0.

This would lead to DIV_ROUND_CLOSEST_ULL(..., 0) before the loop checks if
the endpoint is HDMI.

[Severity: High]
This is also a pre-existing issue, but does VOP2_MAX_DCLK_RATE * 8 overflow on
32-bit architectures?

VOP2_MAX_DCLK_RATE is defined as 600000000UL. On architectures where
unsigned long is 32 bits, 600000000UL * 8 equals 4,800,000,000, which exceeds
ULONG_MAX.

This truncates to 505,032,704 before being promoted to unsigned long long inside
DIV_ROUND_CLOSEST_ULL, improperly forcing higher resolutions to fall back to the
system CRU.

Should this be cast to a 64-bit type first, such as VOP2_MAX_DCLK_RATE * 8ULL?

>  		if (clock <= max_dclk) {
>  			drm_for_each_encoder_mask(encoder, crtc->dev, crtc_state->encoder_mask) {
>  				struct rockchip_encoder *rkencoder = to_rockchip_encoder(encoder);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-dw-hdmi-qp-yuv-v1-0-a665cfd06d7d@collabora.com?part=3

[PATCH 4/9] drm/rockchip: vop2: Consolidate HDMI PHY PLL clock parent switch

[Cristian Ciocaltea]

The DCLK parent switch logic for HDMI0 and HDMI1 PHY PLLs was
duplicated, with each endpoint repeating the same clk_get_parent(),
clk_set_parent() and error handling calls.

Refactor this by first selecting the appropriate PHY PLL clock handle
based on the active HDMI endpoint, then performing the parent switch in
a single shared code path.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 36 +++++++++++-----------------
 1 file changed, 14 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
index 17d21e08ad97..df475173dc8e 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
@@ -1932,42 +1932,34 @@ static void vop2_crtc_atomic_enable(struct drm_crtc *crtc,
 		unsigned int bpc = vcstate->output_mode == ROCKCHIP_OUT_MODE_YUV422 ?
 					8 : vcstate->output_bpc;
 		unsigned long max_dclk = DIV_ROUND_CLOSEST_ULL(VOP2_MAX_DCLK_RATE * 8, bpc);
+		struct clk *pll_hdmiphy = NULL;
 
 		if (clock <= max_dclk) {
 			drm_for_each_encoder_mask(encoder, crtc->dev, crtc_state->encoder_mask) {
 				struct rockchip_encoder *rkencoder = to_rockchip_encoder(encoder);
 
 				if (rkencoder->crtc_endpoint_id == ROCKCHIP_VOP2_EP_HDMI0) {
-					if (!vop2->pll_hdmiphy0)
-						break;
-
-					if (!vp->dclk_src)
-						vp->dclk_src = clk_get_parent(vp->dclk);
-
-					ret = clk_set_parent(vp->dclk, vop2->pll_hdmiphy0);
-					if (ret < 0)
-						drm_warn(vop2->drm,
-							 "Could not switch to HDMI0 PHY PLL: %d\n",
-							 ret);
+					pll_hdmiphy = vop2->pll_hdmiphy0;
 					break;
 				}
 
 				if (rkencoder->crtc_endpoint_id == ROCKCHIP_VOP2_EP_HDMI1) {
-					if (!vop2->pll_hdmiphy1)
-						break;
-
-					if (!vp->dclk_src)
-						vp->dclk_src = clk_get_parent(vp->dclk);
-
-					ret = clk_set_parent(vp->dclk, vop2->pll_hdmiphy1);
-					if (ret < 0)
-						drm_warn(vop2->drm,
-							 "Could not switch to HDMI1 PHY PLL: %d\n",
-							 ret);
+					pll_hdmiphy = vop2->pll_hdmiphy1;
 					break;
 				}
 			}
 		}
+
+		if (pll_hdmiphy) {
+			if (!vp->dclk_src)
+				vp->dclk_src = clk_get_parent(vp->dclk);
+
+			ret = clk_set_parent(vp->dclk, pll_hdmiphy);
+			if (ret < 0)
+				drm_warn(vop2->drm,
+					 "Failed to switch DCLK to HDMI PHY PLL: %d\n",
+					 ret);
+		}
 	}
 
 	clk_set_rate(vp->dclk, clock);

-- 
2.54.0

Re: [PATCH 4/9] drm/rockchip: vop2: Consolidate HDMI PHY PLL clock parent switch

[sashiko-bot]

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

Pre-existing issues:
- [High] Calling sleepable functions (`mutex_lock` and CCF APIs) inside an atomic context.
--

commit daeb5d187e2f25bcf364f65898a9cf55a75558aa
Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>

drm/rockchip: vop2: Consolidate HDMI PHY PLL clock parent switch

This commit refactors the DCLK parent switch logic for HDMI0 and HDMI1
PHY PLLs. It consolidates duplicated clk_get_parent() and clk_set_parent()
calls into a single shared code path based on the active HDMI endpoint.

> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
[ ... ]
> @@ -1932,42 +1932,34 @@ static void vop2_crtc_atomic_enable(struct drm_crtc *crtc,
[ ... ]
> +		if (pll_hdmiphy) {
> +			if (!vp->dclk_src)
> +				vp->dclk_src = clk_get_parent(vp->dclk);
> +
> +			ret = clk_set_parent(vp->dclk, pll_hdmiphy);

[Severity: High]
This is a pre-existing issue, but does calling clk_set_parent() here
result in a sleep inside an atomic context?

The vop2_crtc_atomic_enable() function executes as a CRTC atomic enable
callback, which runs in an atomic context where sleeping is forbidden.

Both clk_set_parent() and clk_prepare_enable() (called earlier in this
function) acquire sleepable mutexes, and vop2_lock() internally calls
mutex_lock(). Can this lead to kernel warnings or potential deadlocks
during the atomic commit phase?

> +			if (ret < 0)
> +				drm_warn(vop2->drm,
> +					 "Failed to switch DCLK to HDMI PHY PLL: %d\n",
> +					 ret);
> +		}
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-dw-hdmi-qp-yuv-v1-0-a665cfd06d7d@collabora.com?part=4

[PATCH 5/9] drm/rockchip: vop2: Switch to enum vop_csc_format

[Cristian Ciocaltea]

Improve code readability in vop2_setup_csc_mode() by using enum
vop_csc_format for the csc_mode variable, as well as for the return type
of the vop2_convert_csc_mode() helper, which already returns CSC_*
enumerators.

While at it, replace the nonsensical 'csc_mode = false' assignment in
the no-conversion branch with the equivalent CSC_BT601L, which carries
the same value (0) but is type-correct.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
index df475173dc8e..e0d6e42fedb1 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
@@ -648,7 +648,7 @@ static void vop2_setup_scale(struct vop2 *vop2, const struct vop2_win *win,
 	}
 }
 
-static int vop2_convert_csc_mode(int csc_mode)
+static enum vop_csc_format vop2_convert_csc_mode(int csc_mode)
 {
 	switch (csc_mode) {
 	case V4L2_COLORSPACE_SMPTE170M:
@@ -711,7 +711,7 @@ static void vop2_setup_csc_mode(struct vop2_video_port *vp,
 	int input_csc = V4L2_COLORSPACE_DEFAULT;
 	int output_csc = vcstate->color_space;
 	bool r2y_en, y2r_en;
-	int csc_mode;
+	enum vop_csc_format csc_mode;
 
 	if (is_input_yuv && !is_output_yuv) {
 		y2r_en = true;
@@ -724,7 +724,7 @@ static void vop2_setup_csc_mode(struct vop2_video_port *vp,
 	} else {
 		y2r_en = false;
 		r2y_en = false;
-		csc_mode = false;
+		csc_mode = CSC_BT601L;
 	}
 
 	vop2_win_write(win, VOP2_WIN_Y2R_EN, y2r_en);

-- 
2.54.0

[PATCH 6/9] drm/bridge: dw-hdmi-qp: Log resolution and refresh rate in atomic_enable()

[Cristian Ciocaltea]

The debug entry in the HDMI branch of dw_hdmi_qp_bridge_atomic_enable()
previously printed the literal string 'HDMI' as the mode field, giving
no information about the actual display timing being configured.

Extend it to include the active resolution and refresh rate by
retrieving the CRTC mode from the incoming atomic state:

  dw_hdmi_qp_bridge_atomic_enable mode=1920x1080@50Hz fmt=RGB rate=185625000 bpc=10

This makes the log line self-contained and directly useful when
debugging mode-setting issues, format negotiation, or TMDS rate
mismatches without having to cross-reference a separate mode dump.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
index 13fddd5ebc82..d73307ac4232 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
@@ -752,6 +752,8 @@ static void dw_hdmi_qp_bridge_atomic_enable(struct drm_bridge *bridge,
 {
 	struct dw_hdmi_qp *hdmi = bridge->driver_private;
 	struct drm_connector_state *conn_state;
+	const struct drm_display_mode *mode;
+	struct drm_crtc_state *crtc_state;
 	struct drm_connector *connector;
 	unsigned int op_mode;
 
@@ -764,9 +766,15 @@ static void dw_hdmi_qp_bridge_atomic_enable(struct drm_bridge *bridge,
 		return;
 
 	if (connector->display_info.is_hdmi) {
-		dev_dbg(hdmi->dev, "%s mode=HDMI %s rate=%llu bpc=%u\n", __func__,
+		crtc_state = drm_atomic_get_new_crtc_state(state, conn_state->crtc);
+		mode = &crtc_state->mode;
+		dev_dbg(hdmi->dev,
+			"%s mode=HDMI %ux%u@%uHz fmt=%s rate=%llu bpc=%u\n",
+			__func__, mode->hdisplay, mode->vdisplay,
+			drm_mode_vrefresh(mode),
 			drm_hdmi_connector_get_output_format_name(conn_state->hdmi.output_format),
 			conn_state->hdmi.tmds_char_rate, conn_state->hdmi.output_bpc);
+
 		op_mode = 0;
 		hdmi->tmds_char_rate = conn_state->hdmi.tmds_char_rate;
 	} else {

-- 
2.54.0

[PATCH 7/9] drm/rockchip: dw_hdmi_qp: Support 10-bit YUV422 output format

[Cristian Ciocaltea]

Rockchip DW HDMI QP encoder supports YUV 4:2:2 output through
ROCKCHIP_OUT_MODE_YUV422, but was limited to 8-bit depth via
MEDIA_BUS_FMT_UYVY8_1X16.  Add support for its 10-bit counterpart
MEDIA_BUS_FMT_UYVY10_1X20, which carries two 10-bit components per clock
cycle on a 20-bit wide bus.

YUV 4:2:2 always transmits two 12-bit components per pixel, regardless
of the color depth.  From a clock-rate perspective this is equivalent to
three 8-bit RGB components, so configure the HDMI PHY with 8 bpc when
YUV 4:2:2 is in use to keep its output clock aligned with the TMDS
character rate.  Otherwise the PHY PLL output would be scaled by bpc/8
for higher color depths, producing a clock rate that confuses downstream
consumers such as the VOP2 display controller.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
index 815f9ea7bcbe..3a1c027aa90b 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
@@ -216,6 +216,7 @@ dw_hdmi_qp_rockchip_encoder_atomic_check(struct drm_encoder *encoder,
 		s->output_mode = ROCKCHIP_OUT_MODE_AAAA;
 		break;
 	case MEDIA_BUS_FMT_UYVY8_1X16:
+	case MEDIA_BUS_FMT_UYVY10_1X20:
 		s->output_mode = ROCKCHIP_OUT_MODE_YUV422;
 		break;
 	case MEDIA_BUS_FMT_UYYVYY8_0_5X24:
@@ -227,7 +228,14 @@ dw_hdmi_qp_rockchip_encoder_atomic_check(struct drm_encoder *encoder,
 	}
 
 	phy_cfg.hdmi.tmds_char_rate = conn_state->hdmi.tmds_char_rate;
-	phy_cfg.hdmi.bpc = conn_state->hdmi.output_bpc;
+	/*
+	 * YUV422 always transmits two 12-bit components per clock cycle,
+	 * regardless of the color depth, which from a rate perspective is
+	 * equivalent to three 8-bit RGB components.  Force 8 bpc here to
+	 * keep the PHY PLL output aligned with the TMDS character rate.
+	 */
+	phy_cfg.hdmi.bpc = (s->output_mode == ROCKCHIP_OUT_MODE_YUV422 ?
+				8 : conn_state->hdmi.output_bpc);
 
 	ret = phy_configure(hdmi->phy, &phy_cfg);
 	if (!ret) {

-- 
2.54.0

Re: [PATCH 7/9] drm/rockchip: dw_hdmi_qp: Support 10-bit YUV422 output format

[sashiko-bot]

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

Pre-existing issues:
- [High] Mutating hardware state and global device tracking variables inside an atomic_check callback.
--

commit ee5f0003f61d5c3a78b5bc65346f39307471c74c
Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>

drm/rockchip: dw_hdmi_qp: Support 10-bit YUV422 output format

This commit adds support for the 10-bit YUV 4:2:2 output format
(MEDIA_BUS_FMT_UYVY10_1X20) to the Rockchip DW HDMI QP encoder. It also
forces 8 bpc when YUV 4:2:2 is in use to ensure the PHY PLL output remains
aligned with the TMDS character rate.

> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> --- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c

[ ... ]

> @@ -227,7 +228,14 @@ dw_hdmi_qp_rockchip_encoder_atomic_check(struct drm_encoder *encoder,
>  	}
>  
>  	phy_cfg.hdmi.tmds_char_rate = conn_state->hdmi.tmds_char_rate;
> -	phy_cfg.hdmi.bpc = conn_state->hdmi.output_bpc;
> +	/*
> +	 * YUV422 always transmits two 12-bit components per clock cycle,
> +	 * regardless of the color depth, which from a rate perspective is
> +	 * equivalent to three 8-bit RGB components.  Force 8 bpc here to
> +	 * keep the PHY PLL output aligned with the TMDS character rate.
> +	 */
> +	phy_cfg.hdmi.bpc = (s->output_mode == ROCKCHIP_OUT_MODE_YUV422 ?
> +				8 : conn_state->hdmi.output_bpc);
>  
>  	ret = phy_configure(hdmi->phy, &phy_cfg);
>  	if (!ret) {
>  		hdmi->tmds_char_rate = conn_state->hdmi.tmds_char_rate;
>  		s->output_type = DRM_MODE_CONNECTOR_HDMIA;
>  		s->output_bpc = conn_state->hdmi.output_bpc;
>  		s->bus_format = ingest_fmt;
>  	} else {

[Severity: High]
This is a pre-existing issue, but does
dw_hdmi_qp_rockchip_encoder_atomic_check() mutate hardware state and global
tracking variables during an atomic check?

In the DRM atomic framework, the atomic_check phase is expected to be
side-effect free. However, this code calls phy_configure(), which writes
directly to the PHY hardware registers, and then modifies the global
hdmi->tmds_char_rate variable.

Could this corrupt the active display configuration if userspace performs a
DRM_MODE_ATOMIC_TEST_ONLY commit or if an atomic commit fails a later
validation stage?

Furthermore, if the device is runtime-suspended, could accessing unclocked
hardware registers here trigger a synchronous abort and hang the system?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-dw-hdmi-qp-yuv-v1-0-a665cfd06d7d@collabora.com?part=7

[PATCH 8/9] drm/rockchip: dw_hdmi_qp: Enable YUV420 output format

[Cristian Ciocaltea]

Both RK3576 and RK3588 SoCs are capable of driving the YUV420 output
color format, and the required bus-format handling and VOP2 support are
already in place.  Advertise it via the platform supported formats so
the HDMI core can select it.

YUV420 halves the TMDS bandwidth compared to RGB/YUV444, which enables
high-resolution modes such as 4K@60Hz on links that cannot otherwise
carry the full-bandwidth signal.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
index 3a1c027aa90b..e29522afc6f0 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
@@ -612,7 +612,8 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
 
 	plat_data.supported_formats = BIT(DRM_OUTPUT_COLOR_FORMAT_RGB444) |
 				      BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR444) |
-				      BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR422);
+				      BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR422) |
+				      BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR420);
 
 	encoder = &hdmi->encoder.encoder;
 	encoder->possible_crtcs = drm_of_find_possible_crtcs(drm, dev->of_node);

-- 
2.54.0

Re: [PATCH 8/9] drm/rockchip: dw_hdmi_qp: Enable YUV420 output format

[sashiko-bot]

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

Pre-existing issues:
- [High] Hardware State Mutation in Atomic Check
--

commit 88f697e7fc21ab15ca1c009f0f830ab90e6ec4e7
Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>

drm/rockchip: dw_hdmi_qp: Enable YUV420 output format

Both RK3576 and RK3588 SoCs are capable of driving the YUV420 output
color format, and the required bus-format handling and VOP2 support are
already in place. Advertise it via the platform supported formats so
the HDMI core can select it.

> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> index 3a1c027aa90bb..e29522afc6f05 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> @@ -612,7 +612,8 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
>  
>  	plat_data.supported_formats = BIT(DRM_OUTPUT_COLOR_FORMAT_RGB444) |
>  				      BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR444) |
> -				      BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR422);
> +				      BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR422) |
> +				      BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR420);

[Severity: High]
This is a pre-existing issue, but I noticed that adding new formats might
exercise the atomic check path more frequently. 

Looking at dw_hdmi_qp_rockchip_encoder_atomic_check() in this same file,
it appears to mutate hardware state during the atomic validation phase:

drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c:dw_hdmi_qp_rockchip_encoder_atomic_check() {
	...
	ret = phy_configure(hdmi->phy, &phy_cfg);
	if (!ret) {
		hdmi->tmds_char_rate = conn_state->hdmi.tmds_char_rate;
		s->output_type = DRM_MODE_CONNECTOR_HDMIA;
		s->output_bpc = conn_state->hdmi.output_bpc;
		s->bus_format = ingest_fmt;
	} else {
	...
}

And in the PHY driver, phy_configure() mutates global state:

drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c:rk_hdptx_phy_configure() {
	...
	ret = rk_hdptx_phy_verify_hdmi_config(hdptx, &opts->hdmi, &hdptx->hdmi_cfg);
	if (ret) {
		dev_err(hdptx->dev, "invalid hdmi params for phy configure\n");
	} else {
		hdptx->restrict_rate_change = true;
	...
}

Does this sequence cause problems when a userspace application (like a display
compositor) submits a test-only commit using the DRM_MODE_ATOMIC_TEST_ONLY 
flag?

Since the atomic check runs for test-only commits, won't this corrupt the
active driver state and potentially break subsequent real commits if they skip
necessary hardware configurations due to the cached state already matching?

>  
>  	encoder = &hdmi->encoder.encoder;
>  	encoder->possible_crtcs = drm_of_find_possible_crtcs(drm, dev->of_node);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-dw-hdmi-qp-yuv-v1-0-a665cfd06d7d@collabora.com?part=8

[PATCH 9/9] arm64: dts: rockchip: Add RK3588 VOP2 resets

[Cristian Ciocaltea]

Add the missing reset properties to VOP2 on RK3588.

Co-developed-by: Andy Yan <andy.yan@rock-chips.com>
Signed-off-by: Andy Yan <andy.yan@rock-chips.com>
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 arch/arm64/boot/dts/rockchip/rk3588-base.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
index 4fb8888c281c..1404cd7ecf02 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
@@ -1453,6 +1453,18 @@ vop: vop@fdd90000 {
 			      "pll_hdmiphy0";
 		iommus = <&vop_mmu>;
 		power-domains = <&power RK3588_PD_VOP>;
+		resets = <&cru SRST_A_VOP>,
+			 <&cru SRST_H_VOP>,
+			 <&cru SRST_D_VOP0>,
+			 <&cru SRST_D_VOP1>,
+			 <&cru SRST_D_VOP2>,
+			 <&cru SRST_D_VOP3>;
+		reset-names = "axi",
+			      "ahb",
+			      "dclk_vp0",
+			      "dclk_vp1",
+			      "dclk_vp2",
+			      "dclk_vp3";
 		rockchip,grf = <&sys_grf>;
 		rockchip,vop-grf = <&vop_grf>;
 		rockchip,vo1-grf = <&vo1_grf>;

-- 
2.54.0