Re: [PATCH 5/6] drm/i915/ringbuffer: Move irq seqno barrier to the GPU for gen5

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 31/12/2018 11:07, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2018-12-31 10:49:37)
>>
>> On 28/12/2018 17:16, Chris Wilson wrote:
>>> @@ -2191,6 +2193,10 @@ static void intel_ring_default_vfuncs(struct drm_i915_private *dev_priv,
>>>    
>>>        engine->emit_breadcrumb = i9xx_emit_breadcrumb;
>>>        engine->emit_breadcrumb_sz = i9xx_emit_breadcrumb_sz;
>>> +     if (IS_GEN(dev_priv, 5)) {
>>> +             engine->emit_breadcrumb = gen5_emit_breadcrumb;
>>> +             engine->emit_breadcrumb_sz = gen5_emit_breadcrumb_sz;
>>
>> I'll only observe in passing that we lost some consistency in this
>> approximate area of the code regarding when we are happy to overwrite
>> the pointers, versus when we do if-ladders-or-so to avoid that.
> 
> I hear you, but my thinking was that this was the default for gen5 where
> we use the same function for both classes of engine. And emit_bb_start
> already looked odd there :) You do remember inconsistency is both my
> middle names?
> 
> Back to tables? With semaphores gone, we've removed the runtime
> conditionals, so we just have a bunch of common functions (for reset
> handling). The counter point is that some subarches (gen2) only vary in
> a single function pointer, so we end up with a lot of duplicate, but
> const, tables.
> 
> I definitely prefer the repetition in tables when it comes to figuring
> out exactly what set of functions are used together for any particular
> setup.

I was also looking at intel_init_bsd_ring_buffer before in the series - 
it ends up if gen >= 6, if !gen6, if gen6, if gen5, else, something like 
that..

I think for now we apply -EDONTCARE here and someone comes back later if 
it bugs them enough.

Regards,

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