Re: [PATCH 49/89] drm/i915/skl: Program the DDB allocation

Ville Syrjälä
2014-10-29T18:43:14Z

On Sat, Sep 27, 2014 at 03:17:58PM +0100, Damien Lespiau wrote:
> On Fri, Sep 19, 2014 at 01:03:15PM +0300, Ville Syrjälä wrote:
> > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > > index 0ddcbad..756ff16 100644
> > > --- a/drivers/gpu/drm/i915/intel_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > > @@ -3393,6 +3393,15 @@ static void skl_write_wm_values(struct drm_i915_private *dev_priv,
> > > I915_WRITE(PLANE_WM_TRANS(pipe, i),
> > > new->plane_trans[pipe][i]);
> > > I915_WRITE(CUR_WM_TRANS(pipe), new->cursor_trans[pipe]);
> > > +
> > > + for (i = 0; i < intel_num_planes(crtc); i++)
> > > + I915_WRITE(PLANE_BUF_CFG(pipe, i),
> > > + new->ddb.plane[pipe][i].end << 16 |
> >
> > The spec doesn't say if end is inclusive or exclusive. Someone actually
> > confirmed that from someone or tested it?
>
> I remember testing it and having the end of a plane match the start of
> another produces artefacts.
>
> > > + new->ddb.plane[pipe][i].start);
> > > +
> > > + I915_WRITE(CUR_BUF_CFG(pipe),
> > > + new->ddb.cursor[pipe].end << 16 |
> > > + new->ddb.cursor[pipe].start);
> > > }
> > > }
> >
> > And here we land in deep doodoo wrt. changing the DDB allocation
> > for active pipes. Someone needs to implement the correct logic to
> > sequence the DDB allocation changes so that the allocations don't
> > overlap at any point in time.
> >
> > So this needs a big FIXME at the very least.
>
> As seen on IRC, the DDB regs are double buffered and the "flush" patch
> is supposed to take care of that.

Just went through the new and improved flush patch. The algorithm is
solid now, just need to keep moving it towards the atomic stuff, but in
the meantime I think we can move forward with this stuff.

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

--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org