Re: [Patch 0/2] [VIA UniChrome DRM] Patch system hang issue caused by 3D scaling+ACPI

[Dave A. <ai...@li...>]

> Are you saying "Yes, it is right to carry version information in the 
> drm.h file"?

No I'm still in no way convinced of this, the fact Thomas doesn't see it
as a requirement either, and *no* other drm driver does it, is all
pointing towards its unnecessary. You seem to think its obvious but we've
survived with radeon and intel and mga and multiple other drivers never
having this requirement, hence why I am asking for clarification.

> And yes, the usage of major, minor and patchlevel is clear, and didn't 
> really need repeating.

It didn't seem to be, you seem to want to have a fixed minor/patch level
support in the DDX which isn't the normal definition of minor/patch. So
clarification was required.

> No, you didn't ship it, yet, and exactly to stop this from shipping the 
> major issues i have with these patches were pointed out.

And thats how the process should work, people review the patches and 
crappy breaks don't get in, not we shove them in, and hope the DDX has
graceful abilities to fall over in a heap.

> But why not have all three numbers _in_ _the_ _same_ _place_. Disperse 
> them, and sooner or later someone tries to be smart and then breaks the 
> versioning scheme.

So you haven't answered the point I raised, the API version isn't only
tied to this file, other internal things like the verifier also influcence
the API, how do you detect these sort of things at build time?

> Userspace is free to try to be compatible as much as it wants then, or 
> ignore as much as it can, or it can just stupidly break down and cry 
> loudly when just the patchlevel gets bumped. At least you get good 
> feedback.

I don't think userspace should be given that option, userspace should be
required to be sane, it should work with any kernel with the same major
version and possibly a minor bump, I wouldn't want distros shipping a user
space that was uniquely tied to a shipping kernel api minor/patch version, 
that is crazy and makes the whole point of having a major/minor/patch 
version stupid. So maybe adding this version number allows for this 
use-case but its not something we should be making any attempt to support.

> Depends on the definition of gracefully. I fully agree that major bumps 
> should be avoided at all cost, but they are sometimes impossible to 
> avoid. And then we can try to deal with it as gracefully as possible.

You cannot just bump major in the kernel version, its easier to 
publish either a while new driver with a new name, *or* have a kernel
option to expose old or new versions, with the new version never
getting enabled by default for something around 5-6 years.

> For unichrome, the dma stuff suddenly lost bouncebuffer (iirc this was 
> once used for pitch conversion) support and the respective structure 
> members. Now in autotool-world, you can check headers for that easily, 
> in Imake world, things were not that easy.
> 
> One can easily make things buildtime compatible, by providing version 
> numbers in the header file, and by bumping the major number. Runtime 
> compatibility can then be checked easily by using the major define 
> directly.
> 
> I still do not see why you are against this. It is not exactly 
> rocketscience, it solves some of the problems we were facing, and it 
> works.
> 
> To be able to deal with things, the version numbering was introduced. 
> API compatibility would have had a better chance of surviving if the 
> versioning was also put in the kernel tree. This whole situation clearly 
> reeks of a broken process. Have you recently diffed the mesa/drm and the 
> kernel/drm and seen what the differences are, or are you just grabbing 
> patches from one and putting them in the other, letting both trees grow 
> apart over time?

> > Okay so if that is these patches, then that is an issue, Novell screwed up 
> > by not upstreaming this first, but thats not uncommon.
> 
> This were VIA patches sent to the dri-devel ml which got ignored 
> completely. Gregkh used these patches that never really became 
> upstreamed, and then failed to even warn the SUSE X developers about it 
> (at least, i missed it at the time it was included, and i was still 
> there, not 100% sure about the others, even though i am sure that i 
> would've been told this immediately).

We didn't ignore any via patches to the mailing list, we reviewed nearly 
every submission of code and found most of it unsuitable for upstreaming
as it was.

> Novell did not have to upstream itself, so please stop suggesting that 
> this was Novell doing stuff behind closed doors.

If Greg did this as part of staging I also objected to this on lkml at
one time.
> 
> Surely redhat never used/shipped code that wasn't fully upstream yet.

Not when the upstream maintainer said there was now way it would ever ship 
in the current form.

> > You can't gracefuly handle major bumps, unless you count disabling DRI on 
> > someones working system acceptable which we don't anymore. The only way I 
> > can think to do a major number rest is with a Kconfig option that distros 
> > can enable for that driver, sort of like what we did for KMS.
> 
> It is better for a driver to throw up its hands and state clearly what 
> is going on than to break without a clear reason.
> 
> Carrying versioning information around allows userspace to immediately 
> tell what happened and gives joe user a better hint at what he can do to 
> get it fixed.
> 
> > I can't see why adding a while new set of icotls at the current API level,
> > won't work, userspace that wants the bug fixed can just use the new APIs,
> > older userspace users the old ones.
> 
> In this case, it is clear that one of the new ioctls should be used 
> before the old ones are useful. In fact, dri initialisation fails 
> completely without this. Hence my original mail.
> 
> With this whole thread i get the standard feeling i have when i put some 
> effort into communicating with you and some others.
> 
> I state what i see as pretty obvious, and it gets trashed with little to 
> nothing backing the thrashing, then i have to spend ages patiently 
> trying to explain what i see as pretty obvious, in a way that it cannot 
> be baselessly thrashed anymore, which often means repeating things a lot 
> (and yes, i do not have the patience for that).
> 
> Now what i am wondering is, are these things really not obvious, or are 
> you just naysaying things for other reasons?

If the reasons were obvious every other DRI driver would have done this by 
now, none of them have. I wold expect Thomas who was involved in the via
community to see the requirement, he has stated he has not. The header 
file is not the only thing to contain API defining information. Also
no other kernel subsystem with a user/kernel api I know off does this. I 
think alsa just ship a copy of the file with both kernel and userspace
and update them independently.

However as I've said if the version number patch is extracted from drm.git 
sent to the list against the latest kernel via_drm.h, I will not object to 
applying it and pushing it upstream. So all I'm really saying is I'm not 
touching things without a patch.

Dave.