From mboxrd@z Thu Jan 1 00:00:00 1970 From: Imre Deak Subject: Re: [PATCH 10/9] drm/i915/bdw: Kill ppgtt->num_pt_pages Date: Mon, 24 Feb 2014 19:17:02 +0200 Message-ID: <1393262222.13131.131.camel@intelbox> References: <1392876349-24684-1-git-send-email-benjamin.widawsky@intel.com> <1393016794-4588-1-git-send-email-benjamin.widawsky@intel.com> Reply-To: imre.deak@intel.com Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1261673445==" Return-path: Received: from mga03.intel.com (mga03.intel.com [143.182.124.21]) by gabe.freedesktop.org (Postfix) with ESMTP id 6DCAEFAB6E for ; Mon, 24 Feb 2014 09:17:32 -0800 (PST) In-Reply-To: <1393016794-4588-1-git-send-email-benjamin.widawsky@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org To: Ben Widawsky Cc: Intel GFX , Ben Widawsky List-Id: intel-gfx@lists.freedesktop.org --===============1261673445== Content-Type: multipart/signed; micalg="pgp-sha1"; protocol="application/pgp-signature"; boundary="=-N6HjquyR2pt1qECiGDM4" --=-N6HjquyR2pt1qECiGDM4 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Fri, 2014-02-21 at 13:06 -0800, Ben Widawsky wrote: > With the original PPGTT implementation if the number of PDPs was not a > power of two, the number of pages for the page tables would end up being > rounded up. The code actually had a bug here afaict, but this is a > theoretical bug as I don't believe this can actually occur with the > current code/HW.. >=20 > With the rework of the page table allocations, there is no longer a > distinction between number of page table pages, and number of page > directory entries. To avoid confusion, kill the redundant (and newer) > struct member. >=20 > Cc: Imre Deak > Signed-off-by: Ben Widawsky Nitpick: keeping num_pt_pages instead would make the code more understandable to me and symmetric with num_pd_pages, but that would've been much more churn. In any case nice simplification, Reviewed-by: Imre Deak > --- > drivers/gpu/drm/i915/i915_debugfs.c | 2 +- > drivers/gpu/drm/i915/i915_drv.h | 3 +-- > drivers/gpu/drm/i915/i915_gem_gtt.c | 14 ++++---------- > 3 files changed, 6 insertions(+), 13 deletions(-) >=20 > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i= 915_debugfs.c > index 509e2e1..e0c42a6 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -1757,7 +1757,7 @@ static void gen8_ppgtt_info(struct seq_file *m, str= uct drm_device *dev) > return; > =20 > seq_printf(m, "Page directories: %d\n", ppgtt->num_pd_pages); > - seq_printf(m, "Page tables: %d\n", ppgtt->num_pt_pages); > + seq_printf(m, "Page tables: %d\n", ppgtt->num_pd_entries); > for_each_ring(ring, dev_priv, unused) { > seq_printf(m, "%s\n", ring->name); > for (i =3D 0; i < 4; i++) { > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_= drv.h > index 2f29558..a9f1cae 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -698,13 +698,12 @@ struct i915_hw_ppgtt { > struct kref ref; > struct drm_mm_node node; > unsigned num_pd_entries; > + unsigned num_pd_pages; /* gen8+ */ > union { > struct page **pt_pages; > struct page **gen8_pt_pages[GEN8_LEGACY_PDPS]; > }; > struct page *pd_pages; > - int num_pd_pages; > - int num_pt_pages; > union { > uint32_t pd_offset; > dma_addr_t pd_dma_addr[GEN8_LEGACY_PDPS]; > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i= 915_gem_gtt.c > index 6c03929..bd815d7 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -433,7 +433,6 @@ static int gen8_ppgtt_allocate_page_tables(struct i91= 5_hw_ppgtt *ppgtt, > const int max_pdp) > { > struct page **pt_pages[GEN8_LEGACY_PDPS]; > - const int num_pt_pages =3D GEN8_PDES_PER_PAGE * max_pdp; > int i, ret; > =20 > for (i =3D 0; i < max_pdp; i++) { > @@ -450,8 +449,6 @@ static int gen8_ppgtt_allocate_page_tables(struct i91= 5_hw_ppgtt *ppgtt, > for (i =3D 0; i < max_pdp; i++) > ppgtt->gen8_pt_pages[i] =3D pt_pages[i]; > =20 > - ppgtt->num_pt_pages =3D 1 << get_order(num_pt_pages << PAGE_SHIFT); > - > return 0; > =20 > unwind_out: > @@ -618,18 +615,15 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *pp= gtt, uint64_t size) > ppgtt->base.insert_entries =3D gen8_ppgtt_insert_entries; > ppgtt->base.cleanup =3D gen8_ppgtt_cleanup; > ppgtt->base.start =3D 0; > - ppgtt->base.total =3D ppgtt->num_pt_pages * GEN8_PTES_PER_PAGE * PAGE_S= IZE; > + ppgtt->base.total =3D ppgtt->num_pd_entries * GEN8_PTES_PER_PAGE * PAGE= _SIZE; > =20 > - ppgtt->base.clear_range(&ppgtt->base, 0, > - ppgtt->num_pd_entries * GEN8_PTES_PER_PAGE * PAGE_SIZE, > - true); > + ppgtt->base.clear_range(&ppgtt->base, 0, ppgtt->base.total, true); > =20 > DRM_DEBUG_DRIVER("Allocated %d pages for page directories (%d wasted)\n= ", > ppgtt->num_pd_pages, ppgtt->num_pd_pages - max_pdp); > DRM_DEBUG_DRIVER("Allocated %d pages for page tables (%lld wasted)\n", > - ppgtt->num_pt_pages, > - (ppgtt->num_pt_pages - min_pt_pages) + > - size % (1<<30)); > + ppgtt->num_pd_entries, > + (ppgtt->num_pd_entries - min_pt_pages) + size % (1<<30)); > return 0; > =20 > bail: --=-N6HjquyR2pt1qECiGDM4 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.14 (GNU/Linux) iQEcBAABAgAGBQJTC36OAAoJEORIIAnNuWDF690IAL5abX5oXVxus6onEqCrMYf8 /aEDLUC44rPn+5TABwoCfZPGDIr5KtJ6aGZT43oaQ9UpbuUsa1rdN5nI2Z5/kimx OGysbfrxGzOq+dSEAK2+buhABDNsZ/g5LqoyPXN8qD2rNS651WGj9KdXo4rFIK/9 2xjL+vsJ1fFA9Uta3RCzZilkg9pR8A3f1aBbd9ryfbzhcMY6UpyE6lU5ye2r5YQD FP7hMFEfnlJyLOQa2/biOyh0edsTa+sKzb0DJzfg5MeQPf4lF5wjE35qRZRm28ER /DHiqr6rSTDBUlVoBvhC+v7aqj97KXXdut+u/0D2krSy8ffNURY+I6Po2F/H/WU= =e4DH -----END PGP SIGNATURE----- --=-N6HjquyR2pt1qECiGDM4-- --===============1261673445== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx --===============1261673445==--