From: Sam Ravnborg <sam@ravnborg.org>
To: Sandor Yu <Sandor.yu@nxp.com>
Cc: andrzej.hajda@intel.com, dri-devel@lists.freedesktop.org,
Laurent.pinchart@ideasonboard.com,
krzysztof.kozlowski+dt@linaro.org, linux-phy@lists.infradead.org,
jernej.skrabec@gmail.com, linux-imx@nxp.com,
devicetree@vger.kernel.org, kernel@pengutronix.de,
jonas@kwiboo.se, s.hauer@pengutronix.de, robh+dt@kernel.org,
linux-arm-kernel@lists.infradead.org, oliver.brown@nxp.com,
neil.armstrong@linaro.org, linux-kernel@vger.kernel.org,
robert.foss@linaro.org, vkoul@kernel.org, shawnguo@kernel.org
Subject: Re: [PATCH v6 3/8] drm: bridge: Cadence: Add MHDP8501 DP driver
Date: Thu, 15 Jun 2023 18:33:08 +0200 [thread overview]
Message-ID: <20230615163308.GA1588386@ravnborg.org> (raw)
In-Reply-To: <67f54be91e91f6291c4ad3f5155598d699b8bc09.1686729444.git.Sandor.yu@nxp.com>
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
next prev parent reply other threads:[~2023-06-15 16:33 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-15 1:38 [PATCH v6 0/8] Initial support for Cadence MHDP8501(HDMI/DP) for i.MX8MQ Sandor Yu
2023-06-15 1:38 ` [PATCH v6 1/8] drm: bridge: Cadence: convert mailbox functions to macro functions Sandor Yu
2023-06-15 1:38 ` [PATCH v6 2/8] dt-bindings: display: bridge: Add Cadence MHDP8501 HDMI and DP Sandor Yu
2023-06-16 9:31 ` Alexander Stein
2023-06-19 3:10 ` [EXT] " Sandor Yu
2023-06-20 15:37 ` Rob Herring
2023-06-20 15:48 ` Rob Herring
2023-06-21 2:23 ` [EXT] " Sandor Yu
2023-06-21 8:14 ` Krzysztof Kozlowski
2023-06-15 1:38 ` [PATCH v6 3/8] drm: bridge: Cadence: Add MHDP8501 DP driver Sandor Yu
2023-06-15 16:33 ` Sam Ravnborg [this message]
2023-06-16 7:13 ` [EXT] " Sandor Yu
2023-06-15 1:38 ` [PATCH v6 4/8] phy: Add HDMI configuration options Sandor Yu
2023-06-21 11:22 ` Vinod Koul
2023-07-10 7:28 ` [EXT] " Sandor Yu
2023-06-24 18:02 ` Dmitry Baryshkov
2023-07-10 7:28 ` [EXT] " Sandor Yu
2023-07-10 7:30 ` Dmitry Baryshkov
2023-07-10 7:41 ` Sandor Yu
2023-07-10 7:44 ` Dmitry Baryshkov
2023-07-10 8:01 ` Sandor Yu
2023-07-10 8:05 ` Dmitry Baryshkov
2023-07-10 8:11 ` Sandor Yu
2023-06-15 1:38 ` [PATCH v6 5/8] drm: bridge: Cadence: Add MHDP8501 HDMI driver Sandor Yu
2023-06-16 9:29 ` Alexander Stein
2023-06-19 3:11 ` [EXT] " Sandor Yu
2023-06-19 5:35 ` Alexander Stein
2023-06-15 1:38 ` [PATCH v6 6/8] dt-bindings: phy: Add Freescale iMX8MQ DP and HDMI PHY Sandor Yu
2023-06-16 9:34 ` Alexander Stein
2023-06-19 3:10 ` [EXT] " Sandor Yu
2023-06-15 1:38 ` [PATCH v6 7/8] phy: freescale: Add DisplayPort PHY driver for i.MX8MQ Sandor Yu
2023-06-21 11:24 ` Vinod Koul
2023-07-10 7:30 ` [EXT] " Sandor Yu
2023-06-15 1:38 ` [PATCH v6 8/8] phy: freescale: Add HDMI " Sandor Yu
2023-06-21 11:57 ` Vinod Koul
2023-07-10 7:30 ` [EXT] " Sandor Yu
2023-06-16 9:37 ` [PATCH v6 0/8] Initial support for Cadence MHDP8501(HDMI/DP) " Alexander Stein
2023-09-21 14:29 ` Vinod Koul
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230615163308.GA1588386@ravnborg.org \
--to=sam@ravnborg.org \
--cc=Laurent.pinchart@ideasonboard.com \
--cc=Sandor.yu@nxp.com \
--cc=andrzej.hajda@intel.com \
--cc=devicetree@vger.kernel.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=jernej.skrabec@gmail.com \
--cc=jonas@kwiboo.se \
--cc=kernel@pengutronix.de \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-imx@nxp.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-phy@lists.infradead.org \
--cc=neil.armstrong@linaro.org \
--cc=oliver.brown@nxp.com \
--cc=robert.foss@linaro.org \
--cc=robh+dt@kernel.org \
--cc=s.hauer@pengutronix.de \
--cc=shawnguo@kernel.org \
--cc=vkoul@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).