[PATCH 4/4] amd/display: indicate support for atomic async page-flips on DCN

[Simon Ser]

On Thursday, August 25th, 2022 at 20:22, Alex Deucher <alexdeucher at gmail.com> wrote:

> On Wed, Aug 24, 2022 at 11:09 AM Simon Ser contact at emersion.fr wrote:
> 
> > amdgpu_dm_commit_planes already sets the flip_immediate flag for
> > async page-flips. This flag is used to set the UNP_FLIP_CONTROL
> > register. Thus, no additional change is required to handle async
> > page-flips with the atomic uAPI.
> > 
> > Note, async page-flips are still unsupported on DCE with the atomic
> > uAPI. The mode_set_base callbacks unconditionally set the
> > GRPH_SURFACE_UPDATE_H_RETRACE_EN field of the GRPH_FLIP_CONTROL
> > register to 0, which disables async page-flips.
> 
> Can you elaborate a bit on this? We don't use hsync flips at all, even
> in non-atomic, as far as I recall. The hardware can also do immediate
> flips which take effect as soon as you update the base address
> register which is what we use for async updates today IIRC.

When user-space performs a page-flip with the legacy KMS uAPI on DCE
ASICs, amdgpu_display_crtc_page_flip_target() is called. This function
checks for the DRM_MODE_PAGE_FLIP_ASYNC flag, sets work->async, which
is then passed as an argument to adev->mode_info.funcs->page_flip() by
amdgpu_display_flip_work_func(). Looking at an implementation, for
instance dce_v10_0_page_flip(), the async flag is used to set that
GRPH_FLIP_CONTROL register:

	/* flip at hsync for async, default is vsync */
	tmp = RREG32(mmGRPH_FLIP_CONTROL + amdgpu_crtc->crtc_offset);
	tmp = REG_SET_FIELD(tmp, GRPH_FLIP_CONTROL,
			    GRPH_SURFACE_UPDATE_H_RETRACE_EN, async ? 1 : 0);
	WREG32(mmGRPH_FLIP_CONTROL + amdgpu_crtc->crtc_offset, tmp);

I don't know how the hardware works, but I assumed it would be
necessary to do the same in the atomic uAPI code-path as well. However
dce_v10_0_crtc_do_set_base() has this code block:

	/* Make sure surface address is updated at vertical blank rather than
	 * horizontal blank
	 */
	tmp = RREG32(mmGRPH_FLIP_CONTROL + amdgpu_crtc->crtc_offset);
	tmp = REG_SET_FIELD(tmp, GRPH_FLIP_CONTROL,
			    GRPH_SURFACE_UPDATE_H_RETRACE_EN, 0);
	WREG32(mmGRPH_FLIP_CONTROL + amdgpu_crtc->crtc_offset, tmp);

Which unconditionally sets that same register.

Either way, it's not a very big deal for this patch series, DCE and DCN
are separate, DCE can be sorted out separately.

Am I completely mistaken here?

Thanks,

Simon