| Message ID | 1470842206-35685-1-git-send-email-tvrtko.ursulin@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 10/08/16 16:16, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Currently to change the firmware one has to update the exported > module firmware string and the major-minor versions used for > verification after load. Consolidate that to a single place > defining correct major and minor versions per platform. > > v2: Rebased for KBL. > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Dave Gordon <david.s.gordon@intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Cc: Peter Antoine <peter.antoine@intel.com> > Cc: Michel Thierry <michel.thierry@intel.com> > Reviewed-by: Dave Gordon <david.s.gordon@intel.com> (v1) You can carry the R-b over to v2 too :) But wouldn't it be even nicer to unify with the HuC and DMC loaders too? .Dave. > --- > drivers/gpu/drm/i915/intel_guc_loader.c | 30 +++++++++++++++++++++--------- > 1 file changed, 21 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c > index 3763e30cc165..bfcf6b5d29ad 100644 > --- a/drivers/gpu/drm/i915/intel_guc_loader.c > +++ b/drivers/gpu/drm/i915/intel_guc_loader.c > @@ -59,13 +59,25 @@ > * > */ > > -#define I915_SKL_GUC_UCODE "i915/skl_guc_ver6_1.bin" > +#define SKL_FW_MAJOR 6 > +#define SKL_FW_MINOR 1 > + > +#define BXT_FW_MAJOR 8 > +#define BXT_FW_MINOR 7 > + > +#define KBL_FW_MAJOR 9 > +#define KBL_FW_MINOR 14 > + > +#define GUC_FW_PATH(platform, major, minor) \ > + "i915/" __stringify(platform) "_guc_ver" __stringify(major) "_" __stringify(minor) ".bin" > + > +#define I915_SKL_GUC_UCODE GUC_FW_PATH(skl, SKL_FW_MAJOR, SKL_FW_MINOR) > MODULE_FIRMWARE(I915_SKL_GUC_UCODE); > > -#define I915_BXT_GUC_UCODE "i915/bxt_guc_ver8_7.bin" > +#define I915_BXT_GUC_UCODE GUC_FW_PATH(bxt, BXT_FW_MAJOR, BXT_FW_MINOR) > MODULE_FIRMWARE(I915_BXT_GUC_UCODE); > > -#define I915_KBL_GUC_UCODE "i915/kbl_guc_ver9_14.bin" > +#define I915_KBL_GUC_UCODE GUC_FW_PATH(kbl, KBL_FW_MAJOR, KBL_FW_MINOR) > MODULE_FIRMWARE(I915_KBL_GUC_UCODE); > > /* User-friendly representation of an enum */ > @@ -697,16 +709,16 @@ void intel_guc_init(struct drm_device *dev) > fw_path = NULL; > } else if (IS_SKYLAKE(dev)) { > fw_path = I915_SKL_GUC_UCODE; > - guc_fw->guc_fw_major_wanted = 6; > - guc_fw->guc_fw_minor_wanted = 1; > + guc_fw->guc_fw_major_wanted = SKL_FW_MAJOR; > + guc_fw->guc_fw_minor_wanted = SKL_FW_MINOR; > } else if (IS_BROXTON(dev)) { > fw_path = I915_BXT_GUC_UCODE; > - guc_fw->guc_fw_major_wanted = 8; > - guc_fw->guc_fw_minor_wanted = 7; > + guc_fw->guc_fw_major_wanted = BXT_FW_MAJOR; > + guc_fw->guc_fw_minor_wanted = BXT_FW_MINOR; > } else if (IS_KABYLAKE(dev)) { > fw_path = I915_KBL_GUC_UCODE; > - guc_fw->guc_fw_major_wanted = 9; > - guc_fw->guc_fw_minor_wanted = 14; > + guc_fw->guc_fw_major_wanted = KBL_FW_MAJOR; > + guc_fw->guc_fw_minor_wanted = KBL_FW_MINOR; > } else { > fw_path = ""; /* unknown device */ > } >
Should we not get the HuC patches merged before we amend them? Peter. -----Original Message----- From: Gordon, David S Sent: Wednesday, August 10, 2016 4:25 PM To: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>; Intel-gfx@lists.freedesktop.org Cc: Ursulin, Tvrtko <tvrtko.ursulin@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>; Antoine, Peter <peter.antoine@intel.com>; Thierry, Michel <michel.thierry@intel.com> Subject: Re: [PATCH v2] drm/i915/guc: Consolidate firmware major-minor to one place On 10/08/16 16:16, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Currently to change the firmware one has to update the exported module > firmware string and the major-minor versions used for verification > after load. Consolidate that to a single place defining correct major > and minor versions per platform. > > v2: Rebased for KBL. > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Dave Gordon <david.s.gordon@intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Cc: Peter Antoine <peter.antoine@intel.com> > Cc: Michel Thierry <michel.thierry@intel.com> > Reviewed-by: Dave Gordon <david.s.gordon@intel.com> (v1) You can carry the R-b over to v2 too :) But wouldn't it be even nicer to unify with the HuC and DMC loaders too? .Dave. > --- > drivers/gpu/drm/i915/intel_guc_loader.c | 30 > +++++++++++++++++++++--------- > 1 file changed, 21 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c > b/drivers/gpu/drm/i915/intel_guc_loader.c > index 3763e30cc165..bfcf6b5d29ad 100644 > --- a/drivers/gpu/drm/i915/intel_guc_loader.c > +++ b/drivers/gpu/drm/i915/intel_guc_loader.c > @@ -59,13 +59,25 @@ > * > */ > > -#define I915_SKL_GUC_UCODE "i915/skl_guc_ver6_1.bin" > +#define SKL_FW_MAJOR 6 > +#define SKL_FW_MINOR 1 > + > +#define BXT_FW_MAJOR 8 > +#define BXT_FW_MINOR 7 > + > +#define KBL_FW_MAJOR 9 > +#define KBL_FW_MINOR 14 > + > +#define GUC_FW_PATH(platform, major, minor) \ > + "i915/" __stringify(platform) "_guc_ver" __stringify(major) "_" __stringify(minor) ".bin" > + > +#define I915_SKL_GUC_UCODE GUC_FW_PATH(skl, SKL_FW_MAJOR, > +SKL_FW_MINOR) > MODULE_FIRMWARE(I915_SKL_GUC_UCODE); > > -#define I915_BXT_GUC_UCODE "i915/bxt_guc_ver8_7.bin" > +#define I915_BXT_GUC_UCODE GUC_FW_PATH(bxt, BXT_FW_MAJOR, > +BXT_FW_MINOR) > MODULE_FIRMWARE(I915_BXT_GUC_UCODE); > > -#define I915_KBL_GUC_UCODE "i915/kbl_guc_ver9_14.bin" > +#define I915_KBL_GUC_UCODE GUC_FW_PATH(kbl, KBL_FW_MAJOR, > +KBL_FW_MINOR) > MODULE_FIRMWARE(I915_KBL_GUC_UCODE); > > /* User-friendly representation of an enum */ @@ -697,16 +709,16 @@ > void intel_guc_init(struct drm_device *dev) > fw_path = NULL; > } else if (IS_SKYLAKE(dev)) { > fw_path = I915_SKL_GUC_UCODE; > - guc_fw->guc_fw_major_wanted = 6; > - guc_fw->guc_fw_minor_wanted = 1; > + guc_fw->guc_fw_major_wanted = SKL_FW_MAJOR; > + guc_fw->guc_fw_minor_wanted = SKL_FW_MINOR; > } else if (IS_BROXTON(dev)) { > fw_path = I915_BXT_GUC_UCODE; > - guc_fw->guc_fw_major_wanted = 8; > - guc_fw->guc_fw_minor_wanted = 7; > + guc_fw->guc_fw_major_wanted = BXT_FW_MAJOR; > + guc_fw->guc_fw_minor_wanted = BXT_FW_MINOR; > } else if (IS_KABYLAKE(dev)) { > fw_path = I915_KBL_GUC_UCODE; > - guc_fw->guc_fw_major_wanted = 9; > - guc_fw->guc_fw_minor_wanted = 14; > + guc_fw->guc_fw_major_wanted = KBL_FW_MAJOR; > + guc_fw->guc_fw_minor_wanted = KBL_FW_MINOR; > } else { > fw_path = ""; /* unknown device */ > } >
On 10/08/16 17:58, Patchwork wrote: > == Series Details == > > Series: drm/i915/guc: Consolidate firmware major-minor to one place (rev2) > URL : https://patchwork.freedesktop.org/series/9318/ > State : warning > > == Summary == > > Series 9318v2 drm/i915/guc: Consolidate firmware major-minor to one place > http://patchwork.freedesktop.org/api/1.0/series/9318/revisions/2/mbox > > Test drv_module_reload_basic: > skip -> PASS (fi-skl-i5-6260u) > Test kms_cursor_legacy: > Subgroup basic-flip-vs-cursor-legacy: > fail -> PASS (fi-hsw-i7-4770k) > fail -> PASS (ro-bdw-i7-5557U) > fail -> PASS (ro-bdw-i5-5250u) > fail -> PASS (ro-skl3-i5-6260u) > Test kms_pipe_crc_basic: > Subgroup suspend-read-crc-pipe-b: > pass -> DMESG-WARN (ro-bdw-i7-5600u) https://bugs.freedesktop.org/show_bug.cgi?id=94992 [ 522.498109] WARNING: CPU: 3 PID: 7786 at drivers/gpu/drm/i915/intel_display.c:13657 intel_atomic_commit_tail+0x10fe/0x1110 [i915] [ 522.498110] pipe A vblank wait timed out > dmesg-warn -> SKIP (ro-bdw-i7-5557U) > Subgroup suspend-read-crc-pipe-c: > skip -> DMESG-WARN (ro-bdw-i5-5250u) https://bugs.freedesktop.org/show_bug.cgi?id=96614 [ 482.345981] [drm:intel_dp_link_training_clock_recovery [i915]] *ERROR* failed to enable link training [ 482.431185] [drm:intel_dp_start_link_train [i915]] *ERROR* failed to start channel equalization > > fi-hsw-i7-4770k total:244 pass:222 dwarn:0 dfail:0 fail:0 skip:22 > fi-skl-i5-6260u total:244 pass:224 dwarn:4 dfail:0 fail:2 skip:14 > fi-skl-i7-6700k total:244 pass:208 dwarn:4 dfail:2 fail:2 skip:28 > fi-snb-i7-2600 total:244 pass:202 dwarn:0 dfail:0 fail:0 skip:42 > ro-bdw-i5-5250u total:240 pass:219 dwarn:2 dfail:0 fail:1 skip:18 > ro-bdw-i7-5557U total:240 pass:220 dwarn:1 dfail:0 fail:0 skip:19 > ro-bdw-i7-5600u total:240 pass:206 dwarn:1 dfail:0 fail:1 skip:32 > ro-bsw-n3050 total:240 pass:194 dwarn:0 dfail:0 fail:4 skip:42 > ro-byt-n2820 total:240 pass:197 dwarn:0 dfail:0 fail:3 skip:40 > ro-hsw-i3-4010u total:240 pass:214 dwarn:0 dfail:0 fail:0 skip:26 > ro-hsw-i7-4770r total:240 pass:214 dwarn:0 dfail:0 fail:0 skip:26 > ro-ilk1-i5-650 total:235 pass:173 dwarn:0 dfail:0 fail:2 skip:60 > ro-ivb-i7-3770 total:240 pass:205 dwarn:0 dfail:0 fail:0 skip:35 > ro-ivb2-i7-3770 total:240 pass:209 dwarn:0 dfail:0 fail:0 skip:31 > ro-skl3-i5-6260u total:240 pass:223 dwarn:0 dfail:0 fail:3 skip:14 > > Results at /archive/results/CI_IGT_test/RO_Patchwork_1824/ > > 3aec82c drm-intel-nightly: 2016y-08m-10d-15h-08m-03s UTC integration manifest > e3c7d5e drm/i915/guc: Consolidate firmware major-minor to one place Merged to dinq, thanks for the review! Regards, Tvrtko
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c index 3763e30cc165..bfcf6b5d29ad 100644 --- a/drivers/gpu/drm/i915/intel_guc_loader.c +++ b/drivers/gpu/drm/i915/intel_guc_loader.c @@ -59,13 +59,25 @@ * */ -#define I915_SKL_GUC_UCODE "i915/skl_guc_ver6_1.bin" +#define SKL_FW_MAJOR 6 +#define SKL_FW_MINOR 1 + +#define BXT_FW_MAJOR 8 +#define BXT_FW_MINOR 7 + +#define KBL_FW_MAJOR 9 +#define KBL_FW_MINOR 14 + +#define GUC_FW_PATH(platform, major, minor) \ + "i915/" __stringify(platform) "_guc_ver" __stringify(major) "_" __stringify(minor) ".bin" + +#define I915_SKL_GUC_UCODE GUC_FW_PATH(skl, SKL_FW_MAJOR, SKL_FW_MINOR) MODULE_FIRMWARE(I915_SKL_GUC_UCODE); -#define I915_BXT_GUC_UCODE "i915/bxt_guc_ver8_7.bin" +#define I915_BXT_GUC_UCODE GUC_FW_PATH(bxt, BXT_FW_MAJOR, BXT_FW_MINOR) MODULE_FIRMWARE(I915_BXT_GUC_UCODE); -#define I915_KBL_GUC_UCODE "i915/kbl_guc_ver9_14.bin" +#define I915_KBL_GUC_UCODE GUC_FW_PATH(kbl, KBL_FW_MAJOR, KBL_FW_MINOR) MODULE_FIRMWARE(I915_KBL_GUC_UCODE); /* User-friendly representation of an enum */ @@ -697,16 +709,16 @@ void intel_guc_init(struct drm_device *dev) fw_path = NULL; } else if (IS_SKYLAKE(dev)) { fw_path = I915_SKL_GUC_UCODE; - guc_fw->guc_fw_major_wanted = 6; - guc_fw->guc_fw_minor_wanted = 1; + guc_fw->guc_fw_major_wanted = SKL_FW_MAJOR; + guc_fw->guc_fw_minor_wanted = SKL_FW_MINOR; } else if (IS_BROXTON(dev)) { fw_path = I915_BXT_GUC_UCODE; - guc_fw->guc_fw_major_wanted = 8; - guc_fw->guc_fw_minor_wanted = 7; + guc_fw->guc_fw_major_wanted = BXT_FW_MAJOR; + guc_fw->guc_fw_minor_wanted = BXT_FW_MINOR; } else if (IS_KABYLAKE(dev)) { fw_path = I915_KBL_GUC_UCODE; - guc_fw->guc_fw_major_wanted = 9; - guc_fw->guc_fw_minor_wanted = 14; + guc_fw->guc_fw_major_wanted = KBL_FW_MAJOR; + guc_fw->guc_fw_minor_wanted = KBL_FW_MINOR; } else { fw_path = ""; /* unknown device */ }