[PATCH RFC v2 2/2] drm: Add YCBCR_DECODE_CSC and YCBCR_CSC_PREOFFSET properties to drm_plane

[Sharma, Shashank]

Regards

Shashank


On 5/5/2017 12:43 PM, Daniel Vetter wrote:
> On Fri, May 5, 2017 at 9:06 AM, Sharma, Shashank
> <shashank.sharma at intel.com> wrote:
>> On 5/5/2017 12:28 PM, Daniel Vetter wrote:
>>> On Thu, May 04, 2017 at 05:51:45PM +0300, Ville Syrjälä wrote:
>>>> On Thu, May 04, 2017 at 03:22:45PM +0200, Daniel Vetter wrote:
>>>>> On Thu, May 04, 2017 at 10:14:26AM +0300, Jyri Sarha wrote:
>>>>>> Add standard optinal property blobs for YCbCr to RGB conversion CSC
>>>>>> matrix and YCbCr preoffset vector in DRM planes. New enums are defined
>>>>>> to YCBCR_ENCODING property to activate the CSC and preoffset
>>>>>> properties. For simplicity the new properties are stored in the
>>>>>> drm_plane object, because the YCBCR_ENCODING is already there. The
>>>>>> blob contents are defined in the uapi/drm/drm_mode.h header.
>>>>>>
>>>>>> Signed-off-by: Jyri Sarha <jsarha at ti.com>
>>>>> Not sure we want to make this yuv2rgb specific, plenty of planes have
>>>>> generic degamma/offset, csc, gamme/offset hw. I think what we want
>>>>> instead
>>>>> is the generic csc support, plus a ycbcr2rgb mode of "bypass".
>>>> My idea is to not expose this. And instead just expose a normal
>>>> ctm, and then just refuse any combination of YCbCr->RGB/degamma/ctm
>>>> that can't be done by the hw.
>>> But that means we'd need to have a bit-perfect match with a canonical
>>> conversion matrix (even if maybe the hw has a rounding bug and implements
>>> something slightly different than the standard). That seems a bit an
>>> awkward interface. Or is your idea that hw with a ctm would simply not
>>> expose the colorspace enum, if it doesn't also have fixed-function bits on
>>> top of the ctm?
>>> -Daniel
>> I think the idea is to have separate properties for CTM and Gamut Mapping,
>> so that we can have more control
>> over linear and non-linear data blocks. All transformation should happen in
>> one property whereas all gamut
>> mapping should go into other, which IMHO seems to be the correct way to
>> operate.
> Yeah, for the programmable hw we should probably go with the
> degamma/ctm/gamma combo, like we have on the CRTC already. My question
> was how this interactions with some fixed-function color space
> conversion the hw might also have, and how these two sets of
> properties are mean to interact.
Even for the fixed function HW we can have this segregation, and the 
core driver can take care of understanding the enum,
and conversion into driving a fix-function block. I had proposed one 
block diagram, which was once prepared by Ville / Me
to handle this situation, but unfortunately we could not discus much there.
https://patchwork.kernel.org/patch/9695875/
> On that topic, I think it'd be good if we put the helper functions and
> property documentation into drm_color_mgmt.c, so that it is all in one
> place, and to make sure we don't accidentally have different meanings
> for gamma table and ctm blobs.
I agree, that would be  good place.
- Shashank
> -Daniel