Re: [PATCH] drm/i915: Enable fastset by default, except on initial modeset
Hi,
On 17-12-18 15:29, Daniel Vetter wrote:
On Mon, Dec 17, 2018 at 3:23 PM Hans de Goede wrote:
From: Maarten Lankhorst
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
Signed-off-by: Hans de Goede
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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/i915: Enable fastset by default, except on initial modeset
On Mon, Dec 17, 2018 at 3:23 PM Hans de Goede wrote:
>
> From: Maarten Lankhorst
>
> 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
> Signed-off-by: Hans de Goede
Do we have igts which exercise this functionality? 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.
-Daniel
> ---
> 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
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
