diff mbox

[1/4] drm/amd/amdgpu: Fold TTM debugfs entries into array

Message ID 20170918123528.26629-2-tom.stdenis@amd.com (mailing list archive)
State New, archived
Headers show

Commit Message

StDenis, Tom Sept. 18, 2017, 12:35 p.m. UTC 
Signed-off-by: Tom St Denis <tom.stdenis@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 53 ++++++++++++++++++---------------
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h |  4 +--
 2 files changed, 31 insertions(+), 26 deletions(-)

Comments

Christian König Sept. 18, 2017, 12:48 p.m. UTC | #1 
Am 18.09.2017 um 14:35 schrieb Tom St Denis:
> Signed-off-by: Tom St Denis <tom.stdenis@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 53 ++++++++++++++++++---------------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h |  4 +--
>   2 files changed, 31 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 8ee16dfdb8af..7848ffa99eb4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1809,6 +1809,18 @@ static const struct file_operations amdgpu_ttm_gtt_fops = {
>   
>   #endif
>   
> +
> +
> +static const struct {
> +	char *name;
> +	const struct file_operations *fops;
> +} ttm_debugfs_entries[] = {
> +	{ "amdgpu_vram", &amdgpu_ttm_vram_fops },
> +#ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS
> +	{ "amdgpu_gtt", &amdgpu_ttm_gtt_fops },
> +#endif
> +};
> +
>   #endif
>   
>   static int amdgpu_ttm_debugfs_init(struct amdgpu_device *adev)
> @@ -1819,22 +1831,21 @@ static int amdgpu_ttm_debugfs_init(struct amdgpu_device *adev)
>   	struct drm_minor *minor = adev->ddev->primary;
>   	struct dentry *ent, *root = minor->debugfs_root;
>   
> -	ent = debugfs_create_file("amdgpu_vram", S_IFREG | S_IRUGO, root,
> -				  adev, &amdgpu_ttm_vram_fops);
> -	if (IS_ERR(ent))
> -		return PTR_ERR(ent);
> -	i_size_write(ent->d_inode, adev->mc.mc_vram_size);
> -	adev->mman.vram = ent;
> -
> -#ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS
> -	ent = debugfs_create_file("amdgpu_gtt", S_IFREG | S_IRUGO, root,
> -				  adev, &amdgpu_ttm_gtt_fops);
> -	if (IS_ERR(ent))
> -		return PTR_ERR(ent);
> -	i_size_write(ent->d_inode, adev->mc.gart_size);
> -	adev->mman.gtt = ent;
> +	for (count = 0; count < ARRAY_SIZE(ttm_debugfs_entries); count++) {
> +		ent = debugfs_create_file(
> +				ttm_debugfs_entries[count].name,
> +				S_IFREG | S_IRUGO, root,
> +				adev,
> +				ttm_debugfs_entries[count].fops);
> +		if (IS_ERR(ent))
> +			return PTR_ERR(ent);
> +		if (!strcmp(ttm_debugfs_entries[count].name, "amdgpu_vram"))
> +			i_size_write(ent->d_inode, adev->mc.mc_vram_size);
> +		else if (!strcmp(ttm_debugfs_entries[count].name, "amdgpu_gtt"))
> +			i_size_write(ent->d_inode, adev->mc.gart_size);

Uff, string compare? That is screaming break me by typo.

Maybe but the domain type into the struct as well?

Apart from that looks good to me,
Christian.

> +		adev->mman.debugfs_entries[count] = ent;
> +	}
>   
> -#endif
>   	count = ARRAY_SIZE(amdgpu_ttm_debugfs_list);
>   
>   #ifdef CONFIG_SWIOTLB
> @@ -1844,7 +1855,6 @@ static int amdgpu_ttm_debugfs_init(struct amdgpu_device *adev)
>   
>   	return amdgpu_debugfs_add_files(adev, amdgpu_ttm_debugfs_list, count);
>   #else
> -
>   	return 0;
>   #endif
>   }
> @@ -1852,14 +1862,9 @@ static int amdgpu_ttm_debugfs_init(struct amdgpu_device *adev)
>   static void amdgpu_ttm_debugfs_fini(struct amdgpu_device *adev)
>   {
>   #if defined(CONFIG_DEBUG_FS)
> +	unsigned i;
>   
> -	debugfs_remove(adev->mman.vram);
> -	adev->mman.vram = NULL;
> -
> -#ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS
> -	debugfs_remove(adev->mman.gtt);
> -	adev->mman.gtt = NULL;
> -#endif
> -
> +	for (i = 0; i < ARRAY_SIZE(ttm_debugfs_entries); i++)
> +		debugfs_remove(adev->mman.debugfs_entries[i]);
>   #endif
>   }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> index 64709e041d5b..7abae6867339 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> @@ -24,6 +24,7 @@
>   #ifndef __AMDGPU_TTM_H__
>   #define __AMDGPU_TTM_H__
>   
> +#include "amdgpu.h"
>   #include "gpu_scheduler.h"
>   
>   #define AMDGPU_PL_GDS		(TTM_PL_PRIV + 0)
> @@ -45,8 +46,7 @@ struct amdgpu_mman {
>   	bool				initialized;
>   
>   #if defined(CONFIG_DEBUG_FS)
> -	struct dentry			*vram;
> -	struct dentry			*gtt;
> +	struct dentry			*debugfs_entries[8];
>   #endif
>   
>   	/* buffer handling */
StDenis, Tom Sept. 18, 2017, 1:01 p.m. UTC | #2 
On 18/09/17 08:48 AM, Christian König wrote:
> Am 18.09.2017 um 14:35 schrieb Tom St Denis:
>> Signed-off-by: Tom St Denis <tom.stdenis@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 53 
>> ++++++++++++++++++---------------
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h |  4 +--
>>   2 files changed, 31 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> index 8ee16dfdb8af..7848ffa99eb4 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> @@ -1809,6 +1809,18 @@ static const struct file_operations 
>> amdgpu_ttm_gtt_fops = {
>>   #endif
>> +
>> +
>> +static const struct {
>> +    char *name;
>> +    const struct file_operations *fops;
>> +} ttm_debugfs_entries[] = {
>> +    { "amdgpu_vram", &amdgpu_ttm_vram_fops },
>> +#ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS
>> +    { "amdgpu_gtt", &amdgpu_ttm_gtt_fops },
>> +#endif
>> +};
>> +
>>   #endif
>>   static int amdgpu_ttm_debugfs_init(struct amdgpu_device *adev)
>> @@ -1819,22 +1831,21 @@ static int amdgpu_ttm_debugfs_init(struct 
>> amdgpu_device *adev)
>>       struct drm_minor *minor = adev->ddev->primary;
>>       struct dentry *ent, *root = minor->debugfs_root;
>> -    ent = debugfs_create_file("amdgpu_vram", S_IFREG | S_IRUGO, root,
>> -                  adev, &amdgpu_ttm_vram_fops);
>> -    if (IS_ERR(ent))
>> -        return PTR_ERR(ent);
>> -    i_size_write(ent->d_inode, adev->mc.mc_vram_size);
>> -    adev->mman.vram = ent;
>> -
>> -#ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS
>> -    ent = debugfs_create_file("amdgpu_gtt", S_IFREG | S_IRUGO, root,
>> -                  adev, &amdgpu_ttm_gtt_fops);
>> -    if (IS_ERR(ent))
>> -        return PTR_ERR(ent);
>> -    i_size_write(ent->d_inode, adev->mc.gart_size);
>> -    adev->mman.gtt = ent;
>> +    for (count = 0; count < ARRAY_SIZE(ttm_debugfs_entries); count++) {
>> +        ent = debugfs_create_file(
>> +                ttm_debugfs_entries[count].name,
>> +                S_IFREG | S_IRUGO, root,
>> +                adev,
>> +                ttm_debugfs_entries[count].fops);
>> +        if (IS_ERR(ent))
>> +            return PTR_ERR(ent);
>> +        if (!strcmp(ttm_debugfs_entries[count].name, "amdgpu_vram"))
>> +            i_size_write(ent->d_inode, adev->mc.mc_vram_size);
>> +        else if (!strcmp(ttm_debugfs_entries[count].name, "amdgpu_gtt"))
>> +            i_size_write(ent->d_inode, adev->mc.gart_size);
> 
> Uff, string compare? That is screaming break me by typo.
> 
> Maybe but the domain type into the struct as well?
> 
> Apart from that looks good to me,


Sure, a quick grep didn't turn up any defines/enums for VRAM vs GTT 
though so just make some up?

Tom
Christian König Sept. 18, 2017, 1:03 p.m. UTC | #3 
Am 18.09.2017 um 15:01 schrieb Tom St Denis:
> On 18/09/17 08:48 AM, Christian König wrote:
>> Am 18.09.2017 um 14:35 schrieb Tom St Denis:
>>> Signed-off-by: Tom St Denis <tom.stdenis@amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 53 
>>> ++++++++++++++++++---------------
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h |  4 +--
>>>   2 files changed, 31 insertions(+), 26 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> index 8ee16dfdb8af..7848ffa99eb4 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> @@ -1809,6 +1809,18 @@ static const struct file_operations 
>>> amdgpu_ttm_gtt_fops = {
>>>   #endif
>>> +
>>> +
>>> +static const struct {
>>> +    char *name;
>>> +    const struct file_operations *fops;
>>> +} ttm_debugfs_entries[] = {
>>> +    { "amdgpu_vram", &amdgpu_ttm_vram_fops },
>>> +#ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS
>>> +    { "amdgpu_gtt", &amdgpu_ttm_gtt_fops },
>>> +#endif
>>> +};
>>> +
>>>   #endif
>>>   static int amdgpu_ttm_debugfs_init(struct amdgpu_device *adev)
>>> @@ -1819,22 +1831,21 @@ static int amdgpu_ttm_debugfs_init(struct 
>>> amdgpu_device *adev)
>>>       struct drm_minor *minor = adev->ddev->primary;
>>>       struct dentry *ent, *root = minor->debugfs_root;
>>> -    ent = debugfs_create_file("amdgpu_vram", S_IFREG | S_IRUGO, root,
>>> -                  adev, &amdgpu_ttm_vram_fops);
>>> -    if (IS_ERR(ent))
>>> -        return PTR_ERR(ent);
>>> -    i_size_write(ent->d_inode, adev->mc.mc_vram_size);
>>> -    adev->mman.vram = ent;
>>> -
>>> -#ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS
>>> -    ent = debugfs_create_file("amdgpu_gtt", S_IFREG | S_IRUGO, root,
>>> -                  adev, &amdgpu_ttm_gtt_fops);
>>> -    if (IS_ERR(ent))
>>> -        return PTR_ERR(ent);
>>> -    i_size_write(ent->d_inode, adev->mc.gart_size);
>>> -    adev->mman.gtt = ent;
>>> +    for (count = 0; count < ARRAY_SIZE(ttm_debugfs_entries); 
>>> count++) {
>>> +        ent = debugfs_create_file(
>>> +                ttm_debugfs_entries[count].name,
>>> +                S_IFREG | S_IRUGO, root,
>>> +                adev,
>>> +                ttm_debugfs_entries[count].fops);
>>> +        if (IS_ERR(ent))
>>> +            return PTR_ERR(ent);
>>> +        if (!strcmp(ttm_debugfs_entries[count].name, "amdgpu_vram"))
>>> +            i_size_write(ent->d_inode, adev->mc.mc_vram_size);
>>> +        else if (!strcmp(ttm_debugfs_entries[count].name, 
>>> "amdgpu_gtt"))
>>> +            i_size_write(ent->d_inode, adev->mc.gart_size);
>>
>> Uff, string compare? That is screaming break me by typo.
>>
>> Maybe but the domain type into the struct as well?
>>
>> Apart from that looks good to me,
>
>
> Sure, a quick grep didn't turn up any defines/enums for VRAM vs GTT 
> though so just make some up?

Just use TTM_PL_VRAM and TTM_PL_TT for this.

Christian.

>
> Tom
diff mbox

Patch

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 8ee16dfdb8af..7848ffa99eb4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1809,6 +1809,18 @@  static const struct file_operations amdgpu_ttm_gtt_fops = {
 
 #endif
 
+
+
+static const struct {
+	char *name;
+	const struct file_operations *fops;
+} ttm_debugfs_entries[] = {
+	{ "amdgpu_vram", &amdgpu_ttm_vram_fops },
+#ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS
+	{ "amdgpu_gtt", &amdgpu_ttm_gtt_fops },
+#endif
+};
+
 #endif
 
 static int amdgpu_ttm_debugfs_init(struct amdgpu_device *adev)
@@ -1819,22 +1831,21 @@  static int amdgpu_ttm_debugfs_init(struct amdgpu_device *adev)
 	struct drm_minor *minor = adev->ddev->primary;
 	struct dentry *ent, *root = minor->debugfs_root;
 
-	ent = debugfs_create_file("amdgpu_vram", S_IFREG | S_IRUGO, root,
-				  adev, &amdgpu_ttm_vram_fops);
-	if (IS_ERR(ent))
-		return PTR_ERR(ent);
-	i_size_write(ent->d_inode, adev->mc.mc_vram_size);
-	adev->mman.vram = ent;
-
-#ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS
-	ent = debugfs_create_file("amdgpu_gtt", S_IFREG | S_IRUGO, root,
-				  adev, &amdgpu_ttm_gtt_fops);
-	if (IS_ERR(ent))
-		return PTR_ERR(ent);
-	i_size_write(ent->d_inode, adev->mc.gart_size);
-	adev->mman.gtt = ent;
+	for (count = 0; count < ARRAY_SIZE(ttm_debugfs_entries); count++) {
+		ent = debugfs_create_file(
+				ttm_debugfs_entries[count].name,
+				S_IFREG | S_IRUGO, root,
+				adev,
+				ttm_debugfs_entries[count].fops);
+		if (IS_ERR(ent))
+			return PTR_ERR(ent);
+		if (!strcmp(ttm_debugfs_entries[count].name, "amdgpu_vram"))
+			i_size_write(ent->d_inode, adev->mc.mc_vram_size);
+		else if (!strcmp(ttm_debugfs_entries[count].name, "amdgpu_gtt"))
+			i_size_write(ent->d_inode, adev->mc.gart_size);
+		adev->mman.debugfs_entries[count] = ent;
+	}
 
-#endif
 	count = ARRAY_SIZE(amdgpu_ttm_debugfs_list);
 
 #ifdef CONFIG_SWIOTLB
@@ -1844,7 +1855,6 @@  static int amdgpu_ttm_debugfs_init(struct amdgpu_device *adev)
 
 	return amdgpu_debugfs_add_files(adev, amdgpu_ttm_debugfs_list, count);
 #else
-
 	return 0;
 #endif
 }
@@ -1852,14 +1862,9 @@  static int amdgpu_ttm_debugfs_init(struct amdgpu_device *adev)
 static void amdgpu_ttm_debugfs_fini(struct amdgpu_device *adev)
 {
 #if defined(CONFIG_DEBUG_FS)
+	unsigned i;
 
-	debugfs_remove(adev->mman.vram);
-	adev->mman.vram = NULL;
-
-#ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS
-	debugfs_remove(adev->mman.gtt);
-	adev->mman.gtt = NULL;
-#endif
-
+	for (i = 0; i < ARRAY_SIZE(ttm_debugfs_entries); i++)
+		debugfs_remove(adev->mman.debugfs_entries[i]);
 #endif
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
index 64709e041d5b..7abae6867339 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
@@ -24,6 +24,7 @@ 
 #ifndef __AMDGPU_TTM_H__
 #define __AMDGPU_TTM_H__
 
+#include "amdgpu.h"
 #include "gpu_scheduler.h"
 
 #define AMDGPU_PL_GDS		(TTM_PL_PRIV + 0)
@@ -45,8 +46,7 @@  struct amdgpu_mman {
 	bool				initialized;
 
 #if defined(CONFIG_DEBUG_FS)
-	struct dentry			*vram;
-	struct dentry			*gtt;
+	struct dentry			*debugfs_entries[8];
 #endif
 
 	/* buffer handling */