Hi,
On Thu, Jun 25, 2026 at 09:55:47AM +0200, Thomas Zimmermann wrote:
> Hi Maxime
>
> Am 17.06.26 um 12:14 schrieb Maxime Ripard:
> > The chrontel-ch7033 bridge still uses the deprecated non-atomic bridge
> > callbacks.
> >
> > Switch to their atomic counterparts, adding the bridge state
> > handlers if not already present.
> >
> > Generated by the following Coccinelle script:
> >
> > @ is_bridge @
> > identifier funcs;
> > @@
> >
> > struct drm_bridge_funcs funcs = {
> > ...,
> > };
> >
> > @ has_create_state depends on is_bridge @
> > identifier funcs, f;
> > @@
> >
> > struct drm_bridge_funcs funcs = {
> > ...,
> > .atomic_create_state = f,
> > ...,
> > };
> >
> > @ update_struct depends on (is_bridge && !has_create_state) @
> > identifier is_bridge.funcs;
> > identifier f;
> > @@
> >
> > struct drm_bridge_funcs funcs = {
> > + .atomic_create_state = drm_atomic_helper_bridge_create_state,
> > + .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
> > + .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
> > ...,
> > };
> >
> > @ update_pre_enable_struct depends on (is_bridge && !has_create_state) @
> > identifier is_bridge.funcs;
> > identifier f;
> > @@
> >
> > struct drm_bridge_funcs funcs = {
> > ...,
> > - .pre_enable = f,
> > + .atomic_pre_enable = f,
> > ...,
> > };
> >
> > @ update_pre_enable_impl depends on update_pre_enable_struct @
> > identifier update_pre_enable_struct.f;
> > identifier b;
> > @@
> >
> > -void f(struct drm_bridge *b)
> > +void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
> > {
> > ...
> > }
> >
> > @ update_enable_struct depends on (is_bridge && !has_create_state) @
> > identifier is_bridge.funcs;
> > identifier f;
> > @@
> >
> > struct drm_bridge_funcs funcs = {
> > ...,
> > - .enable = f,
> > + .atomic_enable = f,
> > ...,
> > };
> >
> > @ update_enable_impl depends on update_enable_struct @
> > identifier update_enable_struct.f;
> > identifier b;
> > @@
> >
> > -void f(struct drm_bridge *b)
> > +void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
> > {
> > ...
> > }
> >
> > @ update_disable_struct depends on (is_bridge && !has_create_state) @
> > identifier is_bridge.funcs;
> > identifier f;
> > @@
> >
> > struct drm_bridge_funcs funcs = {
> > ...,
> > - .disable = f,
> > + .atomic_disable = f,
> > ...,
> > };
> >
> > @ update_disable_impl depends on update_disable_struct @
> > identifier update_disable_struct.f;
> > identifier b;
> > @@
> >
> > -void f(struct drm_bridge *b)
> > +void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
> > {
> > ...
> > }
> >
> > @ update_post_disable_struct depends on (is_bridge && !has_create_state) @
> > identifier is_bridge.funcs;
> > identifier f;
> > @@
> >
> > struct drm_bridge_funcs funcs = {
> > ...,
> > - .post_disable = f,
> > + .atomic_post_disable = f,
> > ...,
> > };
> >
> > @ update_post_disable_impl depends on update_post_disable_struct @
> > identifier update_post_disable_struct.f;
> > identifier b;
> > @@
> >
> > -void f(struct drm_bridge *b)
> > +void f(struct drm_bridge *b, struct drm_atomic_commit *commit)
> > {
> > ...
> > }
> >
> > Signed-off-by: Maxime Ripard <[email protected]>
> > ---
> > drivers/gpu/drm/bridge/chrontel-ch7033.c | 13 +++++++++----
> > 1 file changed, 9 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/chrontel-ch7033.c
> > b/drivers/gpu/drm/bridge/chrontel-ch7033.c
> > index a237c65ebd69..92d464727e41 100644
> > --- a/drivers/gpu/drm/bridge/chrontel-ch7033.c
> > +++ b/drivers/gpu/drm/bridge/chrontel-ch7033.c
> > @@ -328,19 +328,21 @@ static enum drm_mode_status
> > ch7033_bridge_mode_valid(struct drm_bridge *bridge,
> > if (mode->vdisplay >= 1080)
> > return MODE_BAD_VVALUE;
> > return MODE_OK;
> > }
> > -static void ch7033_bridge_disable(struct drm_bridge *bridge)
> > +static void ch7033_bridge_disable(struct drm_bridge *bridge,
> > + struct drm_atomic_commit *commit)
> > {
> > struct ch7033_priv *priv = bridge_to_ch7033_priv(bridge);
> > regmap_write(priv->regmap, 0x03, 0x04);
> > regmap_update_bits(priv->regmap, 0x52, RESETDB, 0x00);
> > }
> > -static void ch7033_bridge_enable(struct drm_bridge *bridge)
> > +static void ch7033_bridge_enable(struct drm_bridge *bridge,
> > + struct drm_atomic_commit *commit)
> > {
> > struct ch7033_priv *priv = bridge_to_ch7033_priv(bridge);
> > regmap_write(priv->regmap, 0x03, 0x04);
> > regmap_update_bits(priv->regmap, 0x52, RESETDB, RESETDB);
> > @@ -512,15 +514,18 @@ static void ch7033_bridge_mode_set(struct drm_bridge
> > *bridge,
> > regmap_write(priv->regmap, 0x11, mode->clock >> 8);
> > regmap_write(priv->regmap, 0x12, mode->clock);
> > }
> > static const struct drm_bridge_funcs ch7033_bridge_funcs = {
> > + .atomic_create_state = drm_atomic_helper_bridge_create_state,
> > + .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
> > + .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
> > .attach = ch7033_bridge_attach,
> > .detach = ch7033_bridge_detach,
> > .mode_valid = ch7033_bridge_mode_valid,
> > - .disable = ch7033_bridge_disable,
> > - .enable = ch7033_bridge_enable,
> > + .atomic_disable = ch7033_bridge_disable,
> > + .atomic_enable = ch7033_bridge_enable,
> > .mode_set = ch7033_bridge_mode_set,
>
> This driver uses mode_set and the docs have a number of comments on that.
> Does this need to be reworked as well?I guess it might, but not right now. mode_set is pretty bug-prone in general, but it's also not easy to mass-convert. drivers/gpu/drm//bridge/ti-dlpc3433.c is trivial to convert for example, but drivers/gpu/drm//bridge/tda998x_drv.c isn't, so I wouldn't be able to do it without a device. I can try to have a look later, but I'm not sure when this might be. Maxime
signature.asc
Description: PGP signature
