Re: [Intel-gfx] [PATCH v2 4/4] drm/i915: Codify our assumption that the Global GTT is <= 4GiB
Joonas Lahtinenwrites: > [ text/plain ] > From: Chris Wilson > > Throughout the code base, we use u32 for offsets into the global GTT. If > we ever see any hardware with a larger GGTT, then we run the real risk > of silent corruption. So test for our assumption up front so that we > have a nice reminder should the time come when it fails. > > Signed-off-by: Chris Wilson > Cc: Ville Syrjälä > Cc: Daniel Vetter > Signed-off-by: Joonas Lahtinen > [Rebased and changed 1ull -> 1ULL] > --- > drivers/gpu/drm/i915/i915_gem_gtt.c | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c > b/drivers/gpu/drm/i915/i915_gem_gtt.c > index 8a9fa03..ba144ba 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -3191,6 +3191,13 @@ int i915_gem_gtt_init(struct drm_device *dev) > if (ret) > return ret; > > + if ((ggtt->base.total - 1) >> 32) { > + DRM_ERROR("We never expected a Global GTT with more than 32bits > of address space! Found %lldM!\n", > + ggtt->base.total >> 20); > + ggtt->base.total = 1ULL << 32; > + ggtt->mappable_end = min(ggtt->mappable_end, > ggtt->base.total); mappable_end is not u64 so perhaps s/min/min_t(u64, The mappable_end is really confusing after you realize it is used in 2 different contexts where only in the other, it holds true. In the pa space, it definitely is not end but len. If we would rename it to mappable_len it would be ambivalent to context and more clear. With min_t change, Reviewed-by: Mika Kuoppala > + } > + > /* >* Initialise stolen early so that we may reserve preallocated >* objects for the BIOS to KMS transition. > -- > 2.5.0 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 4/4] drm/i915: Codify our assumption that the Global GTT is <= 4GiB
On 18/03/16 08:42, Joonas Lahtinen wrote: From: Chris WilsonThroughout the code base, we use u32 for offsets into the global GTT. If we ever see any hardware with a larger GGTT, then we run the real risk of silent corruption. So test for our assumption up front so that we have a nice reminder should the time come when it fails. Signed-off-by: Chris Wilson Cc: Ville Syrjälä Cc: Daniel Vetter Signed-off-by: Joonas Lahtinen [Rebased and changed 1ull -> 1ULL] --- drivers/gpu/drm/i915/i915_gem_gtt.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 8a9fa03..ba144ba 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -3191,6 +3191,13 @@ int i915_gem_gtt_init(struct drm_device *dev) if (ret) return ret; + if ((ggtt->base.total - 1) >> 32) { + DRM_ERROR("We never expected a Global GTT with more than 32bits of address space! Found %lldM!\n", + ggtt->base.total >> 20); MB or MiB? + ggtt->base.total = 1ULL << 32; + ggtt->mappable_end = min(ggtt->mappable_end, ggtt->base.total); + } + /* * Initialise stolen early so that we may reserve preallocated * objects for the BIOS to KMS transition. Reviewed-by: Tvrtko Ursulin Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 4/4] drm/i915: Codify our assumption that the Global GTT is <= 4GiB
Mika Kuoppalawrites: > [ text/plain ] > Joonas Lahtinen writes: > >> [ text/plain ] >> From: Chris Wilson >> >> Throughout the code base, we use u32 for offsets into the global GTT. If >> we ever see any hardware with a larger GGTT, then we run the real risk >> of silent corruption. So test for our assumption up front so that we >> have a nice reminder should the time come when it fails. >> >> Signed-off-by: Chris Wilson >> Cc: Ville Syrjälä >> Cc: Daniel Vetter >> Signed-off-by: Joonas Lahtinen >> [Rebased and changed 1ull -> 1ULL] >> --- >> drivers/gpu/drm/i915/i915_gem_gtt.c | 7 +++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c >> b/drivers/gpu/drm/i915/i915_gem_gtt.c >> index 8a9fa03..ba144ba 100644 >> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c >> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c >> @@ -3191,6 +3191,13 @@ int i915_gem_gtt_init(struct drm_device *dev) >> if (ret) >> return ret; >> >> +if ((ggtt->base.total - 1) >> 32) { >> +DRM_ERROR("We never expected a Global GTT with more than 32bits >> of address space! Found %lldM!\n", >> + ggtt->base.total >> 20); >> +ggtt->base.total = 1ULL << 32; >> +ggtt->mappable_end = min(ggtt->mappable_end, >> ggtt->base.total); > > mappable_end is not u64 so perhaps > s/min/min_t(u64, > Joonas pointed in irc that it is u64. Have been quite long already. You can add my r-b without any change. -Mika > The mappable_end is really confusing after you realize it is used in 2 > different contexts where only in the other, it holds true. In the pa > space, it definitely is not end but len. If we would rename it to > mappable_len it would be ambivalent to context and more clear. > > With min_t change, > Reviewed-by: Mika Kuoppala > >> +} >> + >> /* >> * Initialise stolen early so that we may reserve preallocated >> * objects for the BIOS to KMS transition. >> -- >> 2.5.0 >> >> ___ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 4/4] drm/i915: Codify our assumption that the Global GTT is <= 4GiB
On Fri, Mar 18, 2016 at 10:56:55AM +, Tvrtko Ursulin wrote: > > On 18/03/16 08:42, Joonas Lahtinen wrote: > >From: Chris Wilson> > > >Throughout the code base, we use u32 for offsets into the global GTT. If > >we ever see any hardware with a larger GGTT, then we run the real risk > >of silent corruption. So test for our assumption up front so that we > >have a nice reminder should the time come when it fails. > > > >Signed-off-by: Chris Wilson > >Cc: Ville Syrjälä > >Cc: Daniel Vetter > >Signed-off-by: Joonas Lahtinen > >[Rebased and changed 1ull -> 1ULL] > >--- > > drivers/gpu/drm/i915/i915_gem_gtt.c | 7 +++ > > 1 file changed, 7 insertions(+) > > > >diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c > >b/drivers/gpu/drm/i915/i915_gem_gtt.c > >index 8a9fa03..ba144ba 100644 > >--- a/drivers/gpu/drm/i915/i915_gem_gtt.c > >+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > >@@ -3191,6 +3191,13 @@ int i915_gem_gtt_init(struct drm_device *dev) > > if (ret) > > return ret; > > > >+if ((ggtt->base.total - 1) >> 32) { > >+DRM_ERROR("We never expected a Global GTT with more than 32bits > >of address space! Found %lldM!\n", > >+ ggtt->base.total >> 20); > > MB or MiB? if ((ggtt->base.total - 1) >> 32) { char buf[10]; string_get_size(ggtt->base.total, 1, STRING_UNITS_2, buf, sizeof(buf)); DRM_ERROR("We never expected a Global GTT with more than 32bits of address space! Found %s [%dbits]\n", buf, fls(size)); } -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 4/4] drm/i915: Codify our assumption that the Global GTT is <= 4GiB
From: Chris WilsonThroughout the code base, we use u32 for offsets into the global GTT. If we ever see any hardware with a larger GGTT, then we run the real risk of silent corruption. So test for our assumption up front so that we have a nice reminder should the time come when it fails. Signed-off-by: Chris Wilson Cc: Ville Syrjälä Cc: Daniel Vetter Signed-off-by: Joonas Lahtinen [Rebased and changed 1ull -> 1ULL] --- drivers/gpu/drm/i915/i915_gem_gtt.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 8a9fa03..ba144ba 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -3191,6 +3191,13 @@ int i915_gem_gtt_init(struct drm_device *dev) if (ret) return ret; + if ((ggtt->base.total - 1) >> 32) { + DRM_ERROR("We never expected a Global GTT with more than 32bits of address space! Found %lldM!\n", + ggtt->base.total >> 20); + ggtt->base.total = 1ULL << 32; + ggtt->mappable_end = min(ggtt->mappable_end, ggtt->base.total); + } + /* * Initialise stolen early so that we may reserve preallocated * objects for the BIOS to KMS transition. -- 2.5.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx