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

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

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@baylibre.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