Re: [PATCH 2/2] drm: etnaviv: add further minor features and varyings count

Hi Russell

2016-01-19 11:21 GMT+01:00 Russell King - ARM Linux [off-list ref]:

On Tue, Jan 19, 2016 at 11:09:57AM +0100, Christian Gmeiner wrote:

quoted

Hi Russell,

2016-01-19 10:18 GMT+01:00 Russell King [off-list ref]:

quoted

+       /*
+        * For some cores, two varyings are consumed for position, so the
+        * maximum varying count needs to be reduced by one.
+        */
+       if ((gpu->identity.model == chipModel_GC2000 &&
+            gpu->identity.revision == 0x5108) ||
+           (gpu->identity.model == chipModel_GC880 &&
+            gpu->identity.revision == 0x5106))
+               gpu->identity.varyings_count -= 1;

Should we not include the whole list of GPU cores with that special handling?
See: https://github.com/Freescale/kernel-module-imx-gpu-viv/blob/master/kernel-module-imx-gpu-viv-src/hal/kernel/arch/gc_hal_kernel_hardware.c#L592

I was debating about that - but I think we need to come up with a better
way to do this sort of thing.  At the very least, I've been wondering
whether a macro such as:

#define etnaviv_model_rev(gpu, mod, rev) \
        ((gpu)->identity.model == chipModel_##mod && \
         (gpu)->identity.revision == rev))

would help make some of this code more readable.

Yep that makes the code more readable.

The other thing I've been wondering is whether a table looked up by GPU
model ID and/or revision ID quirks would simplify this.  However, the
downside with the tabular approach is that it becomes harder to compare
what we have against the galcore sources.

The table driven approach could be a little heavy for the number of
quirks we have
currently. Maybe Lucas has an opinion about that?

greets
--
Christian Gmeiner, MSc

https://soundcloud.com/christian-gmeiner