[Intel-gfx] Design review request: DRM color manager

[David Herrmann]

Hi

On Tue, Apr 22, 2014 at 6:11 AM, Sharma, Shashank
<shashank.sharma at intel.com> wrote:
> Gentle reminder

Usual approach is to send any proposals as inline plain-text. It's
really hard to comment on attachments, especially if it's an MS-office
format. Anyhow, some comments on the proposal:

1) Why do you register only a single property? Why not register a
separate property for each color-correction that is available? This
way you can drop the property-id and use the high-level DRM-prop
IDs/names.

2) What is the CRTC-ID for? DRM properties can be set on a specific
CRTC and/or plane. Isn't that enough information for the driver?

3) Please document the payload for each of the properties you define.
If the property is a blob, there is no reason to make the properties
generic. User-space requires a common syntax across all drivers,
otherwise, it cannot make use of generic properties and you should use
driver-dependent properties instead.

4) We have a tuple-type for properties. So in case you only need 32bit
payloads for a given property, you can combine enable/disable and
value in a single 64bit property.

5) Please use common prefixes to group related functions: Use
drm_color_manager_register() instead of drm_register_color_manager().
Similarly, use drm_color_manager_set_property() instead of
drm_set_color_manager_property()..

Thanks
David