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

[Manasi Navare]

On Tue, Nov 29, 2016 at 09:59:14AM +0100, Daniel Vetter wrote:
> 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

Hi Daniel,

Yes this is where I was confused in the beginning that whether we need the flag.
Ok so this property will not have atomic flag, so drm-mode_object_get_properties should
call the drm_object_property_get_value() and that should expose
the value to userspace right?

Manasi