amd-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* Re: [RFC PATCH 1/2] drm/amdgpu: add direct ib pool
@ 2020-03-26  6:36 Koenig, Christian
  2020-03-26  6:45 ` Pan, Xinhui
  0 siblings, 1 reply; 8+ messages in thread
From: Koenig, Christian @ 2020-03-26  6:36 UTC (permalink / raw)
  To: Pan, Xinhui; +Cc: Deucher, Alexander, Kuehling, Felix, amd-gfx


[-- Attachment #1.1: Type: text/plain, Size: 6831 bytes --]



Am 26.03.2020 07:15 schrieb "Pan, Xinhui" <Xinhui.Pan@amd.com>:


> 2020年3月26日 13:38,Koenig, Christian <Christian.Koenig@amd.com> 写道:
>
> Yeah that's on my TODO list for quite a while as well.
>
> But we even need three IB pools. One very small for the IB tests, one for direct VM updates and one for the rest.
>
> So please make the pool a parameter to ib_get() and not the hack you have below.

yep, I will make IB pool  a parameter.

IB tests for gfx need many IBs, PAGE_SIZE for ib pool is still not enough.
but the default size for ib pool is 2MB now, just one hugepage, today we have memory in TB.
so no need make a different size for IB tests pool.

2MB is probably a bit much and we don't have huge page optimisation for kernel allocations at the moment anyway. Keep in mind that we have only limited space in the GART.

Maybe make this 4*PAGE_SIZE for the new IB pool for now and test if that works or not.

Christian.




>
> Thanks,
> Christian.
>
> Am 26.03.2020 03:02 schrieb "Pan, Xinhui" <Xinhui.Pan@amd.com>:
> Another ib poll for direct submit.
> Any jobs schedule IBs without dependence on gpu scheduler should use
> this pool firstly.
>
> Signed-off-by: xinhui pan <xinhui.pan@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h     |  1 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c  |  2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c  | 12 ++++++++++--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_job.c |  8 +++++++-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_job.h |  3 ++-
>  5 files changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 7dd74253e7b6..c01423ffb8ed 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -849,6 +849,7 @@ struct amdgpu_device {
>          struct amdgpu_ring              *rings[AMDGPU_MAX_RINGS];
>          bool                            ib_pool_ready;
>          struct amdgpu_sa_manager        ring_tmp_bo;
> +       struct amdgpu_sa_manager        ring_tmp_bo_direct;
>
>          /* interrupts */
>          struct amdgpu_irq               irq;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 8304d0c87899..28be4efb3d5b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -920,7 +920,7 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device *adev,
>                  parser->entity = entity;
>
>                  ring = to_amdgpu_ring(entity->rq->sched);
> -               r =  amdgpu_ib_get(adev, vm, ring->funcs->parse_cs ?
> +               r =  amdgpu_ib_get(adev, (unsigned long )vm|0x1, ring->funcs->parse_cs ?
>                                     chunk_ib->ib_bytes : 0, ib);
>                  if (r) {
>                          DRM_ERROR("Failed to get ib !\n");
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> index bece01f1cf09..f2e08c372d57 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> @@ -66,7 +66,7 @@ int amdgpu_ib_get(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>          int r;
>
>          if (size) {
> -               r = amdgpu_sa_bo_new(&adev->ring_tmp_bo,
> +               r = amdgpu_sa_bo_new(vm ? &adev->ring_tmp_bo : &adev->ring_tmp_bo_direct,
>                                        &ib->sa_bo, size, 256);
>                  if (r) {
>                          dev_err(adev->dev, "failed to get a new IB (%d)\n", r);
> @@ -75,7 +75,7 @@ int amdgpu_ib_get(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>
>                  ib->ptr = amdgpu_sa_bo_cpu_addr(ib->sa_bo);
>
> -               if (!vm)
> +               if (!((unsigned long)vm & ~0x1))
>                          ib->gpu_addr = amdgpu_sa_bo_gpu_addr(ib->sa_bo);
>          }
>
> @@ -310,6 +310,13 @@ int amdgpu_ib_pool_init(struct amdgpu_device *adev)
>                  return r;
>          }
>
> +       r = amdgpu_sa_bo_manager_init(adev, &adev->ring_tmp_bo_direct,
> +                                     AMDGPU_IB_POOL_SIZE*64*1024,
> +                                     AMDGPU_GPU_PAGE_SIZE,
> +                                     AMDGPU_GEM_DOMAIN_GTT);
> +       if (r) {
> +               return r;
> +       }
>          adev->ib_pool_ready = true;
>
>          return 0;
> @@ -327,6 +334,7 @@ void amdgpu_ib_pool_fini(struct amdgpu_device *adev)
>  {
>          if (adev->ib_pool_ready) {
>                  amdgpu_sa_bo_manager_fini(adev, &adev->ring_tmp_bo);
> +               amdgpu_sa_bo_manager_fini(adev, &adev->ring_tmp_bo_direct);
>                  adev->ib_pool_ready = false;
>          }
>  }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> index 4981e443a884..6a63826c6760 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> @@ -88,6 +88,12 @@ int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned num_ibs,
>
>  int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev, unsigned size,
>                               struct amdgpu_job **job)
> +{
> +       return amdgpu_job_alloc_with_ib_direct(adev, size, job, 0);
> +}
> +
> +int amdgpu_job_alloc_with_ib_direct(struct amdgpu_device *adev, unsigned size,
> +                            struct amdgpu_job **job, int direct)
>  {
>          int r;
>
> @@ -95,7 +101,7 @@ int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev, unsigned size,
>          if (r)
>                  return r;
>
> -       r = amdgpu_ib_get(adev, NULL, size, &(*job)->ibs[0]);
> +       r = amdgpu_ib_get(adev, direct ? NULL : 0x1, size, &(*job)->ibs[0]);
>          if (r)
>                  kfree(*job);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> index 2e2110dddb76..be9dd72b9912 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> @@ -67,7 +67,8 @@ int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned num_ibs,
>                       struct amdgpu_job **job, struct amdgpu_vm *vm);
>  int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev, unsigned size,
>                               struct amdgpu_job **job);
> -
> +int amdgpu_job_alloc_with_ib_direct(struct amdgpu_device *adev, unsigned size,
> +                            struct amdgpu_job **job, int direct);
>  void amdgpu_job_free_resources(struct amdgpu_job *job);
>  void amdgpu_job_free(struct amdgpu_job *job);
>  int amdgpu_job_submit(struct amdgpu_job *job, struct drm_sched_entity *entity,
> --
> 2.17.1



[-- Attachment #1.2: Type: text/html, Size: 14207 bytes --]

[-- Attachment #2: 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	[flat|nested] 8+ messages in thread

* Re: [RFC PATCH 1/2] drm/amdgpu: add direct ib pool
  2020-03-26  6:36 [RFC PATCH 1/2] drm/amdgpu: add direct ib pool Koenig, Christian
@ 2020-03-26  6:45 ` Pan, Xinhui
  0 siblings, 0 replies; 8+ messages in thread
From: Pan, Xinhui @ 2020-03-26  6:45 UTC (permalink / raw)
  To: Koenig, Christian
  Cc: Deucher, Alexander, Kuehling, Felix, Pan, Xinhui, amd-gfx



> 2020年3月26日 14:36,Koenig, Christian <Christian.Koenig@amd.com> 写道:
> 
> 
> 
> Am 26.03.2020 07:15 schrieb "Pan, Xinhui" <Xinhui.Pan@amd.com>:
> 
> 
> > 2020年3月26日 13:38,Koenig, Christian <Christian.Koenig@amd.com> 写道:
> > 
> > Yeah that's on my TODO list for quite a while as well.
> > 
> > But we even need three IB pools. One very small for the IB tests, one for direct VM updates and one for the rest.
> > 
> > So please make the pool a parameter to ib_get() and not the hack you have below.
> 
> yep, I will make IB pool  a parameter.
> 
> IB tests for gfx need many IBs, PAGE_SIZE for ib pool is still not enough.
> but the default size for ib pool is 2MB now, just one hugepage, today we have memory in TB.
> so no need make a different size for IB tests pool.
> 
> 2MB is probably a bit much and we don't have huge page optimisation for kernel allocations at the moment anyway. Keep in mind that we have only limited space in the GART.
gart table is just 512MB.
do you mean every entry in gart table just points to one 4KB page? and need 5 gart table entries for one 2M hugepage? 

> 
> Maybe make this 4*PAGE_SIZE for the new IB pool for now and test if that works or not.
> 
> Christian.
> 
> 
> 
> 
> > 
> > Thanks,
> > Christian.
> > 
> > Am 26.03.2020 03:02 schrieb "Pan, Xinhui" <Xinhui.Pan@amd.com>:
> > Another ib poll for direct submit.
> > Any jobs schedule IBs without dependence on gpu scheduler should use
> > this pool firstly.
> > 
> > Signed-off-by: xinhui pan <xinhui.pan@amd.com>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu.h     |  1 +
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c  |  2 +-
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c  | 12 ++++++++++--
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_job.c |  8 +++++++-
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_job.h |  3 ++-
> >  5 files changed, 21 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > index 7dd74253e7b6..c01423ffb8ed 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > @@ -849,6 +849,7 @@ struct amdgpu_device {
> >          struct amdgpu_ring              *rings[AMDGPU_MAX_RINGS];
> >          bool                            ib_pool_ready;
> >          struct amdgpu_sa_manager        ring_tmp_bo;
> > +       struct amdgpu_sa_manager        ring_tmp_bo_direct;
> >  
> >          /* interrupts */
> >          struct amdgpu_irq               irq;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> > index 8304d0c87899..28be4efb3d5b 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> > @@ -920,7 +920,7 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device *adev,
> >                  parser->entity = entity;
> >  
> >                  ring = to_amdgpu_ring(entity->rq->sched);
> > -               r =  amdgpu_ib_get(adev, vm, ring->funcs->parse_cs ?
> > +               r =  amdgpu_ib_get(adev, (unsigned long )vm|0x1, ring->funcs->parse_cs ?
> >                                     chunk_ib->ib_bytes : 0, ib);
> >                  if (r) {
> >                          DRM_ERROR("Failed to get ib !\n");
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> > index bece01f1cf09..f2e08c372d57 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> > @@ -66,7 +66,7 @@ int amdgpu_ib_get(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> >          int r;
> >  
> >          if (size) {
> > -               r = amdgpu_sa_bo_new(&adev->ring_tmp_bo,
> > +               r = amdgpu_sa_bo_new(vm ? &adev->ring_tmp_bo : &adev->ring_tmp_bo_direct,
> >                                        &ib->sa_bo, size, 256);
> >                  if (r) {
> >                          dev_err(adev->dev, "failed to get a new IB (%d)\n", r);
> > @@ -75,7 +75,7 @@ int amdgpu_ib_get(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> >  
> >                  ib->ptr = amdgpu_sa_bo_cpu_addr(ib->sa_bo);
> >  
> > -               if (!vm)
> > +               if (!((unsigned long)vm & ~0x1))
> >                          ib->gpu_addr = amdgpu_sa_bo_gpu_addr(ib->sa_bo);
> >          }
> >  
> > @@ -310,6 +310,13 @@ int amdgpu_ib_pool_init(struct amdgpu_device *adev)
> >                  return r;
> >          }
> >  
> > +       r = amdgpu_sa_bo_manager_init(adev, &adev->ring_tmp_bo_direct,
> > +                                     AMDGPU_IB_POOL_SIZE*64*1024,
> > +                                     AMDGPU_GPU_PAGE_SIZE,
> > +                                     AMDGPU_GEM_DOMAIN_GTT);
> > +       if (r) {
> > +               return r;
> > +       }
> >          adev->ib_pool_ready = true;
> >  
> >          return 0;
> > @@ -327,6 +334,7 @@ void amdgpu_ib_pool_fini(struct amdgpu_device *adev)
> >  {
> >          if (adev->ib_pool_ready) {
> >                  amdgpu_sa_bo_manager_fini(adev, &adev->ring_tmp_bo);
> > +               amdgpu_sa_bo_manager_fini(adev, &adev->ring_tmp_bo_direct);
> >                  adev->ib_pool_ready = false;
> >          }
> >  }
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> > index 4981e443a884..6a63826c6760 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> > @@ -88,6 +88,12 @@ int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned num_ibs,
> >  
> >  int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev, unsigned size,
> >                               struct amdgpu_job **job)
> > +{
> > +       return amdgpu_job_alloc_with_ib_direct(adev, size, job, 0);
> > +}
> > +
> > +int amdgpu_job_alloc_with_ib_direct(struct amdgpu_device *adev, unsigned size,
> > +                            struct amdgpu_job **job, int direct)
> >  {
> >          int r;
> >  
> > @@ -95,7 +101,7 @@ int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev, unsigned size,
> >          if (r)
> >                  return r;
> >  
> > -       r = amdgpu_ib_get(adev, NULL, size, &(*job)->ibs[0]);
> > +       r = amdgpu_ib_get(adev, direct ? NULL : 0x1, size, &(*job)->ibs[0]);
> >          if (r)
> >                  kfree(*job);
> >  
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> > index 2e2110dddb76..be9dd72b9912 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> > @@ -67,7 +67,8 @@ int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned num_ibs,
> >                       struct amdgpu_job **job, struct amdgpu_vm *vm);
> >  int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev, unsigned size,
> >                               struct amdgpu_job **job);
> > -
> > +int amdgpu_job_alloc_with_ib_direct(struct amdgpu_device *adev, unsigned size,
> > +                            struct amdgpu_job **job, int direct);
> >  void amdgpu_job_free_resources(struct amdgpu_job *job);
> >  void amdgpu_job_free(struct amdgpu_job *job);
> >  int amdgpu_job_submit(struct amdgpu_job *job, struct drm_sched_entity *entity,
> > -- 
> > 2.17.1
> 
> 

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC PATCH 1/2] drm/amdgpu: add direct ib pool
  2020-03-26  7:11 ` Pan, Xinhui
@ 2020-03-26 14:36   ` Felix Kuehling
  0 siblings, 0 replies; 8+ messages in thread
From: Felix Kuehling @ 2020-03-26 14:36 UTC (permalink / raw)
  To: Pan, Xinhui, Koenig, Christian; +Cc: Deucher, Alexander, amd-gfx


On 2020-03-26 3:11, Pan, Xinhui wrote:
>
>> 2020年3月26日 14:51,Koenig, Christian <Christian.Koenig@amd.com> 写道:
>>
>>
>>
>> Am 26.03.2020 07:45 schrieb "Pan, Xinhui" <Xinhui.Pan@amd.com>:
>>
>>
>>> 2020年3月26日 14:36,Koenig, Christian <Christian.Koenig@amd.com> 写道:
>>>
>>>
>>>
>>> Am 26.03.2020 07:15 schrieb "Pan, Xinhui" <Xinhui.Pan@amd.com>:
>>>
>>>
>>>> 2020年3月26日 13:38,Koenig, Christian <Christian.Koenig@amd.com> 写道:
>>>>
>>>> Yeah that's on my TODO list for quite a while as well.
>>>>
>>>> But we even need three IB pools. One very small for the IB tests, one for direct VM updates and one for the rest.
>>>>
>>>> So please make the pool a parameter to ib_get() and not the hack you have below.
>>> yep, I will make IB pool  a parameter.
>>>
>>> IB tests for gfx need many IBs, PAGE_SIZE for ib pool is still not enough.
>>> but the default size for ib pool is 2MB now, just one hugepage, today we have memory in TB.
>>> so no need make a different size for IB tests pool.
>>>
>>> 2MB is probably a bit much and we don't have huge page optimisation for kernel allocations at the moment anyway. Keep in mind that we have only limited space in the GART.
>> gart table is just 512MB.
>> do you mean every entry in gart table just points to one 4KB page? and need 5 gart table entries for one 2M hugepage?
>>
>> Yes, we need 512 * 4KB entries for a 2MB page in GART. The table for the system VM is flat because of hardware restrictions.
>>
> oh yes, 512 entries.
>
>> IIRC we tried 256MB for the GART initially and in general we try to keep that as small as possible because it eats up visible VRAM space.
> that is true. but our roadmap tells that we are a linux ML team.
> I can not imagine that customers use small pci bar servers. well, they care the cost. but I prefer they using new products. :)
>
> anyway, I will chosse a smallest workable default vaule for now.

For Linux ML we want a small GART size. We used to waste lots of memory 
for a big GART that was unnecessary and took away valuable memory from 
ML applications. The GART is only used for kernel mode mappings, most of 
which tend to be short-lived. Statically allocating a big GART is not 
necessary.

The IB pools are statically allocated and long-lived. So if we need to 
increase the GART size to meet that demand, so be it. But we should not 
waste memory for IBs and associated GART space without a good reason.

Regards,
   Felix


>
>> Christian.
>>
>>
>>> Maybe make this 4*PAGE_SIZE for the new IB pool for now and test if that works or not.
>>>
>>> Christian.
>>>
>>>
>>>
>>>
>>>> Thanks,
>>>> Christian.
>>>>
>>>> Am 26.03.2020 03:02 schrieb "Pan, Xinhui" <Xinhui.Pan@amd.com>:
>>>> Another ib poll for direct submit.
>>>> Any jobs schedule IBs without dependence on gpu scheduler should use
>>>> this pool firstly.
>>>>
>>>> Signed-off-by: xinhui pan <xinhui.pan@amd.com>
>>>> ---
>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu.h     |  1 +
>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c  |  2 +-
>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c  | 12 ++++++++++--
>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_job.c |  8 +++++++-
>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_job.h |  3 ++-
>>>>   5 files changed, 21 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>> index 7dd74253e7b6..c01423ffb8ed 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>> @@ -849,6 +849,7 @@ struct amdgpu_device {
>>>>           struct amdgpu_ring              *rings[AMDGPU_MAX_RINGS];
>>>>           bool                            ib_pool_ready;
>>>>           struct amdgpu_sa_manager        ring_tmp_bo;
>>>> +       struct amdgpu_sa_manager        ring_tmp_bo_direct;
>>>>   
>>>>           /* interrupts */
>>>>           struct amdgpu_irq               irq;
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>>> index 8304d0c87899..28be4efb3d5b 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>>> @@ -920,7 +920,7 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device *adev,
>>>>                   parser->entity = entity;
>>>>   
>>>>                   ring = to_amdgpu_ring(entity->rq->sched);
>>>> -               r =  amdgpu_ib_get(adev, vm, ring->funcs->parse_cs ?
>>>> +               r =  amdgpu_ib_get(adev, (unsigned long )vm|0x1, ring->funcs->parse_cs ?
>>>>                                      chunk_ib->ib_bytes : 0, ib);
>>>>                   if (r) {
>>>>                           DRM_ERROR("Failed to get ib !\n");
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>>>> index bece01f1cf09..f2e08c372d57 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>>>> @@ -66,7 +66,7 @@ int amdgpu_ib_get(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>>>>           int r;
>>>>   
>>>>           if (size) {
>>>> -               r = amdgpu_sa_bo_new(&adev->ring_tmp_bo,
>>>> +               r = amdgpu_sa_bo_new(vm ? &adev->ring_tmp_bo : &adev->ring_tmp_bo_direct,
>>>>                                         &ib->sa_bo, size, 256);
>>>>                   if (r) {
>>>>                           dev_err(adev->dev, "failed to get a new IB (%d)\n", r);
>>>> @@ -75,7 +75,7 @@ int amdgpu_ib_get(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>>>>   
>>>>                   ib->ptr = amdgpu_sa_bo_cpu_addr(ib->sa_bo);
>>>>   
>>>> -               if (!vm)
>>>> +               if (!((unsigned long)vm & ~0x1))
>>>>                           ib->gpu_addr = amdgpu_sa_bo_gpu_addr(ib->sa_bo);
>>>>           }
>>>>   
>>>> @@ -310,6 +310,13 @@ int amdgpu_ib_pool_init(struct amdgpu_device *adev)
>>>>                   return r;
>>>>           }
>>>>   
>>>> +       r = amdgpu_sa_bo_manager_init(adev, &adev->ring_tmp_bo_direct,
>>>> +                                     AMDGPU_IB_POOL_SIZE*64*1024,
>>>> +                                     AMDGPU_GPU_PAGE_SIZE,
>>>> +                                     AMDGPU_GEM_DOMAIN_GTT);
>>>> +       if (r) {
>>>> +               return r;
>>>> +       }
>>>>           adev->ib_pool_ready = true;
>>>>   
>>>>           return 0;
>>>> @@ -327,6 +334,7 @@ void amdgpu_ib_pool_fini(struct amdgpu_device *adev)
>>>>   {
>>>>           if (adev->ib_pool_ready) {
>>>>                   amdgpu_sa_bo_manager_fini(adev, &adev->ring_tmp_bo);
>>>> +               amdgpu_sa_bo_manager_fini(adev, &adev->ring_tmp_bo_direct);
>>>>                   adev->ib_pool_ready = false;
>>>>           }
>>>>   }
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
>>>> index 4981e443a884..6a63826c6760 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
>>>> @@ -88,6 +88,12 @@ int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned num_ibs,
>>>>   
>>>>   int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev, unsigned size,
>>>>                                struct amdgpu_job **job)
>>>> +{
>>>> +       return amdgpu_job_alloc_with_ib_direct(adev, size, job, 0);
>>>> +}
>>>> +
>>>> +int amdgpu_job_alloc_with_ib_direct(struct amdgpu_device *adev, unsigned size,
>>>> +                            struct amdgpu_job **job, int direct)
>>>>   {
>>>>           int r;
>>>>   
>>>> @@ -95,7 +101,7 @@ int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev, unsigned size,
>>>>           if (r)
>>>>                   return r;
>>>>   
>>>> -       r = amdgpu_ib_get(adev, NULL, size, &(*job)->ibs[0]);
>>>> +       r = amdgpu_ib_get(adev, direct ? NULL : 0x1, size, &(*job)->ibs[0]);
>>>>           if (r)
>>>>                   kfree(*job);
>>>>   
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
>>>> index 2e2110dddb76..be9dd72b9912 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
>>>> @@ -67,7 +67,8 @@ int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned num_ibs,
>>>>                        struct amdgpu_job **job, struct amdgpu_vm *vm);
>>>>   int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev, unsigned size,
>>>>                                struct amdgpu_job **job);
>>>> -
>>>> +int amdgpu_job_alloc_with_ib_direct(struct amdgpu_device *adev, unsigned size,
>>>> +                            struct amdgpu_job **job, int direct);
>>>>   void amdgpu_job_free_resources(struct amdgpu_job *job);
>>>>   void amdgpu_job_free(struct amdgpu_job *job);
>>>>   int amdgpu_job_submit(struct amdgpu_job *job, struct drm_sched_entity *entity,
>>>> -- 
>>>> 2.17.1
>>>
>>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC PATCH 1/2] drm/amdgpu: add direct ib pool
  2020-03-26  6:51 Koenig, Christian
@ 2020-03-26  7:11 ` Pan, Xinhui
  2020-03-26 14:36   ` Felix Kuehling
  0 siblings, 1 reply; 8+ messages in thread
From: Pan, Xinhui @ 2020-03-26  7:11 UTC (permalink / raw)
  To: Koenig, Christian
  Cc: Deucher, Alexander, Kuehling, Felix, Pan, Xinhui, amd-gfx



> 2020年3月26日 14:51,Koenig, Christian <Christian.Koenig@amd.com> 写道:
> 
> 
> 
> Am 26.03.2020 07:45 schrieb "Pan, Xinhui" <Xinhui.Pan@amd.com>:
> 
> 
> > 2020年3月26日 14:36,Koenig, Christian <Christian.Koenig@amd.com> 写道:
> > 
> > 
> > 
> > Am 26.03.2020 07:15 schrieb "Pan, Xinhui" <Xinhui.Pan@amd.com>:
> > 
> > 
> > > 2020年3月26日 13:38,Koenig, Christian <Christian.Koenig@amd.com> 写道:
> > > 
> > > Yeah that's on my TODO list for quite a while as well.
> > > 
> > > But we even need three IB pools. One very small for the IB tests, one for direct VM updates and one for the rest.
> > > 
> > > So please make the pool a parameter to ib_get() and not the hack you have below.
> > 
> > yep, I will make IB pool  a parameter.
> > 
> > IB tests for gfx need many IBs, PAGE_SIZE for ib pool is still not enough.
> > but the default size for ib pool is 2MB now, just one hugepage, today we have memory in TB.
> > so no need make a different size for IB tests pool.
> > 
> > 2MB is probably a bit much and we don't have huge page optimisation for kernel allocations at the moment anyway. Keep in mind that we have only limited space in the GART.
> gart table is just 512MB.
> do you mean every entry in gart table just points to one 4KB page? and need 5 gart table entries for one 2M hugepage? 
> 
> Yes, we need 512 * 4KB entries for a 2MB page in GART. The table for the system VM is flat because of hardware restrictions.
> 
oh yes, 512 entries.

> IIRC we tried 256MB for the GART initially and in general we try to keep that as small as possible because it eats up visible VRAM space.
that is true. but our roadmap tells that we are a linux ML team. 
I can not imagine that customers use small pci bar servers. well, they care the cost. but I prefer they using new products. :)

anyway, I will chosse a smallest workable default vaule for now.

> 
> Christian.
> 
> 
> > 
> > Maybe make this 4*PAGE_SIZE for the new IB pool for now and test if that works or not.
> > 
> > Christian.
> > 
> > 
> > 
> > 
> > > 
> > > Thanks,
> > > Christian.
> > > 
> > > Am 26.03.2020 03:02 schrieb "Pan, Xinhui" <Xinhui.Pan@amd.com>:
> > > Another ib poll for direct submit.
> > > Any jobs schedule IBs without dependence on gpu scheduler should use
> > > this pool firstly.
> > > 
> > > Signed-off-by: xinhui pan <xinhui.pan@amd.com>
> > > ---
> > >  drivers/gpu/drm/amd/amdgpu/amdgpu.h     |  1 +
> > >  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c  |  2 +-
> > >  drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c  | 12 ++++++++++--
> > >  drivers/gpu/drm/amd/amdgpu/amdgpu_job.c |  8 +++++++-
> > >  drivers/gpu/drm/amd/amdgpu/amdgpu_job.h |  3 ++-
> > >  5 files changed, 21 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > > index 7dd74253e7b6..c01423ffb8ed 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > > @@ -849,6 +849,7 @@ struct amdgpu_device {
> > >          struct amdgpu_ring              *rings[AMDGPU_MAX_RINGS];
> > >          bool                            ib_pool_ready;
> > >          struct amdgpu_sa_manager        ring_tmp_bo;
> > > +       struct amdgpu_sa_manager        ring_tmp_bo_direct;
> > >  
> > >          /* interrupts */
> > >          struct amdgpu_irq               irq;
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> > > index 8304d0c87899..28be4efb3d5b 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> > > @@ -920,7 +920,7 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device *adev,
> > >                  parser->entity = entity;
> > >  
> > >                  ring = to_amdgpu_ring(entity->rq->sched);
> > > -               r =  amdgpu_ib_get(adev, vm, ring->funcs->parse_cs ?
> > > +               r =  amdgpu_ib_get(adev, (unsigned long )vm|0x1, ring->funcs->parse_cs ?
> > >                                     chunk_ib->ib_bytes : 0, ib);
> > >                  if (r) {
> > >                          DRM_ERROR("Failed to get ib !\n");
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> > > index bece01f1cf09..f2e08c372d57 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> > > @@ -66,7 +66,7 @@ int amdgpu_ib_get(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> > >          int r;
> > >  
> > >          if (size) {
> > > -               r = amdgpu_sa_bo_new(&adev->ring_tmp_bo,
> > > +               r = amdgpu_sa_bo_new(vm ? &adev->ring_tmp_bo : &adev->ring_tmp_bo_direct,
> > >                                        &ib->sa_bo, size, 256);
> > >                  if (r) {
> > >                          dev_err(adev->dev, "failed to get a new IB (%d)\n", r);
> > > @@ -75,7 +75,7 @@ int amdgpu_ib_get(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> > >  
> > >                  ib->ptr = amdgpu_sa_bo_cpu_addr(ib->sa_bo);
> > >  
> > > -               if (!vm)
> > > +               if (!((unsigned long)vm & ~0x1))
> > >                          ib->gpu_addr = amdgpu_sa_bo_gpu_addr(ib->sa_bo);
> > >          }
> > >  
> > > @@ -310,6 +310,13 @@ int amdgpu_ib_pool_init(struct amdgpu_device *adev)
> > >                  return r;
> > >          }
> > >  
> > > +       r = amdgpu_sa_bo_manager_init(adev, &adev->ring_tmp_bo_direct,
> > > +                                     AMDGPU_IB_POOL_SIZE*64*1024,
> > > +                                     AMDGPU_GPU_PAGE_SIZE,
> > > +                                     AMDGPU_GEM_DOMAIN_GTT);
> > > +       if (r) {
> > > +               return r;
> > > +       }
> > >          adev->ib_pool_ready = true;
> > >  
> > >          return 0;
> > > @@ -327,6 +334,7 @@ void amdgpu_ib_pool_fini(struct amdgpu_device *adev)
> > >  {
> > >          if (adev->ib_pool_ready) {
> > >                  amdgpu_sa_bo_manager_fini(adev, &adev->ring_tmp_bo);
> > > +               amdgpu_sa_bo_manager_fini(adev, &adev->ring_tmp_bo_direct);
> > >                  adev->ib_pool_ready = false;
> > >          }
> > >  }
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> > > index 4981e443a884..6a63826c6760 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> > > @@ -88,6 +88,12 @@ int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned num_ibs,
> > >  
> > >  int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev, unsigned size,
> > >                               struct amdgpu_job **job)
> > > +{
> > > +       return amdgpu_job_alloc_with_ib_direct(adev, size, job, 0);
> > > +}
> > > +
> > > +int amdgpu_job_alloc_with_ib_direct(struct amdgpu_device *adev, unsigned size,
> > > +                            struct amdgpu_job **job, int direct)
> > >  {
> > >          int r;
> > >  
> > > @@ -95,7 +101,7 @@ int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev, unsigned size,
> > >          if (r)
> > >                  return r;
> > >  
> > > -       r = amdgpu_ib_get(adev, NULL, size, &(*job)->ibs[0]);
> > > +       r = amdgpu_ib_get(adev, direct ? NULL : 0x1, size, &(*job)->ibs[0]);
> > >          if (r)
> > >                  kfree(*job);
> > >  
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> > > index 2e2110dddb76..be9dd72b9912 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> > > @@ -67,7 +67,8 @@ int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned num_ibs,
> > >                       struct amdgpu_job **job, struct amdgpu_vm *vm);
> > >  int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev, unsigned size,
> > >                               struct amdgpu_job **job);
> > > -
> > > +int amdgpu_job_alloc_with_ib_direct(struct amdgpu_device *adev, unsigned size,
> > > +                            struct amdgpu_job **job, int direct);
> > >  void amdgpu_job_free_resources(struct amdgpu_job *job);
> > >  void amdgpu_job_free(struct amdgpu_job *job);
> > >  int amdgpu_job_submit(struct amdgpu_job *job, struct drm_sched_entity *entity,
> > > -- 
> > > 2.17.1
> > 
> > 
> 
> 

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC PATCH 1/2] drm/amdgpu: add direct ib pool
@ 2020-03-26  6:51 Koenig, Christian
  2020-03-26  7:11 ` Pan, Xinhui
  0 siblings, 1 reply; 8+ messages in thread
From: Koenig, Christian @ 2020-03-26  6:51 UTC (permalink / raw)
  To: Pan, Xinhui; +Cc: Deucher, Alexander, Kuehling, Felix, amd-gfx


[-- Attachment #1.1: Type: text/plain, Size: 7730 bytes --]



Am 26.03.2020 07:45 schrieb "Pan, Xinhui" <Xinhui.Pan@amd.com>:


> 2020年3月26日 14:36,Koenig, Christian <Christian.Koenig@amd.com> 写道:
>
>
>
> Am 26.03.2020 07:15 schrieb "Pan, Xinhui" <Xinhui.Pan@amd.com>:
>
>
> > 2020年3月26日 13:38,Koenig, Christian <Christian.Koenig@amd.com> 写道:
> >
> > Yeah that's on my TODO list for quite a while as well.
> >
> > But we even need three IB pools. One very small for the IB tests, one for direct VM updates and one for the rest.
> >
> > So please make the pool a parameter to ib_get() and not the hack you have below.
>
> yep, I will make IB pool  a parameter.
>
> IB tests for gfx need many IBs, PAGE_SIZE for ib pool is still not enough.
> but the default size for ib pool is 2MB now, just one hugepage, today we have memory in TB.
> so no need make a different size for IB tests pool.
>
> 2MB is probably a bit much and we don't have huge page optimisation for kernel allocations at the moment anyway. Keep in mind that we have only limited space in the GART.
gart table is just 512MB.
do you mean every entry in gart table just points to one 4KB page? and need 5 gart table entries for one 2M hugepage?

Yes, we need 512 * 4KB entries for a 2MB page in GART. The table for the system VM is flat because of hardware restrictions.

IIRC we tried 256MB for the GART initially and in general we try to keep that as small as possible because it eats up visible VRAM space.

Christian.


>
> Maybe make this 4*PAGE_SIZE for the new IB pool for now and test if that works or not.
>
> Christian.
>
>
>
>
> >
> > Thanks,
> > Christian.
> >
> > Am 26.03.2020 03:02 schrieb "Pan, Xinhui" <Xinhui.Pan@amd.com>:
> > Another ib poll for direct submit.
> > Any jobs schedule IBs without dependence on gpu scheduler should use
> > this pool firstly.
> >
> > Signed-off-by: xinhui pan <xinhui.pan@amd.com>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu.h     |  1 +
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c  |  2 +-
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c  | 12 ++++++++++--
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_job.c |  8 +++++++-
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_job.h |  3 ++-
> >  5 files changed, 21 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > index 7dd74253e7b6..c01423ffb8ed 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > @@ -849,6 +849,7 @@ struct amdgpu_device {
> >          struct amdgpu_ring              *rings[AMDGPU_MAX_RINGS];
> >          bool                            ib_pool_ready;
> >          struct amdgpu_sa_manager        ring_tmp_bo;
> > +       struct amdgpu_sa_manager        ring_tmp_bo_direct;
> >
> >          /* interrupts */
> >          struct amdgpu_irq               irq;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> > index 8304d0c87899..28be4efb3d5b 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> > @@ -920,7 +920,7 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device *adev,
> >                  parser->entity = entity;
> >
> >                  ring = to_amdgpu_ring(entity->rq->sched);
> > -               r =  amdgpu_ib_get(adev, vm, ring->funcs->parse_cs ?
> > +               r =  amdgpu_ib_get(adev, (unsigned long )vm|0x1, ring->funcs->parse_cs ?
> >                                     chunk_ib->ib_bytes : 0, ib);
> >                  if (r) {
> >                          DRM_ERROR("Failed to get ib !\n");
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> > index bece01f1cf09..f2e08c372d57 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> > @@ -66,7 +66,7 @@ int amdgpu_ib_get(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> >          int r;
> >
> >          if (size) {
> > -               r = amdgpu_sa_bo_new(&adev->ring_tmp_bo,
> > +               r = amdgpu_sa_bo_new(vm ? &adev->ring_tmp_bo : &adev->ring_tmp_bo_direct,
> >                                        &ib->sa_bo, size, 256);
> >                  if (r) {
> >                          dev_err(adev->dev, "failed to get a new IB (%d)\n", r);
> > @@ -75,7 +75,7 @@ int amdgpu_ib_get(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> >
> >                  ib->ptr = amdgpu_sa_bo_cpu_addr(ib->sa_bo);
> >
> > -               if (!vm)
> > +               if (!((unsigned long)vm & ~0x1))
> >                          ib->gpu_addr = amdgpu_sa_bo_gpu_addr(ib->sa_bo);
> >          }
> >
> > @@ -310,6 +310,13 @@ int amdgpu_ib_pool_init(struct amdgpu_device *adev)
> >                  return r;
> >          }
> >
> > +       r = amdgpu_sa_bo_manager_init(adev, &adev->ring_tmp_bo_direct,
> > +                                     AMDGPU_IB_POOL_SIZE*64*1024,
> > +                                     AMDGPU_GPU_PAGE_SIZE,
> > +                                     AMDGPU_GEM_DOMAIN_GTT);
> > +       if (r) {
> > +               return r;
> > +       }
> >          adev->ib_pool_ready = true;
> >
> >          return 0;
> > @@ -327,6 +334,7 @@ void amdgpu_ib_pool_fini(struct amdgpu_device *adev)
> >  {
> >          if (adev->ib_pool_ready) {
> >                  amdgpu_sa_bo_manager_fini(adev, &adev->ring_tmp_bo);
> > +               amdgpu_sa_bo_manager_fini(adev, &adev->ring_tmp_bo_direct);
> >                  adev->ib_pool_ready = false;
> >          }
> >  }
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> > index 4981e443a884..6a63826c6760 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> > @@ -88,6 +88,12 @@ int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned num_ibs,
> >
> >  int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev, unsigned size,
> >                               struct amdgpu_job **job)
> > +{
> > +       return amdgpu_job_alloc_with_ib_direct(adev, size, job, 0);
> > +}
> > +
> > +int amdgpu_job_alloc_with_ib_direct(struct amdgpu_device *adev, unsigned size,
> > +                            struct amdgpu_job **job, int direct)
> >  {
> >          int r;
> >
> > @@ -95,7 +101,7 @@ int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev, unsigned size,
> >          if (r)
> >                  return r;
> >
> > -       r = amdgpu_ib_get(adev, NULL, size, &(*job)->ibs[0]);
> > +       r = amdgpu_ib_get(adev, direct ? NULL : 0x1, size, &(*job)->ibs[0]);
> >          if (r)
> >                  kfree(*job);
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> > index 2e2110dddb76..be9dd72b9912 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> > @@ -67,7 +67,8 @@ int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned num_ibs,
> >                       struct amdgpu_job **job, struct amdgpu_vm *vm);
> >  int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev, unsigned size,
> >                               struct amdgpu_job **job);
> > -
> > +int amdgpu_job_alloc_with_ib_direct(struct amdgpu_device *adev, unsigned size,
> > +                            struct amdgpu_job **job, int direct);
> >  void amdgpu_job_free_resources(struct amdgpu_job *job);
> >  void amdgpu_job_free(struct amdgpu_job *job);
> >  int amdgpu_job_submit(struct amdgpu_job *job, struct drm_sched_entity *entity,
> > --
> > 2.17.1
>
>



[-- Attachment #1.2: Type: text/html, Size: 15657 bytes --]

[-- Attachment #2: 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	[flat|nested] 8+ messages in thread

* Re: [RFC PATCH 1/2] drm/amdgpu: add direct ib pool
  2020-03-26  5:38   ` Koenig, Christian
@ 2020-03-26  6:15     ` Pan, Xinhui
  0 siblings, 0 replies; 8+ messages in thread
From: Pan, Xinhui @ 2020-03-26  6:15 UTC (permalink / raw)
  To: Koenig, Christian
  Cc: Deucher, Alexander, Kuehling, Felix, Pan, Xinhui, amd-gfx



> 2020年3月26日 13:38,Koenig, Christian <Christian.Koenig@amd.com> 写道:
> 
> Yeah that's on my TODO list for quite a while as well.
> 
> But we even need three IB pools. One very small for the IB tests, one for direct VM updates and one for the rest.
> 
> So please make the pool a parameter to ib_get() and not the hack you have below.

yep, I will make IB pool  a parameter.

IB tests for gfx need many IBs, PAGE_SIZE for ib pool is still not enough.
but the default size for ib pool is 2MB now, just one hugepage, today we have memory in TB.
so no need make a different size for IB tests pool.

> 
> Thanks,
> Christian.
> 
> Am 26.03.2020 03:02 schrieb "Pan, Xinhui" <Xinhui.Pan@amd.com>:
> Another ib poll for direct submit.
> Any jobs schedule IBs without dependence on gpu scheduler should use
> this pool firstly.
> 
> Signed-off-by: xinhui pan <xinhui.pan@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h     |  1 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c  |  2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c  | 12 ++++++++++--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_job.c |  8 +++++++-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_job.h |  3 ++-
>  5 files changed, 21 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 7dd74253e7b6..c01423ffb8ed 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -849,6 +849,7 @@ struct amdgpu_device {
>          struct amdgpu_ring              *rings[AMDGPU_MAX_RINGS];
>          bool                            ib_pool_ready;
>          struct amdgpu_sa_manager        ring_tmp_bo;
> +       struct amdgpu_sa_manager        ring_tmp_bo_direct;
>  
>          /* interrupts */
>          struct amdgpu_irq               irq;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 8304d0c87899..28be4efb3d5b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -920,7 +920,7 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device *adev,
>                  parser->entity = entity;
>  
>                  ring = to_amdgpu_ring(entity->rq->sched);
> -               r =  amdgpu_ib_get(adev, vm, ring->funcs->parse_cs ?
> +               r =  amdgpu_ib_get(adev, (unsigned long )vm|0x1, ring->funcs->parse_cs ?
>                                     chunk_ib->ib_bytes : 0, ib);
>                  if (r) {
>                          DRM_ERROR("Failed to get ib !\n");
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> index bece01f1cf09..f2e08c372d57 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> @@ -66,7 +66,7 @@ int amdgpu_ib_get(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>          int r;
>  
>          if (size) {
> -               r = amdgpu_sa_bo_new(&adev->ring_tmp_bo,
> +               r = amdgpu_sa_bo_new(vm ? &adev->ring_tmp_bo : &adev->ring_tmp_bo_direct,
>                                        &ib->sa_bo, size, 256);
>                  if (r) {
>                          dev_err(adev->dev, "failed to get a new IB (%d)\n", r);
> @@ -75,7 +75,7 @@ int amdgpu_ib_get(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>  
>                  ib->ptr = amdgpu_sa_bo_cpu_addr(ib->sa_bo);
>  
> -               if (!vm)
> +               if (!((unsigned long)vm & ~0x1))
>                          ib->gpu_addr = amdgpu_sa_bo_gpu_addr(ib->sa_bo);
>          }
>  
> @@ -310,6 +310,13 @@ int amdgpu_ib_pool_init(struct amdgpu_device *adev)
>                  return r;
>          }
>  
> +       r = amdgpu_sa_bo_manager_init(adev, &adev->ring_tmp_bo_direct,
> +                                     AMDGPU_IB_POOL_SIZE*64*1024,
> +                                     AMDGPU_GPU_PAGE_SIZE,
> +                                     AMDGPU_GEM_DOMAIN_GTT);
> +       if (r) {
> +               return r;
> +       }
>          adev->ib_pool_ready = true;
>  
>          return 0;
> @@ -327,6 +334,7 @@ void amdgpu_ib_pool_fini(struct amdgpu_device *adev)
>  {
>          if (adev->ib_pool_ready) {
>                  amdgpu_sa_bo_manager_fini(adev, &adev->ring_tmp_bo);
> +               amdgpu_sa_bo_manager_fini(adev, &adev->ring_tmp_bo_direct);
>                  adev->ib_pool_ready = false;
>          }
>  }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> index 4981e443a884..6a63826c6760 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> @@ -88,6 +88,12 @@ int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned num_ibs,
>  
>  int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev, unsigned size,
>                               struct amdgpu_job **job)
> +{
> +       return amdgpu_job_alloc_with_ib_direct(adev, size, job, 0);
> +}
> +
> +int amdgpu_job_alloc_with_ib_direct(struct amdgpu_device *adev, unsigned size,
> +                            struct amdgpu_job **job, int direct)
>  {
>          int r;
>  
> @@ -95,7 +101,7 @@ int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev, unsigned size,
>          if (r)
>                  return r;
>  
> -       r = amdgpu_ib_get(adev, NULL, size, &(*job)->ibs[0]);
> +       r = amdgpu_ib_get(adev, direct ? NULL : 0x1, size, &(*job)->ibs[0]);
>          if (r)
>                  kfree(*job);
>  
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> index 2e2110dddb76..be9dd72b9912 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> @@ -67,7 +67,8 @@ int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned num_ibs,
>                       struct amdgpu_job **job, struct amdgpu_vm *vm);
>  int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev, unsigned size,
>                               struct amdgpu_job **job);
> -
> +int amdgpu_job_alloc_with_ib_direct(struct amdgpu_device *adev, unsigned size,
> +                            struct amdgpu_job **job, int direct);
>  void amdgpu_job_free_resources(struct amdgpu_job *job);
>  void amdgpu_job_free(struct amdgpu_job *job);
>  int amdgpu_job_submit(struct amdgpu_job *job, struct drm_sched_entity *entity,
> -- 
> 2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC PATCH 1/2] drm/amdgpu: add direct ib pool
  2020-03-26  2:01 ` [RFC PATCH 1/2] drm/amdgpu: add direct ib pool xinhui pan
@ 2020-03-26  5:38   ` Koenig, Christian
  2020-03-26  6:15     ` Pan, Xinhui
  0 siblings, 1 reply; 8+ messages in thread
From: Koenig, Christian @ 2020-03-26  5:38 UTC (permalink / raw)
  To: Pan, Xinhui; +Cc: Deucher, Alexander, Kuehling, Felix, amd-gfx


[-- Attachment #1.1: Type: text/plain, Size: 5868 bytes --]

Yeah that's on my TODO list for quite a while as well.

But we even need three IB pools. One very small for the IB tests, one for direct VM updates and one for the rest.

So please make the pool a parameter to ib_get() and not the hack you have below.

Thanks,
Christian.

Am 26.03.2020 03:02 schrieb "Pan, Xinhui" <Xinhui.Pan@amd.com>:
Another ib poll for direct submit.
Any jobs schedule IBs without dependence on gpu scheduler should use
this pool firstly.

Signed-off-by: xinhui pan <xinhui.pan@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h     |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c  |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c  | 12 ++++++++++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.c |  8 +++++++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.h |  3 ++-
 5 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 7dd74253e7b6..c01423ffb8ed 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -849,6 +849,7 @@ struct amdgpu_device {
         struct amdgpu_ring              *rings[AMDGPU_MAX_RINGS];
         bool                            ib_pool_ready;
         struct amdgpu_sa_manager        ring_tmp_bo;
+       struct amdgpu_sa_manager        ring_tmp_bo_direct;

         /* interrupts */
         struct amdgpu_irq               irq;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 8304d0c87899..28be4efb3d5b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -920,7 +920,7 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device *adev,
                 parser->entity = entity;

                 ring = to_amdgpu_ring(entity->rq->sched);
-               r =  amdgpu_ib_get(adev, vm, ring->funcs->parse_cs ?
+               r =  amdgpu_ib_get(adev, (unsigned long )vm|0x1, ring->funcs->parse_cs ?
                                    chunk_ib->ib_bytes : 0, ib);
                 if (r) {
                         DRM_ERROR("Failed to get ib !\n");
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
index bece01f1cf09..f2e08c372d57 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
@@ -66,7 +66,7 @@ int amdgpu_ib_get(struct amdgpu_device *adev, struct amdgpu_vm *vm,
         int r;

         if (size) {
-               r = amdgpu_sa_bo_new(&adev->ring_tmp_bo,
+               r = amdgpu_sa_bo_new(vm ? &adev->ring_tmp_bo : &adev->ring_tmp_bo_direct,
                                       &ib->sa_bo, size, 256);
                 if (r) {
                         dev_err(adev->dev, "failed to get a new IB (%d)\n", r);
@@ -75,7 +75,7 @@ int amdgpu_ib_get(struct amdgpu_device *adev, struct amdgpu_vm *vm,

                 ib->ptr = amdgpu_sa_bo_cpu_addr(ib->sa_bo);

-               if (!vm)
+               if (!((unsigned long)vm & ~0x1))
                         ib->gpu_addr = amdgpu_sa_bo_gpu_addr(ib->sa_bo);
         }

@@ -310,6 +310,13 @@ int amdgpu_ib_pool_init(struct amdgpu_device *adev)
                 return r;
         }

+       r = amdgpu_sa_bo_manager_init(adev, &adev->ring_tmp_bo_direct,
+                                     AMDGPU_IB_POOL_SIZE*64*1024,
+                                     AMDGPU_GPU_PAGE_SIZE,
+                                     AMDGPU_GEM_DOMAIN_GTT);
+       if (r) {
+               return r;
+       }
         adev->ib_pool_ready = true;

         return 0;
@@ -327,6 +334,7 @@ void amdgpu_ib_pool_fini(struct amdgpu_device *adev)
 {
         if (adev->ib_pool_ready) {
                 amdgpu_sa_bo_manager_fini(adev, &adev->ring_tmp_bo);
+               amdgpu_sa_bo_manager_fini(adev, &adev->ring_tmp_bo_direct);
                 adev->ib_pool_ready = false;
         }
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
index 4981e443a884..6a63826c6760 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
@@ -88,6 +88,12 @@ int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned num_ibs,

 int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev, unsigned size,
                              struct amdgpu_job **job)
+{
+       return amdgpu_job_alloc_with_ib_direct(adev, size, job, 0);
+}
+
+int amdgpu_job_alloc_with_ib_direct(struct amdgpu_device *adev, unsigned size,
+                            struct amdgpu_job **job, int direct)
 {
         int r;

@@ -95,7 +101,7 @@ int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev, unsigned size,
         if (r)
                 return r;

-       r = amdgpu_ib_get(adev, NULL, size, &(*job)->ibs[0]);
+       r = amdgpu_ib_get(adev, direct ? NULL : 0x1, size, &(*job)->ibs[0]);
         if (r)
                 kfree(*job);

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
index 2e2110dddb76..be9dd72b9912 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
@@ -67,7 +67,8 @@ int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned num_ibs,
                      struct amdgpu_job **job, struct amdgpu_vm *vm);
 int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev, unsigned size,
                              struct amdgpu_job **job);
-
+int amdgpu_job_alloc_with_ib_direct(struct amdgpu_device *adev, unsigned size,
+                            struct amdgpu_job **job, int direct);
 void amdgpu_job_free_resources(struct amdgpu_job *job);
 void amdgpu_job_free(struct amdgpu_job *job);
 int amdgpu_job_submit(struct amdgpu_job *job, struct drm_sched_entity *entity,
--
2.17.1


[-- Attachment #1.2: Type: text/html, Size: 12202 bytes --]

[-- Attachment #2: 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] 8+ messages in thread

* [RFC PATCH 1/2] drm/amdgpu: add direct ib pool
  2020-03-26  2:01 [RFC PATCH 0/2] add direct IB pool xinhui pan
@ 2020-03-26  2:01 ` xinhui pan
  2020-03-26  5:38   ` Koenig, Christian
  0 siblings, 1 reply; 8+ messages in thread
From: xinhui pan @ 2020-03-26  2:01 UTC (permalink / raw)
  To: amd-gfx; +Cc: alexander.deucher, Felix.Kuehling, xinhui pan, christian.koenig

Another ib poll for direct submit.
Any jobs schedule IBs without dependence on gpu scheduler should use
this pool firstly.

Signed-off-by: xinhui pan <xinhui.pan@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h     |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c  |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c  | 12 ++++++++++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.c |  8 +++++++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.h |  3 ++-
 5 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 7dd74253e7b6..c01423ffb8ed 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -849,6 +849,7 @@ struct amdgpu_device {
 	struct amdgpu_ring		*rings[AMDGPU_MAX_RINGS];
 	bool				ib_pool_ready;
 	struct amdgpu_sa_manager	ring_tmp_bo;
+	struct amdgpu_sa_manager	ring_tmp_bo_direct;
 
 	/* interrupts */
 	struct amdgpu_irq		irq;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 8304d0c87899..28be4efb3d5b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -920,7 +920,7 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device *adev,
 		parser->entity = entity;
 
 		ring = to_amdgpu_ring(entity->rq->sched);
-		r =  amdgpu_ib_get(adev, vm, ring->funcs->parse_cs ?
+		r =  amdgpu_ib_get(adev, (unsigned long )vm|0x1, ring->funcs->parse_cs ?
 				   chunk_ib->ib_bytes : 0, ib);
 		if (r) {
 			DRM_ERROR("Failed to get ib !\n");
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
index bece01f1cf09..f2e08c372d57 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
@@ -66,7 +66,7 @@ int amdgpu_ib_get(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 	int r;
 
 	if (size) {
-		r = amdgpu_sa_bo_new(&adev->ring_tmp_bo,
+		r = amdgpu_sa_bo_new(vm ? &adev->ring_tmp_bo : &adev->ring_tmp_bo_direct,
 				      &ib->sa_bo, size, 256);
 		if (r) {
 			dev_err(adev->dev, "failed to get a new IB (%d)\n", r);
@@ -75,7 +75,7 @@ int amdgpu_ib_get(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 
 		ib->ptr = amdgpu_sa_bo_cpu_addr(ib->sa_bo);
 
-		if (!vm)
+		if (!((unsigned long)vm & ~0x1))
 			ib->gpu_addr = amdgpu_sa_bo_gpu_addr(ib->sa_bo);
 	}
 
@@ -310,6 +310,13 @@ int amdgpu_ib_pool_init(struct amdgpu_device *adev)
 		return r;
 	}
 
+	r = amdgpu_sa_bo_manager_init(adev, &adev->ring_tmp_bo_direct,
+				      AMDGPU_IB_POOL_SIZE*64*1024,
+				      AMDGPU_GPU_PAGE_SIZE,
+				      AMDGPU_GEM_DOMAIN_GTT);
+	if (r) {
+		return r;
+	}
 	adev->ib_pool_ready = true;
 
 	return 0;
@@ -327,6 +334,7 @@ void amdgpu_ib_pool_fini(struct amdgpu_device *adev)
 {
 	if (adev->ib_pool_ready) {
 		amdgpu_sa_bo_manager_fini(adev, &adev->ring_tmp_bo);
+		amdgpu_sa_bo_manager_fini(adev, &adev->ring_tmp_bo_direct);
 		adev->ib_pool_ready = false;
 	}
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
index 4981e443a884..6a63826c6760 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
@@ -88,6 +88,12 @@ int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned num_ibs,
 
 int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev, unsigned size,
 			     struct amdgpu_job **job)
+{
+	return amdgpu_job_alloc_with_ib_direct(adev, size, job, 0);
+}
+
+int amdgpu_job_alloc_with_ib_direct(struct amdgpu_device *adev, unsigned size,
+			     struct amdgpu_job **job, int direct)
 {
 	int r;
 
@@ -95,7 +101,7 @@ int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev, unsigned size,
 	if (r)
 		return r;
 
-	r = amdgpu_ib_get(adev, NULL, size, &(*job)->ibs[0]);
+	r = amdgpu_ib_get(adev, direct ? NULL : 0x1, size, &(*job)->ibs[0]);
 	if (r)
 		kfree(*job);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
index 2e2110dddb76..be9dd72b9912 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
@@ -67,7 +67,8 @@ int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned num_ibs,
 		     struct amdgpu_job **job, struct amdgpu_vm *vm);
 int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev, unsigned size,
 			     struct amdgpu_job **job);
-
+int amdgpu_job_alloc_with_ib_direct(struct amdgpu_device *adev, unsigned size,
+			     struct amdgpu_job **job, int direct);
 void amdgpu_job_free_resources(struct amdgpu_job *job);
 void amdgpu_job_free(struct amdgpu_job *job);
 int amdgpu_job_submit(struct amdgpu_job *job, struct drm_sched_entity *entity,
-- 
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-03-26 14:36 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-26  6:36 [RFC PATCH 1/2] drm/amdgpu: add direct ib pool Koenig, Christian
2020-03-26  6:45 ` Pan, Xinhui
  -- strict thread matches above, loose matches on Subject: below --
2020-03-26  6:51 Koenig, Christian
2020-03-26  7:11 ` Pan, Xinhui
2020-03-26 14:36   ` Felix Kuehling
2020-03-26  2:01 [RFC PATCH 0/2] add direct IB pool xinhui pan
2020-03-26  2:01 ` [RFC PATCH 1/2] drm/amdgpu: add direct ib pool xinhui pan
2020-03-26  5:38   ` Koenig, Christian
2020-03-26  6:15     ` Pan, Xinhui

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).