| Message ID | 20210127165402.117829-3-jose.souza@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/4] drm/i915: Nuke not needed members of dram_info | expand |
On Wed, Jan 27, 2021 at 08:54:01AM -0800, Jose Souza wrote: >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> makes sense and seems correct. But this needs to be tested on DG1 that is not on CI and AFAIR misbehaved when trying to get this info from pcode. If that is passing now, Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com> Lucas De Marchi >--- > 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 aec0e870dc25..7ff58ea30c7c 100644 >--- a/drivers/gpu/drm/i915/i915_drv.c >+++ b/drivers/gpu/drm/i915/i915_drv.c >@@ -622,12 +622,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 4d5ab206eacb..6ce56eedaf12 100644 >--- a/drivers/gpu/drm/i915/intel_dram.c >+++ b/drivers/gpu/drm/i915/intel_dram.c >@@ -484,7 +484,7 @@ static int gen12_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; >@@ -497,7 +497,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) >= 12) > ret = gen12_get_dram_info(i915); >@@ -507,13 +507,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
On Wed, 2021-01-27 at 20:21 -0800, Lucas De Marchi wrote: > On Wed, Jan 27, 2021 at 08:54:01AM -0800, Jose Souza wrote: > > 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> > > makes sense and seems correct. But this needs to be tested on DG1 that > is not on CI and AFAIR misbehaved when trying to get this info from > pcode. If that is passing now, Okay, will send the 3 other and merge as soon as I get CI results. Will do some more testing with this one before merging it, it might be causing some regression in fi-glk-dsi. thanks for the reviews. > > > Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com> > > Lucas De Marchi > > > --- > > 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 aec0e870dc25..7ff58ea30c7c 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.c > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > @@ -622,12 +622,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 4d5ab206eacb..6ce56eedaf12 100644 > > --- a/drivers/gpu/drm/i915/intel_dram.c > > +++ b/drivers/gpu/drm/i915/intel_dram.c > > @@ -484,7 +484,7 @@ static int gen12_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; > > @@ -497,7 +497,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) >= 12) > > ret = gen12_get_dram_info(i915); > > @@ -507,13 +507,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
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index aec0e870dc25..7ff58ea30c7c 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -622,12 +622,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 4d5ab206eacb..6ce56eedaf12 100644 --- a/drivers/gpu/drm/i915/intel_dram.c +++ b/drivers/gpu/drm/i915/intel_dram.c @@ -484,7 +484,7 @@ static int gen12_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; @@ -497,7 +497,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) >= 12) ret = gen12_get_dram_info(i915); @@ -507,13 +507,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__ */
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(-)