On Mon, Dec 01, 2014 at 04:09:07PM +0000, Chris Wilson wrote:
> On Mon, Dec 01, 2014 at 08:04:30AM -0800, Jesse Barnes wrote:
> > On Mon, 01 Dec 2014 12:25:45 +0200
> > Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> wrote:
> >
> > > On Thu, 06 Nov 2014, Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxx>
> > > > ---
> > > > 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.
>
> We need to be conservative. If we spot extraneous modesets in the wild,
> we will be motivated to fix it. As a compromise, if you include a
>
> DRM_DEBUG_KMS("infoframes detected - forcing full modeset\n")
>
> that should be sufficient clue for us to pick up the thread again later.
The problem is that with the current code we'll do a modeset for all hdmi
outputs, unconditionally. Even without fastboot=1 set. Which is too much
and a regression. E.g. just having virtual > screen and scrolling with the
mouse old-style would now result in a modeset each time we move one pixel.
Or at least that's my analysis here (didnt test).
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
