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 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 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 > --- > 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, 5); > + if (vc4->hvs->hvs5) { > + unsigned long core_rate = max_t(unsigned long, > + 5, > + > 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_
[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 --- 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, 5); + if (vc4->hvs->hvs5) { + unsigned long core_rate = max_t(unsigned long, + 5, + 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_st