* Re: [Intel-gfx] [10/12] drm/i915: Introduce HPD_PORT_TC<n>
[not found] <20200630215601.28557-11-ville.syrjala@linux.intel.com>
@ 2020-09-12 1:30 ` Souza, Jose
2020-09-14 14:48 ` Ville Syrjälä
0 siblings, 1 reply; 4+ messages in thread
From: Souza, Jose @ 2020-09-12 1:30 UTC (permalink / raw)
To: ville.syrjala@linux.intel.com, intel-gfx@lists.freedesktop.org
On Wed, 2020-07-01 at 00:55 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <
> ville.syrjala@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.syrjala@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
^ permalink raw reply [flat|nested] 4+ messages in thread