diff mbox

[1/3] drm/i915: Reorder early initialization

Message ID 20180322203659.59348-1-michal.wajdeczko@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Michal Wajdeczko March 22, 2018, 8:36 p.m. UTC
In upcoming patch, we want to perform more actions in early
initialization of the uC. This reordering will help resolve
new dependencies that will be introduced by future patch.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

Comments

Chris Wilson March 22, 2018, 8:48 p.m. UTC | #1
Quoting Michal Wajdeczko (2018-03-22 20:36:57)
> In upcoming patch, we want to perform more actions in early
> initialization of the uC. This reordering will help resolve
> new dependencies that will be introduced by future patch.
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c | 17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index a7d3275..f60bc04 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -919,17 +919,21 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv,
>         mutex_init(&dev_priv->wm.wm_mutex);
>         mutex_init(&dev_priv->pps_mutex);
>  
> -       intel_wopcm_init_early(&dev_priv->wopcm);
> -       intel_uc_init_early(dev_priv);
>         i915_memcpy_init_early(dev_priv);
>  
>         ret = i915_workqueues_init(dev_priv);
>         if (ret < 0)
>                 goto err_engines;
>  
> +       ret = i915_gem_load_init(dev_priv);
> +       if (ret < 0)
> +               goto err_workqueues;

Scary. But really misnamed. I was thinking this was i915_gem_init(),
could you rename this i915_gem_init_early() to indicate that all it is
doing is the early allocations and nothing HW related.

>         /* This must be called before any calls to HAS_PCH_* */
>         intel_detect_pch(dev_priv);
>  
> +       intel_wopcm_init_early(&dev_priv->wopcm);
> +       intel_uc_init_early(dev_priv);
>         intel_pm_setup(dev_priv);
>         intel_init_dpio(dev_priv);
>         intel_power_domains_init(dev_priv);
> @@ -938,18 +942,13 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv,
>         intel_init_display_hooks(dev_priv);
>         intel_init_clock_gating_hooks(dev_priv);
>         intel_init_audio_hooks(dev_priv);
> -       ret = i915_gem_load_init(dev_priv);
> -       if (ret < 0)
> -               goto err_irq;
> -
>         intel_display_crc_init(dev_priv);
>  
>         intel_detect_preproduction_hw(dev_priv);
>  
>         return 0;
>  
> -err_irq:
> -       intel_irq_fini(dev_priv);
> +err_workqueues:
>         i915_workqueues_cleanup(dev_priv);
>  err_engines:
>         i915_engines_cleanup(dev_priv);
> @@ -962,8 +961,8 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv,
>   */
>  static void i915_driver_cleanup_early(struct drm_i915_private *dev_priv)
>  {
> -       i915_gem_load_cleanup(dev_priv);
>         intel_irq_fini(dev_priv);
> +       i915_gem_load_cleanup(dev_priv);

Ok, onion preserved.

>         i915_workqueues_cleanup(dev_priv);
>         i915_engines_cleanup(dev_priv);

* mutters about dead code.

Code reordering looks fine,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
Chris Wilson March 23, 2018, 8:53 a.m. UTC | #2
Quoting Patchwork (2018-03-22 21:52:05)
> == Series Details ==
> 
> Series: series starting with [v2,3/3] HAX: Enable GuC for CI (rev4)
> URL   : https://patchwork.freedesktop.org/series/40516/
> State : failure
> 
> == Summary ==
> 
> Applying: HAX: Enable GuC for CI
> Applying: drm/i915/uc: Fetch uC firmware in init_early
> error: Failed to merge in the changes.
> Using index info to reconstruct a base tree...
> M       drivers/gpu/drm/i915/i915_drv.c
> Falling back to patching base and 3-way merge...
> Auto-merging drivers/gpu/drm/i915/i915_drv.c
> CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/i915_drv.c
> Patch failed at 0002 drm/i915/uc: Fetch uC firmware in init_early
> The copy of the patch that failed is found in: .git/rebase-apply/patch
> When you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".

Send it afresh, pw got confused by the reply with a different series
length.
-Chris
Michal Wajdeczko March 23, 2018, 12:29 p.m. UTC | #3
On Thu, 22 Mar 2018 21:48:16 +0100, Chris Wilson  
<chris@chris-wilson.co.uk> wrote:

> Quoting Michal Wajdeczko (2018-03-22 20:36:57)

/snip/

>> @@ -962,8 +961,8 @@ static int i915_driver_init_early(struct  
>> drm_i915_private *dev_priv,
>>   */
>>  static void i915_driver_cleanup_early(struct drm_i915_private  
>> *dev_priv)
>>  {
>> -       i915_gem_load_cleanup(dev_priv);
>>         intel_irq_fini(dev_priv);
>> +       i915_gem_load_cleanup(dev_priv);
>
> Ok, onion preserved.

For symmetry, I'll also rename this to i915_gem_cleanup_early

/m
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index a7d3275..f60bc04 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -919,17 +919,21 @@  static int i915_driver_init_early(struct drm_i915_private *dev_priv,
 	mutex_init(&dev_priv->wm.wm_mutex);
 	mutex_init(&dev_priv->pps_mutex);
 
-	intel_wopcm_init_early(&dev_priv->wopcm);
-	intel_uc_init_early(dev_priv);
 	i915_memcpy_init_early(dev_priv);
 
 	ret = i915_workqueues_init(dev_priv);
 	if (ret < 0)
 		goto err_engines;
 
+	ret = i915_gem_load_init(dev_priv);
+	if (ret < 0)
+		goto err_workqueues;
+
 	/* This must be called before any calls to HAS_PCH_* */
 	intel_detect_pch(dev_priv);
 
+	intel_wopcm_init_early(&dev_priv->wopcm);
+	intel_uc_init_early(dev_priv);
 	intel_pm_setup(dev_priv);
 	intel_init_dpio(dev_priv);
 	intel_power_domains_init(dev_priv);
@@ -938,18 +942,13 @@  static int i915_driver_init_early(struct drm_i915_private *dev_priv,
 	intel_init_display_hooks(dev_priv);
 	intel_init_clock_gating_hooks(dev_priv);
 	intel_init_audio_hooks(dev_priv);
-	ret = i915_gem_load_init(dev_priv);
-	if (ret < 0)
-		goto err_irq;
-
 	intel_display_crc_init(dev_priv);
 
 	intel_detect_preproduction_hw(dev_priv);
 
 	return 0;
 
-err_irq:
-	intel_irq_fini(dev_priv);
+err_workqueues:
 	i915_workqueues_cleanup(dev_priv);
 err_engines:
 	i915_engines_cleanup(dev_priv);
@@ -962,8 +961,8 @@  static int i915_driver_init_early(struct drm_i915_private *dev_priv,
  */
 static void i915_driver_cleanup_early(struct drm_i915_private *dev_priv)
 {
-	i915_gem_load_cleanup(dev_priv);
 	intel_irq_fini(dev_priv);
+	i915_gem_load_cleanup(dev_priv);
 	i915_workqueues_cleanup(dev_priv);
 	i915_engines_cleanup(dev_priv);
 }