* [PATCH 1/2] drm/amdgpu: unify the way to judge RAS feature readiness
@ 2019-03-11 4:30 Evan Quan
[not found] ` <20190311043046.10184-1-evan.quan-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 8+ messages in thread
From: Evan Quan @ 2019-03-11 4:30 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: Alexander.Deucher-5C7GfCeVMHo, xinhui.pan-5C7GfCeVMHo, Evan Quan
Unify the way to judge whether a specific RAS feature is
supported.
Change-Id: I14bb19db49f06e134de903376b14eb27e0e038c7
Signed-off-by: Evan Quan <evan.quan@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 10 ++--------
drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h | 2 +-
2 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
index bf462c59cb76..7f9cbd64cb20 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
@@ -466,12 +466,6 @@ static struct ras_manager *amdgpu_ras_find_obj(struct amdgpu_device *adev,
/* obj end */
/* feature ctl begin */
-static int amdgpu_ras_is_feature_allowed(struct amdgpu_device *adev,
- struct ras_common_if *head)
-{
- return amdgpu_ras_enable && (amdgpu_ras_mask & BIT(head->block));
-}
-
static int amdgpu_ras_is_feature_enabled(struct amdgpu_device *adev,
struct ras_common_if *head)
{
@@ -490,7 +484,7 @@ static int __amdgpu_ras_feature_enable(struct amdgpu_device *adev,
struct amdgpu_ras *con = amdgpu_ras_get_context(adev);
struct ras_manager *obj = amdgpu_ras_find_obj(adev, head);
- if (!amdgpu_ras_is_feature_allowed(adev, head))
+ if (!amdgpu_ras_is_supported(adev, head->block))
return 0;
if (!(!!enable ^ !!amdgpu_ras_is_feature_enabled(adev, head)))
return 0;
@@ -539,7 +533,7 @@ int amdgpu_ras_feature_enable(struct amdgpu_device *adev,
}
/* Do not enable if it is not allowed. */
- WARN_ON(enable && !amdgpu_ras_is_feature_allowed(adev, head));
+ WARN_ON(enable && !amdgpu_ras_is_supported(adev, head->block));
/* Are we alerady in that state we are going to set? */
if (!(!!enable ^ !!amdgpu_ras_is_feature_enabled(adev, head)))
return 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h
index 02cb9a13ddc5..0ef2b91b8fcd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h
@@ -163,7 +163,7 @@ struct ras_debug_if {
/* check if ras is supported on block, say, sdma, gfx */
static inline int amdgpu_ras_is_supported(struct amdgpu_device *adev,
- unsigned int block)
+ enum amdgpu_ras_block block)
{
struct amdgpu_ras *ras = amdgpu_ras_get_context(adev);
--
2.21.0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] drm/amdgpu: drop unnecessary dereference
[not found] ` <20190311043046.10184-1-evan.quan-5C7GfCeVMHo@public.gmane.org>
@ 2019-03-11 4:30 ` Evan Quan
[not found] ` <20190311043046.10184-2-evan.quan-5C7GfCeVMHo@public.gmane.org>
2019-03-11 5:08 ` [PATCH 1/2] drm/amdgpu: unify the way to judge RAS feature readiness Pan, Xinhui
1 sibling, 1 reply; 8+ messages in thread
From: Evan Quan @ 2019-03-11 4:30 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: Alexander.Deucher-5C7GfCeVMHo, xinhui.pan-5C7GfCeVMHo, Evan Quan
It's unnecessary and confusing.
Change-Id: I77fe54a108b7ee2031851b3e11d63c4fb74c0d43
Signed-off-by: Evan Quan <evan.quan@amd.com>
---
drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 26 +++++++++++++-------------
drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 26 +++++++++++++-------------
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 26 +++++++++++++-------------
3 files changed, 39 insertions(+), 39 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index 3fb72bf420e0..31996d448817 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -3532,7 +3532,7 @@ static int gfx_v9_0_process_ras_data_cb(struct amdgpu_device *adev,
static int gfx_v9_0_ecc_late_init(void *handle)
{
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
- struct ras_common_if **ras_if = &adev->gfx.ras_if;
+ struct ras_common_if *ras_if = adev->gfx.ras_if;
struct ras_ih_if ih_info = {
.cb = gfx_v9_0_process_ras_data_cb,
};
@@ -3553,21 +3553,21 @@ static int gfx_v9_0_ecc_late_init(void *handle)
return 0;
}
- if (*ras_if)
+ if (ras_if)
goto resume;
- *ras_if = kmalloc(sizeof(**ras_if), GFP_KERNEL);
- if (!*ras_if)
+ ras_if = kmalloc(sizeof(*ras_if), GFP_KERNEL);
+ if (!ras_if)
return -ENOMEM;
- **ras_if = ras_block;
+ *ras_if = ras_block;
- r = amdgpu_ras_feature_enable(adev, *ras_if, 1);
+ r = amdgpu_ras_feature_enable(adev, ras_if, 1);
if (r)
goto feature;
- ih_info.head = **ras_if;
- fs_info.head = **ras_if;
+ ih_info.head = *ras_if;
+ fs_info.head = *ras_if;
r = amdgpu_ras_interrupt_add_handler(adev, &ih_info);
if (r)
@@ -3587,16 +3587,16 @@ static int gfx_v9_0_ecc_late_init(void *handle)
return 0;
irq:
- amdgpu_ras_sysfs_remove(adev, *ras_if);
+ amdgpu_ras_sysfs_remove(adev, ras_if);
sysfs:
- amdgpu_ras_debugfs_remove(adev, *ras_if);
+ amdgpu_ras_debugfs_remove(adev, ras_if);
debugfs:
amdgpu_ras_interrupt_remove_handler(adev, &ih_info);
interrupt:
- amdgpu_ras_feature_enable(adev, *ras_if, 0);
+ amdgpu_ras_feature_enable(adev, ras_if, 0);
feature:
- kfree(*ras_if);
- *ras_if = NULL;
+ kfree(ras_if);
+ ras_if = NULL;
return -EINVAL;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 5d1ac53f7ddb..229e614d1b76 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -714,7 +714,7 @@ static int gmc_v9_0_allocate_vm_inv_eng(struct amdgpu_device *adev)
static int gmc_v9_0_ecc_late_init(void *handle)
{
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
- struct ras_common_if **ras_if = &adev->gmc.ras_if;
+ struct ras_common_if *ras_if = adev->gmc.ras_if;
struct ras_ih_if ih_info = {
.cb = gmc_v9_0_process_ras_data_cb,
};
@@ -735,21 +735,21 @@ static int gmc_v9_0_ecc_late_init(void *handle)
return 0;
}
/* handle resume path. */
- if (*ras_if)
+ if (ras_if)
goto resume;
- *ras_if = kmalloc(sizeof(**ras_if), GFP_KERNEL);
- if (!*ras_if)
+ ras_if = kmalloc(sizeof(*ras_if), GFP_KERNEL);
+ if (!ras_if)
return -ENOMEM;
- **ras_if = ras_block;
+ *ras_if = ras_block;
- r = amdgpu_ras_feature_enable(adev, *ras_if, 1);
+ r = amdgpu_ras_feature_enable(adev, ras_if, 1);
if (r)
goto feature;
- ih_info.head = **ras_if;
- fs_info.head = **ras_if;
+ ih_info.head = *ras_if;
+ fs_info.head = *ras_if;
r = amdgpu_ras_interrupt_add_handler(adev, &ih_info);
if (r)
@@ -769,16 +769,16 @@ static int gmc_v9_0_ecc_late_init(void *handle)
return 0;
irq:
- amdgpu_ras_sysfs_remove(adev, *ras_if);
+ amdgpu_ras_sysfs_remove(adev, ras_if);
sysfs:
- amdgpu_ras_debugfs_remove(adev, *ras_if);
+ amdgpu_ras_debugfs_remove(adev, ras_if);
debugfs:
amdgpu_ras_interrupt_remove_handler(adev, &ih_info);
interrupt:
- amdgpu_ras_feature_enable(adev, *ras_if, 0);
+ amdgpu_ras_feature_enable(adev, ras_if, 0);
feature:
- kfree(*ras_if);
- *ras_if = NULL;
+ kfree(ras_if);
+ ras_if = NULL;
return -EINVAL;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
index 3ac5abe937f4..521218053477 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
@@ -1501,7 +1501,7 @@ static int sdma_v4_0_process_ras_data_cb(struct amdgpu_device *adev,
static int sdma_v4_0_late_init(void *handle)
{
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
- struct ras_common_if **ras_if = &adev->sdma.ras_if;
+ struct ras_common_if *ras_if = adev->sdma.ras_if;
struct ras_ih_if ih_info = {
.cb = sdma_v4_0_process_ras_data_cb,
};
@@ -1523,21 +1523,21 @@ static int sdma_v4_0_late_init(void *handle)
}
/* handle resume path. */
- if (*ras_if)
+ if (ras_if)
goto resume;
- *ras_if = kmalloc(sizeof(**ras_if), GFP_KERNEL);
- if (!*ras_if)
+ ras_if = kmalloc(sizeof(*ras_if), GFP_KERNEL);
+ if (!ras_if)
return -ENOMEM;
- **ras_if = ras_block;
+ *ras_if = ras_block;
- r = amdgpu_ras_feature_enable(adev, *ras_if, 1);
+ r = amdgpu_ras_feature_enable(adev, ras_if, 1);
if (r)
goto feature;
- ih_info.head = **ras_if;
- fs_info.head = **ras_if;
+ ih_info.head = *ras_if;
+ fs_info.head = *ras_if;
r = amdgpu_ras_interrupt_add_handler(adev, &ih_info);
if (r)
@@ -1563,16 +1563,16 @@ static int sdma_v4_0_late_init(void *handle)
return 0;
irq:
- amdgpu_ras_sysfs_remove(adev, *ras_if);
+ amdgpu_ras_sysfs_remove(adev, ras_if);
sysfs:
- amdgpu_ras_debugfs_remove(adev, *ras_if);
+ amdgpu_ras_debugfs_remove(adev, ras_if);
debugfs:
amdgpu_ras_interrupt_remove_handler(adev, &ih_info);
interrupt:
- amdgpu_ras_feature_enable(adev, *ras_if, 0);
+ amdgpu_ras_feature_enable(adev, ras_if, 0);
feature:
- kfree(*ras_if);
- *ras_if = NULL;
+ kfree(ras_if);
+ ras_if = NULL;
return -EINVAL;
}
--
2.21.0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 8+ messages in thread
* RE: [PATCH 1/2] drm/amdgpu: unify the way to judge RAS feature readiness
[not found] ` <20190311043046.10184-1-evan.quan-5C7GfCeVMHo@public.gmane.org>
2019-03-11 4:30 ` [PATCH 2/2] drm/amdgpu: drop unnecessary dereference Evan Quan
@ 2019-03-11 5:08 ` Pan, Xinhui
[not found] ` <SN6PR12MB28007A2E394FFDC0A570959A87480-kxOKjb6HO/EqkY47FTA1ogdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
1 sibling, 1 reply; 8+ messages in thread
From: Pan, Xinhui @ 2019-03-11 5:08 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Deucher, Alexander, Quan, Evan
Inline.
-----Original Message-----
From: Evan Quan <evan.quan@amd.com>
Sent: 2019年3月11日 12:31
To: amd-gfx@lists.freedesktop.org
Cc: Pan, Xinhui <Xinhui.Pan@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com>; Quan, Evan <Evan.Quan@amd.com>
Subject: [PATCH 1/2] drm/amdgpu: unify the way to judge RAS feature readiness
Unify the way to judge whether a specific RAS feature is supported.
Change-Id: I14bb19db49f06e134de903376b14eb27e0e038c7
Signed-off-by: Evan Quan <evan.quan@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 10 ++-------- drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h | 2 +-
2 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
index bf462c59cb76..7f9cbd64cb20 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
@@ -466,12 +466,6 @@ static struct ras_manager *amdgpu_ras_find_obj(struct amdgpu_device *adev,
/* obj end */
/* feature ctl begin */
-static int amdgpu_ras_is_feature_allowed(struct amdgpu_device *adev,
- struct ras_common_if *head)
-{
- return amdgpu_ras_enable && (amdgpu_ras_mask & BIT(head->block));
-}
-
static int amdgpu_ras_is_feature_enabled(struct amdgpu_device *adev,
struct ras_common_if *head)
{
@@ -490,7 +484,7 @@ static int __amdgpu_ras_feature_enable(struct amdgpu_device *adev,
struct amdgpu_ras *con = amdgpu_ras_get_context(adev);
struct ras_manager *obj = amdgpu_ras_find_obj(adev, head);
- if (!amdgpu_ras_is_feature_allowed(adev, head))
+ if (!amdgpu_ras_is_supported(adev, head->block))
return 0;
[XH] I am thinking of splitting supported to hw_supported and sw_supported.
The code become a little clearer now.
For hw_supported case, we need allow IP disable ras.
For sw_suppporeted case, we need allow IP disable/enable/inject/query, etc .
if (!(!!enable ^ !!amdgpu_ras_is_feature_enabled(adev, head)))
return 0;
@@ -539,7 +533,7 @@ int amdgpu_ras_feature_enable(struct amdgpu_device *adev,
}
/* Do not enable if it is not allowed. */
- WARN_ON(enable && !amdgpu_ras_is_feature_allowed(adev, head));
+ WARN_ON(enable && !amdgpu_ras_is_supported(adev, head->block));
/* Are we alerady in that state we are going to set? */
if (!(!!enable ^ !!amdgpu_ras_is_feature_enabled(adev, head)))
return 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h
index 02cb9a13ddc5..0ef2b91b8fcd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h
@@ -163,7 +163,7 @@ struct ras_debug_if {
/* check if ras is supported on block, say, sdma, gfx */ static inline int amdgpu_ras_is_supported(struct amdgpu_device *adev,
- unsigned int block)
+ enum amdgpu_ras_block block)
{
struct amdgpu_ras *ras = amdgpu_ras_get_context(adev);
--
2.21.0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 8+ messages in thread
* RE: [PATCH 2/2] drm/amdgpu: drop unnecessary dereference
[not found] ` <20190311043046.10184-2-evan.quan-5C7GfCeVMHo@public.gmane.org>
@ 2019-03-11 5:19 ` Pan, Xinhui
[not found] ` <SN6PR12MB280003172F75FEECA9BE96D187480-kxOKjb6HO/EqkY47FTA1ogdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
0 siblings, 1 reply; 8+ messages in thread
From: Pan, Xinhui @ 2019-03-11 5:19 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Deucher, Alexander, Quan, Evan
Well, I think you forgot setting adev->gfx.ras_if = ras_if in the end.
struct ras_common_if **ras_if is pretty simple and easy to use.
I prefer to keep the code as it is.
Thanks
xinhui
-----Original Message-----
From: Evan Quan <evan.quan@amd.com>
Sent: 2019年3月11日 12:31
To: amd-gfx@lists.freedesktop.org
Cc: Pan, Xinhui <Xinhui.Pan@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com>; Quan, Evan <Evan.Quan@amd.com>
Subject: [PATCH 2/2] drm/amdgpu: drop unnecessary dereference
It's unnecessary and confusing.
Change-Id: I77fe54a108b7ee2031851b3e11d63c4fb74c0d43
Signed-off-by: Evan Quan <evan.quan@amd.com>
---
drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 26 +++++++++++++------------- drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 26 +++++++++++++------------- drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 26 +++++++++++++-------------
3 files changed, 39 insertions(+), 39 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index 3fb72bf420e0..31996d448817 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -3532,7 +3532,7 @@ static int gfx_v9_0_process_ras_data_cb(struct amdgpu_device *adev, static int gfx_v9_0_ecc_late_init(void *handle) {
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
- struct ras_common_if **ras_if = &adev->gfx.ras_if;
+ struct ras_common_if *ras_if = adev->gfx.ras_if;
struct ras_ih_if ih_info = {
.cb = gfx_v9_0_process_ras_data_cb,
};
@@ -3553,21 +3553,21 @@ static int gfx_v9_0_ecc_late_init(void *handle)
return 0;
}
- if (*ras_if)
+ if (ras_if)
goto resume;
- *ras_if = kmalloc(sizeof(**ras_if), GFP_KERNEL);
- if (!*ras_if)
+ ras_if = kmalloc(sizeof(*ras_if), GFP_KERNEL);
+ if (!ras_if)
return -ENOMEM;
- **ras_if = ras_block;
+ *ras_if = ras_block;
- r = amdgpu_ras_feature_enable(adev, *ras_if, 1);
+ r = amdgpu_ras_feature_enable(adev, ras_if, 1);
if (r)
goto feature;
- ih_info.head = **ras_if;
- fs_info.head = **ras_if;
+ ih_info.head = *ras_if;
+ fs_info.head = *ras_if;
r = amdgpu_ras_interrupt_add_handler(adev, &ih_info);
if (r)
@@ -3587,16 +3587,16 @@ static int gfx_v9_0_ecc_late_init(void *handle)
return 0;
irq:
- amdgpu_ras_sysfs_remove(adev, *ras_if);
+ amdgpu_ras_sysfs_remove(adev, ras_if);
sysfs:
- amdgpu_ras_debugfs_remove(adev, *ras_if);
+ amdgpu_ras_debugfs_remove(adev, ras_if);
debugfs:
amdgpu_ras_interrupt_remove_handler(adev, &ih_info);
interrupt:
- amdgpu_ras_feature_enable(adev, *ras_if, 0);
+ amdgpu_ras_feature_enable(adev, ras_if, 0);
feature:
- kfree(*ras_if);
- *ras_if = NULL;
+ kfree(ras_if);
+ ras_if = NULL;
return -EINVAL;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 5d1ac53f7ddb..229e614d1b76 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -714,7 +714,7 @@ static int gmc_v9_0_allocate_vm_inv_eng(struct amdgpu_device *adev) static int gmc_v9_0_ecc_late_init(void *handle) {
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
- struct ras_common_if **ras_if = &adev->gmc.ras_if;
+ struct ras_common_if *ras_if = adev->gmc.ras_if;
struct ras_ih_if ih_info = {
.cb = gmc_v9_0_process_ras_data_cb,
};
@@ -735,21 +735,21 @@ static int gmc_v9_0_ecc_late_init(void *handle)
return 0;
}
/* handle resume path. */
- if (*ras_if)
+ if (ras_if)
goto resume;
- *ras_if = kmalloc(sizeof(**ras_if), GFP_KERNEL);
- if (!*ras_if)
+ ras_if = kmalloc(sizeof(*ras_if), GFP_KERNEL);
+ if (!ras_if)
return -ENOMEM;
- **ras_if = ras_block;
+ *ras_if = ras_block;
- r = amdgpu_ras_feature_enable(adev, *ras_if, 1);
+ r = amdgpu_ras_feature_enable(adev, ras_if, 1);
if (r)
goto feature;
- ih_info.head = **ras_if;
- fs_info.head = **ras_if;
+ ih_info.head = *ras_if;
+ fs_info.head = *ras_if;
r = amdgpu_ras_interrupt_add_handler(adev, &ih_info);
if (r)
@@ -769,16 +769,16 @@ static int gmc_v9_0_ecc_late_init(void *handle)
return 0;
irq:
- amdgpu_ras_sysfs_remove(adev, *ras_if);
+ amdgpu_ras_sysfs_remove(adev, ras_if);
sysfs:
- amdgpu_ras_debugfs_remove(adev, *ras_if);
+ amdgpu_ras_debugfs_remove(adev, ras_if);
debugfs:
amdgpu_ras_interrupt_remove_handler(adev, &ih_info);
interrupt:
- amdgpu_ras_feature_enable(adev, *ras_if, 0);
+ amdgpu_ras_feature_enable(adev, ras_if, 0);
feature:
- kfree(*ras_if);
- *ras_if = NULL;
+ kfree(ras_if);
+ ras_if = NULL;
return -EINVAL;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
index 3ac5abe937f4..521218053477 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
@@ -1501,7 +1501,7 @@ static int sdma_v4_0_process_ras_data_cb(struct amdgpu_device *adev, static int sdma_v4_0_late_init(void *handle) {
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
- struct ras_common_if **ras_if = &adev->sdma.ras_if;
+ struct ras_common_if *ras_if = adev->sdma.ras_if;
struct ras_ih_if ih_info = {
.cb = sdma_v4_0_process_ras_data_cb,
};
@@ -1523,21 +1523,21 @@ static int sdma_v4_0_late_init(void *handle)
}
/* handle resume path. */
- if (*ras_if)
+ if (ras_if)
goto resume;
- *ras_if = kmalloc(sizeof(**ras_if), GFP_KERNEL);
- if (!*ras_if)
+ ras_if = kmalloc(sizeof(*ras_if), GFP_KERNEL);
+ if (!ras_if)
return -ENOMEM;
- **ras_if = ras_block;
+ *ras_if = ras_block;
- r = amdgpu_ras_feature_enable(adev, *ras_if, 1);
+ r = amdgpu_ras_feature_enable(adev, ras_if, 1);
if (r)
goto feature;
- ih_info.head = **ras_if;
- fs_info.head = **ras_if;
+ ih_info.head = *ras_if;
+ fs_info.head = *ras_if;
r = amdgpu_ras_interrupt_add_handler(adev, &ih_info);
if (r)
@@ -1563,16 +1563,16 @@ static int sdma_v4_0_late_init(void *handle)
return 0;
irq:
- amdgpu_ras_sysfs_remove(adev, *ras_if);
+ amdgpu_ras_sysfs_remove(adev, ras_if);
sysfs:
- amdgpu_ras_debugfs_remove(adev, *ras_if);
+ amdgpu_ras_debugfs_remove(adev, ras_if);
debugfs:
amdgpu_ras_interrupt_remove_handler(adev, &ih_info);
interrupt:
- amdgpu_ras_feature_enable(adev, *ras_if, 0);
+ amdgpu_ras_feature_enable(adev, ras_if, 0);
feature:
- kfree(*ras_if);
- *ras_if = NULL;
+ kfree(ras_if);
+ ras_if = NULL;
return -EINVAL;
}
--
2.21.0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 8+ messages in thread
* RE: [PATCH 1/2] drm/amdgpu: unify the way to judge RAS feature readiness
[not found] ` <SN6PR12MB28007A2E394FFDC0A570959A87480-kxOKjb6HO/EqkY47FTA1ogdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
@ 2019-03-11 5:27 ` Quan, Evan
0 siblings, 0 replies; 8+ messages in thread
From: Quan, Evan @ 2019-03-11 5:27 UTC (permalink / raw)
To: Pan, Xinhui, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Deucher, Alexander
> -----Original Message-----
> From: Pan, Xinhui
> Sent: 2019年3月11日 13:08
> To: Quan, Evan <Evan.Quan@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Quan, Evan
> <Evan.Quan@amd.com>
> Subject: RE: [PATCH 1/2] drm/amdgpu: unify the way to judge RAS feature
> readiness
>
>
> Inline.
>
> -----Original Message-----
> From: Evan Quan <evan.quan@amd.com>
> Sent: 2019年3月11日 12:31
> To: amd-gfx@lists.freedesktop.org
> Cc: Pan, Xinhui <Xinhui.Pan@amd.com>; Deucher, Alexander
> <Alexander.Deucher@amd.com>; Quan, Evan <Evan.Quan@amd.com>
> Subject: [PATCH 1/2] drm/amdgpu: unify the way to judge RAS feature
> readiness
>
> Unify the way to judge whether a specific RAS feature is supported.
>
> Change-Id: I14bb19db49f06e134de903376b14eb27e0e038c7
> Signed-off-by: Evan Quan <evan.quan@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 10 ++--------
> drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h | 2 +-
> 2 files changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> index bf462c59cb76..7f9cbd64cb20 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> @@ -466,12 +466,6 @@ static struct ras_manager
> *amdgpu_ras_find_obj(struct amdgpu_device *adev,
> /* obj end */
>
> /* feature ctl begin */
> -static int amdgpu_ras_is_feature_allowed(struct amdgpu_device *adev,
> - struct ras_common_if *head)
> -{
> - return amdgpu_ras_enable && (amdgpu_ras_mask & BIT(head-
> >block));
> -}
> -
> static int amdgpu_ras_is_feature_enabled(struct amdgpu_device *adev,
> struct ras_common_if *head)
> {
> @@ -490,7 +484,7 @@ static int __amdgpu_ras_feature_enable(struct
> amdgpu_device *adev,
> struct amdgpu_ras *con = amdgpu_ras_get_context(adev);
> struct ras_manager *obj = amdgpu_ras_find_obj(adev, head);
>
> - if (!amdgpu_ras_is_feature_allowed(adev, head))
> + if (!amdgpu_ras_is_supported(adev, head->block))
> return 0;
>
> [XH] I am thinking of splitting supported to hw_supported and sw_supported.
> The code become a little clearer now.
> For hw_supported case, we need allow IP disable ras.
> For sw_suppporeted case, we need allow IP disable/enable/inject/query,
> etc .
>
[Quan, Evan] "sw_supported" here means amdgpu_ras_enable/mask module parameters. Right?
[Quan, Evan] And do you mean even with "sw_supported" disabled(ras_enable/mask = 0), there will be still RAS disable functionality as long as
hardware supports it? Not quite sure, that sounds a little confusing.
Anyway, I think you can add one more flag to existing amdgpu_ras_is_supported() to support this. Maintaining two APIs which do almost the
same thing is error-prone.
>
> if (!(!!enable ^ !!amdgpu_ras_is_feature_enabled(adev, head)))
> return 0;
> @@ -539,7 +533,7 @@ int amdgpu_ras_feature_enable(struct
> amdgpu_device *adev,
> }
>
> /* Do not enable if it is not allowed. */
> - WARN_ON(enable && !amdgpu_ras_is_feature_allowed(adev,
> head));
> + WARN_ON(enable && !amdgpu_ras_is_supported(adev, head-
> >block));
> /* Are we alerady in that state we are going to set? */
> if (!(!!enable ^ !!amdgpu_ras_is_feature_enabled(adev, head)))
> return 0;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h
> index 02cb9a13ddc5..0ef2b91b8fcd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h
> @@ -163,7 +163,7 @@ struct ras_debug_if {
>
> /* check if ras is supported on block, say, sdma, gfx */ static inline int
> amdgpu_ras_is_supported(struct amdgpu_device *adev,
> - unsigned int block)
> + enum amdgpu_ras_block block)
> {
> struct amdgpu_ras *ras = amdgpu_ras_get_context(adev);
>
> --
> 2.21.0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH 2/2] drm/amdgpu: drop unnecessary dereference
[not found] ` <SN6PR12MB280003172F75FEECA9BE96D187480-kxOKjb6HO/EqkY47FTA1ogdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
@ 2019-03-11 5:41 ` Quan, Evan
[not found] ` <MN2PR12MB3344AFD285AAD5483D92580CE4480-rweVpJHSKToDMgCC8P//OwdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
0 siblings, 1 reply; 8+ messages in thread
From: Quan, Evan @ 2019-03-11 5:41 UTC (permalink / raw)
To: Pan, Xinhui, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Deucher, Alexander
I cannot get your point. "ras_if" was already assigned to be same as "adev->gfx.ras_if" at the start of the APIs.
//// struct ras_common_if *ras_if = adev->gfx.ras_if; /////
Why there is need to set " adev->gfx.ras_if = ras_if" again?
Regards,
Evan
> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Pan,
> Xinhui
> Sent: 2019年3月11日 13:19
> To: Quan, Evan <Evan.Quan@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Quan, Evan
> <Evan.Quan@amd.com>
> Subject: RE: [PATCH 2/2] drm/amdgpu: drop unnecessary dereference
>
> Well, I think you forgot setting adev->gfx.ras_if = ras_if in the end.
>
> struct ras_common_if **ras_if is pretty simple and easy to use.
> I prefer to keep the code as it is.
>
> Thanks
> xinhui
>
>
> -----Original Message-----
> From: Evan Quan <evan.quan@amd.com>
> Sent: 2019年3月11日 12:31
> To: amd-gfx@lists.freedesktop.org
> Cc: Pan, Xinhui <Xinhui.Pan@amd.com>; Deucher, Alexander
> <Alexander.Deucher@amd.com>; Quan, Evan <Evan.Quan@amd.com>
> Subject: [PATCH 2/2] drm/amdgpu: drop unnecessary dereference
>
> It's unnecessary and confusing.
>
> Change-Id: I77fe54a108b7ee2031851b3e11d63c4fb74c0d43
> Signed-off-by: Evan Quan <evan.quan@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 26 +++++++++++++-------------
> drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 26 +++++++++++++------------
> - drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 26 +++++++++++++---------
> ----
> 3 files changed, 39 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index 3fb72bf420e0..31996d448817 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> @@ -3532,7 +3532,7 @@ static int gfx_v9_0_process_ras_data_cb(struct
> amdgpu_device *adev, static int gfx_v9_0_ecc_late_init(void *handle) {
> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> - struct ras_common_if **ras_if = &adev->gfx.ras_if;
> + struct ras_common_if *ras_if = adev->gfx.ras_if;
> struct ras_ih_if ih_info = {
> .cb = gfx_v9_0_process_ras_data_cb,
> };
> @@ -3553,21 +3553,21 @@ static int gfx_v9_0_ecc_late_init(void *handle)
> return 0;
> }
>
> - if (*ras_if)
> + if (ras_if)
> goto resume;
>
> - *ras_if = kmalloc(sizeof(**ras_if), GFP_KERNEL);
> - if (!*ras_if)
> + ras_if = kmalloc(sizeof(*ras_if), GFP_KERNEL);
> + if (!ras_if)
> return -ENOMEM;
>
> - **ras_if = ras_block;
> + *ras_if = ras_block;
>
> - r = amdgpu_ras_feature_enable(adev, *ras_if, 1);
> + r = amdgpu_ras_feature_enable(adev, ras_if, 1);
> if (r)
> goto feature;
>
> - ih_info.head = **ras_if;
> - fs_info.head = **ras_if;
> + ih_info.head = *ras_if;
> + fs_info.head = *ras_if;
>
> r = amdgpu_ras_interrupt_add_handler(adev, &ih_info);
> if (r)
> @@ -3587,16 +3587,16 @@ static int gfx_v9_0_ecc_late_init(void *handle)
>
> return 0;
> irq:
> - amdgpu_ras_sysfs_remove(adev, *ras_if);
> + amdgpu_ras_sysfs_remove(adev, ras_if);
> sysfs:
> - amdgpu_ras_debugfs_remove(adev, *ras_if);
> + amdgpu_ras_debugfs_remove(adev, ras_if);
> debugfs:
> amdgpu_ras_interrupt_remove_handler(adev, &ih_info);
> interrupt:
> - amdgpu_ras_feature_enable(adev, *ras_if, 0);
> + amdgpu_ras_feature_enable(adev, ras_if, 0);
> feature:
> - kfree(*ras_if);
> - *ras_if = NULL;
> + kfree(ras_if);
> + ras_if = NULL;
> return -EINVAL;
> }
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index 5d1ac53f7ddb..229e614d1b76 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -714,7 +714,7 @@ static int gmc_v9_0_allocate_vm_inv_eng(struct
> amdgpu_device *adev) static int gmc_v9_0_ecc_late_init(void *handle) {
> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> - struct ras_common_if **ras_if = &adev->gmc.ras_if;
> + struct ras_common_if *ras_if = adev->gmc.ras_if;
> struct ras_ih_if ih_info = {
> .cb = gmc_v9_0_process_ras_data_cb,
> };
> @@ -735,21 +735,21 @@ static int gmc_v9_0_ecc_late_init(void *handle)
> return 0;
> }
> /* handle resume path. */
> - if (*ras_if)
> + if (ras_if)
> goto resume;
>
> - *ras_if = kmalloc(sizeof(**ras_if), GFP_KERNEL);
> - if (!*ras_if)
> + ras_if = kmalloc(sizeof(*ras_if), GFP_KERNEL);
> + if (!ras_if)
> return -ENOMEM;
>
> - **ras_if = ras_block;
> + *ras_if = ras_block;
>
> - r = amdgpu_ras_feature_enable(adev, *ras_if, 1);
> + r = amdgpu_ras_feature_enable(adev, ras_if, 1);
> if (r)
> goto feature;
>
> - ih_info.head = **ras_if;
> - fs_info.head = **ras_if;
> + ih_info.head = *ras_if;
> + fs_info.head = *ras_if;
>
> r = amdgpu_ras_interrupt_add_handler(adev, &ih_info);
> if (r)
> @@ -769,16 +769,16 @@ static int gmc_v9_0_ecc_late_init(void *handle)
>
> return 0;
> irq:
> - amdgpu_ras_sysfs_remove(adev, *ras_if);
> + amdgpu_ras_sysfs_remove(adev, ras_if);
> sysfs:
> - amdgpu_ras_debugfs_remove(adev, *ras_if);
> + amdgpu_ras_debugfs_remove(adev, ras_if);
> debugfs:
> amdgpu_ras_interrupt_remove_handler(adev, &ih_info);
> interrupt:
> - amdgpu_ras_feature_enable(adev, *ras_if, 0);
> + amdgpu_ras_feature_enable(adev, ras_if, 0);
> feature:
> - kfree(*ras_if);
> - *ras_if = NULL;
> + kfree(ras_if);
> + ras_if = NULL;
> return -EINVAL;
> }
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> index 3ac5abe937f4..521218053477 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> @@ -1501,7 +1501,7 @@ static int sdma_v4_0_process_ras_data_cb(struct
> amdgpu_device *adev, static int sdma_v4_0_late_init(void *handle) {
> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> - struct ras_common_if **ras_if = &adev->sdma.ras_if;
> + struct ras_common_if *ras_if = adev->sdma.ras_if;
> struct ras_ih_if ih_info = {
> .cb = sdma_v4_0_process_ras_data_cb,
> };
> @@ -1523,21 +1523,21 @@ static int sdma_v4_0_late_init(void *handle)
> }
>
> /* handle resume path. */
> - if (*ras_if)
> + if (ras_if)
> goto resume;
>
> - *ras_if = kmalloc(sizeof(**ras_if), GFP_KERNEL);
> - if (!*ras_if)
> + ras_if = kmalloc(sizeof(*ras_if), GFP_KERNEL);
> + if (!ras_if)
> return -ENOMEM;
>
> - **ras_if = ras_block;
> + *ras_if = ras_block;
>
> - r = amdgpu_ras_feature_enable(adev, *ras_if, 1);
> + r = amdgpu_ras_feature_enable(adev, ras_if, 1);
> if (r)
> goto feature;
>
> - ih_info.head = **ras_if;
> - fs_info.head = **ras_if;
> + ih_info.head = *ras_if;
> + fs_info.head = *ras_if;
>
> r = amdgpu_ras_interrupt_add_handler(adev, &ih_info);
> if (r)
> @@ -1563,16 +1563,16 @@ static int sdma_v4_0_late_init(void *handle)
>
> return 0;
> irq:
> - amdgpu_ras_sysfs_remove(adev, *ras_if);
> + amdgpu_ras_sysfs_remove(adev, ras_if);
> sysfs:
> - amdgpu_ras_debugfs_remove(adev, *ras_if);
> + amdgpu_ras_debugfs_remove(adev, ras_if);
> debugfs:
> amdgpu_ras_interrupt_remove_handler(adev, &ih_info);
> interrupt:
> - amdgpu_ras_feature_enable(adev, *ras_if, 0);
> + amdgpu_ras_feature_enable(adev, ras_if, 0);
> feature:
> - kfree(*ras_if);
> - *ras_if = NULL;
> + kfree(ras_if);
> + ras_if = NULL;
> return -EINVAL;
> }
>
> --
> 2.21.0
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH 2/2] drm/amdgpu: drop unnecessary dereference
[not found] ` <MN2PR12MB3344AFD285AAD5483D92580CE4480-rweVpJHSKToDMgCC8P//OwdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
@ 2019-03-11 5:45 ` Pan, Xinhui
[not found] ` <SN6PR12MB28008BD6E89295354485D56B87480-kxOKjb6HO/EqkY47FTA1ogdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
0 siblings, 1 reply; 8+ messages in thread
From: Pan, Xinhui @ 2019-03-11 5:45 UTC (permalink / raw)
To: Quan, Evan, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Deucher, Alexander
adev->gfx.ras_if is a pointer and it is NULL at the first time
-----Original Message-----
From: Quan, Evan <Evan.Quan@amd.com>
Sent: 2019年3月11日 13:41
To: Pan, Xinhui <Xinhui.Pan@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
Subject: RE: [PATCH 2/2] drm/amdgpu: drop unnecessary dereference
I cannot get your point. "ras_if" was already assigned to be same as "adev->gfx.ras_if" at the start of the APIs.
//// struct ras_common_if *ras_if = adev->gfx.ras_if; ///// Why there is need to set " adev->gfx.ras_if = ras_if" again?
Regards,
Evan
> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
> Pan, Xinhui
> Sent: 2019年3月11日 13:19
> To: Quan, Evan <Evan.Quan@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Quan, Evan
> <Evan.Quan@amd.com>
> Subject: RE: [PATCH 2/2] drm/amdgpu: drop unnecessary dereference
>
> Well, I think you forgot setting adev->gfx.ras_if = ras_if in the end.
>
> struct ras_common_if **ras_if is pretty simple and easy to use.
> I prefer to keep the code as it is.
>
> Thanks
> xinhui
>
>
> -----Original Message-----
> From: Evan Quan <evan.quan@amd.com>
> Sent: 2019年3月11日 12:31
> To: amd-gfx@lists.freedesktop.org
> Cc: Pan, Xinhui <Xinhui.Pan@amd.com>; Deucher, Alexander
> <Alexander.Deucher@amd.com>; Quan, Evan <Evan.Quan@amd.com>
> Subject: [PATCH 2/2] drm/amdgpu: drop unnecessary dereference
>
> It's unnecessary and confusing.
>
> Change-Id: I77fe54a108b7ee2031851b3e11d63c4fb74c0d43
> Signed-off-by: Evan Quan <evan.quan@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 26
> +++++++++++++------------- drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 26
> +++++++++++++------------
> - drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 26 +++++++++++++---------
> ----
> 3 files changed, 39 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index 3fb72bf420e0..31996d448817 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> @@ -3532,7 +3532,7 @@ static int gfx_v9_0_process_ras_data_cb(struct
> amdgpu_device *adev, static int gfx_v9_0_ecc_late_init(void *handle) {
> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> - struct ras_common_if **ras_if = &adev->gfx.ras_if;
> + struct ras_common_if *ras_if = adev->gfx.ras_if;
> struct ras_ih_if ih_info = {
> .cb = gfx_v9_0_process_ras_data_cb,
> };
> @@ -3553,21 +3553,21 @@ static int gfx_v9_0_ecc_late_init(void *handle)
> return 0;
> }
>
> - if (*ras_if)
> + if (ras_if)
> goto resume;
>
> - *ras_if = kmalloc(sizeof(**ras_if), GFP_KERNEL);
> - if (!*ras_if)
> + ras_if = kmalloc(sizeof(*ras_if), GFP_KERNEL);
> + if (!ras_if)
> return -ENOMEM;
>
> - **ras_if = ras_block;
> + *ras_if = ras_block;
>
> - r = amdgpu_ras_feature_enable(adev, *ras_if, 1);
> + r = amdgpu_ras_feature_enable(adev, ras_if, 1);
> if (r)
> goto feature;
>
> - ih_info.head = **ras_if;
> - fs_info.head = **ras_if;
> + ih_info.head = *ras_if;
> + fs_info.head = *ras_if;
>
> r = amdgpu_ras_interrupt_add_handler(adev, &ih_info);
> if (r)
> @@ -3587,16 +3587,16 @@ static int gfx_v9_0_ecc_late_init(void
> *handle)
>
> return 0;
> irq:
> - amdgpu_ras_sysfs_remove(adev, *ras_if);
> + amdgpu_ras_sysfs_remove(adev, ras_if);
> sysfs:
> - amdgpu_ras_debugfs_remove(adev, *ras_if);
> + amdgpu_ras_debugfs_remove(adev, ras_if);
> debugfs:
> amdgpu_ras_interrupt_remove_handler(adev, &ih_info);
> interrupt:
> - amdgpu_ras_feature_enable(adev, *ras_if, 0);
> + amdgpu_ras_feature_enable(adev, ras_if, 0);
> feature:
> - kfree(*ras_if);
> - *ras_if = NULL;
> + kfree(ras_if);
> + ras_if = NULL;
> return -EINVAL;
> }
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index 5d1ac53f7ddb..229e614d1b76 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -714,7 +714,7 @@ static int gmc_v9_0_allocate_vm_inv_eng(struct
> amdgpu_device *adev) static int gmc_v9_0_ecc_late_init(void *handle) {
> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> - struct ras_common_if **ras_if = &adev->gmc.ras_if;
> + struct ras_common_if *ras_if = adev->gmc.ras_if;
> struct ras_ih_if ih_info = {
> .cb = gmc_v9_0_process_ras_data_cb,
> };
> @@ -735,21 +735,21 @@ static int gmc_v9_0_ecc_late_init(void *handle)
> return 0;
> }
> /* handle resume path. */
> - if (*ras_if)
> + if (ras_if)
> goto resume;
>
> - *ras_if = kmalloc(sizeof(**ras_if), GFP_KERNEL);
> - if (!*ras_if)
> + ras_if = kmalloc(sizeof(*ras_if), GFP_KERNEL);
> + if (!ras_if)
> return -ENOMEM;
>
> - **ras_if = ras_block;
> + *ras_if = ras_block;
>
> - r = amdgpu_ras_feature_enable(adev, *ras_if, 1);
> + r = amdgpu_ras_feature_enable(adev, ras_if, 1);
> if (r)
> goto feature;
>
> - ih_info.head = **ras_if;
> - fs_info.head = **ras_if;
> + ih_info.head = *ras_if;
> + fs_info.head = *ras_if;
>
> r = amdgpu_ras_interrupt_add_handler(adev, &ih_info);
> if (r)
> @@ -769,16 +769,16 @@ static int gmc_v9_0_ecc_late_init(void *handle)
>
> return 0;
> irq:
> - amdgpu_ras_sysfs_remove(adev, *ras_if);
> + amdgpu_ras_sysfs_remove(adev, ras_if);
> sysfs:
> - amdgpu_ras_debugfs_remove(adev, *ras_if);
> + amdgpu_ras_debugfs_remove(adev, ras_if);
> debugfs:
> amdgpu_ras_interrupt_remove_handler(adev, &ih_info);
> interrupt:
> - amdgpu_ras_feature_enable(adev, *ras_if, 0);
> + amdgpu_ras_feature_enable(adev, ras_if, 0);
> feature:
> - kfree(*ras_if);
> - *ras_if = NULL;
> + kfree(ras_if);
> + ras_if = NULL;
> return -EINVAL;
> }
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> index 3ac5abe937f4..521218053477 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> @@ -1501,7 +1501,7 @@ static int sdma_v4_0_process_ras_data_cb(struct
> amdgpu_device *adev, static int sdma_v4_0_late_init(void *handle) {
> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> - struct ras_common_if **ras_if = &adev->sdma.ras_if;
> + struct ras_common_if *ras_if = adev->sdma.ras_if;
> struct ras_ih_if ih_info = {
> .cb = sdma_v4_0_process_ras_data_cb,
> };
> @@ -1523,21 +1523,21 @@ static int sdma_v4_0_late_init(void *handle)
> }
>
> /* handle resume path. */
> - if (*ras_if)
> + if (ras_if)
> goto resume;
>
> - *ras_if = kmalloc(sizeof(**ras_if), GFP_KERNEL);
> - if (!*ras_if)
> + ras_if = kmalloc(sizeof(*ras_if), GFP_KERNEL);
> + if (!ras_if)
> return -ENOMEM;
>
> - **ras_if = ras_block;
> + *ras_if = ras_block;
>
> - r = amdgpu_ras_feature_enable(adev, *ras_if, 1);
> + r = amdgpu_ras_feature_enable(adev, ras_if, 1);
> if (r)
> goto feature;
>
> - ih_info.head = **ras_if;
> - fs_info.head = **ras_if;
> + ih_info.head = *ras_if;
> + fs_info.head = *ras_if;
>
> r = amdgpu_ras_interrupt_add_handler(adev, &ih_info);
> if (r)
> @@ -1563,16 +1563,16 @@ static int sdma_v4_0_late_init(void *handle)
>
> return 0;
> irq:
> - amdgpu_ras_sysfs_remove(adev, *ras_if);
> + amdgpu_ras_sysfs_remove(adev, ras_if);
> sysfs:
> - amdgpu_ras_debugfs_remove(adev, *ras_if);
> + amdgpu_ras_debugfs_remove(adev, ras_if);
> debugfs:
> amdgpu_ras_interrupt_remove_handler(adev, &ih_info);
> interrupt:
> - amdgpu_ras_feature_enable(adev, *ras_if, 0);
> + amdgpu_ras_feature_enable(adev, ras_if, 0);
> feature:
> - kfree(*ras_if);
> - *ras_if = NULL;
> + kfree(ras_if);
> + ras_if = NULL;
> return -EINVAL;
> }
>
> --
> 2.21.0
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH 2/2] drm/amdgpu: drop unnecessary dereference
[not found] ` <SN6PR12MB28008BD6E89295354485D56B87480-kxOKjb6HO/EqkY47FTA1ogdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
@ 2019-03-11 6:20 ` Quan, Evan
0 siblings, 0 replies; 8+ messages in thread
From: Quan, Evan @ 2019-03-11 6:20 UTC (permalink / raw)
To: Pan, Xinhui, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Deucher, Alexander
OK, i see. Yes, adev->gfx.ras_if needs to point to the new alloced buffer as it was NULL.
I still think drop one level dereference can make code clean and better understandable.
Anyway, it's fine with me to keep old code.
Regards,
Evan
> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Pan,
> Xinhui
> Sent: 2019年3月11日 13:45
> To: Quan, Evan <Evan.Quan@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
> Subject: RE: [PATCH 2/2] drm/amdgpu: drop unnecessary dereference
>
> adev->gfx.ras_if is a pointer and it is NULL at the first time
>
> -----Original Message-----
> From: Quan, Evan <Evan.Quan@amd.com>
> Sent: 2019年3月11日 13:41
> To: Pan, Xinhui <Xinhui.Pan@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
> Subject: RE: [PATCH 2/2] drm/amdgpu: drop unnecessary dereference
>
> I cannot get your point. "ras_if" was already assigned to be same as "adev-
> >gfx.ras_if" at the start of the APIs.
> //// struct ras_common_if *ras_if = adev->gfx.ras_if; ///// Why there is
> need to set " adev->gfx.ras_if = ras_if" again?
>
> Regards,
> Evan
> > -----Original Message-----
> > From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
> > Pan, Xinhui
> > Sent: 2019年3月11日 13:19
> > To: Quan, Evan <Evan.Quan@amd.com>; amd-gfx@lists.freedesktop.org
> > Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Quan, Evan
> > <Evan.Quan@amd.com>
> > Subject: RE: [PATCH 2/2] drm/amdgpu: drop unnecessary dereference
> >
> > Well, I think you forgot setting adev->gfx.ras_if = ras_if in the end.
> >
> > struct ras_common_if **ras_if is pretty simple and easy to use.
> > I prefer to keep the code as it is.
> >
> > Thanks
> > xinhui
> >
> >
> > -----Original Message-----
> > From: Evan Quan <evan.quan@amd.com>
> > Sent: 2019年3月11日 12:31
> > To: amd-gfx@lists.freedesktop.org
> > Cc: Pan, Xinhui <Xinhui.Pan@amd.com>; Deucher, Alexander
> > <Alexander.Deucher@amd.com>; Quan, Evan <Evan.Quan@amd.com>
> > Subject: [PATCH 2/2] drm/amdgpu: drop unnecessary dereference
> >
> > It's unnecessary and confusing.
> >
> > Change-Id: I77fe54a108b7ee2031851b3e11d63c4fb74c0d43
> > Signed-off-by: Evan Quan <evan.quan@amd.com>
> > ---
> > drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 26
> > +++++++++++++------------- drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c |
> 26
> > +++++++++++++------------
> > - drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 26 +++++++++++++-------
> --
> > ----
> > 3 files changed, 39 insertions(+), 39 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> > b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> > index 3fb72bf420e0..31996d448817 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> > @@ -3532,7 +3532,7 @@ static int gfx_v9_0_process_ras_data_cb(struct
> > amdgpu_device *adev, static int gfx_v9_0_ecc_late_init(void *handle) {
> > struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> > - struct ras_common_if **ras_if = &adev->gfx.ras_if;
> > + struct ras_common_if *ras_if = adev->gfx.ras_if;
> > struct ras_ih_if ih_info = {
> > .cb = gfx_v9_0_process_ras_data_cb,
> > };
> > @@ -3553,21 +3553,21 @@ static int gfx_v9_0_ecc_late_init(void *handle)
> > return 0;
> > }
> >
> > - if (*ras_if)
> > + if (ras_if)
> > goto resume;
> >
> > - *ras_if = kmalloc(sizeof(**ras_if), GFP_KERNEL);
> > - if (!*ras_if)
> > + ras_if = kmalloc(sizeof(*ras_if), GFP_KERNEL);
> > + if (!ras_if)
> > return -ENOMEM;
> >
> > - **ras_if = ras_block;
> > + *ras_if = ras_block;
> >
> > - r = amdgpu_ras_feature_enable(adev, *ras_if, 1);
> > + r = amdgpu_ras_feature_enable(adev, ras_if, 1);
> > if (r)
> > goto feature;
> >
> > - ih_info.head = **ras_if;
> > - fs_info.head = **ras_if;
> > + ih_info.head = *ras_if;
> > + fs_info.head = *ras_if;
> >
> > r = amdgpu_ras_interrupt_add_handler(adev, &ih_info);
> > if (r)
> > @@ -3587,16 +3587,16 @@ static int gfx_v9_0_ecc_late_init(void
> > *handle)
> >
> > return 0;
> > irq:
> > - amdgpu_ras_sysfs_remove(adev, *ras_if);
> > + amdgpu_ras_sysfs_remove(adev, ras_if);
> > sysfs:
> > - amdgpu_ras_debugfs_remove(adev, *ras_if);
> > + amdgpu_ras_debugfs_remove(adev, ras_if);
> > debugfs:
> > amdgpu_ras_interrupt_remove_handler(adev, &ih_info);
> > interrupt:
> > - amdgpu_ras_feature_enable(adev, *ras_if, 0);
> > + amdgpu_ras_feature_enable(adev, ras_if, 0);
> > feature:
> > - kfree(*ras_if);
> > - *ras_if = NULL;
> > + kfree(ras_if);
> > + ras_if = NULL;
> > return -EINVAL;
> > }
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> > b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> > index 5d1ac53f7ddb..229e614d1b76 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> > @@ -714,7 +714,7 @@ static int gmc_v9_0_allocate_vm_inv_eng(struct
> > amdgpu_device *adev) static int gmc_v9_0_ecc_late_init(void *handle) {
> > struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> > - struct ras_common_if **ras_if = &adev->gmc.ras_if;
> > + struct ras_common_if *ras_if = adev->gmc.ras_if;
> > struct ras_ih_if ih_info = {
> > .cb = gmc_v9_0_process_ras_data_cb,
> > };
> > @@ -735,21 +735,21 @@ static int gmc_v9_0_ecc_late_init(void *handle)
> > return 0;
> > }
> > /* handle resume path. */
> > - if (*ras_if)
> > + if (ras_if)
> > goto resume;
> >
> > - *ras_if = kmalloc(sizeof(**ras_if), GFP_KERNEL);
> > - if (!*ras_if)
> > + ras_if = kmalloc(sizeof(*ras_if), GFP_KERNEL);
> > + if (!ras_if)
> > return -ENOMEM;
> >
> > - **ras_if = ras_block;
> > + *ras_if = ras_block;
> >
> > - r = amdgpu_ras_feature_enable(adev, *ras_if, 1);
> > + r = amdgpu_ras_feature_enable(adev, ras_if, 1);
> > if (r)
> > goto feature;
> >
> > - ih_info.head = **ras_if;
> > - fs_info.head = **ras_if;
> > + ih_info.head = *ras_if;
> > + fs_info.head = *ras_if;
> >
> > r = amdgpu_ras_interrupt_add_handler(adev, &ih_info);
> > if (r)
> > @@ -769,16 +769,16 @@ static int gmc_v9_0_ecc_late_init(void *handle)
> >
> > return 0;
> > irq:
> > - amdgpu_ras_sysfs_remove(adev, *ras_if);
> > + amdgpu_ras_sysfs_remove(adev, ras_if);
> > sysfs:
> > - amdgpu_ras_debugfs_remove(adev, *ras_if);
> > + amdgpu_ras_debugfs_remove(adev, ras_if);
> > debugfs:
> > amdgpu_ras_interrupt_remove_handler(adev, &ih_info);
> > interrupt:
> > - amdgpu_ras_feature_enable(adev, *ras_if, 0);
> > + amdgpu_ras_feature_enable(adev, ras_if, 0);
> > feature:
> > - kfree(*ras_if);
> > - *ras_if = NULL;
> > + kfree(ras_if);
> > + ras_if = NULL;
> > return -EINVAL;
> > }
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> > b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> > index 3ac5abe937f4..521218053477 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> > @@ -1501,7 +1501,7 @@ static int sdma_v4_0_process_ras_data_cb(struct
> > amdgpu_device *adev, static int sdma_v4_0_late_init(void *handle) {
> > struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> > - struct ras_common_if **ras_if = &adev->sdma.ras_if;
> > + struct ras_common_if *ras_if = adev->sdma.ras_if;
> > struct ras_ih_if ih_info = {
> > .cb = sdma_v4_0_process_ras_data_cb,
> > };
> > @@ -1523,21 +1523,21 @@ static int sdma_v4_0_late_init(void *handle)
> > }
> >
> > /* handle resume path. */
> > - if (*ras_if)
> > + if (ras_if)
> > goto resume;
> >
> > - *ras_if = kmalloc(sizeof(**ras_if), GFP_KERNEL);
> > - if (!*ras_if)
> > + ras_if = kmalloc(sizeof(*ras_if), GFP_KERNEL);
> > + if (!ras_if)
> > return -ENOMEM;
> >
> > - **ras_if = ras_block;
> > + *ras_if = ras_block;
> >
> > - r = amdgpu_ras_feature_enable(adev, *ras_if, 1);
> > + r = amdgpu_ras_feature_enable(adev, ras_if, 1);
> > if (r)
> > goto feature;
> >
> > - ih_info.head = **ras_if;
> > - fs_info.head = **ras_if;
> > + ih_info.head = *ras_if;
> > + fs_info.head = *ras_if;
> >
> > r = amdgpu_ras_interrupt_add_handler(adev, &ih_info);
> > if (r)
> > @@ -1563,16 +1563,16 @@ static int sdma_v4_0_late_init(void *handle)
> >
> > return 0;
> > irq:
> > - amdgpu_ras_sysfs_remove(adev, *ras_if);
> > + amdgpu_ras_sysfs_remove(adev, ras_if);
> > sysfs:
> > - amdgpu_ras_debugfs_remove(adev, *ras_if);
> > + amdgpu_ras_debugfs_remove(adev, ras_if);
> > debugfs:
> > amdgpu_ras_interrupt_remove_handler(adev, &ih_info);
> > interrupt:
> > - amdgpu_ras_feature_enable(adev, *ras_if, 0);
> > + amdgpu_ras_feature_enable(adev, ras_if, 0);
> > feature:
> > - kfree(*ras_if);
> > - *ras_if = NULL;
> > + kfree(ras_if);
> > + ras_if = NULL;
> > return -EINVAL;
> > }
> >
> > --
> > 2.21.0
> >
> > _______________________________________________
> > amd-gfx mailing list
> > amd-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-03-11 6:20 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-11 4:30 [PATCH 1/2] drm/amdgpu: unify the way to judge RAS feature readiness Evan Quan
[not found] ` <20190311043046.10184-1-evan.quan-5C7GfCeVMHo@public.gmane.org>
2019-03-11 4:30 ` [PATCH 2/2] drm/amdgpu: drop unnecessary dereference Evan Quan
[not found] ` <20190311043046.10184-2-evan.quan-5C7GfCeVMHo@public.gmane.org>
2019-03-11 5:19 ` Pan, Xinhui
[not found] ` <SN6PR12MB280003172F75FEECA9BE96D187480-kxOKjb6HO/EqkY47FTA1ogdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2019-03-11 5:41 ` Quan, Evan
[not found] ` <MN2PR12MB3344AFD285AAD5483D92580CE4480-rweVpJHSKToDMgCC8P//OwdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2019-03-11 5:45 ` Pan, Xinhui
[not found] ` <SN6PR12MB28008BD6E89295354485D56B87480-kxOKjb6HO/EqkY47FTA1ogdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2019-03-11 6:20 ` Quan, Evan
2019-03-11 5:08 ` [PATCH 1/2] drm/amdgpu: unify the way to judge RAS feature readiness Pan, Xinhui
[not found] ` <SN6PR12MB28007A2E394FFDC0A570959A87480-kxOKjb6HO/EqkY47FTA1ogdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2019-03-11 5:27 ` Quan, Evan
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.