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

[Ville Syrjälä]

On Fri, Jan 27, 2017 at 01:50:19PM +0000, Brian Starkey wrote:
> 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?

We'll need to keep some note about the RGB order here. Userspace needs
to know that to actually use the matrix. Oh and I guess we should really
put this documentation into the uapi header.

> 
> 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

-- 
Ville Syrjälä
Intel OTC