Re: [PATCH 0/9] drm/i915: Guc code reorg

[Joonas Lahtinen <joonas.lahtinen@linux.intel.com>]

On Fri, 2017-09-29 at 17:41 +0000, Michal Wajdeczko wrote:
> Other pending series will try to fix current GuC code.
> Lets move some functions to dedicated files now to
> make place for these changes and preserve history.
> 
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> 
> Michal Wajdeczko (9):
>   drm/i915: Drop unnecessary forward declaration
>   drm/i915: Move uC fw helper code into dedicated files
>   drm/i915: Move HuC declarations into dedicated header
>   drm/i915: Move GuC declarations into dedicated header
>   drm/i915: Move core GuC functions into dedicated file

If these are new header files, then patch per new header would be good
granularity.

>   drm/i915: Move intel_guc_sample_forcewake to guc.c
>   drm/i915: Move intel_guc_auth_huc to guc.c
>   drm/i915: Move intel_guc_suspend/resume to guc.c
>   drm/i915: Move intel_guc_allocate_vma to guc.c

When you're moving functions to same file .c, you can do it in a single
patch. There're no hard rules, just trying to keep the amount of
patches reasonable vs. having enough GIT history of what was done.

Then, don't do the s/dev_priv/i915/ together with the code motion even
if it may feel convenient. Will be harder to notice from the GIT
history that a mass s/dev_priv/i915/ was done and also the reviewer has
to go through all the code that was moved to check for possible errors
(in indent for example). Always think of the reviewer, and send code
motion in front, so that can be reviewed based on what was moved where,
then any changes so they can be reviewed for correctness.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx