| Message ID | 20210217080306.157876-1-benjamin.gaignard@collabora.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add HANTRO G2/HEVC decoder support for IMX8MQ | expand |
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? thanks, greg k-h
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 ! Benjamin > thanks, > > greg k-h >
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. So what is preventing the existing driver from getting out of staging now? 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? Anyway, the media staging drivers are on their own, I don't touch them, it just feels odd to me... thanks, greg k-h
Hi Benjamin, On Wed 17 Feb 21, 09:28, 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 ! Thanks for working on Hantro G2 support! As the initial author of the HEVC API, it definitely does not feel ready to be destaged. The API has lots of issues and limitation and I think significant work is still needed. This initial implementation was kind of a proof of concept for simple cases. Cheers, Paul
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. Regards, Hans PS: stateful vs stateless decoders: stateful decoders are fully supported today: you just feed the decoder the compressed stream and the decoded frames are produced by the firmware/hardware. I.e. the HW takes care of the decoder state. Stateless decoders require you to pass the compressed frame + decoder state to the hardware, so they do not keep track of the decoder state, that needs to be done in software. And that requires structures to be created that store the state, which luckily can be derived from the codec standards.
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