* [CI 2/2] HAX Enable GuC loading & submission
2017-05-02 10:32 [CI 1/2] drm/i915/guc: Enable send function only after successful init Michal Wajdeczko
@ 2017-05-02 10:32 ` Michal Wajdeczko
2017-05-02 10:50 ` ✓ Fi.CI.BAT: success for series starting with [CI,1/2] drm/i915/guc: Enable send function only after successful init Patchwork
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Michal Wajdeczko @ 2017-05-02 10:32 UTC (permalink / raw)
To: intel-gfx
This is just for CI testing, *** DO NOT MERGE ***
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
---
drivers/gpu/drm/i915/i915_params.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index b6a7e36..abd2894 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -56,8 +56,8 @@ struct i915_params i915 __read_mostly = {
.verbose_state_checks = 1,
.nuclear_pageflip = 0,
.edp_vswing = 0,
- .enable_guc_loading = 0,
- .enable_guc_submission = 0,
+ .enable_guc_loading = 1,
+ .enable_guc_submission = 1,
.guc_log_level = -1,
.guc_firmware_path = NULL,
.huc_firmware_path = NULL,
@@ -221,12 +221,12 @@ MODULE_PARM_DESC(edp_vswing,
module_param_named_unsafe(enable_guc_loading, i915.enable_guc_loading, int, 0400);
MODULE_PARM_DESC(enable_guc_loading,
"Enable GuC firmware loading "
- "(-1=auto, 0=never [default], 1=if available, 2=required)");
+ "(-1=auto, 0=never, 1=if available [default], 2=required)");
module_param_named_unsafe(enable_guc_submission, i915.enable_guc_submission, int, 0400);
MODULE_PARM_DESC(enable_guc_submission,
"Enable GuC submission "
- "(-1=auto, 0=never [default], 1=if available, 2=required)");
+ "(-1=auto, 0=never, 1=if available [default], 2=required)");
module_param_named(guc_log_level, i915.guc_log_level, int, 0400);
MODULE_PARM_DESC(guc_log_level,
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 7+ messages in thread* ✓ Fi.CI.BAT: success for series starting with [CI,1/2] drm/i915/guc: Enable send function only after successful init
2017-05-02 10:32 [CI 1/2] drm/i915/guc: Enable send function only after successful init Michal Wajdeczko
2017-05-02 10:32 ` [CI 2/2] HAX Enable GuC loading & submission Michal Wajdeczko
@ 2017-05-02 10:50 ` Patchwork
2017-05-02 12:04 ` [CI 1/2] " Chris Wilson
2017-05-03 10:47 ` Chris Wilson
3 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2017-05-02 10:50 UTC (permalink / raw)
To: Michal Wajdeczko; +Cc: intel-gfx
== Series Details ==
Series: series starting with [CI,1/2] drm/i915/guc: Enable send function only after successful init
URL : https://patchwork.freedesktop.org/series/23799/
State : success
== Summary ==
Series 23799v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/23799/revisions/1/mbox/
fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time:436s
fi-bdw-gvtdvm total:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time:428s
fi-bsw-n3050 total:278 pass:242 dwarn:0 dfail:0 fail:0 skip:36 time:574s
fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time:491s
fi-bxt-t5700 total:278 pass:258 dwarn:0 dfail:0 fail:0 skip:20 time:527s
fi-byt-j1900 total:278 pass:254 dwarn:0 dfail:0 fail:0 skip:24 time:493s
fi-byt-n2820 total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:478s
fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:414s
fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:403s
fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time:414s
fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:483s
fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:461s
fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:453s
fi-kbl-7560u total:278 pass:267 dwarn:1 dfail:0 fail:0 skip:10 time:555s
fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:438s
fi-skl-6700hq total:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time:559s
fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time:461s
fi-skl-6770hq total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:484s
fi-skl-gvtdvm total:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time:413s
fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:530s
fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time:402s
0d90375e6d38f986224e4dfce1674c5b093590cc drm-tip: 2017y-05m-02d-09h-11m-43s UTC integration manifest
ebc37af HAX Enable GuC loading & submission
66bb87c drm/i915/guc: Enable send function only after successful init
== Logs ==
For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4592/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [CI 1/2] drm/i915/guc: Enable send function only after successful init
2017-05-02 10:32 [CI 1/2] drm/i915/guc: Enable send function only after successful init Michal Wajdeczko
2017-05-02 10:32 ` [CI 2/2] HAX Enable GuC loading & submission Michal Wajdeczko
2017-05-02 10:50 ` ✓ Fi.CI.BAT: success for series starting with [CI,1/2] drm/i915/guc: Enable send function only after successful init Patchwork
@ 2017-05-02 12:04 ` Chris Wilson
2017-05-03 10:47 ` Chris Wilson
3 siblings, 0 replies; 7+ messages in thread
From: Chris Wilson @ 2017-05-02 12:04 UTC (permalink / raw)
To: Michal Wajdeczko; +Cc: intel-gfx
On Tue, May 02, 2017 at 10:32:42AM +0000, Michal Wajdeczko wrote:
> It is safer to setup valid send function after successful GuC
> hardware initialization. In addition we prepare placeholder
> where we can setup any alternate GuC communication mechanism.
>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
> drivers/gpu/drm/i915/intel_uc.c | 27 ++++++++++++++++++++++++++-
> drivers/gpu/drm/i915/intel_uc.h | 1 +
> 2 files changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> index 900e376..5957a95 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -99,7 +99,7 @@ void intel_uc_init_early(struct drm_i915_private *dev_priv)
> struct intel_guc *guc = &dev_priv->guc;
>
> mutex_init(&guc->send_mutex);
> - guc->send = intel_guc_send_mmio;
> + guc->send = intel_guc_send_nop;
> }
>
> static void fetch_uc_fw(struct drm_i915_private *dev_priv,
> @@ -252,13 +252,27 @@ void intel_uc_fini_fw(struct drm_i915_private *dev_priv)
> __intel_uc_fw_fini(&dev_priv->huc.fw);
> }
>
> +static int guc_enable_communication(struct intel_guc *guc)
> +{
> + /* XXX: placeholder for alternate setup */
> + guc->send = intel_guc_send_mmio;
> + return 0;
> +}
> +
> +static void guc_disable_communication(struct intel_guc *guc)
> +{
> + guc->send = intel_guc_send_nop;
> +}
> +
> int intel_uc_init_hw(struct drm_i915_private *dev_priv)
> {
> + struct intel_guc *guc = &dev_priv->guc;
> int ret, attempts;
>
> if (!i915.enable_guc_loading)
> return 0;
>
> + guc_disable_communication(guc);
> gen9_reset_guc_interrupts(dev_priv);
>
> /* We need to notify the guc whenever we change the GGTT */
> @@ -308,6 +322,10 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
> if (ret)
> goto err_submission;
>
> + ret = guc_enable_communication(guc);
> + if (ret)
> + goto err_submission;
> +
> intel_guc_auth_huc(dev_priv);
> if (i915.enable_guc_submission) {
> if (i915.guc_log_level >= 0)
> @@ -330,6 +348,7 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
> * marks the GPU as wedged until reset).
> */
> err_interrupts:
> + guc_disable_communication(guc);
> gen9_disable_guc_interrupts(dev_priv);
> err_submission:
> if (i915.enable_guc_submission)
> @@ -364,6 +383,12 @@ void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
> i915_ggtt_disable_guc(dev_priv);
> }
>
> +int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len)
> +{
> + WARN(1, "Unexpected send: action=%#x\n", *action);
> + return -ENOSYS;
For internal errors we use -ENODEV.
I fixed that whilst applying as I'm sure no one else but checkpatch
cares for a path that should never occur.
Applied and pushed, thanks for the patch and review.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [CI 1/2] drm/i915/guc: Enable send function only after successful init
2017-05-02 10:32 [CI 1/2] drm/i915/guc: Enable send function only after successful init Michal Wajdeczko
` (2 preceding siblings ...)
2017-05-02 12:04 ` [CI 1/2] " Chris Wilson
@ 2017-05-03 10:47 ` Chris Wilson
2017-05-03 15:54 ` Michal Wajdeczko
3 siblings, 1 reply; 7+ messages in thread
From: Chris Wilson @ 2017-05-03 10:47 UTC (permalink / raw)
To: Michal Wajdeczko; +Cc: intel-gfx
On Tue, May 02, 2017 at 10:32:42AM +0000, Michal Wajdeczko wrote:
> It is safer to setup valid send function after successful GuC
> hardware initialization. In addition we prepare placeholder
> where we can setup any alternate GuC communication mechanism.
>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
> drivers/gpu/drm/i915/intel_uc.c | 27 ++++++++++++++++++++++++++-
> drivers/gpu/drm/i915/intel_uc.h | 1 +
> 2 files changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> index 900e376..5957a95 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -99,7 +99,7 @@ void intel_uc_init_early(struct drm_i915_private *dev_priv)
> struct intel_guc *guc = &dev_priv->guc;
>
> mutex_init(&guc->send_mutex);
> - guc->send = intel_guc_send_mmio;
> + guc->send = intel_guc_send_nop;
> }
>
> static void fetch_uc_fw(struct drm_i915_private *dev_priv,
> @@ -252,13 +252,27 @@ void intel_uc_fini_fw(struct drm_i915_private *dev_priv)
> __intel_uc_fw_fini(&dev_priv->huc.fw);
> }
>
> +static int guc_enable_communication(struct intel_guc *guc)
> +{
> + /* XXX: placeholder for alternate setup */
> + guc->send = intel_guc_send_mmio;
> + return 0;
> +}
> +
> +static void guc_disable_communication(struct intel_guc *guc)
> +{
> + guc->send = intel_guc_send_nop;
> +}
> +
> int intel_uc_init_hw(struct drm_i915_private *dev_priv)
> {
> + struct intel_guc *guc = &dev_priv->guc;
> int ret, attempts;
>
> if (!i915.enable_guc_loading)
> return 0;
>
> + guc_disable_communication(guc);
> gen9_reset_guc_interrupts(dev_priv);
>
> /* We need to notify the guc whenever we change the GGTT */
> @@ -308,6 +322,10 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
> if (ret)
> goto err_submission;
>
> + ret = guc_enable_communication(guc);
> + if (ret)
> + goto err_submission;
> +
> intel_guc_auth_huc(dev_priv);
> if (i915.enable_guc_submission) {
> if (i915.guc_log_level >= 0)
> @@ -330,6 +348,7 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
> * marks the GPU as wedged until reset).
> */
> err_interrupts:
> + guc_disable_communication(guc);
> gen9_disable_guc_interrupts(dev_priv);
> err_submission:
> if (i915.enable_guc_submission)
> @@ -364,6 +383,12 @@ void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
> i915_ggtt_disable_guc(dev_priv);
> }
>
> +int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len)
> +{
> + WARN(1, "Unexpected send: action=%#x\n", *action);
> + return -ENOSYS;
> +}
> +
> /*
> * This function implements the MMIO based host to GuC interface.
> */
> diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
> index 2f0229d..1e0eecd 100644
> --- a/drivers/gpu/drm/i915/intel_uc.h
> +++ b/drivers/gpu/drm/i915/intel_uc.h
> @@ -227,6 +227,7 @@ void intel_uc_fini_fw(struct drm_i915_private *dev_priv);
> int intel_uc_init_hw(struct drm_i915_private *dev_priv);
> void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
> int intel_guc_sample_forcewake(struct intel_guc *guc);
> +int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len);
Was there a reason for exporting the nop? Its a purely internal BUG_ON()
function that should only be set when disabling the guc. Afaict, it
should be private.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [CI 1/2] drm/i915/guc: Enable send function only after successful init
2017-05-03 10:47 ` Chris Wilson
@ 2017-05-03 15:54 ` Michal Wajdeczko
2017-05-03 16:02 ` Chris Wilson
0 siblings, 1 reply; 7+ messages in thread
From: Michal Wajdeczko @ 2017-05-03 15:54 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
On Wed, May 03, 2017 at 11:47:12AM +0100, Chris Wilson wrote:
> On Tue, May 02, 2017 at 10:32:42AM +0000, Michal Wajdeczko wrote:
> > It is safer to setup valid send function after successful GuC
> > hardware initialization. In addition we prepare placeholder
> > where we can setup any alternate GuC communication mechanism.
> >
> > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_uc.c | 27 ++++++++++++++++++++++++++-
> > drivers/gpu/drm/i915/intel_uc.h | 1 +
> > 2 files changed, 27 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> > index 900e376..5957a95 100644
> > --- a/drivers/gpu/drm/i915/intel_uc.c
> > +++ b/drivers/gpu/drm/i915/intel_uc.c
> > @@ -99,7 +99,7 @@ void intel_uc_init_early(struct drm_i915_private *dev_priv)
> > struct intel_guc *guc = &dev_priv->guc;
> >
> > mutex_init(&guc->send_mutex);
> > - guc->send = intel_guc_send_mmio;
> > + guc->send = intel_guc_send_nop;
> > }
> >
> > static void fetch_uc_fw(struct drm_i915_private *dev_priv,
> > @@ -252,13 +252,27 @@ void intel_uc_fini_fw(struct drm_i915_private *dev_priv)
> > __intel_uc_fw_fini(&dev_priv->huc.fw);
> > }
> >
> > +static int guc_enable_communication(struct intel_guc *guc)
> > +{
> > + /* XXX: placeholder for alternate setup */
> > + guc->send = intel_guc_send_mmio;
> > + return 0;
> > +}
> > +
> > +static void guc_disable_communication(struct intel_guc *guc)
> > +{
> > + guc->send = intel_guc_send_nop;
> > +}
> > +
> > int intel_uc_init_hw(struct drm_i915_private *dev_priv)
> > {
> > + struct intel_guc *guc = &dev_priv->guc;
> > int ret, attempts;
> >
> > if (!i915.enable_guc_loading)
> > return 0;
> >
> > + guc_disable_communication(guc);
> > gen9_reset_guc_interrupts(dev_priv);
> >
> > /* We need to notify the guc whenever we change the GGTT */
> > @@ -308,6 +322,10 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
> > if (ret)
> > goto err_submission;
> >
> > + ret = guc_enable_communication(guc);
> > + if (ret)
> > + goto err_submission;
> > +
> > intel_guc_auth_huc(dev_priv);
> > if (i915.enable_guc_submission) {
> > if (i915.guc_log_level >= 0)
> > @@ -330,6 +348,7 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
> > * marks the GPU as wedged until reset).
> > */
> > err_interrupts:
> > + guc_disable_communication(guc);
> > gen9_disable_guc_interrupts(dev_priv);
> > err_submission:
> > if (i915.enable_guc_submission)
> > @@ -364,6 +383,12 @@ void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
> > i915_ggtt_disable_guc(dev_priv);
> > }
> >
> > +int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len)
> > +{
> > + WARN(1, "Unexpected send: action=%#x\n", *action);
> > + return -ENOSYS;
> > +}
> > +
> > /*
> > * This function implements the MMIO based host to GuC interface.
> > */
> > diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
> > index 2f0229d..1e0eecd 100644
> > --- a/drivers/gpu/drm/i915/intel_uc.h
> > +++ b/drivers/gpu/drm/i915/intel_uc.h
> > @@ -227,6 +227,7 @@ void intel_uc_fini_fw(struct drm_i915_private *dev_priv);
> > int intel_uc_init_hw(struct drm_i915_private *dev_priv);
> > void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
> > int intel_guc_sample_forcewake(struct intel_guc *guc);
> > +int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len);
>
> Was there a reason for exporting the nop? Its a purely internal BUG_ON()
> function that should only be set when disabling the guc. Afaict, it
> should be private.
By looking only at this patch, agree, this nop function should be private.
But at the same time, this function, to some extend, is similar to the send_mmio
function, that we agree to keep it exposed, as it also may be needed in other
code paths (like upcoming alternate GuC communication implementation) in case
of the recovery or other plumbing.
I should mention that in the commit message to avoid your concern ;)
-Michal
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [CI 1/2] drm/i915/guc: Enable send function only after successful init
2017-05-03 15:54 ` Michal Wajdeczko
@ 2017-05-03 16:02 ` Chris Wilson
0 siblings, 0 replies; 7+ messages in thread
From: Chris Wilson @ 2017-05-03 16:02 UTC (permalink / raw)
To: Michal Wajdeczko; +Cc: intel-gfx
On Wed, May 03, 2017 at 05:54:43PM +0200, Michal Wajdeczko wrote:
> On Wed, May 03, 2017 at 11:47:12AM +0100, Chris Wilson wrote:
> > On Tue, May 02, 2017 at 10:32:42AM +0000, Michal Wajdeczko wrote:
> > > It is safer to setup valid send function after successful GuC
> > > hardware initialization. In addition we prepare placeholder
> > > where we can setup any alternate GuC communication mechanism.
> > >
> > > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > > Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > > ---
> > > drivers/gpu/drm/i915/intel_uc.c | 27 ++++++++++++++++++++++++++-
> > > drivers/gpu/drm/i915/intel_uc.h | 1 +
> > > 2 files changed, 27 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> > > index 900e376..5957a95 100644
> > > --- a/drivers/gpu/drm/i915/intel_uc.c
> > > +++ b/drivers/gpu/drm/i915/intel_uc.c
> > > @@ -99,7 +99,7 @@ void intel_uc_init_early(struct drm_i915_private *dev_priv)
> > > struct intel_guc *guc = &dev_priv->guc;
> > >
> > > mutex_init(&guc->send_mutex);
> > > - guc->send = intel_guc_send_mmio;
> > > + guc->send = intel_guc_send_nop;
> > > }
> > >
> > > static void fetch_uc_fw(struct drm_i915_private *dev_priv,
> > > @@ -252,13 +252,27 @@ void intel_uc_fini_fw(struct drm_i915_private *dev_priv)
> > > __intel_uc_fw_fini(&dev_priv->huc.fw);
> > > }
> > >
> > > +static int guc_enable_communication(struct intel_guc *guc)
> > > +{
> > > + /* XXX: placeholder for alternate setup */
> > > + guc->send = intel_guc_send_mmio;
> > > + return 0;
> > > +}
> > > +
> > > +static void guc_disable_communication(struct intel_guc *guc)
> > > +{
> > > + guc->send = intel_guc_send_nop;
> > > +}
> > > +
> > > int intel_uc_init_hw(struct drm_i915_private *dev_priv)
> > > {
> > > + struct intel_guc *guc = &dev_priv->guc;
> > > int ret, attempts;
> > >
> > > if (!i915.enable_guc_loading)
> > > return 0;
> > >
> > > + guc_disable_communication(guc);
> > > gen9_reset_guc_interrupts(dev_priv);
> > >
> > > /* We need to notify the guc whenever we change the GGTT */
> > > @@ -308,6 +322,10 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
> > > if (ret)
> > > goto err_submission;
> > >
> > > + ret = guc_enable_communication(guc);
> > > + if (ret)
> > > + goto err_submission;
> > > +
> > > intel_guc_auth_huc(dev_priv);
> > > if (i915.enable_guc_submission) {
> > > if (i915.guc_log_level >= 0)
> > > @@ -330,6 +348,7 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
> > > * marks the GPU as wedged until reset).
> > > */
> > > err_interrupts:
> > > + guc_disable_communication(guc);
> > > gen9_disable_guc_interrupts(dev_priv);
> > > err_submission:
> > > if (i915.enable_guc_submission)
> > > @@ -364,6 +383,12 @@ void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
> > > i915_ggtt_disable_guc(dev_priv);
> > > }
> > >
> > > +int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len)
> > > +{
> > > + WARN(1, "Unexpected send: action=%#x\n", *action);
> > > + return -ENOSYS;
> > > +}
> > > +
> > > /*
> > > * This function implements the MMIO based host to GuC interface.
> > > */
> > > diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
> > > index 2f0229d..1e0eecd 100644
> > > --- a/drivers/gpu/drm/i915/intel_uc.h
> > > +++ b/drivers/gpu/drm/i915/intel_uc.h
> > > @@ -227,6 +227,7 @@ void intel_uc_fini_fw(struct drm_i915_private *dev_priv);
> > > int intel_uc_init_hw(struct drm_i915_private *dev_priv);
> > > void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
> > > int intel_guc_sample_forcewake(struct intel_guc *guc);
> > > +int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len);
> >
> > Was there a reason for exporting the nop? Its a purely internal BUG_ON()
> > function that should only be set when disabling the guc. Afaict, it
> > should be private.
>
> By looking only at this patch, agree, this nop function should be private.
>
> But at the same time, this function, to some extend, is similar to the send_mmio
> function, that we agree to keep it exposed, as it also may be needed in other
> code paths (like upcoming alternate GuC communication implementation) in case
> of the recovery or other plumbing.
>
> I should mention that in the commit message to avoid your concern ;)
Always a benefit to mention the grand design if things look out of place
in individual patches. I'll be surprised if you can conjure up a
situation where we want to call intel_guc_send_nop() :)
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread