| Message ID | 1409830075-11139-63-git-send-email-damien.lespiau@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
2014-09-04 8:27 GMT-03:00 Damien Lespiau <damien.lespiau@intel.com>: > From: Satheeshakrishna M <satheeshakrishna.m@intel.com> > > 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 <satheeshakrishna.m@intel.com> > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> > --- > 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. > + > + 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. > + } > +} > + > 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 <paulo.r.zanoni@intel.com> > + 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
On 9/23/2014 1:54 AM, Paulo Zanoni wrote: > 2014-09-04 8:27 GMT-03:00 Damien Lespiau<damien.lespiau@intel.com>: >> From: Satheeshakrishna M<satheeshakrishna.m@intel.com> >> >> 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<satheeshakrishna.m@intel.com> >> Signed-off-by: Damien Lespiau<damien.lespiau@intel.com> >> --- >> 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<paulo.r.zanoni@intel.com> > >> + 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 >
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); + + 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; + } +} + 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)) + 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. */