Message-ID: <5qtfugb6jvmf7e2aixwxf2wdeldrfmybpg5xloeqewloj3z7mb@rf4pbiomsic6>
Date: Wed, 8 Jan 2025 22:58:24 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
Cc: chunkuang.hu@...nel.org, p.zabel@...gutronix.de, airlied@...il.com, 
	simona@...ll.ch, maarten.lankhorst@...ux.intel.com, mripard@...nel.org, 
	tzimmermann@...e.de, robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org, 
	matthias.bgg@...il.com, ck.hu@...iatek.com, jitao.shi@...iatek.com, jie.qiu@...iatek.com, 
	junzhi.zhao@...iatek.com, dri-devel@...ts.freedesktop.org, 
	linux-mediatek@...ts.infradead.org, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-arm-kernel@...ts.infradead.org, kernel@...labora.com, lewis.liao@...iatek.com, 
	ives.chenjh@...iatek.com, tommyyl.chen@...iatek.com, jason-jh.lin@...iatek.com
Subject: Re: [PATCH v4 32/34] drm/mediatek: Introduce HDMI/DDC v2 for
 MT8195/MT8188

On Wed, Jan 08, 2025 at 12:27:42PM +0100, AngeloGioacchino Del Regno wrote:
> Add support for the newer HDMI-TX (Encoder) v2 and DDC v2 IPs
> found in MediaTek's MT8195, MT8188 SoC and their variants, and
> including support for display modes up to 4k60 and for HDMI
> Audio, as per the HDMI 2.0 spec.
> 
> HDCP and CEC functionalities are also supported by this hardware,
> but are not included in this commit and that also poses a slight
> difference between the V2 and V1 controllers in how they handle
> Hotplug Detection (HPD).
> 
> While the v1 controller was using the CEC controller to check
> HDMI cable connection and disconnection, in this driver the v2
> one does not.
> 
> This is due to the fact that on parts with v2 designs, like the
> MT8195 SoC, there is one CEC controller shared between the HDMI
> Transmitter (HDMI-TX) and Receiver (HDMI-RX): before eventually
> adding support to use the CEC HW to wake up the HDMI controllers
> it is necessary to have support for one TX, one RX *and* for both
> at the same time.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
> ---
>  drivers/gpu/drm/mediatek/Kconfig            |    8 +
>  drivers/gpu/drm/mediatek/Makefile           |    2 +
>  drivers/gpu/drm/mediatek/mtk_hdmi_common.c  |   10 +
>  drivers/gpu/drm/mediatek/mtk_hdmi_common.h  |    9 +
>  drivers/gpu/drm/mediatek/mtk_hdmi_ddc_v2.c  |  403 ++++++
>  drivers/gpu/drm/mediatek/mtk_hdmi_regs_v2.h |  263 ++++
>  drivers/gpu/drm/mediatek/mtk_hdmi_v2.c      | 1379 +++++++++++++++++++
>  7 files changed, 2074 insertions(+)
>  create mode 100644 drivers/gpu/drm/mediatek/mtk_hdmi_ddc_v2.c
>  create mode 100644 drivers/gpu/drm/mediatek/mtk_hdmi_regs_v2.h
>  create mode 100644 drivers/gpu/drm/mediatek/mtk_hdmi_v2.c
> 

[...]

> +
> +static int mtk_hdmi_v2_bridge_atomic_check(struct drm_bridge *bridge,
> +					   struct drm_bridge_state *bridge_state,
> +					   struct drm_crtc_state *crtc_state,
> +					   struct drm_connector_state *conn_state)
> +{
> +	return drm_atomic_helper_connector_hdmi_check(conn_state->connector,
> +						      conn_state->state);
> +}

This is now a part of the drm_bridge_connector and can be dropped from
the bridge driver.

> +
> +static const struct drm_bridge_funcs mtk_v2_hdmi_bridge_funcs = {
> +	.attach = mtk_hdmi_v2_bridge_attach,
> +	.detach = mtk_hdmi_v2_bridge_detach,
> +	.mode_fixup = mtk_hdmi_bridge_mode_fixup,
> +	.mode_set = mtk_hdmi_bridge_mode_set,
> +	.atomic_pre_enable = mtk_hdmi_v2_bridge_pre_enable,
> +	.atomic_enable = mtk_hdmi_v2_bridge_enable,
> +	.atomic_disable = mtk_hdmi_v2_bridge_disable,
> +	.atomic_post_disable = mtk_hdmi_v2_bridge_post_disable,
> +	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
> +	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
> +	.atomic_check = mtk_hdmi_v2_bridge_atomic_check,
> +	.atomic_reset = drm_atomic_helper_bridge_reset,
> +	.detect = mtk_hdmi_v2_bridge_detect,
> +	.edid_read = mtk_hdmi_v2_bridge_edid_read,
> +	.hpd_enable = mtk_hdmi_v2_hpd_enable,
> +	.hpd_disable = mtk_hdmi_v2_hpd_disable,
> +	.hdmi_tmds_char_rate_valid = mtk_hdmi_v2_hdmi_tmds_char_rate_valid,
> +	.hdmi_clear_infoframe = mtk_hdmi_v2_hdmi_clear_infoframe,
> +	.hdmi_write_infoframe = mtk_hdmi_v2_hdmi_write_infoframe,

Note: the HDMI Codec framework has been merged. switching to it can come
as a followup patchset.

> +};
> +
> +/*
> + * HDMI audio codec callbacks
> + */
> +static int mtk_hdmi_v2_audio_hook_plugged_cb(struct device *dev, void *data,
> +					     hdmi_codec_plugged_cb fn,
> +					     struct device *codec_dev)
> +{
> +	struct mtk_hdmi *hdmi = dev_get_drvdata(dev);
> +	bool plugged;
> +
> +	if (!hdmi)
> +		return -ENODEV;
> +
> +	mtk_hdmi_audio_set_plugged_cb(hdmi, fn, codec_dev);
> +	plugged = (hdmi->hpd == HDMI_PLUG_IN_AND_SINK_POWER_ON);
> +	mtk_hdmi_v2_handle_plugged_change(hdmi, plugged);
> +
> +	return 0;
> +}
> +
> +static int mtk_hdmi_v2_audio_hw_params(struct device *dev, void *data,
> +				       struct hdmi_codec_daifmt *daifmt,
> +				       struct hdmi_codec_params *params)
> +{
> +	struct mtk_hdmi *hdmi = dev_get_drvdata(dev);
> +
> +	if (hdmi->audio_enable) {
> +		mtk_hdmi_audio_params(hdmi, daifmt, params);
> +		mtk_hdmi_v2_aud_output_config(hdmi, &hdmi->mode);
> +	}
> +	return 0;
> +}
> +
> +static int mtk_hdmi_v2_audio_startup(struct device *dev, void *data)
> +{
> +	struct mtk_hdmi *hdmi = dev_get_drvdata(dev);
> +
> +	mtk_hdmi_v2_hw_aud_enable(hdmi, true);
> +	hdmi->audio_enable = true;
> +
> +	return 0;
> +}
> +
> +static void mtk_hdmi_v2_audio_shutdown(struct device *dev, void *data)
> +{
> +	struct mtk_hdmi *hdmi = dev_get_drvdata(dev);
> +
> +	hdmi->audio_enable = false;
> +	mtk_hdmi_v2_hw_aud_enable(hdmi, false);
> +}
> +
> +static int mtk_hdmi_v2_audio_mute(struct device *dev, void *data, bool enable, int dir)
> +{
> +	struct mtk_hdmi *hdmi = dev_get_drvdata(dev);
> +
> +	mtk_hdmi_v2_hw_aud_mute(hdmi, enable);
> +
> +	return 0;
> +}
> +
> +static const struct hdmi_codec_ops mtk_hdmi_v2_audio_codec_ops = {
> +	.hw_params = mtk_hdmi_v2_audio_hw_params,
> +	.audio_startup = mtk_hdmi_v2_audio_startup,
> +	.audio_shutdown = mtk_hdmi_v2_audio_shutdown,
> +	.mute_stream = mtk_hdmi_v2_audio_mute,
> +	.get_eld = mtk_hdmi_audio_get_eld,
> +	.hook_plugged_cb = mtk_hdmi_v2_audio_hook_plugged_cb,
> +};
> +
> +static __maybe_unused int mtk_hdmi_v2_suspend(struct device *dev)
> +{
> +	struct mtk_hdmi *hdmi = dev_get_drvdata(dev);
> +
> +	mtk_hdmi_v2_disable(hdmi);
> +
> +	return 0;
> +}
> +
> +static __maybe_unused int mtk_hdmi_v2_resume(struct device *dev)
> +{
> +	struct mtk_hdmi *hdmi = dev_get_drvdata(dev);
> +	int ret;
> +
> +	pm_runtime_get_sync(dev);
> +
> +	ret = mtk_hdmi_v2_clk_enable(hdmi);
> +	if (ret)
> +		return ret;
> +
> +	mtk_hdmi_v2_enable_hpd_pord_irq(hdmi, true);
> +
> +	return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(mtk_hdmi_v2_pm_ops, mtk_hdmi_v2_suspend, mtk_hdmi_v2_resume);
> +
> +static const struct mtk_hdmi_ver_conf mtk_hdmi_conf_v2 = {
> +	.bridge_funcs = &mtk_v2_hdmi_bridge_funcs,
> +	.codec_ops = &mtk_hdmi_v2_audio_codec_ops,
> +	.mtk_hdmi_clock_names = mtk_hdmi_v2_clk_names,
> +	.num_clocks = MTK_HDMI_V2_CLK_COUNT
> +};
> +
> +static const struct mtk_hdmi_conf mtk_hdmi_conf_mt8188 = {
> +	.ver_conf = &mtk_hdmi_conf_v2,
> +	.reg_hdmi_tx_cfg = HDMITX_CONFIG_MT8188
> +};
> +
> +static const struct mtk_hdmi_conf mtk_hdmi_conf_mt8195 = {
> +	.ver_conf = &mtk_hdmi_conf_v2,
> +	.reg_hdmi_tx_cfg = HDMITX_CONFIG_MT8195
> +};
> +
> +static int mtk_hdmi_v2_probe(struct platform_device *pdev)
> +{
> +	struct mtk_hdmi *hdmi;
> +	int ret;
> +
> +	hdmi = mtk_hdmi_common_probe(pdev);
> +	if (IS_ERR(hdmi))
> +		return PTR_ERR(hdmi);
> +
> +	hdmi->hpd = HDMI_PLUG_OUT;
> +
> +	/*
> +	 * Disable all HW interrupts at probe stage and install the ISR
> +	 * but keep it disabled, as the rest of the interrupts setup is
> +	 * done in the .bridge_attach() callback, which will enable both
> +	 * the right HW IRQs and the ISR.
> +	 */
> +	mtk_hdmi_v2_hwirq_disable(hdmi);
> +	irq_set_status_flags(hdmi->irq, IRQ_NOAUTOEN);
> +	ret = devm_request_threaded_irq(&pdev->dev, hdmi->irq, mtk_hdmi_v2_isr,
> +					mtk_hdmi_v2_isr_thread,
> +					IRQ_TYPE_LEVEL_HIGH,
> +					dev_name(&pdev->dev), hdmi);
> +	if (ret)
> +		return dev_err_probe(&pdev->dev, ret, "Cannot request IRQ\n");
> +
> +	ret = devm_pm_runtime_enable(&pdev->dev);
> +	if (ret)
> +		return dev_err_probe(&pdev->dev, ret, "Cannot enable Runtime PM\n");
> +
> +	return 0;
> +}
> +
> +static void mtk_hdmi_v2_remove(struct platform_device *pdev)
> +{
> +	struct mtk_hdmi *hdmi = platform_get_drvdata(pdev);
> +
> +	pm_runtime_disable(&pdev->dev);

You have devm_pm_runtime_enable(), so this call must go away.

> +	i2c_put_adapter(hdmi->ddc_adpt);
> +}
> +
> +static const struct of_device_id mtk_drm_hdmi_v2_of_ids[] = {
> +	{ .compatible = "mediatek,mt8188-hdmi-tx", .data = &mtk_hdmi_conf_mt8188 },
> +	{ .compatible = "mediatek,mt8195-hdmi-tx", .data = &mtk_hdmi_conf_mt8195 },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, mtk_drm_hdmi_v2_of_ids);
> +
> +static struct platform_driver mtk_hdmi_v2_driver = {
> +	.probe = mtk_hdmi_v2_probe,
> +	.remove = mtk_hdmi_v2_remove,
> +	.driver = {
> +		.name = "mediatek-drm-hdmi-v2",
> +		.of_match_table = mtk_drm_hdmi_v2_of_ids,
> +		.pm = &mtk_hdmi_v2_pm_ops,
> +	},
> +};
> +module_platform_driver(mtk_hdmi_v2_driver);
> +
> +MODULE_AUTHOR("AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>>");
> +MODULE_DESCRIPTION("MediaTek HDMIv2 Driver");
> +MODULE_LICENSE("GPL");
> +MODULE_IMPORT_NS("DRM_MTK_HDMI");
> -- 
> 2.47.0
> 

-- 
With best wishes
Dmitry

Powered by blists - more mailing lists