Re: [PATCH v12 4/7] drm: bridge: Cadence: Add MHDP8501 DP/HDMI driver
From: Alexander Stein
Date: Wed Jan 17 2024 - 04:47:09 EST
Hi Sandor,
thanks for the update.
Am Mittwoch, 10. Januar 2024, 02:08:45 CET schrieb Sandor Yu:
> Add a new DRM DisplayPort and HDMI 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/HDMI FW was loaded and activated by
> SOC's ROM code. Bootload binary included respective specific firmware
> is required.
>
> Driver will check display connector type and
> then load the corresponding driver.
>
> Signed-off-by: Sandor Yu <Sandor.yu@xxxxxxx>
> Tested-by: Alexander Stein <alexander.stein@xxxxxxxxxxxxxxx>
> ---
> v11->v12:
> - Replace DRM_INFO with dev_info or dev_warn.
> - Replace DRM_ERROR with dev_err.
> - Return ret when cdns_mhdp_dpcd_read failed in function
> cdns_dp_aux_transferi(). - Remove unused parmeter in function
> cdns_dp_get_msa_misc
> and use two separate variables for color space and bpc.
> - Add year 2024 to copyright.
>
> drivers/gpu/drm/bridge/cadence/Kconfig | 16 +
> drivers/gpu/drm/bridge/cadence/Makefile | 2 +
> .../drm/bridge/cadence/cdns-mhdp8501-core.c | 315 ++++++++
> .../drm/bridge/cadence/cdns-mhdp8501-core.h | 365 +++++++++
> .../gpu/drm/bridge/cadence/cdns-mhdp8501-dp.c | 699 ++++++++++++++++++
> .../drm/bridge/cadence/cdns-mhdp8501-hdmi.c | 678 +++++++++++++++++
> 6 files changed, 2075 insertions(+)
> create mode 100644 drivers/gpu/drm/bridge/cadence/cdns-mhdp8501-core.c
> create mode 100644 drivers/gpu/drm/bridge/cadence/cdns-mhdp8501-core.h
> create mode 100644 drivers/gpu/drm/bridge/cadence/cdns-mhdp8501-dp.c
> create mode 100644 drivers/gpu/drm/bridge/cadence/cdns-mhdp8501-hdmi.c
>
> diff --git a/drivers/gpu/drm/bridge/cadence/Kconfig
> b/drivers/gpu/drm/bridge/cadence/Kconfig index e0973339e9e33..45848e741f5f4
> 100644
> --- a/drivers/gpu/drm/bridge/cadence/Kconfig
> +++ b/drivers/gpu/drm/bridge/cadence/Kconfig
> @@ -51,3 +51,19 @@ config DRM_CDNS_MHDP8546_J721E
> initializes the J721E Display Port and sets up the
> clock and data muxes.
> endif
> +
> +config DRM_CDNS_MHDP8501
> + tristate "Cadence MHDP8501 DP/HDMI bridge"
> + select DRM_KMS_HELPER
> + select DRM_PANEL_BRIDGE
> + select DRM_DISPLAY_DP_HELPER
> + select DRM_DISPLAY_HELPER
> + select CDNS_MHDP_HELPER
> + select DRM_CDNS_AUDIO
> + depends on OF
> + help
> + Support Cadence MHDP8501 DisplayPort/HDMI bridge.
> + Cadence MHDP8501 support one or more protocols,
> + including DisplayPort and HDMI.
> + To use the DP and HDMI drivers, their respective
> + specific firmware is required.
> diff --git a/drivers/gpu/drm/bridge/cadence/Makefile
> b/drivers/gpu/drm/bridge/cadence/Makefile index
> 087dc074820d7..02c1a9f3cf6fc 100644
> --- a/drivers/gpu/drm/bridge/cadence/Makefile
> +++ b/drivers/gpu/drm/bridge/cadence/Makefile
> @@ -6,3 +6,5 @@ obj-$(CONFIG_CDNS_MHDP_HELPER) += cdns-mhdp-helper.o
> obj-$(CONFIG_DRM_CDNS_MHDP8546) += cdns-mhdp8546.o
> cdns-mhdp8546-y := cdns-mhdp8546-core.o cdns-mhdp8546-hdcp.o
> cdns-mhdp8546-$(CONFIG_DRM_CDNS_MHDP8546_J721E) += cdns-mhdp8546-j721e.o
> +obj-$(CONFIG_DRM_CDNS_MHDP8501) += cdns-mhdp8501.o
> +cdns-mhdp8501-y := cdns-mhdp8501-core.o cdns-mhdp8501-dp.o
> cdns-mhdp8501-hdmi.o diff --git
> a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8501-core.c
> b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8501-core.c new file mode 100644
> index 0000000000000..3080c7507a012
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8501-core.c
> @@ -0,0 +1,315 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Cadence Display Port Interface (DP) driver
> + *
> + * Copyright (C) 2023, 2024 NXP Semiconductor, Inc.
> + *
> + */
> +#include <drm/drm_of.h>
> +#include <drm/drm_print.h>
> +#include <linux/clk.h>
> +#include <linux/irq.h>
> +#include <linux/mutex.h>
> +#include <linux/of_device.h>
Since commit d57d584ef69de ("of: Stop circularly including of_device.h and
of_platform.h") you to explicitly include linux/platform_device.h here. Please
compile against next tree.
> +#include <linux/phy/phy.h>
> +
> +#include "cdns-mhdp8501-core.h"
> +
> [snip]
> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8501-dp.c
> b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8501-dp.c new file mode 100644
> index 0000000000000..6963c7143a3b0
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8501-dp.c
> @@ -0,0 +1,699 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Cadence MHDP8501 DisplayPort(DP) bridge driver
> + *
> + * Copyright (C) 2019-2024 NXP Semiconductor, Inc.
> + *
> + */
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_edid.h>
> +#include <drm/drm_print.h>
> +#include <linux/phy/phy.h>
> +#include <linux/phy/phy-dp.h>
> +
> +#include "cdns-mhdp8501-core.h"
> +
> +#define LINK_TRAINING_TIMEOUT_MS 500
> +#define LINK_TRAINING_RETRY_MS 20
> +
> +ssize_t cdns_dp_aux_transfer(struct drm_dp_aux *aux,
> + struct drm_dp_aux_msg *msg)
> +{
> + struct cdns_mhdp8501_device *mhdp = dev_get_drvdata(aux->dev);
> + bool native = msg->request & (DP_AUX_NATIVE_WRITE &
DP_AUX_NATIVE_READ);
> + int ret;
> +
> + /* Ignore address only message */
> + if (!msg->size || !msg->buffer) {
> + msg->reply = native ?
> + DP_AUX_NATIVE_REPLY_ACK : DP_AUX_I2C_REPLY_ACK;
> + return msg->size;
> + }
> +
> + if (!native) {
> + dev_err(mhdp->dev, "%s: only native messages supported\n",
__func__);
> + return -EINVAL;
> + }
> +
> + /* msg sanity check */
> + if (msg->size > DP_AUX_MAX_PAYLOAD_BYTES) {
> + dev_err(mhdp->dev, "%s: invalid msg: size(%zu),
request(%x)\n",
> + __func__, msg->size, (unsigned int)msg-
>request);
> + return -EINVAL;
> + }
> +
> + if (msg->request == DP_AUX_NATIVE_WRITE) {
> + const u8 *buf = msg->buffer;
> + int i;
> +
> + for (i = 0; i < msg->size; ++i) {
> + ret = cdns_mhdp_dpcd_write(&mhdp->base,
> + msg->address +
i, buf[i]);
> + if (ret < 0) {
> + dev_err(mhdp->dev, "Failed to write
DPCD\n");
> + return ret;
> + }
> + }
> + msg->reply = DP_AUX_NATIVE_REPLY_ACK;
> + return msg->size;
> + }
> +
> + if (msg->request == DP_AUX_NATIVE_READ) {
> + ret = cdns_mhdp_dpcd_read(&mhdp->base, msg->address,
> + msg->buffer, msg->size);
> + if (ret < 0)
> + return ret;
> + msg->reply = DP_AUX_NATIVE_REPLY_ACK;
> + return msg->size;
> + }
> + return 0;
> +}
> +
> +int cdns_dp_aux_destroy(struct cdns_mhdp8501_device *mhdp)
> +{
> + drm_dp_aux_unregister(&mhdp->dp.aux);
> +
> + return 0;
> +}
> +
> +static int cdns_dp_get_msa_misc(struct video_info *video)
> +{
> + u32 msa_misc;
> + u8 color_space = 0;
> + u8 bpc = 0;
> +
> + switch (video->color_fmt) {
> + /* set YUV default color space conversion to BT601 */
> + case DRM_COLOR_FORMAT_YCBCR444:
> + color_space = 6 + BT_601 * 8;
> + break;
> + case DRM_COLOR_FORMAT_YCBCR422:
> + color_space = 5 + BT_601 * 8;
> + break;
> + case DRM_COLOR_FORMAT_YCBCR420:
> + color_space = 5;
> + break;
> + case DRM_COLOR_FORMAT_RGB444:
> + default:
> + color_space = 0;
> + break;
> + };
> +
> + switch (video->bpc) {
> + case 6:
> + bpc = 0;
> + break;
> + case 10:
> + bpc = 2;
> + break;
> + case 12:
> + bpc = 3;
> + break;
> + case 16:
> + bpc = 4;
> + break;
> + case 8:
> + default:
> + bpc = 1;
> + break;
> + };
> +
> + msa_misc = (color_space << 1) | (bpc << 5);
This looks much nicer, thanks. But please order them in descending shift
width: bpc first then color_space.
> +
> + return msa_misc;
> +}
> +
> [snip]
> +int cdns_dp_set_host_cap(struct cdns_mhdp8501_device *mhdp)
This can be made static.
> +{
> + u8 msg[8];
> + int ret;
> +
> + msg[0] = drm_dp_link_rate_to_bw_code(mhdp->dp.rate);
> + msg[1] = mhdp->dp.num_lanes | SCRAMBLER_EN;
> + msg[2] = VOLTAGE_LEVEL_2;
> + msg[3] = PRE_EMPHASIS_LEVEL_3;
> + msg[4] = PTS1 | PTS2 | PTS3 | PTS4;
> + msg[5] = FAST_LT_NOT_SUPPORT;
> + msg[6] = mhdp->lane_mapping;
> + msg[7] = ENHANCED;
> +
> + mutex_lock(&mhdp->mbox_mutex);
> +
> + ret = cdns_mhdp_mailbox_send(&mhdp->base, MB_MODULE_ID_DP_TX,
> + DPTX_SET_HOST_CAPABILITIES,
> + sizeof(msg), msg);
> +
> + mutex_unlock(&mhdp->mbox_mutex);
> +
> + if (ret)
> + dev_err(mhdp->dev, "set host cap failed: %d\n", ret);
> +
> + return ret;
> +}
> [snip]
> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8501-hdmi.c
> b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8501-hdmi.c new file mode 100644
> index 0000000000000..ae21f7dfe5e94
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8501-hdmi.c
> @@ -0,0 +1,678 @@
> [snip]
> +bool cdns_hdmi_bridge_mode_fixup(struct drm_bridge *bridge,
> + const struct drm_display_mode *mode,
> + struct drm_display_mode
*adjusted_mode)
This can be made static.
Thanks and best regards,
Alexander
> +{
> + struct cdns_mhdp8501_device *mhdp = bridge->driver_private;
> + struct video_info *video = &mhdp->video_info;
> +
> + /* The only currently supported format */
> + video->bpc = 8;
> + video->color_fmt = DRM_COLOR_FORMAT_RGB444;
> +
> + return true;
> +}
> +
> +static enum drm_connector_status
> +cdns_hdmi_bridge_detect(struct drm_bridge *bridge)
> +{
> + struct cdns_mhdp8501_device *mhdp = bridge->driver_private;
> +
> + return cdns_mhdp8501_detect(mhdp);
> +}
> +
> +static struct edid *cdns_hdmi_bridge_get_edid(struct drm_bridge *bridge,
> + struct drm_connector
*connector)
> +{
> + struct cdns_mhdp8501_device *mhdp = bridge->driver_private;
> +
> + return drm_do_get_edid(connector, cdns_hdmi_get_edid_block, mhdp);
> +}
> +
> +static void cdns_hdmi_bridge_atomic_disable(struct drm_bridge *bridge,
> + struct drm_bridge_state
*old_state)
> +{
> + struct cdns_mhdp8501_device *mhdp = bridge->driver_private;
> +
> + mhdp->curr_conn = NULL;
> +
> + /* Mailbox protect for HDMI PHY access */
> + mutex_lock(&mhdp->mbox_mutex);
> + phy_power_off(mhdp->phy);
> + mutex_unlock(&mhdp->mbox_mutex);
> +}
> +
> +static void cdns_hdmi_bridge_atomic_enable(struct drm_bridge *bridge,
> + struct drm_bridge_state
*old_state)
> +{
> + struct cdns_mhdp8501_device *mhdp = bridge->driver_private;
> + struct drm_atomic_state *state = old_state->base.state;
> + struct drm_connector *connector;
> + struct drm_crtc_state *crtc_state;
> + struct drm_connector_state *conn_state;
> + const struct drm_display_mode *mode;
> +
> + connector = drm_atomic_get_new_connector_for_encoder(state,
> +
bridge->encoder);
> + if (WARN_ON(!connector))
> + return;
> +
> + mhdp->curr_conn = connector;
> +
> + conn_state = drm_atomic_get_new_connector_state(state, connector);
> + if (WARN_ON(!conn_state))
> + return;
> +
> + crtc_state = drm_atomic_get_new_crtc_state(state, conn_state->crtc);
> + if (WARN_ON(!crtc_state))
> + return;
> +
> + mode = &crtc_state->adjusted_mode;
> + dev_dbg(mhdp->dev, "Mode: %dx%dp%d\n",
> + mode->hdisplay, mode->vdisplay, mode->clock);
> + memcpy(&mhdp->mode, mode, sizeof(struct drm_display_mode));
> +
> + cdns_hdmi_mode_set(mhdp);
> +}
> +
> +const struct drm_bridge_funcs cdns_hdmi_bridge_funcs = {
> + .attach = cdns_hdmi_bridge_attach,
> + .detect = cdns_hdmi_bridge_detect,
> + .get_edid = cdns_hdmi_bridge_get_edid,
> + .mode_valid = cdns_hdmi_bridge_mode_valid,
> + .mode_fixup = cdns_hdmi_bridge_mode_fixup,
> + .atomic_enable = cdns_hdmi_bridge_atomic_enable,
> + .atomic_disable = cdns_hdmi_bridge_atomic_disable,
> + .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
> + .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
> + .atomic_reset = drm_atomic_helper_bridge_reset,
> +};
--
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/