RE: [EXT] Re: [PATCH v12 4/7] drm: bridge: Cadence: Add MHDP8501 DP/HDMI driver

2024-02-04 Thread Sandor Yu
Hi Alexander,

Thanks your comments,

>
>
> 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 
> > Tested-by: Alexander Stein 
> > ---
> > 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 0..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 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
>
> 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.

OK, I will check it on next tree.

>
> > +#include 
> > +
> > +#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 0..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 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include "cdns-mhdp8501-core.h"
> > +
> > +#define 

Re: [PATCH v12 4/7] drm: bridge: Cadence: Add MHDP8501 DP/HDMI driver

2024-01-17 Thread Alexander Stein
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 
> Tested-by: Alexander Stein 
> ---
> 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 0..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 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 

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 
> +
> +#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 0..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 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#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 

[PATCH v12 4/7] drm: bridge: Cadence: Add MHDP8501 DP/HDMI driver

2024-01-09 Thread 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 
Tested-by: Alexander Stein 
---
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 0..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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "cdns-mhdp8501-core.h"
+
+static int cdns_mhdp8501_read_hpd(struct cdns_mhdp8501_device *mhdp)
+{
+   u8 status;
+   int ret;
+
+   mutex_lock(>mbox_mutex);
+
+   ret = cdns_mhdp_mailbox_send(>base, MB_MODULE_ID_GENERAL,
+GENERAL_GET_HPD_STATE, 0, NULL);
+   if (ret)
+   goto err_get_hpd;
+
+   ret = cdns_mhdp_mailbox_recv_header(>base, MB_MODULE_ID_GENERAL,
+   GENERAL_GET_HPD_STATE,
+   sizeof(status));
+   if (ret)
+   goto err_get_hpd;
+
+   ret = cdns_mhdp_mailbox_recv_data(>base, , sizeof(status));
+   if (ret)
+   goto err_get_hpd;
+
+   mutex_unlock(>mbox_mutex);
+
+   return status;
+
+err_get_hpd:
+   dev_err(mhdp->dev, "read hpd  failed: %d\n", ret);
+   mutex_unlock(>mbox_mutex);
+
+   return ret;
+}
+
+enum drm_connector_status cdns_mhdp8501_detect(struct cdns_mhdp8501_device 
*mhdp)
+{
+   u8 hpd = 0xf;
+
+   hpd = cdns_mhdp8501_read_hpd(mhdp);
+   if (hpd == 1)
+   return connector_status_connected;
+   else if (hpd == 0)
+   return connector_status_disconnected;
+
+   dev_warn(mhdp->dev, "Unknown cable status, hdp=%u\n", hpd);
+   return connector_status_unknown;
+}
+
+static void hotplug_work_func(struct work_struct