Re: [Intel-gfx] [PATCH v2 08/22] drm/i915/guc: Update GuC sample-forcewake command

[John Spotswood]

On Fri, 2019-04-12 at 17:10 -0700, Ceraolo Spurio, Daniele wrote:
> 
> On 4/11/19 1:44 AM, Michal Wajdeczko wrote:
> > 
> > New GuC firmwares use different action code value for this command.
> > 
> > Signed-off-by: Michal Wajdeczko 
> > Cc: John Spotswood 
> > Cc: Daniele Ceraolo Spurio 
> Reviewed-by: Daniele Ceraolo Spurio 

Reviewed-by: John Spotswood 

> 
> > 
> > ---
> >   drivers/gpu/drm/i915/intel_guc_fwif.h | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h
> > b/drivers/gpu/drm/i915/intel_guc_fwif.h
> > index 25d57c819e3f..dd9d99dc2aca 100644
> > --- a/drivers/gpu/drm/i915/intel_guc_fwif.h
> > +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
> > @@ -620,7 +620,6 @@ enum intel_guc_action {
> >     INTEL_GUC_ACTION_DEFAULT = 0x0,
> >     INTEL_GUC_ACTION_REQUEST_PREEMPTION = 0x2,
> >     INTEL_GUC_ACTION_REQUEST_ENGINE_RESET = 0x3,
> > -   INTEL_GUC_ACTION_SAMPLE_FORCEWAKE = 0x6,
> >     INTEL_GUC_ACTION_ALLOCATE_DOORBELL = 0x10,
> >     INTEL_GUC_ACTION_DEALLOCATE_DOORBELL = 0x20,
> >     INTEL_GUC_ACTION_LOG_BUFFER_FILE_FLUSH_COMPLETE = 0x30,
> > @@ -628,6 +627,7 @@ enum intel_guc_action {
> >     INTEL_GUC_ACTION_ENTER_S_STATE = 0x501,
> >     INTEL_GUC_ACTION_EXIT_S_STATE = 0x502,
> >     INTEL_GUC_ACTION_SLPC_REQUEST = 0x3003,
> > +   INTEL_GUC_ACTION_SAMPLE_FORCEWAKE = 0x3005,
> >     INTEL_GUC_ACTION_AUTHENTICATE_HUC = 0x4000,
> >     INTEL_GUC_ACTION_REGISTER_COMMAND_TRANSPORT_BUFFER =
> > 0x4505,
> >     INTEL_GUC_ACTION_DEREGISTER_COMMAND_TRANSPORT_BUFFER =
> > 0x4506,
> > 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 10/22] drm/i915/guc: Always ask GuC to update power domain states

[John Spotswood]

On Mon, 2019-04-15 at 13:46 -0700, Ceraolo Spurio, Daniele wrote:
> 
> On 4/11/19 1:44 AM, Michal Wajdeczko wrote:
> > 
> > With newer GuC firmware it is always ok to ask GuC to update power
> > domain states. Make it an unconditional initialization step.
> > 
> > Signed-off-by: Michal Wajdeczko 
> > Cc: Daniele Ceraolo Spurio 
> > Cc: John Spotswood 
> Reviewed-by: Daniele Ceraolo Spurio 
> 
> Daniele

Reviewed-by: John Spotswood 

> 
> > 
> > ---
> >   drivers/gpu/drm/i915/intel_guc_submission.c | 4 
> >   drivers/gpu/drm/i915/intel_uc.c | 8 
> >   2 files changed, 4 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c
> > b/drivers/gpu/drm/i915/intel_guc_submission.c
> > index dea87253d141..856505dbbe91 100644
> > --- a/drivers/gpu/drm/i915/intel_guc_submission.c
> > +++ b/drivers/gpu/drm/i915/intel_guc_submission.c
> > @@ -1319,10 +1319,6 @@ int intel_guc_submission_enable(struct
> > intel_guc *guc)
> >   
> >     GEM_BUG_ON(!guc->execbuf_client);
> >   
> > -   err = intel_guc_sample_forcewake(guc);
> > -   if (err)
> > -   return err;
> > -
> >     err = guc_clients_enable(guc);
> >     if (err)
> >     return err;
> > diff --git a/drivers/gpu/drm/i915/intel_uc.c
> > b/drivers/gpu/drm/i915/intel_uc.c
> > index 21310b917ccc..8e5e4226df53 100644
> > --- a/drivers/gpu/drm/i915/intel_uc.c
> > +++ b/drivers/gpu/drm/i915/intel_uc.c
> > @@ -405,14 +405,14 @@ int intel_uc_init_hw(struct drm_i915_private
> > *i915)
> >     goto err_communication;
> >     }
> >   
> > +   ret = intel_guc_sample_forcewake(guc);
> > +   if (ret)
> > +   goto err_communication;
> > +
> >     if (USES_GUC_SUBMISSION(i915)) {
> >     ret = intel_guc_submission_enable(guc);
> >     if (ret)
> >     goto err_communication;
> > -   } else if (INTEL_GEN(i915) < 11) {
> > -   ret = intel_guc_sample_forcewake(guc);
> > -   if (ret)
> > -   goto err_communication;
> >     }
> >   
> >     dev_info(i915->drm.dev, "GuC firmware version %u.%u\n",
> > 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2] drm/i915/guc: updated suspend/resume protocol

[John Spotswood]

On Fri, 2019-04-12 at 17:20 -0700, Ceraolo Spurio, Daniele wrote:
> From: Michal Wajdeczko 
> 
> New GuC firmwares use updated sleep status definitions.
> The polling on scratch register 14 is also now required only on
> suspend
> and there is no need to provide the shared page.
> 
> v2: include changes for polling and shared page
> 
> Signed-off-by: Michal Wajdeczko 
> Signed-off-by: Daniele Ceraolo Spurio  m>
> Cc: Joonas Lahtinen 
> Cc: John Spotswood 

Reviewed-by: John Spotswood 

> ---
>  drivers/gpu/drm/i915/intel_guc.c  | 50 +++
> 
>  drivers/gpu/drm/i915/intel_guc_fwif.h |  6 ++--
>  2 files changed, 24 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc.c
> b/drivers/gpu/drm/i915/intel_guc.c
> index 483c7019f817..cf943eb7537c 100644
> --- a/drivers/gpu/drm/i915/intel_guc.c
> +++ b/drivers/gpu/drm/i915/intel_guc.c
> @@ -539,25 +539,33 @@ int intel_guc_auth_huc(struct intel_guc *guc,
> u32 rsa_offset)
>   return intel_guc_send(guc, action, ARRAY_SIZE(action));
>  }
>  
> -/*
> - * The ENTER/EXIT_S_STATE actions queue the save/restore operation
> in GuC FW and
> - * then return, so waiting on the H2G is not enough to guarantee GuC
> is done.
> - * When all the processing is done, GuC writes
> INTEL_GUC_SLEEP_STATE_SUCCESS to
> - * scratch register 14, so we can poll on that. Note that GuC does
> not ensure
> - * that the value in the register is different from
> - * INTEL_GUC_SLEEP_STATE_SUCCESS while the action is in progress so
> we need to
> - * take care of that ourselves as well.
> +/**
> + * intel_guc_suspend() - notify GuC entering suspend state
> + * @guc: the guc
>   */
> -static int guc_sleep_state_action(struct intel_guc *guc,
> -   const u32 *action, u32 len)
> +int intel_guc_suspend(struct intel_guc *guc)
>  {
>   struct drm_i915_private *dev_priv = guc_to_i915(guc);
>   int ret;
>   u32 status;
> + u32 action[] = {
> + INTEL_GUC_ACTION_ENTER_S_STATE,
> + GUC_POWER_D1, /* any value greater than GUC_POWER_D0
> */
> + };
> +
> + /*
> +  * The ENTER_S_STATE action queues the save/restore
> operation in GuC FW
> +  * and then returns, so waiting on the H2G is not enough to
> guarantee
> +  * GuC is done. When all the processing is done, GuC writes
> +  * INTEL_GUC_SLEEP_STATE_SUCCESS to scratch register 14, so
> we can poll
> +  * on that. Note that GuC does not ensure that the value in
> the register
> +  * is different from INTEL_GUC_SLEEP_STATE_SUCCESS while the
> action is
> +  * in progress so we need to take care of that ourselves as
> well.
> +  */
>  
>   I915_WRITE(SOFT_SCRATCH(14),
> INTEL_GUC_SLEEP_STATE_INVALID_MASK);
>  
> - ret = intel_guc_send(guc, action, len);
> + ret = intel_guc_send(guc, action, ARRAY_SIZE(action));
>   if (ret)
>   return ret;
>  
> @@ -577,21 +585,6 @@ static int guc_sleep_state_action(struct
> intel_guc *guc,
>   return 0;
>  }
>  
> -/**
> - * intel_guc_suspend() - notify GuC entering suspend state
> - * @guc: the guc
> - */
> -int intel_guc_suspend(struct intel_guc *guc)
> -{
> - u32 data[] = {
> - INTEL_GUC_ACTION_ENTER_S_STATE,
> - GUC_POWER_D1, /* any value greater than GUC_POWER_D0
> */
> - intel_guc_ggtt_offset(guc, guc->shared_data)
> - };
> -
> - return guc_sleep_state_action(guc, data, ARRAY_SIZE(data));
> -}
> -
>  /**
>   * intel_guc_reset_engine() - ask GuC to reset an engine
>   * @guc: intel_guc structure
> @@ -621,13 +614,12 @@ int intel_guc_reset_engine(struct intel_guc
> *guc,
>   */
>  int intel_guc_resume(struct intel_guc *guc)
>  {
> - u32 data[] = {
> + u32 action[] = {
>   INTEL_GUC_ACTION_EXIT_S_STATE,
>   GUC_POWER_D0,
> - intel_guc_ggtt_offset(guc, guc->shared_data)
>   };
>  
> - return guc_sleep_state_action(guc, data, ARRAY_SIZE(data));
> + return intel_guc_send(guc, action, ARRAY_SIZE(action));
>  }
>  
>  /**
> diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h
> b/drivers/gpu/drm/i915/intel_guc_fwif.h
> index 64b56da9775c..25d57c819e3f 100644
> --- a/drivers/gpu/drm/i915/intel_guc_fwif.h
> +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
> @@ -648,9 +648,9 @@ enum intel_guc_report_status {
>  };
>  
>  enum intel_guc_sleep_state_status {
> - INTEL_GUC_SLEEP_STATE_SUCCESS = 0x0,
> - INTEL_GUC_SLEEP_STATE_PREEMPT_TO_IDLE_FAILED = 0x1,
> - INTEL_GUC_SLEEP_STATE_ENGINE_RESET_FAILED = 0x2
> + INTEL_GUC_SLEEP_STATE_SUCCESS = 0x1,
> + INTEL_GUC_SLEEP_STATE_PREEMPT_TO_IDLE_FAILED = 0x2,
> + INTEL_GUC_SLEEP_STATE_ENGINE_RESET_FAILED = 0x3
>  #define INTEL_GUC_SLEEP_STATE_INVALID_MASK 0x8000
>  };
>  
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 07/22] drm/i915/guc: Update GuC sleep status values

[John Spotswood]

On Fri, 2019-04-12 at 17:24 -0700, Ceraolo Spurio, Daniele wrote:
> 
> On 4/12/19 5:06 PM, Daniele Ceraolo Spurio wrote:
> > 
> > 
> > 
> > On 4/11/19 1:44 AM, Michal Wajdeczko wrote:
> > > 
> > > New GuC firmwares use updated sleep status definitions.
> > > 
> > There is also no need to poll on resume anymore. We're not failing
> > on it 
> > in CI because the wait timeout comes out as a debug message and the
> > guc 
> > is obviously still fine and responsive since we waited for nothing.
> > 
> > I think I had sent you a patch for this already, let me see if I
> > can 
> > find it again and send it in reply to this one (if I do find it,
> > I'm 
> > going to re-compile test it only).
> > 
> > Daniele
> > 
> One more thing: John S had mentioned that the guc suspend/resume 
> protocol mainly handles submission-related data, so it should be 
> possible to skip it when in huc-only mode. Not something that needs
> to 
> be included here, but a possible follow up optimization.
> 
> John, can you confirm this?
> 
> Thanks,
> Daniele
> 

If running with HuC authentication only, the GuC will have nothing to
do once authentication is completed, so GuC will go to sleep on its
own.  The driver can remove those suspend/resume calls if GuC
submission is disabled.

John


> > 
> > > 
> > > Signed-off-by: Michal Wajdeczko 
> > > Cc: Joonas Lahtinen 
> > > Cc: Daniele Ceraolo Spurio 
> > > Cc: John Spotswood 
> > > ---
> > >   drivers/gpu/drm/i915/intel_guc_fwif.h | 6 +++---
> > >   1 file changed, 3 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h 
> > > b/drivers/gpu/drm/i915/intel_guc_fwif.h
> > > index 64b56da9775c..25d57c819e3f 100644
> > > --- a/drivers/gpu/drm/i915/intel_guc_fwif.h
> > > +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
> > > @@ -648,9 +648,9 @@ enum intel_guc_report_status {
> > >   };
> > >   enum intel_guc_sleep_state_status {
> > > -    INTEL_GUC_SLEEP_STATE_SUCCESS = 0x0,
> > > -    INTEL_GUC_SLEEP_STATE_PREEMPT_TO_IDLE_FAILED = 0x1,
> > > -    INTEL_GUC_SLEEP_STATE_ENGINE_RESET_FAILED = 0x2
> > > +    INTEL_GUC_SLEEP_STATE_SUCCESS = 0x1,
> > > +    INTEL_GUC_SLEEP_STATE_PREEMPT_TO_IDLE_FAILED = 0x2,
> > > +    INTEL_GUC_SLEEP_STATE_ENGINE_RESET_FAILED = 0x3
> > >   #define INTEL_GUC_SLEEP_STATE_INVALID_MASK 0x8000
> > >   };
> > > 
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 19/21] drm/i915/huc: New HuC status register for Gen11

[John Spotswood]

On Wed, 2018-08-29 at 12:18 -0700, Wajdeczko, Michal wrote:
> Gen11 defines new register for checking HuC authentication status.
> Look into the right register and bit.
> 
> BSpec: 19686
> 
> Signed-off-by: Michal Wajdeczko 
> Cc: Joonas Lahtinen 
> Cc: Rodrigo Vivi 
> Cc: Tony Ye 
> Cc: Vinay Belgaumkar 
> Cc: Michel Thierry 
> Cc: John Spotswood 
> Cc: Anusha Srivatsa 

Reviewed-by: John Spotswood 

> ---
>  drivers/gpu/drm/i915/intel_guc_reg.h |  3 ++
>  drivers/gpu/drm/i915/intel_huc.c | 58
> +++-
>  2 files changed, 53 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc_reg.h
> b/drivers/gpu/drm/i915/intel_guc_reg.h
> index 2149209..de36595 100644
> --- a/drivers/gpu/drm/i915/intel_guc_reg.h
> +++ b/drivers/gpu/drm/i915/intel_guc_reg.h
> @@ -79,6 +79,9 @@
>  #define HUC_STATUS2 _MMIO(0xD3B0)
>  #define   HUC_FW_VERIFIED   (1<<7)
>  
> +#define GEN11_HUC_KERNEL_LOAD_INFO   _MMIO(0xC1DC)
> +#define   HUC_LOAD_SUCCESSFUL  (1 << 0)
> +
>  #define GUC_WOPCM_SIZE   _MMIO(0xc050)
>  #define   GUC_WOPCM_SIZE_LOCKED    (1<<0)
>  #define   GUC_WOPCM_SIZE_SHIFT   12
> diff --git a/drivers/gpu/drm/i915/intel_huc.c
> b/drivers/gpu/drm/i915/intel_huc.c
> index 37ef540d..a710c0d 100644
> --- a/drivers/gpu/drm/i915/intel_huc.c
> +++ b/drivers/gpu/drm/i915/intel_huc.c
> @@ -40,6 +40,47 @@ int intel_huc_init_misc(struct intel_huc *huc)
>   return 0;
>  }
>  
> +static int gen8_huc_wait_verified(struct intel_huc *huc)
> +{
> + struct drm_i915_private *i915 = huc_to_i915(huc);
> + u32 status;
> + int ret;
> +
> + ret = __intel_wait_for_register(i915,
> + HUC_STATUS2,
> + HUC_FW_VERIFIED,
> + HUC_FW_VERIFIED,
> + 2, 50, );
> + if (ret)
> + DRM_ERROR("HuC: status %#x\n", status);
> + return ret;
> +}
> +
> +static int gen11_huc_wait_verified(struct intel_huc *huc)
> +{
> + struct drm_i915_private *i915 = huc_to_i915(huc);
> + int ret;
> +
> + ret = __intel_wait_for_register(i915,
> + GEN11_HUC_KERNEL_LOAD_INFO,
> + HUC_LOAD_SUCCESSFUL,
> + HUC_LOAD_SUCCESSFUL,
> + 2, 50, NULL);
> + return ret;
> +}
> +
> +static int huc_wait_verified(struct intel_huc *huc)
> +{
> + struct drm_i915_private *i915 = huc_to_i915(huc);
> + int ret;
> +
> + if (INTEL_GEN(i915) >= 11)
> + ret = gen11_huc_wait_verified(huc);
> + else
> + ret = gen8_huc_wait_verified(huc);
> + return ret;
> +}
> +
>  /**
>   * intel_huc_auth() - Authenticate HuC uCode
>   * @huc: intel_huc structure
> @@ -56,7 +97,6 @@ int intel_huc_auth(struct intel_huc *huc)
>   struct drm_i915_private *i915 = huc_to_i915(huc);
>   struct intel_guc *guc = >guc;
>   struct i915_vma *vma;
> - u32 status;
>   int ret;
>  
>   if (huc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
> @@ -79,13 +119,9 @@ int intel_huc_auth(struct intel_huc *huc)
>   }
>  
>   /* Check authentication status, it should be done by now */
> - ret = __intel_wait_for_register(i915,
> - HUC_STATUS2,
> - HUC_FW_VERIFIED,
> - HUC_FW_VERIFIED,
> - 2, 50, );
> + ret = huc_wait_verified(huc);
>   if (ret) {
> - DRM_ERROR("HuC: Firmware not verified %#x\n",
> status);
> + DRM_ERROR("HuC: Firmware not verified %d\n", ret);
>   goto fail_unpin;
>   }
>  
> @@ -120,7 +156,13 @@ int intel_huc_check_status(struct intel_huc
> *huc)
>   return -ENODEV;
>  
>   intel_runtime_pm_get(dev_priv);
> - status = I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED;
> +
> + if (INTEL_GEN(dev_priv) >= 11)
> + status = I915_READ(GEN11_HUC_KERNEL_LOAD_INFO) &
> +  HUC_LOAD_SUCCESSFUL;
> + else
> + status = I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED;
> +
>   intel_runtime_pm_put(dev_priv);
>  
>   return status;
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 03/21] drm/i915/guc: Simplify preparation of GuC parameter block

[John Spotswood]

On Wed, 2018-08-29 at 12:10 -0700, Wajdeczko, Michal wrote:
> Definition of the parameters block passed to GuC is about to change.
> Slightly refactor code now to make upcoming patch smaller.
> 
> Signed-off-by: Michal Wajdeczko 
> Cc: Joonas Lahtinen 
> Cc: John Spotswood 

Reviewed-by: John Spotswood 

> ---
>  drivers/gpu/drm/i915/intel_guc.c | 38 +++---
> 
>  1 file changed, 23 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc.c
> b/drivers/gpu/drm/i915/intel_guc.c
> index 230aea6..982bcc8 100644
> --- a/drivers/gpu/drm/i915/intel_guc.c
> +++ b/drivers/gpu/drm/i915/intel_guc.c
> @@ -320,19 +320,8 @@ static u32 guc_ctl_log_params_flags(struct
> intel_guc *guc)
>   return flags;
>  }
>  
> -/*
> - * Initialise the GuC parameter block before starting the firmware
> - * transfer. These parameters are read by the firmware on startup
> - * and cannot be changed thereafter.
> - */
> -void intel_guc_init_params(struct intel_guc *guc)
> +static void guc_prepare_params(struct intel_guc *guc, u32 *params)
>  {
> - struct drm_i915_private *dev_priv = guc_to_i915(guc);
> - u32 params[GUC_CTL_MAX_DWORDS];
> - int i;
> -
> - memset(params, 0, sizeof(params));
> -
>   /*
>    * GuC ARAT increment is 10 ns. GuC default scheduler
> quantum is one
>    * second. This ARAR is calculated by:
> @@ -347,9 +336,12 @@ void intel_guc_init_params(struct intel_guc
> *guc)
>   params[GUC_CTL_LOG_PARAMS]  = guc_ctl_log_params_flags(guc);
>   params[GUC_CTL_DEBUG] = guc_ctl_debug_flags(guc);
>   params[GUC_CTL_CTXINFO] = guc_ctl_ctxinfo_flags(guc);
> +}
>  
> - for (i = 0; i < GUC_CTL_MAX_DWORDS; i++)
> - DRM_DEBUG_DRIVER("param[%2d] = %#x\n", i,
> params[i]);
> +static void guc_write_params(struct intel_guc *guc, const u32
> *params)
> +{
> + struct drm_i915_private *dev_priv = guc_to_i915(guc);
> + int i;
>  
>   /*
>    * All SOFT_SCRATCH registers are in FORCEWAKE_BLITTER
> domain and
> @@ -360,12 +352,28 @@ void intel_guc_init_params(struct intel_guc
> *guc)
>  
>   I915_WRITE(SOFT_SCRATCH(0), 0);
>  
> - for (i = 0; i < GUC_CTL_MAX_DWORDS; i++)
> + for (i = 0; i < GUC_CTL_MAX_DWORDS; i++) {
> + DRM_DEBUG_DRIVER("param[%2d] = %#x\n", i,
> params[i]);
>   I915_WRITE(SOFT_SCRATCH(1 + i), params[i]);
> + }
>  
>   intel_uncore_forcewake_put(dev_priv, FORCEWAKE_BLITTER);
>  }
>  
> +/*
> + * Initialise the GuC parameter block before starting the firmware
> + * transfer. These parameters are read by the firmware on startup
> + * and cannot be changed thereafter.
> + */
> +void intel_guc_init_params(struct intel_guc *guc)
> +{
> + u32 params[GUC_CTL_MAX_DWORDS];
> +
> + memset(params, 0, sizeof(params));
> + guc_prepare_params(guc, params);
> + guc_write_params(guc, params);
> +}
> +
>  int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32
> len,
>      u32 *response_buf, u32 response_buf_size)
>  {
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 04/21] drm/i915/guc: Support dual Gen9/Gen11 parameters block

[John Spotswood]

On Wed, 2018-08-29 at 12:10 -0700, Wajdeczko, Michal wrote:
> Gen11 GuC boot parameter definitions are different than previously
> used for Gen9. Try to support both definitions until new firmwares
> for pre-Gen11 will be available.
> 
> Signed-off-by: Michal Wajdeczko 
> Cc: Joonas Lahtinen 
> Cc: Rodrigo Vivi 
> Cc: Daniele Ceraolo Spurio 
> Cc: Michel Thierry 
> Cc: John Spotswood 
> Cc: Vinay Belgaumkar 
> Cc: Tony Ye 
> Cc: Anusha Srivatsa 
> Cc: Jeff Mcgee 
> Cc: Antonio Argenziano 
> Cc: Sujaritha Sundaresan 

Reviewed-by: John Spotswood 

> ---
>  drivers/gpu/drm/i915/intel_guc.c  | 76
> +--
>  drivers/gpu/drm/i915/intel_guc_fwif.h | 59 +--
> 
>  2 files changed, 83 insertions(+), 52 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc.c
> b/drivers/gpu/drm/i915/intel_guc.c
> index 982bcc8..a9c2f7b 100644
> --- a/drivers/gpu/drm/i915/intel_guc.c
> +++ b/drivers/gpu/drm/i915/intel_guc.c
> @@ -230,14 +230,7 @@ void intel_guc_fini(struct intel_guc *guc)
>  static u32 guc_ctl_debug_flags(struct intel_guc *guc)
>  {
>   u32 level = intel_guc_log_get_level(>log);
> - u32 flags;
> - u32 ads;
> -
> - ads = intel_guc_ggtt_offset(guc, guc->ads_vma) >>
> PAGE_SHIFT;
> - flags = ads << GUC_ADS_ADDR_SHIFT | GUC_ADS_ENABLED;
> -
> - if (!GUC_LOG_LEVEL_IS_ENABLED(level))
> - flags |= GUC_LOG_DEFAULT_DISABLED;
> + u32 flags = 0;
>  
>   if (!GUC_LOG_LEVEL_IS_VERBOSE(level))
>   flags |= GUC_LOG_DISABLED;
> @@ -248,20 +241,28 @@ static u32 guc_ctl_debug_flags(struct intel_guc
> *guc)
>   return flags;
>  }
>  
> -static u32 guc_ctl_feature_flags(struct intel_guc *guc)
> +static u32 guc9_ctl_debug_flags(struct intel_guc *guc)
>  {
> - u32 flags = 0;
> + u32 level = intel_guc_log_get_level(>log);
> + u32 flags;
> + u32 ads;
>  
> - flags |=  GUC_CTL_VCS2_ENABLED;
> + ads = intel_guc_ggtt_offset(guc, guc->ads_vma) >>
> PAGE_SHIFT;
> + flags = ads << GUC9_ADS_ADDR_SHIFT | GUC9_ADS_ENABLED;
>  
> - if (USES_GUC_SUBMISSION(guc_to_i915(guc)))
> - flags |= GUC_CTL_KERNEL_SUBMISSIONS;
> - else
> - flags |= GUC_CTL_DISABLE_SCHEDULER;
> + if (!GUC_LOG_LEVEL_IS_ENABLED(level))
> + flags |= GUC9_LOG_DEFAULT_DISABLED;
> +
> + flags |= guc_ctl_debug_flags(guc);
>  
>   return flags;
>  }
>  
> +static u32 guc9_ctl_feature_flags(struct intel_guc *guc)
> +{
> + return GUC9_CTL_VCS2_ENABLED | GUC9_CTL_DISABLE_SCHEDULER;
> +}
> +
>  static u32 guc_ctl_ctxinfo_flags(struct intel_guc *guc)
>  {
>   u32 flags = 0;
> @@ -279,6 +280,16 @@ static u32 guc_ctl_ctxinfo_flags(struct
> intel_guc *guc)
>   return flags;
>  }
>  
> +static u32 guc_ctl_feature_flags(struct intel_guc *guc)
> +{
> + u32 flags = 0;
> +
> + if (!USES_GUC_SUBMISSION(guc_to_i915(guc)))
> + flags |= GUC_CTL_DISABLE_SCHEDULER;
> +
> + return flags;
> +}
> +
>  static u32 guc_ctl_log_params_flags(struct intel_guc *guc)
>  {
>   u32 offset = intel_guc_ggtt_offset(guc, guc->log.vma) >>
> PAGE_SHIFT;
> @@ -320,22 +331,39 @@ static u32 guc_ctl_log_params_flags(struct
> intel_guc *guc)
>   return flags;
>  }
>  
> -static void guc_prepare_params(struct intel_guc *guc, u32 *params)
> +static void guc9_prepare_params(struct intel_guc *guc, u32 *params)
>  {
>   /*
>    * GuC ARAT increment is 10 ns. GuC default scheduler
> quantum is one
>    * second. This ARAR is calculated by:
>    * Scheduler-Quantum-in-ns / ARAT-increment-in-ns =
> 10 / 10
>    */
> - params[GUC_CTL_ARAT_HIGH] = 0;
> - params[GUC_CTL_ARAT_LOW] = 1;
> + params[GUC9_CTL_ARAT_HIGH] = 0;
> + params[GUC9_CTL_ARAT_LOW] = 1;
> +
> + params[GUC9_CTL_WA] |= GUC9_CTL_WA_UK_BY_DRIVER;
>  
> - params[GUC_CTL_WA] |= GUC_CTL_WA_UK_BY_DRIVER;
> + params[GUC9_CTL_FEATURE] = guc9_ctl_feature_flags(guc);
> + params[GUC9_CTL_LOG_PARAMS] = guc_ctl_log_params_flags(guc);
> + params[GUC9_CTL_DEBUG] = guc9_ctl_debug_flags(guc);
> + params[GUC_CTL_CTXINFO] = guc_ctl_ctxinfo_flags(guc);
> +}
>  
> +static u32 guc_ctl_ads_flags(struct intel_guc *guc)
> +{
> + u32 ads = intel_guc_ggtt_offset(guc, guc->ads_vma) >>
> PAGE_SHIFT;
> + u32 flags = ads << GUC_ADS_ADDR_SHIFT;
> +
> + return flags;
> +}
> +
> +static void guc11_prepare_params(struct intel_guc *guc, u32 *params)
> +{
> + params[GUC_CTL_CTXINF

Re: [Intel-gfx] [PATCH 02/21] drm/i915/guc: Don't allow GuC submission on pre-Gen11

[John Spotswood]

On Wed, 2018-08-29 at 12:10 -0700, Wajdeczko, Michal wrote:
> Upcoming Gen11 GuC firmware requires new interface that is
> incompatible
> with existing pre-Gen11 firmwares. Updated firmwares for pre-Gen11
> will
> arrive later. In the meantime sanitize the enable_guc option so that
> we
> can enable HuC authentication but nothing else on pre-Gen11.
> 
> Signed-off-by: Michal Wajdeczko 
> Cc: Joonas Lahtinen 
> Cc: Rodrigo Vivi 
> Cc: Daniele Ceraolo Spurio 
> Cc: Michel Thierry 
> Cc: John Spotswood 
> Cc: Vinay Belgaumkar 
> Cc: Tony Ye 
> Cc: Anusha Srivatsa 
> Cc: Jeff Mcgee 
> Cc: Antonio Argenziano 
> Cc: Sujaritha Sundaresan 

Reviewed-by: John Spotswood 

> ---
>  drivers/gpu/drm/i915/intel_uc.c | 15 +++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_uc.c
> b/drivers/gpu/drm/i915/intel_uc.c
> index 7a3a4ca..185b29b 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -63,6 +63,8 @@ static int __get_platform_enable_guc(struct
> drm_i915_private *i915)
>   enable_guc |= ENABLE_GUC_LOAD_HUC;
>  
>   /* Any platform specific fine-tuning can be done here */
> + if (INTEL_GEN(i915) < 11)
> + enable_guc &= ~ENABLE_GUC_SUBMISSION;
>  
>   return enable_guc;
>  }
> @@ -115,6 +117,13 @@ static void sanitize_options_early(struct
> drm_i915_private *i915)
>    yesno(intel_uc_is_using_guc_submission()),
>    yesno(intel_uc_is_using_huc()));
>  
> + /* Verify GuC submission support */
> + if (intel_uc_is_using_guc_submission() && INTEL_GEN(i915) <
> 11) {
> + DRM_WARN("Incompatible option detected: %s=%d,
> %s!\n",
> +  "enable_guc", i915_modparams.enable_guc,
> +  "submission not supported");
> + }
> +
>   /* Verify GuC firmware availability */
>   if (intel_uc_is_using_guc() &&
> !intel_uc_fw_is_selected(guc_fw)) {
>   DRM_WARN("Incompatible option detected: %s=%d,
> %s!\n",
> @@ -292,6 +301,12 @@ int intel_uc_init(struct drm_i915_private *i915)
>   return ret;
>  
>   if (USES_GUC_SUBMISSION(i915)) {
> +
> + if (INTEL_GEN(i915) < 11) {
> + intel_guc_fini(guc);
> + return -EIO;
> + }
> +
>   /*
>    * This is stuff we need to have available at fw
> load time
>    * if we are planning to enable submission later
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] firmware/guc: Remove USES_GUC_SUBMISSION for suspend/resume

[John Spotswood]

On Fri, 2018-06-22 at 10:25 -0700, Daniele Ceraolo Spurio wrote:
> Commit title is slightly misleading, as the USES_GUC_SUBMISSION is
> not 
> removed from a suspend/resume path. the firmware tag is also
> confusing 
> since this fixes an i915 bug. Maybe something like "drm/i915/guc:
> Remove 
> USES_GUC_SUBMISSION for ads programming" would be clearer
> 
> On 22/06/18 10:05, Anusha Srivatsa wrote:
> > 
> > In the guc_ctl_debug_flags, the ads struct is programmed only
> > when USES_GUC_SUBMISSION is satisfied. But, this has to be
> > programmed for all suspend/resume cases.
> > Remove the condition and program the ads struct for
> > both huc loading and guc submission.
> > 
> > This issue was noticed when CI threw errors for enable_guc=2
> > (load huc; disable submission)
> > 
> Do we need a fixes: tag? Not sure we want this backported since GuC
> is 
> off by default.
> 
> > 
> > Credits to: Daniele Ceraolo Spurio  > >
> > Cc: John Spotswood 
> > Cc: Oscar Mateo 
> > Cc: Daniele Ceraolo Spurio 
> > Signed-off-by: Anusha Srivatsa 
> > ---
> >   drivers/gpu/drm/i915/intel_guc.c | 9 -
> >   1 file changed, 4 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_guc.c
> > b/drivers/gpu/drm/i915/intel_guc.c
> > index 1aff30b..b1d1a10 100644
> > --- a/drivers/gpu/drm/i915/intel_guc.c
> > +++ b/drivers/gpu/drm/i915/intel_guc.c
> > @@ -207,6 +207,7 @@ static u32 guc_ctl_debug_flags(struct intel_guc
> > *guc)
> >   {
> >     u32 level = intel_guc_log_get_level(>log);
> >     u32 flags = 0;
> > +   u32 ads = 0;
> >   
> >     if (!GUC_LOG_LEVEL_IS_ENABLED(level))
> >     flags |= GUC_LOG_DEFAULT_DISABLED;
> > @@ -217,12 +218,10 @@ static u32 guc_ctl_debug_flags(struct
> > intel_guc *guc)
> >     flags |= GUC_LOG_LEVEL_TO_VERBOSITY(level) <<
> >      GUC_LOG_VERBOSITY_SHIFT;
> >   
> > -   if (USES_GUC_SUBMISSION(guc_to_i915(guc))) {
> > -   u32 ads = intel_guc_ggtt_offset(guc, guc->ads_vma)
> > -   >> PAGE_SHIFT;
> > +   ads = intel_guc_ggtt_offset(guc, guc->ads_vma) <<
> You've flipped the shift here. With that fixed:
> 
> Reviewed-by: Daniele Ceraolo Spurio 
> 

With Daniele's recommended changes:

Reviewed-by:  John Spotswood 

> > 
> > +   PAGE_SHIFT;
> >   
> > -   flags |= ads << GUC_ADS_ADDR_SHIFT |
> > GUC_ADS_ENABLED;
> > -   }
> > +   flags |= ads << GUC_ADS_ADDR_SHIFT | GUC_ADS_ENABLED;
> >   
> >     return flags;
> >   }
> > 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/3] drm/i915/huc: Load HuC v03.00.2555 for Geminilake.

[John Spotswood]

On Fri, 2018-04-27 at 16:33 -0700, Anusha Srivatsa wrote:
> load the v03.00.2555 huC on geminilake.
> 
> Cc: Tomi Sarvela 
> Cc: Jani Saarinen 
> Signed-off-by: Anusha Srivatsa 
> ---
>  drivers/gpu/drm/i915/intel_huc_fw.c | 12 
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_huc_fw.c
> b/drivers/gpu/drm/i915/intel_huc_fw.c
> index f93d238..5e96690 100644
> --- a/drivers/gpu/drm/i915/intel_huc_fw.c
> +++ b/drivers/gpu/drm/i915/intel_huc_fw.c
> @@ -34,6 +34,10 @@
>  #define KBL_HUC_FW_MINOR 00
>  #define KBL_BLD_NUM 1810
>  
> +#define GLK_HUC_FW_MAJOR 02
> +#define GLK_HUC_FW_MINOR 00
> +#define GLK_BLD_NUM 1810

Is this version number a cut-n-paste error?  It seems to be the
version number as KBL directly above it.

> +
>  #define HUC_FW_PATH(platform, major, minor, bld_num) \
>   "i915/" __stringify(platform) "_huc_ver" __stringify(major)
> "_" \
>   __stringify(minor) "_" __stringify(bld_num) ".bin"
> @@ -50,6 +54,10 @@ MODULE_FIRMWARE(I915_BXT_HUC_UCODE);
>   KBL_HUC_FW_MINOR, KBL_BLD_NUM)
>  MODULE_FIRMWARE(I915_KBL_HUC_UCODE);
>  
> +#define I915_GLK_HUC_UCODE HUC_FW_PATH(glk, GLK_HUC_FW_MAJOR, \
> + GLK_HUC_FW_MINOR, GLK_BLD_NUM)
> +MODULE_FIRMWARE(I915_GLK_HUC_UCODE);
> +
>  static void huc_fw_select(struct intel_uc_fw *huc_fw)
>  {
>   struct intel_huc *huc = container_of(huc_fw, struct
> intel_huc, fw);
> @@ -76,6 +84,10 @@ static void huc_fw_select(struct intel_uc_fw
> *huc_fw)
>   huc_fw->path = I915_KBL_HUC_UCODE;
>   huc_fw->major_ver_wanted = KBL_HUC_FW_MAJOR;
>   huc_fw->minor_ver_wanted = KBL_HUC_FW_MINOR;
> + } else if (IS_GEMINILAKE(dev_priv)) {
> + huc_fw->path = I915_GLK_HUC_UCODE;
> + huc_fw->major_ver_wanted = GLK_HUC_FW_MAJOR;
> + huc_fw->minor_ver_wanted = GLK_HUC_FW_MINOR;
>   } else {
>   DRM_WARN("%s: No firmware known for this
> platform!\n",
>    intel_uc_fw_type_repr(huc_fw->type));
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 5/5] drm/i915/icl/huc: Define the HuC firmware version for Icelake

[John Spotswood]

On Wed, 2018-05-02 at 12:03 -0700, Oscar Mateo wrote:
> This patch adds the support to load HuC on ICL.
> Version 8.02.2678
> 
> v2 (James): Rebase
> 
> Signed-off-by: Oscar Mateo <oscar.ma...@intel.com>
> Cc: Tony Ye <tony...@intel.com>
> Cc: Vinay Belgaumkar <vinay.belgaum...@intel.com>
> Cc: Michel Thierry <michel.thie...@intel.com>
> Cc: Joonas Lahtinen <joonas.lahti...@linux.intel.com>
> Cc: Michal Wajdeczko <michal.wajdec...@intel.com>
> Cc: John Spotswood <john.a.spotsw...@intel.com>

Reviewed-by: John Spotswood <john.a.spotsw...@intel.com>

> Cc: Anusha Srivatsa <anusha.sriva...@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_huc_fw.c | 11 +++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_huc_fw.c
> b/drivers/gpu/drm/i915/intel_huc_fw.c
> index f93d238..795d585 100644
> --- a/drivers/gpu/drm/i915/intel_huc_fw.c
> +++ b/drivers/gpu/drm/i915/intel_huc_fw.c
> @@ -34,6 +34,10 @@
>  #define KBL_HUC_FW_MINOR 00
>  #define KBL_BLD_NUM 1810
>  
> +#define ICL_HUC_FW_MAJOR 8
> +#define ICL_HUC_FW_MINOR 02
> +#define ICL_BLD_NUM 2678
> +
>  #define HUC_FW_PATH(platform, major, minor, bld_num) \
>   "i915/" __stringify(platform) "_huc_ver" __stringify(major)
> "_" \
>   __stringify(minor) "_" __stringify(bld_num) ".bin"
> @@ -50,6 +54,9 @@
>   KBL_HUC_FW_MINOR, KBL_BLD_NUM)
>  MODULE_FIRMWARE(I915_KBL_HUC_UCODE);
>  
> +#define I915_ICL_HUC_UCODE HUC_FW_PATH(icl, ICL_HUC_FW_MAJOR, \
> + ICL_HUC_FW_MINOR, ICL_BLD_NUM)
> +
>  static void huc_fw_select(struct intel_uc_fw *huc_fw)
>  {
>   struct intel_huc *huc = container_of(huc_fw, struct
> intel_huc, fw);
> @@ -76,6 +83,10 @@ static void huc_fw_select(struct intel_uc_fw
> *huc_fw)
>   huc_fw->path = I915_KBL_HUC_UCODE;
>   huc_fw->major_ver_wanted = KBL_HUC_FW_MAJOR;
>   huc_fw->minor_ver_wanted = KBL_HUC_FW_MINOR;
> + } else if (IS_ICELAKE(dev_priv)) {
> + huc->fw.path = I915_ICL_HUC_UCODE;
> + huc->fw.major_ver_wanted = ICL_HUC_FW_MAJOR;
> + huc->fw.minor_ver_wanted = ICL_HUC_FW_MINOR;
>   } else {
>   DRM_WARN("%s: No firmware known for this
> platform!\n",
>    intel_uc_fw_type_repr(huc_fw->type));
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 4/5] drm/i915/icl/huc: Correctly authenticate the HuC for Icelake

[John Spotswood]

On Wed, 2018-05-02 at 12:03 -0700, Oscar Mateo wrote:
> The register to check for correct HuC authentication by the GuC
> has changed in Icelake. Look into the right register & bit.
> 
> v2: rebased.
> v3: rebased.
> v4: Fix I915_PARAM_HUC_STATUS as well (Tony)
> v5: Fix duplicate Cc
> 
> BSpec: 19686
> 
> Signed-off-by: Oscar Mateo <oscar.ma...@intel.com>
> Cc: Tony Ye <tony...@intel.com>
> Cc: Vinay Belgaumkar <vinay.belgaum...@intel.com>
> Cc: Michel Thierry <michel.thie...@intel.com>
> Cc: Michal Wajdeczko <michal.wajdec...@intel.com>
> Cc: John Spotswood <john.a.spotsw...@intel.com>

Reviewed-by: John Spotswood <john.a.spotsw...@intel.com>

> Cc: Anusha Srivatsa <anusha.sriva...@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_guc_reg.h |  3 +++
>  drivers/gpu/drm/i915/intel_huc.c | 23 +++
>  2 files changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc_reg.h
> b/drivers/gpu/drm/i915/intel_guc_reg.h
> index d860847..9f14f9f 100644
> --- a/drivers/gpu/drm/i915/intel_guc_reg.h
> +++ b/drivers/gpu/drm/i915/intel_guc_reg.h
> @@ -76,6 +76,9 @@
>  #define HUC_STATUS2 _MMIO(0xD3B0)
>  #define   HUC_FW_VERIFIED   (1<<7)
>  
> +#define HUC_KERNEL_LOAD_INFO _MMIO(0xC1DC)
> +#define   HUC_LOAD_SUCCESSFUL(1 << 0)
> +
>  #define GUC_WOPCM_SIZE   _MMIO(0xc050)
>  #define   GUC_WOPCM_SIZE_LOCKED    (1<<0)
>  #define   GUC_WOPCM_SIZE_SHIFT   12
> diff --git a/drivers/gpu/drm/i915/intel_huc.c
> b/drivers/gpu/drm/i915/intel_huc.c
> index 2912852..b509756 100644
> --- a/drivers/gpu/drm/i915/intel_huc.c
> +++ b/drivers/gpu/drm/i915/intel_huc.c
> @@ -48,9 +48,19 @@ int intel_huc_auth(struct intel_huc *huc)
>   struct drm_i915_private *i915 = huc_to_i915(huc);
>   struct intel_guc *guc = >guc;
>   struct i915_vma *vma;
> + i915_reg_t status_reg;
>   u32 status;
> + u32 status_ok;
>   int ret;
>  
> + if (INTEL_GEN(i915) >= 11) {
> + status_reg = HUC_KERNEL_LOAD_INFO;
> + status_ok = HUC_LOAD_SUCCESSFUL;
> + } else {
> + status_reg = HUC_STATUS2;
> + status_ok = HUC_FW_VERIFIED;
> + }
> +
>   if (huc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
>   return -ENOEXEC;
>  
> @@ -72,9 +82,9 @@ int intel_huc_auth(struct intel_huc *huc)
>  
>   /* Check authentication status, it should be done by now */
>   ret = __intel_wait_for_register(i915,
> - HUC_STATUS2,
> - HUC_FW_VERIFIED,
> - HUC_FW_VERIFIED,
> + status_reg,
> + status_ok,
> + status_ok,
>   2, 50, );
>   if (ret) {
>   DRM_ERROR("HuC: Firmware not verified %#x\n",
> status);
> @@ -112,7 +122,12 @@ int intel_huc_check_status(struct intel_huc
> *huc)
>   return -ENODEV;
>  
>   intel_runtime_pm_get(dev_priv);
> - status = I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED;
> +
> + if (INTEL_GEN(dev_priv) >= 11)
> + status = I915_READ(HUC_KERNEL_LOAD_INFO) &
> HUC_LOAD_SUCCESSFUL;
> + else
> + status = I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED;
> +
>   intel_runtime_pm_put(dev_priv);
>  
>   return status;
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/5] drm/i915/icl/guc: Define the GuC firmware version for Icelake

[John Spotswood]

On Wed, 2018-05-02 at 12:03 -0700, Oscar Mateo wrote:
> A GuC firmware for Icelake is now available. Let's use it.
> 
> v2: Split out the Cannonlake stuff in a separate patch (Michal)
> 
> v3: Rebased
> 
> v4:
>   - Rebased
>   - Split out MODULE_FIRMWARE so we don't accidentally push it
> before linux-firmware (Joonas)
> 
> v5: Use the latest firmware (v23.120)
> v6: Use the latest firmware (v26.171)
> v7: Rebased (remove guc-core-family)
> v8: Use the latest firmware (v27.182)
> v9: Use the latest firmware (v27.185)
> 
> Signed-off-by: Michel Thierry <michel.thie...@intel.com>
> Signed-off-by: Oscar Mateo <oscar.ma...@intel.com>
> Cc: Michal Wajdeczko <michal.wajdec...@intel.com>
> Cc: John Spotswood <john.a.spotsw...@intel.com>

Reviewed-by: John Spotswood <john.a.spotsw...@intel.com>

> Cc: Tony Ye <tony...@intel.com>
> Cc: Joonas Lahtinen <joonas.lahti...@linux.intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospu...@intel.com>
> Cc: Anusha Srivatsa <anusha.sriva...@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_guc_fw.c | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c
> b/drivers/gpu/drm/i915/intel_guc_fw.c
> index a9e6fcc..dfdf60c 100644
> --- a/drivers/gpu/drm/i915/intel_guc_fw.c
> +++ b/drivers/gpu/drm/i915/intel_guc_fw.c
> @@ -39,6 +39,9 @@
>  #define KBL_FW_MAJOR 9
>  #define KBL_FW_MINOR 39
>  
> +#define ICL_FW_MAJOR 27
> +#define ICL_FW_MINOR 185
> +
>  #define GUC_FW_PATH(platform, major, minor) \
> "i915/" __stringify(platform) "_guc_ver" __stringify(major)
> "_" __stringify(minor) ".bin"
>  
> @@ -51,6 +54,8 @@
>  #define I915_KBL_GUC_UCODE GUC_FW_PATH(kbl, KBL_FW_MAJOR,
> KBL_FW_MINOR)
>  MODULE_FIRMWARE(I915_KBL_GUC_UCODE);
>  
> +#define I915_ICL_GUC_UCODE GUC_FW_PATH(icl, ICL_FW_MAJOR,
> ICL_FW_MINOR)
> +
>  static void guc_fw_select(struct intel_uc_fw *guc_fw)
>  {
>   struct intel_guc *guc = container_of(guc_fw, struct
> intel_guc, fw);
> @@ -77,6 +82,10 @@ static void guc_fw_select(struct intel_uc_fw
> *guc_fw)
>   guc_fw->path = I915_KBL_GUC_UCODE;
>   guc_fw->major_ver_wanted = KBL_FW_MAJOR;
>   guc_fw->minor_ver_wanted = KBL_FW_MINOR;
> + } else if (IS_ICELAKE(dev_priv)) {
> + guc_fw->path = I915_ICL_GUC_UCODE;
> + guc_fw->major_ver_wanted = ICL_FW_MAJOR;
> + guc_fw->minor_ver_wanted = ICL_FW_MINOR;
>   } else {
>   DRM_WARN("%s: No firmware known for this
> platform!\n",
>    intel_uc_fw_type_repr(guc_fw->type));
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/5] drm/i915/icl/guc: Do not allow GuC submission on Icelake for now

[John Spotswood]

On Wed, 2018-05-02 at 12:03 -0700, Oscar Mateo wrote:
> Sanitize the enable_guc option so that we can enable HuC
> authentication,
> but nothing else. The firmware interface has changed quite
> dramatically
> in Gen11, so it will take a while before we can submit workloads to
> the
> GuC with guarantees.
> 
> Signed-off-by: Oscar Mateo <oscar.ma...@intel.com>
> Cc: Joonas Lahtinen <joonas.lahti...@linux.intel.com>
> Cc: Michal Wajdeczko <michal.wajdec...@intel.com>
> Cc: John Spotswood <john.a.spotsw...@intel.com>

Reviewed-by: John Spotswood <john.a.spotsw...@intel.com>

> Cc: Tony Ye <tony...@intel.com>
> Cc: Anusha Srivatsa <anusha.sriva...@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_uc.c | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_uc.c
> b/drivers/gpu/drm/i915/intel_uc.c
> index 1cffaf7..d2a935c 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -63,6 +63,8 @@ static int __get_platform_enable_guc(struct
> drm_i915_private *dev_priv)
>   enable_guc |= ENABLE_GUC_LOAD_HUC;
>  
>   /* Any platform specific fine-tuning can be done here */
> + if (INTEL_GEN(dev_priv) >= 11)
> + enable_guc &= ~ENABLE_GUC_SUBMISSION;
>  
>   return enable_guc;
>  }
> @@ -115,6 +117,14 @@ static void sanitize_options_early(struct
> drm_i915_private *dev_priv)
>    yesno(intel_uc_is_using_guc_submission()),
>    yesno(intel_uc_is_using_huc()));
>  
> + /* Verify GuC submission support */
> + if (intel_uc_is_using_guc_submission() &&
> INTEL_GEN(dev_priv) >= 11) {
> + DRM_WARN("Incompatible option detected: %s=%d,
> %s!\n",
> +  "enable_guc", i915_modparams.enable_guc,
> +  "submission not supported");
> + i915_modparams.enable_guc &= ~ENABLE_GUC_SUBMISSION;
> + }
> +
>   /* Verify GuC firmware availability */
>   if (intel_uc_is_using_guc() &&
> !intel_uc_fw_is_selected(guc_fw)) {
>   DRM_WARN("Incompatible option detected: %s=%d,
> %s!\n",
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/5] drm/i915/icl/guc: Pass the bare minimum GuC init parameters for Icelake

[John Spotswood]

On Wed, 2018-05-02 at 12:03 -0700, Oscar Mateo wrote:
> Only enough to achieve HuC authentication. No GuC submission
> or any other feature for the time being.
> 
> v2: Fix extra space
> 
> Signed-off-by: Oscar Mateo <oscar.ma...@intel.com>
> Cc: Joonas Lahtinen <joonas.lahti...@linux.intel.com>
> Cc: Michal Wajdeczko <michal.wajdec...@intel.com>
> Cc: John Spotswood <john.a.spotsw...@intel.com>

Reviewed-by: John Spotswood <john.a.spotsw...@intel.com>

> Cc: Tony Ye <tony...@intel.com>
> Cc: Anusha Srivatsa <anusha.sriva...@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_guc.c  | 10 --
>  drivers/gpu/drm/i915/intel_guc_fwif.h |  1 +
>  2 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc.c
> b/drivers/gpu/drm/i915/intel_guc.c
> index 116f4cc..b1b69e1 100644
> --- a/drivers/gpu/drm/i915/intel_guc.c
> +++ b/drivers/gpu/drm/i915/intel_guc.c
> @@ -245,8 +245,12 @@ void intel_guc_init_params(struct intel_guc
> *guc)
>  
>   params[GUC_CTL_WA] |= GUC_CTL_WA_UK_BY_DRIVER;
>  
> - params[GUC_CTL_FEATURE] |= GUC_CTL_DISABLE_SCHEDULER |
> - GUC_CTL_VCS2_ENABLED;
> + if (INTEL_GEN(dev_priv) >= 11) {
> + params[GUC_CTL_FEATURE] |=
> GEN11_GUC_CTL_DISABLE_SCHEDULER;
> + } else {
> + params[GUC_CTL_FEATURE] |=
> GUC_CTL_DISABLE_SCHEDULER;
> + params[GUC_CTL_FEATURE] |= GUC_CTL_VCS2_ENABLED;
> + }
>  
>   params[GUC_CTL_LOG_PARAMS] = guc->log.flags;
>  
> @@ -259,6 +263,8 @@ void intel_guc_init_params(struct intel_guc *guc)
>   u32 pgs = intel_guc_ggtt_offset(guc, guc-
> >stage_desc_pool);
>   u32 ctx_in_16 = GUC_MAX_STAGE_DESCRIPTORS / 16;
>  
> + GEM_BUG_ON(INTEL_GEN(dev_priv) >= 11);
> +
>   params[GUC_CTL_DEBUG] |= ads << GUC_ADS_ADDR_SHIFT;
>   params[GUC_CTL_DEBUG] |= GUC_ADS_ENABLED;
>  
> diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h
> b/drivers/gpu/drm/i915/intel_guc_fwif.h
> index 0867ba7..781c0c0 100644
> --- a/drivers/gpu/drm/i915/intel_guc_fwif.h
> +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
> @@ -106,6 +106,7 @@
>  #define   GUC_CTL_PREEMPTION_LOG (1 << 5)
>  #define   GUC_CTL_ENABLE_SLPC(1 << 7)
>  #define   GUC_CTL_RESET_ON_PREMPT_FAILURE(1 << 8)
> +#define   GEN11_GUC_CTL_DISABLE_SCHEDULER(1 << 14)
>  
>  #define GUC_CTL_DEBUG8
>  #define   GUC_LOG_VERBOSITY_SHIFT0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 4/5] drm/i915/icl/huc: Correctly authenticate the HuC for Icelake

[John Spotswood]

On Wed, 2018-05-02 at 12:03 -0700, Oscar Mateo wrote:
> The register to check for correct HuC authentication by the GuC
> has changed in Icelake. Look into the right register & bit.
> 
> v2: rebased.
> v3: rebased.
> v4: Fix I915_PARAM_HUC_STATUS as well (Tony)
> v5: Fix duplicate Cc
> 
> BSpec: 19686
> 
> Signed-off-by: Oscar Mateo <oscar.ma...@intel.com>
> Cc: Tony Ye <tony...@intel.com>
> Cc: Vinay Belgaumkar <vinay.belgaum...@intel.com>
> Cc: Michel Thierry <michel.thie...@intel.com>
> Cc: Michal Wajdeczko <michal.wajdec...@intel.com>
> Cc: John Spotswood <john.a.spotsw...@intel.com>
> Cc: Anusha Srivatsa <anusha.sriva...@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_guc_reg.h |  3 +++
>  drivers/gpu/drm/i915/intel_huc.c | 23 +++
>  2 files changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc_reg.h
> b/drivers/gpu/drm/i915/intel_guc_reg.h
> index d860847..9f14f9f 100644
> --- a/drivers/gpu/drm/i915/intel_guc_reg.h
> +++ b/drivers/gpu/drm/i915/intel_guc_reg.h
> @@ -76,6 +76,9 @@
>  #define HUC_STATUS2 _MMIO(0xD3B0)
>  #define   HUC_FW_VERIFIED   (1<<7)
>  
> +#define HUC_KERNEL_LOAD_INFO _MMIO(0xC1DC)
> +#define   HUC_LOAD_SUCCESSFUL(1 << 0)
> +
>  #define GUC_WOPCM_SIZE   _MMIO(0xc050)
>  #define   GUC_WOPCM_SIZE_LOCKED    (1<<0)
>  #define   GUC_WOPCM_SIZE_SHIFT   12
> diff --git a/drivers/gpu/drm/i915/intel_huc.c
> b/drivers/gpu/drm/i915/intel_huc.c
> index 2912852..b509756 100644
> --- a/drivers/gpu/drm/i915/intel_huc.c
> +++ b/drivers/gpu/drm/i915/intel_huc.c
> @@ -48,9 +48,19 @@ int intel_huc_auth(struct intel_huc *huc)
>   struct drm_i915_private *i915 = huc_to_i915(huc);
>   struct intel_guc *guc = >guc;
>   struct i915_vma *vma;
> + i915_reg_t status_reg;
>   u32 status;
> + u32 status_ok;
>   int ret;
>  
> + if (INTEL_GEN(i915) >= 11) {
> + status_reg = HUC_KERNEL_LOAD_INFO;
> + status_ok = HUC_LOAD_SUCCESSFUL;
> + } else {
> + status_reg = HUC_STATUS2;
> + status_ok = HUC_FW_VERIFIED;
> + }
> +
>   if (huc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
>   return -ENOEXEC;
>  
> @@ -72,9 +82,9 @@ int intel_huc_auth(struct intel_huc *huc)
>  
>   /* Check authentication status, it should be done by now */
>   ret = __intel_wait_for_register(i915,
> - HUC_STATUS2,
> - HUC_FW_VERIFIED,
> - HUC_FW_VERIFIED,
> + status_reg,
> + status_ok,
> + status_ok,
>   2, 50, );

Minor question:  You are checking different registers depending on gen.
 Will the fast and slow timeout values apply equally to both situations
?

>   if (ret) {
>   DRM_ERROR("HuC: Firmware not verified %#x\n",
> status);
> @@ -112,7 +122,12 @@ int intel_huc_check_status(struct intel_huc
> *huc)
>   return -ENODEV;
>  
>   intel_runtime_pm_get(dev_priv);
> - status = I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED;
> +
> + if (INTEL_GEN(dev_priv) >= 11)
> + status = I915_READ(HUC_KERNEL_LOAD_INFO) &
> HUC_LOAD_SUCCESSFUL;
> + else
> + status = I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED;
> +
>   intel_runtime_pm_put(dev_priv);
>  
>   return status;
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/5] drm/i915/icl/guc: Define the GuC firmware version for Icelake

[John Spotswood]

On Fri, 2018-04-27 at 14:31 -0700, Oscar Mateo wrote:
> A GuC firmware for Icelake is now available. Let's use it.
> 
> v2: Split out the Cannonlake stuff in a separate patch (Michal)
> 
> v3: Rebased
> 
> v4:
>   - Rebased
>   - Split out MODULE_FIRMWARE so we don't accidentally push it
> before linux-firmware (Joonas)
> 
> v5: Use the latest firmware (v23.120)
> v6: Use the latest firmware (v26.171)
> v7: Rebased (remove guc-core-family)
> v8: Use the latest firmware (v27.182)
> 
> Cc: Michal Wajdeczko <michal.wajdec...@intel.com>
> Cc: John Spotswood <john.a.spotsw...@intel.com>
> Cc: Tony Ye <tony...@intel.com>
> Cc: Joonas Lahtinen <joonas.lahti...@linux.intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospu...@intel.com>
> Signed-off-by: Michel Thierry <michel.thie...@intel.com>
> Signed-off-by: Oscar Mateo <oscar.ma...@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_guc_fw.c | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c
> b/drivers/gpu/drm/i915/intel_guc_fw.c
> index a9e6fcc..c5c5dd8 100644
> --- a/drivers/gpu/drm/i915/intel_guc_fw.c
> +++ b/drivers/gpu/drm/i915/intel_guc_fw.c
> @@ -39,6 +39,9 @@
>  #define KBL_FW_MAJOR 9
>  #define KBL_FW_MINOR 39
>  
> +#define ICL_FW_MAJOR 27
> +#define ICL_FW_MINOR 182

This version will not be released, so why is this being added here?

> +
>  #define GUC_FW_PATH(platform, major, minor) \
> "i915/" __stringify(platform) "_guc_ver" __stringify(major)
> "_" __stringify(minor) ".bin"
>  
> @@ -51,6 +54,8 @@
>  #define I915_KBL_GUC_UCODE GUC_FW_PATH(kbl, KBL_FW_MAJOR,
> KBL_FW_MINOR)
>  MODULE_FIRMWARE(I915_KBL_GUC_UCODE);
>  
> +#define I915_ICL_GUC_UCODE GUC_FW_PATH(icl, ICL_FW_MAJOR,
> ICL_FW_MINOR)
> +
>  static void guc_fw_select(struct intel_uc_fw *guc_fw)
>  {
>   struct intel_guc *guc = container_of(guc_fw, struct
> intel_guc, fw);
> @@ -77,6 +82,10 @@ static void guc_fw_select(struct intel_uc_fw
> *guc_fw)
>   guc_fw->path = I915_KBL_GUC_UCODE;
>   guc_fw->major_ver_wanted = KBL_FW_MAJOR;
>   guc_fw->minor_ver_wanted = KBL_FW_MINOR;
> + } else if (IS_ICELAKE(dev_priv)) {
> + guc_fw->path = I915_ICL_GUC_UCODE;
> + guc_fw->major_ver_wanted = ICL_FW_MAJOR;
> + guc_fw->minor_ver_wanted = ICL_FW_MINOR;
>   } else {
>   DRM_WARN("%s: No firmware known for this
> platform!\n",
>    intel_uc_fw_type_repr(guc_fw->type));
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/5] drm/i915/icl/guc: Pass the bare minimum GuC init parameters for Icelake

[John Spotswood]

On Fri, 2018-04-27 at 14:31 -0700, Oscar Mateo wrote:
> Only enough to achieve HuC authentication. No GuC submission
> or any other feature for the time being.
> 
> Signed-off-by: Oscar Mateo <oscar.ma...@intel.com>
> Cc: Joonas Lahtinen <joonas.lahti...@linux.intel.com>
> Cc: Michal Wajdeczko <michal.wajdec...@intel.com>
> Cc: John Spotswood <john.a.spotsw...@intel.com>
> Cc: Tony Ye <tony...@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_guc.c  | 10 --
>  drivers/gpu/drm/i915/intel_guc_fwif.h |  1 +
>  2 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc.c
> b/drivers/gpu/drm/i915/intel_guc.c
> index 116f4cc..133747c 100644
> --- a/drivers/gpu/drm/i915/intel_guc.c
> +++ b/drivers/gpu/drm/i915/intel_guc.c
> @@ -245,8 +245,12 @@ void intel_guc_init_params(struct intel_guc
> *guc)
>  
>   params[GUC_CTL_WA] |= GUC_CTL_WA_UK_BY_DRIVER;
>  
> - params[GUC_CTL_FEATURE] |= GUC_CTL_DISABLE_SCHEDULER |
> - GUC_CTL_VCS2_ENABLED;
> + if (INTEL_GEN(dev_priv) >= 11) {
> + params[GUC_CTL_FEATURE] |=
> GEN11_GUC_CTL_DISABLE_SCHEDULER;
> +  } else {
> + params[GUC_CTL_FEATURE] |=
> GUC_CTL_DISABLE_SCHEDULER;
> 
> + params[GUC_CTL_FEATURE] |= GUC_CTL_VCS2_ENABLED;

Should the OR'ing of GUC_CTL_VCS2_ENABLED be outside of the
conditional?  It looks like the only purpose of the conditional is to
distinguish GEN for the scheduler disable flag.

> + }
>  
>   params[GUC_CTL_LOG_PARAMS] = guc->log.flags;
>  
> @@ -259,6 +263,8 @@ void intel_guc_init_params(struct intel_guc *guc)
>   u32 pgs = intel_guc_ggtt_offset(guc, guc-
> >stage_desc_pool);
>   u32 ctx_in_16 = GUC_MAX_STAGE_DESCRIPTORS / 16;
>  
> + GEM_BUG_ON(INTEL_GEN(dev_priv) >= 11);
> +
>   params[GUC_CTL_DEBUG] |= ads << GUC_ADS_ADDR_SHIFT;
>   params[GUC_CTL_DEBUG] |= GUC_ADS_ENABLED;
>  
> diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h
> b/drivers/gpu/drm/i915/intel_guc_fwif.h
> index 0867ba7..781c0c0 100644
> --- a/drivers/gpu/drm/i915/intel_guc_fwif.h
> +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
> @@ -106,6 +106,7 @@
>  #define   GUC_CTL_PREEMPTION_LOG (1 << 5)
>  #define   GUC_CTL_ENABLE_SLPC(1 << 7)
>  #define   GUC_CTL_RESET_ON_PREMPT_FAILURE(1 << 8)
> +#define   GEN11_GUC_CTL_DISABLE_SCHEDULER(1 << 14)
>  
>  #define GUC_CTL_DEBUG8
>  #define   GUC_LOG_VERBOSITY_SHIFT0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3 3/4] drm/i915: Add code to accept valid locked WOPCM register values

[John Spotswood]

On Mon, 2018-04-09 at 17:42 -0700, Jackie Li wrote:
> In current code, we only compare the locked WOPCM register values
> with the
> calculated values. However, we can continue loading GuC/HuC firmware
> if the
> locked (or partially locked) values were valid for current GuC/HuC
> firmware
> sizes.
> 
> This patch added a new code path to verify whether the locked
> register
> values can be used for GuC/HuC firmware loading, it will recalculate
> the
> verify the new values if these registers were partially locked, so
> that we
> won't fail the GuC/HuC firmware loading even if the locked register
> values
> are different from the calculated ones.
> 
> v2:
>  - Update WOPCM register only if it's not locked
> 
> Signed-off-by: Jackie Li <yaodong...@intel.com>
> Cc: Michal Wajdeczko <michal.wajdec...@intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kam...@intel.com>
> Cc: Michal Winiarski <michal.winiar...@intel.com>
> Cc: John Spotswood <john.a.spotsw...@intel.com>
> Cc: Joonas Lahtinen <joonas.lahti...@linux.intel.com>

Reviewed-by: John Spotswood <john.a.spotsw...@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_wopcm.c | 217
> +++--
>  1 file changed, 185 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_wopcm.c
> b/drivers/gpu/drm/i915/intel_wopcm.c
> index b1c08ca..fa8d2be 100644
> --- a/drivers/gpu/drm/i915/intel_wopcm.c
> +++ b/drivers/gpu/drm/i915/intel_wopcm.c
> @@ -140,6 +140,53 @@ static inline int check_hw_restriction(struct
> drm_i915_private *i915,
>   return err;
>  }
>  
> +static inline u32 calculate_min_guc_wopcm_base(u32 huc_fw_size)
> +{
> + return ALIGN(huc_fw_size + WOPCM_RESERVED_SIZE,
> +  GUC_WOPCM_OFFSET_ALIGNMENT);
> +}
> +
> +static inline u32 calculate_min_guc_wopcm_size(u32 guc_fw_size)
> +{
> + return guc_fw_size + GUC_WOPCM_RESERVED +
> GUC_WOPCM_STACK_RESERVED;
> +}
> +
> +static inline int calculate_max_guc_wopcm_size(struct intel_wopcm
> *wopcm,
> +    u32 guc_wopcm_base,
> +    u32 *guc_wopcm_size)
> +{
> + struct drm_i915_private *i915 = wopcm_to_i915(wopcm);
> + u32 ctx_rsvd = context_reserved_size(i915);
> +
> + if (guc_wopcm_base >= wopcm->size ||
> + (guc_wopcm_base + ctx_rsvd) >= wopcm->size) {
> + DRM_ERROR("GuC WOPCM base (%uKiB) is too big.\n",
> +   guc_wopcm_base / 1024);
> + return -E2BIG;
> + }
> +
> + *guc_wopcm_size =
> + (wopcm->size - guc_wopcm_base - ctx_rsvd) &
> GUC_WOPCM_SIZE_MASK;
> +
> + return 0;
> +}
> +
> +static inline int verify_calculated_values(struct drm_i915_private
> *i915,
> +    u32 guc_fw_size, u32
> huc_fw_size,
> +    u32 guc_wopcm_base,
> +    u32 guc_wopcm_size)
> +{
> + if (guc_wopcm_size <
> calculate_min_guc_wopcm_size(guc_fw_size)) {
> + DRM_ERROR("Need %uKiB WOPCM for GuC FW, %uKiB
> available.\n",
> +   calculate_min_guc_wopcm_size(guc_fw_size),
> +   guc_wopcm_size / 1024);
> + return -E2BIG;
> + }
> +
> + return check_hw_restriction(i915, guc_wopcm_base,
> guc_wopcm_size,
> + huc_fw_size);
> +}
> +
>  /**
>   * intel_wopcm_init() - Initialize the WOPCM structure.
>   * @wopcm: pointer to intel_wopcm.
> @@ -157,10 +204,8 @@ int intel_wopcm_init(struct intel_wopcm *wopcm)
>   struct drm_i915_private *i915 = wopcm_to_i915(wopcm);
>   u32 guc_fw_size = intel_uc_fw_get_upload_size(
> >guc.fw);
>   u32 huc_fw_size = intel_uc_fw_get_upload_size(
> >huc.fw);
> - u32 ctx_rsvd = context_reserved_size(i915);
>   u32 guc_wopcm_base;
>   u32 guc_wopcm_size;
> - u32 guc_wopcm_rsvd;
>   int err;
>  
>   GEM_BUG_ON(!wopcm->size);
> @@ -177,35 +222,121 @@ int intel_wopcm_init(struct intel_wopcm
> *wopcm)
>   return -E2BIG;
>   }
>  
> - guc_wopcm_base = ALIGN(huc_fw_size + WOPCM_RESERVED_SIZE,
> -    GUC_WOPCM_OFFSET_ALIGNMENT);
> - if ((guc_wopcm_base + ctx_rsvd) >= wopcm->size) {
> - DRM_ERROR("GuC WOPCM base (%uKiB) is too big.\n",
> -   guc_wopcm_base / 1024);
> + guc_wopcm_base = calculate_min_guc_wopcm_base(huc_fw_size);
> + err = calculate_max_guc_wopcm_size(wop

Re: [Intel-gfx] [PATCH v3 2/4] drm/i915: Always set HUC_LOADING_AGENT_GUC bit in WOPCM offset register

[John Spotswood]

On Mon, 2018-04-09 at 17:42 -0700, Jackie Li wrote:
> The enable_guc modparam is used to enable/disable GuC/HuC FW
> uploading
> dynamcially during i915 module loading. If WOPCM offset register was
> locked
> without having HUC_LOADING_AGENT_GUC bit set to 1, the module
> reloading
> with both GuC and HuC FW will fail since we need to set this bit to 1
> for
> HuC FW uploading.
> 
> Since HUC_LOADING_AGENT_GUC bit has no impact on GuC FW uploading,
> this
> patch updates the register updating code to make sure the WOPCM
> offset
> register is always locked with HUC_LOADING_AGENT_GUC bit set to 1
> which
> will guarantee successful uploading of both GuC and HuC FW. We will
> further
> take care of the locked values in the following enhancement patch.
> 
> Signed-off-by: Jackie Li <yaodong...@intel.com>
> Cc: Michal Wajdeczko <michal.wajdec...@intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kam...@intel.com>
> Cc: Michal Winiarski <michal.winiar...@intel.com>
> Cc: John Spotswood <john.a.spotsw...@intel.com>
> Cc: Joonas Lahtinen <joonas.lahti...@linux.intel.com>

Reviewed-by: John Spotswood <john.a.spotsw...@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_wopcm.c | 8 +++-
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_wopcm.c
> b/drivers/gpu/drm/i915/intel_wopcm.c
> index 74bf76f..b1c08ca 100644
> --- a/drivers/gpu/drm/i915/intel_wopcm.c
> +++ b/drivers/gpu/drm/i915/intel_wopcm.c
> @@ -238,8 +238,6 @@ static inline int write_and_verify(struct
> drm_i915_private *dev_priv,
>  int intel_wopcm_init_hw(struct intel_wopcm *wopcm)
>  {
>   struct drm_i915_private *dev_priv = wopcm_to_i915(wopcm);
> - u32 huc_agent;
> - u32 mask;
>   int err;
>  
>   if (!USES_GUC(dev_priv))
> @@ -255,10 +253,10 @@ int intel_wopcm_init_hw(struct intel_wopcm
> *wopcm)
>   if (err)
>   goto err_out;
>  
> - huc_agent = USES_HUC(dev_priv) ? HUC_LOADING_AGENT_GUC : 0;
> - mask = GUC_WOPCM_OFFSET_MASK | GUC_WOPCM_OFFSET_VALID |
> huc_agent;
>   err = write_and_verify(dev_priv, DMA_GUC_WOPCM_OFFSET,
> -    wopcm->guc.base | huc_agent, mask,
> +    wopcm->guc.base |
> HUC_LOADING_AGENT_GUC,
> +    GUC_WOPCM_OFFSET_MASK |
> HUC_LOADING_AGENT_GUC |
> +    GUC_WOPCM_OFFSET_VALID,
>      GUC_WOPCM_OFFSET_VALID);
>   if (err)
>   goto err_out;
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3 1/4] drm/i915: Always do WOPCM partitioning based on real firmware sizes

[John Spotswood]

On Mon, 2018-04-09 at 17:42 -0700, Jackie Li wrote:
> After enabled the WOPCM write-once registers locking status checking,
> reloading of the i915 module will fail with modparam enable_guc set
> to 3
> (enable GuC and HuC firmware loading) if the module was originally
> loaded
> with enable_guc set to 1 (only enable GuC firmware loading). This is
> because WOPCM registers were updated and locked without considering
> the HuC
> FW size. Since we need both GuC and HuC FW sizes to determine the
> final
> layout of WOPCM, we should always calculate the WOPCM layout based on
> the
> actual sizes of the GuC and HuC firmware available for a specific
> platform
> if we need continue to support enable/disable HuC FW loading
> dynamically
> with enable_guc modparam.
> 
> This patch splits uC firmware fetching into two stages. First stage
> is to
> fetch the firmware image and verify the firmware header. uC firmware
> will
> be marked as verified and this will make FW info available for
> following
> WOPCM layout calculation. The second stage is to create a GEM object
> and
> copy the FW data into the created GEM object which will only be
> available
> when GuC/HuC loading is enabled by enable_guc modparam. This will
> guarantee
> that the WOPCM layout will be always be calculated correctly without
> making
> any assumptions to the GuC and HuC firmware sizes.
> 
> v3:
>  - Rebase
> 
> Signed-off-by: Jackie Li <yaodong...@intel.com>
> Cc: Michal Wajdeczko <michal.wajdec...@intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kam...@intel.com>
> Cc: Michal Winiarski <michal.winiar...@intel.com>
> Cc: John Spotswood <john.a.spotsw...@intel.com>
> Cc: Joonas Lahtinen <joonas.lahti...@linux.intel.com>

Reviewed-by: John Spotswood <john.a.spotsw...@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_uc.c| 14 --
>  drivers/gpu/drm/i915/intel_uc_fw.c | 31 --
> -
>  drivers/gpu/drm/i915/intel_uc_fw.h |  7 +--
>  3 files changed, 29 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_uc.c
> b/drivers/gpu/drm/i915/intel_uc.c
> index 1cffaf7..73b8f6c 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -172,11 +172,8 @@ void intel_uc_init_early(struct drm_i915_private
> *i915)
>  
>   sanitize_options_early(i915);
>  
> - if (USES_GUC(i915))
> - intel_uc_fw_fetch(i915, >fw);
> -
> - if (USES_HUC(i915))
> - intel_uc_fw_fetch(i915, >fw);
> + intel_uc_fw_fetch(i915, >fw, USES_GUC(i915));
> + intel_uc_fw_fetch(i915, >fw, USES_HUC(i915));
>  }
>  
>  void intel_uc_cleanup_early(struct drm_i915_private *i915)
> @@ -184,11 +181,8 @@ void intel_uc_cleanup_early(struct
> drm_i915_private *i915)
>   struct intel_guc *guc = >guc;
>   struct intel_huc *huc = >huc;
>  
> - if (USES_HUC(i915))
> - intel_uc_fw_fini(>fw);
> -
> - if (USES_GUC(i915))
> - intel_uc_fw_fini(>fw);
> + intel_uc_fw_fini(>fw);
> + intel_uc_fw_fini(>fw);
>  
>   guc_free_load_err_log(guc);
>  }
> diff --git a/drivers/gpu/drm/i915/intel_uc_fw.c
> b/drivers/gpu/drm/i915/intel_uc_fw.c
> index 6e8e0b5..a9cb900 100644
> --- a/drivers/gpu/drm/i915/intel_uc_fw.c
> +++ b/drivers/gpu/drm/i915/intel_uc_fw.c
> @@ -33,11 +33,13 @@
>   *
>   * @dev_priv: device private
>   * @uc_fw: uC firmware
> + * @copy_to_obj: whether fetch uC firmware into GEM object or not
>   *
> - * Fetch uC firmware into GEM obj.
> + * Fetch and verify uC firmware and copy firmware data into GEM
> object if
> + * @copy_to_obj is true.
>   */
>  void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
> -    struct intel_uc_fw *uc_fw)
> +    struct intel_uc_fw *uc_fw, bool copy_to_obj)
>  {
>   struct pci_dev *pdev = dev_priv->drm.pdev;
>   struct drm_i915_gem_object *obj;
> @@ -154,17 +156,24 @@ void intel_uc_fw_fetch(struct drm_i915_private
> *dev_priv,
>   goto fail;
>   }
>  
> - obj = i915_gem_object_create_from_data(dev_priv, fw->data,
> fw->size);
> - if (IS_ERR(obj)) {
> - err = PTR_ERR(obj);
> - DRM_DEBUG_DRIVER("%s fw object_create err=%d\n",
> -  intel_uc_fw_type_repr(uc_fw->type), 
> err);
> - goto fail;
> + uc_fw->size = fw->size;
> + uc_fw->fetch_status = INTEL_UC_FIRMWARE_VERIFIED;
> +
> + if (copy_to_obj) {
> + obj = i915_gem_object_create_from_data(dev_priv, fw-
>

Re: [Intel-gfx] [PATCH] drm/i915/guc: Removed unused GuC parameters.

[John Spotswood]

On Mon, 2018-03-05 at 03:12 -0800, Piorkowski, Piotr wrote:
> On Fri, 2018-03-02 at 12:53 +0530, Sagar Arun Kamble wrote:
> > 
> > 
> > On 3/2/2018 12:44 AM, John Spotswood wrote:
> > > 
> > > On Thu, 2018-03-01 at 17:35 +0530, Sagar Arun Kamble wrote:
> > > > 
> > > > On 3/1/2018 1:32 PM, Chris Wilson wrote:
> > > > > 
> > > > > Quoting Michel Thierry (2018-02-28 22:07:51)
> > > > > > 
> > > > > > On 28/02/18 12:26, Michel Thierry wrote:
> > > > > > > 
> > > > > > > On 28/02/18 10:42, Piotr Piórkowski wrote:
> > > > > > > > 
> > > > > > > > In the i915 driver, there is a function,
> > > > > > > > intel_guc_init_params(),
> > > > > > > > which initializes the GuC parameter block which is
> > > > > > > > passed
> > > > > > > > into
> > > > > > > > the GuC. There is parameter GUC_CTL_DEVICE_INFO with
> > > > > > > > values
> > > > > > > > GfxGtType and GfxCoreFamily unused by GuC.
> > > > > > > > 
> > > > > > > > This patch remove GUC_CTL_DEVICE_INFO with GfxGtType
> > > > > > > > and
> > > > > > > > GfxCoreFamily parameters and also unnecessary functions
> > > > > > > > get_gt_type() and get_core_family().
> > > > > > > > 
> > > > > > > Hi,
> > > > > > > 
> > > > > > > Looking at the fw code, you're partially right, GfxGtType
> > > > > > > is
> > > > > > > ignored...
> > > > > > > but GfxCoreFamily isn't.
> > > > > > > 
> > > > > > Unless whoever wrote the fw was smart enough to forget to
> > > > > > call
> > > > > > the
> > > > > > function that is reading GfxCoreFamily... I didn't count on
> > > > > > that.
> > > > > Is the intention to use GfxCoreFamily documented, i.e. are
> > > > > they
> > > > > expecting it part of the interface and may re-instantiate the
> > > > > check
> > > > > "because it was always supposed to exist" in some future
> > > > > version?
> > > > Usage of GfxCoreFamily is only in SLPC and for platform
> > > > specific
> > > > initialization and might be removed in future interfaces.
> > > > If needed, we can add as part of SLPC patches.
> > > Michel and I have traced through the FW code, and both parameters
> > > are
> > > unused.  GfxCoreFamily does appear to be set in the FW, and it
> > > gets
> > > passed into SLPC, but then it never gets used.
> > Hi John,
> > 
> > It is needed for SLPC initialization. Verified on v9 GuC firmware
> > that 
> > SLPC GTPERF gets disabled if i915 does not send this param.
> > We can add this param as part of SLPC patches for GuC versions
> > which 
> > need them.
> Ok, so I think that we should remove this param from i915, and than
> if 
> it is needed, we can add this param as part of SLPC patches, as Sagar
> said.
I have gone back and taken another look at the FW code, and Sagar is
correct. There is a link there.  Apologies for the mistake.  With that
in mind, I'm not clear why we would remove the parameter only to add it
back with the SLPC patches.
> 
> -Piotr 
> > 
> > 
> > Thanks
> > Sagar
> > > 
> > >    I have confirmed with
> > > FW developers that these parameters have been removed for future
> > > gens.
> > > > 
> > > > > 
> > > > > -Chris
> > > > > ___
> > > > > Intel-gfx mailing list
> > > > > Intel-gfx@lists.freedesktop.org
> > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/guc: Removed unused GuC parameters.

[John Spotswood]

On Thu, 2018-03-01 at 17:35 +0530, Sagar Arun Kamble wrote:
> 
> On 3/1/2018 1:32 PM, Chris Wilson wrote:
> > 
> > Quoting Michel Thierry (2018-02-28 22:07:51)
> > > 
> > > On 28/02/18 12:26, Michel Thierry wrote:
> > > > 
> > > > On 28/02/18 10:42, Piotr Piórkowski wrote:
> > > > > 
> > > > > In the i915 driver, there is a function,
> > > > > intel_guc_init_params(),
> > > > > which initializes the GuC parameter block which is passed
> > > > > into
> > > > > the GuC. There is parameter GUC_CTL_DEVICE_INFO with values
> > > > > GfxGtType and GfxCoreFamily unused by GuC.
> > > > > 
> > > > > This patch remove GUC_CTL_DEVICE_INFO with GfxGtType and
> > > > > GfxCoreFamily parameters and also unnecessary functions
> > > > > get_gt_type() and get_core_family().
> > > > > 
> > > > Hi,
> > > > 
> > > > Looking at the fw code, you're partially right, GfxGtType is
> > > > ignored...
> > > > but GfxCoreFamily isn't.
> > > > 
> > > Unless whoever wrote the fw was smart enough to forget to call
> > > the
> > > function that is reading GfxCoreFamily... I didn't count on that.
> > Is the intention to use GfxCoreFamily documented, i.e. are they
> > expecting it part of the interface and may re-instantiate the check
> > "because it was always supposed to exist" in some future version?
> Usage of GfxCoreFamily is only in SLPC and for platform specific 
> initialization and might be removed in future interfaces.
> If needed, we can add as part of SLPC patches.
Michel and I have traced through the FW code, and both parameters are
unused.  GfxCoreFamily does appear to be set in the FW, and it gets
passed into SLPC, but then it never gets used.  I have confirmed with
FW developers that these parameters have been removed for future gens.
> > 
> > -Chris
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/2] drm/i915/GLK/HuC: Load HuC on GLK

[John Spotswood]

On Thu, 2017-03-30 at 13:24 -0700, Anusha Srivatsa wrote:
> Load HuC version 1.07.1748 on GLK.
> 
> v2: rebased.
> v3: Use name of the right platform(John Spotswood)
> v4: rebased.
> 
> Cc: Jeff Mcgee <jeff.mc...@intel.com>
> Cc: John Spotswood <john.a.spotsw...@intel.com>
> Cc: Rodrigo Vivi <rodrigo.v...@intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.sriva...@intel.com>

Reviewed-by: John Spotswood <john.a.spotsw...@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_huc.c | 11 +++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_huc.c
> b/drivers/gpu/drm/i915/intel_huc.c
> index 7af900b..ea2b325 100644
> --- a/drivers/gpu/drm/i915/intel_huc.c
> +++ b/drivers/gpu/drm/i915/intel_huc.c
> @@ -52,6 +52,10 @@
>  #define KBL_HUC_FW_MINOR 00
>  #define KBL_BLD_NUM 1810
>  
> +#define GLK_HUC_FW_MAJOR 01
> +#define GLK_HUC_FW_MINOR 07
> +#define GLK_BLD_NUM 1748
> +
>  #define HUC_FW_PATH(platform, major, minor, bld_num) \
>   "i915/" __stringify(platform) "_huc_ver" __stringify(major)
> "_" \
>   __stringify(minor) "_" __stringify(bld_num) ".bin"
> @@ -68,6 +72,9 @@ MODULE_FIRMWARE(I915_BXT_HUC_UCODE);
>   KBL_HUC_FW_MINOR, KBL_BLD_NUM)
>  MODULE_FIRMWARE(I915_KBL_HUC_UCODE);
>  
> +#define I915_GLK_HUC_UCODE HUC_FW_PATH(glk, GLK_HUC_FW_MAJOR, \
> + GLK_HUC_FW_MINOR, GLK_BLD_NUM)
> +
>  /**
>   * huc_ucode_xfer() - DMA's the firmware
>   * @dev_priv: the drm_i915_private device
> @@ -169,6 +176,10 @@ void intel_huc_select_fw(struct intel_huc *huc)
>   huc->fw.path = I915_KBL_HUC_UCODE;
>   huc->fw.major_ver_wanted = KBL_HUC_FW_MAJOR;
>   huc->fw.minor_ver_wanted = KBL_HUC_FW_MINOR;
> + } else if (IS_GEMINILAKE(dev_priv)) {
> + huc->fw.path = I915_GLK_HUC_UCODE;
> + huc->fw.major_ver_wanted = GLK_HUC_FW_MAJOR;
> + huc->fw.minor_ver_wanted = GLK_HUC_FW_MINOR;
>   } else {
>   DRM_ERROR("No HuC firmware known for platform with
> HuC!\n");
>   return;
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm/i915/GuC/GLK: Load GuC on GLK

[John Spotswood]

On Thu, 2017-03-30 at 13:24 -0700, Anusha Srivatsa wrote:
> Load GuC 10.56 on GLK. Work on firmware is still
> in progress. Testing has not been done yet.
> This patch addresses the initial need to load the GuC
> firmware for HuC authentication
> 
> v2: rebased.
> 
> Cc: Jeff mcgee <jeff.mc...@intel.com>
> Cc: Rodrigo Vivi <rodrigo.v...@intel.com>
> Cc: John Spotswood <john.a.spotsw...@intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.sriva...@intel.com>

Reviewed-by: John Spotswood <john.a.spotsw...@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_guc_loader.c | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c
> b/drivers/gpu/drm/i915/intel_guc_loader.c
> index 7d92321..1f8edf0 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -61,6 +61,9 @@
>  #define KBL_FW_MAJOR 9
>  #define KBL_FW_MINOR 14
>  
> +#define GLK_FW_MAJOR 10
> +#define GLK_FW_MINOR 56
> +
>  #define GUC_FW_PATH(platform, major, minor) \
> "i915/" __stringify(platform) "_guc_ver" __stringify(major)
> "_" __stringify(minor) ".bin"
>  
> @@ -73,6 +76,8 @@ MODULE_FIRMWARE(I915_BXT_GUC_UCODE);
>  #define I915_KBL_GUC_UCODE GUC_FW_PATH(kbl, KBL_FW_MAJOR,
> KBL_FW_MINOR)
>  MODULE_FIRMWARE(I915_KBL_GUC_UCODE);
>  
> +#define I915_GLK_GUC_UCODE GUC_FW_PATH(glk, GLK_FW_MAJOR,
> GLK_FW_MINOR)
> +
>  /* User-friendly representation of an enum */
>  const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status)
>  {
> @@ -421,6 +426,10 @@ int intel_guc_select_fw(struct intel_guc *guc)
>   guc->fw.path = I915_KBL_GUC_UCODE;
>   guc->fw.major_ver_wanted = KBL_FW_MAJOR;
>   guc->fw.minor_ver_wanted = KBL_FW_MINOR;
> + } else if (IS_GEMINILAKE(dev_priv)) {
> + guc->fw.path = I915_GLK_GUC_UCODE;
> + guc->fw.major_ver_wanted = GLK_FW_MAJOR;
> + guc->fw.minor_ver_wanted = GLK_FW_MINOR;
>   } else {
>   DRM_ERROR("No GuC firmware known for platform with
> GuC!\n");
>   return -ENOENT;
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm/i915/GuC/GLK: Load GuC on GLK

[John Spotswood]

On Tue, 2017-03-28 at 17:01 -0700, Vivi, Rodrigo wrote:
> On Tue, 2017-03-28 at 22:11 +, Srivatsa, Anusha wrote:
> > 
> > 
> > > 
> > > -Original Message-
> > > From: Spotswood, John A
> > > Sent: Tuesday, March 28, 2017 2:35 PM
> > > To: Srivatsa, Anusha ; intel-
> > > g...@lists.freedesktop.org
> > > Cc: Mcgee, Jeff ; Vivi, Rodrigo  > > v...@intel.com>
> > > Subject: Re: [PATCH 1/2] drm/i915/GuC/GLK: Load GuC on GLK
> > > 
> > > 
> > > > @@ -73,6 +76,8 @@ MODULE_FIRMWARE(I915_BXT_GUC_UCODE);
> > > >  #define I915_KBL_GUC_UCODE GUC_FW_PATH(kbl, KBL_FW_MAJOR,
> > > > KBL_FW_MINOR)
> > > >  MODULE_FIRMWARE(I915_KBL_GUC_UCODE);
> > > > 
> > > > +#define I915_GLK_GUC_UCODE GUC_FW_PATH(glk, GLK_FW_MAJOR,
> > > > GLK_FW_MINOR)
> > > You need a line after this #define that says the following:
> > >   MODULE_FIRMWARE(I915_GLK_GUC_UCODE);
> >  
> > 
> > Hi John, we have decided to not use it in pre-production platforms.
> > Using MODULE_FIRMWARE is going to shout that the firmware is not
> > available in systems. Since we will not be releasing the firmware
> > binary to the public yet,  it is unnecessary noise.
> +MODULE_FIRMARE() should be in a separated patch and only get merged
> after the firmware got released at 01.org propagated and merged to
> linux-firmware.git.
> 

Makes sense.  My mistake.

John

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

Re: [Intel-gfx] [PATCH 1/2] drm/i915/GuC/GLK: Load GuC on GLK

[John Spotswood]

On Tue, 2017-03-21 at 14:09 -0700, Anusha Srivatsa wrote:
> Load GuC 10.56 on GLK. Work on firmware is still
> in progress. Testing has not been done yet.
> This patch addresses the initial need to load the GuC
> firmware for HuC authentication
> 
> Cc: Jeff mcgee <jeff.mc...@intel.com>
> Cc: Rodrigo Vivi <rodrigo.v...@intel.com>
> Cc: John Spotswood <john.a.spotsw...@intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.sriva...@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_guc_loader.c | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c
> b/drivers/gpu/drm/i915/intel_guc_loader.c
> index 2f270d0..a6899df 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -61,6 +61,9 @@
>  #define KBL_FW_MAJOR 9
>  #define KBL_FW_MINOR 14
>  
> +#define GLK_FW_MAJOR 10
> +#define GLK_FW_MINOR 56
> +
>  #define GUC_FW_PATH(platform, major, minor) \
> "i915/" __stringify(platform) "_guc_ver" __stringify(major)
> "_" __stringify(minor) ".bin"
>  
> @@ -73,6 +76,8 @@ MODULE_FIRMWARE(I915_BXT_GUC_UCODE);
>  #define I915_KBL_GUC_UCODE GUC_FW_PATH(kbl, KBL_FW_MAJOR,
> KBL_FW_MINOR)
>  MODULE_FIRMWARE(I915_KBL_GUC_UCODE);
>  
> +#define I915_GLK_GUC_UCODE GUC_FW_PATH(glk, GLK_FW_MAJOR,
> GLK_FW_MINOR)

You need a line after this #define that says the following:
   MODULE_FIRMWARE(I915_GLK_GUC_UCODE);

> +
>  /* User-friendly representation of an enum */
>  const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status)
>  {
> @@ -423,6 +428,10 @@ int intel_guc_select_fw(struct intel_guc *guc)
>   guc->fw.path = I915_KBL_GUC_UCODE;
>   guc->fw.major_ver_wanted = KBL_FW_MAJOR;
>   guc->fw.minor_ver_wanted = KBL_FW_MINOR;
> + } else if (IS_GEMINILAKE(dev_priv)) {
> + guc->fw.path = I915_GLK_GUC_UCODE;
> + guc->fw.major_ver_wanted = GLK_FW_MAJOR;
> + guc->fw.minor_ver_wanted = GLK_FW_MINOR;
>   } else {
>   DRM_ERROR("No GuC firmware known for platform with
> GuC!\n");
>   return -ENOENT;
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/GLK/HuC: Load HuC on GLK

[John Spotswood]

On Tue, 2017-03-21 at 17:29 -0700, Anusha Srivatsa wrote:
> Load HuC version 1.07.1748 on GLK.
> 
> v2: rebased.
> v3: Use name of the right platform(John Spotswood)
> 
> Cc: Jeff Mcgee <jeff.mc...@intel.com>
> Cc: John Spotswood <john.a.spotsw...@intel.com>
> Cc: Rodrigo Vivi <rodrigo.v...@intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.sriva...@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_huc.c | 11 +++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_huc.c
> b/drivers/gpu/drm/i915/intel_huc.c
> index 7af900b..ea2b325 100644
> --- a/drivers/gpu/drm/i915/intel_huc.c
> +++ b/drivers/gpu/drm/i915/intel_huc.c
> @@ -52,6 +52,10 @@
>  #define KBL_HUC_FW_MINOR 00
>  #define KBL_BLD_NUM 1810
>  
> +#define GLK_HUC_FW_MAJOR 01
> +#define GLK_HUC_FW_MINOR 07
> +#define GLK_BLD_NUM 1748
> +
>  #define HUC_FW_PATH(platform, major, minor, bld_num) \
>   "i915/" __stringify(platform) "_huc_ver" __stringify(major)
> "_" \
>   __stringify(minor) "_" __stringify(bld_num) ".bin"
> @@ -68,6 +72,9 @@ MODULE_FIRMWARE(I915_BXT_HUC_UCODE);
>   KBL_HUC_FW_MINOR, KBL_BLD_NUM)
>  MODULE_FIRMWARE(I915_KBL_HUC_UCODE);
>  
> +#define I915_GLK_HUC_UCODE HUC_FW_PATH(glk, GLK_HUC_FW_MAJOR, \
> + GLK_HUC_FW_MINOR, GLK_BLD_NUM)

I think you need a line after this #define that says the following:
   MODULE_FIRMWARE(I915_GLK_HUC_UCODE);

> +
>  /**
>   * huc_ucode_xfer() - DMA's the firmware
>   * @dev_priv: the drm_i915_private device
> @@ -169,6 +176,10 @@ void intel_huc_select_fw(struct intel_huc *huc)
>   huc->fw.path = I915_KBL_HUC_UCODE;
>   huc->fw.major_ver_wanted = KBL_HUC_FW_MAJOR;
>   huc->fw.minor_ver_wanted = KBL_HUC_FW_MINOR;
> + } else if (IS_GEMINILAKE(dev_priv)) {
> + huc->fw.path = I915_GLK_HUC_UCODE;
> + huc->fw.major_ver_wanted = GLK_HUC_FW_MAJOR;
> + huc->fw.minor_ver_wanted = GLK_HUC_FW_MINOR;
>   } else {
>   DRM_ERROR("No HuC firmware known for platform with
> HuC!\n");
>   return;
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/2] drm/i915/HuC/GLK: Load HuC on GLK

[John Spotswood]

On 03/21/2017 02:10 PM, Anusha Srivatsa wrote:

Load HuC version 1.07.1748 on GLK.

Cc: Jeff Mcgee <jeff.mc...@intel.com>
Cc: John Spotswood <john.a.spotsw...@intel.com>
Cc: Rodrigo Vivi <rodrigo.v...@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.sriva...@intel.com>
---
  drivers/gpu/drm/i915/intel_huc.c | 11 +++
  1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
index 7af900b..8dfb917 100644
--- a/drivers/gpu/drm/i915/intel_huc.c
+++ b/drivers/gpu/drm/i915/intel_huc.c
@@ -52,6 +52,10 @@
  #define KBL_HUC_FW_MINOR 00
  #define KBL_BLD_NUM 1810
  
+#define GLK_HUC_FW_MAJOR 01

+#define GLK_HUC_FW_MINOR 07
+#define GLK_BLD_NUM 1748
+
  #define HUC_FW_PATH(platform, major, minor, bld_num) \
"i915/" __stringify(platform) "_huc_ver" __stringify(major) "_" \
__stringify(minor) "_" __stringify(bld_num) ".bin"
@@ -68,6 +72,9 @@ MODULE_FIRMWARE(I915_BXT_HUC_UCODE);
KBL_HUC_FW_MINOR, KBL_BLD_NUM)
  MODULE_FIRMWARE(I915_KBL_HUC_UCODE);
  
+#define I915_GLK_HUC_UCODE HUC_FW_PATH(glk, GLK_HUC_FW_MAJOR, \

+   GLK_HUC_FW_MINOR, GLK_BLD_NUM)
+
  /**
   * huc_ucode_xfer() - DMA's the firmware
   * @dev_priv: the drm_i915_private device
@@ -169,6 +176,10 @@ void intel_huc_select_fw(struct intel_huc *huc)
huc->fw.path = I915_KBL_HUC_UCODE;
huc->fw.major_ver_wanted = KBL_HUC_FW_MAJOR;
huc->fw.minor_ver_wanted = KBL_HUC_FW_MINOR;
+   } else if (IS_GEMINILAKE(dev_priv)) {
+   huc->fw.path = I915_GLK_HUC_UCODE;
+   huc->fw.major_ver_wanted = KBL_HUC_FW_MAJOR;
+   huc->fw.minor_ver_wanted = KBL_HUC_FW_MINOR;


These should be GLK_HUC_FW_MAJOR and GLK_HUC_FW_MINOR.


} else {
DRM_ERROR("No HuC firmware known for platform with HuC!\n");
return;


Regards,
John

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