Re: Re: Re: Re: [PATCH v7 11/11] media: hantro: Support NV12 on the G2 core

quoted

Dne sreda, 20. oktober 2021 ob 17:25:40 CEST je Ezequiel Garcia napisal(a):
On Wed, 20 Oct 2021 at 12:04, Jernej Škrabec [off-list ref] 
wrote:
Dne sreda, 20. oktober 2021 ob 13:06:59 CEST je Ezequiel Garcia 
napisal(a):
Hi Jernej,

On Tue, 19 Oct 2021 at 13:38, Jernej Škrabec [off-list ref]
wrote:
Hi Andrzej!

Dne petek, 15. oktober 2021 ob 19:19:47 CEST je Andrzej Pietrasiewicz
napisal(a):
Hi Jernej,

W dniu 14.10.2021 o 19:42, Jernej Škrabec pisze:
Hi Andrzej!

Dne sreda, 29. september 2021 ob 18:04:39 CEST je Andrzej
Pietrasiewicz
napisal(a):
The G2 decoder block produces NV12 4x4 tiled format (NV12_4L4).
Enable the G2 post-processor block, in order to produce regular 
NV12.
The logic in hantro_postproc.c is leveraged to take care of
allocating
the extra buffers and configure the post-processor, which is
significantly simpler than the one on the G1.
Quick summary of discussion on LibreELEC Slack:
When using NV12 format on Allwinner H6 variant of G2 (needs some
driver
changes), I get frames out of order. If I use native NV12 tiled
format,
frames
are ordered correctly.

Currently I'm not sure if this is issue with my changes or is this
general
issue.

I would be grateful if anyone can test frame order with and 
without
postprocessing enabled on imx8. Take some dynamic video with a lot 
of
short
scenes. It's pretty obvious when frames are out of order.
I checked on imx8 and cannot observe any such artifacts.
I finally found the issue. As you mentioned on Slack, register write 
order
once
already affected decoding. Well, it's the case again. I made hacky test 
and
moved postproc enable call after output buffers are set and it worked. 
So,
this
is actually core quirk which is obviously fixed in newer variants.
Ugh, good catch.

What happens if you move all the calls to HANTRO_PP_REG_WRITE_S
(HANTRO_PP_REG_WRITE does a relaxed write)?

Or what happens if the HANTRO_PP_REG_WRITE(vpu, out_luma_base, dst_dma)
is moved to be done after all the other registers?
Those two macros aren't used on G2. Andrzej introduced new postproc 
helpers
for G2.
Ah, so the issue is specific on the G2 post-processor.
To be more precise, issue is specific only to old G2 post-processor, found in 
Allwinner H6. Andrzej tested code with newer G2 core and both locations worked 
fine.

This commit solves issue for H6:
https://github.com/jernejsk/linux-1/commit/
a783a977c0843bb4b555dc9d0b5d64915cd219e7
Right, but see this comment:

    /* Turn on pipeline mode. Must be done first. */
    HANTRO_PP_REG_WRITE_S(vpu, pipeline_en, 0x1);

I have vague recollection of why we have that comment,
but I'm reluctant to move post-proc enable to the end.
(or at least not do it on G1?).
I missed that. Any idea what would be the cleanest way to move code for G2 
only? I can only think of quirk flag in platform specific structure.

Best regards,
Jernej

This makes this series with minor adaptations completely working on 
H6. I
see
no reason not to merge whole series.
Do you have plans to submit your H6 work on top of this?
Of course, why would I work on this otherwise? :) But before I do that, I 
have
to clean up and split one commit, which adapts VP9 G2 code for H6 variant.
OK, sounds good.

If you're interested in changes, take a look here:
https://github.com/jernejsk/linux-1/commits/vp9
Will take a look.

Thanks,
Ezequiel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help