[PATCH v2 00/19] DC/DM changes needed for amdgpu PSR-SU
Leo Li
sunpeng.li at amd.com
Mon May 16 16:23:20 UTC 2022
On 2022-05-12 13:39, Daniel Vetter wrote:
> On Thu, 12 May 2022 at 19:22, Zhang, Dingchen (David)
> <Dingchen.Zhang at amd.com> wrote:
>>
>> [AMD Official Use Only - General]
>>
>> Hi Daniel
>>
>> Thanks for your comments and explanations. I replied in-line and look forward to more discussion.
>>
>> regards
>> David
>>
>>
>> From: Daniel Vetter <daniel at ffwll.ch>
>> Sent: Thursday, May 12, 2022 7:22 AM
>> To: Alex Deucher <alexdeucher at gmail.com>
>> Cc: Zhang, Dingchen (David) <Dingchen.Zhang at amd.com>; amd-gfx list <amd-gfx at lists.freedesktop.org>; Wang, Chao-kai (Stylon) <Stylon.Wang at amd.com>; Li, Sun peng (Leo) <Sunpeng.Li at amd.com>; Wentland, Harry <Harry.Wentland at amd.com>; Zhuo, Qingqing (Lillian) <Qingqing.Zhuo at amd.com>; Siqueira, Rodrigo <Rodrigo.Siqueira at amd.com>; Li, Roman <Roman.Li at amd.com>; Chiu, Solomon <Solomon.Chiu at amd.com>; Zuo, Jerry <Jerry.Zuo at amd.com>; Pillai, Aurabindo <Aurabindo.Pillai at amd.com>; Lin, Wayne <Wayne.Lin at amd.com>; Lakha, Bhawanpreet <Bhawanpreet.Lakha at amd.com>; Gutierrez, Agustin <Agustin.Gutierrez at amd.com>; Kotarac, Pavle <Pavle.Kotarac at amd.com>
>> Subject: Re: [PATCH v2 00/19] DC/DM changes needed for amdgpu PSR-SU
>>
>> On Wed, 11 May 2022 at 17:35, Alex Deucher <alexdeucher at gmail.com> wrote:
>>>
>>> On Tue, May 10, 2022 at 4:45 PM David Zhang <dingchen.zhang at amd.com> wrote:
>>>>
>>>> changes in v2:
>>>> -----------------------
>>>> - set vsc_packet_rev2 for PSR1 which is safer
>>>> - add exposure of AMD specific DPCD regs for PSR-SU-RC (rate-control)
>>>> - add DC/DM change related to amdgpu PSR-SU-RC
>>>>
>>>>
>>>> David Zhang (18):
>>>> drm/amd/display: align dmub cmd header to latest dmub FW to support
>>>> PSR-SU
>>>> drm/amd/display: feed PSR-SU as psr version to dmub FW
>>>> drm/amd/display: combine dirty rectangles in DMUB FW
>>>> drm/amd/display: update GSP1 generic info packet for PSRSU
>>>> drm/amd/display: revise Start/End SDP data
>>>> drm/amd/display: program PSR2 DPCD Configuration
>>>> drm/amd/display: Passing Y-granularity to dmub fw
>>>> drm/amd/display: Set default value of line_capture_indication
>>>> drm/amd/display: add vline time in micro sec to PSR context
>>>> drm/amd/display: fix system hang when PSR exits
>>>> drm/amd/display: Set PSR level to enable ALPM by default
>>>> drm/amd/display: use HW lock mgr for PSR-SU
>>>> drm/amd/display: PSRSU+DSC WA for specific TCON
>>>> drm/amd/display: add shared helpers to update psr config fields to
>>>> power module
>>>> drm/amd/display: calculate psr config settings in runtime in DM
>>>> drm/amd/display: update cursor position to DMUB FW
>>>> drm/amd/display: expose AMD source specific DPCD for FreeSync PSR
>>>> support
>>>> drm/amd/display: PSR-SU rate control support in DC
>>>>
>>>> Leo Li (1):
>>>> drm/amd/display: Implement MPO PSR SU
>>>
>>> A couple of suggestions from Daniel on IRC:
>>> 1. Might be good to extract the "calculate total crtc damage" code
>>> from i915 in intel_psr2_sel_fetch_update, stuff that into damage
>>> helpers and reuse for i915 and amdgpu
>>
>> To expand a bit on this. There is currently a helper for total damage,
>> but it's at the fb/plane level for drivers which need to upload
>> buffers (usb/spi or virtual) drm_atomic_helper_damage_merged(). That
>> one probably needs to be renamed to signify it's about the plane, and
>> then we need a new drm_atomic_helper_crtc_damage_merged() which
>> (extract from i915 code ideally) which computes total crtc damage for
>> stuff like psr2/su or the command mode dsi panels (unfortunately none
>> of the drivers for android for these panels have been upstreamed yet).
>>
>> <<<
>> Checked the DRM comment for the helper `drm_atomic_helper_damage_merged()` and
>> quoted below:
>> *****
>> Drivers might want to use the helper functions drm_atomic_helper_damage_iter_init()
>> and drm_atomic_helper_damage_iter_next() or drm_atomic_helper_damage_merged()
>> if the driver can only handle a single damage region at most.
>> *****
>> Currently for amdgpu, the multiple damage clips combination (merging) is handled in
>> DMUB firmware. And the DRM comment shows that the usage of "damage_merged()"
>> helper is for the driver which can only handle single damage region at most.
>>
>> Since AMDGPU is capable of handling multiple damaged clip (in DMUB FW), can I
>> understand that the group of helpers of `damage_merged()` in DRM is not mandatory
>> but optional?
>
> Ah I didn't see from a quick read that this was possible. How does
> this work when the plane is enabled/disabled or resized or moved?
> -Daniel
Hi Daniel,
When a plane is disabled, enabled, and/or resized(*), PSR is temporarily
disabled. The mechanism to do so isn't in this patchset, but was added
when PSR1 was first introduced to amdgpu_dm.
In short, amdgpu_dm will disable PSR whenever DC requires a full update
to program an atomic state (needs bandwidth recalculations and/or
shuffling hw resources). For DC, enabling, disabling, and changing the
scaling of a plane are considered full updates.
When the plane is moved, both the old and new plane bounds are given to
FW as dirty rectangles. (*)Resize should fall under the same bucket,
though DC would consider that as a full update. I think disabling PSR
would take precedence... will give this another spin to check.
Thanks,
Leo
>
>> I also think that the split between dc and kms is a bit funny, I'd put
>> only the resulting damage rect into dc_pipe and do the computation of
>> that in the drm/kms linux code outside of dc functions (or in the glue
>> code for dc), since I'm assuming on windows it's completely different
>> approach in how you compute damage. Especially once we have the crtc
>> damage helper on linux.
>>
>>> 2. The commit message on "drm/amd/display: Implement MPO PSR SU" is a
>>> bit funny, since if you use the helpers right you always get damage
>>> information, just when it's from userspace that doesn't set explicit
>>> damage it's just always the entire plane.
>>
>> <<<
>> The current implementation to mark the entire MPO as dirt RECT is not the final
>> version. Our next step is to implement the translation of DRM damaged clips to
>> DC regions and pass to let DMUB FW to handle, which is able to handle at most
>> 3 damaged regions for each DC surface.
>>
>>
>>
>> Yeah so that one was just another reason to use the helpers more in
>> amdgpu for this.
>> -Daniel
>>
>>>
>>> Alex
>>>
>>>>
>>>> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 142 +++++++++-
>>>> .../drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c | 21 +-
>>>> drivers/gpu/drm/amd/display/dc/core/dc.c | 54 ++++
>>>> drivers/gpu/drm/amd/display/dc/core/dc_link.c | 47 +++-
>>>> drivers/gpu/drm/amd/display/dc/dc_link.h | 4 +
>>>> drivers/gpu/drm/amd/display/dc/dc_stream.h | 5 +
>>>> drivers/gpu/drm/amd/display/dc/dc_types.h | 23 +-
>>>> .../drm/amd/display/dc/dce/dmub_hw_lock_mgr.c | 2 +
>>>> drivers/gpu/drm/amd/display/dc/dce/dmub_psr.c | 64 +++++
>>>> drivers/gpu/drm/amd/display/dc/dce/dmub_psr.h | 2 +
>>>> .../gpu/drm/amd/display/dc/dcn10/dcn10_hubp.c | 2 +
>>>> .../amd/display/dc/dcn10/dcn10_hw_sequencer.c | 131 +++++++++
>>>> .../gpu/drm/amd/display/dc/dcn20/dcn20_hubp.c | 2 +
>>>> .../dc/dcn30/dcn30_dio_stream_encoder.c | 15 ++
>>>> drivers/gpu/drm/amd/display/dc/inc/hw/hubp.h | 1 +
>>>> .../drm/amd/display/dc/inc/hw/link_encoder.h | 21 +-
>>>> .../gpu/drm/amd/display/dmub/inc/dmub_cmd.h | 250 +++++++++++++++++-
>>>> .../amd/display/include/ddc_service_types.h | 1 +
>>>> .../display/modules/info_packet/info_packet.c | 29 +-
>>>> .../amd/display/modules/power/power_helpers.c | 84 ++++++
>>>> .../amd/display/modules/power/power_helpers.h | 6 +
>>>> 21 files changed, 887 insertions(+), 19 deletions(-)
>>>>
>>>> --
>>>> 2.25.1
>>>>
>>
>>
>>
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fblog.ffwll.ch%2F&data=05%7C01%7CSunpeng.Li%40amd.com%7C2e39f7cf022f46ee917b08da343e5867%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637879739624499442%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=5Scos4KwScr%2F5MRPprq%2B0uyp76QRPDNRDt04afOVm5Y%3D&reserved=0
>
>
>
More information about the amd-gfx
mailing list