From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Imre Deak <imre.deak@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH v2 17/23] drm/i915: Sanitize the shared DPLL find/reference interface
Date: Tue, 25 Jun 2019 16:54:56 +0300 [thread overview]
Message-ID: <20190625135456.GF5942@intel.com> (raw)
In-Reply-To: <20190620140600.11357-18-imre.deak@intel.com>
On Thu, Jun 20, 2019 at 05:05:54PM +0300, Imre Deak wrote:
> Pass the PLL HW state to the PLL find/reference functions making it
> clearer what is their input. Also pass to these the atomic state and the
> CRTC object instead of the CRTC state, since they don't require the
> latter.
>
> Move setting the PLL in the crtc_state to the get_dpll() hook, which
> is the more logical place for this, where the related PLL HW state was also
> set.
>
> This refactoring is also a preparation for a follow-up patch that will
> have to find/reference multiple PLLs.
>
> No functional changes.
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 107 ++++++++++++------
> 1 file changed, 70 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> index 3fbc975851fa..14bbab45836d 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> @@ -244,17 +244,18 @@ void intel_disable_shared_dpll(const struct intel_crtc_state *crtc_state)
> }
>
> static struct intel_shared_dpll *
> -intel_find_shared_dpll(struct intel_crtc_state *crtc_state,
> +intel_find_shared_dpll(struct intel_atomic_state *state,
> + const struct intel_crtc *crtc,
> + const struct intel_dpll_hw_state *pll_state,
> enum intel_dpll_id range_min,
> enum intel_dpll_id range_max)
> {
> - struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> struct intel_shared_dpll *pll, *unused_pll = NULL;
> struct intel_shared_dpll_state *shared_dpll;
> enum intel_dpll_id i;
>
> - shared_dpll = intel_atomic_get_shared_dpll_state(crtc_state->base.state);
> + shared_dpll = intel_atomic_get_shared_dpll_state(&state->base);
>
> for (i = range_min; i <= range_max; i++) {
> pll = &dev_priv->shared_dplls[i];
> @@ -266,9 +267,9 @@ intel_find_shared_dpll(struct intel_crtc_state *crtc_state,
> continue;
> }
>
> - if (memcmp(&crtc_state->dpll_hw_state,
> + if (memcmp(pll_state,
> &shared_dpll[i].hw_state,
> - sizeof(crtc_state->dpll_hw_state)) == 0) {
> + sizeof(*pll_state)) == 0) {
> DRM_DEBUG_KMS("[CRTC:%d:%s] sharing existing %s (crtc mask 0x%08x, active %x)\n",
> crtc->base.base.id, crtc->base.name,
> pll->info->name,
> @@ -290,20 +291,19 @@ intel_find_shared_dpll(struct intel_crtc_state *crtc_state,
> }
>
> static void
> -intel_reference_shared_dpll(struct intel_shared_dpll *pll,
> - struct intel_crtc_state *crtc_state)
> +intel_reference_shared_dpll(struct intel_atomic_state *state,
> + const struct intel_crtc *crtc,
> + const struct intel_shared_dpll *pll,
> + const struct intel_dpll_hw_state *pll_state)
> {
> struct intel_shared_dpll_state *shared_dpll;
> - struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> const enum intel_dpll_id id = pll->info->id;
>
> - shared_dpll = intel_atomic_get_shared_dpll_state(crtc_state->base.state);
> + shared_dpll = intel_atomic_get_shared_dpll_state(&state->base);
>
> if (shared_dpll[id].crtc_mask == 0)
> - shared_dpll[id].hw_state =
> - crtc_state->dpll_hw_state;
> + shared_dpll[id].hw_state = *pll_state;
>
> - crtc_state->shared_dpll = pll;
> DRM_DEBUG_DRIVER("using %s for pipe %c\n", pll->info->name,
> pipe_name(crtc->pipe));
>
> @@ -463,7 +463,8 @@ static bool ibx_get_dpll(struct intel_atomic_state *state,
> crtc->base.base.id, crtc->base.name,
> pll->info->name);
> } else {
> - pll = intel_find_shared_dpll(crtc_state,
> + pll = intel_find_shared_dpll(state, crtc,
> + &crtc_state->dpll_hw_state,
> DPLL_ID_PCH_PLL_A,
> DPLL_ID_PCH_PLL_B);
> }
> @@ -472,7 +473,10 @@ static bool ibx_get_dpll(struct intel_atomic_state *state,
> return false;
>
> /* reference the pll */
> - intel_reference_shared_dpll(pll, crtc_state);
> + intel_reference_shared_dpll(state, crtc,
> + pll, &crtc_state->dpll_hw_state);
> +
> + crtc_state->shared_dpll = pll;
>
> return true;
> }
> @@ -791,8 +795,12 @@ hsw_ddi_calculate_wrpll(int clock /* in Hz */,
> *r2_out = best.r2;
> }
>
> -static struct intel_shared_dpll *hsw_ddi_hdmi_get_dpll(struct intel_crtc_state *crtc_state)
> +static struct intel_shared_dpll *
> +hsw_ddi_hdmi_get_dpll(struct intel_atomic_state *state,
> + struct intel_crtc *crtc)
> {
> + struct intel_crtc_state *crtc_state =
> + intel_atomic_get_new_crtc_state(state, crtc);
> struct intel_shared_dpll *pll;
> u32 val;
> unsigned int p, n2, r2;
> @@ -805,7 +813,8 @@ static struct intel_shared_dpll *hsw_ddi_hdmi_get_dpll(struct intel_crtc_state *
>
> crtc_state->dpll_hw_state.wrpll = val;
>
> - pll = intel_find_shared_dpll(crtc_state,
> + pll = intel_find_shared_dpll(state, crtc,
> + &crtc_state->dpll_hw_state,
> DPLL_ID_WRPLL1, DPLL_ID_WRPLL2);
>
> if (!pll)
> @@ -857,7 +866,7 @@ static bool hsw_get_dpll(struct intel_atomic_state *state,
> sizeof(crtc_state->dpll_hw_state));
>
> if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI)) {
> - pll = hsw_ddi_hdmi_get_dpll(crtc_state);
> + pll = hsw_ddi_hdmi_get_dpll(state, crtc);
> } else if (intel_crtc_has_dp_encoder(crtc_state)) {
> pll = hsw_ddi_dp_get_dpll(crtc_state);
> } else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_ANALOG)) {
> @@ -867,7 +876,8 @@ static bool hsw_get_dpll(struct intel_atomic_state *state,
> crtc_state->dpll_hw_state.spll =
> SPLL_PLL_ENABLE | SPLL_FREQ_1350MHz | SPLL_REF_MUXED_SSC;
>
> - pll = intel_find_shared_dpll(crtc_state,
> + pll = intel_find_shared_dpll(state, crtc,
> + &crtc_state->dpll_hw_state,
> DPLL_ID_SPLL, DPLL_ID_SPLL);
> } else {
> return false;
> @@ -876,7 +886,10 @@ static bool hsw_get_dpll(struct intel_atomic_state *state,
> if (!pll)
> return false;
>
> - intel_reference_shared_dpll(pll, crtc_state);
> + intel_reference_shared_dpll(state, crtc,
> + pll, &crtc_state->dpll_hw_state);
> +
> + crtc_state->shared_dpll = pll;
>
> return true;
> }
> @@ -1437,17 +1450,22 @@ static bool skl_get_dpll(struct intel_atomic_state *state,
> }
>
> if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP))
> - pll = intel_find_shared_dpll(crtc_state,
> + pll = intel_find_shared_dpll(state, crtc,
> + &crtc_state->dpll_hw_state,
> DPLL_ID_SKL_DPLL0,
> DPLL_ID_SKL_DPLL0);
> else
> - pll = intel_find_shared_dpll(crtc_state,
> + pll = intel_find_shared_dpll(state, crtc,
> + &crtc_state->dpll_hw_state,
> DPLL_ID_SKL_DPLL1,
> DPLL_ID_SKL_DPLL3);
> if (!pll)
> return false;
>
> - intel_reference_shared_dpll(pll, crtc_state);
> + intel_reference_shared_dpll(state, crtc,
> + pll, &crtc_state->dpll_hw_state);
> +
> + crtc_state->shared_dpll = pll;
>
> return true;
> }
> @@ -1880,7 +1898,10 @@ static bool bxt_get_dpll(struct intel_atomic_state *state,
> DRM_DEBUG_KMS("[CRTC:%d:%s] using pre-allocated %s\n",
> crtc->base.base.id, crtc->base.name, pll->info->name);
>
> - intel_reference_shared_dpll(pll, crtc_state);
> + intel_reference_shared_dpll(state, crtc,
> + pll, &crtc_state->dpll_hw_state);
> +
> + crtc_state->shared_dpll = pll;
>
> return true;
> }
> @@ -2395,7 +2416,8 @@ static bool cnl_get_dpll(struct intel_atomic_state *state,
> return false;
> }
>
> - pll = intel_find_shared_dpll(crtc_state,
> + pll = intel_find_shared_dpll(state, crtc,
> + &crtc_state->dpll_hw_state,
> DPLL_ID_SKL_DPLL0,
> DPLL_ID_SKL_DPLL2);
> if (!pll) {
> @@ -2403,7 +2425,10 @@ static bool cnl_get_dpll(struct intel_atomic_state *state,
> return false;
> }
>
> - intel_reference_shared_dpll(pll, crtc_state);
> + intel_reference_shared_dpll(state, crtc,
> + pll, &crtc_state->dpll_hw_state);
> +
> + crtc_state->shared_dpll = pll;
>
> return true;
> }
> @@ -2545,7 +2570,8 @@ static bool icl_calc_tbt_pll(struct intel_crtc_state *crtc_state,
> }
>
> static bool icl_calc_dpll_state(struct intel_crtc_state *crtc_state,
> - struct intel_encoder *encoder)
> + struct intel_encoder *encoder,
> + struct intel_dpll_hw_state *pll_state)
> {
> struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
> u32 cfgcr0, cfgcr1;
> @@ -2572,11 +2598,10 @@ static bool icl_calc_dpll_state(struct intel_crtc_state *crtc_state,
> DPLL_CFGCR1_PDIV(pll_params.pdiv) |
> DPLL_CFGCR1_CENTRAL_FREQ_8400;
>
> - memset(&crtc_state->dpll_hw_state, 0,
> - sizeof(crtc_state->dpll_hw_state));
> + memset(pll_state, 0, sizeof(*pll_state));
>
> - crtc_state->dpll_hw_state.cfgcr0 = cfgcr0;
> - crtc_state->dpll_hw_state.cfgcr1 = cfgcr1;
> + pll_state->cfgcr0 = cfgcr0;
> + pll_state->cfgcr1 = cfgcr1;
>
> return true;
> }
> @@ -2666,10 +2691,10 @@ static bool icl_mg_pll_find_divisors(int clock_khz, bool is_dp, bool use_ssc,
> * The specification for this function uses real numbers, so the math had to be
> * adapted to integer-only calculation, that's why it looks so different.
> */
> -static bool icl_calc_mg_pll_state(struct intel_crtc_state *crtc_state)
> +static bool icl_calc_mg_pll_state(struct intel_crtc_state *crtc_state,
> + struct intel_dpll_hw_state *pll_state)
> {
> struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
> - struct intel_dpll_hw_state *pll_state = &crtc_state->dpll_hw_state;
> int refclk_khz = dev_priv->cdclk.hw.ref;
> int clock = crtc_state->port_clock;
> u32 dco_khz, m1div, m2div_int, m2div_rem, m2div_frac;
> @@ -2847,7 +2872,8 @@ static bool icl_get_dplls(struct intel_atomic_state *state,
> if (intel_port_is_combophy(dev_priv, port)) {
> min = DPLL_ID_ICL_DPLL0;
> max = DPLL_ID_ICL_DPLL1;
> - ret = icl_calc_dpll_state(crtc_state, encoder);
> + ret = icl_calc_dpll_state(crtc_state, encoder,
> + &crtc_state->dpll_hw_state);
> } else if (intel_port_is_tc(dev_priv, port)) {
> if (encoder->type == INTEL_OUTPUT_DP_MST) {
> struct intel_dp_mst_encoder *mst_encoder;
> @@ -2861,14 +2887,16 @@ static bool icl_get_dplls(struct intel_atomic_state *state,
> if (intel_dig_port->tc_mode == TC_PORT_TBT_ALT) {
> min = DPLL_ID_ICL_TBTPLL;
> max = min;
> - ret = icl_calc_dpll_state(crtc_state, encoder);
> + ret = icl_calc_dpll_state(crtc_state, encoder,
> + &crtc_state->dpll_hw_state);
> } else {
> enum tc_port tc_port;
>
> tc_port = intel_port_to_tc(dev_priv, port);
> min = icl_tc_port_to_pll_id(tc_port);
> max = min;
> - ret = icl_calc_mg_pll_state(crtc_state);
> + ret = icl_calc_mg_pll_state(crtc_state,
> + &crtc_state->dpll_hw_state);
> }
> } else {
> MISSING_CASE(port);
> @@ -2881,13 +2909,18 @@ static bool icl_get_dplls(struct intel_atomic_state *state,
> }
>
>
> - pll = intel_find_shared_dpll(crtc_state, min, max);
> + pll = intel_find_shared_dpll(state, crtc,
> + &crtc_state->dpll_hw_state,
> + min, max);
> if (!pll) {
> DRM_DEBUG_KMS("No PLL selected\n");
> return false;
> }
>
> - intel_reference_shared_dpll(pll, crtc_state);
> + intel_reference_shared_dpll(state, crtc,
> + pll, &crtc_state->dpll_hw_state);
> +
> + crtc_state->shared_dpll = pll;
>
> return true;
> }
> --
> 2.17.1
--
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:[~2019-06-25 13:55 UTC|newest]
Thread overview: 64+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-20 14:05 [PATCH v2 00/23] drm/i915: Fix TypeC port mode switching Imre Deak
2019-06-20 14:05 ` [PATCH v2 01/23] drm/i915/icl: Add support to read out the TBT PLL HW state Imre Deak
2019-06-20 14:05 ` [PATCH v2 02/23] drm/i915: Tune down WARNs about TBT AUX power well enabling Imre Deak
2019-06-20 14:05 ` [PATCH v2 03/23] drm/i915: Move the TypeC port handling code to a separate file Imre Deak
2019-06-20 14:05 ` [PATCH v2 04/23] drm/i915: Sanitize the terminology used for TypeC port modes Imre Deak
2019-06-20 14:05 ` [PATCH v2 05/23] drm/i915: Don't enable the DDI-IO power in the TypeC TBT-alt mode Imre Deak
2019-06-20 14:05 ` [PATCH v2 06/23] drm/i915: Fix the TBT AUX power well enabling Imre Deak
2019-06-20 14:05 ` [PATCH v2 07/23] drm/i915: Use the correct AUX power domain in TypeC TBT-alt mode Imre Deak
2019-06-20 14:05 ` [PATCH v2 08/23] drm/i915: Unify the TypeC port notation in debug/error messages Imre Deak
2019-06-20 14:05 ` [PATCH v2 09/23] drm/i915: Factor out common parts from TypeC port handling functions Imre Deak
2019-06-25 13:05 ` Ville Syrjälä
2019-06-25 13:48 ` Imre Deak
2019-06-26 20:50 ` [CI v3 " Imre Deak
2019-06-26 22:55 ` Souza, Jose
2019-06-27 10:34 ` Imre Deak
2019-06-20 14:05 ` [PATCH v2 10/23] drm/i915: Wait for TypeC PHY complete flag to clear in safe mode Imre Deak
2019-06-20 14:05 ` [PATCH v2 11/23] drm/i915: Handle the TCCOLD power-down event Imre Deak
2019-06-25 13:17 ` Ville Syrjälä
2019-06-25 14:22 ` Imre Deak
2019-06-26 20:50 ` [CI v3 " Imre Deak
2019-06-20 14:05 ` [PATCH v2 12/23] drm/i915: Sanitize the TypeC connect/detect sequences Imre Deak
2019-06-25 13:42 ` Ville Syrjälä
2019-06-25 18:30 ` Imre Deak
2019-06-26 11:51 ` Ville Syrjälä
2019-06-26 23:55 ` Souza, Jose
2019-06-27 9:48 ` Imre Deak
2019-06-27 21:06 ` Souza, Jose
2019-06-20 14:05 ` [PATCH v2 13/23] drm/i915: Fix the TypeC port mode sanitization during loading/resume Imre Deak
2019-06-26 18:04 ` [PATCH v3 " Imre Deak
2019-06-26 20:50 ` [CI " Imre Deak
2019-06-27 20:38 ` Souza, Jose
2019-06-20 14:05 ` [PATCH v2 14/23] drm/i915: Keep the TypeC port mode fixed for detect/AUX transfers Imre Deak
2019-06-25 13:43 ` Ville Syrjälä
2019-06-20 14:05 ` [PATCH v2 15/23] drm/i915: Sanitize the TypeC FIA lane configuration decoding Imre Deak
2019-06-20 14:05 ` [PATCH v2 16/23] drm/i915: Sanitize the shared DPLL reserve/release interface Imre Deak
2019-06-25 13:53 ` Ville Syrjälä
2019-06-25 18:57 ` Imre Deak
2019-06-25 19:48 ` Imre Deak
2019-06-20 14:05 ` [PATCH v2 17/23] drm/i915: Sanitize the shared DPLL find/reference interface Imre Deak
2019-06-25 13:54 ` Ville Syrjälä [this message]
2019-06-20 14:05 ` [PATCH v2 18/23] drm/i915/icl: Split getting the DPLLs to port type specific functions Imre Deak
2019-06-25 13:55 ` Ville Syrjälä
2019-06-20 14:05 ` [PATCH v2 19/23] drm/i915/icl: Reserve all required PLLs for TypeC ports Imre Deak
2019-06-25 13:58 ` Ville Syrjälä
2019-06-20 14:05 ` [PATCH v2 20/23] drm/i915: Keep the TypeC port mode fixed when the port is active Imre Deak
2019-06-25 14:01 ` Ville Syrjälä
2019-06-20 14:05 ` [PATCH v2 21/23] drm/i915: Add state verification for the TypeC port mode Imre Deak
2019-06-25 14:12 ` Ville Syrjälä
2019-06-25 19:23 ` Imre Deak
2019-06-26 11:52 ` Ville Syrjälä
2019-06-26 18:04 ` [v3 " Imre Deak
2019-06-26 20:50 ` [CI v3 " Imre Deak
2019-06-20 14:05 ` [PATCH v2 22/23] drm/i915: Remove unneeded disconnect in TypeC legacy " Imre Deak
2019-06-20 14:06 ` [PATCH v2 23/23] drm/i915: WARN about invalid lane reversal in TBT-alt/DP-alt modes Imre Deak
2019-06-20 17:23 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Fix TypeC port mode switching (rev3) Patchwork
2019-06-20 17:32 ` ✗ Fi.CI.SPARSE: " Patchwork
2019-06-20 18:05 ` ✓ Fi.CI.BAT: success " Patchwork
2019-06-20 22:42 ` ✓ Fi.CI.IGT: " Patchwork
2019-06-26 22:00 ` ✗ Fi.CI.BAT: failure for drm/i915: Fix TypeC port mode switching (rev7) Patchwork
-- strict thread matches above, loose matches on Subject: below --
2019-06-26 18:04 [PATCH v3 09/23] drm/i915: Factor out common parts from TypeC port handling functions Imre Deak
2019-06-26 18:04 ` [PATCH v3 11/23] drm/i915: Handle the TCCOLD power-down event Imre Deak
2019-06-26 22:12 ` Souza, Jose
2019-06-27 10:09 ` Imre Deak
2019-06-27 12:59 ` Ville Syrjälä
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=20190625135456.GF5942@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=daniel.vetter@ffwll.ch \
--cc=imre.deak@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
/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