Patch series covers following things:
1. Split and differentiate between EhlkhartLake and
JasperLake platfrom
2. Update voltage swing table for eDP on JasperLake platform
BSpec: 21257
Tejas Upadhyay (2):
drm/i915/jsl: Split EHL/JSL platform info and PCI ids
drm/i915/edp/jsl: Update vswing table for HBR and HBR2
drivers/gpu/drm/i915/display/intel_ddi.c | 67 ++++++++++++++++++++++--
drivers/gpu/drm/i915/i915_drv.h | 4 +-
drivers/gpu/drm/i915/i915_pci.c | 9 ++++
drivers/gpu/drm/i915/intel_device_info.c | 1 +
drivers/gpu/drm/i915/intel_device_info.h | 1 +
include/drm/i915_pciids.h | 9 ++--
6 files changed, 84 insertions(+), 7 deletions(-)
--
2.25.1
Split the basic platform definition, macros, and PCI IDs to
differentiate between EHL and JSL platforms.
Signed-off-by: Tejas Upadhyay <[email protected]>
---
drivers/gpu/drm/i915/i915_drv.h | 4 +++-
drivers/gpu/drm/i915/i915_pci.c | 9 +++++++++
drivers/gpu/drm/i915/intel_device_info.c | 1 +
drivers/gpu/drm/i915/intel_device_info.h | 1 +
include/drm/i915_pciids.h | 9 ++++++---
5 files changed, 20 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 72a9449b674e..4f20acebb038 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1417,7 +1417,9 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
#define IS_COMETLAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_COMETLAKE)
#define IS_CANNONLAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_CANNONLAKE)
#define IS_ICELAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_ICELAKE)
-#define IS_ELKHARTLAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_ELKHARTLAKE)
+#define IS_ELKHARTLAKE(dev_priv) (IS_PLATFORM(dev_priv, INTEL_ELKHARTLAKE) || \
+ IS_PLATFORM(dev_priv, INTEL_JASPERLAKE))
+#define IS_JASPERLAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_JASPERLAKE)
#define IS_TIGERLAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_TIGERLAKE)
#define IS_ROCKETLAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_ROCKETLAKE)
#define IS_DG1(dev_priv) IS_PLATFORM(dev_priv, INTEL_DG1)
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 366ddfc8df6b..8690b69fcf33 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -846,6 +846,14 @@ static const struct intel_device_info ehl_info = {
.ppgtt_size = 36,
};
+static const struct intel_device_info jsl_info = {
+ GEN11_FEATURES,
+ PLATFORM(INTEL_JASPERLAKE),
+ .require_force_probe = 1,
+ .platform_engine_mask = BIT(RCS0) | BIT(BCS0) | BIT(VCS0) | BIT(VECS0),
+ .ppgtt_size = 36,
+};
+
#define GEN12_FEATURES \
GEN11_FEATURES, \
GEN(12), \
@@ -985,6 +993,7 @@ static const struct pci_device_id pciidlist[] = {
INTEL_CNL_IDS(&cnl_info),
INTEL_ICL_11_IDS(&icl_info),
INTEL_EHL_IDS(&ehl_info),
+ INTEL_JSL_IDS(&jsl_info),
INTEL_TGL_12_IDS(&tgl_info),
INTEL_RKL_IDS(&rkl_info),
{0, 0, 0}
diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
index adc836f15fde..e67cec8fa2aa 100644
--- a/drivers/gpu/drm/i915/intel_device_info.c
+++ b/drivers/gpu/drm/i915/intel_device_info.c
@@ -62,6 +62,7 @@ static const char * const platform_names[] = {
PLATFORM_NAME(CANNONLAKE),
PLATFORM_NAME(ICELAKE),
PLATFORM_NAME(ELKHARTLAKE),
+ PLATFORM_NAME(JASPERLAKE),
PLATFORM_NAME(TIGERLAKE),
PLATFORM_NAME(ROCKETLAKE),
PLATFORM_NAME(DG1),
diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
index 6a3d607218aa..d92fa041c700 100644
--- a/drivers/gpu/drm/i915/intel_device_info.h
+++ b/drivers/gpu/drm/i915/intel_device_info.h
@@ -79,6 +79,7 @@ enum intel_platform {
/* gen11 */
INTEL_ICELAKE,
INTEL_ELKHARTLAKE,
+ INTEL_JASPERLAKE,
/* gen12 */
INTEL_TIGERLAKE,
INTEL_ROCKETLAKE,
diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h
index 7eeecb07c9a1..1b5e09cfa11e 100644
--- a/include/drm/i915_pciids.h
+++ b/include/drm/i915_pciids.h
@@ -579,15 +579,18 @@
INTEL_VGA_DEVICE(0x8A51, info), \
INTEL_VGA_DEVICE(0x8A5D, info)
-/* EHL/JSL */
+/* EHL */
#define INTEL_EHL_IDS(info) \
INTEL_VGA_DEVICE(0x4500, info), \
INTEL_VGA_DEVICE(0x4571, info), \
INTEL_VGA_DEVICE(0x4551, info), \
INTEL_VGA_DEVICE(0x4541, info), \
- INTEL_VGA_DEVICE(0x4E71, info), \
INTEL_VGA_DEVICE(0x4557, info), \
- INTEL_VGA_DEVICE(0x4555, info), \
+ INTEL_VGA_DEVICE(0x4555, info)
+
+/* JSL */
+#define INTEL_JSL_IDS(info) \
+ INTEL_VGA_DEVICE(0x4E71, info), \
INTEL_VGA_DEVICE(0x4E61, info), \
INTEL_VGA_DEVICE(0x4E57, info), \
INTEL_VGA_DEVICE(0x4E55, info), \
--
2.25.1
JSL has update in vswing table for eDP
BSpec: 21257
Signed-off-by: Tejas Upadhyay <[email protected]>
---
drivers/gpu/drm/i915/display/intel_ddi.c | 67 ++++++++++++++++++++++--
1 file changed, 64 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 4d06178cd76c..fa08463bcf2e 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -582,6 +582,34 @@ static const struct cnl_ddi_buf_trans ehl_combo_phy_ddi_translations_dp[] = {
{ 0x6, 0x7F, 0x3F, 0x00, 0x00 }, /* 900 900 0.0 */
};
+static const struct cnl_ddi_buf_trans jsl_combo_phy_ddi_translations_edp_hbr[] = {
+ /* NT mV Trans mV db */
+ { 0x8, 0x7F, 0x3F, 0x00, 0x00 }, /* 200 200 0.0 */
+ { 0x8, 0x7F, 0x38, 0x00, 0x07 }, /* 200 250 1.9 */
+ { 0x1, 0x7F, 0x33, 0x00, 0x0C }, /* 200 300 3.5 */
+ { 0xA, 0x35, 0x36, 0x00, 0x09 }, /* 200 350 4.9 */
+ { 0x8, 0x7F, 0x3F, 0x00, 0x00 }, /* 250 250 0.0 */
+ { 0x1, 0x7F, 0x38, 0x00, 0x07 }, /* 250 300 1.6 */
+ { 0xA, 0x35, 0x35, 0x00, 0x0A }, /* 250 350 2.9 */
+ { 0x1, 0x7F, 0x3F, 0x00, 0x00 }, /* 300 300 0.0 */
+ { 0xA, 0x35, 0x38, 0x00, 0x07 }, /* 300 350 1.3 */
+ { 0xA, 0x35, 0x3F, 0x00, 0x00 }, /* 350 350 0.0 */
+};
+
+static const struct cnl_ddi_buf_trans jsl_combo_phy_ddi_translations_edp_hbr2[] = {
+ /* NT mV Trans mV db */
+ { 0x8, 0x7F, 0x3F, 0x00, 0x00 }, /* 200 200 0.0 */
+ { 0x8, 0x7F, 0x3F, 0x00, 0x00 }, /* 200 250 1.9 */
+ { 0x1, 0x7F, 0x3D, 0x00, 0x02 }, /* 200 300 3.5 */
+ { 0xA, 0x35, 0x38, 0x00, 0x07 }, /* 200 350 4.9 */
+ { 0x8, 0x7F, 0x3F, 0x00, 0x00 }, /* 250 250 0.0 */
+ { 0x1, 0x7F, 0x3F, 0x00, 0x00 }, /* 250 300 1.6 */
+ { 0xA, 0x35, 0x3A, 0x00, 0x05 }, /* 250 350 2.9 */
+ { 0x1, 0x7F, 0x3F, 0x00, 0x00 }, /* 300 300 0.0 */
+ { 0xA, 0x35, 0x38, 0x00, 0x07 }, /* 300 350 1.3 */
+ { 0xA, 0x35, 0x3F, 0x00, 0x00 }, /* 350 350 0.0 */
+};
+
struct icl_mg_phy_ddi_buf_trans {
u32 cri_txdeemph_override_11_6;
u32 cri_txdeemph_override_5_0;
@@ -1069,7 +1097,6 @@ icl_get_mg_buf_trans(struct intel_encoder *encoder, int type, int rate,
*n_entries = ARRAY_SIZE(icl_mg_phy_ddi_translations_rbr_hbr);
return icl_mg_phy_ddi_translations_rbr_hbr;
}
-
static const struct cnl_ddi_buf_trans *
ehl_get_combo_buf_trans(struct intel_encoder *encoder, int type, int rate,
int *n_entries)
@@ -1098,6 +1125,34 @@ ehl_get_combo_buf_trans(struct intel_encoder *encoder, int type, int rate,
}
}
+static const struct cnl_ddi_buf_trans *
+jsl_get_combo_buf_trans(struct intel_encoder *encoder, int type, int rate,
+ int *n_entries)
+{
+ struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+
+ switch (type) {
+ case INTEL_OUTPUT_HDMI:
+ *n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_hdmi);
+ return icl_combo_phy_ddi_translations_hdmi;
+ case INTEL_OUTPUT_EDP:
+ if (dev_priv->vbt.edp.low_vswing) {
+ if (rate > 270000) {
+ *n_entries = ARRAY_SIZE(jsl_combo_phy_ddi_translations_edp_hbr2);
+ return jsl_combo_phy_ddi_translations_edp_hbr2;
+ } else {
+ *n_entries = ARRAY_SIZE(jsl_combo_phy_ddi_translations_edp_hbr);
+ return jsl_combo_phy_ddi_translations_edp_hbr;
+ }
+ }
+ /* fall through */
+ default:
+ /* All combo DP and eDP ports that do not support low_vswing */
+ *n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_dp_hbr2);
+ return icl_combo_phy_ddi_translations_dp_hbr2;
+ }
+}
+
static const struct cnl_ddi_buf_trans *
tgl_get_combo_buf_trans(struct intel_encoder *encoder, int type, int rate,
int *n_entries)
@@ -2265,9 +2320,12 @@ static u8 intel_ddi_dp_voltage_max(struct intel_dp *intel_dp)
tgl_get_dkl_buf_trans(encoder, encoder->type,
intel_dp->link_rate, &n_entries);
} else if (INTEL_GEN(dev_priv) == 11) {
- if (IS_ELKHARTLAKE(dev_priv))
+ if (IS_JASPERLAKE(dev_priv))
+ jsl_get_combo_buf_trans(encoder, encoder->type,
+ intel_dp->link_rate, &n_entries);
+ else if (IS_ELKHARTLAKE(dev_priv))
ehl_get_combo_buf_trans(encoder, encoder->type,
- intel_dp->link_rate, &n_entries);
+ intel_dp->link_rate, &n_entries);
else if (intel_phy_is_combo(dev_priv, phy))
icl_get_combo_buf_trans(encoder, encoder->type,
intel_dp->link_rate, &n_entries);
@@ -2454,6 +2512,9 @@ static void icl_ddi_combo_vswing_program(struct intel_encoder *encoder,
if (INTEL_GEN(dev_priv) >= 12)
ddi_translations = tgl_get_combo_buf_trans(encoder, type, rate,
&n_entries);
+ else if (IS_JASPERLAKE(dev_priv))
+ ddi_translations = jsl_get_combo_buf_trans(encoder, type, rate,
+ &n_entries);
else if (IS_ELKHARTLAKE(dev_priv))
ddi_translations = ehl_get_combo_buf_trans(encoder, type, rate,
&n_entries);
--
2.25.1
On Mon, 28 Sep 2020, Tejas Upadhyay <[email protected]> wrote:
> Split the basic platform definition, macros, and PCI IDs to
> differentiate between EHL and JSL platforms.
>
> Signed-off-by: Tejas Upadhyay <[email protected]>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 4 +++-
> drivers/gpu/drm/i915/i915_pci.c | 9 +++++++++
> drivers/gpu/drm/i915/intel_device_info.c | 1 +
> drivers/gpu/drm/i915/intel_device_info.h | 1 +
> include/drm/i915_pciids.h | 9 ++++++---
> 5 files changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 72a9449b674e..4f20acebb038 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1417,7 +1417,9 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
> #define IS_COMETLAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_COMETLAKE)
> #define IS_CANNONLAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_CANNONLAKE)
> #define IS_ICELAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_ICELAKE)
> -#define IS_ELKHARTLAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_ELKHARTLAKE)
> +#define IS_ELKHARTLAKE(dev_priv) (IS_PLATFORM(dev_priv, INTEL_ELKHARTLAKE) || \
> + IS_PLATFORM(dev_priv, INTEL_JASPERLAKE))
> +#define IS_JASPERLAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_JASPERLAKE)
I think we've learned from history that we want the platform checks to
be independent. I.e. if you need to split ELK and JSP, you need to make
IS_ELKHARTLAKE() match *only* ELK, and you need to replace every current
IS_ELKHARTLAKE() check with IS_ELKHARTLAKE() || IS_JASPERLAKE().
We've been here before, and we've thought before that we can get by with
the minimal change. It's just postponing the inevitable and generates
confusion.
BR,
Jani.
> #define IS_TIGERLAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_TIGERLAKE)
> #define IS_ROCKETLAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_ROCKETLAKE)
> #define IS_DG1(dev_priv) IS_PLATFORM(dev_priv, INTEL_DG1)
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index 366ddfc8df6b..8690b69fcf33 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -846,6 +846,14 @@ static const struct intel_device_info ehl_info = {
> .ppgtt_size = 36,
> };
>
> +static const struct intel_device_info jsl_info = {
> + GEN11_FEATURES,
> + PLATFORM(INTEL_JASPERLAKE),
> + .require_force_probe = 1,
> + .platform_engine_mask = BIT(RCS0) | BIT(BCS0) | BIT(VCS0) | BIT(VECS0),
> + .ppgtt_size = 36,
> +};
> +
> #define GEN12_FEATURES \
> GEN11_FEATURES, \
> GEN(12), \
> @@ -985,6 +993,7 @@ static const struct pci_device_id pciidlist[] = {
> INTEL_CNL_IDS(&cnl_info),
> INTEL_ICL_11_IDS(&icl_info),
> INTEL_EHL_IDS(&ehl_info),
> + INTEL_JSL_IDS(&jsl_info),
> INTEL_TGL_12_IDS(&tgl_info),
> INTEL_RKL_IDS(&rkl_info),
> {0, 0, 0}
> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
> index adc836f15fde..e67cec8fa2aa 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.c
> +++ b/drivers/gpu/drm/i915/intel_device_info.c
> @@ -62,6 +62,7 @@ static const char * const platform_names[] = {
> PLATFORM_NAME(CANNONLAKE),
> PLATFORM_NAME(ICELAKE),
> PLATFORM_NAME(ELKHARTLAKE),
> + PLATFORM_NAME(JASPERLAKE),
> PLATFORM_NAME(TIGERLAKE),
> PLATFORM_NAME(ROCKETLAKE),
> PLATFORM_NAME(DG1),
> diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
> index 6a3d607218aa..d92fa041c700 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.h
> +++ b/drivers/gpu/drm/i915/intel_device_info.h
> @@ -79,6 +79,7 @@ enum intel_platform {
> /* gen11 */
> INTEL_ICELAKE,
> INTEL_ELKHARTLAKE,
> + INTEL_JASPERLAKE,
> /* gen12 */
> INTEL_TIGERLAKE,
> INTEL_ROCKETLAKE,
> diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h
> index 7eeecb07c9a1..1b5e09cfa11e 100644
> --- a/include/drm/i915_pciids.h
> +++ b/include/drm/i915_pciids.h
> @@ -579,15 +579,18 @@
> INTEL_VGA_DEVICE(0x8A51, info), \
> INTEL_VGA_DEVICE(0x8A5D, info)
>
> -/* EHL/JSL */
> +/* EHL */
> #define INTEL_EHL_IDS(info) \
> INTEL_VGA_DEVICE(0x4500, info), \
> INTEL_VGA_DEVICE(0x4571, info), \
> INTEL_VGA_DEVICE(0x4551, info), \
> INTEL_VGA_DEVICE(0x4541, info), \
> - INTEL_VGA_DEVICE(0x4E71, info), \
> INTEL_VGA_DEVICE(0x4557, info), \
> - INTEL_VGA_DEVICE(0x4555, info), \
> + INTEL_VGA_DEVICE(0x4555, info)
> +
> +/* JSL */
> +#define INTEL_JSL_IDS(info) \
> + INTEL_VGA_DEVICE(0x4E71, info), \
> INTEL_VGA_DEVICE(0x4E61, info), \
> INTEL_VGA_DEVICE(0x4E57, info), \
> INTEL_VGA_DEVICE(0x4E55, info), \
--
Jani Nikula, Intel Open Source Graphics Center
On Mon, 28 Sep 2020, Tejas Upadhyay <[email protected]> wrote:
> JSL has update in vswing table for eDP
I've thought the TLA for Jasper Lake is JSP, not JSL. At least we have
PCH_JSP for Jasper Lake PCH.
>
> BSpec: 21257
> Signed-off-by: Tejas Upadhyay <[email protected]>
> ---
> drivers/gpu/drm/i915/display/intel_ddi.c | 67 ++++++++++++++++++++++--
> 1 file changed, 64 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 4d06178cd76c..fa08463bcf2e 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -582,6 +582,34 @@ static const struct cnl_ddi_buf_trans ehl_combo_phy_ddi_translations_dp[] = {
> { 0x6, 0x7F, 0x3F, 0x00, 0x00 }, /* 900 900 0.0 */
> };
>
> +static const struct cnl_ddi_buf_trans jsl_combo_phy_ddi_translations_edp_hbr[] = {
> + /* NT mV Trans mV db */
> + { 0x8, 0x7F, 0x3F, 0x00, 0x00 }, /* 200 200 0.0 */
> + { 0x8, 0x7F, 0x38, 0x00, 0x07 }, /* 200 250 1.9 */
> + { 0x1, 0x7F, 0x33, 0x00, 0x0C }, /* 200 300 3.5 */
> + { 0xA, 0x35, 0x36, 0x00, 0x09 }, /* 200 350 4.9 */
> + { 0x8, 0x7F, 0x3F, 0x00, 0x00 }, /* 250 250 0.0 */
> + { 0x1, 0x7F, 0x38, 0x00, 0x07 }, /* 250 300 1.6 */
> + { 0xA, 0x35, 0x35, 0x00, 0x0A }, /* 250 350 2.9 */
> + { 0x1, 0x7F, 0x3F, 0x00, 0x00 }, /* 300 300 0.0 */
> + { 0xA, 0x35, 0x38, 0x00, 0x07 }, /* 300 350 1.3 */
> + { 0xA, 0x35, 0x3F, 0x00, 0x00 }, /* 350 350 0.0 */
> +};
> +
> +static const struct cnl_ddi_buf_trans jsl_combo_phy_ddi_translations_edp_hbr2[] = {
> + /* NT mV Trans mV db */
> + { 0x8, 0x7F, 0x3F, 0x00, 0x00 }, /* 200 200 0.0 */
> + { 0x8, 0x7F, 0x3F, 0x00, 0x00 }, /* 200 250 1.9 */
> + { 0x1, 0x7F, 0x3D, 0x00, 0x02 }, /* 200 300 3.5 */
> + { 0xA, 0x35, 0x38, 0x00, 0x07 }, /* 200 350 4.9 */
> + { 0x8, 0x7F, 0x3F, 0x00, 0x00 }, /* 250 250 0.0 */
> + { 0x1, 0x7F, 0x3F, 0x00, 0x00 }, /* 250 300 1.6 */
> + { 0xA, 0x35, 0x3A, 0x00, 0x05 }, /* 250 350 2.9 */
> + { 0x1, 0x7F, 0x3F, 0x00, 0x00 }, /* 300 300 0.0 */
> + { 0xA, 0x35, 0x38, 0x00, 0x07 }, /* 300 350 1.3 */
> + { 0xA, 0x35, 0x3F, 0x00, 0x00 }, /* 350 350 0.0 */
> +};
> +
> struct icl_mg_phy_ddi_buf_trans {
> u32 cri_txdeemph_override_11_6;
> u32 cri_txdeemph_override_5_0;
> @@ -1069,7 +1097,6 @@ icl_get_mg_buf_trans(struct intel_encoder *encoder, int type, int rate,
> *n_entries = ARRAY_SIZE(icl_mg_phy_ddi_translations_rbr_hbr);
> return icl_mg_phy_ddi_translations_rbr_hbr;
> }
> -
> static const struct cnl_ddi_buf_trans *
> ehl_get_combo_buf_trans(struct intel_encoder *encoder, int type, int rate,
> int *n_entries)
> @@ -1098,6 +1125,34 @@ ehl_get_combo_buf_trans(struct intel_encoder *encoder, int type, int rate,
> }
> }
>
> +static const struct cnl_ddi_buf_trans *
> +jsl_get_combo_buf_trans(struct intel_encoder *encoder, int type, int rate,
> + int *n_entries)
> +{
> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +
> + switch (type) {
> + case INTEL_OUTPUT_HDMI:
> + *n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_hdmi);
> + return icl_combo_phy_ddi_translations_hdmi;
> + case INTEL_OUTPUT_EDP:
> + if (dev_priv->vbt.edp.low_vswing) {
> + if (rate > 270000) {
> + *n_entries = ARRAY_SIZE(jsl_combo_phy_ddi_translations_edp_hbr2);
> + return jsl_combo_phy_ddi_translations_edp_hbr2;
> + } else {
> + *n_entries = ARRAY_SIZE(jsl_combo_phy_ddi_translations_edp_hbr);
> + return jsl_combo_phy_ddi_translations_edp_hbr;
> + }
> + }
> + /* fall through */
> + default:
> + /* All combo DP and eDP ports that do not support low_vswing */
> + *n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_dp_hbr2);
> + return icl_combo_phy_ddi_translations_dp_hbr2;
> + }
> +}
> +
> static const struct cnl_ddi_buf_trans *
> tgl_get_combo_buf_trans(struct intel_encoder *encoder, int type, int rate,
> int *n_entries)
> @@ -2265,9 +2320,12 @@ static u8 intel_ddi_dp_voltage_max(struct intel_dp *intel_dp)
> tgl_get_dkl_buf_trans(encoder, encoder->type,
> intel_dp->link_rate, &n_entries);
> } else if (INTEL_GEN(dev_priv) == 11) {
> - if (IS_ELKHARTLAKE(dev_priv))
> + if (IS_JASPERLAKE(dev_priv))
> + jsl_get_combo_buf_trans(encoder, encoder->type,
> + intel_dp->link_rate, &n_entries);
> + else if (IS_ELKHARTLAKE(dev_priv))
> ehl_get_combo_buf_trans(encoder, encoder->type,
> - intel_dp->link_rate, &n_entries);
> + intel_dp->link_rate, &n_entries);
This is a good example of a potential trap that having IS_ELKHARTLAKE()
cover both ELK and JSP creates. An unsuspecting coder might change the
if ladder to have IS_ELKHARTLAKE() first, and the subsequent
IS_JASPERLAKE() branch would never be taken.
BR,
Jani.
> else if (intel_phy_is_combo(dev_priv, phy))
> icl_get_combo_buf_trans(encoder, encoder->type,
> intel_dp->link_rate, &n_entries);
> @@ -2454,6 +2512,9 @@ static void icl_ddi_combo_vswing_program(struct intel_encoder *encoder,
> if (INTEL_GEN(dev_priv) >= 12)
> ddi_translations = tgl_get_combo_buf_trans(encoder, type, rate,
> &n_entries);
> + else if (IS_JASPERLAKE(dev_priv))
> + ddi_translations = jsl_get_combo_buf_trans(encoder, type, rate,
> + &n_entries);
> else if (IS_ELKHARTLAKE(dev_priv))
> ddi_translations = ehl_get_combo_buf_trans(encoder, type, rate,
> &n_entries);
--
Jani Nikula, Intel Open Source Graphics Center
On Mon, Sep 28, 2020 at 04:43:11PM +0300, Jani Nikula wrote:
> On Mon, 28 Sep 2020, Tejas Upadhyay <[email protected]> wrote:
> > JSL has update in vswing table for eDP
>
> I've thought the TLA for Jasper Lake is JSP, not JSL. At least we have
> PCH_JSP for Jasper Lake PCH.
JSP == Point (the PCH), JSL == Lake
-James
>
> >
> > BSpec: 21257
> > Signed-off-by: Tejas Upadhyay <[email protected]>
> > ---
> > drivers/gpu/drm/i915/display/intel_ddi.c | 67 ++++++++++++++++++++++--
> > 1 file changed, 64 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index 4d06178cd76c..fa08463bcf2e 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -582,6 +582,34 @@ static const struct cnl_ddi_buf_trans ehl_combo_phy_ddi_translations_dp[] = {
> > { 0x6, 0x7F, 0x3F, 0x00, 0x00 }, /* 900 900 0.0 */
> > };
> >
> > +static const struct cnl_ddi_buf_trans jsl_combo_phy_ddi_translations_edp_hbr[] = {
> > + /* NT mV Trans mV db */
> > + { 0x8, 0x7F, 0x3F, 0x00, 0x00 }, /* 200 200 0.0 */
> > + { 0x8, 0x7F, 0x38, 0x00, 0x07 }, /* 200 250 1.9 */
> > + { 0x1, 0x7F, 0x33, 0x00, 0x0C }, /* 200 300 3.5 */
> > + { 0xA, 0x35, 0x36, 0x00, 0x09 }, /* 200 350 4.9 */
> > + { 0x8, 0x7F, 0x3F, 0x00, 0x00 }, /* 250 250 0.0 */
> > + { 0x1, 0x7F, 0x38, 0x00, 0x07 }, /* 250 300 1.6 */
> > + { 0xA, 0x35, 0x35, 0x00, 0x0A }, /* 250 350 2.9 */
> > + { 0x1, 0x7F, 0x3F, 0x00, 0x00 }, /* 300 300 0.0 */
> > + { 0xA, 0x35, 0x38, 0x00, 0x07 }, /* 300 350 1.3 */
> > + { 0xA, 0x35, 0x3F, 0x00, 0x00 }, /* 350 350 0.0 */
> > +};
> > +
> > +static const struct cnl_ddi_buf_trans jsl_combo_phy_ddi_translations_edp_hbr2[] = {
> > + /* NT mV Trans mV db */
> > + { 0x8, 0x7F, 0x3F, 0x00, 0x00 }, /* 200 200 0.0 */
> > + { 0x8, 0x7F, 0x3F, 0x00, 0x00 }, /* 200 250 1.9 */
> > + { 0x1, 0x7F, 0x3D, 0x00, 0x02 }, /* 200 300 3.5 */
> > + { 0xA, 0x35, 0x38, 0x00, 0x07 }, /* 200 350 4.9 */
> > + { 0x8, 0x7F, 0x3F, 0x00, 0x00 }, /* 250 250 0.0 */
> > + { 0x1, 0x7F, 0x3F, 0x00, 0x00 }, /* 250 300 1.6 */
> > + { 0xA, 0x35, 0x3A, 0x00, 0x05 }, /* 250 350 2.9 */
> > + { 0x1, 0x7F, 0x3F, 0x00, 0x00 }, /* 300 300 0.0 */
> > + { 0xA, 0x35, 0x38, 0x00, 0x07 }, /* 300 350 1.3 */
> > + { 0xA, 0x35, 0x3F, 0x00, 0x00 }, /* 350 350 0.0 */
> > +};
> > +
> > struct icl_mg_phy_ddi_buf_trans {
> > u32 cri_txdeemph_override_11_6;
> > u32 cri_txdeemph_override_5_0;
> > @@ -1069,7 +1097,6 @@ icl_get_mg_buf_trans(struct intel_encoder *encoder, int type, int rate,
> > *n_entries = ARRAY_SIZE(icl_mg_phy_ddi_translations_rbr_hbr);
> > return icl_mg_phy_ddi_translations_rbr_hbr;
> > }
> > -
> > static const struct cnl_ddi_buf_trans *
> > ehl_get_combo_buf_trans(struct intel_encoder *encoder, int type, int rate,
> > int *n_entries)
> > @@ -1098,6 +1125,34 @@ ehl_get_combo_buf_trans(struct intel_encoder *encoder, int type, int rate,
> > }
> > }
> >
> > +static const struct cnl_ddi_buf_trans *
> > +jsl_get_combo_buf_trans(struct intel_encoder *encoder, int type, int rate,
> > + int *n_entries)
> > +{
> > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > +
> > + switch (type) {
> > + case INTEL_OUTPUT_HDMI:
> > + *n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_hdmi);
> > + return icl_combo_phy_ddi_translations_hdmi;
> > + case INTEL_OUTPUT_EDP:
> > + if (dev_priv->vbt.edp.low_vswing) {
> > + if (rate > 270000) {
> > + *n_entries = ARRAY_SIZE(jsl_combo_phy_ddi_translations_edp_hbr2);
> > + return jsl_combo_phy_ddi_translations_edp_hbr2;
> > + } else {
> > + *n_entries = ARRAY_SIZE(jsl_combo_phy_ddi_translations_edp_hbr);
> > + return jsl_combo_phy_ddi_translations_edp_hbr;
> > + }
> > + }
> > + /* fall through */
> > + default:
> > + /* All combo DP and eDP ports that do not support low_vswing */
> > + *n_entries = ARRAY_SIZE(icl_combo_phy_ddi_translations_dp_hbr2);
> > + return icl_combo_phy_ddi_translations_dp_hbr2;
> > + }
> > +}
> > +
> > static const struct cnl_ddi_buf_trans *
> > tgl_get_combo_buf_trans(struct intel_encoder *encoder, int type, int rate,
> > int *n_entries)
> > @@ -2265,9 +2320,12 @@ static u8 intel_ddi_dp_voltage_max(struct intel_dp *intel_dp)
> > tgl_get_dkl_buf_trans(encoder, encoder->type,
> > intel_dp->link_rate, &n_entries);
> > } else if (INTEL_GEN(dev_priv) == 11) {
> > - if (IS_ELKHARTLAKE(dev_priv))
> > + if (IS_JASPERLAKE(dev_priv))
> > + jsl_get_combo_buf_trans(encoder, encoder->type,
> > + intel_dp->link_rate, &n_entries);
> > + else if (IS_ELKHARTLAKE(dev_priv))
> > ehl_get_combo_buf_trans(encoder, encoder->type,
> > - intel_dp->link_rate, &n_entries);
> > + intel_dp->link_rate, &n_entries);
>
> This is a good example of a potential trap that having IS_ELKHARTLAKE()
> cover both ELK and JSP creates. An unsuspecting coder might change the
> if ladder to have IS_ELKHARTLAKE() first, and the subsequent
> IS_JASPERLAKE() branch would never be taken.
>
> BR,
> Jani.
>
> > else if (intel_phy_is_combo(dev_priv, phy))
> > icl_get_combo_buf_trans(encoder, encoder->type,
> > intel_dp->link_rate, &n_entries);
> > @@ -2454,6 +2512,9 @@ static void icl_ddi_combo_vswing_program(struct intel_encoder *encoder,
> > if (INTEL_GEN(dev_priv) >= 12)
> > ddi_translations = tgl_get_combo_buf_trans(encoder, type, rate,
> > &n_entries);
> > + else if (IS_JASPERLAKE(dev_priv))
> > + ddi_translations = jsl_get_combo_buf_trans(encoder, type, rate,
> > + &n_entries);
> > else if (IS_ELKHARTLAKE(dev_priv))
> > ddi_translations = ehl_get_combo_buf_trans(encoder, type, rate,
> > &n_entries);
>
> --
> Jani Nikula, Intel Open Source Graphics Center
On Mon, Sep 28, 2020 at 07:15:43AM -0700, James Ausmus wrote:
> On Mon, Sep 28, 2020 at 04:43:11PM +0300, Jani Nikula wrote:
> > On Mon, 28 Sep 2020, Tejas Upadhyay <[email protected]> wrote:
> > > JSL has update in vswing table for eDP
> >
> > I've thought the TLA for Jasper Lake is JSP, not JSL. At least we have
> > PCH_JSP for Jasper Lake PCH.
>
> JSP == Point (the PCH), JSL == Lake
.PT was "<something> Point", ..P stands just for "<something> PCH" IIRC.
--
Ville Syrj?l?
Intel
On Mon, 28 Sep 2020, "Surendrakumar Upadhyay, TejaskumarX" <[email protected]> wrote:
> ________________________________
> From: Jani Nikula <[email protected]>
> Sent: Monday, September 28, 2020 7:07 PM
> To: Surendrakumar Upadhyay, TejaskumarX <[email protected]>; Vivi, Rodrigo <[email protected]>; [email protected] <[email protected]>; [email protected] <[email protected]>; [email protected] <[email protected]>; [email protected] <[email protected]>; [email protected] <[email protected]>; Ausmus, James <[email protected]>; Roper, Matthew D <[email protected]>; Souza, Jose <[email protected]>; [email protected] <[email protected]>; De Marchi, Lucas <[email protected]>; Pandey, Hariom <[email protected]>
> Subject: Re: [PATCH 1/2] drm/i915/jsl: Split EHL/JSL platform info and PCI ids
Please fix your email quoting when interacting on the public lists.
>
> On Mon, 28 Sep 2020, Tejas Upadhyay <[email protected]> wrote:
>> Split the basic platform definition, macros, and PCI IDs to
>> differentiate between EHL and JSL platforms.
>>
>> Signed-off-by: Tejas Upadhyay <[email protected]>
>> ---
>> drivers/gpu/drm/i915/i915_drv.h | 4 +++-
>> drivers/gpu/drm/i915/i915_pci.c | 9 +++++++++
>> drivers/gpu/drm/i915/intel_device_info.c | 1 +
>> drivers/gpu/drm/i915/intel_device_info.h | 1 +
>> include/drm/i915_pciids.h | 9 ++++++---
>> 5 files changed, 20 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 72a9449b674e..4f20acebb038 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1417,7 +1417,9 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
>> #define IS_COMETLAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_COMETLAKE)
>> #define IS_CANNONLAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_CANNONLAKE)
>> #define IS_ICELAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_ICELAKE)
>> -#define IS_ELKHARTLAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_ELKHARTLAKE)
>> +#define IS_ELKHARTLAKE(dev_priv) (IS_PLATFORM(dev_priv, INTEL_ELKHARTLAKE) || \
>> + IS_PLATFORM(dev_priv, INTEL_JASPERLAKE))
>> +#define IS_JASPERLAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_JASPERLAKE)
>
> I think we've learned from history that we want the platform checks to
> be independent. I.e. if you need to split ELK and JSP, you need to make
> IS_ELKHARTLAKE() match *only* ELK, and you need to replace every current
> IS_ELKHARTLAKE() check with IS_ELKHARTLAKE() || IS_JASPERLAKE().
>
> We've been here before, and we've thought before that we can get by with
> the minimal change. It's just postponing the inevitable and generates
> confusion.
>
> BR,
> Jani.
>
> Tejas : Replacing IS_ELKHARTLAKE() || IS_JASPERLAKE() everywhere will
> make lot of changes at each place. To avoid huge change and to
> differentiate between platforms we have taken this way. Do you think
> we still change it everywhere? Do you have example where it can harm
> this change?
If you need to differentiate between the two platforms, IS_ELKHARTLAKE()
must mean only ELK and IS_JASPERLAKE() must mean only JSP.
It's non-negotiable. We've made the mistake before, we're not doing it
again.
There are 32 references to IS_ELKHARTLAKE(). It's slightly painful, but
the alternative is worse.
BR,
Jani.
>
>> #define IS_TIGERLAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_TIGERLAKE)
>> #define IS_ROCKETLAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_ROCKETLAKE)
>> #define IS_DG1(dev_priv) IS_PLATFORM(dev_priv, INTEL_DG1)
>> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
>> index 366ddfc8df6b..8690b69fcf33 100644
>> --- a/drivers/gpu/drm/i915/i915_pci.c
>> +++ b/drivers/gpu/drm/i915/i915_pci.c
>> @@ -846,6 +846,14 @@ static const struct intel_device_info ehl_info = {
>> .ppgtt_size = 36,
>> };
>>
>> +static const struct intel_device_info jsl_info = {
>> + GEN11_FEATURES,
>> + PLATFORM(INTEL_JASPERLAKE),
>> + .require_force_probe = 1,
>> + .platform_engine_mask = BIT(RCS0) | BIT(BCS0) | BIT(VCS0) | BIT(VECS0),
>> + .ppgtt_size = 36,
>> +};
>> +
>> #define GEN12_FEATURES \
>> GEN11_FEATURES, \
>> GEN(12), \
>> @@ -985,6 +993,7 @@ static const struct pci_device_id pciidlist[] = {
>> INTEL_CNL_IDS(&cnl_info),
>> INTEL_ICL_11_IDS(&icl_info),
>> INTEL_EHL_IDS(&ehl_info),
>> + INTEL_JSL_IDS(&jsl_info),
>> INTEL_TGL_12_IDS(&tgl_info),
>> INTEL_RKL_IDS(&rkl_info),
>> {0, 0, 0}
>> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
>> index adc836f15fde..e67cec8fa2aa 100644
>> --- a/drivers/gpu/drm/i915/intel_device_info.c
>> +++ b/drivers/gpu/drm/i915/intel_device_info.c
>> @@ -62,6 +62,7 @@ static const char * const platform_names[] = {
>> PLATFORM_NAME(CANNONLAKE),
>> PLATFORM_NAME(ICELAKE),
>> PLATFORM_NAME(ELKHARTLAKE),
>> + PLATFORM_NAME(JASPERLAKE),
>> PLATFORM_NAME(TIGERLAKE),
>> PLATFORM_NAME(ROCKETLAKE),
>> PLATFORM_NAME(DG1),
>> diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
>> index 6a3d607218aa..d92fa041c700 100644
>> --- a/drivers/gpu/drm/i915/intel_device_info.h
>> +++ b/drivers/gpu/drm/i915/intel_device_info.h
>> @@ -79,6 +79,7 @@ enum intel_platform {
>> /* gen11 */
>> INTEL_ICELAKE,
>> INTEL_ELKHARTLAKE,
>> + INTEL_JASPERLAKE,
>> /* gen12 */
>> INTEL_TIGERLAKE,
>> INTEL_ROCKETLAKE,
>> diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h
>> index 7eeecb07c9a1..1b5e09cfa11e 100644
>> --- a/include/drm/i915_pciids.h
>> +++ b/include/drm/i915_pciids.h
>> @@ -579,15 +579,18 @@
>> INTEL_VGA_DEVICE(0x8A51, info), \
>> INTEL_VGA_DEVICE(0x8A5D, info)
>>
>> -/* EHL/JSL */
>> +/* EHL */
>> #define INTEL_EHL_IDS(info) \
>> INTEL_VGA_DEVICE(0x4500, info), \
>> INTEL_VGA_DEVICE(0x4571, info), \
>> INTEL_VGA_DEVICE(0x4551, info), \
>> INTEL_VGA_DEVICE(0x4541, info), \
>> - INTEL_VGA_DEVICE(0x4E71, info), \
>> INTEL_VGA_DEVICE(0x4557, info), \
>> - INTEL_VGA_DEVICE(0x4555, info), \
>> + INTEL_VGA_DEVICE(0x4555, info)
>> +
>> +/* JSL */
>> +#define INTEL_JSL_IDS(info) \
>> + INTEL_VGA_DEVICE(0x4E71, info), \
>> INTEL_VGA_DEVICE(0x4E61, info), \
>> INTEL_VGA_DEVICE(0x4E57, info), \
>> INTEL_VGA_DEVICE(0x4E55, info), \
>
> --
> Jani Nikula, Intel Open Source Graphics Center
--
Jani Nikula, Intel Open Source Graphics Center
On Mon, 28 Sep 2020, Ville Syrjälä <[email protected]> wrote:
> On Mon, Sep 28, 2020 at 07:15:43AM -0700, James Ausmus wrote:
>> On Mon, Sep 28, 2020 at 04:43:11PM +0300, Jani Nikula wrote:
>> > On Mon, 28 Sep 2020, Tejas Upadhyay <[email protected]> wrote:
>> > > JSL has update in vswing table for eDP
>> >
>> > I've thought the TLA for Jasper Lake is JSP, not JSL. At least we have
>> > PCH_JSP for Jasper Lake PCH.
>>
>> JSP == Point (the PCH), JSL == Lake
>
> .PT was "<something> Point", ..P stands just for "<something> PCH" IIRC.
Yeah, nowadays it doesn't have "Point", however bspec agrees on the JSL
acronym for Jasper Lake.
Which means we should probably use PCH_JSL for Jasper Lake PCH as
well. Ugh.
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
On Mon, 28 Sep 2020, Matt Roper <[email protected]> wrote:
> Why are we adding IS_JASPERLAKE at all? EHL/JSL are documented as the
> same graphics IP, but are paired with different PCHs in the final SoCs,
> which is what causes the minor differences in programming. My
> understanding is that the voltage programming differences are ultimately
> due to that difference in PCH so we should just use HAS_PCH_MCC (EHL)
> and HAS_PCH_JSP (JSL) to distinguish which type of programming is needed
> rather than using a platform test.
Good point. If the difference is in the PCH, then of course the PCH
check should be used instead. Which avoids the problem altogether.
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
On Mon, Sep 28, 2020 at 08:14:02PM +0300, Jani Nikula wrote:
> On Mon, 28 Sep 2020, "Surendrakumar Upadhyay, TejaskumarX" <[email protected]> wrote:
> > ________________________________
> > From: Jani Nikula <[email protected]>
> > Sent: Monday, September 28, 2020 7:07 PM
> > To: Surendrakumar Upadhyay, TejaskumarX <[email protected]>; Vivi, Rodrigo <[email protected]>; [email protected] <[email protected]>; [email protected] <[email protected]>; [email protected] <[email protected]>; [email protected] <[email protected]>; [email protected] <[email protected]>; Ausmus, James <[email protected]>; Roper, Matthew D <[email protected]>; Souza, Jose <[email protected]>; [email protected] <[email protected]>; De Marchi, Lucas <[email protected]>; Pandey, Hariom <[email protected]>
> > Subject: Re: [PATCH 1/2] drm/i915/jsl: Split EHL/JSL platform info and PCI ids
>
> Please fix your email quoting when interacting on the public lists.
>
> >
> > On Mon, 28 Sep 2020, Tejas Upadhyay <[email protected]> wrote:
> >> Split the basic platform definition, macros, and PCI IDs to
> >> differentiate between EHL and JSL platforms.
> >>
> >> Signed-off-by: Tejas Upadhyay <[email protected]>
> >> ---
> >> drivers/gpu/drm/i915/i915_drv.h | 4 +++-
> >> drivers/gpu/drm/i915/i915_pci.c | 9 +++++++++
> >> drivers/gpu/drm/i915/intel_device_info.c | 1 +
> >> drivers/gpu/drm/i915/intel_device_info.h | 1 +
> >> include/drm/i915_pciids.h | 9 ++++++---
> >> 5 files changed, 20 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >> index 72a9449b674e..4f20acebb038 100644
> >> --- a/drivers/gpu/drm/i915/i915_drv.h
> >> +++ b/drivers/gpu/drm/i915/i915_drv.h
> >> @@ -1417,7 +1417,9 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
> >> #define IS_COMETLAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_COMETLAKE)
> >> #define IS_CANNONLAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_CANNONLAKE)
> >> #define IS_ICELAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_ICELAKE)
> >> -#define IS_ELKHARTLAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_ELKHARTLAKE)
> >> +#define IS_ELKHARTLAKE(dev_priv) (IS_PLATFORM(dev_priv, INTEL_ELKHARTLAKE) || \
> >> + IS_PLATFORM(dev_priv, INTEL_JASPERLAKE))
> >> +#define IS_JASPERLAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_JASPERLAKE)
> >
> > I think we've learned from history that we want the platform checks to
> > be independent. I.e. if you need to split ELK and JSP, you need to make
> > IS_ELKHARTLAKE() match *only* ELK, and you need to replace every current
> > IS_ELKHARTLAKE() check with IS_ELKHARTLAKE() || IS_JASPERLAKE().
> >
> > We've been here before, and we've thought before that we can get by with
> > the minimal change. It's just postponing the inevitable and generates
> > confusion.
> >
> > BR,
> > Jani.
> >
> > Tejas : Replacing IS_ELKHARTLAKE() || IS_JASPERLAKE() everywhere will
> > make lot of changes at each place. To avoid huge change and to
> > differentiate between platforms we have taken this way. Do you think
> > we still change it everywhere? Do you have example where it can harm
> > this change?
>
> If you need to differentiate between the two platforms, IS_ELKHARTLAKE()
> must mean only ELK and IS_JASPERLAKE() must mean only JSP.
>
> It's non-negotiable. We've made the mistake before, we're not doing it
> again.
>
> There are 32 references to IS_ELKHARTLAKE(). It's slightly painful, but
> the alternative is worse.
Why are we adding IS_JASPERLAKE at all? EHL/JSL are documented as the
same graphics IP, but are paired with different PCHs in the final SoCs,
which is what causes the minor differences in programming. My
understanding is that the voltage programming differences are ultimately
due to that difference in PCH so we should just use HAS_PCH_MCC (EHL)
and HAS_PCH_JSP (JSL) to distinguish which type of programming is needed
rather than using a platform test.
Matt
>
>
> BR,
> Jani.
>
>
> >
> >> #define IS_TIGERLAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_TIGERLAKE)
> >> #define IS_ROCKETLAKE(dev_priv) IS_PLATFORM(dev_priv, INTEL_ROCKETLAKE)
> >> #define IS_DG1(dev_priv) IS_PLATFORM(dev_priv, INTEL_DG1)
> >> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> >> index 366ddfc8df6b..8690b69fcf33 100644
> >> --- a/drivers/gpu/drm/i915/i915_pci.c
> >> +++ b/drivers/gpu/drm/i915/i915_pci.c
> >> @@ -846,6 +846,14 @@ static const struct intel_device_info ehl_info = {
> >> .ppgtt_size = 36,
> >> };
> >>
> >> +static const struct intel_device_info jsl_info = {
> >> + GEN11_FEATURES,
> >> + PLATFORM(INTEL_JASPERLAKE),
> >> + .require_force_probe = 1,
> >> + .platform_engine_mask = BIT(RCS0) | BIT(BCS0) | BIT(VCS0) | BIT(VECS0),
> >> + .ppgtt_size = 36,
> >> +};
> >> +
> >> #define GEN12_FEATURES \
> >> GEN11_FEATURES, \
> >> GEN(12), \
> >> @@ -985,6 +993,7 @@ static const struct pci_device_id pciidlist[] = {
> >> INTEL_CNL_IDS(&cnl_info),
> >> INTEL_ICL_11_IDS(&icl_info),
> >> INTEL_EHL_IDS(&ehl_info),
> >> + INTEL_JSL_IDS(&jsl_info),
> >> INTEL_TGL_12_IDS(&tgl_info),
> >> INTEL_RKL_IDS(&rkl_info),
> >> {0, 0, 0}
> >> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
> >> index adc836f15fde..e67cec8fa2aa 100644
> >> --- a/drivers/gpu/drm/i915/intel_device_info.c
> >> +++ b/drivers/gpu/drm/i915/intel_device_info.c
> >> @@ -62,6 +62,7 @@ static const char * const platform_names[] = {
> >> PLATFORM_NAME(CANNONLAKE),
> >> PLATFORM_NAME(ICELAKE),
> >> PLATFORM_NAME(ELKHARTLAKE),
> >> + PLATFORM_NAME(JASPERLAKE),
> >> PLATFORM_NAME(TIGERLAKE),
> >> PLATFORM_NAME(ROCKETLAKE),
> >> PLATFORM_NAME(DG1),
> >> diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
> >> index 6a3d607218aa..d92fa041c700 100644
> >> --- a/drivers/gpu/drm/i915/intel_device_info.h
> >> +++ b/drivers/gpu/drm/i915/intel_device_info.h
> >> @@ -79,6 +79,7 @@ enum intel_platform {
> >> /* gen11 */
> >> INTEL_ICELAKE,
> >> INTEL_ELKHARTLAKE,
> >> + INTEL_JASPERLAKE,
> >> /* gen12 */
> >> INTEL_TIGERLAKE,
> >> INTEL_ROCKETLAKE,
> >> diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h
> >> index 7eeecb07c9a1..1b5e09cfa11e 100644
> >> --- a/include/drm/i915_pciids.h
> >> +++ b/include/drm/i915_pciids.h
> >> @@ -579,15 +579,18 @@
> >> INTEL_VGA_DEVICE(0x8A51, info), \
> >> INTEL_VGA_DEVICE(0x8A5D, info)
> >>
> >> -/* EHL/JSL */
> >> +/* EHL */
> >> #define INTEL_EHL_IDS(info) \
> >> INTEL_VGA_DEVICE(0x4500, info), \
> >> INTEL_VGA_DEVICE(0x4571, info), \
> >> INTEL_VGA_DEVICE(0x4551, info), \
> >> INTEL_VGA_DEVICE(0x4541, info), \
> >> - INTEL_VGA_DEVICE(0x4E71, info), \
> >> INTEL_VGA_DEVICE(0x4557, info), \
> >> - INTEL_VGA_DEVICE(0x4555, info), \
> >> + INTEL_VGA_DEVICE(0x4555, info)
> >> +
> >> +/* JSL */
> >> +#define INTEL_JSL_IDS(info) \
> >> + INTEL_VGA_DEVICE(0x4E71, info), \
> >> INTEL_VGA_DEVICE(0x4E61, info), \
> >> INTEL_VGA_DEVICE(0x4E57, info), \
> >> INTEL_VGA_DEVICE(0x4E55, info), \
> >
> > --
> > Jani Nikula, Intel Open Source Graphics Center
>
> --
> Jani Nikula, Intel Open Source Graphics Center
--
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
On Mon, Sep 28, 2020 at 08:20:59PM +0300, Jani Nikula wrote:
> On Mon, 28 Sep 2020, Ville Syrj?l? <[email protected]> wrote:
> > On Mon, Sep 28, 2020 at 07:15:43AM -0700, James Ausmus wrote:
> >> On Mon, Sep 28, 2020 at 04:43:11PM +0300, Jani Nikula wrote:
> >> > On Mon, 28 Sep 2020, Tejas Upadhyay <[email protected]> wrote:
> >> > > JSL has update in vswing table for eDP
> >> >
> >> > I've thought the TLA for Jasper Lake is JSP, not JSL. At least we have
> >> > PCH_JSP for Jasper Lake PCH.
> >>
> >> JSP == Point (the PCH), JSL == Lake
> >
> > .PT was "<something> Point", ..P stands just for "<something> PCH" IIRC.
>
> Yeah, nowadays it doesn't have "Point", however bspec agrees on the JSL
> acronym for Jasper Lake.
Bspec uses ..P for "<platform> PCH", when it acknowledges the existence
of said PCH (see eg. CNP,ICP,TGP). JSP is not among that select crowd
however, neither really is MCC (although it is mentioned by name in the
JSL section).
I kinda want to nuke the JSP and MCC types entirely. I believe we should
be able to treat them as just ICP and TGP variants respectively. But
theres's still a bit of work left to do before we can get there.
--
Ville Syrj?l?
Intel