[10/14] drm/i915/tgl: Fix dkl phy register space addressing

Message ID	20190913223251.354877-11-jose.souza@intel.com (mailing list archive)
State	New, archived
Headers	show Return-Path: <SRS0=eO8N=XI=lists.freedesktop.org=intel-gfx-bounces@kernel.org> DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 75F2220692 From: =?utf-8?q?Jos=C3=A9_Roberto_de_Souza?= <jose.souza@intel.com> To: intel-gfx@lists.freedesktop.org Date: Fri, 13 Sep 2019 15:32:47 -0700 Message-Id: <20190913223251.354877-11-jose.souza@intel.com> In-Reply-To: <20190913223251.354877-1-jose.souza@intel.com> References: <20190913223251.354877-1-jose.souza@intel.com> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH 10/14] drm/i915/tgl: Fix dkl phy register space addressing Precedence: list Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" <intel-gfx-bounces@lists.freedesktop.org>
Series	TGL TC enabling \| expand [00/14] TGL TC enabling [01/14] drm/i915/tgl: Add missing ddi clock select during DP init sequence [02/14] drm/i915/tgl: TC helper function to return pin mapping [03/14] drm/i915/tgl: Finish modular FIA support on registers [04/14] drm/i915/tgl: Fix driver crash when update_active_dpll is called [05/14] drm/i915/tgl: Add dkl phy registers [06/14] drm/i915/tgl: Add initial dkl pll support [07/14] drm/i915/tgl: Add support for dkl pll write [08/14] drm/i915/tgl: Add dkl phy programming sequences [09/14] drm/i915/icl: Unify disable and enable phy clock gating functions [10/14] drm/i915/tgl: Fix dkl phy register space addressing [11/14] drm/i915/tgl: Check the UC health of tc controllers after power on [12/14] drm/i915: Add dkl phy pll calculations [13/14] drm/i915/tgl: Use dkl pll hardcoded values [14/14] drm/i915/tgl: initialize TC and TBT ports

Message ID

20190913223251.354877-11-jose.souza@intel.com (mailing list archive)

State

New, archived

Headers

DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 75F2220692
From: =?utf-8?q?Jos=C3=A9_Roberto_de_Souza?= <jose.souza@intel.com>
To: intel-gfx@lists.freedesktop.org
Date: Fri, 13 Sep 2019 15:32:47 -0700
Message-Id: <20190913223251.354877-11-jose.souza@intel.com>
In-Reply-To: <20190913223251.354877-1-jose.souza@intel.com>
References: <20190913223251.354877-1-jose.souza@intel.com>
MIME-Version: 1.0
Subject: [Intel-gfx] [PATCH 10/14] drm/i915/tgl: Fix dkl phy register space
 addressing
Precedence: list
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: base64
Errors-To: intel-gfx-bounces@lists.freedesktop.org
Sender: "Intel-gfx" <intel-gfx-bounces@lists.freedesktop.org>

Series

TGL TC enabling | expand

Commit Message

Souza, Jose Sept. 13, 2019, 10:32 p.m. UTC

It was always modifing register space of the first phy in the
HIP_INDEX_REG for all ports while it should shift 8 bits for each
port inside of HIP_INDEX_REG.

Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c      | 34 +++++++++++++++----
 drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 10 ++++--
 drivers/gpu/drm/i915/i915_reg.h               |  3 ++
 3 files changed, 38 insertions(+), 9 deletions(-)

Comments

Lucas De Marchi Sept. 14, 2019, 7:26 a.m. UTC | #1

On Fri, Sep 13, 2019 at 3:33 PM José Roberto de Souza
<jose.souza@intel.com> wrote:
>
> It was always modifing register space of the first phy in the
> HIP_INDEX_REG for all ports while it should shift 8 bits for each
> port inside of HIP_INDEX_REG.
>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c      | 34 +++++++++++++++----
>  drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 10 ++++--
>  drivers/gpu/drm/i915/i915_reg.h               |  3 ++
>  3 files changed, 38 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index a6a2e00cc075..981e24120a87 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -2795,7 +2795,10 @@ tgl_dkl_phy_ddi_vswing_sequence(struct intel_encoder *encoder, int link_clock,
>          * All registers programmed here use HIP_INDEX_REG 0 or 1
>          */
>         for (ln = 0; ln < 2; ln++) {
> -               I915_WRITE(HIP_INDEX_REG(tc_port), ln);
> +               val = I915_READ(HIP_INDEX_REG(tc_port));
> +               val &= ~HIP_INDEX_MASK(tc_port);
> +               val |= HIP_INDEX_INDEX_VAL(tc_port, ln);

INDEX_INDEX?

> +               I915_WRITE(HIP_INDEX_REG(tc_port), val);

we don't really care for a RMW here, do we? We don't access these
registers in parallel for other ports
(It would be inherently racy if we did), so

        I915_WRITE(HIP_INDEX_REG(tc_port), HIP_INDEX_VAL(tc_port, ln));

should be sufficient.

Also I think all these fixes should be squashed in their correspondent
patches in this series with changelogs
updated.

>
>                 /* All the registers are RMW */
>                 val = I915_READ(DKL_TX_DPCNTL0(tc_port));
> @@ -3134,7 +3137,10 @@ tgl_phy_clock_gating(struct intel_digital_port *dig_port, bool enable)
>                DKL_DP_MODE_CFG_GAONPWR_GATING;
>
>         for (ln = 0; ln < 2; ln++) {
> -               I915_WRITE(HIP_INDEX_REG(tc_port), ln);
> +               val = I915_READ(HIP_INDEX_REG(tc_port));
> +               val &= ~HIP_INDEX_MASK(tc_port);
> +               val |= HIP_INDEX_INDEX_VAL(tc_port, ln);
> +               I915_WRITE(HIP_INDEX_REG(tc_port), val);
>
>                 val = I915_READ(DKL_DP_MODE(tc_port));
>                 if (enable)
> @@ -3249,16 +3255,23 @@ static void tgl_program_dkl_dp_mode(struct intel_digital_port *intel_dig_port)
>         struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
>         enum port port = intel_dig_port->base.port;
>         enum tc_port tc_port = intel_port_to_tc(dev_priv, port);
> -       u32 ln0, ln1, lane_mask, pin_mask;
> +       u32 ln0, ln1, lane_mask, pin_mask, val;
>         int num_lanes;
>
>         if (tc_port == PORT_TC_NONE ||
>             intel_dig_port->tc_mode == TC_PORT_TBT_ALT)
>                 return;
>
> -       I915_WRITE(HIP_INDEX_REG(tc_port), 0x0);
> +       val = I915_READ(HIP_INDEX_REG(tc_port));
> +       val &= ~HIP_INDEX_MASK(tc_port);
> +       val |= HIP_INDEX_INDEX_VAL(tc_port, 0x0);
> +       I915_WRITE(HIP_INDEX_REG(tc_port), val);
>         ln0 = I915_READ(DKL_DP_MODE(tc_port));
> -       I915_WRITE(HIP_INDEX_REG(tc_port), 0x1);
> +
> +       val = I915_READ(HIP_INDEX_REG(tc_port));
> +       val &= ~HIP_INDEX_MASK(tc_port);
> +       val |= HIP_INDEX_INDEX_VAL(tc_port, 0x1);
> +       I915_WRITE(HIP_INDEX_REG(tc_port), val);
>         ln1 = I915_READ(DKL_DP_MODE(tc_port));
>
>         num_lanes = intel_dig_port->dp.lane_count;
> @@ -3327,9 +3340,16 @@ static void tgl_program_dkl_dp_mode(struct intel_digital_port *intel_dig_port)
>                 return;
>         }
>
> -       I915_WRITE(HIP_INDEX_REG(tc_port), 0x0);
> +       val = I915_READ(HIP_INDEX_REG(tc_port));
> +       val &= ~HIP_INDEX_MASK(tc_port);
> +       val |= HIP_INDEX_INDEX_VAL(tc_port, 0x0);
> +       I915_WRITE(HIP_INDEX_REG(tc_port), val);
>         I915_WRITE(DKL_DP_MODE(tc_port), ln0);
> -       I915_WRITE(HIP_INDEX_REG(tc_port), 0x1);
> +
> +       val = I915_READ(HIP_INDEX_REG(tc_port));
> +       val &= ~HIP_INDEX_MASK(tc_port);
> +       val |= HIP_INDEX_INDEX_VAL(tc_port, 0x1);
> +       I915_WRITE(HIP_INDEX_REG(tc_port), val);
>         I915_WRITE(DKL_DP_MODE(tc_port), ln1);
>  }
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> index afc9b609b63d..9304606c1c0a 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> @@ -3109,7 +3109,10 @@ static bool dkl_pll_get_hw_state(struct drm_i915_private *dev_priv,
>          * All registers read here have the same HIP_INDEX_REG even though
>          * they are on different building blocks
>          */
> -       I915_WRITE(HIP_INDEX_REG(tc_port), 0x2);
> +       val = I915_READ(HIP_INDEX_REG(tc_port));
> +       val &= ~HIP_INDEX_MASK(tc_port);
> +       val |= HIP_INDEX_INDEX_VAL(tc_port, 0x2);
> +       I915_WRITE(HIP_INDEX_REG(tc_port), val);
>
>         hw_state->mg_refclkin_ctl = I915_READ(DKL_REFCLKIN_CTL(tc_port));
>         hw_state->mg_refclkin_ctl &= MG_REFCLKIN_CTL_OD_2_MUX_MASK;
> @@ -3301,7 +3304,10 @@ static void dkl_pll_write(struct drm_i915_private *dev_priv,
>          * All registers programmed here have the same HIP_INDEX_REG even
>          * though on different building block
>          */
> -       I915_WRITE(HIP_INDEX_REG(tc_port), 0x2);
> +       val = I915_READ(HIP_INDEX_REG(tc_port));
> +       val &= ~HIP_INDEX_MASK(tc_port);
> +       val |= HIP_INDEX_INDEX_VAL(tc_port, 0x2);
> +       I915_WRITE(HIP_INDEX_REG(tc_port), val);
>
>         /* All the registers are RMW */
>         val = I915_READ(DKL_REFCLKIN_CTL(tc_port));
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 95a4232c8e0a..625f458e9b1c 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -10265,6 +10265,9 @@ enum skl_power_gate {
>  #define HIP_INDEX_REG(tc_port) _MMIO((tc_port) < 4 \
>                                       ? _HIP_INDEX_REG0 \
>                                       : _HIP_INDEX_REG1)
> +#define _HIP_INDEX_SHIFT(tc_port) (8 * ((tc_port) % 4))
> +#define HIP_INDEX_MASK(tc_port) (0xff << _HIP_INDEX_SHIFT(tc_port))
> +#define HIP_INDEX_INDEX_VAL(tc_port, index) ((index) << _HIP_INDEX_SHIFT(tc_port))

#define HIP_INDEX_VAL(index) ((index) | (index) << 8 | ( index) << 16
| (index) << 24)
?

Lucas De Marchi

>
>  /* BXT display engine PLL */
>  #define BXT_DE_PLL_CTL                 _MMIO(0x6d000)
> --
> 2.23.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Souza, Jose Sept. 18, 2019, 7:55 p.m. UTC | #2

On Sat, 2019-09-14 at 00:26 -0700, Lucas De Marchi wrote:
> On Fri, Sep 13, 2019 at 3:33 PM José Roberto de Souza
> <jose.souza@intel.com> wrote:
> > It was always modifing register space of the first phy in the
> > HIP_INDEX_REG for all ports while it should shift 8 bits for each
> > port inside of HIP_INDEX_REG.
> > 
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_ddi.c      | 34
> > +++++++++++++++----
> >  drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 10 ++++--
> >  drivers/gpu/drm/i915/i915_reg.h               |  3 ++
> >  3 files changed, 38 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index a6a2e00cc075..981e24120a87 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -2795,7 +2795,10 @@ tgl_dkl_phy_ddi_vswing_sequence(struct
> > intel_encoder *encoder, int link_clock,
> >          * All registers programmed here use HIP_INDEX_REG 0 or 1
> >          */
> >         for (ln = 0; ln < 2; ln++) {
> > -               I915_WRITE(HIP_INDEX_REG(tc_port), ln);
> > +               val = I915_READ(HIP_INDEX_REG(tc_port));
> > +               val &= ~HIP_INDEX_MASK(tc_port);
> > +               val |= HIP_INDEX_INDEX_VAL(tc_port, ln);
> 
> INDEX_INDEX?
> 
> > +               I915_WRITE(HIP_INDEX_REG(tc_port), val);
> 
> we don't really care for a RMW here, do we? We don't access these
> registers in parallel for other ports
> (It would be inherently racy if we did), so
> 
>         I915_WRITE(HIP_INDEX_REG(tc_port), HIP_INDEX_VAL(tc_port,
> ln));
> 
> should be sufficient.
> 

Okay

> Also I think all these fixes should be squashed in their
> correspondent
> patches in this series with changelogs
> updated.
> 

Yeah, I was trying to avoid the fatigue :D
But I will squash in each patch

> >                 /* All the registers are RMW */
> >                 val = I915_READ(DKL_TX_DPCNTL0(tc_port));
> > @@ -3134,7 +3137,10 @@ tgl_phy_clock_gating(struct
> > intel_digital_port *dig_port, bool enable)
> >                DKL_DP_MODE_CFG_GAONPWR_GATING;
> > 
> >         for (ln = 0; ln < 2; ln++) {
> > -               I915_WRITE(HIP_INDEX_REG(tc_port), ln);
> > +               val = I915_READ(HIP_INDEX_REG(tc_port));
> > +               val &= ~HIP_INDEX_MASK(tc_port);
> > +               val |= HIP_INDEX_INDEX_VAL(tc_port, ln);
> > +               I915_WRITE(HIP_INDEX_REG(tc_port), val);
> > 
> >                 val = I915_READ(DKL_DP_MODE(tc_port));
> >                 if (enable)
> > @@ -3249,16 +3255,23 @@ static void tgl_program_dkl_dp_mode(struct
> > intel_digital_port *intel_dig_port)
> >         struct drm_i915_private *dev_priv = to_i915(intel_dig_port-
> > >base.base.dev);
> >         enum port port = intel_dig_port->base.port;
> >         enum tc_port tc_port = intel_port_to_tc(dev_priv, port);
> > -       u32 ln0, ln1, lane_mask, pin_mask;
> > +       u32 ln0, ln1, lane_mask, pin_mask, val;
> >         int num_lanes;
> > 
> >         if (tc_port == PORT_TC_NONE ||
> >             intel_dig_port->tc_mode == TC_PORT_TBT_ALT)
> >                 return;
> > 
> > -       I915_WRITE(HIP_INDEX_REG(tc_port), 0x0);
> > +       val = I915_READ(HIP_INDEX_REG(tc_port));
> > +       val &= ~HIP_INDEX_MASK(tc_port);
> > +       val |= HIP_INDEX_INDEX_VAL(tc_port, 0x0);
> > +       I915_WRITE(HIP_INDEX_REG(tc_port), val);
> >         ln0 = I915_READ(DKL_DP_MODE(tc_port));
> > -       I915_WRITE(HIP_INDEX_REG(tc_port), 0x1);
> > +
> > +       val = I915_READ(HIP_INDEX_REG(tc_port));
> > +       val &= ~HIP_INDEX_MASK(tc_port);
> > +       val |= HIP_INDEX_INDEX_VAL(tc_port, 0x1);
> > +       I915_WRITE(HIP_INDEX_REG(tc_port), val);
> >         ln1 = I915_READ(DKL_DP_MODE(tc_port));
> > 
> >         num_lanes = intel_dig_port->dp.lane_count;
> > @@ -3327,9 +3340,16 @@ static void tgl_program_dkl_dp_mode(struct
> > intel_digital_port *intel_dig_port)
> >                 return;
> >         }
> > 
> > -       I915_WRITE(HIP_INDEX_REG(tc_port), 0x0);
> > +       val = I915_READ(HIP_INDEX_REG(tc_port));
> > +       val &= ~HIP_INDEX_MASK(tc_port);
> > +       val |= HIP_INDEX_INDEX_VAL(tc_port, 0x0);
> > +       I915_WRITE(HIP_INDEX_REG(tc_port), val);
> >         I915_WRITE(DKL_DP_MODE(tc_port), ln0);
> > -       I915_WRITE(HIP_INDEX_REG(tc_port), 0x1);
> > +
> > +       val = I915_READ(HIP_INDEX_REG(tc_port));
> > +       val &= ~HIP_INDEX_MASK(tc_port);
> > +       val |= HIP_INDEX_INDEX_VAL(tc_port, 0x1);
> > +       I915_WRITE(HIP_INDEX_REG(tc_port), val);
> >         I915_WRITE(DKL_DP_MODE(tc_port), ln1);
> >  }
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > index afc9b609b63d..9304606c1c0a 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > @@ -3109,7 +3109,10 @@ static bool dkl_pll_get_hw_state(struct
> > drm_i915_private *dev_priv,
> >          * All registers read here have the same HIP_INDEX_REG even
> > though
> >          * they are on different building blocks
> >          */
> > -       I915_WRITE(HIP_INDEX_REG(tc_port), 0x2);
> > +       val = I915_READ(HIP_INDEX_REG(tc_port));
> > +       val &= ~HIP_INDEX_MASK(tc_port);
> > +       val |= HIP_INDEX_INDEX_VAL(tc_port, 0x2);
> > +       I915_WRITE(HIP_INDEX_REG(tc_port), val);
> > 
> >         hw_state->mg_refclkin_ctl =
> > I915_READ(DKL_REFCLKIN_CTL(tc_port));
> >         hw_state->mg_refclkin_ctl &= MG_REFCLKIN_CTL_OD_2_MUX_MASK;
> > @@ -3301,7 +3304,10 @@ static void dkl_pll_write(struct
> > drm_i915_private *dev_priv,
> >          * All registers programmed here have the same
> > HIP_INDEX_REG even
> >          * though on different building block
> >          */
> > -       I915_WRITE(HIP_INDEX_REG(tc_port), 0x2);
> > +       val = I915_READ(HIP_INDEX_REG(tc_port));
> > +       val &= ~HIP_INDEX_MASK(tc_port);
> > +       val |= HIP_INDEX_INDEX_VAL(tc_port, 0x2);
> > +       I915_WRITE(HIP_INDEX_REG(tc_port), val);
> > 
> >         /* All the registers are RMW */
> >         val = I915_READ(DKL_REFCLKIN_CTL(tc_port));
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 95a4232c8e0a..625f458e9b1c 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -10265,6 +10265,9 @@ enum skl_power_gate {
> >  #define HIP_INDEX_REG(tc_port) _MMIO((tc_port) < 4 \
> >                                       ? _HIP_INDEX_REG0 \
> >                                       : _HIP_INDEX_REG1)
> > +#define _HIP_INDEX_SHIFT(tc_port) (8 * ((tc_port) % 4))
> > +#define HIP_INDEX_MASK(tc_port) (0xff <<
> > _HIP_INDEX_SHIFT(tc_port))
> > +#define HIP_INDEX_INDEX_VAL(tc_port, index) ((index) <<
> > _HIP_INDEX_SHIFT(tc_port))
> 
> #define HIP_INDEX_VAL(index) ((index) | (index) << 8 | ( index) << 16
> > (index) << 24)
> ?

I guess the current one is better, more easy to understand.

> 
> Lucas De Marchi
> 
> >  /* BXT display engine PLL */
> >  #define BXT_DE_PLL_CTL                 _MMIO(0x6d000)
> > --
> > 2.23.0
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
>

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index a6a2e00cc075..981e24120a87 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -2795,7 +2795,10 @@  tgl_dkl_phy_ddi_vswing_sequence(struct intel_encoder *encoder, int link_clock,
 	 * All registers programmed here use HIP_INDEX_REG 0 or 1
 	 */
 	for (ln = 0; ln < 2; ln++) {
-		I915_WRITE(HIP_INDEX_REG(tc_port), ln);
+		val = I915_READ(HIP_INDEX_REG(tc_port));
+		val &= ~HIP_INDEX_MASK(tc_port);
+		val |= HIP_INDEX_INDEX_VAL(tc_port, ln);
+		I915_WRITE(HIP_INDEX_REG(tc_port), val);
 
 		/* All the registers are RMW */
 		val = I915_READ(DKL_TX_DPCNTL0(tc_port));
@@ -3134,7 +3137,10 @@  tgl_phy_clock_gating(struct intel_digital_port *dig_port, bool enable)
 	       DKL_DP_MODE_CFG_GAONPWR_GATING;
 
 	for (ln = 0; ln < 2; ln++) {
-		I915_WRITE(HIP_INDEX_REG(tc_port), ln);
+		val = I915_READ(HIP_INDEX_REG(tc_port));
+		val &= ~HIP_INDEX_MASK(tc_port);
+		val |= HIP_INDEX_INDEX_VAL(tc_port, ln);
+		I915_WRITE(HIP_INDEX_REG(tc_port), val);
 
 		val = I915_READ(DKL_DP_MODE(tc_port));
 		if (enable)
@@ -3249,16 +3255,23 @@  static void tgl_program_dkl_dp_mode(struct intel_digital_port *intel_dig_port)
 	struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
 	enum port port = intel_dig_port->base.port;
 	enum tc_port tc_port = intel_port_to_tc(dev_priv, port);
-	u32 ln0, ln1, lane_mask, pin_mask;
+	u32 ln0, ln1, lane_mask, pin_mask, val;
 	int num_lanes;
 
 	if (tc_port == PORT_TC_NONE ||
 	    intel_dig_port->tc_mode == TC_PORT_TBT_ALT)
 		return;
 
-	I915_WRITE(HIP_INDEX_REG(tc_port), 0x0);
+	val = I915_READ(HIP_INDEX_REG(tc_port));
+	val &= ~HIP_INDEX_MASK(tc_port);
+	val |= HIP_INDEX_INDEX_VAL(tc_port, 0x0);
+	I915_WRITE(HIP_INDEX_REG(tc_port), val);
 	ln0 = I915_READ(DKL_DP_MODE(tc_port));
-	I915_WRITE(HIP_INDEX_REG(tc_port), 0x1);
+
+	val = I915_READ(HIP_INDEX_REG(tc_port));
+	val &= ~HIP_INDEX_MASK(tc_port);
+	val |= HIP_INDEX_INDEX_VAL(tc_port, 0x1);
+	I915_WRITE(HIP_INDEX_REG(tc_port), val);
 	ln1 = I915_READ(DKL_DP_MODE(tc_port));
 
 	num_lanes = intel_dig_port->dp.lane_count;
@@ -3327,9 +3340,16 @@  static void tgl_program_dkl_dp_mode(struct intel_digital_port *intel_dig_port)
 		return;
 	}
 
-	I915_WRITE(HIP_INDEX_REG(tc_port), 0x0);
+	val = I915_READ(HIP_INDEX_REG(tc_port));
+	val &= ~HIP_INDEX_MASK(tc_port);
+	val |= HIP_INDEX_INDEX_VAL(tc_port, 0x0);
+	I915_WRITE(HIP_INDEX_REG(tc_port), val);
 	I915_WRITE(DKL_DP_MODE(tc_port), ln0);
-	I915_WRITE(HIP_INDEX_REG(tc_port), 0x1);
+
+	val = I915_READ(HIP_INDEX_REG(tc_port));
+	val &= ~HIP_INDEX_MASK(tc_port);
+	val |= HIP_INDEX_INDEX_VAL(tc_port, 0x1);
+	I915_WRITE(HIP_INDEX_REG(tc_port), val);
 	I915_WRITE(DKL_DP_MODE(tc_port), ln1);
 }
 
diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
index afc9b609b63d..9304606c1c0a 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
@@ -3109,7 +3109,10 @@  static bool dkl_pll_get_hw_state(struct drm_i915_private *dev_priv,
 	 * All registers read here have the same HIP_INDEX_REG even though
 	 * they are on different building blocks
 	 */
-	I915_WRITE(HIP_INDEX_REG(tc_port), 0x2);
+	val = I915_READ(HIP_INDEX_REG(tc_port));
+	val &= ~HIP_INDEX_MASK(tc_port);
+	val |= HIP_INDEX_INDEX_VAL(tc_port, 0x2);
+	I915_WRITE(HIP_INDEX_REG(tc_port), val);
 
 	hw_state->mg_refclkin_ctl = I915_READ(DKL_REFCLKIN_CTL(tc_port));
 	hw_state->mg_refclkin_ctl &= MG_REFCLKIN_CTL_OD_2_MUX_MASK;
@@ -3301,7 +3304,10 @@  static void dkl_pll_write(struct drm_i915_private *dev_priv,
 	 * All registers programmed here have the same HIP_INDEX_REG even
 	 * though on different building block
 	 */
-	I915_WRITE(HIP_INDEX_REG(tc_port), 0x2);
+	val = I915_READ(HIP_INDEX_REG(tc_port));
+	val &= ~HIP_INDEX_MASK(tc_port);
+	val |= HIP_INDEX_INDEX_VAL(tc_port, 0x2);
+	I915_WRITE(HIP_INDEX_REG(tc_port), val);
 
 	/* All the registers are RMW */
 	val = I915_READ(DKL_REFCLKIN_CTL(tc_port));
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 95a4232c8e0a..625f458e9b1c 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -10265,6 +10265,9 @@  enum skl_power_gate {
 #define HIP_INDEX_REG(tc_port)	_MMIO((tc_port) < 4 \
 				      ? _HIP_INDEX_REG0 \
 				      : _HIP_INDEX_REG1)
+#define _HIP_INDEX_SHIFT(tc_port) (8 * ((tc_port) % 4))
+#define HIP_INDEX_MASK(tc_port) (0xff << _HIP_INDEX_SHIFT(tc_port))
+#define HIP_INDEX_INDEX_VAL(tc_port, index) ((index) << _HIP_INDEX_SHIFT(tc_port))
 
 /* BXT display engine PLL */
 #define BXT_DE_PLL_CTL			_MMIO(0x6d000)

[10/14] drm/i915/tgl: Fix dkl phy register space addressing

Commit Message

Comments

Patch