[Intel-gfx] [PATCH 04/23] drm: Add drm structures for palette color property

[Emil Velikov]

Hi Ville,

On 22 September 2015 at 16:00, Ville Syrjälä
<ville.syrjala at linux.intel.com> wrote:
> On Tue, Sep 22, 2015 at 02:53:54PM +0100, Emil Velikov wrote:
>> On 22 September 2015 at 14:08, Daniel Vetter <daniel at ffwll.ch> 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
>> [snip]
>> >> +struct drm_palette {
>> [snip]
>> > ... extending the ioctl struct at the end ...
>> Is this really going to work, considering that we currently have a
>> zero sized drm_r32g32b32 array at the end ?
>
> Well in this particular case it would be ugly. Sure, it can be done,
> but we couldn't actually define the struct for it using C. Would be
> a neat feature though. Someone should propose it for the next C
> standard :)
>
> In any case, for blobs I think we shouldn't extend them like ioctls.
> In this case the blob is just an array of something, so the blob size
> can of course vary depending how many elements are required for the
> particual platform.
>
Sure it makes sense on its own. I'm curious how likely it will be for
people to get confused/carried away and miss that part. Just a food
for thought.

>> Iirc someone mentioned that using a pointer to the data (over an
>> array) has drawbacks, although for the sake of me I cannot think of
>> any. Can anyone care to enlighten me, please ?
>
> I guess an extensible array at the end of the struct is simply a nice
> fit sometimes. Also makes it more obvious how much junk gets copied over
> I suppose.
>
So no serious 'issues' there, but it's mostly done for convenience.
Thanks for clarifying.

Regards,
Emil