Hey Liu,

Thanks for submitting this series.

This patch looks good to me. Reviewed-by: Robert Foss robert.foss@linaro.org

On Thu, 18 Feb 2021 at 04:56, Liu Ying victor.liu@nxp.com wrote:

This patch allows LVDS PHYs to be configured through the generic functions and through a custom structure added to the generic union.

The parameters added here are based on common LVDS PHY implementation practices. The set of parameters should cover all potential users.

Cc: Kishon Vijay Abraham I kishon@ti.com Cc: Vinod Koul vkoul@kernel.org Cc: NXP Linux Team linux-imx@nxp.com Signed-off-by: Liu Ying victor.liu@nxp.com

---

v3->v4:

• No change.

v2->v3:

• No change.

v1->v2:

• No change.

include/linux/phy/phy-lvds.h | 48 ++++++++++++++++++++++++++++++++++++++++++++ include/linux/phy/phy.h | 4 ++++ 2 files changed, 52 insertions(+) create mode 100644 include/linux/phy/phy-lvds.h

diff --git a/include/linux/phy/phy-lvds.h b/include/linux/phy/phy-lvds.h new file mode 100644 index 00000000..1b5b9d6 --- /dev/null +++ b/include/linux/phy/phy-lvds.h @@ -0,0 +1,48 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/*

• • Copyright 2020 NXP
• */

+#ifndef __PHY_LVDS_H_ +#define __PHY_LVDS_H_

+/**

• • struct phy_configure_opts_lvds - LVDS configuration set
• • This structure is used to represent the configuration state of a
• • LVDS phy.
• */

+struct phy_configure_opts_lvds {

•   /**
  

•    * @bits_per_lane_and_dclk_cycle:
  

•    *
  

•    * Number of bits per data lane and differential clock cycle.
  

•    */
  

•   unsigned int bits_per_lane_and_dclk_cycle;
  

•   /**
  

•    * @differential_clk_rate:
  

•    *
  

•    * Clock rate, in Hertz, of the LVDS differential clock.
  

•    */
  

•   unsigned long differential_clk_rate;
  

•   /**
  

•    * @lanes:
  

•    *
  

•    * Number of active, consecutive, data lanes, starting from
  

•    * lane 0, used for the transmissions.
  

•    */
  

•   unsigned int lanes;
  

•   /**
  

•    * @is_slave:
  

•    *
  

•    * Boolean, true if the phy is a slave which works together
  

•    * with a master phy to support dual link transmission,
  

•    * otherwise a regular phy or a master phy.
  

•    */
  

•   bool is_slave;
  

+};

+#endif /* __PHY_LVDS_H_ */ diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h index e435bdb..d450b44 100644 --- a/include/linux/phy/phy.h +++ b/include/linux/phy/phy.h @@ -17,6 +17,7 @@ #include <linux/regulator/consumer.h>

#include <linux/phy/phy-dp.h> +#include <linux/phy/phy-lvds.h> #include <linux/phy/phy-mipi-dphy.h>

struct phy; @@ -51,10 +52,13 @@ enum phy_mode {

•         the MIPI_DPHY phy mode.
  

• @dp: Configuration set applicable for phys supporting

•         the DisplayPort protocol.
  

• • @lvds: Configuration set applicable for phys supporting

• •         the LVDS phy mode.
    

  */

union phy_configure_opts { struct phy_configure_opts_mipi_dphy mipi_dphy; struct phy_configure_opts_dp dp;

•   struct phy_configure_opts_lvds          lvds;
  

};

/**

2.7.4

Hey Liu,

This patch looks good to me Reviewed-by: Robert Foss robert.foss@linaro.org

On Thu, 18 Feb 2021 at 04:56, Liu Ying victor.liu@nxp.com wrote:

This patch adds RGB666_1X30_CPADLO, RGB888_1X30_CPADLO, RGB666_1X36_CPADLO and RGB888_1X36_CPADLO bus formats used by i.MX8qm/qxp pixel combiner. The RGB pixels with padding low per component are transmitted on a 30-bit input bus(10-bit per component) from a display controller or a 36-bit output bus(12-bit per component) to a pixel link.

Signed-off-by: Liu Ying victor.liu@nxp.com

v3->v4:

• No change.

v2->v3:

• No change.

v1->v2:

• No change.

include/uapi/linux/media-bus-format.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/media-bus-format.h b/include/uapi/linux/media-bus-format.h index 0dfc11e..ec3323d 100644 --- a/include/uapi/linux/media-bus-format.h +++ b/include/uapi/linux/media-bus-format.h @@ -34,7 +34,7 @@

#define MEDIA_BUS_FMT_FIXED 0x0001

-/* RGB - next is 0x101e */ +/* RGB - next is 0x1022 */ #define MEDIA_BUS_FMT_RGB444_1X12 0x1016 #define MEDIA_BUS_FMT_RGB444_2X8_PADHI_BE 0x1001 #define MEDIA_BUS_FMT_RGB444_2X8_PADHI_LE 0x1002 @@ -59,9 +59,13 @@ #define MEDIA_BUS_FMT_RGB888_3X8_DELTA 0x101d #define MEDIA_BUS_FMT_RGB888_1X7X4_SPWG 0x1011 #define MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA 0x1012 +#define MEDIA_BUS_FMT_RGB666_1X30_CPADLO 0x101e +#define MEDIA_BUS_FMT_RGB888_1X30_CPADLO 0x101f #define MEDIA_BUS_FMT_ARGB8888_1X32 0x100d #define MEDIA_BUS_FMT_RGB888_1X32_PADHI 0x100f #define MEDIA_BUS_FMT_RGB101010_1X30 0x1018 +#define MEDIA_BUS_FMT_RGB666_1X36_CPADLO 0x1020 +#define MEDIA_BUS_FMT_RGB888_1X36_CPADLO 0x1021 #define MEDIA_BUS_FMT_RGB121212_1X36 0x1019 #define MEDIA_BUS_FMT_RGB161616_1X48 0x101a

-- 2.7.4

Hey Liu,

This patch looks good to me. Reviewed-by: Robert Foss robert.foss@linaro.org

On Thu, 18 Feb 2021 at 04:56, Liu Ying victor.liu@nxp.com wrote:

This patch adds documentations for RGB666_1X30_CPADLO, RGB888_1X30_CPADLO, RGB666_1X36_CPADLO and RGB888_1X36_CPADLO bus formats used by i.MX8qm/qxp pixel combiner. The RGB pixels with padding low per component are transmitted on a 30-bit input bus(10-bit per component) from a display controller or a 36-bit output bus(12-bit per component) to a pixel link.

Signed-off-by: Liu Ying victor.liu@nxp.com

v3->v4:

• No change.

v2->v3:

• No change.

v1->v2:

• No change.

.../userspace-api/media/v4l/subdev-formats.rst | 156 +++++++++++++++++++++ 1 file changed, 156 insertions(+)

diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst index 7f16cbe..201c16d 100644 --- a/Documentation/userspace-api/media/v4l/subdev-formats.rst +++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst @@ -1488,6 +1488,80 @@ The following tables list existing packed RGB formats. - b\ :sub:`2` - b\ :sub:`1` - b\ :sub:`0`

• • .. _MEDIA-BUS-FMT-RGB666-1X30-CPADLO:

•  - MEDIA_BUS_FMT_RGB666_1X30-CPADLO
  

•  - 0x101e
  

•  -
  

•  - 0
  

•  - 0
  

•  - r\ :sub:`5`
  

•  - r\ :sub:`4`
  

•  - r\ :sub:`3`
  

•  - r\ :sub:`2`
  

•  - r\ :sub:`1`
  

•  - r\ :sub:`0`
  

•  - 0
  

•  - 0
  

•  - 0
  

•  - 0
  

•  - g\ :sub:`5`
  

•  - g\ :sub:`4`
  

•  - g\ :sub:`3`
  

•  - g\ :sub:`2`
  

•  - g\ :sub:`1`
  

•  - g\ :sub:`0`
  

•  - 0
  

•  - 0
  

•  - 0
  

•  - 0
  

•  - b\ :sub:`5`
  

•  - b\ :sub:`4`
  

•  - b\ :sub:`3`
  

•  - b\ :sub:`2`
  

•  - b\ :sub:`1`
  

•  - b\ :sub:`0`
  

•  - 0
  

•  - 0
  

•  - 0
  

•  - 0
  

• • .. _MEDIA-BUS-FMT-RGB888-1X30-CPADLO:

•  - MEDIA_BUS_FMT_RGB888_1X30-CPADLO
  

•  - 0x101f
  

•  -
  

•  - 0
  

•  - 0
  

•  - r\ :sub:`7`
  

•  - r\ :sub:`6`
  

•  - r\ :sub:`5`
  

•  - r\ :sub:`4`
  

•  - r\ :sub:`3`
  

•  - r\ :sub:`2`
  

•  - r\ :sub:`1`
  

•  - r\ :sub:`0`
  

•  - 0
  

•  - 0
  

•  - g\ :sub:`7`
  

•  - g\ :sub:`6`
  

•  - g\ :sub:`5`
  

•  - g\ :sub:`4`
  

•  - g\ :sub:`3`
  

•  - g\ :sub:`2`
  

•  - g\ :sub:`1`
  

•  - g\ :sub:`0`
  

•  - 0
  

•  - 0
  

•  - b\ :sub:`7`
  

•  - b\ :sub:`6`
  

•  - b\ :sub:`5`
  

•  - b\ :sub:`4`
  

•  - b\ :sub:`3`
  

•  - b\ :sub:`2`
  

•  - b\ :sub:`1`
  

•  - b\ :sub:`0`
  

•  - 0
  

•  - 0
  

  • .. _MEDIA-BUS-FMT-ARGB888-1X32:

