Re: [PATCH v3 3/3] drm/omapdrm: Implement gamma_lut atomic crtc property

On Tue, May 24, 2016 at 03:15:46PM +0300, Tomi Valkeinen wrote:

Hi,

On 24/05/16 12:16, Jyri Sarha wrote:

...

Implement gamma_lut atomic crtc property, set crtc gamma size to 256 for all crtcs and use drm_atomic_helper_legacy_gamma_set() as gamma_set func. The tv-out crtc has 1024 element gamma table (with 10bit precision) in HW, but current Xorg server does not accept anything else but 256 elements so that is used for all CRTCs. The dss dispc API converts table of any length for HW and uses linear interpolation in the process.

Signed-off-by: Jyri Sarha jsarha@ti.com

drivers/gpu/drm/omapdrm/omap_crtc.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c index 075f2bb..d5210fe 100644 --- a/drivers/gpu/drm/omapdrm/omap_crtc.c +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c @@ -384,6 +384,15 @@ static void omap_crtc_atomic_flush(struct drm_crtc *crtc,

WARN_ON(omap_crtc->vblank_irq.registered);

• if (crtc->state->color_mgmt_changed) {

• struct drm_color_lut *lut = (struct drm_color_lut *)
  

• 	crtc->state->gamma_lut->data;
  

• unsigned int length = crtc->state->gamma_lut->length /
  

• 	sizeof(*lut);
  

• dispc_mgr_set_gamma(omap_crtc->channel, lut, length);
  

• }

• if (dispc_mgr_is_enabled(omap_crtc->channel)) {

  DBG("%s: GO", omap_crtc->name);

@@ -460,6 +469,7 @@ static const struct drm_crtc_funcs omap_crtc_funcs = { .set_config = drm_atomic_helper_set_config, .destroy = omap_crtc_destroy, .page_flip = drm_atomic_helper_page_flip,

• .gamma_set = drm_atomic_helper_legacy_gamma_set, .set_property = drm_atomic_helper_crtc_set_property, .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state, .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,

@@ -534,6 +544,16 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,

drm_crtc_helper_add(crtc, &omap_crtc_helper_funcs);

• /* The dispc API adapts to what ever size, but the HW supports

• * 256 element gamma table for LCDs and 1024 element table for
  

• * OMAP_DSS_CHANNEL_DIGIT. X server assumes 256 element gamma
  

• * tables so lets use that. Size of HW gamma table can be
  

• * extracted with dispc_mgr_gamma_size(). If it returns 0
  

• * gamma table is not supprted.
  

• */
  

• if (dispc_mgr_gamma_size(channel))

• drm_mode_crtc_set_gamma_size(crtc, 256);
  

• omap_plane_install_properties(crtc->primary, &crtc->base);

  omap_crtcs[channel] = omap_crtc;

I think you also need to attach the GAMMA_LUT and GAMMA_LUT_SIZE properties to the crtc. Otherwise only the legacy API is available. See drm_helper_crtc_enable_color_mgmt() (we can't use that, but as an example).

Hm, the idea once was that this helper doesn't register the props if you pass in a size of 0. And if we add a bool has_ctm on top, you could reuse that helper.

The other offending bit is that it's imo not a helper, but really should be a core part next to setting up the gamma props in drm_crtc.c imo. So in the end:

void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc, int degamma_lut_size, bool has_ctm, int gamma_lut_size);

Volunteered to apply that bit of interface polish?

Thanks, Daniel

Tomi