[Intel-gfx] [PATCH 09/53] drm/i915/bdw: Initialization for Logical Ring Contexts

[Daniel Vetter]

On Thu, Jun 19, 2014 at 11:23 AM, Mateo Lozano, Oscar
<oscar.mateo at intel.com> wrote:
>> > -static bool hw_context_enabled(struct drm_device *dev)
>> > +static bool contexts_enabled(struct drm_device *dev)
>> >  {
>> > -   return to_i915(dev)->hw_context_size;
>> > +   struct drm_i915_private *dev_priv = to_i915(dev);
>> > +
>> > +   /* FIXME: this would be cleaner with a "context type" enum */
>> > +   return dev_priv->lrc_enabled || dev_priv->hw_context_size;
>>
>> Since you have a bunch of if ladders the usual approach isn't an enum but a
>> vfunc table to abstract behaviour. Think object types instead of switch
>> statements. Style bikeshed though (presume code later on doesn't have
>> excesses here).
>> -Daniel
>
> Hmmmm... I offered to do this with vfuncs early on, but you mentioned special-casing should be enough. And I agreed: at the end, the LR contexts are not that different from traditional HW contexts. This is what I had in mind:
>
> CTX_TYPE_FAKE: no backing objects.
> CTX_TYPE_HW: one render backing object at creation time.
> CTX_TYPE_LR: n backing objects, with deferred creation. A few functions are moot (e.g. switch, reset).
>
> The current system (looking at dev_priv->hw_context_size to distinguish fake from hw contexts) is imo a bit obfuscated.
> And we can always abstract this away with vfuncs if it becomes too complex in the future...
>
> What do you think? can special casing made do for the time being?

Yeah I generally promote the rule-of-thumb to only do vfuncs once we
have the 3rd case (and I don't think we should count fake contexts
really). Until then if-ladders and hacks are good enough. Actually
better since usually you need a few completely different platforms to
know what's required of your vfunc intefaces to cover it all.

I really only latched onto this because of your FIXME comment, no
other reason at all. So if we decide that some reorg helps the code a
lot we can do that as a follow-up, but really not required upfront
imo.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch