Re: [PATCH v4 7/7] drm/mgag200: Implement struct drm_crtc_funcs.get_vblank_timestamp

[Jocelyn Falempe <jfalempe@xxxxxxxxxx>]

On 05/07/2024 13:47, Thomas Zimmermann wrote:
> Implement struct drm_crtc_funcs.get_vblank_timestamp with the DRM
> helper drm_crtc_vblank_helper_get_vblank_timestamp() with its helper
> get_scanout_position. Read the scanout position from the MGAREG_VCOUNT
> register.

Thanks, it looks good to me.

Reviewed-by: Jocelyn Falempe <jfalempe@xxxxxxxxxx>

> Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
> ---
>   drivers/gpu/drm/mgag200/mgag200_drv.h  | 10 ++++++++--
>   drivers/gpu/drm/mgag200/mgag200_mode.c | 25 +++++++++++++++++++++++++
>   2 files changed, 33 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h
> index f7b22b195016..acfa05335b09 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_drv.h
> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
> @@ -410,13 +410,18 @@ int mgag200_crtc_helper_atomic_check(struct drm_crtc *crtc, struct drm_atomic_st
>   void mgag200_crtc_helper_atomic_flush(struct drm_crtc *crtc, struct drm_atomic_state *old_state);
>   void mgag200_crtc_helper_atomic_enable(struct drm_crtc *crtc, struct drm_atomic_state *old_state);
>   void mgag200_crtc_helper_atomic_disable(struct drm_crtc *crtc, struct drm_atomic_state *old_state);
> +bool mgag200_crtc_helper_get_scanout_position(struct drm_crtc *crtc, bool in_vblank_irq,
> +					      int *vpos, int *hpos,
> +					      ktime_t *stime, ktime_t *etime,
> +					      const struct drm_display_mode *mode);
>   
>   #define MGAG200_CRTC_HELPER_FUNCS \
>   	.mode_valid = mgag200_crtc_helper_mode_valid, \
>   	.atomic_check = mgag200_crtc_helper_atomic_check, \
>   	.atomic_flush = mgag200_crtc_helper_atomic_flush, \
>   	.atomic_enable = mgag200_crtc_helper_atomic_enable, \
> -	.atomic_disable = mgag200_crtc_helper_atomic_disable
> +	.atomic_disable = mgag200_crtc_helper_atomic_disable, \
> +	.get_scanout_position = mgag200_crtc_helper_get_scanout_position
>   
>   void mgag200_crtc_reset(struct drm_crtc *crtc);
>   struct drm_crtc_state *mgag200_crtc_atomic_duplicate_state(struct drm_crtc *crtc);
> @@ -432,7 +437,8 @@ void mgag200_crtc_disable_vblank(struct drm_crtc *crtc);
>   	.atomic_duplicate_state = mgag200_crtc_atomic_duplicate_state, \
>   	.atomic_destroy_state = mgag200_crtc_atomic_destroy_state, \
>   	.enable_vblank = mgag200_crtc_enable_vblank, \
> -	.disable_vblank = mgag200_crtc_disable_vblank
> +	.disable_vblank = mgag200_crtc_disable_vblank, \
> +	.get_vblank_timestamp = drm_crtc_vblank_helper_get_vblank_timestamp
>   
>   void mgag200_set_mode_regs(struct mga_device *mdev, const struct drm_display_mode *mode);
>   void mgag200_set_format_regs(struct mga_device *mdev, const struct drm_format_info *format);
> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
> index ec6fb1277d6e..fb03422c763b 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
> @@ -718,6 +718,31 @@ void mgag200_crtc_helper_atomic_disable(struct drm_crtc *crtc, struct drm_atomic
>   		funcs->enable_vidrst(mdev);
>   }
>   
> +bool mgag200_crtc_helper_get_scanout_position(struct drm_crtc *crtc, bool in_vblank_irq,
> +					      int *vpos, int *hpos,
> +					      ktime_t *stime, ktime_t *etime,
> +					      const struct drm_display_mode *mode)
> +{
> +	struct mga_device *mdev = to_mga_device(crtc->dev);
> +	u32 vcount;
> +
> +	if (stime)
> +		*stime = ktime_get();
> +
> +	if (vpos) {
> +		vcount = RREG32(MGAREG_VCOUNT);
> +		*vpos = vcount & GENMASK(11, 0);
> +	}
> +
> +	if (hpos)
> +		*hpos = mode->htotal >> 1; // near middle of scanline on average
> +
> +	if (etime)
> +		*etime = ktime_get();
> +
> +	return true;
> +}
> +
>   void mgag200_crtc_reset(struct drm_crtc *crtc)
>   {
>   	struct mgag200_crtc_state *mgag200_crtc_state;