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@xxxxxxx> 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@xxxxxxx> > > --- > > 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.spinics.net%2Flists%2Farm-kernel%2Fmsg879957.html&data=04%7C01%7Cvictor.liu%40nxp.com%7C7c4f1092633c4687fb3108d8f361ddfd%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637526948949839014%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=iNr6RU0xrhcW8epwIPPJDpayjWxfmWsSjCju3XIcmPM%3D&reserved=0 > > > > 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
