RE: [PATCH] drm/amdgpu: fix amdgpu pmu to use hwc->config instead of hwc->conf

["Kim, Jonathan" <Jonathan.Kim@amd.com>]

[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