dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
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,
>>
> 
> 
> 
> 


  reply	other threads:[~2026-09-03 20:45 UTC|newest]

Thread overview: 5+ 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:30 ` sashiko-bot
2026-09-03 15:12 ` Heiko Stübner
2026-09-03 20:44   ` Johan Jonker [this message]
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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox