Re: [PATCH v1 00/18] Add HANTRO G2/HEVC decoder support for IMX8MQ

[Greg KH]

On Wed, Feb 17, 2021 at 10:10:35AM +0100, Hans Verkuil wrote:
> On 17/02/2021 09:36, Greg KH wrote:
> > On Wed, Feb 17, 2021 at 09:28:09AM +0100, Benjamin Gaignard wrote:
> >>
> >> Le 17/02/2021 à 09:08, Greg KH a écrit :
> >>> On Wed, Feb 17, 2021 at 09:02:48AM +0100, Benjamin Gaignard wrote:
> >>>> The IMX8MQ got two VPUs but until now only G1 has been enabled.
> >>>> This series aim to add the second VPU (aka G2) and provide basic
> >>>> HEVC decoding support.
> >>> Why are you adding this directly to drivers/staging/media/ and not
> >>> drivers/media/?  Why can't this just go to the main location and not
> >>> live in staging?
> >>
> >> G2/HEVC is added inside the already exiting Hantro driver, it is "just"
> >> an other codec from Hantro driver point of view.
> >> In addition of that v4l2-hevc uAPI is still unstable.
> >> One goal of this series is to have one more consumer of this v4l2-hevc
> >> uAPI so maybe we can claim it to be stable enough to move away from staging
> >> and then do the same for Hantro driver. That would be a great achievement !
> > 
> > I know I do not like seeing new additions/features/whatever being added
> > to staging drivers as that encourages people to do new stuff on them
> > without doing the real work needed to get them out of staging.
> 
> In order to support a specific codec (MPEG-2, H.264, HEVC, VP8, etc.) for
> stateless codec hardware like the hantro, V4L2 controls need to be defined.
> The contents of these controls is derived directly from the underlying codec
> standards, but it is quite difficult to get this right with the first attempt,
> since these standards are very complex.
> 
> So we went for the strategy of keeping these drivers in staging to make it
> easy to work on, while keeping the APIs for each codec private (i.e., they are
> not exposed in include/uapi/linux).
> 
> Once we have sufficient confidence in the API for a specific codec we move
> it to uapi and thus fix the API. We also renumber the control IDs at that
> time to avoid any confusion between the staging version and the final version.
> 
> We did that for H.264 and I hope we can soon do the same for MPEG-2 and VP8.
> 
> HEVC is definitely not ready for that yet.
> 
> The key phrase is 'sufficient confidence': one requirement is that it is supported
> by at least two drivers to be reasonably certain the API doesn't contain any HW
> specific stuff, and it passes test suites and review by codec experts.
> 
> All this is actively being worked on, so this is very much alive, but it is
> complex and time consuming.
> 
> > So what is preventing the existing driver from getting out of staging
> > now?
> 
> Once MPEG-2 and VP8 are finalized it is probably time to move these drivers
> out of staging, while still keeping the HEVC API part private.
> 
> > 
> > And how are you all creating new userspace apis for staging drivers to
> > the v4l layer?  What happens when you export something new and then
> > userspace starts to rely on it and then you change it?
> 
> Nothing is exported. So if userspace want to use it they have to manually
> copy headers from include/media to their application.
> 
> > 
> > Anyway, the media staging drivers are on their own, I don't touch them,
> > it just feels odd to me...
> 
> It's an unusual situation. But putting the drivers in staging and keeping
> the codec API headers private turns out to be the most effective way to
> develop this.

Ah, ok, thanks for the explaination, makes sense.

good luck!

greg k-h