On Thu, Mar 20, 2014 at 12:01 AM, Matt Roper matthew.d.roper@intel.com wrote:
On Wed, Mar 19, 2014 at 01:24:23PM +0100, Daniel Vetter wrote:
On Tue, Mar 18, 2014 at 05:22:48PM -0700, Matt Roper wrote:
When we expose non-overlay planes to userspace, they will become accessible via standard userspace plane API's. We should be able to handle the standard plane operations against primary planes in a generic way via the page flip handler and modeset handler.
Drivers that can program primary planes more efficiently, that want to use their own primary plane structure to track additional information, or that don't have the limitations assumed by the helpers are free to provide their own implementation of some or all of these handlers.
Signed-off-by: Matt Roper matthew.d.roper@intel.com
One thing I've noticed with planes is that we don't have any per-plane size limits. Which will be annoying for cursors at least, but looking at intel's hw history there are other planes with limits smaller than dev->mode_config.max_widht/height.
I think as part of the universal plane work here we should add this to the getplane ioctl (we can extend at the end with full backwards compat) and use the max_width/height as default for primary/overlay planes and cursor_width/height for cursor planes.
I'm not sure I understand how to (cleanly) extend the existing ioctl safely. Userspace (libdrm) allocates a drm_mode_get_plane structure on the stack with a specific size. If we try to extend this structure to return more information, and have the kernel write into the new fields, aren't we just going to be spilling over into other userspace stack variables if we run an old libdrm on the new kernel? The only approaches I see that could make this work would be huge ugly hacks:
- Reclaiming the top few bits of plane_id to use as "I sent you the new, larger structure" capability flags (which assumes plane ID's are always small enough to leave those bits 0 on current kernels; this would effectively reduce our plane ID address space).
- Figure out a way to encode extra information as bogus pixel formats and shove it into the format list returned to userspace. Presumably userspace would just ignore/skip the bogus formats.
I suppose we could add a new GetPlane2 ioctl or something that returned more info, but I figured it was probably easier to just shove max plane size (and a bunch of other plane capabilities / limitations) into some new read-only plane properties. Read-only properties are easy to extend if we find other pieces of information we want to return in the future, so that seems like the most natural interface to me.
The size userspace expects is passed in through the ioctl number. It then zero-extends the structure correctly for both forward and backwards compatibility. Which means that yeah, we can extend the struct at the end, recompile libdrm and as long as the kernel treats 0 in the new fields sanely. -Daniel