Re: [PATCH v2 05/15] drm/panthor: Add the GPU logical block

2023-08-29 Thread Boris Brezillon
On Mon, 21 Aug 2023 17:09:49 +0100
Robin Murphy  wrote:

> On 2023-08-14 11:54, Steven Price wrote:
> [...]
> >> +/**
> >> + * panthor_gpu_l2_power_on() - Power-on the L2-cache
> >> + * @ptdev: Device.
> >> + *
> >> + * Return: 0 on success, a negative error code otherwise.
> >> + */
> >> +int panthor_gpu_l2_power_on(struct panthor_device *ptdev)
> >> +{
> >> +  u64 core_mask = U64_MAX;
> >> +
> >> +  if (ptdev->gpu_info.l2_present != 1) {
> >> +  /*
> >> +   * Only support one core group now.
> >> +   * ~(l2_present - 1) unsets all bits in l2_present except
> >> +   * the bottom bit. (l2_present - 2) has all the bits in
> >> +   * the first core group set. AND them together to generate
> >> +   * a mask of cores in the first core group.
> >> +   */
> >> +  core_mask = ~(ptdev->gpu_info.l2_present - 1) &
> >> +   (ptdev->gpu_info.l2_present - 2);
> >> +  drm_info_once(>base, "using only 1st core group (%lu 
> >> cores from %lu)\n",
> >> +hweight64(core_mask),
> >> +hweight64(ptdev->gpu_info.shader_present));  
> > 
> > I'm not sure what the point of this complexity is. This boils down to
> > the equivalent of:
> > 
> > if (ptdev->gpu_info.l2_present != 1)
> > core_mask = 1;  
> 
> Hmm, that doesn't look right - the idiom here should be to set all bits 
> of the output below the *second* set bit of the input, i.e. 0x11 -> 
> 0x0f.

Ah ah, I should really read the whole thread before replying :-).

> However since panthor is (somewhat ironically) unlikely to ever 
> run on T628, and everything newer should pretend to have a single L2 
> because software-managed coherency is a terrible idea, I would agree 
> that ultimately it does all seem a bit pointless.

Okay, good to know.

> 
> > If we were doing shader-core power management manually (like on pre-CSF
> > GPUs, rather than letting the firmware control it) then the computed
> > core_mask would be useful. So I guess it comes down to the
> > drm_info_once() output and counting the cores - which is nice to have
> > but it took me some time figuring out what was going on here.  
> As for the complexity, I'd suggest you can have some choice words with 
> the guy who originally suggested that code[1] ;)

:'-)


Re: [PATCH v2 05/15] drm/panthor: Add the GPU logical block

2023-08-29 Thread Boris Brezillon
On Mon, 14 Aug 2023 11:54:27 +0100
Steven Price  wrote:

> On 09/08/2023 17:53, Boris Brezillon wrote:
> > Handles everything that's not related to the FW, the MMU or the
> > scheduler. This is the block dealing with the GPU property retrieval,
> > the GPU block power on/off logic, and some global operations, like
> > global cache flushing.
> > 
> > v2:
> > - Rename the driver (pancsf -> panthor)
> > - Change the license (GPL2 -> MIT + GPL2)
> > - Split the driver addition commit
> > - Use drm_dev_{unplug,enter,exit}() to provide safe device removal
> > - Use the panthor_irq layer to manage/process IRQs
> > 
> > Signed-off-by: Boris Brezillon 
> > ---
> >  drivers/gpu/drm/panthor/panthor_gpu.c | 463 ++
> >  drivers/gpu/drm/panthor/panthor_gpu.h |  52 +++
> >  2 files changed, 515 insertions(+)
> >  create mode 100644 drivers/gpu/drm/panthor/panthor_gpu.c
> >  create mode 100644 drivers/gpu/drm/panthor/panthor_gpu.h
> > 
> > diff --git a/drivers/gpu/drm/panthor/panthor_gpu.c 
> > b/drivers/gpu/drm/panthor/panthor_gpu.c
> > new file mode 100644
> > index ..47d15334b46e
> > --- /dev/null
> > +++ b/drivers/gpu/drm/panthor/panthor_gpu.c
> > @@ -0,0 +1,463 @@
> > +// SPDX-License-Identifier: GPL-2.0 or MIT
> > +/* Copyright 2018 Marty E. Plummer  */
> > +/* Copyright 2019 Linaro, Ltd., Rob Herring  */
> > +/* Copyright 2019 Collabora ltd. */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include 
> > +#include 
> > +
> > +#include "panthor_device.h"
> > +#include "panthor_gpu.h"
> > +#include "panthor_regs.h"
> > +
> > +/**
> > + * struct panthor_gpu - GPU block management data.
> > + */
> > +struct panthor_gpu {
> > +   /** @irq: GPU irq. */
> > +   struct panthor_irq irq;
> > +
> > +   /** @reqs_lock: Lock protecting access to pending_reqs. */
> > +   spinlock_t reqs_lock;
> > +
> > +   /** @pending_reqs: Pending GPU requests. */
> > +   u32 pending_reqs;
> > +
> > +   /** @reqs_acked: GPU request wait queue. */
> > +   wait_queue_head_t reqs_acked;
> > +};
> > +
> > +/**
> > + * struct panthor_model - GPU model description
> > + */
> > +struct panthor_model {
> > +   /** @name: Model name. */
> > +   const char *name;
> > +
> > +   /** @id: Model ID. */
> > +   u32 id;
> > +};
> > +
> > +/**
> > + * GPU_MODEL() - Define a GPU model.
> > + */
> > +#define GPU_MODEL(_name, _id, ...) \
> > +{\
> > +   .name = __stringify(_name), \
> > +   .id = _id,  \
> > +}
> > +
> > +#define GPU_MODEL_ID_MASK  0xf00f  
> 
> I would be nice if we had defines for the two components that make this
> up (ARCH_MAJOR | PRODUCT_MAJOR). It might even be easier to read the
> model list below if we split ID into arch/product combinations (which
> can then be written in decimal rather than hex).

Sure, I can do that.

> 
> > +
> > +static const struct panthor_model gpu_models[] = {
> > +   GPU_MODEL(g610, 0xa007),
> > +   {},
> > +};
> > +
> > +#define GPU_INTERRUPTS_MASK\
> > +   (GPU_IRQ_FAULT | \
> > +GPU_IRQ_PROTM_FAULT | \
> > +GPU_IRQ_RESET_COMPLETED | \
> > +GPU_IRQ_MCU_STATUS_CHANGED | \  
> 
> The code doesn't seem to use the MCU_STATUS_CHANGED interrupt, if it's
> not used then it doesn't make sense to be in the mask.

Oops, I intended to use it and probably never did (I think I'm polling
some register to check the MCU status change). I'll drop this interrupt.

> 
> > +GPU_IRQ_CLEAN_CACHES_COMPLETED)
> > +
> > +static void panthor_gpu_init_info(struct panthor_device *ptdev)
> > +{
> > +   const struct panthor_model *model;
> > +   u32 major, minor, status;
> > +   unsigned int i;
> > +
> > +   ptdev->gpu_info.gpu_id = gpu_read(ptdev, GPU_ID);
> > +   ptdev->gpu_info.csf_id = gpu_read(ptdev, GPU_CSF_ID);
> > +   ptdev->gpu_info.gpu_rev = gpu_read(ptdev, GPU_REVID);
> > +   ptdev->gpu_info.l2_features = gpu_read(ptdev, GPU_L2_FEATURES);
> > +   ptdev->gpu_info.tiler_features = gpu_read(ptdev, GPU_TILER_FEATURES);
> > +   ptdev->gpu_info.mem_features = gpu_read(ptdev, GPU_MEM_FEATURES);
> > +   ptdev->gpu_info.mmu_features = gpu_read(ptdev, GPU_MMU_FEATURES);
> > +   ptdev->gpu_info.thread_features = gpu_read(ptdev, GPU_THREAD_FEATURES);
> > +   ptdev->gpu_info.max_threads = gpu_read(ptdev, GPU_THREAD_MAX_THREADS);
> > +   ptdev->gpu_info.thread_max_workgroup_size = gpu_read(ptdev, 
> > GPU_THREAD_MAX_WORKGROUP_SIZE);
> > +   ptdev->gpu_info.thread_max_barrier_size = gpu_read(ptdev, 
> > GPU_THREAD_MAX_BARRIER_SIZE);
> > +   ptdev->gpu_info.coherency_features = gpu_read(ptdev, 
> > GPU_COHERENCY_FEATURES);
> > +   for (i = 0; i < 4; i++)
> > +   ptdev->gpu_info.texture_features[i] = gpu_read(ptdev, 
> > GPU_TEXTURE_FEATURES(i));
> > +
> > +   ptdev->gpu_info.as_present = gpu_read(ptdev, GPU_AS_PRESENT);
> > +
> > +   ptdev->gpu_info.shader_present = gpu_read(ptdev, 

Re: [PATCH v2 05/15] drm/panthor: Add the GPU logical block

2023-08-23 Thread Steven Price
On 21/08/2023 17:09, Robin Murphy wrote:
> On 2023-08-14 11:54, Steven Price wrote:
> [...]
>>> +/**
>>> + * panthor_gpu_l2_power_on() - Power-on the L2-cache
>>> + * @ptdev: Device.
>>> + *
>>> + * Return: 0 on success, a negative error code otherwise.
>>> + */
>>> +int panthor_gpu_l2_power_on(struct panthor_device *ptdev)
>>> +{
>>> +    u64 core_mask = U64_MAX;
>>> +
>>> +    if (ptdev->gpu_info.l2_present != 1) {
>>> +    /*
>>> + * Only support one core group now.
>>> + * ~(l2_present - 1) unsets all bits in l2_present except
>>> + * the bottom bit. (l2_present - 2) has all the bits in
>>> + * the first core group set. AND them together to generate
>>> + * a mask of cores in the first core group.
>>> + */
>>> +    core_mask = ~(ptdev->gpu_info.l2_present - 1) &
>>> + (ptdev->gpu_info.l2_present - 2);
>>> +    drm_info_once(>base, "using only 1st core group (%lu
>>> cores from %lu)\n",
>>> +  hweight64(core_mask),
>>> +  hweight64(ptdev->gpu_info.shader_present));
>>
>> I'm not sure what the point of this complexity is. This boils down to
>> the equivalent of:
>>
>> if (ptdev->gpu_info.l2_present != 1)
>>     core_mask = 1;
> 
> Hmm, that doesn't look right - the idiom here should be to set all bits
> of the output below the *second* set bit of the input, i.e. 0x11 ->
> 0x0f. However since panthor is (somewhat ironically) unlikely to ever
> run on T628, and everything newer should pretend to have a single L2
> because software-managed coherency is a terrible idea, I would agree
> that ultimately it does all seem a bit pointless.

Sorry I should have been clearer here. Other than the message printed 
(using drm_info_once) the only use of core_mask in this function is in 
the call to panthor_gpu_power_on:

+   return panthor_gpu_power_on(ptdev, L2,
+   ptdev->gpu_info.l2_present & core_mask,
+   2);

Here the core_mask variable is anded with l2_present. So using the value 
1 is equivalent to the actual core mask which is being calculated. 
Obviously '1' isn't likely to be the real core mask (it's an "L2 mask").

Mostly it just seemed odd to calculate the core_mask and then 
effectively throw the value away.

>> If we were doing shader-core power management manually (like on pre-CSF
>> GPUs, rather than letting the firmware control it) then the computed
>> core_mask would be useful. So I guess it comes down to the
>> drm_info_once() output and counting the cores - which is nice to have
>> but it took me some time figuring out what was going on here.
> As for the complexity, I'd suggest you can have some choice words with
> the guy who originally suggested that code[1] ;)

I do often have problems with the code that guy wrote ;)

Steve

> 
> Cheers,
> Robin.
> 
> [1]
> https://lore.kernel.org/dri-devel/b009b4c4-0396-58c2-7779-30c844f36...@arm.com/



Re: [PATCH v2 05/15] drm/panthor: Add the GPU logical block

2023-08-21 Thread Robin Murphy

On 2023-08-14 11:54, Steven Price wrote:
[...]

+/**
+ * panthor_gpu_l2_power_on() - Power-on the L2-cache
+ * @ptdev: Device.
+ *
+ * Return: 0 on success, a negative error code otherwise.
+ */
+int panthor_gpu_l2_power_on(struct panthor_device *ptdev)
+{
+   u64 core_mask = U64_MAX;
+
+   if (ptdev->gpu_info.l2_present != 1) {
+   /*
+* Only support one core group now.
+* ~(l2_present - 1) unsets all bits in l2_present except
+* the bottom bit. (l2_present - 2) has all the bits in
+* the first core group set. AND them together to generate
+* a mask of cores in the first core group.
+*/
+   core_mask = ~(ptdev->gpu_info.l2_present - 1) &
+(ptdev->gpu_info.l2_present - 2);
+   drm_info_once(>base, "using only 1st core group (%lu cores 
from %lu)\n",
+ hweight64(core_mask),
+ hweight64(ptdev->gpu_info.shader_present));


I'm not sure what the point of this complexity is. This boils down to
the equivalent of:

if (ptdev->gpu_info.l2_present != 1)
core_mask = 1;


Hmm, that doesn't look right - the idiom here should be to set all bits 
of the output below the *second* set bit of the input, i.e. 0x11 -> 
0x0f. However since panthor is (somewhat ironically) unlikely to ever 
run on T628, and everything newer should pretend to have a single L2 
because software-managed coherency is a terrible idea, I would agree 
that ultimately it does all seem a bit pointless.



If we were doing shader-core power management manually (like on pre-CSF
GPUs, rather than letting the firmware control it) then the computed
core_mask would be useful. So I guess it comes down to the
drm_info_once() output and counting the cores - which is nice to have
but it took me some time figuring out what was going on here.
As for the complexity, I'd suggest you can have some choice words with 
the guy who originally suggested that code[1] ;)


Cheers,
Robin.

[1] 
https://lore.kernel.org/dri-devel/b009b4c4-0396-58c2-7779-30c844f36...@arm.com/


Re: [PATCH v2 05/15] drm/panthor: Add the GPU logical block