    • MEDIA_BUS_FMT_ARGB888_1X32

@@ -1665,6 +1739,88 @@ The following table list existing packed 36bit wide RGB formats. - 2 - 1 - 0

• • .. _MEDIA-BUS-FMT-RGB666-1X36-CPADLO:

•  - MEDIA_BUS_FMT_RGB666_1X36_CPADLO
  

•  - 0x1020
  

•  -
  

•  - r\ :sub:`5`
  

•  - r\ :sub:`4`
  

•  - r\ :sub:`3`
  

•  - r\ :sub:`2`
  

•  - r\ :sub:`1`
  

•  - r\ :sub:`0`
  

•  - 0
  

•  - 0
  

•  - 0
  

•  - 0
  

•  - 0
  

•  - 0
  

•  - g\ :sub:`5`
  

•  - g\ :sub:`4`
  

•  - g\ :sub:`3`
  

•  - g\ :sub:`2`
  

•  - g\ :sub:`1`
  

•  - g\ :sub:`0`
  

•  - 0
  

•  - 0
  

•  - 0
  

•  - 0
  

•  - 0
  

•  - 0
  

•  - b\ :sub:`5`
  

•  - b\ :sub:`4`
  

•  - b\ :sub:`3`
  

•  - b\ :sub:`2`
  

•  - b\ :sub:`1`
  

•  - b\ :sub:`0`
  

•  - 0
  

•  - 0
  

•  - 0
  

•  - 0
  

•  - 0
  

•  - 0
  

• • .. _MEDIA-BUS-FMT-RGB888-1X36-CPADLO:

•  - MEDIA_BUS_FMT_RGB888_1X36_CPADLO
  

•  - 0x1021
  

•  -
  

•  - r\ :sub:`7`
  

•  - r\ :sub:`6`
  

•  - r\ :sub:`5`
  

•  - r\ :sub:`4`
  

•  - r\ :sub:`3`
  

•  - r\ :sub:`2`
  

•  - r\ :sub:`1`
  

•  - r\ :sub:`0`
  

•  - 0
  

•  - 0
  

•  - 0
  

•  - 0
  

•  - g\ :sub:`7`
  

•  - g\ :sub:`6`
  

•  - g\ :sub:`5`
  

•  - g\ :sub:`4`
  

•  - g\ :sub:`3`
  

•  - g\ :sub:`2`
  

•  - g\ :sub:`1`
  

•  - g\ :sub:`0`
  

•  - 0
  

•  - 0
  

•  - 0
  

•  - 0
  

•  - b\ :sub:`7`
  

•  - b\ :sub:`6`
  

•  - b\ :sub:`5`
  

•  - b\ :sub:`4`
  

•  - b\ :sub:`3`
  

•  - b\ :sub:`2`
  

•  - b\ :sub:`1`
  

•  - b\ :sub:`0`
  

•  - 0
  

•  - 0
  

•  - 0
  

•  - 0
  

  • .. _MEDIA-BUS-FMT-RGB121212-1X36:

    • MEDIA_BUS_FMT_RGB121212_1X36

-- 2.7.4

Hi Robert,

On Fri, 2021-02-26 at 14:07 +0100, Robert Foss wrote:

Hey Liu,

With the below nit straightened out, feel free to add my r-b.

Reviewed-by: Robert Foss robert.foss@linaro.org

Thanks for reviewing this patch.

On Thu, 18 Feb 2021 at 04:58, Liu Ying victor.liu@nxp.com wrote:

...

This patch adds a drm bridge driver for i.MX8qm/qxp pixel combiner. The pixel combiner takes two output streams from a single display controller and manipulates the two streams to support a number of modes(bypass, pixel combine, YUV444 to YUV422, split_RGB) configured as either one screen, two screens, or virtual screens. The pixel combiner is also responsible for generating some of the control signals for the pixel link output channel. For now, the driver only supports the bypass mode.

Signed-off-by: Liu Ying victor.liu@nxp.com

v3->v4:

• No change.

v2->v3:

• No change.

v1->v2:

• No change.

drivers/gpu/drm/bridge/Kconfig | 2 + drivers/gpu/drm/bridge/Makefile | 1 + drivers/gpu/drm/bridge/imx/Kconfig | 8 + drivers/gpu/drm/bridge/imx/Makefile | 1 + .../gpu/drm/bridge/imx/imx8qxp-pixel-combiner.c | 452 +++++++++++++++++++++ 5 files changed, 464 insertions(+) create mode 100644 drivers/gpu/drm/bridge/imx/Kconfig create mode 100644 drivers/gpu/drm/bridge/imx/Makefile create mode 100644 drivers/gpu/drm/bridge/imx/imx8qxp-pixel-combiner.c

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index e4110d6c..84944e0 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -256,6 +256,8 @@ source "drivers/gpu/drm/bridge/adv7511/Kconfig"

source "drivers/gpu/drm/bridge/cadence/Kconfig"

+source "drivers/gpu/drm/bridge/imx/Kconfig"

source "drivers/gpu/drm/bridge/synopsys/Kconfig"

endmenu diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile index 86e7acc..bc80cae 100644 --- a/drivers/gpu/drm/bridge/Makefile +++ b/drivers/gpu/drm/bridge/Makefile @@ -27,4 +27,5 @@ obj-$(CONFIG_DRM_NWL_MIPI_DSI) += nwl-dsi.o

obj-y += analogix/ obj-y += cadence/ +obj-y += imx/ obj-y += synopsys/ diff --git a/drivers/gpu/drm/bridge/imx/Kconfig b/drivers/gpu/drm/bridge/imx/Kconfig new file mode 100644 index 00000000..f1c91b6 --- /dev/null +++ b/drivers/gpu/drm/bridge/imx/Kconfig @@ -0,0 +1,8 @@ +config DRM_IMX8QXP_PIXEL_COMBINER

•   tristate "Freescale i.MX8QM/QXP pixel combiner"
  

•   depends on OF
  

•   depends on COMMON_CLK
  

•   select DRM_KMS_HELPER
  

•   help
  

•     Choose this to enable pixel combiner found in
  

•     Freescale i.MX8qm/qxp processors.
  

diff --git a/drivers/gpu/drm/bridge/imx/Makefile b/drivers/gpu/drm/bridge/imx/Makefile new file mode 100644 index 00000000..7d7c8d6 --- /dev/null +++ b/drivers/gpu/drm/bridge/imx/Makefile @@ -0,0 +1 @@ +obj-$(CONFIG_DRM_IMX8QXP_PIXEL_COMBINER) += imx8qxp-pixel-combiner.o diff --git a/drivers/gpu/drm/bridge/imx/imx8qxp-pixel-combiner.c b/drivers/gpu/drm/bridge/imx/imx8qxp-pixel-combiner.c new file mode 100644 index 00000000..cd5b1be --- /dev/null +++ b/drivers/gpu/drm/bridge/imx/imx8qxp-pixel-combiner.c @@ -0,0 +1,452 @@ +// SPDX-License-Identifier: GPL-2.0+

+/*

• • Copyright 2020 NXP
• */

+#include <linux/bitfield.h> +#include <linux/clk.h> +#include <linux/delay.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_graph.h> +#include <linux/platform_device.h> +#include <linux/pm_runtime.h>

+#include <drm/drm_atomic_state_helper.h> +#include <drm/drm_bridge.h> +#include <drm/drm_print.h>

+#define PC_CTRL_REG 0x0 +#define PC_COMBINE_ENABLE BIT(0) +#define PC_DISP_BYPASS(n) BIT(1 + 21 * (n)) +#define PC_DISP_HSYNC_POLARITY(n) BIT(2 + 11 * (n)) +#define PC_DISP_HSYNC_POLARITY_POS(n) DISP_HSYNC_POLARITY(n) +#define PC_DISP_VSYNC_POLARITY(n) BIT(3 + 11 * (n)) +#define PC_DISP_VSYNC_POLARITY_POS(n) DISP_VSYNC_POLARITY(n) +#define PC_DISP_DVALID_POLARITY(n) BIT(4 + 11 * (n)) +#define PC_DISP_DVALID_POLARITY_POS(n) DISP_DVALID_POLARITY(n) +#define PC_VSYNC_MASK_ENABLE BIT(5) +#define PC_SKIP_MODE BIT(6) +#define PC_SKIP_NUMBER_MASK GENMASK(12, 7) +#define PC_SKIP_NUMBER(n) FIELD_PREP(PC_SKIP_NUMBER_MASK, (n)) +#define PC_DISP0_PIX_DATA_FORMAT_MASK GENMASK(18, 16) +#define PC_DISP0_PIX_DATA_FORMAT(fmt) \

•                           FIELD_PREP(PC_DISP0_PIX_DATA_FORMAT_MASK, (fmt))
  

+#define PC_DISP1_PIX_DATA_FORMAT_MASK GENMASK(21, 19) +#define PC_DISP1_PIX_DATA_FORMAT(fmt) \

•                           FIELD_PREP(PC_DISP1_PIX_DATA_FORMAT_MASK, (fmt))
  

+#define PC_BUF_PARA_REG 0x10

This register is unused, keeping it in here to avoid future headaches seems like a good idea.

Yes, for now, this register is unused. It will be used to set the below PC_BUF_ACTIVE_DEPTH field when non-bypass modes are enabled, I think.

Though you said 'keeping it', you actually want me to drop it for now, right?

...

+#define PC_BUF_ACTIVE_DEPTH_MASK GENMASK(10, 0) +#define PC_BUF_ACTIVE_DEPTH(n) FIELD_PREP(PC_BUF_ACTIVE_DEPTH_MASK, (n))

+#define PC_SW_RESET_REG 0x20 +#define PC_SW_RESET_N BIT(0) +#define PC_DISP_SW_RESET_N(n) BIT(1 + (n)) +#define PC_FULL_RESET_N (PC_SW_RESET_N | \

•                                    PC_DISP_SW_RESET_N(0) |        \
  

•                                    PC_DISP_SW_RESET_N(1))
  

+#define PC_REG_SET 0x4 +#define PC_REG_CLR 0x8

+#define DRIVER_NAME "imx8qxp-pixel-combiner"

+enum imx8qxp_pc_pix_data_format {

•   RGB,
  

•   YUV444,
  

•   YUV422,
  

•   SPLIT_RGB,
  

YUV444, YUV422 & SPLIT_RGB are also unused, but if their values are compatible with the PC_DISP0_PIX_DATA_FORMAT macro I think keeping them around for future reference is a good idea.

Yes, YUV444, YUV422 & SPLIT_RGB are compatible with the PC_DISP{0, 1}_PIX_DATA_FORMAT macros, so I'll keep them here.

Regards, Liu Ying

...

+};

+struct imx8qxp_pc_channel {

•   struct drm_bridge bridge;
  

•   struct drm_bridge *next_bridge;
  

•   struct imx8qxp_pc *pc;
  

•   unsigned int stream_id;
  

•   bool is_available;
  

+};

+struct imx8qxp_pc {

•   struct device *dev;
  

•   struct imx8qxp_pc_channel ch[2];
  

•   struct clk *clk_apb;
  

•   void __iomem *base;
  

+};

+static inline u32 imx8qxp_pc_read(struct imx8qxp_pc *pc, unsigned int offset) +{

•   return readl(pc->base + offset);
  

+}

+static inline void +imx8qxp_pc_write(struct imx8qxp_pc *pc, unsigned int offset, u32 value) +{

•   writel(value, pc->base + offset);
  

+}

+static inline void +imx8qxp_pc_write_set(struct imx8qxp_pc *pc, unsigned int offset, u32 value) +{

•   imx8qxp_pc_write(pc, offset + PC_REG_SET, value);
  

+}

+static inline void +imx8qxp_pc_write_clr(struct imx8qxp_pc *pc, unsigned int offset, u32 value) +{

•   imx8qxp_pc_write(pc, offset + PC_REG_CLR, value);
  

+}

+static enum drm_mode_status +imx8qxp_pc_bridge_mode_valid(struct drm_bridge *bridge,

•                        const struct drm_display_info *info,
  

•                        const struct drm_display_mode *mode)
  

+{

•   if (mode->hdisplay > 2560)
  

•           return MODE_BAD_HVALUE;
  

•   return MODE_OK;
  

+}

+static int imx8qxp_pc_bridge_attach(struct drm_bridge *bridge,

•                               enum drm_bridge_attach_flags flags)
  

+{

•   struct imx8qxp_pc_channel *ch = bridge->driver_private;
  

•   struct imx8qxp_pc *pc = ch->pc;
  

•   if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)) {
  

•           DRM_DEV_ERROR(pc->dev,
  

•                         "do not support creating a drm_connector\n");
  

•           return -EINVAL;
  

•   }
  

•   if (!bridge->encoder) {
  

•           DRM_DEV_ERROR(pc->dev, "missing encoder\n");
  

•           return -ENODEV;
  

•   }
  

•   return drm_bridge_attach(bridge->encoder,
  

•                            ch->next_bridge, bridge,
  

•                            DRM_BRIDGE_ATTACH_NO_CONNECTOR);
  

+}

+static void +imx8qxp_pc_bridge_mode_set(struct drm_bridge *bridge,

•                      const struct drm_display_mode *mode,
  

•                      const struct drm_display_mode *adjusted_mode)
  

+{

•   struct imx8qxp_pc_channel *ch = bridge->driver_private;
  

•   struct imx8qxp_pc *pc = ch->pc;
  

•   u32 val;
  

•   int ret;
  

•   ret = pm_runtime_get_sync(pc->dev);
  

•   if (ret < 0)
  

•           DRM_DEV_ERROR(pc->dev,
  

•                         "failed to get runtime PM sync: %d\n", ret);
  

•   ret = clk_prepare_enable(pc->clk_apb);
  

•   if (ret)
  

•           DRM_DEV_ERROR(pc->dev, "%s: failed to enable apb clock: %d\n",
  

•                                                           __func__,  ret);
  

•   /* HSYNC to pixel link is active low. */
  

•   imx8qxp_pc_write_clr(pc, PC_CTRL_REG,
  

•                           PC_DISP_HSYNC_POLARITY(ch->stream_id));
  

•   /* VSYNC to pixel link is active low. */
  

•   imx8qxp_pc_write_clr(pc, PC_CTRL_REG,
  

•                           PC_DISP_VSYNC_POLARITY(ch->stream_id));
  

•   /* Data enable to pixel link is active high. */
  

•   imx8qxp_pc_write_set(pc, PC_CTRL_REG,
  

•                           PC_DISP_DVALID_POLARITY(ch->stream_id));
  

•   /* Mask the first frame output which may be incomplete. */
  

•   imx8qxp_pc_write_set(pc, PC_CTRL_REG, PC_VSYNC_MASK_ENABLE);
  

•   /* Only support RGB currently. */
  

•   val = imx8qxp_pc_read(pc, PC_CTRL_REG);
  

•   if (ch->stream_id == 0) {
  

•           val &= ~PC_DISP0_PIX_DATA_FORMAT_MASK;
  

•           val |= PC_DISP0_PIX_DATA_FORMAT(RGB);
  

•   } else {
  

•           val &= ~PC_DISP1_PIX_DATA_FORMAT_MASK;
  

•           val |= PC_DISP1_PIX_DATA_FORMAT(RGB);
  

•   }
  

•   imx8qxp_pc_write(pc, PC_CTRL_REG, val);
  

•   /* Only support bypass mode currently. */
  

•   imx8qxp_pc_write_set(pc, PC_CTRL_REG, PC_DISP_BYPASS(ch->stream_id));
  

•   clk_disable_unprepare(pc->clk_apb);
  

+}

+static void imx8qxp_pc_bridge_atomic_disable(struct drm_bridge *bridge,

•                           struct drm_bridge_state *old_bridge_state)
  

+{

•   struct imx8qxp_pc_channel *ch = bridge->driver_private;
  

•   struct imx8qxp_pc *pc = ch->pc;
  

•   int ret;
  

•   ret = pm_runtime_put(pc->dev);
  

•   if (ret < 0)
  

•           DRM_DEV_ERROR(pc->dev, "failed to put runtime PM: %d\n", ret);
  

+}

+static const u32 imx8qxp_pc_bus_output_fmts[] = {

•   MEDIA_BUS_FMT_RGB888_1X36_CPADLO,
  

•   MEDIA_BUS_FMT_RGB666_1X36_CPADLO,
  

+};

+static bool imx8qxp_pc_bus_output_fmt_supported(u32 fmt) +{

•   int i;
  

•   for (i = 0; i < ARRAY_SIZE(imx8qxp_pc_bus_output_fmts); i++) {
  

•           if (imx8qxp_pc_bus_output_fmts[i] == fmt)
  

•                   return true;
  

•   }
  

•   return false;
  

+}

+static u32 * +imx8qxp_pc_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge,

•                                   struct drm_bridge_state *bridge_state,
  

•                                   struct drm_crtc_state *crtc_state,
  

•                                   struct drm_connector_state *conn_state,
  

•                                   u32 output_fmt,
  

•                                   unsigned int *num_input_fmts)
  

+{

•   u32 *input_fmts;
  

•   if (!imx8qxp_pc_bus_output_fmt_supported(output_fmt))
  

•           return NULL;
  

•   *num_input_fmts = 1;
  

•   input_fmts = kmalloc(sizeof(*input_fmts), GFP_KERNEL);
  

•   if (!input_fmts)
  

•           return NULL;
  

•   switch (output_fmt) {
  

•   case MEDIA_BUS_FMT_RGB888_1X36_CPADLO:
  

•           input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X30_CPADLO;
  

•           break;
  

•   case MEDIA_BUS_FMT_RGB666_1X36_CPADLO:
  

•           input_fmts[0] = MEDIA_BUS_FMT_RGB666_1X30_CPADLO;
  

•           break;
  

•   default:
  

•           kfree(input_fmts);
  

•           input_fmts = NULL;
  

•           break;
  

•   }
  

•   return input_fmts;
  

+}

+static u32 * +imx8qxp_pc_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge,

•                                   struct drm_bridge_state *bridge_state,
  

•                                   struct drm_crtc_state *crtc_state,
  

•                                   struct drm_connector_state *conn_state,
  

•                                   unsigned int *num_output_fmts)
  

+{

•   *num_output_fmts = ARRAY_SIZE(imx8qxp_pc_bus_output_fmts);
  

•   return kmemdup(imx8qxp_pc_bus_output_fmts,
  

•                   sizeof(imx8qxp_pc_bus_output_fmts), GFP_KERNEL);
  

+}

+static const struct drm_bridge_funcs imx8qxp_pc_bridge_funcs = {

•   .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,
  

•   .mode_valid             = imx8qxp_pc_bridge_mode_valid,
  

•   .attach                 = imx8qxp_pc_bridge_attach,
  

•   .mode_set               = imx8qxp_pc_bridge_mode_set,
  

•   .atomic_disable         = imx8qxp_pc_bridge_atomic_disable,
  

•   .atomic_get_input_bus_fmts =
  

•                           imx8qxp_pc_bridge_atomic_get_input_bus_fmts,
  

•   .atomic_get_output_bus_fmts =
  

•                           imx8qxp_pc_bridge_atomic_get_output_bus_fmts,
  

+};

