All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.