[PATCH v2 0/4] drm/vc4: Support HDMI QHD or higher output

[PATCH v2 0/4] drm/vc4: Support HDMI QHD or higher output

[Hoegeun Kwon]

Hi everyone,

There is a problem that the output does not work at a resolution
exceeding FHD. To solve this, we need to adjust the bvb clock at a
resolution exceeding FHD.

Rebased on top of next-20200708 and [1].

[1] : [PATCH v4 00/78] drm/vc4: Support BCM2711 Display Pipeline (Maxime's patchset)

Changes from v1:
  - Added dt-bindings documents
  - Change patch order, first fix driver and then device tree

Hoegeun Kwon (4):
  clk: bcm: rpi: Add register to control pixel bvb clk
  drm/vc4: hdmi: Add pixel bvb clock control
  dt-bindings: display: vc4: hdmi: Add bvb clock-names property
  ARM: dts: bcm2711: Add bvb clock for hdmi-pixel

 .../bindings/display/brcm,bcm2711-hdmi.yaml   | 12 ++++++---
 arch/arm/boot/dts/bcm2711-rpi-4-b.dts         |  6 +++--
 drivers/clk/bcm/clk-raspberrypi.c             |  1 +
 drivers/gpu/drm/vc4/vc4_hdmi.c                | 25 +++++++++++++++++++
 drivers/gpu/drm/vc4/vc4_hdmi.h                |  1 +
 5 files changed, 39 insertions(+), 6 deletions(-)

-- 
2.17.1

[PATCH v2 1/4] clk: bcm: rpi: Add register to control pixel bvb clk

[Hoegeun Kwon]

To use QHD or higher, we need to modify the pixel_bvb_clk value. So
add register to control this clock.

Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
---
 drivers/clk/bcm/clk-raspberrypi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk-raspberrypi.c
index 5cc82954e1ce..f89b9cfc4309 100644
--- a/drivers/clk/bcm/clk-raspberrypi.c
+++ b/drivers/clk/bcm/clk-raspberrypi.c
@@ -271,6 +271,7 @@ static int raspberrypi_discover_clocks(struct raspberrypi_clk *rpi,
 		case RPI_FIRMWARE_CORE_CLK_ID:
 		case RPI_FIRMWARE_M2MC_CLK_ID:
 		case RPI_FIRMWARE_V3D_CLK_ID:
+		case RPI_FIRMWARE_PIXEL_BVB_CLK_ID:
 			hw = raspberrypi_clk_register(rpi, clks->parent,
 						      clks->id);
 			if (IS_ERR(hw))
-- 
2.17.1

Re: [PATCH v2 1/4] clk: bcm: rpi: Add register to control pixel bvb clk

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 408 bytes --]

Hi Stephen, Mike,

On Tue, Sep 01, 2020 at 01:07:56PM +0900, Hoegeun Kwon wrote:
> To use QHD or higher, we need to modify the pixel_bvb_clk value. So
> add register to control this clock.
> 
> Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>

Reviewed-by: Maxime Ripard <mripard@kernel.org>

Can you merge this patch through the clk tree?

The rest will go through drm

Thanks!
Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 1/4] clk: bcm: rpi: Add register to control pixel bvb clk

[Nicolas Saenz Julienne]

[-- Attachment #1: Type: text/plain, Size: 1026 bytes --]

On Tue, 2020-09-01 at 13:07 +0900, Hoegeun Kwon wrote:
> To use QHD or higher, we need to modify the pixel_bvb_clk value. So
> add register to control this clock.
> 
> Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
> ---
>  drivers/clk/bcm/clk-raspberrypi.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk-raspberrypi.c
> index 5cc82954e1ce..f89b9cfc4309 100644
> --- a/drivers/clk/bcm/clk-raspberrypi.c
> +++ b/drivers/clk/bcm/clk-raspberrypi.c
> @@ -271,6 +271,7 @@ static int raspberrypi_discover_clocks(struct raspberrypi_clk *rpi,
>  		case RPI_FIRMWARE_CORE_CLK_ID:
>  		case RPI_FIRMWARE_M2MC_CLK_ID:
>  		case RPI_FIRMWARE_V3D_CLK_ID:
> +		case RPI_FIRMWARE_PIXEL_BVB_CLK_ID:
>  			hw = raspberrypi_clk_register(rpi, clks->parent,
>  						      clks->id);
>  			if (IS_ERR(hw))

Acked-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
Tested-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>

Regards,
Nicolas


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 1/4] clk: bcm: rpi: Add register to control pixel bvb clk

[Stephen Boyd]

Quoting Hoegeun Kwon (2020-08-31 21:07:56)
> To use QHD or higher, we need to modify the pixel_bvb_clk value. So
> add register to control this clock.
> 
> Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
> ---

Applied to clk-next

[PATCH v2 2/4] drm/vc4: hdmi: Add pixel bvb clock control

[Hoegeun Kwon]

There is a problem that the output does not work at a resolution
exceeding FHD. To solve this, we need to adjust the bvb clock at a
resolution exceeding FHD.

Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
---
 drivers/gpu/drm/vc4/vc4_hdmi.c | 25 +++++++++++++++++++++++++
 drivers/gpu/drm/vc4/vc4_hdmi.h |  1 +
 2 files changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 95ec5eedea39..eb3192d1fd86 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -80,6 +80,7 @@
 # define VC4_HD_M_ENABLE			BIT(0)
 
 #define CEC_CLOCK_FREQ 40000
+#define VC4_HSM_MID_CLOCK 149985000
 
 static int vc4_hdmi_debugfs_regs(struct seq_file *m, void *unused)
 {
@@ -380,6 +381,7 @@ static void vc4_hdmi_encoder_post_crtc_powerdown(struct drm_encoder *encoder)
 	HDMI_WRITE(HDMI_VID_CTL,
 		   HDMI_READ(HDMI_VID_CTL) & ~VC4_HD_VID_CTL_ENABLE);
 
+	clk_disable_unprepare(vc4_hdmi->pixel_bvb_clock);
 	clk_disable_unprepare(vc4_hdmi->hsm_clock);
 	clk_disable_unprepare(vc4_hdmi->pixel_clock);
 
@@ -638,6 +640,23 @@ static void vc4_hdmi_encoder_pre_crtc_configure(struct drm_encoder *encoder)
 		return;
 	}
 
+	ret = clk_set_rate(vc4_hdmi->pixel_bvb_clock,
+			(hsm_rate > VC4_HSM_MID_CLOCK ? 150000000 : 75000000));
+	if (ret) {
+		DRM_ERROR("Failed to set pixel bvb clock rate: %d\n", ret);
+		clk_disable_unprepare(vc4_hdmi->hsm_clock);
+		clk_disable_unprepare(vc4_hdmi->pixel_clock);
+		return;
+	}
+
+	ret = clk_prepare_enable(vc4_hdmi->pixel_bvb_clock);
+	if (ret) {
+		DRM_ERROR("Failed to turn on pixel bvb clock: %d\n", ret);
+		clk_disable_unprepare(vc4_hdmi->hsm_clock);
+		clk_disable_unprepare(vc4_hdmi->pixel_clock);
+		return;
+	}
+
 	if (vc4_hdmi->variant->reset)
 		vc4_hdmi->variant->reset(vc4_hdmi);
 
@@ -1593,6 +1612,12 @@ static int vc5_hdmi_init_resources(struct vc4_hdmi *vc4_hdmi)
 		return PTR_ERR(vc4_hdmi->audio_clock);
 	}
 
+	vc4_hdmi->pixel_bvb_clock = devm_clk_get(dev, "bvb");
+	if (IS_ERR(vc4_hdmi->pixel_bvb_clock)) {
+		DRM_ERROR("Failed to get pixel bvb clock\n");
+		return PTR_ERR(vc4_hdmi->pixel_bvb_clock);
+	}
+
 	vc4_hdmi->reset = devm_reset_control_get(dev, NULL);
 	if (IS_ERR(vc4_hdmi->reset)) {
 		DRM_ERROR("Failed to get HDMI reset line\n");
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h
index 0806c6d9f24e..63c6f8bddf1d 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.h
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.h
@@ -147,6 +147,7 @@ struct vc4_hdmi {
 	struct clk *pixel_clock;
 	struct clk *hsm_clock;
 	struct clk *audio_clock;
+	struct clk *pixel_bvb_clock;
 
 	struct reset_control *reset;
 
-- 
2.17.1

Re: [PATCH v2 2/4] drm/vc4: hdmi: Add pixel bvb clock control

[Chanwoo Choi]

Hi Hoegeun,

It looks good to me. But, just one comment.

On 9/1/20 1:07 PM, Hoegeun Kwon wrote:
> There is a problem that the output does not work at a resolution
> exceeding FHD. To solve this, we need to adjust the bvb clock at a
> resolution exceeding FHD.
> 
> Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
> ---
>  drivers/gpu/drm/vc4/vc4_hdmi.c | 25 +++++++++++++++++++++++++
>  drivers/gpu/drm/vc4/vc4_hdmi.h |  1 +
>  2 files changed, 26 insertions(+)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index 95ec5eedea39..eb3192d1fd86 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -80,6 +80,7 @@
>  # define VC4_HD_M_ENABLE			BIT(0)
>  
>  #define CEC_CLOCK_FREQ 40000
> +#define VC4_HSM_MID_CLOCK 149985000
>  
>  static int vc4_hdmi_debugfs_regs(struct seq_file *m, void *unused)
>  {
> @@ -380,6 +381,7 @@ static void vc4_hdmi_encoder_post_crtc_powerdown(struct drm_encoder *encoder)
>  	HDMI_WRITE(HDMI_VID_CTL,
>  		   HDMI_READ(HDMI_VID_CTL) & ~VC4_HD_VID_CTL_ENABLE);
>  
> +	clk_disable_unprepare(vc4_hdmi->pixel_bvb_clock);
>  	clk_disable_unprepare(vc4_hdmi->hsm_clock);
>  	clk_disable_unprepare(vc4_hdmi->pixel_clock);
>  
> @@ -638,6 +640,23 @@ static void vc4_hdmi_encoder_pre_crtc_configure(struct drm_encoder *encoder)
>  		return;
>  	}
>  
> +	ret = clk_set_rate(vc4_hdmi->pixel_bvb_clock,
> +			(hsm_rate > VC4_HSM_MID_CLOCK ? 150000000 : 75000000));
> +	if (ret) {
> +		DRM_ERROR("Failed to set pixel bvb clock rate: %d\n", ret);
> +		clk_disable_unprepare(vc4_hdmi->hsm_clock);
> +		clk_disable_unprepare(vc4_hdmi->pixel_clock);
> +		return;
> +	}
> +
> +	ret = clk_prepare_enable(vc4_hdmi->pixel_bvb_clock);
> +	if (ret) {
> +		DRM_ERROR("Failed to turn on pixel bvb clock: %d\n", ret);
> +		clk_disable_unprepare(vc4_hdmi->hsm_clock);
> +		clk_disable_unprepare(vc4_hdmi->pixel_clock);
> +		return;
> +	}

Generally, enable the clock before using clk and then change the clock rate.
I think that you better to change the order between clk_prepare_enable and clk_set_rate.


> +
>  	if (vc4_hdmi->variant->reset)
>  		vc4_hdmi->variant->reset(vc4_hdmi);
>  
> @@ -1593,6 +1612,12 @@ static int vc5_hdmi_init_resources(struct vc4_hdmi *vc4_hdmi)
>  		return PTR_ERR(vc4_hdmi->audio_clock);
>  	}
>  
> +	vc4_hdmi->pixel_bvb_clock = devm_clk_get(dev, "bvb");
> +	if (IS_ERR(vc4_hdmi->pixel_bvb_clock)) {
> +		DRM_ERROR("Failed to get pixel bvb clock\n");
> +		return PTR_ERR(vc4_hdmi->pixel_bvb_clock);
> +	}
> +
>  	vc4_hdmi->reset = devm_reset_control_get(dev, NULL);
>  	if (IS_ERR(vc4_hdmi->reset)) {
>  		DRM_ERROR("Failed to get HDMI reset line\n");
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h
> index 0806c6d9f24e..63c6f8bddf1d 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.h
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.h
> @@ -147,6 +147,7 @@ struct vc4_hdmi {
>  	struct clk *pixel_clock;
>  	struct clk *hsm_clock;
>  	struct clk *audio_clock;
> +	struct clk *pixel_bvb_clock;
>  
>  	struct reset_control *reset;
>  
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH v2 2/4] drm/vc4: hdmi: Add pixel bvb clock control

[Hoegeun Kwon]

Hi Chanwoo,

On 9/1/20 1:27 PM, Chanwoo Choi wrote:
> Hi Hoegeun,
>
> It looks good to me. But, just one comment.
>
> On 9/1/20 1:07 PM, Hoegeun Kwon wrote:
>> There is a problem that the output does not work at a resolution
>> exceeding FHD. To solve this, we need to adjust the bvb clock at a
>> resolution exceeding FHD.
>>
>> Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
>> ---
>>   drivers/gpu/drm/vc4/vc4_hdmi.c | 25 +++++++++++++++++++++++++
>>   drivers/gpu/drm/vc4/vc4_hdmi.h |  1 +
>>   2 files changed, 26 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
>> index 95ec5eedea39..eb3192d1fd86 100644
>> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
>> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
>> @@ -80,6 +80,7 @@
>>   # define VC4_HD_M_ENABLE			BIT(0)
>>   
>>   #define CEC_CLOCK_FREQ 40000
>> +#define VC4_HSM_MID_CLOCK 149985000
>>   
>>   static int vc4_hdmi_debugfs_regs(struct seq_file *m, void *unused)
>>   {
>> @@ -380,6 +381,7 @@ static void vc4_hdmi_encoder_post_crtc_powerdown(struct drm_encoder *encoder)
>>   	HDMI_WRITE(HDMI_VID_CTL,
>>   		   HDMI_READ(HDMI_VID_CTL) & ~VC4_HD_VID_CTL_ENABLE);
>>   
>> +	clk_disable_unprepare(vc4_hdmi->pixel_bvb_clock);
>>   	clk_disable_unprepare(vc4_hdmi->hsm_clock);
>>   	clk_disable_unprepare(vc4_hdmi->pixel_clock);
>>   
>> @@ -638,6 +640,23 @@ static void vc4_hdmi_encoder_pre_crtc_configure(struct drm_encoder *encoder)
>>   		return;
>>   	}
>>   
>> +	ret = clk_set_rate(vc4_hdmi->pixel_bvb_clock,
>> +			(hsm_rate > VC4_HSM_MID_CLOCK ? 150000000 : 75000000));
>> +	if (ret) {
>> +		DRM_ERROR("Failed to set pixel bvb clock rate: %d\n", ret);
>> +		clk_disable_unprepare(vc4_hdmi->hsm_clock);
>> +		clk_disable_unprepare(vc4_hdmi->pixel_clock);
>> +		return;
>> +	}
>> +
>> +	ret = clk_prepare_enable(vc4_hdmi->pixel_bvb_clock);
>> +	if (ret) {
>> +		DRM_ERROR("Failed to turn on pixel bvb clock: %d\n", ret);
>> +		clk_disable_unprepare(vc4_hdmi->hsm_clock);
>> +		clk_disable_unprepare(vc4_hdmi->pixel_clock);
>> +		return;
>> +	}
> Generally, enable the clock before using clk and then change the clock rate.
> I think that you better to change the order between clk_prepare_enable and clk_set_rate.

Thank you for your comment.


As Maxime answered in another patch [1], there is no clear rule of order 
here.

[1] https://lkml.org/lkml/2020/9/1/327


Best regards,

Hoegeun

[PATCH v2 3/4] dt-bindings: display: vc4: hdmi: Add bvb clock-names property

[Hoegeun Kwon]

When using a resolution exceeding FHD, bvb clock is required.
Add bvb clock-names property.

Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
---
 .../bindings/display/brcm,bcm2711-hdmi.yaml          | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml b/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml
index 6091fe3d315b..08cdcc579cf5 100644
--- a/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml
+++ b/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml
@@ -40,10 +40,14 @@ properties:
       - const: hd
 
   clocks:
-    description: The HDMI state machine clock
+    items:
+      - description: The HDMI state machine clock
+      - description: The bvb clock
 
   clock-names:
-    const: hdmi
+    items:
+      - const: hdmi
+      - const: bvb
 
   ddc:
     allOf:
@@ -100,8 +104,8 @@ examples:
                     "csc",
                     "cec",
                     "hd";
-        clocks = <&firmware_clocks 13>;
-        clock-names = "hdmi";
+        clocks = <&firmware_clocks 13>, <&firmware_clocks 14>;
+        clock-names = "hdmi", "bvb";
         resets = <&dvp 0>;
         ddc = <&ddc0>;
     };
-- 
2.17.1

[PATCH v2 4/4] ARM: dts: bcm2711: Add bvb clock for hdmi-pixel

[Hoegeun Kwon]

It is necessary to control the hdmi pixel bvb clock. Add bvb clock.

Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
---
 arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
index b93eb30e1ddb..90dee4cb38bc 100644
--- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
+++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
@@ -172,12 +172,14 @@
 };
 
 &hdmi0 {
-	clocks = <&firmware_clocks 13>, <&dvp 0>;
+	clocks = <&firmware_clocks 13>, <&dvp 0>, <&firmware_clocks 14>;
+	clock-names = "hdmi", "clk-108M", "bvb";
 	status = "okay";
 };
 
 &hdmi1 {
-	clocks = <&firmware_clocks 13>, <&dvp 1>;
+	clocks = <&firmware_clocks 13>, <&dvp 1>, <&firmware_clocks 14>;
+	clock-names = "hdmi", "clk-108M", "bvb";
 	status = "okay";
 };
 
-- 
2.17.1

Re: [PATCH v2 0/4] drm/vc4: Support HDMI QHD or higher output

[Nicolas Saenz Julienne]

[-- Attachment #1: Type: text/plain, Size: 1277 bytes --]

On Tue, 2020-09-01 at 13:07 +0900, Hoegeun Kwon wrote:
> Hi everyone,
> 
> There is a problem that the output does not work at a resolution
> exceeding FHD. To solve this, we need to adjust the bvb clock at a
> resolution exceeding FHD.
> 
> Rebased on top of next-20200708 and [1].
> 
> [1] : [PATCH v4 00/78] drm/vc4: Support BCM2711 Display Pipeline (Maxime's patchset)
> 
> Changes from v1:
>   - Added dt-bindings documents
>   - Change patch order, first fix driver and then device tree
> 
> Hoegeun Kwon (4):
>   clk: bcm: rpi: Add register to control pixel bvb clk
>   drm/vc4: hdmi: Add pixel bvb clock control
>   dt-bindings: display: vc4: hdmi: Add bvb clock-names property
>   ARM: dts: bcm2711: Add bvb clock for hdmi-pixel
> 
>  .../bindings/display/brcm,bcm2711-hdmi.yaml   | 12 ++++++---
>  arch/arm/boot/dts/bcm2711-rpi-4-b.dts         |  6 +++--
>  drivers/clk/bcm/clk-raspberrypi.c             |  1 +
>  drivers/gpu/drm/vc4/vc4_hdmi.c                | 25 +++++++++++++++++++
>  drivers/gpu/drm/vc4/vc4_hdmi.h                |  1 +
>  5 files changed, 39 insertions(+), 6 deletions(-)

Small note to anyone reviewing this, patches 3 & 4 where squashed into this
series: https://lkml.org/lkml/2020/9/3/219

Regards,
Nicolas


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]