Re: [PATCH] drm/bridge: Select DRM_KMS_HELPER for DRM_PANEL_BRIDGE

2024-03-18 Thread Jani Nikula
On Mon, 18 Mar 2024, "Luca Weiss"  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


Re: [PATCH] drm/bridge: Select DRM_KMS_HELPER for DRM_PANEL_BRIDGE

2024-03-18 Thread Neil Armstrong

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  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





Re: [PATCH] drm/bridge: Select DRM_KMS_HELPER for DRM_PANEL_BRIDGE

2024-03-18 Thread Luca Weiss
On Mon Mar 18, 2024 at 11:59 AM CET, Jani Nikula wrote:
> On Mon, 18 Mar 2024, Jani Nikula  wrote:
> > On Mon, 18 Mar 2024, Neil Armstrong  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.




Re: [PATCH] drm/bridge: Select DRM_KMS_HELPER for DRM_PANEL_BRIDGE

2024-03-18 Thread Ville Syrjälä
On Mon, Mar 18, 2024 at 12:52:10PM +0200, Jani Nikula wrote:
> On Mon, 18 Mar 2024, Neil Armstrong  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



Re: [PATCH] drm/bridge: Select DRM_KMS_HELPER for DRM_PANEL_BRIDGE

2024-03-18 Thread Imre Deak
On Mon, Mar 18, 2024 at 12:59:29PM +0200, Jani Nikula wrote:
> On Mon, 18 Mar 2024, Jani Nikula  wrote:
> > On Mon, 18 Mar 2024, Neil Armstrong  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 

Re: [PATCH] drm/bridge: Select DRM_KMS_HELPER for DRM_PANEL_BRIDGE

2024-03-18 Thread Jani Nikula
On Mon, 18 Mar 2024, Jani Nikula  wrote:
> On Mon, 18 Mar 2024, Neil Armstrong  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


Re: [PATCH] drm/bridge: Select DRM_KMS_HELPER for DRM_PANEL_BRIDGE

2024-03-18 Thread Jani Nikula
On Mon, 18 Mar 2024, Neil Armstrong  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


Re: [PATCH] drm/bridge: Select DRM_KMS_HELPER for DRM_PANEL_BRIDGE

2024-03-18 Thread Neil Armstrong
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



Re: [PATCH] drm/bridge: Select DRM_KMS_HELPER for DRM_PANEL_BRIDGE

2024-03-18 Thread neil . armstrong

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  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 
---
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 



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,








Re: [PATCH] drm/bridge: Select DRM_KMS_HELPER for DRM_PANEL_BRIDGE

2024-03-08 Thread Luca Weiss
On Sun Mar 3, 2024 at 9:37 PM CET, Dmitry Baryshkov wrote:
> On Thu, 29 Feb 2024 at 11:27, Luca Weiss  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 
> > > > > ---
> > > > > 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,
> >



Re: [PATCH] drm/bridge: Select DRM_KMS_HELPER for DRM_PANEL_BRIDGE

2024-03-03 Thread Dmitry Baryshkov
On Thu, 29 Feb 2024 at 11:27, Luca Weiss  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 
> > > > ---
> > > > 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


Re: [PATCH] drm/bridge: Select DRM_KMS_HELPER for DRM_PANEL_BRIDGE

2024-02-29 Thread Luca Weiss
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 
> > > ---
> > > 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,



Re: [PATCH] drm/bridge: Select DRM_KMS_HELPER for DRM_PANEL_BRIDGE

2024-01-15 Thread Neil Armstrong

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 
---
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,




[PATCH] drm/bridge: Select DRM_KMS_HELPER for DRM_PANEL_BRIDGE

2024-01-11 Thread Luca Weiss
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 
---
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