From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Souza, Jose" Subject: Re: [PATCH] drm/i915/icl: Fix MG_DP_MODE() register programming Date: Fri, 19 Apr 2019 16:29:05 +0000 Message-ID: References: <20190419071026.32370-1-imre.deak@intel.com> <20190419160444.GB972@ideak-desk.fi.intel.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0016046492==" Return-path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by gabe.freedesktop.org (Postfix) with ESMTPS id 3BF8D892E4 for ; Fri, 19 Apr 2019 16:29:08 +0000 (UTC) In-Reply-To: <20190419160444.GB972@ideak-desk.fi.intel.com> Content-Language: en-US List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: "Deak, Imre" Cc: "De Marchi, Lucas" , "intel-gfx@lists.freedesktop.org" List-Id: intel-gfx@lists.freedesktop.org --===============0016046492== Content-Language: en-US Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="=-ZvcMS0lP6W8vEJfgZ1Cf" --=-ZvcMS0lP6W8vEJfgZ1Cf Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Fri, 2019-04-19 at 19:04 +0300, Imre Deak wrote: > On Fri, Apr 19, 2019 at 07:02:10PM +0300, Souza, Jose wrote: > > On Fri, 2019-04-19 at 10:10 +0300, Imre Deak wrote: > > > Fix the order of lane, port parameters passed to the register > > > macro. > > >=20 > > > Note that this was already partly fixed by commit > > > 37fc7845df7b6 ("drm/i915: Call MG_DP_MODE() macro with the right > > > parameters order") > > >=20 > > > Fixes: 58106b7d816e1 ("drm/i915: Make MG PHY macros semantically > > > consistent") > > > Cc: Jos=C3=A9 Roberto de Souza > > > Cc: Lucas De Marchi > > > Cc: Aditya Swarup > > > Signed-off-by: Imre Deak > > > --- > > > drivers/gpu/drm/i915/intel_ddi.c | 18 ++++++++---------- > > > 1 file changed, 8 insertions(+), 10 deletions(-) > > >=20 > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > > > b/drivers/gpu/drm/i915/intel_ddi.c > > > index 24f9106efcc6..f181c26f62fd 100644 > > > --- a/drivers/gpu/drm/i915/intel_ddi.c > > > +++ b/drivers/gpu/drm/i915/intel_ddi.c > > > @@ -2905,21 +2905,20 @@ static void > > > icl_enable_phy_clock_gating(struct intel_digital_port *dig_port) > > > struct drm_i915_private *dev_priv =3D to_i915(dig_port- > > > > base.base.dev); > > > enum port port =3D dig_port->base.port; > > > enum tc_port tc_port =3D intel_port_to_tc(dev_priv, port); > > > - i915_reg_t mg_regs[2] =3D { MG_DP_MODE(0, port), MG_DP_MODE(1, > > > port) }; > > > u32 val; > > > - int i; > > > + int ln; > > > =20 > > > if (tc_port =3D=3D PORT_TC_NONE) > > > return; > > > =20 > > > - for (i =3D 0; i < ARRAY_SIZE(mg_regs); i++) { > > > - val =3D I915_READ(mg_regs[i]); > > > + for (ln =3D 0; ln < 2; ln++) { > > > + val =3D I915_READ(MG_DP_MODE(ln, port)); > > > val |=3D MG_DP_MODE_CFG_TR2PWR_GATING | > > > MG_DP_MODE_CFG_TRPWR_GATING | > > > MG_DP_MODE_CFG_CLNPWR_GATING | > > > MG_DP_MODE_CFG_DIGPWR_GATING | > > > MG_DP_MODE_CFG_GAONPWR_GATING; > > > - I915_WRITE(mg_regs[i], val); > > > + I915_WRITE(MG_DP_MODE(ln, port), val); > > > } > > > =20 > > > val =3D I915_READ(MG_MISC_SUS0(tc_port)); > > > @@ -2938,21 +2937,20 @@ static void > > > icl_disable_phy_clock_gating(struct intel_digital_port *dig_port) > > > struct drm_i915_private *dev_priv =3D to_i915(dig_port- > > > > base.base.dev); > > > enum port port =3D dig_port->base.port; > > > enum tc_port tc_port =3D intel_port_to_tc(dev_priv, port); > > > - i915_reg_t mg_regs[2] =3D { MG_DP_MODE(port, 0), MG_DP_MODE(port, > > > 1) }; > >=20 > > I would split this fix from the change dropping mg_regs or at least > > tell that while you were fixing it you changed the way it reads > > each MG_DP_MODE line. >=20 > I don't think it's worth a separate patch, since it's a small change > and quite obvious what and how changed. I can add a note to the > commit > message about making things simpler. I suggested split because this will probably be backported so would be nice to be as clean as possible but I'm also okay if you add it to the commit description. >=20 > >=20 > > > u32 val; > > > - int i; > > > + int ln; > > > =20 > > > if (tc_port =3D=3D PORT_TC_NONE) > > > return; > > > =20 > > > - for (i =3D 0; i < ARRAY_SIZE(mg_regs); i++) { > > > - val =3D I915_READ(mg_regs[i]); > > > + for (ln =3D 0; ln < 2; ln++) { > > > + val =3D I915_READ(MG_DP_MODE(ln, port)); > > > val &=3D ~(MG_DP_MODE_CFG_TR2PWR_GATING | > > > MG_DP_MODE_CFG_TRPWR_GATING | > > > MG_DP_MODE_CFG_CLNPWR_GATING | > > > MG_DP_MODE_CFG_DIGPWR_GATING | > > > MG_DP_MODE_CFG_GAONPWR_GATING); > > > - I915_WRITE(mg_regs[i], val); > > > + I915_WRITE(MG_DP_MODE(ln, port), val); > > > } > > > =20 > > > val =3D I915_READ(MG_MISC_SUS0(tc_port)); >=20 >=20 --=-ZvcMS0lP6W8vEJfgZ1Cf Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- iQEzBAABCAAdFiEEVNG051EijGa0MiaQVenbO/mOWkkFAly591AACgkQVenbO/mO Wkkxcgf9EyFPJrI1vFolijIcWlJWCeitJUoI1SjxPgLploKJZay+6/UzoLEZAAVc J8+kBMKQ5XT2Kqv7J7I7FETMtlYmryNUMwpXnL5q/UUCUZkKERBUfRnK0FVdL7uw WbNkjhU/BNWA/8sF7d5yBJcz9k02cgyTM1pbvwMeqMigL5rId1SA7MpSwojbhsG6 jqx8S49Nu2uq0oGynsyOjACvgeadzdFRY0wc8eAZ9ArMbiaeIH/EXsF+t6r+Ol+w kaF07Q5RSBczWTD+MpcFDgDLT0uaZII4fEzvuiJnonLTF3sQbPV3kcS/LolghYs1 qyptD5Y6wd/Lc2epITlpJ7m9bCgnbw== =o2U3 -----END PGP SIGNATURE----- --=-ZvcMS0lP6W8vEJfgZ1Cf-- --===============0016046492== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4 IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vaW50ZWwtZ2Z4 --===============0016046492==--