Re: [PATCH 3/4] drm/i915/tgl: Gen12 csb support

[Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>]

On 7/31/19 12:33 AM, Chris Wilson wrote:
> Quoting Daniele Ceraolo Spurio (2019-07-31 01:49:01)
>> @@ -1401,7 +1453,7 @@ static void process_csb(struct intel_engine_cs *engine)
>>                            engine->name, head,
>>                            buf[2 * head + 0], buf[2 * head + 1]);
>>   
>> -               switch (csb_parse(execlists, buf + 2 * head)) {
>> +               switch (execlists->csb_parse(execlists, buf + 2 * head)) {
> 
> So I worry about the cost of a retpoline here (tucked away inside an
> irqs-off loop), and whether a local func avoids the retpoline or if we
> just have to use an if-ladder.
> -Chris
> 

I've tried with:

static enum intel_csb_step
(*csb_parse[])(const struct intel_engine_execlists *, const u32 *) = {
	[CSB_GEN8] = gen8_csb_parse,
	[CSB_GEN12] = gen12_csb_parse,
};

switch (csb_parse[execlists->csb_format](..))

But AFAICS from the objdump the assembly code generated with GCC 8.3 and 
CONFIG_RETPOLINE=y is more or less the same, there is just 2 extra mov 
instructions when using the array of functions. I can't spot the 
retpoline in neither case thought, so not sure if I'm missing something. 
Should I just go with an if-else?

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