2021-10-25 15:33:56

by Maxime Ripard

[permalink] [raw]
Subject: [PATCH v8 00/10] drm/vc4: hdmi: Support the 4k @ 60Hz modes

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 v7:

- Rebased on current drm-misc-next



Changes from v6:

- Rebased on current drm-misc-next

- Removed stale clk_request pointer



Changes from v5:

- Fixed unused variables warning



Changes from v4:

- Removed the patches already applied

- Added various fixes for the issues that have been discovered on the

downstream tree



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 (10):

drm/vc4: hdmi: Remove the DDC probing for status detection

drm/vc4: hdmi: Fix HPD GPIO detection

drm/vc4: Make vc4_crtc_get_encoder public

drm/vc4: crtc: Add encoder to vc4_crtc_config_pv prototype

drm/vc4: crtc: Rework the encoder retrieval code (again)

drm/vc4: crtc: Add some logging

drm/vc4: Leverage the load tracker on the BCM2711

drm/vc4: hdmi: Raise the maximum clock rate

drm/vc4: hdmi: Enable the scrambler on reconnection

drm/vc4: Increase the core clock based on HVS load



drivers/gpu/drm/vc4/vc4_crtc.c | 60 ++++++++------

drivers/gpu/drm/vc4/vc4_debugfs.c | 7 +-

drivers/gpu/drm/vc4/vc4_drv.h | 8 +-

drivers/gpu/drm/vc4/vc4_hdmi.c | 13 +--

drivers/gpu/drm/vc4/vc4_kms.c | 126 +++++++++++++++++++++++++-----

drivers/gpu/drm/vc4/vc4_plane.c | 5 --

6 files changed, 157 insertions(+), 62 deletions(-)



--

2.31.1




2021-10-25 15:34:13

by Maxime Ripard

[permalink] [raw]
Subject: [PATCH v8 10/10] drm/vc4: Increase the core clock based on HVS load

Depending on a given HVS output (HVS to PixelValves) and input (planes
attached to a channel) load, the HVS needs for the core clock to be
raised above its boot time default.

Failing to do so will result in a vblank timeout and a stalled display
pipeline.

Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/vc4/vc4_crtc.c | 15 +++++
drivers/gpu/drm/vc4/vc4_drv.h | 2 +
drivers/gpu/drm/vc4/vc4_kms.c | 110 ++++++++++++++++++++++++++++++---
3 files changed, 118 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index 6decaa12a078..287dbc89ad64 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -659,12 +659,27 @@ static int vc4_crtc_atomic_check(struct drm_crtc *crtc,
struct vc4_crtc_state *vc4_state = to_vc4_crtc_state(crtc_state);
struct drm_connector *conn;
struct drm_connector_state *conn_state;
+ struct drm_encoder *encoder;
int ret, i;

ret = vc4_hvs_atomic_check(crtc, state);
if (ret)
return ret;

+ encoder = vc4_get_crtc_encoder(crtc, crtc_state);
+ if (encoder) {
+ const struct drm_display_mode *mode = &crtc_state->adjusted_mode;
+ struct vc4_encoder *vc4_encoder = to_vc4_encoder(encoder);
+
+ mode = &crtc_state->adjusted_mode;
+ if (vc4_encoder->type == VC4_ENCODER_TYPE_HDMI0) {
+ vc4_state->hvs_load = max(mode->clock * mode->hdisplay / mode->htotal + 1000,
+ mode->clock * 9 / 10) * 1000;
+ } else {
+ vc4_state->hvs_load = mode->clock * 1000;
+ }
+ }
+
for_each_new_connector_in_state(state, conn, conn_state,
i) {
if (conn_state->crtc != crtc)
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index 813c5d0ea98e..4329e09d357c 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -558,6 +558,8 @@ struct vc4_crtc_state {
unsigned int bottom;
} margins;

+ unsigned long hvs_load;
+
/* Transitional state below, only valid during atomic commits */
bool update_muxing;
};
diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
index 41cb4869da50..79d4d9dd1394 100644
--- a/drivers/gpu/drm/vc4/vc4_kms.c
+++ b/drivers/gpu/drm/vc4/vc4_kms.c
@@ -39,9 +39,11 @@ static struct vc4_ctm_state *to_vc4_ctm_state(struct drm_private_state *priv)

struct vc4_hvs_state {
struct drm_private_state base;
+ unsigned long core_clock_rate;

struct {
unsigned in_use: 1;
+ unsigned long fifo_load;
struct drm_crtc_commit *pending_commit;
} fifo_state[HVS_NUM_CHANNELS];
};
@@ -340,10 +342,19 @@ static void vc4_atomic_commit_tail(struct drm_atomic_state *state)
struct vc4_hvs *hvs = vc4->hvs;
struct drm_crtc_state *old_crtc_state;
struct drm_crtc_state *new_crtc_state;
+ struct vc4_hvs_state *new_hvs_state;
struct drm_crtc *crtc;
struct vc4_hvs_state *old_hvs_state;
int i;

+ old_hvs_state = vc4_hvs_get_old_global_state(state);
+ if (WARN_ON(!old_hvs_state))
+ return;
+
+ new_hvs_state = vc4_hvs_get_new_global_state(state);
+ if (WARN_ON(!new_hvs_state))
+ return;
+
for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
struct vc4_crtc_state *vc4_crtc_state;

@@ -354,12 +365,13 @@ static void vc4_atomic_commit_tail(struct drm_atomic_state *state)
vc4_hvs_mask_underrun(dev, vc4_crtc_state->assigned_channel);
}

