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

Ville Syrjälä
2014-10-29T19:16:51Z

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