diff mbox series

[1/3] drm/i915/dg2: Enable 5th display

Message ID 20220215055154.15363-2-ramalingam.c@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/dg2: 5th Display output | expand

Commit Message

Ramalingam C Feb. 15, 2022, 5:51 a.m. UTC 
From: Matt Roper <matthew.d.roper@intel.com>

DG2 supports a 5th display output which the hardware refers to as "TC1,"
even though it isn't a Type-C output.  This behaves similarly to the TC1
on past platforms with just a couple minor differences:

 * DG2's TC1 bit in SDEISR is at bit 25 rather than 24 as it is on
   ICP/TGP/ADP.
 * DG2 doesn't need the hpd inversion setting that we had to use on DG1

Cc: Swathi Dhanavanthri <swathi.dhanavanthri@intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: José Roberto de Souza <jose.souza@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
---
 drivers/gpu/drm/i915/display/intel_gmbus.c | 16 ++++++++++++++--
 drivers/gpu/drm/i915/i915_irq.c            |  5 ++++-
 drivers/gpu/drm/i915/i915_reg.h            |  1 +
 3 files changed, 19 insertions(+), 3 deletions(-)

Comments

Shankar, Uma Feb. 16, 2022, 8:02 a.m. UTC | #1 
> -----Original Message-----
> From: C, Ramalingam <ramalingam.c@intel.com>
> Sent: Tuesday, February 15, 2022 11:22 AM
> To: intel-gfx <intel-gfx@lists.freedesktop.org>; dri-devel <dri-
> devel@lists.freedesktop.org>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>; Shankar, Uma
> <uma.shankar@intel.com>; Roper, Matthew D <matthew.d.roper@intel.com>;
> Dhanavanthri, Swathi <swathi.dhanavanthri@intel.com>; De Marchi, Lucas
> <lucas.demarchi@intel.com>; Souza, Jose <jose.souza@intel.com>; C, Ramalingam
> <ramalingam.c@intel.com>
> Subject: [PATCH 1/3] drm/i915/dg2: Enable 5th display
> 
> From: Matt Roper <matthew.d.roper@intel.com>
> 
> DG2 supports a 5th display output which the hardware refers to as "TC1,"
> even though it isn't a Type-C output.  This behaves similarly to the TC1 on past
> platforms with just a couple minor differences:
> 
>  * DG2's TC1 bit in SDEISR is at bit 25 rather than 24 as it is on
>    ICP/TGP/ADP.
>  * DG2 doesn't need the hpd inversion setting that we had to use on DG1
> 
> Cc: Swathi Dhanavanthri <swathi.dhanavanthri@intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_gmbus.c | 16 ++++++++++++++--
>  drivers/gpu/drm/i915/i915_irq.c            |  5 ++++-
>  drivers/gpu/drm/i915/i915_reg.h            |  1 +
>  3 files changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_gmbus.c
> b/drivers/gpu/drm/i915/display/intel_gmbus.c
> index 6ce8c10fe975..2fad03250661 100644
> --- a/drivers/gpu/drm/i915/display/intel_gmbus.c
> +++ b/drivers/gpu/drm/i915/display/intel_gmbus.c
> @@ -98,11 +98,21 @@ static const struct gmbus_pin gmbus_pins_dg1[] = {
>  	[GMBUS_PIN_4_CNP] = { "dpd", GPIOE },
>  };
> 
> +static const struct gmbus_pin gmbus_pins_dg2[] = {
> +	[GMBUS_PIN_1_BXT] = { "dpa", GPIOB },
> +	[GMBUS_PIN_2_BXT] = { "dpb", GPIOC },
> +	[GMBUS_PIN_3_BXT] = { "dpc", GPIOD },
> +	[GMBUS_PIN_4_CNP] = { "dpd", GPIOE },
> +	[GMBUS_PIN_9_TC1_ICP] = { "tc1", GPIOJ }, };
> +
>  /* pin is expected to be valid */
>  static const struct gmbus_pin *get_gmbus_pin(struct drm_i915_private *dev_priv,
>  					     unsigned int pin)
>  {
> -	if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1)
> +	if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG2)
> +		return &gmbus_pins_dg2[pin];
> +	else if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1)
>  		return &gmbus_pins_dg1[pin];
>  	else if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP)
>  		return &gmbus_pins_icp[pin];
> @@ -123,7 +133,9 @@ bool intel_gmbus_is_valid_pin(struct drm_i915_private
> *dev_priv,  {
>  	unsigned int size;
> 
> -	if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1)
> +	if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG2)
> +		size = ARRAY_SIZE(gmbus_pins_dg2);
> +	else if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1)
>  		size = ARRAY_SIZE(gmbus_pins_dg1);
>  	else if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP)
>  		size = ARRAY_SIZE(gmbus_pins_icp);
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index
> fdd568ba4a16..4d81063b128c 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -179,6 +179,7 @@ static const u32 hpd_sde_dg1[HPD_NUM_PINS] = {
>  	[HPD_PORT_B] = SDE_DDI_HOTPLUG_ICP(HPD_PORT_B),
>  	[HPD_PORT_C] = SDE_DDI_HOTPLUG_ICP(HPD_PORT_C),
>  	[HPD_PORT_D] = SDE_DDI_HOTPLUG_ICP(HPD_PORT_D),
> +	[HPD_PORT_TC1] = SDE_TC_HOTPLUG_DG2(HPD_PORT_TC1),

Not sure if this applies to DG1, the 5th TC1 port is only for DG2.  Should we not have a separate
DG2 struct. Can you please clarify to help understand.

Regards,
Uma Shankar

>  };
> 
>  static void intel_hpd_init_pins(struct drm_i915_private *dev_priv) @@ -4424,7
> +4425,9 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
>  		if (I915_HAS_HOTPLUG(dev_priv))
>  			dev_priv->hotplug_funcs = &i915_hpd_funcs;
>  	} else {
> -		if (HAS_PCH_DG1(dev_priv))
> +		if (HAS_PCH_DG2(dev_priv))
> +			dev_priv->hotplug_funcs = &icp_hpd_funcs;
> +		else if (HAS_PCH_DG1(dev_priv))
>  			dev_priv->hotplug_funcs = &dg1_hpd_funcs;
>  		else if (DISPLAY_VER(dev_priv) >= 11)
>  			dev_priv->hotplug_funcs = &gen11_hpd_funcs; diff --git
> a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index
> 4ea1713e6b60..4d12abb2d7ff 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6182,6 +6182,7 @@
>  /* south display engine interrupt: ICP/TGP */
>  #define SDE_GMBUS_ICP			(1 << 23)
>  #define SDE_TC_HOTPLUG_ICP(hpd_pin)	REG_BIT(24 +
> _HPD_PIN_TC(hpd_pin))
> +#define SDE_TC_HOTPLUG_DG2(hpd_pin)	REG_BIT(25 +
> _HPD_PIN_TC(hpd_pin)) /* sigh */
>  #define SDE_DDI_HOTPLUG_ICP(hpd_pin)	REG_BIT(16 +
> _HPD_PIN_DDI(hpd_pin))
>  #define SDE_DDI_HOTPLUG_MASK_ICP	(SDE_DDI_HOTPLUG_ICP(HPD_PORT_D) | \
>  					 SDE_DDI_HOTPLUG_ICP(HPD_PORT_C) | \
> --
> 2.20.1
Matt Roper Feb. 17, 2022, 4:33 p.m. UTC | #2 
On Wed, Feb 16, 2022 at 12:02:31AM -0800, Shankar, Uma wrote:
> 
> 
> > -----Original Message-----
> > From: C, Ramalingam <ramalingam.c@intel.com>
> > Sent: Tuesday, February 15, 2022 11:22 AM
> > To: intel-gfx <intel-gfx@lists.freedesktop.org>; dri-devel <dri-
> > devel@lists.freedesktop.org>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>; Shankar, Uma
> > <uma.shankar@intel.com>; Roper, Matthew D <matthew.d.roper@intel.com>;
> > Dhanavanthri, Swathi <swathi.dhanavanthri@intel.com>; De Marchi, Lucas
> > <lucas.demarchi@intel.com>; Souza, Jose <jose.souza@intel.com>; C, Ramalingam
> > <ramalingam.c@intel.com>
> > Subject: [PATCH 1/3] drm/i915/dg2: Enable 5th display
> > 
> > From: Matt Roper <matthew.d.roper@intel.com>
> > 
> > DG2 supports a 5th display output which the hardware refers to as "TC1,"
> > even though it isn't a Type-C output.  This behaves similarly to the TC1 on past
> > platforms with just a couple minor differences:
> > 
> >  * DG2's TC1 bit in SDEISR is at bit 25 rather than 24 as it is on
> >    ICP/TGP/ADP.
> >  * DG2 doesn't need the hpd inversion setting that we had to use on DG1
> > 
> > Cc: Swathi Dhanavanthri <swathi.dhanavanthri@intel.com>
> > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > Cc: José Roberto de Souza <jose.souza@intel.com>
> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_gmbus.c | 16 ++++++++++++++--
> >  drivers/gpu/drm/i915/i915_irq.c            |  5 ++++-
> >  drivers/gpu/drm/i915/i915_reg.h            |  1 +
> >  3 files changed, 19 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_gmbus.c
> > b/drivers/gpu/drm/i915/display/intel_gmbus.c
> > index 6ce8c10fe975..2fad03250661 100644
> > --- a/drivers/gpu/drm/i915/display/intel_gmbus.c
> > +++ b/drivers/gpu/drm/i915/display/intel_gmbus.c
> > @@ -98,11 +98,21 @@ static const struct gmbus_pin gmbus_pins_dg1[] = {
> >  	[GMBUS_PIN_4_CNP] = { "dpd", GPIOE },
> >  };
> > 
> > +static const struct gmbus_pin gmbus_pins_dg2[] = {
> > +	[GMBUS_PIN_1_BXT] = { "dpa", GPIOB },
> > +	[GMBUS_PIN_2_BXT] = { "dpb", GPIOC },
> > +	[GMBUS_PIN_3_BXT] = { "dpc", GPIOD },
> > +	[GMBUS_PIN_4_CNP] = { "dpd", GPIOE },
> > +	[GMBUS_PIN_9_TC1_ICP] = { "tc1", GPIOJ }, };
> > +
> >  /* pin is expected to be valid */
> >  static const struct gmbus_pin *get_gmbus_pin(struct drm_i915_private *dev_priv,
> >  					     unsigned int pin)
> >  {
> > -	if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1)
> > +	if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG2)
> > +		return &gmbus_pins_dg2[pin];
> > +	else if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1)
> >  		return &gmbus_pins_dg1[pin];
> >  	else if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP)
> >  		return &gmbus_pins_icp[pin];
> > @@ -123,7 +133,9 @@ bool intel_gmbus_is_valid_pin(struct drm_i915_private
> > *dev_priv,  {
> >  	unsigned int size;
> > 
> > -	if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1)
> > +	if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG2)
> > +		size = ARRAY_SIZE(gmbus_pins_dg2);
> > +	else if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1)
> >  		size = ARRAY_SIZE(gmbus_pins_dg1);
> >  	else if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP)
> >  		size = ARRAY_SIZE(gmbus_pins_icp);
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index
> > fdd568ba4a16..4d81063b128c 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -179,6 +179,7 @@ static const u32 hpd_sde_dg1[HPD_NUM_PINS] = {
> >  	[HPD_PORT_B] = SDE_DDI_HOTPLUG_ICP(HPD_PORT_B),
> >  	[HPD_PORT_C] = SDE_DDI_HOTPLUG_ICP(HPD_PORT_C),
> >  	[HPD_PORT_D] = SDE_DDI_HOTPLUG_ICP(HPD_PORT_D),
> > +	[HPD_PORT_TC1] = SDE_TC_HOTPLUG_DG2(HPD_PORT_TC1),
> 
> Not sure if this applies to DG1, the 5th TC1 port is only for DG2.  Should we not have a separate
> DG2 struct. Can you please clarify to help understand.

It's fine to add this to the DG1 structure since it's just a mapping
table.  DG1 will never use TC1 as an input into the table, but there
aren't any conflicting mappings between DG1/DG2 so we can use the same
table for both.


Matt

> 
> Regards,
> Uma Shankar
> 
> >  };
> > 
> >  static void intel_hpd_init_pins(struct drm_i915_private *dev_priv) @@ -4424,7
> > +4425,9 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
> >  		if (I915_HAS_HOTPLUG(dev_priv))
> >  			dev_priv->hotplug_funcs = &i915_hpd_funcs;
> >  	} else {
> > -		if (HAS_PCH_DG1(dev_priv))
> > +		if (HAS_PCH_DG2(dev_priv))
> > +			dev_priv->hotplug_funcs = &icp_hpd_funcs;
> > +		else if (HAS_PCH_DG1(dev_priv))
> >  			dev_priv->hotplug_funcs = &dg1_hpd_funcs;
> >  		else if (DISPLAY_VER(dev_priv) >= 11)
> >  			dev_priv->hotplug_funcs = &gen11_hpd_funcs; diff --git
> > a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index
> > 4ea1713e6b60..4d12abb2d7ff 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -6182,6 +6182,7 @@
> >  /* south display engine interrupt: ICP/TGP */
> >  #define SDE_GMBUS_ICP			(1 << 23)
> >  #define SDE_TC_HOTPLUG_ICP(hpd_pin)	REG_BIT(24 +
> > _HPD_PIN_TC(hpd_pin))
> > +#define SDE_TC_HOTPLUG_DG2(hpd_pin)	REG_BIT(25 +
> > _HPD_PIN_TC(hpd_pin)) /* sigh */
> >  #define SDE_DDI_HOTPLUG_ICP(hpd_pin)	REG_BIT(16 +
> > _HPD_PIN_DDI(hpd_pin))
> >  #define SDE_DDI_HOTPLUG_MASK_ICP	(SDE_DDI_HOTPLUG_ICP(HPD_PORT_D) | \
> >  					 SDE_DDI_HOTPLUG_ICP(HPD_PORT_C) | \
> > --
> > 2.20.1
>
Matt Roper Feb. 17, 2022, 4:37 p.m. UTC | #3 
Since it apparently caused some confusion on various websites, maybe we
should change the title of the patch to "Enable 5th port" to make it
more clear that this is only a port, not a pipe.

Also, I believe one last line that we need to add to this patch is an
intel_ddi_init() call for TC1 in the intel_setup_outputs() function.  I
think I previously had that in a separate patch, but it went missing
when we reorganized and refactored some of these patches


Matt

On Tue, Feb 15, 2022 at 11:21:52AM +0530, Ramalingam C wrote:
> From: Matt Roper <matthew.d.roper@intel.com>
> 
> DG2 supports a 5th display output which the hardware refers to as "TC1,"
> even though it isn't a Type-C output.  This behaves similarly to the TC1
> on past platforms with just a couple minor differences:
> 
>  * DG2's TC1 bit in SDEISR is at bit 25 rather than 24 as it is on
>    ICP/TGP/ADP.
>  * DG2 doesn't need the hpd inversion setting that we had to use on DG1
> 
> Cc: Swathi Dhanavanthri <swathi.dhanavanthri@intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_gmbus.c | 16 ++++++++++++++--
>  drivers/gpu/drm/i915/i915_irq.c            |  5 ++++-
>  drivers/gpu/drm/i915/i915_reg.h            |  1 +
>  3 files changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_gmbus.c b/drivers/gpu/drm/i915/display/intel_gmbus.c
> index 6ce8c10fe975..2fad03250661 100644
> --- a/drivers/gpu/drm/i915/display/intel_gmbus.c
> +++ b/drivers/gpu/drm/i915/display/intel_gmbus.c
> @@ -98,11 +98,21 @@ static const struct gmbus_pin gmbus_pins_dg1[] = {
>  	[GMBUS_PIN_4_CNP] = { "dpd", GPIOE },
>  };
>  
> +static const struct gmbus_pin gmbus_pins_dg2[] = {
> +	[GMBUS_PIN_1_BXT] = { "dpa", GPIOB },
> +	[GMBUS_PIN_2_BXT] = { "dpb", GPIOC },
> +	[GMBUS_PIN_3_BXT] = { "dpc", GPIOD },
> +	[GMBUS_PIN_4_CNP] = { "dpd", GPIOE },
> +	[GMBUS_PIN_9_TC1_ICP] = { "tc1", GPIOJ },
> +};
> +
>  /* pin is expected to be valid */
>  static const struct gmbus_pin *get_gmbus_pin(struct drm_i915_private *dev_priv,
>  					     unsigned int pin)
>  {
> -	if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1)
> +	if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG2)
> +		return &gmbus_pins_dg2[pin];
> +	else if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1)
>  		return &gmbus_pins_dg1[pin];
>  	else if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP)
>  		return &gmbus_pins_icp[pin];
> @@ -123,7 +133,9 @@ bool intel_gmbus_is_valid_pin(struct drm_i915_private *dev_priv,
>  {
>  	unsigned int size;
>  
> -	if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1)
> +	if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG2)
> +		size = ARRAY_SIZE(gmbus_pins_dg2);
> +	else if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1)
>  		size = ARRAY_SIZE(gmbus_pins_dg1);
>  	else if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP)
>  		size = ARRAY_SIZE(gmbus_pins_icp);
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index fdd568ba4a16..4d81063b128c 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -179,6 +179,7 @@ static const u32 hpd_sde_dg1[HPD_NUM_PINS] = {
>  	[HPD_PORT_B] = SDE_DDI_HOTPLUG_ICP(HPD_PORT_B),
>  	[HPD_PORT_C] = SDE_DDI_HOTPLUG_ICP(HPD_PORT_C),
>  	[HPD_PORT_D] = SDE_DDI_HOTPLUG_ICP(HPD_PORT_D),
> +	[HPD_PORT_TC1] = SDE_TC_HOTPLUG_DG2(HPD_PORT_TC1),
>  };
>  
>  static void intel_hpd_init_pins(struct drm_i915_private *dev_priv)
> @@ -4424,7 +4425,9 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
>  		if (I915_HAS_HOTPLUG(dev_priv))
>  			dev_priv->hotplug_funcs = &i915_hpd_funcs;
>  	} else {
> -		if (HAS_PCH_DG1(dev_priv))
> +		if (HAS_PCH_DG2(dev_priv))
> +			dev_priv->hotplug_funcs = &icp_hpd_funcs;
> +		else if (HAS_PCH_DG1(dev_priv))
>  			dev_priv->hotplug_funcs = &dg1_hpd_funcs;
>  		else if (DISPLAY_VER(dev_priv) >= 11)
>  			dev_priv->hotplug_funcs = &gen11_hpd_funcs;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 4ea1713e6b60..4d12abb2d7ff 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6182,6 +6182,7 @@
>  /* south display engine interrupt: ICP/TGP */
>  #define SDE_GMBUS_ICP			(1 << 23)
>  #define SDE_TC_HOTPLUG_ICP(hpd_pin)	REG_BIT(24 + _HPD_PIN_TC(hpd_pin))
> +#define SDE_TC_HOTPLUG_DG2(hpd_pin)	REG_BIT(25 + _HPD_PIN_TC(hpd_pin)) /* sigh */
>  #define SDE_DDI_HOTPLUG_ICP(hpd_pin)	REG_BIT(16 + _HPD_PIN_DDI(hpd_pin))
>  #define SDE_DDI_HOTPLUG_MASK_ICP	(SDE_DDI_HOTPLUG_ICP(HPD_PORT_D) | \
>  					 SDE_DDI_HOTPLUG_ICP(HPD_PORT_C) | \
> -- 
> 2.20.1
>
Ramalingam C Feb. 17, 2022, 5:46 p.m. UTC | #4 
On 2022-02-17 at 08:37:47 -0800, Matt Roper wrote:
> Since it apparently caused some confusion on various websites, maybe we
> should change the title of the patch to "Enable 5th port" to make it
> more clear that this is only a port, not a pipe.
Ok sure.

> 
> Also, I believe one last line that we need to add to this patch is an
> intel_ddi_init() call for TC1 in the intel_setup_outputs() function.  I
> think I previously had that in a separate patch, but it went missing
> when we reorganized and refactored some of these patches

like 

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 59961621fe4a..18531ffd4789 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -8760,6 +8760,7 @@ static void intel_setup_outputs(struct drm_i915_private *dev_priv)
                intel_ddi_init(dev_priv, PORT_B);
                intel_ddi_init(dev_priv, PORT_C);
                intel_ddi_init(dev_priv, PORT_D_XELPD);
+               intel_ddi_init(dev_priv, PORT_TC1);
        } else if (IS_ALDERLAKE_P(dev_priv)) {
                intel_ddi_init(dev_priv, PORT_A);
                intel_ddi_init(dev_priv, PORT_B);

Ram
> 
> 
> Matt
> 
> On Tue, Feb 15, 2022 at 11:21:52AM +0530, Ramalingam C wrote:
> > From: Matt Roper <matthew.d.roper@intel.com>
> > 
> > DG2 supports a 5th display output which the hardware refers to as "TC1,"
> > even though it isn't a Type-C output.  This behaves similarly to the TC1
> > on past platforms with just a couple minor differences:
> > 
> >  * DG2's TC1 bit in SDEISR is at bit 25 rather than 24 as it is on
> >    ICP/TGP/ADP.
> >  * DG2 doesn't need the hpd inversion setting that we had to use on DG1
> > 
> > Cc: Swathi Dhanavanthri <swathi.dhanavanthri@intel.com>
> > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > Cc: José Roberto de Souza <jose.souza@intel.com>
> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_gmbus.c | 16 ++++++++++++++--
> >  drivers/gpu/drm/i915/i915_irq.c            |  5 ++++-
> >  drivers/gpu/drm/i915/i915_reg.h            |  1 +
> >  3 files changed, 19 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_gmbus.c b/drivers/gpu/drm/i915/display/intel_gmbus.c
> > index 6ce8c10fe975..2fad03250661 100644
> > --- a/drivers/gpu/drm/i915/display/intel_gmbus.c
> > +++ b/drivers/gpu/drm/i915/display/intel_gmbus.c
> > @@ -98,11 +98,21 @@ static const struct gmbus_pin gmbus_pins_dg1[] = {
> >  	[GMBUS_PIN_4_CNP] = { "dpd", GPIOE },
> >  };
> >  
> > +static const struct gmbus_pin gmbus_pins_dg2[] = {
> > +	[GMBUS_PIN_1_BXT] = { "dpa", GPIOB },
> > +	[GMBUS_PIN_2_BXT] = { "dpb", GPIOC },
> > +	[GMBUS_PIN_3_BXT] = { "dpc", GPIOD },
> > +	[GMBUS_PIN_4_CNP] = { "dpd", GPIOE },
> > +	[GMBUS_PIN_9_TC1_ICP] = { "tc1", GPIOJ },
> > +};
> > +
> >  /* pin is expected to be valid */
> >  static const struct gmbus_pin *get_gmbus_pin(struct drm_i915_private *dev_priv,
> >  					     unsigned int pin)
> >  {
> > -	if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1)
> > +	if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG2)
> > +		return &gmbus_pins_dg2[pin];
> > +	else if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1)
> >  		return &gmbus_pins_dg1[pin];
> >  	else if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP)
> >  		return &gmbus_pins_icp[pin];
> > @@ -123,7 +133,9 @@ bool intel_gmbus_is_valid_pin(struct drm_i915_private *dev_priv,
> >  {
> >  	unsigned int size;
> >  
> > -	if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1)
> > +	if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG2)
> > +		size = ARRAY_SIZE(gmbus_pins_dg2);
> > +	else if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1)
> >  		size = ARRAY_SIZE(gmbus_pins_dg1);
> >  	else if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP)
> >  		size = ARRAY_SIZE(gmbus_pins_icp);
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index fdd568ba4a16..4d81063b128c 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -179,6 +179,7 @@ static const u32 hpd_sde_dg1[HPD_NUM_PINS] = {
> >  	[HPD_PORT_B] = SDE_DDI_HOTPLUG_ICP(HPD_PORT_B),
> >  	[HPD_PORT_C] = SDE_DDI_HOTPLUG_ICP(HPD_PORT_C),
> >  	[HPD_PORT_D] = SDE_DDI_HOTPLUG_ICP(HPD_PORT_D),
> > +	[HPD_PORT_TC1] = SDE_TC_HOTPLUG_DG2(HPD_PORT_TC1),
> >  };
> >  
> >  static void intel_hpd_init_pins(struct drm_i915_private *dev_priv)
> > @@ -4424,7 +4425,9 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
> >  		if (I915_HAS_HOTPLUG(dev_priv))
> >  			dev_priv->hotplug_funcs = &i915_hpd_funcs;
> >  	} else {
> > -		if (HAS_PCH_DG1(dev_priv))
> > +		if (HAS_PCH_DG2(dev_priv))
> > +			dev_priv->hotplug_funcs = &icp_hpd_funcs;
> > +		else if (HAS_PCH_DG1(dev_priv))
> >  			dev_priv->hotplug_funcs = &dg1_hpd_funcs;
> >  		else if (DISPLAY_VER(dev_priv) >= 11)
> >  			dev_priv->hotplug_funcs = &gen11_hpd_funcs;
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 4ea1713e6b60..4d12abb2d7ff 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -6182,6 +6182,7 @@
> >  /* south display engine interrupt: ICP/TGP */
> >  #define SDE_GMBUS_ICP			(1 << 23)
> >  #define SDE_TC_HOTPLUG_ICP(hpd_pin)	REG_BIT(24 + _HPD_PIN_TC(hpd_pin))
> > +#define SDE_TC_HOTPLUG_DG2(hpd_pin)	REG_BIT(25 + _HPD_PIN_TC(hpd_pin)) /* sigh */
> >  #define SDE_DDI_HOTPLUG_ICP(hpd_pin)	REG_BIT(16 + _HPD_PIN_DDI(hpd_pin))
> >  #define SDE_DDI_HOTPLUG_MASK_ICP	(SDE_DDI_HOTPLUG_ICP(HPD_PORT_D) | \
> >  					 SDE_DDI_HOTPLUG_ICP(HPD_PORT_C) | \
> > -- 
> > 2.20.1
> > 
> 
> -- 
> Matt Roper
> Graphics Software Engineer
> VTT-OSGC Platform Enablement
> Intel Corporation
> (916) 356-2795
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_gmbus.c b/drivers/gpu/drm/i915/display/intel_gmbus.c
index 6ce8c10fe975..2fad03250661 100644
--- a/drivers/gpu/drm/i915/display/intel_gmbus.c
+++ b/drivers/gpu/drm/i915/display/intel_gmbus.c
@@ -98,11 +98,21 @@  static const struct gmbus_pin gmbus_pins_dg1[] = {
 	[GMBUS_PIN_4_CNP] = { "dpd", GPIOE },
 };
 
+static const struct gmbus_pin gmbus_pins_dg2[] = {
+	[GMBUS_PIN_1_BXT] = { "dpa", GPIOB },
+	[GMBUS_PIN_2_BXT] = { "dpb", GPIOC },
+	[GMBUS_PIN_3_BXT] = { "dpc", GPIOD },
+	[GMBUS_PIN_4_CNP] = { "dpd", GPIOE },
+	[GMBUS_PIN_9_TC1_ICP] = { "tc1", GPIOJ },
+};
+
 /* pin is expected to be valid */
 static const struct gmbus_pin *get_gmbus_pin(struct drm_i915_private *dev_priv,
 					     unsigned int pin)
 {
-	if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1)
+	if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG2)
+		return &gmbus_pins_dg2[pin];
+	else if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1)
 		return &gmbus_pins_dg1[pin];
 	else if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP)
 		return &gmbus_pins_icp[pin];
