On Tue, May 12, 2015 at 10:23 AM, Thierry Reding <thierry.reding@xxxxxxxxx> wrote: > On Tue, May 12, 2015 at 08:35:58AM +0200, Daniel Vetter wrote: >> On Mon, May 11, 2015 at 04:34:57PM +0000, Mark Zhang wrote: >> > I just want to make things easier. If we adding this in panel's meta >> > data, it will be harder to make crtc gets this, since normally encoder >> > talks with panel and crtc talks with encoder. But yes, adding this in >> > panel's metadata makes more sense so if there is a better way to do >> > that, I'm happy to do the changes. >> >> Adding something to the userspace ABI (which you've done here) because the >> kernel-internals are designed in an awkward way right now is definitely >> the wrong thing to do. With atomic you can easily add a bool >> prefer_oneshot to drm_crtc_state to encode this. But I fear that with the >> plain crtc helpers this just doesn't work properly. You could add a >> driver-private internal in drm_display_mode->private_flags, but that might >> clash with drivers existing use of this field. >> >> In any way, this is definitely not something to add to uapi headers. Hence >> Nacked-by: me. > > Are there use-cases where one-shot mode is worse than continuous mode? > I'm thinking games that run at full FPS and such. If so, exposing this > to userspace is perhaps not a bad idea, albeit not via a mode flag. If > userspace had a way to set the preference, it could do so depending on > use-case. If you have a case with jitter when the pageflips run at not-quite 60fps (just an example) then I think the kernel should cope with that by disabling oneshot mode on its own. At least that's what i915 does. Different story once we'll get variable refresh-rate support, but I think that's an entirely different beast. Also we don't need this as a mode flag, an atomic prop seems better suited. Especially since the atomic prop avoids the mode_changed logic of the helpers (which we want, no need to flicker when updating such a hint), wheres anything that changes the mode will force a full modeset by default. Also if you expose this to userspace, you need userspace to use it before merging. drm_display_mode->private_flags still looks like the most suitable place. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
