On ma, 2016-03-07 at 12:05 +0000, Chris Wilson wrote:
> If the firmware is generic and has a run-anywhere mode, enable it rather
> than completely failing on unknown HW revisions.
> Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
> Cc: Damien Lespiau <damien.lesp...@intel.com>
> Cc: Imre Deak <imre.d...@intel.com>
> Cc: Sunil Kamath <sunil.kam...@intel.com>
> Cc: Daniel Vetter <daniel.vet...@intel.com>
> Cc: Animesh Manna <animesh.ma...@intel.com>
> Cc: Jani Nikula <jani.nik...@intel.com>

Looks ok:
Reviewed-by: Imre Deak <imre.d...@intel.com>

> ---
> This is probably stable@ material since it could allow future hw to
> just work.

Existing platforms will select the generic firmware version already if
one is provided in the image, this change will just allow us not to
provide a stepping info table for new platforms. So not sure why it's
@stable material.

--Imre

> -Chris
> ---
>  drivers/gpu/drm/i915/intel_csr.c | 46 
> +++++++++++++++++-----------------------
>  1 file changed, 19 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_csr.c 
> b/drivers/gpu/drm/i915/intel_csr.c
> index 902054efb902..575a5821d2a1 100644
> --- a/drivers/gpu/drm/i915/intel_csr.c
> +++ b/drivers/gpu/drm/i915/intel_csr.c
> @@ -188,28 +188,31 @@ static const struct stepping_info bxt_stepping_info[] = 
> {
>       {'B', '0'}, {'B', '1'}, {'B', '2'}
>  };
>  
> -static const struct stepping_info *intel_get_stepping_info(struct drm_device 
> *dev)
> +static const struct stepping_info no_stepping_info = { '*', '*' };
> +
> +static const struct stepping_info *
> +intel_get_stepping_info(struct drm_i915_private *dev_priv)
>  {
>       const struct stepping_info *si;
>       unsigned int size;
>  
> -     if (IS_KABYLAKE(dev)) {
> +     if (IS_KABYLAKE(dev_priv)) {
>               size = ARRAY_SIZE(kbl_stepping_info);
>               si = kbl_stepping_info;
> -     } else if (IS_SKYLAKE(dev)) {
> +     } else if (IS_SKYLAKE(dev_priv)) {
>               size = ARRAY_SIZE(skl_stepping_info);
>               si = skl_stepping_info;
> -     } else if (IS_BROXTON(dev)) {
> +     } else if (IS_BROXTON(dev_priv)) {
>               size = ARRAY_SIZE(bxt_stepping_info);
>               si = bxt_stepping_info;
>       } else {
> -             return NULL;
> +             size = 0;
>       }
>  
> -     if (INTEL_REVID(dev) < size)
> -             return si + INTEL_REVID(dev);
> +     if (INTEL_REVID(dev_priv) < size)
> +             return si + INTEL_REVID(dev_priv);
>  
> -     return NULL;
> +     return &no_stepping_info;
>  }
>  
>  /**
> @@ -252,13 +255,11 @@ bool intel_csr_load_program(struct drm_i915_private 
> *dev_priv)
>  static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv,
>                             const struct firmware *fw)
>  {
> -     struct drm_device *dev = dev_priv->dev;
>       struct intel_css_header *css_header;
>       struct intel_package_header *package_header;
>       struct intel_dmc_header *dmc_header;
>       struct intel_csr *csr = &dev_priv->csr;
> -     const struct stepping_info *stepping_info = 
> intel_get_stepping_info(dev);
> -     char stepping, substepping;
> +     const struct stepping_info *si = intel_get_stepping_info(dev_priv);
>       uint32_t dmc_offset = CSR_DEFAULT_FW_OFFSET, readcount = 0, nbytes;
>       uint32_t i;
>       uint32_t *dmc_payload;
> @@ -266,14 +267,6 @@ static uint32_t *parse_csr_fw(struct drm_i915_private 
> *dev_priv,
>       if (!fw)
>               return NULL;
>  
> -     if (!stepping_info) {
> -             DRM_ERROR("Unknown stepping info, firmware loading failed\n");
> -             return NULL;
> -     }
> -
> -     stepping = stepping_info->stepping;
> -     substepping = stepping_info->substepping;
> -
>       /* Extract CSS Header information*/
>       css_header = (struct intel_css_header *)fw->data;
>       if (sizeof(struct intel_css_header) !=
> @@ -285,7 +278,7 @@ static uint32_t *parse_csr_fw(struct drm_i915_private 
> *dev_priv,
>  
>       csr->version = css_header->version;
>  
> -     if ((IS_SKYLAKE(dev) || IS_KABYLAKE(dev)) &&
> +     if ((IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) &&
>           csr->version < SKL_CSR_VERSION_REQUIRED) {
>               DRM_INFO("Refusing to load old Skylake DMC firmware v%u.%u,"
>                        " please upgrade to v%u.%u or later"
> @@ -313,11 +306,11 @@ static uint32_t *parse_csr_fw(struct drm_i915_private 
> *dev_priv,
>       /* Search for dmc_offset to find firware binary. */
>       for (i = 0; i < package_header->num_entries; i++) {
>               if (package_header->fw_info[i].substepping == '*' &&
> -                 stepping == package_header->fw_info[i].stepping) {
> +                 si->stepping == package_header->fw_info[i].stepping) {
>                       dmc_offset = package_header->fw_info[i].offset;
>                       break;
> -             } else if (stepping == package_header->fw_info[i].stepping &&
> -                     substepping == package_header->fw_info[i].substepping) {
> +             } else if (si->stepping == package_header->fw_info[i].stepping 
> &&
> +                        si->substepping == 
> package_header->fw_info[i].substepping) {
>                       dmc_offset = package_header->fw_info[i].offset;
>                       break;
>               } else if (package_header->fw_info[i].stepping == '*' &&
> @@ -325,7 +318,8 @@ static uint32_t *parse_csr_fw(struct drm_i915_private 
> *dev_priv,
>                       dmc_offset = package_header->fw_info[i].offset;
>       }
>       if (dmc_offset == CSR_DEFAULT_FW_OFFSET) {
> -             DRM_ERROR("Firmware not supported for %c stepping\n", stepping);
> +             DRM_ERROR("Firmware not supported for %c stepping\n",
> +                       si->stepping);
>               return NULL;
>       }
>       readcount += dmc_offset;
> @@ -371,9 +365,7 @@ static uint32_t *parse_csr_fw(struct drm_i915_private 
> *dev_priv,
>               return NULL;
>       }
>  
> -     memcpy(dmc_payload, &fw->data[readcount], nbytes);
> -
> -     return dmc_payload;
> +     return memcpy(dmc_payload, &fw->data[readcount], nbytes);
>  }
>  
>  static void csr_load_work_fn(struct work_struct *work)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to