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

* [PATCH 00/16] Pre-calculate SKL-style atomic watermarks
@ 2016-04-01  1:46 Matt Roper
  2016-04-01  1:46 ` [PATCH 01/16] drm/i915: Reorganize WM structs/unions in CRTC state Matt Roper
                   ` (16 more replies)
  0 siblings, 17 replies; 28+ messages in thread
From: Matt Roper @ 2016-04-01  1:46 UTC (permalink / raw
  To: intel-gfx

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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 28+ messages in thread