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

[Russell King - ARM Linux <linux@arm.linux.org.uk>]

On Tue, Jan 19, 2016 at 11:09:57AM +0100, Christian Gmeiner wrote:
> Hi Russell,
> 
> 2016-01-19 10:18 GMT+01:00 Russell King <rmk+kernel@arm.linux.org.uk>:
> > +       /*
> > +        * 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.

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.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel