[Intel-gfx] [PATCH V2] drm/i915/cml : Add TGP PCH support

[Matt Roper]

On Mon, Dec 28, 2020 at 11:42:35AM +0530, Tejas Upadhyay wrote:
> We have TGP PCH support for Tigerlake and Rocketlake. Similarly
> now TGP PCH can be used with Cometlake CPU.

Based on the 'compatibility' section of bspec 49181, I think the TGP PCH
can technically be compatible with any gen9bc platform, not just CML.
Although it seems unlikely that anyone is going to go back and create
new products with a SKL+TGP pairing or something at this point, it's
still probably best to write this patch based on GEN9_BC rather than
CML.

> 
> Changes since V1 :
> 	- Matched HPD Pin mapping for PORT C and PORT D of CML CPU.
> 
> Cc : Matt Roper <matthew.d.roper at intel.com>
> Cc : Ville Syrjälä <ville.syrjala at linux.intel.com> 
> Signed-off-by: Tejas Upadhyay <tejaskumarx.surendrakumar.upadhyay at intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c     | 7 +++++--
>  drivers/gpu/drm/i915/display/intel_display.c | 5 +++++
>  drivers/gpu/drm/i915/display/intel_hdmi.c    | 3 ++-
>  3 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 17eaa56c5a99..181d60a5e145 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -5301,7 +5301,9 @@ static enum hpd_pin dg1_hpd_pin(struct drm_i915_private *dev_priv,
>  static enum hpd_pin tgl_hpd_pin(struct drm_i915_private *dev_priv,
>  				enum port port)
>  {
> -	if (port >= PORT_TC1)
> +	if (IS_COMETLAKE(dev_priv) && port >= PORT_C)
> +		return HPD_PORT_TC1 + port + 1 - PORT_TC1;
> +	else if (port >= PORT_TC1)
>  		return HPD_PORT_TC1 + port - PORT_TC1;
>  	else
>  		return HPD_PORT_A + port - PORT_A;
> @@ -5455,7 +5457,8 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>  
>  	if (IS_DG1(dev_priv))
>  		encoder->hpd_pin = dg1_hpd_pin(dev_priv, port);
> -	else if (IS_ROCKETLAKE(dev_priv))
> +	else if (IS_ROCKETLAKE(dev_priv) || (IS_COMETLAKE(dev_priv) &&
> +					     HAS_PCH_TGP(dev_priv)))
>  		encoder->hpd_pin = rkl_hpd_pin(dev_priv, port);
>  	else if (INTEL_GEN(dev_priv) >= 12)

I'd suggest leaving the RKL condition alone since nothing here has
anything to do with RKL.  Instead change the gen12+ condition to
HAS_PCH_TGP() and update the TGP-specific handler to do the port mapping
described on bspec 49181.

Plus I don't think what you have here would map the ports correctly
anyway.  gen9 PORT_C/PORT_D would map to HPD_PORT_C/HPD_PORT_TC1 with
the logic here, whereas the bspec says they should map to
HPD_PORT_TC1/HPD_PORT_TC2.

>  		encoder->hpd_pin = tgl_hpd_pin(dev_priv, port);
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index f2c48e5cdb43..47014471658f 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -16163,6 +16163,11 @@ static void intel_setup_outputs(struct drm_i915_private *dev_priv)
>  			intel_ddi_init(dev_priv, PORT_F);
>  
>  		icl_dsi_init(dev_priv);
> +	} else if (IS_COMETLAKE(dev_priv) && HAS_PCH_TGP(dev_priv)) {
> +		intel_ddi_init(dev_priv, PORT_A);
> +		intel_ddi_init(dev_priv, PORT_B);
> +		intel_ddi_init(dev_priv, PORT_C);
> +		intel_ddi_init(dev_priv, PORT_D);

As noted before, this relates to gen9bc in general, not just CML.

Is the only reason for this block because TGP's instance of SFUSE_STRAP
doesn't have output presence bits anymore?  If you want, you could keep
using the existing gen9bc block for consistency, but make the
SFUSE_STRAP checks themselves conditional on a platform that has the
presence bits.  E.g.,

    /* ICP+ no longer has port presence bits */
    found = INTEL_PCH_TYPE(dev_priv) >= PCH_ICP ?
        ~0 : intel_de_read(dev_priv, SFUSE_STRAP);

>  	} else if (IS_GEN9_LP(dev_priv)) {
>  		/*
>  		 * FIXME: Broxton doesn't support port detection via the
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index c5959590562b..540c9d54b595 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -3174,7 +3174,8 @@ static u8 intel_hdmi_ddc_pin(struct intel_encoder *encoder)
>  
>  	if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1)
>  		ddc_pin = dg1_port_to_ddc_pin(dev_priv, port);
> -	else if (IS_ROCKETLAKE(dev_priv))
> +	else if (IS_ROCKETLAKE(dev_priv) || (IS_COMETLAKE(dev_priv) &&
> +					     HAS_PCH_TGP(dev_priv)))
>  		ddc_pin = rkl_port_to_ddc_pin(dev_priv, port);

As above, none of the changes in this patch have any relation to RKL, so
it doesn't make sense to update the RKL condition.  Instead just add the
gen9bc port mapping logic to icl_port_to_ddc_pin().

Plus, it looks like what you have here right now will make the same
mapping mistake for C and D that the HPD logic did.


Matt

>  	else if (HAS_PCH_MCC(dev_priv))
>  		ddc_pin = mcc_port_to_ddc_pin(dev_priv, port);
> -- 
> 2.28.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795