| Message ID | 1470764742-20323-2-git-send-email-noralf@tronnes.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Aug 09, 2016 at 07:45:40PM +0200, Noralf Trønnes wrote: > This adds a way for in-kernel users to iterate over the available > DRM minors. The implementation is oops safe so the panic code > can safely use it. > > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> Why iterate over minors? I'd kinda have expected we'd iterate over devices instead ... And looking ahead, that seems to be what you actually want. -Daniel > --- > drivers/gpu/drm/drm_drv.c | 30 ++++++++++++++++++++++++++++++ > include/drm/drmP.h | 13 +++++++++++++ > 2 files changed, 43 insertions(+) > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > index be27ed3..3b14366 100644 > --- a/drivers/gpu/drm/drm_drv.c > +++ b/drivers/gpu/drm/drm_drv.c > @@ -292,6 +292,36 @@ void drm_minor_release(struct drm_minor *minor) > } > > /** > + * drm_minor_lookup - Lookup DRM minor > + * @minor_id: Minor ID of the DRM-minor > + * > + * Looks up the given minor-ID and returns the respective DRM-minor object. > + * No reference is taken on the underlying device. > + * See drm_minor_for_each() for iterating over all minors. > + * > + * Returns: > + * Pointer to minor-object or NULL. > + */ > +struct drm_minor *drm_minor_lookup(unsigned int minor_id) > +{ > + struct drm_minor *minor; > + unsigned long flags; > + int locked = 1; > + > + if (oops_in_progress) > + locked = spin_trylock_irqsave(&drm_minor_lock, flags); > + else > + spin_lock_irqsave(&drm_minor_lock, flags); > + > + minor = idr_find(&drm_minors_idr, minor_id); > + > + if (locked) > + spin_unlock_irqrestore(&drm_minor_lock, flags); > + > + return minor; > +} > + > +/** > * DOC: driver instance overview > * > * A device instance for a drm driver is represented by struct &drm_device. This > diff --git a/include/drm/drmP.h b/include/drm/drmP.h > index d377865..bc7006e 100644 > --- a/include/drm/drmP.h > +++ b/include/drm/drmP.h > @@ -974,6 +974,19 @@ void drm_dev_unregister(struct drm_device *dev); > > struct drm_minor *drm_minor_acquire(unsigned int minor_id); > void drm_minor_release(struct drm_minor *minor); > +struct drm_minor *drm_minor_lookup(unsigned int minor_id); > + > +/** > + * drm_minor_for_each - Iterate over DRM minors > + * @minor: DRM minor handle > + * @type: DRM minor type to iterate over > + * @id: id handle > + * > + * Iterate over the registered DRM minors of a given type. > + */ > +#define drm_minor_for_each(minor, type, id) \ > + for ((id) = 0; (id) < 64; (id)++) \ > + if (((minor) = drm_minor_lookup((id) + (type) * 64))) > > /*@}*/ > > -- > 2.8.2 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Den 10.08.2016 10:43, skrev Daniel Vetter: > On Tue, Aug 09, 2016 at 07:45:40PM +0200, Noralf Trønnes wrote: >> This adds a way for in-kernel users to iterate over the available >> DRM minors. The implementation is oops safe so the panic code >> can safely use it. >> >> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > Why iterate over minors? I'd kinda have expected we'd iterate over devices > instead ... And looking ahead, that seems to be what you actually want. I did this because I couldn't find a list of drm_devices anywhere. Only minors. But I can do a drm_for_each_device() based on the minor "list". Noralf. > -Daniel > >> --- >> drivers/gpu/drm/drm_drv.c | 30 ++++++++++++++++++++++++++++++ >> include/drm/drmP.h | 13 +++++++++++++ >> 2 files changed, 43 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c >> index be27ed3..3b14366 100644 >> --- a/drivers/gpu/drm/drm_drv.c >> +++ b/drivers/gpu/drm/drm_drv.c >> @@ -292,6 +292,36 @@ void drm_minor_release(struct drm_minor *minor) >> } >> >> /** >> + * drm_minor_lookup - Lookup DRM minor >> + * @minor_id: Minor ID of the DRM-minor >> + * >> + * Looks up the given minor-ID and returns the respective DRM-minor object. >> + * No reference is taken on the underlying device. >> + * See drm_minor_for_each() for iterating over all minors. >> + * >> + * Returns: >> + * Pointer to minor-object or NULL. >> + */ >> +struct drm_minor *drm_minor_lookup(unsigned int minor_id) >> +{ >> + struct drm_minor *minor; >> + unsigned long flags; >> + int locked = 1; >> + >> + if (oops_in_progress) >> + locked = spin_trylock_irqsave(&drm_minor_lock, flags); >> + else >> + spin_lock_irqsave(&drm_minor_lock, flags); >> + >> + minor = idr_find(&drm_minors_idr, minor_id); >> + >> + if (locked) >> + spin_unlock_irqrestore(&drm_minor_lock, flags); >> + >> + return minor; >> +} >> + >> +/** >> * DOC: driver instance overview >> * >> * A device instance for a drm driver is represented by struct &drm_device. This >> diff --git a/include/drm/drmP.h b/include/drm/drmP.h >> index d377865..bc7006e 100644 >> --- a/include/drm/drmP.h >> +++ b/include/drm/drmP.h >> @@ -974,6 +974,19 @@ void drm_dev_unregister(struct drm_device *dev); >> >> struct drm_minor *drm_minor_acquire(unsigned int minor_id); >> void drm_minor_release(struct drm_minor *minor); >> +struct drm_minor *drm_minor_lookup(unsigned int minor_id); >> + >> +/** >> + * drm_minor_for_each - Iterate over DRM minors >> + * @minor: DRM minor handle >> + * @type: DRM minor type to iterate over >> + * @id: id handle >> + * >> + * Iterate over the registered DRM minors of a given type. >> + */ >> +#define drm_minor_for_each(minor, type, id) \ >> + for ((id) = 0; (id) < 64; (id)++) \ >> + if (((minor) = drm_minor_lookup((id) + (type) * 64))) >> >> /*@}*/ >> >> -- >> 2.8.2 >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Wed, Aug 10, 2016 at 4:27 PM, Noralf Trønnes <noralf@tronnes.org> wrote: > Den 10.08.2016 10:43, skrev Daniel Vetter: >> >> On Tue, Aug 09, 2016 at 07:45:40PM +0200, Noralf Trønnes wrote: >>> >>> This adds a way for in-kernel users to iterate over the available >>> DRM minors. The implementation is oops safe so the panic code >>> can safely use it. >>> >>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> >> >> Why iterate over minors? I'd kinda have expected we'd iterate over devices >> instead ... And looking ahead, that seems to be what you actually want. > > > I did this because I couldn't find a list of drm_devices anywhere. Only > minors. > But I can do a drm_for_each_device() based on the minor "list". Hm, the drm drivers are all part of the drm class, we should be able to iterate them I think. Otherwise let's just add a new list instead of jumping through hoops. New list with it's own lock (which we then trylock from the panic handler) would be safest I think. -Daniel
Hey On Wed, Aug 10, 2016 at 4:34 PM, Daniel Vetter <daniel@ffwll.ch> wrote: > On Wed, Aug 10, 2016 at 4:27 PM, Noralf Trønnes <noralf@tronnes.org> wrote: >> Den 10.08.2016 10:43, skrev Daniel Vetter: >>> >>> On Tue, Aug 09, 2016 at 07:45:40PM +0200, Noralf Trønnes wrote: >>>> >>>> This adds a way for in-kernel users to iterate over the available >>>> DRM minors. The implementation is oops safe so the panic code >>>> can safely use it. >>>> >>>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> >>> >>> Why iterate over minors? I'd kinda have expected we'd iterate over devices >>> instead ... And looking ahead, that seems to be what you actually want. >> >> >> I did this because I couldn't find a list of drm_devices anywhere. Only >> minors. >> But I can do a drm_for_each_device() based on the minor "list". > > Hm, the drm drivers are all part of the drm class, we should be able > to iterate them I think. Otherwise let's just add a new list instead > of jumping through hoops. > New list with it's own lock (which we then trylock from the panic > handler) would be safest I think. You can use class_for_each_device() (or register a class_interface to be notified of new entries as well). However, that will iterate all minors, so you need to filter for the primary one. I think that should be sufficient, no need for a separate list. Thanks David
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index be27ed3..3b14366 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -292,6 +292,36 @@ void drm_minor_release(struct drm_minor *minor) } /** + * drm_minor_lookup - Lookup DRM minor + * @minor_id: Minor ID of the DRM-minor + * + * Looks up the given minor-ID and returns the respective DRM-minor object. + * No reference is taken on the underlying device. + * See drm_minor_for_each() for iterating over all minors. + * + * Returns: + * Pointer to minor-object or NULL. + */ +struct drm_minor *drm_minor_lookup(unsigned int minor_id) +{ + struct drm_minor *minor; + unsigned long flags; + int locked = 1; + + if (oops_in_progress) + locked = spin_trylock_irqsave(&drm_minor_lock, flags); + else + spin_lock_irqsave(&drm_minor_lock, flags); + + minor = idr_find(&drm_minors_idr, minor_id); + + if (locked) + spin_unlock_irqrestore(&drm_minor_lock, flags); + + return minor; +} + +/** * DOC: driver instance overview * * A device instance for a drm driver is represented by struct &drm_device. This diff --git a/include/drm/drmP.h b/include/drm/drmP.h index d377865..bc7006e 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -974,6 +974,19 @@ void drm_dev_unregister(struct drm_device *dev); struct drm_minor *drm_minor_acquire(unsigned int minor_id); void drm_minor_release(struct drm_minor *minor); +struct drm_minor *drm_minor_lookup(unsigned int minor_id); + +/** + * drm_minor_for_each - Iterate over DRM minors + * @minor: DRM minor handle + * @type: DRM minor type to iterate over + * @id: id handle + * + * Iterate over the registered DRM minors of a given type. + */ +#define drm_minor_for_each(minor, type, id) \ + for ((id) = 0; (id) < 64; (id)++) \ + if (((minor) = drm_minor_lookup((id) + (type) * 64))) /*@}*/
This adds a way for in-kernel users to iterate over the available DRM minors. The implementation is oops safe so the panic code can safely use it. Signed-off-by: Noralf Trønnes <noralf@tronnes.org> --- drivers/gpu/drm/drm_drv.c | 30 ++++++++++++++++++++++++++++++ include/drm/drmP.h | 13 +++++++++++++ 2 files changed, 43 insertions(+)