Re: [PATCH v6 10/14] drm/bridge: imx: Add LDB driver helper support

2021-03-30 Thread Liu Ying
Hi Robert,

On Tue, 2021-03-30 at 11:46 +0200, Robert Foss wrote:
> Hey Liu,
> 
> checkpatch --strict is listing some nits for this patch, with those
> fixed feel free to add my r-b.
> 
> Reviewed-by: Robert Foss 

Thanks for your review.

Will fix those nits in the next version.

Liu Ying



Re: [PATCH v6 10/14] drm/bridge: imx: Add LDB driver helper support

2021-03-30 Thread Robert Foss
Hey Liu,

checkpatch --strict is listing some nits for this patch, with those
fixed feel free to add my r-b.

Reviewed-by: Robert Foss 

On Wed, 17 Mar 2021 at 04:57, Liu Ying  wrote:
>
> This patch adds a helper to support LDB drm bridge drivers for
> i.MX SoCs.  Helper functions supported by this helper should
> implement common logics for all LDB modules embedded in i.MX SoCs.
>
> Signed-off-by: Liu Ying 
> ---
> v5->v6:
> * No change.
>
> v4->v5:
> * Make imx-ldb-helper be a pure object to be linked with i.MX8qxp LDB bridge
>   driver and i.MX8qm LDB bridge driver. (Robert)
> * Move 'imx_ldb_helper.h' to 'drivers/gpu/drm/bridge/imx/imx-ldb-helper.h'.
>   (Robert)
> * s/__FSL_IMX_LDB__/__IMX_LDB_HELPER__/  for 'imx-ldb-helper.h'.
>
> v3->v4:
> * No change.
>
> v2->v3:
> * Call syscon_node_to_regmap() to get regmap instead of
>   syscon_regmap_lookup_by_phandle().
>
> v1->v2:
> * No change.
>
>  drivers/gpu/drm/bridge/imx/imx-ldb-helper.c | 232 
> 
>  drivers/gpu/drm/bridge/imx/imx-ldb-helper.h |  98 
>  2 files changed, 330 insertions(+)
>  create mode 100644 drivers/gpu/drm/bridge/imx/imx-ldb-helper.c
>  create mode 100644 drivers/gpu/drm/bridge/imx/imx-ldb-helper.h
>
> diff --git a/drivers/gpu/drm/bridge/imx/imx-ldb-helper.c 
> b/drivers/gpu/drm/bridge/imx/imx-ldb-helper.c
> new file mode 100644
> index ..d01c4ff9
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/imx/imx-ldb-helper.c
> @@ -0,0 +1,232 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2012 Sascha Hauer, Pengutronix
> + * Copyright 2019,2020 NXP
> + */
> +
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +
> +#include "imx-ldb-helper.h"
> +
> +bool ldb_channel_is_single_link(struct ldb_channel *ldb_ch)
> +{
> +   return ldb_ch->link_type == LDB_CH_SINGLE_LINK;
> +}
> +
> +bool ldb_channel_is_split_link(struct ldb_channel *ldb_ch)
> +{
> +   return ldb_ch->link_type == LDB_CH_DUAL_LINK_EVEN_ODD_PIXELS ||
> +  ldb_ch->link_type == LDB_CH_DUAL_LINK_ODD_EVEN_PIXELS;
> +}
> +
> +int ldb_bridge_atomic_check_helper(struct drm_bridge *bridge,
> +  struct drm_bridge_state *bridge_state,
> +  struct drm_crtc_state *crtc_state,
> +  struct drm_connector_state *conn_state)
> +{
> +   struct ldb_channel *ldb_ch = bridge->driver_private;
> +
> +   ldb_ch->in_bus_format = bridge_state->input_bus_cfg.format;
> +   ldb_ch->out_bus_format = bridge_state->output_bus_cfg.format;
> +
> +   return 0;
> +}
> +
> +void ldb_bridge_mode_set_helper(struct drm_bridge *bridge,
> +   const struct drm_display_mode *mode,
> +   const struct drm_display_mode *adjusted_mode)
> +{
> +   struct ldb_channel *ldb_ch = bridge->driver_private;
> +   struct ldb *ldb = ldb_ch->ldb;
> +   bool is_split = ldb_channel_is_split_link(ldb_ch);
> +
> +   if (is_split)
> +   ldb->ldb_ctrl |= LDB_SPLIT_MODE_EN;
> +
> +   switch (ldb_ch->out_bus_format) {
> +   case MEDIA_BUS_FMT_RGB666_1X7X3_SPWG:
> +   break;
> +   case MEDIA_BUS_FMT_RGB888_1X7X4_SPWG:
> +   if (ldb_ch->chno == 0 || is_split)
> +   ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH0_24;
> +   if (ldb_ch->chno == 1 || is_split)
> +   ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH1_24;
> +   break;
> +   case MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA:
> +   if (ldb_ch->chno == 0 || is_split)
> +   ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH0_24 |
> +LDB_BIT_MAP_CH0_JEIDA;
> +   if (ldb_ch->chno == 1 || is_split)
> +   ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH1_24 |
> +LDB_BIT_MAP_CH1_JEIDA;
> +   break;
> +   }
> +}
> +
> +void ldb_bridge_enable_helper(struct drm_bridge *bridge)
> +{
> +   struct ldb_channel *ldb_ch = bridge->driver_private;
> +   struct ldb *ldb = ldb_ch->ldb;
> +
> +   /*
> +* Platform specific bridge drivers should set ldb_ctrl properly
> +* for the enablement, so just write the ctrl_reg here.
> +*/
> +   regmap_write(ldb->regmap, ldb->ctrl_reg, ldb->ldb_ctrl);
> +}
> +
> +void ldb_bridge_disable_helper(struct drm_bridge *bridge)
> +{
> +   struct ldb_channel *ldb_ch = bridge->driver_private;
> +   struct ldb *ldb = ldb_ch->ldb;
> +   bool is_split = ldb_channel_is_split_link(ldb_ch);
> +
> +   if (ldb_ch->chno == 0 || is_split)
> +   ldb->ldb_ctrl &= ~LDB_CH0_MODE_EN_MASK;
> +   if (ldb_ch->chno == 1 || is_split)
> +   ldb->ldb_ctrl &= ~LDB_CH1_MODE_EN_MASK;
> +
> +   regmap_write(ldb->regmap, ldb->ctrl_reg, ldb->ldb_ctrl);
> +}
> +
> +int ldb_bridge_attach_helper(struct drm_bridge *bridge,
> + 

[PATCH v6 10/14] drm/bridge: imx: Add LDB driver helper support

2021-03-16 Thread Liu Ying
This patch adds a helper to support LDB drm bridge drivers for
i.MX SoCs.  Helper functions supported by this helper should
implement common logics for all LDB modules embedded in i.MX SoCs.

Signed-off-by: Liu Ying 
---
v5->v6:
* No change.

v4->v5:
* Make imx-ldb-helper be a pure object to be linked with i.MX8qxp LDB bridge
  driver and i.MX8qm LDB bridge driver. (Robert)
* Move 'imx_ldb_helper.h' to 'drivers/gpu/drm/bridge/imx/imx-ldb-helper.h'.
  (Robert)
* s/__FSL_IMX_LDB__/__IMX_LDB_HELPER__/  for 'imx-ldb-helper.h'.

v3->v4:
* No change.

v2->v3:
* Call syscon_node_to_regmap() to get regmap instead of
  syscon_regmap_lookup_by_phandle().

v1->v2:
* No change.

 drivers/gpu/drm/bridge/imx/imx-ldb-helper.c | 232 
 drivers/gpu/drm/bridge/imx/imx-ldb-helper.h |  98 
 2 files changed, 330 insertions(+)
 create mode 100644 drivers/gpu/drm/bridge/imx/imx-ldb-helper.c
 create mode 100644 drivers/gpu/drm/bridge/imx/imx-ldb-helper.h

diff --git a/drivers/gpu/drm/bridge/imx/imx-ldb-helper.c 
b/drivers/gpu/drm/bridge/imx/imx-ldb-helper.c
new file mode 100644
index ..d01c4ff9
--- /dev/null
+++ b/drivers/gpu/drm/bridge/imx/imx-ldb-helper.c
@@ -0,0 +1,232 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2012 Sascha Hauer, Pengutronix
+ * Copyright 2019,2020 NXP
+ */
+
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+#include "imx-ldb-helper.h"
+
+bool ldb_channel_is_single_link(struct ldb_channel *ldb_ch)
+{
+   return ldb_ch->link_type == LDB_CH_SINGLE_LINK;
+}
+
+bool ldb_channel_is_split_link(struct ldb_channel *ldb_ch)
+{
+   return ldb_ch->link_type == LDB_CH_DUAL_LINK_EVEN_ODD_PIXELS ||
+  ldb_ch->link_type == LDB_CH_DUAL_LINK_ODD_EVEN_PIXELS;
+}
+
+int ldb_bridge_atomic_check_helper(struct drm_bridge *bridge,
+  struct drm_bridge_state *bridge_state,
+  struct drm_crtc_state *crtc_state,
+  struct drm_connector_state *conn_state)
+{
+   struct ldb_channel *ldb_ch = bridge->driver_private;
+
+   ldb_ch->in_bus_format = bridge_state->input_bus_cfg.format;
+   ldb_ch->out_bus_format = bridge_state->output_bus_cfg.format;
+
+   return 0;
+}
+
+void ldb_bridge_mode_set_helper(struct drm_bridge *bridge,
+   const struct drm_display_mode *mode,
+   const struct drm_display_mode *adjusted_mode)
+{
+   struct ldb_channel *ldb_ch = bridge->driver_private;
+   struct ldb *ldb = ldb_ch->ldb;
+   bool is_split = ldb_channel_is_split_link(ldb_ch);
+
+   if (is_split)
+   ldb->ldb_ctrl |= LDB_SPLIT_MODE_EN;
+
+   switch (ldb_ch->out_bus_format) {
+   case MEDIA_BUS_FMT_RGB666_1X7X3_SPWG:
+   break;
+   case MEDIA_BUS_FMT_RGB888_1X7X4_SPWG:
+   if (ldb_ch->chno == 0 || is_split)
+   ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH0_24;
+   if (ldb_ch->chno == 1 || is_split)
+   ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH1_24;
+   break;
+   case MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA:
+   if (ldb_ch->chno == 0 || is_split)
+   ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH0_24 |
+LDB_BIT_MAP_CH0_JEIDA;
+   if (ldb_ch->chno == 1 || is_split)
+   ldb->ldb_ctrl |= LDB_DATA_WIDTH_CH1_24 |
+LDB_BIT_MAP_CH1_JEIDA;
+   break;
+   }
+}
+
+void ldb_bridge_enable_helper(struct drm_bridge *bridge)
+{
+   struct ldb_channel *ldb_ch = bridge->driver_private;
+   struct ldb *ldb = ldb_ch->ldb;
+
+   /*
+* Platform specific bridge drivers should set ldb_ctrl properly
+* for the enablement, so just write the ctrl_reg here.
+*/
+   regmap_write(ldb->regmap, ldb->ctrl_reg, ldb->ldb_ctrl);
+}
+
+void ldb_bridge_disable_helper(struct drm_bridge *bridge)
+{
+   struct ldb_channel *ldb_ch = bridge->driver_private;
+   struct ldb *ldb = ldb_ch->ldb;
+   bool is_split = ldb_channel_is_split_link(ldb_ch);
+
+   if (ldb_ch->chno == 0 || is_split)
+   ldb->ldb_ctrl &= ~LDB_CH0_MODE_EN_MASK;
+   if (ldb_ch->chno == 1 || is_split)
+   ldb->ldb_ctrl &= ~LDB_CH1_MODE_EN_MASK;
+
+   regmap_write(ldb->regmap, ldb->ctrl_reg, ldb->ldb_ctrl);
+}
+
+int ldb_bridge_attach_helper(struct drm_bridge *bridge,
+enum drm_bridge_attach_flags flags)
+{
+   struct ldb_channel *ldb_ch = bridge->driver_private;
+   struct ldb *ldb = ldb_ch->ldb;
+
+   if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)) {
+   DRM_DEV_ERROR(ldb->dev,
+ "do not support creating a drm_connector\n");
+   return -EINVAL;
+   }
+
+   if (!bridge->encoder) {
+   DRM_DEV_ERROR(ldb->dev, "miss