[PATCH 12/21] drm: mxsfb: Move vblank event arm to CRTC .atomic_flush()

[Stefan Agner]

On 2020-03-09 20:52, Laurent Pinchart wrote:
> The vblank event is armed in the plane .atomic_update(). This works fine
> as we have a single plane, but will break as soon as multiple planes are
> supported (not to mention it's logically the wrong place to perform the
> operation). Move it to CRTC .atomic_flush().

The comment in the bracket make it sound as if there was something wrong
with the code before, but from what I understand that was just fine and
the only option we had with drm_simple_kms_helper, correct? Can you
maybe make that a bit clearer? (e.g. by stating that now that we no
longer use the simple display pipeline we can and should use the flush
callback).

Otherwise looks good.

Reviewed-by: Stefan Agner <stefan at agner.ch>

--
Stefan

> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  drivers/gpu/drm/mxsfb/mxsfb_kms.c | 35 ++++++++++++++++++-------------
>  1 file changed, 20 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_kms.c
> b/drivers/gpu/drm/mxsfb/mxsfb_kms.c
> index 8f339adb8d04..ebe0785694cb 100644
> --- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c
> +++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c
> @@ -295,6 +295,25 @@ static int mxsfb_crtc_atomic_check(struct drm_crtc *crtc,
>  	return drm_atomic_add_affected_planes(state->state, crtc);
>  }
>  
> +static void mxsfb_crtc_atomic_flush(struct drm_crtc *crtc,
> +				    struct drm_crtc_state *old_state)
> +{
> +	struct drm_pending_vblank_event *event;
> +
> +	event = crtc->state->event;
> +	crtc->state->event = NULL;
> +
> +	if (!event)
> +		return;
> +
> +	spin_lock_irq(&crtc->dev->event_lock);
> +	if (drm_crtc_vblank_get(crtc) == 0)
> +		drm_crtc_arm_vblank_event(crtc, event);
> +	else
> +		drm_crtc_send_vblank_event(crtc, event);
> +	spin_unlock_irq(&crtc->dev->event_lock);
> +}
> +
>  static void mxsfb_crtc_atomic_enable(struct drm_crtc *crtc,
>  				     struct drm_crtc_state *old_state)
>  {
> @@ -364,6 +383,7 @@ static void mxsfb_crtc_disable_vblank(struct drm_crtc *crtc)
>  
>  static const struct drm_crtc_helper_funcs mxsfb_crtc_helper_funcs = {
>  	.atomic_check = mxsfb_crtc_atomic_check,
> +	.atomic_flush = mxsfb_crtc_atomic_flush,
>  	.atomic_enable = mxsfb_crtc_atomic_enable,
>  	.atomic_disable = mxsfb_crtc_atomic_disable,
>  };
> @@ -410,23 +430,8 @@ static void mxsfb_plane_atomic_update(struct
> drm_plane *plane,
>  				      struct drm_plane_state *old_pstate)
>  {
>  	struct mxsfb_drm_private *mxsfb = to_mxsfb_drm_private(plane->dev);
> -	struct drm_crtc *crtc = &mxsfb->crtc;
> -	struct drm_pending_vblank_event *event;
>  	dma_addr_t paddr;
>  
> -	spin_lock_irq(&crtc->dev->event_lock);
> -	event = crtc->state->event;
> -	if (event) {
> -		crtc->state->event = NULL;
> -
> -		if (drm_crtc_vblank_get(crtc) == 0) {
> -			drm_crtc_arm_vblank_event(crtc, event);
> -		} else {
> -			drm_crtc_send_vblank_event(crtc, event);
> -		}
> -	}
> -	spin_unlock_irq(&crtc->dev->event_lock);
> -
>  	paddr = mxsfb_get_fb_paddr(mxsfb);
>  	if (paddr) {
>  		mxsfb_enable_axi_clk(mxsfb);