Hi,
Here is a series that enables the higher resolutions on the HDMI0 Controller found in the BCM2711 (RPi4).
In order to work it needs a few adjustments to config.txt, most notably to enable the enable_hdmi_4kp60 option.
The firmware also has a glitch at the moment and will not properly release the BSC controllers, which will make the EDID retrieval fail.
We can work around this using the following config.txt options:
disable_fw_kms_setup=1 hdmi_edid_file:0=1 hdmi_edid_filename:0=1366x768.bin hdmi_ignore_edid:0=1 hdmi_edid_file:1=1 hdmi_edid_filename:1=1366x768.bin hdmi_ignore_edid:1=1
A fix will come for the firmware eventually.
Let me know what you think, Maxime
---
Changes from v1: - Dropped the range accessors - Drop the mention of force_turbo - Reordered the SCRAMBLER_CTL register to match the offset - Removed duplicate HDMI_14_MAX_TMDS_CLK define - Warn about enable_hdmi_4kp60 only if there's some modes that can't be reached - Rework the BVB clock computation
Maxime Ripard (5): drm/vc4: hvs: Make the HVS bind first drm/vc4: hdmi: Properly compute the BVB clock rate drm/vc4: hdmi: Check and warn if we can't reach 4kp60 frequencies drm/vc4: hdmi: Enable the scrambler drm/vc4: hdmi: Raise the maximum clock rate
drivers/gpu/drm/vc4/vc4_drv.c | 11 ++- drivers/gpu/drm/vc4/vc4_hdmi.c | 101 +++++++++++++++++++++++++--- drivers/gpu/drm/vc4/vc4_hdmi.h | 8 +++ drivers/gpu/drm/vc4/vc4_hdmi_regs.h | 3 + 4 files changed, 113 insertions(+), 10 deletions(-)
We'll need to have the HVS binding before the HDMI controllers so that we can check whether the firmware allows to run in 4kp60. Reorder a bit the component list, and document the current constraints we're aware of.
Acked-by: Dave Stevenson dave.stevenson@raspberrypi.com Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_drv.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c index 556ad0f02a0d..0310590ee66e 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.c +++ b/drivers/gpu/drm/vc4/vc4_drv.c @@ -303,12 +303,21 @@ static const struct component_master_ops vc4_drm_ops = { .unbind = vc4_drm_unbind, };
+/* + * This list determines the binding order of our components, and we have + * a few constraints: + * - The TXP driver needs to be bound before the PixelValves (CRTC) + * but after the HVS to set the possible_crtc field properly + * - The HDMI driver needs to be bound after the HVS so that we can + * lookup the HVS maximum core clock rate and figure out if we + * support 4kp60 or not. + */ static struct platform_driver *const component_drivers[] = { + &vc4_hvs_driver, &vc4_hdmi_driver, &vc4_vec_driver, &vc4_dpi_driver, &vc4_dsi_driver, - &vc4_hvs_driver, &vc4_txp_driver, &vc4_crtc_driver, &vc4_v3d_driver,
The BVB clock rate computation doesn't take into account a mode clock of 594MHz that we're going to need to support 4k60.
Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index eee9751009c2..eaee853bb404 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -91,7 +91,6 @@ # define VC4_HD_M_ENABLE BIT(0)
#define CEC_CLOCK_FREQ 40000 -#define VC4_HSM_MID_CLOCK 149985000
#define HDMI_14_MAX_TMDS_CLK (340 * 1000 * 1000)
@@ -739,7 +738,7 @@ static void vc4_hdmi_encoder_pre_crtc_configure(struct drm_encoder *encoder, conn_state_to_vc4_hdmi_conn_state(conn_state); struct drm_display_mode *mode = &encoder->crtc->state->adjusted_mode; struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder); - unsigned long pixel_rate, hsm_rate; + unsigned long bvb_rate, pixel_rate, hsm_rate; int ret;
ret = pm_runtime_get_sync(&vc4_hdmi->pdev->dev); @@ -793,12 +792,14 @@ static void vc4_hdmi_encoder_pre_crtc_configure(struct drm_encoder *encoder,
vc4_hdmi_cec_update_clk_div(vc4_hdmi);
- /* - * FIXME: When the pixel freq is 594MHz (4k60), this needs to be setup - * at 300MHz. - */ - ret = clk_set_min_rate(vc4_hdmi->pixel_bvb_clock, - (hsm_rate > VC4_HSM_MID_CLOCK ? 150000000 : 75000000)); + if (pixel_rate > 297000000) + bvb_rate = 300000000; + else if (pixel_rate > 148500000) + bvb_rate = 150000000; + else + bvb_rate = 75000000; + + ret = clk_set_min_rate(vc4_hdmi->pixel_bvb_clock, bvb_rate); if (ret) { DRM_ERROR("Failed to set pixel bvb clock rate: %d\n", ret); clk_disable_unprepare(vc4_hdmi->hsm_clock);
Hi Maxime
Sorry for the delay in reviewing these patches.
On Thu, 18 Mar 2021 at 09:29, Maxime Ripard maxime@cerno.tech wrote:
The BVB clock rate computation doesn't take into account a mode clock of 594MHz that we're going to need to support 4k60.
Signed-off-by: Maxime Ripard maxime@cerno.tech
Reviewed-by: Dave Stevenson dave.stevenson@raspberrypi.com
drivers/gpu/drm/vc4/vc4_hdmi.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index eee9751009c2..eaee853bb404 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -91,7 +91,6 @@ # define VC4_HD_M_ENABLE BIT(0)
#define CEC_CLOCK_FREQ 40000 -#define VC4_HSM_MID_CLOCK 149985000
#define HDMI_14_MAX_TMDS_CLK (340 * 1000 * 1000)
@@ -739,7 +738,7 @@ static void vc4_hdmi_encoder_pre_crtc_configure(struct drm_encoder *encoder, conn_state_to_vc4_hdmi_conn_state(conn_state); struct drm_display_mode *mode = &encoder->crtc->state->adjusted_mode; struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
unsigned long pixel_rate, hsm_rate;
unsigned long bvb_rate, pixel_rate, hsm_rate; int ret; ret = pm_runtime_get_sync(&vc4_hdmi->pdev->dev);@@ -793,12 +792,14 @@ static void vc4_hdmi_encoder_pre_crtc_configure(struct drm_encoder *encoder,
vc4_hdmi_cec_update_clk_div(vc4_hdmi);
/** FIXME: When the pixel freq is 594MHz (4k60), this needs to be setup* at 300MHz.*/ret = clk_set_min_rate(vc4_hdmi->pixel_bvb_clock,(hsm_rate > VC4_HSM_MID_CLOCK ? 150000000 : 75000000));
if (pixel_rate > 297000000)bvb_rate = 300000000;else if (pixel_rate > 148500000)bvb_rate = 150000000;elsebvb_rate = 75000000;ret = clk_set_min_rate(vc4_hdmi->pixel_bvb_clock, bvb_rate); if (ret) { DRM_ERROR("Failed to set pixel bvb clock rate: %d\n", ret); clk_disable_unprepare(vc4_hdmi->hsm_clock);-- 2.30.2
In order to reach the frequencies needed to output at 594MHz, the firmware needs to be configured with the appropriate parameters in the config.txt file (enable_hdmi_4kp60 and force_turbo).
Let's detect it at bind time, warn the user if we can't, and filter out the relevant modes.
Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 26 ++++++++++++++++++++++++++ drivers/gpu/drm/vc4/vc4_hdmi.h | 8 ++++++++ 2 files changed, 34 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index eaee853bb404..0924a1b9e186 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -210,6 +210,18 @@ 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) { + struct drm_device *drm = connector->dev; + struct drm_display_mode *mode; + + list_for_each_entry(mode, &connector->probed_modes, head) { + if ((mode->clock * 1000) > HDMI_14_MAX_TMDS_CLK) { + drm_warn_once(drm, "The core clock cannot reach frequencies high enough to support 4k @ 60Hz."); + drm_warn_once(drm, "Please change your config.txt file to add hdmi_enable_4kp60."); + } + } + } + return ret; }
@@ -959,6 +971,9 @@ static int vc4_hdmi_encoder_atomic_check(struct drm_encoder *encoder, if (pixel_rate > vc4_hdmi->variant->max_pixel_clock) return -EINVAL;
+ if (vc4_hdmi->disable_4kp60 && (pixel_rate > HDMI_14_MAX_TMDS_CLK)) + return -EINVAL; + vc4_state->pixel_rate = pixel_rate;
return 0; @@ -978,6 +993,9 @@ vc4_hdmi_encoder_mode_valid(struct drm_encoder *encoder, if ((mode->clock * 1000) > vc4_hdmi->variant->max_pixel_clock) return MODE_CLOCK_HIGH;
+ if (vc4_hdmi->disable_4kp60 && ((mode->clock * 1000) > HDMI_14_MAX_TMDS_CLK)) + return MODE_CLOCK_HIGH; + return MODE_OK; }
@@ -1992,6 +2010,14 @@ 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); + long max_rate = clk_round_rate(vc4->hvs->core_clk, 550000000); + + if (max_rate < 550000000) + vc4_hdmi->disable_4kp60 = true; + } + if (vc4_hdmi->variant->reset) vc4_hdmi->variant->reset(vc4_hdmi);
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h index 3cebd1fd00fc..3cd021136402 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.h +++ b/drivers/gpu/drm/vc4/vc4_hdmi.h @@ -154,6 +154,14 @@ 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;
Hi Maxime
On Thu, 18 Mar 2021 at 09:29, Maxime Ripard maxime@cerno.tech wrote:
In order to reach the frequencies needed to output at 594MHz, the firmware needs to be configured with the appropriate parameters in the config.txt file (enable_hdmi_4kp60 and force_turbo).
Let's detect it at bind time, warn the user if we can't, and filter out the relevant modes.
Signed-off-by: Maxime Ripard maxime@cerno.tech
Reviewed-by: Dave Stevenson dave.stevenson@raspberrypi.com
drivers/gpu/drm/vc4/vc4_hdmi.c | 26 ++++++++++++++++++++++++++ drivers/gpu/drm/vc4/vc4_hdmi.h | 8 ++++++++ 2 files changed, 34 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index eaee853bb404..0924a1b9e186 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -210,6 +210,18 @@ 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) {struct drm_device *drm = connector->dev;struct drm_display_mode *mode;list_for_each_entry(mode, &connector->probed_modes, head) {if ((mode->clock * 1000) > HDMI_14_MAX_TMDS_CLK) {drm_warn_once(drm, "The core clock cannot reach frequencies high enough to support 4k @ 60Hz.");drm_warn_once(drm, "Please change your config.txt file to add hdmi_enable_4kp60.");}}}return ret;}
@@ -959,6 +971,9 @@ static int vc4_hdmi_encoder_atomic_check(struct drm_encoder *encoder, if (pixel_rate > vc4_hdmi->variant->max_pixel_clock) return -EINVAL;
if (vc4_hdmi->disable_4kp60 && (pixel_rate > HDMI_14_MAX_TMDS_CLK))return -EINVAL;vc4_state->pixel_rate = pixel_rate; return 0;@@ -978,6 +993,9 @@ vc4_hdmi_encoder_mode_valid(struct drm_encoder *encoder, if ((mode->clock * 1000) > vc4_hdmi->variant->max_pixel_clock) return MODE_CLOCK_HIGH;
if (vc4_hdmi->disable_4kp60 && ((mode->clock * 1000) > HDMI_14_MAX_TMDS_CLK))return MODE_CLOCK_HIGH;return MODE_OK;}
@@ -1992,6 +2010,14 @@ 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);long max_rate = clk_round_rate(vc4->hvs->core_clk, 550000000);if (max_rate < 550000000)vc4_hdmi->disable_4kp60 = true;}if (vc4_hdmi->variant->reset) vc4_hdmi->variant->reset(vc4_hdmi);diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h index 3cebd1fd00fc..3cd021136402 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.h +++ b/drivers/gpu/drm/vc4/vc4_hdmi.h @@ -154,6 +154,14 @@ 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;-- 2.30.2
The HDMI controller on the BCM2711 includes a scrambler in order to reach the modes that require it. Let's add the support for it.
Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 56 +++++++++++++++++++++++++++++ drivers/gpu/drm/vc4/vc4_hdmi_regs.h | 3 ++ 2 files changed, 59 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 0924a1b9e186..530c83097b1a 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -35,6 +35,7 @@ #include <drm/drm_edid.h> #include <drm/drm_probe_helper.h> #include <drm/drm_simple_kms_helper.h> +#include <drm/drm_scdc_helper.h> #include <linux/clk.h> #include <linux/component.h> #include <linux/i2c.h> @@ -76,6 +77,8 @@ #define VC5_HDMI_VERTB_VSPO_SHIFT 16 #define VC5_HDMI_VERTB_VSPO_MASK VC4_MASK(29, 16)
+#define VC5_HDMI_SCRAMBLER_CTL_ENABLE BIT(0) + #define VC5_HDMI_DEEP_COLOR_CONFIG_1_INIT_PACK_PHASE_SHIFT 8 #define VC5_HDMI_DEEP_COLOR_CONFIG_1_INIT_PACK_PHASE_MASK VC4_MASK(10, 8)
@@ -457,6 +460,56 @@ static void vc4_hdmi_set_infoframes(struct drm_encoder *encoder) vc4_hdmi_set_audio_infoframe(encoder); }
+static bool vc4_hdmi_supports_scrambling(struct drm_encoder *encoder, + struct drm_display_mode *mode) +{ + struct vc4_hdmi_encoder *vc4_encoder = to_vc4_hdmi_encoder(encoder); + struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder); + struct drm_display_info *display = &vc4_hdmi->connector.display_info; + + if (!vc4_encoder->hdmi_monitor) + return false; + + if (!display->hdmi.scdc.supported || + !display->hdmi.scdc.scrambling.supported) + return false; + + if ((mode->clock * 1000) < HDMI_14_MAX_TMDS_CLK) + return false; + + return true; +} + +static void vc4_hdmi_enable_scrambling(struct drm_encoder *encoder) +{ + struct drm_display_mode *mode = &encoder->crtc->state->adjusted_mode; + struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder); + + if (!vc4_hdmi_supports_scrambling(encoder, mode)) + return; + + drm_scdc_set_high_tmds_clock_ratio(vc4_hdmi->ddc, true); + drm_scdc_set_scrambling(vc4_hdmi->ddc, true); + + HDMI_WRITE(HDMI_SCRAMBLER_CTL, HDMI_READ(HDMI_SCRAMBLER_CTL) | + VC5_HDMI_SCRAMBLER_CTL_ENABLE); +} + +static void vc4_hdmi_disable_scrambling(struct drm_encoder *encoder) +{ + struct drm_display_mode *mode = &encoder->crtc->mode; + struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder); + + if (!vc4_hdmi_supports_scrambling(encoder, mode)) + return; + + HDMI_WRITE(HDMI_SCRAMBLER_CTL, HDMI_READ(HDMI_SCRAMBLER_CTL) & + ~VC5_HDMI_SCRAMBLER_CTL_ENABLE); + + drm_scdc_set_scrambling(vc4_hdmi->ddc, false); + drm_scdc_set_high_tmds_clock_ratio(vc4_hdmi->ddc, false); +} + static void vc4_hdmi_encoder_post_crtc_disable(struct drm_encoder *encoder, struct drm_atomic_state *state) { @@ -469,6 +522,8 @@ static void vc4_hdmi_encoder_post_crtc_disable(struct drm_encoder *encoder,
HDMI_WRITE(HDMI_VID_CTL, HDMI_READ(HDMI_VID_CTL) | VC4_HD_VID_CTL_BLANKPIX); + + vc4_hdmi_disable_scrambling(encoder); }
static void vc4_hdmi_encoder_post_crtc_powerdown(struct drm_encoder *encoder, @@ -919,6 +974,7 @@ static void vc4_hdmi_encoder_post_crtc_enable(struct drm_encoder *encoder, }
vc4_hdmi_recenter_fifo(vc4_hdmi); + vc4_hdmi_enable_scrambling(encoder); }
static void vc4_hdmi_encoder_enable(struct drm_encoder *encoder) diff --git a/drivers/gpu/drm/vc4/vc4_hdmi_regs.h b/drivers/gpu/drm/vc4/vc4_hdmi_regs.h index e1b58eac766f..19d2fdc446bc 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi_regs.h +++ b/drivers/gpu/drm/vc4/vc4_hdmi_regs.h @@ -100,6 +100,7 @@ enum vc4_hdmi_field { HDMI_RM_FORMAT, HDMI_RM_OFFSET, HDMI_SCHEDULER_CONTROL, + HDMI_SCRAMBLER_CTL, HDMI_SW_RESET_CONTROL, HDMI_TX_PHY_CHANNEL_SWAP, HDMI_TX_PHY_CLK_DIV, @@ -238,6 +239,7 @@ static const struct vc4_hdmi_register __maybe_unused vc5_hdmi_hdmi0_fields[] = { VC4_HDMI_REG(HDMI_GCP_CONFIG, 0x178), VC4_HDMI_REG(HDMI_GCP_WORD_1, 0x17c), VC4_HDMI_REG(HDMI_HOTPLUG, 0x1a8), + VC4_HDMI_REG(HDMI_SCRAMBLER_CTL, 0x1c4),
VC5_DVP_REG(HDMI_CLOCK_STOP, 0x0bc), VC5_DVP_REG(HDMI_VEC_INTERFACE_XBAR, 0x0f0), @@ -317,6 +319,7 @@ static const struct vc4_hdmi_register __maybe_unused vc5_hdmi_hdmi1_fields[] = { VC4_HDMI_REG(HDMI_GCP_CONFIG, 0x178), VC4_HDMI_REG(HDMI_GCP_WORD_1, 0x17c), VC4_HDMI_REG(HDMI_HOTPLUG, 0x1a8), + VC4_HDMI_REG(HDMI_SCRAMBLER_CTL, 0x1c4),
VC5_DVP_REG(HDMI_CLOCK_STOP, 0x0bc), VC5_DVP_REG(HDMI_VEC_INTERFACE_XBAR, 0x0f0),
Hi Maxime
On Thu, 18 Mar 2021 at 09:29, Maxime Ripard maxime@cerno.tech wrote:
The HDMI controller on the BCM2711 includes a scrambler in order to reach the modes that require it. Let's add the support for it.
Nit pick - it's as part of the HDMI2.0 spec.
Signed-off-by: Maxime Ripard maxime@cerno.tech
drivers/gpu/drm/vc4/vc4_hdmi.c | 56 +++++++++++++++++++++++++++++ drivers/gpu/drm/vc4/vc4_hdmi_regs.h | 3 ++ 2 files changed, 59 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 0924a1b9e186..530c83097b1a 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -35,6 +35,7 @@ #include <drm/drm_edid.h> #include <drm/drm_probe_helper.h> #include <drm/drm_simple_kms_helper.h> +#include <drm/drm_scdc_helper.h> #include <linux/clk.h> #include <linux/component.h> #include <linux/i2c.h> @@ -76,6 +77,8 @@ #define VC5_HDMI_VERTB_VSPO_SHIFT 16 #define VC5_HDMI_VERTB_VSPO_MASK VC4_MASK(29, 16)
+#define VC5_HDMI_SCRAMBLER_CTL_ENABLE BIT(0)
#define VC5_HDMI_DEEP_COLOR_CONFIG_1_INIT_PACK_PHASE_SHIFT 8 #define VC5_HDMI_DEEP_COLOR_CONFIG_1_INIT_PACK_PHASE_MASK VC4_MASK(10, 8)
@@ -457,6 +460,56 @@ static void vc4_hdmi_set_infoframes(struct drm_encoder *encoder) vc4_hdmi_set_audio_infoframe(encoder); }
+static bool vc4_hdmi_supports_scrambling(struct drm_encoder *encoder,
struct drm_display_mode *mode)+{
struct vc4_hdmi_encoder *vc4_encoder = to_vc4_hdmi_encoder(encoder);struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);struct drm_display_info *display = &vc4_hdmi->connector.display_info;if (!vc4_encoder->hdmi_monitor)return false;if (!display->hdmi.scdc.supported ||!display->hdmi.scdc.scrambling.supported)return false;
I think I made this comment last time, but possibly not very clearly.
Up to this point in the function is whether the display/hdmi interface *supports* scrambling. The bit after is whether it is *required* to be enabled by the mode.
At the moment, if the display/interface supports scrambling but the mode doesn't need it, then the scrambling setup is never touched. That makes a few assumptions about the default settings. Boot with the firmware selecting 4k60 (ie scrambling on), but video=HDMI-1:1920x1080 in the kernel command line and you'll have a mess with scrambling enabled but not signalled.
I'd be happier if the display/interface says scrambling is supported then we always call drm_scdc_set_high_tmds_clock_ratio, drm_scdc_set_scrambling and set the VC5_HDMI_SCRAMBLER_CTL_ENABLE register bit appropriately for the mode. Feel free to disagree with me though.
Dave
if ((mode->clock * 1000) < HDMI_14_MAX_TMDS_CLK)return false;return true;+}
+static void vc4_hdmi_enable_scrambling(struct drm_encoder *encoder) +{
struct drm_display_mode *mode = &encoder->crtc->state->adjusted_mode;struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);if (!vc4_hdmi_supports_scrambling(encoder, mode))return;drm_scdc_set_high_tmds_clock_ratio(vc4_hdmi->ddc, true);drm_scdc_set_scrambling(vc4_hdmi->ddc, true);HDMI_WRITE(HDMI_SCRAMBLER_CTL, HDMI_READ(HDMI_SCRAMBLER_CTL) |VC5_HDMI_SCRAMBLER_CTL_ENABLE);+}
+static void vc4_hdmi_disable_scrambling(struct drm_encoder *encoder) +{
struct drm_display_mode *mode = &encoder->crtc->mode;struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);if (!vc4_hdmi_supports_scrambling(encoder, mode))return;HDMI_WRITE(HDMI_SCRAMBLER_CTL, HDMI_READ(HDMI_SCRAMBLER_CTL) &~VC5_HDMI_SCRAMBLER_CTL_ENABLE);drm_scdc_set_scrambling(vc4_hdmi->ddc, false);drm_scdc_set_high_tmds_clock_ratio(vc4_hdmi->ddc, false);+}
static void vc4_hdmi_encoder_post_crtc_disable(struct drm_encoder *encoder, struct drm_atomic_state *state) { @@ -469,6 +522,8 @@ static void vc4_hdmi_encoder_post_crtc_disable(struct drm_encoder *encoder,
HDMI_WRITE(HDMI_VID_CTL, HDMI_READ(HDMI_VID_CTL) | VC4_HD_VID_CTL_BLANKPIX);
vc4_hdmi_disable_scrambling(encoder);}
static void vc4_hdmi_encoder_post_crtc_powerdown(struct drm_encoder *encoder, @@ -919,6 +974,7 @@ static void vc4_hdmi_encoder_post_crtc_enable(struct drm_encoder *encoder, }
vc4_hdmi_recenter_fifo(vc4_hdmi);
vc4_hdmi_enable_scrambling(encoder);}
static void vc4_hdmi_encoder_enable(struct drm_encoder *encoder) diff --git a/drivers/gpu/drm/vc4/vc4_hdmi_regs.h b/drivers/gpu/drm/vc4/vc4_hdmi_regs.h index e1b58eac766f..19d2fdc446bc 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi_regs.h +++ b/drivers/gpu/drm/vc4/vc4_hdmi_regs.h @@ -100,6 +100,7 @@ enum vc4_hdmi_field { HDMI_RM_FORMAT, HDMI_RM_OFFSET, HDMI_SCHEDULER_CONTROL,
HDMI_SCRAMBLER_CTL, HDMI_SW_RESET_CONTROL, HDMI_TX_PHY_CHANNEL_SWAP, HDMI_TX_PHY_CLK_DIV,@@ -238,6 +239,7 @@ static const struct vc4_hdmi_register __maybe_unused vc5_hdmi_hdmi0_fields[] = { VC4_HDMI_REG(HDMI_GCP_CONFIG, 0x178), VC4_HDMI_REG(HDMI_GCP_WORD_1, 0x17c), VC4_HDMI_REG(HDMI_HOTPLUG, 0x1a8),
VC4_HDMI_REG(HDMI_SCRAMBLER_CTL, 0x1c4), VC5_DVP_REG(HDMI_CLOCK_STOP, 0x0bc), VC5_DVP_REG(HDMI_VEC_INTERFACE_XBAR, 0x0f0),@@ -317,6 +319,7 @@ static const struct vc4_hdmi_register __maybe_unused vc5_hdmi_hdmi1_fields[] = { VC4_HDMI_REG(HDMI_GCP_CONFIG, 0x178), VC4_HDMI_REG(HDMI_GCP_WORD_1, 0x17c), VC4_HDMI_REG(HDMI_HOTPLUG, 0x1a8),
VC4_HDMI_REG(HDMI_SCRAMBLER_CTL, 0x1c4), VC5_DVP_REG(HDMI_CLOCK_STOP, 0x0bc), VC5_DVP_REG(HDMI_VEC_INTERFACE_XBAR, 0x0f0),-- 2.30.2
Hi Dave,
On Thu, Apr 01, 2021 at 12:30:45PM +0100, Dave Stevenson wrote:
Signed-off-by: Maxime Ripard maxime@cerno.tech
drivers/gpu/drm/vc4/vc4_hdmi.c | 56 +++++++++++++++++++++++++++++ drivers/gpu/drm/vc4/vc4_hdmi_regs.h | 3 ++ 2 files changed, 59 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 0924a1b9e186..530c83097b1a 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -35,6 +35,7 @@ #include <drm/drm_edid.h> #include <drm/drm_probe_helper.h> #include <drm/drm_simple_kms_helper.h> +#include <drm/drm_scdc_helper.h> #include <linux/clk.h> #include <linux/component.h> #include <linux/i2c.h> @@ -76,6 +77,8 @@ #define VC5_HDMI_VERTB_VSPO_SHIFT 16 #define VC5_HDMI_VERTB_VSPO_MASK VC4_MASK(29, 16)
+#define VC5_HDMI_SCRAMBLER_CTL_ENABLE BIT(0)
#define VC5_HDMI_DEEP_COLOR_CONFIG_1_INIT_PACK_PHASE_SHIFT 8 #define VC5_HDMI_DEEP_COLOR_CONFIG_1_INIT_PACK_PHASE_MASK VC4_MASK(10, 8)
@@ -457,6 +460,56 @@ static void vc4_hdmi_set_infoframes(struct drm_encoder *encoder) vc4_hdmi_set_audio_infoframe(encoder); }
+static bool vc4_hdmi_supports_scrambling(struct drm_encoder *encoder,
struct drm_display_mode *mode)+{
struct vc4_hdmi_encoder *vc4_encoder = to_vc4_hdmi_encoder(encoder);struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);struct drm_display_info *display = &vc4_hdmi->connector.display_info;if (!vc4_encoder->hdmi_monitor)return false;if (!display->hdmi.scdc.supported ||!display->hdmi.scdc.scrambling.supported)return false;I think I made this comment last time, but possibly not very clearly.
I must have missed it then, sorry :/
Up to this point in the function is whether the display/hdmi interface *supports* scrambling. The bit after is whether it is *required* to be enabled by the mode.
Thomas made a suggestion to move the mode clock check to a helper, so we'll end up with essentially a helper about whether we support scrambling or not and if the mode requires it.
At the moment, if the display/interface supports scrambling but the mode doesn't need it, then the scrambling setup is never touched. That makes a few assumptions about the default settings. Boot with the firmware selecting 4k60 (ie scrambling on), but video=HDMI-1:1920x1080 in the kernel command line and you'll have a mess with scrambling enabled but not signalled.
I'd be happier if the display/interface says scrambling is supported then we always call drm_scdc_set_high_tmds_clock_ratio, drm_scdc_set_scrambling and set the VC5_HDMI_SCRAMBLER_CTL_ENABLE register bit appropriately for the mode. Feel free to disagree with me though.
I think part of it is due to our custom helpers never being called currently during the boot process. Once that is fixed, the disable helpers will be called and will disable the scrambling so we should be good there.
This creates another issue though. That function takes the mode as the argument and at boot time the mode pointer will be null. I think we can work around it by assuming that we need to disable it at boot all the time (and thus ignore the test if our pointer is null).
Would that work for you?
Maxime
Hi Maxime
On Fri, 9 Apr 2021 at 13:36, Maxime Ripard maxime@cerno.tech wrote:
Hi Dave,
On Thu, Apr 01, 2021 at 12:30:45PM +0100, Dave Stevenson wrote:
Signed-off-by: Maxime Ripard maxime@cerno.tech
drivers/gpu/drm/vc4/vc4_hdmi.c | 56 +++++++++++++++++++++++++++++ drivers/gpu/drm/vc4/vc4_hdmi_regs.h | 3 ++ 2 files changed, 59 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 0924a1b9e186..530c83097b1a 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -35,6 +35,7 @@ #include <drm/drm_edid.h> #include <drm/drm_probe_helper.h> #include <drm/drm_simple_kms_helper.h> +#include <drm/drm_scdc_helper.h> #include <linux/clk.h> #include <linux/component.h> #include <linux/i2c.h> @@ -76,6 +77,8 @@ #define VC5_HDMI_VERTB_VSPO_SHIFT 16 #define VC5_HDMI_VERTB_VSPO_MASK VC4_MASK(29, 16)
+#define VC5_HDMI_SCRAMBLER_CTL_ENABLE BIT(0)
#define VC5_HDMI_DEEP_COLOR_CONFIG_1_INIT_PACK_PHASE_SHIFT 8 #define VC5_HDMI_DEEP_COLOR_CONFIG_1_INIT_PACK_PHASE_MASK VC4_MASK(10, 8)
@@ -457,6 +460,56 @@ static void vc4_hdmi_set_infoframes(struct drm_encoder *encoder) vc4_hdmi_set_audio_infoframe(encoder); }
+static bool vc4_hdmi_supports_scrambling(struct drm_encoder *encoder,
struct drm_display_mode *mode)+{
struct vc4_hdmi_encoder *vc4_encoder = to_vc4_hdmi_encoder(encoder);struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);struct drm_display_info *display = &vc4_hdmi->connector.display_info;if (!vc4_encoder->hdmi_monitor)return false;if (!display->hdmi.scdc.supported ||!display->hdmi.scdc.scrambling.supported)return false;I think I made this comment last time, but possibly not very clearly.
I must have missed it then, sorry :/
Up to this point in the function is whether the display/hdmi interface *supports* scrambling. The bit after is whether it is *required* to be enabled by the mode.
Thomas made a suggestion to move the mode clock check to a helper, so we'll end up with essentially a helper about whether we support scrambling or not and if the mode requires it.
At the moment, if the display/interface supports scrambling but the mode doesn't need it, then the scrambling setup is never touched. That makes a few assumptions about the default settings. Boot with the firmware selecting 4k60 (ie scrambling on), but video=HDMI-1:1920x1080 in the kernel command line and you'll have a mess with scrambling enabled but not signalled.
I'd be happier if the display/interface says scrambling is supported then we always call drm_scdc_set_high_tmds_clock_ratio, drm_scdc_set_scrambling and set the VC5_HDMI_SCRAMBLER_CTL_ENABLE register bit appropriately for the mode. Feel free to disagree with me though.
I think part of it is due to our custom helpers never being called currently during the boot process. Once that is fixed, the disable helpers will be called and will disable the scrambling so we should be good there.
This creates another issue though. That function takes the mode as the argument and at boot time the mode pointer will be null. I think we can work around it by assuming that we need to disable it at boot all the time (and thus ignore the test if our pointer is null).
Would that work for you?
Yes, that all sounds reasonable to me.
Dave
Now that we have the infrastructure in place, we can raise the maximum pixel rate we can reach for HDMI0 on the BCM2711.
HDMI1 is left untouched since its pixelvalve has a smaller FIFO and would need a clock faster than what we can provide to support the same modes.
Reviewed-by: Dave Stevenson dave.stevenson@raspberrypi.com Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 530c83097b1a..a31771dd09e6 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -2190,7 +2190,7 @@ static const struct vc4_hdmi_variant bcm2711_hdmi0_variant = { .encoder_type = VC4_ENCODER_TYPE_HDMI0, .debugfs_name = "hdmi0_regs", .card_name = "vc4-hdmi-0", - .max_pixel_clock = HDMI_14_MAX_TMDS_CLK, + .max_pixel_clock = 600000000, .registers = vc5_hdmi_hdmi0_fields, .num_registers = ARRAY_SIZE(vc5_hdmi_hdmi0_fields), .phy_lane_mapping = {
Hi,
I looked through the patches. I can't say much about the technical side, but for the patches themselves
Acked-by: Thomas Zimmermann tzimmermann@suse.de
Just one nit: I've seen the pattern
(mode->clock * 1000) < HDMI_14_MAX_TMDS_CLK
in several places. I suggest to add a little helper with a descriptive name.
Best regards Thomas
Am 18.03.21 um 10:29 schrieb Maxime Ripard:
Hi,
Here is a series that enables the higher resolutions on the HDMI0 Controller found in the BCM2711 (RPi4).
In order to work it needs a few adjustments to config.txt, most notably to enable the enable_hdmi_4kp60 option.
The firmware also has a glitch at the moment and will not properly release the BSC controllers, which will make the EDID retrieval fail.
We can work around this using the following config.txt options:
disable_fw_kms_setup=1 hdmi_edid_file:0=1 hdmi_edid_filename:0=1366x768.bin hdmi_ignore_edid:0=1 hdmi_edid_file:1=1 hdmi_edid_filename:1=1366x768.bin hdmi_ignore_edid:1=1
A fix will come for the firmware eventually.
Let me know what you think, Maxime
Changes from v1:
- Dropped the range accessors
- Drop the mention of force_turbo
- Reordered the SCRAMBLER_CTL register to match the offset
- Removed duplicate HDMI_14_MAX_TMDS_CLK define
- Warn about enable_hdmi_4kp60 only if there's some modes that can't be reached
- Rework the BVB clock computation
Maxime Ripard (5): drm/vc4: hvs: Make the HVS bind first drm/vc4: hdmi: Properly compute the BVB clock rate drm/vc4: hdmi: Check and warn if we can't reach 4kp60 frequencies drm/vc4: hdmi: Enable the scrambler drm/vc4: hdmi: Raise the maximum clock rate
drivers/gpu/drm/vc4/vc4_drv.c | 11 ++- drivers/gpu/drm/vc4/vc4_hdmi.c | 101 +++++++++++++++++++++++++--- drivers/gpu/drm/vc4/vc4_hdmi.h | 8 +++ drivers/gpu/drm/vc4/vc4_hdmi_regs.h | 3 + 4 files changed, 113 insertions(+), 10 deletions(-)
dri-devel@lists.freedesktop.org
-
Dave Stevenson -
Maxime Ripard -
Thomas Zimmermann