Re: [PATCH 3/4] drm/amd/display: In VRR mode, do DRM core vblank handling at end of vblank.

[Paul Menzel <pmenzel+amd-gfx-KUpvgZVWgV9o1qOY/usvUg@public.gmane.org>]

Dear Mario,


On 18.03.19 18:19, Mario Kleiner wrote:
> In VRR mode, proper vblank/pageflip timestamps can only be computed
> after the display scanout position has left front-porch. Therefore
> delay calls to drm_crtc_handle_vblank(), and thereby calls to
> drm_update_vblank_count() and pageflip event delivery, to after the
> end of front-porch when in VRR mode.
> 
> We add a new vupdate irq, which triggers at the end of the vupdate
> interval, ie. at the end of vblank, and calls the core vblank handler
> function. The new irq handler is not executed in standard non-VRR
> mode, so vblank handling for fixed refresh rate mode is identical
> to the past implementation.
> 
> Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h                |   1 +
>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  | 129 ++++++++++++++++++++-
>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h  |   9 ++
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c  |  22 ++++
>   .../amd/display/dc/irq/dce110/irq_service_dce110.c |   7 +-
>   .../amd/display/dc/irq/dce120/irq_service_dce120.c |   7 +-
>   .../amd/display/dc/irq/dce80/irq_service_dce80.c   |   6 +-
>   .../amd/display/dc/irq/dcn10/irq_service_dcn10.c   |  40 +++++--
>   8 files changed, 205 insertions(+), 16 deletions(-)

[…]

`scripts/checkpatch.pl` shows two errors on your commit.

> +	/* Use VUPDATE interrupt */
> +	for (i = VISLANDS30_IV_SRCID_D1_V_UPDATE_INT; i <= VISLANDS30_IV_SRCID_D6_V_UPDATE_INT; i+=2) {

ERROR: spaces required around that '+=' (ctx:VxV)
#107: FILE: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c:1490:
+	for (i = VISLANDS30_IV_SRCID_D1_V_UPDATE_INT; i <= 
VISLANDS30_IV_SRCID_D6_V_UPDATE_INT; i+=2) {
  	 
                    ^

[…]

>   static inline int dm_set_vblank(struct drm_crtc *crtc, bool enable)
>   {
>   	enum dc_irq_source irq_source;
>   	struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
>   	struct amdgpu_device *adev = crtc->dev->dev_private;
> +	struct dm_crtc_state *acrtc_state = to_dm_crtc_state(crtc->state);
> +	int rc = 0;
> +
> +	if (enable) {
> +		/* vblank irq on -> Only need vupdate irq in vrr mode */
> +		if (amdgpu_dm_vrr_active(acrtc_state))
> +			rc = dm_set_vupdate_irq(crtc, true);
> +	}
> +	else {
> +		/* vblank irq off -> vupdate irq off */
> +		rc = dm_set_vupdate_irq(crtc, false);
> +	}

ERROR: else should follow close brace '}'
#198: FILE: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c:3346:
+	}
+	else {

Also:

WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
#258: FILE: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c:679:
+					   unsigned crtc_id,

[…]


Kind regards,

Paul
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx