On 08/03/2024 10:29, Luca Weiss wrote:
On Sun Mar 3, 2024 at 9:37 PM CET, Dmitry Baryshkov wrote:
On Thu, 29 Feb 2024 at 11:27, Luca Weiss <luca.weiss@xxxxxxxxxxxxx> wrote:
On Wed Jan 17, 2024 at 9:59 AM CET, Luca Weiss wrote:
On Mon Jan 15, 2024 at 9:43 AM CET, Neil Armstrong wrote:
Hi Luca,
On 11/01/2024 13:38, Luca Weiss wrote:
Since the kconfig symbol of DRM_PANEL_BRIDGE is only adding
bridge/panel.o to drm_kms_helper object, we need to select
DRM_KMS_HELPER to make sure the file is actually getting built.
Otherwise with certain defconfigs e.g. devm_drm_of_get_bridge will not
be properly available:
aarch64-linux-gnu-ld: drivers/phy/qualcomm/phy-qcom-qmp-combo.o: in function `qmp_combo_bridge_attach':
drivers/phy/qualcomm/phy-qcom-qmp-combo.c:3204:(.text+0x8f4): undefined reference to `devm_drm_of_get_bridge'
Signed-off-by: Luca Weiss <luca.weiss@xxxxxxxxxxxxx>
---
I can see "depends on DRM_KMS_HELPER" was removed with commit
3c3384050d68 ("drm: Don't make DRM_PANEL_BRIDGE dependent on DRM_KMS_HELPERS")
Could you please make sure that the usecase described in the mentioned
commit message doesn't get broken by your change?
Hi Neil,
The problem fixed in that linked patch (3c3384050d68) is about fixing
undefined reference errors with specific .config setups - similar to
this patch.
Since we're only adding a 'select' and not removing anything I don't see
how it could cause new errors like that, and it does fix the one I'm
describing.
And also I checked again and I don't see any circular dependencies
(something that was also mentioned in the linked patch), so apart from
what I mentioned with that I'm not too familiar when 'select' should be
used and when 'depend' should be used, it's good from my perspective.
Sure, LGTM:
Reviewed-by: Neil Armstrong <neil.armstrong@xxxxxxxxxx>
Regards
Luca
I'm not too familiar with Kconfig but it feels more correct if
PHY_QCOM_QMP_COMBO selects DRM_PANEL_BRIDGE that that's enough; and it
doesn't also has to explicitly select DRM_KMS_HELPER because of how the
objects are built in the Makefile.
Alternatively solution to this patch could be adjusting this line in
include/drm/drm_bridge.h:
-#if defined(CONFIG_OF) && defined(CONFIG_DRM_PANEL_BRIDGE)
+#if defined(CONFIG_OF) && defined(CONFIG_DRM_PANEL_BRIDGE) && defined(CONFIG_DRM_KMS_HELPER)
struct drm_bridge *devm_drm_of_get_bridge(struct device *dev, struct device_node *node,
u32 port, u32 endpoint);
.. and then selecting DRM_KMS_HELPER for PHY_QCOM_QMP_COMBO.
But I think the solution in this patch is better. Let me know what you
think.
I think this is no more the case after on linux-next:
35921910bbd0 phy: qcom: qmp-combo: switch to DRM_AUX_BRIDGE
But could you still check ?
On next-20240117 the error happens in the aux-bridge file instead then.
aarch64-linux-gnu-ld: drivers/gpu/drm/bridge/aux-bridge.o: in function `drm_aux_bridge_probe':
drivers/gpu/drm/bridge/aux-bridge.c:115:(.text+0xe0): undefined reference to `devm_drm_of_get_bridge'
I'm attaching the defconfig with which I can reproduce this but it's
really just DRM_KMS_HELPER=n and PHY_QCOM_QMP_COMBO=y I believe.
Hi Neil,
Ping on this patch
Regards
Luca
Regards
Luca
Neil
---
drivers/gpu/drm/bridge/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index ac9ec5073619..ae782b427829 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -8,6 +8,7 @@ config DRM_BRIDGE
config DRM_PANEL_BRIDGE
def_bool y
depends on DRM_BRIDGE
+ select DRM_KMS_HELPER
select DRM_PANEL
help
DRM bridge wrapper of DRM panels
---
base-commit: b9c3a1fa6fb324e691a03cf124b79f4842e65d76
change-id: 20240111-drm-panel-bridge-fixup-5c2977fb969f
Best regards,
