Re: [PATCH 6/7] misc: bcm-vk: add Broadcom Valkyrie driver

Re: [PATCH 6/7] misc: bcm-vk: add Broadcom Valkyrie driver

[Arnd Bergmann]

On Thu, Aug 22, 2019 at 9:25 PM Scott Branden
<scott.branden@broadcom.com> wrote:
>
> Add Broadcom Valkyrie driver offload engine.
> This driver interfaces to the Valkyrie PCIe offload engine to perform
> should offload functions as video transcoding on multiple streams
> in parallel.  Valkyrie device is booted from files loaded using
> request_firmware_into_buf mechanism.  After booted card status is updated
> and messages can then be sent to the card.
> Such messages contain scatter gather list of addresses
> to pull data from the host to perform operations on.
>
> Signed-off-by: Scott Branden <scott.branden@broadcom.com>
> Signed-off-by: Desmond Yan <desmond.yan@broadcom.com>
> Signed-off-by: James Hu <james.hu@broadcom.com>

Can you explain the decision to make this is a standalone misc driver
rather than hooking into the existing framework in drivers/media?

There is an existing interface that looks like it could fit the hardware
in include/media/v4l2-mem2mem.h. Have you considered using that?

There is also support for video transcoding using GPUs in
driver/gpu/drm/, that could also be used in theory, though it sounds
like a less optimal fit.

      Arnd

Re: [PATCH 6/7] misc: bcm-vk: add Broadcom Valkyrie driver

[Nicolas Dufresne]

[-- Attachment #1: Type: text/plain, Size: 2164 bytes --]

Le mardi 27 août 2019 à 16:14 +0200, Arnd Bergmann a écrit :
> On Thu, Aug 22, 2019 at 9:25 PM Scott Branden
> <scott.branden@broadcom.com> wrote:
> > Add Broadcom Valkyrie driver offload engine.
> > This driver interfaces to the Valkyrie PCIe offload engine to perform
> > should offload functions as video transcoding on multiple streams
> > in parallel.  Valkyrie device is booted from files loaded using
> > request_firmware_into_buf mechanism.  After booted card status is updated
> > and messages can then be sent to the card.
> > Such messages contain scatter gather list of addresses
> > to pull data from the host to perform operations on.
> > 
> > Signed-off-by: Scott Branden <scott.branden@broadcom.com>
> > Signed-off-by: Desmond Yan <desmond.yan@broadcom.com>
> > Signed-off-by: James Hu <james.hu@broadcom.com>
> 
> Can you explain the decision to make this is a standalone misc driver
> rather than hooking into the existing framework in drivers/media?
> 
> There is an existing interface that looks like it could fit the hardware
> in include/media/v4l2-mem2mem.h. Have you considered using that?
> 
> There is also support for video transcoding using GPUs in
> driver/gpu/drm/, that could also be used in theory, though it sounds
> like a less optimal fit.

I believe that a major obstacle with this driver is usability. Even
though I have read through, I believe it's just impossible for anyone
to actually write Open Source userspace for it. The commit message does
not even try to help in this regard.

Note that depending on the feature your transcoder has, there is also
the option to model it around the media controller. That is notably
useful for certain transcoders that will also do scaling and produce
multiple streams (for adaptive streaming usecases were you want to
share a single decoder).

An 1 to 1 transcoder modeled around m2m would eventually required
documentation so that other transcoder can be implemented in a way that
they would share the same userspace. This is currently being worked on
for m2m encoder and decoders (including state-less variants).

regards,
Nicolas



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 195 bytes --]