Re: [Intel-gfx] [PATCH] drm/i915: Check for I915_MODE_FLAG_INHERITED before drm_atomic_helper_check_modeset
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.nik...@linux.intel.com] > Sent: Friday, March 2, 2018 11:37 AM > To: Lyude Paul ; Maarten Lankhorst > ; intel-gfx@lists.freedesktop.org > Cc: Daniel Vetter ; Lofstedt, Marta > ; Hiler, Arkadiusz ; > Sarvela, Tomi P ; Martin Peres > > 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 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 :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@kms_chamel...@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 > >> Cc: Lyude Paul > >> Cc: Daniel Vetter > >> Reviewed-by: Daniel Vetter > >> --- > >> 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 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
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 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 :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@kms_chamel...@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 >> Cc: Lyude Paul >> Cc: Daniel Vetter >> Reviewed-by: Daniel Vetter >> --- >> 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 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Check for I915_MODE_FLAG_INHERITED before drm_atomic_helper_check_modeset
Pushed with some small whitespace changes to make sparse happy, thanks! 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 > Cc: Lyude Paul > Cc: Daniel Vetter > Reviewed-by: Daniel Vetter > --- > 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]"); -- Cheers, Lyude Paul ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Check for I915_MODE_FLAG_INHERITED before drm_atomic_helper_check_modeset
Nice, this is a no-brainer Reviewed-by: Lyude Paul 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 > Cc: Lyude Paul > Cc: Daniel Vetter > Reviewed-by: Daniel Vetter > --- > 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]"); -- Cheers, Lyude Paul ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Check for I915_MODE_FLAG_INHERITED before drm_atomic_helper_check_modeset
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 Cc: Lyude Paul Cc: Daniel Vetter Reviewed-by: Daniel Vetter --- 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]"); -- 2.16.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx