On Mon, Jan 22, 2018 at 04:51:08PM +0000, Ville Syrjälä wrote:
> On Fri, Jan 19, 2018 at 04:05:21PM -0800, Rodrigo Vivi wrote:
> > On CNP boards that are using DDI F,
> > bit 25 (SDE_PORTE_HOTPLUG_SPT) is representing
> > the Digital Port F hotplug line when the Digital
> > Port F hotplug detect input is enabled.
> >
> > v2: Reuse all existent structure instead of adding a
> > new HPD_PORT_F pointing to pin of port E.
> > v3: Use IS_CNL_WITH_PORT_F so we can start upstreaming
> > this right now. If that SKU ever get a proper name
> > we come back and update it.
> > v4: Rebase on top of digital connected port using encoder
> > instead of port.
> > v5: Moved IS_CNL_WITH_PORT_F definition to the PCI IDs patch.
> >
> > Cc: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
> > Cc: Manasi Navare <manasi.d.navare@xxxxxxxxx>
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@xxxxxxxxx>
> > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/i915_drv.h | 6 ++++--
> > drivers/gpu/drm/i915/i915_irq.c | 35 +++++++++++++++++++----------------
> > drivers/gpu/drm/i915/intel_dp.c | 4 +++-
> > drivers/gpu/drm/i915/intel_hdmi.c | 2 +-
> > drivers/gpu/drm/i915/intel_hotplug.c | 19 +++++++++++++++----
> > 5 files changed, 42 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 7206c7c5f81c..0b5f8d887bfd 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -2957,8 +2957,10 @@ void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
> > void intel_hpd_init(struct drm_i915_private *dev_priv);
> > void intel_hpd_init_work(struct drm_i915_private *dev_priv);
> > void intel_hpd_cancel_work(struct drm_i915_private *dev_priv);
> > -enum port intel_hpd_pin_to_port(enum hpd_pin pin);
> > -enum hpd_pin intel_hpd_pin(enum port port);
> > +enum port intel_hpd_pin_to_port(struct drm_i915_private *dev_priv,
> > + enum hpd_pin pin);
> > +enum hpd_pin intel_hpd_pin_default(struct drm_i915_private *dev_priv,
> > + enum port port);
> > bool intel_hpd_disable(struct drm_i915_private *dev_priv, enum hpd_pin pin);
> > void intel_hpd_enable(struct drm_i915_private *dev_priv, enum hpd_pin pin);
> >
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index 0af970d4b3cf..78af4594eb38 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -1568,10 +1568,11 @@ static bool i9xx_port_hotplug_long_detect(enum port port, u32 val)
> > *
> > * Note that the caller is expected to zero out the masks initially.
> > */
> > -static void intel_get_hpd_pins(u32 *pin_mask, u32 *long_mask,
> > - u32 hotplug_trigger, u32 dig_hotplug_reg,
> > - const u32 hpd[HPD_NUM_PINS],
> > - bool long_pulse_detect(enum port port, u32 val))
> > +static void intel_get_hpd_pins(struct drm_i915_private *dev_priv,
> > + u32 *pin_mask, u32 *long_mask,
> > + u32 hotplug_trigger, u32 dig_hotplug_reg,
> > + const u32 hpd[HPD_NUM_PINS],
> > + bool long_pulse_detect(enum port port, u32 val))
> > {
> > enum port port;
> > int i;
> > @@ -1582,7 +1583,7 @@ static void intel_get_hpd_pins(u32 *pin_mask, u32 *long_mask,
> >
> > *pin_mask |= BIT(i);
> >
> > - port = intel_hpd_pin_to_port(i);
> > + port = intel_hpd_pin_to_port(dev_priv, i);
> > if (port == PORT_NONE)
> > continue;
> >
> > @@ -1970,8 +1971,9 @@ static void i9xx_hpd_irq_handler(struct drm_i915_private *dev_priv,
> > u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_G4X;
> >
> > if (hotplug_trigger) {
> > - intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
> > - hotplug_trigger, hpd_status_g4x,
> > + intel_get_hpd_pins(dev_priv, &pin_mask, &long_mask,
> > + hotplug_trigger, hotplug_trigger,
> > + hpd_status_g4x,
> > i9xx_port_hotplug_long_detect);
> >
> > intel_hpd_irq_handler(dev_priv, pin_mask, long_mask);
> > @@ -1983,8 +1985,9 @@ static void i9xx_hpd_irq_handler(struct drm_i915_private *dev_priv,
> > u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_I915;
> >
> > if (hotplug_trigger) {
> > - intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
> > - hotplug_trigger, hpd_status_i915,
> > + intel_get_hpd_pins(dev_priv, &pin_mask, &long_mask,
> > + hotplug_trigger, hotplug_trigger,
> > + hpd_status_i915,
> > i9xx_port_hotplug_long_detect);
> > intel_hpd_irq_handler(dev_priv, pin_mask, long_mask);
> > }
> > @@ -2185,7 +2188,7 @@ static void ibx_hpd_irq_handler(struct drm_i915_private *dev_priv,
> > if (!hotplug_trigger)
> > return;
> >
> > - intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
> > + intel_get_hpd_pins(dev_priv, &pin_mask, &long_mask, hotplug_trigger,
> > dig_hotplug_reg, hpd,
> > pch_port_hotplug_long_detect);
> >
> > @@ -2327,8 +2330,8 @@ static void spt_irq_handler(struct drm_i915_private *dev_priv, u32 pch_iir)
> > dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG);
> > I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg);
> >
> > - intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
> > - dig_hotplug_reg, hpd_spt,
> > + intel_get_hpd_pins(dev_priv, &pin_mask, &long_mask,
> > + hotplug_trigger, dig_hotplug_reg, hpd_spt,
> > spt_port_hotplug_long_detect);
> > }
> >
> > @@ -2338,8 +2341,8 @@ static void spt_irq_handler(struct drm_i915_private *dev_priv, u32 pch_iir)
> > dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG2);
> > I915_WRITE(PCH_PORT_HOTPLUG2, dig_hotplug_reg);
> >
> > - intel_get_hpd_pins(&pin_mask, &long_mask, hotplug2_trigger,
> > - dig_hotplug_reg, hpd_spt,
> > + intel_get_hpd_pins(dev_priv, &pin_mask, &long_mask,
> > + hotplug2_trigger, dig_hotplug_reg, hpd_spt,
> > spt_port_hotplug2_long_detect);
> > }
> >
> > @@ -2359,7 +2362,7 @@ static void ilk_hpd_irq_handler(struct drm_i915_private *dev_priv,
> > dig_hotplug_reg = I915_READ(DIGITAL_PORT_HOTPLUG_CNTRL);
> > I915_WRITE(DIGITAL_PORT_HOTPLUG_CNTRL, dig_hotplug_reg);
> >
> > - intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
> > + intel_get_hpd_pins(dev_priv, &pin_mask, &long_mask, hotplug_trigger,
> > dig_hotplug_reg, hpd,
> > ilk_port_hotplug_long_detect);
> >
> > @@ -2536,7 +2539,7 @@ static void bxt_hpd_irq_handler(struct drm_i915_private *dev_priv,
> > dig_hotplug_reg = I915_READ(PCH_PORT_HOTPLUG);
> > I915_WRITE(PCH_PORT_HOTPLUG, dig_hotplug_reg);
> >
> > - intel_get_hpd_pins(&pin_mask, &long_mask, hotplug_trigger,
> > + intel_get_hpd_pins(dev_priv, &pin_mask, &long_mask, hotplug_trigger,
> > dig_hotplug_reg, hpd,
> > bxt_port_hotplug_long_detect);
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 4ff6fcf542e9..4b963732454d 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -6205,8 +6205,10 @@ intel_dp_init_connector_port_info(struct intel_digital_port *intel_dig_port)
> > {
> > struct intel_encoder *encoder = &intel_dig_port->base;
> > struct intel_dp *intel_dp = &intel_dig_port->dp;
> > + struct intel_encoder *intel_encoder = &intel_dig_port->base;
> > + struct drm_i915_private *dev_priv = to_i915(intel_encoder->base.dev);
> >
> > - encoder->hpd_pin = intel_hpd_pin(encoder->port);
> > + encoder->hpd_pin = intel_hpd_pin_default(dev_priv, encoder->port);
> >
> > switch (encoder->port) {
> > case PORT_A:
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> > index 0d924ba42075..f9a1a32fd272 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > @@ -2334,7 +2334,7 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
> >
> > if (WARN_ON(port == PORT_A))
> > return;
> > - intel_encoder->hpd_pin = intel_hpd_pin(port);
> > + intel_encoder->hpd_pin = intel_hpd_pin_default(dev_priv, port);
> >
> > if (HAS_DDI(dev_priv))
> > intel_connector->get_hw_state = intel_ddi_connector_get_hw_state;
> > diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
> > index 875d5d218d5c..fe28c1ea84a5 100644
> > --- a/drivers/gpu/drm/i915/intel_hotplug.c
> > +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> > @@ -78,12 +78,14 @@
> >
> > /**
> > * intel_hpd_port - return port hard associated with certain pin.
> > + * @dev_priv: private driver data pointer
> > * @pin: the hpd pin to get associated port
> > *
> > * Return port that is associatade with @pin and PORT_NONE if no port is
> > * hard associated with that @pin.
> > */
> > -enum port intel_hpd_pin_to_port(enum hpd_pin pin)
> > +enum port intel_hpd_pin_to_port(struct drm_i915_private *dev_priv,
> > + enum hpd_pin pin)
> > {
> > switch (pin) {
> > case HPD_PORT_A:
> > @@ -95,6 +97,8 @@ enum port intel_hpd_pin_to_port(enum hpd_pin pin)
> > case HPD_PORT_D:
> > return PORT_D;
> > case HPD_PORT_E:
> > + if (IS_CNL_WITH_PORT_F(dev_priv))
> > + return PORT_F;
> > return PORT_E;
> > default:
> > return PORT_NONE; /* no port for this pin */
>
> I'm thinking we could rewrite this function as
>
> for_each_intel_encoder(...) {
> if (encoder->hpd_pin == pin)
> return encoder->port;
> }
>
> That way we have no hardcoded mapping anywhere but
> intel_hpd_pin_default().
>
> But this could be a followup patch.
I considered that, and even start to write here,
but irq handler would need to start knowing about encoder...
worth?
>
> Everything in this patch looks reasonable to me so
> Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
thanks
>
> > @@ -102,13 +106,17 @@ enum port intel_hpd_pin_to_port(enum hpd_pin pin)
> > }
> >
> > /**
> > - * intel_hpd_pin - return pin hard associated with certain port.
> > + * intel_hpd_pin_default - return default pin associated with certain port.
> > + * @dev_priv: private driver data pointer
> > * @port: the hpd port to get associated pin
> > *
> > + * 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.
> > */
> > -enum hpd_pin intel_hpd_pin(enum port port)
> > +enum hpd_pin intel_hpd_pin_default(struct drm_i915_private *dev_priv,
> > + enum port port)
> > {
> > switch (port) {
> > case PORT_A:
> > @@ -121,6 +129,9 @@ enum hpd_pin intel_hpd_pin(enum port port)
> > return HPD_PORT_D;
> > case PORT_E:
> > return HPD_PORT_E;
> > + case PORT_F:
> > + if (IS_CNL_WITH_PORT_F(dev_priv))
> > + return HPD_PORT_E;
> > default:
> > MISSING_CASE(port);
> > return HPD_NONE;
> > @@ -417,7 +428,7 @@ void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
> > if (!(BIT(i) & pin_mask))
> > continue;
> >
> > - port = intel_hpd_pin_to_port(i);
> > + port = intel_hpd_pin_to_port(dev_priv, i);
> > is_dig_port = port != PORT_NONE &&
> > dev_priv->hotplug.irq_port[port];
> >
> > --
> > 2.13.6
>
> --
> Ville Syrjälä
> Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
