From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boszormenyi Zoltan Subject: Re: [PATCH] drm/radeon: fix tiling and command stream checking on evergreen v3 Date: Sat, 09 Jun 2012 18:15:00 +0200 Message-ID: <4FD37684.7060505@pr.hu> References: <1339253861-1765-1-git-send-email-j.glisse@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1"; Format="flowed" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail.pr.hu (mail.pr.hu [87.242.0.5]) by gabe.freedesktop.org (Postfix) with ESMTP id 857BE9EFBB for ; Sat, 9 Jun 2012 09:44:42 -0700 (PDT) Received: from host-87-242-63-98.prtelecom.hu ([87.242.63.98] helo=localhost.localdomain) by mail.pr.hu with esmtpsa (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.69) (envelope-from ) id 1SdOJd-0002yL-Nd for dri-devel@lists.freedesktop.org; Sat, 09 Jun 2012 18:15:19 +0200 In-Reply-To: <1339253861-1765-1-git-send-email-j.glisse@gmail.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: dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org 2012-06-09 16:57 keltez=E9ssel, j.glisse@gmail.com =EDrta: > From: Jerome Glisse > > Fix regresson since the introduction of command stream checking on > evergreen (thread referenced below). Issue is cause by ddx allocating > bo with formula width*height*bpp while programming the GPU command > stream with ALIGN(height, 8). In some case (where page alignment does > not hide the extra size bo should be according to height alignment) > the kernel will reject the command stream. > > This patch reprogram the command stream to slice - 1 (slice is > a derivative value from height) which avoid rejecting the command > stream while keeping the value of command stream checking from a > security point of view. > > This patch also fix wrong computation of layer size for 2D tiled > surface. Which should fix issue when 2D color tiling is enabled. > This dump the radeon KMS_DRIVER_MINOR so userspace can know if > they are on a fixed kernel or not. > > https://lkml.org/lkml/2012/6/3/80 > https://bugs.freedesktop.org/show_bug.cgi?id=3D50892 > https://bugs.freedesktop.org/show_bug.cgi?id=3D50857 > > !!! STABLE need a custom version of this patch for 3.4 !!! > > v2: actually bump the minor version and add comment about stable > v3: do compute the height the ddx was trying to use > > Signed-off-by: Jerome Glisse > --- > drivers/gpu/drm/radeon/evergreen_cs.c | 50 ++++++++++++++++++++++++++= ++++--- > drivers/gpu/drm/radeon/radeon_drv.c | 3 +- > 2 files changed, 48 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/evergreen_cs.c b/drivers/gpu/drm/rade= on/evergreen_cs.c > index 4e7dd2b..29c43c6 100644 > --- a/drivers/gpu/drm/radeon/evergreen_cs.c > +++ b/drivers/gpu/drm/radeon/evergreen_cs.c > @@ -52,6 +52,7 @@ struct evergreen_cs_track { > u32 cb_color_view[12]; > u32 cb_color_pitch[12]; > u32 cb_color_slice[12]; > + u32 cb_color_slice_idx[12]; > u32 cb_color_attrib[12]; > u32 cb_color_cmask_slice[8];/* unused */ > u32 cb_color_fmask_slice[8];/* unused */ > @@ -127,12 +128,14 @@ static void evergreen_cs_track_init(struct evergree= n_cs_track *track) > track->cb_color_info[i] =3D 0; > track->cb_color_view[i] =3D 0xFFFFFFFF; > track->cb_color_pitch[i] =3D 0; > - track->cb_color_slice[i] =3D 0; > + track->cb_color_slice[i] =3D 0xfffffff; > + track->cb_color_slice_idx[i] =3D 0; > } > track->cb_target_mask =3D 0xFFFFFFFF; > track->cb_shader_mask =3D 0xFFFFFFFF; > track->cb_dirty =3D true; > > + track->db_depth_slice =3D 0xffffffff; > track->db_depth_view =3D 0xFFFFC000; > track->db_depth_size =3D 0xFFFFFFFF; > track->db_depth_control =3D 0xFFFFFFFF; > @@ -250,10 +253,9 @@ static int evergreen_surface_check_2d(struct radeon_= cs_parser *p, > { > struct evergreen_cs_track *track =3D p->track; > unsigned palign, halign, tileb, slice_pt; > + unsigned mtile_pr, mtile_ps, mtileb; > > tileb =3D 64 * surf->bpe * surf->nsamples; > - palign =3D track->group_size / (8 * surf->bpe * surf->nsamples); > - palign =3D MAX(8, palign); > slice_pt =3D 1; > if (tileb> surf->tsplit) { > slice_pt =3D tileb / surf->tsplit; > @@ -262,7 +264,10 @@ static int evergreen_surface_check_2d(struct radeon_= cs_parser *p, > /* macro tile width& height */ > palign =3D (8 * surf->bankw * track->npipes) * surf->mtilea; > halign =3D (8 * surf->bankh * surf->nbanks) / surf->mtilea; > - surf->layer_size =3D surf->nbx * surf->nby * surf->bpe * slice_pt; > + mtileb =3D (palign / 8) * (halign / 8) * tileb;; > + mtile_pr =3D surf->nbx / palign; > + mtile_ps =3D (mtile_pr * surf->nby) / halign; > + surf->layer_size =3D mtile_ps * mtileb * slice_pt; > surf->base_align =3D (palign / 8) * (halign / 8) * tileb; > surf->palign =3D palign; > surf->halign =3D halign; > @@ -434,6 +439,39 @@ static int evergreen_cs_track_validate_cb(struct rad= eon_cs_parser *p, unsigned i > > offset +=3D surf.layer_size * mslice; > if (offset> radeon_bo_size(track->cb_color_bo[id])) { > + /* old ddx are broken they allocate bo with w*h*bpp but > + * program slice with ALIGN(h, 8), catch this and patch > + * command stream. > + */ > + if (!surf.mode) { > + volatile u32 *ib =3D p->ib.ptr; > + unsigned long tmp, nby, bsize, size, min =3D 0; > + > + /* find the height the ddx wants */ > + if (surf.nby> 8) { > + min =3D surf.nby - 8; > + } > + bsize =3D radeon_bo_size(track->cb_color_bo[id]); > + tmp =3D track->cb_color_bo_offset[id]<< 8; > + for (nby =3D surf.nby; nby> min; nby--) { > + size =3D nby * surf.nbx * surf.bpe * surf.nsamples; > + if ((tmp + size * mslice)<=3D bsize) { > + break; > + } > + } > + if (nby> min) { > + surf.nby =3D nby; > + slice =3D ((nby * surf.nbx) / 64) - 1; > + if (!evergreen_surface_check(p,&surf, "cb")) { > + /* check if this one works */ > + tmp +=3D surf.layer_size * mslice; > + if (tmp<=3D bsize) { > + ib[track->cb_color_slice_idx[id]] =3D slice; > + goto old_ddx_ok; > + } > + } > + } > + } > dev_warn(p->dev, "%s:%d cb[%d] bo too small (layer size %d, " > "offset %d, max layer %d, bo size %ld, slice %d)\n", > __func__, __LINE__, id, surf.layer_size, > @@ -446,6 +484,7 @@ static int evergreen_cs_track_validate_cb(struct rade= on_cs_parser *p, unsigned i > surf.tsplit, surf.mtilea); > return -EINVAL; > } > +old_ddx_ok: > > return 0; > } > @@ -646,6 +685,7 @@ static int evergreen_cs_track_validate_depth(struct r= adeon_cs_parser *p) > track->db_depth_slice, track->db_z_info); > return r; > } > +DRM_INFO("%s %d surface nbx,y (%d %d) mode %d format %d surf.tsplit %d b= anks (%d %d %d) tilea %d\n", __func__, __LINE__, surf.nbx, surf.nby, surf.m= ode, surf.format, surf.tsplit, surf.nbanks, surf.bankw, surf.bankh, surf.mt= ilea); This DRM_INFO() is very chatty. > > r =3D evergreen_surface_check(p,&surf, "depth"); > if (r) { > @@ -1532,6 +1572,7 @@ static int evergreen_cs_check_reg(struct radeon_cs_= parser *p, u32 reg, u32 idx) > case CB_COLOR7_SLICE: > tmp =3D (reg - CB_COLOR0_SLICE) / 0x3c; > track->cb_color_slice[tmp] =3D radeon_get_ib_value(p, idx); > + track->cb_color_slice_idx[tmp] =3D idx; > track->cb_dirty =3D true; > break; > case CB_COLOR8_SLICE: > @@ -1540,6 +1581,7 @@ static int evergreen_cs_check_reg(struct radeon_cs_= parser *p, u32 reg, u32 idx) > case CB_COLOR11_SLICE: > tmp =3D ((reg - CB_COLOR8_SLICE) / 0x1c) + 8; > track->cb_color_slice[tmp] =3D radeon_get_ib_value(p, idx); > + track->cb_color_slice_idx[tmp] =3D idx; > track->cb_dirty =3D true; > break; > case CB_COLOR0_ATTRIB: > diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon= /radeon_drv.c > index f0bb2b5..03e5f5d 100644 > --- a/drivers/gpu/drm/radeon/radeon_drv.c > +++ b/drivers/gpu/drm/radeon/radeon_drv.c > @@ -57,9 +57,10 @@ > * 2.13.0 - virtual memory support, streamout > * 2.14.0 - add evergreen tiling informations > * 2.15.0 - add max_pipes query > + * 2.16.0 - fix evergreen 2D tiled surface calculation > */ > #define KMS_DRIVER_MAJOR 2 > -#define KMS_DRIVER_MINOR 15 > +#define KMS_DRIVER_MINOR 16 > #define KMS_DRIVER_PATCHLEVEL 0 > int radeon_driver_load_kms(struct drm_device *dev, unsigned long flags); > int radeon_driver_unload_kms(struct drm_device *dev);