On Wed, Nov 01, 2017 at 11:44:58AM -0700, Rodrigo Vivi wrote: > It is unlikely we are getting the -1 here. > But if we propagate that pll_id -1 to the rest of the code > we might have funny calculations on link_clock and who > knows what registers we end up accessing. > > Better to protect the code. > > Also better with errno number instead of generic -1. > > Cc: Manasi Navare <manasi.d.navare@xxxxxxxxx> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> > --- > drivers/gpu/drm/i915/intel_ddi.c | 10 ++++++++++ > drivers/gpu/drm/i915/intel_dpll_mgr.c | 2 +- > 2 files changed, 11 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > index ace674cd79b9..2c6abdbf33ea 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -1283,6 +1283,11 @@ static void cnl_ddi_clock_get(struct intel_encoder *encoder, > > pll_id = intel_get_shared_dpll_id(dev_priv, pipe_config->shared_dpll); > > + if (pll_id < 0) { > + DRM_ERROR("PLL not found\n"); > + return; > + } > + > cfgcr0 = I915_READ(CNL_DPLL_CFGCR0(pll_id)); Don't we have the dpll state already read out? crtc_state->dll_hw_state.cfgcr0 etc.? > > if (cfgcr0 & DPLL_CFGCR0_HDMI_MODE) { > @@ -1337,6 +1342,11 @@ static void skl_ddi_clock_get(struct intel_encoder *encoder, > > pll_id = intel_get_shared_dpll_id(dev_priv, pipe_config->shared_dpll); > > + if (pll_id < 0) { > + DRM_ERROR("PLL not found\n"); > + return; > + } > + > dpll_ctl1 = I915_READ(DPLL_CTRL1); > > if (dpll_ctl1 & DPLL_CTRL1_HDMI_MODE(pll_id)) { > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c > index a83bf1c38e05..c4a7f39e173a 100644 > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c > @@ -102,7 +102,7 @@ intel_get_shared_dpll_id(struct drm_i915_private *dev_priv, > { > if (WARN_ON(pll < dev_priv->shared_dplls|| > pll > &dev_priv->shared_dplls[dev_priv->num_shared_dpll])) > - return -1; > + return -ENOENT; > > return (enum intel_dpll_id) (pll - dev_priv->shared_dplls); > } > -- > 2.13.6 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx