On Thu, Jul 25, 2019 at 04:48:11PM -0700, Lucas De Marchi wrote:
> Add hotdplug detection for all ports on TGP. icp_hpd_detection_setup()
> is refactored to be shared with TGP.
>
> While we increase the number of pins, add a BUILD_BUG_ON() to avoid
> going over the number of bits allowed.
>
> v2: use BITS_PER_TYPE and correct type for BUILD_BUG_ON() check
> (requested by Ville)
>
> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Cc: Jose Souza <jose.souza@xxxxxxxxx>
> Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_hotplug.c | 6 +
> drivers/gpu/drm/i915/i915_drv.h | 4 +
> drivers/gpu/drm/i915/i915_irq.c | 128 +++++++++++++++++--
> drivers/gpu/drm/i915/i915_reg.h | 28 +++-
> 4 files changed, 154 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c b/drivers/gpu/drm/i915/display/intel_hotplug.c
> index 342587d91d57..c844ae4480af 100644
> --- a/drivers/gpu/drm/i915/display/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
> @@ -104,6 +104,12 @@ enum hpd_pin intel_hpd_pin_default(struct drm_i915_private *dev_priv,
> if (IS_CNL_WITH_PORT_F(dev_priv))
> return HPD_PORT_E;
> return HPD_PORT_F;
> + case PORT_G:
> + return HPD_PORT_G;
> + case PORT_H:
> + return HPD_PORT_H;
> + case PORT_I:
> + return HPD_PORT_I;
> default:
> MISSING_CASE(port);
> return HPD_NONE;
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 59d4a1146039..a2e6b495f033 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -153,6 +153,10 @@ enum hpd_pin {
> HPD_PORT_D,
> HPD_PORT_E,
> HPD_PORT_F,
> + HPD_PORT_G,
> + HPD_PORT_H,
> + HPD_PORT_I,
> +
> HPD_NUM_PINS
> };
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index a17d4fd17962..34527cdd9388 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -150,6 +150,18 @@ static const u32 hpd_mcc[HPD_NUM_PINS] = {
> [HPD_PORT_C] = SDE_TC1_HOTPLUG_ICP
> };
>
> +static const u32 hpd_tgp[HPD_NUM_PINS] = {
> + [HPD_PORT_A] = SDE_DDIA_HOTPLUG_ICP,
> + [HPD_PORT_B] = SDE_DDIB_HOTPLUG_ICP,
> + [HPD_PORT_C] = SDE_DDIC_HOTPLUG_TGP,
> + [HPD_PORT_D] = SDE_TC1_HOTPLUG_ICP,
> + [HPD_PORT_E] = SDE_TC2_HOTPLUG_ICP,
> + [HPD_PORT_F] = SDE_TC3_HOTPLUG_ICP,
> + [HPD_PORT_G] = SDE_TC4_HOTPLUG_ICP,
> + [HPD_PORT_H] = SDE_TC5_HOTPLUG_TGP,
> + [HPD_PORT_I] = SDE_TC6_HOTPLUG_TGP,
> +};
> +
> static void gen3_irq_reset(struct intel_uncore *uncore, i915_reg_t imr,
> i915_reg_t iir, i915_reg_t ier)
> {
> @@ -1724,6 +1736,40 @@ static bool icp_tc_port_hotplug_long_detect(enum hpd_pin pin, u32 val)
> }
> }
>
> +static bool tgp_ddi_port_hotplug_long_detect(enum hpd_pin pin, u32 val)
> +{
> + switch (pin) {
> + case HPD_PORT_A:
> + return val & ICP_DDIA_HPD_LONG_DETECT;
> + case HPD_PORT_B:
> + return val & ICP_DDIB_HPD_LONG_DETECT;
> + case HPD_PORT_C:
> + return val & TGP_DDIC_HPD_LONG_DETECT;
> + 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:
> + return val & ICP_TC_HPD_LONG_DETECT(PORT_TC5);
> + case HPD_PORT_I:
> + return val & ICP_TC_HPD_LONG_DETECT(PORT_TC6);
> + default:
> + return false;
> + }
> +}
> +
> static bool spt_port_hotplug2_long_detect(enum hpd_pin pin, u32 val)
> {
> switch (pin) {
> @@ -1803,6 +1849,8 @@ static void intel_get_hpd_pins(struct drm_i915_private *dev_priv,
> {
> enum hpd_pin pin;
>
> + BUILD_BUG_ON(BITS_PER_TYPE(*pin_mask) < HPD_NUM_PINS);
> +
> for_each_hpd_pin(pin) {
> if ((hpd[pin] & hotplug_trigger) == 0)
> continue;
> @@ -2561,6 +2609,43 @@ static void icp_irq_handler(struct drm_i915_private *dev_priv, u32 pch_iir,
> gmbus_irq_handler(dev_priv);
> }
>
> +static void tgp_irq_handler(struct drm_i915_private *dev_priv, u32 pch_iir)
This function winds up being pretty similar to icp_irq_handler. Would
it be simpler to just reuse the icp handler and set the appropriate
masks and long detect handler based on platform at the top?
FWIW, I need to do a similar change to the ICP handler for EHL anyway;
something like:
if (HAS_PCH_MCC(dev_priv)) {
ddi_hotplug_trigger = pch_iir & SDE_DDI_MASK_TGP;
tc_hotplug_trigger = 0;
} else {
ddi_hotplug_trigger = pch_iir & SDE_DDI_MASK_ICP;
tc_hotplug_trigger = pch_iir & SDE_TC_MASK_ICP;
}
to account for the extra port C on EHL that we're currently missing.
Adding TGP as one more case (and assigning the long detect handler too)
might be easier.
Regardless of what you decide, the rest of this patch seems to match
what I see in the TGL bspec, so
Reviewed-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
> +{
> + u32 ddi_hotplug_trigger = pch_iir & SDE_DDI_MASK_TGP;
> + u32 tc_hotplug_trigger = pch_iir & SDE_TC_MASK_TGP;
> + u32 pin_mask = 0, long_mask = 0;
> +
> + if (ddi_hotplug_trigger) {
> + u32 dig_hotplug_reg;
> +
> + dig_hotplug_reg = I915_READ(SHOTPLUG_CTL_DDI);
> + I915_WRITE(SHOTPLUG_CTL_DDI, dig_hotplug_reg);
> +
> + intel_get_hpd_pins(dev_priv, &pin_mask, &long_mask,
> + ddi_hotplug_trigger,
> + dig_hotplug_reg, hpd_tgp,
> + tgp_ddi_port_hotplug_long_detect);
> + }
> +
> + if (tc_hotplug_trigger) {
> + u32 dig_hotplug_reg;
> +
> + dig_hotplug_reg = I915_READ(SHOTPLUG_CTL_TC);
> + I915_WRITE(SHOTPLUG_CTL_TC, dig_hotplug_reg);
> +
> + intel_get_hpd_pins(dev_priv, &pin_mask, &long_mask,
> + tc_hotplug_trigger,
> + dig_hotplug_reg, hpd_tgp,
> + tgp_tc_port_hotplug_long_detect);
> + }
> +
> + if (pin_mask)
> + intel_hpd_irq_handler(dev_priv, pin_mask, long_mask);
> +
> + if (pch_iir & SDE_GMBUS_ICP)
> + gmbus_irq_handler(dev_priv);
> +}
> +
> static void spt_irq_handler(struct drm_i915_private *dev_priv, u32 pch_iir)
> {
> u32 hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK_SPT &
> @@ -2983,7 +3068,9 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
> I915_WRITE(SDEIIR, iir);
> ret = IRQ_HANDLED;
>
> - if (INTEL_PCH_TYPE(dev_priv) >= PCH_MCC)
> + if (INTEL_PCH_TYPE(dev_priv) >= PCH_TGP)
> + tgp_irq_handler(dev_priv, iir);
> + else if (INTEL_PCH_TYPE(dev_priv) >= PCH_MCC)
> icp_irq_handler(dev_priv, iir, hpd_mcc);
> else if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP)
> icp_irq_handler(dev_priv, iir, hpd_icp);
> @@ -3778,20 +3865,18 @@ static void ibx_hpd_irq_setup(struct drm_i915_private *dev_priv)
> ibx_hpd_detection_setup(dev_priv);
> }
>
> -static void icp_hpd_detection_setup(struct drm_i915_private *dev_priv)
> +static void icp_hpd_detection_setup(struct drm_i915_private *dev_priv,
> + u32 ddi_hotplug_enable_mask,
> + u32 tc_hotplug_enable_mask)
> {
> u32 hotplug;
>
> hotplug = I915_READ(SHOTPLUG_CTL_DDI);
> - hotplug |= ICP_DDIA_HPD_ENABLE |
> - ICP_DDIB_HPD_ENABLE;
> + hotplug |= ddi_hotplug_enable_mask;
> I915_WRITE(SHOTPLUG_CTL_DDI, hotplug);
>
> hotplug = I915_READ(SHOTPLUG_CTL_TC);
> - hotplug |= ICP_TC_HPD_ENABLE(PORT_TC1) |
> - ICP_TC_HPD_ENABLE(PORT_TC2) |
> - ICP_TC_HPD_ENABLE(PORT_TC3) |
> - ICP_TC_HPD_ENABLE(PORT_TC4);
> + hotplug |= tc_hotplug_enable_mask;
> I915_WRITE(SHOTPLUG_CTL_TC, hotplug);
> }
>
> @@ -3804,7 +3889,21 @@ static void icp_hpd_irq_setup(struct drm_i915_private *dev_priv)
>
> ibx_display_interrupt_update(dev_priv, hotplug_irqs, enabled_irqs);
>
> - icp_hpd_detection_setup(dev_priv);
> + icp_hpd_detection_setup(dev_priv, ICP_DDI_HPD_ENABLE_MASK,
> + ICP_TC_HPD_ENABLE_MASK);
> +}
> +
> +static void tgp_hpd_irq_setup(struct drm_i915_private *dev_priv)
> +{
> + u32 hotplug_irqs, enabled_irqs;
> +
> + hotplug_irqs = SDE_DDI_MASK_TGP | SDE_TC_MASK_TGP;
> + enabled_irqs = intel_hpd_enabled_irqs(dev_priv, hpd_tgp);
> +
> + ibx_display_interrupt_update(dev_priv, hotplug_irqs, enabled_irqs);
> +
> + icp_hpd_detection_setup(dev_priv, TGP_DDI_HPD_ENABLE_MASK,
> + TGP_TC_HPD_ENABLE_MASK);
> }
>
> static void gen11_hpd_detection_setup(struct drm_i915_private *dev_priv)
> @@ -3841,7 +3940,9 @@ static void gen11_hpd_irq_setup(struct drm_i915_private *dev_priv)
>
> gen11_hpd_detection_setup(dev_priv);
>
> - if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP)
> + if (INTEL_PCH_TYPE(dev_priv) >= PCH_TGP)
> + tgp_hpd_irq_setup(dev_priv);
> + else if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP)
> icp_hpd_irq_setup(dev_priv);
> }
>
> @@ -4291,7 +4392,12 @@ static void icp_irq_postinstall(struct drm_i915_private *dev_priv)
> gen3_assert_iir_is_zero(&dev_priv->uncore, SDEIIR);
> I915_WRITE(SDEIMR, ~mask);
>
> - icp_hpd_detection_setup(dev_priv);
> + if (HAS_PCH_TGP(dev_priv))
> + icp_hpd_detection_setup(dev_priv, TGP_DDI_HPD_ENABLE_MASK,
> + TGP_TC_HPD_ENABLE_MASK);
> + else
> + icp_hpd_detection_setup(dev_priv, ICP_DDI_HPD_ENABLE_MASK,
> + ICP_TC_HPD_ENABLE_MASK);
> }
>
> static void gen11_irq_postinstall(struct drm_i915_private *dev_priv)
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 24f2a52a2b42..c5042396dbb1 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7861,12 +7861,15 @@ enum {
> SDE_FDI_RXB_CPT | \
> SDE_FDI_RXA_CPT)
>
> -/* south display engine interrupt: ICP */
> +/* south display engine interrupt: ICP/TGP */
> +#define SDE_TC6_HOTPLUG_TGP (1 << 29)
> +#define SDE_TC5_HOTPLUG_TGP (1 << 28)
> #define SDE_TC4_HOTPLUG_ICP (1 << 27)
> #define SDE_TC3_HOTPLUG_ICP (1 << 26)
> #define SDE_TC2_HOTPLUG_ICP (1 << 25)
> #define SDE_TC1_HOTPLUG_ICP (1 << 24)
> #define SDE_GMBUS_ICP (1 << 23)
> +#define SDE_DDIC_HOTPLUG_TGP (1 << 18)
> #define SDE_DDIB_HOTPLUG_ICP (1 << 17)
> #define SDE_DDIA_HOTPLUG_ICP (1 << 16)
> #define SDE_TC_HOTPLUG_ICP(tc_port) (1 << ((tc_port) + 24))
> @@ -7877,6 +7880,11 @@ enum {
> SDE_TC3_HOTPLUG_ICP | \
> SDE_TC2_HOTPLUG_ICP | \
> SDE_TC1_HOTPLUG_ICP)
> +#define SDE_DDI_MASK_TGP (SDE_DDIC_HOTPLUG_TGP | \
> + SDE_DDI_MASK_ICP)
> +#define SDE_TC_MASK_TGP (SDE_TC6_HOTPLUG_TGP | \
> + SDE_TC5_HOTPLUG_TGP | \
> + SDE_TC_MASK_ICP)
>
> #define SDEISR _MMIO(0xc4000)
> #define SDEIMR _MMIO(0xc4004)
> @@ -7944,6 +7952,12 @@ enum {
> */
>
> #define SHOTPLUG_CTL_DDI _MMIO(0xc4030)
> +#define TGP_DDIC_HPD_ENABLE (1 << 11)
> +#define TGP_DDIC_HPD_STATUS_MASK (3 << 8)
> +#define TGP_DDIC_HPD_NO_DETECT (0 << 8)
> +#define TGP_DDIC_HPD_SHORT_DETECT (1 << 8)
> +#define TGP_DDIC_HPD_LONG_DETECT (2 << 8)
> +#define TGP_DDIC_HPD_SHORT_LONG_DETECT (3 << 8)
> #define ICP_DDIB_HPD_ENABLE (1 << 7)
> #define ICP_DDIB_HPD_STATUS_MASK (3 << 4)
> #define ICP_DDIB_HPD_NO_DETECT (0 << 4)
> @@ -8067,6 +8081,18 @@ enum {
> #define ICP_TC_HPD_LONG_DETECT(tc_port) (2 << (tc_port) * 4)
> #define ICP_TC_HPD_SHORT_DETECT(tc_port) (1 << (tc_port) * 4)
>
> +#define ICP_DDI_HPD_ENABLE_MASK (ICP_DDIB_HPD_ENABLE | \
> + ICP_DDIA_HPD_ENABLE)
> +#define ICP_TC_HPD_ENABLE_MASK (ICP_TC_HPD_ENABLE(PORT_TC4) | \
> + ICP_TC_HPD_ENABLE(PORT_TC3) | \
> + ICP_TC_HPD_ENABLE(PORT_TC2) | \
> + ICP_TC_HPD_ENABLE(PORT_TC1))
> +#define TGP_DDI_HPD_ENABLE_MASK (TGP_DDIC_HPD_ENABLE | \
> + ICP_DDI_HPD_ENABLE_MASK)
> +#define TGP_TC_HPD_ENABLE_MASK (ICP_TC_HPD_ENABLE(PORT_TC6) | \
> + ICP_TC_HPD_ENABLE(PORT_TC5) | \
> + ICP_TC_HPD_ENABLE_MASK)
> +
> #define _PCH_DPLL_A 0xc6014
> #define _PCH_DPLL_B 0xc6018
> #define PCH_DPLL(pll) _MMIO((pll) == 0 ? _PCH_DPLL_A : _PCH_DPLL_B)
> --
> 2.21.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
