On Wed, Sep 25, 2019 at 04:45:06PM -0700, José Roberto de Souza wrote:
> From: Clinton A Taylor <[email protected]>
> 
> 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 <[email protected]>
> Cc: Lucas De Marchi <[email protected]>
> Signed-off-by: Clinton A Taylor <[email protected]>
> Signed-off-by: José Roberto de Souza <[email protected]>
> ---
>  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 <[email protected]>

>               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
> 
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to