linux-kernel - RE: [EXT] Re: [PATCH v6 3/8] drm: bridge: Cadence: Add MHDP8501 DP 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: <PAXPR04MB9448680E4F589E1F722ADD2CF458A@PAXPR04MB9448.eurprd04.prod.outlook.com>
Date:   Fri, 16 Jun 2023 07:13:56 +0000
From:   Sandor Yu <sandor.yu@....com>
To:     Sam Ravnborg <sam@...nborg.org>
CC:     "andrzej.hajda@...el.com" <andrzej.hajda@...el.com>,
        "neil.armstrong@...aro.org" <neil.armstrong@...aro.org>,
        "robert.foss@...aro.org" <robert.foss@...aro.org>,
        "Laurent.pinchart@...asonboard.com" 
        <Laurent.pinchart@...asonboard.com>,
        "jonas@...boo.se" <jonas@...boo.se>,
        "jernej.skrabec@...il.com" <jernej.skrabec@...il.com>,
        "airlied@...il.com" <airlied@...il.com>,
        "daniel@...ll.ch" <daniel@...ll.ch>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "krzysztof.kozlowski+dt@...aro.org" 
        <krzysztof.kozlowski+dt@...aro.org>,
        "shawnguo@...nel.org" <shawnguo@...nel.org>,
        "s.hauer@...gutronix.de" <s.hauer@...gutronix.de>,
        "festevam@...il.com" <festevam@...il.com>,
        "vkoul@...nel.org" <vkoul@...nel.org>,
        "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-phy@...ts.infradead.org" <linux-phy@...ts.infradead.org>,
        Oliver Brown <oliver.brown@....com>,
        dl-linux-imx <linux-imx@....com>,
        "kernel@...gutronix.de" <kernel@...gutronix.de>
Subject: RE: [EXT] Re: [PATCH v6 3/8] drm: bridge: Cadence: Add MHDP8501 DP
 driver

Hi Sam,

Thanks your comments,

For i.MX8MQ, the display driver DCSS had create its own connector.
I will drop the code in the next version review patch set.

Thanks
Sandor

> -----Original Message-----
> From: Sam Ravnborg <sam@...nborg.org>
> Sent: 2023年6月16日 0:33
> To: Sandor Yu <sandor.yu@....com>
> Cc: andrzej.hajda@...el.com; neil.armstrong@...aro.org;
> robert.foss@...aro.org; Laurent.pinchart@...asonboard.com;
> jonas@...boo.se; jernej.skrabec@...il.com; airlied@...il.com;
> daniel@...ll.ch; robh+dt@...nel.org; krzysztof.kozlowski+dt@...aro.org;
> shawnguo@...nel.org; s.hauer@...gutronix.de; festevam@...il.com;
> vkoul@...nel.org; dri-devel@...ts.freedesktop.org;
> devicetree@...r.kernel.org; linux-arm-kernel@...ts.infradead.org;
> linux-kernel@...r.kernel.org; linux-phy@...ts.infradead.org; Oliver Brown
> <oliver.brown@....com>; dl-linux-imx <linux-imx@....com>;
> kernel@...gutronix.de
> Subject: [EXT] Re: [PATCH v6 3/8] drm: bridge: Cadence: Add MHDP8501 DP
> driver
> 
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report
> this email' button
> 
> 
> Hi Sandor,
> 
> On Thu, Jun 15, 2023 at 09:38:13AM +0800, Sandor Yu wrote:
> > Add a new DRM DisplayPort bridge driver for Candence MHDP8501 used in
> > i.MX8MQ SOC. MHDP8501 could support HDMI or DisplayPort standards
> > according embedded Firmware running in the uCPU.
> >
> > For iMX8MQ SOC, the DisplayPort FW was loaded and activated by SOC
> ROM
> > code. Bootload binary included HDMI FW was required for the driver.
> 
> The bridge driver supports creating a connector, but is this really necessary?
> 
> This part:
> > +static const struct drm_connector_funcs cdns_dp_connector_funcs = {
> > +     .fill_modes = drm_helper_probe_single_connector_modes,
> > +     .destroy = drm_connector_cleanup,
> > +     .reset = drm_atomic_helper_connector_reset,
> > +     .atomic_duplicate_state =
> drm_atomic_helper_connector_duplicate_state,
> > +     .atomic_destroy_state =
> > +drm_atomic_helper_connector_destroy_state,
> > +};
> > +
> > +static const struct drm_connector_helper_funcs
> cdns_dp_connector_helper_funcs = {
> > +     .get_modes = cdns_dp_connector_get_modes, };
> > +
> > +static int cdns_dp_bridge_attach(struct drm_bridge *bridge,
> > +                              enum drm_bridge_attach_flags flags)
> {
> > +     struct cdns_mhdp_device *mhdp = bridge->driver_private;
> > +     struct drm_encoder *encoder = bridge->encoder;
> > +     struct drm_connector *connector = &mhdp->connector;
> > +     int ret;
> > +
> > +     if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)) {
> > +             connector->interlace_allowed = 0;
> > +
> > +             connector->polled = DRM_CONNECTOR_POLL_HPD;
> > +
> > +             drm_connector_helper_add(connector,
> > + &cdns_dp_connector_helper_funcs);
> > +
> > +             drm_connector_init(bridge->dev, connector,
> &cdns_dp_connector_funcs,
> > +
> DRM_MODE_CONNECTOR_DisplayPort);
> > +
> > +             drm_connector_attach_encoder(connector, encoder);
> > +     }
> 
> Unless you have a display driver that do not create their own connector then
> drop the above and error out if DRM_BRIDGE_ATTACH_NO_CONNECTOR is
> not set.
> It is encouraged that display drivers create their own connector.
> 
> This was the only detail I looked for in the driver, I hope some else volunteer
> to review it.
> 
>         Sam