Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Jani Nikula <jani.nikula@intel.com>
Cc: intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org
Subject: Re: [RESEND 6/6] drm/i915/display: stop passing i to for_each_pipe_crtc_modeset_{enable, disable}()
Date: Wed, 13 May 2026 15:23:04 +0300	[thread overview]
Message-ID: <agRtKLDoarnWPChi@intel.com> (raw)
In-Reply-To: <2270d4a10663bb55d5b16902b02798234f440517.1778659089.git.jani.nikula@intel.com>

On Wed, May 13, 2026 at 10:58:40AM +0300, Jani Nikula wrote:
> Refactor for_each_pipe_crtc_modeset_{enable,disable}() and their
> underlying for_each_crtc_in_masks{,_reverse}() helpers to utilize
> __UNIQUE_ID() to avoid having to pass the for loop variable to them.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c     |  8 ++----
>  drivers/gpu/drm/i915/display/intel_display.c | 12 ++++----
>  drivers/gpu/drm/i915/display/intel_display.h | 30 ++++++++++----------
>  drivers/gpu/drm/i915/display/intel_dp_mst.c  |  9 +++---
>  4 files changed, 27 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 34e2b5ea9d0d..1d4bb59a0501 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3230,9 +3230,8 @@ static void intel_ddi_post_disable_hdmi_or_sst(struct intel_atomic_state *state,
>  	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
>  	struct intel_crtc *pipe_crtc;
>  	bool is_hdmi = intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_HDMI);
> -	int i;
>  
> -	for_each_pipe_crtc_modeset_disable(display, pipe_crtc, old_crtc_state, i) {
> +	for_each_pipe_crtc_modeset_disable(display, pipe_crtc, old_crtc_state) {
>  		const struct intel_crtc_state *old_pipe_crtc_state =
>  			intel_atomic_get_old_crtc_state(state, pipe_crtc);
>  
> @@ -3259,7 +3258,7 @@ static void intel_ddi_post_disable_hdmi_or_sst(struct intel_atomic_state *state,
>  
>  	intel_ddi_disable_transcoder_func(old_crtc_state);
>  
> -	for_each_pipe_crtc_modeset_disable(display, pipe_crtc, old_crtc_state, i) {
> +	for_each_pipe_crtc_modeset_disable(display, pipe_crtc, old_crtc_state) {
>  		const struct intel_crtc_state *old_pipe_crtc_state =
>  			intel_atomic_get_old_crtc_state(state, pipe_crtc);
>  
> @@ -3516,7 +3515,6 @@ static void intel_ddi_enable(struct intel_atomic_state *state,
>  	struct intel_crtc *pipe_crtc;
>  	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>  	bool is_hdmi = intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI);
> -	int i;
>  
>  	/* 128b/132b SST */
>  	if (!is_hdmi && intel_dp_is_uhbr(crtc_state)) {
> @@ -3550,7 +3548,7 @@ static void intel_ddi_enable(struct intel_atomic_state *state,
>  
>  	intel_ddi_wait_for_fec_status(encoder, crtc_state, true);
>  
> -	for_each_pipe_crtc_modeset_enable(display, pipe_crtc, crtc_state, i) {
> +	for_each_pipe_crtc_modeset_enable(display, pipe_crtc, 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 cf9afc90e301..ce5a3d5dbb79 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -1653,11 +1653,10 @@ static void hsw_crtc_enable(struct intel_atomic_state *state,
>  		intel_atomic_get_new_crtc_state(state, crtc);
>  	enum transcoder cpu_transcoder = new_crtc_state->cpu_transcoder;
>  	struct intel_crtc *pipe_crtc;
> -	int i;
>  
>  	if (drm_WARN_ON(display->drm, crtc->active))
>  		return;
> -	for_each_pipe_crtc_modeset_enable(display, pipe_crtc, new_crtc_state, i) {
> +	for_each_pipe_crtc_modeset_enable(display, pipe_crtc, new_crtc_state) {
>  		const struct intel_crtc_state *new_pipe_crtc_state =
>  			intel_atomic_get_new_crtc_state(state, pipe_crtc);
>  
> @@ -1671,7 +1670,7 @@ static void hsw_crtc_enable(struct intel_atomic_state *state,
>  
>  	intel_encoders_pre_enable(state, crtc);
>  
> -	for_each_pipe_crtc_modeset_enable(display, pipe_crtc, new_crtc_state, i) {
> +	for_each_pipe_crtc_modeset_enable(display, pipe_crtc, new_crtc_state) {
>  		const struct intel_crtc_state *pipe_crtc_state =
>  			intel_atomic_get_new_crtc_state(state, pipe_crtc);
>  
> @@ -1689,7 +1688,7 @@ 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_pipe_crtc_modeset_enable(display, pipe_crtc, new_crtc_state, i) {
> +	for_each_pipe_crtc_modeset_enable(display, pipe_crtc, new_crtc_state) {
>  		const struct intel_crtc_state *pipe_crtc_state =
>  			intel_atomic_get_new_crtc_state(state, pipe_crtc);
>  
> @@ -1719,7 +1718,7 @@ static void hsw_crtc_enable(struct intel_atomic_state *state,
>  
>  	intel_encoders_enable(state, crtc);
>  
> -	for_each_pipe_crtc_modeset_enable(display, pipe_crtc, new_crtc_state, i) {
> +	for_each_pipe_crtc_modeset_enable(display, pipe_crtc, 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;
> @@ -1787,7 +1786,6 @@ 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 intel_crtc *pipe_crtc;
> -	int i;
>  
>  	/*
>  	 * FIXME collapse everything to one hook.
> @@ -1800,7 +1798,7 @@ static void hsw_crtc_disable(struct intel_atomic_state *state,
>  
>  	intel_encoders_post_pll_disable(state, crtc);
>  
> -	for_each_pipe_crtc_modeset_disable(display, pipe_crtc, old_crtc_state, i) {
> +	for_each_pipe_crtc_modeset_disable(display, pipe_crtc, old_crtc_state) {
>  		const struct intel_crtc_state *old_pipe_crtc_state =
>  			intel_atomic_get_old_crtc_state(state, pipe_crtc);
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
> index 7d4f83ad9412..31588011d659 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.h
> +++ b/drivers/gpu/drm/i915/display/intel_display.h
> @@ -325,29 +325,29 @@ enum phy_fia {
>  			     ((connector) = to_intel_connector((__state)->base.connectors[__i].ptr), \
>  			     (new_connector_state) = to_intel_digital_connector_state((__state)->base.connectors[__i].new_state), 1))
>  
> -#define for_each_crtc_in_masks(display, crtc, first_pipes, second_pipes, i) \
> -	for ((i) = 0; \
> +#define __for_each_crtc_in_masks(display, crtc, first_pipes, second_pipes, i) \
> +	for (int (i) = 0; \
>  	     (i) < (I915_MAX_PIPES * 2) && ((crtc) = intel_crtc_for_pipe(display, (i) % I915_MAX_PIPES), 1); \
>  	     (i)++) \
>  		for_each_if((crtc) && ((first_pipes) | ((second_pipes) << I915_MAX_PIPES)) & BIT(i))
>  
> -#define for_each_crtc_in_masks_reverse(display, crtc, first_pipes, second_pipes, i) \
> -	for ((i) = (I915_MAX_PIPES * 2 - 1); \
> +#define __for_each_crtc_in_masks_reverse(display, crtc, first_pipes, second_pipes, i) \
> +	for (int (i) = (I915_MAX_PIPES * 2 - 1); \
>  	     (i) >= 0 && ((crtc) = intel_crtc_for_pipe(display, (i) % I915_MAX_PIPES), 1); \
>  	     (i)--) \
>  		for_each_if((crtc) && ((first_pipes) | ((second_pipes) << I915_MAX_PIPES)) & BIT(i))
>  
> -#define for_each_pipe_crtc_modeset_disable(display, crtc, crtc_state, i) \
> -	for_each_crtc_in_masks(display, crtc, \
> -			       _intel_modeset_primary_pipes(crtc_state), \
> -			       _intel_modeset_secondary_pipes(crtc_state), \
> -			       i)
> -
> -#define for_each_pipe_crtc_modeset_enable(display, crtc, crtc_state, i) \
> -	for_each_crtc_in_masks_reverse(display, crtc, \
> -				       _intel_modeset_primary_pipes(crtc_state), \
> -				       _intel_modeset_secondary_pipes(crtc_state), \
> -				       i)
> +#define for_each_pipe_crtc_modeset_disable(display, crtc, crtc_state) \
> +	__for_each_crtc_in_masks(display, crtc, \
> +				 _intel_modeset_primary_pipes(crtc_state), \
> +				 _intel_modeset_secondary_pipes(crtc_state), \
> +				 __UNIQUE_ID(i))
> +
> +#define for_each_pipe_crtc_modeset_enable(display, crtc, crtc_state) \
> +	__for_each_crtc_in_masks_reverse(display, crtc,			\
> +					 _intel_modeset_primary_pipes(crtc_state), \
> +					 _intel_modeset_secondary_pipes(crtc_state), \
> +					 __UNIQUE_ID(i))
>  
>  int intel_atomic_check(struct drm_device *dev, struct drm_atomic_commit *state);
>  u8 intel_calc_enabled_pipes(struct intel_atomic_state *state,
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index be8febe3d234..724d3ee23350 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -1065,14 +1065,13 @@ static void mst_stream_post_disable(struct intel_atomic_state *state,
>  		drm_atomic_get_mst_payload_state(new_mst_state, connector->mst.port);
>  	struct intel_crtc *pipe_crtc;
>  	bool last_mst_stream;
> -	int i;
>  
>  	last_mst_stream = intel_dp_mst_dec_active_streams(intel_dp);
>  
>  	drm_WARN_ON(display->drm, DISPLAY_VER(display) >= 12 && last_mst_stream &&
>  		    !intel_dp_mst_is_master_trans(old_crtc_state));
>  
> -	for_each_pipe_crtc_modeset_disable(display, pipe_crtc, old_crtc_state, i) {
> +	for_each_pipe_crtc_modeset_disable(display, pipe_crtc, old_crtc_state) {
>  		const struct intel_crtc_state *old_pipe_crtc_state =
>  			intel_atomic_get_old_crtc_state(state, pipe_crtc);
>  
> @@ -1099,7 +1098,7 @@ static void mst_stream_post_disable(struct intel_atomic_state *state,
>  
>  	intel_ddi_disable_transcoder_func(old_crtc_state);
>  
> -	for_each_pipe_crtc_modeset_disable(display, pipe_crtc, old_crtc_state, i) {
> +	for_each_pipe_crtc_modeset_disable(display, pipe_crtc, old_crtc_state) {
>  		const struct intel_crtc_state *old_pipe_crtc_state =
>  			intel_atomic_get_old_crtc_state(state, pipe_crtc);
>  
> @@ -1310,7 +1309,7 @@ static void mst_stream_enable(struct intel_atomic_state *state,
>  	enum transcoder trans = pipe_config->cpu_transcoder;
>  	bool first_mst_stream = intel_dp_mst_active_streams(intel_dp) == 1;
>  	struct intel_crtc *pipe_crtc;
> -	int ret, i;
> +	int ret;
>  
>  	drm_WARN_ON(display->drm, pipe_config->has_pch_encoder);
>  
> @@ -1355,7 +1354,7 @@ static void mst_stream_enable(struct intel_atomic_state *state,
>  
>  	intel_enable_transcoder(pipe_config);
>  
> -	for_each_pipe_crtc_modeset_enable(display, pipe_crtc, pipe_config, i) {
> +	for_each_pipe_crtc_modeset_enable(display, pipe_crtc, pipe_config) {
>  		const struct intel_crtc_state *pipe_crtc_state =
>  			intel_atomic_get_new_crtc_state(state, pipe_crtc);
>  
> -- 
> 2.47.3

-- 
Ville Syrjälä
Intel

  reply	other threads:[~2026-05-13 12:23 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-13  7:58 [RESEND 0/6] drm/i915: crtc iteration cleanups Jani Nikula
2026-05-13  7:58 ` [RESEND 1/6] drm/{i915, xe}: move xe_display_flush_cleanup_work() to i915 display Jani Nikula
2026-05-13 12:09   ` Ville Syrjälä
2026-05-13 14:19     ` Jani Nikula
2026-05-13 14:56       ` Imre Deak
2026-05-13  7:58 ` [RESEND 2/6] drm/i915/display: switch from drm_for_each_crtc() to for_each_intel_crtc() Jani Nikula
2026-05-13  7:58 ` [RESEND 3/6] drm/i915/display: always pass display->drm to for_each_intel_crtc*() Jani Nikula
2026-05-13 12:22   ` Ville Syrjälä
2026-05-13  7:58 ` [RESEND 4/6] drm/i915/display: pass struct intel_display to all for_each_intel_crtc*() macros Jani Nikula
2026-05-13 12:22   ` Ville Syrjälä
2026-05-13  7:58 ` [RESEND 5/6] drm/i915/display: stop passing i to for_each_*_intel_crtc_in_state() macros Jani Nikula
2026-05-13 12:22   ` Ville Syrjälä
2026-05-13  7:58 ` [RESEND 6/6] drm/i915/display: stop passing i to for_each_pipe_crtc_modeset_{enable, disable}() Jani Nikula
2026-05-13 12:23   ` Ville Syrjälä [this message]
2026-05-13  8:59 ` ✗ i915.CI.BAT: failure for drm/i915: crtc iteration cleanups (rev2) Patchwork
2026-05-13  8:59 ` Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=agRtKLDoarnWPChi@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=jani.nikula@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox