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@xxxxxxxx>
--
Stefan
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> ---
> 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);
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
