[PATCH v2 2/3] drm: Add variable refresh property to DRM CRTC

Tue Oct 16 07:36:24 UTC 2018

On Mon, 15 Oct 2018 12:02:14 -0400
"Kazlauskas, Nicholas" <nicholas.kazlauskas at amd.com> wrote:

> On 10/15/2018 09:57 AM, Pekka Paalanen wrote:
> > On Fri, 12 Oct 2018 08:58:23 -0400
> > "Kazlauskas, Nicholas" <nicholas.kazlauskas at amd.com> wrote:
> >   
> >> On 10/12/2018 07:20 AM, Koenig, Christian wrote:  
> >>> Am 12.10.2018 um 11:21 schrieb Pekka Paalanen:  
> >>>> On Fri, 12 Oct 2018 07:35:57 +0000
> >>>> "Koenig, Christian" <Christian.Koenig at amd.com> wrote:
> >>>>     
> >>>>> Am 12.10.2018 um 09:23 schrieb Pekka Paalanen:  
> >>>>>> On Wed, 10 Oct 2018 09:35:50 -0400
> >>>>>> "Kazlauskas, Nicholas" <nicholas.kazlauskas at amd.com> wrote:
> >>>>>>         

> >> Flickering will really depend on the panel itself. A wider ranger is
> >> more likely to exhibit the issue, but factors like topology, pixel
> >> density and other display technology can affect this.
> >>
> >> It can be hard for a driver to guess at all of this. For many panels
> >> it'd be difficult to notice unless it's consistently jumping between the
> >> min and max range.  
> > 
> > Do you mean that there is no way to know and get that information from
> > the monitor itself? Nothing in EDID that could even be used as a
> > default and quirk the monitors that got it wrong?  
> 
> The variable refresh range is exposed, but that's about it. There's 
> certainly no simple algorithm you can feed monitor information into to 
> determine how bad the luminance flickering is going to look to the user.
> 
> >   
> >> Opening up an API that restricts how much the driver can change the
> >> refresh rate in a VRR scenario seems a bit extreme, but there's probably
> >> some applications that could benefit from this. I'd certainly want to
> >> see the actual use case first, though. This still feels more like a
> >> driver policy to me.  
> > 
> > I don't think anyone suggested that, certainly I did not. The driver
> > should get that information from the monitor hardware so that it can
> > drive it correctly. If the hardware driver doesn't know, then how could
> > the DRM core or userspace know any better? My whole premise was that the
> > driver knows.  
> 
> This was referring to the "How much change of frame timing is allowed 
> between frames to avoid luminescence flickering" comment. I assumed that 
> this implied restriction of the min/max VRR ranges from the driver.

Yes, exactly. The limits on the change of frame timings should be
enforced by the driver if the hardware does not do it already, so that
regardless of when userspace is scheduling flips, the monitor will
never flicker. IOW, even with on-demand rendering apps with totally
random frame timings, the kernel driver or the monitor hardware must
ensure the monitor will never luminance-flicker visibly.

Making good image quality (in this case not flickering) depend on
userspace doing something specific correctly without even a possibility
to know what "correct" is, is just silly in my opinion.

Too bad that ship sailed years ago, so we will need to have very
pessimistic kernel driver expectations on what slew rates are
acceptable. I do believe the kernel driver must enforce limits on slew
rate to prevent screen flickering if the hardware does not prevent it
already.

Besides, the kernel driver needs to know when to re-scan out the current
framebuffer in case userspace decides to take a pause, e.g. a temporary
stall in a game - compiling shaders or whatever. That should not result
in any flickering either, right? How will that be handled?

So, maybe it's not a bad idea to think of an UABI for changing the slew
rate limits. It is similar to color calibration: the defaults should be
ok to watch, but if one wants more correct color reproduction, one
needs to calibrate the monitor and load up color correction factors
through the driver. The luminance flickering should never be disturbing
by default, but maybe some people want to optimize further.

> > 
> > Sounds like VRR hardware was designed not only for a consistent refresh
> > rate but also temporary glitches being ok.
> > 
> > I suppose this will result in choosing a very pessimistic allowed slew
> > rate in the driver that covers 95% of VRR monitors and handle the rest
> > with quirks. That could still work.
> >   
> >> I agree with the nanosecond based timestamp API making the most logical
> >> sense here from an API perspective. This does overlap a little bit with
> >> the target vblank property that's already on the CRTC, perhaps.  
> > 
> > KMS UABI already has a target vblank property defined, or are you
> > talking about your CRTC hardware?
> > 
> > Target vblank counter value makes even less sense with VRR than it ever
> > did with a fixed refresh rate. :-) >  
> >> The target vblank could be determined based on the timestamp. The driver
> >> is likely to exceed the target presentation timestamp by a fair bit if
> >> it was to just do this, however. VRR could be used in this case to get
> >> closer to the timestamp. A naive implementation could iterate over every
> >> rate in the range and take the one with the minimum difference, for
> >> example.  
> > 
> > Could you elaborate on that, who could be doing what exactly to achieve
> > what?
> 
> These comments were mostly discussing about how a kernel driver might 
> approach a target presentation timestamp.
> 
> A target presentation timestamp isn't related to VRR directly. Without 
> VRR this could still be implemented by a kernel driver, but it would 
> effectively be the client asking the driver to pick the right target 
> vblank. There's already some support for this in DRM with the 
> target_vblank CRTC property, but it isn't the easiest thing to use.
> 
> The driver can do much better at getting closer to the client's target 
> presentation timestamp by adjusting the refresh rate accordingly on a 
> VRR capable monitor.

Right. An absolute timestamp should be easier to use than a target
vblank for userspace. On the kernel side it would avoid depending on
estimating vblank counts from a clock when vblank interrupts have been
opportunistically turned off, put to lower rate, or across modesets
that might mess up vblank counters. Also good for self-refreshing
panels, virtual outputs that get forwarded as video streams, etc.

We digress, though. Target timestamps or vblanks are not necessary for
the VRR feature, flip ASAP is what we have right now. The kernel driver
should adjust when ASAP is to avoid too high slew rate and luminance
flickering.

Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20181016/18f7a563/attachment.sig>