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