On Mon, Mar 10, 2014 at 09:50:14AM +0530, Rahul Sharma wrote: > 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. Like I've said I think creating a standard property for RBG and RBGA values makes sense, to group this stuff a bit. But I don't think we need to group things further. Some clear definitions of these properties is needed though - Sagar from our display group is working on that a bit. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
