On 2/18/22 19:38, Lucas Stach wrote:
[...]
@@ -502,8 +548,10 @@ static int tc_pxl_pll_en(struct tc_data *tc, u32 refclk, u32 pixelclock)
/*
* refclk * mul / (ext_pre_div * pre_div)
* should be in the 150 to 650 MHz range
+ * for (e)DP
*/
- if ((clk > 650000000) || (clk < 150000000))
+ if ((tc->bridge.type != DRM_MODE_CONNECTOR_DPI) &&
+ ((clk > 650000000) || (clk < 150000000)))
continue;
Is there any indication what the bounds are for DPI mode? Can we
replace this with a better check, instead of just disabling it?
Apparently the DPI pixel PLL output is limited to 100 MHz, so yes, this
can be improved.
[...]
+static int tc_dpi_stream_enable(struct tc_data *tc)
+{
+ int ret;
+ u32 value;
+
+ dev_dbg(tc->dev, "enable video stream\n");
+
+ /* Setup PLL */
+ ret = tc_set_syspllparam(tc);
+ if (ret)
+ return ret;
+
+ /* Pixel PLL must always be enabled for DPI mode */
+ ret = tc_pxl_pll_en(tc, clk_get_rate(tc->refclk),
+ 1000 * tc->mode.clock);
+ if (ret)
+ return ret;
+
+ regmap_write(tc->regmap, PPI_D0S_CLRSIPOCOUNT, 3);
+ regmap_write(tc->regmap, PPI_D1S_CLRSIPOCOUNT, 3);
+ regmap_write(tc->regmap, PPI_D2S_CLRSIPOCOUNT, 3);
+ regmap_write(tc->regmap, PPI_D3S_CLRSIPOCOUNT, 3);
Hm, those hardcoded always seem kind of fishy, as AFAIK those
parameters are dependent on land frequency and some other things. But
I'm also not sure if we have all the information available to
dynamically calculate them.
We have multiple copies of the same ^ code in other TC358nnn drivers
too, it seems like some de-duplication could be done too. I have this
feeling the Toshiba bridges are all glued together from two (or more)
halves and there is large potential for de-duplication in all those TC
drivers. But does anyone really have all the chips to test, except for
Toshiba ?
+ regmap_write(tc->regmap, PPI_D0S_ATMR, 0);
+ regmap_write(tc->regmap, PPI_D1S_ATMR, 0);
+ regmap_write(tc->regmap, PPI_TX_RX_TA, TTA_GET | TTA_SURE);
+ regmap_write(tc->regmap, PPI_LPTXTIMECNT, LPX_PERIOD);
+
+ value = ((LANEENABLE_L0EN << tc->dsi_lanes) - LANEENABLE_L0EN) |
+ LANEENABLE_CLEN;
+ regmap_write(tc->regmap, PPI_LANEENABLE, value);
+ regmap_write(tc->regmap, DSI_LANEENABLE, value);
+
+ ret = tc_set_common_video_mode(tc, &tc->mode);
+ if (ret)
+ return ret;
+
+ ret = tc_set_dpi_video_mode(tc, &tc->mode);
+ if (ret)
+ return ret;
+
+ /* Set input interface */
+ value = DP0_AUDSRC_NO_INPUT;
+ if (tc_test_pattern)
+ value |= DP0_VIDSRC_COLOR_BAR;
+ else
+ value |= DP0_VIDSRC_DSI_RX;
+ ret = regmap_write(tc->regmap, SYSCTRL, value);
+ if (ret)
+ return ret;
+
+ msleep(100);
What is that used for? PLL stabilization? Some other purpose?
Yes, except that should've been microseconds ... fixed
[...]
+static int tc_mipi_dsi_host_attach(struct tc_data *tc)
+{
+ struct device *dev = tc->dev;
+ struct device_node *host_node;
+ struct device_node *endpoint;
+ struct mipi_dsi_device *dsi;
+ struct mipi_dsi_host *host;
+ const struct mipi_dsi_device_info info = {
+ .type = "tc358767",
+ .channel = 0,
+ .node = NULL,
+ };
+ int ret;
+
+ endpoint = of_graph_get_endpoint_by_regs(dev->of_node, 0, 0);
+ tc->dsi_lanes = of_property_count_u32_elems(endpoint, "data-lanes");
The data-lanes property isn't documented in the DT binding. Please add.
Fixed in a separate patch.
[...]
@@ -1828,15 +2145,23 @@ static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id)
tc->have_irq = true;
}
- ret = tc_aux_link_setup(tc);
- if (ret)
- return ret;
+ if (tc->bridge.type != DRM_MODE_CONNECTOR_DPI) { /* (e)DP output */
+ ret = tc_aux_link_setup(tc);
+ if (ret)
+ return ret;
+ }
tc->bridge.of_node = dev->of_node;
drm_bridge_add(&tc->bridge);
i2c_set_clientdata(client, tc);
+ if (tc->bridge.type == DRM_MODE_CONNECTOR_DPI) { /* DPI output */
+ ret = tc_mipi_dsi_host_attach(tc);
+ if (ret)
+ return ret;
+ }
If tc_mipi_dsi_host_attach fails the drm bridge registered a few lines
above isn't cleaned up properly.
Fixed
