On Fri, Nov 10, 2017 at 02:02:45PM +0100, Daniel Vetter wrote: > On Fri, Nov 10, 2017 at 12:34:56PM +0100, Maarten Lankhorst wrote: > > Some parameters use CHECK_BOOLL, but should really use > > CHECK_BOOL_INCOMPLETE. We cannot currently check whether > > the inherited infoframes and audio are set up correctly. > > > > Signed-off-by: Maarten Lankhorst <[email protected]> > > --- > > drivers/gpu/drm/i915/intel_display.c | 19 +++++++++++++++++-- > > 1 file changed, 17 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > b/drivers/gpu/drm/i915/intel_display.c > > index 425167da560b..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) { \ > > @@ -11104,6 +11107,17 @@ intel_pipe_config_compare(struct drm_i915_private > > *dev_priv, > > ret = false; \ > > } > > > > Maybe add a comment here like > > /* > * Checks state where we only read out the enabling, but not the entire > * state itself (like full infoframes or ELD for audio). These states > * require a full modeset on bootup to fix up.
has_audio shouldn't need the ELD. If audio is enabled then it's enabled, otherwise it's not. So I guess if the BIOS doesn't enable audio, then what we really need is a way to enable it without a full modeset. And I can't immediately recall if we can do that or not. I'm worried that ignoring the issue will just lead to piles of bugs where people complain that their audio doesn't work. > */ > > With that: > > Reviewed-by: Daniel Vetter <[email protected]> > > > +#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), \ > > @@ -11228,10 +11242,10 @@ intel_pipe_config_compare(struct drm_i915_private > > *dev_priv, > > > > PIPE_CONF_CHECK_BOOL(hdmi_scrambling); > > PIPE_CONF_CHECK_BOOL(hdmi_high_tmds_clock_ratio); > > - PIPE_CONF_CHECK_BOOL(has_infoframe); > > + PIPE_CONF_CHECK_BOOL_INCOMPLETE(has_infoframe); > > PIPE_CONF_CHECK_BOOL(ycbcr420); > > > > - PIPE_CONF_CHECK_BOOL(has_audio); > > + PIPE_CONF_CHECK_BOOL_INCOMPLETE(has_audio); > > > > PIPE_CONF_CHECK_FLAGS(base.adjusted_mode.flags, > > DRM_MODE_FLAG_INTERLACE); > > @@ -11306,6 +11320,7 @@ 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 > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch > _______________________________________________ > 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
