Re: [PATCH v8 4/4] drm/tidss: Add OLDI bridge support

["Michael Walle" <mwalle@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2789 bytes --]

Hi Aradhya,

> +static int get_oldi_mode(struct device_node *oldi_tx, int *companion_instance)
> +{
> +	struct device_node *companion;
> +	struct device_node *port0, *port1;
> +	u32 companion_reg;
> +	bool secondary_oldi = false;
> +	int pixel_order;
> +
> +	/*
> +	 * Find if the OLDI is paired with another OLDI for combined OLDI
> +	 * operation (dual-link or clone).
> +	 */
> +	companion = of_parse_phandle(oldi_tx, "ti,companion-oldi", 0);
> +	if (!companion)
> +		/*
> +		 * The OLDI TX does not have a companion, nor is it a
> +		 * secondary OLDI. It will operate independently.
> +		 */
> +		return OLDI_MODE_SINGLE_LINK;

How is this supposed to work? If I read this code correctly, the
second (companion) port is always reported as SINGLE_LINK if its
device tree node doesn't have a ti,companion-oldi property. But
reading the device tree binding, the companion-old property is only
for the first OLDI port.

FWIW, I've tested this series and I get twice the clock rate as
expected and the second link is reported as "OLDI_MODE_SINGLE_LINK".
I'll dig deeper into this tomorrow.

-michael

> +
> +	if (of_property_read_u32(companion, "reg", &companion_reg))
> +		return OLDI_MODE_UNSUPPORTED;
> +
> +	if (companion_reg > (TIDSS_MAX_OLDI_TXES - 1))
> +		/* Invalid companion OLDI reg value. */
> +		return OLDI_MODE_UNSUPPORTED;
> +
> +	*companion_instance = (int)companion_reg;
> +
> +	if (of_property_read_bool(oldi_tx, "ti,secondary-oldi"))
> +		secondary_oldi = true;
> +
> +	/*
> +	 * We need to work out if the sink is expecting us to function in
> +	 * dual-link mode. We do this by looking at the DT port nodes, the
> +	 * OLDI TX ports are connected to. If they are marked as expecting
> +	 * even pixels and odd pixels, then we need to enable dual-link.
> +	 */
> +	port0 = of_graph_get_port_by_id(oldi_tx, 1);
> +	port1 = of_graph_get_port_by_id(companion, 1);
> +	pixel_order = drm_of_lvds_get_dual_link_pixel_order(port0, port1);
> +	of_node_put(port0);
> +	of_node_put(port1);
> +	of_node_put(companion);
> +
> +	switch (pixel_order) {
> +	case -EINVAL:
> +		/*
> +		 * The dual-link properties were not found in at least
> +		 * one of the sink nodes. Since 2 OLDI ports are present
> +		 * in the DT, it can be safely assumed that the required
> +		 * configuration is Clone Mode.
> +		 */
> +		return (secondary_oldi ? OLDI_MODE_CLONE_SECONDARY_SINGLE_LINK :
> +					 OLDI_MODE_CLONE_SINGLE_LINK);
> +
> +	case DRM_LVDS_DUAL_LINK_ODD_EVEN_PIXELS:
> +		return (secondary_oldi ? OLDI_MODE_SECONDARY_DUAL_LINK :
> +					 OLDI_MODE_DUAL_LINK);
> +
> +	/* Unsupported OLDI Modes */
> +	case DRM_LVDS_DUAL_LINK_EVEN_ODD_PIXELS:
> +	default:
> +		return OLDI_MODE_UNSUPPORTED;
> +	}
> +}

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]