From 931577c62638e205271eb302e93d6a8c368711aa Mon Sep 17 00:00:00 2001 From: Alan Kwong Date: Thu, 8 Jun 2017 10:26:46 -0400 Subject: [PATCH 1/2] drm/msm/sde: move current performance setting to crtc object Current performance setting is maintained in crtc state, and its update is synchronized to commit cycle. However, the setting may be committed to clock and bandwidth driver out of sync with respect to commit cycle, e.g. update at end of frame while another commit is validating. As a result, requested settings may be missed and result in older settings being used. Move current performance setting to crtc object, from crtc state, so it can be updated at the same time as the setting is committed to clock and bandwidth driver. CRs-Fixed: 2048612 Change-Id: I0c3047e8e806460105eaba5d46145798bd98d721 Signed-off-by: Alan Kwong Signed-off-by: Abhinav Kumar --- drivers/gpu/drm/msm/sde/sde_core_perf.c | 11 ++++------- drivers/gpu/drm/msm/sde/sde_crtc.c | 9 ++++++--- drivers/gpu/drm/msm/sde/sde_crtc.h | 5 ++++- 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/msm/sde/sde_core_perf.c b/drivers/gpu/drm/msm/sde/sde_core_perf.c index 4d98b89fd007..8c312b74f38d 100644 --- a/drivers/gpu/drm/msm/sde/sde_core_perf.c +++ b/drivers/gpu/drm/msm/sde/sde_core_perf.c @@ -127,8 +127,6 @@ int sde_core_perf_crtc_check(struct drm_crtc *crtc, sde_cstate = to_sde_crtc_state(state); - /* swap state and obtain new values */ - sde_cstate->cur_perf = sde_cstate->new_perf; _sde_core_perf_calc_crtc(crtc, state, &sde_cstate->new_perf); bw_sum_of_intfs = sde_cstate->new_perf.bw_ctl; @@ -155,11 +153,9 @@ int sde_core_perf_crtc_check(struct drm_crtc *crtc, SDE_DEBUG("final threshold bw limit = %d\n", threshold); if (!threshold) { - sde_cstate->new_perf = sde_cstate->cur_perf; SDE_ERROR("no bandwidth limits specified\n"); return -E2BIG; } else if (bw > threshold) { - sde_cstate->new_perf = sde_cstate->cur_perf; SDE_DEBUG("exceeds bandwidth: %ukb > %ukb\n", bw, threshold); return -E2BIG; } @@ -258,6 +254,7 @@ static void _sde_core_perf_crtc_update_bus(struct sde_kms *kms, void sde_core_perf_crtc_release_bw(struct drm_crtc *crtc) { struct drm_crtc *tmp_crtc; + struct sde_crtc *sde_crtc; struct sde_crtc_state *sde_cstate; struct sde_kms *kms; @@ -272,6 +269,7 @@ void sde_core_perf_crtc_release_bw(struct drm_crtc *crtc) return; } + sde_crtc = to_sde_crtc(crtc); sde_cstate = to_sde_crtc_state(crtc->state); /* only do this for command panel or writeback */ @@ -294,8 +292,7 @@ void sde_core_perf_crtc_release_bw(struct drm_crtc *crtc) /* Release the bandwidth */ if (kms->perf.enable_bw_release) { trace_sde_cmd_release_bw(crtc->base.id); - sde_cstate->cur_perf.bw_ctl = 0; - sde_cstate->new_perf.bw_ctl = 0; + sde_crtc->cur_perf.bw_ctl = 0; SDE_DEBUG("Release BW crtc=%d\n", crtc->base.id); _sde_core_perf_crtc_update_bus(kms, crtc, 0); } @@ -362,7 +359,7 @@ void sde_core_perf_crtc_update(struct drm_crtc *crtc, SDE_ATRACE_BEGIN(__func__); - old = &sde_cstate->cur_perf; + old = &sde_crtc->cur_perf; new = &sde_cstate->new_perf; if (_sde_core_perf_crtc_is_power_on(crtc) && !stop_req) { diff --git a/drivers/gpu/drm/msm/sde/sde_crtc.c b/drivers/gpu/drm/msm/sde/sde_crtc.c index f66c135c9fae..2a31bc7fedc7 100644 --- a/drivers/gpu/drm/msm/sde/sde_crtc.c +++ b/drivers/gpu/drm/msm/sde/sde_crtc.c @@ -1888,15 +1888,18 @@ static const struct file_operations __prefix ## _fops = { \ static int sde_crtc_debugfs_state_show(struct seq_file *s, void *v) { struct drm_crtc *crtc = (struct drm_crtc *) s->private; + struct sde_crtc *sde_crtc = to_sde_crtc(crtc); struct sde_crtc_state *cstate = to_sde_crtc_state(crtc->state); seq_printf(s, "num_connectors: %d\n", cstate->num_connectors); seq_printf(s, "is_rt: %d\n", cstate->is_rt); seq_printf(s, "intf_mode: %d\n", sde_crtc_get_intf_mode(crtc)); - seq_printf(s, "bw_ctl: %llu\n", cstate->cur_perf.bw_ctl); - seq_printf(s, "core_clk_rate: %u\n", cstate->cur_perf.core_clk_rate); + + seq_printf(s, "bw_ctl: %llu\n", sde_crtc->cur_perf.bw_ctl); + seq_printf(s, "core_clk_rate: %u\n", + sde_crtc->cur_perf.core_clk_rate); seq_printf(s, "max_per_pipe_ib: %llu\n", - cstate->cur_perf.max_per_pipe_ib); + sde_crtc->cur_perf.max_per_pipe_ib); return 0; } diff --git a/drivers/gpu/drm/msm/sde/sde_crtc.h b/drivers/gpu/drm/msm/sde/sde_crtc.h index 0eed61580cd8..531b790347f2 100644 --- a/drivers/gpu/drm/msm/sde/sde_crtc.h +++ b/drivers/gpu/drm/msm/sde/sde_crtc.h @@ -95,6 +95,7 @@ struct sde_crtc_frame_event { * @frame_event_list : available frame event list * @pending : Whether any page-flip events are pending signal * @spin_lock : spin lock for frame event, transaction status, etc... + * @cur_perf : current performance committed to clock/bandwidth driver */ struct sde_crtc { struct drm_crtc base; @@ -134,6 +135,8 @@ struct sde_crtc { struct sde_crtc_frame_event frame_events[SDE_CRTC_FRAME_EVENT_SIZE]; struct list_head frame_event_list; spinlock_t spin_lock; + + struct sde_core_perf_params cur_perf; }; #define to_sde_crtc(x) container_of(x, struct sde_crtc, base) @@ -148,6 +151,7 @@ struct sde_crtc { * @property_values: Current crtc property values * @input_fence_timeout_ns : Cached input fence timeout, in ns * @property_blobs: Reference pointers for blob properties + * @new_perf: new performance state being requested */ struct sde_crtc_state { struct drm_crtc_state base; @@ -161,7 +165,6 @@ struct sde_crtc_state { uint64_t input_fence_timeout_ns; struct drm_property_blob *property_blobs[CRTC_PROP_COUNT]; - struct sde_core_perf_params cur_perf; struct sde_core_perf_params new_perf; }; From 0fc4f878b3b5c2c3884cfaffa18635aa1869cb33 Mon Sep 17 00:00:00 2001 From: Abhinav Kumar Date: Fri, 13 Oct 2017 21:02:23 -0700 Subject: [PATCH 2/2] drm/msm/sde: synchronize mdp clk with frame update There is a race condition between the commit and the validate, where the mdp clock or the bandwidth can be updated for voting before the actual configuration is taking place. Fix this issue by caching the performance values in the crtc till the current hw configuration is on-going. Change-Id: Icc71c4f58cbc305529d308335f44b8c05702ebee Signed-off-by: Abhinav Kumar --- drivers/gpu/drm/msm/sde/sde_core_perf.c | 41 +++++++++++++++++++------ drivers/gpu/drm/msm/sde/sde_crtc.h | 2 ++ 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/msm/sde/sde_core_perf.c b/drivers/gpu/drm/msm/sde/sde_core_perf.c index 8c312b74f38d..29e746e1fdf5 100644 --- a/drivers/gpu/drm/msm/sde/sde_core_perf.c +++ b/drivers/gpu/drm/msm/sde/sde_core_perf.c @@ -304,18 +304,27 @@ static int _sde_core_select_clk_lvl(struct sde_kms *kms, return clk_round_rate(kms->perf.core_clk, clk_rate); } -static u32 _sde_core_perf_get_core_clk_rate(struct sde_kms *kms) +static u32 _sde_core_perf_get_core_clk_rate(struct sde_kms *kms, + struct sde_core_perf_params *crct_perf, struct drm_crtc *crtc) { u32 clk_rate = 0; - struct drm_crtc *crtc; + struct drm_crtc *tmp_crtc; struct sde_crtc_state *sde_cstate; int ncrtc = 0; + u32 tmp_rate; - drm_for_each_crtc(crtc, kms->dev) { - if (_sde_core_perf_crtc_is_power_on(crtc)) { - sde_cstate = to_sde_crtc_state(crtc->state); - clk_rate = max(sde_cstate->new_perf.core_clk_rate, - clk_rate); + drm_for_each_crtc(tmp_crtc, kms->dev) { + if (_sde_core_perf_crtc_is_power_on(tmp_crtc)) { + + if (crtc->base.id == tmp_crtc->base.id) { + /* for current CRTC, use the cached value */ + tmp_rate = crct_perf->core_clk_rate; + } else { + sde_cstate = to_sde_crtc_state(tmp_crtc->state); + tmp_rate = sde_cstate->new_perf.core_clk_rate; + } + + clk_rate = max(tmp_rate, clk_rate); clk_rate = clk_round_rate(kms->perf.core_clk, clk_rate); } ncrtc++; @@ -359,8 +368,18 @@ void sde_core_perf_crtc_update(struct drm_crtc *crtc, SDE_ATRACE_BEGIN(__func__); + /* + * cache the performance numbers in the crtc prior to the + * crtc kickoff, so the same numbers are used during the + * perf update that happens post kickoff. + */ + + if (params_changed) + memcpy(&sde_crtc->new_perf, &sde_cstate->new_perf, + sizeof(struct sde_core_perf_params)); + old = &sde_crtc->cur_perf; - new = &sde_cstate->new_perf; + new = &sde_crtc->new_perf; if (_sde_core_perf_crtc_is_power_on(crtc) && !stop_req) { /* @@ -401,7 +420,7 @@ void sde_core_perf_crtc_update(struct drm_crtc *crtc, * use the new clock for the rotator bw calculation. */ if (update_clk) - clk_rate = _sde_core_perf_get_core_clk_rate(kms); + clk_rate = _sde_core_perf_get_core_clk_rate(kms, old, crtc); if (update_bus) _sde_core_perf_crtc_update_bus(kms, crtc, clk_rate); @@ -412,7 +431,9 @@ void sde_core_perf_crtc_update(struct drm_crtc *crtc, */ if (update_clk) { SDE_ATRACE_INT(kms->perf.clk_name, clk_rate); - SDE_EVT32(kms->dev, stop_req, clk_rate); + SDE_EVT32(kms->dev, stop_req, clk_rate, params_changed, + old->core_clk_rate, new->core_clk_rate); + ret = sde_power_clk_set_rate(&priv->phandle, kms->perf.clk_name, clk_rate); if (ret) { diff --git a/drivers/gpu/drm/msm/sde/sde_crtc.h b/drivers/gpu/drm/msm/sde/sde_crtc.h index 531b790347f2..200073995d43 100644 --- a/drivers/gpu/drm/msm/sde/sde_crtc.h +++ b/drivers/gpu/drm/msm/sde/sde_crtc.h @@ -96,6 +96,7 @@ struct sde_crtc_frame_event { * @pending : Whether any page-flip events are pending signal * @spin_lock : spin lock for frame event, transaction status, etc... * @cur_perf : current performance committed to clock/bandwidth driver + * @new_perf : new performance committed to clock/bandwidth driver */ struct sde_crtc { struct drm_crtc base; @@ -137,6 +138,7 @@ struct sde_crtc { spinlock_t spin_lock; struct sde_core_perf_params cur_perf; + struct sde_core_perf_params new_perf; }; #define to_sde_crtc(x) container_of(x, struct sde_crtc, base)