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

Modernize dumbbuffer #33

Merged
merged 7 commits into from
May 17, 2019
Merged

Conversation

Drakulix
Copy link
Member

@Drakulix Drakulix commented May 1, 2019

Hey Slabity,

to speed up #25 a bit, I tried to fix the buffer and dumbbuffer apis and would like to have some feedback.

  1. I had to move the ResourceHandle type into the root of the crate, because I think the GEM buffer handle needs this type as well.
  2. The added functions are all in the ControlDevice, because they require DRM_AUTH or DRM_MASTER (except for close_buffer, which is even allowed for RenderNodes).
  3. I have not added the flink call, as it does not exist in drm-ffi yet, do you want to support that?

Some additional questions, that showed themselves, when implementing this:

  1. nix does not seem to have a no_std-variant, but you already use it in drm-ffi, so I went with it. Is this an oversight? Because then we could just go with std. To fix this we could just use libc instead.
  2. Do you have a plan to move to stable? It seems str_internals is not going to be stablized any time soon, while nll is already available.

@Drakulix
Copy link
Member Author

Drakulix commented May 7, 2019

Looking at drmModeAddFb2 the current Buffer-trait might not be optimal and is likely the reason you started the PlanarBuffer-trait @Slabity, right?

Should PlanarBuffer be also implemented for every struct implementing Buffer? Technically every "normal" buffer is a PlanarBuffer with a single plane, right?

@Drakulix Drakulix force-pushed the feature/modernize_dumbbuffer branch from c3c847a to 8134dc3 Compare May 12, 2019 13:22
@Drakulix Drakulix mentioned this pull request May 12, 2019
6 tasks
@Drakulix Drakulix force-pushed the feature/modernize_dumbbuffer branch 2 times, most recently from 7ee9556 to a2fd839 Compare May 12, 2019 13:40
@Drakulix
Copy link
Member Author

Drakulix commented May 12, 2019

Since a lot of PRs started to require the buffer trait, I have now split of #38.
This now only adds the dumbbuffer-related functions.
Like this open discussion points regarding this PR, do not block anything else anymore.

Open points remaining:

  • nix does not seem to have a no_std-variant, but you already use it in drm-ffi, so I went with it. Is this an oversight? Because then we could just go with std. To fix this we could just use libc instead.
  • Do you have a plan to move to stable? It seems str_internals is not going to be stablized any time soon, while nll is already available.

@Drakulix Drakulix force-pushed the feature/modernize_dumbbuffer branch from a2fd839 to 858687c Compare May 16, 2019 12:14
@Drakulix Drakulix force-pushed the feature/modernize_dumbbuffer branch from 858687c to 9140729 Compare May 17, 2019 08:59
@Slabity
Copy link
Collaborator

Slabity commented May 17, 2019

These changes have been pulled from another PR, so closing this one.

To address your questions:

nix does not seem to have a no_std-variant, but you already use it in drm-ffi, so I went with it. Is this an oversight? Because then we could just go with std. To fix this we could just use libc instead.

That's a good question. I'm not sure a no_std-variant would be useful, as that typically means your crate doesn't require features from the operating system. Doesn't really make sense for a crate that specifically exposes an OS subsystem.

Do you have a plan to move to stable? It seems str_internals is not going to be stablized any time soon, while nll is already available.

I don't plan on doing that until at least after the functionality for the control module is done. But it will happen eventually.

@Slabity Slabity closed this May 17, 2019
@Slabity
Copy link
Collaborator

Slabity commented May 17, 2019

Nevermind - I jumped the gun and realized that the actual dumbbuffer.rs changes were not pulled in.

@Slabity Slabity reopened this May 17, 2019
@Drakulix
Copy link
Member Author

These changes have been pulled from another PR, so closing this one.

Many of my PR did depend on the buffer PR, but contain additional commits. (like this one.)
Dumb-buffer support is likely not already merged, is it? I can rebase those, now that #38 is done.

@Slabity Slabity merged commit ade09a7 into Smithay:develop May 17, 2019
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.

2 participants