[PATCH 2/2] drm/exynos: mixer: document YCbCr magic numbers

[Andrzej Hajda]

On 06.03.2017 10:43, Tobias Jakobi wrote:
> Hello Andrzej,
>
>
> Andrzej Hajda wrote:
>> On 03.03.2017 14:40, Tobias Jakobi wrote:
>>> The output stage of the mixer uses YCbCr for the internal
>>> computations, which is the reason that some registers take
>>> YCbCr related data as input. In particular this applies
>>> to MXR_BG_COLOR{0,1,2} and MXR_CM_COEFF_{Y,CB,CR}.
>>>
>>> Document the formatting of the data which we write to
>>> these registers.
>>>
>>> While at it, unify wording of comments in the register header.
>>>
>>> Signed-off-by: Tobias Jakobi <tjakobi at math.uni-bielefeld.de>
>>> ---
>>>  drivers/gpu/drm/exynos/exynos_mixer.c | 35 +++++++++++++++++++++++++++--------
>>>  drivers/gpu/drm/exynos/regs-mixer.h   |  7 +++++--
>>>  2 files changed, 32 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
>>> index 41d0c36..3b0b07d 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
>>> @@ -45,6 +45,20 @@
>>>  #define MIXER_WIN_NR		3
>>>  #define VP_DEFAULT_WIN		2
>>>  
>>> +/*
>>> + * Mixer color space conversion coefficient triplet.
>>> + * Used for CSC from RGB to YCbCr.
>>> + * Each coefficient is a 10-bit fixed point number with
>>> + * sign and no integer part, i.e.
>> Maybe it would be more clear to say in exclusive range (-1,1)
>>> + * [0:8] = fractional part (representing a value y = x / 2^9)
>>> + * [9] = sign
>>> + * Negative values are encoded with two's complement.
>>> + */
>>> +#define MXR_CSC_CT(a0, a1, a2) (((a0) << 20) | ((a1) << 10) | ((a2) << 0))
>> We can take advantage of the fact that floating point numbers are
>> allowed in compile time, aren't they?
>>
>> #define MXR_CSC_C(x) ((int)((x) * 512) & 0x3ff)
>> #define MXR_CSC_CT(a0, a1, a2) ((MXR_CSC_C(a0) << 20) | (MXR_CSC_C(a1)
>> << 10) | (MXR_CSC_C(a2) << 0))
>>
>> and stop using magic hexadecimals, now we will use real coefficients.
>>     MXR_CSC_CT(0.1835,  0.6132,  0.0625)
> I'm not sure if this change of base can be done in a lossless fashion
> without typing out too many digits [see below]. I haven't done the math
> here. 

Difference by one in representation corresponds to 0.0019, so four
digits after dot is enough.

> Like I said, no functional changes.

Decision is yours, I can only advice to do not give up.

Regards
Andrzej