linux-kernel - Re: [PATCH v6 7/7] drm/mediatek: Add mt8195 DisplayPort 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: <CABnWg9sNOWJ_RgnvSdEtAVQrfELzJr8aj-FTB=oj6hQJScFCVA@mail.gmail.com>
Date:   Wed, 15 Dec 2021 07:56:43 -0600
From:   Guillaume Ranquet <granquet@...libre.com>
To:     AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...labora.com>,
        Chun-Kuang Hu <chunkuang.hu@...nel.org>,
        Daniel Vetter <daniel@...ll.ch>,
        David Airlie <airlied@...ux.ie>,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Maxime Ripard <mripard@...nel.org>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Thomas Zimmermann <tzimmermann@...e.de>
Cc:     Markus Schneider-Pargmann <msp@...libre.com>,
        kernel test robot <lkp@...el.com>,
        linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        linux-mediatek@...ts.infradead.org,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v6 7/7] drm/mediatek: Add mt8195 DisplayPort driver

Hi Angelo,

Quoting AngeloGioacchino Del Regno (2021-12-10 11:17:44)
> Il 10/11/21 14:06, Guillaume Ranquet ha scritto:
> > From: Markus Schneider-Pargmann <msp@...libre.com>
> >
> > This patch adds a DisplayPort driver for the Mediatek mt8195 SoC and a
> > according phy driver mediatek-dp-phy.
> >
> > It supports both functional units on the mt8195, the embedded
> > DisplayPort as well as the external DisplayPort units. It offers
> > hot-plug-detection, audio up to 8 channels, and DisplayPort 1.4 with up
> > to 4 lanes.
> >
> > The driver creates a child device for the phy. The child device will
> > never exist without the parent being active. As they are sharing a
> > register range, the parent passes a regmap pointer to the child so that
> > both can work with the same register range. The phy driver sets device
> > data that is read by the parent to get the phy device that can be used
> > to control the phy properties.
> >
> > This driver is based on an initial version by
> > Jason-JH.Lin <jason-jh.lin@...iatek.com>.
> >
> > Signed-off-by: Markus Schneider-Pargmann <msp@...libre.com>
> > Signed-off-by: Guillaume Ranquet <granquet@...libre.com>
> > Reported-by: kernel test robot <lkp@...el.com>
>
> Hello Markus, Guillaume,
>
> there is a critical issue with this patch. Please check below.
>
> > ---
> >   drivers/gpu/drm/drm_edid.c              |    2 +-
> >   drivers/gpu/drm/mediatek/Kconfig        |    7 +
> >   drivers/gpu/drm/mediatek/Makefile       |    2 +
> >   drivers/gpu/drm/mediatek/mtk_dp.c       | 3094 +++++++++++++++++++++++
> >   drivers/gpu/drm/mediatek/mtk_dp_reg.h   |  568 +++++
> >   drivers/gpu/drm/mediatek/mtk_dpi.c      |  111 +-
> >   drivers/gpu/drm/mediatek/mtk_dpi_regs.h |   26 +
> >   drivers/gpu/drm/mediatek/mtk_drm_drv.c  |    1 +
> >   drivers/gpu/drm/mediatek/mtk_drm_drv.h  |    1 +
> >   9 files changed, 3799 insertions(+), 13 deletions(-)
> >   create mode 100644 drivers/gpu/drm/mediatek/mtk_dp.c
> >   create mode 100644 drivers/gpu/drm/mediatek/mtk_dp_reg.h
> >
>
> <snip>
>
> > diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c
> > index 384074f69111b..e6e88e3cd811d 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_dpi.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
>
> <snip>
>
> > @@ -979,6 +1051,16 @@ static int mtk_dpi_probe(struct platform_device *pdev)
> >               return ret;
> >       }
> >
> > +     dpi->dpi_ck_cg = devm_clk_get(dev, "ck_cg");
>
> mtk_dpi is used on MT2701, MT7183, MT8183, MT8192, but these platforms haven't
> got any "ck_cg" clock defined in their device-trees (regardless of whether these
> can support adding this clock or not, any code change shall be retro-compatible
> hence not breaking compatibility/functionality with older device-trees).
>
> Reminding that:
> - mediatek-drm uses the component framework
> - mtk_drm_drv is the component master
> - mtk_drm_drv bind() won't be called unless all of the components added with
>    match aren't calling component_add()
>
> ... this change not only breaks DisplayPort support for *all* of the
> aforementioned SoCs, but also makes the entire mediatek-drm to not finish
> probing, producing a global breakage that also includes DSI and the entire
> stack of components of that master (so, no display on all of them).
>
> To avoid breaking any SoC that's not MT8195, please use devm_clk_get_optional()
> here in the next version.
>
> Thanks,
> - Angelo
>

This is a good catch, I will update for v7.

Thank you very much for your review.

Thx,
Guillaume.