Re: [PATCH 27/29] drm/bridge: tc358775: Switch to atomic commit
From: Dmitry Baryshkov
Date: Wed Jan 15 2025 - 20:06:59 EST
On Wed, Jan 15, 2025 at 10:05:34PM +0100, Maxime Ripard wrote:
> The tc358775 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@xxxxxxxxxx>
> ---
> drivers/gpu/drm/bridge/tc358775.c | 32 +++++++++++---------------------
> 1 file changed, 11 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/tc358775.c b/drivers/gpu/drm/bridge/tc358775.c
> index 0b4efaca6d682320b76ce09ed41824ae7f84ca2d..8f8ed8dc033daf001fc188d919fb38918673bd72 100644
> --- a/drivers/gpu/drm/bridge/tc358775.c
> +++ b/drivers/gpu/drm/bridge/tc358775.c
> @@ -285,11 +285,12 @@ struct tc_data {
> static inline struct tc_data *bridge_to_tc(struct drm_bridge *b)
> {
> return container_of(b, struct tc_data, bridge);
> }
>
> -static void tc_bridge_pre_enable(struct drm_bridge *bridge)
> +static void tc_bridge_atomic_pre_enable(struct drm_bridge *bridge,
> + struct drm_bridge_state *bridge_state)
> {
> struct tc_data *tc = bridge_to_tc(bridge);
> struct device *dev = &tc->dsi->dev;
> int ret;
>
> @@ -308,11 +309,12 @@ static void tc_bridge_pre_enable(struct drm_bridge *bridge)
>
> gpiod_set_value(tc->reset_gpio, 0);
> usleep_range(10, 20);
> }
>
> -static void tc_bridge_post_disable(struct drm_bridge *bridge)
> +static void tc_bridge_atomic_post_disable(struct drm_bridge *bridge,
> + struct drm_bridge_state *bridge_state)
> {
> struct tc_data *tc = bridge_to_tc(bridge);
> struct device *dev = &tc->dsi->dev;
> int ret;
>
> @@ -367,34 +369,22 @@ static void d2l_write(struct i2c_client *i2c, u16 addr, u32 val)
> if (ret < 0)
> dev_err(&i2c->dev, "Error %d writing to subaddress 0x%x\n",
> ret, addr);
> }
>
> -/* helper function to access bus_formats */
> -static struct drm_connector *get_connector(struct drm_encoder *encoder)
> -{
> - struct drm_device *dev = encoder->dev;
> - struct drm_connector *connector;
> -
> - list_for_each_entry(connector, &dev->mode_config.connector_list, head)
> - if (connector->encoder == encoder)
> - return connector;
> -
> - return NULL;
> -}
> -
> -static void tc_bridge_enable(struct drm_bridge *bridge)
> +static void tc_bridge_atomic_enable(struct drm_bridge *bridge,
> + struct drm_bridge_state *bridge_state)
> {
> struct tc_data *tc = bridge_to_tc(bridge);
> u32 hback_porch, hsync_len, hfront_porch, hactive, htime1, htime2;
> u32 vback_porch, vsync_len, vfront_porch, vactive, vtime1, vtime2;
> u32 val = 0;
> u16 dsiclk, clkdiv, byteclk, t1, t2, t3, vsdelay;
> struct drm_display_mode *mode;
> - struct drm_connector *connector = get_connector(bridge->encoder);
> + struct drm_connector *connector = bridge_state->connector;
>
> - mode = &bridge->encoder->crtc->state->adjusted_mode;
> + mode = &bridge_state->crtc->state->adjusted_mode;
>
> hback_porch = mode->htotal - mode->hsync_end;
> hsync_len = mode->hsync_end - mode->hsync_start;
> vback_porch = mode->vtotal - mode->vsync_end;
> vsync_len = mode->vsync_end - mode->vsync_start;
> @@ -599,14 +589,14 @@ static int tc_bridge_attach(struct drm_bridge *bridge,
> &tc->bridge, flags);
> }
>
> static const struct drm_bridge_funcs tc_bridge_funcs = {
> .attach = tc_bridge_attach,
> - .pre_enable = tc_bridge_pre_enable,
> - .enable = tc_bridge_enable,
> + .atomic_pre_enable = tc_bridge_atomic_pre_enable,
> + .atomic_enable = tc_bridge_atomic_enable,
> .mode_valid = tc_mode_valid,
> - .post_disable = tc_bridge_post_disable,
> + .atomic_post_disable = tc_bridge_atomic_post_disable,
Same comment: we have to provide state-management callbacks.
> };
>
> static int tc_attach_host(struct tc_data *tc)
> {
> struct device *dev = &tc->i2c->dev;
>
> --
> 2.47.1
>
--
With best wishes
Dmitry