| Message ID | 20200930055459.31310-1-airlied@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/ttm: fix regression in ttm moves | expand |
Uggh this is part of the mess with the revert, I'm not sure how best to dig out of this one yet. Dave. On Wed, 30 Sep 2020 at 15:55, Dave Airlie <airlied@gmail.com> wrote: > > From: Dave Airlie <airlied@redhat.com> > > This fixes a bug introduced in be1213a341a289afc51f89181c310e368fba0b66 > drm/ttm: remove TTM_MEMTYPE_FLAG_FIXED v2 > > On vmwgfx this causes a Command buffer error WARN to trigger. > > This is because the old code used to check if bo->ttm was true, > and the new code doesn't, fix it code to add back the check resolves > the issue. > > Fixes: be1213a341a2 ("drm/ttm: remove TTM_MEMTYPE_FLAG_FIXED v2") > Signed-off-by: Dave Airlie <airlied@redhat.com> > --- > drivers/gpu/drm/ttm/ttm_bo.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c > index 70b3bee27850..e8aa2fe8e9d1 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo.c > +++ b/drivers/gpu/drm/ttm/ttm_bo.c > @@ -251,9 +251,11 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo, > /* Zero init the new TTM structure if the old location should > * have used one as well. > */ > - ret = ttm_tt_create(bo, old_man->use_tt); > - if (ret) > - goto out_err; > + if (!bo->ttm) { > + ret = ttm_tt_create(bo, old_man->use_tt); > + if (ret) > + goto out_err; > + } > > ret = ttm_tt_set_placement_caching(bo->ttm, mem->placement); > if (ret) > -- > 2.20.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
just FYI I'm seeing a regression on vmwgfx with drm-fixes and drm-next merged into it. I'm going take some time to dig through and work out where, the regression is a command failure and a ioremap failure. Dave. On Wed, 30 Sep 2020 at 16:26, Dave Airlie <airlied@gmail.com> wrote: > > Uggh this is part of the mess with the revert, I'm not sure how best > to dig out of this one yet. > > Dave. > > On Wed, 30 Sep 2020 at 15:55, Dave Airlie <airlied@gmail.com> wrote: > > > > From: Dave Airlie <airlied@redhat.com> > > > > This fixes a bug introduced in be1213a341a289afc51f89181c310e368fba0b66 > > drm/ttm: remove TTM_MEMTYPE_FLAG_FIXED v2 > > > > On vmwgfx this causes a Command buffer error WARN to trigger. > > > > This is because the old code used to check if bo->ttm was true, > > and the new code doesn't, fix it code to add back the check resolves > > the issue. > > > > Fixes: be1213a341a2 ("drm/ttm: remove TTM_MEMTYPE_FLAG_FIXED v2") > > Signed-off-by: Dave Airlie <airlied@redhat.com> > > --- > > drivers/gpu/drm/ttm/ttm_bo.c | 8 +++++--- > > 1 file changed, 5 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c > > index 70b3bee27850..e8aa2fe8e9d1 100644 > > --- a/drivers/gpu/drm/ttm/ttm_bo.c > > +++ b/drivers/gpu/drm/ttm/ttm_bo.c > > @@ -251,9 +251,11 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo, > > /* Zero init the new TTM structure if the old location should > > * have used one as well. > > */ > > - ret = ttm_tt_create(bo, old_man->use_tt); > > - if (ret) > > - goto out_err; > > + if (!bo->ttm) { > > + ret = ttm_tt_create(bo, old_man->use_tt); > > + if (ret) > > + goto out_err; > > + } > > > > ret = ttm_tt_set_placement_caching(bo->ttm, mem->placement); > > if (ret) > > -- > > 2.20.1 > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
That sounds like the same problem I've got when drm-next was merged into drm-misc-next. I've fixed it in this commit: commit 0b06286579b81449b1e8f14f88d3a8db091fd443 Author: Christian König <christian.koenig@amd.com> Date: Wed Aug 19 15:27:48 2020 +0200 drm/ttm: fix broken merge between drm-next and drm-misc-next drm-next reverted the changes to ttm_tt_create() to do the NULL check inside the function, but drm-misc-next adds new users of this approach. Re-apply the NULL check change inside the function to fix this. Signed-off-by: Christian König <christian.koenig@amd.com> Reviewed-by: Dave Airlie <airlied@redhat.com> Link: https://patchwork.freedesktop.org/patch/386628/ Not sure why it should cause problems with drm-fixes and drm-next as well. Regards, Christian. Am 30.09.20 um 09:09 schrieb Dave Airlie: > just FYI I'm seeing a regression on vmwgfx with drm-fixes and drm-next > merged into it. > > I'm going take some time to dig through and work out where, the > regression is a command failure and a ioremap failure. > > Dave. > > On Wed, 30 Sep 2020 at 16:26, Dave Airlie <airlied@gmail.com> wrote: >> Uggh this is part of the mess with the revert, I'm not sure how best >> to dig out of this one yet. >> >> Dave. >> >> On Wed, 30 Sep 2020 at 15:55, Dave Airlie <airlied@gmail.com> wrote: >>> From: Dave Airlie <airlied@redhat.com> >>> >>> This fixes a bug introduced in be1213a341a289afc51f89181c310e368fba0b66 >>> drm/ttm: remove TTM_MEMTYPE_FLAG_FIXED v2 >>> >>> On vmwgfx this causes a Command buffer error WARN to trigger. >>> >>> This is because the old code used to check if bo->ttm was true, >>> and the new code doesn't, fix it code to add back the check resolves >>> the issue. >>> >>> Fixes: be1213a341a2 ("drm/ttm: remove TTM_MEMTYPE_FLAG_FIXED v2") >>> Signed-off-by: Dave Airlie <airlied@redhat.com> >>> --- >>> drivers/gpu/drm/ttm/ttm_bo.c | 8 +++++--- >>> 1 file changed, 5 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c >>> index 70b3bee27850..e8aa2fe8e9d1 100644 >>> --- a/drivers/gpu/drm/ttm/ttm_bo.c >>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c >>> @@ -251,9 +251,11 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo, >>> /* Zero init the new TTM structure if the old location should >>> * have used one as well. >>> */ >>> - ret = ttm_tt_create(bo, old_man->use_tt); >>> - if (ret) >>> - goto out_err; >>> + if (!bo->ttm) { >>> + ret = ttm_tt_create(bo, old_man->use_tt); >>> + if (ret) >>> + goto out_err; >>> + } >>> >>> ret = ttm_tt_set_placement_caching(bo->ttm, mem->placement); >>> if (ret) >>> -- >>> 2.20.1 >>> >>> _______________________________________________ >>> dri-devel mailing list >>> dri-devel@lists.freedesktop.org >>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&data=02%7C01%7Cchristian.koenig%40amd.com%7Ca8e51dce1b1346015c1e08d8650fdc59%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637370466085507013&sdata=QrtSgfkmSpNcNfdJ71YNATS0URyEcMNLeMVmOenRpak%3D&reserved=0
Tracked it down to my init mem type changes, patch is on the list. Dave. On Wed, 30 Sep 2020 at 18:28, Christian König <christian.koenig@amd.com> wrote: > > That sounds like the same problem I've got when drm-next was merged into > drm-misc-next. > > I've fixed it in this commit: > > commit 0b06286579b81449b1e8f14f88d3a8db091fd443 > Author: Christian König <christian.koenig@amd.com> > Date: Wed Aug 19 15:27:48 2020 +0200 > > drm/ttm: fix broken merge between drm-next and drm-misc-next > > drm-next reverted the changes to ttm_tt_create() to do the > NULL check inside the function, but drm-misc-next adds new > users of this approach. > > Re-apply the NULL check change inside the function to fix this. > > Signed-off-by: Christian König <christian.koenig@amd.com> > Reviewed-by: Dave Airlie <airlied@redhat.com> > Link: https://patchwork.freedesktop.org/patch/386628/ > > > Not sure why it should cause problems with drm-fixes and drm-next as well. > > Regards, > Christian. > > Am 30.09.20 um 09:09 schrieb Dave Airlie: > > just FYI I'm seeing a regression on vmwgfx with drm-fixes and drm-next > > merged into it. > > > > I'm going take some time to dig through and work out where, the > > regression is a command failure and a ioremap failure. > > > > Dave. > > > > On Wed, 30 Sep 2020 at 16:26, Dave Airlie <airlied@gmail.com> wrote: > >> Uggh this is part of the mess with the revert, I'm not sure how best > >> to dig out of this one yet. > >> > >> Dave. > >> > >> On Wed, 30 Sep 2020 at 15:55, Dave Airlie <airlied@gmail.com> wrote: > >>> From: Dave Airlie <airlied@redhat.com> > >>> > >>> This fixes a bug introduced in be1213a341a289afc51f89181c310e368fba0b66 > >>> drm/ttm: remove TTM_MEMTYPE_FLAG_FIXED v2 > >>> > >>> On vmwgfx this causes a Command buffer error WARN to trigger. > >>> > >>> This is because the old code used to check if bo->ttm was true, > >>> and the new code doesn't, fix it code to add back the check resolves > >>> the issue. > >>> > >>> Fixes: be1213a341a2 ("drm/ttm: remove TTM_MEMTYPE_FLAG_FIXED v2") > >>> Signed-off-by: Dave Airlie <airlied@redhat.com> > >>> --- > >>> drivers/gpu/drm/ttm/ttm_bo.c | 8 +++++--- > >>> 1 file changed, 5 insertions(+), 3 deletions(-) > >>> > >>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c > >>> index 70b3bee27850..e8aa2fe8e9d1 100644 > >>> --- a/drivers/gpu/drm/ttm/ttm_bo.c > >>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c > >>> @@ -251,9 +251,11 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo, > >>> /* Zero init the new TTM structure if the old location should > >>> * have used one as well. > >>> */ > >>> - ret = ttm_tt_create(bo, old_man->use_tt); > >>> - if (ret) > >>> - goto out_err; > >>> + if (!bo->ttm) { > >>> + ret = ttm_tt_create(bo, old_man->use_tt); > >>> + if (ret) > >>> + goto out_err; > >>> + } > >>> > >>> ret = ttm_tt_set_placement_caching(bo->ttm, mem->placement); > >>> if (ret) > >>> -- > >>> 2.20.1 > >>> > >>> _______________________________________________ > >>> dri-devel mailing list > >>> dri-devel@lists.freedesktop.org > >>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&data=02%7C01%7Cchristian.koenig%40amd.com%7Ca8e51dce1b1346015c1e08d8650fdc59%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637370466085507013&sdata=QrtSgfkmSpNcNfdJ71YNATS0URyEcMNLeMVmOenRpak%3D&reserved=0 >
I don't see the patch, can you point me to it? Thanks, Christian. Am 01.10.20 um 07:25 schrieb Dave Airlie: > Tracked it down to my init mem type changes, patch is on the list. > > Dave. > > On Wed, 30 Sep 2020 at 18:28, Christian König <christian.koenig@amd.com> wrote: >> That sounds like the same problem I've got when drm-next was merged into >> drm-misc-next. >> >> I've fixed it in this commit: >> >> commit 0b06286579b81449b1e8f14f88d3a8db091fd443 >> Author: Christian König <christian.koenig@amd.com> >> Date: Wed Aug 19 15:27:48 2020 +0200 >> >> drm/ttm: fix broken merge between drm-next and drm-misc-next >> >> drm-next reverted the changes to ttm_tt_create() to do the >> NULL check inside the function, but drm-misc-next adds new >> users of this approach. >> >> Re-apply the NULL check change inside the function to fix this. >> >> Signed-off-by: Christian König <christian.koenig@amd.com> >> Reviewed-by: Dave Airlie <airlied@redhat.com> >> Link: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.freedesktop.org%2Fpatch%2F386628%2F&data=02%7C01%7Cchristian.koenig%40amd.com%7C9f3aa2353f0441ccefa408d865ca66e6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637371267277998534&sdata=43ZGNw5IrGmbMsVzm75WXk%2Bp55CIK0Lj%2BVXFCZRSqxE%3D&reserved=0 >> >> >> Not sure why it should cause problems with drm-fixes and drm-next as well. >> >> Regards, >> Christian. >> >> Am 30.09.20 um 09:09 schrieb Dave Airlie: >>> just FYI I'm seeing a regression on vmwgfx with drm-fixes and drm-next >>> merged into it. >>> >>> I'm going take some time to dig through and work out where, the >>> regression is a command failure and a ioremap failure. >>> >>> Dave. >>> >>> On Wed, 30 Sep 2020 at 16:26, Dave Airlie <airlied@gmail.com> wrote: >>>> Uggh this is part of the mess with the revert, I'm not sure how best >>>> to dig out of this one yet. >>>> >>>> Dave. >>>> >>>> On Wed, 30 Sep 2020 at 15:55, Dave Airlie <airlied@gmail.com> wrote: >>>>> From: Dave Airlie <airlied@redhat.com> >>>>> >>>>> This fixes a bug introduced in be1213a341a289afc51f89181c310e368fba0b66 >>>>> drm/ttm: remove TTM_MEMTYPE_FLAG_FIXED v2 >>>>> >>>>> On vmwgfx this causes a Command buffer error WARN to trigger. >>>>> >>>>> This is because the old code used to check if bo->ttm was true, >>>>> and the new code doesn't, fix it code to add back the check resolves >>>>> the issue. >>>>> >>>>> Fixes: be1213a341a2 ("drm/ttm: remove TTM_MEMTYPE_FLAG_FIXED v2") >>>>> Signed-off-by: Dave Airlie <airlied@redhat.com> >>>>> --- >>>>> drivers/gpu/drm/ttm/ttm_bo.c | 8 +++++--- >>>>> 1 file changed, 5 insertions(+), 3 deletions(-) >>>>> >>>>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c >>>>> index 70b3bee27850..e8aa2fe8e9d1 100644 >>>>> --- a/drivers/gpu/drm/ttm/ttm_bo.c >>>>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c >>>>> @@ -251,9 +251,11 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo, >>>>> /* Zero init the new TTM structure if the old location should >>>>> * have used one as well. >>>>> */ >>>>> - ret = ttm_tt_create(bo, old_man->use_tt); >>>>> - if (ret) >>>>> - goto out_err; >>>>> + if (!bo->ttm) { >>>>> + ret = ttm_tt_create(bo, old_man->use_tt); >>>>> + if (ret) >>>>> + goto out_err; >>>>> + } >>>>> >>>>> ret = ttm_tt_set_placement_caching(bo->ttm, mem->placement); >>>>> if (ret) >>>>> -- >>>>> 2.20.1 >>>>> >>>>> _______________________________________________ >>>>> dri-devel mailing list >>>>> dri-devel@lists.freedesktop.org >>>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&data=02%7C01%7Cchristian.koenig%40amd.com%7C9f3aa2353f0441ccefa408d865ca66e6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637371267277998534&sdata=Epqzw7O0bZ6G3JjXRdc2KtSug3JVucUbvlhJLtDMkzA%3D&reserved=0
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 70b3bee27850..e8aa2fe8e9d1 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -251,9 +251,11 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo, /* Zero init the new TTM structure if the old location should * have used one as well. */ - ret = ttm_tt_create(bo, old_man->use_tt); - if (ret) - goto out_err; + if (!bo->ttm) { + ret = ttm_tt_create(bo, old_man->use_tt); + if (ret) + goto out_err; + } ret = ttm_tt_set_placement_caching(bo->ttm, mem->placement); if (ret)