[Intel-gfx] [PATCH v4 1/4] drm/i915/tc: Update DP_MODE programming
Imre Deak
imre.deak at intel.com
Thu Sep 26 19:41:00 UTC 2019
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 at 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 at intel.com>
> > > Cc: Lucas De Marchi <lucas.demarchi at intel.com>
> > > Signed-off-by: Clinton A Taylor <clinton.a.taylor at intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza at 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 at 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
> > >
More information about the Intel-gfx
mailing list