From: Jani Nikula <jani.nikula@linux.intel.com>
To: Ville Syrjala <ville.syrjala@linux.intel.com>,
intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 1/4] drm/i915: Move framestart_delay to crtc_state
Date: Fri, 04 Mar 2022 13:08:55 +0200 [thread overview]
Message-ID: <87bkym3rg8.fsf@intel.com> (raw)
In-Reply-To: <20220221110356.5532-1-ville.syrjala@linux.intel.com>
On Mon, 21 Feb 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> We need to make framestart_delay dynamic for DRRS on PCH
> ports. To that end move it into the crtc state. As a bonus
> we get state check+dump for it. Will also allow us to get
> rid of the somewhat questionable framestart_delay sanitation
> code.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
On the series,
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 40 ++++++++++++++-----
> .../drm/i915/display/intel_display_types.h | 2 +
> .../gpu/drm/i915/display/intel_pch_display.c | 15 +++----
> drivers/gpu/drm/i915/display/intel_vrr.c | 4 +-
> drivers/gpu/drm/i915/i915_drv.h | 2 -
> 5 files changed, 42 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index e2ca70696c05..7e80530b9b00 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -1865,7 +1865,7 @@ static void hsw_set_frame_start_delay(const struct intel_crtc_state *crtc_state)
>
> val = intel_de_read(dev_priv, reg);
> val &= ~HSW_FRAME_START_DELAY_MASK;
> - val |= HSW_FRAME_START_DELAY(dev_priv->framestart_delay - 1);
> + val |= HSW_FRAME_START_DELAY(crtc_state->framestart_delay - 1);
> intel_de_write(dev_priv, reg, val);
> }
>
> @@ -3208,7 +3208,7 @@ static void i9xx_set_pipeconf(const struct intel_crtc_state *crtc_state)
>
> pipeconf |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode);
>
> - pipeconf |= PIPECONF_FRAME_START_DELAY(dev_priv->framestart_delay - 1);
> + pipeconf |= PIPECONF_FRAME_START_DELAY(crtc_state->framestart_delay - 1);
>
> intel_de_write(dev_priv, PIPECONF(crtc->pipe), pipeconf);
> intel_de_posting_read(dev_priv, PIPECONF(crtc->pipe));
> @@ -3398,6 +3398,8 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
>
> pipe_config->gamma_mode = REG_FIELD_GET(PIPECONF_GAMMA_MODE_MASK_I9XX, tmp);
>
> + pipe_config->framestart_delay = REG_FIELD_GET(PIPECONF_FRAME_START_DELAY_MASK, tmp) + 1;
> +
> if (IS_CHERRYVIEW(dev_priv))
> pipe_config->cgm_mode = intel_de_read(dev_priv,
> CGM_PIPE_MODE(crtc->pipe));
> @@ -3523,7 +3525,7 @@ static void ilk_set_pipeconf(const struct intel_crtc_state *crtc_state)
>
> val |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode);
>
> - val |= PIPECONF_FRAME_START_DELAY(dev_priv->framestart_delay - 1);
> + val |= PIPECONF_FRAME_START_DELAY(crtc_state->framestart_delay - 1);
>
> intel_de_write(dev_priv, PIPECONF(pipe), val);
> intel_de_posting_read(dev_priv, PIPECONF(pipe));
> @@ -3831,6 +3833,8 @@ static bool ilk_get_pipe_config(struct intel_crtc *crtc,
>
> pipe_config->gamma_mode = REG_FIELD_GET(PIPECONF_GAMMA_MODE_MASK_ILK, tmp);
>
> + pipe_config->framestart_delay = REG_FIELD_GET(PIPECONF_FRAME_START_DELAY_MASK, tmp) + 1;
> +
> pipe_config->csc_mode = intel_de_read(dev_priv,
> PIPE_CSC_MODE(crtc->pipe));
>
> @@ -4266,6 +4270,15 @@ static bool hsw_get_pipe_config(struct intel_crtc *crtc,
> pipe_config->pixel_multiplier = 1;
> }
>
> + if (!transcoder_is_dsi(pipe_config->cpu_transcoder)) {
> + tmp = intel_de_read(dev_priv, CHICKEN_TRANS(pipe_config->cpu_transcoder));
> +
> + pipe_config->framestart_delay = REG_FIELD_GET(HSW_FRAME_START_DELAY_MASK, tmp) + 1;
> + } else {
> + /* no idea if this is correct */
> + pipe_config->framestart_delay = 1;
> + }
> +
> out:
> intel_display_power_put_all_in_set(dev_priv, &power_domain_set);
>
> @@ -5303,6 +5316,9 @@ static void intel_dump_pipe_config(const struct intel_crtc_state *pipe_config,
> &pipe_config->dp_m2_n2);
> }
>
> + drm_dbg_kms(&dev_priv->drm, "framestart delay: %d\n",
> + pipe_config->framestart_delay);
> +
> drm_dbg_kms(&dev_priv->drm,
> "audio: %i, infoframes: %i, infoframes enabled: 0x%x\n",
> pipe_config->has_audio, pipe_config->has_infoframe,
> @@ -5654,6 +5670,8 @@ intel_modeset_pipe_config(struct intel_atomic_state *state,
> pipe_config->cpu_transcoder =
> (enum transcoder) to_intel_crtc(crtc)->pipe;
>
> + pipe_config->framestart_delay = 1;
> +
> /*
> * Sanitize sync polarity flags based on requested ones. If neither
> * positive or negative polarity is requested, treat this as meaning
> @@ -6191,6 +6209,8 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
>
> PIPE_CONF_CHECK_X(output_types);
>
> + PIPE_CONF_CHECK_I(framestart_delay);
> +
> PIPE_CONF_CHECK_I(hw.pipe_mode.crtc_hdisplay);
> PIPE_CONF_CHECK_I(hw.pipe_mode.crtc_htotal);
> PIPE_CONF_CHECK_I(hw.pipe_mode.crtc_hblank_start);
> @@ -9499,8 +9519,6 @@ int intel_modeset_init_noirq(struct drm_i915_private *i915)
> i915->flip_wq = alloc_workqueue("i915_flip", WQ_HIGHPRI |
> WQ_UNBOUND, WQ_UNBOUND_MAX_ACTIVE);
>
> - i915->framestart_delay = 1; /* 1-4 */
> -
> i915->window2_delay = 0; /* No DSB so no window2 delay */
>
> intel_mode_config_init(i915);
> @@ -9804,12 +9822,14 @@ static bool has_pch_trancoder(struct drm_i915_private *dev_priv,
> (HAS_PCH_LPT_H(dev_priv) && pch_transcoder == PIPE_A);
> }
>
> -static void intel_sanitize_frame_start_delay(const struct intel_crtc_state *crtc_state)
> +static void intel_sanitize_frame_start_delay(struct intel_crtc_state *crtc_state)
> {
> struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>
> + crtc_state->framestart_delay = 1;
> +
> if (DISPLAY_VER(dev_priv) >= 9 ||
> IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv)) {
> i915_reg_t reg = CHICKEN_TRANS(cpu_transcoder);
> @@ -9820,7 +9840,7 @@ static void intel_sanitize_frame_start_delay(const struct intel_crtc_state *crtc
>
> val = intel_de_read(dev_priv, reg);
> val &= ~HSW_FRAME_START_DELAY_MASK;
> - val |= HSW_FRAME_START_DELAY(dev_priv->framestart_delay - 1);
> + val |= HSW_FRAME_START_DELAY(crtc_state->framestart_delay - 1);
> intel_de_write(dev_priv, reg, val);
> } else {
> i915_reg_t reg = PIPECONF(cpu_transcoder);
> @@ -9828,7 +9848,7 @@ static void intel_sanitize_frame_start_delay(const struct intel_crtc_state *crtc
>
> val = intel_de_read(dev_priv, reg);
> val &= ~PIPECONF_FRAME_START_DELAY_MASK;
> - val |= PIPECONF_FRAME_START_DELAY(dev_priv->framestart_delay - 1);
> + val |= PIPECONF_FRAME_START_DELAY(crtc_state->framestart_delay - 1);
> intel_de_write(dev_priv, reg, val);
> }
>
> @@ -9841,7 +9861,7 @@ static void intel_sanitize_frame_start_delay(const struct intel_crtc_state *crtc
>
> val = intel_de_read(dev_priv, reg);
> val &= ~TRANS_FRAME_START_DELAY_MASK;
> - val |= TRANS_FRAME_START_DELAY(dev_priv->framestart_delay - 1);
> + val |= TRANS_FRAME_START_DELAY(crtc_state->framestart_delay - 1);
> intel_de_write(dev_priv, reg, val);
> } else {
> enum pipe pch_transcoder = intel_crtc_pch_transcoder(crtc);
> @@ -9850,7 +9870,7 @@ static void intel_sanitize_frame_start_delay(const struct intel_crtc_state *crtc
>
> val = intel_de_read(dev_priv, reg);
> val &= ~TRANS_CHICKEN2_FRAME_START_DELAY_MASK;
> - val |= TRANS_CHICKEN2_FRAME_START_DELAY(dev_priv->framestart_delay - 1);
> + val |= TRANS_CHICKEN2_FRAME_START_DELAY(crtc_state->framestart_delay - 1);
> intel_de_write(dev_priv, reg, val);
> }
> }
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index b50d0e6efe21..8e50f03f8c92 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1154,6 +1154,8 @@ struct intel_crtc_state {
> /* bitmask of planes that will be updated during the commit */
> u8 update_planes;
>
> + u8 framestart_delay; /* 1-4 */
> +
> struct {
> u32 enable;
> u32 gcp;
> diff --git a/drivers/gpu/drm/i915/display/intel_pch_display.c b/drivers/gpu/drm/i915/display/intel_pch_display.c
> index 9192769e3337..00232dab217d 100644
> --- a/drivers/gpu/drm/i915/display/intel_pch_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_pch_display.c
> @@ -181,7 +181,7 @@ static void ilk_enable_pch_transcoder(const struct intel_crtc_state *crtc_state)
> val |= TRANS_CHICKEN2_TIMING_OVERRIDE;
> /* Configure frame start delay to match the CPU */
> val &= ~TRANS_CHICKEN2_FRAME_START_DELAY_MASK;
> - val |= TRANS_CHICKEN2_FRAME_START_DELAY(dev_priv->framestart_delay - 1);
> + val |= TRANS_CHICKEN2_FRAME_START_DELAY(crtc_state->framestart_delay - 1);
> intel_de_write(dev_priv, reg, val);
> }
>
> @@ -192,7 +192,7 @@ static void ilk_enable_pch_transcoder(const struct intel_crtc_state *crtc_state)
> if (HAS_PCH_IBX(dev_priv)) {
> /* Configure frame start delay to match the CPU */
> val &= ~TRANS_FRAME_START_DELAY_MASK;
> - val |= TRANS_FRAME_START_DELAY(dev_priv->framestart_delay - 1);
> + val |= TRANS_FRAME_START_DELAY(crtc_state->framestart_delay - 1);
>
> /*
> * Make the BPC in transcoder be consistent with
> @@ -466,9 +466,11 @@ void ilk_pch_get_config(struct intel_crtc_state *crtc_state)
> ilk_pch_clock_get(crtc_state);
> }
>
> -static void lpt_enable_pch_transcoder(struct drm_i915_private *dev_priv,
> - enum transcoder cpu_transcoder)
> +static void lpt_enable_pch_transcoder(const struct intel_crtc_state *crtc_state)
> {
> + struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> + enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> u32 val, pipeconf_val;
>
> /* FDI must be feeding us bits for PCH ports */
> @@ -480,7 +482,7 @@ static void lpt_enable_pch_transcoder(struct drm_i915_private *dev_priv,
> val |= TRANS_CHICKEN2_TIMING_OVERRIDE;
> /* Configure frame start delay to match the CPU */
> val &= ~TRANS_CHICKEN2_FRAME_START_DELAY_MASK;
> - val |= TRANS_CHICKEN2_FRAME_START_DELAY(dev_priv->framestart_delay - 1);
> + val |= TRANS_CHICKEN2_FRAME_START_DELAY(crtc_state->framestart_delay - 1);
> intel_de_write(dev_priv, TRANS_CHICKEN2(PIPE_A), val);
>
> val = TRANS_ENABLE;
> @@ -521,7 +523,6 @@ void lpt_pch_enable(struct intel_atomic_state *state,
> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> const struct intel_crtc_state *crtc_state =
> intel_atomic_get_new_crtc_state(state, crtc);
> - enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>
> assert_pch_transcoder_disabled(dev_priv, PIPE_A);
>
> @@ -530,7 +531,7 @@ void lpt_pch_enable(struct intel_atomic_state *state,
> /* Set transcoder timing. */
> ilk_pch_transcoder_set_timings(crtc_state, PIPE_A);
>
> - lpt_enable_pch_transcoder(dev_priv, cpu_transcoder);
> + lpt_enable_pch_transcoder(crtc_state);
> }
>
> void lpt_pch_disable(struct intel_atomic_state *state,
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> index 139e8936edc5..396f2f994fa0 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -69,9 +69,9 @@ static int intel_vrr_vblank_exit_length(const struct intel_crtc_state *crtc_stat
>
> /* The hw imposes the extra scanline before frame start */
> if (DISPLAY_VER(i915) >= 13)
> - return crtc_state->vrr.guardband + i915->framestart_delay + 1;
> + return crtc_state->vrr.guardband + crtc_state->framestart_delay + 1;
> else
> - return crtc_state->vrr.pipeline_full + i915->framestart_delay + 1;
> + return crtc_state->vrr.pipeline_full + crtc_state->framestart_delay + 1;
> }
>
> int intel_vrr_vmin_vblank_start(const struct intel_crtc_state *crtc_state)
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 1c2f4ae4ebf9..f5c249f93dad 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -825,8 +825,6 @@ struct drm_i915_private {
> struct file *mmap_singleton;
> } gem;
>
> - u8 framestart_delay;
> -
> /* Window2 specifies time required to program DSB (Window2) in number of scan lines */
> u8 window2_delay;
--
Jani Nikula, Intel Open Source Graphics Center
prev parent reply other threads:[~2022-03-04 11:09 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-21 11:03 [Intel-gfx] [PATCH 1/4] drm/i915: Move framestart_delay to crtc_state Ville Syrjala
2022-02-21 11:03 ` [Intel-gfx] [PATCH 2/4] drm/i915: Remove framestart_delay sanitation Ville Syrjala
2022-02-21 11:03 ` [Intel-gfx] [PATCH 3/4] drm/i915: Relocate ibx pch port sanitation code Ville Syrjala
2022-02-21 11:03 ` [Intel-gfx] [PATCH 4/4] drm/i915: Relocate a few more pch transcoder bits Ville Syrjala
2022-02-21 16:55 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [1/4] drm/i915: Move framestart_delay to crtc_state Patchwork
2022-02-21 17:23 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2022-02-21 22:06 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2022-03-04 11:08 ` Jani Nikula [this message]
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=87bkym3rg8.fsf@intel.com \
--to=jani.nikula@linux.intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=ville.syrjala@linux.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.