Re: [PATCH 04/23] drm: Add drm structures for palette color property

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxx>

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@xxxxxxxxx>
Signed-off-by: Kausal Malladi <kausalmalladi@xxxxxxxxx>
---
  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@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux