[Intel-gfx] [PATCH 2/3] drm/i915: Use crtc->state->active in ilk/skl watermark calculations (v3)
Daniel Vetter
daniel at ffwll.ch
Mon Mar 9 10:33:59 PDT 2015
On Mon, Mar 09, 2015 at 10:19:24AM -0700, Matt Roper wrote:
> Existing watermark code calls intel_crtc_active() to determine whether a CRTC
> is active for the purpose of watermark calculations (and bails out early if it
> determines the CRTC is not active). However intel_crtc_active() only returns
> true if crtc->primary->fb is non-NULL, which isn't appropriate in the modern
> age of universal planes and atomic modeset since userspace can now disable the
> primary plane, but leave the CRTC (and other planes) running.
>
> Note that commit
>
> commit 0fda65680e92545caea5be7805a7f0a617fb6c20
> Author: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> Date: Fri Feb 27 15:12:35 2015 +0000
>
> drm/i915/skl: Update watermarks for Y tiling
>
> adds a test for primary plane enable/disable to trigger a watermark update
> (previously we ignored updates to primary planes, which wasn't really correct,
> but we got lucky since we always pretended the primary plane was on). Tvrtko's
> patch tries to update watermarks when we re-enable the primary plane, but that
> watermark computation gets aborted early because intel_crtc_active() returns
> false due to the disabled primary plane.
>
> Switch the ILK and SKL watermark code over to use crtc->state->active rather
> than calling intel_crtc_active() so that we'll properly compute watermarks when
> re-enabling the primary plane.
>
> Note that this commit doesn't touch callsites in the watermark code for
> older platforms since there were concerns that doing so would lead to
> other types of breakage.
>
> Also note that all of the watermark calculation at the moment takes place after
> new crtc/plane states are swapped into the DRM objects. This will change in
> the future, so we'll be working with in-flight state objects, but for the time
> being, crtc->state is what we want to operate on.
>
> v2: Don't drop primary->fb check from intel_crtc_active(), but rather replace
> ILK/SKL callsites with direct tests of crtc->state->active. There is
> concern that messing with intel_crtc_active() will lead to other breakage for
> old hardware platforms. (Ville)
>
> v3: Use intel_crtc->active for now rather than crtc->state->active since
> we don't have CRTC states properly hooked up and initialized yet.
> We'll defer the switch to crtc->state->active until the atomic CRTC
> state work is farther along. (Ville)
>
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
> Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
Merged up to this one, thanks.
-Daniel
> ---
> drivers/gpu/drm/i915/intel_pm.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 4f04fab..a06a2c7 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1701,7 +1701,7 @@ hsw_compute_linetime_wm(struct drm_device *dev, struct drm_crtc *crtc)
> struct drm_display_mode *mode = &intel_crtc->config->base.adjusted_mode;
> u32 linetime, ips_linetime;
>
> - if (!intel_crtc_active(crtc))
> + if (!intel_crtc->active)
> return 0;
>
> /* The WM are computed with base on how long it takes to fill a single
> @@ -1956,7 +1956,7 @@ static void ilk_compute_wm_parameters(struct drm_crtc *crtc,
> enum pipe pipe = intel_crtc->pipe;
> struct drm_plane *plane;
>
> - if (!intel_crtc_active(crtc))
> + if (!intel_crtc->active)
> return;
>
> p->active = true;
> @@ -2468,7 +2468,7 @@ skl_ddb_get_pipe_allocation_limits(struct drm_device *dev,
>
> nth_active_pipe = 0;
> for_each_crtc(dev, crtc) {
> - if (!intel_crtc_active(crtc))
> + if (!to_intel_crtc(crtc)->active)
> continue;
>
> if (crtc == for_crtc)
> @@ -2708,7 +2708,7 @@ static void skl_compute_wm_global_parameters(struct drm_device *dev,
> struct drm_plane *plane;
>
> list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
> - config->num_pipes_active += intel_crtc_active(crtc);
> + config->num_pipes_active += to_intel_crtc(crtc)->active;
>
> /* FIXME: I don't think we need those two global parameters on SKL */
> list_for_each_entry(plane, &dev->mode_config.plane_list, head) {
> @@ -2729,7 +2729,7 @@ static void skl_compute_wm_pipe_parameters(struct drm_crtc *crtc,
> struct drm_framebuffer *fb;
> int i = 1; /* Index for sprite planes start */
>
> - p->active = intel_crtc_active(crtc);
> + p->active = intel_crtc->active;
> if (p->active) {
> p->pipe_htotal = intel_crtc->config->base.adjusted_mode.crtc_htotal;
> p->pixel_rate = skl_pipe_pixel_rate(intel_crtc->config);
> @@ -2860,7 +2860,7 @@ static void skl_compute_wm_level(const struct drm_i915_private *dev_priv,
> static uint32_t
> skl_compute_linetime_wm(struct drm_crtc *crtc, struct skl_pipe_wm_parameters *p)
> {
> - if (!intel_crtc_active(crtc))
> + if (!to_intel_crtc(crtc)->active)
> return 0;
>
> return DIV_ROUND_UP(8 * p->pipe_htotal * 1000, p->pixel_rate);
> @@ -3407,7 +3407,7 @@ static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc)
> hw->plane_trans[pipe][i] = I915_READ(PLANE_WM_TRANS(pipe, i));
> hw->cursor_trans[pipe] = I915_READ(CUR_WM_TRANS(pipe));
>
> - if (!intel_crtc_active(crtc))
> + if (!intel_crtc->active)
> return;
>
> hw->dirty[pipe] = true;
> @@ -3462,7 +3462,7 @@ static void ilk_pipe_wm_get_hw_state(struct drm_crtc *crtc)
> if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> hw->wm_linetime[pipe] = I915_READ(PIPE_WM_LINETIME(pipe));
>
> - active->pipe_enabled = intel_crtc_active(crtc);
> + active->pipe_enabled = intel_crtc->active;
>
> if (active->pipe_enabled) {
> u32 tmp = hw->wm_pipe[pipe];
> --
> 1.8.5.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
More information about the Intel-gfx
mailing list