[Intel-gfx] [PATCH] drm/i915: Check for I915_MODE_FLAG_INHERITED before drm_atomic_helper_check_modeset

[Lofstedt, Marta]

There is no open cibuglog bug for below issue. However, if a developer believe that patch didn't cause the issue, they can just mail or ping me on IRC and I'll create a cibuglog issue and FDO bug to cover it. 

In developers we trust.

/Marta

> -----Original Message-----
> From: Jani Nikula [mailto:jani.nikula at linux.intel.com]
> Sent: Friday, March 2, 2018 11:37 AM
> To: Lyude Paul <lyude at redhat.com>; Maarten Lankhorst
> <maarten.lankhorst at linux.intel.com>; intel-gfx at lists.freedesktop.org
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>; Lofstedt, Marta
> <marta.lofstedt at intel.com>; Hiler, Arkadiusz <arkadiusz.hiler at intel.com>;
> Sarvela, Tomi P <tomi.p.sarvela at intel.com>; Martin Peres
> <martin.peres at linux.intel.com>
> Subject: Re: [Intel-gfx] [PATCH] drm/i915: Check for
> I915_MODE_FLAG_INHERITED before drm_atomic_helper_check_modeset
> 
> On Thu, 01 Mar 2018, Lyude Paul <lyude at redhat.com> wrote:
> > Pushed with some small whitespace changes to make sparse happy,
> thanks!
> 
> Please do not push patches before they've passed CI. This patch gives [1]:
> 
> [  281.167033] i915 0000:00:02.0: DP-2: EDID is invalid:
> ...
> [  282.806393] [drm:intel_enable_shared_dpll [i915]] *ERROR* DPLL 1 not
> locked
> 
> I don't know if this is caused by the patch, but since we get this in the BAT
> round, the full IGT testing wasn't even run here.
> 
> 
> BR,
> Jani.
> 
> 
> [1] https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8102/fi-skl-
> 6700k2/igt at kms_chamelium@common-hpd-after-suspend.html
> >
> > On Wed, 2018-02-21 at 10:28 +0100, Maarten Lankhorst wrote:
> >> Moving the check upwards will mean we we no longer have to add planes
> >> and connectors manually, because everything is handled correctly by
> >> drm_atomic_helper_check_modeset() as intended.
> >>
> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> >> Cc: Lyude Paul <lyude at redhat.com>
> >> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> >> Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> >> ---
> >>  drivers/gpu/drm/i915/intel_display.c | 20 +++++---------------
> >>  1 file changed, 5 insertions(+), 15 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_display.c
> >> b/drivers/gpu/drm/i915/intel_display.c
> >> index 65be7af7f647..c5cc9022d545 100644
> >> --- a/drivers/gpu/drm/i915/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/intel_display.c
> >> @@ -11927,6 +11927,11 @@ static int intel_atomic_check(struct
> >> drm_device *dev,
> >>  	int ret, i;
> >>  	bool any_ms = false;
> >>
> >> +	/* Catch I915_MODE_FLAG_INHERITED */
> >> +	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state,
> >> crtc_state, i)
> >> +		if (crtc_state->mode.private_flags !=
> old_crtc_state-
> >> >mode.private_flags)
> >> +			crtc_state->mode_changed =
> true;
> >> +
> >>  	ret = drm_atomic_helper_check_modeset(dev, state);
> >>  	if (ret)
> >>  		return ret;
> >> @@ -11935,10 +11940,6 @@ static int intel_atomic_check(struct
> >> drm_device *dev,
> >>  		struct intel_crtc_state *pipe_config =
> >>  			to_intel_crtc_state(crtc_state);
> >>
> >> -		/* Catch I915_MODE_FLAG_INHERITED */
> >> -		if (crtc_state->mode.private_flags !=
> old_crtc_state-
> >> >mode.private_flags)
> >> -			crtc_state->mode_changed =
> true;
> >> -
> >>  		if (!needs_modeset(crtc_state))
> >>  			continue;
> >>
> >> @@ -11947,13 +11948,6 @@ static int intel_atomic_check(struct
> >> drm_device *dev,
> >>  			continue;
> >>  		}
> >>
> >> -		/* FIXME: For only active_changed we shouldn't
> need to do
> >> any
> >> -		 * state recomputation at all. */
> >> -
> >> -		ret =
> drm_atomic_add_affected_connectors(state, crtc);
> >> -		if (ret)
> >> -			return ret;
> >> -
> >>  		ret = intel_modeset_pipe_config(crtc,
> pipe_config);
> >>  		if (ret) {
> >>
> 	intel_dump_pipe_config(to_intel_crtc(crtc),
> >> @@ -11972,10 +11966,6 @@ static int intel_atomic_check(struct
> >> drm_device *dev,
> >>  		if (needs_modeset(crtc_state))
> >>  			any_ms = true;
> >>
> >> -		ret = drm_atomic_add_affected_planes(state,
> crtc);
> >> -		if (ret)
> >> -			return ret;
> >> -
> >>  		intel_dump_pipe_config(to_intel_crtc(crtc),
> pipe_config,
> >>
> needs_modeset(crtc_state) ?
> >>  				       "[modeset]" :
> "[fastset]");
> 
> --
> Jani Nikula, Intel Open Source Technology Center