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....@nxp.com>
>>>> ---
>>>> 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
