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.
Let me know what you think,
Maxime
---
Changes from v3:
- Rework the encoder retrieval code that was broken on the RPi3 and older
- Fix a scrambling enabling issue on some display
Changes from v2:
- Gathered the various tags
- Added Cc stable when relevant
- Split out the check to test whether the scrambler is required into
an helper
- Fixed a bug where the scrambler state wouldn't be tracked properly
if it was enabled at boot
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 (12):
drm/vc4: txp: Properly set the possible_crtcs mask
drm/vc4: crtc: Skip the TXP
drm/vc4: crtc: Pass the drm_atomic_state to config_pv
drm/vc4: crtc: Fix vc4_get_crtc_encoder logic
drm/vc4: crtc: Lookup the encoder from the register at boot
drm/vc4: hdmi: Prevent clock unbalance
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: Add a workqueue to set scrambling
drm/vc4: hdmi: Raise the maximum clock rate
drivers/gpu/drm/vc4/vc4_crtc.c | 66 ++++++++++---
drivers/gpu/drm/vc4/vc4_drv.c | 11 ++-
drivers/gpu/drm/vc4/vc4_hdmi.c | 147 ++++++++++++++++++++++++++--
drivers/gpu/drm/vc4/vc4_hdmi.h | 10 ++
drivers/gpu/drm/vc4/vc4_hdmi_regs.h | 3 +
drivers/gpu/drm/vc4/vc4_txp.c | 2 +-
6 files changed, 217 insertions(+), 22 deletions(-)
--
2.31.1
The current code does a binary OR on the possible_crtcs variable of the
TXP encoder, while we want to set it to that value instead.
Cc: <[email protected]> # v5.9+
Fixes: 39fcb2808376 ("drm/vc4: txp: Turn the TXP into a CRTC of its own")
Acked-by: Thomas Zimmermann <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/vc4/vc4_txp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vc4/vc4_txp.c b/drivers/gpu/drm/vc4/vc4_txp.c
index c0122d83b651..2fc7f4b5fa09 100644
--- a/drivers/gpu/drm/vc4/vc4_txp.c
+++ b/drivers/gpu/drm/vc4/vc4_txp.c
@@ -507,7 +507,7 @@ static int vc4_txp_bind(struct device *dev, struct device *master, void *data)
return ret;
encoder = &txp->connector.encoder;
- encoder->possible_crtcs |= drm_crtc_mask(crtc);
+ encoder->possible_crtcs = drm_crtc_mask(crtc);
ret = devm_request_irq(dev, irq, vc4_txp_interrupt, 0,
dev_name(dev), txp);
--
2.31.1
The vc4_set_crtc_possible_masks is meant to run over all the encoders
and then set their possible_crtcs mask to their associated pixelvalve.
However, since the commit 39fcb2808376 ("drm/vc4: txp: Turn the TXP into
a CRTC of its own"), the TXP has been turned to a CRTC and encoder of
its own, and while it does indeed register an encoder, it no longer has
an associated pixelvalve. The code will thus run over the TXP encoder
and set a bogus possible_crtcs mask, overriding the one set in the TXP
bind function.
In order to fix this, let's skip any virtual encoder.
Cc: <[email protected]> # v5.9+
Fixes: 39fcb2808376 ("drm/vc4: txp: Turn the TXP into a CRTC of its own")
Acked-by: Thomas Zimmermann <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/vc4/vc4_crtc.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index 269390bc586e..f1f2e8cbce79 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -1018,6 +1018,9 @@ static void vc4_set_crtc_possible_masks(struct drm_device *drm,
struct vc4_encoder *vc4_encoder;
int i;
+ if (encoder->encoder_type == DRM_MODE_ENCODER_VIRTUAL)
+ continue;
+
vc4_encoder = to_vc4_encoder(encoder);
for (i = 0; i < ARRAY_SIZE(pv_data->encoder_types); i++) {
if (vc4_encoder->type == encoder_types[i]) {
--
2.31.1
The vc4_get_crtc_encoder function currently only works when the
connector->state->crtc pointer is set, which is only true when the
connector is currently enabled.
However, we use it as part of the disable path as well, and our lookup
will fail in that case, resulting in it returning a null pointer we
can't act on.
We can access the connector that used to be connected to that crtc
though using the old connector state in the disable path.
Since we want to support both the enable and disable path, we can
support it by passing the state accessor variant as a function pointer,
together with the atomic state.
Fixes: 792c3132bc1b ("drm/vc4: encoder: Add finer-grained encoder callbacks")
Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/vc4/vc4_crtc.c | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index 8a19d6c76605..36ea684a349b 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -262,14 +262,22 @@ static u32 vc4_crtc_get_fifo_full_level_bits(struct vc4_crtc *vc4_crtc,
* allows drivers to push pixels to more than one encoder from the
* same CRTC.
*/
-static struct drm_encoder *vc4_get_crtc_encoder(struct drm_crtc *crtc)
+static struct drm_encoder *vc4_get_crtc_encoder(struct drm_crtc *crtc,
+ struct drm_atomic_state *state,
+ struct drm_connector_state *(*get_state)(struct drm_atomic_state *state,
+ struct drm_connector *connector))
{
struct drm_connector *connector;
struct drm_connector_list_iter conn_iter;
drm_connector_list_iter_begin(crtc->dev, &conn_iter);
drm_for_each_connector_iter(connector, &conn_iter) {
- if (connector->state->crtc == crtc) {
+ struct drm_connector_state *conn_state = get_state(state, connector);
+
+ if (!conn_state)
+ continue;
+
+ if (conn_state->crtc == crtc) {
drm_connector_list_iter_end(&conn_iter);
return connector->encoder;
}
@@ -292,7 +300,8 @@ static void vc4_crtc_config_pv(struct drm_crtc *crtc, struct drm_atomic_state *s
{
struct drm_device *dev = crtc->dev;
struct vc4_dev *vc4 = to_vc4_dev(dev);
- struct drm_encoder *encoder = vc4_get_crtc_encoder(crtc);
+ struct drm_encoder *encoder = vc4_get_crtc_encoder(crtc, state,
+ drm_atomic_get_new_connector_state);
struct vc4_encoder *vc4_encoder = to_vc4_encoder(encoder);
struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
const struct vc4_pv_data *pv_data = vc4_crtc_to_vc4_pv_data(vc4_crtc);
@@ -407,7 +416,8 @@ static int vc4_crtc_disable(struct drm_crtc *crtc,
struct drm_atomic_state *state,
unsigned int channel)
{
- struct drm_encoder *encoder = vc4_get_crtc_encoder(crtc);
+ struct drm_encoder *encoder = vc4_get_crtc_encoder(crtc, state,
+ drm_atomic_get_old_connector_state);
struct vc4_encoder *vc4_encoder = to_vc4_encoder(encoder);
struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
struct drm_device *dev = crtc->dev;
@@ -507,7 +517,8 @@ static void vc4_crtc_atomic_enable(struct drm_crtc *crtc,
{
struct drm_device *dev = crtc->dev;
struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
- struct drm_encoder *encoder = vc4_get_crtc_encoder(crtc);
+ struct drm_encoder *encoder = vc4_get_crtc_encoder(crtc, state,
+ drm_atomic_get_new_connector_state);
struct vc4_encoder *vc4_encoder = to_vc4_encoder(encoder);
require_hvs_enabled(dev);
--
2.31.1
The BVB clock rate computation doesn't take into account a mode clock of
594MHz that we're going to need to support 4k60.
Acked-by: Thomas Zimmermann <[email protected]>
Reviewed-by: Dave Stevenson <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
---
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 9c919472ae84..c50dc5a59b2f 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);
--
2.31.1
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 <[email protected]>
Acked-by: Thomas Zimmermann <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
---
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,
--
2.31.1
The vc4_crtc_config_pv will need to access the drm_atomic_state
structure and its only parent function, vc4_crtc_atomic_enable already
has access to it. Let's pass it as a parameter.
Fixes: 792c3132bc1b ("drm/vc4: encoder: Add finer-grained encoder callbacks")
Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/vc4/vc4_crtc.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index f1f2e8cbce79..8a19d6c76605 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -288,7 +288,7 @@ static void vc4_crtc_pixelvalve_reset(struct drm_crtc *crtc)
CRTC_WRITE(PV_CONTROL, CRTC_READ(PV_CONTROL) | PV_CONTROL_FIFO_CLR);
}
-static void vc4_crtc_config_pv(struct drm_crtc *crtc)
+static void vc4_crtc_config_pv(struct drm_crtc *crtc, struct drm_atomic_state *state)
{
struct drm_device *dev = crtc->dev;
struct vc4_dev *vc4 = to_vc4_dev(dev);
@@ -296,8 +296,8 @@ static void vc4_crtc_config_pv(struct drm_crtc *crtc)
struct vc4_encoder *vc4_encoder = to_vc4_encoder(encoder);
struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
const struct vc4_pv_data *pv_data = vc4_crtc_to_vc4_pv_data(vc4_crtc);
- struct drm_crtc_state *state = crtc->state;
- struct drm_display_mode *mode = &state->adjusted_mode;
+ struct drm_crtc_state *crtc_state = crtc->state;
+ struct drm_display_mode *mode = &crtc_state->adjusted_mode;
bool interlace = mode->flags & DRM_MODE_FLAG_INTERLACE;
u32 pixel_rep = (mode->flags & DRM_MODE_FLAG_DBLCLK) ? 2 : 1;
bool is_dsi = (vc4_encoder->type == VC4_ENCODER_TYPE_DSI0 ||
@@ -522,7 +522,7 @@ static void vc4_crtc_atomic_enable(struct drm_crtc *crtc,
if (vc4_encoder->pre_crtc_configure)
vc4_encoder->pre_crtc_configure(encoder, state);
- vc4_crtc_config_pv(crtc);
+ vc4_crtc_config_pv(crtc, state);
CRTC_WRITE(PV_CONTROL, CRTC_READ(PV_CONTROL) | PV_CONTROL_EN);
--
2.31.1
The HDMI controller on the BCM2711 includes a scrambler in order to
reach the HDMI 2.0 modes that require it. Let's add the support for it.
Acked-by: Thomas Zimmermann <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 64 +++++++++++++++++++++++++++++
drivers/gpu/drm/vc4/vc4_hdmi_regs.h | 3 ++
2 files changed, 67 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 01d24ce8a795..bda12fea0dce 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)
@@ -462,6 +465,64 @@ 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;
+
+ 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;
+
+ if (!vc4_hdmi_mode_needs_scrambling(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 vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
+ struct drm_crtc *crtc = encoder->crtc;
+
+ /*
+ * At boot, encoder->crtc will be NULL. Since we don't know the
+ * state of the scrambler and in order to avoid any
+ * inconsistency, let's disable it all the time.
+ */
+ if (crtc && !vc4_hdmi_supports_scrambling(encoder, &crtc->mode))
+ return;
+
+ if (crtc && !vc4_hdmi_mode_needs_scrambling(&crtc->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)
{
@@ -474,6 +535,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,
@@ -924,6 +987,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.31.1
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.
Acked-by: Thomas Zimmermann <[email protected]>
Reviewed-by: Dave Stevenson <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
---
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 4fa7ea419594..0c64de5d60ec 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -2237,7 +2237,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 = {
--
2.31.1
At boot, we can't rely on the vc4_get_crtc_encoder since we don't have a
state yet and thus will not be able to figure out which connector is
attached to our CRTC.
However, we have a muxing bit in the CRTC register we can use to get the
encoder currently connected to the pixelvalve. We can thus read that
register, lookup the associated register through the vc4_pv_data
structure, and then pass it to vc4_crtc_disable so that we can perform
the proper operations.
Fixes: 875a4d536842 ("drm/vc4: drv: Disable the CRTC at boot time")
Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/vc4/vc4_crtc.c | 38 ++++++++++++++++++++++++++++++----
1 file changed, 34 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index 36ea684a349b..f715648f89dd 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -413,11 +413,10 @@ static void require_hvs_enabled(struct drm_device *dev)
}
static int vc4_crtc_disable(struct drm_crtc *crtc,
+ struct drm_encoder *encoder,
struct drm_atomic_state *state,
unsigned int channel)
{
- struct drm_encoder *encoder = vc4_get_crtc_encoder(crtc, state,
- drm_atomic_get_old_connector_state);
struct vc4_encoder *vc4_encoder = to_vc4_encoder(encoder);
struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
struct drm_device *dev = crtc->dev;
@@ -458,10 +457,29 @@ static int vc4_crtc_disable(struct drm_crtc *crtc,
return 0;
}
+static struct drm_encoder *vc4_crtc_get_encoder_by_type(struct drm_crtc *crtc,
+ enum vc4_encoder_type type)
+{
+ struct drm_encoder *encoder;
+
+ drm_for_each_encoder(encoder, crtc->dev) {
+ struct vc4_encoder *vc4_encoder = to_vc4_encoder(encoder);
+
+ if (vc4_encoder->type == type)
+ return encoder;
+ }
+
+ return NULL;
+}
+
int vc4_crtc_disable_at_boot(struct drm_crtc *crtc)
{
struct drm_device *drm = crtc->dev;
struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
+ enum vc4_encoder_type encoder_type;
+ const struct vc4_pv_data *pv_data;
+ struct drm_encoder *encoder;
+ unsigned encoder_sel;
int channel;
if (!(of_device_is_compatible(vc4_crtc->pdev->dev.of_node,
@@ -480,7 +498,17 @@ int vc4_crtc_disable_at_boot(struct drm_crtc *crtc)
if (channel < 0)
return 0;
- return vc4_crtc_disable(crtc, NULL, channel);
+ encoder_sel = VC4_GET_FIELD(CRTC_READ(PV_CONTROL), PV_CONTROL_CLK_SELECT);
+ if (WARN_ON(encoder_sel != 0))
+ return 0;
+
+ pv_data = vc4_crtc_to_vc4_pv_data(vc4_crtc);
+ encoder_type = pv_data->encoder_types[encoder_sel];
+ encoder = vc4_crtc_get_encoder_by_type(crtc, encoder_type);
+ if (WARN_ON(!encoder))
+ return 0;
+
+ return vc4_crtc_disable(crtc, encoder, NULL, channel);
}
static void vc4_crtc_atomic_disable(struct drm_crtc *crtc,
@@ -489,6 +517,8 @@ static void vc4_crtc_atomic_disable(struct drm_crtc *crtc,
struct drm_crtc_state *old_state = drm_atomic_get_old_crtc_state(state,
crtc);
struct vc4_crtc_state *old_vc4_state = to_vc4_crtc_state(old_state);
+ struct drm_encoder *encoder = vc4_get_crtc_encoder(crtc, state,
+ drm_atomic_get_old_connector_state);
struct drm_device *dev = crtc->dev;
require_hvs_enabled(dev);
@@ -496,7 +526,7 @@ static void vc4_crtc_atomic_disable(struct drm_crtc *crtc,
/* Disable vblank irq handling before crtc is disabled. */
drm_crtc_vblank_off(crtc);
- vc4_crtc_disable(crtc, state, old_vc4_state->assigned_channel);
+ vc4_crtc_disable(crtc, encoder, state, old_vc4_state->assigned_channel);
/*
* Make sure we issue a vblank event after disabling the CRTC if
--
2.31.1
It looks like some displays (like the LG 27UL850-W) don't enable the
scrambling when the HDMI driver enables it. However, if we set later the
scrambler enable bit, the display will work as expected.
Let's create delayed work queue to periodically look at the display
scrambling status, and if it's not set yet try to enable it again.
Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 25 +++++++++++++++++++++++++
drivers/gpu/drm/vc4/vc4_hdmi.h | 2 ++
2 files changed, 27 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index bda12fea0dce..4fa7ea419594 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -482,6 +482,8 @@ static bool vc4_hdmi_supports_scrambling(struct drm_encoder *encoder,
return true;
}
+#define SCRAMBLING_POLLING_DELAY_MS 1000
+
static void vc4_hdmi_enable_scrambling(struct drm_encoder *encoder)
{
struct drm_display_mode *mode = &encoder->crtc->state->adjusted_mode;
@@ -498,6 +500,9 @@ static void vc4_hdmi_enable_scrambling(struct drm_encoder *encoder)
HDMI_WRITE(HDMI_SCRAMBLER_CTL, HDMI_READ(HDMI_SCRAMBLER_CTL) |
VC5_HDMI_SCRAMBLER_CTL_ENABLE);
+
+ queue_delayed_work(system_wq, &vc4_hdmi->scrambling_work,
+ msecs_to_jiffies(SCRAMBLING_POLLING_DELAY_MS));
}
static void vc4_hdmi_disable_scrambling(struct drm_encoder *encoder)
@@ -516,6 +521,9 @@ static void vc4_hdmi_disable_scrambling(struct drm_encoder *encoder)
if (crtc && !vc4_hdmi_mode_needs_scrambling(&crtc->mode))
return;
+ if (delayed_work_pending(&vc4_hdmi->scrambling_work))
+ cancel_delayed_work_sync(&vc4_hdmi->scrambling_work);
+
HDMI_WRITE(HDMI_SCRAMBLER_CTL, HDMI_READ(HDMI_SCRAMBLER_CTL) &
~VC5_HDMI_SCRAMBLER_CTL_ENABLE);
@@ -523,6 +531,22 @@ static void vc4_hdmi_disable_scrambling(struct drm_encoder *encoder)
drm_scdc_set_high_tmds_clock_ratio(vc4_hdmi->ddc, false);
}
+static void vc4_hdmi_scrambling_wq(struct work_struct *work)
+{
+ struct vc4_hdmi *vc4_hdmi = container_of(to_delayed_work(work),
+ struct vc4_hdmi,
+ scrambling_work);
+
+ if (drm_scdc_get_scrambling_status(vc4_hdmi->ddc))
+ return;
+
+ drm_scdc_set_high_tmds_clock_ratio(vc4_hdmi->ddc, true);
+ drm_scdc_set_scrambling(vc4_hdmi->ddc, true);
+
+ queue_delayed_work(system_wq, &vc4_hdmi->scrambling_work,
+ msecs_to_jiffies(SCRAMBLING_POLLING_DELAY_MS));
+}
+
static void vc4_hdmi_encoder_post_crtc_disable(struct drm_encoder *encoder,
struct drm_atomic_state *state)
{
@@ -2031,6 +2055,7 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data)
vc4_hdmi = devm_kzalloc(dev, sizeof(*vc4_hdmi), GFP_KERNEL);
if (!vc4_hdmi)
return -ENOMEM;
+ INIT_DELAYED_WORK(&vc4_hdmi->scrambling_work, vc4_hdmi_scrambling_wq);
dev_set_drvdata(dev, vc4_hdmi);
encoder = &vc4_hdmi->encoder.base.base;
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h
index 3cd021136402..00efcf291c5a 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.h
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.h
@@ -126,6 +126,8 @@ struct vc4_hdmi {
struct vc4_hdmi_encoder encoder;
struct drm_connector connector;
+ struct delayed_work scrambling_work;
+
struct i2c_adapter *ddc;
void __iomem *hdmicore_regs;
void __iomem *hd_regs;
--
2.31.1
Since we fixed the hooks to disable the encoder at boot, we now have an
unbalanced clk_disable call at boot since we never enabled them in the
first place.
Let's mimic the state of the hardware and enable the clocks at boot if
the controller is enabled to get the use-count right.
Cc: <[email protected]> # v5.10+
Fixes: 09c438139b8f ("drm/vc4: hdmi: Implement finer-grained hooks")
Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 1fda574579af..9c919472ae84 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -1995,6 +1995,14 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data)
if (vc4_hdmi->variant->reset)
vc4_hdmi->variant->reset(vc4_hdmi);
+ if ((of_device_is_compatible(dev->of_node, "brcm,bcm2711-hdmi0") ||
+ of_device_is_compatible(dev->of_node, "brcm,bcm2711-hdmi1")) &&
+ HDMI_READ(HDMI_VID_CTL) & VC4_HD_VID_CTL_ENABLE) {
+ clk_prepare_enable(vc4_hdmi->pixel_clock);
+ clk_prepare_enable(vc4_hdmi->hsm_clock);
+ clk_prepare_enable(vc4_hdmi->pixel_bvb_clock);
+ }
+
pm_runtime_enable(dev);
drm_simple_encoder_init(drm, encoder, DRM_MODE_ENCODER_TMDS);
--
2.31.1
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.
Acked-by: Thomas Zimmermann <[email protected]>
Reviewed-by: Dave Stevenson <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 31 +++++++++++++++++++++++++++++++
drivers/gpu/drm/vc4/vc4_hdmi.h | 8 ++++++++
2 files changed, 39 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index c50dc5a59b2f..01d24ce8a795 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -94,6 +94,11 @@
#define HDMI_14_MAX_TMDS_CLK (340 * 1000 * 1000)
+static bool vc4_hdmi_mode_needs_scrambling(const struct drm_display_mode *mode)
+{
+ return (mode->clock * 1000) > HDMI_14_MAX_TMDS_CLK;
+}
+
static int vc4_hdmi_debugfs_regs(struct seq_file *m, void *unused)
{
struct drm_info_node *node = (struct drm_info_node *)m->private;
@@ -210,6 +215,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 (vc4_hdmi_mode_needs_scrambling(mode)) {
+ 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 +976,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 +998,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 && vc4_hdmi_mode_needs_scrambling(mode))
+ return MODE_CLOCK_HIGH;
+
return MODE_OK;
}
@@ -1993,6 +2016,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.31.1
Hi Maxime
Thanks for the patch
On Fri, 7 May 2021 at 16:05, Maxime Ripard <[email protected]> wrote:
>
> The vc4_crtc_config_pv will need to access the drm_atomic_state
> structure and its only parent function, vc4_crtc_atomic_enable already
> has access to it. Let's pass it as a parameter.
>
> Fixes: 792c3132bc1b ("drm/vc4: encoder: Add finer-grained encoder callbacks")
> Signed-off-by: Maxime Ripard <[email protected]>
Reviewed-by: Dave Stevenson <[email protected]>
> ---
> drivers/gpu/drm/vc4/vc4_crtc.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
> index f1f2e8cbce79..8a19d6c76605 100644
> --- a/drivers/gpu/drm/vc4/vc4_crtc.c
> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
> @@ -288,7 +288,7 @@ static void vc4_crtc_pixelvalve_reset(struct drm_crtc *crtc)
> CRTC_WRITE(PV_CONTROL, CRTC_READ(PV_CONTROL) | PV_CONTROL_FIFO_CLR);
> }
>
> -static void vc4_crtc_config_pv(struct drm_crtc *crtc)
> +static void vc4_crtc_config_pv(struct drm_crtc *crtc, struct drm_atomic_state *state)
> {
> struct drm_device *dev = crtc->dev;
> struct vc4_dev *vc4 = to_vc4_dev(dev);
> @@ -296,8 +296,8 @@ static void vc4_crtc_config_pv(struct drm_crtc *crtc)
> struct vc4_encoder *vc4_encoder = to_vc4_encoder(encoder);
> struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
> const struct vc4_pv_data *pv_data = vc4_crtc_to_vc4_pv_data(vc4_crtc);
> - struct drm_crtc_state *state = crtc->state;
> - struct drm_display_mode *mode = &state->adjusted_mode;
> + struct drm_crtc_state *crtc_state = crtc->state;
> + struct drm_display_mode *mode = &crtc_state->adjusted_mode;
> bool interlace = mode->flags & DRM_MODE_FLAG_INTERLACE;
> u32 pixel_rep = (mode->flags & DRM_MODE_FLAG_DBLCLK) ? 2 : 1;
> bool is_dsi = (vc4_encoder->type == VC4_ENCODER_TYPE_DSI0 ||
> @@ -522,7 +522,7 @@ static void vc4_crtc_atomic_enable(struct drm_crtc *crtc,
> if (vc4_encoder->pre_crtc_configure)
> vc4_encoder->pre_crtc_configure(encoder, state);
>
> - vc4_crtc_config_pv(crtc);
> + vc4_crtc_config_pv(crtc, state);
>
> CRTC_WRITE(PV_CONTROL, CRTC_READ(PV_CONTROL) | PV_CONTROL_EN);
>
> --
> 2.31.1
>
Hi Maxime
Thanks for the patch.
On Fri, 7 May 2021 at 16:06, Maxime Ripard <[email protected]> wrote:
>
> It looks like some displays (like the LG 27UL850-W) don't enable the
> scrambling when the HDMI driver enables it. However, if we set later the
> scrambler enable bit, the display will work as expected.
>
> Let's create delayed work queue to periodically look at the display
> scrambling status, and if it's not set yet try to enable it again.
>
> Signed-off-by: Maxime Ripard <[email protected]>
Reviewed-by: Dave Stevenson <[email protected]>
> ---
> drivers/gpu/drm/vc4/vc4_hdmi.c | 25 +++++++++++++++++++++++++
> drivers/gpu/drm/vc4/vc4_hdmi.h | 2 ++
> 2 files changed, 27 insertions(+)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index bda12fea0dce..4fa7ea419594 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -482,6 +482,8 @@ static bool vc4_hdmi_supports_scrambling(struct drm_encoder *encoder,
> return true;
> }
>
> +#define SCRAMBLING_POLLING_DELAY_MS 1000
> +
> static void vc4_hdmi_enable_scrambling(struct drm_encoder *encoder)
> {
> struct drm_display_mode *mode = &encoder->crtc->state->adjusted_mode;
> @@ -498,6 +500,9 @@ static void vc4_hdmi_enable_scrambling(struct drm_encoder *encoder)
>
> HDMI_WRITE(HDMI_SCRAMBLER_CTL, HDMI_READ(HDMI_SCRAMBLER_CTL) |
> VC5_HDMI_SCRAMBLER_CTL_ENABLE);
> +
> + queue_delayed_work(system_wq, &vc4_hdmi->scrambling_work,
> + msecs_to_jiffies(SCRAMBLING_POLLING_DELAY_MS));
> }
>
> static void vc4_hdmi_disable_scrambling(struct drm_encoder *encoder)
> @@ -516,6 +521,9 @@ static void vc4_hdmi_disable_scrambling(struct drm_encoder *encoder)
> if (crtc && !vc4_hdmi_mode_needs_scrambling(&crtc->mode))
> return;
>
> + if (delayed_work_pending(&vc4_hdmi->scrambling_work))
> + cancel_delayed_work_sync(&vc4_hdmi->scrambling_work);
> +
> HDMI_WRITE(HDMI_SCRAMBLER_CTL, HDMI_READ(HDMI_SCRAMBLER_CTL) &
> ~VC5_HDMI_SCRAMBLER_CTL_ENABLE);
>
> @@ -523,6 +531,22 @@ static void vc4_hdmi_disable_scrambling(struct drm_encoder *encoder)
> drm_scdc_set_high_tmds_clock_ratio(vc4_hdmi->ddc, false);
> }
>
> +static void vc4_hdmi_scrambling_wq(struct work_struct *work)
> +{
> + struct vc4_hdmi *vc4_hdmi = container_of(to_delayed_work(work),
> + struct vc4_hdmi,
> + scrambling_work);
> +
> + if (drm_scdc_get_scrambling_status(vc4_hdmi->ddc))
> + return;
> +
> + drm_scdc_set_high_tmds_clock_ratio(vc4_hdmi->ddc, true);
> + drm_scdc_set_scrambling(vc4_hdmi->ddc, true);
> +
> + queue_delayed_work(system_wq, &vc4_hdmi->scrambling_work,
> + msecs_to_jiffies(SCRAMBLING_POLLING_DELAY_MS));
> +}
> +
> static void vc4_hdmi_encoder_post_crtc_disable(struct drm_encoder *encoder,
> struct drm_atomic_state *state)
> {
> @@ -2031,6 +2055,7 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data)
> vc4_hdmi = devm_kzalloc(dev, sizeof(*vc4_hdmi), GFP_KERNEL);
> if (!vc4_hdmi)
> return -ENOMEM;
> + INIT_DELAYED_WORK(&vc4_hdmi->scrambling_work, vc4_hdmi_scrambling_wq);
>
> dev_set_drvdata(dev, vc4_hdmi);
> encoder = &vc4_hdmi->encoder.base.base;
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h
> index 3cd021136402..00efcf291c5a 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.h
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.h
> @@ -126,6 +126,8 @@ struct vc4_hdmi {
> struct vc4_hdmi_encoder encoder;
> struct drm_connector connector;
>
> + struct delayed_work scrambling_work;
> +
> struct i2c_adapter *ddc;
> void __iomem *hdmicore_regs;
> void __iomem *hd_regs;
> --
> 2.31.1
>
Hi Maxime
On Fri, 7 May 2021 at 16:05, Maxime Ripard <[email protected]> wrote:
>
> At boot, we can't rely on the vc4_get_crtc_encoder since we don't have a
> state yet and thus will not be able to figure out which connector is
> attached to our CRTC.
>
> However, we have a muxing bit in the CRTC register we can use to get the
> encoder currently connected to the pixelvalve. We can thus read that
> register, lookup the associated register through the vc4_pv_data
> structure, and then pass it to vc4_crtc_disable so that we can perform
> the proper operations.
>
> Fixes: 875a4d536842 ("drm/vc4: drv: Disable the CRTC at boot time")
> Signed-off-by: Maxime Ripard <[email protected]>
Reviewed-by: Dave Stevenson <[email protected]>
> ---
> drivers/gpu/drm/vc4/vc4_crtc.c | 38 ++++++++++++++++++++++++++++++----
> 1 file changed, 34 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
> index 36ea684a349b..f715648f89dd 100644
> --- a/drivers/gpu/drm/vc4/vc4_crtc.c
> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
> @@ -413,11 +413,10 @@ static void require_hvs_enabled(struct drm_device *dev)
> }
>
> static int vc4_crtc_disable(struct drm_crtc *crtc,
> + struct drm_encoder *encoder,
> struct drm_atomic_state *state,
> unsigned int channel)
> {
> - struct drm_encoder *encoder = vc4_get_crtc_encoder(crtc, state,
> - drm_atomic_get_old_connector_state);
> struct vc4_encoder *vc4_encoder = to_vc4_encoder(encoder);
> struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
> struct drm_device *dev = crtc->dev;
> @@ -458,10 +457,29 @@ static int vc4_crtc_disable(struct drm_crtc *crtc,
> return 0;
> }
>
> +static struct drm_encoder *vc4_crtc_get_encoder_by_type(struct drm_crtc *crtc,
> + enum vc4_encoder_type type)
> +{
> + struct drm_encoder *encoder;
> +
> + drm_for_each_encoder(encoder, crtc->dev) {
> + struct vc4_encoder *vc4_encoder = to_vc4_encoder(encoder);
> +
> + if (vc4_encoder->type == type)
> + return encoder;
> + }
> +
> + return NULL;
> +}
> +
> int vc4_crtc_disable_at_boot(struct drm_crtc *crtc)
> {
> struct drm_device *drm = crtc->dev;
> struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
> + enum vc4_encoder_type encoder_type;
> + const struct vc4_pv_data *pv_data;
> + struct drm_encoder *encoder;
> + unsigned encoder_sel;
> int channel;
>
> if (!(of_device_is_compatible(vc4_crtc->pdev->dev.of_node,
> @@ -480,7 +498,17 @@ int vc4_crtc_disable_at_boot(struct drm_crtc *crtc)
> if (channel < 0)
> return 0;
>
> - return vc4_crtc_disable(crtc, NULL, channel);
> + encoder_sel = VC4_GET_FIELD(CRTC_READ(PV_CONTROL), PV_CONTROL_CLK_SELECT);
> + if (WARN_ON(encoder_sel != 0))
> + return 0;
> +
> + pv_data = vc4_crtc_to_vc4_pv_data(vc4_crtc);
> + encoder_type = pv_data->encoder_types[encoder_sel];
> + encoder = vc4_crtc_get_encoder_by_type(crtc, encoder_type);
> + if (WARN_ON(!encoder))
> + return 0;
> +
> + return vc4_crtc_disable(crtc, encoder, NULL, channel);
> }
>
> static void vc4_crtc_atomic_disable(struct drm_crtc *crtc,
> @@ -489,6 +517,8 @@ static void vc4_crtc_atomic_disable(struct drm_crtc *crtc,
> struct drm_crtc_state *old_state = drm_atomic_get_old_crtc_state(state,
> crtc);
> struct vc4_crtc_state *old_vc4_state = to_vc4_crtc_state(old_state);
> + struct drm_encoder *encoder = vc4_get_crtc_encoder(crtc, state,
> + drm_atomic_get_old_connector_state);
> struct drm_device *dev = crtc->dev;
>
> require_hvs_enabled(dev);
> @@ -496,7 +526,7 @@ static void vc4_crtc_atomic_disable(struct drm_crtc *crtc,
> /* Disable vblank irq handling before crtc is disabled. */
> drm_crtc_vblank_off(crtc);
>
> - vc4_crtc_disable(crtc, state, old_vc4_state->assigned_channel);
> + vc4_crtc_disable(crtc, encoder, state, old_vc4_state->assigned_channel);
>
> /*
> * Make sure we issue a vblank event after disabling the CRTC if
> --
> 2.31.1
>
On Fri, 7 May 2021 at 16:05, Maxime Ripard <[email protected]> wrote:
>
> Since we fixed the hooks to disable the encoder at boot, we now have an
> unbalanced clk_disable call at boot since we never enabled them in the
> first place.
>
> Let's mimic the state of the hardware and enable the clocks at boot if
> the controller is enabled to get the use-count right.
>
> Cc: <[email protected]> # v5.10+
> Fixes: 09c438139b8f ("drm/vc4: hdmi: Implement finer-grained hooks")
> Signed-off-by: Maxime Ripard <[email protected]>
Reviewed-by: Dave Stevenson <[email protected]>
> ---
> drivers/gpu/drm/vc4/vc4_hdmi.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index 1fda574579af..9c919472ae84 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -1995,6 +1995,14 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data)
> if (vc4_hdmi->variant->reset)
> vc4_hdmi->variant->reset(vc4_hdmi);
>
> + if ((of_device_is_compatible(dev->of_node, "brcm,bcm2711-hdmi0") ||
> + of_device_is_compatible(dev->of_node, "brcm,bcm2711-hdmi1")) &&
> + HDMI_READ(HDMI_VID_CTL) & VC4_HD_VID_CTL_ENABLE) {
> + clk_prepare_enable(vc4_hdmi->pixel_clock);
> + clk_prepare_enable(vc4_hdmi->hsm_clock);
> + clk_prepare_enable(vc4_hdmi->pixel_bvb_clock);
> + }
> +
> pm_runtime_enable(dev);
>
> drm_simple_encoder_init(drm, encoder, DRM_MODE_ENCODER_TMDS);
> --
> 2.31.1
>
Hi Maxime
On Fri, 7 May 2021 at 16:06, Maxime Ripard <[email protected]> wrote:
>
> The HDMI controller on the BCM2711 includes a scrambler in order to
> reach the HDMI 2.0 modes that require it. Let's add the support for it.
>
> Acked-by: Thomas Zimmermann <[email protected]>
> Signed-off-by: Maxime Ripard <[email protected]>
Reviewed-by: Dave Stevenson <[email protected]>
> ---
> drivers/gpu/drm/vc4/vc4_hdmi.c | 64 +++++++++++++++++++++++++++++
> drivers/gpu/drm/vc4/vc4_hdmi_regs.h | 3 ++
> 2 files changed, 67 insertions(+)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index 01d24ce8a795..bda12fea0dce 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)
>
> @@ -462,6 +465,64 @@ 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;
> +
> + 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;
> +
> + if (!vc4_hdmi_mode_needs_scrambling(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 vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
> + struct drm_crtc *crtc = encoder->crtc;
> +
> + /*
> + * At boot, encoder->crtc will be NULL. Since we don't know the
> + * state of the scrambler and in order to avoid any
> + * inconsistency, let's disable it all the time.
> + */
> + if (crtc && !vc4_hdmi_supports_scrambling(encoder, &crtc->mode))
> + return;
> +
> + if (crtc && !vc4_hdmi_mode_needs_scrambling(&crtc->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)
> {
> @@ -474,6 +535,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,
> @@ -924,6 +987,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.31.1
>
Hi Maxime
On Fri, 7 May 2021 at 16:05, Maxime Ripard <[email protected]> wrote:
>
> The vc4_get_crtc_encoder function currently only works when the
> connector->state->crtc pointer is set, which is only true when the
> connector is currently enabled.
>
> However, we use it as part of the disable path as well, and our lookup
> will fail in that case, resulting in it returning a null pointer we
> can't act on.
>
> We can access the connector that used to be connected to that crtc
> though using the old connector state in the disable path.
>
> Since we want to support both the enable and disable path, we can
> support it by passing the state accessor variant as a function pointer,
> together with the atomic state.
>
> Fixes: 792c3132bc1b ("drm/vc4: encoder: Add finer-grained encoder callbacks")
> Signed-off-by: Maxime Ripard <[email protected]>
Reviewed-by: Dave Stevenson <[email protected]>
> ---
> drivers/gpu/drm/vc4/vc4_crtc.c | 21 ++++++++++++++++-----
> 1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
> index 8a19d6c76605..36ea684a349b 100644
> --- a/drivers/gpu/drm/vc4/vc4_crtc.c
> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
> @@ -262,14 +262,22 @@ static u32 vc4_crtc_get_fifo_full_level_bits(struct vc4_crtc *vc4_crtc,
> * allows drivers to push pixels to more than one encoder from the
> * same CRTC.
> */
> -static struct drm_encoder *vc4_get_crtc_encoder(struct drm_crtc *crtc)
> +static struct drm_encoder *vc4_get_crtc_encoder(struct drm_crtc *crtc,
> + struct drm_atomic_state *state,
> + struct drm_connector_state *(*get_state)(struct drm_atomic_state *state,
> + struct drm_connector *connector))
> {
> struct drm_connector *connector;
> struct drm_connector_list_iter conn_iter;
>
> drm_connector_list_iter_begin(crtc->dev, &conn_iter);
> drm_for_each_connector_iter(connector, &conn_iter) {
> - if (connector->state->crtc == crtc) {
> + struct drm_connector_state *conn_state = get_state(state, connector);
> +
> + if (!conn_state)
> + continue;
> +
> + if (conn_state->crtc == crtc) {
> drm_connector_list_iter_end(&conn_iter);
> return connector->encoder;
> }
> @@ -292,7 +300,8 @@ static void vc4_crtc_config_pv(struct drm_crtc *crtc, struct drm_atomic_state *s
> {
> struct drm_device *dev = crtc->dev;
> struct vc4_dev *vc4 = to_vc4_dev(dev);
> - struct drm_encoder *encoder = vc4_get_crtc_encoder(crtc);
> + struct drm_encoder *encoder = vc4_get_crtc_encoder(crtc, state,
> + drm_atomic_get_new_connector_state);
> struct vc4_encoder *vc4_encoder = to_vc4_encoder(encoder);
> struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
> const struct vc4_pv_data *pv_data = vc4_crtc_to_vc4_pv_data(vc4_crtc);
> @@ -407,7 +416,8 @@ static int vc4_crtc_disable(struct drm_crtc *crtc,
> struct drm_atomic_state *state,
> unsigned int channel)
> {
> - struct drm_encoder *encoder = vc4_get_crtc_encoder(crtc);
> + struct drm_encoder *encoder = vc4_get_crtc_encoder(crtc, state,
> + drm_atomic_get_old_connector_state);
> struct vc4_encoder *vc4_encoder = to_vc4_encoder(encoder);
> struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
> struct drm_device *dev = crtc->dev;
> @@ -507,7 +517,8 @@ static void vc4_crtc_atomic_enable(struct drm_crtc *crtc,
> {
> struct drm_device *dev = crtc->dev;
> struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
> - struct drm_encoder *encoder = vc4_get_crtc_encoder(crtc);
> + struct drm_encoder *encoder = vc4_get_crtc_encoder(crtc, state,
> + drm_atomic_get_new_connector_state);
> struct vc4_encoder *vc4_encoder = to_vc4_encoder(encoder);
>
> require_hvs_enabled(dev);
> --
> 2.31.1
>
On Fri, May 07, 2021 at 05:05:03PM +0200, Maxime Ripard wrote:
> 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.
>
> Let me know what you think,
> Maxime
Applied all the patches but patch 12 since we have an ongoing bug on the
core clock rate that ends up with the display being stalled.
Maxime