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

[Jani Nikula <jani.nikula@intel.com>]

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