On Wed, 09 Aug 2023, "Lisovskiy, Stanislav" <stanislav.lisovskiy@xxxxxxxxx> wrote:
> On Wed, Aug 09, 2023 at 11:38:08AM +0300, Jani Nikula wrote:
>> On Wed, 09 Aug 2023, Stanislav Lisovskiy <stanislav.lisovskiy@xxxxxxxxx> wrote:
>> > It is supposed to be "!intel_crtc_needs_modeset" - otherwise,
>> > we are active exactly vice versa for active pipes: skipping if modeset
>> > is needed and not skipping if not needed.
>>
>> If the crtc *already* needs a full modeset, there's no need to force a
>> modeset on it.
>>
>> BR,
>> Jani.
>
> We have curently some issue with that. There are multiple places from where
> intel_modeset_all_pipes is called. One is that when we do detect that mbus join
> state had changed. All the previous checks indicated that fastset is enough,
> however once we detect mbus join state had changed in skl_watermarks.c we call
> this function there as well and I think it might act in a wrong way then.
>
> So basically this condition checks whether we need to force a modeset, but not
> if we need it, so no crtc's are supposed to escape this?
> Could be then that we just calling that whole function there wrongly.
Simplified, it's an optimization of:
if (crtc_state->uapi.mode_changed)
continue;
crtc_state->uapi.mode_changed = true;
With your change, it becomes:
if (!crtc_state->uapi.mode_changed)
continue;
crtc_state->uapi.mode_changed = true;
and intel_modeset_all_pipes() roughly becomes a no-op.
BR,
Jani.
>
> Stan
>
>>
>> >
>> > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@xxxxxxxxx>
>> > ---
>> > drivers/gpu/drm/i915/display/intel_display.c | 2 +-
>> > 1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>> > index 763ab569d8f32..4b1d7034078ca 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_display.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
>> > @@ -5439,7 +5439,7 @@ int intel_modeset_all_pipes(struct intel_atomic_state *state,
>> > return PTR_ERR(crtc_state);
>> >
>> > if (!crtc_state->hw.active ||
>> > - intel_crtc_needs_modeset(crtc_state))
>> > + !intel_crtc_needs_modeset(crtc_state))
>> > continue;
>> >
>> > drm_dbg_kms(&dev_priv->drm, "[CRTC:%d:%s] Full modeset due to %s\n",
>>
>> --
>> Jani Nikula, Intel Open Source Graphics Center
--
Jani Nikula, Intel Open Source Graphics Center
