Re: [PATCH 2/8] drm: Don't update property values for atomic drivers

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

On Monday 14 Aug 2017 09:25:47 Daniel Vetter wrote:
> On Sat, Aug 12, 2017 at 12:20 AM, Laurent Pinchart
> 
> <laurent.pinchart@ideasonboard.com> 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 ?

> 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 ?

> Aka we found a using this WARN_ON!
> 
> And if you think the above explanation should be somewhere in
> kerneldoc, pls add :-)

-- 
Regards,

Laurent Pinchart

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