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 19237
Build 18850: arc lint + arc unit

Event Timeline

imp created this revision.Aug 25 2018, 1:21 AM
imp edited the summary of this revision. (Show Details)Aug 25 2018, 1:49 AM
imp updated this revision to Diff 47274.Aug 25 2018, 2:08 AM
  • Put building of drm and drm2 modules behind options.
  • Add big, nasty abandonware tags to this code.
cem added a subscriber: cem.Aug 25 2018, 2:12 AM
cem added inline comments.
sys/dev/drm/drm.h
1155

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

rpokala accepted this revision.Aug 25 2018, 2:13 AM
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
imp updated this revision to Diff 47276.Aug 25 2018, 2:18 AM

cem's suggestions and WITH*DRM stuff

This revision now requires review to proceed.Aug 25 2018, 2:18 AM
rgrimes accepted this revision.Aug 25 2018, 3:47 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
3

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

tools/build/options/WITH_MODULE_DRM
3

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
imp updated this revision to Diff 47277.Aug 25 2018, 3:55 AM

Rod comments

This revision now requires review to proceed.Aug 25 2018, 3:55 AM
cem removed a subscriber: cem.Aug 25 2018, 5:10 AM
rgrimes accepted this revision.Aug 25 2018, 5:51 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
1153

Typo: s/abondon/abandon

sys/dev/drm2/drm_os_freebsd.h
166

Same typo

kbowling added inline comments.Aug 25 2018, 9:47 AM
sys/dev/drm2/drm_os_freebsd.h
157

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
1153

doh!

sys/dev/drm2/drm_os_freebsd.h
157

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

imp updated this revision to Diff 47285.Aug 25 2018, 2:21 PM
  • 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.

kbowling added inline comments.Aug 26 2018, 12:28 PM
sys/dev/drm2/drm_os_freebsd.h
157

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?

imp added inline comments.Aug 26 2018, 2:46 PM
sys/dev/drm2/drm_os_freebsd.h
157

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).

imp added inline comments.Aug 26 2018, 2:56 PM
sys/dev/drm2/drm_os_freebsd.h
157

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
imp added a comment.Aug 26 2018, 3:35 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.

imp updated this revision to Diff 47383.Aug 28 2018, 2:34 PM

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
mmacy added a comment.Oct 25 2018, 7:45 PM

should we abandon?

imp abandoned this revision.Oct 25 2018, 7:46 PM

This has been committed..

In D16894#378335, @imp wrote:

This has been committed..

Could you please add a "committed in" comment.