| Message ID | 20240226142759.93130-1-christian.koenig@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: fix applying placement flag | expand |
Gentle ping. Can I get an rb for that? Thanks, Christian. Am 26.02.24 um 15:27 schrieb Christian König: > Switching from a separate list to flags introduced a bug here. > > We were accidentially ORing the flag before initailizing the placement > and not after. So this code didn't do nothing except producing a > warning. > > Signed-off-by: Christian König <christian.koenig@amd.com> > Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> > Fixes: a78a8da51b36 ("drm/ttm: replace busy placement with flags v6") > --- > 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 a6b0aaf30cbe..7264fb08eee8 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) {
Hi Christian, On Thu, 29 Feb 2024 14:01:05 +0100 Christian König <christian.koenig@amd.com> wrote: > > Gentle ping. Can I get an rb for that? If it is me you are waiting for, I am not really able to review it (or test its functionality), but I did apply it to the merge of the drm tree yesterday and it did fix the build problem. Tested-by: Stephen Rothwell <sfr@canb.auug.org.au> # compile only
On Thu, Feb 29, 2024 at 02:01:05PM +0100, Christian König wrote: >Gentle ping. Can I get an rb for that? > >Thanks, >Christian. Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com> For some reason CI failed, but can't be related with this change. I re-triggered it to see if we can get a green run before merging. thanks Lucas De Marchi > >Am 26.02.24 um 15:27 schrieb Christian König: >>Switching from a separate list to flags introduced a bug here. >> >>We were accidentially ORing the flag before initailizing the placement >>and not after. So this code didn't do nothing except producing a >>warning. >> >>Signed-off-by: Christian König <christian.koenig@amd.com> >>Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> >>Fixes: a78a8da51b36 ("drm/ttm: replace busy placement with flags v6") >>--- >> 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 a6b0aaf30cbe..7264fb08eee8 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) { >
Am 01.03.24 um 17:04 schrieb Lucas De Marchi: > On Thu, Feb 29, 2024 at 02:01:05PM +0100, Christian König wrote: >> Gentle ping. Can I get an rb for that? >> >> Thanks, >> Christian. > > Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com> Thanks! > > For some reason CI failed, but can't be related with this change. > I re-triggered it to see if we can get a green run before merging. Do you want to pick it into a i915 branch or should I push it to drm-misc-next(-fixes) then? Christian. > > thanks > Lucas De Marchi > >> >> Am 26.02.24 um 15:27 schrieb Christian König: >>> Switching from a separate list to flags introduced a bug here. >>> >>> We were accidentially ORing the flag before initailizing the placement >>> and not after. So this code didn't do nothing except producing a >>> warning. >>> >>> Signed-off-by: Christian König <christian.koenig@amd.com> >>> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> >>> Fixes: a78a8da51b36 ("drm/ttm: replace busy placement with flags v6") >>> --- >>> 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 a6b0aaf30cbe..7264fb08eee8 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) { >>
On Fri, Mar 01, 2024 at 05:06:16PM +0100, Christian König wrote: >Am 01.03.24 um 17:04 schrieb Lucas De Marchi: >>On Thu, Feb 29, 2024 at 02:01:05PM +0100, Christian König wrote: >>>Gentle ping. Can I get an rb for that? >>> >>>Thanks, >>>Christian. >> >>Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com> > >Thanks! > >> >>For some reason CI failed, but can't be related with this change. >>I re-triggered it to see if we can get a green run before merging. > >Do you want to pick it into a i915 branch or should I push it to >drm-misc-next(-fixes) then? You can push it through drm-misc since that's where a78a8da51b36 came from. If we feel like waiting CI to come back, let's wait a few hours. Such an obvious fix for a build breakage that I'm not opposed to simply letting it through though. Lucas De Marchi > >Christian. > >> >>thanks >>Lucas De Marchi >> >>> >>>Am 26.02.24 um 15:27 schrieb Christian König: >>>>Switching from a separate list to flags introduced a bug here. >>>> >>>>We were accidentially ORing the flag before initailizing the placement >>>>and not after. So this code didn't do nothing except producing a >>>>warning. >>>> >>>>Signed-off-by: Christian König <christian.koenig@amd.com> >>>>Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> >>>>Fixes: a78a8da51b36 ("drm/ttm: replace busy placement with flags v6") >>>>--- >>>> 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 a6b0aaf30cbe..7264fb08eee8 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 a6b0aaf30cbe..7264fb08eee8 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) {
Switching from a separate list to flags introduced a bug here. We were accidentially ORing the flag before initailizing the placement and not after. So this code didn't do nothing except producing a warning. Signed-off-by: Christian König <christian.koenig@amd.com> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> Fixes: a78a8da51b36 ("drm/ttm: replace busy placement with flags v6") --- drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)