[PATCH 03/36] drm/omap: add crtc background property

[Laurent Pinchart]

Hi Ville,

On Wednesday 30 Nov 2016 16:40:30 Ville Syrjälä wrote:
> On Wed, Nov 30, 2016 at 03:56:38PM +0200, Laurent Pinchart wrote:
> > On Wednesday 30 Nov 2016 15:53:53 Ville Syrjälä wrote:
> >> On Wed, Nov 30, 2016 at 03:34:58PM +0200, Laurent Pinchart wrote:
> >>> On Wednesday 30 Nov 2016 14:51:26 Tomi Valkeinen wrote:
> >>>> On 30/11/16 13:25, Laurent Pinchart wrote:
> >>>>> On Wednesday 30 Nov 2016 13:17:05 Tomi Valkeinen wrote:
> >>>>>> Add DRM property for crtc background color property. Background
> >>>>>> color is shown on areas where there are no planes.
> >>>>> 
> >>>>> The background property is useful for the rcar-du driver as well,
> >>>>> and I assume for a bunch of other display engines too. Could you
> >>>>> please standardize it ? And for the usual bikeshedding required in
> >>>>> those circumstances, should it be called background-color or bgcolor
> >>>>> instead ?
> >>>> 
> >>>> The possible color values for background color property depend on the
> >>>> HW. The current OMAP DSS has 8 bits per component, but other sizes
> >>>> should be supported too.
> >>>> 
> >>>> I'm not sure how to implement that in a generic way... Always use 16
> >>>> bits per component for the property?
> >>> 
> >>> That's one option. Another one would be to decide that 8 bit per
> >>> component is enough. The value of the background colour property could
> >>> also possibly be device-dependent, for instance if a device specifies it
> >>> in YUV.
> >> 
> >> We have enough for 16 bits per component in u64. We can just throw away
> >> the lsbs. I have already suggested that approach about a dozen times
> >> throughout the years ;) We migth even have some i915 patches doing
> >> that...
> >> 
> >> I don't have a very good idea for exposing the actual bpc though. Might
> >> be doable with a parallel immutable mask property. But the big open
> >> question is what happens if the actual bpc can change depending on the
> >> crtc configuration. I would suggest we just ignore that issue for now.
> > 
> > I don't think we need to expose the number of bits per component. If the
> > colours are always expressed with 16 bits per component, the kernel can
> > ignore the LSBs completely transparently for userspace.
> > 
> >> For the RGB vs. YUV, we might just have separate props for those I
> >> suppose. Although again it migth depend on the CRTC configuration
> >> which one is effective. Maybe we need a separate "enable this bg color"
> >> type of thing, which would then allow the driver to return an error if
> >> the user is trying to set the wrong one? Or we could just have an enum
> >> prop to select the format of the bg color property (RGB vs. YCbCr to
> >> start with I guess). Yeah, I think I like that idea. And maybe we'd
> >> need to expand that to include even more information about the actual
> >> colorspace (eg. BT601 vs. BT709 vs. whatever for YCbCr)?
> > 
> > It might be a theoretical issue though, as I'm not sure whether there's
> > hardware out there that expresses the background colour in YUV.
> 
> Not sure about bg color. I know of hardware that has a color palette
> in YUV, and some obviously have color key in YUV.

I can't be 100% sure either of course, but I'd be surprised. And even if that 
happens, the background colour would either always be expressed in YUV for 
that hardware, or would switch between YUV and RGB based on CRTC 
configuration. We can document the bgcolor property as being a set of 4 16-bit 
components, with defined mappings for RGB and YUV, and leave the rest to 
userspace. If userspace configures the CRTC in a way that uses a YUV 
background colour, userspace would be also responsible for updating the 
bgcolor value to a YUV value.

-- 
Regards,

Laurent Pinchart