All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/amdgpu: fix amdgpu pmu to use hwc->config instead of hwc->conf
@ 2020-02-06 17:08 Jonathan Kim
  2020-02-06 20:08 ` Felix Kuehling
  0 siblings, 1 reply; 3+ messages in thread
From: Jonathan Kim @ 2020-02-06 17:08 UTC (permalink / raw)
  To: amd-gfx; +Cc: Felix.Kuehling, Jonathan Kim

hwc->conf was designated specifically for AMD APU IOMMU purposes.  This
could cause problems in performance and/or function since APU IOMMU
implementation is elsewhere.

Signed-off-by: Jonathan Kim <Jonathan.Kim@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
index 07914e34bc25..1311d6aec5d4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
@@ -52,7 +52,7 @@ static int amdgpu_perf_event_init(struct perf_event *event)
 		return -ENOENT;
 
 	/* update the hw_perf_event struct with config data */
-	hwc->conf = event->attr.config;
+	hwc->config = event->attr.config;
 
 	return 0;
 }
@@ -74,9 +74,9 @@ static void amdgpu_perf_start(struct perf_event *event, int flags)
 	switch (pe->pmu_perf_type) {
 	case PERF_TYPE_AMDGPU_DF:
 		if (!(flags & PERF_EF_RELOAD))
-			pe->adev->df.funcs->pmc_start(pe->adev, hwc->conf, 1);
+			pe->adev->df.funcs->pmc_start(pe->adev, hwc->config, 1);
 
-		pe->adev->df.funcs->pmc_start(pe->adev, hwc->conf, 0);
+		pe->adev->df.funcs->pmc_start(pe->adev, hwc->config, 0);
 		break;
 	default:
 		break;
@@ -101,7 +101,7 @@ static void amdgpu_perf_read(struct perf_event *event)
 
 		switch (pe->pmu_perf_type) {
 		case PERF_TYPE_AMDGPU_DF:
-			pe->adev->df.funcs->pmc_get_count(pe->adev, hwc->conf,
+			pe->adev->df.funcs->pmc_get_count(pe->adev, hwc->config,
 							  &count);
 			break;
 		default:
@@ -126,7 +126,7 @@ static void amdgpu_perf_stop(struct perf_event *event, int flags)
 
 	switch (pe->pmu_perf_type) {
 	case PERF_TYPE_AMDGPU_DF:
-		pe->adev->df.funcs->pmc_stop(pe->adev, hwc->conf, 0);
+		pe->adev->df.funcs->pmc_stop(pe->adev, hwc->config, 0);
 		break;
 	default:
 		break;
@@ -156,7 +156,8 @@ static int amdgpu_perf_add(struct perf_event *event, int flags)
 
 	switch (pe->pmu_perf_type) {
 	case PERF_TYPE_AMDGPU_DF:
-		retval = pe->adev->df.funcs->pmc_start(pe->adev, hwc->conf, 1);
+		retval = pe->adev->df.funcs->pmc_start(pe->adev,
+						       hwc->config, 1);
 		break;
 	default:
 		return 0;
@@ -184,7 +185,7 @@ static void amdgpu_perf_del(struct perf_event *event, int flags)
 
 	switch (pe->pmu_perf_type) {
 	case PERF_TYPE_AMDGPU_DF:
-		pe->adev->df.funcs->pmc_stop(pe->adev, hwc->conf, 1);
+		pe->adev->df.funcs->pmc_stop(pe->adev, hwc->config, 1);
 		break;
 	default:
 		break;
-- 
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] 3+ messages in thread
* Re: [PATCH] drm/amdgpu: fix amdgpu pmu to use hwc->config instead of hwc->conf
  2020-02-06 17:08 [PATCH] drm/amdgpu: fix amdgpu pmu to use hwc->config instead of hwc->conf Jonathan Kim
@ 2020-02-06 20:08 ` Felix Kuehling
  2020-02-06 20:23   ` Kim, Jonathan
  0 siblings, 1 reply; 3+ messages in thread
From: Felix Kuehling @ 2020-02-06 20:08 UTC (permalink / raw)
  To: Jonathan Kim, amd-gfx

On 2020-02-06 12:08, Jonathan Kim wrote:
> hwc->conf was designated specifically for AMD APU IOMMU purposes.  This
> could cause problems in performance and/or function since APU IOMMU
> implementation is elsewhere.

It's actually worse than that. hwc is a union of anonymous structures. 
hwc->conf and hwc->config are in different members of that union. So 
hwc->conf aliases some other variable in the structure that hwc->config 
is in. If I did the math right, hwc->conf aliases hwc->last_tag.

Anyway, the patch is

Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>


>
> Signed-off-by: Jonathan Kim <Jonathan.Kim@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 15 ++++++++-------
>   1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> index 07914e34bc25..1311d6aec5d4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> @@ -52,7 +52,7 @@ static int amdgpu_perf_event_init(struct perf_event *event)
>   		return -ENOENT;
>   
>   	/* update the hw_perf_event struct with config data */
> -	hwc->conf = event->attr.config;
> +	hwc->config = event->attr.config;
>   
>   	return 0;
>   }
> @@ -74,9 +74,9 @@ static void amdgpu_perf_start(struct perf_event *event, int flags)
>   	switch (pe->pmu_perf_type) {
>   	case PERF_TYPE_AMDGPU_DF:
>   		if (!(flags & PERF_EF_RELOAD))
> -			pe->adev->df.funcs->pmc_start(pe->adev, hwc->conf, 1);
> +			pe->adev->df.funcs->pmc_start(pe->adev, hwc->config, 1);
>   
> -		pe->adev->df.funcs->pmc_start(pe->adev, hwc->conf, 0);
> +		pe->adev->df.funcs->pmc_start(pe->adev, hwc->config, 0);
>   		break;
>   	default:
>   		break;
> @@ -101,7 +101,7 @@ static void amdgpu_perf_read(struct perf_event *event)
>   
>   		switch (pe->pmu_perf_type) {
>   		case PERF_TYPE_AMDGPU_DF:
> -			pe->adev->df.funcs->pmc_get_count(pe->adev, hwc->conf,
> +			pe->adev->df.funcs->pmc_get_count(pe->adev, hwc->config,
>   							  &count);
>   			break;
>   		default:
> @@ -126,7 +126,7 @@ static void amdgpu_perf_stop(struct perf_event *event, int flags)
>   
>   	switch (pe->pmu_perf_type) {
>   	case PERF_TYPE_AMDGPU_DF:
> -		pe->adev->df.funcs->pmc_stop(pe->adev, hwc->conf, 0);
> +		pe->adev->df.funcs->pmc_stop(pe->adev, hwc->config, 0);
>   		break;
>   	default:
>   		break;
> @@ -156,7 +156,8 @@ static int amdgpu_perf_add(struct perf_event *event, int flags)
>   
>   	switch (pe->pmu_perf_type) {
>   	case PERF_TYPE_AMDGPU_DF:
> -		retval = pe->adev->df.funcs->pmc_start(pe->adev, hwc->conf, 1);
> +		retval = pe->adev->df.funcs->pmc_start(pe->adev,
> +						       hwc->config, 1);
>   		break;
>   	default:
>   		return 0;
> @@ -184,7 +185,7 @@ static void amdgpu_perf_del(struct perf_event *event, int flags)
>   
>   	switch (pe->pmu_perf_type) {
>   	case PERF_TYPE_AMDGPU_DF:
> -		pe->adev->df.funcs->pmc_stop(pe->adev, hwc->conf, 1);
> +		pe->adev->df.funcs->pmc_stop(pe->adev, hwc->config, 1);
>   		break;
>   	default:
>   		break;
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 3+ messages in thread
* RE: [PATCH] drm/amdgpu: fix amdgpu pmu to use hwc->config instead of hwc->conf
  2020-02-06 20:08 ` Felix Kuehling
@ 2020-02-06 20:23   ` Kim, Jonathan
  0 siblings, 0 replies; 3+ messages in thread
From: Kim, Jonathan @ 2020-02-06 20:23 UTC (permalink / raw)
  To: Kuehling, Felix, amd-gfx

[AMD Official Use Only - Approved for External Use]

Thanks for pointing that out Felix.  I'll append that as well to the comments for the commit.

Jon

-----Original Message-----
From: Kuehling, Felix <Felix.Kuehling@amd.com> 
Sent: Thursday, February 6, 2020 3:08 PM
To: Kim, Jonathan <Jonathan.Kim@amd.com>; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amdgpu: fix amdgpu pmu to use hwc->config instead of hwc->conf

On 2020-02-06 12:08, Jonathan Kim wrote:
> hwc->conf was designated specifically for AMD APU IOMMU purposes.  
> hwc->This
> could cause problems in performance and/or function since APU IOMMU 
> implementation is elsewhere.

It's actually worse than that. hwc is a union of anonymous structures. 
hwc->conf and hwc->config are in different members of that union. So 
hwc->conf aliases some other variable in the structure that hwc->config
is in. If I did the math right, hwc->conf aliases hwc->last_tag.

Anyway, the patch is

Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>


>
> Signed-off-by: Jonathan Kim <Jonathan.Kim@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 15 ++++++++-------
>   1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> index 07914e34bc25..1311d6aec5d4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> @@ -52,7 +52,7 @@ static int amdgpu_perf_event_init(struct perf_event *event)
>   		return -ENOENT;
>   
>   	/* update the hw_perf_event struct with config data */
> -	hwc->conf = event->attr.config;
> +	hwc->config = event->attr.config;
>   
>   	return 0;
>   }
> @@ -74,9 +74,9 @@ static void amdgpu_perf_start(struct perf_event *event, int flags)
>   	switch (pe->pmu_perf_type) {
>   	case PERF_TYPE_AMDGPU_DF:
>   		if (!(flags & PERF_EF_RELOAD))
> -			pe->adev->df.funcs->pmc_start(pe->adev, hwc->conf, 1);
> +			pe->adev->df.funcs->pmc_start(pe->adev, hwc->config, 1);
>   
> -		pe->adev->df.funcs->pmc_start(pe->adev, hwc->conf, 0);
> +		pe->adev->df.funcs->pmc_start(pe->adev, hwc->config, 0);
>   		break;
>   	default:
>   		break;
> @@ -101,7 +101,7 @@ static void amdgpu_perf_read(struct perf_event *event)
>   
>   		switch (pe->pmu_perf_type) {
>   		case PERF_TYPE_AMDGPU_DF:
> -			pe->adev->df.funcs->pmc_get_count(pe->adev, hwc->conf,
> +			pe->adev->df.funcs->pmc_get_count(pe->adev, hwc->config,
>   							  &count);
>   			break;
>   		default:
> @@ -126,7 +126,7 @@ static void amdgpu_perf_stop(struct perf_event *event, int flags)
>   
>   	switch (pe->pmu_perf_type) {
>   	case PERF_TYPE_AMDGPU_DF:
> -		pe->adev->df.funcs->pmc_stop(pe->adev, hwc->conf, 0);
> +		pe->adev->df.funcs->pmc_stop(pe->adev, hwc->config, 0);
>   		break;
>   	default:
>   		break;
> @@ -156,7 +156,8 @@ static int amdgpu_perf_add(struct perf_event *event, int flags)
>   
>   	switch (pe->pmu_perf_type) {
>   	case PERF_TYPE_AMDGPU_DF:
> -		retval = pe->adev->df.funcs->pmc_start(pe->adev, hwc->conf, 1);
> +		retval = pe->adev->df.funcs->pmc_start(pe->adev,
> +						       hwc->config, 1);
>   		break;
>   	default:
>   		return 0;
> @@ -184,7 +185,7 @@ static void amdgpu_perf_del(struct perf_event *event, int flags)
>   
>   	switch (pe->pmu_perf_type) {
>   	case PERF_TYPE_AMDGPU_DF:
> -		pe->adev->df.funcs->pmc_stop(pe->adev, hwc->conf, 1);
> +		pe->adev->df.funcs->pmc_stop(pe->adev, hwc->config, 1);
>   		break;
>   	default:
>   		break;
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-02-06 20:23 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-06 17:08 [PATCH] drm/amdgpu: fix amdgpu pmu to use hwc->config instead of hwc->conf Jonathan Kim
2020-02-06 20:08 ` Felix Kuehling
2020-02-06 20:23   ` 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.