Hi Matthijs, [reduced list audience since this is not (yet) an LKML topic].
Am 23.04.2017 um 01:52 schrieb Matthijs van Duin matthijsvanduin@gmail.com:
On 23 April 2017 at 01:35, Matthijs van Duin matthijsvanduin@gmail.com wrote:
On 21 April 2017 at 09:46, H. Nikolaus Schaller hns@goldelico.com wrote:
Unfortunately without any explanation why this is a warning or needed or a problem (and I must admit that I don't understand the code at all because I haven't spent time for it). So it may simple be something that does not need a warning at all.
I'm not immediately sure why I added it, I'd need to dig into it.
It looks like the reasoning is something along the lines of: the patch forces buffers to be pinned at allocation time, hence omap_gem_{get,put}_paddr should only take/drop *additional* references and never let the refcount hit zero, which instead happens in free.
and this is triggered on OMAP3 by some ioctls from Xorg because we also skip TILER setup.
The test is still wrong, even with later patches forcing the fbdev framebuffer being tiled, since userspace could still allocate untiled buffers.
Anyway, this commit lacks a Signed-off and is even marked with "XXX" in the commit summary, I'd appreciate it if this not-so-subtle hint that is it not to be merged into any general-purpose tree is honored in the future, so I don't get emails in my inbox from people complaining about breakage it causes on devices I never even considered for a second while writing the patch.
I see.
To be self-critical: I also don't ever see all side-effects of my work (you know the Pyra hardware issues...). Nobody can. This is why peer-review was invented.
But having to decide between "public+signed-off" and "public+not-signed" (the latter meaning: DON'T USE IT") makes things more complicated than they need to be... It essentially means that there is of today no kernel with working+signed-off TILER support.
This would mean that ED has to reinstall the SSD chip on the display board. Since this is the only working hardware setup he can sell where complete driver software has all the required signed-offs (even if it is also buggy).
So I wonder if it wouldn't be a better strategy to help to fix the issues rather than telling us not to use it? You are the only one who has deep enough understanding how the TILER works. So we can only identify issues and make attempts to fix them (like the OMAP3 fix by Andreas).
And not to forget: more widespread use and testing potentially shows more problems (like missing OMPA3 compatibility).
If we ever want to get this upstream (which I would like to see earlier than later because it is a so fundamental feature for all OMPA5 devices) it has to be tested in different scenarios and someone has to fix bugs anyways.
BR, Nikolaus
On 23 April 2017 at 11:17, H. Nikolaus Schaller hns@goldelico.com wrote:
Hi Matthijs, [reduced list audience since this is not (yet) an LKML topic].
Am 23.04.2017 um 01:52 schrieb Matthijs van Duin matthijsvanduin@gmail.com:
It looks like the reasoning is something along the lines of: the patch forces buffers to be pinned at allocation time, hence omap_gem_{get,put}_paddr should only take/drop *additional* references and never let the refcount hit zero, which instead happens in free.
and this is triggered on OMAP3 by some ioctls from Xorg because we also skip TILER setup.
Yes, because when the first round of obvious breakage due to my patches came around, you ignored my plea to *not* *merge* *them* and instead patched on top of it, which now caused further breakage.
To be self-critical: I also don't ever see all side-effects of my work (you know the Pyra hardware issues...). Nobody can. This is why peer-review was invented.
Except I never asked for "peer-review" since I already stated very clearly and unambiguously that these patches were incredibly hacky and unsuitable for merging. This was never an oversight. The patches had very specific goals: to confirm that the TILER slowness on omap5 was largely due to software (and fixable in software) rather than due to the hardware itself, and to get a demo working. It succeeded in these goals.
Since then this work hasn't received much attention anymore since hw issues had higher priority. Also, as I've said previously, I'd regard fixing the dma-buf api for tiled buffers to be the highest priority task w.r.t. tiler-related kernel stuff currently, and I think that should also yield some of the tools necessary to properly deal with a tiled fbdev.
I don't even really know why I'm trying to explain this again since I'm pretty sure I've already said all this before...