[PATCH] drm/bridge: analogix_dp: Use devm_drm_bridge_alloc() API

[Luca Ceresoli]

On Mon, 7 Jul 2025 11:07:26 +0200
Marek Szyprowski <m.szyprowski at samsung.com> wrote:

> On 03.07.2025 17:50, Luca Ceresoli wrote:
> > On Tue, 1 Jul 2025 16:27:54 +0200
> > Maxime Ripard <mripard at 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 <mripard at 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?  
> 
> Yes, please apply it as a fix :)
> 
> 
> BTW, there are 2 more bridge drivers that need a fix similar to the 
> $subject patch:
> 
> $ git grep "bridge = devm_kzalloc" drivers/gpu
> drivers/gpu/drm/sti/sti_hda.c:  bridge = devm_kzalloc(dev, 
> sizeof(*bridge), GFP_KERNEL);
> drivers/gpu/drm/sti/sti_hdmi.c: bridge = devm_kzalloc(dev, 
> sizeof(*bridge), GFP_KERNEL);

Ouch. My grep logic was probably too clever and missed these obvious
ones. I'm taking care of converting these ones later this week as time
permits, unless patches are sent before.

Thanks for reporting!

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com