Re: [PATCH] drm/i915: Correctly populate user mode h/vdisplay with pipe src size during readout

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>,
	intel-gfx@lists.freedesktop.org, FadeMind <fademind@gmail.com>,
	Dave Jones <davej@codemonkey.org.uk>
Subject: Re: [PATCH] drm/i915: Correctly populate user mode h/vdisplay with pipe src size during readout
Date: Wed, 2 May 2018 19:14:21 +0300	[thread overview]
Message-ID: <20180502161421.GK23723@intel.com> (raw)
In-Reply-To: <152527662990.28472.10149027946790305934@mail.alporthouse.com>

On Wed, May 02, 2018 at 04:57:09PM +0100, Chris Wilson wrote:
> Quoting Ville Syrjälä (2018-05-02 16:52:41)
> > On Wed, May 02, 2018 at 04:33:30PM +0100, Chris Wilson wrote:
> > > Quoting Ville Syrjala (2018-04-26 17:30:15)
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > During state readout we first read out the pipe src size, store
> > > > that information in the user mode h/vdisplay, but later on we overwrite
> > > > that with the actual crtc timings. That makes our read out crtc state
> > > > inconsistent with itself when the BIOS has enabled the panel fitter to
> > > > scale the pipe contents. Let's preserve the pipe src size based
> > > > information in the user mode to make things consistent again.
> > > 
> > > The question I don't feel answered is: If this is the BIOS mode, why
> > > aren't we filling it from get_hw_state?
> > 
> > I suppose the answer is that we're only filling out the bare minimum
> > of information during the basic readout. That is everything we need
> > for intel_pipe_config_compare() to do its job. Later on we fill the
> > gaps to make the state actually presentable to userspace. We don't
> > have to do that if the state we read out isn't actually going to be
> > exposed to userspace.
> > 
> > I suppose we could consider doing a more thorough job up front, but
> > I think we'd need to spend some though on eg. the handling of the
> > mode blob. We probably wouldn't want userspace to gain access to
> > our short lived internal mode blob created from the read out state.
> 
> Will we run into a problem where we say the current mode is 800x600, but
> is in fact 1024x768 scaledfrom 800x600? E.g. if we for whatever reason
> want to switch to a real 800x600 mode?

Seems unlikely that the real 800x600 mode would have the same blanking
lengths and clock as the 1024x768 mode. So we should end up with a full
modeset.

I was actually wondering whether we should make the scaled 800x600 mode
look more like a proper 800x600 mode, ie. that the blanking lengths and 
clock would also get scaled proportionally to the h/vdisplay. That would
more closely match the "fullscreen" scaling mode, whereas the way we
do it here would match the "center" scaling mode. But I guess we generally
just have to ingore the blanking lengths when scaling is involved, so
migth as well leave the original timings in place apart from
hdisplay/vdisplay. It's somewhat unfortunate that we don't have a better
uapi than "fake the timings" for pipe scaling :(

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx