[PATCH 1/2] drm/amdgpu: track MQD size for gfx and compute

[PATCH 1/2] drm/amdgpu: track MQD size for gfx and compute

[Alex Deucher]

It varies by generation and we need to know the size
to expose this via debugfs.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c  | 2 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 1 +
 2 files changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
index c50d59855011..5435f41a3b7f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
@@ -404,6 +404,7 @@ int amdgpu_gfx_mqd_sw_init(struct amdgpu_device *adev,
 					return r;
 				}
 
+				ring->mqd_size = mqd_size;
 				/* prepare MQD backup */
 				adev->gfx.me.mqd_backup[i] = kmalloc(mqd_size, GFP_KERNEL);
 				if (!adev->gfx.me.mqd_backup[i])
@@ -424,6 +425,7 @@ int amdgpu_gfx_mqd_sw_init(struct amdgpu_device *adev,
 				return r;
 			}
 
+			ring->mqd_size = mqd_size;
 			/* prepare MQD backup */
 			adev->gfx.mec.mqd_backup[i] = kmalloc(mqd_size, GFP_KERNEL);
 			if (!adev->gfx.mec.mqd_backup[i])
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
index 7942cb62e52c..deb9f7bead02 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
@@ -257,6 +257,7 @@ struct amdgpu_ring {
 	struct amdgpu_bo	*mqd_obj;
 	uint64_t                mqd_gpu_addr;
 	void                    *mqd_ptr;
+	unsigned                mqd_size;
 	uint64_t                eop_gpu_addr;
 	u32			doorbell_index;
 	bool			use_doorbell;
-- 
2.39.2

[PATCH 2/2] drm/amdgpu: add debugfs interface for reading MQDs

[Alex Deucher]

Provide a debugfs interface to access the MQD.  Useful for
debugging issues with the CP and MES hardware scheduler.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 60 +++++++++++++++++++++++-
 1 file changed, 59 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
index dc474b809604..4da67faef668 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
@@ -478,6 +478,59 @@ static const struct file_operations amdgpu_debugfs_ring_fops = {
 	.llseek = default_llseek
 };
 
+static ssize_t amdgpu_debugfs_mqd_read(struct file *f, char __user *buf,
+				       size_t size, loff_t *pos)
+{
+	struct amdgpu_ring *ring = file_inode(f)->i_private;
+	volatile u32 *mqd;
+	int r;
+	uint32_t value, result;
+
+	if (*pos & 3 || size & 3)
+		return -EINVAL;
+
+	result = 0;
+
+	r = amdgpu_bo_reserve(ring->mqd_obj, false);
+	if (unlikely(r != 0))
+		return r;
+
+	r = amdgpu_bo_kmap(ring->mqd_obj, (void **)&mqd);
+	if (r) {
+		amdgpu_bo_unreserve(ring->mqd_obj);
+		return r;
+	}
+
+	while (size) {
+		if (*pos >= ring->mqd_size)
+			return result;
+
+		value = mqd[*pos/4];
+		r = put_user(value, (uint32_t *)buf);
+		if (r)
+			goto done;
+		buf += 4;
+		result += 4;
+		size -= 4;
+		*pos += 4;
+	}
+
+done:
+	amdgpu_bo_kunmap(ring->mqd_obj);
+	mqd = NULL;
+	amdgpu_bo_unreserve(ring->mqd_obj);
+	if (r)
+		return r;
+
+	return result;
+}
+
+static const struct file_operations amdgpu_debugfs_mqd_fops = {
+	.owner = THIS_MODULE,
+	.read = amdgpu_debugfs_mqd_read,
+	.llseek = default_llseek
+};
+
 #endif
 
 void amdgpu_debugfs_ring_init(struct amdgpu_device *adev,
@@ -492,7 +545,12 @@ void amdgpu_debugfs_ring_init(struct amdgpu_device *adev,
 	debugfs_create_file_size(name, S_IFREG | S_IRUGO, root, ring,
 				 &amdgpu_debugfs_ring_fops,
 				 ring->ring_size + 12);
-
+	if (ring->mqd_obj) {
+		sprintf(name, "amdgpu_mqd_%s", ring->name);
+		debugfs_create_file_size(name, S_IFREG | S_IRUGO, root, ring,
+					 &amdgpu_debugfs_mqd_fops,
+					 ring->mqd_size);
+	}
 #endif
 }
 
-- 
2.39.2

Re: [PATCH 1/2] drm/amdgpu: track MQD size for gfx and compute

[Christian König]

Am 21.03.23 um 20:39 schrieb Alex Deucher:
> It varies by generation and we need to know the size
> to expose this via debugfs.

I suspect we can't just use the BO size for this?

If yes the series is Reviewed-by: Christian König <christian.koenig@amd.com>

>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c  | 2 ++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 1 +
>   2 files changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> index c50d59855011..5435f41a3b7f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> @@ -404,6 +404,7 @@ int amdgpu_gfx_mqd_sw_init(struct amdgpu_device *adev,
>   					return r;
>   				}
>   
> +				ring->mqd_size = mqd_size;
>   				/* prepare MQD backup */
>   				adev->gfx.me.mqd_backup[i] = kmalloc(mqd_size, GFP_KERNEL);
>   				if (!adev->gfx.me.mqd_backup[i])
> @@ -424,6 +425,7 @@ int amdgpu_gfx_mqd_sw_init(struct amdgpu_device *adev,
>   				return r;
>   			}
>   
> +			ring->mqd_size = mqd_size;
>   			/* prepare MQD backup */
>   			adev->gfx.mec.mqd_backup[i] = kmalloc(mqd_size, GFP_KERNEL);
>   			if (!adev->gfx.mec.mqd_backup[i])
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> index 7942cb62e52c..deb9f7bead02 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> @@ -257,6 +257,7 @@ struct amdgpu_ring {
>   	struct amdgpu_bo	*mqd_obj;
>   	uint64_t                mqd_gpu_addr;
>   	void                    *mqd_ptr;
> +	unsigned                mqd_size;
>   	uint64_t                eop_gpu_addr;
>   	u32			doorbell_index;
>   	bool			use_doorbell;

Re: [PATCH 1/2] drm/amdgpu: track MQD size for gfx and compute

[Alex Deucher]

On Wed, Mar 22, 2023 at 4:48 AM Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> Am 21.03.23 um 20:39 schrieb Alex Deucher:
> > It varies by generation and we need to know the size
> > to expose this via debugfs.
>
> I suspect we can't just use the BO size for this?

We could, but it may be larger than the actual MQD.  Maybe that's not
a big deal?

Alex


>
> If yes the series is Reviewed-by: Christian König <christian.koenig@amd.com>
>
> >
> > Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c  | 2 ++
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 1 +
> >   2 files changed, 3 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> > index c50d59855011..5435f41a3b7f 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> > @@ -404,6 +404,7 @@ int amdgpu_gfx_mqd_sw_init(struct amdgpu_device *adev,
> >                                       return r;
> >                               }
> >
> > +                             ring->mqd_size = mqd_size;
> >                               /* prepare MQD backup */
> >                               adev->gfx.me.mqd_backup[i] = kmalloc(mqd_size, GFP_KERNEL);
> >                               if (!adev->gfx.me.mqd_backup[i])
> > @@ -424,6 +425,7 @@ int amdgpu_gfx_mqd_sw_init(struct amdgpu_device *adev,
> >                               return r;
> >                       }
> >
> > +                     ring->mqd_size = mqd_size;
> >                       /* prepare MQD backup */
> >                       adev->gfx.mec.mqd_backup[i] = kmalloc(mqd_size, GFP_KERNEL);
> >                       if (!adev->gfx.mec.mqd_backup[i])
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> > index 7942cb62e52c..deb9f7bead02 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> > @@ -257,6 +257,7 @@ struct amdgpu_ring {
> >       struct amdgpu_bo        *mqd_obj;
> >       uint64_t                mqd_gpu_addr;
> >       void                    *mqd_ptr;
> > +     unsigned                mqd_size;
> >       uint64_t                eop_gpu_addr;
> >       u32                     doorbell_index;
> >       bool                    use_doorbell;
>

Re: [PATCH 1/2] drm/amdgpu: track MQD size for gfx and compute

[Christian König]

Am 22.03.23 um 14:26 schrieb Alex Deucher:
> On Wed, Mar 22, 2023 at 4:48 AM Christian König
> <ckoenig.leichtzumerken@gmail.com> wrote:
>> Am 21.03.23 um 20:39 schrieb Alex Deucher:
>>> It varies by generation and we need to know the size
>>> to expose this via debugfs.
>> I suspect we can't just use the BO size for this?
> We could, but it may be larger than the actual MQD.  Maybe that's not
> a big deal?

I don't really know either. Maybe just go ahead with this approach here, 
but I usually try to avoid stuff like that because it can be an 
additional source of errors when the allocation size is not correct.

Christian.

>
> Alex
>
>
>> If yes the series is Reviewed-by: Christian König <christian.koenig@amd.com>
>>
>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>> ---
>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c  | 2 ++
>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 1 +
>>>    2 files changed, 3 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>>> index c50d59855011..5435f41a3b7f 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>>> @@ -404,6 +404,7 @@ int amdgpu_gfx_mqd_sw_init(struct amdgpu_device *adev,
>>>                                        return r;
>>>                                }
>>>
>>> +                             ring->mqd_size = mqd_size;
>>>                                /* prepare MQD backup */
>>>                                adev->gfx.me.mqd_backup[i] = kmalloc(mqd_size, GFP_KERNEL);
>>>                                if (!adev->gfx.me.mqd_backup[i])
>>> @@ -424,6 +425,7 @@ int amdgpu_gfx_mqd_sw_init(struct amdgpu_device *adev,
>>>                                return r;
>>>                        }
>>>
>>> +                     ring->mqd_size = mqd_size;
>>>                        /* prepare MQD backup */
>>>                        adev->gfx.mec.mqd_backup[i] = kmalloc(mqd_size, GFP_KERNEL);
>>>                        if (!adev->gfx.mec.mqd_backup[i])
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>>> index 7942cb62e52c..deb9f7bead02 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>>> @@ -257,6 +257,7 @@ struct amdgpu_ring {
>>>        struct amdgpu_bo        *mqd_obj;
>>>        uint64_t                mqd_gpu_addr;
>>>        void                    *mqd_ptr;
>>> +     unsigned                mqd_size;
>>>        uint64_t                eop_gpu_addr;
>>>        u32                     doorbell_index;
>>>        bool                    use_doorbell;

Re: [PATCH 1/2] drm/amdgpu: track MQD size for gfx and compute

[Felix Kuehling]

MQDs are smaller than a page. The BO size will always be exactly be one 
page.

KFD can allocate MQDs with a suballocator. On some GPUs we allocate MQDs 
together with the queue's control stack in a single BO. And on some GPUs 
we allocate SDMA "MQDs" in bulk together with the HIQ MQD. So relying on 
the BO size would not work for us.

Regards,
   Felix


Am 2023-03-22 um 09:58 schrieb Christian König:
> Am 22.03.23 um 14:26 schrieb Alex Deucher:
>> On Wed, Mar 22, 2023 at 4:48 AM Christian König
>> <ckoenig.leichtzumerken@gmail.com> wrote:
>>> Am 21.03.23 um 20:39 schrieb Alex Deucher:
>>>> It varies by generation and we need to know the size
>>>> to expose this via debugfs.
>>> I suspect we can't just use the BO size for this?
>> We could, but it may be larger than the actual MQD.  Maybe that's not
>> a big deal?
>
> I don't really know either. Maybe just go ahead with this approach 
> here, but I usually try to avoid stuff like that because it can be an 
> additional source of errors when the allocation size is not correct.
>
> Christian.
>
>>
>> Alex
>>
>>
>>> If yes the series is Reviewed-by: Christian König 
>>> <christian.koenig@amd.com>
>>>
>>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>>> ---
>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c  | 2 ++
>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 1 +
>>>>    2 files changed, 3 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c 
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>>>> index c50d59855011..5435f41a3b7f 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>>>> @@ -404,6 +404,7 @@ int amdgpu_gfx_mqd_sw_init(struct amdgpu_device 
>>>> *adev,
>>>>                                        return r;
>>>>                                }
>>>>
>>>> +                             ring->mqd_size = mqd_size;
>>>>                                /* prepare MQD backup */
>>>>                                adev->gfx.me.mqd_backup[i] = 
>>>> kmalloc(mqd_size, GFP_KERNEL);
>>>>                                if (!adev->gfx.me.mqd_backup[i])
>>>> @@ -424,6 +425,7 @@ int amdgpu_gfx_mqd_sw_init(struct amdgpu_device 
>>>> *adev,
>>>>                                return r;
>>>>                        }
>>>>
>>>> +                     ring->mqd_size = mqd_size;
>>>>                        /* prepare MQD backup */
>>>>                        adev->gfx.mec.mqd_backup[i] = 
>>>> kmalloc(mqd_size, GFP_KERNEL);
>>>>                        if (!adev->gfx.mec.mqd_backup[i])
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h 
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>>>> index 7942cb62e52c..deb9f7bead02 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>>>> @@ -257,6 +257,7 @@ struct amdgpu_ring {
>>>>        struct amdgpu_bo        *mqd_obj;
>>>>        uint64_t                mqd_gpu_addr;
>>>>        void                    *mqd_ptr;
>>>> +     unsigned                mqd_size;
>>>>        uint64_t                eop_gpu_addr;
>>>>        u32                     doorbell_index;
>>>>        bool                    use_doorbell;
>