| Message ID | 1435803961-30079-7-git-send-email-matthew.d.roper@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jul 01, 2015 at 07:25:59PM -0700, Matt Roper wrote: > Calculate pipe watermarks during atomic calculation phase, based on the > contents of the atomic transaction's state structure. We still program > the watermarks at the same time we did before, but the computation now > happens much earlier. > > While this patch isn't too exciting by itself, it paves the way for > future patches. The eventual goal (which will be realized in future > patches in this series) is to calculate multiple sets up watermark > values up front, and then program them at different times (pre- vs > post-vblank) on the platforms that need a two-step watermark update. > > While we're at it, s/intel_compute_pipe_wm/ilk_compute_pipe_wm/ since > this function only applies to ILK-style watermarks and we have a > completely different function for SKL-style watermarks. > > Note that the original code had a memcmp() in ilk_update_wm() to avoid > calling ilk_program_watermarks() if the watermarks hadn't changed. This > memcmp vanishes here, which means we may do some unnecessary result > generation and merging in cases where watermarks didn't change, but the > lower-level function ilk_write_wm_values already makes sure that we > don't actually try to program the watermark registers again. > > v2: Squash a few commits from the original series together; no longer > leave pre-calculated wm's in a separate temporary structure since > it's easier to follow the logic if we just cut over to using the > pre-calculated values directly. > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 2 + > drivers/gpu/drm/i915/intel_display.c | 6 +++ > drivers/gpu/drm/i915/intel_drv.h | 2 + > drivers/gpu/drm/i915/intel_pm.c | 87 ++++++++++++++++++------------------ > 4 files changed, 53 insertions(+), 44 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 6aa8083..2774976 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -621,6 +621,8 @@ struct drm_i915_display_funcs { > int target, int refclk, > struct dpll *match_clock, > struct dpll *best_clock); > + int (*compute_pipe_wm)(struct drm_crtc *crtc, > + struct drm_atomic_state *state); All the new callbacks you add here are per-crtc, but we might want to rebalance the fifo space on some platforms where that's not perfectly per-crtc. Otoh that can wait until someone actually writes that code, maybe we'll decide to only rebalance wm when we need to do a modeset anyway. I think we can stick with this for now. -Daniel > void (*update_wm)(struct drm_crtc *crtc); > void (*update_sprite_wm)(struct drm_plane *plane, > struct drm_crtc *crtc, > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 36ae3f7..46b62cc 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -11857,6 +11857,12 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc, > return ret; > } > > + if (dev_priv->display.compute_pipe_wm) { > + ret = dev_priv->display.compute_pipe_wm(crtc, state); > + if (ret) > + return ret; > + } > + > return intel_atomic_setup_scalers(dev, intel_crtc, pipe_config); > } > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index c23cf7d..335b24b 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1445,6 +1445,8 @@ intel_atomic_get_crtc_state(struct drm_atomic_state *state, > int intel_atomic_setup_scalers(struct drm_device *dev, > struct intel_crtc *intel_crtc, > struct intel_crtc_state *crtc_state); > +int intel_check_crtc(struct drm_crtc *crtc, > + struct drm_crtc_state *state); > > /* intel_atomic_plane.c */ > struct intel_plane_state *intel_create_plane_state(struct drm_plane *plane); > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 0e28806..c6e735f 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -2039,9 +2039,11 @@ static void ilk_compute_wm_level(const struct drm_i915_private *dev_priv, > const struct intel_crtc *intel_crtc, > int level, > struct intel_crtc_state *cstate, > + struct intel_plane_state *pristate, > + struct intel_plane_state *sprstate, > + struct intel_plane_state *curstate, > struct intel_wm_level *result) > { > - struct intel_plane *intel_plane; > uint16_t pri_latency = dev_priv->wm.pri_latency[level]; > uint16_t spr_latency = dev_priv->wm.spr_latency[level]; > uint16_t cur_latency = dev_priv->wm.cur_latency[level]; > @@ -2053,29 +2055,11 @@ static void ilk_compute_wm_level(const struct drm_i915_private *dev_priv, > cur_latency *= 5; > } > > - for_each_intel_plane_on_crtc(dev_priv->dev, intel_crtc, intel_plane) { > - struct intel_plane_state *pstate = > - to_intel_plane_state(intel_plane->base.state); > - > - switch (intel_plane->base.type) { > - case DRM_PLANE_TYPE_PRIMARY: > - result->pri_val = ilk_compute_pri_wm(cstate, pstate, > - pri_latency, > - level); > - result->fbc_val = ilk_compute_fbc_wm(cstate, pstate, > - result->pri_val); > - break; > - case DRM_PLANE_TYPE_OVERLAY: > - result->spr_val = ilk_compute_spr_wm(cstate, pstate, > - spr_latency); > - break; > - case DRM_PLANE_TYPE_CURSOR: > - result->cur_val = ilk_compute_cur_wm(cstate, pstate, > - cur_latency); > - break; > - } > - } > - > + result->pri_val = ilk_compute_pri_wm(cstate, pristate, > + pri_latency, level); > + result->spr_val = ilk_compute_spr_wm(cstate, sprstate, spr_latency); > + result->cur_val = ilk_compute_cur_wm(cstate, curstate, cur_latency); > + result->fbc_val = ilk_compute_fbc_wm(cstate, pristate, result->pri_val); > result->enable = true; > } > > @@ -2355,15 +2339,20 @@ static void ilk_compute_wm_config(struct drm_device *dev, > } > > /* Compute new watermarks for the pipe */ > -static bool intel_compute_pipe_wm(struct intel_crtc_state *cstate, > - struct intel_pipe_wm *pipe_wm) > +static int ilk_compute_pipe_wm(struct drm_crtc *crtc, > + struct drm_atomic_state *state) > { > - struct drm_crtc *crtc = cstate->base.crtc; > + struct intel_pipe_wm *pipe_wm; > struct drm_device *dev = crtc->dev; > const struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > + struct drm_crtc_state *cs; > + struct intel_crtc_state *cstate = NULL; > struct intel_plane *intel_plane; > + struct drm_plane_state *ps; > + struct intel_plane_state *pristate = NULL; > struct intel_plane_state *sprstate = NULL; > + struct intel_plane_state *curstate = NULL; > int level, max_level = ilk_wm_max_level(dev); > /* LP0 watermark maximums depend on this pipe alone */ > struct intel_wm_config config = { > @@ -2371,11 +2360,26 @@ static bool intel_compute_pipe_wm(struct intel_crtc_state *cstate, > }; > struct ilk_wm_maximums max; > > + cs = drm_atomic_get_crtc_state(state, crtc); > + if (IS_ERR(cs)) > + return PTR_ERR(cs); > + else > + cstate = to_intel_crtc_state(cs); > + > + pipe_wm = &cstate->wm.active.ilk; > + > for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) { > - if (intel_plane->base.type == DRM_PLANE_TYPE_OVERLAY) { > - sprstate = to_intel_plane_state(intel_plane->base.state); > - break; > - } > + ps = drm_atomic_get_plane_state(state, > + &intel_plane->base); > + if (IS_ERR(ps)) > + return PTR_ERR(ps); > + > + if (intel_plane->base.type == DRM_PLANE_TYPE_PRIMARY) > + pristate = to_intel_plane_state(ps); > + else if (intel_plane->base.type == DRM_PLANE_TYPE_OVERLAY) > + sprstate = to_intel_plane_state(ps); > + else if (intel_plane->base.type == DRM_PLANE_TYPE_CURSOR) > + curstate = to_intel_plane_state(ps); > } > > config.sprites_enabled = sprstate->visible; > @@ -2384,7 +2388,7 @@ static bool intel_compute_pipe_wm(struct intel_crtc_state *cstate, > drm_rect_height(&sprstate->dst) != drm_rect_height(&sprstate->src) >> 16; > > pipe_wm->pipe_enabled = cstate->base.active; > - pipe_wm->sprites_enabled = sprstate->visible; > + pipe_wm->sprites_enabled = config.sprites_enabled; > pipe_wm->sprites_scaled = config.sprites_scaled; > > /* ILK/SNB: LP2+ watermarks only w/o sprites */ > @@ -2395,7 +2399,8 @@ static bool intel_compute_pipe_wm(struct intel_crtc_state *cstate, > if (config.sprites_scaled) > max_level = 0; > > - ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate, &pipe_wm->wm[0]); > + ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate, > + pristate, sprstate, curstate, &pipe_wm->wm[0]); > > if (IS_HASWELL(dev) || IS_BROADWELL(dev)) > pipe_wm->linetime = hsw_compute_linetime_wm(dev, crtc); > @@ -2405,14 +2410,15 @@ static bool intel_compute_pipe_wm(struct intel_crtc_state *cstate, > > /* At least LP0 must be valid */ > if (!ilk_validate_wm_level(0, &max, &pipe_wm->wm[0])) > - return false; > + return -EINVAL; > > ilk_compute_wm_reg_maximums(dev, 1, &max); > > for (level = 1; level <= max_level; level++) { > struct intel_wm_level wm = {}; > > - ilk_compute_wm_level(dev_priv, intel_crtc, level, cstate, &wm); > + ilk_compute_wm_level(dev_priv, intel_crtc, level, cstate, > + pristate, sprstate, curstate, &wm); > > /* > * Disable any watermark level that exceeds the > @@ -2425,7 +2431,7 @@ static bool intel_compute_pipe_wm(struct intel_crtc_state *cstate, > pipe_wm->wm[level] = wm; > } > > - return true; > + return 0; > } > > /* > @@ -3752,7 +3758,6 @@ static void ilk_update_wm(struct drm_crtc *crtc) > struct drm_i915_private *dev_priv = to_i915(crtc->dev); > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state); > - struct intel_pipe_wm pipe_wm = {}; > > /* > * IVB workaround: must disable low power watermarks for at least > @@ -3766,13 +3771,6 @@ static void ilk_update_wm(struct drm_crtc *crtc) > intel_wait_for_vblank(crtc->dev, intel_crtc->pipe); > } > > - intel_compute_pipe_wm(cstate, &pipe_wm); > - > - if (!memcmp(&cstate->wm.active.ilk, &pipe_wm, sizeof(pipe_wm))) > - return; > - > - cstate->wm.active.ilk = pipe_wm; > - > ilk_program_watermarks(dev_priv); > } > > @@ -7049,6 +7047,7 @@ void intel_init_pm(struct drm_device *dev) > (!IS_GEN5(dev) && dev_priv->wm.pri_latency[0] && > dev_priv->wm.spr_latency[0] && dev_priv->wm.cur_latency[0])) { > dev_priv->display.update_wm = ilk_update_wm; > + dev_priv->display.compute_pipe_wm = ilk_compute_pipe_wm; > } else { > DRM_DEBUG_KMS("Failed to read display plane latency. " > "Disable CxSR\n"); > -- > 2.1.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 6aa8083..2774976 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -621,6 +621,8 @@ struct drm_i915_display_funcs { int target, int refclk, struct dpll *match_clock, struct dpll *best_clock); + int (*compute_pipe_wm)(struct drm_crtc *crtc, + struct drm_atomic_state *state); void (*update_wm)(struct drm_crtc *crtc); void (*update_sprite_wm)(struct drm_plane *plane, struct drm_crtc *crtc, diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 36ae3f7..46b62cc 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -11857,6 +11857,12 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc, return ret; } + if (dev_priv->display.compute_pipe_wm) { + ret = dev_priv->display.compute_pipe_wm(crtc, state); + if (ret) + return ret; + } + return intel_atomic_setup_scalers(dev, intel_crtc, pipe_config); } diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index c23cf7d..335b24b 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1445,6 +1445,8 @@ intel_atomic_get_crtc_state(struct drm_atomic_state *state, int intel_atomic_setup_scalers(struct drm_device *dev, struct intel_crtc *intel_crtc, struct intel_crtc_state *crtc_state); +int intel_check_crtc(struct drm_crtc *crtc, + struct drm_crtc_state *state); /* intel_atomic_plane.c */ struct intel_plane_state *intel_create_plane_state(struct drm_plane *plane); diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 0e28806..c6e735f 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -2039,9 +2039,11 @@ static void ilk_compute_wm_level(const struct drm_i915_private *dev_priv, const struct intel_crtc *intel_crtc, int level, struct intel_crtc_state *cstate, + struct intel_plane_state *pristate, + struct intel_plane_state *sprstate, + struct intel_plane_state *curstate, struct intel_wm_level *result) { - struct intel_plane *intel_plane; uint16_t pri_latency = dev_priv->wm.pri_latency[level]; uint16_t spr_latency = dev_priv->wm.spr_latency[level]; uint16_t cur_latency = dev_priv->wm.cur_latency[level]; @@ -2053,29 +2055,11 @@ static void ilk_compute_wm_level(const struct drm_i915_private *dev_priv, cur_latency *= 5; } - for_each_intel_plane_on_crtc(dev_priv->dev, intel_crtc, intel_plane) { - struct intel_plane_state *pstate = - to_intel_plane_state(intel_plane->base.state); - - switch (intel_plane->base.type) { - case DRM_PLANE_TYPE_PRIMARY: - result->pri_val = ilk_compute_pri_wm(cstate, pstate, - pri_latency, - level); - result->fbc_val = ilk_compute_fbc_wm(cstate, pstate, - result->pri_val); - break; - case DRM_PLANE_TYPE_OVERLAY: - result->spr_val = ilk_compute_spr_wm(cstate, pstate, - spr_latency); - break; - case DRM_PLANE_TYPE_CURSOR: - result->cur_val = ilk_compute_cur_wm(cstate, pstate, - cur_latency); - break; - } - } - + result->pri_val = ilk_compute_pri_wm(cstate, pristate, + pri_latency, level); + result->spr_val = ilk_compute_spr_wm(cstate, sprstate, spr_latency); + result->cur_val = ilk_compute_cur_wm(cstate, curstate, cur_latency); + result->fbc_val = ilk_compute_fbc_wm(cstate, pristate, result->pri_val); result->enable = true; } @@ -2355,15 +2339,20 @@ static void ilk_compute_wm_config(struct drm_device *dev, } /* Compute new watermarks for the pipe */ -static bool intel_compute_pipe_wm(struct intel_crtc_state *cstate, - struct intel_pipe_wm *pipe_wm) +static int ilk_compute_pipe_wm(struct drm_crtc *crtc, + struct drm_atomic_state *state) { - struct drm_crtc *crtc = cstate->base.crtc; + struct intel_pipe_wm *pipe_wm; struct drm_device *dev = crtc->dev; const struct drm_i915_private *dev_priv = dev->dev_private; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + struct drm_crtc_state *cs; + struct intel_crtc_state *cstate = NULL; struct intel_plane *intel_plane; + struct drm_plane_state *ps; + struct intel_plane_state *pristate = NULL; struct intel_plane_state *sprstate = NULL; + struct intel_plane_state *curstate = NULL; int level, max_level = ilk_wm_max_level(dev); /* LP0 watermark maximums depend on this pipe alone */ struct intel_wm_config config = { @@ -2371,11 +2360,26 @@ static bool intel_compute_pipe_wm(struct intel_crtc_state *cstate, }; struct ilk_wm_maximums max; + cs = drm_atomic_get_crtc_state(state, crtc); + if (IS_ERR(cs)) + return PTR_ERR(cs); + else + cstate = to_intel_crtc_state(cs); + + pipe_wm = &cstate->wm.active.ilk; + for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) { - if (intel_plane->base.type == DRM_PLANE_TYPE_OVERLAY) { - sprstate = to_intel_plane_state(intel_plane->base.state); - break; - } + ps = drm_atomic_get_plane_state(state, + &intel_plane->base); + if (IS_ERR(ps)) + return PTR_ERR(ps); + + if (intel_plane->base.type == DRM_PLANE_TYPE_PRIMARY) + pristate = to_intel_plane_state(ps); + else if (intel_plane->base.type == DRM_PLANE_TYPE_OVERLAY) + sprstate = to_intel_plane_state(ps); + else if (intel_plane->base.type == DRM_PLANE_TYPE_CURSOR) + curstate = to_intel_plane_state(ps); } config.sprites_enabled = sprstate->visible; @@ -2384,7 +2388,7 @@ static bool intel_compute_pipe_wm(struct intel_crtc_state *cstate, drm_rect_height(&sprstate->dst) != drm_rect_height(&sprstate->src) >> 16; pipe_wm->pipe_enabled = cstate->base.active; - pipe_wm->sprites_enabled = sprstate->visible; + pipe_wm->sprites_enabled = config.sprites_enabled; pipe_wm->sprites_scaled = config.sprites_scaled; /* ILK/SNB: LP2+ watermarks only w/o sprites */ @@ -2395,7 +2399,8 @@ static bool intel_compute_pipe_wm(struct intel_crtc_state *cstate, if (config.sprites_scaled) max_level = 0; - ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate, &pipe_wm->wm[0]); + ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate, + pristate, sprstate, curstate, &pipe_wm->wm[0]); if (IS_HASWELL(dev) || IS_BROADWELL(dev)) pipe_wm->linetime = hsw_compute_linetime_wm(dev, crtc); @@ -2405,14 +2410,15 @@ static bool intel_compute_pipe_wm(struct intel_crtc_state *cstate, /* At least LP0 must be valid */ if (!ilk_validate_wm_level(0, &max, &pipe_wm->wm[0])) - return false; + return -EINVAL; ilk_compute_wm_reg_maximums(dev, 1, &max); for (level = 1; level <= max_level; level++) { struct intel_wm_level wm = {}; - ilk_compute_wm_level(dev_priv, intel_crtc, level, cstate, &wm); + ilk_compute_wm_level(dev_priv, intel_crtc, level, cstate, + pristate, sprstate, curstate, &wm); /* * Disable any watermark level that exceeds the @@ -2425,7 +2431,7 @@ static bool intel_compute_pipe_wm(struct intel_crtc_state *cstate, pipe_wm->wm[level] = wm; } - return true; + return 0; } /* @@ -3752,7 +3758,6 @@ static void ilk_update_wm(struct drm_crtc *crtc) struct drm_i915_private *dev_priv = to_i915(crtc->dev); struct intel_crtc *intel_crtc = to_intel_crtc(crtc); struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state); - struct intel_pipe_wm pipe_wm = {}; /* * IVB workaround: must disable low power watermarks for at least @@ -3766,13 +3771,6 @@ static void ilk_update_wm(struct drm_crtc *crtc) intel_wait_for_vblank(crtc->dev, intel_crtc->pipe); } - intel_compute_pipe_wm(cstate, &pipe_wm); - - if (!memcmp(&cstate->wm.active.ilk, &pipe_wm, sizeof(pipe_wm))) - return; - - cstate->wm.active.ilk = pipe_wm; - ilk_program_watermarks(dev_priv); } @@ -7049,6 +7047,7 @@ void intel_init_pm(struct drm_device *dev) (!IS_GEN5(dev) && dev_priv->wm.pri_latency[0] && dev_priv->wm.spr_latency[0] && dev_priv->wm.cur_latency[0])) { dev_priv->display.update_wm = ilk_update_wm; + dev_priv->display.compute_pipe_wm = ilk_compute_pipe_wm; } else { DRM_DEBUG_KMS("Failed to read display plane latency. " "Disable CxSR\n");
Calculate pipe watermarks during atomic calculation phase, based on the contents of the atomic transaction's state structure. We still program the watermarks at the same time we did before, but the computation now happens much earlier. While this patch isn't too exciting by itself, it paves the way for future patches. The eventual goal (which will be realized in future patches in this series) is to calculate multiple sets up watermark values up front, and then program them at different times (pre- vs post-vblank) on the platforms that need a two-step watermark update. While we're at it, s/intel_compute_pipe_wm/ilk_compute_pipe_wm/ since this function only applies to ILK-style watermarks and we have a completely different function for SKL-style watermarks. Note that the original code had a memcmp() in ilk_update_wm() to avoid calling ilk_program_watermarks() if the watermarks hadn't changed. This memcmp vanishes here, which means we may do some unnecessary result generation and merging in cases where watermarks didn't change, but the lower-level function ilk_write_wm_values already makes sure that we don't actually try to program the watermark registers again. v2: Squash a few commits from the original series together; no longer leave pre-calculated wm's in a separate temporary structure since it's easier to follow the logic if we just cut over to using the pre-calculated values directly. Signed-off-by: Matt Roper <matthew.d.roper@intel.com> --- drivers/gpu/drm/i915/i915_drv.h | 2 + drivers/gpu/drm/i915/intel_display.c | 6 +++ drivers/gpu/drm/i915/intel_drv.h | 2 + drivers/gpu/drm/i915/intel_pm.c | 87 ++++++++++++++++++------------------ 4 files changed, 53 insertions(+), 44 deletions(-)