[PATCH RFC v3] drm: Add optinal COLOR_ENCODING and COLOR_RANGE properties to drm_plane

Jyri Sarha jsarha at ti.com
Mon May 15 20:41:25 UTC 2017


On 05/15/17 18:19, Ville Syrjälä wrote:
> On Mon, May 15, 2017 at 05:25:02PM +0300, Jyri Sarha wrote:
>> On 05/15/17 16:34, Ville Syrjälä wrote:
>>> On Mon, May 15, 2017 at 02:19:09PM +0300, Jyri Sarha wrote:
...
>>>> +		len++;
>>>> +	}
>>>> +
>>>> +	prop = drm_property_create_enum(dev, DRM_MODE_PROP_ATOMIC,
>>>> +					"COLOR_ENCODING",
>>>> +					enum_list, len);
>>>> +	if (!prop)
>>>> +		return -ENOMEM;
>>>> +	plane->color_encoding_property = prop;
>>>> +	drm_object_attach_property(&plane->base, prop, default_encoding);
>>>
>>> Missing the plane->state->whatever setup here.
>>
>> What do you mean? Initialize the plane->state->color_encoding with the
>> default value?
>>
>> There is no state allocated for a plane at initialization phase, where I
>> attach the properties, at least in omapdrm. I somehow expected this to
>> be taken care by the default value in the attach call, but now I see it
>> does not do that.
> 
> Yeah. For the props we just do 'if (state) state->foo = bar;'.
> 

Isn't it rather useless to initialize the state variables at property
attach time, in case we happen to have them available, and then just
forget about it?

Shouldn't there be some automation to initialize a new state structures
with the default values?

Am I missing something here?

...
>>>> @@ -37,4 +39,22 @@ void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
>>>>  int drm_mode_crtc_set_gamma_size(struct drm_crtc *crtc,
>>>>  				 int gamma_size);
>>>>  
>>>> +enum drm_color_encoding {
>>>> +	DRM_COLOR_YCBCR_BT601 = 0,
>>>
>>> = 0 seems pointless.
>>>
>>
>> I added them just for stating that the code breaks if the enums do not
>> start from 0, but I can remove that.
> 
> I'm pretty sure enums always start at 0.
> 

Sure, I was just thinking if someone gets clever and later marks the
enum to start from 1 to avoid 0 for some special purpose... but yes,
still quite pointless. I am happy to remove the assignment.

Best regards,
Jyri



More information about the dri-devel mailing list