[v2,1/5] drm/bridge/synopsys: dsi: move phy_ops callbacks around panel enablement
diff mbox series

Message ID 20191108000253.8560-2-heiko.stuebner@theobroma-systems.com
State New
Headers show
Series
  • dw-mipi-dsi support for Rockchip px30
Related show

Commit Message

Heiko Stuebner Nov. 8, 2019, 12:02 a.m. UTC
If implementation-specific phy_ops need to be defined they probably
should be enabled before trying to talk to the panel and disabled only
after the panel was disabled.

Right now they are enabled last and disabled first, so might make it
impossible to talk to some panels - example for this being the px30
with an external Innosilicon dphy that needs the phy to be enabled
to transfer commands to the panel.

So move the calls appropriately.

Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
---
 drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

Comments

Neil Armstrong Nov. 12, 2019, 2:26 p.m. UTC | #1
On 08/11/2019 01:02, Heiko Stuebner wrote:
> If implementation-specific phy_ops need to be defined they probably
> should be enabled before trying to talk to the panel and disabled only
> after the panel was disabled.
> 
> Right now they are enabled last and disabled first, so might make it
> impossible to talk to some panels - example for this being the px30
> with an external Innosilicon dphy that needs the phy to be enabled
> to transfer commands to the panel.
> 
> So move the calls appropriately.
> 
> Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> ---
>  drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> index 675442bfc1bd..49f5600a1dea 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> @@ -797,9 +797,6 @@ static void dw_mipi_dsi_bridge_post_disable(struct drm_bridge *bridge)
>  	struct dw_mipi_dsi *dsi = bridge_to_dsi(bridge);
>  	const struct dw_mipi_dsi_phy_ops *phy_ops = dsi->plat_data->phy_ops;
>  
> -	if (phy_ops->power_off)
> -		phy_ops->power_off(dsi->plat_data->priv_data);
> -
>  	/*
>  	 * Switch to command mode before panel-bridge post_disable &
>  	 * panel unprepare.
> @@ -816,6 +813,9 @@ static void dw_mipi_dsi_bridge_post_disable(struct drm_bridge *bridge)
>  	 */
>  	dsi->panel_bridge->funcs->post_disable(dsi->panel_bridge);
>  
> +	if (phy_ops->power_off)
> +		phy_ops->power_off(dsi->plat_data->priv_data);
> +
>  	if (dsi->slave) {
>  		dw_mipi_dsi_disable(dsi->slave);
>  		clk_disable_unprepare(dsi->slave->pclk);
> @@ -882,6 +882,9 @@ static void dw_mipi_dsi_mode_set(struct dw_mipi_dsi *dsi,
>  
>  	/* Switch to cmd mode for panel-bridge pre_enable & panel prepare */
>  	dw_mipi_dsi_set_mode(dsi, 0);
> +
> +	if (phy_ops->power_on)
> +		phy_ops->power_on(dsi->plat_data->priv_data);
>  }
>  
>  static void dw_mipi_dsi_bridge_mode_set(struct drm_bridge *bridge,
> @@ -898,15 +901,11 @@ static void dw_mipi_dsi_bridge_mode_set(struct drm_bridge *bridge,
>  static void dw_mipi_dsi_bridge_enable(struct drm_bridge *bridge)
>  {
>  	struct dw_mipi_dsi *dsi = bridge_to_dsi(bridge);
> -	const struct dw_mipi_dsi_phy_ops *phy_ops = dsi->plat_data->phy_ops;
>  
>  	/* Switch to video mode for panel-bridge enable & panel enable */
>  	dw_mipi_dsi_set_mode(dsi, MIPI_DSI_MODE_VIDEO);
>  	if (dsi->slave)
>  		dw_mipi_dsi_set_mode(dsi->slave, MIPI_DSI_MODE_VIDEO);
> -
> -	if (phy_ops->power_on)
> -		phy_ops->power_on(dsi->plat_data->priv_data);
>  }
>  
>  static enum drm_mode_status
> 

Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
Philippe CORNU Nov. 12, 2019, 3:37 p.m. UTC | #2
Hi Heiko,

On 11/8/19 1:02 AM, Heiko Stuebner wrote:
> If implementation-specific phy_ops need to be defined they probably
> should be enabled before trying to talk to the panel and disabled only
> after the panel was disabled.
> 
> Right now they are enabled last and disabled first, so might make it
> impossible to talk to some panels - example for this being the px30
> with an external Innosilicon dphy that needs the phy to be enabled
> to transfer commands to the panel.
> 
> So move the calls appropriately.
> 
> Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> ---
>   drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 13 ++++++-------
>   1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> index 675442bfc1bd..49f5600a1dea 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> @@ -797,9 +797,6 @@ static void dw_mipi_dsi_bridge_post_disable(struct drm_bridge *bridge)
>   	struct dw_mipi_dsi *dsi = bridge_to_dsi(bridge);
>   	const struct dw_mipi_dsi_phy_ops *phy_ops = dsi->plat_data->phy_ops;
>   
> -	if (phy_ops->power_off)
> -		phy_ops->power_off(dsi->plat_data->priv_data);
> -
>   	/*
>   	 * Switch to command mode before panel-bridge post_disable &
>   	 * panel unprepare.
> @@ -816,6 +813,9 @@ static void dw_mipi_dsi_bridge_post_disable(struct drm_bridge *bridge)
>   	 */
>   	dsi->panel_bridge->funcs->post_disable(dsi->panel_bridge);
>   
> +	if (phy_ops->power_off)
> +		phy_ops->power_off(dsi->plat_data->priv_data);
> +
>   	if (dsi->slave) {
>   		dw_mipi_dsi_disable(dsi->slave);
>   		clk_disable_unprepare(dsi->slave->pclk);
> @@ -882,6 +882,9 @@ static void dw_mipi_dsi_mode_set(struct dw_mipi_dsi *dsi,
>   
>   	/* Switch to cmd mode for panel-bridge pre_enable & panel prepare */
>   	dw_mipi_dsi_set_mode(dsi, 0);
> +
> +	if (phy_ops->power_on)
> +		phy_ops->power_on(dsi->plat_data->priv_data);
>   }
>   
>   static void dw_mipi_dsi_bridge_mode_set(struct drm_bridge *bridge,
> @@ -898,15 +901,11 @@ static void dw_mipi_dsi_bridge_mode_set(struct drm_bridge *bridge,
>   static void dw_mipi_dsi_bridge_enable(struct drm_bridge *bridge)
>   {
>   	struct dw_mipi_dsi *dsi = bridge_to_dsi(bridge);
> -	const struct dw_mipi_dsi_phy_ops *phy_ops = dsi->plat_data->phy_ops;
>   
>   	/* Switch to video mode for panel-bridge enable & panel enable */
>   	dw_mipi_dsi_set_mode(dsi, MIPI_DSI_MODE_VIDEO);
>   	if (dsi->slave)
>   		dw_mipi_dsi_set_mode(dsi->slave, MIPI_DSI_MODE_VIDEO);
> -
> -	if (phy_ops->power_on)
> -		phy_ops->power_on(dsi->plat_data->priv_data);
>   }
>   
>   static enum drm_mode_status
> 

Tested-by: Yannick Fertre <yannick.fertre@st.com>
Reviewed-by: Philippe Cornu <philippe.cornu@st.com>

Many thanks,
Philippe :-)

Patch
diff mbox series

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
index 675442bfc1bd..49f5600a1dea 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
@@ -797,9 +797,6 @@  static void dw_mipi_dsi_bridge_post_disable(struct drm_bridge *bridge)
 	struct dw_mipi_dsi *dsi = bridge_to_dsi(bridge);
 	const struct dw_mipi_dsi_phy_ops *phy_ops = dsi->plat_data->phy_ops;
 
-	if (phy_ops->power_off)
-		phy_ops->power_off(dsi->plat_data->priv_data);
-
 	/*
 	 * Switch to command mode before panel-bridge post_disable &
 	 * panel unprepare.
@@ -816,6 +813,9 @@  static void dw_mipi_dsi_bridge_post_disable(struct drm_bridge *bridge)
 	 */
 	dsi->panel_bridge->funcs->post_disable(dsi->panel_bridge);
 
+	if (phy_ops->power_off)
+		phy_ops->power_off(dsi->plat_data->priv_data);
+
 	if (dsi->slave) {
 		dw_mipi_dsi_disable(dsi->slave);
 		clk_disable_unprepare(dsi->slave->pclk);
@@ -882,6 +882,9 @@  static void dw_mipi_dsi_mode_set(struct dw_mipi_dsi *dsi,
 
 	/* Switch to cmd mode for panel-bridge pre_enable & panel prepare */
 	dw_mipi_dsi_set_mode(dsi, 0);
+
+	if (phy_ops->power_on)
+		phy_ops->power_on(dsi->plat_data->priv_data);
 }
 
 static void dw_mipi_dsi_bridge_mode_set(struct drm_bridge *bridge,
@@ -898,15 +901,11 @@  static void dw_mipi_dsi_bridge_mode_set(struct drm_bridge *bridge,
 static void dw_mipi_dsi_bridge_enable(struct drm_bridge *bridge)
 {
 	struct dw_mipi_dsi *dsi = bridge_to_dsi(bridge);
-	const struct dw_mipi_dsi_phy_ops *phy_ops = dsi->plat_data->phy_ops;
 
 	/* Switch to video mode for panel-bridge enable & panel enable */
 	dw_mipi_dsi_set_mode(dsi, MIPI_DSI_MODE_VIDEO);
 	if (dsi->slave)
 		dw_mipi_dsi_set_mode(dsi->slave, MIPI_DSI_MODE_VIDEO);
-
-	if (phy_ops->power_on)
-		phy_ops->power_on(dsi->plat_data->priv_data);
 }
 
 static enum drm_mode_status