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

[Ville Syrjälä]

On Fri, Nov 18, 2016 at 03:18:06PM +0100, Maarten Lankhorst wrote:
> Op 18-11-16 om 15:11 schreef Ville Syrjälä:
> > 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 at amd.com>
> >>> Acked-by: Tony Cheng <tony.cheng at amd.com>
> >>> Cc: dri-devel at lists.freedesktop.org
> >>> 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>
> >>> Signed-off-by: Manasi Navare <manasi.d.navare at 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?
> 
> I don't think modesets should be done automatically like that, even if ALLOW_MODESET is set a modeset may not be expected by userspace.

Presumably userspace would want a picture on the screen using any means
if it said ALLOW_MODESET. So if it can't tolerate the modeset it should
probably say as much?

-- 
Ville Syrjälä
Intel OTC