* [PATCH] drm/amdgpu: fix gtt_mgr bo's offset
@ 2016-09-20 9:09 Flora Cui
[not found] ` <1474362552-9062-1-git-send-email-Flora.Cui-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 10+ messages in thread
From: Flora Cui @ 2016-09-20 9:09 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Flora Cui
Change-Id: I89e9be3d5c96d46655f3a977fb557b20b4d87609
Signed-off-by: Flora Cui <Flora.Cui@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
index 262e872..b266cf7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
@@ -128,8 +128,8 @@ int amdgpu_gtt_mgr_alloc(struct ttm_mem_type_manager *man,
if (!r) {
mem->start = node->start;
- tbo->offset = (tbo->mem.start << PAGE_SHIFT) +
- tbo->bdev->man[tbo->mem.mem_type].gpu_offset;
+ tbo->offset = (mem->start << PAGE_SHIFT) +
+ tbo->bdev->man[mem->mem_type].gpu_offset;
}
return r;
--
2.7.4
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 10+ messages in thread[parent not found: <1474362552-9062-1-git-send-email-Flora.Cui-5C7GfCeVMHo@public.gmane.org>]
* Re: [PATCH] drm/amdgpu: fix gtt_mgr bo's offset [not found] ` <1474362552-9062-1-git-send-email-Flora.Cui-5C7GfCeVMHo@public.gmane.org> @ 2016-09-20 10:19 ` zhoucm1 [not found] ` <57E10D35.5040006-5C7GfCeVMHo@public.gmane.org> 0 siblings, 1 reply; 10+ messages in thread From: zhoucm1 @ 2016-09-20 10:19 UTC (permalink / raw) To: Flora Cui, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW On 2016年09月20日 17:09, Flora Cui wrote: > Change-Id: I89e9be3d5c96d46655f3a977fb557b20b4d87609 > Signed-off-by: Flora Cui <Flora.Cui@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c > index 262e872..b266cf7 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c > @@ -128,8 +128,8 @@ int amdgpu_gtt_mgr_alloc(struct ttm_mem_type_manager *man, > > if (!r) { > mem->start = node->start; > - tbo->offset = (tbo->mem.start << PAGE_SHIFT) + > - tbo->bdev->man[tbo->mem.mem_type].gpu_offset; > + tbo->offset = (mem->start << PAGE_SHIFT) + > + tbo->bdev->man[mem->mem_type].gpu_offset; mem->mem_type seems not set yet as well if I am correct. IIRC, I feel it's better that tbo->offset is set out of this function after get_node successfully. Regards, David Zhou > } > > return r; _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <57E10D35.5040006-5C7GfCeVMHo@public.gmane.org>]
* Re: [PATCH] drm/amdgpu: fix gtt_mgr bo's offset [not found] ` <57E10D35.5040006-5C7GfCeVMHo@public.gmane.org> @ 2016-09-20 10:41 ` Flora Cui 2016-09-20 11:02 ` Christian König 2016-09-20 10:43 ` Christian König 1 sibling, 1 reply; 10+ messages in thread From: Flora Cui @ 2016-09-20 10:41 UTC (permalink / raw) To: zhoucm1 Cc: christian.koenig-5C7GfCeVMHo, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW On Tue, Sep 20, 2016 at 06:19:33PM +0800, zhoucm1 wrote: > > > On 2016年09月20日 17:09, Flora Cui wrote: > >Change-Id: I89e9be3d5c96d46655f3a977fb557b20b4d87609 > >Signed-off-by: Flora Cui <Flora.Cui@amd.com> > >--- > > drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > >diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c > >index 262e872..b266cf7 100644 > >--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c > >+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c > >@@ -128,8 +128,8 @@ int amdgpu_gtt_mgr_alloc(struct ttm_mem_type_manager *man, > > if (!r) { > > mem->start = node->start; > >- tbo->offset = (tbo->mem.start << PAGE_SHIFT) + > >- tbo->bdev->man[tbo->mem.mem_type].gpu_offset; > >+ tbo->offset = (mem->start << PAGE_SHIFT) + > >+ tbo->bdev->man[mem->mem_type].gpu_offset; > mem->mem_type seems not set yet as well if I am correct. > IIRC, I feel it's better that tbo->offset is set out of this function after > get_node successfully. > how about change to tbo->bdev->man[TTM_PL_TT].gpu_offset? since gtt_mgr is for GTT only. > Regards, > David Zhou > > > } > > return r; > _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/amdgpu: fix gtt_mgr bo's offset 2016-09-20 10:41 ` Flora Cui @ 2016-09-20 11:02 ` Christian König [not found] ` <b4bb5cc9-39de-aff6-ec12-2d97d0cc9fcd-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org> 0 siblings, 1 reply; 10+ messages in thread From: Christian König @ 2016-09-20 11:02 UTC (permalink / raw) To: Flora Cui, zhoucm1 Cc: christian.koenig-5C7GfCeVMHo, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW Am 20.09.2016 um 12:41 schrieb Flora Cui: > On Tue, Sep 20, 2016 at 06:19:33PM +0800, zhoucm1 wrote: >> >> On 2016年09月20日 17:09, Flora Cui wrote: >>> Change-Id: I89e9be3d5c96d46655f3a977fb557b20b4d87609 >>> Signed-off-by: Flora Cui <Flora.Cui@amd.com> >>> --- >>> drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c >>> index 262e872..b266cf7 100644 >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c >>> @@ -128,8 +128,8 @@ int amdgpu_gtt_mgr_alloc(struct ttm_mem_type_manager *man, >>> if (!r) { >>> mem->start = node->start; >>> - tbo->offset = (tbo->mem.start << PAGE_SHIFT) + >>> - tbo->bdev->man[tbo->mem.mem_type].gpu_offset; >>> + tbo->offset = (mem->start << PAGE_SHIFT) + >>> + tbo->bdev->man[mem->mem_type].gpu_offset; >> mem->mem_type seems not set yet as well if I am correct. >> IIRC, I feel it's better that tbo->offset is set out of this function after >> get_node successfully. >> > how about change to tbo->bdev->man[TTM_PL_TT].gpu_offset? since > gtt_mgr is for GTT only. Not 100% correct either. The problem is that sometimes a GTT MM node will be bound for a BO which is still in VRAM (e.g. on eviction for example). So you need to check if (&tbo->mem == mem) and only then update the tbo's offset. Regards, Christian. > >> Regards, >> David Zhou >> >>> } >>> return r; > _______________________________________________ > amd-gfx mailing list > amd-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <b4bb5cc9-39de-aff6-ec12-2d97d0cc9fcd-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>]
* Re: [PATCH] drm/amdgpu: fix gtt_mgr bo's offset [not found] ` <b4bb5cc9-39de-aff6-ec12-2d97d0cc9fcd-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org> @ 2016-09-20 11:16 ` Flora Cui 2016-09-20 11:40 ` Christian König 2016-09-20 11:24 ` Flora Cui 1 sibling, 1 reply; 10+ messages in thread From: Flora Cui @ 2016-09-20 11:16 UTC (permalink / raw) To: Christian König Cc: zhoucm1, christian.koenig-5C7GfCeVMHo, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW [-- Attachment #1: Type: text/plain, Size: 2123 bytes --] Does this change make sense? On Tue, Sep 20, 2016 at 01:02:20PM +0200, Christian König wrote: > Am 20.09.2016 um 12:41 schrieb Flora Cui: > >On Tue, Sep 20, 2016 at 06:19:33PM +0800, zhoucm1 wrote: > >> > >>On 2016年09月20日 17:09, Flora Cui wrote: > >>>Change-Id: I89e9be3d5c96d46655f3a977fb557b20b4d87609 > >>>Signed-off-by: Flora Cui <Flora.Cui-5C7GfCeVMHo@public.gmane.org> > >>>--- > >>> drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 4 ++-- > >>> 1 file changed, 2 insertions(+), 2 deletions(-) > >>> > >>>diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c > >>>index 262e872..b266cf7 100644 > >>>--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c > >>>+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c > >>>@@ -128,8 +128,8 @@ int amdgpu_gtt_mgr_alloc(struct ttm_mem_type_manager *man, > >>> if (!r) { > >>> mem->start = node->start; > >>>- tbo->offset = (tbo->mem.start << PAGE_SHIFT) + > >>>- tbo->bdev->man[tbo->mem.mem_type].gpu_offset; > >>>+ tbo->offset = (mem->start << PAGE_SHIFT) + > >>>+ tbo->bdev->man[mem->mem_type].gpu_offset; > >>mem->mem_type seems not set yet as well if I am correct. > >>IIRC, I feel it's better that tbo->offset is set out of this function after > >>get_node successfully. > >> > >how about change to tbo->bdev->man[TTM_PL_TT].gpu_offset? since > >gtt_mgr is for GTT only. > > Not 100% correct either. > > The problem is that sometimes a GTT MM node will be bound for a BO which is > still in VRAM (e.g. on eviction for example). > > So you need to check if (&tbo->mem == mem) and only then update the tbo's > offset. > > Regards, > Christian. > > > > >>Regards, > >>David Zhou > >> > >>> } > >>> return r; > >_______________________________________________ > >amd-gfx mailing list > >amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org > >https://lists.freedesktop.org/mailman/listinfo/amd-gfx > > > _______________________________________________ > amd-gfx mailing list > amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx [-- Attachment #2: 0001-drm-amdgpu-fix-gtt_mgr-bo-s-offset.patch --] [-- Type: text/x-diff, Size: 1138 bytes --] >From 411f93d78e8f3691ce480146374c1e603c2c6db6 Mon Sep 17 00:00:00 2001 From: Flora Cui <Flora.Cui-5C7GfCeVMHo@public.gmane.org> Date: Tue, 20 Sep 2016 17:07:31 +0800 Subject: [PATCH] drm/amdgpu: fix gtt_mgr bo's offset Change-Id: I89e9be3d5c96d46655f3a977fb557b20b4d87609 Signed-off-by: Flora Cui <Flora.Cui-5C7GfCeVMHo@public.gmane.org> Reviewed-by: Hawking Zhang <Hawking.Zhang-5C7GfCeVMHo@public.gmane.org> --- drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c index 262e872..f86c844 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c @@ -128,8 +128,9 @@ int amdgpu_gtt_mgr_alloc(struct ttm_mem_type_manager *man, if (!r) { mem->start = node->start; - tbo->offset = (tbo->mem.start << PAGE_SHIFT) + - tbo->bdev->man[tbo->mem.mem_type].gpu_offset; + if (&tbo->mem == mem) + tbo->offset = (tbo->mem.start << PAGE_SHIFT) + + tbo->bdev->man[tbo->mem.mem_type].gpu_offset; } return r; -- 2.7.4 [-- Attachment #3: Type: text/plain, Size: 154 bytes --] _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/amdgpu: fix gtt_mgr bo's offset 2016-09-20 11:16 ` Flora Cui @ 2016-09-20 11:40 ` Christian König [not found] ` <ec255145-4ee3-caab-bfc5-644f4afba6ad-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org> 0 siblings, 1 reply; 10+ messages in thread From: Christian König @ 2016-09-20 11:40 UTC (permalink / raw) To: Flora Cui Cc: zhoucm1, christian.koenig-5C7GfCeVMHo, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW Am 20.09.2016 um 13:16 schrieb Flora Cui: > Does this change make sense? Yes, exactly. Patch is Reviewed-by: Christian König <christian.koenig@amd.com>. But please be aware that there is at least one other bug I'm currently investigating. Regards, Christian. > > On Tue, Sep 20, 2016 at 01:02:20PM +0200, Christian König wrote: >> Am 20.09.2016 um 12:41 schrieb Flora Cui: >>> On Tue, Sep 20, 2016 at 06:19:33PM +0800, zhoucm1 wrote: >>>> On 2016年09月20日 17:09, Flora Cui wrote: >>>>> Change-Id: I89e9be3d5c96d46655f3a977fb557b20b4d87609 >>>>> Signed-off-by: Flora Cui <Flora.Cui@amd.com> >>>>> --- >>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 4 ++-- >>>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c >>>>> index 262e872..b266cf7 100644 >>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c >>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c >>>>> @@ -128,8 +128,8 @@ int amdgpu_gtt_mgr_alloc(struct ttm_mem_type_manager *man, >>>>> if (!r) { >>>>> mem->start = node->start; >>>>> - tbo->offset = (tbo->mem.start << PAGE_SHIFT) + >>>>> - tbo->bdev->man[tbo->mem.mem_type].gpu_offset; >>>>> + tbo->offset = (mem->start << PAGE_SHIFT) + >>>>> + tbo->bdev->man[mem->mem_type].gpu_offset; >>>> mem->mem_type seems not set yet as well if I am correct. >>>> IIRC, I feel it's better that tbo->offset is set out of this function after >>>> get_node successfully. >>>> >>> how about change to tbo->bdev->man[TTM_PL_TT].gpu_offset? since >>> gtt_mgr is for GTT only. >> Not 100% correct either. >> >> The problem is that sometimes a GTT MM node will be bound for a BO which is >> still in VRAM (e.g. on eviction for example). >> >> So you need to check if (&tbo->mem == mem) and only then update the tbo's >> offset. >> >> Regards, >> Christian. >> >>>> Regards, >>>> David Zhou >>>> >>>>> } >>>>> return r; >>> _______________________________________________ >>> amd-gfx mailing list >>> amd-gfx@lists.freedesktop.org >>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx >> >> _______________________________________________ >> amd-gfx mailing list >> amd-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/amd-gfx _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <ec255145-4ee3-caab-bfc5-644f4afba6ad-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>]
* Re: [PATCH] drm/amdgpu: fix gtt_mgr bo's offset [not found] ` <ec255145-4ee3-caab-bfc5-644f4afba6ad-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org> @ 2016-09-21 1:54 ` zhoucm1 [not found] ` <57E1E83C.5070107-5C7GfCeVMHo@public.gmane.org> 0 siblings, 1 reply; 10+ messages in thread From: zhoucm1 @ 2016-09-21 1:54 UTC (permalink / raw) To: Christian König, Flora Cui Cc: christian.koenig-5C7GfCeVMHo, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW On 2016年09月20日 19:40, Christian König wrote: > Am 20.09.2016 um 13:16 schrieb Flora Cui: >> Does this change make sense? > > Yes, exactly. Patch is Reviewed-by: Christian König > <christian.koenig@amd.com>. Hi Flora, Does this change work? if (&tbo->mem == mem) this checking seems always be false, right? Regards, David Zhou > > But please be aware that there is at least one other bug I'm currently > investigating. > > Regards, > Christian. > >> >> On Tue, Sep 20, 2016 at 01:02:20PM +0200, Christian König wrote: >>> Am 20.09.2016 um 12:41 schrieb Flora Cui: >>>> On Tue, Sep 20, 2016 at 06:19:33PM +0800, zhoucm1 wrote: >>>>> On 2016年09月20日 17:09, Flora Cui wrote: >>>>>> Change-Id: I89e9be3d5c96d46655f3a977fb557b20b4d87609 >>>>>> Signed-off-by: Flora Cui <Flora.Cui@amd.com> >>>>>> --- >>>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 4 ++-- >>>>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>>>> >>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c >>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c >>>>>> index 262e872..b266cf7 100644 >>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c >>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c >>>>>> @@ -128,8 +128,8 @@ int amdgpu_gtt_mgr_alloc(struct >>>>>> ttm_mem_type_manager *man, >>>>>> if (!r) { >>>>>> mem->start = node->start; >>>>>> - tbo->offset = (tbo->mem.start << PAGE_SHIFT) + >>>>>> - tbo->bdev->man[tbo->mem.mem_type].gpu_offset; >>>>>> + tbo->offset = (mem->start << PAGE_SHIFT) + >>>>>> + tbo->bdev->man[mem->mem_type].gpu_offset; >>>>> mem->mem_type seems not set yet as well if I am correct. >>>>> IIRC, I feel it's better that tbo->offset is set out of this >>>>> function after >>>>> get_node successfully. >>>>> >>>> how about change to tbo->bdev->man[TTM_PL_TT].gpu_offset? since >>>> gtt_mgr is for GTT only. >>> Not 100% correct either. >>> >>> The problem is that sometimes a GTT MM node will be bound for a BO >>> which is >>> still in VRAM (e.g. on eviction for example). >>> >>> So you need to check if (&tbo->mem == mem) and only then update the >>> tbo's >>> offset. >>> >>> Regards, >>> Christian. >>> >>>>> Regards, >>>>> David Zhou >>>>> >>>>>> } >>>>>> return r; >>>> _______________________________________________ >>>> amd-gfx mailing list >>>> amd-gfx@lists.freedesktop.org >>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx >>> >>> _______________________________________________ >>> amd-gfx mailing list >>> amd-gfx@lists.freedesktop.org >>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx > > _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <57E1E83C.5070107-5C7GfCeVMHo@public.gmane.org>]
* Re: [PATCH] drm/amdgpu: fix gtt_mgr bo's offset [not found] ` <57E1E83C.5070107-5C7GfCeVMHo@public.gmane.org> @ 2016-09-21 3:22 ` Flora Cui 0 siblings, 0 replies; 10+ messages in thread From: Flora Cui @ 2016-09-21 3:22 UTC (permalink / raw) To: zhoucm1 Cc: Christian König, christian.koenig-5C7GfCeVMHo, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW On Wed, Sep 21, 2016 at 09:54:04AM +0800, zhoucm1 wrote: > > > On 2016年09月20日 19:40, Christian König wrote: > >Am 20.09.2016 um 13:16 schrieb Flora Cui: > >>Does this change make sense? > > > >Yes, exactly. Patch is Reviewed-by: Christian König > ><christian.koenig@amd.com>. > Hi Flora, > Does this change work? > > if (&tbo->mem == mem) > this checking seems always be false, right? mostly the check is true. eg amdgpu_bo_pin_restricted->amdgpu_ttm_bind(), the mem is &bo->tbo.mem > > Regards, > David Zhou > > > >But please be aware that there is at least one other bug I'm currently > >investigating. > > > >Regards, > >Christian. > > > >> > >>On Tue, Sep 20, 2016 at 01:02:20PM +0200, Christian König wrote: > >>>Am 20.09.2016 um 12:41 schrieb Flora Cui: > >>>>On Tue, Sep 20, 2016 at 06:19:33PM +0800, zhoucm1 wrote: > >>>>>On 2016年09月20日 17:09, Flora Cui wrote: > >>>>>>Change-Id: I89e9be3d5c96d46655f3a977fb557b20b4d87609 > >>>>>>Signed-off-by: Flora Cui <Flora.Cui@amd.com> > >>>>>>--- > >>>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 4 ++-- > >>>>>> 1 file changed, 2 insertions(+), 2 deletions(-) > >>>>>> > >>>>>>diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c > >>>>>>b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c > >>>>>>index 262e872..b266cf7 100644 > >>>>>>--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c > >>>>>>+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c > >>>>>>@@ -128,8 +128,8 @@ int amdgpu_gtt_mgr_alloc(struct > >>>>>>ttm_mem_type_manager *man, > >>>>>> if (!r) { > >>>>>> mem->start = node->start; > >>>>>>- tbo->offset = (tbo->mem.start << PAGE_SHIFT) + > >>>>>>- tbo->bdev->man[tbo->mem.mem_type].gpu_offset; > >>>>>>+ tbo->offset = (mem->start << PAGE_SHIFT) + > >>>>>>+ tbo->bdev->man[mem->mem_type].gpu_offset; > >>>>>mem->mem_type seems not set yet as well if I am correct. > >>>>>IIRC, I feel it's better that tbo->offset is set out of this > >>>>>function after > >>>>>get_node successfully. > >>>>> > >>>>how about change to tbo->bdev->man[TTM_PL_TT].gpu_offset? since > >>>>gtt_mgr is for GTT only. > >>>Not 100% correct either. > >>> > >>>The problem is that sometimes a GTT MM node will be bound for a BO > >>>which is > >>>still in VRAM (e.g. on eviction for example). > >>> > >>>So you need to check if (&tbo->mem == mem) and only then update the > >>>tbo's > >>>offset. > >>> > >>>Regards, > >>>Christian. > >>> > >>>>>Regards, > >>>>>David Zhou > >>>>> > >>>>>> } > >>>>>> return r; > >>>>_______________________________________________ > >>>>amd-gfx mailing list > >>>>amd-gfx@lists.freedesktop.org > >>>>https://lists.freedesktop.org/mailman/listinfo/amd-gfx > >>> > >>>_______________________________________________ > >>>amd-gfx mailing list > >>>amd-gfx@lists.freedesktop.org > >>>https://lists.freedesktop.org/mailman/listinfo/amd-gfx > > > > > _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/amdgpu: fix gtt_mgr bo's offset [not found] ` <b4bb5cc9-39de-aff6-ec12-2d97d0cc9fcd-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org> 2016-09-20 11:16 ` Flora Cui @ 2016-09-20 11:24 ` Flora Cui 1 sibling, 0 replies; 10+ messages in thread From: Flora Cui @ 2016-09-20 11:24 UTC (permalink / raw) To: Christian König Cc: zhoucm1, christian.koenig-5C7GfCeVMHo, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW we need this fix for urgent promotion. so please please reply me asap. On Tue, Sep 20, 2016 at 01:02:20PM +0200, Christian König wrote: > Am 20.09.2016 um 12:41 schrieb Flora Cui: > >On Tue, Sep 20, 2016 at 06:19:33PM +0800, zhoucm1 wrote: > >> > >>On 2016年09月20日 17:09, Flora Cui wrote: > >>>Change-Id: I89e9be3d5c96d46655f3a977fb557b20b4d87609 > >>>Signed-off-by: Flora Cui <Flora.Cui@amd.com> > >>>--- > >>> drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 4 ++-- > >>> 1 file changed, 2 insertions(+), 2 deletions(-) > >>> > >>>diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c > >>>index 262e872..b266cf7 100644 > >>>--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c > >>>+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c > >>>@@ -128,8 +128,8 @@ int amdgpu_gtt_mgr_alloc(struct ttm_mem_type_manager *man, > >>> if (!r) { > >>> mem->start = node->start; > >>>- tbo->offset = (tbo->mem.start << PAGE_SHIFT) + > >>>- tbo->bdev->man[tbo->mem.mem_type].gpu_offset; > >>>+ tbo->offset = (mem->start << PAGE_SHIFT) + > >>>+ tbo->bdev->man[mem->mem_type].gpu_offset; > >>mem->mem_type seems not set yet as well if I am correct. > >>IIRC, I feel it's better that tbo->offset is set out of this function after > >>get_node successfully. > >> > >how about change to tbo->bdev->man[TTM_PL_TT].gpu_offset? since > >gtt_mgr is for GTT only. > > Not 100% correct either. > > The problem is that sometimes a GTT MM node will be bound for a BO which is > still in VRAM (e.g. on eviction for example). > > So you need to check if (&tbo->mem == mem) and only then update the tbo's > offset. > > Regards, > Christian. > > > > >>Regards, > >>David Zhou > >> > >>> } > >>> return r; > >_______________________________________________ > >amd-gfx mailing list > >amd-gfx@lists.freedesktop.org > >https://lists.freedesktop.org/mailman/listinfo/amd-gfx > > > _______________________________________________ > amd-gfx mailing list > amd-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/amdgpu: fix gtt_mgr bo's offset [not found] ` <57E10D35.5040006-5C7GfCeVMHo@public.gmane.org> 2016-09-20 10:41 ` Flora Cui @ 2016-09-20 10:43 ` Christian König 1 sibling, 0 replies; 10+ messages in thread From: Christian König @ 2016-09-20 10:43 UTC (permalink / raw) To: zhoucm1, Flora Cui, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW Am 20.09.2016 um 12:19 schrieb zhoucm1: > > > On 2016年09月20日 17:09, Flora Cui wrote: >> Change-Id: I89e9be3d5c96d46655f3a977fb557b20b4d87609 >> Signed-off-by: Flora Cui <Flora.Cui@amd.com> >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c >> index 262e872..b266cf7 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c >> @@ -128,8 +128,8 @@ int amdgpu_gtt_mgr_alloc(struct >> ttm_mem_type_manager *man, >> if (!r) { >> mem->start = node->start; >> - tbo->offset = (tbo->mem.start << PAGE_SHIFT) + >> - tbo->bdev->man[tbo->mem.mem_type].gpu_offset; >> + tbo->offset = (mem->start << PAGE_SHIFT) + >> + tbo->bdev->man[mem->mem_type].gpu_offset; > mem->mem_type seems not set yet as well if I am correct. > IIRC, I feel it's better that tbo->offset is set out of this function > after get_node successfully. The patch itself is indeed not correct, but it points me to the bug I've been searching for weeks now. So thanks a lot for this. Regards, Christian. > > Regards, > David Zhou > >> } >> return r; > > _______________________________________________ > amd-gfx mailing list > amd-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-09-21 3:22 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-20 9:09 [PATCH] drm/amdgpu: fix gtt_mgr bo's offset Flora Cui
[not found] ` <1474362552-9062-1-git-send-email-Flora.Cui-5C7GfCeVMHo@public.gmane.org>
2016-09-20 10:19 ` zhoucm1
[not found] ` <57E10D35.5040006-5C7GfCeVMHo@public.gmane.org>
2016-09-20 10:41 ` Flora Cui
2016-09-20 11:02 ` Christian König
[not found] ` <b4bb5cc9-39de-aff6-ec12-2d97d0cc9fcd-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
2016-09-20 11:16 ` Flora Cui
2016-09-20 11:40 ` Christian König
[not found] ` <ec255145-4ee3-caab-bfc5-644f4afba6ad-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
2016-09-21 1:54 ` zhoucm1
[not found] ` <57E1E83C.5070107-5C7GfCeVMHo@public.gmane.org>
2016-09-21 3:22 ` Flora Cui
2016-09-20 11:24 ` Flora Cui
2016-09-20 10:43 ` Christian König
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.