[Intel-gfx] [PATCH 1/8] drm/i915/guc: Make the GuC fw loading helper functions general

[Srivatsa, Anusha]

>-----Original Message-----
>From: Wajdeczko, Michal
>Sent: Tuesday, January 3, 2017 6:15 AM
>To: Srivatsa, Anusha <anusha.srivatsa at intel.com>
>Cc: intel-gfx at lists.freedesktop.org
>Subject: Re: [Intel-gfx] [PATCH 1/8] drm/i915/guc: Make the GuC fw loading
>helper functions general
>
>On Tue, Jan 03, 2017 at 01:07:14AM +0100, Srivatsa, Anusha wrote:
>>
>>
>> >-----Original Message-----
>> >From: Wajdeczko, Michal
>> >Sent: Tuesday, December 27, 2016 9:28 AM
>> >To: Srivatsa, Anusha <anusha.srivatsa at intel.com>
>> >Cc: intel-gfx at lists.freedesktop.org; Alex Dai <yu.dai at intel.com>;
>> >Peter Antoine <peter.antoine at intel.com>
>> >Subject: Re: [Intel-gfx] [PATCH 1/8] drm/i915/guc: Make the GuC fw
>> >loading helper functions general
>> >
>> >On Thu, Dec 22, 2016 at 03:12:17PM -0800, Anusha Srivatsa wrote:
>> >> From: Peter Antoine <peter.antoine at intel.com>
>> >>
>> >> Rename some of the GuC fw loading code to make them more general.
>> >> We will utilise them for HuC loading as well.
>> >>      s/intel_guc_fw/intel_uc_fw/g
>> >>      s/GUC_FIRMWARE/UC_FIRMWARE/g
>> >>
>> >> Struct intel_guc_fw is renamed to intel_uc_fw. Prefix of tts
>> >> members, such as 'guc' or 'guc_fw' either is renamed to 'uc' or
>> >> removed for same purpose.
>> >>
>> >> v2: rebased on top of nightly.
>> >>     reapplied the search/replace as upstream code as changed.
>> >> v3: rebased again on drm-nightly.
>> >> v4: removed G from messages in shared fw fetch function.
>> >> v5: rebased.
>> >> v7: rebased.
>> >> v8: rebased.
>> >> v9: rebased.
>> >> v10: rebased.
>> >> v11: rebased.
>> >> v12: rebased on top of drm-tip
>> >> v13: rebased.Updated dev to dev_priv in intel_guc_setup(),
>> >> guc_fw_getch() and intel_guc_init().
>> >> v14: rebased. Remove uint32_t fw_type to patch 2. Add INTEL_ prefix
>> >> for fields in enum intel_uc_fw_status. Remove uc_dev field since
>> >> its never used.Rename uc_fw to just fw and guc_fw to fw to avoid
>redundency.
>> >> v15: rebased. Remove sections of code that were commented and no
>> >> longer required.
>> >>
>> >> Signed-off-by: Anusha Srivatsa <anusha.srivatsa at intel.com>
>> >> Signed-off-by: Alex Dai <yu.dai at intel.com>
>> >> Signed-off-by: Peter Antoine <peter.antoine at intel.com>
>> >> ---
>> >>  drivers/gpu/drm/i915/i915_debugfs.c        |  12 +--
>> >>  drivers/gpu/drm/i915/i915_guc_submission.c |   4 +-
>> >>  drivers/gpu/drm/i915/intel_guc_loader.c    | 156 ++++++++++++++------------
>---
>> >>  drivers/gpu/drm/i915/intel_uc.h            |  36 +++----
>> >>  4 files changed, 104 insertions(+), 104 deletions(-)
>> >>
>
>... CUT ...
>
>> >> diff --git a/drivers/gpu/drm/i915/intel_uc.h
>> >> b/drivers/gpu/drm/i915/intel_uc.h index 11f5608..893bcec 100644
>> >> --- a/drivers/gpu/drm/i915/intel_uc.h
>> >> +++ b/drivers/gpu/drm/i915/intel_uc.h
>> >> @@ -91,28 +91,28 @@ struct i915_guc_client {
>> >>  	uint64_t submissions[I915_NUM_ENGINES];  };
>> >>
>> >> -enum intel_guc_fw_status {
>> >> -	GUC_FIRMWARE_FAIL = -1,
>> >> -	GUC_FIRMWARE_NONE = 0,
>> >> -	GUC_FIRMWARE_PENDING,
>> >> -	GUC_FIRMWARE_SUCCESS
>> >> +enum intel_uc_fw_status {
>> >> +	INTEL_UC_FIRMWARE_FAIL = -1,
>> >> +	INTEL_UC_FIRMWARE_NONE = 0,
>> >> +	INTEL_UC_FIRMWARE_PENDING,
>> >> +	INTEL_UC_FIRMWARE_SUCCESS
>> >>  };
>> >>
>> >>  /*
>> >>   * This structure encapsulates all the data needed during the process
>> >>   * of fetching, caching, and loading the firmware image into the GuC.
>> >>   */
>> >> -struct intel_guc_fw {
>> >> -	const char *			guc_fw_path;
>> >> -	size_t				guc_fw_size;
>> >> -	struct drm_i915_gem_object *	guc_fw_obj;
>> >> -	enum intel_guc_fw_status	guc_fw_fetch_status;
>> >> -	enum intel_guc_fw_status	guc_fw_load_status;
>> >> -
>> >> -	uint16_t			guc_fw_major_wanted;
>> >> -	uint16_t			guc_fw_minor_wanted;
>> >> -	uint16_t			guc_fw_major_found;
>> >> -	uint16_t			guc_fw_minor_found;
>> >> +struct intel_uc_fw {
>> >> +	const char *uc_fw_path;
>> >
>> >Can we drop "uc_fw_" prefix also from path and obj members?
>>
>> Michal, you think we can do this change as a part of the guc refactor effort
>which will happen post this series gets merged? Or do you feel its makes more
>sense to that we have this change in this series....
>>
>
>IMHO we should avoid introducing changes that we already know they are bad
>and easy to fix.
>Number of changes in names shall be minimized to avoid confusion and merge
>conflicts.
>Also relaying on the future refactor effort (that has no known ETA) is not good
>option, as one can forget to fix it ;)

Sure. :)

Anusha
>Thanks,
>Michal