[PATCH 00/16] Pre-calculate SKL-style atomic watermarks

Matt Roper
2016-04-01T01:46:44Z

This series moves DDB allocation and watermark calculation for SKL-style
platforms to the atomic 'check' phase. This is important for two reasons,
described in more detail below.
1.) It allows us to reject atomic updates that would exceed the watermark
capabilities of the platform.
2.) Avoid exponentially redundant computation of watermark values (and
eliminate the WARN_ON(!wm_changed) errors).

1. Reject invalid configurations

If enough pipes and planes are turned on at the same time, it's possible that
we won't be able to find any valid watermark values that satisfy the display
configuration. Today we blindly assume that watermark programming will always
succeed (and potentially program the hardware incorrectly). We should take
advantage of the atomic modesetting design to recognize when a requested
display configuration exceeds our capabilities and reject the display update
before we get to the point of trying to program the hardware.

2. Avoid redundant computation

For historical reasons, our i915 legacy watermark entrypoints operate in a
per-crtc manner. They're called with a specific CRTC as a parameter that
indicates which CRTC is changing, but they actually wind up calculating and
programming state that can be global in nature on some platforms. On SKL-style
platforms specifically, calling update_wm on a single CRTC may actually
re-calculate the DDB allocation and watermarks for _all_ CRTC's. This worked
okay for legacy modesetting where an operation usually only updated a single
CRTC at a time and thus had only a single call into the update_watermarks
entrypoint. However now that our driver is atomic in nature, a single atomic
transaction may trigger changes to multiple CRTC's; our watermark entrypoints
get called for each CRTC of the transaction. On SKL, this means that a
transaction touching M crtc's on a platform with N crtc's will potentially call
the update_wm() entrypoint MxN times. I.e., we're redundantly computing and
programming watermark data more times than we need to. Effectively the
situation today is:

for each CRTC in atomic transaction {
for each affected CRTC {
calculate pipe-specific WM data;
}
combine per-pipe WM data into global WM data;
write global WM data;
}

(due to the nature of DDB programming on SKL-style platforms, the inner loop
may need to recompute for every CRTC on the platform, even if they aren't being
explicitly modified by the update).

Clearly this has a lot of unwanted redundancy (and is the source of the
WARN_ON(!wm_changed) warning that has been haunting the driver for a while
now). This series allows us to just calculate the necessary watermark data once
for the transaction.

A few notes on the series:
* This series deals with how watermark programming fits into the atomic design
of our driver, but doesn't focus on the lower-level computation of watermark
values (i.e., whether or not we're following the bspec's algorithms
properly). I've posted another series at
https://patchwork.freedesktop.org/series/4197/ (largely originating from
Mahesh and Shobhit) that implements new hardware workarounds from the bspec
or fixes bugs in areas where our logic didn't match the latest spec. That
series is mostly orthogonal to this series, but is arguably more important
since it fixes the low-level correctness of the driver whereas this series
is focused on higher-level design.

* This series untangles the computation half of watermarks a bit so that we're
not redundantly programming, but the final update of the hardware that
happens during atomic commit is still being performed more times than it
needs to be. I'll need to address that in a future patch series.

Kumar, Mahesh (1):
drm/i915/skl+: Use plane size for relative data rate calculation

Matt Roper (15):
drm/i915: Reorganize WM structs/unions in CRTC state
drm/i915: Make skl_plane_relative_data_rate use passed CRTC state
drm/i915: Rename s/skl_compute_pipe_wm/skl_build_pipe_wm/
drm/i915/gen9: Allow calculation of data rate for in-flight state
drm/i915: Ensure intel_state->active_crtcs is always set
drm/i915/gen9: Allow skl_allocate_pipe_ddb() to operate on in-flight
state
drm/i915/gen9: Compute DDB allocation at atomic check time
drm/i915/gen9: Drop re-allocation of DDB at atomic commit
drm/i915/gen9: Calculate plane WM's from state
drm/i915/gen9: Allow watermark calculation on in-flight atomic state
drm/i915/gen9: Use a bitmask to track dirty pipe watermarks
drm/i915/gen9: Propagate watermark calculation failures up the call
chain
drm/i915/gen9: Calculate watermarks during atomic 'check'
drm/i915/gen9: Reject display updates that exceed wm limitations
drm/i915: Remove wm_config from dev_priv/intel_atomic_state

drivers/gpu/drm/i915/i915_drv.h | 16 +-
drivers/gpu/drm/i915/intel_display.c | 44 +---
drivers/gpu/drm/i915/intel_drv.h | 86 ++++---
drivers/gpu/drm/i915/intel_pm.c | 440 +++++++++++++++++++++--------------
4 files changed, 344 insertions(+), 242 deletions(-)