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@xxxxxxxxxxx> 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.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
