* [Intel-gfx] [PATCH 1/4] drm/i915: Nuke not needed members of dram_info
@ 2021-01-20 15:16 José Roberto de Souza
2021-01-20 15:16 ` [Intel-gfx] [PATCH 2/4] drm/i915/gen11+: Only load DRAM information from pcode José Roberto de Souza
` (6 more replies)
0 siblings, 7 replies; 12+ messages in thread
From: José Roberto de Souza @ 2021-01-20 15:16 UTC (permalink / raw)
To: intel-gfx
Valid, ranks and bandwidth_kbps are set into dram_info but are not
used anywhere else so nuking it.
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
drivers/gpu/drm/i915/i915_drv.c | 4 +--
drivers/gpu/drm/i915/i915_drv.h | 3 --
drivers/gpu/drm/i915/intel_dram.c | 47 +++++++------------------------
3 files changed, 12 insertions(+), 42 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index f5666b44ea9d..a1cc60de99f0 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -609,8 +609,8 @@ static int i915_driver_hw_probe(struct drm_i915_private *dev_priv)
intel_opregion_setup(dev_priv);
/*
- * Fill the dram structure to get the system raw bandwidth and
- * dram info. This will be used for memory latency calculation.
+ * Fill the dram structure to get the system dram info. This will be
+ * used for memory latency calculation.
*/
intel_dram_detect(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8376cff5ba86..250e92910fa1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1134,11 +1134,8 @@ struct drm_i915_private {
} wm;
struct dram_info {
- bool valid;
bool is_16gb_dimm;
u8 num_channels;
- u8 ranks;
- u32 bandwidth_kbps;
bool symmetric_memory;
enum intel_dram_type {
INTEL_DRAM_UNKNOWN,
diff --git a/drivers/gpu/drm/i915/intel_dram.c b/drivers/gpu/drm/i915/intel_dram.c
index 4754296a250e..694fbd8c9cd4 100644
--- a/drivers/gpu/drm/i915/intel_dram.c
+++ b/drivers/gpu/drm/i915/intel_dram.c
@@ -201,17 +201,7 @@ skl_dram_get_channels_info(struct drm_i915_private *i915)
return -EINVAL;
}
- /*
- * If any of the channel is single rank channel, worst case output
- * will be same as if single rank memory, so consider single rank
- * memory.
- */
- if (ch0.ranks == 1 || ch1.ranks == 1)
- dram_info->ranks = 1;
- else
- dram_info->ranks = max(ch0.ranks, ch1.ranks);
-
- if (dram_info->ranks == 0) {
+ if (ch0.ranks == 0 && ch1.ranks == 0) {
drm_info(&i915->drm, "couldn't get memory rank information\n");
return -EINVAL;
}
@@ -269,16 +259,12 @@ skl_get_dram_info(struct drm_i915_private *i915)
mem_freq_khz = DIV_ROUND_UP((val & SKL_REQ_DATA_MASK) *
SKL_MEMORY_FREQ_MULTIPLIER_HZ, 1000);
- dram_info->bandwidth_kbps = dram_info->num_channels *
- mem_freq_khz * 8;
-
- if (dram_info->bandwidth_kbps == 0) {
+ if (dram_info->num_channels * mem_freq_khz == 0) {
drm_info(&i915->drm,
"Couldn't get system memory bandwidth\n");
return -EINVAL;
}
- dram_info->valid = true;
return 0;
}
@@ -365,7 +351,7 @@ static int bxt_get_dram_info(struct drm_i915_private *i915)
struct dram_info *dram_info = &i915->dram_info;
u32 dram_channels;
u32 mem_freq_khz, val;
- u8 num_active_channels;
+ u8 num_active_channels, valid_ranks = 0;
int i;
val = intel_uncore_read(&i915->uncore, BXT_P_CR_MC_BIOS_REQ_0_0_0);
@@ -375,10 +361,7 @@ static int bxt_get_dram_info(struct drm_i915_private *i915)
dram_channels = val & BXT_DRAM_CHANNEL_ACTIVE_MASK;
num_active_channels = hweight32(dram_channels);
- /* Each active bit represents 4-byte channel */
- dram_info->bandwidth_kbps = (mem_freq_khz * num_active_channels * 4);
-
- if (dram_info->bandwidth_kbps == 0) {
+ if (mem_freq_khz * num_active_channels == 0) {
drm_info(&i915->drm,
"Couldn't get system memory bandwidth\n");
return -EINVAL;
@@ -410,27 +393,18 @@ static int bxt_get_dram_info(struct drm_i915_private *i915)
dimm.size, dimm.width, dimm.ranks,
intel_dram_type_str(type));
- /*
- * If any of the channel is single rank channel,
- * worst case output will be same as if single rank
- * memory, so consider single rank memory.
- */
- if (dram_info->ranks == 0)
- dram_info->ranks = dimm.ranks;
- else if (dimm.ranks == 1)
- dram_info->ranks = 1;
+ if (valid_ranks == 0)
+ valid_ranks = dimm.ranks;
if (type != INTEL_DRAM_UNKNOWN)
dram_info->type = type;
}
- if (dram_info->type == INTEL_DRAM_UNKNOWN || dram_info->ranks == 0) {
+ if (dram_info->type == INTEL_DRAM_UNKNOWN || valid_ranks == 0) {
drm_info(&i915->drm, "couldn't get memory information\n");
return -EINVAL;
}
- dram_info->valid = true;
-
return 0;
}
@@ -456,11 +430,10 @@ void intel_dram_detect(struct drm_i915_private *i915)
if (ret)
return;
- drm_dbg_kms(&i915->drm, "DRAM bandwidth: %u kBps, channels: %u\n",
- dram_info->bandwidth_kbps, dram_info->num_channels);
+ drm_dbg_kms(&i915->drm, "DRAM channels: %u\n", dram_info->num_channels);
- drm_dbg_kms(&i915->drm, "DRAM ranks: %u, 16Gb DIMMs: %s\n",
- dram_info->ranks, yesno(dram_info->is_16gb_dimm));
+ drm_dbg_kms(&i915->drm, "DRAM 16Gb DIMMs: %s\n",
+ yesno(dram_info->is_16gb_dimm));
}
static u32 gen9_edram_size_mb(struct drm_i915_private *i915, u32 cap)
--
2.30.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Intel-gfx] [PATCH 2/4] drm/i915/gen11+: Only load DRAM information from pcode
2021-01-20 15:16 [Intel-gfx] [PATCH 1/4] drm/i915: Nuke not needed members of dram_info José Roberto de Souza
@ 2021-01-20 15:16 ` José Roberto de Souza
2021-01-27 14:49 ` Lucas De Marchi
2021-01-20 15:16 ` [Intel-gfx] [PATCH 3/4] drm/i915: Fail driver probe when unable to load DRAM information José Roberto de Souza
` (5 subsequent siblings)
6 siblings, 1 reply; 12+ messages in thread
From: José Roberto de Souza @ 2021-01-20 15:16 UTC (permalink / raw)
To: intel-gfx
Up to now we were reading some DRAM information from MCHBAR register
and from pcode what is already not good but some GEN12(TGL-H and ADL-S)
platforms have MCHBAR DRAM information in different offsets.
This was notified to HW team that decided that the best alternative is
always apply the 16gb_dimm watermark adjustment for GEN12+ platforms
and read the remaning DRAM information needed to other display
programming from pcode.
So here moving the DRAM pcode function to intel_dram.c, removing
the duplicated fields from intel_qgv_info, setting and using
information from dram_info.
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
drivers/gpu/drm/i915/display/intel_bw.c | 98 +++++--------------------
drivers/gpu/drm/i915/i915_drv.c | 5 +-
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/intel_dram.c | 77 ++++++++++++++++++-
4 files changed, 97 insertions(+), 84 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
index bd060404d249..1368bd96ed73 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.c
+++ b/drivers/gpu/drm/i915/display/intel_bw.c
@@ -19,77 +19,9 @@ struct intel_qgv_point {
struct intel_qgv_info {
struct intel_qgv_point points[I915_NUM_QGV_POINTS];
- u8 num_points;
- u8 num_channels;
u8 t_bl;
- enum intel_dram_type dram_type;
};
-static int icl_pcode_read_mem_global_info(struct drm_i915_private *dev_priv,
- struct intel_qgv_info *qi)
-{
- u32 val = 0;
- int ret;
-
- ret = sandybridge_pcode_read(dev_priv,
- ICL_PCODE_MEM_SUBSYSYSTEM_INFO |
- ICL_PCODE_MEM_SS_READ_GLOBAL_INFO,
- &val, NULL);
- if (ret)
- return ret;
-
- if (IS_GEN(dev_priv, 12)) {
- switch (val & 0xf) {
- case 0:
- qi->dram_type = INTEL_DRAM_DDR4;
- break;
- case 3:
- qi->dram_type = INTEL_DRAM_LPDDR4;
- break;
- case 4:
- qi->dram_type = INTEL_DRAM_DDR3;
- break;
- case 5:
- qi->dram_type = INTEL_DRAM_LPDDR3;
- break;
- default:
- MISSING_CASE(val & 0xf);
- break;
- }
- } else if (IS_GEN(dev_priv, 11)) {
- switch (val & 0xf) {
- case 0:
- qi->dram_type = INTEL_DRAM_DDR4;
- break;
- case 1:
- qi->dram_type = INTEL_DRAM_DDR3;
- break;
- case 2:
- qi->dram_type = INTEL_DRAM_LPDDR3;
- break;
- case 3:
- qi->dram_type = INTEL_DRAM_LPDDR4;
- break;
- default:
- MISSING_CASE(val & 0xf);
- break;
- }
- } else {
- MISSING_CASE(INTEL_GEN(dev_priv));
- qi->dram_type = INTEL_DRAM_LPDDR3; /* Conservative default */
- }
-
- qi->num_channels = (val & 0xf0) >> 4;
- qi->num_points = (val & 0xf00) >> 8;
-
- if (IS_GEN(dev_priv, 12))
- qi->t_bl = qi->dram_type == INTEL_DRAM_DDR4 ? 4 : 16;
- else if (IS_GEN(dev_priv, 11))
- qi->t_bl = qi->dram_type == INTEL_DRAM_DDR4 ? 4 : 8;
-
- return 0;
-}
-
static int icl_pcode_read_qgv_point_info(struct drm_i915_private *dev_priv,
struct intel_qgv_point *sp,
int point)
@@ -139,17 +71,19 @@ int icl_pcode_restrict_qgv_points(struct drm_i915_private *dev_priv,
static int icl_get_qgv_points(struct drm_i915_private *dev_priv,
struct intel_qgv_info *qi)
{
+ struct dram_info *dram_info = &dev_priv->dram_info;
int i, ret;
- ret = icl_pcode_read_mem_global_info(dev_priv, qi);
- if (ret)
- return ret;
+ if (IS_GEN(dev_priv, 12))
+ qi->t_bl = dev_priv->dram_info.type == INTEL_DRAM_DDR4 ? 4 : 16;
+ else if (IS_GEN(dev_priv, 11))
+ qi->t_bl = dev_priv->dram_info.type == INTEL_DRAM_DDR4 ? 4 : 8;
if (drm_WARN_ON(&dev_priv->drm,
- qi->num_points > ARRAY_SIZE(qi->points)))
- qi->num_points = ARRAY_SIZE(qi->points);
+ dram_info->qgv_points > ARRAY_SIZE(qi->points)))
+ dram_info->qgv_points = ARRAY_SIZE(qi->points);
- for (i = 0; i < qi->num_points; i++) {
+ for (i = 0; i < dram_info->qgv_points; i++) {
struct intel_qgv_point *sp = &qi->points[i];
ret = icl_pcode_read_qgv_point_info(dev_priv, sp, i);
@@ -171,12 +105,13 @@ static int icl_calc_bw(int dclk, int num, int den)
return DIV_ROUND_CLOSEST(num * dclk * 100, den * 6);
}
-static int icl_sagv_max_dclk(const struct intel_qgv_info *qi)
+static int icl_sagv_max_dclk(struct drm_i915_private *dev_priv,
+ const struct intel_qgv_info *qi)
{
u16 dclk = 0;
int i;
- for (i = 0; i < qi->num_points; i++)
+ for (i = 0; i < dev_priv->dram_info.qgv_points; i++)
dclk = max(dclk, qi->points[i].dclk);
return dclk;
@@ -207,6 +142,7 @@ static const struct intel_sa_info rkl_sa_info = {
static int icl_get_bw_info(struct drm_i915_private *dev_priv, const struct intel_sa_info *sa)
{
+ struct dram_info *dram_info = &dev_priv->dram_info;
struct intel_qgv_info qi = {};
bool is_y_tile = true; /* assume y tile may be used */
int num_channels;
@@ -222,10 +158,10 @@ static int icl_get_bw_info(struct drm_i915_private *dev_priv, const struct intel
"Failed to get memory subsystem information, ignoring bandwidth limits");
return ret;
}
- num_channels = qi.num_channels;
+ num_channels = dram_info->num_channels;
deinterleave = DIV_ROUND_UP(num_channels, is_y_tile ? 4 : 2);
- dclk_max = icl_sagv_max_dclk(&qi);
+ dclk_max = icl_sagv_max_dclk(dev_priv, &qi);
ipqdepthpch = 16;
@@ -241,9 +177,9 @@ static int icl_get_bw_info(struct drm_i915_private *dev_priv, const struct intel
clpchgroup = (sa->deburst * deinterleave / num_channels) << i;
bi->num_planes = (ipqdepth - clpchgroup) / clpchgroup + 1;
- bi->num_qgv_points = qi.num_points;
+ bi->num_qgv_points = dram_info->qgv_points;
- for (j = 0; j < qi.num_points; j++) {
+ for (j = 0; j < dram_info->qgv_points; j++) {
const struct intel_qgv_point *sp = &qi.points[j];
int ct, bw;
@@ -274,7 +210,7 @@ static int icl_get_bw_info(struct drm_i915_private *dev_priv, const struct intel
* SAGV point, but we can't send PCode commands to restrict it
* as it will fail and pointless anyway.
*/
- if (qi.num_points == 1)
+ if (dram_info->qgv_points == 1)
dev_priv->sagv_status = I915_SAGV_NOT_CONTROLLED;
else
dev_priv->sagv_status = I915_SAGV_ENABLED;
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index a1cc60de99f0..66f763fe7a83 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -608,14 +608,15 @@ static int i915_driver_hw_probe(struct drm_i915_private *dev_priv)
goto err_msi;
intel_opregion_setup(dev_priv);
+
+ intel_pcode_init(dev_priv);
+
/*
* Fill the dram structure to get the system dram info. This will be
* used for memory latency calculation.
*/
intel_dram_detect(dev_priv);
- intel_pcode_init(dev_priv);
-
intel_bw_init_hw(dev_priv);
return 0;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 250e92910fa1..a2ae21082b34 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1144,6 +1144,7 @@ struct drm_i915_private {
INTEL_DRAM_LPDDR3,
INTEL_DRAM_LPDDR4
} type;
+ u8 qgv_points;
} dram_info;
struct intel_bw_info {
diff --git a/drivers/gpu/drm/i915/intel_dram.c b/drivers/gpu/drm/i915/intel_dram.c
index 694fbd8c9cd4..1298823c957c 100644
--- a/drivers/gpu/drm/i915/intel_dram.c
+++ b/drivers/gpu/drm/i915/intel_dram.c
@@ -5,6 +5,7 @@
#include "i915_drv.h"
#include "intel_dram.h"
+#include "intel_sideband.h"
struct dram_dimm_info {
u16 size;
@@ -408,6 +409,78 @@ static int bxt_get_dram_info(struct drm_i915_private *i915)
return 0;
}
+static int icl_pcode_read_mem_global_info(struct drm_i915_private *dev_priv)
+{
+ struct dram_info *dram_info = &dev_priv->dram_info;
+ u32 val = 0;
+ int ret;
+
+ ret = sandybridge_pcode_read(dev_priv,
+ ICL_PCODE_MEM_SUBSYSYSTEM_INFO |
+ ICL_PCODE_MEM_SS_READ_GLOBAL_INFO,
+ &val, NULL);
+ if (ret)
+ return ret;
+
+ if (IS_GEN(dev_priv, 12)) {
+ switch (val & 0xf) {
+ case 0:
+ dram_info->type = INTEL_DRAM_DDR4;
+ break;
+ case 3:
+ dram_info->type = INTEL_DRAM_LPDDR4;
+ break;
+ case 4:
+ dram_info->type = INTEL_DRAM_DDR3;
+ break;
+ case 5:
+ dram_info->type = INTEL_DRAM_LPDDR3;
+ break;
+ default:
+ MISSING_CASE(val & 0xf);
+ return -1;
+ }
+ } else {
+ switch (val & 0xf) {
+ case 0:
+ dram_info->type = INTEL_DRAM_DDR4;
+ break;
+ case 1:
+ dram_info->type = INTEL_DRAM_DDR3;
+ break;
+ case 2:
+ dram_info->type = INTEL_DRAM_LPDDR3;
+ break;
+ case 3:
+ dram_info->type = INTEL_DRAM_LPDDR4;
+ break;
+ default:
+ MISSING_CASE(val & 0xf);
+ return -1;
+ }
+ }
+
+ dram_info->num_channels = (val & 0xf0) >> 4;
+ dram_info->qgv_points = (val & 0xf00) >> 8;
+
+ return 0;
+}
+
+static int gen11_get_dram_info(struct drm_i915_private *i915)
+{
+ if (INTEL_GEN(i915) == 11) {
+ int ret = skl_get_dram_info(i915);
+
+ if (ret)
+ return ret;
+ } else {
+ /* Always needed for GEN12+ */
+ i915->dram_info.is_16gb_dimm = true;
+ }
+
+ return icl_pcode_read_mem_global_info(i915);
+}
+
void intel_dram_detect(struct drm_i915_private *i915)
{
struct dram_info *dram_info = &i915->dram_info;
@@ -423,7 +496,9 @@ void intel_dram_detect(struct drm_i915_private *i915)
if (INTEL_GEN(i915) < 9 || !HAS_DISPLAY(i915))
return;
- if (IS_GEN9_LP(i915))
+ if (INTEL_GEN(i915) >= 11)
+ ret = gen11_get_dram_info(i915);
+ else if (IS_GEN9_LP(i915))
ret = bxt_get_dram_info(i915);
else
ret = skl_get_dram_info(i915);
--
2.30.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Intel-gfx] [PATCH 3/4] drm/i915: Fail driver probe when unable to load DRAM information
2021-01-20 15:16 [Intel-gfx] [PATCH 1/4] drm/i915: Nuke not needed members of dram_info José Roberto de Souza
2021-01-20 15:16 ` [Intel-gfx] [PATCH 2/4] drm/i915/gen11+: Only load DRAM information from pcode José Roberto de Souza
@ 2021-01-20 15:16 ` José Roberto de Souza
2021-01-20 15:16 ` [Intel-gfx] [PATCH 4/4] drm/i915: Rename is_16gb_dimm to wm_lv_0_adjust_needed José Roberto de Souza
` (4 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: José Roberto de Souza @ 2021-01-20 15:16 UTC (permalink / raw)
To: intel-gfx
DRAM information is required to properly program display.
Before "drm/i915/gen11+: Only load DRAM information from pcode" we
were failing driver load if unable to fetch DRAM information from
pcode form GEN11+ but we should also extend it to GEN9 plaforms.
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
drivers/gpu/drm/i915/i915_drv.c | 6 +++++-
drivers/gpu/drm/i915/intel_dram.c | 13 +++++++++----
drivers/gpu/drm/i915/intel_dram.h | 2 +-
3 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 66f763fe7a83..6bfcd3ee6c66 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -615,12 +615,16 @@ static int i915_driver_hw_probe(struct drm_i915_private *dev_priv)
* Fill the dram structure to get the system dram info. This will be
* used for memory latency calculation.
*/
- intel_dram_detect(dev_priv);
+ ret = intel_dram_detect(dev_priv);
+ if (ret)
+ goto err_dram;
intel_bw_init_hw(dev_priv);
return 0;
+err_dram:
+ intel_gvt_driver_remove(dev_priv);
err_msi:
if (pdev->msi_enabled)
pci_disable_msi(pdev);
diff --git a/drivers/gpu/drm/i915/intel_dram.c b/drivers/gpu/drm/i915/intel_dram.c
index 1298823c957c..4871d48589f9 100644
--- a/drivers/gpu/drm/i915/intel_dram.c
+++ b/drivers/gpu/drm/i915/intel_dram.c
@@ -481,7 +481,7 @@ static int gen11_get_dram_info(struct drm_i915_private *i915)
return icl_pcode_read_mem_global_info(i915);
}
-void intel_dram_detect(struct drm_i915_private *i915)
+int intel_dram_detect(struct drm_i915_private *i915)
{
struct dram_info *dram_info = &i915->dram_info;
int ret;
@@ -494,7 +494,7 @@ void intel_dram_detect(struct drm_i915_private *i915)
dram_info->is_16gb_dimm = !IS_GEN9_LP(i915);
if (INTEL_GEN(i915) < 9 || !HAS_DISPLAY(i915))
- return;
+ return 0;
if (INTEL_GEN(i915) >= 11)
ret = gen11_get_dram_info(i915);
@@ -502,13 +502,18 @@ void intel_dram_detect(struct drm_i915_private *i915)
ret = bxt_get_dram_info(i915);
else
ret = skl_get_dram_info(i915);
- if (ret)
- return;
+
+ if (ret) {
+ drm_warn(&i915->drm, "Unable to load dram information\n");
+ return ret;
+ }
drm_dbg_kms(&i915->drm, "DRAM channels: %u\n", dram_info->num_channels);
drm_dbg_kms(&i915->drm, "DRAM 16Gb DIMMs: %s\n",
yesno(dram_info->is_16gb_dimm));
+
+ return 0;
}
static u32 gen9_edram_size_mb(struct drm_i915_private *i915, u32 cap)
diff --git a/drivers/gpu/drm/i915/intel_dram.h b/drivers/gpu/drm/i915/intel_dram.h
index 4ba13c13162c..2a0f283b1a1d 100644
--- a/drivers/gpu/drm/i915/intel_dram.h
+++ b/drivers/gpu/drm/i915/intel_dram.h
@@ -9,6 +9,6 @@
struct drm_i915_private;
void intel_dram_edram_detect(struct drm_i915_private *i915);
-void intel_dram_detect(struct drm_i915_private *i915);
+int intel_dram_detect(struct drm_i915_private *i915);
#endif /* __INTEL_DRAM_H__ */
--
2.30.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Intel-gfx] [PATCH 4/4] drm/i915: Rename is_16gb_dimm to wm_lv_0_adjust_needed
2021-01-20 15:16 [Intel-gfx] [PATCH 1/4] drm/i915: Nuke not needed members of dram_info José Roberto de Souza
2021-01-20 15:16 ` [Intel-gfx] [PATCH 2/4] drm/i915/gen11+: Only load DRAM information from pcode José Roberto de Souza
2021-01-20 15:16 ` [Intel-gfx] [PATCH 3/4] drm/i915: Fail driver probe when unable to load DRAM information José Roberto de Souza
@ 2021-01-20 15:16 ` José Roberto de Souza
2021-01-27 14:56 ` Lucas De Marchi
[not found] ` <20210120183151.jpfuda4htxs5qkxq@ldmartin-desk1>
` (3 subsequent siblings)
6 siblings, 1 reply; 12+ messages in thread
From: José Roberto de Souza @ 2021-01-20 15:16 UTC (permalink / raw)
To: intel-gfx
As it now it is always required for GEN12+ the is_16gb_dimm name
do not make sense for GEN12+.
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 2 +-
drivers/gpu/drm/i915/intel_dram.c | 10 +++++-----
drivers/gpu/drm/i915/intel_pm.c | 2 +-
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a2ae21082b34..adc008c65b14 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1134,7 +1134,7 @@ struct drm_i915_private {
} wm;
struct dram_info {
- bool is_16gb_dimm;
+ bool wm_lv_0_adjust_needed;
u8 num_channels;
bool symmetric_memory;
enum intel_dram_type {
diff --git a/drivers/gpu/drm/i915/intel_dram.c b/drivers/gpu/drm/i915/intel_dram.c
index 4871d48589f9..a5850f0f25aa 100644
--- a/drivers/gpu/drm/i915/intel_dram.c
+++ b/drivers/gpu/drm/i915/intel_dram.c
@@ -207,7 +207,7 @@ skl_dram_get_channels_info(struct drm_i915_private *i915)
return -EINVAL;
}
- dram_info->is_16gb_dimm = ch0.is_16gb_dimm || ch1.is_16gb_dimm;
+ dram_info->wm_lv_0_adjust_needed = ch0.is_16gb_dimm || ch1.is_16gb_dimm;
dram_info->symmetric_memory = intel_is_dram_symmetric(&ch0, &ch1);
@@ -475,7 +475,7 @@ static int gen11_get_dram_info(struct drm_i915_private *i915)
return ret;
} else {
/* Always needed for GEN12+ */
- i915->dram_info.is_16gb_dimm = true;
+ i915->dram_info.wm_lv_0_adjust_needed = true;
}
return icl_pcode_read_mem_global_info(i915);
@@ -491,7 +491,7 @@ int intel_dram_detect(struct drm_i915_private *i915)
* This is only used for the level 0 watermark latency
* w/a which does not apply to bxt/glk.
*/
- dram_info->is_16gb_dimm = !IS_GEN9_LP(i915);
+ dram_info->wm_lv_0_adjust_needed = !IS_GEN9_LP(i915);
if (INTEL_GEN(i915) < 9 || !HAS_DISPLAY(i915))
return 0;
@@ -510,8 +510,8 @@ int intel_dram_detect(struct drm_i915_private *i915)
drm_dbg_kms(&i915->drm, "DRAM channels: %u\n", dram_info->num_channels);
- drm_dbg_kms(&i915->drm, "DRAM 16Gb DIMMs: %s\n",
- yesno(dram_info->is_16gb_dimm));
+ drm_dbg_kms(&i915->drm, "Watermark level 0 adjustment needed: %s\n",
+ yesno(dram_info->wm_lv_0_adjust_needed));
return 0;
}
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 992fce8b8d13..f778aae19f82 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2930,7 +2930,7 @@ static void intel_read_wm_latency(struct drm_i915_private *dev_priv,
* any underrun. If not able to get Dimm info assume 16GB dimm
* to avoid any underrun.
*/
- if (dev_priv->dram_info.is_16gb_dimm)
+ if (dev_priv->dram_info.wm_lv_0_adjust_needed)
wm[0] += 1;
} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
--
2.30.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Intel-gfx] [PATCH 1/4] drm/i915: Nuke not needed members of dram_info
[not found] ` <20210120185245.2eqa42nmbm6yzeki@ldmartin-desk1>
@ 2021-01-20 19:29 ` Souza, Jose
2021-01-27 14:35 ` Lucas De Marchi
0 siblings, 1 reply; 12+ messages in thread
From: Souza, Jose @ 2021-01-20 19:29 UTC (permalink / raw)
To: De Marchi, Lucas; +Cc: intel-gfx
On Wed, 2021-01-20 at 10:52 -0800, Lucas De Marchi wrote:
> On Wed, Jan 20, 2021 at 10:42:46AM -0800, Jose Souza wrote:
> > On Wed, 2021-01-20 at 10:31 -0800, Lucas De Marchi wrote:
> > > On Wed, Jan 20, 2021 at 07:16:08AM -0800, Jose Souza wrote:
> > > > Valid, ranks and bandwidth_kbps are set into dram_info but are not
> > > > used anywhere else so nuking it.
> > > >
> > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > > ---
> > > > drivers/gpu/drm/i915/i915_drv.c | 4 +--
> > > > drivers/gpu/drm/i915/i915_drv.h | 3 --
> > > > drivers/gpu/drm/i915/intel_dram.c | 47 +++++++------------------------
> > > > 3 files changed, 12 insertions(+), 42 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > > > index f5666b44ea9d..a1cc60de99f0 100644
> > > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > > @@ -609,8 +609,8 @@ static int i915_driver_hw_probe(struct drm_i915_private *dev_priv)
> > > >
> > > > intel_opregion_setup(dev_priv);
> > > > /*
> > > > - * Fill the dram structure to get the system raw bandwidth and
> > > > - * dram info. This will be used for memory latency calculation.
> > > > + * Fill the dram structure to get the system dram info. This will be
> > > > + * used for memory latency calculation.
> > > > */
> > > > intel_dram_detect(dev_priv);
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > > > index 8376cff5ba86..250e92910fa1 100644
> > > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > > @@ -1134,11 +1134,8 @@ struct drm_i915_private {
> > > > } wm;
> > > >
> > > > struct dram_info {
> > > > - bool valid;
> > > > bool is_16gb_dimm;
> > > > u8 num_channels;
> > > > - u8 ranks;
> > > > - u32 bandwidth_kbps;
> > > > bool symmetric_memory;
> > > > enum intel_dram_type {
> > > > INTEL_DRAM_UNKNOWN,
> > > > diff --git a/drivers/gpu/drm/i915/intel_dram.c b/drivers/gpu/drm/i915/intel_dram.c
> > > > index 4754296a250e..694fbd8c9cd4 100644
> > > > --- a/drivers/gpu/drm/i915/intel_dram.c
> > > > +++ b/drivers/gpu/drm/i915/intel_dram.c
> > > > @@ -201,17 +201,7 @@ skl_dram_get_channels_info(struct drm_i915_private *i915)
> > > > return -EINVAL;
> > > > }
> > > >
> > > > - /*
> > > > - * If any of the channel is single rank channel, worst case output
> > > > - * will be same as if single rank memory, so consider single rank
> > > > - * memory.
> > > > - */
> > > > - if (ch0.ranks == 1 || ch1.ranks == 1)
> > > > - dram_info->ranks = 1;
> > > > - else
> > > > - dram_info->ranks = max(ch0.ranks, ch1.ranks);
> > > > -
> > > > - if (dram_info->ranks == 0) {
> > > > + if (ch0.ranks == 0 && ch1.ranks == 0) {
> > >
> > > previously if any of them were != 0, we would not fall here.
> >
> > This is the same behavior.
>
> indeed, I misread the condition
>
> >
> > >
> > >
> > > > drm_info(&i915->drm, "couldn't get memory rank information\n");
> > > > return -EINVAL;
> > > > }
> > > > @@ -269,16 +259,12 @@ skl_get_dram_info(struct drm_i915_private *i915)
> > > > mem_freq_khz = DIV_ROUND_UP((val & SKL_REQ_DATA_MASK) *
> > > > SKL_MEMORY_FREQ_MULTIPLIER_HZ, 1000);
> > > >
> > > > - dram_info->bandwidth_kbps = dram_info->num_channels *
> > > > - mem_freq_khz * 8;
> > > > -
> > > > - if (dram_info->bandwidth_kbps == 0) {
> > > > + if (dram_info->num_channels * mem_freq_khz == 0) {
> > > > drm_info(&i915->drm,
> > > > "Couldn't get system memory bandwidth\n");
> > > > return -EINVAL;
> > > > }
> > > >
> > > > - dram_info->valid = true;
> > > > return 0;
> > > > }
> > > >
> > > > @@ -365,7 +351,7 @@ static int bxt_get_dram_info(struct drm_i915_private *i915)
> > > > struct dram_info *dram_info = &i915->dram_info;
> > > > u32 dram_channels;
> > > > u32 mem_freq_khz, val;
> > > > - u8 num_active_channels;
> > > > + u8 num_active_channels, valid_ranks = 0;
> > > > int i;
> > > >
> > > > val = intel_uncore_read(&i915->uncore, BXT_P_CR_MC_BIOS_REQ_0_0_0);
> > > > @@ -375,10 +361,7 @@ static int bxt_get_dram_info(struct drm_i915_private *i915)
> > > > dram_channels = val & BXT_DRAM_CHANNEL_ACTIVE_MASK;
> > > > num_active_channels = hweight32(dram_channels);
> > > >
> > > > - /* Each active bit represents 4-byte channel */
> > > > - dram_info->bandwidth_kbps = (mem_freq_khz * num_active_channels * 4);
> > > > -
> > > > - if (dram_info->bandwidth_kbps == 0) {
> > > > + if (mem_freq_khz * num_active_channels == 0) {
> > >
> > > maybe better to replace with a local var?
> > >
> > > bandwidth_kbps = mem_freq_khz * num_active_channels;
> > >
> > > and then check it where needed.
> >
> > The only place it is used is in this if to return -EINVAL, same for the SKL function.
> > The multiplication fits under the 80 col limit so don't see why add a local var.
>
> ok... maybe `if (!mem_freq_khz || !num_active_channels)` then?
That works too but I still prefer keep it as close as possible from the previous check.
>
>
> LUcas De Marchi
>
> >
> > >
> > > Lucas De Marchi
> > >
> > > > drm_info(&i915->drm,
> > > > "Couldn't get system memory bandwidth\n");
> > > > return -EINVAL;
> > > > @@ -410,27 +393,18 @@ static int bxt_get_dram_info(struct drm_i915_private *i915)
> > > > dimm.size, dimm.width, dimm.ranks,
> > > > intel_dram_type_str(type));
> > > >
> > > > - /*
> > > > - * If any of the channel is single rank channel,
> > > > - * worst case output will be same as if single rank
> > > > - * memory, so consider single rank memory.
> > > > - */
> > > > - if (dram_info->ranks == 0)
> > > > - dram_info->ranks = dimm.ranks;
> > > > - else if (dimm.ranks == 1)
> > > > - dram_info->ranks = 1;
> > > > + if (valid_ranks == 0)
> > > > + valid_ranks = dimm.ranks;
> > > >
> > > > if (type != INTEL_DRAM_UNKNOWN)
> > > > dram_info->type = type;
> > > > }
> > > >
> > > > - if (dram_info->type == INTEL_DRAM_UNKNOWN || dram_info->ranks == 0) {
> > > > + if (dram_info->type == INTEL_DRAM_UNKNOWN || valid_ranks == 0) {
> > > > drm_info(&i915->drm, "couldn't get memory information\n");
> > > > return -EINVAL;
> > > > }
> > > >
> > > > - dram_info->valid = true;
> > > > -
> > > > return 0;
> > > > }
> > > >
> > > > @@ -456,11 +430,10 @@ void intel_dram_detect(struct drm_i915_private *i915)
> > > > if (ret)
> > > > return;
> > > >
> > > > - drm_dbg_kms(&i915->drm, "DRAM bandwidth: %u kBps, channels: %u\n",
> > > > - dram_info->bandwidth_kbps, dram_info->num_channels);
> > > > + drm_dbg_kms(&i915->drm, "DRAM channels: %u\n", dram_info->num_channels);
> > > >
> > > > - drm_dbg_kms(&i915->drm, "DRAM ranks: %u, 16Gb DIMMs: %s\n",
> > > > - dram_info->ranks, yesno(dram_info->is_16gb_dimm));
> > > > + drm_dbg_kms(&i915->drm, "DRAM 16Gb DIMMs: %s\n",
> > > > + yesno(dram_info->is_16gb_dimm));
> > > > }
> > > >
> > > > static u32 gen9_edram_size_mb(struct drm_i915_private *i915, u32 cap)
> > > > --
> > > > 2.30.0
> > > >
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/4] drm/i915: Nuke not needed members of dram_info
2021-01-20 15:16 [Intel-gfx] [PATCH 1/4] drm/i915: Nuke not needed members of dram_info José Roberto de Souza
` (3 preceding siblings ...)
[not found] ` <20210120183151.jpfuda4htxs5qkxq@ldmartin-desk1>
@ 2021-01-20 21:32 ` Patchwork
2021-01-21 22:35 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/4] drm/i915: Nuke not needed members of dram_info (rev2) Patchwork
2021-01-22 18:49 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/4] drm/i915: Nuke not needed members of dram_info (rev3) Patchwork
6 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2021-01-20 21:32 UTC (permalink / raw)
To: Souza, Jose; +Cc: intel-gfx
[-- Attachment #1.1: Type: text/plain, Size: 6545 bytes --]
== Series Details ==
Series: series starting with [1/4] drm/i915: Nuke not needed members of dram_info
URL : https://patchwork.freedesktop.org/series/86092/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_9650 -> Patchwork_19429
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_19429 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_19429, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19429/index.html
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_19429:
### IGT changes ###
#### Possible regressions ####
* igt@i915_module_load@reload:
- fi-glk-dsi: [PASS][1] -> [INCOMPLETE][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9650/fi-glk-dsi/igt@i915_module_load@reload.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19429/fi-glk-dsi/igt@i915_module_load@reload.html
Known issues
------------
Here are the changes found in Patchwork_19429 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@amdgpu/amd_prime@i915-to-amd:
- fi-snb-2520m: NOTRUN -> [SKIP][3] ([fdo#109271]) +17 similar issues
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19429/fi-snb-2520m/igt@amdgpu/amd_prime@i915-to-amd.html
* igt@gem_exec_fence@basic-busy:
- fi-glk-dsi: NOTRUN -> [SKIP][4] ([fdo#109271]) +13 similar issues
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19429/fi-glk-dsi/igt@gem_exec_fence@basic-busy.html
* igt@i915_getparams_basic@basic-subslice-total:
- fi-tgl-y: [PASS][5] -> [DMESG-WARN][6] ([i915#402]) +1 similar issue
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9650/fi-tgl-y/igt@i915_getparams_basic@basic-subslice-total.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19429/fi-tgl-y/igt@i915_getparams_basic@basic-subslice-total.html
* igt@kms_addfb_basic@invalid-set-prop-any:
- fi-glk-dsi: [PASS][7] -> [SKIP][8] ([fdo#109271]) +116 similar issues
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9650/fi-glk-dsi/igt@kms_addfb_basic@invalid-set-prop-any.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19429/fi-glk-dsi/igt@kms_addfb_basic@invalid-set-prop-any.html
* igt@runner@aborted:
- fi-glk-dsi: NOTRUN -> [FAIL][9] ([i915#2292] / [i915#2295] / [k.org#202321] / [k.org#204565])
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19429/fi-glk-dsi/igt@runner@aborted.html
#### Possible fixes ####
* igt@gem_render_tiled_blits@basic:
- fi-tgl-y: [DMESG-WARN][10] ([i915#402]) -> [PASS][11]
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9650/fi-tgl-y/igt@gem_render_tiled_blits@basic.html
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19429/fi-tgl-y/igt@gem_render_tiled_blits@basic.html
* igt@i915_selftest@live@hangcheck:
- fi-snb-2520m: [INCOMPLETE][12] -> [PASS][13]
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9650/fi-snb-2520m/igt@i915_selftest@live@hangcheck.html
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19429/fi-snb-2520m/igt@i915_selftest@live@hangcheck.html
#### Warnings ####
* igt@gem_huc_copy@huc-copy:
- fi-glk-dsi: [SKIP][14] ([fdo#109271] / [i915#2190]) -> [SKIP][15] ([fdo#109271])
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9650/fi-glk-dsi/igt@gem_huc_copy@huc-copy.html
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19429/fi-glk-dsi/igt@gem_huc_copy@huc-copy.html
* igt@kms_chamelium@hdmi-hpd-fast:
- fi-glk-dsi: [SKIP][16] ([fdo#109271] / [fdo#111827]) -> [SKIP][17] ([fdo#109271]) +8 similar issues
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9650/fi-glk-dsi/igt@kms_chamelium@hdmi-hpd-fast.html
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19429/fi-glk-dsi/igt@kms_chamelium@hdmi-hpd-fast.html
* igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d:
- fi-glk-dsi: [SKIP][18] ([fdo#109271] / [i915#533]) -> [SKIP][19] ([fdo#109271])
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9650/fi-glk-dsi/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d.html
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19429/fi-glk-dsi/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
[fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
[i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
[i915#2292]: https://gitlab.freedesktop.org/drm/intel/issues/2292
[i915#2295]: https://gitlab.freedesktop.org/drm/intel/issues/2295
[i915#2575]: https://gitlab.freedesktop.org/drm/intel/issues/2575
[i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402
[i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533
[k.org#202321]: https://bugzilla.kernel.org/show_bug.cgi?id=202321
[k.org#204565]: https://bugzilla.kernel.org/show_bug.cgi?id=204565
Participating hosts (43 -> 37)
------------------------------
Missing (6): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-bsw-cyan fi-ctg-p8600 fi-bdw-samus
Build changes
-------------
* Linux: CI_DRM_9650 -> Patchwork_19429
CI-20190529: 20190529
CI_DRM_9650: 3f989d1bb4cfd91e25549f9fd7a750412581dcc4 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5960: ace82fcd5f3623f8dde7c220a825873dc53dfae4 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_19429: 61037f310a0e0d6ed56e90fd6dd5b90001b71881 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
61037f310a0e drm/i915: Rename is_16gb_dimm to wm_lv_0_adjust_needed
467a885ed1a0 drm/i915: Fail driver probe when unable to load DRAM information
67139f6e2aad drm/i915/gen11+: Only load DRAM information from pcode
4ff0251bf217 drm/i915: Nuke not needed members of dram_info
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19429/index.html
[-- Attachment #1.2: Type: text/html, Size: 7902 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/4] drm/i915: Nuke not needed members of dram_info (rev2)
2021-01-20 15:16 [Intel-gfx] [PATCH 1/4] drm/i915: Nuke not needed members of dram_info José Roberto de Souza
` (4 preceding siblings ...)
2021-01-20 21:32 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/4] " Patchwork
@ 2021-01-21 22:35 ` Patchwork
2021-01-22 18:49 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/4] drm/i915: Nuke not needed members of dram_info (rev3) Patchwork
6 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2021-01-21 22:35 UTC (permalink / raw)
To: Souza, Jose; +Cc: intel-gfx
[-- Attachment #1.1: Type: text/plain, Size: 7840 bytes --]
== Series Details ==
Series: series starting with [1/4] drm/i915: Nuke not needed members of dram_info (rev2)
URL : https://patchwork.freedesktop.org/series/86092/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_9664 -> Patchwork_19441
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_19441 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_19441, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19441/index.html
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_19441:
### IGT changes ###
#### Possible regressions ####
* igt@i915_module_load@reload:
- fi-glk-dsi: [PASS][1] -> [INCOMPLETE][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9664/fi-glk-dsi/igt@i915_module_load@reload.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19441/fi-glk-dsi/igt@i915_module_load@reload.html
Known issues
------------
Here are the changes found in Patchwork_19441 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@amdgpu/amd_basic@semaphore:
- fi-bdw-5557u: NOTRUN -> [SKIP][3] ([fdo#109271]) +22 similar issues
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19441/fi-bdw-5557u/igt@amdgpu/amd_basic@semaphore.html
* igt@core_hotunplug@unbind-rebind:
- fi-bdw-5557u: NOTRUN -> [WARN][4] ([i915#2283])
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19441/fi-bdw-5557u/igt@core_hotunplug@unbind-rebind.html
* igt@gem_exec_fence@basic-busy:
- fi-glk-dsi: NOTRUN -> [SKIP][5] ([fdo#109271]) +13 similar issues
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19441/fi-glk-dsi/igt@gem_exec_fence@basic-busy.html
* igt@gem_exec_suspend@basic-s3:
- fi-tgl-y: [PASS][6] -> [DMESG-WARN][7] ([i915#2411] / [i915#402])
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9664/fi-tgl-y/igt@gem_exec_suspend@basic-s3.html
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19441/fi-tgl-y/igt@gem_exec_suspend@basic-s3.html
* igt@i915_pm_rpm@module-reload:
- fi-kbl-guc: [PASS][8] -> [FAIL][9] ([i915#2203] / [i915#579])
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9664/fi-kbl-guc/igt@i915_pm_rpm@module-reload.html
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19441/fi-kbl-guc/igt@i915_pm_rpm@module-reload.html
* igt@kms_addfb_basic@invalid-set-prop-any:
- fi-glk-dsi: [PASS][10] -> [SKIP][11] ([fdo#109271]) +116 similar issues
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9664/fi-glk-dsi/igt@kms_addfb_basic@invalid-set-prop-any.html
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19441/fi-glk-dsi/igt@kms_addfb_basic@invalid-set-prop-any.html
* igt@prime_self_import@basic-with_one_bo_two_files:
- fi-tgl-y: [PASS][12] -> [DMESG-WARN][13] ([i915#402]) +1 similar issue
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9664/fi-tgl-y/igt@prime_self_import@basic-with_one_bo_two_files.html
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19441/fi-tgl-y/igt@prime_self_import@basic-with_one_bo_two_files.html
* igt@runner@aborted:
- fi-glk-dsi: NOTRUN -> [FAIL][14] ([i915#2292] / [i915#2295] / [k.org#202321] / [k.org#204565])
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19441/fi-glk-dsi/igt@runner@aborted.html
#### Possible fixes ####
* igt@fbdev@read:
- fi-tgl-y: [DMESG-WARN][15] ([i915#402]) -> [PASS][16] +2 similar issues
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9664/fi-tgl-y/igt@fbdev@read.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19441/fi-tgl-y/igt@fbdev@read.html
* igt@i915_module_load@reload:
- fi-byt-j1900: [DMESG-WARN][17] ([i915#1982]) -> [PASS][18]
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9664/fi-byt-j1900/igt@i915_module_load@reload.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19441/fi-byt-j1900/igt@i915_module_load@reload.html
* igt@kms_chamelium@hdmi-crc-fast:
- fi-kbl-7500u: [DMESG-WARN][19] ([i915#2868]) -> [PASS][20]
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9664/fi-kbl-7500u/igt@kms_chamelium@hdmi-crc-fast.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19441/fi-kbl-7500u/igt@kms_chamelium@hdmi-crc-fast.html
#### Warnings ####
* igt@gem_huc_copy@huc-copy:
- fi-glk-dsi: [SKIP][21] ([fdo#109271] / [i915#2190]) -> [SKIP][22] ([fdo#109271])
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9664/fi-glk-dsi/igt@gem_huc_copy@huc-copy.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19441/fi-glk-dsi/igt@gem_huc_copy@huc-copy.html
* igt@kms_chamelium@hdmi-hpd-fast:
- fi-glk-dsi: [SKIP][23] ([fdo#109271] / [fdo#111827]) -> [SKIP][24] ([fdo#109271]) +8 similar issues
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9664/fi-glk-dsi/igt@kms_chamelium@hdmi-hpd-fast.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19441/fi-glk-dsi/igt@kms_chamelium@hdmi-hpd-fast.html
* igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d:
- fi-glk-dsi: [SKIP][25] ([fdo#109271] / [i915#533]) -> [SKIP][26] ([fdo#109271])
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9664/fi-glk-dsi/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d.html
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19441/fi-glk-dsi/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d.html
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
[i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
[i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
[i915#2203]: https://gitlab.freedesktop.org/drm/intel/issues/2203
[i915#2283]: https://gitlab.freedesktop.org/drm/intel/issues/2283
[i915#2292]: https://gitlab.freedesktop.org/drm/intel/issues/2292
[i915#2295]: https://gitlab.freedesktop.org/drm/intel/issues/2295
[i915#2411]: https://gitlab.freedesktop.org/drm/intel/issues/2411
[i915#2868]: https://gitlab.freedesktop.org/drm/intel/issues/2868
[i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402
[i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533
[i915#579]: https://gitlab.freedesktop.org/drm/intel/issues/579
[k.org#202321]: https://bugzilla.kernel.org/show_bug.cgi?id=202321
[k.org#204565]: https://bugzilla.kernel.org/show_bug.cgi?id=204565
Participating hosts (41 -> 37)
------------------------------
Missing (4): fi-ilk-m540 fi-bsw-cyan fi-bdw-samus fi-hsw-4200u
Build changes
-------------
* Linux: CI_DRM_9664 -> Patchwork_19441
CI-20190529: 20190529
CI_DRM_9664: dd119a66c9fc3706c93efa10a7631a4c499d94f3 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5962: 22e3daaed82ab7890018a2f2aabf5082cd536023 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_19441: a871e378b7e4c55928f79259f724df0111cc82b8 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
a871e378b7e4 drm/i915: Rename is_16gb_dimm to wm_lv_0_adjust_needed
1d4c44951302 drm/i915: Fail driver probe when unable to load DRAM information
4901cd6b2e8f drm/i915/gen11+: Only load DRAM information from pcode
51817b52f494 drm/i915: Nuke not needed members of dram_info
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19441/index.html
[-- Attachment #1.2: Type: text/html, Size: 9483 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/4] drm/i915: Nuke not needed members of dram_info (rev3)
2021-01-20 15:16 [Intel-gfx] [PATCH 1/4] drm/i915: Nuke not needed members of dram_info José Roberto de Souza
` (5 preceding siblings ...)
2021-01-21 22:35 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/4] drm/i915: Nuke not needed members of dram_info (rev2) Patchwork
@ 2021-01-22 18:49 ` Patchwork
6 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2021-01-22 18:49 UTC (permalink / raw)
To: Souza, Jose; +Cc: intel-gfx
[-- Attachment #1.1: Type: text/plain, Size: 6347 bytes --]
== Series Details ==
Series: series starting with [1/4] drm/i915: Nuke not needed members of dram_info (rev3)
URL : https://patchwork.freedesktop.org/series/86092/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_9670 -> Patchwork_19460
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_19460 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_19460, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19460/index.html
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_19460:
### IGT changes ###
#### Possible regressions ####
* igt@i915_module_load@reload:
- fi-glk-dsi: [PASS][1] -> [INCOMPLETE][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9670/fi-glk-dsi/igt@i915_module_load@reload.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19460/fi-glk-dsi/igt@i915_module_load@reload.html
Known issues
------------
Here are the changes found in Patchwork_19460 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@amdgpu/amd_basic@userptr:
- fi-byt-j1900: NOTRUN -> [SKIP][3] ([fdo#109271]) +17 similar issues
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19460/fi-byt-j1900/igt@amdgpu/amd_basic@userptr.html
* igt@gem_exec_fence@basic-busy:
- fi-glk-dsi: NOTRUN -> [SKIP][4] ([fdo#109271]) +13 similar issues
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19460/fi-glk-dsi/igt@gem_exec_fence@basic-busy.html
* igt@i915_hangman@error-state-basic:
- fi-tgl-y: [PASS][5] -> [DMESG-WARN][6] ([i915#402]) +1 similar issue
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9670/fi-tgl-y/igt@i915_hangman@error-state-basic.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19460/fi-tgl-y/igt@i915_hangman@error-state-basic.html
* igt@kms_addfb_basic@invalid-set-prop-any:
- fi-glk-dsi: [PASS][7] -> [SKIP][8] ([fdo#109271]) +116 similar issues
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9670/fi-glk-dsi/igt@kms_addfb_basic@invalid-set-prop-any.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19460/fi-glk-dsi/igt@kms_addfb_basic@invalid-set-prop-any.html
* igt@runner@aborted:
- fi-glk-dsi: NOTRUN -> [FAIL][9] ([i915#2292] / [i915#2295] / [k.org#202321] / [k.org#204565])
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19460/fi-glk-dsi/igt@runner@aborted.html
#### Possible fixes ####
* igt@i915_pm_rpm@module-reload:
- fi-byt-j1900: [INCOMPLETE][10] ([i915#142] / [i915#2405]) -> [PASS][11]
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9670/fi-byt-j1900/igt@i915_pm_rpm@module-reload.html
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19460/fi-byt-j1900/igt@i915_pm_rpm@module-reload.html
* igt@vgem_basic@setversion:
- fi-tgl-y: [DMESG-WARN][12] ([i915#402]) -> [PASS][13] +2 similar issues
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9670/fi-tgl-y/igt@vgem_basic@setversion.html
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19460/fi-tgl-y/igt@vgem_basic@setversion.html
#### Warnings ####
* igt@gem_huc_copy@huc-copy:
- fi-glk-dsi: [SKIP][14] ([fdo#109271] / [i915#2190]) -> [SKIP][15] ([fdo#109271])
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9670/fi-glk-dsi/igt@gem_huc_copy@huc-copy.html
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19460/fi-glk-dsi/igt@gem_huc_copy@huc-copy.html
* igt@kms_chamelium@hdmi-hpd-fast:
- fi-glk-dsi: [SKIP][16] ([fdo#109271] / [fdo#111827]) -> [SKIP][17] ([fdo#109271]) +8 similar issues
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9670/fi-glk-dsi/igt@kms_chamelium@hdmi-hpd-fast.html
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19460/fi-glk-dsi/igt@kms_chamelium@hdmi-hpd-fast.html
* igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d:
- fi-glk-dsi: [SKIP][18] ([fdo#109271] / [i915#533]) -> [SKIP][19] ([fdo#109271])
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9670/fi-glk-dsi/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d.html
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19460/fi-glk-dsi/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d.html
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
[i915#142]: https://gitlab.freedesktop.org/drm/intel/issues/142
[i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
[i915#2292]: https://gitlab.freedesktop.org/drm/intel/issues/2292
[i915#2295]: https://gitlab.freedesktop.org/drm/intel/issues/2295
[i915#2405]: https://gitlab.freedesktop.org/drm/intel/issues/2405
[i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402
[i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533
[k.org#202321]: https://bugzilla.kernel.org/show_bug.cgi?id=202321
[k.org#204565]: https://bugzilla.kernel.org/show_bug.cgi?id=204565
Participating hosts (41 -> 37)
------------------------------
Missing (4): fi-ilk-m540 fi-bsw-cyan fi-bdw-samus fi-hsw-4200u
Build changes
-------------
* Linux: CI_DRM_9670 -> Patchwork_19460
CI-20190529: 20190529
CI_DRM_9670: 85fd189b9fbfb6e7af8d956d37be012fdd6ae0ad @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5964: 0949766cb9846d7d55fac9cdf31d3d8e8ed1d0c6 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_19460: c1f73ccb6d94db8505fdc03789e66427e71dcd6e @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
c1f73ccb6d94 drm/i915: Rename is_16gb_dimm to wm_lv_0_adjust_needed
7a0944ee9f63 drm/i915: Fail driver probe when unable to load DRAM information
f532480817a9 drm/i915/gen11+: Only load DRAM information from pcode
d61e40e438c6 drm/i915: Nuke not needed members of dram_info
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19460/index.html
[-- Attachment #1.2: Type: text/html, Size: 7825 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Intel-gfx] [PATCH 1/4] drm/i915: Nuke not needed members of dram_info
2021-01-20 19:29 ` [Intel-gfx] [PATCH 1/4] drm/i915: Nuke not needed members of dram_info Souza, Jose
@ 2021-01-27 14:35 ` Lucas De Marchi
0 siblings, 0 replies; 12+ messages in thread
From: Lucas De Marchi @ 2021-01-27 14:35 UTC (permalink / raw)
To: Souza, Jose; +Cc: intel-gfx
On Wed, Jan 20, 2021 at 07:29:37PM +0000, Jose Souza wrote:
>On Wed, 2021-01-20 at 10:52 -0800, Lucas De Marchi wrote:
>> On Wed, Jan 20, 2021 at 10:42:46AM -0800, Jose Souza wrote:
>> > On Wed, 2021-01-20 at 10:31 -0800, Lucas De Marchi wrote:
>> > > On Wed, Jan 20, 2021 at 07:16:08AM -0800, Jose Souza wrote:
>> > > > Valid, ranks and bandwidth_kbps are set into dram_info but are not
>> > > > used anywhere else so nuking it.
>> > > >
>> > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
>> > > > ---
>> > > > drivers/gpu/drm/i915/i915_drv.c | 4 +--
>> > > > drivers/gpu/drm/i915/i915_drv.h | 3 --
>> > > > drivers/gpu/drm/i915/intel_dram.c | 47 +++++++------------------------
>> > > > 3 files changed, 12 insertions(+), 42 deletions(-)
>> > > >
>> > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>> > > > index f5666b44ea9d..a1cc60de99f0 100644
>> > > > --- a/drivers/gpu/drm/i915/i915_drv.c
>> > > > +++ b/drivers/gpu/drm/i915/i915_drv.c
>> > > > @@ -609,8 +609,8 @@ static int i915_driver_hw_probe(struct drm_i915_private *dev_priv)
>> > > >
>> > > > intel_opregion_setup(dev_priv);
>> > > > /*
>> > > > - * Fill the dram structure to get the system raw bandwidth and
>> > > > - * dram info. This will be used for memory latency calculation.
>> > > > + * Fill the dram structure to get the system dram info. This will be
>> > > > + * used for memory latency calculation.
>> > > > */
>> > > > intel_dram_detect(dev_priv);
>> > > >
>> > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> > > > index 8376cff5ba86..250e92910fa1 100644
>> > > > --- a/drivers/gpu/drm/i915/i915_drv.h
>> > > > +++ b/drivers/gpu/drm/i915/i915_drv.h
>> > > > @@ -1134,11 +1134,8 @@ struct drm_i915_private {
>> > > > } wm;
>> > > >
>> > > > struct dram_info {
>> > > > - bool valid;
>> > > > bool is_16gb_dimm;
>> > > > u8 num_channels;
>> > > > - u8 ranks;
>> > > > - u32 bandwidth_kbps;
>> > > > bool symmetric_memory;
>> > > > enum intel_dram_type {
>> > > > INTEL_DRAM_UNKNOWN,
>> > > > diff --git a/drivers/gpu/drm/i915/intel_dram.c b/drivers/gpu/drm/i915/intel_dram.c
>> > > > index 4754296a250e..694fbd8c9cd4 100644
>> > > > --- a/drivers/gpu/drm/i915/intel_dram.c
>> > > > +++ b/drivers/gpu/drm/i915/intel_dram.c
>> > > > @@ -201,17 +201,7 @@ skl_dram_get_channels_info(struct drm_i915_private *i915)
>> > > > return -EINVAL;
>> > > > }
>> > > >
>> > > > - /*
>> > > > - * If any of the channel is single rank channel, worst case output
>> > > > - * will be same as if single rank memory, so consider single rank
>> > > > - * memory.
>> > > > - */
>> > > > - if (ch0.ranks == 1 || ch1.ranks == 1)
>> > > > - dram_info->ranks = 1;
>> > > > - else
>> > > > - dram_info->ranks = max(ch0.ranks, ch1.ranks);
>> > > > -
>> > > > - if (dram_info->ranks == 0) {
>> > > > + if (ch0.ranks == 0 && ch1.ranks == 0) {
>> > >
>> > > previously if any of them were != 0, we would not fall here.
>> >
>> > This is the same behavior.
>>
>> indeed, I misread the condition
>>
>> >
>> > >
>> > >
>> > > > drm_info(&i915->drm, "couldn't get memory rank information\n");
>> > > > return -EINVAL;
>> > > > }
>> > > > @@ -269,16 +259,12 @@ skl_get_dram_info(struct drm_i915_private *i915)
>> > > > mem_freq_khz = DIV_ROUND_UP((val & SKL_REQ_DATA_MASK) *
>> > > > SKL_MEMORY_FREQ_MULTIPLIER_HZ, 1000);
>> > > >
>> > > > - dram_info->bandwidth_kbps = dram_info->num_channels *
>> > > > - mem_freq_khz * 8;
>> > > > -
>> > > > - if (dram_info->bandwidth_kbps == 0) {
>> > > > + if (dram_info->num_channels * mem_freq_khz == 0) {
>> > > > drm_info(&i915->drm,
>> > > > "Couldn't get system memory bandwidth\n");
>> > > > return -EINVAL;
>> > > > }
>> > > >
>> > > > - dram_info->valid = true;
>> > > > return 0;
>> > > > }
>> > > >
>> > > > @@ -365,7 +351,7 @@ static int bxt_get_dram_info(struct drm_i915_private *i915)
>> > > > struct dram_info *dram_info = &i915->dram_info;
>> > > > u32 dram_channels;
>> > > > u32 mem_freq_khz, val;
>> > > > - u8 num_active_channels;
>> > > > + u8 num_active_channels, valid_ranks = 0;
>> > > > int i;
>> > > >
>> > > > val = intel_uncore_read(&i915->uncore, BXT_P_CR_MC_BIOS_REQ_0_0_0);
>> > > > @@ -375,10 +361,7 @@ static int bxt_get_dram_info(struct drm_i915_private *i915)
>> > > > dram_channels = val & BXT_DRAM_CHANNEL_ACTIVE_MASK;
>> > > > num_active_channels = hweight32(dram_channels);
>> > > >
>> > > > - /* Each active bit represents 4-byte channel */
>> > > > - dram_info->bandwidth_kbps = (mem_freq_khz * num_active_channels * 4);
>> > > > -
>> > > > - if (dram_info->bandwidth_kbps == 0) {
>> > > > + if (mem_freq_khz * num_active_channels == 0) {
>> > >
>> > > maybe better to replace with a local var?
>> > >
>> > > bandwidth_kbps = mem_freq_khz * num_active_channels;
>> > >
>> > > and then check it where needed.
>> >
>> > The only place it is used is in this if to return -EINVAL, same for the SKL function.
>> > The multiplication fits under the 80 col limit so don't see why add a local var.
>>
>> ok... maybe `if (!mem_freq_khz || !num_active_channels)` then?
>
>That works too but I still prefer keep it as close as possible from the previous check.
Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
Lucas De Marchi
>
>>
>>
>> LUcas De Marchi
>>
>> >
>> > >
>> > > Lucas De Marchi
>> > >
>> > > > drm_info(&i915->drm,
>> > > > "Couldn't get system memory bandwidth\n");
>> > > > return -EINVAL;
>> > > > @@ -410,27 +393,18 @@ static int bxt_get_dram_info(struct drm_i915_private *i915)
>> > > > dimm.size, dimm.width, dimm.ranks,
>> > > > intel_dram_type_str(type));
>> > > >
>> > > > - /*
>> > > > - * If any of the channel is single rank channel,
>> > > > - * worst case output will be same as if single rank
>> > > > - * memory, so consider single rank memory.
>> > > > - */
>> > > > - if (dram_info->ranks == 0)
>> > > > - dram_info->ranks = dimm.ranks;
>> > > > - else if (dimm.ranks == 1)
>> > > > - dram_info->ranks = 1;
>> > > > + if (valid_ranks == 0)
>> > > > + valid_ranks = dimm.ranks;
>> > > >
>> > > > if (type != INTEL_DRAM_UNKNOWN)
>> > > > dram_info->type = type;
>> > > > }
>> > > >
>> > > > - if (dram_info->type == INTEL_DRAM_UNKNOWN || dram_info->ranks == 0) {
>> > > > + if (dram_info->type == INTEL_DRAM_UNKNOWN || valid_ranks == 0) {
>> > > > drm_info(&i915->drm, "couldn't get memory information\n");
>> > > > return -EINVAL;
>> > > > }
>> > > >
>> > > > - dram_info->valid = true;
>> > > > -
>> > > > return 0;
>> > > > }
>> > > >
>> > > > @@ -456,11 +430,10 @@ void intel_dram_detect(struct drm_i915_private *i915)
>> > > > if (ret)
>> > > > return;
>> > > >
>> > > > - drm_dbg_kms(&i915->drm, "DRAM bandwidth: %u kBps, channels: %u\n",
>> > > > - dram_info->bandwidth_kbps, dram_info->num_channels);
>> > > > + drm_dbg_kms(&i915->drm, "DRAM channels: %u\n", dram_info->num_channels);
>> > > >
>> > > > - drm_dbg_kms(&i915->drm, "DRAM ranks: %u, 16Gb DIMMs: %s\n",
>> > > > - dram_info->ranks, yesno(dram_info->is_16gb_dimm));
>> > > > + drm_dbg_kms(&i915->drm, "DRAM 16Gb DIMMs: %s\n",
>> > > > + yesno(dram_info->is_16gb_dimm));
>> > > > }
>> > > >
>> > > > static u32 gen9_edram_size_mb(struct drm_i915_private *i915, u32 cap)
>> > > > --
>> > > > 2.30.0
>> > > >
>> > > > _______________________________________________
>> > > > Intel-gfx mailing list
>> > > > Intel-gfx@lists.freedesktop.org
>> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> >
>
>_______________________________________________
>Intel-gfx mailing list
>Intel-gfx@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Intel-gfx] [PATCH 2/4] drm/i915/gen11+: Only load DRAM information from pcode
2021-01-20 15:16 ` [Intel-gfx] [PATCH 2/4] drm/i915/gen11+: Only load DRAM information from pcode José Roberto de Souza
@ 2021-01-27 14:49 ` Lucas De Marchi
2021-01-27 16:37 ` Souza, Jose
0 siblings, 1 reply; 12+ messages in thread
From: Lucas De Marchi @ 2021-01-27 14:49 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx
On Wed, Jan 20, 2021 at 07:16:09AM -0800, Jose Souza wrote:
>Up to now we were reading some DRAM information from MCHBAR register
>and from pcode what is already not good but some GEN12(TGL-H and ADL-S)
>platforms have MCHBAR DRAM information in different offsets.
>
>This was notified to HW team that decided that the best alternative is
>always apply the 16gb_dimm watermark adjustment for GEN12+ platforms
>and read the remaning DRAM information needed to other display
>programming from pcode.
>
>So here moving the DRAM pcode function to intel_dram.c, removing
>the duplicated fields from intel_qgv_info, setting and using
>information from dram_info.
>
>Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
>---
> drivers/gpu/drm/i915/display/intel_bw.c | 98 +++++--------------------
> drivers/gpu/drm/i915/i915_drv.c | 5 +-
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/intel_dram.c | 77 ++++++++++++++++++-
> 4 files changed, 97 insertions(+), 84 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
>index bd060404d249..1368bd96ed73 100644
>--- a/drivers/gpu/drm/i915/display/intel_bw.c
>+++ b/drivers/gpu/drm/i915/display/intel_bw.c
>@@ -19,77 +19,9 @@ struct intel_qgv_point {
>
> struct intel_qgv_info {
> struct intel_qgv_point points[I915_NUM_QGV_POINTS];
>- u8 num_points;
>- u8 num_channels;
> u8 t_bl;
>- enum intel_dram_type dram_type;
humn... given this struct already has padding, we could very well leave
the num_points field. See below.
> static int icl_pcode_read_qgv_point_info(struct drm_i915_private *dev_priv,
> struct intel_qgv_point *sp,
> int point)
>@@ -139,17 +71,19 @@ int icl_pcode_restrict_qgv_points(struct drm_i915_private *dev_priv,
> static int icl_get_qgv_points(struct drm_i915_private *dev_priv,
> struct intel_qgv_info *qi)
> {
>+ struct dram_info *dram_info = &dev_priv->dram_info;
> int i, ret;
>
>- ret = icl_pcode_read_mem_global_info(dev_priv, qi);
>- if (ret)
>- return ret;
>+ if (IS_GEN(dev_priv, 12))
>+ qi->t_bl = dev_priv->dram_info.type == INTEL_DRAM_DDR4 ? 4 : 16;
>+ else if (IS_GEN(dev_priv, 11))
>+ qi->t_bl = dev_priv->dram_info.type == INTEL_DRAM_DDR4 ? 4 : 8;
>
> if (drm_WARN_ON(&dev_priv->drm,
>- qi->num_points > ARRAY_SIZE(qi->points)))
>- qi->num_points = ARRAY_SIZE(qi->points);
>+ dram_info->qgv_points > ARRAY_SIZE(qi->points)))
>+ dram_info->qgv_points = ARRAY_SIZE(qi->points);
previously we were overriding a member of qi. Now we are overriding a
member of dram_info, which seems to cross the boundaries of what this
code should be doing.
So maybe:
qi->num_points = dev_priv->dram_info->qgv_points;
and leave the check alone and also the renames in this file?
Also, dram_info->qgv_points should be named dram_info->num_qgv_points
for consistency with other structs.
>
>- for (i = 0; i < qi->num_points; i++) {
>+ for (i = 0; i < dram_info->qgv_points; i++) {
> struct intel_qgv_point *sp = &qi->points[i];
>
> ret = icl_pcode_read_qgv_point_info(dev_priv, sp, i);
>@@ -171,12 +105,13 @@ static int icl_calc_bw(int dclk, int num, int den)
> return DIV_ROUND_CLOSEST(num * dclk * 100, den * 6);
> }
>
>-static int icl_sagv_max_dclk(const struct intel_qgv_info *qi)
>+static int icl_sagv_max_dclk(struct drm_i915_private *dev_priv,
>+ const struct intel_qgv_info *qi)
> {
> u16 dclk = 0;
> int i;
>
>- for (i = 0; i < qi->num_points; i++)
>+ for (i = 0; i < dev_priv->dram_info.qgv_points; i++)
> dclk = max(dclk, qi->points[i].dclk);
>
> return dclk;
>@@ -207,6 +142,7 @@ static const struct intel_sa_info rkl_sa_info = {
>
> static int icl_get_bw_info(struct drm_i915_private *dev_priv, const struct intel_sa_info *sa)
> {
>+ struct dram_info *dram_info = &dev_priv->dram_info;
> struct intel_qgv_info qi = {};
> bool is_y_tile = true; /* assume y tile may be used */
> int num_channels;
>@@ -222,10 +158,10 @@ static int icl_get_bw_info(struct drm_i915_private *dev_priv, const struct intel
> "Failed to get memory subsystem information, ignoring bandwidth limits");
> return ret;
> }
>- num_channels = qi.num_channels;
>+ num_channels = dram_info->num_channels;
>
> deinterleave = DIV_ROUND_UP(num_channels, is_y_tile ? 4 : 2);
>- dclk_max = icl_sagv_max_dclk(&qi);
>+ dclk_max = icl_sagv_max_dclk(dev_priv, &qi);
>
> ipqdepthpch = 16;
>
>@@ -241,9 +177,9 @@ static int icl_get_bw_info(struct drm_i915_private *dev_priv, const struct intel
> clpchgroup = (sa->deburst * deinterleave / num_channels) << i;
> bi->num_planes = (ipqdepth - clpchgroup) / clpchgroup + 1;
>
>- bi->num_qgv_points = qi.num_points;
>+ bi->num_qgv_points = dram_info->qgv_points;
see above, we already have num_ prefix here, let's keep it in dram_info
as well.
>
>- for (j = 0; j < qi.num_points; j++) {
>+ for (j = 0; j < dram_info->qgv_points; j++) {
> const struct intel_qgv_point *sp = &qi.points[j];
> int ct, bw;
>
>@@ -274,7 +210,7 @@ static int icl_get_bw_info(struct drm_i915_private *dev_priv, const struct intel
> * SAGV point, but we can't send PCode commands to restrict it
> * as it will fail and pointless anyway.
> */
>- if (qi.num_points == 1)
>+ if (dram_info->qgv_points == 1)
> dev_priv->sagv_status = I915_SAGV_NOT_CONTROLLED;
> else
> dev_priv->sagv_status = I915_SAGV_ENABLED;
>diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>index a1cc60de99f0..66f763fe7a83 100644
>--- a/drivers/gpu/drm/i915/i915_drv.c
>+++ b/drivers/gpu/drm/i915/i915_drv.c
>@@ -608,14 +608,15 @@ static int i915_driver_hw_probe(struct drm_i915_private *dev_priv)
> goto err_msi;
>
> intel_opregion_setup(dev_priv);
>+
>+ intel_pcode_init(dev_priv);
>+
> /*
> * Fill the dram structure to get the system dram info. This will be
> * used for memory latency calculation.
> */
> intel_dram_detect(dev_priv);
>
>- intel_pcode_init(dev_priv);
>-
> intel_bw_init_hw(dev_priv);
>
> return 0;
>diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>index 250e92910fa1..a2ae21082b34 100644
>--- a/drivers/gpu/drm/i915/i915_drv.h
>+++ b/drivers/gpu/drm/i915/i915_drv.h
>@@ -1144,6 +1144,7 @@ struct drm_i915_private {
> INTEL_DRAM_LPDDR3,
> INTEL_DRAM_LPDDR4
> } type;
>+ u8 qgv_points;
> } dram_info;
>
> struct intel_bw_info {
>diff --git a/drivers/gpu/drm/i915/intel_dram.c b/drivers/gpu/drm/i915/intel_dram.c
>index 694fbd8c9cd4..1298823c957c 100644
>--- a/drivers/gpu/drm/i915/intel_dram.c
>+++ b/drivers/gpu/drm/i915/intel_dram.c
>@@ -5,6 +5,7 @@
>
> #include "i915_drv.h"
> #include "intel_dram.h"
>+#include "intel_sideband.h"
>
> struct dram_dimm_info {
> u16 size;
>@@ -408,6 +409,78 @@ static int bxt_get_dram_info(struct drm_i915_private *i915)
> return 0;
> }
>
>+static int icl_pcode_read_mem_global_info(struct drm_i915_private *dev_priv)
>+{
>+ struct dram_info *dram_info = &dev_priv->dram_info;
>+ u32 val = 0;
>+ int ret;
>+
>+ ret = sandybridge_pcode_read(dev_priv,
>+ ICL_PCODE_MEM_SUBSYSYSTEM_INFO |
>+ ICL_PCODE_MEM_SS_READ_GLOBAL_INFO,
>+ &val, NULL);
>+ if (ret)
>+ return ret;
>+
>+ if (IS_GEN(dev_priv, 12)) {
>+ switch (val & 0xf) {
>+ case 0:
>+ dram_info->type = INTEL_DRAM_DDR4;
>+ break;
>+ case 3:
>+ dram_info->type = INTEL_DRAM_LPDDR4;
>+ break;
>+ case 4:
>+ dram_info->type = INTEL_DRAM_DDR3;
>+ break;
>+ case 5:
>+ dram_info->type = INTEL_DRAM_LPDDR3;
>+ break;
>+ default:
>+ MISSING_CASE(val & 0xf);
>+ return -1;
>+ }
>+ } else {
>+ switch (val & 0xf) {
>+ case 0:
>+ dram_info->type = INTEL_DRAM_DDR4;
>+ break;
>+ case 1:
>+ dram_info->type = INTEL_DRAM_DDR3;
>+ break;
>+ case 2:
>+ dram_info->type = INTEL_DRAM_LPDDR3;
>+ break;
>+ case 3:
>+ dram_info->type = INTEL_DRAM_LPDDR4;
>+ break;
>+ default:
>+ MISSING_CASE(val & 0xf);
>+ return -1;
>+ }
>+ }
>+
>+ dram_info->num_channels = (val & 0xf0) >> 4;
>+ dram_info->qgv_points = (val & 0xf00) >> 8;
>+
>+ return 0;
>+}
>+
>+static int gen11_get_dram_info(struct drm_i915_private *i915)
>+{
>+ if (INTEL_GEN(i915) == 11) {
>+ int ret = skl_get_dram_info(i915);
>+
>+ if (ret)
>+ return ret;
>+ } else {
>+ /* Always needed for GEN12+ */
>+ i915->dram_info.is_16gb_dimm = true;
>+ }
>+
>+ return icl_pcode_read_mem_global_info(i915);
>+}
>+
> void intel_dram_detect(struct drm_i915_private *i915)
> {
> struct dram_info *dram_info = &i915->dram_info;
>@@ -423,7 +496,9 @@ void intel_dram_detect(struct drm_i915_private *i915)
> if (INTEL_GEN(i915) < 9 || !HAS_DISPLAY(i915))
> return;
>
>- if (IS_GEN9_LP(i915))
>+ if (INTEL_GEN(i915) >= 11)
>+ ret = gen11_get_dram_info(i915);
gen11 and gen12 implementation above are sufficiently different: better
to keep the if/else chain here only
static int gen11_get_dram_info(struct drm_i915_private *i915)
{
int ret = skl_get_dram_info(i915);
if (ret)
return ret;
return icl_pcode_read_mem_global_info(i915);
}
static int gen12_get_dram_info(struct drm_i915_private *i915)
{
/* Always needed for GEN12+ */
i915->dram_info.is_16gb_dimm = true;
return icl_pcode_read_mem_global_info(i915);
}
Also, now it seems we have lots of dead code since we are not calling
skl_get_dram_info() for tgl. See
tgl_get_dram_type(), skl_dram_get_channels_info, etc
Lucas De Marchi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Intel-gfx] [PATCH 4/4] drm/i915: Rename is_16gb_dimm to wm_lv_0_adjust_needed
2021-01-20 15:16 ` [Intel-gfx] [PATCH 4/4] drm/i915: Rename is_16gb_dimm to wm_lv_0_adjust_needed José Roberto de Souza
@ 2021-01-27 14:56 ` Lucas De Marchi
0 siblings, 0 replies; 12+ messages in thread
From: Lucas De Marchi @ 2021-01-27 14:56 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx
On Wed, Jan 20, 2021 at 07:16:11AM -0800, Jose Souza wrote:
>As it now it is always required for GEN12+ the is_16gb_dimm name
>do not make sense for GEN12+.
>
>Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
>---
> drivers/gpu/drm/i915/i915_drv.h | 2 +-
> drivers/gpu/drm/i915/intel_dram.c | 10 +++++-----
> drivers/gpu/drm/i915/intel_pm.c | 2 +-
> 3 files changed, 7 insertions(+), 7 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>index a2ae21082b34..adc008c65b14 100644
>--- a/drivers/gpu/drm/i915/i915_drv.h
>+++ b/drivers/gpu/drm/i915/i915_drv.h
>@@ -1134,7 +1134,7 @@ struct drm_i915_private {
> } wm;
>
> struct dram_info {
>- bool is_16gb_dimm;
>+ bool wm_lv_0_adjust_needed;
> u8 num_channels;
> bool symmetric_memory;
> enum intel_dram_type {
>diff --git a/drivers/gpu/drm/i915/intel_dram.c b/drivers/gpu/drm/i915/intel_dram.c
>index 4871d48589f9..a5850f0f25aa 100644
>--- a/drivers/gpu/drm/i915/intel_dram.c
>+++ b/drivers/gpu/drm/i915/intel_dram.c
>@@ -207,7 +207,7 @@ skl_dram_get_channels_info(struct drm_i915_private *i915)
> return -EINVAL;
> }
>
>- dram_info->is_16gb_dimm = ch0.is_16gb_dimm || ch1.is_16gb_dimm;
>+ dram_info->wm_lv_0_adjust_needed = ch0.is_16gb_dimm || ch1.is_16gb_dimm;
>
> dram_info->symmetric_memory = intel_is_dram_symmetric(&ch0, &ch1);
>
>@@ -475,7 +475,7 @@ static int gen11_get_dram_info(struct drm_i915_private *i915)
> return ret;
> } else {
> /* Always needed for GEN12+ */
>- i915->dram_info.is_16gb_dimm = true;
>+ i915->dram_info.wm_lv_0_adjust_needed = true;
> }
>
> return icl_pcode_read_mem_global_info(i915);
>@@ -491,7 +491,7 @@ int intel_dram_detect(struct drm_i915_private *i915)
> * This is only used for the level 0 watermark latency
> * w/a which does not apply to bxt/glk.
> */
>- dram_info->is_16gb_dimm = !IS_GEN9_LP(i915);
>+ dram_info->wm_lv_0_adjust_needed = !IS_GEN9_LP(i915);
comment above also needs to be updated. With that:
Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
Lucas De Marchi
>
> if (INTEL_GEN(i915) < 9 || !HAS_DISPLAY(i915))
> return 0;
>@@ -510,8 +510,8 @@ int intel_dram_detect(struct drm_i915_private *i915)
>
> drm_dbg_kms(&i915->drm, "DRAM channels: %u\n", dram_info->num_channels);
>
>- drm_dbg_kms(&i915->drm, "DRAM 16Gb DIMMs: %s\n",
>- yesno(dram_info->is_16gb_dimm));
>+ drm_dbg_kms(&i915->drm, "Watermark level 0 adjustment needed: %s\n",
>+ yesno(dram_info->wm_lv_0_adjust_needed));
>
> return 0;
> }
>diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>index 992fce8b8d13..f778aae19f82 100644
>--- a/drivers/gpu/drm/i915/intel_pm.c
>+++ b/drivers/gpu/drm/i915/intel_pm.c
>@@ -2930,7 +2930,7 @@ static void intel_read_wm_latency(struct drm_i915_private *dev_priv,
> * any underrun. If not able to get Dimm info assume 16GB dimm
> * to avoid any underrun.
> */
>- if (dev_priv->dram_info.is_16gb_dimm)
>+ if (dev_priv->dram_info.wm_lv_0_adjust_needed)
> wm[0] += 1;
>
> } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
>--
>2.30.0
>
>_______________________________________________
>Intel-gfx mailing list
>Intel-gfx@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Intel-gfx] [PATCH 2/4] drm/i915/gen11+: Only load DRAM information from pcode
2021-01-27 14:49 ` Lucas De Marchi
@ 2021-01-27 16:37 ` Souza, Jose
0 siblings, 0 replies; 12+ messages in thread
From: Souza, Jose @ 2021-01-27 16:37 UTC (permalink / raw)
To: De Marchi, Lucas; +Cc: intel-gfx
On Wed, 2021-01-27 at 06:49 -0800, Lucas De Marchi wrote:
> On Wed, Jan 20, 2021 at 07:16:09AM -0800, Jose Souza wrote:
> > Up to now we were reading some DRAM information from MCHBAR register
> > and from pcode what is already not good but some GEN12(TGL-H and ADL-S)
> > platforms have MCHBAR DRAM information in different offsets.
> >
> > This was notified to HW team that decided that the best alternative is
> > always apply the 16gb_dimm watermark adjustment for GEN12+ platforms
> > and read the remaning DRAM information needed to other display
> > programming from pcode.
> >
> > So here moving the DRAM pcode function to intel_dram.c, removing
> > the duplicated fields from intel_qgv_info, setting and using
> > information from dram_info.
> >
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> > drivers/gpu/drm/i915/display/intel_bw.c | 98 +++++--------------------
> > drivers/gpu/drm/i915/i915_drv.c | 5 +-
> > drivers/gpu/drm/i915/i915_drv.h | 1 +
> > drivers/gpu/drm/i915/intel_dram.c | 77 ++++++++++++++++++-
> > 4 files changed, 97 insertions(+), 84 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
> > index bd060404d249..1368bd96ed73 100644
> > --- a/drivers/gpu/drm/i915/display/intel_bw.c
> > +++ b/drivers/gpu/drm/i915/display/intel_bw.c
> > @@ -19,77 +19,9 @@ struct intel_qgv_point {
> >
> > struct intel_qgv_info {
> > struct intel_qgv_point points[I915_NUM_QGV_POINTS];
> > - u8 num_points;
> > - u8 num_channels;
> > u8 t_bl;
> > - enum intel_dram_type dram_type;
>
> humn... given this struct already has padding, we could very well leave
> the num_points field. See below.
>
> > static int icl_pcode_read_qgv_point_info(struct drm_i915_private *dev_priv,
> > struct intel_qgv_point *sp,
> > int point)
> > @@ -139,17 +71,19 @@ int icl_pcode_restrict_qgv_points(struct drm_i915_private *dev_priv,
> > static int icl_get_qgv_points(struct drm_i915_private *dev_priv,
> > struct intel_qgv_info *qi)
> > {
> > + struct dram_info *dram_info = &dev_priv->dram_info;
> > int i, ret;
> >
> > - ret = icl_pcode_read_mem_global_info(dev_priv, qi);
> > - if (ret)
> > - return ret;
> > + if (IS_GEN(dev_priv, 12))
> > + qi->t_bl = dev_priv->dram_info.type == INTEL_DRAM_DDR4 ? 4 : 16;
> > + else if (IS_GEN(dev_priv, 11))
> > + qi->t_bl = dev_priv->dram_info.type == INTEL_DRAM_DDR4 ? 4 : 8;
> >
> > if (drm_WARN_ON(&dev_priv->drm,
> > - qi->num_points > ARRAY_SIZE(qi->points)))
> > - qi->num_points = ARRAY_SIZE(qi->points);
> > + dram_info->qgv_points > ARRAY_SIZE(qi->points)))
> > + dram_info->qgv_points = ARRAY_SIZE(qi->points);
>
> previously we were overriding a member of qi. Now we are overriding a
> member of dram_info, which seems to cross the boundaries of what this
> code should be doing.
>
> So maybe:
>
> qi->num_points = dev_priv->dram_info->qgv_points;
Okay, reasonable enough to keep this duplicated.
>
> and leave the check alone and also the renames in this file?
> Also, dram_info->qgv_points should be named dram_info->num_qgv_points
> for consistency with other structs.
>
> >
> > - for (i = 0; i < qi->num_points; i++) {
> > + for (i = 0; i < dram_info->qgv_points; i++) {
> > struct intel_qgv_point *sp = &qi->points[i];
> >
> > ret = icl_pcode_read_qgv_point_info(dev_priv, sp, i);
> > @@ -171,12 +105,13 @@ static int icl_calc_bw(int dclk, int num, int den)
> > return DIV_ROUND_CLOSEST(num * dclk * 100, den * 6);
> > }
> >
> > -static int icl_sagv_max_dclk(const struct intel_qgv_info *qi)
> > +static int icl_sagv_max_dclk(struct drm_i915_private *dev_priv,
> > + const struct intel_qgv_info *qi)
> > {
> > u16 dclk = 0;
> > int i;
> >
> > - for (i = 0; i < qi->num_points; i++)
> > + for (i = 0; i < dev_priv->dram_info.qgv_points; i++)
> > dclk = max(dclk, qi->points[i].dclk);
> >
> > return dclk;
> > @@ -207,6 +142,7 @@ static const struct intel_sa_info rkl_sa_info = {
> >
> > static int icl_get_bw_info(struct drm_i915_private *dev_priv, const struct intel_sa_info *sa)
> > {
> > + struct dram_info *dram_info = &dev_priv->dram_info;
> > struct intel_qgv_info qi = {};
> > bool is_y_tile = true; /* assume y tile may be used */
> > int num_channels;
> > @@ -222,10 +158,10 @@ static int icl_get_bw_info(struct drm_i915_private *dev_priv, const struct intel
> > "Failed to get memory subsystem information, ignoring bandwidth limits");
> > return ret;
> > }
> > - num_channels = qi.num_channels;
> > + num_channels = dram_info->num_channels;
> >
> > deinterleave = DIV_ROUND_UP(num_channels, is_y_tile ? 4 : 2);
> > - dclk_max = icl_sagv_max_dclk(&qi);
> > + dclk_max = icl_sagv_max_dclk(dev_priv, &qi);
> >
> > ipqdepthpch = 16;
> >
> > @@ -241,9 +177,9 @@ static int icl_get_bw_info(struct drm_i915_private *dev_priv, const struct intel
> > clpchgroup = (sa->deburst * deinterleave / num_channels) << i;
> > bi->num_planes = (ipqdepth - clpchgroup) / clpchgroup + 1;
> >
> > - bi->num_qgv_points = qi.num_points;
> > + bi->num_qgv_points = dram_info->qgv_points;
>
>
> see above, we already have num_ prefix here, let's keep it in dram_info
> as well.
Done, but will keep num_point in intel_qgv_info. We can rename it latter.
>
> >
> > - for (j = 0; j < qi.num_points; j++) {
> > + for (j = 0; j < dram_info->qgv_points; j++) {
> > const struct intel_qgv_point *sp = &qi.points[j];
> > int ct, bw;
> >
> > @@ -274,7 +210,7 @@ static int icl_get_bw_info(struct drm_i915_private *dev_priv, const struct intel
> > * SAGV point, but we can't send PCode commands to restrict it
> > * as it will fail and pointless anyway.
> > */
> > - if (qi.num_points == 1)
> > + if (dram_info->qgv_points == 1)
> > dev_priv->sagv_status = I915_SAGV_NOT_CONTROLLED;
> > else
> > dev_priv->sagv_status = I915_SAGV_ENABLED;
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index a1cc60de99f0..66f763fe7a83 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -608,14 +608,15 @@ static int i915_driver_hw_probe(struct drm_i915_private *dev_priv)
> > goto err_msi;
> >
> > intel_opregion_setup(dev_priv);
> > +
> > + intel_pcode_init(dev_priv);
> > +
> > /*
> > * Fill the dram structure to get the system dram info. This will be
> > * used for memory latency calculation.
> > */
> > intel_dram_detect(dev_priv);
> >
> > - intel_pcode_init(dev_priv);
> > -
> > intel_bw_init_hw(dev_priv);
> >
> > return 0;
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 250e92910fa1..a2ae21082b34 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1144,6 +1144,7 @@ struct drm_i915_private {
> > INTEL_DRAM_LPDDR3,
> > INTEL_DRAM_LPDDR4
> > } type;
> > + u8 qgv_points;
> > } dram_info;
> >
> > struct intel_bw_info {
> > diff --git a/drivers/gpu/drm/i915/intel_dram.c b/drivers/gpu/drm/i915/intel_dram.c
> > index 694fbd8c9cd4..1298823c957c 100644
> > --- a/drivers/gpu/drm/i915/intel_dram.c
> > +++ b/drivers/gpu/drm/i915/intel_dram.c
> > @@ -5,6 +5,7 @@
> >
> > #include "i915_drv.h"
> > #include "intel_dram.h"
> > +#include "intel_sideband.h"
> >
> > struct dram_dimm_info {
> > u16 size;
> > @@ -408,6 +409,78 @@ static int bxt_get_dram_info(struct drm_i915_private *i915)
> > return 0;
> > }
> >
> > +static int icl_pcode_read_mem_global_info(struct drm_i915_private *dev_priv)
> > +{
> > + struct dram_info *dram_info = &dev_priv->dram_info;
> > + u32 val = 0;
> > + int ret;
> > +
> > + ret = sandybridge_pcode_read(dev_priv,
> > + ICL_PCODE_MEM_SUBSYSYSTEM_INFO |
> > + ICL_PCODE_MEM_SS_READ_GLOBAL_INFO,
> > + &val, NULL);
> > + if (ret)
> > + return ret;
> > +
> > + if (IS_GEN(dev_priv, 12)) {
> > + switch (val & 0xf) {
> > + case 0:
> > + dram_info->type = INTEL_DRAM_DDR4;
> > + break;
> > + case 3:
> > + dram_info->type = INTEL_DRAM_LPDDR4;
> > + break;
> > + case 4:
> > + dram_info->type = INTEL_DRAM_DDR3;
> > + break;
> > + case 5:
> > + dram_info->type = INTEL_DRAM_LPDDR3;
> > + break;
> > + default:
> > + MISSING_CASE(val & 0xf);
> > + return -1;
> > + }
> > + } else {
> > + switch (val & 0xf) {
> > + case 0:
> > + dram_info->type = INTEL_DRAM_DDR4;
> > + break;
> > + case 1:
> > + dram_info->type = INTEL_DRAM_DDR3;
> > + break;
> > + case 2:
> > + dram_info->type = INTEL_DRAM_LPDDR3;
> > + break;
> > + case 3:
> > + dram_info->type = INTEL_DRAM_LPDDR4;
> > + break;
> > + default:
> > + MISSING_CASE(val & 0xf);
> > + return -1;
> > + }
> > + }
> > +
> > + dram_info->num_channels = (val & 0xf0) >> 4;
> > + dram_info->qgv_points = (val & 0xf00) >> 8;
> > +
> > + return 0;
> > +}
> > +
> > +static int gen11_get_dram_info(struct drm_i915_private *i915)
> > +{
> > + if (INTEL_GEN(i915) == 11) {
> > + int ret = skl_get_dram_info(i915);
> > +
> > + if (ret)
> > + return ret;
> > + } else {
> > + /* Always needed for GEN12+ */
> > + i915->dram_info.is_16gb_dimm = true;
> > + }
> > +
> > + return icl_pcode_read_mem_global_info(i915);
> > +}
> > +
> > void intel_dram_detect(struct drm_i915_private *i915)
> > {
> > struct dram_info *dram_info = &i915->dram_info;
> > @@ -423,7 +496,9 @@ void intel_dram_detect(struct drm_i915_private *i915)
> > if (INTEL_GEN(i915) < 9 || !HAS_DISPLAY(i915))
> > return;
> >
> > - if (IS_GEN9_LP(i915))
> > + if (INTEL_GEN(i915) >= 11)
> > + ret = gen11_get_dram_info(i915);
>
> gen11 and gen12 implementation above are sufficiently different: better
> to keep the if/else chain here only
>
> static int gen11_get_dram_info(struct drm_i915_private *i915)
> {
> int ret = skl_get_dram_info(i915);
> if (ret)
> return ret;
>
> return icl_pcode_read_mem_global_info(i915);
> }
>
>
> static int gen12_get_dram_info(struct drm_i915_private *i915)
> {
> /* Always needed for GEN12+ */
> i915->dram_info.is_16gb_dimm = true;
>
> return icl_pcode_read_mem_global_info(i915);
> }
Done
>
> Also, now it seems we have lots of dead code since we are not calling
> skl_get_dram_info() for tgl. See
> tgl_get_dram_type(), skl_dram_get_channels_info, etc
Can't find those.
>
> Lucas De Marchi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-01-27 16:37 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-20 15:16 [Intel-gfx] [PATCH 1/4] drm/i915: Nuke not needed members of dram_info José Roberto de Souza
2021-01-20 15:16 ` [Intel-gfx] [PATCH 2/4] drm/i915/gen11+: Only load DRAM information from pcode José Roberto de Souza
2021-01-27 14:49 ` Lucas De Marchi
2021-01-27 16:37 ` Souza, Jose
2021-01-20 15:16 ` [Intel-gfx] [PATCH 3/4] drm/i915: Fail driver probe when unable to load DRAM information José Roberto de Souza
2021-01-20 15:16 ` [Intel-gfx] [PATCH 4/4] drm/i915: Rename is_16gb_dimm to wm_lv_0_adjust_needed José Roberto de Souza
2021-01-27 14:56 ` Lucas De Marchi
[not found] ` <20210120183151.jpfuda4htxs5qkxq@ldmartin-desk1>
[not found] ` <5168c1572a7c106313f7c47194cae267835b32af.camel@intel.com>
[not found] ` <20210120185245.2eqa42nmbm6yzeki@ldmartin-desk1>
2021-01-20 19:29 ` [Intel-gfx] [PATCH 1/4] drm/i915: Nuke not needed members of dram_info Souza, Jose
2021-01-27 14:35 ` Lucas De Marchi
2021-01-20 21:32 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/4] " Patchwork
2021-01-21 22:35 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/4] drm/i915: Nuke not needed members of dram_info (rev2) Patchwork
2021-01-22 18:49 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/4] drm/i915: Nuke not needed members of dram_info (rev3) Patchwork
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).