| Message ID | 1428711656-5878-1-git-send-email-anuj.phogat@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Apr 10, 2015 at 5:20 PM, Anuj Phogat <anuj.phogat@gmail.com> wrote: > and use it to initialize the align variable in drm_intel_bo. > > In case of YF/YS tiled buffers libdrm need not know about the tiling > format because these buffers don't have hardware support to be tiled > or detiled through a fenced region. But, libdrm still need to know > about buffer alignment restrictions because kernel uses it when > resolving the relocation. > > Mesa uses drm_intel_gem_bo_alloc_for_render() to allocate Yf/Ys buffers. > So, use the passed alignment value in this function. Note that we continue > ignoring the alignment value passed to drm_intel_gem_bo_alloc() to follow > the previous behavior. > > Cc: Kristian Høgsberg <krh@bitplanet.net> > Cc: Damien Lespiau <damien.lespiau@intel.com> > Cc: Daniel Vetter <daniel@ffwll.ch> > Signed-off-by: Anuj Phogat <anuj.phogat@gmail.com> > --- > intel/intel_bufmgr_gem.c | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c > index 5a67f53..51d87ae 100644 > --- a/intel/intel_bufmgr_gem.c > +++ b/intel/intel_bufmgr_gem.c > @@ -655,7 +655,8 @@ drm_intel_gem_bo_alloc_internal(drm_intel_bufmgr *bufmgr, > unsigned long size, > unsigned long flags, > uint32_t tiling_mode, > - unsigned long stride) > + unsigned long stride, > + unsigned int alignment) > { > drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *) bufmgr; > drm_intel_bo_gem *bo_gem; > @@ -754,6 +755,7 @@ retry: > return NULL; > } > bo_gem->bo.bufmgr = bufmgr; > + bo_gem->bo.align = alignment; > > bo_gem->tiling_mode = I915_TILING_NONE; > bo_gem->swizzle_mode = I915_BIT_6_SWIZZLE_NONE; > @@ -797,7 +799,8 @@ drm_intel_gem_bo_alloc_for_render(drm_intel_bufmgr *bufmgr, > { > return drm_intel_gem_bo_alloc_internal(bufmgr, name, size, > BO_ALLOC_FOR_RENDER, > - I915_TILING_NONE, 0); > + I915_TILING_NONE, 0, > + alignment); > } > > static drm_intel_bo * > @@ -807,7 +810,7 @@ drm_intel_gem_bo_alloc(drm_intel_bufmgr *bufmgr, > unsigned int alignment) > { > return drm_intel_gem_bo_alloc_internal(bufmgr, name, size, 0, > - I915_TILING_NONE, 0); > + I915_TILING_NONE, 0, 0); > } > > static drm_intel_bo * > @@ -859,7 +862,7 @@ drm_intel_gem_bo_alloc_tiled(drm_intel_bufmgr *bufmgr, const char *name, > stride = 0; > > return drm_intel_gem_bo_alloc_internal(bufmgr, name, size, flags, > - tiling, stride); > + tiling, stride, 0); > } > > static drm_intel_bo * > -- > 2.3.4 > Bump
On Wed, May 20, 2015 at 2:01 PM, Anuj Phogat <anuj.phogat@gmail.com> wrote: > On Fri, Apr 10, 2015 at 5:20 PM, Anuj Phogat <anuj.phogat@gmail.com> wrote: >> and use it to initialize the align variable in drm_intel_bo. >> >> In case of YF/YS tiled buffers libdrm need not know about the tiling >> format because these buffers don't have hardware support to be tiled >> or detiled through a fenced region. But, libdrm still need to know >> about buffer alignment restrictions because kernel uses it when >> resolving the relocation. >> >> Mesa uses drm_intel_gem_bo_alloc_for_render() to allocate Yf/Ys buffers. >> So, use the passed alignment value in this function. Note that we continue >> ignoring the alignment value passed to drm_intel_gem_bo_alloc() to follow >> the previous behavior. >> >> Cc: Kristian Høgsberg <krh@bitplanet.net> >> Cc: Damien Lespiau <damien.lespiau@intel.com> >> Cc: Daniel Vetter <daniel@ffwll.ch> >> Signed-off-by: Anuj Phogat <anuj.phogat@gmail.com> >> --- >> intel/intel_bufmgr_gem.c | 11 +++++++---- >> 1 file changed, 7 insertions(+), 4 deletions(-) >> >> diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c >> index 5a67f53..51d87ae 100644 >> --- a/intel/intel_bufmgr_gem.c >> +++ b/intel/intel_bufmgr_gem.c >> @@ -655,7 +655,8 @@ drm_intel_gem_bo_alloc_internal(drm_intel_bufmgr *bufmgr, >> unsigned long size, >> unsigned long flags, >> uint32_t tiling_mode, >> - unsigned long stride) >> + unsigned long stride, >> + unsigned int alignment) >> { >> drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *) bufmgr; >> drm_intel_bo_gem *bo_gem; >> @@ -754,6 +755,7 @@ retry: >> return NULL; >> } >> bo_gem->bo.bufmgr = bufmgr; >> + bo_gem->bo.align = alignment; >> >> bo_gem->tiling_mode = I915_TILING_NONE; >> bo_gem->swizzle_mode = I915_BIT_6_SWIZZLE_NONE; >> @@ -797,7 +799,8 @@ drm_intel_gem_bo_alloc_for_render(drm_intel_bufmgr *bufmgr, >> { >> return drm_intel_gem_bo_alloc_internal(bufmgr, name, size, >> BO_ALLOC_FOR_RENDER, >> - I915_TILING_NONE, 0); >> + I915_TILING_NONE, 0, >> + alignment); >> } >> >> static drm_intel_bo * >> @@ -807,7 +810,7 @@ drm_intel_gem_bo_alloc(drm_intel_bufmgr *bufmgr, >> unsigned int alignment) >> { >> return drm_intel_gem_bo_alloc_internal(bufmgr, name, size, 0, >> - I915_TILING_NONE, 0); >> + I915_TILING_NONE, 0, 0); >> } >> >> static drm_intel_bo * >> @@ -859,7 +862,7 @@ drm_intel_gem_bo_alloc_tiled(drm_intel_bufmgr *bufmgr, const char *name, >> stride = 0; >> >> return drm_intel_gem_bo_alloc_internal(bufmgr, name, size, flags, >> - tiling, stride); >> + tiling, stride, 0); >> } >> >> static drm_intel_bo * >> -- >> 2.3.4 >> > Bump This patch is on the list for 8 weeks now. Please take a look so I can push it upstream.
On Tue, Jun 09, 2015 at 02:59:33PM -0700, Anuj Phogat wrote: > This patch is on the list for 8 weeks now. Please take a look so I can push > it upstream. Could I suggest you nominate a mesa team member working on SKL as well? that would be the ideal match I believe.
On Wed, Jun 10, 2015 at 1:47 AM, Damien Lespiau <damien.lespiau@intel.com> wrote: > On Tue, Jun 09, 2015 at 02:59:33PM -0700, Anuj Phogat wrote: >> This patch is on the list for 8 weeks now. Please take a look so I can push >> it upstream. > > Could I suggest you nominate a mesa team member working on SKL as well? > that would be the ideal match I believe. I will request someone in Mesa time to take a look. But, I still expect "looks fine/incorrect", "acked/nacked", "I don't know this code" comments by people who reviewed the v1. +Ben: Since he's reviewing my mesa patch: "i965/gen9: Allocate YF/YS tiled buffer objects" > > -- > Damien
+Ben. On Fri, Apr 10, 2015 at 5:20 PM, Anuj Phogat <anuj.phogat@gmail.com> wrote: > and use it to initialize the align variable in drm_intel_bo. > > In case of YF/YS tiled buffers libdrm need not know about the tiling > format because these buffers don't have hardware support to be tiled > or detiled through a fenced region. But, libdrm still need to know > about buffer alignment restrictions because kernel uses it when > resolving the relocation. > > Mesa uses drm_intel_gem_bo_alloc_for_render() to allocate Yf/Ys buffers. > So, use the passed alignment value in this function. Note that we continue > ignoring the alignment value passed to drm_intel_gem_bo_alloc() to follow > the previous behavior. > > Cc: Kristian Høgsberg <krh@bitplanet.net> > Cc: Damien Lespiau <damien.lespiau@intel.com> > Cc: Daniel Vetter <daniel@ffwll.ch> > Signed-off-by: Anuj Phogat <anuj.phogat@gmail.com> > --- > intel/intel_bufmgr_gem.c | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c > index 5a67f53..51d87ae 100644 > --- a/intel/intel_bufmgr_gem.c > +++ b/intel/intel_bufmgr_gem.c > @@ -655,7 +655,8 @@ drm_intel_gem_bo_alloc_internal(drm_intel_bufmgr *bufmgr, > unsigned long size, > unsigned long flags, > uint32_t tiling_mode, > - unsigned long stride) > + unsigned long stride, > + unsigned int alignment) > { > drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *) bufmgr; > drm_intel_bo_gem *bo_gem; > @@ -754,6 +755,7 @@ retry: > return NULL; > } > bo_gem->bo.bufmgr = bufmgr; > + bo_gem->bo.align = alignment; > > bo_gem->tiling_mode = I915_TILING_NONE; > bo_gem->swizzle_mode = I915_BIT_6_SWIZZLE_NONE; > @@ -797,7 +799,8 @@ drm_intel_gem_bo_alloc_for_render(drm_intel_bufmgr *bufmgr, > { > return drm_intel_gem_bo_alloc_internal(bufmgr, name, size, > BO_ALLOC_FOR_RENDER, > - I915_TILING_NONE, 0); > + I915_TILING_NONE, 0, > + alignment); > } > > static drm_intel_bo * > @@ -807,7 +810,7 @@ drm_intel_gem_bo_alloc(drm_intel_bufmgr *bufmgr, > unsigned int alignment) > { > return drm_intel_gem_bo_alloc_internal(bufmgr, name, size, 0, > - I915_TILING_NONE, 0); > + I915_TILING_NONE, 0, 0); > } > > static drm_intel_bo * > @@ -859,7 +862,7 @@ drm_intel_gem_bo_alloc_tiled(drm_intel_bufmgr *bufmgr, const char *name, > stride = 0; > > return drm_intel_gem_bo_alloc_internal(bufmgr, name, size, flags, > - tiling, stride); > + tiling, stride, 0); > } > > static drm_intel_bo * > -- > 2.3.4 >
On Fri, Jun 19, 2015 at 03:10:49PM -0700, Anuj Phogat wrote: > On Wed, Jun 10, 2015 at 1:47 AM, Damien Lespiau > <damien.lespiau@intel.com> wrote: > > On Tue, Jun 09, 2015 at 02:59:33PM -0700, Anuj Phogat wrote: > >> This patch is on the list for 8 weeks now. Please take a look so I can push > >> it upstream. > > > > Could I suggest you nominate a mesa team member working on SKL as well? > > that would be the ideal match I believe. > I will request someone in Mesa time to take a look. But, I still expect > "looks fine/incorrect", "acked/nacked", "I don't know this code" comments by > people who reviewed the v1. Ack from kernel pov with the promise that if this passes with mesa folks we'll sign up to maintain this kernel interface forever. -Daniel > > +Ben: Since he's reviewing my mesa patch: > "i965/gen9: Allocate YF/YS tiled buffer objects" > > > > -- > > Damien
On Fri, Jun 19, 2015 at 03:50:44PM -0700, Anuj Phogat wrote: > +Ben. > > On Fri, Apr 10, 2015 at 5:20 PM, Anuj Phogat <anuj.phogat@gmail.com> wrote: > > and use it to initialize the align variable in drm_intel_bo. > > > > In case of YF/YS tiled buffers libdrm need not know about the tiling > > format because these buffers don't have hardware support to be tiled > > or detiled through a fenced region. But, libdrm still need to know > > about buffer alignment restrictions because kernel uses it when > > resolving the relocation. > > > > Mesa uses drm_intel_gem_bo_alloc_for_render() to allocate Yf/Ys buffers. > > So, use the passed alignment value in this function. Note that we continue > > ignoring the alignment value passed to drm_intel_gem_bo_alloc() to follow > > the previous behavior. I think there is a problem here if you're getting the BO from the cache. If you allocate out of the cache, and the alignment is incorrect, I don't think anything will fix it for you. > > > > Cc: Kristian Høgsberg <krh@bitplanet.net> > > Cc: Damien Lespiau <damien.lespiau@intel.com> > > Cc: Daniel Vetter <daniel@ffwll.ch> > > Signed-off-by: Anuj Phogat <anuj.phogat@gmail.com> > > --- > > intel/intel_bufmgr_gem.c | 11 +++++++---- > > 1 file changed, 7 insertions(+), 4 deletions(-) > > > > diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c > > index 5a67f53..51d87ae 100644 > > --- a/intel/intel_bufmgr_gem.c > > +++ b/intel/intel_bufmgr_gem.c > > @@ -655,7 +655,8 @@ drm_intel_gem_bo_alloc_internal(drm_intel_bufmgr *bufmgr, > > unsigned long size, > > unsigned long flags, > > uint32_t tiling_mode, > > - unsigned long stride) > > + unsigned long stride, > > + unsigned int alignment) > > { > > drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *) bufmgr; > > drm_intel_bo_gem *bo_gem; I think you need a check somewhere like: if (alignment && bo_gem->bo.align != alignment) alloc_from_cache = false; > > @@ -754,6 +755,7 @@ retry: > > return NULL; > > } > > bo_gem->bo.bufmgr = bufmgr; > > + bo_gem->bo.align = alignment; > > > > bo_gem->tiling_mode = I915_TILING_NONE; > > bo_gem->swizzle_mode = I915_BIT_6_SWIZZLE_NONE; > > @@ -797,7 +799,8 @@ drm_intel_gem_bo_alloc_for_render(drm_intel_bufmgr *bufmgr, > > { > > return drm_intel_gem_bo_alloc_internal(bufmgr, name, size, > > BO_ALLOC_FOR_RENDER, > > - I915_TILING_NONE, 0); > > + I915_TILING_NONE, 0, > > + alignment); > > } > > > > static drm_intel_bo * > > @@ -807,7 +810,7 @@ drm_intel_gem_bo_alloc(drm_intel_bufmgr *bufmgr, > > unsigned int alignment) > > { > > return drm_intel_gem_bo_alloc_internal(bufmgr, name, size, 0, > > - I915_TILING_NONE, 0); > > + I915_TILING_NONE, 0, 0); > > } > > > > static drm_intel_bo * > > @@ -859,7 +862,7 @@ drm_intel_gem_bo_alloc_tiled(drm_intel_bufmgr *bufmgr, const char *name, > > stride = 0; > > > > return drm_intel_gem_bo_alloc_internal(bufmgr, name, size, flags, > > - tiling, stride); > > + tiling, stride, 0); > > } > > > > static drm_intel_bo * > > -- > > 2.3.4 > >
On Mon, Jun 22, 2015 at 10:01 AM, Ben Widawsky <ben@bwidawsk.net> wrote: > On Fri, Jun 19, 2015 at 03:50:44PM -0700, Anuj Phogat wrote: >> +Ben. >> >> On Fri, Apr 10, 2015 at 5:20 PM, Anuj Phogat <anuj.phogat@gmail.com> wrote: >> > and use it to initialize the align variable in drm_intel_bo. >> > >> > In case of YF/YS tiled buffers libdrm need not know about the tiling >> > format because these buffers don't have hardware support to be tiled >> > or detiled through a fenced region. But, libdrm still need to know >> > about buffer alignment restrictions because kernel uses it when >> > resolving the relocation. >> > >> > Mesa uses drm_intel_gem_bo_alloc_for_render() to allocate Yf/Ys buffers. >> > So, use the passed alignment value in this function. Note that we continue >> > ignoring the alignment value passed to drm_intel_gem_bo_alloc() to follow >> > the previous behavior. > > I think there is a problem here if you're getting the BO from the cache. If you > allocate out of the cache, and the alignment is incorrect, I don't think > anything will fix it for you. > Right. Thanks for catching this. I'll send out v2 with suggested changes. >> > >> > Cc: Kristian Høgsberg <krh@bitplanet.net> >> > Cc: Damien Lespiau <damien.lespiau@intel.com> >> > Cc: Daniel Vetter <daniel@ffwll.ch> >> > Signed-off-by: Anuj Phogat <anuj.phogat@gmail.com> >> > --- >> > intel/intel_bufmgr_gem.c | 11 +++++++---- >> > 1 file changed, 7 insertions(+), 4 deletions(-) >> > >> > diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c >> > index 5a67f53..51d87ae 100644 >> > --- a/intel/intel_bufmgr_gem.c >> > +++ b/intel/intel_bufmgr_gem.c >> > @@ -655,7 +655,8 @@ drm_intel_gem_bo_alloc_internal(drm_intel_bufmgr *bufmgr, >> > unsigned long size, >> > unsigned long flags, >> > uint32_t tiling_mode, >> > - unsigned long stride) >> > + unsigned long stride, >> > + unsigned int alignment) >> > { >> > drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *) bufmgr; >> > drm_intel_bo_gem *bo_gem; > > I think you need a check somewhere like: > if (alignment && > bo_gem->bo.align != alignment) > alloc_from_cache = false; > >> > @@ -754,6 +755,7 @@ retry: >> > return NULL; >> > } >> > bo_gem->bo.bufmgr = bufmgr; >> > + bo_gem->bo.align = alignment; >> > >> > bo_gem->tiling_mode = I915_TILING_NONE; >> > bo_gem->swizzle_mode = I915_BIT_6_SWIZZLE_NONE; >> > @@ -797,7 +799,8 @@ drm_intel_gem_bo_alloc_for_render(drm_intel_bufmgr *bufmgr, >> > { >> > return drm_intel_gem_bo_alloc_internal(bufmgr, name, size, >> > BO_ALLOC_FOR_RENDER, >> > - I915_TILING_NONE, 0); >> > + I915_TILING_NONE, 0, >> > + alignment); >> > } >> > >> > static drm_intel_bo * >> > @@ -807,7 +810,7 @@ drm_intel_gem_bo_alloc(drm_intel_bufmgr *bufmgr, >> > unsigned int alignment) >> > { >> > return drm_intel_gem_bo_alloc_internal(bufmgr, name, size, 0, >> > - I915_TILING_NONE, 0); >> > + I915_TILING_NONE, 0, 0); >> > } >> > >> > static drm_intel_bo * >> > @@ -859,7 +862,7 @@ drm_intel_gem_bo_alloc_tiled(drm_intel_bufmgr *bufmgr, const char *name, >> > stride = 0; >> > >> > return drm_intel_gem_bo_alloc_internal(bufmgr, name, size, flags, >> > - tiling, stride); >> > + tiling, stride, 0); >> > } >> > >> > static drm_intel_bo * >> > -- >> > 2.3.4 >> >
diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c index 5a67f53..51d87ae 100644 --- a/intel/intel_bufmgr_gem.c +++ b/intel/intel_bufmgr_gem.c @@ -655,7 +655,8 @@ drm_intel_gem_bo_alloc_internal(drm_intel_bufmgr *bufmgr, unsigned long size, unsigned long flags, uint32_t tiling_mode, - unsigned long stride) + unsigned long stride, + unsigned int alignment) { drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *) bufmgr; drm_intel_bo_gem *bo_gem; @@ -754,6 +755,7 @@ retry: return NULL; } bo_gem->bo.bufmgr = bufmgr; + bo_gem->bo.align = alignment; bo_gem->tiling_mode = I915_TILING_NONE; bo_gem->swizzle_mode = I915_BIT_6_SWIZZLE_NONE; @@ -797,7 +799,8 @@ drm_intel_gem_bo_alloc_for_render(drm_intel_bufmgr *bufmgr, { return drm_intel_gem_bo_alloc_internal(bufmgr, name, size, BO_ALLOC_FOR_RENDER, - I915_TILING_NONE, 0); + I915_TILING_NONE, 0, + alignment); } static drm_intel_bo * @@ -807,7 +810,7 @@ drm_intel_gem_bo_alloc(drm_intel_bufmgr *bufmgr, unsigned int alignment) { return drm_intel_gem_bo_alloc_internal(bufmgr, name, size, 0, - I915_TILING_NONE, 0); + I915_TILING_NONE, 0, 0); } static drm_intel_bo * @@ -859,7 +862,7 @@ drm_intel_gem_bo_alloc_tiled(drm_intel_bufmgr *bufmgr, const char *name, stride = 0; return drm_intel_gem_bo_alloc_internal(bufmgr, name, size, flags, - tiling, stride); + tiling, stride, 0); } static drm_intel_bo *
and use it to initialize the align variable in drm_intel_bo. In case of YF/YS tiled buffers libdrm need not know about the tiling format because these buffers don't have hardware support to be tiled or detiled through a fenced region. But, libdrm still need to know about buffer alignment restrictions because kernel uses it when resolving the relocation. Mesa uses drm_intel_gem_bo_alloc_for_render() to allocate Yf/Ys buffers. So, use the passed alignment value in this function. Note that we continue ignoring the alignment value passed to drm_intel_gem_bo_alloc() to follow the previous behavior. Cc: Kristian Høgsberg <krh@bitplanet.net> Cc: Damien Lespiau <damien.lespiau@intel.com> Cc: Daniel Vetter <daniel@ffwll.ch> Signed-off-by: Anuj Phogat <anuj.phogat@gmail.com> --- intel/intel_bufmgr_gem.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)