[Intel-gfx] [PATCH 1/2] drm/i915/opregion: add function to check if headless sku

[Hogander, Jouni]

On Fri, 2022-06-03 at 15:43 +0300, Jani Nikula wrote:
> On Fri, 03 Jun 2022, Jouni Högander <jouni.hogander at intel.com> wrote:
> > Export headless sku bit (bit 13) from opregion->header->pcon as an
> > interface to check if our device is headless configuration.
> > 
> > Bspec: 53441
> > Signed-off-by: Jouni Högander <jouni.hogander at intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_opregion.c | 12 ++++++++++++
> >  drivers/gpu/drm/i915/display/intel_opregion.h |  7 +++++++
> >  2 files changed, 19 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_opregion.c
> > b/drivers/gpu/drm/i915/display/intel_opregion.c
> > index f31e8c3f8ce0..eab3f2e6b786 100644
> > --- a/drivers/gpu/drm/i915/display/intel_opregion.c
> > +++ b/drivers/gpu/drm/i915/display/intel_opregion.c
> > @@ -53,6 +53,8 @@
> >  #define MBOX_ASLE_EXT		BIT(4)	/* Mailbox #5 */
> >  #define MBOX_BACKLIGHT		BIT(5)	/* Mailbox #2
> > (valid from v3.x) */
> >  
> > +#define PCON_HEADLESS_SKU	BIT(13)
> 
> Here we go again.
> 
> What does headless mean here? The spec does not say. Does it have
> display hardware? Apparently yes, since otherwise we wouldn't be
> here.

This is for hybrid setup with several display hw and the panel wont be
connected into device driven by i915 driver.

> We have INTEL_DISPLAY_ENABLED() which should do the right thing when
> you
> do have display hardware and have done output setup etc. but want to
> force them disconnected, i.e. you take the hardware over properly,
> but
> put it to sleep for power savings.
> 
> Maybe we should bolt this opregion check in that macro?
> 
> Maybe we need to use INTEL_DISPLAY_ENABLED() also to prevent polling.

Thank you for pointing this out. HAS_DISPLAY I already notice and it's
not suitable for what we want here. I think bolting this check into
INTEL_DISPLAY_ENABLED as you suggested is enough. That will prevent
waking up the hw into D0 state for polling.

> 
> I certainly would not want to add another mode that's separate from
> HAS_DISPLAY() and INTEL_DISPLAY_ENABLED().

No need for this. I think we can go with INTEL_DISPLAY_ENABLED.
> 
> > +
> >  struct opregion_header {
> >  	u8 signature[16];
> >  	u32 size;
> > @@ -1135,6 +1137,16 @@ struct edid *intel_opregion_get_edid(struct
> > intel_connector *intel_connector)
> >  	return new_edid;
> >  }
> >  
> > +bool intel_opregion_headless_sku(struct drm_i915_private *i915)
> > +{
> > +	struct intel_opregion *opregion = &i915->opregion;
> > +
> > +	if (!opregion->header)
> > +		return false;
> > +
> > +	return opregion->header->pcon & PCON_HEADLESS_SKU;
> 
> We should probably start checking for opregion version for this stuff
> too.
> 

Yes, I will do this change.

> 
> BR,
> Jani.
> 
> > +}
> > +
> >  void intel_opregion_register(struct drm_i915_private *i915)
> >  {
> >  	struct intel_opregion *opregion = &i915->opregion;
> > diff --git a/drivers/gpu/drm/i915/display/intel_opregion.h
> > b/drivers/gpu/drm/i915/display/intel_opregion.h
> > index 82cc0ba34af7..5ad96e1d8278 100644
> > --- a/drivers/gpu/drm/i915/display/intel_opregion.h
> > +++ b/drivers/gpu/drm/i915/display/intel_opregion.h
> > @@ -76,6 +76,8 @@ int intel_opregion_notify_adapter(struct
> > drm_i915_private *dev_priv,
> >  int intel_opregion_get_panel_type(struct drm_i915_private
> > *dev_priv);
> >  struct edid *intel_opregion_get_edid(struct intel_connector
> > *connector);
> >  
> > +bool intel_opregion_headless_sku(struct drm_i915_private *i915);
> > +
> >  #else /* CONFIG_ACPI*/
> >  
> >  static inline int intel_opregion_setup(struct drm_i915_private
> > *dev_priv)
> > @@ -127,6 +129,11 @@ intel_opregion_get_edid(struct intel_connector
> > *connector)
> >  	return NULL;
> >  }
> >  
> > +bool intel_opregion_headless_sku(struct drm_i915_private *i915)
> > +{
> > +	return false;
> > +}
> > +
> >  #endif /* CONFIG_ACPI */
> >  
> >  #endif