Re: [RFC 3/4] drm: add generic blob properties for image enhancement

[Matt Roper <matthew.d.roper@intel.com>]

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.h

Note the drmModePropertySet{Alloc,Add,Commit,Free}() functions near the
bottom.


Matt

-- 
Matt Roper
Graphics Software Engineer
ISG Platform Enabling & Development
Intel Corporation
(916) 356-2795