All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Anholt <eric@anholt.net>
To: Daniel Vetter <daniel.vetter@ffwll.ch>,
	dri-devel <dri-devel@lists.freedesktop.org>
Subject: Re: RFC: drm-misc for small drivers?
Date: Thu, 26 Jan 2017 11:48:24 -0800	[thread overview]
Message-ID: <8760l1siuv.fsf@eliezer.anholt.net> (raw)
In-Reply-To: <CAKMK7uFyxiY7ML1Ro_Z+huUD=jDv6Agq4R_eUxiix67j_ZBXNA@mail.gmail.com>


[-- Attachment #1.1: Type: text/plain, Size: 5069 bytes --]

Daniel Vetter <daniel.vetter@ffwll.ch> writes:

> Hi all,
>
> We've discussed this a bit at LCA (with Dave and Eric), and it's
> probably best if I just summarize all the questions and opens and
> throw them out here for discussions:
>
> - When's a driver small enough for a shared tree, and when is a
> separate tree a good idea? i915 and amdgpu are definitely big, and
> there's definitely drivers who are really small and in-between it's
> unclear. Personally I think this is easy to do with a sliding scale,
> with using topic branches (we can do them in drm-misc easily) for
> bigger stuff, and if that's a common thing, split out the driver
> (thanks to the drm-tip integration tree there's not much of a
> difference in handling conflicts due to that anyway).

FWIW, I don't anticipate vc4 needing its own tree again if we can get
into a shared-maintainership one.  I think needing our own tree would
mean that there's something going wrong with the merge process, which we
should work hard to fix.

> - Should it be an entire separate tree for soc drivers? Problem here
> is that we lack a volunteer group (and imo it really should be a group
> to avoid the single-maintainer troubles) to run that. I think it's
> easier to proof the process first, and if we want a separate tree,
> split that out later on. This is the same thing we've done with
> drm-misc, first with a topic branch in drm-intel.git, then separate. I
> think it worked really well.

I'd rather not have an soc tree -- I don't think being soc drivers makes
us special.

> - Should we require review or at least acks for patches committed by
> the author? We have a bunch of drivers with effectively just 1 person
> working on it, where getting real review is hard. But otoh a few of
> those 1-person drivers will become popular, and then it's good to
> start with establishing peer-review early on. I also think that
> requiring peer-review is good to share best practices and knowledge
> between different people in our community, not just to make sure the
> code is correct. For all these reasons I'm leaning towards not making
> an exception for drivers, and requiring the same amount of review for
> them if they go in through drm-misc as for any other patch.

This is the only part I'm concerned about.  Would anyone review vc4
patches?  Voluntarily?  Actually thinking about the
functionality/structure of the code, not just style?

It sucks today that as part of the kernel process I send out patches
"for review", knowing that I won't ever get review, and I just have to
wait a while until I think people won't complain at me for sending a PR
too quickly.  But if the change is to just start blocking my patches on
review, I'm concerned I won't be able to get them in at all.

I think there's a middle ground, where you graduate to waiting for
review once you have multiple involved in that area of the code.  This
is what we do in Mesa -- vc4 and freedreno push directly, but on the
code we share (tgsi_to_nir, for example), we do review.

(This is also the point at which I'll offer: Any other ARM drivers that
want to do review exchange with me, let me know and I'll start paying
attention to your stuff)

> - Who's elligible? I think we could start small with a few volunteers
> and their drivers, and then anyone who's willing.

I volunteer as tribute.

(assuming a resolution for the above question)

> - Should we force new submissions to be managed in that shared treee?
> I think for initial submission a separate pull request for
> approval-by-Dave is good (but we could do that with topic branches
> too). And it's also way too early to tell, probably better to first
> figure out how well this goes.

I'd rather wait on changing policy for new drivers until we have tested
this change with existing drivers for a bit.  And for initial
submission, getting the ack from Dave will always seem mandatory to me.

> - CI, needed? It would be great, but we're not there yet :( Atm
> drm-misc just has a bunch of defconfigs that need to always compile,
> and that's it. Long term I definitely want more, but we're just not
> there yet. And it's a problem in general for drm-misc.

Keeping the defconfigs compiling is about as much CI as I have
currently, and I suspect that dim will actually improve how I do with
this (right now I'm relying on 0day a lot).  Supposedly kernel-ci.org
also boot-tests things, but I've never seen a breakage report so I'm
pretty skeptical of it.

> - dim scripts. Since we don't have a github flow where we can
> reasonably automate stuff on the server side we need something to
> automate on the client side. Thus far almost everyone seemed ok with
> the scripting that's used to drive drm-misc/intel/tip, but we can
> always improve things. And long term we can rework the approach
> however we want to really.

I haven't tested the scripts out yet, but browsing the docs
(https://cgit.freedesktop.org/drm-intel/tree/dim.rst?h=maintainer-tools
and
https://cgit.freedesktop.org/drm-intel/tree/drm-misc.rst?h=maintainer-tools)
they seem fine for me.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  parent reply	other threads:[~2017-01-26 19:48 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-26 17:08 RFC: drm-misc for small drivers? Daniel Vetter
2017-01-26 17:42 ` Liviu Dudau
2017-01-26 19:12   ` Sean Paul
2017-01-26 20:48     ` Liviu Dudau
2017-01-27  6:55       ` Daniel Vetter
2017-01-27  9:44         ` Liviu Dudau
2017-01-26 19:57   ` Daniel Vetter
2017-01-26 20:54     ` Liviu Dudau
2017-01-30  9:30     ` Gerd Hoffmann
2017-01-30  9:49       ` Daniel Vetter
2017-01-30  9:53         ` Tomi Sarvela
2017-01-26 19:11 ` Sean Paul
2017-01-27  6:32   ` Daniel Vetter
2017-01-27 16:30     ` Sean Paul
2017-01-26 19:48 ` Eric Anholt [this message]
2017-01-27  6:52   ` Daniel Vetter
2017-01-27 16:50     ` Alex Deucher
2017-01-30  8:24       ` Daniel Vetter
2017-01-30 10:15 ` Boris Brezillon
2017-01-31  7:48   ` Daniel Vetter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8760l1siuv.fsf@eliezer.anholt.net \
    --to=eric@anholt.net \
    --cc=daniel.vetter@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.