diff mbox series

[1/2] drm/i915/dg1: Compute MEM Bandwidth using MCHBAR

Message ID 20210618161314.723418-1-matthew.auld@intel.com (mailing list archive)
State New, archived
Headers show
Series [1/2] drm/i915/dg1: Compute MEM Bandwidth using MCHBAR | expand

Commit Message

Matthew Auld June 18, 2021, 4:13 p.m. UTC 
From: Clint Taylor <clinton.a.taylor@intel.com>

The PUNIT FW is currently returning 0 for all memory bandwidth
parameters. Read the values directly from MCHBAR offsets 0x5918 and
0x4000(4). This is a temporary WA until the PUNIT FW returns valid
values.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: Jani Saarinen <jani.saarinen@intel.com>
Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_bw.c | 54 ++++++++++++++++++++++++-
 1 file changed, 53 insertions(+), 1 deletion(-)

Comments

Matthew Auld June 18, 2021, 4:18 p.m. UTC | #1 
On Fri, 18 Jun 2021 at 17:14, Matthew Auld <matthew.auld@intel.com> wrote:
>
> From: Clint Taylor <clinton.a.taylor@intel.com>
>
> The PUNIT FW is currently returning 0 for all memory bandwidth
> parameters. Read the values directly from MCHBAR offsets 0x5918 and
> 0x4000(4). This is a temporary WA until the PUNIT FW returns valid
> values.

Any takers for these two patches? They are confirmed to light up the
display for DG1 and get us an accelerated desktop on drm-tip.

>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: Jani Saarinen <jani.saarinen@intel.com>
> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_bw.c | 54 ++++++++++++++++++++++++-
>  1 file changed, 53 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
> index bfb398f0432e..f3d8ff4ee0db 100644
> --- a/drivers/gpu/drm/i915/display/intel_bw.c
> +++ b/drivers/gpu/drm/i915/display/intel_bw.c
> @@ -23,6 +23,53 @@ struct intel_qgv_info {
>         u8 t_bl;
>  };
>
> +#define SA_PERF_STATUS_0_0_0_MCHBAR_PC _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5918)
> +#define  DG1_QCLK_RATIO_MASK (0xFF << 2)
> +#define  DG1_QCLK_RATIO_SHIFT 2
> +#define  DG1_QCLK_REFERENCE (1 << 10)
> +
> +#define MCHBAR_CH0_CR_TC_PRE_0_0_0_MCHBAR _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x4000)
> +#define MCHBAR_CH0_CR_TC_PRE_0_0_0_MCHBAR_HIGH _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x4004)
> +#define MCHBAR_CH1_CR_TC_PRE_0_0_0_MCHBAR _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x4400)
> +#define MCHBAR_CH1_CR_TC_PRE_0_0_0_MCHBAR_HIGH _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x4404)
> +#define  DG1_DRAM_T_RCD_MASK (0x7F << 9)
> +#define  DG1_DRAM_T_RCD_SHIFT 9
> +#define  DG1_DRAM_T_RDPRE_MASK (0x3F << 11)
> +#define  DG1_DRAM_T_RDPRE_SHIFT 11
> +#define  DG1_DRAM_T_RAS_MASK (0xFF << 1)
> +#define  DG1_DRAM_T_RAS_SHIFT 1
> +#define  DG1_DRAM_T_RP_MASK (0x7F << 0)
> +#define  DG1_DRAM_T_RP_SHIFT 0
> +
> +static int dg1_mchbar_read_qgv_point_info(struct drm_i915_private *dev_priv,
> +                                         struct intel_qgv_point *sp,
> +                                         int point)
> +{
> +       u32 val = 0;
> +       u32 dclk_ratio = 0, dclk_reference = 0;
> +
> +       val = intel_uncore_read(&dev_priv->uncore, SA_PERF_STATUS_0_0_0_MCHBAR_PC);
> +       dclk_ratio = (val & DG1_QCLK_RATIO_MASK) >> DG1_QCLK_RATIO_SHIFT;
> +       if (val & DG1_QCLK_REFERENCE)
> +               dclk_reference = 6; /* 6 * 16.666 MHz = 100 MHz */
> +       else
> +               dclk_reference = 8; /* 8 * 16.666 MHz = 133 MHz */
> +       sp->dclk = dclk_ratio * dclk_reference;
> +       if (sp->dclk == 0)
> +               return -EINVAL;
> +
> +       val = intel_uncore_read(&dev_priv->uncore, MCHBAR_CH0_CR_TC_PRE_0_0_0_MCHBAR);
> +       sp->t_rp = (val & DG1_DRAM_T_RP_MASK) >> DG1_DRAM_T_RP_SHIFT;
> +       sp->t_rdpre = (val & DG1_DRAM_T_RDPRE_MASK) >> DG1_DRAM_T_RDPRE_SHIFT;
> +
> +       val = intel_uncore_read(&dev_priv->uncore, MCHBAR_CH0_CR_TC_PRE_0_0_0_MCHBAR_HIGH);
> +       sp->t_rcd = (val & DG1_DRAM_T_RCD_MASK) >> DG1_DRAM_T_RCD_SHIFT;
> +       sp->t_ras = (val & DG1_DRAM_T_RAS_MASK) >> DG1_DRAM_T_RAS_SHIFT;
> +
> +       sp->t_rc = sp->t_rp + sp->t_ras;
> +       return 0;
> +}
> +
>  static int icl_pcode_read_qgv_point_info(struct drm_i915_private *dev_priv,
>                                          struct intel_qgv_point *sp,
>                                          int point)
> @@ -100,7 +147,12 @@ static int icl_get_qgv_points(struct drm_i915_private *dev_priv,
>                 struct intel_qgv_point *sp = &qi->points[i];
>
>                 ret = icl_pcode_read_qgv_point_info(dev_priv, sp, i);
> -               if (ret)
> +               if (IS_DG1(dev_priv) && (ret || sp->dclk == 0)) {
> +                       drm_dbg_kms(&dev_priv->drm, "Failed to get memory subsystem information via pcode. IFWI needs update. Trying with MCHBAR\n");
> +                       ret = dg1_mchbar_read_qgv_point_info(dev_priv, sp, i);
> +                       if (ret)
> +                               return ret;
> +               } else if (ret)
>                         return ret;
>
>                 drm_dbg_kms(&dev_priv->drm,
> --
> 2.26.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Lucas De Marchi June 21, 2021, 5:46 a.m. UTC | #2 
On Fri, Jun 18, 2021 at 9:14 AM Matthew Auld <matthew.auld@intel.com> wrote:
>
> From: Clint Taylor <clinton.a.taylor@intel.com>
>
> The PUNIT FW is currently returning 0 for all memory bandwidth
> parameters. Read the values directly from MCHBAR offsets 0x5918 and
> 0x4000(4). This is a temporary WA until the PUNIT FW returns valid
> values.

This is supposed to be fixed for quite some time and this WA shouldn't
be needed. Is this really happening?

> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: Jani Saarinen <jani.saarinen@intel.com>
> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

humn... no s-o-b from you?

Lucas De Marchi

> ---
>  drivers/gpu/drm/i915/display/intel_bw.c | 54 ++++++++++++++++++++++++-
>  1 file changed, 53 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
> index bfb398f0432e..f3d8ff4ee0db 100644
> --- a/drivers/gpu/drm/i915/display/intel_bw.c
> +++ b/drivers/gpu/drm/i915/display/intel_bw.c
> @@ -23,6 +23,53 @@ struct intel_qgv_info {
>         u8 t_bl;
>  };
>
> +#define SA_PERF_STATUS_0_0_0_MCHBAR_PC _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5918)
> +#define  DG1_QCLK_RATIO_MASK (0xFF << 2)
> +#define  DG1_QCLK_RATIO_SHIFT 2
> +#define  DG1_QCLK_REFERENCE (1 << 10)
> +
> +#define MCHBAR_CH0_CR_TC_PRE_0_0_0_MCHBAR _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x4000)
> +#define MCHBAR_CH0_CR_TC_PRE_0_0_0_MCHBAR_HIGH _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x4004)
> +#define MCHBAR_CH1_CR_TC_PRE_0_0_0_MCHBAR _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x4400)
> +#define MCHBAR_CH1_CR_TC_PRE_0_0_0_MCHBAR_HIGH _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x4404)
> +#define  DG1_DRAM_T_RCD_MASK (0x7F << 9)
> +#define  DG1_DRAM_T_RCD_SHIFT 9
> +#define  DG1_DRAM_T_RDPRE_MASK (0x3F << 11)
> +#define  DG1_DRAM_T_RDPRE_SHIFT 11
> +#define  DG1_DRAM_T_RAS_MASK (0xFF << 1)
> +#define  DG1_DRAM_T_RAS_SHIFT 1
> +#define  DG1_DRAM_T_RP_MASK (0x7F << 0)
> +#define  DG1_DRAM_T_RP_SHIFT 0
> +
> +static int dg1_mchbar_read_qgv_point_info(struct drm_i915_private *dev_priv,
> +                                         struct intel_qgv_point *sp,
> +                                         int point)
> +{
> +       u32 val = 0;
> +       u32 dclk_ratio = 0, dclk_reference = 0;
> +
> +       val = intel_uncore_read(&dev_priv->uncore, SA_PERF_STATUS_0_0_0_MCHBAR_PC);
> +       dclk_ratio = (val & DG1_QCLK_RATIO_MASK) >> DG1_QCLK_RATIO_SHIFT;
> +       if (val & DG1_QCLK_REFERENCE)
> +               dclk_reference = 6; /* 6 * 16.666 MHz = 100 MHz */
> +       else
> +               dclk_reference = 8; /* 8 * 16.666 MHz = 133 MHz */
> +       sp->dclk = dclk_ratio * dclk_reference;
> +       if (sp->dclk == 0)
> +               return -EINVAL;
> +
> +       val = intel_uncore_read(&dev_priv->uncore, MCHBAR_CH0_CR_TC_PRE_0_0_0_MCHBAR);
> +       sp->t_rp = (val & DG1_DRAM_T_RP_MASK) >> DG1_DRAM_T_RP_SHIFT;
> +       sp->t_rdpre = (val & DG1_DRAM_T_RDPRE_MASK) >> DG1_DRAM_T_RDPRE_SHIFT;
> +
> +       val = intel_uncore_read(&dev_priv->uncore, MCHBAR_CH0_CR_TC_PRE_0_0_0_MCHBAR_HIGH);
> +       sp->t_rcd = (val & DG1_DRAM_T_RCD_MASK) >> DG1_DRAM_T_RCD_SHIFT;
> +       sp->t_ras = (val & DG1_DRAM_T_RAS_MASK) >> DG1_DRAM_T_RAS_SHIFT;
> +
> +       sp->t_rc = sp->t_rp + sp->t_ras;
> +       return 0;
> +}
> +
>  static int icl_pcode_read_qgv_point_info(struct drm_i915_private *dev_priv,
>                                          struct intel_qgv_point *sp,
>                                          int point)
> @@ -100,7 +147,12 @@ static int icl_get_qgv_points(struct drm_i915_private *dev_priv,
>                 struct intel_qgv_point *sp = &qi->points[i];
>
>                 ret = icl_pcode_read_qgv_point_info(dev_priv, sp, i);
> -               if (ret)
> +               if (IS_DG1(dev_priv) && (ret || sp->dclk == 0)) {
> +                       drm_dbg_kms(&dev_priv->drm, "Failed to get memory subsystem information via pcode. IFWI needs update. Trying with MCHBAR\n");
> +                       ret = dg1_mchbar_read_qgv_point_info(dev_priv, sp, i);
> +                       if (ret)
> +                               return ret;
> +               } else if (ret)
>                         return ret;
>
>                 drm_dbg_kms(&dev_priv->drm,
> --
> 2.26.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Matthew Auld June 21, 2021, 8:44 a.m. UTC | #3 
On 21/06/2021 06:46, Lucas De Marchi wrote:
> On Fri, Jun 18, 2021 at 9:14 AM Matthew Auld <matthew.auld@intel.com> wrote:
>>
>> From: Clint Taylor <clinton.a.taylor@intel.com>
>>
>> The PUNIT FW is currently returning 0 for all memory bandwidth
>> parameters. Read the values directly from MCHBAR offsets 0x5918 and
>> 0x4000(4). This is a temporary WA until the PUNIT FW returns valid
>> values.
> 
> This is supposed to be fixed for quite some time and this WA shouldn't
> be needed. Is this really happening?

Yes, from the sample of machines I have tried recently they all seem to 
need it. Same for Thomas, who also confirmed that these two patches are 
needed. In dmesg I get:

[drm:icl_get_bw_info.isra.0 [i915]] Failed to get memory subsystem 
information via pcode. IFWI needs update. Trying with MCHBAR

Internal CI looks to be the same from a quick look.

> 
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Cc: Matt Roper <matthew.d.roper@intel.com>
>> Cc: Jani Saarinen <jani.saarinen@intel.com>
>> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> 
> humn... no s-o-b from you?

Will add.

> 
> Lucas De Marchi
> 
>> ---
>>   drivers/gpu/drm/i915/display/intel_bw.c | 54 ++++++++++++++++++++++++-
>>   1 file changed, 53 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
>> index bfb398f0432e..f3d8ff4ee0db 100644
>> --- a/drivers/gpu/drm/i915/display/intel_bw.c
>> +++ b/drivers/gpu/drm/i915/display/intel_bw.c
>> @@ -23,6 +23,53 @@ struct intel_qgv_info {
>>          u8 t_bl;
>>   };
>>
>> +#define SA_PERF_STATUS_0_0_0_MCHBAR_PC _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5918)
>> +#define  DG1_QCLK_RATIO_MASK (0xFF << 2)
>> +#define  DG1_QCLK_RATIO_SHIFT 2
>> +#define  DG1_QCLK_REFERENCE (1 << 10)
>> +
>> +#define MCHBAR_CH0_CR_TC_PRE_0_0_0_MCHBAR _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x4000)
>> +#define MCHBAR_CH0_CR_TC_PRE_0_0_0_MCHBAR_HIGH _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x4004)
>> +#define MCHBAR_CH1_CR_TC_PRE_0_0_0_MCHBAR _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x4400)
>> +#define MCHBAR_CH1_CR_TC_PRE_0_0_0_MCHBAR_HIGH _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x4404)
>> +#define  DG1_DRAM_T_RCD_MASK (0x7F << 9)
>> +#define  DG1_DRAM_T_RCD_SHIFT 9
>> +#define  DG1_DRAM_T_RDPRE_MASK (0x3F << 11)
>> +#define  DG1_DRAM_T_RDPRE_SHIFT 11
>> +#define  DG1_DRAM_T_RAS_MASK (0xFF << 1)
>> +#define  DG1_DRAM_T_RAS_SHIFT 1
>> +#define  DG1_DRAM_T_RP_MASK (0x7F << 0)
>> +#define  DG1_DRAM_T_RP_SHIFT 0
>> +
>> +static int dg1_mchbar_read_qgv_point_info(struct drm_i915_private *dev_priv,
>> +                                         struct intel_qgv_point *sp,
>> +                                         int point)
>> +{
>> +       u32 val = 0;
>> +       u32 dclk_ratio = 0, dclk_reference = 0;
>> +
>> +       val = intel_uncore_read(&dev_priv->uncore, SA_PERF_STATUS_0_0_0_MCHBAR_PC);
>> +       dclk_ratio = (val & DG1_QCLK_RATIO_MASK) >> DG1_QCLK_RATIO_SHIFT;
>> +       if (val & DG1_QCLK_REFERENCE)
>> +               dclk_reference = 6; /* 6 * 16.666 MHz = 100 MHz */
>> +       else
>> +               dclk_reference = 8; /* 8 * 16.666 MHz = 133 MHz */
>> +       sp->dclk = dclk_ratio * dclk_reference;
>> +       if (sp->dclk == 0)
>> +               return -EINVAL;
>> +
>> +       val = intel_uncore_read(&dev_priv->uncore, MCHBAR_CH0_CR_TC_PRE_0_0_0_MCHBAR);
>> +       sp->t_rp = (val & DG1_DRAM_T_RP_MASK) >> DG1_DRAM_T_RP_SHIFT;
>> +       sp->t_rdpre = (val & DG1_DRAM_T_RDPRE_MASK) >> DG1_DRAM_T_RDPRE_SHIFT;
>> +
>> +       val = intel_uncore_read(&dev_priv->uncore, MCHBAR_CH0_CR_TC_PRE_0_0_0_MCHBAR_HIGH);
>> +       sp->t_rcd = (val & DG1_DRAM_T_RCD_MASK) >> DG1_DRAM_T_RCD_SHIFT;
>> +       sp->t_ras = (val & DG1_DRAM_T_RAS_MASK) >> DG1_DRAM_T_RAS_SHIFT;
>> +
>> +       sp->t_rc = sp->t_rp + sp->t_ras;
>> +       return 0;
>> +}
>> +
>>   static int icl_pcode_read_qgv_point_info(struct drm_i915_private *dev_priv,
>>                                           struct intel_qgv_point *sp,
>>                                           int point)
>> @@ -100,7 +147,12 @@ static int icl_get_qgv_points(struct drm_i915_private *dev_priv,
>>                  struct intel_qgv_point *sp = &qi->points[i];
>>
>>                  ret = icl_pcode_read_qgv_point_info(dev_priv, sp, i);
>> -               if (ret)
>> +               if (IS_DG1(dev_priv) && (ret || sp->dclk == 0)) {
>> +                       drm_dbg_kms(&dev_priv->drm, "Failed to get memory subsystem information via pcode. IFWI needs update. Trying with MCHBAR\n");
>> +                       ret = dg1_mchbar_read_qgv_point_info(dev_priv, sp, i);
>> +                       if (ret)
>> +                               return ret;
>> +               } else if (ret)
>>                          return ret;
>>
>>                  drm_dbg_kms(&dev_priv->drm,
>> --
>> 2.26.3
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Lucas De Marchi June 21, 2021, 11:43 p.m. UTC | #4 
On Mon, Jun 21, 2021 at 09:44:35AM +0100, Matthew Auld wrote:
>On 21/06/2021 06:46, Lucas De Marchi wrote:
>>On Fri, Jun 18, 2021 at 9:14 AM Matthew Auld <matthew.auld@intel.com> wrote:
>>>
>>>From: Clint Taylor <clinton.a.taylor@intel.com>
>>>
>>>The PUNIT FW is currently returning 0 for all memory bandwidth
>>>parameters. Read the values directly from MCHBAR offsets 0x5918 and
>>>0x4000(4). This is a temporary WA until the PUNIT FW returns valid
>>>values.
>>
>>This is supposed to be fixed for quite some time and this WA shouldn't
>>be needed. Is this really happening?
>
>Yes, from the sample of machines I have tried recently they all seem 
>to need it. Same for Thomas, who also confirmed that these two patches 
>are needed. In dmesg I get:
>
>[drm:icl_get_bw_info.isra.0 [i915]] Failed to get memory subsystem 
>information via pcode. IFWI needs update. Trying with MCHBAR

My point was:  is this something reproducible on machines outside Intel?
Machines from CI or internal should follow the recommendation from the
message above: "IFWI needs update".

>Internal CI looks to be the same from a quick look.

:(

>
>>
>>>Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>Cc: Matt Roper <matthew.d.roper@intel.com>
>>>Cc: Jani Saarinen <jani.saarinen@intel.com>
>>>Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
>>>Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>
>>humn... no s-o-b from you?
>
>Will add.
>
>>
>>Lucas De Marchi
>>
>>>---
>>>  drivers/gpu/drm/i915/display/intel_bw.c | 54 ++++++++++++++++++++++++-
>>>  1 file changed, 53 insertions(+), 1 deletion(-)
>>>
>>>diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
>>>index bfb398f0432e..f3d8ff4ee0db 100644
>>>--- a/drivers/gpu/drm/i915/display/intel_bw.c
>>>+++ b/drivers/gpu/drm/i915/display/intel_bw.c
>>>@@ -23,6 +23,53 @@ struct intel_qgv_info {
>>>         u8 t_bl;
>>>  };
>>>
>>>+#define SA_PERF_STATUS_0_0_0_MCHBAR_PC _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5918)
>>>+#define  DG1_QCLK_RATIO_MASK (0xFF << 2)
>>>+#define  DG1_QCLK_RATIO_SHIFT 2
>>>+#define  DG1_QCLK_REFERENCE (1 << 10)
>>>+
>>>+#define MCHBAR_CH0_CR_TC_PRE_0_0_0_MCHBAR _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x4000)
>>>+#define MCHBAR_CH0_CR_TC_PRE_0_0_0_MCHBAR_HIGH _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x4004)
>>>+#define MCHBAR_CH1_CR_TC_PRE_0_0_0_MCHBAR _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x4400)
>>>+#define MCHBAR_CH1_CR_TC_PRE_0_0_0_MCHBAR_HIGH _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x4404)
>>>+#define  DG1_DRAM_T_RCD_MASK (0x7F << 9)
>>>+#define  DG1_DRAM_T_RCD_SHIFT 9
>>>+#define  DG1_DRAM_T_RDPRE_MASK (0x3F << 11)
>>>+#define  DG1_DRAM_T_RDPRE_SHIFT 11
>>>+#define  DG1_DRAM_T_RAS_MASK (0xFF << 1)
>>>+#define  DG1_DRAM_T_RAS_SHIFT 1
>>>+#define  DG1_DRAM_T_RP_MASK (0x7F << 0)
>>>+#define  DG1_DRAM_T_RP_SHIFT 0
>>>+
>>>+static int dg1_mchbar_read_qgv_point_info(struct drm_i915_private *dev_priv,
>>>+                                         struct intel_qgv_point *sp,
>>>+                                         int point)
>>>+{
>>>+       u32 val = 0;
>>>+       u32 dclk_ratio = 0, dclk_reference = 0;
>>>+
>>>+       val = intel_uncore_read(&dev_priv->uncore, SA_PERF_STATUS_0_0_0_MCHBAR_PC);
>>>+       dclk_ratio = (val & DG1_QCLK_RATIO_MASK) >> DG1_QCLK_RATIO_SHIFT;
>>>+       if (val & DG1_QCLK_REFERENCE)
>>>+               dclk_reference = 6; /* 6 * 16.666 MHz = 100 MHz */
>>>+       else
>>>+               dclk_reference = 8; /* 8 * 16.666 MHz = 133 MHz */
>>>+       sp->dclk = dclk_ratio * dclk_reference;
>>>+       if (sp->dclk == 0)
>>>+               return -EINVAL;
>>>+
>>>+       val = intel_uncore_read(&dev_priv->uncore, MCHBAR_CH0_CR_TC_PRE_0_0_0_MCHBAR);
>>>+       sp->t_rp = (val & DG1_DRAM_T_RP_MASK) >> DG1_DRAM_T_RP_SHIFT;
>>>+       sp->t_rdpre = (val & DG1_DRAM_T_RDPRE_MASK) >> DG1_DRAM_T_RDPRE_SHIFT;
>>>+
>>>+       val = intel_uncore_read(&dev_priv->uncore, MCHBAR_CH0_CR_TC_PRE_0_0_0_MCHBAR_HIGH);
>>>+       sp->t_rcd = (val & DG1_DRAM_T_RCD_MASK) >> DG1_DRAM_T_RCD_SHIFT;
>>>+       sp->t_ras = (val & DG1_DRAM_T_RAS_MASK) >> DG1_DRAM_T_RAS_SHIFT;
>>>+
>>>+       sp->t_rc = sp->t_rp + sp->t_ras;
>>>+       return 0;
>>>+}
>>>+
>>>  static int icl_pcode_read_qgv_point_info(struct drm_i915_private *dev_priv,
>>>                                          struct intel_qgv_point *sp,
>>>                                          int point)
>>>@@ -100,7 +147,12 @@ static int icl_get_qgv_points(struct drm_i915_private *dev_priv,
>>>                 struct intel_qgv_point *sp = &qi->points[i];
>>>
>>>                 ret = icl_pcode_read_qgv_point_info(dev_priv, sp, i);
>>>-               if (ret)
>>>+               if (IS_DG1(dev_priv) && (ret || sp->dclk == 0)) {
>>>+                       drm_dbg_kms(&dev_priv->drm, "Failed to get memory subsystem information via pcode. IFWI needs update. Trying with MCHBAR\n");
>>>+                       ret = dg1_mchbar_read_qgv_point_info(dev_priv, sp, i);
>>>+                       if (ret)
>>>+                               return ret;
>>>+               } else if (ret)
>>>                         return ret;

IMO this looks a little bit nicer to read:

	ret = icl_pcode_read_qgv_point_info(dev_priv, sp, i);
	if (IS_DG1(dev_priv) && (ret || sp->dclk == 0)) {
		drm_dbg_kms(&dev_priv->drm, "Failed to get memory subsystem information via pcode. IFWI needs update. Trying with MCHBAR\n");
		ret = dg1_mchbar_read_qgv_point_info(dev_priv, sp, i);
	}

	if (ret)
		return ret;


Lucas De Marchi
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
index bfb398f0432e..f3d8ff4ee0db 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.c
+++ b/drivers/gpu/drm/i915/display/intel_bw.c
@@ -23,6 +23,53 @@  struct intel_qgv_info {
 	u8 t_bl;
 };
 
+#define SA_PERF_STATUS_0_0_0_MCHBAR_PC _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5918)
+#define  DG1_QCLK_RATIO_MASK (0xFF << 2)
+#define  DG1_QCLK_RATIO_SHIFT 2
+#define  DG1_QCLK_REFERENCE (1 << 10)
+
+#define MCHBAR_CH0_CR_TC_PRE_0_0_0_MCHBAR _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x4000)
+#define MCHBAR_CH0_CR_TC_PRE_0_0_0_MCHBAR_HIGH _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x4004)
+#define MCHBAR_CH1_CR_TC_PRE_0_0_0_MCHBAR _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x4400)
+#define MCHBAR_CH1_CR_TC_PRE_0_0_0_MCHBAR_HIGH _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x4404)
+#define  DG1_DRAM_T_RCD_MASK (0x7F << 9)
+#define  DG1_DRAM_T_RCD_SHIFT 9
+#define  DG1_DRAM_T_RDPRE_MASK (0x3F << 11)
+#define  DG1_DRAM_T_RDPRE_SHIFT 11
+#define  DG1_DRAM_T_RAS_MASK (0xFF << 1)
+#define  DG1_DRAM_T_RAS_SHIFT 1
+#define  DG1_DRAM_T_RP_MASK (0x7F << 0)
+#define  DG1_DRAM_T_RP_SHIFT 0
+
+static int dg1_mchbar_read_qgv_point_info(struct drm_i915_private *dev_priv,
+					  struct intel_qgv_point *sp,
+					  int point)
+{
+	u32 val = 0;
+	u32 dclk_ratio = 0, dclk_reference = 0;
+
+	val = intel_uncore_read(&dev_priv->uncore, SA_PERF_STATUS_0_0_0_MCHBAR_PC);
+	dclk_ratio = (val & DG1_QCLK_RATIO_MASK) >> DG1_QCLK_RATIO_SHIFT;
+	if (val & DG1_QCLK_REFERENCE)
+		dclk_reference = 6; /* 6 * 16.666 MHz = 100 MHz */
+	else
+		dclk_reference = 8; /* 8 * 16.666 MHz = 133 MHz */
+	sp->dclk = dclk_ratio * dclk_reference;
+	if (sp->dclk == 0)
+		return -EINVAL;
+
+	val = intel_uncore_read(&dev_priv->uncore, MCHBAR_CH0_CR_TC_PRE_0_0_0_MCHBAR);
+	sp->t_rp = (val & DG1_DRAM_T_RP_MASK) >> DG1_DRAM_T_RP_SHIFT;
+	sp->t_rdpre = (val & DG1_DRAM_T_RDPRE_MASK) >> DG1_DRAM_T_RDPRE_SHIFT;
+
+	val = intel_uncore_read(&dev_priv->uncore, MCHBAR_CH0_CR_TC_PRE_0_0_0_MCHBAR_HIGH);
+	sp->t_rcd = (val & DG1_DRAM_T_RCD_MASK) >> DG1_DRAM_T_RCD_SHIFT;
+	sp->t_ras = (val & DG1_DRAM_T_RAS_MASK) >> DG1_DRAM_T_RAS_SHIFT;
+
+	sp->t_rc = sp->t_rp + sp->t_ras;
+	return 0;
+}
+
 static int icl_pcode_read_qgv_point_info(struct drm_i915_private *dev_priv,
 					 struct intel_qgv_point *sp,
 					 int point)
@@ -100,7 +147,12 @@  static int icl_get_qgv_points(struct drm_i915_private *dev_priv,
 		struct intel_qgv_point *sp = &qi->points[i];
 
 		ret = icl_pcode_read_qgv_point_info(dev_priv, sp, i);
-		if (ret)
+		if (IS_DG1(dev_priv) && (ret || sp->dclk == 0)) {
+			drm_dbg_kms(&dev_priv->drm, "Failed to get memory subsystem information via pcode. IFWI needs update. Trying with MCHBAR\n");
+			ret = dg1_mchbar_read_qgv_point_info(dev_priv, sp, i);
+			if (ret)
+				return ret;
+		} else if (ret)
 			return ret;
 
 		drm_dbg_kms(&dev_priv->drm,