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@xxxxxxxxxxxxxxx>
> >
> > 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@xxxxxxxxx>
> > Signed-off-by: Manasi Navare <manasi.d.navare@xxxxxxxxx>
> > Signed-off-by: Jim Bride <jim.bride@xxxxxxxxxxxxxxx>
> > ---
> > 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,
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
