[Lima] [PATCH v2 6/8] gallium: add lima driver
Alyssa Rosenzweig
alyssa at rosenzweig.io
Tue Mar 26 03:33:44 UTC 2019
> + [PPIR_INSTR_SLOT_ALU_VEC_MUL] = ppir_codegen_encode_vec_mul,
> + [PPIR_INSTR_SLOT_ALU_SCL_MUL] = ppir_codegen_encode_scl_mul,
> + [PPIR_INSTR_SLOT_ALU_VEC_ADD] = ppir_codegen_encode_vec_add,
> + [PPIR_INSTR_SLOT_ALU_SCL_ADD] = ppir_codegen_encode_scl_add,
vmul/smul/vadd/sadd are the (official?) names for this on Midgard, fwiw.
> +static const int ppir_codegen_field_size[] = {
> + 34, 62, 41, 43, 30, 44, 31, 30, 41, 73
> +};
What is this?
> +static inline int align_to_word(int size)
> +{
> + return ((size + 0x1f) >> 5);
> +}
Maybe use the align() macro (it's in mesa/src/util/) for clarity?
> + ppir_codegen_combine_scalar_op_atan = 8, /* Arc Tangent Part 1 */
> + ppir_codegen_combine_scalar_op_atan2 = 9, /* Arc Tangent 2 Part 1 */
Any plan to use these ops? Midgard has them too, but inverse trig gets
lowered away anyway...
> + if (node->op == ppir_op_neg)
> + src->negate = !src->negate;
> + else if (node->op == ppir_op_abs)
> + src->absolute = true;
I'm confused. NIR has neg/abs modifiers natively. You can get all
fneg/fabs instructions lowered in NIR to these modifiers via
the lower_source_mods pass. If you don't want them, you'll never have
neg/abs ops at all; they'll all be modifiers. Why duplicate this in
ppir?
> + uint32_t va;
I don't know the Utgard MMU, but will this work on 64-bit?
> + uint32_t dubya;
???
> + uint32_t mrt_bits;
> + uint32_t mrt_pitch;
Utgard supports MRT?!
> + uint32_t width = fui(fb->width);
> + uint32_t height = fui(fb->height);
> + uint32_t reload_gl_pos[] = {
> + width, 0x00000000, 0x00000000, 0x3f800000, /* 0x00000000 */
> + 0x00000000, 0x00000000, 0x00000000, 0x3f800000, /* 0x00000010 */
> + 0x00000000, height, 0x00000000, 0x3f800000, /* 0x00000020 */
> + };
> + memcpy(cpu + lima_reload_gl_pos_offset, reload_gl_pos,
> + sizeof(reload_gl_pos));
> +
> + uint32_t reload_varying[] = {
> + width, 0x00000000, 0x00000000, 0x00000000, /* 0x00000000 */
> + 0x00000000, height, 0x00000000, 0x00000000, /* 0x00000010 */
> + };
> + memcpy(cpu + lima_reload_varying_offset, reload_varying,
> + sizeof(reload_varying));
> +
Why not actual float arrays?
> + uint32_t clear_shader[] = {
> + 0x00021025, 0x0000000c,
> + (ctx->clear.color_16pc << 12) | 0x000007cf,
> + ctx->clear.color_16pc >> 12,
> + ctx->clear.color_16pc >> 44,
> + };
Please don't include shader blobs. This should maybe be generated
from NIR, but at minimum, please include the corresponding annotated
assembly.
> + clear->color_16pc =
> + ((uint64_t)float_to_ushort(color->f[3]) << 48) |
> + ((uint64_t)float_to_ushort(color->f[2]) << 32) |
> + ((uint64_t)float_to_ushort(color->f[1]) << 16) |
> + float_to_ubyte(color->f[0]);
Should that last line be float_to_ushort?
> +enum lima_attrib_type {
> + LIMA_ATTRIB_FLOAT = 0x000,
> + /* todo: find out what lives here. */
LIMA_ATTRIB_FP16 maybe.
LIMA_ATTRIB_I32/LIMA_ATTRIB_U32?.
> +static bool
> +is_modifier_ok(const uint64_t *modifiers, int count, uint64_t mod)
drm_find_modifier in util/u_drm.h
> + /* fs program for clear buffer? */
> + static const uint32_t pp_clear_program[] = {
> + 0x00020425, 0x0000000c, 0x01e007cf, 0xb0000000, /* 0x00000000 */
> + 0x000005f5, 0x00000000, 0x00000000, 0x00000000, /* 0x00000010 */
> + };
> + memcpy(lima_bo_map(screen->pp_buffer) + pp_clear_program_offset,
> + pp_clear_program, sizeof(pp_clear_program));
> +
> + /* copy texture to framebuffer, used to reload gpu tile buffer */
> + static const uint32_t pp_reload_program[] = {
> + 0x000005e6, 0xf1003c20, 0x00000000, 0x39001000, /* 0x00000000 */
> + 0x00000e4e, 0x000007cf, 0x00000000, 0x00000000, /* 0x00000010 */
> + };
> + memcpy(lima_bo_map(screen->pp_buffer) + pp_reload_program_offset,
> + pp_reload_program, sizeof(pp_reload_program));
> +
> + /* 0/1/2 vertex index for reload/clear draw */
> + static const uint32_t pp_shared_index[] = {
> + 0x00020100, 0x00000000, 0x00000000, 0x00000000, /* 0x00000000 */
> + };
> + memcpy(lima_bo_map(screen->pp_buffer) + pp_shared_index_offset,
> + pp_shared_index, sizeof(pp_shared_index));
> +
> + /* 4096x4096 gl pos used for partial clear */
> + static const uint32_t pp_clear_gl_pos[] = {
> + 0x45800000, 0x00000000, 0x3f800000, 0x3f800000, /* 0x00000000 */
> + 0x00000000, 0x00000000, 0x3f800000, 0x3f800000, /* 0x00000010 */
> + 0x00000000, 0x45800000, 0x3f800000, 0x3f800000, /* 0x00000020 */
> + };
> + memcpy(lima_bo_map(screen->pp_buffer) + pp_clear_gl_pos_offset,
> + pp_clear_gl_pos, sizeof(pp_clear_gl_pos));
Again, please no shader blobs.
> +
> + /* reverse calculate the parameter of glViewport */
> + ctx->viewport.x = viewport->translate[0] - viewport->scale[0];
> + ctx->viewport.y = fabsf(viewport->translate[1] - fabsf(viewport->scale[1]));
> + ctx->viewport.width = viewport->scale[0] * 2;
> + ctx->viewport.height = fabsf(viewport->scale[1] * 2);
Careful about flipped-y viewports...?
> +++ b/src/gallium/drivers/lima/lima_tiling.c
We'll maybe want to share this routine somewhere... Also, what's your
plan for autogenerating mipmaps?
More information about the lima
mailing list