diff mbox

[02/83] drm/radeon: reduce number of free VMIDs and pipes in KV

Message ID 1405029027-6085-1-git-send-email-oded.gabbay@amd.com (mailing list archive)
State New, archived
Headers show

Commit Message

Oded Gabbay July 10, 2014, 9:50 p.m. UTC
To support HSA on KV, we need to limit the number of vmids and pipes
that are available for radeon's use with KV.

This patch reserves VMIDs 8-15 for KFD (so radeon can only use VMIDs
0-7) and also makes radeon thinks that KV has only a single MEC with a single
pipe in it

Signed-off-by: Oded Gabbay <oded.gabbay@amd.com>
---
 drivers/gpu/drm/radeon/cik.c | 48 ++++++++++++++++++++++----------------------
 1 file changed, 24 insertions(+), 24 deletions(-)

Comments

Jerome Glisse July 11, 2014, 4:05 p.m. UTC | #1
On Fri, Jul 11, 2014 at 12:50:02AM +0300, Oded Gabbay wrote:
> To support HSA on KV, we need to limit the number of vmids and pipes
> that are available for radeon's use with KV.
> 
> This patch reserves VMIDs 8-15 for KFD (so radeon can only use VMIDs
> 0-7) and also makes radeon thinks that KV has only a single MEC with a single
> pipe in it
> 
> Signed-off-by: Oded Gabbay <oded.gabbay@amd.com>

Reviewed-by: Jérôme Glisse <jglisse@redhat.com>

> ---
>  drivers/gpu/drm/radeon/cik.c | 48 ++++++++++++++++++++++----------------------
>  1 file changed, 24 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c
> index 4bfc2c0..e0c8052 100644
> --- a/drivers/gpu/drm/radeon/cik.c
> +++ b/drivers/gpu/drm/radeon/cik.c
> @@ -4662,12 +4662,11 @@ static int cik_mec_init(struct radeon_device *rdev)
>  	/*
>  	 * KV:    2 MEC, 4 Pipes/MEC, 8 Queues/Pipe - 64 Queues total
>  	 * CI/KB: 1 MEC, 4 Pipes/MEC, 8 Queues/Pipe - 32 Queues total
> +	 * Nonetheless, we assign only 1 pipe because all other pipes will
> +	 * be handled by KFD
>  	 */
> -	if (rdev->family == CHIP_KAVERI)
> -		rdev->mec.num_mec = 2;
> -	else
> -		rdev->mec.num_mec = 1;
> -	rdev->mec.num_pipe = 4;
> +	rdev->mec.num_mec = 1;
> +	rdev->mec.num_pipe = 1;
>  	rdev->mec.num_queue = rdev->mec.num_mec * rdev->mec.num_pipe * 8;
>  
>  	if (rdev->mec.hpd_eop_obj == NULL) {
> @@ -4809,28 +4808,24 @@ static int cik_cp_compute_resume(struct radeon_device *rdev)
>  
>  	/* init the pipes */
>  	mutex_lock(&rdev->srbm_mutex);
> -	for (i = 0; i < (rdev->mec.num_pipe * rdev->mec.num_mec); i++) {
> -		int me = (i < 4) ? 1 : 2;
> -		int pipe = (i < 4) ? i : (i - 4);
>  
> -		eop_gpu_addr = rdev->mec.hpd_eop_gpu_addr + (i * MEC_HPD_SIZE * 2);
> +	eop_gpu_addr = rdev->mec.hpd_eop_gpu_addr;
>  
> -		cik_srbm_select(rdev, me, pipe, 0, 0);
> +	cik_srbm_select(rdev, 0, 0, 0, 0);
>  
> -		/* write the EOP addr */
> -		WREG32(CP_HPD_EOP_BASE_ADDR, eop_gpu_addr >> 8);
> -		WREG32(CP_HPD_EOP_BASE_ADDR_HI, upper_32_bits(eop_gpu_addr) >> 8);
> +	/* write the EOP addr */
> +	WREG32(CP_HPD_EOP_BASE_ADDR, eop_gpu_addr >> 8);
> +	WREG32(CP_HPD_EOP_BASE_ADDR_HI, upper_32_bits(eop_gpu_addr) >> 8);
>  
> -		/* set the VMID assigned */
> -		WREG32(CP_HPD_EOP_VMID, 0);
> +	/* set the VMID assigned */
> +	WREG32(CP_HPD_EOP_VMID, 0);
> +
> +	/* set the EOP size, register value is 2^(EOP_SIZE+1) dwords */
> +	tmp = RREG32(CP_HPD_EOP_CONTROL);
> +	tmp &= ~EOP_SIZE_MASK;
> +	tmp |= order_base_2(MEC_HPD_SIZE / 8);
> +	WREG32(CP_HPD_EOP_CONTROL, tmp);
>  
> -		/* set the EOP size, register value is 2^(EOP_SIZE+1) dwords */
> -		tmp = RREG32(CP_HPD_EOP_CONTROL);
> -		tmp &= ~EOP_SIZE_MASK;
> -		tmp |= order_base_2(MEC_HPD_SIZE / 8);
> -		WREG32(CP_HPD_EOP_CONTROL, tmp);
> -	}
> -	cik_srbm_select(rdev, 0, 0, 0, 0);
>  	mutex_unlock(&rdev->srbm_mutex);
>  
>  	/* init the queues.  Just two for now. */
> @@ -5876,8 +5871,13 @@ int cik_ib_parse(struct radeon_device *rdev, struct radeon_ib *ib)
>   */
>  int cik_vm_init(struct radeon_device *rdev)
>  {
> -	/* number of VMs */
> -	rdev->vm_manager.nvm = 16;
> +	/*
> +	 * number of VMs
> +	 * VMID 0 is reserved for Graphics
> +	 * radeon compute will use VMIDs 1-7
> +	 * KFD will use VMIDs 8-15
> +	 */
> +	rdev->vm_manager.nvm = 8;
>  	/* base offset of vram pages */
>  	if (rdev->flags & RADEON_IS_IGP) {
>  		u64 tmp = RREG32(MC_VM_FB_OFFSET);
> -- 
> 1.9.1
>
Christian König July 11, 2014, 4:18 p.m. UTC | #2
Am 11.07.2014 18:05, schrieb Jerome Glisse:
> On Fri, Jul 11, 2014 at 12:50:02AM +0300, Oded Gabbay wrote:
>> To support HSA on KV, we need to limit the number of vmids and pipes
>> that are available for radeon's use with KV.
>>
>> This patch reserves VMIDs 8-15 for KFD (so radeon can only use VMIDs
>> 0-7) and also makes radeon thinks that KV has only a single MEC with a single
>> pipe in it
>>
>> Signed-off-by: Oded Gabbay <oded.gabbay@amd.com>
> Reviewed-by: Jérôme Glisse <jglisse@redhat.com>

At least fro the VMIDs on demand allocation should be trivial to 
implement, so I would rather prefer this instead of a fixed assignment.

Christian.

>
>> ---
>>   drivers/gpu/drm/radeon/cik.c | 48 ++++++++++++++++++++++----------------------
>>   1 file changed, 24 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c
>> index 4bfc2c0..e0c8052 100644
>> --- a/drivers/gpu/drm/radeon/cik.c
>> +++ b/drivers/gpu/drm/radeon/cik.c
>> @@ -4662,12 +4662,11 @@ static int cik_mec_init(struct radeon_device *rdev)
>>   	/*
>>   	 * KV:    2 MEC, 4 Pipes/MEC, 8 Queues/Pipe - 64 Queues total
>>   	 * CI/KB: 1 MEC, 4 Pipes/MEC, 8 Queues/Pipe - 32 Queues total
>> +	 * Nonetheless, we assign only 1 pipe because all other pipes will
>> +	 * be handled by KFD
>>   	 */
>> -	if (rdev->family == CHIP_KAVERI)
>> -		rdev->mec.num_mec = 2;
>> -	else
>> -		rdev->mec.num_mec = 1;
>> -	rdev->mec.num_pipe = 4;
>> +	rdev->mec.num_mec = 1;
>> +	rdev->mec.num_pipe = 1;
>>   	rdev->mec.num_queue = rdev->mec.num_mec * rdev->mec.num_pipe * 8;
>>   
>>   	if (rdev->mec.hpd_eop_obj == NULL) {
>> @@ -4809,28 +4808,24 @@ static int cik_cp_compute_resume(struct radeon_device *rdev)
>>   
>>   	/* init the pipes */
>>   	mutex_lock(&rdev->srbm_mutex);
>> -	for (i = 0; i < (rdev->mec.num_pipe * rdev->mec.num_mec); i++) {
>> -		int me = (i < 4) ? 1 : 2;
>> -		int pipe = (i < 4) ? i : (i - 4);
>>   
>> -		eop_gpu_addr = rdev->mec.hpd_eop_gpu_addr + (i * MEC_HPD_SIZE * 2);
>> +	eop_gpu_addr = rdev->mec.hpd_eop_gpu_addr;
>>   
>> -		cik_srbm_select(rdev, me, pipe, 0, 0);
>> +	cik_srbm_select(rdev, 0, 0, 0, 0);
>>   
>> -		/* write the EOP addr */
>> -		WREG32(CP_HPD_EOP_BASE_ADDR, eop_gpu_addr >> 8);
>> -		WREG32(CP_HPD_EOP_BASE_ADDR_HI, upper_32_bits(eop_gpu_addr) >> 8);
>> +	/* write the EOP addr */
>> +	WREG32(CP_HPD_EOP_BASE_ADDR, eop_gpu_addr >> 8);
>> +	WREG32(CP_HPD_EOP_BASE_ADDR_HI, upper_32_bits(eop_gpu_addr) >> 8);
>>   
>> -		/* set the VMID assigned */
>> -		WREG32(CP_HPD_EOP_VMID, 0);
>> +	/* set the VMID assigned */
>> +	WREG32(CP_HPD_EOP_VMID, 0);
>> +
>> +	/* set the EOP size, register value is 2^(EOP_SIZE+1) dwords */
>> +	tmp = RREG32(CP_HPD_EOP_CONTROL);
>> +	tmp &= ~EOP_SIZE_MASK;
>> +	tmp |= order_base_2(MEC_HPD_SIZE / 8);
>> +	WREG32(CP_HPD_EOP_CONTROL, tmp);
>>   
>> -		/* set the EOP size, register value is 2^(EOP_SIZE+1) dwords */
>> -		tmp = RREG32(CP_HPD_EOP_CONTROL);
>> -		tmp &= ~EOP_SIZE_MASK;
>> -		tmp |= order_base_2(MEC_HPD_SIZE / 8);
>> -		WREG32(CP_HPD_EOP_CONTROL, tmp);
>> -	}
>> -	cik_srbm_select(rdev, 0, 0, 0, 0);
>>   	mutex_unlock(&rdev->srbm_mutex);
>>   
>>   	/* init the queues.  Just two for now. */
>> @@ -5876,8 +5871,13 @@ int cik_ib_parse(struct radeon_device *rdev, struct radeon_ib *ib)
>>    */
>>   int cik_vm_init(struct radeon_device *rdev)
>>   {
>> -	/* number of VMs */
>> -	rdev->vm_manager.nvm = 16;
>> +	/*
>> +	 * number of VMs
>> +	 * VMID 0 is reserved for Graphics
>> +	 * radeon compute will use VMIDs 1-7
>> +	 * KFD will use VMIDs 8-15
>> +	 */
>> +	rdev->vm_manager.nvm = 8;
>>   	/* base offset of vram pages */
>>   	if (rdev->flags & RADEON_IS_IGP) {
>>   		u64 tmp = RREG32(MC_VM_FB_OFFSET);
>> -- 
>> 1.9.1
>>
Alex Deucher July 11, 2014, 4:22 p.m. UTC | #3
On Fri, Jul 11, 2014 at 12:18 PM, Christian König
<christian.koenig@amd.com> wrote:
> Am 11.07.2014 18:05, schrieb Jerome Glisse:
>
>> On Fri, Jul 11, 2014 at 12:50:02AM +0300, Oded Gabbay wrote:
>>>
>>> To support HSA on KV, we need to limit the number of vmids and pipes
>>> that are available for radeon's use with KV.
>>>
>>> This patch reserves VMIDs 8-15 for KFD (so radeon can only use VMIDs
>>> 0-7) and also makes radeon thinks that KV has only a single MEC with a
>>> single
>>> pipe in it
>>>
>>> Signed-off-by: Oded Gabbay <oded.gabbay@amd.com>
>>
>> Reviewed-by: Jérôme Glisse <jglisse@redhat.com>
>
>
> At least fro the VMIDs on demand allocation should be trivial to implement,
> so I would rather prefer this instead of a fixed assignment.

IIRC, the way the CP hw scheduler works you have to give it a range of
vmids and it assigns them dynamically as queues are mapped so
effectively they are potentially in use once the CP scheduler is set
up.

Alex


>
> Christian.
>
>
>>
>>> ---
>>>   drivers/gpu/drm/radeon/cik.c | 48
>>> ++++++++++++++++++++++----------------------
>>>   1 file changed, 24 insertions(+), 24 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c
>>> index 4bfc2c0..e0c8052 100644
>>> --- a/drivers/gpu/drm/radeon/cik.c
>>> +++ b/drivers/gpu/drm/radeon/cik.c
>>> @@ -4662,12 +4662,11 @@ static int cik_mec_init(struct radeon_device
>>> *rdev)
>>>         /*
>>>          * KV:    2 MEC, 4 Pipes/MEC, 8 Queues/Pipe - 64 Queues total
>>>          * CI/KB: 1 MEC, 4 Pipes/MEC, 8 Queues/Pipe - 32 Queues total
>>> +        * Nonetheless, we assign only 1 pipe because all other pipes
>>> will
>>> +        * be handled by KFD
>>>          */
>>> -       if (rdev->family == CHIP_KAVERI)
>>> -               rdev->mec.num_mec = 2;
>>> -       else
>>> -               rdev->mec.num_mec = 1;
>>> -       rdev->mec.num_pipe = 4;
>>> +       rdev->mec.num_mec = 1;
>>> +       rdev->mec.num_pipe = 1;
>>>         rdev->mec.num_queue = rdev->mec.num_mec * rdev->mec.num_pipe * 8;
>>>         if (rdev->mec.hpd_eop_obj == NULL) {
>>> @@ -4809,28 +4808,24 @@ static int cik_cp_compute_resume(struct
>>> radeon_device *rdev)
>>>         /* init the pipes */
>>>         mutex_lock(&rdev->srbm_mutex);
>>> -       for (i = 0; i < (rdev->mec.num_pipe * rdev->mec.num_mec); i++) {
>>> -               int me = (i < 4) ? 1 : 2;
>>> -               int pipe = (i < 4) ? i : (i - 4);
>>>   -             eop_gpu_addr = rdev->mec.hpd_eop_gpu_addr + (i *
>>> MEC_HPD_SIZE * 2);
>>> +       eop_gpu_addr = rdev->mec.hpd_eop_gpu_addr;
>>>   -             cik_srbm_select(rdev, me, pipe, 0, 0);
>>> +       cik_srbm_select(rdev, 0, 0, 0, 0);
>>>   -             /* write the EOP addr */
>>> -               WREG32(CP_HPD_EOP_BASE_ADDR, eop_gpu_addr >> 8);
>>> -               WREG32(CP_HPD_EOP_BASE_ADDR_HI,
>>> upper_32_bits(eop_gpu_addr) >> 8);
>>> +       /* write the EOP addr */
>>> +       WREG32(CP_HPD_EOP_BASE_ADDR, eop_gpu_addr >> 8);
>>> +       WREG32(CP_HPD_EOP_BASE_ADDR_HI, upper_32_bits(eop_gpu_addr) >>
>>> 8);
>>>   -             /* set the VMID assigned */
>>> -               WREG32(CP_HPD_EOP_VMID, 0);
>>> +       /* set the VMID assigned */
>>> +       WREG32(CP_HPD_EOP_VMID, 0);
>>> +
>>> +       /* set the EOP size, register value is 2^(EOP_SIZE+1) dwords */
>>> +       tmp = RREG32(CP_HPD_EOP_CONTROL);
>>> +       tmp &= ~EOP_SIZE_MASK;
>>> +       tmp |= order_base_2(MEC_HPD_SIZE / 8);
>>> +       WREG32(CP_HPD_EOP_CONTROL, tmp);
>>>   -             /* set the EOP size, register value is 2^(EOP_SIZE+1)
>>> dwords */
>>> -               tmp = RREG32(CP_HPD_EOP_CONTROL);
>>> -               tmp &= ~EOP_SIZE_MASK;
>>> -               tmp |= order_base_2(MEC_HPD_SIZE / 8);
>>> -               WREG32(CP_HPD_EOP_CONTROL, tmp);
>>> -       }
>>> -       cik_srbm_select(rdev, 0, 0, 0, 0);
>>>         mutex_unlock(&rdev->srbm_mutex);
>>>         /* init the queues.  Just two for now. */
>>> @@ -5876,8 +5871,13 @@ int cik_ib_parse(struct radeon_device *rdev,
>>> struct radeon_ib *ib)
>>>    */
>>>   int cik_vm_init(struct radeon_device *rdev)
>>>   {
>>> -       /* number of VMs */
>>> -       rdev->vm_manager.nvm = 16;
>>> +       /*
>>> +        * number of VMs
>>> +        * VMID 0 is reserved for Graphics
>>> +        * radeon compute will use VMIDs 1-7
>>> +        * KFD will use VMIDs 8-15
>>> +        */
>>> +       rdev->vm_manager.nvm = 8;
>>>         /* base offset of vram pages */
>>>         if (rdev->flags & RADEON_IS_IGP) {
>>>                 u64 tmp = RREG32(MC_VM_FB_OFFSET);
>>> --
>>> 1.9.1
>>>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
Bridgman, John July 11, 2014, 5:07 p.m. UTC | #4
>-----Original Message-----

>From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On Behalf

>Of Alex Deucher

>Sent: Friday, July 11, 2014 12:23 PM

>To: Koenig, Christian

>Cc: Oded Gabbay; Lewycky, Andrew; LKML; Maling list - DRI developers;

>Deucher, Alexander

>Subject: Re: [PATCH 02/83] drm/radeon: reduce number of free VMIDs and

>pipes in KV

>

>On Fri, Jul 11, 2014 at 12:18 PM, Christian König <christian.koenig@amd.com>

>wrote:

>> Am 11.07.2014 18:05, schrieb Jerome Glisse:

>>

>>> On Fri, Jul 11, 2014 at 12:50:02AM +0300, Oded Gabbay wrote:

>>>>

>>>> To support HSA on KV, we need to limit the number of vmids and pipes

>>>> that are available for radeon's use with KV.

>>>>

>>>> This patch reserves VMIDs 8-15 for KFD (so radeon can only use VMIDs

>>>> 0-7) and also makes radeon thinks that KV has only a single MEC with

>>>> a single pipe in it

>>>>

>>>> Signed-off-by: Oded Gabbay <oded.gabbay@amd.com>

>>>

>>> Reviewed-by: Jérôme Glisse <jglisse@redhat.com>

>>

>>

>> At least fro the VMIDs on demand allocation should be trivial to

>> implement, so I would rather prefer this instead of a fixed assignment.

>

>IIRC, the way the CP hw scheduler works you have to give it a range of vmids

>and it assigns them dynamically as queues are mapped so effectively they

>are potentially in use once the CP scheduler is set up.

>

>Alex


Right. The SET_RESOURCES packet (kfd_pm4_headers.h, added in patch 49) allocates a range of HW queues, VMIDs and GDS to the HW scheduler, then the scheduler uses the allocated VMIDs to support a potentially larger number of user processes by dynamically mapping PASIDs to VMIDs and memory queue descriptors (MQDs) to HW queues.

BTW Oded I think we have some duplicated defines at the end of kfd_pm4_headers.h, if they are really duplicates it would be great to remove those before the pull request.

Thanks,
JB

>

>

>>

>> Christian.

>>

>>

>>>

>>>> ---

>>>>   drivers/gpu/drm/radeon/cik.c | 48

>>>> ++++++++++++++++++++++----------------------

>>>>   1 file changed, 24 insertions(+), 24 deletions(-)

>>>>

>>>> diff --git a/drivers/gpu/drm/radeon/cik.c

>>>> b/drivers/gpu/drm/radeon/cik.c index 4bfc2c0..e0c8052 100644

>>>> --- a/drivers/gpu/drm/radeon/cik.c

>>>> +++ b/drivers/gpu/drm/radeon/cik.c

>>>> @@ -4662,12 +4662,11 @@ static int cik_mec_init(struct radeon_device

>>>> *rdev)

>>>>         /*

>>>>          * KV:    2 MEC, 4 Pipes/MEC, 8 Queues/Pipe - 64 Queues total

>>>>          * CI/KB: 1 MEC, 4 Pipes/MEC, 8 Queues/Pipe - 32 Queues

>>>> total

>>>> +        * Nonetheless, we assign only 1 pipe because all other

>>>> + pipes

>>>> will

>>>> +        * be handled by KFD

>>>>          */

>>>> -       if (rdev->family == CHIP_KAVERI)

>>>> -               rdev->mec.num_mec = 2;

>>>> -       else

>>>> -               rdev->mec.num_mec = 1;

>>>> -       rdev->mec.num_pipe = 4;

>>>> +       rdev->mec.num_mec = 1;

>>>> +       rdev->mec.num_pipe = 1;

>>>>         rdev->mec.num_queue = rdev->mec.num_mec * rdev-

>>mec.num_pipe * 8;

>>>>         if (rdev->mec.hpd_eop_obj == NULL) { @@ -4809,28 +4808,24 @@

>>>> static int cik_cp_compute_resume(struct radeon_device *rdev)

>>>>         /* init the pipes */

>>>>         mutex_lock(&rdev->srbm_mutex);

>>>> -       for (i = 0; i < (rdev->mec.num_pipe * rdev->mec.num_mec); i++) {

>>>> -               int me = (i < 4) ? 1 : 2;

>>>> -               int pipe = (i < 4) ? i : (i - 4);

>>>>   -             eop_gpu_addr = rdev->mec.hpd_eop_gpu_addr + (i *

>>>> MEC_HPD_SIZE * 2);

>>>> +       eop_gpu_addr = rdev->mec.hpd_eop_gpu_addr;

>>>>   -             cik_srbm_select(rdev, me, pipe, 0, 0);

>>>> +       cik_srbm_select(rdev, 0, 0, 0, 0);

>>>>   -             /* write the EOP addr */

>>>> -               WREG32(CP_HPD_EOP_BASE_ADDR, eop_gpu_addr >> 8);

>>>> -               WREG32(CP_HPD_EOP_BASE_ADDR_HI,

>>>> upper_32_bits(eop_gpu_addr) >> 8);

>>>> +       /* write the EOP addr */

>>>> +       WREG32(CP_HPD_EOP_BASE_ADDR, eop_gpu_addr >> 8);

>>>> +       WREG32(CP_HPD_EOP_BASE_ADDR_HI,

>upper_32_bits(eop_gpu_addr)

>>>> + >>

>>>> 8);

>>>>   -             /* set the VMID assigned */

>>>> -               WREG32(CP_HPD_EOP_VMID, 0);

>>>> +       /* set the VMID assigned */

>>>> +       WREG32(CP_HPD_EOP_VMID, 0);

>>>> +

>>>> +       /* set the EOP size, register value is 2^(EOP_SIZE+1) dwords */

>>>> +       tmp = RREG32(CP_HPD_EOP_CONTROL);

>>>> +       tmp &= ~EOP_SIZE_MASK;

>>>> +       tmp |= order_base_2(MEC_HPD_SIZE / 8);

>>>> +       WREG32(CP_HPD_EOP_CONTROL, tmp);

>>>>   -             /* set the EOP size, register value is 2^(EOP_SIZE+1)

>>>> dwords */

>>>> -               tmp = RREG32(CP_HPD_EOP_CONTROL);

>>>> -               tmp &= ~EOP_SIZE_MASK;

>>>> -               tmp |= order_base_2(MEC_HPD_SIZE / 8);

>>>> -               WREG32(CP_HPD_EOP_CONTROL, tmp);

>>>> -       }

>>>> -       cik_srbm_select(rdev, 0, 0, 0, 0);

>>>>         mutex_unlock(&rdev->srbm_mutex);

>>>>         /* init the queues.  Just two for now. */ @@ -5876,8

>>>> +5871,13 @@ int cik_ib_parse(struct radeon_device *rdev, struct

>>>> radeon_ib *ib)

>>>>    */

>>>>   int cik_vm_init(struct radeon_device *rdev)

>>>>   {

>>>> -       /* number of VMs */

>>>> -       rdev->vm_manager.nvm = 16;

>>>> +       /*

>>>> +        * number of VMs

>>>> +        * VMID 0 is reserved for Graphics

>>>> +        * radeon compute will use VMIDs 1-7

>>>> +        * KFD will use VMIDs 8-15

>>>> +        */

>>>> +       rdev->vm_manager.nvm = 8;

>>>>         /* base offset of vram pages */

>>>>         if (rdev->flags & RADEON_IS_IGP) {

>>>>                 u64 tmp = RREG32(MC_VM_FB_OFFSET);

>>>> --

>>>> 1.9.1

>>>>

>>

>> _______________________________________________

>> dri-devel mailing list

>> dri-devel@lists.freedesktop.org

>> http://lists.freedesktop.org/mailman/listinfo/dri-devel

>_______________________________________________

>dri-devel mailing list

>dri-devel@lists.freedesktop.org

>http://lists.freedesktop.org/mailman/listinfo/dri-devel
Ilyes Gouta July 11, 2014, 5:59 p.m. UTC | #5
Hi,

Just a side question (for information),

On Fri, Jul 11, 2014 at 6:07 PM, Bridgman, John <John.Bridgman@amd.com>
wrote:

>
> Right. The SET_RESOURCES packet (kfd_pm4_headers.h, added in patch 49)
> allocates a range of HW queues, VMIDs and GDS to the HW scheduler, then the
> scheduler uses the allocated VMIDs to support a potentially larger number
> of user processes by dynamically mapping PASIDs to VMIDs and memory queue
> descriptors (MQDs) to HW queues.
>

Are there any documentation/specifications online describing these
mechanisms?

Thanks,
Bridgman, John July 11, 2014, 10:54 p.m. UTC | #6
>From: Ilyes Gouta [mailto:ilyes.gouta@gmail.com] 
>Sent: Friday, July 11, 2014 2:00 PM
>To: Bridgman, John
>Cc: Alex Deucher; Koenig, Christian; Oded Gabbay; Deucher, Alexander; Lewycky, Andrew; LKML; Maling list - DRI developers
>Subject: Re: [PATCH 02/83] drm/radeon: reduce number of free VMIDs and pipes in KV
>
>Hi,
>
>Just a side question (for information),
>
>On Fri, Jul 11, 2014 at 6:07 PM, Bridgman, John <John.Bridgman@amd.com> wrote:
>
>Right. The SET_RESOURCES packet (kfd_pm4_headers.h, added in patch 49) allocates a range of HW queues, VMIDs and GDS to the HW scheduler, then >the scheduler uses the allocated VMIDs to support a potentially larger number of user processes by dynamically mapping PASIDs to VMIDs and memory >queue descriptors (MQDs) to HW queues.
>
>Are there any documentation/specifications online describing these mechanisms?

Nothing yet, but we should write some docco for this similar to what was written for the gfx blocks. I'll add that to the list, thanks.
Christian König July 12, 2014, 9 a.m. UTC | #7
Am 11.07.2014 18:22, schrieb Alex Deucher:
> On Fri, Jul 11, 2014 at 12:18 PM, Christian König
> <christian.koenig@amd.com> wrote:
>> Am 11.07.2014 18:05, schrieb Jerome Glisse:
>>
>>> On Fri, Jul 11, 2014 at 12:50:02AM +0300, Oded Gabbay wrote:
>>>> To support HSA on KV, we need to limit the number of vmids and pipes
>>>> that are available for radeon's use with KV.
>>>>
>>>> This patch reserves VMIDs 8-15 for KFD (so radeon can only use VMIDs
>>>> 0-7) and also makes radeon thinks that KV has only a single MEC with a
>>>> single
>>>> pipe in it
>>>>
>>>> Signed-off-by: Oded Gabbay <oded.gabbay@amd.com>
>>> Reviewed-by: Jérôme Glisse <jglisse@redhat.com>
>>
>> At least fro the VMIDs on demand allocation should be trivial to implement,
>> so I would rather prefer this instead of a fixed assignment.
> IIRC, the way the CP hw scheduler works you have to give it a range of
> vmids and it assigns them dynamically as queues are mapped so
> effectively they are potentially in use once the CP scheduler is set
> up.

That's not what I meant. Changing it completely on the fly is nice to 
have, but we should at least make it configurable as a module parameter.

And even if we hardcode it we should use a define for it somewhere 
instead of hardcoding 8 VMIDs on the KGD side and 8 VMIDs on KFD side 
without any relation to each other.

Christian.

> Alex
>
>
>> Christian.
>>
>>
>>>> ---
>>>>    drivers/gpu/drm/radeon/cik.c | 48
>>>> ++++++++++++++++++++++----------------------
>>>>    1 file changed, 24 insertions(+), 24 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c
>>>> index 4bfc2c0..e0c8052 100644
>>>> --- a/drivers/gpu/drm/radeon/cik.c
>>>> +++ b/drivers/gpu/drm/radeon/cik.c
>>>> @@ -4662,12 +4662,11 @@ static int cik_mec_init(struct radeon_device
>>>> *rdev)
>>>>          /*
>>>>           * KV:    2 MEC, 4 Pipes/MEC, 8 Queues/Pipe - 64 Queues total
>>>>           * CI/KB: 1 MEC, 4 Pipes/MEC, 8 Queues/Pipe - 32 Queues total
>>>> +        * Nonetheless, we assign only 1 pipe because all other pipes
>>>> will
>>>> +        * be handled by KFD
>>>>           */
>>>> -       if (rdev->family == CHIP_KAVERI)
>>>> -               rdev->mec.num_mec = 2;
>>>> -       else
>>>> -               rdev->mec.num_mec = 1;
>>>> -       rdev->mec.num_pipe = 4;
>>>> +       rdev->mec.num_mec = 1;
>>>> +       rdev->mec.num_pipe = 1;
>>>>          rdev->mec.num_queue = rdev->mec.num_mec * rdev->mec.num_pipe * 8;
>>>>          if (rdev->mec.hpd_eop_obj == NULL) {
>>>> @@ -4809,28 +4808,24 @@ static int cik_cp_compute_resume(struct
>>>> radeon_device *rdev)
>>>>          /* init the pipes */
>>>>          mutex_lock(&rdev->srbm_mutex);
>>>> -       for (i = 0; i < (rdev->mec.num_pipe * rdev->mec.num_mec); i++) {
>>>> -               int me = (i < 4) ? 1 : 2;
>>>> -               int pipe = (i < 4) ? i : (i - 4);
>>>>    -             eop_gpu_addr = rdev->mec.hpd_eop_gpu_addr + (i *
>>>> MEC_HPD_SIZE * 2);
>>>> +       eop_gpu_addr = rdev->mec.hpd_eop_gpu_addr;
>>>>    -             cik_srbm_select(rdev, me, pipe, 0, 0);
>>>> +       cik_srbm_select(rdev, 0, 0, 0, 0);
>>>>    -             /* write the EOP addr */
>>>> -               WREG32(CP_HPD_EOP_BASE_ADDR, eop_gpu_addr >> 8);
>>>> -               WREG32(CP_HPD_EOP_BASE_ADDR_HI,
>>>> upper_32_bits(eop_gpu_addr) >> 8);
>>>> +       /* write the EOP addr */
>>>> +       WREG32(CP_HPD_EOP_BASE_ADDR, eop_gpu_addr >> 8);
>>>> +       WREG32(CP_HPD_EOP_BASE_ADDR_HI, upper_32_bits(eop_gpu_addr) >>
>>>> 8);
>>>>    -             /* set the VMID assigned */
>>>> -               WREG32(CP_HPD_EOP_VMID, 0);
>>>> +       /* set the VMID assigned */
>>>> +       WREG32(CP_HPD_EOP_VMID, 0);
>>>> +
>>>> +       /* set the EOP size, register value is 2^(EOP_SIZE+1) dwords */
>>>> +       tmp = RREG32(CP_HPD_EOP_CONTROL);
>>>> +       tmp &= ~EOP_SIZE_MASK;
>>>> +       tmp |= order_base_2(MEC_HPD_SIZE / 8);
>>>> +       WREG32(CP_HPD_EOP_CONTROL, tmp);
>>>>    -             /* set the EOP size, register value is 2^(EOP_SIZE+1)
>>>> dwords */
>>>> -               tmp = RREG32(CP_HPD_EOP_CONTROL);
>>>> -               tmp &= ~EOP_SIZE_MASK;
>>>> -               tmp |= order_base_2(MEC_HPD_SIZE / 8);
>>>> -               WREG32(CP_HPD_EOP_CONTROL, tmp);
>>>> -       }
>>>> -       cik_srbm_select(rdev, 0, 0, 0, 0);
>>>>          mutex_unlock(&rdev->srbm_mutex);
>>>>          /* init the queues.  Just two for now. */
>>>> @@ -5876,8 +5871,13 @@ int cik_ib_parse(struct radeon_device *rdev,
>>>> struct radeon_ib *ib)
>>>>     */
>>>>    int cik_vm_init(struct radeon_device *rdev)
>>>>    {
>>>> -       /* number of VMs */
>>>> -       rdev->vm_manager.nvm = 16;
>>>> +       /*
>>>> +        * number of VMs
>>>> +        * VMID 0 is reserved for Graphics
>>>> +        * radeon compute will use VMIDs 1-7
>>>> +        * KFD will use VMIDs 8-15
>>>> +        */
>>>> +       rdev->vm_manager.nvm = 8;
>>>>          /* base offset of vram pages */
>>>>          if (rdev->flags & RADEON_IS_IGP) {
>>>>                  u64 tmp = RREG32(MC_VM_FB_OFFSET);
>>>> --
>>>> 1.9.1
>>>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
Michel Dänzer July 14, 2014, 7:31 a.m. UTC | #8
On 12.07.2014 18:00, Christian König wrote:
> Am 11.07.2014 18:22, schrieb Alex Deucher:
>> On Fri, Jul 11, 2014 at 12:18 PM, Christian König
>> <christian.koenig@amd.com> wrote:
>>> Am 11.07.2014 18:05, schrieb Jerome Glisse:
>>>
>>>> On Fri, Jul 11, 2014 at 12:50:02AM +0300, Oded Gabbay wrote:
>>>>> To support HSA on KV, we need to limit the number of vmids and pipes
>>>>> that are available for radeon's use with KV.
>>>>>
>>>>> This patch reserves VMIDs 8-15 for KFD (so radeon can only use VMIDs
>>>>> 0-7) and also makes radeon thinks that KV has only a single MEC with a
>>>>> single
>>>>> pipe in it
>>>>>
>>>>> Signed-off-by: Oded Gabbay <oded.gabbay@amd.com>
>>>> Reviewed-by: Jérôme Glisse <jglisse@redhat.com>
>>>
>>> At least fro the VMIDs on demand allocation should be trivial to
>>> implement,
>>> so I would rather prefer this instead of a fixed assignment.
>> IIRC, the way the CP hw scheduler works you have to give it a range of
>> vmids and it assigns them dynamically as queues are mapped so
>> effectively they are potentially in use once the CP scheduler is set
>> up.
> 
> That's not what I meant. Changing it completely on the fly is nice to
> have, but we should at least make it configurable as a module parameter.
> 
> And even if we hardcode it we should use a define for it somewhere
> instead of hardcoding 8 VMIDs on the KGD side and 8 VMIDs on KFD side
> without any relation to each other.

Seconded, and there should be more explanation and rationale for the way
things are set up in the code or at least in the commit log.
Michel Dänzer July 14, 2014, 7:38 a.m. UTC | #9
On 11.07.2014 06:50, Oded Gabbay wrote:
> @@ -5876,8 +5871,13 @@ int cik_ib_parse(struct radeon_device *rdev, struct radeon_ib *ib)
>   */
>  int cik_vm_init(struct radeon_device *rdev)
>  {
> -	/* number of VMs */
> -	rdev->vm_manager.nvm = 16;
> +	/*
> +	 * number of VMs
> +	 * VMID 0 is reserved for Graphics
> +	 * radeon compute will use VMIDs 1-7
> +	 * KFD will use VMIDs 8-15
> +	 */
> +	rdev->vm_manager.nvm = 8;

This comment is inaccurate: Graphics can use VMIDs 1-7 as well.
Christian König July 14, 2014, 7:58 a.m. UTC | #10
Am 14.07.2014 09:38, schrieb Michel Dänzer:
> On 11.07.2014 06:50, Oded Gabbay wrote:
>> @@ -5876,8 +5871,13 @@ int cik_ib_parse(struct radeon_device *rdev, struct radeon_ib *ib)
>>    */
>>   int cik_vm_init(struct radeon_device *rdev)
>>   {
>> -	/* number of VMs */
>> -	rdev->vm_manager.nvm = 16;
>> +	/*
>> +	 * number of VMs
>> +	 * VMID 0 is reserved for Graphics
>> +	 * radeon compute will use VMIDs 1-7
>> +	 * KFD will use VMIDs 8-15
>> +	 */
>> +	rdev->vm_manager.nvm = 8;
> This comment is inaccurate: Graphics can use VMIDs 1-7 as well.

Actually VMID 0 is reserved for system use and graphics operation only 
use VMIDs 1-7.

Christian.
Oded Gabbay July 17, 2014, 11:47 a.m. UTC | #11
On 14/07/14 10:58, Christian König wrote:
> Am 14.07.2014 09:38, schrieb Michel Dänzer:
>> On 11.07.2014 06:50, Oded Gabbay wrote:
>>> @@ -5876,8 +5871,13 @@ int cik_ib_parse(struct radeon_device *rdev, struct
>>> radeon_ib *ib)
>>>    */
>>>   int cik_vm_init(struct radeon_device *rdev)
>>>   {
>>> -    /* number of VMs */
>>> -    rdev->vm_manager.nvm = 16;
>>> +    /*
>>> +     * number of VMs
>>> +     * VMID 0 is reserved for Graphics
>>> +     * radeon compute will use VMIDs 1-7
>>> +     * KFD will use VMIDs 8-15
>>> +     */
>>> +    rdev->vm_manager.nvm = 8;
>> This comment is inaccurate: Graphics can use VMIDs 1-7 as well.
>
> Actually VMID 0 is reserved for system use and graphics operation only use VMIDs
> 1-7.
>
> Christian.
Will be fixed in v2 of the patchset

	Oded
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
diff mbox

Patch

diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c
index 4bfc2c0..e0c8052 100644
--- a/drivers/gpu/drm/radeon/cik.c
+++ b/drivers/gpu/drm/radeon/cik.c
@@ -4662,12 +4662,11 @@  static int cik_mec_init(struct radeon_device *rdev)
 	/*
 	 * KV:    2 MEC, 4 Pipes/MEC, 8 Queues/Pipe - 64 Queues total
 	 * CI/KB: 1 MEC, 4 Pipes/MEC, 8 Queues/Pipe - 32 Queues total
+	 * Nonetheless, we assign only 1 pipe because all other pipes will
+	 * be handled by KFD
 	 */
-	if (rdev->family == CHIP_KAVERI)
-		rdev->mec.num_mec = 2;
-	else
-		rdev->mec.num_mec = 1;
-	rdev->mec.num_pipe = 4;
+	rdev->mec.num_mec = 1;
+	rdev->mec.num_pipe = 1;
 	rdev->mec.num_queue = rdev->mec.num_mec * rdev->mec.num_pipe * 8;
 
 	if (rdev->mec.hpd_eop_obj == NULL) {
@@ -4809,28 +4808,24 @@  static int cik_cp_compute_resume(struct radeon_device *rdev)
 
 	/* init the pipes */
 	mutex_lock(&rdev->srbm_mutex);
-	for (i = 0; i < (rdev->mec.num_pipe * rdev->mec.num_mec); i++) {
-		int me = (i < 4) ? 1 : 2;
-		int pipe = (i < 4) ? i : (i - 4);
 
-		eop_gpu_addr = rdev->mec.hpd_eop_gpu_addr + (i * MEC_HPD_SIZE * 2);
+	eop_gpu_addr = rdev->mec.hpd_eop_gpu_addr;
 
-		cik_srbm_select(rdev, me, pipe, 0, 0);
+	cik_srbm_select(rdev, 0, 0, 0, 0);
 
-		/* write the EOP addr */
-		WREG32(CP_HPD_EOP_BASE_ADDR, eop_gpu_addr >> 8);
-		WREG32(CP_HPD_EOP_BASE_ADDR_HI, upper_32_bits(eop_gpu_addr) >> 8);
+	/* write the EOP addr */
+	WREG32(CP_HPD_EOP_BASE_ADDR, eop_gpu_addr >> 8);
+	WREG32(CP_HPD_EOP_BASE_ADDR_HI, upper_32_bits(eop_gpu_addr) >> 8);
 
-		/* set the VMID assigned */
-		WREG32(CP_HPD_EOP_VMID, 0);
+	/* set the VMID assigned */
+	WREG32(CP_HPD_EOP_VMID, 0);
+
+	/* set the EOP size, register value is 2^(EOP_SIZE+1) dwords */
+	tmp = RREG32(CP_HPD_EOP_CONTROL);
+	tmp &= ~EOP_SIZE_MASK;
+	tmp |= order_base_2(MEC_HPD_SIZE / 8);
+	WREG32(CP_HPD_EOP_CONTROL, tmp);
 
-		/* set the EOP size, register value is 2^(EOP_SIZE+1) dwords */
-		tmp = RREG32(CP_HPD_EOP_CONTROL);
-		tmp &= ~EOP_SIZE_MASK;
-		tmp |= order_base_2(MEC_HPD_SIZE / 8);
-		WREG32(CP_HPD_EOP_CONTROL, tmp);
-	}
-	cik_srbm_select(rdev, 0, 0, 0, 0);
 	mutex_unlock(&rdev->srbm_mutex);
 
 	/* init the queues.  Just two for now. */
@@ -5876,8 +5871,13 @@  int cik_ib_parse(struct radeon_device *rdev, struct radeon_ib *ib)
  */
 int cik_vm_init(struct radeon_device *rdev)
 {
-	/* number of VMs */
-	rdev->vm_manager.nvm = 16;
+	/*
+	 * number of VMs
+	 * VMID 0 is reserved for Graphics
+	 * radeon compute will use VMIDs 1-7
+	 * KFD will use VMIDs 8-15
+	 */
+	rdev->vm_manager.nvm = 8;
 	/* base offset of vram pages */
 	if (rdev->flags & RADEON_IS_IGP) {
 		u64 tmp = RREG32(MC_VM_FB_OFFSET);