On Tue, Mar 26, 2019 at 11:33 AM Alyssa Rosenzweig <aly...@rosenzweig.io> wrote:
>
> > + [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?
PP instruction field (i.e. vadd part length) length in bit.
>
> > +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...
>
Not yet.
> > + 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?
Didn't know this pass, I can use it, thanks.
>
> > + uint32_t va;
>
> I don't know the Utgard MMU, but will this work on 64-bit?
>
It's fine as Utgard MMU only support 32bit address.
>
> > + uint32_t dubya;
>
> ???
Don't know for me either, just copy from lima-ng.
>
> > + uint32_t mrt_bits;
> > + uint32_t mrt_pitch;
>
> Utgard supports MRT?!
>
Utgard can render to 3 FB (WB0~2) in one pass.
>
> > + 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?
>
Right.
>
> > + 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.
>
I think no need to compile this kind of util shader, will add some
comments about it.
> > + 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?
>
Right, will fix it.
>
> > +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...?
>
Right.
> > +++ 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?
Arno did the mipmap implementation, so far no further work from him.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
