| Message ID | 1390467164-951-20-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi On Thu, Jan 23, 2014 at 9:52 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > Driver modules really don't have much business frobbing around with > this: Splitting up modeset resources (if we ever get around to enable > this for real) should be something purely controlled and managed by > the drm core in a driver-agnostic fashion. As usual imx disagrees, but > that's due to the convoluted setup sequence. Russell King is working > on a proper fix for that, so this patch needs to wait for those to > land to avoid breaking imx. I tried working on "modeset-object splitting" as part of the DRM-Master rework, but that's all really racy and we'd break current user-space. For instance the object-indices (compared to object-IDs) need to be constant, but with our current object-lists, removing an object will change indices (thus breaking possible_crtcs/encoders). There are ways to make that work, but it'd require huge drm_crtc.c changes. And I also think our locking doesn't provide for that. Furthermore, resource-retrieval is not atomic (we need multiple ioctls: GetResources, GetCrtc, ...) so if we change the objects in between, we may break running apps in a subtle way. So I'm all for making object-lists immutable after drm_dev_register() was called. Imo, hardware should wait for all sub-devices to be present and then call drm_dev_register(). Once the first sub-device is removed, you call drm_dev_unregister(). And I think that's what Russel's compound-device infrastructure does by hiding the sub-devices in a compound device. If there's ever hardware that truly supports sub-device hotplugging, we can support that by adding a DRM-ClientCap like 3D modes. But as I understand, the current hardware is still a single piece of hardware which just might get probed via different buses and thus is not an atomic entity. So the device is still hotplugged as a whole, we just don't get the events through a single path. Thanks David > > Cc: Sascha Hauer <s.hauer@pengutronix.de> > Cc: Russell King <rmk+kernel@arm.linux.org.uk> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > drivers/gpu/drm/drm_crtc.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index cf134540b675..98084413a842 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -1264,7 +1264,6 @@ int drm_mode_group_init_legacy_group(struct drm_device *dev, > > return 0; > } > -EXPORT_SYMBOL(drm_mode_group_init_legacy_group); > > /** > * drm_crtc_convert_to_umode - convert a drm_display_mode into a modeinfo > -- > 1.8.5.2 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
On Thu, Jan 23, 2014 at 10:42:02AM +0100, David Herrmann wrote: > Hi > > On Thu, Jan 23, 2014 at 9:52 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > > Driver modules really don't have much business frobbing around with > > this: Splitting up modeset resources (if we ever get around to enable > > this for real) should be something purely controlled and managed by > > the drm core in a driver-agnostic fashion. As usual imx disagrees, but > > that's due to the convoluted setup sequence. Russell King is working > > on a proper fix for that, so this patch needs to wait for those to > > land to avoid breaking imx. > > I tried working on "modeset-object splitting" as part of the > DRM-Master rework, but that's all really racy and we'd break current > user-space. For instance the object-indices (compared to object-IDs) > need to be constant, but with our current object-lists, removing an > object will change indices (thus breaking possible_crtcs/encoders). > > There are ways to make that work, but it'd require huge drm_crtc.c > changes. And I also think our locking doesn't provide for that. > Furthermore, resource-retrieval is not atomic (we need multiple > ioctls: GetResources, GetCrtc, ...) so if we change the objects in > between, we may break running apps in a subtle way. > > So I'm all for making object-lists immutable after drm_dev_register() > was called. Imo, hardware should wait for all sub-devices to be > present and then call drm_dev_register(). Once the first sub-device is > removed, you call drm_dev_unregister(). And I think that's what > Russel's compound-device infrastructure does by hiding the sub-devices > in a compound device. > > If there's ever hardware that truly supports sub-device hotplugging, > we can support that by adding a DRM-ClientCap like 3D modes. But as I > understand, the current hardware is still a single piece of hardware > which just might get probed via different buses and thus is not an > atomic entity. So the device is still hotplugged as a whole, we just > don't get the events through a single path. Yeah, if we ever want to allow hotplugging after driver setup that's massive work to do it right. Which is why I want to plug this hole here before someone sneaks in for real ;-) -Daniel
On Thu, Jan 23, 2014 at 11:00:28AM +0100, Daniel Vetter wrote: > On Thu, Jan 23, 2014 at 10:42:02AM +0100, David Herrmann wrote: > > If there's ever hardware that truly supports sub-device hotplugging, > > we can support that by adding a DRM-ClientCap like 3D modes. But as I > > understand, the current hardware is still a single piece of hardware > > which just might get probed via different buses and thus is not an > > atomic entity. So the device is still hotplugged as a whole, we just > > don't get the events through a single path. > > Yeah, if we ever want to allow hotplugging after driver setup that's > massive work to do it right. Which is why I want to plug this hole here > before someone sneaks in for real ;-) David did say at the kernel summit that he's not interested at all in hotplugging the components of a DRM system, and his statement appeared to be very clear in that regard that it was never going to happen.
On Thu, Jan 23, 2014 at 10:05:19AM +0000, Russell King - ARM Linux wrote: > On Thu, Jan 23, 2014 at 11:00:28AM +0100, Daniel Vetter wrote: > > On Thu, Jan 23, 2014 at 10:42:02AM +0100, David Herrmann wrote: > > > If there's ever hardware that truly supports sub-device hotplugging, > > > we can support that by adding a DRM-ClientCap like 3D modes. But as I > > > understand, the current hardware is still a single piece of hardware > > > which just might get probed via different buses and thus is not an > > > atomic entity. So the device is still hotplugged as a whole, we just > > > don't get the events through a single path. > > > > Yeah, if we ever want to allow hotplugging after driver setup that's > > massive work to do it right. Which is why I want to plug this hole here > > before someone sneaks in for real ;-) > > David did say at the kernel summit that he's not interested at all in > hotplugging the components of a DRM system, and his statement appeared > to be very clear in that regard that it was never going to happen. I think we eventually need to be able to hot-plug connectors (and maybe encoders) to support fancy dp1.2 output routing and multiple streams over the same cable. But I agree that hotplugging crtcs is insane and better done by hotplugging an entire drm device with that crtc. In any case not something to worry about right now. -Daniel
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index cf134540b675..98084413a842 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -1264,7 +1264,6 @@ int drm_mode_group_init_legacy_group(struct drm_device *dev, return 0; } -EXPORT_SYMBOL(drm_mode_group_init_legacy_group); /** * drm_crtc_convert_to_umode - convert a drm_display_mode into a modeinfo
Driver modules really don't have much business frobbing around with this: Splitting up modeset resources (if we ever get around to enable this for real) should be something purely controlled and managed by the drm core in a driver-agnostic fashion. As usual imx disagrees, but that's due to the convoluted setup sequence. Russell King is working on a proper fix for that, so this patch needs to wait for those to land to avoid breaking imx. Cc: Sascha Hauer <s.hauer@pengutronix.de> Cc: Russell King <rmk+kernel@arm.linux.org.uk> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/drm_crtc.c | 1 - 1 file changed, 1 deletion(-)