Just posting this so it's on the ML. NAK on this patch, this causes leftover cursors on SKL. Additionally, there's a noticeable lag every time I move the cursor from one monitor to another, and the flickering from the broken watermarks still happens. On Thu, 2016-07-07 at 11:37 -0700, Matt Roper wrote: > When we write watermark values to the hardware, those values are stored > in dev_priv->wm.skl_hw. However with recent watermark changes, the > results structure we're copying from only contains valid watermark and > DDB values for the pipes that are actually changing; the values for > other pipes remain 0. Thus a blind copy of the entire skl_wm_values > structure will clobber the values for unchanged pipes...we need to be > more selective and only copy over the values for the changing pipes. > > This mistake was hidden until recently due to another bug that caused us > to erroneously re-calculate watermarks for all active pipes rather than > changing pipes. Only when that bug was fixed was the impact of this bug > discovered (e.g., modesets failing with "Requested display configuration > exceeds system watermark limitations" messages and leaving watermarks > non-functional, even ones initiated by intel_fbdev_restore_mode). > > Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> > Cc: Lyude Paul <cpaul@xxxxxxxxxx> > Fixes: 734fa01f3a17 ("drm/i915/gen9: Calculate watermarks during atomic > 'check' (v2)") > Fixes: 9b6130227495 ("drm/i915/gen9: Re-allocate DDB only for changed pipes") > Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx> > --- > drivers/gpu/drm/i915/intel_pm.c | 22 ++++++++++++++++++++-- > 1 file changed, 20 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 5a8ee0c..2913535 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -4018,8 +4018,10 @@ static void skl_update_wm(struct drm_crtc *crtc) > struct drm_device *dev = crtc->dev; > struct drm_i915_private *dev_priv = to_i915(dev); > struct skl_wm_values *results = &dev_priv->wm.skl_results; > + struct skl_wm_values *hw_vals = &dev_priv->wm.skl_hw; > struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state); > struct skl_pipe_wm *pipe_wm = &cstate->wm.skl.optimal; > + int pipe; > > if ((results->dirty_pipes & drm_crtc_mask(crtc)) == 0) > return; > @@ -4031,8 +4033,24 @@ static void skl_update_wm(struct drm_crtc *crtc) > skl_write_wm_values(dev_priv, results); > skl_flush_wm_values(dev_priv, results); > > - /* store the new configuration */ > - dev_priv->wm.skl_hw = *results; > + /* > + * Store the new configuration (but only for the pipes that have > + * changed; the other values weren't recomputed). > + */ > + for_each_pipe_masked(dev_priv, pipe, results->dirty_pipes) { > + hw_vals->wm_linetime[pipe] = results->wm_linetime[pipe]; > + hw_vals->ddb.pipe[pipe] = results->ddb.pipe[pipe]; > + memcpy(&hw_vals->ddb.plane[pipe], &results->ddb.plane[pipe], > + sizeof(hw_vals->ddb.plane[pipe])); > + memcpy(&hw_vals->ddb.y_plane[pipe], &results- > >ddb.y_plane[pipe], > + sizeof(hw_vals->ddb.y_plane[pipe])); > + memcpy(&hw_vals->plane[pipe], > + &results->plane[pipe], > + sizeof(results->plane[pipe])); > + memcpy(&hw_vals->plane_trans[pipe], > + &results->plane_trans[pipe], > + sizeof(results->plane_trans[pipe])); > + } > > mutex_unlock(&dev_priv->wm.wm_mutex); > } -- Cheers, Lyude _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx