* 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
* Re: [Intel-gfx] [10/12] drm/i915: Introduce HPD_PORT_TC<n>
2020-09-12 1:30 ` [Intel-gfx] [10/12] drm/i915: Introduce HPD_PORT_TC<n> Souza, Jose
@ 2020-09-14 14:48 ` Ville Syrjälä
2020-09-14 16:58 ` Souza, Jose
0 siblings, 1 reply; 4+ messages in thread
From: Ville Syrjälä @ 2020-09-14 14:48 UTC (permalink / raw)
To: Souza, Jose; +Cc: intel-gfx@lists.freedesktop.org
On Sat, Sep 12, 2020 at 01:30:23AM +0000, Souza, Jose wrote:
> 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.
On rkl+tgp we want:
PORT_A (DDI A) -> HPD_PORT_A
PORT_B (DDI B) -> HPD_PORT_B
PORT_D (DDI TC1) -> HPD_PORT_TC1
PORT_E (DDI TC2) -> HPD_PORT_TC2
On rkl+cmp we want:
PORT_A (DDI A) -> HPD_PORT_A
PORT_B (DDI B) -> HPD_PORT_B
PORT_D (DDI TC1) -> HPD_PORT_C
PORT_E (DDI TC2) -> HPD_PORT_D
>
> > + 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.
On jsl+mcc we want:
PORT_A/D (DDI A/D) -> HPD_PORT_A
PORT_B (DDI B) -> HPD_PORT_B
PORT_C (DDI C) -> HPD_PORT_TC1
on jsl+icp we want:
PORT_A/D (DDI A/D) -> HPD_PORT_A
PORT_B (DDI B) -> HPD_PORT_B
PORT_C (DDI C) -> HPD_PORT_C
--
Ville Syrjälä
Intel
_______________________________________________
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
* Re: [Intel-gfx] [10/12] drm/i915: Introduce HPD_PORT_TC<n>
2020-09-14 14:48 ` Ville Syrjälä
@ 2020-09-14 16:58 ` Souza, Jose
2020-09-14 17:06 ` Ville Syrjälä
0 siblings, 1 reply; 4+ messages in thread
From: Souza, Jose @ 2020-09-14 16:58 UTC (permalink / raw)
To: ville.syrjala@linux.intel.com; +Cc: intel-gfx@lists.freedesktop.org
On Mon, 2020-09-14 at 17:48 +0300, Ville Syrjälä wrote:
> On Sat, Sep 12, 2020 at 01:30:23AM +0000, Souza, Jose wrote:
> > 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.
>
> On rkl+tgp we want:
> PORT_A (DDI A) -> HPD_PORT_A
> PORT_B (DDI B) -> HPD_PORT_B
> PORT_D (DDI TC1) -> HPD_PORT_TC1
> PORT_E (DDI TC2) -> HPD_PORT_TC2
>
> On rkl+cmp we want:
> PORT_A (DDI A) -> HPD_PORT_A
> PORT_B (DDI B) -> HPD_PORT_B
> PORT_D (DDI TC1) -> HPD_PORT_C
> PORT_E (DDI TC2) -> HPD_PORT_D
oohh okay, missed this.
>
> > > + 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.
>
> On jsl+mcc we want:
> PORT_A/D (DDI A/D) -> HPD_PORT_A
> PORT_B (DDI B) -> HPD_PORT_B
> PORT_C (DDI C) -> HPD_PORT_TC1
>
> on jsl+icp we want:
> PORT_A/D (DDI A/D) -> HPD_PORT_A
> PORT_B (DDI B) -> HPD_PORT_B
> PORT_C (DDI C) -> HPD_PORT_C
>
>
The above would be the output of tgl_hpd_pin() but okay as it can be associate with SPT, LPT, ICP and TGP better keep the current code.
Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
_______________________________________________
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
* Re: [Intel-gfx] [10/12] drm/i915: Introduce HPD_PORT_TC<n>
2020-09-14 16:58 ` Souza, Jose
@ 2020-09-14 17:06 ` Ville Syrjälä
0 siblings, 0 replies; 4+ messages in thread
From: Ville Syrjälä @ 2020-09-14 17:06 UTC (permalink / raw)
To: Souza, Jose; +Cc: intel-gfx@lists.freedesktop.org
On Mon, Sep 14, 2020 at 04:58:33PM +0000, Souza, Jose wrote:
> On Mon, 2020-09-14 at 17:48 +0300, Ville Syrjälä wrote:
> > On Sat, Sep 12, 2020 at 01:30:23AM +0000, Souza, Jose wrote:
> > > 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.
> >
> > On rkl+tgp we want:
> > PORT_A (DDI A) -> HPD_PORT_A
> > PORT_B (DDI B) -> HPD_PORT_B
> > PORT_D (DDI TC1) -> HPD_PORT_TC1
> > PORT_E (DDI TC2) -> HPD_PORT_TC2
> >
> > On rkl+cmp we want:
> > PORT_A (DDI A) -> HPD_PORT_A
> > PORT_B (DDI B) -> HPD_PORT_B
> > PORT_D (DDI TC1) -> HPD_PORT_C
> > PORT_E (DDI TC2) -> HPD_PORT_D
>
> oohh okay, missed this.
>
> >
> > > > + 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.
> >
> > On jsl+mcc we want:
> > PORT_A/D (DDI A/D) -> HPD_PORT_A
> > PORT_B (DDI B) -> HPD_PORT_B
> > PORT_C (DDI C) -> HPD_PORT_TC1
> >
> > on jsl+icp we want:
> > PORT_A/D (DDI A/D) -> HPD_PORT_A
> > PORT_B (DDI B) -> HPD_PORT_B
> > PORT_C (DDI C) -> HPD_PORT_C
> >
> >
>
> The above would be the output of tgl_hpd_pin() but okay as it can be associate with SPT, LPT, ICP and TGP better keep the current code.
I suspect we probably want to change this to the already discussed
more declarative approach at some point, so it'll be easier to see
what maps to what. But in the meantime this at least gets this
hpd pin mapping stuff out from the guts of the irq code.
>
> Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
Ta.
--
Ville Syrjälä
Intel
_______________________________________________
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
end of thread, other threads:[~2020-09-14 17:06 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20200630215601.28557-11-ville.syrjala@linux.intel.com>
2020-09-12 1:30 ` [Intel-gfx] [10/12] drm/i915: Introduce HPD_PORT_TC<n> Souza, Jose
2020-09-14 14:48 ` Ville Syrjälä
2020-09-14 16:58 ` Souza, Jose
2020-09-14 17:06 ` Ville Syrjälä
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox