[Intel-gfx] [PATCH 1/8] drm/i915/guc: Make the GuC fw loading helper functions general
Arkadiusz Hiler
arkadiusz.hiler at intel.com
Fri Dec 9 11:49:02 UTC 2016
On Fri, Dec 09, 2016 at 12:28:52PM +0100, Michal Wajdeczko wrote:
> On Thu, Dec 08, 2016 at 03:02:12PM -0800, anushasr wrote:
> > From: Peter Antoine <peter.antoine at intel.com>
> >
> > Rename some of the GuC fw loading code to make them more general. We
> > will utilise them for HuC loading as well.
> > s/intel_guc_fw/intel_uc_fw/g
> > s/GUC_FIRMWARE/UC_FIRMWARE/g
> >
> > Struct intel_guc_fw is renamed to intel_uc_fw. Prefix of tts members,
> > such as 'guc' or 'guc_fw' either is renamed to 'uc' or removed for
> > same purpose.
> >
> > v2: rebased on top of nightly.
> > reapplied the search/replace as upstream code as changed.
> > v3: rebased again on drm-nightly.
> > v4: removed G from messages in shared fw fetch function.
> > v5: rebased.
> > v7: rebased.
> > v8: rebased.
> > v9: rebased.
> > v10: rebased.
> > v11: rebased.
> > v12: rebased on top of drm-tip
> > v13: rebased.Updated dev to dev_priv in intel_guc_setup(), guc_fw_getch()
> > and intel_guc_init().
> >
> > Signed-off-by: Anusha Srivatsa <anusha.srivatsa at intel.com>
> > Signed-off-by: Alex Dai <yu.dai at intel.com>
> > Signed-off-by: Peter Antoine <peter.antoine at intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_debugfs.c | 12 +--
> > drivers/gpu/drm/i915/i915_guc_submission.c | 4 +-
> > drivers/gpu/drm/i915/intel_guc_loader.c | 157 +++++++++++++++--------------
> > drivers/gpu/drm/i915/intel_uc.h | 40 ++++----
> > 4 files changed, 108 insertions(+), 105 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index a746130..0e5ef62 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -2338,7 +2338,7 @@ static int i915_llc(struct seq_file *m, void *data)
> > static int i915_guc_load_status_info(struct seq_file *m, void *data)
> > {
> > struct drm_i915_private *dev_priv = node_to_i915(m->private);
> > - struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw;
> > + struct intel_uc_fw *guc_fw = &dev_priv->guc.guc_fw;
> > u32 tmp, i;
> >
> > if (!HAS_GUC_UCODE(dev_priv))
> > @@ -2346,15 +2346,15 @@ static int i915_guc_load_status_info(struct seq_file *m, void *data)
> >
> > seq_printf(m, "GuC firmware status:\n");
> > seq_printf(m, "\tpath: %s\n",
> > - guc_fw->guc_fw_path);
> > + guc_fw->uc_fw_path);
> > seq_printf(m, "\tfetch: %s\n",
> > - intel_guc_fw_status_repr(guc_fw->guc_fw_fetch_status));
> > + intel_uc_fw_status_repr(guc_fw->fetch_status));
> > seq_printf(m, "\tload: %s\n",
> > - intel_guc_fw_status_repr(guc_fw->guc_fw_load_status));
> > + intel_uc_fw_status_repr(guc_fw->load_status));
> > seq_printf(m, "\tversion wanted: %d.%d\n",
> > - guc_fw->guc_fw_major_wanted, guc_fw->guc_fw_minor_wanted);
> > + guc_fw->major_ver_wanted, guc_fw->minor_ver_wanted);
> > seq_printf(m, "\tversion found: %d.%d\n",
> > - guc_fw->guc_fw_major_found, guc_fw->guc_fw_minor_found);
> > + guc_fw->major_ver_found, guc_fw->minor_ver_found);
> > seq_printf(m, "\theader: offset is %d; size = %d\n",
> > guc_fw->header_offset, guc_fw->header_size);
> > seq_printf(m, "\tuCode: offset is %d; size = %d\n",
> > diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> > index 7fa4e74..e156a4b 100644
> > --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> > +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> > @@ -1493,7 +1493,7 @@ int intel_guc_suspend(struct drm_i915_private *dev_priv)
> > struct i915_gem_context *ctx;
> > u32 data[3];
> >
> > - if (guc->guc_fw.guc_fw_load_status != GUC_FIRMWARE_SUCCESS)
> > + if (guc->guc_fw.load_status != UC_FIRMWARE_SUCCESS)
> > return 0;
> >
> > gen9_disable_guc_interrupts(dev_priv);
> > @@ -1520,7 +1520,7 @@ int intel_guc_resume(struct drm_i915_private *dev_priv)
> > struct i915_gem_context *ctx;
> > u32 data[3];
> >
> > - if (guc->guc_fw.guc_fw_load_status != GUC_FIRMWARE_SUCCESS)
> > + if (guc->guc_fw.load_status != UC_FIRMWARE_SUCCESS)
> > return 0;
> >
> > if (i915.guc_log_level >= 0)
> > diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
> > index 21db697..8f04f6e 100644
> > --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> > +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> > @@ -81,16 +81,16 @@ MODULE_FIRMWARE(I915_BXT_GUC_UCODE);
> > MODULE_FIRMWARE(I915_KBL_GUC_UCODE);
> >
> > /* User-friendly representation of an enum */
> > -const char *intel_guc_fw_status_repr(enum intel_guc_fw_status status)
> > +const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status)
> > {
> > switch (status) {
> > - case GUC_FIRMWARE_FAIL:
> > + case UC_FIRMWARE_FAIL:
> > return "FAIL";
> > - case GUC_FIRMWARE_NONE:
> > + case UC_FIRMWARE_NONE:
> > return "NONE";
> > - case GUC_FIRMWARE_PENDING:
> > + case UC_FIRMWARE_PENDING:
> > return "PENDING";
> > - case GUC_FIRMWARE_SUCCESS:
> > + case UC_FIRMWARE_SUCCESS:
> > return "SUCCESS";
> > default:
> > return "UNKNOWN!";
> > @@ -278,7 +278,7 @@ static inline bool guc_ucode_response(struct drm_i915_private *dev_priv,
> > static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv,
> > struct i915_vma *vma)
> > {
> > - struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw;
> > + struct intel_uc_fw *guc_fw = &dev_priv->guc.guc_fw;
> > unsigned long offset;
> > struct sg_table *sg = vma->pages;
> > u32 status, rsa[UOS_RSA_SCRATCH_MAX_COUNT];
> > @@ -350,17 +350,17 @@ static u32 guc_wopcm_size(struct drm_i915_private *dev_priv)
> > */
> > static int guc_ucode_xfer(struct drm_i915_private *dev_priv)
> > {
> > - struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw;
> > + struct intel_uc_fw *guc_fw = &dev_priv->guc.guc_fw;
> > struct i915_vma *vma;
> > int ret;
> >
> > - ret = i915_gem_object_set_to_gtt_domain(guc_fw->guc_fw_obj, false);
> > + ret = i915_gem_object_set_to_gtt_domain(guc_fw->uc_fw_obj, false);
> > if (ret) {
> > DRM_DEBUG_DRIVER("set-domain failed %d\n", ret);
> > return ret;
> > }
> >
> > - vma = i915_gem_object_ggtt_pin(guc_fw->guc_fw_obj, NULL, 0, 0, 0);
> > + vma = i915_gem_object_ggtt_pin(guc_fw->uc_fw_obj, NULL, 0, 0, 0);
> > if (IS_ERR(vma)) {
> > DRM_DEBUG_DRIVER("pin failed %d\n", (int)PTR_ERR(vma));
> > return PTR_ERR(vma);
> > @@ -450,14 +450,14 @@ static int guc_hw_reset(struct drm_i915_private *dev_priv)
> > */
> > int intel_guc_setup(struct drm_i915_private *dev_priv)
> > {
> > - struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw;
> > - const char *fw_path = guc_fw->guc_fw_path;
> > + struct intel_uc_fw *guc_fw = &dev_priv->guc.guc_fw;
> > + const char *fw_path = guc_fw->uc_fw_path;
> > int retries, ret, err;
> >
> > DRM_DEBUG_DRIVER("GuC fw status: path %s, fetch %s, load %s\n",
> > fw_path,
> > - intel_guc_fw_status_repr(guc_fw->guc_fw_fetch_status),
> > - intel_guc_fw_status_repr(guc_fw->guc_fw_load_status));
> > + intel_uc_fw_status_repr(guc_fw->fetch_status),
> > + intel_uc_fw_status_repr(guc_fw->load_status));
> >
> > /* Loading forbidden, or no firmware to load? */
> > if (!i915.enable_guc_loading) {
> > @@ -475,10 +475,10 @@ int intel_guc_setup(struct drm_i915_private *dev_priv)
> > }
> >
> > /* Fetch failed, or already fetched but failed to load? */
> > - if (guc_fw->guc_fw_fetch_status != GUC_FIRMWARE_SUCCESS) {
> > + if (guc_fw->fetch_status != UC_FIRMWARE_SUCCESS) {
> > err = -EIO;
> > goto fail;
> > - } else if (guc_fw->guc_fw_load_status == GUC_FIRMWARE_FAIL) {
> > + } else if (guc_fw->load_status == UC_FIRMWARE_FAIL) {
> > err = -ENOEXEC;
> > goto fail;
> > }
> > @@ -486,11 +486,11 @@ int intel_guc_setup(struct drm_i915_private *dev_priv)
> > guc_interrupts_release(dev_priv);
> > gen9_reset_guc_interrupts(dev_priv);
> >
> > - guc_fw->guc_fw_load_status = GUC_FIRMWARE_PENDING;
> > + guc_fw->load_status = UC_FIRMWARE_PENDING;
> >
> > DRM_DEBUG_DRIVER("GuC fw status: fetch %s, load %s\n",
> > - intel_guc_fw_status_repr(guc_fw->guc_fw_fetch_status),
> > - intel_guc_fw_status_repr(guc_fw->guc_fw_load_status));
> > + intel_uc_fw_status_repr(guc_fw->fetch_status),
> > + intel_uc_fw_status_repr(guc_fw->load_status));
> >
> > err = i915_guc_submission_init(dev_priv);
> > if (err)
> > @@ -522,11 +522,11 @@ int intel_guc_setup(struct drm_i915_private *dev_priv)
> > "retry %d more time(s)\n", err, retries);
> > }
> >
> > - guc_fw->guc_fw_load_status = GUC_FIRMWARE_SUCCESS;
> > + guc_fw->load_status = UC_FIRMWARE_SUCCESS;
> >
> > DRM_DEBUG_DRIVER("GuC fw status: fetch %s, load %s\n",
> > - intel_guc_fw_status_repr(guc_fw->guc_fw_fetch_status),
> > - intel_guc_fw_status_repr(guc_fw->guc_fw_load_status));
> > + intel_uc_fw_status_repr(guc_fw->fetch_status),
> > + intel_uc_fw_status_repr(guc_fw->load_status));
> >
> > if (i915.enable_guc_submission) {
> > if (i915.guc_log_level >= 0)
> > @@ -541,8 +541,8 @@ int intel_guc_setup(struct drm_i915_private *dev_priv)
> > return 0;
> >
> > fail:
> > - if (guc_fw->guc_fw_load_status == GUC_FIRMWARE_PENDING)
> > - guc_fw->guc_fw_load_status = GUC_FIRMWARE_FAIL;
> > + if (guc_fw->load_status == UC_FIRMWARE_PENDING)
> > + guc_fw->load_status = UC_FIRMWARE_FAIL;
> >
> > guc_interrupts_release(dev_priv);
> > i915_guc_submission_disable(dev_priv);
> > @@ -587,8 +587,8 @@ int intel_guc_setup(struct drm_i915_private *dev_priv)
> > return ret;
> > }
> >
> > -static void guc_fw_fetch(struct drm_i915_private *dev_priv,
> > - struct intel_guc_fw *guc_fw)
> > +void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
> > + struct intel_uc_fw *uc_fw)
> > {
> > struct pci_dev *pdev = dev_priv->drm.pdev;
> > struct drm_i915_gem_object *obj;
> > @@ -597,17 +597,17 @@ static void guc_fw_fetch(struct drm_i915_private *dev_priv,
> > size_t size;
> > int err;
> >
> > - DRM_DEBUG_DRIVER("before requesting firmware: GuC fw fetch status %s\n",
> > - intel_guc_fw_status_repr(guc_fw->guc_fw_fetch_status));
> > + DRM_DEBUG_DRIVER("before requesting firmware: uC fw fetch status %s\n",
> > + intel_uc_fw_status_repr(uc_fw->fetch_status));
> >
> > - err = request_firmware(&fw, guc_fw->guc_fw_path, &pdev->dev);
> > + err = request_firmware(&fw, uc_fw->uc_fw_path, &pdev->dev);
> > if (err)
> > goto fail;
> > if (!fw)
> > goto fail;
> >
> > - DRM_DEBUG_DRIVER("fetch GuC fw from %s succeeded, fw %p\n",
> > - guc_fw->guc_fw_path, fw);
> > + DRM_DEBUG_DRIVER("fetch uC fw from %s succeeded, fw %p\n",
> > + uc_fw->uc_fw_path, fw);
> >
> > /* Check the size of the blob before examining buffer contents */
> > if (fw->size < sizeof(struct guc_css_header)) {
> > @@ -618,36 +618,36 @@ static void guc_fw_fetch(struct drm_i915_private *dev_priv,
> > css = (struct guc_css_header *)fw->data;
> >
> > /* Firmware bits always start from header */
> > - guc_fw->header_offset = 0;
> > - guc_fw->header_size = (css->header_size_dw - css->modulus_size_dw -
> > + uc_fw->header_offset = 0;
> > + uc_fw->header_size = (css->header_size_dw - css->modulus_size_dw -
> > css->key_size_dw - css->exponent_size_dw) * sizeof(u32);
> >
> > - if (guc_fw->header_size != sizeof(struct guc_css_header)) {
> > + if (uc_fw->header_size != sizeof(struct guc_css_header)) {
> > DRM_NOTE("CSS header definition mismatch\n");
> > goto fail;
> > }
> >
> > /* then, uCode */
> > - guc_fw->ucode_offset = guc_fw->header_offset + guc_fw->header_size;
> > - guc_fw->ucode_size = (css->size_dw - css->header_size_dw) * sizeof(u32);
> > + uc_fw->ucode_offset = uc_fw->header_offset + uc_fw->header_size;
> > + uc_fw->ucode_size = (css->size_dw - css->header_size_dw) * sizeof(u32);
> >
> > /* now RSA */
> > if (css->key_size_dw != UOS_RSA_SCRATCH_MAX_COUNT) {
> > DRM_NOTE("RSA key size is bad\n");
> > goto fail;
> > }
> > - guc_fw->rsa_offset = guc_fw->ucode_offset + guc_fw->ucode_size;
> > - guc_fw->rsa_size = css->key_size_dw * sizeof(u32);
> > + uc_fw->rsa_offset = uc_fw->ucode_offset + uc_fw->ucode_size;
> > + uc_fw->rsa_size = css->key_size_dw * sizeof(u32);
> >
> > /* At least, it should have header, uCode and RSA. Size of all three. */
> > - size = guc_fw->header_size + guc_fw->ucode_size + guc_fw->rsa_size;
> > + size = uc_fw->header_size + uc_fw->ucode_size + uc_fw->rsa_size;
> > if (fw->size < size) {
> > DRM_NOTE("Missing firmware components\n");
> > goto fail;
> > }
> >
> > /* Header and uCode will be loaded to WOPCM. Size of the two. */
> > - size = guc_fw->header_size + guc_fw->ucode_size;
> > + size = uc_fw->header_size + uc_fw->ucode_size;
> > if (size > guc_wopcm_size(dev_priv)) {
> > DRM_NOTE("Firmware is too large to fit in WOPCM\n");
> > goto fail;
> > @@ -659,21 +659,21 @@ static void guc_fw_fetch(struct drm_i915_private *dev_priv,
> > * TWO bytes each (i.e. u16), although all pointers and offsets are defined
> > * in terms of bytes (u8).
> > */
> > - guc_fw->guc_fw_major_found = css->guc_sw_version >> 16;
> > - guc_fw->guc_fw_minor_found = css->guc_sw_version & 0xFFFF;
> > -
> > - if (guc_fw->guc_fw_major_found != guc_fw->guc_fw_major_wanted ||
> > - guc_fw->guc_fw_minor_found < guc_fw->guc_fw_minor_wanted) {
> > - DRM_NOTE("GuC firmware version %d.%d, required %d.%d\n",
> > - guc_fw->guc_fw_major_found, guc_fw->guc_fw_minor_found,
> > - guc_fw->guc_fw_major_wanted, guc_fw->guc_fw_minor_wanted);
> > + uc_fw->major_ver_found = css->guc_sw_version >> 16;
> > + uc_fw->minor_ver_found = css->guc_sw_version & 0xFFFF;
> > +
> > + if (uc_fw->major_ver_found != uc_fw->major_ver_wanted ||
> > + uc_fw->minor_ver_found < uc_fw->minor_ver_wanted) {
> > + DRM_NOTE("uC firmware version %d.%d, required %d.%d\n",
> > + uc_fw->major_ver_found, uc_fw->minor_ver_found,
> > + uc_fw->major_ver_wanted, uc_fw->minor_ver_wanted);
> > err = -ENOEXEC;
> > goto fail;
> > }
> >
> > DRM_DEBUG_DRIVER("firmware version %d.%d OK (minimum %d.%d)\n",
> > - guc_fw->guc_fw_major_found, guc_fw->guc_fw_minor_found,
> > - guc_fw->guc_fw_major_wanted, guc_fw->guc_fw_minor_wanted);
> > + uc_fw->major_ver_found, uc_fw->minor_ver_found,
> > + uc_fw->major_ver_wanted, uc_fw->minor_ver_wanted);
> >
> > mutex_lock(&dev_priv->drm.struct_mutex);
> > obj = i915_gem_object_create_from_data(dev_priv, fw->data, fw->size);
> > @@ -683,31 +683,31 @@ static void guc_fw_fetch(struct drm_i915_private *dev_priv,
> > goto fail;
> > }
> >
> > - guc_fw->guc_fw_obj = obj;
> > - guc_fw->guc_fw_size = fw->size;
> > + uc_fw->uc_fw_obj = obj;
> > + uc_fw->uc_fw_size = fw->size;
> >
> > - DRM_DEBUG_DRIVER("GuC fw fetch status SUCCESS, obj %p\n",
> > - guc_fw->guc_fw_obj);
> > + DRM_DEBUG_DRIVER("uC fw fetch status SUCCESS, obj %p\n",
> > + uc_fw->uc_fw_obj);
> >
> > release_firmware(fw);
> > - guc_fw->guc_fw_fetch_status = GUC_FIRMWARE_SUCCESS;
> > + uc_fw->fetch_status = UC_FIRMWARE_SUCCESS;
> > return;
> >
> > fail:
> > - DRM_WARN("Failed to fetch valid GuC firmware from %s (error %d)\n",
> > - guc_fw->guc_fw_path, err);
> > - DRM_DEBUG_DRIVER("GuC fw fetch status FAIL; err %d, fw %p, obj %p\n",
> > - err, fw, guc_fw->guc_fw_obj);
> > + DRM_WARN("Failed to fetch valid uC firmware from %s (error %d)\n",
> > + uc_fw->uc_fw_path, err);
> > + DRM_DEBUG_DRIVER("uC fw fetch status FAIL; err %d, fw %p, obj %p\n",
> > + err, fw, uc_fw->uc_fw_obj);
> >
> > mutex_lock(&dev_priv->drm.struct_mutex);
> > - obj = guc_fw->guc_fw_obj;
> > + obj = uc_fw->uc_fw_obj;
> > if (obj)
> > i915_gem_object_put(obj);
> > - guc_fw->guc_fw_obj = NULL;
> > + uc_fw->uc_fw_obj = NULL;
> > mutex_unlock(&dev_priv->drm.struct_mutex);
> >
> > release_firmware(fw); /* OK even if fw is NULL */
> > - guc_fw->guc_fw_fetch_status = GUC_FIRMWARE_FAIL;
> > + uc_fw->fetch_status = UC_FIRMWARE_FAIL;
> > }
> >
> > /**
> > @@ -721,7 +721,7 @@ static void guc_fw_fetch(struct drm_i915_private *dev_priv,
> > */
> > void intel_guc_init(struct drm_i915_private *dev_priv)
> > {
> > - struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw;
> > + struct intel_uc_fw *guc_fw = &dev_priv->guc.guc_fw;
> > const char *fw_path;
> >
> > if (!HAS_GUC(dev_priv)) {
> > @@ -739,23 +739,24 @@ void intel_guc_init(struct drm_i915_private *dev_priv)
> > fw_path = NULL;
> > } else if (IS_SKYLAKE(dev_priv)) {
> > fw_path = I915_SKL_GUC_UCODE;
> > - guc_fw->guc_fw_major_wanted = SKL_FW_MAJOR;
> > - guc_fw->guc_fw_minor_wanted = SKL_FW_MINOR;
> > + guc_fw->major_ver_wanted = SKL_FW_MAJOR;
> > + guc_fw->minor_ver_wanted = SKL_FW_MINOR;
> > } else if (IS_BROXTON(dev_priv)) {
> > fw_path = I915_BXT_GUC_UCODE;
> > - guc_fw->guc_fw_major_wanted = BXT_FW_MAJOR;
> > - guc_fw->guc_fw_minor_wanted = BXT_FW_MINOR;
> > + guc_fw->major_ver_wanted = BXT_FW_MAJOR;
> > + guc_fw->minor_ver_wanted = BXT_FW_MINOR;
> > } else if (IS_KABYLAKE(dev_priv)) {
> > fw_path = I915_KBL_GUC_UCODE;
> > - guc_fw->guc_fw_major_wanted = KBL_FW_MAJOR;
> > - guc_fw->guc_fw_minor_wanted = KBL_FW_MINOR;
> > + guc_fw->major_ver_wanted = KBL_FW_MAJOR;
> > + guc_fw->minor_ver_wanted = KBL_FW_MINOR;
> > } else {
> > fw_path = ""; /* unknown device */
> > }
> >
> > - guc_fw->guc_fw_path = fw_path;
> > - guc_fw->guc_fw_fetch_status = GUC_FIRMWARE_NONE;
> > - guc_fw->guc_fw_load_status = GUC_FIRMWARE_NONE;
> > + guc_fw->uc_dev = &dev_priv->drm;
> > + guc_fw->uc_fw_path = fw_path;
> > + guc_fw->fetch_status = UC_FIRMWARE_NONE;
> > + guc_fw->load_status = UC_FIRMWARE_NONE;
> >
> > /* Early (and silent) return if GuC loading is disabled */
> > if (!i915.enable_guc_loading)
> > @@ -765,9 +766,9 @@ void intel_guc_init(struct drm_i915_private *dev_priv)
> > if (*fw_path == '\0')
> > return;
> >
> > - guc_fw->guc_fw_fetch_status = GUC_FIRMWARE_PENDING;
> > + guc_fw->fetch_status = UC_FIRMWARE_PENDING;
> > DRM_DEBUG_DRIVER("GuC firmware pending, path %s\n", fw_path);
> > - guc_fw_fetch(dev_priv, guc_fw);
> > + intel_uc_fw_fetch(dev_priv, guc_fw);
> > /* status must now be FAIL or SUCCESS */
> > }
> >
> > @@ -777,17 +778,17 @@ void intel_guc_init(struct drm_i915_private *dev_priv)
> > */
> > void intel_guc_fini(struct drm_i915_private *dev_priv)
> > {
> > - struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw;
> > + struct intel_uc_fw *guc_fw = &dev_priv->guc.guc_fw;
> >
> > mutex_lock(&dev_priv->drm.struct_mutex);
> > guc_interrupts_release(dev_priv);
> > i915_guc_submission_disable(dev_priv);
> > i915_guc_submission_fini(dev_priv);
> >
> > - if (guc_fw->guc_fw_obj)
> > - i915_gem_object_put(guc_fw->guc_fw_obj);
> > - guc_fw->guc_fw_obj = NULL;
> > + if (guc_fw->uc_fw_obj)
> > + i915_gem_object_put(guc_fw->uc_fw_obj);
> > + guc_fw->uc_fw_obj = NULL;
> > mutex_unlock(&dev_priv->drm.struct_mutex);
> >
> > - guc_fw->guc_fw_fetch_status = GUC_FIRMWARE_NONE;
> > + guc_fw->fetch_status = UC_FIRMWARE_NONE;
> > }
> > diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
> > index 11f5608..f9f598d 100644
> > --- a/drivers/gpu/drm/i915/intel_uc.h
> > +++ b/drivers/gpu/drm/i915/intel_uc.h
> > @@ -91,29 +91,31 @@ struct i915_guc_client {
> > uint64_t submissions[I915_NUM_ENGINES];
> > };
> >
> > -enum intel_guc_fw_status {
> > - GUC_FIRMWARE_FAIL = -1,
> > - GUC_FIRMWARE_NONE = 0,
> > - GUC_FIRMWARE_PENDING,
> > - GUC_FIRMWARE_SUCCESS
> > +enum intel_uc_fw_status {
> > + UC_FIRMWARE_FAIL = -1,
> > + UC_FIRMWARE_NONE = 0,
> > + UC_FIRMWARE_PENDING,
> > + UC_FIRMWARE_SUCCESS
>
> Shouldn't we use INTEL_UC_ prefix here (in similar way as in GUC_ACTION)?
>
>
> > };
> >
> > /*
> > * This structure encapsulates all the data needed during the process
> > * of fetching, caching, and loading the firmware image into the GuC.
> > */
> > -struct intel_guc_fw {
> > - const char * guc_fw_path;
> > - size_t guc_fw_size;
> > - struct drm_i915_gem_object * guc_fw_obj;
> > - enum intel_guc_fw_status guc_fw_fetch_status;
> > - enum intel_guc_fw_status guc_fw_load_status;
> > -
> > - uint16_t guc_fw_major_wanted;
> > - uint16_t guc_fw_minor_wanted;
> > - uint16_t guc_fw_major_found;
> > - uint16_t guc_fw_minor_found;
> > -
> > +struct intel_uc_fw {
> > + struct drm_device *uc_dev;
>
> Why do you need this back pointer to drm_dev ?
> More likely all you need is pointer to dev_priv.
> But for dev_priv you can always use guc_to_i915().
It seems like a left over from the old days. I have a series that
refactors all things GuC that I'll push here soon, including the removal
of this field.
Since this series advertises as a simple renaming I didn't force it
here.
> > + const char *uc_fw_path;
> > + size_t uc_fw_size;
> > + struct drm_i915_gem_object *uc_fw_obj;
>
> All these uc_fw_ prefixes seem to be redundant as this struct is just about uc_fw.
>
>
> > + enum intel_uc_fw_status fetch_status;
> > + enum intel_uc_fw_status load_status;
> > +
> > + uint16_t major_ver_wanted;
> > + uint16_t minor_ver_wanted;
> > + uint16_t major_ver_found;
> > + uint16_t minor_ver_found;
> > +
> > + uint32_t fw_type;
>
> What is the purpose of this field ?
It is used in the patch 2, to set {major,minor}_ver_found depending on
which firmware we pass to intel_uc_fw_fetch()
>
> > uint32_t header_size;
> > uint32_t header_offset;
> > uint32_t rsa_size;
> > @@ -139,7 +141,7 @@ struct intel_guc_log {
> > };
> >
> > struct intel_guc {
> > - struct intel_guc_fw guc_fw;
> > + struct intel_uc_fw guc_fw;
>
> Btw, guc_ prefix in field name here is also redundant.
>
>
> > struct intel_guc_log log;
> >
> > /* intel_guc_recv interrupt related state */
> > @@ -181,7 +183,7 @@ int intel_guc_log_control(struct intel_guc *guc, u32 control_val);
> > extern void intel_guc_init(struct drm_i915_private *dev_priv);
> > extern int intel_guc_setup(struct drm_i915_private *dev_priv);
> > extern void intel_guc_fini(struct drm_i915_private *dev_priv);
> > -extern const char *intel_guc_fw_status_repr(enum intel_guc_fw_status status);
> > +extern const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status);
> > extern int intel_guc_suspend(struct drm_i915_private *dev_priv);
> > extern int intel_guc_resume(struct drm_i915_private *dev_priv);
> >
> > --
> > 2.7.4
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Cheers,
Arek
More information about the Intel-gfx
mailing list