On Thu, Apr 23, 2020 at 10:58:56AM +0300, Stanislav Lisovskiy wrote:
> We need to calculate SAGV mask also in a non-modeset
> commit, however currently active_pipes are only calculated
> for modesets in global atomic state, thus now we will be
> tracking those also in bw_state in order to be able to
> properly access global data.
>
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_bw.h | 3 +++
> drivers/gpu/drm/i915/intel_pm.c | 15 ++++++++++-----
> 2 files changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_bw.h b/drivers/gpu/drm/i915/display/intel_bw.h
> index d6df91058223..898b4a85ccab 100644
> --- a/drivers/gpu/drm/i915/display/intel_bw.h
> +++ b/drivers/gpu/drm/i915/display/intel_bw.h
> @@ -26,6 +26,9 @@ struct intel_bw_state {
>
> unsigned int data_rate[I915_MAX_PIPES];
> u8 num_active_planes[I915_MAX_PIPES];
> +
> + /* bitmask of active pipes */
> + u8 active_pipes;
> };
>
> #define to_intel_bw_state(x) container_of((x), struct intel_bw_state, base)
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 7e15cf3368ad..f7249bca3f6f 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3874,6 +3874,7 @@ static int intel_compute_sagv_mask(struct intel_atomic_state *state)
> struct intel_bw_state *new_bw_state = NULL;
> const struct intel_bw_state *old_bw_state = NULL;
> int i;
> + bool active_pipes_calculated = false;
>
> for_each_new_intel_crtc_in_state(state, crtc,
> new_crtc_state, i) {
> @@ -3883,6 +3884,12 @@ static int intel_compute_sagv_mask(struct intel_atomic_state *state)
>
> old_bw_state = intel_atomic_get_old_bw_state(state);
>
> + if (!active_pipes_calculated) {
> + state->active_pipes = new_bw_state->active_pipes =
I don't think we should touch state->active_pipes here.
> + intel_calc_active_pipes(state, old_bw_state->active_pipes);
> + active_pipes_calculated = true;
> + }
I'd do this after the loop so we don't need this extra boolean. As far
as the active_pipes check in intel_crtc_can_enable_sagv(), I think we
can pull it out into intel_compute_sagv_mask() so that we do the check
after computing the mask. And of course change it to use
bw_state->active_pipes instead.
We're also going to need to lock_global_state() if bw_state->active_pipes
mask changes.
> +
> if (intel_crtc_can_enable_sagv(new_crtc_state))
> new_bw_state->pipe_sagv_reject &= ~BIT(crtc->pipe);
> else
> @@ -5911,11 +5918,9 @@ skl_compute_wm(struct intel_atomic_state *state)
> if (ret)
> return ret;
>
> - if (state->modeset) {
> - ret = intel_compute_sagv_mask(state);
> - if (ret)
> - return ret;
> - }
> + ret = intel_compute_sagv_mask(state);
> + if (ret)
> + return ret;
We also need to remove the state->modeset checks around
sagv_{pre,post}_update().
>
> /*
> * skl_compute_ddb() will have adjusted the final watermarks
> --
> 2.24.1.485.gad05a3d8e5
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
