On Thu, Nov 05, 2020 at 05:45:18PM +0100, Maxime Ripard wrote: > Many drivers reference the crtc->pointer in order to get the current CRTC > state in their atomic_begin or atomic_flush hooks, which would be the new > CRTC state in the global atomic state since _swap_state happened when those > hooks are run. > > Use the drm_atomic_get_new_crtc_state helper to get that state to make it > more obvious. > > This was made using the coccinelle script below: > > @ crtc_atomic_func @ > identifier helpers; > identifier func; > @@ > > ( > static struct drm_crtc_helper_funcs helpers = { > ..., > .atomic_begin = func, > ..., > }; > | > static struct drm_crtc_helper_funcs helpers = { > ..., > .atomic_flush = func, > ..., > }; > ) > > @@ > identifier crtc_atomic_func.func; > identifier crtc, state; > symbol crtc_state; > expression e; > @@ > > func(struct drm_crtc *crtc, struct drm_atomic_state *state) { > ... > - struct tegra_dc_state *crtc_state = e; > + struct tegra_dc_state *dc_state = e; > <+... > - crtc_state > + dc_state > ...+> > } > > @@ > identifier crtc_atomic_func.func; > identifier crtc, state; > symbol crtc_state; > expression e; > @@ > > func(struct drm_crtc *crtc, struct drm_atomic_state *state) { > ... > - struct mtk_crtc_state *crtc_state = e; > + struct mtk_crtc_state *mtk_crtc_state = e; > <+... > - crtc_state > + mtk_crtc_state > ...+> > } > > @ replaces_new_state @ > identifier crtc_atomic_func.func; > identifier crtc, state, crtc_state; > @@ > > func(struct drm_crtc *crtc, struct drm_atomic_state *state) { > ... > - struct drm_crtc_state *crtc_state = crtc->state; > + struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, crtc); > ... > } > > @@ > identifier crtc_atomic_func.func; > identifier crtc, state, crtc_state; > @@ > > func(struct drm_crtc *crtc, struct drm_atomic_state *state) { > struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, crtc); > ... > - crtc->state > + crtc_state > ... > } > > @ adds_new_state @ > identifier crtc_atomic_func.func; > identifier crtc, state; > @@ > > func(struct drm_crtc *crtc, struct drm_atomic_state *state) { > + struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, crtc); > ... > - crtc->state > + crtc_state > ... > } > > @ include depends on adds_new_state || replaces_new_state @ > @@ > > #include <drm/drm_atomic.h> > > @ no_include depends on !include && (adds_new_state || replaces_new_state) @ > @@ > > + #include <drm/drm_atomic.h> > #include <drm/...> > > Cc: "James (Qian) Wang" <james.qian.wang@xxxxxxx> > Cc: Liviu Dudau <liviu.dudau@xxxxxxx> > Cc: Mihail Atanassov <mihail.atanassov@xxxxxxx> > Cc: Brian Starkey <brian.starkey@xxxxxxx> > Cc: Russell King <linux@xxxxxxxxxxxxxxx> > Cc: Paul Cercueil <paul@xxxxxxxxxxxxxxx> > Cc: Chun-Kuang Hu <chunkuang.hu@xxxxxxxxxx> > Cc: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx> > Cc: Sandy Huang <hjc@xxxxxxxxxxxxxx> > Cc: "Heiko Stübner" <heiko@xxxxxxxxx> > Cc: Thierry Reding <thierry.reding@xxxxxxxxx> > Cc: Gerd Hoffmann <kraxel@xxxxxxxxxx> > Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > Suggested-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > Signed-off-by: Maxime Ripard <maxime@xxxxxxxxxx> > > --- > > Changes from v1: > - Fixed checkpatch warnings > --- > drivers/gpu/drm/arm/display/komeda/komeda_crtc.c | 4 +++- > drivers/gpu/drm/armada/armada_crtc.c | 8 ++++++-- > drivers/gpu/drm/ast/ast_mode.c | 4 +++- > drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 7 +++++-- > drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 15 +++++++++------ > drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 6 ++++-- > drivers/gpu/drm/tegra/dc.c | 8 +++++--- > drivers/gpu/drm/virtio/virtgpu_display.c | 4 +++- > 8 files changed, 38 insertions(+), 18 deletions(-) > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c > index df0b9eeb8933..4b485eb512e2 100644 > --- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c > @@ -387,10 +387,12 @@ static void > komeda_crtc_atomic_flush(struct drm_crtc *crtc, > struct drm_atomic_state *state) > { > + struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, > + crtc); > struct drm_crtc_state *old = drm_atomic_get_old_crtc_state(state, > crtc); > /* commit with modeset will be handled in enable/disable */ > - if (drm_atomic_crtc_needs_modeset(crtc->state)) > + if (drm_atomic_crtc_needs_modeset(crtc_state)) > return; > > komeda_crtc_do_flush(crtc, old); For the komeda part: Acked-by: Liviu Dudau <liviu@xxxxxxxxxxx> > diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c > index ca643f4e2064..3ebcf5a52c8b 100644 > --- a/drivers/gpu/drm/armada/armada_crtc.c > +++ b/drivers/gpu/drm/armada/armada_crtc.c > @@ -431,11 +431,13 @@ static int armada_drm_crtc_atomic_check(struct drm_crtc *crtc, > static void armada_drm_crtc_atomic_begin(struct drm_crtc *crtc, > struct drm_atomic_state *state) > { > + struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, > + crtc); > struct armada_crtc *dcrtc = drm_to_armada_crtc(crtc); > > DRM_DEBUG_KMS("[CRTC:%d:%s]\n", crtc->base.id, crtc->name); > > - if (crtc->state->color_mgmt_changed) > + if (crtc_state->color_mgmt_changed) > armada_drm_update_gamma(crtc); > > dcrtc->regs_idx = 0; > @@ -445,6 +447,8 @@ static void armada_drm_crtc_atomic_begin(struct drm_crtc *crtc, > static void armada_drm_crtc_atomic_flush(struct drm_crtc *crtc, > struct drm_atomic_state *state) > { > + struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, > + crtc); > struct armada_crtc *dcrtc = drm_to_armada_crtc(crtc); > > DRM_DEBUG_KMS("[CRTC:%d:%s]\n", crtc->base.id, crtc->name); > @@ -455,7 +459,7 @@ static void armada_drm_crtc_atomic_flush(struct drm_crtc *crtc, > * If we aren't doing a full modeset, then we need to queue > * the event here. > */ > - if (!drm_atomic_crtc_needs_modeset(crtc->state)) { > + if (!drm_atomic_crtc_needs_modeset(crtc_state)) { > dcrtc->update_pending = true; > armada_drm_crtc_queue_state_event(crtc); > spin_lock_irq(&dcrtc->irq_lock); > diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c > index 22f0e65fbe9a..9db371f4054f 100644 > --- a/drivers/gpu/drm/ast/ast_mode.c > +++ b/drivers/gpu/drm/ast/ast_mode.c > @@ -782,10 +782,12 @@ static void > ast_crtc_helper_atomic_flush(struct drm_crtc *crtc, > struct drm_atomic_state *state) > { > + struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, > + crtc); > struct drm_crtc_state *old_crtc_state = drm_atomic_get_old_crtc_state(state, > crtc); > struct ast_private *ast = to_ast_private(crtc->dev); > - struct ast_crtc_state *ast_crtc_state = to_ast_crtc_state(crtc->state); > + struct ast_crtc_state *ast_crtc_state = to_ast_crtc_state(crtc_state); > struct ast_crtc_state *old_ast_crtc_state = to_ast_crtc_state(old_crtc_state); > > /* > diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c > index b9c156e13156..7603f86dd0d1 100644 > --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c > +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c > @@ -305,11 +305,13 @@ ingenic_drm_crtc_mode_valid(struct drm_crtc *crtc, const struct drm_display_mode > static void ingenic_drm_crtc_atomic_begin(struct drm_crtc *crtc, > struct drm_atomic_state *state) > { > + struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, > + crtc); > struct ingenic_drm *priv = drm_crtc_get_priv(crtc); > u32 ctrl = 0; > > if (priv->soc_info->has_osd && > - drm_atomic_crtc_needs_modeset(crtc->state)) { > + drm_atomic_crtc_needs_modeset(crtc_state)) { > /* > * If IPU plane is enabled, enable IPU as source for the F1 > * plane; otherwise use regular DMA. > @@ -326,7 +328,8 @@ static void ingenic_drm_crtc_atomic_flush(struct drm_crtc *crtc, > struct drm_atomic_state *state) > { > struct ingenic_drm *priv = drm_crtc_get_priv(crtc); > - struct drm_crtc_state *crtc_state = crtc->state; > + struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, > + crtc); > struct drm_pending_vblank_event *event = crtc_state->event; > > if (drm_atomic_crtc_needs_modeset(crtc_state)) { > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > index 23f5c10b0c67..193848fd7515 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > @@ -11,6 +11,7 @@ > #include <asm/barrier.h> > #include <soc/mediatek/smi.h> > > +#include <drm/drm_atomic.h> > #include <drm/drm_atomic_helper.h> > #include <drm/drm_plane_helper.h> > #include <drm/drm_probe_helper.h> > @@ -577,17 +578,19 @@ static void mtk_drm_crtc_atomic_disable(struct drm_crtc *crtc, > static void mtk_drm_crtc_atomic_begin(struct drm_crtc *crtc, > struct drm_atomic_state *state) > { > - struct mtk_crtc_state *crtc_state = to_mtk_crtc_state(crtc->state); > + struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, > + crtc); > + struct mtk_crtc_state *mtk_crtc_state = to_mtk_crtc_state(crtc_state); > struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc); > > - if (mtk_crtc->event && crtc_state->base.event) > + if (mtk_crtc->event && mtk_crtc_state->base.event) > DRM_ERROR("new event while there is still a pending event\n"); > > - if (crtc_state->base.event) { > - crtc_state->base.event->pipe = drm_crtc_index(crtc); > + if (mtk_crtc_state->base.event) { > + mtk_crtc_state->base.event->pipe = drm_crtc_index(crtc); > WARN_ON(drm_crtc_vblank_get(crtc) != 0); > - mtk_crtc->event = crtc_state->base.event; > - crtc_state->base.event = NULL; > + mtk_crtc->event = mtk_crtc_state->base.event; > + mtk_crtc_state->base.event = NULL; > } > } > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > index 8cd39fca81a3..d1e05482641b 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > @@ -1248,6 +1248,8 @@ static void vop_crtc_gamma_set(struct vop *vop, struct drm_crtc *crtc, > static void vop_crtc_atomic_begin(struct drm_crtc *crtc, > struct drm_atomic_state *state) > { > + struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, > + crtc); > struct drm_crtc_state *old_crtc_state = drm_atomic_get_old_crtc_state(state, > crtc); > struct vop *vop = to_vop(crtc); > @@ -1256,8 +1258,8 @@ static void vop_crtc_atomic_begin(struct drm_crtc *crtc, > * Only update GAMMA if the 'active' flag is not changed, > * otherwise it's updated by .atomic_enable. > */ > - if (crtc->state->color_mgmt_changed && > - !crtc->state->active_changed) > + if (crtc_state->color_mgmt_changed && > + !crtc_state->active_changed) > vop_crtc_gamma_set(vop, crtc, old_crtc_state); > } > > diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c > index 2d86627b0d4e..85dd7131553a 100644 > --- a/drivers/gpu/drm/tegra/dc.c > +++ b/drivers/gpu/drm/tegra/dc.c > @@ -1939,15 +1939,17 @@ static void tegra_crtc_atomic_begin(struct drm_crtc *crtc, > static void tegra_crtc_atomic_flush(struct drm_crtc *crtc, > struct drm_atomic_state *state) > { > - struct tegra_dc_state *crtc_state = to_dc_state(crtc->state); > + struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, > + crtc); > + struct tegra_dc_state *dc_state = to_dc_state(crtc_state); > struct tegra_dc *dc = to_tegra_dc(crtc); > u32 value; > > - value = crtc_state->planes << 8 | GENERAL_UPDATE; > + value = dc_state->planes << 8 | GENERAL_UPDATE; > tegra_dc_writel(dc, value, DC_CMD_STATE_CONTROL); > value = tegra_dc_readl(dc, DC_CMD_STATE_CONTROL); > > - value = crtc_state->planes | GENERAL_ACT_REQ; > + value = dc_state->planes | GENERAL_ACT_REQ; > tegra_dc_writel(dc, value, DC_CMD_STATE_CONTROL); > value = tegra_dc_readl(dc, DC_CMD_STATE_CONTROL); > } > diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c b/drivers/gpu/drm/virtio/virtgpu_display.c > index 4bf74836bd53..a6caebd4a0dd 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_display.c > +++ b/drivers/gpu/drm/virtio/virtgpu_display.c > @@ -119,6 +119,8 @@ static int virtio_gpu_crtc_atomic_check(struct drm_crtc *crtc, > static void virtio_gpu_crtc_atomic_flush(struct drm_crtc *crtc, > struct drm_atomic_state *state) > { > + struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, > + crtc); > struct virtio_gpu_output *output = drm_crtc_to_virtio_gpu_output(crtc); > > /* > @@ -127,7 +129,7 @@ static void virtio_gpu_crtc_atomic_flush(struct drm_crtc *crtc, > * in the plane update callback, and here we just check > * whenever we must force the modeset. > */ > - if (drm_atomic_crtc_needs_modeset(crtc->state)) { > + if (drm_atomic_crtc_needs_modeset(crtc_state)) { > output->needs_modeset = true; > } > } > -- > 2.28.0 > -- ==================== | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --------------- ¯\_(ツ)_/¯ _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel