[PATCH 1/2] drm/i915: Split uncore init into vfunc setup and mmio setup

[PATCH 1/2] drm/i915: Split uncore init into vfunc setup and mmio setup

[Chris Wilson]

Some early initialisation functions (like intel_uc_init_early) would
like to access the table of mmio registers sorted by their powerwell,
which is currently setup later in intel_uncore_init(). Since this is a
static table that now doesn't touch hw, once upon a time we needed to
probe ivb to determine the forcewake register, but now only depends on
pciid (i.e.  gen) we can do the vfunc setup inside init_early, leaving
the mmio setup where it is.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c     | 10 ++++++----
 drivers/gpu/drm/i915/intel_uncore.c | 25 ++++++++++++++-----------
 drivers/gpu/drm/i915/intel_uncore.h |  3 ++-
 3 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 66fc156b294a..7d110797e0dc 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -898,13 +898,15 @@ 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_uc_init_early(dev_priv);
-	i915_memcpy_init_early(dev_priv);
-
 	ret = i915_workqueues_init(dev_priv);
 	if (ret < 0)
 		goto err_engines;
 
+	i915_memcpy_init_early(dev_priv);
+
+	intel_uncore_init_early(dev_priv);
+	intel_uc_init_early(dev_priv);
+
 	/* This must be called before any calls to HAS_PCH_* */
 	intel_detect_pch(dev_priv);
 
@@ -1014,7 +1016,7 @@ static int i915_driver_init_mmio(struct drm_i915_private *dev_priv)
 	if (ret < 0)
 		goto err_bridge;
 
-	intel_uncore_init(dev_priv);
+	intel_uncore_init_mmio(dev_priv);
 
 	intel_uc_init_mmio(dev_priv);
 
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 3d41667919dc..4d81bc066b37 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1247,18 +1247,8 @@ static int i915_pmic_bus_access_notifier(struct notifier_block *nb,
 	return NOTIFY_OK;
 }
 
-void intel_uncore_init(struct drm_i915_private *dev_priv)
+void intel_uncore_init_early(struct drm_i915_private *dev_priv)
 {
-	i915_check_vgpu(dev_priv);
-
-	intel_uncore_edram_detect(dev_priv);
-	intel_uncore_fw_domains_init(dev_priv);
-	__intel_uncore_early_sanitize(dev_priv, false);
-
-	dev_priv->uncore.unclaimed_mmio_check = 1;
-	dev_priv->uncore.pmic_bus_access_nb.notifier_call =
-		i915_pmic_bus_access_notifier;
-
 	if (IS_GEN(dev_priv, 2, 4) || intel_vgpu_active(dev_priv)) {
 		ASSIGN_WRITE_MMIO_VFUNCS(dev_priv, gen2);
 		ASSIGN_READ_MMIO_VFUNCS(dev_priv, gen2);
@@ -1289,6 +1279,19 @@ void intel_uncore_init(struct drm_i915_private *dev_priv)
 		ASSIGN_WRITE_MMIO_VFUNCS(dev_priv, fwtable);
 		ASSIGN_READ_MMIO_VFUNCS(dev_priv, fwtable);
 	}
+}
+
+void intel_uncore_init_mmio(struct drm_i915_private *dev_priv)
+{
+	i915_check_vgpu(dev_priv);
+
+	intel_uncore_edram_detect(dev_priv);
+	intel_uncore_fw_domains_init(dev_priv);
+	__intel_uncore_early_sanitize(dev_priv, false);
+
+	dev_priv->uncore.unclaimed_mmio_check = 1;
+	dev_priv->uncore.pmic_bus_access_nb.notifier_call =
+		i915_pmic_bus_access_notifier;
 
 	iosf_mbi_register_pmic_bus_access_notifier(
 		&dev_priv->uncore.pmic_bus_access_nb);
diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h
index 582771251b57..695ea5600469 100644
--- a/drivers/gpu/drm/i915/intel_uncore.h
+++ b/drivers/gpu/drm/i915/intel_uncore.h
@@ -128,7 +128,8 @@ struct intel_uncore {
 
 
 void intel_uncore_sanitize(struct drm_i915_private *dev_priv);
-void intel_uncore_init(struct drm_i915_private *dev_priv);
+void intel_uncore_init_early(struct drm_i915_private *dev_priv);
+void intel_uncore_init_mmio(struct drm_i915_private *dev_priv);
 bool intel_uncore_unclaimed_mmio(struct drm_i915_private *dev_priv);
 bool intel_uncore_arm_unclaimed_mmio_detection(struct drm_i915_private *dev_priv);
 void intel_uncore_fini(struct drm_i915_private *dev_priv);
-- 
2.14.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/2] drm/i915: Remove early invocations of i915_engines_cleanup()

[Chris Wilson]

Engines are now only allocated during init_mmio, so we can forgo
iterating over the empty list and calling kfree(NULL) prior to
intel_engines_init_mmio().

Fixes: 63ffbcdadcf2 ("drm/i915: Sanitize engine context sizes")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 15 ++-------------
 1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 7d110797e0dc..a45e34dcd04d 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -818,15 +818,6 @@ static int i915_workqueues_init(struct drm_i915_private *dev_priv)
 	return -ENOMEM;
 }
 
-static void i915_engines_cleanup(struct drm_i915_private *i915)
-{
-	struct intel_engine_cs *engine;
-	enum intel_engine_id id;
-
-	for_each_engine(engine, i915, id)
-		kfree(engine);
-}
-
 static void i915_workqueues_cleanup(struct drm_i915_private *dev_priv)
 {
 	destroy_workqueue(dev_priv->hotplug.dp_wq);
@@ -900,7 +891,7 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv,
 
 	ret = i915_workqueues_init(dev_priv);
 	if (ret < 0)
-		goto err_engines;
+		goto err;
 
 	i915_memcpy_init_early(dev_priv);
 
@@ -935,8 +926,7 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv,
 err_irq:
 	intel_irq_fini(dev_priv);
 	i915_workqueues_cleanup(dev_priv);
-err_engines:
-	i915_engines_cleanup(dev_priv);
+err:
 	return ret;
 }
 
@@ -950,7 +940,6 @@ static void i915_driver_cleanup_early(struct drm_i915_private *dev_priv)
 	i915_gem_load_cleanup(dev_priv);
 	intel_irq_fini(dev_priv);
 	i915_workqueues_cleanup(dev_priv);
-	i915_engines_cleanup(dev_priv);
 }
 
 static int i915_mmio_setup(struct drm_i915_private *dev_priv)
-- 
2.14.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Split uncore init into vfunc setup and mmio setup

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Split uncore init into vfunc setup and mmio setup
URL   : https://patchwork.freedesktop.org/series/31531/
State : success

== Summary ==

Series 31531v1 series starting with [1/2] drm/i915: Split uncore init into vfunc setup and mmio setup
https://patchwork.freedesktop.org/api/1.0/series/31531/revisions/1/mbox/

Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                pass       -> DMESG-WARN (fi-byt-j1900) fdo#101705

fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:462s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:479s
fi-blb-e6850     total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:394s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:578s
fi-bwr-2160      total:289  pass:183  dwarn:0   dfail:0   fail:0   skip:106 time:288s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:533s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:532s
fi-byt-j1900     total:289  pass:253  dwarn:1   dfail:0   fail:0   skip:35  time:542s
fi-byt-n2820     total:289  pass:249  dwarn:1   dfail:0   fail:0   skip:39  time:530s
fi-cfl-s         total:289  pass:256  dwarn:1   dfail:0   fail:0   skip:32  time:565s
fi-cnl-y         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:619s
fi-elk-e7500     total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:436s
fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:603s
fi-hsw-4770      total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:436s
fi-hsw-4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:420s
fi-ivb-3520m     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:510s
fi-ivb-3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:474s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:507s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:582s
fi-kbl-7567u     total:289  pass:265  dwarn:4   dfail:0   fail:0   skip:20  time:493s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:595s
fi-pnv-d510      total:289  pass:222  dwarn:1   dfail:0   fail:0   skip:66  time:657s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:478s
fi-skl-6700hq    total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:658s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:530s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:511s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:472s
fi-snb-2520m     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:585s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:0   skip:40  time:430s

aaf31e875e72b50f6a970c11f797b7f5b61a2681 drm-tip: 2017y-10m-06d-17h-24m-22s UTC integration manifest
1d08244ad22c drm/i915: Remove early invocations of i915_engines_cleanup()
77e7c98fbc62 drm/i915: Split uncore init into vfunc setup and mmio setup

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5940/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: failure for series starting with [1/2] drm/i915: Split uncore init into vfunc setup and mmio setup

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Split uncore init into vfunc setup and mmio setup
URL   : https://patchwork.freedesktop.org/series/31531/
State : failure

== Summary ==

Test kms_cursor_legacy:
        Subgroup cursorA-vs-flipA-atomic-transitions:
                fail       -> PASS       (shard-hsw) fdo#102723
Test kms_setmode:
        Subgroup basic:
                pass       -> FAIL       (shard-hsw) fdo#99912
Test perf:
        Subgroup oa-exponents:
                pass       -> FAIL       (shard-hsw)

fdo#102723 https://bugs.freedesktop.org/show_bug.cgi?id=102723
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912

shard-hsw        total:2446 pass:1327 dwarn:6   dfail:0   fail:10  skip:1103 time:10097s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5940/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Split uncore init into vfunc setup and mmio setup

[Michal Wajdeczko]

On Sat, 07 Oct 2017 10:56:58 +0200, Chris Wilson  
<chris@chris-wilson.co.uk> wrote:

> Some early initialisation functions (like intel_uc_init_early) would
> like to access the table of mmio registers sorted by their powerwell,
> which is currently setup later in intel_uncore_init(). Since this is a
> static table that now doesn't touch hw, once upon a time we needed to
> probe ivb to determine the forcewake register, but now only depends on
> pciid (i.e.  gen) we can do the vfunc setup inside init_early, leaving
> the mmio setup where it is.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c     | 10 ++++++----
>  drivers/gpu/drm/i915/intel_uncore.c | 25 ++++++++++++++-----------
>  drivers/gpu/drm/i915/intel_uncore.h |  3 ++-
>  3 files changed, 22 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c  
> b/drivers/gpu/drm/i915/i915_drv.c
> index 66fc156b294a..7d110797e0dc 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -898,13 +898,15 @@ 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_uc_init_early(dev_priv);
> -	i915_memcpy_init_early(dev_priv);
> -
>  	ret = i915_workqueues_init(dev_priv);
>  	if (ret < 0)
>  		goto err_engines;
> +	i915_memcpy_init_early(dev_priv);
> +
> +	intel_uncore_init_early(dev_priv);
> +	intel_uc_init_early(dev_priv);
> +
>  	/* This must be called before any calls to HAS_PCH_* */
>  	intel_detect_pch(dev_priv);
> @@ -1014,7 +1016,7 @@ static int i915_driver_init_mmio(struct  
> drm_i915_private *dev_priv)
>  	if (ret < 0)
>  		goto err_bridge;
> -	intel_uncore_init(dev_priv);
> +	intel_uncore_init_mmio(dev_priv);
> 	intel_uc_init_mmio(dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c  
> b/drivers/gpu/drm/i915/intel_uncore.c
> index 3d41667919dc..4d81bc066b37 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -1247,18 +1247,8 @@ static int i915_pmic_bus_access_notifier(struct  
> notifier_block *nb,
>  	return NOTIFY_OK;
>  }
> -void intel_uncore_init(struct drm_i915_private *dev_priv)
> +void intel_uncore_init_early(struct drm_i915_private *dev_priv)
>  {
> -	i915_check_vgpu(dev_priv);
> -
> -	intel_uncore_edram_detect(dev_priv);
> -	intel_uncore_fw_domains_init(dev_priv);
> -	__intel_uncore_early_sanitize(dev_priv, false);
> -
> -	dev_priv->uncore.unclaimed_mmio_check = 1;
> -	dev_priv->uncore.pmic_bus_access_nb.notifier_call =
> -		i915_pmic_bus_access_notifier;
> -
>  	if (IS_GEN(dev_priv, 2, 4) || intel_vgpu_active(dev_priv)) {
>  		ASSIGN_WRITE_MMIO_VFUNCS(dev_priv, gen2);
>  		ASSIGN_READ_MMIO_VFUNCS(dev_priv, gen2);
> @@ -1289,6 +1279,19 @@ void intel_uncore_init(struct drm_i915_private  
> *dev_priv)
>  		ASSIGN_WRITE_MMIO_VFUNCS(dev_priv, fwtable);
>  		ASSIGN_READ_MMIO_VFUNCS(dev_priv, fwtable);
>  	}
> +}
> +
> +void intel_uncore_init_mmio(struct drm_i915_private *dev_priv)
> +{
> +	i915_check_vgpu(dev_priv);
> +
> +	intel_uncore_edram_detect(dev_priv);
> +	intel_uncore_fw_domains_init(dev_priv);

Do you plan to split fw_domains_init into early/mmio parts?
Without such split use of intel_uncore_forcewake_for_reg
 from uc_init_early will still be not possible.

Michal

> +	__intel_uncore_early_sanitize(dev_priv, false);
> +
> +	dev_priv->uncore.unclaimed_mmio_check = 1;
> +	dev_priv->uncore.pmic_bus_access_nb.notifier_call =
> +		i915_pmic_bus_access_notifier;
> 	iosf_mbi_register_pmic_bus_access_notifier(
>  		&dev_priv->uncore.pmic_bus_access_nb);
> diff --git a/drivers/gpu/drm/i915/intel_uncore.h  
> b/drivers/gpu/drm/i915/intel_uncore.h
> index 582771251b57..695ea5600469 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.h
> +++ b/drivers/gpu/drm/i915/intel_uncore.h
> @@ -128,7 +128,8 @@ struct intel_uncore {
> void intel_uncore_sanitize(struct drm_i915_private *dev_priv);
> -void intel_uncore_init(struct drm_i915_private *dev_priv);
> +void intel_uncore_init_early(struct drm_i915_private *dev_priv);
> +void intel_uncore_init_mmio(struct drm_i915_private *dev_priv);
>  bool intel_uncore_unclaimed_mmio(struct drm_i915_private *dev_priv);
>  bool intel_uncore_arm_unclaimed_mmio_detection(struct drm_i915_private  
> *dev_priv);
>  void intel_uncore_fini(struct drm_i915_private *dev_priv);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Split uncore init into vfunc setup and mmio setup

[Chris Wilson]

Quoting Michal Wajdeczko (2017-10-07 12:14:30)
> On Sat, 07 Oct 2017 10:56:58 +0200, Chris Wilson  
> > +void intel_uncore_init_mmio(struct drm_i915_private *dev_priv)
> > +{
> > +     i915_check_vgpu(dev_priv);
> > +
> > +     intel_uncore_edram_detect(dev_priv);
> > +     intel_uncore_fw_domains_init(dev_priv);
> 
> Do you plan to split fw_domains_init into early/mmio parts?
> Without such split use of intel_uncore_forcewake_for_reg
>  from uc_init_early will still be not possible.

Ah, I was mistaken in thinking the vfunc tables were enough. We still
have some mmio access in fw_domains_init that we need to split into
_early and _mmio parts. On the whole, what do we think, is the motion
still justified? I think it still is -- we have a lot of static
information here that is useful during early setup.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Split uncore init into vfunc setup and mmio setup

[Joonas Lahtinen]

On Sat, 2017-10-07 at 16:28 +0100, Chris Wilson wrote:
> Quoting Michal Wajdeczko (2017-10-07 12:14:30)
> > On Sat, 07 Oct 2017 10:56:58 +0200, Chris Wilson  
> > > +void intel_uncore_init_mmio(struct drm_i915_private *dev_priv)
> > > +{
> > > +     i915_check_vgpu(dev_priv);
> > > +
> > > +     intel_uncore_edram_detect(dev_priv);
> > > +     intel_uncore_fw_domains_init(dev_priv);
> > 
> > Do you plan to split fw_domains_init into early/mmio parts?
> > Without such split use of intel_uncore_forcewake_for_reg
> >  from uc_init_early will still be not possible.
> 
> Ah, I was mistaken in thinking the vfunc tables were enough. We still
> have some mmio access in fw_domains_init that we need to split into
> _early and _mmio parts. On the whole, what do we think, is the motion
> still justified? I think it still is -- we have a lot of static
> information here that is useful during early setup.

Not sure if I fully understood the quesiton but the init code split
makes it more coherent across driver.

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

Re: [PATCH 1/2] drm/i915: Split uncore init into vfunc setup and mmio setup

[Chris Wilson]

Quoting Joonas Lahtinen (2017-10-09 10:19:40)
> On Sat, 2017-10-07 at 16:28 +0100, Chris Wilson wrote:
> > Quoting Michal Wajdeczko (2017-10-07 12:14:30)
> > > On Sat, 07 Oct 2017 10:56:58 +0200, Chris Wilson  
> > > > +void intel_uncore_init_mmio(struct drm_i915_private *dev_priv)
> > > > +{
> > > > +     i915_check_vgpu(dev_priv);
> > > > +
> > > > +     intel_uncore_edram_detect(dev_priv);
> > > > +     intel_uncore_fw_domains_init(dev_priv);
> > > 
> > > Do you plan to split fw_domains_init into early/mmio parts?
> > > Without such split use of intel_uncore_forcewake_for_reg
> > >  from uc_init_early will still be not possible.
> > 
> > Ah, I was mistaken in thinking the vfunc tables were enough. We still
> > have some mmio access in fw_domains_init that we need to split into
> > _early and _mmio parts. On the whole, what do we think, is the motion
> > still justified? I think it still is -- we have a lot of static
> > information here that is useful during early setup.
> 
> Not sure if I fully understood the quesiton but the init code split
> makes it more coherent across driver.

Yes, I was asking if the proposed fixed patch still made sense when the
immediate use case (uc_init) had already been fixed.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Remove early invocations of i915_engines_cleanup()

[Joonas Lahtinen]

On Sat, 2017-10-07 at 09:56 +0100, Chris Wilson wrote:
> Engines are now only allocated during init_mmio, so we can forgo
> iterating over the empty list and calling kfree(NULL) prior to
> intel_engines_init_mmio().
> 
> Fixes: 63ffbcdadcf2 ("drm/i915: Sanitize engine context sizes")

This use of the tag would be correct according to "Fixes:" part of:

https://www.kernel.org/doc/html/v4.13/process/submitting-patches.html#r
eviewer-s-statement-of-oversight

But it definitely says "bug" in there, so the other more stylistic
issues "Impact: None", indeed should not get a Fixes: tag.

> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

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