Message ID | 1428711656-5878-2-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: > Signed-off-by: Anuj Phogat <anuj.phogat@gmail.com> > --- > intel/intel_bufmgr_gem.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c > index 51d87ae..92701a5 100644 > --- a/intel/intel_bufmgr_gem.c > +++ b/intel/intel_bufmgr_gem.c > @@ -459,7 +459,7 @@ drm_intel_add_validate_buffer(drm_intel_bo *bo) > bufmgr_gem->exec_objects[index].handle = bo_gem->gem_handle; > bufmgr_gem->exec_objects[index].relocation_count = bo_gem->reloc_count; > bufmgr_gem->exec_objects[index].relocs_ptr = (uintptr_t) bo_gem->relocs; > - bufmgr_gem->exec_objects[index].alignment = 0; > + bufmgr_gem->exec_objects[index].alignment = bo->align; > bufmgr_gem->exec_objects[index].offset = 0; > bufmgr_gem->exec_bos[index] = bo; > bufmgr_gem->exec_count++; > @@ -501,7 +501,7 @@ drm_intel_add_validate_buffer2(drm_intel_bo *bo, int need_fence) > bufmgr_gem->exec2_objects[index].handle = bo_gem->gem_handle; > bufmgr_gem->exec2_objects[index].relocation_count = bo_gem->reloc_count; > bufmgr_gem->exec2_objects[index].relocs_ptr = (uintptr_t)bo_gem->relocs; > - bufmgr_gem->exec2_objects[index].alignment = 0; > + bufmgr_gem->exec2_objects[index].alignment = bo->align; > bufmgr_gem->exec2_objects[index].offset = 0; > bufmgr_gem->exec_bos[index] = bo; > bufmgr_gem->exec2_objects[index].flags = 0; > -- > 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: >> Signed-off-by: Anuj Phogat <anuj.phogat@gmail.com> >> --- >> intel/intel_bufmgr_gem.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c >> index 51d87ae..92701a5 100644 >> --- a/intel/intel_bufmgr_gem.c >> +++ b/intel/intel_bufmgr_gem.c >> @@ -459,7 +459,7 @@ drm_intel_add_validate_buffer(drm_intel_bo *bo) >> bufmgr_gem->exec_objects[index].handle = bo_gem->gem_handle; >> bufmgr_gem->exec_objects[index].relocation_count = bo_gem->reloc_count; >> bufmgr_gem->exec_objects[index].relocs_ptr = (uintptr_t) bo_gem->relocs; >> - bufmgr_gem->exec_objects[index].alignment = 0; >> + bufmgr_gem->exec_objects[index].alignment = bo->align; >> bufmgr_gem->exec_objects[index].offset = 0; >> bufmgr_gem->exec_bos[index] = bo; >> bufmgr_gem->exec_count++; >> @@ -501,7 +501,7 @@ drm_intel_add_validate_buffer2(drm_intel_bo *bo, int need_fence) >> bufmgr_gem->exec2_objects[index].handle = bo_gem->gem_handle; >> bufmgr_gem->exec2_objects[index].relocation_count = bo_gem->reloc_count; >> bufmgr_gem->exec2_objects[index].relocs_ptr = (uintptr_t)bo_gem->relocs; >> - bufmgr_gem->exec2_objects[index].alignment = 0; >> + bufmgr_gem->exec2_objects[index].alignment = bo->align; >> bufmgr_gem->exec2_objects[index].offset = 0; >> bufmgr_gem->exec_bos[index] = bo; >> bufmgr_gem->exec2_objects[index].flags = 0; >> -- >> 2.3.4 >> > Bump This patch is on the list for 8 weeks now. Please take a look so I can push it upstream.
+Ben On Fri, Apr 10, 2015 at 5:20 PM, Anuj Phogat <anuj.phogat@gmail.com> wrote: > Signed-off-by: Anuj Phogat <anuj.phogat@gmail.com> > --- > intel/intel_bufmgr_gem.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c > index 51d87ae..92701a5 100644 > --- a/intel/intel_bufmgr_gem.c > +++ b/intel/intel_bufmgr_gem.c > @@ -459,7 +459,7 @@ drm_intel_add_validate_buffer(drm_intel_bo *bo) > bufmgr_gem->exec_objects[index].handle = bo_gem->gem_handle; > bufmgr_gem->exec_objects[index].relocation_count = bo_gem->reloc_count; > bufmgr_gem->exec_objects[index].relocs_ptr = (uintptr_t) bo_gem->relocs; > - bufmgr_gem->exec_objects[index].alignment = 0; > + bufmgr_gem->exec_objects[index].alignment = bo->align; > bufmgr_gem->exec_objects[index].offset = 0; > bufmgr_gem->exec_bos[index] = bo; > bufmgr_gem->exec_count++; > @@ -501,7 +501,7 @@ drm_intel_add_validate_buffer2(drm_intel_bo *bo, int need_fence) > bufmgr_gem->exec2_objects[index].handle = bo_gem->gem_handle; > bufmgr_gem->exec2_objects[index].relocation_count = bo_gem->reloc_count; > bufmgr_gem->exec2_objects[index].relocs_ptr = (uintptr_t)bo_gem->relocs; > - bufmgr_gem->exec2_objects[index].alignment = 0; > + bufmgr_gem->exec2_objects[index].alignment = bo->align; > bufmgr_gem->exec2_objects[index].offset = 0; > bufmgr_gem->exec_bos[index] = bo; > bufmgr_gem->exec2_objects[index].flags = 0; > -- > 2.3.4 >
On Fri, Jun 19, 2015 at 03:52:01PM -0700, Anuj Phogat wrote: > +Ben > > On Fri, Apr 10, 2015 at 5:20 PM, Anuj Phogat <anuj.phogat@gmail.com> wrote: > > Signed-off-by: Anuj Phogat <anuj.phogat@gmail.com> > > --- > > intel/intel_bufmgr_gem.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c > > index 51d87ae..92701a5 100644 > > --- a/intel/intel_bufmgr_gem.c > > +++ b/intel/intel_bufmgr_gem.c > > @@ -459,7 +459,7 @@ drm_intel_add_validate_buffer(drm_intel_bo *bo) > > bufmgr_gem->exec_objects[index].handle = bo_gem->gem_handle; > > bufmgr_gem->exec_objects[index].relocation_count = bo_gem->reloc_count; > > bufmgr_gem->exec_objects[index].relocs_ptr = (uintptr_t) bo_gem->relocs; > > - bufmgr_gem->exec_objects[index].alignment = 0; > > + bufmgr_gem->exec_objects[index].alignment = bo->align; > > bufmgr_gem->exec_objects[index].offset = 0; > > bufmgr_gem->exec_bos[index] = bo; > > bufmgr_gem->exec_count++; I'm a bit hesitant about this hunk. We're never going to use this from a mesa that supports Yf/Ys - and going this path wouldn't be expected. Maybe add a warning if bo->align? (From your other patch I don't think it can ever happen, but just to future proof it. > > @@ -501,7 +501,7 @@ drm_intel_add_validate_buffer2(drm_intel_bo *bo, int need_fence) > > bufmgr_gem->exec2_objects[index].handle = bo_gem->gem_handle; > > bufmgr_gem->exec2_objects[index].relocation_count = bo_gem->reloc_count; > > bufmgr_gem->exec2_objects[index].relocs_ptr = (uintptr_t)bo_gem->relocs; > > - bufmgr_gem->exec2_objects[index].alignment = 0; > > + bufmgr_gem->exec2_objects[index].alignment = bo->align; > > bufmgr_gem->exec2_objects[index].offset = 0; > > bufmgr_gem->exec_bos[index] = bo; > > bufmgr_gem->exec2_objects[index].flags = 0; I was about to argue this should be part of patch 1, but nope, it should be a separate patch :-) I started digging into whether we have a reasonable way to determine if a bo alignment failed, and fall back to a softer restriction. It didn't seem doable with the current interfaces, but it's something to think about. With or without the first recommendation: Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
On Mon, Jun 22, 2015 at 10:21:46AM -0700, Ben Widawsky wrote: > On Fri, Jun 19, 2015 at 03:52:01PM -0700, Anuj Phogat wrote: > > +Ben > > > > On Fri, Apr 10, 2015 at 5:20 PM, Anuj Phogat <anuj.phogat@gmail.com> wrote: > > > Signed-off-by: Anuj Phogat <anuj.phogat@gmail.com> > > > --- > > > intel/intel_bufmgr_gem.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c > > > index 51d87ae..92701a5 100644 > > > --- a/intel/intel_bufmgr_gem.c > > > +++ b/intel/intel_bufmgr_gem.c > > > @@ -459,7 +459,7 @@ drm_intel_add_validate_buffer(drm_intel_bo *bo) > > > bufmgr_gem->exec_objects[index].handle = bo_gem->gem_handle; > > > bufmgr_gem->exec_objects[index].relocation_count = bo_gem->reloc_count; > > > bufmgr_gem->exec_objects[index].relocs_ptr = (uintptr_t) bo_gem->relocs; > > > - bufmgr_gem->exec_objects[index].alignment = 0; > > > + bufmgr_gem->exec_objects[index].alignment = bo->align; > > > bufmgr_gem->exec_objects[index].offset = 0; > > > bufmgr_gem->exec_bos[index] = bo; > > > bufmgr_gem->exec_count++; > > I'm a bit hesitant about this hunk. We're never going to use this from a mesa > that supports Yf/Ys - and going this path wouldn't be expected. Maybe add a > warning if bo->align? (From your other patch I don't think it can ever happen, > but just to future proof it. > > > > @@ -501,7 +501,7 @@ drm_intel_add_validate_buffer2(drm_intel_bo *bo, int need_fence) > > > bufmgr_gem->exec2_objects[index].handle = bo_gem->gem_handle; > > > bufmgr_gem->exec2_objects[index].relocation_count = bo_gem->reloc_count; > > > bufmgr_gem->exec2_objects[index].relocs_ptr = (uintptr_t)bo_gem->relocs; > > > - bufmgr_gem->exec2_objects[index].alignment = 0; > > > + bufmgr_gem->exec2_objects[index].alignment = bo->align; > > > bufmgr_gem->exec2_objects[index].offset = 0; > > > bufmgr_gem->exec_bos[index] = bo; > > > bufmgr_gem->exec2_objects[index].flags = 0; > > I was about to argue this should be part of patch 1, but nope, it should be a > separate patch :-) > > I started digging into whether we have a reasonable way to determine if a bo > alignment failed, and fall back to a softer restriction. It didn't seem doable > with the current interfaces, but it's something to think about. On gen2/3 we have a lot more severe alignment problems and there the only thing we've done is to take worst-case space loss for alignment into account for the execbuf space estimation. But if that failed (and it did every now and then) userspace just died. I don't think we need any of this for new tiling layouts since they're all uniformly using a 64kb alignment. The kernel should be able to pack this very well. -Daniel
On Mon, Jun 22, 2015 at 12:49 PM, Daniel Vetter <daniel@ffwll.ch> wrote: > On Mon, Jun 22, 2015 at 10:21:46AM -0700, Ben Widawsky wrote: >> On Fri, Jun 19, 2015 at 03:52:01PM -0700, Anuj Phogat wrote: >> > +Ben >> > >> > On Fri, Apr 10, 2015 at 5:20 PM, Anuj Phogat <anuj.phogat@gmail.com> wrote: >> > > Signed-off-by: Anuj Phogat <anuj.phogat@gmail.com> >> > > --- >> > > intel/intel_bufmgr_gem.c | 4 ++-- >> > > 1 file changed, 2 insertions(+), 2 deletions(-) >> > > >> > > diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c >> > > index 51d87ae..92701a5 100644 >> > > --- a/intel/intel_bufmgr_gem.c >> > > +++ b/intel/intel_bufmgr_gem.c >> > > @@ -459,7 +459,7 @@ drm_intel_add_validate_buffer(drm_intel_bo *bo) >> > > bufmgr_gem->exec_objects[index].handle = bo_gem->gem_handle; >> > > bufmgr_gem->exec_objects[index].relocation_count = bo_gem->reloc_count; >> > > bufmgr_gem->exec_objects[index].relocs_ptr = (uintptr_t) bo_gem->relocs; >> > > - bufmgr_gem->exec_objects[index].alignment = 0; >> > > + bufmgr_gem->exec_objects[index].alignment = bo->align; >> > > bufmgr_gem->exec_objects[index].offset = 0; >> > > bufmgr_gem->exec_bos[index] = bo; >> > > bufmgr_gem->exec_count++; >> >> I'm a bit hesitant about this hunk. We're never going to use this from a mesa >> that supports Yf/Ys - and going this path wouldn't be expected. Maybe add a >> warning if bo->align? (From your other patch I don't think it can ever happen, >> but just to future proof it. >> >> > > @@ -501,7 +501,7 @@ drm_intel_add_validate_buffer2(drm_intel_bo *bo, int need_fence) >> > > bufmgr_gem->exec2_objects[index].handle = bo_gem->gem_handle; >> > > bufmgr_gem->exec2_objects[index].relocation_count = bo_gem->reloc_count; >> > > bufmgr_gem->exec2_objects[index].relocs_ptr = (uintptr_t)bo_gem->relocs; >> > > - bufmgr_gem->exec2_objects[index].alignment = 0; >> > > + bufmgr_gem->exec2_objects[index].alignment = bo->align; >> > > bufmgr_gem->exec2_objects[index].offset = 0; >> > > bufmgr_gem->exec_bos[index] = bo; >> > > bufmgr_gem->exec2_objects[index].flags = 0; >> >> I was about to argue this should be part of patch 1, but nope, it should be a >> separate patch :-) >> >> I started digging into whether we have a reasonable way to determine if a bo >> alignment failed, and fall back to a softer restriction. It didn't seem doable >> with the current interfaces, but it's something to think about. > > On gen2/3 we have a lot more severe alignment problems and there the only > thing we've done is to take worst-case space loss for alignment into > account for the execbuf space estimation. But if that failed (and it did > every now and then) userspace just died. I don't think we need any of this > for new tiling layouts since they're all uniformly using a 64kb alignment. > The kernel should be able to pack this very well. Daniel, I don't know much about how kernel is handling the alignment constraints for new tiling layouts during relocation. I'm here mostly relying on the advice from kernel guys. These two patches are based on your comment on an earlier patch on intel-gfx: "[PATCH 4/5] Align YS tile base address to 64KB" Even if the kernel is already packing the new tiling layouts correctly, these patches are passing some valid alignment value in place of always zero. Isn't it a harmless change? > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
On Tue, Jun 23, 2015 at 04:37:19PM -0700, Anuj Phogat wrote: > On Mon, Jun 22, 2015 at 12:49 PM, Daniel Vetter <daniel@ffwll.ch> wrote: > > On Mon, Jun 22, 2015 at 10:21:46AM -0700, Ben Widawsky wrote: > >> On Fri, Jun 19, 2015 at 03:52:01PM -0700, Anuj Phogat wrote: > >> > +Ben > >> > > >> > On Fri, Apr 10, 2015 at 5:20 PM, Anuj Phogat <anuj.phogat@gmail.com> wrote: > >> > > Signed-off-by: Anuj Phogat <anuj.phogat@gmail.com> > >> > > --- > >> > > intel/intel_bufmgr_gem.c | 4 ++-- > >> > > 1 file changed, 2 insertions(+), 2 deletions(-) > >> > > > >> > > diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c > >> > > index 51d87ae..92701a5 100644 > >> > > --- a/intel/intel_bufmgr_gem.c > >> > > +++ b/intel/intel_bufmgr_gem.c > >> > > @@ -459,7 +459,7 @@ drm_intel_add_validate_buffer(drm_intel_bo *bo) > >> > > bufmgr_gem->exec_objects[index].handle = bo_gem->gem_handle; > >> > > bufmgr_gem->exec_objects[index].relocation_count = bo_gem->reloc_count; > >> > > bufmgr_gem->exec_objects[index].relocs_ptr = (uintptr_t) bo_gem->relocs; > >> > > - bufmgr_gem->exec_objects[index].alignment = 0; > >> > > + bufmgr_gem->exec_objects[index].alignment = bo->align; > >> > > bufmgr_gem->exec_objects[index].offset = 0; > >> > > bufmgr_gem->exec_bos[index] = bo; > >> > > bufmgr_gem->exec_count++; > >> > >> I'm a bit hesitant about this hunk. We're never going to use this from a mesa > >> that supports Yf/Ys - and going this path wouldn't be expected. Maybe add a > >> warning if bo->align? (From your other patch I don't think it can ever happen, > >> but just to future proof it. > >> > >> > > @@ -501,7 +501,7 @@ drm_intel_add_validate_buffer2(drm_intel_bo *bo, int need_fence) > >> > > bufmgr_gem->exec2_objects[index].handle = bo_gem->gem_handle; > >> > > bufmgr_gem->exec2_objects[index].relocation_count = bo_gem->reloc_count; > >> > > bufmgr_gem->exec2_objects[index].relocs_ptr = (uintptr_t)bo_gem->relocs; > >> > > - bufmgr_gem->exec2_objects[index].alignment = 0; > >> > > + bufmgr_gem->exec2_objects[index].alignment = bo->align; > >> > > bufmgr_gem->exec2_objects[index].offset = 0; > >> > > bufmgr_gem->exec_bos[index] = bo; > >> > > bufmgr_gem->exec2_objects[index].flags = 0; > >> > >> I was about to argue this should be part of patch 1, but nope, it should be a > >> separate patch :-) > >> > >> I started digging into whether we have a reasonable way to determine if a bo > >> alignment failed, and fall back to a softer restriction. It didn't seem doable > >> with the current interfaces, but it's something to think about. > > > > On gen2/3 we have a lot more severe alignment problems and there the only > > thing we've done is to take worst-case space loss for alignment into > > account for the execbuf space estimation. But if that failed (and it did > > every now and then) userspace just died. I don't think we need any of this > > for new tiling layouts since they're all uniformly using a 64kb alignment. > > The kernel should be able to pack this very well. > Daniel, I don't know much about how kernel is handling the alignment > constraints for new tiling layouts during relocation. I'm here mostly > relying on the advice from kernel guys. These two patches are based on > your comment on an earlier patch on intel-gfx: > "[PATCH 4/5] Align YS tile base address to 64KB" > > Even if the kernel is already packing the new tiling layouts correctly, these > patches are passing some valid alignment value in place of always zero. > Isn't it a harmless change? I think Daniel is talking here about the estimated GTT usage for a batch. mesa/libdrm tracks the estimated usage so that it can flush before the batch can no longer fit into the GTT. There is a one primitive back-off in mesa, but if the estimate is far, that may not be enough to trim the batch sufficiently for us to be able to render it. To accommodate that worry, you just want to tweak drm_intel_bo_gem_set_in_aperture_size() to include the alignment as overhead (and not assume anything about the ability of the kernel to pack buffers together). -Chris
On Wed, Jun 24, 2015 at 08:33:50AM +0100, Chris Wilson wrote: > On Tue, Jun 23, 2015 at 04:37:19PM -0700, Anuj Phogat wrote: > > On Mon, Jun 22, 2015 at 12:49 PM, Daniel Vetter <daniel@ffwll.ch> wrote: > > > On Mon, Jun 22, 2015 at 10:21:46AM -0700, Ben Widawsky wrote: > > >> On Fri, Jun 19, 2015 at 03:52:01PM -0700, Anuj Phogat wrote: > > >> > +Ben > > >> > > > >> > On Fri, Apr 10, 2015 at 5:20 PM, Anuj Phogat <anuj.phogat@gmail.com> wrote: > > >> > > Signed-off-by: Anuj Phogat <anuj.phogat@gmail.com> > > >> > > --- > > >> > > intel/intel_bufmgr_gem.c | 4 ++-- > > >> > > 1 file changed, 2 insertions(+), 2 deletions(-) > > >> > > > > >> > > diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c > > >> > > index 51d87ae..92701a5 100644 > > >> > > --- a/intel/intel_bufmgr_gem.c > > >> > > +++ b/intel/intel_bufmgr_gem.c > > >> > > @@ -459,7 +459,7 @@ drm_intel_add_validate_buffer(drm_intel_bo *bo) > > >> > > bufmgr_gem->exec_objects[index].handle = bo_gem->gem_handle; > > >> > > bufmgr_gem->exec_objects[index].relocation_count = bo_gem->reloc_count; > > >> > > bufmgr_gem->exec_objects[index].relocs_ptr = (uintptr_t) bo_gem->relocs; > > >> > > - bufmgr_gem->exec_objects[index].alignment = 0; > > >> > > + bufmgr_gem->exec_objects[index].alignment = bo->align; > > >> > > bufmgr_gem->exec_objects[index].offset = 0; > > >> > > bufmgr_gem->exec_bos[index] = bo; > > >> > > bufmgr_gem->exec_count++; > > >> > > >> I'm a bit hesitant about this hunk. We're never going to use this from a mesa > > >> that supports Yf/Ys - and going this path wouldn't be expected. Maybe add a > > >> warning if bo->align? (From your other patch I don't think it can ever happen, > > >> but just to future proof it. > > >> > > >> > > @@ -501,7 +501,7 @@ drm_intel_add_validate_buffer2(drm_intel_bo *bo, int need_fence) > > >> > > bufmgr_gem->exec2_objects[index].handle = bo_gem->gem_handle; > > >> > > bufmgr_gem->exec2_objects[index].relocation_count = bo_gem->reloc_count; > > >> > > bufmgr_gem->exec2_objects[index].relocs_ptr = (uintptr_t)bo_gem->relocs; > > >> > > - bufmgr_gem->exec2_objects[index].alignment = 0; > > >> > > + bufmgr_gem->exec2_objects[index].alignment = bo->align; > > >> > > bufmgr_gem->exec2_objects[index].offset = 0; > > >> > > bufmgr_gem->exec_bos[index] = bo; > > >> > > bufmgr_gem->exec2_objects[index].flags = 0; > > >> > > >> I was about to argue this should be part of patch 1, but nope, it should be a > > >> separate patch :-) > > >> > > >> I started digging into whether we have a reasonable way to determine if a bo > > >> alignment failed, and fall back to a softer restriction. It didn't seem doable > > >> with the current interfaces, but it's something to think about. > > > > > > On gen2/3 we have a lot more severe alignment problems and there the only > > > thing we've done is to take worst-case space loss for alignment into > > > account for the execbuf space estimation. But if that failed (and it did > > > every now and then) userspace just died. I don't think we need any of this > > > for new tiling layouts since they're all uniformly using a 64kb alignment. > > > The kernel should be able to pack this very well. > > Daniel, I don't know much about how kernel is handling the alignment > > constraints for new tiling layouts during relocation. I'm here mostly > > relying on the advice from kernel guys. These two patches are based on > > your comment on an earlier patch on intel-gfx: > > "[PATCH 4/5] Align YS tile base address to 64KB" > > > > Even if the kernel is already packing the new tiling layouts correctly, these > > patches are passing some valid alignment value in place of always zero. > > Isn't it a harmless change? > > I think Daniel is talking here about the estimated GTT usage for a > batch. mesa/libdrm tracks the estimated usage so that it can flush > before the batch can no longer fit into the GTT. There is a one > primitive back-off in mesa, but if the estimate is far, that may not be > enough to trim the batch sufficiently for us to be able to render it. To > accommodate that worry, you just want to tweak > drm_intel_bo_gem_set_in_aperture_size() to include the alignment as > overhead (and not assume anything about the ability of the kernel to > pack buffers together). Well actually I just replied to Ben's concern that alignment might fail stating that we have some tricks already to handle the much worse situation on gen2/3, like Chris explained. But also that I don't think we actually need to update them. But ofc you can update the aperture size estimate like Chris suggested: size + alignment is a good worst-case estimate of what we'll need in gtt space. With that change we _really_ should be covered. -Daniel
diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c index 51d87ae..92701a5 100644 --- a/intel/intel_bufmgr_gem.c +++ b/intel/intel_bufmgr_gem.c @@ -459,7 +459,7 @@ drm_intel_add_validate_buffer(drm_intel_bo *bo) bufmgr_gem->exec_objects[index].handle = bo_gem->gem_handle; bufmgr_gem->exec_objects[index].relocation_count = bo_gem->reloc_count; bufmgr_gem->exec_objects[index].relocs_ptr = (uintptr_t) bo_gem->relocs; - bufmgr_gem->exec_objects[index].alignment = 0; + bufmgr_gem->exec_objects[index].alignment = bo->align; bufmgr_gem->exec_objects[index].offset = 0; bufmgr_gem->exec_bos[index] = bo; bufmgr_gem->exec_count++; @@ -501,7 +501,7 @@ drm_intel_add_validate_buffer2(drm_intel_bo *bo, int need_fence) bufmgr_gem->exec2_objects[index].handle = bo_gem->gem_handle; bufmgr_gem->exec2_objects[index].relocation_count = bo_gem->reloc_count; bufmgr_gem->exec2_objects[index].relocs_ptr = (uintptr_t)bo_gem->relocs; - bufmgr_gem->exec2_objects[index].alignment = 0; + bufmgr_gem->exec2_objects[index].alignment = bo->align; bufmgr_gem->exec2_objects[index].offset = 0; bufmgr_gem->exec_bos[index] = bo; bufmgr_gem->exec2_objects[index].flags = 0;
Signed-off-by: Anuj Phogat <anuj.phogat@gmail.com> --- intel/intel_bufmgr_gem.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)