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

2020-09-02 Thread Jani Nikula
On Wed, 02 Sep 2020, Ville Syrjälä  wrote:
> On Tue, Sep 01, 2020 at 04:18:19PM +0300, Jani Nikula wrote:
>> On Mon, 31 Aug 2020, Jani Nikula  wrote:
>> > On Mon, 31 Aug 2020, Ville Syrjälä  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 1/2] drm/i915/opregion: add support for mailbox #5 EDID

2020-09-02 Thread Ville Syrjälä
On Tue, Sep 01, 2020 at 04:18:19PM +0300, Jani Nikula wrote:
> On Mon, 31 Aug 2020, Jani Nikula  wrote:
> > On Mon, 31 Aug 2020, Ville Syrjälä  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

2020-09-01 Thread Jani Nikula
On Mon, 31 Aug 2020, Jani Nikula  wrote:
> On Mon, 31 Aug 2020, Ville Syrjälä  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 
>>> Signed-off-by: Jani Nikula 
>>> ---
>>>  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(_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 = _connector->base;
>>> +   struct drm_i915_private *i915 = to_i915(connector->dev);
>>> +   struct intel_opregion *opregion = >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(>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(>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 = >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 @@
>>>  

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

2020-08-31 Thread Jani Nikula
On Mon, 31 Aug 2020, Ville Syrjälä  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 
>> Signed-off-by: Jani Nikula 
>> ---
>>  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(_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 = _connector->base;
>> +struct drm_i915_private *i915 = to_i915(connector->dev);
>> +struct intel_opregion *opregion = >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(>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(>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 = >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 
>>  
>>  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,
>> 

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

2020-08-31 Thread 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 
> Signed-off-by: Jani Nikula 
> ---
>  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_MASK0x3
> +
>  /* 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(_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 = _connector->base;
> + struct drm_i915_private *i915 = to_i915(connector->dev);
> + struct intel_opregion *opregion = >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(>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(>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 = >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 
>  
>  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 

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

2020-08-31 Thread Shankar, Uma



> -Original Message-
> From: Jani Nikula 
> Sent: Friday, August 28, 2020 11:58 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: Shankar, Uma 
> Subject: Re: [PATCH 1/2] drm/i915/opregion: add support for mailbox #5 EDID
> 
> On Fri, 28 Aug 2020, 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.
> >
> > Note that the override EDID may be later reset or changed via debugfs,
> > as usual.
> >
> > Cc: Uma Shankar 
> > Signed-off-by: Jani Nikula 
> > ---
> >  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(_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 = _connector->base;
> > +   struct drm_i915_private *i915 = to_i915(connector->dev);
> > +   struct intel_opregion *opregion = >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(>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 

> > +   drm_dbg_kms(>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 = >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 
> >
> >  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,
> >   

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

2020-08-28 Thread Jani Nikula
On Fri, 28 Aug 2020, 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.
>
> Note that the override EDID may be later reset or changed via debugfs,
> as usual.
>
> Cc: Uma Shankar 
> Signed-off-by: Jani Nikula 
> ---
>  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_MASK0x3
> +
>  /* 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(_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 = _connector->base;
> + struct drm_i915_private *i915 = to_i915(connector->dev);
> + struct intel_opregion *opregion = >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(>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(>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 = >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 
>  
>  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 

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

2020-08-28 Thread 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 
Signed-off-by: Jani Nikula 
---
 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(_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 = _connector->base;
+   struct drm_i915_private *i915 = to_i915(connector->dev);
+   struct intel_opregion *opregion = >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(>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(>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 = >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 
 
 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