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

From: Dave Stevenson
Date: Tue Nov 02 2021 - 12:47:19 EST


On Mon, 25 Oct 2021 at 16:29, Maxime Ripard <maxime@xxxxxxxxxx> 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 <maxime@xxxxxxxxxx>

Reviewed-by: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx>

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