On Thu, 06 Jun 2019, Jani Nikula <jani.nik...@linux.intel.com> wrote:
> On Tue, 04 Jun 2019, Imre Deak <imre.d...@intel.com> wrote:
>> Move the TypeC port handling functions to a new file for clarity.
>>
>> While at it:
>> - s/icl_tc_port_connected()/intel_tc_port_connected()/
>>   icl_tc_phy_disconnect(), will be unexported later.
>>
>> - 
>> s/intel_dp_get_fia_supported_lane_count()/intel_tc_port_fia_max_lane_count()/
>>   It's used for HDMI legacy mode too.
>>
>> - Simplify function interfaces by passing only dig_port to them.
>>
>> No functional changes.
>
> Some nitpicks below.

Forgot,

Acked-by: Jani Nikula <jani.nik...@intel.com>

on the idea.

>
> BR,
> Jani.
>
>
>>
>> Cc: Animesh Manna <animesh.ma...@intel.com>
>> Cc: Paulo Zanoni <paulo.r.zan...@intel.com>
>> Cc: José Roberto de Souza <jose.so...@intel.com>
>> Signed-off-by: Imre Deak <imre.d...@intel.com>
>> ---
>>  drivers/gpu/drm/i915/Makefile             |   3 +-
>>  drivers/gpu/drm/i915/Makefile.header-test |   1 +
>>  drivers/gpu/drm/i915/intel_ddi.c          |   6 +-
>>  drivers/gpu/drm/i915/intel_dp.c           | 227 +--------------------
>>  drivers/gpu/drm/i915/intel_dp.h           |   2 -
>>  drivers/gpu/drm/i915/intel_tc.c           | 230 ++++++++++++++++++++++
>>  drivers/gpu/drm/i915/intel_tc.h           |  13 ++
>>  7 files changed, 252 insertions(+), 230 deletions(-)
>>  create mode 100644 drivers/gpu/drm/i915/intel_tc.c
>>  create mode 100644 drivers/gpu/drm/i915/intel_tc.h
>>
>> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
>> index c0a7b2994077..74c4d11d83eb 100644
>> --- a/drivers/gpu/drm/i915/Makefile
>> +++ b/drivers/gpu/drm/i915/Makefile
>> @@ -171,7 +171,8 @@ i915-y += intel_audio.o \
>>        intel_psr.o \
>>        intel_quirks.o \
>>        intel_sideband.o \
>> -      intel_sprite.o
>> +      intel_sprite.o \
>> +      intel_tc.o
>>  i915-$(CONFIG_ACPI)         += intel_acpi.o intel_opregion.o
>>  i915-$(CONFIG_DRM_FBDEV_EMULATION)  += intel_fbdev.o
>>  
>> diff --git a/drivers/gpu/drm/i915/Makefile.header-test 
>> b/drivers/gpu/drm/i915/Makefile.header-test
>> index 6ef3b647ac65..e80e8e45b09c 100644
>> --- a/drivers/gpu/drm/i915/Makefile.header-test
>> +++ b/drivers/gpu/drm/i915/Makefile.header-test
>> @@ -58,6 +58,7 @@ header_test := \
>>      intel_sdvo.h \
>>      intel_sideband.h \
>>      intel_sprite.h \
>> +    intel_tc.h \
>>      intel_tv.h \
>>      intel_uncore.h \
>>      intel_vdsc.h \
>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
>> b/drivers/gpu/drm/i915/intel_ddi.c
>> index 350eaf54f01f..5a1c98438375 100644
>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>> @@ -45,6 +45,7 @@
>>  #include "intel_lspcon.h"
>>  #include "intel_panel.h"
>>  #include "intel_psr.h"
>> +#include "intel_tc.h"
>>  #include "intel_vdsc.h"
>>  
>>  struct ddi_buf_trans {
>> @@ -3904,7 +3905,6 @@ static int intel_ddi_compute_config(struct 
>> intel_encoder *encoder,
>>  static void intel_ddi_encoder_suspend(struct intel_encoder *encoder)
>>  {
>>      struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
>> -    struct drm_i915_private *i915 = to_i915(encoder->base.dev);
>>  
>>      intel_dp_encoder_suspend(encoder);
>>  
>> @@ -3914,7 +3914,7 @@ static void intel_ddi_encoder_suspend(struct 
>> intel_encoder *encoder)
>>       * even if the sink has disappeared while being suspended.
>>       */
>>      if (dig_port->tc_legacy_port)
>> -            icl_tc_phy_disconnect(i915, dig_port);
>> +            icl_tc_phy_disconnect(dig_port);
>>  }
>>  
>>  static void intel_ddi_encoder_reset(struct drm_encoder *drm_encoder)
>> @@ -3936,7 +3936,7 @@ static void intel_ddi_encoder_destroy(struct 
>> drm_encoder *encoder)
>>      intel_dp_encoder_flush_work(encoder);
>>  
>>      if (intel_port_is_tc(i915, dig_port->base.port))
>> -            icl_tc_phy_disconnect(i915, dig_port);
>> +            icl_tc_phy_disconnect(dig_port);
>>  
>>      drm_encoder_cleanup(encoder);
>>      kfree(dig_port);
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c 
>> b/drivers/gpu/drm/i915/intel_dp.c
>> index 24b56b2a76c8..b69310bd9914 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -62,6 +62,7 @@
>>  #include "intel_panel.h"
>>  #include "intel_psr.h"
>>  #include "intel_sideband.h"
>> +#include "intel_tc.h"
>>  #include "intel_vdsc.h"
>>  
>>  #define DP_DPRX_ESI_LEN 14
>> @@ -211,46 +212,13 @@ static int intel_dp_max_common_rate(struct intel_dp 
>> *intel_dp)
>>      return intel_dp->common_rates[intel_dp->num_common_rates - 1];
>>  }
>>  
>> -static int intel_dp_get_fia_supported_lane_count(struct intel_dp *intel_dp)
>> -{
>> -    struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
>> -    struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
>> -    enum tc_port tc_port = intel_port_to_tc(dev_priv, dig_port->base.port);
>> -    intel_wakeref_t wakeref;
>> -    u32 lane_info;
>> -
>> -    if (tc_port == PORT_TC_NONE || dig_port->tc_type != TC_PORT_TYPEC)
>> -            return 4;
>> -
>> -    lane_info = 0;
>> -    with_intel_display_power(dev_priv, POWER_DOMAIN_DISPLAY_CORE, wakeref)
>> -            lane_info = (I915_READ(PORT_TX_DFLEXDPSP) &
>> -                         DP_LANE_ASSIGNMENT_MASK(tc_port)) >>
>> -                            DP_LANE_ASSIGNMENT_SHIFT(tc_port);
>> -
>> -    switch (lane_info) {
>> -    default:
>> -            MISSING_CASE(lane_info);
>> -    case 1:
>> -    case 2:
>> -    case 4:
>> -    case 8:
>> -            return 1;
>> -    case 3:
>> -    case 12:
>> -            return 2;
>> -    case 15:
>> -            return 4;
>> -    }
>> -}
>> -
>>  /* Theoretical max between source and sink */
>>  static int intel_dp_max_common_lane_count(struct intel_dp *intel_dp)
>>  {
>>      struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>>      int source_max = intel_dig_port->max_lanes;
>>      int sink_max = drm_dp_max_lane_count(intel_dp->dpcd);
>> -    int fia_max = intel_dp_get_fia_supported_lane_count(intel_dp);
>> +    int fia_max = intel_tc_port_fia_max_lane_count(intel_dig_port);
>>  
>>      return min3(source_max, sink_max, fia_max);
>>  }
>> @@ -5231,195 +5199,6 @@ static bool icl_combo_port_connected(struct 
>> drm_i915_private *dev_priv,
>>      return I915_READ(SDEISR) & SDE_DDI_HOTPLUG_ICP(port);
>>  }
>>  
>> -static const char *tc_type_name(enum tc_port_type type)
>> -{
>> -    static const char * const names[] = {
>> -            [TC_PORT_UNKNOWN] = "unknown",
>> -            [TC_PORT_LEGACY] = "legacy",
>> -            [TC_PORT_TYPEC] = "typec",
>> -            [TC_PORT_TBT] = "tbt",
>> -    };
>> -
>> -    if (WARN_ON(type >= ARRAY_SIZE(names)))
>> -            type = TC_PORT_UNKNOWN;
>> -
>> -    return names[type];
>> -}
>> -
>> -static void icl_update_tc_port_type(struct drm_i915_private *dev_priv,
>> -                                struct intel_digital_port *intel_dig_port,
>> -                                bool is_legacy, bool is_typec, bool is_tbt)
>> -{
>> -    enum port port = intel_dig_port->base.port;
>> -    enum tc_port_type old_type = intel_dig_port->tc_type;
>> -
>> -    WARN_ON(is_legacy + is_typec + is_tbt != 1);
>> -
>> -    if (is_legacy)
>> -            intel_dig_port->tc_type = TC_PORT_LEGACY;
>> -    else if (is_typec)
>> -            intel_dig_port->tc_type = TC_PORT_TYPEC;
>> -    else if (is_tbt)
>> -            intel_dig_port->tc_type = TC_PORT_TBT;
>> -    else
>> -            return;
>> -
>> -    /* Types are not supposed to be changed at runtime. */
>> -    WARN_ON(old_type != TC_PORT_UNKNOWN &&
>> -            old_type != intel_dig_port->tc_type);
>> -
>> -    if (old_type != intel_dig_port->tc_type)
>> -            DRM_DEBUG_KMS("Port %c has TC type %s\n", port_name(port),
>> -                          tc_type_name(intel_dig_port->tc_type));
>> -}
>> -
>> -/*
>> - * This function implements the first part of the Connect Flow described by 
>> our
>> - * specification, Gen11 TypeC Programming chapter. The rest of the flow 
>> (reading
>> - * lanes, EDID, etc) is done as needed in the typical places.
>> - *
>> - * Unlike the other ports, type-C ports are not available to use as soon as 
>> we
>> - * get a hotplug. The type-C PHYs can be shared between multiple 
>> controllers:
>> - * display, USB, etc. As a result, handshaking through FIA is required 
>> around
>> - * connect and disconnect to cleanly transfer ownership with the controller 
>> and
>> - * set the type-C power state.
>> - *
>> - * We could opt to only do the connect flow when we actually try to use the 
>> AUX
>> - * channels or do a modeset, then immediately run the disconnect flow after
>> - * usage, but there are some implications on this for a dynamic environment:
>> - * things may go away or change behind our backs. So for now our driver is
>> - * always trying to acquire ownership of the controller as soon as it gets 
>> an
>> - * interrupt (or polls state and sees a port is connected) and only gives it
>> - * back when it sees a disconnect. Implementation of a more fine-grained 
>> model
>> - * will require a lot of coordination with user space and thorough testing 
>> for
>> - * the extra possible cases.
>> - */
>> -static bool icl_tc_phy_connect(struct drm_i915_private *dev_priv,
>> -                           struct intel_digital_port *dig_port)
>> -{
>> -    enum tc_port tc_port = intel_port_to_tc(dev_priv, dig_port->base.port);
>> -    u32 val;
>> -
>> -    if (dig_port->tc_type != TC_PORT_LEGACY &&
>> -        dig_port->tc_type != TC_PORT_TYPEC)
>> -            return true;
>> -
>> -    val = I915_READ(PORT_TX_DFLEXDPPMS);
>> -    if (!(val & DP_PHY_MODE_STATUS_COMPLETED(tc_port))) {
>> -            DRM_DEBUG_KMS("DP PHY for TC port %d not ready\n", tc_port);
>> -            WARN_ON(dig_port->tc_legacy_port);
>> -            return false;
>> -    }
>> -
>> -    /*
>> -     * This function may be called many times in a row without an HPD event
>> -     * in between, so try to avoid the write when we can.
>> -     */
>> -    val = I915_READ(PORT_TX_DFLEXDPCSSS);
>> -    if (!(val & DP_PHY_MODE_STATUS_NOT_SAFE(tc_port))) {
>> -            val |= DP_PHY_MODE_STATUS_NOT_SAFE(tc_port);
>> -            I915_WRITE(PORT_TX_DFLEXDPCSSS, val);
>> -    }
>> -
>> -    /*
>> -     * Now we have to re-check the live state, in case the port recently
>> -     * became disconnected. Not necessary for legacy mode.
>> -     */
>> -    if (dig_port->tc_type == TC_PORT_TYPEC &&
>> -        !(I915_READ(PORT_TX_DFLEXDPSP) & TC_LIVE_STATE_TC(tc_port))) {
>> -            DRM_DEBUG_KMS("TC PHY %d sudden disconnect.\n", tc_port);
>> -            icl_tc_phy_disconnect(dev_priv, dig_port);
>> -            return false;
>> -    }
>> -
>> -    return true;
>> -}
>> -
>> -/*
>> - * See the comment at the connect function. This implements the Disconnect
>> - * Flow.
>> - */
>> -void icl_tc_phy_disconnect(struct drm_i915_private *dev_priv,
>> -                       struct intel_digital_port *dig_port)
>> -{
>> -    enum tc_port tc_port = intel_port_to_tc(dev_priv, dig_port->base.port);
>> -
>> -    if (dig_port->tc_type == TC_PORT_UNKNOWN)
>> -            return;
>> -
>> -    /*
>> -     * TBT disconnection flow is read the live status, what was done in
>> -     * caller.
>> -     */
>> -    if (dig_port->tc_type == TC_PORT_TYPEC ||
>> -        dig_port->tc_type == TC_PORT_LEGACY) {
>> -            u32 val;
>> -
>> -            val = I915_READ(PORT_TX_DFLEXDPCSSS);
>> -            val &= ~DP_PHY_MODE_STATUS_NOT_SAFE(tc_port);
>> -            I915_WRITE(PORT_TX_DFLEXDPCSSS, val);
>> -    }
>> -
>> -    DRM_DEBUG_KMS("Port %c TC type %s disconnected\n",
>> -                  port_name(dig_port->base.port),
>> -                  tc_type_name(dig_port->tc_type));
>> -
>> -    dig_port->tc_type = TC_PORT_UNKNOWN;
>> -}
>> -
>> -/*
>> - * The type-C ports are different because even when they are connected, 
>> they may
>> - * not be available/usable by the graphics driver: see the comment on
>> - * icl_tc_phy_connect(). So in our driver instead of adding the additional
>> - * concept of "usable" and make everything check for "connected and usable" 
>> we
>> - * define a port as "connected" when it is not only connected, but also 
>> when it
>> - * is usable by the rest of the driver. That maintains the old assumption 
>> that
>> - * connected ports are usable, and avoids exposing to the users objects they
>> - * can't really use.
>> - */
>> -static bool icl_tc_port_connected(struct drm_i915_private *dev_priv,
>> -                              struct intel_digital_port *intel_dig_port)
>> -{
>> -    enum port port = intel_dig_port->base.port;
>> -    enum tc_port tc_port = intel_port_to_tc(dev_priv, port);
>> -    bool is_legacy, is_typec, is_tbt;
>> -    u32 dpsp;
>> -
>> -    /*
>> -     * Complain if we got a legacy port HPD, but VBT didn't mark the port as
>> -     * legacy. Treat the port as legacy from now on.
>> -     */
>> -    if (!intel_dig_port->tc_legacy_port &&
>> -        I915_READ(SDEISR) & SDE_TC_HOTPLUG_ICP(tc_port)) {
>> -            DRM_ERROR("VBT incorrectly claims port %c is not TypeC 
>> legacy\n",
>> -                      port_name(port));
>> -            intel_dig_port->tc_legacy_port = true;
>> -    }
>> -    is_legacy = intel_dig_port->tc_legacy_port;
>> -
>> -    /*
>> -     * The spec says we shouldn't be using the ISR bits for detecting
>> -     * between TC and TBT. We should use DFLEXDPSP.
>> -     */
>> -    dpsp = I915_READ(PORT_TX_DFLEXDPSP);
>> -    is_typec = dpsp & TC_LIVE_STATE_TC(tc_port);
>> -    is_tbt = dpsp & TC_LIVE_STATE_TBT(tc_port);
>> -
>> -    if (!is_legacy && !is_typec && !is_tbt) {
>> -            icl_tc_phy_disconnect(dev_priv, intel_dig_port);
>> -
>> -            return false;
>> -    }
>> -
>> -    icl_update_tc_port_type(dev_priv, intel_dig_port, is_legacy, is_typec,
>> -                            is_tbt);
>> -
>> -    if (!icl_tc_phy_connect(dev_priv, intel_dig_port))
>> -            return false;
>> -
>> -    return true;
>> -}
>> -
>>  static bool icl_digital_port_connected(struct intel_encoder *encoder)
>>  {
>>      struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>> @@ -5428,7 +5207,7 @@ static bool icl_digital_port_connected(struct 
>> intel_encoder *encoder)
>>      if (intel_port_is_combophy(dev_priv, encoder->port))
>>              return icl_combo_port_connected(dev_priv, dig_port);
>>      else if (intel_port_is_tc(dev_priv, encoder->port))
>> -            return icl_tc_port_connected(dev_priv, dig_port);
>> +            return intel_tc_port_connected(dig_port);
>>      else
>>              MISSING_CASE(encoder->hpd_pin);
>>  
>> diff --git a/drivers/gpu/drm/i915/intel_dp.h 
>> b/drivers/gpu/drm/i915/intel_dp.h
>> index da70b1a41c83..657bbb1f5ed0 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.h
>> +++ b/drivers/gpu/drm/i915/intel_dp.h
>> @@ -112,8 +112,6 @@ bool intel_dp_get_colorimetry_status(struct intel_dp 
>> *intel_dp);
>>  int intel_dp_link_required(int pixel_clock, int bpp);
>>  int intel_dp_max_data_rate(int max_link_clock, int max_lanes);
>>  bool intel_digital_port_connected(struct intel_encoder *encoder);
>> -void icl_tc_phy_disconnect(struct drm_i915_private *dev_priv,
>> -                       struct intel_digital_port *dig_port);
>>  
>>  static inline unsigned int intel_dp_unused_lane_mask(int lane_count)
>>  {
>> diff --git a/drivers/gpu/drm/i915/intel_tc.c 
>> b/drivers/gpu/drm/i915/intel_tc.c
>> new file mode 100644
>> index 000000000000..7a1b5870945f
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/intel_tc.c
>> @@ -0,0 +1,230 @@
>> +// SPDX-License-Identifier: MIT
>> +/*
>> + * Copyright © 2019 Intel Corporation
>> + */
>
> From the Nitpicks Department, please add a blank line here.
>
>> +#include "intel_display.h"
>> +#include "i915_drv.h"
>> +#include "intel_tc.h"
>
> And sort the includes.
>
>> +
>> +static const char *tc_type_name(enum tc_port_type type)
>> +{
>> +    static const char * const names[] = {
>> +            [TC_PORT_UNKNOWN] = "unknown",
>> +            [TC_PORT_LEGACY] = "legacy",
>> +            [TC_PORT_TYPEC] = "typec",
>> +            [TC_PORT_TBT] = "tbt",
>> +    };
>> +
>> +    if (WARN_ON(type >= ARRAY_SIZE(names)))
>> +            type = TC_PORT_UNKNOWN;
>> +
>> +    return names[type];
>> +}
>> +
>> +int intel_tc_port_fia_max_lane_count(struct intel_digital_port *dig_port)
>> +{
>> +    struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
>> +    enum tc_port tc_port = intel_port_to_tc(dev_priv, dig_port->base.port);
>> +    intel_wakeref_t wakeref;
>> +    u32 lane_info;
>> +
>> +    if (tc_port == PORT_TC_NONE || dig_port->tc_type != TC_PORT_TYPEC)
>> +            return 4;
>> +
>> +    lane_info = 0;
>> +    with_intel_display_power(dev_priv, POWER_DOMAIN_DISPLAY_CORE, wakeref)
>> +            lane_info = (I915_READ(PORT_TX_DFLEXDPSP) &
>> +                         DP_LANE_ASSIGNMENT_MASK(tc_port)) >>
>> +                            DP_LANE_ASSIGNMENT_SHIFT(tc_port);
>> +
>> +    switch (lane_info) {
>> +    default:
>> +            MISSING_CASE(lane_info);
>> +    case 1:
>> +    case 2:
>> +    case 4:
>> +    case 8:
>> +            return 1;
>> +    case 3:
>> +    case 12:
>> +            return 2;
>> +    case 15:
>> +            return 4;
>> +    }
>> +}
>> +
>> +/*
>> + * This function implements the first part of the Connect Flow described by 
>> our
>> + * specification, Gen11 TypeC Programming chapter. The rest of the flow 
>> (reading
>> + * lanes, EDID, etc) is done as needed in the typical places.
>> + *
>> + * Unlike the other ports, type-C ports are not available to use as soon as 
>> we
>> + * get a hotplug. The type-C PHYs can be shared between multiple 
>> controllers:
>> + * display, USB, etc. As a result, handshaking through FIA is required 
>> around
>> + * connect and disconnect to cleanly transfer ownership with the controller 
>> and
>> + * set the type-C power state.
>> + *
>> + * We could opt to only do the connect flow when we actually try to use the 
>> AUX
>> + * channels or do a modeset, then immediately run the disconnect flow after
>> + * usage, but there are some implications on this for a dynamic environment:
>> + * things may go away or change behind our backs. So for now our driver is
>> + * always trying to acquire ownership of the controller as soon as it gets 
>> an
>> + * interrupt (or polls state and sees a port is connected) and only gives it
>> + * back when it sees a disconnect. Implementation of a more fine-grained 
>> model
>> + * will require a lot of coordination with user space and thorough testing 
>> for
>> + * the extra possible cases.
>> + */
>> +static bool icl_tc_phy_connect(struct intel_digital_port *dig_port)
>> +{
>> +    struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
>> +    enum tc_port tc_port = intel_port_to_tc(dev_priv, dig_port->base.port);
>> +    u32 val;
>> +
>> +    if (dig_port->tc_type != TC_PORT_LEGACY &&
>> +        dig_port->tc_type != TC_PORT_TYPEC)
>> +            return true;
>> +
>> +    val = I915_READ(PORT_TX_DFLEXDPPMS);
>> +    if (!(val & DP_PHY_MODE_STATUS_COMPLETED(tc_port))) {
>> +            DRM_DEBUG_KMS("DP PHY for TC port %d not ready\n", tc_port);
>> +            WARN_ON(dig_port->tc_legacy_port);
>> +            return false;
>> +    }
>> +
>> +    /*
>> +     * This function may be called many times in a row without an HPD event
>> +     * in between, so try to avoid the write when we can.
>> +     */
>> +    val = I915_READ(PORT_TX_DFLEXDPCSSS);
>> +    if (!(val & DP_PHY_MODE_STATUS_NOT_SAFE(tc_port))) {
>> +            val |= DP_PHY_MODE_STATUS_NOT_SAFE(tc_port);
>> +            I915_WRITE(PORT_TX_DFLEXDPCSSS, val);
>> +    }
>> +
>> +    /*
>> +     * Now we have to re-check the live state, in case the port recently
>> +     * became disconnected. Not necessary for legacy mode.
>> +     */
>> +    if (dig_port->tc_type == TC_PORT_TYPEC &&
>> +        !(I915_READ(PORT_TX_DFLEXDPSP) & TC_LIVE_STATE_TC(tc_port))) {
>> +            DRM_DEBUG_KMS("TC PHY %d sudden disconnect.\n", tc_port);
>> +            icl_tc_phy_disconnect(dig_port);
>> +            return false;
>> +    }
>> +
>> +    return true;
>> +}
>> +
>> +/*
>> + * See the comment at the connect function. This implements the Disconnect
>> + * Flow.
>> + */
>> +void icl_tc_phy_disconnect(struct intel_digital_port *dig_port)
>> +{
>> +    struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
>> +    enum tc_port tc_port = intel_port_to_tc(dev_priv, dig_port->base.port);
>> +
>> +    if (dig_port->tc_type == TC_PORT_UNKNOWN)
>> +            return;
>> +
>> +    /*
>> +     * TBT disconnection flow is read the live status, what was done in
>> +     * caller.
>> +     */
>> +    if (dig_port->tc_type == TC_PORT_TYPEC ||
>> +        dig_port->tc_type == TC_PORT_LEGACY) {
>> +            u32 val;
>> +
>> +            val = I915_READ(PORT_TX_DFLEXDPCSSS);
>> +            val &= ~DP_PHY_MODE_STATUS_NOT_SAFE(tc_port);
>> +            I915_WRITE(PORT_TX_DFLEXDPCSSS, val);
>> +    }
>> +
>> +    DRM_DEBUG_KMS("Port %c TC type %s disconnected\n",
>> +                  port_name(dig_port->base.port),
>> +                  tc_type_name(dig_port->tc_type));
>> +
>> +    dig_port->tc_type = TC_PORT_UNKNOWN;
>> +}
>> +
>> +static void icl_update_tc_port_type(struct drm_i915_private *dev_priv,
>> +                                struct intel_digital_port *intel_dig_port,
>> +                                bool is_legacy, bool is_typec, bool is_tbt)
>> +{
>> +    enum port port = intel_dig_port->base.port;
>> +    enum tc_port_type old_type = intel_dig_port->tc_type;
>> +
>> +    WARN_ON(is_legacy + is_typec + is_tbt != 1);
>> +
>> +    if (is_legacy)
>> +            intel_dig_port->tc_type = TC_PORT_LEGACY;
>> +    else if (is_typec)
>> +            intel_dig_port->tc_type = TC_PORT_TYPEC;
>> +    else if (is_tbt)
>> +            intel_dig_port->tc_type = TC_PORT_TBT;
>> +    else
>> +            return;
>> +
>> +    /* Types are not supposed to be changed at runtime. */
>> +    WARN_ON(old_type != TC_PORT_UNKNOWN &&
>> +            old_type != intel_dig_port->tc_type);
>> +
>> +    if (old_type != intel_dig_port->tc_type)
>> +            DRM_DEBUG_KMS("Port %c has TC type %s\n", port_name(port),
>> +                          tc_type_name(intel_dig_port->tc_type));
>> +}
>> +
>> +
>> +/*
>> + * The type-C ports are different because even when they are connected, 
>> they may
>> + * not be available/usable by the graphics driver: see the comment on
>> + * icl_tc_phy_connect(). So in our driver instead of adding the additional
>> + * concept of "usable" and make everything check for "connected and usable" 
>> we
>> + * define a port as "connected" when it is not only connected, but also 
>> when it
>> + * is usable by the rest of the driver. That maintains the old assumption 
>> that
>> + * connected ports are usable, and avoids exposing to the users objects they
>> + * can't really use.
>> + */
>> +bool intel_tc_port_connected(struct intel_digital_port *dig_port)
>> +{
>> +    struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
>> +    enum port port = dig_port->base.port;
>> +    enum tc_port tc_port = intel_port_to_tc(dev_priv, port);
>> +    bool is_legacy, is_typec, is_tbt;
>> +    u32 dpsp;
>> +
>> +    /*
>> +     * Complain if we got a legacy port HPD, but VBT didn't mark the port as
>> +     * legacy. Treat the port as legacy from now on.
>> +     */
>> +    if (!dig_port->tc_legacy_port &&
>> +        I915_READ(SDEISR) & SDE_TC_HOTPLUG_ICP(tc_port)) {
>> +            DRM_ERROR("VBT incorrectly claims port %c is not TypeC 
>> legacy\n",
>> +                      port_name(port));
>> +            dig_port->tc_legacy_port = true;
>> +    }
>> +    is_legacy = dig_port->tc_legacy_port;
>> +
>> +    /*
>> +     * The spec says we shouldn't be using the ISR bits for detecting
>> +     * between TC and TBT. We should use DFLEXDPSP.
>> +     */
>> +    dpsp = I915_READ(PORT_TX_DFLEXDPSP);
>> +    is_typec = dpsp & TC_LIVE_STATE_TC(tc_port);
>> +    is_tbt = dpsp & TC_LIVE_STATE_TBT(tc_port);
>> +
>> +    if (!is_legacy && !is_typec && !is_tbt) {
>> +            icl_tc_phy_disconnect(dig_port);
>> +
>> +            return false;
>> +    }
>> +
>> +    icl_update_tc_port_type(dev_priv, dig_port, is_legacy, is_typec,
>> +                            is_tbt);
>> +
>> +    if (!icl_tc_phy_connect(dig_port))
>> +            return false;
>> +
>> +    return true;
>> +}
>> +
>> diff --git a/drivers/gpu/drm/i915/intel_tc.h 
>> b/drivers/gpu/drm/i915/intel_tc.h
>> new file mode 100644
>> index 000000000000..94c62ac4a162
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/intel_tc.h
>> @@ -0,0 +1,13 @@
>
> SPDX header here please.
>
>> +#ifndef __INTEL_TC_H__
>> +#define __INTEL_TC_H__
>> +
>> +#include <linux/types.h>
>> +
>> +struct intel_digital_port;
>> +
>> +void icl_tc_phy_disconnect(struct intel_digital_port *dig_port);
>
> I'd like to see this named intel_tc_* in the future.
>
> BR,
> Jani.
>
>
>> +
>> +bool intel_tc_port_connected(struct intel_digital_port *dig_port);
>> +int intel_tc_port_fia_max_lane_count(struct intel_digital_port *dig_port);
>> +
>> +#endif /* __INTEL_TC_H__ */

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to