On 5/4/22 09:59, Raphael Gallais-Pou wrote:
Hi Marek,

Hi,

[...]

@@ -499,8 +512,16 @@ static int dw_mipi_dsi_stm_probe(struct platform_device 
*pdev)
        }
dsi->hw_version = dsi_read(dsi, DSI_VERSION) & VERSION;
+
+       ret = dw_mipi_dsi_phy_regulator_on(dsi);
        clk_disable_unprepare(pclk);
+ if (ret) {
+               DRM_ERROR("%s: Failed to enable wrapper regulator, ret=%d\n",
+                         __func__, ret);
+               goto err_dsi_probe;
+       }
+

I have no problem until here. If I understand this correctly, it enables the 
regulator during all the life of the driver.

If you feel an urge to merge this patch into the Linux kernel, the st display 
team could gladly do it because it enables more hardware bridges. However 
another solution could be to rework a bit the regulator part of the driver so 
that you would have only device tree to change, introducing a 'reg-always-on' 
property.

This driver needs in fact a bit of a rework with the power management. A 
solution could be to move the regulator-related part in 
dw_mipi_dsi_stm_power_on/off() so that it is only activated when needed. Those 
functions would integrate the enabling of the regulator, the switch for the 
internal regulator, and eventually handle the PLL if it cannot lock when the 
regulator is off.

With the DT property, the power management would be only in the 
probe()/remove(). In that way the DSI bridges would have the logic they need to 
work.

Ultimately there is two possibilities :
  * You really need this patch to be merged asap
  * You are ok to wait until we send the solution described above

If you want to write those patches (each for DT and regulator), feel free to do 
it.

What do you think about it ?

Maybe a more generic question first -- is there a way to pull the data lanes to LP11 without enabling the regulator ?

Also note that you likely want to wait with this patch, there is likely soon going to be a discussion about how to handle all those different requirements for initial DSI LP states and clock needed by DSI bridges, encoding such policy into DT is not the right approach.

Reply via email to