On Thu, 28 Nov 2013 14:14:09 +0100 Daniel Vetter <daniel@xxxxxxxx> wrote: > > [Vandana/Pradeep] This patch series is for seamless DRRS > > support and has to be a separate path from mode set as we just > > toggle the PIPE_CONF_RR_SWTICH for seamless RR transition without > > flicker. Complete modeset using setCrtc will result in partial > > blanking or flicker. We are not implementing fast boot as part of > > this series. Most eDP panels today which support multiple refresh > > rates, support seamless DRRS. This means that hardware capability > > is made use of by toggling a bit in pipe config to switch between > > refresh rates on the fly. The patch series is aimed to take > > advantage of this capability to switch refresh rates based on > > idleness or video playback requirement. Hence, we chose the > > connector attribute/ set property path. Also in order to support > > the Media refresh rates of 50/48 we thought it incorrect to > > populate those modes in probed_mode list as those values are not > > from the EDID. So in order for User space to know the media > > refresh rates supported we have exposed a set_property interface > > which makes User space aware of these media refresh rates. Also we > > do not intend to do a complete mode set on media use cases in > > which case we think using setCrtc is not needed. Using setCrtc > > will invariably result in intermittant blanking or flicker which > > cannot be avoided, and gives undesirable effect during playback. > > Seamless DRRS doesn't need a complete modeset and hence no flicker > > or temporary blanking. Changing the toggle RR bit in PIPE_CONF is > > faster. Also since BDW has only single M/N/TU register the current > > code takes care of it easily. Kindly let us know your opinion. > > Since Seamless DRRS for eDP is separate path from modeset we can > > keep the connector property attribute as it is. > > First the technical stuff: > > - Adding the additional modes to the mode list is imo totally ok and as > the design intents. We are already adding other modes from the vbt here > if those are available. The only restriction is that the kernel may not > add modes which don't work. But as long as the vbt tells us that 48Hz > will work we can (and imo should) use it. > > - Second changing the frequency changes the refresh rate. Atm both our > userspace and the kernel support code assume that this can only be done > through a SetCrtc call. So adjusting the dotclock is actually the right > interface and we don't need any additional property. > > Now the more political stuff: > > - We have the long-term goal of solid fastboot support. We've the designed > we've picked that actually means 95% effort to get flicker-free SetCrtc > going and 5% actual code for fastboot. We're not there yet, there's only > a preview available which is disabled by default and uses a few hacks. > But generally all features that are relevant need to use the new > infrastructure and help move things forward. > > - Upstream has a time horizon of 5-10 years for the kernel/userspace > interface. Which means quick hacks are not allowed. And in my opinion > your property is a quick hack, which nicely simplified the > implementation. > > So in short I know that my request extends the scope of your patches. But > upstream also imposes differing constraints than a product tree. It should be possible to still merge the automatic, in-kernel only DRRS bits though, right? Then work on making dotclock/refresh changes from mode sets flicker free (shouldn't be a massive amount of work). -- Jesse Barnes, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
