[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@xxxxxxx>
Sent: Thursday, February 6, 2020 3:08 PM
To: Kim, Jonathan <Jonathan.Kim@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
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@xxxxxxx>
>
> Signed-off-by: Jonathan Kim <Jonathan.Kim@xxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
