| Message ID | 20240227202645.20111-1-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/ttm: Fix TTM_PL_FLAG_DESIRED | expand |
Am 27.02.24 um 21:26 schrieb Ville Syrjala: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > inlined from ‘i915_ttm_get_pages’ at ../drivers/gpu/drm/i915/gem/i915_gem_ttm.c:847:2: > ../drivers/gpu/drm/i915/gem/i915_gem_ttm.c:165:18: warning: ‘places[0].flags’ is used uninitialized [-Wuninitialized] > 165 | places[0].flags |= TTM_PL_FLAG_DESIRED; > | ~~~~~~~~~^~~~~~ > ../drivers/gpu/drm/i915/gem/i915_gem_ttm.c: In function ‘i915_ttm_get_pages’: > ../drivers/gpu/drm/i915/gem/i915_gem_ttm.c:837:26: note: ‘places’ declared here > 837 | struct ttm_place places[I915_TTM_MAX_PLACEMENTS + 1]; > | ^~~~~~ > > Furhermore we then proceed to call i915_ttm_place_from_region() which > memset()s the whole thing back to zero anyway. So in the end we lose > the TTM_PL_FLAG_DESIRED flag (and fortunately also whatever else stack > garbage happened to be in the flags at this point). > > No idea what functional changes this will result in... I've already send out the same patch yesterday. Please review that one. Sorry for the noise, didn't realized that i915_ttm_place_from_region() was initializing the flags and not the caller while converting this. Thanks, Christian. > > Cc: Somalapuram Amaranath <Amaranath.Somalapuram@amd.com> > Cc: Christian König <christian.koenig@amd.com> > Cc: Zack Rusin <zack.rusin@broadcom.com> > Cc: Thomas Zimmermann <tzimmermann@suse.de> > Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com> > Fixes: a78a8da51b36 ("drm/ttm: replace busy placement with flags v6") > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c > index 27dcfd8a34bb..e6f177183c0f 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c > @@ -162,10 +162,10 @@ i915_ttm_placement_from_obj(const struct drm_i915_gem_object *obj, > unsigned int flags = obj->flags; > unsigned int i; > > - places[0].flags |= TTM_PL_FLAG_DESIRED; > i915_ttm_place_from_region(num_allowed ? obj->mm.placements[0] : > obj->mm.region, &places[0], obj->bo_offset, > obj->base.size, flags); > + places[0].flags |= TTM_PL_FLAG_DESIRED; > > /* Cache this on object? */ > for (i = 0; i < num_allowed; ++i) {
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c index 27dcfd8a34bb..e6f177183c0f 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c @@ -162,10 +162,10 @@ i915_ttm_placement_from_obj(const struct drm_i915_gem_object *obj, unsigned int flags = obj->flags; unsigned int i; - places[0].flags |= TTM_PL_FLAG_DESIRED; i915_ttm_place_from_region(num_allowed ? obj->mm.placements[0] : obj->mm.region, &places[0], obj->bo_offset, obj->base.size, flags); + places[0].flags |= TTM_PL_FLAG_DESIRED; /* Cache this on object? */ for (i = 0; i < num_allowed; ++i) {