Re: [Intel-gfx] [PATCH 1/2] drm/i915: Extract intel_sanitize_fifo_underrun_reporting()
On Thu, Jun 16, 2022 at 01:52:38PM +0300, Jani Nikula wrote: > On Wed, 15 Jun 2022, Ville Syrjala wrote: > > From: Ville Syrjälä > > > > Pull the underrun status sanitation into its own helper. > > > > Signed-off-by: Ville Syrjälä > > On the series, > > Reviewed-by: Jani Nikula Thanks. > > I'll respin my state readout extraction on top of this once you've > merged. Pushed now. -- Ville Syrjälä Intel
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Extract intel_sanitize_fifo_underrun_reporting()
On Wed, 15 Jun 2022, Ville Syrjala wrote:
> From: Ville Syrjälä
>
> Pull the underrun status sanitation into its own helper.
>
> Signed-off-by: Ville Syrjälä
On the series,
Reviewed-by: Jani Nikula
I'll respin my state readout extraction on top of this once you've
merged.
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 65 +++-
> 1 file changed, 37 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 7d9c8aeef686..e38d0a85889b 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -9897,11 +9897,46 @@ static struct intel_connector
> *intel_encoder_find_connector(struct intel_encoder
> return NULL;
> }
>
> +static void intel_sanitize_fifo_underrun_reporting(const struct
> intel_crtc_state *crtc_state)
> +{
> + struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> + struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> +
> + if (!crtc_state->hw.active && !HAS_GMCH(i915))
> + return;
> +
> + /*
> + * We start out with underrun reporting disabled to avoid races.
> + * For correct bookkeeping mark this on active crtcs.
> + *
> + * Also on gmch platforms we dont have any hardware bits to
> + * disable the underrun reporting. Which means we need to start
> + * out with underrun reporting disabled also on inactive pipes,
> + * since otherwise we'll complain about the garbage we read when
> + * e.g. coming up after runtime pm.
> + *
> + * No protection against concurrent access is required - at
> + * worst a fifo underrun happens which also sets this to false.
> + */
> + crtc->cpu_fifo_underrun_disabled = true;
> +
> + /*
> + * We track the PCH trancoder underrun reporting state
> + * within the crtc. With crtc for pipe A housing the underrun
> + * reporting state for PCH transcoder A, crtc for pipe B housing
> + * it for PCH transcoder B, etc. LPT-H has only PCH transcoder A,
> + * and marking underrun reporting as disabled for the non-existing
> + * PCH transcoders B and C would prevent enabling the south
> + * error interrupt (see cpt_can_enable_serr_int()).
> + */
> + if (intel_has_pch_trancoder(i915, crtc->pipe))
> + crtc->pch_fifo_underrun_disabled = true;
> +}
> +
> static void intel_sanitize_crtc(struct intel_crtc *crtc,
> struct drm_modeset_acquire_ctx *ctx)
> {
> struct drm_device *dev = crtc->base.dev;
> - struct drm_i915_private *dev_priv = to_i915(dev);
> struct intel_crtc_state *crtc_state =
> to_intel_crtc_state(crtc->base.state);
>
> if (crtc_state->hw.active) {
> @@ -9928,33 +9963,7 @@ static void intel_sanitize_crtc(struct intel_crtc
> *crtc,
> !intel_crtc_is_bigjoiner_slave(crtc_state))
> intel_crtc_disable_noatomic(crtc, ctx);
>
> - if (crtc_state->hw.active || HAS_GMCH(dev_priv)) {
> - /*
> - * We start out with underrun reporting disabled to avoid races.
> - * For correct bookkeeping mark this on active crtcs.
> - *
> - * Also on gmch platforms we dont have any hardware bits to
> - * disable the underrun reporting. Which means we need to start
> - * out with underrun reporting disabled also on inactive pipes,
> - * since otherwise we'll complain about the garbage we read when
> - * e.g. coming up after runtime pm.
> - *
> - * No protection against concurrent access is required - at
> - * worst a fifo underrun happens which also sets this to false.
> - */
> - crtc->cpu_fifo_underrun_disabled = true;
> - /*
> - * We track the PCH trancoder underrun reporting state
> - * within the crtc. With crtc for pipe A housing the underrun
> - * reporting state for PCH transcoder A, crtc for pipe B housing
> - * it for PCH transcoder B, etc. LPT-H has only PCH transcoder
> A,
> - * and marking underrun reporting as disabled for the
> non-existing
> - * PCH transcoders B and C would prevent enabling the south
> - * error interrupt (see cpt_can_enable_serr_int()).
> - */
> - if (intel_has_pch_trancoder(dev_priv, crtc->pipe))
> - crtc->pch_fifo_underrun_disabled = true;
> - }
> + intel_sanitize_fifo_underrun_reporting(crtc_state);
> }
>
> static bool has_bogus_dpll_config(const struct intel_crtc_state *crtc_state)
--
Jani Nikula, Intel Open Source Graphics Center
[Intel-gfx] [PATCH 1/2] drm/i915: Extract intel_sanitize_fifo_underrun_reporting()
From: Ville Syrjälä
Pull the underrun status sanitation into its own helper.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/display/intel_display.c | 65 +++-
1 file changed, 37 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index 7d9c8aeef686..e38d0a85889b 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -9897,11 +9897,46 @@ static struct intel_connector
*intel_encoder_find_connector(struct intel_encoder
return NULL;
}
+static void intel_sanitize_fifo_underrun_reporting(const struct
intel_crtc_state *crtc_state)
+{
+ struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+ struct drm_i915_private *i915 = to_i915(crtc->base.dev);
+
+ if (!crtc_state->hw.active && !HAS_GMCH(i915))
+ return;
+
+ /*
+* We start out with underrun reporting disabled to avoid races.
+* For correct bookkeeping mark this on active crtcs.
+*
+* Also on gmch platforms we dont have any hardware bits to
+* disable the underrun reporting. Which means we need to start
+* out with underrun reporting disabled also on inactive pipes,
+* since otherwise we'll complain about the garbage we read when
+* e.g. coming up after runtime pm.
+*
+* No protection against concurrent access is required - at
+* worst a fifo underrun happens which also sets this to false.
+*/
+ crtc->cpu_fifo_underrun_disabled = true;
+
+ /*
+* We track the PCH trancoder underrun reporting state
+* within the crtc. With crtc for pipe A housing the underrun
+* reporting state for PCH transcoder A, crtc for pipe B housing
+* it for PCH transcoder B, etc. LPT-H has only PCH transcoder A,
+* and marking underrun reporting as disabled for the non-existing
+* PCH transcoders B and C would prevent enabling the south
+* error interrupt (see cpt_can_enable_serr_int()).
+*/
+ if (intel_has_pch_trancoder(i915, crtc->pipe))
+ crtc->pch_fifo_underrun_disabled = true;
+}
+
static void intel_sanitize_crtc(struct intel_crtc *crtc,
struct drm_modeset_acquire_ctx *ctx)
{
struct drm_device *dev = crtc->base.dev;
- struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_crtc_state *crtc_state =
to_intel_crtc_state(crtc->base.state);
if (crtc_state->hw.active) {
@@ -9928,33 +9963,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc,
!intel_crtc_is_bigjoiner_slave(crtc_state))
intel_crtc_disable_noatomic(crtc, ctx);
- if (crtc_state->hw.active || HAS_GMCH(dev_priv)) {
- /*
-* We start out with underrun reporting disabled to avoid races.
-* For correct bookkeeping mark this on active crtcs.
-*
-* Also on gmch platforms we dont have any hardware bits to
-* disable the underrun reporting. Which means we need to start
-* out with underrun reporting disabled also on inactive pipes,
-* since otherwise we'll complain about the garbage we read when
-* e.g. coming up after runtime pm.
-*
-* No protection against concurrent access is required - at
-* worst a fifo underrun happens which also sets this to false.
-*/
- crtc->cpu_fifo_underrun_disabled = true;
- /*
-* We track the PCH trancoder underrun reporting state
-* within the crtc. With crtc for pipe A housing the underrun
-* reporting state for PCH transcoder A, crtc for pipe B housing
-* it for PCH transcoder B, etc. LPT-H has only PCH transcoder
A,
-* and marking underrun reporting as disabled for the
non-existing
-* PCH transcoders B and C would prevent enabling the south
-* error interrupt (see cpt_can_enable_serr_int()).
-*/
- if (intel_has_pch_trancoder(dev_priv, crtc->pipe))
- crtc->pch_fifo_underrun_disabled = true;
- }
+ intel_sanitize_fifo_underrun_reporting(crtc_state);
}
static bool has_bogus_dpll_config(const struct intel_crtc_state *crtc_state)
--
2.35.1
