On 10/14/2024, Dmitry Baryshkov wrote: > On Mon, Oct 14, 2024 at 03:18:15PM +0800, Liu Ying wrote: >> On 10/14/2024, Dmitry Baryshkov wrote: >>> On Sat, Oct 12, 2024 at 03:35:40PM +0800, Liu Ying wrote: >>>> Add basic HDMI video output support. Currently, only RGB888 output >>>> pixel format is supported. At the LVDS input side, the driver >>>> supports single LVDS link and dual LVDS links with "jeida-24" LVDS >>>> mapping. >>>> >>>> Product link: >>>> https://www.ite.com.tw/en/product/cate1/IT6263 >>>> >>>> Signed-off-by: Liu Ying <victor.liu@xxxxxxx> >>>> --- >>>> v2: >>>> * Add AVI inforframe support. (Maxime) >>>> * Add DRM_MODE_CONNECTOR_HDMIA. (Biju) >>>> * Rename it6263_reset() to it6263_hw_reset(). (Biju) >>>> * Check number of LVDS link data lanes. (Biju) >>>> >>>> drivers/gpu/drm/bridge/Kconfig | 8 + >>>> drivers/gpu/drm/bridge/Makefile | 1 + >>>> drivers/gpu/drm/bridge/ite-it6263.c | 919 ++++++++++++++++++++++++++++ >>>> 3 files changed, 928 insertions(+) >>>> create mode 100644 drivers/gpu/drm/bridge/ite-it6263.c >>>> >>> >>> [...] >>> >>>> +static int it6263_parse_dt(struct it6263 *it) >>>> +{ >>>> + struct device *dev = it->dev; >>>> + struct device_node *port0, *port1; >>>> + int ret; >>>> + >>>> + ret = of_property_read_u8(dev->of_node, "ite,lvds-link-num-data-lanes", >>>> + &it->lvds_link_num_dlanes); >>>> + if (ret) { >>>> + dev_err(dev, "failed to get LVDS link number of data lanes: %d\n", >>>> + ret); >>>> + return ret; >>>> + } >>>> + >>>> + it->next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 2, 0); >>>> + if (IS_ERR(it->next_bridge)) >>>> + return dev_err_probe(dev, PTR_ERR(it->next_bridge), >>>> + "failed to get next bridge\n"); >>>> + >>>> + port0 = of_graph_get_port_by_id(dev->of_node, 0); >>>> + port1 = of_graph_get_port_by_id(dev->of_node, 1); >>>> + if (port0 && port1) { >>>> + if (of_property_read_bool(port0, "dual-lvds-even-pixels") && >>>> + of_property_read_bool(port1, "dual-lvds-odd-pixels")) { >>>> + it->lvds_dual_link = true; >>>> + it->lvds_link12_swap = true; >>>> + } else if (of_property_read_bool(port0, "dual-lvds-odd-pixels") && >>>> + of_property_read_bool(port1, "dual-lvds-even-pixels")) { >>>> + it->lvds_dual_link = true; >>>> + } >>>> + >>>> + if (!it->lvds_dual_link) { >>>> + dev_err(dev, >>>> + "failed to get LVDS dual link pixel order\n"); >>>> + ret = -EINVAL; >>>> + } >>> >>> Please use drm_of_lvds_get_dual_link_pixel_order(), it validates that >>> the DT definition is sound: one port for odd pixels, one port for even >>> pixels. >> >> It cannot be used, because it get the pixel order for the LVDS >> source not sink. IT6263 is the LVDS sink. > > Then you need a similar function for the sink side. Add it to the > drm_of.c How about getting remote LVDS source ports first and use drm_of_lvds_get_dual_link_pixel_order() like the snippet below? This way, no need to add a similar function or modify drm_of_lvds_get_dual_link_pixel_order() implementation. If you don't like this, can you please suggest a similar function name or maybe an additional parameter(with type and name) for drm_of_lvds_get_dual_link_pixel_order()? ---8<--- port0_ep = of_graph_get_endpoint_by_regs(dev->of_node, 0, -1); if (port0_ep) { remote_port0 = of_graph_get_remote_port(port0_ep); of_node_put(port0_ep); } port1_ep = of_graph_get_endpoint_by_regs(dev->of_node, 1, -1); if (port1_ep) { remote_port1 = of_graph_get_remote_port(port1_ep); of_node_put(port1_ep); } if (remote_port0 && remote_port1) { order = drm_of_lvds_get_dual_link_pixel_order(remote_port0, remote_port1); if (order < 0) { dev_err(dev, "failed to get dual link pixel order: %d\n", order); ret = order; } else if (order == DRM_LVDS_DUAL_LINK_ODD_EVEN_PIXELS) { it->lvds_dual_link = true; } else if (order == DRM_LVDS_DUAL_LINK_EVEN_ODD_PIXELS) { it->lvds_dual_link = true; it->lvds_link12_swap = true; } } else if (remote_port1) { ret = -EINVAL; dev_err(dev, "single input LVDS port1 is not supported\n"); } else if (!remote_port0) { ret = -EINVAL; dev_err(dev, "no input LVDS port\n"); } of_node_put(remote_port0); of_node_put(remote_port1); ---8<--- > >> >> * drm_of_lvds_get_dual_link_pixel_order - Get LVDS dual-link pixel order >> * @port1: First DT port node of the Dual-link LVDS source >> * @port2: Second DT port node of the Dual-link LVDS source >> >>> >>>> + } else if (port1) { >>>> + ret = -EINVAL; >>>> + dev_err(dev, "single input LVDS port1 is not supported\n"); >>>> + } else if (!port0) { >>>> + ret = -EINVAL; >>>> + dev_err(dev, "no input LVDS port\n"); >>>> + } >>>> + >>>> + of_node_put(port0); >>>> + of_node_put(port1); >>>> + >>>> + return ret; >>>> +} >>>> + >>> >> >> -- >> Regards, >> Liu Ying >> > -- Regards, Liu Ying
