Re: [PATCH 1/8] drm: Add optional COLOR_ENCODING and COLOR_RANGE properties to drm_plane

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Feb 19, 2018 at 10:38:46PM +0200, Jyri Sarha wrote:
> On 19/02/18 22:19, Ville Syrjälä wrote:
> >>> +int drm_plane_create_color_properties(struct drm_plane *plane,
> >>> +				      u32 supported_encodings,
> >>> +				      u32 supported_ranges,
> >> Is 0 in the above two supported_ masks a valid value? If yes, should we
> >> still register the prop in that case? If no, please add a WARN_ON early
> >> exit case to catch this.
> > I guess if we go for that we should also check that the supported
> > bitmasks don't contain undefined enum values, and that the default
> > enum values are included in the bitmasks.
> > 
> 
> Agreed. I wonder if there should still be check in
> drm_property_create_enum() for empty enum list. I can not imagine any
> good reason for having an enum property without valid options. It is
> almost a contradiction in terms.

Yeah checking for that in the the core function sounds best.

> > Jyri said he's not looked at this in a while, so I'll just go ahead
> > and respin this myself.
> > 
> 
> Thanks, please do.
> 
> >> Similar, if there's only 1 possible value I guess we should make the prop
> >> immutable?
> > I wonder if we should put that logic into
> > drm_property_create_enum() & co. actually?

I could imagine that we end up with a property that generic userspace
wants to set, but a driver somehow only wants to expose a single value
(because there's one platform where random constraints result in that).
Not quite a sure we should check for that too in the core, but I guess we
can try and see what happens.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux