2022-10-13 09:24:39

by Maxime Ripard

[permalink] [raw]
Subject: [PATCH v3 0/7] drm/vc4: Fix the core clock behaviour

Hi,

Those patches used to be part of a larger clock fixes series:
https://lore.kernel.org/linux-clk/[email protected]/

However, that series doesn't seem to be getting anywhere, so I've split out
these patches that fix a regression that has been there since 5.18 and that
prevents the 4k output from working on the RaspberryPi4.

Hopefully, we will be able to merge those patches through the DRM tree to avoid
any further disruption.

Let me know what you think,
Maxime

To: Florian Fainelli <[email protected]>
To: Broadcom internal kernel review list <[email protected]>
To: Ray Jui <[email protected]>
To: Scott Branden <[email protected]>
To: Michael Turquette <[email protected]>
To: Stephen Boyd <[email protected]>
To: Emma Anholt <[email protected]>
To: Maxime Ripard <[email protected]>
To: David Airlie <[email protected]>
To: Daniel Vetter <[email protected]>
Cc: Stefan Wahren <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: Dom Cobley <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>

---
Changes in v3:
- Return UINT_MAX when the firmware call fails in the _get_max_rate function
- Link to v2: https://lore.kernel.org/r/[email protected]

Changes in v2:
- Dropped the clock patches, made an ad-hoc function in the firmware driver
- Link to v1: https://lore.kernel.org/r/[email protected]

---
Dom Cobley (1):
drm/vc4: hdmi: Add more checks for 4k resolutions

Maxime Ripard (6):
firmware: raspberrypi: Introduce rpi_firmware_find_node()
firmware: raspberrypi: Move the clock IDs to the firmware header
firmware: raspberrypi: Provide a helper to query a clock max rate
drm/vc4: hdmi: Fix hdmi_enable_4kp60 detection
drm/vc4: hdmi: Rework hdmi_enable_4kp60 detection code
drm/vc4: Make sure we don't end up with a core clock too high

drivers/clk/bcm/clk-raspberrypi.c | 18 -----------
drivers/firmware/raspberrypi.c | 27 ++++++++++++++++
drivers/gpu/drm/vc4/vc4_drv.h | 16 ++++++++++
drivers/gpu/drm/vc4/vc4_hdmi.c | 25 ++++++++-------
drivers/gpu/drm/vc4/vc4_hdmi.h | 8 -----
drivers/gpu/drm/vc4/vc4_hvs.c | 26 +++++++++++++++
drivers/gpu/drm/vc4/vc4_kms.c | 13 +++++---
include/soc/bcm2835/raspberrypi-firmware.h | 51 ++++++++++++++++++++++++++++++
8 files changed, 141 insertions(+), 43 deletions(-)
---
base-commit: 4fe89d07dcc2804c8b562f6c7896a45643d34b2f
change-id: 20220815-rpi-fix-4k-60-17273650429d

Best regards,
--
Maxime Ripard <[email protected]>


2022-10-13 09:26:31

by Maxime Ripard

[permalink] [raw]
Subject: [PATCH v3 5/7] drm/vc4: hdmi: Rework hdmi_enable_4kp60 detection code

In order to support higher HDMI frequencies, users have to set the
hdmi_enable_4kp60 parameter in their config.txt file.

This will have the side-effect of raising the maximum of the core clock,
tied to the HVS, and managed by the HVS driver.

However, we are querying this in the HDMI driver by poking into the HVS
structure to get our struct clk handle.

Let's make this part of the HVS bind implementation to have all the core
clock related setup in the same place.

Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/vc4/vc4_drv.h | 10 ++++++++++
drivers/gpu/drm/vc4/vc4_hdmi.c | 15 ++++-----------
drivers/gpu/drm/vc4/vc4_hdmi.h | 8 --------
drivers/gpu/drm/vc4/vc4_hvs.c | 23 +++++++++++++++++++++++
4 files changed, 37 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index 1beb96b77b8c..09cdbdb7fff0 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -328,6 +328,8 @@ struct vc4_hvs {

struct clk *core_clk;

+ unsigned long max_core_rate;
+
/* Memory manager for CRTCs to allocate space in the display
* list. Units are dwords.
*/
@@ -339,6 +341,14 @@ struct vc4_hvs {
struct drm_mm_node mitchell_netravali_filter;

struct debugfs_regset32 regset;
+
+ /*
+ * Even if HDMI0 on the RPi4 can output modes requiring a pixel
+ * rate higher than 297MHz, it needs some adjustments in the
+ * config.txt file to be able to do so and thus won't always be
+ * available.
+ */
+ bool vc5_hdmi_enable_scrambling;
};

struct vc4_plane {
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 3b75ac6fa0db..f367f93ca832 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -46,7 +46,6 @@
#include <linux/pm_runtime.h>
#include <linux/rational.h>
#include <linux/reset.h>
-#include <soc/bcm2835/raspberrypi-clocks.h>
#include <sound/dmaengine_pcm.h>
#include <sound/hdmi-codec.h>
#include <sound/pcm_drm_eld.h>
@@ -277,6 +276,7 @@ static void vc4_hdmi_connector_destroy(struct drm_connector *connector)
static int vc4_hdmi_connector_get_modes(struct drm_connector *connector)
{
struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
+ struct vc4_dev *vc4 = to_vc4_dev(connector->dev);
int ret = 0;
struct edid *edid;

@@ -293,7 +293,7 @@ static int vc4_hdmi_connector_get_modes(struct drm_connector *connector)
ret = drm_add_edid_modes(connector, edid);
kfree(edid);

- if (vc4_hdmi->disable_4kp60) {
+ if (!vc4->hvs->vc5_hdmi_enable_scrambling) {
struct drm_device *drm = connector->dev;
struct drm_display_mode *mode;

@@ -1480,11 +1480,12 @@ vc4_hdmi_encoder_clock_valid(const struct vc4_hdmi *vc4_hdmi,
{
const struct drm_connector *connector = &vc4_hdmi->connector;
const struct drm_display_info *info = &connector->display_info;
+ struct vc4_dev *vc4 = to_vc4_dev(connector->dev);

if (clock > vc4_hdmi->variant->max_pixel_clock)
return MODE_CLOCK_HIGH;

- if (vc4_hdmi->disable_4kp60 && clock > HDMI_14_MAX_TMDS_CLK)
+ if (!vc4->hvs->vc5_hdmi_enable_scrambling && clock > HDMI_14_MAX_TMDS_CLK)
return MODE_CLOCK_HIGH;

if (info->max_tmds_clock && clock > (info->max_tmds_clock * 1000))
@@ -2965,14 +2966,6 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data)
vc4_hdmi->disable_wifi_frequencies =
of_property_read_bool(dev->of_node, "wifi-2.4ghz-coexistence");

- if (variant->max_pixel_clock == 600000000) {
- struct vc4_dev *vc4 = to_vc4_dev(drm);
- unsigned long max_rate = rpi_firmware_clk_get_max_rate(vc4->hvs->core_clk);
-
- if (max_rate < 550000000)
- vc4_hdmi->disable_4kp60 = true;
- }
-
pm_runtime_enable(dev);

/*
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h
index c3ed2b07df23..7506943050cf 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.h
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.h
@@ -155,14 +155,6 @@ struct vc4_hdmi {
*/
bool disable_wifi_frequencies;

- /*
- * Even if HDMI0 on the RPi4 can output modes requiring a pixel
- * rate higher than 297MHz, it needs some adjustments in the
- * config.txt file to be able to do so and thus won't always be
- * available.
- */
- bool disable_4kp60;
-
struct cec_adapter *cec_adap;
struct cec_msg cec_rx_msg;
bool cec_tx_ok;
diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c
index fbaa741dda5f..e28a13a75ec2 100644
--- a/drivers/gpu/drm/vc4/vc4_hvs.c
+++ b/drivers/gpu/drm/vc4/vc4_hvs.c
@@ -27,6 +27,8 @@
#include <drm/drm_atomic_helper.h>
#include <drm/drm_vblank.h>

+#include <soc/bcm2835/raspberrypi-firmware.h>
+
#include "vc4_drv.h"
#include "vc4_regs.h"

@@ -671,12 +673,33 @@ static int vc4_hvs_bind(struct device *dev, struct device *master, void *data)
hvs->regset.nregs = ARRAY_SIZE(hvs_regs);

if (vc4->is_vc5) {
+ struct rpi_firmware *firmware;
+ struct device_node *node;
+ unsigned long max_rate;
+
+ node = rpi_firmware_find_node();
+ if (!node)
+ return -EINVAL;
+
+ firmware = rpi_firmware_get(node);
+ of_node_put(node);
+ if (!firmware)
+ return -EPROBE_DEFER;
+
hvs->core_clk = devm_clk_get(&pdev->dev, NULL);
if (IS_ERR(hvs->core_clk)) {
dev_err(&pdev->dev, "Couldn't get core clock\n");
return PTR_ERR(hvs->core_clk);
}

+ max_rate = rpi_firmware_clk_get_max_rate(firmware,
+ RPI_FIRMWARE_CORE_CLK_ID);
+ rpi_firmware_put(firmware);
+ if (max_rate >= 550000000)
+ hvs->vc5_hdmi_enable_scrambling = true;
+
+ hvs->max_core_rate = max_rate;
+
ret = clk_prepare_enable(hvs->core_clk);
if (ret) {
dev_err(&pdev->dev, "Couldn't enable the core clock\n");

--
b4 0.11.0-dev-7da52

2022-10-13 09:28:07

by Maxime Ripard

[permalink] [raw]
Subject: [PATCH v3 4/7] drm/vc4: hdmi: Fix hdmi_enable_4kp60 detection

In order to support higher HDMI frequencies, users have to set the
hdmi_enable_4kp60 parameter in their config.txt file.

We were detecting this so far by calling clk_round_rate() on the core
clock with the frequency we're supposed to run at when one of those
modes is enabled. Whether or not the parameter was enabled could then be
inferred by the returned rate since the maximum clock rate reported by
the firmware was one of the side effect of setting that parameter.

However, the recent clock rework we did changed what clk_round_rate()
was returning to always return the minimum allowed, and thus this test
wasn't reliable anymore.

Let's use the new clk_get_max_rate() function to reliably determine the
maximum rate allowed on that clock and fix the 4k@60Hz output.

Fixes: e9d6cea2af1c ("clk: bcm: rpi: Run some clocks at the minimum rate allowed")
Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 1e5f68704d7d..3b75ac6fa0db 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -46,6 +46,7 @@
#include <linux/pm_runtime.h>
#include <linux/rational.h>
#include <linux/reset.h>
+#include <soc/bcm2835/raspberrypi-clocks.h>
#include <sound/dmaengine_pcm.h>
#include <sound/hdmi-codec.h>
#include <sound/pcm_drm_eld.h>
@@ -2966,7 +2967,7 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data)

if (variant->max_pixel_clock == 600000000) {
struct vc4_dev *vc4 = to_vc4_dev(drm);
- long max_rate = clk_round_rate(vc4->hvs->core_clk, 550000000);
+ unsigned long max_rate = rpi_firmware_clk_get_max_rate(vc4->hvs->core_clk);

if (max_rate < 550000000)
vc4_hdmi->disable_4kp60 = true;

--
b4 0.11.0-dev-7da52

2022-10-13 09:36:59

by Maxime Ripard

[permalink] [raw]
Subject: [PATCH v3 2/7] firmware: raspberrypi: Move the clock IDs to the firmware header

We'll need the clock IDs in more drivers than just the clock driver from
now on, so let's move them in the firmware header.

Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/clk/bcm/clk-raspberrypi.c | 18 ------------------
include/soc/bcm2835/raspberrypi-firmware.h | 18 ++++++++++++++++++
2 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk-raspberrypi.c
index 876b37b8683c..1f5e6a1554e6 100644
--- a/drivers/clk/bcm/clk-raspberrypi.c
+++ b/drivers/clk/bcm/clk-raspberrypi.c
@@ -18,24 +18,6 @@

#include <soc/bcm2835/raspberrypi-firmware.h>

-enum rpi_firmware_clk_id {
- RPI_FIRMWARE_EMMC_CLK_ID = 1,
- RPI_FIRMWARE_UART_CLK_ID,
- RPI_FIRMWARE_ARM_CLK_ID,
- RPI_FIRMWARE_CORE_CLK_ID,
- RPI_FIRMWARE_V3D_CLK_ID,
- RPI_FIRMWARE_H264_CLK_ID,
- RPI_FIRMWARE_ISP_CLK_ID,
- RPI_FIRMWARE_SDRAM_CLK_ID,
- RPI_FIRMWARE_PIXEL_CLK_ID,
- RPI_FIRMWARE_PWM_CLK_ID,
- RPI_FIRMWARE_HEVC_CLK_ID,
- RPI_FIRMWARE_EMMC2_CLK_ID,
- RPI_FIRMWARE_M2MC_CLK_ID,
- RPI_FIRMWARE_PIXEL_BVB_CLK_ID,
- RPI_FIRMWARE_NUM_CLK_ID,
-};
-
static char *rpi_firmware_clk_names[] = {
[RPI_FIRMWARE_EMMC_CLK_ID] = "emmc",
[RPI_FIRMWARE_UART_CLK_ID] = "uart",
diff --git a/include/soc/bcm2835/raspberrypi-firmware.h b/include/soc/bcm2835/raspberrypi-firmware.h
index 63426082bcb9..74c7bcc1ac2a 100644
--- a/include/soc/bcm2835/raspberrypi-firmware.h
+++ b/include/soc/bcm2835/raspberrypi-firmware.h
@@ -136,6 +136,24 @@ enum rpi_firmware_property_tag {
RPI_FIRMWARE_GET_DMA_CHANNELS = 0x00060001,
};

+enum rpi_firmware_clk_id {
+ RPI_FIRMWARE_EMMC_CLK_ID = 1,
+ RPI_FIRMWARE_UART_CLK_ID,
+ RPI_FIRMWARE_ARM_CLK_ID,
+ RPI_FIRMWARE_CORE_CLK_ID,
+ RPI_FIRMWARE_V3D_CLK_ID,
+ RPI_FIRMWARE_H264_CLK_ID,
+ RPI_FIRMWARE_ISP_CLK_ID,
+ RPI_FIRMWARE_SDRAM_CLK_ID,
+ RPI_FIRMWARE_PIXEL_CLK_ID,
+ RPI_FIRMWARE_PWM_CLK_ID,
+ RPI_FIRMWARE_HEVC_CLK_ID,
+ RPI_FIRMWARE_EMMC2_CLK_ID,
+ RPI_FIRMWARE_M2MC_CLK_ID,
+ RPI_FIRMWARE_PIXEL_BVB_CLK_ID,
+ RPI_FIRMWARE_NUM_CLK_ID,
+};
+
#if IS_ENABLED(CONFIG_RASPBERRYPI_FIRMWARE)
int rpi_firmware_property(struct rpi_firmware *fw,
u32 tag, void *data, size_t len);

--
b4 0.11.0-dev-7da52

2022-10-13 09:38:06

by Maxime Ripard

[permalink] [raw]
Subject: [PATCH v3 6/7] drm/vc4: hdmi: Add more checks for 4k resolutions

From: Dom Cobley <[email protected]>

At least the 4096x2160@60Hz mode requires some overclocking that isn't
available by default, even if hdmi_enable_4kp60 is enabled.

Let's add some logic to detect whether we can satisfy the core clock
requirements for that mode, and prevent it from being used otherwise.

Signed-off-by: Dom Cobley <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/vc4/vc4_drv.h | 6 ++++++
drivers/gpu/drm/vc4/vc4_hdmi.c | 11 +++++++++--
drivers/gpu/drm/vc4/vc4_hvs.c | 3 +++
3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index 09cdbdb7fff0..094ebe8567e2 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -349,6 +349,12 @@ struct vc4_hvs {
* available.
*/
bool vc5_hdmi_enable_scrambling;
+
+ /*
+ * 4096x2160@60 requires a core overclock to work, so register
+ * whether that is sufficient.
+ */
+ bool vc5_hdmi_enable_4096by2160;
};

struct vc4_plane {
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index f367f93ca832..cf1fee6c29f3 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -1476,6 +1476,7 @@ vc4_hdmi_sink_supports_format_bpc(const struct vc4_hdmi *vc4_hdmi,

static enum drm_mode_status
vc4_hdmi_encoder_clock_valid(const struct vc4_hdmi *vc4_hdmi,
+ const struct drm_display_mode *mode,
unsigned long long clock)
{
const struct drm_connector *connector = &vc4_hdmi->connector;
@@ -1488,6 +1489,12 @@ vc4_hdmi_encoder_clock_valid(const struct vc4_hdmi *vc4_hdmi,
if (!vc4->hvs->vc5_hdmi_enable_scrambling && clock > HDMI_14_MAX_TMDS_CLK)
return MODE_CLOCK_HIGH;

+ /* 4096x2160@60 is not reliable without overclocking core */
+ if (!vc4->hvs->vc5_hdmi_enable_4096by2160 &&
+ mode->hdisplay > 3840 && mode->vdisplay >= 2160 &&
+ drm_mode_vrefresh(mode) >= 50)
+ return MODE_CLOCK_HIGH;
+
if (info->max_tmds_clock && clock > (info->max_tmds_clock * 1000))
return MODE_CLOCK_HIGH;

@@ -1522,7 +1529,7 @@ vc4_hdmi_encoder_compute_clock(const struct vc4_hdmi *vc4_hdmi,
unsigned long long clock;

clock = vc4_hdmi_encoder_compute_mode_clock(mode, bpc, fmt);
- if (vc4_hdmi_encoder_clock_valid(vc4_hdmi, clock) != MODE_OK)
+ if (vc4_hdmi_encoder_clock_valid(vc4_hdmi, mode, clock) != MODE_OK)
return -EINVAL;

vc4_state->tmds_char_rate = clock;
@@ -1685,7 +1692,7 @@ vc4_hdmi_encoder_mode_valid(struct drm_encoder *encoder,
(mode->hsync_end % 2) || (mode->htotal % 2)))
return MODE_H_ILLEGAL;

- return vc4_hdmi_encoder_clock_valid(vc4_hdmi, mode->clock * 1000);
+ return vc4_hdmi_encoder_clock_valid(vc4_hdmi, mode, mode->clock * 1000);
}

static const struct drm_encoder_helper_funcs vc4_hdmi_encoder_helper_funcs = {
diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c
index e28a13a75ec2..32f5ab937ace 100644
--- a/drivers/gpu/drm/vc4/vc4_hvs.c
+++ b/drivers/gpu/drm/vc4/vc4_hvs.c
@@ -698,6 +698,9 @@ static int vc4_hvs_bind(struct device *dev, struct device *master, void *data)
if (max_rate >= 550000000)
hvs->vc5_hdmi_enable_scrambling = true;

+ if (max_rate >= 600000000)
+ hvs->vc5_hdmi_enable_4096by2160 = true;
+
hvs->max_core_rate = max_rate;

ret = clk_prepare_enable(hvs->core_clk);

--
b4 0.11.0-dev-7da52

2022-10-14 19:45:20

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH v3 2/7] firmware: raspberrypi: Move the clock IDs to the firmware header

On 10/13/22 02:13, Maxime Ripard wrote:
> We'll need the clock IDs in more drivers than just the clock driver from
> now on, so let's move them in the firmware header.
>
> Signed-off-by: Maxime Ripard <[email protected]>

Reviewed-by: Florian Fainelli <[email protected]>
--
Florian

2022-10-14 19:54:28

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v3 2/7] firmware: raspberrypi: Move the clock IDs to the firmware header

Quoting Maxime Ripard (2022-10-13 02:13:09)
> We'll need the clock IDs in more drivers than just the clock driver from
> now on, so let's move them in the firmware header.
>
> Signed-off-by: Maxime Ripard <[email protected]>
> ---

Acked-by: Stephen Boyd <[email protected]>