Re: [PATCH 0/4] Enable Hantro G1 post-processor

[Ezequiel Garcia <ezequiel@collabora.com>]

On Thu, 2019-09-12 at 14:52 +0900, Tomasz Figa wrote:
> On Thu, Sep 12, 2019 at 4:49 AM Nicolas Dufresne <nicolas@ndufresne.ca> wrote:
> > Le mercredi 11 septembre 2019 à 09:27 +0100, Ezequiel Garcia a écrit :
> > > On Mon, 2019-09-09 at 16:07 +0900, Tomasz Figa wrote:
> > > > Hi Ezequiel,
> > > > 
> > > > On Wed, Sep 4, 2019 at 3:17 AM Ezequiel Garcia <ezequiel@collabora.com> wrote:
> > > > > Hi all,
> > > > > 
> > > > > This series enables the post-processor support available
> > > > > on the Hantro G1 VPU. The post-processor block can be
> > > > > pipelined with the decoder hardware, allowing to perform
> > > > > operations such as color conversion, scaling, rotation,
> > > > > cropping, among others.
> > > > > 
> > > > > The decoder hardware needs its own set of NV12 buffers
> > > > > (the native decoder format), and the post-processor is the
> > > > > owner of the CAPTURE buffers. This allows the application
> > > > > get processed (scaled, converted, etc) buffers, completely
> > > > > transparently.
> > > > > 
> > > > > This feature is implemented by exposing other CAPTURE pixel
> > > > > formats to the application (ENUM_FMT). When the application
> > > > > sets a pixel format other than NV12, the driver will enable
> > > > > and use the post-processor transparently.
> > > > 
> > > > I'll try to review the series a bit later, but a general comment here
> > > > is that the userspace wouldn't have a way to distinguish between the
> > > > native and post-processed formats. I'm pretty much sure that
> > > > post-processing at least imposes some power penalty, so it would be
> > > > good if the userspace could avoid it if unnecessary.
> > > > 
> > > 
> > > Hm, that's true, good catch.
> > > 
> > > So, it would be desirable to retain the current behavior of allowing
> > > the application to just set a different pixel format and get
> > > a post-processed frame, transparently.
> > > 
> > > But at the same time, it would be nice if the application is somehow
> > > aware of the post-processing happening. Maybe we can expose a more
> > > accurate media controller topology, have applications enable
> > > the post-processing pipeline explicitly.
> > 
> > How it works on the stateful side is that userspace set the encoding
> > type (the codec), then passes a header (in our case, there will be
> > parsed structures replacing this) first. The driver then configure
> > capture format, giving a hint of the "default" or "native" format. This
> > may or may not be sufficient, but it does work in giving userspace a
> > hint.
> 
> The bad side of that is that we can't handle more than 1 native format.
> 
> For the most backwards-compatible behavior, sorting the results of
> ENUM_FMT according to format preference would allow the applications
> that choose the first format returned that works for them to choose
> the best one.
> 
> For a further improvement, an ENUM_FMT flag that tells the userspace
> that a format is preferred could work.
> 
> That said, modelling the pipeline appropriately using the media
> controller is the idea I like the most, because it's the most
> comprehensive solution. That would have to be well specified and
> documented, though, and sounds like a long term effort.
> 

Completely agreed.

Thanks,
Ezequiel