| Message ID | 20250204-bridge-connector-v2-32-35dd6c834e08@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/bridge: Various quality of life improvements | expand |
On Tue, Feb 04, 2025 at 03:58:00PM +0100, Maxime Ripard wrote: > The Cadence DSI driver follows the drm_encoder->crtc pointer that is > deprecated and shouldn't be used by atomic drivers. > > This was due to the fact that we did't have any other alternative to > retrieve the CRTC pointer. See below. > Fortunately, the crtc pointer is now provided Nit: CRTC > in the bridge state, so we can move to atomic callbacks and drop that > deprecated pointer usage. > > Signed-off-by: Maxime Ripard <mripard@kernel.org> > --- > drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c | 27 +++++++++++++++++--------- > 1 file changed, 18 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c > index 8f54c034ac4f3e82c38607a0e52d4745654b571f..c856e7843f83b363340443dec1deb26c9ae1b912 100644 > --- a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c > +++ b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c > @@ -654,11 +654,12 @@ cdns_dsi_bridge_mode_valid(struct drm_bridge *bridge, > return MODE_BAD; > > return MODE_OK; > } > > -static void cdns_dsi_bridge_disable(struct drm_bridge *bridge) > +static void cdns_dsi_bridge_atomic_disable(struct drm_bridge *bridge, > + struct drm_atomic_state *state) > { > struct cdns_dsi_input *input = bridge_to_cdns_dsi_input(bridge); > struct cdns_dsi *dsi = input_to_dsi(input); > u32 val; > > @@ -674,11 +675,12 @@ static void cdns_dsi_bridge_disable(struct drm_bridge *bridge) > dsi->platform_ops->disable(dsi); > > pm_runtime_put(dsi->base.dev); > } > > -static void cdns_dsi_bridge_post_disable(struct drm_bridge *bridge) > +static void cdns_dsi_bridge_atomic_post_disable(struct drm_bridge *bridge, > + struct drm_atomic_state *state) > { > struct cdns_dsi_input *input = bridge_to_cdns_dsi_input(bridge); > struct cdns_dsi *dsi = input_to_dsi(input); > > pm_runtime_put(dsi->base.dev); > @@ -751,15 +753,17 @@ static void cdns_dsi_init_link(struct cdns_dsi *dsi) > writel(val, dsi->regs + MCTL_MAIN_EN); > > dsi->link_initialized = true; > } > > -static void cdns_dsi_bridge_enable(struct drm_bridge *bridge) > +static void cdns_dsi_bridge_atomic_enable(struct drm_bridge *bridge, > + struct drm_atomic_state *state) > { > struct cdns_dsi_input *input = bridge_to_cdns_dsi_input(bridge); > struct cdns_dsi *dsi = input_to_dsi(input); > struct cdns_dsi_output *output = &dsi->output; > + struct drm_bridge_state *bridge_state; > struct drm_display_mode *mode; > struct phy_configure_opts_mipi_dphy *phy_cfg = &output->phy_opts.mipi_dphy; > unsigned long tx_byte_period; > struct cdns_dsi_cfg dsi_cfg; > u32 tmp, reg_wakeup, div; > @@ -769,11 +773,12 @@ static void cdns_dsi_bridge_enable(struct drm_bridge *bridge) > return; > > if (dsi->platform_ops && dsi->platform_ops->enable) > dsi->platform_ops->enable(dsi); > > - mode = &bridge->encoder->crtc->state->adjusted_mode; > + bridge_state = drm_atomic_get_new_bridge_state(state, bridge); > + mode = &bridge_state->crtc->state->adjusted_mode; And then it also follows crtc->state, which might fail for non-blocking commits. I think a proper way would be to connector = drm_atomic_get_new_connector_for_encoder(state, encoder); conn_state = drm_atomic_get_new_connector_state(state, connector); crtc_state = drm_atomic_get_new_crtc_state(state, conn_state->crtc); > nlanes = output->dev->lanes; > > WARN_ON_ONCE(cdns_dsi_check_conf(dsi, mode, &dsi_cfg, false)); > > cdns_dsi_hs_init(dsi); > @@ -891,11 +896,12 @@ static void cdns_dsi_bridge_enable(struct drm_bridge *bridge) > > tmp = readl(dsi->regs + MCTL_MAIN_EN) | IF_EN(input->id); > writel(tmp, dsi->regs + MCTL_MAIN_EN); > } > > -static void cdns_dsi_bridge_pre_enable(struct drm_bridge *bridge) > +static void cdns_dsi_bridge_atomic_pre_enable(struct drm_bridge *bridge, > + struct drm_atomic_state *state) > { > struct cdns_dsi_input *input = bridge_to_cdns_dsi_input(bridge); > struct cdns_dsi *dsi = input_to_dsi(input); > > if (WARN_ON(pm_runtime_get_sync(dsi->base.dev) < 0)) > @@ -906,14 +912,17 @@ static void cdns_dsi_bridge_pre_enable(struct drm_bridge *bridge) > } > > static const struct drm_bridge_funcs cdns_dsi_bridge_funcs = { > .attach = cdns_dsi_bridge_attach, > .mode_valid = cdns_dsi_bridge_mode_valid, > - .disable = cdns_dsi_bridge_disable, > - .pre_enable = cdns_dsi_bridge_pre_enable, > - .enable = cdns_dsi_bridge_enable, > - .post_disable = cdns_dsi_bridge_post_disable, > + .atomic_disable = cdns_dsi_bridge_atomic_disable, > + .atomic_pre_enable = cdns_dsi_bridge_atomic_pre_enable, > + .atomic_enable = cdns_dsi_bridge_atomic_enable, > + .atomic_post_disable = cdns_dsi_bridge_atomic_post_disable, > + .atomic_reset = drm_atomic_helper_bridge_reset, > + .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state, > + .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state, > }; > > static int cdns_dsi_attach(struct mipi_dsi_host *host, > struct mipi_dsi_device *dev) > { > > -- > 2.48.0 >
diff --git a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c index 8f54c034ac4f3e82c38607a0e52d4745654b571f..c856e7843f83b363340443dec1deb26c9ae1b912 100644 --- a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c +++ b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c @@ -654,11 +654,12 @@ cdns_dsi_bridge_mode_valid(struct drm_bridge *bridge, return MODE_BAD; return MODE_OK; } -static void cdns_dsi_bridge_disable(struct drm_bridge *bridge) +static void cdns_dsi_bridge_atomic_disable(struct drm_bridge *bridge, + struct drm_atomic_state *state) { struct cdns_dsi_input *input = bridge_to_cdns_dsi_input(bridge); struct cdns_dsi *dsi = input_to_dsi(input); u32 val; @@ -674,11 +675,12 @@ static void cdns_dsi_bridge_disable(struct drm_bridge *bridge) dsi->platform_ops->disable(dsi); pm_runtime_put(dsi->base.dev); } -static void cdns_dsi_bridge_post_disable(struct drm_bridge *bridge) +static void cdns_dsi_bridge_atomic_post_disable(struct drm_bridge *bridge, + struct drm_atomic_state *state) { struct cdns_dsi_input *input = bridge_to_cdns_dsi_input(bridge); struct cdns_dsi *dsi = input_to_dsi(input); pm_runtime_put(dsi->base.dev); @@ -751,15 +753,17 @@ static void cdns_dsi_init_link(struct cdns_dsi *dsi) writel(val, dsi->regs + MCTL_MAIN_EN); dsi->link_initialized = true; } -static void cdns_dsi_bridge_enable(struct drm_bridge *bridge) +static void cdns_dsi_bridge_atomic_enable(struct drm_bridge *bridge, + struct drm_atomic_state *state) { struct cdns_dsi_input *input = bridge_to_cdns_dsi_input(bridge); struct cdns_dsi *dsi = input_to_dsi(input); struct cdns_dsi_output *output = &dsi->output; + struct drm_bridge_state *bridge_state; struct drm_display_mode *mode; struct phy_configure_opts_mipi_dphy *phy_cfg = &output->phy_opts.mipi_dphy; unsigned long tx_byte_period; struct cdns_dsi_cfg dsi_cfg; u32 tmp, reg_wakeup, div; @@ -769,11 +773,12 @@ static void cdns_dsi_bridge_enable(struct drm_bridge *bridge) return; if (dsi->platform_ops && dsi->platform_ops->enable) dsi->platform_ops->enable(dsi); - mode = &bridge->encoder->crtc->state->adjusted_mode; + bridge_state = drm_atomic_get_new_bridge_state(state, bridge); + mode = &bridge_state->crtc->state->adjusted_mode; nlanes = output->dev->lanes; WARN_ON_ONCE(cdns_dsi_check_conf(dsi, mode, &dsi_cfg, false)); cdns_dsi_hs_init(dsi); @@ -891,11 +896,12 @@ static void cdns_dsi_bridge_enable(struct drm_bridge *bridge) tmp = readl(dsi->regs + MCTL_MAIN_EN) | IF_EN(input->id); writel(tmp, dsi->regs + MCTL_MAIN_EN); } -static void cdns_dsi_bridge_pre_enable(struct drm_bridge *bridge) +static void cdns_dsi_bridge_atomic_pre_enable(struct drm_bridge *bridge, + struct drm_atomic_state *state) { struct cdns_dsi_input *input = bridge_to_cdns_dsi_input(bridge); struct cdns_dsi *dsi = input_to_dsi(input); if (WARN_ON(pm_runtime_get_sync(dsi->base.dev) < 0)) @@ -906,14 +912,17 @@ static void cdns_dsi_bridge_pre_enable(struct drm_bridge *bridge) } static const struct drm_bridge_funcs cdns_dsi_bridge_funcs = { .attach = cdns_dsi_bridge_attach, .mode_valid = cdns_dsi_bridge_mode_valid, - .disable = cdns_dsi_bridge_disable, - .pre_enable = cdns_dsi_bridge_pre_enable, - .enable = cdns_dsi_bridge_enable, - .post_disable = cdns_dsi_bridge_post_disable, + .atomic_disable = cdns_dsi_bridge_atomic_disable, + .atomic_pre_enable = cdns_dsi_bridge_atomic_pre_enable, + .atomic_enable = cdns_dsi_bridge_atomic_enable, + .atomic_post_disable = cdns_dsi_bridge_atomic_post_disable, + .atomic_reset = drm_atomic_helper_bridge_reset, + .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state, + .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state, }; static int cdns_dsi_attach(struct mipi_dsi_host *host, struct mipi_dsi_device *dev) {
The Cadence DSI driver follows the drm_encoder->crtc pointer that is deprecated and shouldn't be used by atomic drivers. This was due to the fact that we did't have any other alternative to retrieve the CRTC pointer. Fortunately, the crtc pointer is now provided in the bridge state, so we can move to atomic callbacks and drop that deprecated pointer usage. Signed-off-by: Maxime Ripard <mripard@kernel.org> --- drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c | 27 +++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-)