+static int imx8qxp_pc_bridge_probe(struct platform_device *pdev) +{

•   struct imx8qxp_pc *pc;
  

•   struct imx8qxp_pc_channel *ch;
  

•   struct device *dev = &pdev->dev;
  

•   struct device_node *np = dev->of_node;
  

•   struct device_node *child, *remote;
  

•   u32 i;
  

•   int ret;
  

•   pc = devm_kzalloc(dev, sizeof(*pc), GFP_KERNEL);
  

•   if (!pc)
  

•           return -ENOMEM;
  

•   pc->base = devm_platform_ioremap_resource(pdev, 0);
  

•   if (IS_ERR(pc->base))
  

•           return PTR_ERR(pc->base);
  

•   pc->dev = dev;
  

•   pc->clk_apb = devm_clk_get(dev, "apb");
  

•   if (IS_ERR(pc->clk_apb)) {
  

•           ret = PTR_ERR(pc->clk_apb);
  

•           if (ret != -EPROBE_DEFER)
  

•                   DRM_DEV_ERROR(dev, "failed to get apb clock: %d\n",
  

•                                                                   ret);
  

•           return ret;
  

•   }
  

•   platform_set_drvdata(pdev, pc);
  

•   pm_runtime_enable(dev);
  

•   for_each_available_child_of_node(np, child) {
  

•           ret = of_property_read_u32(child, "reg", &i);
  

•           if (ret || i > 1) {
  

•                   ret = -EINVAL;
  

•                   DRM_DEV_ERROR(dev,
  

•                                 "invalid channel(%u) node address\n", i);
  

•                   goto free_child;
  

•           }
  

•           ch = &pc->ch[i];
  

•           ch->pc = pc;
  

•           ch->stream_id = i;
  

•           remote = of_graph_get_remote_node(child, 1, 0);
  

•           if (!remote) {
  

•                   ret = -ENODEV;
  

•                   DRM_DEV_ERROR(dev,
  

•                       "channel%u failed to get port1's remote node: %d\n",
  

•                                                                   i, ret);
  

•                   goto free_child;
  

•           }
  

•           ch->next_bridge = of_drm_find_bridge(remote);
  

•           if (!ch->next_bridge) {
  

•                   of_node_put(remote);
  

•                   ret = -EPROBE_DEFER;
  

•                   DRM_DEV_DEBUG_DRIVER(dev,
  

•                           "channel%u failed to find next bridge: %d\n",
  

•                                                                   i, ret);
  

•                   goto free_child;
  

•           }
  

•           of_node_put(remote);
  

•           ch->bridge.driver_private = ch;
  

•           ch->bridge.funcs = &imx8qxp_pc_bridge_funcs;
  

•           ch->bridge.of_node = child;
  

•           ch->is_available = true;
  

•           drm_bridge_add(&ch->bridge);
  

•   }
  

•   return 0;
  

+free_child:

•   of_node_put(child);
  

•   if (i == 1 && pc->ch[0].next_bridge)
  

•           drm_bridge_remove(&pc->ch[0].bridge);
  

•   pm_runtime_disable(dev);
  

•   return ret;
  

+}

+static int imx8qxp_pc_bridge_remove(struct platform_device *pdev) +{

•   struct imx8qxp_pc *pc = platform_get_drvdata(pdev);
  

•   struct imx8qxp_pc_channel *ch;
  

•   int i;
  

•   for (i = 0; i < 2; i++) {
  

•           ch = &pc->ch[i];
  

•           if (!ch->is_available)
  

•                   continue;
  

•           drm_bridge_remove(&ch->bridge);
  

•           ch->is_available = false;
  

•   }
  

•   pm_runtime_disable(&pdev->dev);
  

•   return 0;
  

+}

+static int __maybe_unused imx8qxp_pc_runtime_suspend(struct device *dev) +{

•   struct platform_device *pdev = to_platform_device(dev);
  

•   struct imx8qxp_pc *pc = platform_get_drvdata(pdev);
  

•   int ret;
  

•   ret = clk_prepare_enable(pc->clk_apb);
  

•   if (ret)
  

•           DRM_DEV_ERROR(pc->dev, "%s: failed to enable apb clock: %d\n",
  

•                                                           __func__,  ret);
  

•   /* Disable pixel combiner by full reset. */
  

•   imx8qxp_pc_write_clr(pc, PC_SW_RESET_REG, PC_FULL_RESET_N);
  

•   clk_disable_unprepare(pc->clk_apb);
  

•   /* Ensure the reset takes effect. */
  

•   usleep_range(10, 20);
  

•   return ret;
  

+}

+static int __maybe_unused imx8qxp_pc_runtime_resume(struct device *dev) +{

•   struct platform_device *pdev = to_platform_device(dev);
  

•   struct imx8qxp_pc *pc = platform_get_drvdata(pdev);
  

•   int ret;
  

•   ret = clk_prepare_enable(pc->clk_apb);
  

•   if (ret) {
  

•           DRM_DEV_ERROR(pc->dev, "%s: failed to enable apb clock: %d\n",
  

•                                                           __func__, ret);
  

•           return ret;
  

•   }
  

•   /* out of reset */
  

•   imx8qxp_pc_write_set(pc, PC_SW_RESET_REG, PC_FULL_RESET_N);
  

•   clk_disable_unprepare(pc->clk_apb);
  

•   return ret;
  

+}

+static const struct dev_pm_ops imx8qxp_pc_pm_ops = {

•   SET_RUNTIME_PM_OPS(imx8qxp_pc_runtime_suspend,
  

•                      imx8qxp_pc_runtime_resume, NULL)
  

+};

+static const struct of_device_id imx8qxp_pc_dt_ids[] = {

•   { .compatible = "fsl,imx8qm-pixel-combiner", },
  

•   { .compatible = "fsl,imx8qxp-pixel-combiner", },
  

•   { /* sentinel */ }
  

+}; +MODULE_DEVICE_TABLE(of, imx8qxp_pc_dt_ids);

+static struct platform_driver imx8qxp_pc_bridge_driver = {

•   .probe  = imx8qxp_pc_bridge_probe,
  

•   .remove = imx8qxp_pc_bridge_remove,
  

•   .driver = {
  

•           .pm = &imx8qxp_pc_pm_ops,
  

•           .name = DRIVER_NAME,
  

•           .of_match_table = imx8qxp_pc_dt_ids,
  

•   },
  

+}; +module_platform_driver(imx8qxp_pc_bridge_driver);

+MODULE_DESCRIPTION("i.MX8QM/QXP pixel combiner bridge driver"); +MODULE_AUTHOR("Liu Ying victor.liu@nxp.com"); +MODULE_LICENSE("GPL v2");

+MODULE_ALIAS("platform:" DRIVER_NAME);

2.7.4

On Mon, 2021-03-01 at 11:56 +0100, Robert Foss wrote:

On Mon, 1 Mar 2021 at 10:07, Liu Ying victor.liu@nxp.com wrote:

...

Hi Robert,

On Fri, 2021-02-26 at 14:07 +0100, Robert Foss wrote:

...

Hey Liu,

With the below nit straightened out, feel free to add my r-b.

Reviewed-by: Robert Foss robert.foss@linaro.org

Thanks for reviewing this patch.

...

On Thu, 18 Feb 2021 at 04:58, Liu Ying victor.liu@nxp.com wrote:

...

This patch adds a drm bridge driver for i.MX8qm/qxp pixel combiner. The pixel combiner takes two output streams from a single display controller and manipulates the two streams to support a number of modes(bypass, pixel combine, YUV444 to YUV422, split_RGB) configured as either one screen, two screens, or virtual screens. The pixel combiner is also responsible for generating some of the control signals for the pixel link output channel. For now, the driver only supports the bypass mode.

Signed-off-by: Liu Ying victor.liu@nxp.com

v3->v4:

• No change.

v2->v3:

• No change.

v1->v2:

• No change.

drivers/gpu/drm/bridge/Kconfig | 2 + drivers/gpu/drm/bridge/Makefile | 1 + drivers/gpu/drm/bridge/imx/Kconfig | 8 + drivers/gpu/drm/bridge/imx/Makefile | 1 + .../gpu/drm/bridge/imx/imx8qxp-pixel-combiner.c | 452 +++++++++++++++++++++ 5 files changed, 464 insertions(+) create mode 100644 drivers/gpu/drm/bridge/imx/Kconfig create mode 100644 drivers/gpu/drm/bridge/imx/Makefile create mode 100644 drivers/gpu/drm/bridge/imx/imx8qxp-pixel-combiner.c

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index e4110d6c..84944e0 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -256,6 +256,8 @@ source "drivers/gpu/drm/bridge/adv7511/Kconfig"

source "drivers/gpu/drm/bridge/cadence/Kconfig"

+source "drivers/gpu/drm/bridge/imx/Kconfig"

source "drivers/gpu/drm/bridge/synopsys/Kconfig"

endmenu diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile index 86e7acc..bc80cae 100644 --- a/drivers/gpu/drm/bridge/Makefile +++ b/drivers/gpu/drm/bridge/Makefile @@ -27,4 +27,5 @@ obj-$(CONFIG_DRM_NWL_MIPI_DSI) += nwl-dsi.o

obj-y += analogix/ obj-y += cadence/ +obj-y += imx/ obj-y += synopsys/ diff --git a/drivers/gpu/drm/bridge/imx/Kconfig b/drivers/gpu/drm/bridge/imx/Kconfig new file mode 100644 index 00000000..f1c91b6 --- /dev/null +++ b/drivers/gpu/drm/bridge/imx/Kconfig @@ -0,0 +1,8 @@ +config DRM_IMX8QXP_PIXEL_COMBINER

•   tristate "Freescale i.MX8QM/QXP pixel combiner"
  

•   depends on OF
  

•   depends on COMMON_CLK
  

•   select DRM_KMS_HELPER
  

•   help
  

•     Choose this to enable pixel combiner found in
  

•     Freescale i.MX8qm/qxp processors.
  

diff --git a/drivers/gpu/drm/bridge/imx/Makefile b/drivers/gpu/drm/bridge/imx/Makefile new file mode 100644 index 00000000..7d7c8d6 --- /dev/null +++ b/drivers/gpu/drm/bridge/imx/Makefile @@ -0,0 +1 @@ +obj-$(CONFIG_DRM_IMX8QXP_PIXEL_COMBINER) += imx8qxp-pixel-combiner.o diff --git a/drivers/gpu/drm/bridge/imx/imx8qxp-pixel-combiner.c b/drivers/gpu/drm/bridge/imx/imx8qxp-pixel-combiner.c new file mode 100644 index 00000000..cd5b1be --- /dev/null +++ b/drivers/gpu/drm/bridge/imx/imx8qxp-pixel-combiner.c @@ -0,0 +1,452 @@ +// SPDX-License-Identifier: GPL-2.0+

+/*

• • Copyright 2020 NXP
• */

+#include <linux/bitfield.h> +#include <linux/clk.h> +#include <linux/delay.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_graph.h> +#include <linux/platform_device.h> +#include <linux/pm_runtime.h>

+#include <drm/drm_atomic_state_helper.h> +#include <drm/drm_bridge.h> +#include <drm/drm_print.h>

+#define PC_CTRL_REG 0x0 +#define PC_COMBINE_ENABLE BIT(0) +#define PC_DISP_BYPASS(n) BIT(1 + 21 * (n)) +#define PC_DISP_HSYNC_POLARITY(n) BIT(2 + 11 * (n)) +#define PC_DISP_HSYNC_POLARITY_POS(n) DISP_HSYNC_POLARITY(n) +#define PC_DISP_VSYNC_POLARITY(n) BIT(3 + 11 * (n)) +#define PC_DISP_VSYNC_POLARITY_POS(n) DISP_VSYNC_POLARITY(n) +#define PC_DISP_DVALID_POLARITY(n) BIT(4 + 11 * (n)) +#define PC_DISP_DVALID_POLARITY_POS(n) DISP_DVALID_POLARITY(n) +#define PC_VSYNC_MASK_ENABLE BIT(5) +#define PC_SKIP_MODE BIT(6) +#define PC_SKIP_NUMBER_MASK GENMASK(12, 7) +#define PC_SKIP_NUMBER(n) FIELD_PREP(PC_SKIP_NUMBER_MASK, (n)) +#define PC_DISP0_PIX_DATA_FORMAT_MASK GENMASK(18, 16) +#define PC_DISP0_PIX_DATA_FORMAT(fmt) \

•                           FIELD_PREP(PC_DISP0_PIX_DATA_FORMAT_MASK, (fmt))
  

+#define PC_DISP1_PIX_DATA_FORMAT_MASK GENMASK(21, 19) +#define PC_DISP1_PIX_DATA_FORMAT(fmt) \

•                           FIELD_PREP(PC_DISP1_PIX_DATA_FORMAT_MASK, (fmt))
  

+#define PC_BUF_PARA_REG 0x10

This register is unused, keeping it in here to avoid future headaches seems like a good idea.

Yes, for now, this register is unused. It will be used to set the below PC_BUF_ACTIVE_DEPTH field when non-bypass modes are enabled, I think.

Though you said 'keeping it', you actually want me to drop it for now, right?

If there is a 2nd series coming which will enable non-bypass modes, then maybe add it in that series instead.

Ok, I'll drop PC_BUF_PARA_REG for now in the next version.

Thanks, Liu Ying

...

...

...

+#define PC_BUF_ACTIVE_DEPTH_MASK GENMASK(10, 0) +#define PC_BUF_ACTIVE_DEPTH(n) FIELD_PREP(PC_BUF_ACTIVE_DEPTH_MASK, (n))

+#define PC_SW_RESET_REG 0x20 +#define PC_SW_RESET_N BIT(0) +#define PC_DISP_SW_RESET_N(n) BIT(1 + (n)) +#define PC_FULL_RESET_N (PC_SW_RESET_N | \

•                                    PC_DISP_SW_RESET_N(0) |        \
  

•                                    PC_DISP_SW_RESET_N(1))
  

+#define PC_REG_SET 0x4 +#define PC_REG_CLR 0x8

+#define DRIVER_NAME "imx8qxp-pixel-combiner"

+enum imx8qxp_pc_pix_data_format {

•   RGB,
  

•   YUV444,
  

•   YUV422,
  

•   SPLIT_RGB,
  

YUV444, YUV422 & SPLIT_RGB are also unused, but if their values are compatible with the PC_DISP0_PIX_DATA_FORMAT macro I think keeping them around for future reference is a good idea.

Yes, YUV444, YUV422 & SPLIT_RGB are compatible with the PC_DISP{0, 1}_PIX_DATA_FORMAT macros, so I'll keep them here.

Regards, Liu Ying

...

...

+};

+struct imx8qxp_pc_channel {

•   struct drm_bridge bridge;
  

•   struct drm_bridge *next_bridge;
  

•   struct imx8qxp_pc *pc;
  

•   unsigned int stream_id;
  

•   bool is_available;
  

+};

+struct imx8qxp_pc {

•   struct device *dev;
  

•   struct imx8qxp_pc_channel ch[2];
  

•   struct clk *clk_apb;
  

•   void __iomem *base;
  

+};

+static inline u32 imx8qxp_pc_read(struct imx8qxp_pc *pc, unsigned int offset) +{

•   return readl(pc->base + offset);
  

+}

+static inline void +imx8qxp_pc_write(struct imx8qxp_pc *pc, unsigned int offset, u32 value) +{

•   writel(value, pc->base + offset);
  

+}

+static inline void +imx8qxp_pc_write_set(struct imx8qxp_pc *pc, unsigned int offset, u32 value) +{

•   imx8qxp_pc_write(pc, offset + PC_REG_SET, value);
  

+}

+static inline void +imx8qxp_pc_write_clr(struct imx8qxp_pc *pc, unsigned int offset, u32 value) +{

•   imx8qxp_pc_write(pc, offset + PC_REG_CLR, value);
  

+}

+static enum drm_mode_status +imx8qxp_pc_bridge_mode_valid(struct drm_bridge *bridge,

•                        const struct drm_display_info *info,
  

•                        const struct drm_display_mode *mode)
  

+{

•   if (mode->hdisplay > 2560)
  

•           return MODE_BAD_HVALUE;
  

•   return MODE_OK;
  

+}

+static int imx8qxp_pc_bridge_attach(struct drm_bridge *bridge,

•                               enum drm_bridge_attach_flags flags)
  

+{

•   struct imx8qxp_pc_channel *ch = bridge->driver_private;
  

•   struct imx8qxp_pc *pc = ch->pc;
  

•   if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)) {
  

•           DRM_DEV_ERROR(pc->dev,
  

•                         "do not support creating a drm_connector\n");
  

•           return -EINVAL;
  

•   }
  

•   if (!bridge->encoder) {
  

•           DRM_DEV_ERROR(pc->dev, "missing encoder\n");
  

•           return -ENODEV;
  

•   }
  

•   return drm_bridge_attach(bridge->encoder,
  

•                            ch->next_bridge, bridge,
  

•                            DRM_BRIDGE_ATTACH_NO_CONNECTOR);
  

+}

+static void +imx8qxp_pc_bridge_mode_set(struct drm_bridge *bridge,

•                      const struct drm_display_mode *mode,
  

•                      const struct drm_display_mode *adjusted_mode)
  

+{

•   struct imx8qxp_pc_channel *ch = bridge->driver_private;
  

•   struct imx8qxp_pc *pc = ch->pc;
  

•   u32 val;
  

•   int ret;
  

•   ret = pm_runtime_get_sync(pc->dev);
  

•   if (ret < 0)
  

•           DRM_DEV_ERROR(pc->dev,
  

•                         "failed to get runtime PM sync: %d\n", ret);
  

•   ret = clk_prepare_enable(pc->clk_apb);
  

•   if (ret)
  

•           DRM_DEV_ERROR(pc->dev, "%s: failed to enable apb clock: %d\n",
  

•                                                           __func__,  ret);
  

•   /* HSYNC to pixel link is active low. */
  

•   imx8qxp_pc_write_clr(pc, PC_CTRL_REG,
  

•                           PC_DISP_HSYNC_POLARITY(ch->stream_id));
  

•   /* VSYNC to pixel link is active low. */
  

•   imx8qxp_pc_write_clr(pc, PC_CTRL_REG,
  

•                           PC_DISP_VSYNC_POLARITY(ch->stream_id));
  

•   /* Data enable to pixel link is active high. */
  

•   imx8qxp_pc_write_set(pc, PC_CTRL_REG,
  

•                           PC_DISP_DVALID_POLARITY(ch->stream_id));
  

•   /* Mask the first frame output which may be incomplete. */
  

•   imx8qxp_pc_write_set(pc, PC_CTRL_REG, PC_VSYNC_MASK_ENABLE);
  

•   /* Only support RGB currently. */
  

•   val = imx8qxp_pc_read(pc, PC_CTRL_REG);
  

•   if (ch->stream_id == 0) {
  

•           val &= ~PC_DISP0_PIX_DATA_FORMAT_MASK;
  

•           val |= PC_DISP0_PIX_DATA_FORMAT(RGB);
  

•   } else {
  

•           val &= ~PC_DISP1_PIX_DATA_FORMAT_MASK;
  

•           val |= PC_DISP1_PIX_DATA_FORMAT(RGB);
  

•   }
  

•   imx8qxp_pc_write(pc, PC_CTRL_REG, val);
  

•   /* Only support bypass mode currently. */
  

•   imx8qxp_pc_write_set(pc, PC_CTRL_REG, PC_DISP_BYPASS(ch->stream_id));
  

•   clk_disable_unprepare(pc->clk_apb);
  

+}

+static void imx8qxp_pc_bridge_atomic_disable(struct drm_bridge *bridge,

•                           struct drm_bridge_state *old_bridge_state)
  

+{

•   struct imx8qxp_pc_channel *ch = bridge->driver_private;
  

•   struct imx8qxp_pc *pc = ch->pc;
  

•   int ret;
  

•   ret = pm_runtime_put(pc->dev);
  

•   if (ret < 0)
  

•           DRM_DEV_ERROR(pc->dev, "failed to put runtime PM: %d\n", ret);
  

+}

+static const u32 imx8qxp_pc_bus_output_fmts[] = {

•   MEDIA_BUS_FMT_RGB888_1X36_CPADLO,
  

•   MEDIA_BUS_FMT_RGB666_1X36_CPADLO,
  

+};

+static bool imx8qxp_pc_bus_output_fmt_supported(u32 fmt) +{

•   int i;
  

•   for (i = 0; i < ARRAY_SIZE(imx8qxp_pc_bus_output_fmts); i++) {
  

•           if (imx8qxp_pc_bus_output_fmts[i] == fmt)
  

•                   return true;
  

•   }
  

•   return false;
  

+}

+static u32 * +imx8qxp_pc_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge,

•                                   struct drm_bridge_state *bridge_state,
  

•                                   struct drm_crtc_state *crtc_state,
  

•                                   struct drm_connector_state *conn_state,
  

•                                   u32 output_fmt,
  

•                                   unsigned int *num_input_fmts)
  

+{

•   u32 *input_fmts;
  

•   if (!imx8qxp_pc_bus_output_fmt_supported(output_fmt))
  

•           return NULL;
  

•   *num_input_fmts = 1;
  

•   input_fmts = kmalloc(sizeof(*input_fmts), GFP_KERNEL);
  

•   if (!input_fmts)
  

•           return NULL;
  

•   switch (output_fmt) {
  

•   case MEDIA_BUS_FMT_RGB888_1X36_CPADLO:
  

•           input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X30_CPADLO;
  

•           break;
  

•   case MEDIA_BUS_FMT_RGB666_1X36_CPADLO:
  

•           input_fmts[0] = MEDIA_BUS_FMT_RGB666_1X30_CPADLO;
  

•           break;
  

•   default:
  

•           kfree(input_fmts);
  

•           input_fmts = NULL;
  

•           break;
  

•   }
  

•   return input_fmts;
  

+}

+static u32 * +imx8qxp_pc_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge,

•                                   struct drm_bridge_state *bridge_state,
  

•                                   struct drm_crtc_state *crtc_state,
  

•                                   struct drm_connector_state *conn_state,
  

•                                   unsigned int *num_output_fmts)
  

+{

•   *num_output_fmts = ARRAY_SIZE(imx8qxp_pc_bus_output_fmts);
  

•   return kmemdup(imx8qxp_pc_bus_output_fmts,
  

•                   sizeof(imx8qxp_pc_bus_output_fmts), GFP_KERNEL);
  

+}

+static const struct drm_bridge_funcs imx8qxp_pc_bridge_funcs = {

•   .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,
  

•   .mode_valid             = imx8qxp_pc_bridge_mode_valid,
  

•   .attach                 = imx8qxp_pc_bridge_attach,
  

•   .mode_set               = imx8qxp_pc_bridge_mode_set,
  

•   .atomic_disable         = imx8qxp_pc_bridge_atomic_disable,
  

•   .atomic_get_input_bus_fmts =
  

•                           imx8qxp_pc_bridge_atomic_get_input_bus_fmts,
  

•   .atomic_get_output_bus_fmts =
  

•                           imx8qxp_pc_bridge_atomic_get_output_bus_fmts,
  

+};

+static int imx8qxp_pc_bridge_probe(struct platform_device *pdev) +{

•   struct imx8qxp_pc *pc;
  

•   struct imx8qxp_pc_channel *ch;
  

•   struct device *dev = &pdev->dev;
  

•   struct device_node *np = dev->of_node;
  

•   struct device_node *child, *remote;
  

•   u32 i;
  

•   int ret;
  

•   pc = devm_kzalloc(dev, sizeof(*pc), GFP_KERNEL);
  

•   if (!pc)
  

•           return -ENOMEM;
  

•   pc->base = devm_platform_ioremap_resource(pdev, 0);
  

•   if (IS_ERR(pc->base))
  

•           return PTR_ERR(pc->base);
  

•   pc->dev = dev;
  

•   pc->clk_apb = devm_clk_get(dev, "apb");
  

•   if (IS_ERR(pc->clk_apb)) {
  

•           ret = PTR_ERR(pc->clk_apb);
  

•           if (ret != -EPROBE_DEFER)
  

•                   DRM_DEV_ERROR(dev, "failed to get apb clock: %d\n",
  

•                                                                   ret);
  

•           return ret;
  

•   }
  

•   platform_set_drvdata(pdev, pc);
  

•   pm_runtime_enable(dev);
  

•   for_each_available_child_of_node(np, child) {
  

•           ret = of_property_read_u32(child, "reg", &i);
  

•           if (ret || i > 1) {
  

•                   ret = -EINVAL;
  

•                   DRM_DEV_ERROR(dev,
  

•                                 "invalid channel(%u) node address\n", i);
  

•                   goto free_child;
  

•           }
  

•           ch = &pc->ch[i];
  

•           ch->pc = pc;
  

•           ch->stream_id = i;
  

•           remote = of_graph_get_remote_node(child, 1, 0);
  

•           if (!remote) {
  

•                   ret = -ENODEV;
  

•                   DRM_DEV_ERROR(dev,
  

•                       "channel%u failed to get port1's remote node: %d\n",
  

•                                                                   i, ret);
  

•                   goto free_child;
  

•           }
  

•           ch->next_bridge = of_drm_find_bridge(remote);
  

•           if (!ch->next_bridge) {
  

•                   of_node_put(remote);
  

•                   ret = -EPROBE_DEFER;
  

•                   DRM_DEV_DEBUG_DRIVER(dev,
  

•                           "channel%u failed to find next bridge: %d\n",
  

•                                                                   i, ret);
  

•                   goto free_child;
  

•           }
  

•           of_node_put(remote);
  

•           ch->bridge.driver_private = ch;
  

•           ch->bridge.funcs = &imx8qxp_pc_bridge_funcs;
  

•           ch->bridge.of_node = child;
  

•           ch->is_available = true;
  

•           drm_bridge_add(&ch->bridge);
  

•   }
  

•   return 0;
  

+free_child:

•   of_node_put(child);
  

•   if (i == 1 && pc->ch[0].next_bridge)
  

•           drm_bridge_remove(&pc->ch[0].bridge);
  

•   pm_runtime_disable(dev);
  

•   return ret;
  

+}

+static int imx8qxp_pc_bridge_remove(struct platform_device *pdev) +{

•   struct imx8qxp_pc *pc = platform_get_drvdata(pdev);
  

•   struct imx8qxp_pc_channel *ch;
  

•   int i;
  

•   for (i = 0; i < 2; i++) {
  

•           ch = &pc->ch[i];
  

•           if (!ch->is_available)
  

•                   continue;
  

•           drm_bridge_remove(&ch->bridge);
  

•           ch->is_available = false;
  

•   }
  

•   pm_runtime_disable(&pdev->dev);
  

•   return 0;
  

+}

+static int __maybe_unused imx8qxp_pc_runtime_suspend(struct device *dev) +{

•   struct platform_device *pdev = to_platform_device(dev);
  

•   struct imx8qxp_pc *pc = platform_get_drvdata(pdev);
  

•   int ret;
  

•   ret = clk_prepare_enable(pc->clk_apb);
  

•   if (ret)
  

•           DRM_DEV_ERROR(pc->dev, "%s: failed to enable apb clock: %d\n",
  

•                                                           __func__,  ret);
  

•   /* Disable pixel combiner by full reset. */
  

•   imx8qxp_pc_write_clr(pc, PC_SW_RESET_REG, PC_FULL_RESET_N);
  

•   clk_disable_unprepare(pc->clk_apb);
  

•   /* Ensure the reset takes effect. */
  

•   usleep_range(10, 20);
  

•   return ret;
  

+}

+static int __maybe_unused imx8qxp_pc_runtime_resume(struct device *dev) +{

•   struct platform_device *pdev = to_platform_device(dev);
  

•   struct imx8qxp_pc *pc = platform_get_drvdata(pdev);
  

•   int ret;
  

•   ret = clk_prepare_enable(pc->clk_apb);
  

•   if (ret) {
  

•           DRM_DEV_ERROR(pc->dev, "%s: failed to enable apb clock: %d\n",
  

•                                                           __func__, ret);
  

•           return ret;
  

•   }
  

•   /* out of reset */
  

•   imx8qxp_pc_write_set(pc, PC_SW_RESET_REG, PC_FULL_RESET_N);
  

•   clk_disable_unprepare(pc->clk_apb);
  

•   return ret;
  

+}

+static const struct dev_pm_ops imx8qxp_pc_pm_ops = {

•   SET_RUNTIME_PM_OPS(imx8qxp_pc_runtime_suspend,
  

•                      imx8qxp_pc_runtime_resume, NULL)
  

+};

+static const struct of_device_id imx8qxp_pc_dt_ids[] = {

•   { .compatible = "fsl,imx8qm-pixel-combiner", },
  

•   { .compatible = "fsl,imx8qxp-pixel-combiner", },
  

•   { /* sentinel */ }
  

+}; +MODULE_DEVICE_TABLE(of, imx8qxp_pc_dt_ids);

+static struct platform_driver imx8qxp_pc_bridge_driver = {

•   .probe  = imx8qxp_pc_bridge_probe,
  

•   .remove = imx8qxp_pc_bridge_remove,
  

•   .driver = {
  

•           .pm = &imx8qxp_pc_pm_ops,
  

•           .name = DRIVER_NAME,
  

•           .of_match_table = imx8qxp_pc_dt_ids,
  

•   },
  

+}; +module_platform_driver(imx8qxp_pc_bridge_driver);

+MODULE_DESCRIPTION("i.MX8QM/QXP pixel combiner bridge driver"); +MODULE_AUTHOR("Liu Ying victor.liu@nxp.com"); +MODULE_LICENSE("GPL v2");

+MODULE_ALIAS("platform:" DRIVER_NAME);

2.7.4

Hi Robert,

On Tue, 2021-03-02 at 14:53 +0100, Robert Foss wrote:

Hey Liu,

Thanks for submitting this patch.

I only have one comment below. With that addressed, feel free to add my r-b.

Reviewed-by: Robert Foss robert.foss@linaro.org

Thanks for reviewing this patch.

On Thu, 18 Feb 2021 at 04:59, Liu Ying victor.liu@nxp.com wrote:

...

This patch adds a drm bridge driver for i.MX8qm/qxp display pixel link. The pixel link forms a standard asynchronous linkage between pixel sources(display controller or camera module) and pixel consumers(imaging or displays). It consists of two distinct functions, a pixel transfer function and a control interface.

Signed-off-by: Liu Ying victor.liu@nxp.com

v3->v4:

• No change.

v2->v3:

• Drop two macros which help define functions and define them directly.
• Properly disable all pixel link controls to POR value by calling imx8qxp_pixel_link_disable_all_controls() from imx8qxp_pixel_link_bridge_probe().

v1->v2:

• Rewrite the function to find the next bridge by properly using OF APIs and dropping unnecessary DT validation. (Rob)

drivers/gpu/drm/bridge/imx/Kconfig | 8 + drivers/gpu/drm/bridge/imx/Makefile | 1 + drivers/gpu/drm/bridge/imx/imx8qxp-pixel-link.c | 426 ++++++++++++++++++++++++ 3 files changed, 435 insertions(+) create mode 100644 drivers/gpu/drm/bridge/imx/imx8qxp-pixel-link.c

diff --git a/drivers/gpu/drm/bridge/imx/Kconfig b/drivers/gpu/drm/bridge/imx/Kconfig index f1c91b6..4d1f027 100644 --- a/drivers/gpu/drm/bridge/imx/Kconfig +++ b/drivers/gpu/drm/bridge/imx/Kconfig @@ -6,3 +6,11 @@ config DRM_IMX8QXP_PIXEL_COMBINER help Choose this to enable pixel combiner found in Freescale i.MX8qm/qxp processors.

+config DRM_IMX8QXP_PIXEL_LINK

•   tristate "Freescale i.MX8QM/QXP display pixel link"
  

•   depends on OF
  

•   select DRM_KMS_HELPER
  

•   help
  

•     Choose this to enable display pixel link found in
  

•     Freescale i.MX8qm/qxp processors.
  

diff --git a/drivers/gpu/drm/bridge/imx/Makefile b/drivers/gpu/drm/bridge/imx/Makefile index 7d7c8d6..c15469f 100644 --- a/drivers/gpu/drm/bridge/imx/Makefile +++ b/drivers/gpu/drm/bridge/imx/Makefile @@ -1 +1,2 @@ obj-$(CONFIG_DRM_IMX8QXP_PIXEL_COMBINER) += imx8qxp-pixel-combiner.o +obj-$(CONFIG_DRM_IMX8QXP_PIXEL_LINK) += imx8qxp-pixel-link.o diff --git a/drivers/gpu/drm/bridge/imx/imx8qxp-pixel-link.c b/drivers/gpu/drm/bridge/imx/imx8qxp-pixel-link.c new file mode 100644 index 00000000..2e5ba4a --- /dev/null +++ b/drivers/gpu/drm/bridge/imx/imx8qxp-pixel-link.c @@ -0,0 +1,426 @@ +// SPDX-License-Identifier: GPL-2.0+

+/*

• • Copyright 2020 NXP
• */

+#include <linux/firmware/imx/svc/misc.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_graph.h> +#include <linux/platform_device.h>

+#include <drm/drm_atomic_state_helper.h> +#include <drm/drm_bridge.h> +#include <drm/drm_print.h>

+#include <dt-bindings/firmware/imx/rsrc.h>

+#define DRIVER_NAME "imx8qxp-display-pixel-link" +#define PL_MAX_MST_ADDR 3 +#define PL_MAX_NEXT_BRIDGES 2

+struct imx8qxp_pixel_link {

•   struct drm_bridge bridge;
  

•   struct drm_bridge *next_bridge;
  

•   struct device *dev;
  

•   struct imx_sc_ipc *ipc_handle;
  

•   int id;
  

•   int stream_id;
  

•   int dc_id;
  

•   u32 sink_rsc;
  

•   u32 mst_addr;
  

•   u8 mst_addr_ctrl;
  

•   u8 mst_en_ctrl;
  

•   u8 mst_vld_ctrl;
  

•   u8 sync_ctrl;
  

+};

+static void imx8qxp_pixel_link_enable_mst_en(struct imx8qxp_pixel_link *pl) +{

•   int ret;
  

•   ret = imx_sc_misc_set_control(pl->ipc_handle, pl->sink_rsc,
  

•                                 pl->mst_en_ctrl, true);
  

•   if (ret)
  

•           DRM_DEV_ERROR(pl->dev,
  

•                   "failed to enable DC%d stream%d pixel link mst_en: %d\n",
  

•                                           pl->dc_id, pl->stream_id, ret);
  

+}

+static void imx8qxp_pixel_link_enable_mst_vld(struct imx8qxp_pixel_link *pl) +{

•   int ret;
  

•   ret = imx_sc_misc_set_control(pl->ipc_handle, pl->sink_rsc,
  

•                                 pl->mst_vld_ctrl, true);
  

•   if (ret)
  

•           DRM_DEV_ERROR(pl->dev,
  

•           "failed to enable DC%d stream%d pixel link mst_vld: %d\n",
  

•                                           pl->dc_id, pl->stream_id, ret);
  

+}

+static void imx8qxp_pixel_link_enable_sync(struct imx8qxp_pixel_link *pl) +{

•   int ret;
  

•   ret = imx_sc_misc_set_control(pl->ipc_handle, pl->sink_rsc,
  

•                                 pl->sync_ctrl, true);
  

•   if (ret)
  

•           DRM_DEV_ERROR(pl->dev,
  

•                   "failed to enable DC%d stream%d pixel link sync: %d\n",
  

•                                           pl->dc_id, pl->stream_id, ret);
  

+}

+static int imx8qxp_pixel_link_disable_mst_en(struct imx8qxp_pixel_link *pl) +{

•   int ret;
  

•   ret = imx_sc_misc_set_control(pl->ipc_handle, pl->sink_rsc,
  

•                                 pl->mst_en_ctrl, false);
  

•   if (ret)
  

•           DRM_DEV_ERROR(pl->dev,
  

•           "failed to disable DC%d stream%d pixel link mst_en: %d\n",
  

•                                           pl->dc_id, pl->stream_id, ret);
  

•   return ret;
  

+}

+static int imx8qxp_pixel_link_disable_mst_vld(struct imx8qxp_pixel_link *pl) +{

•   int ret;
  

•   ret = imx_sc_misc_set_control(pl->ipc_handle, pl->sink_rsc,
  

•                                 pl->mst_vld_ctrl, false);
  

•   if (ret)
  

•           DRM_DEV_ERROR(pl->dev,
  

•           "failed to disable DC%d stream%d pixel link mst_vld: %d\n",
  

•                                           pl->dc_id, pl->stream_id, ret);
  

•   return ret;
  

+}

+static int imx8qxp_pixel_link_disable_sync(struct imx8qxp_pixel_link *pl) +{

•   int ret;
  

•   ret = imx_sc_misc_set_control(pl->ipc_handle, pl->sink_rsc,
  

•                                 pl->sync_ctrl, false);
  

•   if (ret)
  

•           DRM_DEV_ERROR(pl->dev,
  

•                   "failed to disable DC%d stream%d pixel link sync: %d\n",
  

•                                           pl->dc_id, pl->stream_id, ret);
  

•   return ret;
  

+}

+static void imx8qxp_pixel_link_set_mst_addr(struct imx8qxp_pixel_link *pl) +{

•   int ret;
  

•   ret = imx_sc_misc_set_control(pl->ipc_handle,
  

•                                 pl->sink_rsc, pl->mst_addr_ctrl,
  

•                                 pl->mst_addr);
  

•   if (ret)
  

•           DRM_DEV_ERROR(pl->dev,
  

•               "failed to set DC%d stream%d pixel link mst addr(%u): %d\n",
  

•               pl->dc_id, pl->stream_id, pl->mst_addr, ret);
  

+}

+static int imx8qxp_pixel_link_bridge_attach(struct drm_bridge *bridge,

•                                       enum drm_bridge_attach_flags flags)
  

+{

•   struct imx8qxp_pixel_link *pl = bridge->driver_private;
  

•   if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)) {
  

•           DRM_DEV_ERROR(pl->dev,
  

•                         "do not support creating a drm_connector\n");
  

•           return -EINVAL;
  

•   }
  

•   if (!bridge->encoder) {
  

•           DRM_DEV_ERROR(pl->dev, "missing encoder\n");
  

•           return -ENODEV;
  

•   }
  

•   return drm_bridge_attach(bridge->encoder,
  

•                            pl->next_bridge, bridge,
  

•                            DRM_BRIDGE_ATTACH_NO_CONNECTOR);
  

+}

+static void +imx8qxp_pixel_link_bridge_mode_set(struct drm_bridge *bridge,

•                              const struct drm_display_mode *mode,
  

•                              const struct drm_display_mode *adjusted_mode)
  

+{

•   struct imx8qxp_pixel_link *pl = bridge->driver_private;
  

•   imx8qxp_pixel_link_set_mst_addr(pl);
  

+}

+static void imx8qxp_pixel_link_bridge_atomic_enable(struct drm_bridge *bridge,

•                         struct drm_bridge_state *old_bridge_state)
  

+{

•   struct imx8qxp_pixel_link *pl = bridge->driver_private;
  

•   imx8qxp_pixel_link_enable_mst_en(pl);
  

•   imx8qxp_pixel_link_enable_mst_vld(pl);
  

•   imx8qxp_pixel_link_enable_sync(pl);
  

+}

+static void imx8qxp_pixel_link_bridge_atomic_disable(struct drm_bridge *bridge,

•                           struct drm_bridge_state *old_bridge_state)
  

+{

•   struct imx8qxp_pixel_link *pl = bridge->driver_private;
  

•   imx8qxp_pixel_link_disable_mst_en(pl);
  

•   imx8qxp_pixel_link_disable_mst_vld(pl);
  

•   imx8qxp_pixel_link_disable_sync(pl);
  

+}

+static const u32 imx8qxp_pixel_link_bus_output_fmts[] = {

•   MEDIA_BUS_FMT_RGB888_1X36_CPADLO,
  

•   MEDIA_BUS_FMT_RGB666_1X36_CPADLO,
  

+};

+static bool imx8qxp_pixel_link_bus_output_fmt_supported(u32 fmt) +{

•   int i;
  

•   for (i = 0; i < ARRAY_SIZE(imx8qxp_pixel_link_bus_output_fmts); i++) {
  

•           if (imx8qxp_pixel_link_bus_output_fmts[i] == fmt)
  

•                   return true;
  

•   }
  

•   return false;
  

+}

+static u32 * +imx8qxp_pixel_link_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge,

•                                   struct drm_bridge_state *bridge_state,
  

•                                   struct drm_crtc_state *crtc_state,
  

•                                   struct drm_connector_state *conn_state,
  

•                                   u32 output_fmt,
  

•                                   unsigned int *num_input_fmts)
  

+{

•   u32 *input_fmts;
  

•   if (!imx8qxp_pixel_link_bus_output_fmt_supported(output_fmt))
  

•           return NULL;
  

•   *num_input_fmts = 1;
  

•   input_fmts = kmalloc(sizeof(*input_fmts), GFP_KERNEL);
  

•   if (!input_fmts)
  

•           return NULL;
  

•   input_fmts[0] = output_fmt;
  

•   return input_fmts;
  

+}

+static u32 * +imx8qxp_pixel_link_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge,

•                                   struct drm_bridge_state *bridge_state,
  

•                                   struct drm_crtc_state *crtc_state,
  

•                                   struct drm_connector_state *conn_state,
  

•                                   unsigned int *num_output_fmts)
  

+{

•   *num_output_fmts = ARRAY_SIZE(imx8qxp_pixel_link_bus_output_fmts);
  

•   return kmemdup(imx8qxp_pixel_link_bus_output_fmts,
  

•                   sizeof(imx8qxp_pixel_link_bus_output_fmts), GFP_KERNEL);
  

+}

+static const struct drm_bridge_funcs imx8qxp_pixel_link_bridge_funcs = {

•   .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,
  

•   .attach                 = imx8qxp_pixel_link_bridge_attach,
  

•   .mode_set               = imx8qxp_pixel_link_bridge_mode_set,
  

•   .atomic_enable          = imx8qxp_pixel_link_bridge_atomic_enable,
  

•   .atomic_disable         = imx8qxp_pixel_link_bridge_atomic_disable,
  

•   .atomic_get_input_bus_fmts =
  

•                   imx8qxp_pixel_link_bridge_atomic_get_input_bus_fmts,
  

•   .atomic_get_output_bus_fmts =
  

•                   imx8qxp_pixel_link_bridge_atomic_get_output_bus_fmts,
  

+};

+static int imx8qxp_pixel_link_disable_all_controls(struct imx8qxp_pixel_link *pl) +{

•   int ret;
  

•   ret = imx8qxp_pixel_link_disable_mst_en(pl);
  

•   if (ret)
  

•           return ret;
  

•   ret = imx8qxp_pixel_link_disable_mst_vld(pl);
  

•   if (ret)
  

•           return ret;
  

•   return imx8qxp_pixel_link_disable_sync(pl);
  

+}

+static struct drm_bridge * +imx8qxp_pixel_link_find_next_bridge(struct imx8qxp_pixel_link *pl) +{

•   struct device_node *np = pl->dev->of_node;
  

•   struct device_node *port, *remote;
  

•   struct drm_bridge *next_bridge[PL_MAX_NEXT_BRIDGES];
  

•   u32 port_id;
  

•   bool found_port = false;
  

•   int reg, ep_cnt = 0;
  

•   int bridge_sel = 0;     /* select the first next bridge by default */
  

Is this comment on the wrong line?

I'll make this comment occupy a line just prior to this line in the next version.

Thanks, Liu Ying

...

•   for (port_id = 1; port_id <= PL_MAX_MST_ADDR + 1; port_id++) {
  

•           port = of_graph_get_port_by_id(np, port_id);
  

•           if (!port)
  

•                   continue;
  

•           if (of_device_is_available(port)) {
  

•                   found_port = true;
  

•                   of_node_put(port);
  

•                   break;
  

•           }
  

•           of_node_put(port);
  

•   }
  

•   if (!found_port) {
  

•           DRM_DEV_ERROR(pl->dev, "no available output port\n");
  

•           return ERR_PTR(-ENODEV);
  

•   }
  

•   for (reg = 0; reg < PL_MAX_NEXT_BRIDGES; reg++) {
  

•           remote = of_graph_get_remote_node(np, port_id, reg);
  

•           if (!remote)
  

•                   continue;
  

•           if (!of_device_is_available(remote->parent)) {
  

•                   DRM_DEV_DEBUG(pl->dev,
  

•                      "port%u endpoint%u remote parent is not available\n",
  

•                                                           port_id, reg);
  

•                   of_node_put(remote);
  

•                   continue;
  

•           }
  

•           next_bridge[ep_cnt] = of_drm_find_bridge(remote);
  

•           if (!next_bridge[ep_cnt]) {
  

•                   of_node_put(remote);
  

•                   return ERR_PTR(-EPROBE_DEFER);
  

•           }
  

•           /* specially select the next bridge with companion PXL2DPI */
  

•           if (of_find_property(remote, "fsl,companion-pxl2dpi", NULL))
  

•                   bridge_sel = ep_cnt;
  

•           ep_cnt++;
  

•           of_node_put(remote);
  

•   }
  

•   pl->mst_addr = port_id - 1;
  

•   return next_bridge[bridge_sel];
  

+}

+static int imx8qxp_pixel_link_bridge_probe(struct platform_device *pdev) +{

•   struct imx8qxp_pixel_link *pl;
  

•   struct device *dev = &pdev->dev;
  

•   struct device_node *np = dev->of_node;
  

•   int ret;
  

•   pl = devm_kzalloc(dev, sizeof(*pl), GFP_KERNEL);
  

•   if (!pl)
  

•           return -ENOMEM;
  

•   ret = imx_scu_get_handle(&pl->ipc_handle);
  

•   if (ret) {
  

•           if (ret != -EPROBE_DEFER)
  

•                   DRM_DEV_ERROR(dev, "failed to get SCU ipc handle: %d\n",
  

•                                                                   ret);
  

•           return ret;
  

•   }
  

•   pl->id = of_alias_get_id(np, "dc_pl");
  

•   if (pl->id < 0) {
  

•           DRM_DEV_ERROR(dev,
  

•                         "failed to get pixel link node alias id: %d\n",
  

•                                                           pl->id);
  

•           return pl->id;
  

•   }
  

•   pl->dev = dev;
  

•   pl->dc_id = pl->id / 2;
  

•   pl->stream_id = pl->id % 2;
  

•   pl->sink_rsc = pl->dc_id ? IMX_SC_R_DC_1 : IMX_SC_R_DC_0;
  

•   if (pl->stream_id == 0) {
  

•           pl->mst_addr_ctrl = IMX_SC_C_PXL_LINK_MST1_ADDR;
  

•           pl->mst_en_ctrl   = IMX_SC_C_PXL_LINK_MST1_ENB;
  

•           pl->mst_vld_ctrl  = IMX_SC_C_PXL_LINK_MST1_VLD;
  

•           pl->sync_ctrl     = IMX_SC_C_SYNC_CTRL0;
  

•   } else {
  

•           pl->mst_addr_ctrl = IMX_SC_C_PXL_LINK_MST2_ADDR;
  

•           pl->mst_en_ctrl   = IMX_SC_C_PXL_LINK_MST2_ENB;
  

•           pl->mst_vld_ctrl  = IMX_SC_C_PXL_LINK_MST2_VLD;
  

•           pl->sync_ctrl     = IMX_SC_C_SYNC_CTRL1;
  

•   }
  

•   /* disable all controls to POR default */
  

•   ret = imx8qxp_pixel_link_disable_all_controls(pl);
  

•   if (ret)
  

•           return ret;
  

•   pl->next_bridge = imx8qxp_pixel_link_find_next_bridge(pl);
  

•   if (IS_ERR(pl->next_bridge)) {
  

•           ret = PTR_ERR(pl->next_bridge);
  

•           if (ret != -EPROBE_DEFER)
  

•                   DRM_DEV_ERROR(dev, "failed to find next bridge: %d\n",
  

•                                                                   ret);
  

•           return ret;
  

•   }
  

•   platform_set_drvdata(pdev, pl);
  

•   pl->bridge.driver_private = pl;
  

•   pl->bridge.funcs = &imx8qxp_pixel_link_bridge_funcs;
  

•   pl->bridge.of_node = np;
  

•   drm_bridge_add(&pl->bridge);
  

•   return ret;
  

+}

+static int imx8qxp_pixel_link_bridge_remove(struct platform_device *pdev) +{

•   struct imx8qxp_pixel_link *pl = platform_get_drvdata(pdev);
  

•   drm_bridge_remove(&pl->bridge);
  

•   return 0;
  

+}

+static const struct of_device_id imx8qxp_pixel_link_dt_ids[] = {

•   { .compatible = "fsl,imx8qm-dc-pixel-link", },
  

•   { .compatible = "fsl,imx8qxp-dc-pixel-link", },
  

•   { /* sentinel */ }
  

+}; +MODULE_DEVICE_TABLE(of, imx8qxp_pixel_link_dt_ids);

+static struct platform_driver imx8qxp_pixel_link_bridge_driver = {

•   .probe  = imx8qxp_pixel_link_bridge_probe,
  

•   .remove = imx8qxp_pixel_link_bridge_remove,
  

•   .driver = {
  

•           .of_match_table = imx8qxp_pixel_link_dt_ids,
  

•           .name = DRIVER_NAME,
  

•   },
  

+}; +module_platform_driver(imx8qxp_pixel_link_bridge_driver);

+MODULE_DESCRIPTION("i.MX8QXP/QM display pixel link bridge driver"); +MODULE_AUTHOR("Liu Ying victor.liu@nxp.com"); +MODULE_LICENSE("GPL v2");

+MODULE_ALIAS("platform:" DRIVER_NAME);

2.7.4