linux-kernel - Re: [PATCH v3 12/12] drm/mediatek: dpi: Add mt8195 hdmi to DPI driver

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <988b0a7a-69bb-34e4-e777-1d9516221077@collabora.com>
Date:   Mon, 7 Nov 2022 12:20:16 +0100
From:   AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...labora.com>
To:     Guillaume Ranquet <granquet@...libre.com>,
        Rob Herring <robh+dt@...nel.org>,
        Chun-Kuang Hu <chunkuang.hu@...nel.org>,
        Chunfeng Yun <chunfeng.yun@...iatek.com>,
        Jitao shi <jitao.shi@...iatek.com>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Vinod Koul <vkoul@...nel.org>, CK Hu <ck.hu@...iatek.com>,
        Daniel Vetter <daniel@...ll.ch>,
        David Airlie <airlied@...il.com>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Kishon Vijay Abraham I <kishon@...com>
Cc:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        linux-arm-kernel@...ts.infradead.org, stuart.lee@...iatek.com,
        linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        linux-mediatek@...ts.infradead.org, devicetree@...r.kernel.org,
        mac.shen@...iatek.com, linux-phy@...ts.infradead.org
Subject: Re: [PATCH v3 12/12] drm/mediatek: dpi: Add mt8195 hdmi to DPI driver

Il 04/11/22 15:09, Guillaume Ranquet ha scritto:
> Add the DPI1 hdmi path support in mtk dpi driver
> 
> Signed-off-by: Guillaume Ranquet <granquet@...libre.com>
> ---
>   drivers/gpu/drm/mediatek/mtk_dpi.c      | 143 ++++++++++++++++++++++++++++++--
>   drivers/gpu/drm/mediatek/mtk_dpi_regs.h |   5 ++
>   2 files changed, 141 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c
> index 508a6d994e83..8052b47042b8 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dpi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
> @@ -14,7 +14,10 @@
>   #include <linux/of_graph.h>
>   #include <linux/pinctrl/consumer.h>
>   #include <linux/platform_device.h>
> +#include <linux/reset.h>
>   #include <linux/types.h>
> +#include <linux/regmap.h>
> +#include <linux/mfd/syscon.h>
>   
>   #include <video/videomode.h>
>   
> @@ -65,10 +68,14 @@ struct mtk_dpi {
>   	struct drm_bridge *next_bridge;
>   	struct drm_connector *connector;
>   	void __iomem *regs;
> +	struct reset_control *reset_ctl;
>   	struct device *dev;
>   	struct clk *engine_clk;
> +	struct clk *dpi_ck_cg;
>   	struct clk *pixel_clk;
> +	struct clk *dpi_sel_clk;
>   	struct clk *tvd_clk;
> +	struct clk *hdmi_cg;

I admit that I didn't really check these clocks, but judging by the names,
it is highly possible that one (or more) of them are supposed to be parents
of some others.

The first suspicious ones are dpi_ck_cg and dpi_sel_clk: please check.

I'm also not sure about the hdmi_cg, shouldn't the DPI have a HDMI port in
the graph that you'd declare in devicetree?

Besides... you're doing a lot of work to check if (is_internal_hdmi) for
power up/down paths, but seeing that you're introducing this change after
adding the HDMI driver makes me mostly sure that the internal hdmi that we're
talking about here is the one that is managed by the HDMIV2 driver... and
this means that you should really, really, really rely on connecting inputs
and outputs the right way in the devicetree, as that will most probably make
you able to write practically 0 code to manage power for the DPI... and may
also remove the need of adding the hdmi_cg clock here...

Regards,
Angelo