From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 46/89] drm/i915/skl: Add DDB allocation management structures Date: Mon, 22 Sep 2014 21:26:32 +0300 Message-ID: <20140922182632.GV12416@intel.com> References: <1409830075-11139-1-git-send-email-damien.lespiau@intel.com> <1409830075-11139-47-git-send-email-damien.lespiau@intel.com> <20140917104754.GK12416@intel.com> <20140922140827.GM10951@strange.ger.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTP id E70D0899E6 for ; Mon, 22 Sep 2014 11:26:50 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20140922140827.GM10951@strange.ger.corp.intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Damien Lespiau Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Mon, Sep 22, 2014 at 03:08:27PM +0100, Damien Lespiau wrote: > On Wed, Sep 17, 2014 at 01:47:54PM +0300, Ville Syrj=E4l=E4 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 > > > --- > > > 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/i= 915_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 =3D=3D 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 adjust= ments > > 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? I'd still change to exclusive. Inclusive is just too troublesome to use IMO, you're bound to forget the magic +/-1 somewhere eventually. -- = Ville Syrj=E4l=E4 Intel OTC