From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 13/13] drm/i915: Get rid of intel_crtc->config in crtc_enable/disable functions
Date: Wed, 3 Oct 2018 17:24:08 +0300 [thread overview]
Message-ID: <20181003142408.GF9144@intel.com> (raw)
In-Reply-To: <20181003133715.3713-14-maarten.lankhorst@linux.intel.com>
On Wed, Oct 03, 2018 at 03:37:15PM +0200, Maarten Lankhorst wrote:
> These functions already have a pointer to the correct state,
> so use it instead of crtc->config.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 60 ++++++++++++++--------------
> 1 file changed, 30 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 22f556b824f9..01ad6bd3b7de 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5594,15 +5594,15 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
> if (pipe_config->has_pch_encoder)
> intel_prepare_shared_dpll(pipe_config);
>
> - if (intel_crtc_has_dp_encoder(intel_crtc->config))
> + if (intel_crtc_has_dp_encoder(pipe_config))
> intel_dp_set_m_n(intel_crtc, M1_N1);
>
> intel_set_pipe_timings(pipe_config);
> intel_set_pipe_src_size(pipe_config);
>
> - if (intel_crtc->config->has_pch_encoder) {
> + if (pipe_config->has_pch_encoder) {
> intel_cpu_transcoder_set_m_n(intel_crtc,
> - &intel_crtc->config->fdi_m_n, NULL);
> + &pipe_config->fdi_m_n, NULL);
> }
>
> ironlake_set_pipeconf(pipe_config);
> @@ -5611,7 +5611,7 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
>
> intel_encoders_pre_enable(crtc, pipe_config, old_state);
>
> - if (intel_crtc->config->has_pch_encoder) {
> + if (pipe_config->has_pch_encoder) {
> /* Note: FDI PLL enabling _must_ be done before we enable the
> * cpu pipes, hence this is separate from all the other fdi/pch
> * enabling. */
> @@ -5630,10 +5630,10 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
> intel_color_load_luts(&pipe_config->base);
>
> if (dev_priv->display.initial_watermarks != NULL)
> - dev_priv->display.initial_watermarks(old_intel_state, intel_crtc->config);
> + dev_priv->display.initial_watermarks(old_intel_state, pipe_config);
> intel_enable_pipe(pipe_config);
>
> - if (intel_crtc->config->has_pch_encoder)
> + if (pipe_config->has_pch_encoder)
> ironlake_pch_enable(old_intel_state, pipe_config);
>
> assert_vblank_disabled(crtc);
> @@ -5650,7 +5650,7 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
> * some interlaced HDMI modes. Let's do the double wait always
> * in case there are more corner cases we don't know about.
> */
> - if (intel_crtc->config->has_pch_encoder) {
> + if (pipe_config->has_pch_encoder) {
> intel_wait_for_vblank(dev_priv, pipe);
> intel_wait_for_vblank(dev_priv, pipe);
> }
> @@ -5699,7 +5699,7 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
> struct drm_i915_private *dev_priv = to_i915(crtc->dev);
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> int pipe = intel_crtc->pipe, hsw_workaround_pipe;
> - enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
> + enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
> struct intel_atomic_state *old_intel_state =
> to_intel_atomic_state(old_state);
> bool psl_clkgate_wa;
> @@ -5718,7 +5718,7 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
>
> intel_encoders_pre_enable(crtc, pipe_config, old_state);
>
> - if (intel_crtc_has_dp_encoder(intel_crtc->config))
> + if (intel_crtc_has_dp_encoder(pipe_config))
> intel_dp_set_m_n(intel_crtc, M1_N1);
>
> if (!transcoder_is_dsi(cpu_transcoder))
> @@ -5729,12 +5729,12 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
> if (cpu_transcoder != TRANSCODER_EDP &&
> !transcoder_is_dsi(cpu_transcoder)) {
> I915_WRITE(PIPE_MULT(cpu_transcoder),
> - intel_crtc->config->pixel_multiplier - 1);
> + pipe_config->pixel_multiplier - 1);
> }
>
> - if (intel_crtc->config->has_pch_encoder) {
> + if (pipe_config->has_pch_encoder) {
> intel_cpu_transcoder_set_m_n(intel_crtc,
> - &intel_crtc->config->fdi_m_n, NULL);
> + &pipe_config->fdi_m_n, NULL);
> }
>
> if (!transcoder_is_dsi(cpu_transcoder))
> @@ -5748,7 +5748,7 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
>
> /* Display WA #1180: WaDisableScalarClockGating: glk, cnl */
> psl_clkgate_wa = (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) &&
> - intel_crtc->config->pch_pfit.enabled;
> + pipe_config->pch_pfit.enabled;
> if (psl_clkgate_wa)
> glk_pipe_scaler_clock_gating_wa(dev_priv, pipe, true);
>
> @@ -5788,10 +5788,10 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
> if (!transcoder_is_dsi(cpu_transcoder))
> intel_enable_pipe(pipe_config);
>
> - if (intel_crtc->config->has_pch_encoder)
> + if (pipe_config->has_pch_encoder)
> lpt_pch_enable(old_intel_state, pipe_config);
>
> - if (intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_DP_MST))
> + if (intel_crtc_has_type(pipe_config, INTEL_OUTPUT_DP_MST))
> intel_ddi_set_vc_payload_alloc(pipe_config, true);
>
> assert_vblank_disabled(crtc);
> @@ -5854,12 +5854,12 @@ static void ironlake_crtc_disable(struct intel_crtc_state *old_crtc_state,
>
> ironlake_pfit_disable(old_crtc_state);
>
> - if (intel_crtc->config->has_pch_encoder)
> + if (old_crtc_state->has_pch_encoder)
> ironlake_fdi_disable(crtc);
>
> intel_encoders_post_disable(crtc, old_crtc_state, old_state);
>
> - if (intel_crtc->config->has_pch_encoder) {
> + if (old_crtc_state->has_pch_encoder) {
> ironlake_disable_pch_transcoder(dev_priv, pipe);
>
> if (HAS_PCH_CPT(dev_priv)) {
> @@ -6060,7 +6060,7 @@ static void valleyview_crtc_enable(struct intel_crtc_state *pipe_config,
> if (WARN_ON(intel_crtc->active))
> return;
>
> - if (intel_crtc_has_dp_encoder(intel_crtc->config))
> + if (intel_crtc_has_dp_encoder(pipe_config))
> intel_dp_set_m_n(intel_crtc, M1_N1);
>
> intel_set_pipe_timings(pipe_config);
> @@ -6082,11 +6082,11 @@ static void valleyview_crtc_enable(struct intel_crtc_state *pipe_config,
> intel_encoders_pre_pll_enable(crtc, pipe_config, old_state);
>
> if (IS_CHERRYVIEW(dev_priv)) {
> - chv_prepare_pll(intel_crtc, intel_crtc->config);
> - chv_enable_pll(intel_crtc, intel_crtc->config);
> + chv_prepare_pll(intel_crtc, pipe_config);
> + chv_enable_pll(intel_crtc, pipe_config);
> } else {
> - vlv_prepare_pll(intel_crtc, intel_crtc->config);
> - vlv_enable_pll(intel_crtc, intel_crtc->config);
> + vlv_prepare_pll(intel_crtc, pipe_config);
> + vlv_enable_pll(intel_crtc, pipe_config);
> }
>
> intel_encoders_pre_enable(crtc, pipe_config, old_state);
> @@ -6105,13 +6105,13 @@ static void valleyview_crtc_enable(struct intel_crtc_state *pipe_config,
> intel_encoders_enable(crtc, pipe_config, old_state);
> }
>
> -static void i9xx_set_pll_dividers(struct intel_crtc *crtc)
> +static void i9xx_set_pll_dividers(const struct intel_crtc_state *crtc_state)
> {
> - struct drm_device *dev = crtc->base.dev;
> - struct drm_i915_private *dev_priv = to_i915(dev);
> + struct intel_crtc_state *crtc = to_intel_crtc(crtc_state->base.crtc);
> + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>
> - I915_WRITE(FP0(crtc->pipe), crtc->config->dpll_hw_state.fp0);
> - I915_WRITE(FP1(crtc->pipe), crtc->config->dpll_hw_state.fp1);
> + I915_WRITE(FP0(crtc->pipe), crtc_state->dpll_hw_state.fp0);
> + I915_WRITE(FP1(crtc->pipe), crtc_state->dpll_hw_state.fp1);
> }
>
> static void i9xx_crtc_enable(struct intel_crtc_state *pipe_config,
> @@ -6130,7 +6130,7 @@ static void i9xx_crtc_enable(struct intel_crtc_state *pipe_config,
>
> i9xx_set_pll_dividers(intel_crtc);
Missed the caller. But maybe you'll take my suggestion and move the
i9xx_set_pll_dividerschange () into the pll patch anyway.
Otherwise lgtm
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
And the compiler should catch what I didn't ;)
>
> - if (intel_crtc_has_dp_encoder(intel_crtc->config))
> + if (intel_crtc_has_dp_encoder(pipe_config))
> intel_dp_set_m_n(intel_crtc, M1_N1);
>
> intel_set_pipe_timings(pipe_config);
> @@ -6153,7 +6153,7 @@ static void i9xx_crtc_enable(struct intel_crtc_state *pipe_config,
>
> if (dev_priv->display.initial_watermarks != NULL)
> dev_priv->display.initial_watermarks(old_intel_state,
> - intel_crtc->config);
> + pipe_config);
> else
> intel_update_watermarks(intel_crtc);
> intel_enable_pipe(pipe_config);
> @@ -6206,7 +6206,7 @@ static void i9xx_crtc_disable(struct intel_crtc_state *old_crtc_state,
>
> intel_encoders_post_disable(crtc, old_crtc_state, old_state);
>
> - if (!intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_DSI)) {
> + if (!intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_DSI)) {
> if (IS_CHERRYVIEW(dev_priv))
> chv_disable_pll(dev_priv, pipe);
> else if (IS_VALLEYVIEW(dev_priv))
> --
> 2.19.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2018-10-03 14:24 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-10-03 13:37 [PATCH 00/13] drm/i915: First cleanup pass to get rid of more crtc->config users Maarten Lankhorst
2018-10-03 13:37 ` [PATCH 01/13] drm/i915: Remove dereferences of crtc->config in set_pipeconf/misc functions Maarten Lankhorst
2018-10-03 13:54 ` Ville Syrjälä
2018-10-03 13:37 ` [PATCH 02/13] drm/i915: Make panel fitter functions take state Maarten Lankhorst
2018-10-03 13:59 ` Ville Syrjälä
2018-10-03 13:37 ` [PATCH 03/13] drm/i915: Make intel_set_pipe_timings/src_size take a pointer to crtc_state Maarten Lankhorst
2018-10-03 14:01 ` Ville Syrjälä
2018-10-03 13:37 ` [PATCH 04/13] drm/i915: Use crtc_state in ironlake_enable_pch_transcoder Maarten Lankhorst
2018-10-03 14:05 ` Ville Syrjälä
2018-10-03 13:37 ` [PATCH 05/13] drm/i915: Make skl_detach_scalers take crtc_state Maarten Lankhorst
2018-10-03 14:07 ` Ville Syrjälä
2018-10-03 13:37 ` [PATCH 06/13] drm/i915: Make pll functions " Maarten Lankhorst
2018-10-03 14:11 ` Ville Syrjälä
2018-10-03 13:37 ` [PATCH 07/13] drm/i915: Make ironlake_pch_transcoder_set_timings " Maarten Lankhorst
2018-10-03 14:11 ` Ville Syrjälä
2018-10-03 13:37 ` [PATCH 08/13] drm/i915: Make shared dpll functions " Maarten Lankhorst
2018-10-03 14:15 ` Ville Syrjälä
2018-10-03 13:37 ` [PATCH 09/13] drm/i915: Get rid of crtc->config from icl_pll_to_ddi_pll_sel Maarten Lankhorst
2018-10-03 14:16 ` Ville Syrjälä
2018-10-03 13:37 ` [PATCH 10/13] drm/i915: Use crtc->state in intel_fbdev_init_bios Maarten Lankhorst
2018-10-03 14:18 ` Ville Syrjälä
2018-10-03 13:37 ` [PATCH 11/13] drm/i915: Get rid of crtc->config dereference in intel_dp_retrain_link Maarten Lankhorst
2018-10-03 14:21 ` Ville Syrjälä
2018-10-03 13:37 ` [PATCH 12/13] drm/i915: Get rid of crtc->config in chv_data_lane_soft_reset Maarten Lankhorst
2018-10-03 14:21 ` Ville Syrjälä
2018-10-03 13:37 ` [PATCH 13/13] drm/i915: Get rid of intel_crtc->config in crtc_enable/disable functions Maarten Lankhorst
2018-10-03 14:24 ` Ville Syrjälä [this message]
2018-10-03 14:41 ` ✗ Fi.CI.BAT: failure for drm/i915: First cleanup pass to get rid of more crtc->config users 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=20181003142408.GF9144@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=maarten.lankhorst@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.