[PATCH v3 weston 3/5] compositor-drm: add support for CTM property

[Daniel Stone]

Hi Harsha,

On Thu, 28 Jun 2018 at 14:29, <harsha.manjulamallikarjun at in.bosch.com> wrote:
> +       struct drm_output *output = to_drm_output(output_base);
> +       struct drm_backend *backend = to_drm_backend(output->base.compositor);
> +       struct drm_color_ctm ctm;
> +       uint32_t ctm_blobid = 0;
> +       uint32_t ctm_prop_id;
> +       int row;
> +       int col;
> +       int rc;
> +       float coefficient;
> +
> +       ctm_prop_id = output->props_crtc[WDRM_CRTC_CTM].prop_id;
> +       if (!ctm_prop_id)
> +               return;
> +
> +       for (row = 0; row < 3; row++) {
> +               for (col = 0; col < 3; col++) {
> +                       coefficient = ctm_matrix->d[(row * 4) + col];
> +                       ctm.matrix[(row * 3) +  col] =
> +                                       drm_convert_to_ctm_format(coefficient);
> +               }
> +       }
> +
> +       rc = drmModeCreatePropertyBlob(backend->drm.fd, &ctm, sizeof(ctm),
> +                                      &ctm_blobid);
> +       if (!rc) {
> +               rc = drmModeObjectSetProperty(backend->drm.fd, output->crtc_id,
> +                                             DRM_MODE_OBJECT_CRTC, ctm_prop_id,
> +                                             ctm_blobid);
> +               if (rc)
> +                       weston_log("failed to set ctm for crtc %d: %m\n",
> +                                  output->crtc_id);
> +
> +               drmModeDestroyPropertyBlob(backend->drm.fd, ctm_blobid);
> +       }
> +}

Rather than having this directly applied in the function, I would like
to see this moved to where drm_output_state is applied to an output.
For atomic, this means that it's just another property set with all
the others, making the code a little shorter. Doing the same for gamma
as well would mean that the atomic path gets much shorter, and also
that gamma and CTM updates are synchronised with each other - plus the
pre-CTM degamma LUT if people want to use that, which would be a
pretty trivial addition to the code.

I'm not sure there's much point in supporting CTM on pre-atomic
drivers: I don't know of any drivers which support CTM but not atomic.

In order to do this, you would need to figure out when the gamma/CTM
have changed relative to the current output configuration, and only
apply it then. Maybe you could do this by checking the blob ID?

Cheers,
Daniel