[Intel-gfx] [PATCH 1/2] drm/i915/opregion: add support for mailbox #5 EDID

[Intel-gfx] [PATCH 1/2] drm/i915/opregion: add support for mailbox #5 EDID

[Jani Nikula]

The ACPI OpRegion Mailbox #5 ASLE extension may contain an EDID to be
used for the embedded display. Add support for using it via the EDID
override mechanism.

Note that the override EDID may be later reset or changed via debugfs,
as usual.

Cc: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_opregion.c | 46 ++++++++++++++++++-
 drivers/gpu/drm/i915/display/intel_opregion.h |  8 ++++
 2 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_opregion.c b/drivers/gpu/drm/i915/display/intel_opregion.c
index de995362f428..13485969fafa 100644
--- a/drivers/gpu/drm/i915/display/intel_opregion.c
+++ b/drivers/gpu/drm/i915/display/intel_opregion.c
@@ -196,6 +196,8 @@ struct opregion_asle_ext {
 #define ASLE_IUER_WINDOWS_BTN		(1 << 1)
 #define ASLE_IUER_POWER_BTN		(1 << 0)
 
+#define ASLE_PHED_EDID_VALID_MASK	0x3
+
 /* Software System Control Interrupt (SWSCI) */
 #define SWSCI_SCIC_INDICATOR		(1 << 0)
 #define SWSCI_SCIC_MAIN_FUNCTION_SHIFT	1
@@ -909,8 +911,10 @@ int intel_opregion_setup(struct drm_i915_private *dev_priv)
 		opregion->asle->ardy = ASLE_ARDY_NOT_READY;
 	}
 
-	if (mboxes & MBOX_ASLE_EXT)
+	if (mboxes & MBOX_ASLE_EXT) {
 		drm_dbg(&dev_priv->drm, "ASLE extension supported\n");
+		opregion->asle_ext = base + OPREGION_ASLE_EXT_OFFSET;
+	}
 
 	if (intel_load_vbt_firmware(dev_priv) == 0)
 		goto out;
@@ -1041,6 +1045,45 @@ intel_opregion_get_panel_type(struct drm_i915_private *dev_priv)
 	return ret - 1;
 }
 
+void intel_opregion_edid_override(struct intel_connector *intel_connector)
+{
+	struct drm_connector *connector = &intel_connector->base;
+	struct drm_i915_private *i915 = to_i915(connector->dev);
+	struct intel_opregion *opregion = &i915->opregion;
+	const void *in_edid;
+	const struct edid *edid;
+	int len, ret;
+
+	if (!opregion->asle_ext)
+		return;
+
+	in_edid = opregion->asle_ext->bddc;
+
+	/* Validity corresponds to number of 128-byte blocks */
+	len = (opregion->asle_ext->phed & ASLE_PHED_EDID_VALID_MASK) * 128;
+	if (!len || !memchr_inv(in_edid, 0, len))
+		return;
+
+	edid = in_edid;
+
+	/*
+	 * FIXME: Might also check drm_edid_is_valid(edid) here but that
+	 * requires mutable edid.
+	 */
+	if (len < EDID_LENGTH * (1 + edid->extensions)) {
+		drm_dbg_kms(&i915->drm, "Invalid EDID in ACPI OpRegion (Mailbox #5)\n");
+		return;
+	}
+
+	connector->override_edid = false;
+	ret = drm_connector_update_edid_property(connector, edid);
+	if (ret)
+		return;
+
+	drm_dbg_kms(&i915->drm, "Using OpRegion EDID for [CONNECTOR:%d:%s]\n",
+		    connector->base.id, connector->name);
+}
+
 void intel_opregion_register(struct drm_i915_private *i915)
 {
 	struct intel_opregion *opregion = &i915->opregion;
@@ -1131,6 +1174,7 @@ void intel_opregion_unregister(struct drm_i915_private *i915)
 	opregion->acpi = NULL;
 	opregion->swsci = NULL;
 	opregion->asle = NULL;
+	opregion->asle_ext = NULL;
 	opregion->vbt = NULL;
 	opregion->lid_state = NULL;
 }
diff --git a/drivers/gpu/drm/i915/display/intel_opregion.h b/drivers/gpu/drm/i915/display/intel_opregion.h
index 4aa68ffbd30e..b407a0744c40 100644
--- a/drivers/gpu/drm/i915/display/intel_opregion.h
+++ b/drivers/gpu/drm/i915/display/intel_opregion.h
@@ -29,12 +29,14 @@
 #include <linux/pci.h>
 
 struct drm_i915_private;
+struct intel_connector;
 struct intel_encoder;
 
 struct opregion_header;
 struct opregion_acpi;
 struct opregion_swsci;
 struct opregion_asle;
+struct opregion_asle_ext;
 
 struct intel_opregion {
 	struct opregion_header *header;
@@ -43,6 +45,7 @@ struct intel_opregion {
 	u32 swsci_gbda_sub_functions;
 	u32 swsci_sbcb_sub_functions;
 	struct opregion_asle *asle;
+	struct opregion_asle_ext *asle_ext;
 	void *rvda;
 	void *vbt_firmware;
 	const void *vbt;
@@ -71,6 +74,7 @@ int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
 int intel_opregion_notify_adapter(struct drm_i915_private *dev_priv,
 				  pci_power_t state);
 int intel_opregion_get_panel_type(struct drm_i915_private *dev_priv);
+void intel_opregion_edid_override(struct intel_connector *connector);
 
 #else /* CONFIG_ACPI*/
 
@@ -117,6 +121,10 @@ static inline int intel_opregion_get_panel_type(struct drm_i915_private *dev)
 	return -ENODEV;
 }
 
+void intel_opregion_edid_override(struct intel_connector *connector)
+{
+}
+
 #endif /* CONFIG_ACPI */
 
 #endif
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 2/2] drm/i915/dp: use opregion mailbox #5 EDID for eDP, if available

[Jani Nikula]

If a panel's EDID is broken, there may be an override EDID set in the
ACPI OpRegion mailbox #5. Use it if available.

Cc: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index c57ac83bf563..d1307be196a2 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -8114,6 +8114,9 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
 		goto out_vdd_off;
 	}
 
+	/* Set up override EDID, if any, from ACPI OpRegion */
+	intel_opregion_edid_override(intel_connector);
+
 	mutex_lock(&dev->mode_config.mutex);
 	edid = drm_get_edid(connector, &intel_dp->aux.ddc);
 	if (edid) {
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/2] drm/i915/dp: use opregion mailbox #5 EDID for eDP, if available

[Shankar, Uma]

> -----Original Message-----
> From: Jani Nikula <jani.nikula@intel.com>
> Sent: Friday, August 28, 2020 11:50 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: Nikula, Jani <jani.nikula@intel.com>; Shankar, Uma
> <uma.shankar@intel.com>
> Subject: [PATCH 2/2] drm/i915/dp: use opregion mailbox #5 EDID for eDP, if
> available
> 
> If a panel's EDID is broken, there may be an override EDID set in the ACPI
> OpRegion mailbox #5. Use it if available.

Looks Good to me.
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

> Cc: Uma Shankar <uma.shankar@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index c57ac83bf563..d1307be196a2 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -8114,6 +8114,9 @@ static bool intel_edp_init_connector(struct intel_dp
> *intel_dp,
>  		goto out_vdd_off;
>  	}
> 
> +	/* Set up override EDID, if any, from ACPI OpRegion */
> +	intel_opregion_edid_override(intel_connector);
> +
>  	mutex_lock(&dev->mode_config.mutex);
>  	edid = drm_get_edid(connector, &intel_dp->aux.ddc);
>  	if (edid) {
> --
> 2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm/i915/opregion: add support for mailbox #5 EDID

[Jani Nikula]

On Fri, 28 Aug 2020, Jani Nikula <jani.nikula@intel.com> wrote:
> The ACPI OpRegion Mailbox #5 ASLE extension may contain an EDID to be
> used for the embedded display. Add support for using it via the EDID
> override mechanism.
>
> Note that the override EDID may be later reset or changed via debugfs,
> as usual.
>
> Cc: Uma Shankar <uma.shankar@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_opregion.c | 46 ++++++++++++++++++-
>  drivers/gpu/drm/i915/display/intel_opregion.h |  8 ++++
>  2 files changed, 53 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_opregion.c b/drivers/gpu/drm/i915/display/intel_opregion.c
> index de995362f428..13485969fafa 100644
> --- a/drivers/gpu/drm/i915/display/intel_opregion.c
> +++ b/drivers/gpu/drm/i915/display/intel_opregion.c
> @@ -196,6 +196,8 @@ struct opregion_asle_ext {
>  #define ASLE_IUER_WINDOWS_BTN		(1 << 1)
>  #define ASLE_IUER_POWER_BTN		(1 << 0)
>  
> +#define ASLE_PHED_EDID_VALID_MASK	0x3
> +
>  /* Software System Control Interrupt (SWSCI) */
>  #define SWSCI_SCIC_INDICATOR		(1 << 0)
>  #define SWSCI_SCIC_MAIN_FUNCTION_SHIFT	1
> @@ -909,8 +911,10 @@ int intel_opregion_setup(struct drm_i915_private *dev_priv)
>  		opregion->asle->ardy = ASLE_ARDY_NOT_READY;
>  	}
>  
> -	if (mboxes & MBOX_ASLE_EXT)
> +	if (mboxes & MBOX_ASLE_EXT) {
>  		drm_dbg(&dev_priv->drm, "ASLE extension supported\n");
> +		opregion->asle_ext = base + OPREGION_ASLE_EXT_OFFSET;
> +	}
>  
>  	if (intel_load_vbt_firmware(dev_priv) == 0)
>  		goto out;
> @@ -1041,6 +1045,45 @@ intel_opregion_get_panel_type(struct drm_i915_private *dev_priv)
>  	return ret - 1;
>  }
>  
> +void intel_opregion_edid_override(struct intel_connector *intel_connector)
> +{
> +	struct drm_connector *connector = &intel_connector->base;
> +	struct drm_i915_private *i915 = to_i915(connector->dev);
> +	struct intel_opregion *opregion = &i915->opregion;
> +	const void *in_edid;
> +	const struct edid *edid;
> +	int len, ret;
> +
> +	if (!opregion->asle_ext)
> +		return;
> +
> +	in_edid = opregion->asle_ext->bddc;
> +
> +	/* Validity corresponds to number of 128-byte blocks */
> +	len = (opregion->asle_ext->phed & ASLE_PHED_EDID_VALID_MASK) * 128;
> +	if (!len || !memchr_inv(in_edid, 0, len))
> +		return;
> +
> +	edid = in_edid;
> +
> +	/*
> +	 * FIXME: Might also check drm_edid_is_valid(edid) here but that
> +	 * requires mutable edid.
> +	 */
> +	if (len < EDID_LENGTH * (1 + edid->extensions)) {
> +		drm_dbg_kms(&i915->drm, "Invalid EDID in ACPI OpRegion (Mailbox #5)\n");
> +		return;
> +	}
> +
> +	connector->override_edid = false;
> +	ret = drm_connector_update_edid_property(connector, edid);
> +	if (ret)
> +		return;
> +

This is missing here:

	connector->override_edid = true;

> +	drm_dbg_kms(&i915->drm, "Using OpRegion EDID for [CONNECTOR:%d:%s]\n",
> +		    connector->base.id, connector->name);
> +}
> +
>  void intel_opregion_register(struct drm_i915_private *i915)
>  {
>  	struct intel_opregion *opregion = &i915->opregion;
> @@ -1131,6 +1174,7 @@ void intel_opregion_unregister(struct drm_i915_private *i915)
>  	opregion->acpi = NULL;
>  	opregion->swsci = NULL;
>  	opregion->asle = NULL;
> +	opregion->asle_ext = NULL;
>  	opregion->vbt = NULL;
>  	opregion->lid_state = NULL;
>  }
> diff --git a/drivers/gpu/drm/i915/display/intel_opregion.h b/drivers/gpu/drm/i915/display/intel_opregion.h
> index 4aa68ffbd30e..b407a0744c40 100644
> --- a/drivers/gpu/drm/i915/display/intel_opregion.h
> +++ b/drivers/gpu/drm/i915/display/intel_opregion.h
> @@ -29,12 +29,14 @@
>  #include <linux/pci.h>
>  
>  struct drm_i915_private;
> +struct intel_connector;
>  struct intel_encoder;
>  
>  struct opregion_header;
>  struct opregion_acpi;
>  struct opregion_swsci;
>  struct opregion_asle;
> +struct opregion_asle_ext;
>  
>  struct intel_opregion {
>  	struct opregion_header *header;
> @@ -43,6 +45,7 @@ struct intel_opregion {
>  	u32 swsci_gbda_sub_functions;
>  	u32 swsci_sbcb_sub_functions;
>  	struct opregion_asle *asle;
> +	struct opregion_asle_ext *asle_ext;
>  	void *rvda;
>  	void *vbt_firmware;
>  	const void *vbt;
> @@ -71,6 +74,7 @@ int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
>  int intel_opregion_notify_adapter(struct drm_i915_private *dev_priv,
>  				  pci_power_t state);
>  int intel_opregion_get_panel_type(struct drm_i915_private *dev_priv);
> +void intel_opregion_edid_override(struct intel_connector *connector);
>  
>  #else /* CONFIG_ACPI*/
>  
> @@ -117,6 +121,10 @@ static inline int intel_opregion_get_panel_type(struct drm_i915_private *dev)
>  	return -ENODEV;
>  }
>  
> +void intel_opregion_edid_override(struct intel_connector *connector)
> +{
> +}
> +
>  #endif /* CONFIG_ACPI */
>  
>  #endif

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm/i915/opregion: add support for mailbox #5 EDID

[Shankar, Uma]

> -----Original Message-----
> From: Jani Nikula <jani.nikula@intel.com>
> Sent: Friday, August 28, 2020 11:58 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: Shankar, Uma <uma.shankar@intel.com>
> Subject: Re: [PATCH 1/2] drm/i915/opregion: add support for mailbox #5 EDID
> 
> On Fri, 28 Aug 2020, Jani Nikula <jani.nikula@intel.com> wrote:
> > The ACPI OpRegion Mailbox #5 ASLE extension may contain an EDID to be
> > used for the embedded display. Add support for using it via the EDID
> > override mechanism.
> >
> > Note that the override EDID may be later reset or changed via debugfs,
> > as usual.
> >
> > Cc: Uma Shankar <uma.shankar@intel.com>
> > Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_opregion.c | 46
> > ++++++++++++++++++-  drivers/gpu/drm/i915/display/intel_opregion.h |
> > 8 ++++
> >  2 files changed, 53 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_opregion.c
> > b/drivers/gpu/drm/i915/display/intel_opregion.c
> > index de995362f428..13485969fafa 100644
> > --- a/drivers/gpu/drm/i915/display/intel_opregion.c
> > +++ b/drivers/gpu/drm/i915/display/intel_opregion.c
> > @@ -196,6 +196,8 @@ struct opregion_asle_ext {
> >  #define ASLE_IUER_WINDOWS_BTN		(1 << 1)
> >  #define ASLE_IUER_POWER_BTN		(1 << 0)
> >
> > +#define ASLE_PHED_EDID_VALID_MASK	0x3
> > +
> >  /* Software System Control Interrupt (SWSCI) */
> >  #define SWSCI_SCIC_INDICATOR		(1 << 0)
> >  #define SWSCI_SCIC_MAIN_FUNCTION_SHIFT	1
> > @@ -909,8 +911,10 @@ int intel_opregion_setup(struct drm_i915_private
> *dev_priv)
> >  		opregion->asle->ardy = ASLE_ARDY_NOT_READY;
> >  	}
> >
> > -	if (mboxes & MBOX_ASLE_EXT)
> > +	if (mboxes & MBOX_ASLE_EXT) {
> >  		drm_dbg(&dev_priv->drm, "ASLE extension supported\n");
> > +		opregion->asle_ext = base + OPREGION_ASLE_EXT_OFFSET;
> > +	}
> >
> >  	if (intel_load_vbt_firmware(dev_priv) == 0)
> >  		goto out;
> > @@ -1041,6 +1045,45 @@ intel_opregion_get_panel_type(struct
> drm_i915_private *dev_priv)
> >  	return ret - 1;
> >  }
> >
> > +void intel_opregion_edid_override(struct intel_connector
> > +*intel_connector) {
> > +	struct drm_connector *connector = &intel_connector->base;
> > +	struct drm_i915_private *i915 = to_i915(connector->dev);
> > +	struct intel_opregion *opregion = &i915->opregion;
> > +	const void *in_edid;
> > +	const struct edid *edid;
> > +	int len, ret;
> > +
> > +	if (!opregion->asle_ext)
> > +		return;
> > +
> > +	in_edid = opregion->asle_ext->bddc;
> > +
> > +	/* Validity corresponds to number of 128-byte blocks */
> > +	len = (opregion->asle_ext->phed & ASLE_PHED_EDID_VALID_MASK) * 128;
> > +	if (!len || !memchr_inv(in_edid, 0, len))
> > +		return;
> > +
> > +	edid = in_edid;
> > +
> > +	/*
> > +	 * FIXME: Might also check drm_edid_is_valid(edid) here but that
> > +	 * requires mutable edid.
> > +	 */
> > +	if (len < EDID_LENGTH * (1 + edid->extensions)) {
> > +		drm_dbg_kms(&i915->drm, "Invalid EDID in ACPI OpRegion
> (Mailbox #5)\n");
> > +		return;
> > +	}
> > +
> > +	connector->override_edid = false;
> > +	ret = drm_connector_update_edid_property(connector, edid);
> > +	if (ret)
> > +		return;
> > +
> 
> This is missing here:
> 
> 	connector->override_edid = true;

Yeah with this added, changes look good to me
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

> > +	drm_dbg_kms(&i915->drm, "Using OpRegion EDID for
> [CONNECTOR:%d:%s]\n",
> > +		    connector->base.id, connector->name); }
> > +
> >  void intel_opregion_register(struct drm_i915_private *i915)  {
> >  	struct intel_opregion *opregion = &i915->opregion; @@ -1131,6
> > +1174,7 @@ void intel_opregion_unregister(struct drm_i915_private *i915)
> >  	opregion->acpi = NULL;
> >  	opregion->swsci = NULL;
> >  	opregion->asle = NULL;
> > +	opregion->asle_ext = NULL;
> >  	opregion->vbt = NULL;
> >  	opregion->lid_state = NULL;
> >  }
> > diff --git a/drivers/gpu/drm/i915/display/intel_opregion.h
> > b/drivers/gpu/drm/i915/display/intel_opregion.h
> > index 4aa68ffbd30e..b407a0744c40 100644
> > --- a/drivers/gpu/drm/i915/display/intel_opregion.h
> > +++ b/drivers/gpu/drm/i915/display/intel_opregion.h
> > @@ -29,12 +29,14 @@
> >  #include <linux/pci.h>
> >
> >  struct drm_i915_private;
> > +struct intel_connector;
> >  struct intel_encoder;
> >
> >  struct opregion_header;
> >  struct opregion_acpi;
> >  struct opregion_swsci;
> >  struct opregion_asle;
> > +struct opregion_asle_ext;
> >
> >  struct intel_opregion {
> >  	struct opregion_header *header;
> > @@ -43,6 +45,7 @@ struct intel_opregion {
> >  	u32 swsci_gbda_sub_functions;
> >  	u32 swsci_sbcb_sub_functions;
> >  	struct opregion_asle *asle;
> > +	struct opregion_asle_ext *asle_ext;
> >  	void *rvda;
> >  	void *vbt_firmware;
> >  	const void *vbt;
> > @@ -71,6 +74,7 @@ int intel_opregion_notify_encoder(struct
> > intel_encoder *intel_encoder,  int intel_opregion_notify_adapter(struct
> drm_i915_private *dev_priv,
> >  				  pci_power_t state);
> >  int intel_opregion_get_panel_type(struct drm_i915_private *dev_priv);
> > +void intel_opregion_edid_override(struct intel_connector *connector);
> >
> >  #else /* CONFIG_ACPI*/
> >
> > @@ -117,6 +121,10 @@ static inline int intel_opregion_get_panel_type(struct
> drm_i915_private *dev)
> >  	return -ENODEV;
> >  }
> >
> > +void intel_opregion_edid_override(struct intel_connector *connector)
> > +{ }
> > +
> >  #endif /* CONFIG_ACPI */
> >
> >  #endif
> 
> --
> Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [1/2] drm/i915/opregion: add support for mailbox #5 EDID

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915/opregion: add support for mailbox #5 EDID
URL   : https://patchwork.freedesktop.org/series/81121/
State : warning

== Summary ==

$ dim sparse --fast origin/drm-tip
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.
-
+drivers/gpu/drm/i915/gt/intel_reset.c:1311:5: warning: context imbalance in 'intel_gt_reset_trylock' - different lock contexts for basic block
+drivers/gpu/drm/i915/gvt/mmio.c:287:23: warning: memcpy with byte count of 279040
+drivers/gpu/drm/i915/i915_perf.c:1440:15: warning: memset with byte count of 16777216
+drivers/gpu/drm/i915/i915_perf.c:1494:15: warning: memset with byte count of 16777216
+./include/linux/seqlock.h:752:24: warning: trying to copy expression type 31
+./include/linux/seqlock.h:778:16: warning: trying to copy expression type 31
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen8_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen8_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen8_write8' - different lock contexts for basic block


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/2] drm/i915/opregion: add support for mailbox #5 EDID

[Patchwork]

[-- Attachment #1.1: Type: text/plain, Size: 6089 bytes --]

== Series Details ==

Series: series starting with [1/2] drm/i915/opregion: add support for mailbox #5 EDID
URL   : https://patchwork.freedesktop.org/series/81121/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_8937 -> Patchwork_18418
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_18418 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_18418, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18418/index.html

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_18418:

### IGT changes ###

#### Possible regressions ####

  * igt@gem_exec_parallel@engines@basic:
    - fi-icl-u2:          [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8937/fi-icl-u2/igt@gem_exec_parallel@engines@basic.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18418/fi-icl-u2/igt@gem_exec_parallel@engines@basic.html

  * igt@i915_selftest@live@requests:
    - fi-cml-s:           [PASS][3] -> [INCOMPLETE][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8937/fi-cml-s/igt@i915_selftest@live@requests.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18418/fi-cml-s/igt@i915_selftest@live@requests.html

  * igt@runner@aborted:
    - fi-icl-u2:          NOTRUN -> [FAIL][5]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18418/fi-icl-u2/igt@runner@aborted.html

  
Known issues
------------

  Here are the changes found in Patchwork_18418 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live@execlists:
    - fi-icl-y:           [PASS][6] -> [INCOMPLETE][7] ([i915#2276])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8937/fi-icl-y/igt@i915_selftest@live@execlists.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18418/fi-icl-y/igt@i915_selftest@live@execlists.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
    - fi-byt-j1900:       [PASS][8] -> [DMESG-WARN][9] ([i915#1982])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8937/fi-byt-j1900/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18418/fi-byt-j1900/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html

  
#### Possible fixes ####

  * igt@i915_module_load@reload:
    - fi-icl-y:           [TIMEOUT][10] -> [PASS][11]
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8937/fi-icl-y/igt@i915_module_load@reload.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18418/fi-icl-y/igt@i915_module_load@reload.html

  * igt@i915_pm_rpm@basic-pci-d3-state:
    - fi-bsw-kefka:       [DMESG-WARN][12] ([i915#1982]) -> [PASS][13]
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8937/fi-bsw-kefka/igt@i915_pm_rpm@basic-pci-d3-state.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18418/fi-bsw-kefka/igt@i915_pm_rpm@basic-pci-d3-state.html

  * igt@i915_pm_rpm@module-reload:
    - fi-icl-y:           [SKIP][14] ([i915#579]) -> [PASS][15]
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8937/fi-icl-y/igt@i915_pm_rpm@module-reload.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18418/fi-icl-y/igt@i915_pm_rpm@module-reload.html

  
#### Warnings ####

  * igt@gem_exec_suspend@basic-s0:
    - fi-kbl-x1275:       [DMESG-WARN][16] ([i915#62] / [i915#92]) -> [DMESG-WARN][17] ([i915#1982] / [i915#62] / [i915#92] / [i915#95])
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8937/fi-kbl-x1275/igt@gem_exec_suspend@basic-s0.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18418/fi-kbl-x1275/igt@gem_exec_suspend@basic-s0.html

  * igt@kms_flip@basic-plain-flip@a-dp1:
    - fi-kbl-x1275:       [DMESG-WARN][18] ([i915#62] / [i915#92] / [i915#95]) -> [DMESG-WARN][19] ([i915#62] / [i915#92]) +1 similar issue
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8937/fi-kbl-x1275/igt@kms_flip@basic-plain-flip@a-dp1.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18418/fi-kbl-x1275/igt@kms_flip@basic-plain-flip@a-dp1.html

  * igt@prime_vgem@basic-fence-flip:
    - fi-kbl-x1275:       [DMESG-WARN][20] ([i915#62] / [i915#92]) -> [DMESG-WARN][21] ([i915#62] / [i915#92] / [i915#95]) +1 similar issue
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8937/fi-kbl-x1275/igt@prime_vgem@basic-fence-flip.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18418/fi-kbl-x1275/igt@prime_vgem@basic-fence-flip.html

  
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#2276]: https://gitlab.freedesktop.org/drm/intel/issues/2276
  [i915#579]: https://gitlab.freedesktop.org/drm/intel/issues/579
  [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 (39 -> 34)
------------------------------

  Missing    (5): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-byt-clapper 


Build changes
-------------

  * Linux: CI_DRM_8937 -> Patchwork_18418

  CI-20190529: 20190529
  CI_DRM_8937: 78b090a913c972368c81f05352a532590200cc89 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5773: 17641a0df4f4a37711a28a9aaf48c0de85ef53d4 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_18418: 0a72649aec5890da74b7a28c78cfdc648743e15f @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

0a72649aec58 drm/i915/dp: use opregion mailbox #5 EDID for eDP, if available
5d569c972e1b drm/i915/opregion: add support for mailbox #5 EDID

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18418/index.html

[-- Attachment #1.2: Type: text/html, Size: 7899 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

Re: [Intel-gfx] [PATCH 1/2] drm/i915/opregion: add support for mailbox #5 EDID

[Ville Syrjälä]

On Fri, Aug 28, 2020 at 09:19:40AM +0300, Jani Nikula wrote:
> The ACPI OpRegion Mailbox #5 ASLE extension may contain an EDID to be
> used for the embedded display. Add support for using it via the EDID
> override mechanism.

Abusing the override for this feels a bit odd.

Also I have a vague recollection that there was perhaps some
linkage between the mailbox and the ACPI _DDC stuff:
git://github.com/vsyrjala/linux.git acpi_edid

> 
> Note that the override EDID may be later reset or changed via debugfs,
> as usual.
> 
> Cc: Uma Shankar <uma.shankar@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_opregion.c | 46 ++++++++++++++++++-
>  drivers/gpu/drm/i915/display/intel_opregion.h |  8 ++++
>  2 files changed, 53 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_opregion.c b/drivers/gpu/drm/i915/display/intel_opregion.c
> index de995362f428..13485969fafa 100644
> --- a/drivers/gpu/drm/i915/display/intel_opregion.c
> +++ b/drivers/gpu/drm/i915/display/intel_opregion.c
> @@ -196,6 +196,8 @@ struct opregion_asle_ext {
>  #define ASLE_IUER_WINDOWS_BTN		(1 << 1)
>  #define ASLE_IUER_POWER_BTN		(1 << 0)
>  
> +#define ASLE_PHED_EDID_VALID_MASK	0x3
> +
>  /* Software System Control Interrupt (SWSCI) */
>  #define SWSCI_SCIC_INDICATOR		(1 << 0)
>  #define SWSCI_SCIC_MAIN_FUNCTION_SHIFT	1
> @@ -909,8 +911,10 @@ int intel_opregion_setup(struct drm_i915_private *dev_priv)
>  		opregion->asle->ardy = ASLE_ARDY_NOT_READY;
>  	}
>  
> -	if (mboxes & MBOX_ASLE_EXT)
> +	if (mboxes & MBOX_ASLE_EXT) {
>  		drm_dbg(&dev_priv->drm, "ASLE extension supported\n");
> +		opregion->asle_ext = base + OPREGION_ASLE_EXT_OFFSET;
> +	}
>  
>  	if (intel_load_vbt_firmware(dev_priv) == 0)
>  		goto out;
> @@ -1041,6 +1045,45 @@ intel_opregion_get_panel_type(struct drm_i915_private *dev_priv)
>  	return ret - 1;
>  }
>  
> +void intel_opregion_edid_override(struct intel_connector *intel_connector)
> +{
> +	struct drm_connector *connector = &intel_connector->base;
> +	struct drm_i915_private *i915 = to_i915(connector->dev);
> +	struct intel_opregion *opregion = &i915->opregion;
> +	const void *in_edid;
> +	const struct edid *edid;
> +	int len, ret;
> +
> +	if (!opregion->asle_ext)
> +		return;
> +
> +	in_edid = opregion->asle_ext->bddc;
> +
> +	/* Validity corresponds to number of 128-byte blocks */
> +	len = (opregion->asle_ext->phed & ASLE_PHED_EDID_VALID_MASK) * 128;
> +	if (!len || !memchr_inv(in_edid, 0, len))
> +		return;
> +
> +	edid = in_edid;
> +
> +	/*
> +	 * FIXME: Might also check drm_edid_is_valid(edid) here but that
> +	 * requires mutable edid.
> +	 */
> +	if (len < EDID_LENGTH * (1 + edid->extensions)) {
> +		drm_dbg_kms(&i915->drm, "Invalid EDID in ACPI OpRegion (Mailbox #5)\n");
> +		return;
> +	}
> +
> +	connector->override_edid = false;
> +	ret = drm_connector_update_edid_property(connector, edid);
> +	if (ret)
> +		return;
> +
> +	drm_dbg_kms(&i915->drm, "Using OpRegion EDID for [CONNECTOR:%d:%s]\n",
> +		    connector->base.id, connector->name);
> +}
> +
>  void intel_opregion_register(struct drm_i915_private *i915)
>  {
>  	struct intel_opregion *opregion = &i915->opregion;
> @@ -1131,6 +1174,7 @@ void intel_opregion_unregister(struct drm_i915_private *i915)
>  	opregion->acpi = NULL;
>  	opregion->swsci = NULL;
>  	opregion->asle = NULL;
> +	opregion->asle_ext = NULL;
>  	opregion->vbt = NULL;
>  	opregion->lid_state = NULL;
>  }
> diff --git a/drivers/gpu/drm/i915/display/intel_opregion.h b/drivers/gpu/drm/i915/display/intel_opregion.h
> index 4aa68ffbd30e..b407a0744c40 100644
> --- a/drivers/gpu/drm/i915/display/intel_opregion.h
> +++ b/drivers/gpu/drm/i915/display/intel_opregion.h
> @@ -29,12 +29,14 @@
>  #include <linux/pci.h>
>  
>  struct drm_i915_private;
> +struct intel_connector;
>  struct intel_encoder;
>  
>  struct opregion_header;
>  struct opregion_acpi;
>  struct opregion_swsci;
>  struct opregion_asle;
> +struct opregion_asle_ext;
>  
>  struct intel_opregion {
>  	struct opregion_header *header;
> @@ -43,6 +45,7 @@ struct intel_opregion {
>  	u32 swsci_gbda_sub_functions;
>  	u32 swsci_sbcb_sub_functions;
>  	struct opregion_asle *asle;
> +	struct opregion_asle_ext *asle_ext;
>  	void *rvda;
>  	void *vbt_firmware;
>  	const void *vbt;
> @@ -71,6 +74,7 @@ int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
>  int intel_opregion_notify_adapter(struct drm_i915_private *dev_priv,
>  				  pci_power_t state);
>  int intel_opregion_get_panel_type(struct drm_i915_private *dev_priv);
> +void intel_opregion_edid_override(struct intel_connector *connector);
>  
>  #else /* CONFIG_ACPI*/
>  
> @@ -117,6 +121,10 @@ static inline int intel_opregion_get_panel_type(struct drm_i915_private *dev)
>  	return -ENODEV;
>  }
>  
> +void intel_opregion_edid_override(struct intel_connector *connector)
> +{
> +}
> +
>  #endif /* CONFIG_ACPI */
>  
>  #endif
> -- 
> 2.20.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm/i915/opregion: add support for mailbox #5 EDID

[Jani Nikula]

On Mon, 31 Aug 2020, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Fri, Aug 28, 2020 at 09:19:40AM +0300, Jani Nikula wrote:
>> The ACPI OpRegion Mailbox #5 ASLE extension may contain an EDID to be
>> used for the embedded display. Add support for using it via the EDID
>> override mechanism.
>
> Abusing the override for this feels a bit odd.

It's the least intrusive way to make this work across the drm and driver
EDID code that I could think of.

BR,
Jani.


>
> Also I have a vague recollection that there was perhaps some
> linkage between the mailbox and the ACPI _DDC stuff:
> git://github.com/vsyrjala/linux.git acpi_edid
>
>> 
>> Note that the override EDID may be later reset or changed via debugfs,
>> as usual.
>> 
>> Cc: Uma Shankar <uma.shankar@intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_opregion.c | 46 ++++++++++++++++++-
>>  drivers/gpu/drm/i915/display/intel_opregion.h |  8 ++++
>>  2 files changed, 53 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_opregion.c b/drivers/gpu/drm/i915/display/intel_opregion.c
>> index de995362f428..13485969fafa 100644
>> --- a/drivers/gpu/drm/i915/display/intel_opregion.c
>> +++ b/drivers/gpu/drm/i915/display/intel_opregion.c
>> @@ -196,6 +196,8 @@ struct opregion_asle_ext {
>>  #define ASLE_IUER_WINDOWS_BTN		(1 << 1)
>>  #define ASLE_IUER_POWER_BTN		(1 << 0)
>>  
>> +#define ASLE_PHED_EDID_VALID_MASK	0x3
>> +
>>  /* Software System Control Interrupt (SWSCI) */
>>  #define SWSCI_SCIC_INDICATOR		(1 << 0)
>>  #define SWSCI_SCIC_MAIN_FUNCTION_SHIFT	1
>> @@ -909,8 +911,10 @@ int intel_opregion_setup(struct drm_i915_private *dev_priv)
>>  		opregion->asle->ardy = ASLE_ARDY_NOT_READY;
>>  	}
>>  
>> -	if (mboxes & MBOX_ASLE_EXT)
>> +	if (mboxes & MBOX_ASLE_EXT) {
>>  		drm_dbg(&dev_priv->drm, "ASLE extension supported\n");
>> +		opregion->asle_ext = base + OPREGION_ASLE_EXT_OFFSET;
>> +	}
>>  
>>  	if (intel_load_vbt_firmware(dev_priv) == 0)
>>  		goto out;
>> @@ -1041,6 +1045,45 @@ intel_opregion_get_panel_type(struct drm_i915_private *dev_priv)
>>  	return ret - 1;
>>  }
>>  
>> +void intel_opregion_edid_override(struct intel_connector *intel_connector)
>> +{
>> +	struct drm_connector *connector = &intel_connector->base;
>> +	struct drm_i915_private *i915 = to_i915(connector->dev);
>> +	struct intel_opregion *opregion = &i915->opregion;
>> +	const void *in_edid;
>> +	const struct edid *edid;
>> +	int len, ret;
>> +
>> +	if (!opregion->asle_ext)
>> +		return;
>> +
>> +	in_edid = opregion->asle_ext->bddc;
>> +
>> +	/* Validity corresponds to number of 128-byte blocks */
>> +	len = (opregion->asle_ext->phed & ASLE_PHED_EDID_VALID_MASK) * 128;
>> +	if (!len || !memchr_inv(in_edid, 0, len))
>> +		return;
>> +
>> +	edid = in_edid;
>> +
>> +	/*
>> +	 * FIXME: Might also check drm_edid_is_valid(edid) here but that
>> +	 * requires mutable edid.
>> +	 */
>> +	if (len < EDID_LENGTH * (1 + edid->extensions)) {
>> +		drm_dbg_kms(&i915->drm, "Invalid EDID in ACPI OpRegion (Mailbox #5)\n");
>> +		return;
>> +	}
>> +
>> +	connector->override_edid = false;
>> +	ret = drm_connector_update_edid_property(connector, edid);
>> +	if (ret)
>> +		return;
>> +
>> +	drm_dbg_kms(&i915->drm, "Using OpRegion EDID for [CONNECTOR:%d:%s]\n",
>> +		    connector->base.id, connector->name);
>> +}
>> +
>>  void intel_opregion_register(struct drm_i915_private *i915)
>>  {
>>  	struct intel_opregion *opregion = &i915->opregion;
>> @@ -1131,6 +1174,7 @@ void intel_opregion_unregister(struct drm_i915_private *i915)
>>  	opregion->acpi = NULL;
>>  	opregion->swsci = NULL;
>>  	opregion->asle = NULL;
>> +	opregion->asle_ext = NULL;
>>  	opregion->vbt = NULL;
>>  	opregion->lid_state = NULL;
>>  }
>> diff --git a/drivers/gpu/drm/i915/display/intel_opregion.h b/drivers/gpu/drm/i915/display/intel_opregion.h
>> index 4aa68ffbd30e..b407a0744c40 100644
>> --- a/drivers/gpu/drm/i915/display/intel_opregion.h
>> +++ b/drivers/gpu/drm/i915/display/intel_opregion.h
>> @@ -29,12 +29,14 @@
>>  #include <linux/pci.h>
>>  
>>  struct drm_i915_private;
>> +struct intel_connector;
>>  struct intel_encoder;
>>  
>>  struct opregion_header;
>>  struct opregion_acpi;
>>  struct opregion_swsci;
>>  struct opregion_asle;
>> +struct opregion_asle_ext;
>>  
>>  struct intel_opregion {
>>  	struct opregion_header *header;
>> @@ -43,6 +45,7 @@ struct intel_opregion {
>>  	u32 swsci_gbda_sub_functions;
>>  	u32 swsci_sbcb_sub_functions;
>>  	struct opregion_asle *asle;
>> +	struct opregion_asle_ext *asle_ext;
>>  	void *rvda;
>>  	void *vbt_firmware;
>>  	const void *vbt;
>> @@ -71,6 +74,7 @@ int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
>>  int intel_opregion_notify_adapter(struct drm_i915_private *dev_priv,
>>  				  pci_power_t state);
>>  int intel_opregion_get_panel_type(struct drm_i915_private *dev_priv);
>> +void intel_opregion_edid_override(struct intel_connector *connector);
>>  
>>  #else /* CONFIG_ACPI*/
>>  
>> @@ -117,6 +121,10 @@ static inline int intel_opregion_get_panel_type(struct drm_i915_private *dev)
>>  	return -ENODEV;
>>  }
>>  
>> +void intel_opregion_edid_override(struct intel_connector *connector)
>> +{
>> +}
>> +
>>  #endif /* CONFIG_ACPI */
>>  
>>  #endif
>> -- 
>> 2.20.1
>> 
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm/i915/opregion: add support for mailbox #5 EDID

[Jani Nikula]

On Mon, 31 Aug 2020, Jani Nikula <jani.nikula@intel.com> wrote:
> On Mon, 31 Aug 2020, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
>> On Fri, Aug 28, 2020 at 09:19:40AM +0300, Jani Nikula wrote:
>>> The ACPI OpRegion Mailbox #5 ASLE extension may contain an EDID to be
>>> used for the embedded display. Add support for using it via the EDID
>>> override mechanism.
>>
>> Abusing the override for this feels a bit odd.
>
> It's the least intrusive way to make this work across the drm and driver
> EDID code that I could think of.
>
> BR,
> Jani.
>
>
>>
>> Also I have a vague recollection that there was perhaps some
>> linkage between the mailbox and the ACPI _DDC stuff:
>> git://github.com/vsyrjala/linux.git acpi_edid

Only looked at this now. The patch at hand is for actually overriding
the EDID from the panel, because the panel EDID is readable but bogus. I
have no idea how the ACPI _DDC stuff would work in this case. Would it
return the EDID from the panel or from mailbox #5 or something
completely different? Who knows.

Using drm_do_get_edid() would of course be doable for reading mailbox #5
directly as well, but you'd have to make that the "primary" method and
fall back to the usual drm_get_edid(). Note that this completely
prevents you from ever reading the actual panel EDID. Using
edid_override lets you get at the panel EDID too.

BR,
Jani.


>>
>>> 
>>> Note that the override EDID may be later reset or changed via debugfs,
>>> as usual.
>>> 
>>> Cc: Uma Shankar <uma.shankar@intel.com>
>>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>> ---
>>>  drivers/gpu/drm/i915/display/intel_opregion.c | 46 ++++++++++++++++++-
>>>  drivers/gpu/drm/i915/display/intel_opregion.h |  8 ++++
>>>  2 files changed, 53 insertions(+), 1 deletion(-)
>>> 
>>> diff --git a/drivers/gpu/drm/i915/display/intel_opregion.c b/drivers/gpu/drm/i915/display/intel_opregion.c
>>> index de995362f428..13485969fafa 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_opregion.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_opregion.c
>>> @@ -196,6 +196,8 @@ struct opregion_asle_ext {
>>>  #define ASLE_IUER_WINDOWS_BTN		(1 << 1)
>>>  #define ASLE_IUER_POWER_BTN		(1 << 0)
>>>  
>>> +#define ASLE_PHED_EDID_VALID_MASK	0x3
>>> +
>>>  /* Software System Control Interrupt (SWSCI) */
>>>  #define SWSCI_SCIC_INDICATOR		(1 << 0)
>>>  #define SWSCI_SCIC_MAIN_FUNCTION_SHIFT	1
>>> @@ -909,8 +911,10 @@ int intel_opregion_setup(struct drm_i915_private *dev_priv)
>>>  		opregion->asle->ardy = ASLE_ARDY_NOT_READY;
>>>  	}
>>>  
>>> -	if (mboxes & MBOX_ASLE_EXT)
>>> +	if (mboxes & MBOX_ASLE_EXT) {
>>>  		drm_dbg(&dev_priv->drm, "ASLE extension supported\n");
>>> +		opregion->asle_ext = base + OPREGION_ASLE_EXT_OFFSET;
>>> +	}
>>>  
>>>  	if (intel_load_vbt_firmware(dev_priv) == 0)
>>>  		goto out;
>>> @@ -1041,6 +1045,45 @@ intel_opregion_get_panel_type(struct drm_i915_private *dev_priv)
>>>  	return ret - 1;
>>>  }
>>>  
>>> +void intel_opregion_edid_override(struct intel_connector *intel_connector)
>>> +{
>>> +	struct drm_connector *connector = &intel_connector->base;
>>> +	struct drm_i915_private *i915 = to_i915(connector->dev);
>>> +	struct intel_opregion *opregion = &i915->opregion;
>>> +	const void *in_edid;
>>> +	const struct edid *edid;
>>> +	int len, ret;
>>> +
>>> +	if (!opregion->asle_ext)
>>> +		return;
>>> +
>>> +	in_edid = opregion->asle_ext->bddc;
>>> +
>>> +	/* Validity corresponds to number of 128-byte blocks */
>>> +	len = (opregion->asle_ext->phed & ASLE_PHED_EDID_VALID_MASK) * 128;
>>> +	if (!len || !memchr_inv(in_edid, 0, len))
>>> +		return;
>>> +
>>> +	edid = in_edid;
>>> +
>>> +	/*
>>> +	 * FIXME: Might also check drm_edid_is_valid(edid) here but that
>>> +	 * requires mutable edid.
>>> +	 */
>>> +	if (len < EDID_LENGTH * (1 + edid->extensions)) {
>>> +		drm_dbg_kms(&i915->drm, "Invalid EDID in ACPI OpRegion (Mailbox #5)\n");
>>> +		return;
>>> +	}
>>> +
>>> +	connector->override_edid = false;
>>> +	ret = drm_connector_update_edid_property(connector, edid);
>>> +	if (ret)
>>> +		return;
>>> +
>>> +	drm_dbg_kms(&i915->drm, "Using OpRegion EDID for [CONNECTOR:%d:%s]\n",
>>> +		    connector->base.id, connector->name);
>>> +}
>>> +
>>>  void intel_opregion_register(struct drm_i915_private *i915)
>>>  {
>>>  	struct intel_opregion *opregion = &i915->opregion;
>>> @@ -1131,6 +1174,7 @@ void intel_opregion_unregister(struct drm_i915_private *i915)
>>>  	opregion->acpi = NULL;
>>>  	opregion->swsci = NULL;
>>>  	opregion->asle = NULL;
>>> +	opregion->asle_ext = NULL;
>>>  	opregion->vbt = NULL;
>>>  	opregion->lid_state = NULL;
>>>  }
>>> diff --git a/drivers/gpu/drm/i915/display/intel_opregion.h b/drivers/gpu/drm/i915/display/intel_opregion.h
>>> index 4aa68ffbd30e..b407a0744c40 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_opregion.h
>>> +++ b/drivers/gpu/drm/i915/display/intel_opregion.h
>>> @@ -29,12 +29,14 @@
>>>  #include <linux/pci.h>
>>>  
>>>  struct drm_i915_private;
>>> +struct intel_connector;
>>>  struct intel_encoder;
>>>  
>>>  struct opregion_header;
>>>  struct opregion_acpi;
>>>  struct opregion_swsci;
>>>  struct opregion_asle;
>>> +struct opregion_asle_ext;
>>>  
>>>  struct intel_opregion {
>>>  	struct opregion_header *header;
>>> @@ -43,6 +45,7 @@ struct intel_opregion {
>>>  	u32 swsci_gbda_sub_functions;
>>>  	u32 swsci_sbcb_sub_functions;
>>>  	struct opregion_asle *asle;
>>> +	struct opregion_asle_ext *asle_ext;
>>>  	void *rvda;
>>>  	void *vbt_firmware;
>>>  	const void *vbt;
>>> @@ -71,6 +74,7 @@ int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
>>>  int intel_opregion_notify_adapter(struct drm_i915_private *dev_priv,
>>>  				  pci_power_t state);
>>>  int intel_opregion_get_panel_type(struct drm_i915_private *dev_priv);
>>> +void intel_opregion_edid_override(struct intel_connector *connector);
>>>  
>>>  #else /* CONFIG_ACPI*/
>>>  
>>> @@ -117,6 +121,10 @@ static inline int intel_opregion_get_panel_type(struct drm_i915_private *dev)
>>>  	return -ENODEV;
>>>  }
>>>  
>>> +void intel_opregion_edid_override(struct intel_connector *connector)
>>> +{
>>> +}
>>> +
>>>  #endif /* CONFIG_ACPI */
>>>  
>>>  #endif
>>> -- 
>>> 2.20.1
>>> 
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm/i915/opregion: add support for mailbox #5 EDID

[Ville Syrjälä]

On Tue, Sep 01, 2020 at 04:18:19PM +0300, Jani Nikula wrote:
> On Mon, 31 Aug 2020, Jani Nikula <jani.nikula@intel.com> wrote:
> > On Mon, 31 Aug 2020, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> >> On Fri, Aug 28, 2020 at 09:19:40AM +0300, Jani Nikula wrote:
> >>> The ACPI OpRegion Mailbox #5 ASLE extension may contain an EDID to be
> >>> used for the embedded display. Add support for using it via the EDID
> >>> override mechanism.
> >>
> >> Abusing the override for this feels a bit odd.
> >
> > It's the least intrusive way to make this work across the drm and driver
> > EDID code that I could think of.
> >
> > BR,
> > Jani.
> >
> >
> >>
> >> Also I have a vague recollection that there was perhaps some
> >> linkage between the mailbox and the ACPI _DDC stuff:
> >> git://github.com/vsyrjala/linux.git acpi_edid
> 
> Only looked at this now. The patch at hand is for actually overriding
> the EDID from the panel, because the panel EDID is readable but bogus.

Do we have an actual use case for this? The commit msg doesn't say so.

> I
> have no idea how the ACPI _DDC stuff would work in this case. Would it
> return the EDID from the panel or from mailbox #5 or something
> completely different? Who knows.
> 
> Using drm_do_get_edid() would of course be doable for reading mailbox #5
> directly as well, but you'd have to make that the "primary" method and
> fall back to the usual drm_get_edid(). Note that this completely
> prevents you from ever reading the actual panel EDID. Using
> edid_override lets you get at the panel EDID too.

Might be nice to make .get_edid() a connector vfunc and let each driver
implement it however they want. That way the driver would be in total
control over the priority of different EDID sources. But haven't really
looked at what that would take. Not even sure if a vfunc is totally
required as I think most EDID reads should be in some connector specific
driver code.

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm/i915/opregion: add support for mailbox #5 EDID

[Jani Nikula]

On Wed, 02 Sep 2020, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Tue, Sep 01, 2020 at 04:18:19PM +0300, Jani Nikula wrote:
>> On Mon, 31 Aug 2020, Jani Nikula <jani.nikula@intel.com> wrote:
>> > On Mon, 31 Aug 2020, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
>> >> On Fri, Aug 28, 2020 at 09:19:40AM +0300, Jani Nikula wrote:
>> >>> The ACPI OpRegion Mailbox #5 ASLE extension may contain an EDID to be
>> >>> used for the embedded display. Add support for using it via the EDID
>> >>> override mechanism.
>> >>
>> >> Abusing the override for this feels a bit odd.
>> >
>> > It's the least intrusive way to make this work across the drm and driver
>> > EDID code that I could think of.
>> >
>> > BR,
>> > Jani.
>> >
>> >
>> >>
>> >> Also I have a vague recollection that there was perhaps some
>> >> linkage between the mailbox and the ACPI _DDC stuff:
>> >> git://github.com/vsyrjala/linux.git acpi_edid
>> 
>> Only looked at this now. The patch at hand is for actually overriding
>> the EDID from the panel, because the panel EDID is readable but bogus.
>
> Do we have an actual use case for this? The commit msg doesn't say so.

It's a bit hazy still, but potentially yes, with a need to backport to
stable as well.

>> I
>> have no idea how the ACPI _DDC stuff would work in this case. Would it
>> return the EDID from the panel or from mailbox #5 or something
>> completely different? Who knows.
>> 
>> Using drm_do_get_edid() would of course be doable for reading mailbox #5
>> directly as well, but you'd have to make that the "primary" method and
>> fall back to the usual drm_get_edid(). Note that this completely
>> prevents you from ever reading the actual panel EDID. Using
>> edid_override lets you get at the panel EDID too.
>
> Might be nice to make .get_edid() a connector vfunc and let each driver
> implement it however they want. That way the driver would be in total
> control over the priority of different EDID sources. But haven't really
> looked at what that would take. Not even sure if a vfunc is totally
> required as I think most EDID reads should be in some connector specific
> driver code.

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/2] drm/i915/dp: use opregion mailbox #5 EDID for eDP, if available

[Lee, Shawn C]

On Fri, Aug 28, 2020 at 06:19AM, Shankar Uma wrote:
>> -----Original Message-----
>> From: Jani Nikula <jani.nikula@intel.com>
>> Sent: Friday, August 28, 2020 11:50 AM
>> To: intel-gfx@lists.freedesktop.org
>> Cc: Nikula, Jani <jani.nikula@intel.com>; Shankar, Uma
>> <uma.shankar@intel.com>
>> Subject: [PATCH 2/2] drm/i915/dp: use opregion mailbox #5 EDID for eDP, if
>> available
>> 
>> If a panel's EDID is broken, there may be an override EDID set in the ACPI
>> OpRegion mailbox #5. Use it if available.
>
>Looks Good to me.
>Reviewed-by: Uma Shankar <uma.shankar@intel.com>
>
>> Cc: Uma Shankar <uma.shankar@intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_dp.c | 3 +++
>>  1 file changed, 3 insertions(+)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
>> b/drivers/gpu/drm/i915/display/intel_dp.c
>> index c57ac83bf563..d1307be196a2 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> @@ -8114,6 +8114,9 @@ static bool intel_edp_init_connector(struct intel_dp
>> *intel_dp,
>>  		goto out_vdd_off;
>>  	}
>> 
>> +	/* Set up override EDID, if any, from ACPI OpRegion */
>> +	intel_opregion_edid_override(intel_connector);
>> +

Customer report DUT still get EDID from eDP panel instead of mailbox #5.
After some investigations, this change can retrieve EDID from mailbox #5 properly.
But driver still used panel's EDID to enable eDP display. This is because of drm_get_edid()
was executed after intel_opregion_edid_override(). drm_get_edid() return panel's EDID
and overwrite mailbox #5's. 

We try to move drm_get_edid() before intel_opregion_edid_override().
The test result is positive, mailbox #5 EDID will substitute for panel's.
It seems we may need some additional change for this patch. Thanks!

Best regards,
Shawn

>>  	mutex_lock(&dev->mode_config.mutex);
>>  	edid = drm_get_edid(connector, &intel_dp->aux.ddc);
>>  	if (edid) {
>> --
>> 2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/2] drm/i915/dp: use opregion mailbox #5 EDID for eDP, if available

[Jani Nikula]

On Mon, 12 Oct 2020, "Lee, Shawn C" <shawn.c.lee@intel.com> wrote:
> On Fri, Aug 28, 2020 at 06:19AM, Shankar Uma wrote:
>>> -----Original Message-----
>>> From: Jani Nikula <jani.nikula@intel.com>
>>> Sent: Friday, August 28, 2020 11:50 AM
>>> To: intel-gfx@lists.freedesktop.org
>>> Cc: Nikula, Jani <jani.nikula@intel.com>; Shankar, Uma
>>> <uma.shankar@intel.com>
>>> Subject: [PATCH 2/2] drm/i915/dp: use opregion mailbox #5 EDID for eDP, if
>>> available
>>>
>>> If a panel's EDID is broken, there may be an override EDID set in the ACPI
>>> OpRegion mailbox #5. Use it if available.
>>
>>Looks Good to me.
>>Reviewed-by: Uma Shankar <uma.shankar@intel.com>
>>
>>> Cc: Uma Shankar <uma.shankar@intel.com>
>>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>> ---
>>>  drivers/gpu/drm/i915/display/intel_dp.c | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
>>> b/drivers/gpu/drm/i915/display/intel_dp.c
>>> index c57ac83bf563..d1307be196a2 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>>> @@ -8114,6 +8114,9 @@ static bool intel_edp_init_connector(struct intel_dp
>>> *intel_dp,
>>>  goto out_vdd_off;
>>>  }
>>>
>>> +/* Set up override EDID, if any, from ACPI OpRegion */
>>> +intel_opregion_edid_override(intel_connector);
>>> +
>
> Customer report DUT still get EDID from eDP panel instead of mailbox #5.
> After some investigations, this change can retrieve EDID from mailbox #5 properly.
> But driver still used panel's EDID to enable eDP display. This is because of drm_get_edid()
> was executed after intel_opregion_edid_override(). drm_get_edid() return panel's EDID
> and overwrite mailbox #5's.

In recent kernels, drm_get_edid() respects EDID override, and calling
drm_get_edid() will return the override EDID from mailbox #5 instead of
retrieving the actual EDID.

Check the kernel version they're using and the drm_get_edid()
implementation.

BR,
Jani.


>
> We try to move drm_get_edid() before intel_opregion_edid_override().
> The test result is positive, mailbox #5 EDID will substitute for panel's.
> It seems we may need some additional change for this patch. Thanks!
>
> Best regards,
> Shawn
>
>>>  mutex_lock(&dev->mode_config.mutex);
>>>  edid = drm_get_edid(connector, &intel_dp->aux.ddc);
>>>  if (edid) {
>>> --
>>> 2.20.1

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/2] drm/i915/dp: use opregion mailbox #5 EDID for eDP, if available

[Lee, Shawn C]

On Mon, Oct 12, 2020 at 5:09 PM, Jani Nikula wrote:
>On Mon, 12 Oct 2020, "Lee, Shawn C" <shawn.c.lee@intel.com> wrote:
>> On Fri, Aug 28, 2020 at 06:19AM, Shankar Uma wrote:
>>>> -----Original Message-----
>>>> From: Jani Nikula <jani.nikula@intel.com>
>>>> Sent: Friday, August 28, 2020 11:50 AM
>>>> To: intel-gfx@lists.freedesktop.org
>>>> Cc: Nikula, Jani <jani.nikula@intel.com>; Shankar, Uma 
>>>> <uma.shankar@intel.com>
>>>> Subject: [PATCH 2/2] drm/i915/dp: use opregion mailbox #5 EDID for 
>>>> eDP, if available
>>>>
>>>> If a panel's EDID is broken, there may be an override EDID set in 
>>>> the ACPI OpRegion mailbox #5. Use it if available.
>>>
>>>Looks Good to me.
>>>Reviewed-by: Uma Shankar <uma.shankar@intel.com>
>>>
>>>> Cc: Uma Shankar <uma.shankar@intel.com>
>>>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>>> ---
>>>>  drivers/gpu/drm/i915/display/intel_dp.c | 3 +++
>>>>  1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
>>>> b/drivers/gpu/drm/i915/display/intel_dp.c
>>>> index c57ac83bf563..d1307be196a2 100644
>>>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>>>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>>>> @@ -8114,6 +8114,9 @@ static bool intel_edp_init_connector(struct 
>>>> intel_dp *intel_dp,  goto out_vdd_off;  }
>>>>
>>>> +/* Set up override EDID, if any, from ACPI OpRegion */ 
>>>> +intel_opregion_edid_override(intel_connector);
>>>> +
>>
>> Customer report DUT still get EDID from eDP panel instead of mailbox #5.
>> After some investigations, this change can retrieve EDID from mailbox #5 properly.
>> But driver still used panel's EDID to enable eDP display. This is 
>> because of drm_get_edid() was executed after 
>> intel_opregion_edid_override(). drm_get_edid() return panel's EDID and overwrite mailbox #5's.
>
>In recent kernels, drm_get_edid() respects EDID override, and calling
>drm_get_edid() will return the override EDID from mailbox #5 instead of retrieving the actual EDID.
>
>Check the kernel version they're using and the drm_get_edid() implementation.
>
>BR,
>Jani.
>

Just confirm customer is using kernel v5.8. Seems drm_edid.c already include
the change for override_edid.

BTW. override_edid should be "true" and used to over EDID in intel_opregion_edid_override() in patch 1. Right? 
But it is "false" in the patch 1.

+	connector->override_edid = false;

Best regards,
Shawn

>
>>
>> We try to move drm_get_edid() before intel_opregion_edid_override().
>> The test result is positive, mailbox #5 EDID will substitute for panel's.
>> It seems we may need some additional change for this patch. Thanks!
>>
>> Best regards,
>> Shawn
>>
>>>>  mutex_lock(&dev->mode_config.mutex);
>>>>  edid = drm_get_edid(connector, &intel_dp->aux.ddc);  if (edid) {
>>>> --
>>>> 2.20.1
>
>-- 
>Jani Nikula, Intel Open Source Graphics Center
>_______________________________________________
>Intel-gfx mailing list
>Intel-gfx@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/2] drm/i915/dp: use opregion mailbox #5 EDID for eDP, if available

[Jani Nikula]

On Tue, 13 Oct 2020, "Lee, Shawn C" <shawn.c.lee@intel.com> wrote:
> On Mon, Oct 12, 2020 at 5:09 PM, Jani Nikula wrote:
>>On Mon, 12 Oct 2020, "Lee, Shawn C" <shawn.c.lee@intel.com> wrote:
>>> On Fri, Aug 28, 2020 at 06:19AM, Shankar Uma wrote:
>>>>> -----Original Message-----
>>>>> From: Jani Nikula <jani.nikula@intel.com>
>>>>> Sent: Friday, August 28, 2020 11:50 AM
>>>>> To: intel-gfx@lists.freedesktop.org
>>>>> Cc: Nikula, Jani <jani.nikula@intel.com>; Shankar, Uma
>>>>> <uma.shankar@intel.com>
>>>>> Subject: [PATCH 2/2] drm/i915/dp: use opregion mailbox #5 EDID for
>>>>> eDP, if available
>>>>>
>>>>> If a panel's EDID is broken, there may be an override EDID set in
>>>>> the ACPI OpRegion mailbox #5. Use it if available.
>>>>
>>>>Looks Good to me.
>>>>Reviewed-by: Uma Shankar <uma.shankar@intel.com>
>>>>
>>>>> Cc: Uma Shankar <uma.shankar@intel.com>
>>>>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>>>> ---
>>>>>  drivers/gpu/drm/i915/display/intel_dp.c | 3 +++
>>>>>  1 file changed, 3 insertions(+)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
>>>>> b/drivers/gpu/drm/i915/display/intel_dp.c
>>>>> index c57ac83bf563..d1307be196a2 100644
>>>>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>>>>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>>>>> @@ -8114,6 +8114,9 @@ static bool intel_edp_init_connector(struct
>>>>> intel_dp *intel_dp,  goto out_vdd_off;  }
>>>>>
>>>>> +/* Set up override EDID, if any, from ACPI OpRegion */
>>>>> +intel_opregion_edid_override(intel_connector);
>>>>> +
>>>
>>> Customer report DUT still get EDID from eDP panel instead of mailbox #5.
>>> After some investigations, this change can retrieve EDID from mailbox #5 properly.
>>> But driver still used panel's EDID to enable eDP display. This is
>>> because of drm_get_edid() was executed after
>>> intel_opregion_edid_override(). drm_get_edid() return panel's EDID and overwrite mailbox #5's.
>>
>>In recent kernels, drm_get_edid() respects EDID override, and calling
>>drm_get_edid() will return the override EDID from mailbox #5 instead of retrieving the actual EDID.
>>
>>Check the kernel version they're using and the drm_get_edid() implementation.
>>
>>BR,
>>Jani.
>>
>
> Just confirm customer is using kernel v5.8. Seems drm_edid.c already include
> the change for override_edid.
>
> BTW. override_edid should be "true" and used to over EDID in intel_opregion_edid_override() in patch 1. Right?
> But it is "false" in the patch 1.
>
> +connector->override_edid = false;
>


+
+	connector->override_edid = false;
+	ret = drm_connector_update_edid_property(connector, edid);
+	if (ret)
+		return;
+
+	connector->override_edid = true;
+	drm_dbg_kms(&i915->drm, "Using OpRegion EDID for [CONNECTOR:%d:%s]\n",
+		    connector->base.id, connector->name);


http://lore.kernel.org/r/20200831083657.15600-1-jani.nikula@intel.com

BR,
Jani.





> Best regards,
> Shawn
>
>>
>>>
>>> We try to move drm_get_edid() before intel_opregion_edid_override().
>>> The test result is positive, mailbox #5 EDID will substitute for panel's.
>>> It seems we may need some additional change for this patch. Thanks!
>>>
>>> Best regards,
>>> Shawn
>>>
>>>>>  mutex_lock(&dev->mode_config.mutex);
>>>>>  edid = drm_get_edid(connector, &intel_dp->aux.ddc);  if (edid) {
>>>>> --
>>>>> 2.20.1
>>
>>--
>>Jani Nikula, Intel Open Source Graphics Center
>>_______________________________________________
>>Intel-gfx mailing list
>>Intel-gfx@lists.freedesktop.org
>>https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/2] drm/i915/dp: use opregion mailbox #5 EDID for eDP, if available

[Lee, Shawn C]

On Tue, Oct 13, 2020 at 7:05 PM, Jani Nikula wrote:
>On Tue, 13 Oct 2020, "Lee, Shawn C" <shawn.c.lee@intel.com> wrote:
>> On Mon, Oct 12, 2020 at 5:09 PM, Jani Nikula wrote:
>>>On Mon, 12 Oct 2020, "Lee, Shawn C" <shawn.c.lee@intel.com> wrote:
>>>> On Fri, Aug 28, 2020 at 06:19AM, Shankar Uma wrote:
>>>>>> -----Original Message-----
>>>>>> From: Jani Nikula <jani.nikula@intel.com>
>>>>>> Sent: Friday, August 28, 2020 11:50 AM
>>>>>> To: intel-gfx@lists.freedesktop.org
>>>>>> Cc: Nikula, Jani <jani.nikula@intel.com>; Shankar, Uma 
>>>>>> <uma.shankar@intel.com>
>>>>>> Subject: [PATCH 2/2] drm/i915/dp: use opregion mailbox #5 EDID for 
>>>>>> eDP, if available
>>>>>>
>>>>>> If a panel's EDID is broken, there may be an override EDID set in 
>>>>>> the ACPI OpRegion mailbox #5. Use it if available.
>>>>>
>>>>>Looks Good to me.
>>>>>Reviewed-by: Uma Shankar <uma.shankar@intel.com>
>>>>>
>>>>>> Cc: Uma Shankar <uma.shankar@intel.com>
>>>>>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>>>>> ---
>>>>>>  drivers/gpu/drm/i915/display/intel_dp.c | 3 +++
>>>>>>  1 file changed, 3 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
>>>>>> b/drivers/gpu/drm/i915/display/intel_dp.c
>>>>>> index c57ac83bf563..d1307be196a2 100644
>>>>>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>>>>>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>>>>>> @@ -8114,6 +8114,9 @@ static bool intel_edp_init_connector(struct 
>>>>>> intel_dp *intel_dp,  goto out_vdd_off;  }
>>>>>>
>>>>>> +/* Set up override EDID, if any, from ACPI OpRegion */ 
>>>>>> +intel_opregion_edid_override(intel_connector);
>>>>>> +
>>>>
>>>> Customer report DUT still get EDID from eDP panel instead of mailbox #5.
>>>> After some investigations, this change can retrieve EDID from mailbox #5 properly.
>>>> But driver still used panel's EDID to enable eDP display. This is 
>>>> because of drm_get_edid() was executed after 
>>>> intel_opregion_edid_override(). drm_get_edid() return panel's EDID and overwrite mailbox #5's.
>>>
>>>In recent kernels, drm_get_edid() respects EDID override, and calling
>>>drm_get_edid() will return the override EDID from mailbox #5 instead of retrieving the actual EDID.
>>>
>>>Check the kernel version they're using and the drm_get_edid() implementation.
>>>
>>>BR,
>>>Jani.
>>>
>>
>> Just confirm customer is using kernel v5.8. Seems drm_edid.c already 
>> include the change for override_edid.
>>
>> BTW. override_edid should be "true" and used to over EDID in intel_opregion_edid_override() in patch 1. Right?
>> But it is "false" in the patch 1.
>>
>> +connector->override_edid = false;
>>
>
>
>+
>+	connector->override_edid = false;
>+	ret = drm_connector_update_edid_property(connector, edid);
>+	if (ret)
>+		return;
>+
>+	connector->override_edid = true;
>+	drm_dbg_kms(&i915->drm, "Using OpRegion EDID for [CONNECTOR:%d:%s]\n",
>+		    connector->base.id, connector->name);
>
>
>http://lore.kernel.org/r/20200831083657.15600-1-jani.nikula@intel.com
>
>BR,
>Jani.
>

I think I got the problem now. The patch you attached is correct.
http://lore.kernel.org/r/20200831083657.15600-1-jani.nikula@intel.com

But customer refer to below patch without "override_edid = true".
https://patchwork.freedesktop.org/patch/388309/?series=81121&rev=1

This is why they can't get EDID from mailbox #5 properly.

Best regards,
Shawn

>
>
>> Best regards,
>> Shawn
>>
>>>
>>>>
>>>> We try to move drm_get_edid() before intel_opregion_edid_override().
>>>> The test result is positive, mailbox #5 EDID will substitute for panel's.
>>>> It seems we may need some additional change for this patch. Thanks!
>>>>
>>>> Best regards,
>>>> Shawn
>>>>
>>>>>>  mutex_lock(&dev->mode_config.mutex);
>>>>>>  edid = drm_get_edid(connector, &intel_dp->aux.ddc);  if (edid) {
>>>>>> --
>>>>>> 2.20.1
>>>
>>>--
>>>Jani Nikula, Intel Open Source Graphics Center 
>>>_______________________________________________
>>>Intel-gfx mailing list
>>>Intel-gfx@lists.freedesktop.org
>>>https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>--
>Jani Nikula, Intel Open Source Graphics Center _______________________________________________
>Intel-gfx mailing list
>Intel-gfx@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/2] drm/i915/dp: use opregion mailbox #5 EDID for eDP, if available

[Jani Nikula]

On Wed, 14 Oct 2020, "Lee, Shawn C" <shawn.c.lee@intel.com> wrote:
> I think I got the problem now. The patch you attached is correct.
> http://lore.kernel.org/r/20200831083657.15600-1-jani.nikula@intel.com
>
> But customer refer to below patch without "override_edid = true".
> https://patchwork.freedesktop.org/patch/388309/?series=81121&rev=1
>
> This is why they can't get EDID from mailbox #5 properly.

Yeah, there was that bug in v1 of the patch.

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