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

Damien Lespiau
2014-09-27T14:18:02Z

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.

--
Damien