Re: [PATCH] drm/i915: Enable fastset by default, except on initial modeset

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 17-12-18 15:29, Daniel Vetter wrote:
> On Mon, Dec 17, 2018 at 3:23 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>
>> We may not be perfect at reading out the initial mode, but when the mode
>> is sanitized by our first modeset we know for sure the state is compatible,
>> so we can compare with intel_pipe_config_compare.
>>
>> Changes in v2 (Hans de Goede):
>> -When checking if we need to reset the mode (initial modeset), check
>>   I915_MODE_FLAG_INHERITED is set in private_flags instead of
>>   private_flags != 0
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> 
> Do we have igts which exercise this functionality?

I'm not sure, but last time Maarten submitted this patch it caused
a bunch of new dmesg warnings CI failures, which seems to indicate that
at least some tests are hitting the new code path where we do a fastset
independent of the fastboot=x parameter.

> Having that is
> kinda the point of enabling this first only for non-bootup modesets,
> so that we can weed out the bugs. I think most (maybe even all) igts
> we have do their modesets by explicitly shutting things down, and so
> will never hit this. But we have lots of igts, so might have missed
> some.
> 
> Anyway, patch imo should have a clear indication which igts we need to
> watch out for this one here.

Hmm, Maarten, can you provide a list of relevant igts to add to the
commit message ?

Regards,

Hans




>> ---
>>   drivers/gpu/drm/i915/intel_display.c | 23 +++++++++++++++++++----
>>   1 file changed, 19 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index c2980643a1a5..071954d255c6 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -12501,6 +12501,22 @@ static int calc_watermark_data(struct drm_atomic_state *state)
>>          return 0;
>>   }
>>
>> +static bool can_fastset(struct drm_i915_private *dev_priv,
>> +                       struct intel_crtc_state *old_crtc_state,
>> +                       struct intel_crtc_state *new_crtc_state)
>> +{
>> +       bool reset_mode =
>> +               (old_crtc_state->base.mode.private_flags & I915_MODE_FLAG_INHERITED) &&
>> +               !(new_crtc_state->base.mode.private_flags & I915_MODE_FLAG_INHERITED);
>> +
>> +       /* Without fastboot, we always want to modeset the initial mode. */
>> +       if (reset_mode && !i915_modparams.fastboot)
>> +               return false;
>> +
>> +       return intel_pipe_config_compare(dev_priv, old_crtc_state,
>> +                                        new_crtc_state, true);
>> +}
>> +
>>   /**
>>    * intel_atomic_check - validate state object
>>    * @dev: drm device
>> @@ -12531,6 +12547,8 @@ static int intel_atomic_check(struct drm_device *dev,
>>          for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, crtc_state, i) {
>>                  struct intel_crtc_state *pipe_config =
>>                          to_intel_crtc_state(crtc_state);
>> +               struct intel_crtc_state *old_intel_crtc_state =
>> +                       to_intel_crtc_state(old_crtc_state);
>>
>>                  if (!needs_modeset(crtc_state))
>>                          continue;
>> @@ -12547,10 +12565,7 @@ static int intel_atomic_check(struct drm_device *dev,
>>                          return ret;
>>                  }
>>
>> -               if (i915_modparams.fastboot &&
>> -                   intel_pipe_config_compare(dev_priv,
>> -                                       to_intel_crtc_state(old_crtc_state),
>> -                                       pipe_config, true)) {
>> +               if (can_fastset(dev_priv, old_intel_crtc_state, pipe_config)) {
>>                          crtc_state->mode_changed = false;
>>                          pipe_config->update_pipe = true;
>>                  }
>> --
>> 2.20.1
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> 
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx