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

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On Mon, 1 Dec 2014 17:16:25 +0100
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Mon, Dec 01, 2014 at 12:25:45PM +0200, Jani Nikula wrote:
> > On Thu, 06 Nov 2014, Jesse Barnes <jbarnes@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@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?

I'd rather just drop the infoframe check if that's causing trouble.
The audio one should probably still stay.

-- 
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx