Re: [Intel-gfx] [PATCH] drm/i915/glk: Disable Guc and HuC on GLK
On Tue, Mar 13, 2018 at 09:46:32AM +0100, Greg KH wrote: > On Mon, Mar 12, 2018 at 03:34:57PM -0700, Rodrigo Vivi wrote: > > From: Anusha Srivatsa > > > > commit 2afba81c7909ac259720c0d3e7616cf54d4a5368 upstream. > > > > Since the firmwares are not yet released to public repo, > > disable them on Geminilake. > > > > v2: Remove the firmware versions (Michal) > > > > v3: Remove unwanted defines (Rodrigo) > > Correct commit message (Michal) > > > > Cc: Michal Wajdeczko > > Cc: Rodrigo Vivi > > Cc: > > Signed-off-by: Anusha Srivatsa > > Fixes: 90f192c8241e ("drm/i915/GuC/GLK: Load GuC on GLK") > > Fixes: db5ba0d8931e ("drm/i915/GLK/HuC: Load HuC on GLK") > > Reviewed-by: Michal Wajdeczko > > Signed-off-by: Rodrigo Vivi > > Link: > > https://patchwork.freedesktop.org/patch/msgid/1515006225-13003-1-git-send-email-anusha.sriva...@intel.com > > (cherry picked from commit a76050a4837860fcadb6ca11d69d41e08f4090d8) > > Signed-off-by: Rodrigo Vivi > > --- > > drivers/gpu/drm/i915/intel_guc_fw.c | 9 - > > drivers/gpu/drm/i915/intel_huc.c| 11 --- > > 2 files changed, 20 deletions(-) > > What stable kernel tree(s) is this for? I'm sorry, I assumed that the in-reply-to for them individually would be enough. I will resent both mentioning the version along with commit 2afba81c7909ac259720c0d3e7616cf54d4a5368 upstream. > > thanks, > > greg k-h > ___ > 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
Re: [Intel-gfx] [PATCH] drm/i915/glk: Disable Guc and HuC on GLK
On Mon, Mar 12, 2018 at 03:34:57PM -0700, Rodrigo Vivi wrote: > From: Anusha Srivatsa > > commit 2afba81c7909ac259720c0d3e7616cf54d4a5368 upstream. > > Since the firmwares are not yet released to public repo, > disable them on Geminilake. > > v2: Remove the firmware versions (Michal) > > v3: Remove unwanted defines (Rodrigo) > Correct commit message (Michal) > > Cc: Michal Wajdeczko > Cc: Rodrigo Vivi > Cc: > Signed-off-by: Anusha Srivatsa > Fixes: 90f192c8241e ("drm/i915/GuC/GLK: Load GuC on GLK") > Fixes: db5ba0d8931e ("drm/i915/GLK/HuC: Load HuC on GLK") > Reviewed-by: Michal Wajdeczko > Signed-off-by: Rodrigo Vivi > Link: > https://patchwork.freedesktop.org/patch/msgid/1515006225-13003-1-git-send-email-anusha.sriva...@intel.com > (cherry picked from commit a76050a4837860fcadb6ca11d69d41e08f4090d8) > Signed-off-by: Rodrigo Vivi > --- > drivers/gpu/drm/i915/intel_guc_fw.c | 9 - > drivers/gpu/drm/i915/intel_huc.c| 11 --- > 2 files changed, 20 deletions(-) What stable kernel tree(s) is this for? thanks, greg k-h ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/glk: Disable Guc and HuC on GLK
On Wed, Jan 03, 2018 at 07:03:45PM +, Anusha Srivatsa wrote: > Since the firmwares are not yet released to public repo, > disable them on Geminilake. > > v2: Remove the firmware versions (Michal) > > v3: Remove unwanted defines (Rodrigo) > Correct commit message (Michal) > > Cc: Michal Wajdeczko > Cc: Rodrigo Vivi > Cc: > Signed-off-by: Anusha Srivatsa > Fixes: 90f192c8241e ("drm/i915/GuC/GLK: Load GuC on GLK") > Fixes: db5ba0d8931e ("drm/i915/GLK/HuC: Load HuC on GLK") > Reviewed-by: Michal Wajdeczko Now merged. Thanks for the patch. > --- > drivers/gpu/drm/i915/intel_guc_fw.c | 9 - > drivers/gpu/drm/i915/intel_huc.c| 11 --- > 2 files changed, 20 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c > b/drivers/gpu/drm/i915/intel_guc_fw.c > index cbc51c9..3b09329 100644 > --- a/drivers/gpu/drm/i915/intel_guc_fw.c > +++ b/drivers/gpu/drm/i915/intel_guc_fw.c > @@ -39,9 +39,6 @@ > #define KBL_FW_MAJOR 9 > #define KBL_FW_MINOR 39 > > -#define GLK_FW_MAJOR 10 > -#define GLK_FW_MINOR 56 > - > #define GUC_FW_PATH(platform, major, minor) \ > "i915/" __stringify(platform) "_guc_ver" __stringify(major) "_" > __stringify(minor) ".bin" > > @@ -54,8 +51,6 @@ MODULE_FIRMWARE(I915_BXT_GUC_UCODE); > #define I915_KBL_GUC_UCODE GUC_FW_PATH(kbl, KBL_FW_MAJOR, KBL_FW_MINOR) > MODULE_FIRMWARE(I915_KBL_GUC_UCODE); > > -#define I915_GLK_GUC_UCODE GUC_FW_PATH(glk, GLK_FW_MAJOR, GLK_FW_MINOR) > - > static void guc_fw_select(struct intel_uc_fw *guc_fw) > { > struct intel_guc *guc = container_of(guc_fw, struct intel_guc, fw); > @@ -82,10 +77,6 @@ static void guc_fw_select(struct intel_uc_fw *guc_fw) > guc_fw->path = I915_KBL_GUC_UCODE; > guc_fw->major_ver_wanted = KBL_FW_MAJOR; > guc_fw->minor_ver_wanted = KBL_FW_MINOR; > - } else if (IS_GEMINILAKE(dev_priv)) { > - guc_fw->path = I915_GLK_GUC_UCODE; > - guc_fw->major_ver_wanted = GLK_FW_MAJOR; > - guc_fw->minor_ver_wanted = GLK_FW_MINOR; > } else { > DRM_WARN("%s: No firmware known for this platform!\n", >intel_uc_fw_type_repr(guc_fw->type)); > diff --git a/drivers/gpu/drm/i915/intel_huc.c > b/drivers/gpu/drm/i915/intel_huc.c > index 974be3d..8ed0518 100644 > --- a/drivers/gpu/drm/i915/intel_huc.c > +++ b/drivers/gpu/drm/i915/intel_huc.c > @@ -54,10 +54,6 @@ > #define KBL_HUC_FW_MINOR 00 > #define KBL_BLD_NUM 1810 > > -#define GLK_HUC_FW_MAJOR 02 > -#define GLK_HUC_FW_MINOR 00 > -#define GLK_BLD_NUM 1748 > - > #define HUC_FW_PATH(platform, major, minor, bld_num) \ > "i915/" __stringify(platform) "_huc_ver" __stringify(major) "_" \ > __stringify(minor) "_" __stringify(bld_num) ".bin" > @@ -74,9 +70,6 @@ MODULE_FIRMWARE(I915_BXT_HUC_UCODE); > KBL_HUC_FW_MINOR, KBL_BLD_NUM) > MODULE_FIRMWARE(I915_KBL_HUC_UCODE); > > -#define I915_GLK_HUC_UCODE HUC_FW_PATH(glk, GLK_HUC_FW_MAJOR, \ > - GLK_HUC_FW_MINOR, GLK_BLD_NUM) > - > static void huc_fw_select(struct intel_uc_fw *huc_fw) > { > struct intel_huc *huc = container_of(huc_fw, struct intel_huc, fw); > @@ -103,10 +96,6 @@ static void huc_fw_select(struct intel_uc_fw *huc_fw) > huc_fw->path = I915_KBL_HUC_UCODE; > huc_fw->major_ver_wanted = KBL_HUC_FW_MAJOR; > huc_fw->minor_ver_wanted = KBL_HUC_FW_MINOR; > - } else if (IS_GEMINILAKE(dev_priv)) { > - huc_fw->path = I915_GLK_HUC_UCODE; > - huc_fw->major_ver_wanted = GLK_HUC_FW_MAJOR; > - huc_fw->minor_ver_wanted = GLK_HUC_FW_MINOR; > } else { > DRM_WARN("%s: No firmware known for this platform!\n", >intel_uc_fw_type_repr(huc_fw->type)); > -- > 2.7.4 > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/glk: Disable Guc and HuC on GLK
>-Original Message- >From: Wajdeczko, Michal >Sent: Wednesday, December 27, 2017 12:53 PM >To: intel-gfx@lists.freedesktop.org; Srivatsa, Anusha > >Cc: Vivi, Rodrigo >Subject: Re: [PATCH] drm/i915/glk: Disable Guc and HuC on GLK > >On Sat, 23 Dec 2017 01:05:14 +0100, Anusha Srivatsa > wrote: > >> Since the firmwares are released yet to public repo, > ^^^ >s/released/not released Oooops Thanks a lot for the catch. Anusha >> disable them on Geminilake. >> >> v2: Remove the firmware versions (Michal) >> >> Cc: Michal Wajdeczko >> Cc: Rodrigo Vivi >> Signed-off-by: Anusha Srivatsa >> --- >Reviewed-by: Michal Wajdeczko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/glk: Disable Guc and HuC on GLK
>-Original Message- >From: Vivi, Rodrigo >Sent: Thursday, December 28, 2017 8:08 AM >To: Srivatsa, Anusha >Cc: intel-gfx@lists.freedesktop.org; Wajdeczko, Michal > >Subject: Re: [PATCH] drm/i915/glk: Disable Guc and HuC on GLK > >On Sat, Dec 23, 2017 at 12:05:14AM +, Anusha Srivatsa wrote: >> Since the firmwares are released yet to public repo, disable them on >> Geminilake. >> >> v2: Remove the firmware versions (Michal) >> >> Cc: Michal Wajdeczko >> Cc: Rodrigo Vivi >> Signed-off-by: Anusha Srivatsa > >Fixes: 90f192c8241e ("drm/i915/GuC/GLK: Load GuC on GLK") >Fixes: db5ba0d8931e ("drm/i915/GLK/HuC: Load HuC on GLK") >Cc: # v4.13+ > >What about a revert on those 2 patches? Revert touches source files which no longer I would prefer having this patch instead. >I'm fine with one patch approach, but the fixes tag needs to be here anyways. Yes, thanks for pointing the suitable tags. >> --- >> drivers/gpu/drm/i915/intel_guc_fw.c | 4 >> drivers/gpu/drm/i915/intel_huc.c| 4 >> 2 files changed, 8 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c >> b/drivers/gpu/drm/i915/intel_guc_fw.c >> index cbc51c9..252b475 100644 >> --- a/drivers/gpu/drm/i915/intel_guc_fw.c >> +++ b/drivers/gpu/drm/i915/intel_guc_fw.c >> @@ -82,10 +82,6 @@ static void guc_fw_select(struct intel_uc_fw *guc_fw) >> guc_fw->path = I915_KBL_GUC_UCODE; >> guc_fw->major_ver_wanted = KBL_FW_MAJOR; >> guc_fw->minor_ver_wanted = KBL_FW_MINOR; >> -} else if (IS_GEMINILAKE(dev_priv)) { >> -guc_fw->path = I915_GLK_GUC_UCODE; >> -guc_fw->major_ver_wanted = GLK_FW_MAJOR; >> -guc_fw->minor_ver_wanted = GLK_FW_MINOR; > >But also, please it is mandatory to also remove the now unused >I915_GLK_GUC_UCODE, GLK_FW_MAJOR, and GLK_FW_MINOR Will change in next revision. Thanks a lot for the feedback. >> } else { >> DRM_WARN("%s: No firmware known for this platform!\n", >> intel_uc_fw_type_repr(guc_fw->type)); >> diff --git a/drivers/gpu/drm/i915/intel_huc.c >> b/drivers/gpu/drm/i915/intel_huc.c >> index 974be3d..3f28ae0 100644 >> --- a/drivers/gpu/drm/i915/intel_huc.c >> +++ b/drivers/gpu/drm/i915/intel_huc.c >> @@ -103,10 +103,6 @@ static void huc_fw_select(struct intel_uc_fw *huc_fw) >> huc_fw->path = I915_KBL_HUC_UCODE; >> huc_fw->major_ver_wanted = KBL_HUC_FW_MAJOR; >> huc_fw->minor_ver_wanted = KBL_HUC_FW_MINOR; >> -} else if (IS_GEMINILAKE(dev_priv)) { >> -huc_fw->path = I915_GLK_HUC_UCODE; >> -huc_fw->major_ver_wanted = GLK_HUC_FW_MAJOR; >> -huc_fw->minor_ver_wanted = GLK_HUC_FW_MINOR; > >and unused I915_GLK_HUC_UCODE, GLK_HUC_FW_MAJOR, and >GLK_HUC_FW_MINOR. Sure. BR, Anusha >> } else { >> DRM_WARN("%s: No firmware known for this platform!\n", >> intel_uc_fw_type_repr(huc_fw->type)); >> -- >> 2.7.4 >> ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/glk: Disable Guc and HuC on GLK
On Sat, Dec 23, 2017 at 12:05:14AM +, Anusha Srivatsa wrote: > Since the firmwares are released yet to public repo, > disable them on Geminilake. > > v2: Remove the firmware versions (Michal) > > Cc: Michal Wajdeczko > Cc: Rodrigo Vivi > Signed-off-by: Anusha Srivatsa Fixes: 90f192c8241e ("drm/i915/GuC/GLK: Load GuC on GLK") Fixes: db5ba0d8931e ("drm/i915/GLK/HuC: Load HuC on GLK") Cc: # v4.13+ What about a revert on those 2 patches? I'm fine with one patch approach, but the fixes tag needs to be here anyways. > --- > drivers/gpu/drm/i915/intel_guc_fw.c | 4 > drivers/gpu/drm/i915/intel_huc.c| 4 > 2 files changed, 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c > b/drivers/gpu/drm/i915/intel_guc_fw.c > index cbc51c9..252b475 100644 > --- a/drivers/gpu/drm/i915/intel_guc_fw.c > +++ b/drivers/gpu/drm/i915/intel_guc_fw.c > @@ -82,10 +82,6 @@ static void guc_fw_select(struct intel_uc_fw *guc_fw) > guc_fw->path = I915_KBL_GUC_UCODE; > guc_fw->major_ver_wanted = KBL_FW_MAJOR; > guc_fw->minor_ver_wanted = KBL_FW_MINOR; > - } else if (IS_GEMINILAKE(dev_priv)) { > - guc_fw->path = I915_GLK_GUC_UCODE; > - guc_fw->major_ver_wanted = GLK_FW_MAJOR; > - guc_fw->minor_ver_wanted = GLK_FW_MINOR; But also, please it is mandatory to also remove the now unused I915_GLK_GUC_UCODE, GLK_FW_MAJOR, and GLK_FW_MINOR > } else { > DRM_WARN("%s: No firmware known for this platform!\n", >intel_uc_fw_type_repr(guc_fw->type)); > diff --git a/drivers/gpu/drm/i915/intel_huc.c > b/drivers/gpu/drm/i915/intel_huc.c > index 974be3d..3f28ae0 100644 > --- a/drivers/gpu/drm/i915/intel_huc.c > +++ b/drivers/gpu/drm/i915/intel_huc.c > @@ -103,10 +103,6 @@ static void huc_fw_select(struct intel_uc_fw *huc_fw) > huc_fw->path = I915_KBL_HUC_UCODE; > huc_fw->major_ver_wanted = KBL_HUC_FW_MAJOR; > huc_fw->minor_ver_wanted = KBL_HUC_FW_MINOR; > - } else if (IS_GEMINILAKE(dev_priv)) { > - huc_fw->path = I915_GLK_HUC_UCODE; > - huc_fw->major_ver_wanted = GLK_HUC_FW_MAJOR; > - huc_fw->minor_ver_wanted = GLK_HUC_FW_MINOR; and unused I915_GLK_HUC_UCODE, GLK_HUC_FW_MAJOR, and GLK_HUC_FW_MINOR. > } else { > DRM_WARN("%s: No firmware known for this platform!\n", >intel_uc_fw_type_repr(huc_fw->type)); > -- > 2.7.4 > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/glk: Disable Guc and HuC on GLK
On Sat, 23 Dec 2017 01:05:14 +0100, Anusha Srivatsa wrote: Since the firmwares are released yet to public repo, ^^^ s/released/not released disable them on Geminilake. v2: Remove the firmware versions (Michal) Cc: Michal Wajdeczko Cc: Rodrigo Vivi Signed-off-by: Anusha Srivatsa --- Reviewed-by: Michal Wajdeczko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/glk: Disable Guc and HuC on GLK
On Mon, 18 Dec 2017 20:25:17 +0100, Srivatsa, Anusha wrote: -Original Message- From: Vivi, Rodrigo Sent: Friday, December 15, 2017 3:03 PM To: Wajdeczko, Michal Cc: intel-gfx@lists.freedesktop.org; Srivatsa, Anusha Subject: Re: [Intel-gfx] [PATCH] drm/i915/glk: Disable Guc and HuC on GLK On Fri, Dec 15, 2017 at 06:07:27AM +, Michal Wajdeczko wrote: On Thu, 14 Dec 2017 23:30:56 +0100, Srivatsa, Anusha wrote: > > > > -Original Message- > > From: Wajdeczko, Michal > > Sent: Thursday, December 14, 2017 2:18 PM > > To: intel-gfx@lists.freedesktop.org; Srivatsa, Anusha > > > > Cc: Vivi, Rodrigo > > Subject: Re: [Intel-gfx] [PATCH] drm/i915/glk: Disable Guc and HuC > > on GLK > > > > On Thu, 14 Dec 2017 22:58:37 +0100, Anusha Srivatsa > > wrote: > > > > > Since the firmwares are released yet to public repo, disable > > > them on Geminilake. > > > > > > Cc: Rodrigo Vivi > > > Signed-off-by: Anusha Srivatsa > > > --- > > > drivers/gpu/drm/i915/i915_pci.c | 5 + > > > 1 file changed, 5 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_pci.c > > > b/drivers/gpu/drm/i915/i915_pci.c index fa67d3d..ddf7530 100644 > > > --- a/drivers/gpu/drm/i915/i915_pci.c > > > +++ b/drivers/gpu/drm/i915/i915_pci.c > > > @@ -521,6 +521,11 @@ static const struct intel_device_info > > > intel_geminilake_info __initconst = { > > > GEN9_LP_FEATURES, > > > .platform = INTEL_GEMINILAKE, > > > .ddb_size = 1024, > > > +/* FIXME Geminilake supports GuC but currently firmwares > > > + * have not made it to public repo. Lets disable the support > > > + * as a temporary fix. > > > + */ > > > +.has_guc = 0, > > > > Maybe better place to put this fix is __get_platform_enable_guc() > > like in [1] [1] https://patchwork.freedesktop.org/patch/192006/ > > Michal, > Hmm the reference patch is controlling guc/huc through a module > parameter but wont the above approach be neater platform wise? With your patch it would be impossible to check even preliminary firmwares from non-public repo without recompilation of the driver. While with variant below it would just require changing modparams like: enable_guc=1 guc_firmware_path=preliminary/glk.bin This is a fair point imo. With has_guc=0 you remove the possibility of testing preliminary GuC/HuC without a kernel patch... Note that auto mode (enable_guc=-1) will still keep GuC disabled for GLK. ... but I believe that the main point here is that if we don't have a public image it doesn't exist from the upstream perspective. Correct The ideal is not need this patch and releasing firmware sooner. I see same happening for all upcoming platforms... So, whatever we decide for this case needs to cover future cases as well. I agree. While both the places discussed is good to make this change, I am inclining towards making at the platform definition level. I do need more feedback on this since we want that to be the norm moving forward. There is a yet another option for now: remove GuC/Huc firmware names. } else if (IS_GEMINILAKE(dev_priv)) { - guc_fw->path = I915_GLK_GUC_UCODE; - guc_fw->major_ver_wanted = GLK_FW_MAJOR; - guc_fw->minor_ver_wanted = GLK_FW_MINOR; } else { And the norm for the future: do not define GuC/HuC firmware names without having those images available in public. Michal Anusha Thanks, Rodrigo. Michal > > Anusha > > diff --git a/drivers/gpu/drm/i915/intel_uc.c > > b/drivers/gpu/drm/i915/intel_uc.c index 49bccc9..22b0afe 100644 > > --- a/drivers/gpu/drm/i915/intel_uc.c > > +++ b/drivers/gpu/drm/i915/intel_uc.c > > @@ -60,6 +60,8 @@ static int __get_platform_enable_guc(struct > > drm_i915_private *dev_priv) > > enable_guc |= ENABLE_GUC_LOAD_HUC; > > > > /* Any platform specific fine-tuning can be done here */ > > + if (IS_GEMINILAKE(dev_priv)) > > + enable_guc = 0; /* no firmware on CI machines */ > > > > return enable_guc; > > } > > > > > > > GLK_COLORS, > > > }; ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/glk: Disable Guc and HuC on GLK
>-Original Message- >From: Vivi, Rodrigo >Sent: Friday, December 15, 2017 3:03 PM >To: Wajdeczko, Michal >Cc: intel-gfx@lists.freedesktop.org; Srivatsa, Anusha > >Subject: Re: [Intel-gfx] [PATCH] drm/i915/glk: Disable Guc and HuC on GLK > >On Fri, Dec 15, 2017 at 06:07:27AM +, Michal Wajdeczko wrote: >> On Thu, 14 Dec 2017 23:30:56 +0100, Srivatsa, Anusha >> wrote: >> >> > >> > >> > > -Original Message- >> > > From: Wajdeczko, Michal >> > > Sent: Thursday, December 14, 2017 2:18 PM >> > > To: intel-gfx@lists.freedesktop.org; Srivatsa, Anusha >> > > >> > > Cc: Vivi, Rodrigo >> > > Subject: Re: [Intel-gfx] [PATCH] drm/i915/glk: Disable Guc and HuC >> > > on GLK >> > > >> > > On Thu, 14 Dec 2017 22:58:37 +0100, Anusha Srivatsa >> > > wrote: >> > > >> > > > Since the firmwares are released yet to public repo, disable >> > > > them on Geminilake. >> > > > >> > > > Cc: Rodrigo Vivi >> > > > Signed-off-by: Anusha Srivatsa >> > > > --- >> > > > drivers/gpu/drm/i915/i915_pci.c | 5 + >> > > > 1 file changed, 5 insertions(+) >> > > > >> > > > diff --git a/drivers/gpu/drm/i915/i915_pci.c >> > > > b/drivers/gpu/drm/i915/i915_pci.c index fa67d3d..ddf7530 100644 >> > > > --- a/drivers/gpu/drm/i915/i915_pci.c >> > > > +++ b/drivers/gpu/drm/i915/i915_pci.c >> > > > @@ -521,6 +521,11 @@ static const struct intel_device_info >> > > > intel_geminilake_info __initconst = { >> > > >GEN9_LP_FEATURES, >> > > >.platform = INTEL_GEMINILAKE, >> > > >.ddb_size = 1024, >> > > > + /* FIXME Geminilake supports GuC but currently firmwares >> > > > + * have not made it to public repo. Lets disable the support >> > > > + * as a temporary fix. >> > > > + */ >> > > > + .has_guc = 0, >> > > >> > > Maybe better place to put this fix is __get_platform_enable_guc() >> > > like in [1] [1] https://patchwork.freedesktop.org/patch/192006/ >> > >> > Michal, >> > Hmm the reference patch is controlling guc/huc through a module >> > parameter but wont the above approach be neater platform wise? >> >> With your patch it would be impossible to check even preliminary >> firmwares from non-public repo without recompilation of the driver. >> >> While with variant below it would just require changing modparams like: >> enable_guc=1 >> guc_firmware_path=preliminary/glk.bin > >This is a fair point imo. With has_guc=0 you remove the possibility of testing >preliminary GuC/HuC without a kernel patch... > >> >> Note that auto mode (enable_guc=-1) will still keep GuC disabled for GLK. > >... but I believe that the main point here is that if we don't have a public >image it >doesn't exist from the upstream perspective. Correct >The ideal is not need this patch and releasing firmware sooner. I see same >happening for all upcoming platforms... > >So, whatever we decide for this case needs to cover future cases as well. I agree. While both the places discussed is good to make this change, I am inclining towards making at the platform definition level. I do need more feedback on this since we want that to be the norm moving forward. Anusha >Thanks, >Rodrigo. > >> >> Michal >> >> > >> > Anusha >> > > diff --git a/drivers/gpu/drm/i915/intel_uc.c >> > > b/drivers/gpu/drm/i915/intel_uc.c index 49bccc9..22b0afe 100644 >> > > --- a/drivers/gpu/drm/i915/intel_uc.c >> > > +++ b/drivers/gpu/drm/i915/intel_uc.c >> > > @@ -60,6 +60,8 @@ static int __get_platform_enable_guc(struct >> > > drm_i915_private *dev_priv) >> > > enable_guc |= ENABLE_GUC_LOAD_HUC; >> > > >> > > /* Any platform specific fine-tuning can be done here */ >> > > +if (IS_GEMINILAKE(dev_priv)) >> > > +enable_guc = 0; /* no firmware on CI machines */ >> > > >> > > return enable_guc; >> > > } >> > > >> > > >> > > >GLK_COLORS, >> > > > }; ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/glk: Disable Guc and HuC on GLK
On Fri, Dec 15, 2017 at 06:07:27AM +, Michal Wajdeczko wrote: > On Thu, 14 Dec 2017 23:30:56 +0100, Srivatsa, Anusha > wrote: > > > > > > > > -Original Message- > > > From: Wajdeczko, Michal > > > Sent: Thursday, December 14, 2017 2:18 PM > > > To: intel-gfx@lists.freedesktop.org; Srivatsa, Anusha > > > > > > Cc: Vivi, Rodrigo > > > Subject: Re: [Intel-gfx] [PATCH] drm/i915/glk: Disable Guc and HuC > > > on GLK > > > > > > On Thu, 14 Dec 2017 22:58:37 +0100, Anusha Srivatsa > > > wrote: > > > > > > > Since the firmwares are released yet to public repo, disable them on > > > > Geminilake. > > > > > > > > Cc: Rodrigo Vivi > > > > Signed-off-by: Anusha Srivatsa > > > > --- > > > > drivers/gpu/drm/i915/i915_pci.c | 5 + > > > > 1 file changed, 5 insertions(+) > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_pci.c > > > > b/drivers/gpu/drm/i915/i915_pci.c index fa67d3d..ddf7530 100644 > > > > --- a/drivers/gpu/drm/i915/i915_pci.c > > > > +++ b/drivers/gpu/drm/i915/i915_pci.c > > > > @@ -521,6 +521,11 @@ static const struct intel_device_info > > > > intel_geminilake_info __initconst = { > > > > GEN9_LP_FEATURES, > > > > .platform = INTEL_GEMINILAKE, > > > > .ddb_size = 1024, > > > > + /* FIXME Geminilake supports GuC but currently firmwares > > > > +* have not made it to public repo. Lets disable the support > > > > +* as a temporary fix. > > > > +*/ > > > > + .has_guc = 0, > > > > > > Maybe better place to put this fix is __get_platform_enable_guc() > > > like in [1] [1] > > > https://patchwork.freedesktop.org/patch/192006/ > > > > Michal, > > Hmm the reference patch is controlling guc/huc through a module > > parameter but wont the above approach be neater platform wise? > > With your patch it would be impossible to check even preliminary > firmwares from non-public repo without recompilation of the driver. > > While with variant below it would just require changing modparams like: > enable_guc=1 > guc_firmware_path=preliminary/glk.bin This is a fair point imo. With has_guc=0 you remove the possibility of testing preliminary GuC/HuC without a kernel patch... > > Note that auto mode (enable_guc=-1) will still keep GuC disabled for GLK. ... but I believe that the main point here is that if we don't have a public image it doesn't exist from the upstream perspective. The ideal is not need this patch and releasing firmware sooner. I see same happening for all upcoming platforms... So, whatever we decide for this case needs to cover future cases as well. Thanks, Rodrigo. > > Michal > > > > > Anusha > > > diff --git a/drivers/gpu/drm/i915/intel_uc.c > > > b/drivers/gpu/drm/i915/intel_uc.c > > > index 49bccc9..22b0afe 100644 > > > --- a/drivers/gpu/drm/i915/intel_uc.c > > > +++ b/drivers/gpu/drm/i915/intel_uc.c > > > @@ -60,6 +60,8 @@ static int __get_platform_enable_guc(struct > > > drm_i915_private *dev_priv) > > > enable_guc |= ENABLE_GUC_LOAD_HUC; > > > > > > /* Any platform specific fine-tuning can be done here */ > > > + if (IS_GEMINILAKE(dev_priv)) > > > + enable_guc = 0; /* no firmware on CI machines */ > > > > > > return enable_guc; > > > } > > > > > > > > > > GLK_COLORS, > > > > }; ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/glk: Disable Guc and HuC on GLK
On Thu, 14 Dec 2017 23:30:56 +0100, Srivatsa, Anusha wrote: -Original Message- From: Wajdeczko, Michal Sent: Thursday, December 14, 2017 2:18 PM To: intel-gfx@lists.freedesktop.org; Srivatsa, Anusha Cc: Vivi, Rodrigo Subject: Re: [Intel-gfx] [PATCH] drm/i915/glk: Disable Guc and HuC on GLK On Thu, 14 Dec 2017 22:58:37 +0100, Anusha Srivatsa wrote: Since the firmwares are released yet to public repo, disable them on Geminilake. Cc: Rodrigo Vivi Signed-off-by: Anusha Srivatsa --- drivers/gpu/drm/i915/i915_pci.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index fa67d3d..ddf7530 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -521,6 +521,11 @@ static const struct intel_device_info intel_geminilake_info __initconst = { GEN9_LP_FEATURES, .platform = INTEL_GEMINILAKE, .ddb_size = 1024, + /* FIXME Geminilake supports GuC but currently firmwares +* have not made it to public repo. Lets disable the support +* as a temporary fix. +*/ + .has_guc = 0, Maybe better place to put this fix is __get_platform_enable_guc() like in [1] [1] https://patchwork.freedesktop.org/patch/192006/ Michal, Hmm the reference patch is controlling guc/huc through a module parameter but wont the above approach be neater platform wise? With your patch it would be impossible to check even preliminary firmwares from non-public repo without recompilation of the driver. While with variant below it would just require changing modparams like: enable_guc=1 guc_firmware_path=preliminary/glk.bin Note that auto mode (enable_guc=-1) will still keep GuC disabled for GLK. Michal Anusha diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c index 49bccc9..22b0afe 100644 --- a/drivers/gpu/drm/i915/intel_uc.c +++ b/drivers/gpu/drm/i915/intel_uc.c @@ -60,6 +60,8 @@ static int __get_platform_enable_guc(struct drm_i915_private *dev_priv) enable_guc |= ENABLE_GUC_LOAD_HUC; /* Any platform specific fine-tuning can be done here */ + if (IS_GEMINILAKE(dev_priv)) + enable_guc = 0; /* no firmware on CI machines */ return enable_guc; } GLK_COLORS, }; ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/glk: Disable Guc and HuC on GLK
>-Original Message- >From: Wajdeczko, Michal >Sent: Thursday, December 14, 2017 2:18 PM >To: intel-gfx@lists.freedesktop.org; Srivatsa, Anusha > >Cc: Vivi, Rodrigo >Subject: Re: [Intel-gfx] [PATCH] drm/i915/glk: Disable Guc and HuC on GLK > >On Thu, 14 Dec 2017 22:58:37 +0100, Anusha Srivatsa > wrote: > >> Since the firmwares are released yet to public repo, disable them on >> Geminilake. >> >> Cc: Rodrigo Vivi >> Signed-off-by: Anusha Srivatsa >> --- >> drivers/gpu/drm/i915/i915_pci.c | 5 + >> 1 file changed, 5 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/i915_pci.c >> b/drivers/gpu/drm/i915/i915_pci.c index fa67d3d..ddf7530 100644 >> --- a/drivers/gpu/drm/i915/i915_pci.c >> +++ b/drivers/gpu/drm/i915/i915_pci.c >> @@ -521,6 +521,11 @@ static const struct intel_device_info >> intel_geminilake_info __initconst = { >> GEN9_LP_FEATURES, >> .platform = INTEL_GEMINILAKE, >> .ddb_size = 1024, >> +/* FIXME Geminilake supports GuC but currently firmwares >> + * have not made it to public repo. Lets disable the support >> + * as a temporary fix. >> + */ >> +.has_guc = 0, > >Maybe better place to put this fix is __get_platform_enable_guc() like in [1] >[1] >https://patchwork.freedesktop.org/patch/192006/ Michal, Hmm the reference patch is controlling guc/huc through a module parameter but wont the above approach be neater platform wise? Anusha >diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c >index 49bccc9..22b0afe 100644 >--- a/drivers/gpu/drm/i915/intel_uc.c >+++ b/drivers/gpu/drm/i915/intel_uc.c >@@ -60,6 +60,8 @@ static int __get_platform_enable_guc(struct >drm_i915_private *dev_priv) > enable_guc |= ENABLE_GUC_LOAD_HUC; > > /* Any platform specific fine-tuning can be done here */ >+ if (IS_GEMINILAKE(dev_priv)) >+ enable_guc = 0; /* no firmware on CI machines */ > > return enable_guc; > } > > >> GLK_COLORS, >> }; ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/glk: Disable Guc and HuC on GLK
On Thu, 14 Dec 2017 22:58:37 +0100, Anusha Srivatsa wrote: Since the firmwares are released yet to public repo, disable them on Geminilake. Cc: Rodrigo Vivi Signed-off-by: Anusha Srivatsa --- drivers/gpu/drm/i915/i915_pci.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index fa67d3d..ddf7530 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -521,6 +521,11 @@ static const struct intel_device_info intel_geminilake_info __initconst = { GEN9_LP_FEATURES, .platform = INTEL_GEMINILAKE, .ddb_size = 1024, + /* FIXME Geminilake supports GuC but currently firmwares +* have not made it to public repo. Lets disable the support +* as a temporary fix. +*/ + .has_guc = 0, Maybe better place to put this fix is __get_platform_enable_guc() like in [1] [1] https://patchwork.freedesktop.org/patch/192006/ diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c index 49bccc9..22b0afe 100644 --- a/drivers/gpu/drm/i915/intel_uc.c +++ b/drivers/gpu/drm/i915/intel_uc.c @@ -60,6 +60,8 @@ static int __get_platform_enable_guc(struct drm_i915_private *dev_priv) enable_guc |= ENABLE_GUC_LOAD_HUC; /* Any platform specific fine-tuning can be done here */ + if (IS_GEMINILAKE(dev_priv)) + enable_guc = 0; /* no firmware on CI machines */ return enable_guc; } GLK_COLORS, }; ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx