From: Johan Jonker <jbx6244@gmail.com>
To: "Heiko Stübner" <heiko@sntech.de>,
"Sandy Huang" <hjc@rock-chips.com>,
"Andy Yan" <andy.yan@rock-chips.com>,
"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
"Maxime Ripard" <mripard@kernel.org>,
"Thomas Zimmermann" <tzimmermann@suse.de>,
"David Airlie" <airlied@gmail.com>,
"Simona Vetter" <simona@ffwll.ch>
Cc: dri-devel@lists.freedesktop.org,
linux-rockchip@lists.infradead.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, "Hüseyin BIYIK" <boogiepop@gmx.com>
Subject: Re: [PATCH v2] drm/rockchip: rk3066_hdmi: convert to devm_drm_bridge_alloc() API
Date: Thu, 3 Sep 2026 22:44:57 +0200 [thread overview]
Message-ID: <afde7cfa-bd1e-40d2-9315-d3d88abb7606@gmail.com> (raw)
In-Reply-To: <2814844.6tgchFWduM@diego>
Hi,
On 9/3/26 17:12, Heiko Stübner wrote:
> Am Montag, 24. August 2026, 12:21:19 Mitteleuropäische Sommerzeit schrieb Johan Jonker via B4 Relay:
>> From: Hüseyin BIYIK <boogiepop@gmx.com>
>>
>> The function devm_drm_bridge_alloc() is the new API for allocating DRM bridges.
>> This conversion was missed during the initial conversion of all bridges to
>> the new API.
>>
>> Signed-off-by: Hüseyin BIYIK <boogiepop@gmx.com>
>> Signed-off-by: Johan Jonker <jbx6244@gmail.com>
>> ---
>> drivers/gpu/drm/rockchip/rk3066_hdmi.c | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/rockchip/rk3066_hdmi.c b/drivers/gpu/drm/rockchip/rk3066_hdmi.c
>> index f28d6a883e09..39c65ee07caa 100644
>> --- a/drivers/gpu/drm/rockchip/rk3066_hdmi.c
>> +++ b/drivers/gpu/drm/rockchip/rk3066_hdmi.c
>> @@ -703,7 +703,6 @@ rk3066_hdmi_register(struct drm_device *drm, struct rk3066_hdmi *hdmi)
>> DRM_MODE_ENCODER_TMDS, NULL);
>>
>> hdmi->bridge.driver_private = hdmi;
Can this be removed?
See link/comment below.
>> - hdmi->bridge.funcs = &rk3066_hdmi_bridge_funcs;
>> hdmi->bridge.ops = DRM_BRIDGE_OP_DETECT |
>> DRM_BRIDGE_OP_EDID |
>> DRM_BRIDGE_OP_HDMI |
>> @@ -747,9 +746,10 @@ static int rk3066_hdmi_bind(struct device *dev, struct device *master,
>> int irq;
>> int ret;
>>
>> - hdmi = devm_kzalloc(dev, sizeof(*hdmi), GFP_KERNEL);
>> - if (!hdmi)
>> - return -ENOMEM;
>> + hdmi = devm_drm_bridge_alloc(dev, struct rk3066_hdmi, bridge,
>> + &rk3066_hdmi_bridge_funcs);
>> + if (IS_ERR(hdmi))
>> + return PTR_ERR(hdmi);
>
> If I'm reading things correctly, the parts in rk3066_hdmi_register setting
> hdmi->bridge.driver_private (should use the container field of struct
> drm_bridge instead)
The driver_private line was added with this patch:
[PATCH] drm/rockchip: rk3066_hdmi: switch to drm bridge
https://lore.kernel.org/all/20250428102309.1501986-1-andyshrk@163.com/
and hdmi->bridge.funcs are redundant by this change?
Not redundant, just funcs are attached in a common function __devm_drm_bridge_alloc()
https://elixir.bootlin.com/linux/v7.2.2/source/drivers/gpu/drm/drm_bridge.c#L383
void *__devm_drm_bridge_alloc(struct device *dev, size_t size, size_t offset,
const struct drm_bridge_funcs *funcs)
{
void *container;
struct drm_bridge *bridge;
int err;
if (!funcs) {
dev_warn(dev, "Missing funcs pointer\n");
return ERR_PTR(-EINVAL);
}
container = kzalloc(size, GFP_KERNEL);
if (!container)
return ERR_PTR(-ENOMEM);
bridge = container + offset;
INIT_LIST_HEAD(&bridge->list);
bridge->container = container;
bridge->funcs = funcs;
kref_init(&bridge->refcount);
err = devm_add_action_or_reset(dev, drm_bridge_put_void, bridge);
if (err)
return ERR_PTR(err);
return container;
}
EXPORT_SYMBOL(__devm_drm_bridge_alloc);
===========
Other example from sti:
https://lore.kernel.org/all/ce9c6aa3-5372-468f-a4bf-5a261259e459@samsung.com/
>>>> 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.
drm/sti: hdmi: convert to devm_drm_bridge_alloc() API
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ac4531424d907f3983e919a7bda2b90ea0cede4f
============
This serie was missing a few bridges too many.
drm: convert all bridges to devm_drm_bridge_alloc()
https://patchwork.freedesktop.org/series/148229/
====
Please advise what changes are needed?
RK3066_hdmi is broken since somewhere 2025-05.
Johan
>
>
> Heiko
>
>
>
>>
>> hdmi->dev = dev;
>> hdmi->drm_dev = drm;
>>
>> ---
>> base-commit: 2709dd5ae32f0828f386327c76bba9f39f63a1c6
>> change-id: 20260824-drm-e4689f65ab8c
>>
>> Best regards,
>>
>
>
>
>
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
WARNING: multiple messages have this Message-ID (diff)
From: Johan Jonker <jbx6244@gmail.com>
To: "Heiko Stübner" <heiko@sntech.de>,
"Sandy Huang" <hjc@rock-chips.com>,
"Andy Yan" <andy.yan@rock-chips.com>,
"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
"Maxime Ripard" <mripard@kernel.org>,
"Thomas Zimmermann" <tzimmermann@suse.de>,
"David Airlie" <airlied@gmail.com>,
"Simona Vetter" <simona@ffwll.ch>
Cc: dri-devel@lists.freedesktop.org,
linux-rockchip@lists.infradead.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, "Hüseyin BIYIK" <boogiepop@gmx.com>
Subject: Re: [PATCH v2] drm/rockchip: rk3066_hdmi: convert to devm_drm_bridge_alloc() API
Date: Thu, 3 Sep 2026 22:44:57 +0200 [thread overview]
Message-ID: <afde7cfa-bd1e-40d2-9315-d3d88abb7606@gmail.com> (raw)
In-Reply-To: <2814844.6tgchFWduM@diego>
Hi,
On 9/3/26 17:12, Heiko Stübner wrote:
> Am Montag, 24. August 2026, 12:21:19 Mitteleuropäische Sommerzeit schrieb Johan Jonker via B4 Relay:
>> From: Hüseyin BIYIK <boogiepop@gmx.com>
>>
>> The function devm_drm_bridge_alloc() is the new API for allocating DRM bridges.
>> This conversion was missed during the initial conversion of all bridges to
>> the new API.
>>
>> Signed-off-by: Hüseyin BIYIK <boogiepop@gmx.com>
>> Signed-off-by: Johan Jonker <jbx6244@gmail.com>
>> ---
>> drivers/gpu/drm/rockchip/rk3066_hdmi.c | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/rockchip/rk3066_hdmi.c b/drivers/gpu/drm/rockchip/rk3066_hdmi.c
>> index f28d6a883e09..39c65ee07caa 100644
>> --- a/drivers/gpu/drm/rockchip/rk3066_hdmi.c
>> +++ b/drivers/gpu/drm/rockchip/rk3066_hdmi.c
>> @@ -703,7 +703,6 @@ rk3066_hdmi_register(struct drm_device *drm, struct rk3066_hdmi *hdmi)
>> DRM_MODE_ENCODER_TMDS, NULL);
>>
>> hdmi->bridge.driver_private = hdmi;
Can this be removed?
See link/comment below.
>> - hdmi->bridge.funcs = &rk3066_hdmi_bridge_funcs;
>> hdmi->bridge.ops = DRM_BRIDGE_OP_DETECT |
>> DRM_BRIDGE_OP_EDID |
>> DRM_BRIDGE_OP_HDMI |
>> @@ -747,9 +746,10 @@ static int rk3066_hdmi_bind(struct device *dev, struct device *master,
>> int irq;
>> int ret;
>>
>> - hdmi = devm_kzalloc(dev, sizeof(*hdmi), GFP_KERNEL);
>> - if (!hdmi)
>> - return -ENOMEM;
>> + hdmi = devm_drm_bridge_alloc(dev, struct rk3066_hdmi, bridge,
>> + &rk3066_hdmi_bridge_funcs);
>> + if (IS_ERR(hdmi))
>> + return PTR_ERR(hdmi);
>
> If I'm reading things correctly, the parts in rk3066_hdmi_register setting
> hdmi->bridge.driver_private (should use the container field of struct
> drm_bridge instead)
The driver_private line was added with this patch:
[PATCH] drm/rockchip: rk3066_hdmi: switch to drm bridge
https://lore.kernel.org/all/20250428102309.1501986-1-andyshrk@163.com/
and hdmi->bridge.funcs are redundant by this change?
Not redundant, just funcs are attached in a common function __devm_drm_bridge_alloc()
https://elixir.bootlin.com/linux/v7.2.2/source/drivers/gpu/drm/drm_bridge.c#L383
void *__devm_drm_bridge_alloc(struct device *dev, size_t size, size_t offset,
const struct drm_bridge_funcs *funcs)
{
void *container;
struct drm_bridge *bridge;
int err;
if (!funcs) {
dev_warn(dev, "Missing funcs pointer\n");
return ERR_PTR(-EINVAL);
}
container = kzalloc(size, GFP_KERNEL);
if (!container)
return ERR_PTR(-ENOMEM);
bridge = container + offset;
INIT_LIST_HEAD(&bridge->list);
bridge->container = container;
bridge->funcs = funcs;
kref_init(&bridge->refcount);
err = devm_add_action_or_reset(dev, drm_bridge_put_void, bridge);
if (err)
return ERR_PTR(err);
return container;
}
EXPORT_SYMBOL(__devm_drm_bridge_alloc);
===========
Other example from sti:
https://lore.kernel.org/all/ce9c6aa3-5372-468f-a4bf-5a261259e459@samsung.com/
>>>> 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.
drm/sti: hdmi: convert to devm_drm_bridge_alloc() API
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ac4531424d907f3983e919a7bda2b90ea0cede4f
============
This serie was missing a few bridges too many.
drm: convert all bridges to devm_drm_bridge_alloc()
https://patchwork.freedesktop.org/series/148229/
====
Please advise what changes are needed?
RK3066_hdmi is broken since somewhere 2025-05.
Johan
>
>
> Heiko
>
>
>
>>
>> hdmi->dev = dev;
>> hdmi->drm_dev = drm;
>>
>> ---
>> base-commit: 2709dd5ae32f0828f386327c76bba9f39f63a1c6
>> change-id: 20260824-drm-e4689f65ab8c
>>
>> Best regards,
>>
>
>
>
>
next prev parent reply other threads:[~2026-09-03 20:45 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-24 10:21 [PATCH v2] drm/rockchip: rk3066_hdmi: convert to devm_drm_bridge_alloc() API Johan Jonker via B4 Relay
2026-08-24 10:21 ` Johan Jonker
2026-08-24 10:21 ` Johan Jonker via B4 Relay
2026-08-24 10:30 ` sashiko-bot
2026-09-03 15:12 ` Heiko Stübner
2026-09-03 15:12 ` Heiko Stübner
2026-09-03 20:44 ` Johan Jonker [this message]
2026-09-03 20:44 ` Johan Jonker
2026-09-03 21:09 ` Heiko Stübner
2026-09-03 21:09 ` Heiko Stübner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=afde7cfa-bd1e-40d2-9315-d3d88abb7606@gmail.com \
--to=jbx6244@gmail.com \
--cc=airlied@gmail.com \
--cc=andy.yan@rock-chips.com \
--cc=boogiepop@gmx.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=heiko@sntech.de \
--cc=hjc@rock-chips.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=maarten.lankhorst@linux.intel.com \
--cc=mripard@kernel.org \
--cc=simona@ffwll.ch \
--cc=tzimmermann@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.