| Message ID | 1476488825-5673-1-git-send-email-anusha.srivatsa@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> == Series Details == > > Series: series starting with [1/1] drm/i915/guc: Sanitory checks for platform > that dont have GuC > URL : https://patchwork.freedesktop.org/series/13815/ > State : warning > > == Summary == > > Series 13815v1 Series without cover letter > https://patchwork.freedesktop.org/api/1.0/series/13815/revisions/1/mbox/ > > Test drv_module_reload_basic: > pass -> SKIP (fi-skl-6770hq) Known unstable still. > Test kms_flip: > Subgroup basic-flip-vs-wf_vblank: > dmesg-warn -> PASS (fi-skl-6770hq) > dmesg-warn -> PASS (fi-skl-6700k) > Subgroup basic-plain-flip: > pass -> DMESG-WARN (fi-ilk-650) dmesg [ 395.814348] [drm:intel_cpu_fifo_underrun_irq_handler [i915]] *ERROR* CPU pipe B FIFO underrun https://bugs.freedesktop.org/show_bug.cgi?id=98251 [ 395.814464] [drm:intel_pch_fifo_underrun_irq_handler [i915]] *ERROR* PCH transcoder B FIFO underrun > Test kms_pipe_crc_basic: > Subgroup read-crc-pipe-a-frame-sequence: > dmesg-warn -> PASS (fi-ilk-650) > Subgroup read-crc-pipe-b: > dmesg-warn -> PASS (fi-ilk-650) > Subgroup suspend-read-crc-pipe-c: > incomplete -> PASS (fi-skl-6700k) > Test vgem_basic: > Subgroup unload: > pass -> SKIP (fi-hsw-4770) > skip -> PASS (fi-skl-6770hq) Still unstable. > > fi-bdw-5557u total:246 pass:231 dwarn:0 dfail:0 fail:0 skip:15 > fi-bsw-n3050 total:246 pass:204 dwarn:0 dfail:0 fail:0 skip:42 > fi-bxt-t5700 total:246 pass:216 dwarn:0 dfail:0 fail:0 skip:30 > fi-byt-j1900 total:246 pass:213 dwarn:1 dfail:0 fail:1 skip:31 > fi-byt-n2820 total:246 pass:210 dwarn:0 dfail:0 fail:1 skip:35 > fi-hsw-4770 total:246 pass:223 dwarn:0 dfail:0 fail:0 skip:23 > fi-hsw-4770r total:246 pass:224 dwarn:0 dfail:0 fail:0 skip:22 > fi-ilk-650 total:246 pass:183 dwarn:1 dfail:0 fail:2 skip:60 > fi-ivb-3520m total:246 pass:221 dwarn:0 dfail:0 fail:0 skip:25 > fi-ivb-3770 total:246 pass:221 dwarn:0 dfail:0 fail:0 skip:25 > fi-kbl-7200u total:246 pass:222 dwarn:0 dfail:0 fail:0 skip:24 > fi-skl-6260u total:246 pass:232 dwarn:0 dfail:0 fail:0 skip:14 > fi-skl-6700hq total:246 pass:223 dwarn:0 dfail:0 fail:0 skip:23 > fi-skl-6700k total:246 pass:221 dwarn:1 dfail:0 fail:0 skip:24 > fi-skl-6770hq total:246 pass:229 dwarn:1 dfail:0 fail:1 skip:15 > fi-snb-2520m total:246 pass:210 dwarn:0 dfail:0 fail:0 skip:36 > fi-snb-2600 total:246 pass:209 dwarn:0 dfail:0 fail:0 skip:37 > > Results at /archive/results/CI_IGT_test/Patchwork_2730/ > > 38ecbe9082e477953fe165265c76e6c6061aeaf6 drm-intel-nightly: 2016y-10m- > 14d-16h-23m-48s UTC integration manifest > 1392686 drm/i915/guc: Sanitory checks for platform that dont have GuC > Jani Saarinen Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
Em Sex, 2016-10-14 às 16:47 -0700, Anusha Srivatsa escreveu: > i915.enable_guc_loading/submission=2 forces the usage of GuC. > For platforms that do not have a GuC, asking the kernel to use a GuC > should not result in an error state. Do extra checks to see if the > platform even has a GuC or not, regardless of the kernel parameter. > > v2: Based on Rodrigo's patch and Paulo's suggestion(Paulo, Rodrigo) > v3: Correct the Indentation(Jani, Paulo) > v4: Added the blank line(Jani, Paulo) > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97573 > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Cc: Zanoni Paulo <paulo.r.zanoni@intel.com> > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com> > --- > drivers/gpu/drm/i915/intel_guc_loader.c | 16 +++++++++++----- > 1 file changed, 11 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c > b/drivers/gpu/drm/i915/intel_guc_loader.c > index 678b51a..94c3ad9 100644 > --- a/drivers/gpu/drm/i915/intel_guc_loader.c > +++ b/drivers/gpu/drm/i915/intel_guc_loader.c > @@ -719,11 +719,17 @@ void intel_guc_init(struct drm_device *dev) > struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw; > const char *fw_path; > > - /* A negative value means "use platform default" */ > - if (i915.enable_guc_loading < 0) > - i915.enable_guc_loading = HAS_GUC_UCODE(dev); > - if (i915.enable_guc_submission < 0) > - i915.enable_guc_submission = HAS_GUC_SCHED(dev); > + if (!HAS_GUC(dev)) { > + i915.enable_guc_loading = 0; > + i915.enable_guc_submission = 0; > + } else { > + /* A negative value means "use platform default" */ > + if (i915.enable_guc_loading < 0) > + i915.enable_guc_loading = > HAS_GUC_UCODE(dev); > + if (i915.enable_guc_submission < 0) > + i915.enable_guc_submission = > HAS_GUC_SCHED(dev); > + } > + Previously you were removing the blank line at this spot, leaving the code without any empty lines between the two "if" statements. Now in this version, instead of just not removing the blank line, you added an extra one, so there are two blank lines between the two "if" statements. Again, this is not the usual coding style. Anyway, I amended your patch, added a R-B tag and merged it. Thanks for the patch. > > if (!HAS_GUC_UCODE(dev)) { > fw_path = NULL;
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c index 678b51a..94c3ad9 100644 --- a/drivers/gpu/drm/i915/intel_guc_loader.c +++ b/drivers/gpu/drm/i915/intel_guc_loader.c @@ -719,11 +719,17 @@ void intel_guc_init(struct drm_device *dev) struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw; const char *fw_path; - /* A negative value means "use platform default" */ - if (i915.enable_guc_loading < 0) - i915.enable_guc_loading = HAS_GUC_UCODE(dev); - if (i915.enable_guc_submission < 0) - i915.enable_guc_submission = HAS_GUC_SCHED(dev); + if (!HAS_GUC(dev)) { + i915.enable_guc_loading = 0; + i915.enable_guc_submission = 0; + } else { + /* A negative value means "use platform default" */ + if (i915.enable_guc_loading < 0) + i915.enable_guc_loading = HAS_GUC_UCODE(dev); + if (i915.enable_guc_submission < 0) + i915.enable_guc_submission = HAS_GUC_SCHED(dev); + } + if (!HAS_GUC_UCODE(dev)) { fw_path = NULL;
i915.enable_guc_loading/submission=2 forces the usage of GuC. For platforms that do not have a GuC, asking the kernel to use a GuC should not result in an error state. Do extra checks to see if the platform even has a GuC or not, regardless of the kernel parameter. v2: Based on Rodrigo's patch and Paulo's suggestion(Paulo, Rodrigo) v3: Correct the Indentation(Jani, Paulo) v4: Added the blank line(Jani, Paulo) Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97573 Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: Zanoni Paulo <paulo.r.zanoni@intel.com> Cc: Jani Nikula <jani.nikula@linux.intel.com> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com> --- drivers/gpu/drm/i915/intel_guc_loader.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)