Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Autotools support #1

Closed
wants to merge 6 commits into from
Closed

Autotools support #1

wants to merge 6 commits into from

Conversation

bluetech
Copy link
Contributor

Hi,
I stumbled upon your project while looking for info about kms, dri, etc. You are only beginning, so I thought it might be nice to jump in and help with what I can. I even registered to github for it :)

This includes the autotools support you asked for; I am by no means a build-system expert, but this ought to work well enough.

This commit adds basic autoconf + automake files to build the project.
It also adds a main.c stub in order to simulate the main binary.

The configure script uses pkg-config to find the libraries. The usual
stuff should work. The only additional option right now is:
        ./configure --enable-debug [To enable debugging symbols]
The Makefile should also support the standard stuff:
        make [To build the kmscon binary]
        make check [To build the test_* binaries]
        make dist [To create a tarball]
        make clean
        make install
        etc.

To start from a clean tree (e.g. git clean -dfx), do something like the
following:
        ./autogen.sh
        ./configure --enable-debug CFLAGS=-O0
        make

It all should work well enough for now.

Signed-off-by: Ran Benita <ran234@gmail.com>
Signed-off-by: Ran Benita <ran234@gmail.com>
Signed-off-by: Ran Benita <ran234@gmail.com>
-Wl,--as-needed

if DEBUG
AM_CFLAGS += -g
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to add -O0 to the CFLAGS when DEBUG is enabled? It's much more convenient to debug applications that haven't been optimized. Adding -O0 here doesn't help because CFLAGS still contains -O2. Do you know another way?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding the following line, just before the LT_PREREQ, should disable the default -g -O2:

: ${CFLAGS=""}

See https://www.gnu.org/software/autoconf/manual/html_node/C-Compiler.html AC_PROG_CC if your'e interested.
You can still add the -O0 to the += if you want to be explicit; the user may still override with ./configure CFLAGS=...

@dvdhrm
Copy link
Owner

dvdhrm commented Nov 27, 2011

Thanks for the patches. They look good, I've added some inline-comments. I can fix them when applying the patches if you want.

Please consider that the whole project is dedicated to the Public Domain as I haven't thought about any license, yet. This may change, though.

I will merge the patches during the next days. Thank you.
Regards
David

@bluetech
Copy link
Contributor Author

Hi, thanks for looking at this. It'd be easier if you make the fix as you see fit.
(btw, what's the proper way to make minor changes to pull requests? Send a new one, add a new commit, rebase? As I said, I'm new here :)

@bluetech bluetech closed this Nov 27, 2011
@bluetech bluetech reopened this Nov 27, 2011
The crtc is saved from kmscon_output_activate, and restored from
kmscon_output_deactivate.
This means that when the program exits, the screen is back to what it
was - we don't need to switch back VTs if we started the program from an
fbcon, etc.
Currently, when a new mode is bound to an output, it is inserted at the
head of the list, i.e. reversed from the order they are present in the
drmModeConnector modes array. But it's better to keep the order, because
it seems the best (high-res) modes come first; this way the lazy user,
who just passes NULL to kmscon_output_activate, would get a nice looking
default mode.
Previously it drew the font to the entire line.
while (iter->next)
iter = iter->next;
iter->next = mode;
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is ugly. The list is not supposed to be sorted, otherwise we would use double-linked lists. The preferred way here would be keeping a "default_mode/def_mode" pointer in each kmscon_output structure and linking this to the best output. We could then set it to the first mode we add or add some more logic which tries to find the most suitable mode.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good idea, but why not keep it sorted though? See also here
http://cgit.freedesktop.org/wayland/wayland-demos/tree/compositor/compositor-drm.c#n384
Which uses a doubly-linked list for the same thing.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know wayland does it the way you want it but wayland doesn't sort it either. If the kernel changes the way it reports the modes (which it can as it is not guaranteed to be sorted) then wayland will need to implement some sort algorithm.

However, I don't want to rely on some undocumented kernel behaviour which might break in the future.
We could, of course, implement some sorting by X-resolution or y-resolution. But I think this should be done at some other place. There is no benefit for us having this list sorted internally. Instead, we should provide a sane default mode and if and only if we visualize this list in a GUI or alike, then we should sort it. But I do not intend to provide some configuration-GUI.

It's just that I see no benefit in sorting this list except having a sane default value (which can be done much easier as I pointed out). If you see some other reason to sort the list, try to convince me ;)

@dvdhrm
Copy link
Owner

dvdhrm commented Dec 1, 2011

I cherry-picked your changes and fixed them on my own. Thanks for the patches.

If you have further patches, I'd prefer one pull-request for each pile of patches. And then rebase the commits if you modified them and update the pull request so if I pull I get the fixed patches not some weird fix-up commits on top of the merge.

And also group the patches logically. I don't mind if this results in one pull-requests for each change as long as each change fixes/modifies different problems.

Again, thanks and pulled ;)
David

@dvdhrm dvdhrm closed this Dec 1, 2011
@bluetech
Copy link
Contributor Author

bluetech commented Dec 2, 2011

Oh, for some reason I hadn't realized that those further commits will show up in the pull request (though it does make sense). I did not intend to push them to you yet, as you saw, but oh well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants