On 1/22/2025 6:38 PM, Ville Syrjälä wrote:
On Wed, Jan 22, 2025 at 04:08:07PM +0530, Nautiyal, Ankit K wrote:
On 12/13/2024 11:01 PM, Ville Syrjälä wrote:
On Mon, Nov 11, 2024 at 02:41:58PM +0530, Ankit Nautiyal wrote:
Even though the VRR timing generator (TG) is primarily used for
variable refresh rates, it can be used for fixed refresh rates as
well. For a fixed refresh rate the Flip Line and Vmax must be equal
(TRANS_VRR_FLIPLINE = TRANS_VRR_VMAX). Beyond that, there are some
dependencies between the VRR timings and the legacy timing generator
registers.
This series is an attempt to use VRR TG for fixed refresh rate.
For platforms XE2LPD+, always go with VRR timing generator for both fixed and
variable refresh rate cases.
Some ideas I had while thinking about this:
- perhaps the most fundemental decision we have to make is how do
we compute the vrr state in each case. My first idea was to
stick to the current way of always computing it as if vrr is enabled,
but that complicates the state checker needlessly, so I'm actually
leaning towards always computing the vrr state based on actual
uapi.vrr_enable knob. So when that knob is disabled we always compute
vmin=flipline=vmax. We're going to anyway have to repgrogram all those
registers when toggle VRR anyway.
- intel_vrr_{enable,disable}() should just flip between the fixed and
variable timings in vmin/flipline/vmax. I think we should just do this
for all the platforms, regarless of whether we also toggle the VRR_CTL
enable bit there. This minimizes the differences between the two
approaches fairly well, and it should also work well with the previous
idea of computing the vrr state based on uapi.vrr_enable. I'm still
a bit uneasy wrt. repgramming the timings on the fly since none of
the registers are double buffered. So not entirely sure if we need
to spend some brain cells on coming up with some kind of safe order
of writing the registers or not.
- guardbad/pipeline full probably can't be reprogrammed on the fly,
so we need to make sure it satisfies both the fixed and variable
timings. I think we should probably just always set vmin=crtc_vtotal
instead of the using the current refresh rate based approach. That
way we never need to change anything to do with the guardband.
- I was initially thinking we chould jsue the PUSH_EN bit to
differentiate between variable and fixed timings, but at least tgl
and adl refuse to play ball and the display just blanks out if you
attempt to use the VRR timing generator without enabling push.
So probably we'll just need make the distinction based on
flipline==vmax.
- cmrr I've not really though about. It'll still involve frobbing the
VRR_CTL and whatnot (which I think might be double buffered unlike
all the other VRR registers). So not sure how to make sure the changes
beween the modes are done safely. I think easiest to just never use
cmrr for now, and we can figure it out later.
So maybe something along the lines of:
1) do the vmin==crtc_vtotal change
2) neuter cmrr
2) change the state computation and
add vmin/vmax/flipline reprogramming to vrr_{enable,disable}()
(need to make sure we can still do the fastset vrr toggle)
I am getting some confusion around the vrr_enable and vrr_disable
functions, with respect to platforms prior to MTL.
For prior platforms TGL-ADL, where we do not want to have fixed timings,
what happens to them when uapi.vrr_enable is not set.
Do we intend to use the same logic as was earlier where we compute vrr
timings and write them anyway without enabling VRR?
I'm thinking we could use basically the same logic
for all platforms. Something like this:
compute_config()
{
if (can do vrr && vrr.uapi.enabled)
compute vrr timings into crtc_state.vrr
else
compute fixed timings into crtc_state.vrr
}
vrr_enable()
{
write VMAX/VMIN/FLIPINE with vrr timings from crtc_state.vrr
if (!always_use_vrr_tg) {
enable PUSH
enable VRR_CTL
// probably wait for vrr live status==enabled here
}
}
vrr_disable()
{
if (!always_use_vrr_tg) {
disable VRR_CTL
// wait for vrr live status==disabled here
disable PUSH
}
write VMAX/VMIN/FLIPINE with fixed timings
}
That way the behaviour is consistent between all the
platforms (apart from the push/vrr enable bits), the state
checker shouldn't need any changes AFAICS, and it's trivial
to change which platforms use the always_use_vrr_tg approach
(should we need to do so).
Or do you see any problems with that approach?
Thanks for clearing this. I think we can work with the above approach.
I am trying to work with something based on above approach, but
currently I am seeing issues with PSR2 with VRR fixed_rr.
Switching between VRR and Fixed RR is fine though.
Perhaps I am missing something around PSR.
Will continue the testing and will post the patches in couple of days.
Thanks again for quick response.
Regards,
Ankit
