diff mbox series

[v2,6/6] drm/i915/display/icl: In port sync mode disable slaves first then master

Message ID 20190909155212.24230-2-manasi.d.navare@intel.com (mailing list archive)
State New, archived
Headers show
Series None | expand

Commit Message

Navare, Manasi Sept. 9, 2019, 3:52 p.m. UTC
In the transcoder port sync mode, the slave transcoders mask their vblanks
until master transcoder's vblank so while disabling them, make
sure slaves are disabled first and then the masters.

v4:
* Obtain slave state from master (Maarten)
v3:
* Rebase
v2:
* Use the intel_old_crtc_state_disables() helper

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 62 ++++++++++++++++++--
 1 file changed, 56 insertions(+), 6 deletions(-)

Comments

Maarten Lankhorst Sept. 17, 2019, 3:05 p.m. UTC | #1
Op 09-09-2019 om 17:52 schreef Manasi Navare:
> In the transcoder port sync mode, the slave transcoders mask their vblanks
> until master transcoder's vblank so while disabling them, make
> sure slaves are disabled first and then the masters.
>
> v4:
> * Obtain slave state from master (Maarten)
> v3:
> * Rebase
> v2:
> * Use the intel_old_crtc_state_disables() helper
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 62 ++++++++++++++++++--
>  1 file changed, 56 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index ac6e880570ea..8c333280d8f5 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -14074,8 +14074,42 @@ static void intel_old_crtc_state_disables(struct intel_atomic_state *state,
>  						     new_crtc_state);
>  }
>  
> +static void intel_trans_port_sync_modeset_disables(struct intel_atomic_state *state,
> +						   struct intel_crtc *crtc,
> +						   struct intel_crtc_state *old_crtc_state,
> +						   struct intel_crtc_state *new_crtc_state)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> +	struct intel_crtc *slave_crtc = intel_get_slave_crtc(dev_priv,
> +							     new_crtc_state);
> +	struct intel_crtc_state *new_slave_crtc_state =
> +		intel_atomic_get_new_crtc_state(state, slave_crtc);
> +	struct intel_crtc_state *old_slave_crtc_state =
> +		intel_atomic_get_old_crtc_state(state, slave_crtc);
> +
> +	WARN_ON(!slave_crtc || !new_slave_crtc_state ||
> +		!old_slave_crtc_state);
> +
> +	/* Disable Slave first */
> +	intel_pre_plane_update(old_slave_crtc_state, new_slave_crtc_state);
> +	if (old_slave_crtc_state->base.active)
> +		intel_old_crtc_state_disables(state,
> +					      old_slave_crtc_state,
> +					      new_slave_crtc_state,
> +					      slave_crtc);
> +
> +	/* Disable Master */
> +	intel_pre_plane_update(old_crtc_state, new_crtc_state);
> +	if (old_crtc_state->base.active)
> +		intel_old_crtc_state_disables(state,
> +					      old_crtc_state,
> +					      new_crtc_state,
> +					      crtc);
> +}
> +
>  static void intel_commit_modeset_disables(struct intel_atomic_state *state)
>  {
> +	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
>  	struct intel_crtc_state *new_crtc_state, *old_crtc_state;
>  	struct intel_crtc *crtc;
>  	int i;
> @@ -14092,13 +14126,29 @@ static void intel_commit_modeset_disables(struct intel_atomic_state *state)
>  		if (!needs_modeset(new_crtc_state))
>  			continue;
>  
> -		intel_pre_plane_update(old_crtc_state, new_crtc_state);
> +		/* In case of Transcoder port Sync master slave CRTCs can be
> +		 * assigned in any order and we need to make sure that
> +		 * slave CRTCs are disabled first and then master CRTC since
> +		 * Slave vblanks are masked till Master Vblanks.
> +		 */
> +		if (is_trans_port_sync_mode(dev_priv, new_crtc_state)) {
> +			if (is_trans_port_sync_master(dev_priv,
> +						      new_crtc_state))
> +				intel_trans_port_sync_modeset_disables(state,
> +								       crtc,
> +								       old_crtc_state,
> +								       new_crtc_state);
> +			else
> +				continue;
> +		} else {
> +			intel_pre_plane_update(old_crtc_state, new_crtc_state);
>  
> -		if (old_crtc_state->base.active)
> -			intel_old_crtc_state_disables(state,
> -						      old_crtc_state,
> -						      new_crtc_state,
> -						      crtc);
> +			if (old_crtc_state->base.active)
> +				intel_old_crtc_state_disables(state,
> +							      old_crtc_state,
> +							      new_crtc_state,
> +							      crtc);
> +		}
>  	}
>  }
>  

Series looks good with those fixes:

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index ac6e880570ea..8c333280d8f5 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -14074,8 +14074,42 @@  static void intel_old_crtc_state_disables(struct intel_atomic_state *state,
 						     new_crtc_state);
 }
 
+static void intel_trans_port_sync_modeset_disables(struct intel_atomic_state *state,
+						   struct intel_crtc *crtc,
+						   struct intel_crtc_state *old_crtc_state,
+						   struct intel_crtc_state *new_crtc_state)
+{
+	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
+	struct intel_crtc *slave_crtc = intel_get_slave_crtc(dev_priv,
+							     new_crtc_state);
+	struct intel_crtc_state *new_slave_crtc_state =
+		intel_atomic_get_new_crtc_state(state, slave_crtc);
+	struct intel_crtc_state *old_slave_crtc_state =
+		intel_atomic_get_old_crtc_state(state, slave_crtc);
+
+	WARN_ON(!slave_crtc || !new_slave_crtc_state ||
+		!old_slave_crtc_state);
+
+	/* Disable Slave first */
+	intel_pre_plane_update(old_slave_crtc_state, new_slave_crtc_state);
+	if (old_slave_crtc_state->base.active)
+		intel_old_crtc_state_disables(state,
+					      old_slave_crtc_state,
+					      new_slave_crtc_state,
+					      slave_crtc);
+
+	/* Disable Master */
+	intel_pre_plane_update(old_crtc_state, new_crtc_state);
+	if (old_crtc_state->base.active)
+		intel_old_crtc_state_disables(state,
+					      old_crtc_state,
+					      new_crtc_state,
+					      crtc);
+}
+
 static void intel_commit_modeset_disables(struct intel_atomic_state *state)
 {
+	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
 	struct intel_crtc_state *new_crtc_state, *old_crtc_state;
 	struct intel_crtc *crtc;
 	int i;
@@ -14092,13 +14126,29 @@  static void intel_commit_modeset_disables(struct intel_atomic_state *state)
 		if (!needs_modeset(new_crtc_state))
 			continue;
 
-		intel_pre_plane_update(old_crtc_state, new_crtc_state);
+		/* In case of Transcoder port Sync master slave CRTCs can be
+		 * assigned in any order and we need to make sure that
+		 * slave CRTCs are disabled first and then master CRTC since
+		 * Slave vblanks are masked till Master Vblanks.
+		 */
+		if (is_trans_port_sync_mode(dev_priv, new_crtc_state)) {
+			if (is_trans_port_sync_master(dev_priv,
+						      new_crtc_state))
+				intel_trans_port_sync_modeset_disables(state,
+								       crtc,
+								       old_crtc_state,
+								       new_crtc_state);
+			else
+				continue;
+		} else {
+			intel_pre_plane_update(old_crtc_state, new_crtc_state);
 
-		if (old_crtc_state->base.active)
-			intel_old_crtc_state_disables(state,
-						      old_crtc_state,
-						      new_crtc_state,
-						      crtc);
+			if (old_crtc_state->base.active)
+				intel_old_crtc_state_disables(state,
+							      old_crtc_state,
+							      new_crtc_state,
+							      crtc);
+		}
 	}
 }