diff mbox

drm/doc: Interlink color manager docs better

Message ID 20170412152006.12233-1-daniel.vetter@ffwll.ch (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Vetter April 12, 2017, 3:20 p.m. UTC
Motivated by a request from Eric.

Cc: Eric Anholt <eric@anholt.net>
Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_atomic_helper.c |  3 ++-
 drivers/gpu/drm/drm_color_mgmt.c    |  9 ++++++---
 include/drm/drm_crtc.h              | 31 +++++++++++++++++++++++++------
 3 files changed, 33 insertions(+), 10 deletions(-)

Comments

Lionel Landwerlin April 13, 2017, 5:55 p.m. UTC | #1
I have a tiny suggestion down there. Regardless this is :

Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>

On 12/04/17 08:20, Daniel Vetter wrote:
> Motivated by a request from Eric.
>
> Cc: Eric Anholt <eric@anholt.net>
> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>   drivers/gpu/drm/drm_atomic_helper.c |  3 ++-
>   drivers/gpu/drm/drm_color_mgmt.c    |  9 ++++++---
>   include/drm/drm_crtc.h              | 31 +++++++++++++++++++++++++------
>   3 files changed, 33 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index c3403edd0285..442724a80010 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -3518,7 +3518,8 @@ EXPORT_SYMBOL(drm_atomic_helper_connector_destroy_state);
>    *
>    * Implements support for legacy gamma correction table for drivers
>    * that support color management through the DEGAMMA_LUT/GAMMA_LUT
> - * properties.
> + * properties. See drm_crtc_enable_color_mgmt() and the containing chapter for
> + * how the atomic color management and gamma tables work.
>    */
>   int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
>   				       u16 *red, u16 *green, u16 *blue,
> diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> index 533f3a3e6877..3eda500fc005 100644
> --- a/drivers/gpu/drm/drm_color_mgmt.c
> +++ b/drivers/gpu/drm/drm_color_mgmt.c
> @@ -43,7 +43,8 @@
>    *
>    *	Setting this to NULL (blob property value set to 0) means a
>    *	linear/pass-thru gamma table should be used. This is generally the
> - *	driver boot-up state too.
> + *	driver boot-up state too. Drivers can access this blob through
> + *	&drm_crtc_state.degamma_lut.
>    *
>    * “DEGAMMA_LUT_SIZE”:
>    *	Unsinged range property to give the size of the lookup table to be set
> @@ -60,7 +61,8 @@
>    *
>    *	Setting this to NULL (blob property value set to 0) means a
>    *	unit/pass-thru matrix should be used. This is generally the driver
> - *	boot-up state too.
> + *	boot-up state too. Drivers can access the blob for the color conversion
> + *	matrix through &drm_crtc_state.ctm.
>    *
>    * “GAMMA_LUT”:
>    *	Blob property to set the gamma lookup table (LUT) mapping pixel data
> @@ -72,7 +74,8 @@
>    *
>    *	Setting this to NULL (blob property value set to 0) means a
>    *	linear/pass-thru gamma table should be used. This is generally the
> - *	driver boot-up state too.
> + *	driver boot-up state too. Drivers can access this blob through
> + *	&drm_crtc_state.gamma_lut.
>    *
>    * “GAMMA_LUT_SIZE”:
>    *	Unsigned range property to give the size of the lookup table to be set
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index a8176a836e25..60b128a9e723 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -93,11 +93,6 @@ struct drm_plane_helper_funcs;
>    * @adjusted_mode: for use by helpers and drivers to compute adjusted mode timings
>    * @mode: current mode timings
>    * @mode_blob: &drm_property_blob for @mode
> - * @degamma_lut: Lookup table for converting framebuffer pixel data
> - *	before apply the conversion matrix
> - * @ctm: Transformation matrix
> - * @gamma_lut: Lookup table for converting pixel data after the
> - *	conversion matrix
>    * @state: backpointer to global drm_atomic_state
>    *
>    * Note that the distinction between @enable and @active is rather subtile:
> @@ -144,9 +139,27 @@ struct drm_crtc_state {
>   	/* blob property to expose current mode to atomic userspace */
>   	struct drm_property_blob *mode_blob;
>   
> -	/* blob property to expose color management to userspace */
> +	/**
> +	 * @degamma_lut:
> +	 *
> +	 * Lookup table for converting framebuffer pixel data before apply the
> +	 * color conversion matrix @ctm. See drm_crtc_enable_color_mgmt().

Maybe you can add that the blob is either NULL or an array of drm_color_lut.

> +	 */
>   	struct drm_property_blob *degamma_lut;
> +
> +	/**
> +	 * @ctm:
> +	 *
> +	 * Color transformation matrix. See drm_crtc_enable_color_mgmt().

Here is blob is drm_color_ctm.

> +	 */
>   	struct drm_property_blob *ctm;
> +
> +	/**
> +	 * @gamma_lut:
> +	 *
> +	 * Lookup table for converting pixel data after the color conversion
> +	 * matrix @ctm.  See drm_crtc_enable_color_mgmt().

Here again NULL or an array of drm_color_lut.

> +	 */
>   	struct drm_property_blob *gamma_lut;
>   
>   	/**
> @@ -313,6 +326,12 @@ struct drm_crtc_funcs {
>   	 *
>   	 * This callback is optional.
>   	 *
> +	 * Atomic drivers who want to support gamma tables should implement the
> +	 * atomic color management support, enabled by calling
> +	 * drm_crtc_enable_color_mgmt(), which then supports the legacy gamma
> +	 * interface through the drm_atomic_helper_legacy_gamma_set()
> +	 * compatibility implementation.
> +	 *
>   	 * NOTE:
>   	 *
>   	 * Drivers that support gamma tables and also fbdev emulation through
Daniel Vetter April 18, 2017, 6:40 a.m. UTC | #2
On Thu, Apr 13, 2017 at 10:55:12AM -0700, Lionel Landwerlin wrote:
> I have a tiny suggestion down there. Regardless this is :
> 
> Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>

Thanks for the suggestions, polished while applying the patch.
-Daniel

> 
> On 12/04/17 08:20, Daniel Vetter wrote:
> > Motivated by a request from Eric.
> > 
> > Cc: Eric Anholt <eric@anholt.net>
> > Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> >   drivers/gpu/drm/drm_atomic_helper.c |  3 ++-
> >   drivers/gpu/drm/drm_color_mgmt.c    |  9 ++++++---
> >   include/drm/drm_crtc.h              | 31 +++++++++++++++++++++++++------
> >   3 files changed, 33 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> > index c3403edd0285..442724a80010 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -3518,7 +3518,8 @@ EXPORT_SYMBOL(drm_atomic_helper_connector_destroy_state);
> >    *
> >    * Implements support for legacy gamma correction table for drivers
> >    * that support color management through the DEGAMMA_LUT/GAMMA_LUT
> > - * properties.
> > + * properties. See drm_crtc_enable_color_mgmt() and the containing chapter for
> > + * how the atomic color management and gamma tables work.
> >    */
> >   int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
> >   				       u16 *red, u16 *green, u16 *blue,
> > diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> > index 533f3a3e6877..3eda500fc005 100644
> > --- a/drivers/gpu/drm/drm_color_mgmt.c
> > +++ b/drivers/gpu/drm/drm_color_mgmt.c
> > @@ -43,7 +43,8 @@
> >    *
> >    *	Setting this to NULL (blob property value set to 0) means a
> >    *	linear/pass-thru gamma table should be used. This is generally the
> > - *	driver boot-up state too.
> > + *	driver boot-up state too. Drivers can access this blob through
> > + *	&drm_crtc_state.degamma_lut.
> >    *
> >    * “DEGAMMA_LUT_SIZE”:
> >    *	Unsinged range property to give the size of the lookup table to be set
> > @@ -60,7 +61,8 @@
> >    *
> >    *	Setting this to NULL (blob property value set to 0) means a
> >    *	unit/pass-thru matrix should be used. This is generally the driver
> > - *	boot-up state too.
> > + *	boot-up state too. Drivers can access the blob for the color conversion
> > + *	matrix through &drm_crtc_state.ctm.
> >    *
> >    * “GAMMA_LUT”:
> >    *	Blob property to set the gamma lookup table (LUT) mapping pixel data
> > @@ -72,7 +74,8 @@
> >    *
> >    *	Setting this to NULL (blob property value set to 0) means a
> >    *	linear/pass-thru gamma table should be used. This is generally the
> > - *	driver boot-up state too.
> > + *	driver boot-up state too. Drivers can access this blob through
> > + *	&drm_crtc_state.gamma_lut.
> >    *
> >    * “GAMMA_LUT_SIZE”:
> >    *	Unsigned range property to give the size of the lookup table to be set
> > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> > index a8176a836e25..60b128a9e723 100644
> > --- a/include/drm/drm_crtc.h
> > +++ b/include/drm/drm_crtc.h
> > @@ -93,11 +93,6 @@ struct drm_plane_helper_funcs;
> >    * @adjusted_mode: for use by helpers and drivers to compute adjusted mode timings
> >    * @mode: current mode timings
> >    * @mode_blob: &drm_property_blob for @mode
> > - * @degamma_lut: Lookup table for converting framebuffer pixel data
> > - *	before apply the conversion matrix
> > - * @ctm: Transformation matrix
> > - * @gamma_lut: Lookup table for converting pixel data after the
> > - *	conversion matrix
> >    * @state: backpointer to global drm_atomic_state
> >    *
> >    * Note that the distinction between @enable and @active is rather subtile:
> > @@ -144,9 +139,27 @@ struct drm_crtc_state {
> >   	/* blob property to expose current mode to atomic userspace */
> >   	struct drm_property_blob *mode_blob;
> > -	/* blob property to expose color management to userspace */
> > +	/**
> > +	 * @degamma_lut:
> > +	 *
> > +	 * Lookup table for converting framebuffer pixel data before apply the
> > +	 * color conversion matrix @ctm. See drm_crtc_enable_color_mgmt().
> 
> Maybe you can add that the blob is either NULL or an array of drm_color_lut.
> 
> > +	 */
> >   	struct drm_property_blob *degamma_lut;
> > +
> > +	/**
> > +	 * @ctm:
> > +	 *
> > +	 * Color transformation matrix. See drm_crtc_enable_color_mgmt().
> 
> Here is blob is drm_color_ctm.
> 
> > +	 */
> >   	struct drm_property_blob *ctm;
> > +
> > +	/**
> > +	 * @gamma_lut:
> > +	 *
> > +	 * Lookup table for converting pixel data after the color conversion
> > +	 * matrix @ctm.  See drm_crtc_enable_color_mgmt().
> 
> Here again NULL or an array of drm_color_lut.
> 
> > +	 */
> >   	struct drm_property_blob *gamma_lut;
> >   	/**
> > @@ -313,6 +326,12 @@ struct drm_crtc_funcs {
> >   	 *
> >   	 * This callback is optional.
> >   	 *
> > +	 * Atomic drivers who want to support gamma tables should implement the
> > +	 * atomic color management support, enabled by calling
> > +	 * drm_crtc_enable_color_mgmt(), which then supports the legacy gamma
> > +	 * interface through the drm_atomic_helper_legacy_gamma_set()
> > +	 * compatibility implementation.
> > +	 *
> >   	 * NOTE:
> >   	 *
> >   	 * Drivers that support gamma tables and also fbdev emulation through
> 
>
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index c3403edd0285..442724a80010 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3518,7 +3518,8 @@  EXPORT_SYMBOL(drm_atomic_helper_connector_destroy_state);
  *
  * Implements support for legacy gamma correction table for drivers
  * that support color management through the DEGAMMA_LUT/GAMMA_LUT
- * properties.
+ * properties. See drm_crtc_enable_color_mgmt() and the containing chapter for
+ * how the atomic color management and gamma tables work.
  */
 int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
 				       u16 *red, u16 *green, u16 *blue,
diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index 533f3a3e6877..3eda500fc005 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -43,7 +43,8 @@ 
  *
  *	Setting this to NULL (blob property value set to 0) means a
  *	linear/pass-thru gamma table should be used. This is generally the
- *	driver boot-up state too.
+ *	driver boot-up state too. Drivers can access this blob through
+ *	&drm_crtc_state.degamma_lut.
  *
  * “DEGAMMA_LUT_SIZE”:
  *	Unsinged range property to give the size of the lookup table to be set
@@ -60,7 +61,8 @@ 
  *
  *	Setting this to NULL (blob property value set to 0) means a
  *	unit/pass-thru matrix should be used. This is generally the driver
- *	boot-up state too.
+ *	boot-up state too. Drivers can access the blob for the color conversion
+ *	matrix through &drm_crtc_state.ctm.
  *
  * “GAMMA_LUT”:
  *	Blob property to set the gamma lookup table (LUT) mapping pixel data
@@ -72,7 +74,8 @@ 
  *
  *	Setting this to NULL (blob property value set to 0) means a
  *	linear/pass-thru gamma table should be used. This is generally the
- *	driver boot-up state too.
+ *	driver boot-up state too. Drivers can access this blob through
+ *	&drm_crtc_state.gamma_lut.
  *
  * “GAMMA_LUT_SIZE”:
  *	Unsigned range property to give the size of the lookup table to be set
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index a8176a836e25..60b128a9e723 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -93,11 +93,6 @@  struct drm_plane_helper_funcs;
  * @adjusted_mode: for use by helpers and drivers to compute adjusted mode timings
  * @mode: current mode timings
  * @mode_blob: &drm_property_blob for @mode
- * @degamma_lut: Lookup table for converting framebuffer pixel data
- *	before apply the conversion matrix
- * @ctm: Transformation matrix
- * @gamma_lut: Lookup table for converting pixel data after the
- *	conversion matrix
  * @state: backpointer to global drm_atomic_state
  *
  * Note that the distinction between @enable and @active is rather subtile:
@@ -144,9 +139,27 @@  struct drm_crtc_state {
 	/* blob property to expose current mode to atomic userspace */
 	struct drm_property_blob *mode_blob;
 
-	/* blob property to expose color management to userspace */
+	/**
+	 * @degamma_lut:
+	 *
+	 * Lookup table for converting framebuffer pixel data before apply the
+	 * color conversion matrix @ctm. See drm_crtc_enable_color_mgmt().
+	 */
 	struct drm_property_blob *degamma_lut;
+
+	/**
+	 * @ctm:
+	 *
+	 * Color transformation matrix. See drm_crtc_enable_color_mgmt().
+	 */
 	struct drm_property_blob *ctm;
+
+	/**
+	 * @gamma_lut:
+	 *
+	 * Lookup table for converting pixel data after the color conversion
+	 * matrix @ctm.  See drm_crtc_enable_color_mgmt().
+	 */
 	struct drm_property_blob *gamma_lut;
 
 	/**
@@ -313,6 +326,12 @@  struct drm_crtc_funcs {
 	 *
 	 * This callback is optional.
 	 *
+	 * Atomic drivers who want to support gamma tables should implement the
+	 * atomic color management support, enabled by calling
+	 * drm_crtc_enable_color_mgmt(), which then supports the legacy gamma
+	 * interface through the drm_atomic_helper_legacy_gamma_set()
+	 * compatibility implementation.
+	 *
 	 * NOTE:
 	 *
 	 * Drivers that support gamma tables and also fbdev emulation through