On Tue, 2021-03-30 at 11:54 +0200, Robert Foss wrote:
Hey Liu,
checkpatch --strict lists some nit and a warning. With those fixed feel free to add my r-b.
On Wed, 17 Mar 2021 at 04:57, Liu Ying victor.liu@nxp.com wrote:
This patch adds a drm bridge driver for i.MX8qxp LVDS display bridge(LDB) which is officially named as pixel mapper. The LDB has two channels. Each of them supports up to 24bpp parallel input color format and can map the input to VESA or JEIDA standards. The two channels cannot be used simultaneously, that is to say, the user should pick one of them to use. Two LDB channels from two LDB instances can work together in LDB split mode to support a dual link LVDS display. The channel indexes have to be different. Channel0 outputs odd pixels and channel1 outputs even pixels. This patch supports the LDB single mode and split mode.
Signed-off-by: Liu Ying victor.liu@nxp.com
Note that this patch depends on the patch 'phy: Add LVDS configuration options', which has already been sent with the following series to add Mixel combo PHY found in i.MX8qxp: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.spinic...
v5->v6:
- No change.
v4->v5:
- Link with the imx-ldb-helper object. (Robert)
- Correspondingly, rename 'imx8qxp-ldb.c' to 'imx8qxp-ldb-drv.c'.
v3->v4:
- No change.
v2->v3:
- No change.
v1->v2:
- Drop unnecessary DT validation.
- Use of_graph_get_endpoint_by_regs() and of_graph_get_remote_endpoint() to get the input remote endpoint in imx8qxp_ldb_set_di_id().
- Avoid using companion_port OF node after putting it in imx8qxp_ldb_parse_dt_companion().
- Mention i.MX8qxp LDB official name 'pixel mapper' in the bridge driver and Kconfig help message.
drivers/gpu/drm/bridge/imx/Kconfig | 9 + drivers/gpu/drm/bridge/imx/Makefile | 3 + drivers/gpu/drm/bridge/imx/imx8qxp-ldb-drv.c | 720 +++++++++++++++++++++++++++ 3 files changed, 732 insertions(+) create mode 100644 drivers/gpu/drm/bridge/imx/imx8qxp-ldb-drv.c
[...]
+static int imx8qxp_ldb_probe(struct platform_device *pdev) +{
struct device *dev = &pdev->dev;
struct imx8qxp_ldb *imx8qxp_ldb;
struct imx8qxp_ldb_channel *imx8qxp_ldb_ch;
struct ldb *ldb;
struct ldb_channel *ldb_ch;
int ret, i;
imx8qxp_ldb = devm_kzalloc(dev, sizeof(*imx8qxp_ldb), GFP_KERNEL);
if (!imx8qxp_ldb)
return -ENOMEM;
imx8qxp_ldb->clk_pixel = devm_clk_get(dev, "pixel");
if (IS_ERR(imx8qxp_ldb->clk_pixel)) {
ret = PTR_ERR(imx8qxp_ldb->clk_pixel);
if (ret != -EPROBE_DEFER)
DRM_DEV_ERROR(dev,
"failed to get pixel clock: %d\n", ret);
return ret;
}
imx8qxp_ldb->clk_bypass = devm_clk_get(dev, "bypass");
if (IS_ERR(imx8qxp_ldb->clk_bypass)) {
ret = PTR_ERR(imx8qxp_ldb->clk_bypass);
if (ret != -EPROBE_DEFER)
DRM_DEV_ERROR(dev,
"failed to get bypass clock: %d\n", ret);
return ret;
}
imx8qxp_ldb->dev = dev;
ldb = &imx8qxp_ldb->base;
ldb->dev = dev;
ldb->ctrl_reg = 0xe0;
for (i = 0; i < MAX_LDB_CHAN_NUM; i++)
ldb->channel[i] = &imx8qxp_ldb->channel[i].base;
ret = ldb_init_helper(ldb);
if (ret)
return ret;
if (ldb->available_ch_cnt == 0) {
DRM_DEV_DEBUG_DRIVER(dev, "no available channel\n");
return 0;
} else if (ldb->available_ch_cnt > 1) {
DRM_DEV_ERROR(dev, "invalid available channel number(%u)\n",
ldb->available_ch_cnt);
return -ENOTSUPP;
}
WARNING: ENOTSUPP is not a SUSV4 error code, prefer EOPNOTSUPP #683: FILE: drivers/gpu/drm/bridge/imx/imx8qxp-ldb-drv.c:625:
return -ENOTSUPP;
Maybe -EINVAL is a better return value.
Will use -EINVAL in the next version.
Liu Ying