Re: [PATCH 3/9] drm/i915/uc: Unify uC FW selection

[Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>]

On 7/23/2019 6:22 AM, Michal Wajdeczko wrote:
> On Tue, 23 Jul 2019 01:20:42 +0200, Daniele Ceraolo Spurio 
> <daniele.ceraolospurio@xxxxxxxxx> wrote:
>
> > +
> > +#define GUC_FW_BLOB(prefix_, major_, minor_, patch_) \
> > +UC_FW_BLOB(prefix_##_guc, major_, minor_, \
> > +       __MAKE_GUC_FW_PATH(prefix_, major_, minor_, patch_))
> > +
> > +GUC_FW_BLOB(skl, 33, 0, 0);
> > +GUC_FW_BLOB(bxt, 33, 0, 0);
> > +GUC_FW_BLOB(kbl, 33, 0, 0);
> > +GUC_FW_BLOB(glk, 33, 0, 0);
> > +GUC_FW_BLOB(icl, 33, 0, 0);
> > +
> > +/* must be ordered base on platform + revid, from newer to older */
> > +static const struct intel_uc_platform_requirement guc_fw_blobs[] = {
> > +    { INTEL_ICELAKE,    0,    &icl_guc_fw_blob },
> > +    { INTEL_COFFEELAKE,    0,    &kbl_guc_fw_blob },
> > +    { INTEL_GEMINILAKE,    0,    &glk_guc_fw_blob },
> > +    { INTEL_KABYLAKE,    0,    &kbl_guc_fw_blob },
> > +    { INTEL_BROXTON,    0,    &bxt_guc_fw_blob },
> > +    { INTEL_SKYLAKE,    0,    &skl_guc_fw_blob },
> > +};
>
> Can we avoid pointers to separate blob definitions ?
> What about defining each fw in single line like below
>
> #define INTEL_GUC_FIRMWARE_DEFS(fw_def) \
>     fw_def(ICELAKE, 0, 33, 0, 0, icl, GUC) \
>     fw_def(COFFEELAKE, 0, 33, 0, 0, kbl, GUC) \
>     fw_def(GEMINILAKE, 0, 33, 0, 0, glk, GUC) \
>     fw_def(KABYLAKE, 0, 33, 0, 0, kbl, GUC) \
>     fw_def(BROXTON, 0, 33, 0, 0, bxt, GUC) \
>     fw_def(SKYLAKE, 0, 33, 0, 0, skl, GUC) \
>     /* end */
>
> with some extra common helpers
>
> #define TO_MODULE_FIRMWARE(_platform, _rev, _major, _minor, _patch, 
> _prefix, _builder) \
>     MODULE_FIRMWARE(_builder##_FW_PATH_BUILDER(_prefix, _major, 
> _minor, _patch));
>
> #define TO_BLOB_ENTRY(_platform, _rev, _major, _minor, _patch, 
> _prefix, _builder) \
> { \
>     .platform = INTEL_##_platform, \
>     .rev = (_rev), \
>     .major = (_major), \
>     .minor = (_minor), \
>     .patch = (_patch), \
>     .blob = _builder##_FW_PATH_BUILDER(_prefix, _major, _minor, 
> _patch), \
> },
>
> then we can have immutable
>
> static const struct intel_uc_blob guc_fw_blobs[] = {
> INTEL_GUC_FIRMWARE_DEFS(TO_BLOB_ENTRY)
> };
> INTEL_GUC_FIRMWARE_DEFS(TO_MODULE_FIRMWARE)
>
> (tested locally for feasibility)
>
> Michal

LGTM, will update. I went with the list because I wanted to ultimately 
unify the GuC and HuC lists into one, but I should be able to set the 
macros appropriately to allow that as the next step.

Daniele

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