[PATCH 04/23] drm: Add drm structures for palette color property
Sharma, Shashank
shashank.sharma at intel.com
Wed Sep 23 01:15:16 PDT 2015
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 at 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 at intel.com>
>> Signed-off-by: Kausal Malladi <kausalmalladi at 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 at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
More information about the dri-devel
mailing list