From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?UTF-8?Q?Christian_K=c3=b6nig?= Subject: Re: [PART1 PATCH v3 7/8] drm/amdgpu: add get clockgating_state method for uvd v5&v6 Date: Fri, 6 Jan 2017 14:45:45 +0100 Message-ID: <1e6d1ea8-00b4-2af2-bedd-c1abaa5d24a6@amd.com> References: <1483699875-27734-1-git-send-email-ray.huang@amd.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1837774609==" Return-path: In-Reply-To: List-Id: Discussion list for AMD gfx List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org Sender: "amd-gfx" To: "StDenis, Tom" , "Huang, Ray" , "Deucher, Alexander" , "amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org" Cc: "Kuehling, Felix" , "Mao, David" , "Zhu, Rex" , "Fu, Ping" , "Zhang, Hawking" --===============1837774609== Content-Type: multipart/alternative; boundary="------------B4969AFE975741A4424325C7" --------------B4969AFE975741A4424325C7 Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 8bit Good point, I wasn't ware that the pm mutex still existed and is taken under all code flows. Christian. Am 06.01.2017 um 14:42 schrieb StDenis, Tom: > > There's already the adev->pm.mutex which is held while gating/ungating > blocks. There's no need for a second mutex right? > > > Tom > > > > ------------------------------------------------------------------------ > *From:* amd-gfx on behalf of > Christian König > *Sent:* Friday, January 6, 2017 07:52 > *To:* Huang, Ray; Deucher, Alexander; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org > *Cc:* Kuehling, Felix; Zhang, Hawking; Zhu, Rex; Fu, Ping; Mao, David > *Subject:* Re: [PART1 PATCH v3 7/8] drm/amdgpu: add get > clockgating_state method for uvd v5&v6 > Am 06.01.2017 um 11:51 schrieb Huang Rui: > > Signed-off-by: Huang Rui > > --- > > > > Changes from V2 -> V3: > > - add mutex to protect the is_powergated flag. > > > > --- > > drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 ++ > > drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c | 37 > +++++++++++++++++++++++++++++++++-- > > drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c | 37 > +++++++++++++++++++++++++++++++++-- > > 3 files changed, 72 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h > b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > > index 530549b..afb3ded 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > > @@ -1183,6 +1183,8 @@ struct amdgpu_uvd { > > bool use_ctx_buf; > > struct amd_sched_entity entity; > > uint32_t srbm_soft_reset; > > + bool is_powergated; > > + struct mutex pg_mutex; > > You are missing a mutex_init for pg_mutex, probably best to put this > into amdgpu_uvd_sw_init(). > > Apart from that the patch looks good to me now. > > Regards, > Christian. > > > }; > > > > /* > > diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c > b/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c > > index 03a35d9..bf797b8 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c > > +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c > > @@ -781,16 +781,48 @@ static int uvd_v5_0_set_powergating_state(void > *handle, > > * the smc and the hw blocks > > */ > > struct amdgpu_device *adev = (struct amdgpu_device *)handle; > > + int ret = 0; > > > > if (!(adev->pg_flags & AMD_PG_SUPPORT_UVD)) > > return 0; > > > > + mutex_lock(&adev->uvd.pg_mutex); > > + > > if (state == AMD_PG_STATE_GATE) { > > + adev->uvd.is_powergated = true; > > uvd_v5_0_stop(adev); > > - return 0; > > } else { > > - return uvd_v5_0_start(adev); > > + ret = uvd_v5_0_start(adev); > > + if (ret) > > + goto out; > > + adev->uvd.is_powergated = false; > > + } > > + > > +out: > > + mutex_unlock(&adev->uvd.pg_mutex); > > + > > + return ret; > > +} > > + > > +static void uvd_v5_0_get_clockgating_state(void *handle, u32 *flags) > > +{ > > + struct amdgpu_device *adev = (struct amdgpu_device *)handle; > > + int data; > > + > > + mutex_lock(&adev->uvd.pg_mutex); > > + > > + if (adev->uvd.is_powergated) { > > + DRM_INFO("Cannot get clockgating state when UVD is > powergated.\n"); > > + goto out; > > } > > + > > + /* AMD_CG_SUPPORT_UVD_MGCG */ > > + data = RREG32(mmUVD_CGC_CTRL); > > + if (data & UVD_CGC_CTRL__DYN_CLOCK_MODE_MASK) > > + *flags |= AMD_CG_SUPPORT_UVD_MGCG; > > + > > +out: > > + mutex_unlock(&adev->uvd.pg_mutex); > > } > > > > static const struct amd_ip_funcs uvd_v5_0_ip_funcs = { > > @@ -808,6 +840,7 @@ static const struct amd_ip_funcs > uvd_v5_0_ip_funcs = { > > .soft_reset = uvd_v5_0_soft_reset, > > .set_clockgating_state = uvd_v5_0_set_clockgating_state, > > .set_powergating_state = uvd_v5_0_set_powergating_state, > > + .get_clockgating_state = uvd_v5_0_get_clockgating_state, > > }; > > > > static const struct amdgpu_ring_funcs uvd_v5_0_ring_funcs = { > > diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c > b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c > > index 8779d4b..0ec692d 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c > > +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c > > @@ -987,18 +987,50 @@ static int uvd_v6_0_set_powergating_state(void > *handle, > > * the smc and the hw blocks > > */ > > struct amdgpu_device *adev = (struct amdgpu_device *)handle; > > + int ret = 0; > > > > if (!(adev->pg_flags & AMD_PG_SUPPORT_UVD)) > > return 0; > > > > WREG32(mmUVD_POWER_STATUS, UVD_POWER_STATUS__UVD_PG_EN_MASK); > > > > + mutex_lock(&adev->uvd.pg_mutex); > > + > > if (state == AMD_PG_STATE_GATE) { > > + adev->uvd.is_powergated = true; > > uvd_v6_0_stop(adev); > > - return 0; > > } else { > > - return uvd_v6_0_start(adev); > > + ret = uvd_v6_0_start(adev); > > + if (ret) > > + goto out; > > + adev->uvd.is_powergated = false; > > + } > > + > > +out: > > + mutex_unlock(&adev->uvd.pg_mutex); > > + > > + return ret; > > +} > > + > > +static void uvd_v6_0_get_clockgating_state(void *handle, u32 *flags) > > +{ > > + struct amdgpu_device *adev = (struct amdgpu_device *)handle; > > + int data; > > + > > + mutex_lock(&adev->uvd.pg_mutex); > > + > > + if (adev->uvd.is_powergated) { > > + DRM_INFO("Cannot get clockgating state when UVD is > powergated.\n"); > > + goto out; > > } > > + > > + /* AMD_CG_SUPPORT_UVD_MGCG */ > > + data = RREG32(mmUVD_CGC_CTRL); > > + if (data & UVD_CGC_CTRL__DYN_CLOCK_MODE_MASK) > > + *flags |= AMD_CG_SUPPORT_UVD_MGCG; > > + > > +out: > > + mutex_unlock(&adev->uvd.pg_mutex); > > } > > > > static const struct amd_ip_funcs uvd_v6_0_ip_funcs = { > > @@ -1019,6 +1051,7 @@ static const struct amd_ip_funcs > uvd_v6_0_ip_funcs = { > > .post_soft_reset = uvd_v6_0_post_soft_reset, > > .set_clockgating_state = uvd_v6_0_set_clockgating_state, > > .set_powergating_state = uvd_v6_0_set_powergating_state, > > + .get_clockgating_state = uvd_v6_0_get_clockgating_state, > > }; > > > > static const struct amdgpu_ring_funcs uvd_v6_0_ring_phys_funcs = { > > > _______________________________________________ > amd-gfx mailing list > amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx > amd-gfx Info Page - lists.freedesktop.org > > lists.freedesktop.org > To see the collection of prior postings to the list, visit the amd-gfx > Archives. Using amd-gfx: To post a message to all the list members, > send email ... > > > --------------B4969AFE975741A4424325C7 Content-Type: text/html; charset="windows-1252" Content-Transfer-Encoding: 8bit
Good point, I wasn't ware that the pm mutex still existed and is taken under all code flows.

Christian.

Am 06.01.2017 um 14:42 schrieb StDenis, Tom:

There's already the adev->pm.mutex which is held while gating/ungating blocks.  There's no need for a second mutex right?


Tom



From: amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org> on behalf of Christian König <christian.koenig-5C7GfCeVMHo@public.gmane.org>
Sent: Friday, January 6, 2017 07:52
To: Huang, Ray; Deucher, Alexander; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Cc: Kuehling, Felix; Zhang, Hawking; Zhu, Rex; Fu, Ping; Mao, David
Subject: Re: [PART1 PATCH v3 7/8] drm/amdgpu: add get clockgating_state method for uvd v5&v6
 
Am 06.01.2017 um 11:51 schrieb Huang Rui:
> Signed-off-by: Huang Rui <ray.huang-5C7GfCeVMHo@public.gmane.org>
> ---
>
> Changes from V2 -> V3:
> - add mutex to protect the is_powergated flag.
>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h   |  2 ++
>   drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c | 37 +++++++++++++++++++++++++++++++++--
>   drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c | 37 +++++++++++++++++++++++++++++++++--
>   3 files changed, 72 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 530549b..afb3ded 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1183,6 +1183,8 @@ struct amdgpu_uvd {
>        bool                    use_ctx_buf;
>        struct amd_sched_entity entity;
>        uint32_t                srbm_soft_reset;
> +     bool                    is_powergated;
> +     struct mutex            pg_mutex;

You are missing a mutex_init for pg_mutex, probably best to put this
into amdgpu_uvd_sw_init().

Apart from that the patch looks good to me now.

Regards,
Christian.

>   };
>  
>   /*
> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
> index 03a35d9..bf797b8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
> @@ -781,16 +781,48 @@ static int uvd_v5_0_set_powergating_state(void *handle,
>         * the smc and the hw blocks
>         */
>        struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> +     int ret = 0;
>  
>        if (!(adev->pg_flags & AMD_PG_SUPPORT_UVD))
>                return 0;
>  
> +     mutex_lock(&adev->uvd.pg_mutex);
> +
>        if (state == AMD_PG_STATE_GATE) {
> +             adev->uvd.is_powergated = true;
>                uvd_v5_0_stop(adev);
> -             return 0;
>        } else {
> -             return uvd_v5_0_start(adev);
> +             ret = uvd_v5_0_start(adev);
> +             if (ret)
> +                     goto out;
> +             adev->uvd.is_powergated = false;
> +     }
> +
> +out:
> +     mutex_unlock(&adev->uvd.pg_mutex);
> +
> +     return ret;
> +}
> +
> +static void uvd_v5_0_get_clockgating_state(void *handle, u32 *flags)
> +{
> +     struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> +     int data;
> +
> +     mutex_lock(&adev->uvd.pg_mutex);
> +
> +     if (adev->uvd.is_powergated) {
> +             DRM_INFO("Cannot get clockgating state when UVD is powergated.\n");
> +             goto out;
>        }
> +
> +     /* AMD_CG_SUPPORT_UVD_MGCG */
> +     data = RREG32(mmUVD_CGC_CTRL);
> +     if (data & UVD_CGC_CTRL__DYN_CLOCK_MODE_MASK)
> +             *flags |= AMD_CG_SUPPORT_UVD_MGCG;
> +
> +out:
> +     mutex_unlock(&adev->uvd.pg_mutex);
>   }
>  
>   static const struct amd_ip_funcs uvd_v5_0_ip_funcs = {
> @@ -808,6 +840,7 @@ static const struct amd_ip_funcs uvd_v5_0_ip_funcs = {
>        .soft_reset = uvd_v5_0_soft_reset,
>        .set_clockgating_state = uvd_v5_0_set_clockgating_state,
>        .set_powergating_state = uvd_v5_0_set_powergating_state,
> +     .get_clockgating_state = uvd_v5_0_get_clockgating_state,
>   };
>  
>   static const struct amdgpu_ring_funcs uvd_v5_0_ring_funcs = {
> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> index 8779d4b..0ec692d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> @@ -987,18 +987,50 @@ static int uvd_v6_0_set_powergating_state(void *handle,
>         * the smc and the hw blocks
>         */
>        struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> +     int ret = 0;
>  
>        if (!(adev->pg_flags & AMD_PG_SUPPORT_UVD))
>                return 0;
>  
>        WREG32(mmUVD_POWER_STATUS, UVD_POWER_STATUS__UVD_PG_EN_MASK);
>  
> +     mutex_lock(&adev->uvd.pg_mutex);
> +
>        if (state == AMD_PG_STATE_GATE) {
> +             adev->uvd.is_powergated = true;
>                uvd_v6_0_stop(adev);
> -             return 0;
>        } else {
> -             return uvd_v6_0_start(adev);
> +             ret = uvd_v6_0_start(adev);
> +             if (ret)
> +                     goto out;
> +             adev->uvd.is_powergated = false;
> +     }
> +
> +out:
> +     mutex_unlock(&adev->uvd.pg_mutex);
> +
> +     return ret;
> +}
> +
> +static void uvd_v6_0_get_clockgating_state(void *handle, u32 *flags)
> +{
> +     struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> +     int data;
> +
> +     mutex_lock(&adev->uvd.pg_mutex);
> +
> +     if (adev->uvd.is_powergated) {
> +             DRM_INFO("Cannot get clockgating state when UVD is powergated.\n");
> +             goto out;
>        }
> +
> +     /* AMD_CG_SUPPORT_UVD_MGCG */
> +     data = RREG32(mmUVD_CGC_CTRL);
> +     if (data & UVD_CGC_CTRL__DYN_CLOCK_MODE_MASK)
> +             *flags |= AMD_CG_SUPPORT_UVD_MGCG;
> +
> +out:
> +     mutex_unlock(&adev->uvd.pg_mutex);
>   }
>  
>   static const struct amd_ip_funcs uvd_v6_0_ip_funcs = {
> @@ -1019,6 +1051,7 @@ static const struct amd_ip_funcs uvd_v6_0_ip_funcs = {
>        .post_soft_reset = uvd_v6_0_post_soft_reset,
>        .set_clockgating_state = uvd_v6_0_set_clockgating_state,
>        .set_powergating_state = uvd_v6_0_set_powergating_state,
> +     .get_clockgating_state = uvd_v6_0_get_clockgating_state,
>   };
>  
>   static const struct amdgpu_ring_funcs uvd_v6_0_ring_phys_funcs = {


_______________________________________________
amd-gfx mailing list
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
lists.freedesktop.org
To see the collection of prior postings to the list, visit the amd-gfx Archives. Using amd-gfx: To post a message to all the list members, send email ...



--------------B4969AFE975741A4424325C7-- --===============1837774609== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KYW1kLWdmeCBt YWlsaW5nIGxpc3QKYW1kLWdmeEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5m cmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9hbWQtZ2Z4Cg== --===============1837774609==--