| Message ID | 20230807015110.30579-3-jason-jh.lin@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix OVL iommu fault in cursor plane | expand |
Hi, Jason: On Mon, 2023-08-07 at 09:51 +0800, Jason-JH.Lin wrote: > The plane_state of drm_atomic_state is not sync to the > mtk_plane_state > stored in mtk_crtc during crtc enabling. > > So we need to update the mtk_plane_state stored in mtk_crtc by the > drm_atomic_state carried from mtk_drm_crtc_atomic_enable(). > > While updating mtk_plane_state, OVL layer should be disabled when the > fb > in plane_state of drm_atomic_state is NULL. > > Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC > MT8173.") > Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com> > --- > Change in RESEND v4: > Remove redundant plane_state assigning. > --- > drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 14 ++++++++++---- > drivers/gpu/drm/mediatek/mtk_drm_plane.c | 11 ++++++++--- > drivers/gpu/drm/mediatek/mtk_drm_plane.h | 2 ++ > 3 files changed, 20 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > index d40142842f85..7db4d6551da7 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > @@ -328,7 +328,7 @@ static void ddp_cmdq_cb(struct mbox_client *cl, > void *mssg) > } > #endif > > -static int mtk_crtc_ddp_hw_init(struct mtk_drm_crtc *mtk_crtc) > +static int mtk_crtc_ddp_hw_init(struct mtk_drm_crtc *mtk_crtc, > struct drm_atomic_state *state) > { > struct drm_crtc *crtc = &mtk_crtc->base; > struct drm_connector *connector; > @@ -405,11 +405,17 @@ static int mtk_crtc_ddp_hw_init(struct > mtk_drm_crtc *mtk_crtc) > /* Initially configure all planes */ > for (i = 0; i < mtk_crtc->layer_nr; i++) { > struct drm_plane *plane = &mtk_crtc->planes[i]; > - struct mtk_plane_state *plane_state; > + struct drm_plane_state *new_state; > + struct mtk_plane_state *plane_state = > to_mtk_plane_state(plane->state); > struct mtk_ddp_comp *comp; > unsigned int local_layer; > > - plane_state = to_mtk_plane_state(plane->state); > + /* sync the new plane state from drm_atomic_state */ > + if (state->planes[i].ptr) { > + new_state = > drm_atomic_get_new_plane_state(state, state->planes[i].ptr); for_each_new_plane_in_state()? > + mtk_plane_update_new_state(new_state, > plane_state); > + } > + > comp = mtk_drm_ddp_comp_for_plane(crtc, plane, > &local_layer); > if (comp) > mtk_ddp_comp_layer_config(comp, local_layer, > @@ -687,7 +693,7 @@ static void mtk_drm_crtc_atomic_enable(struct > drm_crtc *crtc, > return; > } > > - ret = mtk_crtc_ddp_hw_init(mtk_crtc); > + ret = mtk_crtc_ddp_hw_init(mtk_crtc, state); > if (ret) { > pm_runtime_put(comp->dev); > return; > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c > b/drivers/gpu/drm/mediatek/mtk_drm_plane.c > index b1a918ffe457..ef4460f98c07 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c > @@ -134,8 +134,8 @@ static int mtk_plane_atomic_async_check(struct > drm_plane *plane, > true, true); > } > > -static void mtk_plane_update_new_state(struct drm_plane_state > *new_state, > - struct mtk_plane_state > *mtk_plane_state) > +void mtk_plane_update_new_state(struct drm_plane_state *new_state, > + struct mtk_plane_state > *mtk_plane_state) > { > struct drm_framebuffer *fb = new_state->fb; > struct drm_gem_object *gem; > @@ -146,6 +146,11 @@ static void mtk_plane_update_new_state(struct > drm_plane_state *new_state, > dma_addr_t hdr_addr = 0; > unsigned int hdr_pitch = 0; > > + if (!fb) { > + mtk_plane_state->pending.enable = false; > + return; > + } This seems done in mtk_plane_atomic_update(), so you may call mtk_plane_atomic_update() instead of mtk_plane_update_new_state(). Regards, CK > + > gem = fb->obj[0]; > mtk_gem = to_mtk_gem_obj(gem); > addr = mtk_gem->dma_addr; > @@ -180,7 +185,7 @@ static void mtk_plane_update_new_state(struct > drm_plane_state *new_state, > fb->format->cpp[0] * (x_offset_in_blocks + 1); > } > > - mtk_plane_state->pending.enable = true; > + mtk_plane_state->pending.enable = new_state->visible; > mtk_plane_state->pending.pitch = pitch; > mtk_plane_state->pending.hdr_pitch = hdr_pitch; > mtk_plane_state->pending.format = format; > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.h > b/drivers/gpu/drm/mediatek/mtk_drm_plane.h > index 99aff7da0831..0a7d70d13e43 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.h > +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.h > @@ -46,6 +46,8 @@ to_mtk_plane_state(struct drm_plane_state *state) > return container_of(state, struct mtk_plane_state, base); > } > > +void mtk_plane_update_new_state(struct drm_plane_state *new_state, > + struct mtk_plane_state > *mtk_plane_state); > int mtk_plane_init(struct drm_device *dev, struct drm_plane *plane, > unsigned long possible_crtcs, enum drm_plane_type > type, > unsigned int supported_rotations, const u32 > *formats,
Hi Jason, You forgot to put the Reviewed-by got from the V3 in your commit message. On 07/08/2023 03:51, Jason-JH.Lin wrote: > The plane_state of drm_atomic_state is not sync to the mtk_plane_state > stored in mtk_crtc during crtc enabling. > > So we need to update the mtk_plane_state stored in mtk_crtc by the > drm_atomic_state carried from mtk_drm_crtc_atomic_enable(). > > While updating mtk_plane_state, OVL layer should be disabled when the fb > in plane_state of drm_atomic_state is NULL. > > Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC MT8173.") > Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com> > --- > Change in RESEND v4: > Remove redundant plane_state assigning. > ---
Hi Alexandre, On Mon, 2023-08-07 at 11:45 +0200, Alexandre Mergnat wrote: > > External email : Please do not click links or open attachments until > you have verified the sender or the content. > Hi Jason, > > You forgot to put the Reviewed-by got from the V3 in your commit > message. > Since I changed the original method at the new version, I removed all reviewed-by tags. In order to avoid the version you have reviewed is too different. Regards, Jason-JH.Lin > > On 07/08/2023 03:51, Jason-JH.Lin wrote: > > The plane_state of drm_atomic_state is not sync to the > mtk_plane_state > > stored in mtk_crtc during crtc enabling. > > > > So we need to update the mtk_plane_state stored in mtk_crtc by the > > drm_atomic_state carried from mtk_drm_crtc_atomic_enable(). > > > > While updating mtk_plane_state, OVL layer should be disabled when > the fb > > in plane_state of drm_atomic_state is NULL. > > > > Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC > MT8173.") > > Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com> > > --- > > Change in RESEND v4: > > Remove redundant plane_state assigning. > > --- > > > -- > Regards, > Alexandre
Hi CK, On Mon, 2023-08-07 at 08:59 +0000, CK Hu (胡俊光) wrote: > Hi, Jason: > > On Mon, 2023-08-07 at 09:51 +0800, Jason-JH.Lin wrote: > > The plane_state of drm_atomic_state is not sync to the > > mtk_plane_state > > stored in mtk_crtc during crtc enabling. > > > > So we need to update the mtk_plane_state stored in mtk_crtc by the > > drm_atomic_state carried from mtk_drm_crtc_atomic_enable(). > > > > While updating mtk_plane_state, OVL layer should be disabled when > > the > > fb > > in plane_state of drm_atomic_state is NULL. > > > > Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC > > MT8173.") > > Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com> > > --- > > Change in RESEND v4: > > Remove redundant plane_state assigning. > > --- > > drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 14 ++++++++++---- > > drivers/gpu/drm/mediatek/mtk_drm_plane.c | 11 ++++++++--- > > drivers/gpu/drm/mediatek/mtk_drm_plane.h | 2 ++ > > 3 files changed, 20 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > > b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > > index d40142842f85..7db4d6551da7 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > > @@ -328,7 +328,7 @@ static void ddp_cmdq_cb(struct mbox_client *cl, > > void *mssg) > > } > > #endif > > > > -static int mtk_crtc_ddp_hw_init(struct mtk_drm_crtc *mtk_crtc) > > +static int mtk_crtc_ddp_hw_init(struct mtk_drm_crtc *mtk_crtc, > > struct drm_atomic_state *state) > > { > > struct drm_crtc *crtc = &mtk_crtc->base; > > struct drm_connector *connector; > > @@ -405,11 +405,17 @@ static int mtk_crtc_ddp_hw_init(struct > > mtk_drm_crtc *mtk_crtc) > > /* Initially configure all planes */ > > for (i = 0; i < mtk_crtc->layer_nr; i++) { > > struct drm_plane *plane = &mtk_crtc->planes[i]; > > - struct mtk_plane_state *plane_state; > > + struct drm_plane_state *new_state; > > + struct mtk_plane_state *plane_state = > > to_mtk_plane_state(plane->state); > > struct mtk_ddp_comp *comp; > > unsigned int local_layer; > > > > - plane_state = to_mtk_plane_state(plane->state); > > + /* sync the new plane state from drm_atomic_state */ > > + if (state->planes[i].ptr) { > > + new_state = > > drm_atomic_get_new_plane_state(state, state->planes[i].ptr); > > for_each_new_plane_in_state()? I'vd tried for_each_new_plane_in_state(), but it crashed in the fifth round. Because mode_config.num_total_plane in this macro is 8, but mtk_crtc- >layer_nr is 4. So I think we can't use this macro here. > > > + mtk_plane_update_new_state(new_state, > > plane_state); > > + } > > + > > comp = mtk_drm_ddp_comp_for_plane(crtc, plane, > > &local_layer); > > if (comp) > > mtk_ddp_comp_layer_config(comp, local_layer, > > @@ -687,7 +693,7 @@ static void mtk_drm_crtc_atomic_enable(struct > > drm_crtc *crtc, > > return; > > } > > > > - ret = mtk_crtc_ddp_hw_init(mtk_crtc); > > + ret = mtk_crtc_ddp_hw_init(mtk_crtc, state); > > if (ret) { > > pm_runtime_put(comp->dev); > > return; > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c > > b/drivers/gpu/drm/mediatek/mtk_drm_plane.c > > index b1a918ffe457..ef4460f98c07 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c > > @@ -134,8 +134,8 @@ static int mtk_plane_atomic_async_check(struct > > drm_plane *plane, > > true, true); > > } > > > > -static void mtk_plane_update_new_state(struct drm_plane_state > > *new_state, > > - struct mtk_plane_state > > *mtk_plane_state) > > +void mtk_plane_update_new_state(struct drm_plane_state *new_state, > > + struct mtk_plane_state > > *mtk_plane_state) > > { > > struct drm_framebuffer *fb = new_state->fb; > > struct drm_gem_object *gem; > > @@ -146,6 +146,11 @@ static void mtk_plane_update_new_state(struct > > drm_plane_state *new_state, > > dma_addr_t hdr_addr = 0; > > unsigned int hdr_pitch = 0; > > > > + if (!fb) { > > + mtk_plane_state->pending.enable = false; > > + return; > > + } > > This seems done in mtk_plane_atomic_update(), so you may call > mtk_plane_atomic_update() instead of mtk_plane_update_new_state(). mtk_plane_atomic_update() won't update the mtk_plane_state in mtk_crtc, so I use mtk_plane_update_new_state() here. But I found that we can fix up iommu fault issue by handling the NULL fb case. So I'll change the previous method to disable the plane if its fb in new state is NULL. Regards, Jason-JH.Lin > > Regards, > CK > > > + > > gem = fb->obj[0]; > > mtk_gem = to_mtk_gem_obj(gem); > > addr = mtk_gem->dma_addr; > > @@ -180,7 +185,7 @@ static void mtk_plane_update_new_state(struct > > drm_plane_state *new_state, > > fb->format->cpp[0] * (x_offset_in_blocks + 1); > > } > > > > - mtk_plane_state->pending.enable = true; > > + mtk_plane_state->pending.enable = new_state->visible; > > mtk_plane_state->pending.pitch = pitch; > > mtk_plane_state->pending.hdr_pitch = hdr_pitch; > > mtk_plane_state->pending.format = format; > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.h > > b/drivers/gpu/drm/mediatek/mtk_drm_plane.h > > index 99aff7da0831..0a7d70d13e43 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.h > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.h > > @@ -46,6 +46,8 @@ to_mtk_plane_state(struct drm_plane_state *state) > > return container_of(state, struct mtk_plane_state, base); > > } > > > > +void mtk_plane_update_new_state(struct drm_plane_state *new_state, > > + struct mtk_plane_state > > *mtk_plane_state); > > int mtk_plane_init(struct drm_device *dev, struct drm_plane > > *plane, > > unsigned long possible_crtcs, enum drm_plane_type > > type, > > unsigned int supported_rotations, const u32 > > *formats,
Hi Jason, On 8/7/23 04:51, Jason-JH.Lin wrote: > The plane_state of drm_atomic_state is not sync to the mtk_plane_state > stored in mtk_crtc during crtc enabling. > > So we need to update the mtk_plane_state stored in mtk_crtc by the > drm_atomic_state carried from mtk_drm_crtc_atomic_enable(). > > While updating mtk_plane_state, OVL layer should be disabled when the fb > in plane_state of drm_atomic_state is NULL. > > Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC MT8173.") > Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com> > --- > Change in RESEND v4: > Remove redundant plane_state assigning. > --- > drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 14 ++++++++++---- > drivers/gpu/drm/mediatek/mtk_drm_plane.c | 11 ++++++++--- > drivers/gpu/drm/mediatek/mtk_drm_plane.h | 2 ++ > 3 files changed, 20 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > index d40142842f85..7db4d6551da7 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > @@ -328,7 +328,7 @@ static void ddp_cmdq_cb(struct mbox_client *cl, void *mssg) > } > #endif > > -static int mtk_crtc_ddp_hw_init(struct mtk_drm_crtc *mtk_crtc) > +static int mtk_crtc_ddp_hw_init(struct mtk_drm_crtc *mtk_crtc, struct drm_atomic_state *state) > { > struct drm_crtc *crtc = &mtk_crtc->base; > struct drm_connector *connector; > @@ -405,11 +405,17 @@ static int mtk_crtc_ddp_hw_init(struct mtk_drm_crtc *mtk_crtc) > /* Initially configure all planes */ > for (i = 0; i < mtk_crtc->layer_nr; i++) { > struct drm_plane *plane = &mtk_crtc->planes[i]; > - struct mtk_plane_state *plane_state; > + struct drm_plane_state *new_state; > + struct mtk_plane_state *plane_state = to_mtk_plane_state(plane->state); > struct mtk_ddp_comp *comp; > unsigned int local_layer; > > - plane_state = to_mtk_plane_state(plane->state); any reason why you moved the initialization of plane_state at the declaration phase ? > + /* sync the new plane state from drm_atomic_state */ > + if (state->planes[i].ptr) { > + new_state = drm_atomic_get_new_plane_state(state, state->planes[i].ptr); Can drm_atomic_get_new_plane_state fail ? and new_state becomes null ? I see mtk_plane_update_new_state assumes new_state being a correct state/pointer. Regards, > + mtk_plane_update_new_state(new_state, plane_state); > + } > + > comp = mtk_drm_ddp_comp_for_plane(crtc, plane, &local_layer); > if (comp) > mtk_ddp_comp_layer_config(comp, local_layer, > @@ -687,7 +693,7 @@ static void mtk_drm_crtc_atomic_enable(struct drm_crtc *crtc, > return; > } > > - ret = mtk_crtc_ddp_hw_init(mtk_crtc); > + ret = mtk_crtc_ddp_hw_init(mtk_crtc, state); > if (ret) { > pm_runtime_put(comp->dev); > return; > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c > index b1a918ffe457..ef4460f98c07 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c > @@ -134,8 +134,8 @@ static int mtk_plane_atomic_async_check(struct drm_plane *plane, > true, true); > } > > -static void mtk_plane_update_new_state(struct drm_plane_state *new_state, > - struct mtk_plane_state *mtk_plane_state) > +void mtk_plane_update_new_state(struct drm_plane_state *new_state, > + struct mtk_plane_state *mtk_plane_state) > { > struct drm_framebuffer *fb = new_state->fb; > struct drm_gem_object *gem; > @@ -146,6 +146,11 @@ static void mtk_plane_update_new_state(struct drm_plane_state *new_state, > dma_addr_t hdr_addr = 0; > unsigned int hdr_pitch = 0; > > + if (!fb) { > + mtk_plane_state->pending.enable = false; > + return; > + } > + > gem = fb->obj[0]; > mtk_gem = to_mtk_gem_obj(gem); > addr = mtk_gem->dma_addr; > @@ -180,7 +185,7 @@ static void mtk_plane_update_new_state(struct drm_plane_state *new_state, > fb->format->cpp[0] * (x_offset_in_blocks + 1); > } > > - mtk_plane_state->pending.enable = true; > + mtk_plane_state->pending.enable = new_state->visible; > mtk_plane_state->pending.pitch = pitch; > mtk_plane_state->pending.hdr_pitch = hdr_pitch; > mtk_plane_state->pending.format = format; > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.h b/drivers/gpu/drm/mediatek/mtk_drm_plane.h > index 99aff7da0831..0a7d70d13e43 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.h > +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.h > @@ -46,6 +46,8 @@ to_mtk_plane_state(struct drm_plane_state *state) > return container_of(state, struct mtk_plane_state, base); > } > > +void mtk_plane_update_new_state(struct drm_plane_state *new_state, > + struct mtk_plane_state *mtk_plane_state); > int mtk_plane_init(struct drm_device *dev, struct drm_plane *plane, > unsigned long possible_crtcs, enum drm_plane_type type, > unsigned int supported_rotations, const u32 *formats,
Hi Eugen, Thanks for the reviews. On Tue, 2023-08-08 at 14:55 +0300, Eugen Hristev wrote: > Hi Jason, > > On 8/7/23 04:51, Jason-JH.Lin wrote: > > The plane_state of drm_atomic_state is not sync to the > > mtk_plane_state > > stored in mtk_crtc during crtc enabling. > > > > So we need to update the mtk_plane_state stored in mtk_crtc by the > > drm_atomic_state carried from mtk_drm_crtc_atomic_enable(). > > > > While updating mtk_plane_state, OVL layer should be disabled when > > the fb > > in plane_state of drm_atomic_state is NULL. > > > > Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC > > MT8173.") > > Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com> > > --- > > Change in RESEND v4: > > Remove redundant plane_state assigning. > > --- > > drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 14 ++++++++++---- > > drivers/gpu/drm/mediatek/mtk_drm_plane.c | 11 ++++++++--- > > drivers/gpu/drm/mediatek/mtk_drm_plane.h | 2 ++ > > 3 files changed, 20 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > > b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > > index d40142842f85..7db4d6551da7 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > > @@ -328,7 +328,7 @@ static void ddp_cmdq_cb(struct mbox_client *cl, > > void *mssg) > > } > > #endif > > > > -static int mtk_crtc_ddp_hw_init(struct mtk_drm_crtc *mtk_crtc) > > +static int mtk_crtc_ddp_hw_init(struct mtk_drm_crtc *mtk_crtc, > > struct drm_atomic_state *state) > > { > > struct drm_crtc *crtc = &mtk_crtc->base; > > struct drm_connector *connector; > > @@ -405,11 +405,17 @@ static int mtk_crtc_ddp_hw_init(struct > > mtk_drm_crtc *mtk_crtc) > > /* Initially configure all planes */ > > for (i = 0; i < mtk_crtc->layer_nr; i++) { > > struct drm_plane *plane = &mtk_crtc->planes[i]; > > - struct mtk_plane_state *plane_state; > > + struct drm_plane_state *new_state; > > + struct mtk_plane_state *plane_state = > > to_mtk_plane_state(plane->state); > > struct mtk_ddp_comp *comp; > > unsigned int local_layer; > > > > - plane_state = to_mtk_plane_state(plane->state); > > any reason why you moved the initialization of plane_state at the > declaration phase ? > I just like to assign it at the declaration phase. But it's not related to this fix patch, so I'll move it back. Thanks. > > + /* sync the new plane state from drm_atomic_state */ > > + if (state->planes[i].ptr) { > > + new_state = > > drm_atomic_get_new_plane_state(state, state->planes[i].ptr); > > Can drm_atomic_get_new_plane_state fail ? and new_state becomes null > ? drm_atomic_get_new_plane_state() won't fail and new_state won't be NULL. But if state->planes[i].ptr is NULL, it'll crash in drm_plane_index() inside drm_atomic_get_new_plane_state(). > > I see mtk_plane_update_new_state assumes new_state being a correct > state/pointer. The usage is the same as mtk_plane_atomic_update(). Regards, Jason-JH.Lin > > Regards, >
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c index d40142842f85..7db4d6551da7 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c @@ -328,7 +328,7 @@ static void ddp_cmdq_cb(struct mbox_client *cl, void *mssg) } #endif -static int mtk_crtc_ddp_hw_init(struct mtk_drm_crtc *mtk_crtc) +static int mtk_crtc_ddp_hw_init(struct mtk_drm_crtc *mtk_crtc, struct drm_atomic_state *state) { struct drm_crtc *crtc = &mtk_crtc->base; struct drm_connector *connector; @@ -405,11 +405,17 @@ static int mtk_crtc_ddp_hw_init(struct mtk_drm_crtc *mtk_crtc) /* Initially configure all planes */ for (i = 0; i < mtk_crtc->layer_nr; i++) { struct drm_plane *plane = &mtk_crtc->planes[i]; - struct mtk_plane_state *plane_state; + struct drm_plane_state *new_state; + struct mtk_plane_state *plane_state = to_mtk_plane_state(plane->state); struct mtk_ddp_comp *comp; unsigned int local_layer; - plane_state = to_mtk_plane_state(plane->state); + /* sync the new plane state from drm_atomic_state */ + if (state->planes[i].ptr) { + new_state = drm_atomic_get_new_plane_state(state, state->planes[i].ptr); + mtk_plane_update_new_state(new_state, plane_state); + } + comp = mtk_drm_ddp_comp_for_plane(crtc, plane, &local_layer); if (comp) mtk_ddp_comp_layer_config(comp, local_layer, @@ -687,7 +693,7 @@ static void mtk_drm_crtc_atomic_enable(struct drm_crtc *crtc, return; } - ret = mtk_crtc_ddp_hw_init(mtk_crtc); + ret = mtk_crtc_ddp_hw_init(mtk_crtc, state); if (ret) { pm_runtime_put(comp->dev); return; diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c index b1a918ffe457..ef4460f98c07 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c @@ -134,8 +134,8 @@ static int mtk_plane_atomic_async_check(struct drm_plane *plane, true, true); } -static void mtk_plane_update_new_state(struct drm_plane_state *new_state, - struct mtk_plane_state *mtk_plane_state) +void mtk_plane_update_new_state(struct drm_plane_state *new_state, + struct mtk_plane_state *mtk_plane_state) { struct drm_framebuffer *fb = new_state->fb; struct drm_gem_object *gem; @@ -146,6 +146,11 @@ static void mtk_plane_update_new_state(struct drm_plane_state *new_state, dma_addr_t hdr_addr = 0; unsigned int hdr_pitch = 0; + if (!fb) { + mtk_plane_state->pending.enable = false; + return; + } + gem = fb->obj[0]; mtk_gem = to_mtk_gem_obj(gem); addr = mtk_gem->dma_addr; @@ -180,7 +185,7 @@ static void mtk_plane_update_new_state(struct drm_plane_state *new_state, fb->format->cpp[0] * (x_offset_in_blocks + 1); } - mtk_plane_state->pending.enable = true; + mtk_plane_state->pending.enable = new_state->visible; mtk_plane_state->pending.pitch = pitch; mtk_plane_state->pending.hdr_pitch = hdr_pitch; mtk_plane_state->pending.format = format; diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.h b/drivers/gpu/drm/mediatek/mtk_drm_plane.h index 99aff7da0831..0a7d70d13e43 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.h +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.h @@ -46,6 +46,8 @@ to_mtk_plane_state(struct drm_plane_state *state) return container_of(state, struct mtk_plane_state, base); } +void mtk_plane_update_new_state(struct drm_plane_state *new_state, + struct mtk_plane_state *mtk_plane_state); int mtk_plane_init(struct drm_device *dev, struct drm_plane *plane, unsigned long possible_crtcs, enum drm_plane_type type, unsigned int supported_rotations, const u32 *formats,
The plane_state of drm_atomic_state is not sync to the mtk_plane_state stored in mtk_crtc during crtc enabling. So we need to update the mtk_plane_state stored in mtk_crtc by the drm_atomic_state carried from mtk_drm_crtc_atomic_enable(). While updating mtk_plane_state, OVL layer should be disabled when the fb in plane_state of drm_atomic_state is NULL. Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC MT8173.") Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com> --- Change in RESEND v4: Remove redundant plane_state assigning. --- drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 14 ++++++++++---- drivers/gpu/drm/mediatek/mtk_drm_plane.c | 11 ++++++++--- drivers/gpu/drm/mediatek/mtk_drm_plane.h | 2 ++ 3 files changed, 20 insertions(+), 7 deletions(-)