Adding set_blob ioctl to DRM
Thanks Daniel and Bob, Agree, DT is good enough for initial settings. But when need to support on the fly parameter changes, we will end up with 2 solutions. Adding KMS ioctls or improvising blob prop infrastructure (Ruled out existing KMS 64bit props as list of parameters is close to 30, as if now). Later seems better for three reasons; writable blobs are more scalable and can be used for any type of parameter groups. Secondly these are very similar to 64 bit props and use same set_property, get_property infrastructure (else need to add set_gamma kind of callbacks till down the layer). And at last, it looks more complete and appropriate to provide writable blobs just like other KMS property. I am ready with RFC and posting it today. We can also continue this discussion in RFC thread. Regards, Rahul Sharma. On 4 March 2014 23:09, Bob Paauwe wrote: > On Tue, 4 Mar 2014 09:35:57 +0100 > Daniel Vetter wrote: > >> On Thu, Feb 20, 2014 at 11:24:40AM +1000, Dave Airlie wrote: >> > > I am working on enabling a Color Enhancement block for primary display >> > > for Exynos SoC. I need to >> > > set a bunch of parameters like Color Conversion matrix, Contrast >> > > Improvement parameters etc ~ 30 parameters from User Space. >> > > >> > > I am planning to use KDS blob property to receive these parameters. >> > > Currently drivers are not allowed to create a blob property inside >> > > drm. Neither, user space can set the blob. There is no ioctl provided >> > > for same. >> > >> > I don't really like the idea of sticking unstructured data into an >> > ioctl, for the driver to interpret, >> > >> > it opens the door to all kinds of ugly driver hacks, so I think we >> > should have writable blobs, >> > but with well defined structures inside them, not per-driver ones. >> > >> > Per-driver structures will lead to binary userspace drivers that start >> > sticking a pll timings blob on the end and require it to set a mode, >> > because I know driver developers will abuse any interface in the worst >> > way possible. >> > >> > Currently the only blob we really have is EDID and its well defined, >> > so if we are going to add writable blobs, they need to be well defined >> > and as little as possible driver specific, just to avoid driver >> > writings doing what driver writers do. >> >> tbh I don't see a use for structured blobs at all and would much more >> prefer a pile of properties. With the atomic modeset ioctl proposals >> floating around we could then pass in an entire sets of properties, which >> is essentially what the blob prop would be doing. >> >> The upshot of going all-in with explicitly named properties is that we can >> shovel kms configuration descriptions into different formats. I'm thinking >> of shovelling an initial config into DT or something similar as an >> example. For i915 we have some experimental patches which load a DT blob >> as a firmware file and use the property values to set things up. > > I have most of this working with i915 now. Using a DT configuration I > can set module parameters, select which crtc's to enable, which > connectors to enable, set connector properties at init time. I'm > working on some plane property code now. > > It's close to being ready for initial review/feedback but there's one > problem, you can't currently build an x86_64 (32 bit is fine) kernel > with DT enabled because of an unresolved symbol in the apic code. > >> >> So imo a blob property needs some _really_ good justification. My default >> stance is to oppose it ;-) >> >> Cheers, Daniel > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
Adding set_blob ioctl to DRM
On Tue, 4 Mar 2014 09:35:57 +0100 Daniel Vetter wrote: > On Thu, Feb 20, 2014 at 11:24:40AM +1000, Dave Airlie wrote: > > > I am working on enabling a Color Enhancement block for primary display > > > for Exynos SoC. I need to > > > set a bunch of parameters like Color Conversion matrix, Contrast > > > Improvement parameters etc ~ 30 parameters from User Space. > > > > > > I am planning to use KDS blob property to receive these parameters. > > > Currently drivers are not allowed to create a blob property inside > > > drm. Neither, user space can set the blob. There is no ioctl provided > > > for same. > > > > I don't really like the idea of sticking unstructured data into an > > ioctl, for the driver to interpret, > > > > it opens the door to all kinds of ugly driver hacks, so I think we > > should have writable blobs, > > but with well defined structures inside them, not per-driver ones. > > > > Per-driver structures will lead to binary userspace drivers that start > > sticking a pll timings blob on the end and require it to set a mode, > > because I know driver developers will abuse any interface in the worst > > way possible. > > > > Currently the only blob we really have is EDID and its well defined, > > so if we are going to add writable blobs, they need to be well defined > > and as little as possible driver specific, just to avoid driver > > writings doing what driver writers do. > > tbh I don't see a use for structured blobs at all and would much more > prefer a pile of properties. With the atomic modeset ioctl proposals > floating around we could then pass in an entire sets of properties, which > is essentially what the blob prop would be doing. > > The upshot of going all-in with explicitly named properties is that we can > shovel kms configuration descriptions into different formats. I'm thinking > of shovelling an initial config into DT or something similar as an > example. For i915 we have some experimental patches which load a DT blob > as a firmware file and use the property values to set things up. I have most of this working with i915 now. Using a DT configuration I can set module parameters, select which crtc's to enable, which connectors to enable, set connector properties at init time. I'm working on some plane property code now. It's close to being ready for initial review/feedback but there's one problem, you can't currently build an x86_64 (32 bit is fine) kernel with DT enabled because of an unresolved symbol in the apic code. > > So imo a blob property needs some _really_ good justification. My default > stance is to oppose it ;-) > > Cheers, Daniel
Adding set_blob ioctl to DRM
On Thu, Feb 20, 2014 at 11:24:40AM +1000, Dave Airlie wrote: > > I am working on enabling a Color Enhancement block for primary display > > for Exynos SoC. I need to > > set a bunch of parameters like Color Conversion matrix, Contrast > > Improvement parameters etc ~ 30 parameters from User Space. > > > > I am planning to use KDS blob property to receive these parameters. > > Currently drivers are not allowed to create a blob property inside > > drm. Neither, user space can set the blob. There is no ioctl provided > > for same. > > I don't really like the idea of sticking unstructured data into an > ioctl, for the driver to interpret, > > it opens the door to all kinds of ugly driver hacks, so I think we > should have writable blobs, > but with well defined structures inside them, not per-driver ones. > > Per-driver structures will lead to binary userspace drivers that start > sticking a pll timings blob on the end and require it to set a mode, > because I know driver developers will abuse any interface in the worst > way possible. > > Currently the only blob we really have is EDID and its well defined, > so if we are going to add writable blobs, they need to be well defined > and as little as possible driver specific, just to avoid driver > writings doing what driver writers do. tbh I don't see a use for structured blobs at all and would much more prefer a pile of properties. With the atomic modeset ioctl proposals floating around we could then pass in an entire sets of properties, which is essentially what the blob prop would be doing. The upshot of going all-in with explicitly named properties is that we can shovel kms configuration descriptions into different formats. I'm thinking of shovelling an initial config into DT or something similar as an example. For i915 we have some experimental patches which load a DT blob as a firmware file and use the property values to set things up. So imo a blob property needs some _really_ good justification. My default stance is to oppose it ;-) Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
Adding set_blob ioctl to DRM
> I am working on enabling a Color Enhancement block for primary display > for Exynos SoC. I need to > set a bunch of parameters like Color Conversion matrix, Contrast > Improvement parameters etc ~ 30 parameters from User Space. > > I am planning to use KDS blob property to receive these parameters. > Currently drivers are not allowed to create a blob property inside > drm. Neither, user space can set the blob. There is no ioctl provided > for same. I don't really like the idea of sticking unstructured data into an ioctl, for the driver to interpret, it opens the door to all kinds of ugly driver hacks, so I think we should have writable blobs, but with well defined structures inside them, not per-driver ones. Per-driver structures will lead to binary userspace drivers that start sticking a pll timings blob on the end and require it to set a mode, because I know driver developers will abuse any interface in the worst way possible. Currently the only blob we really have is EDID and its well defined, so if we are going to add writable blobs, they need to be well defined and as little as possible driver specific, just to avoid driver writings doing what driver writers do. Dave.
Adding set_blob ioctl to DRM
Thanks Dave, On 20 February 2014 06:54, Dave Airlie wrote: >> I am working on enabling a Color Enhancement block for primary display >> for Exynos SoC. I need to >> set a bunch of parameters like Color Conversion matrix, Contrast >> Improvement parameters etc ~ 30 parameters from User Space. >> >> I am planning to use KDS blob property to receive these parameters. >> Currently drivers are not allowed to create a blob property inside >> drm. Neither, user space can set the blob. There is no ioctl provided >> for same. > > I don't really like the idea of sticking unstructured data into an > ioctl, for the driver to interpret, > > it opens the door to all kinds of ugly driver hacks, so I think we > should have writable blobs, > but with well defined structures inside them, not per-driver ones. > I agree with you. I will define these as generic structures which are complete sets of parameters, required by standard color algorithms. Something like this: Color Reproduction: - R[r,g,b], G[r,g,b], B[r,g,b] - Cyan[r,g,b], Magenta[r,g,b], Yellow[r,g,b] - White[r,g,b] - Black[r,g,b] I will share the code once it is in some shape. Regards, Rahul Sharma. > Per-driver structures will lead to binary userspace drivers that start > sticking a pll timings blob on the end and require it to set a mode, > because I know driver developers will abuse any interface in the worst > way possible. > > Currently the only blob we really have is EDID and its well defined, > so if we are going to add writable blobs, they need to be well defined > and as little as possible driver specific, just to avoid driver > writings doing what driver writers do. > > Dave.
Adding set_blob ioctl to DRM
Hi All, I am working on enabling a Color Enhancement block for primary display for Exynos SoC. I need to set a bunch of parameters like Color Conversion matrix, Contrast Improvement parameters etc ~ 30 parameters from User Space. I am planning to use KDS blob property to receive these parameters. Currently drivers are not allowed to create a blob property inside drm. Neither, user space can set the blob. There is no ioctl provided for same. How about adding set_blob ioctl in DRM? It seems purposely dropped till now. Please help me in understanding explains the rationale. What alternatives do I have inside drm to set multiple parameters otherwise? (other than Exynos specific ioctls) Please guide me. Regards, Rahul Sharma.
