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

2022-06-17 Thread Gupta, Anshuman



> -Original Message-
> From: Nikula, Jani 
> Sent: Thursday, June 16, 2022 6:26 PM
> To: Gupta, Anshuman ; intel-
> g...@lists.freedesktop.org; dri-de...@lists.freedesktop.org
> Cc: Roper, Matthew D ; Nilawar, Badal
> ; Ewins, Jon ; Vivi, Rodrigo
> ; Ursulin, Tvrtko ; Tangudu,
> Tilak ; Gupta, Anshuman
> 
> Subject: Re: [PATCH v2 1/9] drm/i915/dgfx: OpRegion VRAM Self Refresh
> Support
> 
> On Thu, 16 Jun 2022, Anshuman Gupta  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 
> > Cc: Rodrigo Vivi 
> > Signed-off-by: Anshuman Gupta 
> > ---
> >  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 b

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

2022-06-16 Thread Jani Nikula
On Thu, 16 Jun 2022, Anshuman Gupta  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 
> Cc: Rodrigo Vivi 
> Signed-off-by: Anshuman Gupta 
> ---
>  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_EXTBIT(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_SKUBIT(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_MASK0x3
>  
> +/* 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,
>  

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

2022-06-16 Thread Anshuman Gupta
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 
Cc: Rodrigo Vivi 
Signed-off-by: Anshuman Gupta 
---
 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)
+   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)
+   return;
+
+   if (drm_WARN(&i915->drm, !opregion->asle, "ASLE MAILBOX3 is not 
available\n"))
+   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);
 
@