Re: [PATCH] drm/docs: Document where the C8 color lut is stored

On Mon, Jan 24, 2022 at 05:18:07PM -0500, Alex Deucher wrote:

On Mon, Jan 24, 2022 at 5:16 PM Daniel Vetter daniel.vetter@ffwll.ch wrote:

...

Also add notes that for atomic drivers it's really somewhere else and no longer in struct drm_crtc.

Maybe we should put a bigger warning here that this is confusing, since the pixel format is a plane property, but the GAMMA_LUT property is on the crtc. But I think we can fix this if/when someone finds a need for a per-plane CLUT, since I'm not sure such hw even exists. I'm also not sure whether even hardware with a CLUT and a full color correction pipeline with degamm/cgm/gamma exists.

Motivated by comments from Geert that we have a gap here.

v2: More names for color luts (Laurent).

Cc: Laurent Pinchart laurent.pinchart@ideasonboard.com Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com Cc: Geert Uytterhoeven geert@linux-m68k.org Cc: Ville Syrjälä ville.syrjala@linux.intel.com Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Maxime Ripard mripard@kernel.org Cc: Thomas Zimmermann tzimmermann@suse.de Cc: David Airlie airlied@linux.ie Cc: Daniel Vetter daniel@ffwll.ch Signed-off-by: Daniel Vetter daniel.vetter@intel.com

Reviewed-by: Alex Deucher alexander.deucher@amd.com

Thanks for taking a look, pushed to drm-misc-next. -Daniel

...

---

drivers/gpu/drm/drm_color_mgmt.c | 4 ++++ include/drm/drm_crtc.h | 10 ++++++++++ 2 files changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c index bb14f488c8f6..9079fbe21d2f 100644 --- a/drivers/gpu/drm/drm_color_mgmt.c +++ b/drivers/gpu/drm/drm_color_mgmt.c @@ -82,6 +82,10 @@

• driver boot-up state too. Drivers can access this blob through
  

• &drm_crtc_state.gamma_lut.
  

• • Note that for mostly historical reasons stemming from Xorg heritage,
    

• • this is also used to store the color map (also sometimes color lut, CLUT
    

• • or color palette) for indexed formats like DRM_FORMAT_C8.
    

• • “GAMMA_LUT_SIZE”:

  • Unsigned range property to give the size of the lookup table to be set
    

  • on the GAMMA_LUT property (the size depends on the underlying hardware).
    

diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 4d01b4d89775..a70baea0636c 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -285,6 +285,10 @@ struct drm_crtc_state { * Lookup table for converting pixel data after the color conversion * matrix @ctm. See drm_crtc_enable_color_mgmt(). The blob (if not * NULL) is an array of &struct drm_color_lut.

•    *
  

•    * Note that for mostly historical reasons stemming from Xorg heritage,
  

•    * this is also used to store the color map (also sometimes color lut,
  

•    * CLUT or color palette) for indexed formats like DRM_FORMAT_C8.
     */
    struct drm_property_blob *gamma_lut;
  
  

@@ -1075,12 +1079,18 @@ struct drm_crtc { /** * @gamma_size: Size of legacy gamma ramp reported to userspace. Set up * by calling drm_mode_crtc_set_gamma_size().

•    *
  

•    * Note that atomic drivers need to instead use
  

•    * &drm_crtc_state.gamma_lut. See drm_crtc_enable_color_mgmt().
     */
    uint32_t gamma_size;
  
    /**
     * @gamma_store: Gamma ramp values used by the legacy SETGAMMA and
     * GETGAMMA IOCTls. Set up by calling drm_mode_crtc_set_gamma_size().
  

•    *
  

•    * Note that atomic drivers need to instead use
  

•    * &drm_crtc_state.gamma_lut. See drm_crtc_enable_color_mgmt().
     */
    uint16_t *gamma_store;
  
  

-- 2.33.0