@@ -123,7 +133,9 @@  bool intel_gmbus_is_valid_pin(struct drm_i915_private *dev_priv,
 {
 	unsigned int size;
 
-	if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1)
+	if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG2)
+		size = ARRAY_SIZE(gmbus_pins_dg2);
+	else if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1)
 		size = ARRAY_SIZE(gmbus_pins_dg1);
 	else if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP)
 		size = ARRAY_SIZE(gmbus_pins_icp);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index fdd568ba4a16..4d81063b128c 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -179,6 +179,7 @@  static const u32 hpd_sde_dg1[HPD_NUM_PINS] = {
 	[HPD_PORT_B] = SDE_DDI_HOTPLUG_ICP(HPD_PORT_B),
 	[HPD_PORT_C] = SDE_DDI_HOTPLUG_ICP(HPD_PORT_C),
 	[HPD_PORT_D] = SDE_DDI_HOTPLUG_ICP(HPD_PORT_D),
+	[HPD_PORT_TC1] = SDE_TC_HOTPLUG_DG2(HPD_PORT_TC1),
 };
 
 static void intel_hpd_init_pins(struct drm_i915_private *dev_priv)
@@ -4424,7 +4425,9 @@  void intel_irq_init(struct drm_i915_private *dev_priv)
 		if (I915_HAS_HOTPLUG(dev_priv))
 			dev_priv->hotplug_funcs = &i915_hpd_funcs;
 	} else {
-		if (HAS_PCH_DG1(dev_priv))
+		if (HAS_PCH_DG2(dev_priv))
+			dev_priv->hotplug_funcs = &icp_hpd_funcs;
+		else if (HAS_PCH_DG1(dev_priv))
 			dev_priv->hotplug_funcs = &dg1_hpd_funcs;
 		else if (DISPLAY_VER(dev_priv) >= 11)
 			dev_priv->hotplug_funcs = &gen11_hpd_funcs;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 4ea1713e6b60..4d12abb2d7ff 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6182,6 +6182,7 @@ 
 /* south display engine interrupt: ICP/TGP */
 #define SDE_GMBUS_ICP			(1 << 23)
 #define SDE_TC_HOTPLUG_ICP(hpd_pin)	REG_BIT(24 + _HPD_PIN_TC(hpd_pin))
+#define SDE_TC_HOTPLUG_DG2(hpd_pin)	REG_BIT(25 + _HPD_PIN_TC(hpd_pin)) /* sigh */
 #define SDE_DDI_HOTPLUG_ICP(hpd_pin)	REG_BIT(16 + _HPD_PIN_DDI(hpd_pin))
 #define SDE_DDI_HOTPLUG_MASK_ICP	(SDE_DDI_HOTPLUG_ICP(HPD_PORT_D) | \
 					 SDE_DDI_HOTPLUG_ICP(HPD_PORT_C) | \