Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
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

  reply	other threads:[~2018-05-02 16:14 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-26 16:30 [PATCH] drm/i915: Correctly populate user mode h/vdisplay with pipe src size during readout Ville Syrjala
2018-04-27  8:29 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2018-04-27  8:47 ` ✓ Fi.CI.BAT: success " Patchwork
2018-04-27 11:08 ` ✓ Fi.CI.IGT: " Patchwork
2018-05-02 15:33 ` [PATCH] " Chris Wilson
2018-05-02 15:52   ` Ville Syrjälä
2018-05-02 15:57     ` Chris Wilson
2018-05-02 16:14       ` Ville Syrjälä [this message]
2018-05-02 16:23         ` Chris Wilson
2018-05-03  6:50           ` Jani Nikula
2018-05-03 15:11             ` Ville Syrjälä

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180502161421.GK23723@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=chris@chris-wilson.co.uk \
    --cc=daniel.vetter@ffwll.ch \
    --cc=davej@codemonkey.org.uk \
    --cc=fademind@gmail.com \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox