* [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
@ 2020-09-29 12:11 ` Tejas Upadhyay
0 siblings, 0 replies; 35+ messages in thread
From: Tejas Upadhyay @ 2020-09-29 12:11 UTC (permalink / raw)
To: intel-gfx, dri-devel, james.ausmus, matthew.d.roper, jose.souza,
ville.syrjala, lucas.demarchi, hariom.pandey
JSL has update in vswing table for eDP
BSpec: 21257
Changes since V1 :
- IS_ELKHARTLAKE and IS_JASPERLAKE is replaced with
HAS_PCH_MCC(EHL) and HAS_PCH_JSP(JSL) respectively
- Reverted EHL/JSL PCI ids split change
Signed-off-by: Tejas Upadhyay <tejaskumarx.surendrakumar.upadhyay@intel.com>
---
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..e6e93d01d0ce 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,7 +2320,10 @@ 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 (HAS_PCH_JSP(dev_priv))
+ jsl_get_combo_buf_trans(encoder, encoder->type,
+ intel_dp->link_rate, &n_entries);
+ else if (HAS_PCH_MCC(dev_priv))
ehl_get_combo_buf_trans(encoder, encoder->type,
intel_dp->link_rate, &n_entries);
else if (intel_phy_is_combo(dev_priv, phy))
@@ -2454,7 +2512,10 @@ 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_ELKHARTLAKE(dev_priv))
+ else if (HAS_PCH_JSP(dev_priv))
+ ddi_translations = jsl_get_combo_buf_trans(encoder, type, rate,
+ &n_entries);
+ else if (HAS_PCH_MCC(dev_priv))
ddi_translations = ehl_get_combo_buf_trans(encoder, type, rate,
&n_entries);
else
--
2.28.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [Intel-gfx] [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
@ 2020-09-29 12:11 ` Tejas Upadhyay
0 siblings, 0 replies; 35+ messages in thread
From: Tejas Upadhyay @ 2020-09-29 12:11 UTC (permalink / raw)
To: intel-gfx, dri-devel, james.ausmus, matthew.d.roper, jose.souza,
ville.syrjala, lucas.demarchi, hariom.pandey
JSL has update in vswing table for eDP
BSpec: 21257
Changes since V1 :
- IS_ELKHARTLAKE and IS_JASPERLAKE is replaced with
HAS_PCH_MCC(EHL) and HAS_PCH_JSP(JSL) respectively
- Reverted EHL/JSL PCI ids split change
Signed-off-by: Tejas Upadhyay <tejaskumarx.surendrakumar.upadhyay@intel.com>
---
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..e6e93d01d0ce 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,7 +2320,10 @@ 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 (HAS_PCH_JSP(dev_priv))
+ jsl_get_combo_buf_trans(encoder, encoder->type,
+ intel_dp->link_rate, &n_entries);
+ else if (HAS_PCH_MCC(dev_priv))
ehl_get_combo_buf_trans(encoder, encoder->type,
intel_dp->link_rate, &n_entries);
else if (intel_phy_is_combo(dev_priv, phy))
@@ -2454,7 +2512,10 @@ 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_ELKHARTLAKE(dev_priv))
+ else if (HAS_PCH_JSP(dev_priv))
+ ddi_translations = jsl_get_combo_buf_trans(encoder, type, rate,
+ &n_entries);
+ else if (HAS_PCH_MCC(dev_priv))
ddi_translations = ehl_get_combo_buf_trans(encoder, type, rate,
&n_entries);
else
--
2.28.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
2020-09-29 12:11 ` [Intel-gfx] " Tejas Upadhyay
@ 2020-09-29 12:52 ` Ville Syrjälä
-1 siblings, 0 replies; 35+ messages in thread
From: Ville Syrjälä @ 2020-09-29 12:52 UTC (permalink / raw)
To: Tejas Upadhyay
Cc: hariom.pandey, james.ausmus, intel-gfx, lucas.demarchi,
jose.souza, dri-devel
On Tue, Sep 29, 2020 at 05:41:27PM +0530, Tejas Upadhyay wrote:
> JSL has update in vswing table for eDP
>
> BSpec: 21257
>
> Changes since V1 :
> - IS_ELKHARTLAKE and IS_JASPERLAKE is replaced with
> HAS_PCH_MCC(EHL) and HAS_PCH_JSP(JSL) respectively
What do vswing values have to do with the PCH type?
> - Reverted EHL/JSL PCI ids split change
>
> Signed-off-by: Tejas Upadhyay <tejaskumarx.surendrakumar.upadhyay@intel.com>
> ---
> 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..e6e93d01d0ce 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,7 +2320,10 @@ 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 (HAS_PCH_JSP(dev_priv))
> + jsl_get_combo_buf_trans(encoder, encoder->type,
> + intel_dp->link_rate, &n_entries);
> + else if (HAS_PCH_MCC(dev_priv))
> ehl_get_combo_buf_trans(encoder, encoder->type,
> intel_dp->link_rate, &n_entries);
> else if (intel_phy_is_combo(dev_priv, phy))
> @@ -2454,7 +2512,10 @@ 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_ELKHARTLAKE(dev_priv))
> + else if (HAS_PCH_JSP(dev_priv))
> + ddi_translations = jsl_get_combo_buf_trans(encoder, type, rate,
> + &n_entries);
> + else if (HAS_PCH_MCC(dev_priv))
> ddi_translations = ehl_get_combo_buf_trans(encoder, type, rate,
> &n_entries);
> else
> --
> 2.28.0
--
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
@ 2020-09-29 12:52 ` Ville Syrjälä
0 siblings, 0 replies; 35+ messages in thread
From: Ville Syrjälä @ 2020-09-29 12:52 UTC (permalink / raw)
To: Tejas Upadhyay; +Cc: hariom.pandey, intel-gfx, lucas.demarchi, dri-devel
On Tue, Sep 29, 2020 at 05:41:27PM +0530, Tejas Upadhyay wrote:
> JSL has update in vswing table for eDP
>
> BSpec: 21257
>
> Changes since V1 :
> - IS_ELKHARTLAKE and IS_JASPERLAKE is replaced with
> HAS_PCH_MCC(EHL) and HAS_PCH_JSP(JSL) respectively
What do vswing values have to do with the PCH type?
> - Reverted EHL/JSL PCI ids split change
>
> Signed-off-by: Tejas Upadhyay <tejaskumarx.surendrakumar.upadhyay@intel.com>
> ---
> 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..e6e93d01d0ce 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,7 +2320,10 @@ 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 (HAS_PCH_JSP(dev_priv))
> + jsl_get_combo_buf_trans(encoder, encoder->type,
> + intel_dp->link_rate, &n_entries);
> + else if (HAS_PCH_MCC(dev_priv))
> ehl_get_combo_buf_trans(encoder, encoder->type,
> intel_dp->link_rate, &n_entries);
> else if (intel_phy_is_combo(dev_priv, phy))
> @@ -2454,7 +2512,10 @@ 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_ELKHARTLAKE(dev_priv))
> + else if (HAS_PCH_JSP(dev_priv))
> + ddi_translations = jsl_get_combo_buf_trans(encoder, type, rate,
> + &n_entries);
> + else if (HAS_PCH_MCC(dev_priv))
> ddi_translations = ehl_get_combo_buf_trans(encoder, type, rate,
> &n_entries);
> else
> --
> 2.28.0
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 35+ messages in thread
* RE: [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
2020-09-29 12:52 ` [Intel-gfx] " Ville Syrjälä
@ 2020-09-29 13:17 ` Surendrakumar Upadhyay, TejaskumarX
-1 siblings, 0 replies; 35+ messages in thread
From: Surendrakumar Upadhyay, TejaskumarX @ 2020-09-29 13:17 UTC (permalink / raw)
To: Ville Syrjälä
Cc: Pandey, Hariom, Ausmus, James, intel-gfx, De Marchi, Lucas,
Souza, Jose, dri-devel
-----Original Message-----
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Sent: 29 September 2020 18:23
To: Surendrakumar Upadhyay, TejaskumarX <tejaskumarx.surendrakumar.upadhyay@intel.com>
Cc: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Ausmus, James <james.ausmus@intel.com>; Roper, Matthew D <matthew.d.roper@intel.com>; Souza, Jose <jose.souza@intel.com>; De Marchi, Lucas <lucas.demarchi@intel.com>; Pandey, Hariom <hariom.pandey@intel.com>
Subject: Re: [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
On Tue, Sep 29, 2020 at 05:41:27PM +0530, Tejas Upadhyay wrote:
> JSL has update in vswing table for eDP
>
> BSpec: 21257
>
> Changes since V1 :
> - IS_ELKHARTLAKE and IS_JASPERLAKE is replaced with
> HAS_PCH_MCC(EHL) and HAS_PCH_JSP(JSL) respectively
What do vswing values have to do with the PCH type?
Tejas : There is difference in voltage swing values for EHL and JSL. Till now we were not differentiating between EHL and JSL as both were
based on ICL. Now as per compliance test we have found change in JSL eDP vswing values which does not apply to EHL which leads to differentiate
between EHL and JSL. Thus differentiator between JSL and EHL is PCH i.e HAS_PCH_MCC(EHL) and HAS_PCH_JSP(JSL). There is no direct relation of PCH with vswing.
> - Reverted EHL/JSL PCI ids split change
>
> Signed-off-by: Tejas Upadhyay
> <tejaskumarx.surendrakumar.upadhyay@intel.com>
> ---
> 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..e6e93d01d0ce 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,7 +2320,10 @@ 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 (HAS_PCH_JSP(dev_priv))
> + jsl_get_combo_buf_trans(encoder, encoder->type,
> + intel_dp->link_rate, &n_entries);
> + else if (HAS_PCH_MCC(dev_priv))
> ehl_get_combo_buf_trans(encoder, encoder->type,
> intel_dp->link_rate, &n_entries);
> else if (intel_phy_is_combo(dev_priv, phy)) @@ -2454,7 +2512,10 @@
> 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_ELKHARTLAKE(dev_priv))
> + else if (HAS_PCH_JSP(dev_priv))
> + ddi_translations = jsl_get_combo_buf_trans(encoder, type, rate,
> + &n_entries);
> + else if (HAS_PCH_MCC(dev_priv))
> ddi_translations = ehl_get_combo_buf_trans(encoder, type, rate,
> &n_entries);
> else
> --
> 2.28.0
--
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
@ 2020-09-29 13:17 ` Surendrakumar Upadhyay, TejaskumarX
0 siblings, 0 replies; 35+ messages in thread
From: Surendrakumar Upadhyay, TejaskumarX @ 2020-09-29 13:17 UTC (permalink / raw)
To: Ville Syrjälä
Cc: Pandey, Hariom, intel-gfx, De Marchi, Lucas, dri-devel
-----Original Message-----
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Sent: 29 September 2020 18:23
To: Surendrakumar Upadhyay, TejaskumarX <tejaskumarx.surendrakumar.upadhyay@intel.com>
Cc: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Ausmus, James <james.ausmus@intel.com>; Roper, Matthew D <matthew.d.roper@intel.com>; Souza, Jose <jose.souza@intel.com>; De Marchi, Lucas <lucas.demarchi@intel.com>; Pandey, Hariom <hariom.pandey@intel.com>
Subject: Re: [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
On Tue, Sep 29, 2020 at 05:41:27PM +0530, Tejas Upadhyay wrote:
> JSL has update in vswing table for eDP
>
> BSpec: 21257
>
> Changes since V1 :
> - IS_ELKHARTLAKE and IS_JASPERLAKE is replaced with
> HAS_PCH_MCC(EHL) and HAS_PCH_JSP(JSL) respectively
What do vswing values have to do with the PCH type?
Tejas : There is difference in voltage swing values for EHL and JSL. Till now we were not differentiating between EHL and JSL as both were
based on ICL. Now as per compliance test we have found change in JSL eDP vswing values which does not apply to EHL which leads to differentiate
between EHL and JSL. Thus differentiator between JSL and EHL is PCH i.e HAS_PCH_MCC(EHL) and HAS_PCH_JSP(JSL). There is no direct relation of PCH with vswing.
> - Reverted EHL/JSL PCI ids split change
>
> Signed-off-by: Tejas Upadhyay
> <tejaskumarx.surendrakumar.upadhyay@intel.com>
> ---
> 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..e6e93d01d0ce 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,7 +2320,10 @@ 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 (HAS_PCH_JSP(dev_priv))
> + jsl_get_combo_buf_trans(encoder, encoder->type,
> + intel_dp->link_rate, &n_entries);
> + else if (HAS_PCH_MCC(dev_priv))
> ehl_get_combo_buf_trans(encoder, encoder->type,
> intel_dp->link_rate, &n_entries);
> else if (intel_phy_is_combo(dev_priv, phy)) @@ -2454,7 +2512,10 @@
> 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_ELKHARTLAKE(dev_priv))
> + else if (HAS_PCH_JSP(dev_priv))
> + ddi_translations = jsl_get_combo_buf_trans(encoder, type, rate,
> + &n_entries);
> + else if (HAS_PCH_MCC(dev_priv))
> ddi_translations = ehl_get_combo_buf_trans(encoder, type, rate,
> &n_entries);
> else
> --
> 2.28.0
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 35+ messages in thread
* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/edp/jsl: Update vswing table for HBR and HBR2
2020-09-29 12:11 ` [Intel-gfx] " Tejas Upadhyay
(?)
(?)
@ 2020-09-29 17:51 ` Patchwork
-1 siblings, 0 replies; 35+ messages in thread
From: Patchwork @ 2020-09-29 17:51 UTC (permalink / raw)
To: Surendrakumar Upadhyay, TejaskumarX; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/edp/jsl: Update vswing table for HBR and HBR2
URL : https://patchwork.freedesktop.org/series/82206/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
548dfee5bc0f drm/i915/edp/jsl: Update vswing table for HBR and HBR2
-:83: WARNING:UNNECESSARY_ELSE: else is not generally useful after a break or return
#83: FILE: drivers/gpu/drm/i915/display/intel_ddi.c:1143:
+ return jsl_combo_phy_ddi_translations_edp_hbr2;
+ } else {
-:88: WARNING:PREFER_FALLTHROUGH: Prefer 'fallthrough;' over fallthrough comment
#88: FILE: drivers/gpu/drm/i915/display/intel_ddi.c:1148:
+ /* fall through */
total: 0 errors, 2 warnings, 0 checks, 97 lines checked
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 35+ messages in thread
* [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/edp/jsl: Update vswing table for HBR and HBR2
2020-09-29 12:11 ` [Intel-gfx] " Tejas Upadhyay
` (2 preceding siblings ...)
(?)
@ 2020-09-29 18:13 ` Patchwork
-1 siblings, 0 replies; 35+ messages in thread
From: Patchwork @ 2020-09-29 18:13 UTC (permalink / raw)
To: Surendrakumar Upadhyay, TejaskumarX; +Cc: intel-gfx
[-- Attachment #1.1: Type: text/plain, Size: 4564 bytes --]
== Series Details ==
Series: drm/i915/edp/jsl: Update vswing table for HBR and HBR2
URL : https://patchwork.freedesktop.org/series/82206/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_9075 -> Patchwork_18590
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18590/index.html
Known issues
------------
Here are the changes found in Patchwork_18590 that come from known issues:
### IGT changes ###
#### Possible fixes ####
* igt@i915_pm_rpm@basic-pci-d3-state:
- fi-bsw-kefka: [DMESG-WARN][1] ([i915#1982]) -> [PASS][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9075/fi-bsw-kefka/igt@i915_pm_rpm@basic-pci-d3-state.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18590/fi-bsw-kefka/igt@i915_pm_rpm@basic-pci-d3-state.html
* igt@vgem_basic@unload:
- fi-skl-guc: [DMESG-WARN][3] ([i915#2203]) -> [PASS][4]
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9075/fi-skl-guc/igt@vgem_basic@unload.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18590/fi-skl-guc/igt@vgem_basic@unload.html
#### Warnings ####
* igt@i915_pm_rpm@module-reload:
- fi-kbl-x1275: [DMESG-FAIL][5] ([i915#62]) -> [DMESG-FAIL][6] ([i915#62] / [i915#95])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9075/fi-kbl-x1275/igt@i915_pm_rpm@module-reload.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18590/fi-kbl-x1275/igt@i915_pm_rpm@module-reload.html
* igt@kms_flip@basic-flip-vs-wf_vblank@a-dp1:
- fi-kbl-x1275: [DMESG-WARN][7] ([i915#62] / [i915#92] / [i915#95]) -> [DMESG-WARN][8] ([i915#62] / [i915#92]) +1 similar issue
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9075/fi-kbl-x1275/igt@kms_flip@basic-flip-vs-wf_vblank@a-dp1.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18590/fi-kbl-x1275/igt@kms_flip@basic-flip-vs-wf_vblank@a-dp1.html
* igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c:
- fi-kbl-x1275: [DMESG-WARN][9] ([i915#62] / [i915#92]) -> [DMESG-WARN][10] ([i915#62] / [i915#92] / [i915#95]) +7 similar issues
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9075/fi-kbl-x1275/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18590/fi-kbl-x1275/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
* igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
- fi-kbl-x1275: [DMESG-WARN][11] ([i915#1982] / [i915#62] / [i915#92] / [i915#95]) -> [DMESG-WARN][12] ([i915#62] / [i915#92] / [i915#95])
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9075/fi-kbl-x1275/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18590/fi-kbl-x1275/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
* igt@vgem_basic@unload:
- fi-kbl-x1275: [DMESG-WARN][13] ([i915#62] / [i915#92]) -> [DMESG-WARN][14] ([i915#95])
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9075/fi-kbl-x1275/igt@vgem_basic@unload.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18590/fi-kbl-x1275/igt@vgem_basic@unload.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
[i915#2203]: https://gitlab.freedesktop.org/drm/intel/issues/2203
[i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62
[i915#92]: https://gitlab.freedesktop.org/drm/intel/issues/92
[i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95
Participating hosts (46 -> 39)
------------------------------
Missing (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper fi-bdw-samus
Build changes
-------------
* Linux: CI_DRM_9075 -> Patchwork_18590
CI-20190529: 20190529
CI_DRM_9075: fd24361b2b76956b5c056bc430a4c77edecb7744 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5792: cbaf441899f3b4f36cca5996aa6a69e7399b2dbd @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_18590: 548dfee5bc0ff01e16e2313ef9193950370b1a9c @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
548dfee5bc0f drm/i915/edp/jsl: Update vswing table for HBR and HBR2
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18590/index.html
[-- Attachment #1.2: Type: text/html, Size: 6619 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
2020-09-29 12:11 ` [Intel-gfx] " Tejas Upadhyay
@ 2020-09-29 19:33 ` Souza, Jose
-1 siblings, 0 replies; 35+ messages in thread
From: Souza, Jose @ 2020-09-29 19:33 UTC (permalink / raw)
To: Ausmus, James, Pandey, Hariom, Roper, Matthew D, dri-devel,
ville.syrjala, intel-gfx, Surendrakumar Upadhyay, TejaskumarX,
De Marchi, Lucas
On Tue, 2020-09-29 at 17:41 +0530, Tejas Upadhyay wrote:
> JSL has update in vswing table for eDP
Would be nice to mention in the commit description why PCH is being used, that would avoid Ville's question.
>
> BSpec: 21257
>
> Changes since V1 :
> - IS_ELKHARTLAKE and IS_JASPERLAKE is replaced with
> HAS_PCH_MCC(EHL) and HAS_PCH_JSP(JSL) respectively
> - Reverted EHL/JSL PCI ids split change
>
> Signed-off-by: Tejas Upadhyay <
> tejaskumarx.surendrakumar.upadhyay@intel.com
> >
> ---
> 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..e6e93d01d0ce 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 */
> +};
Tables matches specification.
> +
> 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;
> }
> -
Probably not intentional.
Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
Will push with this line fixed as soon as CI finish testing.
> 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,7 +2320,10 @@ 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 (HAS_PCH_JSP(dev_priv))
> + jsl_get_combo_buf_trans(encoder, encoder->type,
> + intel_dp->link_rate, &n_entries);
> + else if (HAS_PCH_MCC(dev_priv))
> ehl_get_combo_buf_trans(encoder, encoder->type,
> intel_dp->link_rate, &n_entries);
> else if (intel_phy_is_combo(dev_priv, phy))
> @@ -2454,7 +2512,10 @@ 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_ELKHARTLAKE(dev_priv))
> + else if (HAS_PCH_JSP(dev_priv))
> + ddi_translations = jsl_get_combo_buf_trans(encoder, type, rate,
> + &n_entries);
> + else if (HAS_PCH_MCC(dev_priv))
> ddi_translations = ehl_get_combo_buf_trans(encoder, type, rate,
> &n_entries);
> else
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
@ 2020-09-29 19:33 ` Souza, Jose
0 siblings, 0 replies; 35+ messages in thread
From: Souza, Jose @ 2020-09-29 19:33 UTC (permalink / raw)
To: Ausmus, James, Pandey, Hariom, Roper, Matthew D, dri-devel,
ville.syrjala, intel-gfx, Surendrakumar Upadhyay, TejaskumarX,
De Marchi, Lucas
On Tue, 2020-09-29 at 17:41 +0530, Tejas Upadhyay wrote:
> JSL has update in vswing table for eDP
Would be nice to mention in the commit description why PCH is being used, that would avoid Ville's question.
>
> BSpec: 21257
>
> Changes since V1 :
> - IS_ELKHARTLAKE and IS_JASPERLAKE is replaced with
> HAS_PCH_MCC(EHL) and HAS_PCH_JSP(JSL) respectively
> - Reverted EHL/JSL PCI ids split change
>
> Signed-off-by: Tejas Upadhyay <
> tejaskumarx.surendrakumar.upadhyay@intel.com
> >
> ---
> 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..e6e93d01d0ce 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 */
> +};
Tables matches specification.
> +
> 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;
> }
> -
Probably not intentional.
Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
Will push with this line fixed as soon as CI finish testing.
> 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,7 +2320,10 @@ 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 (HAS_PCH_JSP(dev_priv))
> + jsl_get_combo_buf_trans(encoder, encoder->type,
> + intel_dp->link_rate, &n_entries);
> + else if (HAS_PCH_MCC(dev_priv))
> ehl_get_combo_buf_trans(encoder, encoder->type,
> intel_dp->link_rate, &n_entries);
> else if (intel_phy_is_combo(dev_priv, phy))
> @@ -2454,7 +2512,10 @@ 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_ELKHARTLAKE(dev_priv))
> + else if (HAS_PCH_JSP(dev_priv))
> + ddi_translations = jsl_get_combo_buf_trans(encoder, type, rate,
> + &n_entries);
> + else if (HAS_PCH_MCC(dev_priv))
> ddi_translations = ehl_get_combo_buf_trans(encoder, type, rate,
> &n_entries);
> else
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
2020-09-29 19:33 ` [Intel-gfx] " Souza, Jose
@ 2020-09-29 20:02 ` Ville Syrjälä
-1 siblings, 0 replies; 35+ messages in thread
From: Ville Syrjälä @ 2020-09-29 20:02 UTC (permalink / raw)
To: Souza, Jose
Cc: Pandey, Hariom, Ausmus, James, intel-gfx, De Marchi, Lucas,
dri-devel, Surendrakumar Upadhyay, TejaskumarX
On Tue, Sep 29, 2020 at 07:33:45PM +0000, Souza, Jose wrote:
> On Tue, 2020-09-29 at 17:41 +0530, Tejas Upadhyay wrote:
> > JSL has update in vswing table for eDP
>
> Would be nice to mention in the commit description why PCH is being used, that would avoid Ville's question.
If the thing has nothing to do PCH then it should not use the PCH type
for the the check. Instead we should just do the EHL/JSL split.
>
> >
> > BSpec: 21257
> >
> > Changes since V1 :
> > - IS_ELKHARTLAKE and IS_JASPERLAKE is replaced with
> > HAS_PCH_MCC(EHL) and HAS_PCH_JSP(JSL) respectively
> > - Reverted EHL/JSL PCI ids split change
> >
> > Signed-off-by: Tejas Upadhyay <
> > tejaskumarx.surendrakumar.upadhyay@intel.com
> > >
> > ---
> > 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..e6e93d01d0ce 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 */
> > +};
>
> Tables matches specification.
>
> > +
> > 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;
> > }
> > -
>
> Probably not intentional.
>
> Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
>
> Will push with this line fixed as soon as CI finish testing.
>
>
> > 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,7 +2320,10 @@ 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 (HAS_PCH_JSP(dev_priv))
> > + jsl_get_combo_buf_trans(encoder, encoder->type,
> > + intel_dp->link_rate, &n_entries);
> > + else if (HAS_PCH_MCC(dev_priv))
> > ehl_get_combo_buf_trans(encoder, encoder->type,
> > intel_dp->link_rate, &n_entries);
> > else if (intel_phy_is_combo(dev_priv, phy))
> > @@ -2454,7 +2512,10 @@ 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_ELKHARTLAKE(dev_priv))
> > + else if (HAS_PCH_JSP(dev_priv))
> > + ddi_translations = jsl_get_combo_buf_trans(encoder, type, rate,
> > + &n_entries);
> > + else if (HAS_PCH_MCC(dev_priv))
> > ddi_translations = ehl_get_combo_buf_trans(encoder, type, rate,
> > &n_entries);
> > else
> >
--
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
@ 2020-09-29 20:02 ` Ville Syrjälä
0 siblings, 0 replies; 35+ messages in thread
From: Ville Syrjälä @ 2020-09-29 20:02 UTC (permalink / raw)
To: Souza, Jose; +Cc: Pandey, Hariom, intel-gfx, De Marchi, Lucas, dri-devel
On Tue, Sep 29, 2020 at 07:33:45PM +0000, Souza, Jose wrote:
> On Tue, 2020-09-29 at 17:41 +0530, Tejas Upadhyay wrote:
> > JSL has update in vswing table for eDP
>
> Would be nice to mention in the commit description why PCH is being used, that would avoid Ville's question.
If the thing has nothing to do PCH then it should not use the PCH type
for the the check. Instead we should just do the EHL/JSL split.
>
> >
> > BSpec: 21257
> >
> > Changes since V1 :
> > - IS_ELKHARTLAKE and IS_JASPERLAKE is replaced with
> > HAS_PCH_MCC(EHL) and HAS_PCH_JSP(JSL) respectively
> > - Reverted EHL/JSL PCI ids split change
> >
> > Signed-off-by: Tejas Upadhyay <
> > tejaskumarx.surendrakumar.upadhyay@intel.com
> > >
> > ---
> > 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..e6e93d01d0ce 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 */
> > +};
>
> Tables matches specification.
>
> > +
> > 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;
> > }
> > -
>
> Probably not intentional.
>
> Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
>
> Will push with this line fixed as soon as CI finish testing.
>
>
> > 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,7 +2320,10 @@ 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 (HAS_PCH_JSP(dev_priv))
> > + jsl_get_combo_buf_trans(encoder, encoder->type,
> > + intel_dp->link_rate, &n_entries);
> > + else if (HAS_PCH_MCC(dev_priv))
> > ehl_get_combo_buf_trans(encoder, encoder->type,
> > intel_dp->link_rate, &n_entries);
> > else if (intel_phy_is_combo(dev_priv, phy))
> > @@ -2454,7 +2512,10 @@ 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_ELKHARTLAKE(dev_priv))
> > + else if (HAS_PCH_JSP(dev_priv))
> > + ddi_translations = jsl_get_combo_buf_trans(encoder, type, rate,
> > + &n_entries);
> > + else if (HAS_PCH_MCC(dev_priv))
> > ddi_translations = ehl_get_combo_buf_trans(encoder, type, rate,
> > &n_entries);
> > else
> >
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
2020-09-29 20:02 ` [Intel-gfx] " Ville Syrjälä
@ 2020-09-29 20:20 ` Souza, Jose
-1 siblings, 0 replies; 35+ messages in thread
From: Souza, Jose @ 2020-09-29 20:20 UTC (permalink / raw)
To: ville.syrjala
Cc: Pandey, Hariom, Ausmus, James, intel-gfx, De Marchi, Lucas,
dri-devel, Surendrakumar Upadhyay, TejaskumarX
On Tue, 2020-09-29 at 23:02 +0300, Ville Syrjälä wrote:
> On Tue, Sep 29, 2020 at 07:33:45PM +0000, Souza, Jose wrote:
> > On Tue, 2020-09-29 at 17:41 +0530, Tejas Upadhyay wrote:
> > > JSL has update in vswing table for eDP
> >
> > Would be nice to mention in the commit description why PCH is being used, that would avoid Ville's question.
>
> If the thing has nothing to do PCH then it should not use the PCH type
> for the the check. Instead we should just do the EHL/JSL split.
In the first version Matt Roper suggested to use PCH to differentiate between EHL and JSL, Jani also agreed with this solution.This 2 PCHs can only be
associate with EHL and JSL respectively, so no downsides here.
>
> > > BSpec: 21257
> > >
> > > Changes since V1 :
> > > - IS_ELKHARTLAKE and IS_JASPERLAKE is replaced with
> > > HAS_PCH_MCC(EHL) and HAS_PCH_JSP(JSL) respectively
> > > - Reverted EHL/JSL PCI ids split change
> > >
> > > Signed-off-by: Tejas Upadhyay <
> > > tejaskumarx.surendrakumar.upadhyay@intel.com
> > >
> > >
> > > ---
> > > 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..e6e93d01d0ce 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 */
> > > +};
> >
> > Tables matches specification.
> >
> > > +
> > > 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;
> > > }
> > > -
> >
> > Probably not intentional.
> >
> > Reviewed-by: José Roberto de Souza <
> > jose.souza@intel.com
> > >
> >
> > Will push with this line fixed as soon as CI finish testing.
> >
> >
> > > 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,7 +2320,10 @@ 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 (HAS_PCH_JSP(dev_priv))
> > > + jsl_get_combo_buf_trans(encoder, encoder->type,
> > > + intel_dp->link_rate, &n_entries);
> > > + else if (HAS_PCH_MCC(dev_priv))
> > > ehl_get_combo_buf_trans(encoder, encoder->type,
> > > intel_dp->link_rate, &n_entries);
> > > else if (intel_phy_is_combo(dev_priv, phy))
> > > @@ -2454,7 +2512,10 @@ 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_ELKHARTLAKE(dev_priv))
> > > + else if (HAS_PCH_JSP(dev_priv))
> > > + ddi_translations = jsl_get_combo_buf_trans(encoder, type, rate,
> > > + &n_entries);
> > > + else if (HAS_PCH_MCC(dev_priv))
> > > ddi_translations = ehl_get_combo_buf_trans(encoder, type, rate,
> > > &n_entries);
> > > else
> > >
>
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
@ 2020-09-29 20:20 ` Souza, Jose
0 siblings, 0 replies; 35+ messages in thread
From: Souza, Jose @ 2020-09-29 20:20 UTC (permalink / raw)
To: ville.syrjala; +Cc: Pandey, Hariom, intel-gfx, De Marchi, Lucas, dri-devel
On Tue, 2020-09-29 at 23:02 +0300, Ville Syrjälä wrote:
> On Tue, Sep 29, 2020 at 07:33:45PM +0000, Souza, Jose wrote:
> > On Tue, 2020-09-29 at 17:41 +0530, Tejas Upadhyay wrote:
> > > JSL has update in vswing table for eDP
> >
> > Would be nice to mention in the commit description why PCH is being used, that would avoid Ville's question.
>
> If the thing has nothing to do PCH then it should not use the PCH type
> for the the check. Instead we should just do the EHL/JSL split.
In the first version Matt Roper suggested to use PCH to differentiate between EHL and JSL, Jani also agreed with this solution.This 2 PCHs can only be
associate with EHL and JSL respectively, so no downsides here.
>
> > > BSpec: 21257
> > >
> > > Changes since V1 :
> > > - IS_ELKHARTLAKE and IS_JASPERLAKE is replaced with
> > > HAS_PCH_MCC(EHL) and HAS_PCH_JSP(JSL) respectively
> > > - Reverted EHL/JSL PCI ids split change
> > >
> > > Signed-off-by: Tejas Upadhyay <
> > > tejaskumarx.surendrakumar.upadhyay@intel.com
> > >
> > >
> > > ---
> > > 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..e6e93d01d0ce 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 */
> > > +};
> >
> > Tables matches specification.
> >
> > > +
> > > 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;
> > > }
> > > -
> >
> > Probably not intentional.
> >
> > Reviewed-by: José Roberto de Souza <
> > jose.souza@intel.com
> > >
> >
> > Will push with this line fixed as soon as CI finish testing.
> >
> >
> > > 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,7 +2320,10 @@ 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 (HAS_PCH_JSP(dev_priv))
> > > + jsl_get_combo_buf_trans(encoder, encoder->type,
> > > + intel_dp->link_rate, &n_entries);
> > > + else if (HAS_PCH_MCC(dev_priv))
> > > ehl_get_combo_buf_trans(encoder, encoder->type,
> > > intel_dp->link_rate, &n_entries);
> > > else if (intel_phy_is_combo(dev_priv, phy))
> > > @@ -2454,7 +2512,10 @@ 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_ELKHARTLAKE(dev_priv))
> > > + else if (HAS_PCH_JSP(dev_priv))
> > > + ddi_translations = jsl_get_combo_buf_trans(encoder, type, rate,
> > > + &n_entries);
> > > + else if (HAS_PCH_MCC(dev_priv))
> > > ddi_translations = ehl_get_combo_buf_trans(encoder, type, rate,
> > > &n_entries);
> > > else
> > >
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
2020-09-29 20:20 ` [Intel-gfx] " Souza, Jose
@ 2020-09-29 20:30 ` Ville Syrjälä
-1 siblings, 0 replies; 35+ messages in thread
From: Ville Syrjälä @ 2020-09-29 20:30 UTC (permalink / raw)
To: Souza, Jose
Cc: Pandey, Hariom, Ausmus, James, intel-gfx, De Marchi, Lucas,
dri-devel, Surendrakumar Upadhyay, TejaskumarX
On Tue, Sep 29, 2020 at 08:20:22PM +0000, Souza, Jose wrote:
> On Tue, 2020-09-29 at 23:02 +0300, Ville Syrjälä wrote:
> > On Tue, Sep 29, 2020 at 07:33:45PM +0000, Souza, Jose wrote:
> > > On Tue, 2020-09-29 at 17:41 +0530, Tejas Upadhyay wrote:
> > > > JSL has update in vswing table for eDP
> > >
> > > Would be nice to mention in the commit description why PCH is being used, that would avoid Ville's question.
> >
> > If the thing has nothing to do PCH then it should not use the PCH type
> > for the the check. Instead we should just do the EHL/JSL split.
>
> In the first version Matt Roper suggested to use PCH to differentiate between EHL and JSL, Jani also agreed with this solution.This 2 PCHs can only be
> associate with EHL and JSL respectively, so no downsides here.
The downside is that the code makes no sense on the first glance.
It's going to generate a "wtf?" exception in the brain and require
me to take a second look to figure what is going on. Exception
handling is expensive and shouldn't be needed in cases where it's
trivial to make the code 100% obvious.
--
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
@ 2020-09-29 20:30 ` Ville Syrjälä
0 siblings, 0 replies; 35+ messages in thread
From: Ville Syrjälä @ 2020-09-29 20:30 UTC (permalink / raw)
To: Souza, Jose; +Cc: Pandey, Hariom, intel-gfx, De Marchi, Lucas, dri-devel
On Tue, Sep 29, 2020 at 08:20:22PM +0000, Souza, Jose wrote:
> On Tue, 2020-09-29 at 23:02 +0300, Ville Syrjälä wrote:
> > On Tue, Sep 29, 2020 at 07:33:45PM +0000, Souza, Jose wrote:
> > > On Tue, 2020-09-29 at 17:41 +0530, Tejas Upadhyay wrote:
> > > > JSL has update in vswing table for eDP
> > >
> > > Would be nice to mention in the commit description why PCH is being used, that would avoid Ville's question.
> >
> > If the thing has nothing to do PCH then it should not use the PCH type
> > for the the check. Instead we should just do the EHL/JSL split.
>
> In the first version Matt Roper suggested to use PCH to differentiate between EHL and JSL, Jani also agreed with this solution.This 2 PCHs can only be
> associate with EHL and JSL respectively, so no downsides here.
The downside is that the code makes no sense on the first glance.
It's going to generate a "wtf?" exception in the brain and require
me to take a second look to figure what is going on. Exception
handling is expensive and shouldn't be needed in cases where it's
trivial to make the code 100% obvious.
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
2020-09-29 20:30 ` [Intel-gfx] " Ville Syrjälä
@ 2020-09-29 20:34 ` Souza, Jose
-1 siblings, 0 replies; 35+ messages in thread
From: Souza, Jose @ 2020-09-29 20:34 UTC (permalink / raw)
To: ville.syrjala
Cc: Pandey, Hariom, Ausmus, James, intel-gfx, De Marchi, Lucas,
dri-devel, Surendrakumar Upadhyay, TejaskumarX
On Tue, 2020-09-29 at 23:30 +0300, Ville Syrjälä wrote:
> On Tue, Sep 29, 2020 at 08:20:22PM +0000, Souza, Jose wrote:
> > On Tue, 2020-09-29 at 23:02 +0300, Ville Syrjälä wrote:
> > > On Tue, Sep 29, 2020 at 07:33:45PM +0000, Souza, Jose wrote:
> > > > On Tue, 2020-09-29 at 17:41 +0530, Tejas Upadhyay wrote:
> > > > > JSL has update in vswing table for eDP
> > > >
> > > > Would be nice to mention in the commit description why PCH is being used, that would avoid Ville's question.
> > >
> > > If the thing has nothing to do PCH then it should not use the PCH type
> > > for the the check. Instead we should just do the EHL/JSL split.
> >
> > In the first version Matt Roper suggested to use PCH to differentiate between EHL and JSL, Jani also agreed with this solution.This 2 PCHs can only be
> > associate with EHL and JSL respectively, so no downsides here.
>
> The downside is that the code makes no sense on the first glance.
> It's going to generate a "wtf?" exception in the brain and require
> me to take a second look to figure what is going on. Exception
> handling is expensive and shouldn't be needed in cases where it's
> trivial to make the code 100% obvious.
>
Adding a comment on the top of jsl_get_combo_buf_trans() would help?
Otherwise Tejas you will need to rework this then.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
@ 2020-09-29 20:34 ` Souza, Jose
0 siblings, 0 replies; 35+ messages in thread
From: Souza, Jose @ 2020-09-29 20:34 UTC (permalink / raw)
To: ville.syrjala; +Cc: Pandey, Hariom, intel-gfx, De Marchi, Lucas, dri-devel
On Tue, 2020-09-29 at 23:30 +0300, Ville Syrjälä wrote:
> On Tue, Sep 29, 2020 at 08:20:22PM +0000, Souza, Jose wrote:
> > On Tue, 2020-09-29 at 23:02 +0300, Ville Syrjälä wrote:
> > > On Tue, Sep 29, 2020 at 07:33:45PM +0000, Souza, Jose wrote:
> > > > On Tue, 2020-09-29 at 17:41 +0530, Tejas Upadhyay wrote:
> > > > > JSL has update in vswing table for eDP
> > > >
> > > > Would be nice to mention in the commit description why PCH is being used, that would avoid Ville's question.
> > >
> > > If the thing has nothing to do PCH then it should not use the PCH type
> > > for the the check. Instead we should just do the EHL/JSL split.
> >
> > In the first version Matt Roper suggested to use PCH to differentiate between EHL and JSL, Jani also agreed with this solution.This 2 PCHs can only be
> > associate with EHL and JSL respectively, so no downsides here.
>
> The downside is that the code makes no sense on the first glance.
> It's going to generate a "wtf?" exception in the brain and require
> me to take a second look to figure what is going on. Exception
> handling is expensive and shouldn't be needed in cases where it's
> trivial to make the code 100% obvious.
>
Adding a comment on the top of jsl_get_combo_buf_trans() would help?
Otherwise Tejas you will need to rework this then.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
2020-09-29 20:30 ` [Intel-gfx] " Ville Syrjälä
@ 2020-09-29 21:01 ` Matt Roper
-1 siblings, 0 replies; 35+ messages in thread
From: Matt Roper @ 2020-09-29 21:01 UTC (permalink / raw)
To: Ville Syrjälä
Cc: Pandey, Hariom, Ausmus, James, intel-gfx, De Marchi, Lucas,
dri-devel, Souza, Jose, Surendrakumar Upadhyay, TejaskumarX
On Tue, Sep 29, 2020 at 11:30:22PM +0300, Ville Syrjälä wrote:
> On Tue, Sep 29, 2020 at 08:20:22PM +0000, Souza, Jose wrote:
> > On Tue, 2020-09-29 at 23:02 +0300, Ville Syrjälä wrote:
> > > On Tue, Sep 29, 2020 at 07:33:45PM +0000, Souza, Jose wrote:
> > > > On Tue, 2020-09-29 at 17:41 +0530, Tejas Upadhyay wrote:
> > > > > JSL has update in vswing table for eDP
> > > >
> > > > Would be nice to mention in the commit description why PCH is being used, that would avoid Ville's question.
> > >
> > > If the thing has nothing to do PCH then it should not use the PCH type
> > > for the the check. Instead we should just do the EHL/JSL split.
> >
> > In the first version Matt Roper suggested to use PCH to differentiate between EHL and JSL, Jani also agreed with this solution.This 2 PCHs can only be
> > associate with EHL and JSL respectively, so no downsides here.
>
> The downside is that the code makes no sense on the first glance.
> It's going to generate a "wtf?" exception in the brain and require
> me to take a second look to figure what is going on. Exception
> handling is expensive and shouldn't be needed in cases where it's
> trivial to make the code 100% obvious.
The bspec documents EHL and JSL as being the same platform and identical
in all programming since they are literally the same display IP; this
vswing table is the one and only place where the two are treated in a
distinct manner for reasons that lie outside the display controller. If
you had to stop and take a closer look at the code here, that's a
probably a good thing since in general there should generally never be a
difference in the behavior between the two. Adding an additional
clarifying comment is probably in order too since this is a very
exceptional special case.
If we deviate from the bspec's guidance and try to split IS_ELKHARTLAKE
and IS_JASPERLAKE across the whole driver, that's going to be a lot more
pain to maintain down the road since we'll almost certainly have cases
where someone silently leaves one or the other off a condition and gets
unexepcted behavior. I could see arguments for using a SUBPLATFORM here
like we do for TGL_U vs TGL_Y, but even that seems like overkill if we
already have a clear way to distinguish the two cases (PCH pairing) and
can just leave a clarifying comment.
Matt
>
> --
> Ville Syrjälä
> Intel
--
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
@ 2020-09-29 21:01 ` Matt Roper
0 siblings, 0 replies; 35+ messages in thread
From: Matt Roper @ 2020-09-29 21:01 UTC (permalink / raw)
To: Ville Syrjälä
Cc: Pandey, Hariom, intel-gfx, De Marchi, Lucas, dri-devel
On Tue, Sep 29, 2020 at 11:30:22PM +0300, Ville Syrjälä wrote:
> On Tue, Sep 29, 2020 at 08:20:22PM +0000, Souza, Jose wrote:
> > On Tue, 2020-09-29 at 23:02 +0300, Ville Syrjälä wrote:
> > > On Tue, Sep 29, 2020 at 07:33:45PM +0000, Souza, Jose wrote:
> > > > On Tue, 2020-09-29 at 17:41 +0530, Tejas Upadhyay wrote:
> > > > > JSL has update in vswing table for eDP
> > > >
> > > > Would be nice to mention in the commit description why PCH is being used, that would avoid Ville's question.
> > >
> > > If the thing has nothing to do PCH then it should not use the PCH type
> > > for the the check. Instead we should just do the EHL/JSL split.
> >
> > In the first version Matt Roper suggested to use PCH to differentiate between EHL and JSL, Jani also agreed with this solution.This 2 PCHs can only be
> > associate with EHL and JSL respectively, so no downsides here.
>
> The downside is that the code makes no sense on the first glance.
> It's going to generate a "wtf?" exception in the brain and require
> me to take a second look to figure what is going on. Exception
> handling is expensive and shouldn't be needed in cases where it's
> trivial to make the code 100% obvious.
The bspec documents EHL and JSL as being the same platform and identical
in all programming since they are literally the same display IP; this
vswing table is the one and only place where the two are treated in a
distinct manner for reasons that lie outside the display controller. If
you had to stop and take a closer look at the code here, that's a
probably a good thing since in general there should generally never be a
difference in the behavior between the two. Adding an additional
clarifying comment is probably in order too since this is a very
exceptional special case.
If we deviate from the bspec's guidance and try to split IS_ELKHARTLAKE
and IS_JASPERLAKE across the whole driver, that's going to be a lot more
pain to maintain down the road since we'll almost certainly have cases
where someone silently leaves one or the other off a condition and gets
unexepcted behavior. I could see arguments for using a SUBPLATFORM here
like we do for TGL_U vs TGL_Y, but even that seems like overkill if we
already have a clear way to distinguish the two cases (PCH pairing) and
can just leave a clarifying comment.
Matt
>
> --
> Ville Syrjälä
> Intel
--
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
2020-09-29 21:01 ` [Intel-gfx] " Matt Roper
@ 2020-09-29 21:11 ` Ville Syrjälä
-1 siblings, 0 replies; 35+ messages in thread
From: Ville Syrjälä @ 2020-09-29 21:11 UTC (permalink / raw)
To: Matt Roper
Cc: Pandey, Hariom, Ausmus, James, intel-gfx, De Marchi, Lucas,
dri-devel, Souza, Jose, Surendrakumar Upadhyay, TejaskumarX
On Tue, Sep 29, 2020 at 02:01:44PM -0700, Matt Roper wrote:
> On Tue, Sep 29, 2020 at 11:30:22PM +0300, Ville Syrjälä wrote:
> > On Tue, Sep 29, 2020 at 08:20:22PM +0000, Souza, Jose wrote:
> > > On Tue, 2020-09-29 at 23:02 +0300, Ville Syrjälä wrote:
> > > > On Tue, Sep 29, 2020 at 07:33:45PM +0000, Souza, Jose wrote:
> > > > > On Tue, 2020-09-29 at 17:41 +0530, Tejas Upadhyay wrote:
> > > > > > JSL has update in vswing table for eDP
> > > > >
> > > > > Would be nice to mention in the commit description why PCH is being used, that would avoid Ville's question.
> > > >
> > > > If the thing has nothing to do PCH then it should not use the PCH type
> > > > for the the check. Instead we should just do the EHL/JSL split.
> > >
> > > In the first version Matt Roper suggested to use PCH to differentiate between EHL and JSL, Jani also agreed with this solution.This 2 PCHs can only be
> > > associate with EHL and JSL respectively, so no downsides here.
> >
> > The downside is that the code makes no sense on the first glance.
> > It's going to generate a "wtf?" exception in the brain and require
> > me to take a second look to figure what is going on. Exception
> > handling is expensive and shouldn't be needed in cases where it's
> > trivial to make the code 100% obvious.
>
> The bspec documents EHL and JSL as being the same platform and identical
> in all programming since they are literally the same display IP; this
> vswing table is the one and only place where the two are treated in a
> distinct manner for reasons that lie outside the display controller. If
> you had to stop and take a closer look at the code here, that's a
> probably a good thing since in general there should generally never be a
> difference in the behavior between the two. Adding an additional
> clarifying comment is probably in order too since this is a very
> exceptional special case.
>
> If we deviate from the bspec's guidance and try to split IS_ELKHARTLAKE
> and IS_JASPERLAKE across the whole driver, that's going to be a lot more
> pain to maintain down the road since we'll almost certainly have cases
> where someone silently leaves one or the other off a condition and gets
> unexepcted behavior. I could see arguments for using a SUBPLATFORM here
> like we do for TGL_U vs TGL_Y, but even that seems like overkill if we
> already have a clear way to distinguish the two cases (PCH pairing) and
> can just leave a clarifying comment.
That fixed PCH pairing is totally undocumented AFAICS. And vswing has
nothing to do with the south display, so the wtf will still happen.
Comment or no comment.
--
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
@ 2020-09-29 21:11 ` Ville Syrjälä
0 siblings, 0 replies; 35+ messages in thread
From: Ville Syrjälä @ 2020-09-29 21:11 UTC (permalink / raw)
To: Matt Roper; +Cc: Pandey, Hariom, intel-gfx, De Marchi, Lucas, dri-devel
On Tue, Sep 29, 2020 at 02:01:44PM -0700, Matt Roper wrote:
> On Tue, Sep 29, 2020 at 11:30:22PM +0300, Ville Syrjälä wrote:
> > On Tue, Sep 29, 2020 at 08:20:22PM +0000, Souza, Jose wrote:
> > > On Tue, 2020-09-29 at 23:02 +0300, Ville Syrjälä wrote:
> > > > On Tue, Sep 29, 2020 at 07:33:45PM +0000, Souza, Jose wrote:
> > > > > On Tue, 2020-09-29 at 17:41 +0530, Tejas Upadhyay wrote:
> > > > > > JSL has update in vswing table for eDP
> > > > >
> > > > > Would be nice to mention in the commit description why PCH is being used, that would avoid Ville's question.
> > > >
> > > > If the thing has nothing to do PCH then it should not use the PCH type
> > > > for the the check. Instead we should just do the EHL/JSL split.
> > >
> > > In the first version Matt Roper suggested to use PCH to differentiate between EHL and JSL, Jani also agreed with this solution.This 2 PCHs can only be
> > > associate with EHL and JSL respectively, so no downsides here.
> >
> > The downside is that the code makes no sense on the first glance.
> > It's going to generate a "wtf?" exception in the brain and require
> > me to take a second look to figure what is going on. Exception
> > handling is expensive and shouldn't be needed in cases where it's
> > trivial to make the code 100% obvious.
>
> The bspec documents EHL and JSL as being the same platform and identical
> in all programming since they are literally the same display IP; this
> vswing table is the one and only place where the two are treated in a
> distinct manner for reasons that lie outside the display controller. If
> you had to stop and take a closer look at the code here, that's a
> probably a good thing since in general there should generally never be a
> difference in the behavior between the two. Adding an additional
> clarifying comment is probably in order too since this is a very
> exceptional special case.
>
> If we deviate from the bspec's guidance and try to split IS_ELKHARTLAKE
> and IS_JASPERLAKE across the whole driver, that's going to be a lot more
> pain to maintain down the road since we'll almost certainly have cases
> where someone silently leaves one or the other off a condition and gets
> unexepcted behavior. I could see arguments for using a SUBPLATFORM here
> like we do for TGL_U vs TGL_Y, but even that seems like overkill if we
> already have a clear way to distinguish the two cases (PCH pairing) and
> can just leave a clarifying comment.
That fixed PCH pairing is totally undocumented AFAICS. And vswing has
nothing to do with the south display, so the wtf will still happen.
Comment or no comment.
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
2020-09-29 21:11 ` [Intel-gfx] " Ville Syrjälä
@ 2020-09-29 21:59 ` Ville Syrjälä
-1 siblings, 0 replies; 35+ messages in thread
From: Ville Syrjälä @ 2020-09-29 21:59 UTC (permalink / raw)
To: Matt Roper
Cc: Pandey, Hariom, Ausmus, James, intel-gfx, De Marchi, Lucas,
Souza, Jose, dri-devel, Surendrakumar Upadhyay, TejaskumarX
On Wed, Sep 30, 2020 at 12:11:48AM +0300, Ville Syrjälä wrote:
> On Tue, Sep 29, 2020 at 02:01:44PM -0700, Matt Roper wrote:
> > On Tue, Sep 29, 2020 at 11:30:22PM +0300, Ville Syrjälä wrote:
> > > On Tue, Sep 29, 2020 at 08:20:22PM +0000, Souza, Jose wrote:
> > > > On Tue, 2020-09-29 at 23:02 +0300, Ville Syrjälä wrote:
> > > > > On Tue, Sep 29, 2020 at 07:33:45PM +0000, Souza, Jose wrote:
> > > > > > On Tue, 2020-09-29 at 17:41 +0530, Tejas Upadhyay wrote:
> > > > > > > JSL has update in vswing table for eDP
> > > > > >
> > > > > > Would be nice to mention in the commit description why PCH is being used, that would avoid Ville's question.
> > > > >
> > > > > If the thing has nothing to do PCH then it should not use the PCH type
> > > > > for the the check. Instead we should just do the EHL/JSL split.
> > > >
> > > > In the first version Matt Roper suggested to use PCH to differentiate between EHL and JSL, Jani also agreed with this solution.This 2 PCHs can only be
> > > > associate with EHL and JSL respectively, so no downsides here.
> > >
> > > The downside is that the code makes no sense on the first glance.
> > > It's going to generate a "wtf?" exception in the brain and require
> > > me to take a second look to figure what is going on. Exception
> > > handling is expensive and shouldn't be needed in cases where it's
> > > trivial to make the code 100% obvious.
> >
> > The bspec documents EHL and JSL as being the same platform and identical
> > in all programming since they are literally the same display IP; this
> > vswing table is the one and only place where the two are treated in a
> > distinct manner for reasons that lie outside the display controller. If
> > you had to stop and take a closer look at the code here, that's a
> > probably a good thing since in general there should generally never be a
> > difference in the behavior between the two. Adding an additional
> > clarifying comment is probably in order too since this is a very
> > exceptional special case.
> >
> > If we deviate from the bspec's guidance and try to split IS_ELKHARTLAKE
> > and IS_JASPERLAKE across the whole driver, that's going to be a lot more
> > pain to maintain down the road since we'll almost certainly have cases
> > where someone silently leaves one or the other off a condition and gets
> > unexepcted behavior. I could see arguments for using a SUBPLATFORM here
> > like we do for TGL_U vs TGL_Y, but even that seems like overkill if we
> > already have a clear way to distinguish the two cases (PCH pairing) and
> > can just leave a clarifying comment.
>
> That fixed PCH pairing is totally undocumented AFAICS. And vswing has
> nothing to do with the south display, so the wtf will still happen.
> Comment or no comment.
Oh and JSP does not show up in bspec even once. MCC appears exactly once
where it talks about the differences between MCC and ICL-N PCH (which I
guess is the same as JSP?).
Furthermore the spec never really talks about EHL except in very select
places. So the IS_ELKHARTLAKE is already totally confusing and IMO more
likely to cause maintenance problems than the split. Making it
IS_JSL||IS_EHL at least gives the reader some hint as to where they
should look in the spec.
Another argument why the split is fine is CFL/CML. Those are more
or less exactly in the same boat as EHL. Ie. only really mentioned
in the "configurations" section. Beyond that only KBL is ever really
mentioned. And yet we have separate IS_FOOs for all of them, and
apparently no one had any objections to that situation.
tldr;we have an established way to handle these things, so IMO lets
just follow it and stop adding special cases.
--
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
@ 2020-09-29 21:59 ` Ville Syrjälä
0 siblings, 0 replies; 35+ messages in thread
From: Ville Syrjälä @ 2020-09-29 21:59 UTC (permalink / raw)
To: Matt Roper; +Cc: Pandey, Hariom, intel-gfx, De Marchi, Lucas, dri-devel
On Wed, Sep 30, 2020 at 12:11:48AM +0300, Ville Syrjälä wrote:
> On Tue, Sep 29, 2020 at 02:01:44PM -0700, Matt Roper wrote:
> > On Tue, Sep 29, 2020 at 11:30:22PM +0300, Ville Syrjälä wrote:
> > > On Tue, Sep 29, 2020 at 08:20:22PM +0000, Souza, Jose wrote:
> > > > On Tue, 2020-09-29 at 23:02 +0300, Ville Syrjälä wrote:
> > > > > On Tue, Sep 29, 2020 at 07:33:45PM +0000, Souza, Jose wrote:
> > > > > > On Tue, 2020-09-29 at 17:41 +0530, Tejas Upadhyay wrote:
> > > > > > > JSL has update in vswing table for eDP
> > > > > >
> > > > > > Would be nice to mention in the commit description why PCH is being used, that would avoid Ville's question.
> > > > >
> > > > > If the thing has nothing to do PCH then it should not use the PCH type
> > > > > for the the check. Instead we should just do the EHL/JSL split.
> > > >
> > > > In the first version Matt Roper suggested to use PCH to differentiate between EHL and JSL, Jani also agreed with this solution.This 2 PCHs can only be
> > > > associate with EHL and JSL respectively, so no downsides here.
> > >
> > > The downside is that the code makes no sense on the first glance.
> > > It's going to generate a "wtf?" exception in the brain and require
> > > me to take a second look to figure what is going on. Exception
> > > handling is expensive and shouldn't be needed in cases where it's
> > > trivial to make the code 100% obvious.
> >
> > The bspec documents EHL and JSL as being the same platform and identical
> > in all programming since they are literally the same display IP; this
> > vswing table is the one and only place where the two are treated in a
> > distinct manner for reasons that lie outside the display controller. If
> > you had to stop and take a closer look at the code here, that's a
> > probably a good thing since in general there should generally never be a
> > difference in the behavior between the two. Adding an additional
> > clarifying comment is probably in order too since this is a very
> > exceptional special case.
> >
> > If we deviate from the bspec's guidance and try to split IS_ELKHARTLAKE
> > and IS_JASPERLAKE across the whole driver, that's going to be a lot more
> > pain to maintain down the road since we'll almost certainly have cases
> > where someone silently leaves one or the other off a condition and gets
> > unexepcted behavior. I could see arguments for using a SUBPLATFORM here
> > like we do for TGL_U vs TGL_Y, but even that seems like overkill if we
> > already have a clear way to distinguish the two cases (PCH pairing) and
> > can just leave a clarifying comment.
>
> That fixed PCH pairing is totally undocumented AFAICS. And vswing has
> nothing to do with the south display, so the wtf will still happen.
> Comment or no comment.
Oh and JSP does not show up in bspec even once. MCC appears exactly once
where it talks about the differences between MCC and ICL-N PCH (which I
guess is the same as JSP?).
Furthermore the spec never really talks about EHL except in very select
places. So the IS_ELKHARTLAKE is already totally confusing and IMO more
likely to cause maintenance problems than the split. Making it
IS_JSL||IS_EHL at least gives the reader some hint as to where they
should look in the spec.
Another argument why the split is fine is CFL/CML. Those are more
or less exactly in the same boat as EHL. Ie. only really mentioned
in the "configurations" section. Beyond that only KBL is ever really
mentioned. And yet we have separate IS_FOOs for all of them, and
apparently no one had any objections to that situation.
tldr;we have an established way to handle these things, so IMO lets
just follow it and stop adding special cases.
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
2020-09-29 21:59 ` [Intel-gfx] " Ville Syrjälä
@ 2020-09-29 23:38 ` Matt Roper
-1 siblings, 0 replies; 35+ messages in thread
From: Matt Roper @ 2020-09-29 23:38 UTC (permalink / raw)
To: Ville Syrjälä
Cc: Pandey, Hariom, Ausmus, James, intel-gfx, De Marchi, Lucas,
Souza, Jose, dri-devel, Surendrakumar Upadhyay, TejaskumarX
On Wed, Sep 30, 2020 at 12:59:58AM +0300, Ville Syrjälä wrote:
> On Wed, Sep 30, 2020 at 12:11:48AM +0300, Ville Syrjälä wrote:
> > On Tue, Sep 29, 2020 at 02:01:44PM -0700, Matt Roper wrote:
> > > On Tue, Sep 29, 2020 at 11:30:22PM +0300, Ville Syrjälä wrote:
> > > > On Tue, Sep 29, 2020 at 08:20:22PM +0000, Souza, Jose wrote:
> > > > > On Tue, 2020-09-29 at 23:02 +0300, Ville Syrjälä wrote:
> > > > > > On Tue, Sep 29, 2020 at 07:33:45PM +0000, Souza, Jose wrote:
> > > > > > > On Tue, 2020-09-29 at 17:41 +0530, Tejas Upadhyay wrote:
> > > > > > > > JSL has update in vswing table for eDP
> > > > > > >
> > > > > > > Would be nice to mention in the commit description why PCH is being used, that would avoid Ville's question.
> > > > > >
> > > > > > If the thing has nothing to do PCH then it should not use the PCH type
> > > > > > for the the check. Instead we should just do the EHL/JSL split.
> > > > >
> > > > > In the first version Matt Roper suggested to use PCH to differentiate between EHL and JSL, Jani also agreed with this solution.This 2 PCHs can only be
> > > > > associate with EHL and JSL respectively, so no downsides here.
> > > >
> > > > The downside is that the code makes no sense on the first glance.
> > > > It's going to generate a "wtf?" exception in the brain and require
> > > > me to take a second look to figure what is going on. Exception
> > > > handling is expensive and shouldn't be needed in cases where it's
> > > > trivial to make the code 100% obvious.
> > >
> > > The bspec documents EHL and JSL as being the same platform and identical
> > > in all programming since they are literally the same display IP; this
> > > vswing table is the one and only place where the two are treated in a
> > > distinct manner for reasons that lie outside the display controller. If
> > > you had to stop and take a closer look at the code here, that's a
> > > probably a good thing since in general there should generally never be a
> > > difference in the behavior between the two. Adding an additional
> > > clarifying comment is probably in order too since this is a very
> > > exceptional special case.
> > >
> > > If we deviate from the bspec's guidance and try to split IS_ELKHARTLAKE
> > > and IS_JASPERLAKE across the whole driver, that's going to be a lot more
> > > pain to maintain down the road since we'll almost certainly have cases
> > > where someone silently leaves one or the other off a condition and gets
> > > unexepcted behavior. I could see arguments for using a SUBPLATFORM here
> > > like we do for TGL_U vs TGL_Y, but even that seems like overkill if we
> > > already have a clear way to distinguish the two cases (PCH pairing) and
> > > can just leave a clarifying comment.
> >
> > That fixed PCH pairing is totally undocumented AFAICS. And vswing has
> > nothing to do with the south display, so the wtf will still happen.
> > Comment or no comment.
>
> Oh and JSP does not show up in bspec even once. MCC appears exactly once
> where it talks about the differences between MCC and ICL-N PCH (which I
> guess is the same as JSP?).
No, ICL-N PCH is something different. :-( There were some early test
chips created that paired the EHL/JSL graphics/media/display IP with an
ICL PCH just for early debug/test purposes, but that pairing isn't
something that actually exists as a real platform.
I think the confusion here arises because most driver developers only
look at (or have access to) the bspec, which does not aim to document
end-user platforms, but rather IP families that the
graphics/media/display hardware IP teams design. The bspec is not an
authoritative document for anything that lies outside the GMD IP itself.
The GMD architects do sometimes try to pull in additional information
from external teams/sources (such as PCH pairing or the electrical
details like the vswing programming here) to make life easier for the
software teams like us that only really deal with the bspec, but that
information comes from external sources, so it's a bit inconsistent in
terms of how much detail there is (or even whether it's described at
all). We could probably file bspec defects to get them to include the
PCH pairing details for EHL/JSL in the bspec, but ultimately EHL="EHL
G/M/D + MCC PCH" and JSL="EHL G/M/D + JSP PCH;" this has already been
confirmed in an offline email thread with the hardware teams.
Elkhart Lake and Jasper Lake are two separate end-user platforms, that
both incorporated the same G/M/D IP design. The name "Jasper Lake"
existed as a codename first, so that's the name that shows up in the
bspec; this wound up being a bit confusing when Elkhart Lake was
actually the first of the two to be released and thus wound up being the
name we have in our code. But the situation seems similar to CHT vs BSW
which are both referred to as "CHV" in the bspec and in our code
(although obviously there was no PCH pairing for those SoCs).
Steppings, workarounds, etc. are unified for EHL/JSL because they're
literally the same IP, rather than one being a derivative of the other.
If you want full details about the PCHs of a platform (most of which is
unimportant to graphics drivers) or the electrical characteristics that
feed into the vswing programming then there are other authoritative
documents that cover that (like the Electrical Design Specification and
such). I'm not sure if those documents are posted anywhere publicly;
fortunately we only need a small amount of information in those areas
and the GMD architects are often nice enough to try to copy the relevant
info into the bspec as a courtesy.
>
> Furthermore the spec never really talks about EHL except in very select
> places. So the IS_ELKHARTLAKE is already totally confusing and IMO more
* > likely to cause maintenance problems than the split. Making it
> IS_JSL||IS_EHL at least gives the reader some hint as to where they
> should look in the spec.
>
> Another argument why the split is fine is CFL/CML. Those are more
> or less exactly in the same boat as EHL. Ie. only really mentioned
> in the "configurations" section. Beyond that only KBL is ever really
> mentioned. And yet we have separate IS_FOOs for all of them, and
> apparently no one had any objections to that situation.
>
> tldr;we have an established way to handle these things, so IMO lets
> just follow it and stop adding special cases.
Isn't CML graphics a derivative of CFL (rather than being exactly the
same IP)? The fact that we have differences in the GMD IP itself that
need different workarounds implies that it's not quite the same
situation we're talking about here (otherwise we'd have been able to
just check the stepping revision ID). IS_CML only got split out from
IS_CFL a couple months ago, so it's probably too soon to see how many
bugs eventually creep in when developers accidentally leave it off a CFL
condition or vice versa.
And we do still unify WHL, AML, etc., in i915 as far as I can see even
though the IP teams track those platforms separately, so the precedent
appears to be keeping them combined as far as I can see?
Matt
>
> --
> Ville Syrjälä
> Intel
--
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
@ 2020-09-29 23:38 ` Matt Roper
0 siblings, 0 replies; 35+ messages in thread
From: Matt Roper @ 2020-09-29 23:38 UTC (permalink / raw)
To: Ville Syrjälä
Cc: Pandey, Hariom, intel-gfx, De Marchi, Lucas, dri-devel
On Wed, Sep 30, 2020 at 12:59:58AM +0300, Ville Syrjälä wrote:
> On Wed, Sep 30, 2020 at 12:11:48AM +0300, Ville Syrjälä wrote:
> > On Tue, Sep 29, 2020 at 02:01:44PM -0700, Matt Roper wrote:
> > > On Tue, Sep 29, 2020 at 11:30:22PM +0300, Ville Syrjälä wrote:
> > > > On Tue, Sep 29, 2020 at 08:20:22PM +0000, Souza, Jose wrote:
> > > > > On Tue, 2020-09-29 at 23:02 +0300, Ville Syrjälä wrote:
> > > > > > On Tue, Sep 29, 2020 at 07:33:45PM +0000, Souza, Jose wrote:
> > > > > > > On Tue, 2020-09-29 at 17:41 +0530, Tejas Upadhyay wrote:
> > > > > > > > JSL has update in vswing table for eDP
> > > > > > >
> > > > > > > Would be nice to mention in the commit description why PCH is being used, that would avoid Ville's question.
> > > > > >
> > > > > > If the thing has nothing to do PCH then it should not use the PCH type
> > > > > > for the the check. Instead we should just do the EHL/JSL split.
> > > > >
> > > > > In the first version Matt Roper suggested to use PCH to differentiate between EHL and JSL, Jani also agreed with this solution.This 2 PCHs can only be
> > > > > associate with EHL and JSL respectively, so no downsides here.
> > > >
> > > > The downside is that the code makes no sense on the first glance.
> > > > It's going to generate a "wtf?" exception in the brain and require
> > > > me to take a second look to figure what is going on. Exception
> > > > handling is expensive and shouldn't be needed in cases where it's
> > > > trivial to make the code 100% obvious.
> > >
> > > The bspec documents EHL and JSL as being the same platform and identical
> > > in all programming since they are literally the same display IP; this
> > > vswing table is the one and only place where the two are treated in a
> > > distinct manner for reasons that lie outside the display controller. If
> > > you had to stop and take a closer look at the code here, that's a
> > > probably a good thing since in general there should generally never be a
> > > difference in the behavior between the two. Adding an additional
> > > clarifying comment is probably in order too since this is a very
> > > exceptional special case.
> > >
> > > If we deviate from the bspec's guidance and try to split IS_ELKHARTLAKE
> > > and IS_JASPERLAKE across the whole driver, that's going to be a lot more
> > > pain to maintain down the road since we'll almost certainly have cases
> > > where someone silently leaves one or the other off a condition and gets
> > > unexepcted behavior. I could see arguments for using a SUBPLATFORM here
> > > like we do for TGL_U vs TGL_Y, but even that seems like overkill if we
> > > already have a clear way to distinguish the two cases (PCH pairing) and
> > > can just leave a clarifying comment.
> >
> > That fixed PCH pairing is totally undocumented AFAICS. And vswing has
> > nothing to do with the south display, so the wtf will still happen.
> > Comment or no comment.
>
> Oh and JSP does not show up in bspec even once. MCC appears exactly once
> where it talks about the differences between MCC and ICL-N PCH (which I
> guess is the same as JSP?).
No, ICL-N PCH is something different. :-( There were some early test
chips created that paired the EHL/JSL graphics/media/display IP with an
ICL PCH just for early debug/test purposes, but that pairing isn't
something that actually exists as a real platform.
I think the confusion here arises because most driver developers only
look at (or have access to) the bspec, which does not aim to document
end-user platforms, but rather IP families that the
graphics/media/display hardware IP teams design. The bspec is not an
authoritative document for anything that lies outside the GMD IP itself.
The GMD architects do sometimes try to pull in additional information
from external teams/sources (such as PCH pairing or the electrical
details like the vswing programming here) to make life easier for the
software teams like us that only really deal with the bspec, but that
information comes from external sources, so it's a bit inconsistent in
terms of how much detail there is (or even whether it's described at
all). We could probably file bspec defects to get them to include the
PCH pairing details for EHL/JSL in the bspec, but ultimately EHL="EHL
G/M/D + MCC PCH" and JSL="EHL G/M/D + JSP PCH;" this has already been
confirmed in an offline email thread with the hardware teams.
Elkhart Lake and Jasper Lake are two separate end-user platforms, that
both incorporated the same G/M/D IP design. The name "Jasper Lake"
existed as a codename first, so that's the name that shows up in the
bspec; this wound up being a bit confusing when Elkhart Lake was
actually the first of the two to be released and thus wound up being the
name we have in our code. But the situation seems similar to CHT vs BSW
which are both referred to as "CHV" in the bspec and in our code
(although obviously there was no PCH pairing for those SoCs).
Steppings, workarounds, etc. are unified for EHL/JSL because they're
literally the same IP, rather than one being a derivative of the other.
If you want full details about the PCHs of a platform (most of which is
unimportant to graphics drivers) or the electrical characteristics that
feed into the vswing programming then there are other authoritative
documents that cover that (like the Electrical Design Specification and
such). I'm not sure if those documents are posted anywhere publicly;
fortunately we only need a small amount of information in those areas
and the GMD architects are often nice enough to try to copy the relevant
info into the bspec as a courtesy.
>
> Furthermore the spec never really talks about EHL except in very select
> places. So the IS_ELKHARTLAKE is already totally confusing and IMO more
* > likely to cause maintenance problems than the split. Making it
> IS_JSL||IS_EHL at least gives the reader some hint as to where they
> should look in the spec.
>
> Another argument why the split is fine is CFL/CML. Those are more
> or less exactly in the same boat as EHL. Ie. only really mentioned
> in the "configurations" section. Beyond that only KBL is ever really
> mentioned. And yet we have separate IS_FOOs for all of them, and
> apparently no one had any objections to that situation.
>
> tldr;we have an established way to handle these things, so IMO lets
> just follow it and stop adding special cases.
Isn't CML graphics a derivative of CFL (rather than being exactly the
same IP)? The fact that we have differences in the GMD IP itself that
need different workarounds implies that it's not quite the same
situation we're talking about here (otherwise we'd have been able to
just check the stepping revision ID). IS_CML only got split out from
IS_CFL a couple months ago, so it's probably too soon to see how many
bugs eventually creep in when developers accidentally leave it off a CFL
condition or vice versa.
And we do still unify WHL, AML, etc., in i915 as far as I can see even
though the IP teams track those platforms separately, so the precedent
appears to be keeping them combined as far as I can see?
Matt
>
> --
> Ville Syrjälä
> Intel
--
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
2020-09-29 23:38 ` [Intel-gfx] " Matt Roper
@ 2020-09-30 10:38 ` Ville Syrjälä
-1 siblings, 0 replies; 35+ messages in thread
From: Ville Syrjälä @ 2020-09-30 10:38 UTC (permalink / raw)
To: Matt Roper
Cc: Pandey, Hariom, Ausmus, James, intel-gfx, De Marchi, Lucas,
Souza, Jose, dri-devel, Surendrakumar Upadhyay, TejaskumarX
On Tue, Sep 29, 2020 at 04:38:22PM -0700, Matt Roper wrote:
> On Wed, Sep 30, 2020 at 12:59:58AM +0300, Ville Syrjälä wrote:
> > On Wed, Sep 30, 2020 at 12:11:48AM +0300, Ville Syrjälä wrote:
> > > On Tue, Sep 29, 2020 at 02:01:44PM -0700, Matt Roper wrote:
> > > > On Tue, Sep 29, 2020 at 11:30:22PM +0300, Ville Syrjälä wrote:
> > > > > On Tue, Sep 29, 2020 at 08:20:22PM +0000, Souza, Jose wrote:
> > > > > > On Tue, 2020-09-29 at 23:02 +0300, Ville Syrjälä wrote:
> > > > > > > On Tue, Sep 29, 2020 at 07:33:45PM +0000, Souza, Jose wrote:
> > > > > > > > On Tue, 2020-09-29 at 17:41 +0530, Tejas Upadhyay wrote:
> > > > > > > > > JSL has update in vswing table for eDP
> > > > > > > >
> > > > > > > > Would be nice to mention in the commit description why PCH is being used, that would avoid Ville's question.
> > > > > > >
> > > > > > > If the thing has nothing to do PCH then it should not use the PCH type
> > > > > > > for the the check. Instead we should just do the EHL/JSL split.
> > > > > >
> > > > > > In the first version Matt Roper suggested to use PCH to differentiate between EHL and JSL, Jani also agreed with this solution.This 2 PCHs can only be
> > > > > > associate with EHL and JSL respectively, so no downsides here.
> > > > >
> > > > > The downside is that the code makes no sense on the first glance.
> > > > > It's going to generate a "wtf?" exception in the brain and require
> > > > > me to take a second look to figure what is going on. Exception
> > > > > handling is expensive and shouldn't be needed in cases where it's
> > > > > trivial to make the code 100% obvious.
> > > >
> > > > The bspec documents EHL and JSL as being the same platform and identical
> > > > in all programming since they are literally the same display IP; this
> > > > vswing table is the one and only place where the two are treated in a
> > > > distinct manner for reasons that lie outside the display controller. If
> > > > you had to stop and take a closer look at the code here, that's a
> > > > probably a good thing since in general there should generally never be a
> > > > difference in the behavior between the two. Adding an additional
> > > > clarifying comment is probably in order too since this is a very
> > > > exceptional special case.
> > > >
> > > > If we deviate from the bspec's guidance and try to split IS_ELKHARTLAKE
> > > > and IS_JASPERLAKE across the whole driver, that's going to be a lot more
> > > > pain to maintain down the road since we'll almost certainly have cases
> > > > where someone silently leaves one or the other off a condition and gets
> > > > unexepcted behavior. I could see arguments for using a SUBPLATFORM here
> > > > like we do for TGL_U vs TGL_Y, but even that seems like overkill if we
> > > > already have a clear way to distinguish the two cases (PCH pairing) and
> > > > can just leave a clarifying comment.
> > >
> > > That fixed PCH pairing is totally undocumented AFAICS. And vswing has
> > > nothing to do with the south display, so the wtf will still happen.
> > > Comment or no comment.
> >
> > Oh and JSP does not show up in bspec even once. MCC appears exactly once
> > where it talks about the differences between MCC and ICL-N PCH (which I
> > guess is the same as JSP?).
>
> No, ICL-N PCH is something different. :-( There were some early test
> chips created that paired the EHL/JSL graphics/media/display IP with an
> ICL PCH just for early debug/test purposes, but that pairing isn't
> something that actually exists as a real platform.
>
> I think the confusion here arises because most driver developers only
> look at (or have access to) the bspec, which does not aim to document
> end-user platforms, but rather IP families that the
> graphics/media/display hardware IP teams design. The bspec is not an
> authoritative document for anything that lies outside the GMD IP itself.
> The GMD architects do sometimes try to pull in additional information
> from external teams/sources (such as PCH pairing or the electrical
> details like the vswing programming here) to make life easier for the
> software teams like us that only really deal with the bspec, but that
> information comes from external sources, so it's a bit inconsistent in
> terms of how much detail there is (or even whether it's described at
> all). We could probably file bspec defects to get them to include the
> PCH pairing details for EHL/JSL in the bspec, but ultimately EHL="EHL
> G/M/D + MCC PCH" and JSL="EHL G/M/D + JSP PCH;" this has already been
> confirmed in an offline email thread with the hardware teams.
>
> Elkhart Lake and Jasper Lake are two separate end-user platforms, that
> both incorporated the same G/M/D IP design. The name "Jasper Lake"
> existed as a codename first, so that's the name that shows up in the
> bspec; this wound up being a bit confusing when Elkhart Lake was
> actually the first of the two to be released and thus wound up being the
> name we have in our code. But the situation seems similar to CHT vs BSW
> which are both referred to as "CHV" in the bspec and in our code
> (although obviously there was no PCH pairing for those SoCs).
> Steppings, workarounds, etc. are unified for EHL/JSL because they're
> literally the same IP, rather than one being a derivative of the other.
>
> If you want full details about the PCHs of a platform (most of which is
> unimportant to graphics drivers) or the electrical characteristics that
> feed into the vswing programming then there are other authoritative
> documents that cover that (like the Electrical Design Specification and
> such). I'm not sure if those documents are posted anywhere publicly;
> fortunately we only need a small amount of information in those areas
> and the GMD architects are often nice enough to try to copy the relevant
> info into the bspec as a courtesy.
>
> >
> > Furthermore the spec never really talks about EHL except in very select
> > places. So the IS_ELKHARTLAKE is already totally confusing and IMO more
> * > likely to cause maintenance problems than the split. Making it
> > IS_JSL||IS_EHL at least gives the reader some hint as to where they
> > should look in the spec.
> >
> > Another argument why the split is fine is CFL/CML. Those are more
> > or less exactly in the same boat as EHL. Ie. only really mentioned
> > in the "configurations" section. Beyond that only KBL is ever really
> > mentioned. And yet we have separate IS_FOOs for all of them, and
> > apparently no one had any objections to that situation.
> >
> > tldr;we have an established way to handle these things, so IMO lets
> > just follow it and stop adding special cases.
>
> Isn't CML graphics a derivative of CFL (rather than being exactly the
> same IP)?
No idea, and I don't really see why it would matter anyway
from a driver developer's POV.
> The fact that we have differences in the GMD IP itself that
> need different workarounds implies that it's not quite the same
> situation we're talking about here (otherwise we'd have been able to
> just check the stepping revision ID). IS_CML only got split out from
> IS_CFL a couple months ago, so it's probably too soon to see how many
> bugs eventually creep in when developers accidentally leave it off a CFL
> condition or vice versa.
>
> And we do still unify WHL, AML, etc., in i915 as far as I can see even
> though the IP teams track those platforms separately, so the precedent
> appears to be keeping them combined as far as I can see?
We split when there is any functional difference. Eg. CML was
considered the same as CFL until an actual difference came up,
at which point it was split up.
Now we have an actual difference between EHL and JSL so we
should split. Granted it's a bit annoying to have to do it
just for some vswing tables. Ideally that stuff would be
specified in a sane way by the VBT. But since VBT is generally
useless we need to deal with this on a platform level.
--
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
@ 2020-09-30 10:38 ` Ville Syrjälä
0 siblings, 0 replies; 35+ messages in thread
From: Ville Syrjälä @ 2020-09-30 10:38 UTC (permalink / raw)
To: Matt Roper; +Cc: Pandey, Hariom, intel-gfx, De Marchi, Lucas, dri-devel
On Tue, Sep 29, 2020 at 04:38:22PM -0700, Matt Roper wrote:
> On Wed, Sep 30, 2020 at 12:59:58AM +0300, Ville Syrjälä wrote:
> > On Wed, Sep 30, 2020 at 12:11:48AM +0300, Ville Syrjälä wrote:
> > > On Tue, Sep 29, 2020 at 02:01:44PM -0700, Matt Roper wrote:
> > > > On Tue, Sep 29, 2020 at 11:30:22PM +0300, Ville Syrjälä wrote:
> > > > > On Tue, Sep 29, 2020 at 08:20:22PM +0000, Souza, Jose wrote:
> > > > > > On Tue, 2020-09-29 at 23:02 +0300, Ville Syrjälä wrote:
> > > > > > > On Tue, Sep 29, 2020 at 07:33:45PM +0000, Souza, Jose wrote:
> > > > > > > > On Tue, 2020-09-29 at 17:41 +0530, Tejas Upadhyay wrote:
> > > > > > > > > JSL has update in vswing table for eDP
> > > > > > > >
> > > > > > > > Would be nice to mention in the commit description why PCH is being used, that would avoid Ville's question.
> > > > > > >
> > > > > > > If the thing has nothing to do PCH then it should not use the PCH type
> > > > > > > for the the check. Instead we should just do the EHL/JSL split.
> > > > > >
> > > > > > In the first version Matt Roper suggested to use PCH to differentiate between EHL and JSL, Jani also agreed with this solution.This 2 PCHs can only be
> > > > > > associate with EHL and JSL respectively, so no downsides here.
> > > > >
> > > > > The downside is that the code makes no sense on the first glance.
> > > > > It's going to generate a "wtf?" exception in the brain and require
> > > > > me to take a second look to figure what is going on. Exception
> > > > > handling is expensive and shouldn't be needed in cases where it's
> > > > > trivial to make the code 100% obvious.
> > > >
> > > > The bspec documents EHL and JSL as being the same platform and identical
> > > > in all programming since they are literally the same display IP; this
> > > > vswing table is the one and only place where the two are treated in a
> > > > distinct manner for reasons that lie outside the display controller. If
> > > > you had to stop and take a closer look at the code here, that's a
> > > > probably a good thing since in general there should generally never be a
> > > > difference in the behavior between the two. Adding an additional
> > > > clarifying comment is probably in order too since this is a very
> > > > exceptional special case.
> > > >
> > > > If we deviate from the bspec's guidance and try to split IS_ELKHARTLAKE
> > > > and IS_JASPERLAKE across the whole driver, that's going to be a lot more
> > > > pain to maintain down the road since we'll almost certainly have cases
> > > > where someone silently leaves one or the other off a condition and gets
> > > > unexepcted behavior. I could see arguments for using a SUBPLATFORM here
> > > > like we do for TGL_U vs TGL_Y, but even that seems like overkill if we
> > > > already have a clear way to distinguish the two cases (PCH pairing) and
> > > > can just leave a clarifying comment.
> > >
> > > That fixed PCH pairing is totally undocumented AFAICS. And vswing has
> > > nothing to do with the south display, so the wtf will still happen.
> > > Comment or no comment.
> >
> > Oh and JSP does not show up in bspec even once. MCC appears exactly once
> > where it talks about the differences between MCC and ICL-N PCH (which I
> > guess is the same as JSP?).
>
> No, ICL-N PCH is something different. :-( There were some early test
> chips created that paired the EHL/JSL graphics/media/display IP with an
> ICL PCH just for early debug/test purposes, but that pairing isn't
> something that actually exists as a real platform.
>
> I think the confusion here arises because most driver developers only
> look at (or have access to) the bspec, which does not aim to document
> end-user platforms, but rather IP families that the
> graphics/media/display hardware IP teams design. The bspec is not an
> authoritative document for anything that lies outside the GMD IP itself.
> The GMD architects do sometimes try to pull in additional information
> from external teams/sources (such as PCH pairing or the electrical
> details like the vswing programming here) to make life easier for the
> software teams like us that only really deal with the bspec, but that
> information comes from external sources, so it's a bit inconsistent in
> terms of how much detail there is (or even whether it's described at
> all). We could probably file bspec defects to get them to include the
> PCH pairing details for EHL/JSL in the bspec, but ultimately EHL="EHL
> G/M/D + MCC PCH" and JSL="EHL G/M/D + JSP PCH;" this has already been
> confirmed in an offline email thread with the hardware teams.
>
> Elkhart Lake and Jasper Lake are two separate end-user platforms, that
> both incorporated the same G/M/D IP design. The name "Jasper Lake"
> existed as a codename first, so that's the name that shows up in the
> bspec; this wound up being a bit confusing when Elkhart Lake was
> actually the first of the two to be released and thus wound up being the
> name we have in our code. But the situation seems similar to CHT vs BSW
> which are both referred to as "CHV" in the bspec and in our code
> (although obviously there was no PCH pairing for those SoCs).
> Steppings, workarounds, etc. are unified for EHL/JSL because they're
> literally the same IP, rather than one being a derivative of the other.
>
> If you want full details about the PCHs of a platform (most of which is
> unimportant to graphics drivers) or the electrical characteristics that
> feed into the vswing programming then there are other authoritative
> documents that cover that (like the Electrical Design Specification and
> such). I'm not sure if those documents are posted anywhere publicly;
> fortunately we only need a small amount of information in those areas
> and the GMD architects are often nice enough to try to copy the relevant
> info into the bspec as a courtesy.
>
> >
> > Furthermore the spec never really talks about EHL except in very select
> > places. So the IS_ELKHARTLAKE is already totally confusing and IMO more
> * > likely to cause maintenance problems than the split. Making it
> > IS_JSL||IS_EHL at least gives the reader some hint as to where they
> > should look in the spec.
> >
> > Another argument why the split is fine is CFL/CML. Those are more
> > or less exactly in the same boat as EHL. Ie. only really mentioned
> > in the "configurations" section. Beyond that only KBL is ever really
> > mentioned. And yet we have separate IS_FOOs for all of them, and
> > apparently no one had any objections to that situation.
> >
> > tldr;we have an established way to handle these things, so IMO lets
> > just follow it and stop adding special cases.
>
> Isn't CML graphics a derivative of CFL (rather than being exactly the
> same IP)?
No idea, and I don't really see why it would matter anyway
from a driver developer's POV.
> The fact that we have differences in the GMD IP itself that
> need different workarounds implies that it's not quite the same
> situation we're talking about here (otherwise we'd have been able to
> just check the stepping revision ID). IS_CML only got split out from
> IS_CFL a couple months ago, so it's probably too soon to see how many
> bugs eventually creep in when developers accidentally leave it off a CFL
> condition or vice versa.
>
> And we do still unify WHL, AML, etc., in i915 as far as I can see even
> though the IP teams track those platforms separately, so the precedent
> appears to be keeping them combined as far as I can see?
We split when there is any functional difference. Eg. CML was
considered the same as CFL until an actual difference came up,
at which point it was split up.
Now we have an actual difference between EHL and JSL so we
should split. Granted it's a bit annoying to have to do it
just for some vswing tables. Ideally that stuff would be
specified in a sane way by the VBT. But since VBT is generally
useless we need to deal with this on a platform level.
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 35+ messages in thread
* [Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915/edp/jsl: Update vswing table for HBR and HBR2
2020-09-29 12:11 ` [Intel-gfx] " Tejas Upadhyay
` (4 preceding siblings ...)
(?)
@ 2020-09-30 10:49 ` Patchwork
-1 siblings, 0 replies; 35+ messages in thread
From: Patchwork @ 2020-09-30 10:49 UTC (permalink / raw)
To: Surendrakumar Upadhyay, TejaskumarX; +Cc: intel-gfx
[-- Attachment #1.1: Type: text/plain, Size: 17099 bytes --]
== Series Details ==
Series: drm/i915/edp/jsl: Update vswing table for HBR and HBR2
URL : https://patchwork.freedesktop.org/series/82206/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_9075_full -> Patchwork_18590_full
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_18590_full absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_18590_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_18590_full:
### IGT changes ###
#### Possible regressions ####
* igt@kms_atomic_transition@plane-toggle-modeset-transition:
- shard-tglb: [PASS][1] -> [INCOMPLETE][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9075/shard-tglb7/igt@kms_atomic_transition@plane-toggle-modeset-transition.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18590/shard-tglb6/igt@kms_atomic_transition@plane-toggle-modeset-transition.html
Known issues
------------
Here are the changes found in Patchwork_18590_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_exec_reloc@basic-many-active@rcs0:
- shard-glk: [PASS][3] -> [FAIL][4] ([i915#2389])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9075/shard-glk4/igt@gem_exec_reloc@basic-many-active@rcs0.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18590/shard-glk7/igt@gem_exec_reloc@basic-many-active@rcs0.html
* igt@gem_ppgtt@flink-and-close-vma-leak:
- shard-glk: [PASS][5] -> [FAIL][6] ([i915#644])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9075/shard-glk5/igt@gem_ppgtt@flink-and-close-vma-leak.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18590/shard-glk2/igt@gem_ppgtt@flink-and-close-vma-leak.html
* igt@i915_selftest@mock@contexts:
- shard-apl: [PASS][7] -> [INCOMPLETE][8] ([i915#1635] / [i915#2278])
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9075/shard-apl3/igt@i915_selftest@mock@contexts.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18590/shard-apl6/igt@i915_selftest@mock@contexts.html
* igt@kms_big_fb@x-tiled-64bpp-rotate-0:
- shard-iclb: [PASS][9] -> [DMESG-WARN][10] ([i915#1982])
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9075/shard-iclb2/igt@kms_big_fb@x-tiled-64bpp-rotate-0.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18590/shard-iclb1/igt@kms_big_fb@x-tiled-64bpp-rotate-0.html
* igt@kms_big_fb@x-tiled-8bpp-rotate-0:
- shard-kbl: [PASS][11] -> [DMESG-WARN][12] ([i915#1982])
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9075/shard-kbl3/igt@kms_big_fb@x-tiled-8bpp-rotate-0.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18590/shard-kbl4/igt@kms_big_fb@x-tiled-8bpp-rotate-0.html
* igt@kms_flip@flip-vs-expired-vblank-interruptible@b-hdmi-a1:
- shard-glk: [PASS][13] -> [FAIL][14] ([i915#2122])
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9075/shard-glk9/igt@kms_flip@flip-vs-expired-vblank-interruptible@b-hdmi-a1.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18590/shard-glk1/igt@kms_flip@flip-vs-expired-vblank-interruptible@b-hdmi-a1.html
* igt@kms_flip@flip-vs-expired-vblank@a-edp1:
- shard-skl: [PASS][15] -> [FAIL][16] ([i915#79]) +1 similar issue
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9075/shard-skl10/igt@kms_flip@flip-vs-expired-vblank@a-edp1.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18590/shard-skl7/igt@kms_flip@flip-vs-expired-vblank@a-edp1.html
* igt@kms_flip@plain-flip-ts-check@b-edp1:
- shard-skl: [PASS][17] -> [FAIL][18] ([i915#2122])
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9075/shard-skl6/igt@kms_flip@plain-flip-ts-check@b-edp1.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18590/shard-skl2/igt@kms_flip@plain-flip-ts-check@b-edp1.html
* igt@kms_frontbuffer_tracking@fbc-rgb101010-draw-mmap-wc:
- shard-apl: [PASS][19] -> [DMESG-WARN][20] ([i915#1635] / [i915#1982])
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9075/shard-apl7/igt@kms_frontbuffer_tracking@fbc-rgb101010-draw-mmap-wc.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18590/shard-apl6/igt@kms_frontbuffer_tracking@fbc-rgb101010-draw-mmap-wc.html
* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render:
- shard-tglb: [PASS][21] -> [DMESG-WARN][22] ([i915#1982])
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9075/shard-tglb5/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18590/shard-tglb6/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render.html
* igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-shrfb-draw-render:
- shard-skl: [PASS][23] -> [DMESG-WARN][24] ([i915#1982]) +9 similar issues
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9075/shard-skl5/igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-shrfb-draw-render.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18590/shard-skl2/igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-shrfb-draw-render.html
* igt@kms_hdr@bpc-switch-suspend:
- shard-skl: [PASS][25] -> [FAIL][26] ([i915#1188])
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9075/shard-skl1/igt@kms_hdr@bpc-switch-suspend.html
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18590/shard-skl4/igt@kms_hdr@bpc-switch-suspend.html
* igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
- shard-skl: [PASS][27] -> [FAIL][28] ([fdo#108145] / [i915#265])
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9075/shard-skl1/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18590/shard-skl5/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
* igt@kms_psr@psr2_primary_render:
- shard-iclb: [PASS][29] -> [SKIP][30] ([fdo#109441])
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9075/shard-iclb2/igt@kms_psr@psr2_primary_render.html
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18590/shard-iclb1/igt@kms_psr@psr2_primary_render.html
* igt@kms_setmode@basic:
- shard-apl: [PASS][31] -> [FAIL][32] ([i915#1635] / [i915#31])
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9075/shard-apl8/igt@kms_setmode@basic.html
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18590/shard-apl8/igt@kms_setmode@basic.html
* igt@kms_vblank@pipe-a-ts-continuation-dpms-suspend:
- shard-skl: [PASS][33] -> [INCOMPLETE][34] ([i915#198])
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9075/shard-skl7/igt@kms_vblank@pipe-a-ts-continuation-dpms-suspend.html
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18590/shard-skl6/igt@kms_vblank@pipe-a-ts-continuation-dpms-suspend.html
* igt@kms_vblank@pipe-a-ts-continuation-suspend:
- shard-kbl: [PASS][35] -> [DMESG-WARN][36] ([i915#180]) +4 similar issues
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9075/shard-kbl6/igt@kms_vblank@pipe-a-ts-continuation-suspend.html
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18590/shard-kbl7/igt@kms_vblank@pipe-a-ts-continuation-suspend.html
#### Possible fixes ####
* {igt@gem_ctx_persistence@heartbeat-many}:
- shard-kbl: [FAIL][37] ([i915#2520]) -> [PASS][38]
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9075/shard-kbl4/igt@gem_ctx_persistence@heartbeat-many.html
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18590/shard-kbl7/igt@gem_ctx_persistence@heartbeat-many.html
- shard-tglb: [FAIL][39] ([i915#2520]) -> [PASS][40]
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9075/shard-tglb3/igt@gem_ctx_persistence@heartbeat-many.html
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18590/shard-tglb6/igt@gem_ctx_persistence@heartbeat-many.html
* igt@gem_exec_create@forked:
- shard-glk: [DMESG-WARN][41] ([i915#118] / [i915#95]) -> [PASS][42]
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9075/shard-glk6/igt@gem_exec_create@forked.html
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18590/shard-glk5/igt@gem_exec_create@forked.html
* igt@gem_exec_whisper@basic-forked-all:
- shard-skl: [DMESG-WARN][43] ([i915#1982]) -> [PASS][44] +11 similar issues
[43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9075/shard-skl8/igt@gem_exec_whisper@basic-forked-all.html
[44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18590/shard-skl1/igt@gem_exec_whisper@basic-forked-all.html
* igt@i915_selftest@live@gt_heartbeat:
- shard-skl: [DMESG-FAIL][45] ([i915#541]) -> [PASS][46]
[45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9075/shard-skl8/igt@i915_selftest@live@gt_heartbeat.html
[46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18590/shard-skl1/igt@i915_selftest@live@gt_heartbeat.html
* igt@i915_selftest@mock@contexts:
- shard-skl: [INCOMPLETE][47] ([i915#198] / [i915#2278]) -> [PASS][48]
[47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9075/shard-skl1/igt@i915_selftest@mock@contexts.html
[48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18590/shard-skl5/igt@i915_selftest@mock@contexts.html
* igt@i915_suspend@debugfs-reader:
- shard-skl: [INCOMPLETE][49] ([i915#198]) -> [PASS][50]
[49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9075/shard-skl6/igt@i915_suspend@debugfs-reader.html
[50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18590/shard-skl8/igt@i915_suspend@debugfs-reader.html
* {igt@kms_async_flips@async-flip-with-page-flip-events}:
- shard-glk: [FAIL][51] ([i915#2521]) -> [PASS][52]
[51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9075/shard-glk7/igt@kms_async_flips@async-flip-with-page-flip-events.html
[52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18590/shard-glk8/igt@kms_async_flips@async-flip-with-page-flip-events.html
* igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions:
- shard-skl: [FAIL][53] ([i915#2346]) -> [PASS][54]
[53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9075/shard-skl5/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html
[54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18590/shard-skl6/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html
* igt@kms_cursor_legacy@short-flip-before-cursor-atomic-transitions-varying-size:
- shard-apl: [DMESG-WARN][55] ([i915#1635] / [i915#1982]) -> [PASS][56]
[55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9075/shard-apl7/igt@kms_cursor_legacy@short-flip-before-cursor-atomic-transitions-varying-size.html
[56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18590/shard-apl6/igt@kms_cursor_legacy@short-flip-before-cursor-atomic-transitions-varying-size.html
- shard-kbl: [DMESG-WARN][57] ([i915#1982]) -> [PASS][58]
[57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9075/shard-kbl3/igt@kms_cursor_legacy@short-flip-before-cursor-atomic-transitions-varying-size.html
[58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18590/shard-kbl6/igt@kms_cursor_legacy@short-flip-before-cursor-atomic-transitions-varying-size.html
* igt@kms_flip@flip-vs-expired-vblank-interruptible@b-dp1:
- shard-apl: [FAIL][59] ([i915#1635] / [i915#79]) -> [PASS][60]
[59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9075/shard-apl6/igt@kms_flip@flip-vs-expired-vblank-interruptible@b-dp1.html
[60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18590/shard-apl7/igt@kms_flip@flip-vs-expired-vblank-interruptible@b-dp1.html
* igt@kms_flip@flip-vs-suspend-interruptible@a-dp1:
- shard-kbl: [DMESG-WARN][61] ([i915#180]) -> [PASS][62] +6 similar issues
[61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9075/shard-kbl7/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html
[62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18590/shard-kbl6/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html
* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-blt:
- shard-tglb: [DMESG-WARN][63] ([i915#1982]) -> [PASS][64]
[63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9075/shard-tglb3/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-blt.html
[64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18590/shard-tglb6/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-blt.html
* igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
- shard-skl: [FAIL][65] ([fdo#108145] / [i915#265]) -> [PASS][66] +1 similar issue
[65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9075/shard-skl6/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
[66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18590/shard-skl2/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
* igt@kms_psr@psr2_sprite_blt:
- shard-iclb: [SKIP][67] ([fdo#109441]) -> [PASS][68] +1 similar issue
[67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9075/shard-iclb1/igt@kms_psr@psr2_sprite_blt.html
[68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18590/shard-iclb2/igt@kms_psr@psr2_sprite_blt.html
#### Warnings ####
* igt@kms_content_protection@atomic-dpms:
- shard-apl: [TIMEOUT][69] ([i915#1319] / [i915#1635] / [i915#1958]) -> [TIMEOUT][70] ([i915#1319] / [i915#1635])
[69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9075/shard-apl6/igt@kms_content_protection@atomic-dpms.html
[70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18590/shard-apl7/igt@kms_content_protection@atomic-dpms.html
* igt@kms_setmode@basic:
- shard-skl: [DMESG-FAIL][71] ([i915#1982] / [i915#31]) -> [FAIL][72] ([i915#31])
[71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9075/shard-skl7/igt@kms_setmode@basic.html
[72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18590/shard-skl6/igt@kms_setmode@basic.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
[fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
[i915#118]: https://gitlab.freedesktop.org/drm/intel/issues/118
[i915#1188]: https://gitlab.freedesktop.org/drm/intel/issues/1188
[i915#1319]: https://gitlab.freedesktop.org/drm/intel/issues/1319
[i915#1635]: https://gitlab.freedesktop.org/drm/intel/issues/1635
[i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
[i915#1958]: https://gitlab.freedesktop.org/drm/intel/issues/1958
[i915#198]: https://gitlab.freedesktop.org/drm/intel/issues/198
[i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
[i915#2122]: https://gitlab.freedesktop.org/drm/intel/issues/2122
[i915#2278]: https://gitlab.freedesktop.org/drm/intel/issues/2278
[i915#2346]: https://gitlab.freedesktop.org/drm/intel/issues/2346
[i915#2389]: https://gitlab.freedesktop.org/drm/intel/issues/2389
[i915#2469]: https://gitlab.freedesktop.org/drm/intel/issues/2469
[i915#2476]: https://gitlab.freedesktop.org/drm/intel/issues/2476
[i915#2519]: https://gitlab.freedesktop.org/drm/intel/issues/2519
[i915#2520]: https://gitlab.freedesktop.org/drm/intel/issues/2520
[i915#2521]: https://gitlab.freedesktop.org/drm/intel/issues/2521
[i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
[i915#31]: https://gitlab.freedesktop.org/drm/intel/issues/31
[i915#541]: https://gitlab.freedesktop.org/drm/intel/issues/541
[i915#644]: https://gitlab.freedesktop.org/drm/intel/issues/644
[i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
[i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95
Participating hosts (11 -> 11)
------------------------------
No changes in participating hosts
Build changes
-------------
* Linux: CI_DRM_9075 -> Patchwork_18590
CI-20190529: 20190529
CI_DRM_9075: fd24361b2b76956b5c056bc430a4c77edecb7744 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5792: cbaf441899f3b4f36cca5996aa6a69e7399b2dbd @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_18590: 548dfee5bc0ff01e16e2313ef9193950370b1a9c @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18590/index.html
[-- Attachment #1.2: Type: text/html, Size: 20251 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
2020-09-29 20:20 ` [Intel-gfx] " Souza, Jose
@ 2020-09-30 12:31 ` Jani Nikula
-1 siblings, 0 replies; 35+ messages in thread
From: Jani Nikula @ 2020-09-30 12:31 UTC (permalink / raw)
To: Souza, Jose, ville.syrjala
Cc: Pandey, Hariom, Ausmus, James, intel-gfx, De Marchi, Lucas,
dri-devel, Surendrakumar Upadhyay, TejaskumarX
On Tue, 29 Sep 2020, "Souza, Jose" <jose.souza@intel.com> wrote:
> On Tue, 2020-09-29 at 23:02 +0300, Ville Syrjälä wrote:
>> If the thing has nothing to do PCH then it should not use the PCH type
>> for the the check. Instead we should just do the EHL/JSL split.
>
> In the first version Matt Roper suggested to use PCH to differentiate
> between EHL and JSL, Jani also agreed with this solution.This 2 PCHs
> can only be associate with EHL and JSL respectively, so no downsides
> here.
FWIW I said, "If the difference is in the PCH", without pondering
further.
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
@ 2020-09-30 12:31 ` Jani Nikula
0 siblings, 0 replies; 35+ messages in thread
From: Jani Nikula @ 2020-09-30 12:31 UTC (permalink / raw)
To: Souza, Jose, ville.syrjala
Cc: Pandey, Hariom, intel-gfx, De Marchi, Lucas, dri-devel
On Tue, 29 Sep 2020, "Souza, Jose" <jose.souza@intel.com> wrote:
> On Tue, 2020-09-29 at 23:02 +0300, Ville Syrjälä wrote:
>> If the thing has nothing to do PCH then it should not use the PCH type
>> for the the check. Instead we should just do the EHL/JSL split.
>
> In the first version Matt Roper suggested to use PCH to differentiate
> between EHL and JSL, Jani also agreed with this solution.This 2 PCHs
> can only be associate with EHL and JSL respectively, so no downsides
> here.
FWIW I said, "If the difference is in the PCH", without pondering
further.
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
2020-09-30 10:38 ` [Intel-gfx] " Ville Syrjälä
@ 2020-09-30 12:57 ` Jani Nikula
-1 siblings, 0 replies; 35+ messages in thread
From: Jani Nikula @ 2020-09-30 12:57 UTC (permalink / raw)
To: Ville Syrjälä, Matt Roper
Cc: Pandey, Hariom, Ausmus, James, intel-gfx, De Marchi, Lucas,
dri-devel, Souza, Jose, Surendrakumar Upadhyay, TejaskumarX
On Wed, 30 Sep 2020, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> Now we have an actual difference between EHL and JSL so we
> should split. Granted it's a bit annoying to have to do it
> just for some vswing tables. Ideally that stuff would be
> specified in a sane way by the VBT. But since VBT is generally
> useless we need to deal with this on a platform level.
Just to recap, we have three basic approaches for differentiating
platforms based on PCI ID:
- Separate platforms, each with their own device info and enum
intel_platform, using IS_<PLATFORM>() for checks.
- Same platform, with subplatforms, using IS_SUBPLATFORM() for
checks. Generally only used for the ULT/ULX checks, but there's also
the CNL/ICL port F case which is perhaps comparable.
- Same platform, each with their own device info, and a feature flag.
(In this case, checking the PCH is a proxy; there is no actual
difference in the PCHs to account for the different values to be
used. Mixing PCHs with the platforms would lead to problems.)
We've been told JSL and EHL are the same, which would argue for keeping
them INTEL_ELKHARTLAKE. We've done this with other platforms that are
identical. However, now it looks like they're not the same... why not if
they're supposed to be identical? What else is there?
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
@ 2020-09-30 12:57 ` Jani Nikula
0 siblings, 0 replies; 35+ messages in thread
From: Jani Nikula @ 2020-09-30 12:57 UTC (permalink / raw)
To: Ville Syrjälä, Matt Roper
Cc: Pandey, Hariom, intel-gfx, De Marchi, Lucas, dri-devel
On Wed, 30 Sep 2020, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> Now we have an actual difference between EHL and JSL so we
> should split. Granted it's a bit annoying to have to do it
> just for some vswing tables. Ideally that stuff would be
> specified in a sane way by the VBT. But since VBT is generally
> useless we need to deal with this on a platform level.
Just to recap, we have three basic approaches for differentiating
platforms based on PCI ID:
- Separate platforms, each with their own device info and enum
intel_platform, using IS_<PLATFORM>() for checks.
- Same platform, with subplatforms, using IS_SUBPLATFORM() for
checks. Generally only used for the ULT/ULX checks, but there's also
the CNL/ICL port F case which is perhaps comparable.
- Same platform, each with their own device info, and a feature flag.
(In this case, checking the PCH is a proxy; there is no actual
difference in the PCHs to account for the different values to be
used. Mixing PCHs with the platforms would lead to problems.)
We've been told JSL and EHL are the same, which would argue for keeping
them INTEL_ELKHARTLAKE. We've done this with other platforms that are
identical. However, now it looks like they're not the same... why not if
they're supposed to be identical? What else is there?
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
2020-09-30 12:57 ` [Intel-gfx] " Jani Nikula
@ 2020-09-30 18:20 ` Matt Roper
-1 siblings, 0 replies; 35+ messages in thread
From: Matt Roper @ 2020-09-30 18:20 UTC (permalink / raw)
To: Jani Nikula
Cc: Pandey, Hariom, Ausmus, James, intel-gfx, De Marchi, Lucas,
Souza, Jose, dri-devel, Surendrakumar Upadhyay, TejaskumarX
On Wed, Sep 30, 2020 at 03:57:58PM +0300, Jani Nikula wrote:
> On Wed, 30 Sep 2020, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > Now we have an actual difference between EHL and JSL so we
> > should split. Granted it's a bit annoying to have to do it
> > just for some vswing tables. Ideally that stuff would be
> > specified in a sane way by the VBT. But since VBT is generally
> > useless we need to deal with this on a platform level.
>
> Just to recap, we have three basic approaches for differentiating
> platforms based on PCI ID:
>
> - Separate platforms, each with their own device info and enum
> intel_platform, using IS_<PLATFORM>() for checks.
>
> - Same platform, with subplatforms, using IS_SUBPLATFORM() for
> checks. Generally only used for the ULT/ULX checks, but there's also
> the CNL/ICL port F case which is perhaps comparable.
>
> - Same platform, each with their own device info, and a feature flag.
>
> (In this case, checking the PCH is a proxy; there is no actual
> difference in the PCHs to account for the different values to be
> used. Mixing PCHs with the platforms would lead to problems.)
>
> We've been told JSL and EHL are the same, which would argue for keeping
> them INTEL_ELKHARTLAKE. We've done this with other platforms that are
> identical. However, now it looks like they're not the same... why not if
> they're supposed to be identical? What else is there?
My understanding is that they are identical, but the design guidelines
for the *motherboards* that they will plug into are different, which
necessitates different electrical tuning values to guarantee clean
display signals. Ville's right that it would be nice if this kind of
stuff was just available from something like the VBT instead of being
hardcoded into the driver, but sadly that's just not the case today.
So yes, none of this is related to the South Display which is the only
place we usually care about the PCH in the graphics driver. But PCH is
correlated with board type, which is why I suggested matching on the PCH
in the first place.
If we really want to split these two platforms then I'd suggest we add a
new macro like
#define IS_EHL_JSL(i915) ( \
IS_PLATFORM(dev_priv, INTEL_ELKHARTLAKE) || \
IS_PLATFORM(dev_priv, INTEL_JASPERLAKE))
and use that everywhere else in the driver. For the vswing code itself
we'd just do a direct IS_PLATFORM() check with just one platform or the
other provided; no need to add IS_ELKHARTLAKE/IS_JASPERLAKE macros in
that case since it would be a bug to differentiate between the two
anywhere else in the driver.
Matt
>
> BR,
> Jani.
>
>
> --
> Jani Nikula, Intel Open Source Graphics Center
--
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
@ 2020-09-30 18:20 ` Matt Roper
0 siblings, 0 replies; 35+ messages in thread
From: Matt Roper @ 2020-09-30 18:20 UTC (permalink / raw)
To: Jani Nikula; +Cc: Pandey, Hariom, intel-gfx, De Marchi, Lucas, dri-devel
On Wed, Sep 30, 2020 at 03:57:58PM +0300, Jani Nikula wrote:
> On Wed, 30 Sep 2020, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > Now we have an actual difference between EHL and JSL so we
> > should split. Granted it's a bit annoying to have to do it
> > just for some vswing tables. Ideally that stuff would be
> > specified in a sane way by the VBT. But since VBT is generally
> > useless we need to deal with this on a platform level.
>
> Just to recap, we have three basic approaches for differentiating
> platforms based on PCI ID:
>
> - Separate platforms, each with their own device info and enum
> intel_platform, using IS_<PLATFORM>() for checks.
>
> - Same platform, with subplatforms, using IS_SUBPLATFORM() for
> checks. Generally only used for the ULT/ULX checks, but there's also
> the CNL/ICL port F case which is perhaps comparable.
>
> - Same platform, each with their own device info, and a feature flag.
>
> (In this case, checking the PCH is a proxy; there is no actual
> difference in the PCHs to account for the different values to be
> used. Mixing PCHs with the platforms would lead to problems.)
>
> We've been told JSL and EHL are the same, which would argue for keeping
> them INTEL_ELKHARTLAKE. We've done this with other platforms that are
> identical. However, now it looks like they're not the same... why not if
> they're supposed to be identical? What else is there?
My understanding is that they are identical, but the design guidelines
for the *motherboards* that they will plug into are different, which
necessitates different electrical tuning values to guarantee clean
display signals. Ville's right that it would be nice if this kind of
stuff was just available from something like the VBT instead of being
hardcoded into the driver, but sadly that's just not the case today.
So yes, none of this is related to the South Display which is the only
place we usually care about the PCH in the graphics driver. But PCH is
correlated with board type, which is why I suggested matching on the PCH
in the first place.
If we really want to split these two platforms then I'd suggest we add a
new macro like
#define IS_EHL_JSL(i915) ( \
IS_PLATFORM(dev_priv, INTEL_ELKHARTLAKE) || \
IS_PLATFORM(dev_priv, INTEL_JASPERLAKE))
and use that everywhere else in the driver. For the vswing code itself
we'd just do a direct IS_PLATFORM() check with just one platform or the
other provided; no need to add IS_ELKHARTLAKE/IS_JASPERLAKE macros in
that case since it would be a bug to differentiate between the two
anywhere else in the driver.
Matt
>
> BR,
> Jani.
>
>
> --
> Jani Nikula, Intel Open Source Graphics Center
--
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 35+ messages in thread
end of thread, other threads:[~2020-09-30 18:21 UTC | newest]
Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-29 12:11 [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2 Tejas Upadhyay
2020-09-29 12:11 ` [Intel-gfx] " Tejas Upadhyay
2020-09-29 12:52 ` Ville Syrjälä
2020-09-29 12:52 ` [Intel-gfx] " Ville Syrjälä
2020-09-29 13:17 ` Surendrakumar Upadhyay, TejaskumarX
2020-09-29 13:17 ` [Intel-gfx] " Surendrakumar Upadhyay, TejaskumarX
2020-09-29 17:51 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2020-09-29 18:13 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-09-29 19:33 ` [PATCH v2] " Souza, Jose
2020-09-29 19:33 ` [Intel-gfx] " Souza, Jose
2020-09-29 20:02 ` Ville Syrjälä
2020-09-29 20:02 ` [Intel-gfx] " Ville Syrjälä
2020-09-29 20:20 ` Souza, Jose
2020-09-29 20:20 ` [Intel-gfx] " Souza, Jose
2020-09-29 20:30 ` Ville Syrjälä
2020-09-29 20:30 ` [Intel-gfx] " Ville Syrjälä
2020-09-29 20:34 ` Souza, Jose
2020-09-29 20:34 ` [Intel-gfx] " Souza, Jose
2020-09-29 21:01 ` Matt Roper
2020-09-29 21:01 ` [Intel-gfx] " Matt Roper
2020-09-29 21:11 ` Ville Syrjälä
2020-09-29 21:11 ` [Intel-gfx] " Ville Syrjälä
2020-09-29 21:59 ` Ville Syrjälä
2020-09-29 21:59 ` [Intel-gfx] " Ville Syrjälä
2020-09-29 23:38 ` Matt Roper
2020-09-29 23:38 ` [Intel-gfx] " Matt Roper
2020-09-30 10:38 ` Ville Syrjälä
2020-09-30 10:38 ` [Intel-gfx] " Ville Syrjälä
2020-09-30 12:57 ` Jani Nikula
2020-09-30 12:57 ` [Intel-gfx] " Jani Nikula
2020-09-30 18:20 ` Matt Roper
2020-09-30 18:20 ` [Intel-gfx] " Matt Roper
2020-09-30 12:31 ` Jani Nikula
2020-09-30 12:31 ` [Intel-gfx] " Jani Nikula
2020-09-30 10:49 ` [Intel-gfx] ✗ Fi.CI.IGT: failure for " Patchwork
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.