> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Ankit
> Nautiyal
> Sent: Thursday, July 18, 2024 1:48 PM
> To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: Lisovskiy, Stanislav <stanislav.lisovskiy@xxxxxxxxx>; Saarinen, Jani
> <jani.saarinen@xxxxxxxxx>; ville.syrjala@xxxxxxxxxxxxxxx
> Subject: [PATCH 11/12] drm/i915: Add new abstraction layer to handle pipe
> order for different joiners
>
> From: Stanislav Lisovskiy <stanislav.lisovskiy@xxxxxxxxx>
>
> Ultrajoiner case requires special treatment where both reverse and staight order
> iteration doesn't work(for instance disabling case requires order to be: primary
> master, slaves, secondary master).
>
> Lets unify our approach by using not only pipe masks for iterating required
> pipes based on joiner type used, but also using different "priority" arrays for
> each of those.
>
> v2: Fix checkpatch warnings. (Ankit)
>
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@xxxxxxxxx>
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_ddi.c | 19 +++--
> drivers/gpu/drm/i915/display/intel_display.c | 89 ++++++++++++++++----
> drivers/gpu/drm/i915/display/intel_display.h | 7 ++
> drivers/gpu/drm/i915/display/intel_dp_mst.c | 19 +++--
> 4 files changed, 102 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index a07aca96e551..d54c9e51209e 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3116,10 +3116,11 @@ static void
> intel_ddi_post_disable_hdmi_or_sst(struct intel_atomic_state *state,
> const struct drm_connector_state
> *old_conn_state) {
> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> - struct intel_crtc *pipe_crtc;
> + struct intel_crtc *pipe_crtc; enum pipe pipe;
Can we have these declarations on different lines
>
> - for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, pipe_crtc,
> -
> intel_crtc_joined_pipe_mask(old_crtc_state)) {
> + for_each_intel_crtc_in_mask_priority(dev_priv, pipe_crtc, pipe,
> +
> intel_crtc_joined_pipe_mask(old_crtc_state),
> +
> intel_get_pipe_order_disable(old_crtc_state)) {
> const struct intel_crtc_state *old_pipe_crtc_state =
> intel_atomic_get_old_crtc_state(state, pipe_crtc);
>
> @@ -3130,8 +3131,9 @@ static void
> intel_ddi_post_disable_hdmi_or_sst(struct intel_atomic_state *state,
>
> intel_ddi_disable_transcoder_func(old_crtc_state);
>
> - for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, pipe_crtc,
> -
> intel_crtc_joined_pipe_mask(old_crtc_state)) {
> + for_each_intel_crtc_in_mask_priority(dev_priv, pipe_crtc, pipe,
> +
> intel_crtc_joined_pipe_mask(old_crtc_state),
> +
> intel_get_pipe_order_disable(old_crtc_state)) {
> const struct intel_crtc_state *old_pipe_crtc_state =
> intel_atomic_get_old_crtc_state(state, pipe_crtc);
>
> @@ -3383,7 +3385,7 @@ static void intel_enable_ddi(struct intel_atomic_state
> *state,
> const struct drm_connector_state *conn_state) {
> struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> - struct intel_crtc *pipe_crtc;
> + struct intel_crtc *pipe_crtc; enum pipe pipe;
>
Ditto
> intel_ddi_enable_transcoder_func(encoder, crtc_state);
>
> @@ -3394,8 +3396,9 @@ static void intel_enable_ddi(struct intel_atomic_state
> *state,
>
> intel_ddi_wait_for_fec_status(encoder, crtc_state, true);
>
> - for_each_intel_crtc_in_pipe_mask_reverse(&i915->drm, pipe_crtc,
> -
> intel_crtc_joined_pipe_mask(crtc_state)) {
> + for_each_intel_crtc_in_mask_priority(i915, pipe_crtc, pipe,
> +
> intel_crtc_joined_pipe_mask(crtc_state),
> +
> intel_get_pipe_order_enable(crtc_state)) {
> const struct intel_crtc_state *pipe_crtc_state =
> intel_atomic_get_new_crtc_state(state, pipe_crtc);
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index d032fd8011d5..b6896058a594 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -1730,6 +1730,56 @@ static void hsw_configure_cpu_transcoder(const
> struct intel_crtc_state *crtc_sta
> hsw_set_transconf(crtc_state);
> }
>
> +static
> +bool intel_is_bigjoiner(const struct intel_crtc_state *pipe_config) {
> + return hweight8(pipe_config->joiner_pipes) == 2; }
> +
Use enums here
Regards,
Suraj Kandpal
> +static
> +bool intel_is_ultrajoiner(const struct intel_crtc_state *pipe_config) {
> + return hweight8(pipe_config->joiner_pipes) == 4; }
This function was already introduced back in the 5th patch as usual bool have a look
> +
> +const enum pipe *intel_get_pipe_order_enable(const struct
> +intel_crtc_state *crtc_state) {
> + static const enum pipe
> ultrajoiner_pipe_order_enable[I915_MAX_PIPES] = {
> + PIPE_B, PIPE_D, PIPE_C, PIPE_A
> + };
> + static const enum pipe bigjoiner_pipe_order_enable[I915_MAX_PIPES]
> = {
> + PIPE_B, PIPE_A, PIPE_D, PIPE_C
> + };
> + static const enum pipe nojoiner_pipe_order_enable[I915_MAX_PIPES] =
> {
> + PIPE_A, PIPE_B, PIPE_C, PIPE_D
> + };
> +
> + if (intel_is_ultrajoiner(crtc_state))
> + return ultrajoiner_pipe_order_enable;
> + else if (intel_is_bigjoiner(crtc_state))
> + return bigjoiner_pipe_order_enable;
> + return nojoiner_pipe_order_enable;
> +}
> +
> +const enum pipe *intel_get_pipe_order_disable(const struct
> +intel_crtc_state *crtc_state) {
> + static const enum pipe
> ultrajoiner_pipe_order_disable[I915_MAX_PIPES] = {
> + PIPE_A, PIPE_B, PIPE_D, PIPE_C
> + };
> + static const enum pipe bigjoiner_pipe_order_disable[I915_MAX_PIPES]
> = {
> + PIPE_A, PIPE_B, PIPE_C, PIPE_D
> + };
> + static const enum pipe nojoiner_pipe_order_disable[I915_MAX_PIPES]
> = {
> + PIPE_A, PIPE_B, PIPE_C, PIPE_D
> + };
> +
> + if (intel_is_ultrajoiner(crtc_state))
> + return ultrajoiner_pipe_order_disable;
> + else if (intel_is_bigjoiner(crtc_state))
> + return bigjoiner_pipe_order_disable;
> + return nojoiner_pipe_order_disable;
> +}
> +
> static void hsw_crtc_enable(struct intel_atomic_state *state,
> struct intel_crtc *crtc)
> {
> @@ -1737,19 +1787,21 @@ static void hsw_crtc_enable(struct
> intel_atomic_state *state,
> intel_atomic_get_new_crtc_state(state, crtc);
> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> enum transcoder cpu_transcoder = new_crtc_state->cpu_transcoder;
> - struct intel_crtc *pipe_crtc;
> + struct intel_crtc *pipe_crtc; enum pipe pipe;
>
> if (drm_WARN_ON(&dev_priv->drm, crtc->active))
> return;
>
> - for_each_intel_crtc_in_pipe_mask_reverse(&dev_priv->drm, pipe_crtc,
> -
> intel_crtc_joined_pipe_mask(new_crtc_state))
> + for_each_intel_crtc_in_mask_priority(dev_priv, pipe_crtc, pipe,
> +
> intel_crtc_joined_pipe_mask(new_crtc_state),
> +
> intel_get_pipe_order_enable(new_crtc_state))
> intel_dmc_enable_pipe(dev_priv, pipe_crtc->pipe);
>
> intel_encoders_pre_pll_enable(state, crtc);
>
> - for_each_intel_crtc_in_pipe_mask_reverse(&dev_priv->drm, pipe_crtc,
> -
> intel_crtc_joined_pipe_mask(new_crtc_state)) {
> + for_each_intel_crtc_in_mask_priority(dev_priv, pipe_crtc, pipe,
> +
> intel_crtc_joined_pipe_mask(new_crtc_state),
> +
> intel_get_pipe_order_enable(new_crtc_state)) {
> const struct intel_crtc_state *pipe_crtc_state =
> intel_atomic_get_new_crtc_state(state, pipe_crtc);
>
> @@ -1759,8 +1811,9 @@ static void hsw_crtc_enable(struct intel_atomic_state
> *state,
>
> intel_encoders_pre_enable(state, crtc);
>
> - for_each_intel_crtc_in_pipe_mask_reverse(&dev_priv->drm, pipe_crtc,
> -
> intel_crtc_joined_pipe_mask(new_crtc_state)) {
> + for_each_intel_crtc_in_mask_priority(dev_priv, pipe_crtc, pipe,
> +
> intel_crtc_joined_pipe_mask(new_crtc_state),
> +
> intel_get_pipe_order_enable(new_crtc_state)) {
> const struct intel_crtc_state *pipe_crtc_state =
> intel_atomic_get_new_crtc_state(state, pipe_crtc);
>
> @@ -1778,8 +1831,9 @@ static void hsw_crtc_enable(struct intel_atomic_state
> *state,
> if (!transcoder_is_dsi(cpu_transcoder))
> hsw_configure_cpu_transcoder(new_crtc_state);
>
> - for_each_intel_crtc_in_pipe_mask_reverse(&dev_priv->drm, pipe_crtc,
> -
> intel_crtc_joined_pipe_mask(new_crtc_state)) {
> + for_each_intel_crtc_in_mask_priority(dev_priv, pipe_crtc, pipe,
> +
> intel_crtc_joined_pipe_mask(new_crtc_state),
> +
> intel_get_pipe_order_enable(new_crtc_state)) {
> const struct intel_crtc_state *pipe_crtc_state =
> intel_atomic_get_new_crtc_state(state, pipe_crtc);
>
> @@ -1814,8 +1868,9 @@ static void hsw_crtc_enable(struct intel_atomic_state
> *state,
>
> intel_encoders_enable(state, crtc);
>
> - for_each_intel_crtc_in_pipe_mask_reverse(&dev_priv->drm, pipe_crtc,
> -
> intel_crtc_joined_pipe_mask(new_crtc_state)) {
> + for_each_intel_crtc_in_mask_priority(dev_priv, pipe_crtc, pipe,
> +
> intel_crtc_joined_pipe_mask(new_crtc_state),
> +
> intel_get_pipe_order_enable(new_crtc_state)) {
> const struct intel_crtc_state *pipe_crtc_state =
> intel_atomic_get_new_crtc_state(state, pipe_crtc);
> enum pipe hsw_workaround_pipe;
> @@ -1900,7 +1955,7 @@ static void hsw_crtc_disable(struct
> intel_atomic_state *state,
> const struct intel_crtc_state *old_crtc_state =
> intel_atomic_get_old_crtc_state(state, crtc);
> struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> - struct intel_crtc *pipe_crtc;
> + struct intel_crtc *pipe_crtc; enum pipe pipe;
>
> /*
> * FIXME collapse everything to one hook.
> @@ -1909,8 +1964,9 @@ static void hsw_crtc_disable(struct
> intel_atomic_state *state,
> intel_encoders_disable(state, crtc);
> intel_encoders_post_disable(state, crtc);
>
> - for_each_intel_crtc_in_pipe_mask(&i915->drm, pipe_crtc,
> -
> intel_crtc_joined_pipe_mask(old_crtc_state)) {
> + for_each_intel_crtc_in_mask_priority(i915, pipe_crtc, pipe,
> +
> intel_crtc_joined_pipe_mask(old_crtc_state),
> +
> intel_get_pipe_order_disable(old_crtc_state)) {
> const struct intel_crtc_state *old_pipe_crtc_state =
> intel_atomic_get_old_crtc_state(state, pipe_crtc);
>
> @@ -1919,8 +1975,9 @@ static void hsw_crtc_disable(struct
> intel_atomic_state *state,
>
> intel_encoders_post_pll_disable(state, crtc);
>
> - for_each_intel_crtc_in_pipe_mask(&i915->drm, pipe_crtc,
> -
> intel_crtc_joined_pipe_mask(old_crtc_state))
> + for_each_intel_crtc_in_mask_priority(i915, pipe_crtc, pipe,
> +
> intel_crtc_joined_pipe_mask(old_crtc_state),
> +
> intel_get_pipe_order_disable(old_crtc_state))
> intel_dmc_disable_pipe(i915, pipe_crtc->pipe); }
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.h
> b/drivers/gpu/drm/i915/display/intel_display.h
> index 35e68e4cc712..4cfd1da0bbc0 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.h
> +++ b/drivers/gpu/drm/i915/display/intel_display.h
> @@ -275,6 +275,11 @@ enum phy_fia {
> &(dev)->mode_config.crtc_list, \
> base.head)
>
> +#define for_each_intel_crtc_in_mask_priority(__dev_priv, intel_crtc, __p,
> __mask, __priolist) \
> + for_each_pipe(__dev_priv, __p) \
> + for_each_if((__mask) & BIT(__priolist[__p])) \
> + for_each_if(intel_crtc = intel_crtc_for_pipe(__dev_priv,
> +__priolist[__p]))
> +
> #define for_each_intel_crtc_in_pipe_mask(dev, intel_crtc, pipe_mask) \
> list_for_each_entry(intel_crtc, \
> &(dev)->mode_config.crtc_list, \
> @@ -432,6 +437,8 @@ bool intel_crtc_is_ultrajoiner(const struct
> intel_crtc_state *crtc_state); bool intel_crtc_is_ultrajoiner_primary(const struct
> intel_crtc_state *crtc_state);
> u8 intel_crtc_joiner_secondary_pipes(const struct intel_crtc_state *crtc_state);
> struct intel_crtc *intel_joiner_primary_crtc(const struct intel_crtc_state
> *crtc_state);
> +const enum pipe *intel_get_pipe_order_enable(const struct
> +intel_crtc_state *crtc_state); const enum pipe
> +*intel_get_pipe_order_disable(const struct intel_crtc_state
> +*crtc_state);
> bool intel_crtc_get_pipe_config(struct intel_crtc_state *crtc_state); bool
> intel_pipe_config_compare(const struct intel_crtc_state *current_config,
> const struct intel_crtc_state *pipe_config, diff --git
> a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 21b23f8eb5e7..d4fc4439ce2b 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -1007,7 +1007,7 @@ static void intel_mst_post_disable_dp(struct
> intel_atomic_state *state,
> struct drm_dp_mst_atomic_payload *new_payload =
> drm_atomic_get_mst_payload_state(new_mst_state, connector-
> >port);
> struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> - struct intel_crtc *pipe_crtc;
> + struct intel_crtc *pipe_crtc; enum pipe pipe;
> bool last_mst_stream;
>
> intel_dp->active_mst_links--;
> @@ -1016,8 +1016,9 @@ static void intel_mst_post_disable_dp(struct
> intel_atomic_state *state,
> DISPLAY_VER(dev_priv) >= 12 && last_mst_stream &&
> !intel_dp_mst_is_master_trans(old_crtc_state));
>
> - for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, pipe_crtc,
> -
> intel_crtc_joined_pipe_mask(old_crtc_state)) {
> + for_each_intel_crtc_in_mask_priority(dev_priv, pipe_crtc, pipe,
> +
> intel_crtc_joined_pipe_mask(old_crtc_state),
> +
> intel_get_pipe_order_disable(old_crtc_state)) {
> const struct intel_crtc_state *old_pipe_crtc_state =
> intel_atomic_get_old_crtc_state(state, pipe_crtc);
>
> @@ -1041,8 +1042,9 @@ static void intel_mst_post_disable_dp(struct
> intel_atomic_state *state,
>
> intel_ddi_disable_transcoder_func(old_crtc_state);
>
> - for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, pipe_crtc,
> -
> intel_crtc_joined_pipe_mask(old_crtc_state)) {
> + for_each_intel_crtc_in_mask_priority(dev_priv, pipe_crtc, pipe,
> +
> intel_crtc_joined_pipe_mask(old_crtc_state),
> +
> intel_get_pipe_order_disable(old_crtc_state)) {
> const struct intel_crtc_state *old_pipe_crtc_state =
> intel_atomic_get_old_crtc_state(state, pipe_crtc);
>
> @@ -1231,7 +1233,7 @@ static void intel_mst_enable_dp(struct
> intel_atomic_state *state,
> drm_atomic_get_new_mst_topology_state(&state->base,
> &intel_dp->mst_mgr);
> enum transcoder trans = pipe_config->cpu_transcoder;
> bool first_mst_stream = intel_dp->active_mst_links == 1;
> - struct intel_crtc *pipe_crtc;
> + struct intel_crtc *pipe_crtc; enum pipe pipe;
>
> drm_WARN_ON(&dev_priv->drm, pipe_config->has_pch_encoder);
>
> @@ -1275,8 +1277,9 @@ static void intel_mst_enable_dp(struct
> intel_atomic_state *state,
>
> intel_enable_transcoder(pipe_config);
>
> - for_each_intel_crtc_in_pipe_mask_reverse(&dev_priv->drm, pipe_crtc,
> -
> intel_crtc_joined_pipe_mask(pipe_config)) {
> + for_each_intel_crtc_in_mask_priority(dev_priv, pipe_crtc, pipe,
> +
> intel_crtc_joined_pipe_mask(pipe_config),
> +
> intel_get_pipe_order_enable(pipe_config)) {
> const struct intel_crtc_state *pipe_crtc_state =
> intel_atomic_get_new_crtc_state(state, pipe_crtc);
>
> --
> 2.45.2
