Re: [RFC] Expanding drm_mode_modeinfo flags

[Sean Paul <sean@poorly.run>]

On Fri, Jul 19, 2019 at 05:15:28PM +0300, Ville Syrjälä wrote:
> On Fri, Jul 19, 2019 at 09:55:58AM -0400, Sean Paul wrote:
> > On Fri, Jul 19, 2019 at 11:05:53AM +0200, Daniel Vetter wrote:
> > > On Thu, Jul 18, 2019 at 11:18:42AM -0700, Jeykumar Sankaran wrote:
> > > > On 2019-07-16 02:07, Daniel Vetter wrote:
> > > > > On Thu, Jul 11, 2019 at 11:46:44AM -0700, Jeykumar Sankaran wrote:

/snip

> > > > > >   drm: add mode flags in uapi for seamless mode switch
> > > > > 
> > > > > I think the uapi is the trivial part here, the real deal is how
> > > > > userspace
> > > > > uses this. Can you pls post the patches for your compositor?
> > > > > 
> > > > > Also note that we already allow userspace to tell the kernel whether
> > > > > flickering is ok or not for a modeset. msm driver could use that to at
> > > > > least tell userspace whether a modeset change is possible. So you can
> > > > > already implement glitch-free modeset changes for at least video mode.
> > > > > -Daniel
> > > > 
> > > > I believe you are referring to the below tv property of the connector.
> > > > 
> > > > /**
> > > >  * @tv_flicker_reduction_property: Optional TV property to control the
> > > >  * flicker reduction mode.
> > > >  */
> > > > struct drm_property *tv_flicker_reduction_property;
> > > 
> > > Not even close :-)
> > > 
> > > I mean the DRM_MODE_ATOMIC_ALLOW_MODESET flag for the atomic ioctl. This
> > > is not a property of a mode, this is a property of a _transition_ between
> > > configurations. Some transitions can be done flicker free, others can't.
> > 
> > Agree that an atomic flag on a commit is the way to accomplish this. It's pretty
> > similar to the psr transitions, where we want to reuse most of the atomic
> > circuitry, but in a specialized way. We'd also have to be careful to only
> > involve the drm objects which are seamless modeset aware (you could imagine
> > a bridge chain where the bridges downstream of the first bridge don't care).
> > 
> > > 
> > > There's then still the question of how to pick video vs command mode, but
> > > imo better to start with implementing the transition behaviour correctly
> > > first.
> > 
> > Connector property? Possibly a terrible idea, but I wonder if we could [re]use
> > the vrr properties for command mode. The docs state that the driver has the
> > option of putting upper and lower bounds on the refresh rate.
> 
> Not really sure why this needs new props and whatnot. This is kinda what
> the i915 "fastset" stuff already does:
> 1. userspace asks for something to be changed via atomic
> 2. driver calculates whether a modeset is actually required
> 3. atomic validates need_modeset() vs. DRM_MODE_ATOMIC_ALLOW_MODESET
> 4. if (need_modeset) heavyweight_commit() else lightweight_commit()
> 
> Ie. why should userspace really care about anything except the
> "flickers are OK" vs. "flickers not wanted" thing?

Agree, I don't think the seamless modeset (ie: changing resolution without
flicker) needs a property. Just need to test the commit without ALLOW_MODESET
and commit it if the test passes.

> 
> Also what's the benefit of using video mode if your panel supportes
> command mode? Can you turn off the memory in the panel and actually
> save power that way? And if there is a benefit can't the driver just
> automagically switch between the two based on how often things are
> getting updated? That would match how eDP PSR already works.

I'm guessing video mode might have some latency benefits over command mode?

Sean

> 
> -- 
> Ville Syrjälä
> Intel

-- 
Sean Paul, Software Engineer, Google / Chromium OS