[Intel-gfx] [PATCH 5/6] drm/i915: dynamically allocate forcewake domains

[Chris Wilson]

Quoting Daniele Ceraolo Spurio (2019-06-19 00:06:39)
> 
> 
> On 6/18/19 2:23 AM, Tvrtko Ursulin wrote:
> > 
> > On 17/06/2019 19:09, Daniele Ceraolo Spurio wrote:
> >> -static void intel_uncore_fw_domains_init(struct intel_uncore *uncore)
> >> +static int intel_uncore_fw_domains_init(struct intel_uncore *uncore)
> >>   {
> >>       struct drm_i915_private *i915 = uncore->i915;
> >> +    int ret;
> >>       GEM_BUG_ON(!intel_uncore_has_forcewake(uncore));
> >> +#define __fw_domain_init(id__, set__, ack__) \
> >> +    ret = fw_domain_init(uncore, (id__), (set__), (ack__)); \
> >> +    if (ret) \
> >> +        goto out_clean;
> > 
> > Hidden control flow is slightly objectionable but I don't offer any nice 
> > alternatives so I guess will have to pass. Or maybe accumulate the error 
> > (err |= fw_domain_init(..)) as you go and then cleanup at the end if any 
> > failed?
> 
> I'd prefer to avoid accumulating the error since it'd just cause us to 
> having to unroll more domains when we could've stopped early.
> 
> > 
> > On the other hand the idea slightly conflicts with my other suggestion 
> > to rename existing fw_domain_init to __fw_domain_init and call the macro 
> > fw_domain_init and avoid all the churn below.
> > 
> 
> I'll pick this suggestion among the 2, unless there is another 
> suggestion on how to avoid the hidden goto.

Be careful, or you'll give Tvrtko the chance to suggest table driven
setup. Maybe?
-Chris