On Wed, 2020-07-01 at 00:55 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <
> ville.syrj...@linux.intel.com
> >
> 
> Make a clean split between hpd pins for DDI vs. TC. This matches
> how the actual hardware is split.
> 
> And with this we move the DDI/PHY->HPD pin mapping into the encoder
> init instead of having to remap yet again in the interrupt code.
> 
> Signed-off-by: Ville Syrjälä <
> ville.syrj...@linux.intel.com
> >
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c     |  65 +++++++++-
>  drivers/gpu/drm/i915/display/intel_hotplug.c |  25 +---
>  drivers/gpu/drm/i915/i915_drv.h              |  17 +--
>  drivers/gpu/drm/i915/i915_irq.c              | 121 +++++--------------
>  4 files changed, 102 insertions(+), 126 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c 
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index d024491738b3..a2c9815c5abc 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -4847,6 +4847,57 @@ intel_ddi_max_lanes(struct intel_digital_port 
> *intel_dport)
>       return max_lanes;
>  }
>  
> +static enum hpd_pin tgl_hpd_pin(struct drm_i915_private *dev_priv,
> +                             enum port port)
> +{
> +     if (port >= PORT_D)
> +             return HPD_PORT_TC1 + port - PORT_D;
> +     else
> +             return HPD_PORT_A + port - PORT_A;
> +}
> +
> +static enum hpd_pin rkl_hpd_pin(struct drm_i915_private *dev_priv,
> +                             enum port port)
> +{
> +     if (HAS_PCH_TGP(dev_priv))
> +             return tgl_hpd_pin(dev_priv, port);
> +
> +     if (port >= PORT_D)
> +             return HPD_PORT_C + port - PORT_D;

The above looks wrong, for it would match with only the return bellow.

> +     else
> +             return HPD_PORT_A + port - PORT_A;
> +}
> +
> +static enum hpd_pin icl_hpd_pin(struct drm_i915_private *dev_priv,
> +                             enum port port)
> +{
> +     if (port >= PORT_C)
> +             return HPD_PORT_TC1 + port - PORT_C;
> +     else
> +             return HPD_PORT_A + port - PORT_A;
> +}
> +
> +static enum hpd_pin ehl_hpd_pin(struct drm_i915_private *dev_priv,
> +                             enum port port)
> +{
> +     if (port == PORT_D)
> +             return HPD_PORT_A;
> +
> +     if (HAS_PCH_MCC(dev_priv))
> +             return icl_hpd_pin(dev_priv, port);

Maybe call tgl_hpd_pin() for HAS_PCH_MCC()? The code bellow will match but just 
for consistency.

Other than the RKL comment it looks good to me.

> +
> +     return HPD_PORT_A + port - PORT_A;
> +}
> +
> +static enum hpd_pin cnl_hpd_pin(struct drm_i915_private *dev_priv,
> +                             enum port port)
> +{
> +     if (port == PORT_F)
> +             return HPD_PORT_E;
> +
> +     return HPD_PORT_A + port - PORT_A;
> +}
> +
>  void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>  {
>       struct intel_digital_port *intel_dig_port;
> @@ -4907,7 +4958,19 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, 
> enum port port)
>       encoder->port = port;
>       encoder->cloneable = 0;
>       encoder->pipe_mask = ~0;
> -     encoder->hpd_pin = intel_hpd_pin_default(dev_priv, port);
> +
> +     if (IS_ROCKETLAKE(dev_priv))
> +             encoder->hpd_pin = rkl_hpd_pin(dev_priv, port);
> +     else if (INTEL_GEN(dev_priv) >= 12)
> +             encoder->hpd_pin = tgl_hpd_pin(dev_priv, port);
> +     else if (IS_ELKHARTLAKE(dev_priv))
> +             encoder->hpd_pin = ehl_hpd_pin(dev_priv, port);
> +     else if (IS_GEN(dev_priv, 11))
> +             encoder->hpd_pin = icl_hpd_pin(dev_priv, port);
> +     else if (IS_GEN(dev_priv, 10))
> +             encoder->hpd_pin = cnl_hpd_pin(dev_priv, port);
> +     else
> +             encoder->hpd_pin = intel_hpd_pin_default(dev_priv, port);
>  
>       if (INTEL_GEN(dev_priv) >= 11)
>               intel_dig_port->saved_port_bits = intel_de_read(dev_priv,
> diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c 
> b/drivers/gpu/drm/i915/display/intel_hotplug.c
> index 80bcfff032e9..8a8e77314a4e 100644
> --- a/drivers/gpu/drm/i915/display/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
> @@ -81,33 +81,12 @@
>   *
>   * It is only valid and used by digital port encoder.
>   *
> - * Return pin that is associatade with @port and HDP_NONE if no pin is
> - * hard associated with that @port.
> + * Return pin that is associatade with @port.
>   */
>  enum hpd_pin intel_hpd_pin_default(struct drm_i915_private *dev_priv,
>                                  enum port port)
>  {
> -     enum phy phy = intel_port_to_phy(dev_priv, port);
> -
> -     /*
> -      * RKL + TGP PCH is a special case; we effectively choose the hpd_pin
> -      * based on the DDI rather than the PHY (i.e., the last two outputs
> -      * shold be HPD_PORT_{D,E} rather than {C,D}.  Note that this differs
> -      * from the behavior of both TGL+TGP and RKL+CMP.
> -      */
> -     if (IS_ROCKETLAKE(dev_priv) && HAS_PCH_TGP(dev_priv))
> -             return HPD_PORT_A + port - PORT_A;
> -
> -     switch (phy) {
> -     case PHY_F:
> -             return IS_CNL_WITH_PORT_F(dev_priv) ? HPD_PORT_E : HPD_PORT_F;
> -     case PHY_A ... PHY_E:
> -     case PHY_G ... PHY_I:
> -             return HPD_PORT_A + phy - PHY_A;
> -     default:
> -             MISSING_CASE(phy);
> -             return HPD_NONE;
> -     }
> +     return HPD_PORT_A + port - PORT_A;
>  }
>  
>  #define HPD_STORM_DETECT_PERIOD              1000
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 6e9072ab30a1..dcd35cd97f01 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -113,13 +113,6 @@
>  
>  struct drm_i915_gem_object;
>  
> -/*
> - * The code assumes that the hpd_pins below have consecutive values and
> - * starting with HPD_PORT_A, the HPD pin associated with any port can be
> - * retrieved by adding the corresponding port (or phy) enum value to
> - * HPD_PORT_A in most cases. For example:
> - * HPD_PORT_C = HPD_PORT_A + PHY_C - PHY_A
> - */
>  enum hpd_pin {
>       HPD_NONE = 0,
>       HPD_TV = HPD_NONE,     /* TV is known to be unreliable */
> @@ -131,10 +124,12 @@ enum hpd_pin {
>       HPD_PORT_C,
>       HPD_PORT_D,
>       HPD_PORT_E,
> -     HPD_PORT_F,
> -     HPD_PORT_G,
> -     HPD_PORT_H,
> -     HPD_PORT_I,
> +     HPD_PORT_TC1,
> +     HPD_PORT_TC2,
> +     HPD_PORT_TC3,
> +     HPD_PORT_TC4,
> +     HPD_PORT_TC5,
> +     HPD_PORT_TC6,
>  
>       HPD_NUM_PINS
>  };
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 92d74448ee03..95ab4432a87d 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -131,40 +131,24 @@ static const u32 hpd_bxt[HPD_NUM_PINS] = {
>  };
>  
>  static const u32 hpd_gen11[HPD_NUM_PINS] = {
> -     [HPD_PORT_C] = GEN11_TC_HOTPLUG(PORT_TC1) | GEN11_TBT_HOTPLUG(PORT_TC1),
> -     [HPD_PORT_D] = GEN11_TC_HOTPLUG(PORT_TC2) | GEN11_TBT_HOTPLUG(PORT_TC2),
> -     [HPD_PORT_E] = GEN11_TC_HOTPLUG(PORT_TC3) | GEN11_TBT_HOTPLUG(PORT_TC3),
> -     [HPD_PORT_F] = GEN11_TC_HOTPLUG(PORT_TC4) | GEN11_TBT_HOTPLUG(PORT_TC4),
> -};
> -
> -static const u32 hpd_gen12[HPD_NUM_PINS] = {
> -     [HPD_PORT_D] = GEN11_TC_HOTPLUG(PORT_TC1) | GEN11_TBT_HOTPLUG(PORT_TC1),
> -     [HPD_PORT_E] = GEN11_TC_HOTPLUG(PORT_TC2) | GEN11_TBT_HOTPLUG(PORT_TC2),
> -     [HPD_PORT_F] = GEN11_TC_HOTPLUG(PORT_TC3) | GEN11_TBT_HOTPLUG(PORT_TC3),
> -     [HPD_PORT_G] = GEN11_TC_HOTPLUG(PORT_TC4) | GEN11_TBT_HOTPLUG(PORT_TC4),
> -     [HPD_PORT_H] = GEN11_TC_HOTPLUG(PORT_TC5) | GEN11_TBT_HOTPLUG(PORT_TC5),
> -     [HPD_PORT_I] = GEN11_TC_HOTPLUG(PORT_TC6) | GEN11_TBT_HOTPLUG(PORT_TC6),
> +     [HPD_PORT_TC1] = GEN11_TC_HOTPLUG(PORT_TC1) | 
> GEN11_TBT_HOTPLUG(PORT_TC1),
> +     [HPD_PORT_TC2] = GEN11_TC_HOTPLUG(PORT_TC2) | 
> GEN11_TBT_HOTPLUG(PORT_TC2),
> +     [HPD_PORT_TC3] = GEN11_TC_HOTPLUG(PORT_TC3) | 
> GEN11_TBT_HOTPLUG(PORT_TC3),
> +     [HPD_PORT_TC4] = GEN11_TC_HOTPLUG(PORT_TC4) | 
> GEN11_TBT_HOTPLUG(PORT_TC4),
> +     [HPD_PORT_TC5] = GEN11_TC_HOTPLUG(PORT_TC5) | 
> GEN11_TBT_HOTPLUG(PORT_TC5),
> +     [HPD_PORT_TC6] = GEN11_TC_HOTPLUG(PORT_TC6) | 
> GEN11_TBT_HOTPLUG(PORT_TC6),
>  };
>  
>  static const u32 hpd_icp[HPD_NUM_PINS] = {
> -     [HPD_PORT_A] = SDE_DDI_HOTPLUG_ICP(PORT_A),
> -     [HPD_PORT_B] = SDE_DDI_HOTPLUG_ICP(PORT_B),
> -     [HPD_PORT_C] = SDE_TC_HOTPLUG_ICP(PORT_TC1),
> -     [HPD_PORT_D] = SDE_TC_HOTPLUG_ICP(PORT_TC2),
> -     [HPD_PORT_E] = SDE_TC_HOTPLUG_ICP(PORT_TC3),
> -     [HPD_PORT_F] = SDE_TC_HOTPLUG_ICP(PORT_TC4),
> -};
> -
> -static const u32 hpd_tgp[HPD_NUM_PINS] = {
>       [HPD_PORT_A] = SDE_DDI_HOTPLUG_ICP(PORT_A),
>       [HPD_PORT_B] = SDE_DDI_HOTPLUG_ICP(PORT_B),
>       [HPD_PORT_C] = SDE_DDI_HOTPLUG_ICP(PORT_C),
> -     [HPD_PORT_D] = SDE_TC_HOTPLUG_ICP(PORT_TC1),
> -     [HPD_PORT_E] = SDE_TC_HOTPLUG_ICP(PORT_TC2),
> -     [HPD_PORT_F] = SDE_TC_HOTPLUG_ICP(PORT_TC3),
> -     [HPD_PORT_G] = SDE_TC_HOTPLUG_ICP(PORT_TC4),
> -     [HPD_PORT_H] = SDE_TC_HOTPLUG_ICP(PORT_TC5),
> -     [HPD_PORT_I] = SDE_TC_HOTPLUG_ICP(PORT_TC6),
> +     [HPD_PORT_TC1] = SDE_TC_HOTPLUG_ICP(PORT_TC1),
> +     [HPD_PORT_TC2] = SDE_TC_HOTPLUG_ICP(PORT_TC2),
> +     [HPD_PORT_TC3] = SDE_TC_HOTPLUG_ICP(PORT_TC3),
> +     [HPD_PORT_TC4] = SDE_TC_HOTPLUG_ICP(PORT_TC4),
> +     [HPD_PORT_TC5] = SDE_TC_HOTPLUG_ICP(PORT_TC5),
> +     [HPD_PORT_TC6] = SDE_TC_HOTPLUG_ICP(PORT_TC6),
>  };
>  
>  static void intel_hpd_init_pins(struct drm_i915_private *dev_priv)
> @@ -180,9 +164,7 @@ static void intel_hpd_init_pins(struct drm_i915_private 
> *dev_priv)
>               return;
>       }
>  
> -     if (INTEL_GEN(dev_priv) >= 12)
> -             hpd->hpd = hpd_gen12;
> -     else if (INTEL_GEN(dev_priv) >= 11)
> +     if (INTEL_GEN(dev_priv) >= 11)
>               hpd->hpd = hpd_gen11;
>       else if (IS_GEN9_LP(dev_priv))
>               hpd->hpd = hpd_bxt;
> @@ -196,9 +178,8 @@ static void intel_hpd_init_pins(struct drm_i915_private 
> *dev_priv)
>       if (!HAS_PCH_SPLIT(dev_priv) || HAS_PCH_NOP(dev_priv))
>               return;
>  
> -     if (HAS_PCH_TGP(dev_priv) || HAS_PCH_JSP(dev_priv))
> -             hpd->pch_hpd = hpd_tgp;
> -     else if (HAS_PCH_ICP(dev_priv) || HAS_PCH_MCC(dev_priv))
> +     if (HAS_PCH_TGP(dev_priv) || HAS_PCH_JSP(dev_priv) ||
> +         HAS_PCH_ICP(dev_priv) || HAS_PCH_MCC(dev_priv))
>               hpd->pch_hpd = hpd_icp;
>       else if (HAS_PCH_CNP(dev_priv) || HAS_PCH_SPT(dev_priv))
>               hpd->pch_hpd = hpd_spt;
> @@ -1048,33 +1029,17 @@ static void ivb_parity_work(struct work_struct *work)
>  static bool gen11_port_hotplug_long_detect(enum hpd_pin pin, u32 val)
>  {
>       switch (pin) {
> -     case HPD_PORT_C:
> +     case HPD_PORT_TC1:
>               return val & GEN11_HOTPLUG_CTL_LONG_DETECT(PORT_TC1);
> -     case HPD_PORT_D:
> +     case HPD_PORT_TC2:
>               return val & GEN11_HOTPLUG_CTL_LONG_DETECT(PORT_TC2);
> -     case HPD_PORT_E:
> +     case HPD_PORT_TC3:
>               return val & GEN11_HOTPLUG_CTL_LONG_DETECT(PORT_TC3);
> -     case HPD_PORT_F:
> +     case HPD_PORT_TC4:
>               return val & GEN11_HOTPLUG_CTL_LONG_DETECT(PORT_TC4);
> -     default:
> -             return false;
> -     }
> -}
> -
> -static bool gen12_port_hotplug_long_detect(enum hpd_pin pin, u32 val)
> -{
> -     switch (pin) {
> -     case HPD_PORT_D:
> -             return val & GEN11_HOTPLUG_CTL_LONG_DETECT(PORT_TC1);
> -     case HPD_PORT_E:
> -             return val & GEN11_HOTPLUG_CTL_LONG_DETECT(PORT_TC2);
> -     case HPD_PORT_F:
> -             return val & GEN11_HOTPLUG_CTL_LONG_DETECT(PORT_TC3);
> -     case HPD_PORT_G:
> -             return val & GEN11_HOTPLUG_CTL_LONG_DETECT(PORT_TC4);
> -     case HPD_PORT_H:
> +     case HPD_PORT_TC5:
>               return val & GEN11_HOTPLUG_CTL_LONG_DETECT(PORT_TC5);
> -     case HPD_PORT_I:
> +     case HPD_PORT_TC6:
>               return val & GEN11_HOTPLUG_CTL_LONG_DETECT(PORT_TC6);
>       default:
>               return false;
> @@ -1112,33 +1077,17 @@ static bool icp_ddi_port_hotplug_long_detect(enum 
> hpd_pin pin, u32 val)
>  static bool icp_tc_port_hotplug_long_detect(enum hpd_pin pin, u32 val)
>  {
>       switch (pin) {
> -     case HPD_PORT_C:
> +     case HPD_PORT_TC1:
>               return val & ICP_TC_HPD_LONG_DETECT(PORT_TC1);
> -     case HPD_PORT_D:
> +     case HPD_PORT_TC2:
>               return val & ICP_TC_HPD_LONG_DETECT(PORT_TC2);
> -     case HPD_PORT_E:
> +     case HPD_PORT_TC3:
>               return val & ICP_TC_HPD_LONG_DETECT(PORT_TC3);
> -     case HPD_PORT_F:
> +     case HPD_PORT_TC4:
>               return val & ICP_TC_HPD_LONG_DETECT(PORT_TC4);
> -     default:
> -             return false;
> -     }
> -}
> -
> -static bool tgp_tc_port_hotplug_long_detect(enum hpd_pin pin, u32 val)
> -{
> -     switch (pin) {
> -     case HPD_PORT_D:
> -             return val & ICP_TC_HPD_LONG_DETECT(PORT_TC1);
> -     case HPD_PORT_E:
> -             return val & ICP_TC_HPD_LONG_DETECT(PORT_TC2);
> -     case HPD_PORT_F:
> -             return val & ICP_TC_HPD_LONG_DETECT(PORT_TC3);
> -     case HPD_PORT_G:
> -             return val & ICP_TC_HPD_LONG_DETECT(PORT_TC4);
> -     case HPD_PORT_H:
> +     case HPD_PORT_TC5:
>               return val & ICP_TC_HPD_LONG_DETECT(PORT_TC5);
> -     case HPD_PORT_I:
> +     case HPD_PORT_TC6:
>               return val & ICP_TC_HPD_LONG_DETECT(PORT_TC6);
>       default:
>               return false;
> @@ -1892,19 +1841,16 @@ static void icp_irq_handler(struct drm_i915_private 
> *dev_priv, u32 pch_iir)
>  {
>       u32 ddi_hotplug_trigger, tc_hotplug_trigger;
>       u32 pin_mask = 0, long_mask = 0;
> -     bool (*tc_port_hotplug_long_detect)(enum hpd_pin pin, u32 val);
>  
>       if (HAS_PCH_TGP(dev_priv)) {
>               ddi_hotplug_trigger = pch_iir & SDE_DDI_MASK_TGP;
>               tc_hotplug_trigger = pch_iir & SDE_TC_MASK_TGP;
> -             tc_port_hotplug_long_detect = tgp_tc_port_hotplug_long_detect;
>       } else if (HAS_PCH_JSP(dev_priv)) {
>               ddi_hotplug_trigger = pch_iir & SDE_DDI_MASK_TGP;
>               tc_hotplug_trigger = 0;
>       } else if (HAS_PCH_MCC(dev_priv)) {
>               ddi_hotplug_trigger = pch_iir & SDE_DDI_MASK_ICP;
>               tc_hotplug_trigger = pch_iir & SDE_TC_HOTPLUG_ICP(PORT_TC1);
> -             tc_port_hotplug_long_detect = icp_tc_port_hotplug_long_detect;
>       } else {
>               drm_WARN(&dev_priv->drm, !HAS_PCH_ICP(dev_priv),
>                        "Unrecognized PCH type 0x%x\n",
> @@ -1912,7 +1858,6 @@ static void icp_irq_handler(struct drm_i915_private 
> *dev_priv, u32 pch_iir)
>  
>               ddi_hotplug_trigger = pch_iir & SDE_DDI_MASK_ICP;
>               tc_hotplug_trigger = pch_iir & SDE_TC_MASK_ICP;
> -             tc_port_hotplug_long_detect = icp_tc_port_hotplug_long_detect;
>       }
>  
>       if (ddi_hotplug_trigger) {
> @@ -1936,7 +1881,7 @@ static void icp_irq_handler(struct drm_i915_private 
> *dev_priv, u32 pch_iir)
>               intel_get_hpd_pins(dev_priv, &pin_mask, &long_mask,
>                                  tc_hotplug_trigger, dig_hotplug_reg,
>                                  dev_priv->hotplug.pch_hpd,
> -                                tc_port_hotplug_long_detect);
> +                                icp_tc_port_hotplug_long_detect);
>       }
>  
>       if (pin_mask)
> @@ -2184,12 +2129,6 @@ static void gen11_hpd_irq_handler(struct 
> drm_i915_private *dev_priv, u32 iir)
>       u32 pin_mask = 0, long_mask = 0;
>       u32 trigger_tc = iir & GEN11_DE_TC_HOTPLUG_MASK;
>       u32 trigger_tbt = iir & GEN11_DE_TBT_HOTPLUG_MASK;
> -     long_pulse_detect_func long_pulse_detect;
> -
> -     if (INTEL_GEN(dev_priv) >= 12)
> -             long_pulse_detect = gen12_port_hotplug_long_detect;
> -     else
> -             long_pulse_detect = gen11_port_hotplug_long_detect;
>  
>       if (trigger_tc) {
>               u32 dig_hotplug_reg;
> @@ -2200,7 +2139,7 @@ static void gen11_hpd_irq_handler(struct 
> drm_i915_private *dev_priv, u32 iir)
>               intel_get_hpd_pins(dev_priv, &pin_mask, &long_mask,
>                                  trigger_tc, dig_hotplug_reg,
>                                  dev_priv->hotplug.hpd,
> -                                long_pulse_detect);
> +                                gen11_port_hotplug_long_detect);
>       }
>  
>       if (trigger_tbt) {
> @@ -2212,7 +2151,7 @@ static void gen11_hpd_irq_handler(struct 
> drm_i915_private *dev_priv, u32 iir)
>               intel_get_hpd_pins(dev_priv, &pin_mask, &long_mask,
>                                  trigger_tbt, dig_hotplug_reg,
>                                  dev_priv->hotplug.hpd,
> -                                long_pulse_detect);
> +                                gen11_port_hotplug_long_detect);
>       }
>  
>       if (pin_mask)
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to