Re: [Intel-gfx] [PATCH v4 1/3] drm/i915/dp: Make sure all tiled connectors get added to the state with full modeset
On Mon, Dec 23, 2019 at 02:40:04PM -0800, Manasi Navare wrote:
> In case of tiled displays, all the tiles are linke dto each other
> for transcoder port sync. So in intel_atomic_check() we need to make
> sure that we add all the tiles to the modeset and if one of the
> tiles needs a full modeset then mark all other tiles for a full modeset.
>
> We also need to force modeset for all synced crtcs after fastset check.
>
> v5:
> * Rebase
> v4:
> * Fix logic for modeset_synced_crtcs (Ville)
> v3:
> * Add tile checks only for Gen >11
> v2:
> * Change crtc_state scope, remove tile_grp_id (Ville)
> * Use intel_connector_needs_modeset() (Ville)
> * Add modeset_synced_crtcs (Ville)
> * Make sure synced crtcs are forced full modeset
> after fastset check (Ville)
>
> Suggested-by: Ville Syrjälä
> Cc: Ville Syrjälä
> Cc: José Roberto de Souza
> Cc: Matt Roper
> Bugzilla: https://gitlab.freedesktop.org/drm/intel/issues/5
> Signed-off-by: Manasi Navare
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 123 +++
> 1 file changed, 123 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 94fc4b5bacc0..45a699bac34a 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -14304,6 +14304,118 @@ static bool
> intel_cpu_transcoder_needs_modeset(struct intel_atomic_state *state,
> return false;
> }
>
> +static void
> +intel_modeset_synced_crtcs(struct intel_atomic_state *state,
> +u8 transcoders)
> +{
> + struct intel_crtc_state *new_crtc_state;
> + struct intel_crtc *crtc;
> + int i;
> +
> + for_each_new_intel_crtc_in_state(state, crtc,
> + new_crtc_state, i) {
> + if (transcoders & BIT(new_crtc_state->cpu_transcoder)) {
> + new_crtc_state->uapi.mode_changed = true;
> + new_crtc_state->update_pipe = false;
> + }
> + }
> +}
> +
> +static void
> +intel_atomic_check_synced_crtcs(struct intel_atomic_state *state)
> +{
> + struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> + struct intel_crtc_state *new_crtc_state;
> + struct intel_crtc *crtc;
> + int i;
> +
> + if (INTEL_GEN(dev_priv) < 11)
> + return;
> +
> + for_each_new_intel_crtc_in_state(state, crtc,
> + new_crtc_state, i) {
> + if (is_trans_port_sync_master(new_crtc_state) &&
> + needs_modeset(new_crtc_state)) {
> + intel_modeset_synced_crtcs(state,
> +
> new_crtc_state->sync_mode_slaves_mask);
> + } else if (is_trans_port_sync_slave(new_crtc_state) &&
> +needs_modeset(new_crtc_state)) {
> + intel_modeset_synced_crtcs(state,
> +
> BIT(new_crtc_state->master_transcoder));
> + }
> + }
> +}
> +
> +static int
> +intel_modeset_all_tiles(struct intel_atomic_state *state, int tile_grp_id)
> +{
> + struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> + struct drm_connector *connector;
> + struct drm_connector_list_iter conn_iter;
> + int ret = 0;
> +
> + drm_connector_list_iter_begin(_priv->drm, _iter);
> + drm_for_each_connector_iter(connector, _iter) {
> + struct drm_connector_state *conn_state;
> + struct drm_crtc_state *crtc_state;
> +
> + if (!connector->has_tile ||
> + connector->tile_group->id != tile_grp_id)
> + continue;
> + conn_state = drm_atomic_get_connector_state(>base,
> + connector);
> + if (IS_ERR(conn_state)) {
> + ret = PTR_ERR(conn_state);
> + break;
> + }
> +
> + if (!conn_state->crtc)
> + continue;
> +
> + crtc_state = drm_atomic_get_crtc_state(>base,
> +conn_state->crtc);
> + if (IS_ERR(crtc_state)) {
> + ret = PTR_ERR(conn_state);
> + break;
> + }
> + crtc_state->mode_changed = true;
> + ret = drm_atomic_add_affected_connectors(>base,
> + conn_state->crtc);
> + if (ret)
> + break;
> + }
> + drm_connector_list_iter_end(_iter);
> +
> + return ret;
> +}
This feels like it could be pulled out to a DRM-level helper since
there's nothing really Intel-specific here. I imagine other drivers
with port sync capabilities will need pretty much exactly the same
logic? But that can wait until a future patch.
> +
> +static int
>
[Intel-gfx] [PATCH v4 1/3] drm/i915/dp: Make sure all tiled connectors get added to the state with full modeset
In case of tiled displays, all the tiles are linke dto each other
for transcoder port sync. So in intel_atomic_check() we need to make
sure that we add all the tiles to the modeset and if one of the
tiles needs a full modeset then mark all other tiles for a full modeset.
We also need to force modeset for all synced crtcs after fastset check.
v5:
* Rebase
v4:
* Fix logic for modeset_synced_crtcs (Ville)
v3:
* Add tile checks only for Gen >11
v2:
* Change crtc_state scope, remove tile_grp_id (Ville)
* Use intel_connector_needs_modeset() (Ville)
* Add modeset_synced_crtcs (Ville)
* Make sure synced crtcs are forced full modeset
after fastset check (Ville)
Suggested-by: Ville Syrjälä
Cc: Ville Syrjälä
Cc: José Roberto de Souza
Cc: Matt Roper
Bugzilla: https://gitlab.freedesktop.org/drm/intel/issues/5
Signed-off-by: Manasi Navare
---
drivers/gpu/drm/i915/display/intel_display.c | 123 +++
1 file changed, 123 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index 94fc4b5bacc0..45a699bac34a 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -14304,6 +14304,118 @@ static bool intel_cpu_transcoder_needs_modeset(struct
intel_atomic_state *state,
return false;
}
+static void
+intel_modeset_synced_crtcs(struct intel_atomic_state *state,
+ u8 transcoders)
+{
+ struct intel_crtc_state *new_crtc_state;
+ struct intel_crtc *crtc;
+ int i;
+
+ for_each_new_intel_crtc_in_state(state, crtc,
+new_crtc_state, i) {
+ if (transcoders & BIT(new_crtc_state->cpu_transcoder)) {
+ new_crtc_state->uapi.mode_changed = true;
+ new_crtc_state->update_pipe = false;
+ }
+ }
+}
+
+static void
+intel_atomic_check_synced_crtcs(struct intel_atomic_state *state)
+{
+ struct drm_i915_private *dev_priv = to_i915(state->base.dev);
+ struct intel_crtc_state *new_crtc_state;
+ struct intel_crtc *crtc;
+ int i;
+
+ if (INTEL_GEN(dev_priv) < 11)
+ return;
+
+ for_each_new_intel_crtc_in_state(state, crtc,
+new_crtc_state, i) {
+ if (is_trans_port_sync_master(new_crtc_state) &&
+ needs_modeset(new_crtc_state)) {
+ intel_modeset_synced_crtcs(state,
+
new_crtc_state->sync_mode_slaves_mask);
+ } else if (is_trans_port_sync_slave(new_crtc_state) &&
+ needs_modeset(new_crtc_state)) {
+ intel_modeset_synced_crtcs(state,
+
BIT(new_crtc_state->master_transcoder));
+ }
+ }
+}
+
+static int
+intel_modeset_all_tiles(struct intel_atomic_state *state, int tile_grp_id)
+{
+ struct drm_i915_private *dev_priv = to_i915(state->base.dev);
+ struct drm_connector *connector;
+ struct drm_connector_list_iter conn_iter;
+ int ret = 0;
+
+ drm_connector_list_iter_begin(_priv->drm, _iter);
+ drm_for_each_connector_iter(connector, _iter) {
+ struct drm_connector_state *conn_state;
+ struct drm_crtc_state *crtc_state;
+
+ if (!connector->has_tile ||
+ connector->tile_group->id != tile_grp_id)
+ continue;
+ conn_state = drm_atomic_get_connector_state(>base,
+ connector);
+ if (IS_ERR(conn_state)) {
+ ret = PTR_ERR(conn_state);
+ break;
+ }
+
+ if (!conn_state->crtc)
+ continue;
+
+ crtc_state = drm_atomic_get_crtc_state(>base,
+ conn_state->crtc);
+ if (IS_ERR(crtc_state)) {
+ ret = PTR_ERR(conn_state);
+ break;
+ }
+ crtc_state->mode_changed = true;
+ ret = drm_atomic_add_affected_connectors(>base,
+conn_state->crtc);
+ if (ret)
+ break;
+ }
+ drm_connector_list_iter_end(_iter);
+
+ return ret;
+}
+
+static int
+intel_atomic_check_tiled_conns(struct intel_atomic_state *state)
+{
+ struct drm_i915_private *dev_priv = to_i915(state->base.dev);
+ struct drm_connector *connector;
+ struct drm_connector_state *old_conn_state, *new_conn_state;
+ int i, ret;
+
+ if (INTEL_GEN(dev_priv) < 11)
+ return 0;
+
+ /* Is tiled, mark all other tiled CRTCs as needing a modeset */
+ for_each_oldnew_connector_in_state(>base,
