On Tue, Jan 10, 2017 at 03:55:35PM +0200, Joonas Lahtinen wrote:
> On ti, 2017-01-10 at 12:15 +0000, Chris Wilson wrote:
> > Start converting over from the byte count to its semantic macro, either
> > we want to allocate the size of a physical page in main memory or we
> > want the size of a virtual page in the GTT. 4096 could mean either, but
> > PAGE_SIZE and I915_GTT_PAGE_SIZE are explicit and should help improve
> > code comprehension and future changes. In the future, we may want to use
> > variable GTT page sizes and so have the challenge of knowing which
> > hardcoded values were used to represent a physical page vs the virtual
> > page.
> >
> > v2: Look for a few more 4096s to convert, discover IS_ALIGNED().
> >
> > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
>
> <SNIP>
>
> >
> > @@ -80,11 +80,11 @@ static void i965_write_fence_reg(struct drm_i915_fence_reg *fence,
> > > unsigned int stride = i915_gem_object_get_stride(vma->obj);
> >
> > GEM_BUG_ON(!i915_vma_is_map_and_fenceable(vma));
> > - GEM_BUG_ON(vma->node.start & 4095);
> > - GEM_BUG_ON(vma->fence_size & 4095);
> > - GEM_BUG_ON(stride & 127);
> > + GEM_BUG_ON(!IS_ALIGNED(vma->node.start, I915_GTT_PAGE_SIZE));
> > + GEM_BUG_ON(!IS_ALIGNED(vma->fence_size, I915_GTT_PAGE_SIZE));
>
> GTT_MIN_ALIGN would make more sense here? I don't think this test
> should change if page size increases.
On the other hand, the 4096 here is FENCE_PAGE units. So perhaps better
as
diff --git a/drivers/gpu/drm/i915/i915_gem_fence_reg.c b/drivers/gpu/drm/i915/i915_gem_fence_reg.c
index cd93468c6db2..d3bcacea63de 100644
--- a/drivers/gpu/drm/i915/i915_gem_fence_reg.c
+++ b/drivers/gpu/drm/i915/i915_gem_fence_reg.c
@@ -60,10 +60,14 @@
static void i965_write_fence_reg(struct drm_i915_fence_reg *fence,
struct i915_vma *vma)
{
+#define I965_FENCE_PAGE 4096UL
i915_reg_t fence_reg_lo, fence_reg_hi;
int fence_pitch_shift;
u64 val;
+ /* Make sure that we are consistent with i915_gem_fence_alignment() */
+ BUILD_BUG_ON(!IS_ALIGNED(I915_GTT_MIN_ALIGNMENT, I965_FENCE_PAGE));
+
if (INTEL_INFO(fence->i915)->gen >= 6) {
fence_reg_lo = FENCE_REG_GEN6_LO(fence->id);
fence_reg_hi = FENCE_REG_GEN6_HI(fence->id);
@@ -80,11 +84,11 @@ static void i965_write_fence_reg(struct drm_i915_fence_reg *fence,
unsigned int stride = i915_gem_object_get_stride(vma->obj);
GEM_BUG_ON(!i915_vma_is_map_and_fenceable(vma));
- GEM_BUG_ON(!IS_ALIGNED(vma->node.start, I915_GTT_PAGE_SIZE));
- GEM_BUG_ON(!IS_ALIGNED(vma->fence_size, I915_GTT_PAGE_SIZE));
+ GEM_BUG_ON(!IS_ALIGNED(vma->node.start, I965_FENCE_PAGE));
+ GEM_BUG_ON(!IS_ALIGNED(vma->fence_size, I965_FENCE_PAGE));
GEM_BUG_ON(!IS_ALIGNED(stride, 128));
- val = (vma->node.start + vma->fence_size - I915_GTT_PAGE_SIZE) << 32;
+ val = (vma->node.start + vma->fence_size - I965_FENCE_PAGE) << 32;
val |= vma->node.start;
val |= (u64)((stride / 128) - 1) << fence_pitch_shift;
if (i915_gem_object_get_tiling(vma->obj) == I915_TILING_Y)
@@ -111,6 +115,7 @@ static void i965_write_fence_reg(struct drm_i915_fence_reg *fence,
I915_WRITE(fence_reg_lo, lower_32_bits(val));
POSTING_READ(fence_reg_lo);
}
+#undef I965_FENCE_PAGE
}
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
