| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAH9NwWdNabvh+FJATuD=0KxHvQv77OuYpLgd5iQAcSNNPbS4ew@mail.gmail.com>
Date: Thu, 9 Oct 2025 23:40:13 +0200
From: Christian Gmeiner <christian.gmeiner@...il.com>
To: Gert Wollny <gert.wollny@...labora.com>
Cc: Lucas Stach <l.stach@...gutronix.de>, Russell King <linux+etnaviv@...linux.org.uk>,
David Airlie <airlied@...il.com>, Daniel Vetter <daniel@...ll.ch>, etnaviv@...ts.freedesktop.org,
dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 4/5] drm/etnaviv: Add PPU flop reset
Hi Gert,
>
> The PPU flop reset is required on some hardware to clear the
> temporary registers. This code follows the implementation
> of the PPU flop reset as found in the public galcore kernel
> module. Compared to that code some superfluous parts were
> removed and only the code path for SoC chip_model = 0x8000
> and revision = 0x6205 is implemented and tested.
>
> v2: - Move flop reset data to etnaviv_drm_private and initialize it
> from etnaviv_gpu_bind (Lucas)
> - Prepare code for more chip IDs and other flop reset types
> - Do some cleanups and rename some functions
>
> v3: - Move initialization of flop reset data to etnaviv_gpu_init (Lucas)
> - Free PPU data suballocation (Lucas)
>
> Signed-off-by: Gert Wollny <gert.wollny@...labora.com>
> ---
> drivers/gpu/drm/etnaviv/Makefile | 1 +
> drivers/gpu/drm/etnaviv/etnaviv_buffer.c | 6 +
> drivers/gpu/drm/etnaviv/etnaviv_drv.c | 3 +
> drivers/gpu/drm/etnaviv/etnaviv_drv.h | 3 +
> drivers/gpu/drm/etnaviv/etnaviv_flop_reset.c | 206 +++++++++++++++++++
> drivers/gpu/drm/etnaviv/etnaviv_flop_reset.h | 25 +++
> drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 5 +
> 7 files changed, 249 insertions(+)
> create mode 100644 drivers/gpu/drm/etnaviv/etnaviv_flop_reset.c
> create mode 100644 drivers/gpu/drm/etnaviv/etnaviv_flop_reset.h
>
> diff --git a/drivers/gpu/drm/etnaviv/Makefile b/drivers/gpu/drm/etnaviv/Makefile
> index 46e5ffad69a6..903101e8751a 100644
> --- a/drivers/gpu/drm/etnaviv/Makefile
> +++ b/drivers/gpu/drm/etnaviv/Makefile
> @@ -14,6 +14,7 @@ etnaviv-y := \
> etnaviv_iommu.o \
> etnaviv_mmu.o \
> etnaviv_perfmon.o \
> + etnaviv_flop_reset.o \
> etnaviv_sched.o
>
> obj-$(CONFIG_DRM_ETNAVIV) += etnaviv.o
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_buffer.c b/drivers/gpu/drm/etnaviv/etnaviv_buffer.c
> index 9e007d977efe..a2da3212592f 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_buffer.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_buffer.c
> @@ -18,6 +18,8 @@
> #include "state_3d.xml.h"
> #include "cmdstream.xml.h"
>
> +#include "etnaviv_flop_reset.h"
> +
> static void etnaviv_cmd_select_pipe(struct etnaviv_gpu *gpu,
> struct etnaviv_cmdbuf *buffer, u8 pipe)
> {
> @@ -100,6 +102,10 @@ u16 etnaviv_buffer_init(struct etnaviv_gpu *gpu)
> /* initialize buffer */
> buffer->user_size = 0;
>
> + /* Queue in PPU flop reset */
> + if (etnaviv_flop_reset_ppu_require(&gpu->identity))
> + etnaviv_flop_reset_ppu_run(gpu);
> +
> CMD_WAIT(buffer, gpu->fe_waitcycles);
> CMD_LINK(buffer, 2,
> etnaviv_cmdbuf_get_va(buffer, &gpu->mmu_context->cmdbuf_mapping)
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> index 3e91747ed339..0e916e6785c8 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> @@ -600,6 +600,9 @@ static void etnaviv_unbind(struct device *dev)
>
> component_unbind_all(dev, drm);
>
> + etnaviv_cmdbuf_free(priv->flop_reset_data_ppu);
> + kfree(priv->flop_reset_data_ppu);
> +
> etnaviv_cmdbuf_suballoc_destroy(priv->cmdbuf_suballoc);
>
> xa_destroy(&priv->active_contexts);
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> index b3eb1662e90c..20dad16fd554 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> @@ -48,6 +48,9 @@ struct etnaviv_drm_private {
> /* list of GEM objects: */
> struct mutex gem_lock;
> struct list_head gem_list;
> +
> + /* ppu flop reset data */
> + struct etnaviv_cmdbuf *flop_reset_data_ppu;
> };
>
> int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data,
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_flop_reset.c b/drivers/gpu/drm/etnaviv/etnaviv_flop_reset.c
> new file mode 100644
> index 000000000000..138af3c33b5d
> --- /dev/null
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_flop_reset.c
> @@ -0,0 +1,206 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2025 Etnaviv Project
> + */
> +
> +#include "asm-generic/int-ll64.h"
I think you want to use #include <linux/types.h>
> +#include "etnaviv_buffer.h"
> +#include "etnaviv_cmdbuf.h"
> +#include "state_3d.xml.h"
> +
> +#include "etnaviv_flop_reset.h"
> +
> +enum etnaviv_flop_reset_type {
> + flop_reset_ppu = 1 << 0,
> + flop_reset_nn = 1 << 1,
> + flop_reset_tp = 1 << 2
> +};
As this patch series only adds support for flop_reset_ppu I think we should
drop this enum for now.
> +
> +#define PPU_IMAGE_STRIDE 64
> +#define PPU_IMAGE_XSIZE 64
> +#define PPU_IMAGE_YSIZE 6
> +
> +#define PPU_FLOP_RESET_INSTR_DWORD_COUNT 16
> +
> +static void
> +etnaviv_emit_flop_reset_state_ppu(struct etnaviv_cmdbuf *cmdbuf,
Return type on separate line is unusual in kernel code - found in
multiple places.
> + u32 buffer_base,
> + u32 input_offset,
> + u32 output_offset,
> + u32 shader_offset,
> + u32 shader_size,
> + u32 shader_register_count)
> +{
> + CMD_LOAD_STATE(cmdbuf, VIVS_GL_API_MODE,
> + VIVS_GL_API_MODE_OPENCL);
> + CMD_SEM(cmdbuf, SYNC_RECIPIENT_FE, SYNC_RECIPIENT_PE);
> + CMD_STALL(cmdbuf, SYNC_RECIPIENT_FE, SYNC_RECIPIENT_PE);
> +
> + CMD_LOAD_STATES_START(cmdbuf, VIVS_SH_HALTI5_UNIFORMS(0), 4);
> +
> + OUT(cmdbuf, buffer_base + input_offset);
> + OUT(cmdbuf, PPU_IMAGE_STRIDE);
> + OUT(cmdbuf, PPU_IMAGE_XSIZE | (PPU_IMAGE_YSIZE << 16));
> + OUT(cmdbuf, 0x444051f0);
> + OUT(cmdbuf, 0xffffffff);
> +
> + CMD_LOAD_STATES_START(cmdbuf, VIVS_SH_HALTI5_UNIFORMS(4), 4);
> + OUT(cmdbuf, buffer_base + output_offset);
> + OUT(cmdbuf, PPU_IMAGE_STRIDE);
> + OUT(cmdbuf, PPU_IMAGE_XSIZE | (PPU_IMAGE_YSIZE << 16));
> + OUT(cmdbuf, 0x444051f0);
> + OUT(cmdbuf, 0xffffffff);
> +
> + CMD_LOAD_STATE(cmdbuf, VIVS_CL_CONFIG,
> + VIVS_CL_CONFIG_DIMENSIONS(2) |
> + VIVS_CL_CONFIG_VALUE_ORDER(3));
> + CMD_LOAD_STATE(cmdbuf, VIVS_VS_ICACHE_INVALIDATE, 0x1f);
> + CMD_LOAD_STATE(cmdbuf, VIVS_PS_VARYING_NUM_COMPONENTS(0), 0);
> + CMD_LOAD_STATE(cmdbuf, VIVS_PS_TEMP_REGISTER_CONTROL,
> + shader_register_count);
> + CMD_LOAD_STATE(cmdbuf, VIVS_PS_SAMPLER_BASE, 0x0);
> + CMD_LOAD_STATE(cmdbuf, VIVS_PS_UNIFORM_BASE, 0x0);
> + CMD_LOAD_STATE(cmdbuf, VIVS_PS_NEWRANGE_LOW, 0x0);
> + CMD_LOAD_STATE(cmdbuf, VIVS_PS_NEWRANGE_HIGH,
> + shader_size / 16);
> + CMD_LOAD_STATE(cmdbuf, VIVS_PS_INST_ADDR,
> + buffer_base + shader_offset);
> + CMD_LOAD_STATE(cmdbuf, VIVS_SH_CONFIG,
> + VIVS_SH_CONFIG_RTNE_ROUNDING);
> + CMD_LOAD_STATE(cmdbuf, VIVS_VS_ICACHE_CONTROL,
> + VIVS_VS_ICACHE_CONTROL_ENABLE);
> + CMD_LOAD_STATE(cmdbuf, VIVS_PS_ICACHE_COUNT,
> + shader_size / 16 - 1);
> + CMD_LOAD_STATE(cmdbuf, VIVS_PS_INPUT_COUNT, 0x1f01);
> + CMD_LOAD_STATE(cmdbuf, VIVS_VS_HALTI5_UNK008A0, 0x0);
> + CMD_LOAD_STATE(cmdbuf, VIVS_PA_VS_OUTPUT_COUNT, 0x0);
> + CMD_LOAD_STATE(cmdbuf, VIVS_GL_VARYING_TOTAL_COMPONENTS, 0x0);
> + CMD_LOAD_STATE(cmdbuf, VIVS_PS_CONTROL_EXT, 0x0);
> + CMD_LOAD_STATE(cmdbuf, VIVS_VS_OUTPUT_COUNT, 0x1);
> + CMD_LOAD_STATE(cmdbuf, VIVS_GL_HALTI5_SH_SPECIALS, 0x0);
> + CMD_LOAD_STATE(cmdbuf, VIVS_PS_ICACHE_PREFETCH, 0x0);
> + CMD_LOAD_STATE(cmdbuf, VIVS_CL_UNK00924, 0x0);
> + CMD_LOAD_STATE(cmdbuf, VIVS_CL_THREAD_ALLOCATION, 0x1);
> +
> + CMD_LOAD_STATE(cmdbuf, VIVS_CL_GLOBAL_WORK_OFFSET_X, 0x0);
> + CMD_LOAD_STATE(cmdbuf, VIVS_CL_GLOBAL_WORK_OFFSET_Y, 0x0);
> + CMD_LOAD_STATE(cmdbuf, VIVS_CL_GLOBAL_WORK_OFFSET_Z, 0x0);
> +
> + CMD_LOAD_STATES_START(cmdbuf, VIVS_CL_WORKGROUP_COUNT_X, 9);
> + OUT(cmdbuf, 0xf);
> + OUT(cmdbuf, 0x5);
> + OUT(cmdbuf, 0xffffffff);
> + OUT(cmdbuf, 0x0);
> + OUT(cmdbuf, 0x0);
> + OUT(cmdbuf, 0x3ff);
> + OUT(cmdbuf, 0x0);
> + OUT(cmdbuf, 0x4);
> + OUT(cmdbuf, 0x1);
> + OUT(cmdbuf, 0x0);
> +
> + CMD_LOAD_STATE(cmdbuf, VIVS_CL_KICKER, 0xbadabeeb);
> + CMD_LOAD_STATE(cmdbuf, VIVS_GL_FLUSH_CACHE,
> + VIVS_GL_FLUSH_CACHE_SHADER_L1 |
> + VIVS_GL_FLUSH_CACHE_UNK10 |
> + VIVS_GL_FLUSH_CACHE_UNK11);
> +}
> +
> +static void
> +etnaviv_flop_reset_ppu_fill_input(u32 *buffer, u32 size)
> +{
> + for (int i = 0; i < size/4; ++i, ++buffer)
> + *buffer = 0x01010101;
Maybe just use memset32(..)?
> +}
> +
> +static void
> +etnaviv_flop_reset_ppu_set_shader(u8 *dest)
> +{
> + const u32 inst[PPU_FLOP_RESET_INSTR_DWORD_COUNT] = {
static const
> + /* img_load.u8 r1, c0, r0.xy */
> + 0x78011779, 0x39000804, 0x00A90050, 0x00000000,
> + /* img_load.u8 r2, c0, r0.xy */
> + 0x78021779, 0x39000804, 0x00A90050, 0x00000000,
> + /* dp2x8 r1, r1, r2, c3_512 */
> + 0xB8017145, 0x390018FC, 0x01C90140, 0x40390028,
> + /* img_store.u8 r1, c2, r0.xy, r1 */
> + 0x380007BA, 0x39001804, 0x00A90050, 0x00390018,
> + };
> + memcpy(dest, inst, sizeof(inst));
> +}
> +
> +static struct etnaviv_flop_reset_entry {
static const
> + u16 chip_model;
> + u16 revision;
> + u32 flags;
> +} etnaviv_flop_reset_db [] = {
> + {
> + .chip_model = 0x8000,
> + .revision = 0x6205,
> + .flags = flop_reset_ppu
> + },
> +};
> +
> +bool
> +etnaviv_flop_reset_ppu_require(const struct etnaviv_chip_identity *chip_id)
> +{
> + const struct etnaviv_flop_reset_entry *e = etnaviv_flop_reset_db;
> +
> + for (int i = 0; i < ARRAY_SIZE(etnaviv_flop_reset_db); ++i, ++e) {
> + if (chip_id->model == e->chip_model &&
> + chip_id->revision == e->revision)
> + return (e->flags & flop_reset_ppu) != 0;
> + }
> +
> + return false;
> +}
> +
> +static const u32 image_data_size = PPU_IMAGE_STRIDE * PPU_IMAGE_YSIZE;
> +static const u32 output_offset = ALIGN(image_data_size, 64);
> +static const u32 shader_offset = ALIGN(output_offset + image_data_size, 64);
> +static const u32 shader_size = PPU_FLOP_RESET_INSTR_DWORD_COUNT * sizeof(u32);
> +static const u32 shader_register_count = 3;
> +static const u32 buffer_size = shader_offset + shader_size;
> +
> +void
I think we need to return an error here - so let's go with an int.
> +etnaviv_flop_reset_ppu_init(struct etnaviv_drm_private *priv)
> +{
> + /* Get some space from the rung buffer to put the payload
> + * (input and output image, and shader), we keep this buffer
> + * for the whole life time the driver is bound
> + */
> + priv->flop_reset_data_ppu =
> + kzalloc(sizeof(*priv->flop_reset_data_ppu), GFP_KERNEL);
> +
Missing NULL check for priv->flop_reset_data_ppu.
> + etnaviv_cmdbuf_init(priv->cmdbuf_suballoc,
> + priv->flop_reset_data_ppu, buffer_size);
> +
etnaviv_cmdbuf_init can fail too.
> + void *buffer_base = priv->flop_reset_data_ppu->vaddr;
> +
> + u32 *input_data = (u32 *)buffer_base;
> + etnaviv_flop_reset_ppu_fill_input(input_data, image_data_size);
> +
> + u8 *shader_data = (u8 *)buffer_base + shader_offset;
> + etnaviv_flop_reset_ppu_set_shader(shader_data);
> +}
> +
> +void
> +etnaviv_flop_reset_ppu_run(struct etnaviv_gpu *gpu)
> +{
> + struct etnaviv_drm_private *priv = gpu->drm->dev_private;
> +
> + if (!priv->flop_reset_data_ppu) {
> + pr_err("Flop reset data was not initialized, skipping\n");
dev_err(gpu->dev, "Flop reset data was not initialized, skipping\n");
> + return;
> + }
How can this happen?
> +
> + u32 buffer_base = etnaviv_cmdbuf_get_va(priv->flop_reset_data_ppu,
> + &gpu->mmu_context->cmdbuf_mapping);
> +
> + etnaviv_emit_flop_reset_state_ppu(&gpu->buffer,
> + buffer_base,
> + 0,
> + output_offset,
> + shader_offset,
> + shader_size,
> + shader_register_count);
> +}
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_flop_reset.h b/drivers/gpu/drm/etnaviv/etnaviv_flop_reset.h
> new file mode 100644
> index 000000000000..f51cece75507
> --- /dev/null
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_flop_reset.h
> @@ -0,0 +1,25 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2025 Etnaviv Project
> + */
> +
> +
> +#ifndef etnaviv_flop_reset_h
> +#define etnaviv_flop_reset_h
To keep the style consistent: __ETNAVIV_FLOP_RESET_H__
> +
> +#include <linux/types.h>
> +
> +struct etnaviv_chip_identity;
> +struct etnaviv_drm_private;
> +struct etnaviv_gpu;
> +
> +bool
> +etnaviv_flop_reset_ppu_require(const struct etnaviv_chip_identity *chip_id);
> +
> +void
> +etnaviv_flop_reset_ppu_init(struct etnaviv_drm_private *priv);
> +
> +void
> +etnaviv_flop_reset_ppu_run(struct etnaviv_gpu *gpu);
> +
> +#endif
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index cf0d9049bcf1..0aac01c1021c 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -18,6 +18,7 @@
>
> #include "etnaviv_cmdbuf.h"
> #include "etnaviv_dump.h"
> +#include "etnaviv_flop_reset.h"
> #include "etnaviv_gpu.h"
> #include "etnaviv_gem.h"
> #include "etnaviv_mmu.h"
> @@ -837,6 +838,10 @@ int etnaviv_gpu_init(struct etnaviv_gpu *gpu)
> goto fail;
> }
>
> + if (etnaviv_flop_reset_ppu_require(&gpu->identity) &&
> + !priv->flop_reset_data_ppu)
> + etnaviv_flop_reset_ppu_init(priv);
> +
> if (gpu->identity.nn_core_count > 0)
> dev_warn(gpu->dev, "etnaviv has been instantiated on a NPU, "
> "for which the UAPI is still experimental\n");
> --
> 2.49.0
>
I am not sure about the overall code style - you did run checkpatch.pl ?
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info/privacypolicy
Powered by blists - more mailing lists