[v7,01/23] drm/i915: make encoder enable and disable hooks optional

Submitted by Jani Nikula on Oct. 15, 2018, 2:27 p.m.

Details

Message ID 2d78a3c521acfce05ba0a31fe1244dd7d6bb2dca.1539613303.git.jani.nikula@intel.com
State New
Headers show
Series "drm/i915/icl: dsi enabling" ( rev: 1 ) in Intel GFX

Not browsing as part of any series.

Commit Message

Jani Nikula Oct. 15, 2018, 2:27 p.m.
Encoders are not alike, make enable and disable hooks optional like
other hooks. Utilize this in DSI code, and remove the silly nop hook.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |  6 ++++--
 drivers/gpu/drm/i915/vlv_dsi.c       | 16 ++++------------
 2 files changed, 8 insertions(+), 14 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 41abd03ce6a6..32ea71bac663 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5461,7 +5461,8 @@  static void intel_encoders_enable(struct drm_crtc *crtc,
 		if (conn_state->crtc != crtc)
 			continue;
 
-		encoder->enable(encoder, crtc_state, conn_state);
+		if (encoder->enable)
+			encoder->enable(encoder, crtc_state, conn_state);
 		intel_opregion_notify_encoder(encoder, true);
 	}
 }
@@ -5482,7 +5483,8 @@  static void intel_encoders_disable(struct drm_crtc *crtc,
 			continue;
 
 		intel_opregion_notify_encoder(encoder, false);
-		encoder->disable(encoder, old_crtc_state, old_conn_state);
+		if (encoder->disable)
+			encoder->disable(encoder, old_crtc_state, old_conn_state);
 	}
 }
 
diff --git a/drivers/gpu/drm/i915/vlv_dsi.c b/drivers/gpu/drm/i915/vlv_dsi.c
index bafeb2a19b90..dbca30460a6b 100644
--- a/drivers/gpu/drm/i915/vlv_dsi.c
+++ b/drivers/gpu/drm/i915/vlv_dsi.c
@@ -794,6 +794,10 @@  static void intel_dsi_msleep(struct intel_dsi *intel_dsi, int msec)
  * - wait t4                                           - wait t4
  */
 
+/*
+ * DSI port enable has to be done before pipe and plane enable, so we do it in
+ * the pre_enable hook instead of the enable hook.
+ */
 static void intel_dsi_pre_enable(struct intel_encoder *encoder,
 				 const struct intel_crtc_state *pipe_config,
 				 const struct drm_connector_state *conn_state)
@@ -896,17 +900,6 @@  static void intel_dsi_pre_enable(struct intel_encoder *encoder,
 }
 
 /*
- * DSI port enable has to be done before pipe and plane enable, so we do it in
- * the pre_enable hook.
- */
-static void intel_dsi_enable_nop(struct intel_encoder *encoder,
-				 const struct intel_crtc_state *pipe_config,
-				 const struct drm_connector_state *conn_state)
-{
-	DRM_DEBUG_KMS("\n");
-}
-
-/*
  * DSI port disable has to be done after pipe and plane disable, so we do it in
  * the post_disable hook.
  */
@@ -1764,7 +1757,6 @@  void vlv_dsi_init(struct drm_i915_private *dev_priv)
 
 	intel_encoder->compute_config = intel_dsi_compute_config;
 	intel_encoder->pre_enable = intel_dsi_pre_enable;
-	intel_encoder->enable = intel_dsi_enable_nop;
 	intel_encoder->disable = intel_dsi_disable;
 	intel_encoder->post_disable = intel_dsi_post_disable;
 	intel_encoder->get_hw_state = intel_dsi_get_hw_state;

Comments

On 10/15/2018 7:57 PM, Jani Nikula wrote:
> Encoders are not alike, make enable and disable hooks optional like
> other hooks. Utilize this in DSI code, and remove the silly nop hook.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_display.c |  6 ++++--
>   drivers/gpu/drm/i915/vlv_dsi.c       | 16 ++++------------
>   2 files changed, 8 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 41abd03ce6a6..32ea71bac663 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5461,7 +5461,8 @@ static void intel_encoders_enable(struct drm_crtc *crtc,
>   		if (conn_state->crtc != crtc)
>   			continue;
>   
> -		encoder->enable(encoder, crtc_state, conn_state);
> +		if (encoder->enable)
> +			encoder->enable(encoder, crtc_state, conn_state);
>   		intel_opregion_notify_encoder(encoder, true);
>   	}
>   }
> @@ -5482,7 +5483,8 @@ static void intel_encoders_disable(struct drm_crtc *crtc,
>   			continue;
>   
>   		intel_opregion_notify_encoder(encoder, false);
> -		encoder->disable(encoder, old_crtc_state, old_conn_state);
> +		if (encoder->disable)
> +			encoder->disable(encoder, old_crtc_state, old_conn_state);
>   	}

encoder->disable() gets called directly inside intel_sanitize_encoder() 
without

intel_encoders_disable().I think, we need to put the check there as well.


With that fix,
Reviewed-by: Madhav Chauhan <madhav.chauhan@intel.com>

Regards,
Madhav

>   }
>   
> diff --git a/drivers/gpu/drm/i915/vlv_dsi.c b/drivers/gpu/drm/i915/vlv_dsi.c
> index bafeb2a19b90..dbca30460a6b 100644
> --- a/drivers/gpu/drm/i915/vlv_dsi.c
> +++ b/drivers/gpu/drm/i915/vlv_dsi.c
> @@ -794,6 +794,10 @@ static void intel_dsi_msleep(struct intel_dsi *intel_dsi, int msec)
>    * - wait t4                                           - wait t4
>    */
>   
> +/*
> + * DSI port enable has to be done before pipe and plane enable, so we do it in
> + * the pre_enable hook instead of the enable hook.
> + */
>   static void intel_dsi_pre_enable(struct intel_encoder *encoder,
>   				 const struct intel_crtc_state *pipe_config,
>   				 const struct drm_connector_state *conn_state)
> @@ -896,17 +900,6 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder,
>   }
>   
>   /*
> - * DSI port enable has to be done before pipe and plane enable, so we do it in
> - * the pre_enable hook.
> - */
> -static void intel_dsi_enable_nop(struct intel_encoder *encoder,
> -				 const struct intel_crtc_state *pipe_config,
> -				 const struct drm_connector_state *conn_state)
> -{
> -	DRM_DEBUG_KMS("\n");
> -}
> -
> -/*
>    * DSI port disable has to be done after pipe and plane disable, so we do it in
>    * the post_disable hook.
>    */
> @@ -1764,7 +1757,6 @@ void vlv_dsi_init(struct drm_i915_private *dev_priv)
>   
>   	intel_encoder->compute_config = intel_dsi_compute_config;
>   	intel_encoder->pre_enable = intel_dsi_pre_enable;
> -	intel_encoder->enable = intel_dsi_enable_nop;
>   	intel_encoder->disable = intel_dsi_disable;
>   	intel_encoder->post_disable = intel_dsi_post_disable;
>   	intel_encoder->get_hw_state = intel_dsi_get_hw_state;