[Intel-gfx] [PATCH 3/4] drm/i915/tgl: Gen12 csb support

Daniele Ceraolo Spurio daniele.ceraolospurio at intel.com
Wed Jul 31 20:18:36 UTC 2019



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


More information about the Intel-gfx mailing list