| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <7b3713d1-df18-4da1-a1e2-16dcff08fe66@collabora.com>
Date: Tue, 22 Apr 2025 15:53:43 +0200
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
To: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.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,
dmitry.baryshkov@...aro.org, lewis.liao@...iatek.com,
ives.chenjh@...iatek.com, tommyyl.chen@...iatek.com,
jason-jh.lin@...iatek.com
Subject: Re: [PATCH v9 22/23] drm/mediatek: Introduce HDMI/DDC v2 for
MT8195/MT8188
Il 21/04/25 21:16, Dmitry Baryshkov ha scritto:
> On Tue, Apr 15, 2025 at 12:43:20PM +0200, 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.
>>
>> Reviewed-by: CK Hu <ck.hu@...iatek.com>
>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
>> ---
>> drivers/gpu/drm/mediatek/Kconfig | 7 +
>> drivers/gpu/drm/mediatek/Makefile | 2 +
>> drivers/gpu/drm/mediatek/mtk_hdmi_common.c | 4 +
>> drivers/gpu/drm/mediatek/mtk_hdmi_common.h | 9 +
>> drivers/gpu/drm/mediatek/mtk_hdmi_ddc_v2.c | 385 +++++
>> drivers/gpu/drm/mediatek/mtk_hdmi_regs_v2.h | 263 ++++
>> drivers/gpu/drm/mediatek/mtk_hdmi_v2.c | 1396 +++++++++++++++++++
>> 7 files changed, 2066 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_setup_audio_infoframe(struct mtk_hdmi *hdmi)
>> +{
>> + struct hdmi_codec_params *params = &hdmi->aud_param.codec_params;
>> + struct hdmi_audio_infoframe frame;
>> + u8 buffer[14];
>> + ssize_t ret;
>> +
>> + memcpy(&frame, ¶ms->cea, sizeof(frame));
>> +
>> + ret = hdmi_audio_infoframe_pack(&frame, buffer, sizeof(buffer));
>> + if (ret < 0)
>> + return ret;
>
> This should really be done via
> drm_atomic_helper_connector_hdmi_update_audio_infoframe() or
> drm_atomic_helper_connector_hdmi_clear_audio_infoframe().
>
> Ideally this should come from the .hw_params() / .prepare() calls so
> that you don't need to store the params in the driver data.
>
When switching to the new hdmi audio helpers yes, but I was planning to do that
later.....
>> +
>> + mtk_hdmi_v2_hw_write_audio_infoframe(hdmi, buffer);
>> +
>> + return 0;
>> +}
>> +
>> +static inline void mtk_hdmi_v2_hw_gcp_avmute(struct mtk_hdmi *hdmi, bool mute)
>> +{
>> + u32 val;
>> +
>> + regmap_read(hdmi->regs, TOP_CFG01, &val);
>> + val &= ~(CP_CLR_MUTE_EN | CP_SET_MUTE_EN);
>> +
>> + if (mute) {
>> + val |= CP_SET_MUTE_EN;
>> + val &= ~CP_CLR_MUTE_EN;
>> + } else {
>> + val |= CP_CLR_MUTE_EN;
>> + val &= ~CP_SET_MUTE_EN;
>> + }
>> + regmap_write(hdmi->regs, TOP_CFG01, val);
>> +
>> + regmap_set_bits(hdmi->regs, TOP_INFO_RPT, CP_RPT_EN);
>> + regmap_set_bits(hdmi->regs, TOP_INFO_EN, CP_EN | CP_EN_WR);
>> +}
>> +
>> +static void mtk_hdmi_v2_hw_ncts_enable(struct mtk_hdmi *hdmi, bool enable)
>> +{
>> + if (enable)
>> + regmap_set_bits(hdmi->regs, AIP_CTRL, CTS_SW_SEL);
>> + else
>> + regmap_clear_bits(hdmi->regs, AIP_CTRL, CTS_SW_SEL);
>> +}
>> +
>> +static void mtk_hdmi_v2_hw_aud_set_channel_status(struct mtk_hdmi *hdmi)
>> +{
>> + u8 *ch_status = hdmi->aud_param.codec_params.iec.status;
>> +
>> + /* Only the first 5 to 7 bytes of Channel Status contain useful information */
>> + regmap_write(hdmi->regs, AIP_I2S_CHST0, mtk_hdmi_v2_format_hw_packet(&ch_status[0], 4));
>> + regmap_write(hdmi->regs, AIP_I2S_CHST1, mtk_hdmi_v2_format_hw_packet(&ch_status[4], 3));
>> +}
>> +
>> +static void mtk_hdmi_v2_hw_aud_set_ncts(struct mtk_hdmi *hdmi,
>> + unsigned int sample_rate,
>> + unsigned int clock)
>> +{
>> + unsigned int n, cts;
>> +
>> + mtk_hdmi_get_ncts(sample_rate, clock, &n, &cts);
>
> drm_hdmi_acr_get_n_cts() ?
>
I'd have to update both HDMI drivers to use that instead, and I was planning to do
that at a later time when switching to the HDMI audio helpers.
>> +
>> + regmap_write(hdmi->regs, AIP_N_VAL, n);
>> + regmap_write(hdmi->regs, AIP_CTS_SVAL, cts);
>> +}
>> +
>
> [...]
>
>> +
>> +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);
>
> Most likely you need to stop sending the AUDIO packet too. Or is it dome
> by the hardware?
>
The call to `mtk_hdmi_v2_hw_aud_enable(hdmi, false)` will set HW registers to both
mute and stop sending the audio packet.
>> +}
>> +
>> +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,
>> +};
>
> Do you plan to switch to the OP_HDMI_AUDIO? I'd really like to see
> bridges use the framework instead of implementing everthing on their
> own.
>
I do, but since I've already reached v9, I really don't want to do that right now
and delay this driver for another two months.
I plan to do the switch after we at least get this in: as the V1 driver would also
need the same cleanup, I may even find a way to throw more stuff in the hdmi_common
when cleaning up both at the same time.
Cheers,
Angelo
Powered by blists - more mailing lists