intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: intel-gfx@lists.freedesktop.org,
	Lucas De Marchi <lucas.demarchi@intel.com>,
	Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Subject: Re: [PATCH 07/10] drm/i915/cnl: Add HPD support for Port F.
Date: Mon, 22 Jan 2018 18:51:08 +0200	[thread overview]
Message-ID: <20180122165108.GF5453@intel.com> (raw)
In-Reply-To: <20180120000524.4746-7-rodrigo.vivi@intel.com>

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@intel.com>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  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.

Everything in this patch looks reasonable to me so
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> @@ -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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2018-01-22 16:51 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-20  0:05 [PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU Rodrigo Vivi
2018-01-20  0:05 ` [PATCH 02/10] drm/i915/cnl: Add AUX-F support Rodrigo Vivi
2018-01-22 23:42   ` Pandiyan, Dhinakaran
2018-01-22 23:59     ` [PATCH] " Rodrigo Vivi
2018-01-23  2:43       ` Pandiyan, Dhinakaran
2018-01-23  4:53         ` Pandiyan, Dhinakaran
2018-01-23 16:12           ` Lucas De Marchi
2018-01-23 16:30           ` Rodrigo Vivi
2018-01-23 18:35             ` Runyan, Arthur J
2018-01-23 21:57               ` [PATCH] drm/i915/cnl: Extend Wa 1178 to Aux F Rodrigo Vivi
2018-01-23 23:21                 ` Lucas De Marchi
2018-01-23 21:10         ` [PATCH] drm/i915/cnl: Add AUX-F support Rodrigo Vivi
2018-01-20  0:05 ` [PATCH 03/10] drm/i915/cnl: Fix _CNL_PORT_TX_DW2_LN0_F definition Rodrigo Vivi
2018-01-20  0:05 ` [PATCH 04/10] drm/i915: Fix DPLCLKA_CFGCR0 bits for Port F Rodrigo Vivi
2018-01-22 21:44   ` Pandiyan, Dhinakaran
2018-01-22 23:08     ` Rodrigo Vivi
2018-01-20  0:05 ` [PATCH 05/10] drm/i915/cnl: Add right GMBUS pin number for HDMI on " Rodrigo Vivi
2018-01-20  0:05 ` [PATCH 06/10] drm/i915: For HPD connected port use hpd_pin instead of port Rodrigo Vivi
2018-01-22 16:40   ` Ville Syrjälä
2018-01-22 23:05     ` [PATCH] " Rodrigo Vivi
2018-01-20  0:05 ` [PATCH 07/10] drm/i915/cnl: Add HPD support for Port F Rodrigo Vivi
2018-01-22 16:51   ` Ville Syrjälä [this message]
2018-01-22 23:20     ` Rodrigo Vivi
2018-01-20  0:05 ` [PATCH 08/10] drm/i915/cnl: Enable DDI-F on Cannonlake Rodrigo Vivi
2018-01-23  3:12   ` Pandiyan, Dhinakaran
2018-01-23 16:29     ` Rodrigo Vivi
2018-01-20  0:05 ` [PATCH 09/10] drm/i915/cnl: Fix DP max rate for Cannonlake with port F Rodrigo Vivi
2018-01-22 16:46   ` Ville Syrjälä
2018-01-23 22:32     ` [PATCH] " Rodrigo Vivi
2018-01-23 22:45       ` Manasi Navare
2018-01-20  0:05 ` [PATCH 10/10] drm/i915/cnl: Don't try to manage Port F power wells on all CNL Rodrigo Vivi
2018-01-22 12:12   ` Imre Deak
2018-01-22 23:48     ` [PATCH] " Rodrigo Vivi
2018-01-23  3:03       ` Pandiyan, Dhinakaran
2018-01-23 16:27         ` Rodrigo Vivi
2018-01-23 23:11           ` Rodrigo Vivi
2018-01-20  0:30 ` ✓ Fi.CI.BAT: success for series starting with [01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU Patchwork
2018-01-20  8:40 ` ✗ Fi.CI.IGT: warning " Patchwork
2018-01-22 16:56 ` [PATCH 01/10] " Ville Syrjälä
2018-01-22 23:00   ` Rodrigo Vivi
2018-01-23  0:11 ` ✓ Fi.CI.BAT: success for series starting with [01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU. (rev4) Patchwork
2018-01-23  0:32 ` ✓ Fi.CI.BAT: success for series starting with [01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU. (rev5) Patchwork
2018-01-23  6:54 ` ✗ Fi.CI.IGT: failure " Patchwork
2018-01-23 22:16 ` ✗ Fi.CI.BAT: failure for series starting with [01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU. (rev6) Patchwork
2018-01-23 22:36 ` ✗ Fi.CI.BAT: failure for series starting with [01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU. (rev7) Patchwork
2018-01-23 23:15 ` ✗ Fi.CI.BAT: failure for series starting with [01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU. (rev8) Patchwork
2018-01-25 17:40 ` [PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU Paulo Zanoni

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180122165108.GF5453@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=dhinakaran.pandiyan@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=lucas.demarchi@intel.com \
    --cc=rodrigo.vivi@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).