[PATCH] drm/color: Include CTM equations in kerneldoc

[Brian Starkey]

On Fri, Jan 27, 2017 at 03:27:09PM +0200, Ville Syrjälä wrote:
>On Fri, Jan 27, 2017 at 10:47:48AM +0000, Brian Starkey wrote:
>> Explicitly state the expected CTM equations in the kerneldoc for the CTM
>> property.
>>
>> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
>> Cc: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
>> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
>> Signed-off-by: Brian Starkey <brian.starkey at arm.com>
>> ---
>>
>> Hi,
>>
>> This captures the outcome of the discussion on #dri-devel yesterday
>> (2017-01-26):
>> https://people.freedesktop.org/~cbrill/dri-log/?channel=dri-devel&date=2017-01-26
>>
>> I'm not sure about the stance on such explicit rst markup in kerneldoc,
>> but without it the equations are pretty unreadable in the rendered
>> output.
>>
>> Cheers,
>> Brian
>>
>>  drivers/gpu/drm/drm_color_mgmt.c |   10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
>> index 789b4c65cd69..63f3a7404fa1 100644
>> --- a/drivers/gpu/drm/drm_color_mgmt.c
>> +++ b/drivers/gpu/drm/drm_color_mgmt.c
>> @@ -62,6 +62,16 @@
>>   *	unit/pass-thru matrix should be used. This is generally the driver
>>   *	boot-up state too.
>>   *
>> + *	Given an input vector ``in[3]`` and an output vector ``out[3]``, the
>> + *	transformation applied is:
>> + *
>> + *	| ``out[0] = matrix[0] * in[0] + matrix[1] * in[1] + matrix[2] * in[2];``
>> + *	| ``out[1] = matrix[3] * in[0] + matrix[4] * in[1] + matrix[5] * in[2];``
>> + *	| ``out[2] = matrix[6] * in[0] + matrix[7] * in[1] + matrix[8] * in[2];``
>> + *
>> + *	| For RGB formats, the input vector is assumed to be ``{ R, G, B }``.
>> + *	| For YCbCr formats, the input vector is assumed to be ``{ Y, Cb, Cr }``.
>
>Talking about formats here could be a little confusing. One might think
>this has something to do with the framebuffer pixel format, when in fact
>it's only about the internal format used by the crtc.

Ah right, yes I see.

Actually, I *was* thinking about the framebuffer format here - but
that is missing the context of us adding a CTM property for each plane
(so that each plane can map the framebuffer format to the CRTC pipe's
internal format).

Our intention (for Mali-DP) is to add CTM on each plane to be used
for framebuffer -> CRTC pipe conversion, and then use the CTM on the
CRTC for CRTC pipe -> output conversion.

Shall I just remove the two lines about pixel formats here, and then
when we land per-plane CTM add some details about plane CTM matrices
being before the CRTC CTM matrix?

Thanks,
-Brian
>
>And actually I don't think we can get away this easily for YCbCr since
>there is no way to indicate to userspace whether the pipe is internally
>RGB or YCbCr. Until we add a property to indicate RGB vs. YCbCr internal
>crtc formt (which userspace could actually set if the hardware allows it)
>we shouldn't even mention YCbCr. If there is hardware out there that
>always uses YCbCr, then I think those folks need to come up with a
>property to indicate that, or they'll just have to do the RGB->YCbCr
>conversion in the driver when populating the matrix.
>
>> + *
>>   * “GAMMA_LUT”:
>>   *	Blob property to set the gamma lookup table (LUT) mapping pixel data
>>   *	after the transformation matrix to data sent to the connector. The
>> --
>> 1.7.9.5
>
>-- 
>Ville Syrjälä
>Intel OTC