Re: [PATCH 46/89] drm/i915/skl: Add DDB allocation management structures

Damien Lespiau
2014-09-22T14:09:03Z

On Wed, Sep 17, 2014 at 01:47:54PM +0300, Ville Syrjälä wrote:
> On Thu, Sep 04, 2014 at 12:27:12PM +0100, Damien Lespiau wrote:
> > We now need to allocate space in the DDB for planes being scanned out
> > ourselves. The data structure to represent an allocation mirrors what
> > we'll need to write in the registers later on: (start, end).
> >
> > We add that allocation datat to the skl_wm_values structure as part of
> > the values to program the hardware with.
> >
> > v2: Split planes and cursor for consistency.
> >
> > v3: Make the skl_ddb_entry_size() parameter const
> >
> > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_drv.h | 19 +++++++++++++++++++
> > 1 file changed, 19 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 3764ad5..de278a5 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1381,8 +1381,27 @@ struct ilk_wm_values {
> > enum intel_ddb_partitioning partitioning;
> > };
> >
> > +struct skl_ddb_entry {
> > + uint16_t start, end; /* in number of blocks */
> > +};
> > +
> > +static inline uint16_t skl_ddb_entry_size(const struct skl_ddb_entry *entry)
> > +{
> > + /* end not set, clearly no allocation here. start can be 0 though */
> > + if (entry->end == 0)
> > + return 0;
> > +
> > + return entry->end - entry->start + 1;
>
> I would make 'end' exclusive as that allows you to represent an empty
> block more naturally, and you don't have to worry about the +/-1 adjustments
> when going between start/end and start/size representation.

The values we program in the registers are inclusive (I had a bug where
the end of one plane and the start of another plane had the same value
and artefacts could be seen, due to the pipes fetching from overlapping
DDB space). I took the option of having those values mirror what we need
to program. Hopefully the small functions around the skl_ddb_entry
structure doesn't make it too bad?

--
Damien