From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [Intel-gfx] [PATCH 1/4] drm: Add struct drm_region and assorted utility functions Date: Wed, 6 Mar 2013 13:10:51 +0200 Message-ID: <20130306111051.GB4469@intel.com> References: <1361482503-3268-1-git-send-email-ville.syrjala@linux.intel.com> <1361482503-3268-2-git-send-email-ville.syrjala@linux.intel.com> <20130306105605.GA32736@cantiga.alporthouse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Content-Disposition: inline In-Reply-To: <20130306105605.GA32736@cantiga.alporthouse.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: Chris Wilson , intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org On Wed, Mar 06, 2013 at 10:56:05AM +0000, Chris Wilson wrote: > On Thu, Feb 21, 2013 at 11:35:00PM +0200, ville.syrjala@linux.intel.com w= rote: > > From: Ville Syrj=E4l=E4 > > = > > struct drm_region represents a two dimensional region. The utility > > functions are there to help driver writers. > > = > > v2: Moved the region stuff into its own file, made the smaller funcs > > static inline, used 64bit maths in the scaled clipping function to > > avoid overflows (instead it will saturate to INT_MIN or INT_MAX). > > = > > Signed-off-by: Ville Syrj=E4l=E4 > = > My criticisms here are mainly that the function names are different than > what I am used to for a region API. Namely drm_region_clip, > drm_region_subsample would more conventionally be drm_region_intersect, > drm_region_downscale. And that a region to me is an ordered list of > rectangles, whereas here you just have a single rectangle. I'm fine with renaming stuff. Ideally the names should be so obvious that people never have to look at the documentation. -- = Ville Syrj=E4l=E4 Intel OTC