Page MenuHomeFreeBSD

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

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



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

Diff Detail

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

Event Timeline

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

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

Actually include new port...

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 inline comments.
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 inline comments.
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

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?

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

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.

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

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.

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.

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.

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

Get rid of some redundant OPSYS checks

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.