- if (vc4->hvs->hvs5)
- clk_set_min_rate(hvs->core_clk, 500000000);
+ if (vc4->hvs->hvs5) {
+ unsigned long core_rate = max_t(unsigned long,
+ 500000000,
+ new_hvs_state->core_clock_rate);

- old_hvs_state = vc4_hvs_get_old_global_state(state);
- if (!old_hvs_state)
- return;
+ clk_set_min_rate(hvs->core_clk, core_rate);
+ }

for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) {
struct vc4_crtc_state *vc4_crtc_state =
@@ -399,8 +411,12 @@ static void vc4_atomic_commit_tail(struct drm_atomic_state *state)

drm_atomic_helper_cleanup_planes(dev, state);

- if (vc4->hvs->hvs5)
- clk_set_min_rate(hvs->core_clk, 0);
+ if (vc4->hvs->hvs5) {
+ drm_dbg(dev, "Running the core clock at %lu Hz\n",
+ new_hvs_state->core_clock_rate);
+
+ clk_set_min_rate(hvs->core_clk, new_hvs_state->core_clock_rate);
+ }
}

static int vc4_atomic_commit_setup(struct drm_atomic_state *state)
@@ -657,9 +673,9 @@ vc4_hvs_channels_duplicate_state(struct drm_private_obj *obj)

__drm_atomic_helper_private_obj_duplicate_state(obj, &state->base);

-
for (i = 0; i < HVS_NUM_CHANNELS; i++) {
state->fifo_state[i].in_use = old_state->fifo_state[i].in_use;
+ state->fifo_state[i].fifo_load = old_state->fifo_state[i].fifo_load;

if (!old_state->fifo_state[i].pending_commit)
continue;
@@ -668,6 +684,8 @@ vc4_hvs_channels_duplicate_state(struct drm_private_obj *obj)
drm_crtc_commit_get(old_state->fifo_state[i].pending_commit);
}

+ state->core_clock_rate = old_state->core_clock_rate;
+
return &state->base;
}

@@ -822,6 +840,76 @@ static int vc4_pv_muxing_atomic_check(struct drm_device *dev,
return 0;
}

+static int
+vc4_core_clock_atomic_check(struct drm_atomic_state *state)
+{
+ struct vc4_dev *vc4 = to_vc4_dev(state->dev);
+ struct drm_private_state *priv_state;
+ struct vc4_hvs_state *hvs_new_state;
+ struct vc4_load_tracker_state *load_state;
+ struct drm_crtc_state *old_crtc_state, *new_crtc_state;
+ struct drm_crtc *crtc;
+ unsigned int num_outputs;
+ unsigned long pixel_rate;
+ unsigned long cob_rate;
+ unsigned int i;
+
+ priv_state = drm_atomic_get_private_obj_state(state,
+ &vc4->load_tracker);
+ if (IS_ERR(priv_state))
+ return PTR_ERR(priv_state);
+
+ load_state = to_vc4_load_tracker_state(priv_state);
+
+ hvs_new_state = vc4_hvs_get_global_state(state);
+ if (!hvs_new_state)
+ return -EINVAL;
+
+ for_each_oldnew_crtc_in_state(state, crtc,
+ old_crtc_state,
+ new_crtc_state,
+ i) {
+ if (old_crtc_state->active) {
+ struct vc4_crtc_state *old_vc4_state =
+ to_vc4_crtc_state(old_crtc_state);
+ unsigned int channel = old_vc4_state->assigned_channel;
+
+ hvs_new_state->fifo_state[channel].fifo_load = 0;
+ }
+
+ if (new_crtc_state->active) {
+ struct vc4_crtc_state *new_vc4_state =
+ to_vc4_crtc_state(new_crtc_state);
+ unsigned int channel = new_vc4_state->assigned_channel;
+
+ hvs_new_state->fifo_state[channel].fifo_load =
+ new_vc4_state->hvs_load;
+ }
+ }
+
+ cob_rate = 0;
+ num_outputs = 0;
+ for (i = 0; i < HVS_NUM_CHANNELS; i++) {
+ if (!hvs_new_state->fifo_state[i].in_use)
+ continue;
+
+ num_outputs++;
+ cob_rate += hvs_new_state->fifo_state[i].fifo_load;
+ }
+
+ pixel_rate = load_state->hvs_load;
+ if (num_outputs > 1) {
+ pixel_rate = (pixel_rate * 40) / 100;
+ } else {
+ pixel_rate = (pixel_rate * 60) / 100;
+ }
+
+ hvs_new_state->core_clock_rate = max(cob_rate, pixel_rate);
+
+ return 0;
+}
+
+
static int
vc4_atomic_check(struct drm_device *dev, struct drm_atomic_state *state)
{
@@ -839,7 +927,11 @@ vc4_atomic_check(struct drm_device *dev, struct drm_atomic_state *state)
if (ret)
return ret;

- return vc4_load_tracker_atomic_check(state);
+ ret = vc4_load_tracker_atomic_check(state);
+ if (ret)
+ return ret;
+
+ return vc4_core_clock_atomic_check(state);
}

static struct drm_mode_config_helper_funcs vc4_mode_config_helpers = {
--
2.31.1

2021-10-25 15:34:27

by Maxime Ripard

[permalink] [raw]
Subject: [PATCH v8 02/10] drm/vc4: hdmi: Fix HPD GPIO detection

Prior to commit 6800234ceee0 ("drm/vc4: hdmi: Convert to gpiod"), in the
detect hook, if we had an HPD GPIO we would only rely on it and return
whatever state it was in.

However, that commit changed that by mistake to only consider the case
where we have a GPIO and it returns a logical high, and would fall back
to the other methods otherwise.

Since we can read the EDIDs when the HPD signal is low on some displays,
we changed the detection status from disconnected to connected, and we
would ignore an HPD pulse.

Fixes: 6800234ceee0 ("drm/vc4: hdmi: Convert to gpiod")
Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 338968275724..dde67b991ae7 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -190,9 +190,9 @@ vc4_hdmi_connector_detect(struct drm_connector *connector, bool force)

WARN_ON(pm_runtime_resume_and_get(&vc4_hdmi->pdev->dev));

- if (vc4_hdmi->hpd_gpio &&
- gpiod_get_value_cansleep(vc4_hdmi->hpd_gpio)) {
- connected = true;
+ if (vc4_hdmi->hpd_gpio) {
+ if (gpiod_get_value_cansleep(vc4_hdmi->hpd_gpio))
+ connected = true;
} else {
unsigned long flags;
u32 hotplug;
--
2.31.1

2021-10-25 15:34:48

by Maxime Ripard

[permalink] [raw]
Subject: [PATCH v8 03/10] drm/vc4: Make vc4_crtc_get_encoder public

We'll need that function in vc4_kms to compute the core clock rate
requirements.

Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/vc4/vc4_crtc.c | 8 ++++----
drivers/gpu/drm/vc4/vc4_drv.h | 5 +++++
2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index e3ed52d96f42..7cfd4a097847 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -281,10 +281,10 @@ 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,
- struct drm_atomic_state *state,
- struct drm_connector_state *(*get_state)(struct drm_atomic_state *state,
- struct drm_connector *connector))
+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;
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index 4b550ebd9572..f5e678491502 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -544,6 +544,11 @@ vc4_crtc_to_vc4_pv_data(const struct vc4_crtc *crtc)
return container_of(data, struct vc4_pv_data, base);
}

+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 vc4_crtc_state {
struct drm_crtc_state base;
/* Dlist area for this CRTC configuration. */
--
2.31.1

2021-10-25 18:27:28

by Maxime Ripard

[permalink] [raw]
Subject: [PATCH v8 09/10] drm/vc4: hdmi: Enable the scrambler on reconnection

If we have a state already and disconnect/reconnect the display, the
SCDC messages won't be sent again since we didn't go through a disable /
enable cycle.

In order to fix this, let's call the vc4_hdmi_enable_scrambling function
in the detect callback if there is a mode and it needs the scrambler to
be enabled.

Fixes: c85695a2016e ("drm/vc4: hdmi: Enable the scrambler")
Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index d36b3b6ebed1..fab9b93e1b84 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -180,6 +180,8 @@ static void vc4_hdmi_cec_update_clk_div(struct vc4_hdmi *vc4_hdmi)
static void vc4_hdmi_cec_update_clk_div(struct vc4_hdmi *vc4_hdmi) {}
#endif

+static void vc4_hdmi_enable_scrambling(struct drm_encoder *encoder);
+
static enum drm_connector_status
vc4_hdmi_connector_detect(struct drm_connector *connector, bool force)
{
@@ -216,6 +218,7 @@ vc4_hdmi_connector_detect(struct drm_connector *connector, bool force)
}
}

+ vc4_hdmi_enable_scrambling(&vc4_hdmi->encoder.base.base);
pm_runtime_put(&vc4_hdmi->pdev->dev);
mutex_unlock(&vc4_hdmi->mutex);
return connector_status_connected;
--
2.31.1

2021-10-25 19:06:24

by Maxime Ripard

[permalink] [raw]
Subject: [PATCH v8 07/10] drm/vc4: Leverage the load tracker on the BCM2711

The load tracker was initially designed to report and warn about a load
too high for the HVS. To do so, it computes for each plane the impact
it's going to have on the HVS, and will warn (if it's enabled) if we go
over what the hardware can process.

While the limits being used are a bit irrelevant to the BCM2711, the
algorithm to compute the HVS load will be one component used in order to
compute the core clock rate on the BCM2711.

Let's remove the hooks to prevent the load tracker to do its
computation, but since we don't have the same limits, don't check them
against them, and prevent the debugfs file to enable it from being
created.

Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/vc4/vc4_debugfs.c | 7 +++++--
drivers/gpu/drm/vc4/vc4_drv.h | 3 ---
drivers/gpu/drm/vc4/vc4_kms.c | 16 +++++-----------
drivers/gpu/drm/vc4/vc4_plane.c | 5 -----
4 files changed, 10 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_debugfs.c b/drivers/gpu/drm/vc4/vc4_debugfs.c
index 6da22af4ee91..ba2d8ea562af 100644
--- a/drivers/gpu/drm/vc4/vc4_debugfs.c
+++ b/drivers/gpu/drm/vc4/vc4_debugfs.c
@@ -7,6 +7,7 @@
#include <linux/circ_buf.h>
#include <linux/ctype.h>
#include <linux/debugfs.h>
+#include <linux/platform_device.h>

#include "vc4_drv.h"
#include "vc4_regs.h"
@@ -26,8 +27,10 @@ vc4_debugfs_init(struct drm_minor *minor)
struct vc4_dev *vc4 = to_vc4_dev(minor->dev);
struct vc4_debugfs_info_entry *entry;

- debugfs_create_bool("hvs_load_tracker", S_IRUGO | S_IWUSR,
- minor->debugfs_root, &vc4->load_tracker_enabled);
+ if (!of_device_is_compatible(vc4->hvs->pdev->dev.of_node,
+ "brcm,bcm2711-vc5"))
+ debugfs_create_bool("hvs_load_tracker", S_IRUGO | S_IWUSR,
+ minor->debugfs_root, &vc4->load_tracker_enabled);

