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

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

On Fri, Jan 27, 2017 at 03:31:06PM +0100, Daniel Vetter wrote:
> On Fri, Jan 27, 2017 at 04:13:42PM +0200, Ville Syrjälä wrote:
> > 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@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.
> > > 
> > > 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.
> 
> Atm properties are documented in kernel-doc since they don't exist at all
> in the uapi. Well this here is somewhat an exception, since there's
> structs in uapi. But since nothing else in uapi thus far is reasonably
> documented I still think we should keep it in the kernel-doc for now. At
> least until someone starts taggling uapi docs properly ...

Ah. Makes sense to go with the flow for now.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel