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(-) -- 2.1.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx