> -----Original Message-----
> From: Christian König <ckoenig.leichtzumerken@xxxxxxxxx>
> Sent: Wednesday, October 17, 2018 3:11 AM
> To: Zhu, Rex <Rex.Zhu@xxxxxxx>; Koenig, Christian
> <Christian.Koenig@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 21:03 schrieb Zhu, Rex:
> >
> >> -----Original Message-----
> >> From: Koenig, Christian
> >> Sent: Wednesday, October 17, 2018 2:59 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: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.
> > This is also requested by Chrome project. Customer wish all the patches are
> cherry-pick from drm-next/linux kernel tree.
> > Maybe we need to fix the vce issue on time.
>
> Yeah, completely agree.
>
> Exactly that's the kind of stuff why Chrome has the restriction to only cherry-
> pick patches from upstream.
>
> So we can either go ahead and disable VCE upstream as well or we sit down
> and fix the issue.
>
> Do you have a bug tracker for that?
Yes, there is a ticket: SWDEV-168119
Regards
Rex
> Regards,
> Christian.
>
> >
> > Best Regards
> > Rex
> >
> >> 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
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
