On Thu, Nov 09, 2017 at 05:24:54PM +0100, Maarten Lankhorst wrote: > Some parameters use CHECK_I, but should really use CHECK_BOOL. > Convert all boools to use CHECK_BOOL, and also create a > CHECK_BOOL_INCOMPLETE for has_audio and has_infoframe, we > cannot currently check whether the inherited infoframes and > audio are set up correctly.
Maybe split the regular BOOL think into prep patch so that we don't introduce functional changes with it? > > Also do not check ips_enabled, it will not be enabled if the > primary plane isn't. It's only a flag indicating IPS can be > enabled. This was actually causing a failure on f2-hsw-4010u > with kms_atomic_transitions. > > Signed-off-by: Maarten Lankhorst <[email protected]> > --- > drivers/gpu/drm/i915/intel_display.c | 49 > +++++++++++++++++++++++++----------- > 1 file changed, 35 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 17665ee06c9a..f5933b0719c9 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -11076,6 +11076,9 @@ intel_pipe_config_compare(struct drm_i915_private > *dev_priv, > bool adjust) > { > bool ret = true; > + bool fixup_inherited = adjust && > + (current_config->base.mode.private_flags & > I915_MODE_FLAG_INHERITED) && > + !(pipe_config->base.mode.private_flags & > I915_MODE_FLAG_INHERITED); > > #define PIPE_CONF_CHECK_X(name) \ > if (current_config->name != pipe_config->name) { \ > @@ -11095,6 +11098,26 @@ intel_pipe_config_compare(struct drm_i915_private > *dev_priv, > ret = false; \ > } > > +#define PIPE_CONF_CHECK_BOOL(name) \ > + if (current_config->name != pipe_config->name) { \ > + pipe_config_err(adjust, __stringify(name), \ > + "(expected %s, found %s)\n", \ > + yesno(current_config->name), \ > + yesno(pipe_config->name)); \ > + ret = false; \ > + } > + > +#define PIPE_CONF_CHECK_BOOL_INCOMPLETE(name) \ > + if (!fixup_inherited || (!current_config->name && !pipe_config->name)) > { \ > + PIPE_CONF_CHECK_BOOL(name); \ > + } else { \ > + pipe_config_err(adjust, __stringify(name), \ > + "unable to verify whether state matches exactly, > forcing modeset (expected %s, found %s)\n", \ > + yesno(current_config->name), \ > + yesno(pipe_config->name)); \ > + ret = false; \ > + } > + > #define PIPE_CONF_CHECK_P(name) \ > if (current_config->name != pipe_config->name) { \ > pipe_config_err(adjust, __stringify(name), \ > @@ -11180,7 +11203,7 @@ intel_pipe_config_compare(struct drm_i915_private > *dev_priv, > > PIPE_CONF_CHECK_I(cpu_transcoder); > > - PIPE_CONF_CHECK_I(has_pch_encoder); > + PIPE_CONF_CHECK_BOOL(has_pch_encoder); > PIPE_CONF_CHECK_I(fdi_lanes); > PIPE_CONF_CHECK_M_N(fdi_m_n); > > @@ -11212,17 +11235,17 @@ intel_pipe_config_compare(struct drm_i915_private > *dev_priv, > PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_vsync_end); > > PIPE_CONF_CHECK_I(pixel_multiplier); > - PIPE_CONF_CHECK_I(has_hdmi_sink); > + PIPE_CONF_CHECK_BOOL(has_hdmi_sink); > if ((INTEL_GEN(dev_priv) < 8 && !IS_HASWELL(dev_priv)) || > IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) > - PIPE_CONF_CHECK_I(limited_color_range); > + PIPE_CONF_CHECK_BOOL(limited_color_range); > > - PIPE_CONF_CHECK_I(hdmi_scrambling); > - PIPE_CONF_CHECK_I(hdmi_high_tmds_clock_ratio); > - PIPE_CONF_CHECK_I(has_infoframe); > - PIPE_CONF_CHECK_I(ycbcr420); > + PIPE_CONF_CHECK_BOOL(hdmi_scrambling); > + PIPE_CONF_CHECK_BOOL(hdmi_high_tmds_clock_ratio); > + PIPE_CONF_CHECK_BOOL_INCOMPLETE(has_infoframe); > + PIPE_CONF_CHECK_BOOL(ycbcr420); > > - PIPE_CONF_CHECK_I(has_audio); > + PIPE_CONF_CHECK_BOOL_INCOMPLETE(has_audio); > > PIPE_CONF_CHECK_FLAGS(base.adjusted_mode.flags, > DRM_MODE_FLAG_INTERLACE); > @@ -11248,7 +11271,7 @@ intel_pipe_config_compare(struct drm_i915_private > *dev_priv, > PIPE_CONF_CHECK_I(pipe_src_w); > PIPE_CONF_CHECK_I(pipe_src_h); > > - PIPE_CONF_CHECK_I(pch_pfit.enabled); > + PIPE_CONF_CHECK_BOOL(pch_pfit.enabled); > if (current_config->pch_pfit.enabled) { > PIPE_CONF_CHECK_X(pch_pfit.pos); > PIPE_CONF_CHECK_X(pch_pfit.size); > @@ -11258,11 +11281,7 @@ intel_pipe_config_compare(struct drm_i915_private > *dev_priv, > PIPE_CONF_CHECK_CLOCK_FUZZY(pixel_rate); > } > > - /* BDW+ don't expose a synchronous way to read the state */ > - if (IS_HASWELL(dev_priv)) > - PIPE_CONF_CHECK_I(ips_enabled); > - > - PIPE_CONF_CHECK_I(double_wide); > + PIPE_CONF_CHECK_BOOL(double_wide); > > PIPE_CONF_CHECK_P(shared_dpll); > PIPE_CONF_CHECK_X(dpll_hw_state.dpll); > @@ -11300,6 +11319,8 @@ intel_pipe_config_compare(struct drm_i915_private > *dev_priv, > > #undef PIPE_CONF_CHECK_X > #undef PIPE_CONF_CHECK_I > +#undef PIPE_CONF_CHECK_BOOL > +#undef PIPE_CONF_CHECK_BOOL_INCOMPLETE > #undef PIPE_CONF_CHECK_P > #undef PIPE_CONF_CHECK_FLAGS > #undef PIPE_CONF_CHECK_CLOCK_FUZZY > -- > 2.15.0 > > _______________________________________________ > Intel-gfx mailing list > [email protected] > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
