Re: [PATCH v2 6/9] drm/bridge: Add ITE IT6263 LVDS to HDMI converter

[Liu Ying <victor.liu@nxp.com>]

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

 * 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