All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Revert "drm/amdgpu: fix compiler warnings for df perfmons"
@ 2019-10-18 17:59 Kim, Jonathan
       [not found] ` <20191018175928.25307-1-jonathan.kim-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 4+ messages in thread
From: Kim, Jonathan @ 2019-10-18 17:59 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: Deucher, Alexander, Kuehling, Felix, Kim, Jonathan

This reverts commit 7dd2eb31fcd564574e8efea6bf23cf504f9e2fd7.

revert fix of compiler warning on incomplete df-cstate race condition
handling solution i.e. smu msg cannot be sent within perfevents

Change-Id: Ia09dd24ef91ef75a79a223f72f0cb6a86cd08667
Signed-off-by: Jonathan Kim <Jonathan.Kim@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/df_v3_6.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/df_v3_6.c b/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
index e1cf7e9c616a..f403c62c944e 100644
--- a/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
+++ b/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
@@ -93,7 +93,7 @@ const struct attribute_group *df_v3_6_attr_groups[] = {
 		NULL
 };
 
-static int df_v3_6_set_df_cstate(struct amdgpu_device *adev, int allow)
+static df_v3_6_set_df_cstate(struct amdgpu_device *adev, int allow)
 {
 	int r = 0;
 
@@ -546,7 +546,7 @@ static void df_v3_6_pmc_get_count(struct amdgpu_device *adev,
 				  uint64_t config,
 				  uint64_t *count)
 {
-	uint32_t lo_base_addr, hi_base_addr, lo_val = 0, hi_val = 0;
+	uint32_t lo_base_addr, hi_base_addr, lo_val, hi_val;
 	*count = 0;
 
 	switch (adev->asic_type) {
-- 
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/2] Revert "drm/amdgpu: disable c-states on xgmi perfmons"
       [not found] ` <20191018175928.25307-1-jonathan.kim-5C7GfCeVMHo@public.gmane.org>
@ 2019-10-18 17:59   ` Kim, Jonathan
       [not found]     ` <20191018175928.25307-2-jonathan.kim-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 4+ messages in thread
From: Kim, Jonathan @ 2019-10-18 17:59 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: Deucher, Alexander, Kuehling, Felix, Kim, Jonathan

This reverts commit 54275cd1649f4034c6450b6c5a8358fcd4f7dda6.

incomplete solution to df c-state race condition.  smu msg in perf events
causes deadlock.

Change-Id: Ia85179df2bd167657e42a2d828c4a7c475c392ff
Signed-off-by: Jonathan Kim <Jonathan.Kim@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/df_v3_6.c | 36 +---------------------------
 1 file changed, 1 insertion(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/df_v3_6.c b/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
index f403c62c944e..16fbd2bc8ad1 100644
--- a/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
+++ b/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
@@ -93,21 +93,6 @@ const struct attribute_group *df_v3_6_attr_groups[] = {
 		NULL
 };
 
-static df_v3_6_set_df_cstate(struct amdgpu_device *adev, int allow)
-{
-	int r = 0;
-
-	if (is_support_sw_smu(adev)) {
-		r = smu_set_df_cstate(&adev->smu, allow);
-	} else if (adev->powerplay.pp_funcs
-			&& adev->powerplay.pp_funcs->set_df_cstate) {
-		r = adev->powerplay.pp_funcs->set_df_cstate(
-			adev->powerplay.pp_handle, allow);
-	}
-
-	return r;
-}
-
 static uint64_t df_v3_6_get_fica(struct amdgpu_device *adev,
 				 uint32_t ficaa_val)
 {
@@ -117,9 +102,6 @@ static uint64_t df_v3_6_get_fica(struct amdgpu_device *adev,
 	address = adev->nbio.funcs->get_pcie_index_offset(adev);
 	data = adev->nbio.funcs->get_pcie_data_offset(adev);
 
-	if (df_v3_6_set_df_cstate(adev, DF_CSTATE_DISALLOW))
-		return 0xFFFFFFFFFFFFFFFF;
-
 	spin_lock_irqsave(&adev->pcie_idx_lock, flags);
 	WREG32(address, smnDF_PIE_AON_FabricIndirectConfigAccessAddress3);
 	WREG32(data, ficaa_val);
@@ -132,8 +114,6 @@ static uint64_t df_v3_6_get_fica(struct amdgpu_device *adev,
 
 	spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
 
-	df_v3_6_set_df_cstate(adev, DF_CSTATE_ALLOW);
-
 	return (((ficadh_val & 0xFFFFFFFFFFFFFFFF) << 32) | ficadl_val);
 }
 
@@ -145,9 +125,6 @@ static void df_v3_6_set_fica(struct amdgpu_device *adev, uint32_t ficaa_val,
 	address = adev->nbio.funcs->get_pcie_index_offset(adev);
 	data = adev->nbio.funcs->get_pcie_data_offset(adev);
 
-	if (df_v3_6_set_df_cstate(adev, DF_CSTATE_DISALLOW))
-		return;
-
 	spin_lock_irqsave(&adev->pcie_idx_lock, flags);
 	WREG32(address, smnDF_PIE_AON_FabricIndirectConfigAccessAddress3);
 	WREG32(data, ficaa_val);
@@ -157,9 +134,8 @@ static void df_v3_6_set_fica(struct amdgpu_device *adev, uint32_t ficaa_val,
 
 	WREG32(address, smnDF_PIE_AON_FabricIndirectConfigAccessDataHi3);
 	WREG32(data, ficadh_val);
-	spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
 
-	df_v3_6_set_df_cstate(adev, DF_CSTATE_ALLOW);
+	spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
 }
 
 /*
@@ -177,17 +153,12 @@ static void df_v3_6_perfmon_rreg(struct amdgpu_device *adev,
 	address = adev->nbio.funcs->get_pcie_index_offset(adev);
 	data = adev->nbio.funcs->get_pcie_data_offset(adev);
 
-	if (df_v3_6_set_df_cstate(adev, DF_CSTATE_DISALLOW))
-		return;
-
 	spin_lock_irqsave(&adev->pcie_idx_lock, flags);
 	WREG32(address, lo_addr);
 	*lo_val = RREG32(data);
 	WREG32(address, hi_addr);
 	*hi_val = RREG32(data);
 	spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
-
-	df_v3_6_set_df_cstate(adev, DF_CSTATE_ALLOW);
 }
 
 /*
@@ -204,17 +175,12 @@ static void df_v3_6_perfmon_wreg(struct amdgpu_device *adev, uint32_t lo_addr,
 	address = adev->nbio.funcs->get_pcie_index_offset(adev);
 	data = adev->nbio.funcs->get_pcie_data_offset(adev);
 
-	if (df_v3_6_set_df_cstate(adev, DF_CSTATE_DISALLOW))
-		return;
-
 	spin_lock_irqsave(&adev->pcie_idx_lock, flags);
 	WREG32(address, lo_addr);
 	WREG32(data, lo_val);
 	WREG32(address, hi_addr);
 	WREG32(data, hi_val);
 	spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
-
-	df_v3_6_set_df_cstate(adev, DF_CSTATE_ALLOW);
 }
 
 /* get the number of df counters available */
-- 
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 2/2] Revert "drm/amdgpu: disable c-states on xgmi perfmons"
       [not found]     ` <20191018175928.25307-2-jonathan.kim-5C7GfCeVMHo@public.gmane.org>
@ 2019-10-18 19:18       ` Kuehling, Felix
  0 siblings, 0 replies; 4+ messages in thread
From: Kuehling, Felix @ 2019-10-18 19:18 UTC (permalink / raw)
  To: Kim, Jonathan, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: Deucher, Alexander

You can squash the two reverts into a single commit so you avoid 
reintroducing a broken intermediate state. Mention both reverted commits 
in the squashed commit description. Checkpatch.pl prefers a different 
format for quoting reverted commits. Run checkpatch.pl on your commit to 
see a proper example.

Regards,
   Felix


On 2019-10-18 1:59 p.m., Kim, Jonathan wrote:
> This reverts commit 54275cd1649f4034c6450b6c5a8358fcd4f7dda6.
>
> incomplete solution to df c-state race condition.  smu msg in perf events
> causes deadlock.
>
> Change-Id: Ia85179df2bd167657e42a2d828c4a7c475c392ff
> Signed-off-by: Jonathan Kim <Jonathan.Kim@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/df_v3_6.c | 36 +---------------------------
>   1 file changed, 1 insertion(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/df_v3_6.c b/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
> index f403c62c944e..16fbd2bc8ad1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
> +++ b/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
> @@ -93,21 +93,6 @@ const struct attribute_group *df_v3_6_attr_groups[] = {
>   		NULL
>   };
>   
> -static df_v3_6_set_df_cstate(struct amdgpu_device *adev, int allow)
> -{
> -	int r = 0;
> -
> -	if (is_support_sw_smu(adev)) {
> -		r = smu_set_df_cstate(&adev->smu, allow);
> -	} else if (adev->powerplay.pp_funcs
> -			&& adev->powerplay.pp_funcs->set_df_cstate) {
> -		r = adev->powerplay.pp_funcs->set_df_cstate(
> -			adev->powerplay.pp_handle, allow);
> -	}
> -
> -	return r;
> -}
> -
>   static uint64_t df_v3_6_get_fica(struct amdgpu_device *adev,
>   				 uint32_t ficaa_val)
>   {
> @@ -117,9 +102,6 @@ static uint64_t df_v3_6_get_fica(struct amdgpu_device *adev,
>   	address = adev->nbio.funcs->get_pcie_index_offset(adev);
>   	data = adev->nbio.funcs->get_pcie_data_offset(adev);
>   
> -	if (df_v3_6_set_df_cstate(adev, DF_CSTATE_DISALLOW))
> -		return 0xFFFFFFFFFFFFFFFF;
> -
>   	spin_lock_irqsave(&adev->pcie_idx_lock, flags);
>   	WREG32(address, smnDF_PIE_AON_FabricIndirectConfigAccessAddress3);
>   	WREG32(data, ficaa_val);
> @@ -132,8 +114,6 @@ static uint64_t df_v3_6_get_fica(struct amdgpu_device *adev,
>   
>   	spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
>   
> -	df_v3_6_set_df_cstate(adev, DF_CSTATE_ALLOW);
> -
>   	return (((ficadh_val & 0xFFFFFFFFFFFFFFFF) << 32) | ficadl_val);
>   }
>   
> @@ -145,9 +125,6 @@ static void df_v3_6_set_fica(struct amdgpu_device *adev, uint32_t ficaa_val,
>   	address = adev->nbio.funcs->get_pcie_index_offset(adev);
>   	data = adev->nbio.funcs->get_pcie_data_offset(adev);
>   
> -	if (df_v3_6_set_df_cstate(adev, DF_CSTATE_DISALLOW))
> -		return;
> -
>   	spin_lock_irqsave(&adev->pcie_idx_lock, flags);
>   	WREG32(address, smnDF_PIE_AON_FabricIndirectConfigAccessAddress3);
>   	WREG32(data, ficaa_val);
> @@ -157,9 +134,8 @@ static void df_v3_6_set_fica(struct amdgpu_device *adev, uint32_t ficaa_val,
>   
>   	WREG32(address, smnDF_PIE_AON_FabricIndirectConfigAccessDataHi3);
>   	WREG32(data, ficadh_val);
> -	spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
>   
> -	df_v3_6_set_df_cstate(adev, DF_CSTATE_ALLOW);
> +	spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
>   }
>   
>   /*
> @@ -177,17 +153,12 @@ static void df_v3_6_perfmon_rreg(struct amdgpu_device *adev,
>   	address = adev->nbio.funcs->get_pcie_index_offset(adev);
>   	data = adev->nbio.funcs->get_pcie_data_offset(adev);
>   
> -	if (df_v3_6_set_df_cstate(adev, DF_CSTATE_DISALLOW))
> -		return;
> -
>   	spin_lock_irqsave(&adev->pcie_idx_lock, flags);
>   	WREG32(address, lo_addr);
>   	*lo_val = RREG32(data);
>   	WREG32(address, hi_addr);
>   	*hi_val = RREG32(data);
>   	spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
> -
> -	df_v3_6_set_df_cstate(adev, DF_CSTATE_ALLOW);
>   }
>   
>   /*
> @@ -204,17 +175,12 @@ static void df_v3_6_perfmon_wreg(struct amdgpu_device *adev, uint32_t lo_addr,
>   	address = adev->nbio.funcs->get_pcie_index_offset(adev);
>   	data = adev->nbio.funcs->get_pcie_data_offset(adev);
>   
> -	if (df_v3_6_set_df_cstate(adev, DF_CSTATE_DISALLOW))
> -		return;
> -
>   	spin_lock_irqsave(&adev->pcie_idx_lock, flags);
>   	WREG32(address, lo_addr);
>   	WREG32(data, lo_val);
>   	WREG32(address, hi_addr);
>   	WREG32(data, hi_val);
>   	spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
> -
> -	df_v3_6_set_df_cstate(adev, DF_CSTATE_ALLOW);
>   }
>   
>   /* get the number of df counters available */
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 2/2] Revert "drm/amdgpu: disable c-states on xgmi perfmons"
       [not found] ` <20191018172934.24716-1-jonathan.kim-5C7GfCeVMHo@public.gmane.org>
@ 2019-10-18 17:30   ` Kim, Jonathan
  0 siblings, 0 replies; 4+ messages in thread
From: Kim, Jonathan @ 2019-10-18 17:30 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Kuehling, Felix, Kim, Jonathan

This reverts commit 54275cd1649f4034c6450b6c5a8358fcd4f7dda6.

Signed-off-by: Jonathan Kim <Jonathan.Kim@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/df_v3_6.c | 36 +---------------------------
 1 file changed, 1 insertion(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/df_v3_6.c b/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
index f403c62c944e..16fbd2bc8ad1 100644
--- a/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
+++ b/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
@@ -93,21 +93,6 @@ const struct attribute_group *df_v3_6_attr_groups[] = {
 		NULL
 };
 
-static df_v3_6_set_df_cstate(struct amdgpu_device *adev, int allow)
-{
-	int r = 0;
-
-	if (is_support_sw_smu(adev)) {
-		r = smu_set_df_cstate(&adev->smu, allow);
-	} else if (adev->powerplay.pp_funcs
-			&& adev->powerplay.pp_funcs->set_df_cstate) {
-		r = adev->powerplay.pp_funcs->set_df_cstate(
-			adev->powerplay.pp_handle, allow);
-	}
-
-	return r;
-}
-
 static uint64_t df_v3_6_get_fica(struct amdgpu_device *adev,
 				 uint32_t ficaa_val)
 {
@@ -117,9 +102,6 @@ static uint64_t df_v3_6_get_fica(struct amdgpu_device *adev,
 	address = adev->nbio.funcs->get_pcie_index_offset(adev);
 	data = adev->nbio.funcs->get_pcie_data_offset(adev);
 
-	if (df_v3_6_set_df_cstate(adev, DF_CSTATE_DISALLOW))
-		return 0xFFFFFFFFFFFFFFFF;
-
 	spin_lock_irqsave(&adev->pcie_idx_lock, flags);
 	WREG32(address, smnDF_PIE_AON_FabricIndirectConfigAccessAddress3);
 	WREG32(data, ficaa_val);
@@ -132,8 +114,6 @@ static uint64_t df_v3_6_get_fica(struct amdgpu_device *adev,
 
 	spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
 
-	df_v3_6_set_df_cstate(adev, DF_CSTATE_ALLOW);
-
 	return (((ficadh_val & 0xFFFFFFFFFFFFFFFF) << 32) | ficadl_val);
 }
 
@@ -145,9 +125,6 @@ static void df_v3_6_set_fica(struct amdgpu_device *adev, uint32_t ficaa_val,
 	address = adev->nbio.funcs->get_pcie_index_offset(adev);
 	data = adev->nbio.funcs->get_pcie_data_offset(adev);
 
-	if (df_v3_6_set_df_cstate(adev, DF_CSTATE_DISALLOW))
-		return;
-
 	spin_lock_irqsave(&adev->pcie_idx_lock, flags);
 	WREG32(address, smnDF_PIE_AON_FabricIndirectConfigAccessAddress3);
 	WREG32(data, ficaa_val);
@@ -157,9 +134,8 @@ static void df_v3_6_set_fica(struct amdgpu_device *adev, uint32_t ficaa_val,
 
 	WREG32(address, smnDF_PIE_AON_FabricIndirectConfigAccessDataHi3);
 	WREG32(data, ficadh_val);
-	spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
 
-	df_v3_6_set_df_cstate(adev, DF_CSTATE_ALLOW);
+	spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
 }
 
 /*
@@ -177,17 +153,12 @@ static void df_v3_6_perfmon_rreg(struct amdgpu_device *adev,
 	address = adev->nbio.funcs->get_pcie_index_offset(adev);
 	data = adev->nbio.funcs->get_pcie_data_offset(adev);
 
-	if (df_v3_6_set_df_cstate(adev, DF_CSTATE_DISALLOW))
-		return;
-
 	spin_lock_irqsave(&adev->pcie_idx_lock, flags);
 	WREG32(address, lo_addr);
 	*lo_val = RREG32(data);
 	WREG32(address, hi_addr);
 	*hi_val = RREG32(data);
 	spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
-
-	df_v3_6_set_df_cstate(adev, DF_CSTATE_ALLOW);
 }
 
 /*
@@ -204,17 +175,12 @@ static void df_v3_6_perfmon_wreg(struct amdgpu_device *adev, uint32_t lo_addr,
 	address = adev->nbio.funcs->get_pcie_index_offset(adev);
 	data = adev->nbio.funcs->get_pcie_data_offset(adev);
 
-	if (df_v3_6_set_df_cstate(adev, DF_CSTATE_DISALLOW))
-		return;
-
 	spin_lock_irqsave(&adev->pcie_idx_lock, flags);
 	WREG32(address, lo_addr);
 	WREG32(data, lo_val);
 	WREG32(address, hi_addr);
 	WREG32(data, hi_val);
 	spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
-
-	df_v3_6_set_df_cstate(adev, DF_CSTATE_ALLOW);
 }
 
 /* get the number of df counters available */
-- 
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-10-18 19:18 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-18 17:59 [PATCH] Revert "drm/amdgpu: fix compiler warnings for df perfmons" Kim, Jonathan
     [not found] ` <20191018175928.25307-1-jonathan.kim-5C7GfCeVMHo@public.gmane.org>
2019-10-18 17:59   ` [PATCH 2/2] Revert "drm/amdgpu: disable c-states on xgmi perfmons" Kim, Jonathan
     [not found]     ` <20191018175928.25307-2-jonathan.kim-5C7GfCeVMHo@public.gmane.org>
2019-10-18 19:18       ` Kuehling, Felix
  -- strict thread matches above, loose matches on Subject: below --
2019-10-18 17:30 [PATCH 1/2] Revert "drm/amdgpu: fix compiler warnings for df perfmons" Kim, Jonathan
     [not found] ` <20191018172934.24716-1-jonathan.kim-5C7GfCeVMHo@public.gmane.org>
2019-10-18 17:30   ` [PATCH 2/2] Revert "drm/amdgpu: disable c-states on xgmi perfmons" Kim, Jonathan

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.