Re: [PATCH 02/11] drm/i915/cnl: Add Port F definition.

[Paulo Zanoni <paulo.r.zanoni@intel.com>]

Em Sex, 2017-12-22 às 15:18 -0800, Rodrigo Vivi escreveu:
> Some Cannonlake SKUs will come with a full split between
> port A and port E. This will be called port F although it
> is not a 6th port, but only a split.

I think that a small comment explaining that port F support is still
not complete even with this patch would help.

One more comment below.

> 
> v2: Fix size of dvo_ports found by Ander.
> v3: Adding missing cases from intel_bios.c for Port_F
> v4: Adding other missing cases and fix the commit message.
> v5: Rebase on top of display headers rework.
> 
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_bios.c     | 9 +++++++++
>  drivers/gpu/drm/i915/intel_display.h  | 1 +
>  drivers/gpu/drm/i915/intel_dp.c       | 2 ++
>  drivers/gpu/drm/i915/intel_vbt_defs.h | 2 ++
>  include/drm/i915_component.h          | 3 +--
>  5 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_bios.c
> b/drivers/gpu/drm/i915/intel_bios.c
> index 51108ffc28d1..59a150e2adce 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -1140,6 +1140,7 @@ static void parse_ddi_port(struct
> drm_i915_private *dev_priv, enum port port,
>  		{DVO_PORT_HDMIC, DVO_PORT_DPC, -1},
>  		{DVO_PORT_HDMID, DVO_PORT_DPD, -1},
>  		{DVO_PORT_CRT, DVO_PORT_HDMIE, DVO_PORT_DPE},
> +		{DVO_PORT_HDMIF, DVO_PORT_DPF, -1},
>  	};
>  
>  	/*
> @@ -1688,6 +1689,7 @@ bool intel_bios_is_port_present(struct
> drm_i915_private *dev_priv, enum port por
>  		[PORT_C] = { DVO_PORT_DPC, DVO_PORT_HDMIC, },
>  		[PORT_D] = { DVO_PORT_DPD, DVO_PORT_HDMID, },
>  		[PORT_E] = { DVO_PORT_DPE, DVO_PORT_HDMIE, },
> +		[PORT_F] = { DVO_PORT_DPF, DVO_PORT_HDMIF, },
>  	};
>  	int i;
>  
> @@ -1726,6 +1728,7 @@ bool intel_bios_is_port_edp(struct
> drm_i915_private *dev_priv, enum port port)
>  		[PORT_C] = DVO_PORT_DPC,
>  		[PORT_D] = DVO_PORT_DPD,
>  		[PORT_E] = DVO_PORT_DPE,
> +		[PORT_F] = DVO_PORT_DPF,
>  	};
>  	int i;
>  
> @@ -1761,6 +1764,7 @@ static bool child_dev_is_dp_dual_mode(const
> struct child_device_config *child,
>  		[PORT_C] = { DVO_PORT_DPC, DVO_PORT_HDMIC, },
>  		[PORT_D] = { DVO_PORT_DPD, DVO_PORT_HDMID, },
>  		[PORT_E] = { DVO_PORT_DPE, DVO_PORT_HDMIE, },
> +		[PORT_F] = { DVO_PORT_DPF, DVO_PORT_HDMIF, },
>  	};
>  
>  	if (port == PORT_A || port >= ARRAY_SIZE(port_mapping))
> @@ -1927,6 +1931,11 @@ intel_bios_is_lspcon_present(struct
> drm_i915_private *dev_priv,
>  			if (port == PORT_D)
>  				return true;
>  			break;
> +		case DVO_PORT_DPF:
> +		case DVO_PORT_HDMIF:
> +			if (port == PORT_F)
> +				return true;
> +			break;
>  		default:
>  			break;
>  		}
> diff --git a/drivers/gpu/drm/i915/intel_display.h
> b/drivers/gpu/drm/i915/intel_display.h
> index a0d2b6169361..e47638931b51 100644
> --- a/drivers/gpu/drm/i915/intel_display.h
> +++ b/drivers/gpu/drm/i915/intel_display.h
> @@ -119,6 +119,7 @@ enum port {
>  	PORT_C,
>  	PORT_D,
>  	PORT_E,
> +	PORT_F,
>  
>  	I915_MAX_PORTS
>  };
> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> b/drivers/gpu/drm/i915/intel_dp.c
> index 35c5299feab6..71721de39e6b 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1378,6 +1378,7 @@ static i915_reg_t skl_aux_ctl_reg(struct
> drm_i915_private *dev_priv,
>  	case PORT_B:
>  	case PORT_C:
>  	case PORT_D:
> +	case PORT_F:
>  		return DP_AUX_CH_CTL(port);
>  	default:
>  		MISSING_CASE(port);
> @@ -1393,6 +1394,7 @@ static i915_reg_t skl_aux_data_reg(struct
> drm_i915_private *dev_priv,
>  	case PORT_B:
>  	case PORT_C:
>  	case PORT_D:
> +	case PORT_F:
>  		return DP_AUX_CH_DATA(port, index);
>  	default:
>  		MISSING_CASE(port);
> diff --git a/drivers/gpu/drm/i915/intel_vbt_defs.h
> b/drivers/gpu/drm/i915/intel_vbt_defs.h
> index e3d7745a9151..8713e8295957 100644
> --- a/drivers/gpu/drm/i915/intel_vbt_defs.h
> +++ b/drivers/gpu/drm/i915/intel_vbt_defs.h
> @@ -299,6 +299,8 @@ struct bdb_general_features {
>  #define DVO_PORT_DPA		10
>  #define DVO_PORT_DPE		11				
> /* 193 */
>  #define DVO_PORT_HDMIE		12				
> /* 193 */
> +#define DVO_PORT_DPF	13
> +#define DVO_PORT_HDMIF	14

Needs ones more tab before the number. Besides, I suppose we could "/*
N/A */" or /* Missing from spec */ or something that clarifies that
it's not that our code is missing the version, it's the spec that
doesn't have it.

With those two things addressed:
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

>  #define DVO_PORT_MIPIA		21				
> /* 171 */
>  #define DVO_PORT_MIPIB		22				
> /* 171 */
>  #define DVO_PORT_MIPIC		23				
> /* 171 */
> diff --git a/include/drm/i915_component.h
> b/include/drm/i915_component.h
> index 545c6e0fea7d..346b1f5cb180 100644
> --- a/include/drm/i915_component.h
> +++ b/include/drm/i915_component.h
> @@ -26,9 +26,8 @@
>  
>  /* MAX_PORT is the number of port
>   * It must be sync with I915_MAX_PORTS defined i915_drv.h
> - * 5 should be enough as only HSW, BDW, SKL need such fix.
>   */
> -#define MAX_PORTS 5
> +#define MAX_PORTS 6
>  
>  /**
>   * struct i915_audio_component_ops - Ops implemented by i915 driver,
> called by hda driver
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx