From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [Intel-gfx] [PATCH] drm/i915: Handle untiled planes when computing their offsets Date: Thu, 21 Feb 2013 21:54:41 +0200 Message-ID: <20130221195441.GI4469@intel.com> References: <20130221165545.GH4469@intel.com> <1361473551-5775-1-git-send-email-chris@chris-wilson.co.uk> 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: <1361473551-5775-1-git-send-email-chris@chris-wilson.co.uk> Sender: stable-owner@vger.kernel.org To: Chris Wilson Cc: intel-gfx@lists.freedesktop.org, Daniel Vetter , stable@vger.kernel.org List-Id: intel-gfx@lists.freedesktop.org On Thu, Feb 21, 2013 at 07:05:51PM +0000, Chris Wilson wrote: > We trim the fb to fit the CRTC by computing the offset of that CRTC t= o > its nearest tile_row origin. This allows us to use framebuffers that = are > larger than the CRTC limits without additional work. >=20 > However, we failed to compute the offset for a linear framebuffer > correctly as we treated its x-advance in whole tiles (instead of the > linear increment expected), leaving the CRTC misaligned with its > contents. >=20 > Fixes regression from commit c2c75131244507c93f812862fdbd4f3a37139401 > Author: Daniel Vetter > Date: Thu Jul 5 12:17:30 2012 +0200 >=20 > drm/i915: adjust framebuffer base address on gen4+ >=20 > v2: Adjust relative x-coordinate after linear alignment (vsyrjala) >=20 > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=3D61152 > Signed-off-by: Chris Wilson > Cc: Daniel Vetter > Cc: stable@vger.kernel.org > --- > drivers/gpu/drm/i915/intel_display.c | 33 +++++++++++++++++++++++-= --------- > drivers/gpu/drm/i915/intel_drv.h | 3 ++- > drivers/gpu/drm/i915/intel_sprite.c | 6 ++++-- > 3 files changed, 29 insertions(+), 13 deletions(-) >=20 > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i= 915/intel_display.c > index a3ca9a8..c32c0dc 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -1906,18 +1906,29 @@ void intel_unpin_fb_obj(struct drm_i915_gem_o= bject *obj) > =20 > /* Computes the linear offset to the base tile and adjusts x, y. byt= es per pixel > * is assumed to be a power-of-two. */ > -unsigned long intel_gen4_compute_offset_xtiled(int *x, int *y, > - unsigned int bpp, > +unsigned long intel_gen4_compute_planar_offset(int *x, int *y, > + unsigned int tiling_mode, > + unsigned int cpp, > unsigned int pitch) > { > - int tile_rows, tiles; > + if (tiling_mode !=3D I915_TILING_NONE) { > + int tile_rows, tiles; > =20 > - tile_rows =3D *y / 8; > - *y %=3D 8; > - tiles =3D *x / (512/bpp); > - *x %=3D 512/bpp; > + tile_rows =3D *y / 8; > + *y %=3D 8; > =20 > - return tile_rows * pitch * 8 + tiles * 4096; > + tiles =3D *x / (512/cpp); > + *x %=3D 512/cpp; > + > + return tile_rows * pitch * 8 + tiles * 4096; > + } else { > + int offset; nit: unsigned > + > + offset =3D *y * pitch + *x * cpp; > + *y =3D 0; > + *x =3D (offset & 4095) / cpp; > + return offset & -4096; That should do it. There is one small concern though. HSW BSpec tells us that the offset + size mustn't exceed the maximum allowed sprite size. That's a bit weird, but perhaps it just means that whaever internal register that tracks the current x/y source positions is the same size as the sprite size register. Hmm, actually it seems I'm complaining about a non-issue. The max horizontal size seems to be 13 bits, which means 8k, and the max x offset from this functions is 2k. Given that displays are probably limited to 4k, that would still leave us 2k to spare. So I suppose we can just ignore this. I do have one other small bikeshed though. Why does the function name have planar in it? Maybe call it intel_gen4_compute_page_offset() or something? --=20 Ville Syrj=E4l=E4 Intel OTC