DRM: Add DRM core files and DRMKPI

[manu]

Summary

Import DRM core from Linux 5.4(-ish).
For ARM and ARM64 driver we need DRM in base for a differents reasons :

• For embedded product we might want to have driver in the kernel
• Linux drivers are GPL licenced and thus can only be used as kernel modules
• The frameworks for FDT based resource is very different between Linux and FreeBSD, making a linuxkpi abstraction really difficult
• Drivers need to be synced with DTS, which we import quickly after a Linux release is out.

Directories are structured as :

• core/ : The drm core files and include, this matches was is present in Linux at drivers/gpu/drm/ and include/drm
  • freebsd/ : FreeBSD specific files, this includes some helpers that cannot be OS agnostic (like gem_cma/fb_cma/gem_frambuffer), some code for framebuffer<->VT relation (and VT switching) and the sysctl/sysfs needed code for drm core.
  • drmkpi/ : A somewhat stripped down copy of linuxkpi The reasons for having a copy are :
  • The full linuxkpi is usefull for importing drivers from Linux, but for FreeBSD drivers it adds some useless stuff like a linux_dev for each device.
  • We don't need nor want sysfs, netdevice code or other stuff not needed for DRM.
    • It should be self-contained for the DRM drivers and subsystem (which is partialy done).
    • bridges/ : This is where drm_bridges device will live, for now it only contain a generic interface for them.

All drm related sysctls are under dev.drm (like dev.drm.debug)
VT switching works even when we panic (tested with sysctl debug.kdb.panic=1)
The only missing pieces for a full sync with Linux 5.4 are the recent changes
in the dma-buf and drm_objsync (like the timeline feature).

[• hselasky]

drmkpi/ : A somewhat stripped down copy of linuxkpi The reasons for having a copy are :
The full linuxkpi is usefull for importing drivers from Linux, but for FreeBSD drivers it adds some useless stuff like a linux_dev for each device.

I really don't like this.

How many are the differences?

The currently layout of sys/compat/linuxkpi , was exactly made so that you could overlay different versions of LinuxKPI, having one base LinuxKPI. We should try to build on that. Your approach likely also breaks the kernel linker, which currently doesn't care if you have multiple symbols with same name. What happens if you load drmkpi and linuxkpi at the same time?

Please integrate drmkpi within the sys/compat/linuxkpi folder, like this:

1) Create sys/compat/linuxkpi/basedrm/xxxx  include and src
2) use #include_next <> where appropriate.

--HPS

[manu]

In D23085#505934, @hselasky wrote:

drmkpi/ : A somewhat stripped down copy of linuxkpi The reasons for having a copy are :
The full linuxkpi is usefull for importing drivers from Linux, but for FreeBSD drivers it adds some useless stuff like a linux_dev for each device.

I really don't like this.

How many are the differences?

The currently layout of sys/compat/linuxkpi , was exactly made so that you could overlay different versions of LinuxKPI, having one base LinuxKPI. We should try to build on that. Your approach likely also breaks the kernel linker, which currently doesn't care if you have multiple symbols with same name. What happens if you load drmkpi and linuxkpi at the same time?

Please integrate drmkpi within the sys/compat/linuxkpi folder, like this:

1) Create sys/compat/linuxkpi/basedrm/xxxx  include and src
2) use #include_next <> where approprate.

--HPS

That would mean creating a .h for every file but I guess this is cleaner, I'll have a look at this.

[• hselasky]

In D23085#505935, @manu wrote:

In D23085#505934, @hselasky wrote:

drmkpi/ : A somewhat stripped down copy of linuxkpi The reasons for having a copy are :
The full linuxkpi is usefull for importing drivers from Linux, but for FreeBSD drivers it adds some useless stuff like a linux_dev for each device.

I really don't like this.

How many are the differences?

The currently layout of sys/compat/linuxkpi , was exactly made so that you could overlay different versions of LinuxKPI, having one base LinuxKPI. We should try to build on that. Your approach likely also breaks the kernel linker, which currently doesn't care if you have multiple symbols with same name. What happens if you load drmkpi and linuxkpi at the same time?

Please integrate drmkpi within the sys/compat/linuxkpi folder, like this:

1) Create sys/compat/linuxkpi/basedrm/xxxx  include and src
2) use #include_next <> where approprate.

--HPS

That would mean creating a .h for every file but I guess this is cleaner, I'll have a look at this.

If the files are identical, then you don't need any wrapper, using include "basedrm" first, then "common" afterwards.

It is great you got things working!

Extend the base LinuxKPI source functions with the flags you need, like don't create linux_dev and have one wrapper in common and one in basedrm.

For example:

common/include/xxx.h

#if defined(LINUXKPI_BASEDRM)
Do it this way
#else
Do it that way
#endif

basedrm/include/xxx put the DRM header files here which you need.

Static inline functions is your friend.

Maybe Johannes has some opinions too.

Is it possible to add this driver as part of the existing KMS DRM driver?

--HPS

[• hselasky]

Also keep in mind that we can split out network related functions from the LinuxKPI, in an own linuxkpi_net .

Like:

linuxkpi_base.ko
linuxkpi_net.ko
linuxkpi_drm.ko
linuxkpi_gplv2.ko

and so on.

This would be the right approach forward.

[manu]

In D23085#505942, @hselasky wrote:

In D23085#505935, @manu wrote:

In D23085#505934, @hselasky wrote:

drmkpi/ : A somewhat stripped down copy of linuxkpi The reasons for having a copy are :
The full linuxkpi is usefull for importing drivers from Linux, but for FreeBSD drivers it adds some useless stuff like a linux_dev for each device.

I really don't like this.

How many are the differences?

The currently layout of sys/compat/linuxkpi , was exactly made so that you could overlay different versions of LinuxKPI, having one base LinuxKPI. We should try to build on that. Your approach likely also breaks the kernel linker, which currently doesn't care if you have multiple symbols with same name. What happens if you load drmkpi and linuxkpi at the same time?

Please integrate drmkpi within the sys/compat/linuxkpi folder, like this:

1) Create sys/compat/linuxkpi/basedrm/xxxx  include and src
2) use #include_next <> where approprate.

--HPS

That would mean creating a .h for every file but I guess this is cleaner, I'll have a look at this.

If the files are identical, then you don't need any wrapper, using include "basedrm" first, then "common" afterwards.

I've just did some basic test and I think I can update the patch using this technique in a few days.

It is great you got things working!

It has been working for almost a year for me and more for @mmel :)

Extend the base LinuxKPI source functions with the flags you need, like don't create linux_dev and have one wrapper in common and one in basedrm.

For example:

common/include/xxx.h

#if defined(LINUXKPI_BASEDRM)
Do it this way
#else
Do it that way
#endif

basedrm/include/xxx put the DRM header files here which you need.

Static inline functions is your friend.

Maybe Johannes has some opinions too.

Is it possible to add this driver as part of the existing KMS DRM driver?

What do you mean ?

--HPS

[manu]

In D23085#505943, @hselasky wrote:

Also keep in mind that we can split out network related functions from the LinuxKPI, in an own linuxkpi_net .

Like:

linuxkpi_base.ko
linuxkpi_net.ko
linuxkpi_drm.ko
linuxkpi_gplv2.ko

and so on.

This would be the right approach forward.

That might be a good solution yes.
For now this doesn't work as a module, the main reason is that I could never make my drm driver works as a module for a few reasons :

• This is a multi driver thing and when in the same kernel module I couldn't find the right incatation for driver ordering
• When in multi-part module I have some weird issue with interfaces between them (like I cannot call the interface function of another module).

[• hselasky]

Is it possible to add this driver as part of the existing KMS DRM driver?

What do you mean ?

I mean as a part of https://github.com/FreeBSDDesktop/kms-drm .

--HPS

[• hselasky]

That might be a good solution yes.
For now this doesn't work as a module, the main reason is that I could never make my drm driver works as a module for a few reasons :

Doesn't matter. Just make corresponding kernel options, like the existing COMPAT_LINUXKPI .

options COMPAT_LINUXKPI_BASE
options COMPAT_LINUXKPI_NET
options COMPAT_LINUXKPI_DRM

...

and so on.

[manu]

In D23085#505946, @hselasky wrote:

Is it possible to add this driver as part of the existing KMS DRM driver?

What do you mean ?

I mean as a part of https://github.com/FreeBSDDesktop/kms-drm .

--HPS

No, why would I do that ?

[manu]

In D23085#505947, @hselasky wrote:

That might be a good solution yes.
For now this doesn't work as a module, the main reason is that I could never make my drm driver works as a module for a few reasons :

Doesn't matter. Just make corresponding kernel options, like the existing COMPAT_LINUXKPI .

options COMPAT_LINUXKPI_BASE
options COMPAT_LINUXKPI_NET
options COMPAT_LINUXKPI_DRM

...

and so on.

Yup.

[• hselasky]

In D23085#505951, @manu wrote:

In D23085#505946, @hselasky wrote:

I mean as a part of https://github.com/FreeBSDDesktop/kms-drm .

--HPS

No, why would I do that ?

It might be easier to maintain in the future. I'm just asking. I don't know all the details.

--HPS

[mmel]

In D23085#505942, @hselasky wrote:

common/include/xxx.h

#if defined(LINUXKPI_BASEDRM)
Do it this way
#else
Do it that way
#endif

basedrm/include/xxx put the DRM header files here which you need.

Static inline functions is your friend.

Maybe Johannes has some opinions too.

Is it possible to add this driver as part of the existing KMS DRM driver?

I don't agree with this. By my experience, static inline functions and #defines overriding standard freebsd functions creates very unlovable environment.
Our goal for drmkpi is not to emulate linux (mainly not task, files,etc...) behavior but create a library for translation of basic linux primitives to freebsd ones (semaphores, mutexes) where its posible, nothing more.
I plan to trying next step by moving all function to C files strictly prefixed by drmpki_<foo>. The rename header (by using #defines or inline wrapper) will be included only to original linux files, and it will not be include any of freebsd headers. This allow us to separate linux and freebsd world.

Also, please, don't take this that linuxkpi critique, I fully understand why you chosed actual implementation style. We simply have slightly different conditions a so we must take drmkpi as adaptation, not emulation, layer.

[manu]

I've pushed some change here : https://github.com/evadot/freebsd/commits/drm_arm_v5.4-rebase-linuxkpi

This moves drmkpi files into sys/compat/linuxkpi
This uses the headers from common/include/ when we can.
This uses the linux_*.c files when we can.
I've only done that so @hselasky can see more the diff between linuxkpi and drmkpi.

[• hselasky]

OK, I'll see if I can spend some time tomorrow to look at the code. Thank you!

[bcr]

Do you need any specific actions from the core team or was core only added as reviewer because Phabricator tried to be smart (too much mention of the word "core" in a different context)?
Let us know if we need to discuss/decide something.

[manu]

In D23085#505975, @bcr wrote:

Do you need any specific actions from the core team or was core only added as reviewer because Phabricator tried to be smart (too much mention of the word "core" in a different context)?
Let us know if we need to discuss/decide something.

core was added because some files are dual licenced GPL/MIT and there is an herald rule that adds core for anything that matches GPL.
We will first fix the technical details and if core is needed we'll ping you :)

[bcr]

In D23085#505979, @manu wrote:

In D23085#505975, @bcr wrote:

Do you need any specific actions from the core team or was core only added as reviewer because Phabricator tried to be smart (too much mention of the word "core" in a different context)?
Let us know if we need to discuss/decide something.

core was added because some files are dual licenced GPL/MIT and there is an herald rule that adds core for anything that matches GPL.
We will first fix the technical details and if core is needed we'll ping you :)

Ah yes, I found the herald rules. Cool, we'll be in touch.

[zeising]

Is the ports drm graphics drivers intended to be switched over to this framework, or is it only for ARM kernel drivers?

[manu]

In D23085#506309, @zeising wrote:

Is the ports drm graphics drivers intended to be switched over to this framework, or is it only for ARM kernel drivers?

I don't plan to do anything in the port drm driver

[• hselasky]

Can you use the port-drm driver at the same time as base DRM? Some people use PCI graphics adapters with ARM.

[manu]

In D23085#506311, @hselasky wrote:

Can you use the port-drm driver at the same time as base DRM? Some people use PCI graphics adapters with ARM.

If they are at the same version that could work but I don't plan to work on that.
There is one board that FreeBSD kinda support with a PCIe controller that support graphics card and it's the macchiatobin which cost $500.
While my allwinner driver supports any H3/H5/A64 boards (there is something like ~40 boards).
@mmel nvidia driver will add support to any nvidia board that we already supports.
And I'll start a rockchip driver soon.
And by "some people" you mean one guy.

[val_packett.cool]

We simply have slightly different conditions a so we must take drmkpi as adaptation, not emulation, layer.

Why not keep the emulation for everything that's already in linuxkpi/drm-kmod, while adding adaptation for the FDT stuff and other missing parts?!

Adding a completely different copy of drm_*.c from what is used on desktop feels like a mess and duplicate work.

There is one board that FreeBSD kinda support with a PCIe controller that support graphics card and it's the macchiatobin which cost $500.

Well, $339 plus shipping but with a DDR4 stick.
The HoneyComb LX2K might work too (they did the ECAM thing!), and the expensive servers (eMAG, ThunderX/2) should work as well btw.

@mmel nvidia driver will add support to any nvidia board that we already supports.

Only tegra display or nouveau too?
Nouveau is for both tegra and desktop… this is why it really sucks that embedded and desktop drivers are in separate worlds >_<

And I'll start a rockchip driver soon.

Have you seen NetBSD's rockchip driver btw?

[manu]

In D23085#506688, @greg_unrelenting.technology wrote:

We simply have slightly different conditions a so we must take drmkpi as adaptation, not emulation, layer.

Why not keep the emulation for everything that's already in linuxkpi/drm-kmod, while adding adaptation for the FDT stuff and other missing parts?!

To what goal ? We don't want to use the linux driver.

Adding a completely different copy of drm_*.c from what is used on desktop feels like a mess and duplicate work.

There is one board that FreeBSD kinda support with a PCIe controller that support graphics card and it's the macchiatobin which cost $500.

Well, $339 plus shipping but with a DDR4 stick.
The HoneyComb LX2K might work too (they did the ECAM thing!), and the expensive servers (eMAG, ThunderX/2) should work as well btw.

And for thoses boards it's better to use the drm-kmod port with radeon/amdgpu.
That does raise the question : do we want multiple GENERIC-like config, one for "server grade" one for SBC.

@mmel nvidia driver will add support to any nvidia board that we already supports.

Only tegra display or nouveau too?
Nouveau is for both tegra and desktop… this is why it really sucks that embedded and desktop drivers are in separate worlds >_<

And I'll start a rockchip driver soon.

Have you seen NetBSD's rockchip driver btw?

[manu]

In D23085#505971, @hselasky wrote:

OK, I'll see if I can spend some time tomorrow to look at the code. Thank you!

Did you had time to look at this ?

Cheers,

[• hselasky]

Sorry, I need more time to review this.

[• hselasky]

Probably I asked already, but can the DRMKPI be used at the same time another driver is using the LinuxKPI?

[manu]

In D23085#509098, @hselasky wrote:

Probably I asked already, but can the DRMKPI be used at the same time another driver is using the LinuxKPI?

Not currently but this needs to be addressed before commit.

[• hselasky]

The DRMKPI needs to be tighter integrated with the current LinuxKPI, before this part can be upstreamed in my opinion.

My main worry is the DRMKPI will not be maintained when bugs are found in the LinuxKPI and/or kernel and/or Linux upstream.

Can you update this patch with the current changes integrating DRMKPI with LinuxKPI?

I believe some work was already done in this area.

[manu]

In D23085#509309, @hselasky wrote:

The DRMKPI needs to be tighter integrated with the current LinuxKPI, before this part can be upstreamed in my opinion.

My main worry is the DRMKPI will not be maintained when bugs are found in the LinuxKPI and/or kernel and/or Linux upstream.

Can you update this patch with the current changes integrating DRMKPI with LinuxKPI?

I believe some work was already done in this area.

So we have two options :

1. Leave it like this
2. "Merge" drmkpi and linuxkpi

Option 1 is easy, it's done, being working for almost two years for me, a little bit more for @mmel.
Option 2 is harder.
This means :

• committing D26728, D26727 and more.
• Be sure that no linux-only (struct linux_file, struct device and more) are added to the shared code and header between linuxkpi and drmkpi.

I know that @bz already expressed some disagreement for option 2.
Personally I think that option 2 could be done but will end up to be a burden for me and @mmel to partially revert changes in linuxkpi because they conflict with drmkpi.
For option 1 there is still a few task to be done :

• Make sure that drm (and the drivers) can be compiled in module form (I have still a few issues) to ensure that we could ship in the default image the new drm<something>.ko in base and still use drm-kmod on aarch64 for amdgpu.
• Make sure that all functions do not collide with linuxkpi ones.
• Something in the task creationg iirc (they are still considered as linuxkpi ones while they are natives).
• Probably something else.

For your worry that drmkpi will not be maintained I assure you that if this goes in we will have two drm drivers soon after (allwinner and tegra) and I plan to work on more (I honestly didn't had the motivation to start working on a rockchip one before this situation was somewhat resolved). I'm also sure that we could trick more people to work on other drm driver (like @gonzo for IMX6/8 and @oskar.holmlund_ohdata.se for BBB :P )

So let's the discussion begin. I want drm on SBC for FreeBSD-13, this is beginning to be ridiculous to not have it considering the number of boards/laptop/tablet that we could run on nowadays.

Cheers,

[mikael]

In D23085#603865, @manu wrote:

So let's the discussion begin. I want drm on SBC for FreeBSD-13, this is beginning to be ridiculous to not have it considering the number of boards/laptop/tablet that we could run on nowadays.

+1

[• hselasky]

Some thoughts:

I don't like that we duplicate the LinuxKPI code and header files in the tree. When we sometimes fix bugs in the LinuxKPI they won't be fixed in the DRMKPI automagically.

Some header files, like the atomic functions I see no reason to duplicate. Same goes for RCU support.

Also the DRMKPI cannot be used at the same time as the LinuxKPI, which is a disadvantage.

Is there some script you can run to remove absolutely all DRMKPI functions and header files that are not needed to build this display driver?

[manu]

In D23085#604159, @hselasky wrote:

Some thoughts:

I don't like that we duplicate the LinuxKPI code and header files in the tree. When we sometimes fix bugs in the LinuxKPI they won't be fixed in the DRMKPI automagically.

Some header files, like the atomic functions I see no reason to duplicate. Same goes for RCU support.

A lot more could be shared in the header files, it will work until some linuxkpi thing (like struct device/struct file) will be added to them.

Also the DRMKPI cannot be used at the same time as the LinuxKPI, which is a disadvantage.

This is something that need to be resolved yes (as noted in my latest message).

Is there some script you can run to remove absolutely all DRMKPI functions and header files that are not needed to build this display driver?

The "advantage" of having pristine (or almost) copy in drmkpi is that it will be easier to sync them in the futur (and so address your first comment).

[• hselasky]

A lot more could be shared in the header files, it will work until some linuxkpi thing (like struct device/struct file) will be added to them.

I think you can depend on that the atomic files will be self-contained.

But I see your point, that once you start including stuff from the LinuxKPI, then you eventually need a MODULE_DEPEND(), and then it will stop working ...

It's just that it is not only one file you need from the LinuxKPI, it is several dozen :-)

Won't the kernel LINT target will break once you import the DRMKPI?

--HPS

[manu]

In D23085#604163, @hselasky wrote:

A lot more could be shared in the header files, it will work until some linuxkpi thing (like struct device/struct file) will be added to them.

I think you can depend on that the atomic files will be self-contained.

But I see your point, that once you start including stuff from the LinuxKPI, then you eventually need a MODULE_DEPEND(), and then it will stop working ...

Yup,

It's just that it is not only one file you need from the LinuxKPI, it is several dozen :-)

Won't the kernel LINT target will break once you import the DRMKPI?

Haven't tested to be honest yet, but I'll fix the eventual issues before committing.

--HPS

[bz]

I think there's a few things which currently concern me are:

(1) the idea of splitting up linuxkpi into base/net/... as Hans once suggested seems rather not a great idea. It's all intertwined.

(2) the more I was thinking of this "drmkpi" it is not actually a "DRM KPI"? It is merely a stripped down "LinuxKPI"? The actual "DRM KPI" is "DRM". Is that a correct view?

(3) I think I am also a bit confused by the multitude of things in this single review. Forgive my ignorance, but the "core/drm" code, will that be specific to this "DRMKPI" or will that be shared again with the drm-kmod world? I am asking (a) if we can split this up about what we are actually talking it'll be a lot easier to see, and (b) if it'll not be "shared", I think "dev/drm/" itself will lead to a lot of confusion and the question where drm-kmod "DRM" bits would go, but that's not the discussion here (hence (a)).

(4) Is there an actual driver somewhere to see how this will plug together? Or a tree containing it all? And is this current version up-to-date or will the drm files be updated again? I just keep finding myself to grep through a huge diff and wondering what that "struct device" is that you are trying to avoid so much as the structure seems not to be defined anywhere ...?

[manu]

In D23085#604220, @bz wrote:

I think there's a few things which currently concern me are:

(1) the idea of splitting up linuxkpi into base/net/... as Hans once suggested seems rather not a great idea. It's all intertwined.

(2) the more I was thinking of this "drmkpi" it is not actually a "DRM KPI"? It is merely a stripped down "LinuxKPI"? The actual "DRM KPI" is "DRM". Is that a correct view?

Yes

(3) I think I am also a bit confused by the multitude of things in this single review. Forgive my ignorance, but the "core/drm" code, will that be specific to this "DRMKPI" or will that be shared again with the drm-kmod world? I am asking (a) if we can split this up about what we are actually talking it'll be a lot easier to see, and (b) if it'll not be "shared", I think "dev/drm/" itself will lead to a lot of confusion and the question where drm-kmod "DRM" bits would go, but that's not the discussion here (hence (a)).

It will not be shared (at least not at first, some files could be, some can't for now).
drm-kmod will not go there (again for now, there is still a lot to do for that and I'm not sure how this will be done).

(4) Is there an actual driver somewhere to see how this will plug together? Or a tree containing it all? And is this current version up-to-date or will the drm files be updated again? I just keep finding myself to grep through a huge diff and wondering what that "struct device" is that you are trying to avoid so much as the structure seems not to be defined anywhere ...?

https://github.com/evadot/freebsd/tree/drm_base_v5.6-20201027
This is my latest tree with an allwinner driver.
The drm code will be updated again of course, don't know when.
Of course struct device and struct file aren't defined, we use the native freebsd struct in those files.

[manu]

In D23085#604221, @manu wrote:

In D23085#604220, @bz wrote:

I think there's a few things which currently concern me are:

(1) the idea of splitting up linuxkpi into base/net/... as Hans once suggested seems rather not a great idea. It's all intertwined.

(2) the more I was thinking of this "drmkpi" it is not actually a "DRM KPI"? It is merely a stripped down "LinuxKPI"? The actual "DRM KPI" is "DRM". Is that a correct view?

Yes

(3) I think I am also a bit confused by the multitude of things in this single review. Forgive my ignorance, but the "core/drm" code, will that be specific to this "DRMKPI" or will that be shared again with the drm-kmod world? I am asking (a) if we can split this up about what we are actually talking it'll be a lot easier to see, and (b) if it'll not be "shared", I think "dev/drm/" itself will lead to a lot of confusion and the question where drm-kmod "DRM" bits would go, but that's not the discussion here (hence (a)).

It will not be shared (at least not at first, some files could be, some can't for now).
drm-kmod will not go there (again for now, there is still a lot to do for that and I'm not sure how this will be done).

(4) Is there an actual driver somewhere to see how this will plug together? Or a tree containing it all? And is this current version up-to-date or will the drm files be updated again? I just keep finding myself to grep through a huge diff and wondering what that "struct device" is that you are trying to avoid so much as the structure seems not to be defined anywhere ...?

https://github.com/evadot/freebsd/tree/drm_base_v5.6-20201027
This is my latest tree with an allwinner driver.
The drm code will be updated again of course, don't know when.
Of course struct device and struct file aren't defined, we use the native freebsd struct in those files.

It's also there : https://reviews.freebsd.org/D23086

[bz]

(3) I think I am also a bit confused by the multitude of things in this single review. Forgive my ignorance, but the "core/drm" code, will that be specific to this "DRMKPI" or will that be shared again with the drm-kmod world? I am asking (a) if we can split this up about what we are actually talking it'll be a lot easier to see, and (b) if it'll not be "shared", I think "dev/drm/" itself will lead to a lot of confusion and the question where drm-kmod "DRM" bits would go, but that's not the discussion here (hence (a)).

It will not be shared (at least not at first, some files could be, some can't for now).
drm-kmod will not go there (again for now, there is still a lot to do for that and I'm not sure how this will be done).

OK, punt for now. Is fine with me.

(4) Is there an actual driver somewhere to see how this will plug together? Or a tree containing it all? And is this current version up-to-date or will the drm files be updated again? I just keep finding myself to grep through a huge diff and wondering what that "struct device" is that you are trying to avoid so much as the structure seems not to be defined anywhere ...?

https://github.com/evadot/freebsd/tree/drm_base_v5.6-20201027
This is my latest tree with an allwinner driver.
The drm code will be updated again of course, don't know when.
Of course struct device and struct file aren't defined, we use the native freebsd struct in those files.

So all the parts of DRM that use "struct device" are to be thought as "#ifdef linux" and FreeBSD won't use them at all?

[manu]

In D23085#604229, @bz wrote:

(3) I think I am also a bit confused by the multitude of things in this single review. Forgive my ignorance, but the "core/drm" code, will that be specific to this "DRMKPI" or will that be shared again with the drm-kmod world? I am asking (a) if we can split this up about what we are actually talking it'll be a lot easier to see, and (b) if it'll not be "shared", I think "dev/drm/" itself will lead to a lot of confusion and the question where drm-kmod "DRM" bits would go, but that's not the discussion here (hence (a)).

It will not be shared (at least not at first, some files could be, some can't for now).
drm-kmod will not go there (again for now, there is still a lot to do for that and I'm not sure how this will be done).

OK, punt for now. Is fine with me.

(4) Is there an actual driver somewhere to see how this will plug together? Or a tree containing it all? And is this current version up-to-date or will the drm files be updated again? I just keep finding myself to grep through a huge diff and wondering what that "struct device" is that you are trying to avoid so much as the structure seems not to be defined anywhere ...?

https://github.com/evadot/freebsd/tree/drm_base_v5.6-20201027
This is my latest tree with an allwinner driver.
The drm code will be updated again of course, don't know when.
Of course struct device and struct file aren't defined, we use the native freebsd struct in those files.

So all the parts of DRM that use "struct device" are to be thought as "#ifdef linux" and FreeBSD won't use them at all?

struct device in drmkpi is struct device from freebsd (device_t).
See : https://github.com/evadot/freebsd/blob/drm_base_v5.6-20201027/sys/dev/drm/core/drm_drv.c#L632
https://github.com/evadot/freebsd/blob/drm_base_v5.6-20201027/sys/dev/drm/allwinner/aw_de2_drm.c#L242

[mmel]

The main issue in SBC DRM implementation is that graphic driver doesn’t handle separate self-contained device but it handle device tightly integrated in system, where many part of SoC are shared between GPU and rest of system. Also, graphic subsystem is not monolithic, it consist from multiple subdevices spreaded over whole SoC. some of these subdevices must be implemented by native drivers (rasterizer, HDMI formatter) we cannot condition presence of (unaccelerated) graphic console by GPL code.
Because of this we cannot use linux_device approach (we must be able to pass same struct device (or device_t) between native and ‘emulated’ code in all possible ways, same is true also for files and threads.
Number 1 for SBC is to have a working (with hotplug monitor) graphic output with a BSD license that can be used for X11 (Wayland). Acceleration 2D / 3D acceleration is optional and, in the worst case, can be performed by a GPL-licensed module.
And about DRMKPI - many goals are very different from LINUXKPI. DRMKPI is an attempt to implement some of the mostly used Linux functions or primitives used in DRM in a direct native way.
Another problem with LINUXKPI is significant namespace pollution caused by #define in the headers. I plan to make significant changes in this area. Each individual function should have the prefix linux_ <foo> defined to its original name only if the header is included in the Linux code.
For these reasons, I prefer to have separate implementations for now. Once the situation stabilizes, we can reconsider.
The problem has many threads that I am not able to express in a clear form, the problem is simply too complex. We have (unlike others) a functional (and I think usable) prototype, it needs some work before it can be committed (first it is necessary to resolve the real conflict LINUXKPI). But first we need a certain level of conclusion - no one is motivated to spread unacceptable code.
Plus, as you can see, I'm not a technical writer, so please excuse my chaotic words and sentences.
And let me not forget, this is by no means an attempt to slander LINUXKPI. But we are, although it does not seem so, in a very different situation.

[val_packett.cool]

I think that having one single codebase, fully based on GPL code for fast porting, is a MUCH more important goal than having display output without GPL (/in base).

Actually I believe that the latter should not have been a goal at all, and splitting the worlds for that was a bad decision :P Literally what is the big deal with including a GPL'd drm-kmod package in the SBC filesystem images?? I don't think anybody would care about the licensing purity of these. The vast majority of users, first thing they do – pkg install more stuff, a lot of which is GPL.

Also, on many of these embedded SoCs (well not on rockchip for some reason) u-boot has a display output driver – and that gives you efifb, so early console is covered, you could even pkg install drm-kmod like on a desktop.

[mmel]

In D23085#604314, @greg_unrelenting.technology wrote:

I think that having one single codebase, fully based on GPL code for fast porting, is a MUCH more important goal than having display output without GPL (/in base).

Why you think that “single codebase, fully based on GPL” has any relation to fast porting in this context?
My understanding is that because the graphics on the SBC SoCs are related to virtually every subsystem, porting (and maintenance) cost increase exponentially.

Only I, as a contractor and self-employed, have FBSD installed on several thousand boards (as control and/or user interaction plane CPU in various industrial machines). Be sure that there are many more similar installations and for all these is use of GPL code unacceptable (and probably illegal - you cannot deliver mix of GPL and not-GPL code in one installation, I think).

Actually I believe that the latter should not have been a goal at all, and splitting the worlds for that was a bad decision :P Literally what is the big deal with including a GPL'd drm-kmod package in the SBC filesystem images?? I don't think anybody would care about the licensing purity of these. The vast majority of users, first thing they do – pkg install more stuff, a lot of which is GPL.

Again, you're just talking about fans with their SBC based desktops on the table, right? But this (probably minor) usage cannot be extrapolated to the whole world.

Also, on many of these embedded SoCs (well not on rockchip for some reason) u-boot has a display output driver – and that gives you efifb, so early console is covered, you could even pkg install drm-kmod like on a desktop.

As I wrote above, EFIFB is clearly not an option. EFIFB is a way to display messages from the bootloader and the initial stages of booting the OS, it should not be misused for anything else.

But if you insist on your original statement, try to do so. Nouveau is such a nice example :) And believe me, I've already tried.

[manu]

So, here is what I will do starting next week with an updated review hopefully next weekend :

• Rename all drmkpi function and structs with a drmkpi_ prefix to avoid clash with linuxkpi
• Strip down drmkpi to what's really needed (i.e. if a function/struct/macro/whatever is defined in linuxkpi and not needed for drmkpi it will be removed).
• Remove drm from GENERIC from arm64/GENERIC for now as we need to have already supported drm-kmod (armv7 isn't problematic as we don't have and never will drm-kmod support)
• Add a GENERIC-DRM kernel config for arm64
• Remove all the dma-bufs related code as I didn't tested it (unless @mmel needs it for tegra but I don't think so), I prefer to do a better/cleaner implem later.

I'll leave this updated review opened for a week and unless there is a good argument for not commiting it I'll do that then.

[• hselasky]

@manu: If the reason for DRM KPI is all about device_t , would it be possible to merge these two device structures somehow? Like we do for curthread and current ?

[manu]

In D23085#607992, @hselasky wrote:

@manu: If the reason for DRM KPI is all about device_t , would it be possible to merge these two device structures somehow? Like we do for curthread and current ?

No it's not just that, please read every reason that @mmel and myself said in this review.

I'm gonna say one last time why we need something different than linuxkpi for drm on arm (to be exact any native drm driver, arch doesn't matter).

Every drm driver for a PCI card does everything, display pipeline and rendering in one device, on embedded devices (arm/arm64/ possibly risc-v soon-ish) does that with multiple devices.
Instead of having one PCI device that does everything, every IP (as in Intellectual Property) is exposed. A classic list is :

• HDMI TX
• HDMI PHY
• DSI TX
• DSI PHY
• Display Engine

On top of that list you can add IOMMU on some devices, sometimes an encoder that sits between the display engine and one of the TX etc ...
All those IPs are represented by a single node in the DTB (think a separate entry in the ACPI TABLE if you want) so you need :

• One driver for each
• A method to make them talk to each other

So if you want to port the Linux driver to FreeBSD you need to port all those drivers.
You will also need to make some linuxkpi compat for all resources that those drivers uses (clocks, regulator, i2c, gpio, etc ...).
Even if, in theory, is possible to do (it's "just code") this is not realistic, so we need native drivers.

But for native drivers you still need the DRM core files as drivers needs to exposes their stuff in a defined way both between them and with userland.

If we want to use linuxkpi for native driver that would mean making a "linux" driver, which is stupid.

But linuxkpi still have a lot of code that we can re-use for making native drm driver and drm core working on FreeBSD which is the point of this review.

To sum up :

• We want native, bsd-licenced drm drivers
• We will never use linuxkpi for native drm drivers
• We're still working on making drmkpi co-exists with linuxkpi but making good progress

I hope that this time it was clear.

[val_packett.cool]

I suspect this would be much more acceptable if rather than "never using linuxkpi" and introducing some duplication, it would be something like a clearer separation of linuxkpi into "bottom layer linuxkpi" (that doesn't force "writing a linux driver" but provides functionality necessary for e.g. drm) and "top layer linuxkpi" (the stuff that makes linux drivers almost just compile). It's already somewhat like that – it is possible to just pick some headers from linuxkpi and use their functions in a native driver, and it doesn't force you to write a linux driver.

So, maybe everyone would be happier if base drm doesn't introduce sys/dev/drm/drmkpi/include/linux, but instead uses sys/compat/linuxkpi/common/include/linux which could be modified to support native drivers where needed. I think @hselasky is right that about the only point of contention would be the device type – lkpi functionality that is independent of the device type is already directly usable by native drivers.

[bz]

Given this is all in sys/dev/drm (at least here) I keep wondering if the main problem we have currently is that people think "drmkpi" == "linuxkpi" and a change away from "KPI" would help that (especially given it is not the DRM KPI but DRM [Linux] Compat").

In general I am the person who thinks "sort everything out first" but I think in this case we could get the stripped linuxkpi copy in and then still see how much of the "copy" we could get rid off one way or another. I think here in the review it might be harder to imagine than it may be once it is in the tree (e.g. by not being able to actually diff to linuxkpi).