| Message ID | 20170222124125.4437-5-arkadiusz.hiler@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On ke, 2017-02-22 at 13:41 +0100, Arkadiusz Hiler wrote: > Trying to have subject_verb_object ordering and more descriptive names, > the intel_huc_init() and intel_guc_init() functions are renamed: > > * `intel_guc` is the subject, so those functions now take intel_guc > structure, instead of the dev_priv > * fetch is the verb > * fw is the object which better describes the function's role > > Same change is done for the huc counterpart. > > Also we bulk call both functions from higher-level intel_uc_fetch_fw: > * intel being the subject (taking the dev_priv as param) > * fetch being the verb > * uc_fw being the subject > > v2: settle on intel_uc_fetch_fw name (M. Wajdeczko) > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> > Cc: Michal Winiarski <michal.winiarski@intel.com> > Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com> <SNIP> > @@ -609,8 +609,7 @@ static int i915_load_modeset_init(struct drm_device *dev) > if (ret) > goto cleanup_irq; > > - intel_huc_init(dev_priv); > - intel_guc_init(dev_priv); > + intel_uc_fetch_fw(dev_priv); intel_uc_init fits this context. (See below) > /** > - * intel_guc_init() - define parameters and fetch firmware > - * @dev_priv: i915 device private > + * intel_guc_fetch_fw() - define parameters and fetch firmware > + * @guc: intel_guc struct > * > * Called early during driver load, but after GEM is initialised. > * > * The firmware will be transferred to the GuC's memory later, > * when intel_guc_init_hw() is called. > */ "define parameters" is little vague, maybe "select and fetch firmware"? > -void intel_guc_init(struct drm_i915_private *dev_priv) > +void intel_guc_fetch_fw(struct intel_guc *guc) > { > - struct intel_uc_fw *guc_fw = &dev_priv->guc.fw; > + struct drm_i915_private *dev_priv = guc_to_i915(guc); > const char *fw_path; > > if (!HAS_GUC(dev_priv)) { This parameter dance needs to be moved away from guc_fetch_fw function, into intel_sanitize_options (I'm pretty sure I've mentioned this earlier). > @@ -751,23 +751,23 @@ void intel_guc_init(struct drm_i915_private *dev_priv) > fw_path = NULL; > } else if (IS_SKYLAKE(dev_priv)) { > fw_path = I915_SKL_GUC_UCODE; > - guc_fw->major_ver_wanted = SKL_FW_MAJOR; > - guc_fw->minor_ver_wanted = SKL_FW_MINOR; > + guc->fw.major_ver_wanted = SKL_FW_MAJOR; > + guc->fw.minor_ver_wanted = SKL_FW_MINOR; > } else if (IS_BROXTON(dev_priv)) { > fw_path = I915_BXT_GUC_UCODE; > - guc_fw->major_ver_wanted = BXT_FW_MAJOR; > - guc_fw->minor_ver_wanted = BXT_FW_MINOR; > + guc->fw.major_ver_wanted = BXT_FW_MAJOR; > + guc->fw.minor_ver_wanted = BXT_FW_MINOR; > } else if (IS_KABYLAKE(dev_priv)) { > fw_path = I915_KBL_GUC_UCODE; > - guc_fw->major_ver_wanted = KBL_FW_MAJOR; > - guc_fw->minor_ver_wanted = KBL_FW_MINOR; > + guc->fw.major_ver_wanted = KBL_FW_MAJOR; > + guc->fw.minor_ver_wanted = KBL_FW_MINOR; > } else { > fw_path = ""; /* unknown device */ > } > > - guc_fw->path = fw_path; > - guc_fw->fetch_status = INTEL_UC_FIRMWARE_NONE; > - guc_fw->load_status = INTEL_UC_FIRMWARE_NONE; > + guc->fw.path = fw_path; Just get rid of fw_path variable and assign directly, also hoist the warning to the else branch, which can then do "return;" > + guc->fw.fetch_status = INTEL_UC_FIRMWARE_NONE; > + guc->fw.load_status = INTEL_UC_FIRMWARE_NONE; Hoist this assignment before the if block, so no need to special for the early return from else branch. <SNIP> > /** > - * intel_huc_init() - initiate HuC firmware loading request > - * @dev_priv: the drm_i915_private device > + * intel_huc_fetch_fw() - initiate HuC firmware loading request Correct this commit too to be more descriptive. > + * @huc: intel_huc struct > * > * Called early during driver load, but after GEM is initialised. The loading > * will continue only when driver explicitly specify firmware name and version. > @@ -152,42 +152,41 @@ static int huc_ucode_xfer(struct drm_i915_private *dev_priv) > * > * The DMA-copying to HW is done later when intel_huc_init_hw() is called. > */ > -void intel_huc_init(struct drm_i915_private *dev_priv) > +void intel_huc_fetch_fw(struct intel_huc *huc) > { > - struct intel_huc *huc = &dev_priv->huc; > - struct intel_uc_fw *huc_fw = &huc->fw; > + struct drm_i915_private *dev_priv = huc_to_i915(huc); > const char *fw_path = NULL; Similarly arrange to get rid of fw_path here. <SNIP> > @@ -30,6 +30,12 @@ void intel_uc_init_early(struct drm_i915_private *dev_priv) > mutex_init(&dev_priv->guc.send_mutex); > } > > +void intel_uc_fetch_fw(struct drm_i915_private *dev_priv) This function might be worth calling intel_uc_init (See above), if the need comes to add other stuff. But either way. Regards, Joonas
On Wed, Feb 22, 2017 at 03:59:01PM +0200, Joonas Lahtinen wrote: > On ke, 2017-02-22 at 13:41 +0100, Arkadiusz Hiler wrote: > > Trying to have subject_verb_object ordering and more descriptive names, > > the intel_huc_init() and intel_guc_init() functions are renamed: > > > > * `intel_guc` is the subject, so those functions now take intel_guc > > structure, instead of the dev_priv > > * fetch is the verb > > * fw is the object which better describes the function's role > > > > Same change is done for the huc counterpart. > > > > Also we bulk call both functions from higher-level intel_uc_fetch_fw: > > * intel being the subject (taking the dev_priv as param) > > * fetch being the verb > > * uc_fw being the subject > > > > v2: settle on intel_uc_fetch_fw name (M. Wajdeczko) > > > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> > > Cc: Michal Winiarski <michal.winiarski@intel.com> > > Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com> > > <SNIP> > > > @@ -609,8 +609,7 @@ static int i915_load_modeset_init(struct drm_device *dev) > > if (ret) > > goto cleanup_irq; > > > > - intel_huc_init(dev_priv); > > - intel_guc_init(dev_priv); > > + intel_uc_fetch_fw(dev_priv); > > intel_uc_init fits this context. (See below) Answer below. > > > /** > > - * intel_guc_init() - define parameters and fetch firmware > > - * @dev_priv: i915 device private > > + * intel_guc_fetch_fw() - define parameters and fetch firmware > > + * @guc: intel_guc struct > > * > > * Called early during driver load, but after GEM is initialised. > > * > > * The firmware will be transferred to the GuC's memory later, > > * when intel_guc_init_hw() is called. > > */ > > "define parameters" is little vague, maybe "select and fetch firmware"? I like those verbs. Let start using it through the whole thing! > > > -void intel_guc_init(struct drm_i915_private *dev_priv) > > +void intel_guc_fetch_fw(struct intel_guc *guc) > > { > > - struct intel_uc_fw *guc_fw = &dev_priv->guc.fw; > > + struct drm_i915_private *dev_priv = guc_to_i915(guc); > > const char *fw_path; > > > > if (!HAS_GUC(dev_priv)) { > > This parameter dance needs to be moved away from guc_fetch_fw function, > into intel_sanitize_options (I'm pretty sure I've mentioned this > earlier). This is removed in patch 8, as the fetch_fw is called conditionally. > > @@ -751,23 +751,23 @@ void intel_guc_init(struct drm_i915_private *dev_priv) > > fw_path = NULL; > > } else if (IS_SKYLAKE(dev_priv)) { > > fw_path = I915_SKL_GUC_UCODE; > > - guc_fw->major_ver_wanted = SKL_FW_MAJOR; > > - guc_fw->minor_ver_wanted = SKL_FW_MINOR; > > + guc->fw.major_ver_wanted = SKL_FW_MAJOR; > > + guc->fw.minor_ver_wanted = SKL_FW_MINOR; > > } else if (IS_BROXTON(dev_priv)) { > > fw_path = I915_BXT_GUC_UCODE; > > - guc_fw->major_ver_wanted = BXT_FW_MAJOR; > > - guc_fw->minor_ver_wanted = BXT_FW_MINOR; > > + guc->fw.major_ver_wanted = BXT_FW_MAJOR; > > + guc->fw.minor_ver_wanted = BXT_FW_MINOR; > > } else if (IS_KABYLAKE(dev_priv)) { > > fw_path = I915_KBL_GUC_UCODE; > > - guc_fw->major_ver_wanted = KBL_FW_MAJOR; > > - guc_fw->minor_ver_wanted = KBL_FW_MINOR; > > + guc->fw.major_ver_wanted = KBL_FW_MAJOR; > > + guc->fw.minor_ver_wanted = KBL_FW_MINOR; > > } else { > > fw_path = ""; /* unknown device */ > > } > > > > - guc_fw->path = fw_path; > > - guc_fw->fetch_status = INTEL_UC_FIRMWARE_NONE; > > - guc_fw->load_status = INTEL_UC_FIRMWARE_NONE; > > + guc->fw.path = fw_path; > > Just get rid of fw_path variable and assign directly, also hoist the > warning to the else branch, which can then do "return;" This is done done in patch 8. > > + guc->fw.fetch_status = INTEL_UC_FIRMWARE_NONE; > > + guc->fw.load_status = INTEL_UC_FIRMWARE_NONE; > > Hoist this assignment before the if block, so no need to special for > the early return from else branch. This is done done in patch 8. > <SNIP> > > > /** > > - * intel_huc_init() - initiate HuC firmware loading request > > - * @dev_priv: the drm_i915_private device > > + * intel_huc_fetch_fw() - initiate HuC firmware loading request > > Correct this commit too to be more descriptive. Okay. > > + * @huc: intel_huc struct > > * > > * Called early during driver load, but after GEM is initialised. The loading > > * will continue only when driver explicitly specify firmware name and version. > > @@ -152,42 +152,41 @@ static int huc_ucode_xfer(struct drm_i915_private *dev_priv) > > * > > * The DMA-copying to HW is done later when intel_huc_init_hw() is called. > > */ > > -void intel_huc_init(struct drm_i915_private *dev_priv) > > +void intel_huc_fetch_fw(struct intel_huc *huc) > > { > > - struct intel_huc *huc = &dev_priv->huc; > > - struct intel_uc_fw *huc_fw = &huc->fw; > > + struct drm_i915_private *dev_priv = huc_to_i915(huc); > > const char *fw_path = NULL; > > Similarly arrange to get rid of fw_path here. Patch 8 in the series addresses that issue as well. Maybe I should move them around? > <SNIP> > > > @@ -30,6 +30,12 @@ void intel_uc_init_early(struct drm_i915_private *dev_priv) > > mutex_init(&dev_priv->guc.send_mutex); > > } > > > > +void intel_uc_fetch_fw(struct drm_i915_private *dev_priv) > > This function might be worth calling intel_uc_init (See above), if the > need comes to add other stuff. But either way. This is quite confusing now. I was fine it being named init, someone suggested to be more descriptive with the name, as it is not general enough to be "init". Seemed reasonable enough for me, so I incorporated that in the respin. This is turning into some heavy bikeshedding now... I agree that it's more than fetch, it actually selects + fetches + populates the structures. I'll gladly ignore previous feedback on being to vague with name and just go with init, but let give the _fw postfix one last chance: intel_guc_init_fw { intel_guc_select_fw if (NULL != guc.fw.path) intel_uc_prepare_fw } Where select does what the guc's fetch fw does sans the uc_fetch call. Also intel_{g,h}uc_select_fw can be made part of the sanitize options, but I think it better belongs here. That's is basing on your suggestions for the other patch.
On ke, 2017-02-22 at 16:29 +0100, Arkadiusz Hiler wrote: > On Wed, Feb 22, 2017 at 03:59:01PM +0200, Joonas Lahtinen wrote: > > > > > + * @huc: intel_huc struct > > > * > > > * Called early during driver load, but after GEM is initialised. The loading > > > * will continue only when driver explicitly specify firmware name and version. > > > @@ -152,42 +152,41 @@ static int huc_ucode_xfer(struct drm_i915_private *dev_priv) > > > * > > > * The DMA-copying to HW is done later when intel_huc_init_hw() is called. > > > */ > > > -void intel_huc_init(struct drm_i915_private *dev_priv) > > > +void intel_huc_fetch_fw(struct intel_huc *huc) > > > { > > > > > > - struct intel_huc *huc = &dev_priv->huc; > > > > > > - struct intel_uc_fw *huc_fw = &huc->fw; > > > > > > + struct drm_i915_private *dev_priv = huc_to_i915(huc); > > > > > > const char *fw_path = NULL; > > > > Similarly arrange to get rid of fw_path here. > > Patch 8 in the series addresses that issue as well. Maybe I should move > them around? Nah, it's fine, the intermediary steps need to be working (for bisecting), but not necessarily 100% pretty. If it's addressed later, it's good. > > > @@ -30,6 +30,12 @@ void intel_uc_init_early(struct drm_i915_private *dev_priv) > > > mutex_init(&dev_priv->guc.send_mutex); > > > } > > > > > > +void intel_uc_fetch_fw(struct drm_i915_private *dev_priv) > > > > This function might be worth calling intel_uc_init (See above), if the > > need comes to add other stuff. But either way. > > This is quite confusing now. I was fine it being named init, someone > suggested to be more descriptive with the name, as it is not general > enough to be "init". Seemed reasonable enough for me, so I incorporated > that in the respin. > > This is turning into some heavy bikeshedding now... That's why actual code in the mailing list is the only right way, discussion in IRC can be misleading :) > > I agree that it's more than fetch, it actually selects + fetches + > populates the structures. > > I'll gladly ignore previous feedback on being to vague with name and > just go with init, but let give the _fw postfix one last chance: > > > intel_guc_init_fw { > intel_guc_select_fw > if (NULL != guc.fw.path) if (guc.fw.patch) to stick to coding style. > intel_uc_prepare_fw > } > > Where select does what the guc's fetch fw does sans the uc_fetch call. Sounds good to me. > Also intel_{g,h}uc_select_fw can be made part of the sanitize options, > but I think it better belongs here. > > That's is basing on your suggestions for the other patch. Thats, correct, select_fw should be here. if (!HAS_GUC(dev_priv)) { 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_priv); if (i915.enable_guc_submission < 0) i915.enable_guc_submission = HAS_GUC_SCHED(dev_priv); } This part is a perfect fit to the sanitize_options function, because that's what it does, makes sure we don't try to enable something we don't have. Regards, Joonas
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index b76e8f7..078ac3e 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -609,8 +609,7 @@ static int i915_load_modeset_init(struct drm_device *dev) if (ret) goto cleanup_irq; - intel_huc_init(dev_priv); - intel_guc_init(dev_priv); + intel_uc_fetch_fw(dev_priv); ret = i915_gem_init(dev_priv); if (ret) diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c index 9f09e26..753aeef 100644 --- a/drivers/gpu/drm/i915/intel_guc_loader.c +++ b/drivers/gpu/drm/i915/intel_guc_loader.c @@ -723,17 +723,17 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv, } /** - * intel_guc_init() - define parameters and fetch firmware - * @dev_priv: i915 device private + * intel_guc_fetch_fw() - define parameters and fetch firmware + * @guc: intel_guc struct * * Called early during driver load, but after GEM is initialised. * * The firmware will be transferred to the GuC's memory later, * when intel_guc_init_hw() is called. */ -void intel_guc_init(struct drm_i915_private *dev_priv) +void intel_guc_fetch_fw(struct intel_guc *guc) { - struct intel_uc_fw *guc_fw = &dev_priv->guc.fw; + struct drm_i915_private *dev_priv = guc_to_i915(guc); const char *fw_path; if (!HAS_GUC(dev_priv)) { @@ -751,23 +751,23 @@ void intel_guc_init(struct drm_i915_private *dev_priv) fw_path = NULL; } else if (IS_SKYLAKE(dev_priv)) { fw_path = I915_SKL_GUC_UCODE; - guc_fw->major_ver_wanted = SKL_FW_MAJOR; - guc_fw->minor_ver_wanted = SKL_FW_MINOR; + guc->fw.major_ver_wanted = SKL_FW_MAJOR; + guc->fw.minor_ver_wanted = SKL_FW_MINOR; } else if (IS_BROXTON(dev_priv)) { fw_path = I915_BXT_GUC_UCODE; - guc_fw->major_ver_wanted = BXT_FW_MAJOR; - guc_fw->minor_ver_wanted = BXT_FW_MINOR; + guc->fw.major_ver_wanted = BXT_FW_MAJOR; + guc->fw.minor_ver_wanted = BXT_FW_MINOR; } else if (IS_KABYLAKE(dev_priv)) { fw_path = I915_KBL_GUC_UCODE; - guc_fw->major_ver_wanted = KBL_FW_MAJOR; - guc_fw->minor_ver_wanted = KBL_FW_MINOR; + guc->fw.major_ver_wanted = KBL_FW_MAJOR; + guc->fw.minor_ver_wanted = KBL_FW_MINOR; } else { fw_path = ""; /* unknown device */ } - guc_fw->path = fw_path; - guc_fw->fetch_status = INTEL_UC_FIRMWARE_NONE; - guc_fw->load_status = INTEL_UC_FIRMWARE_NONE; + guc->fw.path = fw_path; + guc->fw.fetch_status = INTEL_UC_FIRMWARE_NONE; + guc->fw.load_status = INTEL_UC_FIRMWARE_NONE; /* Early (and silent) return if GuC loading is disabled */ if (!i915.enable_guc_loading) @@ -777,9 +777,9 @@ void intel_guc_init(struct drm_i915_private *dev_priv) if (*fw_path == '\0') return; - guc_fw->fetch_status = INTEL_UC_FIRMWARE_PENDING; + guc->fw.fetch_status = INTEL_UC_FIRMWARE_PENDING; DRM_DEBUG_DRIVER("GuC firmware pending, path %s\n", fw_path); - intel_uc_fw_fetch(dev_priv, guc_fw); + intel_uc_fw_fetch(dev_priv, &guc->fw); /* status must now be FAIL or SUCCESS */ } diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c index babe0eb..0725e6f 100644 --- a/drivers/gpu/drm/i915/intel_huc.c +++ b/drivers/gpu/drm/i915/intel_huc.c @@ -141,8 +141,8 @@ static int huc_ucode_xfer(struct drm_i915_private *dev_priv) } /** - * intel_huc_init() - initiate HuC firmware loading request - * @dev_priv: the drm_i915_private device + * intel_huc_fetch_fw() - initiate HuC firmware loading request + * @huc: intel_huc struct * * Called early during driver load, but after GEM is initialised. The loading * will continue only when driver explicitly specify firmware name and version. @@ -152,42 +152,41 @@ static int huc_ucode_xfer(struct drm_i915_private *dev_priv) * * The DMA-copying to HW is done later when intel_huc_init_hw() is called. */ -void intel_huc_init(struct drm_i915_private *dev_priv) +void intel_huc_fetch_fw(struct intel_huc *huc) { - struct intel_huc *huc = &dev_priv->huc; - struct intel_uc_fw *huc_fw = &huc->fw; + struct drm_i915_private *dev_priv = huc_to_i915(huc); const char *fw_path = NULL; - huc_fw->path = NULL; - huc_fw->fetch_status = INTEL_UC_FIRMWARE_NONE; - huc_fw->load_status = INTEL_UC_FIRMWARE_NONE; - huc_fw->fw = INTEL_UC_FW_TYPE_HUC; + huc->fw.path = NULL; + huc->fw.fetch_status = INTEL_UC_FIRMWARE_NONE; + huc->fw.load_status = INTEL_UC_FIRMWARE_NONE; + huc->fw.fw = INTEL_UC_FW_TYPE_HUC; if (!HAS_HUC_UCODE(dev_priv)) return; if (IS_SKYLAKE(dev_priv)) { fw_path = I915_SKL_HUC_UCODE; - huc_fw->major_ver_wanted = SKL_HUC_FW_MAJOR; - huc_fw->minor_ver_wanted = SKL_HUC_FW_MINOR; + huc->fw.major_ver_wanted = SKL_HUC_FW_MAJOR; + huc->fw.minor_ver_wanted = SKL_HUC_FW_MINOR; } else if (IS_BROXTON(dev_priv)) { fw_path = I915_BXT_HUC_UCODE; - huc_fw->major_ver_wanted = BXT_HUC_FW_MAJOR; - huc_fw->minor_ver_wanted = BXT_HUC_FW_MINOR; + huc->fw.major_ver_wanted = BXT_HUC_FW_MAJOR; + huc->fw.minor_ver_wanted = BXT_HUC_FW_MINOR; } else if (IS_KABYLAKE(dev_priv)) { fw_path = I915_KBL_HUC_UCODE; - huc_fw->major_ver_wanted = KBL_HUC_FW_MAJOR; - huc_fw->minor_ver_wanted = KBL_HUC_FW_MINOR; + huc->fw.major_ver_wanted = KBL_HUC_FW_MAJOR; + huc->fw.minor_ver_wanted = KBL_HUC_FW_MINOR; } - huc_fw->path = fw_path; - huc_fw->fetch_status = INTEL_UC_FIRMWARE_PENDING; + huc->fw.path = fw_path; + huc->fw.fetch_status = INTEL_UC_FIRMWARE_PENDING; DRM_DEBUG_DRIVER("HuC firmware pending, path %s\n", fw_path); - WARN(huc_fw->path == NULL, "HuC present but no fw path\n"); + WARN(huc->fw.path == NULL, "HuC present but no fw path\n"); - intel_uc_fw_fetch(dev_priv, huc_fw); + intel_uc_fw_fetch(dev_priv, &huc->fw); } /** diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c index c46bc85..5bb9149 100644 --- a/drivers/gpu/drm/i915/intel_uc.c +++ b/drivers/gpu/drm/i915/intel_uc.c @@ -30,6 +30,12 @@ void intel_uc_init_early(struct drm_i915_private *dev_priv) mutex_init(&dev_priv->guc.send_mutex); } +void intel_uc_fetch_fw(struct drm_i915_private *dev_priv) +{ + intel_huc_fetch_fw(&dev_priv->huc); + intel_guc_fetch_fw(&dev_priv->guc); +} + /* * Read GuC command/status register (SOFT_SCRATCH_0) * Return true if it contains a response rather than a command diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h index 41b7351..db596b6 100644 --- a/drivers/gpu/drm/i915/intel_uc.h +++ b/drivers/gpu/drm/i915/intel_uc.h @@ -185,11 +185,12 @@ struct intel_huc { /* intel_uc.c */ void intel_uc_init_early(struct drm_i915_private *dev_priv); +void intel_uc_fetch_fw(struct drm_i915_private *dev_priv); int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 len); int intel_guc_sample_forcewake(struct intel_guc *guc); /* intel_guc_loader.c */ -void intel_guc_init(struct drm_i915_private *dev_priv); +void intel_guc_fetch_fw(struct intel_guc *guc); int intel_guc_init_hw(struct drm_i915_private *dev_priv); void intel_guc_fini(struct drm_i915_private *dev_priv); const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status); @@ -223,7 +224,7 @@ static inline u32 guc_ggtt_offset(struct i915_vma *vma) } /* intel_huc.c */ -void intel_huc_init(struct drm_i915_private *dev_priv); +void intel_huc_fetch_fw(struct intel_huc *huc); void intel_huc_fini(struct drm_i915_private *dev_priv); int intel_huc_init_hw(struct drm_i915_private *dev_priv); void intel_guc_auth_huc(struct drm_i915_private *dev_priv);
Trying to have subject_verb_object ordering and more descriptive names, the intel_huc_init() and intel_guc_init() functions are renamed: * `intel_guc` is the subject, so those functions now take intel_guc structure, instead of the dev_priv * fetch is the verb * fw is the object which better describes the function's role Same change is done for the huc counterpart. Also we bulk call both functions from higher-level intel_uc_fetch_fw: * intel being the subject (taking the dev_priv as param) * fetch being the verb * uc_fw being the subject v2: settle on intel_uc_fetch_fw name (M. Wajdeczko) Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: Michal Winiarski <michal.winiarski@intel.com> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com> --- drivers/gpu/drm/i915/i915_drv.c | 3 +-- drivers/gpu/drm/i915/intel_guc_loader.c | 30 +++++++++++++------------- drivers/gpu/drm/i915/intel_huc.c | 37 ++++++++++++++++----------------- drivers/gpu/drm/i915/intel_uc.c | 6 ++++++ drivers/gpu/drm/i915/intel_uc.h | 5 +++-- 5 files changed, 43 insertions(+), 38 deletions(-)