[PATCH v5 00/10] drm/vc4: hdmi: Support the 4k @ 60Hz modes

List overview All Threads
Download

newer

older

[PATCH AUTOSEL 5.12 4/5]...

[Bug 213599] New: amdgpu: navi RX...

Maxime Ripard

28 Jun 2021 28 Jun '21

12:42 p.m.

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 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 | 9 ++- drivers/gpu/drm/vc4/vc4_hdmi.c | 20 +++-- drivers/gpu/drm/vc4/vc4_kms.c | 126 +++++++++++++++++++++++++----- drivers/gpu/drm/vc4/vc4_plane.c | 3 - 6 files changed, 164 insertions(+), 61 deletions(-)

-- 2.31.1

Show replies by date

Maxime Ripard

28 Jun 28 Jun

12:42 p.m.

New subject: [PATCH v5 01/10] drm/vc4: hdmi: Remove the DDC probing for status detection

Commit 9d44abbbb8d5 ("drm/vc4: Fall back to using an EDID probe in the absence of a GPIO.") added some code to read the EDID through DDC in the HDMI driver detect hook since the Pi3 had no HPD GPIO back then. However, commit b1b8f45b3130 ("ARM: dts: bcm2837: Add missing GPIOs of Expander") changed that a couple of years later.

This causes an issue though since some TV (like the LG 55C8) when it comes out of standy will deassert the HPD line, but the EDID will remain readable.

It causes an issues nn platforms without an HPD GPIO, like the Pi4, where the DDC probing will be our primary mean to detect a display, and thus we will never detect the HPD pulse. This was fine before since the pulse was small enough that we would never detect it, and we also didn't have anything (like the scrambler) that needed to be set up in the display.

However, now that we have both, the display during the HPD pulse will clear its scrambler status, and since we won't detect the disconnect/reconnect cycle we will never enable the scrambler back.

As our main reason for that DDC probing is gone, let's just remove it.

Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 2 -- 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 4ebe216b10a9..b8b371abf3d7 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -170,8 +170,6 @@ vc4_hdmi_connector_detect(struct drm_connector *connector, bool force) if (vc4_hdmi->hpd_gpio && gpiod_get_value_cansleep(vc4_hdmi->hpd_gpio)) { connected = true; - } else if (drm_probe_ddc(vc4_hdmi->ddc)) { - connected = true; } else if (HDMI_READ(HDMI_HOTPLUG) & VC4_HDMI_HOTPLUG_CONNECTED) { connected = true; }

-- 2.31.1

Maxime Ripard

12:42 p.m.

New subject: [PATCH v5 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 maxime@cerno.tech --- 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 b8b371abf3d7..e7fdda8a1bfa 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -167,9 +167,9 @@ vc4_hdmi_connector_detect(struct drm_connector *connector, bool force) struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector); bool connected = false;

- 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 if (HDMI_READ(HDMI_HOTPLUG) & VC4_HDMI_HOTPLUG_CONNECTED) { connected = true; }

-- 2.31.1

Maxime Ripard

12:42 p.m.

New subject: [PATCH v5 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 maxime@cerno.tech --- 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 18f5009ce90e..902862a67341 100644 --- a/drivers/gpu/drm/vc4/vc4_crtc.c +++ b/drivers/gpu/drm/vc4/vc4_crtc.c @@ -279,10 +279,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 5dceadc61600..d3e5238eadb5 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.h +++ b/drivers/gpu/drm/vc4/vc4_drv.h @@ -515,6 +515,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

Maxime Ripard

12:42 p.m.

New subject: [PATCH v5 04/10] drm/vc4: crtc: Add encoder to vc4_crtc_config_pv prototype

vc4_crtc_config_pv() retrieves the encoder again, even though its only caller, vc4_crtc_atomic_enable(), already did.

Pass the encoder pointer as an argument instead of going through all the connectors to retrieve it again.

Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_crtc.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c index 902862a67341..93d2413d0842 100644 --- a/drivers/gpu/drm/vc4/vc4_crtc.c +++ b/drivers/gpu/drm/vc4/vc4_crtc.c @@ -313,12 +313,11 @@ 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, struct drm_atomic_state *state) +static void vc4_crtc_config_pv(struct drm_crtc *crtc, struct drm_encoder *encoder, + struct drm_atomic_state *state) { struct drm_device *dev = crtc->dev; struct vc4_dev *vc4 = to_vc4_dev(dev); - 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); @@ -580,7 +579,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, state); + vc4_crtc_config_pv(crtc, encoder, state);

CRTC_WRITE(PV_CONTROL, CRTC_READ(PV_CONTROL) | PV_CONTROL_EN);

-- 2.31.1

Maxime Ripard

12:42 p.m.

New subject: [PATCH v5 05/10] drm/vc4: crtc: Rework the encoder retrieval code (again)

It turns out the encoder retrieval code, in addition to being unnecessarily complicated, has a bug when only the planes and crtcs are affected by a given atomic commit.

Indeed, in such a case, either drm_atomic_get_old_connector_state or drm_atomic_get_new_connector_state will return NULL and thus our encoder retrieval code will not match on anything.

We can however simplify the code by using drm_for_each_encoder_mask, the drm_crtc_state storing the encoders a given CRTC is connected to directly and without relying on any other state.

Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_crtc.c | 30 +++++++++--------------------- drivers/gpu/drm/vc4/vc4_drv.h | 4 +--- 2 files changed, 10 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c index 93d2413d0842..c88ce31ec90f 100644 --- a/drivers/gpu/drm/vc4/vc4_crtc.c +++ b/drivers/gpu/drm/vc4/vc4_crtc.c @@ -280,26 +280,14 @@ static u32 vc4_crtc_get_fifo_full_level_bits(struct vc4_crtc *vc4_crtc, * same CRTC. */ 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_crtc_state *state) { - struct drm_connector *connector; - struct drm_connector_list_iter conn_iter; + struct drm_encoder *encoder;

- drm_connector_list_iter_begin(crtc->dev, &conn_iter); - drm_for_each_connector_iter(connector, &conn_iter) { - struct drm_connector_state *conn_state = get_state(state, connector); + WARN_ON(hweight32(state->encoder_mask) > 1);

- if (!conn_state) - continue; - - if (conn_state->crtc == crtc) { - drm_connector_list_iter_end(&conn_iter); - return connector->encoder; - } - } - drm_connector_list_iter_end(&conn_iter); + drm_for_each_encoder_mask(encoder, crtc->dev, state->encoder_mask) + return encoder;

return NULL; } @@ -533,8 +521,7 @@ 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_encoder *encoder = vc4_get_crtc_encoder(crtc, old_state); struct drm_device *dev = crtc->dev;

require_hvs_enabled(dev); @@ -561,10 +548,11 @@ static void vc4_crtc_atomic_disable(struct drm_crtc *crtc, static void vc4_crtc_atomic_enable(struct drm_crtc *crtc, struct drm_atomic_state *state) { + struct drm_crtc_state *new_state = drm_atomic_get_new_crtc_state(state, + 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, state, - drm_atomic_get_new_connector_state); + struct drm_encoder *encoder = vc4_get_crtc_encoder(crtc, new_state); struct vc4_encoder *vc4_encoder = to_vc4_encoder(encoder);

require_hvs_enabled(dev); diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h index d3e5238eadb5..52214a1568fe 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.h +++ b/drivers/gpu/drm/vc4/vc4_drv.h @@ -516,9 +516,7 @@ vc4_crtc_to_vc4_pv_data(const struct vc4_crtc *crtc) }

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_crtc_state *state);

struct vc4_crtc_state { struct drm_crtc_state base;

-- 2.31.1

Maxime Ripard

12:42 p.m.

New subject: [PATCH v5 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 maxime@cerno.tech --- 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 c88ce31ec90f..073b7e528175 100644 --- a/drivers/gpu/drm/vc4/vc4_crtc.c +++ b/drivers/gpu/drm/vc4/vc4_crtc.c @@ -524,6 +524,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. */ @@ -555,6 +558,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

Maxime Ripard

12:42 p.m.

New subject: [PATCH v5 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 maxime@cerno.tech --- 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 | 3 --- 4 files changed, 10 insertions(+), 19 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 52214a1568fe..ac8021639d03 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.h +++ b/drivers/gpu/drm/vc4/vc4_drv.h @@ -200,9 +200,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 f29ac64a5aa5..d6b707711f58 100644 --- a/drivers/gpu/drm/vc4/vc4_kms.c +++ b/drivers/gpu/drm/vc4/vc4_kms.c @@ -551,9 +551,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)) @@ -628,9 +625,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); }

@@ -638,9 +632,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; @@ -868,9 +859,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..01914d65bff5 100644 --- a/drivers/gpu/drm/vc4/vc4_plane.c +++ b/drivers/gpu/drm/vc4/vc4_plane.c @@ -532,9 +532,6 @@ static void vc4_plane_calc_load(struct drm_plane_state *state) 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, state->crtc);

-- 2.31.1

kernel test robot

5:01 p.m.

New subject: [PATCH v5 07/10] drm/vc4: Leverage the load tracker on the BCM2711

Hi Maxime,

I love your patch! Perhaps something to improve:

[auto build test WARNING on drm-intel/for-linux-next] [cannot apply to drm-tip/drm-tip anholt/for-next v5.13 next-20210628] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch]

url: https://github.com/0day-ci/linux/commits/Maxime-Ripard/drm-vc4-hdmi-Support-... base: git://anongit.freedesktop.org/drm-intel for-linux-next config: x86_64-randconfig-a016-20210628 (attached as .config) compiler: gcc-9 (Debian 9.3.0-22) 9.3.0 reproduce (this is a W=1 build): # https://github.com/0day-ci/linux/commit/1a67588c095aa22f70656da4b59474fdc841... git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Maxime-Ripard/drm-vc4-hdmi-Support-the-4k-60Hz-modes/20210628-204514 git checkout 1a67588c095aa22f70656da4b59474fdc8419e58 # save the attached .config to linux build tree make W=1 ARCH=x86_64

If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot lkp@intel.com

All warnings (new ones prefixed by >>):

drivers/gpu/drm/vc4/vc4_plane.c: In function 'vc4_plane_calc_load':

...

...
drivers/gpu/drm/vc4/vc4_plane.c:532:18: warning: variable 'vc4' set but not used [-Wunused-but-set-variable]

532 | struct vc4_dev *vc4; | ^~~

vim +/vc4 +532 drivers/gpu/drm/vc4/vc4_plane.c

5c6799942003df Eric Anholt 2015-12-28 524 4686da83154d87 Boris Brezillon 2019-02-20 525 static void vc4_plane_calc_load(struct drm_plane_state *state) 4686da83154d87 Boris Brezillon 2019-02-20 526 { 4686da83154d87 Boris Brezillon 2019-02-20 527 unsigned int hvs_load_shift, vrefresh, i; 4686da83154d87 Boris Brezillon 2019-02-20 528 struct drm_framebuffer *fb = state->fb; 4686da83154d87 Boris Brezillon 2019-02-20 529 struct vc4_plane_state *vc4_state; 4686da83154d87 Boris Brezillon 2019-02-20 530 struct drm_crtc_state *crtc_state; 4686da83154d87 Boris Brezillon 2019-02-20 531 unsigned int vscale_factor; f437bc1ec731d3 Maxime Ripard 2020-09-03 @532 struct vc4_dev *vc4; f437bc1ec731d3 Maxime Ripard 2020-09-03 533 f437bc1ec731d3 Maxime Ripard 2020-09-03 534 vc4 = to_vc4_dev(state->plane->dev); 4686da83154d87 Boris Brezillon 2019-02-20 535 vc4_state = to_vc4_plane_state(state); 4686da83154d87 Boris Brezillon 2019-02-20 536 crtc_state = drm_atomic_get_existing_crtc_state(state->state, 4686da83154d87 Boris Brezillon 2019-02-20 537 state->crtc); 4686da83154d87 Boris Brezillon 2019-02-20 538 vrefresh = drm_mode_vrefresh(&crtc_state->adjusted_mode); 4686da83154d87 Boris Brezillon 2019-02-20 539 4686da83154d87 Boris Brezillon 2019-02-20 540 /* The HVS is able to process 2 pixels/cycle when scaling the source, 4686da83154d87 Boris Brezillon 2019-02-20 541 * 4 pixels/cycle otherwise. 4686da83154d87 Boris Brezillon 2019-02-20 542 * Alpha blending step seems to be pipelined and it's always operating 4686da83154d87 Boris Brezillon 2019-02-20 543 * at 4 pixels/cycle, so the limiting aspect here seems to be the 4686da83154d87 Boris Brezillon 2019-02-20 544 * scaler block. 4686da83154d87 Boris Brezillon 2019-02-20 545 * HVS load is expressed in clk-cycles/sec (AKA Hz). 4686da83154d87 Boris Brezillon 2019-02-20 546 */ 4686da83154d87 Boris Brezillon 2019-02-20 547 if (vc4_state->x_scaling[0] != VC4_SCALING_NONE || 4686da83154d87 Boris Brezillon 2019-02-20 548 vc4_state->x_scaling[1] != VC4_SCALING_NONE || 4686da83154d87 Boris Brezillon 2019-02-20 549 vc4_state->y_scaling[0] != VC4_SCALING_NONE || 4686da83154d87 Boris Brezillon 2019-02-20 550 vc4_state->y_scaling[1] != VC4_SCALING_NONE) 4686da83154d87 Boris Brezillon 2019-02-20 551 hvs_load_shift = 1; 4686da83154d87 Boris Brezillon 2019-02-20 552 else 4686da83154d87 Boris Brezillon 2019-02-20 553 hvs_load_shift = 2; 4686da83154d87 Boris Brezillon 2019-02-20 554 4686da83154d87 Boris Brezillon 2019-02-20 555 vc4_state->membus_load = 0; 4686da83154d87 Boris Brezillon 2019-02-20 556 vc4_state->hvs_load = 0; 4686da83154d87 Boris Brezillon 2019-02-20 557 for (i = 0; i < fb->format->num_planes; i++) { 4686da83154d87 Boris Brezillon 2019-02-20 558 /* Even if the bandwidth/plane required for a single frame is 4686da83154d87 Boris Brezillon 2019-02-20 559 * 4686da83154d87 Boris Brezillon 2019-02-20 560 * vc4_state->src_w[i] * vc4_state->src_h[i] * cpp * vrefresh 4686da83154d87 Boris Brezillon 2019-02-20 561 * 4686da83154d87 Boris Brezillon 2019-02-20 562 * when downscaling, we have to read more pixels per line in 4686da83154d87 Boris Brezillon 2019-02-20 563 * the time frame reserved for a single line, so the bandwidth 4686da83154d87 Boris Brezillon 2019-02-20 564 * demand can be punctually higher. To account for that, we 4686da83154d87 Boris Brezillon 2019-02-20 565 * calculate the down-scaling factor and multiply the plane 4686da83154d87 Boris Brezillon 2019-02-20 566 * load by this number. We're likely over-estimating the read 4686da83154d87 Boris Brezillon 2019-02-20 567 * demand, but that's better than under-estimating it. 4686da83154d87 Boris Brezillon 2019-02-20 568 */ 4686da83154d87 Boris Brezillon 2019-02-20 569 vscale_factor = DIV_ROUND_UP(vc4_state->src_h[i], 4686da83154d87 Boris Brezillon 2019-02-20 570 vc4_state->crtc_h); 4686da83154d87 Boris Brezillon 2019-02-20 571 vc4_state->membus_load += vc4_state->src_w[i] * 4686da83154d87 Boris Brezillon 2019-02-20 572 vc4_state->src_h[i] * vscale_factor * 4686da83154d87 Boris Brezillon 2019-02-20 573 fb->format->cpp[i]; 4686da83154d87 Boris Brezillon 2019-02-20 574 vc4_state->hvs_load += vc4_state->crtc_h * vc4_state->crtc_w; 4686da83154d87 Boris Brezillon 2019-02-20 575 } 4686da83154d87 Boris Brezillon 2019-02-20 576 4686da83154d87 Boris Brezillon 2019-02-20 577 vc4_state->hvs_load *= vrefresh; 4686da83154d87 Boris Brezillon 2019-02-20 578 vc4_state->hvs_load >>= hvs_load_shift; 4686da83154d87 Boris Brezillon 2019-02-20 579 vc4_state->membus_load *= vrefresh; 4686da83154d87 Boris Brezillon 2019-02-20 580 } 4686da83154d87 Boris Brezillon 2019-02-20 581

--- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

kernel test robot

8:42 p.m.

New subject: [PATCH v5 07/10] drm/vc4: Leverage the load tracker on the BCM2711

Hi Maxime,

I love your patch! Perhaps something to improve:

url: https://github.com/0day-ci/linux/commits/Maxime-Ripard/drm-vc4-hdmi-Support-... base: git://anongit.freedesktop.org/drm-intel for-linux-next config: x86_64-randconfig-a002-20210628 (attached as .config) compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 4c92e31dd0f1bd152eda883af20ff7fbcaa14945) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install x86_64 cross compiling tool for clang build # apt-get install binutils-x86-64-linux-gnu # https://github.com/0day-ci/linux/commit/1a67588c095aa22f70656da4b59474fdc841... git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Maxime-Ripard/drm-vc4-hdmi-Support-the-4k-60Hz-modes/20210628-204514 git checkout 1a67588c095aa22f70656da4b59474fdc8419e58 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64

If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot lkp@intel.com

All warnings (new ones prefixed by >>):

...

...
drivers/gpu/drm/vc4/vc4_plane.c:532:18: warning: variable 'vc4' set but not used [-Wunused-but-set-variable]

struct vc4_dev *vc4; ^ 1 warning generated.

vim +/vc4 +532 drivers/gpu/drm/vc4/vc4_plane.c

--- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

Maxime Ripard

12:42 p.m.

New subject: [PATCH v5 08/10] drm/vc4: hdmi: Raise the maximum clock rate

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 tzimmermann@suse.de 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 e7fdda8a1bfa..46f656fd31b9 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -2280,7 +2280,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

Maxime Ripard

12:42 p.m.

New subject: [PATCH v5 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 maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 46f656fd31b9..7efafc893920 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -161,6 +161,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) { @@ -185,6 +187,8 @@ vc4_hdmi_connector_detect(struct drm_connector *connector, bool force) } }

+ vc4_hdmi_enable_scrambling(&vc4_hdmi->encoder.base.base); + return connector_status_connected; }

@@ -535,9 +539,13 @@ static bool vc4_hdmi_supports_scrambling(struct drm_encoder *encoder,

static void vc4_hdmi_enable_scrambling(struct drm_encoder *encoder) { - struct drm_display_mode *mode = &encoder->crtc->state->adjusted_mode; + struct drm_display_mode *mode; struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);

+ if (!encoder->crtc || !encoder->crtc->state) + return; + + mode = &encoder->crtc->state->adjusted_mode; if (!vc4_hdmi_supports_scrambling(encoder, mode)) return;

-- 2.31.1

Maxime Ripard

12:42 p.m.

New subject: [PATCH v5 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 maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_crtc.c | 15 +++++ drivers/gpu/drm/vc4/vc4_drv.h | 3 + drivers/gpu/drm/vc4/vc4_kms.c | 110 ++++++++++++++++++++++++++++++--- 3 files changed, 119 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c index 073b7e528175..c733b2091d3c 100644 --- a/drivers/gpu/drm/vc4/vc4_crtc.c +++ b/drivers/gpu/drm/vc4/vc4_crtc.c @@ -642,12 +642,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 ac8021639d03..08e3a055f7f6 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.h +++ b/drivers/gpu/drm/vc4/vc4_drv.h @@ -319,6 +319,7 @@ struct vc4_hvs { u32 __iomem *dlist;

struct clk *core_clk; + struct clk_request *core_req;

/* Memory manager for CRTCs to allocate space in the display * list. Units are dwords. @@ -530,6 +531,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 d6b707711f58..e443cfbe3049 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]; }; @@ -339,10 +341,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;

@@ -353,12 +364,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 = @@ -398,8 +410,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) @@ -656,9 +672,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; @@ -667,6 +683,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; }

@@ -821,6 +839,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) { @@ -838,7 +926,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

1410

Age (days ago)

1410

Last active (days ago)

dri-devel@lists.freedesktop.org

12 comments

2 participants

tags (0)

participants (2)

kernel test robot
Maxime Ripard