Re: [PATCH 4/9] drm/amdgpu: Add a bitmask in amdgpu_ctx_mgr

2018-12-06 Thread Kuehling, Felix 
On 2018-12-06 6:32 a.m., Rex Zhu wrote:
> used to manager the reserverd vm space.
>
> Signed-off-by: Rex Zhu 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 8 ++--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h | 4 +++-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 6 +-
>  3 files changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> index 8edf54b..8802ff2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> @@ -529,10 +529,14 @@ int amdgpu_ctx_wait_prev_fence(struct amdgpu_ctx *ctx,
>   return 0;
>  }
>  
> -void amdgpu_ctx_mgr_init(struct amdgpu_ctx_mgr *mgr)
> +int amdgpu_ctx_mgr_init(struct amdgpu_ctx_mgr *mgr)
>  {
>   mutex_init(>lock);
>   idr_init(>ctx_handles);
> + mgr->resv_vm_bitmap = kzalloc(DIV_ROUND_UP(AMDGPU_VM_MAX_NUM_CTX, 
> BITS_PER_BYTE), GFP_KERNEL);

The bitmap is in (unsigned long) units. So you should round to multiples
of (unsigned long), not multiples of byte. Something like this:

mgr->resv_vm_bitmap = kzalloc(DIV_ROUND_UP(AMDGPU_VM_MAX_NUM_CTX,
BITS_PER_BYTE * sizeof(unsigned long)) * sizeof(unsigned long), GFP_KERNEL);

Regards,
  Felix


> + if (unlikely(!mgr->resv_vm_bitmap))
> + return -ENOMEM;
> + return 0;
>  }
>  
>  void amdgpu_ctx_mgr_entity_flush(struct amdgpu_ctx_mgr *mgr)
> @@ -601,7 +605,7 @@ void amdgpu_ctx_mgr_fini(struct amdgpu_ctx_mgr *mgr)
>   if (kref_put(>refcount, amdgpu_ctx_fini) != 1)
>   DRM_ERROR("ctx %p is still alive\n", ctx);
>   }
> -
> + kfree(mgr->resv_vm_bitmap);
>   idr_destroy(>ctx_handles);
>   mutex_destroy(>lock);
>  }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
> index b3b012c..94ac951 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
> @@ -38,6 +38,7 @@ struct amdgpu_ctx_entity {
>  struct amdgpu_ctx {
>   struct kref refcount;
>   struct amdgpu_device*adev;
> +
>   unsignedreset_counter;
>   unsignedreset_counter_query;
>   uint32_tvram_lost_counter;
> @@ -56,6 +57,7 @@ struct amdgpu_ctx_mgr {
>   struct mutexlock;
>   /* protected by lock */
>   struct idr  ctx_handles;
> + unsigned long   *resv_vm_bitmap;
>  };
>  
>  extern const unsigned int amdgpu_ctx_num_entities[AMDGPU_HW_IP_NUM];
> @@ -80,7 +82,7 @@ int amdgpu_ctx_ioctl(struct drm_device *dev, void *data,
>  int amdgpu_ctx_wait_prev_fence(struct amdgpu_ctx *ctx,
>  struct drm_sched_entity *entity);
>  
> -void amdgpu_ctx_mgr_init(struct amdgpu_ctx_mgr *mgr);
> +int amdgpu_ctx_mgr_init(struct amdgpu_ctx_mgr *mgr);
>  void amdgpu_ctx_mgr_entity_fini(struct amdgpu_ctx_mgr *mgr);
>  void amdgpu_ctx_mgr_entity_flush(struct amdgpu_ctx_mgr *mgr);
>  void amdgpu_ctx_mgr_fini(struct amdgpu_ctx_mgr *mgr);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index 52e4e90..338a091 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -988,11 +988,15 @@ int amdgpu_driver_open_kms(struct drm_device *dev, 
> struct drm_file *file_priv)
>   mutex_init(>bo_list_lock);
>   idr_init(>bo_list_handles);
>  
> - amdgpu_ctx_mgr_init(>ctx_mgr);
> + if (amdgpu_ctx_mgr_init(>ctx_mgr))
> + goto error_ctx_mgr;
>  
>   file_priv->driver_priv = fpriv;
>   goto out_suspend;
>  
> +error_ctx_mgr:
> + idr_destroy(>bo_list_handles);
> + mutex_destroy(>bo_list_lock);
>  error_vm:
>   amdgpu_vm_fini(adev, >vm);
>  
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 4/9] drm/amdgpu: Add a bitmask in amdgpu_ctx_mgr

2018-12-06 Thread Koenig, Christian 
Hi Rex,

ok still doesn't make sense to me. VM updates are pipelined with command 
submissions.

So you just need to create the mapping for the specific ID when the CTX 
is created.

And when the CTX is destroyed you unmap the CSA again.

Or is that for each queue inside the context? Please clarify.

Thanks,
Christian.

Am 06.12.18 um 15:48 schrieb Zhu, Rex:
> Hi Christian,
>
> We allocate and map csa per ctx,  need to record the used/free vm space.
> So use bitmap to manager the reserved vm space.
>
> Also add resv_space_id in ctx.
> When amdgpu_ctx_fini, we can clear the bit in the bitmap.
>
>
> Best Regards
> Rex
>> -Original Message-
>> From: Christian König 
>> Sent: Thursday, December 6, 2018 8:34 PM
>> To: Zhu, Rex ; amd-gfx@lists.freedesktop.org
>> Subject: Re: [PATCH 4/9] drm/amdgpu: Add a bitmask in amdgpu_ctx_mgr
>>
>> Am 06.12.18 um 13:14 schrieb Rex Zhu:
>>> used to manager the reserverd vm space.
>> Why do we need that?
>>
>> Christian.
>>
>>> Signed-off-by: Rex Zhu 
>>> ---
>>>drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 8 ++--
>>>drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h | 4 +++-
>>>drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 6 +-
>>>3 files changed, 14 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>>> index 8edf54b..8802ff2 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>>> @@ -529,10 +529,14 @@ int amdgpu_ctx_wait_prev_fence(struct
>> amdgpu_ctx *ctx,
>>> return 0;
>>>}
>>>
>>> -void amdgpu_ctx_mgr_init(struct amdgpu_ctx_mgr *mgr)
>>> +int amdgpu_ctx_mgr_init(struct amdgpu_ctx_mgr *mgr)
>>>{
>>> mutex_init(>lock);
>>> idr_init(>ctx_handles);
>>> +   mgr->resv_vm_bitmap =
>> kzalloc(DIV_ROUND_UP(AMDGPU_VM_MAX_NUM_CTX, BITS_PER_BYTE),
>> GFP_KERNEL);
>>> +   if (unlikely(!mgr->resv_vm_bitmap))
>>> +   return -ENOMEM;
>>> +   return 0;
>>>}
>>>
>>>void amdgpu_ctx_mgr_entity_flush(struct amdgpu_ctx_mgr *mgr) @@
>>> -601,7 +605,7 @@ void amdgpu_ctx_mgr_fini(struct amdgpu_ctx_mgr
>> *mgr)
>>> if (kref_put(>refcount, amdgpu_ctx_fini) != 1)
>>> DRM_ERROR("ctx %p is still alive\n", ctx);
>>> }
>>> -
>>> +   kfree(mgr->resv_vm_bitmap);
>>> idr_destroy(>ctx_handles);
>>> mutex_destroy(>lock);
>>>}
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
>>> index b3b012c..94ac951 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
>>> @@ -38,6 +38,7 @@ struct amdgpu_ctx_entity {
>>>struct amdgpu_ctx {
>>> struct kref refcount;
>>> struct amdgpu_device*adev;
>>> +
>>> unsignedreset_counter;
>>> unsignedreset_counter_query;
>>> uint32_tvram_lost_counter;
>>> @@ -56,6 +57,7 @@ struct amdgpu_ctx_mgr {
>>> struct mutexlock;
>>> /* protected by lock */
>>> struct idr  ctx_handles;
>>> +   unsigned long   *resv_vm_bitmap;
>>>};
>>>
>>>extern const unsigned int
>> amdgpu_ctx_num_entities[AMDGPU_HW_IP_NUM];
>>> @@ -80,7 +82,7 @@ int amdgpu_ctx_ioctl(struct drm_device *dev, void
>> *data,
>>>int amdgpu_ctx_wait_prev_fence(struct amdgpu_ctx *ctx,
>>>struct drm_sched_entity *entity);
>>>
>>> -void amdgpu_ctx_mgr_init(struct amdgpu_ctx_mgr *mgr);
>>> +int amdgpu_ctx_mgr_init(struct amdgpu_ctx_mgr *mgr);
>>>void amdgpu_ctx_mgr_entity_fini(struct amdgpu_ctx_mgr *mgr);
>>>void amdgpu_ctx_mgr_entity_flush(struct amdgpu_ctx_mgr *mgr);
>>>void amdgpu_ctx_mgr_fini(struct amdgpu_ctx_mgr *mgr); diff --git
>>> a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>> index 52e4e90..338a091 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>> @@ -988,11 +988,15 @@ int amdgpu_driver_open_kms(struct drm_device
>> *dev, struct drm_file *file_priv)
>>> mutex_init(>bo_list_lock);
>>> idr_init(>bo_list_handles);
>>>
>>> -   amdgpu_ctx_mgr_init(>ctx_mgr);
>>> +   if (amdgpu_ctx_mgr_init(>ctx_mgr))
>>> +   goto error_ctx_mgr;
>>>
>>> file_priv->driver_priv = fpriv;
>>> goto out_suspend;
>>>
>>> +error_ctx_mgr:
>>> +   idr_destroy(>bo_list_handles);
>>> +   mutex_destroy(>bo_list_lock);
>>>error_vm:
>>> amdgpu_vm_fini(adev, >vm);
>>>

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

RE: [PATCH 4/9] drm/amdgpu: Add a bitmask in amdgpu_ctx_mgr

2018-12-06 Thread Zhu, Rex 
Hi Christian,

We allocate and map csa per ctx,  need to record the used/free vm space.
So use bitmap to manager the reserved vm space.

Also add resv_space_id in ctx.
When amdgpu_ctx_fini, we can clear the bit in the bitmap.


Best Regards
Rex
> -Original Message-
> From: Christian König 
> Sent: Thursday, December 6, 2018 8:34 PM
> To: Zhu, Rex ; amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH 4/9] drm/amdgpu: Add a bitmask in amdgpu_ctx_mgr
> 
> Am 06.12.18 um 13:14 schrieb Rex Zhu:
> > used to manager the reserverd vm space.
> 
> Why do we need that?
> 
> Christian.
> 
> >
> > Signed-off-by: Rex Zhu 
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 8 ++--
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h | 4 +++-
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 6 +-
> >   3 files changed, 14 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> > index 8edf54b..8802ff2 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> > @@ -529,10 +529,14 @@ int amdgpu_ctx_wait_prev_fence(struct
> amdgpu_ctx *ctx,
> > return 0;
> >   }
> >
> > -void amdgpu_ctx_mgr_init(struct amdgpu_ctx_mgr *mgr)
> > +int amdgpu_ctx_mgr_init(struct amdgpu_ctx_mgr *mgr)
> >   {
> > mutex_init(>lock);
> > idr_init(>ctx_handles);
> > +   mgr->resv_vm_bitmap =
> kzalloc(DIV_ROUND_UP(AMDGPU_VM_MAX_NUM_CTX, BITS_PER_BYTE),
> GFP_KERNEL);
> > +   if (unlikely(!mgr->resv_vm_bitmap))
> > +   return -ENOMEM;
> > +   return 0;
> >   }
> >
> >   void amdgpu_ctx_mgr_entity_flush(struct amdgpu_ctx_mgr *mgr) @@
> > -601,7 +605,7 @@ void amdgpu_ctx_mgr_fini(struct amdgpu_ctx_mgr
> *mgr)
> > if (kref_put(>refcount, amdgpu_ctx_fini) != 1)
> > DRM_ERROR("ctx %p is still alive\n", ctx);
> > }
> > -
> > +   kfree(mgr->resv_vm_bitmap);
> > idr_destroy(>ctx_handles);
> > mutex_destroy(>lock);
> >   }
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
> > index b3b012c..94ac951 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
> > @@ -38,6 +38,7 @@ struct amdgpu_ctx_entity {
> >   struct amdgpu_ctx {
> > struct kref refcount;
> > struct amdgpu_device*adev;
> > +
> > unsignedreset_counter;
> > unsignedreset_counter_query;
> > uint32_tvram_lost_counter;
> > @@ -56,6 +57,7 @@ struct amdgpu_ctx_mgr {
> > struct mutexlock;
> > /* protected by lock */
> > struct idr  ctx_handles;
> > +   unsigned long   *resv_vm_bitmap;
> >   };
> >
> >   extern const unsigned int
> amdgpu_ctx_num_entities[AMDGPU_HW_IP_NUM];
> > @@ -80,7 +82,7 @@ int amdgpu_ctx_ioctl(struct drm_device *dev, void
> *data,
> >   int amdgpu_ctx_wait_prev_fence(struct amdgpu_ctx *ctx,
> >struct drm_sched_entity *entity);
> >
> > -void amdgpu_ctx_mgr_init(struct amdgpu_ctx_mgr *mgr);
> > +int amdgpu_ctx_mgr_init(struct amdgpu_ctx_mgr *mgr);
> >   void amdgpu_ctx_mgr_entity_fini(struct amdgpu_ctx_mgr *mgr);
> >   void amdgpu_ctx_mgr_entity_flush(struct amdgpu_ctx_mgr *mgr);
> >   void amdgpu_ctx_mgr_fini(struct amdgpu_ctx_mgr *mgr); diff --git
> > a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> > index 52e4e90..338a091 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> > @@ -988,11 +988,15 @@ int amdgpu_driver_open_kms(struct drm_device
> *dev, struct drm_file *file_priv)
> > mutex_init(>bo_list_lock);
> > idr_init(>bo_list_handles);
> >
> > -   amdgpu_ctx_mgr_init(>ctx_mgr);
> > +   if (amdgpu_ctx_mgr_init(>ctx_mgr))
> > +   goto error_ctx_mgr;
> >
> > file_priv->driver_priv = fpriv;
> > goto out_suspend;
> >
> > +error_ctx_mgr:
> > +   idr_destroy(>bo_list_handles);
> > +   mutex_destroy(>bo_list_lock);
> >   error_vm:
> > amdgpu_vm_fini(adev, >vm);
> >

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

Re: [PATCH 4/9] drm/amdgpu: Add a bitmask in amdgpu_ctx_mgr

2018-12-06 Thread Christian König 

Am 06.12.18 um 13:14 schrieb Rex Zhu:

used to manager the reserverd vm space.


Why do we need that?

Christian.



Signed-off-by: Rex Zhu 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 8 ++--
  drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h | 4 +++-
  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 6 +-
  3 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
index 8edf54b..8802ff2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -529,10 +529,14 @@ int amdgpu_ctx_wait_prev_fence(struct amdgpu_ctx *ctx,
return 0;
  }
  
-void amdgpu_ctx_mgr_init(struct amdgpu_ctx_mgr *mgr)

+int amdgpu_ctx_mgr_init(struct amdgpu_ctx_mgr *mgr)
  {
mutex_init(>lock);
idr_init(>ctx_handles);
+   mgr->resv_vm_bitmap = kzalloc(DIV_ROUND_UP(AMDGPU_VM_MAX_NUM_CTX, 
BITS_PER_BYTE), GFP_KERNEL);
+   if (unlikely(!mgr->resv_vm_bitmap))
+   return -ENOMEM;
+   return 0;
  }
  
  void amdgpu_ctx_mgr_entity_flush(struct amdgpu_ctx_mgr *mgr)

@@ -601,7 +605,7 @@ void amdgpu_ctx_mgr_fini(struct amdgpu_ctx_mgr *mgr)
if (kref_put(>refcount, amdgpu_ctx_fini) != 1)
DRM_ERROR("ctx %p is still alive\n", ctx);
}
-
+   kfree(mgr->resv_vm_bitmap);
idr_destroy(>ctx_handles);
mutex_destroy(>lock);
  }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
index b3b012c..94ac951 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
@@ -38,6 +38,7 @@ struct amdgpu_ctx_entity {
  struct amdgpu_ctx {
struct kref refcount;
struct amdgpu_device*adev;
+
unsignedreset_counter;
unsignedreset_counter_query;
uint32_tvram_lost_counter;
@@ -56,6 +57,7 @@ struct amdgpu_ctx_mgr {
struct mutexlock;
/* protected by lock */
struct idr  ctx_handles;
+   unsigned long   *resv_vm_bitmap;
  };
  
  extern const unsigned int amdgpu_ctx_num_entities[AMDGPU_HW_IP_NUM];

@@ -80,7 +82,7 @@ int amdgpu_ctx_ioctl(struct drm_device *dev, void *data,
  int amdgpu_ctx_wait_prev_fence(struct amdgpu_ctx *ctx,
   struct drm_sched_entity *entity);
  
-void amdgpu_ctx_mgr_init(struct amdgpu_ctx_mgr *mgr);

+int amdgpu_ctx_mgr_init(struct amdgpu_ctx_mgr *mgr);
  void amdgpu_ctx_mgr_entity_fini(struct amdgpu_ctx_mgr *mgr);
  void amdgpu_ctx_mgr_entity_flush(struct amdgpu_ctx_mgr *mgr);
  void amdgpu_ctx_mgr_fini(struct amdgpu_ctx_mgr *mgr);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 52e4e90..338a091 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -988,11 +988,15 @@ int amdgpu_driver_open_kms(struct drm_device *dev, struct 
drm_file *file_priv)
mutex_init(>bo_list_lock);
idr_init(>bo_list_handles);
  
-	amdgpu_ctx_mgr_init(>ctx_mgr);

+   if (amdgpu_ctx_mgr_init(>ctx_mgr))
+   goto error_ctx_mgr;
  
  	file_priv->driver_priv = fpriv;

goto out_suspend;
  
+error_ctx_mgr:

+   idr_destroy(>bo_list_handles);
+   mutex_destroy(>bo_list_lock);
  error_vm:
amdgpu_vm_fini(adev, >vm);
  


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