Regards Shashank
On 9/22/2015 6:38 PM, Daniel Vetter wrote:
On Wed, Sep 16, 2015 at 11:07:01PM +0530, Shashank Sharma wrote:
From: Kausal Malladi kausalmalladi@gmail.com
This patch adds new structures in DRM layer for Palette color correction.These structures will be used by user space agents to configure appropriate number of samples and Palette LUT for a platform.
Signed-off-by: Shashank Sharma shashank.sharma@intel.com Signed-off-by: Kausal Malladi kausalmalladi@gmail.com
include/uapi/drm/drm.h | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index e3c642f..f72b916 100644 --- a/include/uapi/drm/drm.h +++ b/include/uapi/drm/drm.h @@ -840,6 +840,33 @@ struct drm_palette_caps { __u32 num_samples_after_ctm; };
+struct drm_r32g32b32 {
- /*
* Data is in U8.24 fixed point format.* All platforms support values within [0, 1.0] range,* for Red, Green and Blue colors.*/- __u32 r32;
- __u32 g32;
- __u32 b32;
It's not strictly required, but adding a __u32 reserved here to align the struct to 64 bits seems good imo. Slight overhead but meh about that.
Humm, ok, we can check this out.
+};
+struct drm_palette {
- /* Structure version. Should be 1 currently */
- __u32 version;
Definitely great practice to take compat into account and definitely needed for the first design using ioctls but I don't think we need this here. Properties are already extinsible themselves: We can just greate a "ctm-v2", "ctm-v3" if the layout changes, and since the actual ctm matrix is stored in the drm_crtc_state any compat code on the kernel will be shared.
Aside: For an ioctl the recommended way to handle backwards compat and extensions in drm is with a flags bitfield. That's more flexible than a linear version field, and extending the ioctl struct at the end is already handled by the drm core in a transparent fashion (it 0-fills either kernel or userspace side).
Agree, we will drop this. Do you think we should add a flags field, or is it ok without it ?
- /*
* This has to be a supported value during get call.* Feature will be disabled if this is 0 while set*/- __u32 num_samples;
blob properties already have a size, storing it again in the blob is redundnant. Instead I think a small helper to get the number of samples for a given gamma table blob would be needed.
Cheers, Daniel
Please note that they are different. One is the size of blob and other one is the num_samples supported by the property, in the current correction mode. If you check the design doc, num_sample serves the purpose of deciding which correction mode to be applied also. fox ex, for gamma, num_samples=0 indicates disable gamma, whereas num_samples=512 indicates split gamma mode.
Shashank
- /*
* Starting of palette LUT in R32G32B32 format.* Each of RGB value is in U8.24 fixed point format.* Actual number of samples will depend upon num_samples*/- struct drm_r32g32b32 lut[0];
+};
- /* typedef area */ #ifndef __KERNEL__ typedef struct drm_clip_rect drm_clip_rect_t;
-- 1.9.1
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel