Re: [PATCH 83/89] drm/i915/skl: Check the DDB state at modeset

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Sep 04, 2014 at 03:27:49PM +0200, Daniel Vetter wrote:
> On Thu, Sep 04, 2014 at 12:27:49PM +0100, Damien Lespiau wrote:
> > v2: Don't check DDB on pre-SKL platforms
> >     Don't check DDB state on disabled pipes
> > 
> > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> 
> We probably want to do this with a vfunc eventually ... but reall
> deferring to Ville here, he'll know best what might be useful to make sure
> that the wm readout/takeover is accurate enough. So lgtm for now.

Yeah seems fine as is for now. On SKL checking the WMs too would be
fairly easy I guess. On ILK it'd be a bit more fuzzy due to the
(eventual) two part update + multi pipe merging stuff.

Doing it unconditionally at modeset time seems OK since it's a slow
operation anyway. We might want to do it for plane updates as well, but
I think that probably needs to be hidden behind some debug knob since we
don't want to pile on too much overhead there.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> -Daniel
> 
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h      |  9 +++++++
> >  drivers/gpu/drm/i915/intel_display.c | 51 ++++++++++++++++++++++++++++++++++++
> >  2 files changed, 60 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 95e57dc..0baf7f3 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1424,6 +1424,15 @@ static inline uint16_t skl_ddb_entry_size(const struct skl_ddb_entry *entry)
> >  	return entry->end - entry->start + 1;
> >  }
> >  
> > +static inline bool skl_ddb_entry_equal(const struct skl_ddb_entry *e1,
> > +				       const struct skl_ddb_entry *e2)
> > +{
> > +	if (e1->start == e2->start && e1->end == e2->end)
> > +		return true;
> > +
> > +	return false;
> > +}
> > +
> >  struct skl_ddb_allocation {
> >  	struct skl_ddb_entry plane[I915_MAX_PIPES][I915_MAX_PLANES];
> >  	struct skl_ddb_entry cursor[I915_MAX_PIPES];
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 9b31342..35d3389 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -10944,6 +10944,56 @@ intel_pipe_config_compare(struct drm_device *dev,
> >  	return true;
> >  }
> >  
> > +static void check_wm_state(struct drm_device *dev)
> > +{
> > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > +	struct skl_ddb_allocation hw_ddb, *sw_ddb;
> > +	struct intel_crtc *intel_crtc;
> > +	int plane;
> > +
> > +	if (INTEL_INFO(dev)->gen < 9)
> > +		return;
> > +
> > +	skl_ddb_get_hw_state(dev_priv, &hw_ddb);
> > +	sw_ddb = &dev_priv->wm.skl_hw.ddb;
> > +
> > +	for_each_intel_crtc(dev, intel_crtc) {
> > +		struct skl_ddb_entry *hw_entry, *sw_entry;
> > +		const enum pipe pipe = intel_crtc->pipe;
> > +
> > +		if (!intel_crtc->active)
> > +			continue;
> > +
> > +		/* planes */
> > +		for_each_plane(pipe, plane) {
> > +			hw_entry = &hw_ddb.plane[pipe][plane];
> > +			sw_entry = &sw_ddb->plane[pipe][plane];
> > +
> > +			if (skl_ddb_entry_equal(hw_entry, sw_entry))
> > +				continue;
> > +
> > +			DRM_ERROR("mismatch in DDB state pipe %c plane %d "
> > +				  "(expected (%u,%u), found (%u,%u))\n",
> > +				  pipe_name(pipe), plane + 1,
> > +				  sw_entry->start, sw_entry->end,
> > +				  hw_entry->start, hw_entry->end);
> > +		}
> > +
> > +		/* cursor */
> > +		hw_entry = &hw_ddb.cursor[pipe];
> > +		sw_entry = &sw_ddb->cursor[pipe];
> > +
> > +		if (skl_ddb_entry_equal(hw_entry, sw_entry))
> > +			continue;
> > +
> > +		DRM_ERROR("mismatch in DDB state pipe %c cursor "
> > +			  "(expected (%u,%u), found (%u,%u))\n",
> > +			  pipe_name(pipe),
> > +			  sw_entry->start, sw_entry->end,
> > +			  hw_entry->start, hw_entry->end);
> > +	}
> > +}
> > +
> >  static void
> >  check_connector_state(struct drm_device *dev)
> >  {
> > @@ -11143,6 +11193,7 @@ check_shared_dpll_state(struct drm_device *dev)
> >  void
> >  intel_modeset_check_state(struct drm_device *dev)
> >  {
> > +	check_wm_state(dev);
> >  	check_connector_state(dev);
> >  	check_encoder_state(dev);
> >  	check_crtc_state(dev);
> > -- 
> > 1.8.3.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@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
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx