[PATCH 0/5] High color depth support for RK3576/RK3588 HDMI output

[PATCH 0/5] High color depth support for RK3576/RK3588 HDMI output

[Cristian Ciocaltea]

Since both Rockchip RK3576 and RK3588 SoCs are capable of handling 10
bpc color depth HDMI output and we've already landed the required HDPTX
PHY driver changes to support the setup, let's add the missing bits and
pieces to VOP2, DW HDMI QP encoder and bridge library.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
Cristian Ciocaltea (5):
      drm/rockchip: vop2: Add high color depth support
      drm/bridge: dw-hdmi-qp: Handle platform supported formats and color depth
      drm/rockchip: dw_hdmi_qp: Switch to phy_configure()
      drm/rockchip: dw_hdmi_qp: Use bit macros for RK3576 regs
      drm/rockchip: dw_hdmi_qp: Add high color depth support

 drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c   |  11 ++-
 drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c | 113 ++++++++++++++++++-------
 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c   |   3 +
 include/drm/bridge/dw_hdmi_qp.h                |   4 +
 4 files changed, 97 insertions(+), 34 deletions(-)
---
base-commit: 97987520025658f30bb787a99ffbd9bbff9ffc9d
change-id: 20250721-rk3588-10bpc-85ff4c228e63

[PATCH 1/5] drm/rockchip: vop2: Add high color depth support

[Cristian Ciocaltea]

Take the bits per color channel into consideration when computing DCLK
rate.

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

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
index 186f6452a7d359f079662bc580850929632ea8fe..a714bcbb02de16267e7febbaeb1eb270c70aaef2 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
@@ -1731,6 +1731,9 @@ static void vop2_crtc_atomic_enable(struct drm_crtc *crtc,
 		clock *= 2;
 	}
 
+	if (vcstate->output_bpc > 8)
+		clock = DIV_ROUND_CLOSEST(clock * vcstate->output_bpc, 8);
+
 	vop2_vp_write(vp, RK3568_VP_MIPI_CTRL, 0);
 
 	/*

-- 
2.50.0

Re:[PATCH 1/5] drm/rockchip: vop2: Add high color depth support

[Andy Yan]

Hello Cristian，

At 2025-07-22 01:39:04, "Cristian Ciocaltea" <cristian.ciocaltea@collabora.com> wrote:
>Take the bits per color channel into consideration when computing DCLK
>rate.
>
>Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>---
> drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 3 +++
> 1 file changed, 3 insertions(+)
>
>diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>index 186f6452a7d359f079662bc580850929632ea8fe..a714bcbb02de16267e7febbaeb1eb270c70aaef2 100644
>--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>@@ -1731,6 +1731,9 @@ static void vop2_crtc_atomic_enable(struct drm_crtc *crtc,
> 		clock *= 2;
> 	}
> 
>+	if (vcstate->output_bpc > 8)
>+		clock = DIV_ROUND_CLOSEST(clock * vcstate->output_bpc, 8);


This seems not right,  regardless of the value of bpc, the dclk of VOP must be
consistent with mode->crtc_clock.
If the dclk of VOP is increased in accordance with the BPC ratio here, then the refresh rate of VOP will also increase proportionally.
This would be inconsistent with the timing described in the mode.
For a hight color depth,  the frequency needs to be increased for the HDMI PHY's clock.


>+
> 	vop2_vp_write(vp, RK3568_VP_MIPI_CTRL, 0);
> 
> 	/*
>
>-- 
>2.50.0

Re: [PATCH 1/5] drm/rockchip: vop2: Add high color depth support

[Cristian Ciocaltea]

Hi Andy,

On 7/22/25 5:24 AM, Andy Yan wrote:
> 
> Hello Cristian，
> 
> At 2025-07-22 01:39:04, "Cristian Ciocaltea" <cristian.ciocaltea@collabora.com> wrote:
>> Take the bits per color channel into consideration when computing DCLK
>> rate.
>>
>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>> ---
>> drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>> index 186f6452a7d359f079662bc580850929632ea8fe..a714bcbb02de16267e7febbaeb1eb270c70aaef2 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>> @@ -1731,6 +1731,9 @@ static void vop2_crtc_atomic_enable(struct drm_crtc *crtc,
>> 		clock *= 2;
>> 	}
>>
>> +	if (vcstate->output_bpc > 8)
>> +		clock = DIV_ROUND_CLOSEST(clock * vcstate->output_bpc, 8);
> 
> 
> This seems not right,  regardless of the value of bpc, the dclk of VOP must be
> consistent with mode->crtc_clock.
> If the dclk of VOP is increased in accordance with the BPC ratio here, then the refresh rate of VOP will also increase proportionally.
> This would be inconsistent with the timing described in the mode.
> For a hight color depth,  the frequency needs to be increased for the HDMI PHY's clock.

The HDMI PHY's clock is actually computed at HDMI connector framework level
[1], taking into account the current bpc value, which is determined as part
of hdmi_compute_config() [2].

That means conn_state->hdmi.tmds_char_rate in
dw_hdmi_qp_rockchip_encoder_atomic_check() does already include the bpc
related adjustment, and we pass that directly to the PHY via
phy_configure().  Note there's still the need to handle bpc separately via
phy_configure_opts in order to setup CMN_REG(0086) [3].

Since VOP2 switches to PHY PLL as DCLK source for modes up to 4K@60Hz, it
needs to take color depth into account, to keep them in sync.  As a matter
of fact, the clock adjustment in VOP2 is mainly necessary for legacy
reasons, since HDPTX PHY allowed changing TMDS char rate via the Common
Clock Framework API.  We landed a proper solution for that via the HDMI PHY
API, hence the plan would be to make CCF API readonly after the switch to
PHY API is completed, which means VOP2 shouldn't deal anymore with clock
calculations when using the PHY PLL as DCLK source.

Regardless, I should probably move this clock adjustment to the conditional
block handling DCLK source switch.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/display/drm_hdmi_state_helper.c?h=v6.16-rc7#n525
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/display/drm_hdmi_state_helper.c?h=v6.16-rc7#n608
[3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c?h=v6.16-rc7#n1034

Re:Re: [PATCH 1/5] drm/rockchip: vop2: Add high color depth support

[Andy Yan]

Hello Cristian,

在 2025-07-22 14:16:26，"Cristian Ciocaltea" <cristian.ciocaltea@collabora.com> 写道：
>Hi Andy,
>
>On 7/22/25 5:24 AM, Andy Yan wrote:
>> 
>> Hello Cristian，
>> 
>> At 2025-07-22 01:39:04, "Cristian Ciocaltea" <cristian.ciocaltea@collabora.com> wrote:
>>> Take the bits per color channel into consideration when computing DCLK
>>> rate.
>>>
>>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>>> ---
>>> drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>>> index 186f6452a7d359f079662bc580850929632ea8fe..a714bcbb02de16267e7febbaeb1eb270c70aaef2 100644
>>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>>> @@ -1731,6 +1731,9 @@ static void vop2_crtc_atomic_enable(struct drm_crtc *crtc,
>>> 		clock *= 2;
>>> 	}
>>>
>>> +	if (vcstate->output_bpc > 8)
>>> +		clock = DIV_ROUND_CLOSEST(clock * vcstate->output_bpc, 8);
>> 
>> 
>> This seems not right,  regardless of the value of bpc, the dclk of VOP must be
>> consistent with mode->crtc_clock.
>> If the dclk of VOP is increased in accordance with the BPC ratio here, then the refresh rate of VOP will also increase proportionally.
>> This would be inconsistent with the timing described in the mode.
>> For a hight color depth,  the frequency needs to be increased for the HDMI PHY's clock.
>
>The HDMI PHY's clock is actually computed at HDMI connector framework level
>[1], taking into account the current bpc value, which is determined as part
>of hdmi_compute_config() [2].
>
>That means conn_state->hdmi.tmds_char_rate in
>dw_hdmi_qp_rockchip_encoder_atomic_check() does already include the bpc
>related adjustment, and we pass that directly to the PHY via
>phy_configure().  Note there's still the need to handle bpc separately via
>phy_configure_opts in order to setup CMN_REG(0086) [3].
>
>Since VOP2 switches to PHY PLL as DCLK source for modes up to 4K@60Hz, it
>needs to take color depth into account, to keep them in sync.  As a matter
>of fact, the clock adjustment in VOP2 is mainly necessary for legacy
>reasons, since HDPTX PHY allowed changing TMDS char rate via the Common
>Clock Framework API.  We landed a proper solution for that via the HDMI PHY
>API, hence the plan would be to make CCF API readonly after the switch to
>PHY API is completed, which means VOP2 shouldn't deal anymore with clock
>calculations when using the PHY PLL as DCLK source.

What I want to emphasize is that on the vop/crtc side, what we should be concerned about is mode -> clock, 
not the HDMI PHY clock. The HDMI PHY clock is something that the HDMI bridge or the HDMI PHY driver needs to pay attention to.

This patch works just because currently, on RK3576 and RK3588, the HDMI PHY PLL can be used as the dclk source for the vop.
After correctly setting CMN_REG0086, it can precisely adjust the HDMI HPY PLL clock adjusted according to the bpc further to
the frequency of mode->clock, and then use it as the dclk for the vop.
However, we also need to be aware of the following situations:

1) When we are using HDMI, we can still choose the system PLL instead of the HDMI PHY PLL as
    the clock source for dclk. In this case, this patch will cause incorrect mode->clock.
2)  When we are using HDMI above the 4K60 mode(4K120), we can only use the system PLL instead
     of the HDMIPHY PLL as the clock source for the vop dclk,  this patch will cause incorrect mode->clock.
3) For RK3576 and RK3588,  interfaces such as DP, eDP, and DSI also support 10-bit mode. 
    However, in most cases, they will choose the system PLL as the vop dclk clock source,this patch will cause incorrect mode->clock.
4) There are also other platforms, such as RK3568, even for HDMI , it still uses the system PLL as the dclk clock source for vop.
     this patch will cause incorrect mode->clock.


>
>Regardless, I should probably move this clock adjustment to the conditional
>block handling DCLK source switch.
>
>[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/display/drm_hdmi_state_helper.c?h=v6.16-rc7#n525
>[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/display/drm_hdmi_state_helper.c?h=v6.16-rc7#n608
>[3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c?h=v6.16-rc7#n1034

Re: [PATCH 1/5] drm/rockchip: vop2: Add high color depth support

[Cristian Ciocaltea]

Hi Andy,

On 7/22/25 9:55 AM, Andy Yan wrote:
> 
> Hello Cristian,
> 
> 在 2025-07-22 14:16:26，"Cristian Ciocaltea" <cristian.ciocaltea@collabora.com> 写道：
>> Hi Andy,
>>
>> On 7/22/25 5:24 AM, Andy Yan wrote:
>>>
>>> Hello Cristian，
>>>
>>> At 2025-07-22 01:39:04, "Cristian Ciocaltea" <cristian.ciocaltea@collabora.com> wrote:
>>>> Take the bits per color channel into consideration when computing DCLK
>>>> rate.
>>>>
>>>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>>>> ---
>>>> drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 3 +++
>>>> 1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>>>> index 186f6452a7d359f079662bc580850929632ea8fe..a714bcbb02de16267e7febbaeb1eb270c70aaef2 100644
>>>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>>>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>>>> @@ -1731,6 +1731,9 @@ static void vop2_crtc_atomic_enable(struct drm_crtc *crtc,
>>>> 		clock *= 2;
>>>> 	}
>>>>
>>>> +	if (vcstate->output_bpc > 8)
>>>> +		clock = DIV_ROUND_CLOSEST(clock * vcstate->output_bpc, 8);
>>>
>>>
>>> This seems not right,  regardless of the value of bpc, the dclk of VOP must be
>>> consistent with mode->crtc_clock.
>>> If the dclk of VOP is increased in accordance with the BPC ratio here, then the refresh rate of VOP will also increase proportionally.
>>> This would be inconsistent with the timing described in the mode.
>>> For a hight color depth,  the frequency needs to be increased for the HDMI PHY's clock.
>>
>> The HDMI PHY's clock is actually computed at HDMI connector framework level
>> [1], taking into account the current bpc value, which is determined as part
>> of hdmi_compute_config() [2].
>>
>> That means conn_state->hdmi.tmds_char_rate in
>> dw_hdmi_qp_rockchip_encoder_atomic_check() does already include the bpc
>> related adjustment, and we pass that directly to the PHY via
>> phy_configure().  Note there's still the need to handle bpc separately via
>> phy_configure_opts in order to setup CMN_REG(0086) [3].
>>
>> Since VOP2 switches to PHY PLL as DCLK source for modes up to 4K@60Hz, it
>> needs to take color depth into account, to keep them in sync.  As a matter
>> of fact, the clock adjustment in VOP2 is mainly necessary for legacy
>> reasons, since HDPTX PHY allowed changing TMDS char rate via the Common
>> Clock Framework API.  We landed a proper solution for that via the HDMI PHY
>> API, hence the plan would be to make CCF API readonly after the switch to
>> PHY API is completed, which means VOP2 shouldn't deal anymore with clock
>> calculations when using the PHY PLL as DCLK source.
> 
> What I want to emphasize is that on the vop/crtc side, what we should be concerned about is mode -> clock, 
> not the HDMI PHY clock. The HDMI PHY clock is something that the HDMI bridge or the HDMI PHY driver needs to pay attention to.
> 
> This patch works just because currently, on RK3576 and RK3588, the HDMI PHY PLL can be used as the dclk source for the vop.
> After correctly setting CMN_REG0086, it can precisely adjust the HDMI HPY PLL clock adjusted according to the bpc further to
> the frequency of mode->clock, and then use it as the dclk for the vop.
> However, we also need to be aware of the following situations:
> 
> 1) When we are using HDMI, we can still choose the system PLL instead of the HDMI PHY PLL as
>     the clock source for dclk. In this case, this patch will cause incorrect mode->clock.
> 2)  When we are using HDMI above the 4K60 mode(4K120), we can only use the system PLL instead
>      of the HDMIPHY PLL as the clock source for the vop dclk,  this patch will cause incorrect mode->clock.
> 3) For RK3576 and RK3588,  interfaces such as DP, eDP, and DSI also support 10-bit mode. 
>     However, in most cases, they will choose the system PLL as the vop dclk clock source,this patch will cause incorrect mode->clock.
> 4) There are also other platforms, such as RK3568, even for HDMI , it still uses the system PLL as the dclk clock source for vop.
>      this patch will cause incorrect mode->clock.

Oh, I wrongly assumed high color depth is not currently supported/in use for vop2.

In that case my suggestion below is actually a mandatory thing to do, as it should 
correctly reduce the scope to RK3576 & RK3588 only.

>>
>> Regardless, I should probably move this clock adjustment to the conditional
>> block handling DCLK source switch.
>>
>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/display/drm_hdmi_state_helper.c?h=v6.16-rc7#n525
>> [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/display/drm_hdmi_state_helper.c?h=v6.16-rc7#n608
>> [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c?h=v6.16-rc7#n1034

Re:Re: [PATCH 1/5] drm/rockchip: vop2: Add high color depth support

[Andy Yan]

Hello Cristian，
在 2025-07-22 14:16:26，"Cristian Ciocaltea" <cristian.ciocaltea@collabora.com> 写道：
>Hi Andy,
>
>On 7/22/25 5:24 AM, Andy Yan wrote:
>> 
>> Hello Cristian，
>> 
>> At 2025-07-22 01:39:04, "Cristian Ciocaltea" <cristian.ciocaltea@collabora.com> wrote:
>>> Take the bits per color channel into consideration when computing DCLK
>>> rate.
>>>
>>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>>> ---
>>> drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>>> index 186f6452a7d359f079662bc580850929632ea8fe..a714bcbb02de16267e7febbaeb1eb270c70aaef2 100644
>>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>>> @@ -1731,6 +1731,9 @@ static void vop2_crtc_atomic_enable(struct drm_crtc *crtc,
>>> 		clock *= 2;
>>> 	}
>>>
>>> +	if (vcstate->output_bpc > 8)
>>> +		clock = DIV_ROUND_CLOSEST(clock * vcstate->output_bpc, 8);
>> 
>> 
>> This seems not right,  regardless of the value of bpc, the dclk of VOP must be
>> consistent with mode->crtc_clock.
>> If the dclk of VOP is increased in accordance with the BPC ratio here, then the refresh rate of VOP will also increase proportionally.
>> This would be inconsistent with the timing described in the mode.
>> For a hight color depth,  the frequency needs to be increased for the HDMI PHY's clock.
>
>The HDMI PHY's clock is actually computed at HDMI connector framework level
>[1], taking into account the current bpc value, which is determined as part
>of hdmi_compute_config() [2].
>
>That means conn_state->hdmi.tmds_char_rate in
>dw_hdmi_qp_rockchip_encoder_atomic_check() does already include the bpc
>related adjustment, and we pass that directly to the PHY via
>phy_configure().  Note there's still the need to handle bpc separately via
>phy_configure_opts in order to setup CMN_REG(0086) [3].
>
>Since VOP2 switches to PHY PLL as DCLK source for modes up to 4K@60Hz, it
>needs to take color depth into account, to keep them in sync.  As a matter
>of fact, the clock adjustment in VOP2 is mainly necessary for legacy


There might be a misunderstanding here. The values of hdmi.tmds_char_rate and vop dclk are not always equal in all cases.
For vop dclk always match mode->clock,  hdmi.tmds_char_rate will adjust with bpc.
For example,  for a 4KP30 YUV444 8 bit,   vop->dclk = hdmi.tmds_char_rate = 297MHZ,
for 4KP30 YUV444 10 bit,  hdmi.tmds_char_rate = 1.25 * vop->dclk.

In fact, for the HDMI PHY, there are two clocks. One is a clock with the same frequency as the HDMI PLL, namely.tmds_char_rate,
 and the other is a clock obtained by further frequency division of the HDMI PLL, which can be supplied to the vop dclk.

                      ----------------------- >tmds_char_rate 
                      |
PHY PLL------|

                      |
                      ------DIV(REG0086[3-1])-->vop dclk.

So in this case, my suggestion is still to follow the implementation of our BSP kernel:
On the crtc side, it shoud set dclk as the mode->clock.
At the PHY side(rk_hdptx_phy_clk_set_rate) when set the vop dclk, it should adjust the PLL clock according to the bpc:
if (bpc > 8)
                bit_rate = clock * 10 / 8;



>reasons, since HDPTX PHY allowed changing TMDS char rate via the Common
>Clock Framework API.  We landed a proper solution for that via the HDMI PHY
>API, hence the plan would be to make CCF API readonly after the switch to
>PHY API is completed, which means VOP2 shouldn't deal anymore with clock
>calculations when using the PHY PLL as DCLK source.
>
>Regardless, I should probably move this clock adjustment to the conditional
>block handling DCLK source switch.
>
>[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/display/drm_hdmi_state_helper.c?h=v6.16-rc7#n525
>[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/display/drm_hdmi_state_helper.c?h=v6.16-rc7#n608
>[3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c?h=v6.16-rc7#n1034

Re: [PATCH 1/5] drm/rockchip: vop2: Add high color depth support

[Cristian Ciocaltea]

Hi Andy,

On 7/24/25 11:56 AM, Andy Yan wrote:
> 
> 
> 
> Hello Cristian，
> 在 2025-07-22 14:16:26，"Cristian Ciocaltea" <cristian.ciocaltea@collabora.com> 写道：
>> Hi Andy,
>>
>> On 7/22/25 5:24 AM, Andy Yan wrote:
>>>
>>> Hello Cristian，
>>>
>>> At 2025-07-22 01:39:04, "Cristian Ciocaltea" <cristian.ciocaltea@collabora.com> wrote:
>>>> Take the bits per color channel into consideration when computing DCLK
>>>> rate.
>>>>
>>>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>>>> ---
>>>> drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 3 +++
>>>> 1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>>>> index 186f6452a7d359f079662bc580850929632ea8fe..a714bcbb02de16267e7febbaeb1eb270c70aaef2 100644
>>>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>>>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>>>> @@ -1731,6 +1731,9 @@ static void vop2_crtc_atomic_enable(struct drm_crtc *crtc,
>>>> 		clock *= 2;
>>>> 	}
>>>>
>>>> +	if (vcstate->output_bpc > 8)
>>>> +		clock = DIV_ROUND_CLOSEST(clock * vcstate->output_bpc, 8);
>>>
>>>
>>> This seems not right,  regardless of the value of bpc, the dclk of VOP must be
>>> consistent with mode->crtc_clock.
>>> If the dclk of VOP is increased in accordance with the BPC ratio here, then the refresh rate of VOP will also increase proportionally.
>>> This would be inconsistent with the timing described in the mode.
>>> For a hight color depth,  the frequency needs to be increased for the HDMI PHY's clock.
>>
>> The HDMI PHY's clock is actually computed at HDMI connector framework level
>> [1], taking into account the current bpc value, which is determined as part
>> of hdmi_compute_config() [2].
>>
>> That means conn_state->hdmi.tmds_char_rate in
>> dw_hdmi_qp_rockchip_encoder_atomic_check() does already include the bpc
>> related adjustment, and we pass that directly to the PHY via
>> phy_configure().  Note there's still the need to handle bpc separately via
>> phy_configure_opts in order to setup CMN_REG(0086) [3].
>>
>> Since VOP2 switches to PHY PLL as DCLK source for modes up to 4K@60Hz, it
>> needs to take color depth into account, to keep them in sync.  As a matter
>> of fact, the clock adjustment in VOP2 is mainly necessary for legacy
> 
> 
> There might be a misunderstanding here. The values of hdmi.tmds_char_rate and vop dclk are not always equal in all cases.
> For vop dclk always match mode->clock,  hdmi.tmds_char_rate will adjust with bpc.
> For example,  for a 4KP30 YUV444 8 bit,   vop->dclk = hdmi.tmds_char_rate = 297MHZ,
> for 4KP30 YUV444 10 bit,  hdmi.tmds_char_rate = 1.25 * vop->dclk.
> 
> In fact, for the HDMI PHY, there are two clocks. One is a clock with the same frequency as the HDMI PLL, namely.tmds_char_rate,
>  and the other is a clock obtained by further frequency division of the HDMI PLL, which can be supplied to the vop dclk.
> 
>                       ----------------------- >tmds_char_rate 
>                       |
> PHY PLL------|
> 
>                       |
>                       ------DIV(REG0086[3-1])-->vop dclk.

Oh, I missed the fact there's a divider between PLL and DCLK.
All this makes sense now, thanks for clarifying!

Regards,
Cristian

> So in this case, my suggestion is still to follow the implementation of our BSP kernel:
> On the crtc side, it shoud set dclk as the mode->clock.
> At the PHY side(rk_hdptx_phy_clk_set_rate) when set the vop dclk, it should adjust the PLL clock according to the bpc:
> if (bpc > 8)
>                 bit_rate = clock * 10 / 8;
> 
> 
> 
>> reasons, since HDPTX PHY allowed changing TMDS char rate via the Common
>> Clock Framework API.  We landed a proper solution for that via the HDMI PHY
>> API, hence the plan would be to make CCF API readonly after the switch to
>> PHY API is completed, which means VOP2 shouldn't deal anymore with clock
>> calculations when using the PHY PLL as DCLK source.
>>
>> Regardless, I should probably move this clock adjustment to the conditional
>> block handling DCLK source switch.
>>
>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/display/drm_hdmi_state_helper.c?h=v6.16-rc7#n525
>> [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/display/drm_hdmi_state_helper.c?h=v6.16-rc7#n608
>> [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c?h=v6.16-rc7#n1034

[PATCH 2/5] drm/bridge: dw-hdmi-qp: Handle platform supported formats and color depth

[Cristian Ciocaltea]

Extend struct dw_hdmi_qp_plat_data to include the supported display
output formats and maximum bits per color channel.  When provided by the
platform driver, use them to setup the HDMI bridge accordingly.

Additionally, improve debug logging in dw_hdmi_qp_bridge_atomic_enable()
to also show the current HDMI output format and bpc.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c | 11 +++++++++--
 include/drm/bridge/dw_hdmi_qp.h              |  4 ++++
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
index 39332c57f2c54296f39e27612544f4fbf923863f..25edb35feb0a1c9861fdd4ca5fab5c08d44af356 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
@@ -848,8 +848,9 @@ 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 rate=%llu\n",
-			__func__, conn_state->hdmi.tmds_char_rate);
+		dev_dbg(hdmi->dev, "%s mode=HDMI %s rate=%llu bpc=%u\n", __func__,
+			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 {
@@ -1085,6 +1086,12 @@ struct dw_hdmi_qp *dw_hdmi_qp_bind(struct platform_device *pdev,
 	hdmi->bridge.vendor = "Synopsys";
 	hdmi->bridge.product = "DW HDMI QP TX";
 
+	if (plat_data->supported_formats)
+		hdmi->bridge.supported_formats = plat_data->supported_formats;
+
+	if (plat_data->max_bpc)
+		hdmi->bridge.max_bpc = plat_data->max_bpc;
+
 	hdmi->bridge.ddc = dw_hdmi_qp_i2c_adapter(hdmi);
 	if (IS_ERR(hdmi->bridge.ddc))
 		return ERR_CAST(hdmi->bridge.ddc);
diff --git a/include/drm/bridge/dw_hdmi_qp.h b/include/drm/bridge/dw_hdmi_qp.h
index e9be6d507ad9cdc55f5c7d6d3ef37eba41f1ce74..09c86a053136251c25237751e66564386dafdb3c 100644
--- a/include/drm/bridge/dw_hdmi_qp.h
+++ b/include/drm/bridge/dw_hdmi_qp.h
@@ -23,6 +23,10 @@ struct dw_hdmi_qp_plat_data {
 	const struct dw_hdmi_qp_phy_ops *phy_ops;
 	void *phy_data;
 	int main_irq;
+	/* Supported output formats: bitmask of @hdmi_colorspace */
+	unsigned int supported_formats;
+	/* Maximum bits per color channel: 8, 10 or 12 */
+	unsigned int max_bpc;
 };
 
 struct dw_hdmi_qp *dw_hdmi_qp_bind(struct platform_device *pdev,

-- 
2.50.0

[PATCH 3/5] drm/rockchip: dw_hdmi_qp: Switch to phy_configure()

[Cristian Ciocaltea]

Stop relying on phy_set_bus_width() based workaround to setup the TMDS
character rate and, instead, use the recently introduced HDMI PHY
configuration API.  This is also a prerequisite to enable high color
depth and FRL support.

Additionally, move the logic to ->atomic_check() callback where the
current mode rate is already provided by the connector state.  As a
matter of fact this is actually necessary to ensure the link rate is
configured before VOP2 attempts to use the PHY PLL as a DCLK source in
vop2_crtc_atomic_enable().  The rationale is to restrict any changes of
the PHY rate via CCF and, instead, prefer the PHY configuration API for
this purpose.

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

diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
index 7d531b6f4c098c6c548788dad487ce4613a2f32b..b2dd29347338d58640387adb2b455cc1558d4272 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
@@ -13,6 +13,7 @@
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/phy/phy.h>
+#include <linux/phy/phy-hdmi.h>
 #include <linux/regmap.h>
 #include <linux/workqueue.h>
 
@@ -95,6 +96,7 @@ struct rockchip_hdmi_qp {
 	struct delayed_work hpd_work;
 	int port_id;
 	const struct rockchip_hdmi_qp_ctrl_ops *ctrl_ops;
+	unsigned long long tmds_char_rate;
 };
 
 struct rockchip_hdmi_qp_ctrl_ops {
@@ -113,24 +115,9 @@ static struct rockchip_hdmi_qp *to_rockchip_hdmi_qp(struct drm_encoder *encoder)
 static void dw_hdmi_qp_rockchip_encoder_enable(struct drm_encoder *encoder)
 {
 	struct rockchip_hdmi_qp *hdmi = to_rockchip_hdmi_qp(encoder);
-	struct drm_crtc *crtc = encoder->crtc;
-	unsigned long long rate;
 
 	/* Unconditionally switch to TMDS as FRL is not yet supported */
 	gpiod_set_value(hdmi->enable_gpio, 1);
-
-	if (crtc && crtc->state) {
-		rate = drm_hdmi_compute_mode_clock(&crtc->state->adjusted_mode,
-						   8, HDMI_COLORSPACE_RGB);
-		/*
-		 * FIXME: Temporary workaround to pass pixel clock rate
-		 * to the PHY driver until phy_configure_opts_hdmi
-		 * becomes available in the PHY API. See also the related
-		 * comment in rk_hdptx_phy_power_on() from
-		 * drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
-		 */
-		phy_set_bus_width(hdmi->phy, div_u64(rate, 100));
-	}
 }
 
 static int
@@ -138,12 +125,26 @@ dw_hdmi_qp_rockchip_encoder_atomic_check(struct drm_encoder *encoder,
 					 struct drm_crtc_state *crtc_state,
 					 struct drm_connector_state *conn_state)
 {
+	struct rockchip_hdmi_qp *hdmi = to_rockchip_hdmi_qp(encoder);
 	struct rockchip_crtc_state *s = to_rockchip_crtc_state(crtc_state);
+	union phy_configure_opts phy_cfg = {};
+	int ret;
 
-	s->output_mode = ROCKCHIP_OUT_MODE_AAAA;
-	s->output_type = DRM_MODE_CONNECTOR_HDMIA;
+	if (hdmi->tmds_char_rate == conn_state->hdmi.tmds_char_rate)
+		return 0;
 
-	return 0;
+	phy_cfg.hdmi.tmds_char_rate = conn_state->hdmi.tmds_char_rate;
+
+	ret = phy_configure(hdmi->phy, &phy_cfg);
+	if (!ret) {
+		hdmi->tmds_char_rate = conn_state->hdmi.tmds_char_rate;
+		s->output_mode = ROCKCHIP_OUT_MODE_AAAA;
+		s->output_type = DRM_MODE_CONNECTOR_HDMIA;
+	} else {
+		dev_err(hdmi->dev, "Failed to configure phy: %d\n", ret);
+	}
+
+	return ret;
 }
 
 static const struct

-- 
2.50.0

[PATCH 4/5] drm/rockchip: dw_hdmi_qp: Use bit macros for RK3576 regs

[Cristian Ciocaltea]

For consistency and improved readability, redefine a few RK3576 specific
register configurations by relying on GENMASK() and unshifted values for
color depth and output format.  Those are not used at the moment, but
will be needed soon to support the related features.

While at it, drop a few other defines which are unlikely to be ever
required.

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

diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
index b2dd29347338d58640387adb2b455cc1558d4272..578ff5eb87f1e27b9bb9a6a299347b24b45381bb 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
@@ -38,21 +38,16 @@
 #define RK3576_HDMI_HDCP14_MEM_EN	BIT(15)
 
 #define RK3576_VO0_GRF_SOC_CON8		0x0020
-#define RK3576_COLOR_FORMAT_MASK	(0xf << 4)
-#define RK3576_COLOR_DEPTH_MASK		(0xf << 8)
-#define RK3576_RGB			(0 << 4)
-#define RK3576_YUV422			(0x1 << 4)
-#define RK3576_YUV444			(0x2 << 4)
-#define RK3576_YUV420			(0x3 << 4)
-#define RK3576_8BPC			(0x0 << 8)
-#define RK3576_10BPC			(0x6 << 8)
+#define RK3576_COLOR_DEPTH_MASK		GENMASK(11, 8)
+#define RK3576_8BPC			0x0
+#define RK3576_10BPC			0x6
+#define RK3576_COLOR_FORMAT_MASK	GENMASK(7, 4)
+#define RK3576_RGB			0x9
+#define RK3576_YUV422			0x1
+#define RK3576_YUV444			0x2
+#define RK3576_YUV420			0x3
 #define RK3576_CECIN_MASK		BIT(3)
 
-#define RK3576_VO0_GRF_SOC_CON12	0x0030
-#define RK3576_GRF_OSDA_DLYN		(0xf << 12)
-#define RK3576_GRF_OSDA_DIV		(0x7f << 1)
-#define RK3576_GRF_OSDA_DLY_EN		BIT(0)
-
 #define RK3576_VO0_GRF_SOC_CON14	0x0038
 #define RK3576_I2S_SEL_MASK		BIT(0)
 #define RK3576_SPDIF_SEL_MASK		BIT(1)

-- 
2.50.0

[PATCH 5/5] drm/rockchip: dw_hdmi_qp: Add high color depth support

[Cristian Ciocaltea]

Since both RK3576 and RK3588 SoCs are capable of handling 10 bpc color
depth, introduce a pair of new helpers to program the necessary
registers, as well as passing bpc at PHY configuration level.

Note max_bpc is unconditionally set to 10 before initializing the QP
bridge library, as there is no need to adjust it dynamically, i.e. per
SoC variant, for now.

While setting up .enc_init() callbacks of rockchip_hdmi_qp_ctrl_ops,
also replace the unnecessary whitespace chars before .irq_callback()
assignments.

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

diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
index 578ff5eb87f1e27b9bb9a6a299347b24b45381bb..49d58f1321034b325a0741794a62a279971d5f4c 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
@@ -7,6 +7,7 @@
  * Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
  */
 
+#include <linux/bitfield.h>
 #include <linux/clk.h>
 #include <linux/gpio/consumer.h>
 #include <linux/mfd/syscon.h>
@@ -68,6 +69,12 @@
 #define RK3588_HDMI1_LEVEL_INT		BIT(24)
 #define RK3588_GRF_VO1_CON3		0x000c
 #define RK3588_GRF_VO1_CON6		0x0018
+#define RK3588_COLOR_DEPTH_MASK		GENMASK(7, 4)
+#define RK3588_8BPC			0x0
+#define RK3588_10BPC			0x6
+#define RK3588_COLOR_FORMAT_MASK	GENMASK(3, 0)
+#define RK3588_RGB			0x0
+#define RK3588_YUV420			0x3
 #define RK3588_SCLIN_MASK		BIT(9)
 #define RK3588_SDAIN_MASK		BIT(10)
 #define RK3588_MODE_MASK		BIT(11)
@@ -96,6 +103,7 @@ struct rockchip_hdmi_qp {
 
 struct rockchip_hdmi_qp_ctrl_ops {
 	void (*io_init)(struct rockchip_hdmi_qp *hdmi);
+	void (*enc_init)(struct rockchip_hdmi_qp *hdmi, struct rockchip_crtc_state *state);
 	irqreturn_t (*irq_callback)(int irq, void *dev_id);
 	irqreturn_t (*hardirq_callback)(int irq, void *dev_id);
 };
@@ -110,9 +118,16 @@ static struct rockchip_hdmi_qp *to_rockchip_hdmi_qp(struct drm_encoder *encoder)
 static void dw_hdmi_qp_rockchip_encoder_enable(struct drm_encoder *encoder)
 {
 	struct rockchip_hdmi_qp *hdmi = to_rockchip_hdmi_qp(encoder);
+	struct drm_crtc *crtc = encoder->crtc;
 
 	/* Unconditionally switch to TMDS as FRL is not yet supported */
 	gpiod_set_value(hdmi->enable_gpio, 1);
+
+	if (!crtc || !crtc->state)
+		return;
+
+	if (hdmi->ctrl_ops->enc_init)
+		hdmi->ctrl_ops->enc_init(hdmi, to_rockchip_crtc_state(crtc->state));
 }
 
 static int
@@ -125,16 +140,19 @@ dw_hdmi_qp_rockchip_encoder_atomic_check(struct drm_encoder *encoder,
 	union phy_configure_opts phy_cfg = {};
 	int ret;
 
-	if (hdmi->tmds_char_rate == conn_state->hdmi.tmds_char_rate)
+	if (hdmi->tmds_char_rate == conn_state->hdmi.tmds_char_rate &&
+	    s->output_bpc == conn_state->hdmi.output_bpc)
 		return 0;
 
 	phy_cfg.hdmi.tmds_char_rate = conn_state->hdmi.tmds_char_rate;
+	phy_cfg.hdmi.bpc = 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_mode = ROCKCHIP_OUT_MODE_AAAA;
 		s->output_type = DRM_MODE_CONNECTOR_HDMIA;
+		s->output_bpc = conn_state->hdmi.output_bpc;
 	} else {
 		dev_err(hdmi->dev, "Failed to configure phy: %d\n", ret);
 	}
@@ -373,15 +391,49 @@ static void dw_hdmi_qp_rk3588_io_init(struct rockchip_hdmi_qp *hdmi)
 	regmap_write(hdmi->regmap, RK3588_GRF_SOC_CON2, val);
 }
 
+static void dw_hdmi_qp_rk3576_enc_init(struct rockchip_hdmi_qp *hdmi,
+				       struct rockchip_crtc_state *state)
+{
+	u32 val;
+
+	if (state->output_bpc == 10)
+		val = HIWORD_UPDATE(FIELD_PREP(RK3576_COLOR_DEPTH_MASK, RK3576_10BPC),
+				    RK3576_COLOR_DEPTH_MASK);
+	else
+		val = HIWORD_UPDATE(FIELD_PREP(RK3576_COLOR_DEPTH_MASK, RK3576_8BPC),
+				    RK3576_COLOR_DEPTH_MASK);
+
+	regmap_write(hdmi->vo_regmap, RK3576_VO0_GRF_SOC_CON8, val);
+}
+
+static void dw_hdmi_qp_rk3588_enc_init(struct rockchip_hdmi_qp *hdmi,
+				       struct rockchip_crtc_state *state)
+{
+	u32 val;
+
+	if (state->output_bpc == 10)
+		val = HIWORD_UPDATE(FIELD_PREP(RK3588_COLOR_DEPTH_MASK, RK3588_10BPC),
+				    RK3588_COLOR_DEPTH_MASK);
+	else
+		val = HIWORD_UPDATE(FIELD_PREP(RK3588_COLOR_DEPTH_MASK, RK3588_8BPC),
+				    RK3588_COLOR_DEPTH_MASK);
+
+	regmap_write(hdmi->vo_regmap,
+		     hdmi->port_id ? RK3588_GRF_VO1_CON6 : RK3588_GRF_VO1_CON3,
+		     val);
+}
+
 static const struct rockchip_hdmi_qp_ctrl_ops rk3576_hdmi_ctrl_ops = {
 	.io_init		= dw_hdmi_qp_rk3576_io_init,
-	.irq_callback	        = dw_hdmi_qp_rk3576_irq,
+	.enc_init		= dw_hdmi_qp_rk3576_enc_init,
+	.irq_callback		= dw_hdmi_qp_rk3576_irq,
 	.hardirq_callback	= dw_hdmi_qp_rk3576_hardirq,
 };
 
 static const struct rockchip_hdmi_qp_ctrl_ops rk3588_hdmi_ctrl_ops = {
 	.io_init		= dw_hdmi_qp_rk3588_io_init,
-	.irq_callback	        = dw_hdmi_qp_rk3588_irq,
+	.enc_init		= dw_hdmi_qp_rk3588_enc_init,
+	.irq_callback		= dw_hdmi_qp_rk3588_irq,
 	.hardirq_callback	= dw_hdmi_qp_rk3588_hardirq,
 };
 
@@ -476,6 +528,7 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
 
 	plat_data.phy_ops = cfg->phy_ops;
 	plat_data.phy_data = hdmi;
+	plat_data.max_bpc = 10;
 
 	encoder = &hdmi->encoder.encoder;
 	encoder->possible_crtcs = drm_of_find_possible_crtcs(drm, dev->of_node);

-- 
2.50.0