subject:"\[PATCH 1\/1\] drm\/amdkfd\: Do not ignore requested queue size during allocation"

Re: [PATCH 1/1] drm/amdkfd: Do not ignore requested queue size during allocation

2017-12-01 Thread Felix Kuehling


On 2017-11-30 06:51 PM, Jan Vesely wrote:
>
> It's not a userspace queue that stops. I'm using kernel dbgdev to issue
> wave_resume commands. (waves are halted after executing
> s_sendmsg_halt).
> I bumped KFD_KERNEL_QUEUE_SIZE to 16KB to make sure all 320 resume
> commads fit (otherwise I get spurious ENOMEM when the queue is full but
> still advancing).

Sorry, didn't see this part of your message before.

To see the actual state of the DIQ in the hardware, you should look at
the HQD. You can find the matching HQD by looking at the queue base
address (cp_hqd_pq_base) which is at offset 0x220 in the MQD and offset
0xc934 in the register space (HQD).

I've debugged some obscure CP hangs involving the DIQ and wave control
commands before, that required help from the firmware team. The fix was
to remove synchronization with release_mem packets that could hang in
combination with wave control. It turned out the synchronization wasn't
really needed anyway. But it had some implications for how memory was
managed. I had to add code to allocate the IB on the queue (using a NOP
command), so I wouldn't have to free it explicitly (which would require
synchronization). I think that code is still not 100% correct. When the
queue is nearly full, an IB may get overwritten. I'd have to restructure
the code to allocate the IB after the commands that submit the IB, so
that the IB can't get overwritten until after the IB execution is finished.

Regards,
  Felix

>
> thanks,
> Jan
>
>> Regards,
>>   Felix
>>
>>
>> On 2017-11-29 04:43 PM, Jan Vesely wrote:
>>> On Mon, 2017-11-20 at 14:22 -0500, Felix Kuehling wrote:
 I think this patch is not correct. The EOP-mem is not associated with
 the queue size. The EOP buffer is a separate buffer used by the firmware
 to handle command completion. As I understand it, this allows more
 concurrency, while still making it look like all commands in the queue
 are completing in order.
>>> thanks for the explanation. I was looking for a source of a CP hang
>>> (rptr stops advancing), but bumping the eop size actually mode things
>>> worse. Is there a way to find out if a queue got disabled and for what
>>> reason? (I'm running ROCK-1.6.x based kernel)
>>>
>>> thanks,
>>> Jan
>>>
 Regards,
   Felix


 On 2017-11-19 03:19 AM, Oded Gabbay wrote:
> On Thu, Nov 16, 2017 at 11:36 PM, Jan Vesely  
> wrote:
>> Signed-off-by: Jan Vesely 
>> ---
>>  drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c 
>> b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
>> index f1d48281e322..b3bee39661ab 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
>> @@ -37,15 +37,16 @@ static bool initialize_vi(struct kernel_queue *kq, 
>> struct kfd_dev *dev,
>> enum kfd_queue_type type, unsigned int 
>> queue_size)
>>  {
>> int retval;
>> +   unsigned int size = ALIGN(queue_size, PAGE_SIZE);
>>
>> -   retval = kfd_gtt_sa_allocate(dev, PAGE_SIZE, >eop_mem);
>> +   retval = kfd_gtt_sa_allocate(dev, size, >eop_mem);
>> if (retval != 0)
>> return false;
>>
>> kq->eop_gpu_addr = kq->eop_mem->gpu_addr;
>> kq->eop_kernel_addr = kq->eop_mem->cpu_ptr;
>>
>> -   memset(kq->eop_kernel_addr, 0, PAGE_SIZE);
>> +   memset(kq->eop_kernel_addr, 0, size);
>>
>> return true;
>>  }
>> --
>> 2.13.6
>>
>> ___
>> amd-gfx mailing list
>> amd-...@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> Thanks!
> Applied to -next tree
> Oded
> ___
> amd-gfx mailing list
> amd-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/1] drm/amdkfd: Do not ignore requested queue size during allocation

2017-12-01 Thread Felix Kuehling

DIQ is the debug interface queue. Are you running a GPU debugger?
Otherwise I would not expect to even see a DIQ.

Are you not seeing any compute queues in mqds? If there are no compute
queues in mqds, that means your queue has been destroyed. That would
explain why the read pointer is not advancing.

Regards,
  Felix


On 2017-11-30 06:51 PM, Jan Vesely wrote:
> On Wed, 2017-11-29 at 16:58 -0500, Felix Kuehling wrote:
>> You can see the state of the queues in debugfs:
>> /sys/kernel/debug/kfd/... You can look at MQDs and HQDs.
> thanks. how do I decode the information?
> The rptr always stops at pos 60 which looks like this in mqds:
>
>  DIQ on device 45a2
> : c0310800 4000      
> 
> 0020:    0001    
> 
> 0040:        
> 
> 0060:        
> 
>
> If I understood correctly that's the queue dump, so those fs look
> wrong
>
>> If your application isn't stopping queues deliberately, queues get
>> disabled by evictions, usually temporarily. You'll see kernel messages
>> when that happens.
>>
>> A VM fault will result in queues of the offending process getting
>> disabled permanently. Again, you'll see messages about that in the
>> kernel log.
>>
>> The RPTR can also stop advancing if you have an infinite loop in a
>> shader program, or just a shader that takes a very long time to execute.
>> Or maybe if you have some dependencies (barriers) in your AQL packets
>> that never get satisfied.
>>
>> The function you changed only affects the HIQ, the queue that KFD uses
>> to control the HWS. It does not affect user mode queues. If your problem
>> is with a user mode queue, your change should have no effect at all.
> It's not a userspace queue that stops. I'm using kernel dbgdev to issue
> wave_resume commands. (waves are halted after executing
> s_sendmsg_halt).
> I bumped KFD_KERNEL_QUEUE_SIZE to 16KB to make sure all 320 resume
> commads fit (otherwise I get spurious ENOMEM when the queue is full but
> still advancing).
>
> thanks,
> Jan
>
>> Regards,
>>   Felix
>>
>>
>> On 2017-11-29 04:43 PM, Jan Vesely wrote:
>>> On Mon, 2017-11-20 at 14:22 -0500, Felix Kuehling wrote:
 I think this patch is not correct. The EOP-mem is not associated with
 the queue size. The EOP buffer is a separate buffer used by the firmware
 to handle command completion. As I understand it, this allows more
 concurrency, while still making it look like all commands in the queue
 are completing in order.
>>> thanks for the explanation. I was looking for a source of a CP hang
>>> (rptr stops advancing), but bumping the eop size actually mode things
>>> worse. Is there a way to find out if a queue got disabled and for what
>>> reason? (I'm running ROCK-1.6.x based kernel)
>>>
>>> thanks,
>>> Jan
>>>
 Regards,
   Felix


 On 2017-11-19 03:19 AM, Oded Gabbay wrote:
> On Thu, Nov 16, 2017 at 11:36 PM, Jan Vesely  
> wrote:
>> Signed-off-by: Jan Vesely 
>> ---
>>  drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c 
>> b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
>> index f1d48281e322..b3bee39661ab 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
>> @@ -37,15 +37,16 @@ static bool initialize_vi(struct kernel_queue *kq, 
>> struct kfd_dev *dev,
>> enum kfd_queue_type type, unsigned int 
>> queue_size)
>>  {
>> int retval;
>> +   unsigned int size = ALIGN(queue_size, PAGE_SIZE);
>>
>> -   retval = kfd_gtt_sa_allocate(dev, PAGE_SIZE, >eop_mem);
>> +   retval = kfd_gtt_sa_allocate(dev, size, >eop_mem);
>> if (retval != 0)
>> return false;
>>
>> kq->eop_gpu_addr = kq->eop_mem->gpu_addr;
>> kq->eop_kernel_addr = kq->eop_mem->cpu_ptr;
>>
>> -   memset(kq->eop_kernel_addr, 0, PAGE_SIZE);
>> +   memset(kq->eop_kernel_addr, 0, size);
>>
>> return true;
>>  }
>> --
>> 2.13.6
>>
>> ___
>> amd-gfx mailing list
>> amd-...@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> Thanks!
> Applied to -next tree
> Oded
> ___
> amd-gfx mailing list
> amd-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>

___
dri-devel mailing

Re: [PATCH 1/1] drm/amdkfd: Do not ignore requested queue size during allocation

2017-12-01 Thread Felix Kuehling

To answer your questions about decoding MQDs, take a look at struct
vi_mqd in drivers/gpu/drm/amd/include/vi_structs.h. What you're looking
at is a binary dump of that structure, one per queue.

The information in the MQD may not always be up to date, because the MQD
represents an unmapped queue. It mostly gets updated when queues are
unmapped. So you would need to correlate the MQD of the queue you're
interested in with an HQD to see the current HW state.

Regards,
  Felix


On 2017-11-30 06:51 PM, Jan Vesely wrote:
> On Wed, 2017-11-29 at 16:58 -0500, Felix Kuehling wrote:
>> You can see the state of the queues in debugfs:
>> /sys/kernel/debug/kfd/... You can look at MQDs and HQDs.
> thanks. how do I decode the information?
> The rptr always stops at pos 60 which looks like this in mqds:
>
>  DIQ on device 45a2
> : c0310800 4000      
> 
> 0020:    0001    
> 
> 0040:        
> 
> 0060:        
> 
>
> If I understood correctly that's the queue dump, so those fs look
> wrong
>
>> If your application isn't stopping queues deliberately, queues get
>> disabled by evictions, usually temporarily. You'll see kernel messages
>> when that happens.
>>
>> A VM fault will result in queues of the offending process getting
>> disabled permanently. Again, you'll see messages about that in the
>> kernel log.
>>
>> The RPTR can also stop advancing if you have an infinite loop in a
>> shader program, or just a shader that takes a very long time to execute.
>> Or maybe if you have some dependencies (barriers) in your AQL packets
>> that never get satisfied.
>>
>> The function you changed only affects the HIQ, the queue that KFD uses
>> to control the HWS. It does not affect user mode queues. If your problem
>> is with a user mode queue, your change should have no effect at all.
> It's not a userspace queue that stops. I'm using kernel dbgdev to issue
> wave_resume commands. (waves are halted after executing
> s_sendmsg_halt).
> I bumped KFD_KERNEL_QUEUE_SIZE to 16KB to make sure all 320 resume
> commads fit (otherwise I get spurious ENOMEM when the queue is full but
> still advancing).
>
> thanks,
> Jan
>
>> Regards,
>>   Felix
>>
>>
>> On 2017-11-29 04:43 PM, Jan Vesely wrote:
>>> On Mon, 2017-11-20 at 14:22 -0500, Felix Kuehling wrote:
 I think this patch is not correct. The EOP-mem is not associated with
 the queue size. The EOP buffer is a separate buffer used by the firmware
 to handle command completion. As I understand it, this allows more
 concurrency, while still making it look like all commands in the queue
 are completing in order.
>>> thanks for the explanation. I was looking for a source of a CP hang
>>> (rptr stops advancing), but bumping the eop size actually mode things
>>> worse. Is there a way to find out if a queue got disabled and for what
>>> reason? (I'm running ROCK-1.6.x based kernel)
>>>
>>> thanks,
>>> Jan
>>>
 Regards,
   Felix


 On 2017-11-19 03:19 AM, Oded Gabbay wrote:
> On Thu, Nov 16, 2017 at 11:36 PM, Jan Vesely  
> wrote:
>> Signed-off-by: Jan Vesely 
>> ---
>>  drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c 
>> b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
>> index f1d48281e322..b3bee39661ab 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
>> @@ -37,15 +37,16 @@ static bool initialize_vi(struct kernel_queue *kq, 
>> struct kfd_dev *dev,
>> enum kfd_queue_type type, unsigned int 
>> queue_size)
>>  {
>> int retval;
>> +   unsigned int size = ALIGN(queue_size, PAGE_SIZE);
>>
>> -   retval = kfd_gtt_sa_allocate(dev, PAGE_SIZE, >eop_mem);
>> +   retval = kfd_gtt_sa_allocate(dev, size, >eop_mem);
>> if (retval != 0)
>> return false;
>>
>> kq->eop_gpu_addr = kq->eop_mem->gpu_addr;
>> kq->eop_kernel_addr = kq->eop_mem->cpu_ptr;
>>
>> -   memset(kq->eop_kernel_addr, 0, PAGE_SIZE);
>> +   memset(kq->eop_kernel_addr, 0, size);
>>
>> return true;
>>  }
>> --
>> 2.13.6
>>
>> ___
>> amd-gfx mailing list
>> amd-...@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> Thanks!
> Applied to -next tree
> Oded
> ___
> amd-gfx mailing list
>

Re: [PATCH 1/1] drm/amdkfd: Do not ignore requested queue size during allocation

2017-11-30 Thread Jan Vesely

On Wed, 2017-11-29 at 16:58 -0500, Felix Kuehling wrote:
> You can see the state of the queues in debugfs:
> /sys/kernel/debug/kfd/... You can look at MQDs and HQDs.

thanks. how do I decode the information?
The rptr always stops at pos 60 which looks like this in mqds:

 DIQ on device 45a2
: c0310800 4000      

0020:    0001    

0040:        

0060:        


If I understood correctly that's the queue dump, so those fs look
wrong

> 
> If your application isn't stopping queues deliberately, queues get
> disabled by evictions, usually temporarily. You'll see kernel messages
> when that happens.
> 
> A VM fault will result in queues of the offending process getting
> disabled permanently. Again, you'll see messages about that in the
> kernel log.
> 
> The RPTR can also stop advancing if you have an infinite loop in a
> shader program, or just a shader that takes a very long time to execute.
> Or maybe if you have some dependencies (barriers) in your AQL packets
> that never get satisfied.
> 
> The function you changed only affects the HIQ, the queue that KFD uses
> to control the HWS. It does not affect user mode queues. If your problem
> is with a user mode queue, your change should have no effect at all.

It's not a userspace queue that stops. I'm using kernel dbgdev to issue
wave_resume commands. (waves are halted after executing
s_sendmsg_halt).
I bumped KFD_KERNEL_QUEUE_SIZE to 16KB to make sure all 320 resume
commads fit (otherwise I get spurious ENOMEM when the queue is full but
still advancing).

thanks,
Jan

> 
> Regards,
>   Felix
> 
> 
> On 2017-11-29 04:43 PM, Jan Vesely wrote:
> > On Mon, 2017-11-20 at 14:22 -0500, Felix Kuehling wrote:
> > > I think this patch is not correct. The EOP-mem is not associated with
> > > the queue size. The EOP buffer is a separate buffer used by the firmware
> > > to handle command completion. As I understand it, this allows more
> > > concurrency, while still making it look like all commands in the queue
> > > are completing in order.
> > 
> > thanks for the explanation. I was looking for a source of a CP hang
> > (rptr stops advancing), but bumping the eop size actually mode things
> > worse. Is there a way to find out if a queue got disabled and for what
> > reason? (I'm running ROCK-1.6.x based kernel)
> > 
> > thanks,
> > Jan
> > 
> > > Regards,
> > >   Felix
> > > 
> > > 
> > > On 2017-11-19 03:19 AM, Oded Gabbay wrote:
> > > > On Thu, Nov 16, 2017 at 11:36 PM, Jan Vesely  
> > > > wrote:
> > > > > Signed-off-by: Jan Vesely 
> > > > > ---
> > > > >  drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c | 5 +++--
> > > > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c 
> > > > > b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
> > > > > index f1d48281e322..b3bee39661ab 100644
> > > > > --- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
> > > > > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
> > > > > @@ -37,15 +37,16 @@ static bool initialize_vi(struct kernel_queue 
> > > > > *kq, struct kfd_dev *dev,
> > > > > enum kfd_queue_type type, unsigned int 
> > > > > queue_size)
> > > > >  {
> > > > > int retval;
> > > > > +   unsigned int size = ALIGN(queue_size, PAGE_SIZE);
> > > > > 
> > > > > -   retval = kfd_gtt_sa_allocate(dev, PAGE_SIZE, >eop_mem);
> > > > > +   retval = kfd_gtt_sa_allocate(dev, size, >eop_mem);
> > > > > if (retval != 0)
> > > > > return false;
> > > > > 
> > > > > kq->eop_gpu_addr = kq->eop_mem->gpu_addr;
> > > > > kq->eop_kernel_addr = kq->eop_mem->cpu_ptr;
> > > > > 
> > > > > -   memset(kq->eop_kernel_addr, 0, PAGE_SIZE);
> > > > > +   memset(kq->eop_kernel_addr, 0, size);
> > > > > 
> > > > > return true;
> > > > >  }
> > > > > --
> > > > > 2.13.6
> > > > > 
> > > > > ___
> > > > > amd-gfx mailing list
> > > > > amd-...@lists.freedesktop.org
> > > > > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> > > > 
> > > > Thanks!
> > > > Applied to -next tree
> > > > Oded
> > > > ___
> > > > amd-gfx mailing list
> > > > amd-...@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> 
> 


signature.asc
Description: This is a digitally signed message part
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/1] drm/amdkfd: Do not ignore requested queue size during allocation

2017-11-29 Thread Felix Kuehling

You can see the state of the queues in debugfs:
/sys/kernel/debug/kfd/... You can look at MQDs and HQDs.

If your application isn't stopping queues deliberately, queues get
disabled by evictions, usually temporarily. You'll see kernel messages
when that happens.

A VM fault will result in queues of the offending process getting
disabled permanently. Again, you'll see messages about that in the
kernel log.

The RPTR can also stop advancing if you have an infinite loop in a
shader program, or just a shader that takes a very long time to execute.
Or maybe if you have some dependencies (barriers) in your AQL packets
that never get satisfied.

The function you changed only affects the HIQ, the queue that KFD uses
to control the HWS. It does not affect user mode queues. If your problem
is with a user mode queue, your change should have no effect at all.

Regards,
  Felix


On 2017-11-29 04:43 PM, Jan Vesely wrote:
> On Mon, 2017-11-20 at 14:22 -0500, Felix Kuehling wrote:
>> I think this patch is not correct. The EOP-mem is not associated with
>> the queue size. The EOP buffer is a separate buffer used by the firmware
>> to handle command completion. As I understand it, this allows more
>> concurrency, while still making it look like all commands in the queue
>> are completing in order.
> thanks for the explanation. I was looking for a source of a CP hang
> (rptr stops advancing), but bumping the eop size actually mode things
> worse. Is there a way to find out if a queue got disabled and for what
> reason? (I'm running ROCK-1.6.x based kernel)
>
> thanks,
> Jan
>
>> Regards,
>>   Felix
>>
>>
>> On 2017-11-19 03:19 AM, Oded Gabbay wrote:
>>> On Thu, Nov 16, 2017 at 11:36 PM, Jan Vesely  wrote:
 Signed-off-by: Jan Vesely 
 ---
  drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

 diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c 
 b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
 index f1d48281e322..b3bee39661ab 100644
 --- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
 +++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
 @@ -37,15 +37,16 @@ static bool initialize_vi(struct kernel_queue *kq, 
 struct kfd_dev *dev,
 enum kfd_queue_type type, unsigned int queue_size)
  {
 int retval;
 +   unsigned int size = ALIGN(queue_size, PAGE_SIZE);

 -   retval = kfd_gtt_sa_allocate(dev, PAGE_SIZE, >eop_mem);
 +   retval = kfd_gtt_sa_allocate(dev, size, >eop_mem);
 if (retval != 0)
 return false;

 kq->eop_gpu_addr = kq->eop_mem->gpu_addr;
 kq->eop_kernel_addr = kq->eop_mem->cpu_ptr;

 -   memset(kq->eop_kernel_addr, 0, PAGE_SIZE);
 +   memset(kq->eop_kernel_addr, 0, size);

 return true;
  }
 --
 2.13.6

 ___
 amd-gfx mailing list
 amd-...@lists.freedesktop.org
 https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>> Thanks!
>>> Applied to -next tree
>>> Oded
>>> ___
>>> amd-gfx mailing list
>>> amd-...@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/1] drm/amdkfd: Do not ignore requested queue size during allocation

2017-11-29 Thread Jan Vesely

On Mon, 2017-11-20 at 14:22 -0500, Felix Kuehling wrote:
> I think this patch is not correct. The EOP-mem is not associated with
> the queue size. The EOP buffer is a separate buffer used by the firmware
> to handle command completion. As I understand it, this allows more
> concurrency, while still making it look like all commands in the queue
> are completing in order.

thanks for the explanation. I was looking for a source of a CP hang
(rptr stops advancing), but bumping the eop size actually mode things
worse. Is there a way to find out if a queue got disabled and for what
reason? (I'm running ROCK-1.6.x based kernel)

thanks,
Jan

> 
> Regards,
>   Felix
> 
> 
> On 2017-11-19 03:19 AM, Oded Gabbay wrote:
> > On Thu, Nov 16, 2017 at 11:36 PM, Jan Vesely  wrote:
> > > Signed-off-by: Jan Vesely 
> > > ---
> > >  drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c | 5 +++--
> > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c 
> > > b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
> > > index f1d48281e322..b3bee39661ab 100644
> > > --- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
> > > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
> > > @@ -37,15 +37,16 @@ static bool initialize_vi(struct kernel_queue *kq, 
> > > struct kfd_dev *dev,
> > > enum kfd_queue_type type, unsigned int queue_size)
> > >  {
> > > int retval;
> > > +   unsigned int size = ALIGN(queue_size, PAGE_SIZE);
> > > 
> > > -   retval = kfd_gtt_sa_allocate(dev, PAGE_SIZE, >eop_mem);
> > > +   retval = kfd_gtt_sa_allocate(dev, size, >eop_mem);
> > > if (retval != 0)
> > > return false;
> > > 
> > > kq->eop_gpu_addr = kq->eop_mem->gpu_addr;
> > > kq->eop_kernel_addr = kq->eop_mem->cpu_ptr;
> > > 
> > > -   memset(kq->eop_kernel_addr, 0, PAGE_SIZE);
> > > +   memset(kq->eop_kernel_addr, 0, size);
> > > 
> > > return true;
> > >  }
> > > --
> > > 2.13.6
> > > 
> > > ___
> > > amd-gfx mailing list
> > > amd-...@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> > 
> > Thanks!
> > Applied to -next tree
> > Oded
> > ___
> > amd-gfx mailing list
> > amd-...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> 
> 


signature.asc
Description: This is a digitally signed message part
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/1] drm/amdkfd: Do not ignore requested queue size during allocation

2017-11-21 Thread Felix Kuehling

On 2017-11-21 06:44 AM, Oded Gabbay wrote:
> Thanks Felix for catching that, For some reason I remembered  EOP
> buffer should be the same size of the queue.

The EOP queue size is hard-coded to prop.eop_ring_buffer_size =
PAGE_SIZE for kernel queues in initialize in kfd_kernel_queue.c. I'm not
too familiar with the HW/FW details. But I see this comment in
kfd_mqd_manager_vi.c:

/*
 * HW does not clamp this field correctly. Maximum EOP queue size
 * is constrained by per-SE EOP done signal count, which is 8-bit.
 * Limit is 0xFF EOP entries (= 0x7F8 dwords). CP will not submit
 * more than (EOP entry count - 1) so a queue size of 0x800 dwords
 * is safe, giving a maximum field value of 0xA.
 */

With that the maximum possible EOP queue size would be two pages,
regardless of the queue size.

> Then we can remove the queue size parameter from that function ?

Not the way the code is currently organized. Currently struct
kernel_queue_ops is shared for ASIC-independent and ASIC-specific queue
ops. The ASIC-independent initialize function in kfd_kernel_queue.c
still needs this parameter.

That said, the kernel_queue stuff could be cleaned up a bit in general.
IMO the hardware-independent functions don't really need to be called
through function pointers. The ASIC-specific function pointers don't
need to be in the kernel_queue structure, they could be in kfd_dev.

Regards,
  Felix

>
> On Mon, Nov 20, 2017 at 9:22 PM, Felix Kuehling  
> wrote:
>> I think this patch is not correct. The EOP-mem is not associated with
>> the queue size. The EOP buffer is a separate buffer used by the firmware
>> to handle command completion. As I understand it, this allows more
>> concurrency, while still making it look like all commands in the queue
>> are completing in order.
>>
>> Regards,
>>   Felix
>>
>>
>> On 2017-11-19 03:19 AM, Oded Gabbay wrote:
>>> On Thu, Nov 16, 2017 at 11:36 PM, Jan Vesely  wrote:
 Signed-off-by: Jan Vesely 
 ---
  drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

 diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c 
 b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
 index f1d48281e322..b3bee39661ab 100644
 --- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
 +++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
 @@ -37,15 +37,16 @@ static bool initialize_vi(struct kernel_queue *kq, 
 struct kfd_dev *dev,
 enum kfd_queue_type type, unsigned int queue_size)
  {
 int retval;
 +   unsigned int size = ALIGN(queue_size, PAGE_SIZE);

 -   retval = kfd_gtt_sa_allocate(dev, PAGE_SIZE, >eop_mem);
 +   retval = kfd_gtt_sa_allocate(dev, size, >eop_mem);
 if (retval != 0)
 return false;

 kq->eop_gpu_addr = kq->eop_mem->gpu_addr;
 kq->eop_kernel_addr = kq->eop_mem->cpu_ptr;

 -   memset(kq->eop_kernel_addr, 0, PAGE_SIZE);
 +   memset(kq->eop_kernel_addr, 0, size);

 return true;
  }
 --
 2.13.6

 ___
 amd-gfx mailing list
 amd-...@lists.freedesktop.org
 https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>> Thanks!
>>> Applied to -next tree
>>> Oded
>>> ___
>>> amd-gfx mailing list
>>> amd-...@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/1] drm/amdkfd: Do not ignore requested queue size during allocation

2017-11-21 Thread Oded Gabbay

Thanks Felix for catching that, For some reason I remembered  EOP
buffer should be the same size of the queue.
Then we can remove the queue size parameter from that function ?

On Mon, Nov 20, 2017 at 9:22 PM, Felix Kuehling  wrote:
> I think this patch is not correct. The EOP-mem is not associated with
> the queue size. The EOP buffer is a separate buffer used by the firmware
> to handle command completion. As I understand it, this allows more
> concurrency, while still making it look like all commands in the queue
> are completing in order.
>
> Regards,
>   Felix
>
>
> On 2017-11-19 03:19 AM, Oded Gabbay wrote:
>> On Thu, Nov 16, 2017 at 11:36 PM, Jan Vesely  wrote:
>>> Signed-off-by: Jan Vesely 
>>> ---
>>>  drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c | 5 +++--
>>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c 
>>> b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
>>> index f1d48281e322..b3bee39661ab 100644
>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
>>> @@ -37,15 +37,16 @@ static bool initialize_vi(struct kernel_queue *kq, 
>>> struct kfd_dev *dev,
>>> enum kfd_queue_type type, unsigned int queue_size)
>>>  {
>>> int retval;
>>> +   unsigned int size = ALIGN(queue_size, PAGE_SIZE);
>>>
>>> -   retval = kfd_gtt_sa_allocate(dev, PAGE_SIZE, >eop_mem);
>>> +   retval = kfd_gtt_sa_allocate(dev, size, >eop_mem);
>>> if (retval != 0)
>>> return false;
>>>
>>> kq->eop_gpu_addr = kq->eop_mem->gpu_addr;
>>> kq->eop_kernel_addr = kq->eop_mem->cpu_ptr;
>>>
>>> -   memset(kq->eop_kernel_addr, 0, PAGE_SIZE);
>>> +   memset(kq->eop_kernel_addr, 0, size);
>>>
>>> return true;
>>>  }
>>> --
>>> 2.13.6
>>>
>>> ___
>>> amd-gfx mailing list
>>> amd-...@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>> Thanks!
>> Applied to -next tree
>> Oded
>> ___
>> amd-gfx mailing list
>> amd-...@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/1] drm/amdkfd: Do not ignore requested queue size during allocation

2017-11-20 Thread Felix Kuehling

I think this patch is not correct. The EOP-mem is not associated with
the queue size. The EOP buffer is a separate buffer used by the firmware
to handle command completion. As I understand it, this allows more
concurrency, while still making it look like all commands in the queue
are completing in order.

Regards,
  Felix


On 2017-11-19 03:19 AM, Oded Gabbay wrote:
> On Thu, Nov 16, 2017 at 11:36 PM, Jan Vesely  wrote:
>> Signed-off-by: Jan Vesely 
>> ---
>>  drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c 
>> b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
>> index f1d48281e322..b3bee39661ab 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
>> @@ -37,15 +37,16 @@ static bool initialize_vi(struct kernel_queue *kq, 
>> struct kfd_dev *dev,
>> enum kfd_queue_type type, unsigned int queue_size)
>>  {
>> int retval;
>> +   unsigned int size = ALIGN(queue_size, PAGE_SIZE);
>>
>> -   retval = kfd_gtt_sa_allocate(dev, PAGE_SIZE, >eop_mem);
>> +   retval = kfd_gtt_sa_allocate(dev, size, >eop_mem);
>> if (retval != 0)
>> return false;
>>
>> kq->eop_gpu_addr = kq->eop_mem->gpu_addr;
>> kq->eop_kernel_addr = kq->eop_mem->cpu_ptr;
>>
>> -   memset(kq->eop_kernel_addr, 0, PAGE_SIZE);
>> +   memset(kq->eop_kernel_addr, 0, size);
>>
>> return true;
>>  }
>> --
>> 2.13.6
>>
>> ___
>> amd-gfx mailing list
>> amd-...@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> Thanks!
> Applied to -next tree
> Oded
> ___
> amd-gfx mailing list
> amd-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/1] drm/amdkfd: Do not ignore requested queue size during allocation

2017-11-19 Thread Oded Gabbay

On Thu, Nov 16, 2017 at 11:36 PM, Jan Vesely  wrote:
> Signed-off-by: Jan Vesely 
> ---
>  drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c 
> b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
> index f1d48281e322..b3bee39661ab 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
> @@ -37,15 +37,16 @@ static bool initialize_vi(struct kernel_queue *kq, struct 
> kfd_dev *dev,
> enum kfd_queue_type type, unsigned int queue_size)
>  {
> int retval;
> +   unsigned int size = ALIGN(queue_size, PAGE_SIZE);
>
> -   retval = kfd_gtt_sa_allocate(dev, PAGE_SIZE, >eop_mem);
> +   retval = kfd_gtt_sa_allocate(dev, size, >eop_mem);
> if (retval != 0)
> return false;
>
> kq->eop_gpu_addr = kq->eop_mem->gpu_addr;
> kq->eop_kernel_addr = kq->eop_mem->cpu_ptr;
>
> -   memset(kq->eop_kernel_addr, 0, PAGE_SIZE);
> +   memset(kq->eop_kernel_addr, 0, size);
>
> return true;
>  }
> --
> 2.13.6
>
> ___
> amd-gfx mailing list
> amd-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Thanks!
Applied to -next tree
Oded
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/1] drm/amdkfd: Do not ignore requested queue size during allocation

