| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <f122492f-7f2a-4d3f-af74-3b807c9cd742@rock-chips.com>
Date: Mon, 21 Jul 2025 15:36:40 +0800
From: Damon Ding <damon.ding@...k-chips.com>
To: Heiko Stübner <heiko@...ech.de>,
andrzej.hajda@...el.com, neil.armstrong@...aro.org, rfoss@...nel.org
Cc: Laurent.pinchart@...asonboard.com, jonas@...boo.se,
jernej.skrabec@...il.com, maarten.lankhorst@...ux.intel.com,
mripard@...nel.org, tzimmermann@...e.de, airlied@...il.com, simona@...ll.ch,
jingoohan1@...il.com, inki.dae@...sung.com, sw0312.kim@...sung.com,
kyungmin.park@...sung.com, krzk@...nel.org, alim.akhtar@...sung.com,
hjc@...k-chips.com, andy.yan@...k-chips.com,
dmitry.baryshkov@....qualcomm.com, l.stach@...gutronix.de,
dianders@...omium.org, dri-devel@...ts.freedesktop.org,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-samsung-soc@...r.kernel.org, linux-rockchip@...ts.infradead.org
Subject: Re: [PATCH v2 07/12] drm/bridge: analogix_dp: Add support to find
panel or bridge
On 2025/7/11 15:41, Damon Ding wrote:
> Hi Heiko,
>
> On 2025/7/10 4:07, Heiko Stübner wrote:
>> Hi Damon,
>>
>> Am Mittwoch, 9. Juli 2025, 09:01:34 Mitteleuropäische Sommerzeit
>> schrieb Damon Ding:
>>> Since the panel/bridge should logically be positioned behind the
>>> Analogix bridge in the display pipeline, it makes sense to handle
>>> the panel/bridge parsing on the Analogix side.
>>>
>>> Signed-off-by: Damon Ding <damon.ding@...k-chips.com>
>>> ---
>>> .../drm/bridge/analogix/analogix_dp_core.c | 48 +++++++++++++++++++
>>> include/drm/bridge/analogix_dp.h | 2 +
>>> 2 files changed, 50 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/
>>> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>>> index 78d68310e4f6..660f95e90490 100644
>>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>>> @@ -19,12 +19,14 @@
>>> #include <linux/platform_device.h>
>>> #include <drm/bridge/analogix_dp.h>
>>> +#include <drm/display/drm_dp_aux_bus.h>
>>> #include <drm/drm_atomic.h>
>>> #include <drm/drm_atomic_helper.h>
>>> #include <drm/drm_bridge.h>
>>> #include <drm/drm_crtc.h>
>>> #include <drm/drm_device.h>
>>> #include <drm/drm_edid.h>
>>> +#include <drm/drm_of.h>
>>> #include <drm/drm_panel.h>
>>> #include <drm/drm_print.h>
>>> #include <drm/drm_probe_helper.h>
>>> @@ -1707,6 +1709,52 @@ struct drm_dp_aux *analogix_dp_get_aux(struct
>>> analogix_dp_device *dp)
>>> }
>>> EXPORT_SYMBOL_GPL(analogix_dp_get_aux);
>>> +static int analogix_dp_aux_done_probing(struct drm_dp_aux *aux)
>>> +{
>>> + struct analogix_dp_device *dp = to_dp(aux);
>>> + struct analogix_dp_plat_data *plat_data = dp->plat_data;
>>> + int port = plat_data->dev_type == EXYNOS_DP ? 0 : 1;
>>> + int ret;
>>> +
>>> + /*
>>> + * If drm_of_find_panel_or_bridge() returns -ENODEV, there may
>>> be no valid panel
>>> + * or bridge nodes. The driver should go on for the driver-free
>>> bridge or the DP
>>> + * mode applications.
>>> + */
>>> + ret = drm_of_find_panel_or_bridge(dp->dev->of_node, port, 0,
>>> + &plat_data->panel, &plat_data->bridge);
>>
>> Could you check if this can use a panel-bridge?
>> See for example
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/
>> tree/drivers/gpu/drm/rockchip/rockchip_rgb.c#n138
>> or a lot of other places for drm_panel_bridge_add()
>>
>> So if drm_of_find_panel_or_bridge() finds a bridge, just use it; if it
>> finds a panel, drm_panel_bridge_add() just wraps a bridge around it
>> and all the code can just assume everything is bridge after that.
>>
>>
>
> Yes, that is a good approach. Using the panel-bridge will make the
> related checks more concise.
>
>>
>>> + if (ret && ret != -ENODEV)
>>> + return ret;
>>> +
>>> + return component_add(dp->dev, plat_data->ops);
>>> +}
>>> +
>>> +int analogix_dp_find_panel_or_bridge(struct analogix_dp_device *dp)
>>> +{
>>> + int ret;
>>> +
>>> + ret = devm_of_dp_aux_populate_bus(&dp->aux,
>>> analogix_dp_aux_done_probing);
>>> + if (ret) {
>>> + /*
>>> + * If devm_of_dp_aux_populate_bus() returns -ENODEV, the
>>> done_probing() will
>>> + * not be called because there are no EP devices. Then the
>>> callback function
>>> + * analogix_dp_aux_done_probing() will be called directly in
>>> order to support
>>> + * the other valid DT configurations.
>>> + *
>>> + * NOTE: The devm_of_dp_aux_populate_bus() is allowed to
>>> return -EPROBE_DEFER.
>>> + */
>>> + if (ret != -ENODEV) {
>>> + dev_err(dp->dev, "failed to populate aux bus\n");
>>> + return ret;
>>> + }
>>> +
>>> + return analogix_dp_aux_done_probing(&dp->aux);
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +EXPORT_SYMBOL_GPL(analogix_dp_find_panel_or_bridge);
>>> +
>>> MODULE_AUTHOR("Jingoo Han <jg1.han@...sung.com>");
>>> MODULE_DESCRIPTION("Analogix DP Core Driver");
>>> MODULE_LICENSE("GPL v2");
>>> diff --git a/include/drm/bridge/analogix_dp.h b/include/drm/bridge/
>>> analogix_dp.h
>>> index 202e5eafb2cc..0b6d85f1924e 100644
>>> --- a/include/drm/bridge/analogix_dp.h
>>> +++ b/include/drm/bridge/analogix_dp.h
>>> @@ -30,6 +30,7 @@ struct analogix_dp_plat_data {
>>> struct drm_bridge *bridge;
>>> struct drm_encoder *encoder;
>>> struct drm_connector *connector;
>>> + const struct component_ops *ops;
>>> int (*power_on)(struct analogix_dp_plat_data *);
>>> int (*power_off)(struct analogix_dp_plat_data *);
>>> @@ -52,5 +53,6 @@ int analogix_dp_stop_crc(struct drm_connector
>>> *connector);
>>> struct analogix_dp_plat_data *analogix_dp_aux_to_plat_data(struct
>>> drm_dp_aux *aux);
>>> struct drm_dp_aux *analogix_dp_get_aux(struct analogix_dp_device *dp);
>>> +int analogix_dp_find_panel_or_bridge(struct analogix_dp_device *dp);
>>> #endif /* _ANALOGIX_DP_H_ */
>>>
>>
>
While preparing the v3 patch series, I encountered a question regarding
the mode retrieval process. The relevant code in
drivers/gpu/drm/display/drm_bridge_connector.c is as follows:
static int drm_bridge_connector_get_modes(struct drm_connector *connector)
{
......
/*
* If display exposes EDID, then we parse that in the normal way to
* build table of supported modes.
*/
bridge = bridge_connector->bridge_edid;
if (bridge)
return drm_bridge_connector_get_modes_edid(connector, bridge);
/*
* Otherwise if the display pipeline reports modes (e.g. with a fixed
* resolution panel or an analog TV output), query it.
*/
bridge = bridge_connector->bridge_modes;
if (bridge)
return bridge->funcs->get_modes(bridge, connector);
......
}
The &drm_bridge_funcs.get_modes() will never be called if the
bridge_connector supports DRM_BRIDGE_OP_EDID. This suggests that
DRM_BRIDGE_OP_EDID and DRM_BRIDGE_OP_MODES might be mutually exclusive.
And the following comments also confirm it(include/drm/drm_bridge.h):
......
/**
* @get_modes:
*
* Fill all modes currently valid for the sink into the &drm_connector
* with drm_mode_probed_add().
*
* The @get_modes callback is mostly intended to support non-probeable
* displays such as many fixed panels. Bridges that support reading
* EDID shall leave @get_modes unimplemented and implement the
* &drm_bridge_funcs->edid_read callback instead.
*
......
I want to use drm_panel_get_modes()/drm_bridge_get_modes() as a fallback
when drm_bridge_connector_get_modes_edid() returns no modes, compatible
with the historical behavior. Should I apply the related code with
&drm_bridge_funcs.edid_read() then abandon the
&drm_bridge_funcs.get_modes()? or the opposite?
Or maybe the DRM_BRIDGE_OP_MODES can be a certain fallback of
DRM_BRIDGE_OP_EDID for the bridge_connector?
Best regards,
Damon
Powered by blists - more mailing lists