Page MenuHomeFreeBSD

Put building of drm and drm2 modules behind options and Add big, nasty abandonware tags to this code.
AbandonedPublic

Authored by imp on Aug 25 2018, 1:21 AM.

Details

Summary

Make the building of drm dependent on MK_MODULE_DRM and the building
of module drm2 on MK_MODULE_DRM2. The defaults are unchanged.

Add big, nasty abandonware tags to this code.

Diff Detail

Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 19175
Build 18795: arc lint + arc unit

Event Timeline

  • Put building of drm and drm2 modules behind options.
  • Add big, nasty abandonware tags to this code.
cem added inline comments.
sys/dev/drm/drm.h
1154

maybe "drm drivers" to distinguish from "drm2 drivers" below.

rpokala added a subscriber: rpokala.

LGTM from a purely technical standpoint. (I'm actively avoiding the larger policy discussion around drm(4) and drm2(4).)

This revision is now accepted and ready to land.Aug 25 2018, 2:13 AM

cem's suggestions and WITH*DRM stuff

This revision now requires review to proceed.Aug 25 2018, 2:18 AM
rgrimes added a subscriber: rgrimes.

Looks good, my minor nits can be ignored, up to you.
Oh, I see you changed the return code of the module load, I take it that is to lower its preference???

tools/build/options/WITHOUT_MODULE_DRM2
2

Isnt this actually 4 modules, drm2, i915kms, radeonkms and radeonkmsfw.

tools/build/options/WITH_MODULE_DRM
2

Here you do not enumerate the modules, and I see that this is 10 modules, I like consistency, can we just not enumerate in either case and just say:
Enable creation of old drm/* modules
Enable creation of old drm2/* modules
In the respective files?

This revision is now accepted and ready to land.Aug 25 2018, 3:47 AM
This revision now requires review to proceed.Aug 25 2018, 3:55 AM
This revision is now accepted and ready to land.Aug 25 2018, 5:51 AM
juan.molina_club.fr added inline comments.
sys/dev/drm/drm.h
1152

Typo: s/abondon/abandon

sys/dev/drm2/drm_os_freebsd.h
165

Same typo

sys/dev/drm2/drm_os_freebsd.h
156

This is fine for now but this is a rare and true Small Matter Of Programming to fix outside of src to unifdef this.

imp marked an inline comment as done.Aug 25 2018, 2:11 PM
imp added inline comments.
sys/dev/drm/drm.h
1152

doh!

sys/dev/drm2/drm_os_freebsd.h
156

I'm afraid that I don't follow what you are suggesting here...

  • Add big, nasty abandonware tags to this code.
This revision now requires review to proceed.Aug 25 2018, 2:21 PM
imp marked 5 inline comments as done.Aug 25 2018, 4:58 PM

I think I've done all the actionable items in this review.

sys/dev/drm2/drm_os_freebsd.h
156

meaning I think we can have i386 on equal footing via ports changes. LP64 is an accidental heuristic It may need to explode to #if arm, powerpc, and i386 if we dont finish in time? I think this would only be a string change here so it may be able to happen late in the release cycle if so?

sys/dev/drm2/drm_os_freebsd.h
156

No. It's the right heuristic now.

drm-stable-mod supports only 64-bit platforms. And that will not change.
drm-legacy-mod supports both, but is primarily aimed at 32-bit platforms.

This notice isn't used for the non-module drivers in the tree. After 12.0 goes out, we are removing all module builds, the intel driver and the radeon driver. We'll almost certainly move dev/drm2 to arm/dev/drm2 due to the difficulties at the current time with sharing (though there's much discussions about how arm might share, it's still not a done deal as far as a path forward as there's a lot of work there and no funding at the moment). Once we're there, it won't matter if thisis one way or another as I'll likely remove this code after the move. And for drm (not drm2) it won't matter at all: those will all be gone completely, barring issues with the port arising (the port is quite new, less than a week, so it is prudent to have a fallback plan until it's proven).

sys/dev/drm2/drm_os_freebsd.h
156

https://gist.github.com/strejda/c98e513c2ec1d2e23005bb66a7bc6399 suggests that it's quite hard to cope with the drm-next stuff, and maybe all drm due to the drm pager issue. If we take drm2 arm-only, then that could be fixed w/o harming other arch (because they would use something else).

LGTM and I assume there's another patch coming that will fix the loader issues?

This revision is now accepted and ready to land.Aug 26 2018, 3:09 PM

LGTM and I assume there's another patch coming that will fix the loader issues?

Other patches are coming. There's two or three competing ideas on the best way forward, but all are predicated on this change.

Regen latest tree

  • Put building of drm and drm2 modules behind options.
  • Add big, nasty abandonware tags to this code.
This revision now requires review to proceed.Aug 28 2018, 2:34 PM

This has been committed..

In D16894#378335, @imp wrote:

This has been committed..

Could you please add a "committed in" comment.