From mboxrd@z Thu Jan 1 00:00:00 1970 From: Imre Deak Subject: Re: [v2] drm/i915: Add correct hw/sw config check for DSI encoder Date: Tue, 29 Jul 2014 15:22:54 +0300 Message-ID: <1406636574.9702.41.camel@intelbox> References: <1405428329-5575-1-git-send-email-shobhit.kumar@intel.com> Reply-To: imre.deak@intel.com Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0001987881==" Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTP id 1CD9F6E023 for ; Tue, 29 Jul 2014 05:22:59 -0700 (PDT) In-Reply-To: <1405428329-5575-1-git-send-email-shobhit.kumar@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Shobhit Kumar Cc: Daniel Vetter , intel-gfx List-Id: intel-gfx@lists.freedesktop.org --===============0001987881== Content-Type: multipart/signed; micalg="pgp-sha1"; protocol="application/pgp-signature"; boundary="=-qu8MZSmVj3SwpMglCHwZ" --=-qu8MZSmVj3SwpMglCHwZ Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Tue, 2014-07-15 at 18:15 +0530, Shobhit Kumar wrote: > Check in vlv_crtc_clock_get if DPLL is enabled before calling dpio read. > It will not be enabled for DSI and avoid dpio read WARN dumps. >=20 > Absence of ->get_config was causing other WARN dumps as well. Update > dpll_hw_state as well correctly >=20 > v2: Address review comments by Daniel > - Check if DPLL is enabled rather than checking pipe output type > - set adjusted_mode->flags to 0 in compute_config rather than using > pipe_config->quirks > - Add helper function in intel_dsi_pll.c and use that in intel_dsi.c > - updated dpll_hw_state correctly > - Updated commit message and title >=20 > Signed-off-by: Shobhit Kumar Ok, reviewing now the latest version after Daniel pointed me to it. > --- > drivers/gpu/drm/i915/intel_display.c | 4 ++++ > drivers/gpu/drm/i915/intel_dsi.c | 21 +++++++++++++++- > drivers/gpu/drm/i915/intel_dsi.h | 1 + > drivers/gpu/drm/i915/intel_dsi_pll.c | 46 ++++++++++++++++++++++++++++++= ++++++ > 4 files changed, 71 insertions(+), 1 deletion(-) >=20 > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/= intel_display.c > index c89b4ac..d9c34e4 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -6132,6 +6132,10 @@ static void vlv_crtc_clock_get(struct intel_crtc *= crtc, > u32 mdiv; > int refclk =3D 100000; > =20 > + /* In case of MIPI DPLL will not even be used */ > + if (!(pipe_config->dpll_hw_state.dpll & DPLL_VCO_ENABLE)) > + return; > + > mutex_lock(&dev_priv->dpio_lock); > mdiv =3D vlv_dpio_read(dev_priv, pipe, VLV_PLL_DW3(pipe)); > mutex_unlock(&dev_priv->dpio_lock); > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/inte= l_dsi.c > index bfcefbf..43be71bf 100644 > --- a/drivers/gpu/drm/i915/intel_dsi.c > +++ b/drivers/gpu/drm/i915/intel_dsi.c > @@ -92,6 +92,9 @@ static bool intel_dsi_compute_config(struct intel_encod= er *encoder, > if (fixed_mode) > intel_fixed_panel_mode(fixed_mode, adjusted_mode); > =20 > + /* DSI uses short packets for sync events, so clear mode flags for DSI = */ > + adjusted_mode->flags =3D 0; > + > if (intel_dsi->dev.dev_ops->mode_fixup) > return intel_dsi->dev.dev_ops->mode_fixup(&intel_dsi->dev, > mode, adjusted_mode); > @@ -177,6 +180,10 @@ static void intel_dsi_pre_enable(struct intel_encode= r *encoder) > tmp |=3D DPLL_REFA_CLK_ENABLE_VLV; > I915_WRITE(DPLL(pipe), tmp); > =20 > + /* update the hw state for DPLL */ > + intel_crtc->config.dpll_hw_state.dpll =3D DPLL_INTEGRATED_CLOCK_VLV | > + DPLL_REFA_CLK_ENABLE_VLV; > + > tmp =3D I915_READ(DSPCLK_GATE_D); > tmp |=3D DPOUNIT_CLOCK_GATE_DISABLE; > I915_WRITE(DSPCLK_GATE_D, tmp); > @@ -351,9 +358,21 @@ static bool intel_dsi_get_hw_state(struct intel_enco= der *encoder, > static void intel_dsi_get_config(struct intel_encoder *encoder, > struct intel_crtc_config *pipe_config) > { > + u32 pclk; > DRM_DEBUG_KMS("\n"); > =20 > - /* XXX: read flags, set to adjusted_mode */ > + /* > + * DPLL_MD is not used in case of DSI, reading will get some default va= lue > + * set dpll_md =3D 0 > + */ > + pipe_config->dpll_hw_state.dpll_md =3D 0; > + > + pclk =3D vlv_get_dsi_pclk(encoder, pipe_config->pipe_bpp); > + if (!pclk) > + return; > + > + pipe_config->adjusted_mode.crtc_clock =3D pclk; > + pipe_config->port_clock =3D pclk; > } > =20 > static enum drm_mode_status > diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/inte= l_dsi.h > index 31db33d..fd51867 100644 > --- a/drivers/gpu/drm/i915/intel_dsi.h > +++ b/drivers/gpu/drm/i915/intel_dsi.h > @@ -132,6 +132,7 @@ static inline struct intel_dsi *enc_to_intel_dsi(stru= ct drm_encoder *encoder) > =20 > extern void vlv_enable_dsi_pll(struct intel_encoder *encoder); > extern void vlv_disable_dsi_pll(struct intel_encoder *encoder); > +extern u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp)= ; > =20 > extern struct intel_dsi_dev_ops vbt_generic_dsi_display_ops; > =20 > diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/= intel_dsi_pll.c > index ba79ec1..8085afe 100644 > --- a/drivers/gpu/drm/i915/intel_dsi_pll.c > +++ b/drivers/gpu/drm/i915/intel_dsi_pll.c > @@ -50,6 +50,8 @@ static const u32 lfsr_converts[] =3D { > 71, 35 /* 91 - 92 */ > }; > =20 > +static const int num_lfsr_converts =3D sizeof(lfsr_converts) / sizeof(lf= sr_converts[0]); > + This could be just inlined using ARRAY_SIZE. > #ifdef DSI_CLK_FROM_RR > =20 > static u32 dsi_rr_formula(const struct drm_display_mode *mode, > @@ -298,3 +300,47 @@ void vlv_disable_dsi_pll(struct intel_encoder *encod= er) > =20 > mutex_unlock(&dev_priv->dpio_lock); > } > + > +u32 vlv_get_dsi_pclk(struct intel_encoder *encoder, int pipe_bpp) > +{ > + struct drm_i915_private *dev_priv =3D encoder->base.dev->dev_private; > + struct intel_dsi *intel_dsi =3D enc_to_intel_dsi(&encoder->base); > + u32 dsi_clock, pclk; > + u32 pll_ctl, pll_div; > + u32 m =3D 0, p =3D 0; > + int refclk =3D 25000; > + int i; > + > + DRM_DEBUG_KMS("\n"); > + > + mutex_lock(&dev_priv->dpio_lock); > + pll_ctl =3D vlv_cck_read(dev_priv, CCK_REG_DSI_PLL_CONTROL); > + pll_div =3D vlv_cck_read(dev_priv, CCK_REG_DSI_PLL_DIVIDER); > + mutex_unlock(&dev_priv->dpio_lock); > + > + pll_ctl &=3D ~(DSI_PLL_CLK_GATE_DSI0_DSIPLL | DSI_PLL_VCO_EN); > + pll_ctl =3D pll_ctl >> (DSI_PLL_P1_POST_DIV_SHIFT - 2); > + > + while (pll_ctl) { > + pll_ctl =3D pll_ctl >> 1; > + p++; > + } > + p--; > + > + for (i =3D 0; i < num_lfsr_converts; i++) { > + if (lfsr_converts[i] =3D=3D pll_div) > + break; > + } > + > + if (i =3D=3D num_lfsr_converts) { > + DRM_ERROR("wrong m_seed programmed\n"); > + return 0; > + } > + > + m =3D i + 62; > + > + dsi_clock =3D (m * refclk) / p; > + pclk =3D DIV_ROUND_CLOSEST(dsi_clock * intel_dsi->lane_count, pipe_bpp)= ; > + > + return pclk; > +} Please see my comments for intel_dsi_get_config() in my v1 review where they apply in vlv_get_dsi_pclk(). --Imre --=-qu8MZSmVj3SwpMglCHwZ Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.14 (GNU/Linux) iQEcBAABAgAGBQJT15IeAAoJEORIIAnNuWDFHJAH/ixkxZ/w5gQsyKwqGcq52zzv XviOZCKTvsw93cwjfrE/2bb02J4TgcJ/ydSHv3ykQRajgLwBJqd3INW3r3ux221W 1WMVn4CUEP69Ar+n/UGuc7fuXZeN+dKImiZw9eKyOMU3uxAKLqnaFAXFbxmGppm1 5a1Uw8PJ9TO9zTtB9zsIZ1yBURQT+Cb6APnlbcqN+r1rBm4Xxu98M6CEbvYcojNu SbFzFAxkAB93TtVG2sdTapFITWZ5Bhb1t3So5qepArHuZ0JhqJtZpNgM2T2277wG NlM4F0XQNeo1bx4JehilEj6tIkaFCwrXPECJK4k2jsXt+3ctfAQt5zbAIz/Yna8= =YqTd -----END PGP SIGNATURE----- --=-qu8MZSmVj3SwpMglCHwZ-- --===============0001987881== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx --===============0001987881==--