(sending again, I keep missing the reply-all in gmail.)
On Wed, Jun 26, 2019 at 11:56 AM Daniel Vetter <daniel@xxxxxxxx> wrote:
>
> Why the separate, explicit registration step? I think a simpler design for
> drivers would be that we set up cgroups if there's anything to be
> controlled, and then for GEM drivers the basic GEM stuff would be set up
> automically (there's really no reason not to I think).
Is this what you mean with the comment about drm_dev_register below?
I think I understand what you are saying but not super clear. Are you
suggesting the use of driver feature bits (drm_core_check_feature,
etc.) similar to the way Brian Welty did in his proposal in May?
> Also tying to the minor is a bit funky, since we have multiple of these.
> Need to make sure were at least consistent with whether we use the primary
> or render minor - I'd always go with the primary one like you do here.
Um... come to think of it, I can probably embed struct drmcgrp_device
into drm_device and that way I don't really need to keep a separate
array of
known_drmcgrp_devs and get rid of that max_minor thing. Not sure why
I didn't think of this before.
> > +
> > +int drmcgrp_register_device(struct drm_device *dev)
>
> Imo this should be done as part of drm_dev_register (maybe only if the
> driver has set up a controller or something). Definitely with the
> unregister logic below. Also anything used by drivers needs kerneldoc.
>
>
> > + /* init cgroups created before registration (i.e. root cgroup) */
> > + if (root_drmcgrp != NULL) {
> > + struct cgroup_subsys_state *pos;
> > + struct drmcgrp *child;
> > +
> > + rcu_read_lock();
> > + css_for_each_descendant_pre(pos, &root_drmcgrp->css) {
> > + child = css_drmcgrp(pos);
> > + init_drmcgrp(child, dev);
> > + }
> > + rcu_read_unlock();
>
> I have no idea, but is this guaranteed to get them all?
I believe so, base on my understanding about
css_for_each_descendant_pre and how I am starting from the root
cgroup. Hopefully I didn't miss anything.
Regards,
Kenny
