[PATCH] drm/amd/powerplay: don't give up if DPM is already running
Hi Gražvydas, In GPU passthrough case, I can't find better solution than yours from driver. So will apply your patch and just update reproduce step. Thanks. Best Regards Rex -Original Message- From: Zhu, Rex Sent: Wednesday, October 19, 2016 10:00 PM To: 'Grazvydas Ignotas' Cc: Alex Deucher; amd-gfx list; Maling list - DRI developers Subject: RE: [PATCH] drm/amd/powerplay: don't give up if DPM is already running Hi Gražvydas, Sorry for so late response. I can reproduce this issue in gpu passthrough case. When we forced power off the VM or forced reset the VM without shutting down the Os, the rmmod will not be called. So dpm was still running. If we skipped the enable dpm tasks, some clock tables were not be initialized, so kernel panic when we visited those tables in set power state. I will send the fix patch for review tomorrow. Thanks. Best Regards Rex -Original Message- From: Grazvydas Ignotas [mailto:nota...@gmail.com] Sent: Sunday, October 16, 2016 2:55 AM To: Zhu, Rex Cc: Alex Deucher; amd-gfx list; Maling list - DRI developers Subject: Re: [PATCH] drm/amd/powerplay: don't give up if DPM is already running Hi, On Thu, Oct 13, 2016 at 10:45 AM, Zhu, Rex wrote: > > The attached patches were also for this issue. > Disable dpm when rmmod amdgpu. It works for modprobe-rmmod-modprobe test, thanks. However with GPU passthrough (giving control of the GPU to a Windows virtual machine using iommu, then shutting down the VM and loading amdgpu) the problem is still there, same backtrace as in my commit message. It seems the Windows driver leaves DPM enabled on shutdown. With my patch the crash goes away. It would be nice to have GPU passthrough working, this is the only issue that breaks it. Windows can drive the GPU after amdgpu fine already, only amdgpu has problems to run after Windows. Gražvydas -- next part -- A non-text attachment was scrubbed... Name: 0001-drm-amd-powerplay-don-t-give-up-if-DPM-is-already-ru.patch Type: application/octet-stream Size: 1642 bytes Desc: 0001-drm-amd-powerplay-don-t-give-up-if-DPM-is-already-ru.patch URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161020/275e3a07/attachment.obj>
[PATCH] drm/amd/powerplay: don't give up if DPM is already running
Hi Gražvydas, Sorry for so late response. I can reproduce this issue in gpu passthrough case. When we forced power off the VM or forced reset the VM without shutting down the Os, the rmmod will not be called. So dpm was still running. If we skipped the enable dpm tasks, some clock tables were not be initialized, so kernel panic when we visited those tables in set power state. I will send the fix patch for review tomorrow. Thanks. Best Regards Rex -Original Message- From: Grazvydas Ignotas [mailto:nota...@gmail.com] Sent: Sunday, October 16, 2016 2:55 AM To: Zhu, Rex Cc: Alex Deucher; amd-gfx list; Maling list - DRI developers Subject: Re: [PATCH] drm/amd/powerplay: don't give up if DPM is already running Hi, On Thu, Oct 13, 2016 at 10:45 AM, Zhu, Rex wrote: > > The attached patches were also for this issue. > Disable dpm when rmmod amdgpu. It works for modprobe-rmmod-modprobe test, thanks. However with GPU passthrough (giving control of the GPU to a Windows virtual machine using iommu, then shutting down the VM and loading amdgpu) the problem is still there, same backtrace as in my commit message. It seems the Windows driver leaves DPM enabled on shutdown. With my patch the crash goes away. It would be nice to have GPU passthrough working, this is the only issue that breaks it. Windows can drive the GPU after amdgpu fine already, only amdgpu has problems to run after Windows. Gražvydas
[PATCH] drm/amd/powerplay: don't give up if DPM is already running
Hi, On Thu, Oct 13, 2016 at 10:45 AM, Zhu, Rex wrote: > > The attached patches were also for this issue. > Disable dpm when rmmod amdgpu. It works for modprobe-rmmod-modprobe test, thanks. However with GPU passthrough (giving control of the GPU to a Windows virtual machine using iommu, then shutting down the VM and loading amdgpu) the problem is still there, same backtrace as in my commit message. It seems the Windows driver leaves DPM enabled on shutdown. With my patch the crash goes away. It would be nice to have GPU passthrough working, this is the only issue that breaks it. Windows can drive the GPU after amdgpu fine already, only amdgpu has problems to run after Windows. Gražvydas
[PATCH] drm/amd/powerplay: don't give up if DPM is already running
> -Original Message-
> From: amd-gfx [mailto:amd-gfx-bounces at lists.freedesktop.org] On Behalf
> Of Zhu, Rex
> Sent: Friday, October 14, 2016 8:12 AM
> To: Alex Deucher
> Cc: Maling list - DRI developers; amd-gfx list
> Subject: RE: [PATCH] drm/amd/powerplay: don't give up if DPM is already
> running
>
> Hi Alex,
>
> Your new attached patch is Tested-by and Reviewed-by: Rex Zhu
>
>
> Just one question,
>
> Do we need to set clock_gate for smu?
At the moment, no, but if we ever have have clockgating for smu related IPs, we
might as well include it. It matches the current behavior with respect to IP
tear down.
Alex
>
> Best Regards
> Rex
>
>
> -Original Message-
> From: Alex Deucher [mailto:alexdeucher at gmail.com]
> Sent: Thursday, October 13, 2016 11:29 PM
> To: Zhu, Rex
> Cc: Grazvydas Ignotas; amd-gfx list; Maling list - DRI developers
> Subject: Re: [PATCH] drm/amd/powerplay: don't give up if DPM is already
> running
>
> On Thu, Oct 13, 2016 at 3:45 AM, Zhu, Rex wrote:
> > Hi all,
> >
> > The attached patches were also for this issue.
> > Disable dpm when rmmod amdgpu.
> >
> > Please help to review.
>
> Patch 1:
> +r = adev->ip_blocks[AMD_IP_BLOCK_TYPE_SMC].funcs->hw_fini((void
> *)adev);
> +if (r)
> +DRM_DEBUG("hw_fini of IP block <%s> failed %d\n",
> +adev->ip_blocks[AMD_IP_BLOCK_TYPE_SMC].funcs->name, r);
> +
> +adev->ip_block_status[AMD_IP_BLOCK_TYPE_SMC].hw = false;
>
> You can't use AMD_IP_BLOCK_TYPE_* as index. Some chips may not have
> the IP block. Maybe something like the attached patch. That said, I think
> longer term it makes more sense to allows the SOCs to specify the order for
> init, fini, etc. otherwise we'll have lots of variable logic in the common
> code.
>
> Patch 2:
> +if (1 == PHM_READ_VFPF_INDIRECT_FIELD(hwmgr->device,
> CGS_IND_REG__SMC, FEATURE_STATUS, AVS_ON)) {
> +PP_ASSERT_WITH_CODE((0 == smum_send_msg_to_smc(hwmgr-
> >smumgr,
> PPSMC_MSG_DisableAvfs)),
> +"Failed to disable AVFS!",
> +return -1;);
> +}
>
> Use a proper error code there like -EINVAL or something. With that fixed:
> Reviewed-by: Alex Deucher
>
> Alex
>
> >
> > Best Regards
> > Rex
> >
> > -----Original Message-----
> > From: amd-gfx [mailto:amd-gfx-bounces at lists.freedesktop.org] On Behalf
> > Of Zhu, Rex
> > Sent: Wednesday, October 12, 2016 9:45 PM
> > To: Alex Deucher; Grazvydas Ignotas
> > Cc: amd-gfx list; Maling list - DRI developers
> > Subject: RE: [PATCH] drm/amd/powerplay: don't give up if DPM is
> > already running
> >
> > Hi Grazvydas and Alex,
> >
> > We needed to disable dpm when rmmod amdgpu for this issue.
> >
> > I am checking the function of disable dpm task.
> >
> > Best Regards
> > Rex
> >
> > -Original Message-
> > From: Alex Deucher [mailto:alexdeucher at gmail.com]
> > Sent: Wednesday, October 12, 2016 4:01 AM
> > To: Grazvydas Ignotas; Zhu, Rex
> > Cc: Maling list - DRI developers; amd-gfx list
> > Subject: Re: [PATCH] drm/amd/powerplay: don't give up if DPM is
> > already running
> >
> > +Rex to review this.
> >
> > Alex
> >
> > On Sun, Oct 9, 2016 at 3:23 PM, Grazvydas Ignotas
> wrote:
> >> Currently the driver crashes if smu7_enable_dpm_tasks() returns
> >> early, which it does if DPM is already active. It seems to be better
> >> just to continue anyway, at least I haven't noticed any ill effects.
> >> It's also unclear at what state the hardware was left by the previous
> >> driver, so IMO it's better to always fully initialize.
> >>
> >> Way to reproduce:
> >> $ modprobe amdgpu
> >> $ rmmod amdgpu
> >> $ modprobe amdgpu
> >> ...
> >> DPM is already running right now, no need to enable DPM!
> >> ...
> >> failed to send message 18b ret is 0
> >> BUG: unable to handle kernel paging request at ed01fc9ab21f Call
> >> Trace:
> >> smu7_set_power_state_tasks+0x499/0x1940 [amdgpu]
> >> phm_set_power_state+0xcb/0x120 [amdgpu]
> >> psm_adjust_power_state_dynamic+0x11e/0x1b0 [amdgpu]
> >> pem_task_adjust_power_state+0xb9/0xd0 [amdgpu]
> >> pem_excute_event_chain+0x7d/0xe0 [amdgpu]
> >> pem_handle_event_unlocked+0x49/0x60 [amdgpu]
> >> pem_handle_event+0xe/0x10 [amdgpu]
> >> pp_dpm_dispatch_tasks+0xe0/0x190 [amdgpu]
> >> amdgpu_pm_comput
[PATCH] drm/amd/powerplay: don't give up if DPM is already running
Hi Alex,
Your new attached patch is Tested-by and Reviewed-by: Rex Zhu
Just one question,
Do we need to set clock_gate for smu?
Best Regards
Rex
-Original Message-
From: Alex Deucher [mailto:alexdeuc...@gmail.com]
Sent: Thursday, October 13, 2016 11:29 PM
To: Zhu, Rex
Cc: Grazvydas Ignotas; amd-gfx list; Maling list - DRI developers
Subject: Re: [PATCH] drm/amd/powerplay: don't give up if DPM is already running
On Thu, Oct 13, 2016 at 3:45 AM, Zhu, Rex wrote:
> Hi all,
>
> The attached patches were also for this issue.
> Disable dpm when rmmod amdgpu.
>
> Please help to review.
Patch 1:
+r = adev->ip_blocks[AMD_IP_BLOCK_TYPE_SMC].funcs->hw_fini((void *)adev);
+if (r)
+DRM_DEBUG("hw_fini of IP block <%s> failed %d\n",
+adev->ip_blocks[AMD_IP_BLOCK_TYPE_SMC].funcs->name, r);
+
+adev->ip_block_status[AMD_IP_BLOCK_TYPE_SMC].hw = false;
You can't use AMD_IP_BLOCK_TYPE_* as index. Some chips may not have the IP
block. Maybe something like the attached patch. That said, I think longer
term it makes more sense to allows the SOCs to specify the order for init,
fini, etc. otherwise we'll have lots of variable logic in the common code.
Patch 2:
+if (1 == PHM_READ_VFPF_INDIRECT_FIELD(hwmgr->device,
CGS_IND_REG__SMC, FEATURE_STATUS, AVS_ON)) {
+PP_ASSERT_WITH_CODE((0 == smum_send_msg_to_smc(hwmgr->smumgr,
PPSMC_MSG_DisableAvfs)),
+"Failed to disable AVFS!",
+return -1;);
+}
Use a proper error code there like -EINVAL or something. With that fixed:
Reviewed-by: Alex Deucher
Alex
>
> Best Regards
> Rex
>
> -Original Message-
> From: amd-gfx [mailto:amd-gfx-bounces at lists.freedesktop.org] On Behalf
> Of Zhu, Rex
> Sent: Wednesday, October 12, 2016 9:45 PM
> To: Alex Deucher; Grazvydas Ignotas
> Cc: amd-gfx list; Maling list - DRI developers
> Subject: RE: [PATCH] drm/amd/powerplay: don't give up if DPM is
> already running
>
> Hi Grazvydas and Alex,
>
> We needed to disable dpm when rmmod amdgpu for this issue.
>
> I am checking the function of disable dpm task.
>
> Best Regards
> Rex
>
> -Original Message-
> From: Alex Deucher [mailto:alexdeucher at gmail.com]
> Sent: Wednesday, October 12, 2016 4:01 AM
> To: Grazvydas Ignotas; Zhu, Rex
> Cc: Maling list - DRI developers; amd-gfx list
> Subject: Re: [PATCH] drm/amd/powerplay: don't give up if DPM is
> already running
>
> +Rex to review this.
>
> Alex
>
> On Sun, Oct 9, 2016 at 3:23 PM, Grazvydas Ignotas
> wrote:
>> Currently the driver crashes if smu7_enable_dpm_tasks() returns
>> early, which it does if DPM is already active. It seems to be better
>> just to continue anyway, at least I haven't noticed any ill effects.
>> It's also unclear at what state the hardware was left by the previous
>> driver, so IMO it's better to always fully initialize.
>>
>> Way to reproduce:
>> $ modprobe amdgpu
>> $ rmmod amdgpu
>> $ modprobe amdgpu
>> ...
>> DPM is already running right now, no need to enable DPM!
>> ...
>> failed to send message 18b ret is 0
>> BUG: unable to handle kernel paging request at ed01fc9ab21f Call
>> Trace:
>> smu7_set_power_state_tasks+0x499/0x1940 [amdgpu]
>> phm_set_power_state+0xcb/0x120 [amdgpu]
>> psm_adjust_power_state_dynamic+0x11e/0x1b0 [amdgpu]
>> pem_task_adjust_power_state+0xb9/0xd0 [amdgpu]
>> pem_excute_event_chain+0x7d/0xe0 [amdgpu]
>> pem_handle_event_unlocked+0x49/0x60 [amdgpu]
>> pem_handle_event+0xe/0x10 [amdgpu]
>> pp_dpm_dispatch_tasks+0xe0/0x190 [amdgpu]
>> amdgpu_pm_compute_clocks+0x10c/0xc60 [amdgpu]
>> dce_v11_0_crtc_dpms+0x7d/0x150 [amdgpu]
>> dce_v11_0_crtc_disable+0x90/0x4a0 [amdgpu]
>> drm_helper_disable_unused_functions+0x67/0x80 [drm_kms_helper]
>> amdgpu_fbdev_init+0x13e/0x170 [amdgpu]
>> amdgpu_device_init+0x1aeb/0x2490 [amdgpu]
>>
>> Signed-off-by: Grazvydas Ignotas
>> ---
>> drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
>> b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
>> index f6afa6a..327030b 100644
>> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
>> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
>> @@ -1166,8 +1166,8 @@ static int smu7_enable_dpm_tasks(struct
>> pp_hwmgr
>> *hwmgr)
>>
>> tmp_result = (!smum_is_dpm_running(hwmgr)) ? 0 : -1;
>> PP_ASSERT_WITH_CODE(tmp_result == 0,
>> - "DPM is already runnin
[PATCH] drm/amd/powerplay: don't give up if DPM is already running
On Thu, Oct 13, 2016 at 3:45 AM, Zhu, Rex wrote:
> Hi all,
>
> The attached patches were also for this issue.
> Disable dpm when rmmod amdgpu.
>
> Please help to review.
Patch 1:
+r = adev->ip_blocks[AMD_IP_BLOCK_TYPE_SMC].funcs->hw_fini((void *)adev);
+if (r)
+DRM_DEBUG("hw_fini of IP block <%s> failed %d\n",
+adev->ip_blocks[AMD_IP_BLOCK_TYPE_SMC].funcs->name, r);
+
+adev->ip_block_status[AMD_IP_BLOCK_TYPE_SMC].hw = false;
You can't use AMD_IP_BLOCK_TYPE_* as index. Some chips may not have
the IP block. Maybe something like the attached patch. That said, I
think longer term it makes more sense to allows the SOCs to specify
the order for init, fini, etc. otherwise we'll have lots of variable
logic in the common code.
Patch 2:
+if (1 == PHM_READ_VFPF_INDIRECT_FIELD(hwmgr->device,
CGS_IND_REG__SMC, FEATURE_STATUS, AVS_ON)) {
+PP_ASSERT_WITH_CODE((0 == smum_send_msg_to_smc(hwmgr->smumgr,
PPSMC_MSG_DisableAvfs)),
+"Failed to disable AVFS!",
+return -1;);
+}
Use a proper error code there like -EINVAL or something. With that fixed:
Reviewed-by: Alex Deucher
Alex
>
> Best Regards
> Rex
>
> -Original Message-
> From: amd-gfx [mailto:amd-gfx-bounces at lists.freedesktop.org] On Behalf Of
> Zhu, Rex
> Sent: Wednesday, October 12, 2016 9:45 PM
> To: Alex Deucher; Grazvydas Ignotas
> Cc: amd-gfx list; Maling list - DRI developers
> Subject: RE: [PATCH] drm/amd/powerplay: don't give up if DPM is already
> running
>
> Hi Grazvydas and Alex,
>
> We needed to disable dpm when rmmod amdgpu for this issue.
>
> I am checking the function of disable dpm task.
>
> Best Regards
> Rex
>
> -Original Message-
> From: Alex Deucher [mailto:alexdeucher at gmail.com]
> Sent: Wednesday, October 12, 2016 4:01 AM
> To: Grazvydas Ignotas; Zhu, Rex
> Cc: Maling list - DRI developers; amd-gfx list
> Subject: Re: [PATCH] drm/amd/powerplay: don't give up if DPM is already
> running
>
> +Rex to review this.
>
> Alex
>
> On Sun, Oct 9, 2016 at 3:23 PM, Grazvydas Ignotas
> wrote:
>> Currently the driver crashes if smu7_enable_dpm_tasks() returns early,
>> which it does if DPM is already active. It seems to be better just to
>> continue anyway, at least I haven't noticed any ill effects. It's also
>> unclear at what state the hardware was left by the previous driver, so
>> IMO it's better to always fully initialize.
>>
>> Way to reproduce:
>> $ modprobe amdgpu
>> $ rmmod amdgpu
>> $ modprobe amdgpu
>> ...
>> DPM is already running right now, no need to enable DPM!
>> ...
>> failed to send message 18b ret is 0
>> BUG: unable to handle kernel paging request at ed01fc9ab21f Call
>> Trace:
>> smu7_set_power_state_tasks+0x499/0x1940 [amdgpu]
>> phm_set_power_state+0xcb/0x120 [amdgpu]
>> psm_adjust_power_state_dynamic+0x11e/0x1b0 [amdgpu]
>> pem_task_adjust_power_state+0xb9/0xd0 [amdgpu]
>> pem_excute_event_chain+0x7d/0xe0 [amdgpu]
>> pem_handle_event_unlocked+0x49/0x60 [amdgpu]
>> pem_handle_event+0xe/0x10 [amdgpu]
>> pp_dpm_dispatch_tasks+0xe0/0x190 [amdgpu]
>> amdgpu_pm_compute_clocks+0x10c/0xc60 [amdgpu]
>> dce_v11_0_crtc_dpms+0x7d/0x150 [amdgpu]
>> dce_v11_0_crtc_disable+0x90/0x4a0 [amdgpu]
>> drm_helper_disable_unused_functions+0x67/0x80 [drm_kms_helper]
>> amdgpu_fbdev_init+0x13e/0x170 [amdgpu]
>> amdgpu_device_init+0x1aeb/0x2490 [amdgpu]
>>
>> Signed-off-by: Grazvydas Ignotas
>> ---
>> drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
>> b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
>> index f6afa6a..327030b 100644
>> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
>> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
>> @@ -1166,8 +1166,8 @@ static int smu7_enable_dpm_tasks(struct pp_hwmgr
>> *hwmgr)
>>
>> tmp_result = (!smum_is_dpm_running(hwmgr)) ? 0 : -1;
>> PP_ASSERT_WITH_CODE(tmp_result == 0,
>> - "DPM is already running right now, no need to enable
>> DPM!",
>> - return 0);
>> + "DPM is already running",
>> + );
>>
>> if (smu7_voltage_control(hwmgr)) {
>> tmp_result = smu7_enable_voltage_control(hwmgr);
>> --
>> 2.7.4
>>
>> ___
>> amd-gfx mailin
[PATCH] drm/amd/powerplay: don't give up if DPM is already running
Hi all,
The attached patches were also for this issue.
Disable dpm when rmmod amdgpu.
Please help to review.
Best Regards
Rex
-Original Message-
From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf Of Zhu,
Rex
Sent: Wednesday, October 12, 2016 9:45 PM
To: Alex Deucher; Grazvydas Ignotas
Cc: amd-gfx list; Maling list - DRI developers
Subject: RE: [PATCH] drm/amd/powerplay: don't give up if DPM is already running
Hi Grazvydas and Alex,
We needed to disable dpm when rmmod amdgpu for this issue.
I am checking the function of disable dpm task.
Best Regards
Rex
-Original Message-
From: Alex Deucher [mailto:alexdeuc...@gmail.com]
Sent: Wednesday, October 12, 2016 4:01 AM
To: Grazvydas Ignotas; Zhu, Rex
Cc: Maling list - DRI developers; amd-gfx list
Subject: Re: [PATCH] drm/amd/powerplay: don't give up if DPM is already running
+Rex to review this.
Alex
On Sun, Oct 9, 2016 at 3:23 PM, Grazvydas Ignotas wrote:
> Currently the driver crashes if smu7_enable_dpm_tasks() returns early,
> which it does if DPM is already active. It seems to be better just to
> continue anyway, at least I haven't noticed any ill effects. It's also
> unclear at what state the hardware was left by the previous driver, so
> IMO it's better to always fully initialize.
>
> Way to reproduce:
> $ modprobe amdgpu
> $ rmmod amdgpu
> $ modprobe amdgpu
> ...
> DPM is already running right now, no need to enable DPM!
> ...
> failed to send message 18b ret is 0
> BUG: unable to handle kernel paging request at ed01fc9ab21f Call
> Trace:
> smu7_set_power_state_tasks+0x499/0x1940 [amdgpu]
> phm_set_power_state+0xcb/0x120 [amdgpu]
> psm_adjust_power_state_dynamic+0x11e/0x1b0 [amdgpu]
> pem_task_adjust_power_state+0xb9/0xd0 [amdgpu]
> pem_excute_event_chain+0x7d/0xe0 [amdgpu]
> pem_handle_event_unlocked+0x49/0x60 [amdgpu]
> pem_handle_event+0xe/0x10 [amdgpu]
> pp_dpm_dispatch_tasks+0xe0/0x190 [amdgpu]
> amdgpu_pm_compute_clocks+0x10c/0xc60 [amdgpu]
> dce_v11_0_crtc_dpms+0x7d/0x150 [amdgpu]
> dce_v11_0_crtc_disable+0x90/0x4a0 [amdgpu]
> drm_helper_disable_unused_functions+0x67/0x80 [drm_kms_helper]
> amdgpu_fbdev_init+0x13e/0x170 [amdgpu]
> amdgpu_device_init+0x1aeb/0x2490 [amdgpu]
>
> Signed-off-by: Grazvydas Ignotas
> ---
> drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> index f6afa6a..327030b 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> @@ -1166,8 +1166,8 @@ static int smu7_enable_dpm_tasks(struct pp_hwmgr
> *hwmgr)
>
> tmp_result = (!smum_is_dpm_running(hwmgr)) ? 0 : -1;
> PP_ASSERT_WITH_CODE(tmp_result == 0,
> - "DPM is already running right now, no need to enable
> DPM!",
> - return 0);
> + "DPM is already running",
> + );
>
> if (smu7_voltage_control(hwmgr)) {
> tmp_result = smu7_enable_voltage_control(hwmgr);
> --
> 2.7.4
>
> ___
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx at lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
-- next part --
A non-text attachment was scrubbed...
Name: 0001-drm-amdgpu-need-to-disable-dpm-first-when-rmmod-amdg.patch
Type: application/octet-stream
Size: 1469 bytes
Desc: 0001-drm-amdgpu-need-to-disable-dpm-first-when-rmmod-amdg.patch
URL:
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161013/c2213003/attachment.obj>
-- next part --
A non-text attachment was scrubbed...
Name: 0002-drm-amd-powerplay-fix-bug-stop-dpm-can-t-work-on-Vi.patch
Type: application/octet-stream
Size: 3840 bytes
Desc: 0002-drm-amd-powerplay-fix-bug-stop-dpm-can-t-work-on-Vi.patch
URL:
<https://lists.freedesktop.org/archives/dri-devel/attachments/20161013/c2213003/attachment-0001.obj>
[PATCH] drm/amd/powerplay: don't give up if DPM is already running
Hi Grazvydas and Alex,
We needed to disable dpm when rmmod amdgpu for this issue.
I am checking the function of disable dpm task.
Best Regards
Rex
-Original Message-
From: Alex Deucher [mailto:alexdeuc...@gmail.com]
Sent: Wednesday, October 12, 2016 4:01 AM
To: Grazvydas Ignotas; Zhu, Rex
Cc: Maling list - DRI developers; amd-gfx list
Subject: Re: [PATCH] drm/amd/powerplay: don't give up if DPM is already running
+Rex to review this.
Alex
On Sun, Oct 9, 2016 at 3:23 PM, Grazvydas Ignotas wrote:
> Currently the driver crashes if smu7_enable_dpm_tasks() returns early,
> which it does if DPM is already active. It seems to be better just to
> continue anyway, at least I haven't noticed any ill effects. It's also
> unclear at what state the hardware was left by the previous driver, so
> IMO it's better to always fully initialize.
>
> Way to reproduce:
> $ modprobe amdgpu
> $ rmmod amdgpu
> $ modprobe amdgpu
> ...
> DPM is already running right now, no need to enable DPM!
> ...
> failed to send message 18b ret is 0
> BUG: unable to handle kernel paging request at ed01fc9ab21f Call
> Trace:
> smu7_set_power_state_tasks+0x499/0x1940 [amdgpu]
> phm_set_power_state+0xcb/0x120 [amdgpu]
> psm_adjust_power_state_dynamic+0x11e/0x1b0 [amdgpu]
> pem_task_adjust_power_state+0xb9/0xd0 [amdgpu]
> pem_excute_event_chain+0x7d/0xe0 [amdgpu]
> pem_handle_event_unlocked+0x49/0x60 [amdgpu]
> pem_handle_event+0xe/0x10 [amdgpu]
> pp_dpm_dispatch_tasks+0xe0/0x190 [amdgpu]
> amdgpu_pm_compute_clocks+0x10c/0xc60 [amdgpu]
> dce_v11_0_crtc_dpms+0x7d/0x150 [amdgpu]
> dce_v11_0_crtc_disable+0x90/0x4a0 [amdgpu]
> drm_helper_disable_unused_functions+0x67/0x80 [drm_kms_helper]
> amdgpu_fbdev_init+0x13e/0x170 [amdgpu]
> amdgpu_device_init+0x1aeb/0x2490 [amdgpu]
>
> Signed-off-by: Grazvydas Ignotas
> ---
> drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> index f6afa6a..327030b 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> @@ -1166,8 +1166,8 @@ static int smu7_enable_dpm_tasks(struct pp_hwmgr
> *hwmgr)
>
> tmp_result = (!smum_is_dpm_running(hwmgr)) ? 0 : -1;
> PP_ASSERT_WITH_CODE(tmp_result == 0,
> - "DPM is already running right now, no need to enable
> DPM!",
> - return 0);
> + "DPM is already running",
> + );
>
> if (smu7_voltage_control(hwmgr)) {
> tmp_result = smu7_enable_voltage_control(hwmgr);
> --
> 2.7.4
>
> ___
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amd/powerplay: don't give up if DPM is already running
+Rex to review this.
Alex
On Sun, Oct 9, 2016 at 3:23 PM, Grazvydas Ignotas wrote:
> Currently the driver crashes if smu7_enable_dpm_tasks() returns early,
> which it does if DPM is already active. It seems to be better just to
> continue anyway, at least I haven't noticed any ill effects. It's also
> unclear at what state the hardware was left by the previous driver, so
> IMO it's better to always fully initialize.
>
> Way to reproduce:
> $ modprobe amdgpu
> $ rmmod amdgpu
> $ modprobe amdgpu
> ...
> DPM is already running right now, no need to enable DPM!
> ...
> failed to send message 18b ret is 0
> BUG: unable to handle kernel paging request at ed01fc9ab21f
> Call Trace:
> smu7_set_power_state_tasks+0x499/0x1940 [amdgpu]
> phm_set_power_state+0xcb/0x120 [amdgpu]
> psm_adjust_power_state_dynamic+0x11e/0x1b0 [amdgpu]
> pem_task_adjust_power_state+0xb9/0xd0 [amdgpu]
> pem_excute_event_chain+0x7d/0xe0 [amdgpu]
> pem_handle_event_unlocked+0x49/0x60 [amdgpu]
> pem_handle_event+0xe/0x10 [amdgpu]
> pp_dpm_dispatch_tasks+0xe0/0x190 [amdgpu]
> amdgpu_pm_compute_clocks+0x10c/0xc60 [amdgpu]
> dce_v11_0_crtc_dpms+0x7d/0x150 [amdgpu]
> dce_v11_0_crtc_disable+0x90/0x4a0 [amdgpu]
> drm_helper_disable_unused_functions+0x67/0x80 [drm_kms_helper]
> amdgpu_fbdev_init+0x13e/0x170 [amdgpu]
> amdgpu_device_init+0x1aeb/0x2490 [amdgpu]
>
> Signed-off-by: Grazvydas Ignotas
> ---
> drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> index f6afa6a..327030b 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> @@ -1166,8 +1166,8 @@ static int smu7_enable_dpm_tasks(struct pp_hwmgr *hwmgr)
>
> tmp_result = (!smum_is_dpm_running(hwmgr)) ? 0 : -1;
> PP_ASSERT_WITH_CODE(tmp_result == 0,
> - "DPM is already running right now, no need to enable
> DPM!",
> - return 0);
> + "DPM is already running",
> + );
>
> if (smu7_voltage_control(hwmgr)) {
> tmp_result = smu7_enable_voltage_control(hwmgr);
> --
> 2.7.4
>
> ___
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amd/powerplay: don't give up if DPM is already running
Reviewed-by: Edward O'Callaghan
On 10/10/2016 06:23 AM, Grazvydas Ignotas wrote:
> Currently the driver crashes if smu7_enable_dpm_tasks() returns early,
> which it does if DPM is already active. It seems to be better just to
> continue anyway, at least I haven't noticed any ill effects. It's also
> unclear at what state the hardware was left by the previous driver, so
> IMO it's better to always fully initialize.
>
> Way to reproduce:
> $ modprobe amdgpu
> $ rmmod amdgpu
> $ modprobe amdgpu
> ...
> DPM is already running right now, no need to enable DPM!
> ...
> failed to send message 18b ret is 0
> BUG: unable to handle kernel paging request at ed01fc9ab21f
> Call Trace:
> smu7_set_power_state_tasks+0x499/0x1940 [amdgpu]
> phm_set_power_state+0xcb/0x120 [amdgpu]
> psm_adjust_power_state_dynamic+0x11e/0x1b0 [amdgpu]
> pem_task_adjust_power_state+0xb9/0xd0 [amdgpu]
> pem_excute_event_chain+0x7d/0xe0 [amdgpu]
> pem_handle_event_unlocked+0x49/0x60 [amdgpu]
> pem_handle_event+0xe/0x10 [amdgpu]
> pp_dpm_dispatch_tasks+0xe0/0x190 [amdgpu]
> amdgpu_pm_compute_clocks+0x10c/0xc60 [amdgpu]
> dce_v11_0_crtc_dpms+0x7d/0x150 [amdgpu]
> dce_v11_0_crtc_disable+0x90/0x4a0 [amdgpu]
> drm_helper_disable_unused_functions+0x67/0x80 [drm_kms_helper]
> amdgpu_fbdev_init+0x13e/0x170 [amdgpu]
> amdgpu_device_init+0x1aeb/0x2490 [amdgpu]
>
> Signed-off-by: Grazvydas Ignotas
> ---
> drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> index f6afa6a..327030b 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> @@ -1166,8 +1166,8 @@ static int smu7_enable_dpm_tasks(struct pp_hwmgr *hwmgr)
>
> tmp_result = (!smum_is_dpm_running(hwmgr)) ? 0 : -1;
> PP_ASSERT_WITH_CODE(tmp_result == 0,
> - "DPM is already running right now, no need to enable
> DPM!",
> - return 0);
> + "DPM is already running",
> + );
>
> if (smu7_voltage_control(hwmgr)) {
> tmp_result = smu7_enable_voltage_control(hwmgr);
>
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL:
[PATCH] drm/amd/powerplay: don't give up if DPM is already running
Currently the driver crashes if smu7_enable_dpm_tasks() returns early,
which it does if DPM is already active. It seems to be better just to
continue anyway, at least I haven't noticed any ill effects. It's also
unclear at what state the hardware was left by the previous driver, so
IMO it's better to always fully initialize.
Way to reproduce:
$ modprobe amdgpu
$ rmmod amdgpu
$ modprobe amdgpu
...
DPM is already running right now, no need to enable DPM!
...
failed to send message 18b ret is 0
BUG: unable to handle kernel paging request at ed01fc9ab21f
Call Trace:
smu7_set_power_state_tasks+0x499/0x1940 [amdgpu]
phm_set_power_state+0xcb/0x120 [amdgpu]
psm_adjust_power_state_dynamic+0x11e/0x1b0 [amdgpu]
pem_task_adjust_power_state+0xb9/0xd0 [amdgpu]
pem_excute_event_chain+0x7d/0xe0 [amdgpu]
pem_handle_event_unlocked+0x49/0x60 [amdgpu]
pem_handle_event+0xe/0x10 [amdgpu]
pp_dpm_dispatch_tasks+0xe0/0x190 [amdgpu]
amdgpu_pm_compute_clocks+0x10c/0xc60 [amdgpu]
dce_v11_0_crtc_dpms+0x7d/0x150 [amdgpu]
dce_v11_0_crtc_disable+0x90/0x4a0 [amdgpu]
drm_helper_disable_unused_functions+0x67/0x80 [drm_kms_helper]
amdgpu_fbdev_init+0x13e/0x170 [amdgpu]
amdgpu_device_init+0x1aeb/0x2490 [amdgpu]
Signed-off-by: Grazvydas Ignotas
---
drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
index f6afa6a..327030b 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
@@ -1166,8 +1166,8 @@ static int smu7_enable_dpm_tasks(struct pp_hwmgr *hwmgr)
tmp_result = (!smum_is_dpm_running(hwmgr)) ? 0 : -1;
PP_ASSERT_WITH_CODE(tmp_result == 0,
- "DPM is already running right now, no need to enable
DPM!",
- return 0);
+ "DPM is already running",
+ );
if (smu7_voltage_control(hwmgr)) {
tmp_result = smu7_enable_voltage_control(hwmgr);
--
2.7.4
