[Intel-gfx] [PATCH 4/6] drm/i915: check for audio and infoframe changes across mode sets v2

[Daniel Vetter]

On Mon, Dec 01, 2014 at 12:25:45PM +0200, Jani Nikula wrote:
> On Thu, 06 Nov 2014, Jesse Barnes <jbarnes at virtuousgeek.org> wrote:
> > If these change (e.g. after a modeset following a fastboot), we need to
> > do a full mode set.
> >
> > v2:
> >   - put under pipe_config check so we don't deref a null state (Jesse)
> >
> > Signed-off-by: Jesse Barnes <jbarnes at virtuousgeek.org>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 12 +++++++++++-
> >  1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index cb96f11..c86eee6 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -11230,8 +11230,18 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
> >  						   &modeset_pipes,
> >  						   &prepare_pipes,
> >  						   &disable_pipes);
> > -	if (IS_ERR(pipe_config))
> > +	if (IS_ERR(pipe_config)) {
> >  		goto fail;
> > +	} else if (pipe_config) {
> > +		if (to_intel_crtc(set->crtc)->new_config->has_audio !=
> > +		    to_intel_crtc(set->crtc)->config.has_audio)
> > +			config->mode_changed = true;
> > +
> > +		/* Force mode sets for any infoframe stuff */
> > +		if (to_intel_crtc(set->crtc)->new_config->has_infoframe ||
> > +		    to_intel_crtc(set->crtc)->config.has_infoframe)
> 
> Jesse, is "||" correct here? This is bound to force a lot of mode sets.
> 
> See https://bugs.freedesktop.org/show_bug.cgi?id=86683

Indeed. And I think the approach is the wrong way round. Imo we should
check whether the computed pipe_config matches and have a few specific
cases that we can handle when it mismatches (e.g. pipe_src_w/h). Plus some
special logic if quirk flags are set (so that we force a full modeset in
cases like this to get rid of the boot-up takever state quirk flag).

I think best course of action here is to revert this patch, also because
it's kinda at the 1w deadline with set for regression. Even though this
here is imo QA's fault for taking way too long to deliver the bisect
result and noticing the issue (the patch is almost 1 month old by now).

Jesse?

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch