Re: [Intel-gfx] [PATCH resend 06/15] drm/i915/dsi: Make intel_dsi_enable/disable directly exec VBT sequences

[Bob Paauwe]

On Mon, 20 Feb 2017 15:08:36 +0100
Hans de Goede  wrote:

> The drm_panel_enable/disable and drm_panel_prepare/unprepare calls are
> not fine grained enough to abstract all the different steps we need to
> take (and VBT sequences we need to exec) properly. So simply remove the
> panel _enable/disable and prepare/unprepare callbacks and instead
> export intel_dsi_exec_vbt_sequence() from intel_dsi_panel_vbt.c
> and call that from intel_dsi_enable/disable().
> 
> No functional changes.
> 
> Signed-off-by: Hans de Goede 

Reviewed-by: Bob Paauwe 

> ---
>  drivers/gpu/drm/i915/intel_dsi.c   | 14 +++---
>  drivers/gpu/drm/i915/intel_dsi.h   |  3 +++
>  drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 43 
> ++
>  3 files changed, 15 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c 
> b/drivers/gpu/drm/i915/intel_dsi.c
> index 8808f87..78d5884 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -669,7 +669,10 @@ static void intel_dsi_pre_enable(struct intel_encoder 
> *encoder,
>   /* put device in ready state */
>   intel_dsi_device_ready(encoder);
>  
> - drm_panel_prepare(intel_dsi->panel);
> + intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_ASSERT_RESET);
> + intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_POWER_ON);
> + intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_DEASSERT_RESET);
> + intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_INIT_OTP);
>  
>   /* Enable port in pre-enable phase itself because as per hw team
>* recommendation, port should be enabled befor plane & pipe */
> @@ -682,7 +685,8 @@ static void intel_dsi_pre_enable(struct intel_encoder 
> *encoder,
>   dpi_send_cmd(intel_dsi, TURN_ON, false, port);
>   msleep(100);
>  
> - drm_panel_enable(intel_dsi->panel);
> + intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_DISPLAY_ON);
> + intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_BACKLIGHT_ON);
>  
>   intel_dsi_port_enable(encoder);
>   }
> @@ -757,7 +761,8 @@ static void intel_dsi_post_disable(struct intel_encoder 
> *encoder,
>* if disable packets are sent before sending shutdown packet then in
>* some next enable sequence send turn on packet error is observed
>*/
> - drm_panel_disable(intel_dsi->panel);
> + intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_BACKLIGHT_OFF);
> + intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_DISPLAY_OFF);
>  
>   intel_dsi_clear_device_ready(encoder);
>  
> @@ -782,7 +787,8 @@ static void intel_dsi_post_disable(struct intel_encoder 
> *encoder,
>   I915_WRITE(DSPCLK_GATE_D, val);
>   }
>  
> - drm_panel_unprepare(intel_dsi->panel);
> + intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_ASSERT_RESET);
> + intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_POWER_OFF);
>  
>   msleep(intel_dsi->panel_off_delay);
>  
> diff --git a/drivers/gpu/drm/i915/intel_dsi.h 
> b/drivers/gpu/drm/i915/intel_dsi.h
> index d567823..5486491 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.h
> +++ b/drivers/gpu/drm/i915/intel_dsi.h
> @@ -132,6 +132,9 @@ static inline struct intel_dsi *enc_to_intel_dsi(struct 
> drm_encoder *encoder)
>  
>  void wait_for_dsi_fifo_empty(struct intel_dsi *intel_dsi, enum port port);
>  
> +void intel_dsi_exec_vbt_sequence(struct intel_dsi *intel_dsi,
> +  enum mipi_seq seq_id);
> +
>  bool intel_dsi_pll_is_enabled(struct drm_i915_private *dev_priv);
>  int intel_compute_dsi_pll(struct intel_encoder *encoder,
> struct intel_crtc_state *config);
> diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c 
> b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> index 995f72d..0ce1086 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> @@ -426,10 +426,9 @@ static const char *sequence_name(enum mipi_seq seq_id)
>   return "(unknown)";
>  }
>  
> -static void generic_exec_sequence(struct drm_panel *panel, enum mipi_seq 
> seq_id)
> +void intel_dsi_exec_vbt_sequence(struct intel_dsi *intel_dsi,
> +  enum mipi_seq seq_id)
>  {
> - struct vbt_panel *vbt_panel = to_vbt_panel(panel);
> - struct intel_dsi *intel_dsi = vbt_panel->intel_dsi;
>   struct drm_i915_private *dev_priv = to_i915(intel_dsi->base.base.dev);
>   const u8 *data;
>   fn_mipi_elem_exec mipi_elem_exec;
> @@ -493,40 +492,6 @@ static void generic_exec_sequence(struct drm_panel 
> *panel, enum mipi_seq seq_id)
>   }
>  }
>  
> -static int vbt_panel_prepare(struct drm_panel *panel)
> -{
> - generic_exec_sequence(panel, MIPI_SEQ_ASSERT_RESET);
> - generic_exec_sequence(panel, MIPI_SEQ_POWER_ON);
> - generic_exec_sequence(panel, MIPI_SEQ_DEASSERT_RESET);
> -

[Intel-gfx] [PATCH resend 06/15] drm/i915/dsi: Make intel_dsi_enable/disable directly exec VBT sequences

[Hans de Goede]

The drm_panel_enable/disable and drm_panel_prepare/unprepare calls are
not fine grained enough to abstract all the different steps we need to
take (and VBT sequences we need to exec) properly. So simply remove the
panel _enable/disable and prepare/unprepare callbacks and instead
export intel_dsi_exec_vbt_sequence() from intel_dsi_panel_vbt.c
and call that from intel_dsi_enable/disable().

No functional changes.

Signed-off-by: Hans de Goede 
---
 drivers/gpu/drm/i915/intel_dsi.c   | 14 +++---
 drivers/gpu/drm/i915/intel_dsi.h   |  3 +++
 drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 43 ++
 3 files changed, 15 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 8808f87..78d5884 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -669,7 +669,10 @@ static void intel_dsi_pre_enable(struct intel_encoder 
*encoder,
/* put device in ready state */
intel_dsi_device_ready(encoder);
 
-   drm_panel_prepare(intel_dsi->panel);
+   intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_ASSERT_RESET);
+   intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_POWER_ON);
+   intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_DEASSERT_RESET);
+   intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_INIT_OTP);
 
/* Enable port in pre-enable phase itself because as per hw team
 * recommendation, port should be enabled befor plane & pipe */
@@ -682,7 +685,8 @@ static void intel_dsi_pre_enable(struct intel_encoder 
*encoder,
dpi_send_cmd(intel_dsi, TURN_ON, false, port);
msleep(100);
 
-   drm_panel_enable(intel_dsi->panel);
+   intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_DISPLAY_ON);
+   intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_BACKLIGHT_ON);
 
intel_dsi_port_enable(encoder);
}
@@ -757,7 +761,8 @@ static void intel_dsi_post_disable(struct intel_encoder 
*encoder,
 * if disable packets are sent before sending shutdown packet then in
 * some next enable sequence send turn on packet error is observed
 */
-   drm_panel_disable(intel_dsi->panel);
+   intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_BACKLIGHT_OFF);
+   intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_DISPLAY_OFF);
 
intel_dsi_clear_device_ready(encoder);
 
@@ -782,7 +787,8 @@ static void intel_dsi_post_disable(struct intel_encoder 
*encoder,
I915_WRITE(DSPCLK_GATE_D, val);
}
 
-   drm_panel_unprepare(intel_dsi->panel);
+   intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_ASSERT_RESET);
+   intel_dsi_exec_vbt_sequence(intel_dsi, MIPI_SEQ_POWER_OFF);
 
msleep(intel_dsi->panel_off_delay);
 
diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index d567823..5486491 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -132,6 +132,9 @@ static inline struct intel_dsi *enc_to_intel_dsi(struct 
drm_encoder *encoder)
 
 void wait_for_dsi_fifo_empty(struct intel_dsi *intel_dsi, enum port port);
 
+void intel_dsi_exec_vbt_sequence(struct intel_dsi *intel_dsi,
+enum mipi_seq seq_id);
+
 bool intel_dsi_pll_is_enabled(struct drm_i915_private *dev_priv);
 int intel_compute_dsi_pll(struct intel_encoder *encoder,
  struct intel_crtc_state *config);
diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c 
b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
index 995f72d..0ce1086 100644
--- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
+++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
@@ -426,10 +426,9 @@ static const char *sequence_name(enum mipi_seq seq_id)
return "(unknown)";
 }
 
-static void generic_exec_sequence(struct drm_panel *panel, enum mipi_seq 
seq_id)
+void intel_dsi_exec_vbt_sequence(struct intel_dsi *intel_dsi,
+enum mipi_seq seq_id)
 {
-   struct vbt_panel *vbt_panel = to_vbt_panel(panel);
-   struct intel_dsi *intel_dsi = vbt_panel->intel_dsi;
struct drm_i915_private *dev_priv = to_i915(intel_dsi->base.base.dev);
const u8 *data;
fn_mipi_elem_exec mipi_elem_exec;
@@ -493,40 +492,6 @@ static void generic_exec_sequence(struct drm_panel *panel, 
enum mipi_seq seq_id)
}
 }
 
-static int vbt_panel_prepare(struct drm_panel *panel)
-{
-   generic_exec_sequence(panel, MIPI_SEQ_ASSERT_RESET);
-   generic_exec_sequence(panel, MIPI_SEQ_POWER_ON);
-   generic_exec_sequence(panel, MIPI_SEQ_DEASSERT_RESET);
-   generic_exec_sequence(panel, MIPI_SEQ_INIT_OTP);
-
-   return 0;
-}
-
-static int vbt_panel_unprepare(struct drm_panel *panel)
-{
-   generic_exec_sequence(panel, MIPI_SEQ_ASSERT_RESET);
-   generic_exec_sequence(panel, MIPI_SEQ_POWER_OFF);
-
-   return 0;