* [PATCH v2] drm/i915/tgl: Add memory type decoding for bandwidth checking
@ 2019-09-24 22:28 James Ausmus
2019-09-24 23:25 ` ✓ Fi.CI.BAT: success for " Patchwork
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: James Ausmus @ 2019-09-24 22:28 UTC (permalink / raw)
To: intel-gfx
The memory type values have changed in TGL, so we need to translate them
differently than ICL. While we're moving it, fix up the ICL translation
for LPDDR4.
BSpec: 53998
v2: Fix up ICL LPDDR4 entry (Ville); Drop unused values from TGL (Ville)
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Signed-off-by: James Ausmus <james.ausmus@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/display/intel_bw.c | 55 ++++++++++++++++++-------
1 file changed, 39 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
index cd58e47ab7b2..22e83f857de8 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.c
+++ b/drivers/gpu/drm/i915/display/intel_bw.c
@@ -35,22 +35,45 @@ static int icl_pcode_read_mem_global_info(struct drm_i915_private *dev_priv,
if (ret)
return ret;
- 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_LPDDR3;
- break;
- default:
- MISSING_CASE(val & 0xf);
- break;
+ 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;
--
2.22.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 7+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915/tgl: Add memory type decoding for bandwidth checking
2019-09-24 22:28 [PATCH v2] drm/i915/tgl: Add memory type decoding for bandwidth checking James Ausmus
@ 2019-09-24 23:25 ` Patchwork
2019-09-25 14:33 ` [PATCH v2] " Summers, Stuart
2019-09-25 18:54 ` ✓ Fi.CI.IGT: success for " Patchwork
2 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2019-09-24 23:25 UTC (permalink / raw)
To: James Ausmus; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/tgl: Add memory type decoding for bandwidth checking
URL : https://patchwork.freedesktop.org/series/67186/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_6952 -> Patchwork_14522
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14522/
Known issues
------------
Here are the changes found in Patchwork_14522 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_exec_create@basic:
- fi-icl-u2: [PASS][1] -> [INCOMPLETE][2] ([fdo#107713])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6952/fi-icl-u2/igt@gem_exec_create@basic.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14522/fi-icl-u2/igt@gem_exec_create@basic.html
* igt@i915_selftest@live_hangcheck:
- fi-kbl-7500u: [PASS][3] -> [INCOMPLETE][4] ([fdo#108744])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6952/fi-kbl-7500u/igt@i915_selftest@live_hangcheck.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14522/fi-kbl-7500u/igt@i915_selftest@live_hangcheck.html
* igt@kms_chamelium@dp-edid-read:
- fi-kbl-7500u: [PASS][5] -> [WARN][6] ([fdo#109483])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6952/fi-kbl-7500u/igt@kms_chamelium@dp-edid-read.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14522/fi-kbl-7500u/igt@kms_chamelium@dp-edid-read.html
* igt@kms_frontbuffer_tracking@basic:
- fi-hsw-peppy: [PASS][7] -> [DMESG-WARN][8] ([fdo#102614])
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6952/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14522/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html
#### Possible fixes ####
* igt@gem_exec_suspend@basic-s3:
- fi-blb-e6850: [INCOMPLETE][9] ([fdo#107718]) -> [PASS][10]
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6952/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14522/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html
* igt@kms_chamelium@hdmi-hpd-fast:
- fi-kbl-7500u: [FAIL][11] ([fdo#111407]) -> [PASS][12]
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6952/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14522/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
[fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
[fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
[fdo#108744]: https://bugs.freedesktop.org/show_bug.cgi?id=108744
[fdo#109483]: https://bugs.freedesktop.org/show_bug.cgi?id=109483
[fdo#111049]: https://bugs.freedesktop.org/show_bug.cgi?id=111049
[fdo#111155]: https://bugs.freedesktop.org/show_bug.cgi?id=111155
[fdo#111407]: https://bugs.freedesktop.org/show_bug.cgi?id=111407
Participating hosts (49 -> 40)
------------------------------
Additional (1): fi-kbl-soraka
Missing (10): fi-ilk-m540 fi-tgl-u2 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-gdg-551 fi-pnv-d510 fi-icl-y fi-icl-dsi fi-bdw-samus
Build changes
-------------
* CI: CI-20190529 -> None
* Linux: CI_DRM_6952 -> Patchwork_14522
CI-20190529: 20190529
CI_DRM_6952: ec3b5d92c87e554e407f308b4183f21d59c4a13d @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5202: 3499c5eb17054e2abd88023fe962768140d24302 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_14522: 797a36135600d7867b5d6cd46a2a51cabc882c16 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
797a36135600 drm/i915/tgl: Add memory type decoding for bandwidth checking
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14522/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] drm/i915/tgl: Add memory type decoding for bandwidth checking
2019-09-24 22:28 [PATCH v2] drm/i915/tgl: Add memory type decoding for bandwidth checking James Ausmus
2019-09-24 23:25 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2019-09-25 14:33 ` Summers, Stuart
2019-09-25 15:35 ` James Ausmus
2019-09-25 18:54 ` ✓ Fi.CI.IGT: success for " Patchwork
2 siblings, 1 reply; 7+ messages in thread
From: Summers, Stuart @ 2019-09-25 14:33 UTC (permalink / raw)
To: intel-gfx, Ausmus, James
[-- Attachment #1.1: Type: text/plain, Size: 2690 bytes --]
On Tue, 2019-09-24 at 15:28 -0700, James Ausmus wrote:
> The memory type values have changed in TGL, so we need to translate
> them
> differently than ICL. While we're moving it, fix up the ICL
> translation
> for LPDDR4.
>
> BSpec: 53998
>
> v2: Fix up ICL LPDDR4 entry (Ville); Drop unused values from TGL
> (Ville)
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> Signed-off-by: James Ausmus <james.ausmus@intel.com>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_bw.c | 55 ++++++++++++++++++-----
> --
> 1 file changed, 39 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_bw.c
> b/drivers/gpu/drm/i915/display/intel_bw.c
> index cd58e47ab7b2..22e83f857de8 100644
> --- a/drivers/gpu/drm/i915/display/intel_bw.c
> +++ b/drivers/gpu/drm/i915/display/intel_bw.c
> @@ -35,22 +35,45 @@ static int icl_pcode_read_mem_global_info(struct
> drm_i915_private *dev_priv,
> if (ret)
> return ret;
>
> - 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_LPDDR3;
> - break;
> - default:
> - MISSING_CASE(val & 0xf);
> - break;
> + 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;
James, is there a reason we can't just combine these two conditions
into one switch statement? At initial glance it looks like the cases
are the same for the common ones and the only real difference is the
supported bits.
Thanks,
Stuart
> + }
> + } else {
> + MISSING_CASE(INTEL_GEN(dev_priv));
> + qi->dram_type = INTEL_DRAM_LPDDR3; /* Conservative
> default */
> }
>
> qi->num_channels = (val & 0xf0) >> 4;
[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3270 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] drm/i915/tgl: Add memory type decoding for bandwidth checking
2019-09-25 14:33 ` [PATCH v2] " Summers, Stuart
@ 2019-09-25 15:35 ` James Ausmus
2019-09-25 22:35 ` Summers, Stuart
0 siblings, 1 reply; 7+ messages in thread
From: James Ausmus @ 2019-09-25 15:35 UTC (permalink / raw)
To: Summers, Stuart; +Cc: intel-gfx
On Wed, Sep 25, 2019 at 07:33:38AM -0700, Summers, Stuart wrote:
> On Tue, 2019-09-24 at 15:28 -0700, James Ausmus wrote:
> > The memory type values have changed in TGL, so we need to translate
> > them
> > differently than ICL. While we're moving it, fix up the ICL
> > translation
> > for LPDDR4.
> >
> > BSpec: 53998
> >
> > v2: Fix up ICL LPDDR4 entry (Ville); Drop unused values from TGL
> > (Ville)
> >
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> > Signed-off-by: James Ausmus <james.ausmus@intel.com>
> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/display/intel_bw.c | 55 ++++++++++++++++++-----
> > --
> > 1 file changed, 39 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_bw.c
> > b/drivers/gpu/drm/i915/display/intel_bw.c
> > index cd58e47ab7b2..22e83f857de8 100644
> > --- a/drivers/gpu/drm/i915/display/intel_bw.c
> > +++ b/drivers/gpu/drm/i915/display/intel_bw.c
> > @@ -35,22 +35,45 @@ static int icl_pcode_read_mem_global_info(struct
> > drm_i915_private *dev_priv,
> > if (ret)
> > return ret;
> >
> > - 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_LPDDR3;
> > - break;
> > - default:
> > - MISSING_CASE(val & 0xf);
> > - break;
> > + 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;
>
> James, is there a reason we can't just combine these two conditions
> into one switch statement? At initial glance it looks like the cases
> are the same for the common ones and the only real difference is the
> supported bits.
The info I got from the HW guys indicates that the same values are very
likely to have different meanings for different gens, and likely to even
have different values for variants of a single gen, so as more platforms
are added in the future, a single switch would get very messy. Even now,
I think it would be fairly ugly, as it would look something like:
switch (val) {
case 0:
DDR4;
case 1:
if (GEN == 11)
DDR3;
else
MISSING_CASE(val);
case 2:
if (GEN == 11)
LPDDR3;
else
MISSING_CASE(val);
case 3:
LPDDR4;
case 4:
if (GEN == 12)
DDR3;
else
MISSING_CASE(val);
case 5:
if (GEN == 12)
LPDDR3;
else
MISSING_CASE(val);
}
And then start adding special cases for variants within a gen, as well
as additional gen checks, and I think it starts looking fairly
spaghetti. :)
-James
>
> Thanks,
> Stuart
>
> > + }
> > + } else {
> > + MISSING_CASE(INTEL_GEN(dev_priv));
> > + qi->dram_type = INTEL_DRAM_LPDDR3; /* Conservative
> > default */
> > }
> >
> > qi->num_channels = (val & 0xf0) >> 4;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* ✓ Fi.CI.IGT: success for drm/i915/tgl: Add memory type decoding for bandwidth checking
2019-09-24 22:28 [PATCH v2] drm/i915/tgl: Add memory type decoding for bandwidth checking James Ausmus
2019-09-24 23:25 ` ✓ Fi.CI.BAT: success for " Patchwork
2019-09-25 14:33 ` [PATCH v2] " Summers, Stuart
@ 2019-09-25 18:54 ` Patchwork
2 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2019-09-25 18:54 UTC (permalink / raw)
To: James Ausmus; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/tgl: Add memory type decoding for bandwidth checking
URL : https://patchwork.freedesktop.org/series/67186/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_6952_full -> Patchwork_14522_full
====================================================
Summary
-------
**SUCCESS**
No regressions found.
Known issues
------------
Here are the changes found in Patchwork_14522_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_ctx_isolation@bcs0-s3:
- shard-apl: [PASS][1] -> [DMESG-WARN][2] ([fdo#108566]) +3 similar issues
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6952/shard-apl5/igt@gem_ctx_isolation@bcs0-s3.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14522/shard-apl4/igt@gem_ctx_isolation@bcs0-s3.html
* igt@gem_exec_schedule@preempt-other-chain-bsd:
- shard-iclb: [PASS][3] -> [SKIP][4] ([fdo#111325]) +4 similar issues
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6952/shard-iclb5/igt@gem_exec_schedule@preempt-other-chain-bsd.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14522/shard-iclb2/igt@gem_exec_schedule@preempt-other-chain-bsd.html
* igt@i915_suspend@fence-restore-untiled:
- shard-skl: [PASS][5] -> [INCOMPLETE][6] ([fdo#104108])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6952/shard-skl8/igt@i915_suspend@fence-restore-untiled.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14522/shard-skl1/igt@i915_suspend@fence-restore-untiled.html
* igt@kms_cursor_crc@pipe-a-cursor-64x64-random:
- shard-skl: [PASS][7] -> [FAIL][8] ([fdo#103232])
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6952/shard-skl8/igt@kms_cursor_crc@pipe-a-cursor-64x64-random.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14522/shard-skl2/igt@kms_cursor_crc@pipe-a-cursor-64x64-random.html
* igt@kms_flip@flip-vs-expired-vblank-interruptible:
- shard-skl: [PASS][9] -> [FAIL][10] ([fdo#105363])
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6952/shard-skl3/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14522/shard-skl8/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
- shard-glk: [PASS][11] -> [FAIL][12] ([fdo#105363])
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6952/shard-glk9/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14522/shard-glk6/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render:
- shard-iclb: [PASS][13] -> [FAIL][14] ([fdo#103167]) +3 similar issues
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6952/shard-iclb1/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14522/shard-iclb8/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render.html
* igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-mmap-gtt:
- shard-skl: [PASS][15] -> [FAIL][16] ([fdo#103167]) +2 similar issues
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6952/shard-skl8/igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-mmap-gtt.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14522/shard-skl2/igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-mmap-gtt.html
* igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min:
- shard-skl: [PASS][17] -> [FAIL][18] ([fdo#108145])
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6952/shard-skl10/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14522/shard-skl7/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html
* igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
- shard-skl: [PASS][19] -> [FAIL][20] ([fdo#108145] / [fdo#110403])
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6952/shard-skl6/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14522/shard-skl10/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html
* igt@kms_plane_scaling@pipe-b-scaler-with-rotation:
- shard-glk: [PASS][21] -> [DMESG-WARN][22] ([fdo#105763] / [fdo#106538])
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6952/shard-glk1/igt@kms_plane_scaling@pipe-b-scaler-with-rotation.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14522/shard-glk1/igt@kms_plane_scaling@pipe-b-scaler-with-rotation.html
* igt@kms_setmode@basic:
- shard-hsw: [PASS][23] -> [FAIL][24] ([fdo#99912])
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6952/shard-hsw5/igt@kms_setmode@basic.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14522/shard-hsw5/igt@kms_setmode@basic.html
* igt@perf@polling:
- shard-skl: [PASS][25] -> [FAIL][26] ([fdo#110728])
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6952/shard-skl9/igt@perf@polling.html
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14522/shard-skl5/igt@perf@polling.html
* igt@prime_busy@hang-bsd2:
- shard-iclb: [PASS][27] -> [SKIP][28] ([fdo#109276]) +16 similar issues
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6952/shard-iclb4/igt@prime_busy@hang-bsd2.html
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14522/shard-iclb5/igt@prime_busy@hang-bsd2.html
#### Possible fixes ####
* igt@gem_ctx_isolation@vcs0-s3:
- shard-skl: [INCOMPLETE][29] ([fdo#104108]) -> [PASS][30]
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6952/shard-skl1/igt@gem_ctx_isolation@vcs0-s3.html
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14522/shard-skl1/igt@gem_ctx_isolation@vcs0-s3.html
* igt@gem_ctx_switch@bcs0-heavy-queue:
- shard-apl: [INCOMPLETE][31] ([fdo#103927]) -> [PASS][32] +1 similar issue
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6952/shard-apl4/igt@gem_ctx_switch@bcs0-heavy-queue.html
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14522/shard-apl3/igt@gem_ctx_switch@bcs0-heavy-queue.html
* igt@gem_exec_schedule@independent-bsd2:
- shard-iclb: [SKIP][33] ([fdo#109276]) -> [PASS][34] +16 similar issues
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6952/shard-iclb5/igt@gem_exec_schedule@independent-bsd2.html
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14522/shard-iclb2/igt@gem_exec_schedule@independent-bsd2.html
* igt@gem_exec_schedule@reorder-wide-bsd:
- shard-iclb: [SKIP][35] ([fdo#111325]) -> [PASS][36] +8 similar issues
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6952/shard-iclb4/igt@gem_exec_schedule@reorder-wide-bsd.html
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14522/shard-iclb8/igt@gem_exec_schedule@reorder-wide-bsd.html
* igt@gem_softpin@noreloc-s3:
- shard-kbl: [DMESG-WARN][37] ([fdo#108566]) -> [PASS][38]
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6952/shard-kbl3/igt@gem_softpin@noreloc-s3.html
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14522/shard-kbl4/igt@gem_softpin@noreloc-s3.html
* igt@gem_tiled_swapping@non-threaded:
- shard-apl: [DMESG-WARN][39] ([fdo#108686]) -> [PASS][40]
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6952/shard-apl7/igt@gem_tiled_swapping@non-threaded.html
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14522/shard-apl8/igt@gem_tiled_swapping@non-threaded.html
* igt@gem_workarounds@suspend-resume-context:
- shard-apl: [DMESG-WARN][41] ([fdo#108566]) -> [PASS][42] +6 similar issues
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6952/shard-apl7/igt@gem_workarounds@suspend-resume-context.html
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14522/shard-apl8/igt@gem_workarounds@suspend-resume-context.html
* {igt@i915_pm_dc@dc6-psr}:
- shard-iclb: [FAIL][43] ([fdo#110548]) -> [PASS][44]
[43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6952/shard-iclb6/igt@i915_pm_dc@dc6-psr.html
[44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14522/shard-iclb3/igt@i915_pm_dc@dc6-psr.html
* igt@kms_flip@2x-plain-flip-fb-recreate-interruptible:
- shard-hsw: [INCOMPLETE][45] ([fdo#103540]) -> [PASS][46]
[45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6952/shard-hsw4/igt@kms_flip@2x-plain-flip-fb-recreate-interruptible.html
[46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14522/shard-hsw6/igt@kms_flip@2x-plain-flip-fb-recreate-interruptible.html
* igt@kms_flip@flip-vs-suspend:
- shard-skl: [INCOMPLETE][47] ([fdo#109507]) -> [PASS][48]
[47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6952/shard-skl4/igt@kms_flip@flip-vs-suspend.html
[48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14522/shard-skl3/igt@kms_flip@flip-vs-suspend.html
* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-mmap-cpu:
- shard-iclb: [FAIL][49] ([fdo#103167]) -> [PASS][50] +1 similar issue
[49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6952/shard-iclb8/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-mmap-cpu.html
[50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14522/shard-iclb4/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-mmap-cpu.html
* igt@kms_frontbuffer_tracking@fbcpsr-slowdraw:
- shard-iclb: [INCOMPLETE][51] ([fdo#106978] / [fdo#107713]) -> [PASS][52]
[51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6952/shard-iclb1/igt@kms_frontbuffer_tracking@fbcpsr-slowdraw.html
[52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14522/shard-iclb8/igt@kms_frontbuffer_tracking@fbcpsr-slowdraw.html
* igt@kms_plane_lowres@pipe-a-tiling-x:
- shard-iclb: [FAIL][53] ([fdo#103166]) -> [PASS][54] +1 similar issue
[53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6952/shard-iclb8/igt@kms_plane_lowres@pipe-a-tiling-x.html
[54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14522/shard-iclb6/igt@kms_plane_lowres@pipe-a-tiling-x.html
* igt@kms_psr2_su@page_flip:
- shard-iclb: [SKIP][55] ([fdo#109642] / [fdo#111068]) -> [PASS][56]
[55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6952/shard-iclb5/igt@kms_psr2_su@page_flip.html
[56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14522/shard-iclb2/igt@kms_psr2_su@page_flip.html
* igt@kms_setmode@basic:
- shard-apl: [FAIL][57] ([fdo#99912]) -> [PASS][58]
[57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6952/shard-apl7/igt@kms_setmode@basic.html
[58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14522/shard-apl8/igt@kms_setmode@basic.html
- shard-kbl: [FAIL][59] ([fdo#99912]) -> [PASS][60]
[59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6952/shard-kbl3/igt@kms_setmode@basic.html
[60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14522/shard-kbl7/igt@kms_setmode@basic.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
[fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
[fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
[fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
[fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
[fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
[fdo#106538]: https://bugs.freedesktop.org/show_bug.cgi?id=106538
[fdo#106978]: https://bugs.freedesktop.org/show_bug.cgi?id=106978
[fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
[fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
[fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
[fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
[fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
[fdo#109507]: https://bugs.freedesktop.org/show_bug.cgi?id=109507
[fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
[fdo#110403]: https://bugs.freedesktop.org/show_bug.cgi?id=110403
[fdo#110548]: https://bugs.freedesktop.org/show_bug.cgi?id=110548
[fdo#110728]: https://bugs.freedesktop.org/show_bug.cgi?id=110728
[fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
[fdo#111325]: https://bugs.freedesktop.org/show_bug.cgi?id=111325
[fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
Participating hosts (9 -> 9)
------------------------------
No changes in participating hosts
Build changes
-------------
* CI: CI-20190529 -> None
* Linux: CI_DRM_6952 -> Patchwork_14522
CI-20190529: 20190529
CI_DRM_6952: ec3b5d92c87e554e407f308b4183f21d59c4a13d @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5202: 3499c5eb17054e2abd88023fe962768140d24302 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_14522: 797a36135600d7867b5d6cd46a2a51cabc882c16 @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14522/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] drm/i915/tgl: Add memory type decoding for bandwidth checking
2019-09-25 15:35 ` James Ausmus
@ 2019-09-25 22:35 ` Summers, Stuart
2019-09-25 22:39 ` James Ausmus
0 siblings, 1 reply; 7+ messages in thread
From: Summers, Stuart @ 2019-09-25 22:35 UTC (permalink / raw)
To: Ausmus, James; +Cc: intel-gfx
[-- Attachment #1.1: Type: text/plain, Size: 4504 bytes --]
On Wed, 2019-09-25 at 08:35 -0700, James Ausmus wrote:
> On Wed, Sep 25, 2019 at 07:33:38AM -0700, Summers, Stuart wrote:
> > On Tue, 2019-09-24 at 15:28 -0700, James Ausmus wrote:
> > > The memory type values have changed in TGL, so we need to
> > > translate
> > > them
> > > differently than ICL. While we're moving it, fix up the ICL
> > > translation
> > > for LPDDR4.
> > >
> > > BSpec: 53998
> > >
> > > v2: Fix up ICL LPDDR4 entry (Ville); Drop unused values from TGL
> > > (Ville)
> > >
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> > > Signed-off-by: James Ausmus <james.ausmus@intel.com>
> > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > > drivers/gpu/drm/i915/display/intel_bw.c | 55 ++++++++++++++++++-
> > > ----
> > > --
> > > 1 file changed, 39 insertions(+), 16 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_bw.c
> > > b/drivers/gpu/drm/i915/display/intel_bw.c
> > > index cd58e47ab7b2..22e83f857de8 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_bw.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_bw.c
> > > @@ -35,22 +35,45 @@ static int
> > > icl_pcode_read_mem_global_info(struct
> > > drm_i915_private *dev_priv,
> > > if (ret)
> > > return ret;
> > >
> > > - 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_LPDDR3;
> > > - break;
> > > - default:
> > > - MISSING_CASE(val & 0xf);
> > > - break;
> > > + 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;
> >
> > James, is there a reason we can't just combine these two conditions
> > into one switch statement? At initial glance it looks like the
> > cases
> > are the same for the common ones and the only real difference is
> > the
> > supported bits.
>
> The info I got from the HW guys indicates that the same values are
> very
> likely to have different meanings for different gens, and likely to
> even
> have different values for variants of a single gen, so as more
> platforms
> are added in the future, a single switch would get very messy. Even
> now,
> I think it would be fairly ugly, as it would look something like:
>
> switch (val) {
> case 0:
> DDR4;
> case 1:
> if (GEN == 11)
> DDR3;
> else
> MISSING_CASE(val);
> case 2:
> if (GEN == 11)
> LPDDR3;
> else
> MISSING_CASE(val);
> case 3:
> LPDDR4;
> case 4:
> if (GEN == 12)
> DDR3;
> else
> MISSING_CASE(val);
> case 5:
> if (GEN == 12)
> LPDDR3;
> else
> MISSING_CASE(val);
> }
>
> And then start adding special cases for variants within a gen, as
> well
> as additional gen checks, and I think it starts looking fairly
> spaghetti. :)
I understand and thanks for the explanation. Your reasoning makes sense
to me.
I checked bspec and confirmed the TGL entries look right. We also spoke
in IRC and I agree with the changes you're making for ICL. With that:
Reviewed-by: Stuart Summers <stuart.summers@intel.com>
>
> -James
>
> >
> > Thanks,
> > Stuart
> >
> > > + }
> > > + } else {
> > > + MISSING_CASE(INTEL_GEN(dev_priv));
> > > + qi->dram_type = INTEL_DRAM_LPDDR3; /* Conservative
> > > default */
> > > }
> > >
> > > qi->num_channels = (val & 0xf0) >> 4;
>
>
[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3270 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] drm/i915/tgl: Add memory type decoding for bandwidth checking
2019-09-25 22:35 ` Summers, Stuart
@ 2019-09-25 22:39 ` James Ausmus
0 siblings, 0 replies; 7+ messages in thread
From: James Ausmus @ 2019-09-25 22:39 UTC (permalink / raw)
To: Summers, Stuart; +Cc: intel-gfx
On Wed, Sep 25, 2019 at 03:35:28PM -0700, Summers, Stuart wrote:
> On Wed, 2019-09-25 at 08:35 -0700, James Ausmus wrote:
> > On Wed, Sep 25, 2019 at 07:33:38AM -0700, Summers, Stuart wrote:
> > > On Tue, 2019-09-24 at 15:28 -0700, James Ausmus wrote:
> > > > The memory type values have changed in TGL, so we need to
> > > > translate
> > > > them
> > > > differently than ICL. While we're moving it, fix up the ICL
> > > > translation
> > > > for LPDDR4.
> > > >
> > > > BSpec: 53998
> > > >
> > > > v2: Fix up ICL LPDDR4 entry (Ville); Drop unused values from TGL
> > > > (Ville)
> > > >
> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> > > > Signed-off-by: James Ausmus <james.ausmus@intel.com>
> > > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > ---
> > > > drivers/gpu/drm/i915/display/intel_bw.c | 55 ++++++++++++++++++-
> > > > ----
> > > > --
> > > > 1 file changed, 39 insertions(+), 16 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_bw.c
> > > > b/drivers/gpu/drm/i915/display/intel_bw.c
> > > > index cd58e47ab7b2..22e83f857de8 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_bw.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_bw.c
> > > > @@ -35,22 +35,45 @@ static int
> > > > icl_pcode_read_mem_global_info(struct
> > > > drm_i915_private *dev_priv,
> > > > if (ret)
> > > > return ret;
> > > >
> > > > - 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_LPDDR3;
> > > > - break;
> > > > - default:
> > > > - MISSING_CASE(val & 0xf);
> > > > - break;
> > > > + 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;
> > >
> > > James, is there a reason we can't just combine these two conditions
> > > into one switch statement? At initial glance it looks like the
> > > cases
> > > are the same for the common ones and the only real difference is
> > > the
> > > supported bits.
> >
> > The info I got from the HW guys indicates that the same values are
> > very
> > likely to have different meanings for different gens, and likely to
> > even
> > have different values for variants of a single gen, so as more
> > platforms
> > are added in the future, a single switch would get very messy. Even
> > now,
> > I think it would be fairly ugly, as it would look something like:
> >
> > switch (val) {
> > case 0:
> > DDR4;
> > case 1:
> > if (GEN == 11)
> > DDR3;
> > else
> > MISSING_CASE(val);
> > case 2:
> > if (GEN == 11)
> > LPDDR3;
> > else
> > MISSING_CASE(val);
> > case 3:
> > LPDDR4;
> > case 4:
> > if (GEN == 12)
> > DDR3;
> > else
> > MISSING_CASE(val);
> > case 5:
> > if (GEN == 12)
> > LPDDR3;
> > else
> > MISSING_CASE(val);
> > }
> >
> > And then start adding special cases for variants within a gen, as
> > well
> > as additional gen checks, and I think it starts looking fairly
> > spaghetti. :)
>
> I understand and thanks for the explanation. Your reasoning makes sense
> to me.
>
> I checked bspec and confirmed the TGL entries look right. We also spoke
> in IRC and I agree with the changes you're making for ICL. With that:
>
> Reviewed-by: Stuart Summers <stuart.summers@intel.com>
Thanks for the review!
-James
>
> >
> > -James
> >
> > >
> > > Thanks,
> > > Stuart
> > >
> > > > + }
> > > > + } else {
> > > > + MISSING_CASE(INTEL_GEN(dev_priv));
> > > > + qi->dram_type = INTEL_DRAM_LPDDR3; /* Conservative
> > > > default */
> > > > }
> > > >
> > > > qi->num_channels = (val & 0xf0) >> 4;
> >
> >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-09-25 22:38 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-24 22:28 [PATCH v2] drm/i915/tgl: Add memory type decoding for bandwidth checking James Ausmus
2019-09-24 23:25 ` ✓ Fi.CI.BAT: success for " Patchwork
2019-09-25 14:33 ` [PATCH v2] " Summers, Stuart
2019-09-25 15:35 ` James Ausmus
2019-09-25 22:35 ` Summers, Stuart
2019-09-25 22:39 ` James Ausmus
2019-09-25 18:54 ` ✓ Fi.CI.IGT: success for " Patchwork
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.