2023-08-14 Thread Steven Price
On 09/08/2023 17:53, Boris Brezillon wrote:
> Handles everything that's not related to the FW, the MMU or the
> scheduler. This is the block dealing with the GPU property retrieval,
> the GPU block power on/off logic, and some global operations, like
> global cache flushing.
> 
> v2:
> - Rename the driver (pancsf -> panthor)
> - Change the license (GPL2 -> MIT + GPL2)
> - Split the driver addition commit
> - Use drm_dev_{unplug,enter,exit}() to provide safe device removal
> - Use the panthor_irq layer to manage/process IRQs
> 
> Signed-off-by: Boris Brezillon 
> ---
>  drivers/gpu/drm/panthor/panthor_gpu.c | 463 ++
>  drivers/gpu/drm/panthor/panthor_gpu.h |  52 +++
>  2 files changed, 515 insertions(+)
>  create mode 100644 drivers/gpu/drm/panthor/panthor_gpu.c
>  create mode 100644 drivers/gpu/drm/panthor/panthor_gpu.h
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_gpu.c 
> b/drivers/gpu/drm/panthor/panthor_gpu.c
> new file mode 100644
> index ..47d15334b46e
> --- /dev/null
> +++ b/drivers/gpu/drm/panthor/panthor_gpu.c
> @@ -0,0 +1,463 @@
> +// SPDX-License-Identifier: GPL-2.0 or MIT
> +/* Copyright 2018 Marty E. Plummer  */
> +/* Copyright 2019 Linaro, Ltd., Rob Herring  */
> +/* Copyright 2019 Collabora ltd. */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +
> +#include "panthor_device.h"
> +#include "panthor_gpu.h"
> +#include "panthor_regs.h"
> +
> +/**
> + * struct panthor_gpu - GPU block management data.
> + */
> +struct panthor_gpu {
> + /** @irq: GPU irq. */
> + struct panthor_irq irq;
> +
> + /** @reqs_lock: Lock protecting access to pending_reqs. */
> + spinlock_t reqs_lock;
> +
> + /** @pending_reqs: Pending GPU requests. */
> + u32 pending_reqs;
> +
> + /** @reqs_acked: GPU request wait queue. */
> + wait_queue_head_t reqs_acked;
> +};
> +
> +/**
> + * struct panthor_model - GPU model description
> + */
> +struct panthor_model {
> + /** @name: Model name. */
> + const char *name;
> +
> + /** @id: Model ID. */
> + u32 id;
> +};
> +
> +/**
> + * GPU_MODEL() - Define a GPU model.
> + */
> +#define GPU_MODEL(_name, _id, ...) \
> +{\
> + .name = __stringify(_name), \
> + .id = _id,  \
> +}
> +
> +#define GPU_MODEL_ID_MASK0xf00f

I would be nice if we had defines for the two components that make this
up (ARCH_MAJOR | PRODUCT_MAJOR). It might even be easier to read the
model list below if we split ID into arch/product combinations (which
can then be written in decimal rather than hex).

> +
> +static const struct panthor_model gpu_models[] = {
> + GPU_MODEL(g610, 0xa007),
> + {},
> +};
> +
> +#define GPU_INTERRUPTS_MASK  \
> + (GPU_IRQ_FAULT | \
> +  GPU_IRQ_PROTM_FAULT | \
> +  GPU_IRQ_RESET_COMPLETED | \
> +  GPU_IRQ_MCU_STATUS_CHANGED | \

The code doesn't seem to use the MCU_STATUS_CHANGED interrupt, if it's
not used then it doesn't make sense to be in the mask.

> +  GPU_IRQ_CLEAN_CACHES_COMPLETED)
> +
> +static void panthor_gpu_init_info(struct panthor_device *ptdev)
> +{
> + const struct panthor_model *model;
> + u32 major, minor, status;
> + unsigned int i;
> +
> + ptdev->gpu_info.gpu_id = gpu_read(ptdev, GPU_ID);
> + ptdev->gpu_info.csf_id = gpu_read(ptdev, GPU_CSF_ID);
> + ptdev->gpu_info.gpu_rev = gpu_read(ptdev, GPU_REVID);
> + ptdev->gpu_info.l2_features = gpu_read(ptdev, GPU_L2_FEATURES);
> + ptdev->gpu_info.tiler_features = gpu_read(ptdev, GPU_TILER_FEATURES);
> + ptdev->gpu_info.mem_features = gpu_read(ptdev, GPU_MEM_FEATURES);
> + ptdev->gpu_info.mmu_features = gpu_read(ptdev, GPU_MMU_FEATURES);
> + ptdev->gpu_info.thread_features = gpu_read(ptdev, GPU_THREAD_FEATURES);
> + ptdev->gpu_info.max_threads = gpu_read(ptdev, GPU_THREAD_MAX_THREADS);
> + ptdev->gpu_info.thread_max_workgroup_size = gpu_read(ptdev, 
> GPU_THREAD_MAX_WORKGROUP_SIZE);
> + ptdev->gpu_info.thread_max_barrier_size = gpu_read(ptdev, 
> GPU_THREAD_MAX_BARRIER_SIZE);
> + ptdev->gpu_info.coherency_features = gpu_read(ptdev, 
> GPU_COHERENCY_FEATURES);
> + for (i = 0; i < 4; i++)
> + ptdev->gpu_info.texture_features[i] = gpu_read(ptdev, 
> GPU_TEXTURE_FEATURES(i));
> +
> + ptdev->gpu_info.as_present = gpu_read(ptdev, GPU_AS_PRESENT);
> +
> + ptdev->gpu_info.shader_present = gpu_read(ptdev, GPU_SHADER_PRESENT_LO);
> + ptdev->gpu_info.shader_present |= (u64)gpu_read(ptdev, 
> GPU_SHADER_PRESENT_HI) << 32;
> +
> + ptdev->gpu_info.tiler_present = gpu_read(ptdev, GPU_TILER_PRESENT_LO);
> + ptdev->gpu_info.tiler_present |= (u64)gpu_read(ptdev, 
> GPU_TILER_PRESENT_HI) << 32;
> +
> + ptdev->gpu_info.l2_present = gpu_read(ptdev, GPU_L2_PRESENT_LO);
> + ptdev->gpu_info.l2_present |= (u64)gpu_read(ptdev, 

[PATCH v2 05/15] drm/panthor: Add the GPU logical block

2023-08-09 Thread Boris Brezillon
Handles everything that's not related to the FW, the MMU or the
scheduler. This is the block dealing with the GPU property retrieval,
the GPU block power on/off logic, and some global operations, like
global cache flushing.

v2:
- Rename the driver (pancsf -> panthor)
- Change the license (GPL2 -> MIT + GPL2)
- Split the driver addition commit
- Use drm_dev_{unplug,enter,exit}() to provide safe device removal
- Use the panthor_irq layer to manage/process IRQs

Signed-off-by: Boris Brezillon 
---
 drivers/gpu/drm/panthor/panthor_gpu.c | 463 ++
 drivers/gpu/drm/panthor/panthor_gpu.h |  52 +++
 2 files changed, 515 insertions(+)
 create mode 100644 drivers/gpu/drm/panthor/panthor_gpu.c
 create mode 100644 drivers/gpu/drm/panthor/panthor_gpu.h

diff --git a/drivers/gpu/drm/panthor/panthor_gpu.c 
b/drivers/gpu/drm/panthor/panthor_gpu.c
new file mode 100644
index ..47d15334b46e
--- /dev/null
+++ b/drivers/gpu/drm/panthor/panthor_gpu.c
@@ -0,0 +1,463 @@
+// SPDX-License-Identifier: GPL-2.0 or MIT
+/* Copyright 2018 Marty E. Plummer  */
+/* Copyright 2019 Linaro, Ltd., Rob Herring  */
+/* Copyright 2019 Collabora ltd. */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+#include "panthor_device.h"
+#include "panthor_gpu.h"
+#include "panthor_regs.h"
+
+/**
+ * struct panthor_gpu - GPU block management data.
+ */
+struct panthor_gpu {
+   /** @irq: GPU irq. */
+   struct panthor_irq irq;
+
+   /** @reqs_lock: Lock protecting access to pending_reqs. */
+   spinlock_t reqs_lock;
+
+   /** @pending_reqs: Pending GPU requests. */
+   u32 pending_reqs;
+
+   /** @reqs_acked: GPU request wait queue. */
+   wait_queue_head_t reqs_acked;
+};
+
+/**
+ * struct panthor_model - GPU model description
+ */
+struct panthor_model {
+   /** @name: Model name. */
+   const char *name;
+
+   /** @id: Model ID. */
+   u32 id;
+};
+
+/**
+ * GPU_MODEL() - Define a GPU model.
+ */
+#define GPU_MODEL(_name, _id, ...) \
+{\
+   .name = __stringify(_name), \
+   .id = _id,  \
+}
+
+#define GPU_MODEL_ID_MASK  0xf00f
+
+static const struct panthor_model gpu_models[] = {
+   GPU_MODEL(g610, 0xa007),
+   {},
+};
+
+#define GPU_INTERRUPTS_MASK\
+   (GPU_IRQ_FAULT | \
+GPU_IRQ_PROTM_FAULT | \
+GPU_IRQ_RESET_COMPLETED | \
+GPU_IRQ_MCU_STATUS_CHANGED | \
+GPU_IRQ_CLEAN_CACHES_COMPLETED)
+
+static void panthor_gpu_init_info(struct panthor_device *ptdev)
+{
+   const struct panthor_model *model;
+   u32 major, minor, status;
+   unsigned int i;
+
+   ptdev->gpu_info.gpu_id = gpu_read(ptdev, GPU_ID);
+   ptdev->gpu_info.csf_id = gpu_read(ptdev, GPU_CSF_ID);
+   ptdev->gpu_info.gpu_rev = gpu_read(ptdev, GPU_REVID);
+   ptdev->gpu_info.l2_features = gpu_read(ptdev, GPU_L2_FEATURES);
+   ptdev->gpu_info.tiler_features = gpu_read(ptdev, GPU_TILER_FEATURES);
+   ptdev->gpu_info.mem_features = gpu_read(ptdev, GPU_MEM_FEATURES);
+   ptdev->gpu_info.mmu_features = gpu_read(ptdev, GPU_MMU_FEATURES);
+   ptdev->gpu_info.thread_features = gpu_read(ptdev, GPU_THREAD_FEATURES);
+   ptdev->gpu_info.max_threads = gpu_read(ptdev, GPU_THREAD_MAX_THREADS);
+   ptdev->gpu_info.thread_max_workgroup_size = gpu_read(ptdev, 
GPU_THREAD_MAX_WORKGROUP_SIZE);
+   ptdev->gpu_info.thread_max_barrier_size = gpu_read(ptdev, 
GPU_THREAD_MAX_BARRIER_SIZE);
+   ptdev->gpu_info.coherency_features = gpu_read(ptdev, 
GPU_COHERENCY_FEATURES);
+   for (i = 0; i < 4; i++)
+   ptdev->gpu_info.texture_features[i] = gpu_read(ptdev, 
GPU_TEXTURE_FEATURES(i));
+
+   ptdev->gpu_info.as_present = gpu_read(ptdev, GPU_AS_PRESENT);
+
+   ptdev->gpu_info.shader_present = gpu_read(ptdev, GPU_SHADER_PRESENT_LO);
+   ptdev->gpu_info.shader_present |= (u64)gpu_read(ptdev, 
GPU_SHADER_PRESENT_HI) << 32;
+
+   ptdev->gpu_info.tiler_present = gpu_read(ptdev, GPU_TILER_PRESENT_LO);
+   ptdev->gpu_info.tiler_present |= (u64)gpu_read(ptdev, 
GPU_TILER_PRESENT_HI) << 32;
+
+   ptdev->gpu_info.l2_present = gpu_read(ptdev, GPU_L2_PRESENT_LO);
+   ptdev->gpu_info.l2_present |= (u64)gpu_read(ptdev, GPU_L2_PRESENT_HI) 
<< 32;
+   ptdev->gpu_info.core_group_count = 
hweight64(ptdev->gpu_info.l2_present);
+
+   major = (ptdev->gpu_info.gpu_id >> 12) & 0xf;
+   minor = (ptdev->gpu_info.gpu_id >> 4) & 0xff;
+   status = ptdev->gpu_info.gpu_id & 0xf;
+
+   for (model = gpu_models; model->name; model++) {
+   if (model->id == (ptdev->gpu_info.gpu_id & GPU_MODEL_ID_MASK))
+   break;
+   }
+
+   drm_info(>base,
+"mali-%s id 0x%x major 0x%x minor 0x%x status 0x%x",
+model->name ?: "unknown", ptdev->gpu_info.gpu_id >> 16,
+