On Thu, Feb 16, 2012 at 12:12:49AM +0100, Daniel Vetter wrote:
On Wed, Feb 15, 2012 at 05:59:45PM -0500, Adam Jackson wrote:
On 2/15/12 5:42 PM, Jesse Barnes wrote:
+#define DRM_SET_CONFIG_TEST (1<<0) /* don't change the config, just test it for validity */
This would be OK for cases like checking whether the scene can be composited with planes, of if you need to fall back to full GPU composition.
For actaully trying to find out why the config is bad, this is of course insufficient. For that we'd need something a lot more fine grained for each logical part of the full state. And even that could be a bit vague, since it might not be any single piece of the state that's wrong, it could be the combination of some of the pieces.
+struct drm_mode_set_config {
- __u64 crtcs;
- __u64 crtc_fbs;
- __u64 crtc_xpos; /* array of x coords for crtcs */
- __u64 crtc_ypos; /* array of y coords for crtcs */
- __u32 count_crtcs;
- __u64 plane_sets; /* array of set_plane structs */
- __u32 count_planes;
- __u64 connectors;
- __u64 connector_modes;
- __u32 count_connectors;
- __u32 flags;
+};
This appears to be missing some []s, but I think the intent is clear.
#define DRM_MODE_ENCODER_NONE 0 #define DRM_MODE_ENCODER_DAC 1 #define DRM_MODE_ENCODER_TMDS 2
This allows you to bind a bunch of fbs to crtcs with independent positions, as well as set a bunch of planes to specific fbs and layouts. Finally, it lets you change the connector config at the same time, with a flag to simply test a config instead of actually setting it.
Any comments? Do we also need to set gamma or other properties as part of this? What about cursors?
I guess you might want to set gamma atomically, but I can't imagine it being a factor in anyone's "can I do this" logic.
How do you pass in pixel format? Do you just assume the existing fb is already in the correct format? That could work but it kind of sucks for low-memory environments since you'd need to have enough room to pre-create all the fbs. You could still do the "tear everything down first" approach to work around that, but then you'd still have the possibility of having nothing lit up _and_ not being able to set what was requested, and then needing to unwind in userspace.
I'd sort of also want to see audio reflected in this (sigh), since that's going to affect the bandwidth math. DP 1.2 makes that even worse.
Yeah, I think we should include any funky connector, crtc, plane properties
How about taking this idea to its ultimate conclusion: everything is a property.
Then this ioctl would just be a collection of objects and a bunch of changed properties for each.
It would require standardising the properties, or some of them at least and having two property namespaces: one standard, one driver specific.
It would be easy to extend w/o ABI breaks, whereas in the current model you either need to introduce a new "set everything v2" ioctl, or use properties anyway.
It would also make it slightly easier to optimize use cases like page flipping N planes/crtcs at the same time. Ideally you'd just do a few sanity checks and program the new fb addresses into the hardware. With the current structure you either need to redo all scaling calculations, or check that none of the coordinates changed before you can take the optimized path.
(the latter don't exist yet, but I guess they will sooner or later)
That would be one way to implement color space conversion settings, color adjustments etc., and I agree, they should be part of the atomic mode set. The atomic mode set should include pretty much everything.
because they all might affect how many and which hw resources we need (I'm thinking e.g. of plane setups for hw that reuses crtc engines as planes, but where you can use the crtc scanout engine for something else if it's completely occluded or set to just scan out the black borders with a parameter).
Another one of my favorite ideas would be to deprecate the crtc as a scanout engine concept and just represent all scanout engines as planes. As there will be a new API anyway, it might be a good time to make that change.