[Intel-gfx] [PATCH 1/3] drm/i915/csr: keep firmware name and required version together

[Chris Wilson]

Quoting Jani Nikula (2018-09-06 09:21:24)
> Having two separate if ladders gets increasingly hard to maintain. Put
> them together.
> 
> Signed-off-by: Jani Nikula <jani.nikula at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h  |  1 +
>  drivers/gpu/drm/i915/intel_csr.c | 54 ++++++++++++++++------------------------
>  2 files changed, 23 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 767615ecdea5..368066010f94 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -464,6 +464,7 @@ struct drm_i915_display_funcs {
>  struct intel_csr {
>         struct work_struct work;
>         const char *fw_path;
> +       uint32_t required_version;
>         uint32_t *dmc_payload;
>         uint32_t dmc_fw_size;
>         uint32_t version;
> diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
> index 14cf4c367e36..9a60bb9cc443 100644
> --- a/drivers/gpu/drm/i915/intel_csr.c
> +++ b/drivers/gpu/drm/i915/intel_csr.c
> @@ -284,7 +284,6 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv,
>         uint32_t max_fw_size = 0;
>         uint32_t i;
>         uint32_t *dmc_payload;
> -       uint32_t required_version;
>  
>         if (!fw)
>                 return NULL;
> @@ -299,36 +298,19 @@ static uint32_t *parse_csr_fw(struct drm_i915_private *dev_priv,
>                 return NULL;
>         }
>  
> -       csr->version = css_header->version;
> -
> -       if (csr->fw_path == i915_modparams.dmc_firmware_path) {
> -               /* Bypass version check for firmware override. */
> -               required_version = csr->version;
> -       } else if (IS_CANNONLAKE(dev_priv)) {
> -               required_version = CNL_CSR_VERSION_REQUIRED;
> -       } else if (IS_GEMINILAKE(dev_priv)) {
> -               required_version = GLK_CSR_VERSION_REQUIRED;
> -       } else if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) {
> -               required_version = KBL_CSR_VERSION_REQUIRED;
> -       } else if (IS_SKYLAKE(dev_priv)) {
> -               required_version = SKL_CSR_VERSION_REQUIRED;
> -       } else if (IS_BROXTON(dev_priv)) {
> -               required_version = BXT_CSR_VERSION_REQUIRED;
> -       } else {
> -               MISSING_CASE(INTEL_REVID(dev_priv));
> -               required_version = 0;
> -       }
> -
> -       if (csr->version != required_version) {
> +       if (csr->required_version &&
> +           css_header->version != csr->required_version) {
>                 DRM_INFO("Refusing to load DMC firmware v%u.%u,"
>                          " please use v%u.%u\n",
> -                        CSR_VERSION_MAJOR(csr->version),
> -                        CSR_VERSION_MINOR(csr->version),
> -                        CSR_VERSION_MAJOR(required_version),
> -                        CSR_VERSION_MINOR(required_version));
> +                        CSR_VERSION_MAJOR(css_header->version),
> +                        CSR_VERSION_MINOR(css_header->version),
> +                        CSR_VERSION_MAJOR(csr->required_version),
> +                        CSR_VERSION_MINOR(csr->required_version));
>                 return NULL;
>         }
>  
> +       csr->version = css_header->version;

Ok, so far.

> +
>         readcount += sizeof(struct intel_css_header);
>  
>         /* Extract Package Header information*/
> @@ -469,18 +451,26 @@ void intel_csr_ucode_init(struct drm_i915_private *dev_priv)
>         if (!HAS_CSR(dev_priv))
>                 return;
>  
> -       if (i915_modparams.dmc_firmware_path)
> +       if (i915_modparams.dmc_firmware_path) {
>                 csr->fw_path = i915_modparams.dmc_firmware_path;
> -       else if (IS_CANNONLAKE(dev_priv))
> +               /* Bypass version check for firmware override. */
> +               csr->required_version = 0;
Ok.

> +       } else if (IS_CANNONLAKE(dev_priv)) {
>                 csr->fw_path = I915_CSR_CNL;
> -       else if (IS_GEMINILAKE(dev_priv))
> +               csr->required_version = CNL_CSR_VERSION_REQUIRED;
Ok.

> +       } else if (IS_GEMINILAKE(dev_priv)) {
>                 csr->fw_path = I915_CSR_GLK;
> -       else if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv))
> +               csr->required_version = GLK_CSR_VERSION_REQUIRED;
Ok.

> +       } else if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) {
>                 csr->fw_path = I915_CSR_KBL;
> -       else if (IS_SKYLAKE(dev_priv))
> +               csr->required_version = KBL_CSR_VERSION_REQUIRED;
Ok.

> +       } else if (IS_SKYLAKE(dev_priv)) {
>                 csr->fw_path = I915_CSR_SKL;
> -       else if (IS_BROXTON(dev_priv))
> +               csr->required_version = SKL_CSR_VERSION_REQUIRED;
Ok.

> +       } else if (IS_BROXTON(dev_priv)) {
>                 csr->fw_path = I915_CSR_BXT;
> +               csr->required_version = BXT_CSR_VERSION_REQUIRED;
Ok.

Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
-Chris