Op 15-12-16 om 16:38 schreef Ville Syrjälä: > On Thu, Dec 15, 2016 at 04:30:54PM +0100, Maarten Lankhorst wrote: >> Op 12-12-16 om 21:35 schreef ville.syrjala@xxxxxxxxxxxxxxx: >>> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> >>> >>> Start computing the vlv/chv watermarks the atomic way, from the >>> .compute_pipe_wm() hook. We'll recompute the actual watermarks >>> for only planes that are part of the state, the other planes will >>> keep their watermark from the last time it was computed. >>> >>> And the actual watermark programming will happen from the >>> .initial_watermarks() hook. For now we'll just compute the >>> optimal watermarks, and we'll hook up the intermediate >>> watermarks properly later. >>> >>> The DSPARB registers responsible for the FIFO paritioning are >>> double buffered, so they will be programming from >>> intel_begin_crtc_commit(). >>> >>> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> >>> --- >>> drivers/gpu/drm/i915/i915_drv.h | 8 + >>> drivers/gpu/drm/i915/intel_display.c | 21 ++- >>> drivers/gpu/drm/i915/intel_drv.h | 2 - >>> drivers/gpu/drm/i915/intel_pm.c | 327 +++++++++++++++++++++++------------ >>> 4 files changed, 238 insertions(+), 120 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h >>> index 20bc04d5e617..f23698f99685 100644 >>> --- a/drivers/gpu/drm/i915/i915_drv.h >>> +++ b/drivers/gpu/drm/i915/i915_drv.h >>> @@ -493,6 +493,14 @@ struct i915_hotplug { >>> for ((domain) = 0; (domain) < POWER_DOMAIN_NUM; (domain)++) \ >>> for_each_if ((1 << (domain)) & (mask)) >>> >>> +#define for_each_intel_plane_in_state(__state, plane, plane_state, __i) \ >>> + for ((__i) = 0; \ >>> + (__i) < (__state)->base.dev->mode_config.num_total_plane && \ >>> + ((plane) = to_intel_plane((__state)->base.planes[__i].ptr), \ >>> + (plane_state) = to_intel_plane_state((__state)->base.planes[__i].state), 1); \ >>> + (__i)++) \ >>> + for_each_if (plane_state) >>> + >>> struct drm_i915_private; >>> struct i915_mm_struct; >>> struct i915_mmu_object; >>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >>> index 3f027341b0f3..8d80873b6643 100644 >>> --- a/drivers/gpu/drm/i915/intel_display.c >>> +++ b/drivers/gpu/drm/i915/intel_display.c >>> @@ -6736,6 +6736,8 @@ static void valleyview_modeset_commit_cdclk(struct drm_atomic_state *old_state) >>> static void valleyview_crtc_enable(struct intel_crtc_state *pipe_config, >>> struct drm_atomic_state *old_state) >>> { >>> + struct intel_atomic_state *old_intel_state = >>> + to_intel_atomic_state(old_state); >>> struct drm_crtc *crtc = pipe_config->base.crtc; >>> struct drm_device *dev = crtc->dev; >>> struct drm_i915_private *dev_priv = to_i915(dev); >>> @@ -6780,7 +6782,8 @@ static void valleyview_crtc_enable(struct intel_crtc_state *pipe_config, >>> >>> intel_color_load_luts(&pipe_config->base); >>> >>> - intel_update_watermarks(intel_crtc); >>> + dev_priv->display.initial_watermarks(old_intel_state, >>> + pipe_config); >>> intel_enable_pipe(intel_crtc); >>> >>> assert_vblank_disabled(crtc); >>> @@ -6897,6 +6900,9 @@ static void i9xx_crtc_disable(struct intel_crtc_state *old_crtc_state, >>> >>> if (!IS_GEN2(dev_priv)) >>> intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false); >>> + >>> + if (!dev_priv->display.initial_watermarks) >>> + intel_update_watermarks(intel_crtc); >>> } >>> >>> static void intel_crtc_disable_noatomic(struct drm_crtc *crtc) >>> @@ -12980,10 +12986,13 @@ static bool check_digital_port_conflicts(struct drm_atomic_state *state) >>> static void >>> clear_intel_crtc_state(struct intel_crtc_state *crtc_state) >>> { >>> + struct drm_i915_private *dev_priv = >>> + to_i915(crtc_state->base.crtc->dev); >>> struct drm_crtc_state tmp_state; >>> struct intel_crtc_scaler_state scaler_state; >>> struct intel_dpll_hw_state dpll_hw_state; >>> struct intel_shared_dpll *shared_dpll; >>> + struct intel_crtc_wm_state wm_state; >>> bool force_thru; >>> >>> /* FIXME: before the switch to atomic started, a new pipe_config was >>> @@ -12996,6 +13005,8 @@ clear_intel_crtc_state(struct intel_crtc_state *crtc_state) >>> shared_dpll = crtc_state->shared_dpll; >>> dpll_hw_state = crtc_state->dpll_hw_state; >>> force_thru = crtc_state->pch_pfit.force_thru; >>> + if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) >>> + wm_state = crtc_state->wm; >>> >>> memset(crtc_state, 0, sizeof *crtc_state); >>> >>> @@ -13004,6 +13015,8 @@ clear_intel_crtc_state(struct intel_crtc_state *crtc_state) >>> crtc_state->shared_dpll = shared_dpll; >>> crtc_state->dpll_hw_state = dpll_hw_state; >>> crtc_state->pch_pfit.force_thru = force_thru; >>> + if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) >>> + crtc_state->wm = wm_state; >> Is this required? In skl we throw away everything and rebuild using drm_atomic_crtc_state_for_each_plane_state. > I don't want to add all the planes into the state when not necessary. It doesn't. It uses the fact that updating planes requires crtc mutex, and only iterates over all planes in crtc_state->planes_mask. Any planes outside it are disabled so wm values can be considered 0, and roughly does the following: for_each_plane_in_mask(crtc_state->planes_mask) if ((!plane_state = drm_atomic_get_existing_state(state, plane_state)) plane_state = plane->state; plane_state must be const, but that's no problem for wm calculations. ~Maarten _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx