[Freedreno] [RFC] Expanding drm_mode_modeinfo flags

[Daniel Vetter]

On Tue, Jul 23, 2019 at 1:50 AM Jeykumar Sankaran <jsanka at codeaurora.org> 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