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

Dmitry Baryshkov dmitry.baryshkov at linaro.org
Mon Oct 14 11:22:04 UTC 2024


On Mon, Oct 14, 2024 at 04:28:29PM +0800, Liu Ying wrote:
> 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 at 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);                                           
>         }                                                                        

I think getting remotes just to get remotes back is a little bit clumsy.
Adding drm_of_lvds_get_dual_link_pixel_order_sink() looks like a claner
solution.

>                                                                                  
>         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;                             
>                 }                                                                

	it->lvds_dual_link = true;
	order = drm_of_lvds_get_dual_link_pixel_order_sink(port0_ep,
							   port1_ep);
	if (order < 0) {
		dev_err(...);
		ret = order;
	} else if (order == DRM_LVDS_DUAL_LINK_EVEN_ODD_PIXELS) {
		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
> 

-- 
With best wishes
Dmitry


More information about the dri-devel mailing list