Page MenuHomeFreeBSD

[WIP] graphics/drm-*-kmod: make one metaport
ClosedPublic

Authored by jmd on Aug 26 2018, 6:23 AM.

Details

Summary

Create a metaport for the lkpi-based DRM ports. Rename existing ports to correspond to Linux versioning.

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

jmd created this revision.Aug 26 2018, 6:23 AM

The general idea is good. I'm not sure about the naming though, I need to mull this over.

The meta port should probably be called drm-kmod rather than drm-lkpi-kmod though.

linimon retitled this revision from [WIP] drm-*-kmod: make one metaport to [WIP] graphics/drm-*-kmod: make one metaport.Aug 27 2018, 9:41 AM
jmd updated this revision to Diff 48583.Sep 30 2018, 3:05 AM

Do not rename, include drm-legacy-kmod, use devel on 12+, as that's recommended and actively developed/fixed.

jmd updated this revision to Diff 48585.Sep 30 2018, 3:09 AM

Actually include new port...

zeising accepted this revision.Sep 30 2018, 1:42 PM

Looks good.
I'm a little hesitant to enable devel for 12, perhaps we should do that for 13?
I would change the IGNORES, especially for <12.0 i386 to say something like it's not needed, but this is just cosmetics.
Thank you for doing this!

This revision is now accepted and ready to land.Sep 30 2018, 1:42 PM
imp added a subscriber: imp.Sep 30 2018, 5:05 PM
imp added inline comments.
graphics/drm-kmod/Makefile
15 ↗(On Diff #48585)

I'd lose the OPSYS check here and have the check from below moved to the top and make this rather long ifs the 'else' part of it.

31 ↗(On Diff #48585)

Why drm-next here? I thought the messaging was drm-stable-kmod everywhere. It's the module I just installed for the nuc running 12.0ALPHA7. It's 1200084.

33 ↗(On Diff #48585)

and why drm-devel-kmod here?

34 ↗(On Diff #48585)

I'd be inclined to have a .else IGNORE=Go away here :)

37 ↗(On Diff #48585)

I'd move this to the top...

rene added a subscriber: rene.Sep 30 2018, 5:07 PM
rene added inline comments.
graphics/drm-kmod/Makefile
23 ↗(On Diff #48585)

FreeBSD 11.1 will be EOL tomorrow (2018-10-01), perhaps we want to raise the minimum version to 11.2? On the other hand, people are probably still using 11.1

graphics/drm-kmod/pkg-descr
3 ↗(On Diff #48585)

This looks a bit too much like a shopping list to me, what is the advantage of using this port instead of drm-*-kmod directly?

imp added a comment.Sep 30 2018, 5:25 PM

Despite my numerous nit-picks, I love this port. It's a little light on the 'why' and a few comments will fix that.

graphics/drm-kmod/Makefile
14 ↗(On Diff #48585)

This needs some why:

  1. Install the newest drm port the kernel will support. We generally prefer drm-next-kmod, but for older
  2. systems install drm-stable and guard against kernel that can't support the port. The versions correspond
  3. to when the Linux KPI was updated in the base. The one exception to this is the devel version which we
  4. install on bleeding edge systems. #

Normally I wouldn't bother, but there's much confusion here and a little extra documentation wouldn't be a burden. And if people have drm-stable installed now and go to install this, they will understand why drm-next gets installed instead...

32 ↗(On Diff #48585)

I'd make this 1300000 instead.

imp added a comment.Sep 30 2018, 5:28 PM

and s/live/like/ in my last comments :)

graphics/drm-kmod/Makefile
14 ↗(On Diff #48585)

The numbers here should be hash signs that the reviewboard markup starts numbering with..

jmd marked 2 inline comments as done.Sep 30 2018, 5:45 PM

Thanks for reviewing comments, will work on new revision.

graphics/drm-kmod/Makefile
31 ↗(On Diff #48585)

No, that's not the message - as you can see from the config here. The metaport is trying to embody our recommendations to avoid needing to support all possible enumerations (and also clear up the messaging that clearly wasn't ideal). If you want to run an old DRM and it supports your HW and FreeBSD version you can obviously still do that but realistically the work on the actual DRM bits and KPI parts is happening in the later versions so having everybody on "stable" (yeah, we also plan to change these names eventually) is unrealistic, especially if breaking changes get introduced into CURRENT.

32 ↗(On Diff #48585)

Fair enough.

imp added inline comments.Sep 30 2018, 6:11 PM
graphics/drm-kmod/Makefile
31 ↗(On Diff #48585)

It's not the message articulated to me before I added the drm abandonware messages is my point. My bigger point is that the message needs clarity as this port is at odds with what I thought the canonical message was. It's fine if I'm confused and we need a different message than I understood: I just want to make sure that the message is crystal clear, well documented and generally the consensus view so everybody can articulate it and we can enshrine it in docs for as long as it's relevant.

jmd updated this revision to Diff 48598.Sep 30 2018, 6:15 PM

Updated based on comments.

This revision now requires review to proceed.Sep 30 2018, 6:15 PM
jmd marked 13 inline comments as done.Sep 30 2018, 6:16 PM
jmd added inline comments.
graphics/drm-kmod/Makefile
14 ↗(On Diff #48585)

Added language to the description that hopefully encompasses this.

23 ↗(On Diff #48585)

I'd assume there'll be a few people still running on 11.1 and it doesn't hurt to have it?

34 ↗(On Diff #48585)

Used slightly more polite language ;-)

jmd marked 3 inline comments as done.Sep 30 2018, 6:17 PM
jmd updated this revision to Diff 48599.Sep 30 2018, 7:14 PM

Get rid of some redundant OPSYS checks

zeising accepted this revision.Sep 30 2018, 7:17 PM

Looks good.

This revision is now accepted and ready to land.Sep 30 2018, 7:17 PM
This revision was automatically updated to reflect the committed changes.