Re: [PATCH 3/5] drm/i915: Update CRTC state if connector link status property changed

[Manasi Navare <manasi.d.navare@intel.com>]

On Fri, Nov 18, 2016 at 04:11:47PM +0200, Ville Syrjälä wrote:
> On Fri, Nov 18, 2016 at 02:50:52PM +0100, Maarten Lankhorst wrote:
> > Op 18-11-16 om 08:13 schreef Manasi Navare:
> > > CRTC state connector_changed needs to be set to true
> > > if connector link status property has changed. This will tell the
> > > driver to do a complete modeset due to change in connector property.
> > >
> > > Acked-by: Harry Wentland <harry.wentland@amd.com>
> > > Acked-by: Tony Cheng <tony.cheng@amd.com>
> > > Cc: dri-devel@lists.freedesktop.org
> > > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > > Cc: Daniel Vetter <daniel.vetter@intel.com>
> > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> > > ---
> > >  drivers/gpu/drm/drm_atomic_helper.c | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> > > index 0b16587..2125fd1 100644
> > > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > > @@ -519,6 +519,13 @@ static int handle_conflicting_encoders(struct drm_atomic_state *state,
> > >  					       connector_state);
> > >  		if (ret)
> > >  			return ret;
> > > +
> > > +		if (connector->state->crtc) {
> > > +			crtc_state = drm_atomic_get_existing_crtc_state(state,
> > > +									connector->state->crtc);
> > > +			if (connector->link_status == DRM_MODE_LINK_STATUS_BAD)
> > > +				crtc_state->connectors_changed = true;
> > > +		}
> > >  	}
> > >  
> > >  	/*
> > 
> > This will cause ordinary atomic commits that happen to change connector flags to potentially fail with -EINVAL if ALLOW_MODESET is not set.
> > For this reason I'm not sure this flag should be set automatically by the kernel. Could we add add a retrain link property instead, that
> > always return 0 when queried, but writing a 1 causing connectors_changed to be set on bad link status?
> 
> Or just check for allow_modeset before setting connectors_changed=true here?
> 
> -- 
> Ville Syrjälä
> Intel OTC

Yes I think thats a better solution, I can check ALLOW_MODESET set and only then poke the link_status and change the connectors_changed flag.
I am already checking this only if connector->state->crtc, so I thought that should avoid other atomoic commits to get into this check. But I guess
I will have to check for ALLOW MODESET as well. That should fix it.

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