From: Robert Beckett <bob.beckett@collabora.com> To: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>, dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org, "Jani Nikula" <jani.nikula@linux.intel.com>, "Joonas Lahtinen" <joonas.lahtinen@linux.intel.com>, "Rodrigo Vivi" <rodrigo.vivi@intel.com>, "Tvrtko Ursulin" <tvrtko.ursulin@linux.intel.com>, "David Airlie" <airlied@linux.ie>, "Daniel Vetter" <daniel@ffwll.ch> Cc: Matthew Auld <matthew.auld@intel.com>, linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/4] drm/i915: add gen6 ppgtt dummy creation function Date: Mon, 23 May 2022 16:52:30 +0100 [thread overview] Message-ID: <bafcfe8f-3a19-a01c-8b46-9f2a44b1553c@collabora.com> (raw) In-Reply-To: <3d08eb595c4a4eff02be5385c82d1e1d0e781c98.camel@linux.intel.com> On 11/05/2022 11:13, Thomas Hellström wrote: > Hi, > > On Tue, 2022-05-03 at 19:13 +0000, Robert Beckett wrote: >> Internal gem objects will soon just be volatile system memory region >> objects. >> To enable this, create a separate dummy object creation function >> for gen6 ppgtt > > > It's not clear from the commit message why we need a special case for > this. Could you describe more in detail? it always was a special case, that used the internal backend but provided it's own ops, so actually had no benefit from using the internal backend. See b0b0f2d225da6fe58417fae37e3f797e2db27b62 I'll add some further explanation in the commit message for v2. > > Thanks, > Thomas > > >> >> Signed-off-by: Robert Beckett <bob.beckett@collabora.com> >> --- >> drivers/gpu/drm/i915/gt/gen6_ppgtt.c | 43 >> ++++++++++++++++++++++++++-- >> 1 file changed, 40 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/gt/gen6_ppgtt.c >> b/drivers/gpu/drm/i915/gt/gen6_ppgtt.c >> index 1bb766c79dcb..f3b660cfeb7f 100644 >> --- a/drivers/gpu/drm/i915/gt/gen6_ppgtt.c >> +++ b/drivers/gpu/drm/i915/gt/gen6_ppgtt.c >> @@ -372,6 +372,45 @@ static const struct drm_i915_gem_object_ops >> pd_dummy_obj_ops = { >> .put_pages = pd_dummy_obj_put_pages, >> }; >> >> +static struct drm_i915_gem_object * >> +i915_gem_object_create_dummy(struct drm_i915_private *i915, >> phys_addr_t size) >> +{ >> + static struct lock_class_key lock_class; >> + struct drm_i915_gem_object *obj; >> + unsigned int cache_level; >> + >> + GEM_BUG_ON(!size); >> + GEM_BUG_ON(!IS_ALIGNED(size, PAGE_SIZE)); >> + >> + if (overflows_type(size, obj->base.size)) >> + return ERR_PTR(-E2BIG); >> + >> + obj = i915_gem_object_alloc(); >> + if (!obj) >> + return ERR_PTR(-ENOMEM); >> + >> + drm_gem_private_object_init(&i915->drm, &obj->base, size); >> + i915_gem_object_init(obj, &pd_dummy_obj_ops, &lock_class, 0); >> + obj->mem_flags |= I915_BO_FLAG_STRUCT_PAGE; >> + >> + /* >> + * Mark the object as volatile, such that the pages are >> marked as >> + * dontneed whilst they are still pinned. As soon as they are >> unpinned >> + * they are allowed to be reaped by the shrinker, and the >> caller is >> + * expected to repopulate - the contents of this object are >> only valid >> + * whilst active and pinned. >> + */ >> + i915_gem_object_set_volatile(obj); >> + >> + obj->read_domains = I915_GEM_DOMAIN_CPU; >> + obj->write_domain = I915_GEM_DOMAIN_CPU; >> + >> + cache_level = HAS_LLC(i915) ? I915_CACHE_LLC : >> I915_CACHE_NONE; >> + i915_gem_object_set_cache_coherency(obj, cache_level); >> + >> + return obj; >> +} >> + >> static struct i915_page_directory * >> gen6_alloc_top_pd(struct gen6_ppgtt *ppgtt) >> { >> @@ -383,9 +422,7 @@ gen6_alloc_top_pd(struct gen6_ppgtt *ppgtt) >> if (unlikely(!pd)) >> return ERR_PTR(-ENOMEM); >> >> - pd->pt.base = __i915_gem_object_create_internal(ppgtt- >>> base.vm.gt->i915, >> - >> &pd_dummy_obj_o >> ps, >> - I915_PDES * >> SZ_4K); >> + pd->pt.base = i915_gem_object_create_dummy(ppgtt->base.vm.gt- >>> i915, I915_PDES * SZ_4K); >> if (IS_ERR(pd->pt.base)) { >> err = PTR_ERR(pd->pt.base); >> pd->pt.base = NULL; > >
WARNING: multiple messages have this Message-ID (diff)
From: Robert Beckett <bob.beckett@collabora.com> To: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>, dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org, "Jani Nikula" <jani.nikula@linux.intel.com>, "Joonas Lahtinen" <joonas.lahtinen@linux.intel.com>, "Rodrigo Vivi" <rodrigo.vivi@intel.com>, "Tvrtko Ursulin" <tvrtko.ursulin@linux.intel.com>, "David Airlie" <airlied@linux.ie>, "Daniel Vetter" <daniel@ffwll.ch> Cc: Matthew Auld <matthew.auld@intel.com>, linux-kernel@vger.kernel.org Subject: Re: [Intel-gfx] [PATCH 1/4] drm/i915: add gen6 ppgtt dummy creation function Date: Mon, 23 May 2022 16:52:30 +0100 [thread overview] Message-ID: <bafcfe8f-3a19-a01c-8b46-9f2a44b1553c@collabora.com> (raw) In-Reply-To: <3d08eb595c4a4eff02be5385c82d1e1d0e781c98.camel@linux.intel.com> On 11/05/2022 11:13, Thomas Hellström wrote: > Hi, > > On Tue, 2022-05-03 at 19:13 +0000, Robert Beckett wrote: >> Internal gem objects will soon just be volatile system memory region >> objects. >> To enable this, create a separate dummy object creation function >> for gen6 ppgtt > > > It's not clear from the commit message why we need a special case for > this. Could you describe more in detail? it always was a special case, that used the internal backend but provided it's own ops, so actually had no benefit from using the internal backend. See b0b0f2d225da6fe58417fae37e3f797e2db27b62 I'll add some further explanation in the commit message for v2. > > Thanks, > Thomas > > >> >> Signed-off-by: Robert Beckett <bob.beckett@collabora.com> >> --- >> drivers/gpu/drm/i915/gt/gen6_ppgtt.c | 43 >> ++++++++++++++++++++++++++-- >> 1 file changed, 40 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/gt/gen6_ppgtt.c >> b/drivers/gpu/drm/i915/gt/gen6_ppgtt.c >> index 1bb766c79dcb..f3b660cfeb7f 100644 >> --- a/drivers/gpu/drm/i915/gt/gen6_ppgtt.c >> +++ b/drivers/gpu/drm/i915/gt/gen6_ppgtt.c >> @@ -372,6 +372,45 @@ static const struct drm_i915_gem_object_ops >> pd_dummy_obj_ops = { >> .put_pages = pd_dummy_obj_put_pages, >> }; >> >> +static struct drm_i915_gem_object * >> +i915_gem_object_create_dummy(struct drm_i915_private *i915, >> phys_addr_t size) >> +{ >> + static struct lock_class_key lock_class; >> + struct drm_i915_gem_object *obj; >> + unsigned int cache_level; >> + >> + GEM_BUG_ON(!size); >> + GEM_BUG_ON(!IS_ALIGNED(size, PAGE_SIZE)); >> + >> + if (overflows_type(size, obj->base.size)) >> + return ERR_PTR(-E2BIG); >> + >> + obj = i915_gem_object_alloc(); >> + if (!obj) >> + return ERR_PTR(-ENOMEM); >> + >> + drm_gem_private_object_init(&i915->drm, &obj->base, size); >> + i915_gem_object_init(obj, &pd_dummy_obj_ops, &lock_class, 0); >> + obj->mem_flags |= I915_BO_FLAG_STRUCT_PAGE; >> + >> + /* >> + * Mark the object as volatile, such that the pages are >> marked as >> + * dontneed whilst they are still pinned. As soon as they are >> unpinned >> + * they are allowed to be reaped by the shrinker, and the >> caller is >> + * expected to repopulate - the contents of this object are >> only valid >> + * whilst active and pinned. >> + */ >> + i915_gem_object_set_volatile(obj); >> + >> + obj->read_domains = I915_GEM_DOMAIN_CPU; >> + obj->write_domain = I915_GEM_DOMAIN_CPU; >> + >> + cache_level = HAS_LLC(i915) ? I915_CACHE_LLC : >> I915_CACHE_NONE; >> + i915_gem_object_set_cache_coherency(obj, cache_level); >> + >> + return obj; >> +} >> + >> static struct i915_page_directory * >> gen6_alloc_top_pd(struct gen6_ppgtt *ppgtt) >> { >> @@ -383,9 +422,7 @@ gen6_alloc_top_pd(struct gen6_ppgtt *ppgtt) >> if (unlikely(!pd)) >> return ERR_PTR(-ENOMEM); >> >> - pd->pt.base = __i915_gem_object_create_internal(ppgtt- >>> base.vm.gt->i915, >> - >> &pd_dummy_obj_o >> ps, >> - I915_PDES * >> SZ_4K); >> + pd->pt.base = i915_gem_object_create_dummy(ppgtt->base.vm.gt- >>> i915, I915_PDES * SZ_4K); >> if (IS_ERR(pd->pt.base)) { >> err = PTR_ERR(pd->pt.base); >> pd->pt.base = NULL; > >
next prev parent reply other threads:[~2022-05-23 15:53 UTC|newest] Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-05-03 19:13 [PATCH 0/4] ttm for internal Robert Beckett 2022-05-03 19:13 ` [Intel-gfx] " Robert Beckett 2022-05-03 19:13 ` [PATCH 1/4] drm/i915: add gen6 ppgtt dummy creation function Robert Beckett 2022-05-03 19:13 ` [Intel-gfx] " Robert Beckett 2022-05-03 19:13 ` Robert Beckett 2022-05-11 10:13 ` Thomas Hellström 2022-05-11 10:13 ` [Intel-gfx] " Thomas Hellström 2022-05-23 15:52 ` Robert Beckett [this message] 2022-05-23 15:52 ` Robert Beckett 2022-05-03 19:13 ` [PATCH 2/4] drm/i915: setup ggtt scratch page after memory regions Robert Beckett 2022-05-03 19:13 ` [Intel-gfx] " Robert Beckett 2022-05-03 19:13 ` Robert Beckett 2022-05-11 11:24 ` Thomas Hellström 2022-05-11 11:24 ` [Intel-gfx] " Thomas Hellström 2022-05-03 19:13 ` [PATCH 3/4] drm/i915: allow volatile buffers to use ttm pool allocator Robert Beckett 2022-05-03 19:13 ` [Intel-gfx] " Robert Beckett 2022-05-03 19:13 ` Robert Beckett 2022-05-11 12:42 ` Thomas Hellström 2022-05-11 12:42 ` [Intel-gfx] " Thomas Hellström 2022-05-23 15:53 ` Robert Beckett 2022-05-23 15:53 ` [Intel-gfx] " Robert Beckett 2022-05-03 19:13 ` [PATCH 4/4] drm/i915: internal buffers use ttm backend Robert Beckett 2022-05-03 19:13 ` [Intel-gfx] " Robert Beckett 2022-05-03 19:13 ` Robert Beckett 2022-05-11 14:14 ` Thomas Hellström 2022-05-11 14:14 ` [Intel-gfx] " Thomas Hellström 2022-05-23 15:52 ` Robert Beckett 2022-05-23 15:52 ` [Intel-gfx] " Robert Beckett 2022-05-03 19:52 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for ttm for internal Patchwork 2022-05-06 3:44 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for ttm for internal (rev2) Patchwork 2022-05-10 21:26 ` [Intel-gfx] ✓ Fi.CI.BAT: success for ttm for internal (rev3) Patchwork 2022-05-11 2:28 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=bafcfe8f-3a19-a01c-8b46-9f2a44b1553c@collabora.com \ --to=bob.beckett@collabora.com \ --cc=airlied@linux.ie \ --cc=daniel@ffwll.ch \ --cc=dri-devel@lists.freedesktop.org \ --cc=intel-gfx@lists.freedesktop.org \ --cc=jani.nikula@linux.intel.com \ --cc=joonas.lahtinen@linux.intel.com \ --cc=linux-kernel@vger.kernel.org \ --cc=matthew.auld@intel.com \ --cc=rodrigo.vivi@intel.com \ --cc=thomas.hellstrom@linux.intel.com \ --cc=tvrtko.ursulin@linux.intel.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.