On 8 March 2014 06:16, Matt Roper <matthew.d.roper@xxxxxxxxx> 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@xxxxxxxx> 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@xxxxxxxxxxx> > ... >> > 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.h > > Note 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 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html