[PATCH v2] drm/mediatek: Move mtk_crtc_finish_page_flip() to ddp_cmdq_cb()
Jason-JH Lin (林睿祥)
Jason-JH.Lin at mediatek.com
Mon Dec 9 17:45:45 PST 2024
Hi CK,
Thanks for the review.
[snip]
> > static void mtk_drm_finish_page_flip(struct mtk_crtc *mtk_crtc)
> > {
> > unsigned long flags;
> > + struct drm_crtc *crtc = &mtk_crtc->base;
> > + struct mtk_drm_private *priv = crtc->dev->dev_private;
> >
> > drm_crtc_handle_vblank(&mtk_crtc->base);
> >
> > - spin_lock_irqsave(&mtk_crtc->config_lock, flags);
> > - if (!mtk_crtc->config_updating && mtk_crtc-
> > >pending_needs_vblank) {
> > - mtk_crtc_finish_page_flip(mtk_crtc);
> > - mtk_crtc->pending_needs_vblank = false;
> > + if (priv->data->shadow_register) {
>
> if (!priv->data->shadow_register)
> return;
>
> And this would reduce the indent.
>
OK, I'll adjust this.
> > + spin_lock_irqsave(&mtk_crtc->config_lock, flags);
> > + if (!mtk_crtc->config_updating && mtk_crtc-
> > >pending_needs_vblank) {
> > + mtk_crtc_finish_page_flip(mtk_crtc);
> > + mtk_crtc->pending_needs_vblank = false;
> > + }
> > + spin_unlock_irqrestore(&mtk_crtc->config_lock, flags);
> > }
> > - spin_unlock_irqrestore(&mtk_crtc->config_lock, flags);
> > }
> >
> > static void mtk_crtc_destroy(struct drm_crtc *crtc)
> > @@ -284,10 +288,8 @@ static void ddp_cmdq_cb(struct mbox_client
> > *cl, void *mssg)
> > state = to_mtk_crtc_state(mtk_crtc->base.state);
> >
> > spin_lock_irqsave(&mtk_crtc->config_lock, flags);
> > - if (mtk_crtc->config_updating) {
> > - spin_unlock_irqrestore(&mtk_crtc->config_lock, flags);
> > + if (mtk_crtc->config_updating)
> > goto ddp_cmdq_cb_out;
> > - }
> >
> > state->pending_config = false;
> >
> > @@ -315,10 +317,15 @@ static void ddp_cmdq_cb(struct mbox_client
> > *cl, void *mssg)
> > mtk_crtc->pending_async_planes = false;
> > }
> >
> > - spin_unlock_irqrestore(&mtk_crtc->config_lock, flags);
> > -
> > ddp_cmdq_cb_out:
> >
> > + if (mtk_crtc->pending_needs_vblank) {
> > + mtk_crtc_finish_page_flip(mtk_crtc);
> > + mtk_crtc->pending_needs_vblank = false;
>
> In mtk_crtc_update_config(), after mbox_client_txdone() and before
> mtk_crtc->config_updating = false;
> cmdq may have finish the command and callback ddp_cmdq_cb().
> Because mtk_crtc->config_updating is true, so this page flip would be
> skipped.
> But if there is no next mtk_crtc_update_config() exist, this page
> flip would never happen.
> This may cause some deadlock.
>
OK, I'll move the mtk_crtc->config_updating = false; earlier than
mbox_client_txdone() to avoid skipping the page flip.
Regards,
Jason-JH.Lin
> Regards,
> CK
More information about the linux-arm-kernel
mailing list