RE: [PATCH v2 04/10] drm: Add Gamma correction structure

Sharma, Shashank
2015-06-14T09:02:38Z

Hi, Emil Velikov

The reason behind a zero sized array is that we want to use the same variable for various color correction possible across various driver .
Due to current blob implementation, it doesn’t look very efficient to have another pointer in the structure, so we are left with this option only.

I guess as long as we are using gcc (which is for all Linux distributions), we are good. The size of the zero sized array will be zero, so no alignment errors as such.

Regards
Shashank
-----Original Message-----
From: Emil Velikov [mailto:emil.l.velikov@gmail.com]
Sent: Friday, June 12, 2015 10:48 PM
To: Jindal, Sonika
Cc: Malladi, Kausal; Roper, Matthew D; Barnes, Jesse; Lespiau, Damien; R, Durgadoss; Purushothaman, Vijay A; intel-gfx@lists.freedesktop.org; ML dri-devel; Matheson, Annie J; Palleti, Avinash Reddy; Mukherjee, Indranil; R, Dhanya p; Kamath, Sunil; Vetter, Daniel; Sharma, Shashank
Subject: Re: [PATCH v2 04/10] drm: Add Gamma correction structure

Hi Kausal Malladi,

On 5 June 2015 at 13:00, Jindal, Sonika <sonika.jindal@intel.com> wrote:
> On 6/4/2015 7:12 PM, Kausal Malladi wrote:
>>
>> From: Kausal Malladi <Kausal.Malladi@intel.com>
>>
...
>> v2: Addressing Daniel Stone's comment, added a variable sized array
>> to carry Gamma correction values as blob property.
>>
>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>> Signed-off-by: Kausal Malladi <Kausal.Malladi@intel.com>
>> ---
>> include/drm/drm_crtc.h | 3 +++
>> include/uapi/drm/drm.h | 10 ++++++++++
>> 2 files changed, 13 insertions(+)
>>
...
>> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index
>> 3801584..fc2661c 100644
>> --- a/include/uapi/drm/drm.h
>> +++ b/include/uapi/drm/drm.h
>> @@ -829,6 +829,16 @@ struct drm_event_vblank {
>> __u32 reserved;
>> };
>>
>> +/* Color Management structure for Gamma */ struct drm_gamma {
>> + __u32 flags;
>> + __u32 gamma_level;
>> + __u32 gamma_precision;
>> + __u32 num_samples;
>> + __u32 reserved;
>> + __u16 values[0];
Silly question:
Why use zero sized array ? Afaik it's a construct not covered in C90/C99, which makes sizeof(struct drm_gamma) act funny. There seems to be no other instance of a zero-sized array in drm uapi, plus based of Daniel Vettel's "Botching up IOCTLS" I think that using it here might be a bad idea.

The commit message mentions that Daniel Stone suggested it, but that email never made it to the dri-devel mailiing list (and many other emails, as mentioned previously) :'-(

Thanks
Emil