Re: [GIT PULL] exynos-drm-next

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Nov 28, 2017 at 02:45:49PM +0100, Marek Szyprowski wrote:
> Hi Daniel,
> 
> On 2017-11-20 08:33, Daniel Vetter wrote:
> > On Wed, Nov 15, 2017 at 10:26:45AM +0900, Inki Dae wrote:
> > > 2017년 11월 14일 13:22에 Dave Airlie 이(가) 쓴 글:
> > > > On 26 October 2017 at 11:37, Inki Dae <inki.dae@samsung.com> wrote:
> > > > >     Improving Exynos DRM HDMI and Mixer drivers and also adding
> > > > >     HDMI audio support.
> > > > > 
> > > > >     Please kindly let me know if there is any problem.
> > > > > 
> > > > >     Ps. we are reviewing IPP v2 driver[1] which controls post processor devices
> > > > >         such as FIMC, GScaler and Rotator of Exynos SoC. So I plan to request
> > > > >         git pull one more time after review.
> > > > I dropped the ball a bit here, I do think the second pull with IPP is
> > > > probably a bit late,
> > > > I think I'd like more assurance that the UAPI for IPP is to be used in
> > > > some upstream
> > > > shipping project (forks of mpv not withstanding, unless this fork
> > > > ships in some distro
> > > > or product).
> > > I also commented same thing internally to author - Marek - of IPP v2 but I thought existing one is really ugly thing so may be better to change it as soon as possible before other users use this one.
> > > Anyway, we will merge user space for IPP v2 to libdrm first, and then Linux platform. I hope IPPv2 would be merged as soon as possible in next turn.
> > Beyond what Daniel said we unfortunately don't have time machines, so
> > fixing bad uapi isn't really possible. The problem is that even if you
> > create ippv2, then people can still use ippv1, and it needs to keep
> > working (almost) forever. So once a bad uapi is in, it's in, and there's
> > no good reason to add more uapi (since in 2 years you might again realize
> > it's not a good idea) to "fix" that. You can't fix bad uapi.
> > 
> > That's why we have all these rules to make sure as little bad uapi as
> > possible lands, since the cost is so bad.
> > 
> > So yeah unless you have new hw that needs it, or there's another clear
> > need (performance, features), you're stuck with ippv1. "It's cleaner" is
> > not a good reason to rev uapi, since our experience with all the uapi in
> > drm clearly shows we can't predict the future :-)
> 
> I generally agree that UAPI interface has to be stable and well designed.
> 
> There are however some 'features' IPPv1 uapi that really allows us to
> obsolete it:
> 
> 1. IPP API can be optional in Exynos DRM, so userspace should not rely that
> it is always available and should have a software fallback in case it is not
> available.
> 
> 2. The only mode which was initially semi-working was memory-to-memory. The
> remaining modes (lcd-writeback and output) were never operational (both in
> mainline and even vendor kernels).
> 
> 3. IPP mainline uapi compatibility for memory-to-memory got broken very
> early by commit 083500baefd5f4c215a5a93aef2492c1aa775828 ("drm: remove
> DRM_FORMAT_NV12MT", which removed the support for tiled formats, the main
> feature which made this api somehow useful on Exynos platforms (video codec
> that time produced only tiled frames, to implement xvideo or any other
> video overlay, one has to detile them for proper display).
> 
> 4. Broken drivers. Especially once support for IOMMU has been added, it
> revealed that drivers don't configure DMA operations properly and in many
> cases operate outside the provided buffers trashing memory around.
> 
> 5. Need for external patches. Although IPP uapi has been used in some
> vendor kernels, but in such cases there were additional patches applied
> (like reverting mentioned 083500baefd5 patch) what means that those
> userspace apps which might use it, still won't work with the mainline
> version.
> 
> Right, as you already said we don't have time machines, so we cannot change
> it, but IPP extension should never have been merged to mainline in that
> form.
> 
> We can however obsolete it now as it is really non-functional and frankly
> speaking dead-code. If you agree with the above than at least the first
> patch can be merged, which clearly marks that Exynos IPP is broken and
> ever really functional. I bet no one will complain. Then we can continue
> the discussion about IPPv2 uapi/patches.

Ok, if what's currently is upstream is indeed entirely non-functional then
I think we can make a case for ippv2. But imo we should then also first
rip out ippv1. Which given that it's optional, and apparently your
userspace can cope, should be doable without causing any big trouble.

But now that you explained why ippv1 is a hopeless patient that won't make
it, why is ippv2 better? Do we have solid testcases for this stuff now? Is
the userspace production quality or still kinda just a tech demo?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel