[Letux-kernel] [PATCH v4 02/10] drm/ingenic: Add support for JZ4780 and HDMI output

H. Nikolaus Schaller hns at goldelico.com
Wed Sep 29 16:42:10 CEST 2021


Hi Paul,

> Am 29.09.2021 um 16:30 schrieb Paul Cercueil <paul at crapouillou.net>:
> 
> Hi,
> 
> Le mar., sept. 28 2021 at 14:06:03 +0200, H. Nikolaus Schaller <hns at goldelico.com> a écrit :
>> Hi Paul,
>>> Am 28.09.2021 um 12:21 schrieb H. Nikolaus Schaller <hns at goldelico.com>:
>>>>> @@ -1492,10 +1555,16 @@ static int ingenic_drm_init(void)
>>>>> {
>>>>> 	int err;
>>>>> +	if (IS_ENABLED(CONFIG_DRM_INGENIC_DW_HDMI)) {
>>>>> +		err = platform_driver_register(ingenic_dw_hdmi_driver_ptr);
>>>>> +		if (err)
>>>>> +			return err;
>>>>> +	}
>>>> I don't see why you need to register the ingenic-dw-hdmi driver here. Just register it in the ingenic-dw-hdmi driver.
>>> Ok, I never though about this (as the code was not from me). We apparently just followed the IPU code pattern (learning by example).
>>> It indeed looks not necessary and would also avoid the ingenic_dw_hdmi_driver_ptr dependency.
>>> But: what is ingenic_ipu_driver_ptr then good for?
> 
> It's done this way because ingenic-drm-drv.c and ingenic-ipu.c are both compiled within the same module ingenic-drm.

Ah, I see. Hadn't checked this.

> I'm not sure this is still required, maybe ingenic-ipu.c can be its own module now.

What I have seen is that it has its own compatible record. So there could be load-on-demand by DTS.

> 
>>> If we can get rid of this as well, we can drop patch 1/10 ("drm/ingenic: Fix drm_init error path if IPU was registered") completely.
>> A quick test shows that it *is* required. At least if I configure everything as modules.
>> But like you I can't explain why.
> 
> Well, a quick test here shows that it is not required, at least when configuring with everything built-in.

IMHO the hdmi driver (module) should be loaded on demand. Not everyone wants to have it.

Well, that is the problem that needs to be solved...

BR and thanks,
Nikolaus



More information about the Letux-kernel mailing list