On 6/18/19 4:23 PM, Chris Wilson wrote:
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
I did consider using a table myself, but the differences between the
domains are not easy to put in a table since some of them are per-gen
and other per-platform. We could combine a table with information in the
device_info struct like we do for the engines, but that felt a bit like
overkill to me.
Daniele
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
