| Message ID | 20200309195216.31042-13-laurent.pinchart@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: mxsfb: Add i.MX7 support | expand |
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@agner.ch> -- Stefan > > Signed-off-by: Laurent Pinchart <laurent.pinchart@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);
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);
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(). Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- drivers/gpu/drm/mxsfb/mxsfb_kms.c | 35 ++++++++++++++++++------------- 1 file changed, 20 insertions(+), 15 deletions(-)