2017-11-16 Thread Jan Vesely

Signed-off-by: Jan Vesely 
---
 drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
index f1d48281e322..b3bee39661ab 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_vi.c
@@ -37,15 +37,16 @@ static bool initialize_vi(struct kernel_queue *kq, struct 
kfd_dev *dev,
enum kfd_queue_type type, unsigned int queue_size)
 {
int retval;
+   unsigned int size = ALIGN(queue_size, PAGE_SIZE);
 
-   retval = kfd_gtt_sa_allocate(dev, PAGE_SIZE, >eop_mem);
+   retval = kfd_gtt_sa_allocate(dev, size, >eop_mem);
if (retval != 0)
return false;
 
kq->eop_gpu_addr = kq->eop_mem->gpu_addr;
kq->eop_kernel_addr = kq->eop_mem->cpu_ptr;
 
-   memset(kq->eop_kernel_addr, 0, PAGE_SIZE);
+   memset(kq->eop_kernel_addr, 0, size);
 
return true;
 }
-- 
2.13.6

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/1] drm/amdkfd: Do not ignore requested queue size during allocation

Re: [PATCH 1/1] drm/amdkfd: Do not ignore requested queue size during allocation

Re: [PATCH 1/1] drm/amdkfd: Do not ignore requested queue size during allocation

Re: [PATCH 1/1] drm/amdkfd: Do not ignore requested queue size during allocation

Re: [PATCH 1/1] drm/amdkfd: Do not ignore requested queue size during allocation

Re: [PATCH 1/1] drm/amdkfd: Do not ignore requested queue size during allocation

Re: [PATCH 1/1] drm/amdkfd: Do not ignore requested queue size during allocation

Re: [PATCH 1/1] drm/amdkfd: Do not ignore requested queue size during allocation

Re: [PATCH 1/1] drm/amdkfd: Do not ignore requested queue size during allocation

Re: [PATCH 1/1] drm/amdkfd: Do not ignore requested queue size during allocation

[PATCH 1/1] drm/amdkfd: Do not ignore requested queue size during allocation

11 matches

Site Navigation

Mail list logo

Footer information