Re: [Intel-gfx] [PATCH v2 2/9] drm/i915/dg1: OpRegion PCON DG1 MBD config support

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

On Thu, 16 Jun 2022, Anshuman Gupta <anshuman.gupta@intel.com> wrote:
> DGFX cards support both Add in Card(AIC) and Mother Board Down(MBD)
> configs. MBD config requires HOST BIOS GPIO toggling support
> in order to enable/disable VRAM SR using ACPI OpRegion.
>
> i915 requires to check OpRegion PCON MBD Config bits to
> discover whether Gfx Card is MBD config before enabling
> VRSR.
>
> 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 | 43 +++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_opregion.h |  6 +++
>  2 files changed, 49 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_opregion.c b/drivers/gpu/drm/i915/display/intel_opregion.c
> index 11d8c5bb23ac..c8cdcde89dfc 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_DG1_MBD_CONFIG				BIT(9)
> +#define PCON_DG1_MBD_CONFIG_FIELD_VALID			BIT(10)
>  #define PCON_DGFX_BIOS_SUPPORTS_VRSR			BIT(11)
>  #define PCON_DGFX_BIOS_SUPPORTS_VRSR_FIELD_VALID	BIT(12)
>  #define PCON_HEADLESS_SKU	BIT(13)
> @@ -1255,6 +1257,44 @@ void intel_opregion_unregister(struct drm_i915_private *i915)
>  	opregion->lid_state = NULL;
>  }
>  
> +static bool intel_opregion_dg1_mbd_config(struct drm_i915_private *i915)
> +{
> +	struct intel_opregion *opregion = &i915->opregion;
> +
> +	if (!IS_DG1(i915))
> +		return false;
> +
> +	if (!opregion)

Like in previous patch, opregion is always non-NULL. Check for
!opregion->header.

> +		return false;
> +
> +	if (opregion->header->pcon & PCON_DG1_MBD_CONFIG_FIELD_VALID)
> +		return opregion->header->pcon & PCON_DG1_MBD_CONFIG;
> +	else
> +		return false;
> +}
> +
> +/**
> + * intel_opregion_vram_sr_required().
> + * @i915 i915 device priv data.
> + *
> + * It checks whether a DGFX card is Mother Board Down config depending
> + * on respective discrete platform.
> + *
> + * Returns:
> + * It returns a boolean whether opregion vram_sr support is required.
> + */
> +bool
> +intel_opregion_vram_sr_required(struct drm_i915_private *i915)
> +{
> +	if (!IS_DGFX(i915))
> +		return false;
> +
> +	if (IS_DG1(i915))
> +		return intel_opregion_dg1_mbd_config(i915);

Only check for IS_DG1() here or in the function being called, not both.

> +
> +	return false;
> +}
> +
>  /**
>   * intel_opregion_bios_supports_vram_sr() get HOST BIOS VRAM Self
>   * Refresh capability support.
> @@ -1298,6 +1338,9 @@ void intel_opregion_vram_sr(struct drm_i915_private *i915, bool enable)
>  	if (!opregion)
>  		return;
>  
> +	if (!intel_opregion_vram_sr_required(i915))
> +		return;

Feels like maybe this patch should be combined with the previous patch
due to this dependency.

> +
>  	if (drm_WARN(&i915->drm, !opregion->asle, "ASLE MAILBOX3 is not available\n"))
>  		return;
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_opregion.h b/drivers/gpu/drm/i915/display/intel_opregion.h
> index 73c9d81d5ee6..ad40c97f9565 100644
> --- a/drivers/gpu/drm/i915/display/intel_opregion.h
> +++ b/drivers/gpu/drm/i915/display/intel_opregion.h
> @@ -77,6 +77,7 @@ 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_vram_sr_required(struct drm_i915_private *i915);
>  
>  bool intel_opregion_headless_sku(struct drm_i915_private *i915);
>  
> @@ -145,6 +146,11 @@ static void intel_opregion_vram_sr(struct drm_i915_private *i915, bool enable)
>  {
>  }
>  
> +static bool intel_opregion_vram_sr_required(struct drm_i915_private *i915)

static inline.

BR,
Jani.

> +{
> +	return false;
> +}
> +
>  #endif /* CONFIG_ACPI */
>  
>  #endif

-- 
Jani Nikula, Intel Open Source Graphics Center