| Message ID | 20220321055345.10195-1-rex-bc.chen@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [V1] drm/mediatek: Fix issue of vblank callback data is NULL | expand |
Hi, Rex: I would like the title be WHAT does this patch do and the commit message be WHY does this patch do. I think what does this patch do is adding vblank register/unregister function. Regards, CK On Mon, 2022-03-21 at 13:53 +0800, Rex-BC Chen wrote: > We encountered a kernel panic issue that callback data will be NULL > when > it's using in ovl irq handler. There is a timing issue between > mtk_disp_ovl_irq_handler() and mtk_ovl_disable_vblank(). > > To resolve this issue, we use the flow to register/unregister vblank > cb: > - Register callback function and callback data when crtc creates. > - Unregister callback function and callback data when crtc destroies. > > With this solution, we can assure callback data will not be NULL when > vblank is disable. > > Fixes: 9b0704988b15 ("drm/mediatek: Register vblank callback > function") > Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com> > Reviewed-by: jason-jh.lin <jason-jh.lin@mediatek.com> > --- > drivers/gpu/drm/mediatek/mtk_disp_drv.h | 16 +++++++----- > drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 22 ++++++++++++---- > drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 20 +++++++++----- > drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 14 +++++++++- > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 4 +++ > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 29 ++++++++++++++++--- > -- > 6 files changed, 80 insertions(+), 25 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h > b/drivers/gpu/drm/mediatek/mtk_disp_drv.h > index 86c3068894b1..974462831133 100644 > --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h > +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h > @@ -76,9 +76,11 @@ void mtk_ovl_layer_off(struct device *dev, > unsigned int idx, > void mtk_ovl_start(struct device *dev); > void mtk_ovl_stop(struct device *dev); > unsigned int mtk_ovl_supported_rotations(struct device *dev); > -void mtk_ovl_enable_vblank(struct device *dev, > - void (*vblank_cb)(void *), > - void *vblank_cb_data); > +void mtk_ovl_register_vblank_cb(struct device *dev, > + void (*vblank_cb)(void *), > + void *vblank_cb_data); > +void mtk_ovl_unregister_vblank_cb(struct device *dev); > +void mtk_ovl_enable_vblank(struct device *dev); > void mtk_ovl_disable_vblank(struct device *dev); > > void mtk_rdma_bypass_shadow(struct device *dev); > @@ -93,9 +95,11 @@ void mtk_rdma_layer_config(struct device *dev, > unsigned int idx, > struct cmdq_pkt *cmdq_pkt); > void mtk_rdma_start(struct device *dev); > void mtk_rdma_stop(struct device *dev); > -void mtk_rdma_enable_vblank(struct device *dev, > - void (*vblank_cb)(void *), > - void *vblank_cb_data); > +void mtk_rdma_register_vblank_cb(struct device *dev, > + void (*vblank_cb)(void *), > + void *vblank_cb_data); > +void mtk_rdma_unregister_vblank_cb(struct device *dev); > +void mtk_rdma_enable_vblank(struct device *dev); > void mtk_rdma_disable_vblank(struct device *dev); > > #endif > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > index 2146299e5f52..1fa1bbac9f9c 100644 > --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > @@ -96,14 +96,28 @@ static irqreturn_t mtk_disp_ovl_irq_handler(int > irq, void *dev_id) > return IRQ_HANDLED; > } > > -void mtk_ovl_enable_vblank(struct device *dev, > - void (*vblank_cb)(void *), > - void *vblank_cb_data) > +void mtk_ovl_register_vblank_cb(struct device *dev, > + void (*vblank_cb)(void *), > + void *vblank_cb_data) > { > struct mtk_disp_ovl *ovl = dev_get_drvdata(dev); > > ovl->vblank_cb = vblank_cb; > ovl->vblank_cb_data = vblank_cb_data; > +} > + > +void mtk_ovl_unregister_vblank_cb(struct device *dev) > +{ > + struct mtk_disp_ovl *ovl = dev_get_drvdata(dev); > + > + ovl->vblank_cb = NULL; > + ovl->vblank_cb_data = NULL; > +} > + > +void mtk_ovl_enable_vblank(struct device *dev) > +{ > + struct mtk_disp_ovl *ovl = dev_get_drvdata(dev); > + > writel(0x0, ovl->regs + DISP_REG_OVL_INTSTA); > writel_relaxed(OVL_FME_CPL_INT, ovl->regs + > DISP_REG_OVL_INTEN); > } > @@ -112,8 +126,6 @@ void mtk_ovl_disable_vblank(struct device *dev) > { > struct mtk_disp_ovl *ovl = dev_get_drvdata(dev); > > - ovl->vblank_cb = NULL; > - ovl->vblank_cb_data = NULL; > writel_relaxed(0x0, ovl->regs + DISP_REG_OVL_INTEN); > } > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c > b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c > index d41a3970b944..943780fc7bf6 100644 > --- a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c > +++ b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c > @@ -94,24 +94,32 @@ static void rdma_update_bits(struct device *dev, > unsigned int reg, > writel(tmp, rdma->regs + reg); > } > > -void mtk_rdma_enable_vblank(struct device *dev, > - void (*vblank_cb)(void *), > - void *vblank_cb_data) > +void mtk_rdma_register_vblank_cb(struct device *dev, > + void (*vblank_cb)(void *), > + void *vblank_cb_data) > { > struct mtk_disp_rdma *rdma = dev_get_drvdata(dev); > > rdma->vblank_cb = vblank_cb; > rdma->vblank_cb_data = vblank_cb_data; > - rdma_update_bits(dev, DISP_REG_RDMA_INT_ENABLE, > RDMA_FRAME_END_INT, > - RDMA_FRAME_END_INT); > } > > -void mtk_rdma_disable_vblank(struct device *dev) > +void mtk_rdma_unregister_vblank_cb(struct device *dev) > { > struct mtk_disp_rdma *rdma = dev_get_drvdata(dev); > > rdma->vblank_cb = NULL; > rdma->vblank_cb_data = NULL; > +} > + > +void mtk_rdma_enable_vblank(struct device *dev) > +{ > + rdma_update_bits(dev, DISP_REG_RDMA_INT_ENABLE, > RDMA_FRAME_END_INT, > + RDMA_FRAME_END_INT); > +} > + > +void mtk_rdma_disable_vblank(struct device *dev) > +{ > rdma_update_bits(dev, DISP_REG_RDMA_INT_ENABLE, > RDMA_FRAME_END_INT, 0); > } > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > index d661edf7e0fe..e42a9bfa0ecb 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > @@ -152,6 +152,7 @@ static void mtk_drm_cmdq_pkt_destroy(struct > cmdq_pkt *pkt) > static void mtk_drm_crtc_destroy(struct drm_crtc *crtc) > { > struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc); > + int i; > > mtk_mutex_put(mtk_crtc->mutex); > #if IS_REACHABLE(CONFIG_MTK_CMDQ) > @@ -162,6 +163,14 @@ static void mtk_drm_crtc_destroy(struct drm_crtc > *crtc) > mtk_crtc->cmdq_client.chan = NULL; > } > #endif > + > + for (i = 0; i < mtk_crtc->ddp_comp_nr; i++) { > + struct mtk_ddp_comp *comp; > + > + comp = mtk_crtc->ddp_comp[i]; > + mtk_ddp_comp_unregister_vblank_cb(comp); > + } > + > drm_crtc_cleanup(crtc); > } > > @@ -616,7 +625,7 @@ static int mtk_drm_crtc_enable_vblank(struct > drm_crtc *crtc) > struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc); > struct mtk_ddp_comp *comp = mtk_crtc->ddp_comp[0]; > > - mtk_ddp_comp_enable_vblank(comp, mtk_crtc_ddp_irq, &mtk_crtc- > >base); > + mtk_ddp_comp_enable_vblank(comp); > > return 0; > } > @@ -916,6 +925,9 @@ int mtk_drm_crtc_create(struct drm_device > *drm_dev, > if (comp->funcs->ctm_set) > has_ctm = true; > } > + > + mtk_ddp_comp_register_vblank_cb(comp, mtk_crtc_ddp_irq, > + &mtk_crtc->base); > } > > for (i = 0; i < mtk_crtc->ddp_comp_nr; i++) > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > index b4b682bc1991..028cf76b9531 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > @@ -297,6 +297,8 @@ static const struct mtk_ddp_comp_funcs ddp_ovl = > { > .config = mtk_ovl_config, > .start = mtk_ovl_start, > .stop = mtk_ovl_stop, > + .register_vblank_cb = mtk_ovl_register_vblank_cb, > + .unregister_vblank_cb = mtk_ovl_unregister_vblank_cb, > .enable_vblank = mtk_ovl_enable_vblank, > .disable_vblank = mtk_ovl_disable_vblank, > .supported_rotations = mtk_ovl_supported_rotations, > @@ -321,6 +323,8 @@ static const struct mtk_ddp_comp_funcs ddp_rdma = > { > .config = mtk_rdma_config, > .start = mtk_rdma_start, > .stop = mtk_rdma_stop, > + .register_vblank_cb = mtk_rdma_register_vblank_cb, > + .unregister_vblank_cb = mtk_rdma_unregister_vblank_cb, > .enable_vblank = mtk_rdma_enable_vblank, > .disable_vblank = mtk_rdma_disable_vblank, > .layer_nr = mtk_rdma_layer_nr, > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h > b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h > index 4c6a98662305..b83f24cb045f 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h > @@ -48,9 +48,11 @@ struct mtk_ddp_comp_funcs { > unsigned int bpc, struct cmdq_pkt *cmdq_pkt); > void (*start)(struct device *dev); > void (*stop)(struct device *dev); > - void (*enable_vblank)(struct device *dev, > - void (*vblank_cb)(void *), > - void *vblank_cb_data); > + void (*register_vblank_cb)(struct device *dev, > + void (*vblank_cb)(void *), > + void *vblank_cb_data); > + void (*unregister_vblank_cb)(struct device *dev); > + void (*enable_vblank)(struct device *dev); > void (*disable_vblank)(struct device *dev); > unsigned int (*supported_rotations)(struct device *dev); > unsigned int (*layer_nr)(struct device *dev); > @@ -111,12 +113,25 @@ static inline void mtk_ddp_comp_stop(struct > mtk_ddp_comp *comp) > comp->funcs->stop(comp->dev); > } > > -static inline void mtk_ddp_comp_enable_vblank(struct mtk_ddp_comp > *comp, > - void (*vblank_cb)(void > *), > - void *vblank_cb_data) > +static inline void mtk_ddp_comp_register_vblank_cb(struct > mtk_ddp_comp *comp, > + void > (*vblank_cb)(void *), > + void > *vblank_cb_data) > +{ > + if (comp->funcs && comp->funcs->register_vblank_cb) > + comp->funcs->register_vblank_cb(comp->dev, vblank_cb, > + vblank_cb_data); > +} > + > +static inline void mtk_ddp_comp_unregister_vblank_cb(struct > mtk_ddp_comp *comp) > +{ > + if (comp->funcs && comp->funcs->unregister_vblank_cb) > + comp->funcs->unregister_vblank_cb(comp->dev); > +} > + > +static inline void mtk_ddp_comp_enable_vblank(struct mtk_ddp_comp > *comp) > { > if (comp->funcs && comp->funcs->enable_vblank) > - comp->funcs->enable_vblank(comp->dev, vblank_cb, > vblank_cb_data); > + comp->funcs->enable_vblank(comp->dev); > } > > static inline void mtk_ddp_comp_disable_vblank(struct mtk_ddp_comp > *comp)
On Mon, 2022-03-21 at 15:07 +0800, CK Hu wrote: > Hi, Rex: > > I would like the title be WHAT does this patch do and the commit > message be WHY does this patch do. I think what does this patch do is > adding vblank register/unregister function. > > Regards, > CK > Hello CK, Ok, I will modify the title for this series in next version. Thanks! BRs, Rex > On Mon, 2022-03-21 at 13:53 +0800, Rex-BC Chen wrote: > > We encountered a kernel panic issue that callback data will be NULL > > when > > it's using in ovl irq handler. There is a timing issue between > > mtk_disp_ovl_irq_handler() and mtk_ovl_disable_vblank(). > > > > To resolve this issue, we use the flow to register/unregister > > vblank > > cb: > > - Register callback function and callback data when crtc creates. > > - Unregister callback function and callback data when crtc > > destroies. > > > > With this solution, we can assure callback data will not be NULL > > when > > vblank is disable. > > > > Fixes: 9b0704988b15 ("drm/mediatek: Register vblank callback > > function") > > Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com> > > Reviewed-by: jason-jh.lin <jason-jh.lin@mediatek.com> > > --- > > drivers/gpu/drm/mediatek/mtk_disp_drv.h | 16 +++++++----- > > drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 22 ++++++++++++---- > > drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 20 +++++++++----- > > drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 14 +++++++++- > > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 4 +++ > > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 29 ++++++++++++++++- > > -- > > -- > > 6 files changed, 80 insertions(+), 25 deletions(-) > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h > > b/drivers/gpu/drm/mediatek/mtk_disp_drv.h > > index 86c3068894b1..974462831133 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h > > +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h > > @@ -76,9 +76,11 @@ void mtk_ovl_layer_off(struct device *dev, > > unsigned int idx, > > void mtk_ovl_start(struct device *dev); > > void mtk_ovl_stop(struct device *dev); > > unsigned int mtk_ovl_supported_rotations(struct device *dev); > > -void mtk_ovl_enable_vblank(struct device *dev, > > - void (*vblank_cb)(void *), > > - void *vblank_cb_data); > > +void mtk_ovl_register_vblank_cb(struct device *dev, > > + void (*vblank_cb)(void *), > > + void *vblank_cb_data); > > +void mtk_ovl_unregister_vblank_cb(struct device *dev); > > +void mtk_ovl_enable_vblank(struct device *dev); > > void mtk_ovl_disable_vblank(struct device *dev); > > > > void mtk_rdma_bypass_shadow(struct device *dev); > > @@ -93,9 +95,11 @@ void mtk_rdma_layer_config(struct device *dev, > > unsigned int idx, > > struct cmdq_pkt *cmdq_pkt); > > void mtk_rdma_start(struct device *dev); > > void mtk_rdma_stop(struct device *dev); > > -void mtk_rdma_enable_vblank(struct device *dev, > > - void (*vblank_cb)(void *), > > - void *vblank_cb_data); > > +void mtk_rdma_register_vblank_cb(struct device *dev, > > + void (*vblank_cb)(void *), > > + void *vblank_cb_data); > > +void mtk_rdma_unregister_vblank_cb(struct device *dev); > > +void mtk_rdma_enable_vblank(struct device *dev); > > void mtk_rdma_disable_vblank(struct device *dev); > > > > #endif > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > > b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > > index 2146299e5f52..1fa1bbac9f9c 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > > +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > > @@ -96,14 +96,28 @@ static irqreturn_t mtk_disp_ovl_irq_handler(int > > irq, void *dev_id) > > return IRQ_HANDLED; > > } > > > > -void mtk_ovl_enable_vblank(struct device *dev, > > - void (*vblank_cb)(void *), > > - void *vblank_cb_data) > > +void mtk_ovl_register_vblank_cb(struct device *dev, > > + void (*vblank_cb)(void *), > > + void *vblank_cb_data) > > { > > struct mtk_disp_ovl *ovl = dev_get_drvdata(dev); > > > > ovl->vblank_cb = vblank_cb; > > ovl->vblank_cb_data = vblank_cb_data; > > +} > > + > > +void mtk_ovl_unregister_vblank_cb(struct device *dev) > > +{ > > + struct mtk_disp_ovl *ovl = dev_get_drvdata(dev); > > + > > + ovl->vblank_cb = NULL; > > + ovl->vblank_cb_data = NULL; > > +} > > + > > +void mtk_ovl_enable_vblank(struct device *dev) > > +{ > > + struct mtk_disp_ovl *ovl = dev_get_drvdata(dev); > > + > > writel(0x0, ovl->regs + DISP_REG_OVL_INTSTA); > > writel_relaxed(OVL_FME_CPL_INT, ovl->regs + > > DISP_REG_OVL_INTEN); > > } > > @@ -112,8 +126,6 @@ void mtk_ovl_disable_vblank(struct device *dev) > > { > > struct mtk_disp_ovl *ovl = dev_get_drvdata(dev); > > > > - ovl->vblank_cb = NULL; > > - ovl->vblank_cb_data = NULL; > > writel_relaxed(0x0, ovl->regs + DISP_REG_OVL_INTEN); > > } > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c > > b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c > > index d41a3970b944..943780fc7bf6 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c > > +++ b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c > > @@ -94,24 +94,32 @@ static void rdma_update_bits(struct device > > *dev, > > unsigned int reg, > > writel(tmp, rdma->regs + reg); > > } > > > > -void mtk_rdma_enable_vblank(struct device *dev, > > - void (*vblank_cb)(void *), > > - void *vblank_cb_data) > > +void mtk_rdma_register_vblank_cb(struct device *dev, > > + void (*vblank_cb)(void *), > > + void *vblank_cb_data) > > { > > struct mtk_disp_rdma *rdma = dev_get_drvdata(dev); > > > > rdma->vblank_cb = vblank_cb; > > rdma->vblank_cb_data = vblank_cb_data; > > - rdma_update_bits(dev, DISP_REG_RDMA_INT_ENABLE, > > RDMA_FRAME_END_INT, > > - RDMA_FRAME_END_INT); > > } > > > > -void mtk_rdma_disable_vblank(struct device *dev) > > +void mtk_rdma_unregister_vblank_cb(struct device *dev) > > { > > struct mtk_disp_rdma *rdma = dev_get_drvdata(dev); > > > > rdma->vblank_cb = NULL; > > rdma->vblank_cb_data = NULL; > > +} > > + > > +void mtk_rdma_enable_vblank(struct device *dev) > > +{ > > + rdma_update_bits(dev, DISP_REG_RDMA_INT_ENABLE, > > RDMA_FRAME_END_INT, > > + RDMA_FRAME_END_INT); > > +} > > + > > +void mtk_rdma_disable_vblank(struct device *dev) > > +{ > > rdma_update_bits(dev, DISP_REG_RDMA_INT_ENABLE, > > RDMA_FRAME_END_INT, 0); > > } > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > > b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > > index d661edf7e0fe..e42a9bfa0ecb 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > > @@ -152,6 +152,7 @@ static void mtk_drm_cmdq_pkt_destroy(struct > > cmdq_pkt *pkt) > > static void mtk_drm_crtc_destroy(struct drm_crtc *crtc) > > { > > struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc); > > + int i; > > > > mtk_mutex_put(mtk_crtc->mutex); > > #if IS_REACHABLE(CONFIG_MTK_CMDQ) > > @@ -162,6 +163,14 @@ static void mtk_drm_crtc_destroy(struct > > drm_crtc > > *crtc) > > mtk_crtc->cmdq_client.chan = NULL; > > } > > #endif > > + > > + for (i = 0; i < mtk_crtc->ddp_comp_nr; i++) { > > + struct mtk_ddp_comp *comp; > > + > > + comp = mtk_crtc->ddp_comp[i]; > > + mtk_ddp_comp_unregister_vblank_cb(comp); > > + } > > + > > drm_crtc_cleanup(crtc); > > } > > > > @@ -616,7 +625,7 @@ static int mtk_drm_crtc_enable_vblank(struct > > drm_crtc *crtc) > > struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc); > > struct mtk_ddp_comp *comp = mtk_crtc->ddp_comp[0]; > > > > - mtk_ddp_comp_enable_vblank(comp, mtk_crtc_ddp_irq, &mtk_crtc- > > > base); > > > > + mtk_ddp_comp_enable_vblank(comp); > > > > return 0; > > } > > @@ -916,6 +925,9 @@ int mtk_drm_crtc_create(struct drm_device > > *drm_dev, > > if (comp->funcs->ctm_set) > > has_ctm = true; > > } > > + > > + mtk_ddp_comp_register_vblank_cb(comp, mtk_crtc_ddp_irq, > > + &mtk_crtc->base); > > } > > > > for (i = 0; i < mtk_crtc->ddp_comp_nr; i++) > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > > b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > > index b4b682bc1991..028cf76b9531 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > > @@ -297,6 +297,8 @@ static const struct mtk_ddp_comp_funcs ddp_ovl > > = > > { > > .config = mtk_ovl_config, > > .start = mtk_ovl_start, > > .stop = mtk_ovl_stop, > > + .register_vblank_cb = mtk_ovl_register_vblank_cb, > > + .unregister_vblank_cb = mtk_ovl_unregister_vblank_cb, > > .enable_vblank = mtk_ovl_enable_vblank, > > .disable_vblank = mtk_ovl_disable_vblank, > > .supported_rotations = mtk_ovl_supported_rotations, > > @@ -321,6 +323,8 @@ static const struct mtk_ddp_comp_funcs ddp_rdma > > = > > { > > .config = mtk_rdma_config, > > .start = mtk_rdma_start, > > .stop = mtk_rdma_stop, > > + .register_vblank_cb = mtk_rdma_register_vblank_cb, > > + .unregister_vblank_cb = mtk_rdma_unregister_vblank_cb, > > .enable_vblank = mtk_rdma_enable_vblank, > > .disable_vblank = mtk_rdma_disable_vblank, > > .layer_nr = mtk_rdma_layer_nr, > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h > > b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h > > index 4c6a98662305..b83f24cb045f 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h > > @@ -48,9 +48,11 @@ struct mtk_ddp_comp_funcs { > > unsigned int bpc, struct cmdq_pkt *cmdq_pkt); > > void (*start)(struct device *dev); > > void (*stop)(struct device *dev); > > - void (*enable_vblank)(struct device *dev, > > - void (*vblank_cb)(void *), > > - void *vblank_cb_data); > > + void (*register_vblank_cb)(struct device *dev, > > + void (*vblank_cb)(void *), > > + void *vblank_cb_data); > > + void (*unregister_vblank_cb)(struct device *dev); > > + void (*enable_vblank)(struct device *dev); > > void (*disable_vblank)(struct device *dev); > > unsigned int (*supported_rotations)(struct device *dev); > > unsigned int (*layer_nr)(struct device *dev); > > @@ -111,12 +113,25 @@ static inline void mtk_ddp_comp_stop(struct > > mtk_ddp_comp *comp) > > comp->funcs->stop(comp->dev); > > } > > > > -static inline void mtk_ddp_comp_enable_vblank(struct mtk_ddp_comp > > *comp, > > - void (*vblank_cb)(void > > *), > > - void *vblank_cb_data) > > +static inline void mtk_ddp_comp_register_vblank_cb(struct > > mtk_ddp_comp *comp, > > + void > > (*vblank_cb)(void *), > > + void > > *vblank_cb_data) > > +{ > > + if (comp->funcs && comp->funcs->register_vblank_cb) > > + comp->funcs->register_vblank_cb(comp->dev, vblank_cb, > > + vblank_cb_data); > > +} > > + > > +static inline void mtk_ddp_comp_unregister_vblank_cb(struct > > mtk_ddp_comp *comp) > > +{ > > + if (comp->funcs && comp->funcs->unregister_vblank_cb) > > + comp->funcs->unregister_vblank_cb(comp->dev); > > +} > > + > > +static inline void mtk_ddp_comp_enable_vblank(struct mtk_ddp_comp > > *comp) > > { > > if (comp->funcs && comp->funcs->enable_vblank) > > - comp->funcs->enable_vblank(comp->dev, vblank_cb, > > vblank_cb_data); > > + comp->funcs->enable_vblank(comp->dev); > > } > > > > static inline void mtk_ddp_comp_disable_vblank(struct mtk_ddp_comp > > *comp) > >
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h b/drivers/gpu/drm/mediatek/mtk_disp_drv.h index 86c3068894b1..974462831133 100644 --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h @@ -76,9 +76,11 @@ void mtk_ovl_layer_off(struct device *dev, unsigned int idx, void mtk_ovl_start(struct device *dev); void mtk_ovl_stop(struct device *dev); unsigned int mtk_ovl_supported_rotations(struct device *dev); -void mtk_ovl_enable_vblank(struct device *dev, - void (*vblank_cb)(void *), - void *vblank_cb_data); +void mtk_ovl_register_vblank_cb(struct device *dev, + void (*vblank_cb)(void *), + void *vblank_cb_data); +void mtk_ovl_unregister_vblank_cb(struct device *dev); +void mtk_ovl_enable_vblank(struct device *dev); void mtk_ovl_disable_vblank(struct device *dev); void mtk_rdma_bypass_shadow(struct device *dev); @@ -93,9 +95,11 @@ void mtk_rdma_layer_config(struct device *dev, unsigned int idx, struct cmdq_pkt *cmdq_pkt); void mtk_rdma_start(struct device *dev); void mtk_rdma_stop(struct device *dev); -void mtk_rdma_enable_vblank(struct device *dev, - void (*vblank_cb)(void *), - void *vblank_cb_data); +void mtk_rdma_register_vblank_cb(struct device *dev, + void (*vblank_cb)(void *), + void *vblank_cb_data); +void mtk_rdma_unregister_vblank_cb(struct device *dev); +void mtk_rdma_enable_vblank(struct device *dev); void mtk_rdma_disable_vblank(struct device *dev); #endif diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c index 2146299e5f52..1fa1bbac9f9c 100644 --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c @@ -96,14 +96,28 @@ static irqreturn_t mtk_disp_ovl_irq_handler(int irq, void *dev_id) return IRQ_HANDLED; } -void mtk_ovl_enable_vblank(struct device *dev, - void (*vblank_cb)(void *), - void *vblank_cb_data) +void mtk_ovl_register_vblank_cb(struct device *dev, + void (*vblank_cb)(void *), + void *vblank_cb_data) { struct mtk_disp_ovl *ovl = dev_get_drvdata(dev); ovl->vblank_cb = vblank_cb; ovl->vblank_cb_data = vblank_cb_data; +} + +void mtk_ovl_unregister_vblank_cb(struct device *dev) +{ + struct mtk_disp_ovl *ovl = dev_get_drvdata(dev); + + ovl->vblank_cb = NULL; + ovl->vblank_cb_data = NULL; +} + +void mtk_ovl_enable_vblank(struct device *dev) +{ + struct mtk_disp_ovl *ovl = dev_get_drvdata(dev); + writel(0x0, ovl->regs + DISP_REG_OVL_INTSTA); writel_relaxed(OVL_FME_CPL_INT, ovl->regs + DISP_REG_OVL_INTEN); } @@ -112,8 +126,6 @@ void mtk_ovl_disable_vblank(struct device *dev) { struct mtk_disp_ovl *ovl = dev_get_drvdata(dev); - ovl->vblank_cb = NULL; - ovl->vblank_cb_data = NULL; writel_relaxed(0x0, ovl->regs + DISP_REG_OVL_INTEN); } diff --git a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c index d41a3970b944..943780fc7bf6 100644 --- a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c +++ b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c @@ -94,24 +94,32 @@ static void rdma_update_bits(struct device *dev, unsigned int reg, writel(tmp, rdma->regs + reg); } -void mtk_rdma_enable_vblank(struct device *dev, - void (*vblank_cb)(void *), - void *vblank_cb_data) +void mtk_rdma_register_vblank_cb(struct device *dev, + void (*vblank_cb)(void *), + void *vblank_cb_data) { struct mtk_disp_rdma *rdma = dev_get_drvdata(dev); rdma->vblank_cb = vblank_cb; rdma->vblank_cb_data = vblank_cb_data; - rdma_update_bits(dev, DISP_REG_RDMA_INT_ENABLE, RDMA_FRAME_END_INT, - RDMA_FRAME_END_INT); } -void mtk_rdma_disable_vblank(struct device *dev) +void mtk_rdma_unregister_vblank_cb(struct device *dev) { struct mtk_disp_rdma *rdma = dev_get_drvdata(dev); rdma->vblank_cb = NULL; rdma->vblank_cb_data = NULL; +} + +void mtk_rdma_enable_vblank(struct device *dev) +{ + rdma_update_bits(dev, DISP_REG_RDMA_INT_ENABLE, RDMA_FRAME_END_INT, + RDMA_FRAME_END_INT); +} + +void mtk_rdma_disable_vblank(struct device *dev) +{ rdma_update_bits(dev, DISP_REG_RDMA_INT_ENABLE, RDMA_FRAME_END_INT, 0); } diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c index d661edf7e0fe..e42a9bfa0ecb 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c @@ -152,6 +152,7 @@ static void mtk_drm_cmdq_pkt_destroy(struct cmdq_pkt *pkt) static void mtk_drm_crtc_destroy(struct drm_crtc *crtc) { struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc); + int i; mtk_mutex_put(mtk_crtc->mutex); #if IS_REACHABLE(CONFIG_MTK_CMDQ) @@ -162,6 +163,14 @@ static void mtk_drm_crtc_destroy(struct drm_crtc *crtc) mtk_crtc->cmdq_client.chan = NULL; } #endif + + for (i = 0; i < mtk_crtc->ddp_comp_nr; i++) { + struct mtk_ddp_comp *comp; + + comp = mtk_crtc->ddp_comp[i]; + mtk_ddp_comp_unregister_vblank_cb(comp); + } + drm_crtc_cleanup(crtc); } @@ -616,7 +625,7 @@ static int mtk_drm_crtc_enable_vblank(struct drm_crtc *crtc) struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc); struct mtk_ddp_comp *comp = mtk_crtc->ddp_comp[0]; - mtk_ddp_comp_enable_vblank(comp, mtk_crtc_ddp_irq, &mtk_crtc->base); + mtk_ddp_comp_enable_vblank(comp); return 0; } @@ -916,6 +925,9 @@ int mtk_drm_crtc_create(struct drm_device *drm_dev, if (comp->funcs->ctm_set) has_ctm = true; } + + mtk_ddp_comp_register_vblank_cb(comp, mtk_crtc_ddp_irq, + &mtk_crtc->base); } for (i = 0; i < mtk_crtc->ddp_comp_nr; i++) diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c index b4b682bc1991..028cf76b9531 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c @@ -297,6 +297,8 @@ static const struct mtk_ddp_comp_funcs ddp_ovl = { .config = mtk_ovl_config, .start = mtk_ovl_start, .stop = mtk_ovl_stop, + .register_vblank_cb = mtk_ovl_register_vblank_cb, + .unregister_vblank_cb = mtk_ovl_unregister_vblank_cb, .enable_vblank = mtk_ovl_enable_vblank, .disable_vblank = mtk_ovl_disable_vblank, .supported_rotations = mtk_ovl_supported_rotations, @@ -321,6 +323,8 @@ static const struct mtk_ddp_comp_funcs ddp_rdma = { .config = mtk_rdma_config, .start = mtk_rdma_start, .stop = mtk_rdma_stop, + .register_vblank_cb = mtk_rdma_register_vblank_cb, + .unregister_vblank_cb = mtk_rdma_unregister_vblank_cb, .enable_vblank = mtk_rdma_enable_vblank, .disable_vblank = mtk_rdma_disable_vblank, .layer_nr = mtk_rdma_layer_nr, diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h index 4c6a98662305..b83f24cb045f 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h @@ -48,9 +48,11 @@ struct mtk_ddp_comp_funcs { unsigned int bpc, struct cmdq_pkt *cmdq_pkt); void (*start)(struct device *dev); void (*stop)(struct device *dev); - void (*enable_vblank)(struct device *dev, - void (*vblank_cb)(void *), - void *vblank_cb_data); + void (*register_vblank_cb)(struct device *dev, + void (*vblank_cb)(void *), + void *vblank_cb_data); + void (*unregister_vblank_cb)(struct device *dev); + void (*enable_vblank)(struct device *dev); void (*disable_vblank)(struct device *dev); unsigned int (*supported_rotations)(struct device *dev); unsigned int (*layer_nr)(struct device *dev); @@ -111,12 +113,25 @@ static inline void mtk_ddp_comp_stop(struct mtk_ddp_comp *comp) comp->funcs->stop(comp->dev); } -static inline void mtk_ddp_comp_enable_vblank(struct mtk_ddp_comp *comp, - void (*vblank_cb)(void *), - void *vblank_cb_data) +static inline void mtk_ddp_comp_register_vblank_cb(struct mtk_ddp_comp *comp, + void (*vblank_cb)(void *), + void *vblank_cb_data) +{ + if (comp->funcs && comp->funcs->register_vblank_cb) + comp->funcs->register_vblank_cb(comp->dev, vblank_cb, + vblank_cb_data); +} + +static inline void mtk_ddp_comp_unregister_vblank_cb(struct mtk_ddp_comp *comp) +{ + if (comp->funcs && comp->funcs->unregister_vblank_cb) + comp->funcs->unregister_vblank_cb(comp->dev); +} + +static inline void mtk_ddp_comp_enable_vblank(struct mtk_ddp_comp *comp) { if (comp->funcs && comp->funcs->enable_vblank) - comp->funcs->enable_vblank(comp->dev, vblank_cb, vblank_cb_data); + comp->funcs->enable_vblank(comp->dev); } static inline void mtk_ddp_comp_disable_vblank(struct mtk_ddp_comp *comp)
We encountered a kernel panic issue that callback data will be NULL when it's using in ovl irq handler. There is a timing issue between mtk_disp_ovl_irq_handler() and mtk_ovl_disable_vblank(). To resolve this issue, we use the flow to register/unregister vblank cb: - Register callback function and callback data when crtc creates. - Unregister callback function and callback data when crtc destroies. With this solution, we can assure callback data will not be NULL when vblank is disable. Fixes: 9b0704988b15 ("drm/mediatek: Register vblank callback function") Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com> Reviewed-by: jason-jh.lin <jason-jh.lin@mediatek.com> --- drivers/gpu/drm/mediatek/mtk_disp_drv.h | 16 +++++++----- drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 22 ++++++++++++---- drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 20 +++++++++----- drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 14 +++++++++- drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 4 +++ drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 29 ++++++++++++++++----- 6 files changed, 80 insertions(+), 25 deletions(-)