Re: [PATCH v6 10/14] drm/bridge: imx: Add LDB driver helper support
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
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
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