Hi,
On Tue, Jul 08, 2025 at 05:24:43PM +0200, Luca Ceresoli wrote:
> devm_drm_bridge_alloc() is the new API to be used for allocating (and
> partially initializing) a private driver struct embedding a struct
> drm_bridge.
>
> This driver was missed during the automated conversion in commit
> 9c399719cfb9 ("drm: convert many bridge drivers from devm_kzalloc() to
> devm_drm_bridge_alloc() API") and following commits.
>
> The lack of conversion for this driver is a bug since commit a7748dd127ea
> ("drm/bridge: get/put the bridge reference in drm_bridge_add/remove()")
> which is the first commmit having added a drm_bridge_get/put() pair and
> thus exposing the incorrect initial refcount issue.
>
> Fix this by switching the driver to the new API.
>
> Reported-by: Marek Szyprowski <[email protected]>
> Closes:
> https://lore.kernel.org/all/[email protected]/
> Fixes: a7748dd127ea ("drm/bridge: get/put the bridge reference in
> drm_bridge_add/remove()")
> Signed-off-by: Luca Ceresoli <[email protected]>
> ---
> drivers/gpu/drm/sti/sti_hda.c | 27 +++++++++++++--------------
> 1 file changed, 13 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/sti/sti_hda.c b/drivers/gpu/drm/sti/sti_hda.c
> index
> d202b6c1eb8f6032fef547c9f00ca9cd2a914520..2c015f563de96ae58959801493ead870c49f70e5
> 100644
> --- a/drivers/gpu/drm/sti/sti_hda.c
> +++ b/drivers/gpu/drm/sti/sti_hda.c
> @@ -246,6 +246,7 @@ struct sti_hda {
> struct device dev;
> struct drm_device *drm_dev;
> struct drm_display_mode mode;
> + struct drm_bridge bridge;
> void __iomem *regs;
> void __iomem *video_dacs_ctrl;
> struct clk *clk_pix;
> @@ -262,6 +263,11 @@ struct sti_hda_connector {
> #define to_sti_hda_connector(x) \
> container_of(x, struct sti_hda_connector, drm_connector)
>
> +static struct sti_hda *drm_bridge_to_sti_hda(struct drm_bridge *bridge)
> +{
> + return container_of(bridge, struct sti_hda, bridge);
> +}
> +
> static u32 hda_read(struct sti_hda *hda, int offset)
> {
> return readl(hda->regs + offset);
> @@ -401,7 +407,7 @@ static void sti_hda_configure_awg(struct sti_hda *hda,
> u32 *awg_instr, int nb)
>
> static void sti_hda_disable(struct drm_bridge *bridge)
> {
> - struct sti_hda *hda = bridge->driver_private;
> + struct sti_hda *hda = drm_bridge_to_sti_hda(bridge);
> u32 val;
>
> if (!hda->enabled)
> @@ -426,7 +432,7 @@ static void sti_hda_disable(struct drm_bridge *bridge)
>
> static void sti_hda_pre_enable(struct drm_bridge *bridge)
> {
> - struct sti_hda *hda = bridge->driver_private;
> + struct sti_hda *hda = drm_bridge_to_sti_hda(bridge);
> u32 val, i, mode_idx;
> u32 src_filter_y, src_filter_c;
> u32 *coef_y, *coef_c;
> @@ -517,7 +523,7 @@ static void sti_hda_set_mode(struct drm_bridge *bridge,
> const struct drm_display_mode *mode,
> const struct drm_display_mode *adjusted_mode)
> {
> - struct sti_hda *hda = bridge->driver_private;
> + struct sti_hda *hda = drm_bridge_to_sti_hda(bridge);
> u32 mode_idx;
> int hddac_rate;
> int ret;
> @@ -677,7 +683,6 @@ static int sti_hda_bind(struct device *dev, struct device
> *master, void *data)
> struct drm_encoder *encoder;
> struct sti_hda_connector *connector;
> struct drm_connector *drm_connector;
> - struct drm_bridge *bridge;
> int err;
>
> /* Set the drm device handle */
> @@ -693,13 +698,7 @@ static int sti_hda_bind(struct device *dev, struct
> device *master, void *data)
>
> connector->hda = hda;
>
> - bridge = devm_kzalloc(dev, sizeof(*bridge), GFP_KERNEL);
> - if (!bridge)
> - return -ENOMEM;
> -
> - bridge->driver_private = hda;
> - bridge->funcs = &sti_hda_bridge_funcs;
> - drm_bridge_attach(encoder, bridge, NULL, 0);
> + drm_bridge_attach(encoder, &hda->bridge, NULL, 0);It's not entirely related, but the connector is also allocated right before and could be moved into the structure instead of storing a pointer. Either way, Reviewed-by: Maxime Ripard <[email protected]> Maxime
signature.asc
Description: PGP signature
