On 8 March 2014 06:16, Matt Roper matthew.d.roper@intel.com wrote:
On Fri, Mar 07, 2014 at 03:50:54PM +0530, Rahul Sharma wrote:
Hi Daniel,
On 7 March 2014 14:06, Daniel Vetter daniel@ffwll.ch wrote:
On Thu, Mar 06, 2014 at 11:42:13AM +0530, Rahul Sharma wrote:
Add generic KMS blob properties to core drm framework. These are writable blob properties which can be used to set Image Enhancement parameters. The properties which are added here are meant for color reproduction, color saturation and edge enhancement.
Signed-off-by: Rahul Sharma rahul.sharma@samsung.com
...
Tbh I don't really understand why we can't use normal properties for this. We might want to add a new RBG type of property (or YUV fwiw, both with and without alpha) to make this standardized (maybe with some fixed point value for non-normalizes).
I dropped Normal properties (the ones which accepts 64 bit data) because there will be too many of them. As you can see the count is going upto 24 parameters, means 24 such properties, as each can carry one parameter. This is too much of overhead. Please correct me if you mean something different.
I am not sure what are RGB type properties? I know only about 64-bit normal properties which are too compact to carry above structures. Are you talking about set_gamma type of ioctls. Each "set_gamma" type ioctl needs a addition in callbacks till down the layer which looks bit unnecessary, while writable blob properties are using same set_property and get_property infrastructure.
If the only reason you group them is to atomically commit them, then the only thing we need is the atomic ioctl, which can shovel entire groups of properties over the wire at once.
Atomic commit is not an explicit requirement. But splitting them to many small pieces (in case of normal properties) are resulting into many user-kernel switching overhead, which seems avoidable.
The idea with atomic modeset / nuclear pageflip is that you collect a whole bunch of individual property updates into a "property set" container in userspace (libdrm). When you're done setting all the values you want and "commit" the property set, all of the values that you collected get passed to the kernel in one go via a new ioctl (and are then updated in an atomic manner by the DRM driver). So performing your 24 different property updates via the upcoming atomic API's shouldn't be a problem and shouldn't add any unnecessary overhead.
The kernel-side of atomic modeset / nuclear pageflip is still a WIP, so it isn't upstream yet, but you can see the userspace-facing API in Rob Clark's repo here:
https://github.com/robclark/libdrm/blob/atomic/xf86drmMode.hNote the drmModePropertySet{Alloc,Add,Commit,Free}() functions near the bottom.
Thanks Matt,
I went through the share link. Got the idea of atomic modeset. It is a good solution for setting 24 properties in one go. But another issue is still unaddressed. I mean still need to declare 24 properties which are not "Really 24 different properties".
These are sub-parameters to 3 properties (color reproduction, color saturation and edge enhancement). Splitting them to 24 pieces, just because we don't have a a provision in KMS, is a work around to get things working. And, properties like "saturation_hue_gain_red", "saturation_hue_gain_green" ... will be undoubtedly ugly.
In Rob Clark's tree, I noticed
extern int drmModePropertySetAddBlob(drmModePropertySetPtr set, uint32_t object_id, uint32_t property_id, uint64_t length, void *blob);
Seems, already some implementation of writable blobs is there. I am not able to see the kernel though.
I request experts, to review this solution again. I found it quite useful and hold good with the idea of Atomic modeset.
Regards, Rahul Sharma
Matt
-- Matt Roper Graphics Software Engineer ISG Platform Enabling & Development Intel Corporation (916) 356-2795