| Message ID | 1508865685-7722-5-git-send-email-sujaritha.sundaresan@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 24 Oct 2017 19:21:23 +0200, Sujaritha Sundaresan <sujaritha.sundaresan@intel.com> wrote: > Updating GuC and HuC firmware select function to support removing > i915_modparams.enable_guc_loading module parameter. Hmm, is it correct order of patches ? this modparam was already removed in 2/6 > > v2: Clarifying the commit message (Anusha) > > v3: Unify seq_puts messages, Re-factoring code as per review (Michal) > > v4: Rebase > > v5: Separating message unification into a separate patch > > v6: Re-factoring code (Sagar, Michal) > Rebase > > v7: Separating from previuos patch (Sagar) > Rebase > > v8: Including change to intel_uc.c > Applying review comments (Michal) > > Signed-off-by: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com> > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> > Cc: Oscar Mateo <oscar.mateo@intel.com> > Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com> > --- > drivers/gpu/drm/i915/intel_guc_fw.c | 10 +++------- > drivers/gpu/drm/i915/intel_guc_fw.h | 2 +- > drivers/gpu/drm/i915/intel_huc.c | 3 ++- > drivers/gpu/drm/i915/intel_uc.c | 6 ++++++ > 4 files changed, 12 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c > b/drivers/gpu/drm/i915/intel_guc_fw.c > index ef67a36..b9f834f 100644 > --- a/drivers/gpu/drm/i915/intel_guc_fw.c > +++ b/drivers/gpu/drm/i915/intel_guc_fw.c > @@ -60,10 +60,8 @@ > * intel_guc_fw_select() - selects GuC firmware for uploading > * > * @guc: intel_guc struct > - * > - * Return: zero when we know firmware, non-zero in other case > */ > -int intel_guc_fw_select(struct intel_guc *guc) > +void intel_guc_fw_select(struct intel_guc *guc) > { > struct drm_i915_private *dev_priv = guc_to_i915(guc); > @@ -90,11 +88,9 @@ int intel_guc_fw_select(struct intel_guc *guc) > guc->fw.major_ver_wanted = GLK_FW_MAJOR; > guc->fw.minor_ver_wanted = GLK_FW_MINOR; > } else { > - DRM_ERROR("No GuC firmware known for platform with GuC!\n"); > - return -ENOENT; > + DRM_ERROR("No GuC FW known for platform with GuC!\n"); > + return; > } > - > - return 0; > } > /* > diff --git a/drivers/gpu/drm/i915/intel_guc_fw.h > b/drivers/gpu/drm/i915/intel_guc_fw.h > index 023f5ba..7f6ccaf 100644 > --- a/drivers/gpu/drm/i915/intel_guc_fw.h > +++ b/drivers/gpu/drm/i915/intel_guc_fw.h > @@ -27,7 +27,7 @@ > struct intel_guc; > -int intel_guc_fw_select(struct intel_guc *guc); > +void intel_guc_fw_select(struct intel_guc *guc); > int intel_guc_fw_upload(struct intel_guc *guc); > #endif > diff --git a/drivers/gpu/drm/i915/intel_huc.c > b/drivers/gpu/drm/i915/intel_huc.c > index c8a48cb..4e700ab 100644 > --- a/drivers/gpu/drm/i915/intel_huc.c > +++ b/drivers/gpu/drm/i915/intel_huc.c > @@ -108,7 +108,8 @@ void intel_huc_select_fw(struct intel_huc *huc) > huc->fw.major_ver_wanted = GLK_HUC_FW_MAJOR; > huc->fw.minor_ver_wanted = GLK_HUC_FW_MINOR; > } else { > - DRM_ERROR("No HuC firmware known for platform with HuC!\n"); > + if (HAS_GUC(dev_priv)) > + DRM_ERROR("No HuC FW known for platform with HuC!\n"); > return; > } > } > diff --git a/drivers/gpu/drm/i915/intel_uc.c > b/drivers/gpu/drm/i915/intel_uc.c > index 9369ade..dc978a0 100644 > --- a/drivers/gpu/drm/i915/intel_uc.c > +++ b/drivers/gpu/drm/i915/intel_uc.c > @@ -82,11 +82,17 @@ void intel_uc_sanitize_options(struct > drm_i915_private *dev_priv) > void intel_uc_init_early(struct drm_i915_private *dev_priv) > { > + struct intel_guc *guc = &dev_priv->guc; > + struct intel_huc *huc = &dev_priv->huc; Please run checkpatch and add separation line here > intel_guc_init_early(&dev_priv->guc); You can now pass 'guc' as param > + intel_guc_fw_select(guc); > + intel_huc_select_fw(huc); To avoid redundant checks in select_fw() functions we can exit earlier with if (!HAS_GUC(dev_priv)) return; > } > void intel_uc_init_fw(struct drm_i915_private *dev_priv) > { > + if (!HAS_GUC(dev_priv)) > + return; > intel_uc_fw_fetch(dev_priv, &dev_priv->huc.fw); > intel_uc_fw_fetch(dev_priv, &dev_priv->guc.fw); > }
On 10/25/2017 08:56 AM, Michal Wajdeczko wrote: > On Tue, 24 Oct 2017 19:21:23 +0200, Sujaritha Sundaresan > <sujaritha.sundaresan@intel.com> wrote: > >> Updating GuC and HuC firmware select function to support removing >> i915_modparams.enable_guc_loading module parameter. > > Hmm, is it correct order of patches ? this modparam was already > removed in 2/6 I will move this to be the third patch. Since the change to uc.c was separated from 2/6, I had included that as 3/6. > >> >> v2: Clarifying the commit message (Anusha) >> >> v3: Unify seq_puts messages, Re-factoring code as per review (Michal) >> >> v4: Rebase >> >> v5: Separating message unification into a separate patch >> >> v6: Re-factoring code (Sagar, Michal) >> Rebase >> >> v7: Separating from previuos patch (Sagar) >> Rebase >> >> v8: Including change to intel_uc.c >> Applying review comments (Michal) >> >> Signed-off-by: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com> >> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> >> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> >> Cc: Oscar Mateo <oscar.mateo@intel.com> >> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com> >> --- >> drivers/gpu/drm/i915/intel_guc_fw.c | 10 +++------- >> drivers/gpu/drm/i915/intel_guc_fw.h | 2 +- >> drivers/gpu/drm/i915/intel_huc.c | 3 ++- >> drivers/gpu/drm/i915/intel_uc.c | 6 ++++++ >> 4 files changed, 12 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c >> b/drivers/gpu/drm/i915/intel_guc_fw.c >> index ef67a36..b9f834f 100644 >> --- a/drivers/gpu/drm/i915/intel_guc_fw.c >> +++ b/drivers/gpu/drm/i915/intel_guc_fw.c >> @@ -60,10 +60,8 @@ >> * intel_guc_fw_select() - selects GuC firmware for uploading >> * >> * @guc: intel_guc struct >> - * >> - * Return: zero when we know firmware, non-zero in other case >> */ >> -int intel_guc_fw_select(struct intel_guc *guc) >> +void intel_guc_fw_select(struct intel_guc *guc) >> { >> struct drm_i915_private *dev_priv = guc_to_i915(guc); >> @@ -90,11 +88,9 @@ int intel_guc_fw_select(struct intel_guc *guc) >> guc->fw.major_ver_wanted = GLK_FW_MAJOR; >> guc->fw.minor_ver_wanted = GLK_FW_MINOR; >> } else { >> - DRM_ERROR("No GuC firmware known for platform with GuC!\n"); >> - return -ENOENT; >> + DRM_ERROR("No GuC FW known for platform with GuC!\n"); >> + return; >> } >> - >> - return 0; >> } >> /* >> diff --git a/drivers/gpu/drm/i915/intel_guc_fw.h >> b/drivers/gpu/drm/i915/intel_guc_fw.h >> index 023f5ba..7f6ccaf 100644 >> --- a/drivers/gpu/drm/i915/intel_guc_fw.h >> +++ b/drivers/gpu/drm/i915/intel_guc_fw.h >> @@ -27,7 +27,7 @@ >> struct intel_guc; >> -int intel_guc_fw_select(struct intel_guc *guc); >> +void intel_guc_fw_select(struct intel_guc *guc); >> int intel_guc_fw_upload(struct intel_guc *guc); >> #endif >> diff --git a/drivers/gpu/drm/i915/intel_huc.c >> b/drivers/gpu/drm/i915/intel_huc.c >> index c8a48cb..4e700ab 100644 >> --- a/drivers/gpu/drm/i915/intel_huc.c >> +++ b/drivers/gpu/drm/i915/intel_huc.c >> @@ -108,7 +108,8 @@ void intel_huc_select_fw(struct intel_huc *huc) >> huc->fw.major_ver_wanted = GLK_HUC_FW_MAJOR; >> huc->fw.minor_ver_wanted = GLK_HUC_FW_MINOR; >> } else { >> - DRM_ERROR("No HuC firmware known for platform with HuC!\n"); >> + if (HAS_GUC(dev_priv)) >> + DRM_ERROR("No HuC FW known for platform with HuC!\n"); >> return; >> } >> } >> diff --git a/drivers/gpu/drm/i915/intel_uc.c >> b/drivers/gpu/drm/i915/intel_uc.c >> index 9369ade..dc978a0 100644 >> --- a/drivers/gpu/drm/i915/intel_uc.c >> +++ b/drivers/gpu/drm/i915/intel_uc.c >> @@ -82,11 +82,17 @@ void intel_uc_sanitize_options(struct >> drm_i915_private *dev_priv) >> void intel_uc_init_early(struct drm_i915_private *dev_priv) >> { >> + struct intel_guc *guc = &dev_priv->guc; >> + struct intel_huc *huc = &dev_priv->huc; > > Please run checkpatch and add separation line here Will do. > >> intel_guc_init_early(&dev_priv->guc); > > You can now pass 'guc' as param Will do. > >> + intel_guc_fw_select(guc); >> + intel_huc_select_fw(huc); > > To avoid redundant checks in select_fw() functions we can > exit earlier with I will add the early exit condition. Thanks for the review. > > if (!HAS_GUC(dev_priv)) > return; > >> } >> void intel_uc_init_fw(struct drm_i915_private *dev_priv) >> { >> + if (!HAS_GUC(dev_priv)) >> + return; >> intel_uc_fw_fetch(dev_priv, &dev_priv->huc.fw); >> intel_uc_fw_fetch(dev_priv, &dev_priv->guc.fw); >> } Regards, Sujaritha
diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c b/drivers/gpu/drm/i915/intel_guc_fw.c index ef67a36..b9f834f 100644 --- a/drivers/gpu/drm/i915/intel_guc_fw.c +++ b/drivers/gpu/drm/i915/intel_guc_fw.c @@ -60,10 +60,8 @@ * intel_guc_fw_select() - selects GuC firmware for uploading * * @guc: intel_guc struct - * - * Return: zero when we know firmware, non-zero in other case */ -int intel_guc_fw_select(struct intel_guc *guc) +void intel_guc_fw_select(struct intel_guc *guc) { struct drm_i915_private *dev_priv = guc_to_i915(guc); @@ -90,11 +88,9 @@ int intel_guc_fw_select(struct intel_guc *guc) guc->fw.major_ver_wanted = GLK_FW_MAJOR; guc->fw.minor_ver_wanted = GLK_FW_MINOR; } else { - DRM_ERROR("No GuC firmware known for platform with GuC!\n"); - return -ENOENT; + DRM_ERROR("No GuC FW known for platform with GuC!\n"); + return; } - - return 0; } /* diff --git a/drivers/gpu/drm/i915/intel_guc_fw.h b/drivers/gpu/drm/i915/intel_guc_fw.h index 023f5ba..7f6ccaf 100644 --- a/drivers/gpu/drm/i915/intel_guc_fw.h +++ b/drivers/gpu/drm/i915/intel_guc_fw.h @@ -27,7 +27,7 @@ struct intel_guc; -int intel_guc_fw_select(struct intel_guc *guc); +void intel_guc_fw_select(struct intel_guc *guc); int intel_guc_fw_upload(struct intel_guc *guc); #endif diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c index c8a48cb..4e700ab 100644 --- a/drivers/gpu/drm/i915/intel_huc.c +++ b/drivers/gpu/drm/i915/intel_huc.c @@ -108,7 +108,8 @@ void intel_huc_select_fw(struct intel_huc *huc) huc->fw.major_ver_wanted = GLK_HUC_FW_MAJOR; huc->fw.minor_ver_wanted = GLK_HUC_FW_MINOR; } else { - DRM_ERROR("No HuC firmware known for platform with HuC!\n"); + if (HAS_GUC(dev_priv)) + DRM_ERROR("No HuC FW known for platform with HuC!\n"); return; } } diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c index 9369ade..dc978a0 100644 --- a/drivers/gpu/drm/i915/intel_uc.c +++ b/drivers/gpu/drm/i915/intel_uc.c @@ -82,11 +82,17 @@ void intel_uc_sanitize_options(struct drm_i915_private *dev_priv) void intel_uc_init_early(struct drm_i915_private *dev_priv) { + struct intel_guc *guc = &dev_priv->guc; + struct intel_huc *huc = &dev_priv->huc; intel_guc_init_early(&dev_priv->guc); + intel_guc_fw_select(guc); + intel_huc_select_fw(huc); } void intel_uc_init_fw(struct drm_i915_private *dev_priv) { + if (!HAS_GUC(dev_priv)) + return; intel_uc_fw_fetch(dev_priv, &dev_priv->huc.fw); intel_uc_fw_fetch(dev_priv, &dev_priv->guc.fw); }
Updating GuC and HuC firmware select function to support removing i915_modparams.enable_guc_loading module parameter. v2: Clarifying the commit message (Anusha) v3: Unify seq_puts messages, Re-factoring code as per review (Michal) v4: Rebase v5: Separating message unification into a separate patch v6: Re-factoring code (Sagar, Michal) Rebase v7: Separating from previuos patch (Sagar) Rebase v8: Including change to intel_uc.c Applying review comments (Michal) Signed-off-by: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: Oscar Mateo <oscar.mateo@intel.com> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com> --- drivers/gpu/drm/i915/intel_guc_fw.c | 10 +++------- drivers/gpu/drm/i915/intel_guc_fw.h | 2 +- drivers/gpu/drm/i915/intel_huc.c | 3 ++- drivers/gpu/drm/i915/intel_uc.c | 6 ++++++ 4 files changed, 12 insertions(+), 9 deletions(-)