list_for_each_entry(entry, &vc4->debugfs_list, link) {
drm_debugfs_create_files(&entry->info, 1,
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index 60826aca9e5b..813c5d0ea98e 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -202,9 +202,6 @@ struct vc4_dev {

int power_refcount;

- /* Set to true when the load tracker is supported. */
- bool load_tracker_available;
-
/* Set to true when the load tracker is active. */
bool load_tracker_enabled;

diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
index 028f19f7a5f8..41cb4869da50 100644
--- a/drivers/gpu/drm/vc4/vc4_kms.c
+++ b/drivers/gpu/drm/vc4/vc4_kms.c
@@ -552,9 +552,6 @@ static int vc4_load_tracker_atomic_check(struct drm_atomic_state *state)
struct drm_plane *plane;
int i;

- if (!vc4->load_tracker_available)
- return 0;
-
priv_state = drm_atomic_get_private_obj_state(state,
&vc4->load_tracker);
if (IS_ERR(priv_state))
@@ -629,9 +626,6 @@ static void vc4_load_tracker_obj_fini(struct drm_device *dev, void *unused)
{
struct vc4_dev *vc4 = to_vc4_dev(dev);

- if (!vc4->load_tracker_available)
- return;
-
drm_atomic_private_obj_fini(&vc4->load_tracker);
}

@@ -639,9 +633,6 @@ static int vc4_load_tracker_obj_init(struct vc4_dev *vc4)
{
struct vc4_load_tracker_state *load_state;

- if (!vc4->load_tracker_available)
- return 0;
-
load_state = kzalloc(sizeof(*load_state), GFP_KERNEL);
if (!load_state)
return -ENOMEM;
@@ -869,9 +860,12 @@ int vc4_kms_load(struct drm_device *dev)
"brcm,bcm2711-vc5");
int ret;

+ /*
+ * The limits enforced by the load tracker aren't relevant for
+ * the BCM2711, but the load tracker computations are used for
+ * the core clock rate calculation.
+ */
if (!is_vc5) {
- vc4->load_tracker_available = true;
-
/* Start with the load tracker enabled. Can be
* disabled through the debugfs load_tracker file.
*/
diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c
index 19161b6ab27f..ac761c683663 100644
--- a/drivers/gpu/drm/vc4/vc4_plane.c
+++ b/drivers/gpu/drm/vc4/vc4_plane.c
@@ -529,11 +529,6 @@ static void vc4_plane_calc_load(struct drm_plane_state *state)
struct vc4_plane_state *vc4_state;
struct drm_crtc_state *crtc_state;
unsigned int vscale_factor;
- struct vc4_dev *vc4;
-
- vc4 = to_vc4_dev(state->plane->dev);
- if (!vc4->load_tracker_available)
- return;

vc4_state = to_vc4_plane_state(state);
crtc_state = drm_atomic_get_existing_crtc_state(state->state,
--
2.31.1

2021-10-25 21:24:31

by Maxime Ripard

[permalink] [raw]
Subject: [PATCH v8 06/10] drm/vc4: crtc: Add some logging

The encoder retrieval code has been a source of bugs and glitches in the
past and the crtc <-> encoder association been wrong in a number of
different ways.

Add some logging to quickly spot issues if they occur.

Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/vc4/vc4_crtc.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index fbc1d4638650..6decaa12a078 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -541,6 +541,9 @@ static void vc4_crtc_atomic_disable(struct drm_crtc *crtc,
struct drm_encoder *encoder = vc4_get_crtc_encoder(crtc, old_state);
struct drm_device *dev = crtc->dev;

+ drm_dbg(dev, "Disabling CRTC %s (%u) connected to Encoder %s (%u)",
+ crtc->name, crtc->base.id, encoder->name, encoder->base.id);
+
require_hvs_enabled(dev);

/* Disable vblank irq handling before crtc is disabled. */
@@ -572,6 +575,9 @@ static void vc4_crtc_atomic_enable(struct drm_crtc *crtc,
struct drm_encoder *encoder = vc4_get_crtc_encoder(crtc, new_state);
struct vc4_encoder *vc4_encoder = to_vc4_encoder(encoder);

+ drm_dbg(dev, "Enabling CRTC %s (%u) connected to Encoder %s (%u)",
+ crtc->name, crtc->base.id, encoder->name, encoder->base.id);
+
require_hvs_enabled(dev);

/* Enable vblank irq handling before crtc is started otherwise
--
2.31.1

2021-11-02 16:38:35

by Dave Stevenson

[permalink] [raw]
Subject: Re: [PATCH v8 03/10] drm/vc4: Make vc4_crtc_get_encoder public

On Mon, 25 Oct 2021 at 16:29, Maxime Ripard <[email protected]> wrote:
>
> We'll need that function in vc4_kms to compute the core clock rate
> requirements.
>
> Signed-off-by: Maxime Ripard <[email protected]>

Reviewed-by: Dave Stevenson <[email protected]>

> ---
> drivers/gpu/drm/vc4/vc4_crtc.c | 8 ++++----
> drivers/gpu/drm/vc4/vc4_drv.h | 5 +++++
> 2 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
> index e3ed52d96f42..7cfd4a097847 100644
> --- a/drivers/gpu/drm/vc4/vc4_crtc.c
> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
> @@ -281,10 +281,10 @@ 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,
> - struct drm_atomic_state *state,
> - struct drm_connector_state *(*get_state)(struct drm_atomic_state *state,
> - struct drm_connector *connector))
> +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;
> diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
> index 4b550ebd9572..f5e678491502 100644
> --- a/drivers/gpu/drm/vc4/vc4_drv.h
> +++ b/drivers/gpu/drm/vc4/vc4_drv.h
> @@ -544,6 +544,11 @@ vc4_crtc_to_vc4_pv_data(const struct vc4_crtc *crtc)
> return container_of(data, struct vc4_pv_data, base);
> }
>
> +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 vc4_crtc_state {
> struct drm_crtc_state base;
> /* Dlist area for this CRTC configuration. */
> --
> 2.31.1
>

2021-11-02 16:42:35

by Dave Stevenson

[permalink] [raw]
Subject: Re: [PATCH v8 02/10] drm/vc4: hdmi: Fix HPD GPIO detection

On Mon, 25 Oct 2021 at 16:29, Maxime Ripard <[email protected]> wrote:
>
> Prior to commit 6800234ceee0 ("drm/vc4: hdmi: Convert to gpiod"), in the
> detect hook, if we had an HPD GPIO we would only rely on it and return
> whatever state it was in.
>
> However, that commit changed that by mistake to only consider the case
> where we have a GPIO and it returns a logical high, and would fall back
> to the other methods otherwise.
>
> Since we can read the EDIDs when the HPD signal is low on some displays,
> we changed the detection status from disconnected to connected, and we
> would ignore an HPD pulse.
>
> Fixes: 6800234ceee0 ("drm/vc4: hdmi: Convert to gpiod")
> Signed-off-by: Maxime Ripard <[email protected]>

Reviewed-by: Dave Stevenson <[email protected]>

> ---
> drivers/gpu/drm/vc4/vc4_hdmi.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index 338968275724..dde67b991ae7 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -190,9 +190,9 @@ vc4_hdmi_connector_detect(struct drm_connector *connector, bool force)
>
> WARN_ON(pm_runtime_resume_and_get(&vc4_hdmi->pdev->dev));
>
> - if (vc4_hdmi->hpd_gpio &&
> - gpiod_get_value_cansleep(vc4_hdmi->hpd_gpio)) {
> - connected = true;
> + if (vc4_hdmi->hpd_gpio) {
> + if (gpiod_get_value_cansleep(vc4_hdmi->hpd_gpio))
> + connected = true;
> } else {
> unsigned long flags;
> u32 hotplug;
> --
> 2.31.1
>

2021-11-02 16:46:44

by Dave Stevenson

[permalink] [raw]
Subject: Re: [PATCH v8 06/10] drm/vc4: crtc: Add some logging

On Mon, 25 Oct 2021 at 16:29, Maxime Ripard <[email protected]> wrote:
>
> The encoder retrieval code has been a source of bugs and glitches in the
> past and the crtc <-> encoder association been wrong in a number of
> different ways.
>
> Add some logging to quickly spot issues if they occur.
>
> Signed-off-by: Maxime Ripard <[email protected]>

Reviewed-by: Dave Stevenson <[email protected]>

> ---
> drivers/gpu/drm/vc4/vc4_crtc.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
> index fbc1d4638650..6decaa12a078 100644
> --- a/drivers/gpu/drm/vc4/vc4_crtc.c
> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
> @@ -541,6 +541,9 @@ static void vc4_crtc_atomic_disable(struct drm_crtc *crtc,
> struct drm_encoder *encoder = vc4_get_crtc_encoder(crtc, old_state);
> struct drm_device *dev = crtc->dev;
>
> + drm_dbg(dev, "Disabling CRTC %s (%u) connected to Encoder %s (%u)",
> + crtc->name, crtc->base.id, encoder->name, encoder->base.id);
> +
> require_hvs_enabled(dev);
>
> /* Disable vblank irq handling before crtc is disabled. */
> @@ -572,6 +575,9 @@ static void vc4_crtc_atomic_enable(struct drm_crtc *crtc,
> struct drm_encoder *encoder = vc4_get_crtc_encoder(crtc, new_state);
> struct vc4_encoder *vc4_encoder = to_vc4_encoder(encoder);
>
> + drm_dbg(dev, "Enabling CRTC %s (%u) connected to Encoder %s (%u)",
> + crtc->name, crtc->base.id, encoder->name, encoder->base.id);
> +
> require_hvs_enabled(dev);
>
> /* Enable vblank irq handling before crtc is started otherwise
> --
> 2.31.1
>

2021-11-02 16:48:41

by Dave Stevenson

[permalink] [raw]
Subject: Re: [PATCH v8 07/10] drm/vc4: Leverage the load tracker on the BCM2711

On Mon, 25 Oct 2021 at 16:29, Maxime Ripard <[email protected]> wrote:
>
> The load tracker was initially designed to report and warn about a load
> too high for the HVS. To do so, it computes for each plane the impact
> it's going to have on the HVS, and will warn (if it's enabled) if we go
> over what the hardware can process.
>
> While the limits being used are a bit irrelevant to the BCM2711, the
> algorithm to compute the HVS load will be one component used in order to
> compute the core clock rate on the BCM2711.
>
> Let's remove the hooks to prevent the load tracker to do its
> computation, but since we don't have the same limits, don't check them
> against them, and prevent the debugfs file to enable it from being
> created.
>
> Signed-off-by: Maxime Ripard <[email protected]>

Reviewed-by: Dave Stevenson <[email protected]>

> ---
> drivers/gpu/drm/vc4/vc4_debugfs.c | 7 +++++--
> drivers/gpu/drm/vc4/vc4_drv.h | 3 ---
> drivers/gpu/drm/vc4/vc4_kms.c | 16 +++++-----------
> drivers/gpu/drm/vc4/vc4_plane.c | 5 -----
> 4 files changed, 10 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_debugfs.c b/drivers/gpu/drm/vc4/vc4_debugfs.c
> index 6da22af4ee91..ba2d8ea562af 100644
> --- a/drivers/gpu/drm/vc4/vc4_debugfs.c
> +++ b/drivers/gpu/drm/vc4/vc4_debugfs.c
> @@ -7,6 +7,7 @@
> #include <linux/circ_buf.h>
> #include <linux/ctype.h>
> #include <linux/debugfs.h>
> +#include <linux/platform_device.h>
>
> #include "vc4_drv.h"
> #include "vc4_regs.h"
> @@ -26,8 +27,10 @@ vc4_debugfs_init(struct drm_minor *minor)
> struct vc4_dev *vc4 = to_vc4_dev(minor->dev);
> struct vc4_debugfs_info_entry *entry;
>
> - debugfs_create_bool("hvs_load_tracker", S_IRUGO | S_IWUSR,
> - minor->debugfs_root, &vc4->load_tracker_enabled);
> + if (!of_device_is_compatible(vc4->hvs->pdev->dev.of_node,
> + "brcm,bcm2711-vc5"))
> + debugfs_create_bool("hvs_load_tracker", S_IRUGO | S_IWUSR,
> + minor->debugfs_root, &vc4->load_tracker_enabled);
>
> list_for_each_entry(entry, &vc4->debugfs_list, link) {
> drm_debugfs_create_files(&entry->info, 1,
> diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
> index 60826aca9e5b..813c5d0ea98e 100644
> --- a/drivers/gpu/drm/vc4/vc4_drv.h
> +++ b/drivers/gpu/drm/vc4/vc4_drv.h
> @@ -202,9 +202,6 @@ struct vc4_dev {
>
> int power_refcount;
>
> - /* Set to true when the load tracker is supported. */
> - bool load_tracker_available;
> -
> /* Set to true when the load tracker is active. */
> bool load_tracker_enabled;
>
> diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
> index 028f19f7a5f8..41cb4869da50 100644
> --- a/drivers/gpu/drm/vc4/vc4_kms.c
> +++ b/drivers/gpu/drm/vc4/vc4_kms.c
> @@ -552,9 +552,6 @@ static int vc4_load_tracker_atomic_check(struct drm_atomic_state *state)
> struct drm_plane *plane;
> int i;
>
> - if (!vc4->load_tracker_available)
> - return 0;
> -
> priv_state = drm_atomic_get_private_obj_state(state,
> &vc4->load_tracker);
> if (IS_ERR(priv_state))
> @@ -629,9 +626,6 @@ static void vc4_load_tracker_obj_fini(struct drm_device *dev, void *unused)
> {
> struct vc4_dev *vc4 = to_vc4_dev(dev);
>
> - if (!vc4->load_tracker_available)
> - return;
> -
> drm_atomic_private_obj_fini(&vc4->load_tracker);
> }
>
> @@ -639,9 +633,6 @@ static int vc4_load_tracker_obj_init(struct vc4_dev *vc4)
> {
> struct vc4_load_tracker_state *load_state;
>
> - if (!vc4->load_tracker_available)
> - return 0;
> -
> load_state = kzalloc(sizeof(*load_state), GFP_KERNEL);
> if (!load_state)
> return -ENOMEM;
> @@ -869,9 +860,12 @@ int vc4_kms_load(struct drm_device *dev)
> "brcm,bcm2711-vc5");
> int ret;
>
> + /*
> + * The limits enforced by the load tracker aren't relevant for
> + * the BCM2711, but the load tracker computations are used for
> + * the core clock rate calculation.
> + */
> if (!is_vc5) {
> - vc4->load_tracker_available = true;
> -
> /* Start with the load tracker enabled. Can be
> * disabled through the debugfs load_tracker file.
> */
> diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c
> index 19161b6ab27f..ac761c683663 100644
> --- a/drivers/gpu/drm/vc4/vc4_plane.c
> +++ b/drivers/gpu/drm/vc4/vc4_plane.c
> @@ -529,11 +529,6 @@ static void vc4_plane_calc_load(struct drm_plane_state *state)
> struct vc4_plane_state *vc4_state;
> struct drm_crtc_state *crtc_state;
> unsigned int vscale_factor;
> - struct vc4_dev *vc4;
> -
> - vc4 = to_vc4_dev(state->plane->dev);
> - if (!vc4->load_tracker_available)
> - return;
>
> vc4_state = to_vc4_plane_state(state);
> crtc_state = drm_atomic_get_existing_crtc_state(state->state,
> --
> 2.31.1
>

2021-11-02 16:56:03

by Dave Stevenson

[permalink] [raw]
Subject: Re: [PATCH v8 09/10] drm/vc4: hdmi: Enable the scrambler on reconnection

On Mon, 25 Oct 2021 at 16:29, Maxime Ripard <[email protected]> wrote:
>
> If we have a state already and disconnect/reconnect the display, the
> SCDC messages won't be sent again since we didn't go through a disable /
> enable cycle.
>
> In order to fix this, let's call the vc4_hdmi_enable_scrambling function
> in the detect callback if there is a mode and it needs the scrambler to
> be enabled.
>
> Fixes: c85695a2016e ("drm/vc4: hdmi: Enable the scrambler")
> Signed-off-by: Maxime Ripard <[email protected]>

Reviewed-by: Dave Stevenson <[email protected]>

> ---
> drivers/gpu/drm/vc4/vc4_hdmi.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index d36b3b6ebed1..fab9b93e1b84 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -180,6 +180,8 @@ static void vc4_hdmi_cec_update_clk_div(struct vc4_hdmi *vc4_hdmi)
> static void vc4_hdmi_cec_update_clk_div(struct vc4_hdmi *vc4_hdmi) {}
> #endif
>
> +static void vc4_hdmi_enable_scrambling(struct drm_encoder *encoder);
> +
> static enum drm_connector_status
> vc4_hdmi_connector_detect(struct drm_connector *connector, bool force)
> {
> @@ -216,6 +218,7 @@ vc4_hdmi_connector_detect(struct drm_connector *connector, bool force)
> }
> }
>
> + vc4_hdmi_enable_scrambling(&vc4_hdmi->encoder.base.base);
> pm_runtime_put(&vc4_hdmi->pdev->dev);
> mutex_unlock(&vc4_hdmi->mutex);
> return connector_status_connected;
> --
> 2.31.1
>

2021-11-02 17:15:50

by Dave Stevenson

[permalink] [raw]
Subject: Re: [PATCH v8 10/10] drm/vc4: Increase the core clock based on HVS load

On Mon, 25 Oct 2021 at 16:29, Maxime Ripard <[email protected]> wrote:
>
> Depending on a given HVS output (HVS to PixelValves) and input (planes
> attached to a channel) load, the HVS needs for the core clock to be
> raised above its boot time default.
>
> Failing to do so will result in a vblank timeout and a stalled display
> pipeline.
>
> Signed-off-by: Maxime Ripard <[email protected]>

I will make the comment that this does a load of computation of hvs
load when running on hvs4 (BCM2835/6/7), even though it's redundant on
those chips.
The overhead is relatively minimal, but could be bypassed if viewed necessary.

Otherwise
Reviewed-by: Dave Stevenson <[email protected]>

> ---
> drivers/gpu/drm/vc4/vc4_crtc.c | 15 +++++
> drivers/gpu/drm/vc4/vc4_drv.h | 2 +
> drivers/gpu/drm/vc4/vc4_kms.c | 110 ++++++++++++++++++++++++++++++---
> 3 files changed, 118 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
> index 6decaa12a078..287dbc89ad64 100644
> --- a/drivers/gpu/drm/vc4/vc4_crtc.c
> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
> @@ -659,12 +659,27 @@ static int vc4_crtc_atomic_check(struct drm_crtc *crtc,
> struct vc4_crtc_state *vc4_state = to_vc4_crtc_state(crtc_state);
> struct drm_connector *conn;
> struct drm_connector_state *conn_state;
> + struct drm_encoder *encoder;
> int ret, i;
>
> ret = vc4_hvs_atomic_check(crtc, state);
> if (ret)
> return ret;
>
> + encoder = vc4_get_crtc_encoder(crtc, crtc_state);
> + if (encoder) {
> + const struct drm_display_mode *mode = &crtc_state->adjusted_mode;
> + struct vc4_encoder *vc4_encoder = to_vc4_encoder(encoder);
> +
> + mode = &crtc_state->adjusted_mode;
> + if (vc4_encoder->type == VC4_ENCODER_TYPE_HDMI0) {
> + vc4_state->hvs_load = max(mode->clock * mode->hdisplay / mode->htotal + 1000,
> + mode->clock * 9 / 10) * 1000;
> + } else {
> + vc4_state->hvs_load = mode->clock * 1000;
> + }
> + }
> +
> for_each_new_connector_in_state(state, conn, conn_state,
> i) {
> if (conn_state->crtc != crtc)
> diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
> index 813c5d0ea98e..4329e09d357c 100644
> --- a/drivers/gpu/drm/vc4/vc4_drv.h
> +++ b/drivers/gpu/drm/vc4/vc4_drv.h
> @@ -558,6 +558,8 @@ struct vc4_crtc_state {
> unsigned int bottom;
> } margins;
>
> + unsigned long hvs_load;
> +
> /* Transitional state below, only valid during atomic commits */
> bool update_muxing;
> };
> diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
> index 41cb4869da50..79d4d9dd1394 100644
> --- a/drivers/gpu/drm/vc4/vc4_kms.c
> +++ b/drivers/gpu/drm/vc4/vc4_kms.c
> @@ -39,9 +39,11 @@ static struct vc4_ctm_state *to_vc4_ctm_state(struct drm_private_state *priv)
>
> struct vc4_hvs_state {
> struct drm_private_state base;
> + unsigned long core_clock_rate;
>
> struct {
> unsigned in_use: 1;
> + unsigned long fifo_load;
> struct drm_crtc_commit *pending_commit;
> } fifo_state[HVS_NUM_CHANNELS];
> };
> @@ -340,10 +342,19 @@ static void vc4_atomic_commit_tail(struct drm_atomic_state *state)
> struct vc4_hvs *hvs = vc4->hvs;
> struct drm_crtc_state *old_crtc_state;
> struct drm_crtc_state *new_crtc_state;
> + struct vc4_hvs_state *new_hvs_state;
> struct drm_crtc *crtc;
> struct vc4_hvs_state *old_hvs_state;
> int i;
>
> + old_hvs_state = vc4_hvs_get_old_global_state(state);
> + if (WARN_ON(!old_hvs_state))
> + return;
> +
> + new_hvs_state = vc4_hvs_get_new_global_state(state);
> + if (WARN_ON(!new_hvs_state))
> + return;
> +
> for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
> struct vc4_crtc_state *vc4_crtc_state;
>
> @@ -354,12 +365,13 @@ static void vc4_atomic_commit_tail(struct drm_atomic_state *state)
> vc4_hvs_mask_underrun(dev, vc4_crtc_state->assigned_channel);
> }
>
> - if (vc4->hvs->hvs5)
> - clk_set_min_rate(hvs->core_clk, 500000000);
> + if (vc4->hvs->hvs5) {
> + unsigned long core_rate = max_t(unsigned long,
> + 500000000,
> + new_hvs_state->core_clock_rate);
>
> - old_hvs_state = vc4_hvs_get_old_global_state(state);
> - if (!old_hvs_state)
> - return;
> + clk_set_min_rate(hvs->core_clk, core_rate);
> + }
>
> for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) {
> struct vc4_crtc_state *vc4_crtc_state =
> @@ -399,8 +411,12 @@ static void vc4_atomic_commit_tail(struct drm_atomic_state *state)
>
> drm_atomic_helper_cleanup_planes(dev, state);
>
> - if (vc4->hvs->hvs5)
> - clk_set_min_rate(hvs->core_clk, 0);
> + if (vc4->hvs->hvs5) {
> + drm_dbg(dev, "Running the core clock at %lu Hz\n",
> + new_hvs_state->core_clock_rate);
> +
> + clk_set_min_rate(hvs->core_clk, new_hvs_state->core_clock_rate);
> + }
> }
>
> static int vc4_atomic_commit_setup(struct drm_atomic_state *state)
> @@ -657,9 +673,9 @@ vc4_hvs_channels_duplicate_state(struct drm_private_obj *obj)
>
> __drm_atomic_helper_private_obj_duplicate_state(obj, &state->base);
>
> -
> for (i = 0; i < HVS_NUM_CHANNELS; i++) {
> state->fifo_state[i].in_use = old_state->fifo_state[i].in_use;
> + state->fifo_state[i].fifo_load = old_state->fifo_state[i].fifo_load;
>
> if (!old_state->fifo_state[i].pending_commit)
> continue;
> @@ -668,6 +684,8 @@ vc4_hvs_channels_duplicate_state(struct drm_private_obj *obj)
> drm_crtc_commit_get(old_state->fifo_state[i].pending_commit);
> }
>
> + state->core_clock_rate = old_state->core_clock_rate;
> +
> return &state->base;
> }
>
> @@ -822,6 +840,76 @@ static int vc4_pv_muxing_atomic_check(struct drm_device *dev,
> return 0;
> }
>
> +static int
> +vc4_core_clock_atomic_check(struct drm_atomic_state *state)
> +{
> + struct vc4_dev *vc4 = to_vc4_dev(state->dev);
> + struct drm_private_state *priv_state;
> + struct vc4_hvs_state *hvs_new_state;
> + struct vc4_load_tracker_state *load_state;
> + struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> + struct drm_crtc *crtc;
> + unsigned int num_outputs;
> + unsigned long pixel_rate;
> + unsigned long cob_rate;
> + unsigned int i;
> +
> + priv_state = drm_atomic_get_private_obj_state(state,
> + &vc4->load_tracker);
> + if (IS_ERR(priv_state))
> + return PTR_ERR(priv_state);
> +
> + load_state = to_vc4_load_tracker_state(priv_state);
> +
> + hvs_new_state = vc4_hvs_get_global_state(state);
> + if (!hvs_new_state)
> + return -EINVAL;
> +
> + for_each_oldnew_crtc_in_state(state, crtc,
> + old_crtc_state,
> + new_crtc_state,
> + i) {
> + if (old_crtc_state->active) {
> + struct vc4_crtc_state *old_vc4_state =
> + to_vc4_crtc_state(old_crtc_state);
> + unsigned int channel = old_vc4_state->assigned_channel;
> +
> + hvs_new_state->fifo_state[channel].fifo_load = 0;
> + }
> +
> + if (new_crtc_state->active) {
> + struct vc4_crtc_state *new_vc4_state =
> + to_vc4_crtc_state(new_crtc_state);
> + unsigned int channel = new_vc4_state->assigned_channel;
> +
> + hvs_new_state->fifo_state[channel].fifo_load =
> + new_vc4_state->hvs_load;
> + }
> + }
> +
> + cob_rate = 0;
> + num_outputs = 0;
> + for (i = 0; i < HVS_NUM_CHANNELS; i++) {
> + if (!hvs_new_state->fifo_state[i].in_use)
> + continue;
> +
> + num_outputs++;
> + cob_rate += hvs_new_state->fifo_state[i].fifo_load;
> + }
> +
> + pixel_rate = load_state->hvs_load;
> + if (num_outputs > 1) {
> + pixel_rate = (pixel_rate * 40) / 100;
> + } else {
> + pixel_rate = (pixel_rate * 60) / 100;
> + }
> +
> + hvs_new_state->core_clock_rate = max(cob_rate, pixel_rate);
> +
> + return 0;
> +}
> +
> +
> static int
> vc4_atomic_check(struct drm_device *dev, struct drm_atomic_state *state)
> {
> @@ -839,7 +927,11 @@ vc4_atomic_check(struct drm_device *dev, struct drm_atomic_state *state)
> if (ret)
> return ret;
>
> - return vc4_load_tracker_atomic_check(state);
> + ret = vc4_load_tracker_atomic_check(state);
> + if (ret)
> + return ret;
> +
> + return vc4_core_clock_atomic_check(state);
> }
>
> static struct drm_mode_config_helper_funcs vc4_mode_config_helpers = {
> --
> 2.31.1
>

2021-11-04 09:47:02

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH v8 00/10] drm/vc4: hdmi: Support the 4k @ 60Hz modes

On Mon, 25 Oct 2021 17:28:53 +0200, Maxime Ripard wrote:
> 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 to drm/drm-misc (drm-misc-next).

Thanks!
Maxime