| Message ID | 20190925234509.29505-2-jose.souza@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | TGL TC enabling v4 | expand |
On Wed, Sep 25, 2019 at 04:45:06PM -0700, José Roberto de Souza wrote: > From: Clinton A Taylor <clinton.a.taylor@intel.com> > > BSpec was updated(r146548) with a new MG_DP_MODE Programming table, > now taking in consideration the pin assignment and allowing us to > optimize power by shutting down available but not needed lanes. > > It was tested on ICL and TGL, with adaptors that used pin assignment > C and B, reversing the connector and going to different modes testing > the not needed lane shutdown. > > BSpec: 21735 > BSpec: 49292 > > Cc: Imre Deak <imre.deak@intel.com> > Cc: Lucas De Marchi <lucas.demarchi@intel.com> > Signed-off-by: Clinton A Taylor <clinton.a.taylor@intel.com> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > --- > drivers/gpu/drm/i915/display/intel_ddi.c | 82 +++++++++++++----------- > drivers/gpu/drm/i915/display/intel_tc.c | 15 +++++ > drivers/gpu/drm/i915/display/intel_tc.h | 1 + > drivers/gpu/drm/i915/i915_reg.h | 5 ++ > 4 files changed, 66 insertions(+), 37 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c > index aa470c70a198..316cedb16935 100644 > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > @@ -3095,7 +3095,8 @@ static void icl_program_mg_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; > - u32 ln0, ln1, lane_mask; > + u32 ln0, ln1, pin_assignment; > + u8 width; > > if (intel_dig_port->tc_mode == TC_PORT_TBT_ALT) > return; > @@ -3103,50 +3104,57 @@ static void icl_program_mg_dp_mode(struct intel_digital_port *intel_dig_port) > ln0 = I915_READ(MG_DP_MODE(0, port)); > ln1 = I915_READ(MG_DP_MODE(1, port)); > > - switch (intel_dig_port->tc_mode) { > - case TC_PORT_DP_ALT: > - ln0 &= ~(MG_DP_MODE_CFG_DP_X1_MODE | MG_DP_MODE_CFG_DP_X2_MODE); > - ln1 &= ~(MG_DP_MODE_CFG_DP_X1_MODE | MG_DP_MODE_CFG_DP_X2_MODE); > + ln0 &= ~(MG_DP_MODE_CFG_DP_X1_MODE | MG_DP_MODE_CFG_DP_X1_MODE); > + ln1 &= ~(MG_DP_MODE_CFG_DP_X1_MODE | MG_DP_MODE_CFG_DP_X2_MODE); > > - lane_mask = intel_tc_port_get_lane_mask(intel_dig_port); > + /* DPPATC */ > + pin_assignment = intel_tc_port_get_pin_assignment_mask(intel_dig_port); > + width = intel_dig_port->dp.lane_count; Should be crtc_state->lane_count. (dp.lane_count makes no sense for HDMI and it's also only set for link training) > > - switch (lane_mask) { > - case 0x1: > - case 0x4: > - break; > - case 0x2: > + switch (pin_assignment) { > + case 0x0: > + WARN_ON(intel_dig_port->tc_mode != TC_PORT_LEGACY); > + if (width == 1) { > + ln1 |= MG_DP_MODE_CFG_DP_X1_MODE; > + } else { > + ln0 |= MG_DP_MODE_CFG_DP_X2_MODE; > + ln1 |= MG_DP_MODE_CFG_DP_X2_MODE; > + } > + break; > + case 0x1: > + if (width == 4) { > + ln0 |= MG_DP_MODE_CFG_DP_X2_MODE; > + ln1 |= MG_DP_MODE_CFG_DP_X2_MODE; > + } > + break; > + case 0x2: > + if (width == 2) { > + ln0 |= MG_DP_MODE_CFG_DP_X2_MODE; > + ln1 |= MG_DP_MODE_CFG_DP_X2_MODE; > + } nit: WARN_ON(width==4)? > + break; > + case 0x3: > + case 0x5: > + if (width == 1) { > ln0 |= MG_DP_MODE_CFG_DP_X1_MODE; > - break; > - case 0x3: > - ln0 |= MG_DP_MODE_CFG_DP_X1_MODE | > - MG_DP_MODE_CFG_DP_X2_MODE; > - break; > - case 0x8: > ln1 |= MG_DP_MODE_CFG_DP_X1_MODE; > - break; > - case 0xC: > - ln1 |= MG_DP_MODE_CFG_DP_X1_MODE | > - MG_DP_MODE_CFG_DP_X2_MODE; > - break; > - case 0xF: > - ln0 |= MG_DP_MODE_CFG_DP_X1_MODE | > - MG_DP_MODE_CFG_DP_X2_MODE; > - ln1 |= MG_DP_MODE_CFG_DP_X1_MODE | > - MG_DP_MODE_CFG_DP_X2_MODE; > - break; > - default: > - MISSING_CASE(lane_mask); > + } else { > + ln0 |= MG_DP_MODE_CFG_DP_X2_MODE; > + ln1 |= MG_DP_MODE_CFG_DP_X2_MODE; > } > break; > - > - case TC_PORT_LEGACY: > - ln0 |= MG_DP_MODE_CFG_DP_X1_MODE | MG_DP_MODE_CFG_DP_X2_MODE; > - ln1 |= MG_DP_MODE_CFG_DP_X1_MODE | MG_DP_MODE_CFG_DP_X2_MODE; > + case 0x4: > + case 0x6: > + if (width == 1) { > + ln0 |= MG_DP_MODE_CFG_DP_X1_MODE; > + ln1 |= MG_DP_MODE_CFG_DP_X1_MODE; > + } else { > + ln0 |= MG_DP_MODE_CFG_DP_X2_MODE; > + ln1 |= MG_DP_MODE_CFG_DP_X2_MODE; > + } WARN_ON(width==4)? With the lane_count fix: Reviewed-by: Imre Deak <imre.deak@intel.com> > break; > - > default: > - MISSING_CASE(intel_dig_port->tc_mode); > - return; > + MISSING_CASE(pin_assignment); > } > > I915_WRITE(MG_DP_MODE(0, port), ln0); > diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c > index f923f9cbd33c..7773169b7331 100644 > --- a/drivers/gpu/drm/i915/display/intel_tc.c > +++ b/drivers/gpu/drm/i915/display/intel_tc.c > @@ -67,6 +67,21 @@ u32 intel_tc_port_get_lane_mask(struct intel_digital_port *dig_port) > return lane_mask >> DP_LANE_ASSIGNMENT_SHIFT(dig_port->tc_phy_fia_idx); > } > > +u32 intel_tc_port_get_pin_assignment_mask(struct intel_digital_port *dig_port) > +{ > + struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev); > + struct intel_uncore *uncore = &i915->uncore; > + u32 pin_mask; > + > + pin_mask = intel_uncore_read(uncore, > + PORT_TX_DFLEXPA1(dig_port->tc_phy_fia)); > + > + WARN_ON(pin_mask == 0xffffffff); > + > + return (pin_mask & DP_PIN_ASSIGNMENT_MASK(dig_port->tc_phy_fia_idx)) >> > + DP_PIN_ASSIGNMENT_SHIFT(dig_port->tc_phy_fia_idx); > +} > + > int intel_tc_port_fia_max_lane_count(struct intel_digital_port *dig_port) > { > struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev); > diff --git a/drivers/gpu/drm/i915/display/intel_tc.h b/drivers/gpu/drm/i915/display/intel_tc.h > index 783d75531435..463f1b3c836f 100644 > --- a/drivers/gpu/drm/i915/display/intel_tc.h > +++ b/drivers/gpu/drm/i915/display/intel_tc.h > @@ -13,6 +13,7 @@ struct intel_digital_port; > > bool intel_tc_port_connected(struct intel_digital_port *dig_port); > u32 intel_tc_port_get_lane_mask(struct intel_digital_port *dig_port); > +u32 intel_tc_port_get_pin_assignment_mask(struct intel_digital_port *dig_port); > int intel_tc_port_fia_max_lane_count(struct intel_digital_port *dig_port); > void intel_tc_port_set_fia_lane_count(struct intel_digital_port *dig_port, > int required_lanes); > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index e752de9470bd..bcf449c1d152 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -11857,6 +11857,11 @@ enum skl_power_gate { > #define PORT_TX_DFLEXDPCSSS(fia) _MMIO_FIA((fia), 0x00894) > #define DP_PHY_MODE_STATUS_NOT_SAFE(idx) (1 << (idx)) > > +#define PORT_TX_DFLEXPA1(fia) _MMIO_FIA((fia), 0x00880) > +#define DP_PIN_ASSIGNMENT_SHIFT(idx) ((idx) * 4) > +#define DP_PIN_ASSIGNMENT_MASK(idx) (0xf << ((idx) * 4)) > +#define DP_PIN_ASSIGNMENT(idx, x) ((x) << ((idx) * 4)) > + > /* This register controls the Display State Buffer (DSB) engines. */ > #define _DSBSL_INSTANCE_BASE 0x70B00 > #define DSBSL_INSTANCE(pipe, id) (_DSBSL_INSTANCE_BASE + \ > -- > 2.23.0 >
On Thu, 2019-09-26 at 15:02 +0300, Imre Deak wrote: > On Wed, Sep 25, 2019 at 04:45:06PM -0700, José Roberto de Souza > wrote: > > From: Clinton A Taylor <clinton.a.taylor@intel.com> > > > > BSpec was updated(r146548) with a new MG_DP_MODE Programming table, > > now taking in consideration the pin assignment and allowing us to > > optimize power by shutting down available but not needed lanes. > > > > It was tested on ICL and TGL, with adaptors that used pin > > assignment > > C and B, reversing the connector and going to different modes > > testing > > the not needed lane shutdown. > > > > BSpec: 21735 > > BSpec: 49292 > > > > Cc: Imre Deak <imre.deak@intel.com> > > Cc: Lucas De Marchi <lucas.demarchi@intel.com> > > Signed-off-by: Clinton A Taylor <clinton.a.taylor@intel.com> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_ddi.c | 82 +++++++++++++------- > > ---- > > drivers/gpu/drm/i915/display/intel_tc.c | 15 +++++ > > drivers/gpu/drm/i915/display/intel_tc.h | 1 + > > drivers/gpu/drm/i915/i915_reg.h | 5 ++ > > 4 files changed, 66 insertions(+), 37 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c > > b/drivers/gpu/drm/i915/display/intel_ddi.c > > index aa470c70a198..316cedb16935 100644 > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > > @@ -3095,7 +3095,8 @@ static void icl_program_mg_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; > > - u32 ln0, ln1, lane_mask; > > + u32 ln0, ln1, pin_assignment; > > + u8 width; > > > > if (intel_dig_port->tc_mode == TC_PORT_TBT_ALT) > > return; > > @@ -3103,50 +3104,57 @@ static void icl_program_mg_dp_mode(struct > > intel_digital_port *intel_dig_port) > > ln0 = I915_READ(MG_DP_MODE(0, port)); > > ln1 = I915_READ(MG_DP_MODE(1, port)); > > > > - switch (intel_dig_port->tc_mode) { > > - case TC_PORT_DP_ALT: > > - ln0 &= ~(MG_DP_MODE_CFG_DP_X1_MODE | > > MG_DP_MODE_CFG_DP_X2_MODE); > > - ln1 &= ~(MG_DP_MODE_CFG_DP_X1_MODE | > > MG_DP_MODE_CFG_DP_X2_MODE); > > + ln0 &= ~(MG_DP_MODE_CFG_DP_X1_MODE | > > MG_DP_MODE_CFG_DP_X1_MODE); > > + ln1 &= ~(MG_DP_MODE_CFG_DP_X1_MODE | > > MG_DP_MODE_CFG_DP_X2_MODE); > > > > - lane_mask = > > intel_tc_port_get_lane_mask(intel_dig_port); > > + /* DPPATC */ > > + pin_assignment = > > intel_tc_port_get_pin_assignment_mask(intel_dig_port); > > + width = intel_dig_port->dp.lane_count; > > Should be crtc_state->lane_count. (dp.lane_count makes no sense for > HDMI > and it's also only set for link training) Done > > > > > - switch (lane_mask) { > > - case 0x1: > > - case 0x4: > > - break; > > - case 0x2: > > + switch (pin_assignment) { > > + case 0x0: > > + WARN_ON(intel_dig_port->tc_mode != TC_PORT_LEGACY); > > + if (width == 1) { > > + ln1 |= MG_DP_MODE_CFG_DP_X1_MODE; > > + } else { > > + ln0 |= MG_DP_MODE_CFG_DP_X2_MODE; > > + ln1 |= MG_DP_MODE_CFG_DP_X2_MODE; > > + } > > + break; > > + case 0x1: > > + if (width == 4) { > > + ln0 |= MG_DP_MODE_CFG_DP_X2_MODE; > > + ln1 |= MG_DP_MODE_CFG_DP_X2_MODE; > > + } > > + break; > > + case 0x2: > > + if (width == 2) { > > + ln0 |= MG_DP_MODE_CFG_DP_X2_MODE; > > + ln1 |= MG_DP_MODE_CFG_DP_X2_MODE; > > + } > > nit: WARN_ON(width==4)? This lane assignment only has 2 lanes, so it will never be 4. > > > + break; > > + case 0x3: > > + case 0x5: > > + if (width == 1) { > > ln0 |= MG_DP_MODE_CFG_DP_X1_MODE; > > - break; > > - case 0x3: > > - ln0 |= MG_DP_MODE_CFG_DP_X1_MODE | > > - MG_DP_MODE_CFG_DP_X2_MODE; > > - break; > > - case 0x8: > > ln1 |= MG_DP_MODE_CFG_DP_X1_MODE; > > - break; > > - case 0xC: > > - ln1 |= MG_DP_MODE_CFG_DP_X1_MODE | > > - MG_DP_MODE_CFG_DP_X2_MODE; > > - break; > > - case 0xF: > > - ln0 |= MG_DP_MODE_CFG_DP_X1_MODE | > > - MG_DP_MODE_CFG_DP_X2_MODE; > > - ln1 |= MG_DP_MODE_CFG_DP_X1_MODE | > > - MG_DP_MODE_CFG_DP_X2_MODE; > > - break; > > - default: > > - MISSING_CASE(lane_mask); > > + } else { > > + ln0 |= MG_DP_MODE_CFG_DP_X2_MODE; > > + ln1 |= MG_DP_MODE_CFG_DP_X2_MODE; > > } > > break; > > - > > - case TC_PORT_LEGACY: > > - ln0 |= MG_DP_MODE_CFG_DP_X1_MODE | > > MG_DP_MODE_CFG_DP_X2_MODE; > > - ln1 |= MG_DP_MODE_CFG_DP_X1_MODE | > > MG_DP_MODE_CFG_DP_X2_MODE; > > + case 0x4: > > + case 0x6: > > + if (width == 1) { > > + ln0 |= MG_DP_MODE_CFG_DP_X1_MODE; > > + ln1 |= MG_DP_MODE_CFG_DP_X1_MODE; > > + } else { > > + ln0 |= MG_DP_MODE_CFG_DP_X2_MODE; > > + ln1 |= MG_DP_MODE_CFG_DP_X2_MODE; > > + } > > WARN_ON(width==4)? Same as above. > > With the lane_count fix: > Reviewed-by: Imre Deak <imre.deak@intel.com> Thanks > > > break; > > - > > default: > > - MISSING_CASE(intel_dig_port->tc_mode); > > - return; > > + MISSING_CASE(pin_assignment); > > } > > > > I915_WRITE(MG_DP_MODE(0, port), ln0); > > diff --git a/drivers/gpu/drm/i915/display/intel_tc.c > > b/drivers/gpu/drm/i915/display/intel_tc.c > > index f923f9cbd33c..7773169b7331 100644 > > --- a/drivers/gpu/drm/i915/display/intel_tc.c > > +++ b/drivers/gpu/drm/i915/display/intel_tc.c > > @@ -67,6 +67,21 @@ u32 intel_tc_port_get_lane_mask(struct > > intel_digital_port *dig_port) > > return lane_mask >> DP_LANE_ASSIGNMENT_SHIFT(dig_port- > > >tc_phy_fia_idx); > > } > > > > +u32 intel_tc_port_get_pin_assignment_mask(struct > > intel_digital_port *dig_port) > > +{ > > + struct drm_i915_private *i915 = to_i915(dig_port- > > >base.base.dev); > > + struct intel_uncore *uncore = &i915->uncore; > > + u32 pin_mask; > > + > > + pin_mask = intel_uncore_read(uncore, > > + PORT_TX_DFLEXPA1(dig_port- > > >tc_phy_fia)); > > + > > + WARN_ON(pin_mask == 0xffffffff); > > + > > + return (pin_mask & DP_PIN_ASSIGNMENT_MASK(dig_port- > > >tc_phy_fia_idx)) >> > > + DP_PIN_ASSIGNMENT_SHIFT(dig_port->tc_phy_fia_idx); > > +} > > + > > int intel_tc_port_fia_max_lane_count(struct intel_digital_port > > *dig_port) > > { > > struct drm_i915_private *i915 = to_i915(dig_port- > > >base.base.dev); > > diff --git a/drivers/gpu/drm/i915/display/intel_tc.h > > b/drivers/gpu/drm/i915/display/intel_tc.h > > index 783d75531435..463f1b3c836f 100644 > > --- a/drivers/gpu/drm/i915/display/intel_tc.h > > +++ b/drivers/gpu/drm/i915/display/intel_tc.h > > @@ -13,6 +13,7 @@ struct intel_digital_port; > > > > bool intel_tc_port_connected(struct intel_digital_port *dig_port); > > u32 intel_tc_port_get_lane_mask(struct intel_digital_port > > *dig_port); > > +u32 intel_tc_port_get_pin_assignment_mask(struct > > intel_digital_port *dig_port); > > int intel_tc_port_fia_max_lane_count(struct intel_digital_port > > *dig_port); > > void intel_tc_port_set_fia_lane_count(struct intel_digital_port > > *dig_port, > > int required_lanes); > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > b/drivers/gpu/drm/i915/i915_reg.h > > index e752de9470bd..bcf449c1d152 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -11857,6 +11857,11 @@ enum skl_power_gate { > > #define PORT_TX_DFLEXDPCSSS(fia) _MMIO_FIA((fia), > > 0x00894) > > #define DP_PHY_MODE_STATUS_NOT_SAFE(idx) (1 << (idx)) > > > > +#define PORT_TX_DFLEXPA1(fia) _MMIO_FIA((fia) > > , 0x00880) > > +#define DP_PIN_ASSIGNMENT_SHIFT(idx) ((idx) * 4) > > +#define DP_PIN_ASSIGNMENT_MASK(idx) (0xf << ((idx) > > * 4)) > > +#define DP_PIN_ASSIGNMENT(idx, x) ((x) << ((idx) > > * 4)) > > + > > /* This register controls the Display State Buffer (DSB) engines. > > */ > > #define _DSBSL_INSTANCE_BASE 0x70B00 > > #define DSBSL_INSTANCE(pipe, id) (_DSBSL_INSTANCE_BASE + \ > > -- > > 2.23.0 > >
On Thu, Sep 26, 2019 at 10:35:16PM +0300, Souza, Jose wrote: > On Thu, 2019-09-26 at 15:02 +0300, Imre Deak wrote: > > On Wed, Sep 25, 2019 at 04:45:06PM -0700, José Roberto de Souza > > wrote: > > > From: Clinton A Taylor <clinton.a.taylor@intel.com> > > > > > > BSpec was updated(r146548) with a new MG_DP_MODE Programming table, > > > now taking in consideration the pin assignment and allowing us to > > > optimize power by shutting down available but not needed lanes. > > > > > > It was tested on ICL and TGL, with adaptors that used pin > > > assignment > > > C and B, reversing the connector and going to different modes > > > testing > > > the not needed lane shutdown. > > > > > > BSpec: 21735 > > > BSpec: 49292 > > > > > > Cc: Imre Deak <imre.deak@intel.com> > > > Cc: Lucas De Marchi <lucas.demarchi@intel.com> > > > Signed-off-by: Clinton A Taylor <clinton.a.taylor@intel.com> > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > > > --- > > > drivers/gpu/drm/i915/display/intel_ddi.c | 82 +++++++++++++------- > > > ---- > > > drivers/gpu/drm/i915/display/intel_tc.c | 15 +++++ > > > drivers/gpu/drm/i915/display/intel_tc.h | 1 + > > > drivers/gpu/drm/i915/i915_reg.h | 5 ++ > > > 4 files changed, 66 insertions(+), 37 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c > > > b/drivers/gpu/drm/i915/display/intel_ddi.c > > > index aa470c70a198..316cedb16935 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > > > @@ -3095,7 +3095,8 @@ static void icl_program_mg_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; > > > - u32 ln0, ln1, lane_mask; > > > + u32 ln0, ln1, pin_assignment; > > > + u8 width; > > > > > > if (intel_dig_port->tc_mode == TC_PORT_TBT_ALT) > > > return; > > > @@ -3103,50 +3104,57 @@ static void icl_program_mg_dp_mode(struct > > > intel_digital_port *intel_dig_port) > > > ln0 = I915_READ(MG_DP_MODE(0, port)); > > > ln1 = I915_READ(MG_DP_MODE(1, port)); > > > > > > - switch (intel_dig_port->tc_mode) { > > > - case TC_PORT_DP_ALT: > > > - ln0 &= ~(MG_DP_MODE_CFG_DP_X1_MODE | > > > MG_DP_MODE_CFG_DP_X2_MODE); > > > - ln1 &= ~(MG_DP_MODE_CFG_DP_X1_MODE | > > > MG_DP_MODE_CFG_DP_X2_MODE); > > > + ln0 &= ~(MG_DP_MODE_CFG_DP_X1_MODE | > > > MG_DP_MODE_CFG_DP_X1_MODE); > > > + ln1 &= ~(MG_DP_MODE_CFG_DP_X1_MODE | > > > MG_DP_MODE_CFG_DP_X2_MODE); > > > > > > - lane_mask = > > > intel_tc_port_get_lane_mask(intel_dig_port); > > > + /* DPPATC */ > > > + pin_assignment = > > > intel_tc_port_get_pin_assignment_mask(intel_dig_port); > > > + width = intel_dig_port->dp.lane_count; > > > > Should be crtc_state->lane_count. (dp.lane_count makes no sense for > > HDMI > > and it's also only set for link training) > > Done > > > > > > > > > - switch (lane_mask) { > > > - case 0x1: > > > - case 0x4: > > > - break; > > > - case 0x2: > > > + switch (pin_assignment) { > > > + case 0x0: > > > + WARN_ON(intel_dig_port->tc_mode != TC_PORT_LEGACY); > > > + if (width == 1) { > > > + ln1 |= MG_DP_MODE_CFG_DP_X1_MODE; > > > + } else { > > > + ln0 |= MG_DP_MODE_CFG_DP_X2_MODE; > > > + ln1 |= MG_DP_MODE_CFG_DP_X2_MODE; > > > + } > > > + break; > > > + case 0x1: > > > + if (width == 4) { > > > + ln0 |= MG_DP_MODE_CFG_DP_X2_MODE; > > > + ln1 |= MG_DP_MODE_CFG_DP_X2_MODE; > > > + } > > > + break; > > > + case 0x2: > > > + if (width == 2) { > > > + ln0 |= MG_DP_MODE_CFG_DP_X2_MODE; > > > + ln1 |= MG_DP_MODE_CFG_DP_X2_MODE; > > > + } > > > > nit: WARN_ON(width==4)? > > This lane assignment only has 2 lanes, so it will never be 4. Yes, hence the warn in case we attempted to configure 4 lanes. > > > > > > + break; > > > + case 0x3: > > > + case 0x5: > > > + if (width == 1) { > > > ln0 |= MG_DP_MODE_CFG_DP_X1_MODE; > > > - break; > > > - case 0x3: > > > - ln0 |= MG_DP_MODE_CFG_DP_X1_MODE | > > > - MG_DP_MODE_CFG_DP_X2_MODE; > > > - break; > > > - case 0x8: > > > ln1 |= MG_DP_MODE_CFG_DP_X1_MODE; > > > - break; > > > - case 0xC: > > > - ln1 |= MG_DP_MODE_CFG_DP_X1_MODE | > > > - MG_DP_MODE_CFG_DP_X2_MODE; > > > - break; > > > - case 0xF: > > > - ln0 |= MG_DP_MODE_CFG_DP_X1_MODE | > > > - MG_DP_MODE_CFG_DP_X2_MODE; > > > - ln1 |= MG_DP_MODE_CFG_DP_X1_MODE | > > > - MG_DP_MODE_CFG_DP_X2_MODE; > > > - break; > > > - default: > > > - MISSING_CASE(lane_mask); > > > + } else { > > > + ln0 |= MG_DP_MODE_CFG_DP_X2_MODE; > > > + ln1 |= MG_DP_MODE_CFG_DP_X2_MODE; > > > } > > > break; > > > - > > > - case TC_PORT_LEGACY: > > > - ln0 |= MG_DP_MODE_CFG_DP_X1_MODE | > > > MG_DP_MODE_CFG_DP_X2_MODE; > > > - ln1 |= MG_DP_MODE_CFG_DP_X1_MODE | > > > MG_DP_MODE_CFG_DP_X2_MODE; > > > + case 0x4: > > > + case 0x6: > > > + if (width == 1) { > > > + ln0 |= MG_DP_MODE_CFG_DP_X1_MODE; > > > + ln1 |= MG_DP_MODE_CFG_DP_X1_MODE; > > > + } else { > > > + ln0 |= MG_DP_MODE_CFG_DP_X2_MODE; > > > + ln1 |= MG_DP_MODE_CFG_DP_X2_MODE; > > > + } > > > > WARN_ON(width==4)? > > Same as above. > > > > > With the lane_count fix: > > Reviewed-by: Imre Deak <imre.deak@intel.com> > > Thanks > > > > > > break; > > > - > > > default: > > > - MISSING_CASE(intel_dig_port->tc_mode); > > > - return; > > > + MISSING_CASE(pin_assignment); > > > } > > > > > > I915_WRITE(MG_DP_MODE(0, port), ln0); > > > diff --git a/drivers/gpu/drm/i915/display/intel_tc.c > > > b/drivers/gpu/drm/i915/display/intel_tc.c > > > index f923f9cbd33c..7773169b7331 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_tc.c > > > +++ b/drivers/gpu/drm/i915/display/intel_tc.c > > > @@ -67,6 +67,21 @@ u32 intel_tc_port_get_lane_mask(struct > > > intel_digital_port *dig_port) > > > return lane_mask >> DP_LANE_ASSIGNMENT_SHIFT(dig_port- > > > >tc_phy_fia_idx); > > > } > > > > > > +u32 intel_tc_port_get_pin_assignment_mask(struct > > > intel_digital_port *dig_port) > > > +{ > > > + struct drm_i915_private *i915 = to_i915(dig_port- > > > >base.base.dev); > > > + struct intel_uncore *uncore = &i915->uncore; > > > + u32 pin_mask; > > > + > > > + pin_mask = intel_uncore_read(uncore, > > > + PORT_TX_DFLEXPA1(dig_port- > > > >tc_phy_fia)); > > > + > > > + WARN_ON(pin_mask == 0xffffffff); > > > + > > > + return (pin_mask & DP_PIN_ASSIGNMENT_MASK(dig_port- > > > >tc_phy_fia_idx)) >> > > > + DP_PIN_ASSIGNMENT_SHIFT(dig_port->tc_phy_fia_idx); > > > +} > > > + > > > int intel_tc_port_fia_max_lane_count(struct intel_digital_port > > > *dig_port) > > > { > > > struct drm_i915_private *i915 = to_i915(dig_port- > > > >base.base.dev); > > > diff --git a/drivers/gpu/drm/i915/display/intel_tc.h > > > b/drivers/gpu/drm/i915/display/intel_tc.h > > > index 783d75531435..463f1b3c836f 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_tc.h > > > +++ b/drivers/gpu/drm/i915/display/intel_tc.h > > > @@ -13,6 +13,7 @@ struct intel_digital_port; > > > > > > bool intel_tc_port_connected(struct intel_digital_port *dig_port); > > > u32 intel_tc_port_get_lane_mask(struct intel_digital_port > > > *dig_port); > > > +u32 intel_tc_port_get_pin_assignment_mask(struct > > > intel_digital_port *dig_port); > > > int intel_tc_port_fia_max_lane_count(struct intel_digital_port > > > *dig_port); > > > void intel_tc_port_set_fia_lane_count(struct intel_digital_port > > > *dig_port, > > > int required_lanes); > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > > b/drivers/gpu/drm/i915/i915_reg.h > > > index e752de9470bd..bcf449c1d152 100644 > > > --- a/drivers/gpu/drm/i915/i915_reg.h > > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > > @@ -11857,6 +11857,11 @@ enum skl_power_gate { > > > #define PORT_TX_DFLEXDPCSSS(fia) _MMIO_FIA((fia), > > > 0x00894) > > > #define DP_PHY_MODE_STATUS_NOT_SAFE(idx) (1 << (idx)) > > > > > > +#define PORT_TX_DFLEXPA1(fia) _MMIO_FIA((fia) > > > , 0x00880) > > > +#define DP_PIN_ASSIGNMENT_SHIFT(idx) ((idx) * 4) > > > +#define DP_PIN_ASSIGNMENT_MASK(idx) (0xf << ((idx) > > > * 4)) > > > +#define DP_PIN_ASSIGNMENT(idx, x) ((x) << ((idx) > > > * 4)) > > > + > > > /* This register controls the Display State Buffer (DSB) engines. > > > */ > > > #define _DSBSL_INSTANCE_BASE 0x70B00 > > > #define DSBSL_INSTANCE(pipe, id) (_DSBSL_INSTANCE_BASE + \ > > > -- > > > 2.23.0 > > >
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c index aa470c70a198..316cedb16935 100644 --- a/drivers/gpu/drm/i915/display/intel_ddi.c +++ b/drivers/gpu/drm/i915/display/intel_ddi.c @@ -3095,7 +3095,8 @@ static void icl_program_mg_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; - u32 ln0, ln1, lane_mask; + u32 ln0, ln1, pin_assignment; + u8 width; if (intel_dig_port->tc_mode == TC_PORT_TBT_ALT) return; @@ -3103,50 +3104,57 @@ static void icl_program_mg_dp_mode(struct intel_digital_port *intel_dig_port) ln0 = I915_READ(MG_DP_MODE(0, port)); ln1 = I915_READ(MG_DP_MODE(1, port)); - switch (intel_dig_port->tc_mode) { - case TC_PORT_DP_ALT: - ln0 &= ~(MG_DP_MODE_CFG_DP_X1_MODE | MG_DP_MODE_CFG_DP_X2_MODE); - ln1 &= ~(MG_DP_MODE_CFG_DP_X1_MODE | MG_DP_MODE_CFG_DP_X2_MODE); + ln0 &= ~(MG_DP_MODE_CFG_DP_X1_MODE | MG_DP_MODE_CFG_DP_X1_MODE); + ln1 &= ~(MG_DP_MODE_CFG_DP_X1_MODE | MG_DP_MODE_CFG_DP_X2_MODE); - lane_mask = intel_tc_port_get_lane_mask(intel_dig_port); + /* DPPATC */ + pin_assignment = intel_tc_port_get_pin_assignment_mask(intel_dig_port); + width = intel_dig_port->dp.lane_count; - switch (lane_mask) { - case 0x1: - case 0x4: - break; - case 0x2: + switch (pin_assignment) { + case 0x0: + WARN_ON(intel_dig_port->tc_mode != TC_PORT_LEGACY); + if (width == 1) { + ln1 |= MG_DP_MODE_CFG_DP_X1_MODE; + } else { + ln0 |= MG_DP_MODE_CFG_DP_X2_MODE; + ln1 |= MG_DP_MODE_CFG_DP_X2_MODE; + } + break; + case 0x1: + if (width == 4) { + ln0 |= MG_DP_MODE_CFG_DP_X2_MODE; + ln1 |= MG_DP_MODE_CFG_DP_X2_MODE; + } + break; + case 0x2: + if (width == 2) { + ln0 |= MG_DP_MODE_CFG_DP_X2_MODE; + ln1 |= MG_DP_MODE_CFG_DP_X2_MODE; + } + break; + case 0x3: + case 0x5: + if (width == 1) { ln0 |= MG_DP_MODE_CFG_DP_X1_MODE; - break; - case 0x3: - ln0 |= MG_DP_MODE_CFG_DP_X1_MODE | - MG_DP_MODE_CFG_DP_X2_MODE; - break; - case 0x8: ln1 |= MG_DP_MODE_CFG_DP_X1_MODE; - break; - case 0xC: - ln1 |= MG_DP_MODE_CFG_DP_X1_MODE | - MG_DP_MODE_CFG_DP_X2_MODE; - break; - case 0xF: - ln0 |= MG_DP_MODE_CFG_DP_X1_MODE | - MG_DP_MODE_CFG_DP_X2_MODE; - ln1 |= MG_DP_MODE_CFG_DP_X1_MODE | - MG_DP_MODE_CFG_DP_X2_MODE; - break; - default: - MISSING_CASE(lane_mask); + } else { + ln0 |= MG_DP_MODE_CFG_DP_X2_MODE; + ln1 |= MG_DP_MODE_CFG_DP_X2_MODE; } break; - - case TC_PORT_LEGACY: - ln0 |= MG_DP_MODE_CFG_DP_X1_MODE | MG_DP_MODE_CFG_DP_X2_MODE; - ln1 |= MG_DP_MODE_CFG_DP_X1_MODE | MG_DP_MODE_CFG_DP_X2_MODE; + case 0x4: + case 0x6: + if (width == 1) { + ln0 |= MG_DP_MODE_CFG_DP_X1_MODE; + ln1 |= MG_DP_MODE_CFG_DP_X1_MODE; + } else { + ln0 |= MG_DP_MODE_CFG_DP_X2_MODE; + ln1 |= MG_DP_MODE_CFG_DP_X2_MODE; + } break; - default: - MISSING_CASE(intel_dig_port->tc_mode); - return; + MISSING_CASE(pin_assignment); } I915_WRITE(MG_DP_MODE(0, port), ln0); diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c index f923f9cbd33c..7773169b7331 100644 --- a/drivers/gpu/drm/i915/display/intel_tc.c +++ b/drivers/gpu/drm/i915/display/intel_tc.c @@ -67,6 +67,21 @@ u32 intel_tc_port_get_lane_mask(struct intel_digital_port *dig_port) return lane_mask >> DP_LANE_ASSIGNMENT_SHIFT(dig_port->tc_phy_fia_idx); } +u32 intel_tc_port_get_pin_assignment_mask(struct intel_digital_port *dig_port) +{ + struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev); + struct intel_uncore *uncore = &i915->uncore; + u32 pin_mask; + + pin_mask = intel_uncore_read(uncore, + PORT_TX_DFLEXPA1(dig_port->tc_phy_fia)); + + WARN_ON(pin_mask == 0xffffffff); + + return (pin_mask & DP_PIN_ASSIGNMENT_MASK(dig_port->tc_phy_fia_idx)) >> + DP_PIN_ASSIGNMENT_SHIFT(dig_port->tc_phy_fia_idx); +} + int intel_tc_port_fia_max_lane_count(struct intel_digital_port *dig_port) { struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev); diff --git a/drivers/gpu/drm/i915/display/intel_tc.h b/drivers/gpu/drm/i915/display/intel_tc.h index 783d75531435..463f1b3c836f 100644 --- a/drivers/gpu/drm/i915/display/intel_tc.h +++ b/drivers/gpu/drm/i915/display/intel_tc.h @@ -13,6 +13,7 @@ struct intel_digital_port; bool intel_tc_port_connected(struct intel_digital_port *dig_port); u32 intel_tc_port_get_lane_mask(struct intel_digital_port *dig_port); +u32 intel_tc_port_get_pin_assignment_mask(struct intel_digital_port *dig_port); int intel_tc_port_fia_max_lane_count(struct intel_digital_port *dig_port); void intel_tc_port_set_fia_lane_count(struct intel_digital_port *dig_port, int required_lanes); diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index e752de9470bd..bcf449c1d152 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -11857,6 +11857,11 @@ enum skl_power_gate { #define PORT_TX_DFLEXDPCSSS(fia) _MMIO_FIA((fia), 0x00894) #define DP_PHY_MODE_STATUS_NOT_SAFE(idx) (1 << (idx)) +#define PORT_TX_DFLEXPA1(fia) _MMIO_FIA((fia), 0x00880) +#define DP_PIN_ASSIGNMENT_SHIFT(idx) ((idx) * 4) +#define DP_PIN_ASSIGNMENT_MASK(idx) (0xf << ((idx) * 4)) +#define DP_PIN_ASSIGNMENT(idx, x) ((x) << ((idx) * 4)) + /* This register controls the Display State Buffer (DSB) engines. */ #define _DSBSL_INSTANCE_BASE 0x70B00 #define DSBSL_INSTANCE(pipe, id) (_DSBSL_INSTANCE_BASE + \