Hi Maxime,
On Tue, 1 Jul 2025 16:27:54 +0200
Maxime Ripard <mrip...@kernel.org> wrote:
> On Tue, Jul 01, 2025 at 04:02:19PM +0200, Luca Ceresoli wrote:
> > Hello Marek, Maxime,
> >
> > thanks Marek for spotting the issue and sending a patch!
> >
> > On Mon, 30 Jun 2025 18:44:24 +0200
> > Maxime Ripard <mrip...@kernel.org> wrote:
> >
> > > > @@ -1643,7 +1625,7 @@ int analogix_dp_bind(struct analogix_dp_device
> > > > *dp, struct drm_device *drm_dev)
> > > > return ret;
> > > > }
> > > >
> > > > - ret = analogix_dp_create_bridge(drm_dev, dp);
> > > > + ret = drm_bridge_attach(dp->encoder, &dp->bridge, NULL, 0);
> > > > if (ret) {
> > > > DRM_ERROR("failed to create bridge (%d)\n", ret);
> > > > goto err_unregister_aux;
> > >
> > > It looks like you don't set bridge->driver_private anymore. Is it on
> > > purpose?
> >
> > This looks correct to me. In current code, driver_private is used to
> > hold a pointer to the driver private struct (struct
> > analogix_dp_device). With devm_drm_bridge_alloc() container_of() is now
> > enough, no pointer is needed. With the patch applied, driver_private
> > becomes unused.
>
> Then we should remove it from the structure if it's unused.
Makes sense now that struct drm_bridge is meant to be always embedded
in a driver-private struct. But several drivers are still using it, so
those would need to be updated beforehand:
$ git grep -l driver_private -- drivers/gpu/drm/ | wc -l
23
$
So I think this patch should be taken as it fixes a regression. Do you
agree on this?
Luca
--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
