On Tue, Jul 19, 2016 at 12:30:56PM -0400, Lyude wrote:
> Now that we update the watermark values atomically, we still need to fix
> the case of how we update watermarks when we haven't added or removed
> pipes.
>
> When we haven't added or removed any pipes, we don't need to worry about
> the ddb allocation changing. As such there's no order of flushing pipes
> we have to guarantee; e.g. each pipe can be flushed at the earliest
> given oppurtunity. This means all we have to do is:
>
> - Calculate the new wm values
> - Update each plane's settings and wm values
> - Arm the plane's registers to update at the next vblank
>
> Right now the watermark code takes an additional few steps:
> - Rewrite the ddb allocations
> - Arm all of the planes for another update at the start of the next
> vblank
> - *Potentially underrun later if we update the wm registers before the
> start of the next vblank*
>
> This patch prevents us from marking pipes as dirty unless the number of
> pipes, and with that the ddb allocation, has actually changed. This
> results in us skipping the additional steps, preventing the hardware
> from using any intermediate values during each wm update.
>
> This also fixes cursors causing monitors to flicker on Skylake. Finally.
>
This doesn't look right to me. It's true that we don't need to worry
about *inter*-pipe DDB allocation changing, but *intra*-pipe DDB
allocations can still change. We may be resizing planes, changing
scalers, enabling/disabling new planes, etc. Those operations change
the DDB allocations for the planes within the fixed pipe allocation.
The watermark values will also change accordingly in that case.
Matt
> Signed-off-by: Lyude Paul <cpaul@xxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Cc: Daniel Vetter <daniel.vetter@xxxxxxxxx>
> Cc: Radhakrishna Sripada <radhakrishna.sripada@xxxxxxxxx>
> Cc: Hans de Goede <hdegoede@xxxxxxxxxx>
> Cc: Matt Roper <matthew.d.roper@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_pm.c | 16 +++++++++++-----
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 3a7709c..92158e3 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4086,12 +4086,18 @@ skl_compute_wm(struct drm_atomic_state *state)
> if (ret)
> return ret;
>
> - if (changed)
> - results->dirty_pipes |= drm_crtc_mask(crtc);
> + /*
> + * We don't need to do any additional setup for the pipes if we
> + * haven't changed the number of active crtcs
> + */
> + if (intel_state->active_pipe_changes) {
> + if (changed)
> + results->dirty_pipes |= drm_crtc_mask(crtc);
>
> - if ((results->dirty_pipes & drm_crtc_mask(crtc)) == 0)
> - /* This pipe's WM's did not change */
> - continue;
> + if ((results->dirty_pipes & drm_crtc_mask(crtc)) == 0)
> + /* This pipe's WM's did not change */
> + continue;
> + }
>
> intel_cstate->update_wm_pre = true;
> skl_compute_wm_results(crtc->dev, pipe_wm, results, intel_crtc);
> --
> 2.7.4
>
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html