From mboxrd@z Thu Jan 1 00:00:00 1970 From: "M, Satheeshakrishna" Subject: Re: [PATCH 62/89] drm/i915/skl: Query DPLL attached to port on SKL Date: Wed, 01 Oct 2014 16:21:58 +0530 Message-ID: <542BDCCE.7080004@intel.com> References: <1409830075-11139-1-git-send-email-damien.lespiau@intel.com> <1409830075-11139-63-git-send-email-damien.lespiau@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by gabe.freedesktop.org (Postfix) with ESMTP id F1D8A6E337 for ; Wed, 1 Oct 2014 03:52:01 -0700 (PDT) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Paulo Zanoni , Damien Lespiau Cc: Intel Graphics Development List-Id: intel-gfx@lists.freedesktop.org On 9/23/2014 1:54 AM, Paulo Zanoni wrote: > 2014-09-04 8:27 GMT-03:00 Damien Lespiau: >> From: Satheeshakrishna M >> >> Modify the implementation to query DPLL attached to a SKL port. >> >> v2: Rebase on top of the run-time PM on DPMS series (Damien) >> >> Signed-off-by: Satheeshakrishna M >> Signed-off-by: Damien Lespiau >> --- >> drivers/gpu/drm/i915/intel_display.c | 27 ++++++++++++++++++++++++++- >> drivers/gpu/drm/i915/intel_drv.h | 5 ++++- >> 2 files changed, 30 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >> index 69e023a..6e71250 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -7789,6 +7789,28 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc, >> return 0; >> } >> >> +static void skylake_get_ddi_pll(struct drm_i915_private *dev_priv, >> + enum port port, >> + struct intel_crtc_config *pipe_config) >> +{ >> + u32 temp; >> + >> + temp = I915_READ(DPLL_CTRL2) & DPLL_CTRL2_DDI_CLK_SEL_MASK(port); >> + pipe_config->ddi_pll_sel = temp >> (port * 3 + 1); > Since we're relying on the fact that ddi_pll_sel is actually "enum > skl_dpll", I'd change the definition of the enum and explicitly > declare SKL_DPLL0 to be "0", and SKL_DPLL1 to be 1, and so on. Makes sense >> + >> + switch (pipe_config->ddi_pll_sel) { >> + case SKL_DPLL1: >> + pipe_config->shared_dpll = DPLL_ID_SKL_DPLL1; >> + break; >> + case SKL_DPLL2: >> + pipe_config->shared_dpll = DPLL_ID_SKL_DPLL2; >> + break; >> + case SKL_DPLL3: >> + pipe_config->shared_dpll = DPLL_ID_SKL_DPLL3; >> + break; > Please also add a WARN() to the default case, especially since there > are 4 possible values and we're just checking 3. ok >> + } >> +} >> + >> static void haswell_get_ddi_pll(struct drm_i915_private *dev_priv, >> enum port port, >> struct intel_crtc_config *pipe_config) >> @@ -7818,7 +7840,10 @@ static void haswell_get_ddi_port_state(struct intel_crtc *crtc, >> >> port = (tmp & TRANS_DDI_PORT_MASK) >> TRANS_DDI_PORT_SHIFT; >> >> - haswell_get_ddi_pll(dev_priv, port, pipe_config); >> + if (IS_SKYLAKE(dev)) > If you invert the check so that SKL is on the "else" part (as you did > in your previous patch), you'll make sure we're ready for the next > Gens in case they happen to be same as SKL. It's more likely they will > be the same as SKL instead of being the same as HSW :) > > > With at least the WARN added on the switch statement: Reviewed-by: > Paulo Zanoni > >> + skylake_get_ddi_pll(dev_priv, port, pipe_config); >> + else >> + haswell_get_ddi_pll(dev_priv, port, pipe_config); >> >> if (pipe_config->shared_dpll >= 0) { >> pll = &dev_priv->shared_dplls[pipe_config->shared_dpll]; >> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h >> index 559b747..9558f07 100644 >> --- a/drivers/gpu/drm/i915/intel_drv.h >> +++ b/drivers/gpu/drm/i915/intel_drv.h >> @@ -323,7 +323,10 @@ struct intel_crtc_config { >> /* Selected dpll when shared or DPLL_ID_PRIVATE. */ >> enum intel_dpll_id shared_dpll; >> >> - /* PORT_CLK_SEL for DDI ports. */ >> + /* >> + * - PORT_CLK_SEL for DDI ports on HSW/BDW. >> + * - enum skl_dpll on SKL >> + */ >> uint32_t ddi_pll_sel; >> >> /* Actual register state of the dpll, for shared dpll cross-checking. */ >> -- >> 1.8.3.1 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx >