lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <91d1e176-f578-466b-991b-2df1728e2f78@nxp.com>
Date: Mon, 14 Oct 2024 16:28:29 +0800
From: Liu Ying <victor.liu@....com>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
Cc: dri-devel@...ts.freedesktop.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, imx@...ts.linux.dev,
 linux-arm-kernel@...ts.infradead.org, andrzej.hajda@...el.com,
 neil.armstrong@...aro.org, rfoss@...nel.org,
 Laurent.pinchart@...asonboard.com, jonas@...boo.se,
 jernej.skrabec@...il.com, airlied@...il.com, simona@...ll.ch,
 maarten.lankhorst@...ux.intel.com, mripard@...nel.org, tzimmermann@...e.de,
 robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
 shawnguo@...nel.org, s.hauer@...gutronix.de, kernel@...gutronix.de,
 festevam@...il.com, catalin.marinas@....com, will@...nel.org,
 quic_bjorande@...cinc.com, geert+renesas@...der.be, arnd@...db.de,
 nfraprado@...labora.com, o.rempel@...gutronix.de, y.moog@...tec.de,
 marex@...x.de, isaac.scott@...asonboard.com, biju.das.jz@...renesas.com
Subject: Re: [PATCH v2 6/9] drm/bridge: Add ITE IT6263 LVDS to HDMI converter

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


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