This patch series extends the hwdb for an entry for the gc400 found
in the ST STM32 SoC. With this patches we report the same limits and
features for this GPU as the galcore kernel driver does.
Christian Gmeiner (6):
drm/etnaviv: update hardware headers from rnndb
drm/etnaviv: determine product, customer and eco id
drm/etnaviv: show identity information in debugfs
drm/etnaviv: update gc7000 chip identity entry
drm/etnaviv: update hwdb selection logic
drm/etnaviv: add hwdb entry for gc400 found in STM32
drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 29 ++++++++++++++
drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 6 +--
drivers/gpu/drm/etnaviv/etnaviv_hwdb.c | 53 +++++++++++++++++++++++++-
drivers/gpu/drm/etnaviv/state_hi.xml.h | 29 ++++++++------
4 files changed, 102 insertions(+), 15 deletions(-)
--
2.24.1
Signed-off-by: Christian Gmeiner <[email protected]>
---
drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index 253301be9e95..cecef5034db1 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -868,6 +868,18 @@ int etnaviv_gpu_debugfs(struct etnaviv_gpu *gpu, struct seq_file *m)
verify_dma(gpu, &debug);
+ seq_puts(m, "\tidentity\n");
+ seq_printf(m, "\t model: 0x%x\n",
+ gpu->identity.model);
+ seq_printf(m, "\t revision: 0x%x\n",
+ gpu->identity.revision);
+ seq_printf(m, "\t product_id: 0x%x\n",
+ gpu->identity.product_id);
+ seq_printf(m, "\t customer_id: 0x%x\n",
+ gpu->identity.customer_id);
+ seq_printf(m, "\t eco_id: 0x%x\n",
+ gpu->identity.eco_id);
+
seq_puts(m, "\tfeatures\n");
seq_printf(m, "\t major_features: 0x%08x\n",
gpu->identity.features);
--
2.24.1
The information was taken from STM32 glacore driver hw database.
The entry is named as gc7000nano_0x4652.
Signed-off-by: Christian Gmeiner <[email protected]>
---
drivers/gpu/drm/etnaviv/etnaviv_hwdb.c | 31 ++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
index d1744f1b44b1..8495b041a3b7 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
@@ -6,6 +6,37 @@
#include "etnaviv_gpu.h"
static const struct etnaviv_chip_identity etnaviv_chip_identities[] = {
+ {
+ .model = 0x400,
+ .revision = 0x4652,
+ .product_id = 0x70001,
+ .customer_id = 0x100,
+ .eco_id = 0,
+ .stream_count = 4,
+ .register_max = 64,
+ .thread_count = 128,
+ .shader_core_count = 1,
+ .vertex_cache_size = 8,
+ .vertex_output_buffer_size = 1024,
+ .pixel_pipes = 1,
+ .instruction_count = 256,
+ .num_constants = 320,
+ .buffer_size = 0,
+ .varyings_count = 8,
+ .features = 0xa0e9e004,
+ .minor_features0 = 0xe1299fff,
+ .minor_features1 = 0xbe13b219,
+ .minor_features2 = 0xce110010,
+ .minor_features3 = 0x8000001,
+ .minor_features4 = 0x20102,
+ .minor_features5 = 0x120000,
+ .minor_features6 = 0x0,
+ .minor_features7 = 0x0,
+ .minor_features8 = 0x0,
+ .minor_features9 = 0x0,
+ .minor_features10 = 0x0,
+ .minor_features11 = 0x0,
+ },
{
.model = 0x7000,
.revision = 0x6214,
--
2.24.1
Use ~0U as marker for 'I do not care'. I am not sure what
GC7000 based devices are in the wild and I do not want to
break them. In the near future we should extend the hwdb.
Signed-off-by: Christian Gmeiner <[email protected]>
---
drivers/gpu/drm/etnaviv/etnaviv_hwdb.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
index 39b463db76c9..eb0f3eb87ced 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
@@ -9,6 +9,9 @@ static const struct etnaviv_chip_identity etnaviv_chip_identities[] = {
{
.model = 0x7000,
.revision = 0x6214,
+ .product_id = ~0U,
+ .customer_id = ~0U,
+ .eco_id = ~0U,
.stream_count = 16,
.register_max = 64,
.thread_count = 1024,
--
2.24.1
They will be used for extended HWDB support. The eco id logic was taken
from galcore kernel driver sources.
Signed-off-by: Christian Gmeiner <[email protected]>
---
drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 17 +++++++++++++++++
drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 6 +++---
2 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index d47d1a8e0219..253301be9e95 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -321,6 +321,18 @@ static void etnaviv_hw_specs(struct etnaviv_gpu *gpu)
gpu->identity.varyings_count -= 1;
}
+static void etnaviv_hw_eco_id(struct etnaviv_gpu *gpu)
+{
+ const u32 chipDate = gpu_read(gpu, VIVS_HI_CHIP_DATE);
+ gpu->identity.eco_id = gpu_read(gpu, VIVS_HI_CHIP_ECO_ID);
+
+ if (etnaviv_is_model_rev(gpu, GC1000, 0x5037) && (chipDate == 0x20120617))
+ gpu->identity.eco_id = 1;
+
+ if (etnaviv_is_model_rev(gpu, GC320, 0x5303) && (chipDate == 0x20140511))
+ gpu->identity.eco_id = 1;
+}
+
static void etnaviv_hw_identify(struct etnaviv_gpu *gpu)
{
u32 chipIdentity;
@@ -362,6 +374,8 @@ static void etnaviv_hw_identify(struct etnaviv_gpu *gpu)
}
}
+ gpu->identity.product_id = gpu_read(gpu, VIVS_HI_CHIP_PRODUCT_ID);
+
/*
* NXP likes to call the GPU on the i.MX6QP GC2000+, but in
* reality it's just a re-branded GC3000. We can identify this
@@ -375,6 +389,9 @@ static void etnaviv_hw_identify(struct etnaviv_gpu *gpu)
}
}
+ etnaviv_hw_eco_id(gpu);
+ gpu->identity.customer_id = gpu_read(gpu, VIVS_HI_CHIP_CUSTOMER_ID);
+
dev_info(gpu->dev, "model: GC%x, revision: %x\n",
gpu->identity.model, gpu->identity.revision);
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
index 8f9bd4edc96a..68bd966e3916 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
@@ -15,11 +15,11 @@ struct etnaviv_gem_submit;
struct etnaviv_vram_mapping;
struct etnaviv_chip_identity {
- /* Chip model. */
u32 model;
-
- /* Revision value.*/
u32 revision;
+ u32 product_id;
+ u32 customer_id;
+ u32 eco_id;
/* Supported feature fields. */
u32 features;
--
2.24.1
Take product id, customer id and eco id into account. If that
delivers no match try a search for model and revision.
Signed-off-by: Christian Gmeiner <[email protected]>
---
drivers/gpu/drm/etnaviv/etnaviv_hwdb.c | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
index eb0f3eb87ced..d1744f1b44b1 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
@@ -44,9 +44,26 @@ bool etnaviv_fill_identity_from_hwdb(struct etnaviv_gpu *gpu)
struct etnaviv_chip_identity *ident = &gpu->identity;
int i;
+ /* accurate match */
for (i = 0; i < ARRAY_SIZE(etnaviv_chip_identities); i++) {
if (etnaviv_chip_identities[i].model == ident->model &&
- etnaviv_chip_identities[i].revision == ident->revision) {
+ etnaviv_chip_identities[i].revision == ident->revision &&
+ etnaviv_chip_identities[i].product_id == ident->product_id &&
+ etnaviv_chip_identities[i].customer_id == ident->customer_id &&
+ etnaviv_chip_identities[i].eco_id == ident->eco_id) {
+ memcpy(ident, &etnaviv_chip_identities[i],
+ sizeof(*ident));
+ return true;
+ }
+ }
+
+ /* match based only on model and revision */
+ for (i = 0; i < ARRAY_SIZE(etnaviv_chip_identities); i++) {
+ if (etnaviv_chip_identities[i].model == ident->model &&
+ etnaviv_chip_identities[i].revision == ident->revision &&
+ etnaviv_chip_identities[i].product_id == ~0U &&
+ etnaviv_chip_identities[i].customer_id == ~0U &&
+ etnaviv_chip_identities[i].eco_id == ~0U) {
memcpy(ident, &etnaviv_chip_identities[i],
sizeof(*ident));
return true;
--
2.24.1
Update the state HI header from rnndb commit
7f1ce75 ("rnndb: document some GPU identity register")
Signed-off-by: Christian Gmeiner <[email protected]>
---
drivers/gpu/drm/etnaviv/state_hi.xml.h | 29 ++++++++++++++++----------
1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/state_hi.xml.h b/drivers/gpu/drm/etnaviv/state_hi.xml.h
index 41d8da2b6f4f..004d8ddacf6a 100644
--- a/drivers/gpu/drm/etnaviv/state_hi.xml.h
+++ b/drivers/gpu/drm/etnaviv/state_hi.xml.h
@@ -8,17 +8,17 @@ This file was generated by the rules-ng-ng headergen tool in this git repository
git clone git://0x04.net/rules-ng-ng
The rules-ng-ng source files this header was generated from are:
-- state.xml ( 26087 bytes, from 2017-12-18 16:51:59)
-- common.xml ( 35468 bytes, from 2018-01-22 13:48:54)
-- common_3d.xml ( 14615 bytes, from 2017-12-18 16:51:59)
-- state_hi.xml ( 30232 bytes, from 2018-02-15 15:48:01)
-- copyright.xml ( 1597 bytes, from 2016-12-08 16:37:56)
-- state_2d.xml ( 51552 bytes, from 2016-12-08 16:37:56)
-- state_3d.xml ( 79992 bytes, from 2017-12-18 16:51:59)
-- state_blt.xml ( 13405 bytes, from 2017-12-18 16:51:59)
-- state_vg.xml ( 5975 bytes, from 2016-12-08 16:37:56)
-
-Copyright (C) 2012-2018 by the following authors:
+- state.xml ( 26666 bytes, from 2019-12-20 21:20:35)
+- common.xml ( 35468 bytes, from 2018-02-10 13:09:26)
+- common_3d.xml ( 15058 bytes, from 2019-12-28 20:02:03)
+- state_hi.xml ( 30552 bytes, from 2019-12-28 20:02:48)
+- copyright.xml ( 1597 bytes, from 2018-02-10 13:09:26)
+- state_2d.xml ( 51552 bytes, from 2018-02-10 13:09:26)
+- state_3d.xml ( 83098 bytes, from 2019-12-28 20:02:03)
+- state_blt.xml ( 14252 bytes, from 2019-10-20 19:59:15)
+- state_vg.xml ( 5975 bytes, from 2018-02-10 13:09:26)
+
+Copyright (C) 2012-2019 by the following authors:
- Wladimir J. van der Laan <[email protected]>
- Christian Gmeiner <[email protected]>
- Lucas Stach <[email protected]>
@@ -48,6 +48,9 @@ DEALINGS IN THE SOFTWARE.
#define MMU_EXCEPTION_SLAVE_NOT_PRESENT 0x00000001
#define MMU_EXCEPTION_PAGE_NOT_PRESENT 0x00000002
#define MMU_EXCEPTION_WRITE_VIOLATION 0x00000003
+#define MMU_EXCEPTION_OUT_OF_BOUND 0x00000004
+#define MMU_EXCEPTION_READ_SECURITY_VIOLATION 0x00000005
+#define MMU_EXCEPTION_WRITE_SECURITY_VIOLATION 0x00000006
#define VIVS_HI 0x00000000
#define VIVS_HI_CLOCK_CONTROL 0x00000000
@@ -140,6 +143,8 @@ DEALINGS IN THE SOFTWARE.
#define VIVS_HI_CHIP_TIME 0x0000002c
+#define VIVS_HI_CHIP_CUSTOMER_ID 0x00000030
+
#define VIVS_HI_CHIP_MINOR_FEATURE_0 0x00000034
#define VIVS_HI_CACHE_CONTROL 0x00000038
@@ -237,6 +242,8 @@ DEALINGS IN THE SOFTWARE.
#define VIVS_HI_BLT_INTR 0x000000d4
+#define VIVS_HI_CHIP_ECO_ID 0x000000e8
+
#define VIVS_HI_AUXBIT 0x000000ec
#define VIVS_PM 0x00000000
--
2.24.1
On Do, 2020-01-02 at 11:02 +0100, Christian Gmeiner wrote:
> They will be used for extended HWDB support. The eco id logic was taken
> from galcore kernel driver sources.
>
> Signed-off-by: Christian Gmeiner <[email protected]>
> ---
> drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 17 +++++++++++++++++
> drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 6 +++---
> 2 files changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index d47d1a8e0219..253301be9e95 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -321,6 +321,18 @@ static void etnaviv_hw_specs(struct etnaviv_gpu *gpu)
> gpu->identity.varyings_count -= 1;
> }
>
> +static void etnaviv_hw_eco_id(struct etnaviv_gpu *gpu)
> +{
> + const u32 chipDate = gpu_read(gpu, VIVS_HI_CHIP_DATE);
> + gpu->identity.eco_id = gpu_read(gpu, VIVS_HI_CHIP_ECO_ID);
> +
> + if (etnaviv_is_model_rev(gpu, GC1000, 0x5037) && (chipDate == 0x20120617))
> + gpu->identity.eco_id = 1;
> +
> + if (etnaviv_is_model_rev(gpu, GC320, 0x5303) && (chipDate == 0x20140511))
> + gpu->identity.eco_id = 1;
I'm not sure if those two checks warrant a separate function. Maybe
just place them besides the other ID fixups?
> +}
> +
> static void etnaviv_hw_identify(struct etnaviv_gpu *gpu)
> {
> u32 chipIdentity;
> @@ -362,6 +374,8 @@ static void etnaviv_hw_identify(struct etnaviv_gpu *gpu)
> }
> }
>
> + gpu->identity.product_id = gpu_read(gpu, VIVS_HI_CHIP_PRODUCT_ID);
> +
> /*
> * NXP likes to call the GPU on the i.MX6QP GC2000+, but in
> * reality it's just a re-branded GC3000. We can identify this
> @@ -375,6 +389,9 @@ static void etnaviv_hw_identify(struct etnaviv_gpu *gpu)
> }
> }
>
> + etnaviv_hw_eco_id(gpu);
> + gpu->identity.customer_id = gpu_read(gpu, VIVS_HI_CHIP_CUSTOMER_ID);
I don't like this scattering of identity register reads. Please move
all of those reads to the else clause where we currently read
model/rev. I doubt that the customer ID register is available on the
really early cores, that only have the VIVS_HI_CHIP_IDENTITY register.
Regards,
Lucas
> dev_info(gpu->dev, "model: GC%x, revision: %x\n",
> gpu->identity.model, gpu->identity.revision);
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> index 8f9bd4edc96a..68bd966e3916 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> @@ -15,11 +15,11 @@ struct etnaviv_gem_submit;
> struct etnaviv_vram_mapping;
>
> struct etnaviv_chip_identity {
> - /* Chip model. */
> u32 model;
> -
> - /* Revision value.*/
> u32 revision;
> + u32 product_id;
> + u32 customer_id;
> + u32 eco_id;
>
> /* Supported feature fields. */
> u32 features;
On Do, 2020-01-02 at 11:02 +0100, Christian Gmeiner wrote:
> Signed-off-by: Christian Gmeiner <[email protected]>
> ---
> drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index 253301be9e95..cecef5034db1 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -868,6 +868,18 @@ int etnaviv_gpu_debugfs(struct etnaviv_gpu *gpu,
> struct seq_file *m)
>
> verify_dma(gpu, &debug);
>
> + seq_puts(m, "\tidentity\n");
> + seq_printf(m, "\t model: 0x%x\n",
> + gpu->identity.model);
> + seq_printf(m, "\t revision: 0x%x\n",
> + gpu->identity.revision);
> + seq_printf(m, "\t product_id: 0x%x\n",
> + gpu->identity.product_id);
> + seq_printf(m, "\t customer_id: 0x%x\n",
> + gpu->identity.customer_id);
> + seq_printf(m, "\t eco_id: 0x%x\n",
> + gpu->identity.eco_id);
I like having this info in debugfs. Most of those seq_printf don't need
a line break though, as they fit well within the 80 char limit.
Regards,
Lucas
On Do, 2020-01-02 at 11:02 +0100, Christian Gmeiner wrote:
> Take product id, customer id and eco id into account. If that
> delivers no match try a search for model and revision.
>
> Signed-off-by: Christian Gmeiner <[email protected]>
> ---
> drivers/gpu/drm/etnaviv/etnaviv_hwdb.c | 19 ++++++++++++++++++-
> 1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
> index eb0f3eb87ced..d1744f1b44b1 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
> @@ -44,9 +44,26 @@ bool etnaviv_fill_identity_from_hwdb(struct etnaviv_gpu *gpu)
> struct etnaviv_chip_identity *ident = &gpu->identity;
> int i;
>
> + /* accurate match */
> for (i = 0; i < ARRAY_SIZE(etnaviv_chip_identities); i++) {
> if (etnaviv_chip_identities[i].model == ident->model &&
> - etnaviv_chip_identities[i].revision == ident->revision) {
> + etnaviv_chip_identities[i].revision == ident->revision &&
> + etnaviv_chip_identities[i].product_id == ident->product_id &&
Why not simply make this:
(etnaviv_chip_identities[i].product_id == ident->product_id ||
etnaviv_chip_identities[i].product_id == ~0U)
and similar for customer and eco ID?
With this we don't need two different walks through the HWDB, as long
as the more specific entries in the DB are ordered to the front of the
array.
Regards,
Lucas
> + etnaviv_chip_identities[i].customer_id == ident->customer_id &&
> + etnaviv_chip_identities[i].eco_id == ident->eco_id) {
> + memcpy(ident, &etnaviv_chip_identities[i],
> + sizeof(*ident));
> + return true;
> + }
> + }
> +
> + /* match based only on model and revision */
> + for (i = 0; i < ARRAY_SIZE(etnaviv_chip_identities); i++) {
> + if (etnaviv_chip_identities[i].model == ident->model &&
> + etnaviv_chip_identities[i].revision == ident->revision &&
> + etnaviv_chip_identities[i].product_id == ~0U &&
> + etnaviv_chip_identities[i].customer_id == ~0U &&
> + etnaviv_chip_identities[i].eco_id == ~0U) {
> memcpy(ident, &etnaviv_chip_identities[i],
> sizeof(*ident));
> return true;
Hi Lucas
Am Mo., 6. Jan. 2020 um 11:15 Uhr schrieb Lucas Stach <[email protected]>:
>
> On Do, 2020-01-02 at 11:02 +0100, Christian Gmeiner wrote:
> > Take product id, customer id and eco id into account. If that
> > delivers no match try a search for model and revision.
> >
> > Signed-off-by: Christian Gmeiner <[email protected]>
> > ---
> > drivers/gpu/drm/etnaviv/etnaviv_hwdb.c | 19 ++++++++++++++++++-
> > 1 file changed, 18 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
> > index eb0f3eb87ced..d1744f1b44b1 100644
> > --- a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
> > +++ b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
> > @@ -44,9 +44,26 @@ bool etnaviv_fill_identity_from_hwdb(struct etnaviv_gpu *gpu)
> > struct etnaviv_chip_identity *ident = &gpu->identity;
> > int i;
> >
> > + /* accurate match */
> > for (i = 0; i < ARRAY_SIZE(etnaviv_chip_identities); i++) {
> > if (etnaviv_chip_identities[i].model == ident->model &&
> > - etnaviv_chip_identities[i].revision == ident->revision) {
> > + etnaviv_chip_identities[i].revision == ident->revision &&
> > + etnaviv_chip_identities[i].product_id == ident->product_id &&
>
> Why not simply make this:
> (etnaviv_chip_identities[i].product_id == ident->product_id ||
> etnaviv_chip_identities[i].product_id == ~0U)
> and similar for customer and eco ID?
>
> With this we don't need two different walks through the HWDB, as long
> as the more specific entries in the DB are ordered to the front of the
> array.
>
Works for me too.. will be change in v2.
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info/privacypolicy
Hi Lucas
Am Mo., 6. Jan. 2020 um 11:03 Uhr schrieb Lucas Stach <[email protected]>:
>
> On Do, 2020-01-02 at 11:02 +0100, Christian Gmeiner wrote:
> > They will be used for extended HWDB support. The eco id logic was taken
> > from galcore kernel driver sources.
> >
> > Signed-off-by: Christian Gmeiner <[email protected]>
> > ---
> > drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 17 +++++++++++++++++
> > drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 6 +++---
> > 2 files changed, 20 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > index d47d1a8e0219..253301be9e95 100644
> > --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > @@ -321,6 +321,18 @@ static void etnaviv_hw_specs(struct etnaviv_gpu *gpu)
> > gpu->identity.varyings_count -= 1;
> > }
> >
> > +static void etnaviv_hw_eco_id(struct etnaviv_gpu *gpu)
> > +{
> > + const u32 chipDate = gpu_read(gpu, VIVS_HI_CHIP_DATE);
> > + gpu->identity.eco_id = gpu_read(gpu, VIVS_HI_CHIP_ECO_ID);
> > +
> > + if (etnaviv_is_model_rev(gpu, GC1000, 0x5037) && (chipDate == 0x20120617))
> > + gpu->identity.eco_id = 1;
> > +
> > + if (etnaviv_is_model_rev(gpu, GC320, 0x5303) && (chipDate == 0x20140511))
> > + gpu->identity.eco_id = 1;
>
> I'm not sure if those two checks warrant a separate function. Maybe
> just place them besides the other ID fixups?
>
This is almost a 1:1 copy of _GetEcoID(..) but will try to move the fixups.
> > +}
> > +
> > static void etnaviv_hw_identify(struct etnaviv_gpu *gpu)
> > {
> > u32 chipIdentity;
> > @@ -362,6 +374,8 @@ static void etnaviv_hw_identify(struct etnaviv_gpu *gpu)
> > }
> > }
> >
> > + gpu->identity.product_id = gpu_read(gpu, VIVS_HI_CHIP_PRODUCT_ID);
> > +
> > /*
> > * NXP likes to call the GPU on the i.MX6QP GC2000+, but in
> > * reality it's just a re-branded GC3000. We can identify this
> > @@ -375,6 +389,9 @@ static void etnaviv_hw_identify(struct etnaviv_gpu *gpu)
> > }
> > }
> >
> > + etnaviv_hw_eco_id(gpu);
> > + gpu->identity.customer_id = gpu_read(gpu, VIVS_HI_CHIP_CUSTOMER_ID);
>
> I don't like this scattering of identity register reads. Please move
> all of those reads to the else clause where we currently read
> model/rev. I doubt that the customer ID register is available on the
> really early cores, that only have the VIVS_HI_CHIP_IDENTITY register.
>
There is feature bit for it: chipMinorFeatures5_HAS_PRODUCTID
Will change the code to make use of it. Shall I still put it in the
else clause then?
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info/privacypolicy
Hi Lucas,
Am Mo., 6. Jan. 2020 um 11:08 Uhr schrieb Lucas Stach <[email protected]>:
>
> On Do, 2020-01-02 at 11:02 +0100, Christian Gmeiner wrote:
> > Signed-off-by: Christian Gmeiner <[email protected]>
> > ---
> > drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 12 ++++++++++++
> > 1 file changed, 12 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > index 253301be9e95..cecef5034db1 100644
> > --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > @@ -868,6 +868,18 @@ int etnaviv_gpu_debugfs(struct etnaviv_gpu *gpu,
> > struct seq_file *m)
> >
> > verify_dma(gpu, &debug);
> >
> > + seq_puts(m, "\tidentity\n");
> > + seq_printf(m, "\t model: 0x%x\n",
> > + gpu->identity.model);
> > + seq_printf(m, "\t revision: 0x%x\n",
> > + gpu->identity.revision);
> > + seq_printf(m, "\t product_id: 0x%x\n",
> > + gpu->identity.product_id);
> > + seq_printf(m, "\t customer_id: 0x%x\n",
> > + gpu->identity.customer_id);
> > + seq_printf(m, "\t eco_id: 0x%x\n",
> > + gpu->identity.eco_id);
>
> I like having this info in debugfs. Most of those seq_printf don't need
> a line break though, as they fit well within the 80 char limit.
Ok..
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info/privacypolicy
On Mo, 2020-01-06 at 11:57 +0100, Christian Gmeiner wrote:
> Hi Lucas
>
> Am Mo., 6. Jan. 2020 um 11:03 Uhr schrieb Lucas Stach <[email protected]>:
> > On Do, 2020-01-02 at 11:02 +0100, Christian Gmeiner wrote:
> > > They will be used for extended HWDB support. The eco id logic was taken
> > > from galcore kernel driver sources.
> > >
> > > Signed-off-by: Christian Gmeiner <[email protected]>
> > > ---
> > > drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 17 +++++++++++++++++
> > > drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 6 +++---
> > > 2 files changed, 20 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > > index d47d1a8e0219..253301be9e95 100644
> > > --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > > @@ -321,6 +321,18 @@ static void etnaviv_hw_specs(struct etnaviv_gpu *gpu)
> > > gpu->identity.varyings_count -= 1;
> > > }
> > >
> > > +static void etnaviv_hw_eco_id(struct etnaviv_gpu *gpu)
> > > +{
> > > + const u32 chipDate = gpu_read(gpu, VIVS_HI_CHIP_DATE);
> > > + gpu->identity.eco_id = gpu_read(gpu, VIVS_HI_CHIP_ECO_ID);
> > > +
> > > + if (etnaviv_is_model_rev(gpu, GC1000, 0x5037) && (chipDate == 0x20120617))
> > > + gpu->identity.eco_id = 1;
> > > +
> > > + if (etnaviv_is_model_rev(gpu, GC320, 0x5303) && (chipDate == 0x20140511))
> > > + gpu->identity.eco_id = 1;
> >
> > I'm not sure if those two checks warrant a separate function. Maybe
> > just place them besides the other ID fixups?
> >
>
> This is almost a 1:1 copy of _GetEcoID(..) but will try to move the fixups.
>
>
> > > +}
> > > +
> > > static void etnaviv_hw_identify(struct etnaviv_gpu *gpu)
> > > {
> > > u32 chipIdentity;
> > > @@ -362,6 +374,8 @@ static void etnaviv_hw_identify(struct etnaviv_gpu *gpu)
> > > }
> > > }
> > >
> > > + gpu->identity.product_id = gpu_read(gpu, VIVS_HI_CHIP_PRODUCT_ID);
> > > +
> > > /*
> > > * NXP likes to call the GPU on the i.MX6QP GC2000+, but in
> > > * reality it's just a re-branded GC3000. We can identify this
> > > @@ -375,6 +389,9 @@ static void etnaviv_hw_identify(struct etnaviv_gpu *gpu)
> > > }
> > > }
> > >
> > > + etnaviv_hw_eco_id(gpu);
> > > + gpu->identity.customer_id = gpu_read(gpu, VIVS_HI_CHIP_CUSTOMER_ID);
> >
> > I don't like this scattering of identity register reads. Please move
> > all of those reads to the else clause where we currently read
> > model/rev. I doubt that the customer ID register is available on the
> > really early cores, that only have the VIVS_HI_CHIP_IDENTITY register.
> >
>
> There is feature bit for it: chipMinorFeatures5_HAS_PRODUCTID
> Will change the code to make use of it. Shall I still put it in the
> else clause then?
If there's a feature bit we need to move the read toward the end of the
function, as we currently read the features as the last step in the
hw_identify.
But then I'm not sure if the HAS_PRODUCTID feature bit is correct. At
least wumpus' gpus_comparison says that none of the known <= GC3000
cores has it set, which seems... suspicious.
Regards,
Lucas
Hi Lucas
Am Mo., 6. Jan. 2020 um 12:22 Uhr schrieb Lucas Stach <[email protected]>:
>
> On Mo, 2020-01-06 at 11:57 +0100, Christian Gmeiner wrote:
> > Hi Lucas
> >
> > Am Mo., 6. Jan. 2020 um 11:03 Uhr schrieb Lucas Stach <[email protected]>:
> > > On Do, 2020-01-02 at 11:02 +0100, Christian Gmeiner wrote:
> > > > They will be used for extended HWDB support. The eco id logic was taken
> > > > from galcore kernel driver sources.
> > > >
> > > > Signed-off-by: Christian Gmeiner <[email protected]>
> > > > ---
> > > > drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 17 +++++++++++++++++
> > > > drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 6 +++---
> > > > 2 files changed, 20 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > > > index d47d1a8e0219..253301be9e95 100644
> > > > --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > > > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > > > @@ -321,6 +321,18 @@ static void etnaviv_hw_specs(struct etnaviv_gpu *gpu)
> > > > gpu->identity.varyings_count -= 1;
> > > > }
> > > >
> > > > +static void etnaviv_hw_eco_id(struct etnaviv_gpu *gpu)
> > > > +{
> > > > + const u32 chipDate = gpu_read(gpu, VIVS_HI_CHIP_DATE);
> > > > + gpu->identity.eco_id = gpu_read(gpu, VIVS_HI_CHIP_ECO_ID);
> > > > +
> > > > + if (etnaviv_is_model_rev(gpu, GC1000, 0x5037) && (chipDate == 0x20120617))
> > > > + gpu->identity.eco_id = 1;
> > > > +
> > > > + if (etnaviv_is_model_rev(gpu, GC320, 0x5303) && (chipDate == 0x20140511))
> > > > + gpu->identity.eco_id = 1;
> > >
> > > I'm not sure if those two checks warrant a separate function. Maybe
> > > just place them besides the other ID fixups?
> > >
> >
> > This is almost a 1:1 copy of _GetEcoID(..) but will try to move the fixups.
> >
> >
> > > > +}
> > > > +
> > > > static void etnaviv_hw_identify(struct etnaviv_gpu *gpu)
> > > > {
> > > > u32 chipIdentity;
> > > > @@ -362,6 +374,8 @@ static void etnaviv_hw_identify(struct etnaviv_gpu *gpu)
> > > > }
> > > > }
> > > >
> > > > + gpu->identity.product_id = gpu_read(gpu, VIVS_HI_CHIP_PRODUCT_ID);
> > > > +
> > > > /*
> > > > * NXP likes to call the GPU on the i.MX6QP GC2000+, but in
> > > > * reality it's just a re-branded GC3000. We can identify this
> > > > @@ -375,6 +389,9 @@ static void etnaviv_hw_identify(struct etnaviv_gpu *gpu)
> > > > }
> > > > }
> > > >
> > > > + etnaviv_hw_eco_id(gpu);
> > > > + gpu->identity.customer_id = gpu_read(gpu, VIVS_HI_CHIP_CUSTOMER_ID);
> > >
> > > I don't like this scattering of identity register reads. Please move
> > > all of those reads to the else clause where we currently read
> > > model/rev. I doubt that the customer ID register is available on the
> > > really early cores, that only have the VIVS_HI_CHIP_IDENTITY register.
> > >
> >
> > There is feature bit for it: chipMinorFeatures5_HAS_PRODUCTID
> > Will change the code to make use of it. Shall I still put it in the
> > else clause then?
>
> If there's a feature bit we need to move the read toward the end of the
> function, as we currently read the features as the last step in the
> hw_identify.
>
> But then I'm not sure if the HAS_PRODUCTID feature bit is correct. At
> least wumpus' gpus_comparison says that none of the known <= GC3000
> cores has it set, which seems... suspicious.
>
Hmm... what if I just mimic what is done here?
https://github.com/etnaviv/vivante_kernel_drivers/blob/master/imx8_v6.2.3.129602/hal/kernel/arch/gc_hal_kernel_hardware.c#L220
Unconditionally read the product id at the end of the else clause. The
same is done in the stm32 galcore kernel driver.
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info/privacypolicy
On Mo, 2020-01-06 at 12:40 +0100, Christian Gmeiner wrote:
> Hi Lucas
>
> Am Mo., 6. Jan. 2020 um 12:22 Uhr schrieb Lucas Stach <[email protected]>:
> > On Mo, 2020-01-06 at 11:57 +0100, Christian Gmeiner wrote:
> > > Hi Lucas
> > >
> > > Am Mo., 6. Jan. 2020 um 11:03 Uhr schrieb Lucas Stach <[email protected]>:
> > > > On Do, 2020-01-02 at 11:02 +0100, Christian Gmeiner wrote:
> > > > > They will be used for extended HWDB support. The eco id logic was taken
> > > > > from galcore kernel driver sources.
> > > > >
> > > > > Signed-off-by: Christian Gmeiner <[email protected]>
> > > > > ---
> > > > > drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 17 +++++++++++++++++
> > > > > drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 6 +++---
> > > > > 2 files changed, 20 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > > > > index d47d1a8e0219..253301be9e95 100644
> > > > > --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > > > > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > > > > @@ -321,6 +321,18 @@ static void etnaviv_hw_specs(struct etnaviv_gpu *gpu)
> > > > > gpu->identity.varyings_count -= 1;
> > > > > }
> > > > >
> > > > > +static void etnaviv_hw_eco_id(struct etnaviv_gpu *gpu)
> > > > > +{
> > > > > + const u32 chipDate = gpu_read(gpu, VIVS_HI_CHIP_DATE);
> > > > > + gpu->identity.eco_id = gpu_read(gpu, VIVS_HI_CHIP_ECO_ID);
> > > > > +
> > > > > + if (etnaviv_is_model_rev(gpu, GC1000, 0x5037) && (chipDate == 0x20120617))
> > > > > + gpu->identity.eco_id = 1;
> > > > > +
> > > > > + if (etnaviv_is_model_rev(gpu, GC320, 0x5303) && (chipDate == 0x20140511))
> > > > > + gpu->identity.eco_id = 1;
> > > >
> > > > I'm not sure if those two checks warrant a separate function. Maybe
> > > > just place them besides the other ID fixups?
> > > >
> > >
> > > This is almost a 1:1 copy of _GetEcoID(..) but will try to move the fixups.
> > >
> > >
> > > > > +}
> > > > > +
> > > > > static void etnaviv_hw_identify(struct etnaviv_gpu *gpu)
> > > > > {
> > > > > u32 chipIdentity;
> > > > > @@ -362,6 +374,8 @@ static void etnaviv_hw_identify(struct etnaviv_gpu *gpu)
> > > > > }
> > > > > }
> > > > >
> > > > > + gpu->identity.product_id = gpu_read(gpu, VIVS_HI_CHIP_PRODUCT_ID);
> > > > > +
> > > > > /*
> > > > > * NXP likes to call the GPU on the i.MX6QP GC2000+, but in
> > > > > * reality it's just a re-branded GC3000. We can identify this
> > > > > @@ -375,6 +389,9 @@ static void etnaviv_hw_identify(struct etnaviv_gpu *gpu)
> > > > > }
> > > > > }
> > > > >
> > > > > + etnaviv_hw_eco_id(gpu);
> > > > > + gpu->identity.customer_id = gpu_read(gpu, VIVS_HI_CHIP_CUSTOMER_ID);
> > > >
> > > > I don't like this scattering of identity register reads. Please move
> > > > all of those reads to the else clause where we currently read
> > > > model/rev. I doubt that the customer ID register is available on the
> > > > really early cores, that only have the VIVS_HI_CHIP_IDENTITY register.
> > > >
> > >
> > > There is feature bit for it: chipMinorFeatures5_HAS_PRODUCTID
> > > Will change the code to make use of it. Shall I still put it in the
> > > else clause then?
> >
> > If there's a feature bit we need to move the read toward the end of the
> > function, as we currently read the features as the last step in the
> > hw_identify.
> >
> > But then I'm not sure if the HAS_PRODUCTID feature bit is correct. At
> > least wumpus' gpus_comparison says that none of the known <= GC3000
> > cores has it set, which seems... suspicious.
> >
>
> Hmm... what if I just mimic what is done here?
> https://github.com/etnaviv/vivante_kernel_drivers/blob/master/imx8_v6.2.3.129602/hal/kernel/arch/gc_hal_kernel_hardware.c#L220
>
> Unconditionally read the product id at the end of the else clause. The
> same is done in the stm32 galcore kernel driver.
If we read it unconditionally, just move it to the start of the else
clause, next to the model/rev reads and be done with it.
Regards,
Lucas