Re: [PATCH 3/8] drm/display: Introduce a DRM display-helper module

[Javier Martinez Canillas <javierm@redhat.com>]

On 4/6/22 21:08, Thomas Zimmermann wrote:
> Hi Javier
> 
> Am 30.03.22 um 11:23 schrieb Javier Martinez Canillas:
>> On 3/22/22 20:27, Thomas Zimmermann wrote:
>>> Replace the DP-helper module with a display-helper module. Update
>>> all related Kconfig and Makefile rules.
>>>
>>> Besides the existing code for DisplayPort, the new module will
>>> contain helpers for other video-output standards, such as HDMI.
>>> Drivers will still be able to select the required video-output
>>> helpers. Linking all such code into a single module avoids the
>>> proliferation of small kernel modules.
>>>
>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>> ---
>>
>> [snip]
>>
>>> +config DRM_DISPLAY_HELPER
>>> +	tristate
>>> +	depends on DRM
>>> +	help
>>> +	  DRM helpers for display adapters.
>>> +
>>>   config DRM_DP_HELPER
>>>   	tristate
>>>   	depends on DRM
>>> +	select DRM_DISPLAY_HELPER
>>>   	help
>>>   	  DRM helpers for DisplayPort.
>>>
>>
>> I was about to ask why this would still be needed but then re-read the
>> commit message that says drivers will still be able to select required
>> video-output helpers.
>>
>> That makes sense since the fact that all helpers will be in the same module
>> would be transparent to drivers.
> 
> After some more testing, it turns out to be not so easy. For example, if 
> we have DP_HELPER=m and HDMI_HELPER=y, then DISPLAY_HELPER would be 
> auto-selected as 'y'. The code for DP_HELPER would not be linked correctly.
>
> I'm going to make drivers select DISPLAY_HELPER and the rsp helpers 
> explicitly. The individual helpers would be covered boolean options that 
> enable the feature in the display-helper library.
> 
> If you know some Kconfig magic to enable the original design, let me know.
>

I do not. But I wonder if the problem here is the usage of select rather than
depends and if with the later the original design could still be achieved...

But yes, probably the only way to prevent that issue is to make the drivers
to explicitly select both DRM_DISPLAY_HELPER and respective helpers symbol.
 -- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat