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 <[email protected]>
---
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")
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.
---
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,
--
Luca Weiss <[email protected]>
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 <[email protected]>
> ---
> 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")
>
> 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 ?
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,
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 <[email protected]>
> > ---
> > 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")
> >
> > 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.
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,
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 <[email protected]>
> > > ---
> > > 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")
> > >
> > > 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,
On Thu, 29 Feb 2024 at 11:27, Luca Weiss <[email protected]> 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 <[email protected]>
> > > > ---
> > > > 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?
> > > >
> > > > 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,
>
--
With best wishes
Dmitry
On Sun Mar 3, 2024 at 9:37 PM CET, Dmitry Baryshkov wrote:
> On Thu, 29 Feb 2024 at 11:27, Luca Weiss <[email protected]> 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 <[email protected]>
> > > > > ---
> > > > > 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.
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,
> >
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 <[email protected]> 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 <[email protected]>
>>>>>> ---
>>>>>> 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 <[email protected]>
>
> 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,
>>>
>
Hi,
On Thu, 11 Jan 2024 13:38:04 +0100, 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:
>
> [...]
Thanks, Applied to https://gitlab.freedesktop.org/drm/misc/kernel.git (drm-misc-fixes)
[1/1] drm/bridge: Select DRM_KMS_HELPER for DRM_PANEL_BRIDGE
https://gitlab.freedesktop.org/drm/misc/kernel/-/commit/e3f18b0dd1db242791afbc3bd173026163ce0ccc
--
Neil
On Mon, 18 Mar 2024, Neil Armstrong <[email protected]> wrote:
> Hi,
>
> On Thu, 11 Jan 2024 13:38:04 +0100, 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:
>>
>> [...]
>
> Thanks, Applied to https://gitlab.freedesktop.org/drm/misc/kernel.git (drm-misc-fixes)
>
> [1/1] drm/bridge: Select DRM_KMS_HELPER for DRM_PANEL_BRIDGE
> https://gitlab.freedesktop.org/drm/misc/kernel/-/commit/e3f18b0dd1db242791afbc3bd173026163ce0ccc
With my kernel config, e3f18b0dd1db ("drm/bridge: Select DRM_KMS_HELPER
for DRM_PANEL_BRIDGE") leads to:
WARNING: unmet direct dependencies detected for DRM_KMS_HELPER
Depends on [m]: HAS_IOMEM [=y] && DRM [=m]
Selected by [y]:
- DRM_PANEL_BRIDGE [=y] && HAS_IOMEM [=y] && DRM_BRIDGE [=y]
Selected by [m]:
- DRM [=m] && HAS_IOMEM [=y] && (AGP [=y] || AGP [=y]=n) && !EMULATED_CMPXCHG && HAS_DMA [=y] && DRM_FBDEV_EMULATION [=y]
- DRM_MIPI_DBI [=m] && HAS_IOMEM [=y] && DRM [=m]
- DRM_KUNIT_TEST [=m] && HAS_IOMEM [=y] && DRM [=m] && KUNIT [=y] && MMU [=y]
- DRM_RADEON [=m] && HAS_IOMEM [=y] && DRM [=m] && PCI [=y] && MMU [=y] && (AGP [=y] || !AGP [=y])
- DRM_AMDGPU [=m] && HAS_IOMEM [=y] && DRM [=m] && PCI [=y] && MMU [=y] && !UML
- DRM_NOUVEAU [=m] && HAS_IOMEM [=y] && DRM [=m] && PCI [=y] && MMU [=y]
- DRM_I915 [=m] && HAS_IOMEM [=y] && DRM [=m] && X86 [=y] && PCI [=y] && !PREEMPT_RT [=n]
- DRM_XE [=m] && HAS_IOMEM [=y] && DRM [=m] && PCI [=y] && MMU [=y] && (m && MODULES [=y] || y && KUNIT [=y]=y) && 64BIT [=y]
- DRM_VKMS [=m] && HAS_IOMEM [=y] && DRM [=m] && MMU [=y]
- DRM_VMWGFX [=m] && HAS_IOMEM [=y] && DRM [=m] && PCI [=y] && MMU [=y] && (X86 [=y] || ARM64)
- DRM_GMA500 [=m] && HAS_IOMEM [=y] && DRM [=m] && PCI [=y] && X86 [=y] && MMU [=y]
- DRM_UDL [=m] && HAS_IOMEM [=y] && DRM [=m] && USB [=m] && USB_ARCH_HAS_HCD [=y] && MMU [=y]
- DRM_AST [=m] && HAS_IOMEM [=y] && DRM [=m] && PCI [=y] && MMU [=y]
- DRM_MGAG200 [=m] && HAS_IOMEM [=y] && DRM [=m] && PCI [=y] && MMU [=y]
- DRM_QXL [=m] && HAS_IOMEM [=y] && DRM [=m] && PCI [=y] && MMU [=y]
- DRM_VIRTIO_GPU [=m] && HAS_IOMEM [=y] && DRM [=m] && VIRTIO_MENU [=y] && MMU [=y]
- DRM_BOCHS [=m] && HAS_IOMEM [=y] && DRM [=m] && PCI [=y] && MMU [=y]
- DRM_CIRRUS_QEMU [=m] && HAS_IOMEM [=y] && DRM [=m] && PCI [=y] && MMU [=y]
- DRM_GM12U320 [=m] && HAS_IOMEM [=y] && DRM [=m] && USB [=m] && MMU [=y]
- DRM_PANEL_MIPI_DBI [=m] && HAS_IOMEM [=y] && DRM [=m] && SPI [=y]
- DRM_SIMPLEDRM [=m] && HAS_IOMEM [=y] && DRM [=m] && MMU [=y]
- TINYDRM_HX8357D [=m] && HAS_IOMEM [=y] && DRM [=m] && SPI [=y]
- TINYDRM_ILI9163 [=m] && HAS_IOMEM [=y] && DRM [=m] && SPI [=y]
- TINYDRM_ILI9225 [=m] && HAS_IOMEM [=y] && DRM [=m] && SPI [=y]
- TINYDRM_ILI9341 [=m] && HAS_IOMEM [=y] && DRM [=m] && SPI [=y]
- TINYDRM_ILI9486 [=m] && HAS_IOMEM [=y] && DRM [=m] && SPI [=y]
- TINYDRM_MI0283QT [=m] && HAS_IOMEM [=y] && DRM [=m] && SPI [=y]
- TINYDRM_REPAPER [=m] && HAS_IOMEM [=y] && DRM [=m] && SPI [=y]
- TINYDRM_ST7586 [=m] && HAS_IOMEM [=y] && DRM [=m] && SPI [=y]
- TINYDRM_ST7735R [=m] && HAS_IOMEM [=y] && DRM [=m] && SPI [=y]
- DRM_XEN_FRONTEND [=m] && HAS_IOMEM [=y] && XEN [=y] && DRM [=m]
- DRM_VBOXVIDEO [=m] && HAS_IOMEM [=y] && DRM [=m] && X86 [=y] && PCI [=y]
- DRM_GUD [=m] && HAS_IOMEM [=y] && DRM [=m] && USB [=m] && MMU [=y]
- DRM_SSD130X [=m] && HAS_IOMEM [=y] && DRM [=m] && MMU [=y]
- DRM_ANALOGIX_ANX78XX [=m] && HAS_IOMEM [=y] && DRM [=m] && DRM_BRIDGE [=y]
BR,
Jani.
--
Jani Nikula, Intel
On Mon, 18 Mar 2024, Jani Nikula <[email protected]> wrote:
> On Mon, 18 Mar 2024, Neil Armstrong <[email protected]> wrote:
>> Hi,
>>
>> On Thu, 11 Jan 2024 13:38:04 +0100, 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:
>>>
>>> [...]
>>
>> Thanks, Applied to https://gitlab.freedesktop.org/drm/misc/kernel.git (drm-misc-fixes)
>>
>> [1/1] drm/bridge: Select DRM_KMS_HELPER for DRM_PANEL_BRIDGE
>> https://gitlab.freedesktop.org/drm/misc/kernel/-/commit/e3f18b0dd1db242791afbc3bd173026163ce0ccc
>
> With my kernel config, e3f18b0dd1db ("drm/bridge: Select DRM_KMS_HELPER
> for DRM_PANEL_BRIDGE") leads to:
>
> WARNING: unmet direct dependencies detected for DRM_KMS_HELPER
> Depends on [m]: HAS_IOMEM [=y] && DRM [=m]
> Selected by [y]:
> - DRM_PANEL_BRIDGE [=y] && HAS_IOMEM [=y] && DRM_BRIDGE [=y]
> Selected by [m]:
> - DRM [=m] && HAS_IOMEM [=y] && (AGP [=y] || AGP [=y]=n) && !EMULATED_CMPXCHG && HAS_DMA [=y] && DRM_FBDEV_EMULATION [=y]
> - DRM_MIPI_DBI [=m] && HAS_IOMEM [=y] && DRM [=m]
> - DRM_KUNIT_TEST [=m] && HAS_IOMEM [=y] && DRM [=m] && KUNIT [=y] && MMU [=y]
> - DRM_RADEON [=m] && HAS_IOMEM [=y] && DRM [=m] && PCI [=y] && MMU [=y] && (AGP [=y] || !AGP [=y])
> - DRM_AMDGPU [=m] && HAS_IOMEM [=y] && DRM [=m] && PCI [=y] && MMU [=y] && !UML
> - DRM_NOUVEAU [=m] && HAS_IOMEM [=y] && DRM [=m] && PCI [=y] && MMU [=y]
> - DRM_I915 [=m] && HAS_IOMEM [=y] && DRM [=m] && X86 [=y] && PCI [=y] && !PREEMPT_RT [=n]
> - DRM_XE [=m] && HAS_IOMEM [=y] && DRM [=m] && PCI [=y] && MMU [=y] && (m && MODULES [=y] || y && KUNIT [=y]=y) && 64BIT [=y]
> - DRM_VKMS [=m] && HAS_IOMEM [=y] && DRM [=m] && MMU [=y]
> - DRM_VMWGFX [=m] && HAS_IOMEM [=y] && DRM [=m] && PCI [=y] && MMU [=y] && (X86 [=y] || ARM64)
> - DRM_GMA500 [=m] && HAS_IOMEM [=y] && DRM [=m] && PCI [=y] && X86 [=y] && MMU [=y]
> - DRM_UDL [=m] && HAS_IOMEM [=y] && DRM [=m] && USB [=m] && USB_ARCH_HAS_HCD [=y] && MMU [=y]
> - DRM_AST [=m] && HAS_IOMEM [=y] && DRM [=m] && PCI [=y] && MMU [=y]
> - DRM_MGAG200 [=m] && HAS_IOMEM [=y] && DRM [=m] && PCI [=y] && MMU [=y]
> - DRM_QXL [=m] && HAS_IOMEM [=y] && DRM [=m] && PCI [=y] && MMU [=y]
> - DRM_VIRTIO_GPU [=m] && HAS_IOMEM [=y] && DRM [=m] && VIRTIO_MENU [=y] && MMU [=y]
> - DRM_BOCHS [=m] && HAS_IOMEM [=y] && DRM [=m] && PCI [=y] && MMU [=y]
> - DRM_CIRRUS_QEMU [=m] && HAS_IOMEM [=y] && DRM [=m] && PCI [=y] && MMU [=y]
> - DRM_GM12U320 [=m] && HAS_IOMEM [=y] && DRM [=m] && USB [=m] && MMU [=y]
> - DRM_PANEL_MIPI_DBI [=m] && HAS_IOMEM [=y] && DRM [=m] && SPI [=y]
> - DRM_SIMPLEDRM [=m] && HAS_IOMEM [=y] && DRM [=m] && MMU [=y]
> - TINYDRM_HX8357D [=m] && HAS_IOMEM [=y] && DRM [=m] && SPI [=y]
> - TINYDRM_ILI9163 [=m] && HAS_IOMEM [=y] && DRM [=m] && SPI [=y]
> - TINYDRM_ILI9225 [=m] && HAS_IOMEM [=y] && DRM [=m] && SPI [=y]
> - TINYDRM_ILI9341 [=m] && HAS_IOMEM [=y] && DRM [=m] && SPI [=y]
> - TINYDRM_ILI9486 [=m] && HAS_IOMEM [=y] && DRM [=m] && SPI [=y]
> - TINYDRM_MI0283QT [=m] && HAS_IOMEM [=y] && DRM [=m] && SPI [=y]
> - TINYDRM_REPAPER [=m] && HAS_IOMEM [=y] && DRM [=m] && SPI [=y]
> - TINYDRM_ST7586 [=m] && HAS_IOMEM [=y] && DRM [=m] && SPI [=y]
> - TINYDRM_ST7735R [=m] && HAS_IOMEM [=y] && DRM [=m] && SPI [=y]
> - DRM_XEN_FRONTEND [=m] && HAS_IOMEM [=y] && XEN [=y] && DRM [=m]
> - DRM_VBOXVIDEO [=m] && HAS_IOMEM [=y] && DRM [=m] && X86 [=y] && PCI [=y]
> - DRM_GUD [=m] && HAS_IOMEM [=y] && DRM [=m] && USB [=m] && MMU [=y]
> - DRM_SSD130X [=m] && HAS_IOMEM [=y] && DRM [=m] && MMU [=y]
> - DRM_ANALOGIX_ANX78XX [=m] && HAS_IOMEM [=y] && DRM [=m] && DRM_BRIDGE [=y]
Please read Documentation/kbuild/kconfig-language.rst.
Basically boolean DRM_PANEL_BRIDGE selecting tristate DRM_KMS_HELPER
forces it to y while it should remain m.
Please revert.
BR,
Jani.
--
Jani Nikula, Intel
On Mon, Mar 18, 2024 at 12:59:29PM +0200, Jani Nikula wrote:
> On Mon, 18 Mar 2024, Jani Nikula <[email protected]> wrote:
> > On Mon, 18 Mar 2024, Neil Armstrong <[email protected]> wrote:
> >> Hi,
> >>
> >> On Thu, 11 Jan 2024 13:38:04 +0100, 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:
> >>>
> >>> [...]
> >>
> >> Thanks, Applied to https://gitlab.freedesktop.org/drm/misc/kernel.git (drm-misc-fixes)
> >>
> >> [1/1] drm/bridge: Select DRM_KMS_HELPER for DRM_PANEL_BRIDGE
> >> https://gitlab.freedesktop.org/drm/misc/kernel/-/commit/e3f18b0dd1db242791afbc3bd173026163ce0ccc
> >
> > With my kernel config, e3f18b0dd1db ("drm/bridge: Select DRM_KMS_HELPER
> > for DRM_PANEL_BRIDGE") leads to:
> >
> > WARNING: unmet direct dependencies detected for DRM_KMS_HELPER
> > Depends on [m]: HAS_IOMEM [=y] && DRM [=m]
> > Selected by [y]:
> > - DRM_PANEL_BRIDGE [=y] && HAS_IOMEM [=y] && DRM_BRIDGE [=y]
> > Selected by [m]:
> > - DRM [=m] && HAS_IOMEM [=y] && (AGP [=y] || AGP [=y]=n) && !EMULATED_CMPXCHG && HAS_DMA [=y] && DRM_FBDEV_EMULATION [=y]
> > - DRM_MIPI_DBI [=m] && HAS_IOMEM [=y] && DRM [=m]
> > - DRM_KUNIT_TEST [=m] && HAS_IOMEM [=y] && DRM [=m] && KUNIT [=y] && MMU [=y]
> > - DRM_RADEON [=m] && HAS_IOMEM [=y] && DRM [=m] && PCI [=y] && MMU [=y] && (AGP [=y] || !AGP [=y])
> > - DRM_AMDGPU [=m] && HAS_IOMEM [=y] && DRM [=m] && PCI [=y] && MMU [=y] && !UML
> > - DRM_NOUVEAU [=m] && HAS_IOMEM [=y] && DRM [=m] && PCI [=y] && MMU [=y]
> > - DRM_I915 [=m] && HAS_IOMEM [=y] && DRM [=m] && X86 [=y] && PCI [=y] && !PREEMPT_RT [=n]
> > - DRM_XE [=m] && HAS_IOMEM [=y] && DRM [=m] && PCI [=y] && MMU [=y] && (m && MODULES [=y] || y && KUNIT [=y]=y) && 64BIT [=y]
> > - DRM_VKMS [=m] && HAS_IOMEM [=y] && DRM [=m] && MMU [=y]
> > - DRM_VMWGFX [=m] && HAS_IOMEM [=y] && DRM [=m] && PCI [=y] && MMU [=y] && (X86 [=y] || ARM64)
> > - DRM_GMA500 [=m] && HAS_IOMEM [=y] && DRM [=m] && PCI [=y] && X86 [=y] && MMU [=y]
> > - DRM_UDL [=m] && HAS_IOMEM [=y] && DRM [=m] && USB [=m] && USB_ARCH_HAS_HCD [=y] && MMU [=y]
> > - DRM_AST [=m] && HAS_IOMEM [=y] && DRM [=m] && PCI [=y] && MMU [=y]
> > - DRM_MGAG200 [=m] && HAS_IOMEM [=y] && DRM [=m] && PCI [=y] && MMU [=y]
> > - DRM_QXL [=m] && HAS_IOMEM [=y] && DRM [=m] && PCI [=y] && MMU [=y]
> > - DRM_VIRTIO_GPU [=m] && HAS_IOMEM [=y] && DRM [=m] && VIRTIO_MENU [=y] && MMU [=y]
> > - DRM_BOCHS [=m] && HAS_IOMEM [=y] && DRM [=m] && PCI [=y] && MMU [=y]
> > - DRM_CIRRUS_QEMU [=m] && HAS_IOMEM [=y] && DRM [=m] && PCI [=y] && MMU [=y]
> > - DRM_GM12U320 [=m] && HAS_IOMEM [=y] && DRM [=m] && USB [=m] && MMU [=y]
> > - DRM_PANEL_MIPI_DBI [=m] && HAS_IOMEM [=y] && DRM [=m] && SPI [=y]
> > - DRM_SIMPLEDRM [=m] && HAS_IOMEM [=y] && DRM [=m] && MMU [=y]
> > - TINYDRM_HX8357D [=m] && HAS_IOMEM [=y] && DRM [=m] && SPI [=y]
> > - TINYDRM_ILI9163 [=m] && HAS_IOMEM [=y] && DRM [=m] && SPI [=y]
> > - TINYDRM_ILI9225 [=m] && HAS_IOMEM [=y] && DRM [=m] && SPI [=y]
> > - TINYDRM_ILI9341 [=m] && HAS_IOMEM [=y] && DRM [=m] && SPI [=y]
> > - TINYDRM_ILI9486 [=m] && HAS_IOMEM [=y] && DRM [=m] && SPI [=y]
> > - TINYDRM_MI0283QT [=m] && HAS_IOMEM [=y] && DRM [=m] && SPI [=y]
> > - TINYDRM_REPAPER [=m] && HAS_IOMEM [=y] && DRM [=m] && SPI [=y]
> > - TINYDRM_ST7586 [=m] && HAS_IOMEM [=y] && DRM [=m] && SPI [=y]
> > - TINYDRM_ST7735R [=m] && HAS_IOMEM [=y] && DRM [=m] && SPI [=y]
> > - DRM_XEN_FRONTEND [=m] && HAS_IOMEM [=y] && XEN [=y] && DRM [=m]
> > - DRM_VBOXVIDEO [=m] && HAS_IOMEM [=y] && DRM [=m] && X86 [=y] && PCI [=y]
> > - DRM_GUD [=m] && HAS_IOMEM [=y] && DRM [=m] && USB [=m] && MMU [=y]
> > - DRM_SSD130X [=m] && HAS_IOMEM [=y] && DRM [=m] && MMU [=y]
> > - DRM_ANALOGIX_ANX78XX [=m] && HAS_IOMEM [=y] && DRM [=m] && DRM_BRIDGE [=y]
>
> Please read Documentation/kbuild/kconfig-language.rst.
>
> Basically boolean DRM_PANEL_BRIDGE selecting tristate DRM_KMS_HELPER
> forces it to y while it should remain m.
>
> Please revert.
I can also see the above issue with the latest drm-tip, in particular a
CONFIG_DRM=m build will fail with the above kconfig warns and then
multiple linker errors:
ld: drivers/gpu/drm/drm_atomic_helper.o: in function `drm_atomic_helper_check_wb_connector_state':
/home/imre/intel-gfx/drm-misc-fixes/drivers/gpu/drm/drm_atomic_helper.c:832: undefined reference to `__drm_dev_dbg'
ld: drivers/gpu/drm/drm_atomic_helper.o: in function `drm_atomic_helper_async_check':
/home/imre/intel-gfx/drm-misc-fixes/drivers/gpu/drm/drm_atomic_helper.c:1932: undefined reference to `__drm_dev_dbg'
ld: /home/imre/intel-gfx/drm-misc-fixes/drivers/gpu/drm/drm_atomic_helper.c:1924: undefined reference to `__drm_dev_dbg'
ld: /home/imre/intel-gfx/drm-misc-fixes/drivers/gpu/drm/drm_atomic_helper.c:1896: undefined reference to `__drm_dev_dbg'
ld: /home/imre/intel-gfx/drm-misc-fixes/drivers/gpu/drm/drm_atomic_helper.c:1911: undefined reference to `__drm_dev_dbg'
ld: drivers/gpu/drm/drm_atomic_helper.o:/home/imre/intel-gfx/drm-misc-fixes/drivers/gpu/drm/drm_atomic_helper.c:1889: more undefined references to `__drm_dev_dbg' follow
ld: drivers/gpu/drm/drm_atomic_helper.o: in function `drm_atomic_helper_wait_for_dependencies':
/home/imre/intel-gfx/drm-misc-fixes/drivers/gpu/drm/drm_atomic_helper.c:2410: undefined reference to `drm_crtc_commit_wait'
ld: /home/imre/intel-gfx/drm-misc-fixes/drivers/gpu/drm/drm_atomic_helper.c:2418: undefined reference to `drm_crtc_commit_wait'
ld: /home/imre/intel-gfx/drm-misc-fixes/drivers/gpu/drm/drm_atomic_helper.c:2426: undefined reference to `drm_crtc_commit_wait'
ld: drivers/gpu/drm/drm_atomic_helper.o: in function `drm_atomic_helper_fake_vblank':
/home/imre/intel-gfx/drm-misc-fixes/drivers/gpu/drm/drm_atomic_helper.c:2467: undefined reference to `drm_crtc_send_vblank_event'
ld: drivers/gpu/drm/drm_atomic_helper.o: in function `drm_atomic_helper_prepare_planes':
/home/imre/intel-gfx/drm-misc-fixes/drivers/gpu/drm/drm_atomic_helper.c:2589: undefined reference to `drm_writeback_prepare_job'
ld: drivers/gpu/drm/drm_atomic_helper.o: in function `drm_atomic_helper_commit_duplicated_state':
..
CONFIG_DRM=y still builds ok.
> BR,
> Jani.
>
>
> --
> Jani Nikula, Intel
On Mon, Mar 18, 2024 at 12:52:10PM +0200, Jani Nikula wrote:
> On Mon, 18 Mar 2024, Neil Armstrong <[email protected]> wrote:
> > Hi,
> >
> > On Thu, 11 Jan 2024 13:38:04 +0100, 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:
> >>
> >> [...]
> >
> > Thanks, Applied to https://gitlab.freedesktop.org/drm/misc/kernel.git (drm-misc-fixes)
> >
> > [1/1] drm/bridge: Select DRM_KMS_HELPER for DRM_PANEL_BRIDGE
> > https://gitlab.freedesktop.org/drm/misc/kernel/-/commit/e3f18b0dd1db242791afbc3bd173026163ce0ccc
>
> With my kernel config, e3f18b0dd1db ("drm/bridge: Select DRM_KMS_HELPER
> for DRM_PANEL_BRIDGE") leads to:
>
> WARNING: unmet direct dependencies detected for DRM_KMS_HELPER
> Depends on [m]: HAS_IOMEM [=y] && DRM [=m]
..
All the defconfigs in drm-rerere also seem to fail here.
Neil, are you using some weird .config, or did you not actually
build test this before pushing?
PS. the drm-rerere defconfigs seem pretty outdated (eg. missing
tons of panel drivers). Would be good if someone could update
those to provide better coverage
--
Ville Syrj?l?
Intel
On Mon Mar 18, 2024 at 11:59 AM CET, Jani Nikula wrote:
> On Mon, 18 Mar 2024, Jani Nikula <[email protected]> wrote:
> > On Mon, 18 Mar 2024, Neil Armstrong <[email protected]> wrote:
> >> Hi,
> >>
> >> On Thu, 11 Jan 2024 13:38:04 +0100, 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:
> >>>
> >>> [...]
> >>
> >> Thanks, Applied to https://gitlab.freedesktop.org/drm/misc/kernel.git (drm-misc-fixes)
> >>
> >> [1/1] drm/bridge: Select DRM_KMS_HELPER for DRM_PANEL_BRIDGE
> >> https://gitlab.freedesktop.org/drm/misc/kernel/-/commit/e3f18b0dd1db242791afbc3bd173026163ce0ccc
> >
> > With my kernel config, e3f18b0dd1db ("drm/bridge: Select DRM_KMS_HELPER
> > for DRM_PANEL_BRIDGE") leads to:
> >
> > WARNING: unmet direct dependencies detected for DRM_KMS_HELPER
> > Depends on [m]: HAS_IOMEM [=y] && DRM [=m]
> > Selected by [y]:
> > - DRM_PANEL_BRIDGE [=y] && HAS_IOMEM [=y] && DRM_BRIDGE [=y]
> > Selected by [m]:
> > - DRM [=m] && HAS_IOMEM [=y] && (AGP [=y] || AGP [=y]=n) && !EMULATED_CMPXCHG && HAS_DMA [=y] && DRM_FBDEV_EMULATION [=y]
> > - DRM_MIPI_DBI [=m] && HAS_IOMEM [=y] && DRM [=m]
> > - DRM_KUNIT_TEST [=m] && HAS_IOMEM [=y] && DRM [=m] && KUNIT [=y] && MMU [=y]
> > - DRM_RADEON [=m] && HAS_IOMEM [=y] && DRM [=m] && PCI [=y] && MMU [=y] && (AGP [=y] || !AGP [=y])
> > - DRM_AMDGPU [=m] && HAS_IOMEM [=y] && DRM [=m] && PCI [=y] && MMU [=y] && !UML
> > - DRM_NOUVEAU [=m] && HAS_IOMEM [=y] && DRM [=m] && PCI [=y] && MMU [=y]
> > - DRM_I915 [=m] && HAS_IOMEM [=y] && DRM [=m] && X86 [=y] && PCI [=y] && !PREEMPT_RT [=n]
> > - DRM_XE [=m] && HAS_IOMEM [=y] && DRM [=m] && PCI [=y] && MMU [=y] && (m && MODULES [=y] || y && KUNIT [=y]=y) && 64BIT [=y]
> > - DRM_VKMS [=m] && HAS_IOMEM [=y] && DRM [=m] && MMU [=y]
> > - DRM_VMWGFX [=m] && HAS_IOMEM [=y] && DRM [=m] && PCI [=y] && MMU [=y] && (X86 [=y] || ARM64)
> > - DRM_GMA500 [=m] && HAS_IOMEM [=y] && DRM [=m] && PCI [=y] && X86 [=y] && MMU [=y]
> > - DRM_UDL [=m] && HAS_IOMEM [=y] && DRM [=m] && USB [=m] && USB_ARCH_HAS_HCD [=y] && MMU [=y]
> > - DRM_AST [=m] && HAS_IOMEM [=y] && DRM [=m] && PCI [=y] && MMU [=y]
> > - DRM_MGAG200 [=m] && HAS_IOMEM [=y] && DRM [=m] && PCI [=y] && MMU [=y]
> > - DRM_QXL [=m] && HAS_IOMEM [=y] && DRM [=m] && PCI [=y] && MMU [=y]
> > - DRM_VIRTIO_GPU [=m] && HAS_IOMEM [=y] && DRM [=m] && VIRTIO_MENU [=y] && MMU [=y]
> > - DRM_BOCHS [=m] && HAS_IOMEM [=y] && DRM [=m] && PCI [=y] && MMU [=y]
> > - DRM_CIRRUS_QEMU [=m] && HAS_IOMEM [=y] && DRM [=m] && PCI [=y] && MMU [=y]
> > - DRM_GM12U320 [=m] && HAS_IOMEM [=y] && DRM [=m] && USB [=m] && MMU [=y]
> > - DRM_PANEL_MIPI_DBI [=m] && HAS_IOMEM [=y] && DRM [=m] && SPI [=y]
> > - DRM_SIMPLEDRM [=m] && HAS_IOMEM [=y] && DRM [=m] && MMU [=y]
> > - TINYDRM_HX8357D [=m] && HAS_IOMEM [=y] && DRM [=m] && SPI [=y]
> > - TINYDRM_ILI9163 [=m] && HAS_IOMEM [=y] && DRM [=m] && SPI [=y]
> > - TINYDRM_ILI9225 [=m] && HAS_IOMEM [=y] && DRM [=m] && SPI [=y]
> > - TINYDRM_ILI9341 [=m] && HAS_IOMEM [=y] && DRM [=m] && SPI [=y]
> > - TINYDRM_ILI9486 [=m] && HAS_IOMEM [=y] && DRM [=m] && SPI [=y]
> > - TINYDRM_MI0283QT [=m] && HAS_IOMEM [=y] && DRM [=m] && SPI [=y]
> > - TINYDRM_REPAPER [=m] && HAS_IOMEM [=y] && DRM [=m] && SPI [=y]
> > - TINYDRM_ST7586 [=m] && HAS_IOMEM [=y] && DRM [=m] && SPI [=y]
> > - TINYDRM_ST7735R [=m] && HAS_IOMEM [=y] && DRM [=m] && SPI [=y]
> > - DRM_XEN_FRONTEND [=m] && HAS_IOMEM [=y] && XEN [=y] && DRM [=m]
> > - DRM_VBOXVIDEO [=m] && HAS_IOMEM [=y] && DRM [=m] && X86 [=y] && PCI [=y]
> > - DRM_GUD [=m] && HAS_IOMEM [=y] && DRM [=m] && USB [=m] && MMU [=y]
> > - DRM_SSD130X [=m] && HAS_IOMEM [=y] && DRM [=m] && MMU [=y]
> > - DRM_ANALOGIX_ANX78XX [=m] && HAS_IOMEM [=y] && DRM [=m] && DRM_BRIDGE [=y]
>
> Please read Documentation/kbuild/kconfig-language.rst.
>
> Basically boolean DRM_PANEL_BRIDGE selecting tristate DRM_KMS_HELPER
> forces it to y while it should remain m.
Would you know the correct fix for this? I'm aware of the pattern
"select FOO || !FOO" but I guess it's also not applicable here?
In any case building DRM_PANEL_BRIDGE also needs to build
DRM_KMS_HELPER, otherwise the object files just don't get used.
Unfortunately I'm not versed well enough at all in Kconfig :/
>
> Please revert.
>
> BR,
> Jani.
On 18/03/2024 14:41, Ville Syrjälä wrote:
> On Mon, Mar 18, 2024 at 12:52:10PM +0200, Jani Nikula wrote:
>> On Mon, 18 Mar 2024, Neil Armstrong <[email protected]> wrote:
>>> Hi,
>>>
>>> On Thu, 11 Jan 2024 13:38:04 +0100, 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:
>>>>
>>>> [...]
>>>
>>> Thanks, Applied to https://gitlab.freedesktop.org/drm/misc/kernel.git (drm-misc-fixes)
>>>
>>> [1/1] drm/bridge: Select DRM_KMS_HELPER for DRM_PANEL_BRIDGE
>>> https://gitlab.freedesktop.org/drm/misc/kernel/-/commit/e3f18b0dd1db242791afbc3bd173026163ce0ccc
>>
>> With my kernel config, e3f18b0dd1db ("drm/bridge: Select DRM_KMS_HELPER
>> for DRM_PANEL_BRIDGE") leads to:
>>
>> WARNING: unmet direct dependencies detected for DRM_KMS_HELPER
>> Depends on [m]: HAS_IOMEM [=y] && DRM [=m]
> ...
>
> All the defconfigs in drm-rerere also seem to fail here.
>
> Neil, are you using some weird .config, or did you not actually
> build test this before pushing?
It definitely built fine, but my config test is not extensive and went through it,
I'll send a revert patch ASAP.
Neil
>
> PS. the drm-rerere defconfigs seem pretty outdated (eg. missing
> tons of panel drivers). Would be good if someone could update
> those to provide better coverage
>
On Mon, 18 Mar 2024, "Luca Weiss" <[email protected]> wrote:
> Would you know the correct fix for this? I'm aware of the pattern
> "select FOO || !FOO" but I guess it's also not applicable here?
I don't think that pattern works for select, only for depends on.
And I think the problem, again, is the abuse of select for symbols with
dependencies.
$ git grep "select DRM_KMS_HELPER" | wc -l
122
I'm guessing these only work because a) they are tristates, and b) they
directly or indirectly already "depends on DRM", which satisfies
DRM_KMS_HELPER's "depends on DRM".
I think the correct fix for this, and a plethora of other kconfig
problems, is adhering to the note in
Documentation/kbuild/kconfig-language.rst:
Note:
select should be used with care. select will force
a symbol to a value without visiting the dependencies.
By abusing select you are able to select a symbol FOO even
if FOO depends on BAR that is not set.
In general use select only for non-visible symbols
(no prompts anywhere) and for symbols with no dependencies.
That will limit the usefulness but on the other hand avoid
the illegal configurations all over.
The downsides are that it's a lot of churn to fix them, they'll creep
back in, and kconfig doesn't warn about these cases up front while it
could, and menuconfig etc. aren't helpful in enabling dependencies for
you recursively. So here we are, adding bandaid year after year. :(
BR,
Jani.
--
Jani Nikula, Intel