On Mon, Aug 14, 2017 at 01:32:07PM +0300, Laurent Pinchart wrote: > On Monday 14 Aug 2017 09:25:47 Daniel Vetter wrote: > > On Sat, Aug 12, 2017 at 12:20 AM, Laurent Pinchart > > > > <laurent.pinchart@xxxxxxxxxxxxxxxx> wrote: > > > On Tuesday 25 Jul 2017 10:01:16 Daniel Vetter wrote: > > >> diff --git a/drivers/gpu/drm/drm_mode_object.c > > >> b/drivers/gpu/drm/drm_mode_object.c index da9a9adbcc98..92743a796bf0 > > >> 100644 > > >> --- a/drivers/gpu/drm/drm_mode_object.c > > >> +++ b/drivers/gpu/drm/drm_mode_object.c > > >> @@ -233,6 +233,9 @@ int drm_object_property_set_value(struct > > >> drm_mode_object *obj, { > > >> > > >> int i; > > >> > > >> + WARN_ON(drm_drv_uses_atomic_modeset(property->dev) && > > >> + !(property->flags & DRM_MODE_PROP_IMMUTABLE)); > > > > > > It would have been nice to remove the calls to > > > drm_object_property_set_value() for the dpms property from drivers before > > > adding this :-/ Three drivers (rcar- du, shmobile and fsl-dcu) initialize > > > the connector's DPMS property to OFF using this call (the default being > > > ON). > > > > > > Following the DPMS code paths always give me a headache, so if you know by > > > heart how I should replace the set property call, I'm all ears :-) > > > > Remove, it doesn't do anything for kms drivers. The value these calls > > update was never consulted when reading the property, instead > > drm_atomic_connector_get_property() directly looks at > > drm_connector->dpms. > > But DRM_IOCTL_MODE_OBJ_GETPROPERTIES doesn't, it gets the property value > directly, doesn't it ? It redirects to the atomic machinery, so should all work out. > > These calls where only needed for legacy modeset. Also note that the default > > _ON is the right one, since DPMS state is in addition to modeset state. dpms > > should actually be set to _ON when you enable the display using a modeset > > call, atomic takes care of that in > > drm_atomic_helper_update_legacy_modeset_state(). > > Sure, but the connectors should start disabled, shouldn't they ? Sure, and they are, because connector_state->crtc == NULL. DPMS only matters if your connector is connected, which at boot-up/reset state it isnt. Yes DPMS is one of those thing which with hindsight I'd like to never have happened, it's probably the most quirky uabi we have :-/ If you have userspace that gets confused by this we need to: a) either fix your userspace b) if we decide to fix the kernel, fix it for everyone (i.e. in drm_connector_init). Given that the "standard kms driver" (aka i915.ko) doesn't do that, I don't think we need b). -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel