Re: [Intel-gfx] [PATCH v2 1/9] drm/i915/dgfx: OpRegion VRAM Self Refresh Support

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

On Thu, 16 Jun 2022, Anshuman Gupta <anshuman.gupta@intel.com> wrote:
> Intel DGFX cards provides a feature Video Ram Self Refrsh(VRSR).
> DGFX VRSR can be enabled with runtime suspend D3Cold flow and with
> opportunistic S0ix system wide suspend flow as well.
>
> Without VRSR enablement i915 has to evict the lmem objects to
> system memory. Depending on some heuristics driver will evict
> lmem objects without VRSR.
>
> VRSR feature requires Host BIOS support, VRSR will be enable/disable
> by HOST BIOS using ACPI OpRegion.
>
> Adding OpRegion VRSR support in order to enable/disable
> VRSR on discrete cards.
>
> BSpec: 53440
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_opregion.c | 62 ++++++++++++++++++-
>  drivers/gpu/drm/i915/display/intel_opregion.h | 11 ++++
>  2 files changed, 72 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_opregion.c b/drivers/gpu/drm/i915/display/intel_opregion.c
> index 6876ba30d5a9..11d8c5bb23ac 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_DGFX_BIOS_SUPPORTS_VRSR			BIT(11)
> +#define PCON_DGFX_BIOS_SUPPORTS_VRSR_FIELD_VALID	BIT(12)
>  #define PCON_HEADLESS_SKU	BIT(13)
>  
>  struct opregion_header {
> @@ -130,7 +132,8 @@ struct opregion_asle {
>  	u64 rvda;	/* Physical (2.0) or relative from opregion (2.1+)
>  			 * address of raw VBT data. */
>  	u32 rvds;	/* Size of raw vbt data */
> -	u8 rsvd[58];
> +	u8 vrsr;	/* DGFX Video Ram Self Refresh */
> +	u8 rsvd[57];
>  } __packed;
>  
>  /* OpRegion mailbox #5: ASLE ext */
> @@ -201,6 +204,9 @@ struct opregion_asle_ext {
>  
>  #define ASLE_PHED_EDID_VALID_MASK	0x3
>  
> +/* VRAM SR */
> +#define ASLE_VRSR_ENABLE		BIT(0)
> +
>  /* Software System Control Interrupt (SWSCI) */
>  #define SWSCI_SCIC_INDICATOR		(1 << 0)
>  #define SWSCI_SCIC_MAIN_FUNCTION_SHIFT	1
> @@ -921,6 +927,8 @@ int intel_opregion_setup(struct drm_i915_private *dev_priv)
>  		opregion->header->over.minor,
>  		opregion->header->over.revision);
>  
> +	drm_dbg(&dev_priv->drm, "OpRegion PCON values 0x%x\n", opregion->header->pcon);
> +
>  	mboxes = opregion->header->mboxes;
>  	if (mboxes & MBOX_ACPI) {
>  		drm_dbg(&dev_priv->drm, "Public ACPI methods supported\n");
> @@ -1246,3 +1254,55 @@ void intel_opregion_unregister(struct drm_i915_private *i915)
>  	opregion->vbt = NULL;
>  	opregion->lid_state = NULL;
>  }
> +
> +/**
> + * intel_opregion_bios_supports_vram_sr() get HOST BIOS VRAM Self
> + * Refresh capability support.
> + * @i915: pointer to i915 device.
> + *
> + * It checks opregion pcon vram_sr fields to get HOST BIOS vram_sr
> + * capability support. It is only applocable to DGFX.
> + *
> + * Returns:
> + * true when bios supports vram_sr, or false if bios doesn't support.
> + */
> +bool intel_opregion_bios_supports_vram_sr(struct drm_i915_private *i915)
> +{
> +	struct intel_opregion *opregion = &i915->opregion;
> +
> +	if (!IS_DGFX(i915))
> +		return false;
> +
> +	if (!opregion)

This is always true. You should check for !opregion->header.

> +		return false;
> +
> +	if (opregion->header->pcon & PCON_DGFX_BIOS_SUPPORTS_VRSR_FIELD_VALID)
> +		return opregion->header->pcon & PCON_DGFX_BIOS_SUPPORTS_VRSR;
> +	else
> +		return false;
> +}
> +
> +/**
> + * intel_opregion_vram_sr() - enable/disable VRAM Self Refresh.
> + * @i915: pointer to i915 device.
> + * @enable: Argument to enable/disable VRSR.
> + *
> + * It enables/disables vram_sr in opregion ASLE MBOX, based upon that
> + * HOST BIOS will enables and disbales VRAM_SR during
> + * ACPI _PS3/_OFF and _PS/_ON glue method.
> + */
> +void intel_opregion_vram_sr(struct drm_i915_private *i915, bool enable)
> +{
> +	struct intel_opregion *opregion = &i915->opregion;
> +
> +	if (!opregion)

Same as above.

> +		return;
> +
> +	if (drm_WARN(&i915->drm, !opregion->asle, "ASLE MAILBOX3 is not available\n"))
> +		return;

I'd just bundle !opregion->asle into the early return.

> +
> +	if (enable)
> +		opregion->asle->vrsr |= ASLE_VRSR_ENABLE;
> +	else
> +		opregion->asle->vrsr &= ~ASLE_VRSR_ENABLE;
> +}
> diff --git a/drivers/gpu/drm/i915/display/intel_opregion.h b/drivers/gpu/drm/i915/display/intel_opregion.h
> index 2f261f985400..73c9d81d5ee6 100644
> --- a/drivers/gpu/drm/i915/display/intel_opregion.h
> +++ b/drivers/gpu/drm/i915/display/intel_opregion.h
> @@ -75,6 +75,8 @@ 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);
>  struct edid *intel_opregion_get_edid(struct intel_connector *connector);
> +bool intel_opregion_bios_supports_vram_sr(struct drm_i915_private *i915);
> +void intel_opregion_vram_sr(struct drm_i915_private *i915, bool enable);
>  
>  bool intel_opregion_headless_sku(struct drm_i915_private *i915);
>  
> @@ -134,6 +136,15 @@ static inline bool intel_opregion_headless_sku(struct drm_i915_private *i915)
>  	return false;
>  }
>  
> +static bool intel_opregion_bios_supports_vram_sr(struct drm_i915_private *i915)
> +{
> +	return false;
> +}
> +
> +static void intel_opregion_vram_sr(struct drm_i915_private *i915, bool enable)
> +{
> +}
> +

Both of these stubs need to be static inline.

BR,
Jani.

>  #endif /* CONFIG_ACPI */
>  
>  #endif

-- 
Jani Nikula, Intel Open Source Graphics Center