| Message ID | 1472767699-31211-8-git-send-email-manasi.d.navare@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 2016-09-01 at 15:08 -0700, Manasi Navare wrote: > From: Jim Bride <jim.bride@linux.intel.com> > > Add the PLL selection code for HSW/BDW/BXT/SKL into a stand-alone function > in order to allow for the implementation of a platform neutral upfront > link training function. > > v3: > * Add Hooks for all DDI platforms into this standalone function > > v2: > * Change the macro to use dev_priv instead of dev (David Weinehall) > > Reviewed-by: Durgadoss R <durgadoss.r@intel.com> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > Signed-off-by: Jim Bride <jim.bride@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_ddi.c | 38 +++++++++++++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_dpll_mgr.c | 38 +++++++++++++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_dpll_mgr.h | 2 ++ > drivers/gpu/drm/i915/intel_drv.h | 3 ++- > 4 files changed, 80 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > index e4b875e..67a6a0b 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -2393,6 +2393,44 @@ intel_ddi_init_hdmi_connector(struct intel_digital_port *intel_dig_port) > return connector; > } > > +struct intel_shared_dpll * > +intel_ddi_get_link_dpll(struct intel_dp *intel_dp, int clock) > +{ > + struct intel_connector *connector = intel_dp->attached_connector; > + struct intel_encoder *encoder = connector->encoder; > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > + struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp); > + struct intel_shared_dpll *pll = NULL; > + struct intel_shared_dpll_config tmp_pll_config; > + enum intel_dpll_id dpll_id; > + > + if (IS_BROXTON(dev_priv)) { > + dpll_id = (enum intel_dpll_id)dig_port->port; > + /* > + * Select the required PLL. This works for platforms where > + * there is no shared DPLL. > + */ > + pll = &dev_priv->shared_dplls[dpll_id]; > + if (WARN_ON(pll->active_mask)) { > + > + DRM_ERROR("Shared DPLL in use. active_mask:%x\n", > + pll->active_mask); > + pll = NULL; > + } > + tmp_pll_config = pll->config; NULL dereference when pll is in use? > + if (!bxt_ddi_dp_set_dpll_hw_state(clock, > + &pll->config.hw_state)) { > + DRM_ERROR("Could not setup DPLL\n"); > + pll->config = tmp_pll_config; > + } > + } else if (IS_SKYLAKE(dev_priv)) { > + pll = skl_find_link_pll(dev_priv, clock); > + } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { > + pll = hsw_ddi_dp_get_dpll(encoder, clock); > + } > + return pll; > +} > + > void intel_ddi_init(struct drm_device *dev, enum port port) > { > struct drm_i915_private *dev_priv = to_i915(dev); > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c > index 9a1da98..4b067ac 100644 > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c > @@ -24,6 +24,44 @@ > #include "intel_drv.h" > > struct intel_shared_dpll * > +skl_find_link_pll(struct drm_i915_private *dev_priv, int clock) > +{ > + struct intel_shared_dpll *pll = NULL; > + struct intel_dpll_hw_state dpll_hw_state; > + enum intel_dpll_id i; > + bool found = false; > + > + if (!skl_ddi_dp_set_dpll_hw_state(clock, &dpll_hw_state)) > + return pll; > + > + for (i = DPLL_ID_SKL_DPLL1; i <= DPLL_ID_SKL_DPLL3; i++) { > + pll = &dev_priv->shared_dplls[i]; > + > + /* Only want to check enabled timings first */ > + if (pll->config.crtc_mask == 0) > + continue; > + > + if (memcmp(&dpll_hw_state, &pll->config.hw_state, > + sizeof(pll->config.hw_state)) == 0) { > + found = true; > + break; > + } > + } > + > + /* Ok no matching timings, maybe there's a free one? */ > + for (i = DPLL_ID_SKL_DPLL1; > + ((found == false) && (i <= DPLL_ID_SKL_DPLL3)); i++) { > + pll = &dev_priv->shared_dplls[i]; > + if (pll->config.crtc_mask == 0) { > + pll->config.hw_state = dpll_hw_state; > + break; > + } > + } > + > + return pll; > +} > + > +struct intel_shared_dpll * > intel_get_shared_dpll_by_id(struct drm_i915_private *dev_priv, > enum intel_dpll_id id) > { > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h b/drivers/gpu/drm/i915/intel_dpll_mgr.h > index aed7408..f438535 100644 > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.h > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h > @@ -168,6 +168,8 @@ bool bxt_ddi_dp_set_dpll_hw_state(int clock, > /* SKL dpll related functions */ > bool skl_ddi_dp_set_dpll_hw_state(int clock, > struct intel_dpll_hw_state *dpll_hw_state); > +struct intel_shared_dpll *skl_find_link_pll(struct drm_i915_private *dev_priv, > + int clock); > > > /* HSW dpll related functions */ > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 529fa7b..efcd80b 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1159,7 +1159,8 @@ void intel_ddi_clock_get(struct intel_encoder *encoder, > struct intel_crtc_state *pipe_config); > void intel_ddi_set_vc_payload_alloc(struct drm_crtc *crtc, bool state); > uint32_t ddi_signal_levels(struct intel_dp *intel_dp); > - > +struct intel_shared_dpll *intel_ddi_get_link_dpll(struct intel_dp *intel_dp, > + int clock); > unsigned int intel_fb_align_height(struct drm_device *dev, > unsigned int height, > uint32_t pixel_format,
On Fri, Sep 02, 2016 at 01:06:32PM -0700, Pandiyan, Dhinakaran wrote: > On Thu, 2016-09-01 at 15:08 -0700, Manasi Navare wrote: > > From: Jim Bride <jim.bride@linux.intel.com> > > > > Add the PLL selection code for HSW/BDW/BXT/SKL into a stand-alone function > > in order to allow for the implementation of a platform neutral upfront > > link training function. > > > > v3: > > * Add Hooks for all DDI platforms into this standalone function > > > > v2: > > * Change the macro to use dev_priv instead of dev (David Weinehall) > > > > Reviewed-by: Durgadoss R <durgadoss.r@intel.com> > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > Signed-off-by: Jim Bride <jim.bride@linux.intel.com> > > --- > > drivers/gpu/drm/i915/intel_ddi.c | 38 +++++++++++++++++++++++++++++++++++ > > drivers/gpu/drm/i915/intel_dpll_mgr.c | 38 +++++++++++++++++++++++++++++++++++ > > drivers/gpu/drm/i915/intel_dpll_mgr.h | 2 ++ > > drivers/gpu/drm/i915/intel_drv.h | 3 ++- > > 4 files changed, 80 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > > index e4b875e..67a6a0b 100644 > > --- a/drivers/gpu/drm/i915/intel_ddi.c > > +++ b/drivers/gpu/drm/i915/intel_ddi.c > > @@ -2393,6 +2393,44 @@ intel_ddi_init_hdmi_connector(struct intel_digital_port *intel_dig_port) > > return connector; > > } > > > > +struct intel_shared_dpll * > > +intel_ddi_get_link_dpll(struct intel_dp *intel_dp, int clock) > > +{ > > + struct intel_connector *connector = intel_dp->attached_connector; > > + struct intel_encoder *encoder = connector->encoder; > > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > > + struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp); > > + struct intel_shared_dpll *pll = NULL; > > + struct intel_shared_dpll_config tmp_pll_config; > > + enum intel_dpll_id dpll_id; > > + > > + if (IS_BROXTON(dev_priv)) { > > + dpll_id = (enum intel_dpll_id)dig_port->port; > > + /* > > + * Select the required PLL. This works for platforms where > > + * there is no shared DPLL. > > + */ > > + pll = &dev_priv->shared_dplls[dpll_id]; > > + if (WARN_ON(pll->active_mask)) { > > + > > + DRM_ERROR("Shared DPLL in use. active_mask:%x\n", > > + pll->active_mask); > > + pll = NULL; > > + } > > + tmp_pll_config = pll->config; > > NULL dereference when pll is in use? Yes I had given this comment in the internal review. So we moved this assignment out to the calling function, but left this here as well. I will remove this and resubmit. Manasi > > > + if (!bxt_ddi_dp_set_dpll_hw_state(clock, > > + &pll->config.hw_state)) { > > + DRM_ERROR("Could not setup DPLL\n"); > > + pll->config = tmp_pll_config; > > + } > > + } else if (IS_SKYLAKE(dev_priv)) { > > + pll = skl_find_link_pll(dev_priv, clock); > > + } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { > > + pll = hsw_ddi_dp_get_dpll(encoder, clock); > > + } > > + return pll; > > +} > > + > > void intel_ddi_init(struct drm_device *dev, enum port port) > > { > > struct drm_i915_private *dev_priv = to_i915(dev); > > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c > > index 9a1da98..4b067ac 100644 > > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c > > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c > > @@ -24,6 +24,44 @@ > > #include "intel_drv.h" > > > > struct intel_shared_dpll * > > +skl_find_link_pll(struct drm_i915_private *dev_priv, int clock) > > +{ > > + struct intel_shared_dpll *pll = NULL; > > + struct intel_dpll_hw_state dpll_hw_state; > > + enum intel_dpll_id i; > > + bool found = false; > > + > > + if (!skl_ddi_dp_set_dpll_hw_state(clock, &dpll_hw_state)) > > + return pll; > > + > > + for (i = DPLL_ID_SKL_DPLL1; i <= DPLL_ID_SKL_DPLL3; i++) { > > + pll = &dev_priv->shared_dplls[i]; > > + > > + /* Only want to check enabled timings first */ > > + if (pll->config.crtc_mask == 0) > > + continue; > > + > > + if (memcmp(&dpll_hw_state, &pll->config.hw_state, > > + sizeof(pll->config.hw_state)) == 0) { > > + found = true; > > + break; > > + } > > + } > > + > > + /* Ok no matching timings, maybe there's a free one? */ > > + for (i = DPLL_ID_SKL_DPLL1; > > + ((found == false) && (i <= DPLL_ID_SKL_DPLL3)); i++) { > > + pll = &dev_priv->shared_dplls[i]; > > + if (pll->config.crtc_mask == 0) { > > + pll->config.hw_state = dpll_hw_state; > > + break; > > + } > > + } > > + > > + return pll; > > +} > > + > > +struct intel_shared_dpll * > > intel_get_shared_dpll_by_id(struct drm_i915_private *dev_priv, > > enum intel_dpll_id id) > > { > > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h b/drivers/gpu/drm/i915/intel_dpll_mgr.h > > index aed7408..f438535 100644 > > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.h > > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h > > @@ -168,6 +168,8 @@ bool bxt_ddi_dp_set_dpll_hw_state(int clock, > > /* SKL dpll related functions */ > > bool skl_ddi_dp_set_dpll_hw_state(int clock, > > struct intel_dpll_hw_state *dpll_hw_state); > > +struct intel_shared_dpll *skl_find_link_pll(struct drm_i915_private *dev_priv, > > + int clock); > > > > > > /* HSW dpll related functions */ > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > index 529fa7b..efcd80b 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -1159,7 +1159,8 @@ void intel_ddi_clock_get(struct intel_encoder *encoder, > > struct intel_crtc_state *pipe_config); > > void intel_ddi_set_vc_payload_alloc(struct drm_crtc *crtc, bool state); > > uint32_t ddi_signal_levels(struct intel_dp *intel_dp); > > - > > +struct intel_shared_dpll *intel_ddi_get_link_dpll(struct intel_dp *intel_dp, > > + int clock); > > unsigned int intel_fb_align_height(struct drm_device *dev, > > unsigned int height, > > uint32_t pixel_format, >
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index e4b875e..67a6a0b 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -2393,6 +2393,44 @@ intel_ddi_init_hdmi_connector(struct intel_digital_port *intel_dig_port) return connector; } +struct intel_shared_dpll * +intel_ddi_get_link_dpll(struct intel_dp *intel_dp, int clock) +{ + struct intel_connector *connector = intel_dp->attached_connector; + struct intel_encoder *encoder = connector->encoder; + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); + struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp); + struct intel_shared_dpll *pll = NULL; + struct intel_shared_dpll_config tmp_pll_config; + enum intel_dpll_id dpll_id; + + if (IS_BROXTON(dev_priv)) { + dpll_id = (enum intel_dpll_id)dig_port->port; + /* + * Select the required PLL. This works for platforms where + * there is no shared DPLL. + */ + pll = &dev_priv->shared_dplls[dpll_id]; + if (WARN_ON(pll->active_mask)) { + + DRM_ERROR("Shared DPLL in use. active_mask:%x\n", + pll->active_mask); + pll = NULL; + } + tmp_pll_config = pll->config; + if (!bxt_ddi_dp_set_dpll_hw_state(clock, + &pll->config.hw_state)) { + DRM_ERROR("Could not setup DPLL\n"); + pll->config = tmp_pll_config; + } + } else if (IS_SKYLAKE(dev_priv)) { + pll = skl_find_link_pll(dev_priv, clock); + } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { + pll = hsw_ddi_dp_get_dpll(encoder, clock); + } + return pll; +} + void intel_ddi_init(struct drm_device *dev, enum port port) { struct drm_i915_private *dev_priv = to_i915(dev); diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c index 9a1da98..4b067ac 100644 --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c @@ -24,6 +24,44 @@ #include "intel_drv.h" struct intel_shared_dpll * +skl_find_link_pll(struct drm_i915_private *dev_priv, int clock) +{ + struct intel_shared_dpll *pll = NULL; + struct intel_dpll_hw_state dpll_hw_state; + enum intel_dpll_id i; + bool found = false; + + if (!skl_ddi_dp_set_dpll_hw_state(clock, &dpll_hw_state)) + return pll; + + for (i = DPLL_ID_SKL_DPLL1; i <= DPLL_ID_SKL_DPLL3; i++) { + pll = &dev_priv->shared_dplls[i]; + + /* Only want to check enabled timings first */ + if (pll->config.crtc_mask == 0) + continue; + + if (memcmp(&dpll_hw_state, &pll->config.hw_state, + sizeof(pll->config.hw_state)) == 0) { + found = true; + break; + } + } + + /* Ok no matching timings, maybe there's a free one? */ + for (i = DPLL_ID_SKL_DPLL1; + ((found == false) && (i <= DPLL_ID_SKL_DPLL3)); i++) { + pll = &dev_priv->shared_dplls[i]; + if (pll->config.crtc_mask == 0) { + pll->config.hw_state = dpll_hw_state; + break; + } + } + + return pll; +} + +struct intel_shared_dpll * intel_get_shared_dpll_by_id(struct drm_i915_private *dev_priv, enum intel_dpll_id id) { diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h b/drivers/gpu/drm/i915/intel_dpll_mgr.h index aed7408..f438535 100644 --- a/drivers/gpu/drm/i915/intel_dpll_mgr.h +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h @@ -168,6 +168,8 @@ bool bxt_ddi_dp_set_dpll_hw_state(int clock, /* SKL dpll related functions */ bool skl_ddi_dp_set_dpll_hw_state(int clock, struct intel_dpll_hw_state *dpll_hw_state); +struct intel_shared_dpll *skl_find_link_pll(struct drm_i915_private *dev_priv, + int clock); /* HSW dpll related functions */ diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 529fa7b..efcd80b 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1159,7 +1159,8 @@ void intel_ddi_clock_get(struct intel_encoder *encoder, struct intel_crtc_state *pipe_config); void intel_ddi_set_vc_payload_alloc(struct drm_crtc *crtc, bool state); uint32_t ddi_signal_levels(struct intel_dp *intel_dp); - +struct intel_shared_dpll *intel_ddi_get_link_dpll(struct intel_dp *intel_dp, + int clock); unsigned int intel_fb_align_height(struct drm_device *dev, unsigned int height, uint32_t pixel_format,