All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Anshuman Gupta <anshuman.gupta@intel.com>
Cc: jani.nikula@intel.com, intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH v2 2/7] drm/i915: Remove (pipe == crtc->index) assumption
Date: Thu, 20 Feb 2020 19:12:09 +0200	[thread overview]
Message-ID: <20200220171209.GE13686@intel.com> (raw)
In-Reply-To: <20200211172532.14287-3-anshuman.gupta@intel.com>

On Tue, Feb 11, 2020 at 10:55:27PM +0530, Anshuman Gupta wrote:
> we can't have (pipe == crtc->index) assumption in
> driver in order to support 3 non-contiguous
> display pipe system.
> 
> FIXME: Remove the WARN_ON(drm_crtc_index(&crtc->base) != crtc->pipe)
> when we will fix all such assumption.
> 
> changes since RFC:
> - Added again removed (pipe == crtc->index) WARN_ON.
> - Pass drm_crtc_index instead of intel pipe in order to
>   call drm_handle_vblank().
> 
> v2:
> - used drm_crtc_handle_vblank()/drm_crtc_wait_one_vblank()
>   instead of drm_handle_vblank/drm_wait_one_vblank(). [Jani]
> - introduced intel_handle_vblank() helper to avoid sprinkle
>   of intel_crtc across irq_handlers. [Ville]
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c       |  8 ++++----
>  drivers/gpu/drm/i915/display/intel_display_types.h | 14 +++++++++++++-
>  drivers/gpu/drm/i915/i915_irq.c                    | 14 +++++++-------
>  3 files changed, 24 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 80eebdc4c670..5333f7a7db42 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -14395,11 +14395,11 @@ verify_single_dpll_state(struct drm_i915_private *dev_priv,
>  	if (new_crtc_state->hw.active)
>  		I915_STATE_WARN(!(pll->active_mask & crtc_mask),
>  				"pll active mismatch (expected pipe %c in active mask 0x%02x)\n",
> -				pipe_name(drm_crtc_index(&crtc->base)), pll->active_mask);
> +				pipe_name(crtc->pipe), pll->active_mask);
>  	else
>  		I915_STATE_WARN(pll->active_mask & crtc_mask,
>  				"pll active mismatch (didn't expect pipe %c in active mask 0x%02x)\n",
> -				pipe_name(drm_crtc_index(&crtc->base)), pll->active_mask);
> +				pipe_name(crtc->pipe), pll->active_mask);
>  
>  	I915_STATE_WARN(!(pll->state.crtc_mask & crtc_mask),
>  			"pll enabled crtcs mismatch (expected 0x%x in 0x%02x)\n",
> @@ -14428,10 +14428,10 @@ verify_shared_dpll_state(struct intel_crtc *crtc,
>  
>  		I915_STATE_WARN(pll->active_mask & crtc_mask,
>  				"pll active mismatch (didn't expect pipe %c in active mask)\n",
> -				pipe_name(drm_crtc_index(&crtc->base)));
> +				pipe_name(crtc->pipe));
>  		I915_STATE_WARN(pll->state.crtc_mask & crtc_mask,
>  				"pll enabled crtcs mismatch (found %x in enabled mask)\n",
> -				pipe_name(drm_crtc_index(&crtc->base)));
> +				pipe_name(crtc->pipe));
>  	}
>  }
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 283c622f8ba1..14e3d78fef7c 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1595,11 +1595,23 @@ intel_crtc_has_dp_encoder(const struct intel_crtc_state *crtc_state)
>  		 (1 << INTEL_OUTPUT_DP_MST) |
>  		 (1 << INTEL_OUTPUT_EDP));
>  }
> +
>  static inline void
>  intel_wait_for_vblank(struct drm_i915_private *dev_priv, enum pipe pipe)
>  {
> -	drm_wait_one_vblank(&dev_priv->drm, pipe);
> +	struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
> +
> +	drm_crtc_wait_one_vblank(&crtc->base);
> +}
> +
> +static inline void
> +intel_handle_vblank(struct drm_i915_private *dev_priv, enum pipe pipe)
> +{
> +	struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
> +
> +	drm_crtc_handle_vblank(&crtc->base);
>  }

There's no reason to put that into a header. Just put it into
i915_irq.c. With that

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

> +
>  static inline void
>  intel_wait_for_vblank_if_active(struct drm_i915_private *dev_priv, enum pipe pipe)
>  {
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index a26f2bf1b6ea..bfd3b34f2be3 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1364,7 +1364,7 @@ static void i8xx_pipestat_irq_handler(struct drm_i915_private *dev_priv,
>  
>  	for_each_pipe(dev_priv, pipe) {
>  		if (pipe_stats[pipe] & PIPE_VBLANK_INTERRUPT_STATUS)
> -			drm_handle_vblank(&dev_priv->drm, pipe);
> +			intel_handle_vblank(dev_priv, pipe);
>  
>  		if (pipe_stats[pipe] & PIPE_CRC_DONE_INTERRUPT_STATUS)
>  			i9xx_pipe_crc_irq_handler(dev_priv, pipe);
> @@ -1382,7 +1382,7 @@ static void i915_pipestat_irq_handler(struct drm_i915_private *dev_priv,
>  
>  	for_each_pipe(dev_priv, pipe) {
>  		if (pipe_stats[pipe] & PIPE_VBLANK_INTERRUPT_STATUS)
> -			drm_handle_vblank(&dev_priv->drm, pipe);
> +			intel_handle_vblank(dev_priv, pipe);
>  
>  		if (pipe_stats[pipe] & PIPE_LEGACY_BLC_EVENT_STATUS)
>  			blc_event = true;
> @@ -1406,7 +1406,7 @@ static void i965_pipestat_irq_handler(struct drm_i915_private *dev_priv,
>  
>  	for_each_pipe(dev_priv, pipe) {
>  		if (pipe_stats[pipe] & PIPE_START_VBLANK_INTERRUPT_STATUS)
> -			drm_handle_vblank(&dev_priv->drm, pipe);
> +			intel_handle_vblank(dev_priv, pipe);
>  
>  		if (pipe_stats[pipe] & PIPE_LEGACY_BLC_EVENT_STATUS)
>  			blc_event = true;
> @@ -1432,7 +1432,7 @@ static void valleyview_pipestat_irq_handler(struct drm_i915_private *dev_priv,
>  
>  	for_each_pipe(dev_priv, pipe) {
>  		if (pipe_stats[pipe] & PIPE_START_VBLANK_INTERRUPT_STATUS)
> -			drm_handle_vblank(&dev_priv->drm, pipe);
> +			intel_handle_vblank(dev_priv, pipe);
>  
>  		if (pipe_stats[pipe] & PIPE_CRC_DONE_INTERRUPT_STATUS)
>  			i9xx_pipe_crc_irq_handler(dev_priv, pipe);
> @@ -1970,7 +1970,7 @@ static void ilk_display_irq_handler(struct drm_i915_private *dev_priv,
>  
>  	for_each_pipe(dev_priv, pipe) {
>  		if (de_iir & DE_PIPE_VBLANK(pipe))
> -			drm_handle_vblank(&dev_priv->drm, pipe);
> +			intel_handle_vblank(dev_priv, pipe);
>  
>  		if (de_iir & DE_PIPE_FIFO_UNDERRUN(pipe))
>  			intel_cpu_fifo_underrun_irq_handler(dev_priv, pipe);
> @@ -2023,7 +2023,7 @@ static void ivb_display_irq_handler(struct drm_i915_private *dev_priv,
>  
>  	for_each_pipe(dev_priv, pipe) {
>  		if (de_iir & (DE_PIPE_VBLANK_IVB(pipe)))
> -			drm_handle_vblank(&dev_priv->drm, pipe);
> +			intel_handle_vblank(dev_priv, pipe);
>  	}
>  
>  	/* check event from PCH */
> @@ -2336,7 +2336,7 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
>  		I915_WRITE(GEN8_DE_PIPE_IIR(pipe), iir);
>  
>  		if (iir & GEN8_PIPE_VBLANK)
> -			drm_handle_vblank(&dev_priv->drm, pipe);
> +			intel_handle_vblank(dev_priv, pipe);
>  
>  		if (iir & GEN8_PIPE_CDCLK_CRC_DONE)
>  			hsw_pipe_crc_irq_handler(dev_priv, pipe);
> -- 
> 2.24.0

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2020-02-20 17:12 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-11 17:25 [Intel-gfx] [PATCH v2 0/7] 3 display pipes combination system support Anshuman Gupta
2020-02-11 17:25 ` [Intel-gfx] [PATCH v2 1/7] drm/i915: Iterate over pipe and skip the disabled one Anshuman Gupta
2020-02-12 13:50   ` Ville Syrjälä
2020-02-18 16:41     ` Anshuman Gupta
2020-02-11 17:25 ` [Intel-gfx] [PATCH v2 2/7] drm/i915: Remove (pipe == crtc->index) assumption Anshuman Gupta
2020-02-20 17:12   ` Ville Syrjälä [this message]
2020-02-11 17:25 ` [Intel-gfx] [PATCH v2 3/7] drm/i915: Fix broken transcoder err state Anshuman Gupta
2020-02-20 17:16   ` Ville Syrjälä
2020-02-20 20:02     ` Ville Syrjälä
2020-02-11 17:25 ` [Intel-gfx] [PATCH v2 4/7] drm/i915: Fix wrongly populated plane possible_crtcs bit mask Anshuman Gupta
2020-02-20 17:17   ` Ville Syrjälä
2020-02-11 17:25 ` [Intel-gfx] [PATCH v2 5/7] drm/i915: Get first crtc instead of PIPE_A crtc Anshuman Gupta
2020-02-17  5:22   ` Anshuman Gupta
2020-02-17 14:17     ` Ville Syrjälä
2020-02-18 11:57       ` Anshuman Gupta
2020-02-11 17:25 ` [Intel-gfx] [PATCH v2 6/7] drm/i915: Add WARN_ON in intel_get_crtc_for_pipe() Anshuman Gupta
2020-02-11 17:25 ` [Intel-gfx] [PATCH v2 7/7] drm/i915: Fix broken num_entries in skl_ddb_allocation_overlaps Anshuman Gupta
2020-02-20 17:18   ` Ville Syrjälä
2020-02-12 20:31 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for 3 display pipes combination system support (rev3) Patchwork
2020-02-12 21:16 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-02-15 14:16 ` [Intel-gfx] ✓ Fi.CI.IGT: " 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=20200220171209.GE13686@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=anshuman.gupta@intel.com \
    --cc=intel-gfx@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.