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

2 messages, 2 authors, 2016-01-21 · open the first message on its own page

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

From: Christian Gmeiner <hidden>
Date: 2016-01-20 18:01:36

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

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

From: Lucas Stach <l.stach@pengutronix.de>
Date: 2016-01-21 09:13:12

Am Mittwoch, den 20.01.2016, 19:01 +0100 schrieb Christian Gmeiner:
Hi Russell

2016-01-19 11:21 GMT+01:00 Russell King - ARM Linux [off-list ref]:
quoted
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.
This macro seems like a very reasonable simplification.
quoted
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?
I don't think that having a quirk table is useful for the current number
of quirks and from what I have seen from the Vivante kernel sources.

Regards,
Lucas 

-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help