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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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@linux.intel.com>
> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Brian Starkey <brian.starkey@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.

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel