Re: [PATCH v2 00/19] DC/DM changes needed for amdgpu PSR-SU

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, 11 May 2022 at 17:35, Alex Deucher <alexdeucher@gmail.com> wrote:
>
> On Tue, May 10, 2022 at 4:45 PM David Zhang <dingchen.zhang@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).

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.

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
http://blog.ffwll.ch