Am 16.10.2018 um 20:54 schrieb Zhu, Rex:
>
>> -----Original Message-----
>> From: Christian König <ckoenig.leichtzumerken@xxxxxxxxx>
>> Sent: Wednesday, October 17, 2018 2:31 AM
>> To: Zhu, Rex <Rex.Zhu@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
>> Subject: Re: [PATCH v2] drm/amdgpu: Poweroff uvd/vce/vcn/acp block if
>> they were disabled by user
>>
>> Am 16.10.2018 um 20:25 schrieb Rex Zhu:
>>> If user disable uvd/vce/vcn/acp blocks via module parameter
>>> ip_block_mask, driver power off thoser blocks to save power.
>>>
>>> v2: power off uvd/vce/vcn/acp via smu.
>> I'm not sure if that is a good idea.
>>
>> The meaning of the flag is to NOT touch the blocks at all, e.g. pretend they
>> don't even exists.
>>
>> Additional to that the ip_block_mask is only for bringup and debugging and
>> not meant for production use and power saving shouldn't be an issue there.
> Hi Christian,
>
> This is requested by Chrome project. As there is an S3 issue caused by VCE, and they still don't find the root cause.
> And in Chrome project, they don't need to use vce engine.
> So as a workaround, they want to disable vce and power off the vce engine to save power.
Oh, please not that stupid idea again! In this case that is a clear NAK.
The mask isn't necessary stable and this module parameter is absolutely
NOT intended for production use!
If we want to disable a specific block permanently in a project we
should do so by adding a patch to the specific branch and NOT by abusing
ip_block_mask for it.
Regards,
Christian.
>
> Best Regards
> Rex
>
>> So I would rather say that this is a NAK.
>>
>> Christian.
>>
>>> Reviewed-by: Alex Deucher <alexander.deucher@xxxxxxx>
>>> Signed-off-by: Rex Zhu <Rex.Zhu@xxxxxxx>
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 19
>> +++++++++++++++++++
>>> 1 file changed, 19 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> index 6fe6ea9..ef9fe50 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> @@ -1776,6 +1776,24 @@ static int amdgpu_device_set_pg_state(struct
>>> amdgpu_device *adev, enum amd_power
>>>
>>> for (j = 0; j < adev->num_ip_blocks; j++) {
>>> i = state == AMD_PG_STATE_GATE ? j : adev->num_ip_blocks
>> - j - 1;
>>> +
>>> + /* try to power off VCE/UVD/VCN/ACP if they were disabled
>> by user */
>>> + if ((adev->ip_blocks[i].version->type ==
>> AMD_IP_BLOCK_TYPE_UVD ||
>>> + adev->ip_blocks[i].version->type ==
>> AMD_IP_BLOCK_TYPE_VCE ||
>>> + adev->ip_blocks[i].version->type ==
>> AMD_IP_BLOCK_TYPE_VCN ||
>>> + adev->ip_blocks[i].version->type ==
>> AMD_IP_BLOCK_TYPE_ACP) &&
>>> + adev->powerplay.pp_funcs->set_powergating_by_smu) {
>>> + if (!adev->ip_blocks[i].status.valid) {
>>> +
>> amdgpu_dpm_set_powergating_by_smu(adev, adev-
>>> ip_blocks[i].version->type, state == AMD_PG_STATE_GATE ?
>>> +
>> true : false);
>>> + if (r) {
>>> +
>> DRM_ERROR("set_powergating_state(gate) of IP block <%s>
>> failed %d\n",
>>> + adev->ip_blocks[i].version->funcs-
>>> name, r);
>>> + return r;
>>> + }
>>> + }
>>> + }
>>> +
>>> if (!adev->ip_blocks[i].status.late_initialized)
>>> continue;
>>> /* skip CG for VCE/UVD, it's handled specially */ @@ -1793,6
>>> +1811,7 @@ static int amdgpu_device_set_pg_state(struct amdgpu_device
>> *adev, enum amd_power
>>> }
>>> }
>>> }
>>> +
>>> return 0;
>>> }
>>>
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
