On Tue, Jul 23, 2019 at 1:50 AM Jeykumar Sankaran <jsanka@xxxxxxxxxxxxxx> wrote: > > On 2019-07-19 07:29, Sean Paul wrote: > > 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. > > > > Agreed that a TEST_ONLY commit without ALLOW_MODESET flag can be used to > check > whether the modeset can be done seamless. But since there are no error > code returns, > the client cannot distinguish the test_only commit failures from other > invalid config failures. > > Also, note that when the client sees two 1080p modes (vid/cmd) and it is > interested only > to do *only* seamless switches, without any additional flag it cannot > distinguish between > these two 1080p modes. The client has to invoke two test_only commits > with these > modes to identify the seamless one. Is that a preferred approach? > > Intel's "fastset" calculates the need for modeset internally based on > the > configuration and chooses the best commit path. But the requirement here > is to expose the information up-front since the use case cannot afford > to fall back to the normal modeset when it has requested for a seamless > one. > > >> > >> 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 > > Yes. Pretty much those are reasons we need to switch to video mode. But > instead > of making the decision internal to the driver based on the frequency of > frame updates, > we have proprietary use cases where the client has to trigger the switch > explicitly. > So we are trying to find ways to represent the same resolution in both > video/cmd modes. If "proprietary" here means closed source or not upstream, then that's the part you need to fix first. See https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#open-source-userspace-requirements Cheers, Daniel > > Thanks and Regards, > Jeykumar S. > > > > >> > >> -- > >> Ville Syrjälä > >> Intel > > -- > Jeykumar S -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
