Re: [PATCH v2 12/15] drm/mediatek: mtk_hdmi: Split driver and add common probe function

[AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>]

Il 05/12/24 13:29, Dmitry Baryshkov ha scritto:
> On Thu, Dec 05, 2024 at 12:45:14PM +0100, AngeloGioacchino Del Regno wrote:
>> In preparation for adding a new driver for the HDMI TX v2 IP,
>> split out the functions that will be common between the already
>> present mtk_hdmi (v1) driver and the new one.
>>
>> Since the probe flow for both drivers is 90% similar, add a common
>> probe function that will be called from each driver's .probe()
>> callback, avoiding lots of code duplication.
>>
>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>> ---
>>   drivers/gpu/drm/mediatek/Kconfig           |  11 +-
>>   drivers/gpu/drm/mediatek/Makefile          |   1 +
>>   drivers/gpu/drm/mediatek/mtk_hdmi.c        | 724 +++------------------
>>   drivers/gpu/drm/mediatek/mtk_hdmi_common.c | 425 ++++++++++++
>>   drivers/gpu/drm/mediatek/mtk_hdmi_common.h | 203 ++++++
>>   5 files changed, 729 insertions(+), 635 deletions(-)
>>   create mode 100644 drivers/gpu/drm/mediatek/mtk_hdmi_common.c
>>   create mode 100644 drivers/gpu/drm/mediatek/mtk_hdmi_common.h
>>
>> @@ -1026,19 +812,12 @@ static int mtk_hdmi_setup_vendor_specific_infoframe(struct mtk_hdmi *hdmi,
>>   	return 0;
>>   }
>>   
>> -static int mtk_hdmi_output_init(struct mtk_hdmi *hdmi)
>> +static void mtk_hdmi_send_infoframe(struct mtk_hdmi *hdmi, u8 *buffer_spd, size_t bufsz_spd,
>> +				    u8 *buffer_avi, size_t bufsz_avi,
>> +				    struct drm_display_mode *mode)
>>   {
>> -	struct hdmi_audio_param *aud_param = &hdmi->aud_param;
>> -
>> -	hdmi->csp = HDMI_COLORSPACE_RGB;
>> -	aud_param->aud_codec = HDMI_AUDIO_CODING_TYPE_PCM;
>> -	aud_param->aud_sample_size = HDMI_AUDIO_SAMPLE_SIZE_16;
>> -	aud_param->aud_input_type = HDMI_AUD_INPUT_I2S;
>> -	aud_param->aud_i2s_fmt = HDMI_I2S_MODE_I2S_24BIT;
>> -	aud_param->aud_mclk = HDMI_AUD_MCLK_128FS;
>> -	aud_param->aud_input_chan_type = HDMI_AUD_CHAN_TYPE_2_0;
>> -
>> -	return 0;
>> +	mtk_hdmi_setup_avi_infoframe(hdmi, buffer_avi, bufsz_avi, mode);
>> +	mtk_hdmi_setup_spd_infoframe(hdmi, buffer_spd, bufsz_spd, "mediatek", "On-chip HDMI");
> 
> Please use the HDMI Connector framework instead of handling everything
> on your own.
> 

Dmitry, my plan is to get the MediaTek HDMIv2 driver upstreamed *with* using
the HDMI helpers - this commit is only splitting the old HDMI v1 driver in
common parts and v1 handling.

This "handling everything on my own" is something that was already present
into the old v1 driver, I am *not adding* this code, but *moving it around*.

I cannot migrate the v1 driver to the HDMI helpers right now because in this
moment I have no way to test that and I don't want to break any functionality.

If you check patch 15, you'll see that I am indeed using all of the helper
goodies - but for now only on the new driver, and not on the old one.

I do plan to migrate the v1 driver to the new helpers as well, but I would
(please) really prefer to do that only after the v2 driver is upstreamed.

Is that okay for you?

Cheers,
Angelo

>>   }
>>   
>>   static void mtk_hdmi_audio_enable(struct mtk_hdmi *hdmi)
>