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@xxxxxxxxx>
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@xxxxxxxxxx>
> > Cc: Lionel Landwerlin <lionel.g.landwerlin@xxxxxxxxx>
> > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx>
> > ---
> > 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
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
