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

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On Mon, 01 Dec 2014 12:25:45 +0200
Jani Nikula <jani.nikula@linux.intel.com> 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

Yeah, we talked about it on IRC.  I was being extra conservative here,
since we don't yet have code to analyze whether we need a full mode set
due to something that will require an infoframe change.  Currently, the
only way for us to write a new infoframe is through a mode set, and the
old code would just happen to do a mode set most of the time we wanted
it to (i.e. when the mode timings changed), but I don't think we'd ever
thought of the implications of infoframe changes for stuff that doesn't
require a mode set.

We could drop this hunk and continue to play it fast & loose, but what
we really need here is a fuller check about whether we need a mode set
due to requiring an infoframe change, which means pulling them apart a
bit.

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