[Intel-gfx] [PATCH RFC v3] drm: Add a new connector atomic property for link status

[Daniel Vetter]

On Mon, Nov 28, 2016 at 05:07:52PM -0800, Manasi Navare wrote:
> On Thu, Nov 24, 2016 at 08:51:35AM +0100, Daniel Vetter wrote:
> > On Wed, Nov 23, 2016 at 11:28:21PM -0800, Manasi Navare wrote:
> > > This is RFC patch for adding a connector link-status property
> > > and making it atomic by adding it to the drm_connector_state.
> > > This is to make sure its wired properly in drm_atomic_connector_set_property
> > > and drm_atomic_connector_get_property functions.
> > > 
> > > v3:
> > > * Fixed a build error (Jani Saarinen)
> > > v2:
> > > * Removed connector->link_status (Daniel Vetter)
> > > * Set connector->state->link_status in drm_mode_connector_set_link_status_property
> > > (Daniel Vetter)
> > > * Set the connector_changed flag to true if connector->state->link_status changed.
> > > * Reset link_status to GOOD in update_output_state (Daniel Vetter)
> > > * Never allow userspace to set link status from Good To Bad (Daniel Vetter)
> > > 
> > > Cc: Jani Nikula <jani.nikula at linux.intel.com>
> > > Cc: Daniel Vetter <daniel.vetter at intel.com>
> > > Cc: Ville Syrjala <ville.syrjala at linux.intel.com>
> > > Cc: Chris Wilson <chris at chris-wilson.co.uk>
> > > Cc: Sean Paul <seanpaul at chromium.org>
> > 
> > You lost all the acked-by from AMD about the link-status property. We need
> > those.
> > 
> > > Signed-off-by: Manasi Navare <manasi.d.navare at intel.com>
> > 
> > Yeah I think this should work, but obviously testing has the final say.
> > Some nitpicks below, then it's r-b: me. But I think we also need to polish
> > the kernel-doc a bit more to address Sean Paul's questions.
> > -Daniel
> >
> 
> I tested it with SNA driver with Chris's changes to read the link-status
> property, but It is not able to detect the link-status property being set to BAD
> and hence it does not trigger a new modeset.
> Do we need to make any changes to the SNA driver now that this is made a ATOMIC
> property so that GETCONNECTOR IOCTL can still read the correct value
> of this property throught drm_atomic_get_property() interface?
> 
> Chris, Daniel, any thoughts? 

We agreed that it must _not_ be a PROPERTY_ATOMIC property, so that old
userspace can see it.

> > > @@ -666,6 +683,13 @@ int drm_connector_create_standard_properties(struct drm_device *dev)
> > >  		return -ENOMEM;
> > >  	dev->mode_config.tile_property = prop;
> > >  
> > > +	prop = drm_property_create_enum(dev, DRM_MODE_PROP_ATOMIC, "link-status",

I.e. remote DRM_MODE_PROP_ATOMIC here. I thought we've discussed this a
lot already ...?

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch